netdev - Re: [PATCH/RFC net-next] rocker: by default accept untagged packets

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: <CAE4R7bBYK0zJbb_q0BUowh_jrrCMapVq8xouD5bv56hPXYGsWQ@mail.gmail.com>
Date:	Tue, 26 May 2015 02:04:00 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Simon Horman <simon.horman@...ronome.com>
Cc:	Jiri Pirko <jiri@...nulli.us>, David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH/RFC net-next] rocker: by default accept untagged packets

On Tue, May 26, 2015 at 12:28 AM, Scott Feldman <sfeldma@...il.com> wrote:
> On Mon, May 25, 2015 at 5:55 PM, Simon Horman
> <simon.horman@...ronome.com> wrote:
>> This will occur anyway if the 8021q module is loaded as it will
>> call rocker_port_vlan_rx_add_vid for vlan 0. This code is here
>> to handle the case where the 8021q module is not loaded.
>>
>> This patch also handles the case where the 8021q is unloaded
>> removing all VLANs from all ports.
>>
>> This change should not affect bridging, although the rules are
>> harmlessly installed anyway. This is in keeping with the behaviour
>> for VLANs when the 8021q modules is loaded.
>>
>> To aid implementation of the above provide a helper
>> and use it to replace some existing code.
>>
>> Signed-off-by: Simon Horman <simon.horman@...ronome.com>

[cut]

>
> Hi Simon,
>
> Thanks for looking into this one.  I looked at your patch and the code
> and I think we can streamline it a little bit more.  For the
> no-8021q-module case we use rocker_port_vlan_add() and
> rocker_port_vlan_del() to add/del bridge vlans.  We should be able to
> move those functions up in the file so they can be called from
> rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid(),
> passing trans=SWITCHDEV_TRANS_NONE, but only if vid != 0.  Next, like
> in your patch, we need to call rocker_port_vlan_add() in
> rocker_port_open(), passing in vid=0 for untagged.  And in
> rocker_port_stop(), call rocker_port_vlan_del(), again passing in
> vid=0.
>
> To summarize:
>
> Call rocker_port_vlan_add() from:
>
> 1) rocker_port_open with vid=0
> 2) rocker_port_vlans_add()                             // bridge vlan
> 3) rocker_port_vlan_rx_add_vid() if vid != 0       // 8021q vlan
>
> Call rocker_port_vlan_del() from:
>
> 1) rocker_port_stop with vid=0
> 2) rocker_port_vlans_del()                              // bridge vlan
> 3) rocker_port_vlan_rx_kill_vid() if vid != 0        // 8021q vlan
>
> Does this look right?


Hmmmm...things get simpler if we removed support for 8021q module in
rocker driver by removing NETIF_F_HW_VLAN_CTAG_FILTER.  That gets rid
of rocker_port_vlan_rx_add_vid() and rocker_port_vlan_rx_kill_vid().
Leaving us with the bridge VLAN interface to add/del/show vlans on the
port.  I'm wondering if we can also avoid setting up untagged traffic
on the port during port open by requiring a explicit command on the
port from the user:

bridge vlan add vid 0 dev DEV master self        // enable untagged
traffic on port

Do you have a requirement for 8021q module?  I'm leaning towards a
clean break from 8021q and using just the built-in bridge VLAN
support.  I'm curious if others have an opinion about 8021q module
used with switchdev devices.

-scott
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html