[PATCH net 0/2] bnxt_en: Fix SW USO padding

[PATCH net 0/2] bnxt_en: Fix SW USO padding

[Joe Damato]

Greetings:

As a followup to Eric's fix a51d233ccd48 ("bnxt_en: fix DMA mapping length
for padded small packets") which fixes padding for normal TX, this series
fixes padding for SW USO.

There are two cases I try to address:

  1. The last segment could be shorter than BNXT_MIN_PKT_SIZE. So, add an
     extra BD if needed to pad to that minimum.
  2. The gso_size is so small that every segment would be shorter than
     BNXT_MIN_PKT_SIZE (under 10 bytes with IPv4). This seems exceedingly
     unlikely, but just in case, it is handled by clearing the GSO features
     for these skbs so the stack segments them.

Patch 1 is a small refactor to move code into a helper that patch 2 uses to
avoid duplicating code.

I ran this on a bnxt machine and re-ran the uso.py test and it passed, but
the existing uso.py test doesn't test this specific case, so running the
test was more of a regression test.

Thanks,
Joe

Joe Damato (2):
  bnxt_en: Add helper to fill SW USO payload BDs
  bnxt_en: Pad short SW USO segments to BNXT_MIN_PKT_SIZE

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 12 ++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c | 56 ++++++++++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h | 15 +++++
 3 files changed, 70 insertions(+), 13 deletions(-)


base-commit: 6d25ffca055a77787c21a36b66c253f76239411b
-- 
2.53.0-Meta

[PATCH net 1/2] bnxt_en: Add helper to fill SW USO payload BDs

[Joe Damato]

Move the code that fills a payload BD and its software ring entry in
bnxt_sw_udp_gso_xmit() into a helper, bnxt_sw_gso_data_bd().

No functional change. A following patch uses the helper to add a pad BD
to short SW USO segments.

Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c | 33 ++++++++++++-------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
index 6c1060fa2ea5..ef04c9d08066 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c
@@ -31,6 +31,26 @@ static u32 bnxt_sw_gso_lhint(unsigned int len)
 		return TX_BD_FLAGS_LHINT_2048_AND_LARGER;
 }
 
+static struct tx_bd *bnxt_sw_gso_data_bd(struct bnxt *bp,
+					 struct bnxt_tx_ring_info *txr,
+					 u16 prod, dma_addr_t addr,
+					 unsigned int len)
+{
+	struct tx_bd *txbd = &txr->tx_desc_ring[TX_RING(bp, prod)][TX_IDX(prod)];
+	struct bnxt_sw_tx_bd *tx_buf = &txr->tx_buf_ring[RING_TX(bp, prod)];
+
+	txbd->tx_bd_haddr = cpu_to_le64(addr);
+	txbd->tx_bd_len_flags_type = cpu_to_le32(len << TX_BD_LEN_SHIFT);
+	txbd->tx_bd_opaque = 0;
+
+	dma_unmap_addr_set(tx_buf, mapping, addr);
+	dma_unmap_len_set(tx_buf, len, 0);
+	tx_buf->skb = NULL;
+	tx_buf->is_sw_gso = 0;
+
+	return txbd;
+}
+
 /* Transmit an skb requiring software UDP segmentation.
  *
  * Returns 1 if the skb was queued and new BDs were produced, 0 if the skb
@@ -181,15 +201,10 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
 		while (tso_dma_map_next(&map, &dma_addr, &chunk_len,
 					&mapping_len, seg_payload)) {
 			prod = NEXT_TX(prod);
-			txbd = &txr->tx_desc_ring[TX_RING(bp, prod)][TX_IDX(prod)];
+			txbd = bnxt_sw_gso_data_bd(bp, txr, prod, dma_addr,
+						   chunk_len);
 			tx_buf = &txr->tx_buf_ring[RING_TX(bp, prod)];
 
-			txbd->tx_bd_haddr = cpu_to_le64(dma_addr);
-			dma_unmap_addr_set(tx_buf, mapping, dma_addr);
-			dma_unmap_len_set(tx_buf, len, 0);
-			tx_buf->skb = NULL;
-			tx_buf->is_sw_gso = 0;
-
 			if (mapping_len) {
 				if (last_unmap_buf) {
 					dma_unmap_addr_set(last_unmap_buf,
@@ -204,10 +219,6 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
 			}
 			last_unmap_buf = tx_buf;
 
-			flags = chunk_len << TX_BD_LEN_SHIFT;
-			txbd->tx_bd_len_flags_type = cpu_to_le32(flags);
-			txbd->tx_bd_opaque = 0;
-
 			seg_payload -= chunk_len;
 		}
 
-- 
2.53.0-Meta

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

[Joe Damato]

Each SW USO segment is hdr_len + seg_payload bytes. With IPv4, hdr_len
is 42 bytes, so a segment carrying less than 10 bytes of UDP payload is
sent shorter than BNXT_MIN_PKT_SIZE. This happens with a short last
segment (e.g. gso_size 1400 and 1405 bytes of payload) or with a
gso_size below 10 bytes.

Fix the short last segment in bnxt_sw_udp_gso_xmit(): zero the pad
bytes after the header in the segment's inline header slot and add a
pad BD pointing at them after the payload BDs. Only the last segment
can need padding, and its pad BD fits within the existing bound in
bds_needed and BNXT_SW_USO_MAX_DESCS, since payload BDs are at most
num_segs + nr_frags.

If gso_size itself is too small, every segment would need a pad BD,
exceeding the descriptor bound. This case is unlikely, so
bnxt_features_check() clears GSO features for these skbs and the stack
segments them instead; bnxt_start_xmit() then pads each segment.

Fixes: cc5d90667db8 ("net: bnxt: Implement software USO")
Cc: stable@vger.kernel.org
Signed-off-by: Joe Damato <joe@dama.to>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 12 ++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.c | 23 +++++++++++++++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h | 15 ++++++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 15a8349ccf7b..d766dd469797 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -14268,6 +14268,18 @@ static netdev_features_t bnxt_features_check(struct sk_buff *skb,
 	u8 *l4_proto;
 
 	features = vlan_features_check(skb, features);
+
+	/* SW USO pads at most the last segment; let the stack segment skbs
+	 * where every segment would be shorter than BNXT_MIN_PKT_SIZE.
+	 */
+	if (skb_is_gso(skb) &&
+	    (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) &&
+	    !(bp->flags & BNXT_FLAG_UDP_GSO_CAP) &&
+	    bnxt_sw_gso_pad_len(skb_transport_offset(skb) +
+				sizeof(struct udphdr),
+				skb_shinfo(skb)->gso_size))
+		features &= ~NETIF_F_GSO_MASK;
+
 	switch (vlan_get_protocol(skb)) {
 	case htons(ETH_P_IP):
 		if (!skb->encapsulation)
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;
+
 	/* 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;
 
@@ -133,12 +136,14 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
 		dma_addr_t dma_addr;
 		struct tx_bd *txbd;
 		struct udphdr *uh;
+		unsigned int pad;
 		void *this_hdr;
 		int bd_count;
 		bool last;
 		u32 flags;
 
 		last = (i == num_segs - 1);
+		pad = bnxt_sw_gso_pad_len(hdr_len, seg_payload);
 		offset = slot * TSO_HEADER_SIZE;
 		this_hdr = txr->tx_inline_buf + offset;
 		this_hdr_dma = txr->tx_inline_dma + offset;
@@ -156,10 +161,18 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
 			iph->check = 0;
 		}
 
+		/* Pad bytes follow the header in the inline slot. Zero them
+		 * so stale header bytes from an earlier packet are not sent.
+		 */
+		if (pad)
+			memset(this_hdr + hdr_len, 0, pad);
+
 		dma_sync_single_for_device(&pdev->dev, this_hdr_dma,
-					   hdr_len, DMA_TO_DEVICE);
+					   hdr_len + pad, DMA_TO_DEVICE);
 
 		bd_count = tso_dma_map_count(&map, seg_payload);
+		if (pad)
+			bd_count++;
 
 		tx_buf = &txr->tx_buf_ring[RING_TX(bp, prod)];
 		txbd = &txr->tx_desc_ring[TX_RING(bp, prod)][TX_IDX(prod)];
@@ -222,6 +235,12 @@ int bnxt_sw_udp_gso_xmit(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
 			seg_payload -= chunk_len;
 		}
 
+		if (pad) {
+			prod = NEXT_TX(prod);
+			txbd = bnxt_sw_gso_data_bd(bp, txr, prod,
+						   this_hdr_dma + hdr_len, pad);
+		}
+
 		txbd->tx_bd_len_flags_type |=
 			cpu_to_le32(TX_BD_FLAGS_PACKET_END);
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h
index 77d9af97cc22..5916ee08c0e6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_gso.h
@@ -19,10 +19,25 @@
  * Each segment: 1 long BD + 1 ext BD + payload BDs.
  * Total payload BDs across all segs <= num_segs + nr_frags (each frag
  * boundary crossing adds at most 1 extra BD).
+ * The last segment may need 1 pad BD (see bnxt_sw_gso_pad_len()).
  * So: 3 * max_segs + MAX_SKB_FRAGS + 1 = 3 * 64 + 17 + 1 = 210.
  */
 #define BNXT_SW_USO_MAX_DESCS	(3 * BNXT_SW_USO_MAX_SEGS + MAX_SKB_FRAGS + 1)
 
+/* Bytes of padding needed to bring a segment carrying @seg_payload bytes
+ * up to BNXT_MIN_PKT_SIZE, or 0 if no padding is needed.
+ */
+static inline unsigned int bnxt_sw_gso_pad_len(unsigned int hdr_len,
+					       unsigned int seg_payload)
+{
+	unsigned int len = hdr_len + seg_payload;
+
+	if (len < BNXT_MIN_PKT_SIZE)
+		return BNXT_MIN_PKT_SIZE - len;
+	else
+		return 0;
+}
+
 static inline u16 bnxt_inline_avail(struct bnxt_tx_ring_info *txr)
 {
 	return BNXT_SW_USO_MAX_SEGS -
-- 
2.53.0-Meta

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

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

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

>         /* 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.
Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

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

[Joe Damato]

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

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

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]

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

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 #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

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

[Joe Damato]

On Fri, Oct 09, 2026 at 11:33:07AM -0700, Michael Chan wrote:
> On Fri, Oct 9, 2026 at 10:33 AM Joe Damato <joe@dama.to> 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@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.
> >
> 
> 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?

Hm, I'm probably missing something, but the case I'm describing is generic XDP
bouncing received packets back out with XDP_TX.

I could be wrong (and if so I am happy to drop this if statement from the
code), but IIUC generic XDP runs from __netif_receive_skb_core() after
software GRO, so the packet it sees can be a GRO'd UDP packet with
SKB_GSO_UDP_L4 set (because maybe the socket had UDP_GRO set?).

The generic XDP path seems to keep all of the gso fields. Later generic
XDP TX calls netdev_start_xmit without validate_xmit_skb being called
and so gso_size could potentially be some tiny value.

LMK if that makes sense? I think that's what happens and if so, it is a weird
corner case that is probably pretty unlikely to happen so it felt like drop
might be the most appropriate thing to do.

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

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On Fri, Oct 9, 2026 at 12:45 PM Joe Damato <joe@dama.to> wrote:
>
> On Fri, Oct 09, 2026 at 11:33:07AM -0700, Michael Chan wrote:
> > 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?
>
> Hm, I'm probably missing something, but the case I'm describing is generic XDP
> bouncing received packets back out with XDP_TX.
>
> I could be wrong (and if so I am happy to drop this if statement from the
> code), but IIUC generic XDP runs from __netif_receive_skb_core() after
> software GRO, so the packet it sees can be a GRO'd UDP packet with
> SKB_GSO_UDP_L4 set (because maybe the socket had UDP_GRO set?).
>
> The generic XDP path seems to keep all of the gso fields. Later generic
> XDP TX calls netdev_start_xmit without validate_xmit_skb being called
> and so gso_size could potentially be some tiny value.

I don't know.  You may be right, but this is a highly unusual code
path.  Other features checked in bnxt_features_check() do not get
rechecked again.

If you decide to keep the check, please add a comment.  Thanks.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

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

[Joe Damato]

On Fri, Oct 09, 2026 at 01:07:59PM -0700, Michael Chan wrote:
> On Fri, Oct 9, 2026 at 12:45 PM Joe Damato <joe@dama.to> wrote:
> >
> > On Fri, Oct 09, 2026 at 11:33:07AM -0700, Michael Chan wrote:
> > > 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?
> >
> > Hm, I'm probably missing something, but the case I'm describing is generic XDP
> > bouncing received packets back out with XDP_TX.
> >
> > I could be wrong (and if so I am happy to drop this if statement from the
> > code), but IIUC generic XDP runs from __netif_receive_skb_core() after
> > software GRO, so the packet it sees can be a GRO'd UDP packet with
> > SKB_GSO_UDP_L4 set (because maybe the socket had UDP_GRO set?).
> >
> > The generic XDP path seems to keep all of the gso fields. Later generic
> > XDP TX calls netdev_start_xmit without validate_xmit_skb being called
> > and so gso_size could potentially be some tiny value.
> 
> I don't know.  You may be right, but this is a highly unusual code
> path.  Other features checked in bnxt_features_check() do not get
> rechecked again.
> 
> If you decide to keep the check, please add a comment.  Thanks.

OK, I'll submit a v2 with the following comment. Let me know if this OK
with you before I respin:

  /* bnxt_features_check() makes the stack segment these, but paths
   * that skip validate_xmit_skb() (e.g. generic XDP_TX of a GRO'd
   * skb) can still get here. Padding every segment would need more
   * BDs than bds_needed reserves below, so drop instead.
   */
  if (unlikely(bnxt_sw_gso_pad_len(hdr_len, mss)))
          goto drop;

Also, since I haven't seen this issue in production myself and according
to my math it is not possible to hit this with QUIC on ipv4, Jakub asked
me to submit this to net-next instead of net.

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

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On Fri, Oct 9, 2026 at 1:23 PM Joe Damato <joe@dama.to> wrote:
> OK, I'll submit a v2 with the following comment. Let me know if this OK
> with you before I respin:
>
>   /* bnxt_features_check() makes the stack segment these, but paths
>    * that skip validate_xmit_skb() (e.g. generic XDP_TX of a GRO'd
>    * skb) can still get here. Padding every segment would need more
>    * BDs than bds_needed reserves below, so drop instead.
>    */
>   if (unlikely(bnxt_sw_gso_pad_len(hdr_len, mss)))
>           goto drop;
>
> Also, since I haven't seen this issue in production myself and according
> to my math it is not possible to hit this with QUIC on ipv4, Jakub asked
> me to submit this to net-next instead of net.

Sounds good.  For both patches:
Reviewed-by: Michael Chan <michael.chan@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]