* [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
@ 2022-02-28 8:27 Harold Huang
2022-03-04 16:30 ` Ferruh Yigit
2022-03-08 14:35 ` Harold Huang
0 siblings, 2 replies; 6+ messages in thread
From: Harold Huang @ 2022-02-28 8:27 UTC (permalink / raw)
To: dev; +Cc: ophirmu, Harold Huang, stable, Keith Wiles, Raslan Darawsheh
The gso size is calculated with all of the headers and payload. As a
result, the l2 header should not be included when comparing gso size
with mtu.
Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
Cc: stable@dpdk.org
Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
---
drivers/net/tap/rte_eth_tap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f1b48cae82..2b561d232c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
mbuf_in->l4_len;
tso_segsz = mbuf_in->tso_segsz + hdrs_len;
if (unlikely(tso_segsz == hdrs_len) ||
- tso_segsz > *txq->mtu) {
+ tso_segsz > *txq->mtu + mbuf_in->l2_len) {
txq->stats.errs++;
break;
}
--
2.27.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
2022-02-28 8:27 [PATCH] net/tap: do not include l2 header in gso size when compared with mtu Harold Huang
@ 2022-03-04 16:30 ` Ferruh Yigit
2022-03-08 14:35 ` Harold Huang
1 sibling, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2022-03-04 16:30 UTC (permalink / raw)
To: Harold Huang, dev, Ophir Munk, Raslan Darawsheh; +Cc: stable, Keith Wiles
On 2/28/2022 8:27 AM, Harold Huang wrote:
> The gso size is calculated with all of the headers and payload. As a
> result, the l2 header should not be included when comparing gso size
> with mtu.
>
> Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
> Cc: stable@dpdk.org
> Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index f1b48cae82..2b561d232c 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> mbuf_in->l4_len;
> tso_segsz = mbuf_in->tso_segsz + hdrs_len;
> if (unlikely(tso_segsz == hdrs_len) ||
> - tso_segsz > *txq->mtu) {
> + tso_segsz > *txq->mtu + mbuf_in->l2_len) {
> txq->stats.errs++;
> break;
> }
update emails for Ophir & Raslan.
Hi Ophir, since original code is from you can you please
help on review?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
2022-02-28 8:27 [PATCH] net/tap: do not include l2 header in gso size when compared with mtu Harold Huang
2022-03-04 16:30 ` Ferruh Yigit
@ 2022-03-08 14:35 ` Harold Huang
2022-05-20 22:08 ` Ferruh Yigit
2023-10-17 16:47 ` Stephen Hemminger
1 sibling, 2 replies; 6+ messages in thread
From: Harold Huang @ 2022-03-08 14:35 UTC (permalink / raw)
To: dev; +Cc: ophirmu, stable, Keith Wiles, Raslan Darawsheh, jiayu.hu
On Mon, Feb 28, 2022 at 4:27 PM Harold Huang <baymaxhuang@gmail.com> wrote:
>
> The gso size is calculated with all of the headers and payload. As a
> result, the l2 header should not be included when comparing gso size
> with mtu.
>
> Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
> Cc: stable@dpdk.org
> Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index f1b48cae82..2b561d232c 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> mbuf_in->l4_len;
> tso_segsz = mbuf_in->tso_segsz + hdrs_len;
> if (unlikely(tso_segsz == hdrs_len) ||
> - tso_segsz > *txq->mtu) {
> + tso_segsz > *txq->mtu + mbuf_in->l2_len) {
> txq->stats.errs++;
> break;
> }
> --
> 2.27.0
>
Hi, Jiayu,
This is the only example in the driver to use GSO. I think it is
important for us to calculate a correct GSO size. I see you are the
GSO lib maintainer, could you please help review this patch?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
2022-03-08 14:35 ` Harold Huang
@ 2022-05-20 22:08 ` Ferruh Yigit
2022-05-24 14:01 ` Ophir Munk
2023-10-17 16:47 ` Stephen Hemminger
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2022-05-20 22:08 UTC (permalink / raw)
To: Harold Huang, dev, jiayu.hu, Ophir Munk, Raslan Darawsheh
Cc: stable, Keith Wiles
On 3/8/2022 2:35 PM, Harold Huang wrote:
> On Mon, Feb 28, 2022 at 4:27 PM Harold Huang <baymaxhuang@gmail.com> wrote:
>>
>> The gso size is calculated with all of the headers and payload. As a
>> result, the l2 header should not be included when comparing gso size
>> with mtu.
>>
>> Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
>> Cc: stable@dpdk.org
>> Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index f1b48cae82..2b561d232c 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>> mbuf_in->l4_len;
>> tso_segsz = mbuf_in->tso_segsz + hdrs_len;
>> if (unlikely(tso_segsz == hdrs_len) ||
>> - tso_segsz > *txq->mtu) {
>> + tso_segsz > *txq->mtu + mbuf_in->l2_len) {
>> txq->stats.errs++;
>> break;
>> }
>> --
>> 2.27.0
>>
>
> Hi, Jiayu,
>
> This is the only example in the driver to use GSO. I think it is
> important for us to calculate a correct GSO size. I see you are the
> GSO lib maintainer, could you please help review this patch?
Hi Jiayu, Ophir,
Can you please review this patch?
For reference, patchwork link:
https://patches.dpdk.org/project/dpdk/patch/20220228082724.1646930-1-baymaxhuang@gmail.com/
Thanks,
ferruh
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
2022-05-20 22:08 ` Ferruh Yigit
@ 2022-05-24 14:01 ` Ophir Munk
0 siblings, 0 replies; 6+ messages in thread
From: Ophir Munk @ 2022-05-24 14:01 UTC (permalink / raw)
To: Ferruh Yigit, Harold Huang, dev, jiayu.hu, Raslan Darawsheh
Cc: stable, Keith Wiles, NBU-Contact-Thomas Monjalon (EXTERNAL)
Hi all,
Please note that max size is calculated in the same function (drivers/net/tap/rte_eth_tap.c) as:
max_size = *txq->mtu + (RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN + 4);
Since tso_setgsz should not exceed the max packet size - the desired update should be:
tso_segsz > max_size
rather than the suggested one:
tso_segsz > *txq->mtu + mbuf_in->l2_len)
Regards,
Ophir
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Saturday, May 21, 2022 1:08 AM
> To: Harold Huang <baymaxhuang@gmail.com>; dev@dpdk.org;
> jiayu.hu@intel.com; Ophir Munk <ophirmu@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>
> Cc: stable@dpdk.org; Keith Wiles <keith.wiles@intel.com>
> Subject: Re: [PATCH] net/tap: do not include l2 header in gso size when
> compared with mtu
>
> On 3/8/2022 2:35 PM, Harold Huang wrote:
> > On Mon, Feb 28, 2022 at 4:27 PM Harold Huang
> <baymaxhuang@gmail.com> wrote:
> >>
> >> The gso size is calculated with all of the headers and payload. As a
> >> result, the l2 header should not be included when comparing gso size
> >> with mtu.
> >>
> >> Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
> >> Cc: stable@dpdk.org
> >> Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
> >> ---
> >> drivers/net/tap/rte_eth_tap.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/tap/rte_eth_tap.c
> >> b/drivers/net/tap/rte_eth_tap.c index f1b48cae82..2b561d232c 100644
> >> --- a/drivers/net/tap/rte_eth_tap.c
> >> +++ b/drivers/net/tap/rte_eth_tap.c
> >> @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf
> **bufs, uint16_t nb_pkts)
> >> mbuf_in->l4_len;
> >> tso_segsz = mbuf_in->tso_segsz + hdrs_len;
> >> if (unlikely(tso_segsz == hdrs_len) ||
> >> - tso_segsz > *txq->mtu) {
> >> + tso_segsz > *txq->mtu +
> >> + mbuf_in->l2_len) {
> >> txq->stats.errs++;
> >> break;
> >> }
> >> --
> >> 2.27.0
> >>
> >
> > Hi, Jiayu,
> >
> > This is the only example in the driver to use GSO. I think it is
> > important for us to calculate a correct GSO size. I see you are the
> > GSO lib maintainer, could you please help review this patch?
>
>
> Hi Jiayu, Ophir,
>
> Can you please review this patch?
>
> For reference, patchwork link:
> https://patches.dpdk.org/project/dpdk/patch/20220228082724.1646930-1-
> baymaxhuang@gmail.com/
>
> Thanks,
> ferruh
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu
2022-03-08 14:35 ` Harold Huang
2022-05-20 22:08 ` Ferruh Yigit
@ 2023-10-17 16:47 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2023-10-17 16:47 UTC (permalink / raw)
To: Harold Huang
Cc: dev, ophirmu, stable, Keith Wiles, Raslan Darawsheh, jiayu.hu
On Tue, 8 Mar 2022 22:35:18 +0800
Harold Huang <baymaxhuang@gmail.com> wrote:
> On Mon, Feb 28, 2022 at 4:27 PM Harold Huang <baymaxhuang@gmail.com> wrote:
> >
> > The gso size is calculated with all of the headers and payload. As a
> > result, the l2 header should not be included when comparing gso size
> > with mtu.
> >
> > Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
> > Cc: stable@dpdk.org
> > Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
> > ---
> > drivers/net/tap/rte_eth_tap.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> > index f1b48cae82..2b561d232c 100644
> > --- a/drivers/net/tap/rte_eth_tap.c
> > +++ b/drivers/net/tap/rte_eth_tap.c
> > @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> > mbuf_in->l4_len;
> > tso_segsz = mbuf_in->tso_segsz + hdrs_len;
> > if (unlikely(tso_segsz == hdrs_len) ||
> > - tso_segsz > *txq->mtu) {
> > + tso_segsz > *txq->mtu + mbuf_in->l2_len) {
> > txq->stats.errs++;
> > break;
> > }
> > --
> > 2.27.0
> >
>
> Hi, Jiayu,
>
> This is the only example in the driver to use GSO. I think it is
> important for us to calculate a correct GSO size. I see you are the
> GSO lib maintainer, could you please help review this patch?
See Ophir's comment and address it in new version
Please note that max size is calculated in the same function (drivers/net/tap/rte_eth_tap.c) as:
max_size = *txq->mtu + (RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN + 4);
Since tso_setgsz should not exceed the max packet size - the desired update should be:
tso_segsz > max_size
rather than the suggested one:
tso_segsz > *txq->mtu + mbuf_in->l2_len)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-17 16:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 8:27 [PATCH] net/tap: do not include l2 header in gso size when compared with mtu Harold Huang
2022-03-04 16:30 ` Ferruh Yigit
2022-03-08 14:35 ` Harold Huang
2022-05-20 22:08 ` Ferruh Yigit
2022-05-24 14:01 ` Ophir Munk
2023-10-17 16:47 ` Stephen Hemminger
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).