[PATCH net v2 02/03] vxlan: handle skb_clone failure

[PATCH net v2 02/03] vxlan: handle skb_clone failure

[Stephen Hemminger]

skb_clone can fail if out of memory. Just skip the fanout.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

---
The multi-destination was added in 3.10, does not need to goto stable

--- a/drivers/net/vxlan.c	2013-06-06 08:29:07.890429399 -0700
+++ b/drivers/net/vxlan.c	2013-06-06 09:38:52.723669153 -0700
@@ -1198,9 +1198,11 @@ static netdev_tx_t vxlan_xmit(struct sk_
 		struct sk_buff *skb1;
 
 		skb1 = skb_clone(skb, GFP_ATOMIC);
-		rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc);
-		if (rc == NETDEV_TX_OK)
-			rc = rc1;
+		if (skb1) {
+			rc1 = vxlan_xmit_one(skb1, dev, rdst, did_rsc);
+			if (rc == NETDEV_TX_OK)
+				rc = rc1;
+		}
 	}
 
 	rc1 = vxlan_xmit_one(skb, dev, rdst0, did_rsc);

Re: [PATCH net v2 02/03] vxlan: handle skb_clone failure

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 10 Jun 2013 13:12:30 -0700

> skb_clone can fail if out of memory. Just skip the fanout.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Applied.

[PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[Stephen Hemminger]

If vxlan is removed with active vxlan's it would crash because
rtnl_link_unregister (which calls vxlan_dellink), was invoked
before unregister_pernet_device (which calls vxlan_stop).

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Should goto stable

--- a/drivers/net/vxlan.c	2013-06-06 08:29:07.910429205 -0700
+++ b/drivers/net/vxlan.c	2013-06-06 09:38:52.091675246 -0700
@@ -1785,8 +1785,9 @@ late_initcall(vxlan_init_module);
 
 static void __exit vxlan_cleanup_module(void)
 {
-	rtnl_link_unregister(&vxlan_link_ops);
 	unregister_pernet_device(&vxlan_net_ops);
+	rtnl_link_unregister(&vxlan_link_ops);
+	destroy_workqueue(vxlan_wq);
 	rcu_barrier();
 }
 module_exit(vxlan_cleanup_module);

Re: [PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 10 Jun 2013 13:13:15 -0700

> If vxlan is removed with active vxlan's it would crash because
> rtnl_link_unregister (which calls vxlan_dellink), was invoked
> before unregister_pernet_device (which calls vxlan_stop).
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> Should goto stable

Applied and queued up for -stable.

Thanks.

Re: [PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[David Miller]

From: David Miller <davem@redhat.com>
Date: Mon, 10 Jun 2013 14:01:14 -0700 (PDT)

> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Mon, 10 Jun 2013 13:13:15 -0700
> 
>> If vxlan is removed with active vxlan's it would crash because
>> rtnl_link_unregister (which calls vxlan_dellink), was invoked
>> before unregister_pernet_device (which calls vxlan_stop).
>> 
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>> Should goto stable
> 
> Applied and queued up for -stable.

Actually, this doesn't even compile.

+	destroy_workqueue(vxlan_wq);

vxlan_wq doesn't exist in the 'net' tree.

It doesn't exist in net-next either.

It gets added as part of your vxlan-next series which you posted at
the same time.

I'm tossing both vxlan patch sets, you'll need to resubmit them
properly.

I'm tossing both becuase I can tell the -next one won't apply properly
on top of this series up when you fix it up.

Re: [PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 10 Jun 2013 14:14:21 -0700 (PDT)

> Actually, this doesn't even compile.

One last thing, no compile testing means no functional testing was
done on these patches in the environment in which they were meant to
be applied.

Re: [PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[Stephen Hemminger]

The issue was trying to split them into net and net-next series.
If I sent one series, would you be able to handle that?

Re: [PATCH net v2 03/03] [PATCH] vxlan: fix crash on module removal

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 10 Jun 2013 14:48:14 -0700

> The issue was trying to split them into net and net-next series.
> If I sent one series, would you be able to handle that?

Everyone who wants to do this, does it like this:

1) You give me the "net" series with the bug fixes.

2) You give me the "net-next" series, and you tell me "this series
   depends upon the bug fixes going into 'net' from series X, please
   merge that into 'net-next' before applying this series.

And if you're really testing this stuff at all, that's how you're
developing and testing it too.  You put together the net patch set,
you _build_ it and you _test_ it.  Then you make a branch on
net-next, pull your 'net' stuff into it, and the build your 'net-next'
series on that branch.  Then you _build_ it and you _test_ it before
even thinking about posting it here.

[PATCH net v2 01/03] vxlan: only migrate dynamic FDB entries

[Stephen Hemminger]

Only migrate dynamic forwarding table entries, don't modify
static entries. If packet received from incorrect source IP address
assume it is an imposter and drop it.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

---
Should go to -stable as well.

--- a/drivers/net/vxlan.c	2013-06-06 08:27:07.499595174 -0700
+++ b/drivers/net/vxlan.c	2013-06-06 09:38:54.291654035 -0700
@@ -603,9 +603,10 @@ skip:
 
 /* Watch incoming packets to learn mapping between Ethernet address
  * and Tunnel endpoint.
+ * Return true if packet is bogus and should be droppped.
  */
-static void vxlan_snoop(struct net_device *dev,
-			__be32 src_ip, const u8 *src_mac)
+static bool vxlan_snoop(struct net_device *dev,
+		       __be32 src_ip, const u8 *src_mac)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	struct vxlan_fdb *f;
@@ -614,7 +615,11 @@ static void vxlan_snoop(struct net_devic
 	f = vxlan_find_mac(vxlan, src_mac);
 	if (likely(f)) {
 		if (likely(f->remote.remote_ip == src_ip))
-			return;
+			return false;
+
+		/* Don't migrate static entries, drop packets */
+		if (!(f->flags & NTF_SELF))
+			return true;
 
 		if (net_ratelimit())
 			netdev_info(dev,
@@ -634,6 +639,8 @@ static void vxlan_snoop(struct net_devic
 				       0, NTF_SELF);
 		spin_unlock(&vxlan->hash_lock);
 	}
+
+	return false;
 }
 
 
@@ -766,8 +773,9 @@ static int vxlan_udp_encap_recv(struct s
 			       vxlan->dev->dev_addr) == 0)
 		goto drop;
 
-	if (vxlan->flags & VXLAN_F_LEARN)
-		vxlan_snoop(skb->dev, oip->saddr, eth_hdr(skb)->h_source);
+	if ((vxlan->flags & VXLAN_F_LEARN) &&
+	    vxlan_snoop(skb->dev, oip->saddr, eth_hdr(skb)->h_source))
+		goto drop;
 
 	__skb_tunnel_rx(skb, vxlan->dev);
 	skb_reset_network_header(skb);

Re: [PATCH net v2 01/03] vxlan: only migrate dynamic FDB entries

[Sergei Shtylyov]

Hello.

On 06/11/2013 12:13 AM, Stephen Hemminger wrote:

> Only migrate dynamic forwarding table entries, don't modify
> static entries. If packet received from incorrect source IP address
> assume it is an imposter and drop it.

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

> ---
> Should go to -stable as well.

> --- a/drivers/net/vxlan.c	2013-06-06 08:27:07.499595174 -0700
> +++ b/drivers/net/vxlan.c	2013-06-06 09:38:54.291654035 -0700
> @@ -603,9 +603,10 @@ skip:
>
>   /* Watch incoming packets to learn mapping between Ethernet address
>    * and Tunnel endpoint.
> + * Return true if packet is bogus and should be droppped.
>    */
> -static void vxlan_snoop(struct net_device *dev,
> -			__be32 src_ip, const u8 *src_mac)
> +static bool vxlan_snoop(struct net_device *dev,
> +		       __be32 src_ip, const u8 *src_mac)

    Not clear why you changed the indentation of the second line...

WBR, Sergei

Re: [PATCH net v2 01/03] vxlan: only migrate dynamic FDB entries

[Stephen Hemminger]

On Tue, 11 Jun 2013 00:27:02 +0400
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> Hello.
> 
> On 06/11/2013 12:13 AM, Stephen Hemminger wrote:
> 
> > Only migrate dynamic forwarding table entries, don't modify
> > static entries. If packet received from incorrect source IP address
> > assume it is an imposter and drop it.
> 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> > ---
> > Should go to -stable as well.
> 
> > --- a/drivers/net/vxlan.c	2013-06-06 08:27:07.499595174 -0700
> > +++ b/drivers/net/vxlan.c	2013-06-06 09:38:54.291654035 -0700
> > @@ -603,9 +603,10 @@ skip:
> >
> >   /* Watch incoming packets to learn mapping between Ethernet address
> >    * and Tunnel endpoint.
> > + * Return true if packet is bogus and should be droppped.
> >    */
> > -static void vxlan_snoop(struct net_device *dev,
> > -			__be32 src_ip, const u8 *src_mac)
> > +static bool vxlan_snoop(struct net_device *dev,
> > +		       __be32 src_ip, const u8 *src_mac)
> 
>     Not clear why you changed the indentation of the second line...
> 
> WBR, Sergei
> 

It was int, now bool, and second line didn't get reindented.
Now dave will make resend the whole series...

Re: [PATCH net v2 01/03] vxlan: only migrate dynamic FDB entries

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 10 Jun 2013 13:32:01 -0700

> Now dave will make resend the whole series...

Attention to detail generally tends to show how much the submitter
cares about the quality of their work :-)

Re: [PATCH net v2 01/03] vxlan: only migrate dynamic FDB entries

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 10 Jun 2013 13:13:17 -0700

> Only migrate dynamic forwarding table entries, don't modify
> static entries. If packet received from incorrect source IP address
> assume it is an imposter and drop it.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> ---
> Should go to -stable as well.

Applied and queued up for -stable.