DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/iavf: fix segmentation offload buffer size
@ 2022-02-10 13:09 Radu Nicolau
  2022-02-15  6:16 ` Huang, ZhiminX
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Radu Nicolau @ 2022-02-10 13:09 UTC (permalink / raw)
  To: Jingjing Wu, Beilei Xing; +Cc: dev, yux.jiang, Radu Nicolau

This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
being computed for TSO packets.

Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/iavf/iavf_rxtx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index b0fed1b992..ba272bb211 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -2464,9 +2464,10 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
 	desc->buffer_addr = rte_mbuf_data_iova(m);
 
 	/* calculate data buffer size less set header lengths */
-	if (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG)) {
-		if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
-			hdrlen += m->outer_l3_len;
+	if ((m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) &&
+			(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
+					RTE_MBUF_F_TX_UDP_SEG))) {
+		hdrlen += m->outer_l3_len;
 		if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
 			hdrlen += m->l3_len + m->l4_len;
 		else
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-10 13:09 [PATCH] net/iavf: fix segmentation offload buffer size Radu Nicolau
@ 2022-02-15  6:16 ` Huang, ZhiminX
  2022-02-15  7:59   ` Zhang, Qi Z
  2022-02-15 10:06 ` Ferruh Yigit
  2022-02-15 15:50 ` [PATCH v2] " Radu Nicolau
  2 siblings, 1 reply; 14+ messages in thread
From: Huang, ZhiminX @ 2022-02-15  6:16 UTC (permalink / raw)
  To: Nicolau, Radu, Wu, Jingjing, Xing, Beilei; +Cc: dev, Jiang, YuX, Nicolau, Radu

> -----Original Message-----
> From: Radu Nicolau <radu.nicolau@intel.com>
> Sent: Thursday, February 10, 2022 9:09 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Jiang, YuX <yux.jiang@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>
> Subject: [PATCH] net/iavf: fix segmentation offload buffer size
> 
> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size being
> computed for TSO packets.
> 
> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
Tested-by: Zhimin Huang <zhiminx.huang@intel.com >




^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15  6:16 ` Huang, ZhiminX
@ 2022-02-15  7:59   ` Zhang, Qi Z
  0 siblings, 0 replies; 14+ messages in thread
From: Zhang, Qi Z @ 2022-02-15  7:59 UTC (permalink / raw)
  To: Huang, ZhiminX, Nicolau, Radu, Wu, Jingjing, Xing, Beilei
  Cc: dev, Jiang, YuX, Nicolau, Radu



> -----Original Message-----
> From: Huang, ZhiminX <zhiminx.huang@intel.com>
> Sent: Tuesday, February 15, 2022 2:17 PM
> To: Nicolau, Radu <radu.nicolau@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Jiang, YuX <yux.jiang@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>
> Subject: RE: [PATCH] net/iavf: fix segmentation offload buffer size
> 
> > -----Original Message-----
> > From: Radu Nicolau <radu.nicolau@intel.com>
> > Sent: Thursday, February 10, 2022 9:09 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Jiang, YuX <yux.jiang@intel.com>; Nicolau, Radu
> > <radu.nicolau@intel.com>
> > Subject: [PATCH] net/iavf: fix segmentation offload buffer size
> >
> > This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer
> > size being computed for TSO packets.
> >
> > Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
> >
> > Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> > ---
> >  drivers/net/iavf/iavf_rxtx.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> Tested-by: Zhimin Huang <zhiminx.huang@intel.com >

Applied to dpdk-next-net-intel.

Thanks
Qi
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-10 13:09 [PATCH] net/iavf: fix segmentation offload buffer size Radu Nicolau
  2022-02-15  6:16 ` Huang, ZhiminX
@ 2022-02-15 10:06 ` Ferruh Yigit
  2022-02-15 10:46   ` Kevin Traynor
  2022-02-15 10:47   ` Nicolau, Radu
  2022-02-15 15:50 ` [PATCH v2] " Radu Nicolau
  2 siblings, 2 replies; 14+ messages in thread
From: Ferruh Yigit @ 2022-02-15 10:06 UTC (permalink / raw)
  To: Radu Nicolau, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi

On 2/10/2022 1:09 PM, Radu Nicolau wrote:
> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
> being computed for TSO packets.
> 
> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
> 

Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
are we having the initial issue back when this patch reverted?
Is there a fix for that initial issue?

+Kevin, Luca,
Since this patch reverts a fix done after 21.11, is there a way
to stop that fix to be merged for 21.11.x?

> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>   drivers/net/iavf/iavf_rxtx.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
> index b0fed1b992..ba272bb211 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -2464,9 +2464,10 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
>   	desc->buffer_addr = rte_mbuf_data_iova(m);
>   
>   	/* calculate data buffer size less set header lengths */
> -	if (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG)) {
> -		if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
> -			hdrlen += m->outer_l3_len;
> +	if ((m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) &&
> +			(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
> +					RTE_MBUF_F_TX_UDP_SEG))) {
> +		hdrlen += m->outer_l3_len;
>   		if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
>   			hdrlen += m->l3_len + m->l4_len;
>   		else


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 10:06 ` Ferruh Yigit
@ 2022-02-15 10:46   ` Kevin Traynor
  2022-02-15 11:25     ` Ferruh Yigit
  2022-02-15 10:47   ` Nicolau, Radu
  1 sibling, 1 reply; 14+ messages in thread
From: Kevin Traynor @ 2022-02-15 10:46 UTC (permalink / raw)
  To: Ferruh Yigit, Radu Nicolau, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Luca Boccassi

On 15/02/2022 10:06, Ferruh Yigit wrote:
> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
>> being computed for TSO packets.
>>
>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>
> 
> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
> are we having the initial issue back when this patch reverted?
> Is there a fix for that initial issue?
> 
> +Kevin, Luca,
> Since this patch reverts a fix done after 21.11, is there a way
> to stop that fix to be merged for 21.11.x?
> 

This revert patch would show up for us as a relevant one to backport, so 
in the worst case where initial fix was already pushed to dpdk.org, the 
revert would be taken too and the net result would be ok by release time.

In this case, as the initial fix is not pushed to dpdk.org stable 
branches yet and as you have let us know, we can just make a note to 
skip it, or remove from a personal staging repo if already backported there.

I'll add an email filter for 'reverts' to catch this so you won't have 
to ping manually, but feel free to do that as I suspect (hope :-)) it's 
only a handful of patches per release.

Thanks for highlighting, Kevin.

>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>>    drivers/net/iavf/iavf_rxtx.c | 7 ++++---
>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
>> index b0fed1b992..ba272bb211 100644
>> --- a/drivers/net/iavf/iavf_rxtx.c
>> +++ b/drivers/net/iavf/iavf_rxtx.c
>> @@ -2464,9 +2464,10 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
>>    	desc->buffer_addr = rte_mbuf_data_iova(m);
>>    
>>    	/* calculate data buffer size less set header lengths */
>> -	if (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG)) {
>> -		if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
>> -			hdrlen += m->outer_l3_len;
>> +	if ((m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) &&
>> +			(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
>> +					RTE_MBUF_F_TX_UDP_SEG))) {
>> +		hdrlen += m->outer_l3_len;
>>    		if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
>>    			hdrlen += m->l3_len + m->l4_len;
>>    		else
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 10:06 ` Ferruh Yigit
  2022-02-15 10:46   ` Kevin Traynor
@ 2022-02-15 10:47   ` Nicolau, Radu
  2022-02-15 11:19     ` Ferruh Yigit
  1 sibling, 1 reply; 14+ messages in thread
From: Nicolau, Radu @ 2022-02-15 10:47 UTC (permalink / raw)
  To: Ferruh Yigit, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi


On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer 
>> size
>> being computed for TSO packets.
>>
>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>
>
> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
> are we having the initial issue back when this patch reverted?
> Is there a fix for that initial issue?
The initial issue was around a very specific usecase - IPsec transport 
mode TSO and it is very likely back with this revert. But it's a much 
less severe impact than general traffic TSO.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 10:47   ` Nicolau, Radu
@ 2022-02-15 11:19     ` Ferruh Yigit
  2022-02-15 12:30       ` Nicolau, Radu
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-02-15 11:19 UTC (permalink / raw)
  To: Nicolau, Radu, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi

On 2/15/2022 10:47 AM, Nicolau, Radu wrote:
> 
> On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
>>> being computed for TSO packets.
>>>
>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>>
>>
>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>> are we having the initial issue back when this patch reverted?
>> Is there a fix for that initial issue?
> The initial issue was around a very specific usecase - IPsec transport mode TSO and it is very likely back with this revert. But it's a much less severe impact than general traffic TSO.

Got it, so this revert brings back an issue, should
we document this in the commit log of this patch?
Plus should we create a Bugzilla etc to record the
introduced issue?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 10:46   ` Kevin Traynor
@ 2022-02-15 11:25     ` Ferruh Yigit
  0 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2022-02-15 11:25 UTC (permalink / raw)
  To: Kevin Traynor, Radu Nicolau, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Luca Boccassi

On 2/15/2022 10:46 AM, Kevin Traynor wrote:
> On 15/02/2022 10:06, Ferruh Yigit wrote:
>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
>>> being computed for TSO packets.
>>>
>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>>
>>
>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>> are we having the initial issue back when this patch reverted?
>> Is there a fix for that initial issue?
>>
>> +Kevin, Luca,
>> Since this patch reverts a fix done after 21.11, is there a way
>> to stop that fix to be merged for 21.11.x?
>>
> 
> This revert patch would show up for us as a relevant one to backport, so in the worst case where initial fix was already pushed to dpdk.org, the revert would be taken too and the net result would be ok by release time.
> 
> In this case, as the initial fix is not pushed to dpdk.org stable branches yet and as you have let us know, we can just make a note to skip it, or remove from a personal staging repo if already backported there.
> 

ack

> I'll add an email filter for 'reverts' to catch this so you won't have to ping manually, but feel free to do that as I suspect (hope :-)) it's only a handful of patches per release.
> 

Ack.
I was asking for a method, you adding a filter is a way,
but if there is another defined method we (maintainers) can
do it to help LTS maintainers.
Like replying to a patch in stable list with a keyword, like
'do not apply this' to let your scripts to detect it ...

> Thanks for highlighting, Kevin.
> 
>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>> ---
>>>    drivers/net/iavf/iavf_rxtx.c | 7 ++++---
>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
>>> index b0fed1b992..ba272bb211 100644
>>> --- a/drivers/net/iavf/iavf_rxtx.c
>>> +++ b/drivers/net/iavf/iavf_rxtx.c
>>> @@ -2464,9 +2464,10 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
>>>        desc->buffer_addr = rte_mbuf_data_iova(m);
>>>        /* calculate data buffer size less set header lengths */
>>> -    if (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG)) {
>>> -        if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
>>> -            hdrlen += m->outer_l3_len;
>>> +    if ((m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) &&
>>> +            (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
>>> +                    RTE_MBUF_F_TX_UDP_SEG))) {
>>> +        hdrlen += m->outer_l3_len;
>>>            if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
>>>                hdrlen += m->l3_len + m->l4_len;
>>>            else
>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 11:19     ` Ferruh Yigit
@ 2022-02-15 12:30       ` Nicolau, Radu
  2022-02-15 12:44         ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolau, Radu @ 2022-02-15 12:30 UTC (permalink / raw)
  To: Ferruh Yigit, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi


On 2/15/2022 11:19 AM, Ferruh Yigit wrote:
> On 2/15/2022 10:47 AM, Nicolau, Radu wrote:
>>
>> On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
>>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect 
>>>> buffer size
>>>> being computed for TSO packets.
>>>>
>>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>>>
>>>
>>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>>> are we having the initial issue back when this patch reverted?
>>> Is there a fix for that initial issue?
>> The initial issue was around a very specific usecase - IPsec 
>> transport mode TSO and it is very likely back with this revert. But 
>> it's a much less severe impact than general traffic TSO.
>
> Got it, so this revert brings back an issue, should
> we document this in the commit log of this patch?
> Plus should we create a Bugzilla etc to record the
> introduced issue?

I plan to follow up with a fix shortly.

As for the commit message, since this was not merged to the LTS and 
main, do we really need it?



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 12:30       ` Nicolau, Radu
@ 2022-02-15 12:44         ` Ferruh Yigit
  2022-02-15 12:48           ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-02-15 12:44 UTC (permalink / raw)
  To: Nicolau, Radu, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi

On 2/15/2022 12:30 PM, Nicolau, Radu wrote:
> 
> On 2/15/2022 11:19 AM, Ferruh Yigit wrote:
>> On 2/15/2022 10:47 AM, Nicolau, Radu wrote:
>>>
>>> On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
>>>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
>>>>> being computed for TSO packets.
>>>>>
>>>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>>>>
>>>>
>>>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>>>> are we having the initial issue back when this patch reverted?
>>>> Is there a fix for that initial issue?
>>> The initial issue was around a very specific usecase - IPsec transport mode TSO and it is very likely back with this revert. But it's a much less severe impact than general traffic TSO.
>>
>> Got it, so this revert brings back an issue, should
>> we document this in the commit log of this patch?
>> Plus should we create a Bugzilla etc to record the
>> introduced issue?
> 
> I plan to follow up with a fix shortly.
> 
> As for the commit message, since this was not merged to the LTS and main, do we really need it?
> 

Not for the LTS but for main repo. We know merging this patch
will introduce an previous issue back but that is not obvious
from the commit log.

I think it helps to document, in commit log, what issue this
commit brings.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 12:44         ` Ferruh Yigit
@ 2022-02-15 12:48           ` Ferruh Yigit
  2022-02-15 15:52             ` Nicolau, Radu
  0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-02-15 12:48 UTC (permalink / raw)
  To: Nicolau, Radu, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi

On 2/15/2022 12:44 PM, Ferruh Yigit wrote:
> On 2/15/2022 12:30 PM, Nicolau, Radu wrote:
>>
>> On 2/15/2022 11:19 AM, Ferruh Yigit wrote:
>>> On 2/15/2022 10:47 AM, Nicolau, Radu wrote:
>>>>
>>>> On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
>>>>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>>>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect buffer size
>>>>>> being computed for TSO packets.
>>>>>>
>>>>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
>>>>>>
>>>>>
>>>>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>>>>> are we having the initial issue back when this patch reverted?
>>>>> Is there a fix for that initial issue?
>>>> The initial issue was around a very specific usecase - IPsec transport mode TSO and it is very likely back with this revert. But it's a much less severe impact than general traffic TSO.
>>>
>>> Got it, so this revert brings back an issue, should
>>> we document this in the commit log of this patch?
>>> Plus should we create a Bugzilla etc to record the
>>> introduced issue?
>>
>> I plan to follow up with a fix shortly.
>>

Or does it make sense to combine that fix with this patch?

>> As for the commit message, since this was not merged to the LTS and main, do we really need it?
>>
> 
> Not for the LTS but for main repo. We know merging this patch
> will introduce an previous issue back but that is not obvious
> from the commit log.
> 
> I think it helps to document, in commit log, what issue this
> commit brings.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] net/iavf: fix segmentation offload buffer size
  2022-02-10 13:09 [PATCH] net/iavf: fix segmentation offload buffer size Radu Nicolau
  2022-02-15  6:16 ` Huang, ZhiminX
  2022-02-15 10:06 ` Ferruh Yigit
@ 2022-02-15 15:50 ` Radu Nicolau
  2022-02-20  5:35   ` Zhang, Qi Z
  2 siblings, 1 reply; 14+ messages in thread
From: Radu Nicolau @ 2022-02-15 15:50 UTC (permalink / raw)
  To: Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, zhiminx.huang, ferruh.yigit, ingjing.wu, bluca,
	ktraynor, Radu Nicolau

This fix commit ff8b8bcd2ebe, which resulted in incorrect buffer size
being computed for non IPses TSO packets.

Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v2: reworked it to fix the initial issue instead of just reverting ff8b8bcd2ebe

 drivers/net/iavf/iavf_rxtx.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 41244055e5..ca54c70dd0 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -2463,9 +2463,10 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
 	desc->buffer_addr = rte_mbuf_data_iova(m);
 
 	/* calculate data buffer size less set header lengths */
-	if (m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_UDP_SEG)) {
-		if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK)
-			hdrlen += m->outer_l3_len;
+	if ((m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK) &&
+			(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
+					RTE_MBUF_F_TX_UDP_SEG))) {
+		hdrlen += m->outer_l3_len;
 		if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
 			hdrlen += m->l3_len + m->l4_len;
 		else
@@ -2473,6 +2474,14 @@ iavf_fill_data_desc(volatile struct iavf_tx_desc *desc,
 		if (m->ol_flags & RTE_MBUF_F_TX_SEC_OFFLOAD)
 			hdrlen += ipseclen;
 		bufsz = hdrlen + tlen;
+	} else if ((m->ol_flags & RTE_MBUF_F_TX_SEC_OFFLOAD) &&
+			(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
+					RTE_MBUF_F_TX_UDP_SEG))) {
+		hdrlen += m->outer_l3_len + m->l3_len + ipseclen;
+		if (m->ol_flags & RTE_MBUF_F_TX_L4_MASK)
+			hdrlen += m->l4_len;
+		bufsz = hdrlen + tlen;
+
 	} else {
 		bufsz = m->data_len;
 	}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] net/iavf: fix segmentation offload buffer size
  2022-02-15 12:48           ` Ferruh Yigit
@ 2022-02-15 15:52             ` Nicolau, Radu
  0 siblings, 0 replies; 14+ messages in thread
From: Nicolau, Radu @ 2022-02-15 15:52 UTC (permalink / raw)
  To: Ferruh Yigit, Jingjing Wu, Beilei Xing
  Cc: dev, yux.jiang, Kevin Traynor, Luca Boccassi


On 2/15/2022 12:48 PM, Ferruh Yigit wrote:
> On 2/15/2022 12:44 PM, Ferruh Yigit wrote:
>> On 2/15/2022 12:30 PM, Nicolau, Radu wrote:
>>>
>>> On 2/15/2022 11:19 AM, Ferruh Yigit wrote:
>>>> On 2/15/2022 10:47 AM, Nicolau, Radu wrote:
>>>>>
>>>>> On 2/15/2022 10:06 AM, Ferruh Yigit wrote:
>>>>>> On 2/10/2022 1:09 PM, Radu Nicolau wrote:
>>>>>>> This reverts commit ff8b8bcd2ebe, which resulted in incorrect 
>>>>>>> buffer size
>>>>>>> being computed for TSO packets.
>>>>>>>
>>>>>>> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload 
>>>>>>> condition")
>>>>>>>
>>>>>>
>>>>>> Commit 'ff8b8bcd2ebe' was already fixing segmentation offload,
>>>>>> are we having the initial issue back when this patch reverted?
>>>>>> Is there a fix for that initial issue?
>>>>> The initial issue was around a very specific usecase - IPsec 
>>>>> transport mode TSO and it is very likely back with this revert. 
>>>>> But it's a much less severe impact than general traffic TSO.
>>>>
>>>> Got it, so this revert brings back an issue, should
>>>> we document this in the commit log of this patch?
>>>> Plus should we create a Bugzilla etc to record the
>>>> introduced issue?
>>>
>>> I plan to follow up with a fix shortly.
>>>
>
> Or does it make sense to combine that fix with this patch?
>
I sent a v2 that does this. Sorry all for the mess :(

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH v2] net/iavf: fix segmentation offload buffer size
  2022-02-15 15:50 ` [PATCH v2] " Radu Nicolau
@ 2022-02-20  5:35   ` Zhang, Qi Z
  0 siblings, 0 replies; 14+ messages in thread
From: Zhang, Qi Z @ 2022-02-20  5:35 UTC (permalink / raw)
  To: Nicolau, Radu, Wu, Jingjing, Xing, Beilei
  Cc: dev, Jiang, YuX, Huang,  ZhiminX, Yigit, Ferruh, ingjing.wu,
	bluca, ktraynor, Nicolau, Radu



> -----Original Message-----
> From: Radu Nicolau <radu.nicolau@intel.com>
> Sent: Tuesday, February 15, 2022 11:50 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Jiang, YuX <yux.jiang@intel.com>; Huang, ZhiminX
> <zhiminx.huang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> ingjing.wu@intel.com; bluca@debian.org; ktraynor@redhat.com; Nicolau, Radu
> <radu.nicolau@intel.com>
> Subject: [PATCH v2] net/iavf: fix segmentation offload buffer size
> 
> This fix commit ff8b8bcd2ebe, which resulted in incorrect buffer size being
> computed for non IPses TSO packets.
> 
> Fixes: ff8b8bcd2ebe ("net/iavf: fix segmentation offload condition")
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-02-20  5:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 13:09 [PATCH] net/iavf: fix segmentation offload buffer size Radu Nicolau
2022-02-15  6:16 ` Huang, ZhiminX
2022-02-15  7:59   ` Zhang, Qi Z
2022-02-15 10:06 ` Ferruh Yigit
2022-02-15 10:46   ` Kevin Traynor
2022-02-15 11:25     ` Ferruh Yigit
2022-02-15 10:47   ` Nicolau, Radu
2022-02-15 11:19     ` Ferruh Yigit
2022-02-15 12:30       ` Nicolau, Radu
2022-02-15 12:44         ` Ferruh Yigit
2022-02-15 12:48           ` Ferruh Yigit
2022-02-15 15:52             ` Nicolau, Radu
2022-02-15 15:50 ` [PATCH v2] " Radu Nicolau
2022-02-20  5:35   ` Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).