From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIf7x-0000Lc-TA for qemu-devel@nongnu.org; Tue, 03 Feb 2015 10:10:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YIf7t-0008R2-QZ for qemu-devel@nongnu.org; Tue, 03 Feb 2015 10:10:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40004) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIf7t-0008Qu-HW for qemu-devel@nongnu.org; Tue, 03 Feb 2015 10:10:53 -0500 Message-ID: <54D0E4F5.5000902@redhat.com> Date: Tue, 03 Feb 2015 08:10:45 -0700 From: Eric Blake MIME-Version: 1.0 References: <1421913839-22448-1-git-send-email-sfeldma@gmail.com> <1421913839-22448-8-git-send-email-sfeldma@gmail.com> In-Reply-To: <1421913839-22448-8-git-send-email-sfeldma@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bcuXJ0pDwgsM1F2iJVjTheM8bEMsujvTH" Subject: Re: [Qemu-devel] [PATCH v5 07/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, pbonzini@redhat.com, stefanha@gmail.com, dsahern@gmail.com, jasowang@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bcuXJ0pDwgsM1F2iJVjTheM8bEMsujvTH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/22/2015 01:03 AM, sfeldma@gmail.com wrote: > From: Scott Feldman >=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 QMP interface review: > +++ b/qapi-schema.json > @@ -3523,3 +3523,6 @@ > # Since: 2.1 > ## > { 'command': 'rtc-reset-reinjection' } > + > +# Rocker ethernet network switch > +{ 'include': 'qapi/rocker.json' } > diff --git a/qapi/rocker.json b/qapi/rocker.json > new file mode 100644 > index 0000000..326c6c7 > --- /dev/null > +++ b/qapi/rocker.json > @@ -0,0 +1,259 @@ > +## > +# @Rocker: > +# > +# Rocker switch information. > +# > +# @name: switch name > +# > +# @id: switch ID > +# > +# @ports: number of front-panel ports > +## Missing a 'Since: 2.3' designation. > +{ 'type': 'RockerSwitch', > + 'data': { 'name': 'str', 'id': 'uint64', 'ports': 'uint32' } } > + > +## > +# @rocker: > +# > +# Return rocker switch information. > +# > +# Returns: @Rocker information > +# > +# Since: 2.3 > +## > +{ 'command': 'rocker', > + 'data': { 'name': 'str' }, > + 'returns': 'RockerSwitch' } Should this command be named 'query-rocker', as it is used for queries? Should the 'name' argument be optional, and the output be an array (all rocker devices, rather than just a given rocker name lookup)? > + > +## > +# @RockerPortDuplex: > +# > +# An eumeration of port duplex states. > +# > +# @half: half duplex > +# > +# @full: full duplex > +## Missing a 'Since: 2.3' designation. > +{ 'enum': 'RockerPortDuplex', 'data': [ 'half', 'full' ] } > + > +## > +# @RockerPortAutoneg: > +# > +# An eumeration of port autoneg states. > +# > +# @off: autoneg is off > +# > +# @on: autoneg is on > +## Missing a 'Since: 2.3' designation. > +{ 'enum': 'RockerPortAutoneg', 'data': [ 'off', 'on' ] } > + > +## > +# @RockerPort: > +# > +# Rocker switch port information. > +# > +# @name: port name > +# > +# @enabled: port is enabled for I/O > +# > +# @link-up: physical link is UP on port > +# > +# @speed: port link speed in Mbps > +# > +# @duplex: port link duplex > +# > +# @autoneg: port link autoneg > +## Missing a 'Since: 2.3' designation. > +{ 'type': 'RockerPort', > + 'data': { 'name': 'str', 'enabled': 'bool', 'link-up': 'bool', > + 'speed': 'uint32', 'duplex': 'RockerPortDuplex', > + 'autoneg': 'RockerPortAutoneg' } } > + > +## > +# @rocker-ports: > +# > +# Return rocker switch information. > +# > +# Returns: @Rocker information > +# > +# Since: 2.3 > +## > +{ 'command': 'rocker-ports', Should this be named 'query-rocker-ports'? Should the port information be returned as part of the more generic 'rocker' command rather than having to do a two-stage query (what are my rocker devices, then for each device what are the ports)? > + 'data': { 'name': 'str' }, > + 'returns': ['RockerPort'] } > + > +## > +# @RockerOfDpaFlowKey: > +# > +# Rocker switch OF-DPA flow key > +# > +# @priority: key priority, 0 being lowest priority > +# > +# @tbl-id: flow table ID > +# > +# @in-pport: physical input port > +# > +# @tunnel-id: tunnel ID > +# > +# @vlan-id: VLAN ID > +# > +# @eth-type: Ethernet header type > +# > +# @eth-src: Ethernet header source MAC address > +# > +# @eth-dst: Ethernet header destination MAC address > +# > +# @ip-proto: IP Header protocol field > +# > +# @ip-tos: IP header TOS field > +# > +# @ip-dst: IP header destination address > +## Missing a 'Since: 2.3' designation. > +{ 'type': 'RockerOfDpaFlowKey', > + 'data' : { 'priority': 'uint32', 'tbl-id': 'uint32', '*in-pport': 'u= int32', > + '*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= ' } } Missing '#optional' tags on the various optional fields. Why are certain fields optional? Does it mean they have a default value, or that they don't make sense in some configurations? The docs could be more clear on that. > + > +## > +# @RockerOfDpaFlowMask: > +# > +# Rocker switch OF-DPA flow mask > +# > +# @in-pport: physical input port > +# > +# @tunnel-id: tunnel ID > +# > +# @vlan-id: VLAN ID > +# > +# @eth-src: Ethernet header source MAC address > +# > +# @eth-dst: Ethernet header destination MAC address > +# > +# @ip-proto: IP Header protocol field > +# > +# @ip-tos: IP header TOS field > +## Missing a 'Since: 2.3' designation. > +{ 'type': 'RockerOfDpaFlowMask', > + 'data' : { '*in-pport': 'uint32', '*tunnel-id': 'uint32', > + '*vlan-id': 'uint16', '*eth-src': 'str', '*eth-dst': 'str= ', > + '*ip-proto': 'uint8', '*ip-tos': 'uint8' } } Again, missing #optional tags in the docs, as well as what it means when a field is omitted. > + > +## > +# @RockerOfDpaFlowAction: > +# > +# Rocker switch OF-DPA flow action > +# > +# @goto-tbl: next table ID > +# > +# @group-id: group ID > +# > +# @tunnel-lport: tunnel logical port ID > +# > +# @vlan-id: VLAN ID > +# > +# @new-vlan-id: new VLAN ID > +# > +# @out-pport: physical output port > +## Missing a 'Since: 2.3' designation. > +{ 'type': 'RockerOfDpaFlowAction', > + 'data' : { '*goto-tbl': 'uint32', '*group-id': 'uint32', > + '*tunnel-lport': 'uint32', '*vlan-id': 'uint16', > + '*new-vlan-id': 'uint16', '*out-pport': 'uint32' } } Repeat of #optional comments... > + > +## > +# @RockerOfDpaFlow: > +# > +# Rocker switch OF-DPA flow > +# > +# @cookie: flow unique cookie ID > +# > +# @hits: count of matches (hits) on flow > +# > +# @key: flow key > +# > +# @mask: flow mask > +# > +# @action: flow action > +# > +# Since: 2.3 > +## > +{ 'type': 'RockerOfDpaFlow', > + 'data': { 'cookie': 'uint64', 'hits': 'uint64', 'key': 'RockerOfDpaF= lowKey', > + 'mask': 'RockerOfDpaFlowMask', 'action': 'RockerOfDpaFlowA= ction' } } > + > +## > +# @rocker-of-dpa-flows: > +# > +# Return rocker OF-DPA flow information. > +# > +# @name: switch name > +# > +# @tbl-id: (optional) flow table ID. If tbl-id is not specified, retu= rns s/(optional)/#optional/ for consistency with other .json files > +# flow information for all tables. > +# > +# Returns: @Rocker OF-DPA flow information > +# > +# Since: 2.3 > +## > +{ 'command': 'rocker-of-dpa-flows', Should this command be in the query-* namespace? > + 'data': { 'name': 'str', '*tbl-id': 'uint32' }, > + 'returns': ['RockerOfDpaFlow'] } > + > +## > +# @RockerOfDpaGroup: > +# > +# Rocker switch OF-DPA group > +# > +# @id: group unique ID > +# > +# @type: group type > +# > +# @vlan-id: VLAN ID > +# > +# @pport: physical port number > +# > +# @index: group index, unique with group type > +# > +# @out-pport: output physical port number > +# > +# @group-id: next group ID > +# > +# @set-vlan-id: VLAN ID to set > +# > +# @pop-vlan: pop VLAN headr from packet > +# > +# @group-ids: list of next group IDs > +# > +# @set-eth-src: set source MAC address in Ethernet header > +# > +# @set-eth-dst: set destination MAC address in Ethernet header > +# > +# @ttl-check: perform TTL check > +## Missing a 'Since: 2.3' designation. > +{ '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' } } Again on #optional... > + > +## > +# @rocker-of-dpa-groups: > +# > +# Return rocker OF-DPA group information. > +# > +# @name: switch name > +# > +# @type: (optional) group type. If type is not specified, returns > +# group information for all group types. > +# > +# Returns: @Rocker OF-DPA group information > +# > +# Since: 2.3 > +## > +{ 'command': 'rocker-of-dpa-groups', > + 'data': { 'name': 'str', '*type': 'uint8' }, > + 'returns': ['RockerOfDpaGroup'] } > + > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 8957201..9007c12 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3902,3 +3902,100 @@ Move mouse pointer to absolute coordinates (200= 00, 400). > <- { "return": {} } > =20 > EQMP > + > + { > + .name =3D "query-rocker", Doesn't match the naming in your .json file (but I do like query-rocker better). > + .args_type =3D "name:s", > + .mhandler.cmd_new =3D qmp_marshal_input_rocker, > + }, > + > +SQMP > +Show rocker switch > +------------------ > + > +Arguments: > + > +- "name": switch name > + > +Example: > + > +-> { "execute": "query-rocker", "arguments": { "name": "sw1" } } > +<- { "return": {"name": "sw1", "ports": 2, "id": 1327446905938}} Is there a command to learn which switch names exist? As written, this command assumes you already know the switch name, which makes the switch name on output a bit redundant. My suggestion above was to make 'name' be optional on input, and return an array on output. > + > +EQMP > + > + { > + .name =3D "query-rocker-ports", Doesn't match .json naming. > + .args_type =3D "name:s", > + .mhandler.cmd_new =3D qmp_marshal_input_rocker_ports, > + }, > + > +SQMP > +Show rocker switch ports > +------------------------ > + > +Arguments: > + > +- "name": switch name > + > +Example: > + > +-> { "execute": "query-rocker-ports", "arguments": { "name": "sw1" } }= > +<- { "return": [ {"duplex": "full", "enabled": true, "name": "sw1.1", = "autoneg": "off", "link-up": true, "speed": 10000}, > + {"duplex": "full", "enabled": true, "name": "sw1.2", = "autoneg": "off", "link-up": true, "speed": 10000} > + ]} > + > +EQMP > + > + { > + .name =3D "query-rocker-of-dpa-flows", Another naming mismatch. > + .args_type =3D "name:s,tbl-id:i?", > + .mhandler.cmd_new =3D qmp_marshal_input_rocker_of_dpa_flows, > + }, > + > +SQMP > +Show rocker switch OF-DPA flow tables > +------------------------------------- > + > +Arguments: > + > +- "name": switch name > +- "tbl-id": (optional) flow table ID > + > +Example: > + > +-> { "execute": "query-rocker-of-dpa-flows", "arguments": { "name": "s= w1" } } > +<- { "return": [ {"key": {"in-pport": 0, "priority": 1, "tbl-id": 0}, > + "hits": 138, > + "cookie": 0, > + "action": {"goto-tbl": 10}, > + "mask": {"in-pport": 4294901760} > + }, > + {...more...}, > + ]} > + > +EQMP > + > + { > + .name =3D "query-rocker-of-dpa-groups", and again > + .args_type =3D "name:s,type:i?", > + .mhandler.cmd_new =3D qmp_marshal_input_rocker_of_dpa_groups, > + }, > + > +SQMP > +Show rocker OF-DPA group tables > +------------------------------- > + > +Arguments: > + > +- "name": switch name > +- "type": (optional) group type > + > +Example: > + > +-> { "execute": "query-rocker-of-dpa-groups", "arguments": { "name": "= sw1" } } > +<- { "return": [ {"type": 0, "out-pport": 2, "pport": 2, "vlan-id": 38= 41, "pop-vlan": 1, "id": 251723778}, > + {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 38= 41, "pop-vlan": 1, "id": 251723776}, > + {"type": 0, "out-pport": 1, "pport": 1, "vlan-id": 38= 40, "pop-vlan": 1, "id": 251658241}, > + {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 38= 40, "pop-vlan": 1, "id": 251658240} > + ]} >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --bcuXJ0pDwgsM1F2iJVjTheM8bEMsujvTH 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/ iQEcBAEBCAAGBQJU0OT2AAoJEKeha0olJ0Nqfq4H/07dEMkt3IDXWtVOWKTIUKqx 6ztKPW0ItVCTiNqx9EAmpLq29oUp6EXHu8TuxW0GvamG5cJMbnpCKVhbeDhVSAfy tlZd+dRU+kdCVwWuO2aFSDXzMX5XzOLdwA60mI440yfEO3eskN8v/iCIakXr7h94 lZOZlESBfTcil5EER/k8R0KST28TVP8ELlb6ro+94ulI7fpj6zHf1LquVxCXj4j0 rGQTXu6eb5IWiiu1Cpny7gz8GYuv5OamfW50cd1tFraIu/6/aSywDDFze5JbJs6N Z6XF5PRSEcaM3+RKOgpsXrpaga7mFwoZMmn561D0/we58VNYvk0EMmr3bH7RYPo= =SEmy -----END PGP SIGNATURE----- --bcuXJ0pDwgsM1F2iJVjTheM8bEMsujvTH--