diff mbox

[net] vxlan: do not use fdb in metadata mode

Message ID 6003cfff14ce8e36bfbe4929fce56d06e4ebec58.1455657466.git.jbenc@redhat.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jiri Benc Feb. 16, 2016, 9:18 p.m. UTC
In metadata mode, the vxlan interface is not supposed to use the fdb control
plane but an external one (openvswitch or static routes). With the current
code, packets may leak into the fdb handling code which usually causes them
to be dropped anyway but may have strange side effects.

Just drop the packets directly when in metadata mode if the destination data
are not correctly provided on egress.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 drivers/net/vxlan.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Simon Horman Feb. 18, 2016, 6:32 a.m. UTC | #1
Hi Jiri,

On Tue, Feb 16, 2016 at 10:18:26PM +0100, Jiri Benc wrote:
> In metadata mode, the vxlan interface is not supposed to use the fdb control
> plane but an external one (openvswitch or static routes). With the current
> code, packets may leak into the fdb handling code which usually causes them
> to be dropped anyway but may have strange side effects.
> 
> Just drop the packets directly when in metadata mode if the destination data
> are not correctly provided on egress.
> 
> Signed-off-by: Jiri Benc <jbenc@redhat.com>

The logic here looks correct to me but I am curious to know
what circumstances would lead to the kfree_skb() case.

> ---
>  drivers/net/vxlan.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index db96f3a16f6c..e6944b29588e 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2171,9 +2171,11 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
>  #endif
>  	}
>  
> -	if (vxlan->flags & VXLAN_F_COLLECT_METADATA &&
> -	    info && info->mode & IP_TUNNEL_INFO_TX) {
> -		vxlan_xmit_one(skb, dev, NULL, false);
> +	if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
> +		if (info && info->mode & IP_TUNNEL_INFO_TX)
> +			vxlan_xmit_one(skb, dev, NULL, false);
> +		else
> +			kfree_skb(skb);
>  		return NETDEV_TX_OK;
>  	}
>  
> -- 
> 1.8.3.1
>
Jiri Benc Feb. 18, 2016, 9:11 a.m. UTC | #2
On Thu, 18 Feb 2016 15:32:40 +0900, Simon Horman wrote:
> The logic here looks correct to me but I am curious to know
> what circumstances would lead to the kfree_skb() case.

Setting up the interface in "external" (VXLAN_F_COLLECT_METADATA) mode,
assigning an IP address and not setting a route with the "encap"
parameter.

 Jiri
David Miller Feb. 18, 2016, 8:01 p.m. UTC | #3
From: Jiri Benc <jbenc@redhat.com>
Date: Tue, 16 Feb 2016 22:18:26 +0100

> In metadata mode, the vxlan interface is not supposed to use the fdb control
> plane but an external one (openvswitch or static routes). With the current
> code, packets may leak into the fdb handling code which usually causes them
> to be dropped anyway but may have strange side effects.
> 
> Just drop the packets directly when in metadata mode if the destination data
> are not correctly provided on egress.
> 
> Signed-off-by: Jiri Benc <jbenc@redhat.com>

Applied, thanks.
diff mbox

Patch

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index db96f3a16f6c..e6944b29588e 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2171,9 +2171,11 @@  static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 #endif
 	}
 
-	if (vxlan->flags & VXLAN_F_COLLECT_METADATA &&
-	    info && info->mode & IP_TUNNEL_INFO_TX) {
-		vxlan_xmit_one(skb, dev, NULL, false);
+	if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
+		if (info && info->mode & IP_TUNNEL_INFO_TX)
+			vxlan_xmit_one(skb, dev, NULL, false);
+		else
+			kfree_skb(skb);
 		return NETDEV_TX_OK;
 	}