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
next prev parent 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®