* 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: [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: [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 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] 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: 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: 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
* [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: [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
* 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: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-12 19:04 UTC (permalink / raw)
To: Caleb Cushing; +Cc: Frans Pop, Andi Kleen, linux-kernel, netdev
In-Reply-To: <81bfc67a0911120546g26627ac5q5860d85f446b29bb@mail.gmail.com>
Caleb Cushing wrote, On 11/12/2009 02:46 PM:
>> On 11-11-2009 22:47, Andi Kleen wrote:
>> ...
>>> It might be also useful if you could describe what kind
>>> of network devices you use and how you determine
>>> the packet loss.
>> Btw, you didn't send the stats you compared, and your wireshark dump
>> doesn't show anything wrong either.
>>
>> Jarek P.
>>
>
> I didn't see that sorry. I wasn't sure if the dump would or not (I'm
> not a networking expert, just know more than the average joe).
>
> from dmesg (networking device)
>
> e1000e: Intel(R) PRO/1000 Network Driver - 1.0.2-k2
>
> from lspci
>
> 00:19.0 Ethernet controller: Intel Corporation 82562V-2 10/100 Network
> Connection (rev 02)
So I assume it's your only network device on this box and according
to these reports it's 192.168.1.3 with 192.168.1.1 as the gateway,
and your only change is kernel on this 192.168.1.3 box, right?
> the attached png's show mtr with bad being when I have the problem.
> for those not familiar mtr sends an icmp packet to each hop in 1
> second then loops. when I'm having this kind of packet loss (and
> sometimes it's higher) all services including dhcp, dns, and http (web
> browsing) get flaky, or don't work at all (I really can't browse the
> web).
Since the loss is seen on the first hop already, it seems it should be
enough to query 192.168.1.1 only - did you try this? If so, does this
happen from the beginning of the test or after many loops? Could you
try to repeat this wireshark dump with more data than before (but just
to be sure there are a few unanswered pings). If possible it would be
nice to have wireshark or tcpdump data from 192.168.1.1 too, while
pinged from 192.168.1.3. Please, send it gzipped to bugzilla only plus
ifconfig eth0 before and after the test (and let us know here).
Btw, mtr has text reporting too (--report). Larger things send to
bugzilla only.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Stephen Hemminger @ 2009-11-12 19:09 UTC (permalink / raw)
To: Ed Cashin; +Cc: ecashin, karaluh, roel.kluin, harvey.harrison, bzolnier, netdev
In-Reply-To: <054b5fbe4b446c6342704cd1bd5265db@coraid.com>
On Thu, 12 Nov 2009 13:07:35 -0500
Ed Cashin <ecashin@coraid.com> wrote:
> 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.
>
Okay, you might want to ratelimit the printk
--
^ permalink raw reply
* RE: [net-next-2.6 PATCH 2/3] ixgbe: Set MSI-X vectors to NOBALANCING and set affinity
From: Waskiewicz Jr, Peter P @ 2009-11-12 19:12 UTC (permalink / raw)
To: David Miller, Kirsher, Jeffrey T; +Cc: gospo@redhat.com, netdev@vger.kernel.org
In-Reply-To: <20091021.215031.57955781.davem@davemloft.net>
>From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>Date: Tue, 20 Oct 2009 19:27:14 -0700
>
>> From: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>>
>> This patch will set each MSI-X vector to IRQF_NOBALANCING to
>> prevent autobalance of the interrupts, then applies a CPU
>> affinity. This will only be done when Flow Director is enabled,
>> which needs interrupts to be processed on the same CPUs where the
>> applications are running.
>>
>> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
>Just explain to me why irqbalanced in userspace cannot take care
>of this issue.
>
>Second, even if we cannot use irqbalanced for some reason, the last
>thing I want to see is drivers directly fiddling with interrupt
>states and attributes. Every driver is going to do it every so
>slightly differently, and often will get it wrong.
Jesse Brandeburg and I talked with Arjan yesterday regarding these patches. We all agreed the drivers need some mechanism to help irqbalance make smarter decisions when enforcing its balancing policies. Arjan did convince me though that enforcing the interrupt's policy from the driver isn't the right thing to do (which echoes your stance too :-) ).
The current proposal is to add a new interface to the interrupt management that drivers can specify an allowable CPU mask. This way a driver can be specific with where interrupt vectors can be balanced, e.g. specify a CPU mask within a NUMA node that a vector can be balanced. Then irqbalance can make the right decisions based on cache locality of CPU cores; as long as we land on the correct NUMA node with the interrupt, we should be fine. This new interface can also be used in the PCI space to assist setting the correct NUMA mask for a driver on load, further streamlining the NUMA affinity of a device.
This change will also require an update to irqbalance to comprehend the new mask. This should be fairly trivial.
We also discussed the need for irqbalance to distinguish between Rx and Tx queue vectors. Right now, irqbalance can identify an interrupt belong to an Ethernet device, but it stops there. It needs to also distinguish the directional vectors, and make sure to balance the right queue vector with its paired queue (i.e. make sure Tx queue 0's vector lines up with Rx queue 0's vector).
I'll be getting patches together for this interface, and will push them separately. In the meantime, I'll repush the one patch from this patch series to pair the Rx and Tx queues onto a shared vector. That is an independent patch from this whole series, so it should be fine to apply regardless of the interrupt affinity changes.
Cheers,
-PJ Waskiewicz
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()
From: Stephen Hemminger @ 2009-11-12 19:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AFC4979.7030007@gmail.com>
On Thu, 12 Nov 2009 18:44:25 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 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
One case where that might happen is:
modprobe dummy numdummies=10000
But dummy device should really be added with netlink, not at boot time.
--
^ permalink raw reply
* Re: [PATCH v2] Documentation: rw_lock lessons learned
From: Stephen Clark @ 2009-11-12 19:13 UTC (permalink / raw)
To: William Allen Simpson
Cc: Stephen Hemminger, Paul E. McKenney, Linus Torvalds,
Linux Kernel Developers, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <4AFBEC44.9030409@gmail.com>
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.
>
> Merged with editorial changes by Stephen Hemminger.
>
> Signed-off-by: William.Allen.Simpson@gmail.com
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> Documentation/spinlocks.txt | 186
> ++++++++++++++++++++-----------------------
> 1 files changed, 86 insertions(+), 100 deletions(-)
>
How up to date is this doc?
http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/index.html
Should it be in the Documentation directory?
--
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety." (Ben Franklin)
"The course of history shows that as a government grows, liberty
decreases." (Thomas Jefferson)
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()
From: Eric Dumazet @ 2009-11-12 19:14 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <20091112111218.0a561166@nehalam>
Stephen Hemminger a écrit :
>
> One case where that might happen is:
> modprobe dummy numdummies=10000
>
> But dummy device should really be added with netlink, not at boot time.
>
At least we cannot exceed 32768 dummies :)
^ permalink raw reply
* RE: [PATCH] s2io: fixing a ethtool test that is broken
From: Sivakumar Subramani @ 2009-11-12 18:59 UTC (permalink / raw)
To: leitao, netdev; +Cc: Sreenivasa Honnur, Ramkrishna Vepa
In-Reply-To: <1257882263-7507-1-git-send-email-leitao@linux.vnet.ibm.com>
[Siva] Reviewed the patch. Please accept it.
Acked-by: Sivakumar Subramani <sivakumar.subramani@neterion.com>
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of leitao@linux.vnet.ibm.com
Sent: Tuesday, November 10, 2009 11:44 AM
To: netdev@vger.kernel.org
Cc: Sreenivasa Honnur; Breno Leitao
Subject: [PATCH] s2io: fixing a ethtool test that is broken
Due commit 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff, it is not more
possible to pci_restore_state() more than once without calling
pci_save_state() in the middle.
Actually running a ethtool test on s2io makes the card inactive,
and it needs to unload/reload the module to fix.
This patch just save the state just after it restore in order to
keep the old behaviour
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
---
drivers/net/s2io.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index ddccf5f..0dd7839 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -3494,6 +3494,7 @@ static void s2io_reset(struct s2io_nic *sp)
/* Restore the PCI state saved during initialization. */
pci_restore_state(sp->pdev);
+ pci_save_state(sp->pdev);
pci_read_config_word(sp->pdev, 0x2, &val16);
if (check_pci_device_id(val16) != (u16)PCI_ANY_ID)
break;
--
1.6.0.2
--
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: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Joel Becker @ 2009-11-12 19:17 UTC (permalink / raw)
To: Ingo Molnar
Cc: Randy Dunlap, Stephen M. Cameron, Mike Christie, David Airlie,
James Bottomley, Jens Axboe, Evgeniy Polyakov, iss_storagedev,
Eric Dumazet, Andy Whitcroft, Dave Airlie, Hannes Eder, dri-devel,
Alexey Dobriyan, Mike Miller, Mark Fasheh, Karsten Keil, rostedt,
Karen Xie, James E.J. Bottomley, Hannes Reinecke, Andreas
In-Reply-To: <20091112081043.GA25345@elte.hu>
On Thu, Nov 12, 2009 at 09:10:43AM +0100, Ingo Molnar wrote:
> 5 out of 8 places look buggy - i.e. more than 60% - a checkpatch warning
> would avoid real bugs here. (even ignoring the cleanliness effects of
> using proper error propagation)
>
> Cc:-ed affected maintainers. The rightmost column are my observations.
> Below is the patch fixing these.
Acked-by: Joel Becker <joel.becker@oracle.com>
--
"Can any of you seriously say the Bill of Rights could get through
Congress today? It wouldn't even get out of committee."
- F. Lee Bailey
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127
^ permalink raw reply
* [PATCH net-next-2.6] inetpeer: Optimize inet_getid()
From: Eric Dumazet @ 2009-11-12 19:33 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
While investigating for network latencies, I found inet_getid() was a
contention point for some workloads, as inet_peer_idlock is shared
by all inet_getid() users regardless of peers.
One way to fix this is to make ip_id_count an atomic_t instead
of __u16, and use atomic_add_return().
In order to keep sizeof(struct inet_peer) = 64 on 64bit arches
tcp_ts_stamp is also converted to __u32 instead of "unsigned long".
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/inetpeer.h | 16 +++++-----------
net/ipv4/inetpeer.c | 5 +----
net/ipv4/route.c | 2 +-
net/ipv4/tcp_ipv4.c | 16 ++++++++--------
4 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 35ad7b9..87b1df0 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -17,15 +17,15 @@ struct inet_peer {
/* group together avl_left,avl_right,v4daddr to speedup lookups */
struct inet_peer *avl_left, *avl_right;
__be32 v4daddr; /* peer's address */
- __u16 avl_height;
- __u16 ip_id_count; /* IP ID for the next packet */
+ __u32 avl_height;
struct list_head unused;
__u32 dtime; /* the time of last use of not
* referenced entries */
atomic_t refcnt;
atomic_t rid; /* Frag reception counter */
+ atomic_t ip_id_count; /* IP ID for the next packet */
__u32 tcp_ts;
- unsigned long tcp_ts_stamp;
+ __u32 tcp_ts_stamp;
};
void inet_initpeers(void) __init;
@@ -36,17 +36,11 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create);
/* can be called from BH context or outside */
extern void inet_putpeer(struct inet_peer *p);
-extern spinlock_t inet_peer_idlock;
/* can be called with or without local BH being disabled */
static inline __u16 inet_getid(struct inet_peer *p, int more)
{
- __u16 id;
-
- spin_lock_bh(&inet_peer_idlock);
- id = p->ip_id_count;
- p->ip_id_count += 1 + more;
- spin_unlock_bh(&inet_peer_idlock);
- return id;
+ more++;
+ return atomic_add_return(more, &p->ip_id_count) - more;
}
#endif /* _NET_INETPEER_H */
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index b1fbe18..6bcfe52 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -67,9 +67,6 @@
* ip_id_count: idlock
*/
-/* Exported for inet_getid inline function. */
-DEFINE_SPINLOCK(inet_peer_idlock);
-
static struct kmem_cache *peer_cachep __read_mostly;
#define node_height(x) x->avl_height
@@ -390,7 +387,7 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create)
n->v4daddr = daddr;
atomic_set(&n->refcnt, 1);
atomic_set(&n->rid, 0);
- n->ip_id_count = secure_ip_id(daddr);
+ atomic_set(&n->ip_id_count, secure_ip_id(daddr));
n->tcp_ts_stamp = 0;
write_lock_bh(&peer_pool_lock);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ff258b5..4284cee 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2852,7 +2852,7 @@ static int rt_fill_info(struct net *net,
error = rt->u.dst.error;
expires = rt->u.dst.expires ? rt->u.dst.expires - jiffies : 0;
if (rt->peer) {
- id = rt->peer->ip_id_count;
+ id = atomic_read(&rt->peer->ip_id_count) & 0xffff;
if (rt->peer->tcp_ts_stamp) {
ts = rt->peer->tcp_ts;
tsage = get_seconds() - rt->peer->tcp_ts_stamp;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 657ae33..b7b8c74 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -204,7 +204,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
* when trying new connection.
*/
if (peer != NULL &&
- peer->tcp_ts_stamp + TCP_PAWS_MSL >= get_seconds()) {
+ (u32)get_seconds() - peer->tcp_ts_stamp <= TCP_PAWS_MSL) {
tp->rx_opt.ts_recent_stamp = peer->tcp_ts_stamp;
tp->rx_opt.ts_recent = peer->tcp_ts;
}
@@ -1308,7 +1308,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
tcp_death_row.sysctl_tw_recycle &&
(peer = rt_get_peer((struct rtable *)dst)) != NULL &&
peer->v4daddr == saddr) {
- if (get_seconds() < peer->tcp_ts_stamp + TCP_PAWS_MSL &&
+ if ((u32)get_seconds() - peer->tcp_ts_stamp < TCP_PAWS_MSL &&
(s32)(peer->tcp_ts - req->ts_recent) >
TCP_PAWS_WINDOW) {
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_PAWSPASSIVEREJECTED);
@@ -1727,9 +1727,9 @@ int tcp_v4_remember_stamp(struct sock *sk)
if (peer) {
if ((s32)(peer->tcp_ts - tp->rx_opt.ts_recent) <= 0 ||
- (peer->tcp_ts_stamp + TCP_PAWS_MSL < get_seconds() &&
- peer->tcp_ts_stamp <= tp->rx_opt.ts_recent_stamp)) {
- peer->tcp_ts_stamp = tp->rx_opt.ts_recent_stamp;
+ ((u32)get_seconds() - peer->tcp_ts_stamp > TCP_PAWS_MSL &&
+ peer->tcp_ts_stamp <= (u32)tp->rx_opt.ts_recent_stamp)) {
+ peer->tcp_ts_stamp = (u32)tp->rx_opt.ts_recent_stamp;
peer->tcp_ts = tp->rx_opt.ts_recent;
}
if (release_it)
@@ -1748,9 +1748,9 @@ int tcp_v4_tw_remember_stamp(struct inet_timewait_sock *tw)
const struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw);
if ((s32)(peer->tcp_ts - tcptw->tw_ts_recent) <= 0 ||
- (peer->tcp_ts_stamp + TCP_PAWS_MSL < get_seconds() &&
- peer->tcp_ts_stamp <= tcptw->tw_ts_recent_stamp)) {
- peer->tcp_ts_stamp = tcptw->tw_ts_recent_stamp;
+ ((u32)get_seconds() - peer->tcp_ts_stamp > TCP_PAWS_MSL &&
+ peer->tcp_ts_stamp <= (u32)tcptw->tw_ts_recent_stamp)) {
+ peer->tcp_ts_stamp = (u32)tcptw->tw_ts_recent_stamp;
peer->tcp_ts = tcptw->tw_ts_recent;
}
inet_putpeer(peer);
^ permalink raw reply related
* Re: sysfs question
From: Kurt Van Dijck @ 2009-11-12 20:01 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Stephen Hemminger, Wolfgang Grandegger, netdev
In-Reply-To: <4AFC4984.8000803@hartkopp.net>
On Thu, Nov 12, 2009 at 06:44:36PM +0100, Oliver Hartkopp wrote:
> Stephen Hemminger wrote:
> > On Thu, 12 Nov 2009 11:39:41 +0100
> > Kurt Van Dijck <kurt.van.dijck@eia.be> wrote:
> >
[...]
> >
> > 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 :-)
>
[...]
>
> It's definitely no upper network layer stuff.
>
Ack. It's definitely datalink or even physical layer we're talking about.
> Regards,
> Oliver
I also encountered an issue with regard to the uevent that is generated
when a netdevice is registered.
If I add sysfs files before register_netdevice, the system complains on
creating the sysfs files.
If I add those after register_netdevice, the uevent is _seems_ to have
triggered userspace already, where the udev (in fact, I use another
home-brew one for boot speed) does not find the extra sysfs file.
Is there a mechanism to hold temporarily the uevent until after the
driver has registered some extra sysfs files?
Kurt
^ permalink raw reply
* SFP support for BCM54XX PHYs
From: Marcus D. Leech @ 2009-11-12 20:05 UTC (permalink / raw)
To: netdev
Has anyone gotten SFP support working for the BCM54XX series multi-mode
PHYs using
the ..drivers/net/phy/broadcom.c
I have a board that uses the BCM5464, and I need SFP to work. Anyone
done this?
Cheers
Marcus
^ permalink raw reply
* Re: [PATCH 1/2] rps: core implementation
From: Eric Dumazet @ 2009-11-12 20:23 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, netdev
In-Reply-To: <65634d660911102253o2b4f7a19kfed5849e5c88bfe1@mail.gmail.com>
Tom Herbert a écrit :
> Third version of RPS.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
> include/linux/interrupt.h | 1 +
> include/linux/netdevice.h | 18 ++++
> include/linux/skbuff.h | 2 +
> net/core/dev.c | 227 ++++++++++++++++++++++++++++++++++++++-------
> net/core/net-sysfs.c | 135 +++++++++++++++++++++++++++
> 5 files changed, 348 insertions(+), 35 deletions(-)
>
@@ -2696,21 +2842,24 @@ static int process_backlog(struct napi_struct
*napi, int quota)
int work = 0;
struct softnet_data *queue = &__get_cpu_var(softnet_data);
unsigned long start_time = jiffies;
+ unsigned long flags;
napi->weight = weight_p;
do {
struct sk_buff *skb;
<<HERE>> local_irq_disable();
+ spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
skb = __skb_dequeue(&queue->input_pkt_queue);
if (!skb) {
__napi_complete(napi);
- local_irq_enable();
+ spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
+ flags);
break;
}
- local_irq_enable();
+ spin_unlock_irqrestore(&queue->input_pkt_queue.lock, flags);
- netif_receive_skb(skb);
+ __netif_receive_skb(skb);
} while (++work < quota && jiffies == start_time);
return work;
Not sure you still want the local_irq_disable() before
spin_lock_irqsave(&queue->input_pkt_queue.lock, flags); ?
^ permalink raw reply
* Re: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Joel Becker @ 2009-11-12 20:27 UTC (permalink / raw)
To: Ingo Molnar
Cc: Randy Dunlap, Stephen M. Cameron, Mike Christie, David Airlie,
James Bottomley, Jens Axboe, Evgeniy Polyakov, iss_storagedev,
Eric Dumazet, Andy Whitcroft, Dave Airlie, Hannes Eder, dri-devel,
Alexey Dobriyan, Mike Miller, Mark Fasheh, Karsten Keil, rostedt,
Karen Xie, James E.J. Bottomley, Hannes Reinecke, Andreas
In-Reply-To: <20091112191758.GA22985@mail.oracle.com>
On Thu, Nov 12, 2009 at 11:17:58AM -0800, Joel Becker wrote:
> On Thu, Nov 12, 2009 at 09:10:43AM +0100, Ingo Molnar wrote:
> > 5 out of 8 places look buggy - i.e. more than 60% - a checkpatch warning
> > would avoid real bugs here. (even ignoring the cleanliness effects of
> > using proper error propagation)
> >
> > Cc:-ed affected maintainers. The rightmost column are my observations.
> > Below is the patch fixing these.
>
> Acked-by: Joel Becker <joel.becker@oracle.com>
I take that back. NAK.
Sorry, I read the code wrong. This function is just a handler.
The caller, dlm_send_begin_reco_message(), expects the positive EAGAIN
as a non-error case.
Joel
--
Life's Little Instruction Book #337
"Reread your favorite book."
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127
^ permalink raw reply
* [NEXT 1/1] Please pull small fix for ieee802.15.4
From: Dmitry Eremin-Solenikov @ 2009-11-12 21:17 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-zigbee-devel, netdev, Sergey Lapin
Hello,
The following changes since commit f6d773cd4f3c18c40ab25a5cb92453756237840e:
David S. Miller (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless-next-2.6
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lowpan/lowpan.git for-next
Dmitry Eremin-Solenikov (1):
ieee802154: make wpan-phy class registration to subsys_initcall
net/ieee802154/wpan-class.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ieee802154/wpan-class.c b/net/ieee802154/wpan-class.c
index 38bac70..2686912 100644
--- a/net/ieee802154/wpan-class.c
+++ b/net/ieee802154/wpan-class.c
@@ -205,7 +205,7 @@ err_nl:
err:
return rc;
}
-module_init(wpan_phy_class_init);
+subsys_initcall(wpan_phy_class_init);
static void __exit wpan_phy_class_exit(void)
{
--
With best wishes
Dmitry
^ permalink raw reply related
* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-12 21:47 UTC (permalink / raw)
To: Caleb Cushing; +Cc: Frans Pop, Andi Kleen, linux-kernel, netdev
In-Reply-To: <4AFC5C58.9030207@gmail.com>
Jarek Poplawski wrote, On 11/12/2009 08:04 PM:
> Caleb Cushing wrote, On 11/12/2009 02:46 PM:
...
>>> Btw, you didn't send the stats you compared, and your wireshark dump
>>> doesn't show anything wrong either.
...
>> I didn't see that sorry. I wasn't sure if the dump would or not (I'm
>> not a networking expert, just know more than the average joe).
Hmm... I didn't see that either, sorry! After re-checking I can see
unanswered requests in this dump. Anyway, the main thing to test now is
the first hop to 192.168.1.1 (some info about it?), as I wrote before.
Jarek P.
^ 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