From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y7C59-0006bm-PO for qemu-devel@nongnu.org; Fri, 02 Jan 2015 18:56:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y7C56-0006Zz-HM for qemu-devel@nongnu.org; Fri, 02 Jan 2015 18:56:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y7C56-0006Zt-9L for qemu-devel@nongnu.org; Fri, 02 Jan 2015 18:56:36 -0500 Message-ID: <54A7302E.50807@redhat.com> Date: Fri, 02 Jan 2015 16:56:30 -0700 From: Eric Blake MIME-Version: 1.0 References: <1419916451-49258-9-git-send-email-sfeldma@gmail.com> In-Reply-To: <1419916451-49258-9-git-send-email-sfeldma@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="382JO5iMtK9KcaFSepKHfJousV0BDdwfx" Subject: Re: [Qemu-devel] [PATCH 08/10] qmp: add rocker device support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sfeldma@gmail.com, qemu-devel@nongnu.org, jiri@resnulli.us, roopa@cumulusnetworks.com, john.fastabend@gmail.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --382JO5iMtK9KcaFSepKHfJousV0BDdwfx Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/29/2014 10:14 PM, sfeldma@gmail.com wrote: > From: Scott Feldman [your message came through as a top-level thread instead of in-reply-to the 0/10 cover letter; please see if you can fix that before the next submission] >=20 > Add QMP/HMP support for rocker devices. This is mostly for debugging p= urposes > to see inside the device's tables and port configurations. Some exampl= es: >=20 In this mail, I'll review just the QMP interface portion: > +++ b/qapi-schema.json > @@ -3515,3 +3515,54 @@ > # Since: 2.1 > ## > { 'command': 'rtc-reset-reinjection' } > + > +{ 'type': 'Rocker', > + 'data': { 'name': 'str', 'id': 'uint64', 'ports': 'uint32' } } Please add documentation for each new type and each member of that type, including mention of which qemu release will first contain it. > + > +{ 'command': 'rocker', > + 'data': { 'name': 'str' }, > + 'returns': 'Rocker' } > + > +{ 'type': 'RockerPort', > + 'data': { 'name': 'str', 'enabled': 'bool', 'link_up': 'bool', > + 'speed': 'uint32', 'duplex': 'uint8', 'autoneg': 'uint8' }= } Why is 'duplex' a 'uint8'? Shouldn't it instead be an enum type? Similar for 'autoneg'. > + > +{ 'command': 'rocker-ports', > + 'data': { 'name': 'str' }, > + 'returns': ['RockerPort'] } > + > +{ 'type': 'RockerOfDpaFlowKey', > + 'data' : { 'priority': 'uint32', 'tbl_id': 'uint32', '*in_pport': 'u= int32', Please use '-' rather than '_' in the spelling of new commands. > + '*tunnel_id': 'uint32', '*vlan_id': 'uint16', > + '*eth_type': 'uint16', '*eth_src': 'str', '*eth_dst': 'st= r', > + '*ip_proto': 'uint8', '*ip_tos': 'uint8', '*ip_dst': 'str= ' } } Is 'str' the right type for IP addresses, or should it be a more specific type? > + > +{ 'type': 'RockerOfDpaFlowMask', > + 'data' : { '*in_pport': 'uint32', '*tunnel_id': 'uint32', > + '*vlan_id': 'uint16', '*eth_src': 'str', '*eth_dst': 'str= ', > + '*ip_proto': 'uint8', '*ip_tos': 'uint8' } } > + > +{ 'type': 'RockerOfDpaFlowAction', > + 'data' : { '*goto_tbl': 'uint32', '*group_id': 'uint32', > + '*tun_log_pport': 'uint32', '*vlan_id': 'uint16', > + '*new_vlan_id': 'uint16', '*out_pport': 'uint32' } } > + > +{ 'type': 'RockerOfDpaFlow', > + 'data': { 'cookie': 'uint64', 'hits': 'uint64', 'key': 'RockerOfDpaF= lowKey', > + 'mask': 'RockerOfDpaFlowMask', 'action': 'RockerOfDpaFlowA= ction' } } > + > +{ 'command': 'rocker-of-dpa-flows', > + 'data': { 'name': 'str', '*tbl_id': 'uint32' }, > + 'returns': ['RockerOfDpaFlow'] } Is the idea here that omitting 'tbl_id' (should be spelled 'tbl-id') will give you an array of all table entries, while specifying an id will give you an array of just the one requested entry? > + > +{ 'type': 'RockerOfDpaGroup', > + 'data': { 'id': 'uint32', 'type': 'uint8', '*vlan_id': 'uint16', > + '*pport': 'uint32', '*index': 'uint32', '*out_pport': 'uin= t32', > + '*group_id': 'uint32', '*set_vlan_id': 'uint16', > + '*pop_vlan': 'uint8', '*group_ids': ['uint32'], > + '*set_eth_src': 'str', '*set_eth_dst': 'str', > + '*ttl_check': 'uint8' } } > + > +{ 'command': 'rocker-of-dpa-groups', > + 'data': { 'name': 'str', '*type': 'uint8' }, > + 'returns': ['RockerOfDpaGroup'] } Needs documentation before I can tell for sure, but based on just the qapi specification, I think it will be reasonable once you fix naming conventions. > +++ b/qmp-commands.hx > @@ -3860,3 +3860,27 @@ Move mouse pointer to absolute coordinates (2000= 0, 400). > <- { "return": {} } > =20 > EQMP > + > + { > + .name =3D "rocker", > + .args_type =3D "name:s", > + .mhandler.cmd_new =3D qmp_marshal_input_rocker, > + }, > + Could you also provide example exchanges for each command added (what the user passes in, and what qemu responds)? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --382JO5iMtK9KcaFSepKHfJousV0BDdwfx 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJUpzAuAAoJEKeha0olJ0NqS5wH/2nwInl6537QSVNTswCW6rWZ YjNU4NEAlHwaudHdqz8bGXX7Wn6J5h+BKQuttEJanJTBtofkHnl1IIjB87MfUeex qbF74IORA6+giMz7kp6wYrYHxbg48XqGErY4nnZVasrsuwO8yutLMnDw+OhBbJSV 9H522koLHUO7z85TkJj6csCePBU3kEiQQcw8LbSvhGbO9e+MoGLeUGz3SCk4OXDZ OrdpKCSZdZejLTtlMbOfWmQ0hAeTqw2x5Hh5p/ICji3d/xxBCJ4858Ut4T0lWINf pAHW30BS6CKTmjMULh2vK5XlDzrPoqxzLMw3FWMaC3l3yy9LC9PBua+m5ZMY3B0= =KcyY -----END PGP SIGNATURE----- --382JO5iMtK9KcaFSepKHfJousV0BDdwfx--