From: Ben Pfaff <blp@nicira.com>
Date: Sat, 5 May 2012 18:07:42 +0000 (-0700)
Subject: ofproto-dpif: Segregate CFM, LACP, and STP traffic into separate queues.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=625b07205a37dd8c7a8b53b2e53e5f0b55203b38;p=openvswitch

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 3b2fba3c..256c9d6c 100644
--- 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 594ce592..0641d306 100644
--- 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 4d7e8b39..ce7c5801 100644
--- 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 ff6dac18..094cbd00 100644
--- 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;
 }