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

Shu Shen shu.shen at radisys.com
Thu Nov 13 17:56:57 UTC 2014


Use wire_reason() to translate between different OF versions.
OF1.3 and earlier will still use OFPR_ACTION instead of
OFPR_GROUP.

Async config is initiated and checked againt OF version to
make sure OFPR_GROUP is only reported for OF1.4+. Controllers
running OF1.3+ cannot override the mask and enable OFPR_GROUP
reporting.

Signed-off-by: Shu Shen <shu.shen at radisys.com>
---
 DESIGN.md                    |  1 +
 OPENFLOW-1.1+.md             |  1 +
 lib/ofp-msgs.h               |  2 +-
 lib/ofp-util.c               |  4 +-
 ofproto/connmgr.c            | 41 +++++++++++++++++++--
 ofproto/ofproto-dpif-xlate.c |  2 +-
 tests/ofproto-dpif.at        | 88 ++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 131 insertions(+), 8 deletions(-)

diff --git a/DESIGN.md b/DESIGN.md
index bd0ed27..ff5bdf4 100644
--- a/DESIGN.md
+++ b/DESIGN.md
@@ -54,6 +54,7 @@ sent, an entry labeled "---" means that the message is suppressed.
     OFPR_NO_MATCH                              yes       ---
     OFPR_ACTION                                yes       ---
     OFPR_INVALID_TTL                           ---       ---
+    OFPR_GROUP (OF1.4+)                        yes       ---
 
   OFPT_FLOW_REMOVED / NXT_FLOW_REMOVED
     OFPRR_IDLE_TIMEOUT                         yes       ---
diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index 07f7f86..96c25b9 100644
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -180,6 +180,7 @@ OpenFlow 1.4
   * More descriptive reasons for packet-in
     Distinguish OFPR_APPLY_ACTION, OFPR_ACTION_SET, OFPR_GROUP,
     OFPR_PACKET_OUT.  NO_MATCH was renamed to OFPR_TABLE_MISS.
+    (OFPR_GROUP is now supported)
     [EXT-136]
     [required for OF1.4+]
 
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index fc69586..ad6dc6f 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -148,7 +148,7 @@ enum ofpraw {
     OFPRAW_OFPT11_PACKET_IN,
     /* OFPT 1.2 (10): struct ofp12_packet_in, uint8_t[]. */
     OFPRAW_OFPT12_PACKET_IN,
-    /* OFPT 1.3 (10): struct ofp13_packet_in, uint8_t[]. */
+    /* OFPT 1.3+ (10): struct ofp13_packet_in, uint8_t[]. */
     OFPRAW_OFPT13_PACKET_IN,
     /* NXT 1.0+ (17): struct nx_packet_in, uint8_t[]. */
     OFPRAW_NXT_PACKET_IN,
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 94047fa..86cf8a1 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3527,7 +3527,7 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
         packet_in_size = sizeof (struct ofp12_packet_in);
     } else {
         packet_in_raw = OFPRAW_OFPT13_PACKET_IN;
-        packet_in_version = OFP13_VERSION;
+        packet_in_version = ofputil_protocol_to_ofp_version(protocol);
         packet_in_size = sizeof (struct ofp13_packet_in);
     }
 
@@ -3547,7 +3547,7 @@ ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
     opi->pi.total_len = htons(pin->total_len);
     opi->pi.reason = pin->reason;
     opi->pi.table_id = pin->table_id;
-    if (protocol == OFPUTIL_P_OF13_OXM) {
+    if (protocol & OFPUTIL_P_OF13_UP) {
         opi->cookie = pin->cookie;
     }
 
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 627f326..25a52fc 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -996,6 +996,14 @@ void
 ofconn_set_protocol(struct ofconn *ofconn, enum ofputil_protocol protocol)
 {
     ofconn->protocol = protocol;
+    if (!(protocol & OFPUTIL_P_OF14_UP)) {
+        uint32_t *master = ofconn->master_async_config;
+        uint32_t *slave = ofconn->slave_async_config;
+
+        /* OFPR_GROUP is not supported before OF1.4 */
+        master[OAM_PACKET_IN] &= ~(1u << OFPR_GROUP);
+        slave [OAM_PACKET_IN] &= ~(1u << OFPR_GROUP);
+    }
 }
 
 /* Returns the currently configured packet in format for 'ofconn', one of
@@ -1056,6 +1064,13 @@ ofconn_get_async_config(struct ofconn *ofconn,
                         uint32_t *master_masks, uint32_t *slave_masks)
 {
     size_t size = sizeof ofconn->master_async_config;
+
+    /* Make sure we know the protocol version and the async_config
+     * masks are properly updated by calling ofconn_get_protocol() */
+    if (OFPUTIL_P_NONE == ofconn_get_protocol(ofconn)){
+        OVS_NOT_REACHED();
+    }
+
     memcpy(master_masks, ofconn->master_async_config, size);
     memcpy(slave_masks, ofconn->slave_async_config, size);
 }
@@ -1235,7 +1250,9 @@ ofconn_flush(struct ofconn *ofconn)
         /* "master" and "other" roles get all asynchronous messages by default,
          * except that the controller needs to enable nonstandard "packet-in"
          * reasons itself. */
-        master[OAM_PACKET_IN] = (1u << OFPR_NO_MATCH) | (1u << OFPR_ACTION);
+        master[OAM_PACKET_IN] = ((1u << OFPR_NO_MATCH)
+                                 | (1u << OFPR_ACTION)
+                                 | (1u << OFPR_GROUP));
         master[OAM_PORT_STATUS] = ((1u << OFPPR_ADD)
                                    | (1u << OFPPR_DELETE)
                                    | (1u << OFPPR_MODIFY));
@@ -1651,16 +1668,32 @@ connmgr_send_flow_removed(struct connmgr *mgr,
 static enum ofp_packet_in_reason
 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) {
-        enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
 
         if (protocol != OFPUTIL_P_NONE
             && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
             return OFPR_NO_MATCH;
         }
     }
-    return pin->up.reason;
+
+    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_NO_MATCH:
+	case OFPR_ACTION:
+	case OFPR_INVALID_TTL:
+	case OFPR_N_REASONS:
+        default:
+           return pin->up.reason;
+    }
 }
 
 /* Given 'pin', sends an OFPT_PACKET_IN message to each OpenFlow controller as
@@ -1677,7 +1710,7 @@ connmgr_send_packet_in(struct connmgr *mgr,
         enum ofp_packet_in_reason reason = wire_reason(ofconn, pin);
 
         if (ofconn_wants_packet_in_on_miss(ofconn, pin)
-            && ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason)
+            && ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
             && ofconn->controller_id == pin->controller_id) {
             schedule_packet_in(ofconn, *pin, reason);
         }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f781bc5..c573c80 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3397,7 +3397,7 @@ xlate_output_action(struct xlate_ctx *ctx,
         flood_packets(ctx, true);
         break;
     case OFPP_CONTROLLER:
-        execute_controller_action(ctx, max_len, OFPR_ACTION, 0);
+        execute_controller_action(ctx, max_len, ctx->in_group?OFPR_GROUP:OFPR_ACTION, 0);
         break;
     case OFPP_NONE:
         break;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 5349386..33be9d0 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2652,6 +2652,94 @@ OFPST_FLOW reply (OF1.3):
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+
+AT_SETUP([ofproto-dpif - packet-in reason in group table (Openflow 1.3)])
+OVS_VSWITCHD_START([dnl
+   add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+# A table-miss flow has priority 0 and no match
+AT_CHECK([ovs-ofctl --protocols=OpenFlow13 add-group br0 'group_id=1234,type=all,bucket=output:10,bucket=output:CONTROLLER'])
+AT_CHECK([ovs-ofctl --protocols=OpenFlow13 add-flow br0 'in_port=1 actions=group:1234'])
+
+dnl Singleton controller action.
+AT_CHECK([ovs-ofctl monitor -P openflow10 --protocols=OpenFlow13 br0 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3 ; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=10:11:11:11:11:11,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=10),tcp_flags(0x002)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via action) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([ovs-ofctl --protocols=OpenFlow13 dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ n_packets=3, n_bytes=180, in_port=1 actions=group:1234
+OFPST_FLOW reply (OF1.3):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([ofproto-dpif - packet-in reason in group table (Openflow 1.4)])
+OVS_VSWITCHD_START([dnl
+   add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+# A table-miss flow has priority 0 and no match
+AT_CHECK([ovs-ofctl --protocols=OpenFlow14 add-group br0 'group_id=1234,type=all,bucket=output:10,bucket=output:CONTROLLER'])
+AT_CHECK([ovs-ofctl --protocols=OpenFlow14 add-flow br0 'in_port=1 actions=group:1234'])
+
+dnl Singleton controller action.
+AT_CHECK([ovs-ofctl monitor -P openflow10 --protocols=OpenFlow14 br0 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log])
+
+for i in 1 2 3 ; do
+    ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=10:11:11:11:11:11,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=10),tcp_flags(0x002)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+OFPT_PACKET_IN (OF1.4) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via group) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.4) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via group) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.4) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via group) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x0000,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn tcp_csum:0
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([ovs-ofctl --protocols=OpenFlow14 dump-flows br0 | ofctl_strip | sort], [0], [dnl
+ n_packets=3, n_bytes=180, in_port=1 actions=group:1234
+OFPST_FLOW reply (OF1.4):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - ARP modification slow-path])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [1], [2])
-- 
1.9.1




More information about the dev mailing list