I take that back, just now noticed your response to patch 2/2.
Jarno
> On Feb 24, 2016, at 2:44 PM, Jarno Rajahalme <ja...@ovn.org> wrote:
>
> Ping.
>
>> On Feb 18, 2016, at 10:46 AM, Jarno Rajahalme <ja...@ovn.org> wrote:
>>
>> 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
