Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] netdevice: provide common routine for macvlan and vlan operstate management
From: Patrick McHardy @ 2009-11-12 18:21 UTC (permalink / raw)
  To: Patrick Mullaney; +Cc: linux-kernel, arnd, netdev, bridge
In-Reply-To: <20091112180137.10186.9386.stgit@mimic.site>

Patrick Mullaney wrote:
> This patch is intended to address your comment on moving the operstate
> transition function. I decided to move it to netdevice.h, perhaps that
> is a bad idea? It didn't seem to logically fall into dev.c or link_watch.c.
> I am not against moving them to either one though. Your other comments
> are addressed and I will send out a second series once this gets
> reviewed and agreed on.

Thanks. I don't mind much where exactly it is located, but I'd prefer
to not have it inlined. It doesn't seem terribly wrong to move it to
dev.c, there are even some helpers for stacked devices already, like
address list synchronization.

Besides that the patch looks fine to me.

^ permalink raw reply

* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Ed Cashin @ 2009-11-12 18:07 UTC (permalink / raw)
  To: shemminger, ecashin, karaluh, roel.kluin, harvey.harrison,
	bzolnier, netdev
In-Reply-To: <20091112091048.00d00d1b@nehalam>

Thanks for the responses.

On Thu Nov 12 12:11:12 EST 2009, shemminger@vyatta.com wrote:
...
> Since it is emulating a block device, why not propgate error back
> up the stack like a disk that's offline.

The lack of local interfaces to use for AoE might be temporary.  For
example, the admin might be loading a new network driver, or a new
link might go online through which the AoE target can be reached.

If AoE command packets are not sent or resent but instead are
effectively dropped while there are no local interfaces through which
to send, then the AoE commands will timeout through the normal
mechanisms, at which time the error will be signaled to the block
layer.  It will be like normal unreliable ethernet transport.

In the common case, I think that's going to be the expected behavior,
but a printk would probably still be helpful in case the admin doesn't
realize why the AoE device is timing out.

-- 
  Ed Cashin <ecashin@coraid.com>
  http://www.coraid.com/
  http://noserose.net/e/

^ permalink raw reply

* [PATCH] netdevice: provide common routine for macvlan and vlan operstate management
From: Patrick Mullaney @ 2009-11-12 18:05 UTC (permalink / raw)
  To: kaber; +Cc: linux-kernel, arnd, netdev, bridge
In-Reply-To: <4AFAD86A.9070506@trash.net>

Patrick,

This patch is intended to address your comment on moving the operstate
transition function. I decided to move it to netdevice.h, perhaps that
is a bad idea? It didn't seem to logically fall into dev.c or link_watch.c.
I am not against moving them to either one though. Your other comments
are addressed and I will send out a second series once this gets
reviewed and agreed on.

Thanks for your review/comments.
----------

Provide common routine for the transition of operational state for a leaf
device during a root device transition.

Signed-off-by: Patrick Mullaney <pmullaney@novell.com>
---

 include/linux/netdevice.h |   19 +++++++++++++++++++
 net/8021q/vlan.c          |   29 ++++-------------------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d4a4d98..a15920a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1999,6 +1999,25 @@ static inline u32 dev_ethtool_get_flags(struct net_device *dev)
 		return 0;
 	return dev->ethtool_ops->get_flags(dev);
 }
+
+static inline
+void netif_stacked_transfer_operstate(const struct net_device *rootdev,
+					struct net_device *dev)
+{
+        if (rootdev->operstate == IF_OPER_DORMANT)
+                netif_dormant_on(dev);
+        else
+                netif_dormant_off(dev);
+
+        if (netif_carrier_ok(rootdev)) {
+                if (!netif_carrier_ok(dev))
+                        netif_carrier_on(dev);
+        } else {
+                if (netif_carrier_ok(dev))
+                        netif_carrier_off(dev);
+        }
+}
+
 #endif /* __KERNEL__ */
 
 #endif	/* _LINUX_NETDEVICE_H */
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index fe64908..5d11c12 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -183,27 +183,6 @@ void unregister_vlan_dev(struct net_device *dev)
 	dev_put(real_dev);
 }
 
-static void vlan_transfer_operstate(const struct net_device *dev,
-				    struct net_device *vlandev)
-{
-	/* Have to respect userspace enforced dormant state
-	 * of real device, also must allow supplicant running
-	 * on VLAN device
-	 */
-	if (dev->operstate == IF_OPER_DORMANT)
-		netif_dormant_on(vlandev);
-	else
-		netif_dormant_off(vlandev);
-
-	if (netif_carrier_ok(dev)) {
-		if (!netif_carrier_ok(vlandev))
-			netif_carrier_on(vlandev);
-	} else {
-		if (netif_carrier_ok(vlandev))
-			netif_carrier_off(vlandev);
-	}
-}
-
 int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
 {
 	const char *name = real_dev->name;
@@ -267,7 +246,7 @@ int register_vlan_dev(struct net_device *dev)
 	/* Account for reference in struct vlan_dev_info */
 	dev_hold(real_dev);
 
-	vlan_transfer_operstate(real_dev, dev);
+	netif_stacked_transfer_operstate(real_dev, dev);
 	linkwatch_fire_event(dev); /* _MUST_ call rfc2863_policy() */
 
 	/* So, got the sucker initialized, now lets place
@@ -449,7 +428,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 			if (!vlandev)
 				continue;
 
-			vlan_transfer_operstate(dev, vlandev);
+			netif_stacked_transfer_operstate(dev, vlandev);
 		}
 		break;
 
@@ -492,7 +471,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 				continue;
 
 			dev_change_flags(vlandev, flgs & ~IFF_UP);
-			vlan_transfer_operstate(dev, vlandev);
+			netif_stacked_transfer_operstate(dev, vlandev);
 		}
 		break;
 
@@ -508,7 +487,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 				continue;
 
 			dev_change_flags(vlandev, flgs | IFF_UP);
-			vlan_transfer_operstate(dev, vlandev);
+			netif_stacked_transfer_operstate(dev, vlandev);
 		}
 		break;
 

^ permalink raw reply related

* Re: sysfs question
From: Oliver Hartkopp @ 2009-11-12 17:44 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Kurt Van Dijck, Wolfgang Grandegger, netdev
In-Reply-To: <20091112090041.1536ccc8@nehalam>

Stephen Hemminger wrote:
> On Thu, 12 Nov 2009 11:39:41 +0100
> Kurt Van Dijck <kurt.van.dijck@eia.be> wrote:
> 
>> Hi,
>>
>> Within the socketcan project, we came into a situation that
>> might benefit with input from the netdev mailing list.
>>
>> The main issue is the policy to add sysfs properties in
>> /sys/class/net/canX.
>>
>> The reason for this is that cards (devices) with multiple network
>> interfaces may require properties per network.
>>
>> An obvious property would be the 'channel number of the card'. Other
>> properties could be things like type of output circuitry, ..., rather
>> hardware specific.
>>
>> I see currently 3 options:
>> 1) such properties in /sys/class/net/canX would be allowed.
>> 2) such properties would belong in /sys/class/net/canX/<subdirectory tbd>/
>> 3) such properties would go somewhere else.
>>
>> Some input with regard to sysfs policies would be helpfull.
>>
>> Kurt
> 
> It sounds like the property you are proposing is a property of the
> upper network layer not the hardware. Putting more properties in sysfs
> is good if is hardware related, but awkward if it is really a protocol
> attribute.

Then it would be good here :-)

The problem that can occur here is:

You plug-in a PCMCIA CAN card that has two CAN controllers on-board.

That means, you're getting two CAN independed network interfaces can0 and can1
pointing to ONE pcmcia device. Both of the CAN netdevs may have different
bitrates or other CAN controller specific settings (like the type of output
circuitry Kurt mentioned).

Finally the PCMCIA card itself may contain a serial number and/or a license
information that needs to be made available.

It's definitely no upper network layer stuff.

Regards,
Oliver

^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()
From: Eric Dumazet @ 2009-11-12 17:44 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <20091112091221.3d523253@nehalam>

Stephen Hemminger a écrit :
> On Thu, 12 Nov 2009 15:11:36 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>> When handling large number of netdevices, inet_dump_ifaddr()
>> is very slow because it has O(N^2) complexity.
>>
>> Instead of scanning one single list, we can use the NETDEV_HASHENTRIES
>> sub lists of the dev_index hash table, and RCU lookups.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> You might be able to make RCU critical section smaller by moving
> it into loop.
> 

Indeed. But we dump at most one skb (<= 8192 bytes ?), so rcu_read_lock
holding time is small, unless we meet many netdevices without
addresses. I wonder if its really common...

Thanks

[PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()

When handling large number of netdevices, inet_dump_ifaddr()
is very slow because it has O(N2) complexity.

Instead of scanning one single list, we can use the NETDEV_HASHENTRIES
sub lists of the dev_index hash table, and RCU lookups.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
 net/ipv4/devinet.c |   61 ++++++++++++++++++++++++++-----------------
 1 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c2045f9..7620382 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1174,39 +1174,54 @@ nla_put_failure:
 static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net *net = sock_net(skb->sk);
-	int idx, ip_idx;
+	int h, s_h;
+	int idx, s_idx;
+	int ip_idx, s_ip_idx;
 	struct net_device *dev;
 	struct in_device *in_dev;
 	struct in_ifaddr *ifa;
-	int s_ip_idx, s_idx = cb->args[0];
+	struct hlist_head *head;
+	struct hlist_node *node;
 
-	s_ip_idx = ip_idx = cb->args[1];
-	idx = 0;
-	for_each_netdev(net, dev) {
-		if (idx < s_idx)
-			goto cont;
-		if (idx > s_idx)
-			s_ip_idx = 0;
-		in_dev = __in_dev_get_rtnl(dev);
-		if (!in_dev)
-			goto cont;
+	s_h = cb->args[0];
+	s_idx = idx = cb->args[1];
+	s_ip_idx = ip_idx = cb->args[2];
 
-		for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
-		     ifa = ifa->ifa_next, ip_idx++) {
-			if (ip_idx < s_ip_idx)
-				continue;
-			if (inet_fill_ifaddr(skb, ifa, NETLINK_CB(cb->skb).pid,
+	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+		idx = 0;
+		head = &net->dev_index_head[h];
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(dev, node, head, index_hlist) {
+			if (idx < s_idx)
+				goto cont;
+			if (idx > s_idx)
+				s_ip_idx = 0;
+			in_dev = __in_dev_get_rcu(dev);
+			if (!in_dev)
+				goto cont;
+
+			for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
+			     ifa = ifa->ifa_next, ip_idx++) {
+				if (ip_idx < s_ip_idx)
+					continue;
+				if (inet_fill_ifaddr(skb, ifa,
+					     NETLINK_CB(cb->skb).pid,
 					     cb->nlh->nlmsg_seq,
-					     RTM_NEWADDR, NLM_F_MULTI) <= 0)
-				goto done;
-		}
+					     RTM_NEWADDR, NLM_F_MULTI) <= 0) {
+					rcu_read_unlock();
+					goto done;
+				}
+			}
 cont:
-		idx++;
+			idx++;
+		}
+		rcu_read_unlock();
 	}
 
 done:
-	cb->args[0] = idx;
-	cb->args[1] = ip_idx;
+	cb->args[0] = h;
+	cb->args[1] = idx;
+	cb->args[2] = ip_idx;
 
 	return skb->len;
 }

^ permalink raw reply related

* Re: [PATCH] can: Add missing debug flag for making USB CAN devices
From: Wolfgang Grandegger @ 2009-11-12 17:42 UTC (permalink / raw)
  To: Linux Netdev List; +Cc: SocketCAN Core Mailing List
In-Reply-To: <4AFC2B6B.5090000-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Wolfgang Grandegger wrote:
> Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> ---
>  drivers/net/can/usb/Makefile |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: net-next-2.6/drivers/net/can/usb/Makefile
> ===================================================================
> --- net-next-2.6.orig/drivers/net/can/usb/Makefile
> +++ net-next-2.6/drivers/net/can/usb/Makefile
> @@ -3,3 +3,5 @@
>  #
> 
>  obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> +
> +ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG

Please drop this patch. I just realized that Oliver has already fixed
this issue in his "[PATCH net-2.6] can: Fix driver Kconfig structure"
here (see http://marc.info/?l=linux-netdev&m=125802571320568&w=2).

Sorry for the noise.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()
From: Stephen Hemminger @ 2009-11-12 17:12 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AFC1798.1060501@gmail.com>

On Thu, 12 Nov 2009 15:11:36 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> When handling large number of netdevices, inet_dump_ifaddr()
> is very slow because it has O(N^2) complexity.
> 
> Instead of scanning one single list, we can use the NETDEV_HASHENTRIES
> sub lists of the dev_index hash table, and RCU lookups.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

You might be able to make RCU critical section smaller by moving
it into loop.

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

^ permalink raw reply

* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-12 17:10 UTC (permalink / raw)
  To: Ed Cashin; +Cc: karaluh, ecashin, roel.kluin, harvey.harrison, bzolnier, netdev
In-Reply-To: <e160a17e366ae2f676df822db1e62bc2@coraid.com>

On Thu, 12 Nov 2009 09:33:16 -0500
Ed Cashin <ecashin@coraid.com> wrote:

> Thanks again for providing this patch to help get things started.
> It's very helpful.  I appreciate the way it reflects and fits into the
> rest of the driver, too.
> 
> In the patch, there's a loop around getif, ejectif inside the new
> aoecmd_flushnet function:
> 
>   void aoecmd_flushnet(struct aoedev *d, struct net_device *nd)
>   {
>   	struct aoetgt **tt, **te;
>   	tt = d->targets;
>   	te = tt + NTARGETS;
>   	for (; tt < te && *tt; tt++) {
>   		struct aoetgt *t = *tt;
>   		struct aoeif *ifp;
>   
>   		while ( (ifp = getif(t, nd)) )
>   			ejectif(t, ifp);
>   	}
>   }
> 
> ... but an "if" seems appropriate, since duplicates are avoided when
> network devices are added in aoecmd_cfg_rsp:
> 
>   	ifp = getif(t, skb->dev);
>   	if (!ifp) {
>   		ifp = addif(t, skb->dev);

Yeah if works, wasn't sure if you could have multiples.

> 
> If there's some other reason for it to be "while" in aoecmd_flushnet
> that I'm missing, please let me know.
> 
> Regarding the new aoe_device_event function,
> 
>   /* Callback on change of state of network device. */
>   static int aoe_device_event(struct notifier_block *unused,
>   			    unsigned long event, void *ptr)
>   {
>   	struct net_device *nd = ptr;
>   
>   	if (is_aoe_netif(nd) && event == NETDEV_UNREGISTER)
>   		aoedev_ejectnet(nd);
>   
>   	return NOTIFY_DONE;
>   }
> 
> ...  the is_aoe_netif function really answers the question, "Are we
> allowed by the user to do AoE on this local network interface?" The
> user can modify the list of allowable interfaces at runtime via sysfs,
> as it's a module parameter.  So I don't think we can use is_aoe_netif
> in the new aoe_device_event function.  We should eject a net_device in
> this handler even if the user has decided not to use it for AoE.
> Please let me know if I'm missing the point.

Ok, you know the code better, I just wanted to avoid doing unnecessary work.

> You said that,
> 
>   > It needs to:
>   > 
>   > 1. Get a device ref count when it remembers a device: (ie addif)
>   > 2. Install a notifier that looks for device removal events
>   > 3. In notifier, remove interface, including flushing all pending
>   >    skb's for that device.
> 
> For 3, does the starter patch flush all pending skbs?  Perhaps you
> could elaborate on what you had in mind?

I wasn't sure if you ended up queuing skb's, but it looks like the
code queues requests not skb's.  But you may need to disable preempt
over code the work queue code that processes requests, to make sure
that device doesn't disappear while doing stuff.

> 
> I am trying to find the best way for the aoe driver to handle the
> situation where ther are no usable local network interfaces.  It does
> seem to me that the user would benefit from a notice letting them know
> that they're trying to do AoE without any usable ethernet.  I'm
> thinking that doing something like a printk_once would be appropriate.

Since it is emulating a block device, why not propgate error back
up the stack like a disk that's offline.



-- 

^ permalink raw reply

* Re: [PATCH v2] Documentation: rw_lock lessons learned
From: Stephen Hemminger @ 2009-11-12 17:04 UTC (permalink / raw)
  To: William Allen Simpson
  Cc: Paul E. McKenney, Linus Torvalds, Linux Kernel Developers,
	Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <4AFBEC44.9030409@gmail.com>

On Thu, 12 Nov 2009 06:06:44 -0500
William Allen Simpson <william.allen.simpson@gmail.com> wrote:

> +   NOTE! We are working hard to remove reader-writer spinlocks from the
> +   network stack, so please don't add a new one.  (Instead, see
> +   Documentation/RCU/rcu.txt for complete information.)

It is not just networking, so don't single that out.

Also, should mention -RT kernel and locking here (Appendix?)


-- 

^ permalink raw reply

* Re: sysfs question
From: Stephen Hemminger @ 2009-11-12 17:00 UTC (permalink / raw)
  To: Kurt Van Dijck; +Cc: Wolfgang Grandegger, netdev
In-Reply-To: <20091112103940.GC322@e-circ.dyndns.org>

On Thu, 12 Nov 2009 11:39:41 +0100
Kurt Van Dijck <kurt.van.dijck@eia.be> wrote:

> Hi,
> 
> Within the socketcan project, we came into a situation that
> might benefit with input from the netdev mailing list.
> 
> The main issue is the policy to add sysfs properties in
> /sys/class/net/canX.
> 
> The reason for this is that cards (devices) with multiple network
> interfaces may require properties per network.
> 
> An obvious property would be the 'channel number of the card'. Other
> properties could be things like type of output circuitry, ..., rather
> hardware specific.
> 
> I see currently 3 options:
> 1) such properties in /sys/class/net/canX would be allowed.
> 2) such properties would belong in /sys/class/net/canX/<subdirectory tbd>/
> 3) such properties would go somewhere else.
> 
> Some input with regard to sysfs policies would be helpfull.
> 
> Kurt

It sounds like the property you are proposing is a property of the
upper network layer not the hardware. Putting more properties in sysfs
is good if is hardware related, but awkward if it is really a protocol
attribute.

^ permalink raw reply

* Re: [RACE] net: in process_backlog
From: Stephen Hemminger @ 2009-11-12 16:57 UTC (permalink / raw)
  To: Changli Gao; +Cc: David S. Miller, Patrick McHardy, netdev
In-Reply-To: <412e6f7f0911120050w740377c7j2cdf24ef9fd2ca59@mail.gmail.com>

On Thu, 12 Nov 2009 16:50:53 +0800
Changli Gao <xiaosuo@gmail.com> wrote:

> Dear Stephen:
> 
> I don't think this change
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=6e583ce5242f32e925dcb198f7123256d0798370
> is correct.
> 
>                         local_irq_enable();
>                         break;
>                 }
> -
>                 local_irq_enable();
> 
> -               dev = skb->dev;
> -
> on MP system, flush_backlog() will be called here, and after that
> skb->dev will be invalid, if we access it, sth. unexpected may
> happens.
> 
>                 netif_receive_skb(skb);
> -
> -               dev_put(dev);
>         } while (++work < quota && jiffies == start_time);
> 
>         return work;

There is are a couple of issues here, but it is not what you thought
you saw.

The receive process is always done in soft IRQ context. The backlog queue's
are per-cpu. When a device is deleted an IPI is sent to all cpu's to
scan there backlog queue.  What should protect the skb is the fact that
the network device destruction process waits for an RCU grace period.
So skb->dev points to valid data. 

BUT the flush_backlog is run too late in the device destruction process.
It should be moved out of netdev_run_todo, to right after dev_shutdown().
Also adding a check for skb->dev->reg_state in netif_receive_skb would
be wise to drop packets.


-- 

^ permalink raw reply

* Re: [PATCH 2/2] au1000-eth: convert to platform_driver model
From: Ralf Baechle @ 2009-11-12 16:42 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: linux-mips, netdev, David Miller
In-Reply-To: <200911100113.38685.florian@openwrt.org>

On Tue, Nov 10, 2009 at 01:13:38AM +0100, Florian Fainelli wrote:

> 
> This patch converts the au1000-eth driver to become a full
> platform-driver as it ought to be. We now pass PHY-speficic
> configurations through platform_data but for compatibility
> the driver still assumes the default settings (search for PHY1 on
> MAC0) when no platform_data is passed. Tested on my MTX-1 board.
> 
> Acked-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>

Queued for 2.6.33.  Thanks everybody!

  Ralf

^ permalink raw reply

* Re: [PATCH 1/2] alchemy: add au1000-eth platform device
From: Ralf Baechle @ 2009-11-12 16:41 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, David Miller, linux-mips
In-Reply-To: <200911100113.31471.florian@openwrt.org>

On Tue, Nov 10, 2009 at 01:13:30AM +0100, Florian Fainelli wrote:

> (resending per Ralf's request as the patch had some checkpatch errors)

You missed the trailing whitespace ...  Anyway, fixed that up and queued
it for 2.6.33.

  Ralf

^ permalink raw reply

* Re: [PATCH]NET:PCNET32: poll method return 0 when done
From: Don Fry @ 2009-11-12 16:13 UTC (permalink / raw)
  To: Figo.zhang; +Cc: David S. Miller, netdev
In-Reply-To: <1257952296.1920.3.camel@myhost>

I do not understand why you are suggesting this change.  With NAPI
the amount of work done is returned on exit, not zero.  All other
drivers I have looked at do not force a zero on exit.

NAK

Don

-----Original Message-----
From: Figo.zhang <figo1802@gmail.com>
To: David S. Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>
Subject: [PATCH]NET:PCNET32: poll method return 0 when done
Date: Wed, 11 Nov 2009 23:11:36 +0800

Poll method return 0 when it is done.

Signed-off-by: Figo.zhang <figo1802@gmail.com>
--- 
drivers/net/pcnet32.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index c1b3f09..58894a3 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1396,6 +1396,7 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
 	spin_unlock_irqrestore(&lp->lock, flags);
 
 	if (work_done < budget) {
+		work_done = 0;
 		spin_lock_irqsave(&lp->lock, flags);
 
 		__napi_complete(napi);


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


^ permalink raw reply related

* re
From: David Rosenblatt @ 2009-11-12 16:08 UTC (permalink / raw)


Are you in need of a loan? We offer loan as low as 3%, If interested contact us via E-mail.

^ permalink raw reply

* Re: [PATCH 0/4] vbus: venet macvlan support
From: Patrick McHardy @ 2009-11-12 15:59 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
	evb, netdev
In-Reply-To: <4AFC2FCC.5090103@gmail.com>

Gregory Haskins wrote:
> Patrick McHardy wrote:
>> Gregory Haskins wrote:
>>> Patrick Mullaney wrote:
>>>> (Applies to alacrityvm.git/master:34534534)
>>>>
>>>> This patchset implements a vbus venet device with a
>>>> macvlan backend.
>>> Thanks Pat, applied.
>> As I mentioned in my response to these patches, the macvlan part
>> need more work.
> 
> Yeah, I talked to Pat offline.  He is going to patch it incrementally as
> its not a trivial problem to cleanup.  For the time being, they are
> sitting in the alacrityvm tree, but they are not going upstream until
> the macvlan stuff is resolved.

I see, thanks.

^ permalink raw reply

* Re: [PATCH 0/4] vbus: venet macvlan support
From: Gregory Haskins @ 2009-11-12 15:54 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
	evb, netdev
In-Reply-To: <4AFC2F71.1060408@trash.net>

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

Patrick McHardy wrote:
> Gregory Haskins wrote:
>> Patrick Mullaney wrote:
>>> (Applies to alacrityvm.git/master:34534534)
>>>
>>> This patchset implements a vbus venet device with a
>>> macvlan backend.
>> Thanks Pat, applied.
> 
> As I mentioned in my response to these patches, the macvlan part
> need more work.

Yeah, I talked to Pat offline.  He is going to patch it incrementally as
its not a trivial problem to cleanup.  For the time being, they are
sitting in the alacrityvm tree, but they are not going upstream until
the macvlan stuff is resolved.  All but the last patch are not really my
business to push up, anyway.

In the meantime, Pat or others can enhance the work that he's done to
date, so I will carry them out-of-tree.

Kind Regards,
-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]

^ permalink raw reply

* Re: [PATCH 0/4] vbus: venet macvlan support
From: Patrick McHardy @ 2009-11-12 15:53 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
	evb, netdev
In-Reply-To: <4AFC2D65.9040903@gmail.com>

Gregory Haskins wrote:
> Patrick Mullaney wrote:
>> (Applies to alacrityvm.git/master:34534534)
>>
>> This patchset implements a vbus venet device with a
>> macvlan backend.
> 
> Thanks Pat, applied.

As I mentioned in my response to these patches, the macvlan part
need more work.

^ permalink raw reply

* Re: [PATCH 0/4] vbus: venet macvlan support
From: Gregory Haskins @ 2009-11-12 15:44 UTC (permalink / raw)
  To: Patrick Mullaney
  Cc: alacrityvm-devel, linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>

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

Patrick Mullaney wrote:
> (Applies to alacrityvm.git/master:34534534)
> 
> This patchset implements a vbus venet device with a
> macvlan backend.

Thanks Pat, applied.

If possible, please submit a patch for the userspace side "-net
venet-macvlan[,macaddr][,lower-devname]" feature ASAP and I will merge
that as well.

-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]

^ permalink raw reply

* Re: [PATCH v2] Documentation: rw_lock lessons learned
From: Linus Torvalds @ 2009-11-12 15:40 UTC (permalink / raw)
  To: William Allen Simpson
  Cc: Stephen Hemminger, Paul E. McKenney, Linux Kernel Developers,
	Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <4AFBEC44.9030409@gmail.com>



On Thu, 12 Nov 2009, William Allen Simpson wrote:
>
> In recent weeks, two different network projects erroneously
> strayed down the rw_lock path.  Update the Documentation
> based upon comments by Eric Dumazet and Paul E. McKenney in
> those threads.

This still retains some pretty old and bogus language. For example, that 
file still talks about spinlocks being "faster than a global interrupt 
lock", which is kind of amusing - because we've not done that global 
interrupt lock thing for the last ten years or so. 

So I certainly agree with discouraging rwlocks - I don't think we've ever 
really found a situation where they are useful except for some really 
special cases - but I suspect the thing needs a bigger overhaul.

I also suspect somebody should actually take a look at our _users_ of 
rwlocks. We have a few fairly central ones like 'tasklist_lock', and it 
may be an example of a _good_ case of rwlocks, but for a very non-obvious 
reason.

In the case of 'tasklist_lock', the magic subtle reason that makes it a 
good idea is that that lock is commonly taken for reading in _interrupts_. 
And the way rwlocks are defined, that means that you can take it for 
reading without doing the *_irq() or *_irqsave() versions, because rwlocks 
are not fair, so an active reader will never block a new reader even if 
there is a blocked writer pending.

So for tasklist_lock, raw rwlocks are still slower than raw spinlocks, but 
because of the rwlock semantics the common case doesn't need to disable 
and enable interrupts, so for the common case the comparison is not "raw 
rwlock vs raw spinlock", but "raw rwlock vs interrupt-disabling spinlock", 
and then it turns out rwlocks tend to win again.

(Of course, lock_write() needs to disable interrupts for tasklist_lock, 
but that tends to be the uncommon case).

Ho humm..

		Linus

^ permalink raw reply

* 3% Offer
From: David Rosenblatt @ 2009-11-12 15:30 UTC (permalink / raw)


Are you in need of a loan? We offer loan as low as 3%, If interested contact us via E-mail.

^ permalink raw reply

* [PATCH] can: Add missing debug flag for making USB CAN devices
From: Wolfgang Grandegger @ 2009-11-12 15:36 UTC (permalink / raw)
  To: Linux Netdev List; +Cc: SocketCAN Core Mailing List

Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
 drivers/net/can/usb/Makefile |    2 ++
 1 file changed, 2 insertions(+)

Index: net-next-2.6/drivers/net/can/usb/Makefile
===================================================================
--- net-next-2.6.orig/drivers/net/can/usb/Makefile
+++ net-next-2.6/drivers/net/can/usb/Makefile
@@ -3,3 +3,5 @@
 #

 obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
+
+ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG

^ permalink raw reply

* [PATCH] can: add the missing netlink get_xstats_size callback
From: Wolfgang Grandegger @ 2009-11-12 15:34 UTC (permalink / raw)
  To: Linux Netdev List; +Cc: SocketCAN Core Mailing List

This patch adds the missing "get_xstats_size" callback for the
netlink interface, which is required if "fill_xstats" is used,
as pointed out by Patrick McHardy.

Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
 drivers/net/can/dev.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: net-next-2.6/drivers/net/can/dev.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/dev.c
+++ net-next-2.6/drivers/net/can/dev.c
@@ -677,6 +677,11 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
+static size_t can_get_xstats_size(const struct net_device *dev)
+{
+	return sizeof(struct can_device_stats);
+}
+
 static int can_fill_xstats(struct sk_buff *skb, const struct net_device *dev)
 {
 	struct can_priv *priv = netdev_priv(dev);
@@ -705,6 +710,7 @@ static struct rtnl_link_ops can_link_ops
 	.changelink	= can_changelink,
 	.get_size	= can_get_size,
 	.fill_info	= can_fill_info,
+	.get_xstats_size = can_get_xstats_size,
 	.fill_xstats	= can_fill_xstats,
 };

^ permalink raw reply

* Re: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Mike Christie @ 2009-11-12 15:10 UTC (permalink / raw)
  To: roel kluin
  Cc: Randy Dunlap, Stephen M. Cameron, David Airlie, Jens Axboe,
	Evgeniy Polyakov, iss_storagedev, Eric Dumazet, Joel Becker,
	Jeff Liu, Andy Whitcroft, Dave Airlie, Ingo Molnar, dri-devel,
	Alexey Dobriyan, Mike Miller, Mark Fasheh, Karsten Keil, rostedt,
	Karen Xie, James Bottomley, James E.J. Bottomley, Hannes Reinecke,
	Andreas Eversberg, Hannes Eder, Keith
In-Reply-To: <25e057c00911120131q577f18c5v479b9d6f5f8616a6@mail.gmail.com>

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

roel kluin wrote:
>> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>>> Andy, can we have a checkpatch rule please?
>> Note, that will upset creative uses of error codes i guess, such as
>> fs/xfs/.
>>
>> But yeah, +1 from me too.
>>
>> Ob'post'mortem - looked for similar patterns in the kernel and there's
>> quite a few bugs there:
>>
>>  include/net/inet_hashtables.h:                  return ENOMEM;         # bug
>>  drivers/scsi/aic7xxx/aic7xxx_osm.c:             return ENOMEM;         # works but weird
>>  drivers/scsi/cxgb3i/cxgb3i_offload.c:           return ENOMEM;         # works but weird

I think cxgb3i is actually in the buggy category. cxgb3i_c3cn_send_pdus 
can propagate the positive EXYZ error value to other functions which 
assume that errors are negative.

Karen, I made the attached patch over James's scsi-rc-fixes tree while 
reviewing the code. Could you test, finish up and send upstream?

[-- Attachment #2: cxgb3i-use-negative-errno.patch --]
[-- Type: text/x-patch, Size: 2523 bytes --]

diff --git a/drivers/scsi/cxgb3i/cxgb3i_offload.c b/drivers/scsi/cxgb3i/cxgb3i_offload.c
index c1d5be4..f6a1fb9 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_offload.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_offload.c
@@ -291,7 +291,7 @@ static void act_open_req_arp_failure(struct t3cdev *dev, struct sk_buff *skb)
 	c3cn_hold(c3cn);
 	spin_lock_bh(&c3cn->lock);
 	if (c3cn->state == C3CN_STATE_CONNECTING)
-		fail_act_open(c3cn, EHOSTUNREACH);
+		fail_act_open(c3cn, -EHOSTUNREACH);
 	spin_unlock_bh(&c3cn->lock);
 	c3cn_put(c3cn);
 	__kfree_skb(skb);
@@ -792,18 +792,18 @@ static int act_open_rpl_status_to_errno(int status)
 {
 	switch (status) {
 	case CPL_ERR_CONN_RESET:
-		return ECONNREFUSED;
+		return -ECONNREFUSED;
 	case CPL_ERR_ARP_MISS:
-		return EHOSTUNREACH;
+		return -EHOSTUNREACH;
 	case CPL_ERR_CONN_TIMEDOUT:
-		return ETIMEDOUT;
+		return -ETIMEDOUT;
 	case CPL_ERR_TCAM_FULL:
-		return ENOMEM;
+		return -ENOMEM;
 	case CPL_ERR_CONN_EXIST:
 		cxgb3i_log_error("ACTIVE_OPEN_RPL: 4-tuple in use\n");
-		return EADDRINUSE;
+		return -EADDRINUSE;
 	default:
-		return EIO;
+		return -EIO;
 	}
 }
 
@@ -817,7 +817,7 @@ static void act_open_retry_timer(unsigned long data)
 	spin_lock_bh(&c3cn->lock);
 	skb = alloc_skb(sizeof(struct cpl_act_open_req), GFP_ATOMIC);
 	if (!skb)
-		fail_act_open(c3cn, ENOMEM);
+		fail_act_open(c3cn, -ENOMEM);
 	else {
 		skb->sk = (struct sock *)c3cn;
 		set_arp_failure_handler(skb, act_open_req_arp_failure);
@@ -966,14 +966,14 @@ static int abort_status_to_errno(struct s3_conn *c3cn, int abort_reason,
 	case CPL_ERR_BAD_SYN: /* fall through */
 	case CPL_ERR_CONN_RESET:
 		return c3cn->state > C3CN_STATE_ESTABLISHED ?
-			EPIPE : ECONNRESET;
+			-EPIPE : -ECONNRESET;
 	case CPL_ERR_XMIT_TIMEDOUT:
 	case CPL_ERR_PERSIST_TIMEDOUT:
 	case CPL_ERR_FINWAIT2_TIMEDOUT:
 	case CPL_ERR_KEEPALIVE_TIMEDOUT:
-		return ETIMEDOUT;
+		return -ETIMEDOUT;
 	default:
-		return EIO;
+		return -EIO;
 	}
 }
 
diff --git a/drivers/scsi/cxgb3i/cxgb3i_pdu.c b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
index 7091050..1fe3b0f 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_pdu.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
@@ -388,8 +388,8 @@ int cxgb3i_conn_xmit_pdu(struct iscsi_task *task)
 	if (err > 0) {
 		int pdulen = err;
 
-	cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
-			task, skb, skb->len, skb->data_len, err);
+		cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
+				task, skb, skb->len, skb->data_len, err);
 
 		if (task->conn->hdrdgst_en)
 			pdulen += ISCSI_DIGEST_SIZE;

[-- Attachment #3: Type: text/plain, Size: 354 bytes --]

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

^ permalink raw reply related

* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-12 15:11 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
	netdev
In-Reply-To: <412e6f7f0911120148r48fa553bt610e013d96522a3d@mail.gmail.com>

Changli Gao wrote:
> The corresponding iprout2 patch is attached.

Printing the value during dumps is missing from this patch.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox