[ovs-dev] [PATCH v2] ofproto: add support of OFPR_GROUP as packet-in reason for OF1.4+

[Ben Pfaff]

On Thu, Dec 04, 2014 at 12:52:41PM -0800, Shu Shen wrote:
> When the Packet-In message is triggered by a group action, OFPR_GROUP is
> passed internally as the reason. The wire_reason() function converts the
> reason to OFPR_ACTION if the wire protocol is earlier than OF1.4.
> 
> The wire_reason() function also converts other unsupported reasons
> (i.e., OFPR_ACTION_SET and OFPR_PACKET_OUT) to OFPR_ACTION if it detects
> a wire protocol earlier than OF1.4.
> 
> By default reason code OFPR_GROUP for Packet-In will be enabled for
> async messages as in ofconn_flush(). Upon a connection being established
> with a controller, the protocol version is checked and OFPR_GROUP will
> be disabled in async config if the protocol is lower than OF1.4. Any
> controller running OF1.4+ is still be able to enable OFPR_GROUP at its
> will without being affected by this check.
> 
> The patch also includes tests cases for both OF1.3 and OF1.4 to ensure
> proper reason code is given for packet-in message triggered by group
> action.
> 
> Signed-off-by: Shu Shen <shu.shen at radisys.com>

Thanks.  We're getting closer to OF1.4 support!

I applied this to master, with the following stylistic adjustments:

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 25a52fc..75f4198 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1671,28 +1671,27 @@ wire_reason(struct ofconn *ofconn, const struct ofproto_packet_in *pin)
     enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
 
     if (pin->miss_type == OFPROTO_PACKET_IN_MISS_FLOW
-        && pin->up.reason == OFPR_ACTION) {
-
-        if (protocol != OFPUTIL_P_NONE
-            && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
-            return OFPR_NO_MATCH;
-        }
+        && pin->up.reason == OFPR_ACTION
+        && protocol != OFPUTIL_P_NONE
+        && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
+        return OFPR_NO_MATCH;
     }
 
     switch (pin->up.reason) {
-        case OFPR_ACTION_SET:
-        case OFPR_GROUP:
-        case OFPR_PACKET_OUT:
-           if (!(protocol & OFPUTIL_P_OF14_UP)) {
-                /* Only supported in OF1.4+ */
-                return OFPR_ACTION;
-           } /* else fall through */
+    case OFPR_ACTION_SET:
+    case OFPR_GROUP:
+    case OFPR_PACKET_OUT:
+        if (!(protocol & OFPUTIL_P_OF14_UP)) {
+            /* Only supported in OF1.4+ */
+            return OFPR_ACTION;
+        }
+        /* Fall through. */
 	case OFPR_NO_MATCH:
 	case OFPR_ACTION:
 	case OFPR_INVALID_TTL:
 	case OFPR_N_REASONS:
-        default:
-           return pin->up.reason;
+    default:
+        return pin->up.reason;
     }
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 52abfa0..9f0fdce 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3405,7 +3405,8 @@ xlate_output_action(struct xlate_ctx *ctx,
         flood_packets(ctx, true);
         break;
     case OFPP_CONTROLLER:
-        execute_controller_action(ctx, max_len, ctx->in_group?OFPR_GROUP:OFPR_ACTION, 0);
+        execute_controller_action(ctx, max_len,
+                                  ctx->in_group ? OFPR_GROUP : OFPR_ACTION, 0);
         break;
     case OFPP_NONE:
         break;