Re: [PATCH net v5 1/4] igb: do not let XDP frames span more than one Rx buffer

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] On i354 with VFs allocated, igb_set_rlpml()
  (drivers/net/ethernet/intel/igb/igb_main.c) programs no receive-length…

--

igb: do not let XDP frames span more than one Rx buffer

While XDP is attached, this patch limits the receive length to the
configured maximum frame size. It uses RLPML when no VFs are allocated
and the PF pool's VMOLR otherwise. It also tightens the XDP frame size
check, refuses XDP with rx-all, blocks legacy-rx changes while XDP is
attached, and moves the limit programming into a new igb_set_rlpml()
called from igb_configure().

Link: https://lore.kernel.org/all/20261028083344.3047350-3-p@1g4.org/

> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index ce91dda00ec0e..8abdd4e58550d 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -1964,6 +1964,54 @@ static void igb_setup_tx_mode(struct igb_adapter *adapter)

[ ... ]

> +static void igb_set_rlpml(struct igb_adapter *adapter)
> +{
> +	u32 rlpml = MAX_JUMBO_FRAME_SIZE, pool_rlpml = MAX_JUMBO_FRAME_SIZE;
> +	unsigned int vfn = adapter->vfs_allocated_count;
> +	struct e1000_hw *hw = &adapter->hw;
> +	u32 vmolr;
> +

[ ... ]

> +	/* XDP does not support frames that span more than one Rx buffer.
> +	 * With VFs the global limit stays large and the PF pool is limited
> +	 * through VMOLR.
> +	 */
> +	if (igb_xdp_is_enabled(adapter)) {
> +		pool_rlpml = min(pool_rlpml, adapter->max_frame_size);
> +		if (!vfn)
> +			rlpml = min(rlpml, adapter->max_frame_size);
> +	}
> +	wr32(E1000_RLPML, rlpml);
> +
> +	/* the PF pool's VMOLR is used on these MACs, see igb_set_rx_mode() */
> +	if (hw->mac.type < e1000_82576 || hw->mac.type > e1000_i350)
> +		return;

[Severity: High]
What happens on i354 when VFs are allocated and an XDP program is attached?

With vfn != 0, RLPML is written as MAX_JUMBO_FRAME_SIZE. In enum
e1000_mac_type, e1000_i354 comes right after e1000_i350:

	e1000_i350,
	e1000_i354,
	e1000_i210,

So on i354 this early return is taken and pool_rlpml is never written
to VMOLR(vfn). No receive length limit is programmed in that setup, and
the comment above ("the PF pool is limited through VMOLR") does not hold.

igb_probe_vfs() only rules out SR-IOV on i210, i211 and 82580, so VFs
can be allocated on i354:

	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211) ||
	    (hw->mac.type == e1000_82580))
		return;

igb_xdp_setup() and igb_xdp_frame_fits() do not refuse XDP in this
setup either.

Could frames up to 9728 bytes still reach the PF rings over several
descriptors in that case? igb_clean_rx_irq() would then run the XDP
program on a continuation buffer as if it were a new frame. That is the
failure the commit message says this patch fixes.

The skip does not look like a hardware limit. igb_set_vf_rlpml() writes
the RLPML field of VMOLR(n) with no MAC type check:

	vmolr = rd32(E1000_VMOLR(vfn));
	vmolr &= ~E1000_VMOLR_RLPML_MASK;
	vmolr |= size | E1000_VMOLR_LPE;

The > e1000_i350 check seems to come from the unrelated early return in
igb_set_rx_mode().

Should the PF pool VMOLR limit also be written on i354? Or should XDP be
refused on i354 while VFs are allocated? The commit message only says:

  As before, the PF pool's VMOLR is not written on i354.

It does not mention that this setup stays unprotected.

This also matters for a later patch in the series, "igb: do not split
AF_XDP zero-copy frames over several Rx buffers". That patch programs
2 KiB and 3 KiB SRRCTL buffers for XSK rings whose XSK frame can be as
small as 1792 bytes. It relies on this receive length limit, as its
commit message says: "Without it, the hardware can write past the XSK
frame".

On i354 with VFs, could the NIC then DMA past the end of the XSK frame?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104-igb-rx-ts-fix-v5-0-bf4b4367129a%40aweta.nl