mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Damato <joe@dama.to>
To: Michael Chan <michael.chan@broadcom.com>
Cc: netdev@vger.kernel.org, Pavan Chebbi <pavan.chebbi@broadcom.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	edumazet@google.com, horms@kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH net 2/2] bnxt_en: Pad short SW USO segments to BNXT_MIN_PKT_SIZE
Date: Fri, 9 Oct 2026 10:33:08 -0700	[thread overview]
Message-ID: <asklVFvUJx8P/k1d@devvm20253.cco0.facebook.com> (raw)
In-Reply-To: <CACKFLikWkH6QaOG=EO_zu8cqCFRxGqtA2-+R-btp0TwUbJjVBg@mail.gmail.com>

On Thu, Oct 08, 2026 at 10:26:06PM -0700, Michael Chan wrote:
> On Thu, Oct 8, 2026 at 12:11 PM Joe Damato <joe@dama.to> wrote:
> 
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
> > index ef04c9d08066..f87e02ef3033 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
> > @@ -82,11 +82,14 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
> >         if (unlikely(num_segs <= 1))
> >                 goto drop;
> >
> > +       if (unlikely(bnxt_sw_gso_pad_len(hdr_len, mss)))
> > +               goto drop;
> > +
> 
> This looks redundant since you already have the same check in
> bnxt_features_check().

It's redundant for skbs that go through validate_xmit_skb(), but generic XDP
doesn't AFAIU. Not sure how else to handle this case, but open to suggestions.

If you agree and are OK with dropping these, then I could add a comment
clarifying this in the v2. LMK what you think.

> >         /* Upper bound on the number of descriptors needed.
> >          *
> >          * Each segment uses 1 long BD + 1 ext BD + payload BDs, which is
> >          * at most num_segs + nr_frags (each frag boundary crossing adds at
> > -        * most 1 extra BD).
> > +        * most 1 extra BD). The last segment may need 1 pad BD.
> >          */
> >         bds_needed = 3 * num_segs + skb_shinfo(skb)->nr_frags + 1;
> 
> I don't quite understand why we don't need to add 1 more for possible
> padding here.

Re-reading the code, I think the +1 was never needed in the first place.
Payload BDs come from at most 1 + nr_frags DMA regions. Going from one region
to the next adds at most one BD on top of one BD per segment. There are
nr_frags boundaries at most, so I think the math is:

  2*num_segs + num_segs + nr_frags

which simplifies to 3*num_segs + nr_frags.

I would appreciate a double check on the math :) but I think the +1 from the
original code was actually unnecessary and so for this change we can use it
for the pad BD.

Shockingly: it seems sashiko didn't find any issues (?). If you agree with the
above I can respin to add the comment for the generic XDP path and re-send
later today.

Thanks,
Joe

  reply	other threads:[~2026-10-09 17:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-08 19:11 [PATCH net 0/2] bnxt_en: Fix SW USO padding Joe Damato
2026-10-08 19:11 ` [PATCH net 1/2] bnxt_en: Add helper to fill SW USO payload BDs Joe Damato
2026-10-08 19:11 ` [PATCH net 2/2] bnxt_en: Pad short SW USO segments to BNXT_MIN_PKT_SIZE Joe Damato
2026-10-09  5:26   ` Michael Chan
2026-10-09 17:33     ` Joe Damato [this message]
2026-10-09 18:33       ` Michael Chan
2026-10-09 19:45         ` Joe Damato
2026-10-09 20:07           ` Michael Chan
2026-10-09 20:23             ` Joe Damato
2026-10-09 20:28               ` Michael Chan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=asklVFvUJx8P/k1d@devvm20253.cco0.facebook.com \
    --to=joe@dama.to \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavan.chebbi@broadcom.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®