ofproto-dpif: Segregate CFM, LACP, and STP traffic into separate queues.

Until now, packets for these special protocols have been mixed with general
traffic in the kernel-to-userspace queues.  This means that a big-enough
storm of new flows in these queues can cause packets for these special
protocols to be dropped at this interface, fooling userspace into believing
that, say, no CFM packets have been received even though they are arriving
at the expected rate.

This commit moves special protocols to a dedicated kernel-to-userspace
queue to avoid the problem.

Bug #7550.
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 3b2fba3c0f87d053c475fea6e2fd2989023f659e..256c9d6ccffa38da7031bac6bd7bd9dd631f2cfb 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -61,8 +61,9 @@
 VLOG_DEFINE_THIS_MODULE(dpif_linux);
 enum { MAX_PORTS = USHRT_MAX };
 
-enum { N_UPCALL_SOCKS = 16 };
-BUILD_ASSERT_DECL(IS_POW2(N_UPCALL_SOCKS));
+enum { N_UPCALL_SOCKS = 17 };
+BUILD_ASSERT_DECL(IS_POW2(N_UPCALL_SOCKS - 1));
+BUILD_ASSERT_DECL(N_UPCALL_SOCKS > 1);
 BUILD_ASSERT_DECL(N_UPCALL_SOCKS <= 32); /* We use a 32-bit word as a mask. */
 
 /* This ethtool flag was introduced in Linux 2.6.24, so it might be
@@ -460,7 +461,11 @@ dpif_linux_port_get_pid(const struct dpif *dpif_, uint16_t port_no)
     if (dpif->epoll_fd < 0) {
         return 0;
     } else {
-        int idx = port_no & (N_UPCALL_SOCKS - 1);
+        int idx;
+
+        idx = (port_no != UINT16_MAX
+               ? 1 + (port_no & (N_UPCALL_SOCKS - 2))
+               : 0);
         return nl_sock_pid(dpif->upcall_socks[idx]);
     }
 }

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 594ce592031b0a268915be5343d8a38bdee278a6..0641d30696c8d26374cb437aa467b1ebe3fd296f 100644 (file)
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -136,6 +136,10 @@ struct dpif_class {
      * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
      * flows whose packets arrived on port 'port_no'.
      *
+     * A 'port_no' of UINT16_MAX should be treated as a special case.  The
+     * implementation should return a reserved PID, not allocated to any port,
+     * that the client may use for special purposes.
+     *
      * The return value only needs to be meaningful when DPIF_UC_ACTION has
      * been enabled in the 'dpif''s listen mask, and it is allowed to change
      * when DPIF_UC_ACTION is disabled and then re-enabled.

diff --git a/lib/dpif.c b/lib/dpif.c
index 4d7e8b3928b09ad7d869cbdf28269a1fa3c85107..ce7c5801f780ad272da02c1ad20cb22a54c411fb 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -541,6 +541,9 @@ dpif_get_max_ports(const struct dpif *dpif)
  * as the OVS_USERSPACE_ATTR_PID attribute's value, for use in flows whose
  * packets arrived on port 'port_no'.
  *
+ * A 'port_no' of UINT16_MAX is a special case: it returns a reserved PID, not
+ * allocated to any port, that the client may use for special purposes.
+ *
  * The return value is only meaningful when DPIF_UC_ACTION has been enabled in
  * the 'dpif''s listen mask.  It is allowed to change when DPIF_UC_ACTION is
  * disabled and then re-enabled, so a client that does that must be prepared to

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ff6dac18b20fd23f9af1e55b330f62d3253179d5..094cbd00e0ecf75d52a3f4787d071a02d14fc4b6 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3469,7 +3469,12 @@ expire_batch(struct ofproto_dpif *ofproto, struct subfacet **subfacets, int n)
 static void
 expire_subfacets(struct ofproto_dpif *ofproto, int dp_max_idle)
 {
-    long long int cutoff = time_msec() - dp_max_idle;
+    /* Cutoff time for most flows. */
+    long long int normal_cutoff = time_msec() - dp_max_idle;
+
+    /* We really want to keep flows for special protocols around, so use a more
+     * conservative cutoff. */
+    long long int special_cutoff = time_msec() - 10000;
 
     struct subfacet *subfacet, *next_subfacet;
     struct subfacet *batch[EXPIRE_MAX_BATCH];
@@ -3478,6 +3483,11 @@ expire_subfacets(struct ofproto_dpif *ofproto, int dp_max_idle)
     n_batch = 0;
     HMAP_FOR_EACH_SAFE (subfacet, next_subfacet, hmap_node,
                         &ofproto->subfacets) {
+        long long int cutoff;
+
+        cutoff = (subfacet->slow & (SLOW_CFM | SLOW_LACP | SLOW_STP)
+                  ? special_cutoff
+                  : normal_cutoff);
         if (subfacet->used < cutoff) {
             if (subfacet->path != SF_NOT_INSTALLED) {
                 batch[n_batch++] = subfacet;
@@ -4714,7 +4724,12 @@ compose_slow_path(const struct ofproto_dpif *ofproto, const struct flow *flow,
     cookie.slow_path.reason = slow;
 
     ofpbuf_use_stack(&buf, stub, stub_size);
-    put_userspace_action(ofproto, &buf, flow, &cookie);
+    if (slow & (SLOW_CFM | SLOW_LACP | SLOW_STP)) {
+        uint32_t pid = dpif_port_get_pid(ofproto->dpif, UINT16_MAX);
+        odp_put_userspace_action(pid, &cookie, &buf);
+    } else {
+        put_userspace_action(ofproto, &buf, flow, &cookie);
+    }
     *actionsp = buf.data;
     *actions_lenp = buf.size;
 }