Netdev List
 help / color / mirror / Atom feed
* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Octavian Purdila @ 2009-11-14  0:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20091113160445.7a3538ef@nehalam>

On Saturday 14 November 2009 02:04:45 you wrote:
> On Fri, 13 Nov 2009 07:20:19 +0200
> 
> Octavian Purdila <opurdila@ixiacom.com> wrote:
> > On Friday 13 November 2009 07:01:14 you wrote:
> > > This patch speeds up the network device name allocation for the case
> > > where a significant number of devices of the same type are created
> > > consecutively.
> > >
> > > Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> > > and sysfs entries disabled:
> > >
> > > Without the patch           With the patch
> > >
> > > real    0m 43.43s	    real    0m 0.49s
> > > user    0m 0.00s	    user    0m 0.00s
> > > sys     0m 43.43s	    sys     0m 0.48s
> 
> Since the main overhead here is building the bitmap table used in the
> name scan. Why not mantain the bitmap table between calls by
> implementing a rbtree with prefix -> bitmap.
> The tree would have to be limited and per namespace but then you
> could handle the general case of adding a device, then its vlans,
> then another device, ...
> 

I'll do that !

That was my original intent but I thought it would be too much bloat :) But I 
see your point, even if it is more complex, its more useful.

Thanks,
tavi

^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Stephen Hemminger @ 2009-11-14  0:20 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev
In-Reply-To: <200911140214.21493.opurdila@ixiacom.com>

On Sat, 14 Nov 2009 02:14:21 +0200
Octavian Purdila <opurdila@ixiacom.com> wrote:

> On Saturday 14 November 2009 02:04:45 you wrote:
> > On Fri, 13 Nov 2009 07:20:19 +0200
> > 
> > Octavian Purdila <opurdila@ixiacom.com> wrote:
> > > On Friday 13 November 2009 07:01:14 you wrote:
> > > > This patch speeds up the network device name allocation for the case
> > > > where a significant number of devices of the same type are created
> > > > consecutively.
> > > >
> > > > Tests performed on a PPC750 @ 800Mhz machine with per device sysctl
> > > > and sysfs entries disabled:
> > > >
> > > > Without the patch           With the patch
> > > >
> > > > real    0m 43.43s	    real    0m 0.49s
> > > > user    0m 0.00s	    user    0m 0.00s
> > > > sys     0m 43.43s	    sys     0m 0.48s
> > 
> > Since the main overhead here is building the bitmap table used in the
> > name scan. Why not mantain the bitmap table between calls by
> > implementing a rbtree with prefix -> bitmap.
> > The tree would have to be limited and per namespace but then you
> > could handle the general case of adding a device, then its vlans,
> > then another device, ...
> > 
> 
> I'll do that !
> 
> That was my original intent but I thought it would be too much bloat :) But I 
> see your point, even if it is more complex, its more useful.

There might even be a VM notifier hook that could be used to drop the whole
tree if any memory pressure was felt.

-- 

^ permalink raw reply

* [PATCH] check the return value of ndo_select_queue()
From: Changli Gao @ 2009-11-14  0:50 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eric Dumazet, Changli Gao

check the return value of ndo_select_queue()

Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/core/dev.c |   33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index bf629ac..74a3824 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1849,22 +1849,37 @@ static struct netdev_queue *dev_pick_tx(struct
net_device *dev,
 {
 	u16 queue_index;
 	struct sock *sk = skb->sk;
+	unsigned int real_num_tx_queues = dev->real_num_tx_queues;

 	if (sk_tx_queue_recorded(sk)) {
 		queue_index = sk_tx_queue_get(sk);
+		if (unlikely(queue_index >= real_num_tx_queues)) {
+			do {
+				queue_index -= real_num_tx_queues;
+			} while (queue_index >= real_num_tx_queues);
+			sk_tx_queue_set(sk, queue_index);
+		}
 	} else {
-		const struct net_device_ops *ops = dev->netdev_ops;
+		u16 (*select_queue)(struct net_device *, struct sk_buff *);

-		if (ops->ndo_select_queue) {
-			queue_index = ops->ndo_select_queue(dev, skb);
-		} else {
+		if (real_num_tx_queues == 1) {
 			queue_index = 0;
-			if (dev->real_num_tx_queues > 1)
-				queue_index = skb_tx_hash(dev, skb);
-
-			if (sk && sk->sk_dst_cache)
-				sk_tx_queue_set(sk, queue_index);
+		} else if((select_queue = dev->netdev_ops->ndo_select_queue)) {
+			queue_index = select_queue(dev, skb);
+			if (unlikely(queue_index >= real_num_tx_queues)) {
+				if (net_ratelimit())
+					WARN(1, "%s selects TX queue %d, but "
+					     "real number of TX queues is %d\n",
+					     dev->name, queue_index,
+					     real_num_tx_queues);
+				queue_index = 0;
+			}
+		} else {
+			queue_index = skb_tx_hash(dev, skb);
 		}
+
+		if (sk && sk->sk_dst_cache)
+			sk_tx_queue_set(sk, queue_index);
 	}

 	skb_set_queue_mapping(skb, queue_index);

^ permalink raw reply related

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Herbert Xu @ 2009-11-14  1:12 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Michael S. Tsirkin, alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <4AF98A8C.9040201@novell.com>

On Tue, Nov 10, 2009 at 10:45:16AM -0500, Gregory Haskins wrote:
>
> What I am getting at is as follows:  From a real basic perspective, you
> can look at all of this as a simple synchronous call (i.e. sendmsg()).
> The "app" (be it a userspace app, or a guest) prepares a buffer for
> transmission, and offers it to the next layer in the stack.  The app
> must maintain the integrity of that buffer at least until the layer
> below it signifies that it is "consumed".  This may mean its a
> synchronous call, like sendmsg(), or it may be asynchronous, like AIO.

Neither sendmsg() nor sendfile() is synchronous in the way you
imagine.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [RFC] vlan: GRO rx statistics
From: Herbert Xu @ 2009-11-14  1:27 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AFD8C31.10703@gmail.com>

On Fri, Nov 13, 2009 at 05:41:21PM +0100, Eric Dumazet wrote:
>
> Replying to myself, I found root of the problem and posted a fix.

Just curious, do you have a link to that fix?

> Still the race while updating dev->stats.tx_{bytes|packets} should be addressed eventually...

Well making it per-napi_struct should do the trick.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [RFC PATCH] net: add dataref destructor to sk_buff
From: Gregory Haskins @ 2009-11-14  1:33 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
	linux-kernel, netdev
In-Reply-To: <20091114011229.GA18580@gondor.apana.org.au>

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

Herbert Xu wrote:
> On Tue, Nov 10, 2009 at 10:45:16AM -0500, Gregory Haskins wrote:
>> What I am getting at is as follows:  From a real basic perspective, you
>> can look at all of this as a simple synchronous call (i.e. sendmsg()).
>> The "app" (be it a userspace app, or a guest) prepares a buffer for
>> transmission, and offers it to the next layer in the stack.  The app
>> must maintain the integrity of that buffer at least until the layer
>> below it signifies that it is "consumed".  This may mean its a
>> synchronous call, like sendmsg(), or it may be asynchronous, like AIO.
> 
> Neither sendmsg() nor sendfile() is synchronous in the way you
> imagine.

Well, not with respect to the overall protocol, of course not.  But with
respect to the buffer in question, it _has_ to be.  Or am I missing
something?

Kind Regards,
-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]

^ permalink raw reply

* [PATCH NEXT] netxen: update MAINTAINERS
From: Amit Kumar Salecha @ 2009-11-14  1:47 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay, Amit Kumar Salecha

Changing MAINTAINERS for netxen nic driver.

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f2f29c..e8f0a8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3704,7 +3704,7 @@ F:	include/linux/if_*
 F:	include/linux/*device.h
 
 NETXEN (1/10) GbE SUPPORT
-M:	Dhananjay Phadke <dhananjay@netxen.com>
+M:	Amit Kumar Salecha <amit.salecha@qlogic.com>
 L:	netdev@vger.kernel.org
 W:	http://www.netxen.com
 S:	Supported
-- 
1.6.0.2


^ permalink raw reply related

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Herbert Xu @ 2009-11-14  2:21 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
	linux-kernel, netdev
In-Reply-To: <4AFE08EF.2030308@gmail.com>

On Fri, Nov 13, 2009 at 08:33:35PM -0500, Gregory Haskins wrote:
>
> Well, not with respect to the overall protocol, of course not.  But with
> respect to the buffer in question, it _has_ to be.  Or am I missing
> something?

sendfile() has never guaranteed that the kernel is finished with
the underlying pages when it returns.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [RFC PATCH] net: add dataref destructor to sk_buff
From: Gregory Haskins @ 2009-11-14  2:27 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
	linux-kernel, netdev
In-Reply-To: <20091114022103.GA19020@gondor.apana.org.au>

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

Herbert Xu wrote:
> On Fri, Nov 13, 2009 at 08:33:35PM -0500, Gregory Haskins wrote:
>> Well, not with respect to the overall protocol, of course not.  But with
>> respect to the buffer in question, it _has_ to be.  Or am I missing
>> something?
> 
> sendfile() has never guaranteed that the kernel is finished with
> the underlying pages when it returns.
> 
> Cheers,

Clearly there must be _some_ mechanism to synchronize (e.g.
flush/barrier) though, right?  Otherwise, that interface would seem to
be quite prone to races and would likely be unusable.   So what does
said flush use to know when the buffer is free?

-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]

^ permalink raw reply

* Re: [PATCH NEXT] netxen: update MAINTAINERS
From: Joe Perches @ 2009-11-14  2:29 UTC (permalink / raw)
  To: Amit Kumar Salecha; +Cc: davem, netdev, dhananjay
In-Reply-To: <1258163229-25088-1-git-send-email-amit.salecha@qlogic.com>

On Fri, 2009-11-13 at 17:47 -0800, Amit Kumar Salecha wrote:
> Changing MAINTAINERS for netxen nic driver.
> 
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
>  MAINTAINERS |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f2f29c..e8f0a8a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3704,7 +3704,7 @@ F:	include/linux/if_*
>  F:	include/linux/*device.h
>  
>  NETXEN (1/10) GbE SUPPORT
> -M:	Dhananjay Phadke <dhananjay@netxen.com>
> +M:	Amit Kumar Salecha <amit.salecha@qlogic.com>
>  L:	netdev@vger.kernel.org
>  W:	http://www.netxen.com
>  S:	Supported

Maybe change the W: as well
W:	http://www.qlogic.com



^ permalink raw reply

* [PATCH NEXT] netxen: update MAINTAINERS
From: Amit Kumar Salecha @ 2009-11-14  2:37 UTC (permalink / raw)
  To: davem; +Cc: netdev, dhananjay, Amit Kumar Salecha

Changing MAINTAINERS for netxen nic driver.

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 MAINTAINERS |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f2f29c..fe1cd41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3704,9 +3704,9 @@ F:	include/linux/if_*
 F:	include/linux/*device.h
 
 NETXEN (1/10) GbE SUPPORT
-M:	Dhananjay Phadke <dhananjay@netxen.com>
+M:	Amit Kumar Salecha <amit.salecha@qlogic.com>
 L:	netdev@vger.kernel.org
-W:	http://www.netxen.com
+W:	http://www.qlogic.com
 S:	Supported
 F:	drivers/net/netxen/
 
-- 
1.6.0.2


^ permalink raw reply related

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Herbert Xu @ 2009-11-14  2:43 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
	linux-kernel, netdev
In-Reply-To: <4AFE15AD.6000208@gmail.com>

On Fri, Nov 13, 2009 at 09:27:57PM -0500, Gregory Haskins wrote:
>
> Clearly there must be _some_ mechanism to synchronize (e.g.
> flush/barrier) though, right?  Otherwise, that interface would seem to
> be quite prone to races and would likely be unusable.   So what does
> said flush use to know when the buffer is free?

It is up to the application to devise its own sync mechanism.
For TCP, you may query the kernel for the last sequence number
acked by the other side.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [RFC PATCH] net: add dataref destructor to sk_buff
From: Stephen Hemminger @ 2009-11-14  2:45 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Herbert Xu, Gregory Haskins, Michael S. Tsirkin, alacrityvm-devel,
	linux-kernel, netdev
In-Reply-To: <4AFE15AD.6000208@gmail.com>

On Fri, 13 Nov 2009 21:27:57 -0500
Gregory Haskins <gregory.haskins@gmail.com> wrote:

> Herbert Xu wrote:
> > On Fri, Nov 13, 2009 at 08:33:35PM -0500, Gregory Haskins wrote:
> >> Well, not with respect to the overall protocol, of course not.  But with
> >> respect to the buffer in question, it _has_ to be.  Or am I missing
> >> something?
> > 
> > sendfile() has never guaranteed that the kernel is finished with
> > the underlying pages when it returns.
> > 
> > Cheers,
> 
> Clearly there must be _some_ mechanism to synchronize (e.g.
> flush/barrier) though, right?  Otherwise, that interface would seem to
> be quite prone to races and would likely be unusable.   So what does
> said flush use to know when the buffer is free?

No all the interfaces require a copy. Actually, sendfile makes no guarantee about synchronization
because the receiver of said file could be arbitrarily slow, and any attempt at locking down
current contents of file is a denial of service exposure.

People have tried doing copy-less send by page flipping, but the overhead of the IPI to
invalidate the TLB exceeded the overhead of the copy. There was an Intel paper on this in
at Linux Symposium (Ottawa) several years ago.

^ permalink raw reply

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Herbert Xu @ 2009-11-14  2:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Gregory Haskins, Gregory Haskins, Michael S. Tsirkin,
	alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <20091113184503.13f6d447@s6510>

On Fri, Nov 13, 2009 at 06:45:03PM -0800, Stephen Hemminger wrote:
> 
> No all the interfaces require a copy. Actually, sendfile makes no guarantee about synchronization

Actually sendfile does not require a copy.

> because the receiver of said file could be arbitrarily slow, and any attempt at locking down
> current contents of file is a denial of service exposure.

Agreed.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: David Miller @ 2009-11-14  2:59 UTC (permalink / raw)
  To: bcrl; +Cc: shemminger, opurdila, eric.dumazet, netdev
In-Reply-To: <20091113235210.GR19478@kvack.org>

From: Benjamin LaHaise <bcrl@lhnet.ca>
Date: Fri, 13 Nov 2009 18:52:10 -0500

> If you don't want the overhead from this kind of scaling, stick it under a 
> config option, but please don't stop other people from pushing Linux into 
> new uses which have these scaling requirements.

This 'scaling requirement' only exists in environments where people
undersubsribe their networks, right?

I'm not saying we won't put scaling into these areas, I'm just trying
to make a point to show that this "need" only exists because people
have purposefully created these situations where they feel the need to
massively control their users usage in order to generate revenue.

^ permalink raw reply

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: David Miller @ 2009-11-14  3:04 UTC (permalink / raw)
  To: gregory.haskins
  Cc: herbert, ghaskins, mst, alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <4AFE08EF.2030308@gmail.com>

From: Gregory Haskins <gregory.haskins@gmail.com>
Date: Fri, 13 Nov 2009 20:33:35 -0500

> Well, not with respect to the overall protocol, of course not.  But with
> respect to the buffer in question, it _has_ to be.  Or am I missing
> something?

sendfile() absolutely, and positively, is not.

Any entity can write to the pages being send via sendfile(), at will,
and those writes will show up in the packet stream if they occur
before the NIC DMA's the memory backed by those pages into it's
buffer.

There is zero data synchronization whatsoever, we don't lock the
pages, we don't block their usage while they are queued up in the
socket send queue, nothing like that.

The user returns long before it every hits the wire and there is zero
"notification" to the user that the pages in question for the
sendfile() request are no longer in use.

It seems that your understanding of how buffering and synchronization
works in the TCP stack has come out of a fairy tale :-)

^ permalink raw reply

* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: David Miller @ 2009-11-14  3:09 UTC (permalink / raw)
  To: gregory.haskins
  Cc: herbert, ghaskins, mst, alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <4AFE15AD.6000208@gmail.com>

From: Gregory Haskins <gregory.haskins@gmail.com>
Date: Fri, 13 Nov 2009 21:27:57 -0500

> Clearly there must be _some_ mechanism to synchronize (e.g.
> flush/barrier) though, right?  Otherwise, that interface would seem to
> be quite prone to races and would likely be unusable.   So what does
> said flush use to know when the buffer is free?

There is no such synchronization at all.

If some synchronization is required, it must be done at a higher
level.

For example, SAMBA can only use sendfile() to serve file contents when
the client holds an SMB "OP Lock" on the file.  Luckily, by default
pretty much all SMB client implementations hold the OP Lock on a file
even just to read it (this is so that self-modifying autoexec.bat
scripts work :-)

But frankly most sendfile() consumers don't care, and the only thing
that really matters is that the checksum on the final TCP packet that
goes out is correct, and since we require card checksums to sendfile()
without copying, that is taken care of transparently.

^ permalink raw reply

* Re: [PATCH] s2io: fixing a ethtool test that is broken
From: David Miller @ 2009-11-14  3:36 UTC (permalink / raw)
  To: Sivakumar.Subramani; +Cc: leitao, netdev, Sreenivasa.Honnur, Ramkrishna.Vepa
In-Reply-To: <78C9135A3D2ECE4B8162EBDCE82CAD7705E74E6F@nekter>

From: "Sivakumar Subramani" <Sivakumar.Subramani@neterion.com>
Date: Thu, 12 Nov 2009 13:59:53 -0500

> [Siva] Reviewed the patch. Please accept it.
> 
> Acked-by: Sivakumar Subramani <sivakumar.subramani@neterion.com>

Please use reasonable mechanisms to reply to patches when you review
them.  Something in your mail client software or elsewhere has messed
with the Message-Id and other elements of the email headers, and
therefore the threading of messages so that patchwork and other tools
can attach your reply to the patch posting itself simply do not work.

This makes more work for me and I am not going to go searching for
your replies by hand.

So you should fix this because otherwise your review reply ACKs
will get lost.

Also you have this very non-standard way to attributing your content
by adding these "[Name]" tags.  Please don't do that, it is
non-standard and therefore most people don't understand it at all.

The normal email reply quoting mechanisms of your email client will
let us know exactly what parts are written by you, and what parts are
said by other people in quoted text.

Thank you.

^ permalink raw reply

* Re: [PATCH]NET:PCNET32: poll method return 0 when done
From: David Miller @ 2009-11-14  3:39 UTC (permalink / raw)
  To: pcnet32; +Cc: figo1802, netdev
In-Reply-To: <1258042401.7727.3.camel@localhost.localdomain>

From: Don Fry <pcnet32@verizon.net>
Date: Thu, 12 Nov 2009 08:13:21 -0800

> I do not understand why you are suggesting this change.  With NAPI
> the amount of work done is returned on exit, not zero.  All other
> drivers I have looked at do not force a zero on exit.
> 
> NAK

Right, this change is completely wrong.

^ permalink raw reply

* Re: [PATCH] ppp: fix BUG on non-linear SKB (multilink receive)
From: David Miller @ 2009-11-14  3:46 UTC (permalink / raw)
  To: ben; +Cc: paulus, netdev, linux-ppp
In-Reply-To: <Pine.LNX.4.58.0911121144380.25398@benxen>

From: Ben McKeegan <ben@netservers.co.uk>
Date: Thu, 12 Nov 2009 13:09:57 +0000 (GMT)

> @@ -1944,7 +1944,13 @@ ppp_receive_mp_frame(struct ppp *ppp, st
> 
>  	/* Pull completed packets off the queue and receive them. */
>  	while ((skb = ppp_mp_reconstruct(ppp)))
> -		ppp_receive_nonmp_frame(ppp, skb);
> +	  	if (pskb_may_pull(skb, 2))
> +			ppp_receive_nonmp_frame(ppp, skb);
> +		else {
> +			++ppp->dev->stats.rx_length_errors;
> +			kfree_skb(skb);
> +			ppp_receive_error(ppp);
> +		}
> 
>  	return;

This fix looks correct, but could you please enclose the while()
loop in braces, that dangling else and the subsequent "return;"
statement look confusing otherwise.

Thanks.

^ permalink raw reply

* Re: [PATCH] forcedeth: mac address fix
From: David Miller @ 2009-11-14  3:52 UTC (permalink / raw)
  To: stas; +Cc: aabdulla, yinghai, netdev, linux-kernel
In-Reply-To: <1258114975-5715-1-git-send-email-stas@lvk.cs.msu.su>

From: "Stanislav O\. Bezzubtsev" <stas@lvk.cs.msu.su>
Date: Fri, 13 Nov 2009 15:22:55 +0300

> Set second bit of randomly generated mac. That marks MAC
> as locally assigned. IEEE802.3, section one, 3.2.3.
> 
> Signed-off-by: Stanislav O. Bezzubtsev <stas@lvk.cs.msu.su>

Do you know why this driver even has this bug?

It's because it does not use random_ether_addr() and tries to
do it by hand all by itself.

Please fix this bug for real by having the driver use the proper
interface to calculate a randomized ethernet address.

Thank you.

^ permalink raw reply

* Re: [PATCH] sctp: Set source addresses on the association before adding transports
From: David Miller @ 2009-11-14  3:57 UTC (permalink / raw)
  To: vladislav.yasevich; +Cc: netdev, linux-sctp
In-Reply-To: <1257879454-31661-1-git-send-email-vladislav.yasevich@hp.com>

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 10 Nov 2009 13:57:34 -0500

> Recent commit 8da645e101a8c20c6073efda3c7cc74eec01b87f
> 	sctp: Get rid of an extra routing lookup when adding a transport
> introduced a regression in the connection setup.  The behavior was
> different between IPv4 and IPv6.  IPv4 case ended up working because the
> route lookup routing returned a NULL route, which triggered another
> route lookup later in the output patch that succeeded.  In the IPv6 case,
> a valid route was returned for first call, but we could not find a valid
> source address at the time since the source addresses were not set on the
> association yet.  Thus resulted in a hung connection.
> 
> The solution is to set the source addresses on the association prior to
> adding peers.
> 
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

^ permalink raw reply

* Re: [PATCH] sctp: Fix regression introduced by new sctp_connectx api
From: David Miller @ 2009-11-14  3:57 UTC (permalink / raw)
  To: vladislav.yasevich; +Cc: netdev, linux-sctp
In-Reply-To: <1257963564-26376-1-git-send-email-vladislav.yasevich@hp.com>

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 11 Nov 2009 13:19:24 -0500

> A new (unrealeased to the user) sctp_connectx api
> 
> c6ba68a26645dbc5029a9faa5687ebe6fcfc53e4
>     sctp: support non-blocking version of the new sctp_connectx() API
> 
> introduced a regression cought by the user regression test
> suite.  In particular, the API requires the user library to
> re-allocate the buffer and could potentially trigger a SIGFAULT.
> 
> This change corrects that regression by passing the original
> address buffer to the kernel unmodified, but still allows for
> a returned association id.
> 
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

^ permalink raw reply

* Re: [PATCH] sctp: Set socket source address when additing first transport
From: David Miller @ 2009-11-14  3:57 UTC (permalink / raw)
  To: vladislav.yasevich; +Cc: netdev, linux-sctp
In-Reply-To: <1257976477-31326-1-git-send-email-vladislav.yasevich@hp.com>

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 11 Nov 2009 16:54:37 -0500

> Recent commits
> 	sctp: Get rid of an extra routing lookup when adding a transport
> and
> 	sctp: Set source addresses on the association before adding transports
> 
> changed when routes are added to the sctp transports.  As such,
> we didn't set the socket source address correctly when adding the first
> transport.  The first transport is always the primary/active one, so
> when adding it, set the socket source address.  This was causing
> regression failures in SCTP tests.
> 
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

^ permalink raw reply

* Re: [PATCH] s2io: fixing a ethtool test that is broken
From: David Miller @ 2009-11-14  3:57 UTC (permalink / raw)
  To: leitao; +Cc: netdev, sreenivasa.honnur
In-Reply-To: <1257882263-7507-1-git-send-email-leitao@linux.vnet.ibm.com>

From: leitao@linux.vnet.ibm.com
Date: Tue, 10 Nov 2009 14:44:23 -0500

> Due commit 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff, it is not more
> possible to pci_restore_state() more than once without calling
> pci_save_state() in the middle.
> 
> Actually running a ethtool test on s2io makes the card inactive, 
> and it needs to unload/reload the module to fix. 
> 
> This patch just save the state just after it restore in order to
> keep the old behaviour
> 
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>

Applied.

^ 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