netdev - Re: [PATCH net-next 4/4] net: dsa: set configure_vlan_while_not

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]

Date:   Thu, 10 Sep 2020 14:58:04 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     davem@...emloft.net, vivien.didelot@...il.com, andrew@...n.ch,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/4] net: dsa: set
 configure_vlan_while_not_filtering to true by default



On 9/9/2020 11:34 AM, Florian Fainelli wrote:
> 
> 
> On 9/9/2020 10:53 AM, Vladimir Oltean wrote:
>> On Wed, Sep 09, 2020 at 10:22:42AM -0700, Florian Fainelli wrote:
>>> How do you make sure that the CPU port sees the frame untagged which 
>>> would
>>> be necessary for a VLAN-unaware bridge? Do you have a special remapping
>>> rule?
>>
>> No, I don't have any remapping rules that would be relevant here.
>> Why would the frames need to be necessarily untagged for a VLAN-unaware
>> bridge, why is it a problem if they aren't?
>>
>> bool br_allowed_ingress(const struct net_bridge *br,
>>             struct net_bridge_vlan_group *vg, struct sk_buff *skb,
>>             u16 *vid, u8 *state)
>> {
>>     /* If VLAN filtering is disabled on the bridge, all packets are
>>      * permitted.
>>      */
>>     if (!br_opt_get(br, BROPT_VLAN_ENABLED)) {
>>         BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
>>         return true;
>>     }
>>
>>     return __allowed_ingress(br, vg, skb, vid, state);
>> }
>>
>> If I have a VLAN on a bridged switch port where the bridge is not
>> filtering, I have an 8021q upper of the bridge with that VLAN ID.
> 
> Yes that is the key right there, you need an 8021q upper to pop the VLAN 
> ID or push it, that is another thing that users need to be aware of 
> which is a bit awkward, most expect things to just work. Maybe we should 
> just refuse to have bridge devices that are not VLAN-aware, because this 
> is just too cumbersome to deal with.

With the drivers that you currently maintain and with the CPU port being 
always tagged in the VLANs added to the user-facing ports, when you are 
using a non-VLAN aware bridge, do you systematically add an br0.1 upper 
802.1Q device to pop/push the VLAN tag?

I am about ready to submit the changes we discussed to b53, but I am 
still a bit uncomfortable with this part of the change because it will 
make the CPU port follow the untagged attribute of an user-facing port.

@@ -1444,7 +1427,7 @@ void b53_vlan_add(struct dsa_switch *ds, int port,
                         untagged = true;

                 vl->members |= BIT(port);
-               if (untagged && !dsa_is_cpu_port(ds, port))
+               if (untagged)
                         vl->untag |= BIT(port);
                 else
                         vl->untag &= ~BIT(port);
@@ -1482,7 +1465,7 @@ int b53_vlan_del(struct dsa_switch *ds, int port,
                 if (pvid == vid)
                         pvid = b53_default_pvid(dev);

-               if (untagged && !dsa_is_cpu_port(ds, port))
+               if (untagged)
                         vl->untag &= ~(BIT(port));

-- 
Florian