* Re: [PATCH] net: cpuvert to new cpumask API
From: KOSAKI Motohiro @ 2011-05-18 0:14 UTC (permalink / raw)
To: davem; +Cc: linux-kernel, therbert, eric.dumazet, netdev
In-Reply-To: <20110516.115357.193701317.davem@davemloft.net>
(2011/05/17 3:53), David Miller wrote:
> From: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
> Date: Thu, 28 Apr 2011 23:56:35 +0900 (JST)
>
>> We plan to remove cpu_xx() old api later. Thus this patch
>> convert it.
>>
>> This patch has no functional change.
>>
>> Signed-off-by: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
>
> Applied, thanks.
Great! :)
^ permalink raw reply
* Re: [PATCH 06/18] virtio_ring: avail event index interface
From: Rusty Russell @ 2011-05-18 0:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <20110517060052.GB26989-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, 17 May 2011 09:00:52 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, May 16, 2011 at 03:53:19PM +0930, Rusty Russell wrote:
> > On Sun, 15 May 2011 15:47:27 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > > On Mon, May 09, 2011 at 01:43:15PM +0930, Rusty Russell wrote:
> > > > On Wed, 4 May 2011 23:51:19 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > > > > #define VIRTIO_RING_F_USED_EVENT_IDX 29
> > > > > +/* The Host publishes the avail index for which it expects a kick
> > > > > + * at the end of the used ring. Guest should ignore the used->flags field. */
> > > > > +#define VIRTIO_RING_F_AVAIL_EVENT_IDX 32
> > > >
> > > > Are you really sure we want to separate the two? Seems a little simpler
> > > > to have one bit to mean "we're publishing our threshold". For someone
> > > > implementing this from scratch, it's a little simpler.
> > > >
> > > > Or are there cases where the old style makes more sense?
> > > >
> > > > Thanks,
> > > > Rusty.
> > >
> > > Hmm, it makes debugging easier as each side can disable
> > > publishing separately - I used it all the time when I saw
> > > e.g. networking stuck to guess whether I need to investigate the
> > > interrupt or the exit handling.
> > >
> > > But I'm not hung up on this.
> > >
> > > Let me know pls.
> >
> > If we combine them into one, then these patches no longer depend on
> > the feature bit expansion, which is worthwhile (though I'll take both).
> >
> > Thanks,
> > Rusty.
>
> Yes, I know. But if we do expand feature bits anyway, for debugging
> and profiling if nothing else it's useful to have them separate ...
> If you take both why does the order matter?
Damage control. Then if something breaks, it doesn't break everything.
Cheers,
Rusty.
^ permalink raw reply
* Security Warning
From: Webmaster Administrator @ 2011-05-17 23:28 UTC (permalink / raw)
Your mailbox has exceeded the storage limit which is 20GB as set by your
administrator,you are currently running on 20.9GB,you may not be able to
send or receive new mail until you re-validate your mailbox. To
re-validate your mailbox, please click the link below:
http://doiop.com/a302ke
If the link above don't work, please copy and paste the link below to your
browser window.
http://doiop.com/a302ke
Thanks
System Administrator Team.
^ permalink raw reply
* [PATCH] IPv6 transmit hashing for bonding driver
From: John @ 2011-05-17 23:55 UTC (permalink / raw)
To: netdev
Currently the "bonding" driver does not support load balancing outgoing traffic in LACP mode for IPv6 traffic. IPv4 (and
TCP over IPv4) are currently supported; this patch adds transmit hashing for IPv6 (and TCP over IPv6), bringing IPv6 up
to par with IPv4 support in the bonding driver.
The algorithm chosen (xor'ing the bottom three quads and then xor'ing that down into the bottom byte) was chosen after
testing almost 400,000 unique IPv6 addresses harvested from server logs. This algorithm had the most even distribution
for both big- and little-endian architectures while still using few instructions.
This patch also adds missing configuration information the MODULE_PARM_DESC.
Patch has been tested on various machines and performs as expected. Thanks to Stephen Hemminger and Andy Gospodarek for
advice and guidance.
John
--- drivers/net/bonding/bond_main.c.orig 2011-04-18 17:23:09.202894000 -0700
+++ drivers/net/bonding/bond_main.c 2011-04-19 18:12:30.287929000 -0700
@@ -152,7 +152,7 @@
MODULE_PARM_DESC(ad_select, "803.ad aggregation selection logic: stable (0, default), bandwidth (1), count (2)");
module_param(xmit_hash_policy, charp, 0);
MODULE_PARM_DESC(xmit_hash_policy, "XOR hashing method: 0 for layer 2 (default)"
- ", 1 for layer 3+4");
+ ", 1 for layer 3+4, 2 for layer 2+3");
module_param(arp_interval, int, 0);
MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
module_param_array(arp_ip_target, charp, NULL, 0);
@@ -3720,11 +3720,20 @@
static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph = ip_hdr(skb);
return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
(data->h_dest[5] ^ data->h_source[5])) % count;
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+ u32 v6hash = (
+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3])
+ );
+ v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;
+ return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count;
}
return (data->h_dest[5] ^ data->h_source[5]) % count;
@@ -3738,11 +3747,11 @@
static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
{
struct ethhdr *data = (struct ethhdr *)skb->data;
- struct iphdr *iph = ip_hdr(skb);
- __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
- int layer4_xor = 0;
+ u32 layer4_xor = 0;
if (skb->protocol == htons(ETH_P_IP)) {
+ struct iphdr *iph = ip_hdr(skb);
+ __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
if (!(iph->frag_off & htons(IP_MF|IP_OFFSET)) &&
(iph->protocol == IPPROTO_TCP ||
iph->protocol == IPPROTO_UDP)) {
@@ -3750,7 +3759,18 @@
}
return (layer4_xor ^
((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
-
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+ __be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(*ipv6h));
+ if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
+ layer4_xor = (*layer4hdrv6 ^ *(layer4hdrv6 + 1));
+ }
+ layer4_xor ^= (
+ (ipv6h->saddr.s6_addr32[1] ^ ipv6h->daddr.s6_addr32[1]) ^
+ (ipv6h->saddr.s6_addr32[2] ^ ipv6h->daddr.s6_addr32[2]) ^
+ (ipv6h->saddr.s6_addr32[3] ^ ipv6h->daddr.s6_addr32[3])
+ );
+ return ((layer4_xor >> 16) ^ (layer4_xor >> 8) ^ layer4_xor) % count;
}
return (data->h_dest[5] ^ data->h_source[5]) % count;
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Changli Gao @ 2011-05-17 23:59 UTC (permalink / raw)
To: David Miller; +Cc: therbert, netdev
In-Reply-To: <20110517.164929.1737248436066795381.davem@davemloft.net>
On Wed, May 18, 2011 at 4:49 AM, David Miller <davem@davemloft.net> wrote:
>
> Actually, I think it won't work. Even Linux emits fragments last to
> first, so we won't see the UDP header until the last packet where it's
> no longer useful.
>
No. Linux emits fragments first to last now. You should check the
current code. :)
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Shirley Ma @ 2011-05-17 23:44 UTC (permalink / raw)
To: Michał Mirosław
Cc: Michael S. Tsirkin, Ben Hutchings, David Miller, Eric Dumazet,
Avi Kivity, Arnd Bergmann, netdev, kvm, linux-kernel
In-Reply-To: <BANLkTik6aqP5BVrQNFKrnU3v=KA9ip1RoA@mail.gmail.com>
On Wed, 2011-05-18 at 00:58 +0200, Michał Mirosław wrote:
> W dniu 18 maja 2011 00:28 użytkownik Shirley Ma <mashirle@us.ibm.com>
> napisał:
> > On Tue, 2011-05-17 at 23:48 +0200, Michał Mirosław wrote:
> >> 2011/5/17 Shirley Ma <mashirle@us.ibm.com>:
> >> > Looks like to use a new flag requires more time/work. I am
> thinking
> >> > whether we can just use HIGHDMA flag to enable zero-copy in
> macvtap
> >> to
> >> > avoid the new flag for now since mavctap uses real NICs as lower
> >> device?
> >>
> >> Is there any other restriction besides requiring driver to not
> recycle
> >> the skb? Are there any drivers that recycle TX skbs?
> > Not more other restrictions, skb clone is OK. pskb_expand_head()
> looks
> > OK to me from code review.
>
> > Currently there is no drivers recycle TX skbs.
>
> So why do you require the target device to have some flags at all?
We could use macvtap to check lower device HIGHDMA to enable zero-copy,
but I am not sure whether it is sufficient. If it's sufficient then we
don't need to use a new flag here. To be safe, it's better to use a new
flag to enable each device who can pass zero-copy test.
> Do I understand correctly, that this zero-copy feature is about
> packets received from VMs?
Yes, packets sent from VMs, and received in local host for TX zero-copy
here.
Thanks
Shirley
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: Michał Mirosław @ 2011-05-17 23:19 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Miller, bhutchings, netdev
In-Reply-To: <4DD2EFEC.9040504@pobox.com>
W dniu 18 maja 2011 00:00 użytkownik Jeff Garzik <jgarzik@pobox.com> napisał:
> On 05/17/2011 03:09 PM, Michał Mirosław wrote:
>> 2011/5/17 Jeff Garzik<jgarzik@pobox.com>:
>>> 2011/5/17 David Miller<davem@davemloft.net>:
>>>> An enumeration is not a bitmask, instead it means one out of the set
>>>> of enumerated values will be used.
>>> It's a decade-old kernel practice to use 'enum' to define typed
>>> constants, preferred over macros that convey no type information and
>>> disappear after cpp phase.
>>>
>>> So your assertion about enumerations is demonstrably not true, as it
>>> is often used in the kernel. Call it enum abuse if you want, but it
>>> is consistent with code all over the kernel.
>> Old age of the mistake doesn't make it correct.
> It is not a mistake, but a useful coding tool.
Being it a mistake does not contradict usefulness. ;-)
C has bitfields for the uses where masks and shifts are usually used.
Unfortunately, those usually require more typing and sometimes compile
to worse code.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH net-2.6] net: add skb_dst_force() in sock_queue_err_skb()
From: Eric Dumazet @ 2011-05-17 23:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Witold Baryluk, Stephen Hemminger
Commit 7fee226ad239 (add a noref bit on skb dst) forgot to use
skb_dst_force() on packets queued in sk_error_queue
This triggers following warning, for applications using IP_CMSG_PKTINFO
receiving one error status
------------[ cut here ]------------
WARNING: at include/linux/skbuff.h:457 ip_cmsg_recv_pktinfo+0xa6/0xb0()
Hardware name: 2669UYD
Modules linked in: isofs vboxnetadp vboxnetflt nfsd ebtable_nat ebtables
lib80211_crypt_ccmp uinput xcbc hdaps tp_smapi thinkpad_ec radeonfb fb_ddc
radeon ttm drm_kms_helper drm ipw2200 intel_agp intel_gtt libipw i2c_algo_bit
i2c_i801 agpgart rng_core cfbfillrect cfbcopyarea cfbimgblt video raid10 raid1
raid0 linear md_mod vboxdrv
Pid: 4697, comm: miredo Not tainted 2.6.39-rc6-00569-g5895198-dirty #22
Call Trace:
[<c17746b6>] ? printk+0x1d/0x1f
[<c1058302>] warn_slowpath_common+0x72/0xa0
[<c15bbca6>] ? ip_cmsg_recv_pktinfo+0xa6/0xb0
[<c15bbca6>] ? ip_cmsg_recv_pktinfo+0xa6/0xb0
[<c1058350>] warn_slowpath_null+0x20/0x30
[<c15bbca6>] ip_cmsg_recv_pktinfo+0xa6/0xb0
[<c15bbdd7>] ip_cmsg_recv+0x127/0x260
[<c154f82d>] ? skb_dequeue+0x4d/0x70
[<c1555523>] ? skb_copy_datagram_iovec+0x53/0x300
[<c178e834>] ? sub_preempt_count+0x24/0x50
[<c15bdd2d>] ip_recv_error+0x23d/0x270
[<c15de554>] udp_recvmsg+0x264/0x2b0
[<c15ea659>] inet_recvmsg+0xd9/0x130
[<c1547752>] sock_recvmsg+0xf2/0x120
[<c11179cb>] ? might_fault+0x4b/0xa0
[<c15546bc>] ? verify_iovec+0x4c/0xc0
[<c1547660>] ? sock_recvmsg_nosec+0x100/0x100
[<c1548294>] __sys_recvmsg+0x114/0x1e0
[<c1093895>] ? __lock_acquire+0x365/0x780
[<c1148b66>] ? fget_light+0xa6/0x3e0
[<c1148b7f>] ? fget_light+0xbf/0x3e0
[<c1148aee>] ? fget_light+0x2e/0x3e0
[<c1549f29>] sys_recvmsg+0x39/0x60
Close bug https://bugzilla.kernel.org/show_bug.cgi?id=34622
Reported-by: Witold Baryluk <baryluk@smp.if.uj.edu.pl>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Stephen Hemminger <shemminger@vyatta.com>
---
net/core/skbuff.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7ebeed0..3e934fe 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2993,6 +2993,9 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
skb->destructor = sock_rmem_free;
atomic_add(skb->truesize, &sk->sk_rmem_alloc);
+ /* before exiting rcu section, make sure dst is refcounted */
+ skb_dst_force(skb);
+
skb_queue_tail(&sk->sk_error_queue, skb);
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_data_ready(sk, skb->len);
^ permalink raw reply related
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Michał Mirosław @ 2011-05-17 22:58 UTC (permalink / raw)
To: Shirley Ma
Cc: Michael S. Tsirkin, Ben Hutchings, David Miller, Eric Dumazet,
Avi Kivity, Arnd Bergmann, netdev, kvm, linux-kernel
In-Reply-To: <1305671318.10756.49.camel@localhost.localdomain>
W dniu 18 maja 2011 00:28 użytkownik Shirley Ma <mashirle@us.ibm.com> napisał:
> On Tue, 2011-05-17 at 23:48 +0200, Michał Mirosław wrote:
>> 2011/5/17 Shirley Ma <mashirle@us.ibm.com>:
>> > Looks like to use a new flag requires more time/work. I am thinking
>> > whether we can just use HIGHDMA flag to enable zero-copy in macvtap
>> to
>> > avoid the new flag for now since mavctap uses real NICs as lower
>> device?
>>
>> Is there any other restriction besides requiring driver to not recycle
>> the skb? Are there any drivers that recycle TX skbs?
> Not more other restrictions, skb clone is OK. pskb_expand_head() looks
> OK to me from code review.
> Currently there is no drivers recycle TX skbs.
So why do you require the target device to have some flags at all?
Do I understand correctly, that this zero-copy feature is about
packets received from VMs?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Rick Jones @ 2011-05-17 22:42 UTC (permalink / raw)
To: David Miller; +Cc: andi, netdev
In-Reply-To: <20110517.175044.2057517197524794568.davem@davemloft.net>
On Tue, 2011-05-17 at 17:50 -0400, David Miller wrote:
> From: Andi Kleen <andi@firstfloor.org>
> Date: Tue, 17 May 2011 14:48:28 -0700
>
> > David Miller <davem@davemloft.net> writes:
> >
> >> Guys we can't time out fragments if we are not the final
> >> destination.
> >
> > If you're not the final destination you should never even
> > try to reassemble them?
> >
> > I'm probably missing something...
>
> We're discussing the idea to do the defragmentation first
> so we can choose the flow properly and steer the packet
> to the correct cpu.
>
> This also would allos each fragmented packet to traverse the
> stack only once (one route lookup etc.) instead of once per
> fragment.
>
> Please read the rest of this thread, we have discussed this
> and now I'm repeating information solely for your benefit.
Well, I should probably be beaten with that stick too because I wasn't
thinking about forwarding, only being the destination system when I
broached the suggestion of doing RFS after reassembly. I can see where
one *might* be able to do limited RPS when forwarding, but I didn't know
that RFS had been extended to forwarding.
Now though I see why you were rightfully concerned about timeouts -
given all the concerns about added latency from bufferbloat, I wouldn't
think that an additional 10 or perhaps even 1ms timeout on a reassembly
attempt to get the layer four header when forwarding would sit well with
folks - they will expect the fragments to flow through without
additional delay.
rick jones
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
From: Eric Dumazet @ 2011-05-17 22:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, kaber, netdev, remi.denis-courmont
In-Reply-To: <20110517152503.2f7244f3@nehalam>
Le mardi 17 mai 2011 à 15:25 -0700, Stephen Hemminger a écrit :
> I am beginning to think the idea of optimizing case of "ip link show"
> running in background with adding/removing network devices is silly.
> And switching to RCU leads to all sorts of corner cases where only
> partial data will be read. Is it really worth it?
>
Just try "modprobe dummy numdummies=1000", and watch load average
raising, because so many processes want to hold RTNL, and the
rtnl_trylock() crazy thing make them spin (restart syscall, and find
RTNL locked again... wooo...)
Some workloads hit RTNL quite hard, and monitoring tools were sometime
frozen for unexpected long times. (before the patch, when we used to
hold RTNL in dump())
I dont know, if it happens to be too hard, we'll just stick again rtnl
for "ip link show" ;)
^ permalink raw reply
* Re: [PATCH V5 2/6 net-next] netdevice.h: Add zero-copy flag in netdevice
From: Shirley Ma @ 2011-05-17 22:28 UTC (permalink / raw)
To: Michał Mirosław
Cc: Michael S. Tsirkin, Ben Hutchings, David Miller, Eric Dumazet,
Avi Kivity, Arnd Bergmann, netdev, kvm, linux-kernel
In-Reply-To: <BANLkTimvOK3tyo6+L5AD=so5DjiDPV-AAA@mail.gmail.com>
On Tue, 2011-05-17 at 23:48 +0200, Michał Mirosław wrote:
> 2011/5/17 Shirley Ma <mashirle@us.ibm.com>:
> > Hello Michael,
> >
> > Looks like to use a new flag requires more time/work. I am thinking
> > whether we can just use HIGHDMA flag to enable zero-copy in macvtap
> to
> > avoid the new flag for now since mavctap uses real NICs as lower
> device?
>
> Is there any other restriction besides requiring driver to not recycle
> the skb? Are there any drivers that recycle TX skbs?
Not more other restrictions, skb clone is OK. pskb_expand_head() looks
OK to me from code review.
Currently there is no drivers recycle TX skbs.
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
From: Stephen Hemminger @ 2011-05-17 22:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, kaber, netdev, remi.denis-courmont
In-Reply-To: <1305670692.6741.14.camel@edumazet-laptop>
On Wed, 18 May 2011 00:18:12 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 02 mai 2011 à 15:27 -0700, David Miller a écrit :
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Thu, 28 Apr 2011 08:43:37 -0700
> >
> > > On Thu, 28 Apr 2011 10:56:07 +0200
> > > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > >
> > >> Four years ago, Patrick made a change to hold rtnl mutex during netlink
> > >> dump callbacks.
> > >>
> > >> I believe it was a wrong move. This slows down concurrent dumps, making
> > >> good old /proc/net/ files faster than rtnetlink in some situations.
> > >>
> > >> This occurred to me because one "ip link show dev ..." was _very_ slow
> > >> on a workload adding/removing network devices in background.
> > >>
> > >> All dump callbacks are able to use RCU locking now, so this patch does
> > >> roughly a revert of commits :
> > >>
> > >> 1c2d670f366 : [RTNETLINK]: Hold rtnl_mutex during netlink dump callbacks
> > >> 6313c1e0992 : [RTNETLINK]: Remove unnecessary locking in dump callbacks
> > >>
> > >> This let writers fight for rtnl mutex and readers going full speed.
> > >>
> > >> It also takes care of phonet : phonet_route_get() is now called from rcu
> > >> read section. I renamed it to phonet_route_get_rcu()
> > >>
> > >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > >> Cc: Patrick McHardy <kaber@trash.net>
> > >> Cc: Remi Denis-Courmont <remi.denis-courmont@nokia.com>
> > >
> > > Acked-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > Applied, thanks everyone!
>
> I think we probably have to make some fixes, because rtnl_fill_ifinfo()
> can access fields that were previously protected with RTNL
>
> Let's see if it's doable, before considering re-adding rtnl_lock() in
> rtnl_dump_ifinfo() ?
>
> First step : dev->ifalias
>
> Its late here, I'll continue the work tomorrow.
>
> Thanks
>
> [PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
>
> Avoid holding RTNL in show_ifalias() and rtnl_fill_ifinfo() to
> manipulate netdev->ifalias
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
I am beginning to think the idea of optimizing case of "ip link show"
running in background with adding/removing network devices is silly.
And switching to RCU leads to all sorts of corner cases where only
partial data will be read. Is it really worth it?
^ permalink raw reply
* Re: [PATCH V5 4/6 net-next] vhost: vhost TX zero-copy support
From: Shirley Ma @ 2011-05-17 22:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
kvm, linux-kernel
In-Reply-To: <20110517212827.GC7589@redhat.com>
On Wed, 2011-05-18 at 00:28 +0300, Michael S. Tsirkin wrote:
> On Tue, May 17, 2011 at 01:50:19PM -0700, Shirley Ma wrote:
> > Resubmit the patch with most update. This patch passed some
> > live-migration test against RHEL6.2. I will run more stress test w/i
> > live migration.
> >
> > Signed-off-by: Shirley Ma <xma@us.ibm.com>
>
> Cool. cleanup path needs a fix - are you use you can
> not use kobj or some other existing refcounting?
>
> Is perf regressiion caused by tx ring overrun gone now?
>
> I added some comments about how we might be aqble
> to complete requests out of order but it's not a must.
Oops, I used the old version vhost patch instead of most update one.
I will integrate your comments below and submit V6 patch instead.
thanks
Shirley
^ permalink raw reply
* [PATCH] e100: Fix race condition in e100_down() while testing
From: prasanna.panchamukhi @ 2011-05-17 22:20 UTC (permalink / raw)
To: bruce.w.allan, jeffrey.t.kirsher, jesse.brandeburg
Cc: e1000-devel, netdev, prasanna.panchamukhi
There is a race condition between e100_down() and e100_diag_test().
During device testing e100_diag_test() ends up calling e100_up()/e100_down()
even while the driver is already in e100_down().
This patch fixes the above race condition by changing
e100_up()/e100_down() to dev_open() and dev_close().
Also fixes the race between e100_open() and e100_diag_test().
Signed-off-by: Prasanna S. Panchamukh <prasanna.panchamukhi@riverbed.com>
---
drivers/net/e100.c | 21 +++++++++++++++++----
1 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index b0aa9e6..abbf229 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -425,6 +425,10 @@ enum cb_command {
cb_el = 0x8000,
};
+enum e100_state_t {
+ __E100_TESTING
+};
+
struct rfd {
__le16 status;
__le16 command;
@@ -623,6 +627,7 @@ struct nic {
__le16 eeprom[256];
spinlock_t mdio_lock;
const struct firmware *fw;
+ unsigned long state;
};
static inline void e100_write_flush(struct nic *nic)
@@ -2570,8 +2575,10 @@ static void e100_diag_test(struct net_device *netdev,
{
struct ethtool_cmd cmd;
struct nic *nic = netdev_priv(netdev);
+ int if_running = netif_running(netdev);
int i, err;
+ set_bit(__E100_TESTING, &nic->state);
memset(data, 0, E100_TEST_LEN * sizeof(u64));
data[0] = !mii_link_ok(&nic->mii);
data[1] = e100_eeprom_load(nic);
@@ -2580,8 +2587,9 @@ static void e100_diag_test(struct net_device *netdev,
/* save speed, duplex & autoneg settings */
err = mii_ethtool_gset(&nic->mii, &cmd);
- if (netif_running(netdev))
- e100_down(nic);
+ if (if_running)
+ /* indicate we're in test mode */
+ dev_open(netdev);
data[2] = e100_self_test(nic);
data[3] = e100_loopback_test(nic, lb_mac);
data[4] = e100_loopback_test(nic, lb_phy);
@@ -2589,12 +2597,13 @@ static void e100_diag_test(struct net_device *netdev,
/* restore speed, duplex & autoneg settings */
err = mii_ethtool_sset(&nic->mii, &cmd);
- if (netif_running(netdev))
- e100_up(nic);
+ if (if_running)
+ dev_open(netdev);
}
for (i = 0; i < E100_TEST_LEN; i++)
test->flags |= data[i] ? ETH_TEST_FL_FAILED : 0;
+ clear_bit(__E100_TESTING, &nic->state);
msleep_interruptible(4 * 1000);
}
@@ -2724,6 +2733,10 @@ static int e100_open(struct net_device *netdev)
struct nic *nic = netdev_priv(netdev);
int err = 0;
+ /* disallow open during testing */
+ if (test_bit(__E100_TESTING, &nic->state))
+ return -EBUSY;
+
netif_carrier_off(netdev);
if ((err = e100_up(nic)))
netif_err(nic, ifup, nic->netdev, "Cannot open interface, aborting\n");
--
1.7.0.4
^ permalink raw reply related
* [PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
From: Eric Dumazet @ 2011-05-17 22:18 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, kaber, netdev, remi.denis-courmont
In-Reply-To: <20110502.152733.48516094.davem@davemloft.net>
Le lundi 02 mai 2011 à 15:27 -0700, David Miller a écrit :
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 28 Apr 2011 08:43:37 -0700
>
> > On Thu, 28 Apr 2011 10:56:07 +0200
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> Four years ago, Patrick made a change to hold rtnl mutex during netlink
> >> dump callbacks.
> >>
> >> I believe it was a wrong move. This slows down concurrent dumps, making
> >> good old /proc/net/ files faster than rtnetlink in some situations.
> >>
> >> This occurred to me because one "ip link show dev ..." was _very_ slow
> >> on a workload adding/removing network devices in background.
> >>
> >> All dump callbacks are able to use RCU locking now, so this patch does
> >> roughly a revert of commits :
> >>
> >> 1c2d670f366 : [RTNETLINK]: Hold rtnl_mutex during netlink dump callbacks
> >> 6313c1e0992 : [RTNETLINK]: Remove unnecessary locking in dump callbacks
> >>
> >> This let writers fight for rtnl mutex and readers going full speed.
> >>
> >> It also takes care of phonet : phonet_route_get() is now called from rcu
> >> read section. I renamed it to phonet_route_get_rcu()
> >>
> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >> Cc: Patrick McHardy <kaber@trash.net>
> >> Cc: Remi Denis-Courmont <remi.denis-courmont@nokia.com>
> >
> > Acked-by: Stephen Hemminger <shemminger@vyatta.com>
>
> Applied, thanks everyone!
I think we probably have to make some fixes, because rtnl_fill_ifinfo()
can access fields that were previously protected with RTNL
Let's see if it's doable, before considering re-adding rtnl_lock() in
rtnl_dump_ifinfo() ?
First step : dev->ifalias
Its late here, I'll continue the work tomorrow.
Thanks
[PATCH net-next-2.6] net: add rcu protection to netdev->ifalias
Avoid holding RTNL in show_ifalias() and rtnl_fill_ifinfo() to
manipulate netdev->ifalias
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 7 ++++++-
net/core/dev.c | 28 ++++++++++++++++------------
net/core/net-sysfs.c | 13 +++++++------
net/core/rtnetlink.c | 8 ++++++--
4 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a134d80..09b3df4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -980,6 +980,11 @@ struct net_device_ops {
u32 features);
};
+struct ifalias {
+ struct rcu_head rcu;
+ char name[0];
+};
+
/*
* The DEVICE structure.
* Actually, this whole structure is a big mistake. It mixes I/O
@@ -1004,7 +1009,7 @@ struct net_device {
/* device name hash chain */
struct hlist_node name_hlist;
/* snmp alias */
- char *ifalias;
+ struct ifalias __rcu *ifalias;
/*
* I/O specific fields
diff --git a/net/core/dev.c b/net/core/dev.c
index 155de20..01e3be0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1035,6 +1035,12 @@ rollback:
return err;
}
+
+static void ifalias_kfree_rcu(struct rcu_head *head)
+{
+ kfree(container_of(head, struct ifalias, rcu));
+}
+
/**
* dev_set_alias - change ifalias of a device
* @dev: device
@@ -1045,24 +1051,22 @@ rollback:
*/
int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
{
+ struct ifalias *ifalias, *new = NULL;
ASSERT_RTNL();
if (len >= IFALIASZ)
return -EINVAL;
- if (!len) {
- if (dev->ifalias) {
- kfree(dev->ifalias);
- dev->ifalias = NULL;
- }
- return 0;
+ ifalias = rtnl_dereference(dev->ifalias);
+ if (len) {
+ new = kmalloc(sizeof(*new) + len + 1, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+ strlcpy(new->name, alias, len + 1);
}
-
- dev->ifalias = krealloc(dev->ifalias, len + 1, GFP_KERNEL);
- if (!dev->ifalias)
- return -ENOMEM;
-
- strlcpy(dev->ifalias, alias, len+1);
+ if (ifalias)
+ call_rcu(&ifalias->rcu, ifalias_kfree_rcu);
+ rcu_assign_pointer(dev->ifalias, new);
return len;
}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 1b12217..e2acf15 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -281,13 +281,14 @@ static ssize_t show_ifalias(struct device *dev,
struct device_attribute *attr, char *buf)
{
const struct net_device *netdev = to_net_dev(dev);
+ struct ifalias *ifalias;
ssize_t ret = 0;
- if (!rtnl_trylock())
- return restart_syscall();
- if (netdev->ifalias)
- ret = sprintf(buf, "%s\n", netdev->ifalias);
- rtnl_unlock();
+ rcu_read_lock();
+ ifalias = rcu_dereference(netdev->ifalias);
+ if (ifalias)
+ ret = sprintf(buf, "%s\n", ifalias->name);
+ rcu_read_unlock();
return ret;
}
@@ -1265,7 +1266,7 @@ static void netdev_release(struct device *d)
BUG_ON(dev->reg_state != NETREG_RELEASED);
- kfree(dev->ifalias);
+ kfree(rcu_dereference_protected(dev->ifalias, 1));
kfree((char *)dev - dev->padded);
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d2ba259..c8f0a11 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -849,6 +849,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
const struct rtnl_link_stats64 *stats;
struct nlattr *attr, *af_spec;
struct rtnl_af_ops *af_ops;
+ struct ifalias *ifalias;
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
if (nlh == NULL)
@@ -879,8 +880,11 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (dev->qdisc)
NLA_PUT_STRING(skb, IFLA_QDISC, dev->qdisc->ops->id);
- if (dev->ifalias)
- NLA_PUT_STRING(skb, IFLA_IFALIAS, dev->ifalias);
+ rcu_read_lock();
+ ifalias = rcu_dereference(dev->ifalias);
+ if (ifalias)
+ NLA_PUT_STRING(skb, IFLA_IFALIAS, ifalias->name);
+ rcu_read_unlock();
if (1) {
struct rtnl_link_ifmap map = {
^ permalink raw reply related
* Bug, kernel panic, NULL dereference , cleanup_once / icmp_route_lookup.clone.19.clone / nat , 2.6.39-rc7-git11
From: Denys Fedoryshchenko @ 2011-05-17 22:16 UTC (permalink / raw)
To: netdev
Just got recently. 32Bit, PPPoE NAS, shapers, firewall, NAT
Kernel i mention in subject, 2.6.39-rc7-git11
If required i can give more information
sharanal (sorry for ugly name) is libpcap based traffic analyser, sure
userspace
[44528.153869] BUG: unable to handle kernel NULL pointer dereference
at 0000001a
[44528.153869] IP: [<c02e8614>] cleanup_once+0x49/0x1cf
[44528.153869] *pdpt = 0000000034a73001 *pde = 0000000000000000
[44528.153869] Oops: 0002 [#1] SMP
[44528.153869] last sysfs file:
/sys/devices/system/cpu/cpu3/cache/index1/shared_cpu_map
[44528.153869] Modules linked in: nf_conntrack_netlink nfnetlink
ipt_LOG rtc_cmos rtc_core rtc_lib act_skbedit sch_ingress sch_prio
bridge cls_flow cls_u32 em_meta cls_basic xt_dscp ipt_REJECT xt_hl ifb
cls_fw sch_tbf sch_htb a
[44528.153869]
[44528.153869] Pid: 1744, comm: sharanal Not tainted
2.6.39-rc7-git11-build-0058 #6 Intel
/SE7520BD2S
[44528.153869] EIP: 0060:[<c02e8614>] EFLAGS: 00010286 CPU: 1
[44528.153869] EIP is at cleanup_once+0x49/0x1cf
[44528.153869] EAX: dd378920 EBX: dd378900 ECX: 00000016 EDX: 06000001
[44528.153869] ESI: 00000000 EDI: e4b6a59c EBP: f5485b8c ESP: f5485b68
[44528.153869] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[44528.153869] Process sharanal (pid: 1744, ti=f5484000 task=f4c9b6c0
task.ti=f4fa4000)
[44528.153869] Stack:
[44528.153869] f5485c6c f5485ba4 f5485ba4 00000013 f5485ba4 f5485b84
e4b6a500 f5485c6c
[44528.153869] e4b6a59c f5485c50 c02e8b30 f5485bec f5485c58 00000000
c04602d4 c04602d4
[44528.153869] e4252504 f0aecc04 e3ee3200 e8d1b200 efd0ab00 e489dc00
e422e900 e9b18100
[44528.153869] Call Trace:
[44528.153869] [<c02e8b30>] inet_getpeer+0x2ab/0x2cf
[44528.153869] [<c03080bc>] ?
icmp_route_lookup.clone.19.clone.20+0x197/0x1cb
[44528.153869] [<c01320a5>] ? _local_bh_enable_ip.clone.6+0x18/0x71
[44528.153869] [<c0132106>] ? local_bh_enable_ip+0x8/0xa
[44528.153869] [<f88c0929>] ? nf_nat_setup_info+0x3b0/0x3db [nf_nat]
[44528.153869] [<c0318591>] ? ipt_do_table+0x41c/0x437
[44528.153869] [<c02e56a8>] inet_getpeer_v4+0x17/0x19
[44528.153869] [<c02e7a72>] rt_bind_peer+0xe/0x39
[44528.153869] [<c030818d>] icmpv4_xrlim_allow.clone.22+0x4b/0x5f
[44528.153869] [<c03083d4>] icmp_send+0x203/0x282
[44528.153869] [<f88b23aa>] ? ipv4_confirm+0x131/0x13e
[nf_conntrack_ipv4]
[44528.153869] [<c030633b>] __udp4_lib_rcv+0x2d7/0x3f9
[44528.153869] [<c02e2d25>] ? nf_iterate+0x52/0x65
[44528.153869] [<c02e8efb>] ? xfrm4_policy_check.clone.10+0x47/0x47
[44528.153869] [<c030646f>] udp_rcv+0x12/0x14
[44528.153869] [<c02e8fc5>] ip_local_deliver_finish+0xca/0x171
[44528.153869] [<c02e8efb>] ? xfrm4_policy_check.clone.10+0x47/0x47
[44528.153869] [<c02e90b2>] NF_HOOK.clone.11+0x46/0x4d
[44528.153869] [<c02e8efb>] ? xfrm4_policy_check.clone.10+0x47/0x47
[44528.153869] [<c02e91b6>] ip_local_deliver+0x41/0x45
[44528.153869] [<c02e8efb>] ? xfrm4_policy_check.clone.10+0x47/0x47
[44528.153869] [<c02e8e92>] ip_rcv_finish+0x2b4/0x2d6
[44528.153869] [<c02e8bde>] ? pskb_may_pull+0x30/0x30
[44528.153869] [<c02e90b2>] NF_HOOK.clone.11+0x46/0x4d
[44528.153869] [<c02e8bde>] ? pskb_may_pull+0x30/0x30
[44528.153869] [<c02e93a0>] ip_rcv+0x1e6/0x21a
[44528.153869] [<c02e8bde>] ? pskb_may_pull+0x30/0x30
[44528.153869] [<c02c929d>] __netif_receive_skb+0x351/0x379
[44528.153869] [<c02c93f5>] netif_receive_skb+0x46/0x4c
[44528.153869] [<c02c9826>] ? __napi_gro_receive+0x9e/0xa6
[44528.153869] [<c02c94b6>] napi_skb_finish+0x1e/0x34
[44528.153869] [<c02c987d>] napi_gro_receive+0x20/0x24
[44528.153869] [<f84e6516>] e1000_receive_skb+0x5a/0x62 [e1000]
[44528.153869] [<f84e8ac9>] e1000_clean_rx_irq+0x28d/0x323 [e1000]
[44528.153869] [<f84e845d>] e1000_clean+0x2cc/0x43e [e1000]
[44528.153869] [<c02db37e>] ? qdisc_watchdog_schedule+0x39/0x3e
[44528.153869] [<f8cd8221>] ? tbf_dequeue+0x1d/0x1b6 [sch_tbf]
[44528.153869] [<c02b0d50>] ? dma_issue_pending_all+0x60/0x6e
[44528.153869] [<c02c9961>] net_rx_action+0x86/0x139
[44528.153869] [<c0132179>] __do_softirq+0x67/0xf3
[44528.153869] [<c0132112>] ? local_bh_enable+0xa/0xa
[44528.153869] <IRQ>
[44528.153869] [<c0132362>] ? irq_exit+0x35/0x70
[44528.153869] [<c0103d13>] ? do_IRQ+0x79/0x8d
[44528.153869] [<c0132379>] ? irq_exit+0x4c/0x70
[44528.153869] [<c011598a>] ? smp_apic_timer_interrupt+0x66/0x73
[44528.153869] [<c0337c29>] ? common_interrupt+0x29/0x30
[44528.153869] [<c033007b>] ? cpu_init+0x65/0x1c5
[44528.153869] Code: c8 02 46 c0 74 3a 8d 58 e0 8b 15 40 9a 44 c0 2b
53 28 39 f2 73 0f b8 d0 02 46 c0 e8 6b e7 04 00 e9 81 01 00 00 8b 4b 20
8b 53 24
9>[44528.153869] 51 04 89 0a 89 43 20 89 43 24 83 c0 0c e8 cd fd ff
ff eb 02
[44528.153869] EIP: [<c02e8614>] cleanup_once+0x49/0x1cf SS:ESP
0068:f5485b68
[44528.153869] CR2: 000000000000001a
[44528.167278] ---[ end trace dd3639ec5ab2f01f ]---
[44528.167468] Kernel panic - not syncing: Fatal exception in
interrupt
[44528.167660] Pid: 1744, comm: sharanal Tainted: G D
2.6.39-rc7-git11-build-0058 #6
[44528.167992] Call Trace:
[44528.168190] [<c0335548>] ? printk+0x18/0x20
[44528.168374] [<c0335435>] panic+0x57/0x152
[44528.168567] [<c0104e7a>] oops_end+0x92/0x9f
[44528.168759] [<c011b95e>] no_context+0x151/0x159
[44528.168948] [<c011ba72>] __bad_area_nosemaphore+0x10c/0x114
[44528.169154] [<c02ca7df>] ? dev_hard_start_xmit+0x338/0x3f8
[44528.169353] [<c011ba8c>] bad_area_nosemaphore+0x12/0x14
[44528.169544] [<c011bd0c>] do_page_fault+0x12e/0x2ee
[44528.169737] [<c021ba8f>] ? memcmp+0xe/0x25
[44528.169922] [<c01320a5>] ? _local_bh_enable_ip.clone.6+0x18/0x71
[44528.170125] [<c0132110>] ? local_bh_enable+0x8/0xa
[44528.170326] [<c02d1a97>] ? neigh_lookup+0x8b/0x95
[44528.170517] [<c011bbde>] ? vmalloc_sync_all+0x5/0x5
[44528.170713] [<c03374da>] error_code+0x5a/0x60
[44528.170907] [<c01300d8>] ? wait_consider_task+0x3f2/0x76f
[44528.171111] [<c011bbde>] ? vmalloc_sync_all+0x5/0x5
[44528.171304] [<c02e8614>] ? cleanup_once+0x49/0x1cf
[44528.171484] [<c02e8b30>] inet_getpeer+0x2ab/0x2cf
[44528.171672] [<c03080bc>] ?
icmp_route_lookup.clone.19.clone.20+0x197/0x1cb
[44528.171861] [<c01320a5>] ? _local_bh_enable_ip.clone.6+0x18/0x71
[44528.172063] [<c0132106>] ? local_bh_enable_ip+0x8/0xa
[44528.172262] [<f88c0929>] ? nf_nat_setup_info+0x3b0/0x3db [nf_nat]
[44528.172445] [<c0318591>] ? ipt_do_table+0x41c/0x437
[44528.172626] [<c02e56a8>] inet_getpeer_v4+0x17/0x19
[44528.172809] [<c02e7a72>] rt_bind_peer+0xe/0x39
[44528.172990] [<c030818d>] icmpv4_xrlim_allow.clone.22+0x4b/0x5f
[44528.173196] [<c03083d4>] icmp_send+0x203/0x282+++
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Andi Kleen @ 2011-05-17 22:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110517.175044.2057517197524794568.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
>
> We're discussing the idea to do the defragmentation first
> so we can choose the flow properly and steer the packet
> to the correct cpu.
>
> This also would allos each fragmented packet to traverse the
> stack only once (one route lookup etc.) instead of once per
> fragment.
You could always check first in a cheap way (e.g. a small hash table) if
it's local or not (and bypass the defragmentation if routing is turned
off or the hash table would have collisions)
On the other hand if fragmentation is expensive it's probably
better to do it later anyways to spread it out better.
-Andi
--
ak@linux.intel.com -- Speaking for myself only
^ permalink raw reply
* Re: small RPS cache for fragments?
From: Andi Kleen @ 2011-05-17 22:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1305669140.6741.4.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> writes:
>> They're not rwlocks, but especially if the locking was more finegrained
>> that's likely not needed anymore.
>
> Well, there is the rehashing stuff, and this locks the whole table.
>
> Not easy to switch to rcu or something like that.
No need to switch to RCU, just a more finegrained bucket lock.
If you move a chain between queues you just lock both for the move.
It sounds easy enough. I should probably just code it up.
>
> Anyway I hardly use frags here at work, so never considered it was a
> field to spend time ;)
Yes that's the problem. On the other hand most scalability problems hurt
sooner or later, so sometimes it's good to fix them in advance.
-Andi
--
ak@linux.intel.com -- Speaking for myself only
^ permalink raw reply
* Re: sfc: an enumeration is not a bitmask
From: Jeff Garzik @ 2011-05-17 22:00 UTC (permalink / raw)
To: Michał Mirosław; +Cc: David Miller, bhutchings, netdev
In-Reply-To: <BANLkTinaO0VVGpRRqRauH6FucvwbcXyRUg@mail.gmail.com>
On 05/17/2011 03:09 PM, Michał Mirosław wrote:
> 2011/5/17 Jeff Garzik<jgarzik@pobox.com>:
>> 2011/5/17 David Miller<davem@davemloft.net>:
>>> An enumeration is not a bitmask, instead it means one out of the set
>>> of enumerated values will be used.
>>
>> It's a decade-old kernel practice to use 'enum' to define typed
>> constants, preferred over macros that convey no type information and
>> disappear after cpp phase.
>>
>> So your assertion about enumerations is demonstrably not true, as it
>> is often used in the kernel. Call it enum abuse if you want, but it
>> is consistent with code all over the kernel.
> Old age of the mistake doesn't make it correct.
It is not a mistake, but a useful coding tool.
Jeff
^ permalink raw reply
* net-2.6 --> net-next-2.6 merge...
From: David Miller @ 2011-05-17 21:55 UTC (permalink / raw)
To: netdev
I've merged the two trees in order to resolve some
conflicts.
Just FYI...
^ permalink raw reply
* [PATCH] sfc: Don't use enums as a bitmask.
From: David Miller @ 2011-05-17 21:54 UTC (permalink / raw)
To: netdev
This fixes:
drivers/net/sfc/mcdi_mac.c: In function ‘efx_mcdi_set_mac’:
drivers/net/sfc/mcdi_mac.c:36:2: warning: case value ‘3’ not in enumerated type ‘enum efx_fc_type’
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/sfc/efx.c | 2 +-
drivers/net/sfc/efx.h | 2 +-
drivers/net/sfc/ethtool.c | 2 +-
drivers/net/sfc/mdio_10g.c | 2 +-
drivers/net/sfc/mdio_10g.h | 2 +-
drivers/net/sfc/net_driver.h | 12 +++++-------
6 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 05502b3..c914729 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -833,7 +833,7 @@ void efx_link_set_advertising(struct efx_nic *efx, u32 advertising)
}
}
-void efx_link_set_wanted_fc(struct efx_nic *efx, enum efx_fc_type wanted_fc)
+void efx_link_set_wanted_fc(struct efx_nic *efx, u8 wanted_fc)
{
efx->wanted_fc = wanted_fc;
if (efx->link_advertising) {
diff --git a/drivers/net/sfc/efx.h b/drivers/net/sfc/efx.h
index 3d83a1f..b0d1209 100644
--- a/drivers/net/sfc/efx.h
+++ b/drivers/net/sfc/efx.h
@@ -142,6 +142,6 @@ static inline void efx_schedule_channel(struct efx_channel *channel)
extern void efx_link_status_changed(struct efx_nic *efx);
extern void efx_link_set_advertising(struct efx_nic *efx, u32);
-extern void efx_link_set_wanted_fc(struct efx_nic *efx, enum efx_fc_type);
+extern void efx_link_set_wanted_fc(struct efx_nic *efx, u8);
#endif /* EFX_EFX_H */
diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 348437a..d229027 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
@@ -698,7 +698,7 @@ static int efx_ethtool_set_pauseparam(struct net_device *net_dev,
struct ethtool_pauseparam *pause)
{
struct efx_nic *efx = netdev_priv(net_dev);
- enum efx_fc_type wanted_fc, old_fc;
+ u8 wanted_fc, old_fc;
u32 old_adv;
bool reset;
int rc = 0;
diff --git a/drivers/net/sfc/mdio_10g.c b/drivers/net/sfc/mdio_10g.c
index 7115914..7ab385c 100644
--- a/drivers/net/sfc/mdio_10g.c
+++ b/drivers/net/sfc/mdio_10g.c
@@ -284,7 +284,7 @@ void efx_mdio_an_reconfigure(struct efx_nic *efx)
efx_mdio_write(efx, MDIO_MMD_AN, MDIO_CTRL1, reg);
}
-enum efx_fc_type efx_mdio_get_pause(struct efx_nic *efx)
+u8 efx_mdio_get_pause(struct efx_nic *efx)
{
BUILD_BUG_ON(EFX_FC_AUTO & (EFX_FC_RX | EFX_FC_TX));
diff --git a/drivers/net/sfc/mdio_10g.h b/drivers/net/sfc/mdio_10g.h
index df07039..a97dbbd 100644
--- a/drivers/net/sfc/mdio_10g.h
+++ b/drivers/net/sfc/mdio_10g.h
@@ -92,7 +92,7 @@ extern void efx_mdio_an_reconfigure(struct efx_nic *efx);
/* Get pause parameters from AN if available (otherwise return
* requested pause parameters)
*/
-enum efx_fc_type efx_mdio_get_pause(struct efx_nic *efx);
+u8 efx_mdio_get_pause(struct efx_nic *efx);
/* Wait for specified MMDs to exit reset within a timeout */
extern int efx_mdio_wait_reset_mmds(struct efx_nic *efx,
diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index ce9697b..e8d5f03 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
@@ -449,11 +449,9 @@ enum nic_state {
struct efx_nic;
/* Pseudo bit-mask flow control field */
-enum efx_fc_type {
- EFX_FC_RX = FLOW_CTRL_RX,
- EFX_FC_TX = FLOW_CTRL_TX,
- EFX_FC_AUTO = 4,
-};
+#define EFX_FC_RX FLOW_CTRL_RX
+#define EFX_FC_TX FLOW_CTRL_TX
+#define EFX_FC_AUTO 4
/**
* struct efx_link_state - Current state of the link
@@ -465,7 +463,7 @@ enum efx_fc_type {
struct efx_link_state {
bool up;
bool fd;
- enum efx_fc_type fc;
+ u8 fc;
unsigned int speed;
};
@@ -784,7 +782,7 @@ struct efx_nic {
bool promiscuous;
union efx_multicast_hash multicast_hash;
- enum efx_fc_type wanted_fc;
+ u8 wanted_fc;
atomic_t rx_reset;
enum efx_loopback_mode loopback_mode;
--
1.7.4.4
^ permalink raw reply related
* Re: small RPS cache for fragments?
From: Eric Dumazet @ 2011-05-17 21:52 UTC (permalink / raw)
To: Andi Kleen; +Cc: David Miller, netdev
In-Reply-To: <m2aaelypbp.fsf@firstfloor.org>
Le mardi 17 mai 2011 à 14:44 -0700, Andi Kleen a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> >
> > OK but do we have workloads actually needing this optimization at all ?
>
> That's a good a question.
> >
> > (IP defrag hits a read_lock(&ip4_frags.lock)), so maybe steer all frags
> > on a given cpu ?)
>
> Couldn't the lock just be replaced with a hashed or bitmap lock or
> bit in low bits of pointer lock?
>
> iirc it just protects the heads of the hash table.
>
> They're not rwlocks, but especially if the locking was more finegrained
> that's likely not needed anymore.
Well, there is the rehashing stuff, and this locks the whole table.
Not easy to switch to rcu or something like that.
Anyway I hardly use frags here at work, so never considered it was a
field to spend time ;)
^ permalink raw reply
* Re: small RPS cache for fragments?
From: David Miller @ 2011-05-17 21:50 UTC (permalink / raw)
To: andi; +Cc: netdev
In-Reply-To: <m262p9yp5f.fsf@firstfloor.org>
From: Andi Kleen <andi@firstfloor.org>
Date: Tue, 17 May 2011 14:48:28 -0700
> David Miller <davem@davemloft.net> writes:
>
>> Guys we can't time out fragments if we are not the final
>> destination.
>
> If you're not the final destination you should never even
> try to reassemble them?
>
> I'm probably missing something...
We're discussing the idea to do the defragmentation first
so we can choose the flow properly and steer the packet
to the correct cpu.
This also would allos each fragmented packet to traverse the
stack only once (one route lookup etc.) instead of once per
fragment.
Please read the rest of this thread, we have discussed this
and now I'm repeating information solely for your benefit.
^ permalink raw reply
* [PATCH v2] ipconfig wait for carrier
From: Micha Nelissen @ 2011-05-17 21:48 UTC (permalink / raw)
To: netdev; +Cc: Micha Nelissen, David Miller
In-Reply-To: <1305405386-25187-1-git-send-email-micha@neli.hopto.org>
Currently the ip auto configuration has a hardcoded delay of 1 second.
When (ethernet) link takes longer to come up (e.g. more than 3 seconds),
nfs root may not be found.
Remove the hardcoded delay, and wait for carrier on at least one network
device.
Signed-off-by: Micha Nelissen <micha@neli.hopto.org>
Cc: David Miller <davem@davemloft.net>
---
net/ipv4/ipconfig.c | 32 +++++++++++++++++++++-----------
1 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..953808f 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -87,8 +87,8 @@
#endif
/* Define the friendly delay before and after opening net devices */
-#define CONF_PRE_OPEN 500 /* Before opening: 1/2 second */
-#define CONF_POST_OPEN 1 /* After opening: 1 second */
+#define CONF_POST_OPEN 10 /* After opening: 10 msecs */
+#define CONF_CARRIER_TIMEOUT 120000 /* Wait for carrier timeout */
/* Define the timeout for waiting for a DHCP/BOOTP/RARP reply */
#define CONF_OPEN_RETRIES 2 /* (Re)open devices twice */
@@ -188,14 +188,14 @@ struct ic_device {
static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
static struct net_device *ic_dev __initdata = NULL; /* Selected device */
-static bool __init ic_device_match(struct net_device *dev)
+static bool __init ic_is_init_dev(struct net_device *dev)
{
- if (user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
+ if (dev->flags & IFF_LOOPBACK)
+ return 0;
+ return user_dev_name[0] ? !strcmp(dev->name, user_dev_name) :
(!(dev->flags & IFF_LOOPBACK) &&
(dev->flags & (IFF_POINTOPOINT|IFF_BROADCAST)) &&
- strncmp(dev->name, "dummy", 5)))
- return true;
- return false;
+ strncmp(dev->name, "dummy", 5));
}
static int __init ic_open_devs(void)
@@ -203,6 +203,7 @@ static int __init ic_open_devs(void)
struct ic_device *d, **last;
struct net_device *dev;
unsigned short oflags;
+ unsigned long start;
last = &ic_first_dev;
rtnl_lock();
@@ -216,9 +217,7 @@ static int __init ic_open_devs(void)
}
for_each_netdev(&init_net, dev) {
- if (dev->flags & IFF_LOOPBACK)
- continue;
- if (ic_device_match(dev)) {
+ if (ic_is_init_dev(dev)) {
int able = 0;
if (dev->mtu >= 364)
able |= IC_BOOTP;
@@ -252,6 +251,17 @@ static int __init ic_open_devs(void)
dev->name, able, d->xid));
}
}
+
+ /* wait for a carrier on at least one device */
+ start = jiffies;
+ while (jiffies - start < msecs_to_jiffies(CONF_CARRIER_TIMEOUT)) {
+ for_each_netdev(&init_net, dev)
+ if (ic_is_init_dev(dev) && netif_carrier_ok(dev))
+ goto have_carrier;
+
+ msleep(1);
+ }
+have_carrier:
rtnl_unlock();
*last = NULL;
@@ -1378,7 +1388,7 @@ static int __init ip_auto_config(void)
return err;
/* Give drivers a chance to settle */
- ssleep(CONF_POST_OPEN);
+ msleep(CONF_POST_OPEN);
/*
* If the config information is insufficient (e.g., our IP address or
--
1.7.4.4
^ permalink raw reply related
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