From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43188) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfrKU-0004Wp-HV for qemu-devel@nongnu.org; Fri, 24 May 2013 08:42:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UfrKP-0005qn-Cf for qemu-devel@nongnu.org; Fri, 24 May 2013 08:42:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2954) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UfrKP-0005qd-4K for qemu-devel@nongnu.org; Fri, 24 May 2013 08:42:37 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4OCgZqo027627 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 24 May 2013 08:42:36 -0400 Message-ID: <519F603A.3080501@redhat.com> Date: Fri, 24 May 2013 06:42:34 -0600 From: Eric Blake MIME-Version: 1.0 References: <1369377846-18439-1-git-send-email-akong@redhat.com> In-Reply-To: <1369377846-18439-1-git-send-email-akong@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2SAEMSOUKULUFDMMMIQSR" Subject: Re: [Qemu-devel] [PATCH v4] net: introduce command to query rx-filter information List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: mst@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2SAEMSOUKULUFDMMMIQSR Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/24/2013 12:44 AM, Amos Kong wrote: > We want to implement mac programming over macvtap through Libvirt. The > related rx-filter information of the nic contains main mac, rx-mode > items. >=20 > This patch adds a QMP event to notify management of rx-filter change, > and adds a monitor command for management to query rx-filter > information. >=20 > A flag for each nic is used to avoid events flooding, if management > doesn't query rx-filter after it receives one event, new events won't > be emitted to QMP monitor. >=20 > There maybe exist an uncontrollable delay, guests normally expect > rx-filter updates immediately, but it's another separate issue, we > can investigate it after Libvirt work is done. >=20 > Signed-off-by: Amos Kong > --- > +++ b/QMP/qmp-events.txt > @@ -172,6 +172,25 @@ Data: > }, > "timestamp": { "seconds": 1265044230, "microseconds": 450486 } } > =20 > +NIC_RX_FILTER_CHANGED > +----------------- > + > +Emitted when rx-filter configuration of nic is changed by the guest. > +Each nic has a flag to control event emit, the flag is set to false > +when it emits one event of the nic, the flag is set to true when > +management queries the rx-filter of the nic. This is used to avoid > +events flooding. > + > +Data: > + > +- "name": net client name (json-string) Missing documentation on the "path" member. > + > +{ "event": "NIC_RX_FILTER_CHANGED", > + "data": { "name": "vnet0", > + "path": "/machine/peripheral/vnet0/virtio-backend" }, > + "timestamp": { "seconds": 1368697518, "microseconds": 326866 } } > +} Hmm - if we are going to add introspection of events, we probably need to improve this file to call out ALL the events in JSON format (not just English text describing the event and then an example of an emitted event) - but that's a different task for a different series. > +++ b/include/net/net.h > @@ -74,6 +76,7 @@ struct NetClientState { > unsigned receive_disabled : 1; > NetClientDestructor *destructor; > unsigned int queue_index; > + bool rxfilter_notify_enabled:true; Very unusual to use 'true' instead of '1' for a bitfield length (valid C, but I've never seen it done in the wild). Also, the code base has very few bit fields of type bool (I only found 4: two in block/raw-posix.c and two in hw/intc/openpic.c); we generally prefer 'int' or 'unsigned' bitfields, where bitfields make sense. For that matter, is this really a struct where we are trying to cram as much information into space such that a bitfield helps us, or can we just use plain 'bool' instead of trying to make it a bitfield? And if a bitfield is important, then why not group this next to the receive_disabled bitfield so that the two share the same byte, instead of wasting alignment for two disjoint bitfields? Everything else looked okay to me. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2SAEMSOUKULUFDMMMIQSR Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRn2A6AAoJEKeha0olJ0Nq0I8IAIutQeMjVOa60UI17y6t/gIb AGrPVCja3BtX5ShLfzh3hloTdnIm3pVttBrridMLbJ0OQsyNVHNSxIeaT6bjsowQ 3FVEo/xnc7AcHWfM5H/6hsEQryfc3C/Rr1/FssorspcofXk0OeJ4lXtjgh4VvCh2 i6zwP38KkgB9Z1BJgPCbLzMWprCJot9/cFXbv1iLL2hO15qRn8p0bifbNLnisR76 wiaOU66Mc6yg5Vptd1Psq6RkeRwhGyb210Mv9aiX4ILGT0/KYnHbESjPvAjA/bbJ l64rG40KjbOvDtfMjzCSv0aVf/VcqBbmWdGsaB0ZR8BOdAAT6oxLMRaOQm9VDJs= =5a/s -----END PGP SIGNATURE----- ------enig2SAEMSOUKULUFDMMMIQSR--