diff mbox series

[2/6] net/hns3: fix compiling error for using SVE algorithm

Message ID 1616507156-35880-3-git-send-email-humin29@huawei.com (mailing list archive)
State Accepted
Delegated to: Ferruh Yigit
Headers show
Series bugfixes for hns3 | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Min Hu (Connor) March 23, 2021, 1:45 p.m. UTC
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

Ferruh Yigit March 29, 2021, 4:10 p.m. UTC | #1
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?
Aaron Conole March 29, 2021, 6:31 p.m. UTC | #2
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.
David Marchand March 29, 2021, 7:42 p.m. UTC | #3
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.
Honnappa Nagarahalli March 29, 2021, 10:13 p.m. UTC | #4
<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
Min Hu (Connor) March 31, 2021, 12:55 a.m. UTC | #5
在 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?
> 
> 
> .
Ferruh Yigit March 31, 2021, 9:14 a.m. UTC | #6
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.
diff mbox series

Patch

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;
 	}