* [PATCH] net: VRF: Fix receiving multicast traffic
@ 2016-09-22 4:13 Mark Tomlinson
2016-09-22 15:14 ` David Ahern
0 siblings, 1 reply; 6+ messages in thread
From: Mark Tomlinson @ 2016-09-22 4:13 UTC (permalink / raw)
To: netdev, dsa; +Cc: Mark Tomlinson
The previous patch to ensure that the original iif was used when
checking for forwarding also meant that this same interface was used to
determine whether multicast packets should be received or not. This was
incorrect, and would cause multicast packets to be dropped.
The fix here is to use skb->dev when checking multicast addresses.
skb->dev has been set to the l3mdev by this point, so the check will be
against that, rather than the ingress interface.
Fixes: "net:VRF: Pass original iif to ip_route_input()"
Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>
---
net/ipv4/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a1f2830..75e1de6 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1971,7 +1971,7 @@ int ip_route_input_noref(struct sk_buff *skb, __be32 daddr, __be32 saddr,
route cache entry is created eventually.
*/
if (ipv4_is_multicast(daddr)) {
- struct in_device *in_dev = __in_dev_get_rcu(dev);
+ struct in_device *in_dev = __in_dev_get_rcu(skb->dev);
if (in_dev) {
int our = ip_check_mc_rcu(in_dev, daddr, saddr,
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
2016-09-22 4:13 [PATCH] net: VRF: Fix receiving multicast traffic Mark Tomlinson
@ 2016-09-22 15:14 ` David Ahern
2016-09-22 22:10 ` Mark Tomlinson
0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2016-09-22 15:14 UTC (permalink / raw)
To: Mark Tomlinson, netdev
On 9/21/16 10:13 PM, Mark Tomlinson wrote:
> The previous patch to ensure that the original iif was used when
> checking for forwarding also meant that this same interface was used to
> determine whether multicast packets should be received or not. This was
> incorrect, and would cause multicast packets to be dropped.
>
> The fix here is to use skb->dev when checking multicast addresses.
> skb->dev has been set to the l3mdev by this point, so the check will be
> against that, rather than the ingress interface.
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
2016-09-22 15:14 ` David Ahern
@ 2016-09-22 22:10 ` Mark Tomlinson
2016-09-22 22:41 ` David Ahern
0 siblings, 1 reply; 6+ messages in thread
From: Mark Tomlinson @ 2016-09-22 22:10 UTC (permalink / raw)
To: David Ahern, netdev
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
2016-09-22 22:10 ` Mark Tomlinson
@ 2016-09-22 22:41 ` David Ahern
2016-09-23 3:06 ` Mark Tomlinson
0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2016-09-22 22:41 UTC (permalink / raw)
To: Mark Tomlinson, netdev
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
2016-09-22 22:41 ` David Ahern
@ 2016-09-23 3:06 ` Mark Tomlinson
2016-09-23 13:49 ` David Ahern
0 siblings, 1 reply; 6+ messages in thread
From: Mark Tomlinson @ 2016-09-23 3:06 UTC (permalink / raw)
To: David Ahern, netdev
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
2016-09-23 3:06 ` Mark Tomlinson
@ 2016-09-23 13:49 ` David Ahern
0 siblings, 0 replies; 6+ messages in thread
From: David Ahern @ 2016-09-23 13:49 UTC (permalink / raw)
To: Mark Tomlinson, netdev
On 9/22/16 9:06 PM, Mark Tomlinson wrote:
>
> 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?
>
Yes, but with the context of the bigger IPv4 multicast solution. I am on PTO today - about to get on a plane. How about we leave the upstream kernel as is - i.e., drop this patch. You can carry it locally with the others. We can take a look at the bigger mcast picture for 4.10. Agree?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-23 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 4:13 [PATCH] net: VRF: Fix receiving multicast traffic Mark Tomlinson
2016-09-22 15:14 ` David Ahern
2016-09-22 22:10 ` Mark Tomlinson
2016-09-22 22:41 ` David Ahern
2016-09-23 3:06 ` Mark Tomlinson
2016-09-23 13:49 ` David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.