Netdev List
 help / color / mirror / Atom feed
* Re: [patch 1/3] net: serialize hrtimer callback in sched_cbq
From: Linus Torvalds @ 2009-07-14 16:42 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: David Miller, tglx, netdev, linux-kernel, kaber
In-Reply-To: <1247588890.7500.186.camel@twins>



On Tue, 14 Jul 2009, Peter Zijlstra wrote:
> 
> Linus really hated the softirq mode, which is what prompted me to change
> that.
> 
> Now, it might be he only hated the particular interface and the
> resulting code, but I think to remember he simply thought the whole
> thing daft.

Yes. And I hated the bugs it had. 

Don't make something as core as timers any more complicated. Don't take 
locks in timers and then complain about deadlocks. If your locking is 
broken, don't make the core timers be idiotically broken.

Because it was. The code was a total mess to follow, and had bugs.

		Linus

^ permalink raw reply

* Re: [PATCH] gre: fix ToS/DiffServ inherit bug
From: David Miller @ 2009-07-14 16:43 UTC (permalink / raw)
  To: aj; +Cc: voyo, netdev, kuznet, kaber
In-Reply-To: <20090714.093433.216171575.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 14 Jul 2009 09:34:33 -0700 (PDT)

> From: Andreas Jaggi <aj@open.ch>
> Date: Tue, 14 Jul 2009 17:36:48 +0200
> 
>> Fixes two bugs:
>> - ToS/DiffServ inheritance was unintentionally activated when using impair fixed ToS values
>> - ECN bit was lost during ToS/DiffServ inheritance
>> 
>> Signed-off-by: Andreas Jaggi <aj@open.ch>

In case it wasn't clear, I've applied this patch.

^ permalink raw reply

* Re: [PATCH] iproute2 flush: handle larger tables and deleted entries
From: Gautam Kachroo @ 2009-07-14 16:45 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4A5C5233.4010007@trash.net>

On Tue, Jul 14, 2009 at 2:38 AM, Patrick McHardy<kaber@trash.net> wrote:
> Gautam Kachroo wrote:
>> use a new netlink socket when sending flush messages to avoid reading
>> any pending data on the existing netlink socket.
>>
>> read all of the response from the netlink request -- this response can
>> be split over multiple recv calls, pretty much one per netlink request
>> message. ENOENT errors, which correspond to attempts to delete an
>> already deleted entry, are ignored. Other errors are not ignored.
>
> In which case would there be any pending data? From what I can see,
> this can only happen when using batching, but in that case the
> previous command should continue reading until it has received all
> responses (which the netlink functions appear to be doing properly).

What is the "previous command"?
Are you referring to rtnl_dump_filter? If rtnl_send_check comes across
a failure, rtnl_dump_filter will not continue reading.

Here's the situation that I'm referring to:

If rtnl_send_check detects an error, it returns -1. rtnl_send_check is
called from flush_update. The multiple implementations of flush_update
(e.g. in ipneigh.c, ipaddress.c) propagate this return value to their
caller, e.g. print_neigh or print_addrinfo.

print_neigh, print_addrinfo, etc. are called from rtnl_dump_filter.
rtnl_dump_filter sits in a loop calling recvmsg on the netlink socket.
However, it returns the error value if the filter function (e.g.
print_neigh) returns an error. In this case, rtnl_dump_filter can
return before it's read all the responses.
The error return from rtnl_dump_filter causes the program to exit.

Note, rtnl_send_check treats any pending data as an error. It could be
changed to only complain if the response contains NLMSG_ERROR
messages, but that delays error detection because it will read all the
responses to the dump request first.

thanks,
-gk

^ permalink raw reply

* Re: [PATCH 1/1] Tell linkwatch about new interfaces
From: Sergio Luis @ 2009-07-14 17:17 UTC (permalink / raw)
  To: David Miller; +Cc: amluto, netdev, linux-kernel
In-Reply-To: <20090404.170539.148727646.davem@davemloft.net>

Hello Dave,

On Sat, Apr 4, 2009 at 9:05 PM, David Miller<davem@davemloft.net> wrote:
> From: Andrew Lutomirski <amluto@gmail.com>
> Date: Wed, 1 Apr 2009 11:40:06 -0400
>
>> When a network driver registers a new interface, linkwatch will not notice,
>> and hence not set the rfc2863 operstate, until netif_carrier_on gets called.
>> If the new interface has no carrier when it is connected, then a status of
>> "unknown" is reported to userspace, which confuses various tools
>> (NetworkManager, for example).
>>
>> This fires a linkwatch event for all new interfaces, so that operstate
>> gets set reasonably quickly.
>>
>> Signed-off-by: Andrew Lutomirski <amluto@gmail.com>
>
> The default assumed state for a freshly registered network
> device is that the link is up.
>
> If that disagrees from reality, the driver should make the
> appropriate netif_carrier_off() call.
>
> I'm sure you'll find that the e1000 driver is not doing this
> and that is what causes the bug you are seeing.
> --

is this patch incorrect, though? with the linkwatch_fire_event() call,
the rfc2863 operstate will be set for everyone at device register
time.
in here I am having the interface operstate as 'unknown', but I do
ifconfig down and up or unplug/plug the cable again it will finally
set the correct rfc2863 operstate.

or should this be fixed on a per-driver basis, like it apparently was
in this case, for his e1000? (drivers/net/skge.c in here).

thanks,
sergio

^ permalink raw reply

* Re: [PATCH] gre: fix ToS/DiffServ inherit bug
From: Alexey Kuznetsov @ 2009-07-14 17:28 UTC (permalink / raw)
  To: David Miller; +Cc: aj, voyo, netdev, kaber
In-Reply-To: <20090714.093433.216171575.davem@davemloft.net>

Hello!

On Tue, Jul 14, 2009 at 09:34:33AM -0700, David Miller wrote:
> Note that this only works out because one wouldn't specify ECN
> bits in the TOS to use for the tunnel.
> 
> But it demonstrates the outright danger of trying to use
> "undefined" flag bits in protocol header fields to pass
> private control flags into the driver from user interfaces.

Yes. My fault.

Well, in the form after the patch it will survive at least until
the next change of semantics of tos. :-)

Alexey

^ permalink raw reply

* Re: use after free bug in socket code
From: Wolfgang Grandegger @ 2009-07-14 17:53 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Oliver Hartkopp, Herbert Xu, davem, netdev, urs.thuermann,
	Urs Thuermann
In-Reply-To: <19036.41388.634723.358236@ipc1.ka-ro>

Lothar Waßmann wrote:
> Hi,
> 
> Oliver Hartkopp writes:
>>> The situation where this triggers a bug is when the chip driver's
>>> hard_start_xmit function returns a NETDEV_TX_BUSY and subsequently the
>>> can interface is deconfigured.
>>>
>>> Maybe you could try this on different hardware?
>> Will do when i'm back at work ;-)
>>
> Should I wait for the results of your tests before submitting the
> patches?

A user has reported sporadic kernel panics with a down/up/restart stress
test a while ago on the Socket-CAN mailing list and the patch seems to
solve his problem:

https://lists.berlios.de/pipermail/socketcan-users/2009-July/000985.html

From my point of view you can go ahead.

Thanks.

Wolfgang.

^ permalink raw reply

* Re: [PATCH] [net/9p]: default 9p transport module fix
From: Eric Van Hensbergen @ 2009-07-14 18:29 UTC (permalink / raw)
  To: Abhishek Kulkarni; +Cc: linux-kernel, v9fs-developer, netdev
In-Reply-To: <1247492991-26009-2-git-send-email-adkulkar@umail.iu.edu>

applied v9fs-devel

On Mon, Jul 13, 2009 at 8:49 AM, Abhishek Kulkarni<adkulkar@umail.iu.edu> wrote:
> The default 9p transport module is not chosen unless an option parameter (any)
> is passed to mount, which thus returns a ENOPROTOSUPPORT. This fix moves the
> check out of parse_opts into p9_client_create.
>
> Signed-off-by: Abhishek Kulkarni <adkulkar@umail.iu.edu>
> ---
> :100644 100644 cc25e63... 787ccdd... M  net/9p/client.c
>  net/9p/client.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index cc25e63..787ccdd 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -117,9 +117,6 @@ static int parse_opts(char *opts, struct p9_client *clnt)
>                }
>        }
>
> -       if (!clnt->trans_mod)
> -               clnt->trans_mod = v9fs_get_default_trans();
> -
>        kfree(options);
>        return ret;
>  }
> @@ -689,6 +686,9 @@ struct p9_client *p9_client_create(const char *dev_name, char *options)
>        if (err < 0)
>                goto error;
>
> +       if (!clnt->trans_mod)
> +               clnt->trans_mod = v9fs_get_default_trans();
> +
>        if (clnt->trans_mod == NULL) {
>                err = -EPROTONOSUPPORT;
>                P9_DPRINTK(P9_DEBUG_ERROR,
> --
> 1.6.0.4
>
>

^ permalink raw reply

* Re: [PATCH] [net/9p] Possible regression in p9_client_stat
From: Eric Van Hensbergen @ 2009-07-14 18:29 UTC (permalink / raw)
  To: Abhishek Kulkarni; +Cc: linux-kernel, v9fs-developer, netdev
In-Reply-To: <1247492991-26009-1-git-send-email-adkulkar@umail.iu.edu>

applied v9fs-devel

On Mon, Jul 13, 2009 at 8:49 AM, Abhishek Kulkarni<adkulkar@umail.iu.edu> wrote:
> Fix a possible regression with p9_client_stat where it can try to kfree an ERR_PTR
> after an erroneous p9pdu_readf. Also remove an unnecessary data buffer increment
> in p9_client_read.
>
> Signed-off-by: Abhishek Kulkarni <adkulkar@umail.iu.edu>
> ---
> :100644 100644 dd43a82... cc25e63... M  net/9p/client.c
>  net/9p/client.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index dd43a82..cc25e63 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1098,7 +1098,6 @@ p9_client_read(struct p9_fid *fid, char *data, char __user *udata, u64 offset,
>
>        if (data) {
>                memmove(data, dataptr, count);
> -               data += count;
>        }
>
>        if (udata) {
> @@ -1192,9 +1191,9 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
>
>        err = p9pdu_readf(req->rc, clnt->dotu, "wS", &ignored, ret);
>        if (err) {
> -               ret = ERR_PTR(err);
>                p9pdu_dump(1, req->rc);
> -               goto free_and_error;
> +               p9_free_req(clnt, req);
> +               goto error;
>        }
>
>        P9_DPRINTK(P9_DEBUG_9P,
> @@ -1211,8 +1210,6 @@ struct p9_wstat *p9_client_stat(struct p9_fid *fid)
>        p9_free_req(clnt, req);
>        return ret;
>
> -free_and_error:
> -       p9_free_req(clnt, req);
>  error:
>        kfree(ret);
>        return ERR_PTR(err);
> --
> 1.6.0.4
>
>

^ permalink raw reply

* [PATCH net-next] Re: rib_trie / Fix inflate_threshold_root. Now=15 size=11 bits
From: Jarek Poplawski @ 2009-07-14 18:33 UTC (permalink / raw)
  To: David Miller
  Cc: Paul E. McKenney, Paweł Staszewski,
	Linux Network Development list, Robert Olsson
In-Reply-To: <20090705213232.GG8943@linux.vnet.ibm.com>

On Sun, Jul 05, 2009 at 02:32:32PM -0700, Paul E. McKenney wrote:
> On Sun, Jul 05, 2009 at 07:32:08PM +0200, Jarek Poplawski wrote:
> > On Sun, Jul 05, 2009 at 06:20:03PM +0200, Jarek Poplawski wrote:
> > > On Sun, Jul 05, 2009 at 02:30:03AM +0200, Paweł Staszewski wrote:
> > > > Oh
> > > >
> > > > I forgot - please Jarek give me patch with sync rcu and i will make test  
> > > > on preempt kernel
> > > 
> > > Probably non-preempt kernel might need something like this more, but
> > > comparing is always interesting. This patch is based on Paul's
> > > suggestion (I hope).
> > 
> > Hold on ;-) Here is something even better... Syncing after 128 pages
> > might be still too slow, so here is a higher initial value, 1000, plus
> > you can change this while testing in:
> > 
> > /sys/module/fib_trie/parameters/sync_pages
> > 
> > It would be interesting to find the lowest acceptable value.
> 
> Looks like a promising approach to me!
> 
> 							Thanx, Paul

Below is a simpler version of this patch, without the sysfs parameter.
(I left the previous version quoted for comparison.) Thanks.

> > Jarek P.
> > ---> (synchronize take 8; apply on top of the 2.6.29.x with the last
> >  	all-in-one patch, or net-2.6)
> > 
> >  net/ipv4/fib_trie.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> > index 00a54b2..decc8d0 100644
> > --- a/net/ipv4/fib_trie.c
> > +++ b/net/ipv4/fib_trie.c
> > @@ -71,6 +71,7 @@
> >  #include <linux/netlink.h>
> >  #include <linux/init.h>
> >  #include <linux/list.h>
> > +#include <linux/moduleparam.h>
> >  #include <net/net_namespace.h>
> >  #include <net/ip.h>
> >  #include <net/protocol.h>
> > @@ -164,6 +165,10 @@ static struct tnode *inflate(struct trie *t, struct tnode *tn);
> >  static struct tnode *halve(struct trie *t, struct tnode *tn);
> >  /* tnodes to free after resize(); protected by RTNL */
> >  static struct tnode *tnode_free_head;
> > +static size_t tnode_free_size;
> > +
> > +static int sync_pages __read_mostly = 1000;
> > +module_param(sync_pages, int, 0640);
> > 
> >  static struct kmem_cache *fn_alias_kmem __read_mostly;
> >  static struct kmem_cache *trie_leaf_kmem __read_mostly;
> > @@ -393,6 +398,8 @@ static void tnode_free_safe(struct tnode *tn)
> >  	BUG_ON(IS_LEAF(tn));
> >  	tn->tnode_free = tnode_free_head;
> >  	tnode_free_head = tn;
> > +	tnode_free_size += sizeof(struct tnode) +
> > +			   (sizeof(struct node *) << tn->bits);
> >  }
> > 
> >  static void tnode_free_flush(void)
> > @@ -404,6 +411,11 @@ static void tnode_free_flush(void)
> >  		tn->tnode_free = NULL;
> >  		tnode_free(tn);
> >  	}
> > +
> > +	if (tnode_free_size >= PAGE_SIZE * sync_pages) {
> > +		tnode_free_size = 0;
> > +		synchronize_rcu();
> > +	}
> >  }
> > 
> >  static struct leaf *leaf_new(void)
> > --

------------------------>
ipv4: Use synchronize_rcu() during trie_rebalance()

During trie_rebalance() we free memory after resizing with call_rcu(),
but large updates, especially with PREEMPT_NONE configs, can cause
memory stresses, so this patch calls synchronize_rcu() in
tnode_free_flush() after each sync_pages to guarantee such freeing
(especially before resizing the root node).

The value of sync_pages = 128 is based on Pawel Staszewski's tests as
the lowest which doesn't hinder updating times. (For testing purposes
there was a sysfs module parameter to change it on demand, but it's
removed until we're sure it could be really useful.)

The patch is based on suggestions by: Paul E. McKenney
<paulmck@linux.vnet.ibm.com>

Reported-by: Pawel Staszewski <pstaszewski@itcare.pl>
Tested-by: Pawel Staszewski <pstaszewski@itcare.pl>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---

 net/ipv4/fib_trie.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 63c2fa7..58ba9f4 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -164,6 +164,14 @@ static struct tnode *inflate(struct trie *t, struct tnode *tn);
 static struct tnode *halve(struct trie *t, struct tnode *tn);
 /* tnodes to free after resize(); protected by RTNL */
 static struct tnode *tnode_free_head;
+static size_t tnode_free_size;
+
+/*
+ * synchronize_rcu after call_rcu for that many pages; it should be especially
+ * useful before resizing the root node with PREEMPT_NONE configs; the value was
+ * obtained experimentally, aiming to avoid visible slowdown.
+ */
+static const int sync_pages = 128;
 
 static struct kmem_cache *fn_alias_kmem __read_mostly;
 static struct kmem_cache *trie_leaf_kmem __read_mostly;
@@ -393,6 +401,8 @@ static void tnode_free_safe(struct tnode *tn)
 	BUG_ON(IS_LEAF(tn));
 	tn->tnode_free = tnode_free_head;
 	tnode_free_head = tn;
+	tnode_free_size += sizeof(struct tnode) +
+			   (sizeof(struct node *) << tn->bits);
 }
 
 static void tnode_free_flush(void)
@@ -404,6 +414,11 @@ static void tnode_free_flush(void)
 		tn->tnode_free = NULL;
 		tnode_free(tn);
 	}
+
+	if (tnode_free_size >= PAGE_SIZE * sync_pages) {
+		tnode_free_size = 0;
+		synchronize_rcu();
+	}
 }
 
 static struct leaf *leaf_new(void)

^ permalink raw reply related

* Re: [PATCH 1/1] Tell linkwatch about new interfaces
From: David Miller @ 2009-07-14 18:33 UTC (permalink / raw)
  To: eeeesti; +Cc: amluto, netdev, linux-kernel
In-Reply-To: <ce0170d90907141017h732c2cd7ja5990303093b6edd@mail.gmail.com>

From: Sergio Luis <eeeesti@gmail.com>
Date: Tue, 14 Jul 2009 14:17:21 -0300

> is this patch incorrect, though? with the linkwatch_fire_event() call,
> the rfc2863 operstate will be set for everyone at device register
> time.

The issue is dumb drivers that do not manage their link state
at all.  We want them to always have their links up, from the
moment they are registered.

This is especially important for virtual devices.

^ permalink raw reply

* Re: [PATCH 1/1] Tell linkwatch about new interfaces
From: Sergio Luis @ 2009-07-14 18:37 UTC (permalink / raw)
  To: David Miller; +Cc: amluto, netdev, linux-kernel
In-Reply-To: <20090714.113348.155881037.davem@davemloft.net>

On Tue, Jul 14, 2009 at 3:33 PM, David Miller<davem@davemloft.net> wrote:
> From: Sergio Luis <eeeesti@gmail.com>
> Date: Tue, 14 Jul 2009 14:17:21 -0300
>
>> is this patch incorrect, though? with the linkwatch_fire_event() call,
>> the rfc2863 operstate will be set for everyone at device register
>> time.
>
> The issue is dumb drivers that do not manage their link state
> at all.  We want them to always have their links up, from the
> moment they are registered.
>
> This is especially important for virtual devices.
>

I see. I will try to take a look at the driver in question, then. Thanks.

^ permalink raw reply

* [TCP_CA_CWR] Causes for entering TCP_CA_CWR state with 0 retransmissions
From: Yinglin Sun @ 2009-07-14 18:38 UTC (permalink / raw)
  To: netdev

Hi,

I have a question about tcp_ca_state "TCP_CA_CWR".  Under what
circumstances does it enter this state?
I'm doing some experiments and have trace from tcp_info. From tcp_info
trace, I found that many places show ca_state is 2 (TCP_CA_CWR). Under
this state, the congestion window size decreases but the total number
of retransmissions is still 0.

I got some explanation from a very old message posted in 2003:
TCP_CA_CWR: The congestion window should be reduced due to some
congestion notification event, which can be ECN, ICMP source quench,
or local device congestion. Both sender and receiver don't have ECN
enabled, so ECN is not the case. ICMP source quench is so out-of-date
with RFC 1812 saying the routers should not issue these packets. So it
might be caused by local congestion. But this post is too old, almost
done 6 years ago. So I want to check if there are other possible
causes in today's kernel leading to TCP_CA_CWR with 0 retransmissions?

I need to find the exact cause for this phenomena, local congestion,
or other possible causes. Any hint will be very helpful for me.

Thanks a lot!

Yinglin

^ permalink raw reply

* Re: [TCP_CA_CWR] Causes for entering TCP_CA_CWR state with 0 retransmissions
From: David Miller @ 2009-07-14 18:42 UTC (permalink / raw)
  To: yinglin.s; +Cc: netdev
In-Reply-To: <46bf9ede0907141138q3b665aefv46a18591c3cfc2ce@mail.gmail.com>

From: Yinglin Sun <yinglin.s@gmail.com>
Date: Tue, 14 Jul 2009 14:38:07 -0400

> I have a question about tcp_ca_state "TCP_CA_CWR".  Under what
> circumstances does it enter this state?
> I'm doing some experiments and have trace from tcp_info. From tcp_info
> trace, I found that many places show ca_state is 2 (TCP_CA_CWR). Under
> this state, the congestion window size decreases but the total number
> of retransmissions is still 0.

What kind of "retransmissions"?  I bet the counter you are looking
at is counting timeout based retransmissions, which means it isn't
including the most predominant type of retransmissions, which are
fast retransmissions which are triggered in response to ACKs.

^ permalink raw reply

* Re: [PATCH 1/1] Tell linkwatch about new interfaces
From: Andrew Lutomirski @ 2009-07-14 18:58 UTC (permalink / raw)
  To: David Miller; +Cc: eeeesti, netdev, linux-kernel
In-Reply-To: <20090714.113348.155881037.davem@davemloft.net>

On Tue, Jul 14, 2009 at 2:33 PM, David Miller<davem@davemloft.net> wrote:
> From: Sergio Luis <eeeesti@gmail.com>
> Date: Tue, 14 Jul 2009 14:17:21 -0300
>
>> is this patch incorrect, though? with the linkwatch_fire_event() call,
>> the rfc2863 operstate will be set for everyone at device register
>> time.
>
> The issue is dumb drivers that do not manage their link state
> at all.  We want them to always have their links up, from the
> moment they are registered.

Such dumb drivers still end up with bogus operstate.

>
> This is especially important for virtual devices.

$ ip link show lo
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00

I've never noticed this causing a problem, but it seems a little
silly.  Presumably lo should be "UP."

--Andy

>

^ permalink raw reply

* Re: [TCP_CA_CWR] Causes for entering TCP_CA_CWR state with 0 retransmissions
From: Yinglin Sun @ 2009-07-14 19:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20090714.114211.254914419.davem@davemloft.net>

Hi David,

I use the total number of retransmissions reported by tcp_info. The
field name is tcpi_total_retrans. From the kernel source code, I found
that this number is for both fast retransmit and timeout
retransmissions. On the other hand, if fast retransmission happened,
ca_state should be TCP_CA_Recovery or TCP_CA_Disorder, but it's
TCP_CA_CWR.

Thanks.

Yinglin

On Tue, Jul 14, 2009 at 2:42 PM, David Miller<davem@davemloft.net> wrote:
> From: Yinglin Sun <yinglin.s@gmail.com>
> Date: Tue, 14 Jul 2009 14:38:07 -0400
>
>> I have a question about tcp_ca_state "TCP_CA_CWR".  Under what
>> circumstances does it enter this state?
>> I'm doing some experiments and have trace from tcp_info. From tcp_info
>> trace, I found that many places show ca_state is 2 (TCP_CA_CWR). Under
>> this state, the congestion window size decreases but the total number
>> of retransmissions is still 0.
>
> What kind of "retransmissions"?  I bet the counter you are looking
> at is counting timeout based retransmissions, which means it isn't
> including the most predominant type of retransmissions, which are
> fast retransmissions which are triggered in response to ACKs.
>

^ permalink raw reply

* Re: [TCP_CA_CWR] Causes for entering TCP_CA_CWR state with 0 retransmissions
From: David Miller @ 2009-07-14 19:15 UTC (permalink / raw)
  To: yinglinsun; +Cc: netdev
In-Reply-To: <46bf9ede0907141212x7a7a84f8l1882ffefbaf21e14@mail.gmail.com>

From: Yinglin Sun <yinglinsun@cs.pitt.edu>
Date: Tue, 14 Jul 2009 15:12:16 -0400

> I use the total number of retransmissions reported by tcp_info. The
> field name is tcpi_total_retrans. From the kernel source code, I found
> that this number is for both fast retransmit and timeout
> retransmissions. On the other hand, if fast retransmission happened,
> ca_state should be TCP_CA_Recovery or TCP_CA_Disorder, but it's
> TCP_CA_CWR.

Great, that if you're reading the code you also see that there
are many code paths that invoke tcp_enter_cwr() that can occur
without any retransmissions. :-)

One such case is when ECN congestion notification bits are
seen in an ACK packet.

^ permalink raw reply

* [GIT]: Networking
From: David Miller @ 2009-07-14 19:27 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) When Stephen Hemminger and others converted all of our net drivers
   to netdev_ops, some methods for setting MTU and the MAC address
   got lost in ethernet drivers.  This was an oversight and a regression,
   and Ben Hutchings did a complete audit and fixed up all of the
   cases where this happened.

   I did a bunch of builds to try and make sure this is clean and
   won't hiccup the build even by accident.

2) GRE tunnel misinterprets ECN bits as a tunnel configuration setting.
   Fix from Andreas Jaggi.

3) Two socket crash regression fixes from Eric Dumazet.

   a) The RCU hash table changes don't work properly if the socket
      allocation code just writes the socket refcnt explicitly to
      some value, we have to use inc/dec just like other transient
      references to that RCU freed memory might be doing.

    b) The socket refcount increment/decrement abatement changes made
       in the merge window forgot to remove two stray sock_put() operations
       in ip_push_pending_frames() and friends.

3) skb->dst cannot be released before transmit in SIT driver, regression
   fixes by Sascha Hlusiak.

4) phy_device code forgets to release mutex in error path, from
   Jiri Slaby.

5) atlx driver tests MCAST flag twice, it means to test the UCAST
   flag instead.  Fix from Roel Kluin.

6) NET_IP_ALIGN comment fixup from Tobias Klauser.

7) IGB driver doesn't build with gcc-3.4.x, due to inline functions
   being called before their definition.  Fix from Eric Dumazet.

8) Locking fix in TTY based networking drivers, from Ralf Baechle.

The regression count for networking seems pretty low at this point,
although there appears to be a few naggling wireless driver issues
to sort out still.  Hopefully John Linville and co. can sort those
out soon.

Please pull, thanks a lot!

The following changes since commit 6847e154e3cd74fca6084124c097980a7634285a:
  Linus Torvalds (1):
        Linux 2.6.31-rc3

are available in the git repository at:

  master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.git master

Andreas Jaggi (1):
      gre: fix ToS/DiffServ inherit bug

Ben Hutchings (2):
      netdev: restore MAC address set and validate operations
      netdev: restore MTU change operation

Dongdong Deng (1):
      drivers/net: using spin_lock_irqsave() in net_send_packet()

Eric Dumazet (3):
      net: sk_prot_alloc() should not blindly overwrite memory
      net: ip_push_pending_frames() fix
      igb: gcc-3.4.6 fix

Jiri Slaby (1):
      NET: phy_device, fix lock imbalance

Ralf Baechle (1):
      NET: Fix locking issues in PPP, 6pack, mkiss and strip line disciplines.

Sascha Hlusiak (1):
      sit: fix regression: do not release skb->dst before xmit

Tobias Klauser (1):
      skbuff.h: Fix comment for NET_IP_ALIGN

roel kluin (1):
      atlx: duplicate testing of MCAST flag

 drivers/net/8139too.c               |    1 +
 drivers/net/arm/ixp4xx_eth.c        |    4 +-
 drivers/net/atlx/atl2.c             |    2 +-
 drivers/net/cs89x0.c                |    7 ++-
 drivers/net/ehea/ehea_main.c        |    2 +
 drivers/net/fec.c                   |    1 +
 drivers/net/gianfar.c               |    2 +
 drivers/net/hamradio/6pack.c        |   10 +++--
 drivers/net/hamradio/mkiss.c        |   41 +++++++++++--------
 drivers/net/igb/igb_main.c          |   77 +++++++++++++++++------------------
 drivers/net/isa-skeleton.c          |    5 +-
 drivers/net/phy/phy_device.c        |    4 +-
 drivers/net/plip.c                  |    3 +
 drivers/net/ppp_async.c             |   11 +++--
 drivers/net/ppp_synctty.c           |   11 +++--
 drivers/net/ps3_gelic_net.c         |    1 +
 drivers/net/ps3_gelic_wireless.c    |    1 +
 drivers/net/smc91x.c                |    1 +
 drivers/net/smsc911x.c              |    1 +
 drivers/net/sunvnet.c               |    2 +
 drivers/net/usb/kaweth.c            |    3 +
 drivers/net/usb/pegasus.c           |    3 +
 drivers/net/via-rhine.c             |    1 +
 drivers/net/wireless/orinoco/main.c |    3 +-
 drivers/net/wireless/strip.c        |   39 +++++++++++-------
 include/linux/skbuff.h              |    4 +-
 net/core/sock.c                     |   19 ++++++++-
 net/ipv4/ip_gre.c                   |    4 +-
 net/ipv4/ip_output.c                |    1 -
 net/ipv6/ip6_output.c               |    1 -
 net/ipv6/sit.c                      |    1 +
 31 files changed, 165 insertions(+), 101 deletions(-)

^ permalink raw reply

* Re: [PATCH 1/2] NET: phy_device, fix lock imbalance
From: David Miller @ 2009-07-14 19:27 UTC (permalink / raw)
  To: jirislaby; +Cc: netdev, linux-kernel
In-Reply-To: <1247520220-31960-1-git-send-email-jirislaby@gmail.com>

From: Jiri Slaby <jirislaby@gmail.com>
Date: Mon, 13 Jul 2009 23:23:39 +0200

> Don't forget to unlock a mutex in phy_scan_fixups on a fail path.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

Applied.

^ permalink raw reply

* Re: [GIT]: Networking
From: Alan Cox @ 2009-07-14 19:31 UTC (permalink / raw)
  To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20090714.122704.21107059.davem@davemloft.net>

> 8) Locking fix in TTY based networking drivers, from Ralf Baechle.

I thought we had established that this one was wrong following further
discussion. The tty drivers that were calling up from IRQ handlers have
been fixed by the cull of bogus tty->low_latency setting.

^ permalink raw reply

* Re: [PATCH v2] Receive Packet Steering
From: David Miller @ 2009-07-14 19:33 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <65634d660907131504u35154059m5934cca3cb9363e0@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Mon, 13 Jul 2009 15:04:51 -0700

> On Mon, Jul 13, 2009 at 10:49 AM, David Miller <davem@davemloft.net> wrote:
>>
>> Why not take this to it's logical end point, which is to monitor
>> transmits using a tiny flow lookup table, and map receives of the same
>> flow to the same cpu?
> 
> Is it better do use transmits, or monitor where recvmsg was called?

First of all, using recvmsg would be on the more stateful side, and
we try to avoid that.  All of the logic should work if we had to
put it purely inside of dev_hard_xmit() and netif_receive_skb(),
way below the stack itself ever gets involved.

Second, Intel has committed the "monitor the TXs to device where to
steer RX" logic into silicon with their Flow Director stuff.  What
does that tell you about it's effectiveness? :-)

It seems the best heuristic to me, especially since it satisfies
many of the design constraints.

> Right.  In fact, just using the hash as the key is what you want when
> device provides the hash (i.e. Toeplitz).  The caveat is that we
> should to prevent OOO packets when threads migrate, I think I have a
> reasonable solution for that following your earlier suggestion.

But as you found, compared to jhash, Toeplitz is expensive to compute
and you would need to do this if implemented by monitoring transmits.

And furthermore, from the device perspective:

1) some (such as NIU) provide different hash values, not Toeplitz

2) it is expensive to extract this value (must enable different,
   larger, RX descriptor modes, thus more DMA traffic, etc.)

I really don't think Toeplitz is a net win, to be honest.

> I hope to have a new patch soon for steering packets to application
> CPU and using device hash, thanks for bugging me on it!

I had no choice, as I'm giving a presentation on this stuff tomorrow
night here in NYC :-)

^ permalink raw reply

* Re: [GIT]: Networking
From: David Miller @ 2009-07-14 19:34 UTC (permalink / raw)
  To: alan; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20090714203123.0ab1f7b8@lxorguk.ukuu.org.uk>

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Tue, 14 Jul 2009 20:31:23 +0100

>> 8) Locking fix in TTY based networking drivers, from Ralf Baechle.
> 
> I thought we had established that this one was wrong following further
> discussion. The tty drivers that were calling up from IRQ handlers have
> been fixed by the cull of bogus tty->low_latency setting.

Where was this change made?

At a minimum it likely needs to be added to the -stable trees.

^ permalink raw reply

* Re: Debounce code vs. dma_sync_single_range_for_cpu() and e100 driver.
From: Krzysztof Halasa @ 2009-07-14 19:34 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20090713181237.GD31979@n2100.arm.linux.org.uk>

A copy of my mail to linux-arm list.

Arch = ARM, CPU = IXP425, net device = 82551 using e100 driver.

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> Cache handling is performed when ownership of buffers transfer from the
> CPU to the device.  At this point in time, the CPU must not read or write
> any cache lines associated with the buffer.

Well, e100 driver uses streaming mapping for it's RX/TX buffers as well
as for the descriptors. Now it gets tricky - RX ring descriptors can be
written by the device at any time (when completing RX) and at the same
time the CPU has to be able to check the descriptor's status. It seems
it can't be formally done with the streaming DMA API, since it doesn't
provide invalidate and flush operations, it's rather about transfering
control.

I guess the coherent/consistent allocations should be used for this
purpose (= uncached RAM pages on IXP4xx if I understand it correctly).

Now that I know the inner working of DMA API the attached patch "fixes"
the e100 problems, but it still doesn't seem to be a valid use of the
API.

Comments?

--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -1762,6 +1762,9 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
 
 			if (ioread8(&nic->csr->scb.status) & rus_no_res)
 				nic->ru_running = RU_SUSPENDED;
+		pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr,
+					       sizeof(struct rfd),
+					       PCI_DMA_BIDIRECTIONAL);
 		return -ENODATA;
 	}
 

Without the patch:

$ ping 10.150 -i 0.2
PING 10.150 (10.0.0.150) 56(84) bytes of data.
64 bytes from 10.0.0.150: icmp_seq=1 ttl=64 time=619 ms
64 bytes from 10.0.0.150: icmp_seq=2 ttl=64 time=410 ms
64 bytes from 10.0.0.150: icmp_seq=3 ttl=64 time=200 ms
64 bytes from 10.0.0.150: icmp_seq=4 ttl=64 time=0.498 ms
64 bytes from 10.0.0.150: icmp_seq=5 ttl=64 time=2880 ms

With the patch:

$ ping 10.150 -i 0.2
PING 10.150 (10.0.0.150) 56(84) bytes of data.
64 bytes from 10.0.0.150: icmp_seq=1 ttl=64 time=7.05 ms
64 bytes from 10.0.0.150: icmp_seq=2 ttl=64 time=0.269 ms
64 bytes from 10.0.0.150: icmp_seq=3 ttl=64 time=0.887 ms
64 bytes from 10.0.0.150: icmp_seq=4 ttl=64 time=0.311 ms
64 bytes from 10.0.0.150: icmp_seq=5 ttl=64 time=0.878 ms

IXP425 533 MHz, i82551, 2.6.30.

OTOH, nobody uses e100 on non-coherent ARM? Perhaps a slower CPU (with
maybe higher IRQ latency) would hit the problem less frequently?

Thanks for your help.
-- 
Krzysztof Halasa

^ permalink raw reply

* Re: [PATCH] drivers/net: using spin_lock_irqsave() in net_send_packet()
From: David Miller @ 2009-07-14 19:35 UTC (permalink / raw)
  To: dongdong.deng; +Cc: netdev
In-Reply-To: <1247466426-14205-1-git-send-email-dongdong.deng@windriver.com>

From: Dongdong Deng <dongdong.deng@windriver.com>
Date: Mon, 13 Jul 2009 14:27:06 +0800

> spin_unlock_irq() will enable interrupt in net_send_packet(),
> this patch changes it to spin_lock_irqsave/spin_lock_irqrestore,
> so that it doesn't enable interrupts when already disabled,
> and netconsole would work properly over cs89x0/isa-skeleton.
> 
> Call trace:
> netconsole write_msg()
> {
>  ...
>  -> spin_lock_irqsave();
>         -> netpoll_send_udp()
>           -> netpoll_send_skb()
>             -> net_send_packet()
>               ->...
> 
>  -> spin_unlock_irqrestore();
>  ...
> }
> 
> Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com>

Applied.

^ permalink raw reply

* Re: [PATCH] skbuff.h: Fix comment for NET_IP_ALIGN
From: David Miller @ 2009-07-14 19:36 UTC (permalink / raw)
  To: klto; +Cc: netdev
In-Reply-To: <1247561296-25010-1-git-send-email-klto@zhaw.ch>

From: Tobias Klauser <klto@zhaw.ch>
Date: Tue, 14 Jul 2009 10:48:16 +0200

> Use the correct function call for skb_reserve in the comment for
> NET_IP_ALIGN.
> 
> Signed-off-by: Tobias Klauser <klto@zhaw.ch>

Applied, thanks.

^ permalink raw reply

* Re: Debounce code vs. dma_sync_single_range_for_cpu() and e100 driver.
From: David Miller @ 2009-07-14 19:38 UTC (permalink / raw)
  To: khc; +Cc: netdev
In-Reply-To: <m3d483m53q.fsf@intrepid.localdomain>

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Tue, 14 Jul 2009 21:34:01 +0200

> Well, e100 driver uses streaming mapping for it's RX/TX buffers as well
> as for the descriptors. Now it gets tricky - RX ring descriptors can be
> written by the device at any time (when completing RX) and at the same
> time the CPU has to be able to check the descriptor's status. It seems
> it can't be formally done with the streaming DMA API, since it doesn't
> provide invalidate and flush operations, it's rather about transfering
> control.
> 
> I guess the coherent/consistent allocations should be used for this
> purpose (= uncached RAM pages on IXP4xx if I understand it correctly).
> 
> Now that I know the inner working of DMA API the attached patch "fixes"
> the e100 problems, but it still doesn't seem to be a valid use of the
> API.

E100's use of streaming mappings for RX descriptors is a bug, it
should be using consistent mappings for sure.

And it's especially buggy if it isn't doing DMA API sync calls before
looking at descriptor fields, as your patch seems to cure.

^ 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