netdev - Re: [PATCH v3 net-next 0/5] IGMP snooping for local traffic

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: <c9ad07c3-1e45-6814-02be-97c52d981405@gmail.com>
Date:   Tue, 7 Nov 2017 10:10:30 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:     David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 net-next 0/5] IGMP snooping for local traffic

Hi Andrew,

On 11/07/2017 09:42 AM, Andrew Lunn wrote:
> On Tue, Nov 07, 2017 at 12:03:54PM -0500, Vivien Didelot wrote:
>> Hi Andrew,
>>
>> Andrew Lunn <andrew@...n.ch> writes:
>>
>>> Then starts the work passing down to the hardware that the host has
>>> joined/left a group. The existing switchdev mdb object cannot be used,
>>> since the semantics are different. The existing
>>> SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
>>> group should be forwarded out that port of the switch. However here we
>>> require the exact opposite. We want multicast frames for the group
>>> received on the port to the forwarded to the host. Hence add a new
>>> object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
>>> forward to the host. This new object is then propagated through the
>>> DSA layers. No DSA driver changes should be needed, this should just
>>> work...
>>
>> The patchset looks good. I would like to ask you some details though,
>> because I don't understand why the semantics are different.
>>
>> Technically, what happens is that an MDB entry is programmed on the
>> bridge interface. From the _bridge point of view_, there is no technical
>> difference as in programming an MDB entry on a bridged port. Correct?
> 
> Hi Vivien
> 
> It is not quite as simple as that. Image:
> 
> brctl addbr br0
> brctl addif br0 eth2
> brctl addif br0 lan0
> 
> where eth2 is just a regular ethernet interface. Say there is a join
> received on eth2 for group 224.42.42.42. The IGMP snooping code in the
> software bridge then needs to tell lan0 to forward all multicast
> traffic for 224.42.42.42 to the software bridge, so it can forward it
> to eth2. The br0 interface is not involved.
> 
> Now, my patchset is not implementing this use case. But at some point,
> we probably will want to implement it. We want a generic switchdev API
> which says forward all the traffic for a group to the host. The host
> will then decide what to do with it.

What this seems to indicate though is that if any of the member of the
bridge (say lan0 here) is part of a switchdev/offload capable device, we
also need to have an event targeting the management interface of that
offload device (DSA master netdev in our case) to let the specific
traffic go all the way to the software bridge and then have the software
bridge send back to the other member, right?

> 
> Now, your suggestion would be to pass br0 for the use case i'm
> implementing here. And we could pass eth2 for the above use cases. But
> for the hardware offload, it does not matter what interface the frames
> are heading towards. All the offload needs to know is that the host
> software bridge wants the frames.

Correct, but the software bridge needs to be "backed" by some kind of
management network device, here that would be lan0 + the DSA master
netdev, so we should have 3 MDB notifications to let multicast flow through:

- one targeting lan0, which would be treated as a normal switch port
- one targeting lan0's DSA master netdev, to make multicast go all the
way to the host, via lan0
- one targeting eth2, which would be a normal NIC

Does that make sense?

> 
> Multicast is often special in that you need to specify where the
> frames are coming from, not where they are going to. This makes the
> semantics different. And you need to keep the differences clear,
> otherwise you quickly get confused. Using a different API should be a
> clear warning. Be careful, the semantics are different. Re-using an
> existing API, and needing to carefully look at the parameters to know
> you need to do something completed different is just asking for
> trouble.
> 
>       Andrew
> 


-- 
Florian