Fix KeepCarrier tun/tap device option

[KonishchevDmitry]

When KeepCarrier is set, networkd doesn't close tun/tap file descriptor preserving the active interface state, but doesn't disable its queue which makes kernel to think that it's still active and send packets to it.

This patch disables the created queue right after tun/tap interface creation.

Here is the steps to reproduce the bug:

Having:

systemd/network/10-tun-test.netdev:

[NetDev]
Name=tun-test
Kind=tun

[Tun]
MultiQueue=yes
KeepCarrier=yes

systemd/network/10-tun-test.network:

[Match]
Name=tun-test

[Network]
DHCP=no
IPv6AcceptRA=false

LLMNR=false
MulticastDNS=false

Address=172.31.0.1/24

app.c:

#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <linux/if.h>
#include <sys/ioctl.h>
#include <linux/if_tun.h>

int main() {
    int fd;
    struct ifreq ifr;

    memset(&ifr, 0, sizeof ifr);
    strcpy(ifr.ifr_name, "tun-test");
    ifr.ifr_flags = IFF_TUN | IFF_NO_PI | IFF_MULTI_QUEUE;

    if((fd = open("/dev/net/tun", O_RDWR)) < 0) {
        perror("Open error");
        return 1;
    }

    if(ioctl(fd, TUNSETIFF, &ifr)) {
        perror("Configure error");
        return 1;
    }

    puts("Ready.");

    char buf[1500];

    while(1) {
        int size = read(fd, buf, sizeof buf);
        if(size < 0) {
            perror("Read error");
            return 1;
        }
        printf("Read %d bytes.\n", size);
    }

    return 0;
}

Run:

• gcc -o app app.c && ./app
• ping -I tun-test 172.31.0.2

Before the patch the app shows no pings, but after it works properly.

[poettering]

here you check for multi_queue, not for keep_fd?

[KonishchevDmitry]

Yes, it's intentional: networkd never reads packets from its devices, so it would be more correct to always disable the queue right after device creation / attaching to existing device to prevent packet dropping even for the short time of device configuration. So I would even remove if (t->multi_queue) and do it unconditionally, but kernel doesn't allow to disable queue on non-multi queue devices. It would be best to attach in disabled queue mode, but kernel doesn't have one, so the best we can do is disable the queue right after attaching.

[poettering]

hmm, so this requires an in-code comment.

But what I am not grokking: this detaches the queue, but if a carrier comes, then something needs to attach the queue again? what am i missing here?

(my insight into tun/tap behaviour is a bit superficial, in case you didn't notice)

[KonishchevDmitry]

When we open tun/tap device, kernel creates a queue for each open FD.

Link is considered to have carrier if its open FD count is not zero.

When packet arrives, kernel selects a queue to send the packet to. From a quick look at the kernel sources the logic is something like "take current CPU ID and map it to index in active queue list". So if some packets are handled by CPU #N and N maps to our queue, all these packets will be sent to our queue - even if no one reads it.

So from my view the whole picture should be the following:

• I create a tun/tap device using networkd, which properly configures it and leaves FD open, but disables its queue. So kernel considering the device as connected.
• Then I start some rootless service which opens the device and kernels starts send all packets to its queue(s).
• When the service stops, we have one open FD (by networkd) and 0 active and 1 disabled queue (networkd's). Kernel remains to consider link as active, and continues to route packets to it, but they are effectively dropped since we don't have active queues now. And this is desired behaviour for us - for example in case of VPN. In other case we might need to create some blackhole routing table with lower priority to make kernel send packets to it when the main interface lose its carrier.
• When the service starts again, we have one active queue and packets start flow to it.

[poettering]

ok, this makes more sense to me now.

should we default to MultiQueue=true btw?

will leave final review to @yuwata

[KonishchevDmitry]

should we default to MultiQueue=true btw?

I would say no, because it will break backward compatibility: when you attach to existing tun/tap device, you must specify the same flags it was created with.

[yuwata]

LGTM. Sorry for late review.