ipvlan: use per device spinlock to protect addrs list updates

This changeset moves ipvlan address under RCU protection, using
a per ipvlan device spinlock to protect list mutation and RCU
read access to protect list traversal.

Also explicitly use RCU read lock to traverse the per port
ipvlans list, so that we can now perform a full address lookup
without asserting the RTNL lock.

Overall this allows the ipvlan driver to check fully for duplicate
addresses - before this commit ipv6 addresses assigned by autoconf
via prefix delegation where accepted without any check - and avoid
the following rntl assertion failure still in the same code path:

 RTNL: assertion failed at drivers/net/ipvlan/ipvlan_core.c (124)
 WARNING: CPU: 15 PID: 0 at drivers/net/ipvlan/ipvlan_core.c:124 ipvlan_addr_busy+0x97/0xa0 [ipvlan]
 Modules linked in: ipvlan(E) ixgbe
 CPU: 15 PID: 0 Comm: swapper/15 Tainted: G            E    4.16.0-rc2.ipvlan+ #1782
 Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016
 RIP: 0010:ipvlan_addr_busy+0x97/0xa0 [ipvlan]
 RSP: 0018:ffff881ff9e03768 EFLAGS: 00010286
 RAX: 0000000000000000 RBX: ffff881fdf2a9000 RCX: 0000000000000000
 RDX: 0000000000000001 RSI: 00000000000000f6 RDI: 0000000000000300
 RBP: ffff881fdf2a8000 R08: 0000000000000000 R09: 0000000000000000
 R10: 0000000000000001 R11: ffff881ff9e034c0 R12: ffff881fe07bcc00
 R13: 0000000000000001 R14: ffffffffa02002b0 R15: 0000000000000001
 FS:  0000000000000000(0000) GS:ffff881ff9e00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00007fc5c1a4f248 CR3: 000000207e012005 CR4: 00000000001606e0
 Call Trace:
  <IRQ>
  ipvlan_addr6_event+0x6c/0xd0 [ipvlan]
  notifier_call_chain+0x49/0x90
  atomic_notifier_call_chain+0x6a/0x100
  ipv6_add_addr+0x5f9/0x720
  addrconf_prefix_rcv_add_addr+0x244/0x3c0
  addrconf_prefix_rcv+0x2f3/0x790
  ndisc_router_discovery+0x633/0xb70
  ndisc_rcv+0x155/0x180
  icmpv6_rcv+0x4ac/0x5f0
  ip6_input_finish+0x138/0x6a0
  ip6_input+0x41/0x1f0
  ipv6_rcv+0x4db/0x8d0
  __netif_receive_skb_core+0x3d5/0xe40
  netif_receive_skb_internal+0x89/0x370
  napi_gro_receive+0x14f/0x1e0
  ixgbe_clean_rx_irq+0x4ce/0x1020 [ixgbe]
  ixgbe_poll+0x31a/0x7a0 [ixgbe]
  net_rx_action+0x296/0x4f0
  __do_softirq+0xcf/0x4f5
  irq_exit+0xf5/0x110
  do_IRQ+0x62/0x110
  common_interrupt+0x91/0x91
  </IRQ>

 v1 -> v2: drop unneeded in_softirq check in ipvlan_addr6_validator_event()

Fixes: e9997c2938b2 ("ipvlan: fix check for IP addresses in control path")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 5166575..a115f12 100644 (file)
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -74,6 +74,7 @@ struct ipvl_dev {
        DECLARE_BITMAP(mac_filters, IPVLAN_MAC_FILTER_SIZE);
        netdev_features_t       sfeatures;
        u32                     msg_enable;
+       spinlock_t              addrs_lock;
 };
 
 struct ipvl_addr {

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index dbbb884..17daebd 100644 (file)
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -109,25 +109,33 @@ void ipvlan_ht_addr_del(struct ipvl_addr *addr)
 struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
                                   const void *iaddr, bool is_v6)
 {
-       struct ipvl_addr *addr;
+       struct ipvl_addr *addr, *ret = NULL;
 
-       list_for_each_entry(addr, &ipvlan->addrs, anode)
-               if (addr_equal(is_v6, addr, iaddr))
-                       return addr;
-       return NULL;
+       rcu_read_lock();
+       list_for_each_entry_rcu(addr, &ipvlan->addrs, anode) {
+               if (addr_equal(is_v6, addr, iaddr)) {
+                       ret = addr;
+                       break;
+               }
+       }
+       rcu_read_unlock();
+       return ret;
 }
 
 bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6)
 {
        struct ipvl_dev *ipvlan;
+       bool ret = false;
 
-       ASSERT_RTNL();
-
-       list_for_each_entry(ipvlan, &port->ipvlans, pnode) {
-               if (ipvlan_find_addr(ipvlan, iaddr, is_v6))
-                       return true;
+       rcu_read_lock();
+       list_for_each_entry_rcu(ipvlan, &port->ipvlans, pnode) {
+               if (ipvlan_find_addr(ipvlan, iaddr, is_v6)) {
+                       ret = true;
+                       break;
+               }
        }
-       return false;
+       rcu_read_unlock();
+       return ret;
 }
 
 static void *ipvlan_get_L3_hdr(struct ipvl_port *port, struct sk_buff *skb, int *type)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index d05b902..3efc1c9 100644 (file)
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -227,8 +227,10 @@ static int ipvlan_open(struct net_device *dev)
        else
                dev->flags &= ~IFF_NOARP;
 
-       list_for_each_entry(addr, &ipvlan->addrs, anode)
+       rcu_read_lock();
+       list_for_each_entry_rcu(addr, &ipvlan->addrs, anode)
                ipvlan_ht_addr_add(ipvlan, addr);
+       rcu_read_unlock();
 
        return dev_uc_add(phy_dev, phy_dev->dev_addr);
 }
@@ -244,8 +246,10 @@ static int ipvlan_stop(struct net_device *dev)
 
        dev_uc_del(phy_dev, phy_dev->dev_addr);
 
-       list_for_each_entry(addr, &ipvlan->addrs, anode)
+       rcu_read_lock();
+       list_for_each_entry_rcu(addr, &ipvlan->addrs, anode)
                ipvlan_ht_addr_del(addr);
+       rcu_read_unlock();
 
        return 0;
 }
@@ -588,6 +592,7 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
        ipvlan->sfeatures = IPVLAN_FEATURES;
        ipvlan_adjust_mtu(ipvlan, phy_dev);
        INIT_LIST_HEAD(&ipvlan->addrs);
+       spin_lock_init(&ipvlan->addrs_lock);
 
        /* TODO Probably put random address here to be presented to the
         * world but keep using the physical-dev address for the outgoing
@@ -665,11 +670,13 @@ void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
        struct ipvl_dev *ipvlan = netdev_priv(dev);
        struct ipvl_addr *addr, *next;
 
+       spin_lock_bh(&ipvlan->addrs_lock);
        list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
                ipvlan_ht_addr_del(addr);
-               list_del(&addr->anode);
+               list_del_rcu(&addr->anode);
                kfree_rcu(addr, rcu);
        }
+       spin_unlock_bh(&ipvlan->addrs_lock);
 
        ida_simple_remove(&ipvlan->port->ida, dev->dev_id);
        list_del_rcu(&ipvlan->pnode);
@@ -760,8 +767,7 @@ static int ipvlan_device_event(struct notifier_block *unused,
                if (dev->reg_state != NETREG_UNREGISTERING)
                        break;
 
-               list_for_each_entry_safe(ipvlan, next, &port->ipvlans,
-                                        pnode)
+               list_for_each_entry_safe(ipvlan, next, &port->ipvlans, pnode)
                        ipvlan->dev->rtnl_link_ops->dellink(ipvlan->dev,
                                                            &lst_kill);
                unregister_netdevice_many(&lst_kill);
@@ -793,6 +799,7 @@ static int ipvlan_device_event(struct notifier_block *unused,
        return NOTIFY_DONE;
 }
 
+/* the caller must held the addrs lock */
 static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
 {
        struct ipvl_addr *addr;
@@ -811,7 +818,8 @@ static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
                addr->atype = IPVL_IPV6;
 #endif
        }
-       list_add_tail(&addr->anode, &ipvlan->addrs);
+
+       list_add_tail_rcu(&addr->anode, &ipvlan->addrs);
 
        /* If the interface is not up, the address will be added to the hash
         * list by ipvlan_open.
@@ -826,15 +834,17 @@ static void ipvlan_del_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
 {
        struct ipvl_addr *addr;
 
+       spin_lock_bh(&ipvlan->addrs_lock);
        addr = ipvlan_find_addr(ipvlan, iaddr, is_v6);
-       if (!addr)
+       if (!addr) {
+               spin_unlock_bh(&ipvlan->addrs_lock);
                return;
+       }
 
        ipvlan_ht_addr_del(addr);
-       list_del(&addr->anode);
+       list_del_rcu(&addr->anode);
+       spin_unlock_bh(&ipvlan->addrs_lock);
        kfree_rcu(addr, rcu);
-
-       return;
 }
 
 static bool ipvlan_is_valid_dev(const struct net_device *dev)
@@ -853,14 +863,17 @@ static bool ipvlan_is_valid_dev(const struct net_device *dev)
 #if IS_ENABLED(CONFIG_IPV6)
 static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 {
-       if (ipvlan_addr_busy(ipvlan->port, ip6_addr, true)) {
+       int ret = -EINVAL;
+
+       spin_lock_bh(&ipvlan->addrs_lock);
+       if (ipvlan_addr_busy(ipvlan->port, ip6_addr, true))
                netif_err(ipvlan, ifup, ipvlan->dev,
                          "Failed to add IPv6=%pI6c addr for %s intf\n",
                          ip6_addr, ipvlan->dev->name);
-               return -EINVAL;
-       }
-
-       return ipvlan_add_addr(ipvlan, ip6_addr, true);
+       else
+               ret = ipvlan_add_addr(ipvlan, ip6_addr, true);
+       spin_unlock_bh(&ipvlan->addrs_lock);
+       return ret;
 }
 
 static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
@@ -899,10 +912,6 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
        struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
        struct ipvl_dev *ipvlan = netdev_priv(dev);
 
-       /* FIXME IPv6 autoconf calls us from bh without RTNL */
-       if (in_softirq())
-               return NOTIFY_DONE;
-
        if (!ipvlan_is_valid_dev(dev))
                return NOTIFY_DONE;
 
@@ -922,14 +931,17 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
 
 static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 {
-       if (ipvlan_addr_busy(ipvlan->port, ip4_addr, false)) {
+       int ret = -EINVAL;
+
+       spin_lock_bh(&ipvlan->addrs_lock);
+       if (ipvlan_addr_busy(ipvlan->port, ip4_addr, false))
                netif_err(ipvlan, ifup, ipvlan->dev,
                          "Failed to add IPv4=%pI4 on %s intf.\n",
                          ip4_addr, ipvlan->dev->name);
-               return -EINVAL;
-       }
-
-       return ipvlan_add_addr(ipvlan, ip4_addr, false);
+       else
+               ret = ipvlan_add_addr(ipvlan, ip4_addr, false);
+       spin_unlock_bh(&ipvlan->addrs_lock);
+       return ret;
 }
 
 static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)