>> >> >> >> >> Hi, all >> >> >> >> >> >> >> >> >> >> Do live migration if emulated NIC's MAC has been changed, >> >> >> >> >> RARP with wrong MAC address will broadcast via >> >> >> >> >> qemu_announce_self in destination, so, long time network >> >> >> >> >> disconnection probably happen. >> >> >> >> > >> >> >> >> >Good catch. >> >> >> >> > >> >> >> >> >> I want to do below works to resolve this problem, 1. >> >> >> >> >> change NICConf's MAC as soon as emulated NIC's MAC changed >> >> >> >> >> in guest >> >> >> >> > >> >> >> >> >This will make it impossible to revert it correctly on reset, >> >> >> >> >won't it? >> >> >> >> > >> >> >> >> You are right. >> >> >> >> virsh reboot <domain>, or virsh reset <domain>, or reboot VM >> >> >> >> from guest, will revert emulated NIC's MAC to original one >> >> >> >> maintained in NICConf. >> >> >> >> During the reboot/reset flow in qemu, emulated NIC's reset >> >> >> >> handler will sync the MAC address in NICConf to the MAC >> >> >> >> address in emulated NIC structure, e.g., virtio_net_reset >> >> >> >> sync the MAC address in NICConf to VirtIONet'mac. >> >> >> >> >> >> >> >> BTW, in native scenario, reboot will revert the changed MAC >> >> >> >> to original one, too. >> >> >> >> >> >> >> >> >> 2. sync NIC's (more precisely, queue) MAC to corresponding >> >> >> >> >> NICConf in NIC's migration load handler >> >> >> >> >> >> >> >> >> >> Any better ideas? >> >> >> >> >> >> >> >> >> >> Thanks, >> >> >> >> >> Zhang Haoyu >> >> >> >> > >> >> >> >> >I think announce needs to poke at the current MAC instead of >> >> >> >> >the default one in NICConf. >> >> >> >> >We can make it respect link down state while we are at it. >> >> >> >> > >> >> >> >> NICConf structures are incorporated in different emulated >> >> >> >> NIC's structure, e.g., VirtIONet, E1000State_st, >> >> >> >> RTL8139State, etc., since so many kinds of emulated NICs, >> >> >> >> they are described by different structures, how to find all NICs' >> >> >> >> current MAC? >> >> >> >> >> >> >> >> Maybe we can introduce a pointer member 'current_mac' to >> >> >> >> NICConf structure, which points to the current MAC, then we >> >> >> >> can find all current MACs from NICConf.current_mac. >> >> >> > >> >> >> >I wouldn't make it a pointer, just a buffer with the mac, copy it >> >> >> >there. >> >> >> >Maybe call it "softmac" that's what it is really. >> >> >> > >> >> >> >> Can we broadcast the RARP with current MAC in NIC's migration >> >> >> >> load handler respectively? >> >> >> >> >> >> >> >> Thanks, >> >> >> >> Zhang Haoyu >> >> >> > >> >> >> >It's not so simple, you need to retry several times. >> >> >> > >> >> >> Could you make a statement for 'retry several times' ? >> >> >> Is it the process of retrying several times to sending RARP in >> >> >> qemu_announce_self_once? >> >> > >> >> >yes >> >> > >> >> >> 'broadcast the RARP with current MAC in NIC's migration load >> >> >> handler respectively' is distributing the job of what >> >> >> qemu_announce_self does to every NIC's migration load handler, >> >> >> e.g., in virtio NIC's migration load handler virtio_net_load, we >> >> >> can create a timer to retry several times to send ARAP with >> >> >> current MAC for this NIC, just as same as qemu_announce_self does. >> >> > >> >> >I don't see a lot of value in this yet. >> >> > >> >> In my opinion, it's not so good to introduce a 'softmac' member to >> >> NICConf, which is not essential function of NICConf. >> > >> > Maybe not essential but 100% of hardware we emulate supports softmacs. >> >> Yes, but NICConf is about NIC *configuration*, not random common NIC >> state. >> >> We can capture common NIC state in a separate, properly named data type. >> >> If we want to bunch it together with common configuration in NICConf >> instead, then better rename NICConf to something that actually >> reflects its changed purpose. I doubt this would be a good idea. > >I agree, it should go into NetClientState, not NICConf. >My main point is it's a common thing, let's not duplicate code. > Yes, put it into NetClientState is better. But, need to add updating code for NetClientState.softmac to all devices, right?
>> >> And, distributing the job of what qemu_announce_self does to every >> >> NIC's migration load handler has no disadvantages over >> >> qemu_announce_self, >> > >> > I see some disadvantages, yes. >> > You are going to add code to all devices instead of doing it in one >> > place, there better be a good reason for this. Comparing with qemu_announce_self, there is indeed no advantages, on the contrary, has disadvantages, just as what you said. but comparing with introducing 'softmac' or something into NIC-related structures, it does not need to add any data to NIC-related structures, and introducing 'softmac' also need to add updating code for 'softmac' to all devices, right? And, I don't think it's a good idea to store the identical data in two buffers, its consistency should be guaranteed. Thanks, Zhang Haoyu >> >> Keeping code common to many (most?) NICs factored out makes sense. >> We've started doing that for block devices, in hw/block/block.c. So >> far, the only code there is about configuration, thus we work with >> BlockConf. >> >> [...]