netdev - Re: [PATCH] i40e: Prevent setting MTU if greater than MFS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zd8ASR9ocx-Xk4OT@nanopsycho>
Date: Wed, 28 Feb 2024 10:43:37 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Erwan Velu <erwanaliasr1@...il.com>
Cc: Erwan Velu <e.velu@...teo.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Tony Nguyen <anthony.l.nguyen@...el.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i40e: Prevent setting MTU if greater than MFS

Tue, Feb 27, 2024 at 08:27:03PM CET, erwanaliasr1@...il.com wrote:
>Commit 6871a7de705b6f6a4046f0d19da9bcd689c3bc8e from iPXE project is
>setting the MFS to 0x600 = 1536.
>
>At boot time the i40e driver complains about it with
>the following message but continues.
>
>	MFS for port 1 has been set below the default: 600
>
>If the MTU size is increased, the driver accept it but large packets will not
>be processed by the firmware generating tx_errors. The issue is pretty
>silent for users. i.e doing TCP in such context will generates lots of
>retransmissions until the proper window size (below 1500) will be used.
>
>To fix this case, it would have been ideal to increase the MFS,
>via i40e_aqc_opc_set_mac_config, but I didn't found a reliable way to do it.
>
>At least, this commit prevents setting up an MTU greater than the current MFS.
>It will avoid being in the position of having an MTU set to 9000 on the
>netdev with a firmware refusing packets larger than 1536.
>
>A typical trace looks like the following :
>[  377.548696] i40e 0000:5d:00.0 eno5: Error changing mtu to 9000 which is greater than the current mfs: 1536
>
>Signed-off-by: Erwan Velu <e.velu@...teo.com>

Reviewed-by: Jiri Pirko <jiri@...dia.com>

Next time, could you please indicate the target tree in the patch
subject prefix, like this: "[patch net-next] xxx" ?


>---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>index 54eb55464e31..14fc70d854d3 100644
>--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>@@ -2950,7 +2950,7 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
> 	struct i40e_netdev_priv *np = netdev_priv(netdev);
> 	struct i40e_vsi *vsi = np->vsi;
> 	struct i40e_pf *pf = vsi->back;
>-	int frame_size;
>+	int frame_size, mfs;
> 
> 	frame_size = i40e_max_vsi_frame_size(vsi, vsi->xdp_prog);
> 	if (new_mtu > frame_size - I40E_PACKET_HDR_PAD) {
>@@ -2959,6 +2959,13 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
> 		return -EINVAL;
> 	}
> 
>+	mfs = pf->hw.phy.link_info.max_frame_size;
>+	if (new_mtu > mfs) {
>+		netdev_err(netdev, "Error changing mtu to %d which is greater than the current mfs: %d\n",
>+			   new_mtu, mfs);
>+		return -EINVAL;
>+	}
>+
> 	netdev_dbg(netdev, "changing MTU from %d to %d\n",
> 		   netdev->mtu, new_mtu);
> 	netdev->mtu = new_mtu;
>-- 
>2.43.2
>
>