[PATCH v2 0/5] ipvlan: fix ipv6 autoconfiguration

[PATCH v2 0/5] ipvlan: fix ipv6 autoconfiguration

[Konstantin Khlebnikov]

This patchset fixes ipvlan and its interaction with ipv6 RA. Now ipvlan l2
ports get dev_id and construct unique ipv6 addresses using one mac address.

Changes since v1 (http://comments.gmane.org/gmane.linux.network/363346)
* locking for ipv6 addresses fixed inside ipvlan
* rcu splat will be fixed with this: https://patchwork.ozlabs.org/patch/471481/
* new fix for trivial memory leak and patch which removes address counters

---

Konstantin Khlebnikov (5):
      ipvlan: remove counters of ipv4 and ipv6 addresses
      ipvlan: plug memory leak in ipvlan_link_delete
      ipvlan: unhash addresses without synchronize_rcu
      ipvlan: protect addresses with internal spinlock
      ipvlan: set dev_id for l2 ports to generate unique IPv6 addresses


 Documentation/networking/ipvlan.txt |   11 +++
 drivers/net/ipvlan/ipvlan.h         |   16 ++++-
 drivers/net/ipvlan/ipvlan_core.c    |    6 --
 drivers/net/ipvlan/ipvlan_main.c    |  118 +++++++++++++++++++++++------------
 4 files changed, 100 insertions(+), 51 deletions(-)

--
Konstantin

[PATCH v2 1/5] ipvlan: remove counters of ipv4 and ipv6 addresses

[Konstantin Khlebnikov]

They are unused after commit f631c44bbe15 ("ipvlan: Always set broadcast bit in
multicast filter").

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 drivers/net/ipvlan/ipvlan.h      |    2 -
 drivers/net/ipvlan/ipvlan_main.c |   65 +++++++++++++++-----------------------
 2 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 953a97492fab..68e2549c28c6 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -67,8 +67,6 @@ struct ipvl_dev {
 	struct ipvl_port	*port;
 	struct net_device	*phy_dev;
 	struct list_head	addrs;
-	int			ipv4cnt;
-	int			ipv6cnt;
 	struct ipvl_pcpu_stats	__percpu *pcpu_stats;
 	DECLARE_BITMAP(mac_filters, IPVLAN_MAC_FILTER_SIZE);
 	netdev_features_t	sfeatures;
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 1acc283160d9..62577b3f01f2 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -153,10 +153,9 @@ static int ipvlan_open(struct net_device *dev)
 	else
 		dev->flags &= ~IFF_NOARP;
 
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_add(ipvlan, addr);
-	}
+	list_for_each_entry(addr, &ipvlan->addrs, anode)
+		ipvlan_ht_addr_add(ipvlan, addr);
+
 	return dev_uc_add(phy_dev, phy_dev->dev_addr);
 }
 
@@ -171,10 +170,9 @@ static int ipvlan_stop(struct net_device *dev)
 
 	dev_uc_del(phy_dev, phy_dev->dev_addr);
 
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_del(addr, !dev->dismantle);
-	}
+	list_for_each_entry(addr, &ipvlan->addrs, anode)
+		ipvlan_ht_addr_del(addr, !dev->dismantle);
+
 	return 0;
 }
 
@@ -471,8 +469,6 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 	ipvlan->port = port;
 	ipvlan->sfeatures = IPVLAN_FEATURES;
 	INIT_LIST_HEAD(&ipvlan->addrs);
-	ipvlan->ipv4cnt = 0;
-	ipvlan->ipv6cnt = 0;
 
 	/* TODO Probably put random address here to be presented to the
 	 * world but keep using the physical-dev address for the outgoing
@@ -508,12 +504,11 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 	struct ipvl_dev *ipvlan = netdev_priv(dev);
 	struct ipvl_addr *addr, *next;
 
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
-			ipvlan_ht_addr_del(addr, !dev->dismantle);
-			list_del(&addr->anode);
-		}
+	list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
+		ipvlan_ht_addr_del(addr, !dev->dismantle);
+		list_del(&addr->anode);
 	}
+
 	list_del_rcu(&ipvlan->pnode);
 	unregister_netdevice_queue(dev, head);
 	netdev_upper_dev_unlink(ipvlan->phy_dev, dev);
@@ -627,8 +622,9 @@ static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 	memcpy(&addr->ip6addr, ip6_addr, sizeof(struct in6_addr));
 	addr->atype = IPVL_IPV6;
 	list_add_tail(&addr->anode, &ipvlan->addrs);
-	ipvlan->ipv6cnt++;
-	/* If the interface is not up, the address will be added to the hash
+
+	/*
+	 * If the interface is not up, the address will be added to the hash
 	 * list by ipvlan_open.
 	 */
 	if (netif_running(ipvlan->dev))
@@ -642,16 +638,11 @@ static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 	struct ipvl_addr *addr;
 
 	addr = ipvlan_find_addr(ipvlan, ip6_addr, true);
-	if (!addr)
-		return;
-
-	ipvlan_ht_addr_del(addr, true);
-	list_del(&addr->anode);
-	ipvlan->ipv6cnt--;
-	WARN_ON(ipvlan->ipv6cnt < 0);
-	kfree_rcu(addr, rcu);
-
-	return;
+	if (addr) {
+		ipvlan_ht_addr_del(addr, true);
+		list_del(&addr->anode);
+		kfree_rcu(addr, rcu);
+	}
 }
 
 static int ipvlan_addr6_event(struct notifier_block *unused,
@@ -699,8 +690,9 @@ static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 	memcpy(&addr->ip4addr, ip4_addr, sizeof(struct in_addr));
 	addr->atype = IPVL_IPV4;
 	list_add_tail(&addr->anode, &ipvlan->addrs);
-	ipvlan->ipv4cnt++;
-	/* If the interface is not up, the address will be added to the hash
+
+	/*
+	 * If the interface is not up, the address will be added to the hash
 	 * list by ipvlan_open.
 	 */
 	if (netif_running(ipvlan->dev))
@@ -714,16 +706,11 @@ static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 	struct ipvl_addr *addr;
 
 	addr = ipvlan_find_addr(ipvlan, ip4_addr, false);
-	if (!addr)
-		return;
-
-	ipvlan_ht_addr_del(addr, true);
-	list_del(&addr->anode);
-	ipvlan->ipv4cnt--;
-	WARN_ON(ipvlan->ipv4cnt < 0);
-	kfree_rcu(addr, rcu);
-
-	return;
+	if (addr) {
+		ipvlan_ht_addr_del(addr, true);
+		list_del(&addr->anode);
+		kfree_rcu(addr, rcu);
+	}
 }
 
 static int ipvlan_addr4_event(struct notifier_block *unused,

Re: [PATCH v2 1/5] ipvlan: remove counters of ipv4 and ipv6 addresses

[Mahesh Bandewar]

On Fri, Jul 3, 2015 at 5:58 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> They are unused after commit f631c44bbe15 ("ipvlan: Always set broadcast bit in
> multicast filter").
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>  drivers/net/ipvlan/ipvlan.h      |    2 -
>  drivers/net/ipvlan/ipvlan_main.c |   65 +++++++++++++++-----------------------
>  2 files changed, 26 insertions(+), 41 deletions(-)
>

> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 1acc283160d9..62577b3f01f2 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -627,8 +622,9 @@ static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
>         memcpy(&addr->ip6addr, ip6_addr, sizeof(struct in6_addr));
>         addr->atype = IPVL_IPV6;
>         list_add_tail(&addr->anode, &ipvlan->addrs);
> -       ipvlan->ipv6cnt++;
> -       /* If the interface is not up, the address will be added to the hash
> +
> +       /*
> +        * If the interface is not up, the address will be added to the hash
Why? Preferred commenting style in net is
      /* multi-line
       * comment
       */
>          * list by ipvlan_open.
>          */
>         if (netif_running(ipvlan->dev))
> @@ -642,16 +638,11 @@ static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
>         struct ipvl_addr *addr;
>
>         addr = ipvlan_find_addr(ipvlan, ip6_addr, true);
> -       if (!addr)
> -               return;
> -
> -       ipvlan_ht_addr_del(addr, true);
> -       list_del(&addr->anode);
> -       ipvlan->ipv6cnt--;
> -       WARN_ON(ipvlan->ipv6cnt < 0);
> -       kfree_rcu(addr, rcu);
> -
> -       return;
> +       if (addr) {
> +               ipvlan_ht_addr_del(addr, true);
> +               list_del(&addr->anode);
> +               kfree_rcu(addr, rcu);
> +       }
This delta is unnecessarily big and can be reduced to deleting just two lines.
>  }
>
>  static int ipvlan_addr6_event(struct notifier_block *unused,
> @@ -699,8 +690,9 @@ static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
>         memcpy(&addr->ip4addr, ip4_addr, sizeof(struct in_addr));
>         addr->atype = IPVL_IPV4;
>         list_add_tail(&addr->anode, &ipvlan->addrs);
> -       ipvlan->ipv4cnt++;
> -       /* If the interface is not up, the address will be added to the hash
> +
> +       /*
> +        * If the interface is not up, the address will be added to the hash
same here (multi line comments)
>          * list by ipvlan_open.
>          */
>         if (netif_running(ipvlan->dev))
> @@ -714,16 +706,11 @@ static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
>         struct ipvl_addr *addr;
>
>         addr = ipvlan_find_addr(ipvlan, ip4_addr, false);
> -       if (!addr)
> -               return;
> -
> -       ipvlan_ht_addr_del(addr, true);
> -       list_del(&addr->anode);
> -       ipvlan->ipv4cnt--;
> -       WARN_ON(ipvlan->ipv4cnt < 0);
> -       kfree_rcu(addr, rcu);
> -
> -       return;
> +       if (addr) {
> +               ipvlan_ht_addr_del(addr, true);
> +               list_del(&addr->anode);
> +               kfree_rcu(addr, rcu);
> +       }
This delta is unnecessarily big and can be reduced to deleting just two lines.
>  }
>
>  static int ipvlan_addr4_event(struct notifier_block *unused,
>

[PATCH v2 2/5] ipvlan: plug memory leak in ipvlan_link_delete

[Konstantin Khlebnikov]

Add missing kfree_rcu(addr, rcu);

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 drivers/net/ipvlan/ipvlan_main.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 62577b3f01f2..4c3a0ac85381 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -507,6 +507,7 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 	list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
 		ipvlan_ht_addr_del(addr, !dev->dismantle);
 		list_del(&addr->anode);
+		kfree_rcu(addr, rcu);
 	}
 
 	list_del_rcu(&ipvlan->pnode);

[PATCH v2 3/5] ipvlan: unhash addresses without synchronize_rcu

[Konstantin Khlebnikov]

All structures used in traffic forwarding are rcu-protected:
ipvl_addr, ipvl_dev and ipvl_port. Thus we can unhash addresses
without synchronization. We'll anyway hash it back into the same
bucket, in worst case lockless lookup will scan hash once again.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 drivers/net/ipvlan/ipvlan.h      |    2 +-
 drivers/net/ipvlan/ipvlan_core.c |    4 +---
 drivers/net/ipvlan/ipvlan_main.c |    8 ++++----
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 68e2549c28c6..40f9d7e4a0ea 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -122,5 +122,5 @@ struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
 bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6);
 struct ipvl_addr *ipvlan_ht_addr_lookup(const struct ipvl_port *port,
 					const void *iaddr, bool is_v6);
-void ipvlan_ht_addr_del(struct ipvl_addr *addr, bool sync);
+void ipvlan_ht_addr_del(struct ipvl_addr *addr);
 #endif /* __IPVLAN_H */
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 8afbedad620d..8f8628a0adba 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -85,11 +85,9 @@ void ipvlan_ht_addr_add(struct ipvl_dev *ipvlan, struct ipvl_addr *addr)
 		hlist_add_head_rcu(&addr->hlnode, &port->hlhead[hash]);
 }
 
-void ipvlan_ht_addr_del(struct ipvl_addr *addr, bool sync)
+void ipvlan_ht_addr_del(struct ipvl_addr *addr)
 {
 	hlist_del_init_rcu(&addr->hlnode);
-	if (sync)
-		synchronize_rcu();
 }
 
 struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 4c3a0ac85381..c7172b6277e2 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -171,7 +171,7 @@ 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)
-		ipvlan_ht_addr_del(addr, !dev->dismantle);
+		ipvlan_ht_addr_del(addr);
 
 	return 0;
 }
@@ -505,7 +505,7 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 	struct ipvl_addr *addr, *next;
 
 	list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
-		ipvlan_ht_addr_del(addr, !dev->dismantle);
+		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}
@@ -640,7 +640,7 @@ static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 
 	addr = ipvlan_find_addr(ipvlan, ip6_addr, true);
 	if (addr) {
-		ipvlan_ht_addr_del(addr, true);
+		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}
@@ -708,7 +708,7 @@ static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 
 	addr = ipvlan_find_addr(ipvlan, ip4_addr, false);
 	if (addr) {
-		ipvlan_ht_addr_del(addr, true);
+		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}

[PATCH v2 4/5] ipvlan: protect addresses with internal spinlock

[Konstantin Khlebnikov]

Inet6addr notifier is atomic and runs in bh context without RTNL when
ipv6 receives router advertisement packet and performs autoconfiguration.

This patch adds ipvl_port->addr_lock and helpers: ipvlan_addr_lock_bh,
ipvlan_addr_unlock_bh for protecting ipvlan addresses and hash table.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 drivers/net/ipvlan/ipvlan.h      |   11 +++++++++++
 drivers/net/ipvlan/ipvlan_core.c |    2 --
 drivers/net/ipvlan/ipvlan_main.c |   33 ++++++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 40f9d7e4a0ea..a23069aec4d9 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -97,6 +97,7 @@ struct ipvl_port {
 	struct sk_buff_head	backlog;
 	int			count;
 	u16			mode;
+	spinlock_t		addr_lock;
 };
 
 static inline struct ipvl_port *ipvlan_port_get_rcu(const struct net_device *d)
@@ -109,6 +110,16 @@ static inline struct ipvl_port *ipvlan_port_get_rtnl(const struct net_device *d)
 	return rtnl_dereference(d->rx_handler_data);
 }
 
+static inline void ipvlan_addr_lock_bh(struct ipvl_dev *ipvlan)
+{
+	spin_lock_bh(&ipvlan->port->addr_lock);
+}
+
+static inline void ipvlan_addr_unlock_bh(struct ipvl_dev *ipvlan)
+{
+	spin_unlock_bh(&ipvlan->port->addr_lock);
+}
+
 void ipvlan_adjust_mtu(struct ipvl_dev *ipvlan, struct net_device *dev);
 void ipvlan_set_port_mode(struct ipvl_port *port, u32 nval);
 void ipvlan_init_secret(void);
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 8f8628a0adba..e7b6359814d0 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -109,8 +109,6 @@ bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6)
 {
 	struct ipvl_dev *ipvlan;
 
-	ASSERT_RTNL();
-
 	list_for_each_entry(ipvlan, &port->ipvlans, pnode) {
 		if (ipvlan_find_addr(ipvlan, iaddr, is_v6))
 			return true;
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index c7172b6277e2..83a936f76248 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -56,6 +56,7 @@ static int ipvlan_port_create(struct net_device *dev)
 
 	skb_queue_head_init(&port->backlog);
 	INIT_WORK(&port->wq, ipvlan_process_multicast);
+	spin_lock_init(&port->addr_lock);
 
 	err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port);
 	if (err)
@@ -153,8 +154,10 @@ static int ipvlan_open(struct net_device *dev)
 	else
 		dev->flags &= ~IFF_NOARP;
 
+	ipvlan_addr_lock_bh(ipvlan);
 	list_for_each_entry(addr, &ipvlan->addrs, anode)
 		ipvlan_ht_addr_add(ipvlan, addr);
+	ipvlan_addr_unlock_bh(ipvlan);
 
 	return dev_uc_add(phy_dev, phy_dev->dev_addr);
 }
@@ -170,8 +173,10 @@ static int ipvlan_stop(struct net_device *dev)
 
 	dev_uc_del(phy_dev, phy_dev->dev_addr);
 
+	ipvlan_addr_lock_bh(ipvlan);
 	list_for_each_entry(addr, &ipvlan->addrs, anode)
 		ipvlan_ht_addr_del(addr);
+	ipvlan_addr_unlock_bh(ipvlan);
 
 	return 0;
 }
@@ -504,11 +509,13 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 	struct ipvl_dev *ipvlan = netdev_priv(dev);
 	struct ipvl_addr *addr, *next;
 
+	ipvlan_addr_lock_bh(ipvlan);
 	list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
 		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}
+	ipvlan_addr_unlock_bh(ipvlan);
 
 	list_del_rcu(&ipvlan->pnode);
 	unregister_netdevice_queue(dev, head);
@@ -609,15 +616,21 @@ static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 {
 	struct ipvl_addr *addr;
 
+	ipvlan_addr_lock_bh(ipvlan);
+
 	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);
+		ipvlan_addr_unlock_bh(ipvlan);
 		return -EINVAL;
 	}
+
 	addr = kzalloc(sizeof(struct ipvl_addr), GFP_ATOMIC);
-	if (!addr)
+	if (!addr) {
+		ipvlan_addr_unlock_bh(ipvlan);
 		return -ENOMEM;
+	}
 
 	addr->master = ipvlan;
 	memcpy(&addr->ip6addr, ip6_addr, sizeof(struct in6_addr));
@@ -631,6 +644,8 @@ static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 	if (netif_running(ipvlan->dev))
 		ipvlan_ht_addr_add(ipvlan, addr);
 
+	ipvlan_addr_unlock_bh(ipvlan);
+
 	return 0;
 }
 
@@ -638,12 +653,14 @@ static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 {
 	struct ipvl_addr *addr;
 
+	ipvlan_addr_lock_bh(ipvlan);
 	addr = ipvlan_find_addr(ipvlan, ip6_addr, true);
 	if (addr) {
 		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}
+	ipvlan_addr_unlock_bh(ipvlan);
 }
 
 static int ipvlan_addr6_event(struct notifier_block *unused,
@@ -677,15 +694,21 @@ static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 {
 	struct ipvl_addr *addr;
 
+	ipvlan_addr_lock_bh(ipvlan);
+
 	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);
+		ipvlan_addr_unlock_bh(ipvlan);
 		return -EINVAL;
 	}
-	addr = kzalloc(sizeof(struct ipvl_addr), GFP_KERNEL);
-	if (!addr)
+
+	addr = kzalloc(sizeof(struct ipvl_addr), GFP_ATOMIC);
+	if (!addr) {
+		ipvlan_addr_unlock_bh(ipvlan);
 		return -ENOMEM;
+	}
 
 	addr->master = ipvlan;
 	memcpy(&addr->ip4addr, ip4_addr, sizeof(struct in_addr));
@@ -699,6 +722,8 @@ static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 	if (netif_running(ipvlan->dev))
 		ipvlan_ht_addr_add(ipvlan, addr);
 
+	ipvlan_addr_unlock_bh(ipvlan);
+
 	return 0;
 }
 
@@ -706,12 +731,14 @@ static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
 {
 	struct ipvl_addr *addr;
 
+	ipvlan_addr_lock_bh(ipvlan);
 	addr = ipvlan_find_addr(ipvlan, ip4_addr, false);
 	if (addr) {
 		ipvlan_ht_addr_del(addr);
 		list_del(&addr->anode);
 		kfree_rcu(addr, rcu);
 	}
+	ipvlan_addr_unlock_bh(ipvlan);
 }
 
 static int ipvlan_addr4_event(struct notifier_block *unused,

Re: [PATCH v2 4/5] ipvlan: protect addresses with internal spinlock

[Mahesh Bandewar]

On Fri, Jul 3, 2015 at 5:58 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> Inet6addr notifier is atomic and runs in bh context without RTNL when
> ipv6 receives router advertisement packet and performs autoconfiguration.
>
> This patch adds ipvl_port->addr_lock and helpers: ipvlan_addr_lock_bh,
> ipvlan_addr_unlock_bh for protecting ipvlan addresses and hash table.
>
Frankly I'm not comfortable adding spin-locks all over. I think any
config that mostly takes place with RTNL makes sense but this
inet6addr needs to be thought through and implanting spin-locks in
IPvlan is a work-around for a problem some where else.
Why can't a work-queue that takes RTNL to call inet6addr-notifier be
implemented when called from bh?

> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>  drivers/net/ipvlan/ipvlan.h      |   11 +++++++++++
>  drivers/net/ipvlan/ipvlan_core.c |    2 --
>  drivers/net/ipvlan/ipvlan_main.c |   33 ++++++++++++++++++++++++++++++---
>  3 files changed, 41 insertions(+), 5 deletions(-)
>
[snip]

Re: [PATCH v2 4/5] ipvlan: protect addresses with internal spinlock

[Konstantin Khlebnikov]

On 08.07.2015 07:05, Mahesh Bandewar wrote:
> On Fri, Jul 3, 2015 at 5:58 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>> Inet6addr notifier is atomic and runs in bh context without RTNL when
>> ipv6 receives router advertisement packet and performs autoconfiguration.
>>
>> This patch adds ipvl_port->addr_lock and helpers: ipvlan_addr_lock_bh,
>> ipvlan_addr_unlock_bh for protecting ipvlan addresses and hash table.
>>
> Frankly I'm not comfortable adding spin-locks all over. I think any
> config that mostly takes place with RTNL makes sense but this
> inet6addr needs to be thought through and implanting spin-locks in
> IPvlan is a work-around for a problem some where else.
> Why can't a work-queue that takes RTNL to call inet6addr-notifier be
> implemented when called from bh?

We already have that work: ipv6 DAD. In v1 patchset was a patch
which moves inet6addr-notifier into it and makes it blockable:

http://www.spinics.net/lists/netdev/msg329017.html

But David Miller objected.

I'm ok with any solution: when spinlock nests inside locked RTNL it
never spins in most cases.

>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> ---
>>   drivers/net/ipvlan/ipvlan.h      |   11 +++++++++++
>>   drivers/net/ipvlan/ipvlan_core.c |    2 --
>>   drivers/net/ipvlan/ipvlan_main.c |   33 ++++++++++++++++++++++++++++++---
>>   3 files changed, 41 insertions(+), 5 deletions(-)
>>
> [snip]
>


-- 
Konstantin

[PATCH v2 5/5] ipvlan: set dev_id for l2 ports to generate unique IPv6 addresses

[Konstantin Khlebnikov]

All ipvlan ports use one MAC address, this way ipv6 RA tries to assign
one ipv6 address to all of them. This patch assigns unique dev_id to each
ipvlan port. This field is used instead of common FF-FE in Modified EUI-64.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 Documentation/networking/ipvlan.txt |   11 ++++++++++-
 drivers/net/ipvlan/ipvlan.h         |    1 +
 drivers/net/ipvlan/ipvlan_main.c    |   19 +++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ipvlan.txt b/Documentation/networking/ipvlan.txt
index cf996394e466..d1123de81469 100644
--- a/Documentation/networking/ipvlan.txt
+++ b/Documentation/networking/ipvlan.txt
@@ -24,7 +24,7 @@ using IProute2/ip utility.
 
 	ip link add link <master-dev> <slave-dev> type ipvlan mode { l2 | L3 }
 
-	e.g. ip link add link ipvl0 eth0 type ipvlan mode l2
+	e.g. ip link add link eth0 ipvl0 type ipvlan mode l2
 
 
 4. Operating modes:
@@ -41,6 +41,15 @@ slave device and packets are switched and queued to the master device to send
 out. In this mode the slaves will RX/TX multicast and broadcast (if applicable)
 as well.
 
+	In L2 mode slave devices receive Router Advertisements from the network
+and perform autoconfiguration as well as master device. Each port has unique
+16-bit device id which is used for filling octets 4-5 of Modified EUI-64.
+That gives 65533 addresses (FF-FE used by master, FF-FF/00-00 reserved/not used).
+
+	Also lower half of IPv6 address could be set as interface token:
+
+	ip token set ::aaaa:bbbb:cccc:dddd dev ipvl0
+
 4.2 L3 mode:
 	In this mode TX processing upto L3 happens on the stack instance attached
 to the slave device and packets are switched to the stack instance of the
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index a23069aec4d9..0d1f5e8ed75f 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -98,6 +98,7 @@ struct ipvl_port {
 	int			count;
 	u16			mode;
 	spinlock_t		addr_lock;
+	struct ida		ida;
 };
 
 static inline struct ipvl_port *ipvlan_port_get_rcu(const struct net_device *d)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 83a936f76248..d465b2c287cb 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -53,6 +53,7 @@ static int ipvlan_port_create(struct net_device *dev)
 	INIT_LIST_HEAD(&port->ipvlans);
 	for (idx = 0; idx < IPVLAN_HASH_SIZE; idx++)
 		INIT_HLIST_HEAD(&port->hlhead[idx]);
+	ida_init(&port->ida);
 
 	skb_queue_head_init(&port->backlog);
 	INIT_WORK(&port->wq, ipvlan_process_multicast);
@@ -78,6 +79,7 @@ static void ipvlan_port_destroy(struct net_device *dev)
 	netdev_rx_handler_unregister(dev);
 	cancel_work_sync(&port->wq);
 	__skb_queue_purge(&port->backlog);
+	ida_destroy(&port->ida);
 	kfree_rcu(port, rcu);
 }
 
@@ -481,6 +483,18 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 	 */
 	memcpy(dev->dev_addr, phy_dev->dev_addr, ETH_ALEN);
 
+	if (port->mode == IPVLAN_MODE_L2) {
+		/*
+		 * IPv6 addrconf uses it to produce unique addresses,
+		 * see function addrconf_ifid_eui48.
+		 */
+		err = ida_simple_get(&port->ida, 1, 0xFFFE, GFP_KERNEL);
+		if (err > 0)
+			dev->dev_id = err;
+		else if (err != -ENOSPC)
+			goto ipvlan_destroy_port;
+	}
+
 	dev->priv_flags |= IFF_IPVLAN_SLAVE;
 
 	port->count += 1;
@@ -517,6 +531,11 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 	}
 	ipvlan_addr_unlock_bh(ipvlan);
 
+	if (dev->dev_id) {
+		ida_simple_remove(&ipvlan->port->ida, dev->dev_id);
+		dev->dev_id = 0;
+	}
+
 	list_del_rcu(&ipvlan->pnode);
 	unregister_netdevice_queue(dev, head);
 	netdev_upper_dev_unlink(ipvlan->phy_dev, dev);