Netdev List
 help / color / mirror / Atom feed
* 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: [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: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

* [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: [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

* 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: [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

* [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: 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

* 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: 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

* [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: [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

* 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:04 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev
In-Reply-To: <200911130720.19671.opurdila@ixiacom.com>

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, ...


-- 

^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Benjamin LaHaise @ 2009-11-13 23:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Octavian Purdila, Eric Dumazet, netdev
In-Reply-To: <20091113153924.6130135f@nehalam>

On Fri, Nov 13, 2009 at 03:39:24PM -0800, Stephen Hemminger wrote:
> Well TCP handles lots of connections, but a socket has different overhead
> than a network device. Why should 10Gbps need 10K PPPoE sessions?
> Even Vlan's are less overhead than PPP

PPP's overhead is acceptable.  It makes managing networks a lot easier, since 
the authentication done by PPP is able to look up any end user specific 
information required (ie static ips and routes), while the access part of 
the network is a fairly generic config that uses switchs and things like the 
GVRP.  Without that, the configuration of any aggregation switch becomes a 
huge management nightmare.

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.

		-ben

^ permalink raw reply

* Re: [PATCH net-next-2.6] bonding: fix 802.3ad standards compliance error
From: Jay Vosburgh @ 2009-11-13 23:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, David S. Miller, martin
In-Reply-To: <20091113153058.4b739ea3@nehalam>

Stephen Hemminger <shemminger@vyatta.com> wrote:

>On Fri, 13 Nov 2009 15:13:01 -0800
>Jay Vosburgh <fubar@us.ibm.com> wrote:
>
>> +	// check if all parameters are alike
>> +	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
>> +	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
>> +	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
>> +	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
>> +	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
>> +	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
>> +	    // or this is individual link(aggregation == FALSE)
>
>I know the code here doesn't follow kernel style, but please don't
>add more C99 comments or overly long lines.

	I didn't, really; I moved the function up in the file rather
than adding a forward declaration.

	-J

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

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 23:42 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jarek Poplawski, Eric Dumazet, David S. Miller, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <20091113153229.408c889f@nehalam>

On Sat, Nov 14, 2009 at 7:32 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Sat, 14 Nov 2009 07:28:42 +0800
> Changli Gao <xiaosuo@gmail.com> wrote:
>
>> On Sat, Nov 14, 2009 at 12:15 AM, Stephen Hemminger
>>
>> It needs to send remote SoftIRQ, as Receiving Packet Steering, and we
>> must support a extra interface to map load to CPUs.
>>
>
> But it still could use NAPI to avoid causing excess packet overhead.
> The softirq could be equivalent of hardirq in network device.
>

Yea, the overhead of SoftIRQ is less than kernel threads, and I'll try
to find a way to solve both flexibility and efficiency. Maybe I need
some real NIC drivers as examples. Is there a standard API to bind RQs
of NIC to CPUs, such as ioctl or setsockopt?


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Stephen Hemminger @ 2009-11-13 23:39 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: Octavian Purdila, Eric Dumazet, netdev
In-Reply-To: <20091113233504.GQ19478@kvack.org>

On Fri, 13 Nov 2009 18:35:04 -0500
Benjamin LaHaise <bcrl@lhnet.ca> wrote:

> On Fri, Nov 13, 2009 at 02:49:37PM -0800, Stephen Hemminger wrote:
> > Then maybe network devices aren't the right layering model. At some
> > point the paradigm has to be re-examined.
> 
> What is the right model for dealing with lots of connections to users and 
> routes?  This problem isn't going to go away given the increases in 
> connectivity and processing power that happen each year.  Today, software 
> routing of 10Gbps links is a reality -- part of what comes with that ability 
> of hardware is the need to deal with the fact that 10Gbps aggregates a lot 
> of users.
> 
> 		-ben

Well TCP handles lots of connections, but a socket has different overhead
than a network device. Why should 10Gbps need 10K PPPoE sessions?
Even Vlan's are less overhead than PPP

-- 

^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: fast consecutive name allocation
From: Benjamin LaHaise @ 2009-11-13 23:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Octavian Purdila, Eric Dumazet, netdev
In-Reply-To: <20091113144937.23693bb4@nehalam>

On Fri, Nov 13, 2009 at 02:49:37PM -0800, Stephen Hemminger wrote:
> Then maybe network devices aren't the right layering model. At some
> point the paradigm has to be re-examined.

What is the right model for dealing with lots of connections to users and 
routes?  This problem isn't going to go away given the increases in 
connectivity and processing power that happen each year.  Today, software 
routing of 10Gbps links is a reality -- part of what comes with that ability 
of hardware is the need to deal with the fact that 10Gbps aggregates a lot 
of users.

		-ben

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Stephen Hemminger @ 2009-11-13 23:32 UTC (permalink / raw)
  To: Changli Gao
  Cc: Jarek Poplawski, Eric Dumazet, David S. Miller, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <412e6f7f0911131528i3105648y49213143e12f2522@mail.gmail.com>

On Sat, 14 Nov 2009 07:28:42 +0800
Changli Gao <xiaosuo@gmail.com> wrote:

> On Sat, Nov 14, 2009 at 12:15 AM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
> > On Fri, 13 Nov 2009 17:38:56 +0800
> > Changli Gao <xiaosuo@gmail.com> wrote:
> >>
> >> Oh, :) . I know more than one companies use kernel threads to forward
> >> packets, and there isn't explicit extra overhead at all. And as you
> >> know, as throughput increases, NAPI will bind the NIC to a CPU, and
> >> softirqd will be waked up to do the work, which should be done in
> >> SoftIRQ context. At that time, there isn't any difference between my
> >> approach and the current kernel's.
> >>
> > Why not make IFB a NAPI device. This would get rid of the extra soft-irq
> > round trip from going through netif_rx().  It would also behave like
> > regular multi-queue recieive device, and eliminate need for seperate
> > tasklets or threads.
> >
> 
> It needs to send remote SoftIRQ, as Receiving Packet Steering, and we
> must support a extra interface to map load to CPUs.
> 

But it still could use NAPI to avoid causing excess packet overhead.
The softirq could be equivalent of hardirq in network device.

-- 

^ permalink raw reply

* Re: [PATCH net-next-2.6] bonding: fix 802.3ad standards compliance error
From: Stephen Hemminger @ 2009-11-13 23:30 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: netdev, David S. Miller, martin
In-Reply-To: <3048.1258153981@death.nxdomain.ibm.com>

On Fri, 13 Nov 2009 15:13:01 -0800
Jay Vosburgh <fubar@us.ibm.com> wrote:

> +	// check if all parameters are alike
> +	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
> +	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
> +	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
> +	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
> +	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
> +	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
> +	    // or this is individual link(aggregation == FALSE)

I know the code here doesn't follow kernel style, but please don't
add more C99 comments or overly long lines.

-- 

^ permalink raw reply

* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 23:28 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jarek Poplawski, Eric Dumazet, David S. Miller, Patrick McHardy,
	Tom Herbert, netdev
In-Reply-To: <20091113081553.0568296c@s6510>

On Sat, Nov 14, 2009 at 12:15 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 13 Nov 2009 17:38:56 +0800
> Changli Gao <xiaosuo@gmail.com> wrote:
>>
>> Oh, :) . I know more than one companies use kernel threads to forward
>> packets, and there isn't explicit extra overhead at all. And as you
>> know, as throughput increases, NAPI will bind the NIC to a CPU, and
>> softirqd will be waked up to do the work, which should be done in
>> SoftIRQ context. At that time, there isn't any difference between my
>> approach and the current kernel's.
>>
> Why not make IFB a NAPI device. This would get rid of the extra soft-irq
> round trip from going through netif_rx().  It would also behave like
> regular multi-queue recieive device, and eliminate need for seperate
> tasklets or threads.
>

It needs to send remote SoftIRQ, as Receiving Packet Steering, and we
must support a extra interface to map load to CPUs.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* [PATCH net-next-2.6] bonding: fix 802.3ad standards compliance error
From: Jay Vosburgh @ 2009-11-13 23:13 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, martin


	The language of 802.3ad 43.4.9 requires the "recordPDU" function
to, in part, compare the Partner parameter values in a received LACPDU
to the stored Actor values.  If those match, then the Partner's
synchronization state is set to true.

	The current 802.3ad implementation is performing these steps out
of order; first, the synchronization check is done, then the paramters are
checked to see if they match (the synch check being done against a match
check of a prior LACPDU).  This causes delays in establishing aggregators
in some circumstances.

	This patch modifies the 802.3ad code to call __choose_matched,
the function that does the "match" comparisions, as the first step of
__record_pdu, instead of immediately afterwards.  This new behavior is
in compliance with the language of the standard.

	Some additional commentary relating to code vs. standard is also
added.

	Reported by Martin Patterson <martin@gear6.com> who also supplied
the logic of the fix and verified the patch.

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 drivers/net/bonding/bond_3ad.c |   85 ++++++++++++++++++++--------------------
 1 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 1d05819..88c3fe8 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -446,6 +446,48 @@ static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 /////////////////////////////////////////////////////////////////////////////////
 
 /**
+ * __choose_matched - update a port's matched variable from a received lacpdu
+ * @lacpdu: the lacpdu we've received
+ * @port: the port we're looking at
+ *
+ * Update the value of the matched variable, using parameter values from a
+ * newly received lacpdu. Parameter values for the partner carried in the
+ * received PDU are compared with the corresponding operational parameter
+ * values for the actor. Matched is set to TRUE if all of these parameters
+ * match and the PDU parameter partner_state.aggregation has the same value as
+ * actor_oper_port_state.aggregation and lacp will actively maintain the link
+ * in the aggregation. Matched is also set to TRUE if the value of
+ * actor_state.aggregation in the received PDU is set to FALSE, i.e., indicates
+ * an individual link and lacp will actively maintain the link. Otherwise,
+ * matched is set to FALSE. LACP is considered to be actively maintaining the
+ * link if either the PDU's actor_state.lacp_activity variable is TRUE or both
+ * the actor's actor_oper_port_state.lacp_activity and the PDU's
+ * partner_state.lacp_activity variables are TRUE.
+ *
+ * Note: the AD_PORT_MATCHED "variable" is not specified by 802.3ad; it is
+ * used here to implement the language from 802.3ad 43.4.9 that requires
+ * recordPDU to "match" the LACPDU parameters to the stored values.
+ */
+static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
+{
+	// check if all parameters are alike
+	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
+	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
+	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
+	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
+	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
+	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
+	    // or this is individual link(aggregation == FALSE)
+	    ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)
+		) {
+		// update the state machine Matched variable
+		port->sm_vars |= AD_PORT_MATCHED;
+	} else {
+		port->sm_vars &= ~AD_PORT_MATCHED;
+	}
+}
+
+/**
  * __record_pdu - record parameters from a received lacpdu
  * @lacpdu: the lacpdu we've received
  * @port: the port we're looking at
@@ -459,6 +501,7 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 	if (lacpdu && port) {
 		struct port_params *partner = &port->partner_oper;
 
+		__choose_matched(lacpdu, port);
 		// record the new parameter values for the partner operational
 		partner->port_number = ntohs(lacpdu->actor_port);
 		partner->port_priority = ntohs(lacpdu->actor_port_priority);
@@ -563,47 +606,6 @@ static void __update_default_selected(struct port *port)
 }
 
 /**
- * __choose_matched - update a port's matched variable from a received lacpdu
- * @lacpdu: the lacpdu we've received
- * @port: the port we're looking at
- *
- * Update the value of the matched variable, using parameter values from a
- * newly received lacpdu. Parameter values for the partner carried in the
- * received PDU are compared with the corresponding operational parameter
- * values for the actor. Matched is set to TRUE if all of these parameters
- * match and the PDU parameter partner_state.aggregation has the same value as
- * actor_oper_port_state.aggregation and lacp will actively maintain the link
- * in the aggregation. Matched is also set to TRUE if the value of
- * actor_state.aggregation in the received PDU is set to FALSE, i.e., indicates
- * an individual link and lacp will actively maintain the link. Otherwise,
- * matched is set to FALSE. LACP is considered to be actively maintaining the
- * link if either the PDU's actor_state.lacp_activity variable is TRUE or both
- * the actor's actor_oper_port_state.lacp_activity and the PDU's
- * partner_state.lacp_activity variables are TRUE.
- */
-static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
-{
-	// validate lacpdu and port
-	if (lacpdu && port) {
-		// check if all parameters are alike
-		if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
-		     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
-		     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
-		     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
-		     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
-		     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
-		    // or this is individual link(aggregation == FALSE)
-		    ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)
-		   ) {
-			// update the state machine Matched variable
-			port->sm_vars |= AD_PORT_MATCHED;
-		} else {
-			port->sm_vars &= ~AD_PORT_MATCHED;
-		}
-	}
-}
-
-/**
  * __update_ntt - update a port's ntt variable from a received lacpdu
  * @lacpdu: the lacpdu we've received
  * @port: the port we're looking at
@@ -1134,7 +1136,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			__update_selected(lacpdu, port);
 			__update_ntt(lacpdu, port);
 			__record_pdu(lacpdu, port);
-			__choose_matched(lacpdu, port);
 			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT));
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			// verify that if the aggregator is enabled, the port is enabled too.
-- 
1.5.4.5


^ permalink raw reply related

* Re: [PATCH 0/3] macvlan: support for guest vm direct rx/tx
From: Patrick Mullaney @ 2009-11-13 23:05 UTC (permalink / raw)
  To: shemminger; +Cc: arnd, netdev, bridge, linux-kernel, alacrityvm-devel

On Fri, 2009-11-13 at 13:27 -0800, Stephen Hemminger wrote:
> On Fri, 13 Nov 2009 14:55:06 -0500
> Patrick Mullaney <pmullaney@novell.com> wrote:
> 
> > These patches allow other modules to override the receive
> > path of a macvlan. This is being done to support guest
> > VMs operating directly over a macvlan. Routines to allow
> > creation and deletion of macvlans from in-kernel modules
> > were also exposed/added.
> 
> Which guest VM, how will it use it? The kernel is not in the business
> of providing infrastructure for out of tree patches.

Actually, any guest vm or container. macvtap was the first to suggest
a patch like this to provide this functionality. This infrastructure
is generic enough to allow others to use it. My interest is in
supporting kvm guests via vbus drivers which are being integrated
in the alacrityvm tree.

> 
> Also, macvlan should really being calling netif_receive_skb()
> not going through another queue/softirq cycle.
I understand but you are talking about the current behavior of
macvlans - my goal wasn't to change the current behavior
of macvlans, as I didn't want to disturb what the original author
intended, just provide the ability to override the rx path and
provide for management of macvlans from other kernel modules(not
just via rtnl).

^ 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