linux-kernel - Re: [PATCH RFC net 0/2] net: dsa: felix: fix VLAN-unaware reception

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <908ec18c-3d04-4cc9-a152-e41b17c5b315@digi.com>
Date: Mon, 16 Dec 2024 11:10:05 +0100
From: Robert Hodaszi <robert.hodaszi@...i.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, claudiu.manoil@....com,
 alexandre.belloni@...tlin.com, UNGLinuxDriver@...rochip.com, andrew@...n.ch,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net 0/2] net: dsa: felix: fix VLAN-unaware reception

On Sunday, 15.12.2024 at 18:09 +0100, Vladimir Oltean <vladimir.oltean@....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