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 3b2fba3..256c9d6 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 594ce59..0641d30 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 4d7e8b3..ce7c580 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 ff6dac1..094cbd0 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;
 }