Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ixp4xx_eth: set the device dma_coherent_mask
From: David Miller @ 2013-03-19 16:33 UTC (permalink / raw)
  To: aeschlimann; +Cc: khc, netdev, linux-kernel, c.aeschlimann
In-Reply-To: <1363708765-20778-1-git-send-email-c.aeschlimann@acn-group.ch>

From: Christophe Aeschlimann <aeschlimann@gmail.com>
Date: Tue, 19 Mar 2013 16:59:25 +0100

> Without the mask it is impossible to take the network interface up
> since it returns the following error:
> 
>> net eth1: coherent DMA mask is unset
>> ifconfig: SIOCSIFFLAGS: Cannot allocate memory
> 
> Tested on an out-of-tree ixp425 based board.
> 
> Signed-off-by: Christophe Aeschlimann <c.aeschlimann@acn-group.ch>
 ...
> @@ -1398,6 +1398,7 @@ static int eth_init_one(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	SET_NETDEV_DEV(dev, &pdev->dev);
> +	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);

Hmmm, shouldn't this be the default value, set by the bus layer or
similar?

^ permalink raw reply

* Re: [PATCH v6] net: sh_eth: Add support of device tree probe
From: Sergei Shtylyov @ 2013-03-19 16:29 UTC (permalink / raw)
  To: Nobuhiro Iwamatsu
  Cc: netdev, devicetree-discuss, magnus.damm, kda, horms+renesas,
	mark.rutland, grant.likely
In-Reply-To: <51486699.6000908@cogentembedded.com>

Hello.

On 19-03-2013 17:22, Sergei Shtylyov wrote:

>> This adds support of device tree probe for Renesas sh-ether driver.

>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>

>> V6: - Add renesas,sh-eth-gigabit, renesas,sh-eth-sh4 and
>>        renesas,sh-eth-sh3-sh2 to compatible string.
>>     - Remove sh-eth,register-type. This is supplemented by the
>>        compatible string.
>>      - Use the of_property_read_bool instead of of_find_property.
>>      - Add sanity chheck for of_property_read_u32.
>>      - Update document.
>> V5: - Rewrite sh_eth_parse_dt().
>>        Remove of_device_is_available() and CONFIG_OF from support OF
>>        checking function and re-add empty sh_eth_parse_dt().
>>      - Add CONFIG_PM to definition of dev_pm_ops.
>>     - Add CONFIG_OF to definition of of_device_id.
>> V4: - Remove empty sh_eth_parse_dt().
>> V3: - Remove empty sh_eth_parse_dt().
>> V3: - Removed sentnece of "needs-init" from document.
>> V2: - Removed ether_setup().
>>      - Fixed typo from "sh-etn" to "sh-eth".
>>     - Removed "needs-init" and sh-eth,endian from definition of DT.
>>     - Changed "sh-eth,edmac-endian" instead of "sh-eth,edmac-big-endain"
>>       in definition of DT.

    The version history is usually "hidded" under -- tear line.

>> ---
>>   Documentation/devicetree/bindings/net/sh_ether.txt |   48 +++++++
>>   drivers/net/ethernet/renesas/sh_eth.c              |  145
>> ++++++++++++++++----
>>   2 files changed, 168 insertions(+), 25 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/net/sh_ether.txt

[...]

>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 7a6471d..d9df68e 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
[...]
>> @@ -2391,12 +2451,33 @@ static int sh_eth_drv_probe(struct platform_device
>> *pdev)
>>           goto out_release;
>>       }
>>
>> +    if (np) {
>> +        pd = sh_eth_parse_dt(&pdev->dev, ndev, np);
>> +        if (pdev->dev.platform_data && pd) {
>> +            struct sh_eth_plat_data *tmp =
>> +                pdev->dev.platform_data;
>> +            pd->set_mdio_gate = tmp->set_mdio_gate;
>> +            pd->needs_init = tmp->needs_init;

>     OK, so we can't fully convert this driver to the device tree due to
> procedural platform data. I then would advice just using OF_DEV_AUXDATA() in
> the platform data instead of trying to convert the driver to device tree.

    I meant to type "ïnstead of trying to convert the paltform data to device 
tree".

>> @@ -2515,17 +2595,32 @@ static int sh_eth_runtime_nop(struct device *dev)
[...]
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id sh_eth_match[] = {
>> +    { .compatible = "renesas,sh-eth-gigabit", },
>> +    { .compatible = "renesas,sh-eth-sh4", },
>> +    { .compatible = "renesas,sh-eth-sh3-sh2", },

>     Biut this is not really enough: the driver supports much more variations
> of the SH and ARM SoCs all of which have difference not only in register
> layout but also in the registers bits or even presence of the whole register
> blocks. All this IMO should be reflected in the different values of the
> compatible "property". BTW, it seems another register layout and instance

    I forgot to type istance of what: 'sh_eth_my_cpu_data'.

> needs to be added for the R-Car SoCs (instead of the current ugly hack).

WBR, Sergei

^ permalink raw reply

* [PATCH 12/12] l2tp: unhash l2tp sessions on delete, not on free
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

If we postpone unhashing of l2tp sessions until the structure is freed, we
risk:

 1. further packets arriving and getting queued while the pseudowire is being
    closed down
 2. the recv path hitting "scheduling while atomic" errors in the case that
    recv drops the last reference to a session and calls l2tp_session_free
    while in atomic context

As such, l2tp sessions should be unhashed from l2tp_core data structures early
in the teardown process prior to calling pseudowire close.  For pseudowires
like l2tp_ppp which have multiple shutdown codepaths, provide an unhash hook.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |   75 +++++++++++++++++++++++---------------------------
 net/l2tp/l2tp_core.h |    1 +
 net/l2tp/l2tp_ppp.c  |   12 ++------
 3 files changed, 38 insertions(+), 50 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 97d30ac..8aecf5d 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1316,26 +1316,12 @@ again:
 
 			hlist_del_init(&session->hlist);
 
-			/* Since we should hold the sock lock while
-			 * doing any unbinding, we need to release the
-			 * lock we're holding before taking that lock.
-			 * Hold a reference to the sock so it doesn't
-			 * disappear as we're jumping between locks.
-			 */
 			if (session->ref != NULL)
 				(*session->ref)(session);
 
 			write_unlock_bh(&tunnel->hlist_lock);
 
-			if (tunnel->version != L2TP_HDR_VER_2) {
-				struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
-
-				spin_lock_bh(&pn->l2tp_session_hlist_lock);
-				hlist_del_init_rcu(&session->global_hlist);
-				spin_unlock_bh(&pn->l2tp_session_hlist_lock);
-				synchronize_rcu();
-			}
-
+			__l2tp_session_unhash(session);
 			l2tp_session_queue_purge(session);
 
 			if (session->session_close != NULL)
@@ -1732,64 +1718,71 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
  */
 void l2tp_session_free(struct l2tp_session *session)
 {
-	struct l2tp_tunnel *tunnel;
+	struct l2tp_tunnel *tunnel = session->tunnel;
 
 	BUG_ON(atomic_read(&session->ref_count) != 0);
 
-	tunnel = session->tunnel;
-	if (tunnel != NULL) {
+	if (tunnel) {
 		BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC);
+		if (session->session_id != 0)
+			atomic_dec(&l2tp_session_count);
+		sock_put(tunnel->sock);
+		session->tunnel = NULL;
+		l2tp_tunnel_dec_refcount(tunnel);
+	}
+
+	kfree(session);
+
+	return;
+}
+EXPORT_SYMBOL_GPL(l2tp_session_free);
+
+/* Remove an l2tp session from l2tp_core's hash lists.
+ * Provides a tidyup interface for pseudowire code which can't just route all
+ * shutdown via. l2tp_session_delete and a pseudowire-specific session_close
+ * callback.
+ */
+void __l2tp_session_unhash(struct l2tp_session *session)
+{
+	struct l2tp_tunnel *tunnel = session->tunnel;
 
-		/* Delete the session from the hash */
+	/* Remove the session from core hashes */
+	if (tunnel) {
+		/* Remove from the per-tunnel hash */
 		write_lock_bh(&tunnel->hlist_lock);
 		hlist_del_init(&session->hlist);
 		write_unlock_bh(&tunnel->hlist_lock);
 
-		/* Unlink from the global hash if not L2TPv2 */
+		/* For L2TPv3 we have a per-net hash: remove from there, too */
 		if (tunnel->version != L2TP_HDR_VER_2) {
 			struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
-
 			spin_lock_bh(&pn->l2tp_session_hlist_lock);
 			hlist_del_init_rcu(&session->global_hlist);
 			spin_unlock_bh(&pn->l2tp_session_hlist_lock);
 			synchronize_rcu();
 		}
-
-		if (session->session_id != 0)
-			atomic_dec(&l2tp_session_count);
-
-		sock_put(tunnel->sock);
-
-		/* This will delete the tunnel context if this
-		 * is the last session on the tunnel.
-		 */
-		session->tunnel = NULL;
-		l2tp_tunnel_dec_refcount(tunnel);
 	}
-
-	kfree(session);
-
-	return;
 }
-EXPORT_SYMBOL_GPL(l2tp_session_free);
+EXPORT_SYMBOL_GPL(__l2tp_session_unhash);
 
 /* This function is used by the netlink SESSION_DELETE command and by
    pseudowire modules.
  */
 int l2tp_session_delete(struct l2tp_session *session)
 {
+	if (session->ref)
+		(*session->ref)(session);
+	__l2tp_session_unhash(session);
 	l2tp_session_queue_purge(session);
-
 	if (session->session_close != NULL)
 		(*session->session_close)(session);
-
+	if (session->deref)
+		(*session->ref)(session);
 	l2tp_session_dec_refcount(session);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(l2tp_session_delete);
 
-
 /* We come here whenever a session's send_seq, cookie_len or
  * l2specific_len parameters are set.
  */
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 519b013..485a490 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -242,6 +242,7 @@ extern int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_i
 extern void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
 extern int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 extern struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunnel, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg);
+extern void __l2tp_session_unhash(struct l2tp_session *session);
 extern int l2tp_session_delete(struct l2tp_session *session);
 extern void l2tp_session_free(struct l2tp_session *session);
 extern void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, unsigned char *ptr, unsigned char *optr, u16 hdrflags, int length, int (*payload_hook)(struct sk_buff *skb));
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 9d0eb8c..637a341 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -466,19 +466,12 @@ static void pppol2tp_session_close(struct l2tp_session *session)
  */
 static void pppol2tp_session_destruct(struct sock *sk)
 {
-	struct l2tp_session *session;
-
-	if (sk->sk_user_data != NULL) {
-		session = sk->sk_user_data;
-		if (session == NULL)
-			goto out;
-
+	struct l2tp_session *session = sk->sk_user_data;
+	if (session) {
 		sk->sk_user_data = NULL;
 		BUG_ON(session->magic != L2TP_SESSION_MAGIC);
 		l2tp_session_dec_refcount(session);
 	}
-
-out:
 	return;
 }
 
@@ -509,6 +502,7 @@ static int pppol2tp_release(struct socket *sock)
 
 	/* Purge any queued data */
 	if (session != NULL) {
+		__l2tp_session_unhash(session);
 		l2tp_session_queue_purge(session);
 		sock_put(sk);
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 10/12] l2tp: push all ppp pseudowire shutdown through .release handler
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

If userspace deletes a ppp pseudowire using the netlink API, either by
directly deleting the session or by deleting the tunnel that contains the
session, we need to tear down the corresponding pppox channel.

Rather than trying to manage two pppox unbind codepaths, switch the netlink
and l2tp_core session_close handlers to close via. the l2tp_ppp socket
.release handler.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_ppp.c |   53 ++++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 43 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 6a53371..7e3e16a 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -97,6 +97,7 @@
 #include <net/ip.h>
 #include <net/udp.h>
 #include <net/xfrm.h>
+#include <net/inet_common.h>
 
 #include <asm/byteorder.h>
 #include <linux/atomic.h>
@@ -447,34 +448,16 @@ static void pppol2tp_session_close(struct l2tp_session *session)
 {
 	struct pppol2tp_session *ps = l2tp_session_priv(session);
 	struct sock *sk = ps->sock;
-	struct sk_buff *skb;
+	struct socket *sock = sk->sk_socket;
 
 	BUG_ON(session->magic != L2TP_SESSION_MAGIC);
 
-	if (session->session_id == 0)
-		goto out;
-
-	if (sk != NULL) {
-		lock_sock(sk);
-
-		if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
-			pppox_unbind_sock(sk);
-			sk->sk_state = PPPOX_DEAD;
-			sk->sk_state_change(sk);
-		}
-
-		/* Purge any queued data */
-		skb_queue_purge(&sk->sk_receive_queue);
-		skb_queue_purge(&sk->sk_write_queue);
-		while ((skb = skb_dequeue(&session->reorder_q))) {
-			kfree_skb(skb);
-			sock_put(sk);
-		}
 
-		release_sock(sk);
+	if (sock) {
+		inet_shutdown(sock, 2);
+		/* Don't let the session go away before our socket does */
+		l2tp_session_inc_refcount(session);
 	}
-
-out:
 	return;
 }
 
@@ -525,16 +508,12 @@ static int pppol2tp_release(struct socket *sock)
 	session = pppol2tp_sock_to_session(sk);
 
 	/* Purge any queued data */
-	skb_queue_purge(&sk->sk_receive_queue);
-	skb_queue_purge(&sk->sk_write_queue);
 	if (session != NULL) {
-		struct sk_buff *skb;
-		while ((skb = skb_dequeue(&session->reorder_q))) {
-			kfree_skb(skb);
-			sock_put(sk);
-		}
+		l2tp_session_queue_purge(session);
 		sock_put(sk);
 	}
+	skb_queue_purge(&sk->sk_receive_queue);
+	skb_queue_purge(&sk->sk_write_queue);
 
 	release_sock(sk);
 
@@ -880,18 +859,6 @@ out:
 	return error;
 }
 
-/* Called when deleting sessions via the netlink interface.
- */
-static int pppol2tp_session_delete(struct l2tp_session *session)
-{
-	struct pppol2tp_session *ps = l2tp_session_priv(session);
-
-	if (ps->sock == NULL)
-		l2tp_session_dec_refcount(session);
-
-	return 0;
-}
-
 #endif /* CONFIG_L2TP_V3 */
 
 /* getname() support.
@@ -1839,7 +1806,7 @@ static const struct pppox_proto pppol2tp_proto = {
 
 static const struct l2tp_nl_cmd_ops pppol2tp_nl_cmd_ops = {
 	.session_create	= pppol2tp_session_create,
-	.session_delete	= pppol2tp_session_delete,
+	.session_delete	= l2tp_session_delete,
 };
 
 #endif /* CONFIG_L2TP_V3 */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 11/12] l2tp: avoid deadlock in l2tp stats update
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

l2tp's u64_stats writers were incorrectly synchronised, making it possible to
deadlock a 64bit machine running a 32bit kernel simply by sending the l2tp
code netlink commands while passing data through l2tp sessions.

Previous discussion on netdev determined that alternative solutions such as
spinlock writer synchronisation or per-cpu data would bring unjustified
overhead, given that most users interested in high volume traffic will likely
be running 64bit kernels on 64bit hardware.

As such, this patch replaces l2tp's use of u64_stats with atomic_long_t,
thereby avoiding the deadlock.

Ref:
http://marc.info/?l=linux-netdev&m=134029167910731&w=2
http://marc.info/?l=linux-netdev&m=134079868111131&w=2

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c    |   75 ++++++++++++-----------------------------------
 net/l2tp/l2tp_core.h    |   19 ++++++------
 net/l2tp/l2tp_debugfs.c |   28 +++++++++---------
 net/l2tp/l2tp_netlink.c |   72 ++++++++++++++++++---------------------------
 net/l2tp/l2tp_ppp.c     |   46 ++++++++++++++---------------
 5 files changed, 93 insertions(+), 147 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index c00f31b..97d30ac 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -374,10 +374,8 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
 	struct sk_buff *skbp;
 	struct sk_buff *tmp;
 	u32 ns = L2TP_SKB_CB(skb)->ns;
-	struct l2tp_stats *sstats;
 
 	spin_lock_bh(&session->reorder_q.lock);
-	sstats = &session->stats;
 	skb_queue_walk_safe(&session->reorder_q, skbp, tmp) {
 		if (L2TP_SKB_CB(skbp)->ns > ns) {
 			__skb_queue_before(&session->reorder_q, skbp, skb);
@@ -385,9 +383,7 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
 				 "%s: pkt %hu, inserted before %hu, reorder_q len=%d\n",
 				 session->name, ns, L2TP_SKB_CB(skbp)->ns,
 				 skb_queue_len(&session->reorder_q));
-			u64_stats_update_begin(&sstats->syncp);
-			sstats->rx_oos_packets++;
-			u64_stats_update_end(&sstats->syncp);
+			atomic_long_inc(&session->stats.rx_oos_packets);
 			goto out;
 		}
 	}
@@ -404,23 +400,16 @@ static void l2tp_recv_dequeue_skb(struct l2tp_session *session, struct sk_buff *
 {
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	int length = L2TP_SKB_CB(skb)->length;
-	struct l2tp_stats *tstats, *sstats;
 
 	/* We're about to requeue the skb, so return resources
 	 * to its current owner (a socket receive buffer).
 	 */
 	skb_orphan(skb);
 
-	tstats = &tunnel->stats;
-	u64_stats_update_begin(&tstats->syncp);
-	sstats = &session->stats;
-	u64_stats_update_begin(&sstats->syncp);
-	tstats->rx_packets++;
-	tstats->rx_bytes += length;
-	sstats->rx_packets++;
-	sstats->rx_bytes += length;
-	u64_stats_update_end(&tstats->syncp);
-	u64_stats_update_end(&sstats->syncp);
+	atomic_long_inc(&tunnel->stats.rx_packets);
+	atomic_long_add(length, &tunnel->stats.rx_bytes);
+	atomic_long_inc(&session->stats.rx_packets);
+	atomic_long_add(length, &session->stats.rx_bytes);
 
 	if (L2TP_SKB_CB(skb)->has_seq) {
 		/* Bump our Nr */
@@ -451,7 +440,6 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
 {
 	struct sk_buff *skb;
 	struct sk_buff *tmp;
-	struct l2tp_stats *sstats;
 
 	/* If the pkt at the head of the queue has the nr that we
 	 * expect to send up next, dequeue it and any other
@@ -459,13 +447,10 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
 	 */
 start:
 	spin_lock_bh(&session->reorder_q.lock);
-	sstats = &session->stats;
 	skb_queue_walk_safe(&session->reorder_q, skb, tmp) {
 		if (time_after(jiffies, L2TP_SKB_CB(skb)->expires)) {
-			u64_stats_update_begin(&sstats->syncp);
-			sstats->rx_seq_discards++;
-			sstats->rx_errors++;
-			u64_stats_update_end(&sstats->syncp);
+			atomic_long_inc(&session->stats.rx_seq_discards);
+			atomic_long_inc(&session->stats.rx_errors);
 			l2tp_dbg(session, L2TP_MSG_SEQ,
 				 "%s: oos pkt %u len %d discarded (too old), waiting for %u, reorder_q_len=%d\n",
 				 session->name, L2TP_SKB_CB(skb)->ns,
@@ -624,7 +609,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	int offset;
 	u32 ns, nr;
-	struct l2tp_stats *sstats = &session->stats;
 
 	/* The ref count is increased since we now hold a pointer to
 	 * the session. Take care to decrement the refcnt when exiting
@@ -641,9 +625,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 				  "%s: cookie mismatch (%u/%u). Discarding.\n",
 				  tunnel->name, tunnel->tunnel_id,
 				  session->session_id);
-			u64_stats_update_begin(&sstats->syncp);
-			sstats->rx_cookie_discards++;
-			u64_stats_update_end(&sstats->syncp);
+			atomic_long_inc(&session->stats.rx_cookie_discards);
 			goto discard;
 		}
 		ptr += session->peer_cookie_len;
@@ -712,9 +694,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 			l2tp_warn(session, L2TP_MSG_SEQ,
 				  "%s: recv data has no seq numbers when required. Discarding.\n",
 				  session->name);
-			u64_stats_update_begin(&sstats->syncp);
-			sstats->rx_seq_discards++;
-			u64_stats_update_end(&sstats->syncp);
+			atomic_long_inc(&session->stats.rx_seq_discards);
 			goto discard;
 		}
 
@@ -733,9 +713,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 			l2tp_warn(session, L2TP_MSG_SEQ,
 				  "%s: recv data has no seq numbers when required. Discarding.\n",
 				  session->name);
-			u64_stats_update_begin(&sstats->syncp);
-			sstats->rx_seq_discards++;
-			u64_stats_update_end(&sstats->syncp);
+			atomic_long_inc(&session->stats.rx_seq_discards);
 			goto discard;
 		}
 	}
@@ -789,9 +767,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 			 * packets
 			 */
 			if (L2TP_SKB_CB(skb)->ns != session->nr) {
-				u64_stats_update_begin(&sstats->syncp);
-				sstats->rx_seq_discards++;
-				u64_stats_update_end(&sstats->syncp);
+				atomic_long_inc(&session->stats.rx_seq_discards);
 				l2tp_dbg(session, L2TP_MSG_SEQ,
 					 "%s: oos pkt %u len %d discarded, waiting for %u, reorder_q_len=%d\n",
 					 session->name, L2TP_SKB_CB(skb)->ns,
@@ -817,9 +793,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 	return;
 
 discard:
-	u64_stats_update_begin(&sstats->syncp);
-	sstats->rx_errors++;
-	u64_stats_update_end(&sstats->syncp);
+	atomic_long_inc(&session->stats.rx_errors);
 	kfree_skb(skb);
 
 	if (session->deref)
@@ -861,7 +835,6 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 	u32 tunnel_id, session_id;
 	u16 version;
 	int length;
-	struct l2tp_stats *tstats;
 
 	if (tunnel->sock && l2tp_verify_udp_checksum(tunnel->sock, skb))
 		goto discard_bad_csum;
@@ -950,10 +923,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 discard_bad_csum:
 	LIMIT_NETDEBUG("%s: UDP: bad checksum\n", tunnel->name);
 	UDP_INC_STATS_USER(tunnel->l2tp_net, UDP_MIB_INERRORS, 0);
-	tstats = &tunnel->stats;
-	u64_stats_update_begin(&tstats->syncp);
-	tstats->rx_errors++;
-	u64_stats_update_end(&tstats->syncp);
+	atomic_long_inc(&tunnel->stats.rx_errors);
 	kfree_skb(skb);
 
 	return 0;
@@ -1080,7 +1050,6 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	unsigned int len = skb->len;
 	int error;
-	struct l2tp_stats *tstats, *sstats;
 
 	/* Debug */
 	if (session->send_seq)
@@ -1109,21 +1078,15 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
 		error = ip_queue_xmit(skb, fl);
 
 	/* Update stats */
-	tstats = &tunnel->stats;
-	u64_stats_update_begin(&tstats->syncp);
-	sstats = &session->stats;
-	u64_stats_update_begin(&sstats->syncp);
 	if (error >= 0) {
-		tstats->tx_packets++;
-		tstats->tx_bytes += len;
-		sstats->tx_packets++;
-		sstats->tx_bytes += len;
+		atomic_long_inc(&tunnel->stats.tx_packets);
+		atomic_long_add(len, &tunnel->stats.tx_bytes);
+		atomic_long_inc(&session->stats.tx_packets);
+		atomic_long_add(len, &session->stats.tx_bytes);
 	} else {
-		tstats->tx_errors++;
-		sstats->tx_errors++;
+		atomic_long_inc(&tunnel->stats.tx_errors);
+		atomic_long_inc(&session->stats.tx_errors);
 	}
-	u64_stats_update_end(&tstats->syncp);
-	u64_stats_update_end(&sstats->syncp);
 
 	return 0;
 }
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index d40713d..519b013 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -36,16 +36,15 @@ enum {
 struct sk_buff;
 
 struct l2tp_stats {
-	u64			tx_packets;
-	u64			tx_bytes;
-	u64			tx_errors;
-	u64			rx_packets;
-	u64			rx_bytes;
-	u64			rx_seq_discards;
-	u64			rx_oos_packets;
-	u64			rx_errors;
-	u64			rx_cookie_discards;
-	struct u64_stats_sync	syncp;
+	atomic_long_t		tx_packets;
+	atomic_long_t		tx_bytes;
+	atomic_long_t		tx_errors;
+	atomic_long_t		rx_packets;
+	atomic_long_t		rx_bytes;
+	atomic_long_t		rx_seq_discards;
+	atomic_long_t		rx_oos_packets;
+	atomic_long_t		rx_errors;
+	atomic_long_t		rx_cookie_discards;
 };
 
 struct l2tp_tunnel;
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index c3813bc..072d720 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -146,14 +146,14 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
 		   tunnel->sock ? atomic_read(&tunnel->sock->sk_refcnt) : 0,
 		   atomic_read(&tunnel->ref_count));
 
-	seq_printf(m, " %08x rx %llu/%llu/%llu rx %llu/%llu/%llu\n",
+	seq_printf(m, " %08x rx %ld/%ld/%ld rx %ld/%ld/%ld\n",
 		   tunnel->debug,
-		   (unsigned long long)tunnel->stats.tx_packets,
-		   (unsigned long long)tunnel->stats.tx_bytes,
-		   (unsigned long long)tunnel->stats.tx_errors,
-		   (unsigned long long)tunnel->stats.rx_packets,
-		   (unsigned long long)tunnel->stats.rx_bytes,
-		   (unsigned long long)tunnel->stats.rx_errors);
+		   atomic_long_read(&tunnel->stats.tx_packets),
+		   atomic_long_read(&tunnel->stats.tx_bytes),
+		   atomic_long_read(&tunnel->stats.tx_errors),
+		   atomic_long_read(&tunnel->stats.rx_packets),
+		   atomic_long_read(&tunnel->stats.rx_bytes),
+		   atomic_long_read(&tunnel->stats.rx_errors));
 
 	if (tunnel->show != NULL)
 		tunnel->show(m, tunnel);
@@ -203,14 +203,14 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
 		seq_printf(m, "\n");
 	}
 
-	seq_printf(m, "   %hu/%hu tx %llu/%llu/%llu rx %llu/%llu/%llu\n",
+	seq_printf(m, "   %hu/%hu tx %ld/%ld/%ld rx %ld/%ld/%ld\n",
 		   session->nr, session->ns,
-		   (unsigned long long)session->stats.tx_packets,
-		   (unsigned long long)session->stats.tx_bytes,
-		   (unsigned long long)session->stats.tx_errors,
-		   (unsigned long long)session->stats.rx_packets,
-		   (unsigned long long)session->stats.rx_bytes,
-		   (unsigned long long)session->stats.rx_errors);
+		   atomic_long_read(&session->stats.tx_packets),
+		   atomic_long_read(&session->stats.tx_bytes),
+		   atomic_long_read(&session->stats.tx_errors),
+		   atomic_long_read(&session->stats.rx_packets),
+		   atomic_long_read(&session->stats.rx_bytes),
+		   atomic_long_read(&session->stats.rx_errors));
 
 	if (session->show != NULL)
 		session->show(m, session);
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index c1bab22..0825ff2 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -246,8 +246,6 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 portid, u32 seq, int fla
 #if IS_ENABLED(CONFIG_IPV6)
 	struct ipv6_pinfo *np = NULL;
 #endif
-	struct l2tp_stats stats;
-	unsigned int start;
 
 	hdr = genlmsg_put(skb, portid, seq, &l2tp_nl_family, flags,
 			  L2TP_CMD_TUNNEL_GET);
@@ -265,28 +263,22 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 portid, u32 seq, int fla
 	if (nest == NULL)
 		goto nla_put_failure;
 
-	do {
-		start = u64_stats_fetch_begin(&tunnel->stats.syncp);
-		stats.tx_packets = tunnel->stats.tx_packets;
-		stats.tx_bytes = tunnel->stats.tx_bytes;
-		stats.tx_errors = tunnel->stats.tx_errors;
-		stats.rx_packets = tunnel->stats.rx_packets;
-		stats.rx_bytes = tunnel->stats.rx_bytes;
-		stats.rx_errors = tunnel->stats.rx_errors;
-		stats.rx_seq_discards = tunnel->stats.rx_seq_discards;
-		stats.rx_oos_packets = tunnel->stats.rx_oos_packets;
-	} while (u64_stats_fetch_retry(&tunnel->stats.syncp, start));
-
-	if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx_bytes) ||
-	    nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx_errors) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx_bytes) ||
+	if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS,
+		    atomic_long_read(&tunnel->stats.tx_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_TX_BYTES,
+		    atomic_long_read(&tunnel->stats.tx_bytes)) ||
+	    nla_put_u64(skb, L2TP_ATTR_TX_ERRORS,
+		    atomic_long_read(&tunnel->stats.tx_errors)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_PACKETS,
+		    atomic_long_read(&tunnel->stats.rx_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_BYTES,
+		    atomic_long_read(&tunnel->stats.rx_bytes)) ||
 	    nla_put_u64(skb, L2TP_ATTR_RX_SEQ_DISCARDS,
-			stats.rx_seq_discards) ||
+		    atomic_long_read(&tunnel->stats.rx_seq_discards)) ||
 	    nla_put_u64(skb, L2TP_ATTR_RX_OOS_PACKETS,
-			stats.rx_oos_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx_errors))
+		    atomic_long_read(&tunnel->stats.rx_oos_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_ERRORS,
+		    atomic_long_read(&tunnel->stats.rx_errors)))
 		goto nla_put_failure;
 	nla_nest_end(skb, nest);
 
@@ -612,8 +604,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
 	struct nlattr *nest;
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	struct sock *sk = NULL;
-	struct l2tp_stats stats;
-	unsigned int start;
 
 	sk = tunnel->sock;
 
@@ -656,28 +646,22 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
 	if (nest == NULL)
 		goto nla_put_failure;
 
-	do {
-		start = u64_stats_fetch_begin(&session->stats.syncp);
-		stats.tx_packets = session->stats.tx_packets;
-		stats.tx_bytes = session->stats.tx_bytes;
-		stats.tx_errors = session->stats.tx_errors;
-		stats.rx_packets = session->stats.rx_packets;
-		stats.rx_bytes = session->stats.rx_bytes;
-		stats.rx_errors = session->stats.rx_errors;
-		stats.rx_seq_discards = session->stats.rx_seq_discards;
-		stats.rx_oos_packets = session->stats.rx_oos_packets;
-	} while (u64_stats_fetch_retry(&session->stats.syncp, start));
-
-	if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx_bytes) ||
-	    nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx_errors) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx_bytes) ||
+	if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS,
+		atomic_long_read(&session->stats.tx_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_TX_BYTES,
+		atomic_long_read(&session->stats.tx_bytes)) ||
+	    nla_put_u64(skb, L2TP_ATTR_TX_ERRORS,
+		atomic_long_read(&session->stats.tx_errors)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_PACKETS,
+		atomic_long_read(&session->stats.rx_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_BYTES,
+		atomic_long_read(&session->stats.rx_bytes)) ||
 	    nla_put_u64(skb, L2TP_ATTR_RX_SEQ_DISCARDS,
-			stats.rx_seq_discards) ||
+		atomic_long_read(&session->stats.rx_seq_discards)) ||
 	    nla_put_u64(skb, L2TP_ATTR_RX_OOS_PACKETS,
-			stats.rx_oos_packets) ||
-	    nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx_errors))
+		atomic_long_read(&session->stats.rx_oos_packets)) ||
+	    nla_put_u64(skb, L2TP_ATTR_RX_ERRORS,
+		atomic_long_read(&session->stats.rx_errors)))
 		goto nla_put_failure;
 	nla_nest_end(skb, nest);
 
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 7e3e16a..9d0eb8c 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -260,7 +260,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
 			  session->name);
 
 		/* Not bound. Nothing we can do, so discard. */
-		session->stats.rx_errors++;
+		atomic_long_inc(&session->stats.rx_errors);
 		kfree_skb(skb);
 	}
 
@@ -992,14 +992,14 @@ end:
 static void pppol2tp_copy_stats(struct pppol2tp_ioc_stats *dest,
 				struct l2tp_stats *stats)
 {
-	dest->tx_packets = stats->tx_packets;
-	dest->tx_bytes = stats->tx_bytes;
-	dest->tx_errors = stats->tx_errors;
-	dest->rx_packets = stats->rx_packets;
-	dest->rx_bytes = stats->rx_bytes;
-	dest->rx_seq_discards = stats->rx_seq_discards;
-	dest->rx_oos_packets = stats->rx_oos_packets;
-	dest->rx_errors = stats->rx_errors;
+	dest->tx_packets = atomic_long_read(&stats->tx_packets);
+	dest->tx_bytes = atomic_long_read(&stats->tx_bytes);
+	dest->tx_errors = atomic_long_read(&stats->tx_errors);
+	dest->rx_packets = atomic_long_read(&stats->rx_packets);
+	dest->rx_bytes = atomic_long_read(&stats->rx_bytes);
+	dest->rx_seq_discards = atomic_long_read(&stats->rx_seq_discards);
+	dest->rx_oos_packets = atomic_long_read(&stats->rx_oos_packets);
+	dest->rx_errors = atomic_long_read(&stats->rx_errors);
 }
 
 /* Session ioctl helper.
@@ -1633,14 +1633,14 @@ static void pppol2tp_seq_tunnel_show(struct seq_file *m, void *v)
 		   tunnel->name,
 		   (tunnel == tunnel->sock->sk_user_data) ? 'Y' : 'N',
 		   atomic_read(&tunnel->ref_count) - 1);
-	seq_printf(m, " %08x %llu/%llu/%llu %llu/%llu/%llu\n",
+	seq_printf(m, " %08x %ld/%ld/%ld %ld/%ld/%ld\n",
 		   tunnel->debug,
-		   (unsigned long long)tunnel->stats.tx_packets,
-		   (unsigned long long)tunnel->stats.tx_bytes,
-		   (unsigned long long)tunnel->stats.tx_errors,
-		   (unsigned long long)tunnel->stats.rx_packets,
-		   (unsigned long long)tunnel->stats.rx_bytes,
-		   (unsigned long long)tunnel->stats.rx_errors);
+		   atomic_long_read(&tunnel->stats.tx_packets),
+		   atomic_long_read(&tunnel->stats.tx_bytes),
+		   atomic_long_read(&tunnel->stats.tx_errors),
+		   atomic_long_read(&tunnel->stats.rx_packets),
+		   atomic_long_read(&tunnel->stats.rx_bytes),
+		   atomic_long_read(&tunnel->stats.rx_errors));
 }
 
 static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
@@ -1675,14 +1675,14 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
 		   session->lns_mode ? "LNS" : "LAC",
 		   session->debug,
 		   jiffies_to_msecs(session->reorder_timeout));
-	seq_printf(m, "   %hu/%hu %llu/%llu/%llu %llu/%llu/%llu\n",
+	seq_printf(m, "   %hu/%hu %ld/%ld/%ld %ld/%ld/%ld\n",
 		   session->nr, session->ns,
-		   (unsigned long long)session->stats.tx_packets,
-		   (unsigned long long)session->stats.tx_bytes,
-		   (unsigned long long)session->stats.tx_errors,
-		   (unsigned long long)session->stats.rx_packets,
-		   (unsigned long long)session->stats.rx_bytes,
-		   (unsigned long long)session->stats.rx_errors);
+		   atomic_long_read(&session->stats.tx_packets),
+		   atomic_long_read(&session->stats.tx_bytes),
+		   atomic_long_read(&session->stats.tx_errors),
+		   atomic_long_read(&session->stats.rx_packets),
+		   atomic_long_read(&session->stats.rx_bytes),
+		   atomic_long_read(&session->stats.rx_errors));
 
 	if (po)
 		seq_printf(m, "   interface %s\n", ppp_dev_name(&po->chan));
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 07/12] l2tp: don't BUG_ON sk_socket being NULL
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

It is valid for an existing struct sock object to have a NULL sk_socket
pointer, so don't BUG_ON in l2tp_tunnel_del_work if that should occur.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 45373fe..e841ef2 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1412,19 +1412,21 @@ static void l2tp_tunnel_del_work(struct work_struct *work)
 		return;
 
 	sock = sk->sk_socket;
-	BUG_ON(!sock);
 
-	/* If the tunnel socket was created directly by the kernel, use the
-	 * sk_* API to release the socket now.  Otherwise go through the
-	 * inet_* layer to shut the socket down, and let userspace close it.
+	/* If the tunnel socket was created by userspace, then go through the
+	 * inet layer to shut the socket down, and let userspace close it.
+	 * Otherwise, if we created the socket directly within the kernel, use
+	 * the sk API to release it here.
 	 * In either case the tunnel resources are freed in the socket
 	 * destructor when the tunnel socket goes away.
 	 */
-	if (sock->file == NULL) {
-		kernel_sock_shutdown(sock, SHUT_RDWR);
-		sk_release_kernel(sk);
+	if (tunnel->fd >= 0) {
+		if (sock)
+			inet_shutdown(sock, 2);
 	} else {
-		inet_shutdown(sock, 2);
+		if (sock)
+			kernel_sock_shutdown(sock, SHUT_RDWR);
+		sk_release_kernel(sk);
 	}
 
 	l2tp_tunnel_sock_put(sk);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 09/12] l2tp: purge session reorder queue on delete
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

Add calls to l2tp_session_queue_purge as a part of l2tp_tunnel_closeall
and l2tp_session_delete.  Pseudowire implementations which are deleted only
via. l2tp_core l2tp_session_delete calls can dispense with their own code for
flushing the reorder queue.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 69c316d..c00f31b 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1373,6 +1373,8 @@ again:
 				synchronize_rcu();
 			}
 
+			l2tp_session_queue_purge(session);
+
 			if (session->session_close != NULL)
 				(*session->session_close)(session);
 
@@ -1813,6 +1815,8 @@ EXPORT_SYMBOL_GPL(l2tp_session_free);
  */
 int l2tp_session_delete(struct l2tp_session *session)
 {
+	l2tp_session_queue_purge(session);
+
 	if (session->session_close != NULL)
 		(*session->session_close)(session);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 08/12] l2tp: add session reorder queue purge function to core
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

If an l2tp session is deleted, it is necessary to delete skbs in-flight
on the session's reorder queue before taking it down.

Rather than having each pseudowire implementation reaching into the
l2tp_session struct to handle this itself, provide a function in l2tp_core to
purge the session queue.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |   17 +++++++++++++++++
 net/l2tp/l2tp_core.h |    1 +
 2 files changed, 18 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index e841ef2..69c316d 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -829,6 +829,23 @@ discard:
 }
 EXPORT_SYMBOL(l2tp_recv_common);
 
+/* Drop skbs from the session's reorder_q
+ */
+int l2tp_session_queue_purge(struct l2tp_session *session)
+{
+	struct sk_buff *skb = NULL;
+	BUG_ON(!session);
+	BUG_ON(session->magic != L2TP_SESSION_MAGIC);
+	while ((skb = skb_dequeue(&session->reorder_q))) {
+		atomic_long_inc(&session->stats.rx_errors);
+		kfree_skb(skb);
+		if (session->deref)
+			(*session->deref)(session);
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(l2tp_session_queue_purge);
+
 /* Internal UDP receive frame. Do the real work of receiving an L2TP data frame
  * here. The skb is not on a list when we get here.
  * Returns 0 if the packet was a data packet and was successfully passed on.
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index b0861f6..d40713d 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -246,6 +246,7 @@ extern struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunne
 extern int l2tp_session_delete(struct l2tp_session *session);
 extern void l2tp_session_free(struct l2tp_session *session);
 extern void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, unsigned char *ptr, unsigned char *optr, u16 hdrflags, int length, int (*payload_hook)(struct sk_buff *skb));
+extern int l2tp_session_queue_purge(struct l2tp_session *session);
 extern int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb);
 
 extern int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 06/12] l2tp: take a reference for kernel sockets in l2tp_tunnel_sock_lookup
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

When looking up the tunnel socket in struct l2tp_tunnel, hold a reference
whether the socket was created by the kernel or by userspace.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 0dd50c0..45373fe 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -191,6 +191,7 @@ struct sock *l2tp_tunnel_sock_lookup(struct l2tp_tunnel *tunnel)
 	} else {
 		/* Socket is owned by kernelspace */
 		sk = tunnel->sock;
+		sock_hold(sk);
 	}
 
 out:
@@ -209,6 +210,7 @@ void l2tp_tunnel_sock_put(struct sock *sk)
 		}
 		sock_put(sk);
 	}
+	sock_put(sk);
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_sock_put);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 05/12] l2tp: close sessions before initiating tunnel delete
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

When a user deletes a tunnel using netlink, all the sessions in the tunnel
should also be deleted.  Since running sessions will pin the tunnel socket
with the references they hold, have the l2tp_tunnel_delete close all sessions
in a tunnel before finally closing the tunnel socket.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 287e327..0dd50c0 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1737,6 +1737,7 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
  */
 int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
 {
+	l2tp_tunnel_closeall(tunnel);
 	return (false == queue_work(l2tp_wq, &tunnel->del_work));
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 02/12] l2tp: add udp encap socket destroy handler
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

L2TP sessions hold a reference to the tunnel socket to prevent it going away
while sessions are still active.  However, since tunnel destruction is handled
by the sock sk_destruct callback there is a catch-22: a tunnel with sessions
cannot be deleted since each session holds a reference to the tunnel socket.
If userspace closes a managed tunnel socket, or dies, the tunnel will persist
and it will be neccessary to individually delete the sessions using netlink
commands.  This is ugly.

To prevent this occuring, this patch leverages the udp encapsulation socket
destroy callback to gain early notification when the tunnel socket is closed.
This allows us to safely close the sessions running in the tunnel, dropping
the tunnel socket references in the process.  The tunnel socket is then
destroyed as normal, and the tunnel resources deallocated in sk_destruct.

While we're at it, ensure that l2tp_tunnel_closeall correctly drops session
references to allow the sessions to be deleted rather than leaking.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index d36875f..ee726a7 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1282,6 +1282,7 @@ static void l2tp_tunnel_destruct(struct sock *sk)
 		/* No longer an encapsulation socket. See net/ipv4/udp.c */
 		(udp_sk(sk))->encap_type = 0;
 		(udp_sk(sk))->encap_rcv = NULL;
+		(udp_sk(sk))->encap_destroy = NULL;
 		break;
 	case L2TP_ENCAPTYPE_IP:
 		break;
@@ -1360,6 +1361,8 @@ again:
 			if (session->deref != NULL)
 				(*session->deref)(session);
 
+			l2tp_session_dec_refcount(session);
+
 			write_lock_bh(&tunnel->hlist_lock);
 
 			/* Now restart from the beginning of this hash
@@ -1373,6 +1376,16 @@ again:
 	write_unlock_bh(&tunnel->hlist_lock);
 }
 
+/* Tunnel socket destroy hook for UDP encapsulation */
+static void l2tp_udp_encap_destroy(struct sock *sk)
+{
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+}
+
 /* Really kill the tunnel.
  * Come here only when all sessions have been cleared from the tunnel.
  */
@@ -1668,6 +1681,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 		/* Mark socket as an encapsulation socket. See net/ipv4/udp.c */
 		udp_sk(sk)->encap_type = UDP_ENCAP_L2TPINUDP;
 		udp_sk(sk)->encap_rcv = l2tp_udp_encap_recv;
+		udp_sk(sk)->encap_destroy = l2tp_udp_encap_destroy;
 #if IS_ENABLED(CONFIG_IPV6)
 		if (sk->sk_family == PF_INET6)
 			udpv6_encap_enable();
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 04/12] l2tp: close sessions in ip socket destroy callback
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

l2tp_core hooks UDP's .destroy handler to gain advance warning of a tunnel
socket being closed from userspace.  We need to do the same thing for
IP-encapsulation sockets.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_ip.c  |    6 ++++++
 net/l2tp/l2tp_ip6.c |    7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 7f41b70..571db8d 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -228,10 +228,16 @@ static void l2tp_ip_close(struct sock *sk, long timeout)
 static void l2tp_ip_destroy_sock(struct sock *sk)
 {
 	struct sk_buff *skb;
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
 
 	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
 		kfree_skb(skb);
 
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+
 	sk_refcnt_debug_dec(sk);
 }
 
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 41f2f81..c74f5a9 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -241,10 +241,17 @@ static void l2tp_ip6_close(struct sock *sk, long timeout)
 
 static void l2tp_ip6_destroy_sock(struct sock *sk)
 {
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
+
 	lock_sock(sk);
 	ip6_flush_pending_frames(sk);
 	release_sock(sk);
 
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+
 	inet6_destroy_sock(sk);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 03/12] l2tp: export l2tp_tunnel_closeall
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

l2tp_core internally uses l2tp_tunnel_closeall to close all sessions in a
tunnel when a UDP-encapsulation socket is destroyed.  We need to do something
similar for IP-encapsulation sockets.

Export l2tp_tunnel_closeall as a GPL symbol to enable l2tp_ip and l2tp_ip6 to
call it from their .destroy handlers.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 net/l2tp/l2tp_core.c |    4 ++--
 net/l2tp/l2tp_core.h |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ee726a7..287e327 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -114,7 +114,6 @@ struct l2tp_net {
 
 static void l2tp_session_set_header_len(struct l2tp_session *session, int version);
 static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);
-static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
 
 static inline struct l2tp_net *l2tp_pernet(struct net *net)
 {
@@ -1312,7 +1311,7 @@ end:
 
 /* When the tunnel is closed, all the attached sessions need to go too.
  */
-static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
+void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
 {
 	int hash;
 	struct hlist_node *walk;
@@ -1375,6 +1374,7 @@ again:
 	}
 	write_unlock_bh(&tunnel->hlist_lock);
 }
+EXPORT_SYMBOL_GPL(l2tp_tunnel_closeall);
 
 /* Tunnel socket destroy hook for UDP encapsulation */
 static void l2tp_udp_encap_destroy(struct sock *sk)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 8eb8f1d..b0861f6 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -240,6 +240,7 @@ extern struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id);
 extern struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth);
 
 extern int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp);
+extern void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
 extern int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 extern struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunnel, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg);
 extern int l2tp_session_delete(struct l2tp_session *session);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 00/12] l2tp bugfix patchset
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin

This l2tp bugfix patchset addresses a number of issues.

The first five patches in the series prevent l2tp sessions pinning an l2tp
tunnel open.  This occurs because the l2tp tunnel is torn down in the tunnel
socket destructor, but each session holds a tunnel socket reference which
prevents tunnels with sessions being deleted.  The solution I've implemented
here involves adding a .destroy hook to udp code, as discussed previously on
netdev[1].

The subsequent seven patches address futher bugs exposed by fixing the problem
above, or exposed through stress testing the implementation above.  Patch 11
(avoid deadlock in l2tp stats update) isn't directly related to tunnel/session
lifetimes, but it does prevent deadlocks on i386 kernels running on 64 bit
hardware.

This patchset has been tested on 32 and 64 bit preempt/non-preempt kernels,
using iproute2, openl2tp, and custom-made stress test code.

[1] http://comments.gmane.org/gmane.linux.network/259169

Tom Parkin (12):
  udp: add encap_destroy callback
  l2tp: add udp encap socket destroy handler
  l2tp: export l2tp_tunnel_closeall
  l2tp: close sessions in ip socket destroy callback
  l2tp: close sessions before initiating tunnel delete
  l2tp: take a reference for kernel sockets in l2tp_tunnel_sock_lookup
  l2tp: don't BUG_ON sk_socket being NULL
  l2tp: add session reorder queue purge function to core
  l2tp: purge session reorder queue on delete
  l2tp: push all ppp pseudowire shutdown through .release handler
  l2tp: avoid deadlock in l2tp stats update
  l2tp: unhash l2tp sessions on delete, not on free

 include/linux/udp.h     |    1 +
 net/ipv4/udp.c          |    7 ++
 net/ipv6/udp.c          |    8 ++
 net/l2tp/l2tp_core.c    |  206 +++++++++++++++++++++++------------------------
 net/l2tp/l2tp_core.h    |   22 ++---
 net/l2tp/l2tp_debugfs.c |   28 +++----
 net/l2tp/l2tp_ip.c      |    6 ++
 net/l2tp/l2tp_ip6.c     |    7 ++
 net/l2tp/l2tp_netlink.c |   72 +++++++----------
 net/l2tp/l2tp_ppp.c     |  111 +++++++++----------------
 10 files changed, 220 insertions(+), 248 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH 01/12] udp: add encap_destroy callback
From: Tom Parkin @ 2013-03-19 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Tom Parkin, James Chapman
In-Reply-To: <1363709483-24021-1-git-send-email-tparkin@katalix.com>

Users of udp encapsulation currently have an encap_rcv callback which they can
use to hook into the udp receive path.

In situations where a encapsulation user allocates resources associated with a
udp encap socket, it may be convenient to be able to also hook the proto
.destroy operation.  For example, if an encap user holds a reference to the
udp socket, the destroy hook might be used to relinquish this reference.

This patch adds a socket destroy hook into udp, which is set and enabled
in the same way as the existing encap_rcv hook.

Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
 include/linux/udp.h |    1 +
 net/ipv4/udp.c      |    7 +++++++
 net/ipv6/udp.c      |    8 ++++++++
 3 files changed, 16 insertions(+)

diff --git a/include/linux/udp.h b/include/linux/udp.h
index 9d81de1..42278bb 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -68,6 +68,7 @@ struct udp_sock {
 	 * For encapsulation sockets.
 	 */
 	int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+	void (*encap_destroy)(struct sock *sk);
 };
 
 static inline struct udp_sock *udp_sk(const struct sock *sk)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 41760e0..7117d14 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1762,9 +1762,16 @@ int udp_rcv(struct sk_buff *skb)
 
 void udp_destroy_sock(struct sock *sk)
 {
+	struct udp_sock *up = udp_sk(sk);
 	bool slow = lock_sock_fast(sk);
 	udp_flush_pending_frames(sk);
 	unlock_sock_fast(sk, slow);
+	if (static_key_false(&udp_encap_needed) && up->encap_type) {
+		void (*encap_destroy)(struct sock *sk);
+		encap_destroy = ACCESS_ONCE(up->encap_destroy);
+		if (encap_destroy)
+			encap_destroy(sk);
+	}
 }
 
 /*
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 3ed57ec..da6019b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1286,10 +1286,18 @@ do_confirm:
 
 void udpv6_destroy_sock(struct sock *sk)
 {
+	struct udp_sock *up = udp_sk(sk);
 	lock_sock(sk);
 	udp_v6_flush_pending_frames(sk);
 	release_sock(sk);
 
+	if (static_key_false(&udpv6_encap_needed) && up->encap_type) {
+		void (*encap_destroy)(struct sock *sk);
+		encap_destroy = ACCESS_ONCE(up->encap_destroy);
+		if (encap_destroy)
+			encap_destroy(sk);
+	}
+
 	inet6_destroy_sock(sk);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] ixp4xx_eth: set the device dma_coherent_mask
From: Christophe Aeschlimann @ 2013-03-19 15:59 UTC (permalink / raw)
  To: khc; +Cc: netdev, linux-kernel, Christophe Aeschlimann

Without the mask it is impossible to take the network interface up
since it returns the following error:

> net eth1: coherent DMA mask is unset
> ifconfig: SIOCSIFFLAGS: Cannot allocate memory

Tested on an out-of-tree ixp425 based board.

Signed-off-by: Christophe Aeschlimann <c.aeschlimann@acn-group.ch>
---
 drivers/net/ethernet/xscale/ixp4xx_eth.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index 6958a5e..ff23c5c 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1398,6 +1398,7 @@ static int eth_init_one(struct platform_device *pdev)
 		return -ENOMEM;
 
 	SET_NETDEV_DEV(dev, &pdev->dev);
+	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
 	port = netdev_priv(dev);
 	port->netdev = dev;
 	port->id = pdev->id;
-- 
1.7.9

^ permalink raw reply related

* Re: Can we rely on ethernet header padding?
From: Michal Kubecek @ 2013-03-19 15:48 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, netfilter-devel
In-Reply-To: <1363706495.2558.14.camel@edumazet-glaptop>

On Tue, Mar 19, 2013 at 08:21:35AM -0700, Eric Dumazet wrote:
> 
> Normally a driver has NET_SKB_PAD bytes of headroom before the ethernet
> header, so the bridge code is safe only if all drivers use this
> NET_SKB_PAD padding on receive side. And they really should for
> performance reasons.
> 
> Better not touch bridge code to catch offending drivers

That makes sense. Thank you for your reply.

                                                   Michal Kubecek


^ permalink raw reply

* Re: [GIT PULL nf-next] IPVS changes for v3.10
From: Pablo Neira Ayuso @ 2013-03-19 15:43 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov
In-Reply-To: <1363697028-15314-1-git-send-email-horms@verge.net.au>

On Tue, Mar 19, 2013 at 09:43:46PM +0900, Simon Horman wrote:
> Hi Pablo,
> 
> The following changes since commit 1cdb09056b27b2a06b06dc7187d2c33d57082d20:
> 
>   netfilter: nfnetlink_queue: use xor hash function to distribute instances (2013-03-15 12:38:40 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git tags/ipvs-for-v3.10

Pulled, thanks!

^ permalink raw reply

* Re: [GIT PULL nf v2] IPVS fixes for v3.9
From: Pablo Neira Ayuso @ 2013-03-19 15:42 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov
In-Reply-To: <1363696850-14766-1-git-send-email-horms@verge.net.au>

On Tue, Mar 19, 2013 at 09:40:47PM +0900, Simon Horman wrote:
> Hi Pablo,
> 
> The following changes since commit a82783c91d5dce680dbd290ebf301a520b0e72a5:
> 
>   netfilter: ip6t_NPT: restrict to mangle table (2013-03-15 12:58:21 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git tags/ipvs-fixes-for-v3.9

Pulled, thanks Simon.

^ permalink raw reply

* Re: sfc fixes for stable
From: David Miller @ 2013-03-19 15:39 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363707481.3491.6.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 19 Mar 2013 15:38:01 +0000

> The last is not yet in Linus's tree but I assume you will ask him to
> pull from net soon.

You should really, as I do, wait until it actually hits Linus tree
before proposing it for -stable.

^ permalink raw reply

* Re: [PATCH v2 net-next 1/4] flow_keys: include thoff into flow_keys for later usage
From: Eric Dumazet @ 2013-03-19 15:38 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: netdev, davem, jasowang
In-Reply-To: <1363705390.2558.3.camel@edumazet-glaptop>

On Tue, 2013-03-19 at 08:03 -0700, Eric Dumazet wrote:
> On Tue, 2013-03-19 at 15:34 +0100, Daniel Borkmann wrote:
> > In skb_flow_dissect(), we perform a dissection of a skbuff. Since we're
> > doing the work here anyway, also store thoff for a later usage, e.g. in
> > the BPF filter. Also, by having thoff 16 Bit, we do not need to pack
> > flow_keys and reorder choke_skb_cb.
> > 
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> > ---
> >  This patch also needs to go into the net tree, since Eric or Jason will
> >  post a bug fix on top of this one.
> > 
> >  include/net/flow_keys.h   | 1 +
> >  net/core/flow_dissector.c | 5 ++++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> Oh well, you left the choke_skb_cb description in changelog
> 
> Acked-by: Eric Dumazet <edumazet@google.com>
> 

Actually this patch has a bug

the nhoff variable should stay as an int, or a malicious user could
trigger an infinite loop with a big packet, as nhoff could wrap.

^ permalink raw reply

* sfc fixes for stable
From: Ben Hutchings @ 2013-03-19 15:38 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

David,

Please consider the attached patchsets for 3.0.y, 3.2.y, 3.4.y and 3.8.y
respectively.

They contain backports of the following fixes, respectively:

3.0             4017dbdc14af sfc: Fix loop condition for efx_filter_search() when !for_insert
3.0             a659b2a94d87 sfc: Fix Siena mac statistics on big endian platforms
3.0 3.2 3.4     3dca9d2dc285 sfc: Do not attempt to flush queues if DMA is disabled
3.0 3.2 3.4     bfeed902946a sfc: Convert firmware subtypes to native byte order in efx_mcdi_get_board_cfg()
        3.4     9724a8504c87 sfc: Add parentheses around use of bitfield macro arguments
        3.4     0a6e5008a9df sfc: Fix MCDI structure field lookup
3.0 3.2         a606f4325dca sfc: Disable flow control during flushes
3.0 3.2 3.4     d5e8cc6c946e sfc: Really disable flow control while flushing
3.0 3.2 3.4     525d9e824018 sfc: Work-around flush timeout when flushes have completed
3.0 3.2 3.4     c2f3b8e3a44b sfc: lock TX queues when calling netif_device_detach()
3.0 3.2 3.4     ebf98e797b4e sfc: Fix timekeeping in efx_mcdi_poll()
        3.4     d4f2cecce138 sfc: Disable VF queues during register self-test
        3.4     450783747f42 sfc: Avoid generating over-length MC_CMD_FLUSH_RX_QUEUES request
        3.4     ef492f11efed sfc: Correctly initialise reset_method in siena_test_chip()
            3.8 56567c6f8751 drivers/net/ethernet/sfc/ptp.c: adjust duplicate test
3.0 3.2 3.4 3.8 3a68f19d7afb sfc: Properly sync RX DMA buffer when it is not the last in the page
3.0 3.2 3.4 3.8 b590ace09d51 sfc: Fix efx_rx_buf_offset() in the presence of swiotlb
3.0 3.2 3.4 3.8 c73e787a8db9 sfc: Correct efx_rx_buffer::page_offset when EFX_PAGE_IP_ALIGN != 0
3.0 3.2 3.4 3.8 29c69a488264 sfc: Detach net device when stopping queues for reconfiguration
3.0 3.2 3.4 3.8 35205b211c8d sfc: Disable soft interrupt handling during efx_device_detach_sync()
3.0 3.2 3.4 3.8 fae8563b25f7 sfc: Only use TX push if a single descriptor is to be written

The last is not yet in Linus's tree but I assume you will ask him to
pull from net soon.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[-- Attachment #2: sfc_3.0.mbox --]
[-- Type: application/mbox, Size: 35238 bytes --]

[-- Attachment #3: sfc_3.2.mbox --]
[-- Type: application/mbox, Size: 31246 bytes --]

[-- Attachment #4: sfc_3.4.mbox --]
[-- Type: application/mbox, Size: 51892 bytes --]

[-- Attachment #5: sfc_3.8.mbox --]
[-- Type: application/mbox, Size: 13636 bytes --]

^ permalink raw reply

* Re: [Xen-devel] [PATCH 1/4] xen-netfront: remove unused variable `extra'
From: Wei Liu @ 2013-03-19 15:26 UTC (permalink / raw)
  To: Paul Durrant
  Cc: wei.liu2, annie li, Ian Campbell, netdev@vger.kernel.org,
	konrad.wilk@oracle.com, xen-devel@lists.xen.org
In-Reply-To: <291EDFCB1E9E224A99088639C4762022013F7D2D3543@LONPMAILBOX01.citrite.net>

On Tue, 2013-03-19 at 09:28 +0000, Paul Durrant wrote:
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> > bounces@lists.xen.org] On Behalf Of annie li
> > Sent: 19 March 2013 02:39
> > To: Ian Campbell
> > Cc: netdev@vger.kernel.org; konrad.wilk@oracle.com; Wei Liu; xen-
> > devel@lists.xen.org
> > Subject: Re: [Xen-devel] [PATCH 1/4] xen-netfront: remove unused variable
> > `extra'
> > 
> > 
> > On 2013-3-18 20:14, Ian Campbell wrote:
> > > On Mon, 2013-03-18 at 12:04 +0000, Wei Liu wrote:
> > >> On Mon, 2013-03-18 at 11:42 +0000, Ian Campbell wrote:
> > >>> On Mon, 2013-03-18 at 10:35 +0000, Wei Liu wrote:
> > >>>
> > >>> I think a few more words are needed here since from the code you are
> > >>> removing it seems very much like gso is used for something. If you have
> > >>> a proof that the "extra = gso" case is never hit then please explain it.
> > >>> Perhaps a reference to the removal of the last user?
> > >>>
> > >>> Or maybe it is the case that it should be used and the bug is that it
> > >>> isn't?
> > >>>
> > >> Looks like the latter one. 'extra' field should  be used to get hold of
> > >> the last extra info in the ring. ;-)
> > >>
> > >> But, the only extra info in upstream kernel is
> > XEN_NETIF_EXTRA_TYPE_GSO,
> > >> so there's really no other extra info in the ring at that point. Could
> > >> it be possible that it is something from classic Xen kernel?
> > > The classic kernel netfront has exactly the same code it seems and
> > > netif_extra_type_gso is the only one I've ever heard of.
> > >
> > > Maybe this extra thing is just redundant unless/until a second extra
> > > comes along.
> > 
> > In our windows pv driver, we do not process this for GSO in tx path
> > either. Maybe we ignored processing for some special GSO?
> > 
> > BTW, what is XEN_NETIF_EXTRA_FLAG_MORE actually for? Backend only
> > processes it in xen_netback_tx_build_gops, but netfront xmit path does
> > not really set this flag. I did process it in rx path of my windows pv
> > driver(linux netfront did that too), but it seems unnecessary since
> > netback does not set this flag at all.
> > 
> 
> The flag is there to denote the existence of an 'extra' segment in the packet. The 'extra' segment goes after the 1st segment and specifies metadata such as the GSO type (TCPv4 is the only one at the moment but we'll need TCPv6 very shortly) and the MSS.
> Extra segments are certainly not redundant; the Citrix Windows PV drivers send TSOs using them and handle LRO using them too.
> 

I think Ian's (and my) idea of redundant is that this 'extra' variable
is never used in the code now and causes confusion. It can be removed
now and add back in the future if necessary.


Wei.

>   Paul

^ permalink raw reply

* Re: [Xen-devel] [PATCH 2/4] xen-netfront: drop skb when skb->len > 65535
From: Wei Liu @ 2013-03-19 15:23 UTC (permalink / raw)
  To: David Vrabel
  Cc: wei.liu2, Ian Campbell, netdev@vger.kernel.org,
	xen-devel@lists.xen.org, annie.li@oracle.com,
	konrad.wilk@oracle.com
In-Reply-To: <51486ADD.5050603@citrix.com>

On Tue, 2013-03-19 at 13:40 +0000, David Vrabel wrote:
> On 18/03/13 14:19, Wei Liu wrote:
> > On Mon, 2013-03-18 at 14:00 +0000, David Vrabel wrote:
> >> On 18/03/13 13:48, Ian Campbell wrote:
> >>> On Mon, 2013-03-18 at 13:46 +0000, David Vrabel wrote:
> >>>> On 18/03/13 10:35, Wei Liu wrote:
> >>>>> The `size' field of Xen network wire format is uint16_t, anything bigger than
> >>>>> 65535 will cause overflow.
> >>>>
> >>>> The backend needs to be able to handle these bad packets without
> >>>> disconnecting the VIF -- we can't fix all the frontend drivers.
> >>>
> >>> Agreed, although that doesn't imply that we shouldn't fix the frontend
> >>> where we can -- such as upstream as Wei does here.
> >>
> >> Yes, frontends should be fixed where possible.
> >>
> >> This is what I came up with for the backend.  I don't have time to look
> >> into it further but, Wei, feel free to use it as a starting point.
> >>
> > 
> > Thanks for this patch.
> > 
> > I haven't gone through XSA-39 discussion, this is why I didn't come up
> > with a fix for backend -- I need to make sure dropping packet like this
> > won't re-exhibit the security hole.
> 
> How are these overlarge packets generated?  How do you reproduce the issue?
> 

Inside a VM, ifconfig eth0 mtu 100, iperf -c XXXX .

But other people seeing this could not be using the same method because
nobody would set mtu to 100 in production system AFAICT.


Wei.

> David

^ permalink raw reply

* Re: Can we rely on ethernet header padding?
From: Eric Dumazet @ 2013-03-19 15:21 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, netfilter-devel
In-Reply-To: <20130319150545.GA18218@unicorn.suse.cz>

On Tue, 2013-03-19 at 16:05 +0100, Michal Kubecek wrote:
> Hello,
> 
> a customer of ours ran into
> 
>   http://bugzilla.netfilter.org/show_bug.cgi?id=765
> 
> They checked that commit a504b86e prevents the crash but I'm not sure it
> is sufficient.
> 
> The crash happens when br_nf_pre_routing_finish_bridge() calls
> neigh_hh_bridge() which copies not only destination MAC address but also
> the padding with it. IIUC this is for performance reasons (so that
> aligned 8 bytes are copied rather than 6).
> 
> But I wonder whether we can rely on the fact that every skb on an
> ethernet-like device has ethernet header padded at least to the 16 bytes
> expected by neigh_hh_bridge() and neigh_hh_output() or whether the
> bridge code should make sure. I tried to look for such test but couldn't
> find any, even if commit a504b86e description mentions reallocating the
> skb rather than a crash.

Thats a side effect.

Before calling netif_rx() the driver usually calls eth_type_trans()
to pull the ethernet header, so there is the room for 14 bytes.

Normally a driver has NET_SKB_PAD bytes of headroom before the ethernet
header, so the bridge code is safe only if all drivers use this
NET_SKB_PAD padding on receive side. And they really should for
performance reasons.

Better not touch bridge code to catch offending drivers






^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox