qemu_announce_self() is triggered by qemu at the end of migrations
to update the network regarding the path to the guest l2addr.

however it is also useful when there is a network change such as
an active bond slave swap. Essentially, it's the same as a migration
from a network perspective - the guest moves to a different point
in the network topology.

this exposes the function via qmp.

Signed-off-by: Germano Veit Michel <germano@redhat.com>
---
 include/migration/vmstate.h |  5 +++++
 migration/savevm.c          | 30 +++++++++++++++++++-----------
 qapi-schema.json            | 18 ++++++++++++++++++
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 63e7b02..a08715c 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
     return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
 }

+struct AnnounceRound {
+    QEMUTimer *timer;
+    int count;
+};
+
 void dump_vmstate_json_to_file(FILE *out_fp);

 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 5ecd264..44e196b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
*nic, void *opaque)
     qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
 }

-
 static void qemu_announce_self_once(void *opaque)
 {
-    static int count = SELF_ANNOUNCE_ROUNDS;
-    QEMUTimer *timer = *(QEMUTimer **)opaque;
+    struct AnnounceRound *round = opaque;

     qemu_foreach_nic(qemu_announce_self_iter, NULL);

-    if (--count) {
+    round->count--;
+    if (round->count) {
         /* delay 50ms, 150ms, 250ms, ... */
-        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
-                  self_announce_delay(count));
+        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
+                  self_announce_delay(round->count));
     } else {
-            timer_del(timer);
-            timer_free(timer);
+            timer_del(round->timer);
+            timer_free(round->timer);
+            g_free(round);
     }
 }

 void qemu_announce_self(void)
 {
-    static QEMUTimer *timer;
-    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
-    qemu_announce_self_once(&timer);
+    struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));
+    if (!round)
+        return;
+    round->count = SELF_ANNOUNCE_ROUNDS;
+    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
qemu_announce_self_once, round);
+    qemu_announce_self_once(round);
+}
+
+void qmp_announce_self(Error **errp)
+{
+    qemu_announce_self();
 }

 /***********************************************************/
diff --git a/qapi-schema.json b/qapi-schema.json
index baa0d26..0d9bffd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -6080,3 +6080,21 @@
 #
 ##
 { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+
+##
+# @announce-self:
+#
+# Trigger generation of broadcast RARP frames to update network switches.
+# This can be useful when network bonds fail-over the active slave.
+#
+# Arguments: None.
+#
+# Example:
+#
+# -> { "execute": "announce-self" }
+# <- { "return": {} }
+#
+# Since: 2.9
+##
+{ 'command': 'announce-self' }
+
-- 
2.9.3

* Germano Veit Michel (germano@redhat.com) wrote:
> qemu_announce_self() is triggered by qemu at the end of migrations
> to update the network regarding the path to the guest l2addr.
> 
> however it is also useful when there is a network change such as
> an active bond slave swap. Essentially, it's the same as a migration
> from a network perspective - the guest moves to a different point
> in the network topology.
> 
> this exposes the function via qmp.

Markus: Since you're asking for tests for qmp commands; how would you
test this?

Jason: Does this look OK from the networking side of things?

> Signed-off-by: Germano Veit Michel <germano@redhat.com>
> ---
>  include/migration/vmstate.h |  5 +++++
>  migration/savevm.c          | 30 +++++++++++++++++++-----------
>  qapi-schema.json            | 18 ++++++++++++++++++
>  3 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 63e7b02..a08715c 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
>      return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
>  }
> 
> +struct AnnounceRound {
> +    QEMUTimer *timer;
> +    int count;
> +};
> +
>  void dump_vmstate_json_to_file(FILE *out_fp);
> 
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5ecd264..44e196b 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
> *nic, void *opaque)
>      qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
>  }
> 
> -
>  static void qemu_announce_self_once(void *opaque)
>  {
> -    static int count = SELF_ANNOUNCE_ROUNDS;
> -    QEMUTimer *timer = *(QEMUTimer **)opaque;
> +    struct AnnounceRound *round = opaque;
> 
>      qemu_foreach_nic(qemu_announce_self_iter, NULL);
> 
> -    if (--count) {
> +    round->count--;
> +    if (round->count) {
>          /* delay 50ms, 150ms, 250ms, ... */
> -        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> -                  self_announce_delay(count));
> +        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> +                  self_announce_delay(round->count));
>      } else {
> -            timer_del(timer);
> -            timer_free(timer);
> +            timer_del(round->timer);
> +            timer_free(round->timer);
> +            g_free(round);
>      }
>  }
> 
>  void qemu_announce_self(void)
>  {
> -    static QEMUTimer *timer;
> -    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
> -    qemu_announce_self_once(&timer);
> +    struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));

I prefer g_new0 - i.e.
   struct AnnounceRound *round = g_new0(struct AnnounceRound, 1)

> +    if (!round)
> +        return;
> +    round->count = SELF_ANNOUNCE_ROUNDS;
> +    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> qemu_announce_self_once, round);

An odd line break?

> +    qemu_announce_self_once(round);
> +}
> +
> +void qmp_announce_self(Error **errp)
> +{
> +    qemu_announce_self();
>  }
> 
>  /***********************************************************/
> diff --git a/qapi-schema.json b/qapi-schema.json
> index baa0d26..0d9bffd 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6080,3 +6080,21 @@
>  #
>  ##
>  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @announce-self:
> +#
> +# Trigger generation of broadcast RARP frames to update network switches.
> +# This can be useful when network bonds fail-over the active slave.
> +#
> +# Arguments: None.
> +#
> +# Example:
> +#
> +# -> { "execute": "announce-self" }
> +# <- { "return": {} }
> +#
> +# Since: 2.9
> +##
> +{ 'command': 'announce-self' }
> +

Please wire hmp up as well (see hmp-commands.hx).

Dave

> -- 
> 2.9.3
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On 2017年03月03日 18:39, Dr. David Alan Gilbert wrote:
> * Germano Veit Michel (germano@redhat.com) wrote:
>> qemu_announce_self() is triggered by qemu at the end of migrations
>> to update the network regarding the path to the guest l2addr.
>>
>> however it is also useful when there is a network change such as
>> an active bond slave swap. Essentially, it's the same as a migration
>> from a network perspective - the guest moves to a different point
>> in the network topology.
>>
>> this exposes the function via qmp.
> Markus: Since you're asking for tests for qmp commands; how would you
> test this?
>
> Jason: Does this look OK from the networking side of things?
>

Good as a start I think. We probably want to add callbacks for each 
kinds of nic. This will be useful for virtio, since some guest can 
announce themselves with complex configurations (e.g vlans).

Thanks

On 02/20/2017 07:16 PM, Germano Veit Michel wrote:
> qemu_announce_self() is triggered by qemu at the end of migrations
> to update the network regarding the path to the guest l2addr.
> 
> however it is also useful when there is a network change such as
> an active bond slave swap. Essentially, it's the same as a migration
> from a network perspective - the guest moves to a different point
> in the network topology.
> 
> this exposes the function via qmp.
> 
> Signed-off-by: Germano Veit Michel <germano@redhat.com>
> ---
>  include/migration/vmstate.h |  5 +++++
>  migration/savevm.c          | 30 +++++++++++++++++++-----------
>  qapi-schema.json            | 18 ++++++++++++++++++
>  3 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 63e7b02..a08715c 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
>      return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
>  }
> 
> +struct AnnounceRound {
> +    QEMUTimer *timer;
> +    int count;
> +};
> +
>  void dump_vmstate_json_to_file(FILE *out_fp);
> 
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5ecd264..44e196b 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
> *nic, void *opaque)
>      qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
>  }
> 
> -
>  static void qemu_announce_self_once(void *opaque)
>  {
> -    static int count = SELF_ANNOUNCE_ROUNDS;
> -    QEMUTimer *timer = *(QEMUTimer **)opaque;
> +    struct AnnounceRound *round = opaque;
> 
>      qemu_foreach_nic(qemu_announce_self_iter, NULL);
> 
> -    if (--count) {
> +    round->count--;
> +    if (round->count) {
>          /* delay 50ms, 150ms, 250ms, ... */
> -        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> -                  self_announce_delay(count));
> +        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> +                  self_announce_delay(round->count));
>      } else {
> -            timer_del(timer);
> -            timer_free(timer);
> +            timer_del(round->timer);
> +            timer_free(round->timer);
> +            g_free(round);
>      }
>  }
> 
>  void qemu_announce_self(void)
>  {
> -    static QEMUTimer *timer;
> -    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
> -    qemu_announce_self_once(&timer);
> +    struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));
> +    if (!round)
> +        return;
> +    round->count = SELF_ANNOUNCE_ROUNDS;
> +    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> qemu_announce_self_once, round);
> +    qemu_announce_self_once(round);
> +}

So, I've been looking and this code and have been playing with it and with David's
patches and my patches to include virtio self announcements as well.  What I've discovered
is what I think is a possible packet amplification issue here.

This creates a new timer every time we do do a announce_self.  With just migration,
this is not an issue since you only migrate once at a time, so there is only 1 timer.
With exposing this as an API, a user can potentially call it in a tight loop
and now you have a ton of timers being created.  Add in David's patches allowing timeouts
and retries to be configurable, and you may now have a ton of long lived timers.
Add in the patches I am working on to let virtio do self announcements too (to really fix
bonding issues), and now you add in a possibility of a lot of packets being sent for
each timeout (RARP, GARP, NA, IGMPv4 Reports, IGMPv6 Reports [even worse if MLD1 is used]).

As you can see, this can get rather ugly...

I think we need timer user here.  Migration and QMP being two to begin with.  Each
one would get a single timer to play with.  If a given user already has a timer running,
we could return an error or just not do anything.

-vlad

> +
> +void qmp_announce_self(Error **errp)
> +{
> +    qemu_announce_self();
>  }
> 
>  /***********************************************************/
> diff --git a/qapi-schema.json b/qapi-schema.json
> index baa0d26..0d9bffd 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6080,3 +6080,21 @@
>  #
>  ##
>  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @announce-self:
> +#
> +# Trigger generation of broadcast RARP frames to update network switches.
> +# This can be useful when network bonds fail-over the active slave.
> +#
> +# Arguments: None.
> +#
> +# Example:
> +#
> +# -> { "execute": "announce-self" }
> +# <- { "return": {} }
> +#
> +# Since: 2.9
> +##
> +{ 'command': 'announce-self' }
> +
> 

* Vlad Yasevich (vyasevic@redhat.com) wrote:
> On 02/20/2017 07:16 PM, Germano Veit Michel wrote:
> > qemu_announce_self() is triggered by qemu at the end of migrations
> > to update the network regarding the path to the guest l2addr.
> > 
> > however it is also useful when there is a network change such as
> > an active bond slave swap. Essentially, it's the same as a migration
> > from a network perspective - the guest moves to a different point
> > in the network topology.
> > 
> > this exposes the function via qmp.
> > 
> > Signed-off-by: Germano Veit Michel <germano@redhat.com>
> > ---
> >  include/migration/vmstate.h |  5 +++++
> >  migration/savevm.c          | 30 +++++++++++++++++++-----------
> >  qapi-schema.json            | 18 ++++++++++++++++++
> >  3 files changed, 42 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 63e7b02..a08715c 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
> >      return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
> >  }
> > 
> > +struct AnnounceRound {
> > +    QEMUTimer *timer;
> > +    int count;
> > +};
> > +
> >  void dump_vmstate_json_to_file(FILE *out_fp);
> > 
> >  #endif
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 5ecd264..44e196b 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
> > *nic, void *opaque)
> >      qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
> >  }
> > 
> > -
> >  static void qemu_announce_self_once(void *opaque)
> >  {
> > -    static int count = SELF_ANNOUNCE_ROUNDS;
> > -    QEMUTimer *timer = *(QEMUTimer **)opaque;
> > +    struct AnnounceRound *round = opaque;
> > 
> >      qemu_foreach_nic(qemu_announce_self_iter, NULL);
> > 
> > -    if (--count) {
> > +    round->count--;
> > +    if (round->count) {
> >          /* delay 50ms, 150ms, 250ms, ... */
> > -        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> > -                  self_announce_delay(count));
> > +        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> > +                  self_announce_delay(round->count));
> >      } else {
> > -            timer_del(timer);
> > -            timer_free(timer);
> > +            timer_del(round->timer);
> > +            timer_free(round->timer);
> > +            g_free(round);
> >      }
> >  }
> > 
> >  void qemu_announce_self(void)
> >  {
> > -    static QEMUTimer *timer;
> > -    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
> > -    qemu_announce_self_once(&timer);
> > +    struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));
> > +    if (!round)
> > +        return;
> > +    round->count = SELF_ANNOUNCE_ROUNDS;
> > +    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> > qemu_announce_self_once, round);
> > +    qemu_announce_self_once(round);
> > +}
> 
> So, I've been looking and this code and have been playing with it and with David's
> patches and my patches to include virtio self announcements as well.  What I've discovered
> is what I think is a possible packet amplification issue here.
> 
> This creates a new timer every time we do do a announce_self.  With just migration,
> this is not an issue since you only migrate once at a time, so there is only 1 timer.
> With exposing this as an API, a user can potentially call it in a tight loop
> and now you have a ton of timers being created.  Add in David's patches allowing timeouts
> and retries to be configurable, and you may now have a ton of long lived timers.
> Add in the patches I am working on to let virtio do self announcements too (to really fix
> bonding issues), and now you add in a possibility of a lot of packets being sent for
> each timeout (RARP, GARP, NA, IGMPv4 Reports, IGMPv6 Reports [even worse if MLD1 is used]).
> 
> As you can see, this can get rather ugly...
> 
> I think we need timer user here.  Migration and QMP being two to begin with.  Each
> one would get a single timer to play with.  If a given user already has a timer running,
> we could return an error or just not do anything.

If you did have specific timers, then you could add to/reset the counts
rather than doing nothing.  That way it's less racy; if you issue the
command just as you reconfigure your network, there's no chance the
command would fail, you will send the packets out.

Dave

> -vlad
> 
> > +
> > +void qmp_announce_self(Error **errp)
> > +{
> > +    qemu_announce_self();
> >  }
> > 
> >  /***********************************************************/
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index baa0d26..0d9bffd 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -6080,3 +6080,21 @@
> >  #
> >  ##
> >  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > +
> > +##
> > +# @announce-self:
> > +#
> > +# Trigger generation of broadcast RARP frames to update network switches.
> > +# This can be useful when network bonds fail-over the active slave.
> > +#
> > +# Arguments: None.
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "announce-self" }
> > +# <- { "return": {} }
> > +#
> > +# Since: 2.9
> > +##
> > +{ 'command': 'announce-self' }
> > +
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On 05/12/2017 03:24 PM, Dr. David Alan Gilbert wrote:
> * Vlad Yasevich (vyasevic@redhat.com) wrote:
>> On 02/20/2017 07:16 PM, Germano Veit Michel wrote:
>>> qemu_announce_self() is triggered by qemu at the end of migrations
>>> to update the network regarding the path to the guest l2addr.
>>>
>>> however it is also useful when there is a network change such as
>>> an active bond slave swap. Essentially, it's the same as a migration
>>> from a network perspective - the guest moves to a different point
>>> in the network topology.
>>>
>>> this exposes the function via qmp.
>>>
>>> Signed-off-by: Germano Veit Michel <germano@redhat.com>
>>> ---
>>>  include/migration/vmstate.h |  5 +++++
>>>  migration/savevm.c          | 30 +++++++++++++++++++-----------
>>>  qapi-schema.json            | 18 ++++++++++++++++++
>>>  3 files changed, 42 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>>> index 63e7b02..a08715c 100644
>>> --- a/include/migration/vmstate.h
>>> +++ b/include/migration/vmstate.h
>>> @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
>>>      return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
>>>  }
>>>
>>> +struct AnnounceRound {
>>> +    QEMUTimer *timer;
>>> +    int count;
>>> +};
>>> +
>>>  void dump_vmstate_json_to_file(FILE *out_fp);
>>>
>>>  #endif
>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>> index 5ecd264..44e196b 100644
>>> --- a/migration/savevm.c
>>> +++ b/migration/savevm.c
>>> @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
>>> *nic, void *opaque)
>>>      qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
>>>  }
>>>
>>> -
>>>  static void qemu_announce_self_once(void *opaque)
>>>  {
>>> -    static int count = SELF_ANNOUNCE_ROUNDS;
>>> -    QEMUTimer *timer = *(QEMUTimer **)opaque;
>>> +    struct AnnounceRound *round = opaque;
>>>
>>>      qemu_foreach_nic(qemu_announce_self_iter, NULL);
>>>
>>> -    if (--count) {
>>> +    round->count--;
>>> +    if (round->count) {
>>>          /* delay 50ms, 150ms, 250ms, ... */
>>> -        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
>>> -                  self_announce_delay(count));
>>> +        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
>>> +                  self_announce_delay(round->count));
>>>      } else {
>>> -            timer_del(timer);
>>> -            timer_free(timer);
>>> +            timer_del(round->timer);
>>> +            timer_free(round->timer);
>>> +            g_free(round);
>>>      }
>>>  }
>>>
>>>  void qemu_announce_self(void)
>>>  {
>>> -    static QEMUTimer *timer;
>>> -    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
>>> -    qemu_announce_self_once(&timer);
>>> +    struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));
>>> +    if (!round)
>>> +        return;
>>> +    round->count = SELF_ANNOUNCE_ROUNDS;
>>> +    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
>>> qemu_announce_self_once, round);
>>> +    qemu_announce_self_once(round);
>>> +}
>>
>> So, I've been looking and this code and have been playing with it and with David's
>> patches and my patches to include virtio self announcements as well.  What I've discovered
>> is what I think is a possible packet amplification issue here.
>>
>> This creates a new timer every time we do do a announce_self.  With just migration,
>> this is not an issue since you only migrate once at a time, so there is only 1 timer.
>> With exposing this as an API, a user can potentially call it in a tight loop
>> and now you have a ton of timers being created.  Add in David's patches allowing timeouts
>> and retries to be configurable, and you may now have a ton of long lived timers.
>> Add in the patches I am working on to let virtio do self announcements too (to really fix
>> bonding issues), and now you add in a possibility of a lot of packets being sent for
>> each timeout (RARP, GARP, NA, IGMPv4 Reports, IGMPv6 Reports [even worse if MLD1 is used]).
>>
>> As you can see, this can get rather ugly...
>>
>> I think we need timer user here.  Migration and QMP being two to begin with.  Each
>> one would get a single timer to play with.  If a given user already has a timer running,
>> we could return an error or just not do anything.
> 
> If you did have specific timers, then you could add to/reset the counts
> rather than doing nothing.  That way it's less racy; if you issue the
> command just as you reconfigure your network, there's no chance the
> command would fail, you will send the packets out.

Yes.  That's another possible way to handle this.

-vlad
> 
> Dave
> 
>> -vlad
>>
>>> +
>>> +void qmp_announce_self(Error **errp)
>>> +{
>>> +    qemu_announce_self();
>>>  }
>>>
>>>  /***********************************************************/
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index baa0d26..0d9bffd 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -6080,3 +6080,21 @@
>>>  #
>>>  ##
>>>  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
>>> +
>>> +##
>>> +# @announce-self:
>>> +#
>>> +# Trigger generation of broadcast RARP frames to update network switches.
>>> +# This can be useful when network bonds fail-over the active slave.
>>> +#
>>> +# Arguments: None.
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "announce-self" }
>>> +# <- { "return": {} }
>>> +#
>>> +# Since: 2.9
>>> +##
>>> +{ 'command': 'announce-self' }
>>> +
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

Ohh, sorry. I don't know how or why, but I missed all your replies (this
was archived in gmail)

Below is qmp-net-test.c, It's just a copy and paste of what Markus
suggested. I could try doing it with a socket (virtio-net-test.c) as Jason
suggested but I'm afraid I won't have time this week as support is quite
busy.

Thanks Vlad for actively working on this.

/*
>  * Test cases for network-related QMP commands
>  *
>  * Copyright (c) 2017 Red Hat Inc.
>  *
>  * Authors:
>  *  Markus Armbruster <armbru@redhat.com>,
>  *
>  * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
>  * See the COPYING file in the top-level directory.
>  */
>
> #include "qemu/osdep.h"
> #include "libqtest.h"
> #include "qapi/error.h"
>
> const char common_args[] = "-nodefaults -machine none";
>
> static void test_qmp_announce_self(void)
> {
>     QDict *resp, *ret;
>
>     qtest_start(common_args);
>
>     resp = qmp("{ 'execute': 'announce-self' }");
>     ret = qdict_get_qdict(resp, "return");
>     g_assert(ret && !qdict_size(ret));
>     QDECREF(resp);
>
>     qtest_end();
> }
>
> int main(int argc, char *argv[])
> {
>     g_test_init(&argc, &argv, NULL);
>
>     qtest_add_func("qmp/net/announce_self", test_qmp_announce_self);
>
>     return g_test_run();
> }
>


On Sat, May 13, 2017 at 7:16 AM, Vlad Yasevich <vyasevic@redhat.com> wrote:

> On 05/12/2017 03:24 PM, Dr. David Alan Gilbert wrote:
> > * Vlad Yasevich (vyasevic@redhat.com) wrote:
> >> On 02/20/2017 07:16 PM, Germano Veit Michel wrote:
> >>> qemu_announce_self() is triggered by qemu at the end of migrations
> >>> to update the network regarding the path to the guest l2addr.
> >>>
> >>> however it is also useful when there is a network change such as
> >>> an active bond slave swap. Essentially, it's the same as a migration
> >>> from a network perspective - the guest moves to a different point
> >>> in the network topology.
> >>>
> >>> this exposes the function via qmp.
> >>>
> >>> Signed-off-by: Germano Veit Michel <germano@redhat.com>
> >>> ---
> >>>  include/migration/vmstate.h |  5 +++++
> >>>  migration/savevm.c          | 30 +++++++++++++++++++-----------
> >>>  qapi-schema.json            | 18 ++++++++++++++++++
> >>>  3 files changed, 42 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> >>> index 63e7b02..a08715c 100644
> >>> --- a/include/migration/vmstate.h
> >>> +++ b/include/migration/vmstate.h
> >>> @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
> >>>      return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
> >>>  }
> >>>
> >>> +struct AnnounceRound {
> >>> +    QEMUTimer *timer;
> >>> +    int count;
> >>> +};
> >>> +
> >>>  void dump_vmstate_json_to_file(FILE *out_fp);
> >>>
> >>>  #endif
> >>> diff --git a/migration/savevm.c b/migration/savevm.c
> >>> index 5ecd264..44e196b 100644
> >>> --- a/migration/savevm.c
> >>> +++ b/migration/savevm.c
> >>> @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
> >>> *nic, void *opaque)
> >>>      qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
> >>>  }
> >>>
> >>> -
> >>>  static void qemu_announce_self_once(void *opaque)
> >>>  {
> >>> -    static int count = SELF_ANNOUNCE_ROUNDS;
> >>> -    QEMUTimer *timer = *(QEMUTimer **)opaque;
> >>> +    struct AnnounceRound *round = opaque;
> >>>
> >>>      qemu_foreach_nic(qemu_announce_self_iter, NULL);
> >>>
> >>> -    if (--count) {
> >>> +    round->count--;
> >>> +    if (round->count) {
> >>>          /* delay 50ms, 150ms, 250ms, ... */
> >>> -        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> >>> -                  self_announce_delay(count));
> >>> +        timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
> +
> >>> +                  self_announce_delay(round->count));
> >>>      } else {
> >>> -            timer_del(timer);
> >>> -            timer_free(timer);
> >>> +            timer_del(round->timer);
> >>> +            timer_free(round->timer);
> >>> +            g_free(round);
> >>>      }
> >>>  }
> >>>
> >>>  void qemu_announce_self(void)
> >>>  {
> >>> -    static QEMUTimer *timer;
> >>> -    timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> qemu_announce_self_once, &timer);
> >>> -    qemu_announce_self_once(&timer);
> >>> +    struct AnnounceRound *round = g_malloc(sizeof(struct
> AnnounceRound));
> >>> +    if (!round)
> >>> +        return;
> >>> +    round->count = SELF_ANNOUNCE_ROUNDS;
> >>> +    round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> >>> qemu_announce_self_once, round);
> >>> +    qemu_announce_self_once(round);
> >>> +}
> >>
> >> So, I've been looking and this code and have been playing with it and
> with David's
> >> patches and my patches to include virtio self announcements as well.
> What I've discovered
> >> is what I think is a possible packet amplification issue here.
> >>
> >> This creates a new timer every time we do do a announce_self.  With
> just migration,
> >> this is not an issue since you only migrate once at a time, so there is
> only 1 timer.
> >> With exposing this as an API, a user can potentially call it in a tight
> loop
> >> and now you have a ton of timers being created.  Add in David's patches
> allowing timeouts
> >> and retries to be configurable, and you may now have a ton of long
> lived timers.
> >> Add in the patches I am working on to let virtio do self announcements
> too (to really fix
> >> bonding issues), and now you add in a possibility of a lot of packets
> being sent for
> >> each timeout (RARP, GARP, NA, IGMPv4 Reports, IGMPv6 Reports [even
> worse if MLD1 is used]).
> >>
> >> As you can see, this can get rather ugly...
> >>
> >> I think we need timer user here.  Migration and QMP being two to begin
> with.  Each
> >> one would get a single timer to play with.  If a given user already has
> a timer running,
> >> we could return an error or just not do anything.
> >
> > If you did have specific timers, then you could add to/reset the counts
> > rather than doing nothing.  That way it's less racy; if you issue the
> > command just as you reconfigure your network, there's no chance the
> > command would fail, you will send the packets out.
>
> Yes.  That's another possible way to handle this.
>
> -vlad
> >
> > Dave
> >
> >> -vlad
> >>
> >>> +
> >>> +void qmp_announce_self(Error **errp)
> >>> +{
> >>> +    qemu_announce_self();
> >>>  }
> >>>
> >>>  /***********************************************************/
> >>> diff --git a/qapi-schema.json b/qapi-schema.json
> >>> index baa0d26..0d9bffd 100644
> >>> --- a/qapi-schema.json
> >>> +++ b/qapi-schema.json
> >>> @@ -6080,3 +6080,21 @@
> >>>  #
> >>>  ##
> >>>  { 'command': 'query-hotpluggable-cpus', 'returns':
> ['HotpluggableCPU'] }
> >>> +
> >>> +##
> >>> +# @announce-self:
> >>> +#
> >>> +# Trigger generation of broadcast RARP frames to update network
> switches.
> >>> +# This can be useful when network bonds fail-over the active slave.
> >>> +#
> >>> +# Arguments: None.
> >>> +#
> >>> +# Example:
> >>> +#
> >>> +# -> { "execute": "announce-self" }
> >>> +# <- { "return": {} }
> >>> +#
> >>> +# Since: 2.9
> >>> +##
> >>> +{ 'command': 'announce-self' }
> >>> +
> >>>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
>


-- 
Germano Veit Michel <germano@redhat.com>
Senior Software Maintenance Engineer, Virtualization, Red Hat