Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-2.6] net/ipv6/udp.c: fix typo in flush_stack()
From: Jiri Pirko @ 2010-12-09 14:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1291902488.4063.28.camel@edumazet-laptop>

Thu, Dec 09, 2010 at 02:48:08PM CET, eric.dumazet@gmail.com wrote:
>Le jeudi 09 décembre 2010 à 14:40 +0100, Jiri Pirko a écrit :
>> skb1 should be passed as parameter to sk_rcvqueues_full() here.
>> 
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> 
>> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>> index 91def93..d2b268e 100644
>> --- a/net/ipv6/udp.c
>> +++ b/net/ipv6/udp.c
>> @@ -602,7 +602,7 @@ static void flush_stack(struct sock **stack, unsigned int count,
>>  
>>  		sk = stack[i];
>>  		if (skb1) {
>> -			if (sk_rcvqueues_full(sk, skb)) {
>> +			if (sk_rcvqueues_full(sk, skb1)) {
>>  				kfree_skb(skb1);
>>  				goto drop;
>>  			}
>
>net-2.6 ? Its the same truesize anyway...

I know it is.

>
>So it's not a bug, but a cosmetic change.
>(I am ok with it, but for net-next-2.6)

Anything you like.

>
>/*
> * Take into account size of receive queue and backlog queue
> */
>static inline bool sk_rcvqueues_full(const struct sock *sk, const struct sk_buff *skb)
>{
>        unsigned int qsize = sk->sk_backlog.len + atomic_read(&sk->sk_rmem_alloc);
>
>        return qsize + skb->truesize > sk->sk_rcvbuf;
>}
>
>

^ permalink raw reply

* [PATCH] inet6: Remove redundant unlikely()
From: Tobias Klauser @ 2010-12-09 14:37 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI <yosh
  Cc: netdev, kernel-janitors

IS_ERR() already implies unlikely(), so it can be omitted here.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 net/ipv6/af_inet6.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 54e8e42..059a3de 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -810,7 +810,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb, int features)
 	}
 	rcu_read_unlock();
 
-	if (unlikely(IS_ERR(segs)))
+	if (IS_ERR(segs))
 		goto out;
 
 	for (skb = segs; skb; skb = skb->next) {
-- 
1.7.0.4


^ permalink raw reply related

* RE: Adding Support for SG,GSO,GRO
From: Govindarajan, Sriramakrishnan @ 2010-12-09 14:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev@vger.kernel.org
In-Reply-To: <1291891816.4063.5.camel@edumazet-laptop>

Hi Eric,
> > Internally the module supports scatter gather DMA(which is currently not
> > exercised) but there is no HW checksum support.
> >
> > To specifically implement GRO, GSO support would it be sufficient to add
> > SG support to the driver? Are there other means of increasing the
> throughput
> > and decreasing the CPU loading?
> >
> > Any pointers to reference implementation for adding SG/GRO/GSO support
> will be helpful.
> 
> Adding GRO is pretty easy, since you already are NAPI.
> 
> call
> 	napi_gro_receive(&adapter->napi, skb)
> instead of
> 	netif_receive_skb(skb);
> 

[Sriram] I have tried this, but didn't see any improvement in performance.
For GRO and GSO, will adding SG capability alone suffice or the real gains
are realized only if HW checksum is also supported.

Can adding SG support alone yield any improvement?

Regards
Sriram

^ permalink raw reply

* Re: [PATCH net-next] bnx2x: Add Nic partitioning mode (57712 devices)
From: Eilon Greenstein @ 2010-12-09 14:49 UTC (permalink / raw)
  To: Matt Domsch
  Cc: Dimitris Michailidis, Dmitry Kravkov, davem@davemloft.net,
	netdev@vger.kernel.org, narendra_k@dell.com,
	jordan_hargrave@dell.com
In-Reply-To: <4CFD29BE.2060201@chelsio.com>

On Mon, 2010-12-06 at 10:21 -0800, Dimitris Michailidis wrote:
> Matt Domsch wrote:
...
> /sys/class/net/<ifname>/dev_id indicates the physical port <ifname> is 
> associated with.  At least a few drivers set up dev_id this way.
> 
> 

So we are on agreement? This can satisf all needs? If so, we will add
this scheme to the bnx2x as well.

Thanks,
Eilon



^ permalink raw reply

* [PATCH] stmmac: Remove redundant unlikely()
From: Tobias Klauser @ 2010-12-09 14:50 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev; +Cc: kernel-janitors

IS_ERR() already implies unlikely(), so it can be omitted here.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/stmmac/stmmac_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index c0dc785..20f803d 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -949,7 +949,7 @@ static int stmmac_sw_tso(struct stmmac_priv *priv, struct sk_buff *skb)
 	       skb, skb->len);
 
 	segs = skb_gso_segment(skb, priv->dev->features & ~NETIF_F_TSO);
-	if (unlikely(IS_ERR(segs)))
+	if (IS_ERR(segs))
 		goto sw_tso_end;
 
 	do {
-- 
1.7.0.4


^ permalink raw reply related

* Re: Adding Support for SG,GSO,GRO
From: Ben Hutchings @ 2010-12-09 15:02 UTC (permalink / raw)
  To: Govindarajan, Sriramakrishnan; +Cc: netdev@vger.kernel.org
In-Reply-To: <FCCFB4CDC6E5564B9182F639FC356087035F21D9F9@dbde02.ent.ti.com>

On Thu, 2010-12-09 at 16:03 +0530, Govindarajan, Sriramakrishnan wrote:
> Hi
> We have a NAPI compliant driver(net/drivers/davinci_emac.c), that does
> well at 10/100Mbps loads. Now the same controller/driver is used for
> 1000Mbps
> mode as well, where the CPU gets saturated easily
> 
> Internally the module supports scatter gather DMA(which is currently not
> exercised) but there is no HW checksum support.
> 
> To specifically implement GRO, GSO support would it be sufficient to add
> SG support to the driver? Are there other means of increasing the throughput
> and decreasing the CPU loading?
[...]

On the RX side, the driver is allocating buffers and the stack doesn't
actually care whether you can do DMA scatter.  However GRO depends on
hardware checksum validation.

On the TX side, NETIF_F_SG means that the stack may include data in the
skb by reference to arbitrary pages *even if their contents are still
being changed* (think sendfile()), which means it depends on hardware
checksum generation.

So you really have no choice - you must implement hardware checksum
offload if you want any of the others.

Ben.

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


^ permalink raw reply

* Re: ctnetlink loop
From: Holger Eitzenberger @ 2010-12-09 15:23 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, netdev, LKML, David S. Miller
In-Reply-To: <4D00B5CD.3050406@netfilter.org>

> Sorry, I finally found your email reporting this:
> 
> > nfnetlink: avoid unbound loop on busy Netlink socket

Fair enough, thanks.

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Brian Haley @ 2010-12-09 15:28 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: David Miller, netdev, Mahesh Kelkar, Lorenzo Colitti,
	YOSHIFUJI Hideaki, Stephen Hemminger, stable
In-Reply-To: <m1mxofa8dg.fsf_-_@fess.ebiederm.org>

On 12/08/2010 11:16 PM, Eric W. Biederman wrote:
> Finding the real bug is beyond me right now, but fixing the regression
> in disable_ipv6 is simple.  We can just delete ::1 when we bring down
> the loopback interface, and it will be restored automatically when we
> bring the loopback interface back up.

Hi Eric,

This would work as well, same check, different way.

Signed-off-by: Brian Haley <brian.haley@hp.com>

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 23cc8e1..5d16a9d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2728,7 +2728,8 @@ static int addrconf_ifdown(struct net_device *dev, int how)
 		   and not link-local, then retain it. */
 		if (!how &&
 		    (ifa->flags&IFA_F_PERMANENT) &&
-		    !(ipv6_addr_type(&ifa->addr) & IPV6_ADDR_LINKLOCAL)) {
+		    !(ipv6_addr_type(&ifa->addr) &
+		      (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_LOOPBACK))) {
 			list_move_tail(&ifa->if_list, &keep_list);
 
 			/* If not doing DAD on this address, just keep it. */

^ permalink raw reply related

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Stephen Hemminger @ 2010-12-09 16:27 UTC (permalink / raw)
  To: Brian Haley
  Cc: Eric W. Biederman, David Miller, netdev, Mahesh Kelkar,
	Lorenzo Colitti, YOSHIFUJI Hideaki, stable
In-Reply-To: <4D00F58A.2050307@hp.com>

On Thu, 09 Dec 2010 10:28:10 -0500
Brian Haley <brian.haley@hp.com> wrote:

> On 12/08/2010 11:16 PM, Eric W. Biederman wrote:
> > Finding the real bug is beyond me right now, but fixing the regression
> > in disable_ipv6 is simple.  We can just delete ::1 when we bring down
> > the loopback interface, and it will be restored automatically when we
> > bring the loopback interface back up.
> 
> Hi Eric,
> 
> This would work as well, same check, different way.
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 23cc8e1..5d16a9d 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2728,7 +2728,8 @@ static int addrconf_ifdown(struct net_device *dev, int how)
>  		   and not link-local, then retain it. */
>  		if (!how &&
>  		    (ifa->flags&IFA_F_PERMANENT) &&
> -		    !(ipv6_addr_type(&ifa->addr) & IPV6_ADDR_LINKLOCAL)) {
> +		    !(ipv6_addr_type(&ifa->addr) &
> +		      (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_LOOPBACK))) {
>  			list_move_tail(&ifa->if_list, &keep_list);
>  
>  			/* If not doing DAD on this address, just keep it. */

Checking the address type is incorrect. Any type of address can be applied to
loopback interface. If you look a couple lines there is a special case for
loopback which keeps the address.

> 		/* If just doing link down, and address is permanent
> 		   and not link-local, then retain it. */
> 		if (!how &&
> 		    (ifa->flags&IFA_F_PERMANENT) &&
> 		    !(ipv6_addr_type(&ifa->addr) & IPV6_ADDR_LINKLOCAL)) {
> 			list_move_tail(&ifa->if_list, &keep_list);
> 
> 			/* If not doing DAD on this address, just keep it. */
> 			if ((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
> 			    idev->cnf.accept_dad <= 0 ||
> 			    (ifa->flags & IFA_F_NODAD))

I think the problem is on coming back up, not on the down step.


^ permalink raw reply

* Re: [PATCH] bridge: Fix return value of br_multicast_add_group()
From: Stephen Hemminger @ 2010-12-09 16:29 UTC (permalink / raw)
  To: Tobias Klauser; +Cc: David S. Miller, bridge, netdev
In-Reply-To: <1291903356-30618-1-git-send-email-tklauser@distanz.ch>

On Thu,  9 Dec 2010 15:02:36 +0100
Tobias Klauser <tklauser@distanz.ch> wrote:

> If br_multicast_new_group returns NULL, we would return 0 (no error) to
> the caller, which is not what we want. Instead we should return -ENOMEM
> in this case.
> 
> Also replace IS_ERR(x) || !x by IS_ERR_OR_NULL(x)
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  net/bridge/br_multicast.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 326e599..d4e1e81 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -713,8 +713,11 @@ static int br_multicast_add_group(struct net_bridge *br,
>  
>  	mp = br_multicast_new_group(br, port, group);
>  	err = PTR_ERR(mp);
> -	if (unlikely(IS_ERR(mp) || !mp))
> +	if (IS_ERR_OR_NULL(mp)) {
> +		if (!mp)
> +			err = -ENOMEM;
>  		goto err;
> +	}
>  
>  	if (!port) {
>  		hlist_add_head(&mp->mglist, &br->mglist);

I would rather fix br_multicast_new_group so it never returns
NULL. Instead return PTR_ERR(-ENOMEM) on out of memory.

-- 

^ permalink raw reply

* Re: [regression, 2.6.37-rc1] 'ip link tap0 up' stuck in do_exit()
From: Américo Wang @ 2010-12-09 16:47 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Florian Mickler, Ingo Molnar, Américo Wang, Dave Chinner,
	Eric Dumazet, linux-kernel, netdev
In-Reply-To: <20101208140822.GA18681@redhat.com>

On Wed, Dec 08, 2010 at 03:08:22PM +0100, Oleg Nesterov wrote:
>On 12/08, Oleg Nesterov wrote:
>>
>> On 12/08, Oleg Nesterov wrote:
>> >
>> > On 12/08, Florian Mickler wrote:
>> > >
>> > > [ ccing Ingo and Oleg ] as suggested
>> >
>> > Well. Of course I can't explain this bug. But, looking at this email
>> > I do not see amything strange in exit/schedule/etc.
>> >
>> > > > >> > > > This is resulting in the command 'ip link set tap0 up' hanging as a zombie:
>> > > > >> > > >
>> > > > >> > > > root      3005     1  0 16:53 pts/3    00:00:00 /bin/sh /vm-images/qemu-ifup tap0
>> > > > >> > > > root      3011  3005  0 16:53 pts/3    00:00:00 /usr/bin/sudo /sbin/ip link set tap0 up
>> > > > >> > > > root      3012  3011  0 16:53 pts/3    00:00:00 [ip] <defunct>
>> >
>> > That is. ip is a zombie.
>>
>> And. I do not know if this matters or not, but "the command 'ip link
>> set tap0 up' hanging as a zombie" does not look right.
>>
>> This was spawned by
>>
>> > >> > > > if [ -n "$1" ];then
>> > >> > > >         /usr/bin/sudo /sbin/ip link set $1 up
>> > >> > > >         sleep 0.5s
>> > >> > > >         /usr/bin/sudo /usr/sbin/brctl addif $switch $1
>> > >> > > >      exit 0
>> > >> > > > fi
>>
>> The command does not hang. But it forks the child with pid == 3012,
>> this child exits.
>
>Damn, sorry for noise, forgot to mention...
>
>The parent's trace (pid == 3011) can be more useful. Say, if it
>hangs in do_wait(), then the kernel is obviously wrong.
>

Yeah, there is no point that a zombie can trigger a BUG_ON in kenrel.
But it is still interesting to know why it becomes a zombie...

^ permalink raw reply

* Re: rndis gadget: Inconsistent locking
From: Neil Jones @ 2010-12-09 17:00 UTC (permalink / raw)
  To: Michał Nazarewicz; +Cc: linux-usb, netdev
In-Reply-To: <op.vnenkvz67p4s8u@pikus>

> Does this help: <https://patchwork.kernel.org/patch/195562/>?

Yes cheers, warning gone and driver seems fine so far.

has this been accepted upstream ?

Neil

2010/12/8 Michał Nazarewicz <m.nazarewicz@samsung.com>:
> On Wed, 08 Dec 2010 16:11:30 +0100, Neil Jones <neiljay@gmail.com> wrote:
>>
>> Im getting another lockdep warning when using the RNDIS gadget:
>>
>>  WARNING: at kernel/softirq.c:98 ___local_bh_disable+0xc4/0xd0()
>>  Modules linked in: g_ether
>>
>>  Call trace:
>>  [<40003bf8>] _show_stack+0x68/0x7c
>>  [<40003c20>] _dump_stack+0x14/0x28
>>  [<40013c3c>] _warn_slowpath_common+0x5c/0x7c
>>  [<40013c74>] _warn_slowpath_null+0x18/0x2c
>>  [<4001b17c>] ___local_bh_disable+0xc0/0xd0
>>  [<4001b1a0>] _local_bh_disable+0x14/0x28
>>  [<402e57f8>] __raw_spin_lock_bh+0x18/0x54
>>  [<40257f4c>] _dev_txq_stats_fold+0x7c/0x13c
>>  [<402580c4>] _dev_get_stats+0xb8/0xc0
>>  [<781d4e60>] _rndis_msg_parser+0x288/0xa04 [g_ether]
>>  [<781d5600>] _rndis_command_complete+0x24/0x70 [g_ether]
>>  [<401d66fc>] _dwc_otg_request_done+0xd8/0x220
>>  [<401d928c>] _ep0_complete_request+0x3f4/0x578
>>  [<401d95bc>] _handle_ep0+0x1ac/0x146c
>>  [<401daf7c>] _dwc_otg_pcd_handle_in_ep_intr+0x1c0/0x8bc
>>  [<401db8dc>] _dwc_otg_pcd_handle_intr+0x264/0x294
>>  [<401d6288>] _dwc_otg_pcd_irq+0x10/0x30
>>  [<40054cf4>] _handle_IRQ_event+0x4c/0x184
>>  [<40057b4c>] _handle_level_irq+0xac/0x15c
>>  [<4000b204>] _metag_soc_irq_demux+0xac/0xb4
>>  [<40002dd4>] _do_IRQ+0x4c/0x78
>>  [<40004000>] _trigger_handler+0x38/0xac
>>  [<40000b18>] ___TBIBoingVec+0xc/0x10
>>  [<40003588>] _cpu_idle+0x54/0x78
>>
>>  no locks held by swapper/0.
>>  ---[ end trace 77ac3cfee0ae5b25 ]---
>
> Known problem.
>
>> It looks like we are calling spin_lock_bh in the completion function
>> which is running in hard_irq, I think the driver should defer handling
>> this msg (and maybe all requests) to a workqueue?
>
> Does this help: <https://patchwork.kernel.org/patch/195562/>?
>
> --
> Best regards,                                        _     _
> | Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
> | Computer Science,  Michał "mina86" Nazarewicz       (o o)
> +----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
>

^ permalink raw reply

* Re: [regression, 2.6.37-rc1] 'ip link tap0 up' stuck in do_exit()
From: Eric Dumazet @ 2010-12-09 17:07 UTC (permalink / raw)
  To: Américo Wang
  Cc: Oleg Nesterov, Florian Mickler, Ingo Molnar, Dave Chinner,
	linux-kernel, netdev
In-Reply-To: <20101209164748.GA3218@hack>

Le vendredi 10 décembre 2010 à 00:47 +0800, Américo Wang a écrit :

> Yeah, there is no point that a zombie can trigger a BUG_ON in kenrel.
> But it is still interesting to know why it becomes a zombie...
> 

A zombie is very easy to get.

Technically speaking, all processes die and become zombies, unless
parent said : signal(SIGCLD, SIG_IGN) before fork()

The parent is buggy (sudo in this case ?) and doesnt call wait() to
'free' one of its children.

^ permalink raw reply

* Re: [regression, 2.6.37-rc1] 'ip link tap0 up' stuck in do_exit()
From: Eric Dumazet @ 2010-12-09 17:09 UTC (permalink / raw)
  To: Américo Wang
  Cc: Oleg Nesterov, Florian Mickler, Ingo Molnar, Dave Chinner,
	linux-kernel, netdev
In-Reply-To: <1291914474.4063.68.camel@edumazet-laptop>

Le jeudi 09 décembre 2010 à 18:07 +0100, Eric Dumazet a écrit :
> Le vendredi 10 décembre 2010 à 00:47 +0800, Américo Wang a écrit :
> 
> > Yeah, there is no point that a zombie can trigger a BUG_ON in kenrel.
> > But it is still interesting to know why it becomes a zombie...
> > 
> 
> A zombie is very easy to get.
> 
> Technically speaking, all processes die and become zombies, unless
> parent said : signal(SIGCLD, SIG_IGN) before fork()
> 
> The parent is buggy (sudo in this case ?) and doesnt call wait() to
> 'free' one of its children.
> 
> 

Before you ask :)

If the parent dies before the child, task is re-parented to init.

Then, with namespaces, I dont know what happens (is there one init per
namespace ?)

^ permalink raw reply

* [PATCH] Sysctl interface to UNIX_INFLIGHT_TRIGGER_GC v.2
From: Pavel Vasilyev @ 2010-12-09 17:26 UTC (permalink / raw)
  To: netdev

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

Sysctl interface to UNIX_INFLIGHT_TRIGGER_GC.
IMHO convenient for testing.



[-- Attachment #2: sysctl.inflight_trigger_gc.patch --]
[-- Type: text/x-patch, Size: 2797 bytes --]

 Documentation/networking/ip-sysctl.txt |    4 ++++
 include/net/af_unix.h                  |    1 +
 net/unix/garbage.c                     |    5 ++---
 net/unix/sysctl_net_unix.c             |    9 +++++++++
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 3c5e465..11e6adf 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1463,6 +1463,10 @@ max_dgram_qlen - INTEGER
 
 	Default: 10
 
+inflight_trigger_gc - INTEGER
+	The maximal number of inflight sockets for force garbage collect.
+
+	Default: 16000
 
 UNDOCUMENTED:
 
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 18e5c3f..ea580e4 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -15,6 +15,7 @@ extern struct sock *unix_get_socket(struct file *filp);
 #define UNIX_HASH_SIZE	256
 
 extern unsigned int unix_tot_inflight;
+extern unsigned int sysctl_inflight_trigger_gc;
 
 struct unix_address {
 	atomic_t	refcnt;
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index f89f83b..c646c6b 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -94,7 +94,7 @@ static DEFINE_SPINLOCK(unix_gc_lock);
 static DECLARE_WAIT_QUEUE_HEAD(unix_gc_wait);
 
 unsigned int unix_tot_inflight;
-
+unsigned int sysctl_inflight_trigger_gc = 16000;
 
 struct sock *unix_get_socket(struct file *filp)
 {
@@ -259,7 +259,6 @@ static void inc_inflight_move_tail(struct unix_sock *u)
 }
 
 static bool gc_in_progress = false;
-#define UNIX_INFLIGHT_TRIGGER_GC 16000
 
 void wait_for_unix_gc(void)
 {
@@ -267,7 +266,7 @@ void wait_for_unix_gc(void)
 	 * If number of inflight sockets is insane,
 	 * force a garbage collect right now.
 	 */
-	if (unix_tot_inflight > UNIX_INFLIGHT_TRIGGER_GC && !gc_in_progress)
+	if (unix_tot_inflight > sysctl_inflight_trigger_gc && !gc_in_progress)
 		unix_gc();
 	wait_event(unix_gc_wait, gc_in_progress == false);
 }
diff --git a/net/unix/sysctl_net_unix.c b/net/unix/sysctl_net_unix.c
index 397cffe..c807235 100644
--- a/net/unix/sysctl_net_unix.c
+++ b/net/unix/sysctl_net_unix.c
@@ -23,6 +23,13 @@ static ctl_table unix_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "inflight_trigger_gc",
+		.data		= &sysctl_inflight_trigger_gc,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };
 
@@ -41,6 +48,8 @@ int __net_init unix_sysctl_register(struct net *net)
 		goto err_alloc;
 
 	table[0].data = &net->unx.sysctl_max_dgram_qlen;
+	table[1].data = &sysctl_inflight_trigger_gc;
+
 	net->unx.ctl = register_net_sysctl_table(net, unix_path, table);
 	if (net->unx.ctl == NULL)
 		goto err_reg;

--- 

Signed-off-by: Pavel Vasilyev <pavel@pavlinux.ru>

^ permalink raw reply related

* [PATCH] connector: add module alias
From: Stephen Hemminger @ 2010-12-09 17:36 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: netdev

Since connector can be built as a module and uses netlink socket
to communicate. The module should have an alias to autoload when socket
of NETLINK_CONNECTOR type is requested.

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

--- a/drivers/connector/connector.c	2010-12-09 09:19:05.549120343 -0800
+++ b/drivers/connector/connector.c	2010-12-09 09:19:52.795051458 -0800
@@ -36,6 +36,7 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Evgeniy Polyakov <zbr@ioremap.net>");
 MODULE_DESCRIPTION("Generic userspace <-> kernelspace connector.");
+MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_CONNECTOR);
 
 static struct cn_dev cdev;
 

^ permalink raw reply

* Re: [Bugme-new] [Bug 24472] New: Kernel panic - not syncing: Fatal Exception
From: Jarek Poplawski @ 2010-12-09 17:55 UTC (permalink / raw)
  To: Paweł Staszewski
  Cc: Andrew Morton, netdev, Paul Mackerras, bugzilla-daemon,
	bugme-daemon, pstaszewski, Andrej Ota, Eric Dumazet
In-Reply-To: <4D00CE33.8040006@itcare.pl>

Paweł Staszewski wrote:
> W dniu 2010-12-08 23:01, Jarek Poplawski pisze:
>> Paweł Staszewski wrote:
>>> W dniu 2010-12-08 21:22, Andrew Morton pisze:
>>>> (switched to email.  Please respond via emailed reply-to-all, not
>>>> via the
>>>> bugzilla web interface).
>>>>
>>>> On Wed, 8 Dec 2010 20:14:45 GMT
>>>> bugzilla-daemon@bugzilla.kernel.org wrote:
>>>>
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=24472
>>>>>
>>>>>              Summary: Kernel panic - not syncing: Fatal Exception
>>>>>              Product: Drivers
>>>>>              Version: 2.5
>>>>>       Kernel Version: 2.6.36.1
>> Hi,
>> Could you try to revert this patch?:
>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commitdiff;h=55c95e738da85373965cb03b4f975d0fd559865b
>>
>>
> After reverting this patch all is working
> 200 connects-disconnects and no kernel panic
> 
> I will make more session and test more.

OK. I CC Andrej and Eric, who diagnosed it in this thread:
http://lkml.org/lkml/2010/12/3/116
[unable to handle kernel NULL pointer dereference in skb_dequeue]

This should be also interesting:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=db7bf6d97c6956b7eb0f22131cb5c37bd41f33c0
 
Thanks for testing,
Jarek P.

^ permalink raw reply

* Re: [regression, 2.6.37-rc1] 'ip link tap0 up' stuck in do_exit()
From: Jim Bos @ 2010-12-09 17:59 UTC (permalink / raw)
  To: Américo Wang
  Cc: Oleg Nesterov, Florian Mickler, Ingo Molnar, Dave Chinner,
	Eric Dumazet, linux-kernel, netdev
In-Reply-To: <20101209164748.GA3218@hack>

On 12/09/2010 05:47 PM, Américo Wang wrote:
> On Wed, Dec 08, 2010 at 03:08:22PM +0100, Oleg Nesterov wrote:
>> On 12/08, Oleg Nesterov wrote:
>>>
>>> On 12/08, Oleg Nesterov wrote:
>>>>
>>>> On 12/08, Florian Mickler wrote:
>>>>>
>>>>> [ ccing Ingo and Oleg ] as suggested
>>>>
>>>> Well. Of course I can't explain this bug. But, looking at this email
>>>> I do not see amything strange in exit/schedule/etc.
>>>>
>>>>>>>>>>> This is resulting in the command 'ip link set tap0 up' hanging as a zombie:
>>>>>>>>>>>
>>>>>>>>>>> root      3005     1  0 16:53 pts/3    00:00:00 /bin/sh /vm-images/qemu-ifup tap0
>>>>>>>>>>> root      3011  3005  0 16:53 pts/3    00:00:00 /usr/bin/sudo /sbin/ip link set tap0 up
>>>>>>>>>>> root      3012  3011  0 16:53 pts/3    00:00:00 [ip] <defunct>
>>>>
>>>> That is. ip is a zombie.
>>>
>>> And. I do not know if this matters or not, but "the command 'ip link
>>> set tap0 up' hanging as a zombie" does not look right.
>>>
>>> This was spawned by
>>>
>>>>>>>>> if [ -n "$1" ];then
>>>>>>>>>         /usr/bin/sudo /sbin/ip link set $1 up
>>>>>>>>>         sleep 0.5s
>>>>>>>>>         /usr/bin/sudo /usr/sbin/brctl addif $switch $1
>>>>>>>>>      exit 0
>>>>>>>>> fi
>>>
>>> The command does not hang. But it forks the child with pid == 3012,
>>> this child exits.
>>
>> Damn, sorry for noise, forgot to mention...
>>
>> The parent's trace (pid == 3011) can be more useful. Say, if it
>> hangs in do_wait(), then the kernel is obviously wrong.
>>
> 
> Yeah, there is no point that a zombie can trigger a BUG_ON in kenrel.
> But it is still interesting to know why it becomes a zombie...
> 
> --
> 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/

ip link tap0 up not working might be this issue:

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

( Latest Virtualbox 3.2.12 works around this issue )

Jim

^ permalink raw reply

* Re: Behaviour of ETHTOOL_GLINK for an interface that's down
From: Ben Hutchings @ 2010-12-09 18:05 UTC (permalink / raw)
  To: Dan Williams; +Cc: netdev, sf-linux-drivers
In-Reply-To: <1291874559.24551.10.camel@dcbw.foobar.com>

On Thu, 2010-12-09 at 00:02 -0600, Dan Williams wrote:
> On Wed, 2010-12-08 at 23:47 -0600, Dan Williams wrote:
> > On Mon, 2010-12-06 at 21:59 +0000, Ben Hutchings wrote:
> > > ETHTOOL_GLINK is yet another ethtool operation that has unclear
> > > semantics that results in differing behaviour when the interface is
> > > down.
[...]
> > > DaveM said that Network Manager may require (2), although I don't think
> > > this is correct.  At least the current version brings all managed
> > > interfaces up whether or not they have link-up already.
> > 
> > NM has used netlink + IFF_RUNNING (not ethtool) for a few years for
> > actual carrier detection.  Ethtool (and MII ioctls) are called as a
> > "best effort" method of determining that the device actually *has*
> > carrier detection at all, since if the device has gone to the trouble to
> > implement either MII or ethtool, it probably also has carrier detection.
> > 
> > But ethtool isn't actually used to determine carrier status in NM.  It's
> > netlink all the way down.
> 
> And as a follow-on, yes, NM does bring all devices it is allowed to
> manager IFF_UP because that's the only way (at this point) that we can
> guarantee functional carrier detect from the card.  I'd love it if that
> weren't the case, and if we could have some indicator that the driver
> could do carrier detect while in a lower-power state and !IFF_UP, but we
> don't have that yet.

Thanks for the information, Dan.  Presumably it would actually be
sufficient for NM's requirements to implement 'energy detect' which some
PHYs can do even in a low power state?  (Though I wonder whether that
works between two PHYs both in a low power state.)  You could then use
this as a trigger to bring the interface up, while still relying on the
existing link change notification to trigger interface configuration.
But this clearly has to be separate from ETHTOOL_GLINK, not least
because you want notification rather than having to poll.

Ben.

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


^ permalink raw reply

* Re: [stable] [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too
From: Greg KH @ 2010-12-09 18:19 UTC (permalink / raw)
  To: avagin@gmail.com
  Cc: Greg Kroah-Hartman, krkumar2, avagin, eric.dumazet, netdev, mjt,
	David Miller, stable
In-Reply-To: <4D007A77.8050102@gmail.com>

On Thu, Dec 09, 2010 at 09:43:03AM +0300, avagin@gmail.com wrote:
> I add the patch in attachments

Ok, thanks, I'll queue it up for the next .32 release after this one.

greg k-h

^ permalink raw reply

* Re: Adding Support for SG,GSO,GRO
From: David Miller @ 2010-12-09 18:29 UTC (permalink / raw)
  To: bhutchings; +Cc: srk, netdev
In-Reply-To: <1291906948.19763.16.camel@localhost>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 09 Dec 2010 15:02:28 +0000

> So you really have no choice - you must implement hardware checksum
> offload if you want any of the others.

Right, advertising SG support is entirely pointless if you aren't
also advertising HW checksumming support.

^ permalink raw reply

* Re: Behaviour of ETHTOOL_GLINK for an interface that's down
From: Dan Williams @ 2010-12-09 18:29 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, sf-linux-drivers
In-Reply-To: <1291917939.2647.10.camel@bwh-desktop>

On Thu, 2010-12-09 at 18:05 +0000, Ben Hutchings wrote:
> On Thu, 2010-12-09 at 00:02 -0600, Dan Williams wrote:
> > On Wed, 2010-12-08 at 23:47 -0600, Dan Williams wrote:
> > > On Mon, 2010-12-06 at 21:59 +0000, Ben Hutchings wrote:
> > > > ETHTOOL_GLINK is yet another ethtool operation that has unclear
> > > > semantics that results in differing behaviour when the interface is
> > > > down.
> [...]
> > > > DaveM said that Network Manager may require (2), although I don't think
> > > > this is correct.  At least the current version brings all managed
> > > > interfaces up whether or not they have link-up already.
> > > 
> > > NM has used netlink + IFF_RUNNING (not ethtool) for a few years for
> > > actual carrier detection.  Ethtool (and MII ioctls) are called as a
> > > "best effort" method of determining that the device actually *has*
> > > carrier detection at all, since if the device has gone to the trouble to
> > > implement either MII or ethtool, it probably also has carrier detection.
> > > 
> > > But ethtool isn't actually used to determine carrier status in NM.  It's
> > > netlink all the way down.
> > 
> > And as a follow-on, yes, NM does bring all devices it is allowed to
> > manager IFF_UP because that's the only way (at this point) that we can
> > guarantee functional carrier detect from the card.  I'd love it if that
> > weren't the case, and if we could have some indicator that the driver
> > could do carrier detect while in a lower-power state and !IFF_UP, but we
> > don't have that yet.
> 
> Thanks for the information, Dan.  Presumably it would actually be
> sufficient for NM's requirements to implement 'energy detect' which some
> PHYs can do even in a low power state?  (Though I wonder whether that
> works between two PHYs both in a low power state.)  You could then use
> this as a trigger to bring the interface up, while still relying on the
> existing link change notification to trigger interface configuration.
> But this clearly has to be separate from ETHTOOL_GLINK, not least
> because you want notification rather than having to poll.

+1 to all of that.  If that showed up, I'd love to use it for NM.

Dan



^ permalink raw reply

* Re: [PATCH] connector: add module alias
From: Evgeniy Polyakov @ 2010-12-09 18:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, davem
In-Reply-To: <20101209093646.1b5a6abb@nehalam>

On Thu, Dec 09, 2010 at 09:36:46AM -0800, Stephen Hemminger (shemminger@vyatta.com) wrote:
> Since connector can be built as a module and uses netlink socket
> to communicate. The module should have an alias to autoload when socket
> of NETLINK_CONNECTOR type is requested.

Ack, thank you.
David, please apply.

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/drivers/connector/connector.c	2010-12-09 09:19:05.549120343 -0800
> +++ b/drivers/connector/connector.c	2010-12-09 09:19:52.795051458 -0800
> @@ -36,6 +36,7 @@
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Evgeniy Polyakov <zbr@ioremap.net>");
>  MODULE_DESCRIPTION("Generic userspace <-> kernelspace connector.");
> +MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_CONNECTOR);
>  
>  static struct cn_dev cdev;
>  

-- 
	Evgeniy Polyakov

^ permalink raw reply

* Re: Adding Support for SG,GSO,GRO
From: Michał Mirosław @ 2010-12-09 18:47 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Govindarajan, Sriramakrishnan, netdev@vger.kernel.org
In-Reply-To: <1291906948.19763.16.camel@localhost>

2010/12/9 Ben Hutchings <bhutchings@solarflare.com>:
> On Thu, 2010-12-09 at 16:03 +0530, Govindarajan, Sriramakrishnan wrote:
>> Hi
>> We have a NAPI compliant driver(net/drivers/davinci_emac.c), that does
>> well at 10/100Mbps loads. Now the same controller/driver is used for
>> 1000Mbps
>> mode as well, where the CPU gets saturated easily
>>
>> Internally the module supports scatter gather DMA(which is currently not
>> exercised) but there is no HW checksum support.
>>
>> To specifically implement GRO, GSO support would it be sufficient to add
>> SG support to the driver? Are there other means of increasing the throughput
>> and decreasing the CPU loading?
[...]
> On the TX side, NETIF_F_SG means that the stack may include data in the
> skb by reference to arbitrary pages *even if their contents are still
> being changed* (think sendfile()), which means it depends on hardware
> checksum generation.

Isn't that condition too broad? If the data could change after packet
is submitted to the driver then results would be unpredictable and
allow sending wrong data with correct (because hw-calculated)
checksum.

Right now NETIF_F_SG is removed from dev->features by
netdev_fix_features() if no checksum offloads are enabled.

Just an idea: would driver with NETIF_F_SG|NETIF_F_HW_CSUM using
skb_checksum_help() in xmit path work? This would allow to use DMA
scatter-gather without hardware checksumming (and avoid copying the
packet's data before sending).

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: Adding Support for SG,GSO,GRO
From: Ben Hutchings @ 2010-12-09 18:58 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Govindarajan, Sriramakrishnan, netdev@vger.kernel.org
In-Reply-To: <AANLkTi=8zWMYWG0DU147B08nN7xcvwaB9Bpq9fE8_fBe@mail.gmail.com>

On Thu, 2010-12-09 at 19:47 +0100, Michał Mirosław wrote:
> 2010/12/9 Ben Hutchings <bhutchings@solarflare.com>:
> > On Thu, 2010-12-09 at 16:03 +0530, Govindarajan, Sriramakrishnan wrote:
> >> Hi
> >> We have a NAPI compliant driver(net/drivers/davinci_emac.c), that does
> >> well at 10/100Mbps loads. Now the same controller/driver is used for
> >> 1000Mbps
> >> mode as well, where the CPU gets saturated easily
> >>
> >> Internally the module supports scatter gather DMA(which is currently not
> >> exercised) but there is no HW checksum support.
> >>
> >> To specifically implement GRO, GSO support would it be sufficient to add
> >> SG support to the driver? Are there other means of increasing the throughput
> >> and decreasing the CPU loading?
> [...]
> > On the TX side, NETIF_F_SG means that the stack may include data in the
> > skb by reference to arbitrary pages *even if their contents are still
> > being changed* (think sendfile()), which means it depends on hardware
> > checksum generation.
> 
> Isn't that condition too broad? If the data could change after packet
> is submitted to the driver then results would be unpredictable and
> allow sending wrong data with correct (because hw-calculated)
> checksum.

This is not done for a regular send(), only for functions such as
sendfile() which are specified to read the data asynchronously.

> Right now NETIF_F_SG is removed from dev->features by
> netdev_fix_features() if no checksum offloads are enabled.
> 
> Just an idea: would driver with NETIF_F_SG|NETIF_F_HW_CSUM using
> skb_checksum_help() in xmit path work? This would allow to use DMA
> scatter-gather without hardware checksumming (and avoid copying the
> packet's data before sending).

No, you cannot calculate a checksum for the fragments without also
copying them to ensure the data doesn't change afterward and invalidate
the checksum.  You could in theory make a copy into multiple fragments,
but there's no point in doing that unless the frame size is larger than
a page.

Ben.

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


^ 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