Re: [PATCH net 6/6] iavf: cap advertised max_pkt_size at the single-buffer HW limit

[netdev-bot+sashiko@kernel.org]

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] The patch caps the Rx max_pkt_size sent to the PF (the per-
  queue Rx context 'rxmax') at LIBIE_MAX_RX_BUF_LEN (9728) in…

--

iavf: cap advertised max_pkt_size at the single-buffer HW limit

This commit caps the max_pkt_size that iavf_configure_queues() sends to
the PF in VIRTCHNL_OP_CONFIG_VSI_QUEUES at LIBIE_MAX_RX_BUF_LEN (9728).
Without the cap, a PF that advertises vf_res->max_mtu as 0 (the ESXi icen
PF on E810) receives 16382 and rejects the queue configuration.

Link: https://community.intel.com/t5/Ethernet-Products/E810-C-iavf-driver-issue-on-Linux-6-12/m-p/1737490
Link: https://access.redhat.com/solutions/6973766
Link: https://knowledge.broadcom.com/external/article/404315/sriov-enabled-vms-network-adaptor-goes-d.html

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> index ec234cc8bd9db..680a28a739bf0 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> @@ -382,6 +382,14 @@ void iavf_configure_queues(struct iavf_adapter *adapter)
>  
>  	max_frame = LIBIE_MAX_RX_FRM_LEN(adapter->rx_rings->pp->p.offset);
>  	max_frame = min_not_zero(adapter->vf_res->max_mtu, max_frame);
> +	/* The PF programs max_pkt_size into the per-queue Rx context "rxmax".
> +	 * LIBIE_MAX_RX_FRM_LEN is the multi-descriptor (S/G) frame ceiling
> +	 * (16382), but that exceeds the E810 max MAC frame size (9728); some
> +	 * PFs reject the out-of-range value with VIRTCHNL_STATUS_ERR_PARAM.
> +	 * Cap it at the single-buffer HW limit (== the MAC frame max),
> +	 * restoring the pre-Page-Pool behaviour.
> +	 */
> +	max_frame = min(max_frame, LIBIE_MAX_RX_BUF_LEN);

[Severity: Medium]

Should netdev->max_mtu be clamped to match? Rx max_pkt_size is now capped
at 9728. However, iavf_init_config_adapter() in iavf_main.c still derives
the MTU ceiling from the 16382 S/G limit when the PF sends max_mtu == 0:

drivers/net/ethernet/intel/iavf/iavf_main.c:iavf_init_config_adapter() {
    ...
	if (!adapter->vf_res->max_mtu) {
		netdev->max_mtu = LIBIE_MAX_MTU;
	} else if (adapter->vf_res->max_mtu < LIBETH_RX_LL_LEN + ETH_MIN_MTU ||
		   adapter->vf_res->max_mtu >
			   LIBETH_RX_LL_LEN + LIBIE_MAX_MTU) {
		...
		netdev->max_mtu = LIBIE_MAX_MTU;
	} else {
		netdev->max_mtu = adapter->vf_res->max_mtu - LIBETH_RX_LL_LEN;
	}
    ...
}

LIBIE_MAX_MTU is 16382 - 26 = 16356. A PF-advertised frame size above
9728 that still passes the range check also gives a max_mtu above 9702.

In the icen case this patch targets, can a user still set an MTU the
queues can't carry? For example:

    ip link set dev ethX mtu 12000

dev_validate_mtu() only checks against dev->max_mtu. iavf_change_mtu()
stores the new MTU and triggers a reset without a check of its own. The
queues are then reprogrammed with max_pkt_size = 9728.

Received frames larger than 9728 bytes would then be silently dropped by
the Rx queue context, instead of the MTU change failing. Per the commit
message, E810 also discards Tx frames above MAX_TXFRAME (9728).

Before this patch, rxmax and max_mtu both came from the 16382 ceiling and
matched each other.

The commit message and the new comment both describe this as "restoring
the pre-Page-Pool behaviour". Commit 5fa4caff59f2 changed two things. It
changed max_frame from IAVF_MAX_RXBUFFER to LIBIE_MAX_RX_FRM_LEN(). It
also changed netdev->max_mtu from IAVF_MAX_RXBUFFER - IAVF_PACKET_HDR_PAD
(9710) to LIBIE_MAX_MTU. This patch only reverts the first change.

Could netdev->max_mtu also be clamped to LIBIE_MAX_RX_BUF_LEN -
LIBETH_RX_LL_LEN, in both the zero-value branch and the invalid-value
fallback?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928230429.495442-1-anthony.l.nguyen%40intel.com