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

[Akihiko Odaki <akihiko.odaki@daynix.com>]

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