Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace

[David Ahern <dsahern@gmail.com>]

On 9/3/18 3:10 AM, Thomas Haller wrote:
> Hi,
> 
> On Sat, 2018-09-01 at 17:45 -0600, David Ahern wrote:
>> On 9/1/18 3:05 AM, Lorenzo Bianconi wrote:
>>>
>>> I was thinking about the commit 38e01b30563a and then I realized I
>>> misread the code
>>> yesterday. The commit 38e01b30563a provides all relevant info but
>>> it
>>> emits the event
>>> for veth1 (the device moved in the new namespace).
>>> An userspace application will not receive that message if it
>>> filters
>>> events for just
>>> a specific device (veth0 in this case) despite that some device
>>> properties have changed
>>> (since veth0 and veth1 are paired devices). To fix that behavior in
>>> veth_notify routine
>>> I emits a RTM_NEWLINK event for veth0.
>>
>> Userspace is managing a veth a pair. It moves one of them to a new
>> namespace and decides to filter messages related to that device
>> before
>> the move. If it did not filter the DELLINK message it would get the
>> information you want. That to me is a userspace problem. What am I
>> missing?
> 
> The userspace component which moves the veth peer is likely not the
> same that is listening to these events.
> 
>> Fundamentally, your proposal means 3 messages when moving a device to
>> a
>> new namespace which is what I think is unnecessary.
> 
> You are correct, the necessary information is signaled in this case.
> 
> Usually, one can manage the information about one link by considering
> only RTM_NEWLINK/RTM_DELLINK message for that link. That seems
> desirable behavior of the netlink API, as it simplifies user space
> implementations.
> 
> For example, libnl3's cache for links doesn't properly handles this,
> and you end up with invalid data in the cache. You may call that a
> userspace problem and bug. But does it really need to be that
> complicated?
> 
> FWIW, a similar thing was also NACK'ed earlier:
>   http://lists.openwall.net/netdev/2016/06/12/8

And from what I can see, I agree with that NACK; a 3rd message is not
necessary. Userspace has the ability to learn what it needs and should
be looking at the existing options that bleed data across namespaces.

> 
> 
> 
> Paolo and Lorenzo pointed out another issue when moving the peer to a
> third namespace:
> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> 
> ip netns del ns1
> ip netns del ns2
> ip netns del ns3
> ip netns add ns1
> ip netns add ns2
> ip netns add ns3
> 
> 
> ip -n ns1 link add dev veth0 type veth peer name veth1
> ip -n ns1 monitor link &
> 
> ip -n ns1 link set veth1 netns ns2
> # Deleted 9: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default 
> #    link/ether 86:79:1d:c6:45:bc brd ff:ff:ff:ff:ff:ff new-netnsid 0 new-ifindex 9
> 
> ip -n ns2 link set veth1 netns ns3
> # no event.


ip li add veth1 type veth peer name veth2
ip netns add foo
ip netns add bar
--> start ip monitor here; see below

ip li set veth2 netns foo
ip -netns foo li set veth2 netns bar


>From init_net:
$ ip monitor all-nsid
[nsid current]Deleted inet veth2
[nsid current]Deleted inet6 veth2
[nsid current]nsid 0 (iproute2 netns name: foo)
[nsid current]Deleted 16: veth2@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu
1500 qdisc noop state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff new-netns foo
new-ifindex 16
[nsid current]inet if16 forwarding on rp_filter off mc_forwarding off
proxy_neigh off ignore_routes_with_linkdown off
[nsid current]inet6 if16 forwarding off mc_forwarding off proxy_neigh
off ignore_routes_with_linkdown off
[nsid current]nsid 0 (iproute2 netns name: foo)
[nsid current]16: veth2@if17: <BROADCAST,MULTICAST> mtu 1500 qdisc noop
state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0
[nsid current]Deleted inet veth2
[nsid current]Deleted inet6 veth2
[nsid current]nsid 1
[nsid current]Deleted 16: veth2@if17: <BROADCAST,MULTICAST> mtu 1500
qdisc noop state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0
new-netnsid 1 new-ifindex 16