On Wed, Jan 15, 2025 at 1:17 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote: > > On 2025/01/13 11:59, Jason Wang wrote: > > On Sat, Jan 11, 2025 at 1:43 PM Akihiko Odaki <akihiko.od...@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.od...@daynix.com> wrote: > >>>>> > >>>>> On 2024/10/09 16:41, Jason Wang wrote: > >>>>>> On Tue, Oct 8, 2024 at 2:52 PM Akihiko Odaki > >>>>>> <akihiko.od...@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.od...@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 > >> > > >