[PATCH v5 net-next] net: ftmac100: support mtu > 1500

[PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Sergei Antonov]

The ftmac100 controller considers packets >1518 (1500 + Ethernet + FCS)
FTL (frame too long) and drops them. That is fine with mtu 1500 or less
and it saves CPU time. When DSA is present, mtu is bigger (for VLAN
tagging) and the controller's built-in behavior is not desired then. We
can make the controller deliver FTL packets to the driver by setting
FTMAC100_MACCR_RX_FTL. Then we have to check ftmac100_rxdes_frame_length()
(packet length sans FCS) on packets marked with FTMAC100_RXDES0_FTL flag.

Check for mtu > 1500 in .ndo_open() and set FTMAC100_MACCR_RX_FTL to let
the driver FTL packets. Implement .ndo_change_mtu() and check for
mtu > 1500 to set/clear FTMAC100_MACCR_RX_FTL dynamically.

Fixes: 8d77c036b57c ("net: add Faraday FTMAC100 10/100 Ethernet driver")
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
v5:
* Handle ndo_change_mtu().
* Make description and code comments correct (hopefully).

v4:
* Set FTMAC100_MACCR_RX_FTL depending on the "mtu > 1500" condition.
* DSA tagging seems unrelated to the issue - updated description and a
code comment accordingly.

v3:
* Corrected the explanation of the problem: datasheet is correct.
* Rewrote the code to use the currently set mtu to handle DSA frames.

v2:
* Typos in description fixed.

 drivers/net/ethernet/faraday/ftmac100.c | 52 +++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index d95d78230828..f276d54bcd85 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -159,6 +159,7 @@ static void ftmac100_set_mac(struct ftmac100 *priv, const unsigned char *mac)
 static int ftmac100_start_hw(struct ftmac100 *priv)
 {
 	struct net_device *netdev = priv->netdev;
+	unsigned int maccr;
 
 	if (ftmac100_reset(priv))
 		return -EIO;
@@ -175,7 +176,20 @@ static int ftmac100_start_hw(struct ftmac100 *priv)
 
 	ftmac100_set_mac(priv, netdev->dev_addr);
 
-	iowrite32(MACCR_ENABLE_ALL, priv->base + FTMAC100_OFFSET_MACCR);
+	maccr = MACCR_ENABLE_ALL;
+
+	/* We have to set FTMAC100_MACCR_RX_FTL in case MTU > 1500
+	 * and do extra length check in ftmac100_rx_packet_error().
+	 * Otherwise the controller silently drops these packets.
+	 *
+	 * When the MTU of the interface is standard 1500, rely on
+	 * the controller's functionality to drop too long packets
+	 * and save some CPU time.
+	 */
+	if (netdev->mtu > 1500)
+		maccr |= FTMAC100_MACCR_RX_FTL;
+
+	iowrite32(maccr, priv->base + FTMAC100_OFFSET_MACCR);
 	return 0;
 }
 
@@ -337,9 +351,18 @@ static bool ftmac100_rx_packet_error(struct ftmac100 *priv,
 		error = true;
 	}
 
-	if (unlikely(ftmac100_rxdes_frame_too_long(rxdes))) {
+	/* If the frame-too-long flag FTMAC100_RXDES0_FTL is set, check
+	 * if ftmac100_rxdes_frame_length(rxdes) exceeds the currently
+	 * set MTU plus ETH_HLEN. FCS is not included here.
+	 * The controller would set FTMAC100_RXDES0_FTL for all incoming
+	 * frames longer than 1518 (includeing FCS) in the presence of
+	 * FTMAC100_MACCR_RX_FTL in the MAC Control Register.
+	 */
+	if (unlikely(ftmac100_rxdes_frame_too_long(rxdes) &&
+		     ftmac100_rxdes_frame_length(rxdes) > netdev->mtu + ETH_HLEN)) {
 		if (net_ratelimit())
-			netdev_info(netdev, "rx frame too long\n");
+			netdev_info(netdev, "rx frame too long (%u)\n",
+				    ftmac100_rxdes_frame_length(rxdes));
 
 		netdev->stats.rx_length_errors++;
 		error = true;
@@ -1037,6 +1060,28 @@ static int ftmac100_do_ioctl(struct net_device *netdev, struct ifreq *ifr, int c
 	return generic_mii_ioctl(&priv->mii, data, cmd, NULL);
 }
 
+static int ftmac100_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	struct ftmac100 *priv = netdev_priv(netdev);
+	unsigned int maccr;
+
+	maccr = ioread32(priv->base + FTMAC100_OFFSET_MACCR);
+	if (new_mtu <= 1500) {
+		/* Let the controller drop incoming packets greater
+		 * than 1518 (that is 1500 + 14 Ethernet + 4 FCS).
+		 */
+		maccr &= ~FTMAC100_MACCR_RX_FTL;
+	} else {
+		/* process FTL packets in the driver */
+		maccr |= FTMAC100_MACCR_RX_FTL;
+	}
+	iowrite32(maccr, priv->base + FTMAC100_OFFSET_MACCR);
+
+	netdev->mtu = new_mtu;
+	netdev_info(netdev, "changed mtu to %d\n", new_mtu);
+	return 0;
+}
+
 static const struct net_device_ops ftmac100_netdev_ops = {
 	.ndo_open		= ftmac100_open,
 	.ndo_stop		= ftmac100_stop,
@@ -1044,6 +1089,7 @@ static const struct net_device_ops ftmac100_netdev_ops = {
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_eth_ioctl		= ftmac100_do_ioctl,
+	.ndo_change_mtu		= ftmac100_change_mtu,
 };
 
 /******************************************************************************
-- 
2.34.1

Re: [PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Paolo Abeni]

Hello,

On Mon, 2022-10-24 at 20:58 +0300, Sergei Antonov wrote:
> The ftmac100 controller considers packets >1518 (1500 + Ethernet + FCS)
> FTL (frame too long) and drops them. That is fine with mtu 1500 or less
> and it saves CPU time. When DSA is present, mtu is bigger (for VLAN
> tagging) and the controller's built-in behavior is not desired then. We
> can make the controller deliver FTL packets to the driver by setting
> FTMAC100_MACCR_RX_FTL. Then we have to check ftmac100_rxdes_frame_length()
> (packet length sans FCS) on packets marked with FTMAC100_RXDES0_FTL flag.
> 
> Check for mtu > 1500 in .ndo_open() and set FTMAC100_MACCR_RX_FTL to let
> the driver FTL packets. Implement .ndo_change_mtu() and check for
> mtu > 1500 to set/clear FTMAC100_MACCR_RX_FTL dynamically.
> 
> Fixes: 8d77c036b57c ("net: add Faraday FTMAC100 10/100 Ethernet driver")

For the records, Vladimir explicitly asked you to drop the 'Fixes' tag.
Such tag makes little sense for a net-next commit, especially when
referring to an old change - e.g. that did not enter mainline in this
release cycle.

Cheers,

Paolo

Re: [PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Vladimir Oltean]

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

Hi Sergei,

On Mon, Oct 24, 2022 at 08:58:23PM +0300, Sergei Antonov wrote:
> The ftmac100 controller considers packets >1518 (1500 + Ethernet + FCS)
> FTL (frame too long) and drops them. That is fine with mtu 1500 or less
> and it saves CPU time. When DSA is present, mtu is bigger (for VLAN
> tagging) and the controller's built-in behavior is not desired then. We
> can make the controller deliver FTL packets to the driver by setting
> FTMAC100_MACCR_RX_FTL. Then we have to check ftmac100_rxdes_frame_length()
> (packet length sans FCS) on packets marked with FTMAC100_RXDES0_FTL flag.
> 
> Check for mtu > 1500 in .ndo_open() and set FTMAC100_MACCR_RX_FTL to let
> the driver FTL packets. Implement .ndo_change_mtu() and check for
> mtu > 1500 to set/clear FTMAC100_MACCR_RX_FTL dynamically.
> 
> Fixes: 8d77c036b57c ("net: add Faraday FTMAC100 10/100 Ethernet driver")
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> ---

I think it's clear there are problems in communication between us, so
let me try differently.

Does the attached series of 3 patches work for you? I only compile
tested them.

I tried to keep as much of your work and authorship as possible, the
intention was to rewrite the justification in the commit message and to
fix the things which your patches didn't do (as separate patches).

[-- Attachment #2: 0001-net-ftmac100-prepare-data-path-for-receiving-single-.patch --]
[-- Type: text/x-diff, Size: 2713 bytes --]

From 97be2778e5eec620d7b65c30cda3a70b9af0a88c Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Thu, 27 Oct 2022 13:54:21 +0300
Subject: [PATCH 1/3] net: ftmac100: prepare data path for receiving single
 segment packets > 1514

Eliminate one check in the data path and move it elsewhere, to where our
real limitation is. We'll want to start processing "too long" frames in
the driver (currently there is a hardware MAC setting which drops
theses).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/faraday/ftmac100.c | 29 ++++++++++---------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index d95d78230828..8013f85fc148 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -218,11 +218,6 @@ static bool ftmac100_rxdes_crc_error(struct ftmac100_rxdes *rxdes)
 	return rxdes->rxdes0 & cpu_to_le32(FTMAC100_RXDES0_CRC_ERR);
 }
 
-static bool ftmac100_rxdes_frame_too_long(struct ftmac100_rxdes *rxdes)
-{
-	return rxdes->rxdes0 & cpu_to_le32(FTMAC100_RXDES0_FTL);
-}
-
 static bool ftmac100_rxdes_runt(struct ftmac100_rxdes *rxdes)
 {
 	return rxdes->rxdes0 & cpu_to_le32(FTMAC100_RXDES0_RUNT);
@@ -337,13 +332,7 @@ static bool ftmac100_rx_packet_error(struct ftmac100 *priv,
 		error = true;
 	}
 
-	if (unlikely(ftmac100_rxdes_frame_too_long(rxdes))) {
-		if (net_ratelimit())
-			netdev_info(netdev, "rx frame too long\n");
-
-		netdev->stats.rx_length_errors++;
-		error = true;
-	} else if (unlikely(ftmac100_rxdes_runt(rxdes))) {
+	if (unlikely(ftmac100_rxdes_runt(rxdes))) {
 		if (net_ratelimit())
 			netdev_info(netdev, "rx runt\n");
 
@@ -356,6 +345,11 @@ static bool ftmac100_rx_packet_error(struct ftmac100 *priv,
 		netdev->stats.rx_length_errors++;
 		error = true;
 	}
+	/*
+	 * FTMAC100_RXDES0_FTL is not an error, it just indicates that the
+	 * frame is longer than 1518 octets. Receiving these is possible when
+	 * we told the hardware not to drop them, via FTMAC100_MACCR_RX_FTL.
+	 */
 
 	return error;
 }
@@ -400,12 +394,13 @@ static bool ftmac100_rx_packet(struct ftmac100 *priv, int *processed)
 		return true;
 	}
 
-	/*
-	 * It is impossible to get multi-segment packets
-	 * because we always provide big enough receive buffers.
-	 */
+	/* We don't support multi-segment packets for now, so drop them. */
 	ret = ftmac100_rxdes_last_segment(rxdes);
-	BUG_ON(!ret);
+	if (unlikely(!ret)) {
+		netdev->stats.rx_length_errors++;
+		ftmac100_rx_drop_packet(priv);
+		return true;
+	}
 
 	/* start processing */
 	skb = netdev_alloc_skb_ip_align(netdev, 128);
-- 
2.34.1


[-- Attachment #3: 0002-net-ftmac100-report-the-correct-maximum-MTU-of-1500.patch --]
[-- Type: text/x-diff, Size: 1983 bytes --]

From 3574f1730e262154e901da140baca76463d0bc3f Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Thu, 27 Oct 2022 13:59:03 +0300
Subject: [PATCH 2/3] net: ftmac100: report the correct maximum MTU of 1500

The driver uses the MAX_PKT_SIZE (1518) for both MTU reporting and for
TX. However, the 2 places do not measure the same thing.

On TX, skb->len measures the entire L2 packet length (without FCS, which
software does not possess). So the comparison against 1518 there is
correct.

What is not correct is the reporting of dev->max_mtu as 1518. Since MTU
measures L2 *payload* length (excluding L2 overhead) and not total L2
packet length, it means that the correct max_mtu supported by this
device is the standard 1500. Anything higher than that will be dropped
on RX currently.

To fix this, subtract VLAN_ETH_HLEN from MAX_PKT_SIZE when reporting the
max_mtu, since that is the difference between L2 payload length and
total L2 length as seen by software.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/faraday/ftmac100.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index 8013f85fc148..7c571b4515a9 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -11,6 +11,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -1070,7 +1071,7 @@ static int ftmac100_probe(struct platform_device *pdev)
 	SET_NETDEV_DEV(netdev, &pdev->dev);
 	netdev->ethtool_ops = &ftmac100_ethtool_ops;
 	netdev->netdev_ops = &ftmac100_netdev_ops;
-	netdev->max_mtu = MAX_PKT_SIZE;
+	netdev->max_mtu = MAX_PKT_SIZE - VLAN_ETH_HLEN;
 
 	err = platform_get_ethdev_address(&pdev->dev, netdev);
 	if (err == -EPROBE_DEFER)
-- 
2.34.1


[-- Attachment #4: 0003-net-ftmac100-allow-increasing-MTU-to-make-most-use-o.patch --]
[-- Type: text/x-diff, Size: 4283 bytes --]

From 893e534e7196ba1dee398e1f77e906d76e8c5a3f Mon Sep 17 00:00:00 2001
From: Sergei Antonov <saproj@gmail.com>
Date: Thu, 27 Oct 2022 14:06:59 +0300
Subject: [PATCH 3/3] net: ftmac100: allow increasing MTU to make most use of
 single-segment buffers

If the FTMAC100 is used as a DSA master, then it is expected that frames
which are MTU sized on the wire facing the external switch port (1500
octets in L2 payload, plus L2 header) also get a DSA tag when seen by
the host port.

This extra tag increases the length of the packet as the host port sees
it, and the FTMAC100 is not prepared to handle frames whose length
exceeds 1518 octets (including FCS) at all.

Only a minimal rework is needed to support this configuration. Since
MTU-sized DSA-tagged frames still fit within a single buffer (RX_BUF_SIZE),
we just need to optimize the resource management rather than implement
multi buffer RX.

In ndo_change_mtu(), we toggle the FTMAC100_MACCR_RX_FTL bit to tell the
hardware to drop (or not) frames with an L2 payload length larger than
1500. We need to replicate the MACCR configuration in ftmac100_start_hw()
as well, since there is a hardware reset there which clears previous
settings.

The advantage of dynamically changing FTMAC100_MACCR_RX_FTL is that when
dev->mtu is at the default value of 1500, large frames are automatically
dropped in hardware and we do not spend CPU cycles dropping them.

Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
---
 drivers/net/ethernet/faraday/ftmac100.c | 33 +++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index 7c571b4515a9..6c8c78018ce6 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -11,6 +11,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_ether.h>
 #include <linux/if_vlan.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -28,8 +29,8 @@
 #define RX_QUEUE_ENTRIES	128	/* must be power of 2 */
 #define TX_QUEUE_ENTRIES	16	/* must be power of 2 */
 
-#define MAX_PKT_SIZE		1518
 #define RX_BUF_SIZE		2044	/* must be smaller than 0x7ff */
+#define MAX_PKT_SIZE		RX_BUF_SIZE /* multi-segment not supported */
 
 #if MAX_PKT_SIZE > 0x7ff
 #error invalid MAX_PKT_SIZE
@@ -160,6 +161,7 @@ static void ftmac100_set_mac(struct ftmac100 *priv, const unsigned char *mac)
 static int ftmac100_start_hw(struct ftmac100 *priv)
 {
 	struct net_device *netdev = priv->netdev;
+	unsigned int maccr = MACCR_ENABLE_ALL;
 
 	if (ftmac100_reset(priv))
 		return -EIO;
@@ -176,7 +178,11 @@ static int ftmac100_start_hw(struct ftmac100 *priv)
 
 	ftmac100_set_mac(priv, netdev->dev_addr);
 
-	iowrite32(MACCR_ENABLE_ALL, priv->base + FTMAC100_OFFSET_MACCR);
+	 /* See ftmac100_change_mtu() */
+	if (netdev->mtu > ETH_DATA_LEN)
+		maccr |= FTMAC100_MACCR_RX_FTL;
+
+	iowrite32(maccr, priv->base + FTMAC100_OFFSET_MACCR);
 	return 0;
 }
 
@@ -1033,6 +1039,28 @@ static int ftmac100_do_ioctl(struct net_device *netdev, struct ifreq *ifr, int c
 	return generic_mii_ioctl(&priv->mii, data, cmd, NULL);
 }
 
+static int ftmac100_change_mtu(struct net_device *netdev, int mtu)
+{
+	struct ftmac100 *priv = netdev_priv(netdev);
+	unsigned int maccr;
+
+	maccr = ioread32(priv->base + FTMAC100_OFFSET_MACCR);
+	if (mtu > ETH_DATA_LEN) {
+		/* process long packets in the driver */
+		maccr |= FTMAC100_MACCR_RX_FTL;
+	} else {
+		/* Let the controller drop incoming packets greater
+		 * than 1518 (that is 1500 + 14 Ethernet + 4 FCS).
+		 */
+		maccr &= ~FTMAC100_MACCR_RX_FTL;
+	}
+	iowrite32(maccr, priv->base + FTMAC100_OFFSET_MACCR);
+
+	netdev->mtu = mtu;
+
+	return 0;
+}
+
 static const struct net_device_ops ftmac100_netdev_ops = {
 	.ndo_open		= ftmac100_open,
 	.ndo_stop		= ftmac100_stop,
@@ -1040,6 +1068,7 @@ static const struct net_device_ops ftmac100_netdev_ops = {
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_eth_ioctl		= ftmac100_do_ioctl,
+	.ndo_change_mtu		= ftmac100_change_mtu,
 };
 
 /******************************************************************************
-- 
2.34.1

Re: [PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Sergei Antonov]

On Thu, 27 Oct 2022 at 14:35, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Sergei,
>
> On Mon, Oct 24, 2022 at 08:58:23PM +0300, Sergei Antonov wrote:
> > The ftmac100 controller considers packets >1518 (1500 + Ethernet + FCS)
> > FTL (frame too long) and drops them. That is fine with mtu 1500 or less
> > and it saves CPU time. When DSA is present, mtu is bigger (for VLAN
> > tagging) and the controller's built-in behavior is not desired then. We
> > can make the controller deliver FTL packets to the driver by setting
> > FTMAC100_MACCR_RX_FTL. Then we have to check ftmac100_rxdes_frame_length()
> > (packet length sans FCS) on packets marked with FTMAC100_RXDES0_FTL flag.
> >
> > Check for mtu > 1500 in .ndo_open() and set FTMAC100_MACCR_RX_FTL to let
> > the driver FTL packets. Implement .ndo_change_mtu() and check for
> > mtu > 1500 to set/clear FTMAC100_MACCR_RX_FTL dynamically.
> >
> > Fixes: 8d77c036b57c ("net: add Faraday FTMAC100 10/100 Ethernet driver")
> > Signed-off-by: Sergei Antonov <saproj@gmail.com>
> > ---
>
> I think it's clear there are problems in communication between us, so
> let me try differently.
>
> Does the attached series of 3 patches work for you? I only compile
> tested them.

I have tested your patches. They fix the problem I have. If they can
make it into mainline Linux - great. Thanks for your help!

A remark on 0002-net-ftmac100-report-the-correct-maximum-MTU-of-1500.patch:
I can not make a case for VLAN_ETH_HLEN because it includes 4 bytes
from a switch and ftmac100 is not always used with a switch.

Re: [PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Vladimir Oltean]

On Thu, Oct 27, 2022 at 07:59:11PM +0300, Sergei Antonov wrote:
> On Thu, 27 Oct 2022 at 14:35, Vladimir Oltean <olteanv@gmail.com> wrote:
> > Does the attached series of 3 patches work for you? I only compile
> > tested them.
> 
> I have tested your patches. They fix the problem I have. If they can
> make it into mainline Linux - great. Thanks for your help!

Do you mind submitting these patches yourself, to get a better
understanding of the process? You only need to make sure that you
preserve the "From:" field (the authorship), and that below the existing
Signed-off-by line, you also add yours (to make it clear that the
patches authored by me were not submitted by me). Like this:

| From: Vladimir Oltean <vladimir.oltean@nxp.com>
| 
| bla bla
| 
| Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> <- same as author
| Signed-off-by: Sergei Antonov <saproj@gmail.com> <- patch carried by X
| ...etc
| when patch is merged, the netdev maintainer adds his own sign off at
| the end to indicate that the patch went through his own hands

I would do the same if I was the one submitting the series; I would add
my sign-off to patch 3/3, which has your authorship.

> A remark on 0002-net-ftmac100-report-the-correct-maximum-MTU-of-1500.patch:
> I can not make a case for VLAN_ETH_HLEN because it includes 4 bytes
> from a switch and ftmac100 is not always used with a switch.

Why do you think that? What VLAN are you talking about? 802.1Q or
802.1ad? What VLAN ID? Where does it come from, where do you see it?

VLAN_ETH_HLEN in patch 2 has nothing to do with a switch. I tried to
preserve the functionality of the driver as best I could. It accepts
skb->len on TX up to 1518, and it drops in hardware packets with a
skb->len larger than 1514 on RX (this includes L2 header, and is
measured before the eth_type_trans() call; the latter consumes ETH_HLEN
bytes and thus, skb->len becomes 1500).

A VLAN in the real sense (ip link add link eth0 name eth0.100 type vlan id 100)
does not increase the MTU of eth0 specifically because the 802.1Q header
is part of the L2 overhead, and not part of the L2 payload. So this is
why drivers could have a valid reason to subtract the VLAN header length
from the maximum packet size (which is total L2 length).

There's no way, really, to reconcile the fact that the driver can
transmit VLAN-tagged frames with an L2 payload length of 1500 bytes but
it cannot receive them (or at least I think that it can't, based on what
you said; maybe the hardware is smart and makes an exception for the
VLAN header on RX, letting packets with a size of up to 1522 bytes
length + CRC to be accepted?). In any case, 1500 is the MTU value that
the driver supports as of patch 2 (given the definition of MTU as L2
payload length)  and I wanted to make that absolutely clear by bringing
in sync what is reported with what is supported, prior to making any
other change to the max_mtu.

Re: [PATCH v5 net-next] net: ftmac100: support mtu > 1500

[Sergei Antonov]

On Thu, 27 Oct 2022 at 21:54, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Thu, Oct 27, 2022 at 07:59:11PM +0300, Sergei Antonov wrote:
> > On Thu, 27 Oct 2022 at 14:35, Vladimir Oltean <olteanv@gmail.com> wrote:
> > > Does the attached series of 3 patches work for you? I only compile
> > > tested them.
> >
> > I have tested your patches. They fix the problem I have. If they can
> > make it into mainline Linux - great. Thanks for your help!
>
> Do you mind submitting these patches yourself, to get a better
> understanding of the process? You only need to make sure that you
> preserve the "From:" field (the authorship), and that below the existing
> Signed-off-by line, you also add yours (to make it clear that the
> patches authored by me were not submitted by me). Like this:
>
> | From: Vladimir Oltean <vladimir.oltean@nxp.com>
> |
> | bla bla
> |
> | Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> <- same as author
> | Signed-off-by: Sergei Antonov <saproj@gmail.com> <- patch carried by X
> | ...etc
> | when patch is merged, the netdev maintainer adds his own sign off at
> | the end to indicate that the patch went through his own hands
>
> I would do the same if I was the one submitting the series; I would add
> my sign-off to patch 3/3, which has your authorship.

OK. I will do it.

> > A remark on 0002-net-ftmac100-report-the-correct-maximum-MTU-of-1500.patch:
> > I can not make a case for VLAN_ETH_HLEN because it includes 4 bytes
> > from a switch and ftmac100 is not always used with a switch.
>
> Why do you think that? What VLAN are you talking about? 802.1Q or
> 802.1ad? What VLAN ID? Where does it come from, where do you see it?

Patch 2 contains this change:
-       netdev->max_mtu = MAX_PKT_SIZE;
+       netdev->max_mtu = MAX_PKT_SIZE - VLAN_ETH_HLEN;

VLAN_ETH_HLEN is equal to 18 which is 6+6+4+2
It includes 4 bytes of 802.1Q.

> VLAN_ETH_HLEN in patch 2 has nothing to do with a switch.

OK then.