| Message ID | 1365724799.3563.5.camel@sridhar.usor.ibm.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
----- Original Message ----- > > The following patch fixes the issue for me. can you try it out? > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 9a64715..d6509de 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -1013,7 +1013,7 @@ static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, > struct net_device *dev, > } > > /* Bypass encapsulation if the destination is local */ > - if (rt->rt_flags & RTCF_LOCAL) { > + if (rt->dst.dev->flags & IFF_LOOPBACK) { > struct vxlan_dev *dst_vxlan; > > ip_rt_put(rt); > It almost surely can fix the problem, but do you really just want to bypass encap for loopback devcie? Not all local devices? The title of your original commit "vxlan: Bypass encapsulation if the destination is local" is confusing... (Sorry for the delay, I am on vacation.) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sridhar Samudrala <sri@us.ibm.com> Date: Thu, 11 Apr 2013 16:59:59 -0700 > @@ -1013,7 +1013,7 @@ static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, > } > > /* Bypass encapsulation if the destination is local */ > - if (rt->rt_flags & RTCF_LOCAL) { > + if (rt->dst.dev->flags & IFF_LOOPBACK) { > struct vxlan_dev *dst_vxlan; This looks terrible, and ad-hoc. You need to find a cleaner way to express exactly what you're trying to do otherwise I'm reverting your change. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/12/2013 12:19 PM, David Miller wrote: > From: Sridhar Samudrala <sri@us.ibm.com> > Date: Thu, 11 Apr 2013 16:59:59 -0700 > >> @@ -1013,7 +1013,7 @@ static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, >> } >> >> /* Bypass encapsulation if the destination is local */ >> - if (rt->rt_flags & RTCF_LOCAL) { >> + if (rt->dst.dev->flags & IFF_LOOPBACK) { >> struct vxlan_dev *dst_vxlan; > This looks terrible, and ad-hoc. You need to find a cleaner > way to express exactly what you're trying to do otherwise I'm > reverting your change. The idea is to bypass encap if the destination vxlan endpoint is on the same host. To do this, i thought it is good enough to check if the route to reach the destination ip is a local route.(RTCF_LOCAL is set). This works fine for all the cases where destination ip is assigned to a normal ethernet device. rt->dst.dev points to loopback device. In case of veth(veth0,veth1), where the peer(veth1) and the destination vxlan endpoint are on a different netns, ip_route_output for veth1's ipis returning a route entry that has RTCF_LOCAL set and rt->dst.dev pointing to veth0. Is it a bug that RTCF_LOCAL is set here when veth0's peer is moved to a different netns? We don't want to bypass encap in this scenario. To address this behavior seen with veth, i had to change the if condition to check for rt->dst.dev->flags rather than rt->rt_flags. Thanks Sridhar -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sridhar Samudrala <sri@us.ibm.com> Date: Fri, 12 Apr 2013 16:07:41 -0700 > Is it a bug that RTCF_LOCAL is set here when veth0's peer is moved > to a different netns? Nope. RTCF_LOCAL is set in several situations. For example, it would be set on a broadcast route for a subnet we are on. It'd also be set on a multicast route for a multicast group the local system is subscribed to. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-04-12 at 16:07 -0700, Sridhar Samudrala wrote: > To address this behavior seen with veth, i had to change the if > condition to > check for rt->dst.dev->flags rather than rt->rt_flags. There is no specific IFF_ flag for veth, nor I think introducing one is a correct fix. I think we can just revert it for now, since it is not very easy to fix. You can, of course, send a bug-free version later. This regression blocks my VXLAN IPv6 support patches, by the way. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/14/2013 7:31 PM, Cong Wang wrote: > On Fri, 2013-04-12 at 16:07 -0700, Sridhar Samudrala wrote: >> To address this behavior seen with veth, i had to change the if >> condition to >> check for rt->dst.dev->flags rather than rt->rt_flags. > There is no specific IFF_ flag for veth, nor I think introducing one is > a correct fix. > > I think we can just revert it for now, since it is not very easy to fix. > You can, of course, send a bug-free version later. This regression > blocks my VXLAN IPv6 support patches, by the way. > > I am not suggesting adding a new IFF_ flag for veth. I was referring to the IFF_LOOPBACKflag and it should work fine for your setup. However, i think Mike Rapaport't patch that adds a check to test for multicast/ broadcast routeisa better fix. Did you try that patch? I tried it in my setup using both vxlan/DOVE configuration between VMs in the same networknamespace on 2 different bridges as well as the veth configuration between 2 network namespsaces. Could you try his patch in your configuration. I think it will work and if so we should go withthat patch. Thanks Sridhar -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 9a64715..d6509de 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1013,7 +1013,7 @@ static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, } /* Bypass encapsulation if the destination is local */ - if (rt->rt_flags & RTCF_LOCAL) { + if (rt->dst.dev->flags & IFF_LOOPBACK) { struct vxlan_dev *dst_vxlan; ip_rt_put(rt);