On Tue, May 21, 2013 at 01:04:55PM +0800, Amos Kong wrote:
> 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.
I still think it will work, since the event does not have any
information, what does it matter that we send one and not many 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.
Looks a bit too complex, to me.
--
MST
