From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>,
netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>
Cc: Andrew Lunn <andrew@lunn.ch>, Vivien Didelot <vivien.didelot@gmail.com>
Subject: Re: [PATCH net-next 2/3] net: dsa: sja1105: make sure untagged packets are dropped on ingress ports with no pvid
Date: Wed, 28 Jul 2021 21:23:21 -0700 [thread overview]
Message-ID: <047854fe-a850-d105-1b62-6cbb49823fc4@gmail.com> (raw)
In-Reply-To: <20210728215429.3989666-3-vladimir.oltean@nxp.com>
On 7/28/2021 2:54 PM, Vladimir Oltean wrote:
> Surprisingly, this configuration:
>
> ip link add br0 type bridge vlan_filtering 1
> ip link set swp2 master br0
> bridge vlan del dev swp2 vid 1
>
> still has the sja1105 switch sending untagged packets to the CPU (and
> failing to decode them, since dsa_find_designated_bridge_port_by_vid
> searches by VID 1 and rightfully finds no bridge VLAN 1 on a port).
>
> Dumping the switch configuration, the VLANs are managed properly:
> - the pvid of swp2 is 1 in the MAC Configuration Table, but
> - only the CPU port is in the port membership of VLANID 1 in the VLAN
> Lookup Table
>
> When the ingress packets are tagged with VID 1, they are properly
> dropped. But when they are untagged, they are able to reach the CPU
> port. Also, when the pvid in the MAC Configuration Table is changed to
> e.g. 55 (an unused VLAN), the untagged packets are also dropped.
>
> So it looks like:
> - the switch bypasses ingress VLAN membership checks for untagged traffic
> - the reason why the untagged traffic is dropped when I make the pvid 55
> is due to the lack of valid destination ports in VLAN 55, rather than
> an ingress membership violation
> - the ingress VLAN membership cheks are only done for VLAN-tagged traffic
>
> Interesting. It looks like there is an explicit bit to drop untagged
> traffic, so we should probably be using that to preserve user expectations.
>
> Note that only VLAN-aware ports should drop untagged packets due to no
> pvid - when VLAN-unaware, the software bridge doesn't do this even if
> there is no pvid on any bridge port and on the bridge itself. So the new
> sja1105_drop_untagged() function cannot simply be called with "false"
> from sja1105_bridge_vlan_add() and with "true" from sja1105_bridge_vlan_del.
> Instead, we need to also consider the VLAN awareness state. That means
> we need to hook the "drop untagged" setting in all the same places where
> the "commit pvid" logic is, and it needs to factor in all the state when
> flipping the "drop untagged" bit: is our current pvid in the VLAN Lookup
> Table, and is the current port in that VLAN's port membership list?
> VLAN-unaware ports will never drop untagged frames because these checks
> always succeed by construction, and the tag_8021q VLANs cannot be changed
> by the user.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
next prev parent reply other threads:[~2021-07-29 4:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-28 21:54 [PATCH net-next 0/3] NXP SJA1105 VLAN regressions Vladimir Oltean
2021-07-28 21:54 ` [PATCH net-next 1/3] net: dsa: sja1105: reset the port pvid when leaving a VLAN-aware bridge Vladimir Oltean
2021-07-29 4:22 ` Florian Fainelli
2021-07-28 21:54 ` [PATCH net-next 2/3] net: dsa: sja1105: make sure untagged packets are dropped on ingress ports with no pvid Vladimir Oltean
2021-07-29 4:23 ` Florian Fainelli [this message]
2021-07-28 21:54 ` [PATCH net-next 3/3] net: dsa: tag_sja1105: fix control packets on SJA1110 being received on an imprecise port Vladimir Oltean
2021-07-29 14:40 ` [PATCH net-next 0/3] NXP SJA1105 VLAN regressions patchwork-bot+netdevbpf
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=047854fe-a850-d105-1b62-6cbb49823fc4@gmail.com \
--to=f.fainelli@gmail.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.