* [PATCH 1/5] d80211: per-queue TX flow control
From: Jiri Benc @ 2006-06-12 19:16 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville, Michael Buesch
In-Reply-To: <20060612211454.409884000.midnight@suse.cz>
Currently, before a packet is passed to the driver, the driver is asked
about status of its TX queues (i.e. how many packets are queued in each
queue and how large are queues).
This is different from the way generic networking works in Linux and it
doesn't allow easy implementation of resubmitting fragments to the driver
when the queue gets filled up during transmitting.
This patch changes the stack not to ask driver about queue status but
require driver to do TX flow control.
Please note that this breaks drivers.
Signed-off-by: Jiri Benc <jbenc@suse.cz>
---
include/net/d80211.h | 30 ++++++++++++++++++++++++++++++
net/d80211/ieee80211.c | 27 +++++++++++++++++++++++++++
net/d80211/ieee80211_i.h | 6 ++++++
net/d80211/wme.c | 6 +-----
4 files changed, 64 insertions(+), 5 deletions(-)
--- dscape.orig/include/net/d80211.h
+++ dscape/include/net/d80211.h
@@ -781,6 +781,7 @@ int ieee80211_get_hdrlen(u16 fc);
* netdevices for each hardware device. The low-level driver does not "see"
* these interfaces, so it should use this function to perform netif
* operations on all interface. */
+/* This function is deprecated. */
typedef enum {
NETIF_ATTACH, NETIF_DETACH, NETIF_START, NETIF_STOP, NETIF_WAKE,
NETIF_IS_STOPPED, NETIF_UPDATE_TX_START
@@ -788,6 +789,35 @@ typedef enum {
int ieee80211_netif_oper(struct net_device *dev, Netif_Oper op);
/**
+ * ieee80211_wake_queue - wake specific queue
+ * @dev: pointer to &struct net_device as obtained from
+ * ieee80211_alloc_hw().
+ * @queue: queue number (counted from zero).
+ *
+ * Drivers should use this function instead of netif_wake_queue.
+ */
+void ieee80211_wake_queue(struct net_device *dev, int queue);
+
+/**
+ * ieee80211_stop_queue - stop specific queue
+ * @dev: pointer to &struct net_device as obtained from
+ * ieee80211_alloc_hw().
+ * @queue: queue number (counted from zero).
+ *
+ * Drivers should use this function instead of netif_stop_queue.
+ */
+void ieee80211_stop_queue(struct net_device *dev, int queue);
+
+/**
+ * ieee80211_start_queues - start all queues
+ * @dev: pointer to &struct net_device as obtained from
+ * ieee80211_alloc_hw().
+ *
+ * Drivers should use this function instead of netif_start_queue.
+ */
+void ieee80211_start_queues(struct net_device *dev);
+
+/**
* ieee80211_get_mc_list_item - iteration over items in multicast list
* @dev: pointer to &struct net_device as obtained from
* ieee80211_alloc_hw().
--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -4421,6 +4421,30 @@ int ieee80211_netif_oper(struct net_devi
return 0;
}
+void ieee80211_wake_queue(struct net_device *dev, int queue)
+{
+ struct ieee80211_local *local = dev->ieee80211_ptr;
+
+ if (test_and_clear_bit(IEEE80211_LINK_STATE_XOFF,
+ &local->state[queue]))
+ __netif_schedule(dev);
+}
+
+void ieee80211_stop_queue(struct net_device *dev, int queue)
+{
+ struct ieee80211_local *local = dev->ieee80211_ptr;
+
+ set_bit(IEEE80211_LINK_STATE_XOFF, &local->state[queue]);
+}
+
+void ieee80211_start_queues(struct net_device *dev)
+{
+ struct ieee80211_local *local = dev->ieee80211_ptr;
+ int i;
+
+ for (i = 0; i < local->hw->queues; i++)
+ clear_bit(IEEE80211_LINK_STATE_XOFF, &local->state[i]);
+}
void * ieee80211_dev_hw_data(struct net_device *dev)
{
@@ -4548,6 +4572,9 @@ EXPORT_SYMBOL(ieee80211_tx_status);
EXPORT_SYMBOL(ieee80211_beacon_get);
EXPORT_SYMBOL(ieee80211_get_buffered_bc);
EXPORT_SYMBOL(ieee80211_netif_oper);
+EXPORT_SYMBOL(ieee80211_wake_queue);
+EXPORT_SYMBOL(ieee80211_stop_queue);
+EXPORT_SYMBOL(ieee80211_start_queues);
EXPORT_SYMBOL(ieee80211_dev_hw_data);
EXPORT_SYMBOL(ieee80211_dev_stats);
EXPORT_SYMBOL(ieee80211_get_hw_conf);
--- dscape.orig/net/d80211/ieee80211_i.h
+++ dscape/net/d80211/ieee80211_i.h
@@ -353,6 +353,8 @@ struct ieee80211_local {
struct sta_info *sta_hash[STA_HASH_SIZE];
struct timer_list sta_cleanup;
+ unsigned long state[NUM_TX_DATA_QUEUES];
+
int mc_count; /* total count of multicast entries in all interfaces */
int iff_allmultis, iff_promiscs;
/* number of interfaces with corresponding IFF_ flags */
@@ -514,6 +516,10 @@ struct ieee80211_local {
int user_space_mlme;
};
+enum ieee80211_link_state_t {
+ IEEE80211_LINK_STATE_XOFF = 0,
+};
+
struct sta_attribute {
struct attribute attr;
ssize_t (*show)(const struct sta_info *, char *buf);
--- dscape.orig/net/d80211/wme.c
+++ dscape/net/d80211/wme.c
@@ -316,18 +316,14 @@ static struct sk_buff *wme_qdiscop_deque
struct net_device *dev = qd->dev;
struct ieee80211_local *local = dev->ieee80211_ptr;
struct ieee80211_hw *hw = local->hw;
- struct ieee80211_tx_queue_stats stats;
struct sk_buff *skb;
struct Qdisc *qdisc;
int queue;
- /* find which hardware queues have space in them */
- hw->get_tx_stats(dev, &stats);
-
/* check all the h/w queues in numeric/priority order */
for (queue = 0; queue < hw->queues; queue++) {
/* see if there is room in this hardware queue */
- if (stats.data[queue].len >= stats.data[queue].limit)
+ if (test_bit(IEEE80211_LINK_STATE_XOFF, &local->state[queue]))
continue;
/* there is space - try and get a frame */
^ permalink raw reply
* [incomplete 2/5] bcm43xx-d80211: per-queue TX flow control
From: Jiri Benc @ 2006-06-12 19:16 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville, Michael Buesch
In-Reply-To: <20060612211454.409884000.midnight@suse.cz>
This is an attempt to fix bcm43xx driver. It is for DMA mode only and
incomplete even for that mode - ieee80211_hw->tx() callback should return
NETDEV_TX_* constants which is not completely fixed by this patch.
---
drivers/net/wireless/d80211/bcm43xx/bcm43xx_dma.c | 6 +++++-
drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
--- dscape.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_dma.c
+++ dscape/drivers/net/wireless/d80211/bcm43xx/bcm43xx_dma.c
@@ -778,13 +778,16 @@ int bcm43xx_dma_tx(struct bcm43xx_privat
* recognizes if the device queue is full and does
* not send data anymore.
*/
+ ieee80211_stop_queue(bcm->net_dev, 0);
printk(KERN_ERR PFX "DMA queue overflow\n");
- return -ENOMEM;
+ return NETDEV_TX_BUSY;
}
err = dma_tx_fragment(ring, skb, ctl);
if (likely(!err))
ring->nr_tx_packets++;
+ if (free_slots(ring) < SLOTS_PER_PACKET)
+ ieee80211_stop_queue(bcm->net_dev, 0);
return err;
}
@@ -833,6 +836,7 @@ void bcm43xx_dma_handle_xmitstatus(struc
slot = next_slot(ring, slot);
}
bcm->stats.last_tx = jiffies;
+ ieee80211_wake_queue(bcm->net_dev, 0);
}
void bcm43xx_dma_get_tx_stats(struct bcm43xx_private *bcm,
--- dscape.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
+++ dscape/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
@@ -3737,6 +3737,7 @@ static int bcm43xx_init_board(struct bcm
bcm43xx_security_init(bcm);
bcm43xx_measure_channel_change_time(bcm);
ieee80211_update_hw(bcm->net_dev, bcm->ieee);
+ ieee80211_start_queues(bcm->net_dev);
ieee80211_netif_oper(bcm->net_dev, NETIF_ATTACH);
ieee80211_netif_oper(bcm->net_dev, NETIF_START);
ieee80211_netif_oper(bcm->net_dev, NETIF_WAKE);
^ permalink raw reply
* [PATCH 0/5] d80211: better fragmentation handling
From: Jiri Benc @ 2006-06-12 19:16 UTC (permalink / raw)
To: netdev; +Cc: John W. Linville, Michael Buesch
Following patches allow proper handling of situation when hw queue gets
filled up while sending 802.11 fragments.
This breaks drivers; an example how to fix them is in second patch. Also,
get_tx_stats callback is optional now.
--
Jiri Benc
SUSE Labs
^ permalink raw reply
* Re: [PATCH] netpoll: break recursive loop in netpoll rx path
From: Neil Horman @ 2006-06-12 19:13 UTC (permalink / raw)
To: Matt Mackall; +Cc: netdev, jmoyer
In-Reply-To: <20060612155120.GO24227@waste.org>
On Mon, Jun 12, 2006 at 10:51:21AM -0500, Matt Mackall wrote:
> On Mon, Jun 12, 2006 at 11:40:29AM -0400, Neil Horman wrote:
> > Hey there-
> > the netpoll system currently has a rx to tx path via:
> > netpoll_rx
> > __netpoll_rx
> > arp_reply
> > netpoll_send_skb
> > dev->hard_start_tx
> >
> > This rx->tx loop places network drivers at risk of
> > inadvertently causing a deadlock or BUG halt by recursively trying
> > to acquire a spinlock that is used in both their rx and tx paths
> > (this problem was origionally reported to me in the 3c59x driver,
> > which shares a spinlock between the boomerang_interrupt and
> > boomerang_start_xmit routines).
>
> Grumble.
>
> > This patch breaks this loop, by queueing arp frames, so that they
> > can be responded to after all receive operations have been
> > completed. Tested by myself and the reported with successful
> > results.
>
> Tested how? kgdb?
>
Specifically It was tested with netdump. Heres the BZ with details:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=194055
> > + if (likely(npi))
> > + while ((skb = skb_dequeue(&npi->arp_tx)) != NULL)
> > + arp_reply(skb);
> > +
>
> Assignment inside tests is frowned upon. I'd prefer pulling this out
> to its own function too.
>
Sure, no problem. New patch attached with suggested modifications made
Regards
Neil
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
include/linux/netpoll.h | 1 +
net/core/netpoll.c | 27 +++++++++++++++++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
--- linux-2.6/include/linux/netpoll.h.orig 2006-06-12 09:11:01.000000000 -0400
+++ linux-2.6/include/linux/netpoll.h 2006-06-12 09:34:17.000000000 -0400
@@ -31,6 +31,7 @@ struct netpoll_info {
int rx_flags;
spinlock_t rx_lock;
struct netpoll *rx_np; /* netpoll that registered an rx_hook */
+ struct sk_buff_head arp_tx; /* list of arp requests to reply to */
};
void netpoll_poll(struct netpoll *np);
--- linux-2.6/net/core/netpoll.c.orig 2006-06-12 09:11:01.000000000 -0400
+++ linux-2.6/net/core/netpoll.c 2006-06-12 13:43:25.000000000 -0400
@@ -54,6 +54,7 @@ static atomic_t trapped;
sizeof(struct iphdr) + sizeof(struct ethhdr))
static void zap_completion_queue(void);
+static void arp_reply(struct sk_buff *skb);
static void queue_process(void *p)
{
@@ -153,8 +154,25 @@ static void poll_napi(struct netpoll *np
}
}
+static void service_arp_queue(struct netpoll_info *npi)
+{
+ struct sk_buff *skb;
+
+ if(unlikely(!npi))
+ return;
+
+ skb = skb_dequeue(&npi->arp_tx);
+
+ while (skb != NULL) {
+ arp_reply(skb);
+ skb = skb_dequeue(&npi->arp_tx);
+ }
+ return;
+}
+
void netpoll_poll(struct netpoll *np)
{
+
if(!np->dev || !netif_running(np->dev) || !np->dev->poll_controller)
return;
@@ -163,6 +181,8 @@ void netpoll_poll(struct netpoll *np)
if (np->dev->poll)
poll_napi(np);
+ service_arp_queue(np->dev->npinfo);
+
zap_completion_queue();
}
@@ -449,7 +469,9 @@ int __netpoll_rx(struct sk_buff *skb)
int proto, len, ulen;
struct iphdr *iph;
struct udphdr *uh;
- struct netpoll *np = skb->dev->npinfo->rx_np;
+ struct netpoll_info *npi = skb->dev->npinfo;
+ struct netpoll *np = npi->rx_np;
+
if (!np)
goto out;
@@ -459,7 +481,7 @@ int __netpoll_rx(struct sk_buff *skb)
/* check if netpoll clients need ARP */
if (skb->protocol == __constant_htons(ETH_P_ARP) &&
atomic_read(&trapped)) {
- arp_reply(skb);
+ skb_queue_tail(&npi->arp_tx, skb);
return 1;
}
@@ -654,6 +676,7 @@ int netpoll_setup(struct netpoll *np)
npinfo->poll_owner = -1;
npinfo->tries = MAX_RETRIES;
spin_lock_init(&npinfo->rx_lock);
+ skb_queue_head_init(&npinfo->arp_tx);
} else
npinfo = ndev->npinfo;
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* [PATCH] d80211: update tx.skb after TX handler calls
From: Jiri Benc @ 2006-06-12 19:09 UTC (permalink / raw)
To: NetDev; +Cc: John W. Linville
TX and RX handlers are allowed to change skb. Fix (hopefully) the last place
where this is not taken into account.
Signed-off-by: Jiri Benc <jbenc@suse.cz>
---
net/d80211/ieee80211.c | 1 +
1 files changed, 1 insertion(+)
--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -1760,6 +1760,7 @@ ieee80211_get_buffered_bc(struct net_dev
break;
}
dev_put(tx.dev);
+ skb = tx.skb; /* handlers are allowed to change skb */
if (res == TXRX_DROP) {
I802_DEBUG_INC(local->tx_handlers_drop);
^ permalink raw reply
* Re: [PATCH] wan/sdla section fixes
From: Krzysztof Halasa @ 2006-06-12 19:05 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: netdev, mike.mclagan, akpm
In-Reply-To: <20060612110523.300a3420.rdunlap@xenotime.net>
"Randy.Dunlap" <rdunlap@xenotime.net> writes:
>> > static const char* version = "SDLA driver v0.30, 12 Sep 1996,
>> > mike.mclagan@linux.org";
>>
>> 1996 doesn't look encouraging but it may be misleading.
>
> Yep. You could ignore it :) or rm drivers/net/wan/slda* :)
I don't know, maybe Mike will say something?
--
Krzysztof Halasa
^ permalink raw reply
* Re: [RFT] Realtek 8168 ethernet support
From: Francois Romieu @ 2006-06-12 18:49 UTC (permalink / raw)
To: Mourad De Clerck; +Cc: netdev
In-Reply-To: <448D4236.9050505@aquazul.com>
Mourad De Clerck <mourad@aquazul.com> :
[...]
> I just tried this patch set, but it doesn't do anything for the "freeze
> at high speed" I mentioned on 2006-06-09. It still locks up. (As an
> additional data point: I installed win2k on this machine, and it seems
> to have no problems transferring at high speeds. Just wanted to try to
> rule out faulty hardware.)
Please send .config and complete dmesg (starting with 'Linux version ...').
The output of a 'vmstat 1' until it freezes could give some hint.
So could trying a different PCI slot. How do you generate traffic ?
15Mo/s of usual traffic means roughly 1000pps. It is not really high speed.
Unrelated: have you checked the link setting ?
--
Ueimor
^ permalink raw reply
* Re: [PATCH 2.6.17-rc6] Remove Prism II support from Orinoco
From: Faidon Liambotis @ 2006-06-12 15:39 UTC (permalink / raw)
To: John W. Linville; +Cc: Dave Jones, netdev, proski, hermes
In-Reply-To: <20060612152434.GA14851@tuxdriver.com>
On Mon, Jun 12, 2006 at 11:24:39AM -0400, John W. Linville wrote:
> On Mon, Jun 12, 2006 at 01:49:54AM +0300, Faidon Liambotis wrote:
>
> > Having two drivers supporting the same set of hardware seems pretty
> > pointless to me. Plus, it confuses hotplugging/automatic detection.
>
> This subject comes-up from time to time. In fact, I'm pretty sure
> it came-up very recently w.r.t. orinoco and hostap.
I remember a patch that added all of Orinoco PCI IDs to HostAP. I'm not
sure if you're referring to that, but that's pretty different (and
obviously wrong).
> The consensus seems to be that drivers should have IDs for all devices
> they support, even if that means that some devices are supported by
> multiple drivers. This leaves the choice of which driver to use in
> the hands of the user and/or distro.
I'd mostly agree to that if distributors had a way to enable/disable
Prism2 support on the orinoco_cs driver based on a build-time
configuration option.
Should I prepare such a patch?
FWIW, I think we've experienced a similar situation like this in the
past in the networking land and the consensus was to completely remove
the other driver. I'm referring to e100/eepro100, of course.
Regards,
Faidon
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Neil Horman @ 2006-06-12 18:06 UTC (permalink / raw)
To: Mitch Williams
Cc: Jeff Moyer, Kok, Auke-jan H, Matt Mackall, Garzik, Jeff, netdev,
Brandeburg, Jesse, Kok, Auke
In-Reply-To: <1150130534.2879.9.camel@strongmad>
On Mon, Jun 12, 2006 at 09:42:14AM -0700, Mitch Williams wrote:
> On Sun, 2006-06-11 at 17:13 -0700, Neil Horman wrote:
> > Any further thoughts on this guys? I still think my last solution
> > solves all of
> > the netpoll problems, and isn't going to have any noticable impact on
> > performance.
> >
> I haven't had time to evaluate performance on your patch (sorry!), but
> after thinking about it, I agree that it should not have any noticeable
> impact. OTOH, performance tuning is a funny thing, and things you think
> won't cause problems often do.
>
Thats ok, I just didn't hear out of anyone on friday, so I was curious as to
where we were on this. I don't have the ability to do any real world
performance testing here, but I'll try to record the run time of the interrupt
routine on a limited number of frames here.
> Anyway, I'm still not quite ready to ACK this because it's just not
> future-proof. Eventually, we will need to support multiple RX queues,
> and this solution will not work in that situation.
>
Not sure I understand this. My patch:
http://marc.theaimsgroup.com/?l=linux-netdev&m=114970506406155&w=2
still allows for the use of multiple rx
queues in the nominal case. Only when we have to use a relatively slow netpoll
driven operation (kgdb, netdump, etc), do we need to receive on the same
interface that we transmit on.
> A simpler short-term solution is just to schedule our NAPI polling on
> the "real" netdev instead of our polling netdev. This is a trivial
> change and works correctly with a single queue. But, like your patch,
> it isn't future-proof.
>
Again, not sure what you mean here. My last patch proposal:
http://marc.theaimsgroup.com/?l=linux-netdev&m=114970807606096&w=2
Does precisely what you describe, but it still allows for multiple rx queues in
the nominal (non poll_controller driven) receive case.
> So, I'm still thinking and pondering on this one.
>
> If we get a patch in to fix the recursive loop in netpoll, my original
> patch will work, right? Or is there still another issue?
>
I assume you are referring to this patch:
http://marc.theaimsgroup.com/?l=linux-netdev&m=114970506406155&w=2
If so, then no, that patch is still broken for the reasons Jeff outlined
previously, The recursion patch that I proposed earlier today is related to a
different recursion problem, and while the e1000 driver might be suceptable to
it, your patch is also suceptible to the data corruption that arises from when
the poll_controller calls adapter->clean_rx at the same that that dev->poll is
called for the same adapter on another cpu. If that happens we can have two
cpu's writing to the same private net_device data without the benefit of a
spinlock to protect them. And yes, you can add a spin lock to protect the case
where you have a dev->poll_controller and a dev->poll operation at the same
time, but that seems to me like it will also re-serialize all the parallel
operations that you could otherwise do with multiple rx queues
I'll post again if I get a chance to do some performance measurements
Regrds
Neil
> -Mitch
> -
> 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
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: [PATCH] wan/sdla section fixes
From: Randy.Dunlap @ 2006-06-12 18:05 UTC (permalink / raw)
To: Krzysztof Halasa; +Cc: netdev, mike.mclagan, akpm
In-Reply-To: <m3d5de461t.fsf@defiant.localdomain>
On Mon, 12 Jun 2006 19:50:22 +0200 Krzysztof Halasa wrote:
> Hi,
>
> "Randy.Dunlap" <rdunlap@xenotime.net> writes:
>
> > Priority: tossup.
> > netdev->set_config can be called at any time, so these references
> > to __initdata would be a real problem.
> > However, problem has not been observed AFAIK.
> >
> > Fix section mismatch warnings:
> > WARNING: drivers/net/wan/sdla.o - Section mismatch: reference to .init.data: from .text between 'sdla_set_config' (at offset 0x1b8e) and 'sdla_stats'
> > WARNING: drivers/net/wan/sdla.o - Section mismatch: reference to .init.data: from .text between 'sdla_set_config' (at offset 0x1e76) and 'sdla_stats'
>
> Is sdla.c still in use? I remember someone mentioned that Sangoma
> drivers are now outside the kernel but I don't know how do they
> relate to sdla.c and friend(s).
No idea.
> > static const char* version = "SDLA driver v0.30, 12 Sep 1996,
> > mike.mclagan@linux.org";
>
> 1996 doesn't look encouraging but it may be misleading.
Yep. You could ignore it :) or rm drivers/net/wan/slda* :)
---
~Randy
^ permalink raw reply
* Re: [PATCH] wan/sdla section fixes
From: Krzysztof Halasa @ 2006-06-12 17:50 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: netdev, mike.mclagan, akpm
In-Reply-To: <20060608203616.16796ec3.rdunlap@xenotime.net>
Hi,
"Randy.Dunlap" <rdunlap@xenotime.net> writes:
> Priority: tossup.
> netdev->set_config can be called at any time, so these references
> to __initdata would be a real problem.
> However, problem has not been observed AFAIK.
>
> Fix section mismatch warnings:
> WARNING: drivers/net/wan/sdla.o - Section mismatch: reference to .init.data: from .text between 'sdla_set_config' (at offset 0x1b8e) and 'sdla_stats'
> WARNING: drivers/net/wan/sdla.o - Section mismatch: reference to .init.data: from .text between 'sdla_set_config' (at offset 0x1e76) and 'sdla_stats'
Is sdla.c still in use? I remember someone mentioned that Sangoma
drivers are now outside the kernel but I don't know how do they
relate to sdla.c and friend(s).
> static const char* version = "SDLA driver v0.30, 12 Sep 1996,
> mike.mclagan@linux.org";
1996 doesn't look encouraging but it may be misleading.
--
Krzysztof Halasa
^ permalink raw reply
* Re: The AI parameter of tcp_highspeed.c (in 2.6.18)
From: Xiaoliang (David) Wei @ 2006-06-12 17:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: John Heffner, netdev
In-Reply-To: <20060602120507.7e1206d8@dxpl.pdx.osdl.net>
Thanks, Stephen. But I think there is still a problem with the AIMD
parameter update in HighSpeed TCP code.
Line 125~138 of the code (net/ipv4/tcp_highspeed.c):
/* Update AIMD parameters */
if (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd) {
while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
ca->ai < HSTCP_AIMD_MAX - 1)
ca->ai++;
} else if (tp->snd_cwnd < hstcp_aimd_vals[ca->ai].cwnd) {
while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
ca->ai > 0)
ca->ai--;
In fact, the second part (decreasing ca->ai) never decreases since the
while loop's inequality is in the reverse direction. This leads to
unfairness with multiple flows (once a flow happens to enjoy a higher
ca->ai, it keeps enjoying that even its cwnd decreases)
Here is a tentative fix (I also added a comment, trying to keep the
change clear):
--- /home/weixl/linux-2.6.16.18/net/ipv4/tcp_highspeed.c 2006-05-22
11:04:35.000000000 -0700
+++ /home/weixl/linux-2.6.16.18/net/ipv4/tcp_highspeed.c.new 2006-06-12
09:56:40.000000000 -0700
@@ -123,13 +123,13 @@
tcp_slow_start(tp);
else {
/* Update AIMD parameters */
+ /* We want to guarantee that hstcp_aimd_vals[ca->ai-1].cwnd <
snd_cwnd <= hstcp_aimd_vals[ca->ai].cwnd */
if (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd) {
while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
ca->ai < HSTCP_AIMD_MAX - 1)
ca->ai++;
- } else if (tp->snd_cwnd < hstcp_aimd_vals[ca->ai].cwnd) {
- while (tp->snd_cwnd > hstcp_aimd_vals[ca->ai].cwnd &&
- ca->ai > 0)
+ } else if (ca->ai && tp->snd_cwnd <= hstcp_aimd_vals[ca->ai-1].cwnd) {
+ while (ca->ai && tp->snd_cwnd <= hstcp_aimd_vals[ca->ai-1].cwnd)
ca->ai--;
}
Thanks.
-David
On 6/2/06, Stephen Hemminger <shemminger@osdl.org> wrote:
> Went backed and looked at the RFC. The problem was just a simple
> translation of table to C array (0 based). Added this to the TCP testing repository.
>
> Subject: [PATCH] Problem observed by Xiaoliang (David) Wei:
>
> When snd_cwnd is smaller than 38 and the connection is in
> congestion avoidance phase (snd_cwnd > snd_ssthresh), the snd_cwnd
> seems to stop growing.
>
> The additive increase was confused because C array's are 0 based.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> ---
>
> net/ipv4/tcp_highspeed.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> 121685b7b61c8faeb87e6c6f0c346b0fe1c46fd2
> diff --git a/net/ipv4/tcp_highspeed.c b/net/ipv4/tcp_highspeed.c
> index b72fa55..ba7c63c 100644
> --- a/net/ipv4/tcp_highspeed.c
> +++ b/net/ipv4/tcp_highspeed.c
> @@ -135,7 +135,8 @@ static void hstcp_cong_avoid(struct sock
>
> /* Do additive increase */
> if (tp->snd_cwnd < tp->snd_cwnd_clamp) {
> - tp->snd_cwnd_cnt += ca->ai;
> + /* cwnd = cwnd + a(w) / cwnd */
> + tp->snd_cwnd_cnt += ca->ai + 1;
> if (tp->snd_cwnd_cnt >= tp->snd_cwnd) {
> tp->snd_cwnd_cnt -= tp->snd_cwnd;
> tp->snd_cwnd++;
> --
> 1.3.3
>
>
--
Xiaoliang (David) Wei Graduate Student, CS@Caltech
http://davidwei.org
***********************************************
^ permalink raw reply
* Heads up: additional work on SELinux xfrm
From: Venkat Yekkirala @ 2006-06-12 16:47 UTC (permalink / raw)
To: netdev; +Cc: Chad Hanson, Darrel Goeddel
Hello,
This is just a heads up for anyone interested that I have submitted a patch
set that involves core networking and the xfrm subsystem to the selinux
list. I plan on submitting it to this list as soon as it gets properly
reviewed on the SELinux side.
I am shooting for the 2.6.18 window.
Broad description/overview of changes at:
http://marc.theaimsgroup.com/?l=selinux&m=115012769919108&w=2
Core networking changes at:
http://marc.theaimsgroup.com/?l=selinux&m=115012770004292&w=2
xfrm subsystem changes at:
http://marc.theaimsgroup.com/?l=selinux&m=115012769903910&w=2
Thanks,
venkat
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Mitch Williams @ 2006-06-12 16:42 UTC (permalink / raw)
To: Neil Horman
Cc: Jeff Moyer, Kok, Auke-jan H, Matt Mackall, Garzik, Jeff, netdev,
Brandeburg, Jesse, Kok, Auke
In-Reply-To: <20060612001356.GA5112@localhost.localdomain>
On Sun, 2006-06-11 at 17:13 -0700, Neil Horman wrote:
> Any further thoughts on this guys? I still think my last solution
> solves all of
> the netpoll problems, and isn't going to have any noticable impact on
> performance.
>
I haven't had time to evaluate performance on your patch (sorry!), but
after thinking about it, I agree that it should not have any noticeable
impact. OTOH, performance tuning is a funny thing, and things you think
won't cause problems often do.
Anyway, I'm still not quite ready to ACK this because it's just not
future-proof. Eventually, we will need to support multiple RX queues,
and this solution will not work in that situation.
A simpler short-term solution is just to schedule our NAPI polling on
the "real" netdev instead of our polling netdev. This is a trivial
change and works correctly with a single queue. But, like your patch,
it isn't future-proof.
So, I'm still thinking and pondering on this one.
If we get a patch in to fix the recursive loop in netpoll, my original
patch will work, right? Or is there still another issue?
-Mitch
^ permalink raw reply
* Re: [PATCH 1/2] PHYLIB: Add get_link ethtool helper function
From: Nathaniel Case @ 2006-06-12 16:12 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andy Fleming, netdev, galak
In-Reply-To: <448C4131.3000504@garzik.org>
On Sun, 2006-06-11 at 12:13 -0400, Jeff Garzik wrote:
> > This adds a phy_ethtool_get_link() function along the same lines as
> > phy_ethtool_gset(). This provides drivers utilizing PHYLIB an
> > alternative to using ethtool_op_get_link(). This is more desirable
> > since the "Link detected" field in ethtool would actually reflect the
> > state of the PHY register.
> >
[snip]
> NAK, needs an EXPORT
In that case, shouldn't we EXPORT phy_ethtool_sset() and
phy_ethtool_gset() in drivers/net/phy/phy.c as well?
- Nate Case <ncase@xes-inc.com>
^ permalink raw reply
* Re: [PATCH] netpoll: break recursive loop in netpoll rx path
From: Matt Mackall @ 2006-06-12 15:51 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, jmoyer
In-Reply-To: <20060612154029.GA3790@hmsreliant.homelinux.net>
On Mon, Jun 12, 2006 at 11:40:29AM -0400, Neil Horman wrote:
> Hey there-
> the netpoll system currently has a rx to tx path via:
> netpoll_rx
> __netpoll_rx
> arp_reply
> netpoll_send_skb
> dev->hard_start_tx
>
> This rx->tx loop places network drivers at risk of
> inadvertently causing a deadlock or BUG halt by recursively trying
> to acquire a spinlock that is used in both their rx and tx paths
> (this problem was origionally reported to me in the 3c59x driver,
> which shares a spinlock between the boomerang_interrupt and
> boomerang_start_xmit routines).
Grumble.
> This patch breaks this loop, by queueing arp frames, so that they
> can be responded to after all receive operations have been
> completed. Tested by myself and the reported with successful
> results.
Tested how? kgdb?
> + if (likely(npi))
> + while ((skb = skb_dequeue(&npi->arp_tx)) != NULL)
> + arp_reply(skb);
> +
Assignment inside tests is frowned upon. I'd prefer pulling this out
to its own function too.
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply
* Re: [PATCH v2 4/7] AMSO1100 Memory Management.
From: Tom Tucker @ 2006-06-12 16:05 UTC (permalink / raw)
To: Andrew Morton
Cc: Steve Wise, rdreier, mshefty, linux-kernel, netdev,
openib-general
In-Reply-To: <20060608011744.1a66e85a.akpm@osdl.org>
On Thu, 2006-06-08 at 01:17 -0700, Andrew Morton wrote:
> On Wed, 07 Jun 2006 15:06:55 -0500
> Steve Wise <swise@opengridcomputing.com> wrote:
>
> >
> > +void c2_free(struct c2_alloc *alloc, u32 obj)
> > +{
> > + spin_lock(&alloc->lock);
> > + clear_bit(obj, alloc->table);
> > + spin_unlock(&alloc->lock);
> > +}
>
> The spinlock is unneeded here.
Good point.
>
>
> What does all the code in this file do, anyway? It looks totally generic
> (and hence inappropriate for drivers/infiniband/hw/amso1100/) and somewhat
> similar to idr trees, perhaps.
>
We mimicked the mthca driver. It may be code that should be replaced
with Linux core services for new drivers. We'll investigate.
> > +int c2_array_set(struct c2_array *array, int index, void *value)
> > +{
> > + int p = (index * sizeof(void *)) >> PAGE_SHIFT;
> > +
> > + /* Allocate with GFP_ATOMIC because we'll be called with locks held. */
> > + if (!array->page_list[p].page)
> > + array->page_list[p].page =
> > + (void **) get_zeroed_page(GFP_ATOMIC);
> > +
> > + if (!array->page_list[p].page)
> > + return -ENOMEM;
>
> This _will_ happen under load. What will the result of that be, in the
> context of thise driver?
A higher level object allocation will fail. In this case, a kernel
application request will fail and the application must handle the error.
>
> This function is incorrectly designed - it should receive a gfp_t argument.
> Because you don't *know* that the caller will always hold a spinlock. And
> GFP_KERNEL is far, far stronger than GFP_ATOMIC.
This service is allocating a page that the adapter will DMA 2B message
indices into.
>
> > +static int c2_alloc_mqsp_chunk(gfp_t gfp_mask, struct sp_chunk **head)
> > +{
> > + int i;
> > + struct sp_chunk *new_head;
> > +
> > + new_head = (struct sp_chunk *) __get_free_page(gfp_mask | GFP_DMA);
>
> Why is __GFP_DMA in there? Unless you've cornered the ISA bus infiniband
> market, it's likely to be wrong.
>
Flag confusion about what GFP_DMA means. We'll revisit this whole
file ...
>
> -
> 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
^ permalink raw reply
* Re: [PATCH v2 1/2] iWARP Connection Manager.
From: Tom Tucker @ 2006-06-12 15:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Steve Wise, rdreier, mshefty, linux-kernel, netdev,
openib-general
In-Reply-To: <20060608005452.087b34db.akpm@osdl.org>
Andrew, thanks for the review, comments inline...
On Thu, 2006-06-08 at 00:54 -0700, Andrew Morton wrote:
> On Wed, 07 Jun 2006 15:06:05 -0500
> Steve Wise <swise@opengridcomputing.com> wrote:
>
> >
> > This patch provides the new files implementing the iWARP Connection
> > Manager.
> >
> > Review Changes:
> >
> > - sizeof -> sizeof()
> >
> > - removed printks
> >
> > - removed TT debug code
> >
> > - cleaned up lock/unlock around switch statements.
> >
> > - waitqueue -> completion for destroy path.
> >
> > ...
> >
> > +/*
> > + * This function is called on interrupt context. Schedule events on
> > + * the iwcm_wq thread to allow callback functions to downcall into
> > + * the CM and/or block. Events are queued to a per-CM_ID
> > + * work_list. If this is the first event on the work_list, the work
> > + * element is also queued on the iwcm_wq thread.
> > + *
> > + * Each event holds a reference on the cm_id. Until the last posted
> > + * event has been delivered and processed, the cm_id cannot be
> > + * deleted.
> > + */
> > +static void cm_event_handler(struct iw_cm_id *cm_id,
> > + struct iw_cm_event *iw_event)
> > +{
> > + struct iwcm_work *work;
> > + struct iwcm_id_private *cm_id_priv;
> > + unsigned long flags;
> > +
> > + work = kmalloc(sizeof(*work), GFP_ATOMIC);
> > + if (!work)
> > + return;
>
> This allocation _will_ fail sometimes. The driver must recover from it.
> Will it do so?
Er...no. It will lose this event. Depending on the event...the carnage
varies. We'll take a look at this.
>
> > +EXPORT_SYMBOL(iw_cm_init_qp_attr);
>
> This file exports a ton of symbols. It's usual to provide some justifying
> commentary in the changelog when this happens.
This module is a logical instance of the xx_cm where xx is the transport
type. I think there is some discussion warranted on whether or not these
should all be built into and exported by rdma_cm. One rationale would be
that the rdma_cm is the only client for many of these functions (this
being a particularly good example) and doing so would reduce the export
count. Others would be reasonably needed for any application (connect,
etc...)
All that said, we'll be sure to document the exported symbols in a
follow-up patch.
>
> > +/*
> > + * Copyright (c) 2005 Network Appliance, Inc. All rights reserved.
> > + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> > + *
> > + * This software is available to you under a choice of one of two
> > + * licenses. You may choose to be licensed under the terms of the GNU
> > + * General Public License (GPL) Version 2, available from the file
> > + * COPYING in the main directory of this source tree, or the
> > + * OpenIB.org BSD license below:
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > + * without modification, are permitted provided that the following
> > + * conditions are met:
> > + *
> > + * - Redistributions of source code must retain the above
> > + * copyright notice, this list of conditions and the following
> > + * disclaimer.
> > + *
> > + * - Redistributions in binary form must reproduce the above
> > + * copyright notice, this list of conditions and the following
> > + * disclaimer in the documentation and/or other materials
> > + * provided with the distribution.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +#if !defined(IW_CM_PRIVATE_H)
> > +#define IW_CM_PRIVATE_H
>
> We normally use #ifndef here.
We'll change this..
>
>
> -
> 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
^ permalink raw reply
* Re: Using netconsole for debugging suspend/resume
From: Andi Kleen @ 2006-06-12 15:46 UTC (permalink / raw)
To: Mark Lord
Cc: Jeremy Fitzhardinge, Matt Mackall, Linux Kernel Mailing List,
netdev
In-Reply-To: <448D8A90.3040508@rtr.ca>
On Monday 12 June 2006 17:38, Mark Lord wrote:
> Andi Kleen wrote:
> > On Friday 09 June 2006 17:24, Mark Lord wrote:
> >> Andi Kleen wrote:
> >>> If your laptop has firewire you can also use firescope.
> >>> (ftp://ftp.suse.com/pub/people/ak/firescope/)
> >> ..
> >>> FW keeps running as long as nobody resets the ieee1394 chip.
> >> This looks interesting. But how does one set it up for use
> >> on the *other* end of that firewire cable? The Quickstart and
> >> manpage don't seem to describe this fully.
> >
> > It's in the manpage:
> >
> >> .SH NOTES
> >> The target must have the ohci1394 driver loaded. This implies
> >> that firescope cannot be used in early boot.
> >
> > That's it.
>
> Okay, so I'm daft. But.. *what* is "it" ??
>
> We have two machines: target (being debugged), and host (anything).
> Sure, the target has to have ohci1394 loaded, and firescope running.
> But what about the *other* end of the connection? What commands?
>From the same manpage:
"The raw1394 module must be loaded and its device node
be writable (this normally requires root)"
Ok it doesn't say you need ohci1394 too and doesn't say that's the target.
If I do a new revision I'll perhaps expand the docs a bit.
So load ohci1394/raw1394 and run firescope as root. Your distribution
will hopefully take care of the device nodes. Usually you want
something like firescope -Au System.map
-Andi
>
^ permalink raw reply
* [PATCH] netpoll: break recursive loop in netpoll rx path
From: Neil Horman @ 2006-06-12 15:40 UTC (permalink / raw)
To: netdev; +Cc: mpm, jmoyer
Hey there-
the netpoll system currently has a rx to tx path via:
netpoll_rx
__netpoll_rx
arp_reply
netpoll_send_skb
dev->hard_start_tx
This rx->tx loop places network drivers at risk of inadvertently causing
a deadlock or BUG halt by recursively trying to acquire a spinlock that is used
in both their rx and tx paths (this problem was origionally reported to me in
the 3c59x driver, which shares a spinlock between the boomerang_interrupt and
boomerang_start_xmit routines). This patch breaks this loop, by queueing arp
frames, so that they can be responded to after all receive operations have been
completed. Tested by myself and the reported with successful results.
Regards
Neil
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
include/linux/netpoll.h | 1 +
net/core/netpoll.c | 17 +++++++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
--- linux-2.6/include/linux/netpoll.h.orig 2006-06-12 09:11:01.000000000 -0400
+++ linux-2.6/include/linux/netpoll.h 2006-06-12 09:34:17.000000000 -0400
@@ -31,6 +31,7 @@ struct netpoll_info {
int rx_flags;
spinlock_t rx_lock;
struct netpoll *rx_np; /* netpoll that registered an rx_hook */
+ struct sk_buff_head arp_tx; /* list of arp requests to reply to */
};
void netpoll_poll(struct netpoll *np);
--- linux-2.6/net/core/netpoll.c.orig 2006-06-12 09:11:01.000000000 -0400
+++ linux-2.6/net/core/netpoll.c 2006-06-12 11:10:13.000000000 -0400
@@ -54,6 +54,7 @@ static atomic_t trapped;
sizeof(struct iphdr) + sizeof(struct ethhdr))
static void zap_completion_queue(void);
+static void arp_reply(struct sk_buff *skb);
static void queue_process(void *p)
{
@@ -155,14 +156,23 @@ static void poll_napi(struct netpoll *np
void netpoll_poll(struct netpoll *np)
{
+ struct sk_buff *skb;
+ struct netpoll_info *npi;
+
if(!np->dev || !netif_running(np->dev) || !np->dev->poll_controller)
return;
+ npi = np->dev->npinfo;
+
/* Process pending work on NIC */
np->dev->poll_controller(np->dev);
if (np->dev->poll)
poll_napi(np);
+ if (likely(npi))
+ while ((skb = skb_dequeue(&npi->arp_tx)) != NULL)
+ arp_reply(skb);
+
zap_completion_queue();
}
@@ -449,7 +459,9 @@ int __netpoll_rx(struct sk_buff *skb)
int proto, len, ulen;
struct iphdr *iph;
struct udphdr *uh;
- struct netpoll *np = skb->dev->npinfo->rx_np;
+ struct netpoll_info *npi = skb->dev->npinfo;
+ struct netpoll *np = npi->rx_np;
+
if (!np)
goto out;
@@ -459,7 +471,7 @@ int __netpoll_rx(struct sk_buff *skb)
/* check if netpoll clients need ARP */
if (skb->protocol == __constant_htons(ETH_P_ARP) &&
atomic_read(&trapped)) {
- arp_reply(skb);
+ skb_queue_tail(&npi->arp_tx, skb);
return 1;
}
@@ -654,6 +666,7 @@ int netpoll_setup(struct netpoll *np)
npinfo->poll_owner = -1;
npinfo->tries = MAX_RETRIES;
spin_lock_init(&npinfo->rx_lock);
+ skb_queue_head_init(&npinfo->arp_tx);
} else
npinfo = ndev->npinfo;
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: Using netconsole for debugging suspend/resume
From: Mark Lord @ 2006-06-12 15:38 UTC (permalink / raw)
To: Andi Kleen
Cc: Jeremy Fitzhardinge, Matt Mackall, Linux Kernel Mailing List,
netdev
In-Reply-To: <200606121321.30388.ak@suse.de>
Andi Kleen wrote:
> On Friday 09 June 2006 17:24, Mark Lord wrote:
>> Andi Kleen wrote:
>>> If your laptop has firewire you can also use firescope.
>>> (ftp://ftp.suse.com/pub/people/ak/firescope/)
>> ..
>>> FW keeps running as long as nobody resets the ieee1394 chip.
>> This looks interesting. But how does one set it up for use
>> on the *other* end of that firewire cable? The Quickstart and
>> manpage don't seem to describe this fully.
>
> It's in the manpage:
>
>> .SH NOTES
>> The target must have the ohci1394 driver loaded. This implies
>> that firescope cannot be used in early boot.
>
> That's it.
Okay, so I'm daft. But.. *what* is "it" ??
We have two machines: target (being debugged), and host (anything).
Sure, the target has to have ohci1394 loaded, and firescope running.
But what about the *other* end of the connection? What commands?
Thanks
^ permalink raw reply
* Re: [PATCH 2.6.17-rc6] Remove Prism II support from Orinoco
From: John W. Linville @ 2006-06-12 15:24 UTC (permalink / raw)
To: Faidon Liambotis; +Cc: Dave Jones, netdev, proski, hermes
In-Reply-To: <20060611224954.GA2880@divinity>
On Mon, Jun 12, 2006 at 01:49:54AM +0300, Faidon Liambotis wrote:
> Having two drivers supporting the same set of hardware seems pretty
> pointless to me. Plus, it confuses hotplugging/automatic detection.
This subject comes-up from time to time. In fact, I'm pretty sure
it came-up very recently w.r.t. orinoco and hostap.
The consensus seems to be that drivers should have IDs for all devices
they support, even if that means that some devices are supported by
multiple drivers. This leaves the choice of which driver to use in
the hands of the user and/or distro.
If the Orinoco guys want this patch, I'll consider it. Otherwise,
I'm not inclined to take it.
John
--
John W. Linville
linville@tuxdriver.com
^ permalink raw reply
* [PATCH] bcm43xx: suspend MAC while executing long pwork
From: Michael Buesch @ 2006-06-12 15:02 UTC (permalink / raw)
To: John Linville; +Cc: bcm43xx-dev, netdev
Hi John,
Please queue this for 2.6.18.
--
Suspend MAC (and make MAC-suspend refcounting) when doing
long periodic work.
On long periodic work, we disable IRQs on the device, so
we don't want the MAC to stay operating and probably miss
packets due do non-delivery of interrupts.
Signed-off-by: Michael Buesch <mb@bu3sch.de>
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-06-05 18:19:59.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-06-12 16:20:28.000000000 +0200
@@ -723,6 +723,8 @@
u32 irq_savedstate;
/* Link Quality calculation context. */
struct bcm43xx_noise_calculation noisecalc;
+ /* if > 0 MAC is suspended. if == 0 MAC is enabled. */
+ int mac_suspended;
/* Threshold values. */
//TODO: The RTS thr has to be _used_. Currently, it is only set via WX.
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c 2006-06-05 20:17:36.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c 2006-06-12 16:41:41.000000000 +0200
@@ -2284,13 +2284,17 @@
/* http://bcm-specs.sipsolutions.net/EnableMac */
void bcm43xx_mac_enable(struct bcm43xx_private *bcm)
{
- bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
- | BCM43xx_SBF_MAC_ENABLED);
- bcm43xx_write32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON, BCM43xx_IRQ_READY);
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD); /* dummy read */
- bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
- bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
+ bcm->mac_suspended--;
+ assert(bcm->mac_suspended >= 0);
+ if (bcm->mac_suspended == 0) {
+ bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
+ bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
+ | BCM43xx_SBF_MAC_ENABLED);
+ bcm43xx_write32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON, BCM43xx_IRQ_READY);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD); /* dummy read */
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
+ bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
+ }
}
/* http://bcm-specs.sipsolutions.net/SuspendMAC */
@@ -2299,18 +2303,23 @@
int i;
u32 tmp;
- bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
- bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
- & ~BCM43xx_SBF_MAC_ENABLED);
- bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
- for (i = 100000; i; i--) {
- tmp = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
- if (tmp & BCM43xx_IRQ_READY)
- return;
- udelay(10);
+ assert(bcm->mac_suspended >= 0);
+ if (bcm->mac_suspended == 0) {
+ bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
+ bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
+ bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
+ & ~BCM43xx_SBF_MAC_ENABLED);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
+ for (i = 100000; i; i--) {
+ tmp = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
+ if (tmp & BCM43xx_IRQ_READY)
+ goto out;
+ udelay(10);
+ }
+ printkl(KERN_ERR PFX "MAC suspend failed\n");
}
- printkl(KERN_ERR PFX "MAC suspend failed\n");
+out:
+ bcm->mac_suspended++;
}
void bcm43xx_set_iwmode(struct bcm43xx_private *bcm,
@@ -3168,8 +3177,10 @@
/* Periodic work will take a long time, so we want it to
* be preemtible.
*/
- bcm43xx_lock_irqonly(bcm, flags);
netif_stop_queue(bcm->net_dev);
+ synchronize_net();
+ bcm43xx_lock_irqonly(bcm, flags);
+ bcm43xx_mac_suspend(bcm);
if (bcm43xx_using_pio(bcm))
bcm43xx_pio_freeze_txqueues(bcm);
savedirqs = bcm43xx_interrupt_disable(bcm, BCM43xx_IRQ_ALL);
@@ -3192,6 +3203,7 @@
bcm43xx_interrupt_enable(bcm, savedirqs);
if (bcm43xx_using_pio(bcm))
bcm43xx_pio_thaw_txqueues(bcm);
+ bcm43xx_mac_enable(bcm);
}
netif_wake_queue(bcm->net_dev);
mmiowb();
@@ -3774,6 +3786,7 @@
bcm->softmac->set_channel = bcm43xx_ieee80211_set_chan;
bcm->irq_savedstate = BCM43xx_IRQ_INITIAL;
+ bcm->mac_suspended = 1;
bcm->pci_dev = pci_dev;
bcm->net_dev = net_dev;
bcm->bad_frames_preempt = modparam_bad_frames_preempt;
--
Greetings Michael.
^ permalink raw reply
* Re: link-local address via ifconfig
From: Ingo Oeser @ 2006-06-12 14:02 UTC (permalink / raw)
To: Anand Kumria; +Cc: netdev
In-Reply-To: <20060603025743.GL549@progsoc.uts.edu.au>
Hi Anand,
Anand Kumria wrote:
> There are plenty of people who still use ifconfig to list the addresses
> assigned to their network interfaces (I know, ifconfig is broken) and
> who then parse the output.
>
> However the kernel puts link-local scoped address first if the address
> list of an interface, so an interface like:
> Is there any reason to put the link-local address first in the list?
>
> I've had a number of bugreports (or outright panic attacks) where the
> problem turned out to be that ifconfig was reporting the link-local
> address first, rather than the global/site one.
This can be worked around by defining an "alias interface" this way
"ip address add 169.254.182.108/16 brd 169.254.255.255 scope link dev eth0 label eth0:0"
(see: "label eth0:0" is appended)
So in reality this is no problem for users.
Regards
Ingo Oeser
^ permalink raw reply
* Re: Using netconsole for debugging suspend/resume
From: Andi Kleen @ 2006-06-12 11:21 UTC (permalink / raw)
To: Mark Lord
Cc: Jeremy Fitzhardinge, Matt Mackall, Linux Kernel Mailing List,
netdev
In-Reply-To: <448992B7.1050906@rtr.ca>
On Friday 09 June 2006 17:24, Mark Lord wrote:
> Andi Kleen wrote:
> >
> > If your laptop has firewire you can also use firescope.
> > (ftp://ftp.suse.com/pub/people/ak/firescope/)
> ..
> > FW keeps running as long as nobody resets the ieee1394 chip.
>
> This looks interesting. But how does one set it up for use
> on the *other* end of that firewire cable? The Quickstart and
> manpage don't seem to describe this fully.
It's in the manpage:
>.SH NOTES
>The target must have the ohci1394 driver loaded. This implies
>that firescope cannot be used in early boot.
That's it.
-Andi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox