Re: [PATCH net 2/2] bnxt_en: Pad short SW USO segments to BNXT_MIN_PKT_SIZE

[Michael Chan]

On Fri, Oct 9, 2026 at 10:33 AM Joe Damato <joe@xxxxxxx> wrote:
>
> 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@xxxxxxx> 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.
>

Did you mean generic XDP turning around incoming packets with XDP_TX
actions and transmitting them through the software stack?  This path
cannot generate USO packets, right?

> 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.

I think the math is right.  If you have n nr_frags, you can only cross
it at most n times.  So the +1 was not necessary until we now deal
with the padding. Thanks.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature