Linux Ethernet Bridging - Feb 2019 - [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check in mcast queries

Hi Linus,

Sorry, my mail client somehow droped your message and I didn't see your
reply.
I find this mail after Nikolay pointed out yesterday.
> Hi and thanks for your reply!
> 
> On Fri, Dec 14, 2018 at 10:32:16AM +0800, Ying Xu wrote:
> >  I think the scenario mentioned above is abnormal.
> 
> Can we agree, that this scenario, if switch A and B were using the
> current bridge code, has issues right now which it did
> not have before that patch?
Yes, I agree. But this "regression" could be fixed by setting up
correct
switch configuration. See more explains below.
> 
> I also do not quite understand what you mean with "abnormal". Do
> you think that it is unlikely to have two snooping switches and
> general queries with a 0.0.0.0 source?
The "abnormal" means we shouldn't use this kind of configuration
on
snooping switch.

We could have general queries with a 0.0.0.0 source, but these queries
are only from proxy Querier, not from real Querier.

Based on RFC 4541,
2.1.1.  IGMP Forwarding Rules
      b) The arrival port for IGMP Queries (sent by multicast routers)
         where the source address is not 0.0.0.0.

         The 0.0.0.0 address represents a special case where the switch
         is proxying IGMP Queries for faster network convergence, but is
         not itself the Querier.  The switch does not use its own IP
         address (even if it has one), because this would cause the
         Queries to be seen as coming from a newly elected Querier.  The
         0.0.0.0 address is used to indicate that the Query packets are
         NOT from a multicast router.

This paragraph specified "0.0.0.0" is only for switch proxying
queries.
It should not be used as a IGMP Querier.

"because this would cause the Queries to be seen as coming from a newly
 elected Querier" means other address could be elected as a Querier but
"0.0.0.0" should not.

AFAIK, most verdors(Cisco, HW, etc. expect H3C) use none-zero address
as default address on proxying switches. But H3C also respect the
b) section.
>
> Note that with the current bridge code and according to RFC3376
> and RFC2236, as soon as a query with a 0.0.0.0 source is sent somewhere
> in the broadcast domain, it will become the selected querier [*].
Yes, In RFC 3376

6.6.2. Querier Election

   IGMPv3 elects a single querier per subnet using the same querier
   election mechanism as IGMPv2, namely by IP address.  When a router
   receives a query with a lower IP address, it sets the Other-Querier-
   Present timer to Other Querier Present Interval and ceases to send
   queries on the network if it was the previously elected querier.
   After its Other-Querier Present timer expires, it should begin
   sending General Queries.

It only said we should select lower IP address as Querier, but didn't
specify
the "0.0.0.0" address.

While in the later RFC 4541

Abstract
   This memo describes the recommendations for Internet Group Management
   Protocol (IGMP) and Multicast Listener Discovery (MLD) snooping
   switches.  These are based on best current practices for IGMPv2, with
   further considerations for IGMPv3- and MLDv2-snooping.

And section 2.1.1.  IGMP Forwarding Rules(I just pasted above). It did
specify the "0.0.0.0" address issue.

I think we should respect the later and accurater standard, shouldn't we?
>
>
> > The source of query indicats that is a real router or only a
switch.(0.0.0.0
> > means switch,non-zero means router).
> > In the scenario above,the switch A was selected to be a querier that
means A
> > performs as a router,
> > so switch A should config its query source address to non-zero,and
then Host A
> > can recieve the traffic from B.
>
> Even if in the described scenario switch A were configured to use a a
non-zero
> source address to become a router, so that switch B would mark the port
> to switch A as a multicast router port, switch A would still loose
> in the querier election right now, as 0.0.0.0 is lower (RFC3376, RFC2236).
So
> switch B would then become the selected querier with its 0.0.0.0 source
> and switch A would become silent even though it had a non-zero
> source address.
>
> And then we would have the same issue again, only swapped between
> host+switch A and host+switch B.
>
> Would you agree, does that make sense?
>
Yes, that's why Ying Xu said the topology is "abnormal", or
illegal. Because
in a correct configured topology, there should has a Querier with none-zero
IP address.
> Regards, Linus
>
>
> [*]: Looking at br_ip4_multicast_select_querier():
>
> If previously selected querier were 0.0.0.0, it would accept any
> source as a new querier ("!br->ip4_querier.addr.u.ip4").
However,
> if the previously selected querier were non-zero, a query with
> 0.0.0.0 would win, too
> ("ntohl(saddr) <= ntohl(br->ip4_querier.addr.u.ip4)").
In RFC 3810

7.6.2.  Querier Election
   When a router starts operating on a subnet, by default it
   considers itself as being the Querier.

   When a router receives a query with a lower IPv6 address than its
   own, it sets the Other Querier Present timer to Other Querier Present
   Timeout; if it was previously in Querier state, it switches to Non-
   Querier state and ceases to send queries on the link.

   All MLDv2 queries MUST be sent with the FE80::/64 link-local source
   address prefix.

MLDv2 also sepcified the default Querier is itself and electe a lower adddress
as new Querier. And the source address should be link-loacl source address
instead of 0("::"). This is another evidence that we should not use
"0.0.0.0"
as a Querier. So I think we should fix the IPv4 election in function
br_ip4_multicast_select_querier().

This looks like a little extreme and may have "regression", but the
"regression"
should be fixed by setting up correct router/switch configuration.

What do you think?

Regards
Hangbin

On 21/02/2019 10:01, Hangbin Liu wrote:
> Hi Linus,
> 
> Sorry, my mail client somehow droped your message and I didn't see your
reply.
> I find this mail after Nikolay pointed out yesterday.
> 
>> Hi and thanks for your reply!
>>
>> On Fri, Dec 14, 2018 at 10:32:16AM +0800, Ying Xu wrote:
>>>  I think the scenario mentioned above is abnormal.
>>
>> Can we agree, that this scenario, if switch A and B were using the
>> current bridge code, has issues right now which it did
>> not have before that patch?
> 
> Yes, I agree. But this "regression" could be fixed by setting up
correct
> switch configuration. See more explains below.
> 
That is irrelevant, if the setup once worked we should not break it unless
it's in RFC requirement violation and RFC 4541 is only suggestive, not
required.
>>
>> I also do not quite understand what you mean with "abnormal".
Do
>> you think that it is unlikely to have two snooping switches and
>> general queries with a 0.0.0.0 source?
> 
> The "abnormal" means we shouldn't use this kind of
configuration on
> snooping switch.
> 
> We could have general queries with a 0.0.0.0 source, but these queries
> are only from proxy Querier, not from real Querier.
> 
> Based on RFC 4541,
> 2.1.1.  IGMP Forwarding Rules
>       b) The arrival port for IGMP Queries (sent by multicast routers)
>          where the source address is not 0.0.0.0.
> 
>          The 0.0.0.0 address represents a special case where the switch
>          is proxying IGMP Queries for faster network convergence, but is
>          not itself the Querier.  The switch does not use its own IP
>          address (even if it has one), because this would cause the
>          Queries to be seen as coming from a newly elected Querier.  The
>          0.0.0.0 address is used to indicate that the Query packets are
>          NOT from a multicast router.
> 
> This paragraph specified "0.0.0.0" is only for switch proxying
queries.
> It should not be used as a IGMP Querier.
> 
> "because this would cause the Queries to be seen as coming from a
newly
>  elected Querier" means other address could be elected as a Querier
but
> "0.0.0.0" should not.
> 
But this change hasn't been incorporated, has it ? A 0.0.0.0 address
currently
will always win the election and silence all of the rest. Current bridge state
is simply broken for some cases because of that.
> AFAIK, most verdors(Cisco, HW, etc. expect H3C) use none-zero address
> as default address on proxying switches. But H3C also respect the
> b) section.
> 
>>
>> Note that with the current bridge code and according to RFC3376
>> and RFC2236, as soon as a query with a 0.0.0.0 source is sent somewhere
>> in the broadcast domain, it will become the selected querier [*].
> 
> Yes, In RFC 3376
> 
> 6.6.2. Querier Election
> 
>    IGMPv3 elects a single querier per subnet using the same querier
>    election mechanism as IGMPv2, namely by IP address.  When a router
>    receives a query with a lower IP address, it sets the Other-Querier-
>    Present timer to Other Querier Present Interval and ceases to send
>    queries on the network if it was the previously elected querier.
>    After its Other-Querier Present timer expires, it should begin
>    sending General Queries.
> 
> It only said we should select lower IP address as Querier, but didn't
specify
> the "0.0.0.0" address.
> 
> While in the later RFC 4541
> 
> Abstract
>    This memo describes the recommendations for Internet Group Management
>    Protocol (IGMP) and Multicast Listener Discovery (MLD) snooping
>    switches.  These are based on best current practices for IGMPv2, with
>    further considerations for IGMPv3- and MLDv2-snooping.
> 
> And section 2.1.1.  IGMP Forwarding Rules(I just pasted above). It did
> specify the "0.0.0.0" address issue.
> 
> I think we should respect the later and accurater standard, shouldn't
we?
> 
No if it is breaking valid setups, RFC 4541 is only a suggestion and in this
case
the bridge has worked like that for a long time. The current situation is broken
because a 0.0.0.0 querier will win the election always and will silence the
rest.

If it's not involved in the election then we'll have snooping
effectively disabled
unless a non-zero querier shows up which again sounds wrong and was already
pointed
out by Linus. I think the best course of action is to revert this change.
>>
>>
>>> The source of query indicats that is a real router or only a
switch.(0.0.0.0
>>> means switch,non-zero means router).
>>> In the scenario above,the switch A was selected to be a querier
that means A
>>> performs as a router,
>>> so switch A should config its query source address to non-zero,and
then Host A
>>> can recieve the traffic from B.
>>
>> Even if in the described scenario switch A were configured to use a a
non-zero
>> source address to become a router, so that switch B would mark the port
>> to switch A as a multicast router port, switch A would still loose
>> in the querier election right now, as 0.0.0.0 is lower (RFC3376,
RFC2236). So
>> switch B would then become the selected querier with its 0.0.0.0 source
>> and switch A would become silent even though it had a non-zero
>> source address.
>>
>> And then we would have the same issue again, only swapped between
>> host+switch A and host+switch B.
>>
>> Would you agree, does that make sense?
>>
> 
> Yes, that's why Ying Xu said the topology is "abnormal", or
illegal. Because
> in a correct configured topology, there should has a Querier with none-zero
> IP address.
> 
Certainly not illegal.
>> Regards, Linus
>>
>>
>> [*]: Looking at br_ip4_multicast_select_querier():
>>
>> If previously selected querier were 0.0.0.0, it would accept any
>> source as a new querier ("!br->ip4_querier.addr.u.ip4").
However,
>> if the previously selected querier were non-zero, a query with
>> 0.0.0.0 would win, too
>> ("ntohl(saddr) <= ntohl(br->ip4_querier.addr.u.ip4)").
> 
> In RFC 3810
> 
> 7.6.2.  Querier Election
>    When a router starts operating on a subnet, by default it
>    considers itself as being the Querier.
> 
>    When a router receives a query with a lower IPv6 address than its
>    own, it sets the Other Querier Present timer to Other Querier Present
>    Timeout; if it was previously in Querier state, it switches to Non-
>    Querier state and ceases to send queries on the link.
> 
>    All MLDv2 queries MUST be sent with the FE80::/64 link-local source
>    address prefix.
> 
> MLDv2 also sepcified the default Querier is itself and electe a lower
adddress
> as new Querier. And the source address should be link-loacl source address
> instead of 0("::"). This is another evidence that we should not
use "0.0.0.0"
> as a Querier. So I think we should fix the IPv4 election in function
> br_ip4_multicast_select_querier().
> 
> This looks like a little extreme and may have "regression", but
the "regression"
> should be fixed by setting up correct router/switch configuration.
> 
> What do you think?
> 
Removing 0.0.0.0 from the election will effectively disable snooping even if
there's
a configured bridge unless it has an address. You can see that this will end up
in
people having suddenly their multicast flooded with current setups, right ?
Any big behaviour change like that should be optional, but I don't think we
need
another option as this is not so big of a deal because we're not breaking
any
required behaviour.
In case you decide to follow the option path, please use the new boolopt api to
avoid
adding new fields to the bridge, this should be an on/off thing. I still vote
for a
revert though.
> Regards
> Hangbin
> 
Thanks,
 Nik