Netdev List
 help / color / mirror / Atom feed
* Re: Dscape ieee80211: enabling/disabling the radio
From: Michael Wu @ 2006-05-09 22:36 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: netdev, Olivier, Jiri Benc
In-Reply-To: <200605100001.20180.IvDoorn@gmail.com>

On Tuesday 09 May 2006 18:01, Ivo van Doorn wrote:
> A user on the forums Olivier Cornu (added to the CC list) has done some
> investigation into the scanning behaviour of the dscape stack.
> Basicly the dscape stack is performing active scanning while the device is
> down, but during the active scan it is sending packets out, or at least
> attempting to do so. Besides the question if active scanning is preferred
> over passive scanning while interface is down, active scanning fails
> because the packets that should be send are being send through the regular
> xmit routines of the interface. (IFF_UP is not set for the interface)
>
> This means that besides enabling the radio which should be done in the
> driver, the stack should either bring up the interface when doing an active
> scan, or resort to passive scanning while interface is down.
>
If you can passive scan while the interface is down, I don't think it's really 
down. In adm8211, nothing can be sent or received when the interface is down. 
The radio is always off when the interface is down. Taking the interface up 
just for a scan and then taking it back down doesn't sound too good either. I 
think scans should be prohibited while the interface is down, since leaving 
the interface on isn't gonna do anything bad unless you put in the info to 
associate.

-Michael Wu

^ permalink raw reply

* Re: [Xen-devel] [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Herbert Xu @ 2006-05-09 22:41 UTC (permalink / raw)
  To: Chris Wright
  Cc: linux-kernel, virtualization, Christian.Limpach, xen-devel,
	netdev, ian.pratt
In-Reply-To: <20060509085201.446830000@sous-sol.org>

Chris Wright <chrisw@sous-sol.org> wrote:
>
> +       netdev->features        = NETIF_F_IP_CSUM;

Any reason why IP_CSUM was chosen instead of HW_CSUM? Doing the latter
would seem to be in fact easier for a virtual driver, no?

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] netdev sysfs failure handling
From: David S. Miller @ 2006-05-09 22:43 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20060509144049.0924d41c@localhost.localdomain>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Tue, 9 May 2006 14:40:49 -0700

> Agreed, especially since rtnl is now a real mutex.  The case, that
> I was worried about:
> 	rtnl_lock()
> 	spin_lock_irq(&mylock);
> 	x = register_netdevice();
> ...
> 
> Doesn't show up in any current code, even for the pseudo devices
> and funny virtualized interfaces.

Right, therefore I think we should put something like your patch in
there now.... perhaps.

The case where we really needed the todo list is unregister, so that
we can safely wait for all references to the net device to go away.

I still wonder about those mentioned hotplug races wrt. linkwatch
in the comment above netdev_run_todo().

Linkwatch is such a nuissance because it combines asynchronous link
state change processing with keventd and RTNL locking.  It sleeps
waiting for __LINK_STATE_SCHED to clear with the RTNL held (via
dev_deactivate()).  But then again dev_close() code paths do this
too, so the dev_deactivate() bit should be OK.

Linkwatch, after doing the dev_activate(), emits a NETDEV_CHANGE
notifier on netdev_chain and also sends out an RTM_NETLINK
message.  This is for the case where IFF_UP is set.

Until we release the RTNL semaphore, during netdev register, nobody
can go in an inspect the state of a net device.  So doing the sysfs
node creation in register_netdevice() should be OK as far as I can
tell.

Can anyone find a problem with this?

^ permalink raw reply

* Re: [RFC] netdev sysfs failure handling
From: Stephen Hemminger @ 2006-05-09 22:50 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <20060509.154322.09145595.davem@davemloft.net>

On Tue, 09 May 2006 15:43:22 -0700 (PDT)
"David S. Miller" <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Tue, 9 May 2006 14:40:49 -0700
> 
> > Agreed, especially since rtnl is now a real mutex.  The case, that
> > I was worried about:
> > 	rtnl_lock()
> > 	spin_lock_irq(&mylock);
> > 	x = register_netdevice();
> > ...
> > 
> > Doesn't show up in any current code, even for the pseudo devices
> > and funny virtualized interfaces.
> 
> Right, therefore I think we should put something like your patch in
> there now.... perhaps.
> 
> The case where we really needed the todo list is unregister, so that
> we can safely wait for all references to the net device to go away.
> 
> I still wonder about those mentioned hotplug races wrt. linkwatch
> in the comment above netdev_run_todo().
> 
> Linkwatch is such a nuissance because it combines asynchronous link
> state change processing with keventd and RTNL locking.  It sleeps
> waiting for __LINK_STATE_SCHED to clear with the RTNL held (via
> dev_deactivate()).  But then again dev_close() code paths do this
> too, so the dev_deactivate() bit should be OK.
> 
> Linkwatch, after doing the dev_activate(), emits a NETDEV_CHANGE
> notifier on netdev_chain and also sends out an RTM_NETLINK
> message.  This is for the case where IFF_UP is set.
> 
> Until we release the RTNL semaphore, during netdev register, nobody
> can go in an inspect the state of a net device.  So doing the sysfs
> node creation in register_netdevice() should be OK as far as I can
> tell.
> 
> Can anyone find a problem with this?

Also, by getting the netdevice fully in sysfs under RTNL,
we are safe from races with the hotplug uevent that occurs.

Right now, it might be possible on SMP for the hotplug to happen
after register_netdevice, but before the device shows up
in sysfs.

^ permalink raw reply

* Re: [Xen-devel] [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Chris Wright @ 2006-05-09 23:35 UTC (permalink / raw)
  To: David Boutcher
  Cc: Christian Limpach, chrisw, Herbert Xu, ian.pratt, linux-kernel,
	netdev, virtualization, xen-devel
In-Reply-To: <OFE128D80F.BD59DF3E-ON86257169.004F91EA-86257169.004F6870@us.ibm.com>

* David Boutcher (boutcher@us.ibm.com) wrote:
> Then make a generic solution.  VMWare supports migration, the Power 
> virtualization will get around to it eventually.  All will need something
> similar.  So either make a common user-land tool, or (if you insist on
> incorrectly driving this into the kernel) add some kind of common hook to
> the TCP/IP stack.

I'm not that fond of the in-kernel solution either.  HA failover does
this stuff in userspace, and has the same gratuitous arp requirements.
Perhaps we should see some numbers showing the migration latency
introduced.  At the very least, it's easy to factor out as suggested.

thanks,
-chris

^ permalink raw reply

* Re: [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Chris Wright @ 2006-05-09 23:37 UTC (permalink / raw)
  To: Christoph Hellwig, Chris Wright, linux-kernel, virtualization,
	xen-devel, Ian Pratt, Christian Limpach, netdev
In-Reply-To: <20060509115826.GA2213@infradead.org>

* Christoph Hellwig (hch@infradead.org) wrote:
> On Tue, May 09, 2006 at 12:00:34AM -0700, Chris Wright wrote:
> > The network device frontend driver allows the kernel to access network
> > devices exported exported by a virtual machine containing a physical
> > network device driver.
> 
> Please don't add procfs code to new network drivers.  Especially if it's oopsable
> like the code in this driver by simple device renaming.

Agreed, no reason to keep the cruft around.  I thought I had a comment
of the sort in there.

thanks,
-chris

^ permalink raw reply

* Re: [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Chris Wright @ 2006-05-09 23:39 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: xen-devel, Ian Pratt, netdev, linux-kernel, Chris Wright,
	virtualization, Christian Limpach
In-Reply-To: <20060509115633.36b4879e@localhost.localdomain>

* Stephen Hemminger (shemminger@osdl.org) wrote:
> The stuff in /proc could easily just be added attributes to the class_device kobject
> of the net device (and then show up in sysfs).

Agreed, it's on the todo list to drop proc support there.  Thought that
was marked in the patch.

> > +#define GRANT_INVALID_REF	0
> > +
> > +#define NET_TX_RING_SIZE __RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE)
> > +#define NET_RX_RING_SIZE __RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE)
> > +
> > +static inline void init_skb_shinfo(struct sk_buff *skb)
> > +{
> > +	atomic_set(&(skb_shinfo(skb)->dataref), 1);
> > +	skb_shinfo(skb)->nr_frags = 0;
> > +	skb_shinfo(skb)->frag_list = NULL;
> > +}
> 
> Could you use existing sk_buff_head instead of inventing your
> own skb queue?

Hmm, there is some standard skb_queue_tail happening.  I don't have a
clear idea what you mean.

> > +	u8 mac[ETH_ALEN];
> 
> Isn't mac address already stored in dev->dev_addr and/or dev->perm_addr?

Yes, I don't see the reason to keep in twice.  It's basically a temp
buffer, but it certainly appears we can eliminate it.

thanks,
-chris

^ permalink raw reply

* Re: [Xen-devel] [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Chris Wright @ 2006-05-09 23:51 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Chris Wright, linux-kernel, virtualization, Christian.Limpach,
	xen-devel, netdev, ian.pratt
In-Reply-To: <E1FdatV-0000lj-00@gondolin.me.apana.org.au>

* Herbert Xu (herbert@gondor.apana.org.au) wrote:
> Chris Wright <chrisw@sous-sol.org> wrote:
> >
> > +       netdev->features        = NETIF_F_IP_CSUM;
> 
> Any reason why IP_CSUM was chosen instead of HW_CSUM? Doing the latter
> would seem to be in fact easier for a virtual driver, no?

That, I really don't know.

thanks,
-chris

^ permalink raw reply

* [PATCH] sis900 Foxconn 661FX7MI-S PHY support
From: James Cameron @ 2006-05-10  0:04 UTC (permalink / raw)
  To: venza, netdev

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

This patch is required to get sis900 ethernet working well on a Foxconn
661FX7MI-S motherboard which uses the SiS 661FX chipset.  The patch adds
an entry to mii_chip_info for the transceiver.  

The PHY ids were found using the sis900_c_122.diff patch from
http://brownhat.org/sis900.html but that patch didn't solve the problem,
because the PHY at address 1 was already being chosen.

Without my patch, when bursts of packets arrive from other hosts on a
LAN, the interface dropped one roughly 10% of the time, causing
retransmits.  There were fifth second pauses in refresh of large xterms,
and it made Netrek suck.  I can provide further test data.

Workaround in lieu of patch is to use mii-tool to advertise
100baseTx-HD, then force renegotiation.

I wasn't able to identify the actual transceiver, so the description
field is a guess.

This patch is similar to Artur Skawina's patch:
http://marc.theaimsgroup.com/?l=linux-netdev&m=114297516729079&w=2

I'm not sure, but I wonder if it means the default behaviour should be
changed, so as to better handle future transceivers.

Diff is against 2.6.16.13.

Signed-off-by: James Cameron <james.cameron@hp.com>

[-- Attachment #2: sis900_661fx.diff --]
[-- Type: text/plain, Size: 509 bytes --]

--- sis900.c.orig	2006-05-09 10:36:54.000000000 +1000
+++ sis900.c	2006-05-09 12:16:26.000000000 +1000
@@ -127,6 +127,7 @@
 } mii_chip_table[] = {
 	{ "SiS 900 Internal MII PHY", 		0x001d, 0x8000, LAN },
 	{ "SiS 7014 Physical Layer Solution", 	0x0016, 0xf830, LAN },
+	{ "SiS 900 on Foxconn 661 7MI",         0x0143, 0xBC70, LAN },
 	{ "Altimata AC101LF PHY",               0x0022, 0x5520, LAN },
 	{ "AMD 79C901 10BASE-T PHY",  		0x0000, 0x6B70, LAN },
 	{ "AMD 79C901 HomePNA PHY",		0x0000, 0x6B90, HOME},

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Olivier @ 2006-05-10  5:29 UTC (permalink / raw)
  To: Michael Wu; +Cc: Ivo van Doorn, netdev, Jiri Benc
In-Reply-To: <200605091836.10138.flamingice@sourmilk.net>

2006/5/10, Michael Wu <flamingice@sourmilk.net>:
> On Tuesday 09 May 2006 18:01, Ivo van Doorn wrote:
> > A user on the forums Olivier Cornu (added to the CC list) has done some
> > investigation into the scanning behaviour of the dscape stack.
> > Basicly the dscape stack is performing active scanning while the device is
> > down, but during the active scan it is sending packets out, or at least
> > attempting to do so. Besides the question if active scanning is preferred
> > over passive scanning while interface is down, active scanning fails
> > because the packets that should be send are being send through the regular
> > xmit routines of the interface. (IFF_UP is not set for the interface)
> >
> > This means that besides enabling the radio which should be done in the
> > driver, the stack should either bring up the interface when doing an active
> > scan, or resort to passive scanning while interface is down.
> >
> If you can passive scan while the interface is down, I don't think it's really
> down. In adm8211, nothing can be sent or received when the interface is down.
> The radio is always off when the interface is down. Taking the interface up
> just for a scan and then taking it back down doesn't sound too good either. I
> think scans should be prohibited while the interface is down, since leaving
> the interface on isn't gonna do anything bad unless you put in the info to
> associate.
>
> -Michael Wu
>

What do you mean exactly by "device is down, but not really down"?
I thought a network device was called "down" when the IFF_UP flag of
its net_device struct was set to 0 (thus not appearing in ifconfig,
among other things). Is there any other alternative understanding of
it being "down" i'm not aware of?

I confirm scanning actively with the device down is possible using
another NIC/driver. Thus I guess it's normal behaviour.
I still have to figure out how to do it using the dscape 802.11 stack
(if it's actually possible)...

Olivier Cornu

^ permalink raw reply

* Re: [RFC][SECMARK 03/08] Add xtables SECMARK target
From: Patrick McHardy @ 2006-05-10  6:03 UTC (permalink / raw)
  To: James Morris
  Cc: selinux, netdev, netfilter-devel, Stephen Smalley, Daniel J Walsh
In-Reply-To: <Pine.LNX.4.64.0605071134250.8588@d.namei>

James Morris wrote:
> This patch adds a SECMARK target to xtables, allowing
> the admin to apply security marks to packets via both
> iptables and ip6tables.
> 
> The target currently handles SELinux security marking,
> but can be extended for other purposes as needed.

The netfilter parts all look fine too me (just one question,
see below). Shall I add the userspace parts to SVN or do you
want to do it yourself?


> +static int checkentry_selinux(struct xt_secmark_target_info *info)
> +{
> +	int err;
> +	struct xt_secmark_target_selinux_info *sel = &info->u.sel;
> +
> +	err = selinux_string_to_sid(sel->selctx, &sel->selsid);
> +	if (err) {
> +		if (err == -EINVAL)
> +			printk(KERN_INFO PFX "invalid SELinux context \'%s\'\n",
> +			       sel->selctx);
> +		return 0;
> +	}
> +	
> +	if (!sel->selsid) {
> +		printk(KERN_INFO PFX "unable to map SELinux context \'%s\'\n",
> +		       sel->selctx);
> +		return 0;
> +	}
> +
> +	return 1;
> +}

I wonder if the result of this check could be invalidated later
by removal of the selinux context and if it would matter, since
you reject contexts not known at initialization time.


^ permalink raw reply

* Re: [Xen-devel] [RFC PATCH 34/35] Add the Xen virtual network device driver.
From: Keir Fraser @ 2006-05-10  6:36 UTC (permalink / raw)
  To: Chris Wright
  Cc: virtualization, linux-kernel, xen-devel, Herbert Xu, ian.pratt,
	netdev
In-Reply-To: <20060509235122.GJ24291@moss.sous-sol.org>


On 10 May 2006, at 00:51, Chris Wright wrote:

> * Herbert Xu (herbert@gondor.apana.org.au) wrote:
>> Chris Wright <chrisw@sous-sol.org> wrote:
>>>
>>> +       netdev->features        = NETIF_F_IP_CSUM;
>>
>> Any reason why IP_CSUM was chosen instead of HW_CSUM? Doing the latter
>> would seem to be in fact easier for a virtual driver, no?
>
> That, I really don't know.

Checksum offload was added late to the virtual transport and currently 
not enough info is carried to identify protocol checksum fields in 
arbitrary locations. When we rev the virtual interface, and include a 
proper checksum-offset field, we'll be able to switch to 
NETIF_F_HW_CSUM.

  -- Keir


^ permalink raw reply

* [PATCH] skb leakage in inet6_csk_xmit
From: Alexey Kuznetsov @ 2006-05-10  9:02 UTC (permalink / raw)
  To: netdev, davem

Hello!

inet6_csk_xit does not free skb when routing fails.

Signed-off-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>

diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index f8f3a37..eb2865d 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -173,6 +173,7 @@ int inet6_csk_xmit(struct sk_buff *skb, 
 
 		if (err) {
 			sk->sk_err_soft = -err;
+			kfree_skb(skb);
 			return err;
 		}
 
@@ -181,6 +182,7 @@ int inet6_csk_xmit(struct sk_buff *skb, 
 
 		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
 			sk->sk_route_caps = 0;
+			kfree_skb(skb);
 			return err;
 		}
 

^ permalink raw reply related

* Re: [Bugme-new] [Bug 6530] New: MAINLINE
From: Andrew Morton @ 2006-05-10  9:33 UTC (permalink / raw)
  To: bugme-daemon; +Cc: netdev, Paul Mackerras, xeb
In-Reply-To: <200605100920.k4A9KC91018259@fire-2.osdl.org>

bugme-daemon@bugzilla.kernel.org wrote:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=6530
> 
>            Summary: MAINLINE
>     Kernel Version: 2.6.16
>             Status: NEW
>           Severity: normal
>              Owner: jgarzik@pobox.com
>          Submitter: xeb@mail.ru
> 
> 
> Most recent kernel where this bug did not occur:
> Distribution:  gentoo 2005.1
> Hardware Environment: i386 (Intel(R) Celeron(R) M processor         1.40GHz), RAM 256M
> Software Environment:
> Problem Description:
> Hello.
> I tried to setup pptp (pptpd) service on my linux box, but it don't work correctly.
> Problem: traffic freezes when sending large amount of data.
> 
> I find lack in ppp_async.c. 
> 
> 	/* try to push more stuff out */
> 	if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap))
> 		ppp_output_wakeup(&ap->chan);
> ---->> I added this lines and pptpd starts work.
> 	else
> 	{
> 	    if (test_bit(XMIT_FULL, &ap->xmit_flags))
> 		ppp_asynctty_wakeup(ap->tty);
> 	}
> 

hm, a PPP fix.  We seem to need some of those lately.

Paul, does this look sane?


From: <xeb@mail.ru>

Adapted from  http://bugzilla.kernel.org/show_bug.cgi?id=6530

Reschedule the async Tx tasklet if the transmit queue was full.

Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/net/ppp_async.c |    2 ++
 1 files changed, 2 insertions(+)

diff -puN drivers/net/ppp_async.c~ppp_async-hang-fix drivers/net/ppp_async.c
--- devel/drivers/net/ppp_async.c~ppp_async-hang-fix	2006-05-10 02:28:15.000000000 -0700
+++ devel-akpm/drivers/net/ppp_async.c	2006-05-10 02:28:53.000000000 -0700
@@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l
 	/* try to push more stuff out */
 	if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap))
 		ppp_output_wakeup(&ap->chan);
+	else if (test_bit(XMIT_FULL, &ap->xmit_flags))
+		ppp_asynctty_wakeup(ap->tty);
 }
 
 /*
_


^ permalink raw reply

* Re: tcp compound
From: Angelo P. Castellani @ 2006-05-10 10:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20060509110118.197b9a92@localhost.localdomain>

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

On 5/9/06, Stephen Hemminger <shemminger@osdl.org> wrote:
> Moved discussion over to netdev mailing list..
>
> Could you export symbols in tcp_vegas (and change config dependencies) to
> allow code reuse rather than having to copy/paste everything from vegas?

I hope I've done that properly.

[-- Attachment #2: tcp_compound.patch.gz --]
[-- Type: application/x-gzip, Size: 4112 bytes --]

^ permalink raw reply

* Re: [Bugme-new] [Bug 6530] New: MAINLINE
From: Paul Mackerras @ 2006-05-10 10:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: bugme-daemon, netdev, xeb
In-Reply-To: <20060510023357.0d1260ee.akpm@osdl.org>

Andrew Morton writes:

> hm, a PPP fix.  We seem to need some of those lately.
> 
> Paul, does this look sane?

/me pages in 7 year old code...

> @@ -516,6 +516,8 @@ static void ppp_async_process(unsigned l
>  	/* try to push more stuff out */
>  	if (test_bit(XMIT_WAKEUP, &ap->xmit_flags) && ppp_async_push(ap))
>  		ppp_output_wakeup(&ap->chan);
> +	else if (test_bit(XMIT_FULL, &ap->xmit_flags))
> +		ppp_asynctty_wakeup(ap->tty);

ppp_asynctty_wakeup is supposed to be called by the serial driver when
it can take more output.  It's slightly bogus having ppp_async call it
itself whether or not the serial driver can take more output at the
moment, but I suppose it won't hurt.  I would really like to know the
precise circumstances where we need this fake wakeup though.  Is the
serial driver failing to give us a wakeup call where it should, or is
ppp_async ignoring a wakeup for some reason?

I think the same effect could be achieved without an extra trip
through tasklet_schedule et al. by making those lines look like this
(untested):

	if ((test_bit(XMIT_WAKEUP, &ap->xmit_flags) ||
             test_bit(XMIT_FULL, &ap->xmit_flags)) && ppp_async_push(ap))
		ppp_output_wakeup(&ap->chan);

so that ppp_async_push gets called if either XMIT_WAKEUP or XMIT_FULL
is set.

This is all relying on getting some input to kick off more output when
the wakeup gets missed, though.  That's a reasonable workaround in most
situations, I guess, but I'd really like to know why the wakeup is
getting missed.

Paul.

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Jiri Benc @ 2006-05-10 10:42 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: netdev
In-Reply-To: <200605061401.01795.IvDoorn@gmail.com>

On Sat, 6 May 2006 14:00:58 +0200, Ivo van Doorn wrote:
> In rt2x00 the config() handler schedules all configuration changes by using a workqueue,
> this is required since several configuration changes in rt2x00 need sleeping and for
> USB devices all register access requires sleeping. And the config() handler is often
> called from interrupt context so it complains a lot when the workqueue is not used.
> 
> This seemed fine, untill the radio_enabled field was introduced to the configuration structure.
> When the radio_enable field is set, the radio must be enabled, but enabling
> the radio is something that can (at least in rt2x00) fail. So scheduling the enabling of the radio
> to the workqueue is not something that is desired since the stack can not be notified that the
> device is not able to enable the radio.

This is probably more general problem. I can imagine a card that will
need to sleep to switch channels and can fail. This doesn't matter now
(as we cannot handle failure in switching channels) but it should be
solved in some way.

Currently, only switching channels is made in interrupt context. You can
depend on this for now.

> Moving the enabling of the radio outside the workqueue function and into the config()
> handler results in scheduling while atomic issues since the enabling of the radio requires
> sleeping for both PCI and USB devices.

Or am I wrong? I see no place when radio_enabled is changed in interrupt context.

> Instead of using a config field radio_enabled, wouldn't it be better to add 2 handlers
> to the ieee80211_hw structure, something like enable_radio() and disable_radio()?
> If these functions are called from normal context the dscape stack can still enable
> and disable the radio whenever it is desired, and it is able to check the return value
> to see if the request has actually succeeded.

I don't think this is the best idea. In such case we would need to move
almost all the stuff from ieee80211_conf to separate functions. This is
something we don't want.

> What I am wondering about afterwards is what exactly should happen when the open()
> and stop() handlers are being called? Because those are basicly intented to enable
> and stop the radio as well. I checked bcm43xx to see what they do, and they don't seem
> to check the radio_enabled field, so I don't know what they do besides enabling the radio.

open and stop callbacks are obsolete. They were replaced by
add_interface and remove_interface callbacks and will be removed after
drivers are converted to use the new callbacks.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Jiri Benc @ 2006-05-10 10:52 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: netdev, Olivier
In-Reply-To: <200605100001.20180.IvDoorn@gmail.com>

On Wed, 10 May 2006 00:01:16 +0200, Ivo van Doorn wrote:
> Basicly the dscape stack is performing active scanning while the device is down,
> but during the active scan it is sending packets out, or at least attempting to do so.
> Besides the question if active scanning is preferred over passive scanning while interface is down,
> active scanning fails because the packets that should be send are being send through the regular
> xmit routines of the interface. (IFF_UP is not set for the interface)

When all interfaces are down, the card must be completely disabled. No
transmitting, no receiving. Currently, there is a (minor) bug in IBSS
code in d80211 stack which tries to perform scanning even when the
interface is down. It's not so important because generated probe
requests are never delivered to the driver if the card is disabled (i.e.
no interfaces are up).

> This means that besides enabling the radio which should be done in the driver, the stack should either bring
> up the interface when doing an active scan, or resort to passive scanning while interface is down.

The stack should not perform any scanning when the interface is not up.

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply

* Re: Hardware flow control on RTL8169
From: s.munaut @ 2006-05-10 13:13 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev
In-Reply-To: <20060508194625.GA15174@electric-eye.fr.zoreil.com>

Hi Francois,

Selon Francois Romieu <romieu@fr.zoreil.com>:

> s.munaut@intopix.com <s.munaut@intopix.com> :
> [...]
> > I'm using a kurobox (www.kurobox.com) with a 2.6.15 kernel and I'd like to
> use
> > hardware flow control with it. However it seems the driver doesn't support
> it,
> > is that correct ?
> >
> > At least I see the device continue sending even when the other device is
> > sending PAUSE frames with 0xffff pausetime continuously.
> >
> > I've tried adding RTL_W16(TBI_ANAR, 0x00a0); to the hw_start function but
> > that doesn't seem to have any effect ...
> >
> > Any clue ?
>
> - use netdev@vger.kernel.org instead of netdev@oss.sgi.com

Sorry about that ... I don't know what version of MAINTAINER I looked at
to come up with that address ...

> - don't trim the Cc: (just a remainder...);
> - try the hack below with and without the first hunk. Use 'ethtool -s ethX'
>   to see what the device reports. It may not settle immediately.
>
>    [...diff snipped...]

Thanks, I'll try that.


Regards,

    Sylvain

^ permalink raw reply

* Re: [RFC][SECMARK 03/08] Add xtables SECMARK target
From: James Morris @ 2006-05-10 13:30 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: selinux, netdev, netfilter-devel, Stephen Smalley, Daniel J Walsh
In-Reply-To: <44618233.5020704@trash.net>

On Wed, 10 May 2006, Patrick McHardy wrote:

> The netfilter parts all look fine too me (just one question,
> see below). Shall I add the userspace parts to SVN or do you
> want to do it yourself?

Might be better if you do it, although I'm still looking into one issue at 
this stage.

> I wonder if the result of this check could be invalidated later
> by removal of the selinux context and if it would matter, since
> you reject contexts not known at initialization time.

If the context is removed later by a change to SELinux policy, the kernel 
will remap them to "unlabeled_t", which is how packets would then be 
labeled by the rule.


- James 
-- 
James Morris <jmorris@namei.org>

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Ivo van Doorn @ 2006-05-10 13:37 UTC (permalink / raw)
  To: Michael Wu; +Cc: netdev, Olivier, Jiri Benc
In-Reply-To: <200605091836.10138.flamingice@sourmilk.net>

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

On Wednesday 10 May 2006 00:36, Michael Wu wrote:
> On Tuesday 09 May 2006 18:01, Ivo van Doorn wrote:
> > A user on the forums Olivier Cornu (added to the CC list) has done some
> > investigation into the scanning behaviour of the dscape stack.
> > Basicly the dscape stack is performing active scanning while the device is
> > down, but during the active scan it is sending packets out, or at least
> > attempting to do so. Besides the question if active scanning is preferred
> > over passive scanning while interface is down, active scanning fails
> > because the packets that should be send are being send through the regular
> > xmit routines of the interface. (IFF_UP is not set for the interface)
> >
> > This means that besides enabling the radio which should be done in the
> > driver, the stack should either bring up the interface when doing an active
> > scan, or resort to passive scanning while interface is down.
> >
> If you can passive scan while the interface is down, I don't think it's really 
> down. In adm8211, nothing can be sent or received when the interface is down. 
> The radio is always off when the interface is down. Taking the interface up 
> just for a scan and then taking it back down doesn't sound too good either. I 
> think scans should be prohibited while the interface is down, since leaving 
> the interface on isn't gonna do anything bad unless you put in the info to 
> associate.

True, I agree here. But when rt2x00 was using the ipw stack a much
requested feature from users was to be able to perform scanning while
interface was down. (The requests did not specify if it they wanted passive or active scanning)
But the reason for these requests was that some user space applications and/or
distribution network scripts relied on the fact that the driver was capable of scanning
while the interface was down, so the interface would only be brought up at boot time,
when there was no desired AP in the scan result.
I don't know which distributions or user space application these were, but due to the amount
of these requests it had been considered an important feature.

Ivo

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Ivo van Doorn @ 2006-05-10 13:53 UTC (permalink / raw)
  To: Jiri Benc; +Cc: netdev, Olivier
In-Reply-To: <20060510124217.0d3810bd@griffin.suse.cz>

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

On Wednesday 10 May 2006 12:42, Jiri Benc wrote:
> On Sat, 6 May 2006 14:00:58 +0200, Ivo van Doorn wrote:
> > In rt2x00 the config() handler schedules all configuration changes by using a workqueue,
> > this is required since several configuration changes in rt2x00 need sleeping and for
> > USB devices all register access requires sleeping. And the config() handler is often
> > called from interrupt context so it complains a lot when the workqueue is not used.
> > 
> > This seemed fine, untill the radio_enabled field was introduced to the configuration structure.
> > When the radio_enable field is set, the radio must be enabled, but enabling
> > the radio is something that can (at least in rt2x00) fail. So scheduling the enabling of the radio
> > to the workqueue is not something that is desired since the stack can not be notified that the
> > device is not able to enable the radio.
> 
> This is probably more general problem. I can imagine a card that will
> need to sleep to switch channels and can fail. This doesn't matter now
> (as we cannot handle failure in switching channels) but it should be
> solved in some way.
> 
> Currently, only switching channels is made in interrupt context. You can
> depend on this for now.
> 
> > Moving the enabling of the radio outside the workqueue function and into the config()
> > handler results in scheduling while atomic issues since the enabling of the radio requires
> > sleeping for both PCI and USB devices.
> 
> Or am I wrong? I see no place when radio_enabled is changed in interrupt context.

I think the problem is what the dscape exactly expects the driver to do when
add_interface() is called by the stack. When that call has finished, does the stack
expects the radio to be enabled, or should it instruct the driver to enable the radio later
on through a field in the config structure.

What currently happens in rt2x00 (Which might not be doing what the stack expects)
is that when add_interface() is called the radio remains off untill open() is called or
radio_enabled has been set in the config structure.
What happens is that the first call of config() (which is being made before open()) comes
from the scan handler requesting a channel change, which is in interrupt context, but
when the channel is changed, the radio is also supposed to be enabled which results
in the problems of radio enabling requirement to be scheduled.

But this might be resolved by demanding that the driver is keeping track of the number
of interfaces added, and enabling the radio when at least 1 interface has been added
and only disable the radio when all interfaces have been removed. And completely
remove the open() and stop() handlers since they would only confuse matters.

> > Instead of using a config field radio_enabled, wouldn't it be better to add 2 handlers
> > to the ieee80211_hw structure, something like enable_radio() and disable_radio()?
> > If these functions are called from normal context the dscape stack can still enable
> > and disable the radio whenever it is desired, and it is able to check the return value
> > to see if the request has actually succeeded.
> 
> I don't think this is the best idea. In such case we would need to move
> almost all the stuff from ieee80211_conf to separate functions. This is
> something we don't want.

True, but I actually meant that these 2 functions would replace the open() and stop() functions.
Which is not required if the above suggestion of the number of added interfaces counter
is applied to the drivers.

> > What I am wondering about afterwards is what exactly should happen when the open()
> > and stop() handlers are being called? Because those are basicly intented to enable
> > and stop the radio as well. I checked bcm43xx to see what they do, and they don't seem
> > to check the radio_enabled field, so I don't know what they do besides enabling the radio.
> 
> open and stop callbacks are obsolete. They were replaced by
> add_interface and remove_interface callbacks and will be removed after
> drivers are converted to use the new callbacks.

Ah ok, I'll fix rt2x00 then to do the correct behaviour and no longer rely on open() and stop()
for the radio, and add the counter for the number of interfaces that are up so the radio
will behave in the correct fashion. :)

Thanks.

Ivo

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Jiri Benc @ 2006-05-10 13:55 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: Michael Wu, netdev, Olivier
In-Reply-To: <200605101537.15213.IvDoorn@gmail.com>

On Wed, 10 May 2006 15:37:11 +0200, Ivo van Doorn wrote:
> True, I agree here. But when rt2x00 was using the ipw stack a much
> requested feature from users was to be able to perform scanning while
> interface was down. (The requests did not specify if it they wanted passive or active scanning)
> But the reason for these requests was that some user space applications and/or
> distribution network scripts relied on the fact that the driver was capable of scanning
> while the interface was down, so the interface would only be brought up at boot time,
> when there was no desired AP in the scan result.

Such applications and scripts are broken. Let your users fill bug
reports to their distribution bugzillas.

> I don't know which distributions or user space application these were, but due to the amount
> of these requests it had been considered an important feature.

I wouldn't consider breaking the way the whole Linux networking works as
a feature.

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Ivo van Doorn @ 2006-05-10 13:55 UTC (permalink / raw)
  To: Jiri Benc; +Cc: netdev, Olivier
In-Reply-To: <20060510125241.0bb0978b@griffin.suse.cz>

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

On Wednesday 10 May 2006 12:52, Jiri Benc wrote:
> On Wed, 10 May 2006 00:01:16 +0200, Ivo van Doorn wrote:
> > Basicly the dscape stack is performing active scanning while the device is down,
> > but during the active scan it is sending packets out, or at least attempting to do so.
> > Besides the question if active scanning is preferred over passive scanning while interface is down,
> > active scanning fails because the packets that should be send are being send through the regular
> > xmit routines of the interface. (IFF_UP is not set for the interface)
> 
> When all interfaces are down, the card must be completely disabled. No
> transmitting, no receiving. Currently, there is a (minor) bug in IBSS
> code in d80211 stack which tries to perform scanning even when the
> interface is down. It's not so important because generated probe
> requests are never delivered to the driver if the card is disabled (i.e.
> no interfaces are up).
> 
> > This means that besides enabling the radio which should be done in the driver, the stack should either bring
> > up the interface when doing an active scan, or resort to passive scanning while interface is down.
> 
> The stack should not perform any scanning when the interface is not up.

Ok thanks. Along with the previous mail about the add_interface and remove_interface this
clears up some misunderstanding about the scanning behaviour. ;)

Ivo

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Dscape ieee80211: enabling/disabling the radio
From: Jiri Benc @ 2006-05-10 14:10 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: netdev, Olivier
In-Reply-To: <200605101553.52031.IvDoorn@gmail.com>

On Wed, 10 May 2006 15:53:48 +0200, Ivo van Doorn wrote:
> I think the problem is what the dscape exactly expects the driver to do when
> add_interface() is called by the stack. When that call has finished, does the stack
> expects the radio to be enabled, or should it instruct the driver to enable the radio later
> on through a field in the config structure.

At least for now, enabled if this is the first interface going up. Of
course, if a second (or so) interface is added, you shouldn't reenable
radio if it was disabled by previous config() call.

> But this might be resolved by demanding that the driver is keeping track of the number
> of interfaces added, and enabling the radio when at least 1 interface has been added
> and only disable the radio when all interfaces have been removed. And completely
> remove the open() and stop() handlers since they would only confuse matters.

Yes, this is the way it is supposed to work.

> Ah ok, I'll fix rt2x00 then to do the correct behaviour and no longer rely on open() and stop()
> for the radio, and add the counter for the number of interfaces that are up so the radio
> will behave in the correct fashion. :)

Please see http://www.spinics.net/lists/netdev/msg03975.html (and
following thread probably as well) for a description how to implement
this correctly.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

^ 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