[PATCH net-next v2 0/6] eth: mpnic: checksum and segmentation offloads

[PATCH net-next v2 0/6] eth: mpnic: checksum and segmentation offloads

[Daniel Zahka]

Add checksum and segmentation offload support. Tx checksum offload
follows the generic csum_start/csum_offset model. Rx checksum also
follows the normal unnecessary/complete model. The device can validate l4
checksums for tcp or udp, when the l3 type is one of: ipv4, ipv6,
ipv4-in-ipv6, or ipv6-in-ipv6. The full l2 checksum of a frame is always
passed to driver via a Rx completion descriptor field, which can be used
for checksum complete on other packet types.

Tx segmentation supports tcp or udp in ipv4, ipv6, 4-in-6, or 6-in-6
natively, and can support most other tunnels with GSO_PARTIAL.

Counters for CSO/LSO are exported via the netdev qstats API.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Changes in v2:
- eth: mpnic: Tx checksum offload
  - use skb_network_offset() for the L2 header length in
    mpnic_features_check(), since the mac header is not yet set when
    called from the IP GSO slow path
  - also enable Tx checksum offload for encapsulated packets via
    hw_enc_features (moved from the IPv4 and IPv6 in IPv6 patch)
- eth: mpnic: Rx checksum offload
  - convert l2_csum with cpu_to_le16() before csum_unfold(), since the
    device reports it in network byte order and skb->csum was wrong on
    big-endian hosts
- eth: mpnic: TCP segmentation offload
  - count discarded skbs rather than segments in twq.dropped, matching
    the xmit error path and the core's tx_dropped
  - report only hw_gso_wire_packets; drop the super-packet counter
- eth: mpnic: UDP segmentation offload
  - fall back to SW GSO for skbs shorter than MPNIC_MIN_FRAME_LEN, since
    HW LSO would segment the zero padding as payload
- eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation
  - derive inner L3 header length from csum_start rather than
    inner_transport_header, which can be stale for DODGY skbs
  - check the outer header is IPv6 in mpnic_ipxip6_gso_ok(), since
    SKB_GSO_IPXIP6 can be set by a nested tunnel under an IPv4 outer
  - move hw_enc_features setup to the Tx checksum offload patch
- eth: mpnic: add GSO_PARTIAL for other tunnels
  - derive the L3 header length from csum_start rather than
    inner_transport_header, matching mpnic_features_check() and
    mpnic_tx_lso()
- Link to v1: https://lore.kernel.org/r/20261106-mpnic-lso-v1-0-e943ff1493e4@gmail.com

---
Daniel Zahka (6):
      eth: mpnic: Tx checksum offload
      eth: mpnic: Rx checksum offload
      eth: mpnic: TCP segmentation offload
      eth: mpnic: UDP segmentation offload
      eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation
      eth: mpnic: add GSO_PARTIAL for other tunnels

 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  41 +++++
 drivers/net/ethernet/meta/mpnic/mpnic_init.c   |  14 ++
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c |  40 +++-
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.h |   5 +
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 242 ++++++++++++++++++++++++-
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.h   |   8 +
 6 files changed, 346 insertions(+), 4 deletions(-)
---
base-commit: 97ced2c090bfc2156db31fca9f52cf253e4bc186
change-id: 20261102-mpnic-lso-19aeb38e5648

Best regards,
-- 
Daniel Zahka <daniel.zahka@gmail.com>

[PATCH net-next v2 1/6] eth: mpnic: Tx checksum offload

[Daniel Zahka]

Support generic Tx checksum offload per NETIF_F_HW_CSUM. Propagate all
netdev features to hw_enc_features, so CSO also works on encapsulated
skbs.

Add mpnic_features_check() to ensure that skb metadata can be mapped
into Tx descriptor fields. The headers and offsets used have units of
2B. The device also requires any headers modified to be in the first
256B of the packet.

Zero checksums are inverted by HW, matching the behavior of
skb_checksum_help().

Count CSO frames and report them via netdev qstats API.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- use skb_network_offset() for the L2 header length in
  mpnic_features_check(), since the mac header is not yet set when
  called from the IP GSO slow path
- also enable Tx checksum offload for encapsulated packets via
  hw_enc_features (moved from the IPv4 and IPv6 in IPv6 patch)
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  6 +++
 drivers/net/ethernet/meta/mpnic/mpnic_init.c   |  2 +
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c | 11 ++++-
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.h |  1 +
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 60 ++++++++++++++++++++++++++
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.h   |  4 ++
 6 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index d6ab9df58168..62304be203e6 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -13,6 +13,10 @@
 #define DESC_GENMASK(h, l)	GENMASK_ULL(h, l)
 
 /* Transmit Work Descriptor Format */
+#define MPNIC_TWD_L2_HLEN		DESC_GENMASK(5, 0)
+#define MPNIC_TWD_L3_IHLEN		DESC_GENMASK(23, 16)
+#define MPNIC_TWD_CSUM_OFFSET		DESC_GENMASK(27, 24)
+#define MPNIC_TWD_FLAG_REQ_CSO		DESC_BIT(36)
 #define MPNIC_TWD_FLAG_REQ_COMPLETION	DESC_BIT(37)
 #define MPNIC_TWD_FLAG_DEST_MAC		DESC_BIT(43)
 #define MPNIC_TWD_TYPE			DESC_GENMASK(47, 46)
@@ -305,6 +309,8 @@ enum {
 
 /* NIC_CORE_TOF */
 #define MPNIC_TOF_TCAM_DEST_REMAP	0x7e3022		/* 0x1f8c088 */
+#define MPNIC_TOF_MODE			0x7e302e		/* 0x1f8c0b8 */
+#define MPNIC_TOF_MODE_L4_ZERO_CSUM		CSR_BIT(4)
 
 /* PEMO_WRAPPER */
 #define MPNIC_OB_ATTR_RO			CSR_BIT(1)
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_init.c b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
index ace4124158fb..71d13bec7ec4 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_init.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
@@ -298,6 +298,8 @@ static void mpnic_txglb_init(struct mpnic_dev *mpd)
 	 * programmed with its addresses.
 	 */
 	mpnic_wr64(mpd, MPNIC_TOF_TCAM_DEST_REMAP, 0);
+	mpnic_wr64(mpd, MPNIC_TOF_MODE,
+		   FIELD_PREP(MPNIC_TOF_MODE_L4_ZERO_CSUM, 0));
 
 	mpnic_wr64(mpd, MPNIC_TCM_MEM_INIT_REQ, MPNIC_MEM_INIT_TCM_VAL);
 	mpnic_wr64(mpd, MPNIC_TDE_MEM_INIT_REQ, MPNIC_MEM_INIT_TDE_VAL);
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index 7fdf57f70151..d386672738fc 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -144,6 +144,7 @@ static const struct net_device_ops mpnic_netdev_ops = {
 	.ndo_stop		= mpnic_stop,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_start_xmit		= mpnic_xmit_frame,
+	.ndo_features_check	= mpnic_features_check,
 	.ndo_get_stats64	= mpnic_get_stats64,
 };
 
@@ -200,6 +201,7 @@ static void mpnic_get_queue_stats_tx(struct net_device *dev, int idx,
 	u64 bytes, packets, stop, wake;
 	struct mpnic_q_triad *qt;
 	unsigned int start;
+	u64 csum_partial;
 
 	if (!txr)
 		return;
@@ -217,11 +219,13 @@ static void mpnic_get_queue_stats_tx(struct net_device *dev, int idx,
 	stats = &txr->stats;
 	do {
 		start = u64_stats_fetch_begin(&stats->syncp);
+		csum_partial = u64_stats_read(&stats->twq.csum_partial);
 		stop = u64_stats_read(&stats->twq.stop);
 	} while (u64_stats_fetch_retry(&stats->syncp, start));
 
 	tx->bytes = bytes;
 	tx->packets = packets;
+	tx->needs_csum = csum_partial;
 	tx->stop = stop;
 	tx->wake = wake;
 }
@@ -237,6 +241,7 @@ static void mpnic_get_base_stats(struct net_device *dev,
 
 	tx->bytes = base->tx.bytes;
 	tx->packets = base->tx.packets;
+	tx->needs_csum = base->tx.csum_partial;
 	tx->stop = base->tx.stop;
 	tx->wake = base->tx.wake;
 
@@ -303,9 +308,13 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 	mpn->num_rx_queues = queues;
 	mpn->num_napi = queues;
 
-	netdev->features |= NETIF_F_SG;
+	netdev->features |=
+		NETIF_F_SG |
+		NETIF_F_HW_CSUM;
+
 	netdev->hw_features |= netdev->features;
 	netdev->vlan_features |= netdev->features;
+	netdev->hw_enc_features |= netdev->features;
 
 	netdev->min_mtu = IPV6_MIN_MTU;
 	netdev->max_mtu = MPNIC_MAX_JUMBO_FRAME_SIZE - ETH_HLEN;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
index 6711766ebc3c..94f7a3355c5c 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
@@ -16,6 +16,7 @@ struct mpnic_base_stats {
 		u64 packets;
 		u64 bytes;
 		u64 dropped;
+		u64 csum_partial;
 		u64 stop;
 		u64 wake;
 	} tx;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index 93abdec44642..65de6d999d54 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <net/netdev_lock.h>
 #include <net/page_pool/helpers.h>
+#include <net/tcp.h>
 
 #include "mpnic.h"
 #include "mpnic_netdev.h"
@@ -18,6 +19,10 @@ struct mpnic_xmit_cb {
 	u8 desc_count;
 };
 
+struct mpnic_tx_hdrs {
+	unsigned int i3len;
+};
+
 #define MPNIC_XMIT_CB(__skb) ((struct mpnic_xmit_cb *)((__skb)->cb))
 #define MPNIC_TWD_TYPE_MASK(_type) \
 	cpu_to_le64(FIELD_PREP(MPNIC_TWD_TYPE, MPNIC_TWD_TYPE_##_type))
@@ -208,6 +213,34 @@ mpnic_tx_map(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 	return true;
 }
 
+static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
+{
+	hdrs->i3len = skb_checksum_start(skb) - skb_network_header(skb);
+}
+
+static void
+mpnic_tx_offloads(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
+{
+	struct mpnic_tx_hdrs hdrs = {};
+
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		return;
+
+	mpnic_tx_fill_hdrs(skb, &hdrs);
+
+	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_CSUM_OFFSET,
+					skb->csum_offset / 2) |
+			     MPNIC_TWD_FLAG_REQ_CSO);
+
+	u64_stats_update_begin(&ring->stats.syncp);
+	u64_stats_inc(&ring->stats.twq.csum_partial);
+	u64_stats_update_end(&ring->stats.syncp);
+
+	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L2_HLEN,
+					skb_mac_header_len(skb) / 2) |
+			     FIELD_PREP(MPNIC_TWD_L3_IHLEN, hdrs.i3len / 2));
+}
+
 #define MPNIC_MIN_FRAME_LEN	60
 
 static netdev_tx_t mpnic_xmit_frame_ring(struct sk_buff *skb,
@@ -230,6 +263,8 @@ static netdev_tx_t mpnic_xmit_frame_ring(struct sk_buff *skb,
 	MPNIC_XMIT_CB(skb)->bytecount = skb->len;
 	MPNIC_XMIT_CB(skb)->desc_count = 0;
 
+	mpnic_tx_offloads(ring, skb, meta);
+
 	if (mpnic_tx_map(ring, skb, meta))
 		goto err_free;
 
@@ -256,6 +291,30 @@ netdev_tx_t mpnic_xmit_frame(struct sk_buff *skb, struct net_device *dev)
 	return mpnic_xmit_frame_ring(skb, mpn->tx[skb_get_queue_mapping(skb)]);
 }
 
+netdev_features_t mpnic_features_check(struct sk_buff *skb,
+				       struct net_device *dev,
+				       netdev_features_t features)
+{
+	unsigned int l2len, i3len;
+
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		return features;
+
+	l2len = skb_network_offset(skb);
+	if (skb_vlan_tag_present(skb))
+		l2len += VLAN_HLEN;
+	i3len = skb_checksum_start(skb) - skb_network_header(skb);
+
+	if ((l2len | i3len | skb->csum_offset) % 2 ||
+	    !FIELD_FIT(MPNIC_TWD_L2_HLEN, l2len / 2) ||
+	    !FIELD_FIT(MPNIC_TWD_L3_IHLEN, i3len / 2) ||
+	    !FIELD_FIT(MPNIC_TWD_CSUM_OFFSET, skb->csum_offset / 2) ||
+	    l2len + i3len + sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE > 256)
+		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
+
+	return features;
+}
+
 static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
 			     struct mpnic_q_triad *qt, bool discard,
 			     unsigned int hw_head)
@@ -721,6 +780,7 @@ static void mpnic_aggregate_ring_twq_counters(struct mpnic_base_stats *base,
 					      struct mpnic_ring *twq)
 {
 	base->tx.dropped += u64_stats_read(&twq->stats.twq.dropped);
+	base->tx.csum_partial += u64_stats_read(&twq->stats.twq.csum_partial);
 	base->tx.stop += u64_stats_read(&twq->stats.twq.stop);
 }
 
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
index 52492e3f484f..a6dff9dc32d6 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
@@ -89,6 +89,7 @@ struct mpnic_queue_stats {
 	union {
 		struct {
 			u64_stats_t dropped;
+			u64_stats_t csum_partial;
 			u64_stats_t stop;
 		} twq;
 		struct {
@@ -168,6 +169,9 @@ struct mpnic_napi_vector {
 };
 
 netdev_tx_t mpnic_xmit_frame(struct sk_buff *skb, struct net_device *dev);
+netdev_features_t mpnic_features_check(struct sk_buff *skb,
+				       struct net_device *dev,
+				       netdev_features_t features);
 int mpnic_alloc_napi_vectors(struct mpnic_net *mpn);
 void mpnic_free_napi_vectors(struct mpnic_net *mpn);
 void mpnic_stats_attach_rings(struct mpnic_net *mpn);

-- 
2.52.0

Re: [PATCH net-next v2 1/6] eth: mpnic: Tx checksum offload

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new per-queue Tx counter `twq.csum_partial`, which reaches
  userspace as the qstats `tx-needs-csum` value, goes up too early.

--

eth: mpnic: Tx checksum offload

This adds generic Tx checksum offload (NETIF_F_HW_CSUM) to mpnic, including
for encapsulated skbs via hw_enc_features. It adds mpnic_features_check() to
make sure the skb header layout fits the Tx descriptor fields. CSO frames are
counted and reported through the netdev qstats needs_csum counter.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> index 93abdec44642f..65de6d999d548 100644
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c

[ ... ]

> +static void
> +mpnic_tx_offloads(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
> +{
> +	struct mpnic_tx_hdrs hdrs = {};
> +
> +	if (skb->ip_summed != CHECKSUM_PARTIAL)
> +		return;
> +
> +	mpnic_tx_fill_hdrs(skb, &hdrs);
> +
> +	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_CSUM_OFFSET,
> +					skb->csum_offset / 2) |
> +			     MPNIC_TWD_FLAG_REQ_CSO);
> +
> +	u64_stats_update_begin(&ring->stats.syncp);
> +	u64_stats_inc(&ring->stats.twq.csum_partial);
> +	u64_stats_update_end(&ring->stats.syncp);

[Severity: Low]
Should this counter be bumped only after the frame is actually queued to
the hardware?

mpnic_tx_offloads() increments twq.csum_partial for every CHECKSUM_PARTIAL
skb before mpnic_xmit_frame_ring() calls mpnic_tx_map():

> @@ -230,6 +263,8 @@ static netdev_tx_t mpnic_xmit_frame_ring(struct sk_buff *skb,
>  	MPNIC_XMIT_CB(skb)->bytecount = skb->len;
>  	MPNIC_XMIT_CB(skb)->desc_count = 0;
>  
> +	mpnic_tx_offloads(ring, skb, meta);
> +
>  	if (mpnic_tx_map(ring, skb, meta))
>  		goto err_free;

mpnic_tx_map() can fail in two ways. One is dma_map_single() or
skb_frag_dma_map() returning a mapping error. The other is the head or a
fragment being larger than FIELD_MAX(MPNIC_TWD_LEN). In either case the
error path frees the skb and counts it as dropped:

mpnic_xmit_frame_ring() {
    ...
err_free:
	dev_kfree_skb_any(skb);
	ring->tx_buf[tail] = NULL;
	ring->tail = tail;
err_drop:
	mpnic_tx_flush_doorbell(ring);

	u64_stats_update_begin(&ring->stats.syncp);
	u64_stats_inc(&ring->stats.twq.dropped);
	u64_stats_update_end(&ring->stats.syncp);
    ...
}

The csum_partial increment is never undone. mpnic_get_queue_stats_tx()
then reports it as tx->needs_csum, so the same packet shows up as both
dropped and checksummed by the device, even though it never reached the
hardware.

The same ordering is still there later in the series. mpnic_tx_offloads()
also bumps hw_gso_wire through mpnic_tx_lso() before mpnic_tx_map() runs.

One option is to have mpnic_tx_offloads() report which offload it used,
and update the stats only after mpnic_tx_map() succeeds. Note that fbnic
uses the same count-before-map pattern in fbnic_tx_offloads().

> +
> +	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L2_HLEN,
> +					skb_mac_header_len(skb) / 2) |
> +			     FIELD_PREP(MPNIC_TWD_L3_IHLEN, hdrs.i3len / 2));
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-mpnic-lso-v2-0-ccd37b051d08%40gmail.com

[PATCH net-next v2 2/6] eth: mpnic: Rx checksum offload

[Daniel Zahka]

The device reports two Rx completion fields to the driver related to
checksums: l2_csum and l4_csum_unnecessary. l2_csum is the ones'
complement sum of the frame starting from the end of the ethernet
header. l4_csum_unnecessary means the device validated the l4 checksum of
the packet. This is only supported for tcp or udp in
ipv4/ipv6/4-in-6/6-in-6.

Report checksum none/complete/unnecessary counts via netdev qstats API.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- convert l2_csum with cpu_to_le16() before csum_unfold(), since the
  device reports it in network byte order and skb->csum was wrong on
  big-endian hosts
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  2 ++
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c | 14 ++++++++++-
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.h |  3 +++
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 34 ++++++++++++++++++++++++++
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.h   |  3 +++
 5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index 62304be203e6..1c86f114ab0c 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -60,6 +60,8 @@ enum {
 #define MPNIC_RCD_AL_PAGE_FIN		DESC_BIT(53)
 
 /* Metadata Completion Descriptors */
+#define MPNIC_RCD_META_L2_CSUM			DESC_GENMASK(47, 32)
+#define MPNIC_RCD_META_L4_CSUM_UNNECESSARY	DESC_BIT(52)
 #define MPNIC_RCD_META_ERR_MAC_EOP		DESC_BIT(53)
 #define MPNIC_RCD_META_ERR_TRUNCATED_FRAME	DESC_BIT(54)
 #define MPNIC_RCD_META_UNCORRECTABLE_ERR_MASK	\
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index d386672738fc..35173f8999cb 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -167,10 +167,12 @@ static void mpnic_get_queue_stats_rx(struct net_device *dev, int idx,
 {
 	struct mpnic_net *mpn = netdev_priv(dev);
 	struct mpnic_ring *rxr = mpn->rx[idx];
+	u64 csum_unnecessary, csum_complete;
 	u64 bytes, packets, alloc_failed;
 	struct mpnic_queue_stats *stats;
 	struct mpnic_q_triad *qt;
 	unsigned int start;
+	u64 csum_none;
 
 	if (!rxr)
 		return;
@@ -181,6 +183,9 @@ static void mpnic_get_queue_stats_rx(struct net_device *dev, int idx,
 		bytes = u64_stats_read(&stats->rcq.bytes);
 		packets = u64_stats_read(&stats->rcq.packets);
 		alloc_failed = u64_stats_read(&stats->rcq.alloc_failed);
+		csum_complete = u64_stats_read(&stats->rcq.csum_complete);
+		csum_unnecessary = u64_stats_read(&stats->rcq.csum_unnecessary);
+		csum_none = u64_stats_read(&stats->rcq.csum_none);
 	} while (u64_stats_fetch_retry(&stats->syncp, start));
 
 	qt = container_of(rxr, struct mpnic_q_triad, cmpl);
@@ -190,6 +195,9 @@ static void mpnic_get_queue_stats_rx(struct net_device *dev, int idx,
 	rx->bytes = bytes;
 	rx->packets = packets;
 	rx->alloc_fail = alloc_failed;
+	rx->csum_complete = csum_complete;
+	rx->csum_unnecessary = csum_unnecessary;
+	rx->csum_none = csum_none;
 }
 
 static void mpnic_get_queue_stats_tx(struct net_device *dev, int idx,
@@ -248,6 +256,9 @@ static void mpnic_get_base_stats(struct net_device *dev,
 	rx->bytes = base->rx.bytes;
 	rx->packets = base->rx.packets;
 	rx->alloc_fail = base->rx.alloc_failed;
+	rx->csum_complete = base->rx.csum_complete;
+	rx->csum_unnecessary = base->rx.csum_unnecessary;
+	rx->csum_none = base->rx.csum_none;
 }
 
 static const struct netdev_stat_ops mpnic_stat_ops = {
@@ -310,7 +321,8 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 
 	netdev->features |=
 		NETIF_F_SG |
-		NETIF_F_HW_CSUM;
+		NETIF_F_HW_CSUM |
+		NETIF_F_RXCSUM;
 
 	netdev->hw_features |= netdev->features;
 	netdev->vlan_features |= netdev->features;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
index 94f7a3355c5c..76ec9811e79e 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
@@ -26,6 +26,9 @@ struct mpnic_base_stats {
 		u64 dropped;
 		u64 alloc_failed;
 		u64 errors;
+		u64 csum_complete;
+		u64 csum_unnecessary;
+		u64 csum_none;
 	} rx;
 
 	unsigned int num_tx_queues;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index 65de6d999d54..a35ce8998409 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -655,9 +655,34 @@ static void mpnic_put_pkt_buff(struct mpnic_pkt_ctxt *ctxt, bool napi)
 	page_pool_put_full_page(page->pp, page, napi);
 }
 
+static void mpnic_rx_csum(u64 rcd, struct sk_buff *skb,
+			  unsigned int *csum_complete,
+			  unsigned int *csum_unnecessary,
+			  unsigned int *csum_none)
+{
+	skb_checksum_none_assert(skb);
+
+	if (!(skb->dev->features & NETIF_F_RXCSUM)) {
+		(*csum_none)++;
+		return;
+	}
+
+	if (FIELD_GET(MPNIC_RCD_META_L4_CSUM_UNNECESSARY, rcd)) {
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		(*csum_unnecessary)++;
+	} else {
+		u16 csum = FIELD_GET(MPNIC_RCD_META_L2_CSUM, rcd);
+
+		skb->ip_summed = CHECKSUM_COMPLETE;
+		skb->csum = csum_unfold((__force __sum16)cpu_to_le16(csum));
+		(*csum_complete)++;
+	}
+}
+
 static int mpnic_clean_rcq(struct mpnic_napi_vector *nv,
 			   struct mpnic_q_triad *qt, int budget)
 {
+	unsigned int csum_complete = 0, csum_unnecessary = 0, csum_none = 0;
 	unsigned int packets = 0, bytes = 0, dropped = 0, errors = 0;
 	struct mpnic_ring *rcq = &qt->cmpl;
 	struct mpnic_rcq_state *state;
@@ -700,6 +725,8 @@ static int mpnic_clean_rcq(struct mpnic_napi_vector *nv,
 			}
 
 			if (likely(skb)) {
+				mpnic_rx_csum(rcd, skb, &csum_complete,
+					      &csum_unnecessary, &csum_none);
 				napi_gro_receive(&nv->napi, skb);
 			} else {
 				mpnic_put_pkt_buff(&state->pkt, true);
@@ -732,6 +759,9 @@ static int mpnic_clean_rcq(struct mpnic_napi_vector *nv,
 	u64_stats_add(&rcq->stats.rcq.dropped, dropped);
 	u64_stats_add(&rcq->stats.rcq.alloc_failed, alloc_failed);
 	u64_stats_add(&rcq->stats.rcq.errors, errors);
+	u64_stats_add(&rcq->stats.rcq.csum_complete, csum_complete);
+	u64_stats_add(&rcq->stats.rcq.csum_unnecessary, csum_unnecessary);
+	u64_stats_add(&rcq->stats.rcq.csum_none, csum_none);
 	u64_stats_update_end(&rcq->stats.syncp);
 
 	/* Allocate buffers, force dma_wmb(), and then start writing tails */
@@ -806,6 +836,10 @@ static void mpnic_aggregate_ring_rcq_counters(struct mpnic_base_stats *base,
 	base->rx.dropped += u64_stats_read(&rcq->stats.rcq.dropped);
 	base->rx.alloc_failed += u64_stats_read(&rcq->stats.rcq.alloc_failed);
 	base->rx.errors += u64_stats_read(&rcq->stats.rcq.errors);
+	base->rx.csum_complete += u64_stats_read(&rcq->stats.rcq.csum_complete);
+	base->rx.csum_unnecessary +=
+		u64_stats_read(&rcq->stats.rcq.csum_unnecessary);
+	base->rx.csum_none += u64_stats_read(&rcq->stats.rcq.csum_none);
 }
 
 static struct mpnic_base_stats *mpnic_base_stats_prepare(struct mpnic_net *mpn)
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
index a6dff9dc32d6..714f09f4160e 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
@@ -103,6 +103,9 @@ struct mpnic_queue_stats {
 			u64_stats_t dropped;
 			u64_stats_t alloc_failed;
 			u64_stats_t errors;
+			u64_stats_t csum_complete;
+			u64_stats_t csum_unnecessary;
+			u64_stats_t csum_none;
 		} rcq;
 		struct {
 			u64_stats_t alloc_failed;

-- 
2.52.0

[PATCH net-next v2 3/6] eth: mpnic: TCP segmentation offload

[Daniel Zahka]

Support segmentation of non-encapsulated tcp4/tcp6 packets. When doing
segmentation the device expects the l4 checksum to be seeded with 0
length.

IPv4 header and TCP checksums that compute to zero are sent as 0 rather
than 0xffff when the HW segments packets. IPv4 IDs are incrementing
starting from the value that is in the skb's IPv4 header.

Count gso wire packets and report via the netdev qstats API.

Completions are counted in wire packets, but dropped TSO skbs are still
counted once per super packet, matching the core's per skb tx_dropped
accounting that gets summed into the same counter.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- count discarded skbs rather than segments in twq.dropped, matching
  the xmit error path and the core's tx_dropped
- report only hw_gso_wire_packets; drop the super-packet counter
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    | 27 ++++++++
 drivers/net/ethernet/meta/mpnic/mpnic_init.c   | 11 ++++
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c | 14 ++--
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.h |  1 +
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 88 ++++++++++++++++++++++----
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.h   |  1 +
 6 files changed, 125 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index 1c86f114ab0c..672ce0a98c22 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -16,8 +16,21 @@
 #define MPNIC_TWD_L2_HLEN		DESC_GENMASK(5, 0)
 #define MPNIC_TWD_L3_IHLEN		DESC_GENMASK(23, 16)
 #define MPNIC_TWD_CSUM_OFFSET		DESC_GENMASK(27, 24)
+#define MPNIC_TWD_L4_HLEN		DESC_GENMASK(31, 28)
+#define MPNIC_TWD_L4_TYPE		DESC_GENMASK(33, 32)
+enum {
+	MPNIC_TWD_L4_TYPE_TCP	= 1,
+};
+
+#define MPNIC_TWD_FLAG_REQ_LSO		DESC_BIT(35)
 #define MPNIC_TWD_FLAG_REQ_CSO		DESC_BIT(36)
 #define MPNIC_TWD_FLAG_REQ_COMPLETION	DESC_BIT(37)
+#define MPNIC_TWD_L3_TYPE		DESC_GENMASK(42, 40)
+enum {
+	MPNIC_TWD_L3_TYPE_IPV4	= 1,
+	MPNIC_TWD_L3_TYPE_IPV6	= 2,
+};
+
 #define MPNIC_TWD_FLAG_DEST_MAC		DESC_BIT(43)
 #define MPNIC_TWD_TYPE			DESC_GENMASK(47, 46)
 enum {
@@ -26,6 +39,8 @@ enum {
 	MPNIC_TWD_TYPE_LAST_AL	= 3,
 };
 
+#define MPNIC_TWD_MSS			DESC_GENMASK(61, 48)
+
 #define MPNIC_TWD_ADDR			DESC_GENMASK(45, 0)
 #define MPNIC_TWD_LEN			DESC_GENMASK(63, 48)
 
@@ -312,7 +327,19 @@ enum {
 /* NIC_CORE_TOF */
 #define MPNIC_TOF_TCAM_DEST_REMAP	0x7e3022		/* 0x1f8c088 */
 #define MPNIC_TOF_MODE			0x7e302e		/* 0x1f8c0b8 */
+#define MPNIC_TOF_MODE_IPV4_ID_MODE		CSR_GENMASK(0, 0)
+enum {
+	MPNIC_IPV4_ID_MODE_FIXED	= 0,
+	MPNIC_IPV4_ID_MODE_INCREMENT	= 1,
+};
+
+#define MPNIC_TOF_MODE_IPV4_ZERO_CSUM		CSR_BIT(1)
+#define MPNIC_TOF_MODE_TCP_ZERO_CSUM		CSR_BIT(2)
 #define MPNIC_TOF_MODE_L4_ZERO_CSUM		CSR_BIT(4)
+#define MPNIC_TOF_TCP_FLAG_CLR		0x7e3030		/* 0x1f8c0c0 */
+#define MPNIC_TOF_TCP_FLAG_CLR_FIRST_SEGMENT	CSR_GENMASK(8, 0)
+#define MPNIC_TOF_TCP_FLAG_CLR_MIDDLE_SEGMENT	CSR_GENMASK(24, 16)
+#define MPNIC_TOF_TCP_FLAG_CLR_LAST_SEGMENT	CSR_GENMASK(40, 32)
 
 /* PEMO_WRAPPER */
 #define MPNIC_OB_ATTR_RO			CSR_BIT(1)
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_init.c b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
index 71d13bec7ec4..b9a41656f8f7 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_init.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
@@ -8,6 +8,7 @@
 #include <linux/iopoll.h>
 #include <linux/log2.h>
 #include <linux/sizes.h>
+#include <net/tcp.h>
 
 #include "mpnic.h"
 #include "mpnic_txrx.h"
@@ -299,7 +300,17 @@ static void mpnic_txglb_init(struct mpnic_dev *mpd)
 	 */
 	mpnic_wr64(mpd, MPNIC_TOF_TCAM_DEST_REMAP, 0);
 	mpnic_wr64(mpd, MPNIC_TOF_MODE,
+		   FIELD_PREP(MPNIC_TOF_MODE_IPV4_ID_MODE,
+			      MPNIC_IPV4_ID_MODE_INCREMENT) |
+		   FIELD_PREP(MPNIC_TOF_MODE_IPV4_ZERO_CSUM, 1) |
+		   FIELD_PREP(MPNIC_TOF_MODE_TCP_ZERO_CSUM, 1) |
 		   FIELD_PREP(MPNIC_TOF_MODE_L4_ZERO_CSUM, 0));
+	mpnic_wr64(mpd, MPNIC_TOF_TCP_FLAG_CLR,
+		   FIELD_PREP(MPNIC_TOF_TCP_FLAG_CLR_FIRST_SEGMENT,
+			      TCPHDR_PSH | TCPHDR_FIN) |
+		   FIELD_PREP(MPNIC_TOF_TCP_FLAG_CLR_MIDDLE_SEGMENT,
+			      TCPHDR_PSH | TCPHDR_CWR | TCPHDR_FIN) |
+		   FIELD_PREP(MPNIC_TOF_TCP_FLAG_CLR_LAST_SEGMENT, TCPHDR_CWR));
 
 	mpnic_wr64(mpd, MPNIC_TCM_MEM_INIT_REQ, MPNIC_MEM_INIT_TCM_VAL);
 	mpnic_wr64(mpd, MPNIC_TDE_MEM_INIT_REQ, MPNIC_MEM_INIT_TDE_VAL);
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index 35173f8999cb..bde327ad3154 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -207,9 +207,9 @@ static void mpnic_get_queue_stats_tx(struct net_device *dev, int idx,
 	struct mpnic_ring *txr = mpn->tx[idx];
 	struct mpnic_queue_stats *stats;
 	u64 bytes, packets, stop, wake;
+	u64 csum_partial, hw_gso_wire;
 	struct mpnic_q_triad *qt;
 	unsigned int start;
-	u64 csum_partial;
 
 	if (!txr)
 		return;
@@ -228,12 +228,14 @@ static void mpnic_get_queue_stats_tx(struct net_device *dev, int idx,
 	do {
 		start = u64_stats_fetch_begin(&stats->syncp);
 		csum_partial = u64_stats_read(&stats->twq.csum_partial);
+		hw_gso_wire = u64_stats_read(&stats->twq.hw_gso_wire);
 		stop = u64_stats_read(&stats->twq.stop);
 	} while (u64_stats_fetch_retry(&stats->syncp, start));
 
 	tx->bytes = bytes;
 	tx->packets = packets;
-	tx->needs_csum = csum_partial;
+	tx->needs_csum = csum_partial + hw_gso_wire;
+	tx->hw_gso_wire_packets = hw_gso_wire;
 	tx->stop = stop;
 	tx->wake = wake;
 }
@@ -249,7 +251,8 @@ static void mpnic_get_base_stats(struct net_device *dev,
 
 	tx->bytes = base->tx.bytes;
 	tx->packets = base->tx.packets;
-	tx->needs_csum = base->tx.csum_partial;
+	tx->needs_csum = base->tx.csum_partial + base->tx.hw_gso_wire;
+	tx->hw_gso_wire_packets = base->tx.hw_gso_wire;
 	tx->stop = base->tx.stop;
 	tx->wake = base->tx.wake;
 
@@ -322,7 +325,10 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 	netdev->features |=
 		NETIF_F_SG |
 		NETIF_F_HW_CSUM |
-		NETIF_F_RXCSUM;
+		NETIF_F_RXCSUM |
+		NETIF_F_TSO |
+		NETIF_F_TSO_ECN |
+		NETIF_F_TSO6;
 
 	netdev->hw_features |= netdev->features;
 	netdev->vlan_features |= netdev->features;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
index 76ec9811e79e..697b3ebb74b9 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.h
@@ -17,6 +17,7 @@ struct mpnic_base_stats {
 		u64 bytes;
 		u64 dropped;
 		u64 csum_partial;
+		u64 hw_gso_wire;
 		u64 stop;
 		u64 wake;
 	} tx;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index a35ce8998409..b78f8599bbe8 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -16,11 +16,14 @@
 
 struct mpnic_xmit_cb {
 	u32 bytecount;
+	u16 gso_segs;
 	u8 desc_count;
 };
 
 struct mpnic_tx_hdrs {
 	unsigned int i3len;
+	unsigned int l3_type;
+	unsigned int l4_type;
 };
 
 #define MPNIC_XMIT_CB(__skb) ((struct mpnic_xmit_cb *)((__skb)->cb))
@@ -215,30 +218,81 @@ mpnic_tx_map(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 
 static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
 {
-	hdrs->i3len = skb_checksum_start(skb) - skb_network_header(skb);
+	if (!skb_is_gso(skb)) {
+		hdrs->i3len = skb_checksum_start(skb) - skb_network_header(skb);
+		return;
+	}
+
+	hdrs->l4_type = MPNIC_TWD_L4_TYPE_TCP;
+	hdrs->i3len = skb_transport_offset(skb) - skb_network_offset(skb);
+	hdrs->l3_type = ip_hdr(skb)->version == 4 ?
+			MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
 }
 
-static void
+static bool
+mpnic_tx_lso(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
+{
+	struct skb_shared_info *shinfo;
+	unsigned int l4len, hdrlen;
+	struct tcphdr *tcph;
+	__be16 payload_len;
+
+	if (skb_cow_head(skb, 0))
+		return true;
+
+	shinfo = skb_shinfo(skb);
+
+	tcph = (struct tcphdr *)skb_checksum_start(skb);
+	l4len = __tcp_hdrlen(tcph);
+	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
+	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);
+
+	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L4_HLEN, l4len / 4) |
+			     FIELD_PREP(MPNIC_TWD_MSS, shinfo->gso_size) |
+			     MPNIC_TWD_FLAG_REQ_LSO);
+
+	hdrlen = skb_checksum_start_offset(skb) + l4len;
+	MPNIC_XMIT_CB(skb)->bytecount += (shinfo->gso_segs - 1) * hdrlen;
+	MPNIC_XMIT_CB(skb)->gso_segs = shinfo->gso_segs;
+
+	u64_stats_update_begin(&ring->stats.syncp);
+	u64_stats_add(&ring->stats.twq.hw_gso_wire, shinfo->gso_segs);
+	u64_stats_update_end(&ring->stats.syncp);
+
+	return false;
+}
+
+static bool
 mpnic_tx_offloads(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 {
 	struct mpnic_tx_hdrs hdrs = {};
 
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
-		return;
+		return false;
 
 	mpnic_tx_fill_hdrs(skb, &hdrs);
 
 	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_CSUM_OFFSET,
-					skb->csum_offset / 2) |
-			     MPNIC_TWD_FLAG_REQ_CSO);
+					skb->csum_offset / 2));
 
-	u64_stats_update_begin(&ring->stats.syncp);
-	u64_stats_inc(&ring->stats.twq.csum_partial);
-	u64_stats_update_end(&ring->stats.syncp);
+	if (skb_is_gso(skb)) {
+		if (mpnic_tx_lso(ring, skb, meta))
+			return true;
+	} else {
+		*meta |= cpu_to_le64(MPNIC_TWD_FLAG_REQ_CSO);
+
+		u64_stats_update_begin(&ring->stats.syncp);
+		u64_stats_inc(&ring->stats.twq.csum_partial);
+		u64_stats_update_end(&ring->stats.syncp);
+	}
 
 	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L2_HLEN,
 					skb_mac_header_len(skb) / 2) |
-			     FIELD_PREP(MPNIC_TWD_L3_IHLEN, hdrs.i3len / 2));
+			     FIELD_PREP(MPNIC_TWD_L3_IHLEN, hdrs.i3len / 2) |
+			     FIELD_PREP(MPNIC_TWD_L3_TYPE, hdrs.l3_type) |
+			     FIELD_PREP(MPNIC_TWD_L4_TYPE, hdrs.l4_type));
+
+	return false;
 }
 
 #define MPNIC_MIN_FRAME_LEN	60
@@ -261,9 +315,11 @@ static netdev_tx_t mpnic_xmit_frame_ring(struct sk_buff *skb,
 	*meta = cpu_to_le64(MPNIC_TWD_FLAG_DEST_MAC);
 
 	MPNIC_XMIT_CB(skb)->bytecount = skb->len;
+	MPNIC_XMIT_CB(skb)->gso_segs = 1;
 	MPNIC_XMIT_CB(skb)->desc_count = 0;
 
-	mpnic_tx_offloads(ring, skb, meta);
+	if (mpnic_tx_offloads(ring, skb, meta))
+		goto err_free;
 
 	if (mpnic_tx_map(ring, skb, meta))
 		goto err_free;
@@ -312,6 +368,10 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
 	    l2len + i3len + sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE > 256)
 		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 
+	if (skb_is_gso(skb) &&
+	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
+		return features & ~NETIF_F_GSO_MASK;
+
 	return features;
 }
 
@@ -319,7 +379,7 @@ static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
 			     struct mpnic_q_triad *qt, bool discard,
 			     unsigned int hw_head)
 {
-	u64 total_bytes = 0, total_packets = 0;
+	u64 total_bytes = 0, total_packets = 0, total_skbs = 0;
 	struct mpnic_ring *ring = &qt->sub0;
 	struct mpnic_ring *cmpl = &qt->cmpl;
 	unsigned int head = ring->head;
@@ -357,7 +417,8 @@ static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
 		}
 
 		total_bytes += MPNIC_XMIT_CB(skb)->bytecount;
-		total_packets++;
+		total_packets += MPNIC_XMIT_CB(skb)->gso_segs;
+		total_skbs++;
 
 		napi_consume_skb(skb, napi_budget);
 	}
@@ -370,7 +431,7 @@ static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,
 	if (discard) {
 		local_bh_disable();
 		u64_stats_update_begin(&ring->stats.syncp);
-		u64_stats_add(&ring->stats.twq.dropped, total_packets);
+		u64_stats_add(&ring->stats.twq.dropped, total_skbs);
 		u64_stats_update_end(&ring->stats.syncp);
 		local_bh_enable();
 		return;
@@ -811,6 +872,7 @@ static void mpnic_aggregate_ring_twq_counters(struct mpnic_base_stats *base,
 {
 	base->tx.dropped += u64_stats_read(&twq->stats.twq.dropped);
 	base->tx.csum_partial += u64_stats_read(&twq->stats.twq.csum_partial);
+	base->tx.hw_gso_wire += u64_stats_read(&twq->stats.twq.hw_gso_wire);
 	base->tx.stop += u64_stats_read(&twq->stats.twq.stop);
 }
 
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
index 714f09f4160e..0d732ac4dde9 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.h
@@ -90,6 +90,7 @@ struct mpnic_queue_stats {
 		struct {
 			u64_stats_t dropped;
 			u64_stats_t csum_partial;
+			u64_stats_t hw_gso_wire;
 			u64_stats_t stop;
 		} twq;
 		struct {

-- 
2.52.0

Re: [PATCH net-next v2 3/6] eth: mpnic: TCP segmentation offload

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] At this commit, mpnic_xmit_frame_ring()
  (drivers/net/ethernet/meta/mpnic/mpnic_txrx.c:306) calls…
- [Medium] eth: mpnic: GSO validation bypass in mpnic_features_check()

--

eth: mpnic: TCP segmentation offload

This adds hardware segmentation for non-encapsulated TCPv4/TCPv6 packets
to the mpnic driver. It also programs the TOF mode and TCP flag clear
registers, and reports hw_gso_wire_packets through the netdev qstats API.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> index a35ce8998409d..b78f8599bbe8f 100644
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c

[ ... ]

> @@ -312,6 +368,10 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
>  	    l2len + i3len + sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE > 256)
>  		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
>  
> +	if (skb_is_gso(skb) &&
> +	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
> +		return features & ~NETIF_F_GSO_MASK;
> +
>  	return features;
>  }

[Severity: Medium]
Does this MSS check cover every GSO skb that reaches mpnic_tx_lso()?
Earlier in mpnic_features_check() there is an early return when
skb->ip_summed != CHECKSUM_PARTIAL. For those skbs, neither the header
geometry checks above nor the new gso_size check run, and NETIF_F_TSO
and NETIF_F_TSO6 stay set.

Those skbs can still end up offloaded. __virtio_net_hdr_to_skb()
accepts VIRTIO_NET_HDR_GSO_TCPV4/TCPV6 without
VIRTIO_NET_HDR_F_NEEDS_CSUM. So a tap or virtio guest, or a
PACKET_VNET_HDR sender, can hand us a GSO skb with CHECKSUM_NONE and
SKB_GSO_DODGY.

The path from there looks like this:

- validate_xmit_skb() sees netif_needs_gso() and calls
  skb_gso_segment().
- tcp4_gso_segment() or tcp6_gso_segment() seeds the pseudo-header
  checksum and switches ip_summed to CHECKSUM_PARTIAL.
- tcp_gso_segment() then returns NULL, because skb_gso_ok() is still
  true for the advertised features.

The original skb then goes to mpnic_xmit_frame() unsegmented, having
skipped every check in this function.

In mpnic_tx_lso(), shinfo->gso_size then goes into the 14-bit
MPNIC_TWD_MSS field unchecked. gso_features_check() only bounds
gso_segs and skb->len. A gso_size of 16384 with a total length under
64K therefore gets through, and FIELD_PREP() truncates it to an MSS
of 0.

The header lengths that mpnic_tx_offloads() writes into
MPNIC_TWD_L2_HLEN and MPNIC_TWD_L3_IHLEN are also unvalidated for
these skbs. The same goes for the even-length and 256 byte header
limits. Here the transport header was set by the flow dissector, not
by a trusted stack.

What does the hardware do with an MSS of 0, or with truncated header
lengths? At best it seems to put malformed segments on the wire.

I see two possible fixes, and I'm not sure which you'd prefer:

- Clear NETIF_F_GSO_MASK for GSO skbs that are not CHECKSUM_PARTIAL
  yet.
- Move the gso_size check above the early return, and validate the
  headers from skb_transport_header() in that case.

Either way, this patch is the one that turns TSO on, so the fix
probably belongs here.

[Severity: Medium]
Can a TCP GSO skb shorter than MPNIC_MIN_FRAME_LEN get to hardware LSO
here? The only GSO fallback in this check is for a gso_size that doesn't
fit MPNIC_TWD_MSS. Eth + IPv4 + TCP headers add up to 54 bytes, so a TCPv4
GSO skb can be 56 bytes long with gso_size 1 and gso_segs 2.

mpnic_xmit_frame_ring() pads every skb before the offloads are set up:

	if (skb_put_padto(skb, MPNIC_MIN_FRAME_LEN))
		goto err_drop;

That puts the zero pad inside the buffer lengths that mpnic_tx_map()
programs. Will the LSO engine then segment the pad as TCP payload? That
would produce more wire segments than shinfo->gso_segs, carrying zero
bytes at sequence numbers past the real data.

The checksum seed also looks wrong for these skbs. mpnic_tx_lso() takes
the length out of the pseudo-header using the padded skb->len:

	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);

The sender, or tcp4_gro_complete(), seeded tcph->check with the unpadded
L4 length. So the seed ends up off by the pad size. Won't every segment
the hardware generates then carry a bad TCP checksum?

One way to reach this is a remote peer sending back-to-back non-PSH
1-byte segments. A forwarding host GRO-merges them and sends them out an
mpnic port. tun/tap/virtio and PACKET_VNET_HDR sources can also build
such skbs.

Now that NETIF_F_TSO is advertised, these skbs reach the driver
unsegmented. Before this patch GSO was never offloaded, so the padding
did no harm.

The next patch in the series, "eth: mpnic: UDP segmentation offload",
adds skb->len < MPNIC_MIN_FRAME_LEN to this condition. Could that check
move into this patch, so this commit doesn't send bad segments on its own?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-mpnic-lso-v2-0-ccd37b051d08%40gmail.com

Re: [PATCH net-next v2 3/6] eth: mpnic: TCP segmentation offload

[Daniel Zahka]

On Thu Nov 12, 2026 at 5:08 AM EST, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] At this commit, mpnic_xmit_frame_ring()
>   (drivers/net/ethernet/meta/mpnic/mpnic_txrx.c:306) calls…
> - [Medium] eth: mpnic: GSO validation bypass in mpnic_features_check()
>
> --
>
> eth: mpnic: TCP segmentation offload
>
> This adds hardware segmentation for non-encapsulated TCPv4/TCPv6 packets
> to the mpnic driver. It also programs the TOF mode and TCP flag clear
> registers, and reports hw_gso_wire_packets through the netdev qstats API.
>
>> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> index a35ce8998409d..b78f8599bbe8f 100644
>> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>
> [ ... ]
>
>> @@ -312,6 +368,10 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
>>  	    l2len + i3len + sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE > 256)
>>  		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
>>  
>> +	if (skb_is_gso(skb) &&
>> +	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
>> +		return features & ~NETIF_F_GSO_MASK;
>> +
>>  	return features;
>>  }
>
> [Severity: Medium]
> Does this MSS check cover every GSO skb that reaches mpnic_tx_lso()?
> Earlier in mpnic_features_check() there is an early return when
> skb->ip_summed != CHECKSUM_PARTIAL. For those skbs, neither the header
> geometry checks above nor the new gso_size check run, and NETIF_F_TSO
> and NETIF_F_TSO6 stay set.
>
> Those skbs can still end up offloaded. __virtio_net_hdr_to_skb()
> accepts VIRTIO_NET_HDR_GSO_TCPV4/TCPV6 without
> VIRTIO_NET_HDR_F_NEEDS_CSUM. So a tap or virtio guest, or a
> PACKET_VNET_HDR sender, can hand us a GSO skb with CHECKSUM_NONE and
> SKB_GSO_DODGY.
>
> The path from there looks like this:
>
> - validate_xmit_skb() sees netif_needs_gso() and calls
>   skb_gso_segment().
> - tcp4_gso_segment() or tcp6_gso_segment() seeds the pseudo-header
>   checksum and switches ip_summed to CHECKSUM_PARTIAL.
> - tcp_gso_segment() then returns NULL, because skb_gso_ok() is still
>   true for the advertised features.
>
> The original skb then goes to mpnic_xmit_frame() unsegmented, having
> skipped every check in this function.
>
> In mpnic_tx_lso(), shinfo->gso_size then goes into the 14-bit
> MPNIC_TWD_MSS field unchecked. gso_features_check() only bounds
> gso_segs and skb->len. A gso_size of 16384 with a total length under
> 64K therefore gets through, and FIELD_PREP() truncates it to an MSS
> of 0.
>
> The header lengths that mpnic_tx_offloads() writes into
> MPNIC_TWD_L2_HLEN and MPNIC_TWD_L3_IHLEN are also unvalidated for
> these skbs. The same goes for the even-length and 256 byte header
> limits. Here the transport header was set by the flow dissector, not
> by a trusted stack.
>
> What does the hardware do with an MSS of 0, or with truncated header
> lengths? At best it seems to put malformed segments on the wire.
>
> I see two possible fixes, and I'm not sure which you'd prefer:
>
> - Clear NETIF_F_GSO_MASK for GSO skbs that are not CHECKSUM_PARTIAL
>   yet.

This came up in a local sashiko run, but I declined it. The issue is
from vnet headers that request gso without csum. A handful of drivers
have an early return passing through the features if the skb is not
CHECKSUM_PARTIAL. Seems like something core might want to handle.

> - Move the gso_size check above the early return, and validate the
>   headers from skb_transport_header() in that case.
>
> Either way, this patch is the one that turns TSO on, so the fix
> probably belongs here.
>
> [Severity: Medium]
> Can a TCP GSO skb shorter than MPNIC_MIN_FRAME_LEN get to hardware LSO
> here? The only GSO fallback in this check is for a gso_size that doesn't
> fit MPNIC_TWD_MSS. Eth + IPv4 + TCP headers add up to 54 bytes, so a TCPv4
> GSO skb can be 56 bytes long with gso_size 1 and gso_segs 2.
>

This is the same feedback I addressed in the udp segmentation patch.
Shouldn't really be reachable for TSO skbs, but the check is there in
the next patch anyway.

> mpnic_xmit_frame_ring() pads every skb before the offloads are set up:
>
> 	if (skb_put_padto(skb, MPNIC_MIN_FRAME_LEN))
> 		goto err_drop;
>
> That puts the zero pad inside the buffer lengths that mpnic_tx_map()
> programs. Will the LSO engine then segment the pad as TCP payload? That
> would produce more wire segments than shinfo->gso_segs, carrying zero
> bytes at sequence numbers past the real data.
>
> The checksum seed also looks wrong for these skbs. mpnic_tx_lso() takes
> the length out of the pseudo-header using the padded skb->len:
>
> 	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
> 	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);
>
> The sender, or tcp4_gro_complete(), seeded tcph->check with the unpadded
> L4 length. So the seed ends up off by the pad size. Won't every segment
> the hardware generates then carry a bad TCP checksum?
>
> One way to reach this is a remote peer sending back-to-back non-PSH
> 1-byte segments. A forwarding host GRO-merges them and sends them out an
> mpnic port. tun/tap/virtio and PACKET_VNET_HDR sources can also build
> such skbs.
>
> Now that NETIF_F_TSO is advertised, these skbs reach the driver
> unsegmented. Before this patch GSO was never offloaded, so the padding
> did no harm.
>
> The next patch in the series, "eth: mpnic: UDP segmentation offload",
> adds skb->len < MPNIC_MIN_FRAME_LEN to this condition. Could that check
> move into this patch, so this commit doesn't send bad segments on its own?

[PATCH net-next v2 4/6] eth: mpnic: UDP segmentation offload

[Daniel Zahka]

Support segmentation of non-encapsulated udp4/udp6 packets. Like for
tcp, the segmentation engine expects the udp csum to be seeded with 0
length.

The HW inverts UDP checksums computed as 0 during segmentation.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- fall back to SW GSO for skbs shorter than MPNIC_MIN_FRAME_LEN, since
  HW LSO would segment the zero padding as payload
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  2 ++
 drivers/net/ethernet/meta/mpnic/mpnic_init.c   |  1 +
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c |  3 ++-
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 36 +++++++++++++++++++-------
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index 672ce0a98c22..2cd627d62bf5 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -20,6 +20,7 @@
 #define MPNIC_TWD_L4_TYPE		DESC_GENMASK(33, 32)
 enum {
 	MPNIC_TWD_L4_TYPE_TCP	= 1,
+	MPNIC_TWD_L4_TYPE_UDP	= 2,
 };
 
 #define MPNIC_TWD_FLAG_REQ_LSO		DESC_BIT(35)
@@ -335,6 +336,7 @@ enum {
 
 #define MPNIC_TOF_MODE_IPV4_ZERO_CSUM		CSR_BIT(1)
 #define MPNIC_TOF_MODE_TCP_ZERO_CSUM		CSR_BIT(2)
+#define MPNIC_TOF_MODE_UDP_ZERO_CSUM		CSR_BIT(3)
 #define MPNIC_TOF_MODE_L4_ZERO_CSUM		CSR_BIT(4)
 #define MPNIC_TOF_TCP_FLAG_CLR		0x7e3030		/* 0x1f8c0c0 */
 #define MPNIC_TOF_TCP_FLAG_CLR_FIRST_SEGMENT	CSR_GENMASK(8, 0)
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_init.c b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
index b9a41656f8f7..ab832ec5de0e 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_init.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_init.c
@@ -304,6 +304,7 @@ static void mpnic_txglb_init(struct mpnic_dev *mpd)
 			      MPNIC_IPV4_ID_MODE_INCREMENT) |
 		   FIELD_PREP(MPNIC_TOF_MODE_IPV4_ZERO_CSUM, 1) |
 		   FIELD_PREP(MPNIC_TOF_MODE_TCP_ZERO_CSUM, 1) |
+		   FIELD_PREP(MPNIC_TOF_MODE_UDP_ZERO_CSUM, 0) |
 		   FIELD_PREP(MPNIC_TOF_MODE_L4_ZERO_CSUM, 0));
 	mpnic_wr64(mpd, MPNIC_TOF_TCP_FLAG_CLR,
 		   FIELD_PREP(MPNIC_TOF_TCP_FLAG_CLR_FIRST_SEGMENT,
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index bde327ad3154..bf3b459ba5be 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -328,7 +328,8 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 		NETIF_F_RXCSUM |
 		NETIF_F_TSO |
 		NETIF_F_TSO_ECN |
-		NETIF_F_TSO6;
+		NETIF_F_TSO6 |
+		NETIF_F_GSO_UDP_L4;
 
 	netdev->hw_features |= netdev->features;
 	netdev->vlan_features |= netdev->features;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index b78f8599bbe8..2a0560b7dd19 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -223,29 +223,44 @@ static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
 		return;
 	}
 
-	hdrs->l4_type = MPNIC_TWD_L4_TYPE_TCP;
+	if (skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
+		hdrs->l4_type = MPNIC_TWD_L4_TYPE_TCP;
+	else
+		hdrs->l4_type = MPNIC_TWD_L4_TYPE_UDP;
+
 	hdrs->i3len = skb_transport_offset(skb) - skb_network_offset(skb);
 	hdrs->l3_type = ip_hdr(skb)->version == 4 ?
 			MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
 }
 
 static bool
-mpnic_tx_lso(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
+mpnic_tx_lso(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta,
+	     const struct mpnic_tx_hdrs *hdrs)
 {
 	struct skb_shared_info *shinfo;
 	unsigned int l4len, hdrlen;
-	struct tcphdr *tcph;
-	__be16 payload_len;
+	unsigned char *l4hdr;
 
 	if (skb_cow_head(skb, 0))
 		return true;
 
 	shinfo = skb_shinfo(skb);
+	l4hdr = skb_checksum_start(skb);
+
+	if (hdrs->l4_type == MPNIC_TWD_L4_TYPE_TCP) {
+		struct tcphdr *tcph = (struct tcphdr *)l4hdr;
+		__be16 payload_len;
 
-	tcph = (struct tcphdr *)skb_checksum_start(skb);
-	l4len = __tcp_hdrlen(tcph);
-	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
-	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);
+		l4len = __tcp_hdrlen(tcph);
+		payload_len = cpu_to_be16(skb->len -
+					  skb_checksum_start_offset(skb));
+		csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);
+	} else {
+		struct udphdr *udph = (struct udphdr *)l4hdr;
+
+		l4len = sizeof(*udph);
+		csum_replace_by_diff(&udph->check, (__force __wsum)udph->len);
+	}
 
 	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L4_HLEN, l4len / 4) |
 			     FIELD_PREP(MPNIC_TWD_MSS, shinfo->gso_size) |
@@ -276,7 +291,7 @@ mpnic_tx_offloads(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 					skb->csum_offset / 2));
 
 	if (skb_is_gso(skb)) {
-		if (mpnic_tx_lso(ring, skb, meta))
+		if (mpnic_tx_lso(ring, skb, meta, &hdrs))
 			return true;
 	} else {
 		*meta |= cpu_to_le64(MPNIC_TWD_FLAG_REQ_CSO);
@@ -369,7 +384,8 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
 		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
 
 	if (skb_is_gso(skb) &&
-	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
+	    (skb->len < MPNIC_MIN_FRAME_LEN ||
+	     !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size)))
 		return features & ~NETIF_F_GSO_MASK;
 
 	return features;

-- 
2.52.0

Re: [PATCH net-next v2 4/6] eth: mpnic: UDP segmentation offload

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch adds `skb->len < MPNIC_MIN_FRAME_LEN ||` to the
  generic `skb_is_gso(skb)` branch of mpnic_features_check()…

--

eth: mpnic: UDP segmentation offload

This commit adds NETIF_F_GSO_UDP_L4 support for non-encapsulated udp4
and udp6 packets. As for TCP, the UDP checksum is seeded with a zero
length. It also makes mpnic_features_check() fall back to software GSO
for skbs shorter than MPNIC_MIN_FRAME_LEN.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> index b78f8599bbe8f..2a0560b7dd19e 100644
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c

[ ... ]

> @@ -369,7 +384,8 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
>  		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
>  
>  	if (skb_is_gso(skb) &&
> -	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
> +	    (skb->len < MPNIC_MIN_FRAME_LEN ||
> +	     !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size)))
>  		return features & ~NETIF_F_GSO_MASK;

[Severity: Medium]
This check applies to every GSO type. That means TCP GSO skbs shorter
than 60 bytes now fall back to software GSO too, not just UDP ones.

The commit message body only talks about UDP. The only mention of this
check is the v2 changelog below the "---", and that text is dropped when
the patch is applied:

  v2:
  - fall back to SW GSO for skbs shorter than MPNIC_MIN_FRAME_LEN, since
    HW LSO would segment the zero padding as payload

Should the commit message describe the effect on TCP? Or should the
check move into the earlier "eth: mpnic: TCP segmentation offload"
patch?

With only the TSO patch applied, TCP seems to have the same problem. In
that version, mpnic_features_check() rejected GSO only when gso_size
didn't fit MPNIC_TWD_MSS. So a short TCP GSO skb reaches
mpnic_xmit_frame_ring() with HW TSO enabled, and gets padded first:

drivers/net/ethernet/meta/mpnic/mpnic_txrx.c:mpnic_xmit_frame_ring() {
    ...
	if (skb_put_padto(skb, MPNIC_MIN_FRAME_LEN))
		goto err_drop;
    ...
}

Then mpnic_tx_lso() seeds the TCP pseudo-checksum from the padded
length:

	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);

Locally generated TCP GSO skbs are always longer than 60 bytes. A GSO
skb built from a virtio_net_hdr can be shorter, though. Sources include
tun/vhost and packet sockets with a vnet header. Such an skb can carry
a tiny gso_size and only a few bytes of payload.

In that case, wouldn't the receiver get the zero padding as TCP stream
data, with a valid checksum?

The check is still present at the end of the series, so the final tree
is fine. The TSO commit on its own would still corrupt data if someone
bisects to it or backports it alone.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-mpnic-lso-v2-0-ccd37b051d08%40gmail.com

Re: [PATCH net-next v2 4/6] eth: mpnic: UDP segmentation offload

[Daniel Zahka]

On Thu Nov 12, 2026 at 5:08 AM EST, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] This patch adds `skb->len < MPNIC_MIN_FRAME_LEN ||` to the
>   generic `skb_is_gso(skb)` branch of mpnic_features_check()…
>
> --
>
> eth: mpnic: UDP segmentation offload
>
> This commit adds NETIF_F_GSO_UDP_L4 support for non-encapsulated udp4
> and udp6 packets. As for TCP, the UDP checksum is seeded with a zero
> length. It also makes mpnic_features_check() fall back to software GSO
> for skbs shorter than MPNIC_MIN_FRAME_LEN.
>
>> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> index b78f8599bbe8f..2a0560b7dd19e 100644
>> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>
> [ ... ]
>
>> @@ -369,7 +384,8 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
>>  		return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
>>  
>>  	if (skb_is_gso(skb) &&
>> -	    !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size))
>> +	    (skb->len < MPNIC_MIN_FRAME_LEN ||
>> +	     !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size)))
>>  		return features & ~NETIF_F_GSO_MASK;
>
> [Severity: Medium]
> This check applies to every GSO type. That means TCP GSO skbs shorter
> than 60 bytes now fall back to software GSO too, not just UDP ones.
>
> The commit message body only talks about UDP. The only mention of this
> check is the v2 changelog below the "---", and that text is dropped when
> the patch is applied:
>
>   v2:
>   - fall back to SW GSO for skbs shorter than MPNIC_MIN_FRAME_LEN, since
>     HW LSO would segment the zero padding as payload
>
> Should the commit message describe the effect on TCP? Or should the
> check move into the earlier "eth: mpnic: TCP segmentation offload"
> patch?
>
> With only the TSO patch applied, TCP seems to have the same problem. In
> that version, mpnic_features_check() rejected GSO only when gso_size
> didn't fit MPNIC_TWD_MSS. So a short TCP GSO skb reaches
> mpnic_xmit_frame_ring() with HW TSO enabled, and gets padded first:
>
> drivers/net/ethernet/meta/mpnic/mpnic_txrx.c:mpnic_xmit_frame_ring() {
>     ...
> 	if (skb_put_padto(skb, MPNIC_MIN_FRAME_LEN))
> 		goto err_drop;
>     ...
> }
>
> Then mpnic_tx_lso() seeds the TCP pseudo-checksum from the padded
> length:
>
> 	payload_len = cpu_to_be16(skb->len - skb_checksum_start_offset(skb));
> 	csum_replace_by_diff(&tcph->check, (__force __wsum)payload_len);
>
> Locally generated TCP GSO skbs are always longer than 60 bytes. A GSO
> skb built from a virtio_net_hdr can be shorter, though. Sources include
> tun/vhost and packet sockets with a vnet header. Such an skb can carry
> a tiny gso_size and only a few bytes of payload.
>

Local sashiko review surfaced the same complaint and wanted me to pull
the check forward one commit. I decided against it. The UDP case is
easily reachable because the user chooses the gso size. I don't think
producing a TSO skb shorter than 60B is really possible. The
virtio_net_hdr cases should have a min GSO size of TCP_MIN_GSO_SIZE set
in virtio_net_hdr_to_skb().

> In that case, wouldn't the receiver get the zero padding as TCP stream
> data, with a valid checksum?
>
> The check is still present at the end of the series, so the final tree
> is fine. The TSO commit on its own would still corrupt data if someone
> bisects to it or backports it alone.

[PATCH net-next v2 5/6] eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation

[Daniel Zahka]

The device can fully segment 4-in-6 and 6-in-6 packet types. Add
mpnic_features_check_encap_gso() to check the skb for some additional
constraints, such as ip6 non-atomic frag ext header.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- derive inner L3 header length from csum_start rather than
  inner_transport_header, which can be stale for DODGY skbs
- check the outer header is IPv6 in mpnic_ipxip6_gso_ok(), since
  SKB_GSO_IPXIP6 can be set by a nested tunnel under an IPv4 outer
- move hw_enc_features setup to the Tx checksum offload patch
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  3 ++
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c |  1 +
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 53 ++++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index 2cd627d62bf5..63282916eb13 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -14,6 +14,7 @@
 
 /* Transmit Work Descriptor Format */
 #define MPNIC_TWD_L2_HLEN		DESC_GENMASK(5, 0)
+#define MPNIC_TWD_L3_OHLEN		DESC_GENMASK(15, 8)
 #define MPNIC_TWD_L3_IHLEN		DESC_GENMASK(23, 16)
 #define MPNIC_TWD_CSUM_OFFSET		DESC_GENMASK(27, 24)
 #define MPNIC_TWD_L4_HLEN		DESC_GENMASK(31, 28)
@@ -30,6 +31,8 @@ enum {
 enum {
 	MPNIC_TWD_L3_TYPE_IPV4	= 1,
 	MPNIC_TWD_L3_TYPE_IPV6	= 2,
+	MPNIC_TWD_L3_TYPE_V6V6	= 3,
+	MPNIC_TWD_L3_TYPE_V6V4	= 4,
 };
 
 #define MPNIC_TWD_FLAG_DEST_MAC		DESC_BIT(43)
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index bf3b459ba5be..032a5ee92a08 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -323,6 +323,7 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 	mpn->num_napi = queues;
 
 	netdev->features |=
+		NETIF_F_GSO_IPXIP6 |
 		NETIF_F_SG |
 		NETIF_F_HW_CSUM |
 		NETIF_F_RXCSUM |
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index 2a0560b7dd19..839feb4716b6 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -22,6 +22,7 @@ struct mpnic_xmit_cb {
 
 struct mpnic_tx_hdrs {
 	unsigned int i3len;
+	unsigned int o3len;
 	unsigned int l3_type;
 	unsigned int l4_type;
 };
@@ -218,6 +219,9 @@ mpnic_tx_map(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 
 static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
 {
+	unsigned int net_off = skb_network_offset(skb);
+	unsigned int inner_off;
+
 	if (!skb_is_gso(skb)) {
 		hdrs->i3len = skb_checksum_start(skb) - skb_network_header(skb);
 		return;
@@ -228,9 +232,17 @@ static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
 	else
 		hdrs->l4_type = MPNIC_TWD_L4_TYPE_UDP;
 
-	hdrs->i3len = skb_transport_offset(skb) - skb_network_offset(skb);
-	hdrs->l3_type = ip_hdr(skb)->version == 4 ?
-			MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
+	if (!skb->encapsulation) {
+		hdrs->i3len = skb_transport_offset(skb) - net_off;
+		hdrs->l3_type = ip_hdr(skb)->version == 4 ?
+				MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
+	} else {
+		inner_off = skb_inner_network_offset(skb);
+		hdrs->o3len = inner_off - net_off;
+		hdrs->i3len = skb_checksum_start_offset(skb) - inner_off;
+		hdrs->l3_type = inner_ip_hdr(skb)->version == 4 ?
+				MPNIC_TWD_L3_TYPE_V6V4 : MPNIC_TWD_L3_TYPE_V6V6;
+	}
 }
 
 static bool
@@ -304,6 +316,7 @@ mpnic_tx_offloads(struct mpnic_ring *ring, struct sk_buff *skb, __le64 *meta)
 	*meta |= cpu_to_le64(FIELD_PREP(MPNIC_TWD_L2_HLEN,
 					skb_mac_header_len(skb) / 2) |
 			     FIELD_PREP(MPNIC_TWD_L3_IHLEN, hdrs.i3len / 2) |
+			     FIELD_PREP(MPNIC_TWD_L3_OHLEN, hdrs.o3len / 2) |
 			     FIELD_PREP(MPNIC_TWD_L3_TYPE, hdrs.l3_type) |
 			     FIELD_PREP(MPNIC_TWD_L4_TYPE, hdrs.l4_type));
 
@@ -362,6 +375,35 @@ netdev_tx_t mpnic_xmit_frame(struct sk_buff *skb, struct net_device *dev)
 	return mpnic_xmit_frame_ring(skb, mpn->tx[skb_get_queue_mapping(skb)]);
 }
 
+static bool mpnic_ipxip6_gso_ok(struct sk_buff *skb)
+{
+	struct ipv6hdr *ip6h;
+	__be16 frag_off;
+	u8 nexthdr;
+	int start;
+
+	ip6h = ipv6_hdr(skb);
+	if (ip6h->version != 6)
+		return false;
+
+	nexthdr = ip6h->nexthdr;
+	start = skb_network_offset(skb) + sizeof(*ip6h);
+	start = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
+
+	return !frag_off && start == skb_inner_network_offset(skb) &&
+	       (nexthdr == IPPROTO_IPV6 || nexthdr == IPPROTO_IPIP);
+}
+
+static netdev_features_t
+mpnic_features_check_encap_gso(struct sk_buff *skb, netdev_features_t features)
+{
+	if (skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP6 &&
+	    mpnic_ipxip6_gso_ok(skb))
+		return features;
+
+	return features & ~NETIF_F_GSO_IPXIP6;
+}
+
 netdev_features_t mpnic_features_check(struct sk_buff *skb,
 				       struct net_device *dev,
 				       netdev_features_t features)
@@ -388,7 +430,10 @@ netdev_features_t mpnic_features_check(struct sk_buff *skb,
 	     !FIELD_FIT(MPNIC_TWD_MSS, skb_shinfo(skb)->gso_size)))
 		return features & ~NETIF_F_GSO_MASK;
 
-	return features;
+	if (!skb->encapsulation || !skb_is_gso(skb))
+		return features;
+
+	return mpnic_features_check_encap_gso(skb, features);
 }
 
 static void mpnic_clean_twq0(struct mpnic_napi_vector *nv, int napi_budget,

-- 
2.52.0

Re: [PATCH net-next v2 5/6] eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] mpnic_ipxip6_gso_ok() only checks the outer IPv6 header chain.

--

eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation

This patch turns on NETIF_F_GSO_IPXIP6 for mpnic. The device can segment
4-in-6 and 6-in-6 packets. The patch adds the outer L3 header length and
two new L3 types to the Tx descriptor, and adds
mpnic_features_check_encap_gso() to reject skbs the hardware cannot
handle.

> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> index 2a0560b7dd19e..839feb4716b60 100644
> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c

[ ... ]

> @@ -228,9 +232,17 @@ static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
>  	else
>  		hdrs->l4_type = MPNIC_TWD_L4_TYPE_UDP;
>  
> -	hdrs->i3len = skb_transport_offset(skb) - skb_network_offset(skb);
> -	hdrs->l3_type = ip_hdr(skb)->version == 4 ?
> -			MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
> +	if (!skb->encapsulation) {
> +		hdrs->i3len = skb_transport_offset(skb) - net_off;
> +		hdrs->l3_type = ip_hdr(skb)->version == 4 ?
> +				MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
> +	} else {
> +		inner_off = skb_inner_network_offset(skb);
> +		hdrs->o3len = inner_off - net_off;
> +		hdrs->i3len = skb_checksum_start_offset(skb) - inner_off;
> +		hdrs->l3_type = inner_ip_hdr(skb)->version == 4 ?
> +				MPNIC_TWD_L3_TYPE_V6V4 : MPNIC_TWD_L3_TYPE_V6V6;
> +	}
>  }

[ ... ]

> @@ -362,6 +375,35 @@ netdev_tx_t mpnic_xmit_frame(struct sk_buff *skb, struct net_device *dev)
>  	return mpnic_xmit_frame_ring(skb, mpn->tx[skb_get_queue_mapping(skb)]);
>  }
>  
> +static bool mpnic_ipxip6_gso_ok(struct sk_buff *skb)
> +{
> +	struct ipv6hdr *ip6h;
> +	__be16 frag_off;
> +	u8 nexthdr;
> +	int start;
> +
> +	ip6h = ipv6_hdr(skb);
> +	if (ip6h->version != 6)
> +		return false;
> +
> +	nexthdr = ip6h->nexthdr;
> +	start = skb_network_offset(skb) + sizeof(*ip6h);
> +	start = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
> +
> +	return !frag_off && start == skb_inner_network_offset(skb) &&
> +	       (nexthdr == IPPROTO_IPV6 || nexthdr == IPPROTO_IPIP);
> +}

[Severity: Medium]
Can a nested tunnel still get past this check?

mpnic_ipxip6_gso_ok() validates only the outer IPv6 header chain up to
skb_inner_network_offset(). It never checks that the inner L3 header
leads straight to the L4 header at csum_start.

Some paths reset the inner headers on an skb that is already
encapsulated. One is bpf_lwt_push_ip_encap() in net/core/lwt_bpf.c:

    skb_reset_inner_headers(skb);
    skb_reset_inner_mac_header(skb);  /* mac header is not yet set */
    skb_set_inner_protocol(skb, skb->protocol);
    skb->encapsulation = 1;

For an IPv6 outer header with nexthdr IPV6, handle_gso_encap() then adds
SKB_GSO_IPXIP6. BPF_LWT_ENCAP_SEG6 via bpf_push_seg6_encap() looks
similar.

One way to reach this is an ip6_tnl device whose underlay route has an
LWT BPF xmit program that pushes another IPv6 header:

    ip6_tnl xmit (encapsulation=1, SKB_GSO_IPXIP6)
      ip6_finish_output2()->lwtunnel_xmit()->bpf_xmit()
        bpf_lwt_push_ip_encap()   (inner headers reset)

The packet then looks like this:

    outer IPv6 -> middle IPv6 -> innermost IPv4/IPv6 -> TCP

Here inner_network_header points at the middle header. So
mpnic_ipxip6_gso_ok() sees an outer v6 header, nexthdr IPV6, and
start == inner offset, and it returns true.

The skb is DODGY, so validate_xmit_skb() runs software GSO validation. That
recursion ends in tcp_gso_segment(), where skb_gso_ok() succeeds against
hw_enc_features and NULL is returned. The skb then reaches
mpnic_xmit_frame() unsegmented.

mpnic_tx_fill_hdrs() would then program o3len = 40, i3len = 80 (or 60 for
an IPv4 innermost header), and L3_TYPE taken from the middle header's
version.

The descriptor only carries OHLEN and IHLEN. Would the hardware then
skip the per-segment fixups on the innermost header, i.e. IPv6
payload_len, or IPv4 tot_len/ID/checksum? That would put malformed
segments on the wire where software GSO would have produced correct
ones.

The v2 changelog says nested tunnels under an IPv4 outer are now
rejected, but this layout still has an IPv6 outer.

Should mpnic_ipxip6_gso_ok() also check that the inner L3 header ends
exactly at skb_checksum_start_offset() with TCP or UDP as its protocol?
For IPv4 that is ihl * 4. For IPv6 that is the header plus any extension
headers.

> +
> +static netdev_features_t
> +mpnic_features_check_encap_gso(struct sk_buff *skb, netdev_features_t features)
> +{
> +	if (skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP6 &&
> +	    mpnic_ipxip6_gso_ok(skb))
> +		return features;
> +
> +	return features & ~NETIF_F_GSO_IPXIP6;
> +}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-mpnic-lso-v2-0-ccd37b051d08%40gmail.com

Re: [PATCH net-next v2 5/6] eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation

[Daniel Zahka]

On Thu Nov 12, 2026 at 5:08 AM EST, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] mpnic_ipxip6_gso_ok() only checks the outer IPv6 header chain.
>
> --
>
> eth: mpnic: IPv4 and IPv6 in IPv6 tunnel segmentation
>
> This patch turns on NETIF_F_GSO_IPXIP6 for mpnic. The device can segment
> 4-in-6 and 6-in-6 packets. The patch adds the outer L3 header length and
> two new L3 types to the Tx descriptor, and adds
> mpnic_features_check_encap_gso() to reject skbs the hardware cannot
> handle.
>
>> diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> index 2a0560b7dd19e..839feb4716b60 100644
>> --- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>> +++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
>
> [ ... ]
>
>> @@ -228,9 +232,17 @@ static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
>>  	else
>>  		hdrs->l4_type = MPNIC_TWD_L4_TYPE_UDP;
>>  
>> -	hdrs->i3len = skb_transport_offset(skb) - skb_network_offset(skb);
>> -	hdrs->l3_type = ip_hdr(skb)->version == 4 ?
>> -			MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
>> +	if (!skb->encapsulation) {
>> +		hdrs->i3len = skb_transport_offset(skb) - net_off;
>> +		hdrs->l3_type = ip_hdr(skb)->version == 4 ?
>> +				MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
>> +	} else {
>> +		inner_off = skb_inner_network_offset(skb);
>> +		hdrs->o3len = inner_off - net_off;
>> +		hdrs->i3len = skb_checksum_start_offset(skb) - inner_off;
>> +		hdrs->l3_type = inner_ip_hdr(skb)->version == 4 ?
>> +				MPNIC_TWD_L3_TYPE_V6V4 : MPNIC_TWD_L3_TYPE_V6V6;
>> +	}
>>  }
>
> [ ... ]
>
>> @@ -362,6 +375,35 @@ netdev_tx_t mpnic_xmit_frame(struct sk_buff *skb, struct net_device *dev)
>>  	return mpnic_xmit_frame_ring(skb, mpn->tx[skb_get_queue_mapping(skb)]);
>>  }
>>  
>> +static bool mpnic_ipxip6_gso_ok(struct sk_buff *skb)
>> +{
>> +	struct ipv6hdr *ip6h;
>> +	__be16 frag_off;
>> +	u8 nexthdr;
>> +	int start;
>> +
>> +	ip6h = ipv6_hdr(skb);
>> +	if (ip6h->version != 6)
>> +		return false;
>> +
>> +	nexthdr = ip6h->nexthdr;
>> +	start = skb_network_offset(skb) + sizeof(*ip6h);
>> +	start = ipv6_skip_exthdr(skb, start, &nexthdr, &frag_off);
>> +
>> +	return !frag_off && start == skb_inner_network_offset(skb) &&
>> +	       (nexthdr == IPPROTO_IPV6 || nexthdr == IPPROTO_IPIP);
>> +}
>
> [Severity: Medium]
> Can a nested tunnel still get past this check?
>
> mpnic_ipxip6_gso_ok() validates only the outer IPv6 header chain up to
> skb_inner_network_offset(). It never checks that the inner L3 header
> leads straight to the L4 header at csum_start.
>
> Some paths reset the inner headers on an skb that is already
> encapsulated. One is bpf_lwt_push_ip_encap() in net/core/lwt_bpf.c:
>
>     skb_reset_inner_headers(skb);
>     skb_reset_inner_mac_header(skb);  /* mac header is not yet set */
>     skb_set_inner_protocol(skb, skb->protocol);
>     skb->encapsulation = 1;
>
> For an IPv6 outer header with nexthdr IPV6, handle_gso_encap() then adds
> SKB_GSO_IPXIP6. BPF_LWT_ENCAP_SEG6 via bpf_push_seg6_encap() looks
> similar.
>
> One way to reach this is an ip6_tnl device whose underlay route has an
> LWT BPF xmit program that pushes another IPv6 header:
>
>     ip6_tnl xmit (encapsulation=1, SKB_GSO_IPXIP6)
>       ip6_finish_output2()->lwtunnel_xmit()->bpf_xmit()
>         bpf_lwt_push_ip_encap()   (inner headers reset)
>
> The packet then looks like this:
>
>     outer IPv6 -> middle IPv6 -> innermost IPv4/IPv6 -> TCP
>
> Here inner_network_header points at the middle header. So
> mpnic_ipxip6_gso_ok() sees an outer v6 header, nexthdr IPV6, and
> start == inner offset, and it returns true.
>
> The skb is DODGY, so validate_xmit_skb() runs software GSO validation. That
> recursion ends in tcp_gso_segment(), where skb_gso_ok() succeeds against
> hw_enc_features and NULL is returned. The skb then reaches
> mpnic_xmit_frame() unsegmented.
>
> mpnic_tx_fill_hdrs() would then program o3len = 40, i3len = 80 (or 60 for
> an IPv4 innermost header), and L3_TYPE taken from the middle header's
> version.
>
> The descriptor only carries OHLEN and IHLEN. Would the hardware then
> skip the per-segment fixups on the innermost header, i.e. IPv6
> payload_len, or IPv4 tot_len/ID/checksum? That would put malformed
> segments on the wire where software GSO would have produced correct
> ones.
>
> The v2 changelog says nested tunnels under an IPv4 outer are now
> rejected, but this layout still has an IPv6 outer.
>
> Should mpnic_ipxip6_gso_ok() also check that the inner L3 header ends
> exactly at skb_checksum_start_offset() with TCP or UDP as its protocol?
> For IPv4 that is ihl * 4. For IPv6 that is the header plus any extension
> headers.
>

This suggestion is insane. I thought I could count on the stack to not
reset the inner headers more than once and handle nested encap like
iptunnel_handle_offloads() does, but I wasn't aware of these two cases
with LWT and seg6 encap...

It would be helpful if we could establish a definition about what the
inner headers of an skb refer to, or otherwise put some other bit of
metadata on the skb so that drivers don't need to parse the skb for
these cases.

>> +
>> +static netdev_features_t
>> +mpnic_features_check_encap_gso(struct sk_buff *skb, netdev_features_t features)
>> +{
>> +	if (skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP6 &&
>> +	    mpnic_ipxip6_gso_ok(skb))
>> +		return features;
>> +
>> +	return features & ~NETIF_F_GSO_IPXIP6;
>> +}

[PATCH net-next v2 6/6] eth: mpnic: add GSO_PARTIAL for other tunnels

[Daniel Zahka]

Tunnels that are not natively supported for segmentation can be
supported with GSO_PARTIAL. For GSO_PARTIAL, the driver uses L3 type
OTHER, which tells the device to suppress any updates to the l3 headers
of the packet. As a consequence of this, MANGLEID is required when the
inner l3 is ipv4, and for tunnels that have an outer l3 of ipv4, we need
to check if DF is set in mpnic_features_check_encap_gso(), because the
device will not edit the IDs of the wire packets.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- derive the L3 header length from csum_start rather than
  inner_transport_header, matching mpnic_features_check() and
  mpnic_tx_lso()
---
 drivers/net/ethernet/meta/mpnic/mpnic_csr.h    |  1 +
 drivers/net/ethernet/meta/mpnic/mpnic_netdev.c |  9 +++++++++
 drivers/net/ethernet/meta/mpnic/mpnic_txrx.c   | 21 ++++++++++++++++++++-
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
index 63282916eb13..03304e22d6ce 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_csr.h
@@ -29,6 +29,7 @@ enum {
 #define MPNIC_TWD_FLAG_REQ_COMPLETION	DESC_BIT(37)
 #define MPNIC_TWD_L3_TYPE		DESC_GENMASK(42, 40)
 enum {
+	MPNIC_TWD_L3_TYPE_OTHER	= 0,
 	MPNIC_TWD_L3_TYPE_IPV4	= 1,
 	MPNIC_TWD_L3_TYPE_IPV6	= 2,
 	MPNIC_TWD_L3_TYPE_V6V6	= 3,
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
index 032a5ee92a08..198aad631c65 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_netdev.c
@@ -322,7 +322,15 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 	mpn->num_rx_queues = queues;
 	mpn->num_napi = queues;
 
+	netdev->gso_partial_features =
+		NETIF_F_GSO_GRE |
+		NETIF_F_GSO_GRE_CSUM |
+		NETIF_F_GSO_IPXIP4 |
+		NETIF_F_GSO_UDP_TUNNEL |
+		NETIF_F_GSO_UDP_TUNNEL_CSUM;
+
 	netdev->features |=
+		netdev->gso_partial_features |
 		NETIF_F_GSO_IPXIP6 |
 		NETIF_F_SG |
 		NETIF_F_HW_CSUM |
@@ -330,6 +338,7 @@ struct net_device *mpnic_netdev_alloc(struct mpnic_dev *mpd)
 		NETIF_F_TSO |
 		NETIF_F_TSO_ECN |
 		NETIF_F_TSO6 |
+		NETIF_F_GSO_PARTIAL |
 		NETIF_F_GSO_UDP_L4;
 
 	netdev->hw_features |= netdev->features;
diff --git a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
index 839feb4716b6..af5e490c112d 100644
--- a/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
+++ b/drivers/net/ethernet/meta/mpnic/mpnic_txrx.c
@@ -236,6 +236,9 @@ static void mpnic_tx_fill_hdrs(struct sk_buff *skb, struct mpnic_tx_hdrs *hdrs)
 		hdrs->i3len = skb_transport_offset(skb) - net_off;
 		hdrs->l3_type = ip_hdr(skb)->version == 4 ?
 				MPNIC_TWD_L3_TYPE_IPV4 : MPNIC_TWD_L3_TYPE_IPV6;
+	} else if (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) {
+		hdrs->i3len = skb_checksum_start_offset(skb) - net_off;
+		hdrs->l3_type = MPNIC_TWD_L3_TYPE_OTHER;
 	} else {
 		inner_off = skb_inner_network_offset(skb);
 		hdrs->o3len = inner_off - net_off;
@@ -397,11 +400,27 @@ static bool mpnic_ipxip6_gso_ok(struct sk_buff *skb)
 static netdev_features_t
 mpnic_features_check_encap_gso(struct sk_buff *skb, netdev_features_t features)
 {
+	struct iphdr *iph;
+
 	if (skb_shinfo(skb)->gso_type & SKB_GSO_IPXIP6 &&
 	    mpnic_ipxip6_gso_ok(skb))
 		return features;
 
-	return features & ~NETIF_F_GSO_IPXIP6;
+	features &= ~NETIF_F_GSO_IPXIP6;
+
+	iph = ip_hdr(skb);
+	if (iph->version == 4 && !(iph->frag_off & htons(IP_DF)))
+		return features & ~NETIF_F_GSO_MASK;
+
+	/* HW can segment inner udp4 with fixed IDs, but there is no UDP
+	 * analog for MANGLEID, so defer to SW GSO.
+	 */
+	if (inner_ip_hdr(skb)->version == 4)
+		features &= ~NETIF_F_GSO_UDP_L4;
+	if (!(features & NETIF_F_TSO_MANGLEID))
+		features &= ~NETIF_F_TSO;
+
+	return features;
 }
 
 netdev_features_t mpnic_features_check(struct sk_buff *skb,

-- 
2.52.0