Netdev List
 help / color / mirror / Atom feed
* Re: Libertas related kernel crash
From: Daniel Mack @ 2009-11-10  6:54 UTC (permalink / raw)
  To: Dan Williams; +Cc: netdev, libertas-dev, Michael Hirsch, linux-kernel
In-Reply-To: <1257835893.15493.37.camel@localhost.localdomain>

On Mon, Nov 09, 2009 at 10:51:33PM -0800, Dan Williams wrote:
> On Mon, 2009-11-09 at 16:53 +0100, Daniel Mack wrote:
> > On Thu, Nov 05, 2009 at 01:05:49PM +0100, Daniel Mack wrote:
> > > On an ARM (PXA300) embdedded platform with a libertas chip connected via
> > > SDIO, we happen to see the kernel Ooops below once in a while.
> > > 
> > > Any pointer on where to dig?
> > 
> > Some more input on this. Oopses similar to the one below are likely
> > triggered when switching from Ad-hoc to managed mode multiple times in a
> > row, and something seems corrupt the memory badly. I've searched for the
> > obvious (double frees, out-of-bound writes etc), but I couldn't find
> > anything yet. It is, however, related to the wireless core and/or the
> > libertas driver.
> 
> Probably just libertas.  Any chance you can enable debugging options
> (either in libertas, or in the kernel allocator) to help narrow down the
> issue?

Sure. I already enabled DEBUG_VM, but that didn't spit out anything
before it crashes. Which other debug flags would you recommend to set?

Daniel

^ permalink raw reply

* Re: Libertas related kernel crash
From: Dan Williams @ 2009-11-10  7:04 UTC (permalink / raw)
  To: Daniel Mack; +Cc: netdev, libertas-dev, Michael Hirsch, linux-kernel
In-Reply-To: <20091110065457.GL14091@buzzloop.caiaq.de>

On Tue, 2009-11-10 at 07:54 +0100, Daniel Mack wrote:
> On Mon, Nov 09, 2009 at 10:51:33PM -0800, Dan Williams wrote:
> > On Mon, 2009-11-09 at 16:53 +0100, Daniel Mack wrote:
> > > On Thu, Nov 05, 2009 at 01:05:49PM +0100, Daniel Mack wrote:
> > > > On an ARM (PXA300) embdedded platform with a libertas chip connected via
> > > > SDIO, we happen to see the kernel Ooops below once in a while.
> > > > 
> > > > Any pointer on where to dig?
> > > 
> > > Some more input on this. Oopses similar to the one below are likely
> > > triggered when switching from Ad-hoc to managed mode multiple times in a
> > > row, and something seems corrupt the memory badly. I've searched for the
> > > obvious (double frees, out-of-bound writes etc), but I couldn't find
> > > anything yet. It is, however, related to the wireless core and/or the
> > > libertas driver.
> > 
> > Probably just libertas.  Any chance you can enable debugging options
> > (either in libertas, or in the kernel allocator) to help narrow down the
> > issue?
> 
> Sure. I already enabled DEBUG_VM, but that didn't spit out anything
> before it crashes. Which other debug flags would you recommend to set?

Offhand I don't actually know; but ISTR there used to be slab debugging
options in the kernel that could help find memory corruption issues.  I
can't think of anything in libertas offhand that would trigger this, but
maybe something is not getting properly cleaned up when switching modes?

Dan



^ permalink raw reply

* Re: [PATCH 01/75] netx: declare MODULE_FIRMWARE
From: Sascha Hauer @ 2009-11-10  7:36 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev
In-Reply-To: <1257629856.15927.370.camel@localhost>

On Sat, Nov 07, 2009 at 09:37:36PM +0000, Ben Hutchings wrote:
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Note that the filenames are generated in xc_request_firmware(), defined
> in arch/arm/mach-netx/xc.c.  I'm not sure whether it's actually worth
> doing this for a platform driver.

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

> 
> Ben.
> 
>  drivers/net/netx-eth.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/netx-eth.c b/drivers/net/netx-eth.c
> index 9f42354..a0d65f5 100644
> --- a/drivers/net/netx-eth.c
> +++ b/drivers/net/netx-eth.c
> @@ -510,3 +510,6 @@ module_exit(netx_eth_cleanup);
>  MODULE_AUTHOR("Sascha Hauer, Pengutronix");
>  MODULE_LICENSE("GPL");
>  MODULE_ALIAS("platform:" CARDNAME);
> +MODULE_FIRMWARE("xc0.bin");
> +MODULE_FIRMWARE("xc1.bin");
> +MODULE_FIRMWARE("xc2.bin");
> -- 
> 1.6.5.2
> 
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH] act_mirred: don't go back.
From: jamal @ 2009-11-10  7:40 UTC (permalink / raw)
  To: Changli Gao; +Cc: Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <412e6f7f0911090433j2b270663ycdf277110fbc6bac@mail.gmail.com>


I apologize - I am still not convinced this is a cleanup and i can
already see holes you are introducing (example not freeing skb etc). 
You are putting me in a dilemma of not wanting to discourage you
but at the same time not seeing this as a useful change to be made.
Can we let this one slide?

cheers,
jamal

On Mon, 2009-11-09 at 20:33 +0800, Changli Gao wrote:

[..]
> How about this version.
> 
> 1. move skb_act_clone() after all the necessary checks, and it can
> eliminate unnecessary skb_act_clone() if tcfm_eaction isn't correct.
> 2. there is one exit of the critical section.
> 3. jump forward instead of backward.



^ permalink raw reply

* Re: Possible bug: SO_TIMESTAMPING 2.6.30+
From: Christopher Zimmermann @ 2009-11-10  8:12 UTC (permalink / raw)
  To: Marcus D. Leech; +Cc: netdev
In-Reply-To: <4AF8B67E.3030604@ripnet.com>

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

On Mon, 09 Nov 2009 19:40:30 -0500
"Marcus D. Leech" <mleech@ripnet.com> wrote:


> I know that Patrick Ohly has essentially moved on from doing the 
> SO_TIMESTAMPING stuff, so
>    who's maintaining it now?

I worked on it a month ago or so and have a patchset from Patick Ohly
and some changes by me which fix software timestamping and make the
ioctl interface to the hardware more flexible (keeping backwards
compatibility). Patches are attached.
Still I never tried IPv6 and don't think the patches do anything about
it.
It would be nice to know weather software tx timestamps work
with/without the patches.


Christopher

[-- Attachment #2: 0001-timecompare-use-ARRAY_SIZE-macro.patch --]
[-- Type: text/x-patch, Size: 941 bytes --]

>From 5e408d962dbbafc581d4442d2e920dc667c0d078 Mon Sep 17 00:00:00 2001
From: Christopher Zimmermann <madroach@zakweb.de>
Date: Mon, 17 Aug 2009 21:26:17 +0200
Subject: [PATCH 1/3] timecompare: use ARRAY_SIZE() macro

---
 kernel/time/timecompare.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timecompare.c b/kernel/time/timecompare.c
index 71e7f1a..fe24977 100644
--- a/kernel/time/timecompare.c
+++ b/kernel/time/timecompare.c
@@ -56,11 +56,11 @@ int timecompare_offset(struct timecompare *sync,
 	int index;
 	int num_samples = sync->num_samples;
 
-	if (num_samples > sizeof(buffer)/sizeof(buffer[0])) {
+	if (num_samples > ARRAY_SIZE(buffer)) {
 		samples = kmalloc(sizeof(*samples) * num_samples, GFP_ATOMIC);
 		if (!samples) {
 			samples = buffer;
-			num_samples = sizeof(buffer)/sizeof(buffer[0]);
+			num_samples = ARRAY_SIZE(buffer);
 		}
 	} else {
 		samples = buffer;
-- 
1.6.3.3


[-- Attachment #3: 0002-net-software-TX-time-stamping.patch --]
[-- Type: text/x-patch, Size: 6336 bytes --]

>From 50d8dfcf781fd3e9ad1d6123ff204b9f77a185ea Mon Sep 17 00:00:00 2001
From: Christopher Zimmermann <madroach@zakweb.de>
Date: Mon, 17 Aug 2009 21:27:50 +0200
Subject: [PATCH 2/3] net: software TX time stamping

This patch implements the software fallback to TX time stamping. The
necessary access to the buffer and socket are secured by taking
references before calling ndo_start_xmit().

That avoids race conditions (buffer remains available even if
transmission completes before ndo_start_xmit() returns) and works
even if the driver calls skb_orphan().

The caller of skb_tstamp_tx() is now responsible for providing the
socket, which requires minor changes in users of the previous call:
add skb->sk as parameter to get the old behavior.
---
 drivers/net/igb/igb_main.c |    2 +-
 include/linux/skbuff.h     |   11 +++++---
 net/core/dev.c             |   65 ++++++++++++++++++++++++++++++++++++++++++++
 net/core/skbuff.c          |    4 +-
 4 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index adb09d3..3ce4d78 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -4370,7 +4370,7 @@ static void igb_tx_hwtstamp(struct igb_adapter *adapter, struct sk_buff *skb)
 			shhwtstamps.hwtstamp = ns_to_ktime(ns);
 			shhwtstamps.syststamp =
 				timecompare_transform(&adapter->compare, ns);
-			skb_tstamp_tx(skb, &shhwtstamps);
+			skb_tstamp_tx(skb, skb->sk, &shhwtstamps);
 		}
 	}
 }
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f2c69a2..e29e43d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1873,17 +1873,20 @@ static inline ktime_t net_invalid_timestamp(void)
 
 /**
  * skb_tstamp_tx - queue clone of skb with send time stamps
- * @orig_skb:	the original outgoing packet
+ * @skb:	the original outgoing packet
+ * @sk:		sending socket (either from skb->sk or previous sock_get()),
+ *		may be NULL
  * @hwtstamps:	hardware time stamps, may be NULL if not available
  *
- * If the skb has a socket associated, then this function clones the
+ * If the socket is available, then this function clones the
  * skb (thus sharing the actual data and optional structures), stores
  * the optional hardware time stamping information (if non NULL) or
  * generates a software time stamp (otherwise), then queues the clone
  * to the error queue of the socket.  Errors are silently ignored.
  */
-extern void skb_tstamp_tx(struct sk_buff *orig_skb,
-			struct skb_shared_hwtstamps *hwtstamps);
+extern void skb_tstamp_tx(struct sk_buff *skb,
+			  struct sock *sk,
+			  struct skb_shared_hwtstamps *hwtstamps);
 
 extern __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len);
 extern __sum16 __skb_checksum_complete(struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 6a94475..e20e3ca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1679,10 +1679,71 @@ static int dev_gso_segment(struct sk_buff *skb)
 	return 0;
 }
 
+/**
+ * struct tx_tstamp_context - context for software TX time stamping
+ * @sk: socket reference, NULL if nothing to do
+ * @skb: packet reference
+ */
+struct tx_tstamp_context {
+	struct sock *sk;
+	struct sk_buff *skb;
+};
+
+/**
+ * tx_tstamp_start - check for TX software time stamping and prepare for it
+ * @skb: buffer which is being sent
+ * @context: needs to be initialized for tx_tstamp_end()
+ */
+static void tx_tstamp_start(struct sk_buff *skb,
+			    struct tx_tstamp_context *context)
+{
+	union skb_shared_tx *shtx = skb_tx(skb);
+	/*
+	 * Prepare for TX time stamping in software if requested.
+	 * This could be optimized so that device drivers
+	 * do that themselves, which avoids the skb/sk ref/unref
+	 * overhead.
+	 */
+	if (unlikely(shtx->software &&
+		     skb->sk)) {
+		context->sk = skb->sk;
+		sock_hold(skb->sk);
+		context->skb = skb_get(skb);
+	} else {
+		/* TX software time stamping not requested/not possible. */
+		context->sk = NULL;
+	}
+}
+
+/**
+ * tx_tstamp_end - finish the work started by tx_tstamp_end()
+ * @context: may contain socket and buffer references
+ * @rc: result of ndo_start_xmit() - only do time stamping if packet was sent
+ */
+static void tx_tstamp_end(struct tx_tstamp_context *context, int rc)
+{
+	if (unlikely(context->sk)) {
+		union skb_shared_tx *shtx = skb_tx(context->skb);
+		/*
+		 * Checking shtx->software again is a bit redundant: it must
+		 * have been set in tx_tstamp_start(), but perhaps it
+		 * was cleared in the meantime to disable the TX software
+		 * fallback.
+		 */
+		if (likely(!rc) &&
+		    unlikely(shtx->software &&
+			     !shtx->in_progress))
+			skb_tstamp_tx(context->skb, context->sk, NULL);
+		sock_put(context->sk);
+		kfree_skb(context->skb);
+	}
+}
+
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			struct netdev_queue *txq)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
+	struct tx_tstamp_context context;
 	int rc;
 
 	if (likely(!skb->next)) {
@@ -1703,6 +1764,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 		if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
 			skb_dst_drop(skb);
 
+		tx_tstamp_start(skb, &context);
 		rc = ops->ndo_start_xmit(skb, dev);
 		if (rc == 0)
 			txq_trans_update(txq);
@@ -1720,6 +1782,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 		 * the skb destructor before the call and restoring it
 		 * afterwards, then doing the skb_orphan() ourselves?
 		 */
+		tx_tstamp_end(&context, rc);
 		return rc;
 	}
 
@@ -1729,7 +1792,9 @@ gso:
 
 		skb->next = nskb->next;
 		nskb->next = NULL;
+		tx_tstamp_start(skb, &context);
 		rc = ops->ndo_start_xmit(nskb, dev);
+		tx_tstamp_end(&context, rc);
 		if (unlikely(rc)) {
 			nskb->next = skb->next;
 			skb->next = nskb;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9e0597d..78ae3e9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2971,9 +2971,9 @@ int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
 EXPORT_SYMBOL_GPL(skb_cow_data);
 
 void skb_tstamp_tx(struct sk_buff *orig_skb,
-		struct skb_shared_hwtstamps *hwtstamps)
+		   struct sock *sk,
+		   struct skb_shared_hwtstamps *hwtstamps)
 {
-	struct sock *sk = orig_skb->sk;
 	struct sock_exterr_skb *serr;
 	struct sk_buff *skb;
 	int err;
-- 
1.6.3.3


[-- Attachment #4: 0003-Change-kernel-timestamping-interface.patch --]
[-- Type: text/x-patch, Size: 11963 bytes --]

>From fc396501b8d0906253b1f85945b86da4130ee665 Mon Sep 17 00:00:00 2001
From: Christopher Zimmermann <madroach@sancho.(none)>
Date: Wed, 7 Oct 2009 17:28:26 +0200
Subject: [PATCH 3/3] Change kernel timestamping interface

Allow userspace to fine-tune hardware registers.
Stay backwards compatible to old userspace?
---
 .gitignore                   |    1 +
 drivers/net/igb/e1000_regs.h |   41 ++++++++++--------
 drivers/net/igb/igb_main.c   |   95 +++++++++++++++++++++++++----------------
 include/linux/net_tstamp.h   |   35 +++++++++++----
 4 files changed, 107 insertions(+), 65 deletions(-)

diff --git a/.gitignore b/.gitignore
index b93fb7e..6fac88c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@
 #
 # Top-level generic files
 #
+debian
 tags
 TAGS
 vmlinux
diff --git a/drivers/net/igb/e1000_regs.h b/drivers/net/igb/e1000_regs.h
index 6e59245..1785b30 100644
--- a/drivers/net/igb/e1000_regs.h
+++ b/drivers/net/igb/e1000_regs.h
@@ -83,30 +83,33 @@
 #define E1000_TSYNCRXCTL_VALID (1<<0)
 #define E1000_TSYNCRXCTL_ENABLED (1<<4)
 enum {
+	/* bits 1 to 3 need leftshift <<1 for direct register access. */
 	E1000_TSYNCRXCTL_TYPE_L2_V2 = 0,
-	E1000_TSYNCRXCTL_TYPE_L4_V1 = (1<<1),
-	E1000_TSYNCRXCTL_TYPE_L2_L4_V2 = (1<<2),
-	E1000_TSYNCRXCTL_TYPE_ALL = (1<<3),
-	E1000_TSYNCRXCTL_TYPE_EVENT_V2 = (1<<3) | (1<<1),
+	E1000_TSYNCRXCTL_TYPE_L4_V1 = (1<<0),
+	E1000_TSYNCRXCTL_TYPE_L2_L4_V2 = (1<<1),
+	E1000_TSYNCRXCTL_TYPE_ALL = (1<<2),
+	E1000_TSYNCRXCTL_TYPE_EVENT_V2 = (1<<2) | (1<<0),
 };
 #define E1000_TSYNCRXCFG 0x05F50
 enum {
-	E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE = 0<<0,
-	E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE = 1<<0,
-	E1000_TSYNCRXCFG_PTP_V1_FOLLOWUP_MESSAGE = 2<<0,
-	E1000_TSYNCRXCFG_PTP_V1_DELAY_RESP_MESSAGE = 3<<0,
-	E1000_TSYNCRXCFG_PTP_V1_MANAGEMENT_MESSAGE = 4<<0,
+	/* bits 0 to 7 */
+	E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE = 0,
+	E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE = 1,
+	E1000_TSYNCRXCFG_PTP_V1_FOLLOWUP_MESSAGE = 2,
+	E1000_TSYNCRXCFG_PTP_V1_DELAY_RESP_MESSAGE = 3,
+	E1000_TSYNCRXCFG_PTP_V1_MANAGEMENT_MESSAGE = 4,
 
-	E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE = 0<<8,
-	E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE = 1<<8,
-	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_REQ_MESSAGE = 2<<8,
-	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_RESP_MESSAGE = 3<<8,
-	E1000_TSYNCRXCFG_PTP_V2_FOLLOWUP_MESSAGE = 8<<8,
-	E1000_TSYNCRXCFG_PTP_V2_DELAY_RESP_MESSAGE = 9<<8,
-	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_FOLLOWUP_MESSAGE = 0xA<<8,
-	E1000_TSYNCRXCFG_PTP_V2_ANNOUNCE_MESSAGE = 0xB<<8,
-	E1000_TSYNCRXCFG_PTP_V2_SIGNALLING_MESSAGE = 0xC<<8,
-	E1000_TSYNCRXCFG_PTP_V2_MANAGEMENT_MESSAGE = 0xD<<8,
+	/* bits 8 to 11 need leftshift <<8 for direct register access */
+	E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE = 0,
+	E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE = 1,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_REQ_MESSAGE = 2,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_RESP_MESSAGE = 3,
+	E1000_TSYNCRXCFG_PTP_V2_FOLLOWUP_MESSAGE = 8,
+	E1000_TSYNCRXCFG_PTP_V2_DELAY_RESP_MESSAGE = 9,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_FOLLOWUP_MESSAGE = 0xA,
+	E1000_TSYNCRXCFG_PTP_V2_ANNOUNCE_MESSAGE = 0xB,
+	E1000_TSYNCRXCFG_PTP_V2_SIGNALLING_MESSAGE = 0xC,
+	E1000_TSYNCRXCFG_PTP_V2_MANAGEMENT_MESSAGE = 0xD,
 };
 #define E1000_SYSTIML 0x0B600
 #define E1000_SYSTIMH 0x0B604
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index adb09d3..17bfd5b 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1512,7 +1512,7 @@ static int __devinit igb_probe(struct pci_dev *pdev,
 	wr32(E1000_TIMINCA,
 	     (1<<24) |
 	     IGB_TSYNC_CYCLE_TIME_IN_NANOSECONDS * IGB_TSYNC_SCALE);
-#if 0
+#if 1
 	/*
 	 * Avoid rollover while we initialize by resetting the time counter.
 	 */
@@ -4881,28 +4881,33 @@ static int igb_hwtstamp_ioctl(struct net_device *netdev,
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	struct hwtstamp_config config;
+#if 0
 	u32 tsync_tx_ctl_bit = E1000_TSYNCTXCTL_ENABLED;
 	u32 tsync_rx_ctl_bit = E1000_TSYNCRXCTL_ENABLED;
 	u32 tsync_rx_ctl_type = 0;
 	u32 tsync_rx_cfg = 0;
 	int is_l4 = 0;
 	int is_l2 = 0;
-	short port = 319; /* PTP */
+#endif
 	u32 regval;
 
+	memset(&config, 0, sizeof(config));
+        /* TODO: first check, weather old or new interface is passed. */
 	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
 		return -EFAULT;
 
 	/* reserved for future extensions */
 	if (config.flags)
-		return -EINVAL;
+		goto manual;
 
 	switch (config.tx_type) {
 	case HWTSTAMP_TX_OFF:
-		tsync_tx_ctl_bit = 0;
+		//tsync_tx_ctl_bit = 0;
+		config.flags &= ~HWTSTAMP_TX;
 		break;
 	case HWTSTAMP_TX_ON:
-		tsync_tx_ctl_bit = E1000_TSYNCTXCTL_ENABLED;
+		//tsync_tx_ctl_bit = E1000_TSYNCTXCTL_ENABLED;
+		config.flags |= HWTSTAMP_TX;
 		break;
 	default:
 		return -ERANGE;
@@ -4910,7 +4915,7 @@ static int igb_hwtstamp_ioctl(struct net_device *netdev,
 
 	switch (config.rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
-		tsync_rx_ctl_bit = 0;
+		config.flags &= ~HWTSTAMP_RX;
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
@@ -4921,57 +4926,68 @@ static int igb_hwtstamp_ioctl(struct net_device *netdev,
 		 * possible to time stamp both Sync and Delay_Req messages
 		 * => fall back to time stamping all packets
 		 */
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_ALL;
+		//tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_ALL;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_ALL;
 		config.rx_filter = HWTSTAMP_FILTER_ALL;
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE;
-		is_l4 = 1;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_L4_V1;
+		config.igb.ptp1_control = E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE;
+		config.igb.port = htons(319); /* PTP */
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE;
-		is_l4 = 1;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_L4_V1;
+		config.igb.ptp1_control = E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE;
+		config.igb.port = htons(319); /* PTP */
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE;
-		is_l2 = 1;
-		is_l4 = 1;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
+		config.igb.ptp2_id = E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE;
+		config.igb.etype = 0x88F7;
+		config.igb.port = htons(319); /* PTP */
 		config.rx_filter = HWTSTAMP_FILTER_SOME;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
 	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
-		tsync_rx_cfg = E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE;
-		is_l2 = 1;
-		is_l4 = 1;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_L2_L4_V2;
+		config.igb.ptp2_id = E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE;
+		config.igb.etype = 0x88F7;
+		config.igb.port = htons(319); /* PTP */
 		config.rx_filter = HWTSTAMP_FILTER_SOME;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
-		tsync_rx_ctl_type = E1000_TSYNCRXCTL_TYPE_EVENT_V2;
+		config.igb.ptp_type = E1000_TSYNCRXCTL_TYPE_EVENT_V2;
 		config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
-		is_l2 = 1;
+		config.igb.etype = 0x88F7;
 		break;
 	default:
 		return -ERANGE;
 	}
 
+manual:
 	/* enable/disable TX */
 	regval = rd32(E1000_TSYNCTXCTL);
-	regval = (regval & ~E1000_TSYNCTXCTL_ENABLED) | tsync_tx_ctl_bit;
+	if(config.flags & HWTSTAMP_TX)
+		regval = (regval | E1000_TSYNCTXCTL_ENABLED);
+	else	regval = (regval & ~E1000_TSYNCTXCTL_ENABLED);
 	wr32(E1000_TSYNCTXCTL, regval);
 
 	/* enable/disable RX, define which PTP packets are time stamped */
 	regval = rd32(E1000_TSYNCRXCTL);
-	regval = (regval & ~E1000_TSYNCRXCTL_ENABLED) | tsync_rx_ctl_bit;
-	regval = (regval & ~0xE) | tsync_rx_ctl_type;
+	if(config.flags & HWTSTAMP_RX)
+		regval = (regval | E1000_TSYNCRXCTL_ENABLED);
+	else	regval = (regval & ~E1000_TSYNCRXCTL_ENABLED);
+	regval = (regval & ~0xE) | config.igb.ptp_type << 1;
 	wr32(E1000_TSYNCRXCTL, regval);
-	wr32(E1000_TSYNCRXCFG, tsync_rx_cfg);
+
+	/* we don't need to read since we can reset the whole register. */
+	regval =  config.igb.ptp1_control
+		| config.igb.ptp2_id << 8
+		| config.igb.ptp2_val << 12;
+	wr32(E1000_TSYNCRXCFG, regval);
 
 	/*
 	 * Ethertype Filter Queue Filter[0][15:0] = 0x88F7
@@ -4979,27 +4995,32 @@ static int igb_hwtstamp_ioctl(struct net_device *netdev,
 	 * Ethertype Filter Queue Filter[0][26] = 0x1 (Enable filter)
 	 * Ethertype Filter Queue Filter[0][30] = 0x1 (Enable Timestamping)
 	 */
-	wr32(E1000_ETQF0, is_l2 ? 0x440088f7 : 0);
+	wr32(E1000_ETQF0, config.igb.etype ? 0x4400 << 16 | config.igb.etype : 0);
 
 	/* L4 Queue Filter[0]: only filter by source and destination port */
-	wr32(E1000_SPQF0, htons(port));
-	wr32(E1000_IMIREXT(0), is_l4 ?
+	/* wr32(E1000_SPQF0, port); XXX why source port? */
+	wr32(E1000_IMIREXT(0), config.igb.port ?
 	     ((1<<12) | (1<<19) /* bypass size and control flags */) : 0);
-	wr32(E1000_IMIR(0), is_l4 ?
-	     (htons(port)
+	wr32(E1000_IMIR(0), config.igb.port ?
+	     (config.igb.port
 	      | (0<<16) /* immediate interrupt disabled */
-	      | 0 /* (1<<17) bit cleared: do not bypass
+	      | (0<<17) /* (1<<17) bit cleared: do not bypass
 		     destination port check */)
 		: 0);
-	wr32(E1000_FTQF0, is_l4 ?
+	wr32(E1000_FTQF0, config.igb.port ?
 	     (0x11 /* UDP */
 	      | (1<<15) /* VF not compared */
 	      | (1<<27) /* Enable Timestamping */
+#if 0
 	      | (7<<28) /* only source port filter enabled,
 			   source/target address and protocol
-			   masked */)
-	     : ((1<<15) | (15<<28) /* all mask bits set = filter not
-				      enabled */));
+			   masked */
+#endif
+	      | (0xe<<28) /* only protocol filter enabled
+			   * everything else is masked. */
+	     )
+	     : (1<<15) | (15<<28) /* all mask bits set = filter not
+				      enabled */);
 
 	wrfl();
 
diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
index a3b8546..7b069dd 100644
--- a/include/linux/net_tstamp.h
+++ b/include/linux/net_tstamp.h
@@ -40,26 +40,43 @@ enum {
  */
 struct hwtstamp_config {
 	int flags;
-	int tx_type;
-	int rx_filter;
+	/* deprecated, only used for backwards compatibility when flags == 0 */
+	int tx_type;            /* put this into flags ? */
+	int rx_filter;          /* deprecated */
+	/* hardware dependend part starts here */
+	union {
+		struct {
+			unsigned short port;  /* for dst port filter, zero to disable */
+			unsigned short etype; /* for ethertype filter, zero to disable */
+			unsigned ptp1_control	:8; /* TSYNCRXCFG.CTRLT */
+			/* Careful! According to reference manual message ids
+			 * 2 and 3 always get timestamped. */
+			unsigned ptp2_id	:4; /* TSYNCRXCFG.MSGT */
+			unsigned ptp2_val	:4; /* TSYNCRXCFG.TRNSSPC */
+			unsigned ptp_type	:3; /* TSYNCRXCTL.Type */
+		} igb;
+	};
 };
 
-/* possible values for hwtstamp_config->tx_type */
+/* flags for hwtstamp_config */
 enum {
 	/*
-	 * No outgoing packet will need hardware time stamping;
-	 * should a packet arrive which asks for it, no hardware
-	 * time stamping will be done.
+	 * these first two values are the only allowed values in
+	 * deprecated hwtstamp_config->tx_type
 	 */
-	HWTSTAMP_TX_OFF,
-
 	/*
 	 * Enables hardware time stamping for outgoing packets;
 	 * the sender of the packet decides which are to be
 	 * time stamped by setting %SOF_TIMESTAMPING_TX_SOFTWARE
 	 * before sending the packet.
+	 * If disabled no packet will be timestamped; even if it
+	 * asks for it.
 	 */
-	HWTSTAMP_TX_ON,
+	HWTSTAMP_TX_OFF = 0, /* only used in hwtstamp_config->tx_type */
+	HWTSTAMP_TX_ON = 1,
+	HWTSTAMP_TX = 1,
+
+	HWTSTAMP_RX = 2
 };
 
 /* possible values for hwtstamp_config->rx_filter */
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-10  8:30 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, xiaosuo, Tom Herbert

ifb: add multi-queue support

Add multi-queue support, and one kernel thread is created for per queue.
It can used to emulate multi-queue NIC in software, and distribute work
among CPUs.
gentux linux # modprobe ifb numtxqs=2
gentux linux # ifconfig ifb0 up
gentux linux # pgrep ifb0
18508
18509
gentux linux # taskset -p 1 18508
pid 18508's current affinity mask: 3
pid 18508's new affinity mask: 1
gentux linux # taskset -p 2 18509
pid 18509's current affinity mask: 3
pid 18509's new affinity mask: 2
gentux linux # tc qdisc add dev br0 ingress
gentux linux # tc filter add dev br0 parent ffff: protocol ip basic
action mirred egress redirect dev ifb0

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
drivers/net/ifb.c | 309
++++++++++++++++++++++++++++++++----------------------
1 file changed, 186 insertions(+), 123 deletions(-)

diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 030913f..6e04188 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -33,139 +33,101 @@
 #include <linux/etherdevice.h>
 #include <linux/init.h>
 #include <linux/moduleparam.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+#include <linux/kthread.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/ip.h>
 #include <net/pkt_sched.h>
 #include <net/net_namespace.h>
 
-#define TX_TIMEOUT  (2*HZ)
-
 #define TX_Q_LIMIT    32
+
 struct ifb_private {
-	struct tasklet_struct   ifb_tasklet;
-	int     tasklet_pending;
-	/* mostly debug stats leave in for now */
-	unsigned long   st_task_enter; /* tasklet entered */
-	unsigned long   st_txq_refl_try; /* transmit queue refill attempt */
-	unsigned long   st_rxq_enter; /* receive queue entered */
-	unsigned long   st_rx2tx_tran; /* receive to trasmit transfers */
-	unsigned long   st_rxq_notenter; /*receiveQ not entered, resched */
-	unsigned long   st_rx_frm_egr; /* received from egress path */
-	unsigned long   st_rx_frm_ing; /* received from ingress path */
-	unsigned long   st_rxq_check;
-	unsigned long   st_rxq_rsch;
-	struct sk_buff_head     rq;
-	struct sk_buff_head     tq;
+	struct net_device	*dev;
+	struct sk_buff_head	rq;
+	struct sk_buff_head	tq;
+	wait_queue_head_t	wq;
+	struct task_struct	*task;
 };
 
+/* Number of ifb devices to be set up by this module. */
 static int numifbs = 2;
+module_param(numifbs, int, 0444);
+MODULE_PARM_DESC(numifbs, "Number of ifb devices");
 
-static void ri_tasklet(unsigned long dev);
-static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev);
-static int ifb_open(struct net_device *dev);
-static int ifb_close(struct net_device *dev);
+/* Number of TX queues per ifb */
+static int numtxqs = 1;
+module_param(numtxqs, int, 0444);
+MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
 
-static void ri_tasklet(unsigned long dev)
+static int ifb_thread(void *priv)
 {
-
-	struct net_device *_dev = (struct net_device *)dev;
-	struct ifb_private *dp = netdev_priv(_dev);
-	struct net_device_stats *stats = &_dev->stats;
-	struct netdev_queue *txq;
+	struct ifb_private *dp = (struct ifb_private*)priv;
+	struct net_device *dev = dp->dev;
+	struct net_device_stats *stats = &dev->stats;
+	unsigned int num = dp - (struct ifb_private*)netdev_priv(dev);
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
 	struct sk_buff *skb;
-
-	txq = netdev_get_tx_queue(_dev, 0);
-	dp->st_task_enter++;
-	if ((skb = skb_peek(&dp->tq)) == NULL) {
-		dp->st_txq_refl_try++;
-		if (__netif_tx_trylock(txq)) {
-			dp->st_rxq_enter++;
-			while ((skb = skb_dequeue(&dp->rq)) != NULL) {
+	DEFINE_WAIT(wait);
+
+	while (1) {
+		/* move skb from rq to tq */
+		while (1) {
+			prepare_to_wait(&dp->wq, &wait, TASK_UNINTERRUPTIBLE);
+			while (!__netif_tx_trylock(txq))
+				yield();
+			while ((skb = skb_dequeue(&dp->rq)) != NULL)
 				skb_queue_tail(&dp->tq, skb);
-				dp->st_rx2tx_tran++;
-			}
+			if (netif_queue_stopped(dev))
+				netif_wake_queue(dev);
 			__netif_tx_unlock(txq);
-		} else {
-			/* reschedule */
-			dp->st_rxq_notenter++;
-			goto resched;
+			if (kthread_should_stop() || !skb_queue_empty(&dp->tq))
+				break;
+			schedule();
 		}
-	}
-
-	while ((skb = skb_dequeue(&dp->tq)) != NULL) {
-		u32 from = G_TC_FROM(skb->tc_verd);
-
-		skb->tc_verd = 0;
-		skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
-		stats->tx_packets++;
-		stats->tx_bytes +=skb->len;
-
-		skb->dev = dev_get_by_index(&init_net, skb->iif);
-		if (!skb->dev) {
-			dev_kfree_skb(skb);
-			stats->tx_dropped++;
+		finish_wait(&dp->wq, &wait);
+		if (kthread_should_stop())
 			break;
-		}
-		dev_put(skb->dev);
-		skb->iif = _dev->ifindex;
-
-		if (from & AT_EGRESS) {
-			dp->st_rx_frm_egr++;
-			dev_queue_xmit(skb);
-		} else if (from & AT_INGRESS) {
-			dp->st_rx_frm_ing++;
-			skb_pull(skb, skb->dev->hard_header_len);
-			netif_rx(skb);
-		} else
-			BUG();
-	}
 
-	if (__netif_tx_trylock(txq)) {
-		dp->st_rxq_check++;
-		if ((skb = skb_peek(&dp->rq)) == NULL) {
-			dp->tasklet_pending = 0;
-			if (netif_queue_stopped(_dev))
-				netif_wake_queue(_dev);
-		} else {
-			dp->st_rxq_rsch++;
-			__netif_tx_unlock(txq);
-			goto resched;
+		/* transfer packets */
+		while ((skb = skb_dequeue(&dp->tq)) != NULL) {
+			u32 from = G_TC_FROM(skb->tc_verd);
+	
+			skb->tc_verd = 0;
+			skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
+			stats->tx_packets++;
+			stats->tx_bytes +=skb->len;
+	
+			skb->dev = dev_get_by_index(&init_net, skb->iif);
+			if (!skb->dev) {
+				dev_kfree_skb(skb);
+				stats->tx_dropped++;
+				break;
+			}
+			dev_put(skb->dev);
+			skb->iif = dev->ifindex;
+	
+			if (from & AT_EGRESS) {
+				dev_queue_xmit(skb);
+			} else if (from & AT_INGRESS) {
+				skb_pull(skb, skb->dev->hard_header_len);
+				netif_rx_ni(skb);
+			} else
+				BUG();
 		}
-		__netif_tx_unlock(txq);
-	} else {
-resched:
-		dp->tasklet_pending = 1;
-		tasklet_schedule(&dp->ifb_tasklet);
 	}
 
-}
-
-static const struct net_device_ops ifb_netdev_ops = {
-	.ndo_open	= ifb_open,
-	.ndo_stop	= ifb_close,
-	.ndo_start_xmit	= ifb_xmit,
-	.ndo_validate_addr = eth_validate_addr,
-};
-
-static void ifb_setup(struct net_device *dev)
-{
-	/* Initialize the device structure. */
-	dev->destructor = free_netdev;
-	dev->netdev_ops = &ifb_netdev_ops;
-
-	/* Fill in device structure with ethernet-generic values. */
-	ether_setup(dev);
-	dev->tx_queue_len = TX_Q_LIMIT;
-
-	dev->flags |= IFF_NOARP;
-	dev->flags &= ~IFF_MULTICAST;
-	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
-	random_ether_addr(dev->dev_addr);
+	return 0;
 }
 
 static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct ifb_private *dp = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	u32 from = G_TC_FROM(skb->tc_verd);
+	int num = skb_get_queue_mapping(skb);
+	struct ifb_private *dp = ((struct ifb_private*)netdev_priv(dev)) + num;
 
 	stats->rx_packets++;
 	stats->rx_bytes+=skb->len;
@@ -182,10 +144,8 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	dev->trans_start = jiffies;
 	skb_queue_tail(&dp->rq, skb);
-	if (!dp->tasklet_pending) {
-		dp->tasklet_pending = 1;
-		tasklet_schedule(&dp->ifb_tasklet);
-	}
+	if (skb_queue_len(&dp->rq) == 1)
+		wake_up(&dp->wq);
 
 	return NETDEV_TX_OK;
 }
@@ -193,26 +153,132 @@ static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
 static int ifb_close(struct net_device *dev)
 {
 	struct ifb_private *dp = netdev_priv(dev);
+	int i;
+
+	for (i = 0; i < dev->real_num_tx_queues; i++) {
+		kthread_stop(dp[i].task);
+		skb_queue_purge(&dp[i].tq);
+		skb_queue_purge(&dp[i].rq);
+	}
 
-	tasklet_kill(&dp->ifb_tasklet);
 	netif_stop_queue(dev);
-	skb_queue_purge(&dp->rq);
-	skb_queue_purge(&dp->tq);
+
 	return 0;
 }
 
 static int ifb_open(struct net_device *dev)
 {
 	struct ifb_private *dp = netdev_priv(dev);
+	int i;
+	
+	for (i = 0; i < dev->real_num_tx_queues; i++) {
+		dp[i].dev = dev;
+		skb_queue_head_init(&dp[i].rq);
+		skb_queue_head_init(&dp[i].tq);
+		init_waitqueue_head(&dp[i].wq);
+		dp[i].task = kthread_run(ifb_thread, &dp[i], "%s/%d", dev->name,
+					i);
+		if (IS_ERR(dp[i].task)) {
+			int err = PTR_ERR(dp[i].task);
+			while (--i >= 0)
+				kthread_stop(dp[i].task);
+			return err;
+		}
+	}
 
-	tasklet_init(&dp->ifb_tasklet, ri_tasklet, (unsigned long)dev);
-	skb_queue_head_init(&dp->rq);
-	skb_queue_head_init(&dp->tq);
 	netif_start_queue(dev);
 
 	return 0;
 }
 
+static u32 simple_tx_hashrnd;
+
+static u16 ifb_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+	u32 addr1, addr2;
+	u32 hash, ihl;
+	union {
+		u16 in16[2];
+		u32 in32;
+	} ports;
+	u8 ip_proto;
+
+	if ((hash = skb_rx_queue_recorded(skb))) {
+		while (hash >= dev->real_num_tx_queues)
+			hash -= dev->real_num_tx_queues;
+		return hash;
+	}
+
+	switch (skb->protocol) {
+	case __constant_htons(ETH_P_IP):
+		if (!(ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)))
+			ip_proto = ip_hdr(skb)->protocol;
+		else
+			ip_proto = 0;
+		addr1 = ip_hdr(skb)->saddr;
+		addr2 = ip_hdr(skb)->daddr;
+		ihl = ip_hdr(skb)->ihl << 2;
+		break;
+	case __constant_htons(ETH_P_IPV6):
+		ip_proto = ipv6_hdr(skb)->nexthdr;
+		addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
+		addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
+		ihl = 10;
+		break;
+	default:
+		return 0;
+	}
+	if (addr1 > addr2)
+		swap(addr1, addr2);
+
+	switch (ip_proto) {
+	case IPPROTO_TCP:
+	case IPPROTO_UDP:
+	case IPPROTO_DCCP:
+	case IPPROTO_ESP:
+	case IPPROTO_AH:
+	case IPPROTO_SCTP:
+	case IPPROTO_UDPLITE:
+		ports.in32 = *((u32 *) (skb_network_header(skb) + ihl));
+		if (ports.in16[0] > ports.in16[1])
+			swap(ports.in16[0], ports.in16[1]);
+		break;
+
+	default:
+		ports.in32 = 0;
+		break;
+	}
+
+	hash = jhash_3words(addr1, addr2, ports.in32,
+			    simple_tx_hashrnd ^ ip_proto);
+
+	return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
+}
+
+static const struct net_device_ops ifb_netdev_ops = {
+	.ndo_open		= ifb_open,
+	.ndo_stop		= ifb_close,
+	.ndo_start_xmit		= ifb_xmit,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_select_queue	= ifb_select_queue,
+};
+
+static void ifb_setup(struct net_device *dev)
+{
+	/* Initialize the device structure. */
+	dev->destructor = free_netdev;
+	dev->netdev_ops = &ifb_netdev_ops;
+
+	/* Fill in device structure with ethernet-generic values. */
+	ether_setup(dev);
+	dev->tx_queue_len = TX_Q_LIMIT;
+
+	dev->flags |= IFF_NOARP;
+	dev->flags &= ~IFF_MULTICAST;
+	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
+	random_ether_addr(dev->dev_addr);
+}
+
 static int ifb_validate(struct nlattr *tb[], struct nlattr *data[])
 {
 	if (tb[IFLA_ADDRESS]) {
@@ -231,17 +297,13 @@ static struct rtnl_link_ops ifb_link_ops __read_mostly = {
 	.validate	= ifb_validate,
 };
 
-/* Number of ifb devices to be set up by this module. */
-module_param(numifbs, int, 0);
-MODULE_PARM_DESC(numifbs, "Number of ifb devices");
-
 static int __init ifb_init_one(int index)
 {
 	struct net_device *dev_ifb;
 	int err;
 
-	dev_ifb = alloc_netdev(sizeof(struct ifb_private),
-				 "ifb%d", ifb_setup);
+	dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d",
+				  ifb_setup, numtxqs);
 
 	if (!dev_ifb)
 		return -ENOMEM;
@@ -266,6 +328,7 @@ static int __init ifb_init_module(void)
 {
 	int i, err;
 
+	get_random_bytes(&simple_tx_hashrnd, 4);
 	rtnl_lock();
 	err = __rtnl_link_register(&ifb_link_ops);
 



^ permalink raw reply related

* Re: Possible bug: SO_TIMESTAMPING 2.6.30+
From: Eric Dumazet @ 2009-11-10  8:44 UTC (permalink / raw)
  To: Christopher Zimmermann; +Cc: Marcus D. Leech, netdev
In-Reply-To: <20091110091252.1667d27d@pundit>

Christopher Zimmermann a écrit :
> On Mon, 09 Nov 2009 19:40:30 -0500
> "Marcus D. Leech" <mleech@ripnet.com> wrote:
> 
> 
>> I know that Patrick Ohly has essentially moved on from doing the 
>> SO_TIMESTAMPING stuff, so
>>    who's maintaining it now?
> 
> I worked on it a month ago or so and have a patchset from Patick Ohly
> and some changes by me which fix software timestamping and make the
> ioctl interface to the hardware more flexible (keeping backwards
> compatibility). Patches are attached.
> Still I never tried IPv6 and don't think the patches do anything about
> it.
> It would be nice to know weather software tx timestamps work
> with/without the patches.
> 
> 
> Christopher
> 

I see some sock_put()/sock_hold() stuff in your second patch.

We removed these things on transmit path, please dont add them back...

If skb->sk is set, it probably also has a destructor and a reference already taken.

(This reference being on sk_wmem_alloc by the way, not on sk_refcnt)

You probably can try to change skb destructor ?



^ permalink raw reply

* Re: [PATCH] act_mirred: don't go back.
From: Changli Gao @ 2009-11-10  8:50 UTC (permalink / raw)
  To: hadi; +Cc: Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <1257838834.3647.4.camel@bigi>

On Tue, Nov 10, 2009 at 3:40 PM, jamal <hadi@cyberus.ca> wrote:
>
> I apologize - I am still not convinced this is a cleanup and i can
> already see holes you are introducing (example not freeing skb etc).

Where? After skb2 is allocated, there won't be any failure any more.

> You are putting me in a dilemma of not wanting to discourage you
> but at the same time not seeing this as a useful change to be made.
> Can we let this one slide?
>

It's just OK. When using tc, I also found act_mirred doesn't support
ingress, then I realized that there isn't any difference between
ingress and egress, as it depends on its parent. However I do think it
is confused, when it prints:
filter parent ffff: protocol ip pref 49152 basic handle 0x1
        action order 1: mirred (Egress Redirect to device ifb0) stolen
        index 5 ref 1 bind 1.
And the TODO note still is in the source code of act_mirred, it do
make me wonder for a while!

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Eric Dumazet @ 2009-11-10  9:07 UTC (permalink / raw)
  To: xiaosuo; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <4AF924A5.1050303@gmail.com>

Changli Gao a écrit :
> ifb: add multi-queue support
> 
> Add multi-queue support, and one kernel thread is created for per queue.
> It can used to emulate multi-queue NIC in software, and distribute work
> among CPUs.
> gentux linux # modprobe ifb numtxqs=2
> gentux linux # ifconfig ifb0 up
> gentux linux # pgrep ifb0
> 18508
> 18509
> gentux linux # taskset -p 1 18508
> pid 18508's current affinity mask: 3
> pid 18508's new affinity mask: 1
> gentux linux # taskset -p 2 18509
> pid 18509's current affinity mask: 3
> pid 18509's new affinity mask: 2
> gentux linux # tc qdisc add dev br0 ingress
> gentux linux # tc filter add dev br0 parent ffff: protocol ip basic
> action mirred egress redirect dev ifb0

Seems pretty cool !

I have some comments 

> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> drivers/net/ifb.c | 309
> ++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 186 insertions(+), 123 deletions(-)
> 
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index 030913f..6e04188 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -33,139 +33,101 @@
>  #include <linux/etherdevice.h>
>  #include <linux/init.h>
>  #include <linux/moduleparam.h>
> +#include <linux/wait.h>
> +#include <linux/sched.h>
> +#include <linux/kthread.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <net/ip.h>
>  #include <net/pkt_sched.h>
>  #include <net/net_namespace.h>
>  
> -#define TX_TIMEOUT  (2*HZ)
> -
>  #define TX_Q_LIMIT    32
> +
>  struct ifb_private {
> -	struct tasklet_struct   ifb_tasklet;
> -	int     tasklet_pending;
> -	/* mostly debug stats leave in for now */
> -	unsigned long   st_task_enter; /* tasklet entered */
> -	unsigned long   st_txq_refl_try; /* transmit queue refill attempt */
> -	unsigned long   st_rxq_enter; /* receive queue entered */
> -	unsigned long   st_rx2tx_tran; /* receive to trasmit transfers */
> -	unsigned long   st_rxq_notenter; /*receiveQ not entered, resched */
> -	unsigned long   st_rx_frm_egr; /* received from egress path */
> -	unsigned long   st_rx_frm_ing; /* received from ingress path */
> -	unsigned long   st_rxq_check;
> -	unsigned long   st_rxq_rsch;
> -	struct sk_buff_head     rq;
> -	struct sk_buff_head     tq;
> +	struct net_device	*dev;
> +	struct sk_buff_head	rq;
> +	struct sk_buff_head	tq;
> +	wait_queue_head_t	wq;
> +	struct task_struct	*task;
>  };

Maybe you should allocate true per_cpu structure, to avoid cache line sharing
and get appropriate NUMA properties.

At least use a __cacheline_aligned_in_smp ...

>  
> +/* Number of ifb devices to be set up by this module. */
>  static int numifbs = 2;
> +module_param(numifbs, int, 0444);
> +MODULE_PARM_DESC(numifbs, "Number of ifb devices");
>  
> -static void ri_tasklet(unsigned long dev);
> -static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev);
> -static int ifb_open(struct net_device *dev);
> -static int ifb_close(struct net_device *dev);
> +/* Number of TX queues per ifb */
> +static int numtxqs = 1;
> +module_param(numtxqs, int, 0444);
> +MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
>  
> -static void ri_tasklet(unsigned long dev)
> +static int ifb_thread(void *priv)
>  {
> -
> -	struct net_device *_dev = (struct net_device *)dev;
> -	struct ifb_private *dp = netdev_priv(_dev);
> -	struct net_device_stats *stats = &_dev->stats;
> -	struct netdev_queue *txq;

> +	struct ifb_private *dp = (struct ifb_private*)priv;

A space is required before * : (struct ifb_private *)priv;
(in many places in your patch)

> +	struct net_device *dev = dp->dev;


> +	struct net_device_stats *stats = &dev->stats;

Here you use a net_device_stats that is shared by all your queues,
your updates wont be protected and some will be lost.
You should use txq->tx_ counters.

> +	unsigned int num = dp - (struct ifb_private*)netdev_priv(dev);
> +	struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
>  	struct sk_buff *skb;
> -
> -	txq = netdev_get_tx_queue(_dev, 0);
> -	dp->st_task_enter++;
> -	if ((skb = skb_peek(&dp->tq)) == NULL) {
> -		dp->st_txq_refl_try++;
> -		if (__netif_tx_trylock(txq)) {
> -			dp->st_rxq_enter++;
> -			while ((skb = skb_dequeue(&dp->rq)) != NULL) {
> +	DEFINE_WAIT(wait);
> +
> +	while (1) {
> +		/* move skb from rq to tq */
> +		while (1) {
> +			prepare_to_wait(&dp->wq, &wait, TASK_UNINTERRUPTIBLE);
> +			while (!__netif_tx_trylock(txq))
> +				yield();
> +			while ((skb = skb_dequeue(&dp->rq)) != NULL)
>  				skb_queue_tail(&dp->tq, skb);
> -				dp->st_rx2tx_tran++;
> -			}
> +			if (netif_queue_stopped(dev))
> +				netif_wake_queue(dev);
>  			__netif_tx_unlock(txq);
> -		} else {
> -			/* reschedule */
> -			dp->st_rxq_notenter++;
> -			goto resched;
> +			if (kthread_should_stop() || !skb_queue_empty(&dp->tq))
> +				break;
> +			schedule();
>  		}
> -	}
> -
> -	while ((skb = skb_dequeue(&dp->tq)) != NULL) {
> -		u32 from = G_TC_FROM(skb->tc_verd);
> -
> -		skb->tc_verd = 0;
> -		skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
> -		stats->tx_packets++;
> -		stats->tx_bytes +=skb->len;
> -
> -		skb->dev = dev_get_by_index(&init_net, skb->iif);
> -		if (!skb->dev) {
> -			dev_kfree_skb(skb);
> -			stats->tx_dropped++;
> +		finish_wait(&dp->wq, &wait);
> +		if (kthread_should_stop())
>  			break;
> -		}
> -		dev_put(skb->dev);
> -		skb->iif = _dev->ifindex;
> -
> -		if (from & AT_EGRESS) {
> -			dp->st_rx_frm_egr++;
> -			dev_queue_xmit(skb);
> -		} else if (from & AT_INGRESS) {
> -			dp->st_rx_frm_ing++;
> -			skb_pull(skb, skb->dev->hard_header_len);
> -			netif_rx(skb);
> -		} else
> -			BUG();
> -	}
>  
> -	if (__netif_tx_trylock(txq)) {
> -		dp->st_rxq_check++;
> -		if ((skb = skb_peek(&dp->rq)) == NULL) {
> -			dp->tasklet_pending = 0;
> -			if (netif_queue_stopped(_dev))
> -				netif_wake_queue(_dev);
> -		} else {
> -			dp->st_rxq_rsch++;
> -			__netif_tx_unlock(txq);
> -			goto resched;
> +		/* transfer packets */
> +		while ((skb = skb_dequeue(&dp->tq)) != NULL) {
> +			u32 from = G_TC_FROM(skb->tc_verd);
> +	
> +			skb->tc_verd = 0;
> +			skb->tc_verd = SET_TC_NCLS(skb->tc_verd);

> +			stats->tx_packets++;
> +			stats->tx_bytes +=skb->len;

Use :			txq->tx_packets++
			txq->tx_bytes += skb->len;
> +	
> +			skb->dev = dev_get_by_index(&init_net, skb->iif);
> +			if (!skb->dev) {
> +				dev_kfree_skb(skb);

> +				stats->tx_dropped++;
			txq->tx_dropped ?

> +				break;
> +			}
> +			dev_put(skb->dev);
> +			skb->iif = dev->ifindex;
> +	
> +			if (from & AT_EGRESS) {
> +				dev_queue_xmit(skb);
> +			} else if (from & AT_INGRESS) {
> +				skb_pull(skb, skb->dev->hard_header_len);
> +				netif_rx_ni(skb);
> +			} else
> +				BUG();
>  		}
> -		__netif_tx_unlock(txq);
> -	} else {
> -resched:
> -		dp->tasklet_pending = 1;
> -		tasklet_schedule(&dp->ifb_tasklet);
>  	}
>  
> -}
> -
> -static const struct net_device_ops ifb_netdev_ops = {
> -	.ndo_open	= ifb_open,
> -	.ndo_stop	= ifb_close,
> -	.ndo_start_xmit	= ifb_xmit,
> -	.ndo_validate_addr = eth_validate_addr,
> -};
> -
> -static void ifb_setup(struct net_device *dev)
> -{
> -	/* Initialize the device structure. */
> -	dev->destructor = free_netdev;
> -	dev->netdev_ops = &ifb_netdev_ops;
> -
> -	/* Fill in device structure with ethernet-generic values. */
> -	ether_setup(dev);
> -	dev->tx_queue_len = TX_Q_LIMIT;
> -
> -	dev->flags |= IFF_NOARP;
> -	dev->flags &= ~IFF_MULTICAST;
> -	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> -	random_ether_addr(dev->dev_addr);
> +	return 0;
>  }
>  
>  static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
> -	struct ifb_private *dp = netdev_priv(dev);
>  	struct net_device_stats *stats = &dev->stats;
>  	u32 from = G_TC_FROM(skb->tc_verd);
> +	int num = skb_get_queue_mapping(skb);
> +	struct ifb_private *dp = ((struct ifb_private*)netdev_priv(dev)) + num;
>  

>  	stats->rx_packets++;
>  	stats->rx_bytes+=skb->len;

Not sure how to solve this problem (several cpus can updates counter in //)

Thanks

^ permalink raw reply

* Re: [PATCH 2/2] au1000-eth: convert to platform_driver model
From: Manuel Lauss @ 2009-11-10  9:20 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Ralf Baechle, linux-mips, netdev, David Miller
In-Reply-To: <200911100113.38685.florian@openwrt.org>

Hi Florian,

On Tue, Nov 10, 2009 at 1:13 AM, Florian Fainelli <florian@openwrt.org> wrote:
> This patch converts the au1000-eth driver to become a full
> platform-driver as it ought to be. We now pass PHY-speficic
> configurations through platform_data but for compatibility
> the driver still assumes the default settings (search for PHY1 on
> MAC0) when no platform_data is passed. Tested on my MTX-1 board.
>
> Acked-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
> index ce6f1ac..6d5a2cb 100644
> --- a/drivers/net/au1000_eth.c
> +++ b/drivers/net/au1000_eth.c

> -# if defined(AU1XXX_PHY1_SEARCH_ON_MAC0)
> -       /* try harder to find a PHY */
> -       if (!phydev && (aup->mac_id == 1)) {
> -               /* no PHY found, maybe we have a dual PHY? */
> -               printk (KERN_INFO DRV_NAME ": no PHY found on MAC1, "
> -                       "let's see if it's attached to MAC0...\n");
> -
> -               BUG_ON(!au_macs[0]);


> -               /* find the first (lowest address) non-attached PHY on
> -                * the MAC0 MII bus */
> -               for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++) {
> -                       struct phy_device *const tmp_phydev =
> -                               au_macs[0]->mii_bus->phy_map[phy_addr];
> -
> -                       if (!tmp_phydev)
> -                               continue; /* no PHY here... */
> -
> -                       if (tmp_phydev->attached_dev)
> -                               continue; /* already claimed by MAC0 */
> +       } else {
> +               int phy_addr;
> +
> +               /* find the first (lowest address) PHY on the current MAC's MII bus */
> +               for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++)
> +                       if (aup->mii_bus->phy_map[phy_addr]) {
> +                               phydev = aup->mii_bus->phy_map[phy_addr];
> +                               if (!aup->phy_search_highest_addr)
> +                                       break; /* break out with first one found */
> +                       }
>
> -                       phydev = tmp_phydev;
> -                       break; /* found it */
> +               if (aup->phy1_search_mac0) {
> +                       /* try harder to find a PHY */
> +                       if (!phydev && (aup->mac_id == 1)) {
> +                               /* no PHY found, maybe we have a dual PHY? */
> +                               printk (KERN_INFO DRV_NAME ": no PHY found on MAC1, "
> +                                       "let's see if it's attached to MAC0...\n");
> +
> +                               /* find the first (lowest address) non-attached PHY on
> +                                * the MAC0 MII bus */
> +                               for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++) {
> +                                       if (aup->mac_id == 1)
> +                                               break;

aup->mac_id needs to be 1 for this loop to be executed in the first
place, and here
you immediately bail out if it is.
Also, how do you access the phy map of the other controller without use of the
au_macs[] structure? (which is unused after this patch and could be
removed, along
with the NUM_ETH_INTERFACES constant)


> +                                       struct phy_device *const tmp_phydev =
> +                                                       aup->mii_bus->phy_map[phy_addr];

My compiler complains about mixed code/declarations.


Thanks!
      Manuel Lauss

^ permalink raw reply

* Re: [PATCH] e1000e: Fix usage under kexec
From: Andi Kleen @ 2009-11-10  9:35 UTC (permalink / raw)
  To: Sven Anders; +Cc: netdev
In-Reply-To: <4AF87AB1.1070603@anduras.de>

Sven Anders <sven@anduras.de> writes:
>
> We get the following error:
>   e1000e: probe of 0000:02:00.0 failed with error -2
>
> We cannot unload the driver of the first kernel, because it's
> compiled in statically.
>
> It's caused by the PHY, because it's already initialized by the
> first driver and therefore does not respond correctly. The driver
> assumes the PHY to be in 'reset' state, which is done by the BIOS.
>
> We fixed it by adding a
>
> +       /* Reset PHY before initializing it. Allows re-init after kexec. */
> +       if (!e1000_check_reset_block(hw))
> +         e1000_phy_hw_reset(hw);

The problem of resetting PHYs in the driver init path is that it tends
to lose link-state, and if the link takes some time to re-negotiate
you can significantly slow down the "total time to network access"
at boot.

Perhaps you could only reset when the message above would trigger
and try again?

-Andi

^ permalink raw reply

* Re: [RFC,PATCH] mutex: mutex_is_owner() helper
From: Peter Zijlstra @ 2009-11-10  9:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ingo Molnar, Linus Torvalds, David S. Miller, Linux Netdev List,
	linux kernel, Thomas Gleixner
In-Reply-To: <4AF8A40B.10708@gmail.com>

On Tue, 2009-11-10 at 00:21 +0100, Eric Dumazet wrote:
> Peter Zijlstra a écrit :
> > On Wed, 2009-11-04 at 18:19 +0100, Eric Dumazet wrote:
> >> BTW, I was thinking of a mutex_yield() implementation, but could not
> >> cook it without hard thinking, maybe you already have some nice
> >> implementation ?
> > 
> > Why? Yield sets off alarm bells, since 99.9%, and possibly more, of its
> > uses are wrong.
> 
> If I remember well, I had problems doing "modprobe dummy numdummies=30000",
> because it creates 30000 netdevices, and thanks to hotplug starts 30000 udev
> that all wait that my modprobe is finished... Nice to see load average going
> so big by the way :)

lol :-) With a bit of luck udev will spawn a python interpreter for each
of those things too..

> I tried following patch without success, because rtnl_unlock()/rtnl_lock()
> is too fast (awaken process(es) ha(s/ve) no chance to get the lock, as we
> take it immediately after releasing it)

Right, due to lock-stealing.

> diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
> index 37dcfdc..108c4fa 100644
> --- a/drivers/net/dummy.c
> +++ b/drivers/net/dummy.c
> @@ -138,8 +138,12 @@ static int __init dummy_init_module(void)
>  	rtnl_lock();
>  	err = __rtnl_link_register(&dummy_link_ops);
>  
> -	for (i = 0; i < numdummies && !err; i++)
> +	for (i = 0; i < numdummies && !err; i++) {
>  		err = dummy_init_one();
> +		rtnl_unlock();
> +		msleep(1);
> +		rtnl_lock();
> +	}
>  	if (err < 0)
>  		__rtnl_link_unregister(&dummy_link_ops);
>  	rtnl_unlock();
> 
> But if hotplug is disabled, this force a useless msleep(1) * 30000 -> this is bit slow
> 
> Yes, this code is stupid, but I use it to stress network stack
> with insane number of devices, to spot scalability problems.

Right...

> mutex_yield() could help in this situation.

Agreed, except I don't like the name, but I could be tained from
sched_yield().

> mutex is said to be FIFO, but its not exactly true : A new comer can take the mutex
> even if 10000 threads are waiting on mutex...

Yep, lock-stealing, you don't want to see the regression reports if you
'fix' that :-)

> I wont mention other problems, because mutex_{try}lock() has no timedwait variant

Nobody needed it I guess.. also I never quite understood the need for
timedwait, either you need to get the work done or you don't, not maybe.

Use mutex_lock_interruptible() and set a timer or something.

> , and funny code doing :
> 
> if (!rtnl_trylock())
> 	return restart_syscall();
> 
> Making 30000 processes running/fighting to get the mutex :(

Funny definition of funny ;-) That's some seriously fugly code there.



^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-10  9:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <4AF92D6D.8060300@gmail.com>

2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
>
> Not sure how to solve this problem (several cpus can updates counter in //)
>
Thanks, and follow your suggestions. I can maintain the counter per TX
queue, and update it in a timer handler like ixgbe or implement our
own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev),
and update the counters when it gets called.



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-10 10:29 UTC (permalink / raw)
  To: xiaosuo; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <4AF924A5.1050303@gmail.com>

Changli Gao wrote:
> ifb: add multi-queue support
> 
> Add multi-queue support, and one kernel thread is created for per queue.
> It can used to emulate multi-queue NIC in software, and distribute work
> among CPUs.
> gentux linux # modprobe ifb numtxqs=2
> gentux linux # ifconfig ifb0 up
> gentux linux # pgrep ifb0
> 18508
> 18509
> gentux linux # taskset -p 1 18508
> pid 18508's current affinity mask: 3
> pid 18508's new affinity mask: 1
> gentux linux # taskset -p 2 18509
> pid 18509's current affinity mask: 3
> pid 18509's new affinity mask: 2
> gentux linux # tc qdisc add dev br0 ingress
> gentux linux # tc filter add dev br0 parent ffff: protocol ip basic
> action mirred egress redirect dev ifb0

I'm not sure how this will help, ifb device transmission is
still serialized by the ingress queue lock and the mirred lock.

^ permalink raw reply

* netfilter -stable 00/02: netfilter -stable fixes
From: Patrick McHardy @ 2009-11-10 10:40 UTC (permalink / raw)
  To: stable; +Cc: netdev, Patrick McHardy, netfilter-devel, davem

The following two patches fix two bug in netfilter:

- a bug in TCP conntrack sequence tracking when used with NAT helpers that
  enlarge packets

- a regression in the xt_connlimit match introduced in 2.6.29, causing
  false negatives

Please apply, thanks.


 include/net/netfilter/nf_conntrack.h   |    8 +--
 include/net/netfilter/nf_nat_helper.h  |    4 ++
 net/ipv4/netfilter/nf_nat_core.c       |    3 +
 net/ipv4/netfilter/nf_nat_helper.c     |   34 +++++++++++-----
 net/netfilter/nf_conntrack_core.c      |    8 ++++
 net/netfilter/nf_conntrack_proto_tcp.c |   64 +++++++++++++-------------------
 net/netfilter/xt_connlimit.c           |   10 ++---
 7 files changed, 71 insertions(+), 60 deletions(-)

Jan Engelhardt (1):
      netfilter: xt_connlimit: fix regression caused by zero family value

Jozsef Kadlecsik (1):
      netfilter: nf_nat: fix NAT issue in 2.6.30.4+

^ permalink raw reply

* netfilter -stable 01/02: nf_nat: fix NAT issue in 2.6.30.4+
From: Patrick McHardy @ 2009-11-10 10:40 UTC (permalink / raw)
  To: stable; +Cc: netdev, Patrick McHardy, netfilter-devel, davem
In-Reply-To: <20091110104014.8250.89589.sendpatchset@x2.localnet>

commit eb3336f5b440ae5c15d09abe20d7bdd45a88d157
Author: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date:   Tue Nov 10 10:53:08 2009 +0100

    netfilter: nf_nat: fix NAT issue in 2.6.30.4+
    
    Upstream commit f9dd09c7:
    
    Vitezslav Samel discovered that since 2.6.30.4+ active FTP can not work
    over NAT. The "cause" of the problem was a fix of unacknowledged data
    detection with NAT (commit a3a9f79e361e864f0e9d75ebe2a0cb43d17c4272).
    However, actually, that fix uncovered a long standing bug in TCP conntrack:
    when NAT was enabled, we simply updated the max of the right edge of
    the segments we have seen (td_end), by the offset NAT produced with
    changing IP/port in the data. However, we did not update the other parameter
    (td_maxend) which is affected by the NAT offset. Thus that could drift
    away from the correct value and thus resulted breaking active FTP.
    
    The patch below fixes the issue by *not* updating the conntrack parameters
    from NAT, but instead taking into account the NAT offsets in conntrack in a
    consistent way. (Updating from NAT would be more harder and expensive because
    it'd need to re-calculate parameters we already calculated in conntrack.)
    
    Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index 5d9a848..a96b835 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -252,11 +252,9 @@ static inline bool nf_ct_kill(struct nf_conn *ct)
 }
 
 /* These are for NAT.  Icky. */
-/* Update TCP window tracking data when NAT mangles the packet */
-extern void nf_conntrack_tcp_update(const struct sk_buff *skb,
-				    unsigned int dataoff,
-				    struct nf_conn *ct, int dir,
-				    s16 offset);
+extern s16 (*nf_ct_nat_offset)(const struct nf_conn *ct,
+			       enum ip_conntrack_dir dir,
+			       u32 seq);
 
 /* Fake conntrack entry for untracked connections */
 extern struct nf_conn nf_conntrack_untracked;
diff --git a/include/net/netfilter/nf_nat_helper.h b/include/net/netfilter/nf_nat_helper.h
index 237a961..4222220 100644
--- a/include/net/netfilter/nf_nat_helper.h
+++ b/include/net/netfilter/nf_nat_helper.h
@@ -32,4 +32,8 @@ extern int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
  * to port ct->master->saved_proto. */
 extern void nf_nat_follow_master(struct nf_conn *ct,
 				 struct nf_conntrack_expect *this);
+
+extern s16 nf_nat_get_offset(const struct nf_conn *ct,
+			     enum ip_conntrack_dir dir,
+			     u32 seq);
 #endif
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index b6ddd56..d396abf 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -750,6 +750,8 @@ static int __init nf_nat_init(void)
 	BUG_ON(nfnetlink_parse_nat_setup_hook != NULL);
 	rcu_assign_pointer(nfnetlink_parse_nat_setup_hook,
 			   nfnetlink_parse_nat_setup);
+	BUG_ON(nf_ct_nat_offset != NULL);
+	rcu_assign_pointer(nf_ct_nat_offset, nf_nat_get_offset);
 	return 0;
 
  cleanup_extend:
@@ -764,6 +766,7 @@ static void __exit nf_nat_cleanup(void)
 	nf_ct_extend_unregister(&nat_extend);
 	rcu_assign_pointer(nf_nat_seq_adjust_hook, NULL);
 	rcu_assign_pointer(nfnetlink_parse_nat_setup_hook, NULL);
+	rcu_assign_pointer(nf_ct_nat_offset, NULL);
 	synchronize_net();
 }
 
diff --git a/net/ipv4/netfilter/nf_nat_helper.c b/net/ipv4/netfilter/nf_nat_helper.c
index 05ede41..79e28ad 100644
--- a/net/ipv4/netfilter/nf_nat_helper.c
+++ b/net/ipv4/netfilter/nf_nat_helper.c
@@ -73,6 +73,28 @@ adjust_tcp_sequence(u32 seq,
 	DUMP_OFFSET(this_way);
 }
 
+/* Get the offset value, for conntrack */
+s16 nf_nat_get_offset(const struct nf_conn *ct,
+		      enum ip_conntrack_dir dir,
+		      u32 seq)
+{
+	struct nf_conn_nat *nat = nfct_nat(ct);
+	struct nf_nat_seq *this_way;
+	s16 offset;
+
+	if (!nat)
+		return 0;
+
+	this_way = &nat->seq[dir];
+	spin_lock_bh(&nf_nat_seqofs_lock);
+	offset = after(seq, this_way->correction_pos)
+		 ? this_way->offset_after : this_way->offset_before;
+	spin_unlock_bh(&nf_nat_seqofs_lock);
+
+	return offset;
+}
+EXPORT_SYMBOL_GPL(nf_nat_get_offset);
+
 /* Frobs data inside this packet, which is linear. */
 static void mangle_contents(struct sk_buff *skb,
 			    unsigned int dataoff,
@@ -189,11 +211,6 @@ nf_nat_mangle_tcp_packet(struct sk_buff *skb,
 		adjust_tcp_sequence(ntohl(tcph->seq),
 				    (int)rep_len - (int)match_len,
 				    ct, ctinfo);
-		/* Tell TCP window tracking about seq change */
-		nf_conntrack_tcp_update(skb, ip_hdrlen(skb),
-					ct, CTINFO2DIR(ctinfo),
-					(int)rep_len - (int)match_len);
-
 		nf_conntrack_event_cache(IPCT_NATSEQADJ, ct);
 	}
 	return 1;
@@ -415,12 +432,7 @@ nf_nat_seq_adjust(struct sk_buff *skb,
 	tcph->seq = newseq;
 	tcph->ack_seq = newack;
 
-	if (!nf_nat_sack_adjust(skb, tcph, ct, ctinfo))
-		return 0;
-
-	nf_conntrack_tcp_update(skb, ip_hdrlen(skb), ct, dir, seqoff);
-
-	return 1;
+	return nf_nat_sack_adjust(skb, tcph, ct, ctinfo);
 }
 
 /* Setup NAT on this expected conntrack so it follows master. */
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 0d961ee..56ed268 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1296,6 +1296,11 @@ err_stat:
 	return ret;
 }
 
+s16 (*nf_ct_nat_offset)(const struct nf_conn *ct,
+			enum ip_conntrack_dir dir,
+			u32 seq);
+EXPORT_SYMBOL_GPL(nf_ct_nat_offset);
+
 int nf_conntrack_init(struct net *net)
 {
 	int ret;
@@ -1313,6 +1318,9 @@ int nf_conntrack_init(struct net *net)
 		/* For use by REJECT target */
 		rcu_assign_pointer(ip_ct_attach, nf_conntrack_attach);
 		rcu_assign_pointer(nf_ct_destroy, destroy_conntrack);
+
+		/* Howto get NAT offsets */
+		rcu_assign_pointer(nf_ct_nat_offset, NULL);
 	}
 	return 0;
 
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index a38bc22..718c334 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -482,6 +482,21 @@ static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff,
 	}
 }
 
+#ifdef CONFIG_NF_NAT_NEEDED
+static inline s16 nat_offset(const struct nf_conn *ct,
+			     enum ip_conntrack_dir dir,
+			     u32 seq)
+{
+	typeof(nf_ct_nat_offset) get_offset = rcu_dereference(nf_ct_nat_offset);
+
+	return get_offset != NULL ? get_offset(ct, dir, seq) : 0;
+}
+#define NAT_OFFSET(pf, ct, dir, seq) \
+	(pf == NFPROTO_IPV4 ? nat_offset(ct, dir, seq) : 0)
+#else
+#define NAT_OFFSET(pf, ct, dir, seq)	0
+#endif
+
 static bool tcp_in_window(const struct nf_conn *ct,
 			  struct ip_ct_tcp *state,
 			  enum ip_conntrack_dir dir,
@@ -496,6 +511,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
 	struct ip_ct_tcp_state *receiver = &state->seen[!dir];
 	const struct nf_conntrack_tuple *tuple = &ct->tuplehash[dir].tuple;
 	__u32 seq, ack, sack, end, win, swin;
+	s16 receiver_offset;
 	bool res;
 
 	/*
@@ -509,11 +525,16 @@ static bool tcp_in_window(const struct nf_conn *ct,
 	if (receiver->flags & IP_CT_TCP_FLAG_SACK_PERM)
 		tcp_sack(skb, dataoff, tcph, &sack);
 
+	/* Take into account NAT sequence number mangling */
+	receiver_offset = NAT_OFFSET(pf, ct, !dir, ack - 1);
+	ack -= receiver_offset;
+	sack -= receiver_offset;
+
 	pr_debug("tcp_in_window: START\n");
 	pr_debug("tcp_in_window: ");
 	nf_ct_dump_tuple(tuple);
-	pr_debug("seq=%u ack=%u sack=%u win=%u end=%u\n",
-		 seq, ack, sack, win, end);
+	pr_debug("seq=%u ack=%u+(%d) sack=%u+(%d) win=%u end=%u\n",
+		 seq, ack, receiver_offset, sack, receiver_offset, win, end);
 	pr_debug("tcp_in_window: sender end=%u maxend=%u maxwin=%u scale=%i "
 		 "receiver end=%u maxend=%u maxwin=%u scale=%i\n",
 		 sender->td_end, sender->td_maxend, sender->td_maxwin,
@@ -599,8 +620,8 @@ static bool tcp_in_window(const struct nf_conn *ct,
 
 	pr_debug("tcp_in_window: ");
 	nf_ct_dump_tuple(tuple);
-	pr_debug("seq=%u ack=%u sack =%u win=%u end=%u\n",
-		 seq, ack, sack, win, end);
+	pr_debug("seq=%u ack=%u+(%d) sack=%u+(%d) win=%u end=%u\n",
+		 seq, ack, receiver_offset, sack, receiver_offset, win, end);
 	pr_debug("tcp_in_window: sender end=%u maxend=%u maxwin=%u scale=%i "
 		 "receiver end=%u maxend=%u maxwin=%u scale=%i\n",
 		 sender->td_end, sender->td_maxend, sender->td_maxwin,
@@ -686,7 +707,7 @@ static bool tcp_in_window(const struct nf_conn *ct,
 			before(seq, sender->td_maxend + 1) ?
 			after(end, sender->td_end - receiver->td_maxwin - 1) ?
 			before(sack, receiver->td_end + 1) ?
-			after(ack, receiver->td_end - MAXACKWINDOW(sender)) ? "BUG"
+			after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1) ? "BUG"
 			: "ACK is under the lower bound (possible overly delayed ACK)"
 			: "ACK is over the upper bound (ACKed data not seen yet)"
 			: "SEQ is under the lower bound (already ACKed data retransmitted)"
@@ -701,39 +722,6 @@ static bool tcp_in_window(const struct nf_conn *ct,
 	return res;
 }
 
-#ifdef CONFIG_NF_NAT_NEEDED
-/* Update sender->td_end after NAT successfully mangled the packet */
-/* Caller must linearize skb at tcp header. */
-void nf_conntrack_tcp_update(const struct sk_buff *skb,
-			     unsigned int dataoff,
-			     struct nf_conn *ct, int dir,
-			     s16 offset)
-{
-	const struct tcphdr *tcph = (const void *)skb->data + dataoff;
-	const struct ip_ct_tcp_state *sender = &ct->proto.tcp.seen[dir];
-	const struct ip_ct_tcp_state *receiver = &ct->proto.tcp.seen[!dir];
-	__u32 end;
-
-	end = segment_seq_plus_len(ntohl(tcph->seq), skb->len, dataoff, tcph);
-
-	write_lock_bh(&tcp_lock);
-	/*
-	 * We have to worry for the ack in the reply packet only...
-	 */
-	if (ct->proto.tcp.seen[dir].td_end + offset == end)
-		ct->proto.tcp.seen[dir].td_end = end;
-	ct->proto.tcp.last_end = end;
-	write_unlock_bh(&tcp_lock);
-	pr_debug("tcp_update: sender end=%u maxend=%u maxwin=%u scale=%i "
-		 "receiver end=%u maxend=%u maxwin=%u scale=%i\n",
-		 sender->td_end, sender->td_maxend, sender->td_maxwin,
-		 sender->td_scale,
-		 receiver->td_end, receiver->td_maxend, receiver->td_maxwin,
-		 receiver->td_scale);
-}
-EXPORT_SYMBOL_GPL(nf_conntrack_tcp_update);
-#endif
-
 #define	TH_FIN	0x01
 #define	TH_SYN	0x02
 #define	TH_RST	0x04

^ permalink raw reply related

* netfilter -stable 02/02: xt_connlimit: fix regression caused by zero family value
From: Patrick McHardy @ 2009-11-10 10:40 UTC (permalink / raw)
  To: stable; +Cc: netdev, Patrick McHardy, netfilter-devel, davem
In-Reply-To: <20091110104014.8250.89589.sendpatchset@x2.localnet>

commit 4dcaa8fa124a6d8ab0f64dc1c289b13a724f8fc1
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Tue Nov 10 10:53:33 2009 +0100

    netfilter: xt_connlimit: fix regression caused by zero family value
    
    Upstream commit 539054a8:
    
    Commit v2.6.28-rc1~717^2~109^2~2 was slightly incomplete; not all
    instances of par->match->family were changed to par->family.
    
    References: http://bugzilla.netfilter.org/show_bug.cgi?id=610
    Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index 6809809..38f03f7 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -103,7 +103,7 @@ static int count_them(struct xt_connlimit_data *data,
 		      const struct nf_conntrack_tuple *tuple,
 		      const union nf_inet_addr *addr,
 		      const union nf_inet_addr *mask,
-		      const struct xt_match *match)
+		      u_int8_t family)
 {
 	const struct nf_conntrack_tuple_hash *found;
 	struct xt_connlimit_conn *conn;
@@ -113,8 +113,7 @@ static int count_them(struct xt_connlimit_data *data,
 	bool addit = true;
 	int matches = 0;
 
-
-	if (match->family == NFPROTO_IPV6)
+	if (family == NFPROTO_IPV6)
 		hash = &data->iphash[connlimit_iphash6(addr, mask)];
 	else
 		hash = &data->iphash[connlimit_iphash(addr->ip & mask->ip)];
@@ -157,8 +156,7 @@ static int count_them(struct xt_connlimit_data *data,
 			continue;
 		}
 
-		if (same_source_net(addr, mask, &conn->tuple.src.u3,
-		    match->family))
+		if (same_source_net(addr, mask, &conn->tuple.src.u3, family))
 			/* same source network -> be counted! */
 			++matches;
 		nf_ct_put(found_ct);
@@ -207,7 +205,7 @@ connlimit_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 
 	spin_lock_bh(&info->data->lock);
 	connections = count_them(info->data, tuple_ptr, &addr,
-	                         &info->mask, par->match);
+	                         &info->mask, par->family);
 	spin_unlock_bh(&info->data->lock);
 
 	if (connections < 0) {

^ permalink raw reply related

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-10 10:48 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <4AF9406C.8010603@trash.net>

On Tue, Nov 10, 2009 at 6:29 PM, Patrick McHardy <kaber@trash.net> wrote:
>
> I'm not sure how this will help, ifb device transmission is
> still serialized by the ingress queue lock and the mirred lock.
>

But not in ifb_thread. For firewall the main routine is after
netif_rx_ni(). For server applications, the packets belong to the same
flow are always routed to the same CPU, and the applications can
benefit with better localization.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Eric Dumazet @ 2009-11-10 10:55 UTC (permalink / raw)
  To: Changli Gao; +Cc: Patrick McHardy, David S. Miller, netdev, Tom Herbert
In-Reply-To: <412e6f7f0911100248mbc14287p26c43141d6ab202c@mail.gmail.com>

Changli Gao a écrit :
> On Tue, Nov 10, 2009 at 6:29 PM, Patrick McHardy <kaber@trash.net> wrote:
>> I'm not sure how this will help, ifb device transmission is
>> still serialized by the ingress queue lock and the mirred lock.
>>
> 
> But not in ifb_thread. For firewall the main routine is after
> netif_rx_ni(). For server applications, the packets belong to the same
> flow are always routed to the same CPU, and the applications can
> benefit with better localization.
> 

Hmm, you know many cache lines already were bringed into cpu receiving
the original softirq ? But yes, this is a possible way to go / try :)

Please submit your future patch on top of net-next-2.6, because
I see you still use dev_get_by_index() :-(

commit 05e8689c9a3a208bf75b60662778d81e23eac460
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date:   Sun Nov 1 19:45:16 2009 +0000

    ifb: RCU locking avoids touching dev refcount
    
    Avoids touching dev refcount in hotpath
    
    Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit db519144243de6b17ff0c56c26f06059743110a7
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date:   Tue Oct 20 02:35:50 2009 +0000

    ifb: should not use __dev_get_by_index() without locks
    
    At this point (ri_tasklet()), RTNL or dev_base_lock are not held,
    we must use dev_get_by_index() instead of __dev_get_by_index()
    
    Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>



^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Eric Dumazet @ 2009-11-10 10:57 UTC (permalink / raw)
  To: Changli Gao; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <412e6f7f0911100143k68284faes92e39d8ca94aa9d4@mail.gmail.com>

Changli Gao a écrit :
> 2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
>> Not sure how to solve this problem (several cpus can updates counter in //)
>>
> Thanks, and follow your suggestions. I can maintain the counter per TX
> queue, and update it in a timer handler like ixgbe or implement our
> own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev),
> and update the counters when it gets called.

Please no timer stuff :)


^ permalink raw reply

* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Patrick McHardy @ 2009-11-10 11:04 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Netdev List, Jarek Poplawski, David S. Miller,
	Stephen Hemminger
In-Reply-To: <20091109195000.GA10325@gondor.apana.org.au>

Herbert Xu wrote:
> On Mon, Nov 09, 2009 at 08:41:36PM +0100, Patrick McHardy wrote:
>> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
>>   skbs is optimal. When the driver returns an error, it is assumed
>>   the current segment has been freed. The patch then frees the
>>   entire GSO skb, including all remaining segments. Alternatively
>>   it could try to transmit the remaining segments later.
> 
> Well driver errors (not queueing errors) should never happen.

Yeah, usually there will only be queueing errors. One case for
a non-queueing error might be to return EHOSTUNREACH from ipip
or gre when there's no route to the peer.

> And if they do then they're likely to persist.  So freeing the
> rest should be sufficient, unless of course if doing it some
> other way is simpler :)

This way seems simpler. Thanks.

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-10 11:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <4AF9471C.5080606@gmail.com>

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

2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
> Changli Gao a écrit :
>> 2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
>>> Not sure how to solve this problem (several cpus can updates counter in //)
>>>
>> Thanks, and follow your suggestions. I can maintain the counter per TX
>> queue, and update it in a timer handler like ixgbe or implement our
>> own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev),
>> and update the counters when it gets called.
>
> Please no timer stuff :)

The whole ifb.c file is attached, please review and test it. Thanks!


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

[-- Attachment #2: ifb.c --]
[-- Type: application/octet-stream, Size: 9063 bytes --]

/* drivers/net/ifb.c:

	The purpose of this driver is to provide a device that allows
	for sharing of resources:

	1) qdiscs/policies that are per device as opposed to system wide.
	ifb allows for a device which can be redirected to thus providing
	an impression of sharing.

	2) Allows for queueing incoming traffic for shaping instead of
	dropping.

	The original concept is based on what is known as the IMQ
	driver initially written by Martin Devera, later rewritten
	by Patrick McHardy and then maintained by Andre Correa.

	You need the tc action  mirror or redirect to feed this device
       	packets.

	This program is free software; you can redistribute it and/or
	modify it under the terms of the GNU General Public License
	as published by the Free Software Foundation; either version
	2 of the License, or (at your option) any later version.

  	Authors:	Jamal Hadi Salim (2005)

*/


#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/init.h>
#include <linux/moduleparam.h>
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/kthread.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <net/ip.h>
#include <net/pkt_sched.h>
#include <net/net_namespace.h>

#define TX_Q_LIMIT    32

struct ifb_private {
	struct net_device	*dev;
	struct sk_buff_head	rq;
	struct sk_buff_head	tq;
	wait_queue_head_t	wq;
	struct task_struct	*task;
	unsigned long		rx_packets;
	unsigned long		rx_bytes;
	unsigned long		rx_dropped;
} ____cacheline_aligned_in_smp;

/* Number of ifb devices to be set up by this module. */
static int numifbs = 2;
module_param(numifbs, int, 0444);
MODULE_PARM_DESC(numifbs, "Number of ifb devices");

/* Number of TX queues per ifb */
static int numtxqs = 1;
module_param(numtxqs, int, 0444);
MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");

static int ifb_thread(void *priv)
{
	struct ifb_private *dp = (struct ifb_private *)priv;
	struct net_device *dev = dp->dev;
	unsigned int num = dp - (struct ifb_private *)netdev_priv(dev);
	struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
	struct sk_buff *skb;
	DEFINE_WAIT(wait);

	while (1) {
		/* move skb from rq to tq */
		while (1) {
			prepare_to_wait(&dp->wq, &wait, TASK_UNINTERRUPTIBLE);
			while (!__netif_tx_trylock(txq))
				yield();
			while ((skb = skb_dequeue(&dp->rq)) != NULL)
				skb_queue_tail(&dp->tq, skb);
			if (netif_queue_stopped(dev))
				netif_wake_queue(dev);
			__netif_tx_unlock(txq);
			if (kthread_should_stop() || !skb_queue_empty(&dp->tq))
				break;
			schedule();
		}
		finish_wait(&dp->wq, &wait);
		if (kthread_should_stop())
			break;

		/* transfer packets */
		while ((skb = skb_dequeue(&dp->tq)) != NULL) {
			u32 from = G_TC_FROM(skb->tc_verd);
	
			skb->tc_verd = 0;
			skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
			txq->tx_packets++;
			txq->tx_bytes +=skb->len;
	
			rcu_read_lock();
			skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
			if (!skb->dev) {
				rcu_read_unlock();
				dev_kfree_skb(skb);
				txq->tx_dropped++;
				break;
			}
			rcu_read_unlock();
			skb->iif = dev->ifindex;
	
			if (from & AT_EGRESS) {
				dev_queue_xmit(skb);
			} else if (from & AT_INGRESS) {
				skb_pull(skb, skb->dev->hard_header_len);
				netif_rx_ni(skb);
			} else
				BUG();
		}
	}

	return 0;
}

struct net_device_stats* ifb_get_stats(struct net_device *dev)
{
	struct net_device_stats *stats = &dev->stats;
	struct ifb_private *dp = netdev_priv(dev);
	struct netdev_queue *txq;
	int i;
	unsigned long rx_packets = 0, rx_bytes = 0, rx_dropped = 0;
	unsigned long tx_packets = 0, tx_bytes = 0, tx_dropped = 0;

	for (i = 0; i < dev->real_num_tx_queues; i++) {
		rx_packets += dp[i].rx_packets;
		rx_bytes += dp[i].rx_bytes;
		rx_dropped += dp[i].rx_dropped;
		txq = netdev_get_tx_queue(dev, i);
		tx_packets = txq->tx_packets;
		tx_bytes = txq->tx_bytes;
		tx_dropped += txq->tx_dropped;
	}

	stats->rx_packets = rx_packets;
	stats->rx_bytes = rx_bytes;
	stats->rx_dropped = rx_dropped;
	stats->tx_packets = tx_packets;
	stats->tx_bytes = tx_bytes;
	stats->tx_dropped = tx_dropped;

	return stats;
}

static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)
{
	u32 from = G_TC_FROM(skb->tc_verd);
	int num = skb_get_queue_mapping(skb);
	struct ifb_private *dp = ((struct ifb_private *)netdev_priv(dev)) + num;
	struct netdev_queue *txq = netdev_get_tx_queue(dev, num);

	dp->rx_packets++;
	dp->rx_bytes+=skb->len;

	if (!(from & (AT_INGRESS|AT_EGRESS)) || !skb->iif) {
		dev_kfree_skb(skb);
		dp->rx_dropped++;
		return NETDEV_TX_OK;
	}

	if (skb_queue_len(&dp->rq) >= dev->tx_queue_len) {
		netif_stop_queue(dev);
	}

	txq->trans_start = jiffies;
	skb_queue_tail(&dp->rq, skb);
	if (skb_queue_len(&dp->rq) == 1)
		wake_up(&dp->wq);

	return NETDEV_TX_OK;
}

static int ifb_close(struct net_device *dev)
{
	struct ifb_private *dp = netdev_priv(dev);
	int i;

	for (i = 0; i < dev->real_num_tx_queues; i++) {
		kthread_stop(dp[i].task);
		skb_queue_purge(&dp[i].tq);
		skb_queue_purge(&dp[i].rq);
	}

	netif_stop_queue(dev);

	return 0;
}

static int ifb_open(struct net_device *dev)
{
	struct ifb_private *dp = netdev_priv(dev);
	int i;
	
	for (i = 0; i < dev->real_num_tx_queues; i++) {
		dp[i].dev = dev;
		skb_queue_head_init(&dp[i].rq);
		skb_queue_head_init(&dp[i].tq);
		init_waitqueue_head(&dp[i].wq);
		dp[i].task = kthread_run(ifb_thread, &dp[i], "%s/%d", dev->name,
					i);
		if (IS_ERR(dp[i].task)) {
			int err = PTR_ERR(dp[i].task);
			while (--i >= 0)
				kthread_stop(dp[i].task);
			return err;
		}
	}

	netif_start_queue(dev);

	return 0;
}

static u32 simple_tx_hashrnd;

static u16 ifb_select_queue(struct net_device *dev, struct sk_buff *skb)
{
	u32 addr1, addr2;
	u32 hash, ihl;
	union {
		u16 in16[2];
		u32 in32;
	} ports;
	u8 ip_proto;

	if ((hash = skb_rx_queue_recorded(skb))) {
		while (hash >= dev->real_num_tx_queues)
			hash -= dev->real_num_tx_queues;
		return hash;
	}

	switch (skb->protocol) {
	case __constant_htons(ETH_P_IP):
		if (!(ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)))
			ip_proto = ip_hdr(skb)->protocol;
		else
			ip_proto = 0;
		addr1 = ip_hdr(skb)->saddr;
		addr2 = ip_hdr(skb)->daddr;
		ihl = ip_hdr(skb)->ihl << 2;
		break;
	case __constant_htons(ETH_P_IPV6):
		ip_proto = ipv6_hdr(skb)->nexthdr;
		addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
		addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
		ihl = 10;
		break;
	default:
		return 0;
	}
	if (addr1 > addr2)
		swap(addr1, addr2);

	switch (ip_proto) {
	case IPPROTO_TCP:
	case IPPROTO_UDP:
	case IPPROTO_DCCP:
	case IPPROTO_ESP:
	case IPPROTO_AH:
	case IPPROTO_SCTP:
	case IPPROTO_UDPLITE:
		ports.in32 = *((u32 *) (skb_network_header(skb) + ihl));
		if (ports.in16[0] > ports.in16[1])
			swap(ports.in16[0], ports.in16[1]);
		break;

	default:
		ports.in32 = 0;
		break;
	}

	hash = jhash_3words(addr1, addr2, ports.in32,
			    simple_tx_hashrnd ^ ip_proto);

	return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
}

static const struct net_device_ops ifb_netdev_ops = {
	.ndo_open		= ifb_open,
	.ndo_stop		= ifb_close,
	.ndo_start_xmit		= ifb_xmit,
	.ndo_validate_addr	= eth_validate_addr,
	.ndo_select_queue	= ifb_select_queue,
	.ndo_get_stats		= ifb_get_stats,
};

static void ifb_setup(struct net_device *dev)
{
	/* Initialize the device structure. */
	dev->destructor = free_netdev;
	dev->netdev_ops = &ifb_netdev_ops;

	/* Fill in device structure with ethernet-generic values. */
	ether_setup(dev);
	dev->tx_queue_len = TX_Q_LIMIT;

	dev->flags |= IFF_NOARP;
	dev->flags &= ~IFF_MULTICAST;
	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
	random_ether_addr(dev->dev_addr);
}

static int ifb_validate(struct nlattr *tb[], struct nlattr *data[])
{
	if (tb[IFLA_ADDRESS]) {
		if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
			return -EINVAL;
		if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
			return -EADDRNOTAVAIL;
	}
	return 0;
}

static struct rtnl_link_ops ifb_link_ops __read_mostly = {
	.kind		= "ifb",
	.priv_size	= sizeof(struct ifb_private),
	.setup		= ifb_setup,
	.validate	= ifb_validate,
};

static int __init ifb_init_one(int index)
{
	struct net_device *dev_ifb;
	int err;

	dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d",
				  ifb_setup, numtxqs);

	if (!dev_ifb)
		return -ENOMEM;

	err = dev_alloc_name(dev_ifb, dev_ifb->name);
	if (err < 0)
		goto err;

	dev_ifb->rtnl_link_ops = &ifb_link_ops;
	err = register_netdevice(dev_ifb);
	if (err < 0)
		goto err;

	return 0;

err:
	free_netdev(dev_ifb);
	return err;
}

static int __init ifb_init_module(void)
{
	int i, err;

	get_random_bytes(&simple_tx_hashrnd, 4);
	rtnl_lock();
	err = __rtnl_link_register(&ifb_link_ops);

	for (i = 0; i < numifbs && !err; i++)
		err = ifb_init_one(i);
	if (err)
		__rtnl_link_unregister(&ifb_link_ops);
	rtnl_unlock();

	return err;
}

static void __exit ifb_cleanup_module(void)
{
	rtnl_link_unregister(&ifb_link_ops);
}

module_init(ifb_init_module);
module_exit(ifb_cleanup_module);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Jamal Hadi Salim");
MODULE_ALIAS_RTNL_LINK("ifb");

^ permalink raw reply

* Re: [PATCH] can: fix WARN_ON dump in net/core/rtnetlink.c:rtmsg_ifinfo()
From: Patrick McHardy @ 2009-11-10 11:24 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: David Miller, netdev, socketcan-core
In-Reply-To: <4AF68FCB.4050103@grandegger.com>

Wolfgang Grandegger wrote:
> David Miller wrote:
>> From: Wolfgang Grandegger <wg@grandegger.com>
>> Date: Sat, 07 Nov 2009 10:53:13 +0100
>>
>>> On older kernels, e.g. 2.6.27, a WARN_ON dump in rtmsg_ifinfo()
>>> is thrown when the CAN device is registered due to insufficient
>>> skb space, as reported by various users. This patch adds the
>>> rtnl_link_ops "get_size" to fix the problem. I think this patch
>>> is required for more recent kernels as well, even if no WARN_ON
>>> dumps are triggered. Maybe we also need "get_xstats_size" for
>>> the CAN xstats.
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
>> Applied to net-2.6, thanks Wolfgang.
> 
> Thanks, the commit message included some questions. What is the rule
> using the rtnl_link_ops "get_size" or "get_xstats_size". Are these
> mandatory if the corresponding fill functions are used?

Yes. You also need a get_xstats_size() function.

^ permalink raw reply

* Re: [PATCHv9 3/3] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-11-10 11:36 UTC (permalink / raw)
  To: Rusty Russell
  Cc: netdev, virtualization, kvm, linux-kernel, mingo, linux-mm, akpm,
	hpa, gregory.haskins, s.hetze, Daniel Walker, Eric Dumazet
In-Reply-To: <200911101349.09783.rusty@rustcorp.com.au>

On Tue, Nov 10, 2009 at 01:49:09PM +1030, Rusty Russell wrote:
> One fix:
> 
> vhost: fix TUN=m VHOST_NET=y
> 
> 	drivers/built-in.o: In function `get_tun_socket':
> 	net.c:(.text+0x15436e): undefined reference to `tun_get_socket'
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> ---
>  drivers/vhost/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -1,6 +1,6 @@
>  config VHOST_NET
>  	tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)"
> -	depends on NET && EVENTFD && EXPERIMENTAL
> +	depends on NET && EVENTFD && TUN && EXPERIMENTAL
>  	---help---
>  	  This kernel module can be loaded in host kernel to accelerate
>  	  guest networking with virtio_net. Not to be confused with virtio_net

In fact, vhost can be built with TUN=n VHOST_NET=y as well
(tun_get_socket is stubbed out in that case).
So I think this is better (it looks strange
until you realize that for tristate variables
boolean logic math does not apply):

--->

From: Michael S. Tsirkin <mst@redhat.com>
Subject: vhost: fix TUN=m VHOST_NET=y

    drivers/built-in.o: In function `get_tun_socket':
    net.c:(.text+0x15436e): undefined reference to `tun_get_socket'

If tun is a module, vhost must be a module, too.
If tun is built-in or disabled, vhost can be built-in.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 9f409f4..9e93553 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -1,6 +1,6 @@
 config VHOST_NET
 	tristate "Host kernel accelerator for virtio net (EXPERIMENTAL)"
-	depends on NET && EVENTFD && EXPERIMENTAL
+	depends on NET && EVENTFD && (TUN || !TUN) && EXPERIMENTAL
 	---help---
 	  This kernel module can be loaded in host kernel to accelerate
 	  guest networking with virtio_net. Not to be confused with virtio_net


-- 
MST

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-10 11:41 UTC (permalink / raw)
  To: Changli Gao; +Cc: Eric Dumazet, David S. Miller, netdev, Tom Herbert
In-Reply-To: <412e6f7f0911100314i33d6a49dl5163b5e8472babcf@mail.gmail.com>

Changli Gao wrote:
> 2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
>> Changli Gao a écrit :
>>> 2009/11/10 Eric Dumazet <eric.dumazet@gmail.com>:
>>>> Not sure how to solve this problem (several cpus can updates counter in //)
>>>>
>>> Thanks, and follow your suggestions. I can maintain the counter per TX
>>> queue, and update it in a timer handler like ixgbe or implement our
>>> own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev),
>>> and update the counters when it gets called.
>> Please no timer stuff :)
> 
> The whole ifb.c file is attached, please review and test it. Thanks!

> /* Number of TX queues per ifb */
> static int numtxqs = 1;
> module_param(numtxqs, int, 0444);
> MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");

Module parameters suck as a configuration API. The existing numifbs
option exists purely for compatibility reasons, I'd prefer if you'd
this to the netlink interface.

> static int __init ifb_init_one(int index)
> {
> 	struct net_device *dev_ifb;
> 	int err;
> 
> 	dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d",
> 				  ifb_setup, numtxqs);
> 

This won't work for the rtnl_link setup since the size must
be constant.

^ 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