Received-SPF: pass (google.com: domain of linux-kernel+bounces-84786-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 28 Feb 2024 10:43:37 +0100
From: Jiri Pirko <jiri@resnulli.us>
To: Erwan Velu <erwanaliasr1@gmail.com>
Cc: Erwan Velu <e.velu@criteo.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] i40e: Prevent setting MTU if greater than MFS
Message-ID: <Zd8ASR9ocx-Xk4OT@nanopsycho>
References: <20240227192704.376176-1-e.velu@criteo.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240227192704.376176-1-e.velu@criteo.com>

Tue, Feb 27, 2024 at 08:27:03PM CET, erwanaliasr1@gmail.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@criteo.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.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
>
>