From: Andrew Lunn <andrew@lunn.ch>
To: Colin Foster <colin.foster@in-advantage.com>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
linux-omap@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 net-next 1/1] net: ethernet: ti: cpsw: allow MTU > 1500 when overridden by module parameter
Date: Thu, 22 Jul 2021 16:05:33 +0200 [thread overview]
Message-ID: <YPl7LdLMMTmhSu1z@lunn.ch> (raw)
In-Reply-To: <20210721210538.22394-1-colin.foster@in-advantage.com>
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?
> Signed-off-by: Colin Foster <colin.foster@in-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.
Andrew
next prev parent reply other threads:[~2021-07-22 14:05 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-21 21:05 [RFC PATCH v1 net-next 1/1] net: ethernet: ti: cpsw: allow MTU > 1500 when overridden by module parameter Colin Foster
2021-07-22 14:05 ` Andrew Lunn [this message]
2021-07-22 15:33 ` Colin Foster
2021-07-22 16:50 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YPl7LdLMMTmhSu1z@lunn.ch \
--to=andrew@lunn.ch \
--cc=colin.foster@in-advantage.com \
--cc=davem@davemloft.net \
--cc=grygorii.strashko@ti.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox