Re: [Intel-wired-lan] [PATCH v3] i40e: Prevent setting MTU if greater than MFS

From: Erwan Velu
Date: Tue Mar 12 2024 - 06:18:37 EST


> Am 12.03.24 um 10:42 schrieb Erwan Velu:
> > Commit 6871a7de705b6f6a4046f0d19da9bcd689c3bc8e from iPXE project is
> > setting the MFS to 0x600 = 1536.
>
> Please add a link, as most people do not have the iPXE archive checked
> out. Maybe also add the commit message summary.
I will, thanks.

> > 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
> Hmm, but 1536 > 600. So the log message is incorrect?

As mentioned earlier in the commit message, the 600 is 0x600 = 1536.
I can offer a patch to report it in decimal or add an explicit 0x prefix.

> > If the MTU size is increased, the driver accept it but large packets will not
> accept*s*
Fixed.


[...]
> > 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.
> Maybe add the new log message.
Done.

> One last formal nit: Please use a line length limit of 75 characters per
> line.
Done.

> > + mfs = pf->hw.phy.link_info.max_frame_size;
> > + max_mtu = mfs - I40E_PACKET_HDR_PAD;
> > + if (new_mtu > max_mtu) {
> > + netdev_err(netdev, "Error changing mtu to %d, Max is %d. MFS is too small.\n",
> > + new_mtu, max_mtu);
>
> The other log messages capitalize MTU.
Yeah but the exact previous one was in the same case. Shall I bump all
of them to upper or lower cast ?


> The rest looks reasonable.
Thx for the review.