[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACKFLi=nJgVLC+PaXsyGFd4x+QbbMrk6_hRY9APqshrP0xo31w@mail.gmail.com>
Date: Mon, 5 Dec 2022 14:47:44 -0800
From: Michael Chan <michael.chan@...adcom.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Coco Li <lixiaoyan@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
David Ahern <dsahern@...nel.org>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC net-next v4 2/2] bnxt: Use generic HBH removal helper in tx path
On Sun, Dec 4, 2022 at 9:14 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Fri, Dec 2, 2022 at 11:12 PM Coco Li <lixiaoyan@...gle.com> wrote:
> >
> > Eric Dumazet implemented Big TCP that allowed bigger TSO/GRO packet sizes
> > for IPv6 traffic. See patch series:
> > 'commit 89527be8d8d6 ("net: add IFLA_TSO_{MAX_SIZE|SEGS} attributes")'
> >
> > This reduces the number of packets traversing the networking stack and
> > should usually improves performance. However, it also inserts a
> > temporary Hop-by-hop IPv6 extension header.
> >
> > Using the HBH header removal method in the previous path, the extra header
> > be removed in bnxt drivers to allow it to send big TCP packets (bigger
> > TSO packets) as well.
> >
> > Tested:
> > Compiled locally
> >
> > To further test functional correctness, update the GSO/GRO limit on the
> > physical NIC:
> >
> > ip link set eth0 gso_max_size 181000
> > ip link set eth0 gro_max_size 181000
> >
> > Note that if there are bonding or ipvan devices on top of the physical
> > NIC, their GSO sizes need to be updated as well.
> >
> > Then, IPv6/TCP packets with sizes larger than 64k can be observed.
> >
> > Big TCP functionality is tested by Michael, feature checks not yet.
> >
> > Tested by Michael:
> > I've confirmed with our hardware team that this is supported by our
> > chips, and I've tested it up to gso_max_size of 524280. Thanks.
> >
> > Tested-by: Michael Chan <michael.chan@...adcom.com>
> > Reviewed-by: Michael Chan <michael.chan@...adcom.com>
> > Signed-off-by: Coco Li <lixiaoyan@...gle.com>
> > ---
> > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 26 ++++++++++++++++++++++-
> > 1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 0fe164b42c5d..c2713cb5debd 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > @@ -389,6 +389,9 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > return NETDEV_TX_BUSY;
> > }
> >
> > + if (unlikely(ipv6_hopopt_jumbo_remove(skb)))
> > + goto tx_free;
> > +
> > length = skb->len;
> > len = skb_headlen(skb);
> > last_frag = skb_shinfo(skb)->nr_frags;
> > @@ -11342,9 +11345,28 @@ static bool bnxt_exthdr_check(struct bnxt *bp, struct sk_buff *skb, int nw_off,
> >
> > if (hdrlen > 64)
> > return false;
> > +
> > + /* The ext header may be a hop-by-hop header inserted for
> > + * big TCP purposes. This will be removed before sending
> > + * from NIC, so do not count it.
> > + */
> > + if (*nexthdr == NEXTHDR_HOP) {
> > + if (likely(skb->len <= GRO_LEGACY_MAX_SIZE))
> > + goto increment_hdr;
> > +
> > + struct hop_jumbo_hdr *jhdr = (struct hop_jumbo_hdr *)(nexthdr + hdrlen);
>
> We discourage adding a variable declaration in the middle of code.
>
This doesn't work. Initially nexthdr points to the nexthdr in the
ipv6 header. It should be coded like this:
struct hop_jumbo_hdr *jhdr = (struct hop_jumbo_hdr *)hp;
Thanks.
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)
Powered by blists - more mailing lists