* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 1:32 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <4AFC25A7.10301@trash.net>
On Thu, Nov 12, 2009 at 11:11 PM, Patrick McHardy <kaber@trash.net> wrote:
> Changli Gao wrote:
>> The corresponding iprout2 patch is attached.
>
> Printing the value during dumps is missing from this patch.
>
I doesn't know the rules about adding more dumps. Can I add it in the
end of the corresponding line?
ifb0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN qlen 32 ntxqs 4
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 1:28 UTC (permalink / raw)
To: Patrick McHardy
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <4AFC2563.1020902@trash.net>
On Thu, Nov 12, 2009 at 11:10 PM, Patrick McHardy <kaber@trash.net> wrote:
> Changli Gao wrote:
>>>> + rcu_read_lock();
>>>> + skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
>>>> + if (!skb->dev) {
>>>> + rcu_read_unlock();
>>>> + dev_kfree_skb(skb);
>>>> + txq->tx_dropped++;
>>>> + break;
>>>> + }
>>>> + rcu_read_unlock();
>>>> + skb->iif = dev->ifindex;
>>> What protects the device from disappearing here and below during
>>> dev_queue_xmit() and netif_rx_ni()?
>>
>> For dev_queue_xmit(), dev is holded by skb->_dst, so there is no
>> problem. But for netif_rx_ni(), I don't know how to prevent the device
>> disappearing, and it seems that all the NIC drivers have this problem.
>> Maybe there was the assumption about the execution context of
>> netif_rx() before. Now softirq can't be executed by softirqd, so the
>> packet receiving path maybe interleaved. I don't know how to prevent
>> it happening.
>
> You can either take a reference of move the rcu_read_unlock()
> after the skb submission.
good idea!
>
>>>> + break;
>>>> + case __constant_htons(ETH_P_IPV6):
>>>> + ip_proto = ipv6_hdr(skb)->nexthdr;
>>>> + addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
>>>> + addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
>>>> + ihl = 10;
>>> Where does 10 come from?
>>
>> It should be 40, after reviewing IPV6, I found that I need to loop
>> until finding the right protocol value.
>
> There is a helper for this (ipv6_skip_exthdr).
>
It is a little late. Thanks all the same, and I'll use it.
>>>> +static int ifb_get_tx_queues(struct net *net, struct nlattr *tb[],
>>>> + unsigned int *num_tx_queues,
>>>> + unsigned int *real_num_tx_queues)
>>>> +{
>>>> + if (tb[IFLA_NTXQ]) {
>>>> + *num_tx_queues = nla_get_u16(tb[IFLA_NTXQ]);
>>> We currently use unsigned ints for the queue number, so please
>>> use an u32 for the attribute as well.
>>>
>>>> + *real_num_tx_queues = *num_tx_queues;
>>>> + } else {
>>>> + *num_tx_queues = numtxqs;
>>>> + *real_num_tx_queues = numtxqs;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>> u16 (*ndo_select_queue)(struct net_device *dev,
>> struct sk_buff *skb);
>> use u16 as the return value so ..., and I think u16 is big enough. If
>> you insist on this, I'll use u32 instead.
>
> Well, get_tx_queues() uses unsigned int, as does struct net_device.
> I agree that it probably won't ever be needed, but there's no harm
> in using the correct type, the attribute encoding doesn't even need
> more room.
>
So, in the last patch, I use it instead. :)
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] ifb: add multi-queue support
From: Changli Gao @ 2009-11-13 1:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Stephen Hemminger, Patrick McHardy, Tom Herbert,
netdev
In-Reply-To: <4AFC0436.9090906@gmail.com>
2009/11/12 Eric Dumazet <eric.dumazet@gmail.com>:
>
> I believe this patch is fine, but maybe Jarek concern about workqueue
> vs tasklet should be addressed...
>
> We could use the previous handling in case numtxqs==1 , ie use a tasklet
> instead of a work queue ?
I don't think it is a good idea. If we do so, the code will be messy,
and lost the flexibility of process. In fact, latency isn't a problem
when system load isn't high, and when system load is high (due to too
many NIC IRQs), throughput and interaction is more important, and the
current linux networking subsystem just dose so through the softirqd
mechanism.
>
>
> Sorry for this late idea...
>
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RACE] net: in process_backlog
From: Changli Gao @ 2009-11-13 1:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20091112161123.561d87fe@nehalam>
On Fri, Nov 13, 2009 at 8:11 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 13 Nov 2009 07:54:14 +0800
> Changli Gao <xiaosuo@gmail.com> wrote:
>
>>
>> Yea, if the process waits for a RCU grace period, there will be no
>> race. But think about another case:
>> 1. flush_backlog().
> After flush backlog there should be no more skb's with that device
> in the queue, and if more are added, the device is buggy.
>
>> 2. dev_hold(skb->dev); netif_rx(). dev_put(skb->dev);
> There is no dev_hold in netif_rx path.
>
If the caller of netif_rx or netif_rx_ni isn't the device driver self,
it must find and hold the dev, then call netif_rx and put it.
>> 3. wait_for_refs();
>> 4. free(dev);
>> 5. netif_receive_skb(); //skb->dev doesn't present.
>> flush_backlog() can't prevent new skbs are added to backlog. If we
>> swap the flush_backlog() and wait_for_refs(), this case will be OK
>> too.
>
> It is still up to device driver not to add skb's to queue when stopped.
>
do you mean before calling netif_rx, the caller should check the dev
is alive or not? If so, why not add the check into netif_rx or
netif_rx_ni()? Could you explains why swapping the flush_backlog() and
wait_for_refs() can't work?
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RACE] net: in process_backlog
From: Stephen Hemminger @ 2009-11-13 0:11 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Patrick McHardy, netdev
In-Reply-To: <412e6f7f0911121554n22a6f975h1fb5df59bd4b84a2@mail.gmail.com>
On Fri, 13 Nov 2009 07:54:14 +0800
Changli Gao <xiaosuo@gmail.com> wrote:
> On Fri, Nov 13, 2009 at 12:57 AM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
> > On Thu, 12 Nov 2009 16:50:53 +0800
> > Changli Gao <xiaosuo@gmail.com> wrote:
> >
> >
> > There is are a couple of issues here, but it is not what you thought
> > you saw.
> >
> > The receive process is always done in soft IRQ context. The backlog queue's
> > are per-cpu. When a device is deleted an IPI is sent to all cpu's to
> > scan there backlog queue. What should protect the skb is the fact that
> > the network device destruction process waits for an RCU grace period.
> > So skb->dev points to valid data.
>
> Yea, if the process waits for a RCU grace period, there will be no
> race. But think about another case:
> 1. flush_backlog().
After flush backlog there should be no more skb's with that device
in the queue, and if more are added, the device is buggy.
> 2. dev_hold(skb->dev); netif_rx(). dev_put(skb->dev);
There is no dev_hold in netif_rx path.
> 3. wait_for_refs();
> 4. free(dev);
> 5. netif_receive_skb(); //skb->dev doesn't present.
> flush_backlog() can't prevent new skbs are added to backlog. If we
> swap the flush_backlog() and wait_for_refs(), this case will be OK
> too.
It is still up to device driver not to add skb's to queue when stopped.
--
^ permalink raw reply
* Re: [RACE] net: in process_backlog
From: Changli Gao @ 2009-11-12 23:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20091112085739.1137f690@nehalam>
On Fri, Nov 13, 2009 at 12:57 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Thu, 12 Nov 2009 16:50:53 +0800
> Changli Gao <xiaosuo@gmail.com> wrote:
>
>
> There is are a couple of issues here, but it is not what you thought
> you saw.
>
> The receive process is always done in soft IRQ context. The backlog queue's
> are per-cpu. When a device is deleted an IPI is sent to all cpu's to
> scan there backlog queue. What should protect the skb is the fact that
> the network device destruction process waits for an RCU grace period.
> So skb->dev points to valid data.
Yea, if the process waits for a RCU grace period, there will be no
race. But think about another case:
1. flush_backlog().
2. dev_hold(skb->dev); netif_rx(). dev_put(skb->dev);
3. wait_for_refs();
4. free(dev);
5. netif_receive_skb(); //skb->dev doesn't present.
flush_backlog() can't prevent new skbs are added to backlog. If we
swap the flush_backlog() and wait_for_refs(), this case will be OK
too.
>
> BUT the flush_backlog is run too late in the device destruction process.
> It should be moved out of netdev_run_todo, to right after dev_shutdown().
> Also adding a check for skb->dev->reg_state in netif_receive_skb would
> be wise to drop packets.
>
>
> --
>
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH v2] Documentation: rw_lock lessons learned
From: Stephen Hemminger @ 2009-11-12 23:00 UTC (permalink / raw)
To: sclark46
Cc: William Allen Simpson, Paul E. McKenney, Linus Torvalds,
Linux Kernel Developers, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <4AFC5E3F.7070609@earthlink.net>
On Thu, 12 Nov 2009 14:13:03 -0500
Stephen Clark <sclark46@earthlink.net> wrote:
> 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
>
Out of date.
1. Missing mutex's which have largely replaced semaphores.
2. Missing change to lock initialization in later kernels.
3. Missing description of lock dependency checker which should be in same guide.
^ permalink raw reply
* Re: [Bugme-new] [Bug 14586] New: bridge on bonding interface: DHCP replies don't get through
From: Stephen Hemminger @ 2009-11-12 22:55 UTC (permalink / raw)
To: harald.dunkel
Cc: Andrew Morton, bridge, bugzilla-daemon, bugme-daemon, netdev
In-Reply-To: <20091112143913.3d89e4f4.akpm@linux-foundation.org>
>
> I would like to run a bridge for kvm on a bonding interface (4 * 1Gbit, Intel
> e1000e). Problem: The DHCPDISCOVER packets sent by the guest show up on my dhcp
> server as expected, but the DHCPOFFER sent as a reply doesn't reach the guest
> behind the bridge.
>
> Using tcpdump on host and guest I can see the DHCPOFFER on the bond0 and br0
> interface, but it never shows up on vnet0 or on the guest's eth0.
>
> If I drop the bonding interface and use the host's eth2 for the bridge instead,
> then there is no such problem.
>
> Kernel on host and guest is 2.6.31.5. Attached you can find more information
> about my setup.
>
What is the configuration?
# brctl showstp virbr0
# brctl showmacs virbr0
Is dhclient being run on the bridge interface?
# cat /proc/net/ptype
# cat /proc/net/bonding/bond0
How is bond and bridge configured? Are bonding bridges (wrong)
or bridging bonded interfaces?
Are all links up?
Since this is the initial packet it will have to be flood forwarded by
the bridge, is there any iptables/netfilter rule that might be blocking
packets?
^ permalink raw reply
* Re: [Bugme-new] [Bug 14586] New: bridge on bonding interface: DHCP replies don't get through
From: Andrew Morton @ 2009-11-12 22:39 UTC (permalink / raw)
To: Stephen Hemminger, bridge
Cc: bugzilla-daemon, bugme-daemon, netdev, harald.dunkel
In-Reply-To: <bug-14586-10286@http.bugzilla.kernel.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Wed, 11 Nov 2009 14:28:00 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=14586
>
> Summary: bridge on bonding interface: DHCP replies don't get
> through
> Product: Networking
> Version: 2.5
> Kernel Version: 2.6.31.5
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> AssignedTo: acme@ghostprotocols.net
> ReportedBy: harald.dunkel@t-online.de
> Regression: No
>
>
> I would like to run a bridge for kvm on a bonding interface (4 * 1Gbit, Intel
> e1000e). Problem: The DHCPDISCOVER packets sent by the guest show up on my dhcp
> server as expected, but the DHCPOFFER sent as a reply doesn't reach the guest
> behind the bridge.
>
> Using tcpdump on host and guest I can see the DHCPOFFER on the bond0 and br0
> interface, but it never shows up on vnet0 or on the guest's eth0.
>
> If I drop the bonding interface and use the host's eth2 for the bridge instead,
> then there is no such problem.
>
> Kernel on host and guest is 2.6.31.5. Attached you can find more information
> about my setup.
>
> I had sent this information to the linux kvm mailing list before, but consensus
> was that this is a bridging problem. See
>
> http://www.spinics.net/lists/kvm/msg25153.html
>
> There was no reply on the linux bridge mailing list, see
>
>
> https://lists.linux-foundation.org/pipermail/bridge/2009-November/006749.html
^ permalink raw reply
* 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
* [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: [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
* 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
* 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: 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
* [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: [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
* 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 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 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: 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: [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 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: 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] 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
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