Handle local-incoming option:
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
include/net/tap.h | 4 ++
net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
2 files changed, 119 insertions(+), 21 deletions(-)
diff --git a/include/net/tap.h b/include/net/tap.h
index 6f34f13eae..3ef2e2dbae 100644
--- a/include/net/tap.h
+++ b/include/net/tap.h
@@ -30,7 +30,11 @@
int tap_enable(NetClientState *nc);
int tap_disable(NetClientState *nc);
+bool tap_local_incoming(NetClientState *nc);
int tap_get_fd(NetClientState *nc);
+int tap_load(NetClientState *nc, QEMUFile *f);
+int tap_save(NetClientState *nc, QEMUFile *f);
+
#endif /* QEMU_NET_TAP_H */
diff --git a/net/tap.c b/net/tap.c
index a9d955ac5f..499db756ea 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -35,6 +35,8 @@
#include "net/eth.h"
#include "net/net.h"
#include "clients.h"
+#include "migration/migration.h"
+#include "migration/qemu-file.h"
#include "monitor/monitor.h"
#include "system/system.h"
#include "qapi/error.h"
@@ -82,6 +84,7 @@ typedef struct TAPState {
VHostNetState *vhost_net;
unsigned host_vnet_hdr_len;
Notifier exit;
+ bool local_incoming;
} TAPState;
static void launch_script(const char *setup_script, const char *ifname,
@@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
return 0;
}
+int tap_save(NetClientState *nc, QEMUFile *f)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ qemu_file_put_fd(f, s->fd);
+ qemu_put_byte(f, s->using_vnet_hdr);
+ qemu_put_byte(f, s->has_ufo);
+ qemu_put_byte(f, s->has_uso);
+ qemu_put_byte(f, s->enabled);
+ qemu_put_be32(f, s->host_vnet_hdr_len);
+
+ return 0;
+}
+
+int tap_load(NetClientState *nc, QEMUFile *f)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ s->fd = qemu_file_get_fd(f);
+ if (s->fd < 0) {
+ return -1;
+ }
+
+ s->using_vnet_hdr = qemu_get_byte(f);
+ s->has_ufo = qemu_get_byte(f);
+ s->has_uso = qemu_get_byte(f);
+ s->enabled = qemu_get_byte(f);
+ qemu_get_be32s(f, &s->host_vnet_hdr_len);
+
+ tap_read_poll(s, true);
+
+ return net_tap_init_vhost(s, NULL);
+}
+
static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
const char *model, const char *name,
const char *ifname, const char *script,
@@ -810,30 +847,40 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
const char *vhostfdname,
int vnet_hdr, int fd, Error **errp)
{
- const NetdevTapOptions *tap;
+ const NetdevTapOptions *tap = NULL;
int ret;
NetClientState *nc;
TAPState *s;
+ bool local_incoming = false;
+
+ if (netdev->type == NET_CLIENT_DRIVER_TAP) {
+ tap = &netdev->u.tap;
+ local_incoming = tap->local_incoming;
+ }
nc = qemu_new_net_client(&net_tap_info, peer, model, name);
s = DO_UPCAST(TAPState, nc, nc);
-
- s->fd = fd;
- s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
- s->using_vnet_hdr = false;
- s->has_ufo = tap_probe_has_ufo(s->fd);
- s->has_uso = tap_probe_has_uso(s->fd);
- s->enabled = true;
- tap_set_offload(&s->nc, 0, 0, 0, 0, 0, 0, 0);
- /*
- * Make sure host header length is set correctly in tap:
- * it might have been modified by another instance of qemu.
- */
- if (vnet_hdr) {
- tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len);
+ s->local_incoming = local_incoming;
+
+ if (!local_incoming) {
+ s->fd = fd;
+ s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
+ s->using_vnet_hdr = false;
+ s->has_ufo = tap_probe_has_ufo(s->fd);
+ s->has_uso = tap_probe_has_uso(s->fd);
+ s->enabled = true;
+ tap_set_offload(&s->nc, 0, 0, 0, 0, 0, 0, 0);
+ /*
+ * Make sure host header length is set correctly in tap:
+ * it might have been modified by another instance of qemu.
+ */
+ if (vnet_hdr) {
+ tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len);
+ }
+ tap_read_poll(s, true);
}
- tap_read_poll(s, true);
+
s->vhost_net = NULL;
s->exit.notify = NULL;
@@ -845,9 +892,8 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
}
assert(netdev->type == NET_CLIENT_DRIVER_TAP);
- tap = &netdev->u.tap;
- if (tap_set_sndbuf(s->fd, tap, errp) < 0) {
+ if (!local_incoming && tap_set_sndbuf(s->fd, tap, errp) < 0) {
goto failed;
}
@@ -873,9 +919,11 @@ static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
goto failed;
}
- ret = net_tap_init_vhost(s, errp);
- if (ret < 0) {
- goto failed;
+ if (!local_incoming) {
+ ret = net_tap_init_vhost(s, errp);
+ if (ret < 0) {
+ goto failed;
+ }
}
return 0;
@@ -942,6 +990,38 @@ static int net_tap_open(const Netdev *netdev,
return 0;
}
+static int net_init_local_incoming(const Netdev *netdev,
+ const char *name,
+ NetClientState *peer,
+ Error **errp)
+{
+ const NetdevTapOptions *tap = &netdev->u.tap;
+ const char *downscript = tap->downscript;
+ int queues = tap->has_queues ? tap->queues : 1;
+ g_autofree char *default_downscript = NULL;
+ int i, ret;
+
+ assert(netdev->type == NET_CLIENT_DRIVER_TAP);
+ assert(!tap->script);
+
+ if (!downscript) {
+ downscript = default_downscript =
+ get_relocated_path(DEFAULT_NETWORK_DOWN_SCRIPT);
+ }
+
+ for (i = 0; i < queues; i++) {
+ ret = net_tap_fd_init_common(netdev, peer, "tap", name,
+ tap->ifname ?: "",
+ "no", downscript,
+ tap->vhostfd, -1, -1, errp);
+ if (ret < 0) {
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int net_init_tap_fds(const Netdev *netdev, const char *name,
NetClientState *peer, Error **errp)
{
@@ -1030,6 +1110,13 @@ int net_init_tap(const Netdev *netdev, const char *name,
}
return net_init_bridge(netdev, name, peer, errp);
+ } else if (tap->local_incoming) {
+ if (tap->script) {
+ error_setg(errp, "script= is invalid with local-incoming");
+ return -1;
+ }
+
+ return net_init_local_incoming(netdev, name, peer, errp);
}
if (tap->vhostfds) {
@@ -1078,3 +1165,10 @@ int tap_disable(NetClientState *nc)
return ret;
}
}
+
+bool tap_local_incoming(NetClientState *nc)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ return s->local_incoming && runstate_check(RUN_STATE_INMIGRATE);
+}
--
2.48.1On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Handle local-incoming option:
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> include/net/tap.h | 4 ++
> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 119 insertions(+), 21 deletions(-)
>
> diff --git a/include/net/tap.h b/include/net/tap.h
> index 6f34f13eae..3ef2e2dbae 100644
> --- a/include/net/tap.h
> +++ b/include/net/tap.h
> @@ -30,7 +30,11 @@
>
> int tap_enable(NetClientState *nc);
> int tap_disable(NetClientState *nc);
> +bool tap_local_incoming(NetClientState *nc);
>
> int tap_get_fd(NetClientState *nc);
>
> +int tap_load(NetClientState *nc, QEMUFile *f);
> +int tap_save(NetClientState *nc, QEMUFile *f);
> +
> #endif /* QEMU_NET_TAP_H */
> diff --git a/net/tap.c b/net/tap.c
> index a9d955ac5f..499db756ea 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -35,6 +35,8 @@
> #include "net/eth.h"
> #include "net/net.h"
> #include "clients.h"
> +#include "migration/migration.h"
> +#include "migration/qemu-file.h"
> #include "monitor/monitor.h"
> #include "system/system.h"
> #include "qapi/error.h"
> @@ -82,6 +84,7 @@ typedef struct TAPState {
> VHostNetState *vhost_net;
> unsigned host_vnet_hdr_len;
> Notifier exit;
> + bool local_incoming;
> } TAPState;
>
> static void launch_script(const char *setup_script, const char *ifname,
> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
> return 0;
> }
>
> +int tap_save(NetClientState *nc, QEMUFile *f)
> +{
> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
> +
> + qemu_file_put_fd(f, s->fd);
> + qemu_put_byte(f, s->using_vnet_hdr);
> + qemu_put_byte(f, s->has_ufo);
> + qemu_put_byte(f, s->has_uso);
> + qemu_put_byte(f, s->enabled);
> + qemu_put_be32(f, s->host_vnet_hdr_len);
Is it neccessary to transfer that metadata, or is there perhaps a way
for the other side to query the TAP FD configuration from the kernel
to detect this ?
I'm concerned that this code / wire format is not extensible if we ever
add another similar field to TAPState in the future.
> +
> + return 0;
> +}
> +
> +int tap_load(NetClientState *nc, QEMUFile *f)
> +{
> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
> +
> + s->fd = qemu_file_get_fd(f);
> + if (s->fd < 0) {
> + return -1;
> + }
> +
> + s->using_vnet_hdr = qemu_get_byte(f);
> + s->has_ufo = qemu_get_byte(f);
> + s->has_uso = qemu_get_byte(f);
> + s->enabled = qemu_get_byte(f);
> + qemu_get_be32s(f, &s->host_vnet_hdr_len);
> +
> + tap_read_poll(s, true);
> +
> + return net_tap_init_vhost(s, NULL);
> +}
> +
> static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
> const char *model, const char *name,
> const char *ifname, const char *script,
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 03.09.25 17:34, Daniel P. Berrangé wrote:
> On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Handle local-incoming option:
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>> include/net/tap.h | 4 ++
>> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
>> 2 files changed, 119 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/net/tap.h b/include/net/tap.h
>> index 6f34f13eae..3ef2e2dbae 100644
>> --- a/include/net/tap.h
>> +++ b/include/net/tap.h
>> @@ -30,7 +30,11 @@
>>
>> int tap_enable(NetClientState *nc);
>> int tap_disable(NetClientState *nc);
>> +bool tap_local_incoming(NetClientState *nc);
>>
>> int tap_get_fd(NetClientState *nc);
>>
>> +int tap_load(NetClientState *nc, QEMUFile *f);
>> +int tap_save(NetClientState *nc, QEMUFile *f);
>> +
>> #endif /* QEMU_NET_TAP_H */
>> diff --git a/net/tap.c b/net/tap.c
>> index a9d955ac5f..499db756ea 100644
>> --- a/net/tap.c
>> +++ b/net/tap.c
>> @@ -35,6 +35,8 @@
>> #include "net/eth.h"
>> #include "net/net.h"
>> #include "clients.h"
>> +#include "migration/migration.h"
>> +#include "migration/qemu-file.h"
>> #include "monitor/monitor.h"
>> #include "system/system.h"
>> #include "qapi/error.h"
>> @@ -82,6 +84,7 @@ typedef struct TAPState {
>> VHostNetState *vhost_net;
>> unsigned host_vnet_hdr_len;
>> Notifier exit;
>> + bool local_incoming;
>> } TAPState;
>>
>> static void launch_script(const char *setup_script, const char *ifname,
>> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
>> return 0;
>> }
>>
>> +int tap_save(NetClientState *nc, QEMUFile *f)
>> +{
>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>> +
>> + qemu_file_put_fd(f, s->fd);
>> + qemu_put_byte(f, s->using_vnet_hdr);
>> + qemu_put_byte(f, s->has_ufo);
>> + qemu_put_byte(f, s->has_uso);
>> + qemu_put_byte(f, s->enabled);
>> + qemu_put_be32(f, s->host_vnet_hdr_len);
>
>
> Is it neccessary to transfer that metadata, or is there perhaps a way
> for the other side to query the TAP FD configuration from the kernel
> to detect this ?
Oh, good question, thanks for it. I just added everything and then I was debugging other places.
for hdr_len we have TUNGETVNETHDRSZ, so it's possible.
using_vnet_hdr, seems is equal to initial vnet_hdr option (with default to 1 if not specified), will doublecheck
for ufo/uso, which are set through TUNSETOFFLOAD, we don't have direct way to
get the state. But we can use the fact, that qemu tries to set them once,
and these variables are unchanged after initialization. So we can try set
same flags on target the same way, to understand what we have. Still,
this doesn't seem absolutely safe.. Kernel may behave differently than
for previous initialization, probably due to some changed settings.
for enabled it seems not possible, but we handle it in virtio layer.. Oops,
probably I always migrate enabled=false with this code, will check.
---
On the other hand, calling extra ioctls to learn something lead to extra downtime
(should be measured to be a good argument).
Also, just architecturally: seems better not ask third agent about metadata that we already know.
---
About forward-compatibility (if we add new fields here) - agree.
Maybe turn several boolean fields into one flags field. This way we'll get several "reserved" bits for future changes.
>
> I'm concerned that this code / wire format is not extensible if we ever
> add another similar field to TAPState in the future.
>
>> +
>> + return 0;
>> +}
>> +
>> +int tap_load(NetClientState *nc, QEMUFile *f)
>> +{
>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>> +
>> + s->fd = qemu_file_get_fd(f);
>> + if (s->fd < 0) {
>> + return -1;
>> + }
>> +
>> + s->using_vnet_hdr = qemu_get_byte(f);
>> + s->has_ufo = qemu_get_byte(f);
>> + s->has_uso = qemu_get_byte(f);
>> + s->enabled = qemu_get_byte(f);
>> + qemu_get_be32s(f, &s->host_vnet_hdr_len);
>> +
>> + tap_read_poll(s, true);
>> +
>> + return net_tap_init_vhost(s, NULL);
>> +}
>> +
>> static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
>> const char *model, const char *name,
>> const char *ifname, const char *script,
>
> With regards,
> Daniel
--
Best regards,
Vladimir
On 9/3/2025 11:31 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 03.09.25 17:34, Daniel P. Berrangé wrote:
>> On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>> Handle local-incoming option:
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>> ---
>>> include/net/tap.h | 4 ++
>>> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
>>> 2 files changed, 119 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/include/net/tap.h b/include/net/tap.h
>>> index 6f34f13eae..3ef2e2dbae 100644
>>> --- a/include/net/tap.h
>>> +++ b/include/net/tap.h
>>> @@ -30,7 +30,11 @@
>>> int tap_enable(NetClientState *nc);
>>> int tap_disable(NetClientState *nc);
>>> +bool tap_local_incoming(NetClientState *nc);
>>> int tap_get_fd(NetClientState *nc);
>>> +int tap_load(NetClientState *nc, QEMUFile *f);
>>> +int tap_save(NetClientState *nc, QEMUFile *f);
>>> +
>>> #endif /* QEMU_NET_TAP_H */
>>> diff --git a/net/tap.c b/net/tap.c
>>> index a9d955ac5f..499db756ea 100644
>>> --- a/net/tap.c
>>> +++ b/net/tap.c
>>> @@ -35,6 +35,8 @@
>>> #include "net/eth.h"
>>> #include "net/net.h"
>>> #include "clients.h"
>>> +#include "migration/migration.h"
>>> +#include "migration/qemu-file.h"
>>> #include "monitor/monitor.h"
>>> #include "system/system.h"
>>> #include "qapi/error.h"
>>> @@ -82,6 +84,7 @@ typedef struct TAPState {
>>> VHostNetState *vhost_net;
>>> unsigned host_vnet_hdr_len;
>>> Notifier exit;
>>> + bool local_incoming;
>>> } TAPState;
>>> static void launch_script(const char *setup_script, const char *ifname,
>>> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
>>> return 0;
>>> }
>>> +int tap_save(NetClientState *nc, QEMUFile *f)
>>> +{
>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>> +
>>> + qemu_file_put_fd(f, s->fd);
>>> + qemu_put_byte(f, s->using_vnet_hdr);
>>> + qemu_put_byte(f, s->has_ufo);
>>> + qemu_put_byte(f, s->has_uso);
>>> + qemu_put_byte(f, s->enabled);
>>> + qemu_put_be32(f, s->host_vnet_hdr_len);
>>
>>
>> Is it neccessary to transfer that metadata, or is there perhaps a way
>> for the other side to query the TAP FD configuration from the kernel
>> to detect this ?
>
> Oh, good question, thanks for it. I just added everything and then I was debugging other places.
>
> for hdr_len we have TUNGETVNETHDRSZ, so it's possible.
>
> using_vnet_hdr, seems is equal to initial vnet_hdr option (with default to 1 if not specified), will doublecheck
>
> for ufo/uso, which are set through TUNSETOFFLOAD, we don't have direct way to
> get the state. But we can use the fact, that qemu tries to set them once,
> and these variables are unchanged after initialization. So we can try set
> same flags on target the same way, to understand what we have. Still,
> this doesn't seem absolutely safe.. Kernel may behave differently than
> for previous initialization, probably due to some changed settings.
>
> for enabled it seems not possible, but we handle it in virtio layer.. Oops,
> probably I always migrate enabled=false with this code, will check.
>
> ---
>
> On the other hand, calling extra ioctls to learn something lead to extra downtime
> (should be measured to be a good argument).
>
> Also, just architecturally: seems better not ask third agent about metadata that we already know.
>
> ---
>
> About forward-compatibility (if we add new fields here) - agree.
>
> Maybe turn several boolean fields into one flags field. This way we'll get several "reserved" bits for future changes.
>
>>
>> I'm concerned that this code / wire format is not extensible if we ever
>> add another similar field to TAPState in the future.
tap_save and tap_load should be replaced with a VMStateDescription for future
extensibility. Use VMSTATE_FD for the fd. Define a postload hook for
tap_read_poll and net_tap_init_vhost.
- Steve
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int tap_load(NetClientState *nc, QEMUFile *f)
>>> +{
>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>> +
>>> + s->fd = qemu_file_get_fd(f);
>>> + if (s->fd < 0) {
>>> + return -1;
>>> + }
>>> +
>>> + s->using_vnet_hdr = qemu_get_byte(f);
>>> + s->has_ufo = qemu_get_byte(f);
>>> + s->has_uso = qemu_get_byte(f);
>>> + s->enabled = qemu_get_byte(f);
>>> + qemu_get_be32s(f, &s->host_vnet_hdr_len);
>>> +
>>> + tap_read_poll(s, true);
>>> +
>>> + return net_tap_init_vhost(s, NULL);
>>> +}
>>> +
>>> static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
>>> const char *model, const char *name,
>>> const char *ifname, const char *script,
>>
>> With regards,
>> Daniel
>
>
On 03.09.25 19:09, Steven Sistare wrote:
> On 9/3/2025 11:31 AM, Vladimir Sementsov-Ogievskiy wrote:
>> On 03.09.25 17:34, Daniel P. Berrangé wrote:
>>> On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> Handle local-incoming option:
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>> ---
>>>> include/net/tap.h | 4 ++
>>>> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
>>>> 2 files changed, 119 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/include/net/tap.h b/include/net/tap.h
>>>> index 6f34f13eae..3ef2e2dbae 100644
>>>> --- a/include/net/tap.h
>>>> +++ b/include/net/tap.h
>>>> @@ -30,7 +30,11 @@
>>>> int tap_enable(NetClientState *nc);
>>>> int tap_disable(NetClientState *nc);
>>>> +bool tap_local_incoming(NetClientState *nc);
>>>> int tap_get_fd(NetClientState *nc);
>>>> +int tap_load(NetClientState *nc, QEMUFile *f);
>>>> +int tap_save(NetClientState *nc, QEMUFile *f);
>>>> +
>>>> #endif /* QEMU_NET_TAP_H */
>>>> diff --git a/net/tap.c b/net/tap.c
>>>> index a9d955ac5f..499db756ea 100644
>>>> --- a/net/tap.c
>>>> +++ b/net/tap.c
>>>> @@ -35,6 +35,8 @@
>>>> #include "net/eth.h"
>>>> #include "net/net.h"
>>>> #include "clients.h"
>>>> +#include "migration/migration.h"
>>>> +#include "migration/qemu-file.h"
>>>> #include "monitor/monitor.h"
>>>> #include "system/system.h"
>>>> #include "qapi/error.h"
>>>> @@ -82,6 +84,7 @@ typedef struct TAPState {
>>>> VHostNetState *vhost_net;
>>>> unsigned host_vnet_hdr_len;
>>>> Notifier exit;
>>>> + bool local_incoming;
>>>> } TAPState;
>>>> static void launch_script(const char *setup_script, const char *ifname,
>>>> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
>>>> return 0;
>>>> }
>>>> +int tap_save(NetClientState *nc, QEMUFile *f)
>>>> +{
>>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>>> +
>>>> + qemu_file_put_fd(f, s->fd);
>>>> + qemu_put_byte(f, s->using_vnet_hdr);
>>>> + qemu_put_byte(f, s->has_ufo);
>>>> + qemu_put_byte(f, s->has_uso);
>>>> + qemu_put_byte(f, s->enabled);
>>>> + qemu_put_be32(f, s->host_vnet_hdr_len);
>>>
>>>
>>> Is it neccessary to transfer that metadata, or is there perhaps a way
>>> for the other side to query the TAP FD configuration from the kernel
>>> to detect this ?
>>
>> Oh, good question, thanks for it. I just added everything and then I was debugging other places.
>>
>> for hdr_len we have TUNGETVNETHDRSZ, so it's possible.
>>
>> using_vnet_hdr, seems is equal to initial vnet_hdr option (with default to 1 if not specified), will doublecheck
>>
>> for ufo/uso, which are set through TUNSETOFFLOAD, we don't have direct way to
>> get the state. But we can use the fact, that qemu tries to set them once,
>> and these variables are unchanged after initialization. So we can try set
>> same flags on target the same way, to understand what we have. Still,
>> this doesn't seem absolutely safe.. Kernel may behave differently than
>> for previous initialization, probably due to some changed settings.
>>
>> for enabled it seems not possible, but we handle it in virtio layer.. Oops,
>> probably I always migrate enabled=false with this code, will check.
>>
>> ---
>>
>> On the other hand, calling extra ioctls to learn something lead to extra downtime
>> (should be measured to be a good argument).
>>
>> Also, just architecturally: seems better not ask third agent about metadata that we already know.
>>
>> ---
>>
>> About forward-compatibility (if we add new fields here) - agree.
>>
>> Maybe turn several boolean fields into one flags field. This way we'll get several "reserved" bits for future changes.
>>
>>>
>>> I'm concerned that this code / wire format is not extensible if we ever
>>> add another similar field to TAPState in the future.
>
> tap_save and tap_load should be replaced with a VMStateDescription for future
> extensibility. Use VMSTATE_FD for the fd. Define a postload hook for
> tap_read_poll and net_tap_init_vhost.
How it works? I thought, if I add new field to vmsd, destination will try to load it anyway (as it loads them in a loop in vmstate_load_state()).. So, we'll have to add same new capabilities anyway to "enable" new fields (with help of .field_exists)? Same way we can add new field to current realization, with new migration capability and "if" in _load() function..
Still, seems using VMSD is better anyway, so I should do it.
>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int tap_load(NetClientState *nc, QEMUFile *f)
>>>> +{
>>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>>> +
>>>> + s->fd = qemu_file_get_fd(f);
>>>> + if (s->fd < 0) {
>>>> + return -1;
>>>> + }
>>>> +
>>>> + s->using_vnet_hdr = qemu_get_byte(f);
>>>> + s->has_ufo = qemu_get_byte(f);
>>>> + s->has_uso = qemu_get_byte(f);
>>>> + s->enabled = qemu_get_byte(f);
>>>> + qemu_get_be32s(f, &s->host_vnet_hdr_len);
>>>> +
>>>> + tap_read_poll(s, true);
>>>> +
>>>> + return net_tap_init_vhost(s, NULL);
>>>> +}
>>>> +
>>>> static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
>>>> const char *model, const char *name,
>>>> const char *ifname, const char *script,
>>>
>>> With regards,
>>> Daniel
>>
>>
>
--
Best regards,
Vladimir
On 04.09.25 10:41, Vladimir Sementsov-Ogievskiy wrote:
> On 03.09.25 19:09, Steven Sistare wrote:
>> On 9/3/2025 11:31 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> On 03.09.25 17:34, Daniel P. Berrangé wrote:
>>>> On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>>> Handle local-incoming option:
>>>>>
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>>> ---
>>>>> include/net/tap.h | 4 ++
>>>>> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
>>>>> 2 files changed, 119 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/include/net/tap.h b/include/net/tap.h
>>>>> index 6f34f13eae..3ef2e2dbae 100644
>>>>> --- a/include/net/tap.h
>>>>> +++ b/include/net/tap.h
>>>>> @@ -30,7 +30,11 @@
>>>>> int tap_enable(NetClientState *nc);
>>>>> int tap_disable(NetClientState *nc);
>>>>> +bool tap_local_incoming(NetClientState *nc);
>>>>> int tap_get_fd(NetClientState *nc);
>>>>> +int tap_load(NetClientState *nc, QEMUFile *f);
>>>>> +int tap_save(NetClientState *nc, QEMUFile *f);
>>>>> +
>>>>> #endif /* QEMU_NET_TAP_H */
>>>>> diff --git a/net/tap.c b/net/tap.c
>>>>> index a9d955ac5f..499db756ea 100644
>>>>> --- a/net/tap.c
>>>>> +++ b/net/tap.c
>>>>> @@ -35,6 +35,8 @@
>>>>> #include "net/eth.h"
>>>>> #include "net/net.h"
>>>>> #include "clients.h"
>>>>> +#include "migration/migration.h"
>>>>> +#include "migration/qemu-file.h"
>>>>> #include "monitor/monitor.h"
>>>>> #include "system/system.h"
>>>>> #include "qapi/error.h"
>>>>> @@ -82,6 +84,7 @@ typedef struct TAPState {
>>>>> VHostNetState *vhost_net;
>>>>> unsigned host_vnet_hdr_len;
>>>>> Notifier exit;
>>>>> + bool local_incoming;
>>>>> } TAPState;
>>>>> static void launch_script(const char *setup_script, const char *ifname,
>>>>> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
>>>>> return 0;
>>>>> }
>>>>> +int tap_save(NetClientState *nc, QEMUFile *f)
>>>>> +{
>>>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>>>> +
>>>>> + qemu_file_put_fd(f, s->fd);
>>>>> + qemu_put_byte(f, s->using_vnet_hdr);
>>>>> + qemu_put_byte(f, s->has_ufo);
>>>>> + qemu_put_byte(f, s->has_uso);
>>>>> + qemu_put_byte(f, s->enabled);
>>>>> + qemu_put_be32(f, s->host_vnet_hdr_len);
>>>>
>>>>
>>>> Is it neccessary to transfer that metadata, or is there perhaps a way
>>>> for the other side to query the TAP FD configuration from the kernel
>>>> to detect this ?
>>>
>>> Oh, good question, thanks for it. I just added everything and then I was debugging other places.
>>>
>>> for hdr_len we have TUNGETVNETHDRSZ, so it's possible.
>>>
>>> using_vnet_hdr, seems is equal to initial vnet_hdr option (with default to 1 if not specified), will doublecheck
>>>
>>> for ufo/uso, which are set through TUNSETOFFLOAD, we don't have direct way to
>>> get the state. But we can use the fact, that qemu tries to set them once,
>>> and these variables are unchanged after initialization. So we can try set
>>> same flags on target the same way, to understand what we have. Still,
>>> this doesn't seem absolutely safe.. Kernel may behave differently than
>>> for previous initialization, probably due to some changed settings.
>>>
>>> for enabled it seems not possible, but we handle it in virtio layer.. Oops,
>>> probably I always migrate enabled=false with this code, will check.
>>>
>>> ---
>>>
>>> On the other hand, calling extra ioctls to learn something lead to extra downtime
>>> (should be measured to be a good argument).
>>>
>>> Also, just architecturally: seems better not ask third agent about metadata that we already know.
>>>
>>> ---
>>>
>>> About forward-compatibility (if we add new fields here) - agree.
>>>
>>> Maybe turn several boolean fields into one flags field. This way we'll get several "reserved" bits for future changes.
>>>
>>>>
>>>> I'm concerned that this code / wire format is not extensible if we ever
>>>> add another similar field to TAPState in the future.
>>
>> tap_save and tap_load should be replaced with a VMStateDescription for future
>> extensibility. Use VMSTATE_FD for the fd. Define a postload hook for
>> tap_read_poll and net_tap_init_vhost.
>
> How it works? I thought, if I add new field to vmsd, destination will try to load it anyway (as it loads them in a loop in vmstate_load_state()).. So, we'll have to add same new capabilities anyway to "enable" new fields (with help of .field_exists)? Same way we can add new field to current realization, with new migration capability and "if" in _load() function..
>
> Still, seems using VMSD is better anyway, so I should do it.
>
answering myself: at least, fields has versioning feature, which is intended for future-compatibility.
>>
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int tap_load(NetClientState *nc, QEMUFile *f)
>>>>> +{
>>>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>>>> +
>>>>> + s->fd = qemu_file_get_fd(f);
>>>>> + if (s->fd < 0) {
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + s->using_vnet_hdr = qemu_get_byte(f);
>>>>> + s->has_ufo = qemu_get_byte(f);
>>>>> + s->has_uso = qemu_get_byte(f);
>>>>> + s->enabled = qemu_get_byte(f);
>>>>> + qemu_get_be32s(f, &s->host_vnet_hdr_len);
>>>>> +
>>>>> + tap_read_poll(s, true);
>>>>> +
>>>>> + return net_tap_init_vhost(s, NULL);
>>>>> +}
>>>>> +
>>>>> static int net_tap_fd_init_common(const Netdev *netdev, NetClientState *peer,
>>>>> const char *model, const char *name,
>>>>> const char *ifname, const char *script,
>>>>
>>>> With regards,
>>>> Daniel
>>>
>>>
>>
>
>
--
Best regards,
Vladimir
On 9/5/2025 6:14 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 04.09.25 10:41, Vladimir Sementsov-Ogievskiy wrote:
>> On 03.09.25 19:09, Steven Sistare wrote:
>>> On 9/3/2025 11:31 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> On 03.09.25 17:34, Daniel P. Berrangé wrote:
>>>>> On Wed, Sep 03, 2025 at 04:37:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>>>> Handle local-incoming option:
>>>>>>
>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>>>> ---
>>>>>> include/net/tap.h | 4 ++
>>>>>> net/tap.c | 136 +++++++++++++++++++++++++++++++++++++++-------
>>>>>> 2 files changed, 119 insertions(+), 21 deletions(-)
>>>>>>
>>>>>> diff --git a/include/net/tap.h b/include/net/tap.h
>>>>>> index 6f34f13eae..3ef2e2dbae 100644
>>>>>> --- a/include/net/tap.h
>>>>>> +++ b/include/net/tap.h
>>>>>> @@ -30,7 +30,11 @@
>>>>>> int tap_enable(NetClientState *nc);
>>>>>> int tap_disable(NetClientState *nc);
>>>>>> +bool tap_local_incoming(NetClientState *nc);
>>>>>> int tap_get_fd(NetClientState *nc);
>>>>>> +int tap_load(NetClientState *nc, QEMUFile *f);
>>>>>> +int tap_save(NetClientState *nc, QEMUFile *f);
>>>>>> +
>>>>>> #endif /* QEMU_NET_TAP_H */
>>>>>> diff --git a/net/tap.c b/net/tap.c
>>>>>> index a9d955ac5f..499db756ea 100644
>>>>>> --- a/net/tap.c
>>>>>> +++ b/net/tap.c
>>>>>> @@ -35,6 +35,8 @@
>>>>>> #include "net/eth.h"
>>>>>> #include "net/net.h"
>>>>>> #include "clients.h"
>>>>>> +#include "migration/migration.h"
>>>>>> +#include "migration/qemu-file.h"
>>>>>> #include "monitor/monitor.h"
>>>>>> #include "system/system.h"
>>>>>> #include "qapi/error.h"
>>>>>> @@ -82,6 +84,7 @@ typedef struct TAPState {
>>>>>> VHostNetState *vhost_net;
>>>>>> unsigned host_vnet_hdr_len;
>>>>>> Notifier exit;
>>>>>> + bool local_incoming;
>>>>>> } TAPState;
>>>>>> static void launch_script(const char *setup_script, const char *ifname,
>>>>>> @@ -803,6 +806,40 @@ static int net_tap_init_vhost(TAPState *s, Error **errp) {
>>>>>> return 0;
>>>>>> }
>>>>>> +int tap_save(NetClientState *nc, QEMUFile *f)
>>>>>> +{
>>>>>> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>>>>> +
>>>>>> + qemu_file_put_fd(f, s->fd);
>>>>>> + qemu_put_byte(f, s->using_vnet_hdr);
>>>>>> + qemu_put_byte(f, s->has_ufo);
>>>>>> + qemu_put_byte(f, s->has_uso);
>>>>>> + qemu_put_byte(f, s->enabled);
>>>>>> + qemu_put_be32(f, s->host_vnet_hdr_len);
>>>>>
>>>>>
>>>>> Is it neccessary to transfer that metadata, or is there perhaps a way
>>>>> for the other side to query the TAP FD configuration from the kernel
>>>>> to detect this ?
>>>>
>>>> Oh, good question, thanks for it. I just added everything and then I was debugging other places.
>>>>
>>>> for hdr_len we have TUNGETVNETHDRSZ, so it's possible.
>>>>
>>>> using_vnet_hdr, seems is equal to initial vnet_hdr option (with default to 1 if not specified), will doublecheck
>>>>
>>>> for ufo/uso, which are set through TUNSETOFFLOAD, we don't have direct way to
>>>> get the state. But we can use the fact, that qemu tries to set them once,
>>>> and these variables are unchanged after initialization. So we can try set
>>>> same flags on target the same way, to understand what we have. Still,
>>>> this doesn't seem absolutely safe.. Kernel may behave differently than
>>>> for previous initialization, probably due to some changed settings.
>>>>
>>>> for enabled it seems not possible, but we handle it in virtio layer.. Oops,
>>>> probably I always migrate enabled=false with this code, will check.
>>>>
>>>> ---
>>>>
>>>> On the other hand, calling extra ioctls to learn something lead to extra downtime
>>>> (should be measured to be a good argument).
>>>>
>>>> Also, just architecturally: seems better not ask third agent about metadata that we already know.
>>>>
>>>> ---
>>>>
>>>> About forward-compatibility (if we add new fields here) - agree.
>>>>
>>>> Maybe turn several boolean fields into one flags field. This way we'll get several "reserved" bits for future changes.
>>>>
>>>>>
>>>>> I'm concerned that this code / wire format is not extensible if we ever
>>>>> add another similar field to TAPState in the future.
>>>
>>> tap_save and tap_load should be replaced with a VMStateDescription for future
>>> extensibility. Use VMSTATE_FD for the fd. Define a postload hook for
>>> tap_read_poll and net_tap_init_vhost.
>>
>> How it works? I thought, if I add new field to vmsd, destination will try to load it anyway (as it loads them in a loop in vmstate_load_state()).. So, we'll have to add same new capabilities anyway to "enable" new fields (with help of .field_exists)? Same way we can add new field to current realization, with new migration capability and "if" in _load() function..
>>
>> Still, seems using VMSD is better anyway, so I should do it.
>
> answering myself: at least, fields has versioning feature, which is intended for future-compatibility.
Plus you can add new fields compatibly by adding a subsection.
See Subsections in docs/devel/migration/main.rst.
- steve
© 2016 - 2025 Red Hat, Inc.