Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support

[Hangbin Liu <liuhangbin@gmail.com>]

2016-11-18 18:31 GMT+08:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 18/11/16 11:09, Nikolay Aleksandrov wrote:
>>
>> On 18/11/16 11:04, Nikolay Aleksandrov wrote:
>>>
>>> (+CC Roopa)
>>> Hi Hangbin,
>>> This patch is not correct, you should not use the net device IGMP config
>>> in the bridge.
>>
>> * bridge net device, sorry not port

Hi Nikolay,

Thanks for the comments. You are right. I was just thinking to make the IGMP/MLD
version configurable and use existing variables directly. But we
should control it
per-bridge basis.

>>
>>> These packets may never make it to the host and they may not be
>>> reflected, even more the
>>> host may have very different igmp config for the port net device than the
>>> bridge does.
>>
>> * again bridge net device, not port
>>
>>> Also your current implementation switches to V3 only if it is globally
>>> set, and that should
>>> be controlled on a per-bridge basis, even per-vlan later.
>>
>> * it is per-bridge, you use the global net device IGMP config, but it
>> should be in the bridge
>> mcast control options, so we can do it per-vlan later and also be able to
>> do the compat parts
>> of the RFC and in general, the bridge is configured via its own options
>> not the host net device
>> because as I said these packets may never be received locally
>
>
> Having the host netdev options affect the bridge-specific config is really
> undesirable and confusing.
>
>>
>>> One more thing, if someone does a V2 leave for a group, you can end up
>>> sending them V3
>>> group-specific query.
>>>
>>> I have been working on an implementation for IGMPv3/MLDv2 querier for the
>>> bridge device, it re-uses
>>> most of the current code and allows you to configure it per-bridge (and
>>> later per-vlan). The only
>>> reason I've not yet sent it to upstream is that we (at Cumulus) are
>>> working out the compatibility
>>> parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear
>>> cases.

Yeah, even in the host igmp/mld code we have some vague areas and different
handles.

>
>
> Err, RFC3376 sec 7.3.1, 7.3.2, and RFC3810 sec 8.3.1, 8.3.2
>
> I really should get coffee.. sorry for the multiple emails :-)
>
>
>>> But I can rip the compatibility out and send it early for review if you'd
>>> like, we can add the
>>> compat code later.

OK, then I will wait for your patch.

Thanks
Hangbin