Netdev List
 help / color / mirror / Atom feed
* Re: txqueuelen has wrong units; should be time
From: Stephen Hemminger @ 2011-03-02  7:07 UTC (permalink / raw)
  To: Mikael Abrahamsson
  Cc: John Heffner, Bill Sommerfeld, Hagen Paul Pfeifer, Albert Cahalan,
	Jussi Kivilinna, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <alpine.DEB.1.10.1103020736260.7942@uplift.swm.pp.se>

On Wed, 2 Mar 2011 07:41:30 +0100 (CET)
Mikael Abrahamsson <swmike@swm.pp.se> wrote:

> On Tue, 1 Mar 2011, Stephen Hemminger wrote:
> 
> > It is possible to build an equivalent to WRED out existing GRED queuing
> > discipline but it does require a lot of tc knowledge to get right.  
> 
> To me who has worked with cisco routers for 10+ years and who is used to 
> the different variants Cisco use, tc is just weird. It must come from a 
> completely different school of thinking compared to what router people are 
> used to, because I have tried and failed twice to do anything sensible 
> with it.

Vyatta has scripting that handles all that:

vyatta@napa:~$ configure
[edit]
yatta@napa# set traffic-policy random-detect MyWFQ bandwidth 1gbps
[edit]
vyatta@napa# set interfaces ethernet eth0 traffic-policy out MyWFQ 
[edit]
vyatta@napa# commit
[edit]
vyatta@napa# exit
vyatta@napa:~$ show queueing ethernet eth0

eth0 Queueing:
Class      Policy               Sent     Rate  Dropped Overlimit  Backlog
root       weighted-random     16550                 0        0        0

vyatta@napa:~$ /sbin/tc qdisc show dev eth0
qdisc dsmark 1: root refcnt 2 indices 0x0008 set_tc_index 
qdisc gred 2: parent 1: 
 DP:0 (prio 8) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 82 (bytes 9540)  ewma 3 Plog 17 Scell_log 3
 DP:1 (prio 7) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 17 Scell_log 2
 DP:2 (prio 6) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 17 Scell_log 2
 DP:3 (prio 5) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 16 Scell_log 2
 DP:4 (prio 4) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 16 Scell_log 2
 DP:5 (prio 3) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 16 Scell_log 2
 DP:6 (prio 2) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 2 Plog 15 Scell_log 2
 DP:7 (prio 1) Average Queue 0b Measured Queue 0b  
	 Packet drops: 0 (forced 0 early 0)  
	 Packet totals: 0 (bytes 0)  ewma 1 Plog 15 Scell_log 1



QoS on Cisco has different/other problems mostly because various groups
tried to fix the QoS problem over time and never got it quite right.
Also WRED is not default on faster links because it can't be done
fast enough.


^ permalink raw reply

* Re: [PATCH] ipv6: Make icmp route lookup code a bit clearer.
From: Herbert Xu @ 2011-03-02  7:02 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20110301.221543.226770795.davem@davemloft.net>

On Tue, Mar 01, 2011 at 10:15:43PM -0800, David Miller wrote:
> 
> The route lookup code in icmpv6_send() is slightly tricky as a result of
> having to handle all of the requirements of RFC 4301 host relookups.
> 
> Pull the route resolution into a seperate function, so that the error
> handling and route reference counting is hopefully easier to see and
> contained wholly within this new routine.
> 
> Signed-off-by: David S. Miller <davem@davemloft.net>

Looks good to me too.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [Bug-Fix][Patch 1/1] dccp: fix oops on Reset after close
From: David Miller @ 2011-03-02  7:02 UTC (permalink / raw)
  To: gerrit; +Cc: netdev, jhovold
In-Reply-To: <20110302063447.GB4902@gerrit.erg.abdn.ac.uk>

From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Date: Wed, 2 Mar 2011 07:34:47 +0100

> could you please apply the below bug fix for a condition where a simple packet
> reception can trigger oops.

Applied, thanks Gerrit.

^ permalink raw reply

* Re: inet: Replace left-over references to inet->cork
From: David Miller @ 2011-03-02  7:01 UTC (permalink / raw)
  To: herbert; +Cc: rick.jones2, therbert, wsommerfeld, daniel.baluta, netdev, tgraf
In-Reply-To: <20110302061517.GA17891@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 2 Mar 2011 14:15:17 +0800

> On Tue, Mar 01, 2011 at 08:36:47PM +0800, Herbert Xu wrote:
>> inet: Remove explicit write references to sk/inet in ip_append_data
> 
> Just found a couple of spots where inet->cork was still used
> instead of just cork.
> 
> inet: Replace left-over references to inet->cork

Applied, thanks Herbert.

^ permalink raw reply

* Re: [PATCH net-next] pfkey: fix warning
From: David Miller @ 2011-03-02  6:52 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20110301223913.2caf53c3@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 1 Mar 2011 22:39:13 -0800

> If CONFIG_NET_KEY_MIGRATE is not defined the arguments of
> pfkey_migrate stub do not match causing warning.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied, thanks!

^ permalink raw reply

* Re: txqueuelen has wrong units; should be time
From: Mikael Abrahamsson @ 2011-03-02  6:41 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: John Heffner, Bill Sommerfeld, Hagen Paul Pfeifer, Albert Cahalan,
	Jussi Kivilinna, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <20110301222531.24832a93@nehalam>

On Tue, 1 Mar 2011, Stephen Hemminger wrote:

> It is possible to build an equivalent to WRED out existing GRED queuing
> discipline but it does require a lot of tc knowledge to get right.

To me who has worked with cisco routers for 10+ years and who is used to 
the different variants Cisco use, tc is just weird. It must come from a 
completely different school of thinking compared to what router people are 
used to, because I have tried and failed twice to do anything sensible 
with it.

> The inventor of RED (Van Jacobsen) has issues with WRED because of the 
> added complexity of queue selection. RED requires some parameters which 
> the average user has no idea how to set.

Of course there are issues and some of them can be adressed by simply 
lowering the queue depth. Yes, that might bring down the performance of 
some sessions, but for most of the interactive traffic, never buffering 
more than 40ms is a good thing.

> There are several problems with RED that prevent prevent VJ from
> recommending it in the current form.

Ask if he prefers FIFO+tail drop to RED in current form.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* [PATCH net-next] pfkey: fix warning
From: Stephen Hemminger @ 2011-03-02  6:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

If CONFIG_NET_KEY_MIGRATE is not defined the arguments of
pfkey_migrate stub do not match causing warning.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/key/af_key.c	2011-03-01 22:32:30.608511083 -0800
+++ b/net/key/af_key.c	2011-03-01 22:33:55.703791987 -0800
@@ -2560,7 +2560,7 @@ static int pfkey_migrate(struct sock *sk
 }
 #else
 static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
-			 struct sadb_msg *hdr, void **ext_hdrs)
+			 const struct sadb_msg *hdr, void * const *ext_hdrs)
 {
 	return -ENOPROTOOPT;
 }

^ permalink raw reply

* [Bug-Fix][Patch 1/1] dccp: fix oops on Reset after close
From: Gerrit Renker @ 2011-03-02  6:34 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Johan Hovold

Dave,

could you please apply the below bug fix for a condition where a simple packet
reception can trigger oops.

>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Patch <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
dccp: fix oops on Reset after close

This fixes a bug in the order of dccp_rcv_state_process() that still permitted
reception even after closing the socket. A Reset after close thus causes a NULL
pointer dereference by not preventing operations on an already torn-down socket.

 dccp_v4_do_rcv() 
	|
	| state other than OPEN
	v
 dccp_rcv_state_process()
	|
	| DCCP_PKT_RESET
	v
 dccp_rcv_reset()
	|
	v
 dccp_time_wait()

 WARNING: at net/ipv4/inet_timewait_sock.c:141 __inet_twsk_hashdance+0x48/0x128()
 Modules linked in: arc4 ecb carl9170 rt2870sta(C) mac80211 r8712u(C) crc_ccitt ah
 [<c0038850>] (unwind_backtrace+0x0/0xec) from [<c0055364>] (warn_slowpath_common)
 [<c0055364>] (warn_slowpath_common+0x4c/0x64) from [<c0055398>] (warn_slowpath_n)
 [<c0055398>] (warn_slowpath_null+0x1c/0x24) from [<c02b72d0>] (__inet_twsk_hashd)
 [<c02b72d0>] (__inet_twsk_hashdance+0x48/0x128) from [<c031caa0>] (dccp_time_wai)
 [<c031caa0>] (dccp_time_wait+0x40/0xc8) from [<c031c15c>] (dccp_rcv_state_proces)
 [<c031c15c>] (dccp_rcv_state_process+0x120/0x538) from [<c032609c>] (dccp_v4_do_)
 [<c032609c>] (dccp_v4_do_rcv+0x11c/0x14c) from [<c0286594>] (release_sock+0xac/0)
 [<c0286594>] (release_sock+0xac/0x110) from [<c031fd34>] (dccp_close+0x28c/0x380)
 [<c031fd34>] (dccp_close+0x28c/0x380) from [<c02d9a78>] (inet_release+0x64/0x70)

The fix is by testing the socket state first. Receiving a packet in Closed state
now also produces the required "No connection" Reset reply of RFC 4340, 8.3.1.

Reported-and-tested-by: Johan Hovold <jhovold@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/input.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -614,6 +614,9 @@ int dccp_rcv_state_process(struct sock *
 		/* Caller (dccp_v4_do_rcv) will send Reset */
 		dcb->dccpd_reset_code = DCCP_RESET_CODE_NO_CONNECTION;
 		return 1;
+	} else if (sk->sk_state == DCCP_CLOSED) {
+		dcb->dccpd_reset_code = DCCP_RESET_CODE_NO_CONNECTION;
+		return 1;
 	}
 
 	if (sk->sk_state != DCCP_REQUESTING && sk->sk_state != DCCP_RESPOND) {
@@ -668,10 +671,6 @@ int dccp_rcv_state_process(struct sock *
 	}
 
 	switch (sk->sk_state) {
-	case DCCP_CLOSED:
-		dcb->dccpd_reset_code = DCCP_RESET_CODE_NO_CONNECTION;
-		return 1;
-
 	case DCCP_REQUESTING:
 		queued = dccp_rcv_request_sent_state_process(sk, skb, dh, len);
 		if (queued >= 0)

^ permalink raw reply

* Re: txqueuelen has wrong units; should be time
From: Stephen Hemminger @ 2011-03-02  6:25 UTC (permalink / raw)
  To: Mikael Abrahamsson
  Cc: John Heffner, Bill Sommerfeld, Hagen Paul Pfeifer, Albert Cahalan,
	Jussi Kivilinna, Eric Dumazet, linux-kernel, netdev
In-Reply-To: <alpine.DEB.1.10.1103010137400.7942@uplift.swm.pp.se>

On Tue, 1 Mar 2011 01:46:51 +0100 (CET)
Mikael Abrahamsson <swmike@swm.pp.se> wrote:

> On Mon, 28 Feb 2011, John Heffner wrote:
> 
> > Right... while I generally agree that a fixed-length drop-tail queue 
> > isn't optimal, isn't this problem what the various AQM schemes try to 
> > solve?
> 
> I am not an expert on exactly how Linux does this, but for Cisco and for 
> instance ATM interfaces, there are two stages of queuing. One is the 
> "hardware queue", which is a FIFO queue going into the ATM framer. If one 
> wants low CPU usage, then this needs to be high so multiple packets can be 
> put there per interrupt. Since AQM is working before this, it also means 
> the low-latency-queue will have a higher latency as it ends up behind 
> larger packets in the hw queue.
> 
> So on what level does the AQM work in Linux? Does it work similarily, that 
> txqueuelen is a FIFO queue to the hardware that AQM feeds packets into?
> 
> Also, when one uses WRED the thinking is generally to keep the average 
> queue len down, but still allow for bursts by dynamically changing the 
> drop probability and where it happens. When there is no queuing, allow for 
> big queue (so it can fill up if needed), but if the queue is large for 
> several seconds, start to apply WRED to bring it down.
> 
> There is generally no need at all to constantly buffer > 50 ms of data, 
> then it's better to just start selectively dropping it. In time of 
> burstyness (perhaps when re-routing traffic) there is need to buffer 
> 200-500ms of during perhaps 1-2 seconds before things stabilize.
> 
> So one queuing scheme and one queue limit isn't going to solve this, there 
> need to be some dynamic built into the system for it to work well.
> 
> AQM needs to feed into a relatively short hw queue and AQM needs to exist 
> on output also when the traffic is sourced from the box itself, no tonly 
> routed. It would also help if the default would be to use let's say 25% of 
> the bandwidth for smaller packets (< 200 bytes or so) which generally are 
> for interactive uses or are ACKs.
> 

It is possible to build an equivalent to WRED out existing GRED queuing
discipline but it does require a lot of tc knowledge to get right.
The inventor of RED (Van Jacobsen) has issues with WRED because of
the added complexity of queue selection. RED requires some parameters
which the average user has no idea how to set.

There are several problems with RED that prevent prevent VJ from
recommending it in the current form.

  http://gettys.wordpress.com/2010/12/17/red-in-a-different-light/


-- 

^ permalink raw reply

* inet: Replace left-over references to inet->cork
From: Herbert Xu @ 2011-03-02  6:15 UTC (permalink / raw)
  To: David Miller, rick.jones2, therbert, wsommerfeld, daniel.baluta,
	netdev, Thomas Graf
In-Reply-To: <E1PuOop-0001xh-HK@gondolin.me.apana.org.au>

On Tue, Mar 01, 2011 at 08:36:47PM +0800, Herbert Xu wrote:
> inet: Remove explicit write references to sk/inet in ip_append_data

Just found a couple of spots where inet->cork was still used
instead of just cork.

inet: Replace left-over references to inet->cork

The patch to replace inet->cork with cork left out two spots in
__ip_append_data that can result in bogus packet construction.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 460308c..3e8637c 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -789,7 +789,7 @@ static int __ip_append_data(struct sock *sk, struct sk_buff_head *queue,
 	struct inet_sock *inet = inet_sk(sk);
 	struct sk_buff *skb;
 
-	struct ip_options *opt = inet->cork.opt;
+	struct ip_options *opt = cork->opt;
 	int hh_len;
 	int exthdrlen;
 	int mtu;
@@ -803,7 +803,7 @@ static int __ip_append_data(struct sock *sk, struct sk_buff_head *queue,
 	exthdrlen = transhdrlen ? rt->dst.header_len : 0;
 	length += exthdrlen;
 	transhdrlen += exthdrlen;
-	mtu = inet->cork.fragsize;
+	mtu = cork->fragsize;
 
 	hh_len = LL_RESERVED_SPACE(rt->dst.dev);

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related

* [PATCH] ipv6: Make icmp route lookup code a bit clearer.
From: David Miller @ 2011-03-02  6:15 UTC (permalink / raw)
  To: netdev; +Cc: herbert


The route lookup code in icmpv6_send() is slightly tricky as a result of
having to handle all of the requirements of RFC 4301 host relookups.

Pull the route resolution into a seperate function, so that the error
handling and route reference counting is hopefully easier to see and
contained wholly within this new routine.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/icmp.c |  117 +++++++++++++++++++++++++++++++------------------------
 1 files changed, 66 insertions(+), 51 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index a31d91b..e332bae 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -300,6 +300,70 @@ static void mip6_addr_swap(struct sk_buff *skb)
 static inline void mip6_addr_swap(struct sk_buff *skb) {}
 #endif
 
+static struct dst_entry *icmpv6_route_lookup(struct net *net, struct sk_buff *skb,
+					     struct sock *sk, struct flowi *fl)
+{
+	struct dst_entry *dst, *dst2;
+	struct flowi fl2;
+	int err;
+
+	err = ip6_dst_lookup(sk, &dst, fl);
+	if (err)
+		return ERR_PTR(err);
+
+	/*
+	 * We won't send icmp if the destination is known
+	 * anycast.
+	 */
+	if (((struct rt6_info *)dst)->rt6i_flags & RTF_ANYCAST) {
+		LIMIT_NETDEBUG(KERN_DEBUG "icmpv6_send: acast source\n");
+		dst_release(dst);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* No need to clone since we're just using its address. */
+	dst2 = dst;
+
+	err = xfrm_lookup(net, &dst, fl, sk, 0);
+	switch (err) {
+	case 0:
+		if (dst != dst2)
+			return dst;
+		break;
+	case -EPERM:
+		dst = NULL;
+		break;
+	default:
+		return ERR_PTR(err);
+	}
+
+	err = xfrm_decode_session_reverse(skb, &fl2, AF_INET6);
+	if (err)
+		goto relookup_failed;
+
+	err = ip6_dst_lookup(sk, &dst2, &fl2);
+	if (err)
+		goto relookup_failed;
+
+	err = xfrm_lookup(net, &dst2, &fl2, sk, XFRM_LOOKUP_ICMP);
+	switch (err) {
+	case 0:
+		dst_release(dst);
+		dst = dst2;
+		break;
+	case -EPERM:
+		dst_release(dst);
+		return ERR_PTR(err);
+	default:
+		goto relookup_failed;
+	}
+
+relookup_failed:
+	if (dst)
+		return dst;
+	return ERR_PTR(err);
+}
+
 /*
  *	Send an ICMP message in response to a packet in error
  */
@@ -312,10 +376,8 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	struct ipv6_pinfo *np;
 	struct in6_addr *saddr = NULL;
 	struct dst_entry *dst;
-	struct dst_entry *dst2;
 	struct icmp6hdr tmp_hdr;
 	struct flowi fl;
-	struct flowi fl2;
 	struct icmpv6_msg msg;
 	int iif = 0;
 	int addr_type = 0;
@@ -408,57 +470,10 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	if (!fl.oif && ipv6_addr_is_multicast(&fl.fl6_dst))
 		fl.oif = np->mcast_oif;
 
-	err = ip6_dst_lookup(sk, &dst, &fl);
-	if (err)
-		goto out;
-
-	/*
-	 * We won't send icmp if the destination is known
-	 * anycast.
-	 */
-	if (((struct rt6_info *)dst)->rt6i_flags & RTF_ANYCAST) {
-		LIMIT_NETDEBUG(KERN_DEBUG "icmpv6_send: acast source\n");
-		goto out_dst_release;
-	}
-
-	/* No need to clone since we're just using its address. */
-	dst2 = dst;
-
-	err = xfrm_lookup(net, &dst, &fl, sk, 0);
-	switch (err) {
-	case 0:
-		if (dst != dst2)
-			goto route_done;
-		break;
-	case -EPERM:
-		dst = NULL;
-		break;
-	default:
+	dst = icmpv6_route_lookup(net, skb, sk, &fl);
+	if (IS_ERR(dst))
 		goto out;
-	}
-
-	if (xfrm_decode_session_reverse(skb, &fl2, AF_INET6))
-		goto relookup_failed;
-
-	if (ip6_dst_lookup(sk, &dst2, &fl2))
-		goto relookup_failed;
-
-	err = xfrm_lookup(net, &dst2, &fl2, sk, XFRM_LOOKUP_ICMP);
-	switch (err) {
-	case 0:
-		dst_release(dst);
-		dst = dst2;
-		break;
-	case -EPERM:
-		goto out_dst_release;
-	default:
-relookup_failed:
-		if (!dst)
-			goto out;
-		break;
-	}
 
-route_done:
 	if (ipv6_addr_is_multicast(&fl.fl6_dst))
 		hlimit = np->mcast_hops;
 	else
-- 
1.7.4.1


^ permalink raw reply related

* fcoe: correct checking for bonding
From: Jiri Pirko @ 2011-03-02  6:05 UTC (permalink / raw)
  To: linux-scsi; +Cc: devel, robert.w.love, netdev, fubar, joe.eykholt

Check for bonding master and refuse to use that.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/scsi/fcoe/fcoe.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9f9600b..3becc6a 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
 	}
 
 	/* Do not support for bonding device */
-	if ((netdev->priv_flags & IFF_MASTER_ALB) ||
-	    (netdev->priv_flags & IFF_SLAVE_INACTIVE) ||
-	    (netdev->priv_flags & IFF_MASTER_8023AD)) {
+	if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
 		FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
 		return -EOPNOTSUPP;
 	}
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH 0/7] Final BKL removal, take 2
From: Greg KH @ 2011-03-02  4:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, Andi Kleen, Andrew Hendry, Arnaldo Carvalho de Melo,
	David Miller, Eric Dumazet, Evgeniy Dushistov, linux-fsdevel,
	linux-x25, Mauro Carvalho Chehab, Max Vozeler, Mikulas Patocka,
	netdev, Nick Bowler, Nick Piggin, Palash Bandyopadhyay,
	Takahiro Hirofuchi
In-Reply-To: <1299021191-17961-1-git-send-email-arnd@arndb.de>

On Wed, Mar 02, 2011 at 12:13:04AM +0100, Arnd Bergmann wrote:
> This is the set of patches that remain from
> my previous submission one month ago. I've
> dropped the ones that have either gone into
> linux-next or got a sufficient number of
> Acked-by:s so I put them into my own tree.
> 
> I've updated the usbip, hpfs, ufs and appletalk
> patches according to the feedback I got.
> 
> If possible, I'd like the three networking patches
> to go through the net-next tree, and the two
> staging patches through the staging tree. I'll
> add the other ones to my own series if I hear
> no objections.

I'll queue up the staging patches in the staging-next tree in a day or
so, thanks for digging them up.

greg k-h

^ permalink raw reply

* Re: [PATCH] ipv4: Make icmp route lookup code a bit clearer.
From: David Miller @ 2011-03-02  4:53 UTC (permalink / raw)
  To: herbert; +Cc: netdev
In-Reply-To: <20110302004325.GB15009@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 2 Mar 2011 08:43:25 +0800

> Looks good to me.

Thanks for reviewing.

^ permalink raw reply

* Re: [PATCH] Fix possible null pointer dereference in ariadne.c
From: Huzaifa Sidhpurwala @ 2011-03-02  4:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, geert, tj, eric.dumazet
In-Reply-To: <20110301.115300.112595321.davem@davemloft.net>

On 03/02/2011 01:23 AM, David Miller wrote:
> From: huzaifas@redhat.com
> Date: Tue,  1 Mar 2011 17:03:27 +0530
> 
>> +    *lance = (struct Am79C960*)dev->base_addr;
> 
> If you had actually compiled this, the compiler would have given
> you a warning shoing that the leading "*" in "*lance" is not
> appropriate for this assignment.
i did build, but looks like something happened after that :(
Sorry about the mess-up , will repost the patch.

^ permalink raw reply

* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Hugh Dickins @ 2011-03-02  4:31 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Lai Jiangshan, Ingo Molnar, Paul E. McKenney,
	Eric Dumazet, David S. Miller, Matt Mackall, linux-mm,
	linux-kernel, netdev
In-Reply-To: <alpine.DEB.2.00.1103010909320.6253@router.home>

On Tue, Mar 1, 2011 at 7:11 AM, Christoph Lameter <cl@linux.com> wrote:
> On Tue, 1 Mar 2011, Pekka Enberg wrote:
>
> > The SLAB and SLUB patches are fine by me if there are going to be real
> > users for this. Christoph, Paul?
>
> The solution is a bit overkill. It would be much simpler to add a union to
> struct page that has lru and the rcu in there similar things can be done
> for SLAB and the network layer. A similar issue already exists for the
> spinlock in struct page. Lets follow the existing way of handling this.
>
> Struct page may be larger for debugging purposes already because of the
> need for extended spinlock data.

That was so for a long time, but I stopped it just over a year ago
with commit a70caa8ba48f21f46d3b4e71b6b8d14080bbd57a, stop ptlock
enlarging struct page.

Partly out of shame at how large struct page was growing when lockdep
is on, but also a subtle KSM reason which might apply here too: KSM
relies on the content of page->mapping to be kernel pointer to a
relevant structure, NULLed when the page is freed.

If a union leads to "random junk" overwriting the page->mapping field
when the page is reused, and that junk could resemble the pointer in
question, then KSM would mistakenly think it still owned the page.
Very remote chance, and maybe it amounts to no more than a leak.  But
I'd still prefer we keep page->mapping for pointers (sometimes with
lower bits set as flags).

Hugh

^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-02  3:44 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Thomas Graf, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <20110302030902.GA16416@gondor.apana.org.au>

Le mercredi 02 mars 2011 à 11:09 +0800, Herbert Xu a écrit :
> On Wed, Mar 02, 2011 at 03:56:38AM +0100, Eric Dumazet wrote:
> >
> > Anyway, the SYN-ACK is generated by softirq, not really user choice.
> > CPU depends if NIC is RX multiqueue or RPS is setup.
> 
> Which is exactly what we want.  The RX queue selection should
> determine the TX cpu.
> 

This is working today with RFS/XPS. Or it should, indirectly.

OOO problem is handled as well.




^ permalink raw reply

* Re: [PATCH] sysctl: ipv6: use correct net in ipv6_sysctl_rtcache_flush
From: Shan Wei @ 2011-03-02  3:25 UTC (permalink / raw)
  To: Lucian Adrian Grijincu
  Cc: netdev, David S . Miller, Daniel Lezcano, Benjamin Thery
In-Reply-To: <1298612883-18762-1-git-send-email-lucian.grijincu@gmail.com>

Lucian Adrian Grijincu wrote, at 02/25/2011 01:48 PM:
> Before this patch issuing these commands:
> 
>   fd = open("/proc/sys/net/ipv6/route/flush")
>   unshare(CLONE_NEWNET)
>   write(fd, "stuff")
> 
> would flush the newly created net, not the original one.

After appling your patch, when excuting above commands, 
router cache still not be flushed in init net namespace.
But IPv4 is ok.

Host1:                   Host2 
ping6 Host2
                        (shell1)                                 
			open(/proc/sys/net/ipv6/route/flush)
			unshare(CLONE_NEWNET)
			while(1) {write(fd, "stuff")}           


			(shell2)  
                        ip -6 route show cache <==1*
			1*: IPv6 will show one route cache entry.
			    But IPv4 shows nothing.

As changlog said, after this patch, shell1 only fulsh the original
net namespace, not the newly created one. But from shell2, we 
can see that IPv6 route cache is not flushed.

Have i missed something?


-- 
Best Regards
-----
Shan Wei

> 
> The equivalent ipv4 code is correct (stores the net inside ->extra1).
> ---
>  net/ipv6/route.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f786aed..522563e 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2604,14 +2604,16 @@ static
>  int ipv6_sysctl_rtcache_flush(ctl_table *ctl, int write,
>  			      void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> -	struct net *net = current->nsproxy->net_ns;
> -	int delay = net->ipv6.sysctl.flush_delay;
> -	if (write) {
> -		proc_dointvec(ctl, write, buffer, lenp, ppos);
> -		fib6_run_gc(delay <= 0 ? ~0UL : (unsigned long)delay, net);
> -		return 0;
> -	} else
> +	struct net *net;
> +	int delay;
> +	if (!write)
>  		return -EINVAL;
> +
> +	net = (struct net *)ctl->extra1;
> +	delay = net->ipv6.sysctl.flush_delay;
> +	proc_dointvec(ctl, write, buffer, lenp, ppos);
> +	fib6_run_gc(delay <= 0 ? ~0UL : (unsigned long)delay, net);
> +	return 0;
>  }
>  
>  ctl_table ipv6_route_table_template[] = {
> @@ -2698,6 +2700,7 @@ struct ctl_table * __net_init ipv6_route_sysctl_init(struct net *net)
>  
>  	if (table) {
>  		table[0].data = &net->ipv6.sysctl.flush_delay;
> +		table[0].extra1 = net;
>  		table[1].data = &net->ipv6.ip6_dst_ops.gc_thresh;
>  		table[2].data = &net->ipv6.sysctl.ip6_rt_max_size;
>  		table[3].data = &net->ipv6.sysctl.ip6_rt_gc_min_interval;


^ permalink raw reply

* Re: txqueuelen has wrong units; should be time
From: Mikael Abrahamsson @ 2011-03-02  3:10 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Albert Cahalan, David Miller, johnwheffner, linville,
	jussi.kivilinna, linux-kernel, netdev
In-Reply-To: <1298964381.2676.58.camel@edumazet-laptop>

On Tue, 1 Mar 2011, Eric Dumazet wrote:

> We should allow some trafic spikes, or many applications will stop
> working. Unless all applications are fixed, we are stuck.
>
> Only if the queue stay loaded a long time (yet another parameter) we can
> try to drop packets.

Are we talking forwarding of packets or originating them ourselves, or 
trying to use the same mechanism for both?

In the case of routing a packet, I envision a WRED kind of behaviour is 
the most efficient.

<http://www.cisco.com/en/US/docs/ios/12_0s/feature/guide/12stbwr.html>

"QoS: Time-Based Thresholds for WRED and Queue Limit for the Cisco 12000 
Series Router" You can set the drop probabilites in milliseconds. 
Unfortunately ECN isn't supported on this platform but on other platforms 
it can be configured and used instead of WRED dropping packets.

For the case when we're ourselves originating the traffic (for instance to 
a wifi card with varying speed and jitter due to retransmits on the wifi 
layer), I think it's taking the too easy way out to use the same 
mechanisms (dropping packets or marking ECN for our own originated packets 
seems really weird), here we should be able to pushback information to the 
applications somehow and do prioritization between flows since we're 
sitting on all information ourselves including the application.

For this case, I think there is something to be learnt from:

<http://www.cisco.com/en/US/tech/tk39/tk824/technologies_tech_note09186a00800fbafc.shtml>

Here you have the IP part and the ATM part, and you can limit the number 
of cells/packets sent to the ATM hardware at any given time (this queue is 
FIFO so no AQM when the packet has been sent here). We need the same here, 
to properly keep latency down and make AQM work, the hardware FIFO queue 
needs to be kept low.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Herbert Xu @ 2011-03-02  3:09 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Thomas Graf, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <1299034598.2930.47.camel@edumazet-laptop>

On Wed, Mar 02, 2011 at 03:56:38AM +0100, Eric Dumazet wrote:
>
> Anyway, the SYN-ACK is generated by softirq, not really user choice.
> CPU depends if NIC is RX multiqueue or RPS is setup.

Which is exactly what we want.  The RX queue selection should
determine the TX cpu.

> All this discussion is about letting process scheduler decide TX queue,
> (because user/admin used cpu affinity) or let network stack drive
> scheduler : Please migrate this thread on this cpu.
> 
> Both schems should be allowed/configurable so that best results are
> available.

Whatever scheme we end up with, hashing different sockets running
in the same thread to different queues is just broken.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] sched: QFQ - quick fair queue scheduler (v2)
From: Fabio Checconi @ 2011-03-02  2:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Luigi Rizzo, netdev
In-Reply-To: <20110228171738.2cc8c9a0@nehalam>

Hi,

> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Mon, Feb 28, 2011 05:17:38PM -0800
>
> This is an implementation of the Quick Fair Queue scheduler developed
> by Fabio Checconi. The same algorithm is already implemented in ipfw
> in FreeBSD. Fabio had an earlier version developed on Linux, I just
> cleaned it up and tested it. All bugs are mine.
> 

  thanks for posting, I'm pretty sure that bugs are more likely to be
ours than yours :)

During the development of the algorithm we used a simple traffic
simulator to verify (informally) the timestamping and the guarantees
provided.  I've tested this version of the code with the simulator and
so far it worked fine, so I think that timestamping and guarantees should
be OK.

Please let us know if we can be of any help,
fabio

^ permalink raw reply

* Re: [PATCH 4/4 V2] net,rcu: don't assume the size of struct rcu_head
From: Eric Dumazet @ 2011-03-02  3:02 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: David Miller, mingo, paulmck, cl, penberg, mpm, linux-mm,
	linux-kernel, netdev
In-Reply-To: <4D6DAF86.2000407@cn.fujitsu.com>

Le mercredi 02 mars 2011 à 10:46 +0800, Lai Jiangshan a écrit :

> Is it the cause of false sharing? I thought that all are rare write(except __refcnt)
> since it is protected by RCU.
> 
> Do you allow me just move the seldom access rcu_head to the end of the structure
> and add pads before __refcnt? I guess it increases about 3% the size of dst_entry.


dst_entry is a base class.

Its included at the beginning of other structs.

Moving rcu_head "at the end" just move it right in the middle of upper
objects as a matter of fact. This might add one cache line miss on
critical network object. A complete audit is needed.

David is doing some changes in this area, so things move fast anyway.

> I accept that I leave this code as is, when I change rcu_head I will
notify you.
> 

Thanks


--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: SO_REUSEPORT - can it be done in kernel?
From: Eric Dumazet @ 2011-03-02  2:56 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Thomas Graf, David Miller, rick.jones2, therbert, wsommerfeld,
	daniel.baluta, netdev
In-Reply-To: <20110302023920.GA16072@gondor.apana.org.au>

Le mercredi 02 mars 2011 à 10:39 +0800, Herbert Xu a écrit :

> UDP is a datagram protocol, TCP is not.
> 
> Anyway, here is an alternate proposal.  When a TCP socket transmits
> for the first time (SYN or SYN-ACK), we pick a queue based on CPU and
> store it in the socket.  From then on we stick to that selection.
> 

Many TCP apps I know use one thread to perform listen/accept and a pool
of threads to handle each new conn.

Anyway, the SYN-ACK is generated by softirq, not really user choice.
CPU depends if NIC is RX multiqueue or RPS is setup.

All this discussion is about letting process scheduler decide TX queue,
(because user/admin used cpu affinity) or let network stack drive
scheduler : Please migrate this thread on this cpu.

Both schems should be allowed/configurable so that best results are
available.

> We would only allow changes if we can ensure that all transmitted
> packets have left the queue.  Or we just never change it like we
> do now.
> 

We do change in case of dst/route change. Each device can have different
number of TX queues.



^ permalink raw reply

* Re: [PATCH] bonding: added 802.3ad round-robin hashing policy and source mac selection mode
From: Jay Vosburgh @ 2011-03-02  2:56 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Oleg V. Ukhno, netdev, David S. Miller
In-Reply-To: <20110301152951.646451c6@nehalam>

Stephen Hemminger <shemminger@vyatta.com> wrote:

>On Wed, 2 Mar 2011 01:34:58 +0300
>"Oleg V. Ukhno" <olegu@yandex-team.ru> wrote:
>
>> Patch introduces two new (related) features to bonding module.
>> First feature is round-robin hashing policy, which is primarily
>> intended for use with 802.3ad mode, and puts every next IPv4 and
>> IPv6 packet into  next availables slave without taling into account
>> which layer3 and above protocol is used.
>> Second feature makes possible choosing which MAC-address will be set
>> in the transmitted packet - when set to src-mac it will force setting
>> slave's interface real MAC address as source MAC address in every
>> packet, sent via this slave interface.
>> Main goal of this patch is to make possible single TCP stream
>> equally striped for both transmitted and received packets over all
>> available slaves.
>> This operating mode is not fully 802.3ad compliant, and will cause
>> some packet reordering in TCP stream, to some kernel tuning may be
>> required.
>> For correct working enabling round-robin hashing policy plus using
>> real slave's MAC addresses as source MAC addresses in transmitted
>> packets requires specific switch setting)hashing mode for port-channel
>> ("etherchannel) should be set to src-mac or src-dst-mac to get
>> correct load-striping on the receiving host's etherchannel.
>> General requirements for using bonding in this operating mode are:
>> - even and preferrably equal number of slaves on sending and receiving
>> hosts;
>> - equal RTT between sending and receiving hosts on all slaves;
>> - switch capable of doing etherchannels and using src-mac or src-dst-mac
>> hashing policy for egress load striping
>> 
>> Signed-off-by: Oleg V. Ukhno <olegu@yandex-team.ru>
>
>It seems to me the whole bonding policy is getting so complex
>that the code is a mess. Perhaps it should be somehow linked into
>existing packet classification or firewall mechanisms.  This would
>increase the flexibility and reduce the amount of policy code
>in the bonding driver itself.

	Hmm.

	Yes, the number of special case knobs in bonding is getting
rather large, and there are one or two other proposals in the pipe
besides this one.

	It would be handy to be able to do things like run ebtables
style rules against traffic going in and out of the bond.  Right now
ebtables is pretty tightly coupled with the bridge, so we'd need to add
a whole new set of netfilter "bondtables" or something.  Or add hooks
for ebtables outside of the bridge.

	For this particular patch, the src-mac business could be handled
by a netfilter module.  The round-robin hash policy part would probably
have to stay in bonding.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Lai Jiangshan @ 2011-03-02  2:55 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Ingo Molnar, Paul E. McKenney, Eric Dumazet,
	David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <alpine.DEB.2.00.1103010909320.6253@router.home>

On 03/01/2011 11:11 PM, Christoph Lameter wrote:
> On Tue, 1 Mar 2011, Pekka Enberg wrote:
> 
>> The SLAB and SLUB patches are fine by me if there are going to be real
>> users for this. Christoph, Paul?
> 
> The solution is a bit overkill. It would be much simpler to add a union to
> struct page that has lru and the rcu in there similar things can be done
> for SLAB and the network layer. A similar issue already exists for the
> spinlock in struct page. Lets follow the existing way of handling this.

I don't want to impact the whole system too much to
touch struct page. The solution changes existed things little,
and the reversed data may just make use of the pad data.

> 
> Struct page may be larger for debugging purposes already because of the
> need for extended spinlock data.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ 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