net: ibm: emac: Reserve VLAN header in MJS limit

[PATCH net-next] net: ibm: emac: Reserve VLAN header in MJS limit

Posted by Rosen Penev 1 week, 5 days ago

The IBM EMAC programs its Maximum Jumbo Size (MJS) drop
threshold from ndev->mtu directly. The hardware sizes the threshold
against the L2 frame minus the ethernet header, but does not
discount the 802.1Q tag, so a frame carrying a VLAN tag and a full
1500-byte payload exceeds MJS by exactly 4 bytes and is dropped.

This is normally hidden because JPSM (and therefore the MJS check)
only engages when the MTU is raised above ETH_DATA_LEN.  With the
qca8k DSA tagger the conduit MTU is bumped by QCA_HDR_LEN to 1502
during dsa_conduit_setup(), which is enough to enable JPSM and
expose the off-by-VLAN-tag in the limit.

Pad MJS by VLAN_HLEN so a VLAN-tagged full-MTU frame passes.

Reported on Meraki MX60 (qca8k switch): tagged VLAN
traffic drops at 1500-byte payload, while 1496 bytes works
and untagged 1500 bytes works.

Assisted-by: Claude:Opus-4.7
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/ibm/emac/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 11d673a9f119..90471cff37dd 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -30,6 +30,7 @@
 #include <linux/skbuff.h>
 #include <linux/crc32.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 #include <linux/mii.h>
 #include <linux/bitops.h>
 #include <linux/of.h>
@@ -457,7 +458,7 @@ static inline u32 emac_iff2rmr(struct net_device *ndev)
 
 	if (emac_has_feature(dev, EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE)) {
 		r &= ~EMAC4_RMR_MJS_MASK;
-		r |= EMAC4_RMR_MJS(ndev->mtu);
+		r |= EMAC4_RMR_MJS(ndev->mtu + VLAN_HLEN);
 	}
 
 	return r;
-- 
2.54.0

Re: [PATCH net-next] net: ibm: emac: Reserve VLAN header in MJS limit

Posted by Jacob Keller 1 week, 2 days ago

On 5/26/2026 1:22 PM, Rosen Penev wrote:
> The IBM EMAC programs its Maximum Jumbo Size (MJS) drop
> threshold from ndev->mtu directly. The hardware sizes the threshold
> against the L2 frame minus the ethernet header, but does not
> discount the 802.1Q tag, so a frame carrying a VLAN tag and a full
> 1500-byte payload exceeds MJS by exactly 4 bytes and is dropped.
> 
> This is normally hidden because JPSM (and therefore the MJS check)
> only engages when the MTU is raised above ETH_DATA_LEN.  With the
> qca8k DSA tagger the conduit MTU is bumped by QCA_HDR_LEN to 1502
> during dsa_conduit_setup(), which is enough to enable JPSM and
> expose the off-by-VLAN-tag in the limit.
> 
> Pad MJS by VLAN_HLEN so a VLAN-tagged full-MTU frame passes.
> 
> Reported on Meraki MX60 (qca8k switch): tagged VLAN
> traffic drops at 1500-byte payload, while 1496 bytes works
> and untagged 1500 bytes works.
> 
> Assisted-by: Claude:Opus-4.7
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

If this results in dropped packets, that seems like a user visible bug..
Should this have a Fixes tag and target the net-next tree? I guess its
not a "regression" since the driver has always behaved this way, but it
still seems like a bug to me.

> ---
>  drivers/net/ethernet/ibm/emac/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> index 11d673a9f119..90471cff37dd 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> @@ -30,6 +30,7 @@
>  #include <linux/skbuff.h>
>  #include <linux/crc32.h>
>  #include <linux/ethtool.h>
> +#include <linux/if_vlan.h>
>  #include <linux/mii.h>
>  #include <linux/bitops.h>
>  #include <linux/of.h>
> @@ -457,7 +458,7 @@ static inline u32 emac_iff2rmr(struct net_device *ndev)
>  
>  	if (emac_has_feature(dev, EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE)) {
>  		r &= ~EMAC4_RMR_MJS_MASK;
> -		r |= EMAC4_RMR_MJS(ndev->mtu);
> +		r |= EMAC4_RMR_MJS(ndev->mtu + VLAN_HLEN);
>  	}
>  
>  	return r;

Re: [PATCH net-next] net: ibm: emac: Reserve VLAN header in MJS limit

Posted by Paolo Abeni 1 week ago

On 5/30/26 1:18 AM, Jacob Keller wrote:
> On 5/26/2026 1:22 PM, Rosen Penev wrote:
>> The IBM EMAC programs its Maximum Jumbo Size (MJS) drop
>> threshold from ndev->mtu directly. The hardware sizes the threshold
>> against the L2 frame minus the ethernet header, but does not
>> discount the 802.1Q tag, so a frame carrying a VLAN tag and a full
>> 1500-byte payload exceeds MJS by exactly 4 bytes and is dropped.
>>
>> This is normally hidden because JPSM (and therefore the MJS check)
>> only engages when the MTU is raised above ETH_DATA_LEN.  With the
>> qca8k DSA tagger the conduit MTU is bumped by QCA_HDR_LEN to 1502
>> during dsa_conduit_setup(), which is enough to enable JPSM and
>> expose the off-by-VLAN-tag in the limit.
>>
>> Pad MJS by VLAN_HLEN so a VLAN-tagged full-MTU frame passes.
>>
>> Reported on Meraki MX60 (qca8k switch): tagged VLAN
>> traffic drops at 1500-byte payload, while 1496 bytes works
>> and untagged 1500 bytes works.
>>
>> Assisted-by: Claude:Opus-4.7
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> 
> If this results in dropped packets, that seems like a user visible bug..
> Should this have a Fixes tag and target the net-next tree? I guess its
> not a "regression" since the driver has always behaved this way, but it
> still seems like a bug to me.

Given it never worked and we are trying to keep the amount of changes
into net under control, I tend to consider this suitable for net-next
i.e. enable DSA support.

/P

Re: [PATCH net-next] net: ibm: emac: Reserve VLAN header in MJS limit

Posted by Jacob Keller 6 days, 20 hours ago

On 6/1/2026 6:35 AM, Paolo Abeni wrote:
> On 5/30/26 1:18 AM, Jacob Keller wrote:
>> On 5/26/2026 1:22 PM, Rosen Penev wrote:
>>> The IBM EMAC programs its Maximum Jumbo Size (MJS) drop
>>> threshold from ndev->mtu directly. The hardware sizes the threshold
>>> against the L2 frame minus the ethernet header, but does not
>>> discount the 802.1Q tag, so a frame carrying a VLAN tag and a full
>>> 1500-byte payload exceeds MJS by exactly 4 bytes and is dropped.
>>>
>>> This is normally hidden because JPSM (and therefore the MJS check)
>>> only engages when the MTU is raised above ETH_DATA_LEN.  With the
>>> qca8k DSA tagger the conduit MTU is bumped by QCA_HDR_LEN to 1502
>>> during dsa_conduit_setup(), which is enough to enable JPSM and
>>> expose the off-by-VLAN-tag in the limit.
>>>
>>> Pad MJS by VLAN_HLEN so a VLAN-tagged full-MTU frame passes.
>>>
>>> Reported on Meraki MX60 (qca8k switch): tagged VLAN
>>> traffic drops at 1500-byte payload, while 1496 bytes works
>>> and untagged 1500 bytes works.
>>>
>>> Assisted-by: Claude:Opus-4.7
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>
>> If this results in dropped packets, that seems like a user visible bug..
>> Should this have a Fixes tag and target the net-next tree? I guess its
>> not a "regression" since the driver has always behaved this way, but it
>> still seems like a bug to me.
> 
> Given it never worked and we are trying to keep the amount of changes
> into net under control, I tend to consider this suitable for net-next
> i.e. enable DSA support.
> 
> /P
> 

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net-next] net: ibm: emac: Reserve VLAN header in MJS limit

Posted by Rosen Penev 1 week, 2 days ago

On Fri, May 29, 2026 at 4:18 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
>
> On 5/26/2026 1:22 PM, Rosen Penev wrote:
> > The IBM EMAC programs its Maximum Jumbo Size (MJS) drop
> > threshold from ndev->mtu directly. The hardware sizes the threshold
> > against the L2 frame minus the ethernet header, but does not
> > discount the 802.1Q tag, so a frame carrying a VLAN tag and a full
> > 1500-byte payload exceeds MJS by exactly 4 bytes and is dropped.
> >
> > This is normally hidden because JPSM (and therefore the MJS check)
> > only engages when the MTU is raised above ETH_DATA_LEN.  With the
> > qca8k DSA tagger the conduit MTU is bumped by QCA_HDR_LEN to 1502
> > during dsa_conduit_setup(), which is enough to enable JPSM and
> > expose the off-by-VLAN-tag in the limit.
> >
> > Pad MJS by VLAN_HLEN so a VLAN-tagged full-MTU frame passes.
> >
> > Reported on Meraki MX60 (qca8k switch): tagged VLAN
> > traffic drops at 1500-byte payload, while 1496 bytes works
> > and untagged 1500 bytes works.
> >
> > Assisted-by: Claude:Opus-4.7
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> If this results in dropped packets, that seems like a user visible bug..
> Should this have a Fixes tag and target the net-next tree? I guess its
> not a "regression" since the driver has always behaved this way, but it
> still seems like a bug to me.
Good question. This bug is because of DSA. When this driver was
written, DSA didn't exist.

net instead of net-next makes sense but it's not my call to make.

Trivia: downstream OpenWrt used its own downstream switching mechanism
called swconfig. Before I transitioned this device to DSA, it worked
fine. After DSA it started to fail on tagged VLAN.
>
> > ---
> >  drivers/net/ethernet/ibm/emac/core.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> > index 11d673a9f119..90471cff37dd 100644
> > --- a/drivers/net/ethernet/ibm/emac/core.c
> > +++ b/drivers/net/ethernet/ibm/emac/core.c
> > @@ -30,6 +30,7 @@
> >  #include <linux/skbuff.h>
> >  #include <linux/crc32.h>
> >  #include <linux/ethtool.h>
> > +#include <linux/if_vlan.h>
> >  #include <linux/mii.h>
> >  #include <linux/bitops.h>
> >  #include <linux/of.h>
> > @@ -457,7 +458,7 @@ static inline u32 emac_iff2rmr(struct net_device *ndev)
> >
> >       if (emac_has_feature(dev, EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE)) {
> >               r &= ~EMAC4_RMR_MJS_MASK;
> > -             r |= EMAC4_RMR_MJS(ndev->mtu);
> > +             r |= EMAC4_RMR_MJS(ndev->mtu + VLAN_HLEN);
> >       }
> >
> >       return r;
>