[PATCH 0/3] ipvlan: fix ipv6 autoconfiguration via RA

[PATCH 0/3] ipvlan: fix ipv6 autoconfiguration via RA

[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.

---

Konstantin Khlebnikov (3):
      ipv6: make inet6addr_chain blocking and always call with rtnl locked
      ipvlan: grab rcu_read_lock on xmit path
      ipvlan: set dev_id for l2 ports to generate unique IPv6 addresses


 Documentation/networking/ipvlan.txt |   12 +++++++++++-
 drivers/net/ipvlan/ipvlan.h         |    1 +
 drivers/net/ipvlan/ipvlan_main.c    |   24 ++++++++++++++++++++++++
 net/ipv6/addrconf.c                 |    7 ++++---
 net/ipv6/addrconf_core.c            |    8 ++++----
 5 files changed, 44 insertions(+), 8 deletions(-)

--
Konstantin

[PATCH 1/3] ipv6: make inet6addr_chain blocking and always call with rtnl locked

[Konstantin Khlebnikov]

Unlike to inetaddr_chain inet6addr_chain is atomic and called from bh
context without rtnl when ipv6 receives router advertisement packet.

Several drivers don't know about that: ipvlan thinks that it has rtnl
here, ocrdma locks mutex inside callback. Probably there is more.

This patch makes it blocking and calls from first stage of DAD work.
Looks like this is completely safe and rtnl already locked here.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

---

Splash in ipvlan when ipv6 addrconf got RA:

[   44.673784] RTNL: assertion failed at drivers/net/ipvlan/ipvlan_core.c (114)
[   44.674761] CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.1.0-rc3+ #69
[   44.674763] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011
[   44.674765]  ffff880216273000 ffff88021613b7e8 ffffffff818c6054 ffff88021fc10c10
[   44.674767]  0000000000000000 ffff88021613b818 ffffffff815544e8 0000000000000000
[   44.674769]  ffff8800db5d0000 ffff880216270000 ffff8802162707c0 ffff88021613b848
[   44.674771] Call Trace:
[   44.674787]  [<ffffffff818c6054>] dump_stack+0x45/0x57
[   44.674800]  [<ffffffff815544e8>] ipvlan_addr_busy+0x98/0xa0
[   44.674802]  [<ffffffff81555755>] ipvlan_addr6_event+0x115/0x200
[   44.674810]  [<ffffffff810733fd>] notifier_call_chain+0x4d/0x70
[   44.674812]  [<ffffffff81073445>] atomic_notifier_call_chain+0x15/0x20
[   44.674829]  [<ffffffff817dbb16>] inet6addr_notifier_call_chain+0x16/0x20
[   44.674836]  [<ffffffff817a46c0>] ipv6_add_addr+0xa0/0x420
[   44.674838]  [<ffffffff817aa058>] addrconf_prefix_rcv+0x568/0x7d0
[   44.674842]  [<ffffffff817b7bad>] ndisc_rcv+0x8ad/0xf40
[   44.674845]  [<ffffffff817bec10>] icmpv6_rcv+0x430/0x870
[   44.674847]  [<ffffffff817dbd56>] ? ipv6_skip_exthdr+0x46/0x170
[   44.674850]  [<ffffffff818cfe79>] ? _raw_read_unlock_bh+0x19/0x20
[   44.674852]  [<ffffffff817c33f0>] ? ipv6_chk_mcast_addr+0x110/0x130
[   44.674854]  [<ffffffff817a1ffb>] ip6_input_finish+0x11b/0x3e0
[   44.674855]  [<ffffffff817a281a>] ip6_input+0x6a/0x80
[   44.674857]  [<ffffffff817c330a>] ? ipv6_chk_mcast_addr+0x2a/0x130
[   44.674859]  [<ffffffff817a1ee0>] ? ip6_rcv_finish+0xa0/0xa0
[   44.674860]  [<ffffffff817a28c0>] ip6_mc_input+0x90/0xb0
[   44.674861]  [<ffffffff817a1edd>] ip6_rcv_finish+0x9d/0xa0
[   44.674863]  [<ffffffff817a2602>] ipv6_rcv+0x342/0x4f0
[   44.674864]  [<ffffffff817a1e40>] ? ip6_make_skb+0x1a0/0x1a0
[   44.674873]  [<ffffffff816dc876>] __netif_receive_skb_core+0x216/0x900
[   44.674877]  [<ffffffff8155ce00>] ? virtnet_receive+0x170/0x750
[   44.674879]  [<ffffffff816dcf81>] __netif_receive_skb+0x21/0x70
[   44.674880]  [<ffffffff816dd06d>] process_backlog+0x9d/0x140
[   44.674882]  [<ffffffff816dd896>] net_rx_action+0x146/0x330
[   44.674884]  [<ffffffff81089baa>] ? pick_next_task_fair+0x47a/0x490
[   44.674887]  [<ffffffff81059e37>] __do_softirq+0xe7/0x2e0
[   44.674888]  [<ffffffff8105a04b>] run_ksoftirqd+0x1b/0x60
[   44.674890]  [<ffffffff810758c6>] smpboot_thread_fn+0x116/0x170
[   44.674891]  [<ffffffff810757b0>] ? sort_range+0x20/0x20
[   44.674893]  [<ffffffff81072924>] kthread+0xc4/0xe0
[   44.674895]  [<ffffffff81000303>] ? do_one_initcall+0xb3/0x1c0
[   44.674897]  [<ffffffff81072860>] ? flush_kthread_worker+0x90/0x90
[   44.674899]  [<ffffffff818d07e2>] ret_from_fork+0x42/0x70
[   44.674901]  [<ffffffff81072860>] ? flush_kthread_worker+0x90/0x90
---
 net/ipv6/addrconf.c      |    7 ++++---
 net/ipv6/addrconf_core.c |    8 ++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 37b70e82bff8..61512e10ec92 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -910,9 +910,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 out2:
 	rcu_read_unlock_bh();
 
-	if (likely(err == 0))
-		inet6addr_notifier_call_chain(NETDEV_UP, ifa);
-	else {
+	if (err) {
 		kfree(ifa);
 		ifa = ERR_PTR(err);
 	}
@@ -2735,6 +2733,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 		spin_lock_bh(&ifp->lock);
 		ifp->flags &= ~IFA_F_TENTATIVE;
 		spin_unlock_bh(&ifp->lock);
+		inet6addr_notifier_call_chain(NETDEV_UP, ifp);
 		ipv6_ifa_notify(RTM_NEWADDR, ifp);
 		in6_ifa_put(ifp);
 	}
@@ -3415,6 +3414,8 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
 	struct inet6_dev *idev = ifp->idev;
 	struct net_device *dev = idev->dev;
 
+	inet6addr_notifier_call_chain(NETDEV_UP, ifp);
+
 	addrconf_join_solict(dev, &ifp->addr);
 
 	prandom_seed((__force u32) ifp->addr.s6_addr32[3]);
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index d873ceea86e6..6997bd7946d3 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -87,23 +87,23 @@ int __ipv6_addr_type(const struct in6_addr *addr)
 }
 EXPORT_SYMBOL(__ipv6_addr_type);
 
-static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
+static BLOCKING_NOTIFIER_HEAD(inet6addr_chain);
 
 int register_inet6addr_notifier(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_register(&inet6addr_chain, nb);
+	return blocking_notifier_chain_register(&inet6addr_chain, nb);
 }
 EXPORT_SYMBOL(register_inet6addr_notifier);
 
 int unregister_inet6addr_notifier(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_unregister(&inet6addr_chain, nb);
+	return blocking_notifier_chain_unregister(&inet6addr_chain, nb);
 }
 EXPORT_SYMBOL(unregister_inet6addr_notifier);
 
 int inet6addr_notifier_call_chain(unsigned long val, void *v)
 {
-	return atomic_notifier_call_chain(&inet6addr_chain, val, v);
+	return blocking_notifier_call_chain(&inet6addr_chain, val, v);
 }
 EXPORT_SYMBOL(inet6addr_notifier_call_chain);
 

[PATCH 2/3] ipvlan: grab rcu_read_lock on xmit path

[Konstantin Khlebnikov]

ipvlan_start_xmit() is called with rcu_read_lock_bh() while its internal
structures requre normal rcu_read_lock().

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

---

[  802.945151] ===============================
[  802.945160] [ INFO: suspicious RCU usage. ]
[  802.945164] 4.1.0-rc3+ #71 Not tainted
[  802.945165] -------------------------------
[  802.945167] drivers/net/ipvlan/ipvlan.h:103 suspicious rcu_dereference_check() usage!
[  802.945168]
[  802.945168] other info that might help us debug this:
[  802.945168]
[  802.945170]
[  802.945170] rcu_scheduler_active = 1, debug_locks = 1
[  802.945173] 3 locks held by ping6/3813:
[  802.945174]  #0:  (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff81880f72>] rawv6_sendmsg+0x512/0xbb0
[  802.945197]  #1:  (rcu_read_lock_bh){......}, at: [<ffffffff8185e577>] ip6_finish_output2+0x57/0x790
[  802.945205]  #2:  (rcu_read_lock_bh){......}, at: [<ffffffff8177a68b>] __dev_queue_xmit+0x4b/0x930
[  802.945218]
[  802.945218] stack backtrace:
[  802.945221] CPU: 2 PID: 3813 Comm: ping6 Not tainted 4.1.0-rc3+ #71
[  802.945222] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011
[  802.945224]  0000000000000001 ffff8800db7b7888 ffffffff819de6f8 0000000000000007
[  802.945226]  ffff8800db738000 ffff8800db7b78b8 ffffffff810a7f92 ffff880214fc8c00
[  802.945229]  ffff88021595b000 ffff88021595a000 ffff880214adf800 ffff8800db7b7968
[  802.945232] Call Trace:
[  802.945248]  [<ffffffff819de6f8>] dump_stack+0x4c/0x65
[  802.945253]  [<ffffffff810a7f92>] lockdep_rcu_suspicious+0xe2/0x130
[  802.945265]  [<ffffffff815e24ac>] ipvlan_queue_xmit+0x17c/0x5a0
[  802.945268]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
[  802.945271]  [<ffffffff815e300c>] ipvlan_start_xmit+0x1c/0x50
[  802.945272]  [<ffffffff8177a117>] dev_hard_start_xmit+0x2f7/0x820
[  802.945274]  [<ffffffff817797b6>] ? netif_skb_features+0xf6/0x1d0
[  802.945276]  [<ffffffff81779b84>] ? validate_xmit_skb.isra.99.part.100+0x24/0x2c0
[  802.945278]  [<ffffffff8177ad04>] __dev_queue_xmit+0x6c4/0x930
[  802.945280]  [<ffffffff8177a68b>] ? __dev_queue_xmit+0x4b/0x930
[  802.945281]  [<ffffffff810a963a>] ? mark_held_locks+0x6a/0x90
[  802.945283]  [<ffffffff8177af8e>] dev_queue_xmit_sk+0xe/0x10
[  802.945285]  [<ffffffff8185e824>] ip6_finish_output2+0x304/0x790
[  802.945287]  [<ffffffff81860dde>] ? ip6_finish_output+0x9e/0x1e0
[  802.945288]  [<ffffffff81860dde>] ip6_finish_output+0x9e/0x1e0
[  802.945290]  [<ffffffff81860fdb>] ip6_output+0xbb/0x180
[  802.945302]  [<ffffffff818a7d0a>] ? ip6_find_1stfragopt+0x9a/0xa0
[  802.945304]  [<ffffffff81860d40>] ? ip6_fragment+0xe80/0xe80
[  802.945306]  [<ffffffff818a805c>] ip6_local_out_sk+0x2c/0x70
[  802.945308]  [<ffffffff818a80b0>] ip6_local_out+0x10/0x20
[  802.945309]  [<ffffffff81861571>] ip6_send_skb+0x31/0xd0
[  802.945311]  [<ffffffff81861644>] ip6_push_pending_frames+0x34/0x40
[  802.945313]  [<ffffffff81881368>] rawv6_sendmsg+0x908/0xbb0
[  802.945328]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
[  802.945340]  [<ffffffff81817e9e>] inet_sendmsg+0x10e/0x1f0
[  802.945343]  [<ffffffff81817d90>] ? inet_recvmsg+0x200/0x200
[  802.945351]  [<ffffffff81758275>] sock_sendmsg+0x45/0x50
[  802.945354]  [<ffffffff8175a719>] SYSC_sendto+0xd9/0x110
[  802.945357]  [<ffffffff8175ac99>] SyS_sendto+0x9/0x10
[  802.945362]  [<ffffffff819ebfae>] system_call_fastpath+0x12/0x76
---
 drivers/net/ipvlan/ipvlan_main.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 77b92a0fe557..0cafd3e6f02d 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -173,6 +173,7 @@ static int ipvlan_stop(struct net_device *dev)
 	return 0;
 }
 
+/* called with rcu_read_lock_bh. */
 static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
 				     struct net_device *dev)
 {
@@ -180,6 +181,7 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
 	int skblen = skb->len;
 	int ret;
 
+	rcu_read_lock();
 	ret = ipvlan_queue_xmit(skb, dev);
 	if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
 		struct ipvl_pcpu_stats *pcptr;
@@ -193,6 +195,8 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
 	} else {
 		this_cpu_inc(ipvlan->pcpu_stats->tx_drps);
 	}
+	rcu_read_unlock();
+
 	return ret;
 }
 

[PATCH 3/3] 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 |   12 +++++++++++-
 drivers/net/ipvlan/ipvlan.h         |    1 +
 drivers/net/ipvlan/ipvlan_main.c    |   20 ++++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ipvlan.txt b/Documentation/networking/ipvlan.txt
index cf996394e466..cb0b777bce58 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
@@ -105,3 +114,4 @@ namespace where L2 on the slave could be changed / misused.
 			(4) ip -4 addr add 127.0.0.1 dev lo
 			(5) ip -4 addr add $IPADDR dev ipvl1
 			(6) ip -4 route add default via $ROUTER dev ipvl1
+
diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 54549a6223dd..1ebab84e7a0e 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -95,6 +95,7 @@ struct ipvl_port {
 	struct rcu_head		rcu;
 	int			count;
 	u16			mode;
+	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 0cafd3e6f02d..dee0e8441150 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);
 
 	err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port);
 	if (err)
@@ -72,6 +73,7 @@ static void ipvlan_port_destroy(struct net_device *dev)
 
 	dev->priv_flags &= ~IFF_IPVLAN_MASTER;
 	netdev_rx_handler_unregister(dev);
+	ida_destroy(&port->ida);
 	kfree_rcu(port, rcu);
 }
 
@@ -484,6 +486,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;
@@ -518,6 +532,12 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
 			list_del(&addr->anode);
 		}
 	}
+
+	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);

Re: [PATCH 1/3] ipv6: make inet6addr_chain blocking and always call with rtnl locked

[David Miller]

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Thu, 14 May 2015 16:56:18 +0300

> Unlike to inetaddr_chain inet6addr_chain is atomic and called from bh
> context without rtnl when ipv6 receives router advertisement packet.
> 
> Several drivers don't know about that: ipvlan thinks that it has rtnl
> here, ocrdma locks mutex inside callback. Probably there is more.
> 
> This patch makes it blocking and calls from first stage of DAD work.
> Looks like this is completely safe and rtnl already locked here.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

I don't see how you can make the inet6addr_chain blocking when it is
invoked from software interrupt context.

You also cannot try to defer these operations to a workqueue or
similar to get into a blockable context, because various ipv6
testsuites depend upon the addressing state change happening
when we process the packet that triggers that change.

Instead, I think you have to make the users of inet6addr_chain
aware of the context in which they execute.

Thanks.

Re: [PATCH 1/3] ipv6: make inet6addr_chain blocking and always call with rtnl locked

[Konstantin Khlebnikov]

On 17.05.2015 00:22, David Miller wrote:
> From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Date: Thu, 14 May 2015 16:56:18 +0300
>
>> Unlike to inetaddr_chain inet6addr_chain is atomic and called from bh
>> context without rtnl when ipv6 receives router advertisement packet.
>>
>> Several drivers don't know about that: ipvlan thinks that it has rtnl
>> here, ocrdma locks mutex inside callback. Probably there is more.
>>
>> This patch makes it blocking and calls from first stage of DAD work.
>> Looks like this is completely safe and rtnl already locked here.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>
> I don't see how you can make the inet6addr_chain blocking when it is
> invoked from software interrupt context.
>
> You also cannot try to defer these operations to a workqueue or
> similar to get into a blockable context, because various ipv6
> testsuites depend upon the addressing state change happening
> when we process the packet that triggers that change.
>
> Instead, I think you have to make the users of inet6addr_chain
> aware of the context in which they execute.
>
> Thanks.
>

I've defer only calls of inet6addr_notifier_call_chain.
Ipv6 addresses still appears right in interrupt context.

Ordering with netlink events RTM_NEWADDR/RTM_DELADDR stays the same:

inet6addr_notifier_call_chain(NETDEV_UP, ifp);
ipv6_ifa_notify(RTM_NEWADDR, ifp);
...
ipv6_ifa_notify(RTM_DELADDR, ifp);
inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);

As I see ipv6 always send RTM_NEWADDR from dad-work even for
IFA_F_OPTIMISTIC addresses

The only visible change is ordering with event RTM_NEWPREFIX.


And another problem which my patch fixes. at this path:
addrconf_prefix_rcv -> ipv6_add_addr -> inet6addr_notifier_call_chain

inet6addr_notifier_call_chain called without any locks.
Theoretically NETDEV_DOWN event could be delivered before NETDEV_UP
if somebody removes that half-baked address right in that moment.

-- 
Konstantin

Re: [PATCH 2/3] ipvlan: grab rcu_read_lock on xmit path

[Mahesh Bandewar]

On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
>
> ipvlan_start_xmit() is called with rcu_read_lock_bh() while its internal
> structures requre normal rcu_read_lock().
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>
> ---
>
> [  802.945151] ===============================
> [  802.945160] [ INFO: suspicious RCU usage. ]
> [  802.945164] 4.1.0-rc3+ #71 Not tainted
> [  802.945165] -------------------------------
> [  802.945167] drivers/net/ipvlan/ipvlan.h:103 suspicious rcu_dereference_check() usage!
> [  802.945168]
> [  802.945168] other info that might help us debug this:
> [  802.945168]
> [  802.945170]
> [  802.945170] rcu_scheduler_active = 1, debug_locks = 1
> [  802.945173] 3 locks held by ping6/3813:
> [  802.945174]  #0:  (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff81880f72>] rawv6_sendmsg+0x512/0xbb0
> [  802.945197]  #1:  (rcu_read_lock_bh){......}, at: [<ffffffff8185e577>] ip6_finish_output2+0x57/0x790
> [  802.945205]  #2:  (rcu_read_lock_bh){......}, at: [<ffffffff8177a68b>] __dev_queue_xmit+0x4b/0x930
> [  802.945218]
> [  802.945218] stack backtrace:
> [  802.945221] CPU: 2 PID: 3813 Comm: ping6 Not tainted 4.1.0-rc3+ #71
> [  802.945222] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011
> [  802.945224]  0000000000000001 ffff8800db7b7888 ffffffff819de6f8 0000000000000007
> [  802.945226]  ffff8800db738000 ffff8800db7b78b8 ffffffff810a7f92 ffff880214fc8c00
> [  802.945229]  ffff88021595b000 ffff88021595a000 ffff880214adf800 ffff8800db7b7968
> [  802.945232] Call Trace:
> [  802.945248]  [<ffffffff819de6f8>] dump_stack+0x4c/0x65
> [  802.945253]  [<ffffffff810a7f92>] lockdep_rcu_suspicious+0xe2/0x130
> [  802.945265]  [<ffffffff815e24ac>] ipvlan_queue_xmit+0x17c/0x5a0
> [  802.945268]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
> [  802.945271]  [<ffffffff815e300c>] ipvlan_start_xmit+0x1c/0x50
> [  802.945272]  [<ffffffff8177a117>] dev_hard_start_xmit+0x2f7/0x820
> [  802.945274]  [<ffffffff817797b6>] ? netif_skb_features+0xf6/0x1d0
> [  802.945276]  [<ffffffff81779b84>] ? validate_xmit_skb.isra.99.part.100+0x24/0x2c0
> [  802.945278]  [<ffffffff8177ad04>] __dev_queue_xmit+0x6c4/0x930
> [  802.945280]  [<ffffffff8177a68b>] ? __dev_queue_xmit+0x4b/0x930
> [  802.945281]  [<ffffffff810a963a>] ? mark_held_locks+0x6a/0x90
> [  802.945283]  [<ffffffff8177af8e>] dev_queue_xmit_sk+0xe/0x10
> [  802.945285]  [<ffffffff8185e824>] ip6_finish_output2+0x304/0x790
> [  802.945287]  [<ffffffff81860dde>] ? ip6_finish_output+0x9e/0x1e0
> [  802.945288]  [<ffffffff81860dde>] ip6_finish_output+0x9e/0x1e0
> [  802.945290]  [<ffffffff81860fdb>] ip6_output+0xbb/0x180
> [  802.945302]  [<ffffffff818a7d0a>] ? ip6_find_1stfragopt+0x9a/0xa0
> [  802.945304]  [<ffffffff81860d40>] ? ip6_fragment+0xe80/0xe80
> [  802.945306]  [<ffffffff818a805c>] ip6_local_out_sk+0x2c/0x70
> [  802.945308]  [<ffffffff818a80b0>] ip6_local_out+0x10/0x20
> [  802.945309]  [<ffffffff81861571>] ip6_send_skb+0x31/0xd0
> [  802.945311]  [<ffffffff81861644>] ip6_push_pending_frames+0x34/0x40
> [  802.945313]  [<ffffffff81881368>] rawv6_sendmsg+0x908/0xbb0
> [  802.945328]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
> [  802.945340]  [<ffffffff81817e9e>] inet_sendmsg+0x10e/0x1f0
> [  802.945343]  [<ffffffff81817d90>] ? inet_recvmsg+0x200/0x200
> [  802.945351]  [<ffffffff81758275>] sock_sendmsg+0x45/0x50
> [  802.945354]  [<ffffffff8175a719>] SYSC_sendto+0xd9/0x110
> [  802.945357]  [<ffffffff8175ac99>] SyS_sendto+0x9/0x10
> [  802.945362]  [<ffffffff819ebfae>] system_call_fastpath+0x12/0x76
> ---
>  drivers/net/ipvlan/ipvlan_main.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 77b92a0fe557..0cafd3e6f02d 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -173,6 +173,7 @@ static int ipvlan_stop(struct net_device *dev)
>         return 0;
>  }
>
> +/* called with rcu_read_lock_bh. */
>  static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>                                      struct net_device *dev)
>  {
> @@ -180,6 +181,7 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>         int skblen = skb->len;
>         int ret;
>
> +       rcu_read_lock();

I don't believe this is correct. The correct way would be the way  it
is fixed in the following patch -
https://patchwork.ozlabs.org/patch/471481/

>         ret = ipvlan_queue_xmit(skb, dev);
>         if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
>                 struct ipvl_pcpu_stats *pcptr;
> @@ -193,6 +195,8 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>         } else {
>                 this_cpu_inc(ipvlan->pcpu_stats->tx_drps);
>         }
> +       rcu_read_unlock();
> +
>         return ret;
>  }
>
>

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

[Mahesh Bandewar]

On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> 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 |   12 +++++++++++-
>  drivers/net/ipvlan/ipvlan.h         |    1 +
>  drivers/net/ipvlan/ipvlan_main.c    |   20 ++++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/networking/ipvlan.txt b/Documentation/networking/ipvlan.txt
> index cf996394e466..cb0b777bce58 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).
> +
This is nice, thanks for fixing this! However how is "unique" id
guaranteed? Especially when multiple virtual drivers are stacked? Not
necessarily all of them may use the dev_id, but to avoid any possible
collision, shouldn't the device hierarchy (especially lower_dev) be
traversed before settling on the initial value?

> +       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
> @@ -105,3 +114,4 @@ namespace where L2 on the slave could be changed / misused.
>                         (4) ip -4 addr add 127.0.0.1 dev lo
>                         (5) ip -4 addr add $IPADDR dev ipvl1
>                         (6) ip -4 route add default via $ROUTER dev ipvl1
> +
> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
> index 54549a6223dd..1ebab84e7a0e 100644
> --- a/drivers/net/ipvlan/ipvlan.h
> +++ b/drivers/net/ipvlan/ipvlan.h
> @@ -95,6 +95,7 @@ struct ipvl_port {
>         struct rcu_head         rcu;
>         int                     count;
>         u16                     mode;
> +       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 0cafd3e6f02d..dee0e8441150 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);
>
>         err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port);
>         if (err)
> @@ -72,6 +73,7 @@ static void ipvlan_port_destroy(struct net_device *dev)
>
>         dev->priv_flags &= ~IFF_IPVLAN_MASTER;
>         netdev_rx_handler_unregister(dev);
> +       ida_destroy(&port->ida);
>         kfree_rcu(port, rcu);
>  }
>
> @@ -484,6 +486,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;
> @@ -518,6 +532,12 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
>                         list_del(&addr->anode);
>                 }
>         }
> +
> +       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);
>

Re: [PATCH 2/3] ipvlan: grab rcu_read_lock on xmit path

[Konstantin Khlebnikov]

On 20.05.2015 02:33, Mahesh Bandewar wrote:
> On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>>
>> ipvlan_start_xmit() is called with rcu_read_lock_bh() while its internal
>> structures requre normal rcu_read_lock().
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>
>> ---
>>
>> [  802.945151] ===============================
>> [  802.945160] [ INFO: suspicious RCU usage. ]
>> [  802.945164] 4.1.0-rc3+ #71 Not tainted
>> [  802.945165] -------------------------------
>> [  802.945167] drivers/net/ipvlan/ipvlan.h:103 suspicious rcu_dereference_check() usage!
>> [  802.945168]
>> [  802.945168] other info that might help us debug this:
>> [  802.945168]
>> [  802.945170]
>> [  802.945170] rcu_scheduler_active = 1, debug_locks = 1
>> [  802.945173] 3 locks held by ping6/3813:
>> [  802.945174]  #0:  (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff81880f72>] rawv6_sendmsg+0x512/0xbb0
>> [  802.945197]  #1:  (rcu_read_lock_bh){......}, at: [<ffffffff8185e577>] ip6_finish_output2+0x57/0x790
>> [  802.945205]  #2:  (rcu_read_lock_bh){......}, at: [<ffffffff8177a68b>] __dev_queue_xmit+0x4b/0x930
>> [  802.945218]
>> [  802.945218] stack backtrace:
>> [  802.945221] CPU: 2 PID: 3813 Comm: ping6 Not tainted 4.1.0-rc3+ #71
>> [  802.945222] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011
>> [  802.945224]  0000000000000001 ffff8800db7b7888 ffffffff819de6f8 0000000000000007
>> [  802.945226]  ffff8800db738000 ffff8800db7b78b8 ffffffff810a7f92 ffff880214fc8c00
>> [  802.945229]  ffff88021595b000 ffff88021595a000 ffff880214adf800 ffff8800db7b7968
>> [  802.945232] Call Trace:
>> [  802.945248]  [<ffffffff819de6f8>] dump_stack+0x4c/0x65
>> [  802.945253]  [<ffffffff810a7f92>] lockdep_rcu_suspicious+0xe2/0x130
>> [  802.945265]  [<ffffffff815e24ac>] ipvlan_queue_xmit+0x17c/0x5a0
>> [  802.945268]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
>> [  802.945271]  [<ffffffff815e300c>] ipvlan_start_xmit+0x1c/0x50
>> [  802.945272]  [<ffffffff8177a117>] dev_hard_start_xmit+0x2f7/0x820
>> [  802.945274]  [<ffffffff817797b6>] ? netif_skb_features+0xf6/0x1d0
>> [  802.945276]  [<ffffffff81779b84>] ? validate_xmit_skb.isra.99.part.100+0x24/0x2c0
>> [  802.945278]  [<ffffffff8177ad04>] __dev_queue_xmit+0x6c4/0x930
>> [  802.945280]  [<ffffffff8177a68b>] ? __dev_queue_xmit+0x4b/0x930
>> [  802.945281]  [<ffffffff810a963a>] ? mark_held_locks+0x6a/0x90
>> [  802.945283]  [<ffffffff8177af8e>] dev_queue_xmit_sk+0xe/0x10
>> [  802.945285]  [<ffffffff8185e824>] ip6_finish_output2+0x304/0x790
>> [  802.945287]  [<ffffffff81860dde>] ? ip6_finish_output+0x9e/0x1e0
>> [  802.945288]  [<ffffffff81860dde>] ip6_finish_output+0x9e/0x1e0
>> [  802.945290]  [<ffffffff81860fdb>] ip6_output+0xbb/0x180
>> [  802.945302]  [<ffffffff818a7d0a>] ? ip6_find_1stfragopt+0x9a/0xa0
>> [  802.945304]  [<ffffffff81860d40>] ? ip6_fragment+0xe80/0xe80
>> [  802.945306]  [<ffffffff818a805c>] ip6_local_out_sk+0x2c/0x70
>> [  802.945308]  [<ffffffff818a80b0>] ip6_local_out+0x10/0x20
>> [  802.945309]  [<ffffffff81861571>] ip6_send_skb+0x31/0xd0
>> [  802.945311]  [<ffffffff81861644>] ip6_push_pending_frames+0x34/0x40
>> [  802.945313]  [<ffffffff81881368>] rawv6_sendmsg+0x908/0xbb0
>> [  802.945328]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
>> [  802.945340]  [<ffffffff81817e9e>] inet_sendmsg+0x10e/0x1f0
>> [  802.945343]  [<ffffffff81817d90>] ? inet_recvmsg+0x200/0x200
>> [  802.945351]  [<ffffffff81758275>] sock_sendmsg+0x45/0x50
>> [  802.945354]  [<ffffffff8175a719>] SYSC_sendto+0xd9/0x110
>> [  802.945357]  [<ffffffff8175ac99>] SyS_sendto+0x9/0x10
>> [  802.945362]  [<ffffffff819ebfae>] system_call_fastpath+0x12/0x76
>> ---
>>   drivers/net/ipvlan/ipvlan_main.c |    4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
>> index 77b92a0fe557..0cafd3e6f02d 100644
>> --- a/drivers/net/ipvlan/ipvlan_main.c
>> +++ b/drivers/net/ipvlan/ipvlan_main.c
>> @@ -173,6 +173,7 @@ static int ipvlan_stop(struct net_device *dev)
>>          return 0;
>>   }
>>
>> +/* called with rcu_read_lock_bh. */
>>   static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>>                                       struct net_device *dev)
>>   {
>> @@ -180,6 +181,7 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>>          int skblen = skb->len;
>>          int ret;
>>
>> +       rcu_read_lock();
>
> I don't believe this is correct. The correct way would be the way  it
> is fixed in the following patch -
> https://patchwork.ozlabs.org/patch/471481/

I'm not sure, that might just plug the warning without fixing problem.
The rest code uses call_rcu()/synchronize_rcu(). Probably relation
between quiescent states of rcu and rcu_bh isn't that easy.

>
>>          ret = ipvlan_queue_xmit(skb, dev);
>>          if (likely(ret == NET_XMIT_SUCCESS || ret == NET_XMIT_CN)) {
>>                  struct ipvl_pcpu_stats *pcptr;
>> @@ -193,6 +195,8 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
>>          } else {
>>                  this_cpu_inc(ipvlan->pcpu_stats->tx_drps);
>>          }
>> +       rcu_read_unlock();
>> +
>>          return ret;
>>   }
>>
>>


-- 
Konstantin

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

[Konstantin Khlebnikov]

On 20.05.2015 02:59, Mahesh Bandewar wrote:
> On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>> 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 |   12 +++++++++++-
>>   drivers/net/ipvlan/ipvlan.h         |    1 +
>>   drivers/net/ipvlan/ipvlan_main.c    |   20 ++++++++++++++++++++
>>   3 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/networking/ipvlan.txt b/Documentation/networking/ipvlan.txt
>> index cf996394e466..cb0b777bce58 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).
>> +
> This is nice, thanks for fixing this! However how is "unique" id
> guaranteed? Especially when multiple virtual drivers are stacked? Not
> necessarily all of them may use the dev_id, but to avoid any possible
> collision, shouldn't the device hierarchy (especially lower_dev) be
> traversed before settling on the initial value?

Well, uniqueness isn't guaranteed but that should work in most cases.
ipv6 anyway checks for duplicate addresses after configuration.

As I see creation of ipvlan on ipvlan just creates slave at original
master device, so this will work as expected. And ipvlan cannot share 
physical device with bonding/bridge/macvlan, so I don't see how to
stack more than one layer of ipvlans accidentally.

>
>> +       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
>> @@ -105,3 +114,4 @@ namespace where L2 on the slave could be changed / misused.
>>                          (4) ip -4 addr add 127.0.0.1 dev lo
>>                          (5) ip -4 addr add $IPADDR dev ipvl1
>>                          (6) ip -4 route add default via $ROUTER dev ipvl1
>> +
>> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
>> index 54549a6223dd..1ebab84e7a0e 100644
>> --- a/drivers/net/ipvlan/ipvlan.h
>> +++ b/drivers/net/ipvlan/ipvlan.h
>> @@ -95,6 +95,7 @@ struct ipvl_port {
>>          struct rcu_head         rcu;
>>          int                     count;
>>          u16                     mode;
>> +       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 0cafd3e6f02d..dee0e8441150 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);
>>
>>          err = netdev_rx_handler_register(dev, ipvlan_handle_frame, port);
>>          if (err)
>> @@ -72,6 +73,7 @@ static void ipvlan_port_destroy(struct net_device *dev)
>>
>>          dev->priv_flags &= ~IFF_IPVLAN_MASTER;
>>          netdev_rx_handler_unregister(dev);
>> +       ida_destroy(&port->ida);
>>          kfree_rcu(port, rcu);
>>   }
>>
>> @@ -484,6 +486,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;
>> @@ -518,6 +532,12 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
>>                          list_del(&addr->anode);
>>                  }
>>          }
>> +
>> +       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);
>>


-- 
Konstantin

Re: [PATCH 2/3] ipvlan: grab rcu_read_lock on xmit path

[Hannes Frederic Sowa]

On Thu, May 21, 2015, at 11:51, Konstantin Khlebnikov wrote:
> On 20.05.2015 02:33, Mahesh Bandewar wrote:
> > On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
> > <khlebnikov@yandex-team.ru> wrote:
> >>
> >> ipvlan_start_xmit() is called with rcu_read_lock_bh() while its internal
> >> structures requre normal rcu_read_lock().
> >>
> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> >>
> >> ---
> >>
> >> [  802.945151] ===============================
> >> [  802.945160] [ INFO: suspicious RCU usage. ]
> >> [  802.945164] 4.1.0-rc3+ #71 Not tainted
> >> [  802.945165] -------------------------------
> >> [  802.945167] drivers/net/ipvlan/ipvlan.h:103 suspicious rcu_dereference_check() usage!
> >> [  802.945168]
> >> [  802.945168] other info that might help us debug this:
> >> [  802.945168]
> >> [  802.945170]
> >> [  802.945170] rcu_scheduler_active = 1, debug_locks = 1
> >> [  802.945173] 3 locks held by ping6/3813:
> >> [  802.945174]  #0:  (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff81880f72>] rawv6_sendmsg+0x512/0xbb0
> >> [  802.945197]  #1:  (rcu_read_lock_bh){......}, at: [<ffffffff8185e577>] ip6_finish_output2+0x57/0x790
> >> [  802.945205]  #2:  (rcu_read_lock_bh){......}, at: [<ffffffff8177a68b>] __dev_queue_xmit+0x4b/0x930
> >> [  802.945218]
> >> [  802.945218] stack backtrace:
> >> [  802.945221] CPU: 2 PID: 3813 Comm: ping6 Not tainted 4.1.0-rc3+ #71
> >> [  802.945222] Hardware name: OpenStack Foundation OpenStack Nova, BIOS Bochs 01/01/2011
> >> [  802.945224]  0000000000000001 ffff8800db7b7888 ffffffff819de6f8 0000000000000007
> >> [  802.945226]  ffff8800db738000 ffff8800db7b78b8 ffffffff810a7f92 ffff880214fc8c00
> >> [  802.945229]  ffff88021595b000 ffff88021595a000 ffff880214adf800 ffff8800db7b7968
> >> [  802.945232] Call Trace:
> >> [  802.945248]  [<ffffffff819de6f8>] dump_stack+0x4c/0x65
> >> [  802.945253]  [<ffffffff810a7f92>] lockdep_rcu_suspicious+0xe2/0x130
> >> [  802.945265]  [<ffffffff815e24ac>] ipvlan_queue_xmit+0x17c/0x5a0
> >> [  802.945268]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
> >> [  802.945271]  [<ffffffff815e300c>] ipvlan_start_xmit+0x1c/0x50
> >> [  802.945272]  [<ffffffff8177a117>] dev_hard_start_xmit+0x2f7/0x820
> >> [  802.945274]  [<ffffffff817797b6>] ? netif_skb_features+0xf6/0x1d0
> >> [  802.945276]  [<ffffffff81779b84>] ? validate_xmit_skb.isra.99.part.100+0x24/0x2c0
> >> [  802.945278]  [<ffffffff8177ad04>] __dev_queue_xmit+0x6c4/0x930
> >> [  802.945280]  [<ffffffff8177a68b>] ? __dev_queue_xmit+0x4b/0x930
> >> [  802.945281]  [<ffffffff810a963a>] ? mark_held_locks+0x6a/0x90
> >> [  802.945283]  [<ffffffff8177af8e>] dev_queue_xmit_sk+0xe/0x10
> >> [  802.945285]  [<ffffffff8185e824>] ip6_finish_output2+0x304/0x790
> >> [  802.945287]  [<ffffffff81860dde>] ? ip6_finish_output+0x9e/0x1e0
> >> [  802.945288]  [<ffffffff81860dde>] ip6_finish_output+0x9e/0x1e0
> >> [  802.945290]  [<ffffffff81860fdb>] ip6_output+0xbb/0x180
> >> [  802.945302]  [<ffffffff818a7d0a>] ? ip6_find_1stfragopt+0x9a/0xa0
> >> [  802.945304]  [<ffffffff81860d40>] ? ip6_fragment+0xe80/0xe80
> >> [  802.945306]  [<ffffffff818a805c>] ip6_local_out_sk+0x2c/0x70
> >> [  802.945308]  [<ffffffff818a80b0>] ip6_local_out+0x10/0x20
> >> [  802.945309]  [<ffffffff81861571>] ip6_send_skb+0x31/0xd0
> >> [  802.945311]  [<ffffffff81861644>] ip6_push_pending_frames+0x34/0x40
> >> [  802.945313]  [<ffffffff81881368>] rawv6_sendmsg+0x908/0xbb0
> >> [  802.945328]  [<ffffffff810aa5a4>] ? __lock_is_held+0x54/0x70
> >> [  802.945340]  [<ffffffff81817e9e>] inet_sendmsg+0x10e/0x1f0
> >> [  802.945343]  [<ffffffff81817d90>] ? inet_recvmsg+0x200/0x200
> >> [  802.945351]  [<ffffffff81758275>] sock_sendmsg+0x45/0x50
> >> [  802.945354]  [<ffffffff8175a719>] SYSC_sendto+0xd9/0x110
> >> [  802.945357]  [<ffffffff8175ac99>] SyS_sendto+0x9/0x10
> >> [  802.945362]  [<ffffffff819ebfae>] system_call_fastpath+0x12/0x76
> >> ---
> >>   drivers/net/ipvlan/ipvlan_main.c |    4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> >> index 77b92a0fe557..0cafd3e6f02d 100644
> >> --- a/drivers/net/ipvlan/ipvlan_main.c
> >> +++ b/drivers/net/ipvlan/ipvlan_main.c
> >> @@ -173,6 +173,7 @@ static int ipvlan_stop(struct net_device *dev)
> >>          return 0;
> >>   }
> >>
> >> +/* called with rcu_read_lock_bh. */
> >>   static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
> >>                                       struct net_device *dev)
> >>   {
> >> @@ -180,6 +181,7 @@ static netdev_tx_t ipvlan_start_xmit(struct sk_buff *skb,
> >>          int skblen = skb->len;
> >>          int ret;
> >>
> >> +       rcu_read_lock();
> >
> > I don't believe this is correct. The correct way would be the way  it
> > is fixed in the following patch -
> > https://patchwork.ozlabs.org/patch/471481/
> 
> I'm not sure, that might just plug the warning without fixing problem.
> The rest code uses call_rcu()/synchronize_rcu(). Probably relation
> between quiescent states of rcu and rcu_bh isn't that easy.
> 

In theory both, rcu_read_lock and rcu_read_lock_bh must be held,
otherwise call_rcu would be allowed to execute callback if no
rcu_read_lock but only rcu_read_lock_bh is taken. *AFAIK* in normal
kernels, grace period for call_rcu is just longer, call_rcu_bh does
check transition from and to softirqs.

Bye,
Hannes

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

[Hannes Frederic Sowa]

On Thu, May 21, 2015, at 13:38, Konstantin Khlebnikov wrote:
> On 20.05.2015 02:59, Mahesh Bandewar wrote:
> > On Thu, May 14, 2015 at 6:56 AM, Konstantin Khlebnikov
> > <khlebnikov@yandex-team.ru> wrote:
> >> 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 |   12 +++++++++++-
> >>   drivers/net/ipvlan/ipvlan.h         |    1 +
> >>   drivers/net/ipvlan/ipvlan_main.c    |   20 ++++++++++++++++++++
> >>   3 files changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/networking/ipvlan.txt b/Documentation/networking/ipvlan.txt
> >> index cf996394e466..cb0b777bce58 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).
> >> +
> > This is nice, thanks for fixing this! However how is "unique" id
> > guaranteed? Especially when multiple virtual drivers are stacked? Not
> > necessarily all of them may use the dev_id, but to avoid any possible
> > collision, shouldn't the device hierarchy (especially lower_dev) be
> > traversed before settling on the initial value?
> 
> Well, uniqueness isn't guaranteed but that should work in most cases.
> ipv6 anyway checks for duplicate addresses after configuration.
> 
> As I see creation of ipvlan on ipvlan just creates slave at original
> master device, so this will work as expected. And ipvlan cannot share 
> physical device with bonding/bridge/macvlan, so I don't see how to
> stack more than one layer of ipvlans accidentally.

I hope that stable ipv6 privacy addresses will be used in future and old
eui-48 based LL addresses will just disappear. That said, I would be
even fine with a RNG generated dev_id, because we cannot really ensure
uniqueness.

Stable privacy addresses even use DAD retry counter to regenerate a new
LL address in case the address becomes IFA_F_DADFAILED.

Bye,
Hannes