Re: [PATCH RFC net-next v4 07/14] xsk: try to skip validating skb list in xmit path

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Critical] Bypassing `validate_xmit_skb_list()` based on `NETIF_F_SG` skips critical hardware capability checks, software fallbacks, and SKB initialization, leading to kernel panics and corrupted packets.
- [High] Breaking out of the transmit loop early leaves packets stuck in the TX ring indefinitely because the hardware doorbell is never flushed.
--

commit 7603ac608a36b8d39e026a60b567ea2b106617f0
Author: Jason Xing <kernelxing@tencent.com>

xsk: try to skip validating skb list in xmit path

This patch attempts to optimize the XSK transmit path by skipping
validate_xmit_skb_list() if the device supports Scatter-Gather,
aiming to improve performance for drivers like ixgbe.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 99188353abec6..12643cbab6530 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
[ ... ]
> @@ -4878,15 +4879,17 @@ int xsk_direct_xmit_batch(struct xdp_sock *xs, struct net_device *dev)
>  	local_bh_disable();
>  	HARD_TX_LOCK(dev, txq, smp_processor_id());
>  	while ((skb = __skb_dequeue(send_queue)) != NULL) {
> -		struct sk_buff *orig_skb = skb;
> -		bool again = false;
> -
> -		skb = validate_xmit_skb_list(skb, dev, &again);
> -		if (skb != orig_skb) {
> -			dev_core_stats_tx_dropped_inc(dev);
> -			kfree_skb_list(skb);
> -			ret = NET_XMIT_DROP;
> -			break;
> +		if (unlikely(need_validate)) {
> +			struct sk_buff *orig_skb = skb;
> +			bool again = false;
> +
> +			skb = validate_xmit_skb_list(skb, dev, &again);

Does skipping validate_xmit_skb_list() based on NETIF_F_SG bypass necessary
hardware capability checks and software fallbacks?

validate_xmit_skb() performs dynamic feature verification via
ndo_features_check(), transport header initialization via
skb_set_transport_header(), and software checksum fallbacks via
skb_csum_hwoffload_help().

AF_XDP users can request TX checksum offload, setting skb->ip_summed to
CHECKSUM_PARTIAL. By skipping validation solely because the device supports
Scatter-Gather, packets bypass skb_set_transport_header(), leaving
skb->transport_header uninitialized.

Could this cause drivers that use skb_transport_offset() to compute massive
sizes, corrupting TX descriptors and potentially hanging the NIC?

Additionally, if the device lacks NETIF_F_HW_CSUM, skipping
skb_csum_hwoffload_help() sends CHECKSUM_PARTIAL to drivers unable to handle
it. Might this result in silent data corruption on the wire or driver crashes?

> +			if (skb != orig_skb) {
> +				dev_core_stats_tx_dropped_inc(dev);
> +				kfree_skb_list(skb);
> +				ret = NET_XMIT_DROP;
> +				break;

If this loop breaks early, does it leave packets stuck in the TX ring?

While this behavior was introduced earlier in the series and not by this
specific patch, the issue is present in this modified block.

The batching logic calls netdev_start_xmit() with more = true for all packets
except the last one in the queue.

If the loop breaks early here due to a validate_xmit_skb_list() failure,
the function returns without ever calling netdev_start_xmit() with
more = false.

Will this cause the driver to defer ringing the hardware TX doorbell,
leaving previously submitted packets stranded in the TX ring and causing
latency spikes or TX watchdog timeouts?

> +			}
>  		}
>  
>  		if (netif_xmit_frozen_or_drv_stopped(txq)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260415082654.21026-1-kerneljasonxing@gmail.com?part=7