ofproto-dpif: Clean up and centralize sFlow logic.

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

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 6fc34fe78c67f79625e46dcfa55072046ccd79a0..85c9d1f5024cdc73ba5273c2d333eced1dffacc7 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -191,15 +191,16 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
 
         switch (cookie.type) {
         case USER_ACTION_COOKIE_SFLOW:
-            ds_put_format(ds, ",sFlow,n_output=%"PRIu8","
-                          "vid=%"PRIu16",pcp=%"PRIu8",ifindex=%"PRIu32,
-                          cookie.n_output, vlan_tci_to_vid(cookie.vlan_tci),
-                          vlan_tci_to_pcp(cookie.vlan_tci), cookie.data);
+            ds_put_format(ds, ",sFlow,"
+                          "vid=%"PRIu16",pcp=%"PRIu8",output=%"PRIu32,
+                          vlan_tci_to_vid(cookie.vlan_tci),
+                          vlan_tci_to_pcp(cookie.vlan_tci), cookie.output);
             break;
 
         case USER_ACTION_COOKIE_UNSPEC:
         default:
             ds_put_format(ds, ",userdata=0x%"PRIx64, userdata);
+            break;
         }
     }
 
@@ -336,18 +337,17 @@ parse_odp_action(const char *s, const struct shash *port_names,
 
     {
         unsigned long long int pid;
-        unsigned long long int ifindex;
+        unsigned long long int output;
         char userdata_s[32];
-        int n_output;
         int vid, pcp;
         int n = -1;
 
         if (sscanf(s, "userspace(pid=%lli)%n", &pid, &n) > 0 && n > 0) {
             odp_put_userspace_action(pid, NULL, actions);
             return n;
-        } else if (sscanf(s, "userspace(pid=%lli,sFlow,n_output=%i,vid=%i,"
-                          "pcp=%i,ifindex=%lli)%n", &pid, &n_output,
-                          &vid, &pcp, &ifindex, &n) > 0 && n > 0) {
+        } else if (sscanf(s, "userspace(pid=%lli,sFlow,vid=%i,"
+                          "pcp=%i,output=%lli)%n",
+                          &pid, &vid, &pcp, &output, &n) > 0 && n > 0) {
             struct user_action_cookie cookie;
             uint16_t tci;
 
@@ -357,9 +357,8 @@ parse_odp_action(const char *s, const struct shash *port_names,
             }
 
             cookie.type = USER_ACTION_COOKIE_SFLOW;
-            cookie.n_output = n_output;
             cookie.vlan_tci = htons(tci);
-            cookie.data = ifindex;
+            cookie.output = output;
             odp_put_userspace_action(pid, &cookie, actions);
             return n;
         } else if (sscanf(s, "userspace(pid=%lli,userdata="

diff --git a/lib/odp-util.h b/lib/odp-util.h
index 3d337a7a151e9d548899f945d5270cc3791b648c..cdafbe46c056d379ede3419eb72da06eb452eb21 100644 (file)
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -124,11 +124,11 @@ enum user_action_cookie_type {
 /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE.
  * Since it is passed to kernel as u64, its size has to be 8 bytes. */
 struct user_action_cookie {
-    uint8_t   type;                 /* enum user_action_cookie_type. */
-    uint8_t   n_output;             /* No of output ports. used by sflow. */
-    ovs_be16  vlan_tci;             /* Used by sFlow */
-    uint32_t  data;                 /* Data is len for OFPP_CONTROLLER action.
-                                       For sFlow it is port_ifindex. */
+    uint16_t type;              /* enum user_action_cookie_type. */
+
+    /* The following members are used only by USER_ACTION_COOKIE_SFLOW. */
+    ovs_be16 vlan_tci;          /* Destination VLAN TCI. */
+    uint32_t output;            /* SFL_FLOW_SAMPLE_TYPE 'output' value. */
 };
 
 BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 8);

diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index fb09b2f52e1de4b5fd08a719ff8f6aacb7013954..78a6dd6ac4836bba7b47f3600531ef8f04786179 100644 (file)
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -552,18 +552,7 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
     switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(cookie->vlan_tci);
     switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(cookie->vlan_tci);
 
-    /* Set output port, as defined by http://www.sflow.org/sflow_version_5.txt
-       (search for "Input/output port information"). */
-    if (!cookie->n_output) {
-        /* This value indicates that the packet was dropped for an unknown
-         * reason. */
-        fs.output = 0x40000000 | 256;
-    } else if (cookie->n_output > 1 || !cookie->data) {
-        /* Setting the high bit means "multiple output ports". */
-        fs.output = 0x80000000 | cookie->n_output;
-    } else {
-        fs.output = cookie->data;
-    }
+    fs.output = cookie->output;
 
     /* Submit the flow sample to be encoded into the next datagram. */
     SFLADD_ELEMENT(&fs, &hdrElem);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b8ea49a26247ba9461ae48d0fb2609719cc6dbb3..cb28cf6b80f448066ad2a227acf2e8995d42d133 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4518,6 +4518,38 @@ put_userspace_action(const struct ofproto_dpif *ofproto,
     return odp_put_userspace_action(pid, cookie, odp_actions);
 }
 
+static void
+compose_sflow_cookie(const struct ofproto_dpif *ofproto,
+                     ovs_be16 vlan_tci, uint32_t odp_port,
+                     unsigned int n_outputs, struct user_action_cookie *cookie)
+{
+    int ifindex;
+
+    cookie->type = USER_ACTION_COOKIE_SFLOW;
+    cookie->vlan_tci = vlan_tci;
+
+    /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output
+     * port information") for the interpretation of cookie->output. */
+    switch (n_outputs) {
+    case 0:
+        /* 0x40000000 | 256 means "packet dropped for unknown reason". */
+        cookie->output = 0x40000000 | 256;
+        break;
+
+    case 1:
+        ifindex = dpif_sflow_odp_port_to_ifindex(ofproto->sflow, odp_port);
+        if (ifindex) {
+            cookie->output = ifindex;
+            break;
+        }
+        /* Fall through. */
+    default:
+        /* 0x80000000 means "multiple output ports. */
+        cookie->output = 0x80000000 | n_outputs;
+        break;
+    }
+}
+
 /* Compose SAMPLE action for sFlow. */
 static size_t
 compose_sflow_action(const struct ofproto_dpif *ofproto,
@@ -4525,24 +4557,15 @@ compose_sflow_action(const struct ofproto_dpif *ofproto,
                      const struct flow *flow,
                      uint32_t odp_port)
 {
-    uint32_t port_ifindex;
     uint32_t probability;
     struct user_action_cookie cookie;
     size_t sample_offset, actions_offset;
-    int cookie_offset, n_output;
+    int cookie_offset;
 
     if (!ofproto->sflow || flow->in_port == OFPP_NONE) {
         return 0;
     }
 
-    if (odp_port == OVSP_NONE) {
-        port_ifindex = 0;
-        n_output = 0;
-    } else {
-        port_ifindex = dpif_sflow_odp_port_to_ifindex(ofproto->sflow, odp_port);
-        n_output = 1;
-    }
-
     sample_offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SAMPLE);
 
     /* Number of packets out of UINT_MAX to sample. */
@@ -4550,11 +4573,8 @@ compose_sflow_action(const struct ofproto_dpif *ofproto,
     nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability);
 
     actions_offset = nl_msg_start_nested(odp_actions, OVS_SAMPLE_ATTR_ACTIONS);
-
-    cookie.type = USER_ACTION_COOKIE_SFLOW;
-    cookie.data = port_ifindex;
-    cookie.n_output = n_output;
-    cookie.vlan_tci = 0;
+    compose_sflow_cookie(ofproto, htons(0), odp_port,
+                         odp_port == OVSP_NONE ? 0 : 1, &cookie);
     cookie_offset = put_userspace_action(ofproto, odp_actions, flow, &cookie);
 
     nl_msg_end_nested(odp_actions, actions_offset);
@@ -4589,20 +4609,11 @@ fix_sflow_action(struct action_xlate_ctx *ctx)
     }
 
     cookie = ofpbuf_at(ctx->odp_actions, ctx->user_cookie_offset,
-                     sizeof(*cookie));
-    assert(cookie != NULL);
+                       sizeof(*cookie));
     assert(cookie->type == USER_ACTION_COOKIE_SFLOW);
 
-    if (ctx->sflow_n_outputs) {
-        cookie->data = dpif_sflow_odp_port_to_ifindex(ctx->ofproto->sflow,
-                                                    ctx->sflow_odp_port);
-    }
-    if (ctx->sflow_n_outputs >= 255) {
-        cookie->n_output = 255;
-    } else {
-        cookie->n_output = ctx->sflow_n_outputs;
-    }
-    cookie->vlan_tci = base->vlan_tci;
+    compose_sflow_cookie(ctx->ofproto, base->vlan_tci,
+                         ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie);
 }
 
 static void