This patch looks good, though it looks like the general issue with examining the Actions within a Write Actions instruction has several instances beyond what is addressed by this patch.
I have noticed that an invalid Group ID within Write Actions is not caught (Flow is programmed with no error). That appears to be a problem in ofproto.c -> ofproto_check_ofpacts. A similar fix seemed to address that issue. I did a global search for OFPACT_FOR_EACH and examined the action processing. I see several other potential problems with Write Actions. ofp-actions.c -> ofpacts_check ofp-actions.c -> ofpacts_get_meter ofproto.c -> ofproto_check_ofpacts Thanks for your help. From: Ben Pfaff <b...@nicira.com> To: dev@openvswitch.org Cc: Ben Pfaff <b...@nicira.com>, Gavin Remaley <gavin_rema...@selinc.com> Date: 09/08/2015 04:55 PM Subject: [PATCH 1/3] ofp-actions: Look inside write_actions for output ports and groups. The out_port and out_group matches only looked at apply_actions instructions, but my interpretation of the OpenFlow spec is that they should also look inside write_actions. Reported-by: Gavin Remaley <gavin_rema...@selinc.com> Signed-off-by: Ben Pfaff <b...@nicira.com> --- AUTHORS | 1 + lib/ofp-actions.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/AUTHORS b/AUTHORS index a7f40bb..0c4d020 100644 --- a/AUTHORS +++ b/AUTHORS @@ -259,6 +259,7 @@ Eivind Bulie Haanaes Eric Lopez elo...@nicira.com Frido Roose fr.ro...@gmail.com Gaetano Catalli gaetano.cata...@gmail.com +Gavin Remaley gavin_rema...@selinc.com George Shuklin ama...@desunote.ru Gerald Rogers gerald.rog...@intel.com Ghanem Bahri bahri.gha...@gmail.com diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index c46ce97..ee686c1 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -6072,6 +6072,12 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) return ofpact_get_ENQUEUE(ofpact)->port == port; case OFPACT_CONTROLLER: return port == OFPP_CONTROLLER; + case OFPACT_WRITE_ACTIONS: { + const struct ofpact_nest *nested = ofpact_get_WRITE_ACTIONS(ofpact); + return ofpacts_output_to_port(nested->actions, + ofpact_nest_get_action_len(nested), + port); + } case OFPACT_OUTPUT_REG: case OFPACT_BUNDLE: @@ -6113,7 +6119,6 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, ofp_port_t port) case OFPACT_POP_MPLS: case OFPACT_SAMPLE: case OFPACT_CLEAR_ACTIONS: - case OFPACT_WRITE_ACTIONS: case OFPACT_GOTO_TABLE: case OFPACT_METER: case OFPACT_GROUP: @@ -6149,9 +6154,17 @@ ofpacts_output_to_group(const struct ofpact *ofpacts, size_t ofpacts_len, const struct ofpact *a; OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { - if (a->type == OFPACT_GROUP - && ofpact_get_GROUP(a)->group_id == group_id) { - return true; + if (a->type == OFPACT_GROUP) { + if (ofpact_get_GROUP(a)->group_id == group_id) { + return true; + } + } else if (a->type == OFPACT_WRITE_ACTIONS) { + const struct ofpact_nest *nested = ofpact_get_WRITE_ACTIONS(a); + if (ofpacts_output_to_group(nested->actions, + ofpact_nest_get_action_len(nested), + group_id)) { + return true; + } } } -- 2.1.3 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev