| Message ID | 20100728095730.11450561@nehalam |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Stephen Hemminger <shemminger@vyatta.com> Date: Wed, 28 Jul 2010 09:57:30 -0700 > Long ago, when bridge was converted to RCU, rcu lock was equivalent > to having preempt disabled. RCU has changed a lot since then and > bridge code was still assuming the since transmit was called with > bottom half disabled, it was RCU safe. > > In addition to fixing the code, update the comments about > locking to match current state as well. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> As I stated in another email I added the original commit. This version still didn't apply to net-2.6 and also you failed to add a tested-by tag for Johannes to the commit message, which I also did alongside backporting the original patch to net-2.6 -- 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
On Wed, 28 Jul 2010 10:52:09 -0700 (PDT) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Wed, 28 Jul 2010 09:57:30 -0700 > > > Long ago, when bridge was converted to RCU, rcu lock was equivalent > > to having preempt disabled. RCU has changed a lot since then and > > bridge code was still assuming the since transmit was called with > > bottom half disabled, it was RCU safe. > > > > In addition to fixing the code, update the comments about > > locking to match current state as well. > > > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > As I stated in another email I added the original commit. > > This version still didn't apply to net-2.6 and also you failed > to add a tested-by tag for Johannes to the commit message, which > I also did alongside backporting the original patch to net-2.6 I didn't see yours, if you look at the date it was ships in the night.
--- a/net/bridge/br_device.c 2010-07-27 08:57:43.169399349 -0700 +++ b/net/bridge/br_device.c 2010-07-27 13:57:20.278665500 -0700 @@ -22,7 +22,7 @@ #include <asm/uaccess.h> #include "br_private.h" -/* net device transmit always called with no BH (preempt_disabled) */ +/* net device transmit always called with BH disabled */ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) { struct net_bridge *br = netdev_priv(dev); @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * skb_reset_mac_header(skb); skb_pull(skb, ETH_HLEN); + rcu_read_lock(); if (is_multicast_ether_addr(dest)) { if (unlikely(netpoll_tx_running(dev))) { br_flood_deliver(br, skb); @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff * br_flood_deliver(br, skb); out: + rcu_read_unlock(); return NETDEV_TX_OK; } --- a/net/bridge/br_fdb.c 2010-07-27 11:18:30.815320981 -0700 +++ b/net/bridge/br_fdb.c 2010-07-27 11:18:59.597710975 -0700 @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br spin_unlock_bh(&br->hash_lock); } -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */ +/* No locking or refcounting, assumes caller has rcu_read_lock */ struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br, const unsigned char *addr) { --- a/net/bridge/br_input.c 2010-07-27 11:19:05.266181492 -0700 +++ b/net/bridge/br_input.c 2010-07-27 11:19:29.148163149 -0700 @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu netif_receive_skb); } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ int br_handle_frame_finish(struct sk_buff *skb) { const unsigned char *dest = eth_hdr(skb)->h_dest; @@ -110,7 +110,7 @@ drop: goto out; } -/* note: already called with rcu_read_lock (preempt_disabled) */ +/* note: already called with rcu_read_lock */ static int br_handle_local_finish(struct sk_buff *skb) { struct net_bridge_port *p = br_port_get_rcu(skb->dev); @@ -133,8 +133,7 @@ static inline int is_link_local(const un /* * Return NULL if skb is handled - * note: already called with rcu_read_lock (preempt_disabled) from - * netif_receive_skb + * note: already called with rcu_read_lock from netif_receive_skb */ struct sk_buff *br_handle_frame(struct sk_buff *skb) { --- a/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:34.092573294 -0700 +++ b/net/bridge/br_stp_bpdu.c 2010-07-27 11:19:40.725123403 -0700 @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_ /* * Called from llc. * - * NO locks, but rcu_read_lock (preempt_disabled) + * NO locks, but rcu_read_lock */ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, struct net_device *dev)
Long ago, when bridge was converted to RCU, rcu lock was equivalent to having preempt disabled. RCU has changed a lot since then and bridge code was still assuming the since transmit was called with bottom half disabled, it was RCU safe. In addition to fixing the code, update the comments about locking to match current state as well. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> --- net/bridge/br_device.c | 4 +++- net/bridge/br_fdb.c | 2 +- net/bridge/br_input.c | 7 +++---- net/bridge/br_stp_bpdu.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) -- 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