* [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Eric Dumazet
In-Reply-To: <20101115163809.215552143@vyatta.com>
[-- Attachment #1: bridge-hook-typedef.patch --]
[-- Type: text/plain, Size: 3069 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Add br_should_route_hook_t typedef, this is the only way we can
get a clean RCU implementation for function pointer.
Move route_hook to location where it is used.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
include/linux/if_bridge.h | 4 +++-
net/bridge/br.c | 4 ----
net/bridge/br_input.c | 10 +++++++---
net/bridge/netfilter/ebtable_broute.c | 3 ++-
4 files changed, 12 insertions(+), 9 deletions(-)
--- a/net/bridge/br.c 2010-11-14 11:18:54.048692005 -0800
+++ b/net/bridge/br.c 2010-11-14 11:19:47.347027185 -0800
@@ -22,8 +22,6 @@
#include "br_private.h"
-int (*br_should_route_hook)(struct sk_buff *skb);
-
static const struct stp_proto br_stp_proto = {
.rcv = br_stp_rcv,
};
@@ -102,8 +100,6 @@ static void __exit br_deinit(void)
br_fdb_fini();
}
-EXPORT_SYMBOL(br_should_route_hook);
-
module_init(br_init)
module_exit(br_deinit)
MODULE_LICENSE("GPL");
--- a/net/bridge/br_input.c 2010-11-14 11:18:54.048692005 -0800
+++ b/net/bridge/br_input.c 2010-11-14 11:41:40.558700481 -0800
@@ -21,6 +21,10 @@
/* Bridge group multicast address 802.1d (pg 51). */
const u8 br_group_address[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
+/* Hook for brouter */
+br_should_route_hook_t __rcu *br_should_route_hook __read_mostly;
+EXPORT_SYMBOL(br_should_route_hook);
+
static int br_pass_frame_up(struct sk_buff *skb)
{
struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev;
@@ -139,7 +143,7 @@ struct sk_buff *br_handle_frame(struct s
{
struct net_bridge_port *p;
const unsigned char *dest = eth_hdr(skb)->h_dest;
- int (*rhook)(struct sk_buff *skb);
+ br_should_route_hook_t *rhook;
if (unlikely(skb->pkt_type == PACKET_LOOPBACK))
return skb;
@@ -173,8 +177,8 @@ forward:
switch (p->state) {
case BR_STATE_FORWARDING:
rhook = rcu_dereference(br_should_route_hook);
- if (rhook != NULL) {
- if (rhook(skb))
+ if (rhook) {
+ if ((*rhook)(skb))
return skb;
dest = eth_hdr(skb)->h_dest;
}
--- a/include/linux/if_bridge.h 2010-11-14 11:18:54.048692005 -0800
+++ b/include/linux/if_bridge.h 2010-11-14 11:19:47.351028008 -0800
@@ -102,7 +102,9 @@ struct __fdb_entry {
#include <linux/netdevice.h>
extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *));
-extern int (*br_should_route_hook)(struct sk_buff *skb);
+
+typedef int (*br_should_route_hook_t)(struct sk_buff *skb);
+extern br_should_route_hook_t __rcu *br_should_route_hook;
#endif
--- a/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:20:39.745149494 -0800
+++ b/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:21:01.020917528 -0800
@@ -87,7 +87,8 @@ static int __init ebtable_broute_init(vo
if (ret < 0)
return ret;
/* see br_input.c */
- rcu_assign_pointer(br_should_route_hook, ebt_broute);
+ rcu_assign_pointer(br_should_route_hook,
+ (br_should_route_hook_t *)ebt_broute);
return 0;
}
^ permalink raw reply
* [PATCH 1/5] bridge: add RCU annotation to bridge multicast table
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Eric Dumazet
In-Reply-To: <20101115163809.215552143@vyatta.com>
[-- Attachment #1: bridge-mlock-rcu.patch --]
[-- Type: text/plain, Size: 10232 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Add modern __rcu annotatations to bridge multicast table.
Use newer hlist macros to avoid direct access to hlist internals.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
v2. Fix hlist_next_rcu call
net/bridge/br_forward.c | 4 +-
net/bridge/br_multicast.c | 78 ++++++++++++++++++++++++++++++----------------
net/bridge/br_private.h | 6 +--
3 files changed, 56 insertions(+), 32 deletions(-)
--- a/net/bridge/br_multicast.c 2010-11-14 12:36:30.383348571 -0800
+++ b/net/bridge/br_multicast.c 2010-11-14 12:36:37.084167303 -0800
@@ -33,6 +33,9 @@
#include "br_private.h"
+#define mlock_dereference(X, br) \
+ rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock))
+
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static inline int ipv6_is_local_multicast(const struct in6_addr *addr)
{
@@ -135,7 +138,7 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br,
struct sk_buff *skb)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb = rcu_dereference(br->mdb);
struct br_ip ip;
if (br->multicast_disabled)
@@ -235,7 +238,8 @@ static void br_multicast_group_expired(u
if (mp->ports)
goto out;
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
+
hlist_del_rcu(&mp->hlist[mdb->ver]);
mdb->size--;
@@ -249,16 +253,20 @@ out:
static void br_multicast_del_pg(struct net_bridge *br,
struct net_bridge_port_group *pg)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
+
+ mdb = mlock_dereference(br->mdb, br);
mp = br_mdb_ip_get(mdb, &pg->addr);
if (WARN_ON(!mp))
return;
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (p != pg)
continue;
@@ -294,10 +302,10 @@ out:
spin_unlock(&br->multicast_lock);
}
-static int br_mdb_rehash(struct net_bridge_mdb_htable **mdbp, int max,
+static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
int elasticity)
{
- struct net_bridge_mdb_htable *old = *mdbp;
+ struct net_bridge_mdb_htable *old = rcu_dereference_protected(*mdbp, 1);
struct net_bridge_mdb_htable *mdb;
int err;
@@ -569,7 +577,7 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge *br, struct net_bridge_port *port,
struct br_ip *group, int hash)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
struct hlist_node *p;
unsigned count = 0;
@@ -577,6 +585,7 @@ static struct net_bridge_mdb_entry *br_m
int elasticity;
int err;
+ mdb = rcu_dereference_protected(br->mdb, 1);
hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) {
count++;
if (unlikely(br_ip_equal(group, &mp->addr)))
@@ -642,10 +651,11 @@ static struct net_bridge_mdb_entry *br_m
struct net_bridge *br, struct net_bridge_port *port,
struct br_ip *group)
{
- struct net_bridge_mdb_htable *mdb = br->mdb;
+ struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
int hash;
+ mdb = rcu_dereference_protected(br->mdb, 1);
if (!mdb) {
if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0))
return NULL;
@@ -660,7 +670,7 @@ static struct net_bridge_mdb_entry *br_m
case -EAGAIN:
rehash:
- mdb = br->mdb;
+ mdb = rcu_dereference_protected(br->mdb, 1);
hash = br_ip_hash(mdb, group);
break;
@@ -692,7 +702,7 @@ static int br_multicast_add_group(struct
{
struct net_bridge_mdb_entry *mp;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
unsigned long now = jiffies;
int err;
@@ -712,7 +722,9 @@ static int br_multicast_add_group(struct
goto out;
}
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (p->port == port)
goto found;
if ((unsigned long)p->port < (unsigned long)port)
@@ -1106,7 +1118,7 @@ static int br_ip4_multicast_query(struct
struct net_bridge_mdb_entry *mp;
struct igmpv3_query *ih3;
struct net_bridge_port_group *p;
- struct net_bridge_port_group **pp;
+ struct net_bridge_port_group __rcu **pp;
unsigned long max_delay;
unsigned long now = jiffies;
__be32 group;
@@ -1145,7 +1157,7 @@ static int br_ip4_multicast_query(struct
if (!group)
goto out;
- mp = br_mdb_ip4_get(br->mdb, group);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group);
if (!mp)
goto out;
@@ -1157,7 +1169,9 @@ static int br_ip4_multicast_query(struct
try_to_del_timer_sync(&mp->timer) >= 0))
mod_timer(&mp->timer, now + max_delay);
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (timer_pending(&p->timer) ?
time_after(p->timer.expires, now + max_delay) :
try_to_del_timer_sync(&p->timer) >= 0)
@@ -1178,7 +1192,8 @@ static int br_ip6_multicast_query(struct
struct mld_msg *mld = (struct mld_msg *) icmp6_hdr(skb);
struct net_bridge_mdb_entry *mp;
struct mld2_query *mld2q;
- struct net_bridge_port_group *p, **pp;
+ struct net_bridge_port_group *p;
+ struct net_bridge_port_group __rcu **pp;
unsigned long max_delay;
unsigned long now = jiffies;
struct in6_addr *group = NULL;
@@ -1214,7 +1229,7 @@ static int br_ip6_multicast_query(struct
if (!group)
goto out;
- mp = br_mdb_ip6_get(br->mdb, group);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group);
if (!mp)
goto out;
@@ -1225,7 +1240,9 @@ static int br_ip6_multicast_query(struct
try_to_del_timer_sync(&mp->timer) >= 0))
mod_timer(&mp->timer, now + max_delay);
- for (pp = &mp->ports; (p = *pp); pp = &p->next) {
+ for (pp = &mp->ports;
+ (p = mlock_dereference(*pp, br)) != NULL;
+ pp = &p->next) {
if (timer_pending(&p->timer) ?
time_after(p->timer.expires, now + max_delay) :
try_to_del_timer_sync(&p->timer) >= 0)
@@ -1254,7 +1271,7 @@ static void br_multicast_leave_group(str
timer_pending(&br->multicast_querier_timer))
goto out;
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
mp = br_mdb_ip_get(mdb, group);
if (!mp)
goto out;
@@ -1277,7 +1294,9 @@ static void br_multicast_leave_group(str
goto out;
}
- for (p = mp->ports; p; p = p->next) {
+ for (p = mlock_dereference(mp->ports, br);
+ p != NULL;
+ p = mlock_dereference(p->next, br)) {
if (p->port != port)
continue;
@@ -1625,7 +1644,7 @@ void br_multicast_stop(struct net_bridge
del_timer_sync(&br->multicast_query_timer);
spin_lock_bh(&br->multicast_lock);
- mdb = br->mdb;
+ mdb = mlock_dereference(br->mdb, br);
if (!mdb)
goto out;
@@ -1729,6 +1748,7 @@ int br_multicast_toggle(struct net_bridg
{
struct net_bridge_port *port;
int err = 0;
+ struct net_bridge_mdb_htable *mdb;
spin_lock(&br->multicast_lock);
if (br->multicast_disabled == !val)
@@ -1741,15 +1761,16 @@ int br_multicast_toggle(struct net_bridg
if (!netif_running(br->dev))
goto unlock;
- if (br->mdb) {
- if (br->mdb->old) {
+ mdb = mlock_dereference(br->mdb, br);
+ if (mdb) {
+ if (mdb->old) {
err = -EEXIST;
rollback:
br->multicast_disabled = !!val;
goto unlock;
}
- err = br_mdb_rehash(&br->mdb, br->mdb->max,
+ err = br_mdb_rehash(&br->mdb, mdb->max,
br->hash_elasticity);
if (err)
goto rollback;
@@ -1774,6 +1795,7 @@ int br_multicast_set_hash_max(struct net
{
int err = -ENOENT;
u32 old;
+ struct net_bridge_mdb_htable *mdb;
spin_lock(&br->multicast_lock);
if (!netif_running(br->dev))
@@ -1782,7 +1804,9 @@ int br_multicast_set_hash_max(struct net
err = -EINVAL;
if (!is_power_of_2(val))
goto unlock;
- if (br->mdb && val < br->mdb->size)
+
+ mdb = mlock_dereference(br->mdb, br);
+ if (mdb && val < mdb->size)
goto unlock;
err = 0;
@@ -1790,8 +1814,8 @@ int br_multicast_set_hash_max(struct net
old = br->hash_max;
br->hash_max = val;
- if (br->mdb) {
- if (br->mdb->old) {
+ if (mdb) {
+ if (mdb->old) {
err = -EEXIST;
rollback:
br->hash_max = old;
--- a/net/bridge/br_private.h 2010-11-14 12:36:30.399350527 -0800
+++ b/net/bridge/br_private.h 2010-11-14 12:44:07.257410977 -0800
@@ -72,7 +72,7 @@ struct net_bridge_fdb_entry
struct net_bridge_port_group {
struct net_bridge_port *port;
- struct net_bridge_port_group *next;
+ struct net_bridge_port_group __rcu *next;
struct hlist_node mglist;
struct rcu_head rcu;
struct timer_list timer;
@@ -86,7 +86,7 @@ struct net_bridge_mdb_entry
struct hlist_node hlist[2];
struct hlist_node mglist;
struct net_bridge *br;
- struct net_bridge_port_group *ports;
+ struct net_bridge_port_group __rcu *ports;
struct rcu_head rcu;
struct timer_list timer;
struct timer_list query_timer;
@@ -227,7 +227,7 @@ struct net_bridge
unsigned long multicast_startup_query_interval;
spinlock_t multicast_lock;
- struct net_bridge_mdb_htable *mdb;
+ struct net_bridge_mdb_htable __rcu *mdb;
struct hlist_head router_list;
struct hlist_head mglist;
--- a/net/bridge/br_forward.c 2010-11-14 12:36:47.833478598 -0800
+++ b/net/bridge/br_forward.c 2010-11-14 12:42:22.001208297 -0800
@@ -223,7 +223,7 @@ static void br_multicast_flood(struct ne
struct net_bridge_port_group *p;
struct hlist_node *rp;
- rp = rcu_dereference(br->router_list.first);
+ rp = rcu_dereference(hlist_first_rcu(&br->router_list));
p = mdst ? rcu_dereference(mdst->ports) : NULL;
while (p || rp) {
struct net_bridge_port *port, *lport, *rport;
@@ -242,7 +242,7 @@ static void br_multicast_flood(struct ne
if ((unsigned long)lport >= (unsigned long)port)
p = rcu_dereference(p->next);
if ((unsigned long)rport >= (unsigned long)port)
- rp = rcu_dereference(rp->next);
+ rp = rcu_dereference(hlist_next_rcu(rp));
}
if (!prev)
^ permalink raw reply
* [PATCH 5/5] bridge: add RCU annotations to bridge port lookup
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Eric Dumazet
In-Reply-To: <20101115163809.215552143@vyatta.com>
[-- Attachment #1: bridge-port_get_rtnl.patch --]
[-- Type: text/plain, Size: 2300 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
br_port_get() renamed to br_port_get_rtnl() to make clear RTNL is held.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_if.c | 2 +-
net/bridge/br_netlink.c | 4 ++--
net/bridge/br_notify.c | 4 ++--
net/bridge/br_private.h | 9 +++++----
4 files changed, 10 insertions(+), 9 deletions(-)
--- a/net/bridge/br_netlink.c 2010-11-15 08:25:50.049646467 -0800
+++ b/net/bridge/br_netlink.c 2010-11-15 08:27:57.772866001 -0800
@@ -119,7 +119,7 @@ static int br_dump_ifinfo(struct sk_buff
idx = 0;
for_each_netdev(net, dev) {
- struct net_bridge_port *port = br_port_get(dev);
+ struct net_bridge_port *port = br_port_get_rtnl(dev);
/* not a bridge port */
if (!port || idx < cb->args[0])
@@ -171,7 +171,7 @@ static int br_rtm_setlink(struct sk_buff
if (!dev)
return -ENODEV;
- p = br_port_get(dev);
+ p = br_port_get_rtnl(dev);
if (!p)
return -EINVAL;
--- a/net/bridge/br_private.h 2010-11-15 08:27:46.747612478 -0800
+++ b/net/bridge/br_private.h 2010-11-15 08:27:57.776866451 -0800
@@ -159,9 +159,10 @@ static inline struct net_bridge_port *br
return br_port_exists(dev) ? port : NULL;
}
-static inline struct net_bridge_port *br_port_get(struct net_device *dev)
+static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
{
- return br_port_exists(dev) ? dev->rx_handler_data : NULL;
+ return br_port_exists(dev) ?
+ rtnl_dereference(dev->rx_handler_data) : NULL;
}
struct br_cpu_netstats {
--- a/net/bridge/br_if.c 2010-11-15 08:25:50.049646467 -0800
+++ b/net/bridge/br_if.c 2010-11-15 08:27:57.776866451 -0800
@@ -475,7 +475,7 @@ int br_del_if(struct net_bridge *br, str
{
struct net_bridge_port *p;
- p = br_port_get(dev);
+ p = br_port_get_rtnl(dev);
if (!p || p->br != br)
return -EINVAL;
--- a/net/bridge/br_notify.c 2010-11-15 08:25:50.045645976 -0800
+++ b/net/bridge/br_notify.c 2010-11-15 08:27:57.780866901 -0800
@@ -37,10 +37,10 @@ static int br_device_event(struct notifi
int err;
/* not a port of a bridge */
- if (!br_port_exists(dev))
+ p = br_port_get_rtnl(dev);
+ if (!p)
return NOTIFY_DONE;
- p = br_port_get(dev);
br = p->br;
switch (event) {
^ permalink raw reply
* [PATCH 0/5] bridge RCU patches (rev2)
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
^ permalink raw reply
* [PATCH 4/5] bridge: fix RCU races with bridge port
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20101115163809.215552143@vyatta.com>
[-- Attachment #1: br-port-rcu-race.patch --]
[-- Type: text/plain, Size: 6605 bytes --]
The macro br_port_exists() is not enough protection when only
RCU is being used. There is a tiny race where other CPU has cleared port
handler hook, but is bridge port flag might still be set.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_fdb.c | 15 +++++++++------
net/bridge/br_if.c | 5 +----
net/bridge/br_netfilter.c | 13 +++++++------
net/bridge/br_netlink.c | 10 ++++++----
net/bridge/br_notify.c | 2 +-
net/bridge/br_private.h | 16 +++++++++++++---
net/bridge/br_stp_bpdu.c | 8 ++++----
net/bridge/netfilter/ebtables.c | 11 +++++------
8 files changed, 46 insertions(+), 34 deletions(-)
--- a/net/bridge/br_netfilter.c 2010-11-15 08:24:41.696606136 -0800
+++ b/net/bridge/br_netfilter.c 2010-11-15 08:25:50.037644994 -0800
@@ -131,17 +131,18 @@ void br_netfilter_rtable_init(struct net
static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
- return &br_port_get_rcu(dev)->br->fake_rtable;
+ struct net_bridge_port *port;
+
+ port = br_port_get_rcu(dev);
+ return port ? &port->br->fake_rtable : NULL;
}
static inline struct net_device *bridge_parent(const struct net_device *dev)
{
- if (!br_port_exists(dev))
- return NULL;
+ struct net_bridge_port *port;
- return br_port_get_rcu(dev)->br->dev;
+ port = br_port_get_rcu(dev);
+ return port ? port->br->dev : NULL;
}
static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
--- a/net/bridge/br_stp_bpdu.c 2010-11-15 08:24:41.696606136 -0800
+++ b/net/bridge/br_stp_bpdu.c 2010-11-15 08:25:50.037644994 -0800
@@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto *
struct net_bridge *br;
const unsigned char *buf;
- if (!br_port_exists(dev))
- goto err;
- p = br_port_get_rcu(dev);
-
if (!pskb_may_pull(skb, 4))
goto err;
@@ -153,6 +149,10 @@ void br_stp_rcv(const struct stp_proto *
if (buf[0] != 0 || buf[1] != 0 || buf[2] != 0)
goto err;
+ p = br_port_get_rcu(dev);
+ if (!p)
+ goto err;
+
br = p->br;
spin_lock(&br->lock);
--- a/net/bridge/netfilter/ebtables.c 2010-11-15 08:24:41.696606136 -0800
+++ b/net/bridge/netfilter/ebtables.c 2010-11-15 08:25:50.041645485 -0800
@@ -128,6 +128,7 @@ ebt_basic_match(const struct ebt_entry *
const struct net_device *in, const struct net_device *out)
{
const struct ethhdr *h = eth_hdr(skb);
+ const struct net_bridge_port *p;
__be16 ethproto;
int verdict, i;
@@ -148,13 +149,11 @@ ebt_basic_match(const struct ebt_entry *
if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT))
return 1;
/* rcu_read_lock()ed by nf_hook_slow */
- if (in && br_port_exists(in) &&
- FWINV2(ebt_dev_check(e->logical_in, br_port_get_rcu(in)->br->dev),
- EBT_ILOGICALIN))
+ if (in && (p = br_port_get_rcu(in)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_in, p->br->dev), EBT_ILOGICALIN))
return 1;
- if (out && br_port_exists(out) &&
- FWINV2(ebt_dev_check(e->logical_out, br_port_get_rcu(out)->br->dev),
- EBT_ILOGICALOUT))
+ if (out && (p = br_port_get_rcu(out)) != NULL &&
+ FWINV2(ebt_dev_check(e->logical_out, p->br->dev), EBT_ILOGICALOUT))
return 1;
if (e->bitmask & EBT_SOURCEMAC) {
--- a/net/bridge/br_fdb.c 2010-11-15 08:20:20.020385965 -0800
+++ b/net/bridge/br_fdb.c 2010-11-15 08:25:50.041645485 -0800
@@ -238,15 +238,18 @@ struct net_bridge_fdb_entry *__br_fdb_ge
int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
{
struct net_bridge_fdb_entry *fdb;
+ struct net_bridge_port *port;
int ret;
- if (!br_port_exists(dev))
- return 0;
-
rcu_read_lock();
- fdb = __br_fdb_get(br_port_get_rcu(dev)->br, addr);
- ret = fdb && fdb->dst->dev != dev &&
- fdb->dst->state == BR_STATE_FORWARDING;
+ port = br_port_get_rcu(dev);
+ if (!port)
+ ret = 0;
+ else {
+ fdb = __br_fdb_get(port->br, addr);
+ ret = fdb && fdb->dst->dev != dev &&
+ fdb->dst->state == BR_STATE_FORWARDING;
+ }
rcu_read_unlock();
return ret;
--- a/net/bridge/br_notify.c 2010-11-15 08:24:41.696606136 -0800
+++ b/net/bridge/br_notify.c 2010-11-15 08:25:50.045645976 -0800
@@ -32,7 +32,7 @@ struct notifier_block br_device_notifier
static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
- struct net_bridge_port *p = br_port_get(dev);
+ struct net_bridge_port *p;
struct net_bridge *br;
int err;
--- a/net/bridge/br_private.h 2010-11-15 08:24:47.829474102 -0800
+++ b/net/bridge/br_private.h 2010-11-15 08:27:46.747612478 -0800
@@ -151,11 +151,19 @@ struct net_bridge_port
#endif
};
-#define br_port_get_rcu(dev) \
- ((struct net_bridge_port *) rcu_dereference(dev->rx_handler_data))
-#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data)
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
+static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev)
+{
+ struct net_bridge_port *port = rcu_dereference(dev->rx_handler_data);
+ return br_port_exists(dev) ? port : NULL;
+}
+
+static inline struct net_bridge_port *br_port_get(struct net_device *dev)
+{
+ return br_port_exists(dev) ? dev->rx_handler_data : NULL;
+}
+
struct br_cpu_netstats {
u64 rx_packets;
u64 rx_bytes;
--- a/net/bridge/br_if.c 2010-11-15 08:20:20.084389475 -0800
+++ b/net/bridge/br_if.c 2010-11-15 08:25:50.049646467 -0800
@@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, str
{
struct net_bridge_port *p;
- if (!br_port_exists(dev))
- return -EINVAL;
-
p = br_port_get(dev);
- if (p->br != br)
+ if (!p || p->br != br)
return -EINVAL;
del_nbp(p);
--- a/net/bridge/br_netlink.c 2010-11-15 08:24:41.696606136 -0800
+++ b/net/bridge/br_netlink.c 2010-11-15 08:25:50.049646467 -0800
@@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff
idx = 0;
for_each_netdev(net, dev) {
+ struct net_bridge_port *port = br_port_get(dev);
+
/* not a bridge port */
- if (!br_port_exists(dev) || idx < cb->args[0])
+ if (!port || idx < cb->args[0])
goto skip;
- if (br_fill_ifinfo(skb, br_port_get(dev),
+ if (br_fill_ifinfo(skb, port,
NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, RTM_NEWLINK,
NLM_F_MULTI) < 0)
@@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff
if (!dev)
return -ENODEV;
- if (!br_port_exists(dev))
- return -EINVAL;
p = br_port_get(dev);
+ if (!p)
+ return -EINVAL;
/* if kernel STP is running, don't allow changes */
if (p->br->stp_enabled == BR_KERNEL_STP)
^ permalink raw reply
* [PATCH 3/5] netdev: add rcu annotations to receive handler hook
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20101115163809.215552143@vyatta.com>
[-- Attachment #1: rx_handler_rcu.patch --]
[-- Type: text/plain, Size: 595 bytes --]
Suggested by Eric's bridge RCU changes.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/include/linux/netdevice.h 2010-11-14 11:41:53.224298362 -0800
+++ b/include/linux/netdevice.h 2010-11-14 11:42:42.546359900 -0800
@@ -995,8 +995,8 @@ struct net_device {
unsigned int real_num_rx_queues;
#endif
- rx_handler_func_t *rx_handler;
- void *rx_handler_data;
+ rx_handler_func_t __rcu *rx_handler;
+ void __rcu *rx_handler_data;
struct netdev_queue __rcu *ingress_queue;
^ permalink raw reply
* Re: [RFC PATCH] network: return errors if we know tcp_connect failed
From: Patrick McHardy @ 2010-11-15 16:36 UTC (permalink / raw)
To: Eric Paris
Cc: Hua Zhong, netdev, linux-kernel, davem, kuznet, pekkas, jmorris,
yoshfuji, Netfilter Development Mailinglist
In-Reply-To: <4CE15885.90003@trash.net>
[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]
On 15.11.2010 16:57, Patrick McHardy wrote:
> On 15.11.2010 16:47, Eric Paris wrote:
>> I notice the heavy lifting for this is done in
>> net/ipv4/netfilter/ipt_REJECT.c::send_rest()
>> (and something very similar for IPv6)
>>
>> I really don't want to duplicate that code into SELinux (for obvious
>> reasons) and I'm wondering if anyone has objections to me making it
>> available outside of netlink and/or suggestions on how to make that code
>> available outside of netfilter (aka what header to expose it, and does
>> it still make logical sense in ipt_REJECT.c or somewhere else?)
>
> I don't think having SELinux sending packets to handle local
> connections is a very elegant design, its not a firewall after
> all. What's wrong with reacting only to specific errno codes
> in tcp_connect()? You could f.i. return -ECONNREFUSED from
> SELinux, that one is pretty much guaranteed not to occur in
> the network stack itself and can be returned directly.
>
> That would need minor changes to nf_hook_slow so we can
> encode errno values in the upper 16 bits of the verdict,
> as we already do with the queue number. The added benefit
> is that we don't have to return EPERM anymore when f.i.
> rerouting fails.
Patch for demonstration purposes attached. I've modified the
MARK target so it returns NF_DROP with an errno code of
-ECONNREFUSED:
# iptables -A OUTPUT -d 1.2.3.4 -j MARK --set-mark 1
# ping 1.2.3.4
PING 1.2.3.4 (1.2.3.4) 56(84) bytes of data.
ping: sendmsg: Connection refused
# telnet 1.2.3.4
Trying 1.2.3.4...
telnet: Unable to connect to remote host: Connection refused
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2252 bytes --]
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 89341c3..ef2af8f 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -33,6 +33,8 @@
#define NF_QUEUE_NR(x) ((((x) << NF_VERDICT_BITS) & NF_VERDICT_QMASK) | NF_QUEUE)
+#define NF_DROP_ERR(x) (((-x) << NF_VERDICT_BITS) | NF_DROP)
+
/* only for userspace compatibility */
#ifndef __KERNEL__
/* Generic cache responses from hook functions.
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 05b1ecf..bb8f547 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2592,6 +2592,7 @@ int tcp_connect(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *buff;
+ int err;
tcp_connect_init(sk);
@@ -2614,7 +2615,9 @@ int tcp_connect(struct sock *sk)
sk->sk_wmem_queued += buff->truesize;
sk_mem_charge(sk, buff->truesize);
tp->packets_out += tcp_skb_pcount(buff);
- tcp_transmit_skb(sk, buff, 1, sk->sk_allocation);
+ err = tcp_transmit_skb(sk, buff, 1, sk->sk_allocation);
+ if (err == -ECONNREFUSED)
+ return err;
/* We change tp->snd_nxt after the tcp_transmit_skb() call
* in order to make this packet get counted in tcpOutSegs.
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 85dabb8..32fcbe2 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -173,9 +173,11 @@ next_hook:
outdev, &elem, okfn, hook_thresh);
if (verdict == NF_ACCEPT || verdict == NF_STOP) {
ret = 1;
- } else if (verdict == NF_DROP) {
+ } else if ((verdict & NF_VERDICT_MASK) == NF_DROP) {
kfree_skb(skb);
- ret = -EPERM;
+ ret = -(verdict >> NF_VERDICT_BITS);
+ if (ret == 0)
+ ret = -EPERM;
} else if ((verdict & NF_VERDICT_MASK) == NF_QUEUE) {
if (!nf_queue(skb, elem, pf, hook, indev, outdev, okfn,
verdict >> NF_VERDICT_BITS))
diff --git a/net/netfilter/xt_mark.c b/net/netfilter/xt_mark.c
index 2334523..185330c 100644
--- a/net/netfilter/xt_mark.c
+++ b/net/netfilter/xt_mark.c
@@ -30,7 +30,7 @@ mark_tg(struct sk_buff *skb, const struct xt_action_param *par)
const struct xt_mark_tginfo2 *info = par->targinfo;
skb->mark = (skb->mark & ~info->mask) ^ info->mark;
- return XT_CONTINUE;
+ return NF_DROP_ERR(-ECONNREFUSED);
}
static bool
^ permalink raw reply related
* Re: [PATCH 14/44] drivers/net/ixgbe: Remove unnecessary semicolons
From: Rose, Gregory V @ 2010-11-15 16:24 UTC (permalink / raw)
To: Joe Perches, Jiri Kosina
Cc: e1000-devel@lists.sourceforge.net, Allan, Bruce W,
Brandeburg, Jesse, linux-kernel@vger.kernel.org, Ronciak, John,
Kirsher, Jeffrey T, netdev@vger.kernel.org
In-Reply-To: <7d2c334daa75c5221946a17d45c9de1901cf06e7.1289789604.git.joe@perches.com>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Sunday, November 14, 2010 7:05 PM
> To: Jiri Kosina
> Cc: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny,
> Carolyn; Skidmore, Donald C; Rose, Gregory V; Waskiewicz Jr, Peter P;
> Duyck, Alexander H; Ronciak, John; e1000-devel@lists.sourceforge.net;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 14/44] drivers/net/ixgbe: Remove unnecessary semicolons
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/ixgbe/ixgbe_sriov.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_sriov.c
> b/drivers/net/ixgbe/ixgbe_sriov.c
> index 5428153..93f40bc 100644
> --- a/drivers/net/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ixgbe/ixgbe_sriov.c
> @@ -68,7 +68,7 @@ static int ixgbe_set_vf_multicasts(struct ixgbe_adapter
> *adapter,
> * addresses
> */
> for (i = 0; i < entries; i++) {
> - vfinfo->vf_mc_hashes[i] = hash_list[i];;
> + vfinfo->vf_mc_hashes[i] = hash_list[i];
> }
>
> for (i = 0; i < vfinfo->num_vf_mc_hashes; i++) {
> --
> 1.7.3.1.g432b3.dirty
Acked By: Greg Rose <Gregory.v.rose@intel.com>
------------------------------------------------------------------------------
Centralized Desktop Delivery: Dell and VMware Reference Architecture
Simplifying enterprise desktop deployment and management using
Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
client virtualization framework. Read more!
http://p.sf.net/sfu/dell-eql-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH 0/5] bridge: RCU annotation and cleanup
From: Stephen Hemminger @ 2010-11-15 16:19 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: davem, eric.dumazet, netdev, bridge
In-Reply-To: <201011152123.HHB21896.HFOOVSMFOtLJQF@I-love.SAKURA.ne.jp>
On Mon, 15 Nov 2010 21:23:37 +0900
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> > + rp = rcu_dereference(hlist_next_rcu(rp->next));
>
> I think this one is hlist_next_rcu(rp).
Yes, you are correct.
--
^ permalink raw reply
* Re: [RFC PATCH] network: return errors if we know tcp_connect failed
From: Patrick McHardy @ 2010-11-15 16:04 UTC (permalink / raw)
To: Eric Paris
Cc: Hua Zhong, netdev, linux-kernel, davem, kuznet, pekkas, jmorris,
yoshfuji
In-Reply-To: <4CE15885.90003@trash.net>
On 15.11.2010 16:57, Patrick McHardy wrote:
> On 15.11.2010 16:47, Eric Paris wrote:
>>> iptables -A OUTPUT -p tcp -j REJECT --reject-with tcp-reset
>>>
>>> The second one will cause a hard error for the connection.
>>
>> Well I'm (I guess?) surprised that the --reject-with icmp doesn't do
>> anything with a local outgoing connection but --reject-with tcp-reset
>> does something like what I'm looking for.
>>
>> I notice the heavy lifting for this is done in
>> net/ipv4/netfilter/ipt_REJECT.c::send_rest()
>> (and something very similar for IPv6)
>>
>> I really don't want to duplicate that code into SELinux (for obvious
>> reasons) and I'm wondering if anyone has objections to me making it
>> available outside of netlink and/or suggestions on how to make that code
>> available outside of netfilter (aka what header to expose it, and does
>> it still make logical sense in ipt_REJECT.c or somewhere else?)
>
> I don't think having SELinux sending packets to handle local
> connections is a very elegant design, its not a firewall after
> all. What's wrong with reacting only to specific errno codes
> in tcp_connect()? You could f.i. return -ECONNREFUSED from
> SELinux, that one is pretty much guaranteed not to occur in
> the network stack itself and can be returned directly.
One more note: there is also the problem that the RST might never
reach the socket, f.i. because netfilter drops it, or TC actions
reroute it etc. With netfilter users are expected to make sure the
entire combination of network features does what the expect, but
that's probably not what you want for SELinux.
> That would need minor changes to nf_hook_slow so we can
> encode errno values in the upper 16 bits of the verdict,
> as we already do with the queue number. The added benefit
> is that we don't have to return EPERM anymore when f.i.
> rerouting fails.
^ permalink raw reply
* [PATCH 0/1] rtnetlink/netdevice include triggers userspace compiler errors
From: Andy Whitcroft @ 2010-11-15 16:01 UTC (permalink / raw)
To: David S. Miller", Eric Dumazet
Cc: netdev, linux-kernel, Andy Whitcroft, Tim Gardner
We have seen a number of reports of userspace applications (including
eglibc) which fail to compile when trying to use linux/rtnetlink.h.
It appears that a new helper function has necessitated the inclusion of
linux/netdevice.h which in turn causes a collision with userspace headers
from libc, with net/if.h.
It appears that this header is not required for the userspace exported
components of rtnetlink.h. Following this email is a patch to pull this
include down in the the kernel specific section of this header. It seems
to both fix this issue for userspace and still compiles correctly for
kernel use.
Against v2.6.37-rc1.
-apw
Andy Whitcroft (1):
net: rtnetlink.h -- only include linux/netdevice.h when used by the
kernel
include/linux/rtnetlink.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
^ permalink raw reply
* [PATCH 1/1] net: rtnetlink.h -- only include linux/netdevice.h when used by the kernel
From: Andy Whitcroft @ 2010-11-15 16:01 UTC (permalink / raw)
To: David S. Miller", Eric Dumazet
Cc: netdev, linux-kernel, Andy Whitcroft, Tim Gardner
In-Reply-To: <1289836919-19153-1-git-send-email-apw@canonical.com>
The commit below added a new helper dev_ingress_queue to cleanly obtain the
ingress queue pointer. This necessitated including 'linux/netdevice.h':
commit 24824a09e35402b8d58dcc5be803a5ad3937bdba
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat Oct 2 06:11:55 2010 +0000
net: dynamic ingress_queue allocation
However this include triggers issues for applications in userspace
which use the rtnetlink interfaces. Commonly this requires they include
'net/if.h' and 'linux/rtnetlink.h' leading to a compiler error as below:
In file included from /usr/include/linux/netdevice.h:28:0,
from /usr/include/linux/rtnetlink.h:9,
from t.c:2:
/usr/include/linux/if.h:135:8: error: redefinition of ‘struct ifmap’
/usr/include/net/if.h:112:8: note: originally defined here
/usr/include/linux/if.h:169:8: error: redefinition of ‘struct ifreq’
/usr/include/net/if.h:127:8: note: originally defined here
/usr/include/linux/if.h:218:8: error: redefinition of ‘struct ifconf’
/usr/include/net/if.h:177:8: note: originally defined here
The new helper is only defined for the kernel and protected by __KERNEL__
therefore we can simply pull the include down into the same protected
section.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
include/linux/rtnetlink.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index d42f274..bbad657 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -6,7 +6,6 @@
#include <linux/if_link.h>
#include <linux/if_addr.h>
#include <linux/neighbour.h>
-#include <linux/netdevice.h>
/* rtnetlink families. Values up to 127 are reserved for real address
* families, values above 128 may be used arbitrarily.
@@ -606,6 +605,7 @@ struct tcamsg {
#ifdef __KERNEL__
#include <linux/mutex.h>
+#include <linux/netdevice.h>
static __inline__ int rtattr_strcmp(const struct rtattr *rta, const char *str)
{
--
1.7.0.4
^ permalink raw reply related
* Re: [RFC PATCH] network: return errors if we know tcp_connect failed
From: Patrick McHardy @ 2010-11-15 15:57 UTC (permalink / raw)
To: Eric Paris
Cc: Hua Zhong, netdev, linux-kernel, davem, kuznet, pekkas, jmorris,
yoshfuji
In-Reply-To: <1289836066.14282.7.camel@localhost.localdomain>
On 15.11.2010 16:47, Eric Paris wrote:
> On Mon, 2010-11-15 at 11:32 +0100, Patrick McHardy wrote:
>> On 13.11.2010 00:14, Hua Zhong wrote:
>>>> On 11.11.2010 22:58, Hua Zhong wrote:
>>>>>> Yes, I realize this is little different than if the
>>>>>> SYN was dropped in the first network device, but it is different
>>>>>> because we know what happened! We know that connect() call failed
>>>>>> and that there isn't anything coming back.
>>>>>
>>>>> I would argue that -j DROP should behave exactly as the packet is
>>>> dropped in the network, while -j REJECT should signal the failure to
>>>> the application as soon as possible (which it doesn't seem to do).
>>>>
>>>> It sends an ICMP error or TCP reset. Interpretation is up to TCP.
>>>
>>> Huh? It's the OUTPUT chain we are talking about. There is no ICMP error or
>>> TCP reset.
>>
>> Of course there is.
>>
>> ICMP (default):
>>
>> iptables -A OUTPUT -p tcp -j REJECT
>>
>> TCP reset:
>>
>> iptables -A OUTPUT -p tcp -j REJECT --reject-with tcp-reset
>>
>> The second one will cause a hard error for the connection.
>
> Well I'm (I guess?) surprised that the --reject-with icmp doesn't do
> anything with a local outgoing connection but --reject-with tcp-reset
> does something like what I'm looking for.
>
> I notice the heavy lifting for this is done in
> net/ipv4/netfilter/ipt_REJECT.c::send_rest()
> (and something very similar for IPv6)
>
> I really don't want to duplicate that code into SELinux (for obvious
> reasons) and I'm wondering if anyone has objections to me making it
> available outside of netlink and/or suggestions on how to make that code
> available outside of netfilter (aka what header to expose it, and does
> it still make logical sense in ipt_REJECT.c or somewhere else?)
I don't think having SELinux sending packets to handle local
connections is a very elegant design, its not a firewall after
all. What's wrong with reacting only to specific errno codes
in tcp_connect()? You could f.i. return -ECONNREFUSED from
SELinux, that one is pretty much guaranteed not to occur in
the network stack itself and can be returned directly.
That would need minor changes to nf_hook_slow so we can
encode errno values in the upper 16 bits of the verdict,
as we already do with the queue number. The added benefit
is that we don't have to return EPERM anymore when f.i.
rerouting fails.
^ permalink raw reply
* Re: [RFC PATCH] network: return errors if we know tcp_connect failed
From: Eric Paris @ 2010-11-15 15:47 UTC (permalink / raw)
To: Patrick McHardy
Cc: Hua Zhong, netdev, linux-kernel, davem, kuznet, pekkas, jmorris,
yoshfuji
In-Reply-To: <4CE10C2A.1050801@trash.net>
On Mon, 2010-11-15 at 11:32 +0100, Patrick McHardy wrote:
> On 13.11.2010 00:14, Hua Zhong wrote:
> >> On 11.11.2010 22:58, Hua Zhong wrote:
> >>>> Yes, I realize this is little different than if the
> >>>> SYN was dropped in the first network device, but it is different
> >>>> because we know what happened! We know that connect() call failed
> >>>> and that there isn't anything coming back.
> >>>
> >>> I would argue that -j DROP should behave exactly as the packet is
> >> dropped in the network, while -j REJECT should signal the failure to
> >> the application as soon as possible (which it doesn't seem to do).
> >>
> >> It sends an ICMP error or TCP reset. Interpretation is up to TCP.
> >
> > Huh? It's the OUTPUT chain we are talking about. There is no ICMP error or
> > TCP reset.
>
> Of course there is.
>
> ICMP (default):
>
> iptables -A OUTPUT -p tcp -j REJECT
>
> TCP reset:
>
> iptables -A OUTPUT -p tcp -j REJECT --reject-with tcp-reset
>
> The second one will cause a hard error for the connection.
Well I'm (I guess?) surprised that the --reject-with icmp doesn't do
anything with a local outgoing connection but --reject-with tcp-reset
does something like what I'm looking for.
I notice the heavy lifting for this is done in
net/ipv4/netfilter/ipt_REJECT.c::send_rest()
(and something very similar for IPv6)
I really don't want to duplicate that code into SELinux (for obvious
reasons) and I'm wondering if anyone has objections to me making it
available outside of netlink and/or suggestions on how to make that code
available outside of netfilter (aka what header to expose it, and does
it still make logical sense in ipt_REJECT.c or somewhere else?)
-Eric
^ permalink raw reply
* [PATCH net-next-2.6] clarify documentation for net.ipv4.igmp_max_memberships
From: Jeremy Eder @ 2010-11-15 15:41 UTC (permalink / raw)
To: netdev
Cc: rdunlap, davem, opurdila, apetlund, William.Allen.Simpson,
ian.campbell, linux-doc, linux-kernel, Jiri Pirko
This patch helps clarify documentation for
net.ipv4.igmp_max_memberships by providing a formula for
calculating the maximum number of multicast groups that can be
subscribed to, plus defining the theoretical limit.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Signed-off-by: Jeremy Eder <jeder@redhat.com>
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index fe95105..ae55227 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -707,10 +707,28 @@ igmp_max_memberships - INTEGER
Change the maximum number of multicast groups we can subscribe to.
Default: 20
-conf/interface/* changes special settings per interface (where "interface" is
- the name of your network interface)
-conf/all/* is special, changes the settings for all interfaces
+ Theoretical maximum value is bounded by having to send a membership
+ report in a single datagram (i.e. the report can't span multiple
+ datagrams, or risk confusing the switch and leaving groups you don't
+ intend to).
+ The number of supported groups 'M' is bounded by the number of group
+ report entries you can fit into a single datagram of 65535 bytes.
+
+ M = 65536-sizeof (ip header)/(sizeof(Group record))
+
+ Group records are variable length, with a minimum of 12 bytes.
+ So net.ipv4.igmp_max_memberships should not be set higher than:
+
+ (65536-24) / 12 = 5459
+
+ The value 5459 assumes no IP header options, so in practice
+ this number may be lower.
+
+ conf/interface/* changes special settings per interface (where
+ "interface" is the name of your network interface)
+
+ conf/all/* is special, changes the settings for all interfaces
log_martians - BOOLEAN
Log packets with impossible addresses to kernel log.
^ permalink raw reply related
* Re: NETPOLL on bond interfaces
From: sergey belov @ 2010-11-15 15:35 UTC (permalink / raw)
To: Netdev
In-Reply-To: <AANLkTingysT7=+AiGCh4kiY27RK9jz40ci1y_of4o-FC@mail.gmail.com>
found this thread:
http://amailbox.org/mailarchive/linux-netdev/2010/6/25/6280002
and this:
static int disable_netpoll = 1; in drivers/net/bonding/bond_main.c
have switched value to 0, so nevermind now :)
On Mon, Nov 15, 2010 at 4:49 PM, sergey belov <gexlie@gmail.com> wrote:
> Before 2.6.36 came out I was using this patch to enable netpoll on bond
> interfaces (see below)
>
> When I tried to apply this patch to 2.6.36 sources some chunks was failed.
>
> [root@kernel-x32 experimental]# patch -p0 < netpoll.patch
> patching file a/drivers/net/bonding/bond_main.c
> Hunk #1 FAILED at 75.
> Hunk #2 FAILED at 416.
> Hunk #3 succeeded at 1334 with fuzz 2 (offset 23 lines).
> Hunk #4 succeeded at 1729 with fuzz 1 (offset 15 lines).
> Hunk #5 succeeded at 1837 (offset 42 lines).
> Hunk #6 succeeded at 2026 (offset 19 lines).
> Hunk #7 succeeded at 4490 with fuzz 1 (offset 11 lines).
> Hunk #8 succeeded at 4674 (offset 68 lines).
> 2 out of 8 hunks FAILED -- saving rejects to file
> a/drivers/net/bonding/bond_main.c.rej
> patching file a/drivers/net/bonding/bonding.h
> Hunk #2 succeeded at 227 (offset 4 lines).
> patching file a/include/linux/netdevice.h
> Hunk #1 FAILED at 52.
> Hunk #2 FAILED at 625.
> 2 out of 2 hunks FAILED -- saving rejects to file
> a/include/linux/netdevice.h.rej
> patching file a/include/linux/netpoll.h
> Hunk #1 succeeded at 43 with fuzz 2 (offset 9 lines).
> patching file a/net/core/netpoll.c
> Reversed (or previously applied) patch detected! Assume -R? [n]
>
>
> Inspecting changelog at
> http://www.kernel.org/pub/linux/kernel/v2.6/ChangeLog-2.6.36 I found there
> are a lot of changes was done inside of the bonding and netpoll subsystem.
>
> Could please someone provide me new patch to enable netpoll on bond ifaces
> or send me a workaround how to enable it in any other way.
>
> We need to use it only to enable netconsole on our servers with bond.
>
>
> 1. diff --git a/drivers/net/bonding/bond_main.c
> b/drivers/net/bonding/bond_main.c
> 2. index d927f71..6304720 100644
> 3. --- a/drivers/net/bonding/bond_main.c
> 4. +++ b/drivers/net/bonding/bond_main.c
> 5. @@ -75,6 +75,7 @@
> 6. #include <linux/jiffies.h>
> 7. #include <net/route.h>
> 8. #include <net/net_namespace.h>
> 9. +#include <linux/netpoll.h>
> 10. #include "bonding.h"
> 11. #include "bond_3ad.h"
> 12. #include "bond_alb.h"
> 13. @@ -415,7 +416,12 @@ int bond_dev_queue_xmit(struct bonding *bond,
> struct sk_buff *skb,
> 14. }
> 15.
> 16. skb->priority = 1;
> 17. - dev_queue_xmit(skb);
> 18. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 19. + if (bond->netpoll)
> 20. + netpoll_send_skb(bond->netpoll, skb);
> 21. + else
> 22. +#endif
> 23. + dev_queue_xmit(skb);
> 24.
> 25. return 0;
> 26. }
> 27. @@ -1305,6 +1311,44 @@ static void bond_detach_slave(struct bonding
> *bond, struct slave *slave)
> 28. bond->slave_cnt--;
> 29. }
> 30.
> 31. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 32. +static int slaves_support_netpoll(struct net_device *bond_dev)
> 33. +{
> 34. + struct bonding *bond = netdev_priv(bond_dev);
> 35. + struct slave *slave;
> 36. + int i;
> 37. +
> 38. + bond_for_each_slave(bond, slave, i)
> 39. + if (!slave->dev->netdev_ops->ndo_poll_controller)
> 40. + return 0;
> 41. + return 1;
> 42. +}
> 43. +
> 44. +static void bond_poll_controller(struct net_device *bond_dev)
> 45. +{
> 46. + struct bonding *bond = netdev_priv(bond_dev);
> 47. + struct slave *slave;
> 48. + int i;
> 49. +
> 50. + if (slaves_support_netpoll(bond_dev))
> 51. + bond_for_each_slave(bond, slave, i)
> 52. + netpoll_poll_dev(slave->dev);
> 53. +}
> 54. +
> 55. +static int bond_netpoll_setup(struct net_device *bond_dev,
> 56. + struct netpoll_info *npinfo)
> 57. +{
> 58. + struct bonding *bond = netdev_priv(bond_dev);
> 59. + struct slave *slave;
> 60. + int i;
> 61. +
> 62. + bond_for_each_slave(bond, slave, i)
> 63. + if (slave->dev)
> 64. + slave->dev->npinfo = npinfo;
> 65. + return 0;
> 66. +}
> 67. +#endif
> 68. +
> 69. /*---------------------------------- IOCTL
> ----------------------------------*/
> 70.
> 71. static int bond_sethwaddr(struct net_device *bond_dev,
> 72. @@ -1670,6 +1714,16 @@ int bond_enslave(struct net_device *bond_dev,
> struct net_device *slave_dev)
> 73. bond->primary_slave = new_slave;
> 74. }
> 75.
> 76. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 77. + if (slaves_support_netpoll(bond_dev))
> 78. + slave_dev->npinfo = bond_dev->npinfo;
> 79. + else {
> 80. + pr_err(DRV_NAME "New slave device %s does not support
> netpoll.\n",
> 81. + slave_dev->name);
> 82. + pr_err(DRV_NAME "netpoll disabled for %s.\n",
> bond_dev->name);
> 83. + }
> 84. +#endif
> 85. +
> 86. write_lock_bh(&bond->curr_slave_lock);
> 87.
> 88. switch (bond->params.mode) {
> 89. @@ -1741,6 +1795,10 @@ int bond_enslave(struct net_device *bond_dev,
> struct net_device *slave_dev)
> 90.
> 91. /* Undo stages on error */
> 92. err_close:
> 93. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 94. + if (slave_dev->npinfo)
> 95. + slave_dev->npinfo = NULL;
> 96. +#endif
> 97. dev_close(slave_dev);
> 98.
> 99. err_unset_master:
> 100. @@ -1949,6 +2007,10 @@ int bond_release(struct net_device *bond_dev,
> struct net_device *slave_dev)
> 101. IFF_SLAVE_INACTIVE | IFF_BONDING
> |
> 102. IFF_SLAVE_NEEDARP);
> 103.
> 104. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 105. + if (slave_dev->npinfo)
> 106. + slave_dev->npinfo = NULL;
> 107. +#endif
> 108. kfree(slave);
> 109.
> 110. return 0; /* deletion OK */
> 111. @@ -4417,6 +4479,20 @@ out:
> 112. return 0;
> 113. }
> 114.
> 115. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 116. +int bond_netpoll_start_xmit(struct netpoll *np, struct sk_buff *skb
> )
> 117. +{
> 118. + struct bonding *bond = netdev_priv(skb->dev);
> 119. + int ret;
> 120. +
> 121. + bond->netpoll = np;
> 122. + ret = bond->dev->netdev_ops->ndo_start_xmit(skb, bond->dev);
> 123. + bond->netpoll = NULL;
> 124. +
> 125. + return ret;
> 126. +}
> 127. +#endif
> 128. +
> 129. /*------------------------- Device initialization
> ---------------------------*/
> 130.
> 131. static void bond_set_xmit_hash_policy(struct bonding *bond)
> 132. @@ -4530,6 +4606,9 @@ static const struct net_device_ops
> bond_netdev_ops = {
> 133. .ndo_change_mtu = bond_change_mtu,
> 134. .ndo_set_mac_address = bond_set_mac_address,
> 135. .ndo_neigh_setup = bond_neigh_setup,
> 136. + .ndo_netpoll_setup = bond_netpoll_setup,
> 137. + .ndo_netpoll_start_xmit = bond_netpoll_start_xmit,
> 138. + .ndo_poll_controller = bond_poll_controller,
> 139. .ndo_vlan_rx_register = bond_vlan_rx_register,
> 140. .ndo_vlan_rx_add_vid = bond_vlan_rx_add_vid,
> 141. .ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid,
> 142. diff --git a/drivers/net/bonding/bonding.h
> b/drivers/net/bonding/bonding.h
> 143. index 6290a50..563d28c 100644
> 144. --- a/drivers/net/bonding/bonding.h
> 145. +++ b/drivers/net/bonding/bonding.h
> 146. @@ -18,6 +18,7 @@
> 147. #include <linux/timer.h>
> 148. #include <linux/proc_fs.h>
> 149. #include <linux/if_bonding.h>
> 150. +#include <linux/netpoll.h>
> 151. #include <linux/kobject.h>
> 152. #include <linux/in6.h>
> 153. #include "bond_3ad.h"
> 154. @@ -222,6 +223,10 @@ struct bonding {
> 155. #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> 156. struct in6_addr master_ipv6;
> 157. #endif
> 158. +#ifdef CONFIG_NET_POLL_CONTROLLER
> 159. + struct netpoll *netpoll;
> 160. +#endif
> 161. +
> 162. };
> 163.
> 164. /**
> 165. diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> 166. index d4a4d98..e1724b2 100644
> 167. --- a/include/linux/netdevice.h
> 168. +++ b/include/linux/netdevice.h
> 169. @@ -52,6 +52,7 @@
> 170.
> 171. struct vlan_group;
> 172. struct netpoll_info;
> 173. +struct netpoll;
> 174. /* 802.11 specific */
> 175. struct wireless_dev;
> 176. /* source back-compat hooks
> */
> 177. @@ -624,6 +625,10 @@ struct net_device_ops {
> 178. unsigned
> short vid);
> 179. #ifdef CONFIG_NET_POLL_CONTROLLER
> 180. #define HAVE_NETDEV_POLL
> 181. + int (*ndo_netpoll_setup)(struct
> net_device *dev,
> 182. + struct
> netpoll_info *npinfo);
> 183. + int (*ndo_netpoll_start_xmit)(struct
> netpoll *np,
> 184. + struct
> sk_buff *skb);
> 185. void (*ndo_poll_controller)(struct
> net_device *dev);
> 186. #endif
> 187. #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
> 188. diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> 189. index 2524267..39d42e4 100644
> 190. --- a/include/linux/netpoll.h
> 191. +++ b/include/linux/netpoll.h
> 192. @@ -34,7 +34,9 @@ struct netpoll_info {
> 193. };
> 194.
> 195. void netpoll_poll(struct netpoll *np);
> 196. +void netpoll_poll_dev(struct net_device *dev);
> 197. void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
> ;
> 198. +void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
> 199. void netpoll_print_options(struct netpoll *np);
> 200. int netpoll_parse_options(struct netpoll *np, char *opt);
> 201. int netpoll_setup(struct netpoll *np);
> 202. diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> 203. index 9675f31..3776b26 100644
> 204. --- a/net/core/netpoll.c
> 205. +++ b/net/core/netpoll.c
> 206. @@ -174,9 +174,8 @@ static void service_arp_queue(struct
> netpoll_info *npi)
> 207. }
> 208. }
> 209.
> 210. -void netpoll_poll(struct netpoll *np)
> 211. +void netpoll_poll_dev(struct net_device *dev)
> 212. {
> 213. - struct net_device *dev = np->dev;
> 214. const struct net_device_ops *ops;
> 215.
> 216. if (!dev || !netif_running(dev))
> 217. @@ -196,6 +195,11 @@ void netpoll_poll(struct netpoll *np)
> 218. zap_completion_queue();
> 219. }
> 220.
> 221. +void netpoll_poll(struct netpoll *np)
> 222. +{
> 223. + netpoll_poll_dev(np->dev);
> 224. +}
> 225. +
> 226. static void refill_skbs(void)
> 227. {
> 228. struct sk_buff *skb;
> 229. @@ -277,11 +281,11 @@ static int netpoll_owner_active(struct
> net_device *dev)
> 230. return 0;
> 231. }
> 232.
> 233. -static void netpoll_send_skb(struct netpoll *np, struct sk_buff
> *skb)
> 234. +void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
> 235. {
> 236. int status = NETDEV_TX_BUSY;
> 237. unsigned long tries;
> 238. - struct net_device *dev = np->dev;
> 239. + struct net_device *dev = skb->dev;
> 240. const struct net_device_ops *ops = dev->netdev_ops;
> 241. struct netpoll_info *npinfo = np->dev->npinfo;
> 242.
> 243. @@ -303,7 +307,10 @@ static void netpoll_send_skb(struct netpoll
> *np, struct sk_buff *skb)
> 244. tries > 0; --tries) {
> 245. if (__netif_tx_trylock(txq)) {
> 246. if (!netif_tx_queue_stopped(txq)) {
> 247. - status = ops->ndo_start_xmit
> (skb, dev);
> 248. + if (
> ops->ndo_netpoll_start_xmit)
> 249. + status =
> ops->ndo_netpoll_start_xmit(np,skb);
> 250. + else
> 251. + status =
> ops->ndo_start_xmit(skb, dev);
> 252. if (status == NETDEV_TX_OK)
> 253. txq_trans_update(txq
> );
> 254. }
> 255. @@ -789,6 +796,9 @@ int netpoll_setup(struct netpoll *np)
> 256. /* avoid racing with NAPI reading npinfo */
> 257. synchronize_rcu();
> 258.
> 259. + if (ndev->netdev_ops->ndo_netpoll_setup)
> 260. + ndev->netdev_ops->ndo_netpoll_setup(ndev, npinfo);
> 261. +
> 262. return 0;
> 263.
> 264. release:
> 265. @@ -859,4 +869,6 @@ EXPORT_SYMBOL(netpoll_parse_options);
> 266. EXPORT_SYMBOL(netpoll_setup);
> 267. EXPORT_SYMBOL(netpoll_cleanup);
> 268. EXPORT_SYMBOL(netpoll_send_udp);
> 269. +EXPORT_SYMBOL(netpoll_send_skb);
> 270. EXPORT_SYMBOL(netpoll_poll);
> 271. +EXPORT_SYMBOL(netpoll_poll_dev);
>
^ permalink raw reply
* ipheth "UI" problem
From: Bastien Nocera @ 2010-11-15 15:13 UTC (permalink / raw)
To: Diego Giagio; +Cc: netdev
Hey Diego,
ipheth recently got added to the kernel, and seeped into Fedora. I love
the functionality of it (even though I'm more likely to be using the
Bluetooth tethering, to be honest).
I already filed a bug against NetworkManager connecting to the interface
automatically:
https://bugzilla.gnome.org/show_bug.cgi?id=633465
My problem is while I have disabled NetworkManager's automatic
connection, the kernel module still connects to the device, and I have a
constant "Internet Tethering" appearing on my iPhone.
Wouldn't it be possible to only check for the service being offered by
the device when plugging it in, and actually do the connection to the
service when we want to use it (when the interface is upped)?
Cheers
PS: Re-sent with the correct ML address, sorry Diego
^ permalink raw reply
* Re: [PATCH] arch/tile: fix rwlock so would-be write lockers don't block new readers
From: Chris Metcalf @ 2010-11-15 15:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, Américo Wang, netdev, Cypher Wu
In-Reply-To: <1289832730.2607.87.camel@edumazet-laptop>
On 11/15/2010 9:52 AM, Eric Dumazet wrote:
> Le lundi 15 novembre 2010 à 09:18 -0500, Chris Metcalf a écrit :
>> This avoids a deadlock in the IGMP code where one core gets a read
>> lock, another core starts trying to get a write lock (thus blocking
>> new readers), and then the first core tries to recursively re-acquire
>> the read lock.
>>
>> We still try to preserve some degree of balance by giving priority
>> to additional write lockers that come along while the lock is held
>> for write, so they can all complete quickly and return the lock to
>> the readers.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
>> ---
>> This should apply relatively cleanly to 2.6.26.7 source code too.
>>
>> arch/tile/lib/spinlock_32.c | 29 ++++++++++++++++++-----------
>> 1 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/tile/lib/spinlock_32.c b/arch/tile/lib/spinlock_32.c
>> index 485e24d..5cd1c40 100644
>> --- a/arch/tile/lib/spinlock_32.c
>> +++ b/arch/tile/lib/spinlock_32.c
>> @@ -167,23 +167,30 @@ void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
>> * when we compare them.
>> */
>> u32 my_ticket_;
>> + u32 iterations = 0;
>>
>> - /* Take out the next ticket; this will also stop would-be readers. */
>> - if (val & 1)
>> - val = get_rwlock(rwlock);
>> - rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
>> + /*
>> + * Wait until there are no readers, then bump up the next
>> + * field and capture the ticket value.
>> + */
>> + for (;;) {
>> + if (!(val & 1)) {
>> + if ((val >> RD_COUNT_SHIFT) == 0)
>> + break;
>> + rwlock->lock = val;
>> + }
>> + delay_backoff(iterations++);
> Are you sure a writer should have a growing delay_backoff() ?
We always do this bounded exponential backoff on all locking operations
that require memory-network traffic. With 64 cores, it's possible
otherwise to get into a situation where the cores are attempting to acquire
the lock sufficiently aggressively that lock acquisition performance is
worse than it would be with backoff. In any case this path is unlikely to
run many times, since it only triggers if two cores both try to pull a
ticket at the same time; it doesn't correspond to writers actually waiting
once they have their ticket, which is handled later in this function.
> It seems to me this only allow new readers to come (so adding more
> unfairness to the rwlock, that already favor readers against writer[s])
Well, that is apparently the required semantic. Once there is one reader,
you must allow new readers, to handle the case of recursive
re-acquisition. In principle you could imagine doing something like having
a bitmask of cores that held the readlock (instead of a count), and only
allowing recursive re-acquisition when a write lock request is pending, but
this would make the lock structure bigger, and I'm not sure it's worth it.
x86 certainly doesn't bother.
> Maybe allow one cpu to spin, and eventually other 'writers' be queued ?
Other than the brief spin to acquire the ticket, writers don't actually
spin on the lock. They just wait for their ticket to come up. This does
require spinning on memory reads, but those are satisfied out of the local
core's cache, with the exception that each time a writer completes, the
cache line is invalidated on the readers, and they have to re-fetch it from
the home cache.
>> + val = __insn_tns((int *)&rwlock->lock);
>> + }
>>
>> - /* Extract my ticket value from the original word. */
>> + /* Take out the next ticket and extract my ticket value. */
>> + rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
>> my_ticket_ = val >> WR_NEXT_SHIFT;
>>
>> - /*
>> - * Wait until the "current" field matches our ticket, and
>> - * there are no remaining readers.
>> - */
>> + /* Wait until the "current" field matches our ticket. */
>> for (;;) {
>> u32 curr_ = val >> WR_CURR_SHIFT;
>> - u32 readers = val >> RD_COUNT_SHIFT;
>> - u32 delta = ((my_ticket_ - curr_) & WR_MASK) + !!readers;
>> + u32 delta = ((my_ticket_ - curr_) & WR_MASK);
>> if (likely(delta == 0))
>> break;
>>
>
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply
* Re: [PATCH] arch/tile: fix rwlock so would-be write lockers don't block new readers
From: Eric Dumazet @ 2010-11-15 14:52 UTC (permalink / raw)
To: Chris Metcalf; +Cc: linux-kernel, Américo Wang, netdev, Cypher Wu
In-Reply-To: <201011151425.oAFEPU3W005682@farm-0010.internal.tilera.com>
Le lundi 15 novembre 2010 à 09:18 -0500, Chris Metcalf a écrit :
> This avoids a deadlock in the IGMP code where one core gets a read
> lock, another core starts trying to get a write lock (thus blocking
> new readers), and then the first core tries to recursively re-acquire
> the read lock.
>
> We still try to preserve some degree of balance by giving priority
> to additional write lockers that come along while the lock is held
> for write, so they can all complete quickly and return the lock to
> the readers.
>
> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> ---
> This should apply relatively cleanly to 2.6.26.7 source code too.
>
> arch/tile/lib/spinlock_32.c | 29 ++++++++++++++++++-----------
> 1 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/tile/lib/spinlock_32.c b/arch/tile/lib/spinlock_32.c
> index 485e24d..5cd1c40 100644
> --- a/arch/tile/lib/spinlock_32.c
> +++ b/arch/tile/lib/spinlock_32.c
> @@ -167,23 +167,30 @@ void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
> * when we compare them.
> */
> u32 my_ticket_;
> + u32 iterations = 0;
>
> - /* Take out the next ticket; this will also stop would-be readers. */
> - if (val & 1)
> - val = get_rwlock(rwlock);
> - rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
> + /*
> + * Wait until there are no readers, then bump up the next
> + * field and capture the ticket value.
> + */
> + for (;;) {
> + if (!(val & 1)) {
> + if ((val >> RD_COUNT_SHIFT) == 0)
> + break;
> + rwlock->lock = val;
> + }
> + delay_backoff(iterations++);
Are you sure a writer should have a growing delay_backoff() ?
It seems to me this only allow new readers to come (so adding more
unfairness to the rwlock, that already favor readers against writer[s])
Maybe allow one cpu to spin, and eventually other 'writers' be queued ?
> + val = __insn_tns((int *)&rwlock->lock);
> + }
>
> - /* Extract my ticket value from the original word. */
> + /* Take out the next ticket and extract my ticket value. */
> + rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
> my_ticket_ = val >> WR_NEXT_SHIFT;
>
> - /*
> - * Wait until the "current" field matches our ticket, and
> - * there are no remaining readers.
> - */
> + /* Wait until the "current" field matches our ticket. */
> for (;;) {
> u32 curr_ = val >> WR_CURR_SHIFT;
> - u32 readers = val >> RD_COUNT_SHIFT;
> - u32 delta = ((my_ticket_ - curr_) & WR_MASK) + !!readers;
> + u32 delta = ((my_ticket_ - curr_) & WR_MASK);
> if (likely(delta == 0))
> break;
>
^ permalink raw reply
* Re: [PATCH] atomic: add atomic_inc_not_zero_hint()
From: Eric Dumazet @ 2010-11-15 14:47 UTC (permalink / raw)
To: Christoph Lameter
Cc: Paul E. McKenney, Andrew Morton, linux-kernel, David Miller,
netdev, Arnaldo Carvalho de Melo, Ingo Molnar, Andi Kleen,
Nick Piggin
In-Reply-To: <alpine.DEB.2.00.1011150821000.19175@router.home>
Le lundi 15 novembre 2010 à 08:25 -0600, Christoph Lameter a écrit :
> On Mon, 15 Nov 2010, Eric Dumazet wrote:
>
> > Exclusive access ? As soon as another cpu takes it again, you lose.
>
> Sure but you want to avoid the fetch in shared mode here.
>
Yes, this is what cmpxchg() does for sure.
> > Its not really the same thing... Maybe you miss the 'hint' intention at
> > all. We know the probable value of the counter, we dont want to read it.
>
> Ok may be in thise case you can predict the value but in general it is
> difficult to always provide an expected value. It would be easier to be
> able to tell the processor that the cacheline should not be fetched as
> shared but immediately in exclusive state.
>
Maybe its not clear, but atomic_inc_not_zero_hint() is going to be used
only in contexts we know the expected value, and not as a generic
replacement for atomic_inc_not_zero(). Even if cache line is already hot
in this cpu cache, it should be faster or same speed.
Then, in high contention contexts, using atomic_inc_not_zero_hint() with
whatever initial hint might also be a win over atomic_inc_not_zero(),
but we try to remove such contexts ;)
And two atomic_cmpxchg() are probably slower in non contended contexts,
in particular is cache line is already hot in this cpu cache.
> > atomic_read() and atomic_cmpxchg(). We tried prefetchw() and it was a
> > performance drop. It was with only 16 cpus contending on neighbour
>
> Does prefetchw work? Andi claims that prefetchw is not working on
> x86 and I doubt that you ran tests on Itanium.
In fact, in benchmarks, prefetch() or prefetchw() are a pain on x86, or
at least "perf tools" show artifact on them (high number of cycles
consumed on these instructions)
Andi had a patch to disable prefetch() in list iterators, and its a win.
I dont have Itanium platform to run tests. Is cmpxchg() that bad on
ia64 ? I also have old AMD cpus, so I cannot say if recent ones handle
prefetchw() better...
^ permalink raw reply
* Re: ethtool maintenance
From: Ben Hutchings @ 2010-11-15 14:46 UTC (permalink / raw)
To: Jeff Garzik; +Cc: NetDev, David Miller, Peter Martuccelli
In-Reply-To: <1289658614.2816.105.camel@localhost>
On Sat, 2010-11-13 at 14:30 +0000, Ben Hutchings wrote:
> On Sat, 2010-11-13 at 03:45 -0500, Jeff Garzik wrote:
> > So, a recent emergency surgery has really set me back, work-wise.
> > ethtool [the userspace utility] 2.6.36 is still not out, and personally
> > it remains a third or fourth priority.
> >
> > While it's likely that I could get back to ethtool's patch queue next
> > week, it continues to be low man on the totem pole. Seems only fair to
> > see if anyone else is interested in maintaining it.
> >
> > I emailed Ben Hutchings privately about this, but haven't heard back, so
> > I thought I'd go ahead and email the list.
> >
> > Anyone interested?
>
> I am interested, but will need to clear it with my boss before making
> such a commitment.
OK, this is fine. Are there any patches waiting other than mine and
these?
http://patchwork.ozlabs.org/patch/67662/
http://patchwork.ozlabs.org/patch/68237/
http://patchwork.ozlabs.org/patch/69155/
I'll need to sort out my kernel.org account before I can publish a git
tree.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] atomic: add atomic_inc_not_zero_hint()
From: Andi Kleen @ 2010-11-15 14:39 UTC (permalink / raw)
To: Christoph Lameter
Cc: Eric Dumazet, Paul E. McKenney, Andrew Morton, linux-kernel,
David Miller, netdev, Arnaldo Carvalho de Melo, Ingo Molnar,
Andi Kleen, Nick Piggin
In-Reply-To: <alpine.DEB.2.00.1011150821000.19175@router.home>
> > atomic_read() and atomic_cmpxchg(). We tried prefetchw() and it was a
> > performance drop. It was with only 16 cpus contending on neighbour
>
> Does prefetchw work? Andi claims that prefetchw is not working on
> x86 and I doubt that you ran tests on Itanium.
AMD supports it due to their MOESI protocol, but it's not supported
in MESIF as used by Intel QPI. The kernel maps it on Intel to
ordinary prefetch.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [PATCH] atomic: add atomic_inc_not_zero_hint()
From: Christoph Lameter @ 2010-11-15 14:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paul E. McKenney, Andrew Morton, linux-kernel, David Miller,
netdev, Arnaldo Carvalho de Melo, Ingo Molnar, Andi Kleen,
Nick Piggin
In-Reply-To: <1289830636.2607.70.camel@edumazet-laptop>
On Mon, 15 Nov 2010, Eric Dumazet wrote:
> Exclusive access ? As soon as another cpu takes it again, you lose.
Sure but you want to avoid the fetch in shared mode here.
> Its not really the same thing... Maybe you miss the 'hint' intention at
> all. We know the probable value of the counter, we dont want to read it.
Ok may be in thise case you can predict the value but in general it is
difficult to always provide an expected value. It would be easier to be
able to tell the processor that the cacheline should not be fetched as
shared but immediately in exclusive state.
> atomic_read() and atomic_cmpxchg(). We tried prefetchw() and it was a
> performance drop. It was with only 16 cpus contending on neighbour
Does prefetchw work? Andi claims that prefetchw is not working on
x86 and I doubt that you ran tests on Itanium.
^ permalink raw reply
* [PATCH] arch/tile: fix rwlock so would-be write lockers don't block new readers
From: Chris Metcalf @ 2010-11-15 14:18 UTC (permalink / raw)
To: linux-kernel; +Cc: Américo Wang, Eric Dumazet, netdev, Cypher Wu
In-Reply-To: <AANLkTind92uwcigzmDn8yn9a22exDy7zcreGQ5-6NLV-@mail.gmail.com>
This avoids a deadlock in the IGMP code where one core gets a read
lock, another core starts trying to get a write lock (thus blocking
new readers), and then the first core tries to recursively re-acquire
the read lock.
We still try to preserve some degree of balance by giving priority
to additional write lockers that come along while the lock is held
for write, so they can all complete quickly and return the lock to
the readers.
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
This should apply relatively cleanly to 2.6.26.7 source code too.
arch/tile/lib/spinlock_32.c | 29 ++++++++++++++++++-----------
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/tile/lib/spinlock_32.c b/arch/tile/lib/spinlock_32.c
index 485e24d..5cd1c40 100644
--- a/arch/tile/lib/spinlock_32.c
+++ b/arch/tile/lib/spinlock_32.c
@@ -167,23 +167,30 @@ void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
* when we compare them.
*/
u32 my_ticket_;
+ u32 iterations = 0;
- /* Take out the next ticket; this will also stop would-be readers. */
- if (val & 1)
- val = get_rwlock(rwlock);
- rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
+ /*
+ * Wait until there are no readers, then bump up the next
+ * field and capture the ticket value.
+ */
+ for (;;) {
+ if (!(val & 1)) {
+ if ((val >> RD_COUNT_SHIFT) == 0)
+ break;
+ rwlock->lock = val;
+ }
+ delay_backoff(iterations++);
+ val = __insn_tns((int *)&rwlock->lock);
+ }
- /* Extract my ticket value from the original word. */
+ /* Take out the next ticket and extract my ticket value. */
+ rwlock->lock = __insn_addb(val, 1 << WR_NEXT_SHIFT);
my_ticket_ = val >> WR_NEXT_SHIFT;
- /*
- * Wait until the "current" field matches our ticket, and
- * there are no remaining readers.
- */
+ /* Wait until the "current" field matches our ticket. */
for (;;) {
u32 curr_ = val >> WR_CURR_SHIFT;
- u32 readers = val >> RD_COUNT_SHIFT;
- u32 delta = ((my_ticket_ - curr_) & WR_MASK) + !!readers;
+ u32 delta = ((my_ticket_ - curr_) & WR_MASK);
if (likely(delta == 0))
break;
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Wolfgang Grandegger @ 2010-11-15 14:21 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE0EFA7.9020007-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Hello,
On 11/15/2010 09:30 AM, Tomoya MORINAGA wrote:
> * Add Flow control
> * Fix Data copy issue (endianness)
> * Add Macro prefix "PCH_"
> * Separate interface register structure
> * Some functions are unified.
> * Change MessageObject indication(PCH_RX_OBJ_START, etc..)
> * Enumerate LEC macro
> * Move MSI processing from open/close to probe/remove processing
> * Use BIT(x)
> * and more...
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
> drivers/net/can/pch_can.c | 1348 ++++++++++++++++++++-------------------------
> 1 files changed, 595 insertions(+), 753 deletions(-)
>
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 6727182..6a38593 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
...
> - if (status & PCH_LEC_ALL) {
> + lec = status & PCH_LEC_ALL;
> + switch (lec) {
> + case PCH_STUF_ERR:
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> priv->can.can_stats.bus_error++;
> stats->rx_errors++;
> - switch (status & PCH_LEC_ALL) {
> - case PCH_STUF_ERR:
> - cf->data[2] |= CAN_ERR_PROT_STUFF;
> - break;
> - case PCH_FORM_ERR:
> - cf->data[2] |= CAN_ERR_PROT_FORM;
> - break;
> - case PCH_ACK_ERR:
> - cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
> - CAN_ERR_PROT_LOC_ACK_DEL;
> - break;
> - case PCH_BIT1_ERR:
> - case PCH_BIT0_ERR:
> - cf->data[2] |= CAN_ERR_PROT_BIT;
> - break;
> - case PCH_CRC_ERR:
> - cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> - CAN_ERR_PROT_LOC_CRC_DEL;
> - break;
> - default:
> - iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
> - break;
> - }
> -
> + break;
> + case PCH_FORM_ERR:
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_ACK_ERR:
> + cf->can_id |= CAN_ERR_ACK;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_BIT1_ERR:
> + case PCH_BIT0_ERR:
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_CRC_ERR:
> + cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL;
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + break;
> + case PCH_LEC_ALL: /* Written by CPU. No error status */
> + break;
> }
More comments to the lec handling below.
> + cf->data[6] = ioread32(&priv->regs->errc) & PCH_TEC;
> + cf->data[7] = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;
Could be handle with just *one* register access.
...
> static int pch_can_rx_poll(struct napi_struct *napi, int quota)
> {
> struct net_device *ndev = napi->dev;
> struct pch_can_priv *priv = netdev_priv(ndev);
> - struct net_device_stats *stats = &(priv->ndev->stats);
> - u32 dlc;
> u32 int_stat;
> int rcv_pkts = 0;
> u32 reg_stat;
> - unsigned long flags;
>
> int_stat = pch_can_int_pending(priv);
> if (!int_stat)
> - return 0;
> + goto end;
>
> -INT_STAT:
> - if (int_stat == CAN_STATUS_INT) {
> + if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
> reg_stat = ioread32(&priv->regs->stat);
> if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
> - if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
> + if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
> pch_can_error(ndev, reg_stat);
> + quota--;
> + }
Should be:
if (reg_stat & PCH_BUS_OFF ||
(reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
Your lec handling is still not correc, I believe. The driver needs to
write PCH_LEC_ALL to the "stat" register once in the initialization code
and then after each error observed (lec != PCH_LEC_ALL). I still do not
find such code. Could you show us the output of
"# candump any,0:0,#FFFFFFFF"
when yo send CAN messages *without* a cable connected?.
Thanks,
Wolfgang.
^ 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