I have noticed in patchwork that this patch has been set to "Not Applicable" http://patchwork.ozlabs.org/patch/630624/
What does that mean? Is there anything I did wrong? How should I fix this? I checked today and the patch still applies on master except for the NEWS file. I can submit a new version that fixes this. Thanks, Jan > -----Original Message----- > From: Jan Scheurich [mailto:jan.scheur...@web.de] > Sent: Monday, 06 June, 2016 00:31 > To: dev@openvswitch.org > Cc: jan.scheur...@web.de > Subject: [PATCH v2]: ofproto: Add relaxed group_mod command > ADD_OR_MOD > > This patch adds support for a new Group Mod command > OFPGC_ADD_OR_MOD to OVS for all OpenFlow versions that support groups > (OF11 and higher). The new ADD_OR_MOD creates a group that does not yet > exist (like ADD) and modifies an existing group (like MODIFY). > > Rational: In OpenFlow 1.x the Group Mod commands OFPGC_ADD and > OFPGC_MODIFY have strict semantics: ADD fails if the group exists, while > MODIFY fails if the group does not exist. This requires a controller to > exactly > know the state of the switch when programming a group in order not run the > risk of getting an OFP Error message in response. This is hard to achieve and > maintain at all times in view of possible switch and controller restarts or > other > connection losses between switch and controller. > > Due to the un-acknowledged nature of the Group Mod message > programming groups safely and efficiently at the same time is virtually > impossible as the controller has to either query the existence of the group > prior to each Group Mod message or to insert a Barrier Request/Reply after > every group to be sure that no Error can be received at a later stage and > require a complicated roll-back of any dependent actions taken between the > failed Group Mod and the Error. > > In the ovs-ofctl command line the ADD_OR_MOD command is made > available through the new option --may-create in the mod-group command: > > $ ovs-ofctl -Oopenflow13 del-groups br-int group_id=100 > > $ ovs-ofctl -Oopenflow13 mod-group br-int > group_id=100,type=indirect,bucket=actions=2 > OFPT_ERROR (OF1.3) (xid=0x2): OFPGMFC_UNKNOWN_GROUP > OFPT_GROUP_MOD (OF1.3) (xid=0x2): > MOD group_id=100,type=indirect,bucket=actions=output:2 > > $ ovs-ofctl -Oopenflow13 --may-create mod-group br-int > group_id=100,type=indirect,bucket=actions=2 > > $ ovs-ofctl -Oopenflow13 dump-groups br-int OFPST_GROUP_DESC reply > (OF1.3) (xid=0x2): > group_id=100,type=indirect,bucket=actions=output:2 > > $ ovs-ofctl -Oopenflow13 --may-create mod-group br-int > group_id=100,type=indirect,bucket=actions=3 > > $ ovs-ofctl -Oopenflow13 dump-groups br-int OFPST_GROUP_DESC reply > (OF1.3) (xid=0x2): > group_id=100,type=indirect,bucket=actions=output:3 > > > Patch v2: > - Replaced new ovs-ofctl write-group command with --may-create option for > mod-group > - Updated ovs-ofctl --help message > - Added a test for the new command option > - Updated documentation > > Signed-off-by: Jan Scheurich <jan.scheurich at web.de> > > To aid review this patch is available at: > tree: https://github.com/JScheurich/openvswitch > branch: dev/group_add_modify > tag: group_add_modify_v2 > > --- > > NEWS | 3 +++ > include/openflow/openflow-1.1.h | 1 + > include/openflow/openflow-1.5.h | 1 + > lib/ofp-parse.c | 4 ++++ > lib/ofp-print.c | 4 ++++ > lib/ofp-util.c | 7 ++++++- > ofproto/ofproto.c | 25 +++++++++++++++++++++++++ > tests/ofproto.at | 26 ++++++++++++++++++++++++++ > utilities/ovs-ofctl.8.in | 12 +++++++++--- > utilities/ovs-ofctl.c | 14 +++++++++++++- > 10 files changed, 92 insertions(+), 5 deletions(-) > > > diff --git a/NEWS b/NEWS > index ba201cf..74bda47 100644 > --- a/NEWS > +++ b/NEWS > @@ -15,10 +15,13 @@ Post-v2.5.0 > now implemented. Only flow mod and port mod messages are > supported > in bundles. > * New OpenFlow extension NXM_NX_MPLS_TTL to provide access to > MPLS TTL. > + * New command OFPGC_ADD_OR_MOD for OFPT_GROUP_MOD > message that adds a > + new group or modifies an existing groups > - ovs-ofctl: > * queue-get-config command now allows a queue ID to be specified. > * '--bundle' option can now be used with OpenFlow 1.3. > * New option "--color" to produce colorized output for some commands. > + * '--may-create' option to use OFPGC_ADD_OR_MOD in mod-group > command. > - DPDK: > * New option "n_rxq" for PMD interfaces. > Old 'other_config:n-dpdk-rxqs' is no longer supported. > diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow- > 1.1.h index 805f222..de28475 100644 > --- a/include/openflow/openflow-1.1.h > +++ b/include/openflow/openflow-1.1.h > @@ -172,6 +172,7 @@ enum ofp11_group_mod_command { > OFPGC11_ADD, /* New group. */ > OFPGC11_MODIFY, /* Modify all matching groups. */ > OFPGC11_DELETE, /* Delete all matching groups. */ > + OFPGC11_ADD_OR_MOD = 0x8000, /* Create new or modify existing > + group. */ > }; > > /* OpenFlow 1.1 specific capabilities supported by the datapath (struct diff > -- > git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h > index 0c478d1..3649e6c 100644 > --- a/include/openflow/openflow-1.5.h > +++ b/include/openflow/openflow-1.5.h > @@ -59,6 +59,7 @@ enum ofp15_group_mod_command { > /* OFPGCXX_YYY = 4, */ /* Reserved for future use. */ > OFPGC15_REMOVE_BUCKET = 5,/* Remove all action buckets or any > specific > action bucket from matching group */ > + OFPGC15_ADD_OR_MOD = 0x8000, /* Create new or modify existing > + group. */ > }; > > /* Group bucket property types. */ > diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 71c5cdf..4af6d9b 100644 > --- a/lib/ofp-parse.c > +++ b/lib/ofp-parse.c > @@ -1380,6 +1380,10 @@ parse_ofp_group_mod_str__(struct > ofputil_group_mod *gm, uint16_t > fields = F_GROUP_TYPE | F_BUCKETS; > break; > > + case OFPGC11_ADD_OR_MOD: > + fields = F_GROUP_TYPE | F_BUCKETS; > + break; > + > case OFPGC15_INSERT_BUCKET: > fields = F_BUCKETS | F_COMMAND_BUCKET_ID; > *usable_protocols &= OFPUTIL_P_OF15_UP; diff --git a/lib/ofp-print.c > b/lib/ofp-print.c index b21d76f..ce2eecb 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -2673,6 +2673,10 @@ ofp_print_group_mod__(struct ds *s, enum > ofp_version ofp_version, > ds_put_cstr(s, "MOD"); > break; > > + case OFPGC11_ADD_OR_MOD: > + ds_put_cstr(s, "ADD_OR_MOD"); > + break; > + > case OFPGC11_DELETE: > ds_put_cstr(s, "DEL"); > break; > diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 2c6fb1f..83685ca 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -8787,6 +8787,7 @@ parse_group_prop_ntr_selection_method(struct > ofpbuf *payload, > switch (group_cmd) { > case OFPGC15_ADD: > case OFPGC15_MODIFY: > + case OFPGC15_ADD_OR_MOD: > break; > case OFPGC15_DELETE: > case OFPGC15_INSERT_BUCKET: > @@ -9115,6 +9116,7 @@ bad_group_cmd(enum > ofp15_group_mod_command cmd) > switch (cmd) { > case OFPGC15_ADD: > case OFPGC15_MODIFY: > + case OFPGC15_ADD_OR_MOD: > case OFPGC15_DELETE: > version = "1.1"; > opt_version = "11"; > @@ -9136,6 +9138,7 @@ bad_group_cmd(enum > ofp15_group_mod_command cmd) > break; > > case OFPGC15_MODIFY: > + case OFPGC15_ADD_OR_MOD: > cmd_str = "mod-group"; > break; > > @@ -9175,7 +9178,7 @@ ofputil_encode_group_mod(enum ofp_version > ofp_version, > case OFP12_VERSION: > case OFP13_VERSION: > case OFP14_VERSION: > - if (gm->command > OFPGC11_DELETE) { > + if (gm->command > OFPGC11_DELETE && gm->command != > + OFPGC11_ADD_OR_MOD) { > bad_group_cmd(gm->command); > } > return ofputil_encode_ofp11_group_mod(ofp_version, gm); @@ - > 9246,6 +9249,7 @@ ofputil_pull_ofp15_group_mod(struct ofpbuf *msg, > enum ofp_version > > case OFPGC11_ADD: > case OFPGC11_MODIFY: > + case OFPGC11_ADD_OR_MOD: > case OFPGC11_DELETE: > default: > if (gm->command_bucket_id == OFPG15_BUCKET_ALL) { @@ -9324,6 > +9328,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh, > switch (gm->command) { > case OFPGC11_ADD: > case OFPGC11_MODIFY: > + case OFPGC11_ADD_OR_MOD: > case OFPGC11_DELETE: > case OFPGC15_INSERT_BUCKET: > break; > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 835a397..6321756 > 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -6634,6 +6634,27 @@ out: > return error; > } > > +/* Implements the OFPGC11_ADD_OR_MOD command which creates the > group > +when it does not > + * exist yet and modifies it otherwise */ static enum ofperr > +add_or_modify_group(struct ofproto *ofproto, const struct > +ofputil_group_mod *gm) { > + struct ofgroup *ofgroup; > + enum ofperr error; > + bool exists; > + > + ovs_rwlock_rdlock(&ofproto->groups_rwlock); > + exists = ofproto_group_lookup__(ofproto, gm->group_id, &ofgroup); > + ovs_rwlock_unlock(&ofproto->groups_rwlock); > + > + if (!exists) { > + error = add_group(ofproto, gm); > + } else { > + error = modify_group(ofproto, gm); > + } > + return error; > +} > + > static void > delete_group__(struct ofproto *ofproto, struct ofgroup *ofgroup) > OVS_RELEASES(ofproto->groups_rwlock) > @@ -6723,6 +6744,10 @@ handle_group_mod(struct ofconn *ofconn, const > struct ofp_header * > error = modify_group(ofproto, &gm); > break; > > + case OFPGC11_ADD_OR_MOD: > + error = add_or_modify_group(ofproto, &gm); > + break; > + > case OFPGC11_DELETE: > delete_group(ofproto, gm.group_id); > error = 0; > diff --git a/tests/ofproto.at b/tests/ofproto.at index f8c0d07..57ca50a 100644 > --- a/tests/ofproto.at > +++ b/tests/ofproto.at > @@ -393,6 +393,32 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add- > group br0 'group_id=12 OVS_VSWITCHD_STOP AT_CLEANUP > > +AT_SETUP([ofproto - group_mod with mod and add_or_mod command]) > +OVS_VSWITCHD_START dnl Check that mod-group for non-existing group > +fails without --may-create AT_DATA([stderr], [dnl OFPT_ERROR (OF1.3) > +(xid=0x2): OFPGMFC_UNKNOWN_GROUP OFPT_GROUP_MOD (OF1.3) > (xid=0x2): > + MOD group_id=1234,type=indirect,bucket=actions=output:2 > +]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn mod-group br0 > +'group_id=1234,type=indirect,buc dnl Check that mod-group for > +non-existing group succeeds with --may-create AT_CHECK([ovs-ofctl -O > +OpenFlow13 -vwarn --may-create mod-group br0 'group_id=1234,type > +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn dump-groups br0], [0], > +[stdout]) AT_CHECK([strip_xids < stdout], [0], [dnl OFPST_GROUP_DESC > reply (OF1.3): > + group_id=1234,type=indirect,bucket=actions=output:2 > +]) > +dnl Check that mod-group for existing group succeeds with --may-create > +AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn --may-create mod-group br0 > +'group_id=1234,type AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn > +dump-groups br0], [0], [stdout]) AT_CHECK([strip_xids < stdout], [0], > +[dnl OFPST_GROUP_DESC reply (OF1.3): > + group_id=1234,type=indirect,bucket=actions=output:3 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > dnl This is really bare-bones. > dnl It at least checks request and reply serialization and deserialization. > dnl Actions definition listed in both supported formats (w/ actions=) diff > --git > a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in old mode 100644 new mode > 100755 index e2e26f7..843480c > --- a/utilities/ovs-ofctl.8.in > +++ b/utilities/ovs-ofctl.8.in > @@ -411,10 +411,10 @@ zero or more groups in the same syntax, one per > line. > .IQ "\fBadd\-groups \fIswitch file\fR" > Add each group entry to \fIswitch\fR's tables. > . > -.IP "\fBmod\-group \fIswitch group\fR" > -.IQ "\fBmod\-group \fIswitch \fB\- < \fIfile\fR" > +.IP "[\fB\-\-may\-create\fR] \fBmod\-group \fIswitch group\fR" > +.IQ "[\fB\-\-may\-create\fR] \fBmod\-group \fIswitch \fB\- < \fIfile\fR" > Modify the action buckets in entries from \fIswitch\fR's tables for -each > group entry. > +each group entry. Optionally create non-existing group entries. > . > .IP "\fBdel\-groups \fIswitch\fR" > .IQ "\fBdel\-groups \fIswitch \fR[\fIgroup\fR]" > @@ -2911,8 +2911,14 @@ Bundles require OpenFlow 1.4 or higher. An > explicit \fB-O OpenFlow14\fR option is not needed, but you may need to > enable OpenFlow 1.4 support for OVS by setting the OVSDB \fIprotocols\fR > column in the \fIbridge\fR table. > +. > .RE > . > +.IP "\fB\-\-may\-create\fR" > +A mod-group command creates a group if it doesn't exist yet. This uses > +an Open vSwitch extension to OpenFlow and only works with Open vSwitch > +2.6 and later. > +. > .so lib/ofp-version.man > . > .IP "\fB\-F \fIformat\fR[\fB,\fIformat\fR...]" > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index > a8116d9..5b90a82 > 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -84,6 +84,10 @@ static bool enable_color; > */ > static bool strict; > > +/* --may-create: A mod-group command creates a group that does not yet > exist. > + */ > +static bool may_create = false; > + > /* --readd: If true, on replace-flows, re-add even flows that have not > changed > * (to reset flow counters). */ > static bool readd; > @@ -175,6 +179,7 @@ parse_options(int argc, char *argv[]) > OPT_UNIXCTL, > OPT_BUNDLE, > OPT_COLOR, > + OPT_MAY_CREATE, > DAEMON_OPTION_ENUMS, > OFP_VERSION_OPTION_ENUMS, > VLOG_OPTION_ENUMS > @@ -194,6 +199,7 @@ parse_options(int argc, char *argv[]) > {"option", no_argument, NULL, 'o'}, > {"bundle", no_argument, NULL, OPT_BUNDLE}, > {"color", optional_argument, NULL, OPT_COLOR}, > + {"may-create", no_argument, NULL, OPT_MAY_CREATE}, > DAEMON_LONG_OPTIONS, > OFP_VERSION_LONG_OPTIONS, > VLOG_LONG_OPTIONS, > @@ -319,6 +325,10 @@ parse_options(int argc, char *argv[]) > } > break; > > + case OPT_MAY_CREATE: > + may_create = true; > + break; > + > DAEMON_OPTION_HANDLERS > OFP_VERSION_OPTION_HANDLERS > VLOG_OPTION_HANDLERS > @@ -440,6 +450,7 @@ usage(void) > vlog_usage(); > printf("\nOther options:\n" > " --strict use strict match for flow > commands\n" > + " --may-create mod-group creates a non-existing > group\n" > " --readd replace flows that haven't > changed\n" > " -F, --flow-format=FORMAT force particular flow format\n" > " -P, --packet-in-format=FRMT force particular packet in > format\n" > @@ -2520,7 +2531,8 @@ ofctl_add_groups(struct ovs_cmdl_context *ctx) > static void ofctl_mod_group(struct ovs_cmdl_context *ctx) { > - ofctl_group_mod(ctx->argc, ctx->argv, OFPGC11_MODIFY); > + ofctl_group_mod(ctx->argc, ctx->argv, > + may_create ? OFPGC11_ADD_OR_MOD : OFPGC11_MODIFY); > } > > static void _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
