I was able to implement the Extension 230 on top of these, so looking good :-)
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 27, 2016, at 3:50 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> ONF introduced a number of "standard extensions" that use its own
> vendor (experimenter) ID. This commit adds support for such extensions to
> ofp-msgs.
>
> These extensions were already half-supported, so there's barely any change
> to build-aux/extract-ofp-msgs.
>
> This isn't fully tested, since nothing adds support for such a message yet.
>
> Requested-by: Jarno Rajahalme <ja...@ovn.org>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> build-aux/extract-ofp-msgs | 2 +-
> include/openflow/nicira-ext.h | 16 --------
> include/openflow/openflow-1.1.h | 11 ------
> include/openflow/openflow-common.h | 11 ------
> lib/ofp-msgs.c | 75 ++++++++++++++++++++++----------------
> lib/ofp-msgs.h | 17 ++++++---
> 6 files changed, 56 insertions(+), 76 deletions(-)
>
> diff --git a/build-aux/extract-ofp-msgs b/build-aux/extract-ofp-msgs
> index b00039d..d3c4d4c 100755
> --- a/build-aux/extract-ofp-msgs
> +++ b/build-aux/extract-ofp-msgs
> @@ -161,7 +161,7 @@ def extract_ofp_msgs(output_file_name):
> hdrs = (version, OFPT10_STATS_REPLY, number, 0, 0)
> else:
> hdrs = (version, OFPT11_STATS_REPLY, number, 0, 0)
> - elif type_ == 'ONF':
> + elif type_ == 'ONFT':
> hdrs = (version, OFPT_VENDOR, 0, ONF_VENDOR_ID, number)
> elif type_ == 'ONFST' and name.endswith('_REQUEST'):
> if version == VERSION["1.0"]:
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index dad8707..fdee330 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -68,14 +68,6 @@ struct nx_vendor_error {
>
> /* Nicira vendor requests and replies. */
>
> -/* Header for Nicira vendor requests and replies. */
> -struct nicira_header {
> - struct ofp_header header;
> - ovs_be32 vendor; /* NX_VENDOR_ID. */
> - ovs_be32 subtype; /* See the NXT numbers in ofp-msgs.h. */
> -};
> -OFP_ASSERT(sizeof(struct nicira_header) == 16);
> -
> /* Header for Nicira vendor stats request and reply messages in OpenFlow
> * 1.0. */
> struct nicira10_stats_msg {
> @@ -85,14 +77,6 @@ struct nicira10_stats_msg {
> };
> OFP_ASSERT(sizeof(struct nicira10_stats_msg) == 24);
>
> -/* Header for Nicira vendor stats request and reply messages in OpenFlow
> - * 1.1. */
> -struct nicira11_stats_msg {
> - struct ofp11_vendor_stats_msg vsm; /* Vendor NX_VENDOR_ID. */
> - ovs_be32 subtype; /* One of NXST_* below. */
> -};
> -OFP_ASSERT(sizeof(struct nicira11_stats_msg) == 24);
> -
> /* Fields to use when hashing flows. */
> enum nx_hash_fields {
> /* Ethernet source address (NXM_OF_ETH_SRC) only. */
> diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
> index 361150a..63d8b41 100644
> --- a/include/openflow/openflow-1.1.h
> +++ b/include/openflow/openflow-1.1.h
> @@ -391,17 +391,6 @@ struct ofp11_stats_msg {
> };
> OFP_ASSERT(sizeof(struct ofp11_stats_msg) == 16);
>
> -/* Vendor extension stats message. */
> -struct ofp11_vendor_stats_msg {
> - struct ofp11_stats_msg osm; /* Type OFPST_VENDOR. */
> - ovs_be32 vendor; /* Vendor ID:
> - * - MSB 0: low-order bytes are IEEE OUI.
> - * - MSB != 0: defined by OpenFlow
> - * consortium. */
> - /* Followed by vendor-defined arbitrary additional data. */
> -};
> -OFP_ASSERT(sizeof(struct ofp11_vendor_stats_msg) == 20);
> -
> /* Stats request of type OFPST_FLOW. */
> struct ofp11_flow_stats_request {
> uint8_t table_id; /* ID of table to read (from ofp_table_stats),
> diff --git a/include/openflow/openflow-common.h
> b/include/openflow/openflow-common.h
> index da2b7a5..39c0e66 100644
> --- a/include/openflow/openflow-common.h
> +++ b/include/openflow/openflow-common.h
> @@ -422,17 +422,6 @@ struct ofp_hello_elem_header {
> };
> OFP_ASSERT(sizeof(struct ofp_hello_elem_header) == 4);
>
> -/* Vendor extension. */
> -struct ofp_vendor_header {
> - struct ofp_header header; /* Type OFPT_VENDOR or OFPT_EXPERIMENTER. */
> - ovs_be32 vendor; /* Vendor ID:
> - * - MSB 0: low-order bytes are IEEE OUI.
> - * - MSB != 0: defined by OpenFlow
> - * consortium. */
> - /* Vendor-defined arbitrary additional data. */
> -};
> -OFP_ASSERT(sizeof(struct ofp_vendor_header) == 12);
> -
> /* Table numbering. Tables can use any number up to OFPT_MAX. */
> enum ofp_table {
> /* Last usable table number. */
> diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
> index 944ab33..fcfbb50 100644
> --- a/lib/ofp-msgs.c
> +++ b/lib/ofp-msgs.c
> @@ -35,6 +35,39 @@ VLOG_DEFINE_THIS_MODULE(ofp_msgs);
> #define OFPT11_STATS_REPLY 19
> #define OFPST_VENDOR 0xffff
>
> +/* Vendor extension message. */
> +struct ofp_vendor_header {
> + struct ofp_header header; /* OFPT_VENDOR. */
> + ovs_be32 vendor; /* Vendor ID:
> + * - MSB 0: low-order bytes are IEEE OUI.
> + * - MSB != 0: defined by OpenFlow
> + * consortium. */
> +
> + /* In theory everything after 'vendor' is vendor specific. In practice,
> + * the vendors we support put a 32-bit subtype here. We'll change this
> + * structure if we start adding support for other vendor formats. */
> + ovs_be32 subtype; /* Vendor-specific subtype. */
> +
> + /* Followed by vendor-defined additional data. */
> +};
> +OFP_ASSERT(sizeof(struct ofp_vendor_header) == 16);
> +
> +/* Vendor extension stats message. */
> +struct ofp11_vendor_stats_msg {
> + struct ofp11_stats_msg osm; /* Type OFPST_VENDOR. */
> + ovs_be32 vendor; /* Vendor ID:
> + * - MSB 0: low-order bytes are IEEE OUI.
> + * - MSB != 0: defined by OpenFlow
> + * consortium. */
> +
> + /* In theory everything after 'vendor' is vendor specific. In practice,
> + * the vendors we support put a 32-bit subtype here. We'll change this
> + * structure if we start adding support for other vendor formats. */
> + ovs_be32 subtype; /* Vendor-specific subtype. */
> +
> + /* Followed by vendor-defined additional data. */
> +};
> +OFP_ASSERT(sizeof(struct ofp11_vendor_stats_msg) == 24);
> /* A thin abstraction of OpenFlow headers:
> *
> * - 'version' and 'type' come straight from struct ofp_header, so these are
> @@ -159,15 +192,8 @@ ofphdrs_decode(struct ofphdrs *hdrs,
>
> ovh = (const struct ofp_vendor_header *) oh;
> hdrs->vendor = ntohl(ovh->vendor);
> - if (hdrs->vendor == NX_VENDOR_ID) {
> - /* Get Nicira message subtype (NXT_*). */
> - const struct nicira_header *nh;
> -
> - if (length < sizeof *nh) {
> - return OFPERR_OFPBRC_BAD_LEN;
> - }
> - nh = (const struct nicira_header *) oh;
> - hdrs->subtype = ntohl(nh->subtype);
> + if (hdrs->vendor == NX_VENDOR_ID || hdrs->vendor == ONF_VENDOR_ID) {
> + hdrs->subtype = ntohl(ovh->subtype);
> } else {
> log_bad_vendor(hdrs->vendor);
> return OFPERR_OFPBRC_BAD_VENDOR;
> @@ -230,15 +256,9 @@ ofphdrs_decode(struct ofphdrs *hdrs,
>
> ovsm = (const struct ofp11_vendor_stats_msg *) oh;
> hdrs->vendor = ntohl(ovsm->vendor);
> - if (hdrs->vendor == NX_VENDOR_ID) {
> - /* Get Nicira statistic type (NXST_*). */
> - const struct nicira11_stats_msg *nsm;
> -
> - if (length < sizeof *nsm) {
> - return OFPERR_OFPBRC_BAD_LEN;
> - }
> - nsm = (const struct nicira11_stats_msg *) oh;
> - hdrs->subtype = ntohl(nsm->subtype);
> + if (hdrs->vendor == NX_VENDOR_ID ||
> + hdrs->vendor == ONF_VENDOR_ID) {
> + hdrs->subtype = ntohl(ovsm->subtype);
> } else {
> log_bad_vendor(hdrs->vendor);
> return OFPERR_OFPBRC_BAD_VENDOR;
> @@ -308,7 +328,7 @@ size_t
> ofphdrs_len(const struct ofphdrs *hdrs)
> {
> if (hdrs->type == OFPT_VENDOR) {
> - return sizeof(struct nicira_header);
> + return sizeof(struct ofp_vendor_header);
> }
>
> switch ((enum ofp_version) hdrs->version) {
> @@ -329,7 +349,7 @@ ofphdrs_len(const struct ofphdrs *hdrs)
> if (hdrs->type == OFPT11_STATS_REQUEST ||
> hdrs->type == OFPT11_STATS_REPLY) {
> return (hdrs->stat == OFPST_VENDOR
> - ? sizeof(struct nicira11_stats_msg)
> + ? sizeof(struct ofp11_vendor_stats_msg)
> : sizeof(struct ofp11_stats_msg));
> }
> break;
> @@ -675,11 +695,10 @@ ofpraw_put__(enum ofpraw raw, uint8_t version, ovs_be32
> xid,
> oh->xid = xid;
>
> if (hdrs->type == OFPT_VENDOR) {
> - struct nicira_header *nh = buf->header;
> + struct ofp_vendor_header *ovh = buf->header;
>
> - ovs_assert(hdrs->vendor == NX_VENDOR_ID);
> - nh->vendor = htonl(hdrs->vendor);
> - nh->subtype = htonl(hdrs->subtype);
> + ovh->vendor = htonl(hdrs->vendor);
> + ovh->subtype = htonl(hdrs->subtype);
> } else if (version == OFP10_VERSION
> && (hdrs->type == OFPT10_STATS_REQUEST ||
> hdrs->type == OFPT10_STATS_REPLY)) {
> @@ -714,13 +733,7 @@ ofpraw_put__(enum ofpraw raw, uint8_t version, ovs_be32
> xid,
> struct ofp11_vendor_stats_msg *ovsm = buf->header;
>
> ovsm->vendor = htonl(hdrs->vendor);
> - if (hdrs->vendor == NX_VENDOR_ID) {
> - struct nicira11_stats_msg *nsm = buf->header;
> -
> - nsm->subtype = htonl(hdrs->subtype);
> - } else {
> - OVS_NOT_REACHED();
> - }
> + ovsm->subtype = htonl(hdrs->subtype);
> }
> }
> }
> diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
> index a15efb6..38a2803 100644
> --- a/lib/ofp-msgs.h
> +++ b/lib/ofp-msgs.h
> @@ -60,9 +60,14 @@ struct ovs_list;
> *
> * where the syntax of each part is:
> *
> - * - type: One of OFPT (standard OpenFlow message), OFPST (standard
> OpenFlow
> - * statistics message), NXT (Nicira extension message), or NXST (Nicira
> - * extension statistics message).
> + * - type: One of the following:
> + *
> + * * OFPT: standard OpenFlow message.
> + * * OFPST: standard OpenFlow statistics or multipart message.
> + * * NXT: Nicira extension message.
> + * * NXST: Nicira extension statistics or multipart message.
> + * * ONFT: Open Networking Foundation extension message.
> + * * ONFST: Open Networking Foundation multipart message.
> *
> * As new vendors implement extensions it will make sense to expand the
> * dictionary of possible types.
> @@ -73,9 +78,9 @@ struct ovs_list;
> * - number:
> * For OFPT, the 'type' in struct ofp_header.
> * For OFPST, the 'type' in struct ofp_stats_msg or ofp11_stats_msg.
> - * For NXT, the 'subtype' in struct nicira_header.
> - * For NXST, the 'subtype' in struct nicira10_stats_msg or
> - * nicira11_stats_msg.
> + * For NXT or ONFT, the 'subtype' in struct ofp_vendor_header.
> + * For NXST or ONFST, the 'subtype' in an appropriate vendor stats
> + * struct.
> *
> * - arguments: The types of data that follow the OpenFlow headers (the
> * message "body"). This can be "void" if the message has no body.
> --
> 2.1.3
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
