Re: [PATCH iproute2 v2] ip: reset netns after each command in batch mode

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri,  7 Jun 2019 22:41:22 +0200
Matteo Croce <mcroce@redhat.com> wrote:

> When creating a new netns or executing a program into an existing one,
> the unshare() or setns() calls will change the current netns.
> In batch mode, this can run commands on the wrong interfaces, as the
> ifindex value is meaningful only in the current netns. For example, this
> command fails because veth-c doesn't exists in the init netns:
> 
>     # ip -b - <<-'EOF'
>         netns add client
>         link add name veth-c type veth peer veth-s netns client
>         addr add 192.168.2.1/24 dev veth-c
>     EOF
>     Cannot find device "veth-c"
>     Command failed -:7
> 
> But if there are two devices with the same name in the init and new netns,
> ip will build a wrong ll_map with indexes belonging to the new netns,
> and will execute actions in the init netns using this wrong mapping.
> This script will flush all eth0 addresses and bring it down, as it has
> the same ifindex of veth0 in the new netns:
> 
>     # ip addr
>     1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
>         link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>         inet 127.0.0.1/8 scope host lo
>            valid_lft forever preferred_lft forever
>     2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
>         link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
>         inet 192.168.122.76/24 brd 192.168.122.255 scope global dynamic eth0
>            valid_lft 3598sec preferred_lft 3598sec
> 
>     # ip -b - <<-'EOF'
>         netns add client
>         link add name veth0 type veth peer name veth1
>         link add name veth-ns type veth peer name veth0 netns client
>         link set veth0 down
>         address flush veth0
>     EOF
> 
>     # ip addr
>     1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
>         link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>         inet 127.0.0.1/8 scope host lo
>            valid_lft forever preferred_lft forever
>     2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN group default qlen 1000
>         link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
>     3: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default qlen 1000
>         link/ether c2:db:d0:34:13:4a brd ff:ff:ff:ff:ff:ff
>     4: veth0@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default qlen 1000
>         link/ether ca:9d:6b:5f:5f:8f brd ff:ff:ff:ff:ff:ff
>     5: veth-ns@if2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
>         link/ether 32:ef:22:df:51:0a brd ff:ff:ff:ff:ff:ff link-netns client
> 
> The same issue can be triggered by the netns exec subcommand with a
> sligthy different script:
> 
>     # ip netns add client
>     # ip -b - <<-'EOF'
>         netns exec client true
>         link add name veth0 type veth peer name veth1
>         link add name veth-ns type veth peer name veth0 netns client
>         link set veth0 down
>         address flush veth0
>     EOF
> 
> Fix this by adding two netns_{save,reset} functions, which are used
> to get a file descriptor for the init netns, and restore it after
> each batch command.
> netns_save() is called before the unshare() or setns(),
> while netns_restore() is called after each command.
> 
> Fixes: 0dc34c7713bb ("iproute2: Add processless network namespace support")
> Reviewed-and-tested-by: Andrea Claudi <aclaudi@redhat.com>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

Applied, thanks.