[PATCH] tap-linux: Open ipvtap and macvtap

[PATCH] tap-linux: Open ipvtap and macvtap

[Akihiko Odaki]

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.

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));
+        if (fd < 0) {
+            error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
+            return -1;
+        }
     }
     memset(&ifr, 0, sizeof(ifr));
     ifr.ifr_flags = IFF_TAP | IFF_NO_PI;

---
base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
change-id: 20241008-macvtap-b152e5abb457

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

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

[Jason Wang]

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).

> 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 (fd < 0) {
> +            error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
> +            return -1;
> +        }
>      }
>      memset(&ifr, 0, sizeof(ifr));
>      ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>
> ---
> base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
> change-id: 20241008-macvtap-b152e5abb457
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>

Thanks

>

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

[Akihiko Odaki]

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.

> 
>> 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.

This is also consistent with libvirt. libvirt first checks if 
g_strdup_printf("/dev/tap%d", ifindex) exists, and falls back to tuntap 
otherwise.

Regards,
Akihiko Odaki

> 
>> +        if (fd < 0) {
>> +            error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
>> +            return -1;
>> +        }
>>       }
>>       memset(&ifr, 0, sizeof(ifr));
>>       ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>>
>> ---
>> base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
>> change-id: 20241008-macvtap-b152e5abb457
>>
>> Best regards,
>> --
>> Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Thanks
> 
>>
> 

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

[Jason Wang]

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.

>
> >
> >> 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?

>
> 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.

Thanks

>
> Regards,
> Akihiko Odaki
>
> >
> >> +        if (fd < 0) {
> >> +            error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
> >> +            return -1;
> >> +        }
> >>       }
> >>       memset(&ifr, 0, sizeof(ifr));
> >>       ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> >>
> >> ---
> >> base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
> >> change-id: 20241008-macvtap-b152e5abb457
> >>
> >> Best regards,
> >> --
> >> Akihiko Odaki <akihiko.odaki@daynix.com>
> >
> > Thanks
> >
> >>
> >
>

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

[Akihiko Odaki]

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

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

[Akihiko Odaki]

Hi Jason,

Can you check this patch again?

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

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
>

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

[Akihiko Odaki]

On 2025/01/13 11:59, Jason Wang wrote:
> 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

I need this to make QEMU work with macvtap on mkosi, and this patch is 
an effective way to accomplish the goal.

Requiring to pass a file descriptor is simply less convenient. Most (if 
not all) aspects of QEMU can be configured without file descriptors; I 
don't think there is a reason to make tap exceptional.

Regards,
Akihiko Odaki

> 
> 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
>>
> 

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

[Jason Wang]

On Wed, Jan 15, 2025 at 1:17 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2025/01/13 11:59, Jason Wang wrote:
> > 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
>
> I need this to make QEMU work with macvtap on mkosi, and this patch is
> an effective way to accomplish the goal.

I'm not sure how to define "effective" here.

>
> Requiring to pass a file descriptor is simply less convenient. Most (if
> not all) aspects of QEMU can be configured without file descriptors; I
> don't think there is a reason to make tap exceptional.

TUNSETIFF requires CAP_NET_ADMIN and qemu doesn't want to run with
privilege, so fd is prefered in the case of tuntap.

For macvtap,ipvtap, though open, doesn't require any privilege.
Passing fd via SCM_RIGHTS is still preferable as it eases the
interaction with security facilities (for example, you may want to
whitelist /dev/tapX for Qemu to access etc).

Thanks

>
> Regards,
> Akihiko Odaki
>
> >
> > 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
> >>
> >
>

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

[Akihiko Odaki]

On 2025/01/16 10:17, Jason Wang wrote:
> On Wed, Jan 15, 2025 at 1:17 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> On 2025/01/13 11:59, Jason Wang wrote:
>>> 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
>>
>> I need this to make QEMU work with macvtap on mkosi, and this patch is
>> an effective way to accomplish the goal.
> 
> I'm not sure how to define "effective" here.

I just meant it requires me writing less code.

> 
>>
>> Requiring to pass a file descriptor is simply less convenient. Most (if
>> not all) aspects of QEMU can be configured without file descriptors; I
>> don't think there is a reason to make tap exceptional.
> 
> TUNSETIFF requires CAP_NET_ADMIN and qemu doesn't want to run with
> privilege, so fd is prefered in the case of tuntap.
> 
> For macvtap,ipvtap, though open, doesn't require any privilege.
> Passing fd via SCM_RIGHTS is still preferable as it eases the
> interaction with security facilities (for example, you may want to
> whitelist /dev/tapX for Qemu to access etc).

That is true for almost any kind of files, and QEMU provides options to 
specify files with file descriptor for this reason. However, it also 
provides alternative options to specify files with e.g., path for 
convenience.

This patch does not add a entirely-new complex, high-level feature. It 
only pushes the macvtap/ipvtap support to the same level with tuntap and 
other features interacting with files.

Regards,
Akihiko Odaki

> 
> Thanks
> 
>>
>> Regards,
>> Akihiko Odaki
>>
>>>
>>> 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
>>>>
>>>
>>
> 

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

[Jason Wang]

On Thu, Jan 16, 2025 at 1:27 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2025/01/16 10:17, Jason Wang wrote:
> > On Wed, Jan 15, 2025 at 1:17 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> >>
> >> On 2025/01/13 11:59, Jason Wang wrote:
> >>> 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
> >>
> >> I need this to make QEMU work with macvtap on mkosi, and this patch is
> >> an effective way to accomplish the goal.
> >
> > I'm not sure how to define "effective" here.
>
> I just meant it requires me writing less code.
>
> >
> >>
> >> Requiring to pass a file descriptor is simply less convenient. Most (if
> >> not all) aspects of QEMU can be configured without file descriptors; I
> >> don't think there is a reason to make tap exceptional.
> >
> > TUNSETIFF requires CAP_NET_ADMIN and qemu doesn't want to run with
> > privilege, so fd is prefered in the case of tuntap.
> >
> > For macvtap,ipvtap, though open, doesn't require any privilege.
> > Passing fd via SCM_RIGHTS is still preferable as it eases the
> > interaction with security facilities (for example, you may want to
> > whitelist /dev/tapX for Qemu to access etc).
>
> That is true for almost any kind of files, and QEMU provides options to
> specify files with file descriptor for this reason. However, it also
> provides alternative options to specify files with e.g., path for
> convenience.
>
> This patch does not add a entirely-new complex, high-level feature. It
> only pushes the macvtap/ipvtap support to the same level with tuntap and
> other features interacting with files.

Fair enough. I've queued this.

Thanks

>
> Regards,
> Akihiko Odaki
>
> >
> > Thanks
> >
> >>
> >> Regards,
> >> Akihiko Odaki
> >>
> >>>
> >>> 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
> >>>>
> >>>
> >>
> >
>