[patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[David Kimdon]

[-- Attachment #1: remove_d80211_fifo_qdisc.patch --]
[-- Type: text/plain, Size: 4040 bytes --]

wme.c needs a generic fifo qdisc for each hardware queue.  Switch 
wme.c to use the generic fifo qdisc in net/sched/sch_fifo.c.  This allows
removal of net/d80211/fifo_qdisc.c which isn't particularily tied to
IEEE 802.11 in any way.

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: wireless-dev/net/d80211/Makefile
===================================================================
--- wireless-dev.orig/net/d80211/Makefile
+++ wireless-dev/net/d80211/Makefile
@@ -21,6 +21,3 @@ obj-$(CONFIG_D80211) += 80211.o rc80211_
 	wme.o \
 	$(80211-objs-y)
 
-ifeq ($(CONFIG_NET_SCHED),)
-  80211-objs += fifo_qdisc.o
-endif
Index: wireless-dev/net/d80211/wme.c
===================================================================
--- wireless-dev.orig/net/d80211/wme.c
+++ wireless-dev/net/d80211/wme.c
@@ -18,8 +18,6 @@
 #include "ieee80211_i.h"
 #include "wme.h"
 
-#define CHILD_QDISC_OPS pfifo_qdisc_ops
-
 static inline int WLAN_FC_IS_QOS_DATA(u16 fc)
 {
 	return (fc & 0x8C) == 0x88;
@@ -433,7 +431,7 @@ static int wme_qdiscop_init(struct Qdisc
 	/* create child queues */
 	for (i = 0; i < queues; i++) {
 		skb_queue_head_init(&q->requeued[i]);
-		q->queues[i] = qdisc_create_dflt(qd->dev, &CHILD_QDISC_OPS);
+		q->queues[i] = qdisc_create_dflt(qd->dev, &pfifo_qdisc_ops);
 		if (q->queues[i] == 0) {
 			q->queues[i] = &noop_qdisc;
 			printk(KERN_ERR "%s child qdisc %i creation failed", dev->name, i);
Index: wireless-dev/net/d80211/fifo_qdisc.c
===================================================================
--- wireless-dev.orig/net/d80211/fifo_qdisc.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright 2005, Devicescape Software, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * If building without CONFIG_NET_SCHED we need a simple
- * fifo qdisc to install by default as the sub-qdisc.
- * This is a simple replacement for sch_fifo.
- */
-
-#include <linux/skbuff.h>
-#include <net/pkt_sched.h>
-#include <net/d80211.h>
-#include "ieee80211_i.h"
-#include "wme.h"
-
-static int pfifo_enqueue(struct sk_buff *skb, struct Qdisc* qd)
-{
-	struct sk_buff_head *q = qdisc_priv(qd);
-
-	if (skb_queue_len(q) > qd->dev->tx_queue_len) {
-		qd->qstats.drops++;
-		kfree_skb(skb);
-		return NET_XMIT_DROP;
-	}
-
-	skb_queue_tail(q, skb);
-	qd->q.qlen++;
-	qd->bstats.bytes += skb->len;
-	qd->bstats.packets++;
-
-	return NET_XMIT_SUCCESS;
-}
-
-
-static int pfifo_requeue(struct sk_buff *skb, struct Qdisc* qd)
-{
-	struct sk_buff_head *q = qdisc_priv(qd);
-
-	skb_queue_head(q, skb);
-	qd->q.qlen++;
-	qd->bstats.bytes += skb->len;
-	qd->bstats.packets++;
-
-	return NET_XMIT_SUCCESS;
-}
-
-
-static struct sk_buff *pfifo_dequeue(struct Qdisc* qd)
-{
-	struct sk_buff_head *q = qdisc_priv(qd);
-
-	return skb_dequeue(q);
-}
-
-
-static int pfifo_init(struct Qdisc* qd, struct rtattr *opt)
-{
-	struct sk_buff_head *q = qdisc_priv(qd);
-
-	skb_queue_head_init(q);
-	return 0;
-}
-
-
-static void pfifo_reset(struct Qdisc* qd)
-{
-	struct sk_buff_head *q = qdisc_priv(qd);
-
-	skb_queue_purge(q);
-	qd->q.qlen = 0;
-}
-
-
-static int pfifo_dump(struct Qdisc *qd, struct sk_buff *skb)
-{
-	return skb->len;
-}
-
-
-struct Qdisc_ops pfifo_qdisc_ops =
-{
-	.next = NULL,
-	.cl_ops = NULL,
-	.id = "ieee80211_pfifo",
-	.priv_size = sizeof(struct sk_buff_head),
-
-	.enqueue = pfifo_enqueue,
-	.dequeue = pfifo_dequeue,
-	.requeue = pfifo_requeue,
-	.drop = NULL,
-
-	.init = pfifo_init,
-	.reset = pfifo_reset,
-	.destroy = NULL,
-	.change = NULL,
-
-	.dump = pfifo_dump,
-};
-
Index: wireless-dev/net/d80211/Kconfig
===================================================================
--- wireless-dev.orig/net/d80211/Kconfig
+++ wireless-dev/net/d80211/Kconfig
@@ -3,6 +3,7 @@ config D80211
 	select CRYPTO
 	select CRYPTO_ARC4
 	select CRYPTO_AES
+	select NET_SCHED
 	---help---
 	This option enables the hardware independent IEEE 802.11
 	networking stack.

--

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Patrick McHardy]

David Kimdon wrote:
> wme.c needs a generic fifo qdisc for each hardware queue.  Switch 
> wme.c to use the generic fifo qdisc in net/sched/sch_fifo.c.  This allows
> removal of net/d80211/fifo_qdisc.c which isn't particularily tied to
> IEEE 802.11 in any way.
> 
> -#define CHILD_QDISC_OPS pfifo_qdisc_ops
> -
>  static inline int WLAN_FC_IS_QOS_DATA(u16 fc)
>  {
>  	return (fc & 0x8C) == 0x88;
> @@ -433,7 +431,7 @@ static int wme_qdiscop_init(struct Qdisc
>  	/* create child queues */
>  	for (i = 0; i < queues; i++) {
>  		skb_queue_head_init(&q->requeued[i]);
> -		q->queues[i] = qdisc_create_dflt(qd->dev, &CHILD_QDISC_OPS);
> +		q->queues[i] = qdisc_create_dflt(qd->dev, &pfifo_qdisc_ops);
>  		if (q->queues[i] == 0) {
>  			q->queues[i] = &noop_qdisc;
>  			printk(KERN_ERR "%s child qdisc %i creation failed", dev->name, i);
> Index: wireless-dev/net/d80211/Kconfig
> ===================================================================
> --- wireless-dev.orig/net/d80211/Kconfig
> +++ wireless-dev/net/d80211/Kconfig
> @@ -3,6 +3,7 @@ config D80211
>  	select CRYPTO
>  	select CRYPTO_ARC4
>  	select CRYPTO_AES
> +	select NET_SCHED


pfifo_fast is available even without CONFIG_NET_SCHED, maybe
thats a better choice to avoid unnecessary bloat.

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Patrick McHardy]

Patrick McHardy wrote:
> David Kimdon wrote:
> 
>>wme.c needs a generic fifo qdisc for each hardware queue.  Switch 
>>wme.c to use the generic fifo qdisc in net/sched/sch_fifo.c.  This allows
>>removal of net/d80211/fifo_qdisc.c which isn't particularily tied to
>>IEEE 802.11 in any way.
>>
>>-#define CHILD_QDISC_OPS pfifo_qdisc_ops
>>-
>> static inline int WLAN_FC_IS_QOS_DATA(u16 fc)
>> {
>> 	return (fc & 0x8C) == 0x88;
>>@@ -433,7 +431,7 @@ static int wme_qdiscop_init(struct Qdisc
>> 	/* create child queues */
>> 	for (i = 0; i < queues; i++) {
>> 		skb_queue_head_init(&q->requeued[i]);
>>-		q->queues[i] = qdisc_create_dflt(qd->dev, &CHILD_QDISC_OPS);
>>+		q->queues[i] = qdisc_create_dflt(qd->dev, &pfifo_qdisc_ops);
>> 		if (q->queues[i] == 0) {
>> 			q->queues[i] = &noop_qdisc;
>> 			printk(KERN_ERR "%s child qdisc %i creation failed", dev->name, i);
>>Index: wireless-dev/net/d80211/Kconfig
>>===================================================================
>>--- wireless-dev.orig/net/d80211/Kconfig
>>+++ wireless-dev/net/d80211/Kconfig
>>@@ -3,6 +3,7 @@ config D80211
>> 	select CRYPTO
>> 	select CRYPTO_ARC4
>> 	select CRYPTO_AES
>>+	select NET_SCHED
> 
> 
> 
> pfifo_fast is available even without CONFIG_NET_SCHED, maybe
> thats a better choice to avoid unnecessary bloat.

BTW, I noticed a few bugs while looking at the qdisc handling in
wireless-dev:

- wme_qdiscop_enqueue doesn't increment q.qlen for packets queued
  to q->requeued[], which might cause upper layer code to stop
  dequeueing if q.qlen reaches zero.

- classify_1d doesn't care about tc_classify return values.
  tc_classify may decide to steal packets, drop them, etc. In case
  of stolen packets this causes use-after-free, otherwise just
  malfunctions.

- classify_1d returns res.class if it is != -1, which can never happen
  (except with an empty classifier list because of the explicit
  initialization, but you should check the return code) since ->get()
  and ->bind_tcf() both return 0 for invalid classes and the classid
  otherwise. There's also an off-by-one, classids start at one, so it
  should return res.class - 1 (or better res.classid - 1, which is
  meant to be a numerical identifier).

- wme_discop_destroy leaks classifier module references and memory
  when destroying classifiers, it should use tcf_destroy()

Considering that it is possibly and may be desirable to attach a
different qdisc than the built-in multiband qdisc, it might also
make sense to split the 80211 specific classification in a seperate
classifier module to allow simple classification of management traffic
with other qdiscs.

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jeff Garzik]

On Thu, Oct 26, 2006 at 03:21:10AM +0200, Patrick McHardy wrote:
> Considering that it is possibly and may be desirable to attach a
> different qdisc than the built-in multiband qdisc, it might also
> make sense to split the 80211 specific classification in a seperate
> classifier module to allow simple classification of management traffic
> with other qdiscs.

I've been telling the wireless people this for ages...

	Jeff

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

Doing this will slow down the qdisc - it does already run an external
classifier first if you install one. On typical laptops performance is
not a problem, but one common usage does have problems. The performance
of a wireless home gateway based on a simple linux kernel configuration
(NAT, iptables, bridging) is significantly slower than a VxWorks based
one. With the arrival of 802.11n improving the performance of the stack
is very important. The VxWorks solution may be less flexible in terms of
what it can do, but it will allow the full 802.11n bandwidth to be
achieved on slower embedded CPUs than Linux does. How to improve the
linux kernel networking to improve it's efficiency here is a hard
problem.

Simon


-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Jeff Garzik
Sent: Wednesday, October 25, 2006 7:39 PM
To: Patrick McHardy
Cc: David Kimdon; netdev@vger.kernel.org; John W. Linville; Jiri Benc
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

On Thu, Oct 26, 2006 at 03:21:10AM +0200, Patrick McHardy wrote:
> Considering that it is possibly and may be desirable to attach a 
> different qdisc than the built-in multiband qdisc, it might also make 
> sense to split the 80211 specific classification in a seperate 
> classifier module to allow simple classification of management traffic

> with other qdiscs.

I've been telling the wireless people this for ages...

	Jeff



-
To unsubscribe from this list: send the line "unsubscribe netdev" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jeff Garzik]

On Wed, Oct 25, 2006 at 08:37:04PM -0700, Simon Barber wrote:
> Doing this will slow down the qdisc - it does already run an external
> classifier first if you install one. On typical laptops performance is
> not a problem, but one common usage does have problems. The performance
> of a wireless home gateway based on a simple linux kernel configuration
> (NAT, iptables, bridging) is significantly slower than a VxWorks based
> one. With the arrival of 802.11n improving the performance of the stack
> is very important. The VxWorks solution may be less flexible in terms of
> what it can do, but it will allow the full 802.11n bandwidth to be
> achieved on slower embedded CPUs than Linux does. How to improve the
> linux kernel networking to improve it's efficiency here is a hard
> problem.

First do it right, then tune performance.  If you aren't doing it right,
then who cares about VxWorks.

We also need 802.11 to register as a real protocol, rather than faking
ethernet.  David Miller did an example stub:
http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/davem-p80211.tar.bz2

	Jeff

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

Re: registering as a real protocol - yes this I have been going on about
for a while. This needs a few changes in how things work:

1. Register as a real protocol.
2. Change drivers to use netif_rx to receive frames (will also be more
efficient)

I would also like to see:

Drivers to use register_netdev - not special ieee80211 register to
register themselves
Drivers to use a normal hard_start_xmit function
- need to move 802.11 pre xmit frame processing into the qdisc dequeue
function
Get rid of gunky beacon & multicast buffered frame handling - make them
into normal queues like everything else (so same hard_start_xmit can be
used).

Simon


-----Original Message-----
From: Jeff Garzik [mailto:jeff@garzik.org] 
Sent: Wednesday, October 25, 2006 10:04 PM
To: Simon Barber
Cc: Patrick McHardy; David Kimdon; netdev@vger.kernel.org; John W.
Linville; Jiri Benc
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

On Wed, Oct 25, 2006 at 08:37:04PM -0700, Simon Barber wrote:
> Doing this will slow down the qdisc - it does already run an external 
> classifier first if you install one. On typical laptops performance is

> not a problem, but one common usage does have problems. The 
> performance of a wireless home gateway based on a simple linux kernel 
> configuration (NAT, iptables, bridging) is significantly slower than a

> VxWorks based one. With the arrival of 802.11n improving the 
> performance of the stack is very important. The VxWorks solution may 
> be less flexible in terms of what it can do, but it will allow the 
> full 802.11n bandwidth to be achieved on slower embedded CPUs than 
> Linux does. How to improve the linux kernel networking to improve it's

> efficiency here is a hard problem.

First do it right, then tune performance.  If you aren't doing it right,
then who cares about VxWorks.

We also need 802.11 to register as a real protocol, rather than faking
ethernet.  David Miller did an example stub:
http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/dave
m-p80211.tar.bz2

	Jeff

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jiri Benc]

On Wed, 25 Oct 2006 22:15:23 -0700, Simon Barber wrote:
> Re: registering as a real protocol - yes this I have been going on about
> for a while. This needs a few changes in how things work:
> 
> 1. Register as a real protocol.
> 2. Change drivers to use netif_rx to receive frames (will also be more
> efficient)

This is something I really want too.

However, the biggest problem is bridging. A lot of people bridge their
ethernet and wifi interfaces together. We'll need to change the
bridging code to make it possible again.

> I would also like to see:
> 
> Drivers to use register_netdev - not special ieee80211 register to
> register themselves

I don't see a benefit in this, but well, let's consider it when there
is a patch.

> Drivers to use a normal hard_start_xmit function
> - need to move 802.11 pre xmit frame processing into the qdisc dequeue
> function

Agreed. It's not a priority, though.

> Get rid of gunky beacon & multicast buffered frame handling - make them
> into normal queues like everything else (so same hard_start_xmit can be
> used).

I'm not sure about beacons here - don't some drivers need to handle
them specially?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[John W. Linville]

On Wed, Nov 01, 2006 at 11:28:05AM +0100, Jiri Benc wrote:
> On Wed, 25 Oct 2006 22:15:23 -0700, Simon Barber wrote:
> > Re: registering as a real protocol - yes this I have been going on about
> > for a while. This needs a few changes in how things work:
> > 
> > 1. Register as a real protocol.
> > 2. Change drivers to use netif_rx to receive frames (will also be more
> > efficient)
> 
> This is something I really want too.

I see Simon's suggestions in the category of making wireless devices
"1st class citizens".  I mostly agree with his suggestions, and they
are "on my list".  Do we see these as merge requirements?  I have them
as a lower priority than locking analysis/fixes, cfg80211 migration,
fullmac support, and driver porting.

Is this agreeable?  Or do we think this needs to happen before a
merge to -mm?

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[James Ketrenos]

John W. Linville wrote:
> On Wed, Nov 01, 2006 at 11:28:05AM +0100, Jiri Benc wrote:
>> On Wed, 25 Oct 2006 22:15:23 -0700, Simon Barber wrote:
>>> Re: registering as a real protocol - yes this I have been going on about
>>> for a while. This needs a few changes in how things work:
>>>
>>> 1. Register as a real protocol.
>>> 2. Change drivers to use netif_rx to receive frames (will also be more
>>> efficient)
>> This is something I really want too.
> 
> I see Simon's suggestions in the category of making wireless devices
> "1st class citizens".  I mostly agree with his suggestions, and they
> are "on my list".  Do we see these as merge requirements?  

The only merge requirement should be that merging d80211 doesn't break
existing in-tree wireless drivers.  We have that today -- you can have
both stacks in the tree and running in parallel.  With that, we have the
ability to migrate over time.

The other 'merge requirements' are functionally irrelevant.  We know the
d80211 stack is where wireless on Linux is going -- the sooner steps are
taken to get that stack out to the masses the faster it can be adopted
and improved.

The agreement to switch to d80211 was reached back in April -- its been
six months and there still doesn't appear to be traction at putting in
action the decisions reached at that time.

If people have issues with with specific components of d80211 prior to
its merging, stand up and state what they are and how not fixing them
would negatively impact people that aren't using the d80211 subsystem.

Don't take the above as me saying there aren't items that need to be
fixed/improved in d80211 -- there is work to be done.  But that
shouldn't stop it from being merged w/ the EXPERIMENTAL flag set.

> Is this agreeable?  Or do we think this needs to happen before a
> merge to -mm?

We reached the point where we should be in -mm a long time ago as soon
as both stacks could exist concurrently.  d80211 should have been in
Linus' tree a long time ago.

James

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jeff Garzik]

James Ketrenos wrote:
> If people have issues with with specific components of d80211 prior to
> its merging, stand up and state what they are and how not fixing them
> would negatively impact people that aren't using the d80211 subsystem.
> 
> Don't take the above as me saying there aren't items that need to be
> fixed/improved in d80211 -- there is work to be done.  But that
> shouldn't stop it from being merged w/ the EXPERIMENTAL flag set.

> We reached the point where we should be in -mm a long time ago as soon
> as both stacks could exist concurrently.  d80211 should have been in
> Linus' tree a long time ago.

d80211 merge stoppers:
- SMP issues (lack of locking, and overlocking via use of Big Network Lock)
- userspace ABI

It definitely shouldn't go upstream without that stuff.

	Jeff

d80211 merge (was Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc)

[James Ketrenos]

Jeff Garzik wrote:
> James Ketrenos wrote:
>> If people have issues with with specific components of d80211 prior to
>> its merging, stand up and state what they are and how not fixing them
>> would negatively impact people that aren't using the d80211 subsystem.
>>
>> Don't take the above as me saying there aren't items that need to be
>> fixed/improved in d80211 -- there is work to be done.  But that
>> shouldn't stop it from being merged w/ the EXPERIMENTAL flag set.
> 
>> We reached the point where we should be in -mm a long time ago as soon
>> as both stacks could exist concurrently.  d80211 should have been in
>> Linus' tree a long time ago.
> 
> d80211 merge stoppers:
> - SMP issues (lack of locking, and overlocking via use of Big Network Lock)

So we can set BROKEN_ON_SMP on it and we're good.  The entire
description of EXPERIMENTAL in init/Kconfig matches perfectly what we
want -- bug reports, wider exposure, and a clear articulation that the
feature is unstable.

> - userspace ABI

What's the requirement on the userspace ABI?  That the existing wireless
extension interface needs to work, or ?

We can use the wireless extension currently to match (most of) the
existing WE functionality -- at least to get the majority of wireless
users up and running (for those that have cards that have d80211 drivers
anyway).

> It definitely shouldn't go upstream without that stuff.

How will merging with the existing locking problems or ABI issue
negatively impact people that aren't using the d80211 subsystem anyway?

If having the code there doesn't hurt anyone, why not get it out for
wider adoption, exposure, and contribution?

Why is there such a hesitation to merge this stack and help to
accelerate the set of functionality available with wireless on Linux?

James

Re: d80211 merge (was Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc)

[Jeff Garzik]

James Ketrenos wrote:
> Jeff Garzik wrote:
>> James Ketrenos wrote:
>>> If people have issues with with specific components of d80211 prior to
>>> its merging, stand up and state what they are and how not fixing them
>>> would negatively impact people that aren't using the d80211 subsystem.
>>>
>>> Don't take the above as me saying there aren't items that need to be
>>> fixed/improved in d80211 -- there is work to be done.  But that
>>> shouldn't stop it from being merged w/ the EXPERIMENTAL flag set.
>>> We reached the point where we should be in -mm a long time ago as soon
>>> as both stacks could exist concurrently.  d80211 should have been in
>>> Linus' tree a long time ago.
>> d80211 merge stoppers:
>> - SMP issues (lack of locking, and overlocking via use of Big Network Lock)
> 
> So we can set BROKEN_ON_SMP on it and we're good.  The entire
> description of EXPERIMENTAL in init/Kconfig matches perfectly what we
> want -- bug reports, wider exposure, and a clear articulation that the
> feature is unstable.

heh, no, we don't merge stuff that's obviously incomplete or broken. 
BROKEN_ON_SMP is one step before driver death, not the first baby step 
of a driver.


>> - userspace ABI
> 
> What's the requirement on the userspace ABI?  That the existing wireless
> extension interface needs to work, or ?

The requirement is that code should not go upstream if its userspace 
interface is highly likely to change.  ABI mistakes are locked in stone 
once they hit the upstream kernel, for the most part.

As long as the userspace ABI is "baked" and generally agreed upon, then 
the requirement is satisfied.

Speaking specifically, Linus noted recently d80211 should maintain 
backwards compatibility with the WE userspace ABI, so that existing 
wireless tools keep working, and I definitely agree.  For additional 
functionality/flexibility, I presume there will be some sort of netlink 
extensible interface (cfg80211 I think?) that newer wireless tools will 
use.  In time, the WE back-compat ABI could become an optional module 
that users can disable.


> How will merging with the existing locking problems or ABI issue
> negatively impact people that aren't using the d80211 subsystem anyway?

Irrelevant.

We don't merge code with obvious bugs, especially when the justification 
is "merging means the bugs will be fixed."

If a codebase isn't getting bugs fixed as it is, that is a strong reason 
AGAINST merging it.


> If having the code there doesn't hurt anyone, why not get it out for
> wider adoption, exposure, and contribution?
> 
> Why is there such a hesitation to merge this stack and help to
> accelerate the set of functionality available with wireless on Linux?

A codebase has to have developers who will put in the minimum effort to 
make sure it is completely SMP safe, and that the ABI issues are handled 
in the way they are handled throughout the rest of the kernel.

"Wider participation" is not a blank check to merge every half-baked 
codebase out there.  One of the things that's nice about git is that 
things enter the kernel MORE baked than in the past, because distributed 
development means stuff occurs in parallel.

"Getting is upstream" is the eventual goal, NOT a magic panacea that 
wipes away existing ills.

	Jeff, who, just like you, wants to see d80211 merged

cfg80211/nl80211/WE (was: Re: d80211 merge)

[Johannes Berg]

On Wed, 2006-11-01 at 21:55 -0500, Jeff Garzik wrote:

> Speaking specifically, Linus noted recently d80211 should maintain 
> backwards compatibility with the WE userspace ABI, so that existing 
> wireless tools keep working, and I definitely agree.  For additional 
> functionality/flexibility, I presume there will be some sort of netlink 
> extensible interface (cfg80211 I think?) that newer wireless tools will 
> use.  In time, the WE back-compat ABI could become an optional module 
> that users can disable.

I'm pretty sure I've said this like a dozen times but apparently it
hasn't sunk in yet:

The latest cfg80211/nl80211 code comes with WE compatibility code that
users can disable if they want to, which, however, gives them the
ability to use WE in userspace while the drivers are happily using
cfg80211.

My goal is to remove WE from *all* drivers, even from the ancient
pre-802.11 and early 802.11 fullmac drivers, and leave WE merely as
another userspace interface for cfg80211, albeit with limitations that
the netlink interface nl80211 doesn't have.

Until all drivers are converted, however, cfg80211-WE and regular WE
coexist peacefully, it is even possible to convert one ioctl at a time
over to cfg80211 (which is only really useful while working at it, but
still).

johannes

Re: cfg80211/nl80211/WE

[Jeff Garzik]

Johannes Berg wrote:
> My goal is to remove WE from *all* drivers, even from the ancient
> pre-802.11 and early 802.11 fullmac drivers, and leave WE merely as
> another userspace interface for cfg80211, albeit with limitations that
> the netlink interface nl80211 doesn't have.

Sounds good to me...

	Jeff

Re: cfg80211/nl80211/WE (was: Re: d80211 merge)

[Christoph Hellwig]

On Thu, Nov 02, 2006 at 09:49:01AM +0100, Johannes Berg wrote:
> I'm pretty sure I've said this like a dozen times but apparently it
> hasn't sunk in yet:
> 
> The latest cfg80211/nl80211 code comes with WE compatibility code that
> users can disable if they want to, which, however, gives them the
> ability to use WE in userspace while the drivers are happily using
> cfg80211.
> 
> My goal is to remove WE from *all* drivers, even from the ancient
> pre-802.11 and early 802.11 fullmac drivers, and leave WE merely as
> another userspace interface for cfg80211, albeit with limitations that
> the netlink interface nl80211 doesn't have.

Yes, that is exatly the right way to go forward.  The userspace interface
is however more than just the WE ioctls.  It's also that for every wireless
card one single ethX device appears and not multiple devices with other
names.  This might sound like nitpicking, but without this all kinds of
scripts break.

Re: cfg80211/nl80211/WE (was: Re: d80211 merge)

[Johannes Berg]

On Thu, 2006-11-02 at 10:56 +0000, Christoph Hellwig wrote:

> Yes, that is exatly the right way to go forward.  The userspace interface
> is however more than just the WE ioctls.  It's also that for every wireless
> card one single ethX device appears and not multiple devices with other
> names.  This might sound like nitpicking, but without this all kinds of
> scripts break.

cfg80211 doesn't care about that, it only requires you to register one
logical wiphy device. Whether it has more than one netdev or not isn't
significant.

For d80211, however, trying to get rid of the wmasterX interface
requires tighter integration in order to run a qdisc on a non-netdev
wireless physical device. Which gets us back to how this thread
started :)

johannes

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Christoph Hellwig]

On Wed, Nov 01, 2006 at 10:31:19AM -0800, James Ketrenos wrote:
> The only merge requirement should be that merging d80211 doesn't break
> existing in-tree wireless drivers.  We have that today -- you can have
> both stacks in the tree and running in parallel.  With that, we have the
> ability to migrate over time.
> 
> The other 'merge requirements' are functionally irrelevant.  We know the
> d80211 stack is where wireless on Linux is going -- the sooner steps are
> taken to get that stack out to the masses the faster it can be adopted
> and improved.
> 

We had that discussion offline last week, but let's repeat it here for
the general public.

The most important merge requirement is to not break userspace.  That means
proper support of WE (hopefully via cfg80211), and a single ethX network
device.  The second most important is proper smp support, or good code
quality in general.  We can't afford to merge buggy code.  Third most
important is to make sure it's a full replacment for the current ieee80211
code - for the softmac cards that's mostly trivial, but the half-hard mac
old intel cards are hard.  That's where intel comes into play because you
support that hardware _and_ are pushing for d80211.  That makes your team
pretty much volunteer to fix that up in my eyes.

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jiri Benc]

On Thu, 2 Nov 2006 12:16:57 +0000
Christoph Hellwig <hch@infradead.org> wrote:
> The most important merge requirement is to not break userspace.  That means
> proper support of WE (hopefully via cfg80211), and a single ethX network
> device.

Sounds reasonable. Though:
- I don't think ethX is the appropriate name. It's not an ethernet
  device and many wireless drivers currently use wlanX name. I think
  the current wlanX name is a better choice here.
- Single device is not as easy as it sounds. It would require rather
  invasive changes in the networking core or ugly hacks in d80211. I'm
  afraid this is something not achievable in a near future with current
  number of people working on d80211. But in general, it's doable and
  desirable, yes.

> The second most important is proper smp support, or good code
> quality in general.  We can't afford to merge buggy code.

I definitely agree here.

> Third most
> important is to make sure it's a full replacment for the current ieee80211
> code - for the softmac cards that's mostly trivial, but the half-hard mac
> old intel cards are hard.  That's where intel comes into play because you
> support that hardware _and_ are pushing for d80211.  That makes your team
> pretty much volunteer to fix that up in my eyes.

Let's see...

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Christoph Hellwig]

On Thu, Nov 02, 2006 at 03:05:50PM +0100, Jiri Benc wrote:
> On Thu, 2 Nov 2006 12:16:57 +0000
> Christoph Hellwig <hch@infradead.org> wrote:
> > The most important merge requirement is to not break userspace.  That means
> > proper support of WE (hopefully via cfg80211), and a single ethX network
> > device.
> 
> Sounds reasonable. Though:
> - I don't think ethX is the appropriate name. It's not an ethernet
>   device and many wireless drivers currently use wlanX name. I think
>   the current wlanX name is a better choice here.

This is not about the name that makes sense.  I think using ethX names
for wireless devices is utterly stupid, but it's what all current upstream
drivers do, and at least for WE compat we'll have to stick to it.

> - Single device is not as easy as it sounds. It would require rather
>   invasive changes in the networking core or ugly hacks in d80211. I'm
>   afraid this is something not achievable in a near future with current
>   number of people working on d80211. But in general, it's doable and
>   desirable, yes.

Again, I'm not sure it's even desirable in the end.  We do however need
a mode in which you can configure a single ethX device with WE and it'll
just work.  It's entirely reasonable to only support a subset of the
functionality that way (no 802.11n, no AP mode, etc)

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Johannes Berg]

On Thu, 2006-11-02 at 14:18 +0000, Christoph Hellwig wrote:

> This is not about the name that makes sense.  I think using ethX names
> for wireless devices is utterly stupid, but it's what all current upstream
> drivers do, and at least for WE compat we'll have to stick to it.

No, that's not true, zd1201 doesn't [1] :) Has anyoen ever complained
about that? In any case, tools iterate all netdevs and call some
wireless ioctl to check if they are wireless, or
check /proc/net/wireless (which is actually bad, but ...).

Besides, WE never said that names have to have certain names. What makes
you think so?

johannes

[1]
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;h=36b29ff058141a712619e55184566b39ce414efd;hb=aefba081d7b7dfd1c5752f6e6e709d8b5ab80ab7;f=drivers/net/wireless/zd1201.c#l1790

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jochen Friedrich]

Hi Johannes,

> On Thu, 2006-11-02 at 14:18 +0000, Christoph Hellwig wrote:
> 
>> This is not about the name that makes sense.  I think using ethX names
>> for wireless devices is utterly stupid, but it's what all current upstream
>> drivers do, and at least for WE compat we'll have to stick to it.
> 
> No, that's not true, zd1201 doesn't [1] :)

Neither does hostap [2] :)

Thanks,
Jochen

[2] http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;h=ed00ebb6e7f4bd255d1a1fe8bf05a6f7a5574d6e;hb=aefba081d7b7dfd1c5752f6e6e709d8b5ab80ab7;f=drivers/net/wireless/hostap/hostap_hw.c#l84

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Christoph Hellwig]

On Thu, Nov 02, 2006 at 03:32:34PM +0100, Johannes Berg wrote:
> On Thu, 2006-11-02 at 14:18 +0000, Christoph Hellwig wrote:
> 
> > This is not about the name that makes sense.  I think using ethX names
> > for wireless devices is utterly stupid, but it's what all current upstream
> > drivers do, and at least for WE compat we'll have to stick to it.
> 
> No, that's not true, zd1201 doesn't [1] :) Has anyoen ever complained
> about that? In any case, tools iterate all netdevs and call some
> wireless ioctl to check if they are wireless, or
> check /proc/net/wireless (which is actually bad, but ...).
> 
> Besides, WE never said that names have to have certain names. What makes
> you think so?

I never said the name was related to WE - in fact there are tons of out of
tree drivers with different names.  The important bit is that upgrading a
kernel adn changing the wireless stack must not change the device name.
All my scripts expect my bcm43xx card to be eth1 and not wifi0 or
wlansomething.  There's in fact another much worse migration prblem
when we want to do wlan support correctly: the advertised frametype still
is 802.3 for all wlan devices (which make the current ethX names correct
in some obscure very technical way) and we need to make them raw 802.11
long-term.  It will probably make sense to do both migrations at the
same time and have a CONFIG_ option to force the old behaviour [1].

[1] I really hate config options affecting the abi, but this is a tough
    problem as we can't have a etdevice with different frametypes for
    the same piece of hardware.  maybewe could at least make it a module
    option..

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Johannes Berg]

On Thu, 2006-11-02 at 14:45 +0000, Christoph Hellwig wrote:

> I never said the name was related to WE - in fact there are tons of out of
> tree drivers with different names.  The important bit is that upgrading a
> kernel adn changing the wireless stack must not change the device name.

Ah, but if that is your point then you should be arguing that the stack
may not dictate the device name at all, in order to keep existing
drivers able to keep whatever devicenames they were creating now...

> All my scripts expect my bcm43xx card to be eth1 and not wifi0 or
> wlansomething.  

But, but.. Kernel-assigned device names aren't really stable in
practise, and in most distros udev will actually assign a stable name
based on the mac address (which will, incidentally, apparently be ethX).

Really though, I don't care about having it ethX, I rename it to
whatever I want in userspace anyway.

> There's in fact another much worse migration prblem
> when we want to do wlan support correctly: the advertised frametype still
> is 802.3 for all wlan devices (which make the current ethX names correct
> in some obscure very technical way) and we need to make them raw 802.11
> long-term.  It will probably make sense to do both migrations at the
> same time and have a CONFIG_ option to force the old behaviour [1].

Making it 802.11 will probably upset dhcp, I'd think. I have no idea how
to handle that, at this time.

johannes

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

I don't believe it makes sense to change the frame type except for the
master interface. The interface to an 802.11 MAC (the MAC-SAP) is very
similar to the interface to an 802.3 MAC. It is a 2 address format - not
3 or 4. The master interface is an internal part of the MAC, and here
802.11 frames are essential.

Simon 

-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org] 
Sent: Thursday, November 02, 2006 6:46 AM
To: Johannes Berg
Cc: Christoph Hellwig; Jiri Benc; James Ketrenos; John W. Linville;
Simon Barber; Jeff Garzik; Patrick McHardy; David Kimdon;
netdev@vger.kernel.org
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

On Thu, Nov 02, 2006 at 03:32:34PM +0100, Johannes Berg wrote:
> On Thu, 2006-11-02 at 14:18 +0000, Christoph Hellwig wrote:
> 
> > This is not about the name that makes sense.  I think using ethX 
> > names for wireless devices is utterly stupid, but it's what all 
> > current upstream drivers do, and at least for WE compat we'll have
to stick to it.
> 
> No, that's not true, zd1201 doesn't [1] :) Has anyoen ever complained 
> about that? In any case, tools iterate all netdevs and call some 
> wireless ioctl to check if they are wireless, or check 
> /proc/net/wireless (which is actually bad, but ...).
> 
> Besides, WE never said that names have to have certain names. What 
> makes you think so?

I never said the name was related to WE - in fact there are tons of out
of tree drivers with different names.  The important bit is that
upgrading a kernel adn changing the wireless stack must not change the
device name.
All my scripts expect my bcm43xx card to be eth1 and not wifi0 or
wlansomething.  There's in fact another much worse migration prblem when
we want to do wlan support correctly: the advertised frametype still is
802.3 for all wlan devices (which make the current ethX names correct in
some obscure very technical way) and we need to make them raw 802.11
long-term.  It will probably make sense to do both migrations at the
same time and have a CONFIG_ option to force the old behaviour [1].

[1] I really hate config options affecting the abi, but this is a tough
    problem as we can't have a etdevice with different frametypes for
    the same piece of hardware.  maybewe could at least make it a module
    option..

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jiri Benc]

On Thu, 2 Nov 2006 14:18:15 +0000, Christoph Hellwig wrote:
> This is not about the name that makes sense.  I think using ethX names
> for wireless devices is utterly stupid, but it's what all current upstream
> drivers do, and at least for WE compat we'll have to stick to it.

That's not true. Look at hostap or zd1201, for example. Also, ethX name
is not needed for WE compatibility. And because of various names used
by various drivers (madwifi uses ath0, for example) config scripts
don't depend on ethX name today anyway.

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Sven-Haegar Koch]

On Thu, 2 Nov 2006, Jiri Benc wrote:

> On Thu, 2 Nov 2006 14:18:15 +0000, Christoph Hellwig wrote:
>> This is not about the name that makes sense.  I think using ethX names
>> for wireless devices is utterly stupid, but it's what all current upstream
>> drivers do, and at least for WE compat we'll have to stick to it.
>
> That's not true. Look at hostap or zd1201, for example. Also, ethX name
> is not needed for WE compatibility. And because of various names used
> by various drivers (madwifi uses ath0, for example) config scripts
> don't depend on ethX name today anyway.

At least the ones used on my machines all do.
(Debian Sarge, Etch and Unstable)
The configfiles can be changed without problems to use any device name, 
but needing to edit various files just for a kernel update would inhibt me 
from just trying a new version until I really know I would not need to go 
back to the old version again.

(Switching wireless from eth1 to eth2 after copying the harddisk contents 
to a new laptop was work enough, but at least expected with a new machine 
and changed hardware)

c'ya
sven

-- 

The Internet treats censorship as a routing problem, and routes around it.
(John Gilmore on http://www.cygnus.com/~gnu/)

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Jiri Benc]

On Thu, 2 Nov 2006 17:09:24 +0100 (CET), Sven-Haegar Koch wrote:
> At least the ones used on my machines all do.
> (Debian Sarge, Etch and Unstable)
> The configfiles can be changed without problems to use any device name, 
> but needing to edit various files just for a kernel update would inhibt me 
> from just trying a new version until I really know I would not need to go 
> back to the old version again.

Then your config scripts don't work with Prism and Zydas cards. Something
is broken with your setup.

> (Switching wireless from eth1 to eth2 after copying the harddisk contents 
> to a new laptop was work enough, but at least expected with a new machine 
> and changed hardware)

Because nobody can guarantee you even the order of your Ethernet interfaces
with newer kernels (e.g. your eth0 Ethernet card may become eth1 at any
time) and it will become even worse with the parallel device discovery,
your setup is going to explode anyway.

Sorry, I can't take this as an argument. Using your argumentation, we must
preserve wlan0 name because scripts of zd1201 users will break otherwise.
Or - wait a moment - no no, we must preserve ath0 name because of madwifi
users - and oh, we must preserve eth1 (not ethX because the scripts cannot
deal with it) because of ipw users. And we must definitely ensure that card
numbers won't ever change. Even if you uplug them and replug to another
slots in a different order.

Sorry, that cannot work.

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Dan Williams]

On Thu, 2006-11-02 at 19:38 +0100, Jiri Benc wrote:
> On Thu, 2 Nov 2006 17:09:24 +0100 (CET), Sven-Haegar Koch wrote:
> > At least the ones used on my machines all do.
> > (Debian Sarge, Etch and Unstable)
> > The configfiles can be changed without problems to use any device name, 
> > but needing to edit various files just for a kernel update would inhibt me 
> > from just trying a new version until I really know I would not need to go 
> > back to the old version again.
> 
> Then your config scripts don't work with Prism and Zydas cards. Something
> is broken with your setup.
> 
> > (Switching wireless from eth1 to eth2 after copying the harddisk contents 
> > to a new laptop was work enough, but at least expected with a new machine 
> > and changed hardware)
> 
> Because nobody can guarantee you even the order of your Ethernet interfaces
> with newer kernels (e.g. your eth0 Ethernet card may become eth1 at any
> time) and it will become even worse with the parallel device discovery,
> your setup is going to explode anyway.
> 
> Sorry, I can't take this as an argument. Using your argumentation, we must
> preserve wlan0 name because scripts of zd1201 users will break otherwise.
> Or - wait a moment - no no, we must preserve ath0 name because of madwifi
> users - and oh, we must preserve eth1 (not ethX because the scripts cannot
> deal with it) because of ipw users. And we must definitely ensure that card
> numbers won't ever change. Even if you uplug them and replug to another
> slots in a different order.
> 
> Sorry, that cannot work.

Right.

Nothing should be relying on the actual device name, they should be
relying on the MAC address or some other UID of the actual device.  In
many distros, the config scripts depend on a set name, but only _AFTER_
they have check the devices MAC address, matched that up with a
MAC->devname table, and renamed the device.  That's somewhat sane, but
scripts cannot just rely on the device name without checking some other
unique attribute of the device.

Dan

>  Jiri
> 

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

Perhaps the solution is to allow the prefix to be a kernel configuration
item?

Simon 

-----Original Message-----
From: Jiri Benc [mailto:jbenc@suse.cz] 
Sent: Thursday, November 02, 2006 10:39 AM
To: Sven-Haegar Koch
Cc: Christoph Hellwig; James Ketrenos; John W. Linville; Simon Barber;
Jeff Garzik; Patrick McHardy; David Kimdon; netdev@vger.kernel.org
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

On Thu, 2 Nov 2006 17:09:24 +0100 (CET), Sven-Haegar Koch wrote:
> At least the ones used on my machines all do.
> (Debian Sarge, Etch and Unstable)
> The configfiles can be changed without problems to use any device 
> name, but needing to edit various files just for a kernel update would

> inhibt me from just trying a new version until I really know I would 
> not need to go back to the old version again.

Then your config scripts don't work with Prism and Zydas cards.
Something is broken with your setup.

> (Switching wireless from eth1 to eth2 after copying the harddisk 
> contents to a new laptop was work enough, but at least expected with a

> new machine and changed hardware)

Because nobody can guarantee you even the order of your Ethernet
interfaces with newer kernels (e.g. your eth0 Ethernet card may become
eth1 at any
time) and it will become even worse with the parallel device discovery,
your setup is going to explode anyway.

Sorry, I can't take this as an argument. Using your argumentation, we
must preserve wlan0 name because scripts of zd1201 users will break
otherwise.
Or - wait a moment - no no, we must preserve ath0 name because of
madwifi users - and oh, we must preserve eth1 (not ethX because the
scripts cannot deal with it) because of ipw users. And we must
definitely ensure that card numbers won't ever change. Even if you uplug
them and replug to another slots in a different order.

Sorry, that cannot work.

 Jiri

--
Jiri Benc
SUSE Labs

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Stephen Hemminger]

On Thu, 2 Nov 2006 13:27:59 -0800
"Simon Barber" <simon@devicescape.com> wrote:

> Perhaps the solution is to allow the prefix to be a kernel configuration
> item?
> 
> Simon 

All modern distro's support device naming stuff through hotplug.
Don't put effort into keeping the kernel defaults absolute.

Please have Ethernet (and wireless) devices show up as eth%d. For the master device,
choose something else (mac%d ?).

-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Luis R. Rodriguez]

On 11/2/06, Stephen Hemminger <shemminger@osdl.org> wrote:

> Please have Ethernet (and wireless) devices show up as eth%d. For the
> master device, choose something else (mac%d ?).

If ultimately we're going to make wireless devices, as John puts it,
"1st class citizens" by making 802.11 a full protocol we should just
start sticking to a new convention. Before when we didn't have any
softmac drivers in the kernel ethX made sense, with softmac this isn't
so clear, but with 802.11 as a full protocol perhaps wlanX will
finally make perfect sense. For the master device yes, how about mac%d
as its there do 802.11 MAC management.

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Johannes Berg]

On Thu, 2006-11-02 at 15:05 +0100, Jiri Benc wrote:
> - Single device is not as easy as it sounds. It would require rather
>   invasive changes in the networking core or ugly hacks in d80211. I'm
>   afraid this is something not achievable in a near future with current
>   number of people working on d80211. But in general, it's doable and
>   desirable, yes.

Actually, just as a note to all the others (talked to Jiri about it
already), I think it's easier than thought since the code is so
convoluted. The only purpose of the wmaster device appears to be to have
an outgoing frame queue through the qdisc. Since we require the 802.11
qdisc to be installed at the root for all netdevices anyway, we could
just as well hand the frame to it right away.

I'll look into that later today.

johannes

RE: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specific qdisc

[Simon Barber]

There are many reasons to have the master device - and many reasons not
to get rid of it. The only excpetion is when using a limited fullmac
card.  Some fullmac cards only support a single ethernet interface, so
for these cards a single device may be OK, if we want to allow multiple
different appearences for different types of card (a bad thing in my
book). (Note - older prism 54g fullmac cards support WDS, hence need
multiple interfaces).

The master interface represents the physical networking device that
works using native 802.11 frames. The qdisc works on 802.11 frames - not
ethernet, and it can change how 802.11 management frames are queued. It
can only be got rid of in the case of a fullmac card with only a single
interface, where all management frames are processed in the card.

In addition as we move to make 802.11 devices native (i.e. create an
802.11 protocol) the 802.11 master interface will be essential.

I don't see the problem with 802.11 master devices - they are not
ethernet devices, and should not confuse any user space. Any user space
that gets confused by netdevices with arp-hdr types that they do not
recognise is broken, and needs to be fixed.

Simon


-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Thursday, November 02, 2006 6:23 AM
To: Christoph Hellwig
Cc: James Ketrenos; John W. Linville; Simon Barber; Jeff Garzik; Patrick
McHardy; David Kimdon; netdev@vger.kernel.org
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather
thand80211-specific qdisc

On Thu, 2006-11-02 at 15:05 +0100, Jiri Benc wrote:
> - Single device is not as easy as it sounds. It would require rather
>   invasive changes in the networking core or ugly hacks in d80211. I'm
>   afraid this is something not achievable in a near future with
current
>   number of people working on d80211. But in general, it's doable and
>   desirable, yes.

Actually, just as a note to all the others (talked to Jiri about it
already), I think it's easier than thought since the code is so
convoluted. The only purpose of the wmaster device appears to be to have
an outgoing frame queue through the qdisc. Since we require the 802.11
qdisc to be installed at the root for all netdevices anyway, we could
just as well hand the frame to it right away.

I'll look into that later today.

johannes

RE: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specific qdisc

[Johannes Berg]

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

On Thu, 2006-11-02 at 08:33 -0800, Simon Barber wrote:
> There are many reasons to have the master device - and many reasons not
> to get rid of it. The only excpetion is when using a limited fullmac
> card.  Some fullmac cards only support a single ethernet interface, so
> for these cards a single device may be OK, if we want to allow multiple
> different appearences for different types of card (a bad thing in my
> book). (Note - older prism 54g fullmac cards support WDS, hence need
> multiple interfaces).
> 
> The master interface represents the physical networking device that
> works using native 802.11 frames. The qdisc works on 802.11 frames - not
> ethernet, and it can change how 802.11 management frames are queued. It
> can only be got rid of in the case of a fullmac card with only a single
> interface, where all management frames are processed in the card.
> 
> In addition as we move to make 802.11 devices native (i.e. create an
> 802.11 protocol) the 802.11 master interface will be essential.

While these are good points, I don't believe having the master interface
as a *netdev* is essential. In fact, it now has no functionality as a
netdev, if you try routing packets over it they're simply dropped.

Of course we need some notion of the wireless PHY and cfg80211
explicitly represents that by assigning a wiphy index, but there's no
need for it to be a netdev.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specific qdisc

[Stephen Hemminger]

On Thu, 02 Nov 2006 17:43:57 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:

> On Thu, 2006-11-02 at 08:33 -0800, Simon Barber wrote:
> > There are many reasons to have the master device - and many reasons not
> > to get rid of it. The only excpetion is when using a limited fullmac
> > card.  Some fullmac cards only support a single ethernet interface, so
> > for these cards a single device may be OK, if we want to allow multiple
> > different appearences for different types of card (a bad thing in my
> > book). (Note - older prism 54g fullmac cards support WDS, hence need
> > multiple interfaces).
> > 
> > The master interface represents the physical networking device that
> > works using native 802.11 frames. The qdisc works on 802.11 frames - not
> > ethernet, and it can change how 802.11 management frames are queued. It
> > can only be got rid of in the case of a fullmac card with only a single
> > interface, where all management frames are processed in the card.
> > 
> > In addition as we move to make 802.11 devices native (i.e. create an
> > 802.11 protocol) the 802.11 master interface will be essential.
> 
> While these are good points, I don't believe having the master interface
> as a *netdev* is essential. In fact, it now has no functionality as a
> netdev, if you try routing packets over it they're simply dropped.

It makes 802.11 packet capture easier as well.  Please don't invent yet
another network access object for the master device.

> Of course we need some notion of the wireless PHY and cfg80211
> explicitly represents that by assigning a wiphy index, but there's no
> need for it to be a netdev.
> 
> johannes


-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specific qdisc

[Johannes Berg]

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

On Thu, 2006-11-02 at 14:34 -0800, Stephen Hemminger wrote:

> It makes 802.11 packet capture easier as well.  Please don't invent yet
> another network access object for the master device.

Oh, but the master device doesn't get any packets you could capture. For
that, you need to add a monitor interface.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

RE: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specificqdisc

[Simon Barber]

I'm not sure why we don't use the master device for packet capture - I
can't think of any good reason for requiring a separate device. I would
think the master device is the perfect place to do packet capture, and
raw packet transmission.

Simon 

-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Thursday, November 02, 2006 2:57 PM
To: Stephen Hemminger
Cc: Simon Barber; Christoph Hellwig; James Ketrenos; John W. Linville;
Jeff Garzik; Patrick McHardy; David Kimdon; netdev@vger.kernel.org
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather
thand80211-specificqdisc

On Thu, 2006-11-02 at 14:34 -0800, Stephen Hemminger wrote:

> It makes 802.11 packet capture easier as well.  Please don't invent 
> yet another network access object for the master device.

Oh, but the master device doesn't get any packets you could capture. For
that, you need to add a monitor interface.

johannes

RE: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specificqdisc

[Simon Barber]

I should elaborate - if 802.11 is made into a real protocol - then raw
packet capture works correctly on the master device. (raw sockets opened
on the device see all frames before they are passed to the protocol).
This is the right way to go.

Simon 

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Simon Barber
Sent: Friday, November 03, 2006 11:24 AM
To: Johannes Berg; Stephen Hemminger
Cc: Christoph Hellwig; James Ketrenos; John W. Linville; Jeff Garzik;
Patrick McHardy; David Kimdon; netdev@vger.kernel.org
Subject: RE: [patch] d80211: use pfifo_qdisc_ops rather
thand80211-specificqdisc

I'm not sure why we don't use the master device for packet capture - I
can't think of any good reason for requiring a separate device. I would
think the master device is the perfect place to do packet capture, and
raw packet transmission.

Simon 

-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net]
Sent: Thursday, November 02, 2006 2:57 PM
To: Stephen Hemminger
Cc: Simon Barber; Christoph Hellwig; James Ketrenos; John W. Linville;
Jeff Garzik; Patrick McHardy; David Kimdon; netdev@vger.kernel.org
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather
thand80211-specificqdisc

On Thu, 2006-11-02 at 14:34 -0800, Stephen Hemminger wrote:

> It makes 802.11 packet capture easier as well.  Please don't invent 
> yet another network access object for the master device.

Oh, but the master device doesn't get any packets you could capture. For
that, you need to add a monitor interface.

johannes
-
To unsubscribe from this list: send the line "unsubscribe netdev" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

Re: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specificqdisc

[John W. Linville]

On Fri, Nov 03, 2006 at 11:29:33AM -0800, Simon Barber wrote:
> I should elaborate - if 802.11 is made into a real protocol - then raw
> packet capture works correctly on the master device. (raw sockets opened
> on the device see all frames before they are passed to the protocol).
> This is the right way to go.

There is some merit to this idea.  Then we could attach IBSS, WDS,
STA, or AP sub-interfaces (with or without ethernet encapsulation) in
a manner similar to how VLAN sub-interfaces are attached to ethernet
devices today.

Of course, I seem to remember ranting against that model a few
months ago.  I'll just wave my hands and call it pragmatism... :-)

John
-- 
John W. Linville
linville@tuxdriver.com

RE: [patch] d80211: use pfifo_qdisc_ops rather thand80211-specificqdisc

[Johannes Berg]

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

On Fri, 2006-11-03 at 11:23 -0800, Simon Barber wrote:
> I would
> think the master device is the perfect place to do packet capture,

Sort of. Since it'll be an 802.11 protocol thing, you won't be getting
any signal strength information etc. You really do need that.

>  and
> raw packet transmission.

Same here, you really do want to be able to submit a frame with the
bitrate to use etc. All not part of 802.11.

All the meta info you want from RX or need for proper TX cannot be
transported over that master netdev since it'll have a proper 802.11
protocol, so it's pretty much useless.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

RE: [patch] d80211: use pfifo_qdisc_ops ratherthand80211-specificqdisc

[Simon Barber]

I guess right now when you transmit on the master device frames are only
accepted with a cookie and all that info in the CB. It would work just
as well to move that info from the CB and into a protocol header. It
would also make it easier to expand the info without the size constraint
of the CB. Same could apply on receive.

Simon 

-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net] 
Sent: Friday, November 03, 2006 3:07 PM
To: Simon Barber
Cc: Stephen Hemminger; Christoph Hellwig; James Ketrenos; John W.
Linville; Jeff Garzik; Patrick McHardy; David Kimdon;
netdev@vger.kernel.org
Subject: RE: [patch] d80211: use pfifo_qdisc_ops
ratherthand80211-specificqdisc

On Fri, 2006-11-03 at 11:23 -0800, Simon Barber wrote:
> I would
> think the master device is the perfect place to do packet capture,

Sort of. Since it'll be an 802.11 protocol thing, you won't be getting
any signal strength information etc. You really do need that.

>  and
> raw packet transmission.

Same here, you really do want to be able to submit a frame with the
bitrate to use etc. All not part of 802.11.

All the meta info you want from RX or need for proper TX cannot be
transported over that master netdev since it'll have a proper 802.11
protocol, so it's pretty much useless.

johannes

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[John W. Linville]

On Thu, Nov 02, 2006 at 12:16:57PM +0000, Christoph Hellwig wrote:

> The most important merge requirement is to not break userspace.  That means
> proper support of WE (hopefully via cfg80211), and a single ethX network
> device.  The second most important is proper smp support, or good code
> quality in general.  We can't afford to merge buggy code.  Third most
> important is to make sure it's a full replacment for the current ieee80211
> code - for the softmac cards that's mostly trivial, but the half-hard mac
> old intel cards are hard.  That's where intel comes into play because you
> support that hardware _and_ are pushing for d80211.  That makes your team
> pretty much volunteer to fix that up in my eyes.

+1

This aligns very well with my position.

John
-- 
John W. Linville
linville@tuxdriver.com

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

Pfifo_fast does not make sense because the 802.11 qdisc already
categorizes the frames based on DSCP. The better thing would be to
extract the pfifo qdisc so that it does not require NET_SCHED, but this
is more work.

Simon 

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Patrick McHardy
Sent: Wednesday, October 25, 2006 4:29 PM
To: David Kimdon
Cc: netdev@vger.kernel.org; John W. Linville; Jiri Benc
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

David Kimdon wrote:
> wme.c needs a generic fifo qdisc for each hardware queue.  Switch 
> wme.c to use the generic fifo qdisc in net/sched/sch_fifo.c.  This 
> allows removal of net/d80211/fifo_qdisc.c which isn't particularily 
> tied to IEEE 802.11 in any way.
> 
> -#define CHILD_QDISC_OPS pfifo_qdisc_ops
> -
>  static inline int WLAN_FC_IS_QOS_DATA(u16 fc)  {
>  	return (fc & 0x8C) == 0x88;
> @@ -433,7 +431,7 @@ static int wme_qdiscop_init(struct Qdisc
>  	/* create child queues */
>  	for (i = 0; i < queues; i++) {
>  		skb_queue_head_init(&q->requeued[i]);
> -		q->queues[i] = qdisc_create_dflt(qd->dev,
&CHILD_QDISC_OPS);
> +		q->queues[i] = qdisc_create_dflt(qd->dev,
&pfifo_qdisc_ops);
>  		if (q->queues[i] == 0) {
>  			q->queues[i] = &noop_qdisc;
>  			printk(KERN_ERR "%s child qdisc %i creation
failed", dev->name, 
> i);
> Index: wireless-dev/net/d80211/Kconfig 
> ===================================================================
> --- wireless-dev.orig/net/d80211/Kconfig
> +++ wireless-dev/net/d80211/Kconfig
> @@ -3,6 +3,7 @@ config D80211
>  	select CRYPTO
>  	select CRYPTO_ARC4
>  	select CRYPTO_AES
> +	select NET_SCHED


pfifo_fast is available even without CONFIG_NET_SCHED, maybe thats a
better choice to avoid unnecessary bloat.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Patrick McHardy]

Simon Barber wrote:
> Pfifo_fast does not make sense because the 802.11 qdisc already
> categorizes the frames based on DSCP. The better thing would be to
> extract the pfifo qdisc so that it does not require NET_SCHED, but this
> is more work.

It wouldn't really hurt though since all frames queued to a band already
have the same priority, so pfifo_fast behaves similar to fifo.

BTW, is there a specific reason for using DSCP? Normally Linux uses
rt_tos2priority and prio2band for determining a priority for IP packets.
I don't want to argue that one is better than the other, but there
is some value in keeping things consistent ..

RE: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Simon Barber]

The Wi-Fi alliance's test plans for their WMM specification are written
assuming you use certain specific DSCP values. Since WMM is the only QoS
mechanism that is widely used with 802.11 I followed the WMM values in
the default 802.11 qdisc implementation. Most windows 802.11 drivers do
the same thing.

Simon
 

-----Original Message-----
From: Patrick McHardy [mailto:kaber@trash.net] 
Sent: Wednesday, October 25, 2006 6:50 PM
To: Simon Barber
Cc: David Kimdon; netdev@vger.kernel.org; John W. Linville; Jiri Benc
Subject: Re: [patch] d80211: use pfifo_qdisc_ops rather than
d80211-specific qdisc

Simon Barber wrote:
> Pfifo_fast does not make sense because the 802.11 qdisc already 
> categorizes the frames based on DSCP. The better thing would be to 
> extract the pfifo qdisc so that it does not require NET_SCHED, but 
> this is more work.

It wouldn't really hurt though since all frames queued to a band already
have the same priority, so pfifo_fast behaves similar to fifo.

BTW, is there a specific reason for using DSCP? Normally Linux uses
rt_tos2priority and prio2band for determining a priority for IP packets.
I don't want to argue that one is better than the other, but there is
some value in keeping things consistent ..

Re: [patch] d80211: use pfifo_qdisc_ops rather than d80211-specific qdisc

[Patrick McHardy]

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

Simon Barber wrote:
> Pfifo_fast does not make sense because the 802.11 qdisc already
> categorizes the frames based on DSCP. The better thing would be to
> extract the pfifo qdisc so that it does not require NET_SCHED, but this
> is more work.

This patch should be enough to use it without NET_SCHED. You could of
course also allow modular build.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1488 bytes --]

diff --git a/net/d80211/Kconfig b/net/d80211/Kconfig
index 0f07d41..2cb79c1 100644
--- a/net/d80211/Kconfig
+++ b/net/d80211/Kconfig
@@ -3,6 +3,7 @@ config D80211
 	select CRYPTO
 	select CRYPTO_ARC4
 	select CRYPTO_AES
+	select NET_SCH_FIFO
 	---help---
 	This option enables the hardware independent IEEE 802.11
 	networking stack.
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 8298ea9..1d8b0dc 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -40,6 +40,10 @@ config NET_SCHED
 	  The available schedulers are listed in the following questions; you
 	  can say Y to as many as you like. If unsure, say N now.
 
+config NET_SCH_FIFO
+	bool
+	default y if NET_SCHED=y
+
 if NET_SCHED
 
 choice
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 0f06aec..ff2d6e5 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -4,7 +4,7 @@ #
 
 obj-y	:= sch_generic.o
 
-obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_fifo.o sch_blackhole.o
+obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_blackhole.o
 obj-$(CONFIG_NET_CLS)		+= cls_api.o
 obj-$(CONFIG_NET_CLS_ACT)	+= act_api.o
 obj-$(CONFIG_NET_ACT_POLICE)	+= act_police.o
@@ -14,6 +14,7 @@ obj-$(CONFIG_NET_ACT_MIRRED)	+= act_mirr
 obj-$(CONFIG_NET_ACT_IPT)	+= act_ipt.o
 obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
 obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
+obj-$(CONFIG_NET_SCH_FIFO)	+= sch_fifo.o
 obj-$(CONFIG_NET_SCH_CBQ)	+= sch_cbq.o
 obj-$(CONFIG_NET_SCH_HTB)	+= sch_htb.o
 obj-$(CONFIG_NET_SCH_HPFQ)	+= sch_hpfq.o