netdev - Re: [PATCH net-next 10/14] net: vlan: Propagate MC addresses with VID through switchdev

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, 17 Jan 2019 11:12:24 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Ido Schimmel <idosch@...lanox.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>,
        "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>,
        "ivan.khoronzhuk@...aro.org" <ivan.khoronzhuk@...aro.org>,
        "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
        "nikolay@...ulusnetworks.com" <nikolay@...ulusnetworks.com>
Subject: Re: [PATCH net-next 10/14] net: vlan: Propagate MC addresses with VID
 through switchdev

On 1/17/19 6:49 AM, Ido Schimmel wrote:
> On Wed, Jan 16, 2019 at 12:00:58PM -0800, Florian Fainelli wrote:
>> The VLAN real device could be an Ethernet switch port and that switch
>> might have VLAN filtering globally enabled (because of a bridge
>> requesting VLAN filtering on the switch on another port) and so when
>> programming multicast addresses, we need the multicast filter
>> programming to be aware of the correct VLAN ID as well.
> 
> This looks like a quirk of a specific device. How bad is it to patch the
> driver to add a multicast address for every configured VLAN?

There is at least another driver that can be benefit from that which is
cpsw, if I understand Ivan's use case correctly.

If there is a ndo_set_rx_mode() function implemented by the virtual
device, and that does call dev_{mc,uc}_sync(master, dev), then this
means that you do want to be able to filter UC and MC addresses. If we
added the entire class D range of multicast addresses to the switch's
MDB, that would not be filtering, we would be passing up everything to
the stack and let it filter in software because there is no multicast
socket listening on that address.

> 
> Also, I think it's weird that we have one API to program address and a
> completely different API (via switchdev) to program address+VID pairs.
> Extending current API might make more sense.
> 

Do you mean ndo_set_rx_mode() and dev_mc_sync()? That is what Ivan
proposed doing not so long ago here:

https://www.spinics.net/lists/netdev/msg537424.html

but that is IMHO wasting storage space, because the kernel is
maintaining the address lists, and now also needs to gain knowledge
about the VID. With up to 4K - 2 VLAN interfaces per switch port, this
bloats the memory footprint, we arguably still need to maintain those
address lists anyway...

The reason why I chose switchdev here is because:

- this is mostly relevant for switch devices, not so much for NICs (it
seems), if it was, they would have solved the problem by now

- this allows to have an unified path from the switch driver perspective
to program MDB addresses targeting the CPU/management port, no need to
have X different ways of doing the same operation
-- 
Florian