net/tap-linux.c | 5 +- net/tap.c | 548 ++++++++++++++++++++++-------------------------- net/tap_int.h | 2 +- 3 files changed, 256 insertions(+), 299 deletions(-)
Hi all!
As preparation for further development of TAP live local migration
(passing open fds through unix socket), here is a refactoring
of initialization code, to improve its readability and get rid
of duplication.
Below are the initialization flow diagrams showing the changes.
BEFORE REFACTORING:
==================
```
net_init_tap()
|
+-- if (tap->fd)
| +-- duplicated logic*
| +-- net_init_tap_one()
|
+-- else if (tap->fds)
| +-- for each fd:
| +-- duplicated logic*
| +-- net_init_tap_one()
|
+-- else if (tap->helper)
| +-- duplicated logic*
| +-- net_init_bridge()
|
+-- else (normal case)
+-- for each queue:
+-- net_tap_init()
+-- net_init_tap_one()
net_init_bridge()
|
+-- duplicated logic*
+-- net_tap_fd_init()
net_init_tap_one()
|
+-- net_tap_fd_init()
net_tap_init()
|
+-- tap_open()
net_tap_fd_init()
|
+-- qemu_new_net_client()
+-- Initialize TAPState
* duplicated logic: set fd nonblocking + probe vnet_hdr
```
AFTER REFACTORING:
=================
```
net_init_tap()
|
+-- if (tap->fd)
| +-- net_tap_from_monitor_fd()
|
+-- else if (tap->fds)
| +-- for each fd:
| +-- net_tap_from_monitor_fd()
|
+-- else if (tap->helper)
| +-- net_init_bridge()
|
+-- else (normal case)
+-- net_tap_open()
net_tap_open()
|
+-- for each queue:
+-- net_tap_open_one()
net_tap_open_one()
|
+-- tap_open()
+-- net_tap_fd_init_common()
net_tap_from_monitor_fd()
|
+-- net_tap_fd_init_external()
net_tap_fd_init_external()
|
+-- net_tap_fd_init_common()
net_init_bridge()
|
+-- net_tap_fd_init_external()
net_tap_fd_init_common()
|
+-- qemu_new_net_client()
+-- Initialize TAPState
```
Solved problems:
- duplicated logic to handle external
file descriptors (set nonblocking, probe vnet_hdr)
- duplication between tap/helper case in
net_init_tap() and net_init_bridge()
- confusing naming and functionality spread between functions (we had
net_init_tap(), together with net_tap_init(); also main central
function was net_init_tap_one(), and part of its logic (not clear
why) moved to separate net_tap_fd_init()),
Vladimir Sementsov-Ogievskiy (19):
net/tap: net_init_tap_one(): add return value
net/tap: add set_fd_nonblocking() helper
net/tap: tap_set_sndbuf(): add return value
net/tap: net_init_tap_one(): drop extra error propagation
net/tap: net_init_tap_one(): move parameter checking earlier
net/tap: net_init_tap(): refactor parameter checking
net/tap: net_init_tap(): drop extra variable vhostfdname
net/tap: move local variables related to the latter case to else
branch
net/tap: use glib strings vector and g_strsplit for fds case
net/tap: drop extra tap_fd_get_ifname() call
net/tap: net_init_tap_one(): refactor to use netdev as first arg
net/tap: net_init_tap_one(): support bridge
net/tap: net_init_bridge(): support tap
net/tap: refactor net_tap_init() into net_tap_open_one()
net/tap: introduce net_tap_open()
net/tap: introduce net_tap_fd_init_external()
net/tap: introduce net_tap_from_monitor_fd() helper
net/tap: split net_tap_setup_vhost() separate function
net/tap: drop net_tap_fd_init()
net/tap-linux.c | 5 +-
net/tap.c | 548 ++++++++++++++++++++++--------------------------
net/tap_int.h | 2 +-
3 files changed, 256 insertions(+), 299 deletions(-)
--
2.48.1
On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > Hi all! > > As preparation for further development of TAP live local migration > (passing open fds through unix socket), I'm not sure I understand this, but I think it has been supported now, or anything I miss? Thanks
On 19.08.25 05:45, Jason Wang wrote: > On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy > <vsementsov@yandex-team.ru> wrote: >> >> Hi all! >> >> As preparation for further development of TAP live local migration >> (passing open fds through unix socket), > > I'm not sure I understand this, but I think it has been supported now, > or anything I miss? > Hmm, may be I missing something, but where? I see no code which put TAP fd into migration channel. Nothing about migration in tap.c at all. So, normally, to make local migration with TAP device, you have to create a new TAP for new QEMU process. I want to add a migration state, which will include needed part of TAPState, including fd, to continue using same TAP device in target process, avoiding also any initialization steps on that fd. -- Best regards, Vladimir
On Tue, Aug 19, 2025 at 4:41 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > On 19.08.25 05:45, Jason Wang wrote: > > On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy > > <vsementsov@yandex-team.ru> wrote: > >> > >> Hi all! > >> > >> As preparation for further development of TAP live local migration > >> (passing open fds through unix socket), > > > > I'm not sure I understand this, but I think it has been supported now, > > or anything I miss? > > > > Hmm, may be I missing something, but where? > > I see no code which put TAP fd into migration channel. Nothing about migration in tap.c at all. > > So, normally, to make local migration with TAP device, you have to create a new TAP for new QEMU > process. > > I want to add a migration state, which will include needed part of TAPState, including fd, > to continue using same TAP device in target process, avoiding also any initialization steps > on that fd. Ok, I see. But the question is that TAP is not something that is visible to guests. Doing that may be a suboptimal as we need to deal with the migration compatibility. Can we simply teach the management layer to use the same TAP fd instead assuming it's a local migration. Thanks > > -- > Best regards, > Vladimir >
On 20.08.25 05:33, Jason Wang wrote: > On Tue, Aug 19, 2025 at 4:41 PM Vladimir Sementsov-Ogievskiy > <vsementsov@yandex-team.ru> wrote: >> >> On 19.08.25 05:45, Jason Wang wrote: >>> On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy >>> <vsementsov@yandex-team.ru> wrote: >>>> >>>> Hi all! >>>> >>>> As preparation for further development of TAP live local migration >>>> (passing open fds through unix socket), >>> >>> I'm not sure I understand this, but I think it has been supported now, >>> or anything I miss? >>> >> >> Hmm, may be I missing something, but where? >> >> I see no code which put TAP fd into migration channel. Nothing about migration in tap.c at all. >> >> So, normally, to make local migration with TAP device, you have to create a new TAP for new QEMU >> process. >> >> I want to add a migration state, which will include needed part of TAPState, including fd, >> to continue using same TAP device in target process, avoiding also any initialization steps >> on that fd. > > Ok, I see. But the question is that TAP is not something that is > visible to guests. Doing that may be a suboptimal as we need to deal > with the migration compatibility. > > Can we simply teach the management layer to use the same TAP fd > instead assuming it's a local migration. > Theoretically yes: we can pass same fds to the target. We'll still need some new option, to avoid some operations with TAP on initialization (as when we initialize it on target, source is still running, and TAP should work on source), but that's possible. Still, at least in our case, making an additional migration state looks more straightforward. 1. Management layer doesn't store these fds. And its not simple to store them in our architecture, as management layer just doesn't live as long as QEMU, it's being restarted more often. 2. So, we'll need to add interfaces to get all these fds from QEMU, and than pass them to the target QEMU. So, it's a lot simpler for QEMU to care about these fds. QEMU owns them, why not to pass them through migration stream? Also, that's not a precedent of passing fds (which are not visible to the guest) through migration channel, see docs/devel/migration/CPR.rst. (Hmm, I missed that Steve recently resent his "[RFC V2 0/8] Live update: tap and vhost", which transfers TAP fds through CPR state.. I still think, using one migration channel is quite possible and more convenient. But that's a topic to discuss on corresponding series, and this one is just a refactoring, which just make the code more readable) > >> >> -- >> Best regards, >> Vladimir >> > -- Best regards, Vladimir
Hi Vladimir From the QE perspective, this series of patches cause a regression issues for multi queues: [qemu output] qemu-system-x86_64: -netdev ..."vhostfds": "20:21:22:23", "fds": "10:16:17:19" : vhostfds= is invalid if fds= wasn't specified Thanks Lei On Tue, Aug 19, 2025 at 4:42 PM Vladimir Sementsov-Ogievskiy < vsementsov@yandex-team.ru> wrote: > On 19.08.25 05:45, Jason Wang wrote: > > On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy > > <vsementsov@yandex-team.ru> wrote: > >> > >> Hi all! > >> > >> As preparation for further development of TAP live local migration > >> (passing open fds through unix socket), > > > > I'm not sure I understand this, but I think it has been supported now, > > or anything I miss? > > > > Hmm, may be I missing something, but where? > > I see no code which put TAP fd into migration channel. Nothing about > migration in tap.c at all. > > So, normally, to make local migration with TAP device, you have to create > a new TAP for new QEMU > process. > > I want to add a migration state, which will include needed part of > TAPState, including fd, > to continue using same TAP device in target process, avoiding also any > initialization steps > on that fd. > > -- > Best regards, > Vladimir > >
On 19.08.25 16:14, Lei Yang wrote: > Hi Vladimir > > From the QE perspective, this series of patches cause a regression issues for multi queues: > [qemu output] qemu-system-x86_64: -netdev ..."vhostfds": "20:21:22:23", "fds": "10:16:17:19" : vhostfds= is invalid if fds= wasn't specified Oh right, thanks, I see the mistake in "[PATCH 15/19] net/tap: introduce net_tap_open()", I've refactored the end of net_init_tap() like it's only for "open" case, but fds case doesn't have own "return" operator actually, and goes to the end and fails on the last check. Will fix. Which tests should I execute? At least "make check" doesn't see this problem. > > Thanks > Lei > > > On Tue, Aug 19, 2025 at 4:42 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote: > > On 19.08.25 05:45, Jason Wang wrote: > > On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy > > <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote: > >> > >> Hi all! > >> > >> As preparation for further development of TAP live local migration > >> (passing open fds through unix socket), > > > > I'm not sure I understand this, but I think it has been supported now, > > or anything I miss? > > > > Hmm, may be I missing something, but where? > > I see no code which put TAP fd into migration channel. Nothing about migration in tap.c at all. > > So, normally, to make local migration with TAP device, you have to create a new TAP for new QEMU > process. > > I want to add a migration state, which will include needed part of TAPState, including fd, > to continue using same TAP device in target process, avoiding also any initialization steps > on that fd. > > -- > Best regards, > Vladimir > -- Best regards, Vladimir
On Tue, Aug 19, 2025 at 9:28 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > On 19.08.25 16:14, Lei Yang wrote: > > Hi Vladimir > > > > From the QE perspective, this series of patches cause a regression issues for multi queues: > > [qemu output] qemu-system-x86_64: -netdev ..."vhostfds": "20:21:22:23", "fds": "10:16:17:19" : vhostfds= is invalid if fds= wasn't specified > > Oh right, thanks, I see the mistake in "[PATCH 15/19] net/tap: introduce net_tap_open()", I've refactored the end of net_init_tap() like it's only for "open" case, but fds case doesn't have own "return" operator actually, and goes to the end and fails on the last check. Will fix. > > Which tests should I execute? At least "make check" doesn't see this problem. The test step is to use the qemu binary with the current patch applied to boot a guest that uses a multi-queues nic. I will test this series of patches again when you update to V2. > > > > > Thanks > > Lei > > > > > > On Tue, Aug 19, 2025 at 4:42 PM Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote: > > > > On 19.08.25 05:45, Jason Wang wrote: > > > On Mon, Aug 18, 2025 at 10:06 PM Vladimir Sementsov-Ogievskiy > > > <vsementsov@yandex-team.ru <mailto:vsementsov@yandex-team.ru>> wrote: > > >> > > >> Hi all! > > >> > > >> As preparation for further development of TAP live local migration > > >> (passing open fds through unix socket), > > > > > > I'm not sure I understand this, but I think it has been supported now, > > > or anything I miss? > > > > > > > Hmm, may be I missing something, but where? > > > > I see no code which put TAP fd into migration channel. Nothing about migration in tap.c at all. > > > > So, normally, to make local migration with TAP device, you have to create a new TAP for new QEMU > > process. > > > > I want to add a migration state, which will include needed part of TAPState, including fd, > > to continue using same TAP device in target process, avoiding also any initialization steps > > on that fd. > > > > -- > > Best regards, > > Vladimir > > > > > -- > Best regards, > Vladimir >
© 2016 - 2025 Red Hat, Inc.