Re: [PATCH] tap-linux: Open ipvtap and macvtap

[Jason Wang]

On Sat, Jan 11, 2025 at 1:43 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Hi Jason,
>
> Can you check this patch again?

I would like to have this if

1) it would be used by libvirt.

or

2) there's no other way to do this

Thanks

>
> Regards,
> Akihiko Odaki
>
> On 2024/10/22 13:59, Akihiko Odaki wrote:
> > On 2024/10/18 17:10, Jason Wang wrote:
> >> On Sat, Oct 12, 2024 at 5:05 PM Akihiko Odaki
> >> <akihiko.odaki@daynix.com> wrote:
> >>>
> >>> On 2024/10/09 16:41, Jason Wang wrote:
> >>>> On Tue, Oct 8, 2024 at 2:52 PM Akihiko Odaki
> >>>> <akihiko.odaki@daynix.com> wrote:
> >>>>>
> >>>>> ipvtap and macvtap create a file for each interface unlike tuntap,
> >>>>> which
> >>>>> creates one file shared by all interfaces. Try to open a file
> >>>>> dedicated
> >>>>> to the interface first for ipvtap and macvtap.
> >>>>>
> >>>>
> >>>> Management layers usually pass these fds via SCM_RIGHTS. Is this for
> >>>> testing purposes? (Note that we can use something like -netdev
> >>>> tap,fd=10 10<>/dev/tap0).
> >>>
> >>> I used this for testing.
> >>
> >> Anything that prevents you from using fd redirection? If not
> >> management interest and we had already had a way for testing, I tend
> >> to not introduce new code as it may bring bugs.
> >
> > I don't know what ifindex the macvtap device has so it's easier to use
> > if QEMU can automatically figure out the it.
> >
> >>
> >>>
> >>>>
> >>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >>>>> ---
> >>>>>    net/tap-linux.c | 17 ++++++++++++++---
> >>>>>    1 file changed, 14 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/net/tap-linux.c b/net/tap-linux.c
> >>>>> index 1226d5fda2d9..22ec2f45d2b7 100644
> >>>>> --- a/net/tap-linux.c
> >>>>> +++ b/net/tap-linux.c
> >>>>> @@ -45,10 +45,21 @@ int tap_open(char *ifname, int ifname_size, int
> >>>>> *vnet_hdr,
> >>>>>        int len = sizeof(struct virtio_net_hdr);
> >>>>>        unsigned int features;
> >>>>>
> >>>>> -    fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));
> >>>>> +
> >>>>> +    ret = if_nametoindex(ifname);
> >>>>> +    if (ret) {
> >>>>> +        g_autofree char *file = g_strdup_printf("/dev/tap%d", ret);
> >>>>> +        fd = open(file, O_RDWR);
> >>>>> +    } else {
> >>>>> +        fd = -1;
> >>>>> +    }
> >>>>> +
> >>>>>        if (fd < 0) {
> >>>>> -        error_setg_errno(errp, errno, "could not open %s",
> >>>>> PATH_NET_TUN);
> >>>>> -        return -1;
> >>>>> +        fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));
> >>>>
> >>>> Any reason tuntap were tried after the macvtap/ipvtap?
> >>>
> >>> If we try tuntap first, we will know that it is not tuntap when calling
> >>> TUNSETIFF. We will need to call TUNGETFEATURES and TUNSETVNETHDRSZ again
> >>> in such a case because they precede TUNSETIFF. Calling them twice is
> >>> troublesome.
> >>
> >> I may miss something, we are only at the phase of open() not TUNSETIFF?
> >
> > We can tell if it is macvtap/ipvtap just by trying opening the device
> > file. That is not possible with tuntap because tuntap uses /dev/net/tun,
> > a device file common for all tuntap interfaces and its presence does not
> > tell if the interface is tuntap.
> >
> >>
> >>>
> >>> This is also consistent with libvirt. libvirt first checks if
> >>> g_strdup_printf("/dev/tap%d", ifindex) exists, and falls back to tuntap
> >>> otherwise.
> >>
> >> This is not what I understand from how layered products work. Libvirt
> >> should align with Qemu for low level things like TAP, not the reverse.
> >
> > This change is intended for the use case where libvirt is not in use. In
> > particular, I use mkosi, which is not a full fledged layering mechanism.
> >
> > Regards,
> > Akihiko Odaki
>