diff mbox

[net,v3] veth: advertise peer link once both links are tied together

Message ID 1464623917-11536-1-git-send-email-vincent@bernat.im
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Vincent Bernat May 30, 2016, 3:58 p.m. UTC
When the peer link is created, its "iflink" information is not
advertised through Netlink. Once created, the local device is advertised
with this information but if a user is maintaining a cache from all
updates, it will still miss the iflink for the peer link:

    2: veth0@NONE: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
        link/ether ae:0e:08:af:fb:a0 brd ff:ff:ff:ff:ff:ff
    3: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default
        link/ether 3a:31:f1:36:2e:e5 brd ff:ff:ff:ff:ff:ff

With this patch, we advertise again the peer link to let any user pick
the appropriate iflink information:

    3: veth0@NONE: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
        link/ether fa:ba:12:26:99:00 brd ff:ff:ff:ff:ff:ff
    3: veth0@veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default
        link/ether fa:ba:12:26:99:00 brd ff:ff:ff:ff:ff:ff
    4: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default
        link/ether ea:e4:e2:26:3c:87 brd ff:ff:ff:ff:ff:ff

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
v3:
 - send an additional netlink messages once the peer link is tied to
   avoid any chicken/egg problem

v2:
 - ensure the device is unregistered in case of link configuration failure

 drivers/net/veth.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Vincent Bernat May 30, 2016, 4:01 p.m. UTC | #1
❦ 30 mai 2016 17:58 CEST, Vincent Bernat <vincent@bernat.im> :

> +
> +	rtmsg_ifinfo(RTM_NEWLINK, peer, IFF_SLAVE, GFP_KERNEL);

Maybe ~0U would be better than hijacking IFF_SLAVE?
Nicolas Dichtel May 30, 2016, 4:27 p.m. UTC | #2
Le 30/05/2016 18:01, Vincent Bernat a écrit :
>  ❦ 30 mai 2016 17:58 CEST, Vincent Bernat <vincent@bernat.im> :
> 
>> +
>> +	rtmsg_ifinfo(RTM_NEWLINK, peer, IFF_SLAVE, GFP_KERNEL);
> 
> Maybe ~0U would be better than hijacking IFF_SLAVE?
IFF_SLAVE is wrong. It's a flag here, that will be put in the ifi_change field
not an attribute number.
Vincent Bernat May 31, 2016, 6:29 a.m. UTC | #3
❦ 30 mai 2016 18:27 CEST, Nicolas Dichtel <nicolas.dichtel@6wind.com> :

>>> +
>>> +	rtmsg_ifinfo(RTM_NEWLINK, peer, IFF_SLAVE, GFP_KERNEL);
>> 
>> Maybe ~0U would be better than hijacking IFF_SLAVE?
> IFF_SLAVE is wrong. It's a flag here, that will be put in the ifi_change field
> not an attribute number.

There are some use of IFF_SLAVE (in bonding/bond_main.c). But, I'll
update the patch nonetheless.
Nicolas Dichtel May 31, 2016, 9:17 a.m. UTC | #4
Le 31/05/2016 08:29, Vincent Bernat a écrit :
>  ❦ 30 mai 2016 18:27 CEST, Nicolas Dichtel <nicolas.dichtel@6wind.com> :
> 
>>>> +
>>>> +	rtmsg_ifinfo(RTM_NEWLINK, peer, IFF_SLAVE, GFP_KERNEL);
>>>
>>> Maybe ~0U would be better than hijacking IFF_SLAVE?
>> IFF_SLAVE is wrong. It's a flag here, that will be put in the ifi_change field
>> not an attribute number.
> 
> There are some use of IFF_SLAVE (in bonding/bond_main.c). But, I'll
> update the patch nonetheless.
Sorry, I read it too quickly, IFF_SLAVE is a flag, not an attribute.
But this field indicates to the userland which flags has changed and this flag
does not change here ;-)
diff mbox

Patch

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f37a6e61d4ad..2376d17b8f53 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -466,6 +466,8 @@  static int veth_newlink(struct net *src_net, struct net_device *dev,
 
 	priv = netdev_priv(peer);
 	rcu_assign_pointer(priv->peer, dev);
+
+	rtmsg_ifinfo(RTM_NEWLINK, peer, IFF_SLAVE, GFP_KERNEL);
 	return 0;
 
 err_register_dev: