diff mbox

[net-next] vxlan: revert "vxlan: Bypass encapsulation if the destination is local"

Message ID 1365530913.29336.50.camel@oc1677441337.ibm.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Sridhar Samudrala April 9, 2013, 6:08 p.m. UTC
On Tue, 2013-04-09 at 17:57 +0800, Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> This reverts commit 9dcc71e1fdbb7aa10d92a3d35e8a201adc84abd0.
> It apparently breaks my vxlan tests between different namespaces.
> 
I haven't tried vxlan with network namespaces.
This patch effects the following 2 code paths
- when source and destination endpoints are on the same bridge and
  route short-circuiting is enabled. I guess you are not hitting
  this path as this is possible only if you specify 'rsc' flag when
  creating vxlan device.
- when source and destination endpoints belonging to different vni's
  are on 2 different bridges on the same host. encap bypass is done
  in this scenario by checking if rt_flags has RTCF_LOCAL set. I think
  you must be hitting this path and the following patch should fix
  it by only doing bypass if the source and dest devices belong to 
  the same net. Can you try it and see if it fixes your tests?


  
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

Comments

Sergei Shtylyov April 10, 2013, 2:29 p.m. UTC | #1
Hello.

On 09-04-2013 22:08, Sridhar Samudrala wrote:

>> From: Cong Wang <amwang@redhat.com>

>> This reverts commit 9dcc71e1fdbb7aa10d92a3d35e8a201adc84abd0.
>> It apparently breaks my vxlan tests between different namespaces.

> I haven't tried vxlan with network namespaces.
> This patch effects the following 2 code paths
> - when source and destination endpoints are on the same bridge and
>    route short-circuiting is enabled. I guess you are not hitting
>    this path as this is possible only if you specify 'rsc' flag when
>    creating vxlan device.
> - when source and destination endpoints belonging to different vni's
>    are on 2 different bridges on the same host. encap bypass is done
>    in this scenario by checking if rt_flags has RTCF_LOCAL set. I think
>    you must be hitting this path and the following patch should fix
>    it by only doing bypass if the source and dest devices belong to
>    the same net. Can you try it and see if it fixes your tests?

> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 9a64715..d53d8cb 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1012,12 +1012,15 @@ static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>   		goto tx_error;
>   	}
>
> -	/* Bypass encapsulation if the destination is local */
> -	if (rt->rt_flags & RTCF_LOCAL) {
> +	/* Bypass encapsulation if the destination is local and in the same
> +	   network namespace.
> +	 */

    Note that the preferred multi-line comment style in the networking code is:

/* bla
  * bla
  */

WBR, Sergei

--
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
Amerigo Wang April 11, 2013, 2:10 a.m. UTC | #2
On Tue, 2013-04-09 at 11:08 -0700, Sridhar Samudrala wrote:
> On Tue, 2013-04-09 at 17:57 +0800, Cong Wang wrote:
> > From: Cong Wang <amwang@redhat.com>
> > 
> > This reverts commit 9dcc71e1fdbb7aa10d92a3d35e8a201adc84abd0.
> > It apparently breaks my vxlan tests between different namespaces.
> > 
> I haven't tried vxlan with network namespaces.
> This patch effects the following 2 code paths
> - when source and destination endpoints are on the same bridge and
>   route short-circuiting is enabled. I guess you are not hitting
>   this path as this is possible only if you specify 'rsc' flag when
>   creating vxlan device.

No, I didn't specify this flag.

> - when source and destination endpoints belonging to different vni's
>   are on 2 different bridges on the same host. encap bypass is done
>   in this scenario by checking if rt_flags has RTCF_LOCAL set. I think
>   you must be hitting this path and the following patch should fix
>   it by only doing bypass if the source and dest devices belong to 
>   the same net. Can you try it and see if it fixes your tests?

I just tested it, unfortunately it doesn't work, the bug still exists.

If you need any other info, please let me know.

Thanks.

--
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
Sridhar Samudrala April 11, 2013, 4:53 a.m. UTC | #3
On 4/10/2013 7:10 PM, Cong Wang wrote:
>> - when source and destination endpoints belonging to different vni's
>>    are on 2 different bridges on the same host. encap bypass is done
>>    in this scenario by checking if rt_flags has RTCF_LOCAL set. I think
>>    you must be hitting this path and the following patch should fix
>>    it by only doing bypass if the source and dest devices belong to
>>    the same net. Can you try it and see if it fixes your tests?
> I just tested it, unfortunately it doesn't work, the bug still exists.
>
> If you need any other info, please let me know.
So does it mean that you are hitting the if condition that does encap 
bypass
even afterthe net_eq() check? Do the tests pass If you comment out the 
'if' block?

Can you share your test config/scripts so that i can try out your setup if
it is not toocomplicated?

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 mbox

Patch

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 9a64715..d53d8cb 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1012,12 +1012,15 @@  static netdev_tx_t vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		goto tx_error;
 	}
 
-	/* Bypass encapsulation if the destination is local */
-	if (rt->rt_flags & RTCF_LOCAL) {
+	/* Bypass encapsulation if the destination is local and in the same
+	   network namespace.
+	 */
+	if (net_eq(dev_net(dev), dev_net(rt->dst.dev)) &&
+	     rt->rt_flags & RTCF_LOCAL) {
 		struct vxlan_dev *dst_vxlan;
 
+		dst_vxlan = vxlan_find_vni(dev_net(rt->dst.dev), vni);
 		ip_rt_put(rt);
-		dst_vxlan = vxlan_find_vni(dev_net(dev), vni);
 		if (!dst_vxlan)
 			goto tx_error;
 		vxlan_encap_bypass(skb, vxlan, dst_vxlan);