* [2.6.17-git22] lock debugging output
From: Alessandro Suardi @ 2006-07-04 18:13 UTC (permalink / raw)
To: Linux Kernel, netdev
Hoping gmail doesn't mess it too badly...
eth0: tg3 (BCM5751 Gbit Ethernet)
eth1: ipw2200 (Intel PRO/Wireless 2200BG)
Sequence:
1. boot with eth0 disconnected (eth1 doesn't come up on boot)
2. ifup eth1, bring wpa-supplicant up
3. run 'dig' ---> <lock debug info gets printed on console>
Note that due to my very variable network setup, I had no /etc/resolv.conf
in place at the moment I ran 'dig'. Second execution of 'dig' did not print
any lock debug output but just (properly) stalled; then I realized I didn't
put my home resolv.conf in place, did that and 'dig' just worked.
System appears to work and I'm actually typing this report from the
same kernel that reported the following upon invoking 'dig' :
=================================
[ INFO: inconsistent lock state ]
---------------------------------
inconsistent {softirq-on-W} -> {in-softirq-R} usage.
dig/2373 [HC0[0]:SC1[2]:HE1:SE0] takes:
(&sk->sk_dst_lock){---?}, at: [<c028cf72>] sk_dst_check+0x1b/0xe6
{softirq-on-W} state was registered at:
[<c0127a6a>] lock_acquire+0x60/0x80
[<c02e151d>] _write_lock+0x19/0x28
[<c028c0af>] sock_setsockopt+0x351/0x49c
[<c0289d0d>] sys_setsockopt+0x5b/0x8d
[<c028ac22>] sys_socketcall+0x148/0x186
[<c0102699>] sysenter_past_esp+0x56/0x8d
irq event stamp: 1130
hardirqs last enabled at (1130): [<c01161ed>] local_bh_enable_ip+0xb2/0xbb
hardirqs last disabled at (1129): [<c011618e>] local_bh_enable_ip+0x53/0xbb
softirqs last enabled at (1120): [<c029423c>] dev_queue_xmit+0x205/0x211
softirqs last disabled at (1121): [<c01040e6>] do_softirq+0x4d/0xac
other info that might help us debug this:
2 locks held by dig/2373:
#0: (sk_lock-AF_INET6){--..}, at: [<f8cf1168>]
udpv6_sendmsg+0x546/0x818 [ipv6]
#1: (slock-AF_INET6){-...}, at: [<f8cf3228>] icmpv6_send+0x222/0x549 [ipv6]
stack backtrace:
[<c0102e44>] show_trace+0xd/0x10
[<c010335e>] dump_stack+0x19/0x1b
[<c01260e1>] print_usage_bug+0x1cc/0x1d9
[<c01265e2>] mark_lock+0x193/0x360
[<c01271ee>] __lock_acquire+0x3b7/0x969
[<c0127a6a>] lock_acquire+0x60/0x80
[<c02e15ff>] _read_lock+0x19/0x28
[<c028cf72>] sk_dst_check+0x1b/0xe6
[<f8ce1305>] ip6_dst_lookup+0x31/0x16d [ipv6]
[<f8cf3338>] icmpv6_send+0x332/0x549 [ipv6]
[<f8cf09a1>] udpv6_rcv+0x4ab/0x4d6 [ipv6]
[<f8ce2900>] ip6_input+0x19c/0x228 [ipv6]
[<f8ce2d61>] ipv6_rcv+0x188/0x1b7 [ipv6]
[<c02925b7>] netif_receive_skb+0x18d/0x1d8
[<c0293d6a>] process_backlog+0x80/0xf9
[<c0293f43>] net_rx_action+0x80/0x174
[<c01162fd>] __do_softirq+0x46/0x9c
[<c01040e6>] do_softirq+0x4d/0xac
=======================
[<c0116117>] local_bh_enable+0xc8/0xec
[<c029423c>] dev_queue_xmit+0x205/0x211
[<c0298a8b>] neigh_resolve_output+0x1db/0x207
[<f8ce0bee>] ip6_output2+0x1e4/0x202 [ipv6]
[<f8ce12aa>] ip6_output+0x69e/0x6c8 [ipv6]
[<f8ce1706>] ip6_push_pending_frames+0x2c5/0x377 [ipv6]
[<f8cefd8e>] udp_v6_push_pending_frames+0x154/0x176 [ipv6]
[<f8cf122a>] udpv6_sendmsg+0x608/0x818 [ipv6]
[<c02c6b1d>] inet_sendmsg+0x3b/0x48
[<c02894f9>] sock_sendmsg+0xe8/0x103
[<c0289b18>] sys_sendmsg+0x14f/0x1aa
[<c028ac45>] sys_socketcall+0x16b/0x186
[<c0102699>] sysenter_past_esp+0x56/0x8d
Hope this may be useful to lock debug devs / netdev folks...
Ciao,
--alessandro
"I can't change what makes me high and I can't change what I believe in"
(Heather Nova, "My Fidelity")
^ permalink raw reply
* Re: [PATCH 2/4] d80211: fix receiving through virtual interfaces
From: Jiri Benc @ 2006-07-04 18:05 UTC (permalink / raw)
To: netdev
In-Reply-To: <20060703172408.C9183483BB@silver.suse.cz>
On Mon, 3 Jul 2006 19:24:08 +0200 (CEST), Jiri Benc wrote:
> - Packet type (PACKET_HOST and PACKET_OTHER_HOST) is set correctly now.
Uhm, not really.
> @@ -3057,7 +3048,9 @@ ieee80211_rx_h_check(struct ieee80211_tx
> return TXRX_DROP;
> }
>
> - if (memcmp(rx->dev->dev_addr, hdr->addr1, ETH_ALEN) == 0)
> + if (rx->fc & WLAN_FC_TODS)
> + rx->skb->pkt_type = PACKET_OTHERHOST;
I'm not sure how something so obviously wrong slipped there.
The corrected version of the patch follows.
---
net/d80211/ieee80211.c | 171 +++++++++++++++++++++++++++++++----------------
net/d80211/ieee80211_i.h | 5 +
net/d80211/wpa.c | 4 +
3 files changed, 124 insertions(+), 56 deletions(-)
--- dscape.orig/net/d80211/ieee80211.c
+++ dscape/net/d80211/ieee80211.c
@@ -2463,27 +2463,15 @@ ieee80211_rx_h_data(struct ieee80211_txr
memcpy(ehdr->h_source, src, ETH_ALEN);
ehdr->h_proto = len;
}
-
- if (rx->sta && !rx->sta->assoc_ap &&
- !(rx->sta && (rx->sta->flags & WLAN_STA_WDS)))
- skb->dev = rx->sta->dev;
- else
- skb->dev = dev;
+ skb->dev = dev;
skb2 = NULL;
- sdata = IEEE80211_DEV_TO_SUB_IF(dev);
- /*
- * don't count the master since the low level code
- * counts it already for us.
- */
- if (skb->dev != sdata->master) {
- sdata->stats.rx_packets++;
- sdata->stats.rx_bytes += skb->len;
- }
+ sdata->stats.rx_packets++;
+ sdata->stats.rx_bytes += skb->len;
if (local->bridge_packets && (sdata->type == IEEE80211_IF_TYPE_AP
- || sdata->type == IEEE80211_IF_TYPE_VLAN)) {
+ || sdata->type == IEEE80211_IF_TYPE_VLAN) && rx->u.rx.ra_match) {
if (is_multicast_ether_addr(skb->data)) {
/* send multicast frames both to higher layers in
* local net stack and back to the wireless media */
@@ -2760,13 +2748,14 @@ static int ap_sta_ps_end(struct net_devi
static ieee80211_txrx_result
-ieee80211_rx_h_ieee80211_rx_h_ps_poll(struct ieee80211_txrx_data *rx)
+ieee80211_rx_h_ps_poll(struct ieee80211_txrx_data *rx)
{
struct sk_buff *skb;
int no_pending_pkts;
if (likely(!rx->sta || WLAN_FC_GET_TYPE(rx->fc) != WLAN_FC_TYPE_CTRL ||
- WLAN_FC_GET_STYPE(rx->fc) != WLAN_FC_STYPE_PSPOLL))
+ WLAN_FC_GET_STYPE(rx->fc) != WLAN_FC_STYPE_PSPOLL ||
+ !rx->u.rx.ra_match))
return TXRX_CONTINUE;
skb = skb_dequeue(&rx->sta->tx_filtered);
@@ -3042,8 +3031,10 @@ ieee80211_rx_h_check(struct ieee80211_tx
if (unlikely(rx->fc & WLAN_FC_RETRY &&
rx->sta->last_seq_ctrl[rx->u.rx.queue] ==
hdr->seq_ctrl)) {
- rx->local->dot11FrameDuplicateCount++;
- rx->sta->num_duplicates++;
+ if (rx->u.rx.ra_match) {
+ rx->local->dot11FrameDuplicateCount++;
+ rx->sta->num_duplicates++;
+ }
return TXRX_DROP;
} else
rx->sta->last_seq_ctrl[rx->u.rx.queue] = hdr->seq_ctrl;
@@ -3057,7 +3048,9 @@ ieee80211_rx_h_check(struct ieee80211_tx
return TXRX_DROP;
}
- if (memcmp(rx->dev->dev_addr, hdr->addr1, ETH_ALEN) == 0)
+ if (!rx->u.rx.ra_match)
+ rx->skb->pkt_type = PACKET_OTHERHOST;
+ else if (memcmp(rx->dev->dev_addr, hdr->addr1, ETH_ALEN) == 0)
rx->skb->pkt_type = PACKET_HOST;
else if (is_multicast_ether_addr(hdr->addr1)) {
if (is_broadcast_ether_addr(hdr->addr1))
@@ -3080,8 +3073,10 @@ ieee80211_rx_h_check(struct ieee80211_tx
WLAN_FC_GET_STYPE(rx->fc) == WLAN_FC_STYPE_PSPOLL)) &&
rx->sdata->type != IEEE80211_IF_TYPE_IBSS &&
(!rx->sta || !(rx->sta->flags & WLAN_STA_ASSOC)))) {
- if (!(rx->fc & WLAN_FC_FROMDS) && !(rx->fc & WLAN_FC_TODS)) {
- /* Drop IBSS frames silently. */
+ if ((!(rx->fc & WLAN_FC_FROMDS) && !(rx->fc & WLAN_FC_TODS)) ||
+ !rx->u.rx.ra_match) {
+ /* Drop IBSS frames and frames for other hosts
+ * silently. */
return TXRX_DROP;
}
@@ -3113,6 +3108,8 @@ ieee80211_rx_h_check(struct ieee80211_tx
rx->key = rx->sdata->keys[keyidx];
}
if (!rx->key) {
+ if (!rx->u.rx.ra_match)
+ return TXRX_DROP;
printk(KERN_DEBUG "%s: RX WEP frame with "
"unknown keyidx %d (A1=" MACSTR " A2="
MACSTR " A3=" MACSTR ")\n",
@@ -3128,7 +3125,7 @@ ieee80211_rx_h_check(struct ieee80211_tx
}
}
- if (rx->fc & WLAN_FC_ISWEP && rx->key) {
+ if (rx->fc & WLAN_FC_ISWEP && rx->key && rx->u.rx.ra_match) {
rx->key->tx_rx_count++;
if (unlikely(rx->local->key_tx_rx_threshold &&
rx->key->tx_rx_count >
@@ -3168,6 +3165,10 @@ ieee80211_rx_h_sta_process(struct ieee80
*/
sta->last_rx = jiffies;
}
+
+ if (!rx->u.rx.ra_match)
+ return TXRX_CONTINUE;
+
sta->rx_fragments++;
sta->rx_bytes += rx->skb->len;
sta->last_rssi = rx->u.rx.status->ssi;
@@ -3203,7 +3204,7 @@ ieee80211_rx_h_wep_weak_iv_detection(str
{
if (!rx->sta || !(rx->fc & WLAN_FC_ISWEP) ||
WLAN_FC_GET_TYPE(rx->fc) != WLAN_FC_TYPE_DATA || !rx->key ||
- rx->key->alg != ALG_WEP)
+ rx->key->alg != ALG_WEP || !rx->u.rx.ra_match)
return TXRX_CONTINUE;
/* Check for weak IVs, if hwaccel did not remove IV from the frame */
@@ -3260,7 +3261,7 @@ static ieee80211_txrx_result
ieee80211_rx_h_802_1x_pae(struct ieee80211_txrx_data *rx)
{
if (rx->sdata->eapol && ieee80211_is_eapol(rx->skb) &&
- rx->sdata->type != IEEE80211_IF_TYPE_STA) {
+ rx->sdata->type != IEEE80211_IF_TYPE_STA && rx->u.rx.ra_match) {
/* Pass both encrypted and unencrypted EAPOL frames to user
* space for processing. */
ieee80211_rx_mgmt(rx->dev, rx->skb, rx->u.rx.status,
@@ -3313,6 +3314,10 @@ static ieee80211_txrx_result
ieee80211_rx_h_mgmt(struct ieee80211_txrx_data *rx)
{
struct ieee80211_sub_if_data *sdata;
+
+ if (!rx->u.rx.ra_match)
+ return TXRX_DROP;
+
sdata = IEEE80211_DEV_TO_SUB_IF(rx->dev);
if ((sdata->type == IEEE80211_IF_TYPE_STA ||
sdata->type == IEEE80211_IF_TYPE_IBSS) &&
@@ -3351,7 +3356,8 @@ ieee80211_rx_h_passive_scan(struct ieee8
fc = le16_to_cpu(hdr->frame_control);
if (WLAN_FC_GET_TYPE(fc) == WLAN_FC_TYPE_MGMT &&
- WLAN_FC_GET_STYPE(fc) == WLAN_FC_STYPE_BEACON) {
+ WLAN_FC_GET_STYPE(fc) == WLAN_FC_STYPE_BEACON &&
+ rx->dev == local->mdev) {
local->scan.rx_beacon++;
/* Need to trim FCS here because it is normally
* removed only after this passive scan handler. */
@@ -3502,14 +3508,16 @@ static void ieee80211_rx_michael_mic_rep
rx->skb = NULL;
}
-static inline void ieee80211_invoke_rx_handlers(struct ieee80211_local *local,
- struct ieee80211_txrx_data *rx,
- struct sta_info *sta)
+static inline ieee80211_txrx_result __ieee80211_invoke_rx_handlers(
+ struct ieee80211_local *local,
+ ieee80211_rx_handler *handlers,
+ struct ieee80211_txrx_data *rx,
+ struct sta_info *sta)
{
ieee80211_rx_handler *handler;
ieee80211_txrx_result res = TXRX_DROP;
- for (handler = local->rx_handlers; *handler != NULL; handler++) {
+ for (handler = handlers; *handler != NULL; handler++) {
res = (*handler)(rx);
if (res != TXRX_CONTINUE) {
if (res == TXRX_DROP) {
@@ -3523,8 +3531,19 @@ static inline void ieee80211_invoke_rx_h
}
}
+ if (res == TXRX_DROP) {
+ dev_kfree_skb(rx->skb);
+ }
+ return res;
+}
- if (res == TXRX_DROP || *handler == NULL)
+static inline void ieee80211_invoke_rx_handlers(struct ieee80211_local *local,
+ ieee80211_rx_handler *handlers,
+ struct ieee80211_txrx_data *rx,
+ struct sta_info *sta)
+{
+ if (__ieee80211_invoke_rx_handlers(local, handlers, rx, sta) ==
+ TXRX_CONTINUE)
dev_kfree_skb(rx->skb);
}
@@ -3568,37 +3587,57 @@ void __ieee80211_rx(struct net_device *d
if (unlikely(local->sta_scanning || local->scan.in_scan))
rx.u.rx.in_scan = 1;
+ if (__ieee80211_invoke_rx_handlers(local, local->rx_pre_handlers, &rx,
+ sta) != TXRX_CONTINUE)
+ goto end;
+ skb = rx.skb;
+
if (sta && !sta->assoc_ap && !(sta->flags & WLAN_STA_WDS) &&
!local->iff_promiscs && !multicast) {
rx.dev = sta->dev;
rx.sdata = IEEE80211_DEV_TO_SUB_IF(rx.dev);
- ieee80211_invoke_rx_handlers(local, &rx, sta);
+ rx.u.rx.ra_match = 1;
+ ieee80211_invoke_rx_handlers(local, local->rx_handlers, &rx,
+ sta);
} else {
struct ieee80211_sub_if_data *prev = NULL;
struct sk_buff *skb_new;
u8 *bssid = ieee80211_get_bssid(hdr, skb->len);
list_for_each_entry(sdata, &local->sub_if_list, list) {
+ rx.u.rx.ra_match = 1;
switch (sdata->type) {
case IEEE80211_IF_TYPE_STA:
- if (!bssid || memcmp(sdata->u.sta.bssid,
- bssid, ETH_ALEN) != 0)
- continue;
- if (!multicast && !sdata->promisc &&
- memcmp(sdata->dev->dev_addr, hdr->addr1,
- ETH_ALEN) != 0)
+ if (!bssid)
continue;
+ if (!ieee80211_bssid_match(bssid,
+ sdata->u.sta.bssid)) {
+ if (!rx.u.rx.in_scan)
+ continue;
+ rx.u.rx.ra_match = 0;
+ } else if (!multicast &&
+ memcmp(sdata->dev->dev_addr,
+ hdr->addr1, ETH_ALEN) != 0) {
+ if (!sdata->promisc)
+ continue;
+ rx.u.rx.ra_match = 0;
+ }
break;
case IEEE80211_IF_TYPE_IBSS:
- if (!bssid ||
- !ieee80211_bssid_match(bssid,
- sdata->u.sta.bssid))
+ if (!bssid)
continue;
- if (!multicast && !sdata->promisc &&
- memcmp(sdata->dev->dev_addr, hdr->addr1,
- ETH_ALEN) != 0)
- continue;
- if (sta == NULL) {
+ if (!ieee80211_bssid_match(bssid,
+ sdata->u.sta.bssid)) {
+ if (!rx.u.rx.in_scan)
+ continue;
+ rx.u.rx.ra_match = 0;
+ } else if (!multicast &&
+ memcmp(sdata->dev->dev_addr,
+ hdr->addr1, ETH_ALEN) != 0) {
+ if (!sdata->promisc)
+ continue;
+ rx.u.rx.ra_match = 0;
+ } else if (sta == NULL) {
sta = rx.sta =
ieee80211_ibss_add_sta(dev, skb, bssid,
hdr->addr2);
@@ -3610,10 +3649,12 @@ void __ieee80211_rx(struct net_device *d
if (memcmp(sdata->dev->dev_addr,
hdr->addr1, ETH_ALEN) != 0)
continue;
- } else if (!rx.u.rx.in_scan &&
- !ieee80211_bssid_match(bssid,
- sdata->dev->dev_addr))
- continue;
+ } else if (!ieee80211_bssid_match(bssid,
+ sdata->dev->dev_addr)) {
+ if (!rx.u.rx.in_scan)
+ continue;
+ rx.u.rx.ra_match = 0;
+ }
if (sdata->dev == sdata->master &&
!rx.u.rx.in_scan)
/* do not receive anything via
@@ -3643,7 +3684,9 @@ void __ieee80211_rx(struct net_device *d
rx.skb = skb_new;
rx.dev = prev->dev;
rx.sdata = prev;
- ieee80211_invoke_rx_handlers(local, &rx, sta);
+ ieee80211_invoke_rx_handlers(local,
+ local->rx_handlers,
+ &rx, sta);
}
prev = sdata;
}
@@ -3651,7 +3694,8 @@ void __ieee80211_rx(struct net_device *d
rx.skb = skb;
rx.dev = prev->dev;
rx.sdata = prev;
- ieee80211_invoke_rx_handlers(local, &rx, sta);
+ ieee80211_invoke_rx_handlers(local, local->rx_handlers,
+ &rx, sta);
} else
dev_kfree_skb(skb);
}
@@ -3765,11 +3809,17 @@ ieee80211_rx_h_load_stats(struct ieee802
local->channel_use_raw += load;
if (rx->sta)
rx->sta->channel_use_raw += load;
- rx->sdata->channel_use_raw += load;
+ rx->u.rx.load = load;
return TXRX_CONTINUE;
}
+static ieee80211_txrx_result
+ieee80211_rx_h_if_stats(struct ieee80211_txrx_data *rx)
+{
+ rx->sdata->channel_use_raw += rx->u.rx.load;
+ return TXRX_CONTINUE;
+}
static void ieee80211_stat_refresh(unsigned long data)
{
@@ -4113,10 +4163,18 @@ void ieee80211_tx_status(struct net_devi
/* TODO: implement register/unregister functions for adding TX/RX handlers
* into ordered list */
-static ieee80211_rx_handler ieee80211_rx_handlers[] =
+/* rx_pre handlers don't have dev and sdata fields available in
+ * ieee80211_txrx_data */
+static ieee80211_rx_handler ieee80211_rx_pre_handlers[] =
{
ieee80211_rx_h_parse_qos,
ieee80211_rx_h_load_stats,
+ NULL
+};
+
+static ieee80211_rx_handler ieee80211_rx_handlers[] =
+{
+ ieee80211_rx_h_if_stats,
ieee80211_rx_h_monitor,
ieee80211_rx_h_passive_scan,
ieee80211_rx_h_check,
@@ -4126,7 +4184,7 @@ static ieee80211_rx_handler ieee80211_rx
ieee80211_rx_h_wep_weak_iv_detection,
ieee80211_rx_h_wep_decrypt,
ieee80211_rx_h_defragment,
- ieee80211_rx_h_ieee80211_rx_h_ps_poll,
+ ieee80211_rx_h_ps_poll,
ieee80211_rx_h_michael_mic_verify,
/* this must be after decryption - so header is counted in MPDU mic
* must be before pae and data, so QOS_DATA format frames
@@ -4284,6 +4342,7 @@ struct net_device *ieee80211_alloc_hw(si
NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST);
local->mdev = mdev;
+ local->rx_pre_handlers = ieee80211_rx_pre_handlers;
local->rx_handlers = ieee80211_rx_handlers;
local->tx_handlers = ieee80211_tx_handlers;
--- dscape.orig/net/d80211/ieee80211_i.h
+++ dscape/net/d80211/ieee80211_i.h
@@ -137,7 +137,11 @@ struct ieee80211_txrx_data {
struct ieee80211_rx_status *status;
int sent_ps_buffered;
int queue;
+ int load;
int in_scan:1;
+ int ra_match:1; /* frame is destined to interface
+ * currently processed (including
+ * multicast frames) */
} rx;
} u;
#ifdef CONFIG_HOSTAPD_WPA_TESTING
@@ -413,6 +417,7 @@ struct ieee80211_local {
struct ieee80211_passive_scan scan;
+ ieee80211_rx_handler *rx_pre_handlers;
ieee80211_rx_handler *rx_handlers;
ieee80211_tx_handler *tx_handlers;
--- dscape.orig/net/d80211/wpa.c
+++ dscape/net/d80211/wpa.c
@@ -235,6 +235,10 @@ ieee80211_rx_h_michael_mic_verify(struct
#ifdef CONFIG_HOSTAPD_WPA_TESTING
int i;
#endif /* CONFIG_HOSTAPD_WPA_TESTING */
+
+ if (!rx->u.rx.ra_match)
+ return TXRX_DROP;
+
printk(KERN_DEBUG "%s: invalid Michael MIC in data frame from "
MACSTR "\n", rx->dev->name, MAC2STR(sa));
#ifdef CONFIG_HOSTAPD_WPA_TESTING
--
Jiri Benc
SUSE Labs
^ permalink raw reply
* [e1000]: flow control on by default - good idea really?
From: jamal @ 2006-07-04 17:11 UTC (permalink / raw)
To: David S. Miller, Jeff Garzik, Auke Kok, Ben Greear, john.ronciak,
Robert Olsson, jesse.brandeburg
Cc: netdev
CCing anybody who may have stakes on this. Ignore the email if this
doesnt interest you.
Ok, folks - i had deferred this discussion but it bit me in the ass.
I just spend an hour debugging it (and in the process blew up a gbic i
borrowed, so my day aint going well since i actually have to pay for
this and cant really do the testing i was planning to;-<).
I have a device connected to a e1000 that was erroneously advertising
both tx/rx flow control but wasnt properly reacting to it.
The default setup on the e1000 has rx flow control turned on.
I was sending at wire rate gige from the device - which is about
1.48Mpps. The e1000 was in turn sending me flow control packets
as per default/expected behavior. Unfortunately, it was sending
a very large amount of packets. At one point i was seeing upto
1Mpps and on average, the flow control packets were consuming
60-70% of the bandwidth. Even when i fixed this behavior to act
properly, allowing flow control on consumed up to 15% of the bandwidth.
Clearly, this is a bad thing. Yes, the device in the first instance was
at fault. But i have argued in the past that NAPI does just fine without
flow control being turned on, so even chewing 5% of bandwidth on flow
control is a bad thing..
As a compromise, can we declare flow control as an advanced feature
and turn it off by default? People who feel it is valuable and know
what they are doing can turn it off.
If you want more details just shoot.
cheers,
jamal
PS:- BTW, even turning off flow control on e1000 didnt give as good
performance as in the old days on this machine - but i dont want to go
into that discussion.
^ permalink raw reply
* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: Shailabh Nagar @ 2006-07-04 16:37 UTC (permalink / raw)
To: Shailabh Nagar
Cc: hadi, Andrew Morton, netdev, linux-kernel, csturtiv, balbir, jlan,
Valdis.Kletnieks, pj
In-Reply-To: <44AA86BF.3090600@watson.ibm.com>
Shailabh Nagar wrote:
> jamal wrote:
>
>> On Mon, 2006-03-07 at 18:01 -0700, Andrew Morton wrote:
>>
>>> On Mon, 03 Jul 2006 20:54:37 -0400
>>> Shailabh Nagar <nagar@watson.ibm.com> wrote:
>>>
>>>
>>>>> What happens when a listener exits without doing deregistration
>>>>> (or if the listener attempts to register another cpumask while a
>>>>> current
>>>>> registration is still active).
>>>>>
>>>>
>>>> ( Jamal, your thoughts on this problem would be appreciated)
>>>>
>>>> Problem is that we have a listener task which has "registered" with
>>>> taskstats and caused
>>>> its pid to be stored in various per-cpu lists of listeners. Later,
>>>> when some other task exits on a given cpu, its exit data is sent
>>>> using genlmsg_unicast on each pid present on that cpu's list.
>>>>
>>>> If the listener exits without doing a "deregister", its pid
>>>> continues to be kept around, obviously not a good thing. So we need
>>>> some way of detecting the situation (task is no longer listening on
>>>> these cpus events) that is efficient.
>>>
>>>
>>> Also need to address the case where the listener has closed off his file
>>> descriptor but continues to run.
>>>
>>> So hooking into listener's exit() isn't appropriate - the teardown is
>>> associated with the lifetime of the fd, not of the process. If we do
>>> that,
>>> exit() gets handled for free.
>>
>>
>>
>> If you are always going to send unicast messages, then -ECONNREFUSED
>> will tell you the listener has closed their fd - this doesnt meant it
>> has exited.
>
>
> Thats good. So we have atleast one way of detecting the "closed fd without
> deregistering" within taskstats itself.
>
>> Besides that one process could open several sockets. I know
>> that would not be the app you would write - but it doesnt stop other
>> people from doing it.
>
>
> As far as API is concerned, even a taskstats listener is not being
> prevented from opening multiple sockets. As Andrew also pointed out,
> everything needs to be done per-socket.
>
>> I think i may not follow what you are doing - for some reason i thought
>> you may have many listeners in user space and these messages get
>> multicast to them?
>
>
> That was the design earlier. In the past week, the design has changed to
> one where there are still many listeners in user space but messages
> get unicast to each of them. Earlier listeners would get messages generated
> on task exit from every cpu, now they get it only from cpus for which
> they have explicitly registered interest (via a cpumask passed in through
> another genetlink command).
>
>> Does the user space program somehow communicate its pid to the kernel?
>
>
> Yes. When the listener registers interest in a set of cpus, as described
> above, its (genl_info->pid) is being stored in the per-cpu list of
> listeners for those cpus. When a task exits on one of those cpus, the
> exit data is only sent via genetlink_unicast to those pids
> (really, nl_pids) who are on that cpu's listener list.
>
>
> Now that I think more about it, netlink is really maintaining a pidhash
> of nl_pids, not process pids, right ? So if one userapp were to open
> multiple sockets using NETLINK_GENERIC protocol (regardless of how many
> of those are for the taskstats), each of them would have to use a
> different nl_pid. Hence, it would be valid for the taskstats layer to
> use netlink_lookup() at any time to see if the corresponding socket were
> closed ?
>
Here's a strawman for the problem we're trying to solve: get
notification of the close of a NETLINK_GENERIC socket that had
been used to register interest for some cpus within taskstats.
From looking at the netlink code, the way to go seems to be
- it maintains a pidhash of nl_pids that are currently
registered to listen to atleast one cpu. It also stores the
cpumask used.
- taskstats registers a notifier block within netlink_chain
and receives a callback on the NETLINK_URELEASE event, similar
to drivers/scsci/scsi_transport_iscsi.c: iscsi_rcv_nl_event()
- the callback checks to see that the protocol is NETLINK_GENERIC
and that the nl_pid for the socket is in taskstat's pidhash. If so, it
does a cleanup using the stored cpumask and releases the nl_pid
from the pidhash.
We can even do away with the deregister command altogether and
simply rely on this autocleanup.
--Shailabh
^ permalink raw reply
* Re: [patch 1/7] net_device list cleanup: core
From: Alexey Kuznetsov @ 2006-07-04 16:35 UTC (permalink / raw)
To: Andrey Savochkin
Cc: Christoph Hellwig, netdev, Kirill Korotaev, Eric W. Biederman
In-Reply-To: <20060704185035.A29778@castle.nmd.msu.ru>
Hello!
> Different modules want different kinds of lookup.
> So, I'm thinking about something like ilookup5.
....
> The next question: would people agree to review a patch doing this for
> net_devices? :)
One not original suggestion, which did not sound nevertheless:
to implement netdev_iterate_list() or whatever, update only core
and a few of devices and deprecate dev_base_head
with __deprecated_for_modules adding it to
Documentation/feature-removal-schedule.txt
Alexey
^ permalink raw reply
* Re: possible recursive locking in ATM layer
From: Arjan van de Ven @ 2006-07-04 16:13 UTC (permalink / raw)
To: Duncan Sands; +Cc: netdev, linux-kernel, Ingo Molnar, chas
In-Reply-To: <200607041759.43064.duncan.sands@math.u-psud.fr>
From: Arjan van de Ven <arjan@linux.intel.com>
> Linux version 2.6.17-git22 (duncan@baldrick) (gcc version 4.0.3 (Ubuntu 4.0.3-1ubuntu5)) #20 PREEMPT Tue Jul 4 10:35:04 CEST 2006
>
> [ 2381.598609] =============================================
> [ 2381.619314] [ INFO: possible recursive locking detected ]
> [ 2381.635497] ---------------------------------------------
> [ 2381.651706] atmarpd/2696 is trying to acquire lock:
> [ 2381.666354] (&skb_queue_lock_key){-+..}, at: [<c028c540>] skb_migrate+0x24/0x6c
> [ 2381.688848]
ok this is a real potential deadlock in a way, it takes two locks of 2
skbuffs without doing any kind of lock ordering; I think the following
patch should fix it. Just sort the lock taking order by address of the
skb.. it's not pretty but it's the best this can do in a minimally
invasive way.
I still agree with the comment that this code shouldn't live in the atm
layer...
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
net/atm/ipcommon.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
Index: linux-2.6.17-mm6/net/atm/ipcommon.c
===================================================================
--- linux-2.6.17-mm6.orig/net/atm/ipcommon.c
+++ linux-2.6.17-mm6/net/atm/ipcommon.c
@@ -25,8 +25,8 @@
/*
* skb_migrate appends the list at "from" to "to", emptying "from" in the
* process. skb_migrate is atomic with respect to all other skb operations on
- * "from" and "to". Note that it locks both lists at the same time, so beware
- * of potential deadlocks.
+ * "from" and "to". Note that it locks both lists at the same time, so to deal
+ * with the lock ordering, the locks are taken in address order.
*
* This function should live in skbuff.c or skbuff.h.
*/
@@ -39,8 +39,13 @@ void skb_migrate(struct sk_buff_head *fr
struct sk_buff *skb_to = (struct sk_buff *) to;
struct sk_buff *prev;
- spin_lock_irqsave(&from->lock,flags);
- spin_lock(&to->lock);
+ if (from<to) {
+ spin_lock_irqsave(&from->lock,flags);
+ spin_lock_nested(&to->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ spin_lock_irqsave(&to->lock, flags);
+ spin_lock_nested(&from->lock, SINGLE_DEPTH_NESTING);
+ }
prev = from->prev;
from->next->prev = to->prev;
prev->next = skb_to;
^ permalink raw reply
* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: Shailabh Nagar @ 2006-07-04 15:18 UTC (permalink / raw)
To: hadi
Cc: Andrew Morton, netdev, linux-kernel, csturtiv, balbir, jlan,
Valdis.Kletnieks, pj
In-Reply-To: <1152018353.5214.14.camel@jzny2>
jamal wrote:
> On Mon, 2006-03-07 at 18:01 -0700, Andrew Morton wrote:
>
>>On Mon, 03 Jul 2006 20:54:37 -0400
>>Shailabh Nagar <nagar@watson.ibm.com> wrote:
>>
>>
>>>>What happens when a listener exits without doing deregistration
>>>>(or if the listener attempts to register another cpumask while a current
>>>>registration is still active).
>>>>
>>>
>>>( Jamal, your thoughts on this problem would be appreciated)
>>>
>>>Problem is that we have a listener task which has "registered" with
>>>taskstats and caused
>>>its pid to be stored in various per-cpu lists of listeners. Later, when
>>>some other task exits on a given cpu, its exit data is sent using
>>>genlmsg_unicast on each pid present on that cpu's list.
>>>
>>>If the listener exits without doing a "deregister", its pid continues to
>>>be kept around, obviously not a good thing. So we need some way of
>>>detecting the situation (task is no longer listening on
>>>these cpus events) that is efficient.
>>
>>Also need to address the case where the listener has closed off his file
>>descriptor but continues to run.
>>
>>So hooking into listener's exit() isn't appropriate - the teardown is
>>associated with the lifetime of the fd, not of the process. If we do that,
>>exit() gets handled for free.
>
>
> If you are always going to send unicast messages, then -ECONNREFUSED
> will tell you the listener has closed their fd - this doesnt meant it
> has exited.
Thats good. So we have atleast one way of detecting the "closed fd without
deregistering" within taskstats itself.
> Besides that one process could open several sockets. I know
> that would not be the app you would write - but it doesnt stop other
> people from doing it.
As far as API is concerned, even a taskstats listener is not being
prevented from opening multiple sockets. As Andrew also pointed out,
everything needs to be done per-socket.
> I think i may not follow what you are doing - for some reason i thought
> you may have many listeners in user space and these messages get
> multicast to them?
That was the design earlier. In the past week, the design has changed to
one where there are still many listeners in user space but messages
get unicast to each of them. Earlier listeners would get messages generated
on task exit from every cpu, now they get it only from cpus for which
they have explicitly registered interest (via a cpumask passed in through
another genetlink command).
> Does the user space program somehow communicate its pid to the kernel?
Yes. When the listener registers interest in a set of cpus, as described
above, its (genl_info->pid) is being stored in the per-cpu list of
listeners for those cpus. When a task exits on one of those cpus, the
exit data is only sent via genetlink_unicast to those pids
(really, nl_pids) who are on that cpu's listener list.
Now that I think more about it, netlink is really maintaining a pidhash
of nl_pids, not process pids, right ? So if one userapp were to open
multiple sockets using NETLINK_GENERIC protocol (regardless of how many
of those are for the taskstats), each of them would have to use a
different nl_pid. Hence, it would be valid for the taskstats layer to use
netlink_lookup() at any time to see if the corresponding socket were
closed ?
--Shailabh
^ permalink raw reply
* Re: [patch 1/7] net_device list cleanup: core
From: Andrey Savochkin @ 2006-07-04 14:50 UTC (permalink / raw)
To: Christoph Hellwig
Cc: netdev, Kirill Korotaev, Eric W. Biederman, Alexey Kuznetsov
In-Reply-To: <20060704091003.GA17961@infradead.org>
On Tue, Jul 04, 2006 at 10:10:03AM +0100, Christoph Hellwig wrote:
> On Tue, Jul 04, 2006 at 11:24:05AM +0400, Andrey Savochkin wrote:
> > > Yes, it's a little more work as you need to audit all drivers to see what
> > > they are doing and find suitable abstractions but it's a must have that
> > > should have been done a lot earlier.
> >
> > Hiding dev_base_head can be done by converting first_netdev/next_netdev into
> > functions and implementing for_each_netdev loop through them.
> >
> > Or are you talking about abstractions like functions
> > for_each_netdev/find_netdev with callbacks?
>
> an for_each_netdev with a callback makes sense and gives a cleaner
> abstraction, yes. I don't think you should need a callback for the lookup
> structure.
Different modules want different kinds of lookup.
So, I'm thinking about something like ilookup5.
>
> > Do you think that hiding the list internals is worth the additional
> > complexity and substantial increase of the patch size?
>
> Yes, absolutely. We've converted scsi hosts and devices from a model
> where drivers could directly access the list to strict iterators in the
> 2.5 series. It's quite a lot of work as you have to understand what
> the drivers actually do (and to at least 50% they were doing something
> really stupid) and convert them to the right abstractions.
The next question: would people agree to review a patch doing this for
net_devices? :)
Andrey
^ permalink raw reply
* Re: [PATCH 0/2] NET: Accurate packet scheduling for ATM/ADSL
From: Patrick McHardy @ 2006-07-04 13:29 UTC (permalink / raw)
To: Russell Stuart
Cc: Russell Stuart, hadi, Alan Cox, Stephen Hemminger, netdev,
Jesper Dangaard Brouer
In-Reply-To: <44A0CE01.4010109@stuart.id.au>
[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]
Russell Stuart wrote:
> On 26/06/2006 9:10 PM, Patrick McHardy wrote:
>
>>> 5. We still did have to modify the kernel for ATM. That was
>>> because of its rather unusual characteristics. However,
>>> it you look at the size of modifications made to the kernel
>>> verses the size made to the user space tool, (37 lines
>>> versus 303 lines,) the bulk of the work was does in user
>>> space.
>>
>>
>> I'm sorry, but arguing that a limited special case solution is
>> better because it needs slightly less code is just not reasonable.
>
>
> Without seeing your actual proposal it is difficult to
> judge whether this is a reasonable trade-off or not.
> Hopefully we will see your code soon. Do you have any
> idea when?
Unfortunately I still didn't got to cleaning them up, so I'm sending
them in their preliminary state. Its not much that is missing, but
the netem usage of skb->cb needs to be integrated better, I failed
to move it to the qdisc_skb_cb so far because of circular includes.
But nothing unfixable. I'm mostly interested if the current size-tables
can express what you need for ATM, I wasn't able to understand the
big comment in tc_core.c in your patch.
[-- Attachment #2: 01.diff --]
[-- Type: text/plain, Size: 13169 bytes --]
[NET_SCHED]: Add accessor function for packet length for qdiscs
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 2a6508576111d82246ee018edbcc4b0f0d18acad
tree 8be27ab6040ea90ed11728763e5b8fcf9e221b67
parent 31304c909e6945b005af62cd55a582e9c010a0b4
author Patrick McHardy <kaber@trash.net> Tue, 04 Jul 2006 15:03:01 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 04 Jul 2006 15:03:01 +0200
include/net/sch_generic.h | 9 +++++++--
net/sched/sch_atm.c | 4 ++--
net/sched/sch_cbq.c | 12 ++++++------
net/sched/sch_dsmark.c | 2 +-
net/sched/sch_fifo.c | 2 +-
net/sched/sch_gred.c | 12 ++++++------
net/sched/sch_hfsc.c | 8 ++++----
net/sched/sch_htb.c | 8 ++++----
net/sched/sch_netem.c | 6 +++---
net/sched/sch_prio.c | 2 +-
net/sched/sch_red.c | 2 +-
net/sched/sch_sfq.c | 14 +++++++-------
net/sched/sch_tbf.c | 6 +++---
net/sched/sch_teql.c | 4 ++--
14 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index b0e9108..75d7a55 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -184,12 +184,17 @@ tcf_destroy(struct tcf_proto *tp)
kfree(tp);
}
+static inline unsigned int qdisc_tx_len(struct sk_buff *skb)
+{
+ return skb->len;
+}
+
static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff_head *list)
{
__skb_queue_tail(list, skb);
- sch->qstats.backlog += skb->len;
- sch->bstats.bytes += skb->len;
+ sch->qstats.backlog += qdisc_tx_len(skb);
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index dbf44da..4df305e 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -453,9 +453,9 @@ #endif
if (flow) flow->qstats.drops++;
return ret;
}
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
- flow->bstats.bytes += skb->len;
+ flow->bstats.bytes += qdisc_tx_len(skb);
flow->bstats.packets++;
/*
* Okay, this may seem weird. We pretend we've dropped the packet if
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 80b7f6a..5d705e2 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -404,7 +404,7 @@ static int
cbq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
{
struct cbq_sched_data *q = qdisc_priv(sch);
- int len = skb->len;
+ int len = qdisc_tx_len(skb);
int ret;
struct cbq_class *cl = cbq_classify(skb, sch, &ret);
@@ -688,7 +688,7 @@ #ifdef CONFIG_NET_CLS_POLICE
static int cbq_reshape_fail(struct sk_buff *skb, struct Qdisc *child)
{
- int len = skb->len;
+ int len = qdisc_tx_len(skb);
struct Qdisc *sch = child->__parent;
struct cbq_sched_data *q = qdisc_priv(sch);
struct cbq_class *cl = q->rx_class;
@@ -915,7 +915,7 @@ cbq_dequeue_prio(struct Qdisc *sch, int
if (skb == NULL)
goto skip_class;
- cl->deficit -= skb->len;
+ cl->deficit -= qdisc_tx_len(skb);
q->tx_class = cl;
q->tx_borrowed = borrow;
if (borrow != cl) {
@@ -923,11 +923,11 @@ #ifndef CBQ_XSTATS_BORROWS_BYTES
borrow->xstats.borrows++;
cl->xstats.borrows++;
#else
- borrow->xstats.borrows += skb->len;
- cl->xstats.borrows += skb->len;
+ borrow->xstats.borrows += qdisc_tx_len(skb);
+ cl->xstats.borrows += qdisc_tx_len(skb);
#endif
}
- q->tx_len = skb->len;
+ q->tx_len = qdisc_tx_len(skb);
if (cl->deficit <= 0) {
q->active[prio] = cl;
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 11c8a21..53346c6 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -265,7 +265,7 @@ #endif
return err;
}
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
sch->q.qlen++;
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index c2689f4..ec99321 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -28,7 +28,7 @@ static int bfifo_enqueue(struct sk_buff
{
struct fifo_sched_data *q = qdisc_priv(sch);
- if (likely(sch->qstats.backlog + skb->len <= q->limit))
+ if (likely(sch->qstats.backlog + qdisc_tx_len(skb) <= q->limit))
return qdisc_enqueue_tail(skb, sch);
return qdisc_reshape_fail(skb, sch);
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 0cafdd5..f0bf5d7 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -189,7 +189,7 @@ static int gred_enqueue(struct sk_buff *
}
q->packetsin++;
- q->bytesin += skb->len;
+ q->bytesin += qdisc_tx_len(skb);
if (gred_wred_mode(t))
gred_load_wred_set(t, q);
@@ -227,8 +227,8 @@ static int gred_enqueue(struct sk_buff *
break;
}
- if (q->backlog + skb->len <= q->limit) {
- q->backlog += skb->len;
+ if (q->backlog + qdisc_tx_len(skb) <= q->limit) {
+ q->backlog += qdisc_tx_len(skb);
return qdisc_enqueue_tail(skb, sch);
}
@@ -255,7 +255,7 @@ static int gred_requeue(struct sk_buff *
} else {
if (red_is_idling(&q->parms))
red_end_of_idle_period(&q->parms);
- q->backlog += skb->len;
+ q->backlog += qdisc_tx_len(skb);
}
return qdisc_requeue(skb, sch);
@@ -278,7 +278,7 @@ static struct sk_buff *gred_dequeue(stru
"VQ 0x%x after dequeue, screwing up "
"backlog.\n", tc_index_to_dp(skb));
} else {
- q->backlog -= skb->len;
+ q->backlog -= qdisc_tx_len(skb);
if (!q->backlog && !gred_wred_mode(t))
red_start_of_idle_period(&q->parms);
@@ -300,7 +300,7 @@ static unsigned int gred_drop(struct Qdi
skb = qdisc_dequeue_tail(sch);
if (skb) {
- unsigned int len = skb->len;
+ unsigned int len = qdisc_tx_len(skb);
struct gred_sched_data *q;
u16 dp = tc_index_to_dp(skb);
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 6b1b4a9..3fc8351 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -942,7 +942,7 @@ qdisc_peek_len(struct Qdisc *sch)
printk("qdisc_peek_len: non work-conserving qdisc ?\n");
return 0;
}
- len = skb->len;
+ len = qdisc_tx_len(skb);
if (unlikely(sch->ops->requeue(skb, sch) != NET_XMIT_SUCCESS)) {
if (net_ratelimit())
printk("qdisc_peek_len: failed to requeue\n");
@@ -1648,7 +1648,7 @@ hfsc_enqueue(struct sk_buff *skb, struct
return err;
}
- len = skb->len;
+ len = qdisc_tx_len(skb);
err = cl->qdisc->enqueue(skb, cl->qdisc);
if (unlikely(err != NET_XMIT_SUCCESS)) {
cl->qstats.drops++;
@@ -1712,9 +1712,9 @@ hfsc_dequeue(struct Qdisc *sch)
return NULL;
}
- update_vf(cl, skb->len, cur_time);
+ update_vf(cl, qdisc_tx_len(skb), cur_time);
if (realtime)
- cl->cl_cumul += skb->len;
+ cl->cl_cumul += qdisc_tx_len(skb);
if (cl->qdisc->q.qlen != 0) {
if (cl->cl_flags & HFSC_RSC) {
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 34afe41..b26fa9a 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -733,12 +733,12 @@ #endif
cl->qstats.drops++;
return NET_XMIT_DROP;
} else {
- cl->bstats.packets++; cl->bstats.bytes += skb->len;
+ cl->bstats.packets++; cl->bstats.bytes += qdisc_tx_len(skb);
htb_activate (q,cl);
}
sch->q.qlen++;
- sch->bstats.packets++; sch->bstats.bytes += skb->len;
+ sch->bstats.packets++; sch->bstats.bytes += qdisc_tx_len(skb);
HTB_DBG(1,1,"htb_enq_ok cl=%X skb=%p\n",(cl && cl != HTB_DIRECT)?cl->classid:0,skb);
return NET_XMIT_SUCCESS;
}
@@ -1067,7 +1067,7 @@ next:
} while (cl != start);
if (likely(skb != NULL)) {
- if ((cl->un.leaf.deficit[level] -= skb->len) < 0) {
+ if ((cl->un.leaf.deficit[level] -= qdisc_tx_len(skb)) < 0) {
HTB_DBG(4,2,"htb_next_cl oldptr=%p quant_add=%d\n",
level?cl->parent->un.inner.ptr[prio]:q->ptr[0][prio],cl->un.leaf.quantum);
cl->un.leaf.deficit[level] += cl->un.leaf.quantum;
@@ -1077,7 +1077,7 @@ next:
gives us slightly better performance */
if (!cl->un.leaf.q->q.qlen)
htb_deactivate (q,cl);
- htb_charge_class (q,cl,level,skb->len);
+ htb_charge_class (q,cl,level,qdisc_tx_len(skb));
}
return skb;
}
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index c5bd806..aa97ecb 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -225,7 +225,7 @@ static int netem_enqueue(struct sk_buff
if (likely(ret == NET_XMIT_SUCCESS)) {
sch->q.qlen++;
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
} else
sch->qstats.drops++;
@@ -507,8 +507,8 @@ static int tfifo_enqueue(struct sk_buff
__skb_queue_after(list, skb, nskb);
- sch->qstats.backlog += nskb->len;
- sch->bstats.bytes += nskb->len;
+ sch->qstats.backlog += qdisc_tx_len(nskb);
+ sch->bstats.bytes += qdisc_tx_len(nskb);
sch->bstats.packets++;
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index a5fa03c..2175732 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -99,7 +99,7 @@ #ifdef CONFIG_NET_CLS_ACT
#endif
if ((ret = qdisc->enqueue(skb, qdisc)) == NET_XMIT_SUCCESS) {
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
sch->q.qlen++;
return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index d65cadd..24ec0b2 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -95,7 +95,7 @@ static int red_enqueue(struct sk_buff *s
ret = child->enqueue(skb, child);
if (likely(ret == NET_XMIT_SUCCESS)) {
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
sch->q.qlen++;
} else {
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d0d6e59..2a57d0d 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -225,7 +225,7 @@ static unsigned int sfq_drop(struct Qdis
if (d > 1) {
sfq_index x = q->dep[d+SFQ_DEPTH].next;
skb = q->qs[x].prev;
- len = skb->len;
+ len = qdisc_tx_len(skb);
__skb_unlink(skb, &q->qs[x]);
kfree_skb(skb);
sfq_dec(q, x);
@@ -241,7 +241,7 @@ static unsigned int sfq_drop(struct Qdis
q->next[q->tail] = q->next[d];
q->allot[q->next[d]] += q->quantum;
skb = q->qs[d].prev;
- len = skb->len;
+ len = qdisc_tx_len(skb);
__skb_unlink(skb, &q->qs[d]);
kfree_skb(skb);
sfq_dec(q, d);
@@ -267,7 +267,7 @@ sfq_enqueue(struct sk_buff *skb, struct
q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
q->hash[x] = hash;
}
- sch->qstats.backlog += skb->len;
+ sch->qstats.backlog += qdisc_tx_len(skb);
__skb_queue_tail(&q->qs[x], skb);
sfq_inc(q, x);
if (q->qs[x].qlen == 1) { /* The flow is new */
@@ -282,7 +282,7 @@ sfq_enqueue(struct sk_buff *skb, struct
}
}
if (++sch->q.qlen < q->limit-1) {
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
return 0;
}
@@ -303,7 +303,7 @@ sfq_requeue(struct sk_buff *skb, struct
q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
q->hash[x] = hash;
}
- sch->qstats.backlog += skb->len;
+ sch->qstats.backlog += qdisc_tx_len(skb);
__skb_queue_head(&q->qs[x], skb);
sfq_inc(q, x);
if (q->qs[x].qlen == 1) { /* The flow is new */
@@ -347,7 +347,7 @@ sfq_dequeue(struct Qdisc* sch)
skb = __skb_dequeue(&q->qs[a]);
sfq_dec(q, a);
sch->q.qlen--;
- sch->qstats.backlog -= skb->len;
+ sch->qstats.backlog -= qdisc_tx_len(skb);
/* Is the slot empty? */
if (q->qs[a].qlen == 0) {
@@ -359,7 +359,7 @@ sfq_dequeue(struct Qdisc* sch)
}
q->next[q->tail] = a;
q->allot[a] += q->quantum;
- } else if ((q->allot[a] -= skb->len) <= 0) {
+ } else if ((q->allot[a] -= qdisc_tx_len(skb)) <= 0) {
q->tail = a;
a = q->next[a];
q->allot[a] += q->quantum;
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index d9a5d29..c87b0e6 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -139,7 +139,7 @@ static int tbf_enqueue(struct sk_buff *s
struct tbf_sched_data *q = qdisc_priv(sch);
int ret;
- if (skb->len > q->max_size) {
+ if (qdisc_tx_len(skb) > q->max_size) {
sch->qstats.drops++;
#ifdef CONFIG_NET_CLS_POLICE
if (sch->reshape_fail == NULL || sch->reshape_fail(skb, sch))
@@ -155,7 +155,7 @@ #endif
}
sch->q.qlen++;
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
return 0;
}
@@ -204,7 +204,7 @@ static struct sk_buff *tbf_dequeue(struc
psched_time_t now;
long toks, delay;
long ptoks = 0;
- unsigned int len = skb->len;
+ unsigned int len = qdisc_tx_len(skb);
PSCHED_GET_TIME(now);
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 4c16ad5..538f63f 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -97,7 +97,7 @@ teql_enqueue(struct sk_buff *skb, struct
__skb_queue_tail(&q->q, skb);
if (q->q.qlen <= dev->tx_queue_len) {
- sch->bstats.bytes += skb->len;
+ sch->bstats.bytes += qdisc_tx_len(skb);
sch->bstats.packets++;
return 0;
}
@@ -278,7 +278,7 @@ static int teql_master_xmit(struct sk_bu
struct Qdisc *start, *q;
int busy;
int nores;
- int len = skb->len;
+ int len = qdisc_tx_len(skb);
struct sk_buff *skb_res = NULL;
start = master->slaves;
[-- Attachment #3: 02.diff --]
[-- Type: text/plain, Size: 2212 bytes --]
[NET_SCHED]: Move top-level device queueing code to seperate function
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit a39585afe71dafab96208515a8fa99c92b108fee
tree fbb7672a3061a38edc9f75d3fb8f34652796b109
parent 2a6508576111d82246ee018edbcc4b0f0d18acad
author Patrick McHardy <kaber@trash.net> Tue, 04 Jul 2006 15:03:28 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 04 Jul 2006 15:03:28 +0200
include/net/pkt_sched.h | 1 +
net/core/dev.c | 10 +---------
net/sched/sch_generic.c | 12 ++++++++++++
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 1925c65..44cf69e 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -224,6 +224,7 @@ extern struct qdisc_rate_table *qdisc_ge
struct rtattr *tab);
extern void qdisc_put_rtab(struct qdisc_rate_table *tab);
+extern int qdisc_enqueue_root(struct net_device *dev, struct sk_buff *skb);
extern void __qdisc_run(struct net_device *dev);
static inline void qdisc_run(struct net_device *dev)
diff --git a/net/core/dev.c b/net/core/dev.c
index 066a60a..8599120 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1449,15 +1449,7 @@ #ifdef CONFIG_NET_CLS_ACT
skb->tc_verd = SET_TC_AT(skb->tc_verd,AT_EGRESS);
#endif
if (q->enqueue) {
- /* Grab device queue */
- spin_lock(&dev->queue_lock);
-
- rc = q->enqueue(skb, q);
-
- qdisc_run(dev);
-
- spin_unlock(&dev->queue_lock);
- rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc;
+ rc = qdisc_enqueue_root(dev, skb);
goto out;
}
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index d735f51..2bab466 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -77,6 +77,18 @@ void qdisc_unlock_tree(struct net_device
if one is grabbed, another must be free.
*/
+int qdisc_enqueue_root(struct net_device *dev, struct sk_buff *skb)
+{
+ int ret;
+
+ spin_lock(&dev->queue_lock);
+ ret = dev->qdisc->enqueue(skb, dev->qdisc);
+ qdisc_run(dev);
+ spin_unlock(&dev->queue_lock);
+
+ return ret == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : ret;
+}
+
/* Kick device.
Note, that this procedure can be called by a watchdog timer, so that
[-- Attachment #4: 03.diff --]
[-- Type: text/plain, Size: 8494 bytes --]
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index d10f353..2ce55d5 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -83,6 +83,21 @@ struct tc_ratespec
__u32 rate;
};
+struct tc_sizespec
+{
+ unsigned int cell_log;
+ unsigned int addend;
+};
+
+enum {
+ TCA_STAB_UNSPEC,
+ TCA_STAB_BASE,
+ TCA_STAB_DATA,
+ __TCA_STAB_MAX
+};
+
+#define TCA_STAB_MAX (__TCA_STAB_MAX - 1)
+
/* FIFO section */
struct tc_fifo_qopt
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index facd9ee..167cc22 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -821,6 +821,7 @@ enum
TCA_RATE,
TCA_FCNT,
TCA_STATS2,
+ TCA_STAB,
__TCA_MAX
};
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 44cf69e..8fd9a42 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -223,6 +223,7 @@ extern struct Qdisc *qdisc_lookup_class(
extern struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
struct rtattr *tab);
extern void qdisc_put_rtab(struct qdisc_rate_table *tab);
+extern void qdisc_put_stab(struct qdisc_size_table *tab);
extern int qdisc_enqueue_root(struct net_device *dev, struct sk_buff *skb);
extern void __qdisc_run(struct net_device *dev);
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 75d7a55..76c50a1 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -23,6 +23,15 @@ struct qdisc_rate_table
int refcnt;
};
+struct qdisc_size_table
+{
+ struct list_head list;
+ struct tc_sizespec size;
+ int refcnt;
+ unsigned int tsize;
+ u32 data[];
+};
+
struct Qdisc
{
int (*enqueue)(struct sk_buff *skb, struct Qdisc *dev);
@@ -33,6 +42,7 @@ #define TCQ_F_THROTTLED 2
#define TCQ_F_INGRESS 4
int padded;
struct Qdisc_ops *ops;
+ struct qdisc_size_table *stab;
u32 handle;
u32 parent;
atomic_t refcnt;
@@ -184,9 +194,19 @@ tcf_destroy(struct tcf_proto *tp)
kfree(tp);
}
+struct qdisc_skb_cb {
+ unsigned int len;
+ char data[];
+};
+
+static inline struct qdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
+{
+ return (struct qdisc_skb_cb *)skb->cb;
+}
+
static inline unsigned int qdisc_tx_len(struct sk_buff *skb)
{
- return skb->len;
+ return qdisc_skb_cb(skb)->len;
}
static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c7844ba..479fc85 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -286,6 +286,78 @@ void qdisc_put_rtab(struct qdisc_rate_ta
}
}
+static LIST_HEAD(qdisc_stab_list);
+
+static struct qdisc_size_table *qdisc_get_stab(struct rtattr *tab, int *err)
+{
+ struct qdisc_size_table *stab;
+ struct rtattr *tb[TCA_STAB_MAX];
+ unsigned int tsize;
+
+ *err = -EINVAL;
+ if (rtattr_parse_nested(tb, TCA_STAB_MAX, tab))
+ return NULL;
+ if (tb[TCA_STAB_BASE-1] == NULL ||
+ RTA_PAYLOAD(tb[TCA_STAB_BASE-1]) < sizeof(struct tc_sizespec))
+ return NULL;
+
+ tsize = 0;
+ if (tb[TCA_STAB_DATA-1] != NULL)
+ tsize = RTA_PAYLOAD(tb[TCA_STAB_DATA-1]) / sizeof(u32);
+
+ list_for_each_entry(stab, &qdisc_stab_list, list) {
+ if (stab->tsize != tsize)
+ continue;
+ if (memcmp(&stab->size, RTA_DATA(tb[TCA_STAB_BASE-1]),
+ sizeof(stab->size)))
+ continue;
+ if (tsize > 0 &&
+ memcmp(stab->data, RTA_DATA(tb[TCA_STAB_DATA-1]),
+ sizeof(u32) * tsize));
+ continue;
+ stab->refcnt++;
+ return stab;
+ }
+
+ *err = -ENOMEM;
+ stab = kmalloc(sizeof(*stab) + sizeof(u32) * tsize, GFP_KERNEL);
+ if (stab == NULL)
+ return stab;
+ memcpy(&stab->size, RTA_DATA(tb[TCA_STAB_BASE-1]), sizeof(stab->size));
+ stab->tsize = tsize;
+ if (tsize > 0)
+ memcpy(stab->data, RTA_DATA(tb[TCA_STAB_DATA-1]),
+ sizeof(u32) * tsize);
+ list_add_tail(&stab->list, &qdisc_stab_list);
+ *err = 0;
+ return stab;
+}
+
+void qdisc_put_stab(struct qdisc_size_table *stab)
+{
+ if (!stab || --stab->refcnt)
+ return;
+ list_del(&stab->list);
+ kfree(stab);
+}
+
+static int
+qdisc_dump_stab(struct sk_buff *skb, struct qdisc_size_table *stab)
+{
+ unsigned char *b = skb->tail;
+ struct rtattr *rta = (struct rtattr *)b;
+
+ RTA_PUT(skb, TCA_STAB, 0, NULL);
+ RTA_PUT(skb, TCA_STAB_BASE, sizeof(stab->size), &stab->size);
+ RTA_PUT(skb, TCA_STAB_DATA, sizeof(stab->data[0]) * stab->tsize,
+ stab->data);
+ rta->rta_len = skb->tail - b;
+ return skb->len;
+
+rtattr_failure:
+ skb_trim(skb, b - skb->data);
+ return -1;
+}
/* Allocate an unique handle from space managed by kernel */
@@ -453,6 +525,11 @@ #endif
sch->handle = handle;
if (!ops->init || (err = ops->init(sch, tca[TCA_OPTIONS-1])) == 0) {
+ if (tca[TCA_STAB-1]) {
+ sch->stab = qdisc_get_stab(tca[TCA_STAB-1], &err);
+ if (sch->stab == NULL)
+ goto err_out3;
+ }
#ifdef CONFIG_NET_ESTIMATOR
if (tca[TCA_RATE-1]) {
err = gen_new_estimator(&sch->bstats, &sch->rate_est,
@@ -477,6 +554,7 @@ #endif
return sch;
}
err_out3:
+ qdisc_put_stab(sch->stab);
dev_put(dev);
kfree((char *) sch - sch->padded);
err_out2:
@@ -488,15 +566,26 @@ err_out:
static int qdisc_change(struct Qdisc *sch, struct rtattr **tca)
{
- if (tca[TCA_OPTIONS-1]) {
- int err;
+ int err;
+ if (tca[TCA_OPTIONS-1]) {
if (sch->ops->change == NULL)
return -EINVAL;
err = sch->ops->change(sch, tca[TCA_OPTIONS-1]);
if (err)
return err;
}
+ if (tca[TCA_STAB-1]) {
+ struct qdisc_size_table *stab;
+
+ stab = qdisc_get_stab(tca[TCA_STAB-1], &err);
+ if (stab == NULL)
+ return err;
+ spin_lock_bh(&sch->dev->queue_lock);
+ qdisc_put_stab(sch->stab);
+ sch->stab = stab;
+ spin_unlock_bh(&sch->dev->queue_lock);
+ }
#ifdef CONFIG_NET_ESTIMATOR
if (tca[TCA_RATE-1])
gen_replace_estimator(&sch->bstats, &sch->rate_est,
@@ -769,6 +858,9 @@ static int tc_fill_qdisc(struct sk_buff
goto rtattr_failure;
q->qstats.qlen = q->q.qlen;
+ if (q->stab != NULL && qdisc_dump_stab(skb, q->stab) < 0)
+ goto rtattr_failure;
+
if (gnet_stats_start_copy_compat(skb, TCA_STATS2, TCA_STATS,
TCA_XSTATS, q->stats_lock, &d) < 0)
goto rtattr_failure;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 2bab466..9022650 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -67,6 +67,21 @@ void qdisc_unlock_tree(struct net_device
write_unlock_bh(&qdisc_tree_lock);
}
+static void qdisc_init_len(struct sk_buff *skb, struct Qdisc *q)
+{
+ unsigned int idx, len = skb->len;
+ struct qdisc_size_table *stab = q->stab;
+
+ if (stab == NULL)
+ goto out;
+ idx = len >> stab->size.cell_log;
+ if (idx < stab->tsize)
+ len = stab->data[idx];
+ len += stab->size.addend;
+out:
+ ((struct qdisc_skb_cb *)skb->cb)->len = len;
+}
+
/*
dev->queue_lock serializes queue accesses for this device
AND dev->qdisc pointer itself.
@@ -82,6 +97,7 @@ int qdisc_enqueue_root(struct net_device
int ret;
spin_lock(&dev->queue_lock);
+ qdisc_init_len(skb, dev->qdisc);
ret = dev->qdisc->enqueue(skb, dev->qdisc);
qdisc_run(dev);
spin_unlock(&dev->queue_lock);
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index aa97ecb..15dde88 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -148,7 +148,7 @@ static long tabledist(unsigned long mu,
static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
{
struct netem_sched_data *q = qdisc_priv(sch);
- struct netem_skb_cb *cb = (struct netem_skb_cb *)skb->cb;
+ struct netem_skb_cb *cb = (struct netem_skb_cb *)qdisc_skb_cb(skb)->data;
struct sk_buff *skb2;
int ret;
int count = 1;
@@ -268,7 +268,7 @@ static struct sk_buff *netem_dequeue(str
skb = q->qdisc->dequeue(q->qdisc);
if (skb) {
const struct netem_skb_cb *cb
- = (const struct netem_skb_cb *)skb->cb;
+ = (const struct netem_skb_cb *)qdisc_skb_cb(skb)->data;
psched_time_t now;
/* if more time remaining? */
@@ -493,13 +493,13 @@ static int tfifo_enqueue(struct sk_buff
struct fifo_sched_data *q = qdisc_priv(sch);
struct sk_buff_head *list = &sch->q;
const struct netem_skb_cb *ncb
- = (const struct netem_skb_cb *)nskb->cb;
+ = (const struct netem_skb_cb *)qdisc_skb_cb(nskb)->data;
struct sk_buff *skb;
if (likely(skb_queue_len(list) < q->limit)) {
skb_queue_reverse_walk(list, skb) {
const struct netem_skb_cb *cb
- = (const struct netem_skb_cb *)skb->cb;
+ = (const struct netem_skb_cb *)qdisc_skb_cb(skb)->data;
if (!PSCHED_TLESS(ncb->time_to_send, cb->time_to_send))
break;
[-- Attachment #5: iproute.diff --]
[-- Type: text/plain, Size: 4483 bytes --]
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index d10f353..2ce55d5 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -83,6 +83,21 @@ struct tc_ratespec
__u32 rate;
};
+struct tc_sizespec
+{
+ unsigned int cell_log;
+ unsigned int addend;
+};
+
+enum {
+ TCA_STAB_UNSPEC,
+ TCA_STAB_BASE,
+ TCA_STAB_DATA,
+ __TCA_STAB_MAX
+};
+
+#define TCA_STAB_MAX (__TCA_STAB_MAX - 1)
+
/* FIFO section */
struct tc_fifo_qopt
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 5e33a20..addf5fb 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -821,6 +821,7 @@ enum
TCA_RATE,
TCA_FCNT,
TCA_STATS2,
+ TCA_STAB,
__TCA_MAX
};
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index e9174ab..c38fa87 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -41,10 +41,79 @@ static int usage(void)
return -1;
}
+static int parse_stab(int *argcp, char ***argvp, struct tc_sizespec *stab,
+ __u32 **datap)
+{
+ int argc = *argcp;
+ char **argv = *argvp;
+
+ NEXT_ARG();
+ while (argc > 0) {
+ if (matches("overhead", *argv) == 0) {
+ NEXT_ARG();
+ if (stab->addend)
+ duparg("overhead", *argv);
+ if (get_size(&stab->addend, *argv))
+ return -1;
+ NEXT_ARG();
+ } else if (matches("cell_log", *argv) == 0) {
+ NEXT_ARG();
+ if (stab->cell_log)
+ duparg("cell_log", *argv);
+ if (get_u32(&stab->cell_log, *argv, 0))
+ return -1;
+ NEXT_ARG();
+ } else if (get_size(*datap, *argv) == 0) {
+ argv++, argc--;
+ ++*datap;
+ } else
+ break;
+ }
+ if (!stab->addend && !stab->cell_log)
+ return -1;
+ *argcp = argc;
+ *argvp = argv;
+ return 0;
+}
+
+static void print_stab(FILE *f, char *prefix, struct rtattr *tab)
+{
+ struct rtattr *tb[TCA_STAB_MAX+1];
+ struct tc_sizespec *size;
+ unsigned int i;
+ __u32 *data;
+ SPRINT_BUF(buf);
+
+ parse_rtattr_nested(tb, TCA_STAB_MAX, tab);
+ if (tb[TCA_STAB_BASE] == NULL ||
+ RTA_PAYLOAD(tb[TCA_STAB_BASE]) < sizeof(struct tc_sizespec))
+ return;
+ fprintf(f, "%s", prefix);
+ size = RTA_DATA(tb[TCA_STAB_BASE]);
+ if (size->addend) {
+ print_size(buf, SPRINT_BSIZE-1, size->addend);
+ fprintf(f, "overhead %s ", buf);
+ }
+ if (size->cell_log)
+ fprintf(f, "cell_log %u ", size->cell_log);
+ if (tb[TCA_STAB_DATA] == NULL)
+ return;
+ data = RTA_DATA(tb[TCA_STAB_DATA]);
+ for (i = 0; i < RTA_PAYLOAD(tb[TCA_STAB_DATA]) / sizeof(__u32); i++) {
+ print_size(buf, SPRINT_BSIZE-1, data[i]);
+ fprintf(f, "%s ", buf);
+ }
+}
+
int tc_qdisc_modify(int cmd, unsigned flags, int argc, char **argv)
{
struct qdisc_util *q = NULL;
struct tc_estimator est;
+ struct {
+ struct tc_sizespec size;
+ __u32 data[256];
+ } stab;
+ __u32 *stabdata = &stab.data[0];
char d[16];
char k[16];
struct {
@@ -55,6 +124,7 @@ int tc_qdisc_modify(int cmd, unsigned fl
memset(&req, 0, sizeof(req));
memset(&est, 0, sizeof(est));
+ memset(&stab, 0, sizeof(stab));
memset(&d, 0, sizeof(d));
memset(&k, 0, sizeof(k));
@@ -108,6 +178,10 @@ #endif
} else if (matches(*argv, "estimator") == 0) {
if (parse_estimator(&argc, &argv, &est))
return -1;
+ } else if (matches(*argv, "stab") == 0) {
+ if (parse_stab(&argc, &argv, &stab.size, &stabdata))
+ return -1;
+ continue;
} else if (matches(*argv, "help") == 0) {
usage();
} else {
@@ -124,6 +198,16 @@ #endif
addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
if (est.ewma_log)
addattr_l(&req.n, sizeof(req), TCA_RATE, &est, sizeof(est));
+ if (stab.size.addend || stab.size.cell_log) {
+ struct rtattr *tail = NLMSG_TAIL(&req.n);
+
+ addattr_l(&req.n, sizeof(req), TCA_STAB, NULL, 0);
+ addattr_l(&req.n, sizeof(req), TCA_STAB_BASE, &stab.size,
+ sizeof(stab.size));
+ addattr_l(&req.n, sizeof(req), TCA_STAB_DATA, stab.data,
+ (void *)stabdata - (void *)stab.data);
+ tail->rta_len = (void *)NLMSG_TAIL(&req.n) - (void *)tail;
+ }
if (q) {
if (!q->parse_qopt) {
@@ -215,7 +299,7 @@ static int print_qdisc(const struct sock
q = get_qdisc_kind("prio");
else
q = get_qdisc_kind(RTA_DATA(tb[TCA_KIND]));
-
+
if (tb[TCA_OPTIONS]) {
if (q)
q->print_qopt(q, fp, tb[TCA_OPTIONS]);
@@ -223,6 +307,12 @@ static int print_qdisc(const struct sock
fprintf(fp, "[cannot parse qdisc parameters]");
}
fprintf(fp, "\n");
+
+ if (tb[TCA_STAB]) {
+ print_stab(fp, " ", tb[TCA_STAB]);
+ fprintf(fp, "\n");
+ }
+
if (show_stats) {
struct rtattr *xstats = NULL;
^ permalink raw reply related
* Re: strict isolation of net interfaces
From: Daniel Lezcano @ 2006-07-04 13:19 UTC (permalink / raw)
To: Sam Vilain
Cc: Andrey Savochkin, Serge E. Hallyn, Cedric Le Goater, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Ben Greear, Dave Hansen, Alexey Kuznetsov,
Eric W. Biederman, Russel Coker
In-Reply-To: <44AA6994.5010202@vilain.net>
Sam Vilain wrote:
> Daniel Lezcano wrote:
>
>>If it is ok for you, we can collaborate to merge the two solutions in
>>one. I will focus on layer 3 isolation and you on the layer 2.
>
>
> So, you're writing a LSM module or adapting the BSD Jail LSM, right? :)
>
> Sam.
No. I am adapting a prototype of network application container we did.
-- Daniel
^ permalink raw reply
* Re: [IPROUTE]: Introduce tc monitor
From: jamal @ 2006-07-04 13:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Thomas Graf, netdev, Stephen Hemminger
In-Reply-To: <44A8EDE1.7030503@trash.net>
On Mon, 2006-03-07 at 12:13 +0200, Patrick McHardy wrote:
> Speaking of actions, do you have any plans to
> add help-texts? Currently the output is very confusing, whenever
> I use them I need to google for examples.
>
Thanks for reminding me. There are examples in the doc/ directory of
iproute2, but they may be insufficient.
In any case, I wont have time today or the rest of the week but will get
some patch after that.
[Actually, I have about half a day off but I want to spend time
reviewing the qdisc_is_running thing in a test environment( It takes me
at least 2 hours to steal hardware and set it up)].
cheers,
jamal
^ permalink raw reply
* Re: strict isolation of net interfaces
From: Sam Vilain @ 2006-07-04 13:13 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Andrey Savochkin, Serge E. Hallyn, Cedric Le Goater, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Ben Greear, Dave Hansen, Alexey Kuznetsov,
Eric W. Biederman, Russel Coker
In-Reply-To: <44AA5F28.9040109@fr.ibm.com>
Daniel Lezcano wrote:
>
> If it is ok for you, we can collaborate to merge the two solutions in
> one. I will focus on layer 3 isolation and you on the layer 2.
So, you're writing a LSM module or adapting the BSD Jail LSM, right? :)
Sam.
^ permalink raw reply
* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: jamal @ 2006-07-04 13:05 UTC (permalink / raw)
To: Andrew Morton
Cc: netdev, linux-kernel, csturtiv, balbir, jlan, Valdis.Kletnieks,
pj, Shailabh Nagar
In-Reply-To: <20060703180151.56f61b31.akpm@osdl.org>
On Mon, 2006-03-07 at 18:01 -0700, Andrew Morton wrote:
> On Mon, 03 Jul 2006 20:54:37 -0400
> Shailabh Nagar <nagar@watson.ibm.com> wrote:
>
> > > What happens when a listener exits without doing deregistration
> > > (or if the listener attempts to register another cpumask while a current
> > > registration is still active).
> > >
> > ( Jamal, your thoughts on this problem would be appreciated)
> >
> > Problem is that we have a listener task which has "registered" with
> > taskstats and caused
> > its pid to be stored in various per-cpu lists of listeners. Later, when
> > some other task exits on a given cpu, its exit data is sent using
> > genlmsg_unicast on each pid present on that cpu's list.
> >
> > If the listener exits without doing a "deregister", its pid continues to
> > be kept around, obviously not a good thing. So we need some way of
> > detecting the situation (task is no longer listening on
> > these cpus events) that is efficient.
>
> Also need to address the case where the listener has closed off his file
> descriptor but continues to run.
>
> So hooking into listener's exit() isn't appropriate - the teardown is
> associated with the lifetime of the fd, not of the process. If we do that,
> exit() gets handled for free.
If you are always going to send unicast messages, then -ECONNREFUSED
will tell you the listener has closed their fd - this doesnt meant it
has exited. Besides that one process could open several sockets. I know
that would not be the app you would write - but it doesnt stop other
people from doing it.
I think i may not follow what you are doing - for some reason i thought
you may have many listeners in user space and these messages get
multicast to them?
Does the user space program somehow communicate its pid to the kernel?
cheers,
jamal
^ permalink raw reply
* Re: strict isolation of net interfaces
From: Daniel Lezcano @ 2006-07-04 12:29 UTC (permalink / raw)
To: Andrey Savochkin
Cc: Sam Vilain, Serge E. Hallyn, Cedric Le Goater, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Ben Greear, Dave Hansen, Alexey Kuznetsov,
Eric W. Biederman
In-Reply-To: <20060703185350.A16826@castle.nmd.msu.ru>
Andrey Savochkin wrote:
>
> I still can't completely understand your direction of thoughts.
> Could you elaborate on IP address assignment in your diagram, please? For
> example, guest0 wants 127.0.0.1 and 192.168.0.1 addresses on its lo
> interface, and 10.1.1.1 on its eth0 interface.
> Does this diagram assume any local IP addresses on v* interfaces in the
> "host"?
>
> And the second question.
> Are vlo0, veth0, etc. devices supposed to have hard_xmit routines?
Andrey,
some people are interested by a network full isolation/virtualization
like you did with the layer 2 isolation and some other people are
interested by a light network isolation done at the layer 3. This one is
intended to implement "application container" aka "lightweight container".
In the case of a layer 3 isolation, the network interface is not totally
isolated and the debate here is to find a way to have something
intuitive to manage the network devices.
IHMO, all the discussion we had convinced me of the needs to have the
possibility to choose between a layer 2 or a layer 3 isolation.
If it is ok for you, we can collaborate to merge the two solutions in
one. I will focus on layer 3 isolation and you on the layer 2.
Regards
- Daniel
^ permalink raw reply
* Re: Network performance degradation from 2.6.11.12 to 2.6.16.20
From: Andi Kleen @ 2006-07-04 11:54 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Willy Tarreau, Harry Edmon, linux-kernel, netdev
In-Reply-To: <Pine.LNX.4.61.0607041333030.18483@ask.diku.dk>
On Tuesday 04 July 2006 13:41, Jesper Dangaard Brouer wrote:
>
> On Mon, 26 Jun 2006, Andi Kleen wrote:
>
> >> I encountered the same problem on a dual core opteron equipped with a
> >> broadcom NIC (tg3) under 2.4. It could receive 1 Mpps when using TSC
> >> as the clock source, but the time jumped back and forth, so I changed
> >> it to 'notsc', then the performance dropped dramatically to around the
> >> same value as above with one CPU saturated. I suspect that the clock
> >> precision is needed by the tg3 driver to correctly decide to switch to
> >> polling mode, but unfortunately, the performance drop rendered the
> >> solution so much unusable that I finally decided to use it only in
> >> uniprocessor with TSC enabled.
> >
> > 2.6 is more clever at this than 2.4. In particular it does the timestamp
> > for each packet only when actually needed, which is relativelt rare.
> >
> > Old experiences do not always apply to new kernels.
>
> Note, that I experinced this problem on 2.6.
>
> Actually the change happens between kernel version 2.6.15 and 2.6.16.
The timestamp optimizations are older. Don't remember the exact release,
but earlier 2.6.
> And
> is a result of Andi's changes to arch/x86_64/Kconfig and
> drivers/acpi/Kconfig, which "allows/activates" the use of the timer on
> x86_64.
Not sure what you mean here?
2.6.18 will likely be more aggressive at using the TSC on i386 on
Intel systems where possible, but x86-64 did this already for a long time.
When x86-64 uses non TSC then it's because using the TSC is not safe.
-Andi
^ permalink raw reply
* Re: Network performance degradation from 2.6.11.12 to 2.6.16.20
From: Jesper Dangaard Brouer @ 2006-07-04 11:41 UTC (permalink / raw)
To: Andi Kleen; +Cc: Willy Tarreau, Harry Edmon, linux-kernel, netdev
In-Reply-To: <200606260723.43209.ak@suse.de>
On Mon, 26 Jun 2006, Andi Kleen wrote:
>> I encountered the same problem on a dual core opteron equipped with a
>> broadcom NIC (tg3) under 2.4. It could receive 1 Mpps when using TSC
>> as the clock source, but the time jumped back and forth, so I changed
>> it to 'notsc', then the performance dropped dramatically to around the
>> same value as above with one CPU saturated. I suspect that the clock
>> precision is needed by the tg3 driver to correctly decide to switch to
>> polling mode, but unfortunately, the performance drop rendered the
>> solution so much unusable that I finally decided to use it only in
>> uniprocessor with TSC enabled.
>
> 2.6 is more clever at this than 2.4. In particular it does the timestamp
> for each packet only when actually needed, which is relativelt rare.
>
> Old experiences do not always apply to new kernels.
Note, that I experinced this problem on 2.6.
Actually the change happens between kernel version 2.6.15 and 2.6.16. And
is a result of Andi's changes to arch/x86_64/Kconfig and
drivers/acpi/Kconfig, which "allows/activates" the use of the timer on
x86_64.
Cheers,
Jesper Brouer
--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
^ permalink raw reply
* Re: [VLAN]: translate IF_OPER_DORMANT to netif_dormant_on()
From: Patrick McHardy @ 2006-07-04 10:07 UTC (permalink / raw)
To: stefan; +Cc: Linux Netdev List
In-Reply-To: <200603211829.k2LITMNR029085@hera.kernel.org>
> commit ddd7bf9fe4e59afc0a041378f82b6e1aa88f714b
> tree 98764adba1bae7d128d2e7db7d9fc1e2fe5826d8
> parent b00055aacdb172c05067612278ba27265fcd05ce
> author Stefan Rompf <stefan@loplof.de> Tue, 21 Mar 2006 09:11:41 -0800
> committer David S. Miller <davem@davemloft.net> Tue, 21 Mar 2006 09:11:41 -0800
>
> [VLAN]: translate IF_OPER_DORMANT to netif_dormant_on()
>
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index fa76220..3948949 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -69,7 +69,7 @@ static struct packet_type vlan_packet_ty
>
> /* Bits of netdev state that are propagated from real device to virtual */
> #define VLAN_LINK_STATE_MASK \
> - ((1<<__LINK_STATE_PRESENT)|(1<<__LINK_STATE_NOCARRIER))
> + ((1<<__LINK_STATE_PRESENT)|(1<<__LINK_STATE_NOCARRIER)|(1<<__LINK_STATE_DORMANT))
>
> /* End of global variables definitions. */
>
> @@ -450,7 +470,7 @@ static struct net_device *register_vlan_
> new_dev->flags = real_dev->flags;
> new_dev->flags &= ~IFF_UP;
>
> - new_dev->state = real_dev->state & VLAN_LINK_STATE_MASK;
> + new_dev->state = real_dev->state & ~(1<<__LINK_STATE_START);
>
> /* need 4 bytes for extra VLAN header info,
> * hope the underlying device can handle it.
This introduced a regression by propagating the __LINK_STATE_XOFF flag,
when the queue of the underlying device is stopped it will be stopped
for the VLAN device too and never be woken up. Since you changed
VLAN_LINK_STATE_MASK, I assume the intention was to just add
__LINK_STATE_DORMANT to the propagated flags and keep using it here?
^ permalink raw reply
* Re: tiacx - don't use UTS_RELEASE
From: Arjan van de Ven @ 2006-07-04 10:00 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Andrew Morton, netdev
In-Reply-To: <20060704095105.GB24298@mars.ravnborg.org>
On Tue, 2006-07-04 at 11:51 +0200, Sam Ravnborg wrote:
> On Tue, Jul 04, 2006 at 11:27:27AM +0200, Arjan van de Ven wrote:
> > On Tue, 2006-07-04 at 02:25 -0700, Andrew Morton wrote:
> > > On Tue, 04 Jul 2006 11:07:59 +0200
> > > Arjan van de Ven <arjan@infradead.org> wrote:
> > >
> > > > patch below removes the use of UTS_RELEASE from the tiacx driver; there
> > > > is absolutely no reason for a driver to print the kernel version or use
> > > > the UTS_RELEASE field; in addition this field changes all the time so
> > > > this causes spurious rebuilds..
> > >
> > > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/usb-storage-uname-in-pr-sc-unneeded-message.patch did it too.
> > >
> > > UTS_RELEASE doesn't change much. It's "2.6.17".
> >
> > no but the header that it's in changes all the time iirc, at least it
> > used to (one of those kbuild regenerated files)
> Yesterday I pushed a change that splitted include/linux/version.h in two
> parts.
> Now include/linux/version.h only contains:
> #define LINUX_VERSION_CODE 132625
> #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
>
> And the file wil only be regenerated when the file-content actually
> changes.
>
> And UTS_RELEASE has moved to include/linux/utsrelease.h which contains:
> #define UTS_RELEASE "2.6.17-g05668381-dirty"
>
> This is the file that will change often - at least for git users.
> But with the patch only users of UTS_RELEASE will be rebuild which is
> far less than users of version.h.
which is a good thing, and we should keep users of utsrelease.h to a
minimum... hence my patch to eliminate a user ;) (which used it to do a
printk.. but if you use a kernel the version is already in dmesg, no
need to printk it again :)
^ permalink raw reply
* Re: tiacx - don't use UTS_RELEASE
From: Sam Ravnborg @ 2006-07-04 9:51 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: Andrew Morton, netdev
In-Reply-To: <1152005247.3109.22.camel@laptopd505.fenrus.org>
On Tue, Jul 04, 2006 at 11:27:27AM +0200, Arjan van de Ven wrote:
> On Tue, 2006-07-04 at 02:25 -0700, Andrew Morton wrote:
> > On Tue, 04 Jul 2006 11:07:59 +0200
> > Arjan van de Ven <arjan@infradead.org> wrote:
> >
> > > patch below removes the use of UTS_RELEASE from the tiacx driver; there
> > > is absolutely no reason for a driver to print the kernel version or use
> > > the UTS_RELEASE field; in addition this field changes all the time so
> > > this causes spurious rebuilds..
> >
> > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/usb-storage-uname-in-pr-sc-unneeded-message.patch did it too.
> >
> > UTS_RELEASE doesn't change much. It's "2.6.17".
>
> no but the header that it's in changes all the time iirc, at least it
> used to (one of those kbuild regenerated files)
Yesterday I pushed a change that splitted include/linux/version.h in two
parts.
Now include/linux/version.h only contains:
#define LINUX_VERSION_CODE 132625
#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
And the file wil only be regenerated when the file-content actually
changes.
And UTS_RELEASE has moved to include/linux/utsrelease.h which contains:
#define UTS_RELEASE "2.6.17-g05668381-dirty"
This is the file that will change often - at least for git users.
But with the patch only users of UTS_RELEASE will be rebuild which is
far less than users of version.h.
Sam
^ permalink raw reply
* Get the freshest It is fully guaranteed it will help you to shoot more and more
From: Bradley @ 2006-07-04 9:47 UTC (permalink / raw)
To: nux-newbie-owner
Dear member.
Prolong your ssex. You have smalll peniis? How would you like a much bigger peenis? Come on in here: http://basszass.com/gal/ms
Coin a phrase Time is a great healer Out of sight, out of mind When It Rains and the Sun Shines at the Same Time the Devil is Beating his Wife Windows will never cease. For they shall be an ornament of grace unto thy head, and chains about thy neck.
^ permalink raw reply
* Re: tiacx - don't use UTS_RELEASE
From: Arjan van de Ven @ 2006-07-04 9:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: netdev
In-Reply-To: <20060704022537.97f703b4.akpm@osdl.org>
On Tue, 2006-07-04 at 02:25 -0700, Andrew Morton wrote:
> On Tue, 04 Jul 2006 11:07:59 +0200
> Arjan van de Ven <arjan@infradead.org> wrote:
>
> > patch below removes the use of UTS_RELEASE from the tiacx driver; there
> > is absolutely no reason for a driver to print the kernel version or use
> > the UTS_RELEASE field; in addition this field changes all the time so
> > this causes spurious rebuilds..
>
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/usb-storage-uname-in-pr-sc-unneeded-message.patch did it too.
>
> UTS_RELEASE doesn't change much. It's "2.6.17".
no but the header that it's in changes all the time iirc, at least it
used to (one of those kbuild regenerated files)
^ permalink raw reply
* Re: tiacx - don't use UTS_RELEASE
From: Andrew Morton @ 2006-07-04 9:25 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: netdev
In-Reply-To: <1152004080.3109.18.camel@laptopd505.fenrus.org>
On Tue, 04 Jul 2006 11:07:59 +0200
Arjan van de Ven <arjan@infradead.org> wrote:
> patch below removes the use of UTS_RELEASE from the tiacx driver; there
> is absolutely no reason for a driver to print the kernel version or use
> the UTS_RELEASE field; in addition this field changes all the time so
> this causes spurious rebuilds..
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-04-usb/usb-storage-uname-in-pr-sc-unneeded-message.patch did it too.
UTS_RELEASE doesn't change much. It's "2.6.17".
^ permalink raw reply
* Re: [patch 1/7] net_device list cleanup: core
From: Christoph Hellwig @ 2006-07-04 9:10 UTC (permalink / raw)
To: Andrey Savochkin
Cc: Christoph Hellwig, netdev, Kirill Korotaev, Eric W. Biederman,
Alexey Kuznetsov
In-Reply-To: <20060704112405.A25305@castle.nmd.msu.ru>
On Tue, Jul 04, 2006 at 11:24:05AM +0400, Andrey Savochkin wrote:
> > Yes, it's a little more work as you need to audit all drivers to see what
> > they are doing and find suitable abstractions but it's a must have that
> > should have been done a lot earlier.
>
> Hiding dev_base_head can be done by converting first_netdev/next_netdev into
> functions and implementing for_each_netdev loop through them.
>
> Or are you talking about abstractions like functions
> for_each_netdev/find_netdev with callbacks?
an for_each_netdev with a callback makes sense and gives a cleaner
abstraction, yes. I don't think you should need a callback for the lookup
structure.
> Do you think that hiding the list internals is worth the additional
> complexity and substantial increase of the patch size?
Yes, absolutely. We've converted scsi hosts and devices from a model
where drivers could directly access the list to strict iterators in the
2.5 series. It's quite a lot of work as you have to understand what
the drivers actually do (and to at least 50% they were doing something
really stupid) and convert them to the right abstractions.
^ permalink raw reply
* tiacx - don't use UTS_RELEASE
From: Arjan van de Ven @ 2006-07-04 9:07 UTC (permalink / raw)
To: akpm; +Cc: netdev
Hi,
patch below removes the use of UTS_RELEASE from the tiacx driver; there
is absolutely no reason for a driver to print the kernel version or use
the UTS_RELEASE field; in addition this field changes all the time so
this causes spurious rebuilds..
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
drivers/net/wireless/tiacx/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6.17-mm4/drivers/net/wireless/tiacx/pci.c
===================================================================
--- linux-2.6.17-mm4.orig/drivers/net/wireless/tiacx/pci.c
+++ linux-2.6.17-mm4/drivers/net/wireless/tiacx/pci.c
@@ -1705,8 +1705,8 @@ acxpci_e_probe(struct pci_dev *pdev, con
/* acx_sem_unlock(adev); */
printk("acx "ACX_RELEASE": net device %s, driver compiled "
- "against wireless extensions %d and Linux %s\n",
- ndev->name, WIRELESS_EXT, UTS_RELEASE);
+ "against wireless extensions %d\n",
+ ndev->name, WIRELESS_EXT);
#if CMD_DISCOVERY
great_inquisitor(adev);
^ permalink raw reply
* Re: [patch 1/7] net_device list cleanup: core
From: Andrey Savochkin @ 2006-07-04 7:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: netdev, Kirill Korotaev, Eric W. Biederman, Alexey Kuznetsov
In-Reply-To: <20060703174650.GA22295@infradead.org>
Christoph,
On Mon, Jul 03, 2006 at 06:46:50PM +0100, Christoph Hellwig wrote:
> On Mon, Jul 03, 2006 at 12:18:51PM +0400, Andrey Savochkin wrote:
> > Cleanup of net_device list use in net_dev core and IP.
> > The cleanup consists of
> > - converting the to list_head, to make the list double-linked (thus making
> > remove operation O(1)), and list walks more readable;
> > - introducing of for_each_netdev wrapper over list_for_each.
>
> When you change all this please make sure dev_base_head is never directly
> accessed anymore, not even through macros and dev_base_head is not exported
> anymore. That's the only way to keep drivers messing with it.
>
> Yes, it's a little more work as you need to audit all drivers to see what
> they are doing and find suitable abstractions but it's a must have that
> should have been done a lot earlier.
Hiding dev_base_head can be done by converting first_netdev/next_netdev into
functions and implementing for_each_netdev loop through them.
Or are you talking about abstractions like functions
for_each_netdev/find_netdev with callbacks?
Do you think that hiding the list internals is worth the additional
complexity and substantial increase of the patch size?
Andrey
^ 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