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]

Message-ID: <a5e6cb01-88d0-a479-3262-b53dec0682cd@gmail.com>
Date:   Tue, 8 Sep 2020 15:28:24 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, davem@...emloft.net
Cc:     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/7/2020 9:07 PM, Florian Fainelli wrote:
> 
> 
> On 9/7/2020 11:29 AM, Vladimir Oltean wrote:
>> From: Vladimir Oltean <vladimir.oltean@....com>
>>
>> As explained in commit 54a0ed0df496 ("net: dsa: provide an option for
>> drivers to always receive bridge VLANs"), DSA has historically been
>> skipping VLAN switchdev operations when the bridge wasn't in
>> vlan_filtering mode, but the reason why it was doing that has never been
>> clear. So the configure_vlan_while_not_filtering option is there merely
>> to preserve functionality for existing drivers. It isn't some behavior
>> that drivers should opt into. Ideally, when all drivers leave this flag
>> set, we can delete the dsa_port_skip_vlan_configuration() function.
>>
>> New drivers always seem to omit setting this flag, for some reason. So
>> let's reverse the logic: the DSA core sets it by default to true before
>> the .setup() callback, and legacy drivers can turn it off. This way, new
>> drivers get the new behavior by default, unless they explicitly set the
>> flag to false, which is more obvious during review.
>>
>> Remove the assignment from drivers which were setting it to true, and
>> add the assignment to false for the drivers that didn't previously have
>> it. This way, it should be easier to see how many we have left.
>>
>> The following drivers: lan9303, mv88e6060 were skipped from setting this
>> flag to false, because they didn't have any VLAN offload ops in the
>> first place.
>>
>> Also, print a message to the console every time a VLAN has been skipped.
>> This is mildly annoying on purpose, so that (a) it is at least clear
>> that VLANs are being skipped - the legacy behavior in itself is
>> confusing - and (b) people have one more incentive to convert to the new
>> behavior.
>>
>> No behavior change except for the added prints is intended at this time.
>>
>> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> 
> You should be able to make b53 and bcm_sf2 also use 
> configure_vlan_while_not_filtering to true (or rather not specify it), 
> give me until tomorrow to run various tests if you don't mind.

For a reason I do not understand, we should be able to set 
configure_vlan_while_not_filtering to true, and get things to work, 
however this does not work for bridged ports unless the bridge also 
happens to have VLAN filtering enabled. There is a valid VLAN entry 
configured for the desired port, but nothing ingress or egresses, I will 
keep debugging but you can send this patch as-is I believe and I will 
amend b53 later once I understand what is going on.
-- 
Florian