[ovs-dev] [PATCH 1/2] ofp-msgs: Add support for ONF extension messages.

Jarno Rajahalme jarno at ovn.org
Thu Feb 18 18:46:29 UTC 2016


I was able to implement the Extension 230 on top of these, so looking good :-)

Acked-by: Jarno Rajahalme <jarno at ovn.org>

> On Jan 27, 2016, at 3:50 PM, Ben Pfaff <blp at 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 <jarno at ovn.org>
> Signed-off-by: Ben Pfaff <blp at 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
> 




More information about the dev mailing list