netdev - Re: [RFC PATCH v1 net-next 1/1] net: ethernet: ti: cpsw: allow MTU

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 22 Jul 2021 08:33:51 -0700
From:   Colin Foster <colin.foster@...advantage.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Grygorii Strashko <grygorii.strashko@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-omap@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v1 net-next 1/1] net: ethernet: ti: cpsw: allow MTU >
 1500 when overridden by module parameter

On Thu, Jul 22, 2021 at 04:05:33PM +0200, Andrew Lunn wrote:
> On Wed, Jul 21, 2021 at 02:05:38PM -0700, Colin Foster wrote:
> > The module parameter rx_packet_max can be overridden at module load or
> > boot args. But it doesn't adjust the max_mtu for the device accordingly.
> > 
> > If a CPSW device is to be used in a DSA architecture, increasing the
> > MTU by small amounts to account for switch overhead becomes necessary.
> > This way, a boot arg of cpsw.rx_packet_max=1600 should allow the MTU
> > to be increased to values of 1520, which is necessary for DSA tagging
> > protocols like "ocelot" and "seville".
> 
> Hi Colin
> 
> As far as your patch goes, it makes sense.
> 
> However, module parameters are unlikely by netdev maintainers. Having
> to set one in order to make DSA work is not nice. What is involved in
> actually removing the module parameter and making the MTU change work
> without it?

Thanks for the feedback Andrew.

That's a good idea. I used the module parameter because it was already 
there.

My intent was to not change any existing default behavior. The below 
forum post makes me think that simply changing the default value of 
rx_packet_max from 1500 to 1998 alongside this patch is all that is 
needed. It all seems too easy, so either my use-case is rare enough 
that nobody considered it, or there's some limitation I'm missing.

https://e2e.ti.com/support/processors-group/processors/f/processors-forum/461724/how-to-send-receive-jumbo-packet-by-am335x-emac

> 
> > Signed-off-by: Colin Foster <colin.foster@...advantage.com>
> > ---
> >  drivers/net/ethernet/ti/cpsw.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> > index c0cd7de88316..d400163c4ef2 100644
> > --- a/drivers/net/ethernet/ti/cpsw.c
> > +++ b/drivers/net/ethernet/ti/cpsw.c
> > @@ -1625,6 +1625,14 @@ static int cpsw_probe(struct platform_device *pdev)
> >  		goto clean_cpts;
> >  	}
> >  
> > +	/* adjust max_mtu to match module parameter rx_packet_max */
> > +	if (cpsw->rx_packet_max > CPSW_MAX_PACKET_SIZE) {
> > +		ndev->max_mtu = ETH_DATA_LEN + (cpsw->rx_packet_max -
> > +				CPSW_MAX_PACKET_SIZE);
> > +		dev_info(dev, "overriding default MTU to %d\n\n",
> > +			 ndev->max_mtu);
> 
> There is no need for dev_info(). You could consider dev_dbg(), or just
> remove it.

Understood.

> 
>        Andrew