Re: [PATCH RFC net-next 4/4] veth: use generic skb XDP handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing statistic accounting for skb-backed XDP_TX failures.
--

commit 3f79161429d884ed3854644cf3a7b111ce3c40fa
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

veth: use generic skb XDP handling

This commit transitions veth to use the generic skb XDP helper for skb-backed
packets. It routes XDP_REDIRECT and XDP_TX through the generic paths while
retaining the existing veth xdp_frame path for packets arriving as frames.

> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c

[ ... ]

> @@ -808,94 +764,33 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
>  		goto out;
>  	}
>  
> +	ret = __do_xdp_generic(xdp_prog, &skb, &ctx);
> +	switch (ctx.act) {
>  	case XDP_PASS:
>  		break;
>  	case XDP_TX:
> +		if (!ctx.err)
> +			stats->xdp_skb_tx++;

Should there be an else branch here to increment stats->xdp_tx_err?

It looks like if the generic XDP_TX fails and ctx.err is non-zero, the
failure is not recorded in the local stats structure.

[ ... ]

> @@ -947,7 +842,9 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget,
>  
>  	u64_stats_update_begin(&rq->stats.syncp);
>  	rq->stats.vs.xdp_redirect += stats->xdp_redirect;
> +	rq->stats.vs.xdp_tx += stats->xdp_skb_tx;
>  	rq->stats.vs.xdp_bytes += stats->xdp_bytes;
> +	rq->stats.vs.xdp_tx_err += stats->xdp_tx_err;

This aggregation was added to collect the errors, but since stats->xdp_tx_err
is never incremented in veth_xdp_rcv_skb(), are the skb-backed XDP_TX errors
entirely missing from veth's private ethtool statistics?

If the local increment was intentionally omitted to prevent double-counting in
the global dev_core_stats->tx_dropped (which generic_xdp_tx() increments),
does adding this aggregation line leave the ethtool stats inconsistent by
only tracking xdp_frame drops?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509084858.773921-1-maciej.fijalkowski@intel.com?part=4