netdev - Re: [Intel-wired-lan] [PATCH v3] 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: <CAL2JzuzjRTHzKPVU9+zfFeEgfDQPeRDUuaUjsqgDfgQ865en8Q@mail.gmail.com>
Date: Tue, 12 Mar 2024 11:18:17 +0100
From: Erwan Velu <erwanaliasr1@...il.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: Erwan Velu <e.velu@...teo.com>, linux-kernel@...r.kernel.org, 
	Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org, 
	Tony Nguyen <anthony.l.nguyen@...el.com>, intel-wired-lan@...ts.osuosl.org, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [Intel-wired-lan] [PATCH v3] i40e: Prevent setting MTU if greater
 than MFS

> 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.