Netdev List
 help / color / mirror / Atom feed
* Re: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: David Miller @ 2010-09-28  2:41 UTC (permalink / raw)
  To: akpm; +Cc: netdev, bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <20100927161540.776f748e.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 27 Sep 2010 16:15:40 -0700

> Yes, I think seglen should be size_t.  Do you know if making that
> change fixes the bug?

Oh boy, what a rats nest.

Just scanning generically I see that net/core/iovec.c:verify_iovec()
has similar issues, it should probably use "long" instead of "int"
because it's trying to prevent the return value being interpreted
as an error code.

So if you fix that and make it return "long" this leads to another
set of problems, even if you fix that TCP bit sys_sendmsg() holds
the total length in an 'int' too.  So more stuff to fix.

I'll try to do the whole audit, but no promises ;)

^ permalink raw reply

* Re: bonding vs GSO vs checksumming.
From: Herbert Xu @ 2010-09-28  2:49 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Linux NetDev
In-Reply-To: <AANLkTim2FF2n9JXfpdZmmnV5ZJ1ptC7se_JiU_z91okt@mail.gmail.com>

On Thu, Sep 23, 2010 at 05:56:43PM -0700, Maciej Żenczykowski wrote:
> I'm seeing the following:
> 
> bonding: caps=(0x1113a3, 0x1113ab) len=18192 data_len=18120 ip_summed=0
> 
> on a 2.6.34.5 based kernel originating from v6 tcp transmit (mtu 1280)
> on a bonded device of tg3 (presumably with TSO6) and forcedeth
> (presumably without TSO6).
> 
> I can't quite figure out what this truly means...

Did it come with a kernel backtrace?

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: pull-request: bluetooth-2.6 2010-09-27
From: David Miller @ 2010-09-28  3:00 UTC (permalink / raw)
  To: padovan-Y3ZbgMPKUGA34EUeqzHoZw
  Cc: linville-2XuSBdqkA4R54TAoqtyWWQ, marcel-kz+m5ild9QBg9hUCZPvPmw,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100928023035.GA3033@vigoh>

From: "Gustavo F. Padovan" <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
Date: Mon, 27 Sep 2010 23:30:35 -0300

> And a fix for a deadlock issue between the sk_sndbuf and the backlog
> queue in ERTM. The rest are also needed bug fixes.

This fix is still under discussion.

That change effects quite a few code paths.  And when I looked
at them, I was not at all convinced that dropping the socket
lock like that is safe.

Are you sure there are no pieces of socket or socket related state
that might change under us while we drop that lock, which would thus
make the operation suddenly invalid or cause a state corruption or
crash?

You really need to audit this.

^ permalink raw reply

* Re: bonding vs GSO vs checksumming.
From: Herbert Xu @ 2010-09-28  3:21 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Linux NetDev
In-Reply-To: <20100928024907.GA24948@gondor.apana.org.au>

On Tue, Sep 28, 2010 at 11:49:07AM +0900, Herbert Xu wrote:
> On Thu, Sep 23, 2010 at 05:56:43PM -0700, Maciej Żenczykowski wrote:
> > I'm seeing the following:
> > 
> > bonding: caps=(0x1113a3, 0x1113ab) len=18192 data_len=18120 ip_summed=0
> > 
> > on a 2.6.34.5 based kernel originating from v6 tcp transmit (mtu 1280)
> > on a bonded device of tg3 (presumably with TSO6) and forcedeth
> > (presumably without TSO6).
> > 
> > I can't quite figure out what this truly means...
> 
> Did it come with a kernel backtrace?

Actually your particular problem should be fixed by

commit 6afff0caa721211e8c04bdc7627ee3bff95bcb95
Author: John Fastabend <john.r.fastabend@intel.com>
Date:   Wed Jun 16 14:18:12 2010 +0000

    net: consolidate netif_needs_gso() checks

which went in after 2.6.34.

However, the fact that your feature bits show that bonding is
declaring TSO6 support without IPv6 checksum support also means
that the bonding feature computation is broken.

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: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: David Miller @ 2010-09-28  3:24 UTC (permalink / raw)
  To: akpm; +Cc: netdev, bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <20100927161540.776f748e.akpm@linux-foundation.org>


Ok, I suspect the following is enough to fix this specific bug
report, TCP sending.

However, as I stated in my previous reply there are creepy
crawlies all over the place.

For example, all of the routines in net/core/iovec.c (memcpy_toiovec,
memcpy_toiovecend, memcpy_fromiovec, memcpy_fromiovecend,
csum_partial_copy_fromiovecend) that cast using min_t() are currently
casting "down" to "unsigned int".

They should probably case "up" to "size_t".

Otherwise 4GB iov_len's on 64-bit will be truncated to zero just
as they do in the TCP path being fixed here.

If, alterntatively, we want to try and take the size_t values all
the way down the code paths to the individual copies, that is
a huge undertaking.

It's huge because we end up getting to the architecture specific
csum_copy_*() routines, which all take 'int' as the length and
many are written in assembler and would need audits and potentially
changes before we can make the type 'long' or 'size_t'.

Anyways, this part here is simple enough and I'll push it to
Linus and -stable.

--------------------
tcp: Fix >4GB writes on 64-bit.

Fixes kernel bugzilla #16603

tcp_sendmsg() truncates iov_len to an 'int' which a 4GB write to write
zero bytes, for example.

There is also the problem higher up of how verify_iovec() works.  It
wants to prevent the total length from looking like an error return
value.

However it does this using 'int', but syscalls return 'long' (and
thus signed 64-bit on 64-bit machines).  So it could trigger
false-positives on 64-bit as written.  So fix it to use 'long'.

Reported-by: Olaf Bonorden <bono@onlinehome.de>
Reported-by: Daniel Büse <dbuese@gmx.de>
Reported-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/linux/socket.h b/include/linux/socket.h
index a2fada9..a8f56e1 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -322,7 +322,7 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
 					  int offset, 
 					  unsigned int len, __wsum *csump);
 
-extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr *address, int mode);
+extern long verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr *address, int mode);
 extern int memcpy_toiovec(struct iovec *v, unsigned char *kdata, int len);
 extern int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
 			     int offset, int len);
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 1cd98df..e6b133b 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -35,9 +35,10 @@
  *	in any case.
  */
 
-int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr *address, int mode)
+long verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr *address, int mode)
 {
-	int size, err, ct;
+	int size, ct;
+	long err;
 
 	if (m->msg_namelen) {
 		if (mode == VERIFY_READ) {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 95d75d4..f115ea6 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -943,7 +943,7 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	sg = sk->sk_route_caps & NETIF_F_SG;
 
 	while (--iovlen >= 0) {
-		int seglen = iov->iov_len;
+		size_t seglen = iov->iov_len;
 		unsigned char __user *from = iov->iov_base;
 
 		iov++;

^ permalink raw reply related

* [PATCH V3] fs: allow for more than 2^31 files
From: Eric Dumazet @ 2010-09-28  3:46 UTC (permalink / raw)
  To: David Miller
  Cc: dipankar, holt, viro, bcrl, den, mingo, mszeredi, cmm, npiggin,
	xemul, linux-kernel, netdev
In-Reply-To: <20100927.153639.212415479.davem@davemloft.net>

Le lundi 27 septembre 2010 à 15:36 -0700, David Miller a écrit :

> Is someone following up on integrating this upstream so this thing
> gets fixed?
> 

Thanks for the heads-up.

I am not sure V2 of my patch was reviewed, maybe it did not reach the
list.

Here is V3 of it. I removed the ATOMIC_INIT(0) I left in V2.
It should be an ATOMIC_LONG_INIT(0), but then, it can be avoided.

CC netdev

Thanks

[PATCH V3] fs: allow for more than 2^31 files

Robin Holt tried to boot a 16TB system and found af_unix was overflowing
a 32bit value :

<quote>

We were seeing a failure which prevented boot.  The kernel was incapable
of creating either a named pipe or unix domain socket.  This comes down
to a common kernel function called unix_create1() which does:

        atomic_inc(&unix_nr_socks);
        if (atomic_read(&unix_nr_socks) > 2 * get_max_files())
                goto out;

The function get_max_files() is a simple return of files_stat.max_files.
files_stat.max_files is a signed integer and is computed in
fs/file_table.c's files_init().

        n = (mempages * (PAGE_SIZE / 1024)) / 10;
        files_stat.max_files = n;

In our case, mempages (total_ram_pages) is approx 3,758,096,384
(0xe0000000).  That leaves max_files at approximately 1,503,238,553.
This causes 2 * get_max_files() to integer overflow.

</quote>

Fix is to let /proc/sys/fs/file-nr & /proc/sys/fs/file-max use long
integers, and change af_unix to use an atomic_long_t instead of
atomic_t.

get_max_files() is changed to return an unsigned long.
get_nr_files() is changed to return a long.

unix_nr_socks is changed from atomic_t to atomic_long_t, while not
strictly needed to address Robin problem.
 
Before patch (on a 64bit kernel) :
# echo 2147483648 >/proc/sys/fs/file-max
# cat /proc/sys/fs/file-max
-18446744071562067968

After patch:
# echo 2147483648 >/proc/sys/fs/file-max
# cat /proc/sys/fs/file-max
2147483648


Reported-by: Robin Holt <holt@sgi.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 fs/file_table.c    |   15 ++++++---------
 include/linux/fs.h |    8 ++++----
 kernel/sysctl.c    |    8 ++++----
 net/unix/af_unix.c |   14 +++++++-------
 4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index a04bdd8..46457ba 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -60,7 +60,7 @@ static inline void file_free(struct file *f)
 /*
  * Return the total number of open files in the system
  */
-static int get_nr_files(void)
+static long get_nr_files(void)
 {
 	return percpu_counter_read_positive(&nr_files);
 }
@@ -68,7 +68,7 @@ static int get_nr_files(void)
 /*
  * Return the maximum number of open files in the system
  */
-int get_max_files(void)
+unsigned long get_max_files(void)
 {
 	return files_stat.max_files;
 }
@@ -105,7 +105,7 @@ int proc_nr_files(ctl_table *table, int write,
 struct file *get_empty_filp(void)
 {
 	const struct cred *cred = current_cred();
-	static int old_max;
+	static long old_max;
 	struct file * f;
 
 	/*
@@ -140,8 +140,7 @@ struct file *get_empty_filp(void)
 over:
 	/* Ran out of filps - report that */
 	if (get_nr_files() > old_max) {
-		printk(KERN_INFO "VFS: file-max limit %d reached\n",
-					get_max_files());
+		pr_info("VFS: file-max limit %lu reached\n", get_max_files());
 		old_max = get_nr_files();
 	}
 	goto fail;
@@ -487,7 +486,7 @@ retry:
 
 void __init files_init(unsigned long mempages)
 { 
-	int n; 
+	unsigned long n;
 
 	filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
 			SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
@@ -498,9 +497,7 @@ void __init files_init(unsigned long mempages)
 	 */ 
 
 	n = (mempages * (PAGE_SIZE / 1024)) / 10;
-	files_stat.max_files = n; 
-	if (files_stat.max_files < NR_FILE)
-		files_stat.max_files = NR_FILE;
+	files_stat.max_files = max_t(unsigned long, n, NR_FILE);
 	files_defer_init();
 	lg_lock_init(files_lglock);
 	percpu_counter_init(&nr_files, 0);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 63d069b..8c06590 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -34,9 +34,9 @@
 
 /* And dynamically-tunable limits and defaults: */
 struct files_stat_struct {
-	int nr_files;		/* read only */
-	int nr_free_files;	/* read only */
-	int max_files;		/* tunable */
+	unsigned long nr_files;		/* read only */
+	unsigned long nr_free_files;	/* read only */
+	unsigned long max_files;		/* tunable */
 };
 
 struct inodes_stat_t {
@@ -404,7 +404,7 @@ extern void __init inode_init_early(void);
 extern void __init files_init(unsigned long);
 
 extern struct files_stat_struct files_stat;
-extern int get_max_files(void);
+extern unsigned long get_max_files(void);
 extern int sysctl_nr_open;
 extern struct inodes_stat_t inodes_stat;
 extern int leases_enable, lease_break_time;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f88552c..fc667bf 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1352,16 +1352,16 @@ static struct ctl_table fs_table[] = {
 	{
 		.procname	= "file-nr",
 		.data		= &files_stat,
-		.maxlen		= 3*sizeof(int),
+		.maxlen		= sizeof(files_stat),
 		.mode		= 0444,
-		.proc_handler	= proc_nr_files,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "file-max",
 		.data		= &files_stat.max_files,
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(files_stat.max_files),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "nr_open",
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 0b39b24..3e1d7d1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -117,7 +117,7 @@
 
 static struct hlist_head unix_socket_table[UNIX_HASH_SIZE + 1];
 static DEFINE_SPINLOCK(unix_table_lock);
-static atomic_t unix_nr_socks = ATOMIC_INIT(0);
+static atomic_long_t unix_nr_socks;
 
 #define unix_sockets_unbound	(&unix_socket_table[UNIX_HASH_SIZE])
 
@@ -360,13 +360,13 @@ static void unix_sock_destructor(struct sock *sk)
 	if (u->addr)
 		unix_release_addr(u->addr);
 
-	atomic_dec(&unix_nr_socks);
+	atomic_long_dec(&unix_nr_socks);
 	local_bh_disable();
 	sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
 	local_bh_enable();
 #ifdef UNIX_REFCNT_DEBUG
-	printk(KERN_DEBUG "UNIX %p is destroyed, %d are still alive.\n", sk,
-		atomic_read(&unix_nr_socks));
+	printk(KERN_DEBUG "UNIX %p is destroyed, %ld are still alive.\n", sk,
+		atomic_long_read(&unix_nr_socks));
 #endif
 }
 
@@ -606,8 +606,8 @@ static struct sock *unix_create1(struct net *net, struct socket *sock)
 	struct sock *sk = NULL;
 	struct unix_sock *u;
 
-	atomic_inc(&unix_nr_socks);
-	if (atomic_read(&unix_nr_socks) > 2 * get_max_files())
+	atomic_long_inc(&unix_nr_socks);
+	if (atomic_long_read(&unix_nr_socks) > 2 * get_max_files())
 		goto out;
 
 	sk = sk_alloc(net, PF_UNIX, GFP_KERNEL, &unix_proto);
@@ -632,7 +632,7 @@ static struct sock *unix_create1(struct net *net, struct socket *sock)
 	unix_insert_socket(unix_sockets_unbound, sk);
 out:
 	if (sk == NULL)
-		atomic_dec(&unix_nr_socks);
+		atomic_long_dec(&unix_nr_socks);
 	else {
 		local_bh_disable();
 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);

^ permalink raw reply related

* Re: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: Andrew Morton @ 2010-09-28  3:56 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <20100927.202425.71120040.davem@davemloft.net>

On Mon, 27 Sep 2010 20:24:25 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> Ok, I suspect the following is enough to fix this specific bug
> report, TCP sending.
> 
> However, as I stated in my previous reply there are creepy
> crawlies all over the place.
> 
> For example, all of the routines in net/core/iovec.c (memcpy_toiovec,
> memcpy_toiovecend, memcpy_fromiovec, memcpy_fromiovecend,
> csum_partial_copy_fromiovecend) that cast using min_t() are currently
> casting "down" to "unsigned int".
> 
> They should probably case "up" to "size_t".

eep.

A blanket suckyfix might be, at the syscall level:

	if (size > 4g) {
		do_it_in_4g_hunks();
		do_the_last_bit();
	}

unless that would break some networking syscall->framesize guarantees
or something?

And such a "fix" would make it hard to test the real fix!


I'm surprised that this issue hasn't come up before.

^ permalink raw reply

* Re: [Bugme-new] [Bug 16603] New: send of data > 4 GB fails on 64 bit systems
From: David Miller @ 2010-09-28  4:07 UTC (permalink / raw)
  To: akpm; +Cc: netdev, bugzilla-daemon, bugme-daemon, bono
In-Reply-To: <20100927205624.1564649e.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 27 Sep 2010 20:56:24 -0700

> A blanket suckyfix might be, at the syscall level:
> 
> 	if (size > 4g) {
> 		do_it_in_4g_hunks();
> 		do_the_last_bit();
> 	}
> 
> unless that would break some networking syscall->framesize guarantees
> or something?

There is not a single length passed in, but a vector of them, that's
what the iovec conveys.

Even if you could, socket send calls have atomicity guarentees.  For a
datagram socket, for example, a single send call corresponds to one
packet on the network.

BTW, this aspect of datagram sockets is a part of the reason we don't
see many reports about this stuff. :-)  Only stream protocols can
really take such enormous lengths, and the most popular (TCP) does
much of the iovec handling by hand.

We just need to fix our junk, and the {min,max}_t(size_t, ...) change
I suggested is likely the easiest path.

^ permalink raw reply

* Re: [PATCH V3] fs: allow for more than 2^31 files
From: David Miller @ 2010-09-28  4:10 UTC (permalink / raw)
  To: eric.dumazet
  Cc: dipankar, holt, viro, bcrl, den, mingo, mszeredi, cmm, npiggin,
	xemul, linux-kernel, netdev
In-Reply-To: <1285645611.10438.27.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 28 Sep 2010 05:46:51 +0200

> [PATCH V3] fs: allow for more than 2^31 files
...
> Reported-by: Robin Holt <holt@sgi.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH net-next-2.6 12/17] mv643xx_eth: Use netif_set_real_num_{rx,tx}_queues()
From: Lennert Buytenhek @ 2010-09-28  4:18 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1285612205.2263.310.camel@achroite.uk.solarflarecom.com>

On Mon, Sep 27, 2010 at 07:30:05PM +0100, Ben Hutchings wrote:

> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>

^ permalink raw reply

* Re: [PATCH net-next-2.6 0/5] XFRM,IPv6: Removal of RH2/HAO from IPsec-protected MIPv6 traffic
From: David Miller @ 2010-09-28  4:25 UTC (permalink / raw)
  To: arno; +Cc: eric.dumazet, herbert, yoshfuji, netdev
In-Reply-To: <87bp7nrlvy.fsf@small.ssi.corp>


Please resubmit this after you've done some more exhaustive
build testing:

---
net/ipv6/xfrm6_input.c: In function ‘xfrm6_input_addr_check’:
net/ipv6/xfrm6_input.c:173: error: implicit declaration of function ‘ipv6_chk_home_addr’
net/ipv6/xfrm6_input.c:179: error: ‘struct sec_path’ has no member named ‘irodst’
net/ipv6/xfrm6_input.c:202: error: ‘struct sec_path’ has no member named ‘irosrc’
---
davem@sunset:~/src/GIT/net-next-2.6$ egrep XFRM .config
CONFIG_XFRM=y
CONFIG_XFRM_USER=m
# CONFIG_XFRM_SUB_POLICY is not set
CONFIG_XFRM_MIGRATE=y
# CONFIG_XFRM_STATISTICS is not set
CONFIG_XFRM_IPCOMP=y
CONFIG_INET_XFRM_TUNNEL=y
CONFIG_INET_XFRM_MODE_TRANSPORT=y
CONFIG_INET_XFRM_MODE_TUNNEL=y
CONFIG_INET_XFRM_MODE_BEET=y
CONFIG_INET6_XFRM_TUNNEL=m
CONFIG_INET6_XFRM_MODE_TRANSPORT=m
CONFIG_INET6_XFRM_MODE_TUNNEL=m
CONFIG_INET6_XFRM_MODE_BEET=m
# CONFIG_INET6_XFRM_MODE_ROUTEOPTIMIZATION is not set
davem@sunset:~/src/GIT/net-next-2.6$ 

^ permalink raw reply

* Re: [PATCH v2 1/2] Phonet: Implement Pipe Controller to support Nokia Slim Modems
From: David Miller @ 2010-09-28  4:50 UTC (permalink / raw)
  To: kumar.sanghvi
  Cc: remi.denis-courmont, netdev, STEricsson_nomadik_linux,
	sudeep.divakaran, gulshan.karmani, linus.walleij
In-Reply-To: <1285564079-23066-1-git-send-email-kumar.sanghvi@stericsson.com>

From: Kumar A Sanghvi <kumar.sanghvi@stericsson.com>
Date: Mon, 27 Sep 2010 10:37:59 +0530

> From: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
> 
> Phonet stack assumes the presence of Pipe Controller, either in Modem or
> on Application Processing Engine user-space for the Pipe data.
> Nokia Slim Modems like WG2.5 used in ST-Ericsson U8500 platform do not
> implement Pipe controller in them.
> This patch adds Pipe Controller implemenation to Phonet stack to support
> Pipe data over Phonet stack for Nokia Slim Modems.
> 
> Signed-off-by: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>

Applied, thanks.

Please resubmit patch 2/2 if you want me to apply it.

^ permalink raw reply

* Re: [PATCH net-next-2.6] vlan: use this_cpu_ptr() in vlan_skb_recv()
From: David Miller @ 2010-09-28  4:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1285577229.10965.3.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Sep 2010 10:47:09 +0200

> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 0/4] tunnels: SMP safe accounting
From: David Miller @ 2010-09-28  4:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1285583568.23938.81.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Sep 2010 12:32:48 +0200

> Before making ip_gre, ipip, and sit transmit path lockless, it is
> necessary to make accounting SMP safe, not only for correctness but to
> avoid cache line ping pongs.
> 
> This also takes care of receive path, already lockless and using unsafe
> stats accounting.
> 
> I chose to use small percpu structures, holding {rx|tx}_{packets|bytes}
> only, and keep all other counters in netdev->stats, since they should be
> seldom used.

Series applied, thanks a lot Eric!

^ permalink raw reply

* Re: [PATCH net-next-2.6] fib: use atomic_inc_not_zero() in fib_rules_lookup
From: David Miller @ 2010-09-28  4:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, paulmck
In-Reply-To: <1285597107.23938.250.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Sep 2010 16:18:27 +0200

> It seems we dont use appropriate refcount increment in an
> rcu_read_lock() protected section.
> 
> fib_rule_get() might increment a null refcount and bad things could
> happen.
> 
> While fib_nl_delrule() respects an rcu grace period before calling
> fib_rule_put(), fib_rules_cleanup_ops() calls fib_rule_put() without a
> grace period.
> 
> Note : after this patch, we might avoid the synchronize_rcu() call done
> in fib_nl_delrule()
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] net: sk_{detach|attach}_filter() rcu fixes
From: David Miller @ 2010-09-28  4:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, paulmck
In-Reply-To: <1285603650.3017.57.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Sep 2010 18:07:30 +0200

> Le lundi 27 septembre 2010 à 17:57 +0200, Eric Dumazet a écrit :
>> sk_attach_filter() is run with socket locked.
>> 
>> Use the appropriate rcu_dereference_protected() instead of blocking BH,
>> and rcu_dereference_bh().
>> There is no point adding BH prevention and memory barrier.
> 
> Hmm, same thing can be done in sk_detach_filter, here is a v2.
> 
> Thanks
> 
> [PATCH net-next-2.6 v2] net: sk_{detach|attach}_filter() rcu fixes
> 
> sk_attach_filter() and sk_detach_filter() are run with socket locked.
> 
> Use the appropriate rcu_dereference_protected() instead of blocking BH,
> and rcu_dereference_bh().
> There is no point adding BH prevention and memory barrier.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [Bugme-new] [Bug 18952] New: The mount of SYN retries is not equal to /proc/sys/net/ipv4/tcp_syn_retries
From: David Miller @ 2010-09-28  4:52 UTC (permalink / raw)
  To: damian; +Cc: yuri, akpm, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <1285617608.9983.30.camel@nexus>

From: Damian Lukowski <damian@tvk.rwth-aachen.de>
Date: Mon, 27 Sep 2010 22:00:08 +0200

> My suggestion for solving this issue:
> Introducing a third boolean parameter for retransmits_timed_out()
> indicating whether the socket is in SYN state or not. In the SYN case, 
> TCP_TIMEOUT_INIT will be used for the calculation instead of
> TCP_RTO_MIN.
> 
> Is that ok?

Sounds fine to me, please prepare a patch.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next-2.6 00/17] netdev: Fix queue count initialisation
From: David Miller @ 2010-09-28  5:12 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1285611791.2263.287.camel@achroite.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 27 Sep 2010 19:23:11 +0100

> This adds the functions:
> - netif_set_real_num_rx_queues() - set actual number of RX queues used
> - netif_copy_real_num_queues() - copy queue counts from another device
> 
> and changes all drivers that currently set
> net_device::real_num_tx_queues to use netif_set_real_num_tx_queues()
> and/or these functions.
> 
> The changes are compile-tested only, except that:
> - sfc and 8021q have been briefly tested
> - gianfar and mv643xx_eth have not been compiled, since they are
>   platform-specific
> 
> I noticed that the bonding driver sets its numbers of queues without
> regard to its slave devices.  This makes some sense since a bond device
> initially has no slave devices.  However, it seems to mean that a bond
> device can pass up an skb with an out-of-range queue_index, triggering a
> warning in get_rps_cpu().

Series applied, thanks Ben.

^ permalink raw reply

* Re: [PATCH net-next-2.6 16/17] tg3: Use netif_set_real_num_{rx,tx}_queues()
From: David Miller @ 2010-09-28  5:13 UTC (permalink / raw)
  To: mcarlson; +Cc: bhutchings, netdev, linux-net-drivers, mchan
In-Reply-To: <20100927214416.GA32384@mcarlson.broadcom.com>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Mon, 27 Sep 2010 14:44:16 -0700

> On Mon, Sep 27, 2010 at 02:41:07PM -0700, Matt Carlson wrote:
>> This should be tp->irq_cnt - 1, not tp->irq_cnt.  The first MSI-X vector
>> only handles link interrupts and device errors.
> 
> I need to correct myself.  If the irq_cnt > 1, then it needs to be
> tp->irq_cnt - 1.  If the irq_cnt == 1, then it should be 1.

I fixed this when I applied Ben's patch, thanks!

^ permalink raw reply

* Re: TCP: orphans broken by RFC 2525 #2.17
From: Willy Tarreau @ 2010-09-28  5:12 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <20100927232857.GA23803@gondor.apana.org.au>

On Tue, Sep 28, 2010 at 08:28:57AM +0900, Herbert Xu wrote:
> Willy Tarreau <w@1wt.eu> wrote:
> > 
> > Two quick facts :
> >  - HTTP allows the client to send whatever it wants whenever it wants
> >    and allows the server to close after whatever response it wants.
> >    Thus the server cannot predict that the client will talk.
> 
> No it does not.  Only buggy HTTP clients do that.

I don't agree Herbert. Pipelining consists exactly in that, and is an
encouraged practice. If you say that only buggy clients send an extra
CRLF, then yes I agree. But you might very well get a new request you
can't predict at all, and the arrival of this new request must not 
destroy the end of last response's data.

Once again, for now, I will only handle this precise case of the extra
CRLF, but this will not fix the issue with pipelined requests.

Regards,
Willy


^ permalink raw reply

* Re: [PATCH v2 1/2] Phonet: Implement Pipe Controller to support Nokia Slim Modems
From: Kumar SANGHVI @ 2010-09-28  5:31 UTC (permalink / raw)
  To: David Miller
  Cc: remi.denis-courmont@nokia.com, netdev@vger.kernel.org,
	STEricsson_nomadik_linux, Sudeep DIVAKARAN, Gulshan KARMANI,
	Linus WALLEIJ
In-Reply-To: <20100927.215059.39193491.davem@davemloft.net>

On Tue, Sep 28, 2010 at 06:50:59 +0200, David Miller wrote:
 
> Applied, thanks.
> 
> Please resubmit patch 2/2 if you want me to apply it.

Thanks David.
I will re-submit the patch 2/2.

Best regards,
Kumar.

^ permalink raw reply

* Re: TCP: orphans broken by RFC 2525 #2.17
From: David Miller @ 2010-09-28  5:32 UTC (permalink / raw)
  To: w; +Cc: herbert, netdev
In-Reply-To: <20100928051258.GB12373@1wt.eu>

From: Willy Tarreau <w@1wt.eu>
Date: Tue, 28 Sep 2010 07:12:58 +0200

> Once again, for now, I will only handle this precise case of the extra
> CRLF, but this will not fix the issue with pipelined requests.

You might want to read server/connection.c in the apache httpd
sources.

It does exactly what Eric Dumazet and I have suggested to you.

But, I guess the entire world is wrong and you're right Willy.

^ permalink raw reply

* [PATCH 2/2] Documentation: Update Phonet doc for Pipe Controller implementation
From: Kumar A Sanghvi @ 2010-09-28  5:35 UTC (permalink / raw)
  To: remi.denis-courmont, davem, netdev
  Cc: STEricsson_nomadik_linux, sudeep.divakaran, gulshan.karmani,
	Kumar Sanghvi, Linus Walleij

From: Kumar Sanghvi <kumar.sanghvi@stericsson.com>

Updates the Phonet document with description related to Pipe controller
implementation

Signed-off-by: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
---
 Documentation/networking/phonet.txt |   53 +++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index cf76608..cccf5ff 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -182,6 +182,59 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
     or zero if encapsulation is off.
 
 
+Phonet Pipe-controller Implementation
+-------------------------------------
+
+Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR Kconfig
+option. It is useful when communicating with those Nokia Modems which do not
+implement Pipe controller in them e.g. Nokia Slim Modem used in ST-Ericsson
+U8500 platform.
+
+The implementation is based on the Data Connection Establishment Sequence
+depicted in 'Nokia Wireless Modem API - Wireless_modem_user_guide.pdf'
+document.
+
+It allows a phonet sequenced socket (host-pep) to initiate a Pipe connection
+between itself and a remote pipe-end point (e.g. modem).
+
+The implementation adds socket options at SOL_PNPIPE level:
+
+ PNPIPE_CREATE
+	It accepts an integer argument where-in
+		lower order 16 bits: pn_dev and pn_port pair for remote pep.
+		higher order 16 bits: 8 bit pipe-handle
+
+	It sends a PNS_PEP_CONNECT_REQ on sequenced socket itself. On getting
+	PNS_PEP_CONNECT_RESP, it sends PNS_PEP_CONNECT_REQ to remote pep. On
+	getting response from remote pep, it selects the best possible Flow
+	control mechanism supported by remote-pep (modem) and then it sends
+	PNS_PEP_CREATED_IND to the sequenced socket and to the remote pep.
+
+	It then updates the pipe state associated with the sequenced socket to
+	be PIPE_DISABLED.
+
+  PNPIPE_ENABLE
+	It follows the same sequence as above for enabling a pipe by sending
+	PNS_PEP_ENABLE_REQ initially and then sending PNS_PEP_ENABLED_IND after
+	getting responses from sequenced socket and remote-pep.
+	It will also update the pipe state associated with the sequenced socket
+	to PIPE_ENABLED.
+
+   PNPIPE_DESTROY
+	This will send out PNS_PEP_DISCONNECT_REQ on the sequenced socket and
+	the remote pep.
+	It will also update the pipe state associated with the sequenced socket
+	to PIPE_IDLE
+
+   PNPIPE_INQ
+	This getsocktopt allows the user-space running on the sequenced socket
+	to examine the pipe state associated with that socket ie. whether the
+	pipe is created (PIPE_DISABLED) or enabled (PIPE_ENABLED) or disabled
+	(PIPE_DISABLED) or no pipe exists (PIPE_IDLE).
+
+After a pipe has been created and enabled successfully, the Pipe data can be
+exchanged between the host-pep and remote-pep (modem).
+
 Authors
 -------
 
-- 
1.7.2.dirty


^ permalink raw reply related

* Re: TCP: orphans broken by RFC 2525 #2.17
From: Willy Tarreau @ 2010-09-28  5:37 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, netdev
In-Reply-To: <20100927.223231.179927955.davem@davemloft.net>

On Mon, Sep 27, 2010 at 10:32:31PM -0700, David Miller wrote:
> From: Willy Tarreau <w@1wt.eu>
> Date: Tue, 28 Sep 2010 07:12:58 +0200
> 
> > Once again, for now, I will only handle this precise case of the extra
> > CRLF, but this will not fix the issue with pipelined requests.
> 
> You might want to read server/connection.c in the apache httpd
> sources.
>
> It does exactly what Eric Dumazet and I have suggested to you.

I've seen that. Though nginx, squid, thttpd, haproxy and lighttpd all
make use of the standard orphans and are all affected by the same issue.

> But, I guess the entire world is wrong and you're right Willy.

Given the figures above, that's not how I analyze it, I'm sorry.

Willy


^ permalink raw reply

* Re: [PATCH 2/2] Documentation: Update Phonet doc for Pipe Controller implementation
From: David Miller @ 2010-09-28  6:30 UTC (permalink / raw)
  To: kumar.sanghvi
  Cc: remi.denis-courmont, netdev, STEricsson_nomadik_linux,
	sudeep.divakaran, gulshan.karmani, linus.walleij
In-Reply-To: <1285652106-29966-1-git-send-email-kumar.sanghvi@stericsson.com>

From: Kumar A Sanghvi <kumar.sanghvi@stericsson.com>
Date: Tue, 28 Sep 2010 11:05:06 +0530

> From: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
> 
> Updates the Phonet document with description related to Pipe controller
> implementation
> 
> Signed-off-by: Kumar Sanghvi <kumar.sanghvi@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>

Applied, thanks.

^ 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