* [PATCH 05/10] parisc: use RCU to find network device
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Kyle McMartin, Helge Deller, Alexander Beregalov
Cc: netdev, linux-parisc
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: parisc-rdlock.patch --]
[-- Type: text/plain, Size: 943 bytes --]
Another place where RCU can be used instead of read_lock(&dev_base_lock)
This is by inspection, don't have platform or cross-build environment
to validate.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/parisc/led.c 2009-11-09 22:19:07.223480872 -0800
+++ b/drivers/parisc/led.c 2009-11-10 09:28:38.279438787 -0800
@@ -354,9 +354,8 @@ static __inline__ int led_get_net_activi
/* we are running as a workqueue task, so locking dev_base
* for reading should be OK */
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
const struct net_device_stats *stats;
struct in_device *in_dev = __in_dev_get_rcu(dev);
if (!in_dev || !in_dev->ifa_list)
@@ -368,7 +367,6 @@ static __inline__ int led_get_net_activi
tx_total += stats->tx_packets;
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
retval = 0;
--
^ permalink raw reply
* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Jarek Poplawski @ 2009-11-10 17:57 UTC (permalink / raw)
To: Patrick McHardy
Cc: Linux Netdev List, Herbert Xu, David S. Miller, Stephen Hemminger
In-Reply-To: <4AF9A36F.7070807@trash.net>
On Tue, Nov 10, 2009 at 06:31:27PM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > On Mon, Nov 09, 2009 at 08:41:36PM +0100, Patrick McHardy wrote:
> >> I've updated my patch to propagate error values (errno and NET_XMIT
> >> codes) through ndo_hard_start_xmit() and incorporated the suggestions
> >> made last time, namely:
> >>
> >> - move slightly complicated return value check to inline function and
> >> add a few comments
> >>
> >> - fix error handling while in the middle of transmitting GSO skbs
> >>
> >> I've also audited the tree once again for invalid return values and
> >> found a single remaining instance in a Wimax driver, I'll take care
> >> of that later.
> >>
> >> Two questions remain:
> >>
> >> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
> >> skbs is optimal. When the driver returns an error, it is assumed
> >> the current segment has been freed. The patch then frees the
> >> entire GSO skb, including all remaining segments. Alternatively
> >> it could try to transmit the remaining segments later.
> >
> > Anyway, it seems this freeing should be described in the changelog,
> > if not moved to a separate patch, since it fixes another problem,
> > unless I forgot something.
>
> What other problem are you refering to? I'm not aware of any
> problems in the existing function.
This patch is about propagating errors, so it's not clear why there
are some additional kfrees mixed with this. (But I see it's explained
below.)
>
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index bf629ac..1f5752d 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -1756,7 +1756,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> >> struct netdev_queue *txq)
> >> {
> >> const struct net_device_ops *ops = dev->netdev_ops;
> >> - int rc;
> >> + int rc = NETDEV_TX_OK;
> >
> > Isn't it enough to add this in one place only: before this
> > "goto out_kfree_skb"?
>
> Its only exists once in the version I sent out earlier.
>
> >> if (likely(!skb->next)) {
> >> if (!list_empty(&ptype_all))
> >> @@ -1804,6 +1804,8 @@ gso:
> >> nskb->next = NULL;
> >> rc = ops->ndo_start_xmit(nskb, dev);
> >> if (unlikely(rc != NETDEV_TX_OK)) {
> >> + if (rc & ~NETDEV_TX_MASK)
> >> + goto out_kfree_gso_skb;
> >
> > If e.g. (rc == NETDEV_TX_OK | NET_XMIT_CN), why exactly is this freeing
> > necessary now?
> >
> > Is e.g. (rc == NETDEV_TX_BUSY | NET_XMIT_CN) legal? If so, there would
> > be use after kfree, I guess. Otherwise, it should be documented above
> > (and maybe checked somewhere as well).
>
> NET_XMIT_CN is a valid return value, yes. But its not freeing the
> transmitted segment but the remaining ones. Its not strictly
> necessary, but its the easiest way to treat all errors similar.
> Otherwise you get complicated cases, f.i. when the driver returns
> NET_XMIT_CN for the first segment and NETDEV_TX_OK for the
> remaining ones.
It should be in the changelog and maybe a comment too. Even if it's
right it's a change of functionality/behavior here.
I still don't know if/why (rc == NETDEV_TX_BUSY | NET_XMIT_CN) is
OK. IMHO skb will be requeued after kfree here.
>
> >> nskb->next = skb->next;
> >> skb->next = nskb;
> >> return rc;
> >> @@ -1813,11 +1815,14 @@ gso:
> >> return NETDEV_TX_BUSY;
> >> } while (skb->next);
> >>
> >> - skb->destructor = DEV_GSO_CB(skb)->destructor;
> >> + rc = NETDEV_TX_OK;
> >
> > When is (rc != NETDEV_TX_OK) possible in this place?
>
> Its gone in the current version.
Why don't you send the current version?
Jarek P.
^ permalink raw reply
* [PATCH 10/10] CAN: use dev_get_by_index_rcu
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Oliver Hartkopp, Alexey Dobriyan, Lothar Wassmann; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: bcm-dev-rcu.patch --]
[-- Type: text/plain, Size: 614 bytes --]
Use new function to avoid doing read_lock().
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/can/bcm.c 2009-11-10 09:45:16.301376272 -0800
+++ b/net/can/bcm.c 2009-11-10 09:46:30.125005956 -0800
@@ -139,13 +139,13 @@ static char *bcm_proc_getifname(char *re
if (!ifindex)
return "any";
- read_lock(&dev_base_lock);
- dev = __dev_get_by_index(&init_net, ifindex);
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(&init_net, ifindex);
if (dev)
strcpy(result, dev->name);
else
strcpy(result, "???");
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return result;
}
--
^ permalink raw reply
* [PATCH 07/10] decnet: use RCU to find network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Christine Caulfield, David S. Miller, Hannes Eder,
Alexey
Cc: netdev, linux-decnet-users
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: decnet-readlock.patch --]
[-- Type: text/plain, Size: 1576 bytes --]
When showing device statistics use RCU rather than read_lock(&dev_base_lock)
Compile tested only.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/decnet/dn_dev.c 2009-11-10 09:30:55.557376454 -0800
+++ b/net/decnet/dn_dev.c 2009-11-10 09:40:03.847005394 -0800
@@ -856,9 +856,7 @@ int dn_dev_bind_default(__le16 *addr)
dev = dn_dev_get_default();
last_chance:
if (dev) {
- read_lock(&dev_base_lock);
rv = dn_dev_get_first(dev, addr);
- read_unlock(&dev_base_lock);
dev_put(dev);
if (rv == 0 || dev == init_net.loopback_dev)
return rv;
@@ -1323,18 +1321,18 @@ static inline int is_dn_dev(struct net_d
}
static void *dn_dev_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(&dev_base_lock)
+ __acquires(rcu)
{
int i;
struct net_device *dev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
if (*pos == 0)
return SEQ_START_TOKEN;
i = 1;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
if (!is_dn_dev(dev))
continue;
@@ -1355,7 +1353,7 @@ static void *dn_dev_seq_next(struct seq_
if (v == SEQ_START_TOKEN)
dev = net_device_entry(&init_net.dev_base_head);
- for_each_netdev_continue(&init_net, dev) {
+ for_each_netdev_continue_rcu(&init_net, dev) {
if (!is_dn_dev(dev))
continue;
@@ -1366,9 +1364,9 @@ static void *dn_dev_seq_next(struct seq_
}
static void dn_dev_seq_stop(struct seq_file *seq, void *v)
- __releases(&dev_base_lock)
+ __releases(rcu)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static char *dn_type2asc(char type)
--
^ permalink raw reply
* [PATCH 09/10] IPV4: use rcu to walk list of devices in IGMP
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: ipv4-igmp.patch --]
[-- Type: text/plain, Size: 2400 bytes --]
This also needs to be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv4/igmp.c 2009-11-09 22:19:08.899543825 -0800
+++ b/net/ipv4/igmp.c 2009-11-10 09:28:38.636438291 -0800
@@ -2311,7 +2311,7 @@ static inline struct ip_mc_list *igmp_mc
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
state->in_dev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct in_device *in_dev;
in_dev = in_dev_get(state->dev);
if (!in_dev)
@@ -2361,9 +2361,9 @@ static struct ip_mc_list *igmp_mc_get_id
}
static void *igmp_mc_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp_mc_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2379,7 +2379,7 @@ static void *igmp_mc_seq_next(struct seq
}
static void igmp_mc_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
if (likely(state->in_dev != NULL)) {
@@ -2388,7 +2388,7 @@ static void igmp_mc_seq_stop(struct seq_
state->in_dev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp_mc_seq_show(struct seq_file *seq, void *v)
@@ -2462,7 +2462,7 @@ static inline struct ip_sf_list *igmp_mc
state->idev = NULL;
state->im = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct in_device *idev;
idev = in_dev_get(state->dev);
if (unlikely(idev == NULL))
@@ -2528,8 +2528,9 @@ static struct ip_sf_list *igmp_mcf_get_i
}
static void *igmp_mcf_seq_start(struct seq_file *seq, loff_t *pos)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp_mcf_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2545,6 +2546,7 @@ static void *igmp_mcf_seq_next(struct se
}
static void igmp_mcf_seq_stop(struct seq_file *seq, void *v)
+ __releases(rcu)
{
struct igmp_mcf_iter_state *state = igmp_mcf_seq_private(seq);
if (likely(state->im != NULL)) {
@@ -2557,7 +2559,7 @@ static void igmp_mcf_seq_stop(struct seq
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp_mcf_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 08/10] ipv6: use RCU to walk list of network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: ipv6-rdlock.patch --]
[-- Type: text/plain, Size: 4200 bytes --]
No longer need read_lock(&dev_base_lock), use RCU instead.
This also needs to be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv6/addrconf.c 2009-11-09 22:19:08.925480813 -0800
+++ b/net/ipv6/addrconf.c 2009-11-10 09:28:38.577438386 -0800
@@ -3828,9 +3828,9 @@ static int inet6_dump_ifinfo(struct sk_b
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
idx = 0;
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if (idx < s_idx)
goto cont;
if ((idev = in6_dev_get(dev)) == NULL)
@@ -3843,7 +3843,7 @@ static int inet6_dump_ifinfo(struct sk_b
cont:
idx++;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
cb->args[0] = idx;
return skb->len;
--- a/net/ipv6/anycast.c 2009-11-09 22:19:08.926480759 -0800
+++ b/net/ipv6/anycast.c 2009-11-10 09:28:38.577438386 -0800
@@ -431,7 +431,7 @@ static inline struct ifacaddr6 *ac6_get_
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (!idev)
@@ -482,9 +482,9 @@ static struct ifacaddr6 *ac6_get_idx(str
}
static void *ac6_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return ac6_get_idx(seq, *pos);
}
@@ -497,14 +497,14 @@ static void *ac6_seq_next(struct seq_fil
}
static void ac6_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct ac6_iter_state *state = ac6_seq_private(seq);
if (likely(state->idev != NULL)) {
read_unlock_bh(&state->idev->lock);
in6_dev_put(state->idev);
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int ac6_seq_show(struct seq_file *seq, void *v)
--- a/net/ipv6/mcast.c 2009-11-09 22:19:08.929480873 -0800
+++ b/net/ipv6/mcast.c 2009-11-10 09:28:38.578438329 -0800
@@ -2375,7 +2375,7 @@ static inline struct ifmcaddr6 *igmp6_mc
struct net *net = seq_file_net(seq);
state->idev = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (!idev)
@@ -2426,9 +2426,9 @@ static struct ifmcaddr6 *igmp6_mc_get_id
}
static void *igmp6_mc_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return igmp6_mc_get_idx(seq, *pos);
}
@@ -2441,7 +2441,7 @@ static void *igmp6_mc_seq_next(struct se
}
static void igmp6_mc_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
if (likely(state->idev != NULL)) {
@@ -2450,7 +2450,7 @@ static void igmp6_mc_seq_stop(struct seq
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mc_seq_show(struct seq_file *seq, void *v)
@@ -2507,7 +2507,7 @@ static inline struct ip6_sf_list *igmp6_
state->idev = NULL;
state->im = NULL;
- for_each_netdev(net, state->dev) {
+ for_each_netdev_rcu(net, state->dev) {
struct inet6_dev *idev;
idev = in6_dev_get(state->dev);
if (unlikely(idev == NULL))
@@ -2573,9 +2573,9 @@ static struct ip6_sf_list *igmp6_mcf_get
}
static void *igmp6_mcf_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
- read_lock(&dev_base_lock);
+ rcu_read_lock();
return *pos ? igmp6_mcf_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -2591,7 +2591,7 @@ static void *igmp6_mcf_seq_next(struct s
}
static void igmp6_mcf_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
struct igmp6_mcf_iter_state *state = igmp6_mcf_seq_private(seq);
if (likely(state->im != NULL)) {
@@ -2604,7 +2604,7 @@ static void igmp6_mcf_seq_stop(struct se
state->idev = NULL;
}
state->dev = NULL;
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int igmp6_mcf_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 06/10] s390: use RCU to walk list of network devices
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Martin Schwidefsky, Heiko Carstens; +Cc: netdev, linux390
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: s390-readlock.patch --]
[-- Type: text/plain, Size: 1004 bytes --]
This is similar to other cases where for_each_netdev_rcu
can be used when gathering information.
By inspection, don't have platform or cross-build environment
to validate.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/arch/s390/appldata/appldata_net_sum.c 2009-11-09 22:19:05.593480476 -0800
+++ b/arch/s390/appldata/appldata_net_sum.c 2009-11-10 09:28:38.335438652 -0800
@@ -83,8 +83,9 @@ static void appldata_get_net_sum_data(vo
rx_dropped = 0;
tx_dropped = 0;
collisions = 0;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
const struct net_device_stats *stats = dev_get_stats(dev);
rx_packets += stats->rx_packets;
@@ -98,7 +99,8 @@ static void appldata_get_net_sum_data(vo
collisions += stats->collisions;
i++;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
+
net_data->nr_interfaces = i;
net_data->rx_packets = rx_packets;
net_data->tx_packets = tx_packets;
--
^ permalink raw reply
* [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Ed L. Cashin, Harvey Harrison,
Bartlomiej Zolnierkiewicz
Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: aoe-rcu.patch --]
[-- Type: text/plain, Size: 1240 bytes --]
This gets rid of another use of read_lock(&dev_base_lock) by using
RCU. Also, it only increments the reference count of the device actually
used rather than holding and releasing every device
Compile tested only.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/block/aoe/aoecmd.c 2009-11-09 22:19:06.082480836 -0800
+++ b/drivers/block/aoe/aoecmd.c 2009-11-10 09:28:38.222438732 -0800
@@ -296,17 +296,18 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
struct sk_buff *skb;
struct net_device *ifp;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, ifp) {
- dev_hold(ifp);
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, ifp) {
if (!is_aoe_netif(ifp))
- goto cont;
+ continue;
skb = new_skb(sizeof *h + sizeof *ch);
if (skb == NULL) {
printk(KERN_INFO "aoe: skb alloc failure\n");
- goto cont;
+ continue;
}
+
+ dev_hold(ifp);
skb_put(skb, sizeof *h + sizeof *ch);
skb->dev = ifp;
__skb_queue_tail(queue, skb);
@@ -320,11 +321,8 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigne
h->major = cpu_to_be16(aoemajor);
h->minor = aoeminor;
h->cmd = AOECMD_CFG;
-
-cont:
- dev_put(ifp);
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static void
--
^ permalink raw reply
* [PATCH 03/10] net: use rcu for network scheduler API
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: sched-rcu.patch --]
[-- Type: text/plain, Size: 752 bytes --]
Use RCU to walk list of network devices in qdisc dump.
This could be optimized for large number of devices.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/sched/sch_api.c 2009-11-09 22:19:09.002480816 -0800
+++ b/net/sched/sch_api.c 2009-11-10 09:28:38.166439067 -0800
@@ -1279,9 +1279,10 @@ static int tc_dump_qdisc(struct sk_buff
s_idx = cb->args[0];
s_q_idx = q_idx = cb->args[1];
- read_lock(&dev_base_lock);
+
+ rcu_read_lock();
idx = 0;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
struct netdev_queue *dev_queue;
if (idx < s_idx)
@@ -1302,7 +1303,7 @@ cont:
}
done:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
cb->args[0] = idx;
cb->args[1] = q_idx;
--
^ permalink raw reply
* [PATCH 02/10] vlan: eliminate use of dev_base_lock
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Patrick McHardy; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: vlan-rdlock.patch --]
[-- Type: text/plain, Size: 1311 bytes --]
Do not need to use read_lock(&dev_base_lock), use RCU instead.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/8021q/vlanproc.c 2009-11-09 22:19:08.712481032 -0800
+++ b/net/8021q/vlanproc.c 2009-11-10 09:27:28.165378176 -0800
@@ -201,18 +201,17 @@ int vlan_proc_rem_dev(struct net_device
/* start read of /proc/net/vlan/config */
static void *vlan_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(rcu)
{
struct net_device *dev;
struct net *net = seq_file_net(seq);
loff_t i = 1;
- read_lock(&dev_base_lock);
-
+ rcu_read_lock();
if (*pos == 0)
return SEQ_START_TOKEN;
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if (!is_vlan_dev(dev))
continue;
@@ -234,7 +233,7 @@ static void *vlan_seq_next(struct seq_fi
if (v == SEQ_START_TOKEN)
dev = net_device_entry(&net->dev_base_head);
- for_each_netdev_continue(net, dev) {
+ for_each_netdev_continue_rcu(net, dev) {
if (!is_vlan_dev(dev))
continue;
@@ -245,9 +244,9 @@ static void *vlan_seq_next(struct seq_fi
}
static void vlan_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(rcu)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int vlan_seq_show(struct seq_file *seq, void *v)
--
^ permalink raw reply
* [PATCH 01/10] netdev: add netdev_continue_rcu
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller, Paul E. McKenney; +Cc: netdev
In-Reply-To: <20091110175446.280423729@vyatta.com>
[-- Attachment #1: continue-rcu.patch --]
[-- Type: text/plain, Size: 1771 bytes --]
This adds an RCU macro for continuing search, useful for some
network devices like vlan.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/include/linux/netdevice.h 2009-11-09 22:19:08.511480873 -0800
+++ b/include/linux/netdevice.h 2009-11-10 09:27:17.973376267 -0800
@@ -1079,6 +1079,8 @@ extern rwlock_t dev_base_lock; /* De
list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list)
#define for_each_netdev_continue(net, d) \
list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_continue_rcu(net, d) \
+ list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
#define net_device_entry(lh) list_entry(lh, struct net_device, dev_list)
static inline struct net_device *next_net_device(struct net_device *dev)
--- a/include/linux/rculist.h 2009-11-09 22:19:08.529480859 -0800
+++ b/include/linux/rculist.h 2009-11-10 09:27:17.974376609 -0800
@@ -262,6 +262,20 @@ static inline void list_splice_init_rcu(
(pos) = rcu_dereference((pos)->next))
/**
+ * list_for_each_entry_continue_rcu - continue iteration over list of given type
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_struct within the struct.
+ *
+ * Continue to iterate over list of given type, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_rcu(pos, head, member) \
+ for (pos = list_entry_rcu(pos->member.next, typeof(*pos), member); \
+ prefetch(pos->member.next), &pos->member != (head); \
+ pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
+
+/**
* hlist_del_rcu - deletes entry from hash list without re-initialization
* @n: the element to delete from the hash list.
*
--
^ permalink raw reply
* [PATCH 00/10] netdev: get rid of read_lock(&dev_base_lock) usages
From: Stephen Hemminger @ 2009-11-10 17:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The goal is to eliminate dev_base_lock completely, and just use RCU
and rtnl_mutex for network devices. This series gets rid of the many
of the users of dev_base_lock just for reading.
--
^ permalink raw reply
* Re: sunrpc port allocation and IANA reserved list
From: Ben Hutchings @ 2009-11-10 17:53 UTC (permalink / raw)
To: Chris Friesen; +Cc: netdev, Linux kernel
In-Reply-To: <4AF9A63B.6010101@nortel.com>
On Tue, 2009-11-10 at 11:43 -0600, Chris Friesen wrote:
> By default sunrpc ports are allocated at random in the range 665-1023U.
> However, there are many ports within this range which have been
> reserved by the IANA (and others like port 921 which are not formally
> reserved but are "well-known").
>
> Given that a userspace application can be stopped and restarted at any
> time, and a sunrpc registration can happen at any time, what is the
> expected mechanism to prevent the kernel from allocating a port for use
> by sunrpc that reserved or well-known?
>
> Apparently Redhat and Debian have distro-specific ways of dealing with
> this, but is there a standard solution? Should there be?
>
> The current setup seems suboptimal.
I believe both RH and Debian are using the same implementation:
<http://cyberelk.net/tim/software/portreserve/>.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* sunrpc port allocation and IANA reserved list
From: Chris Friesen @ 2009-11-10 17:43 UTC (permalink / raw)
To: netdev, Linux kernel
By default sunrpc ports are allocated at random in the range 665-1023U.
However, there are many ports within this range which have been
reserved by the IANA (and others like port 921 which are not formally
reserved but are "well-known").
Given that a userspace application can be stopped and restarted at any
time, and a sunrpc registration can happen at any time, what is the
expected mechanism to prevent the kernel from allocating a port for use
by sunrpc that reserved or well-known?
Apparently Redhat and Debian have distro-specific ways of dealing with
this, but is there a standard solution? Should there be?
The current setup seems suboptimal.
Chris
^ permalink raw reply
* Re: [PATCH] netdev: fold name hash properly (v3)
From: Eric Dumazet @ 2009-11-10 17:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20091110092034.3d4ee9b0@nehalam>
Stephen Hemminger a écrit :
> The full_name_hash function does not produce well distributed values in
> the lower bits, so most code uses hash_32() to fold it. This is really
> a bug introduced when name hashing was added, back in 2.5 when I added
> name hashing.
>
> hash_32 is all that is needed since full_name_hash returns unsigned int
> which is only 32 bits on 64 bit platforms.
>
> Also, there is no point in using hash_32 on ifindex, because the is naturally
> sequential and usually well distributed.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks Stephen
^ permalink raw reply
* Re: [RFC PATCH] net: add dataref destructor to sk_buff
From: Michael S. Tsirkin @ 2009-11-10 17:36 UTC (permalink / raw)
To: Gregory Haskins; +Cc: alacrityvm-devel, herbert.xu, linux-kernel, netdev
In-Reply-To: <4AF98A8C.9040201@novell.com>
On Tue, Nov 10, 2009 at 10:45:16AM -0500, Gregory Haskins wrote:
> Michael S. Tsirkin wrote:
> > On Tue, Nov 10, 2009 at 09:11:10AM -0500, Gregory Haskins wrote:
> >> Michael S. Tsirkin wrote:
> >>> On Tue, Nov 10, 2009 at 05:40:50AM -0700, Gregory Haskins wrote:
> >>>>>>> On 11/10/2009 at 6:53 AM, in message <20091110115335.GC6989@redhat.com>,
> >>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>
> >>>>> Last time this was tried, this is the objection that was voiced:
> >>>>>
> >>>>> The problem with this patch is that it's tracking skb's, while
> >>>>> you want use it to track pages for zero-copy. That just doesn't
> >>>>> work. Through mechanisms like splice, individual pages in the
> >>>>> skb can be detached and metastasize to other locations, e.g.,
> >>>>> the VFS.
> >>>> Right, and I don't think this applies here because I specifically chose the shinfo level to try to properly
> >>>> track the page level avoid this issue. Multiple skb's can point to a single shinfo, iiuc.
> >>> VFS does not know about shinfo either, does it?
> >> I do not follow the reference. Where does VFS come into play?
> >
> > "Through mechanisms like splice, individual pages in the
> > skb can be detached and metastasize to other locations, e.g.,
> > the VFS"
>
> Right, understood. What I mean is: How is that actually used in
> real-life in a way that is valid?
>
> What I am getting at is as follows: From a real basic perspective, you
> can look at all of this as a simple synchronous call (i.e. sendmsg()).
> The "app" (be it a userspace app, or a guest) prepares a buffer for
> transmission, and offers it to the next layer in the stack. The app
> must maintain the integrity of that buffer at least until the layer
> below it signifies that it is "consumed". This may mean its a
> synchronous call, like sendmsg(), or it may be asynchronous, like AIO.
>
> But the key thing here is that at some point, the lower layer has to
> signify that the buffer stability constraint has been met. In either
> case, we have a clear delineated event: the io-completes = the buffer is
> free to be reused.
>
> In the simple case, the buffer in question is copied to a kernel buffer,
> and the io completes immediately. In other cases (such as zero copy),
> the buffer is mapped into the skb, and we have to wait for even lower
> layers to signify the completion.
>
> I am not a stack expert, but I was under the impression that we use this
> model for userspace pages today as well using the wmem callbacks in
> skb->destructor(). If so, I do not see how you could do something like
> detach a page from a pskb and still expect to have a proper event that
> delineates the io-completion to the higher layers.
I think linux only cares about that for accounting purposes (stuff like
socket sndbuff size). If someone takes over the page, the socket can
stop worrying about it.
> So the questions are:
>
> 1) do we in fact map userspace pages to pskbs today?
I don't think so.
> 2a) if so, how do we delineate the completion event?
> 2b) and how do we prevent worrying about the get_page() issue you refer
> to.
>
>
> >>
> >>>>> In other words, this only *seems*
> >>>>> to work for you because you are not trying to do things like
> >>>>> guest to host communication, with host doing smart things.
> >>>> I am not following what you mean here, as I do use this for guest->host and guest->host->remote, and
> >>>> it works quite nicely. I map the guest pages in, and when the last reference to the pages are dropped,
> >>>> I release the pages back to the guest. It doesn't matter if the skb egresses out a physical adapter or is
> >>>> received locally. All that matters is the lifetime of the shinfo (and thus its pages) is handled correctly.
> >>> Not if someone else is referencing the pages without a reference to shinfo.
> >> I agree that if we can reference pages outside of the skb/shinfo then
> >> there is a problem. I wasn't aware that we could do this, tbh.
> >>
> >> However, it seems to me that this is a problem with the overall stack,
> >> if true....isn't it? For instance, if I do a sendmsg() from a userspace
> >> app and block until its consumed,
> >
> > consumed == memcpy_from_iovec?
>
> For non-zero-copy, sure why not.
>
> >
> >> how can the system function sanely if
> >> the app returns from the call but something is still referencing the
> >> page(s)?
> >
> > which pages?
>
> You said that there are paths that get_page() out of shinfo without
> holding a shinfo reference.
Without zero copy, application does not care about these,
they have been allocated by kernel.
--
MST
^ permalink raw reply
* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Patrick McHardy @ 2009-11-10 17:31 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Linux Netdev List, Herbert Xu, David S. Miller, Stephen Hemminger
In-Reply-To: <20091110170838.GA4195@ami.dom.local>
Jarek Poplawski wrote:
> On Mon, Nov 09, 2009 at 08:41:36PM +0100, Patrick McHardy wrote:
>> I've updated my patch to propagate error values (errno and NET_XMIT
>> codes) through ndo_hard_start_xmit() and incorporated the suggestions
>> made last time, namely:
>>
>> - move slightly complicated return value check to inline function and
>> add a few comments
>>
>> - fix error handling while in the middle of transmitting GSO skbs
>>
>> I've also audited the tree once again for invalid return values and
>> found a single remaining instance in a Wimax driver, I'll take care
>> of that later.
>>
>> Two questions remain:
>>
>> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
>> skbs is optimal. When the driver returns an error, it is assumed
>> the current segment has been freed. The patch then frees the
>> entire GSO skb, including all remaining segments. Alternatively
>> it could try to transmit the remaining segments later.
>
> Anyway, it seems this freeing should be described in the changelog,
> if not moved to a separate patch, since it fixes another problem,
> unless I forgot something.
What other problem are you refering to? I'm not aware of any
problems in the existing function.
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index bf629ac..1f5752d 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1756,7 +1756,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>> struct netdev_queue *txq)
>> {
>> const struct net_device_ops *ops = dev->netdev_ops;
>> - int rc;
>> + int rc = NETDEV_TX_OK;
>
> Isn't it enough to add this in one place only: before this
> "goto out_kfree_skb"?
Its only exists once in the version I sent out earlier.
>> if (likely(!skb->next)) {
>> if (!list_empty(&ptype_all))
>> @@ -1804,6 +1804,8 @@ gso:
>> nskb->next = NULL;
>> rc = ops->ndo_start_xmit(nskb, dev);
>> if (unlikely(rc != NETDEV_TX_OK)) {
>> + if (rc & ~NETDEV_TX_MASK)
>> + goto out_kfree_gso_skb;
>
> If e.g. (rc == NETDEV_TX_OK | NET_XMIT_CN), why exactly is this freeing
> necessary now?
>
> Is e.g. (rc == NETDEV_TX_BUSY | NET_XMIT_CN) legal? If so, there would
> be use after kfree, I guess. Otherwise, it should be documented above
> (and maybe checked somewhere as well).
NET_XMIT_CN is a valid return value, yes. But its not freeing the
transmitted segment but the remaining ones. Its not strictly
necessary, but its the easiest way to treat all errors similar.
Otherwise you get complicated cases, f.i. when the driver returns
NET_XMIT_CN for the first segment and NETDEV_TX_OK for the
remaining ones.
>> nskb->next = skb->next;
>> skb->next = nskb;
>> return rc;
>> @@ -1813,11 +1815,14 @@ gso:
>> return NETDEV_TX_BUSY;
>> } while (skb->next);
>>
>> - skb->destructor = DEV_GSO_CB(skb)->destructor;
>> + rc = NETDEV_TX_OK;
>
> When is (rc != NETDEV_TX_OK) possible in this place?
Its gone in the current version.
>> +out_kfree_gso_skb:
>> + if (likely(skb->next == NULL))
>> + skb->destructor = DEV_GSO_CB(skb)->destructor;
>> out_kfree_skb:
>> kfree_skb(skb);
>> - return NETDEV_TX_OK;
>> + return rc;
>> }
>> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
>> index 4ae6aa5..b13821a 100644
>> --- a/net/sched/sch_generic.c
>> +++ b/net/sched/sch_generic.c
>> @@ -120,8 +120,15 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>>
>> HARD_TX_LOCK(dev, txq, smp_processor_id());
>> if (!netif_tx_queue_stopped(txq) &&
>> - !netif_tx_queue_frozen(txq))
>> + !netif_tx_queue_frozen(txq)) {
>> ret = dev_hard_start_xmit(skb, dev, txq);
>> +
>> + /* an error implies that the skb was consumed */
>> + if (ret < 0)
>> + ret = NETDEV_TX_OK;
>
> + else (?)
Another branch vs. an unconditional and operation. I chose the later.
>> + /* all NET_XMIT codes map to NETDEV_TX_OK */
>> + ret &= ~NET_XMIT_MASK;
>> + }
>> HARD_TX_UNLOCK(dev, txq);
>>
>> spin_lock(root_lock);
^ permalink raw reply
* Re: PATCH: Network Device Naming mechanism and policy
From: Stephen Hemminger @ 2009-11-10 17:23 UTC (permalink / raw)
To: Narendra_K
Cc: md, Matt_Domsch, bryan, dannf, bhutchings, netdev, linux-hotplug,
Jordan_Hargrave, Charles_Rose, Sandeep_K_Shandilya
In-Reply-To: <EDA0A4495861324DA2618B4C45DCB3EE589727@blrx3m08.blr.amer.dell.com>
On Mon, 9 Nov 2009 20:11:47 +0530
<Narendra_K@Dell.com> wrote:
>
> >> > As a distribution developer I highly value solutions like
> >this which
> >> > do not require patching every application which deals with
> >interface
> >> > names and then teaching users about aliases which only
> >work in some
> >> > places and are unknown to the kernel.
> >> Fair enough - but would you object if we changed the naming scheme
> >> from eth%d to something else?
> >I suppose that this would depend on what else. :-) Since you
> >want radical changes I recommend that you design the new
> >persistent naming infrastructure in a way that will allow root
> >to choose to use the classic naming scheme, or many users will
> >scream a lot and at least some distributions will do it anyway.
> >I also expect that providing choice at the beginning of
> >development may lead to more acceptance later if and when the
> >new scheme will have proved itself to be superior (at least in
> >some situations).
> >You have tought about this for a long time and if so far you
> >have not found a solution which is widely considered superior
> >then I doubt that one will appear soon. Providing your
> >favourite naming scheme as an optional add on will immediately
> >benefit those who like it and greatly reduce opposition from
> >those who do not.
>
> In that way, I suppose char device node solution fits the scheme
> perfectly. It doesn't change or interfere with the kernel's default
> naming scheme (ethN) in any way. Also, the applications continue to work
> the way they did and in addition to supporting traditional names, they
> would also support pathnames. Whether all the user space applications
> need to be patched can be discussed and debated. But, we can patch
> applications like, installers and firewall code, which when don't see
> determinism ("eth0 mapping to integrated port 1"), fail and cause very
> high impact could be patched. Since users are already familiar with
> pathnames like /dev/disk/by-id{label, uuid}, I suppose it might not be
> very difficult to get used to pathnames like
> /dev/netdev/by-chassis-label/Embedded_NIC_1. Would that be acceptable ?
>
IFNAMSIZ = 16 is hardwired as part of the kernel binary user space API.
Have you observed that the only developers arguing for this come from
outside the normal circle of networking? It seems to be favored only
by those who come to networking from a system or disk point of view.
--
^ permalink raw reply
* DM9000: Wake on LAN support
From: Ben Dooks @ 2009-11-10 17:22 UTC (permalink / raw)
To: netdev; +Cc: davem, Simtec Linux Team
[-- Attachment #1: simtec/ready/dm9000-wakeonlan.patch --]
[-- Type: text/plain, Size: 7418 bytes --]
Add support for Wake on LAN (WOL) reception and waking the device up from
this signal via the ethtool interface. Currently we are only supporting
the magic-packet variant of wakeup.
WOL is enabled by specifying a second interrupt resource to the driver
which indicates where the interrupt for the WOL is being signalled. This
then enables the necessary ethtool calls to leave the device in a state
to receive WOL frames when going into suspend.
Signed-off-by: Ben Dooks <ben@simtec.co.uk>
Signed-off-by: Simtec Linux Team <linux@simtec.co.uk>
---
drivers/net/dm9000.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++---
drivers/net/dm9000.h | 7 ++
2 files changed, 142 insertions(+), 8 deletions(-)
Index: b/drivers/net/dm9000.c
===================================================================
--- a/drivers/net/dm9000.c 2009-11-10 10:22:37.000000000 +0000
+++ b/drivers/net/dm9000.c 2009-11-10 10:26:21.000000000 +0000
@@ -100,6 +100,7 @@ typedef struct board_info {
unsigned int flags;
unsigned int in_suspend :1;
+ unsigned int wake_supported :1;
int debug_level;
enum dm9000_type type;
@@ -116,6 +117,8 @@ typedef struct board_info {
struct resource *data_req;
struct resource *irq_res;
+ int irq_wake;
+
struct mutex addr_lock; /* phy and eeprom access lock */
struct delayed_work phy_poll;
@@ -125,6 +128,7 @@ typedef struct board_info {
struct mii_if_info mii;
u32 msg_enable;
+ u32 wake_state;
int rx_csum;
int can_csum;
@@ -568,6 +572,54 @@ static int dm9000_set_eeprom(struct net_
return 0;
}
+static void dm9000_get_wol(struct net_device *dev, struct ethtool_wolinfo *w)
+{
+ board_info_t *dm = to_dm9000_board(dev);
+
+ memset(w, 0, sizeof(struct ethtool_wolinfo));
+
+ /* note, we could probably support wake-phy too */
+ w->supported = dm->wake_supported ? WAKE_MAGIC : 0;
+ w->wolopts = dm->wake_state;
+}
+
+static int dm9000_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
+{
+ board_info_t *dm = to_dm9000_board(dev);
+ unsigned long flags;
+ u32 opts = w->wolopts;
+ u32 wcr = 0;
+
+ if (!dm->wake_supported)
+ return -EOPNOTSUPP;
+
+ if (opts & ~WAKE_MAGIC)
+ return -EINVAL;
+
+ if (opts & WAKE_MAGIC)
+ wcr |= WCR_MAGICEN;
+
+ mutex_lock(&dm->addr_lock);
+
+ spin_lock_irqsave(&dm->lock, flags);
+ iow(dm, DM9000_WCR, wcr);
+ spin_unlock_irqrestore(&dm->lock, flags);
+
+ mutex_unlock(&dm->addr_lock);
+
+ if (dm->wake_state != opts) {
+ /* change in wol state, update IRQ state */
+
+ if (!dm->wake_state)
+ set_irq_wake(dm->irq_wake, 1);
+ else if (dm->wake_state & !opts)
+ set_irq_wake(dm->irq_wake, 0);
+ }
+
+ dm->wake_state = opts;
+ return 0;
+}
+
static const struct ethtool_ops dm9000_ethtool_ops = {
.get_drvinfo = dm9000_get_drvinfo,
.get_settings = dm9000_get_settings,
@@ -576,6 +628,8 @@ static const struct ethtool_ops dm9000_e
.set_msglevel = dm9000_set_msglevel,
.nway_reset = dm9000_nway_reset,
.get_link = dm9000_get_link,
+ .get_wol = dm9000_get_wol,
+ .set_wol = dm9000_set_wol,
.get_eeprom_len = dm9000_get_eeprom_len,
.get_eeprom = dm9000_get_eeprom,
.set_eeprom = dm9000_set_eeprom,
@@ -722,6 +776,7 @@ dm9000_init_dm9000(struct net_device *de
{
board_info_t *db = netdev_priv(dev);
unsigned int imr;
+ unsigned int ncr;
dm9000_dbg(db, 1, "entering %s\n", __func__);
@@ -736,8 +791,15 @@ dm9000_init_dm9000(struct net_device *de
iow(db, DM9000_GPCR, GPCR_GEP_CNTL); /* Let GPIO0 output */
iow(db, DM9000_GPR, 0); /* Enable PHY */
- if (db->flags & DM9000_PLATF_EXT_PHY)
- iow(db, DM9000_NCR, NCR_EXT_PHY);
+ ncr = (db->flags & DM9000_PLATF_EXT_PHY) ? NCR_EXT_PHY : 0;
+
+ /* if wol is needed, then always set NCR_WAKEEN otherwise we end
+ * up dumping the wake events if we disable this. There is already
+ * a wake-mask in DM9000_WCR */
+ if (db->wake_supported)
+ ncr |= NCR_WAKEEN;
+
+ iow(db, DM9000_NCR, ncr);
/* Program operating register */
iow(db, DM9000_TCR, 0); /* TX Polling clear */
@@ -1045,6 +1107,41 @@ static irqreturn_t dm9000_interrupt(int
return IRQ_HANDLED;
}
+static irqreturn_t dm9000_wol_interrupt(int irq, void *dev_id)
+{
+ struct net_device *dev = dev_id;
+ board_info_t *db = netdev_priv(dev);
+ unsigned long flags;
+ unsigned nsr, wcr;
+
+ spin_lock_irqsave(&db->lock, flags);
+
+ nsr = ior(db, DM9000_NSR);
+ wcr = ior(db, DM9000_WCR);
+
+ dev_dbg(db->dev, "%s: NSR=0x%02x, WCR=0x%02x\n", __func__, nsr, wcr);
+
+ if (nsr & NSR_WAKEST) {
+ /* clear, so we can avoid */
+ iow(db, DM9000_NSR, NSR_WAKEST);
+
+ if (wcr & WCR_LINKST)
+ dev_info(db->dev, "wake by link status change\n");
+ if (wcr & WCR_SAMPLEST)
+ dev_info(db->dev, "wake by sample packet\n");
+ if (wcr & WCR_MAGICST )
+ dev_info(db->dev, "wake by magic packet\n");
+ if (!(wcr & (WCR_LINKST | WCR_SAMPLEST | WCR_MAGICST)))
+ dev_err(db->dev, "wake signalled with no reason? "
+ "NSR=0x%02x, WSR=0x%02x\n", nsr, wcr);
+
+ }
+
+ spin_unlock_irqrestore(&db->lock, flags);
+
+ return (nsr & NSR_WAKEST) ? IRQ_HANDLED : IRQ_NONE;
+}
+
#ifdef CONFIG_NET_POLL_CONTROLLER
/*
*Used by netconsole
@@ -1299,6 +1396,29 @@ dm9000_probe(struct platform_device *pde
goto out;
}
+ db->irq_wake = platform_get_irq(pdev, 1);
+ if (db->irq_wake >= 0) {
+ dev_dbg(db->dev, "wakeup irq %d\n", db->irq_wake);
+
+ ret = request_irq(db->irq_wake, dm9000_wol_interrupt,
+ IRQF_SHARED, dev_name(db->dev), ndev);
+ if (ret) {
+ dev_err(db->dev, "cannot get wakeup irq (%d)\n", ret);
+ } else {
+
+ /* test to see if irq is really wakeup capable */
+ ret = set_irq_wake(db->irq_wake, 1);
+ if (ret) {
+ dev_err(db->dev, "irq %d cannot set wakeup (%d)\n",
+ db->irq_wake, ret);
+ ret = 0;
+ } else {
+ set_irq_wake(db->irq_wake, 0);
+ db->wake_supported = 1;
+ }
+ }
+ }
+
iosize = resource_size(db->addr_res);
db->addr_req = request_mem_region(db->addr_res->start, iosize,
pdev->name);
@@ -1490,10 +1610,14 @@ dm9000_drv_suspend(struct device *dev)
db = netdev_priv(ndev);
db->in_suspend = 1;
- if (netif_running(ndev)) {
- netif_device_detach(ndev);
+ if (!netif_running(ndev))
+ return 0;
+
+ netif_device_detach(ndev);
+
+ /* only shutdown if not using WoL */
+ if (!db->wake_state)
dm9000_shutdown(ndev);
- }
}
return 0;
}
@@ -1506,10 +1630,13 @@ dm9000_drv_resume(struct device *dev)
board_info_t *db = netdev_priv(ndev);
if (ndev) {
-
if (netif_running(ndev)) {
- dm9000_reset(db);
- dm9000_init_dm9000(ndev);
+ /* reset if we were not in wake mode to ensure if
+ * the device was powered off it is in a known state */
+ if (!db->wake_state) {
+ dm9000_reset(db);
+ dm9000_init_dm9000(ndev);
+ }
netif_device_attach(ndev);
}
Index: b/drivers/net/dm9000.h
===================================================================
--- a/drivers/net/dm9000.h 2009-11-10 10:22:37.000000000 +0000
+++ b/drivers/net/dm9000.h 2009-11-10 10:25:44.000000000 +0000
@@ -111,6 +111,13 @@
#define RSR_CE (1<<1)
#define RSR_FOE (1<<0)
+#define WCR_LINKEN (1 << 5)
+#define WCR_SAMPLEEN (1 << 4)
+#define WCR_MAGICEN (1 << 3)
+#define WCR_LINKST (1 << 2)
+#define WCR_SAMPLEST (1 << 1)
+#define WCR_MAGICST (1 << 0)
+
#define FCTR_HWOT(ot) (( ot & 0xf ) << 4 )
#define FCTR_LWOT(ot) ( ot & 0xf )
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* [PATCH] netdev: fold name hash properly (v3)
From: Stephen Hemminger @ 2009-11-10 17:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The full_name_hash function does not produce well distributed values in
the lower bits, so most code uses hash_32() to fold it. This is really
a bug introduced when name hashing was added, back in 2.5 when I added
name hashing.
hash_32 is all that is needed since full_name_hash returns unsigned int
which is only 32 bits on 64 bit platforms.
Also, there is no point in using hash_32 on ifindex, because the is naturally
sequential and usually well distributed.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
Patch against net-next-2.6
--- a/net/core/dev.c 2009-11-09 22:19:08.731481514 -0800
+++ b/net/core/dev.c 2009-11-10 08:42:23.755003727 -0800
@@ -79,6 +79,7 @@
#include <linux/cpu.h>
#include <linux/types.h>
#include <linux/kernel.h>
+#include <linux/hash.h>
#include <linux/sched.h>
#include <linux/mutex.h>
#include <linux/string.h>
@@ -196,7 +197,7 @@ EXPORT_SYMBOL(dev_base_lock);
static inline struct hlist_head *dev_name_hash(struct net *net, const char *name)
{
unsigned hash = full_name_hash(name, strnlen(name, IFNAMSIZ));
- return &net->dev_name_head[hash & (NETDEV_HASHENTRIES - 1)];
+ return &net->dev_name_head[hash_32(hash, NETDEV_HASHBITS)];
}
static inline struct hlist_head *dev_index_hash(struct net *net, int ifindex)
^ permalink raw reply
* Re: next tree for October 27: znet build failure
From: Randy Dunlap @ 2009-11-10 17:19 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Sachin Sant, linux-next, netdev, David Miller, John W. Linville,
Greg KH
In-Reply-To: <20091028013721.049c0146.sfr@canb.auug.org.au>
On Wed, 28 Oct 2009 01:37:21 +1100 Stephen Rothwell wrote:
> Just adding John and Greg to the cc list as this is caused by patches in
> the staging tree.
ping ping. still a problem.
> On Tue, 27 Oct 2009 15:05:03 +0530 Sachin Sant <sachinp@in.ibm.com> wrote:
> >
> > Fails to build on x86_32 with lots of errors related to znet
> >
> > drivers/net/znet.c:107:29: error: wireless/i82593.h: No such file or directory
> > drivers/net/znet.c:133: error: field 'i593_init' has incomplete type
> > drivers/net/znet.c: In function 'znet_set_multicast_list':
> > drivers/net/znet.c:235: error: invalid application of 'sizeof' to incomplete type 'struct i82593_conf_block'
> > drivers/net/znet.c:247: error: dereferencing pointer to incomplete type
> > .....
> >
> > Commit 879e9304... moved wireless/i82593.h to drivers/staging/wavelan/i82593.h
> >
> > Thanks
> > -Sachin
---
~Randy
^ permalink raw reply
* Re: RFC: net: allow to propagate errors through ->ndo_hard_start_xmit()
From: Jarek Poplawski @ 2009-11-10 17:08 UTC (permalink / raw)
To: Patrick McHardy
Cc: Linux Netdev List, Herbert Xu, David S. Miller, Stephen Hemminger
In-Reply-To: <4AF87070.6020007@trash.net>
On Mon, Nov 09, 2009 at 08:41:36PM +0100, Patrick McHardy wrote:
> I've updated my patch to propagate error values (errno and NET_XMIT
> codes) through ndo_hard_start_xmit() and incorporated the suggestions
> made last time, namely:
>
> - move slightly complicated return value check to inline function and
> add a few comments
>
> - fix error handling while in the middle of transmitting GSO skbs
>
> I've also audited the tree once again for invalid return values and
> found a single remaining instance in a Wimax driver, I'll take care
> of that later.
>
> Two questions remain:
>
> - I'm not sure the error handling in dev_hard_start_xmit() for GSO
> skbs is optimal. When the driver returns an error, it is assumed
> the current segment has been freed. The patch then frees the
> entire GSO skb, including all remaining segments. Alternatively
> it could try to transmit the remaining segments later.
Anyway, it seems this freeing should be described in the changelog,
if not moved to a separate patch, since it fixes another problem,
unless I forgot something.
>
> - Stephen recently introduced an enum for the netdev_tx codes, with
> this patch drivers are allowed to return different values.
> The mainly useful part about Stephen's patch IMO is the netdev_tx
> typedef to make it easier to locate ndo_start_xmit() functions
> which are defined in different files than the netdev_ops.
> So we could remove the enum again and simply typedef an int.
>
> Any opinions are welcome :)
> commit 08a98f11bc1c1452df74c171409218d2243f0818
> Author: Patrick McHardy <kaber@trash.net>
> Date: Mon Nov 9 20:33:14 2009 +0100
>
> net: allow to propagate errors through ->ndo_hard_start_xmit()
>
> Currently the ->ndo_hard_start_xmit() callbacks are only permitted to return
> one of the NETDEV_TX codes. This prevents any kind of error propagation for
> virtual devices, like queue congestion of the underlying device in case of
> layered devices, or unreachability in case of tunnels.
>
> This patches changes the NET_XMIT codes to avoid clashes with the NETDEV_TX
- This patches changes the NET_XMIT codes to avoid clashes with the NETDEV_TX
+ This patch changes the NET_XMIT codes to avoid clashes with the NETDEV_TX
> codes and changes the two callers of dev_hard_start_xmit() to expect either
> errno codes, NET_XMIT codes or NETDEV_TX codes as return value.
>
> In case of qdisc_restart(), all non NETDEV_TX codes are mapped to NETDEV_TX_OK
> since no error propagation is possible when using qdiscs. In case of
> dev_queue_xmit(), the error is propagated upwards.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 465add6..ab2812c 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -63,27 +63,48 @@ struct wireless_dev;
> #define HAVE_FREE_NETDEV /* free_netdev() */
> #define HAVE_NETDEV_PRIV /* netdev_priv() */
>
> -#define NET_XMIT_SUCCESS 0
> -#define NET_XMIT_DROP 1 /* skb dropped */
> -#define NET_XMIT_CN 2 /* congestion notification */
> -#define NET_XMIT_POLICED 3 /* skb is shot by police */
> -#define NET_XMIT_MASK 0xFFFF /* qdisc flags in net/sch_generic.h */
> +/*
> + * Transmit return codes: transmit return codes originate from three different
> + * namespaces:
> + *
> + * - qdisc return codes
> + * - driver transmit return codes
> + * - errno values
> + *
> + * Drivers are allowed to return any one of those in their hard_start_xmit()
> + * function. Real network devices commonly used with qdiscs should only return
> + * the driver transmit return codes though - when qdiscs are used, the actual
> + * transmission happens asynchronously, so the value is not propagated to
> + * higher layers. Virtual network devices transmit synchronously, in this case
> + * the driver transmit return codes are consumed by dev_queue_xmit(), all
> + * others are propagated to higher layers.
> + */
> +
> +/* qdisc ->enqueue() return codes. */
> +#define NET_XMIT_SUCCESS 0x00
> +#define NET_XMIT_DROP 0x10 /* skb dropped */
> +#define NET_XMIT_CN 0x20 /* congestion notification */
> +#define NET_XMIT_POLICED 0x30 /* skb is shot by police */
> +#define NET_XMIT_MASK 0xf0 /* qdisc flags in net/sch_generic.h */
>
> /* Backlog congestion levels */
> -#define NET_RX_SUCCESS 0 /* keep 'em coming, baby */
> -#define NET_RX_DROP 1 /* packet dropped */
> +#define NET_RX_SUCCESS 0 /* keep 'em coming, baby */
> +#define NET_RX_DROP 1 /* packet dropped */
Should these NET_RX codes be mixed with transmit codes?
>
> /* NET_XMIT_CN is special. It does not guarantee that this packet is lost. It
> * indicates that the device will soon be dropping packets, or already drops
> * some packets of the same priority; prompting us to send less aggressively. */
> -#define net_xmit_eval(e) ((e) == NET_XMIT_CN? 0 : (e))
> +#define net_xmit_eval(e) ((e) == NET_XMIT_CN ? 0 : (e))
> #define net_xmit_errno(e) ((e) != NET_XMIT_CN ? -ENOBUFS : 0)
>
> /* Driver transmit return codes */
> +#define NETDEV_TX_MASK 0xf
> +
> enum netdev_tx {
> - NETDEV_TX_OK = 0, /* driver took care of packet */
> - NETDEV_TX_BUSY, /* driver tx path was busy*/
> - NETDEV_TX_LOCKED = -1, /* driver tx lock was already taken */
> + __NETDEV_TX_MIN = INT_MIN, /* make sure enum is signed */
> + NETDEV_TX_OK = 0, /* driver took care of packet */
> + NETDEV_TX_BUSY = 1, /* driver tx path was busy*/
> + NETDEV_TX_LOCKED = 2, /* driver tx lock was already taken */
> };
> typedef enum netdev_tx netdev_tx_t;
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index bf629ac..1f5752d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1756,7 +1756,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> struct netdev_queue *txq)
> {
> const struct net_device_ops *ops = dev->netdev_ops;
> - int rc;
> + int rc = NETDEV_TX_OK;
Isn't it enough to add this in one place only: before this
"goto out_kfree_skb"?
>
> if (likely(!skb->next)) {
> if (!list_empty(&ptype_all))
> @@ -1804,6 +1804,8 @@ gso:
> nskb->next = NULL;
> rc = ops->ndo_start_xmit(nskb, dev);
> if (unlikely(rc != NETDEV_TX_OK)) {
> + if (rc & ~NETDEV_TX_MASK)
> + goto out_kfree_gso_skb;
If e.g. (rc == NETDEV_TX_OK | NET_XMIT_CN), why exactly is this freeing
necessary now?
Is e.g. (rc == NETDEV_TX_BUSY | NET_XMIT_CN) legal? If so, there would
be use after kfree, I guess. Otherwise, it should be documented above
(and maybe checked somewhere as well).
> nskb->next = skb->next;
> skb->next = nskb;
> return rc;
> @@ -1813,11 +1815,14 @@ gso:
> return NETDEV_TX_BUSY;
> } while (skb->next);
>
> - skb->destructor = DEV_GSO_CB(skb)->destructor;
> + rc = NETDEV_TX_OK;
When is (rc != NETDEV_TX_OK) possible in this place?
>
> +out_kfree_gso_skb:
> + if (likely(skb->next == NULL))
> + skb->destructor = DEV_GSO_CB(skb)->destructor;
> out_kfree_skb:
> kfree_skb(skb);
> - return NETDEV_TX_OK;
> + return rc;
> }
>
> static u32 skb_tx_hashrnd;
> @@ -1905,6 +1910,23 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
> return rc;
> }
>
> +static inline bool dev_xmit_complete(int rc)
> +{
> + /* successful transmission */
> + if (rc == NETDEV_TX_OK)
> + return true;
> +
> + /* error while transmitting, driver consumed skb */
> + if (rc < 0)
> + return true;
> +
> + /* error while queueing to a different device, driver consumed skb */
> + if (rc & NET_XMIT_MASK)
> + return true;
> +
> + return false;
> +}
> +
> /**
> * dev_queue_xmit - transmit a buffer
> * @skb: buffer to transmit
> @@ -2002,8 +2024,8 @@ gso:
> HARD_TX_LOCK(dev, txq, cpu);
>
> if (!netif_tx_queue_stopped(txq)) {
> - rc = NET_XMIT_SUCCESS;
> - if (!dev_hard_start_xmit(skb, dev, txq)) {
> + rc = dev_hard_start_xmit(skb, dev, txq);
> + if (dev_xmit_complete(rc)) {
> HARD_TX_UNLOCK(dev, txq);
> goto out;
> }
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 4ae6aa5..b13821a 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -120,8 +120,15 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>
> HARD_TX_LOCK(dev, txq, smp_processor_id());
> if (!netif_tx_queue_stopped(txq) &&
> - !netif_tx_queue_frozen(txq))
> + !netif_tx_queue_frozen(txq)) {
> ret = dev_hard_start_xmit(skb, dev, txq);
> +
> + /* an error implies that the skb was consumed */
> + if (ret < 0)
> + ret = NETDEV_TX_OK;
+ else (?)
Jarek P.
> + /* all NET_XMIT codes map to NETDEV_TX_OK */
> + ret &= ~NET_XMIT_MASK;
> + }
> HARD_TX_UNLOCK(dev, txq);
>
> spin_lock(root_lock);
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Stephen Hemminger @ 2009-11-10 17:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Ben Greear, Eric Dumazet, NetDev
In-Reply-To: <4AE8A098.8040207@trash.net>
>
> How about this? It uses 1MB as receive buf limit by default (without
> increasing /proc/sys/net/core/rmem_max it will be limited by less
> however) and allows to specify the size manually using "-rcvbuf X"
> (-r is already used, so you need to specify at least -rc).
>
> Additionally rtnl_listen() continues on ENOBUFS after printing the
> error message.
Applied, seems like the best workaround
^ permalink raw reply
* Re: Bugfix for iproute2
From: Stephen Hemminger @ 2009-11-10 17:12 UTC (permalink / raw)
To: Sven Anders; +Cc: netdev
In-Reply-To: <4AEF6F14.7080802@anduras.de>
On Tue, 03 Nov 2009 00:45:24 +0100
Sven Anders <anders@anduras.de> wrote:
> Hello!
>
> I already send this mail to Stephen Hemminger, but I'm not sure if it reached him.
> Moreover I tried to join the LARTC mailing list (http://mailman.ds9a.nl/mailman/listinfo/lartc)
> but it did not work. Is this mailing list dead?
> Therefore I sent this mail again over this mailing list...
>
>
> I experienced an error, if I try to perform a
>
> ip route flush proto 4
>
> with many routes in a complex environment, it
> gave me the following error:
>
> Failed to send flush request: Success
> Flush terminated
>
> I'm using version 2.6.29.
> I check GIT, but there was only the "MSG_PEEK" fix.
>
> I tracked it down to the rtnl_send_check() function
> in lib/libnetlink.c.
>
> In this function there is the following for loop:
>
> for (h = (struct nlmsghdr *)resp; NLMSG_OK(h, status);
> h = NLMSG_NEXT(h, status)) {
> if (h->nlmsg_type == NLMSG_ERROR) {
> struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
> if (h->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr)))
> fprintf(stderr, "ERROR truncated\n");
> else
> errno = -err->error;
> }
> return -1;
> }
>
> I think the "return -1;" should be inside the if statement.
>
> I attached a patch for this. The first part of the patch is taken from
> one of the late git commits.
>
> Please note me, if the fix is wrong...
>
> Regards
> Sven Anders
>
Applied thanks.
--
^ permalink raw reply
* Re: vlan/macvlan 02/02: propagate transmission state to upper layers
From: Patrick McHardy @ 2009-11-10 16:56 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <20091110084814.02593603@nehalam>
Stephen Hemminger wrote:
> On Tue, 10 Nov 2009 17:14:24 +0100
> Patrick McHardy <kaber@trash.net> wrote:
>
>> vlan/macvlan: propagate transmission state to upper layers
>>
>> Both vlan and macvlan devices usually don't use a qdisc and immediately
>> queue packets to the underlying device. Propagate transmission state of
>> the underlying device to the upper layers so they can react on congestion
>> and/or inform the sending process.
>>
>> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
>
> Bridging and bonding have same issue, but the solution is more difficult.
Yes, in both cases the packet might be sent out on multiple
interfaces, so its not really clear which state we should
propagate. I guess it would make sense to indicate an error
if transmission on *all* interfaces fail, but I'm not sure
about the other cases.
^ 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