Re: [PATCH v4 iwl-net] i40e: Prevent setting MTU if greater than MFS

[Simon Horman <horms@kernel.org>]

On Tue, Mar 19, 2024 at 12:38:03PM +0100, Erwan Velu wrote:
> 
> Le 18/03/2024 à 18:45, Simon Horman a écrit :
> > [...]
> > Hi Erwan, all,
> > 
> > As a fix, I think this patch warrants a fixes tag.
> > Perhaps this one is appropriate?
> > 
> > Fixes: 41c445ff0f48 ("i40e: main driver core")
> 
> Simon
> 
> Isn't that a bit too generic ?

Yes, maybe it is.
What we would be after is the first commit where the
user can hit the problem the patch addresses.

> [..]
> 
> > I am fine with this patch, so please take what follows as a suggestion
> > for improvement, possibly as a follow-up. Not as a hard requirement from
> > my side.
> > 
> > The part of this function between the two hunks of this patch is:
> > 
> >                  netdev_err(netdev, "Error changing mtu to %d, Max is %d\n",
> >                             new_mtu, frame_size - I40E_PACKET_HDR_PAD);
> > 
> > My reading is that with this patch two different limits are
> > checked wrt maximum MTU size:
> > 
> > 1. A VSI level limit, which relates to RX buffer size
> > 2. A PHY level limit that relates to the MFS
> > 
> > That seems fine to me. But the log message for 1 (above) does
> > not seem particularly informative wrt which limit has been exceeded.
> 
> I got some comments around this.
> 
> I wanted to keep my patch being focused on the mfs issue, but I can offer a
> patch to get a similar output for this. What WRT stands for ?
> 
> 
> I wanted also to make another patch for this :
> 
> dev_warn(&pdev->dev, "MFS for port %x has been set below the default:
> %x\n",pf->hw.port, val);
> 
> The MFS reported as hex without a "0x" prefix is very misleading, I can
> offer a patch for this too.

FWIIW, I think handling these questions in follow-up patches is fine.