Netdev List
 help / color / mirror / Atom feed
* Re: Very low latency TCP for clusters
From: Eric Dumazet @ 2010-07-19 22:03 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev
In-Reply-To: <AANLkTiniRmnwG4vT-MrQQHQjrdQ4sfOn6Uxi42Dsr8nu@mail.gmail.com>

Le lundi 19 juillet 2010 à 11:44 -0700, Tom Herbert a écrit :

> I see about 7 usecs as best number on loopback, so I believe this is
> in the ballpark.  As I mentioned above, this about "best case" latency
> of a single thread, so we assume any amount of pinning or other
> customized configuration to that purpose.

Well, given I get 29 us on a ping between two machines (Gb link, no
process involved on receiver, only softirq), I really doubt we can reach
5 us on a tcp test involving a user process on both side ;)

$ ping -f -c 10000 -q 192.168.0.1
PING 192.168.0.1 (192.168.0.1) 56(84) bytes of data.

--- 192.168.0.1 ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 395ms
rtt min/avg/max/mdev = 0.029/0.032/0.434/0.010 ms, ipg/ewma 0.039/0.033
ms


On loopback, its a bit better of course, but there is no cache miss nor
scheduler involved.

$ ping -f -c 10000 -q 127.0.0.1
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.

--- 127.0.0.1 ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 55ms
rtt min/avg/max/mdev = 0.002/0.002/0.015/0.001 ms, ipg/ewma 0.005/0.002
ms



^ permalink raw reply

* Re: [PATCH net-next-2.6] net: this_cpu_xxx conversions
From: David Miller @ 2010-07-19 22:13 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1279572529.2458.15.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 19 Jul 2010 22:48:49 +0200

> Use modern this_cpu_xxx() api, saving few bytes on x86
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH] LSM: Add post accept() hook.
From: Paul Moore @ 2010-07-19 22:15 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: davem, eric.dumazet, jmorris, sam, serge, netdev,
	linux-security-module
In-Reply-To: <201007191325.IDI17618.VJStMOFOOLFFHQ@I-love.SAKURA.ne.jp>

On Monday, July 19, 2010 12:25:25 am Tetsuo Handa wrote:
> Current pre accept hook (i.e. security_socket_accept()) has two problems.
> 
> One is that it will cause eating 100% of CPU time if the caller does not
> close() the socket when accept() failed due to security_socket_accept(),
> for subsequent select() notifies the caller of readiness for accept()
> since the connection which would have been already picked up if
> security_socket_accept() did not return error is remaining in the queue.
> 
> The other is that it is racy if LSM module wants to do filtering based on
> "which process can pick up connections from which source" because the
> process which picks up the connection is not known until
> sock->ops->accept() and lock is not held between security_socket_accept()
> and sock->ops->accept.
> 
> This patch introduces post accept hook (i.e. security_socket_post_accept())
> in order to solve above problems at the cost of ability to pick up the
> connection which would have been picked up if preceding
> security_socket_post_accept() did not return error.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

I think you need to show how you plan to use this hook in an LSM before we can 
consider merging it with mainline.  What you are proposing here is giving an 
LSM the ability to drop a connection _after_ allowing it to be established in 
the first place; this seems very wrong to me and I want to make sure everyone 
else is aware of that before accepting this code into the kernel.  I 
understand that TOMOYO's security model does not allow it to reject incoming 
connections at the beginning of the connection request like some of the LSMs 
currently in use, but I'm just not very happy with the idea of finishing a 
connection handshake only to later drop the connection on the floor.

> ---
>  include/linux/security.h |   21 +++++++++++++++++++++
>  net/socket.c             |    7 +++++++
>  security/capability.c    |    6 ++++++
>  security/security.c      |    5 +++++
>  4 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 409c44d..2ed73c1 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -866,6 +866,19 @@ static inline void security_free_mnt_opts(struct
> security_mnt_opts *opts) *	@sock contains the listening socket structure.
>   *	@newsock contains the newly created server socket for connection.
>   *	Return 0 if permission is granted.
> + * @socket_post_accept:
> + *	Check permission after accepting a new connection.
> + *	The connection is discarded if permission is not granted.
> + *	Return 0 after updating security information on the socket if you 
want
> + *	to restrict some of socket syscalls on the connection (e.g. forbid 
only
> + *	sending data). But you can't use this hook for updating security
> + *	information of the socket for preventing the connection from 
receiving
> + *	incoming data, for the kernel already started receiving incoming data
> + *	before accept() syscall. Return error if updating security 
information
> + *	failed or you want to forbid all of socket syscalls on the 
connection.
> + *	@sock contains the listening socket structure.
> + *	@newsock contains the accepted socket structure.
> + *	Return 0 if permission is granted.
>   * @socket_sendmsg:
>   *	Check permission before transmitting a message to another socket.
>   *	@sock contains the socket structure.
> @@ -1577,6 +1590,7 @@ struct security_operations {
>  			       struct sockaddr *address, int addrlen);
>  	int (*socket_listen) (struct socket *sock, int backlog);
>  	int (*socket_accept) (struct socket *sock, struct socket *newsock);
> +	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
>  	int (*socket_sendmsg) (struct socket *sock,
>  			       struct msghdr *msg, int size);
>  	int (*socket_recvmsg) (struct socket *sock,
> @@ -2530,6 +2544,7 @@ int security_socket_bind(struct socket *sock, struct
> sockaddr *address, int addr int security_socket_connect(struct socket
> *sock, struct sockaddr *address, int addrlen); int
> security_socket_listen(struct socket *sock, int backlog);
>  int security_socket_accept(struct socket *sock, struct socket *newsock);
> +int security_socket_post_accept(struct socket *sock, struct socket
> *newsock); int security_socket_sendmsg(struct socket *sock, struct msghdr
> *msg, int size); int security_socket_recvmsg(struct socket *sock, struct
> msghdr *msg, int size, int flags);
> @@ -2612,6 +2627,12 @@ static inline int security_socket_accept(struct
> socket *sock, return 0;
>  }
> 
> +static inline int security_socket_post_accept(struct socket *sock,
> +					      struct socket *newsock)
> +{
> +	return 0;
> +}
> +
>  static inline int security_socket_sendmsg(struct socket *sock,
>  					  struct msghdr *msg, int size)
>  {
> diff --git a/net/socket.c b/net/socket.c
> index 367d547..97d644c 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1473,6 +1473,7 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr
> __user *, upeer_sockaddr, if (!sock)
>  		goto out;
> 
> + retry:
>  	err = -ENFILE;
>  	if (!(newsock = sock_alloc()))
>  		goto out_put;
> @@ -1500,6 +1501,12 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr
> __user *, upeer_sockaddr, err = sock->ops->accept(sock, newsock,
> sock->file->f_flags);
>  	if (err < 0)
>  		goto out_fd;
> +	err = security_socket_post_accept(sock, newsock);
> +	if (unlikely(err)) {
> +		fput(newfile);
> +		put_unused_fd(newfd);
> +		goto retry;
> +	}
> 
>  	if (upeer_sockaddr) {
>  		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
> diff --git a/security/capability.c b/security/capability.c
> index 709aea3..1fb88f5 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -586,6 +586,11 @@ static int cap_socket_accept(struct socket *sock,
> struct socket *newsock) return 0;
>  }
> 
> +static int cap_socket_post_accept(struct socket *sock, struct socket
> *newsock) +{
> +	return 0;
> +}
> +
>  static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int
> size) {
>  	return 0;
> @@ -1004,6 +1009,7 @@ void __init security_fixup_ops(struct
> security_operations *ops) set_to_cap_if_null(ops, socket_connect);
>  	set_to_cap_if_null(ops, socket_listen);
>  	set_to_cap_if_null(ops, socket_accept);
> +	set_to_cap_if_null(ops, socket_post_accept);
>  	set_to_cap_if_null(ops, socket_sendmsg);
>  	set_to_cap_if_null(ops, socket_recvmsg);
>  	set_to_cap_if_null(ops, socket_post_recvmsg);
> diff --git a/security/security.c b/security/security.c
> index 4291bd7..5c9ab0a 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1026,6 +1026,11 @@ int security_socket_accept(struct socket *sock,
> struct socket *newsock) return security_ops->socket_accept(sock, newsock);
>  }
> 
> +int security_socket_post_accept(struct socket *sock, struct socket
> *newsock) +{
> +	return security_ops->socket_post_accept(sock, newsock);
> +}
> +
>  int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int
> size) {
>  	return security_ops->socket_sendmsg(sock, msg, size);

-- 
paul moore
linux @ hp

^ permalink raw reply

* Re: [PATCH v4 2.6.35-rc3] drivers/net: ks8842 driver
From: Andrew Morton @ 2010-07-19 22:17 UTC (permalink / raw)
  To: David Miller; +Cc: David.Choi, netdev, Charles.Li, horms
In-Reply-To: <20100713.101434.180417674.davem@davemloft.net>

On Tue, 13 Jul 2010 10:14:34 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: David J. Choi <david.choi@micrel.com>
> Date: Tue, 13 Jul 2010 10:09:19 -0700
> Subject: [PATCH] drivers/net: Add Micrel KS8841/42 support to ks8842 driver
> 
> Body of the explanation:
>    -support 16bit and 32bit bus width.
>    -add device reset for ks8842/8841 Micrel device.
>    -set 100Mbps as a default for Micrel device.
>    -set MAC address in both MAC/Switch layer with different sequence for Micrel
>     device, as mentioned in data sheet.
>    -use private data to set options both 16/32bit bus width and Micrel device/
>     Timberdale(FPGA).
>    -update Kconfig in order to put more information about ks8842 device.

gcc says

drivers/net/ks8842.c: In function 'ks8842_handle_rx':
drivers/net/ks8842.c:428: warning: 'status' may be used uninitialized in this function

and I think I agree with it..

^ permalink raw reply

* Re: [PATCH v4 2.6.35-rc3] drivers/net: ks8842 driver
From: David Miller @ 2010-07-19 22:26 UTC (permalink / raw)
  To: akpm; +Cc: David.Choi, netdev, Charles.Li, horms
In-Reply-To: <20100719151725.fc11b40d.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 19 Jul 2010 15:17:25 -0700

> On Tue, 13 Jul 2010 10:14:34 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: David J. Choi <david.choi@micrel.com>
>> Date: Tue, 13 Jul 2010 10:09:19 -0700
>> Subject: [PATCH] drivers/net: Add Micrel KS8841/42 support to ks8842 driver
>> 
>> Body of the explanation:
>>    -support 16bit and 32bit bus width.
>>    -add device reset for ks8842/8841 Micrel device.
>>    -set 100Mbps as a default for Micrel device.
>>    -set MAC address in both MAC/Switch layer with different sequence for Micrel
>>     device, as mentioned in data sheet.
>>    -use private data to set options both 16/32bit bus width and Micrel device/
>>     Timberdale(FPGA).
>>    -update Kconfig in order to put more information about ks8842 device.
> 
> gcc says
> 
> drivers/net/ks8842.c: In function 'ks8842_handle_rx':
> drivers/net/ks8842.c:428: warning: 'status' may be used uninitialized in this function
> 
> and I think I agree with it..

I'll fix this, as below.

David Choi, I'm quite upset, this shows that you did not test the
16-bit code paths (the whole _point_ of this change) at all.  You
didn't even validate the build for new warnings.

--------------------
ks8842: Fix ks8842_tx_frame() for 16bit case.

As reported by Andrew:

drivers/net/ks8842.c: In function 'ks8842_handle_rx':
drivers/net/ks8842.c:428: warning: 'status' may be used uninitialized in this function

Just use the 32-bit status for all reads, and delete the useless
cast to 'int' when reading a u16 into 'len'.

Reported-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/ks8842.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ks8842.c b/drivers/net/ks8842.c
index ee69dea..634dad1 100644
--- a/drivers/net/ks8842.c
+++ b/drivers/net/ks8842.c
@@ -424,16 +424,14 @@ static int ks8842_tx_frame(struct sk_buff *skb, struct net_device *netdev)
 static void ks8842_rx_frame(struct net_device *netdev,
 	struct ks8842_adapter *adapter)
 {
-	u16 status16;
 	u32 status;
 	int len;
 
 	if (adapter->conf_flags & KS884X_16BIT) {
-		status16 = ks8842_read16(adapter, 17, REG_QMU_DATA_LO);
-		len  = (int)ks8842_read16(adapter, 17, REG_QMU_DATA_HI);
-		len &= 0xffff;
+		status = ks8842_read16(adapter, 17, REG_QMU_DATA_LO);
+		len = ks8842_read16(adapter, 17, REG_QMU_DATA_HI);
 		netdev_dbg(netdev, "%s - rx_data: status: %x\n",
-			   __func__, status16);
+			   __func__, status);
 	} else {
 		status = ks8842_read32(adapter, 17, REG_QMU_DATA_LO);
 		len = (status >> 16) & 0x7ff;
-- 
1.7.1.1


^ permalink raw reply related

* Priority/queueing on vitrual interfaces -- do virtual interfaces have a queue?
From: Richard Hartmann @ 2010-07-19 22:36 UTC (permalink / raw)
  To: netdev, netfilter, lartc

Hi all,

I am using a HTB qdisc with one intermediate class and half a dozen
leaf classes. All leaf classes (other than the first) are allowed to
starve all classes below them (far from ideal, but a hard requirement).

Things work fine, but it seems that prio is more or less ignored.

I found one online source that claimed that due to the fact that
virtual interfaces don't have a buffer, their send queue is always
empty and thus prio would not do anything. Yet, I can hardly
believe that.

I would appreciate any input on this.

You can find my current config at [1]. Please, no discussions
about starving the lower classes. I know.


-- 
Richard

[1] http://paste.debian.net/80919/

^ permalink raw reply

* Re: [PATCH v4 2.6.35-rc3] drivers/net: ks8842 driver
From: Andrew Morton @ 2010-07-19 22:45 UTC (permalink / raw)
  To: David Miller; +Cc: David.Choi, netdev, Charles.Li, horms
In-Reply-To: <20100719.152621.93431422.davem@davemloft.net>

On Mon, 19 Jul 2010 15:26:21 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> David Choi, I'm quite upset, this shows that you did not test the
> 16-bit code paths (the whole _point_ of this change) at all.

The code could have passed runtime testing by luck.

> You didn't even validate the build for new warnings.

yeah.  The kernel build is such a stinky mess of warnings that it's
easy to miss new ones.  You're basically forced to manually compile the
files which you touched to see if you added warnings.

That's pretty easy to do:

- unapply your patch
- make
- apply your patch
- make

and `make' will go and build just the files you affected.

We really should be more aggressive about all those warnings though.

^ permalink raw reply

* Re: Raise initial congestion window size / speedup slow start?
From: H.K. Jerry Chu @ 2010-07-19 22:51 UTC (permalink / raw)
  To: Rick Jones
  Cc: Patrick McManus, David Miller, davidsen, lists, linux-kernel,
	netdev
In-Reply-To: <4C448688.1070507@hp.com>

 Mon, Jul 19, 2010 at 10:08 AM, Rick Jones <rick.jones2@hp.com> wrote:
> H.K. Jerry Chu wrote:
>>
>> On Fri, Jul 16, 2010 at 10:01 AM, Patrick McManus <mcmanus@ducksong.com>
>> wrote:
>>>
>>> can you tell us more about the impl concerns of initcwnd stored on the
>>> route?
>>
>>
>> We have found two issues when altering initcwnd through the ip route cmd:
>> 1. initcwnd is actually capped by sndbuf (i.e., tcp_wmem[1], which is
>> defaulted to a small value of 16KB). This problem has been made obscured
>> by the TSO code, which fudges the flow control limit (and could be a bug
>> by
>> itself).
>
> I'll ask my Emily Litella question of the day and inquire as to why that
> would be unique to altering initcwnd via the route?
>
> The slightly less Emily Litella-esque question is why an appliction with a
> desire to know it could send more than 16K at one time wouldn't have either
> asked via its install docs to have the minimum tweaked (certainly if one is
> already tweaking routes...), or "gone all the way" and made an explicit
> setsockopt(SO_SNDBUF) call?  We are in a realm of applications for which
> there was a proposal to allow them to pick their own initcwnd right?  Having

Per app setting of initcwnd is just one case. Another is per route setting of
initcwnd basis through the ip route cmd. For the latter the initcwnd change is
more or less supposed to be transparent to apps.

This wasn't a big issue and can probably be easily fixed by
initializing sk_sndbuf
to max(tcp_wmem[1], initcwnd) as you alluded to below. It is just our
experiements got hindered by this little bug but we weren't aware of it sooner
due to TSO fudging sndbuf.

Jerry

> them pick an SO_SNDBUF size would seem to be no more to ask.
>
> rick jones
>
> sendbuf_init = max(tcp_mem,initcwnd)?
>

^ permalink raw reply

* Re: Very low latency TCP for clusters
From: Tom Herbert @ 2010-07-19 23:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1279576980.2458.56.camel@edumazet-laptop>

On Mon, Jul 19, 2010 at 3:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 19 juillet 2010 à 11:44 -0700, Tom Herbert a écrit :
>
>> I see about 7 usecs as best number on loopback, so I believe this is
>> in the ballpark.  As I mentioned above, this about "best case" latency
>> of a single thread, so we assume any amount of pinning or other
>> customized configuration to that purpose.
>
> Well, given I get 29 us on a ping between two machines (Gb link, no
> process involved on receiver, only softirq), I really doubt we can reach
> 5 us on a tcp test involving a user process on both side ;)
>
That's pretty pokey ;-) I see numbers around 25 usecs between to
machines, this is with TCP_NBRR.  With TCP_RR it's more like 35 usecs,
so eliminating the scheduler is already a big reduction.  That leaves
18 usecs in device time, interrupt processing, network, and cache
misses; 7 usecs in TCP processing, user space.  While 5 usecs is an
aggressive goal, I am not ready to concede that there's an
architectural limit in either NICs, TCP, or sockets that can't be
overcome.

^ permalink raw reply

* Re: Raise initial congestion window size / speedup slow start?
From: Hagen Paul Pfeifer @ 2010-07-19 23:42 UTC (permalink / raw)
  To: H.K. Jerry Chu
  Cc: Rick Jones, Patrick McManus, David Miller, davidsen, lists,
	linux-kernel, netdev, Stephen Hemminger, Alan Cox
In-Reply-To: <AANLkTilI3rXF9ikiQOIqCOXpq4s3cfqOULBn_P8jQVrp@mail.gmail.com>

Maybe someone is interested: on the Transport Modeling Research Group (TMRG)
mailing list a new thread named "Proposal to increase TCP initial CWND"
starts one day ago.

Cheers, Hagen


^ permalink raw reply

* [net-next-2.6 PATCH] e1000: allow option to limit number of descriptors down to 48 per ring
From: Jeff Kirsher @ 2010-07-19 23:43 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Alexander Duyck, Jeff Kirsher

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it possible to limit the number of descriptors down to 48
per ring.  The reason for this change is to address a variation on hardware
errata 10 for 82546GB in which descriptors will be lost if more than 32
descriptors are fetched and the PCI-X MRBC is 512.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000/e1000.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 40b62b4..65298a6 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -86,12 +86,12 @@ struct e1000_adapter;
 /* TX/RX descriptor defines */
 #define E1000_DEFAULT_TXD                  256
 #define E1000_MAX_TXD                      256
-#define E1000_MIN_TXD                       80
+#define E1000_MIN_TXD                       48
 #define E1000_MAX_82544_TXD               4096
 
 #define E1000_DEFAULT_RXD                  256
 #define E1000_MAX_RXD                      256
-#define E1000_MIN_RXD                       80
+#define E1000_MIN_RXD                       48
 #define E1000_MAX_82544_RXD               4096
 
 #define E1000_MIN_ITR_USECS		10 /* 100000 irq/sec */


^ permalink raw reply related

* [net-next-2.6 PATCH 1/5] ixgbe: dcb, set DPF bit when PFC is enabled
From: Jeff Kirsher @ 2010-07-19 23:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, John Fastabend, Don Skidmore,
	Jeff Kirsher

From: John Fastabend <john.r.fastabend@intel.com>

Set the DPF bit when PFC is enabled.  This will discard
PFC frames so they do not get passed up the stack.

The DPF bit is set for flow control, but not priority
flow control this brings pfc inline with fc.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_dcb_82599.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_dcb_82599.c b/drivers/net/ixgbe/ixgbe_dcb_82599.c
index 4f7a26a..25b02fb 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_82599.c
+++ b/drivers/net/ixgbe/ixgbe_dcb_82599.c
@@ -346,7 +346,7 @@ s32 ixgbe_dcb_config_pfc_82599(struct ixgbe_hw *hw,
 	 */
 	reg = IXGBE_READ_REG(hw, IXGBE_MFLCN);
 	reg &= ~IXGBE_MFLCN_RFCE;
-	reg |= IXGBE_MFLCN_RPFCE;
+	reg |= IXGBE_MFLCN_RPFCE | IXGBE_MFLCN_DPF;
 	IXGBE_WRITE_REG(hw, IXGBE_MFLCN, reg);
 out:
 	return 0;


^ permalink raw reply related

* [net-next-2.6 PATCH 2/5] ixgbe: drop support for UDP in RSS hash generation
From: Jeff Kirsher @ 2010-07-19 23:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Alexander Duyck, Don Skidmore,
	Jeff Kirsher
In-Reply-To: <20100719235831.14112.14175.stgit@localhost.localdomain>

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change removes UDP from the supported protocols for RSS hashing.  The
reason for removing this protocol is because IP fragmentation was causing a
network flow to be broken into two streams, one for fragmented, and one for
non-fragmented and this in turn was causing out-of-order issues.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index b235aa1..813d2cb 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -2800,10 +2800,8 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
 		    /* Perform hash on these packet types */
 		mrqc |= IXGBE_MRQC_RSS_FIELD_IPV4
 		      | IXGBE_MRQC_RSS_FIELD_IPV4_TCP
-		      | IXGBE_MRQC_RSS_FIELD_IPV4_UDP
 		      | IXGBE_MRQC_RSS_FIELD_IPV6
-		      | IXGBE_MRQC_RSS_FIELD_IPV6_TCP
-		      | IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
+		      | IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
 	}
 	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
 


^ permalink raw reply related

* [net-next-2.6 PATCH 3/5] ixgbe: properly toggling netdev feature flags when disabling FCoE
From: Jeff Kirsher @ 2010-07-19 23:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Yi Zou, Jeff Kirsher
In-Reply-To: <20100719235831.14112.14175.stgit@localhost.localdomain>

From: Yi Zou <yi.zou@intel.com>

When FCoE is disabled, there is a race condition that FCoE offload is
turned off but the FCoE protocol driver is still queuing I/O thinking
offload support still exists. This patch toggles off corresponding FCoE
netdev feature flags and notify the FCoE stack first, allowing FCoE
protocol stack driver to update its flags upon NETDEV_FEAT_CHANGE so no
I/O will be using offload.

Also, indicate FCoE offload flags in vlan_features in ixgbe_probe once
and do not toggle them in ixgbe_fcoe_enable/disable so when FCoE is
created on the VLAN interface, vlan_transfer_features() would properly
update the VLAN netdev features flag and notify the FCoE protocol driver
for NETDEV_FEAT_CHANGE.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_fcoe.c |   19 ++++++-------------
 drivers/net/ixgbe/ixgbe_main.c |    5 +++++
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
index f6ef4cd..1737d2b 100644
--- a/drivers/net/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ixgbe/ixgbe_fcoe.c
@@ -622,9 +622,6 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
 	netdev->features |= NETIF_F_FCOE_CRC;
 	netdev->features |= NETIF_F_FSO;
 	netdev->features |= NETIF_F_FCOE_MTU;
-	netdev->vlan_features |= NETIF_F_FCOE_CRC;
-	netdev->vlan_features |= NETIF_F_FSO;
-	netdev->vlan_features |= NETIF_F_FCOE_MTU;
 	netdev->fcoe_ddp_xid = IXGBE_FCOE_DDP_MAX - 1;
 
 	ixgbe_init_interrupt_scheme(adapter);
@@ -658,24 +655,20 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
 		goto out_disable;
 
 	e_info(drv, "Disabling FCoE offload features.\n");
+	netdev->features &= ~NETIF_F_FCOE_CRC;
+	netdev->features &= ~NETIF_F_FSO;
+	netdev->features &= ~NETIF_F_FCOE_MTU;
+	netdev->fcoe_ddp_xid = 0;
+	netdev_features_change(netdev);
+
 	if (netif_running(netdev))
 		netdev->netdev_ops->ndo_stop(netdev);
 
 	ixgbe_clear_interrupt_scheme(adapter);
-
 	adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
 	adapter->ring_feature[RING_F_FCOE].indices = 0;
-	netdev->features &= ~NETIF_F_FCOE_CRC;
-	netdev->features &= ~NETIF_F_FSO;
-	netdev->features &= ~NETIF_F_FCOE_MTU;
-	netdev->vlan_features &= ~NETIF_F_FCOE_CRC;
-	netdev->vlan_features &= ~NETIF_F_FSO;
-	netdev->vlan_features &= ~NETIF_F_FCOE_MTU;
-	netdev->fcoe_ddp_xid = 0;
-
 	ixgbe_cleanup_fcoe(adapter);
 	ixgbe_init_interrupt_scheme(adapter);
-	netdev_features_change(netdev);
 
 	if (netif_running(netdev))
 		netdev->netdev_ops->ndo_open(netdev);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 813d2cb..7d619d6 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -6740,6 +6740,11 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 				adapter->flags &= ~IXGBE_FLAG_FCOE_CAPABLE;
 		}
 	}
+	if (adapter->flags & IXGBE_FLAG_FCOE_CAPABLE) {
+		netdev->vlan_features |= NETIF_F_FCOE_CRC;
+		netdev->vlan_features |= NETIF_F_FSO;
+		netdev->vlan_features |= NETIF_F_FCOE_MTU;
+	}
 #endif /* IXGBE_FCOE */
 	if (pci_using_dac)
 		netdev->features |= NETIF_F_HIGHDMA;


^ permalink raw reply related

* [net-next-2.6 PATCH 4/5] ixgbe: use GFP_ATOMIC when allocating FCoE DDP context from the dma pool
From: Jeff Kirsher @ 2010-07-20  0:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Yi Zou, Jeff Kirsher
In-Reply-To: <20100719235831.14112.14175.stgit@localhost.localdomain>

From: Yi Zou <yi.zou@intel.com>

The FCoE protocol stack may hold a lock when this gets called.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_fcoe.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
index 1737d2b..072327c 100644
--- a/drivers/net/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ixgbe/ixgbe_fcoe.c
@@ -190,7 +190,7 @@ int ixgbe_fcoe_ddp_get(struct net_device *netdev, u16 xid,
 	}
 
 	/* alloc the udl from our ddp pool */
-	ddp->udl = pci_pool_alloc(fcoe->pool, GFP_KERNEL, &ddp->udp);
+	ddp->udl = pci_pool_alloc(fcoe->pool, GFP_ATOMIC, &ddp->udp);
 	if (!ddp->udl) {
 		e_err(drv, "failed allocated ddp context\n");
 		goto out_noddp_unmap;


^ permalink raw reply related

* [net-next-2.6 PATCH 5/5] ixgbe: fix version string for ixgbe
From: Jeff Kirsher @ 2010-07-20  0:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, Don Skidmore, Jeff Kirsher
In-Reply-To: <20100719235831.14112.14175.stgit@localhost.localdomain>

From: Don Skidmore <donald.c.skidmore@intel.com>

Bump the version string to better reflect what is in the driver.

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/ixgbe/ixgbe_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 7d619d6..9203759 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -52,7 +52,7 @@ char ixgbe_driver_name[] = "ixgbe";
 static const char ixgbe_driver_string[] =
                               "Intel(R) 10 Gigabit PCI Express Network Driver";
 
-#define DRV_VERSION "2.0.62-k2"
+#define DRV_VERSION "2.0.84-k2"
 const char ixgbe_driver_version[] = DRV_VERSION;
 static char ixgbe_copyright[] = "Copyright (c) 1999-2010 Intel Corporation.";
 


^ permalink raw reply related

* [PATCH net-next 1/4] bnx2: Use proper counter for net_device_stats->multicast.
From: Michael Chan @ 2010-07-20  0:15 UTC (permalink / raw)
  To: davem; +Cc: netdev

We were using the wrong tx multicast counter instead of the rx multicast
counter.

Reported-by: Peter Snellman <peter.snellman@cinnober.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index ce3217b..deb7f83 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6631,7 +6631,7 @@ bnx2_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *net_stats)
 		GET_64BIT_NET_STATS(stat_IfHCOutOctets);
 
 	net_stats->multicast =
-		GET_64BIT_NET_STATS(stat_IfHCOutMulticastPkts);
+		GET_64BIT_NET_STATS(stat_IfHCInMulticastPkts);
 
 	net_stats->collisions =
 		GET_32BIT_NET_STATS(stat_EtherStatsCollisions);
-- 
1.6.4.GIT



^ permalink raw reply related

* [PATCH net-next 3/4] bnx2: Remove some unnecessary smp_mb() in tx fast path.
From: Michael Chan @ 2010-07-20  0:15 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1279584905-15084-2-git-send-email-mchan@broadcom.com>

smp_mb() inside bnx2_tx_avail() is used twice in the normal
bnx2_start_xmit() path (see illustration below).  The full memory
barrier is only necessary during race conditions with tx completion.
We can speed up the tx path by replacing smp_mb() in bnx2_tx_avail()
with a compiler barrier.  The compiler barrier is to force the
compiler to fetch the tx_prod and tx_cons from memory.

In the race condition between bnx2_start_xmit() and bnx2_tx_int(),
we have the following situation:

bnx2_start_xmit()                       bnx2_tx_int()
    if (!bnx2_tx_avail())
            BUG();

    ...

    if (!bnx2_tx_avail())
            netif_tx_stop_queue();          update_tx_index();
            smp_mb();                       smp_mb();
            if (bnx2_tx_avail())            if (netif_tx_queue_stopped() &&
                    netif_tx_wake_queue();      bnx2_tx_avail())

With smp_mb() removed from bnx2_tx_avail(), we need to add smp_mb() to
bnx2_start_xmit() as shown above to properly order netif_tx_stop_queue()
and bnx2_tx_avail() to check the ring index.  If it is not strictly
ordered, the tx queue can be stopped forever.

This improves performance by about 5% with 2 ports running bi-directional
64-byte packets.

Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index d44ecc3..2af570d 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -253,7 +253,8 @@ static inline u32 bnx2_tx_avail(struct bnx2 *bp, struct bnx2_tx_ring_info *txr)
 {
 	u32 diff;
 
-	smp_mb();
+	/* Tell compiler to fetch tx_prod and tx_cons from memory. */
+	barrier();
 
 	/* The ring uses 256 indices for 255 entries, one of them
 	 * needs to be skipped.
@@ -6534,6 +6535,13 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (unlikely(bnx2_tx_avail(bp, txr) <= MAX_SKB_FRAGS)) {
 		netif_tx_stop_queue(txq);
+
+		/* netif_tx_stop_queue() must be done before checking
+		 * tx index in bnx2_tx_avail() below, because in
+		 * bnx2_tx_int(), we update tx index before checking for
+		 * netif_tx_queue_stopped().
+		 */
+		smp_mb();
 		if (bnx2_tx_avail(bp, txr) > bp->tx_wake_thresh)
 			netif_tx_wake_queue(txq);
 	}
-- 
1.6.4.GIT



^ permalink raw reply related

* [PATCH net-next 2/4] bnx2: Call pci_enable_msix() with actual number of vectors.
From: Michael Chan @ 2010-07-20  0:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Breno Leitão
In-Reply-To: <1279584905-15084-1-git-send-email-mchan@broadcom.com>

Based on original patch by Breno Leitão <leitao@linux.vnet.ibm.com>.

Allocate the actual number of vectors and make use of fewer vectors
if pci_enable_msix() returns > 0.  We must allocate one additional
vector for the cnic driver.

Cc: Breno Leitão <leitao@linux.vnet.ibm.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
Reviewed-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |   24 ++++++++++++++++++++----
 drivers/net/bnx2.h |    9 ++++++---
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index deb7f83..d44ecc3 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -864,7 +864,7 @@ bnx2_alloc_mem(struct bnx2 *bp)
 	bnapi->hw_rx_cons_ptr =
 		&bnapi->status_blk.msi->status_rx_quick_consumer_index0;
 	if (bp->flags & BNX2_FLAG_MSIX_CAP) {
-		for (i = 1; i < BNX2_MAX_MSIX_VEC; i++) {
+		for (i = 1; i < bp->irq_nvecs; i++) {
 			struct status_block_msix *sblk;
 
 			bnapi = &bp->bnx2_napi[i];
@@ -6152,7 +6152,7 @@ bnx2_free_irq(struct bnx2 *bp)
 static void
 bnx2_enable_msix(struct bnx2 *bp, int msix_vecs)
 {
-	int i, rc;
+	int i, total_vecs, rc;
 	struct msix_entry msix_ent[BNX2_MAX_MSIX_VEC];
 	struct net_device *dev = bp->dev;
 	const int len = sizeof(bp->irq_tbl[0].name);
@@ -6171,13 +6171,29 @@ bnx2_enable_msix(struct bnx2 *bp, int msix_vecs)
 		msix_ent[i].vector = 0;
 	}
 
-	rc = pci_enable_msix(bp->pdev, msix_ent, BNX2_MAX_MSIX_VEC);
+	total_vecs = msix_vecs;
+#ifdef BCM_CNIC
+	total_vecs++;
+#endif
+	rc = -ENOSPC;
+	while (total_vecs >= BNX2_MIN_MSIX_VEC) {
+		rc = pci_enable_msix(bp->pdev, msix_ent, total_vecs);
+		if (rc <= 0)
+			break;
+		if (rc > 0)
+			total_vecs = rc;
+	}
+
 	if (rc != 0)
 		return;
 
+	msix_vecs = total_vecs;
+#ifdef BCM_CNIC
+	msix_vecs--;
+#endif
 	bp->irq_nvecs = msix_vecs;
 	bp->flags |= BNX2_FLAG_USING_MSIX | BNX2_FLAG_ONE_SHOT_MSI;
-	for (i = 0; i < BNX2_MAX_MSIX_VEC; i++) {
+	for (i = 0; i < total_vecs; i++) {
 		bp->irq_tbl[i].vector = msix_ent[i].vector;
 		snprintf(bp->irq_tbl[i].name, len, "%s-%d", dev->name, i);
 		bp->irq_tbl[i].handler = bnx2_msi_1shot;
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index b9af6bc..2104c10 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6637,9 +6637,12 @@ struct flash_spec {
 
 #define BNX2_MAX_MSIX_HW_VEC	9
 #define BNX2_MAX_MSIX_VEC	9
-#define BNX2_BASE_VEC		0
-#define BNX2_TX_VEC		1
-#define BNX2_TX_INT_NUM	(BNX2_TX_VEC << BNX2_PCICFG_INT_ACK_CMD_INT_NUM_SHIFT)
+#ifdef BCM_CNIC
+#define BNX2_MIN_MSIX_VEC	2
+#else
+#define BNX2_MIN_MSIX_VEC	1
+#endif
+
 
 struct bnx2_irq {
 	irq_handler_t	handler;
-- 
1.6.4.GIT



^ permalink raw reply related

* [PATCH net-next 4/4] bnx2: Update version to 2.0.17.
From: Michael Chan @ 2010-07-20  0:15 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1279584905-15084-3-git-send-email-mchan@broadcom.com>

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 2af570d..e6a803f 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -58,8 +58,8 @@
 #include "bnx2_fw.h"
 
 #define DRV_MODULE_NAME		"bnx2"
-#define DRV_MODULE_VERSION	"2.0.16"
-#define DRV_MODULE_RELDATE	"July 2, 2010"
+#define DRV_MODULE_VERSION	"2.0.17"
+#define DRV_MODULE_RELDATE	"July 18, 2010"
 #define FW_MIPS_FILE_06		"bnx2/bnx2-mips-06-5.0.0.j6.fw"
 #define FW_RV2P_FILE_06		"bnx2/bnx2-rv2p-06-5.0.0.j3.fw"
 #define FW_MIPS_FILE_09		"bnx2/bnx2-mips-09-5.0.0.j15.fw"
-- 
1.6.4.GIT



^ permalink raw reply related

* [RFC PATCH v3 0/5] netdev: show a process of packets
From: Koki Sanagi @ 2010-07-20  0:43 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, davem, kaneshige.kenji, izumi.taku, kosaki.motohiro,
	nhorman, laijs, scott.a.mcmillan, rostedt, eric.dumazet, fweisbec,
	mathieu.desnoyers

CHANGE-LOG since v2:
    1) let all tracepoints of softirq use DECLARE_EVENT_CLASS
    2) let tracepoint of netdev_queue and netdev_receive use DECLARE_EVENT_CLASS
    3) add tracepoint to skb_free_datagram_locked
    4) show function and time when received packet is freed

These patch-set adds tracepoints to show us a process of packets.
Using these tracepoints and existing points, we can get the time when
packet passes through some points in transmit or receive sequence.
For example, this is an output of perf script which is attached by patch 5/5.

79074.756672832sec cpu=1
irq_entry(+0.000000msec,irq=77:eth3)
         |------------softirq_raise(+0.001277msec)
irq_exit (+0.002278msec)     |
                             |
                      softirq_entry(+0.003562msec)
                             |
                             |---netif_receive_skb(+0.006279msec,len=100)
                             |            |
                             |   skb_copy_datagram_iovec(+0.038778msec, 2285:sshd)
                             |
                      napi_poll_exit(+0.017160msec, eth3)
                             |
                      softirq_exit(+0.018248msec)

The above is a receive side. Like this, it can show receive sequence from
interrupt(irq_entry) to application(skb_copy_datagram_iovec). There are eight
points in this side. All events except for skb_copy_datagram_iovec and
freeing skb events can be associated with each other by CPU number.
skb_copy_datagram_iovec and freeing skb events can be associated with
netif_receive_skb by skbaddr.
This script shows one NET_RX softirq and events related to it. All relative
time bases on first irq_entry which raise NET_RX softirq.

   dev    len      Qdisc               netdevice             free
   eth3   114  79044.417123332sec     0.005242msec          0.103843msec
   eth3   114  79044.580090422sec     0.002306msec          0.103632msec
   eth3   114  79044.719078251sec     0.002288msec          0.104093msec

The above is a transmit side. There are three tracepoints in this side.
Point1 is before putting a packet to Qdisc. point2 is after ndo_start_xmit in
dev_hard_start_xmit. It indicates finishing putting a packet to driver.
point3 is in consume_skb and dev_kfree_skb_irq. It indicates freeing a
transmitted packet.
Values of this script are, from left, device name, length of a packet, a time of
point1, an interval time between point1 and point2 and an interval time between
point2 and point3.

These times are useful to analyze a performance or to detect a point where
packet delays. For example,
- NET_RX softirq calling is late.
- Application is late to take a packet.
- It takes much time to put a transmitting packet to driver
  (It may be caused by packed queue)

And also, these tracepoint help us to investigate a network driver's trouble
from memory dump because ftrace records it to memory. And ftrace is so light
even if always trace on. So, in a case investigating a problem which doesn't
reproduce, it is useful.

Thanks,
Koki Sanagi.

^ permalink raw reply

* [RFC PATCH v3 1/5] irq: add tracepoint to softirq_raise
From: Koki Sanagi @ 2010-07-20  0:45 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, davem, kaneshige.kenji, izumi.taku, kosaki.motohiro,
	nhorman, laijs, scott.a.mcmillan, rostedt, eric.dumazet, fweisbec,
	mathieu.desnoyers
In-Reply-To: <4C44F12F.5090908@jp.fujitsu.com>

From: Lai Jiangshan <laijs@cn.fujitsu.com>

Add a tracepoint for tracing when softirq action is raised.

It and the existed tracepoints complete softirq's tracepoints:
softirq_raise, softirq_entry and softirq_exit.

And when this tracepoint is used in combination with
the softirq_entry tracepoint we can determine
the softirq raise latency.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

[ factorize softirq events with DECLARE_EVENT_CLASS ]
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
 include/linux/interrupt.h  |    8 +++++-
 include/trace/events/irq.h |   57 ++++++++++++++++++++++++++-----------------
 kernel/softirq.c           |    4 +-
 3 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index c233113..1cb5726 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -18,6 +18,7 @@
 #include <asm/atomic.h>
 #include <asm/ptrace.h>
 #include <asm/system.h>
+#include <trace/events/irq.h>
 
 /*
  * These correspond to the IORESOURCE_IRQ_* defines in
@@ -402,7 +403,12 @@ asmlinkage void do_softirq(void);
 asmlinkage void __do_softirq(void);
 extern void open_softirq(int nr, void (*action)(struct softirq_action *));
 extern void softirq_init(void);
-#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
+static inline void __raise_softirq_irqoff(unsigned int nr)
+{
+	trace_softirq_raise(nr);
+	or_softirq_pending(1UL << nr);
+}
+
 extern void raise_softirq_irqoff(unsigned int nr);
 extern void raise_softirq(unsigned int nr);
 extern void wakeup_softirqd(void);
diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 0e4cfb6..717744c 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -5,7 +5,9 @@
 #define _TRACE_IRQ_H
 
 #include <linux/tracepoint.h>
-#include <linux/interrupt.h>
+
+struct irqaction;
+struct softirq_action;
 
 #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
 #define show_softirq_name(val)				\
@@ -84,56 +86,65 @@ TRACE_EVENT(irq_handler_exit,
 
 DECLARE_EVENT_CLASS(softirq,
 
-	TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
+	TP_PROTO(unsigned int nr),
 
-	TP_ARGS(h, vec),
+	TP_ARGS(nr),
 
 	TP_STRUCT__entry(
-		__field(	int,	vec			)
+		__field(	unsigned int,	vec	)
 	),
 
 	TP_fast_assign(
-		__entry->vec = (int)(h - vec);
+		__entry->vec	= nr;
 	),
 
 	TP_printk("vec=%d [action=%s]", __entry->vec,
-		  show_softirq_name(__entry->vec))
+		show_softirq_name(__entry->vec))
+);
+
+/**
+ * softirq_raise - called immediately when a softirq is raised
+ * @nr: softirq vector number
+ *
+ * Tracepoint for tracing when softirq action is raised.
+ * Also, when used in combination with the softirq_entry tracepoint
+ * we can determine the softirq raise latency.
+ */
+DEFINE_EVENT(softirq, softirq_raise,
+
+	TP_PROTO(unsigned int nr),
+
+	TP_ARGS(nr)
 );
 
 /**
  * softirq_entry - called immediately before the softirq handler
- * @h: pointer to struct softirq_action
- * @vec: pointer to first struct softirq_action in softirq_vec array
+ * @nr: softirq vector number
  *
- * The @h parameter, contains a pointer to the struct softirq_action
- * which has a pointer to the action handler that is called. By subtracting
- * the @vec pointer from the @h pointer, we can determine the softirq
- * number. Also, when used in combination with the softirq_exit tracepoint
+ * Tracepoint for tracing when softirq action starts.
+ * Also, when used in combination with the softirq_exit tracepoint
  * we can determine the softirq latency.
  */
 DEFINE_EVENT(softirq, softirq_entry,
 
-	TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
+	TP_PROTO(unsigned int nr),
 
-	TP_ARGS(h, vec)
+	TP_ARGS(nr)
 );
 
 /**
  * softirq_exit - called immediately after the softirq handler returns
- * @h: pointer to struct softirq_action
- * @vec: pointer to first struct softirq_action in softirq_vec array
+ * @nr: softirq vector number
  *
- * The @h parameter contains a pointer to the struct softirq_action
- * that has handled the softirq. By subtracting the @vec pointer from
- * the @h pointer, we can determine the softirq number. Also, when used in
- * combination with the softirq_entry tracepoint we can determine the softirq
- * latency.
+ * Tracepoint for tracing when softirq action ends.
+ * Also, when used in combination with the softirq_entry tracepoint
+ * we can determine the softirq latency.
  */
 DEFINE_EVENT(softirq, softirq_exit,
 
-	TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
+	TP_PROTO(unsigned int nr),
 
-	TP_ARGS(h, vec)
+	TP_ARGS(nr)
 );
 
 #endif /*  _TRACE_IRQ_H */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 825e112..6790599 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -215,9 +215,9 @@ restart:
 			int prev_count = preempt_count();
 			kstat_incr_softirqs_this_cpu(h - softirq_vec);
 
-			trace_softirq_entry(h, softirq_vec);
+			trace_softirq_entry(h - softirq_vec);
 			h->action(h);
-			trace_softirq_exit(h, softirq_vec);
+			trace_softirq_exit(h - softirq_vec);
 			if (unlikely(prev_count != preempt_count())) {
 				printk(KERN_ERR "huh, entered softirq %td %s %p"
 				       "with preempt_count %08x,"

^ permalink raw reply related

* [RFC PATCH v3 2/5] napi: convert trace_napi_poll to TRACE_EVENT
From: Koki Sanagi @ 2010-07-20  0:46 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, davem, kaneshige.kenji, izumi.taku, kosaki.motohiro,
	nhorman, laijs, scott.a.mcmillan, rostedt, eric.dumazet, fweisbec,
	mathieu.desnoyers
In-Reply-To: <4C44F12F.5090908@jp.fujitsu.com>

From: Neil Horman <nhorman@tuxdriver.com>

This patch converts trace_napi_poll from DECLARE_EVENT to TRACE_EVENT to improve
the usability of napi_poll tracepoint.

          <idle>-0     [001] 241302.750777: napi_poll: napi poll on napi struct f6acc480 for device eth3
          <idle>-0     [000] 241302.852389: napi_poll: napi poll on napi struct f5d0d70c for device eth1

An original patch is below.
http://marc.info/?l=linux-kernel&m=126021713809450&w=2
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

And add a fix by Steven Rostedt.
http://marc.info/?l=linux-kernel&m=126150506519173&w=2

Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
 include/trace/events/napi.h |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
index 188deca..8fe1e93 100644
--- a/include/trace/events/napi.h
+++ b/include/trace/events/napi.h
@@ -6,10 +6,31 @@
 
 #include <linux/netdevice.h>
 #include <linux/tracepoint.h>
+#include <linux/ftrace.h>
+
+#define NO_DEV "(no_device)"
+
+TRACE_EVENT(napi_poll,
 
-DECLARE_TRACE(napi_poll,
 	TP_PROTO(struct napi_struct *napi),
-	TP_ARGS(napi));
+
+	TP_ARGS(napi),
+
+	TP_STRUCT__entry(
+		__field(	struct napi_struct *,	napi)
+		__string(	dev_name, napi->dev ? napi->dev->name : NO_DEV)
+	),
+
+	TP_fast_assign(
+		__entry->napi = napi;
+		__assign_str(dev_name, napi->dev ? napi->dev->name : NO_DEV);
+	),
+
+	TP_printk("napi poll on napi struct %p for device %s",
+		__entry->napi, __get_str(dev_name))
+);
+
+#undef NO_DEV
 
 #endif /* _TRACE_NAPI_H_ */
 


^ permalink raw reply related

* [RFC PATCH v3 3/5] netdev: add tracepoints to netdev layer
From: Koki Sanagi @ 2010-07-20  0:47 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, davem, kaneshige.kenji, izumi.taku, kosaki.motohiro,
	nhorman, laijs, scott.a.mcmillan, rostedt, eric.dumazet, fweisbec,
	mathieu.desnoyers
In-Reply-To: <4C44F12F.5090908@jp.fujitsu.com>

This patch adds tracepoint to dev_queue_xmit, dev_hard_start_xmit and
netif_receive_skb. These tracepoints help you to monitor network driver's
input/output.

            sshd-4445  [001] 241367.066046: net_dev_queue: dev=eth3 skbaddr=dd6b2538 len=114
            sshd-4445  [001] 241367.066047: net_dev_xmit: dev=eth3 skbaddr=dd6b2538 len=114 rc=0
          <idle>-0     [001] 241367.067472: net_dev_receive: dev=eth3 skbaddr=f5e59000 len=52

Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
 include/trace/events/net.h |   75 ++++++++++++++++++++++++++++++++++++++++++++
 net/core/dev.c             |    5 +++
 net/core/net-traces.c      |    1 +
 3 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/include/trace/events/net.h b/include/trace/events/net.h
new file mode 100644
index 0000000..8a21361
--- /dev/null
+++ b/include/trace/events/net.h
@@ -0,0 +1,75 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM net
+
+#if !defined(_TRACE_NET_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NET_H
+
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/ip.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(net_dev_xmit,
+
+	TP_PROTO(struct sk_buff *skb,
+		 int rc),
+
+	TP_ARGS(skb, rc),
+
+	TP_STRUCT__entry(
+		__field(	void *,		skbaddr		)
+		__field(	unsigned int,	len		)
+		__field(	int,		rc		)
+		__string(	name,		skb->dev->name	)
+	),
+
+	TP_fast_assign(
+		__entry->skbaddr = skb;
+		__entry->len = skb->len;
+		__entry->rc = rc;
+		__assign_str(name, skb->dev->name);
+	),
+
+	TP_printk("dev=%s skbaddr=%p len=%u rc=%d",
+		__get_str(name), __entry->skbaddr, __entry->len, __entry->rc)
+);
+
+DECLARE_EVENT_CLASS(net_dev_template,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb),
+
+	TP_STRUCT__entry(
+		__field(	void *,		skbaddr		)
+		__field(	unsigned int,	len		)
+		__string(	name,		skb->dev->name	)
+	),
+
+	TP_fast_assign(
+		__entry->skbaddr = skb;
+		__entry->len = skb->len;
+		__assign_str(name, skb->dev->name);
+	),
+
+	TP_printk("dev=%s skbaddr=%p len=%u",
+		__get_str(name), __entry->skbaddr, __entry->len)
+)
+
+DEFINE_EVENT(net_dev_template, net_dev_queue,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template, net_dev_receive,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb)
+);
+#endif /* _TRACE_NET_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/core/dev.c b/net/core/dev.c
index 93b8929..4acfec6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -130,6 +130,7 @@
 #include <linux/jhash.h>
 #include <linux/random.h>
 #include <trace/events/napi.h>
+#include <trace/events/net.h>
 #include <linux/pci.h>
 
 #include "net-sysfs.h"
@@ -1955,6 +1956,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 		}
 
 		rc = ops->ndo_start_xmit(skb, dev);
+		trace_net_dev_xmit(skb, rc);
 		if (rc == NETDEV_TX_OK)
 			txq_trans_update(txq);
 		return rc;
@@ -1975,6 +1977,7 @@ gso:
 			skb_dst_drop(nskb);
 
 		rc = ops->ndo_start_xmit(nskb, dev);
+		trace_net_dev_xmit(nskb, rc);
 		if (unlikely(rc != NETDEV_TX_OK)) {
 			if (rc & ~NETDEV_TX_MASK)
 				goto out_kfree_gso_skb;
@@ -2165,6 +2168,7 @@ int dev_queue_xmit(struct sk_buff *skb)
 #ifdef CONFIG_NET_CLS_ACT
 	skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_EGRESS);
 #endif
+	trace_net_dev_queue(skb);
 	if (q->enqueue) {
 		rc = __dev_xmit_skb(skb, q, dev, txq);
 		goto out;
@@ -2939,6 +2943,7 @@ int netif_receive_skb(struct sk_buff *skb)
 	if (netdev_tstamp_prequeue)
 		net_timestamp_check(skb);
 
+	trace_net_dev_receive(skb);
 #ifdef CONFIG_RPS
 	{
 		struct rps_dev_flow voidflow, *rflow = &voidflow;
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index afa6380..7f1bb2a 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -26,6 +26,7 @@
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/skb.h>
+#include <trace/events/net.h>
 #include <trace/events/napi.h>
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(kfree_skb);


^ permalink raw reply related

* [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
From: Koki Sanagi @ 2010-07-20  0:49 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, davem, kaneshige.kenji, izumi.taku, kosaki.motohiro,
	nhorman, laijs, scott.a.mcmillan, rostedt, eric.dumazet, fweisbec,
	mathieu.desnoyers
In-Reply-To: <4C44F12F.5090908@jp.fujitsu.com>

[RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
This patch adds tracepoint to consume_skb, dev_kfree_skb_irq and
skb_free_datagram_locked. Combinating with tracepoint on dev_hard_start_xmit,
we can check how long it takes to free transmited packets. And using it, we can
calculate how many packets driver had at that time. It is useful when a drop of
transmited packet is a problem.

          <idle>-0     [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
          <idle>-0     [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840

        udp-recv-302   [001] 515031.206008: skb_free_datagram_locked: skbaddr=f5b1d900


Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
 include/trace/events/skb.h |   42 ++++++++++++++++++++++++++++++++++++++++++
 net/core/datagram.c        |    1 +
 net/core/dev.c             |    2 ++
 net/core/skbuff.c          |    1 +
 4 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 4b2be6d..84c9041 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -35,6 +35,48 @@ TRACE_EVENT(kfree_skb,
 		__entry->skbaddr, __entry->protocol, __entry->location)
 );
 
+DECLARE_EVENT_CLASS(free_skb,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb),
+
+	TP_STRUCT__entry(
+		__field(	void *,	skbaddr	)
+	),
+
+	TP_fast_assign(
+		__entry->skbaddr = skb;
+	),
+
+	TP_printk("skbaddr=%p", __entry->skbaddr)
+
+);
+
+DEFINE_EVENT(free_skb, consume_skb,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb)
+
+);
+
+DEFINE_EVENT(free_skb, dev_kfree_skb_irq,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb)
+
+);
+
+DEFINE_EVENT(free_skb, skb_free_datagram_locked,
+
+	TP_PROTO(struct sk_buff *skb),
+
+	TP_ARGS(skb)
+
+);
+
 TRACE_EVENT(skb_copy_datagram_iovec,
 
 	TP_PROTO(const struct sk_buff *skb, int len),
diff --git a/net/core/datagram.c b/net/core/datagram.c
index f5b6f43..1ea32a0 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -231,6 +231,7 @@ void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb)
 {
 	bool slow;
 
+	trace_skb_free_datagram_locked(skb);
 	if (likely(atomic_read(&skb->users) == 1))
 		smp_rmb();
 	else if (likely(!atomic_dec_and_test(&skb->users)))
diff --git a/net/core/dev.c b/net/core/dev.c
index 4acfec6..d979847 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,6 +131,7 @@
 #include <linux/random.h>
 #include <trace/events/napi.h>
 #include <trace/events/net.h>
+#include <trace/events/skb.h>
 #include <linux/pci.h>
 
 #include "net-sysfs.h"
@@ -1581,6 +1582,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
 		struct softnet_data *sd;
 		unsigned long flags;
 
+		trace_dev_kfree_skb_irq(skb);
 		local_irq_save(flags);
 		sd = &__get_cpu_var(softnet_data);
 		skb->next = sd->completion_queue;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 34432b4..a7b4036 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
 		smp_rmb();
 	else if (likely(!atomic_dec_and_test(&skb->users)))
 		return;
+	trace_consume_skb(skb);
 	__kfree_skb(skb);
 }
 EXPORT_SYMBOL(consume_skb);


^ permalink raw reply related


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