netdev - Re: [PATCH net] net: mscc: ocelot: fix untagged packet drops when enslaving to vlan aware bridge

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] [day] [month] [year] [list]

Date:   Wed, 15 Apr 2020 12:29:35 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     olteanv@...il.com
Cc:     horatiu.vultur@...rochip.com, alexandre.belloni@...tlin.com,
        antoine.tenart@...tlin.com, andrew@...n.ch, f.fainelli@...il.com,
        vivien.didelot@...il.com, joergen.andreasen@...rochip.com,
        allan.nielsen@...rochip.com, claudiu.manoil@....com,
        netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com,
        alexandru.marginean@....com, xiaoliang.yang_1@....com,
        yangbo.lu@....com, po.liu@....com, jiri@...lanox.com,
        idosch@...sch.org, kuba@...nel.org
Subject: Re: [PATCH net] net: mscc: ocelot: fix untagged packet drops when
 enslaving to vlan aware bridge

From: Vladimir Oltean <olteanv@...il.com>
Date: Tue, 14 Apr 2020 22:36:15 +0300

> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> To rehash a previous explanation given in commit 1c44ce560b4d ("net:
> mscc: ocelot: fix vlan_filtering when enslaving to bridge before link is
> up"), the switch driver operates the in a mode where a single VLAN can
> be transmitted as untagged on a particular egress port. That is the
> "native VLAN on trunk port" use case.
> 
> The configuration for this native VLAN is driven in 2 ways:
>  - Set the egress port rewriter to strip the VLAN tag for the native
>    VID (as it is egress-untagged, after all).
>  - Configure the ingress port to drop untagged and priority-tagged
>    traffic, if there is no native VLAN. The intention of this setting is
>    that a trunk port with no native VLAN should not accept untagged
>    traffic.
> 
> Since both of the above configurations for the native VLAN should only
> be done if VLAN awareness is requested, they are actually done from the
> ocelot_port_vlan_filtering function, after the basic procedure of
> toggling the VLAN awareness flag of the port.
> 
> But there's a problem with that simplistic approach: we are trying to
> juggle with 2 independent variables from a single function:
>  - Native VLAN of the port - its value is held in port->vid.
>  - VLAN awareness state of the port - currently there are some issues
>    here, more on that later*.
> The actual problem can be seen when enslaving the switch ports to a VLAN
> filtering bridge:
>  0. The driver configures a pvid of zero for each port, when in
>     standalone mode. While the bridge configures a default_pvid of 1 for
>     each port that gets added as a slave to it.
>  1. The bridge calls ocelot_port_vlan_filtering with vlan_aware=true.
>     The VLAN-filtering-dependent portion of the native VLAN
>     configuration is done, considering that the native VLAN is 0.
>  2. The bridge calls ocelot_vlan_add with vid=1, pvid=true,
>     untagged=true. The native VLAN changes to 1 (change which gets
>     propagated to hardware).
>  3. ??? - nobody calls ocelot_port_vlan_filtering again, to reapply the
>     VLAN-filtering-dependent portion of the native VLAN configuration,
>     for the new native VLAN of 1. One can notice that after toggling "ip
>     link set dev br0 type bridge vlan_filtering 0 && ip link set dev br0
>     type bridge vlan_filtering 1", the new native VLAN finally makes it
>     through and untagged traffic finally starts flowing again. But
>     obviously that shouldn't be needed.
> 
> So it is clear that 2 independent variables need to both re-trigger the
> native VLAN configuration. So we introduce the second variable as
> ocelot_port->vlan_aware.
> 
> *Actually both the DSA Felix driver and the Ocelot driver already had
> each its own variable:
>  - Ocelot: ocelot_port_private->vlan_aware
>  - Felix: dsa_port->vlan_filtering
> but the common Ocelot library needs to work with a single, common,
> variable, so there is some refactoring done to move the vlan_aware
> property from the private structure into the common ocelot_port
> structure.
> 
> Fixes: 97bb69e1e36e ("net: mscc: ocelot: break apart ocelot_vlan_port_apply")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Applied and queued up for -stable, thanks.