* [PATCH] tcp: Fix a connect() race with timewait sockets
From: Eric Dumazet @ 2009-12-01 15:00 UTC (permalink / raw)
To: kapil dakhane; +Cc: netdev, netfilter, David S. Miller, Evgeniy Polyakov
In-Reply-To: <99d458640911301802i4bde20f4wa314668d543e3170@mail.gmail.com>
kapil dakhane a écrit :
> Hello,
>
> I am trying to analyze the capacity of linux network stack on x6270
> which has 16 Hyper threads on two 8-core Intel(r) Xeon(r) CPU. I see
> that at around 150000 simultaneous connections, after around 1.6 gbps,
> a cpu get stuck in an infinite loop in inet_csk_bind_conflict, then
> other cpus get locked up doing spin_lock. Before the lockup cpu usage
> was around 25%. It appears to be a bug, unless I am hitting some kind
> of resource limit. It would be good if someone familiar with network
> code would confirm this, or point me in the right direction.
>
> Important details are:
>
> I am using kernel version 2.6.31.4 recompiled with TPROXY related
> options: NF_CONNTRACK, NETFILTER_TPROXY, NETFILTER_XT_MATCH_SOCKET,
> NETFILTER_XT_TARGET_TPROXY.
>
>
> I have enabled transparent capture and transparent forward using
> iptables and ip rules. I have 10 instances of a single threaded user
> space bits-forwarding-proxy (fast), each bound to different
> hyper-threads (CPUs). Rest 6 CPUs are dedicated to interrupt
> processing, each handling interrupts from six different network cards.
> TCP flow from a 4-tuple always get handled by the same proxy process,
> interrupt thread, and network card. In this way, network traffic is
> segregated as much as possible to achieve high degree of parallelism.
>
> First /var/log/message entry shows CPU#7 is stuck in inet_csk_bind_conflict
>
> Nov 17 23:02:04 cap-x6270-01 kernel: BUG: soft lockup - CPU#7 stuck
> for 61s! [fast:20701]
After some more audit and coffee, I finally found one subtle bug in our
connect() code, that periodically triggers but never got tracked.
Here is a patch cooked on top of current linux-2.6 git tree, it should probably
apply on 2.6.31.6 as well...
Thanks
[PATCH] tcp: Fix a connect() race with timewait sockets
When we find a timewait connection in __inet_hash_connect() and reuse
it for a new connection request, we have a race window, releasing bind
list lock and reacquiring it in __inet_twsk_kill() to remove timewait
socket from list.
Another thread might find the timewait socket we already chose, leading to
list corruption and crashes.
Fix is to remove timewait socket from bind list before releasing the lock.
Reported-by: kapil dakhane <kdakhane@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/inet_timewait_sock.h | 4 +++
net/ipv4/inet_hashtables.c | 4 +++
net/ipv4/inet_timewait_sock.c | 37 ++++++++++++++++++++---------
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index f93ad90..e18e5df 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -206,6 +206,10 @@ extern void __inet_twsk_hashdance(struct inet_timewait_sock *tw,
struct sock *sk,
struct inet_hashinfo *hashinfo);
+extern void inet_twsk_unhash(struct inet_timewait_sock *tw,
+ struct inet_hashinfo *hashinfo,
+ bool mustlock);
+
extern void inet_twsk_schedule(struct inet_timewait_sock *tw,
struct inet_timewait_death_row *twdr,
const int timeo, const int timewait_len);
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 625cc5f..76d81e4 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -488,6 +488,10 @@ ok:
inet_sk(sk)->sport = htons(port);
hash(sk);
}
+
+ if (tw)
+ inet_twsk_unhash(tw, hinfo, false);
+
spin_unlock(&head->lock);
if (tw) {
diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 13f0781..2d6d543 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -14,12 +14,34 @@
#include <net/inet_timewait_sock.h>
#include <net/ip.h>
+
+void inet_twsk_unhash(struct inet_timewait_sock *tw,
+ struct inet_hashinfo *hashinfo,
+ bool mustlock)
+{
+ struct inet_bind_hashbucket *bhead;
+ struct inet_bind_bucket *tb = tw->tw_tb;
+
+ if (!tb)
+ return;
+
+ /* Disassociate with bind bucket. */
+ bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw),
+ tw->tw_num,
+ hashinfo->bhash_size)];
+ if (mustlock)
+ spin_lock(&bhead->lock);
+ __hlist_del(&tw->tw_bind_node);
+ tw->tw_tb = NULL;
+ inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+ if (mustlock)
+ spin_unlock(&bhead->lock);
+}
+
/* Must be called with locally disabled BHs. */
static void __inet_twsk_kill(struct inet_timewait_sock *tw,
struct inet_hashinfo *hashinfo)
{
- struct inet_bind_hashbucket *bhead;
- struct inet_bind_bucket *tb;
/* Unlink from established hashes. */
spinlock_t *lock = inet_ehash_lockp(hashinfo, tw->tw_hash);
@@ -32,15 +54,8 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
sk_nulls_node_init(&tw->tw_node);
spin_unlock(lock);
- /* Disassociate with bind bucket. */
- bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), tw->tw_num,
- hashinfo->bhash_size)];
- spin_lock(&bhead->lock);
- tb = tw->tw_tb;
- __hlist_del(&tw->tw_bind_node);
- tw->tw_tb = NULL;
- inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
- spin_unlock(&bhead->lock);
+ inet_twsk_unhash(tw, hashinfo, true);
+
#ifdef SOCK_REFCNT_DEBUG
if (atomic_read(&tw->tw_refcnt) != 1) {
printk(KERN_DEBUG "%s timewait_sock %p refcnt=%d\n",
^ permalink raw reply related
* Re: [PATCH net-next-2.6] bonding: allow arp_ip_targets to be on a separate vlan from bond device
From: Andy Gospodarek @ 2009-12-01 14:44 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <6611.1259632635@death.nxdomain.ibm.com>
On Mon, Nov 30, 2009 at 05:57:15PM -0800, Jay Vosburgh wrote:
> Andy Gospodarek <andy@greyhouse.net> wrote:
>
> >On Mon, Nov 30, 2009 at 04:00:38PM -0800, Jay Vosburgh wrote:
> >> Andy Gospodarek <andy@greyhouse.net> wrote:
> >>
> >> >This allows a bond device to specify an arp_ip_target as a host that is
> >> >not on the same vlan as the base bond device. A configuration like
> >> >this, now works:
> >> >
> >> >1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue
> >> > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >> > inet 127.0.0.1/8 scope host lo
> >> > inet6 ::1/128 scope host
> >> > valid_lft forever preferred_lft forever
> >> >2: eth1: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000
> >> > link/ether 00:13:21:be:33:e9 brd ff:ff:ff:ff:ff:ff
> >> >3: eth0: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000
> >> > link/ether 00:13:21:be:33:e9 brd ff:ff:ff:ff:ff:ff
> >> >8: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue
> >> > link/ether 00:13:21:be:33:e9 brd ff:ff:ff:ff:ff:ff
> >> > inet6 fe80::213:21ff:febe:33e9/64 scope link
> >> > valid_lft forever preferred_lft forever
> >> >9: bond0.100@bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue
> >> > link/ether 00:13:21:be:33:e9 brd ff:ff:ff:ff:ff:ff
> >> > inet 10.0.100.2/24 brd 10.0.100.255 scope global bond0.100
> >> > inet6 fe80::213:21ff:febe:33e9/64 scope link
> >> > valid_lft forever preferred_lft forever
> >>
> >> I'm not quite clear here on exactly what it is that doesn't
> >> work.
> >>
> >> Putting the arp_ip_target on a VLAN destination already works
> >> (and has for a long time); I just checked against a 2.6.32-rc to make
> >> sure I wasn't misremembering.
> >>
> >> Perhaps there's some nuance of "not on the same vlan as the base
> >> bond device" that I'm missing. What I see working before me is, e.g., a
> >> bond0.777 VLAN interface atop a regular bond0 active-backup with a
> >> couple of slaves; bond0 may or may not have an IP address of its own.
> >> The arp_ip_target destination is on VLAN 777 somewhere.
> >
> >Do you have net.ipv4.conf.all.arp_ignore set to 0 and/or an IP address
> >assigned on bond0? I can easily reproduce this with no IP on bond0 and
> >net.ipv4.conf.all.arp_ignore = 1.
> >
> >I can't say for sure that the sysctl setting makes a difference, but I
> >have that on all my test rigs, so it's worth mentioning.
> >
> >> Is this what your patch is meant to enable, or is it something
> >> different? I'm pulling down today's net-next to see if this is
> >> something that broke recently.
> >>
> >
> >I first tested and found the problem while running 2.6.30-rc series
> >after it was reported to be a problem on RHEL5. It's not clear how long
> >it has been broken, but this situation is odd enough that it probably
> >never worked as it was never tested.
>
> I tried it with both arp_ignore set to 1 and 0, and with the
> bond0 interface with and without an IP address. It works fine in all
> four cases. I'm using net-next-2.6 pulled earlier today; it claims to
> be 2.6.32-rc7.
>
> I've tested "ARP monitor over VLAN" in the past, so it's worked
> for me before. Heck, it's working right now.
>
> I thought maybe you have "arp_validate" enabled (which doesn't
> work over a VLAN), but your patch doesn't help there, so presumably not.
> Fixing that is a totally separate adventure into hook-ville; I'd briefly
> hoped you'd found a better way.
>
I am using arp_validate, actually. I forgot that the arp_validate
option doesn't show up in the output of /proc/net/bonding/bondX and I
intended to have that in the subject, but somehow dropped it.
Here is a bit more detail on my setup:
# grep -v ^# /etc/sysconfig/network-scripts/ifcfg-bond0
DEVICE=bond0
BOOTPROTO=none
ONBOOT=no
BONDING_OPTS="mode=active-backup arp_interval=1000 arp_ip_target=10.0.100.1 arp_validate=3"
(The 'active' and 'backup' arp_validate options work just as well as
'all.')
Here is the dmesg output for the above config:
bonding: bond0: Warning: failed to get speed and duplex from eth3, assumed to be 100Mb/sec and Full.
bonding: bond0: enslaving eth3 as a backup interface with an up link.
bonding: bond0: setting mode to active-backup (1).
bonding: bond0: Setting ARP monitoring interval to 1000.
bonding: bond0: ARP target 10.0.100.1 is already present
bonding: bond0: setting arp_validate to all (3).
bnx2: eth2 NIC Copper Link is Up, 100 Mbps full duplex, receive & transmit flow control ON
bnx2: eth3 NIC Copper Link is Up, 100 Mbps full duplex, receive & transmit flow control ON
bonding: bond0: link status definitely down for interface eth2, disabling it
bonding: bond0: making interface eth3 the new active one.
bonding: bond0: no route to arp_ip_target 10.0.100.1
bonding: bond0: link status definitely up for interface eth2.
bond0: no IPv6 routers present
bond0.100: no IPv6 routers present
# cat /proc/net/bonding/bond0
Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)
Bonding Mode: fault-tolerance (active-backup)
Primary Slave: None
Currently Active Slave: eth3
MII Status: up
MII Polling Interval (ms): 0
Up Delay (ms): 0
Down Delay (ms): 0
ARP Polling Interval (ms): 1000
ARP IP target/s (n.n.n.n form): 10.0.100.1
Slave Interface: eth2
MII Status: up
Link Failure Count: 1
Permanent HW addr: 00:10:18:36:0a:d4
Slave Interface: eth3
MII Status: up
Link Failure Count: 0
Permanent HW addr: 00:10:18:36:0a:d6
> When it's failing, are you getting any messages in dmesg? I'm
> wondering specifically about any of the various routing-related things
> that bond_arp_send_all might kick out.
>
When it doesn't work, dmesg just looks like this:
bonding: bond0: Warning: failed to get speed and duplex from eth3, assumed to be 100Mb/sec and Full.
bonding: bond0: enslaving eth3 as a backup interface with an up link.
bonding: bond0: setting mode to active-backup (1).
bonding: bond0: Setting ARP monitoring interval to 1000.
bonding: bond0: ARP target 10.0.100.1 is already present
bonding: bond0: setting arp_validate to all (3).
bnx2: eth2 NIC Copper Link is Up, 100 Mbps full duplex, receive & transmit flow control ON
bnx2: eth3 NIC Copper Link is Up, 100 Mbps full duplex, receive & transmit flow control ON
bonding: bond0: link status definitely down for interface eth2, disabling it
bonding: bond0: making interface eth3 the new active one.
bonding: bond0: no route to arp_ip_target 10.0.100.1
bonding: bond0: link status definitely down for interface eth3, disabling it
bonding: bond0: now running without any active interface !
bond0: no IPv6 routers present
bond0.100: no IPv6 routers present
# cat /proc/net/bonding/bond0
Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)
Bonding Mode: fault-tolerance (active-backup)
Primary Slave: None
Currently Active Slave: None
MII Status: down
MII Polling Interval (ms): 0
Up Delay (ms): 0
Down Delay (ms): 0
ARP Polling Interval (ms): 1000
ARP IP target/s (n.n.n.n form): 10.0.100.1
Slave Interface: eth2
MII Status: down
Link Failure Count: 1
Permanent HW addr: 00:10:18:36:0a:d4
Slave Interface: eth3
MII Status: down
Link Failure Count: 1
Permanent HW addr: 00:10:18:36:0a:d6
Though I think that information won't be that useful now that I've
actually explained this patch makes the arp_validate options work with a
vlan setup like this.
^ permalink raw reply
* notifier chain on br0
From: ratheesh k @ 2009-12-01 14:26 UTC (permalink / raw)
To: bridge; +Cc: netdev
Hi All ,
i have a software bridge br0 on eth0 and eth1 . Can i use NETLINK
sockets to get ip address change notification on br0 .???
Thanks,
Ratheesh
^ permalink raw reply
* Re: linux-next: manual merge of the net tree with the wireless-current tree
From: John W. Linville @ 2009-12-01 14:06 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, netdev, linux-next, linux-kernel, Johannes Berg
In-Reply-To: <20091201123142.fe6bc824.sfr@canb.auug.org.au>
On Tue, Dec 01, 2009 at 12:31:42PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net tree got a conflict in net/mac80211/ht.c between commit 827d42c9ac91ddd728e4f4a31fefb906ef2ceff7 ("mac80211: fix spurious delBA handling") from the wireless-current tree and commit c951ad3550ab40071bb0f222ba6125845769c08a ("mac80211: convert aggregation to operate on vifs/stas") from the net tree.
>
> I fixed it up (by using the version from the wireless-current tree) and
> can carry the fix as necessary. This may well not be correct.
That sounds like the correct fix...thanks!
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: Netlink event for network device statistics
From: Marcel Holtmann @ 2009-12-01 13:36 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B14FDFB.7060600@trash.net>
Hi Patrick,
> > so I was playing with monitoring network device statistics and storing
> > them over a period of time. While netlink and ethtool provides all the
> > network packet statistics, I have a problem with devices that are
> > actually hotplug. Especially 3G cards that get taken off the bus via
> > RFKILL and external dongles that can be unplugged at any time.
> >
> > So I was thinking that before we send the DELLINK netlink event for that
> > interface, we should send the latest statistic details via netlink. Is
> > that a good idea or is there another way to get up-to-date statistics
> > without polling for them?
>
> Don't we already include them in the DELLINK message? rtnl_fill_ifinfo()
> unconditionally includes the statistics.
you are correct. We already have the IFLA_STATS in DELLINK. I got
confused with ip monitor not decoding these and assumed they were not
included at all.
Regards
Marcel
^ permalink raw reply
* Re: [tproxy,regression] tproxy broken in 2.6.32
From: jamal @ 2009-12-01 13:34 UTC (permalink / raw)
To: KOVACS Krisztian
Cc: KOVACS Krisztian, Patrick McHardy, Andreas Schultz, tproxy,
netdev
In-Reply-To: <1259589577.873.30.camel@bigi>
[-- Attachment #1: Type: text/plain, Size: 247 bytes --]
On Mon, 2009-11-30 at 08:59 -0500, jamal wrote:
> [I could move the check into fib_validate, but that would punish other
> users with a few extra cycles].
As in the following patch (gleaned from Patrick's patch on send to self)
cheers,
jamal
[-- Attachment #2: fib-val-sysctl2 --]
[-- Type: text/x-patch, Size: 2734 bytes --]
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ad27c7d..9cd0bcf 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -83,6 +83,7 @@ static inline void ipv4_devconf_setall(struct in_device *in_dev)
#define IN_DEV_FORWARD(in_dev) IN_DEV_CONF_GET((in_dev), FORWARDING)
#define IN_DEV_MFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), MC_FORWARDING)
#define IN_DEV_RPFILTER(in_dev) IN_DEV_MAXCONF((in_dev), RP_FILTER)
+#define IN_DEV_SRC_VMARK(in_dev) IN_DEV_ORCONF((in_dev), SRC_VMARK)
#define IN_DEV_SOURCE_ROUTE(in_dev) IN_DEV_ANDCONF((in_dev), \
ACCEPT_SOURCE_ROUTE)
#define IN_DEV_BOOTP_RELAY(in_dev) IN_DEV_ANDCONF((in_dev), BOOTP_RELAY)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 1e4743e..843f71b 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -490,6 +490,7 @@ enum
NET_IPV4_CONF_PROMOTE_SECONDARIES=20,
NET_IPV4_CONF_ARP_ACCEPT=21,
NET_IPV4_CONF_ARP_NOTIFY=22,
+ NET_IPV4_CONF_SRC_VMARK=23,
__NET_IPV4_CONF_MAX
};
diff --git a/kernel/sysctl_check.c b/kernel/sysctl_check.c
index b6e7aae..469193c 100644
--- a/kernel/sysctl_check.c
+++ b/kernel/sysctl_check.c
@@ -220,6 +220,7 @@ static const struct trans_ctl_table trans_net_ipv4_conf_vars_table[] = {
{ NET_IPV4_CONF_PROMOTE_SECONDARIES, "promote_secondaries" },
{ NET_IPV4_CONF_ARP_ACCEPT, "arp_accept" },
{ NET_IPV4_CONF_ARP_NOTIFY, "arp_notify" },
+ { NET_IPV4_CONF_SRC_VMARK, "src_valid_mark" },
{}
};
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5df2f6a..0030e73 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1450,6 +1450,7 @@ static struct devinet_sysctl_table {
DEVINET_SYSCTL_RW_ENTRY(SEND_REDIRECTS, "send_redirects"),
DEVINET_SYSCTL_RW_ENTRY(ACCEPT_SOURCE_ROUTE,
"accept_source_route"),
+ DEVINET_SYSCTL_RW_ENTRY(SRC_VMARK, "src_valid_mark"),
DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP, "proxy_arp"),
DEVINET_SYSCTL_RW_ENTRY(MEDIUM_ID, "medium_id"),
DEVINET_SYSCTL_RW_ENTRY(BOOTP_RELAY, "bootp_relay"),
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index aa00398..b489135 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -241,16 +241,19 @@ int fib_validate_source(__be32 src, __be32 dst, u8 tos, int oif,
.iif = oif };
struct fib_result res;
- int no_addr, rpf;
+ int no_addr, rpf, validate_mark;
int ret;
struct net *net;
- no_addr = rpf = 0;
+ no_addr = rpf = validate_mark = 0;
rcu_read_lock();
in_dev = __in_dev_get_rcu(dev);
if (in_dev) {
no_addr = in_dev->ifa_list == NULL;
rpf = IN_DEV_RPFILTER(in_dev);
+ validate_mark = IN_DEV_SRC_VMARK(in_dev);
+ if (!validate_mark)
+ mark = 0;
}
rcu_read_unlock();
^ permalink raw reply related
* Re: net 04/05: fib_rules: allow to delete local rule
From: jamal @ 2009-12-01 13:23 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, kuznet, robert
In-Reply-To: <20091130175534.7555.48216.sendpatchset@x2.localnet>
Nice. I recall there was a lot of sentiment against this back
when - in particular from Alexey. I cant remember the details
neither can i think off top of my head why this would be bad
other than allowing people to shoot their big toe without
knowing it.
CCing Robert and Alexey. Mass quoting to provide context for
both Alexey and Robert.
cheers,
jamal
On Mon, 2009-11-30 at 18:55 +0100, Patrick McHardy wrote:
> commit ca1ba96aaa05cc0a2a7f172990e7787354c8b7b9
> Author: Patrick McHardy <kaber@trash.net>
> Date: Mon Nov 30 16:05:51 2009 +0100
>
> net: fib_rules: allow to delete local rule
>
> Allow to delete the local rule and recreate it with a lower priority. This
> can be used to force packets with a local destination out on the wire instead
> of routing them to loopback. Additionally this patch allows to recreate rules
> with a priority of 0.
>
> Combined with the previous patch to allow oif classification, a socket can
> be bound to the desired interface and packets routed to the wire like this:
>
> # move local rule to lower priority
> ip rule add pref 1000 lookup local
> ip rule del pref 0
>
> # route packets of sockets bound to eth0 to the wire independant
> # of the destination address
> ip rule add pref 100 oif eth0 lookup 100
> ip route add default dev eth0 lookup 100
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index d1a70ad..ef0e7d9 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -287,7 +287,7 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
> rule->flags = frh->flags;
> rule->table = frh_get_table(frh, tb);
>
> - if (!rule->pref && ops->default_pref)
> + if (!tb[FRA_PRIORITY] && ops->default_pref)
> rule->pref = ops->default_pref(ops);
>
> err = -EINVAL;
> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
> index 835262c..1239ed2 100644
> --- a/net/ipv4/fib_rules.c
> +++ b/net/ipv4/fib_rules.c
> @@ -284,7 +284,7 @@ static int fib_default_rules_init(struct fib_rules_ops *ops)
> {
> int err;
>
> - err = fib_default_rule_add(ops, 0, RT_TABLE_LOCAL, FIB_RULE_PERMANENT);
> + err = fib_default_rule_add(ops, 0, RT_TABLE_LOCAL, 0);
> if (err < 0)
> return err;
> err = fib_default_rule_add(ops, 0x7FFE, RT_TABLE_MAIN, 0);
> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> index 00a7a5e..3b38f49 100644
> --- a/net/ipv6/fib6_rules.c
> +++ b/net/ipv6/fib6_rules.c
> @@ -276,7 +276,7 @@ static int fib6_rules_net_init(struct net *net)
> INIT_LIST_HEAD(&net->ipv6.fib6_rules_ops->rules_list);
>
> err = fib_default_rule_add(net->ipv6.fib6_rules_ops, 0,
> - RT6_TABLE_LOCAL, FIB_RULE_PERMANENT);
> + RT6_TABLE_LOCAL, 0);
> if (err)
> goto out_fib6_rules_ops;
>
> --
> 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
* Re: Netlink event for network device statistics
From: Patrick McHardy @ 2009-12-01 11:28 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: netdev
In-Reply-To: <1259535397.4421.3.camel@violet>
Marcel Holtmann wrote:
> Hi everyone,
>
> so I was playing with monitoring network device statistics and storing
> them over a period of time. While netlink and ethtool provides all the
> network packet statistics, I have a problem with devices that are
> actually hotplug. Especially 3G cards that get taken off the bus via
> RFKILL and external dongles that can be unplugged at any time.
>
> So I was thinking that before we send the DELLINK netlink event for that
> interface, we should send the latest statistic details via netlink. Is
> that a good idea or is there another way to get up-to-date statistics
> without polling for them?
Don't we already include them in the DELLINK message? rtnl_fill_ifinfo()
unconditionally includes the statistics.
^ permalink raw reply
* iplink_vlan: add support for VLAN loose binding flag
From: Patrick McHardy @ 2009-12-01 11:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 103 bytes --]
This patch adds support for the VLAN loose binding flag that is
supported in net-next to iplink_vlan.
[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 1790 bytes --]
commit 870970deb6cbea7a5d4881bdd717304d5284d315
Author: Patrick McHardy <kaber@trash.net>
Date: Tue Dec 1 12:21:15 2009 +0100
iplink_vlan: add support for VLAN loose binding flag
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 2dc4a57..329b354 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -33,6 +33,7 @@ enum vlan_ioctl_cmds {
enum vlan_flags {
VLAN_FLAG_REORDER_HDR = 0x1,
VLAN_FLAG_GVRP = 0x2,
+ VLAN_FLAG_LOOSE_BINDING = 0x4,
};
enum vlan_name_types {
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 9724482..223feb3 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -27,6 +27,7 @@ static void explain(void)
"VLANID := 0-4095\n"
"FLAG-LIST := [ FLAG-LIST ] FLAG\n"
"FLAG := [ reorder_hdr { on | off } ] [ gvrp { on | off } ]\n"
+ " [ loose_binding { on | off } ]\n"
"QOS-MAP := [ QOS-MAP ] QOS-MAPPING\n"
"QOS-MAPPING := FROM:TO\n"
);
@@ -102,6 +103,15 @@ static int vlan_parse_opt(struct link_util *lu, int argc, char **argv,
flags.flags &= ~VLAN_FLAG_GVRP;
else
return on_off("gvrp");
+ } else if (matches(*argv, "loose_binding") == 0) {
+ NEXT_ARG();
+ flags.mask |= VLAN_FLAG_LOOSE_BINDING;
+ if (strcmp(*argv, "on") == 0)
+ flags.flags |= VLAN_FLAG_LOOSE_BINDING;
+ else if (strcmp(*argv, "off") == 0)
+ flags.flags &= ~VLAN_FLAG_LOOSE_BINDING;
+ else
+ return on_off("loose_binding");
} else if (matches(*argv, "ingress-qos-map") == 0) {
NEXT_ARG();
if (vlan_parse_qos_map(&argc, &argv, n,
@@ -156,6 +166,7 @@ static void vlan_print_flags(FILE *fp, __u32 flags)
}
_PF(REORDER_HDR);
_PF(GVRP);
+ _PF(LOOSE_BINDING);
#undef _PF
if (flags)
fprintf(fp, "%x", flags);
^ permalink raw reply related
* Re: net 03/05: fib_rules: add oif classification
From: Jarek Poplawski @ 2009-12-01 9:48 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4B14E2B8.3030507@trash.net>
On Tue, Dec 01, 2009 at 10:32:40AM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > Patrick McHardy wrote, On 11/30/2009 06:55 PM:
> >
> >> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
> >> index 62bebcb..d4e875a 100644
> >> --- a/include/net/fib_rules.h
> >> +++ b/include/net/fib_rules.h
> >> @@ -11,6 +11,7 @@ struct fib_rule {
> >> struct list_head list;
> >> atomic_t refcnt;
> >> int iifindex;
> >> + int oifindex;
> >>
> >
> > Doesn't it "break" the cacheline fix from 01/05?
>
> No, there's a 4 byte hole which is plugged by this:
>
Right, I missed it, sorry.
Jarek P.
^ permalink raw reply
* Re: [PATCH net-next-2.6] bonding: allow arp_ip_targets to be on a separate vlan from bond device
From: Patrick McHardy @ 2009-12-01 9:42 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: netdev, fubar
In-Reply-To: <20091130205333.GG1639@gospo.rdu.redhat.com>
Andy Gospodarek wrote:
> On Mon, Nov 30, 2009 at 09:22:33PM +0100, Patrick McHardy wrote:
>> Andy Gospodarek wrote:
>>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>>> index e75a2f3..8d8a778 100644
>>> --- a/net/8021q/vlan_core.c
>>> +++ b/net/8021q/vlan_core.c
>>> @@ -14,6 +14,7 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
>>> if (skb_bond_should_drop(skb))
>>> goto drop;
>>>
>>> + skb->skb_iif = skb->dev->ifindex;
>>> __vlan_hwaccel_put_tag(skb, vlan_tci);
>>> skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
>>>
>>> @@ -85,6 +86,7 @@ vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
>>> if (skb_bond_should_drop(skb))
>>> goto drop;
>>>
>>> + skb->skb_iif = skb->dev->ifindex;
>>> __vlan_hwaccel_put_tag(skb, vlan_tci);
>>> skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
>>>
>> How about pulling the skb->iif assignment in netif_receive_skb() up
>> before the vlan_hwaccel_do_receive() call instead?
>
> I'm not sure how that will help. My goal with the two changes in
> vlan_core.c was to capture the input device index before we lost it --
> which is precisely what happens when coming up through a driver like
> bnx2 that supports vlan acceleration:
>
> (NAPI poll)
> bnx2_poll
> bnx2_poll_work
> bnx2_rx_int
> vlan_hwaccel_receive_skb
> __vlan_hwaccel_rx
> (skb->dev set to vlan device)
> netif_receive_skb
>
> In that case, setting skb_iif in netif_receive_skb would be too late.
> The original dev would no longer be known.
Right, I've misread your patch, sorry.
^ permalink raw reply
* Re: [PATCH] sch_htb: ix the deficit overflows
From: Jarek Poplawski @ 2009-12-01 9:39 UTC (permalink / raw)
To: Changli Gao; +Cc: Jamal Hadi Salim, David S. Miller, netdev, Martin Devera
In-Reply-To: <412e6f7f0912010118l19b3a759n925138fbc6dd6f56@mail.gmail.com>
On Tue, Dec 01, 2009 at 05:18:32PM +0800, Changli Gao wrote:
> On Tue, Dec 1, 2009 at 4:43 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On Tue, Dec 01, 2009 at 08:01:51AM +0000, Jarek Poplawski wrote:
> >> On Tue, Dec 01, 2009 at 10:32:26AM +0800, Changli Gao wrote:
> >> > On Mon, Nov 30, 2009 at 7:10 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > ...
> >> > > And this patch is very similar, except ->peek()/dequeue(). Additional
> >> > > lookups are done instead of dequeuing the first found class, which
> >> > > might be quite long in some cases.
> >> >
> >> > If the quantum is set correctly, there isn't difference except of a
> >> > comparison. In the other case, I think some additional CPU cycles are
> >> > better than overflow.
> >>
> >> No, my main point is there _is_ a difference when the quantum is set
> >> correctly. Just these additional lookups.
> >
> > And, again, there are less invasive ways to fix such overflow, like
> >
> > htb_dequeue_tree()
> > {
> > ...
> > if (likely(skb != NULL)) {
> > cl->un.leaf.deficit[level] -= qdisc_pkt_len(skb);
> > if (cl->un.leaf.deficit[level] < 0) {
> > cl->un.leaf.deficit[level] += cl->quantum;
> >
> > + if (cl->un.leaf.deficit[level] < 0)
> > + cl->un.leaf.deficit[level] = -cl->quantum;
>
> How about this:
> if (cl->un.leaf.deficit[level] < 0) {
> cl->un.leaf.deficit[level] = 0;
> if (!(cl->warned & HTB_WARN_QUANTUM_SMALL)) {
> printk(KERN_WARNING
> "HTB: quantum of class %X is small.
> Consider r2q change.\n",
> cl->common.classid);
> cl->warned |= HTB_WARN_QUANTUM_SMALL;
> }
> }
I guess you mean q->warned. Maybe unlikely() would be useful too.
Otherwise, it's acceptable to me, especially when you write you really
hit this problem (not theoretical only ;-)
Jarek P.
^ permalink raw reply
* Re: net 03/05: fib_rules: add oif classification
From: Patrick McHardy @ 2009-12-01 9:32 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: netdev
In-Reply-To: <4B1447DD.1070904@gmail.com>
Jarek Poplawski wrote:
> Patrick McHardy wrote, On 11/30/2009 06:55 PM:
>
>> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
>> index 62bebcb..d4e875a 100644
>> --- a/include/net/fib_rules.h
>> +++ b/include/net/fib_rules.h
>> @@ -11,6 +11,7 @@ struct fib_rule {
>> struct list_head list;
>> atomic_t refcnt;
>> int iifindex;
>> + int oifindex;
>>
>
> Doesn't it "break" the cacheline fix from 01/05?
No, there's a 4 byte hole which is plugged by this:
struct fib_rule {
struct list_head list; /* 0 16 */
atomic_t refcnt; /* 16 4 */
int iifindex; /* 20 4 */
int oifindex; /* 24 4 */
u32 mark; /* 28 4 */
u32 mark_mask; /* 32 4 */
u32 pref; /* 36 4 */
u32 flags; /* 40 4 */
u32 table; /* 44 4 */
u8 action; /* 48 1 */
/* XXX 3 bytes hole, try to pack */
u32 target; /* 52 4 */
struct fib_rule * ctarget; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
char iifname[16]; /* 64 16 */
char oifname[16]; /* 80 16 */
struct rcu_head rcu; /* 96 16 */
struct net * fr_net; /* 112 8 */
/* size: 120, cachelines: 2 */
/* sum members: 117, holes: 1, sum holes: 3 */
/* last cacheline: 56 bytes */
}; /* definitions: 1 */
^ permalink raw reply
* Re: [PATCH] sch_htb: ix the deficit overflows
From: Changli Gao @ 2009-12-01 9:18 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Jamal Hadi Salim, David S. Miller, netdev, Martin Devera
In-Reply-To: <20091201084332.GB6408@ff.dom.local>
On Tue, Dec 1, 2009 at 4:43 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Tue, Dec 01, 2009 at 08:01:51AM +0000, Jarek Poplawski wrote:
>> On Tue, Dec 01, 2009 at 10:32:26AM +0800, Changli Gao wrote:
>> > On Mon, Nov 30, 2009 at 7:10 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> ...
>> > > And this patch is very similar, except ->peek()/dequeue(). Additional
>> > > lookups are done instead of dequeuing the first found class, which
>> > > might be quite long in some cases.
>> >
>> > If the quantum is set correctly, there isn't difference except of a
>> > comparison. In the other case, I think some additional CPU cycles are
>> > better than overflow.
>>
>> No, my main point is there _is_ a difference when the quantum is set
>> correctly. Just these additional lookups.
>
> And, again, there are less invasive ways to fix such overflow, like
>
> htb_dequeue_tree()
> {
> ...
> if (likely(skb != NULL)) {
> cl->un.leaf.deficit[level] -= qdisc_pkt_len(skb);
> if (cl->un.leaf.deficit[level] < 0) {
> cl->un.leaf.deficit[level] += cl->quantum;
>
> + if (cl->un.leaf.deficit[level] < 0)
> + cl->un.leaf.deficit[level] = -cl->quantum;
How about this:
if (cl->un.leaf.deficit[level] < 0) {
cl->un.leaf.deficit[level] = 0;
if (!(cl->warned & HTB_WARN_QUANTUM_SMALL)) {
printk(KERN_WARNING
"HTB: quantum of class %X is small.
Consider r2q change.\n",
cl->common.classid);
cl->warned |= HTB_WARN_QUANTUM_SMALL;
}
}
> + /* or other limit */
>
> htb_next_rb_node((level ? cl->parent->un.inner.ptr : q->
> ptr[0]) + prio);
> }
> /* this used to be after charge_class but this constelation
> gives us slightly better performance */
> if (!cl->un.leaf.q->q.qlen)
> htb_deactivate(q, cl);
> htb_charge_class(q, cl, level, skb);
> }
> return skb;
> }
>
> or even always zeroing cl->un.leaf.deficit[level] on activation or
> deactivation (it's seems unlikely one activity period is enough for
> such an overflow).
>
I found this from http://luxik.cdi.cz/~devik/qos/htb/manual/theory.htm:
HTB uses "real" DRR as defined in [4]. CBQ in Linux uses one where
the quantum can be lower than MTU - it is more generic but it is also
no longer O(1) complexity. It also means that you have to use right
scale for rate->quantum conversion so that all quantums are larger
than MTU.
To keep it O(1) complexity, HTB requires users use the right scale for
quantum. So my first two patches are in the wrong direction.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [Patch] net: fix an array index overflow
From: Cong Wang @ 2009-12-01 8:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <4B14D878.1070802@gmail.com>
Eric Dumazet wrote:
> Amerigo Wang a écrit :
>> Don't use the address of an out-of-boundary element.
>>
>> Maybe this is not harmful at runtime, but it is still
>> good to improve it.
>
> Why ?
>
> for (ptr = start; ptr < end; ptr++) {}
>
> is valid, even if 'end' is 'outside of bounds'
>
> It also works if start == end.
I knew it's valid, that is why I said it's "not harmful".
>
>> Signed-off-by: WANG Cong <amwang@redhat.com>
>> Cc: David S. Miller <davem@davemloft.net>
>>
>> ---
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index 57737b8..2669361 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -1586,7 +1586,7 @@ static int __init inet_init(void)
>> #endif
>>
>> /* Register the socket-side information for inet_create. */
>> - for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
>> + for (r = &inetsw[0]; r <= &inetsw[SOCK_MAX-1]; ++r)
>> INIT_LIST_HEAD(r);
>>
>> for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
>> --
>
> I wonder why you want to 'fix' this loop and let following loop unchanged...
>
> for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
> inet_register_protosw(q);
>
Oh, I didn't catch it.
> If this really hurts your eyes, why not using basic loops ?
Yes, definitely this one is better.
Ack.
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 7d12c6a..476cda7 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1540,8 +1540,7 @@ static struct packet_type ip_packet_type __read_mostly = {
> static int __init inet_init(void)
> {
> struct sk_buff *dummy_skb;
> - struct inet_protosw *q;
> - struct list_head *r;
> + int i;
> int rc = -EINVAL;
>
> BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
> @@ -1584,11 +1583,11 @@ static int __init inet_init(void)
> #endif
>
> /* Register the socket-side information for inet_create. */
> - for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
> - INIT_LIST_HEAD(r);
> + for (i = 0; i < SOCK_MAX; i++)
> + INIT_LIST_HEAD(&inetsw[i]);
>
> - for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
> - inet_register_protosw(q);
> + for (i = 0; i < INETSW_ARRAY_LEN; i++)
> + inet_register_protosw(&inetsw_array[i]);
>
> /*
> * Set the ARP module up
^ permalink raw reply
* Re: [Patch] net: fix an array index overflow
From: Eric Dumazet @ 2009-12-01 8:48 UTC (permalink / raw)
To: Amerigo Wang; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <20091201082901.4678.16688.sendpatchset@localhost.localdomain>
Amerigo Wang a écrit :
> Don't use the address of an out-of-boundary element.
>
> Maybe this is not harmful at runtime, but it is still
> good to improve it.
Why ?
for (ptr = start; ptr < end; ptr++) {}
is valid, even if 'end' is 'outside of bounds'
It also works if start == end.
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
>
> ---
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 57737b8..2669361 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1586,7 +1586,7 @@ static int __init inet_init(void)
> #endif
>
> /* Register the socket-side information for inet_create. */
> - for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
> + for (r = &inetsw[0]; r <= &inetsw[SOCK_MAX-1]; ++r)
> INIT_LIST_HEAD(r);
>
> for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
> --
I wonder why you want to 'fix' this loop and let following loop unchanged...
for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
inet_register_protosw(q);
If this really hurts your eyes, why not using basic loops ?
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 7d12c6a..476cda7 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1540,8 +1540,7 @@ static struct packet_type ip_packet_type __read_mostly = {
static int __init inet_init(void)
{
struct sk_buff *dummy_skb;
- struct inet_protosw *q;
- struct list_head *r;
+ int i;
int rc = -EINVAL;
BUILD_BUG_ON(sizeof(struct inet_skb_parm) > sizeof(dummy_skb->cb));
@@ -1584,11 +1583,11 @@ static int __init inet_init(void)
#endif
/* Register the socket-side information for inet_create. */
- for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
- INIT_LIST_HEAD(r);
+ for (i = 0; i < SOCK_MAX; i++)
+ INIT_LIST_HEAD(&inetsw[i]);
- for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
- inet_register_protosw(q);
+ for (i = 0; i < INETSW_ARRAY_LEN; i++)
+ inet_register_protosw(&inetsw_array[i]);
/*
* Set the ARP module up
^ permalink raw reply related
* Re: [PATCH] sch_htb: ix the deficit overflows
From: Jarek Poplawski @ 2009-12-01 8:43 UTC (permalink / raw)
To: Changli Gao; +Cc: Jamal Hadi Salim, David S. Miller, netdev, Martin Devera
In-Reply-To: <20091201080151.GA6408@ff.dom.local>
On Tue, Dec 01, 2009 at 08:01:51AM +0000, Jarek Poplawski wrote:
> On Tue, Dec 01, 2009 at 10:32:26AM +0800, Changli Gao wrote:
> > On Mon, Nov 30, 2009 at 7:10 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
...
> > > And this patch is very similar, except ->peek()/dequeue(). Additional
> > > lookups are done instead of dequeuing the first found class, which
> > > might be quite long in some cases.
> >
> > If the quantum is set correctly, there isn't difference except of a
> > comparison. In the other case, I think some additional CPU cycles are
> > better than overflow.
>
> No, my main point is there _is_ a difference when the quantum is set
> correctly. Just these additional lookups.
And, again, there are less invasive ways to fix such overflow, like
htb_dequeue_tree()
{
...
if (likely(skb != NULL)) {
cl->un.leaf.deficit[level] -= qdisc_pkt_len(skb);
if (cl->un.leaf.deficit[level] < 0) {
cl->un.leaf.deficit[level] += cl->quantum;
+ if (cl->un.leaf.deficit[level] < 0)
+ cl->un.leaf.deficit[level] = -cl->quantum;
+ /* or other limit */
htb_next_rb_node((level ? cl->parent->un.inner.ptr : q->
ptr[0]) + prio);
}
/* this used to be after charge_class but this constelation
gives us slightly better performance */
if (!cl->un.leaf.q->q.qlen)
htb_deactivate(q, cl);
htb_charge_class(q, cl, level, skb);
}
return skb;
}
or even always zeroing cl->un.leaf.deficit[level] on activation or
deactivation (it's seems unlikely one activity period is enough for
such an overflow).
Jarek P.
^ permalink raw reply
* [Patch] net: fix an array index overflow
From: Amerigo Wang @ 2009-12-01 8:26 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Amerigo Wang, David S. Miller
Don't use the address of an out-of-boundary element.
Maybe this is not harmful at runtime, but it is still
good to improve it.
Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
---
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 57737b8..2669361 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1586,7 +1586,7 @@ static int __init inet_init(void)
#endif
/* Register the socket-side information for inet_create. */
- for (r = &inetsw[0]; r < &inetsw[SOCK_MAX]; ++r)
+ for (r = &inetsw[0]; r <= &inetsw[SOCK_MAX-1]; ++r)
INIT_LIST_HEAD(r);
for (q = inetsw_array; q < &inetsw_array[INETSW_ARRAY_LEN]; ++q)
^ permalink raw reply related
* Re: [PATCH] sch_htb: ix the deficit overflows
From: Jarek Poplawski @ 2009-12-01 8:01 UTC (permalink / raw)
To: Changli Gao; +Cc: Jamal Hadi Salim, David S. Miller, netdev, Martin Devera
In-Reply-To: <412e6f7f0911301832o53e479f0x42345065b0b1616f@mail.gmail.com>
On Tue, Dec 01, 2009 at 10:32:26AM +0800, Changli Gao wrote:
> On Mon, Nov 30, 2009 at 7:10 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On Mon, Nov 30, 2009 at 12:26:33PM +0800, Changli Gao wrote:
> >
> > Users can control this with "r2q" and "quantum", and there is a hint
> > on quantum size in the user's guide.
>
> Yes. But I think most of users will ignore it like me.
In most cases this shouldn't matter. Default r2q/quantum should be
OK for higher rates, and lower ones (< 10pps) are probably controlled
mainly by their state, so even an overflowed deficit doesn't have to
matter (unless your tests show something else ;-).
In other cases those users should see some problems or quantum
warnings, and that's when they should stop ignoring the docs.
>
> >
> >> And
> >> if we use IMQ to shape traffic, the skb will be defragmented by
> >> conntrack, and its size will be larger than MTU.
> >
> > IMQ is a very nice thing, but it's considered broken as well, so it
> > can't be the reason for changing HTB.
>
> I find IMQ is used by many network equipments Linux based. Why not fix
> and integrate it into official Linux?
Even I ;-) don't know exact reasons, but I believe some people here
know better.
>
> > And this patch is very similar, except ->peek()/dequeue(). Additional
> > lookups are done instead of dequeuing the first found class, which
> > might be quite long in some cases.
>
> If the quantum is set correctly, there isn't difference except of a
> comparison. In the other case, I think some additional CPU cycles are
> better than overflow.
No, my main point is there _is_ a difference when the quantum is set
correctly. Just these additional lookups.
>
> >
> > It's not acceptable to me mainly because the real change done by this
> > patch is different than you describe: preventing an overflow might be
> > simple. You change the way DRR is implemented here, and even if it's
> > right, it should be written explicitly and proved with tests results.
> >
>
> This way is used by CBQ.
HTB is different by design:
http://luxik.cdi.cz/~devik/qos/htb/manual/theory.htm
Regards,
Jarek P.
^ permalink raw reply
* [PATCH] ip: update the description of rp_filter in ip-sysctl.txt
From: Shan Wei @ 2009-12-01 7:04 UTC (permalink / raw)
To: David Miller, shemminger; +Cc: netdev@vger.kernel.org, rdunlap
The commit 27fed4175acf81ddd91d9a4ee2fd298981f60295
（ip: fix logic of reverse path filter sysctl）has changed the logic of rp_filter.
The document about rp_filter is out of date. Now, setting conf/all/rp_filte with 0
can also enable source validation.
Update the document according to the commit.
Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
Documentation/networking/ip-sysctl.txt | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index fbe427a..5dcc067 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -738,8 +738,8 @@ rp_filter - INTEGER
to prevent IP spoofing from DDos attacks. If using asymmetric routing
or other complicated routing, then loose mode is recommended.
- conf/all/rp_filter must also be set to non-zero to do source validation
- on the interface
+ The max value from conf/{all,interface}/rp_filter is used
+ when doing source validation on the {interface}.
Default value is 0. Note that some distributions enable it
in startup scripts.
--
1.6.3.3
^ permalink raw reply related
* Re: soft lockup in inet_csk_get_port
From: Eric Dumazet @ 2009-12-01 6:10 UTC (permalink / raw)
To: kapil dakhane; +Cc: netdev, netfilter
In-Reply-To: <99d458640911301802i4bde20f4wa314668d543e3170@mail.gmail.com>
kapil dakhane a écrit :
> Hello,
>
> I am trying to analyze the capacity of linux network stack on x6270
> which has 16 Hyper threads on two 8-core Intel(r) Xeon(r) CPU. I see
> that at around 150000 simultaneous connections, after around 1.6 gbps,
> a cpu get stuck in an infinite loop in inet_csk_bind_conflict, then
> other cpus get locked up doing spin_lock. Before the lockup cpu usage
> was around 25%. It appears to be a bug, unless I am hitting some kind
> of resource limit. It would be good if someone familiar with network
> code would confirm this, or point me in the right direction.
>
> Important details are:
>
> I am using kernel version 2.6.31.4 recompiled with TPROXY related
> options: NF_CONNTRACK, NETFILTER_TPROXY, NETFILTER_XT_MATCH_SOCKET,
> NETFILTER_XT_TARGET_TPROXY.
>
>
> I have enabled transparent capture and transparent forward using
> iptables and ip rules. I have 10 instances of a single threaded user
> space bits-forwarding-proxy (fast), each bound to different
> hyper-threads (CPUs). Rest 6 CPUs are dedicated to interrupt
> processing, each handling interrupts from six different network cards.
> TCP flow from a 4-tuple always get handled by the same proxy process,
> interrupt thread, and network card. In this way, network traffic is
> segregated as much as possible to achieve high degree of parallelism.
>
> First /var/log/message entry shows CPU#7 is stuck in inet_csk_bind_conflict
>
> Nov 17 23:02:04 cap-x6270-01 kernel: BUG: soft lockup - CPU#7 stuck
> for 61s! [fast:20701]
> Nov 17 23:02:04 cap-x6270-01 kernel: Modules linked in: xt_TPROXY
> xt_MARK xt_socket nf_defrag_ipv4 nf_tproxy_core iptable_mangle ipv6
> autofs4 hidp rfcomm l2cap bluetooth rfkill sunrpc 8021q xt_state
> nf_conntrack xt_tcpudp iptable_filter ip_tables x_tables
> cpufreq_ondemand acpi_cpufreq freq_table dm_multipath scsi_dh video
> output sbs sbshc battery acpi_memhotplug ac parport_pc lp parport
> joydev sg rtc_cmos serio_raw rtc_core button igb rtc_lib niu i2c_i801
> i2c_core pcspkr dm_snapshot dm_zero dm_mirror dm_region_hash dm_log
> dm_mod usb_storage ahci libata shpchp aacraid sd_mod scsi_mod ext3 jbd
> uhci_hcd ohci_hcd ehci_hcd [last unloaded: microcode]
> Nov 17 23:02:04 cap-x6270-01 kernel: CPU 7:
> Nov 17 23:02:04 cap-x6270-01 kernel: Modules linked in: ...
> Nov 17 23:02:04 cap-x6270-01 kernel: Pid: 20701, comm: fast Not
> tainted 2.6.31.4 #1 SUN BLADE X6270 SERVER MODULE
> Nov 17 23:02:04 cap-x6270-01 kernel: RIP: 0010:[<ffffffff81285c53>]
> [<ffffffff81285c53>] inet_csk_bind_conflict+0x99/0xa6
> Nov 17 23:02:04 cap-x6270-01 kernel: RSP: 0018:ffff88095ac7fe30
> EFLAGS: 00000202
> Nov 17 23:02:04 cap-x6270-01 kernel: RAX: 000000003c0ba8c0 RBX:
> ffff88097b14bae0 RCX: ffff8804b3d57820
> Nov 17 23:02:04 cap-x6270-01 kernel: RDX: ffff8804b3d57800 RSI:
> 0000000000000000 RDI: ffff880940421840
> Nov 17 23:02:04 cap-x6270-01 kernel: RBP: ffffffff8100c42e R08:
> 000000002d01960c R09: ffff880909832ea0
> Nov 17 23:02:04 cap-x6270-01 kernel: R10: 00007fffc5d92700 R11:
> ffff880940421840 R12: 0000000000000001
> Nov 17 23:02:04 cap-x6270-01 kernel: R13: ffff88097c5e9400 R14:
> ffffffff810b7a92 R15: 0000000000000001
> Nov 17 23:02:04 cap-x6270-01 kernel: FS: 00007f20a08416e0(0000)
> GS:ffffc90000e00000(0000) knlGS:0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
> 0000000080050033
> Nov 17 23:02:04 cap-x6270-01 kernel: CR2: 00000000081df408 CR3:
> 000000049ac01000 CR4: 00000000000006e0
> Nov 17 23:02:04 cap-x6270-01 kernel: DR0: 0000000000000000 DR1:
> 0000000000000000 DR2: 0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: DR3: 0000000000000000 DR6:
> 00000000ffff0ff0 DR7: 0000000000000400
> Nov 17 23:02:04 cap-x6270-01 kernel: Call Trace:
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff812859e4>] ?
> inet_csk_get_port+0x1b2/0x29e
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff812a1596>] ?
> inet_bind+0x10c/0x1b7
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8124ef53>] ? sys_bind+0x6e/0x9e
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8106de14>] ?
> audit_syscall_entry+0x1a4/0x1cf
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8100b92b>] ?
> system_call_fastpath+0x16/0x1b
>
> While other CPUs get stuck doing _spin_lock:
>
>
> Nov 17 23:02:04 cap-x6270-01 kernel: BUG: soft lockup - CPU#15 stuck
> for 61s! [fast:20702]
> Nov 17 23:02:04 cap-x6270-01 kernel: Modules linked in: ...
> Nov 17 23:02:04 cap-x6270-01 kernel: CPU 15:
> Nov 17 23:02:04 cap-x6270-01 kernel: Modules linked in: ...
> Nov 17 23:02:04 cap-x6270-01 kernel: Pid: 20702, comm: fast Not
> tainted 2.6.31.4 #1 SUN BLADE X6270 SERVER MODULE
> Nov 17 23:02:04 cap-x6270-01 kernel: RIP: 0010:[<ffffffff812dedff>]
> [<ffffffff812dedff>] _spin_lock+0x10/0x15
> Nov 17 23:02:04 cap-x6270-01 kernel: RSP: 0018:ffff88090ecabe30
> EFLAGS: 00000297
> Nov 17 23:02:04 cap-x6270-01 kernel: RAX: 0000000000000504 RBX:
> 00000000ffffffea RCX: 0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: RDX: 00000000000000a2 RSI:
> 0000000000000fa2 RDI: ffffc90019f82a20
> Nov 17 23:02:04 cap-x6270-01 kernel: RBP: ffffffff8100c42e R08:
> ffff880905d89840 R09: 0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: R10: 00007fffd8db4574 R11:
> ffff880905d89840 R12: ffffffff812509d4
> Nov 17 23:02:04 cap-x6270-01 kernel: R13: ffff88094c0bd280 R14:
> 0000000000000246 R15: 0000000000000001
> Nov 17 23:02:04 cap-x6270-01 kernel: FS: 00007f0deff696e0(0000)
> GS:ffffc90001e00000(0000) knlGS:0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
> 0000000080050033
> Nov 17 23:02:04 cap-x6270-01 kernel: CR2: 0000000007ec2258 CR3:
> 00000009494b1000 CR4: 00000000000006e0
> Nov 17 23:02:04 cap-x6270-01 kernel: DR0: 0000000000000000 DR1:
> 0000000000000000 DR2: 0000000000000000
> Nov 17 23:02:04 cap-x6270-01 kernel: DR3: 0000000000000000 DR6:
> 00000000ffff0ff0 DR7: 0000000000000400
> Nov 17 23:02:04 cap-x6270-01 kernel: Call Trace:
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff81285989>] ?
> inet_csk_get_port+0x157/0x29e
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff812a1596>] ?
> inet_bind+0x10c/0x1b7
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8124ef53>] ? sys_bind+0x6e/0x9e
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8106de14>] ?
> audit_syscall_entry+0x1a4/0x1cf
> Nov 17 23:02:04 cap-x6270-01 kernel: [<ffffffff8100b92b>] ?
> system_call_fastpath+0x16/0x1b
> --
Hmm, I did an one hour audit and could not yet find the bug.
Is it a reproductible error, and any chance I can have a snapshot of
"netstat -atn" before the lockup ? (maybe privately, since it might be
too big for netdev)
What is the 'fast' program, is it freely available somewhere ?
Thanks
^ permalink raw reply
* Re: [PATCH net-next-2.6] ieee802154: merge cleanup
From: Eric Dumazet @ 2009-12-01 6:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091130.215501.267396660.davem@davemloft.net>
David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 01 Dec 2009 04:59:20 +0100
>
>> A small cleanup after last net-2.6 merge into net-next-2.6
>>
>> As we are going to free skb, no need to set skb->skb_iif or skb->dev
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> -ECORRUPTEDPATCH
>
> Are you really sure you want to keep using Thunderbird for
> your email? :-)
Sorry, my bad, sending patches at 4h30 am is error prone,
I did a stupid copy/paste from a wrong window.
[PATCH net-next-2.6] ieee802154: merge cleanup
A small cleanup after last net-2.6 merge into net-next-2.6
As we are going to free skb, no need to set skb->skb_iif or skb->dev
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
drivers/ieee802154/fakehard.c | 3 ---
1 files changed, 3 deletions(-)
diff --git a/drivers/ieee802154/fakehard.c b/drivers/ieee802154/fakehard.c
index 5f67540..d9d0e13 100644
--- a/drivers/ieee802154/fakehard.c
+++ b/drivers/ieee802154/fakehard.c
@@ -282,9 +282,6 @@ static int ieee802154_fake_close(struct net_device *dev)
static netdev_tx_t ieee802154_fake_xmit(struct sk_buff *skb,
struct net_device *dev)
{
- skb->skb_iif = dev->ifindex;
- skb->dev = dev;
-
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
^ permalink raw reply related
* Re: [PATCH net-next-2.6] ieee802154: merge cleanup
From: David Miller @ 2009-12-01 5:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4B149498.4030307@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Dec 2009 04:59:20 +0100
> A small cleanup after last net-2.6 merge into net-next-2.6
>
> As we are going to free skb, no need to set skb->skb_iif or skb->dev
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
-ECORRUPTEDPATCH
Are you really sure you want to keep using Thunderbird for
your email? :-)
^ permalink raw reply
* RE: TSOv6 broken in atl1e
From: Jie Yang @ 2009-12-01 5:17 UTC (permalink / raw)
To: Ben Hutchings; +Cc: 558426@bugs.debian.org, netdev
In-Reply-To: <1259637579.3709.196.camel@localhost>
On Tuesday, December 01, 2009 11:20 AM
Ben Hutchings <ben@decadent.org.uk>
>
> I received a bug report <http://bugs.debian.org/558426> that
> shows atl1e corrupting IPv6 packets. I have reproduced this
> on an Eee PC 901 and found that it is linked to TSO. The
> most obvious thing wrong with the driver code is that it
> calculates the super-packet length incorrectly.
> However, fixing that:
>
> --- a/drivers/net/atl1e/atl1e_main.c
> +++ b/drivers/net/atl1e/atl1e_main.c
> @@ -1667,6 +1667,7 @@ static int atl1e_tso_csum(struct
> atl1e_adapter *adapter,
>
> if (offload_type & SKB_GSO_TCPV6) {
> real_len = (((unsigned char
> *)ipv6_hdr(skb) - skb->data)
> + + sizeof(struct ipv6hdr)
> +
> ntohs(ipv6_hdr(skb)->payload_len));
> if (real_len < skb->len)
> pskb_trim(skb, real_len);
> --- END ---
>
> does not solve the problem. Presumably this function is not
> constructing correct DMA descriptors for TSOv6.
>
> Please fix this, or I will submit a patch to remove this
> feature from the driver.
>
> Ben.
>
ok, I will try to reproduce it.
Best wishes
jie
^ permalink raw reply
* Re: NULL pointer dereference at 2.6.32-rc8:net/ipv4/ip_fragment.c:566
From: David Miller @ 2009-12-01 5:01 UTC (permalink / raw)
To: herbert; +Cc: firefighterblu3, linux-kernel, david, netdev
In-Reply-To: <20091201043759.GA22655@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 1 Dec 2009 12:37:59 +0800
> David Ford <firefighterblu3@gmail.com> wrote:
>>
>> hmm. that shows a patch of code which is just prior to ip_defrag(), i.e.:
>>
>> 565 out_fail:
>> 566 IP_INC_STATS_BH(dev_net(dev), IPSTATS_MIB_REASMFAILS);
>
> Looks like a namespace problem.
>
> Please open a bug report on bugzilla.kernel.org to ensure that
> this is tracked.
Herbert, David posted a patch to fix this which went into my tree the
other day and now has reached Linus's tree as well.
^ 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