[2/6] net/hns3: fix compiling error for using SVE algorithm
Checks
Commit Message
From: Huisong Li <lihuisong@huawei.com>
The 'queue_full_cnt' stats have been encapsulated in 'dfx_stats'.
However, the modification in the SVE algorithm is omitted.
As a result, the driver fails to be compiled when the SVE
algorithm is used.
Fixes: 9b77f1fe303f ("net/hns3: encapsulate DFX stats in datapath")
Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
drivers/net/hns3/hns3_rxtx_vec_sve.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 3/23/2021 1:45 PM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
>
> The 'queue_full_cnt' stats have been encapsulated in 'dfx_stats'.
> However, the modification in the SVE algorithm is omitted.
> As a result, the driver fails to be compiled when the SVE
> algorithm is used.
>
> Fixes: 9b77f1fe303f ("net/hns3: encapsulate DFX stats in datapath")
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> drivers/net/hns3/hns3_rxtx_vec_sve.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> index f8655fa..e1a1731 100644
> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> @@ -439,7 +439,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict tx_queue,
>
> nb_pkts = RTE_MIN(txq->tx_bd_ready, nb_pkts);
> if (unlikely(nb_pkts == 0)) {
> - txq->queue_full_cnt++;
> + txq->dfx_stats.queue_full_cnt++;
> return 0;
> }
>
>
Hi Connor,
This is a very obvious build error, I am concerned how this is released. Do you
have any internal testing?
+ Aaron & Honnappa,
If we can have a build test in our public CI with SVE?
Ferruh Yigit <ferruh.yigit@intel.com> writes:
> On 3/23/2021 1:45 PM, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> The 'queue_full_cnt' stats have been encapsulated in 'dfx_stats'.
>> However, the modification in the SVE algorithm is omitted.
>> As a result, the driver fails to be compiled when the SVE
>> algorithm is used.
>>
>> Fixes: 9b77f1fe303f ("net/hns3: encapsulate DFX stats in datapath")
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> drivers/net/hns3/hns3_rxtx_vec_sve.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> index f8655fa..e1a1731 100644
>> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> @@ -439,7 +439,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict tx_queue,
>> nb_pkts = RTE_MIN(txq->tx_bd_ready, nb_pkts);
>> if (unlikely(nb_pkts == 0)) {
>> - txq->queue_full_cnt++;
>> + txq->dfx_stats.queue_full_cnt++;
>> return 0;
>> }
>>
>>
>
> Hi Connor,
>
> This is a very obvious build error, I am concerned how this is
> released. Do you have any internal testing?
>
> + Aaron & Honnappa,
>
> If we can have a build test in our public CI with SVE?
Maybe it's possible - we might need to expand the aarch64 cross builds
with a bit of extra information. I'm not sure which compiler is needed,
though. Currently, we install whatever ubuntu18.04 is providing - I
don't know if it has the SVE extensions needed. Honnappa/ARM folks can
provide more information that way - though I would expect it's just a
setting that can be changed in the host_machine or properties section of
the cross-config file.
If that's known (maybe we need to use the thunderx2 config or something
else? I don't know too much about enabling compiler support for SVE),
then we should probably hook it up.
On Mon, Mar 29, 2021 at 8:32 PM Aaron Conole <aconole@redhat.com> wrote:
> Ferruh Yigit <ferruh.yigit@intel.com> writes:
> > This is a very obvious build error, I am concerned how this is
> > released. Do you have any internal testing?
> >
> > + Aaron & Honnappa,
> >
> > If we can have a build test in our public CI with SVE?
>
> Maybe it's possible - we might need to expand the aarch64 cross builds
> with a bit of extra information. I'm not sure which compiler is needed,
> though. Currently, we install whatever ubuntu18.04 is providing - I
> don't know if it has the SVE extensions needed. Honnappa/ARM folks can
> provide more information that way - though I would expect it's just a
> setting that can be changed in the host_machine or properties section of
> the cross-config file.
>
> If that's known (maybe we need to use the thunderx2 config or something
> else? I don't know too much about enabling compiler support for SVE),
> then we should probably hook it up.
Last time I tried SVE builds, I needed a gcc 10 + tweaking cross
compiler config.
http://inbox.dpdk.org/dev/CAJFAV8ywp0qA7qg7cGAhb1ULY72AwjZM6foDBej4roYNE8Oy5Q@mail.gmail.com/
Ruifeng had replied there was a gcc 10 for Ubuntu.
Not sure how others compile with SVE.
<snip>
> Subject: Re: [PATCH 2/6] net/hns3: fix compiling error for using SVE algorithm
>
> On Mon, Mar 29, 2021 at 8:32 PM Aaron Conole <aconole@redhat.com>
> wrote:
> > Ferruh Yigit <ferruh.yigit@intel.com> writes:
> > > This is a very obvious build error, I am concerned how this is
> > > released. Do you have any internal testing?
> > >
> > > + Aaron & Honnappa,
> > >
> > > If we can have a build test in our public CI with SVE?
> >
> > Maybe it's possible - we might need to expand the aarch64 cross builds
> > with a bit of extra information. I'm not sure which compiler is
> > needed, though. Currently, we install whatever ubuntu18.04 is
> > providing - I don't know if it has the SVE extensions needed.
> > Honnappa/ARM folks can provide more information that way - though I
> > would expect it's just a setting that can be changed in the
> > host_machine or properties section of the cross-config file.
> >
> > If that's known (maybe we need to use the thunderx2 config or
> > something else? I don't know too much about enabling compiler support
> > for SVE), then we should probably hook it up.
Since, Kunpeng SoC has SVE enabled, we should have a Kunpeng config [1]. We could use that config to compile the code, through cross compile or through SoC target option for meson scripts[2].
[1] http://patches.dpdk.org/project/dpdk/patch/1616808435-25166-2-git-send-email-oulijun@huawei.com/
[2] http://patches.dpdk.org/project/dpdk/patch/1612361037-12746-3-git-send-email-juraj.linkes@pantheon.tech/
>
> Last time I tried SVE builds, I needed a gcc 10 + tweaking cross compiler
> config.
> http://inbox.dpdk.org/dev/CAJFAV8ywp0qA7qg7cGAhb1ULY72AwjZM6foDB
> ej4roYNE8Oy5Q@mail.gmail.com/
>
> Ruifeng had replied there was a gcc 10 for Ubuntu.
> Not sure how others compile with SVE.
>
> --
> David Marchand
在 2021/3/30 0:10, Ferruh Yigit 写道:
> On 3/23/2021 1:45 PM, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> The 'queue_full_cnt' stats have been encapsulated in 'dfx_stats'.
>> However, the modification in the SVE algorithm is omitted.
>> As a result, the driver fails to be compiled when the SVE
>> algorithm is used.
>>
>> Fixes: 9b77f1fe303f ("net/hns3: encapsulate DFX stats in datapath")
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> drivers/net/hns3/hns3_rxtx_vec_sve.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> index f8655fa..e1a1731 100644
>> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>> @@ -439,7 +439,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict
>> tx_queue,
>> nb_pkts = RTE_MIN(txq->tx_bd_ready, nb_pkts);
>> if (unlikely(nb_pkts == 0)) {
>> - txq->queue_full_cnt++;
>> + txq->dfx_stats.queue_full_cnt++;
>> return 0;
>> }
>>
>
> Hi Connor,
>
> This is a very obvious build error, I am concerned how this is released.
> Do you have any internal testing?
>
Hi Ferruh,
Well, we admit it is our mistake for this issue. Let me
describe the reason:
Firstly, we must declare that we have our inner CI system for
building and testing. While when we upstream the ""support SVE" patch,
our CI does not support SVE building. Instead we build and test SVE on
our local platform.
Then when we upstream the "encapsulate DFX stats in datapath"
patch, we only build it in CI(at that time, SVE building is still not
supported), regardless of SVE building on local platform.
Now, SVE building is supported in our CI， So the building error
occurs.
We'll pay more attention to the issue in the future. Thanks.
> + Aaron & Honnappa,
>
> If we can have a build test in our public CI with SVE?
>
>
> .
On 3/31/2021 1:55 AM, Min Hu (Connor) wrote:
>
>
> 在 2021/3/30 0:10, Ferruh Yigit 写道:
>> On 3/23/2021 1:45 PM, Min Hu (Connor) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> The 'queue_full_cnt' stats have been encapsulated in 'dfx_stats'.
>>> However, the modification in the SVE algorithm is omitted.
>>> As a result, the driver fails to be compiled when the SVE
>>> algorithm is used.
>>>
>>> Fixes: 9b77f1fe303f ("net/hns3: encapsulate DFX stats in datapath")
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> ---
>>> drivers/net/hns3/hns3_rxtx_vec_sve.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c
>>> b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>>> index f8655fa..e1a1731 100644
>>> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
>>> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
>>> @@ -439,7 +439,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict tx_queue,
>>> nb_pkts = RTE_MIN(txq->tx_bd_ready, nb_pkts);
>>> if (unlikely(nb_pkts == 0)) {
>>> - txq->queue_full_cnt++;
>>> + txq->dfx_stats.queue_full_cnt++;
>>> return 0;
>>> }
>>>
>>
>> Hi Connor,
>>
>> This is a very obvious build error, I am concerned how this is released. Do
>> you have any internal testing?
>>
> Hi Ferruh,
> Well, we admit it is our mistake for this issue. Let me
> describe the reason:
> Firstly, we must declare that we have our inner CI system for
> building and testing. While when we upstream the ""support SVE" patch,
> our CI does not support SVE building. Instead we build and test SVE on
> our local platform.
> Then when we upstream the "encapsulate DFX stats in datapath"
> patch, we only build it in CI(at that time, SVE building is still not
> supported), regardless of SVE building on local platform.
> Now, SVE building is supported in our CI， So the building error
> occurs.
> We'll pay more attention to the issue in the future. Thanks.
>
Good to hear your internal testing covers SVE now, thanks for clarification.
@@ -439,7 +439,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict tx_queue,
nb_pkts = RTE_MIN(txq->tx_bd_ready, nb_pkts);
if (unlikely(nb_pkts == 0)) {
- txq->queue_full_cnt++;
+ txq->dfx_stats.queue_full_cnt++;
return 0;
}