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

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

[Zhanghaoyu (A)]

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.

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
2. sync NIC's (more precisely, queue) MAC to corresponding NICConf in NIC's migration load handler

Any better ideas?

Thanks,
Zhang Haoyu

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 Sun, Sep 22, 2013 at 08:35:29AM +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?

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

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

-- 
MST

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

[Zhanghaoyu (A)]

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

Can we broadcast the RARP with current MAC in NIC's migration load handler respectively?

Thanks,
Zhang Haoyu

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

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 09:02:54AM +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.

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

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

[Zhanghaoyu (A)]

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

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

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

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 09:55:02AM +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.

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

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

[Zhanghaoyu (A)]

>> >> >> 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.
And, distributing the job of what qemu_announce_self does to every NIC's migration load handler has no disadvantages over qemu_announce_self,
maybe more immediately to updating the forwarding table of switches/bridges.

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

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

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

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

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.

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

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.

[...]

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 01:39:48PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > 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.
> 
> 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.

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

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

[Zhanghaoyu (A)]

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

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

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Sep 25, 2013 at 01:39:48PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > 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.
>> 
>> 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.

NICState?

> My main point is it's a common thing, let's not duplicate code.

No argument.

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

[Jason Wang]

On 09/22/2013 04:35 PM, 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.
>
> 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
> 2. sync NIC's (more precisely, queue) MAC to corresponding NICConf in NIC's migration load handler
>
> Any better ideas?

As Michael points out. The only possible solution is to use do it inside
the guest instead of qemu ( and using a pv device). You can have a look
at my RFCs in
http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg01127.html
which let virtio driver send the gARP. Xen, Hyperv does the same thing.

The point is qemu does not know how the macs was used. So your method
only solves the issue partially becuase:

- A card can have several macs, see virtio_net and e1000's mac table and
it can be overflowed also.
- Vlan could be used so we need to send tagged gARP instead of untagged.
>
> Thanks,
> Zhang Haoyu
>

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

[Zhanghaoyu (A)]

>> 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.
>>
>> 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 2. sync NIC's (more 
>> precisely, queue) MAC to corresponding NICConf in NIC's migration load 
>> handler
>>
>> Any better ideas?
>
>As Michael points out. The only possible solution is to use do it inside the guest instead of qemu ( and using a pv device). You can have a look at my RFCs in http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg01127.html
>which let virtio driver send the gARP. Xen, Hyperv does the same thing.
>
How about other emulated NICs, like etl8139, etc. ?

>The point is qemu does not know how the macs was used. So your method only solves the issue partially becuase:
>
>- A card can have several macs, see virtio_net and e1000's mac table and it can be overflowed also.
>- Vlan could be used so we need to send tagged gARP instead of untagged.

Does the emulated NIC in qemu have knowledge of all of its MACs?
We can provide an interface nic_announce_self(NetClientState *nc, uint8_t *mac_addr) which will try several times to send RARP just as same as 
what qemu_announce_self does, all emulated NICs' migration load handler can call nic_announce_self to announce itself for its all MACs.


>>
>> Thanks,
>> Zhang Haoyu

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

[Jason Wang]

On 09/26/2013 11:42 AM, 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.
>>>
>>> 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 2. sync NIC's (more 
>>> precisely, queue) MAC to corresponding NICConf in NIC's migration load 
>>> handler
>>>
>>> Any better ideas?
>> As Michael points out. The only possible solution is to use do it inside the guest instead of qemu ( and using a pv device). You can have a look at my RFCs in http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg01127.html
>> which let virtio driver send the gARP. Xen, Hyperv does the same thing.
>>
> How about other emulated NICs, like etl8139, etc. ?

A possible solution is to toggle the link status after migration and
make use of arp_notify. But it may make the migration aware of guest.
Xen guys has posted this idea but get rejected upstream.
>
>> The point is qemu does not know how the macs was used. So your method only solves the issue partially becuase:
>>
>> - A card can have several macs, see virtio_net and e1000's mac table and it can be overflowed also.
>> - Vlan could be used so we need to send tagged gARP instead of untagged.
> Does the emulated NIC in qemu have knowledge of all of its MACs?

Looks not. For e1000, if the number of macs tracked by guest is greater
than the number of RAR, it will use unicast promiscuous mode. For
virtio-net, the mac table can be overflowd, and then it will allow all
unicast mac address to be received.

So emulated NIC may not have knowledge of all its MACs, and more
important: how it could be used.
> We can provide an interface nic_announce_self(NetClientState *nc, uint8_t *mac_addr) which will try several times to send RARP just as same as 
> what qemu_announce_self does, all emulated NICs' migration load handler can call nic_announce_self to announce itself for its all MACs.

See http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg01130.html.
>
>>> Thanks,
>>> Zhang Haoyu