public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Robert Hodaszi <robert.hodaszi@digi.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev@vger.kernel.org, claudiu.manoil@nxp.com,
	alexandre.belloni@bootlin.com, UNGLinuxDriver@microchip.com,
	andrew@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC net 0/2] net: dsa: felix: fix VLAN-unaware reception
Date: Mon, 16 Dec 2024 11:10:05 +0100	[thread overview]
Message-ID: <908ec18c-3d04-4cc9-a152-e41b17c5b315@digi.com> (raw)
In-Reply-To: <20241215170921.5qlundy4jzutvze7@skbuf>

On Sunday, 15.12.2024 at 18:09 +0100, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> Give me an example traffic pattern, Linux configuration and corruption,
> please. I spent a lot of time trying to make sure I am not introducing
> regressions, and I have no idea what you are seeing that is wrong.
> Please don't try to make assumptions, just let me see what you see.

The config I'm using:
 - Using the 2.5Gbps as CPU port in 'ocelot-8021q' mode, Linux interface name is 'eth0'
 - Using 2 downstream ports as external Ethernet ports: 'eth1' and 'eth2'
 - 'eth1' port of the device is directly connected with my PC (Ethernet interface #1, 192.168.1.1)
 - 'eth2' port of the device is directly connected with my PC (Ethernet interface #2, 192.168.2.1)

DTS:

  &mscc_felix_port0 {
    label = "eth1";
    managed = "in-band-status";
    phy-handle = <&qsgmii_phy0>;
    phy-mode = "qsgmii";
    status = "okay";
  };

  &mscc_felix_port1 {
    label = "eth2";
    managed = "in-band-status";
    phy-handle = <&qsgmii_phy1>;
    phy-mode = "qsgmii";
    status = "okay";
  };

  &mscc_felix_port4 {
    ethernet = <&enetc_port2>;
    status = "okay";
    dsa-tag-protocol = "ocelot-8021q";
  };

LS1028 unit's Linux config:

  # Static IP to 'eth1'
  $ ifconfig eth1 192.168.1.2 up

  # Create a VLAN-unaware bridge, and add 'eth2' to that
  $ brctl addbr br0
  $ brctl addif br0 eth2

  # Set static IP to the bridge
  $ ifconfig br0 192.168.2.2 up
  $ ifconfig eth2 up

Now at this point:

  1. I can ping perfectly fine the eth1 interface from my PC ("ping 192.168.1.2"), and vice-versa
  2. Pinging 'br0' from my PC is not working ("ping 192.168.2.2"). I can see the ARP requests, but there are not ARP replies at all.

If I enable VLAN-filtering on 'br0', it starts working:

  $ echo 1 > /sys/class/net/br0/bridge/vlan_filtering


So basically:

  1. Raw interface -> working
  2. VLAN-aware bridge -> working
  3. VLAN-unaware bridge -> NOT working

I traced what is happening. When VLAN-filtering is not enabled on the bridge, LS1028's switch is configured with 'push_inner_tag = OCELOT_NO_ES0_TAG'. But ds->untag_vlan_aware_bridge_pvid is always set to true at switch setup, in felix_tag_8021q_setup(). That makes dsa_switch_rcv() call dsa_software_vlan_untag() for each packets.


Now in dsa_software_vlan_untag(), if the port is not part of the bridge (case #1), it returns with the skb early. That's OK.


  static inline struct sk_buff *dsa_software_vlan_untag(struct sk_buff *skb)
  {
    struct dsa_port *dp = dsa_user_to_port(skb->dev);
    struct net_device *br = dsa_port_bridge_dev_get(dp);
    u16 vid;

    /* software untagging for standalone ports not yet necessary */
    if (!br)
      return skb;


But if port is part of a bridge, no matter "push_inner_tag" is set as OCELOT_ES0_TAG or OCELOT_NO_ES0_TAG, it always untags it:

    /* Move VLAN tag from data to hwaccel */
    if (!skb_vlan_tag_present(skb)) {
      skb = skb_vlan_untag(skb);
      if (!skb)
        return NULL;
    }

As the "untag_vlan_aware_bridge_pvid" is a switch-specific thing, not port-specific, I cannot change it to false/true depending on the port is added to a VLAN-unaware/aware bridge, as the other port may be added to another bridge (eth1 -> VLAN-aware (tags enabled), eth2 -> VLAN-unaware (tags disabled)).

Also, in the past this code part looked like this:

    /* Move VLAN tag from data to hwaccel */
    if (!skb_vlan_tag_present(skb) && skb->protocol == htons(proto)) {
      skb = skb_vlan_untag(skb);
      if (!skb)
        return NULL;
    }

So we had a protocol check. This wouldn't work 100% neither, because what if a VLAN packet arrives from the outer world into a VLAN-unaware bridge? I assume, that shouldn't be untagged, still, it would do that.


I'm not that happy with my patch though, as I had to add another flag for each ports. But that seems to be the "cleanest" solution. That's why as marked it as RFC.

Thanks,
Robert

  reply	other threads:[~2024-12-16 10:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-15 16:33 [PATCH RFC net 0/2] net: dsa: felix: fix VLAN-unaware reception Robert Hodaszi
2024-12-15 16:33 ` [PATCH RFC net 1/2] net: dsa: provide a way to exclude switch ports from VLAN untagging Robert Hodaszi
2024-12-15 16:33 ` [PATCH RFC net 2/2] net: dsa: felix: fix reception from VLAN-unaware ports Robert Hodaszi
2024-12-15 17:09 ` [PATCH RFC net 0/2] net: dsa: felix: fix VLAN-unaware reception Vladimir Oltean
2024-12-16 10:10   ` Robert Hodaszi [this message]
2024-12-16 13:51     ` Vladimir Oltean
2024-12-16 14:39       ` Robert Hodaszi
2024-12-16 14:48         ` Vladimir Oltean
2024-12-17 10:15           ` Simon Horman

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=908ec18c-3d04-4cc9-a152-e41b17c5b315@digi.com \
    --to=robert.hodaszi@digi.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vladimir.oltean@nxp.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