Re: [Qemu-devel] [PATCH v2 1/2] net: introduce MAC_TABLE_CHANGED event

[Amos Kong <akong@redhat.com>]

On Thu, May 16, 2013 at 03:17:45PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 16, 2013 at 07:07:24PM +0800, Amos Kong wrote:
> > Introduce this new QMP event to notify management after guest changes
> > mac-table configuration.


> > +    event_data = qobject_from_jsonf("{ 'name': %s }", n->netclient_name);
> > +    monitor_protocol_event(QEVENT_MAC_TABLE_CHANGED, event_data);
> > +    qobject_decref(event_data);
> > +
> >      return VIRTIO_NET_OK;
> >  }
> >  
> 
> Sorry, pls ignore my previous mail, I see you actually
> emit this on rx mode change as well.
> 
> I find the name misleading or at least it mislead me :)
> RX_FILTER_CHANGED?

Agree.

What we query contain some of rx modes & mac-table content,
rx_filter is better.

I will also change monitor cmd to 'query-rx-filter'.
 
> > @@ -425,6 +432,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> >  {
> >      struct virtio_net_ctrl_mac mac_data;
> >      size_t s;
> > +    QObject *event_data;
> >  
> >      if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
> >          if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
> > @@ -497,6 +505,10 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> >          n->mac_table.multi_overflow = 1;
> >      }
> >  
> > +    event_data = qobject_from_jsonf("{ 'name': %s }", n->netclient_name);
> > +    monitor_protocol_event(QEVENT_MAC_TABLE_CHANGED, event_data);
> > +    qobject_decref(event_data);
> > +
> >      return VIRTIO_NET_OK;
> >  }
> > 
> 
> This makes it easy for guest to flood management with
> spurious events.

> How about we set a flag after this, and avoid sending any more
> events until management queries the filter status?

As you discussed in this thread, we need a flag to turn on/off
the event notification to avoid the flooding.

But we could not set the flag in first mac-table change to turn off
the notification. Becase one action(execute one cmd in guest) might
cause multiple events.

It would be flexible to add a parameter for query-mac-table to change
the flag. Or add a new command to change the flag.
 

-- 
			Amos.