With minor questions below,
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 18, 2016, at 11:27 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> These will see increasing use in upcoming commits.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> include/openflow/openflow-1.4.h | 27 -----
> include/openflow/openflow-1.5.h | 18 ----
> lib/ofp-prop.c | 126 ++++++++++++++++++++++-
> lib/ofp-prop.h | 9 ++
> lib/ofp-util.c | 218 ++++++++++------------------------------
> lib/ofpbuf.h | 11 +-
> 6 files changed, 192 insertions(+), 217 deletions(-)
>
> diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
> index daf6cf4..b65eeb8 100644
> --- a/include/openflow/openflow-1.4.h
> +++ b/include/openflow/openflow-1.4.h
> @@ -94,15 +94,6 @@ enum ofp14_port_mod_prop_type {
> OFPPMPT14_EXPERIMENTER = 0xFFFF, /* Experimenter property. */
> };
>
> -/* Ethernet port mod property. */
> -struct ofp14_port_mod_prop_ethernet {
> - ovs_be16 type; /* OFPPMPT14_ETHERNET. */
> - ovs_be16 length; /* Length in bytes of this property. */
> - ovs_be32 advertise; /* Bitmap of OFPPF_*. Zero all bits to prevent
> - any action taking place. */
> -};
> -OFP_ASSERT(sizeof(struct ofp14_port_mod_prop_ethernet) == 8);
> -
> struct ofp14_port_mod {
> ovs_be32 port_no;
> uint8_t pad[4];
> @@ -137,13 +128,6 @@ enum ofp14_table_reason {
> OFPTR_N_REASONS /* Denotes number of reasons. */
> };
>
> -struct ofp14_table_mod_prop_eviction {
> - ovs_be16 type; /* OFPTMPT14_EVICTION. */
> - ovs_be16 length; /* Length in bytes of this property. */
> - ovs_be32 flags; /* Bitmap of OFPTMPEF14_* flags */
> -};
> -OFP_ASSERT(sizeof(struct ofp14_table_mod_prop_eviction) == 8);
> -
> struct ofp14_table_mod_prop_vacancy {
> ovs_be16 type; /* OFPTMPT14_VACANCY. */
> ovs_be16 length; /* Length in bytes of this property. */
> @@ -273,17 +257,6 @@ enum ofp14_async_config_prop_type {
> OFPTFPT_EXPERIMENTER_MASTER = 0xFFFF, /* Experimenter for master. */
> };
>
> -/* Various reason based properties */
> -struct ofp14_async_config_prop_reasons {
> - /* 'type' is one of OFPACPT_PACKET_IN_*, OFPACPT_PORT_STATUS_*,
> - * OFPACPT_FLOW_REMOVED_*, OFPACPT_ROLE_STATUS_*,
> - * OFPACPT_TABLE_STATUS_*, OFPACPT_REQUESTFORWARD_*. */
> - ovs_be16 type;
> - ovs_be16 length; /* Length in bytes of this property. */
> - ovs_be32 mask; /* Bitmasks of reason values. */
> -};
> -OFP_ASSERT(sizeof(struct ofp14_async_config_prop_reasons) == 8);
> -
> /* Experimenter async config property */
> struct ofp14_async_config_prop_experimenter {
> ovs_be16 type; /* One of OFPTFPT_EXPERIMENTER_SLAVE,
> diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
> index b3deb2d..0c478d1 100644
> --- a/include/openflow/openflow-1.5.h
> +++ b/include/openflow/openflow-1.5.h
> @@ -69,24 +69,6 @@ enum ofp15_group_bucket_prop_type {
> OFPGBPT15_EXPERIMENTER = 0xFFFF, /* Experimenter defined. */
> };
>
> -/* Group bucket weight property, for select groups only. */
> -struct ofp15_group_bucket_prop_weight {
> - ovs_be16 type; /* OFPGBPT15_WEIGHT. */
> - ovs_be16 length; /* 8. */
> - ovs_be16 weight; /* Relative weight of bucket. */
> - uint8_t pad[2]; /* Pad to 64 bits. */
> -};
> -OFP_ASSERT(sizeof(struct ofp15_group_bucket_prop_weight) == 8);
> -
> -/* Group bucket watch port or watch group property, for fast failover groups
> - * only. */
> -struct ofp15_group_bucket_prop_watch {
> - ovs_be16 type; /* OFPGBPT15_WATCH_PORT or
> OFPGBPT15_WATCH_GROUP. */
> - ovs_be16 length; /* 8. */
> - ovs_be32 watch; /* The port or the group. */
> -};
> -OFP_ASSERT(sizeof(struct ofp15_group_bucket_prop_watch) == 8);
> -
> /* Bucket for use in groups. */
> struct ofp15_bucket {
> ovs_be16 len; /* Length the bucket in bytes, including
> diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c
> index 83d72ab..4215e40 100644
> --- a/lib/ofp-prop.c
> +++ b/lib/ofp-prop.c
> @@ -24,6 +24,21 @@
> #include "openvswitch/vlog.h"
> #include "util.h"
>
> +struct ofp_prop_be16 {
> + ovs_be16 type;
> + ovs_be16 len;
> + ovs_be16 value;
> + uint8_t pad[2];
> +};
> +BUILD_ASSERT_DECL(sizeof(struct ofp_prop_be16) == 8);
> +
> +struct ofp_prop_be32 {
> + ovs_be16 type;
> + ovs_be16 len;
> + ovs_be32 value;
> +};
> +BUILD_ASSERT_DECL(sizeof(struct ofp_prop_be32) == 8);
> +
> static uint32_t
> ofpprop_type_to_experimenter(uint64_t type)
> {
> @@ -84,11 +99,11 @@ ofpprop_pull__(struct ofpbuf *msg, struct ofpbuf
> *property,
>
> if (property) {
> ofpbuf_use_const(property, msg->data, len);
> - msg->header = msg->data;
> - msg->msg = ((uint8_t *) msg->data
> - + (type < min_exp
> - ? sizeof(struct ofp_prop_header)
> - : sizeof(struct ofp_prop_experimenter)));
> + property->header = property->data;
> + property->msg = ((uint8_t *) property->data
> + + (type < min_exp
> + ? sizeof(struct ofp_prop_header)
> + : sizeof(struct ofp_prop_experimenter)));
Is this a bug fix to a previous patch or just a convention change?
> }
> ofpbuf_pull(msg, padded_len);
> return 0;
> @@ -107,6 +122,68 @@ ofpprop_pull(struct ofpbuf *msg, struct ofpbuf
> *property, uint64_t *typep)
> return ofpprop_pull__(msg, property, 8, 0xffff, typep);
> }
>
> +/* Attempts to parse 'property' as a property containing a 16-bit value. If
> + * successful, stores the value into '*value' and returns 0; otherwise
> returns
> + * an OpenFlow error. */
> +enum ofperr
> +ofpprop_parse_be16(const struct ofpbuf *property, ovs_be16 *value)
> +{
> + /* OpenFlow uses 8-byte properties for 16-bit values, which doesn't
> really
> + * make sense. Be forgiving by allowing any size payload as long as it's
> + * at least big enough. */
> + ovs_be16 *p = property->msg;
> + if (ofpbuf_msgsize(property) < sizeof *p) {
> + return OFPERR_OFPBPC_BAD_LEN;
> + }
> + *value = *p;
> + return 0;
> +}
> +
> +/* Attempts to parse 'property' as a property containing a 32-bit value. If
> + * successful, stores the value into '*value' and returns 0; otherwise
> returns
> + * an OpenFlow error. */
> +enum ofperr
> +ofpprop_parse_be32(const struct ofpbuf *property, ovs_be32 *value)
> +{
> + ovs_be32 *p = property->msg;
> + if (ofpbuf_msgsize(property) != sizeof *p) {
> + return OFPERR_OFPBPC_BAD_LEN;
> + }
> + *value = *p;
> + return 0;
> +}
> +
> +/* Attempts to parse 'property' as a property containing a 16-bit value. If
> + * successful, stores the value into '*value' and returns 0; otherwise
> returns
> + * an OpenFlow error. */
> +enum ofperr
> +ofpprop_parse_u16(const struct ofpbuf *property, uint16_t *value)
> +{
> + /* OpenFlow uses 8-byte properties for 16-bit values, which doesn't
> really
> + * make sense. Be forgiving by allowing any size payload as long as it's
> + * at least big enough. */
> + ovs_be16 *p = property->msg;
> + if (ofpbuf_msgsize(property) < sizeof *p) {
> + return OFPERR_OFPBPC_BAD_LEN;
> + }
> + *value = ntohs(*p);
> + return 0;
> +}
> +
> +/* Attempts to parse 'property' as a property containing a 32-bit value. If
> + * successful, stores the value into '*value' and returns 0; otherwise
> returns
> + * an OpenFlow error. */
> +enum ofperr
> +ofpprop_parse_u32(const struct ofpbuf *property, uint32_t *value)
> +{
> + ovs_be32 *p = property->msg;
> + if (ofpbuf_msgsize(property) != sizeof *p) {
> + return OFPERR_OFPBPC_BAD_LEN;
> + }
> + *value = ntohl(*p);
> + return 0;
> +}
> +
> /* Adds a property with the given 'type' and 'len'-byte contents 'value' to
> * 'msg', padding the property out to a multiple of 8 bytes. */
> void
> @@ -117,6 +194,45 @@ ofpprop_put(struct ofpbuf *msg, uint64_t type, const
> void *value, size_t len)
> ofpprop_end(msg, start_ofs);
> }
>
> +/* Adds a property with the given 'type' and 16-bit 'value' to 'msg'. */
> +void
> +ofpprop_put_be16(struct ofpbuf *msg, uint64_t type, ovs_be16 value)
> +{
> + if (!ofpprop_is_experimenter(type)) {
> + /* The OpenFlow specs consistently (at least they're consistent!)
> give
> + * properties with a 16-bit integer value a length of 8, not 6, so
> add
> + * two bytes of padding. */
> + ovs_be16 padded_value[2] = { value, 0 };
> + ofpprop_put(msg, type, padded_value, sizeof padded_value);
> + } else {
> + /* There's no precedent but let's assume that this is generally done
> + * sanely. */
> + ofpprop_put(msg, type, &value, sizeof value);
> + }
> +}
> +
> +/* Adds a property with the given 'type' and 32-bit 'value' to 'msg'. */
> +void
> +ofpprop_put_be32(struct ofpbuf *msg, uint64_t type, ovs_be32 value)
> +{
> + ofpprop_put(msg, type, &value, sizeof value);
> +}
> +
> +/* Adds a property with the given 'type' and 16-bit 'value' to 'msg'. */
> +void
> +ofpprop_put_u16(struct ofpbuf *msg, uint64_t type, uint16_t value)
> +{
> + ofpprop_put_be16(msg, type, htons(value));
> +}
> +
> +/* Adds a property with the given 'type' and 32-bit 'value' to 'msg'. */
> +void
> +ofpprop_put_u32(struct ofpbuf *msg, uint64_t type, uint32_t value)
> +{
> + ofpprop_put_be32(msg, type, htonl(value));
> +}
> +
> +/* Adds a property header to 'msg' for each 1-bit in 'bitmap'. */
> /* Appends a property to 'msg' whose type is 'type' and whose contents is a
> * series of property headers, one for each 1-bit in 'bitmap'. */
> void
> diff --git a/lib/ofp-prop.h b/lib/ofp-prop.h
> index fc0c76d..2b4908a 100644
> --- a/lib/ofp-prop.h
> +++ b/lib/ofp-prop.h
> @@ -72,9 +72,18 @@ enum ofperr ofpprop_pull__(struct ofpbuf *msg, struct
> ofpbuf *property,
> enum ofperr ofpprop_pull(struct ofpbuf *msg, struct ofpbuf *property,
> uint64_t *typep);
>
> +enum ofperr ofpprop_parse_be16(const struct ofpbuf *, ovs_be16 *value);
> +enum ofperr ofpprop_parse_be32(const struct ofpbuf *, ovs_be32 *value);
> +enum ofperr ofpprop_parse_u16(const struct ofpbuf *, uint16_t *value);
> +enum ofperr ofpprop_parse_u32(const struct ofpbuf *, uint32_t *value);
> +
> /* Serializing properties. */
> void ofpprop_put(struct ofpbuf *, uint64_t type,
> const void *value, size_t len);
> +void ofpprop_put_be16(struct ofpbuf *, uint64_t type, ovs_be16 value);
> +void ofpprop_put_be32(struct ofpbuf *, uint64_t type, ovs_be32 value);
> +void ofpprop_put_u16(struct ofpbuf *, uint64_t type, uint16_t value);
> +void ofpprop_put_u32(struct ofpbuf *, uint64_t type, uint32_t value);
> void ofpprop_put_bitmap(struct ofpbuf *, uint64_t type, uint64_t bitmap);
>
> size_t ofpprop_start(struct ofpbuf *, uint64_t type);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 449f383..c813afd 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -4327,14 +4327,14 @@ static enum ofperr
> parse_port_mod_ethernet_property(struct ofpbuf *property,
> struct ofputil_port_mod *pm)
> {
> - struct ofp14_port_mod_prop_ethernet *eth = property->data;
> + ovs_be32 advertise;
> + enum ofperr error;
>
> - if (property->size != sizeof *eth) {
> - return OFPERR_OFPBRC_BAD_LEN;
> + error = ofpprop_parse_be32(property, &advertise);
> + if (!error) {
> + pm->advertise = netdev_port_features_from_ofp11(advertise);
> }
> -
> - pm->advertise = netdev_port_features_from_ofp11(eth->advertise);
> - return 0;
> + return error;
> }
>
> /* Decodes the OpenFlow "port mod" message in '*oh' into an abstract form in
> @@ -4457,10 +4457,9 @@ ofputil_encode_port_mod(const struct ofputil_port_mod
> *pm,
> }
> case OFP14_VERSION:
> case OFP15_VERSION: {
> - struct ofp14_port_mod_prop_ethernet *eth;
> struct ofp14_port_mod *opm;
>
> - b = ofpraw_alloc(OFPRAW_OFPT14_PORT_MOD, ofp_version, sizeof *eth);
> + b = ofpraw_alloc(OFPRAW_OFPT14_PORT_MOD, ofp_version, 0);
> opm = ofpbuf_put_zeros(b, sizeof *opm);
> opm->port_no = ofputil_port_to_ofp11(pm->port_no);
> opm->hw_addr = pm->hw_addr;
> @@ -4468,10 +4467,8 @@ ofputil_encode_port_mod(const struct ofputil_port_mod
> *pm,
> opm->mask = htonl(pm->mask & OFPPC11_ALL);
>
> if (pm->advertise) {
> - eth = ofpbuf_put_zeros(b, sizeof *eth);
> - eth->type = htons(OFPPMPT14_ETHERNET);
> - eth->length = htons(sizeof *eth);
> - eth->advertise = netdev_port_features_to_ofp11(pm->advertise);
> + ofpprop_put_be32(b, OFPPMPT14_ETHERNET,
> + netdev_port_features_to_ofp11(pm->advertise));
> }
> break;
> }
> @@ -4492,7 +4489,7 @@ pull_table_feature_property(struct ofpbuf *msg, struct
> ofpbuf *payload,
>
> error = ofpprop_pull(msg, payload, typep);
> if (payload && !error) {
> - ofpbuf_pull(payload, (uint8_t *)msg->msg - (uint8_t *)msg->header);
> + ofpbuf_pull(payload, (char *)payload->msg - (char *)payload->data);
Why not ‘payload->header’?
> }
> return error;
> }
> @@ -4889,20 +4886,6 @@ ofputil_append_table_features_reply(const struct
> ofputil_table_features *tf,
> }
(snip)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
