Netdev List
 help / color / mirror / Atom feed
* [PATCH] Make hmac algorithm selection for cookie generation dynamic
From: Neil Horman @ 2012-10-19 15:52 UTC (permalink / raw)
  To: linux-sctp; +Cc: Neil Horman, Vlad Yasevich, David S. Miller, netdev

Currently sctp allows for the optional use of md5 of sha1 hmac algorithms to
generate cookie values when establishing new connections via two build time
config options.  Theres no real reason to make this a static selection.  We can
add a sysctl that allows for the dynamic selection of these algorithms at run
time, with the default value determined by the corresponding crypto library
config options.  It saves us two needless configuration settings and enables the
freedom for administrators to select which algorithm a particular system uses.
This comes in handy when, for example running a system in FIPS mode, where use
of md5 is disallowed, but SHA1 is permitted.

Note: This new sysctl has no corresponding socket option to select the cookie
hmac algorithm.  I chose not to implement that intentionally, as RFC 6458
contains no option for this value, and I opted not to pollute the socket option
namespace.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org
---
 Documentation/networking/ip-sysctl.txt | 14 ++++++++
 include/net/netns/sctp.h               |  3 ++
 include/net/sctp/constants.h           |  8 -----
 include/net/sctp/structs.h             |  1 +
 net/sctp/Kconfig                       | 30 -----------------
 net/sctp/protocol.c                    |  9 ++++++
 net/sctp/socket.c                      | 11 ++++---
 net/sctp/sysctl.c                      | 59 ++++++++++++++++++++++++++++++++++
 8 files changed, 93 insertions(+), 42 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index c7fc107..98ac0d7 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1514,6 +1514,20 @@ cookie_preserve_enable - BOOLEAN
 
 	Default: 1
 
+cookie_hmac_alg - STRING
+	Select the hmac algorithm used when generating the cookie value sent by
+	a listening sctp socket to a connecting client in the INIT-ACK chunk.
+	Valid values are:
+	* md5
+	* sha1
+	* none
+	Ability to assign md5 or sha1 as the selected alg is predicated on the
+	configuarion of those algorithms at build time (CONFIG_CRYPTO_MD5 and
+	CONFIG_CRYPTO_SHA1).
+
+	Default: Dependent on configuration.  MD5 if available, else SHA1 if
+	available, else none.
+
 rcvbuf_policy - INTEGER
 	Determines if the receive buffer is attributed to the socket or to
 	association.   SCTP supports the capability to create multiple
diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index 5e5eb1f..3573a81 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -62,6 +62,9 @@ struct netns_sctp {
 	/* Whether Cookie Preservative is enabled(1) or not(0) */
 	int cookie_preserve_enable;
 
+	/* The namespace default hmac alg */
+	char *sctp_hmac_alg;
+
 	/* Valid.Cookie.Life	    - 60  seconds  */
 	unsigned int valid_cookie_life;
 
diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index d053d2e..c29707d 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -312,14 +312,6 @@ enum { SCTP_MAX_GABS = 16 };
 				 * functions simpler to write.
 				 */
 
-#if defined (CONFIG_SCTP_HMAC_MD5)
-#define SCTP_COOKIE_HMAC_ALG "hmac(md5)"
-#elif defined (CONFIG_SCTP_HMAC_SHA1)
-#define SCTP_COOKIE_HMAC_ALG "hmac(sha1)"
-#else
-#define SCTP_COOKIE_HMAC_ALG NULL
-#endif
-
 /* These return values describe the success or failure of a number of
  * routines which form the lower interface to SCTP_outqueue.
  */
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0fef00f..ce5f957 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -177,6 +177,7 @@ struct sctp_sock {
 
 	/* Access to HMAC transform. */
 	struct crypto_hash *hmac;
+	char *sctp_hmac_alg;
 
 	/* What is our base endpointer? */
 	struct sctp_endpoint *ep;
diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
index 126b014..44ffd3e 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -9,7 +9,6 @@ menuconfig IP_SCTP
 	select CRYPTO
 	select CRYPTO_HMAC
 	select CRYPTO_SHA1
-	select CRYPTO_MD5 if SCTP_HMAC_MD5
 	select LIBCRC32C
 	---help---
 	  Stream Control Transmission Protocol
@@ -68,33 +67,4 @@ config SCTP_DBG_OBJCNT
 
 	  If unsure, say N
 
-choice
-	prompt "SCTP: Cookie HMAC Algorithm"
-	default SCTP_HMAC_MD5
-	help
-	  HMAC algorithm to be used during association initialization.  It
-	  is strongly recommended to use HMAC-SHA1 or HMAC-MD5.  See 
-	  configuration for Cryptographic API and enable those algorithms
-          to make usable by SCTP. 
-
-config SCTP_HMAC_NONE
-	bool "None"
-	help 
-	  Choosing this disables the use of an HMAC during association 
-	  establishment.  It is advised to use either HMAC-MD5 or HMAC-SHA1.
-
-config SCTP_HMAC_SHA1
-	bool "HMAC-SHA1"
-	help 
-	  Enable the use of HMAC-SHA1 during association establishment.  It 
-	  is advised to use either HMAC-MD5 or HMAC-SHA1.
-
-config SCTP_HMAC_MD5
-	bool "HMAC-MD5"
-	help
-	  Enable the use of HMAC-MD5 during association establishment.  It is 
-	  advised to use either HMAC-MD5 or HMAC-SHA1.
-
-endchoice
-
 endif # IP_SCTP
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 2d51842..456bc3d 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1190,6 +1190,15 @@ static int sctp_net_init(struct net *net)
 	/* Whether Cookie Preservative is enabled(1) or not(0) */
 	net->sctp.cookie_preserve_enable 	= 1;
 
+	/* Default sctp sockets to use md5 as their hmac alg */
+#if defined (CONFIG_CRYPTO_MD5)
+	net->sctp.sctp_hmac_alg			= "md5";
+#elif defined (CONFIG_CRYPTO_SHA1)
+	net->sctp.sctp_hmac_alg			= "sha1";
+#else
+	net->sctp.sctp_hmac_alg			= NULL;
+#endif
+
 	/* Max.Burst		    - 4 */
 	net->sctp.max_burst			= SCTP_DEFAULT_MAX_BURST;
 
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d37d24f..c388262 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -109,7 +109,6 @@ static int sctp_do_bind(struct sock *, union sctp_addr *, int);
 static int sctp_autobind(struct sock *sk);
 static void sctp_sock_migrate(struct sock *, struct sock *,
 			      struct sctp_association *, sctp_socket_type_t);
-static char *sctp_hmac_alg = SCTP_COOKIE_HMAC_ALG;
 
 extern struct kmem_cache *sctp_bucket_cachep;
 extern long sysctl_sctp_mem[3];
@@ -3889,6 +3888,8 @@ SCTP_STATIC int sctp_init_sock(struct sock *sk)
 	sp->default_rcv_context = 0;
 	sp->max_burst = net->sctp.max_burst;
 
+	sp->sctp_hmac_alg = net->sctp.sctp_hmac_alg;
+
 	/* Initialize default setup parameters. These parameters
 	 * can be modified with the SCTP_INITMSG socket option or
 	 * overridden by the SCTP_INIT CMSG.
@@ -5966,13 +5967,15 @@ SCTP_STATIC int sctp_listen_start(struct sock *sk, int backlog)
 	struct sctp_sock *sp = sctp_sk(sk);
 	struct sctp_endpoint *ep = sp->ep;
 	struct crypto_hash *tfm = NULL;
+	char alg[32];
 
 	/* Allocate HMAC for generating cookie. */
-	if (!sctp_sk(sk)->hmac && sctp_hmac_alg) {
-		tfm = crypto_alloc_hash(sctp_hmac_alg, 0, CRYPTO_ALG_ASYNC);
+	if (!sp->hmac && sp->sctp_hmac_alg) {
+		sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);
+		tfm = crypto_alloc_hash(alg, 0, CRYPTO_ALG_ASYNC);
 		if (IS_ERR(tfm)) {
 			net_info_ratelimited("failed to load transform for %s: %ld\n",
-					     sctp_hmac_alg, PTR_ERR(tfm));
+					     sp->sctp_hmac_alg, PTR_ERR(tfm));
 			return -ENOSYS;
 		}
 		sctp_sk(sk)->hmac = tfm;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 70e3ba5..043889a 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -62,6 +62,11 @@ extern long sysctl_sctp_mem[3];
 extern int sysctl_sctp_rmem[3];
 extern int sysctl_sctp_wmem[3];
 
+static int proc_sctp_do_hmac_alg(ctl_table *ctl,
+				int write,
+				void __user *buffer, size_t *lenp,
+
+				loff_t *ppos);
 static ctl_table sctp_table[] = {
 	{
 		.procname	= "sctp_mem",
@@ -147,6 +152,12 @@ static ctl_table sctp_net_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
+		.procname	= "cookie_hmac_alg",
+		.maxlen		= 8,
+		.mode		= 0644,
+		.proc_handler	= proc_sctp_do_hmac_alg,
+	},
+	{
 		.procname	= "valid_cookie_life",
 		.data		= &init_net.sctp.valid_cookie_life,
 		.maxlen		= sizeof(unsigned int),
@@ -289,6 +300,54 @@ static ctl_table sctp_net_table[] = {
 	{ /* sentinel */ }
 };
 
+static int proc_sctp_do_hmac_alg(ctl_table *ctl,
+				int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos)
+{
+	struct net *net = current->nsproxy->net_ns;
+	char tmp[8];
+	ctl_table tbl;
+	int ret;
+	int changed = 0;
+	char *none = "none";
+
+	memset(&tbl, 0, sizeof(struct ctl_table));
+
+	if (write) {
+		tbl.data = tmp;
+		tbl.maxlen = 8;
+	} else {
+		tbl.data = net->sctp.sctp_hmac_alg ? : none;
+		tbl.maxlen = strlen(tbl.data);
+	}
+		ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
+
+	if (write) {
+#ifdef CONFIG_CRYPTO_MD5
+		if (!strncmp(tmp, "md5", 3)) {
+			net->sctp.sctp_hmac_alg = "md5";
+			changed = 1;
+		}
+#endif
+#ifdef CONFIG_CRYPTO_SHA1
+		if (!strncmp(tmp, "sha1", 4)) {
+			net->sctp.sctp_hmac_alg = "sha1";
+			changed = 1;
+		}
+#endif
+		if (!strncmp(tmp, "none", 4)) {
+			net->sctp.sctp_hmac_alg = NULL;
+			changed = 1;
+		}
+
+		if (!changed)
+			ret = -EINVAL;
+	}
+
+	return ret;
+}
+
 int sctp_sysctl_net_register(struct net *net)
 {
 	struct ctl_table *table;
-- 
1.7.11.7

^ permalink raw reply related

* Re: [ath9k-devel] [PATCH net-next 00/21] treewide: Use consistent api style for address testing
From: Pavel Roskin @ 2012-10-19 15:55 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w,
	users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
	linux-sctp-u79uwXL29TY76Z2rM5mHXA, devel-s9riP+hp16TNLxjTenLetw,
	dev-yBygre7rU0TnMu66kgdUjQ, linux-s390-u79uwXL29TY76Z2rM5mHXA,
	wimax-BPSAo7wm5JOHVYUYWc+uSQ, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	lvs-devel-u79uwXL29TY76Z2rM5mHXA, coreteam-Cap9r6Oaw4JrovVCs/uTlw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	cbe-oss-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	user-mode-linux-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	netfilter-u79uwXL29TY76Z2rM5mHXA,
	user-mode-linux-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, l
In-Reply-To: <1350630254.2293.183.camel@edumazet-glaptop>

On Fri, 19 Oct 2012 09:04:14 +0200
Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Yes they are some names discrepancies, thats a big deal.
> 
> And we have alloc_skb() / kfree_skb() / skb_clone() 
> 
> Why not skb_alloc() / skb_free() / skb_clone() ?
> 
> Some people actually know current code by name of functions, they dont
> want to change their mind and having to grep include files and git log
> to learn the new names of an old function, especially when traveling
> and using a laptop.

I agree.

Also, it makes sense to introduce a more consistent name for a function
when it's improved in some way and the callers need to be adjusted or
re-checked.

That way, the old name can be phased out as the code is made compatible
with the new function.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [ath9k-devel] [PATCH net-next 00/21] treewide: Use consistent api style for address testing
From: Joe Perches @ 2012-10-19 16:39 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Eric Dumazet, linux-kernel, netdev, ath9k-devel
In-Reply-To: <20121019115517.7b8514a8@mj>

(cc list trimmed)

On Fri, 2012-10-19 at 11:55 -0400, Pavel Roskin wrote:
> On Fri, 19 Oct 2012 09:04:14 +0200
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > Yes they are some names discrepancies, thats a big deal.
> > 
> > And we have alloc_skb() / kfree_skb() / skb_clone() 
> > 
> > Why not skb_alloc() / skb_free() / skb_clone() ?
> > 
> > Some people actually know current code by name of functions, they dont
> > want to change their mind and having to grep include files and git log
> > to learn the new names of an old function, especially when traveling
> > and using a laptop.
> 
> I agree.
> 
> Also, it makes sense to introduce a more consistent name for a function
> when it's improved in some way and the callers need to be adjusted or
> re-checked.
> 
> That way, the old name can be phased out as the code is made compatible
> with the new function.

That can also be done, as was done with this series, with backward
compatible #defines.

^ permalink raw reply

* RE: [PATCH] qla3xxx: Ensure request/response queue addr writes to the registers
From: Jitendra Kalsaria @ 2012-10-19 17:12 UTC (permalink / raw)
  To: Joe Jin, Ron Mercer, Dept-Eng Linux Driver
  Cc: netdev, linux-kernel, Greg Marsden
In-Reply-To: <5080ABD7.90006@oracle.com>



>From: Joe Jin [mailto:joe.jin@oracle.com] 
>Sent: Thursday, October 18, 2012 6:25 PM
>To: Ron Mercer; Dept-Eng Linux Driver; Jitendra Kalsaria; Joe Jin
>Cc: netdev; linux-kernel; Greg Marsden
>Subject: [PATCH] qla3xxx: Ensure request/response queue addr writes to the registers
>
>Before use the request and response queue addr, make sure it has wrote
>to the registers.
>
>Signed-off-by: Joe Jin <joe.jin@oracle.com>
>Cc: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>Cc: Ron Mercer <ron.mercer@qlogic.com>
>---
> drivers/net/ethernet/qlogic/qla3xxx.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
>index df09b1c..f745ade 100644
>--- a/drivers/net/ethernet/qlogic/qla3xxx.c
>+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
>@@ -2525,6 +2525,12 @@ static int ql_alloc_net_req_rsp_queues(struct ql3_adapter *qdev)
> 	qdev->req_q_size =
> 	    (u32) (NUM_REQ_Q_ENTRIES * sizeof(struct ob_mac_iocb_req));
> 
>+	/*
>+	 * The barrier is required to ensure request and response queue
>+	 * addr writes to the registers.
>+	 */
>+	wmb();
>+
> 	qdev->req_q_virt_addr =
> 	    pci_alloc_consistent(qdev->pdev,
> 				 (size_t) qdev->req_q_size,

Joe,

You need to move below line above the write memory barrier to get response queue address write to register
	qdev->rsp_q_size = NUM_RSP_Q_ENTRIES * sizeof(struct net_rsp_iocb);

Your patch will be like below:
<snip>
	qdev->req_q_size =
		(u32) (NUM_REQ_Q_ENTRIES * sizeof(struct ob_mac_iocb_req));

+	qdev->rsp_q_size = NUM_RSP_Q_ENTRIES * sizeof(struct net_rsp_iocb);
+
+	/*
+	 * The barrier is required to ensure request and response queue
+	 * addr writes to the registers.
+	 */
+	wmb();
+

<\snip>


thanks,
	Jiten
-- 
1.7.11.7

^ permalink raw reply

* Re: sock_getsockopt() not exported
From: David Miller @ 2012-10-19 17:12 UTC (permalink / raw)
  To: David.Laight; +Cc: eric.dumazet, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7056@saturn3.aculab.com>

From: "David Laight" <David.Laight@ACULAB.COM>
Date: Fri, 19 Oct 2012 10:26:52 +0100

> The Linux interface is all done with wrapper functions
> that get compiled on the target system so we can ship
> a big binary 'blob'.

Your maintainence problem, not our's.

^ permalink raw reply

* Re: PROBLEM: Memory leak (at least with SLUB) from "secpath_dup" (xfrm) in 3.5+ kernels
From: Mike Kazantsev @ 2012-10-19 17:36 UTC (permalink / raw)
  To: linux-mm; +Cc: paul, netdev
In-Reply-To: <20121019205055.2b258d09@sacrilege>

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

On Fri, 19 Oct 2012 20:50:55 +0600
Mike Kazantsev <mk.fraggod@gmail.com> wrote:

> slabtop showed "kmalloc-64" being the 99% offender in the past, but
> with recent kernels (3.6.1), it has changed to "secpath_cache"

To be more specific, on 3.5.4 kernel leak looks like this:

 Active / Total Objects (% used)    : 19971419 / 20084060 (99.4%)
 Active / Total Slabs (% used)      : 318645 / 318645 (100.0%)
 Active / Total Caches (% used)     : 79 / 121 (65.3%)
 Active / Total Size (% used)       : 1285299.85K / 1307992.83K (98.3%)
 Minimum / Average / Maximum Object : 0.01K / 0.06K / 8.00K

  OBJS ACTIVE  USE OBJ SIZE  SLABS OBJ/SLAB CACHE SIZE NAME                   
19678272 19678272 100%    0.06K 307473       64   1229892K kmalloc-64             
159198  95262  59%    0.10K   4082       39     16328K buffer_head            
 32865  17515  53%    0.19K   1565       21      6260K dentry                 
 20480  19456  95%    0.02K     80      256       320K ext4_io_page           
 16896  10380  61%    0.03K    132      128       528K kmalloc-32             
 16164  16164 100%    0.11K    449       36      1796K sysfs_dir_cache        
 15980  15980 100%    0.02K     94      170       376K fsnotify_event_holder  
 14742   9205  62%    0.87K    819       18     13104K ext4_inode_cache       
 13916   5494  39%    0.55K    497       28      7952K radix_tree_node        
 10030   5172  51%    0.05K    118       85       472K anon_vma_chain         
 10020  10020 100%    0.13K    334       30      1336K ext4_allocation_context
  9486   9398  99%    0.04K     93      102       372K Acpi-Namespace         
  8192   8192 100%    0.01K     16      512        64K kmalloc-8              
  6960   6016  86%    0.25K    435       16      1740K kmalloc-256            
  6641   5412  81%    0.55K    229       29      3664K inode_cache            
  5124   4333  84%    0.19K    244       21       976K kmalloc-192

Unfortunately, kernel on this machine isn't booted with slub_debug
options (yet), so there're no specific on whether it's allocated (as I
understand it) in the same call or a different one.

Not sure if it's even possible that it might be the same call.


-- 
Mike Kazantsev // fraggod.net

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH 00/12] at91_ether cleanup
From: David Miller @ 2012-10-19 17:48 UTC (permalink / raw)
  To: manabian; +Cc: nicolas.ferre, netdev
In-Reply-To: <1350594079-4337-1-git-send-email-manabian@gmail.com>

From: Joachim Eastwood <manabian@gmail.com>
Date: Thu, 18 Oct 2012 23:01:05 +0200

> This patch series prepares the old at91_ether driver for code
> sharing with the macb driver. The hardware is similar except for DMA
> TX/RX, so its not quite clear if it is practical to support both in
> one driver. But stuff like MDIO and statistics should be possible to
> share.

All applied, thanks.

Can you get rid of that HAVE_NET_MACB thing?  It's completely
pointless.  This is a platform driver for crying out loud,
therefore it can successfully build on any architecture.  And
we want this exposed for build testing as far as we can.

Thanks.

^ permalink raw reply

* Re: [PATCH V2 12/20] qlcnic: 83xx base driver
From: David Miller @ 2012-10-19 17:49 UTC (permalink / raw)
  To: sony.chacko; +Cc: netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1350635858-27549-13-git-send-email-sony.chacko@qlogic.com>


Patch is way too large and therefore didn't make it to the mailing
list.

Nobody can review your patch if it doesn't make the list.

So I'm tossing this entire series, please correct this and
repost.

^ permalink raw reply

* Re: bpf filter : support for vlan tag
From: Ani Sinha @ 2012-10-19 18:32 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Daniel Borkmann, netdev
In-Reply-To: <1350386917.3954.890.camel@edumazet-glaptop>

how about this?

On Tue, Oct 16, 2012 at 4:28 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> @@ -341,6 +342,12 @@ load_b:
>                 case BPF_S_ANC_CPU:
>                         A = raw_smp_processor_id();
>                         continue;
> +               case BPF_S_ANC_VLAN_TAG:
> +                       A = vlan_tx_tag_get(skb);
> +                       continue;
> +               case BPF_S_ANC_VLAN_TAG_PRESENT:
> +                       A = !!vlan_tx_tag_present(skb);
> +                       continue;
>                 case BPF_S_ANC_NLATTR: {
>                         struct nlattr *nla;


+               case BPF_S_ANC_VLAN_TAG:
+                       if (!vlan_tx_tag_present(skb)) {
+                               return 0;
+                       }
+                       A = vlan_tx_tag_get(skb);
+                       continue;
+               case BPF_S_ANC_VLAN_TAG_PRESENT :
+                       A = !! vlan_tx_tag_present(skb);
+                       continue;

Now, is there any particular reason we are not using clan_get_tag() api?

Thanks
ani

^ permalink raw reply

* [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Benjamin LaHaise @ 2012-10-19 19:13 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev

This is v2 of an attempt to pull in the relevant fixes for a problem in
v2.6.32 kernels where invalid cached routes are retained even after changes
to the routing table have been made.  A simple test case can be found at 
http://marc.info/?l=linux-netdev&m=135015076708950&w=2 .  Based on feedback 
from David Miller, additional changes have been pulled in, including fixes
for the same issue in IPv6.  Most of the patches required some rework owing
to the large differences in the networking stack between 2.6.32 and 3.6.

I have performed basic tests to confirm that the cases I was hitting are
now fixed, including a couple of tests with IPv4 and IPv6.  Comments?  
Thanks again to David for the pointers to the additional fixes required in
this area.

Benjamin LaHaise (6):
  ipv4: check_rt_genid in dst_check
  net: Document dst->obsolete better.
  ipv6: use DST_* macro to set obselete field
  netns: move net->ipv4.rt_genid to net->rt_genid
  ipv6: use net->rt_genid to check dst validity
  xfrm: invalidate dst on policy insertion/deletion

 include/net/dst.h                |   14 +++++++++++++-
 include/net/ip6_fib.h            |    4 +---
 include/net/net_namespace.h      |   12 ++++++++++++
 include/net/netns/ipv4.h         |    1 -
 net/core/dst.c                   |    4 ++--
 net/ipv4/route.c                 |   28 +++++++++++++++-------------
 net/ipv6/inet6_connection_sock.c |   24 +-----------------------
 net/ipv6/route.c                 |   35 ++++++++++++++++++++++++-----------
 net/sctp/output.c                |    2 +-
 net/xfrm/xfrm_policy.c           |   22 ++++++++++++----------
 security/selinux/include/xfrm.h  |    1 +
 11 files changed, 82 insertions(+), 65 deletions(-)


-- 
"Thought is the essence of where you are now."

^ permalink raw reply

* [PATCH 1/6] ipv4: check rt_genid in dst_check
From: Benjamin LaHaise @ 2012-10-19 19:21 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92
Author: Timo Teräs <timo.teras@iki.fi>
Date:   Thu Mar 18 23:20:20 2010 +0000

    ipv4: check rt_genid in dst_check

    Xfrm_dst keeps a reference to ipv4 rtable entries on each
    cached bundle. The only way to renew xfrm_dst when the underlying
    route has changed, is to implement dst_check for this. This is
    what ipv6 side does too.

    The problems started after 87c1e12b5eeb7b30b4b41291bef8e0b41fc3dde9
    ("ipsec: Fix bogus bundle flowi") which fixed a bug causing xfrm_dst
    to not get reused, until that all lookups always generated new
    xfrm_dst with new route reference and path mtu worked. But after the
    fix, the old routes started to get reused even after they were expired
    causing pmtu to break (well it would occationally work if the rtable
    gc had run recently and marked the route obsolete causing dst_check to
    get called).

    Signed-off-by: Timo Teras <timo.teras@iki.fi>
    Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: David S. Miller <davem@davemloft.net>

This commit is based on the above, with the addition of verifying blackhole
routes in the same manner.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/ipv4/route.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 58f141b..f16d19b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1412,7 +1412,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 					dev_hold(rt->u.dst.dev);
 				if (rt->idev)
 					in_dev_hold(rt->idev);
-				rt->u.dst.obsolete	= 0;
+				rt->u.dst.obsolete	= -1;
 				rt->u.dst.lastuse	= jiffies;
 				rt->u.dst.path		= &rt->u.dst;
 				rt->u.dst.neighbour	= NULL;
@@ -1477,7 +1477,7 @@ static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst)
 	struct dst_entry *ret = dst;
 
 	if (rt) {
-		if (dst->obsolete) {
+		if (dst->obsolete > 0) {
 			ip_rt_put(rt);
 			ret = NULL;
 		} else if ((rt->rt_flags & RTCF_REDIRECTED) ||
@@ -1700,7 +1700,9 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
 
 static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
 {
-	return NULL;
+	if (rt_is_expired((struct rtable *)dst))
+		return NULL;
+	return dst;
 }
 
 static void ipv4_dst_destroy(struct dst_entry *dst)
@@ -1862,7 +1864,8 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	if (!rth)
 		goto e_nobufs;
 
-	rth->u.dst.output= ip_rt_bug;
+	rth->u.dst.output = ip_rt_bug;
+	rth->u.dst.obsolete = -1;
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
 	rth->u.dst.flags= DST_HOST;
@@ -2023,6 +2026,7 @@ static int __mkroute_input(struct sk_buff *skb,
 	rth->fl.oif 	= 0;
 	rth->rt_spec_dst= spec_dst;
 
+	rth->u.dst.obsolete = -1;
 	rth->u.dst.input = ip_forward;
 	rth->u.dst.output = ip_output;
 	rth->rt_genid = rt_genid(dev_net(rth->u.dst.dev));
@@ -2187,6 +2191,7 @@ local_input:
 		goto e_nobufs;
 
 	rth->u.dst.output= ip_rt_bug;
+	rth->u.dst.obsolete = -1;
 	rth->rt_genid = rt_genid(net);
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
@@ -2411,7 +2416,8 @@ static int __mkroute_output(struct rtable **result,
 	rth->rt_gateway = fl->fl4_dst;
 	rth->rt_spec_dst= fl->fl4_src;
 
-	rth->u.dst.output=ip_output;
+	rth->u.dst.output = ip_output;
+	rth->u.dst.obsolete = -1;
 	rth->rt_genid = rt_genid(dev_net(dev_out));
 
 	RT_CACHE_STAT_INC(out_slow_tot);
@@ -2741,6 +2747,7 @@ static int ipv4_dst_blackhole(struct net *net, struct rtable **rp, struct flowi
 	if (rt) {
 		struct dst_entry *new = &rt->u.dst;
 
+		new->obsolete = -1;
 		atomic_set(&new->__refcnt, 1);
 		new->__use = 1;
 		new->input = dst_discard;
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH 2/6] net: Document dst->obsolete better.
From: Benjamin LaHaise @ 2012-10-19 19:21 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit f5b0a8743601a4477419171f5046bd07d1c080a0
Author: David S. Miller <davem@davemloft.net>
Date:   Thu Jul 19 12:31:33 2012 -0700

    net: Document dst->obsolete better.

    Add a big comment explaining how the field works, and use defines
    instead of magic constants for the values assigned to it.

    Suggested by Joe Perches.

    Signed-off-by: David S. Miller <davem@davemloft.net>

This is a slightly modified backport of the above commit to cover additional
locationsi of dst.u.obsolete = -1.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 include/net/dst.h      |   14 +++++++++++++-
 net/core/dst.c         |    4 ++--
 net/ipv4/route.c       |   12 ++++++------
 net/sctp/output.c      |    2 +-
 net/xfrm/xfrm_policy.c |   21 +++++++++++----------
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 5a900dd..938b525 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -42,7 +42,19 @@ struct dst_entry
 	struct dst_entry	*child;
 	struct net_device       *dev;
 	short			error;
+
+	/* A non-zero value of dst->obsolete forces by-hand validation
+	 * of the route entry.  Positive values are set by the generic
+	 * dst layer to indicate that the entry has been forcefully
+	 * destroyed.
+	 *
+	 * Negative values are used by the implementation layer code to
+	 * force invocation of the dst_ops->check() method.
+	 */
 	short			obsolete;
+#define DST_OBSOLETE_NONE	0
+#define DST_OBSOLETE_DEAD	2
+#define DST_OBSOLETE_FORCE_CHK	-1
 	int			flags;
 #define DST_HOST		1
 #define DST_NOXFRM		2
@@ -200,7 +212,7 @@ extern struct dst_entry *dst_destroy(struct dst_entry * dst);
 
 static inline void dst_free(struct dst_entry * dst)
 {
-	if (dst->obsolete > 1)
+	if (dst->obsolete > 0)
 		return;
 	if (!atomic_read(&dst->__refcnt)) {
 		dst = dst_destroy(dst);
diff --git a/net/core/dst.c b/net/core/dst.c
index cb1b348..5eb9929 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -99,7 +99,7 @@ loop:
 			 * But we do not have state "obsoleted, but
 			 * referenced by parent", so it is right.
 			 */
-			if (dst->obsolete > 1)
+			if (dst->obsolete > 0)
 				continue;
 
 			___dst_free(dst);
@@ -193,7 +193,7 @@ static void ___dst_free(struct dst_entry * dst)
 	if (dst->dev == NULL || !(dst->dev->flags&IFF_UP)) {
 		dst->input = dst->output = dst_discard;
 	}
-	dst->obsolete = 2;
+	dst->obsolete = DST_OBSOLETE_DEAD;
 }
 
 void __dst_free(struct dst_entry * dst)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f16d19b..f79a9a8 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1412,7 +1412,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 					dev_hold(rt->u.dst.dev);
 				if (rt->idev)
 					in_dev_hold(rt->idev);
-				rt->u.dst.obsolete	= -1;
+				rt->u.dst.obsolete	= DST_OBSOLETE_FORCE_CHK;
 				rt->u.dst.lastuse	= jiffies;
 				rt->u.dst.path		= &rt->u.dst;
 				rt->u.dst.neighbour	= NULL;
@@ -1865,7 +1865,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		goto e_nobufs;
 
 	rth->u.dst.output = ip_rt_bug;
-	rth->u.dst.obsolete = -1;
+	rth->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
 	rth->u.dst.flags= DST_HOST;
@@ -2026,7 +2026,7 @@ static int __mkroute_input(struct sk_buff *skb,
 	rth->fl.oif 	= 0;
 	rth->rt_spec_dst= spec_dst;
 
-	rth->u.dst.obsolete = -1;
+	rth->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 	rth->u.dst.input = ip_forward;
 	rth->u.dst.output = ip_output;
 	rth->rt_genid = rt_genid(dev_net(rth->u.dst.dev));
@@ -2191,7 +2191,7 @@ local_input:
 		goto e_nobufs;
 
 	rth->u.dst.output= ip_rt_bug;
-	rth->u.dst.obsolete = -1;
+	rth->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 	rth->rt_genid = rt_genid(net);
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
@@ -2417,7 +2417,7 @@ static int __mkroute_output(struct rtable **result,
 	rth->rt_spec_dst= fl->fl4_src;
 
 	rth->u.dst.output = ip_output;
-	rth->u.dst.obsolete = -1;
+	rth->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 	rth->rt_genid = rt_genid(dev_net(dev_out));
 
 	RT_CACHE_STAT_INC(out_slow_tot);
@@ -2747,7 +2747,7 @@ static int ipv4_dst_blackhole(struct net *net, struct rtable **rp, struct flowi
 	if (rt) {
 		struct dst_entry *new = &rt->u.dst;
 
-		new->obsolete = -1;
+		new->obsolete = DST_OBSOLETE_FORCE_CHK;
 		atomic_set(&new->__refcnt, 1);
 		new->__use = 1;
 		new->input = dst_discard;
diff --git a/net/sctp/output.c b/net/sctp/output.c
index d494100..a6fc861 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -375,7 +375,7 @@ int sctp_packet_transmit(struct sctp_packet *packet)
 	skb_set_owner_w(nskb, sk);
 
 	/* The 'obsolete' field of dst is set to 2 when a dst is freed. */
-	if (!dst || (dst->obsolete > 1)) {
+	if (!dst || dst->obsolete) {
 		dst_release(dst);
 		sctp_transport_route(tp, NULL, sctp_sk(sk));
 		if (asoc && (asoc->param_flags & SPP_PMTUD_ENABLE)) {
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index cb81ca3..1ae61bd 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1421,7 +1421,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 		dst1->xfrm = xfrm[i];
 		xdst->genid = xfrm[i]->genid;
 
-		dst1->obsolete = -1;
+		dst1->obsolete = DST_OBSOLETE_FORCE_CHK;
 		dst1->flags |= DST_HOST;
 		dst1->lastuse = now;
 
@@ -2049,12 +2049,13 @@ EXPORT_SYMBOL(__xfrm_route_forward);
 static struct dst_entry *xfrm_dst_check(struct dst_entry *dst, u32 cookie)
 {
 	/* Code (such as __xfrm4_bundle_create()) sets dst->obsolete
-	 * to "-1" to force all XFRM destinations to get validated by
-	 * dst_ops->check on every use.  We do this because when a
-	 * normal route referenced by an XFRM dst is obsoleted we do
-	 * not go looking around for all parent referencing XFRM dsts
-	 * so that we can invalidate them.  It is just too much work.
-	 * Instead we make the checks here on every use.  For example:
+	 * to DST_OBSOLETE_FORCE_CHK to force all XFRM destinations to
+	 * get validated by dst_ops->check on every use.  We do this
+	 * because when a normal route referenced by an XFRM dst is
+	 * obsoleted we do not go looking around for all parent
+	 * referencing XFRM dsts so that we can invalidate them.  It
+	 * is just too much work.  Instead we make the checks here on
+	 * every use.  For example:
 	 *
 	 *	XFRM dst A --> IPv4 dst X
 	 *
@@ -2064,9 +2065,9 @@ static struct dst_entry *xfrm_dst_check(struct dst_entry *dst, u32 cookie)
 	 * stale_bundle() check.
 	 *
 	 * When a policy's bundle is pruned, we dst_free() the XFRM
-	 * dst which causes it's ->obsolete field to be set to a
-	 * positive non-zero integer.  If an XFRM dst has been pruned
-	 * like this, we want to force a new route lookup.
+	 * dst which causes it's ->obsolete field to be set to
+	 * DST_OBSOLETE_DEAD.  If an XFRM dst has been pruned like
+	 * this, we want to force a new route lookup.
 	 */
 	if (dst->obsolete < 0 && !stale_bundle(dst))
 		return dst;
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH 3/6] ipv6: use DST_* macro to set obselete field
From: Benjamin LaHaise @ 2012-10-19 19:21 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit 2c20cbd7e3aa6e9dddc07975d3f3a89fe1f69c00
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Mon Sep 10 22:09:47 2012 +0000

    ipv6: use DST_* macro to set obselete field

    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Acked-by: Eric Dumazet <edumazet@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

This is a slightly modified backport of the above commit that covers
additional locations setting dst->u.obsolete = -1.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/ipv6/route.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e307517..b420ea9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -129,7 +129,7 @@ static struct rt6_info ip6_null_entry_template = {
 		.dst = {
 			.__refcnt	= ATOMIC_INIT(1),
 			.__use		= 1,
-			.obsolete	= -1,
+			.obsolete	= DST_OBSOLETE_FORCE_CHK,
 			.error		= -ENETUNREACH,
 			.metrics	= { [RTAX_HOPLIMIT - 1] = 255, },
 			.input		= ip6_pkt_discard,
@@ -152,7 +152,7 @@ static struct rt6_info ip6_prohibit_entry_template = {
 		.dst = {
 			.__refcnt	= ATOMIC_INIT(1),
 			.__use		= 1,
-			.obsolete	= -1,
+			.obsolete	= DST_OBSOLETE_FORCE_CHK,
 			.error		= -EACCES,
 			.metrics	= { [RTAX_HOPLIMIT - 1] = 255, },
 			.input		= ip6_pkt_prohibit,
@@ -170,7 +170,7 @@ static struct rt6_info ip6_blk_hole_entry_template = {
 		.dst = {
 			.__refcnt	= ATOMIC_INIT(1),
 			.__use		= 1,
-			.obsolete	= -1,
+			.obsolete	= DST_OBSOLETE_FORCE_CHK,
 			.error		= -EINVAL,
 			.metrics	= { [RTAX_HOPLIMIT - 1] = 255, },
 			.input		= dst_discard,
@@ -1161,7 +1161,7 @@ int ip6_route_add(struct fib6_config *cfg)
 		goto out;
 	}
 
-	rt->u.dst.obsolete = -1;
+	rt->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 	rt->rt6i_expires = (cfg->fc_flags & RTF_EXPIRES) ?
 				jiffies + clock_t_to_jiffies(cfg->fc_expires) :
 				0;
@@ -1960,7 +1960,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 	rt->u.dst.metrics[RTAX_MTU-1] = ipv6_get_mtu(rt->rt6i_dev);
 	rt->u.dst.metrics[RTAX_ADVMSS-1] = ipv6_advmss(net, dst_mtu(&rt->u.dst));
 	rt->u.dst.metrics[RTAX_HOPLIMIT-1] = -1;
-	rt->u.dst.obsolete = -1;
+	rt->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 
 	rt->rt6i_flags = RTF_UP | RTF_NONEXTHOP;
 	if (anycast)
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH 4/6] netns: move net->ipv4.rt_genid to net->rt_genid
From: Benjamin LaHaise @ 2012-10-19 19:21 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit b42664f898c976247f7f609b8bb9c94d7475ca10
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Mon Sep 10 22:09:44 2012 +0000

    netns: move net->ipv4.rt_genid to net->rt_genid

    This commit prepares the use of rt_genid by both IPv4 and IPv6.
    Initialization is left in IPv4 part.

    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Since 2.6.32 still has the routing cache, this patch has been modified to
retain the IPv4 method for updating rt_genid in rt_cache_invalidate() to
perturb the route cache's hash function.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 include/net/net_namespace.h |   12 ++++++++++++
 include/net/netns/ipv4.h    |    1 -
 net/ipv4/route.c            |   11 +++--------
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index a120284..b810fa1 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -84,6 +84,7 @@ struct net {
 	struct sk_buff_head	wext_nlevents;
 #endif
 	struct net_generic	*gen;
+	atomic_t		rt_genid;
 };
 
 
@@ -272,4 +273,15 @@ extern struct ctl_table_header *register_net_sysctl_rotable(
 	const struct ctl_path *path, struct ctl_table *table);
 extern void unregister_net_sysctl_table(struct ctl_table_header *header);
 
+static inline int rt_genid(struct net *net)
+{
+	return atomic_read(&net->rt_genid);
+}
+
+extern void rt_cache_invalidate(struct net *net);
+static inline void rt_genid_bump(struct net *net)
+{
+	rt_cache_invalidate(net);
+}
+
 #endif /* __NET_NET_NAMESPACE_H */
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 9a4b8b7..5f0d561 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -54,7 +54,6 @@ struct netns_ipv4 {
 	int current_rt_cache_rebuild_count;
 
 	struct timer_list rt_secret_timer;
-	atomic_t rt_genid;
 
 #ifdef CONFIG_IP_MROUTE
 	struct sock		*mroute_sk;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f79a9a8..83f16e3 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -267,11 +267,6 @@ static inline unsigned int rt_hash(__be32 daddr, __be32 saddr, int idx,
 		& rt_hash_mask;
 }
 
-static inline int rt_genid(struct net *net)
-{
-	return atomic_read(&net->ipv4.rt_genid);
-}
-
 #ifdef CONFIG_PROC_FS
 struct rt_cache_iter_state {
 	struct seq_net_private p;
@@ -884,12 +879,12 @@ static void rt_worker_func(struct work_struct *work)
  * many times (2^24) without giving recent rt_genid.
  * Jenkins hash is strong enough that litle changes of rt_genid are OK.
  */
-static void rt_cache_invalidate(struct net *net)
+void rt_cache_invalidate(struct net *net)
 {
 	unsigned char shuffle;
 
 	get_random_bytes(&shuffle, sizeof(shuffle));
-	atomic_add(shuffle + 1U, &net->ipv4.rt_genid);
+	atomic_add(shuffle + 1U, &net->rt_genid);
 }
 
 /*
@@ -3364,7 +3359,7 @@ static __net_initdata struct pernet_operations sysctl_route_ops = {
 
 static __net_init int rt_secret_timer_init(struct net *net)
 {
-	atomic_set(&net->ipv4.rt_genid,
+	atomic_set(&net->rt_genid,
 			(int) ((num_physpages ^ (num_physpages>>8)) ^
 			(jiffies ^ (jiffies >> 7))));
 
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH 5/6] ipv6: use net->rt_genid to check dst validity
From: Benjamin LaHaise @ 2012-10-19 19:22 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit 6f3118b571b8a4c06c7985dc3172c3526cb86253
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Mon Sep 10 22:09:46 2012 +0000

    ipv6: use net->rt_genid to check dst validity

    IPv6 dst should take care of rt_genid too. When a xfrm policy is inserted or
    deleted, all dst should be invalidated.
    To force the validation, dst entries should be created with ->obsolete set t
o
    DST_OBSOLETE_FORCE_CHK. This was already the case for all functions calling
    ip6_dst_alloc(), except for ip6_rt_copy().

    As a consequence, we can remove the specific code in inet6_connection_sock.

    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

This version of the above commit is slightly modified to compensate for
differences in ip6_dst_alloc().

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 include/net/ip6_fib.h            |    4 +---
 net/ipv6/inet6_connection_sock.c |   24 +-----------------------
 net/ipv6/route.c                 |   29 +++++++++++++++++++++--------
 3 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 15b492a..c6672d6 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -115,9 +115,7 @@ struct rt6_info
 
 	struct rt6key			rt6i_dst;
 
-#ifdef CONFIG_XFRM
-	u32				rt6i_flow_cache_genid;
-#endif
+	u32				rt6i_genid;
 
 	struct rt6key			rt6i_src;
 };
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index cc4797d..835bfe4 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -148,34 +148,12 @@ void __inet6_csk_dst_store(struct sock *sk, struct dst_entry *dst,
 			   struct in6_addr *daddr, struct in6_addr *saddr)
 {
 	__ip6_dst_store(sk, dst, daddr, saddr);
-
-#ifdef CONFIG_XFRM
-	{
-		struct rt6_info *rt = (struct rt6_info  *)dst;
-		rt->rt6i_flow_cache_genid = atomic_read(&flow_cache_genid);
-	}
-#endif
 }
 
 static inline
 struct dst_entry *__inet6_csk_dst_check(struct sock *sk, u32 cookie)
 {
-	struct dst_entry *dst;
-
-	dst = __sk_dst_check(sk, cookie);
-
-#ifdef CONFIG_XFRM
-	if (dst) {
-		struct rt6_info *rt = (struct rt6_info *)dst;
-		if (rt->rt6i_flow_cache_genid != atomic_read(&flow_cache_genid)) {
-			sk->sk_dst_cache = NULL;
-			dst_release(dst);
-			dst = NULL;
-		}
-	}
-#endif
-
-	return dst;
+	return __sk_dst_check(sk, cookie);
 }
 
 int inet6_csk_xmit(struct sk_buff *skb, int ipfragok)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b420ea9..80ab9cd 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -186,9 +186,17 @@ static struct rt6_info ip6_blk_hole_entry_template = {
 #endif
 
 /* allocate dst with ip6_dst_ops */
-static inline struct rt6_info *ip6_dst_alloc(struct dst_ops *ops)
+static inline struct rt6_info *ip6_dst_alloc(struct net *net,
+					     struct dst_ops *ops)
 {
-	return (struct rt6_info *)dst_alloc(ops);
+	struct rt6_info *rt = (struct rt6_info *)dst_alloc(ops);
+
+	if (rt) {
+		rt->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
+		rt->rt6i_genid = rt_genid(net);
+	}
+
+	return rt;
 }
 
 static void ip6_dst_destroy(struct dst_entry *dst)
@@ -886,6 +894,13 @@ static struct dst_entry *ip6_dst_check(struct dst_entry *dst, u32 cookie)
 
 	rt = (struct rt6_info *) dst;
 
+	/* All IPV6 dsts are created with ->obsolete set to the value
+	 * DST_OBSOLETE_FORCE_CHK which forces validation calls down
+	 * into this function always.
+	 */
+	if (rt->rt6i_genid != rt_genid(dev_net(rt->u.dst.dev)))
+		return NULL;
+
 	if (rt && rt->rt6i_node && (rt->rt6i_node->fn_sernum == cookie))
 		return dst;
 
@@ -970,7 +985,7 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	if (unlikely(idev == NULL))
 		return NULL;
 
-	rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops);
+	rt = ip6_dst_alloc(net, &net->ipv6.ip6_dst_ops);
 	if (unlikely(rt == NULL)) {
 		in6_dev_put(idev);
 		goto out;
@@ -1154,14 +1169,13 @@ int ip6_route_add(struct fib6_config *cfg)
 		goto out;
 	}
 
-	rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops);
+	rt = ip6_dst_alloc(net, &net->ipv6.ip6_dst_ops);
 
 	if (rt == NULL) {
 		err = -ENOMEM;
 		goto out;
 	}
 
-	rt->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 	rt->rt6i_expires = (cfg->fc_flags & RTF_EXPIRES) ?
 				jiffies + clock_t_to_jiffies(cfg->fc_expires) :
 				0;
@@ -1663,7 +1677,7 @@ void rt6_pmtu_discovery(struct in6_addr *daddr, struct in6_addr *saddr,
 static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
 {
 	struct net *net = dev_net(ort->rt6i_dev);
-	struct rt6_info *rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops);
+	struct rt6_info *rt = ip6_dst_alloc(net, &net->ipv6.ip6_dst_ops);
 
 	if (rt) {
 		rt->u.dst.input = ort->u.dst.input;
@@ -1943,7 +1957,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 				    int anycast)
 {
 	struct net *net = dev_net(idev->dev);
-	struct rt6_info *rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops);
+	struct rt6_info *rt = ip6_dst_alloc(net, &net->ipv6.ip6_dst_ops);
 	struct neighbour *neigh;
 
 	if (rt == NULL)
@@ -1960,7 +1974,6 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 	rt->u.dst.metrics[RTAX_MTU-1] = ipv6_get_mtu(rt->rt6i_dev);
 	rt->u.dst.metrics[RTAX_ADVMSS-1] = ipv6_advmss(net, dst_mtu(&rt->u.dst));
 	rt->u.dst.metrics[RTAX_HOPLIMIT-1] = -1;
-	rt->u.dst.obsolete = DST_OBSOLETE_FORCE_CHK;
 
 	rt->rt6i_flags = RTF_UP | RTF_NONEXTHOP;
 	if (anycast)
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH 6/6] xfrm: invalidate dst on policy insertion/deletion
From: Benjamin LaHaise @ 2012-10-19 19:22 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

commit ee8372dd1989287c5eedb69d44bac43f69e496f1
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Mon Sep 10 22:09:45 2012 +0000

    xfrm: invalidate dst on policy insertion/deletion

    When a policy is inserted or deleted, all dst should be recalculated.

    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/xfrm/xfrm_policy.c          |    1 +
 security/selinux/include/xfrm.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1ae61bd..3aa00e1 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -587,6 +587,7 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 	xfrm_pol_hold(policy);
 	net->xfrm.policy_count[dir]++;
 	atomic_inc(&flow_cache_genid);
+	rt_genid_bump(net);
 	if (delpol)
 		__xfrm_policy_unlink(delpol, dir);
 	policy->index = delpol ? delpol->index : xfrm_gen_index(net, dir);
diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h
index 13128f9..9acf6fa 100644
--- a/security/selinux/include/xfrm.h
+++ b/security/selinux/include/xfrm.h
@@ -49,6 +49,7 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall);
 static inline void selinux_xfrm_notify_policyload(void)
 {
 	atomic_inc(&flow_cache_genid);
+	rt_genid_bump(&init_net);
 }
 #else
 static inline int selinux_xfrm_enabled(void)
-- 
1.7.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* Re: Bug#690845: ethtool: incorrect WoL detection on Broadcom NX II rev < 12
From: Michael Chan @ 2012-10-19 19:38 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: 690845, Teodor MICU, netdev
In-Reply-To: <1350610209.22276.6.camel@deadeye.wl.decadent.org.uk>

On Fri, 2012-10-19 at 02:30 +0100, Ben Hutchings wrote: 
> Teodor MICU reported that:
> > On Broadcom NX II rev < 12 network interfaces it's not possible to activate
> > WoL using 'ethtool' (it can only be done from the MBA Configuration Menu).
> > 
> > However, even if "Pre-boot Wake On LAN" was enabled 'ethtool' doesn't
> > see this change:
> > 	Supports Wake-on: d
> > 	Wake-on: d
> > 
> > I've just tested that WoL works for this system.
> 
> He's been using Linux 2.6.32 and Linux 3.2, but there don't seem to have
> been any later changes to WoL support in bnx2.
> 
Teodor MICU, please provide lspci -vvv and ethtool -i eth? on the
Broadcom bnx2 device.  Thanks.

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Willy Tarreau @ 2012-10-19 19:48 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019191347.GD13515@kvack.org>

Hi Ben,

On Fri, Oct 19, 2012 at 03:13:48PM -0400, Benjamin LaHaise wrote:
> This is v2 of an attempt to pull in the relevant fixes for a problem in
> v2.6.32 kernels where invalid cached routes are retained even after changes
> to the routing table have been made.  A simple test case can be found at 
> http://marc.info/?l=linux-netdev&m=135015076708950&w=2 .  Based on feedback 
> from David Miller, additional changes have been pulled in, including fixes
> for the same issue in IPv6.  Most of the patches required some rework owing
> to the large differences in the networking stack between 2.6.32 and 3.6.
> 
> I have performed basic tests to confirm that the cases I was hitting are
> now fixed, including a couple of tests with IPv4 and IPv6.  Comments?  
> Thanks again to David for the pointers to the additional fixes required in
> this area.

Looks like you've done an amazing work. I'm not the best person to judge
if these changes are valid or not, so I'll trust you and David on this.

I just have two questions: since you had to backport them from 3.6 to 2.6.32,
I'm assuming that the equivalent was not ready in closer LTS kernels (2.6.34,
3.0, 3.2, 3.4). Does this mean they're affected by the same issue too or are
they immune ? And if they're affected, are you going to proceed with forward
porting of your work for the closest ones (.34 and 3.0 come to mind) ? And if
they aren't affected, do you think that picking some changes there would have
been easier ?

As you probably know, we want to ensure that people who upgrade from an old
LTS to a newer one won't experience regressions due to problems which were
fixed only in the older version.

Thanks !
Willy

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: David Miller @ 2012-10-19 19:49 UTC (permalink / raw)
  To: w; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019194822.GF17417@1wt.eu>


How about checking if these changes are already in 3.0/3.2/etc. or not
before asking such questions?

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Willy Tarreau @ 2012-10-19 19:55 UTC (permalink / raw)
  To: David Miller; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019.154930.215119663249874810.davem@davemloft.net>

On Fri, Oct 19, 2012 at 03:49:30PM -0400, David Miller wrote:
> 
> How about checking if these changes are already in 3.0/3.2/etc. or not
> before asking such questions?

Because I didn't find the patches in 3.0 and Ben said he backported them
from 3.6, I think these are two valid reasons to ask, no ?

Willy

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: David Miller @ 2012-10-19 20:01 UTC (permalink / raw)
  To: w; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019195557.GG17417@1wt.eu>

From: Willy Tarreau <w@1wt.eu>
Date: Fri, 19 Oct 2012 21:55:57 +0200

> On Fri, Oct 19, 2012 at 03:49:30PM -0400, David Miller wrote:
>> 
>> How about checking if these changes are already in 3.0/3.2/etc. or not
>> before asking such questions?
> 
> Because I didn't find the patches in 3.0 and Ben said he backported them
> from 3.6, I think these are two valid reasons to ask, no ?

Well, the thing is, I personally don't consider them appropriate for
3.x.y -stable backports, and that's why I haven't submitted them.

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Willy Tarreau @ 2012-10-19 20:03 UTC (permalink / raw)
  To: David Miller; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019.160104.115897882147794932.davem@davemloft.net>

On Fri, Oct 19, 2012 at 04:01:04PM -0400, David Miller wrote:
> From: Willy Tarreau <w@1wt.eu>
> Date: Fri, 19 Oct 2012 21:55:57 +0200
> 
> > On Fri, Oct 19, 2012 at 03:49:30PM -0400, David Miller wrote:
> >> 
> >> How about checking if these changes are already in 3.0/3.2/etc. or not
> >> before asking such questions?
> > 
> > Because I didn't find the patches in 3.0 and Ben said he backported them
> > from 3.6, I think these are two valid reasons to ask, no ?
> 
> Well, the thing is, I personally don't consider them appropriate for
> 3.x.y -stable backports, and that's why I haven't submitted them.

OK. Is is because the issue is less important there or because the fix are
more risky than the issues they fix (or any other reason) ?

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: David Miller @ 2012-10-19 20:07 UTC (permalink / raw)
  To: w; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019200318.GH17417@1wt.eu>

From: Willy Tarreau <w@1wt.eu>
Date: Fri, 19 Oct 2012 22:03:18 +0200

> On Fri, Oct 19, 2012 at 04:01:04PM -0400, David Miller wrote:
>> From: Willy Tarreau <w@1wt.eu>
>> Date: Fri, 19 Oct 2012 21:55:57 +0200
>> 
>> > On Fri, Oct 19, 2012 at 03:49:30PM -0400, David Miller wrote:
>> >> 
>> >> How about checking if these changes are already in 3.0/3.2/etc. or not
>> >> before asking such questions?
>> > 
>> > Because I didn't find the patches in 3.0 and Ben said he backported them
>> > from 3.6, I think these are two valid reasons to ask, no ?
>> 
>> Well, the thing is, I personally don't consider them appropriate for
>> 3.x.y -stable backports, and that's why I haven't submitted them.
> 
> OK. Is is because the issue is less important there or because the fix are
> more risky than the issues they fix (or any other reason) ?

I have a different opinion about the risk/benefit ratio than Ben does.

I do not think these cases are important enough to enough people to
justify -stable inclusion at all.

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Willy Tarreau @ 2012-10-19 20:14 UTC (permalink / raw)
  To: David Miller; +Cc: bcrl, stable, netdev
In-Reply-To: <20121019.160711.1159467896676133280.davem@davemloft.net>

On Fri, Oct 19, 2012 at 04:07:11PM -0400, David Miller wrote:
> From: Willy Tarreau <w@1wt.eu>
> Date: Fri, 19 Oct 2012 22:03:18 +0200
> 
> > On Fri, Oct 19, 2012 at 04:01:04PM -0400, David Miller wrote:
> >> From: Willy Tarreau <w@1wt.eu>
> >> Date: Fri, 19 Oct 2012 21:55:57 +0200
> >> 
> >> > On Fri, Oct 19, 2012 at 03:49:30PM -0400, David Miller wrote:
> >> >> 
> >> >> How about checking if these changes are already in 3.0/3.2/etc. or not
> >> >> before asking such questions?
> >> > 
> >> > Because I didn't find the patches in 3.0 and Ben said he backported them
> >> > from 3.6, I think these are two valid reasons to ask, no ?
> >> 
> >> Well, the thing is, I personally don't consider them appropriate for
> >> 3.x.y -stable backports, and that's why I haven't submitted them.
> > 
> > OK. Is is because the issue is less important there or because the fix are
> > more risky than the issues they fix (or any other reason) ?
> 
> I have a different opinion about the risk/benefit ratio than Ben does.
> 
> I do not think these cases are important enough to enough people to
> justify -stable inclusion at all.

OK, thanks for the precision.

So maybe in the end we should just merge d11a4dc18 that Ben found to be
the least invasive one fixing the issues, and we'd be in sync with the
rest of the stable branches, even if, as you noted a few days ago, it's
only a partial fix for the issue.

Ben, what's your opinion on this ? I know it's never fun to do backports
and not merge them later, but I trust David more than anyone else on the
network part, so if he decided that while incomplete, the patch above
was all that was needed for other stable branches, maybe we should just
stay on the safe side and do the same ?

Willy

^ permalink raw reply

* Re: [stable 2.6.32.y PATCH 0/6] net: fixes for cached dsts are never invalidated
From: Benjamin LaHaise @ 2012-10-19 20:18 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: David Miller, stable, netdev
In-Reply-To: <20121019200318.GH17417@1wt.eu>

On Fri, Oct 19, 2012 at 10:03:18PM +0200, Willy Tarreau wrote:
> OK. Is is because the issue is less important there or because the fix are
> more risky than the issues they fix (or any other reason) ?

The behaviour of 2.6.32.x is certainly a regression from previous stable 
kernels used by various distributions (ie 2.6.18), and is quite serious 
for those of us running routers on top of these releases.

		-ben
-- 
"Thought is the essence of where you are now."

^ 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