Re: [PATCH bpf v4 6/9] i40e: use xdp.frame_sz as XDP RxQ info frag_size

[bot+bpf-ci@kernel.org]

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

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3567,6 +3567,7 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
>  	struct i40e_vsi *vsi = ring->vsi;
>  	u32 chain_len = vsi->back->hw.func_caps.rx_buf_chain_len;
>  	u16 pf_q = vsi->base_queue + ring->queue_index;
> +	u32 xdp_frame_sz = i40e_rx_pg_size(ring) / 2;
>  	struct i40e_hw *hw = &vsi->back->hw;

On PAGE_SIZE < 8192 systems, i40e_rx_pg_size() calls
i40e_rx_pg_order() which checks ring->rx_buf_len:

i40e_txrx.h:i40e_rx_pg_order() {
    #if (PAGE_SIZE < 8192)
        if (ring->rx_buf_len > (PAGE_SIZE / 2))
            return 1;
    #endif
        return 0;
}

At this point in the function, ring->rx_buf_len still holds the
value from the previous configuration. It is not updated until
later in the function body:

i40e_main.c:i40e_configure_rx_ring() {
    ...
    u32 xdp_frame_sz = i40e_rx_pg_size(ring) / 2;  // stale rx_buf_len
    ...
    ring->rx_buf_len = vsi->rx_buf_len;  // updated here, after init
    ...
}

Could this lead to a wrong xdp_frame_sz on 4K page systems when
ring->rx_buf_len changes across the PAGE_SIZE/2 (2048) boundary
between configurations?

For example, after AF_XDP ZC teardown on a standard MTU interface,
the previous rx_buf_len from XSK (~3840 with default 4K chunks) is
above 2048, giving pg_order=1 and xdp_frame_sz=4096. But the new
vsi->rx_buf_len for standard MTU is 1534, which would give
pg_order=0 and the correct xdp_frame_sz=2048.

With frag_size set to 4096 but actual buffers only 2048 bytes,
bpf_xdp_frags_increase_tail() would see frag_size > frame_sz and
return -EOPNOTSUPP.

The previous code computed i40e_rx_pg_size(ring) / 2 at the skip
label, after ring->rx_buf_len was already updated, so this ordering
issue did not exist before.

Would it work to move the computation after the rx_buf_len
assignment, for example:

    ring->rx_buf_len = vsi->rx_buf_len;

    xdp_frame_sz = i40e_rx_pg_size(ring) / 2;

    if (ring->vsi->type != I40E_VSI_MAIN)
        goto skip;

> [ ... ]

> @@ -3616,7 +3618,7 @@
>  	}
>
>  skip:
> -	xdp_init_buff(&ring->xdp, i40e_rx_pg_size(ring) / 2, &ring->xdp_rxq);
> +	xdp_init_buff(&ring->xdp, xdp_frame_sz, &ring->xdp_rxq);

> [ ... ]



---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22586694870

AI-authorship-score: low
AI-authorship-explanation: The commit shows domain-specific driver knowledge, targeted fix with minimal changes, and a natural commit message style consistent with human kernel developers.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: Variable initialization order causes wrong xdp_frame_sz on 4K page systems when rx_buf_len crosses the PAGE_SIZE/2 boundary between reconfigurations, leading to bpf_xdp_adjust_tail returning -EOPNOTSUPP incorrectly but no memory corruption.