Re: [Qemu-devel] [RFC] sync NIC's MAC maintained in NICConf as soon as emualted NIC's MAC changed in guest

[Michael S. Tsirkin]

On Wed, Sep 25, 2013 at 10:14:49AM +0000, Zhanghaoyu (A) wrote:
> >> >> >> 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.

> 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.

> maybe more immediately to updating the forwarding table of switches/bridges.

I don't see how this will update tables faster.

> >> >> >Happily recent linux guests aren't affected since they do 
> >> >> >announcements from guest.
> >> >> >
> >> >> >--
> >> >> >MST