netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Paolo Abeni <pabeni@redhat.com>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/3] dsa: marvell: Provide per device information about max frame size
Date: Fri, 13 Jan 2023 14:20:17 +0100	[thread overview]
Message-ID: <20230113142017.78184ce1@wsk> (raw)
In-Reply-To: <20230113122754.52qvl3pvwpdy5iqk@skbuf>

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

Hi Vladimir,

> On Fri, Jan 13, 2023 at 01:13:31PM +0100, Lukasz Majewski wrote:
> > I think that this commit [1], made the adjustment to fix yet another
> > issue.
> > [1] -
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b9c587fed61cf88bd45822c3159644445f6d5aa6
> >  
> 
> It appears that this is the commit to blame, indeed.
> 
> > It looks like the missing 8 bytes are added in the
> > mv88e6xxx_change_mtu() function.  
> 
> Only for DSA and CPU ports. The driver still behaves as if the max MTU
> on user ports is 1492 bytes.
> 

It looks so...

> > > I wonder, shouldn't we first fix that, and apply this patch set
> > > afterwards?  
> > 
> > IMHO, it is up to Andrew to decide how to proceed, as the
> > aforementioned patch [1] is an attempt to fix yet another issue [2].
> > [2] -
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1baf0fac10fbe3084975d7cb0a4378eb18871482
> >  
> 
> I think the handling for those switches were neither
> port_set_jumbo_size() nor set_max_frame_size() is present is just a
> roundabout way of saying "hey, I only support ETH_DATA_LEN MTU and
> can't change it, leave me alone". But it isn't what the code does.

I tend to agree... The number of switched which suppor 1522 B max frame
is only six. This may be why the problem was not noticed.

The fixed function maybe should look like below:


static int mv88e6xxx_get_max_mtu(struct dsa_switch *ds, int port)
{
	....

	
	int max_mtu;

	max_mtu = chip->info->max_frame_size - VLAN_ETH_HLEN -
		  ETH_FCS_LE;

	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
		  max_mtu -= EDSA_HLEN;

	return max_mtu;
}

Comments more than welcome.



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2023-01-13 13:29 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06 10:16 [PATCH v4 1/3] dsa: marvell: Provide per device information about max frame size Lukasz Majewski
2023-01-06 10:16 ` [PATCH v4 2/3] net: dsa: mv88e6xxx: add support for MV88E6020 switch Lukasz Majewski
2023-01-06 13:06   ` Andrew Lunn
2023-01-06 10:16 ` [PATCH v4 3/3] net: dsa: mv88e6xxx: add support for MV88E6071 switch Lukasz Majewski
2023-01-06 13:06   ` Andrew Lunn
2023-01-06 13:08 ` [PATCH v4 1/3] dsa: marvell: Provide per device information about max frame size Andrew Lunn
2023-01-09  9:00   ` Lukasz Majewski
2023-01-13 10:39   ` Lukasz Majewski
2023-01-13 10:49     ` Vladimir Oltean
2023-01-13 11:02       ` Lukasz Majewski
2023-01-13 11:14         ` Vladimir Oltean
2023-01-13 11:53           ` Lukasz Majewski
2023-01-06 14:51 ` Vladimir Oltean
2023-01-13 12:13   ` Lukasz Majewski
2023-01-13 12:27     ` Vladimir Oltean
2023-01-13 13:20       ` Lukasz Majewski [this message]
2023-01-13 13:53         ` Andrew Lunn
2023-01-13 13:59         ` Vladimir Oltean
2023-01-13 14:16 ` Russell King (Oracle)
2023-01-16  9:51   ` Lukasz Majewski
2023-01-25 11:24     ` Lukasz Majewski
2023-01-25 15:12       ` Russell King (Oracle)
2023-01-30 11:57         ` Lukasz Majewski
2023-01-30 12:30           ` Russell King (Oracle)

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=20230113142017.78184ce1@wsk \
    --to=lukma@denx.de \
    --cc=alexander.duyck@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).