From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeiHk-0000LV-Os for qemu-devel@nongnu.org; Tue, 21 May 2013 04:51:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UeiHe-0001rP-Hb for qemu-devel@nongnu.org; Tue, 21 May 2013 04:51:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62698) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UeiHe-0001rI-AC for qemu-devel@nongnu.org; Tue, 21 May 2013 04:51:02 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4L8p1lg004794 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 21 May 2013 04:51:01 -0400 Date: Tue, 21 May 2013 11:51:17 +0300 From: "Michael S. Tsirkin" Message-ID: <20130521085117.GC27391@redhat.com> References: <1368702445-30733-1-git-send-email-akong@redhat.com> <1368702445-30733-2-git-send-email-akong@redhat.com> <20130516121745.GE31841@redhat.com> <20130521050455.GD3915@t430s.nay.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130521050455.GD3915@t430s.nay.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 1/2] net: introduce MAC_TABLE_CHANGED event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: qemu-devel@nongnu.org, stefanha@redhat.com, lcapitulino@redhat.com 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