pull request (net): ipsec 2016-03-10

pull request (net): ipsec 2016-03-10

[Steffen Klassert]

1) Add pmtu handling to vti, we need it do report
   pmtu informations for local generated packets.

2) The flowcache can hit an OOM condition if too
   many entries are in the gc_list. Fix this by
   counting the entries in the gc_list and refuse
   new allocations if the value is too high.

3) The inner headers are invalid after a xfrm transformation,
   so reset the skb encapsulation field ensure nobody tries
   access the inner headers. Otherwise tunnel divices stacked
   on top of xfrm may build the outer headers based on wrong
   informations.

4) Fix recource leeks for vti4/vti6 introduced with the
   pmtu handling.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit aac8d3c282e024c344c5b86dc1eab7af88bb9716:

  qmi_wwan: add "4G LTE usb-modem U901" (2016-02-16 20:39:32 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 52717aa430949249993f1f7d570f70be863c2652:

  vti: Fix recource leeks on pmtu discovery (2016-03-03 07:45:09 +0100)

----------------------------------------------------------------
Steffen Klassert (5):
      vti: Add pmtu handling to vti_xmit.
      flowcache: Avoid OOM condition under preasure
      xfrm: Reset encapsulation field of the skb before transformation
      vti6: Fix dst_entry leek on pmtu discovery
      vti: Fix recource leeks on pmtu discovery

 include/net/netns/xfrm.h |  1 +
 net/core/flow.c          | 14 +++++++++++++-
 net/ipv4/ip_vti.c        | 14 ++++++++++++++
 net/ipv6/ip6_vti.c       |  3 ++-
 net/xfrm/xfrm_output.c   |  3 +++
 5 files changed, 33 insertions(+), 2 deletions(-)

[PATCH 1/5] vti: Add pmtu handling to vti_xmit.

[Steffen Klassert]

We currently rely on the PMTU discovery of xfrm.
However if a packet is localy sent, the PMTU mechanism
of xfrm tries to to local socket notification what
might not work for applications like ping that don't
check for this. So add pmtu handling to vti_xmit to
report MTU changes immediately.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_vti.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 5cf10b7..6862305 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -156,6 +156,7 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
 	struct dst_entry *dst = skb_dst(skb);
 	struct net_device *tdev;	/* Device to other host */
 	int err;
+	int mtu;
 
 	if (!dst) {
 		dev->stats.tx_carrier_errors++;
@@ -196,6 +197,18 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
 	skb_dst_set(skb, dst);
 	skb->dev = skb_dst(skb)->dev;
 
+	mtu = dst_mtu(dst);
+	if (!skb->ignore_df && skb->len > mtu) {
+		skb_dst(skb)->ops->update_pmtu(dst, NULL, skb, mtu);
+		if (skb->protocol == htons(ETH_P_IP))
+			icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+				  htonl(mtu));
+		else
+			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+
+		return -EMSGSIZE;
+	}
+
 	err = dst_output(tunnel->net, skb->sk, skb);
 	if (net_xmit_eval(err) == 0)
 		err = skb->len;
-- 
1.9.1

[PATCH 2/5] flowcache: Avoid OOM condition under preasure

[Steffen Klassert]

We can hit an OOM condition if we are under presure because
we can not free the entries in gc_list fast enough. So add
a counter for the not yet freed entries in the gc_list and
refuse new allocations if the value is too high.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/netns/xfrm.h |  1 +
 net/core/flow.c          | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 730d82a..24cd394 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -80,6 +80,7 @@ struct netns_xfrm {
 	struct flow_cache	flow_cache_global;
 	atomic_t		flow_cache_genid;
 	struct list_head	flow_cache_gc_list;
+	atomic_t		flow_cache_gc_count;
 	spinlock_t		flow_cache_gc_lock;
 	struct work_struct	flow_cache_gc_work;
 	struct work_struct	flow_cache_flush_work;
diff --git a/net/core/flow.c b/net/core/flow.c
index 1033725..3937b1b 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -92,8 +92,11 @@ static void flow_cache_gc_task(struct work_struct *work)
 	list_splice_tail_init(&xfrm->flow_cache_gc_list, &gc_list);
 	spin_unlock_bh(&xfrm->flow_cache_gc_lock);
 
-	list_for_each_entry_safe(fce, n, &gc_list, u.gc_list)
+	list_for_each_entry_safe(fce, n, &gc_list, u.gc_list) {
 		flow_entry_kill(fce, xfrm);
+		atomic_dec(&xfrm->flow_cache_gc_count);
+		WARN_ON(atomic_read(&xfrm->flow_cache_gc_count) < 0);
+	}
 }
 
 static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
@@ -101,6 +104,7 @@ static void flow_cache_queue_garbage(struct flow_cache_percpu *fcp,
 				     struct netns_xfrm *xfrm)
 {
 	if (deleted) {
+		atomic_add(deleted, &xfrm->flow_cache_gc_count);
 		fcp->hash_count -= deleted;
 		spin_lock_bh(&xfrm->flow_cache_gc_lock);
 		list_splice_tail(gc_list, &xfrm->flow_cache_gc_list);
@@ -232,6 +236,13 @@ flow_cache_lookup(struct net *net, const struct flowi *key, u16 family, u8 dir,
 		if (fcp->hash_count > fc->high_watermark)
 			flow_cache_shrink(fc, fcp);
 
+		if (fcp->hash_count > 2 * fc->high_watermark ||
+		    atomic_read(&net->xfrm.flow_cache_gc_count) > fc->high_watermark) {
+			atomic_inc(&net->xfrm.flow_cache_genid);
+			flo = ERR_PTR(-ENOBUFS);
+			goto ret_object;
+		}
+
 		fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC);
 		if (fle) {
 			fle->net = net;
@@ -446,6 +457,7 @@ int flow_cache_init(struct net *net)
 	INIT_WORK(&net->xfrm.flow_cache_gc_work, flow_cache_gc_task);
 	INIT_WORK(&net->xfrm.flow_cache_flush_work, flow_cache_flush_task);
 	mutex_init(&net->xfrm.flow_flush_sem);
+	atomic_set(&net->xfrm.flow_cache_gc_count, 0);
 
 	fc->hash_shift = 10;
 	fc->low_watermark = 2 * flow_cache_hash_size(fc);
-- 
1.9.1

[PATCH 3/5] xfrm: Reset encapsulation field of the skb before transformation

[Steffen Klassert]

The inner headers are invalid after a xfrm transformation.
So reset the skb encapsulation field to ensure nobody tries
to access the inner headers.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_output.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index ff4a91f..637387b 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -99,6 +99,9 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 
 		skb_dst_force(skb);
 
+		/* Inner headers are invalid now. */
+		skb->encapsulation = 0;
+
 		err = x->type->output(x, skb);
 		if (err == -EINPROGRESS)
 			goto out;
-- 
1.9.1

[PATCH 4/5] vti6: Fix dst_entry leek on pmtu discovery

[Steffen Klassert]

We may exit without releasing the dst_entry on pmtu discovery,
so don't return directly but goto the te error handling. This
also makes sure that the statistic counter gets updated.

Fixes: ccd740cbc6e0 ("vti6: Add pmtu handling to vti6_xmit.")
Reported-by: Mark McKinstry <Mark.McKinstry@alliedtelesis.co.nz>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/ip6_vti.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 0a8610b..555ac2b 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -479,7 +479,8 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
 			icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
 				  htonl(mtu));
 
-		return -EMSGSIZE;
+		err = -EMSGSIZE;
+		goto tx_err_dst_release;
 	}
 
 	err = dst_output(t->net, skb->sk, skb);
-- 
1.9.1

[PATCH 5/5] vti: Fix recource leeks on pmtu discovery

[Steffen Klassert]

A recent patch introduced pmtu handling directly in the
vti transmit routine. Unfortunately we now return without
releasing the dst_entry and freeing the sk_buff. This patch
fixes the issue.

Fixes: 325b71fe0f57 ("vti: Add pmtu handling to vti_xmit.")
Reported-by: Mark McKinstry <Mark.McKinstry@alliedtelesis.co.nz>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_vti.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 6862305..2ea2b6e 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -206,7 +206,8 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
 		else
 			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 
-		return -EMSGSIZE;
+		dst_release(dst);
+		goto tx_error;
 	}
 
 	err = dst_output(tunnel->net, skb->sk, skb);
-- 
1.9.1

Re: [PATCH 4/5] vti6: Fix dst_entry leek on pmtu discovery

[Sergei Shtylyov]

Hello.

    s/leek/leak/ in the subject.

On 3/10/2016 1:24 PM, Steffen Klassert wrote:
> We may exit without releasing the dst_entry on pmtu discovery,
> so don't return directly but goto the te error handling. This

    Te?

> also makes sure that the statistic counter gets updated.
>
> Fixes: ccd740cbc6e0 ("vti6: Add pmtu handling to vti6_xmit.")
> Reported-by: Mark McKinstry <Mark.McKinstry@alliedtelesis.co.nz>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
[...]

MBR, Sergei

Re: [PATCH 5/5] vti: Fix recource leeks on pmtu discovery

[Sergei Shtylyov]

    s/leeks/leaks/.

Re: pull request (net): ipsec 2016-03-10

[David Miller]

Patches 4 and 5 were given some feedback, please fix those issues up and
respin this pull request.

Thank you!

Re: pull request (net): ipsec 2016-03-10

[Steffen Klassert]

On Mon, Mar 14, 2016 at 03:21:29PM -0400, David Miller wrote:
> 
> Patches 4 and 5 were given some feedback, please fix those issues up and
> respin this pull request.

I did a respin on my testing branch already, but new pmtu
problems popped up. So I want to hold off the new pull
request until everything is fixed.