Acked-by: Jarno Rajahalme <ja...@ovn.org> > On Jan 18, 2016, at 11:27 PM, Ben Pfaff <b...@ovn.org> wrote: > > The OF1.0 through OF1.3 "set async config" set the whole configuration, > OF1.4+ only update parts of it piecemeal, but the decoding function always > set the whole configuration. This commit fixes the problem by changing the > interface to require the caller to provide an initial state. (It would > be possible to simply make it mutate existing state in-place, but that > interface seems a little more error-prone.) > > Found by inspection. > > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > OPENFLOW-1.1+.md | 5 +---- > lib/ofp-print.c | 7 +++++-- > lib/ofp-util.c | 8 ++++++++ > lib/ofp-util.h | 1 + > ofproto/ofproto.c | 5 +++-- > 5 files changed, 18 insertions(+), 8 deletions(-) > > diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md > index 537f660..62ebddc 100644 > --- a/OPENFLOW-1.1+.md > +++ b/OPENFLOW-1.1+.md > @@ -192,10 +192,7 @@ OpenFlow 1.4 features are listed in the previous section. > > * More extensible wire protocol > Many on-wire structures got TLVs. > - Already implemented: port desc properties, port mod properties, > - port stats properties, table mod properties, > - queue stats, unified property errors, queue desc. > - Remaining required: set-async > + All required features are now supported. > Remaining optional: table desc, table-status > [EXT-262] > [required for OF1.4+] > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index accde8e..f36335b 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -2130,9 +2130,12 @@ ofp_print_nxt_set_async_config(struct ds *string, > } > } else if (raw == OFPRAW_OFPT14_SET_ASYNC || > raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { > - struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT; > + struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT; > + struct ofputil_async_cfg ac; > + > bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY; > - enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, > &ac); > + enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, > + &basis, &ac); > if (error) { > ofp_print_error(string, error); > return; > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 19d3775..5bb0b74 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -9570,6 +9570,11 @@ decode_legacy_async_masks(const ovs_be32 masks[2], > /* Decodes the OpenFlow "set async config" request and "get async config > * reply" message in '*oh' into an abstract form in 'ac'. > * > + * Some versions of the "set async config" request change only some of the > + * settings and leave the others alone. This function uses 'basis' as the > + * initial state for decoding these. Other versions of the request change > all > + * the settings; this function ignores 'basis' when decoding these. > + * > * If 'loose' is true, this function ignores properties and values that it > does > * not understand, as a controller would want to do when interpreting > * capabilities provided by a switch. If 'loose' is false, this function > @@ -9585,6 +9590,7 @@ decode_legacy_async_masks(const ovs_be32 masks[2], > * supported.*/ > enum ofperr > ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, > + const struct ofputil_async_cfg *basis, > struct ofputil_async_cfg *ac) > { > enum ofpraw raw; > @@ -9598,6 +9604,7 @@ ofputil_decode_set_async_config(const struct ofp_header > *oh, bool loose, > raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) { > const struct nx_async_config *msg = ofpmsg_body(oh); > > + *ac = OFPUTIL_ASYNC_CFG_INIT; > decode_legacy_async_masks(msg->packet_in_mask, OAM_PACKET_IN, > oh->version, ac); > decode_legacy_async_masks(msg->port_status_mask, OAM_PORT_STATUS, > @@ -9606,6 +9613,7 @@ ofputil_decode_set_async_config(const struct ofp_header > *oh, bool loose, > oh->version, ac); > } else if (raw == OFPRAW_OFPT14_SET_ASYNC || > raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) { > + *ac = *basis; > while (b.size > 0) { > struct ofpbuf property; > enum ofperr error; > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 52268d8..88c67f9 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -1317,6 +1317,7 @@ struct ofputil_async_cfg { > > enum ofperr ofputil_decode_set_async_config(const struct ofp_header *, > bool loose, > + const struct ofputil_async_cfg *, > struct ofputil_async_cfg *); > > struct ofpbuf *ofputil_encode_get_async_config( > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index fbaf7dd..957e323 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -5403,10 +5403,11 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn, > static enum ofperr > handle_nxt_set_async_config(struct ofconn *ofconn, const struct ofp_header > *oh) > { > - struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT; > + struct ofputil_async_cfg basis = ofconn_get_async_config(ofconn); > + struct ofputil_async_cfg ac; > enum ofperr error; > > - error = ofputil_decode_set_async_config(oh, false, &ac); > + error = ofputil_decode_set_async_config(oh, false, &basis, &ac); > if (error) { > return error; > } > -- > 2.1.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev