Re: [PATCH 0/9] vhost-user: Migration support

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 17 Jan 2025 14:27:38 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> On 17/01/2025 13:44, Stefano Brivio wrote:
> > On Fri, 17 Jan 2025 13:13:36 +0100
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> On 19/12/2024 12:13, Laurent Vivier wrote:  
> >>> This series allows a QEMU guest to be migrated while it is connected
> >>> to Passt using vhost-user interface.
> >>>
> >>> There are two parts:
> >>>
> >>> - first part enables the migration of QEMU without transferring the
> >>>     internal state of Passt. All connections are lost.
> >>>
> >>>     This is done by implementing VHOST_USER_SET_LOG_FD and
> >>>     VHOST_USER_SET_LOG_BASE commands (and enabling
> >>>     VHOST_USER_PROTOCOL_F_LOG_SHMFD feature)
> >>>
> >>>     "vhost-user: add VHOST_USER_SET_LOG_FD command"
> >>>     "vhost-user: add VHOST_USER_SET_LOG_BASE command"
> >>>     "vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_LOG_SHMFD"
> >>>
> >>> - second part allows source Passt instance to send its internal
> >>>     state using QEMU migration channel to destination Passt instance.
> >>>
> >>>     This is done implementing VHOST_USER_SET_DEVICE_STATE_FD and
> >>>     VHOST_USER_CHECK_DEVICE_STATE (and enabling
> >>>     VHOST_USER_PROTOCOL_F_DEVICE_STATE feature).
> >>>
> >>>     "vhost-user: add VHOST_USER_CHECK_DEVICE_STATE command"
> >>>     "vhost-user: add VHOST_USER_SET_DEVICE_STATE_FD command"
> >>>     "vhost-user: Report to front-end we support VHOST_USER_PROTOCOL_F_DEVICE_STATE"
> >>>
> >>>     For now, it only implements the function needed to transfer the
> >>>     state but no state is transferred.
> >>>
> >>>     VHOST_USER_PROTOCOL_F_DEVICE_STATE is implemented in QEMU
> >>>     only for vhost-user-fs, to be able to use it with virtio-net
> >>>     I have proposed a patch upstream:
> >>>
> >>>     https://patchew.org/QEMU/20241218143453.1573185-1-lvivier@redhat.com/
> >>>
> >>> Laurent Vivier (9):
> >>>     virtio: Use const pointer for vu_dev
> >>>     vhost-user: update protocol features and commands list
> >>>     vhost-user: add VHOST_USER_SET_LOG_FD command
> >>>     vhost-user: Pass vu_dev to more virtio functions
> >>>     vhost-user: add VHOST_USER_SET_LOG_BASE command
> >>>     vhost-user: Report to front-end we support
> >>>       VHOST_USER_PROTOCOL_F_LOG_SHMFD
> >>>     vhost-user: add VHOST_USER_CHECK_DEVICE_STATE command
> >>>     vhost-user: add VHOST_USER_SET_DEVICE_STATE_FD command
> >>>     vhost-user: Report to front-end we support
> >>>       VHOST_USER_PROTOCOL_F_DEVICE_STATE
> >>>
> >>>    epoll_type.h |   2 +
> >>>    passt.c      |   4 +
> >>>    util.h       |   3 +
> >>>    vhost_user.c | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>    vhost_user.h |  50 +++++++++-
> >>>    virtio.c     | 116 +++++++++++++++++++++---
> >>>    virtio.h     |  32 +++++--
> >>>    vu_common.c  |  59 +++++++++++-
> >>>    vu_common.h  |   3 +-
> >>>    9 files changed, 484 insertions(+), 36 deletions(-)
> >>>      
> >>
> >> Another point:
> >>
> >> vhost-user can ask backend (passt) to send a notification at the end of the migration on
> >> the destination side to the network to update the network topology. Do we need this?
> >>
> >> This is VHOST_USER_SEND_RARP:
> >> "Ask vhost user back-end to broadcast a fake RARP to notify the migration is terminated
> >> for guest that does not support GUEST_ANNOUNCE.
> >>
> >> Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
> >> VHOST_USER_GET_FEATURES and protocol feature bit VHOST_USER_PROTOCOL_F_RARP is present in
> >> VHOST_USER_GET_PROTOCOL_FEATURES. The first 6 bytes of the payload contain the mac address
> >> of the guest to allow the vhost user back-end to construct and broadcast the fake RARP."  
> > 
> > This isn't really clear :( ...what does "fake RARP" even mean? RARP is
> > a protocol, and an obsolete one, despite:
> > 
> >    https://en.wikipedia.org/wiki/Reverse_Address_Resolution_Protocol#Modern_Day_Uses
> > 
> > Anyway, it comes from 3e866365e1eb ("vhost user: add rarp sending after
> > live migration for legacy guest").  
> 
> I think legacy is for guest that doesn't support VIRTIO_NET_F_GUEST_ANNOUNCE:
> See "5.1.6.5.4 Gratuitous Packet Sending"
> https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-2560004
> 
> With VIRTIO_NET_F_GUEST_ANNOUNCE, QEMU notifies the guest kernel to send a packet to the 
> network
> to update the configuration.
>
> In the guest kernel, it seems to be done by (it sends an ARP packet):
> 
> /**
>   * __netdev_notify_peers - notify network peers about existence of @dev,
>   * to be called when rtnl lock is already held.
>   * @dev: network device
>   *
>   * Generate traffic such that interested network peers are aware of
>   * @dev, such as by generating a gratuitous ARP. This may be used when
>   * a device wants to inform the rest of the network about some sort of
>   * reconfiguration such as a failover event or virtual machine
>   * migration.
>   */
> void __netdev_notify_peers(struct net_device *dev)
> {
>          ASSERT_RTNL();
>          call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, dev);
>          call_netdevice_notifiers(NETDEV_RESEND_IGMP, dev);
> }

Sure, gratuitous ARP, makes sense.

> On its side, QEMU always send a RARP packet (see qemu_announce_self_iter()) that is 
> catched in vhost_user_receive() to be changed into a VHOST_USER_SEND_RARP by
> vhost_user_migration_done().

...but why RARP?

> > I have no idea what "legacy" means here, but I'm under the impression that
> > somebody just needed a way to notify the guest of the finished migration
> > (nothing related to networking) and, correct me if I'm wrong, the French
> > word for "fake" is rather similar to the one for "dummy".  
> 
> Not the guest, the network.
> The guest is notified with the GUEST_ANNOUNCE stuff (if supported)

Hah, I see! So it's sent *outside*. And it's dummy, as I thought, not
fake.

> > So my understanding of it is that they would just send a dummy packet,
> > and a RARP broadcast would be a good candidate because it's otherwise
> > unused, but wouldn't raise any firewall alert.
> > 
> > Or maybe "legacy" has something to do with VMware vSphere, and there was
> > some guest previously running on VMware vSphere that needs a RARP packet
> > to "proceed" with something after the migration.
> > 
> > If that's the case, and if there's still something like that around, I
> > guess that ideally we want it for compatibility, but it's *very* unlikely
> > to ever bite us.
> > 
> > So all in all I would say that if the implementation is very small and
> > unproblematic we can hack up something in arp.c to build a RARP packet
> > (example here: https://wiki.wireshark.org/RARP but that's a request, not
> > a broadcast). Otherwise I wouldn't really care until somebody asks.
> 
> It only depends on how passt will inform its neighbors about its new location in the 
> network after migration. But my feeling is passt doesn't need to do that as it shares the 
> IP address of the host and the guest MAC address is not shown beyond passt.

Right, it's just a process running on the new host. And besides, we
couldn't send it anyway, we can't craft packets like that.

> So we can implement VHOST_USER_SEND_RARP with an empty function (at least to silence the 
> "Vhost user backend fails to broadcast fake RARP" message).

Oh, sure, if it's so annoying. And we can't just keep
VHOST_USER_PROTOCOL_F_RARP off in the feature flags?

-- 
Stefano