On 09/23/2016 10:41 AM, David Ahern wrote:
> On 9/22/16 4:10 PM, Mark Tomlinson wrote:
>> On 09/23/2016 03:14 AM, David Ahern wrote:
>>> l3mdev devices do not support IPv4 multicast so checking mcast against that 
>>> device should not be working at all. For that reason I was fine with the 
>>> change in the previous patch. ie., you want the real ingress device there 
>>> not the vrf device.
>>>
>>> What test are you running that says your previous patch broke something?
>> Although we do not expect any multicast routing to work in an l3mdev,
>> (IGMP snooping or PIM), we still want to have multicast packets
>> delivered for protocols such as RIP. This was working before my previous
>> patch, but these multicast packets are now dropped. This current patch
>> fixes that again, hopefully still with the benefits of my first patch.
>>
> can you discern which check is making that happen?
>
> It does not make sense to look at the in_device of a vrf device for mcast 
> addresses. For IPv6 linklocal and mcast is specifically blocked. IPv4 should 
> do the same. So, how is RIP getting the packet at all?
This might be due to some other changes we've made for VRF and multicast 
but haven't sent upstream. In particular, a change to do_ip_setsockopt() 
and its handling of IP_MULTICAST_IF as well as IP_ADD/DROP_MEMBERSHIP. I 
am guessing that without these changes, we wouldn't be able to receive 
multicast packets in RIP. With our changes, the in_dev->mc_list does 
contain the RIP MC address (224.0.0.9) in the master interface, and so 
the function ip_check_mc_rcu() returns success with the master only.

Our RIP daemon is VRF-aware. So it does use setsockopt(SO_BINDTODEVICE, 
"vrf-master") when running in a VRF. Without following it all the way 
down, I believe that it is this that allows the multicast lookup at the 
top of ip_check_mc_rcu() to succeed on the vrf-master, but not the 
ingress interface. That is, in_dev->mc_list does contain 224.0.0.9 only 
on the vrf-master. Provided the lookup in ip_check_mc_rcu() succeeds (im 
!= NULL), this function can return success.

Are you interested in the other patches at the moment?

Reply via email to