Skip to content

Commit

Permalink
ofp-util: Fix OF1.4+ version of ofputil_decode_set_async_config().
Browse files Browse the repository at this point in the history
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 <[email protected]>
Acked-by: Jarno Rajahalme <[email protected]>
  • Loading branch information
blp committed Jan 20, 2016
1 parent 8fd0bb6 commit 5876b4d
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 8 deletions.
5 changes: 1 addition & 4 deletions OPENFLOW-1.1+.md
Original file line number Diff line number Diff line change
Expand Up @@ -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+]
Expand Down
7 changes: 5 additions & 2 deletions lib/ofp-print.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions lib/ofp-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -9572,6 +9572,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
Expand All @@ -9587,6 +9592,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;
Expand All @@ -9600,6 +9606,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,
Expand All @@ -9608,6 +9615,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;
Expand Down
1 change: 1 addition & 0 deletions lib/ofp-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 3 additions & 2 deletions ofproto/ofproto.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 5876b4d

Please sign in to comment.