connmgr: Fix internal packet-in reason code mask.

Starting from OpenFlow 1.4+, OFPR_ACTION is split into four more descriptive
reasons, OFPR_APPLY_ACTION, OFPR_ACTION_SET, OFPR_GROUP, and OFPR_PACKET_OUT.
OVS maintains the new reason code internally, and it currently supports the
first three reason code. If the version of an established OpenFlow connection
is less than 1.4, OVS converts the internal reason code back to OFPR_ACTION to
be backward compatible. However, the internal packet-in reason code mask is
not properly maintained for the older OpenFlow version that may emit the
packet-in messages wth the new reason code. It is because OVS does not enable
the new reason code internally in the reason code mask for older OpenFlow
version. This commit tries to address the aforementioned issue.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 530c105286acad8a7c8acf95e0e64ca50a991042..5936e3f096c59efc15c412b0a3eba4eb477b1bfa 100644 (file)
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -291,9 +291,14 @@ enum ofp_packet_in_reason {
 
 #define OFPR10_BITS                                                     \
     ((1u << OFPR_NO_MATCH) | (1u << OFPR_ACTION) | (1u << OFPR_INVALID_TTL))
+
+/* From OF1.4+, OFPR_ACTION is split into four more descriptive reasons,
+ * OFPR_APPLY_ACTION, OFPR_ACTION_SET, OFPR_GROUP, and OFPR_PACKET_OUT.
+ * OFPR_APPLY_ACTION shares the same number as OFPR_ACTION. */
+#define OFPR14_ACTION_BITS                                              \
+    ((1u << OFPR_ACTION_SET) | (1u << OFPR_GROUP) | (1u << OFPR_PACKET_OUT))
 #define OFPR14_BITS                                                     \
-    (OFPR10_BITS |                                                      \
-     (1u << OFPR_ACTION_SET) | (1u << OFPR_GROUP) | (1u << OFPR_PACKET_OUT))
+    (OFPR10_BITS | OFPR14_ACTION_BITS)
 
     /* Nonstandard reason--not exposed via OpenFlow. */
     OFPR_EXPLICIT_MISS,

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 854868e7be78fbe3a7525bdef18bed9303049f06..2b0b78f6192dbb03045ca1d2818f5b33a9c96181 100644 (file)
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1092,6 +1092,16 @@ ofconn_set_async_config(struct ofconn *ofconn,
         ofconn->async_cfg = xmalloc(sizeof *ofconn->async_cfg);
     }
     *ofconn->async_cfg = *ac;
+
+    if (ofputil_protocol_to_ofp_version(ofconn_get_protocol(ofconn))
+        < OFP14_VERSION) {
+        if (ofconn->async_cfg->master[OAM_PACKET_IN] & (1u << OFPR_ACTION)) {
+            ofconn->async_cfg->master[OAM_PACKET_IN] |= OFPR14_ACTION_BITS;
+        }
+        if (ofconn->async_cfg->slave[OAM_PACKET_IN] & (1u << OFPR_ACTION)) {
+            ofconn->async_cfg->slave[OAM_PACKET_IN] |= OFPR14_ACTION_BITS;
+        }
+    }
 }
 
 struct ofputil_async_cfg

diff --git a/tests/ofproto.at b/tests/ofproto.at
index e191c498854f74fda888666188acab086e904efd..a73dde0687c4b63ed8cdf3c33ec3168c340eb9d7 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -3361,7 +3361,9 @@ OVS_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([ofproto - asynchronous message control (OpenFlow 1.3)])
-OVS_VSWITCHD_START
+OVS_VSWITCHD_START([dnl
+    add-port br0 p1 -- set Interface p1 type=dummy ofport_request=10
+])
 AT_CHECK([ovs-ofctl -O OpenFlow13 -P standard monitor br0 --detach --no-chdir --pidfile])
 check_async () {
     printf '\n\n--- check_async %d ---\n\n\n' $1
@@ -3373,9 +3375,14 @@ check_async () {
     : > expout
 
     # OFPT_PACKET_IN, OFPR_ACTION (controller_id=0)
+    # OFPR_ACTION_SET is treated as OFPR_ACTION in OpenFlow 1.3
     ovs-ofctl -O OpenFlow13 -v packet-out br0 none controller '0001020304050010203040501234'
+    ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=10 actions=write_actions(output(CONTROLLER))'
+    ovs-appctl netdev-dummy/receive p1 'in_port(10),eth(src=00:10:20:30:40:50,dst=00:01:02:03:04:05),eth_type(0x1234)'
     if test X"$1" = X"OFPR_ACTION"; then shift;
         echo >>expout "OFPT_PACKET_IN (OF1.3): total_len=14 in_port=ANY (via action) data_len=14 (unbuffered)
+vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
+        echo >>expout "OFPT_PACKET_IN (OF1.3): cookie=0x0 total_len=14 in_port=10 (via action) data_len=14 (unbuffered)
 vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
     fi
 
@@ -3470,7 +3477,9 @@ OVS_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([ofproto - asynchronous message control (OpenFlow 1.4)])
-OVS_VSWITCHD_START
+OVS_VSWITCHD_START([dnl
+    add-port br0 p1 -- set Interface p1 type=dummy ofport_request=10
+])
 AT_CHECK([ovs-ofctl -O OpenFlow14 -P standard monitor br0 --detach --no-chdir --pidfile])
 check_async () {
     printf '\n\n--- check_async %d ---\n\n\n' $1
@@ -3488,6 +3497,14 @@ check_async () {
 vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
     fi
 
+    # OFPT_PACKET_IN, OFPR_ACTION_SET (controller_id=0)
+    ovs-ofctl -O OpenFlow14 add-flow br0 'in_port=10 actions=write_actions(output(CONTROLLER))'
+    ovs-appctl netdev-dummy/receive p1 'in_port(10),eth(src=00:10:20:30:40:50,dst=00:01:02:03:04:05),eth_type(0x1234)'
+    if test X"$1" = X"OFPR_ACTION_SET"; then shift;
+        echo >>expout "OFPT_PACKET_IN (OF1.4): cookie=0x0 total_len=14 in_port=10 (via action_set) data_len=14 (unbuffered)
+vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
+    fi
+
     # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
     ovs-ofctl -O OpenFlow14 -v packet-out br0 none 'controller(reason=no_match,id=123)' '0001020304050010203040501234'
     if test X"$1" = X"OFPR_NO_MATCH"; then shift;
@@ -3644,7 +3661,7 @@ check_async 1
 
 # Set miss_send_len to 128, turning on packet-ins for our service connection.
 ovs-appctl -t ovs-ofctl ofctl/send 0509000c0123456700000080
-check_async 2 OFPR_ACTION OFPPR_ADD OFPPR_MODIFY OFPPR_DELETE OFPRR_DELETE OFPRR_GROUP_DELETE
+check_async 2 OFPR_ACTION OFPR_ACTION_SET OFPPR_ADD OFPPR_MODIFY OFPPR_DELETE OFPRR_DELETE OFPRR_GROUP_DELETE
 
 # Become slave (OF 1.4), which should disable everything except port status.
 ovs-appctl -t ovs-ofctl ofctl/send 051800180000000200000003000000000000000000000001