[ovs-dev] [PATCH] sFlow export: include standard tunnel structures (for GRE, VXLAN etc.)

Neil Mckee neil.mckee at inmon.com
Mon Oct 7 05:11:11 UTC 2013


 Please comment on this proposed patch.   It adds the standard
sFlow-TUNNEL structures to the sFlow export,  which will be helpful
for tracing tunneled traffic through a network fabric in real-time.

As part of doing that,  it switches over from using odp_port to ofp_port
as the port reference used in dpif-sflow,  which allows some only-for-sflow
code to be removed elsewhere.  It also completely removes the private
port-lookup hash table that dpif-sflow was maintaining and replaces
it with a simple callback to the parent dpif - presenting the parent with
a structure to fill in with just the necessary details about the port.
This makes it  easier for the parent dpif to control what port state is 
exposed to the sFlow module during the processing of a packet-sample.

(This same mechanism can now be extended to expose some
bundle/LAG information,  so the standard sFlow-LAG structures can
be exported too,  but that can be in another patch).

This patch also cleans up the way sFlow is treated in ofproto-xlate.c, so
that the upcall cookie just contains the essential details and the
sFlow-specific encoding is migrated into ofproto-dpif-sflow.c.

Finally a new test for sFlow tunnel structures was added that exercises
the new code.  So 'make check TESTSUITEFLAGS="-k sflow"' now runs
two separate tests.

One very specific question is asked in an XXX comment in ofproto-dpif.c,
regarding which lock should be acquired during the new callback.  A little
guidance there would be most appreciated.

Regards,
Neil

---
 lib/odp-util.c               |  15 +--
 lib/odp-util.h               |   3 +-
 lib/sflow.h                  |   2 +
 lib/sflow_agent.c            |  15 +++
 lib/sflow_api.h              |   1 +
 lib/sflow_receiver.c         |   4 +
 manpages.mk                  |   8 +-
 ofproto/ofproto-dpif-sflow.c | 250 +++++++++++++++++++++++--------------------
 ofproto/ofproto-dpif-sflow.h |  22 ++--
 ofproto/ofproto-dpif-xlate.c |  50 +++------
 ofproto/ofproto-dpif.c       |  38 +++++--
 tests/odp.at                 |   2 +-
 tests/ofproto-dpif.at        |  80 ++++++++++++++
 tests/test-sflow.c           |  39 +++++++
 14 files changed, 351 insertions(+), 178 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5ca8baf..19102ad 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -296,10 +296,11 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
             if (userdata_len == sizeof cookie.sflow
                 && cookie.type == USER_ACTION_COOKIE_SFLOW) {
                 ds_put_format(ds, ",sFlow("
-                              "vid=%"PRIu16",pcp=%"PRIu8",output=%"PRIu32")",
+                              "vid=%"PRIu16",pcp=%"PRIu8
+			      ",n_outputs=%"PRIu32",output_port=%"PRIu32")",
                               vlan_tci_to_vid(cookie.sflow.vlan_tci),
                               vlan_tci_to_pcp(cookie.sflow.vlan_tci),
-                              cookie.sflow.output);
+                              cookie.sflow.n_outputs,cookie.sflow.output_port);
             } else if (userdata_len == sizeof cookie.slow_path
                        && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) {
                 const char *reason;
@@ -507,7 +508,8 @@ parse_odp_action(const char *s, const struct simap *port_names,
 
     {
         unsigned long long int pid;
-        unsigned long long int output;
+        unsigned long long int n_outputs;
+        unsigned long long int output_port;
         unsigned long long int probability;
         unsigned long long int collector_set_id;
         unsigned long long int obs_domain_id;
@@ -519,8 +521,8 @@ parse_odp_action(const char *s, const struct simap *port_names,
             odp_put_userspace_action(pid, NULL, 0, actions);
             return n;
         } else if (sscanf(s, "userspace(pid=%lli,sFlow(vid=%i,"
-                          "pcp=%i,output=%lli))%n",
-                          &pid, &vid, &pcp, &output, &n) > 0 && n > 0) {
+                          "pcp=%i,n_outputs=%lli,output_port=%lli))%n",
+                          &pid, &vid, &pcp, &n_outputs, &output_port, &n) > 0 && n > 0) {
             union user_action_cookie cookie;
             uint16_t tci;
 
@@ -531,7 +533,8 @@ parse_odp_action(const char *s, const struct simap *port_names,
 
             cookie.type = USER_ACTION_COOKIE_SFLOW;
             cookie.sflow.vlan_tci = htons(tci);
-            cookie.sflow.output = output;
+            cookie.sflow.n_outputs = n_outputs;
+            cookie.sflow.output_port = output_port;
             odp_put_userspace_action(pid, &cookie, sizeof cookie.sflow,
                                      actions);
             return n;
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 192cfa0..2440394 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -156,7 +156,8 @@ union user_action_cookie {
     struct {
         uint16_t type;          /* USER_ACTION_COOKIE_SFLOW. */
         ovs_be16 vlan_tci;      /* Destination VLAN TCI. */
-        uint32_t output;        /* SFL_FLOW_SAMPLE_TYPE 'output' value. */
+	uint32_t n_outputs;     /* Number of output ports. */
+	odp_port_t output_port; /* Set if n_outputs == 1. */
     } sflow;
 
     struct {
diff --git a/lib/sflow.h b/lib/sflow.h
index 0d1f2b9..6e73bba 100644
--- a/lib/sflow.h
+++ b/lib/sflow.h
@@ -285,6 +285,8 @@ enum SFLFlow_type_tag {
     SFLFLOW_EX_MPLS_FTN     = 1010,
     SFLFLOW_EX_MPLS_LDP_FEC = 1011,
     SFLFLOW_EX_VLAN_TUNNEL  = 1012,   /* VLAN stack */
+    SFLFLOW_EX_IPV4_TUNNEL_EGRESS  = 1023, /* http://sflow.org/sflow_tunnels.txt */
+    SFLFLOW_EX_IPV4_TUNNEL_INGRESS = 1024,
 };
 
 typedef union _SFLFlow_type {
diff --git a/lib/sflow_agent.c b/lib/sflow_agent.c
index 817420d..9c2e028 100644
--- a/lib/sflow_agent.c
+++ b/lib/sflow_agent.c
@@ -363,6 +363,21 @@ SFLPoller *sfl_agent_getPoller(SFLAgent *agent, SFLDataSource_instance *pdsi)
     return NULL;
 }
 
+/*_________________-----------------------------------__________________
+  _________________  sfl_agent_getPollerByBridgePort  __________________
+  -----------------___________________________________------------------
+*/
+
+SFLPoller *sfl_agent_getPollerByBridgePort(SFLAgent *agent, uint32_t port_no)
+{
+  /* find it and return it */
+    SFLPoller *pl = agent->pollers;
+    for(; pl != NULL; pl = pl->nxt)
+	if(pl->bridgePort == port_no) return pl;
+    /* not found */
+    return NULL;
+}
+
 /*_________________---------------------------__________________
   _________________  sfl_agent_getReceiver    __________________
   -----------------___________________________------------------
diff --git a/lib/sflow_api.h b/lib/sflow_api.h
index 3cc060b..2730a4c 100644
--- a/lib/sflow_api.h
+++ b/lib/sflow_api.h
@@ -281,6 +281,7 @@ void sfl_agent_set_agentSubId(SFLAgent *agent, u_int32_t subId);
    to get counters if it is not the same as the global ifIndex */
 void sfl_poller_set_bridgePort(SFLPoller *poller, u_int32_t port_no);
 u_int32_t sfl_poller_get_bridgePort(SFLPoller *poller);
+SFLPoller *sfl_agent_getPollerByBridgePort(SFLAgent *agent, u_int32_t port_no);
 
 /* call this to indicate a discontinuity with a counter like samplePool so that the
    sflow collector will ignore the next delta */
diff --git a/lib/sflow_receiver.c b/lib/sflow_receiver.c
index 3e5a67a..3e7c68e 100644
--- a/lib/sflow_receiver.c
+++ b/lib/sflow_receiver.c
@@ -460,6 +460,8 @@ static int computeFlowSampleSize(SFLReceiver *receiver, SFL_FLOW_SAMPLE_TYPE *fs
 	case SFLFLOW_EX_MPLS_FTN: elemSiz = mplsFtnEncodingLength(&elem->flowType.mpls_ftn); break;
 	case SFLFLOW_EX_MPLS_LDP_FEC: elemSiz = mplsLdpFecEncodingLength(&elem->flowType.mpls_ldp_fec); break;
 	case SFLFLOW_EX_VLAN_TUNNEL: elemSiz = vlanTunnelEncodingLength(&elem->flowType.vlan_tunnel); break;
+	case SFLFLOW_EX_IPV4_TUNNEL_EGRESS: elemSiz = sizeof(SFLSampled_ipv4); break;
+	case SFLFLOW_EX_IPV4_TUNNEL_INGRESS: elemSiz = sizeof(SFLSampled_ipv4); break;
 	default:
 	    sflError(receiver, "unexpected packet_data_tag");
 	    return -1;
@@ -556,6 +558,8 @@ int sfl_receiver_writeFlowSample(SFLReceiver *receiver, SFL_FLOW_SAMPLE_TYPE *fs
 		putNet32(receiver, elem->flowType.ethernet.eth_type);
 		break;
 	    case SFLFLOW_IPV4:
+	    case SFLFLOW_EX_IPV4_TUNNEL_EGRESS:
+	    case SFLFLOW_EX_IPV4_TUNNEL_INGRESS:
 		putNet32(receiver, elem->flowType.ipv4.length);
 		putNet32(receiver, elem->flowType.ipv4.protocol);
 		put32(receiver, elem->flowType.ipv4.src_ip.addr);
diff --git a/manpages.mk b/manpages.mk
index 811d2f9..2a34f04 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -116,6 +116,10 @@ lib/vconn-active.man:
 lib/vconn-passive.man:
 lib/vlog.man:
 
+utilities/ovs-dpctl-top.8: \
+	utilities/ovs-dpctl-top.8.in
+utilities/ovs-dpctl-top.8.in:
+
 utilities/ovs-dpctl.8: \
 	utilities/ovs-dpctl.8.in \
 	lib/common.man \
@@ -124,10 +128,6 @@ utilities/ovs-dpctl.8.in:
 lib/common.man:
 lib/vlog.man:
 
-utilities/ovs-dpctl-top.8: \
-	utilities/ovs-dpctl-top.8.in
-utilities/ovs-dpctl-top.8.in:
-
 utilities/ovs-l3ping.8: \
 	utilities/ovs-l3ping.8.in \
 	lib/common-syn.man \
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 158887f..de679ed 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -24,8 +24,6 @@
 #include "collectors.h"
 #include "compiler.h"
 #include "dpif.h"
-#include "hash.h"
-#include "hmap.h"
 #include "netdev.h"
 #include "netlink.h"
 #include "ofpbuf.h"
@@ -44,27 +42,18 @@ VLOG_DEFINE_THIS_MODULE(sflow);
 
 static struct ovs_mutex mutex;
 
-struct dpif_sflow_port {
-    struct hmap_node hmap_node; /* In struct dpif_sflow's "ports" hmap. */
-    SFLDataSource_instance dsi; /* sFlow library's notion of port number. */
-    struct ofport *ofport;      /* To retrive port stats. */
-    odp_port_t odp_port;
-};
-
 struct dpif_sflow {
     struct collectors *collectors;
     SFLAgent *sflow_agent;
     struct ofproto_sflow_options *options;
     time_t next_tick;
     size_t n_flood, n_all;
-    struct hmap ports;          /* Contains "struct dpif_sflow_port"s. */
     uint32_t probability;
     atomic_int ref_cnt;
+    dpif_sflow_callback port_callback;
+    void *port_callback_magic;
 };
 
-static void dpif_sflow_del_port__(struct dpif_sflow *,
-                                  struct dpif_sflow_port *);
-
 #define RECEIVER_INDEX 1
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -144,21 +133,6 @@ sflow_agent_send_packet_cb(void *ds_, SFLAgent *agent OVS_UNUSED,
     collectors_send(ds->collectors, pkt, pktLen);
 }
 
-static struct dpif_sflow_port *
-dpif_sflow_find_port(const struct dpif_sflow *ds, odp_port_t odp_port)
-    OVS_REQUIRES(mutex)
-{
-    struct dpif_sflow_port *dsp;
-
-    HMAP_FOR_EACH_IN_BUCKET (dsp, hmap_node, hash_odp_port(odp_port),
-                             &ds->ports) {
-        if (dsp->odp_port == odp_port) {
-            return dsp;
-        }
-    }
-    return NULL;
-}
-
 static void
 sflow_agent_get_counters(void *ds_, SFLPoller *poller,
                          SFL_COUNTERS_SAMPLE_TYPE *cs)
@@ -167,21 +141,24 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller,
     struct dpif_sflow *ds = ds_;
     SFLCounters_sample_element elem;
     enum netdev_features current;
-    struct dpif_sflow_port *dsp;
     SFLIf_counters *counters;
     struct netdev_stats stats;
     enum netdev_flags flags;
+    struct dpif_sflow_port_lookup port_lookup;
 
-    dsp = dpif_sflow_find_port(ds, u32_to_odp(poller->bridgePort));
-    if (!dsp) {
-        return;
+    memset(&port_lookup, 0, sizeof port_lookup);
+    if(!(*ds->port_callback)(ds->port_callback_magic,
+			     (ofp_port_t)(poller->bridgePort),
+			     &port_lookup)) {
+      return;
     }
-
+    
+    memset(&elem, 0, sizeof elem);
     elem.tag = SFLCOUNTERS_GENERIC;
     counters = &elem.counterBlock.generic;
     counters->ifIndex = SFL_DS_INDEX(poller->dsi);
     counters->ifType = 6;
-    if (!netdev_get_features(dsp->ofport->netdev, &current, NULL, NULL, NULL)) {
+    if (!netdev_get_features(port_lookup.ofport->netdev, &current, NULL, NULL, NULL)) {
         /* The values of ifDirection come from MAU MIB (RFC 2668): 0 = unknown,
            1 = full-duplex, 2 = half-duplex, 3 = in, 4=out */
         counters->ifSpeed = netdev_features_to_bps(current, 0);
@@ -191,9 +168,9 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller,
         counters->ifSpeed = 100000000;
         counters->ifDirection = 0;
     }
-    if (!netdev_get_flags(dsp->ofport->netdev, &flags) && flags & NETDEV_UP) {
+    if (!netdev_get_flags(port_lookup.ofport->netdev, &flags) && flags & NETDEV_UP) {
         counters->ifStatus = 1; /* ifAdminStatus up. */
-        if (netdev_get_carrier(dsp->ofport->netdev)) {
+        if (netdev_get_carrier(port_lookup.ofport->netdev)) {
             counters->ifStatus |= 2; /* ifOperStatus us. */
         }
     } else {
@@ -205,7 +182,7 @@ sflow_agent_get_counters(void *ds_, SFLPoller *poller,
        2. Does the multicast counter include broadcasts?
        3. Does the rx_packets counter include multicasts/broadcasts?
     */
-    ofproto_port_get_stats(dsp->ofport, &stats);
+    ofproto_port_get_stats(port_lookup.ofport, &stats);
     counters->ifInOctets = stats.rx_bytes;
     counters->ifInUcastPkts = stats.rx_packets;
     counters->ifInMulticastPkts = stats.multicast;
@@ -324,7 +301,6 @@ dpif_sflow_create(void)
 
     ds = xcalloc(1, sizeof *ds);
     ds->next_tick = time_now() + 1;
-    hmap_init(&ds->ports);
     ds->probability = 0;
     route_table_register();
     atomic_init(&ds->ref_cnt, 1);
@@ -369,84 +345,54 @@ dpif_sflow_unref(struct dpif_sflow *ds) OVS_EXCLUDED(mutex)
     atomic_sub(&ds->ref_cnt, 1, &orig);
     ovs_assert(orig > 0);
     if (orig == 1) {
-        struct dpif_sflow_port *dsp, *next;
-
         route_table_unregister();
         dpif_sflow_clear(ds);
-        HMAP_FOR_EACH_SAFE (dsp, next, hmap_node, &ds->ports) {
-            dpif_sflow_del_port__(ds, dsp);
-        }
-        hmap_destroy(&ds->ports);
         free(ds);
     }
 }
 
 static void
-dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp)
+dpif_sflow_add_poller(struct dpif_sflow *ds, uint32_t ifindex, ofp_port_t ofp_port)
     OVS_REQUIRES(mutex)
 {
-    SFLPoller *poller = sfl_agent_addPoller(ds->sflow_agent, &dsp->dsi, ds,
-                                            sflow_agent_get_counters);
-    sfl_poller_set_sFlowCpInterval(poller, ds->options->polling_interval);
-    sfl_poller_set_sFlowCpReceiver(poller, RECEIVER_INDEX);
-    sfl_poller_set_bridgePort(poller, odp_to_u32(dsp->odp_port));
+    SFLDataSource_instance dsi;
+    SFLPoller *poller;
+    if(ds->sflow_agent) {
+	SFL_DS_SET(dsi, SFL_DSCLASS_IFINDEX, ifindex, 0);
+	poller = sfl_agent_addPoller(ds->sflow_agent, &dsi, ds,
+				     sflow_agent_get_counters);
+	sfl_poller_set_sFlowCpInterval(poller, ds->options->polling_interval);
+	sfl_poller_set_sFlowCpReceiver(poller, RECEIVER_INDEX);
+	sfl_poller_set_bridgePort(poller, (uint32_t)ofp_port);
+    }
 }
 
 void
-dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport,
-                    odp_port_t odp_port) OVS_EXCLUDED(mutex)
+dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport) OVS_EXCLUDED(mutex)
 {
-    struct dpif_sflow_port *dsp;
     int ifindex;
 
     ovs_mutex_lock(&mutex);
-    dpif_sflow_del_port(ds, odp_port);
-
+    dpif_sflow_del_port(ds, ofport->ofp_port);
     ifindex = netdev_get_ifindex(ofport->netdev);
-
-    if (ifindex <= 0) {
-        /* Not an ifindex port, so do not add a cross-reference to it here */
-        goto out;
+    if (ifindex > 0) {
+        /* ifindex port, so add a poller for it here */
+        dpif_sflow_add_poller(ds, ifindex, ofport->ofp_port);
     }
-
-    /* Add to table of ports. */
-    dsp = xmalloc(sizeof *dsp);
-    dsp->ofport = ofport;
-    dsp->odp_port = odp_port;
-    SFL_DS_SET(dsp->dsi, SFL_DSCLASS_IFINDEX, ifindex, 0);
-    hmap_insert(&ds->ports, &dsp->hmap_node, hash_odp_port(odp_port));
-
-    /* Add poller. */
-    if (ds->sflow_agent) {
-        dpif_sflow_add_poller(ds, dsp);
-    }
-
-out:
     ovs_mutex_unlock(&mutex);
 }
 
-static void
-dpif_sflow_del_port__(struct dpif_sflow *ds, struct dpif_sflow_port *dsp)
-    OVS_REQUIRES(mutex)
-{
-    if (ds->sflow_agent) {
-        sfl_agent_removePoller(ds->sflow_agent, &dsp->dsi);
-        sfl_agent_removeSampler(ds->sflow_agent, &dsp->dsi);
-    }
-    hmap_remove(&ds->ports, &dsp->hmap_node);
-    free(dsp);
-}
-
 void
-dpif_sflow_del_port(struct dpif_sflow *ds, odp_port_t odp_port)
+dpif_sflow_del_port(struct dpif_sflow *ds, ofp_port_t ofp_port)
     OVS_EXCLUDED(mutex)
 {
-    struct dpif_sflow_port *dsp;
-
+    SFLPoller *poller;
     ovs_mutex_lock(&mutex);
-    dsp = dpif_sflow_find_port(ds, odp_port);
-    if (dsp) {
-        dpif_sflow_del_port__(ds, dsp);
+    if(ds->sflow_agent) {
+	poller = sfl_agent_getPollerByBridgePort(ds->sflow_agent, (uint32_t)ofp_port);
+	if(poller) {
+	    sfl_agent_removePoller(ds->sflow_agent, &poller->dsi);
+	}
     }
     ovs_mutex_unlock(&mutex);
 }
@@ -456,7 +402,6 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
                        const struct ofproto_sflow_options *options)
     OVS_EXCLUDED(mutex)
 {
-    struct dpif_sflow_port *dsp;
     bool options_changed;
     SFLReceiver *receiver;
     SFLAddress agentIP;
@@ -543,33 +488,64 @@ dpif_sflow_set_options(struct dpif_sflow *ds,
     sfl_sampler_set_sFlowFsMaximumHeaderSize(sampler, ds->options->header_len);
     sfl_sampler_set_sFlowFsReceiver(sampler, RECEIVER_INDEX);
 
-    /* Add pollers for the currently known ifindex-ports */
-    HMAP_FOR_EACH (dsp, hmap_node, &ds->ports) {
-        dpif_sflow_add_poller(ds, dsp);
-    }
-
-
 out:
     ovs_mutex_unlock(&mutex);
 }
 
-int
-dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *ds,
-                               odp_port_t odp_port) OVS_EXCLUDED(mutex)
+void
+dpif_sflow_set_port_lookup_callback(struct dpif_sflow *ds,
+				    dpif_sflow_callback callback,
+				    void *magic)
+    OVS_EXCLUDED(mutex)
 {
-    struct dpif_sflow_port *dsp;
-    int ret;
-
     ovs_mutex_lock(&mutex);
-    dsp = dpif_sflow_find_port(ds, odp_port);
-    ret = dsp ? SFL_DS_INDEX(dsp->dsi) : 0;
+    ds->port_callback = callback;
+    ds->port_callback_magic = magic;
     ovs_mutex_unlock(&mutex);
-    return ret;
+}
+
+static bool
+dpif_sflow_ipv4_tunnel_encode(SFLSampled_ipv4 *tunnel4,
+			      struct dpif_sflow_port_lookup *port_lookup)
+{
+    const char *netdev_type;
+    const struct netdev_tunnel_config *tunnel_config;
+
+    /* XXX will need the struct flow here too - for those tunnels that
+       get their config dynamically from the flow. */
+
+    tunnel_config = netdev_get_tunnel_config(port_lookup->ofport->netdev);
+    if(tunnel_config == NULL) {
+	return false;
+    }
+    netdev_type = netdev_get_type(port_lookup->ofport->netdev);
+    tunnel4->src_ip.addr = tunnel_config->ip_src;
+    tunnel4->dst_ip.addr = tunnel_config->ip_dst;
+    /* Indicate 0==unknown for the src_port. It may be set to a random
+       number on a flow-by-flow basis to increase entropy for ECMP fabrics.
+       The assumption being made here is that it is not so important to
+       report this.  At least not important enough to justify the effort
+       of making it accessible here. */
+    tunnel4->src_port = 0;
+    tunnel4->dst_port = tunnel_config->dst_port;
+    tunnel4->tos = tunnel_config->tos;
+    /* Use the netdev_type to determine the IP protocol
+       that was (or will be) seen on the wire. */
+    if(!strncmp(netdev_type, "gre", strlen("gre"))) {
+	tunnel4->protocol = IPPROTO_GRE;
+    }
+    else if(!strncmp(netdev_type, "vxlan", strlen("vxlan"))) {
+	tunnel4->protocol = IPPROTO_UDP;
+    }
+    else if(!strncmp(netdev_type, "ipsec", strlen("ipsec"))) {
+	tunnel4->protocol = IPPROTO_ESP;
+    }
+    return true;
 }
 
 void
 dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
-                    const struct flow *flow, odp_port_t odp_in_port,
+                    const struct flow *flow,
                     const union user_action_cookie *cookie)
     OVS_EXCLUDED(mutex)
 {
@@ -578,10 +554,13 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
     SFLSampled_header *header;
     SFLFlow_sample_element switchElem;
     SFLSampler *sampler;
-    struct dpif_sflow_port *in_dsp;
     ovs_be16 vlan_tci;
+    struct dpif_sflow_port_lookup in_lookup, out_lookup;
+    SFLFlow_sample_element in_tunnelElem, out_tunnelElem;
+    int in_ifindex, out_ifindex;
 
     ovs_mutex_lock(&mutex);
+
     sampler = ds->sflow_agent->samplers;
     if (!sampler) {
         goto out;
@@ -590,11 +569,23 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
     /* Build a flow sample. */
     memset(&fs, 0, sizeof fs);
 
-    /* Look up the input ifIndex if this port has one. Otherwise just
-     * leave it as 0 (meaning 'unknown') and continue. */
-    in_dsp = dpif_sflow_find_port(ds, odp_in_port);
-    if (in_dsp) {
-        fs.input = SFL_DS_INDEX(in_dsp->dsi);
+    memset(&in_lookup, 0, sizeof in_lookup);
+    if((*ds->port_callback)(ds->port_callback_magic, flow->in_port.ofp_port, &in_lookup)) {
+	if(in_lookup.ofport->netdev) {
+	    /* Look up the input ifIndex if this port has one. Otherwise just
+	     * leave it as 0 (meaning 'unknown') and continue. */
+	    in_ifindex = netdev_get_ifindex(in_lookup.ofport->netdev);
+	    if(in_ifindex > 0) {
+		fs.input = in_ifindex;
+	    }
+	}
+	if(in_lookup.is_tunnel) {
+	    memset(&in_tunnelElem, 0, sizeof in_tunnelElem);
+	    in_tunnelElem.tag = SFLFLOW_EX_IPV4_TUNNEL_INGRESS;
+	    if(dpif_sflow_ipv4_tunnel_encode(&in_tunnelElem.flowType.ipv4, &in_lookup)) {
+		SFLADD_ELEMENT(&fs, &in_tunnelElem);
+	    }
+	}
     }
 
     /* Make the assumption that the random number generator in the datapath converges
@@ -627,7 +618,36 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
     switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(vlan_tci);
     switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(vlan_tci);
 
-    fs.output = cookie->sflow.output;
+    /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output
+     * port information") for the interpretation of cookie->output. */
+    if(cookie->sflow.n_outputs == 0) {
+      /* 0x40000000 | 256 means "packet dropped for unknown reason". */
+	fs.output = 0x40000000 | 256;
+    }
+    else if(cookie->sflow.n_outputs > 1) {
+	/* 0x80000000 means "multiple output ports. */
+	fs.output = 0x80000000 | cookie->sflow.n_outputs;
+    }
+    else {
+	memset(&out_lookup, 0, sizeof out_lookup);
+	if((*ds->port_callback)(ds->port_callback_magic, cookie->sflow.output_port, &out_lookup)) {
+	    if(out_lookup.ofport->netdev) {
+		/* Look up the output ifIndex if this port has one. Otherwise just
+		 * leave it as 0 (meaning 'unknown') and continue. */
+		out_ifindex = netdev_get_ifindex(out_lookup.ofport->netdev);
+		if(out_ifindex > 0) {
+		    fs.output = out_ifindex;
+		}
+	    }
+	    if(out_lookup.is_tunnel) {
+		memset(&out_tunnelElem, 0, sizeof out_tunnelElem);
+		out_tunnelElem.tag = SFLFLOW_EX_IPV4_TUNNEL_EGRESS;
+		if(dpif_sflow_ipv4_tunnel_encode(&out_tunnelElem.flowType.ipv4, &out_lookup)) {
+		    SFLADD_ELEMENT(&fs, &out_tunnelElem);
+		}
+	    }
+	}
+    }
 
     /* Submit the flow sample to be encoded into the next datagram. */
     SFLADD_ELEMENT(&fs, &hdrElem);
diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h
index d53c95c..8ce3a69 100644
--- a/ofproto/ofproto-dpif-sflow.h
+++ b/ofproto/ofproto-dpif-sflow.h
@@ -34,14 +34,26 @@ void dpif_sflow_unref(struct dpif_sflow *);
 
 uint32_t dpif_sflow_get_probability(const struct dpif_sflow *);
 
+struct dpif_sflow_port_lookup {
+    struct ofport *ofport;
+    /* indicate tunnel */
+    bool is_tunnel;
+    /* LAG/bundle details here? */
+};
+
+typedef bool (*dpif_sflow_callback)(void *, ofp_port_t,  struct dpif_sflow_port_lookup *);
+
+void dpif_sflow_set_port_lookup_callback(struct dpif_sflow *, dpif_sflow_callback, void *);
+
 void dpif_sflow_set_options(struct dpif_sflow *,
                             const struct ofproto_sflow_options *);
+				
 void dpif_sflow_clear(struct dpif_sflow *);
 bool dpif_sflow_is_enabled(const struct dpif_sflow *);
 
-void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport,
-                         odp_port_t odp_port);
-void dpif_sflow_del_port(struct dpif_sflow *, odp_port_t odp_port);
+void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport);
+
+void dpif_sflow_del_port(struct dpif_sflow *, ofp_port_t ofp_port);
 
 void dpif_sflow_run(struct dpif_sflow *);
 void dpif_sflow_wait(struct dpif_sflow *);
@@ -49,10 +61,6 @@ void dpif_sflow_wait(struct dpif_sflow *);
 void dpif_sflow_received(struct dpif_sflow *,
                          struct ofpbuf *,
                          const struct flow *,
-                         odp_port_t odp_port,
                          const union user_action_cookie *);
 
-int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *,
-                                   odp_port_t odp_port);
-
 #endif /* ofproto/ofproto-dpif-sflow.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5b6814..f0132b91 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -162,7 +162,7 @@ struct xlate_ctx {
     uint32_t orig_skb_priority; /* Priority when packet arrived. */
     uint8_t table_id;           /* OpenFlow table ID where flow was found. */
     uint32_t sflow_n_outputs;   /* Number of output ports. */
-    odp_port_t sflow_odp_port;  /* Output port for composing sFlow action. */
+    odp_port_t sflow_ofp_port;  /* Output port for composing sFlow action. */
     uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
     bool exit;                  /* No further actions should be processed. */
 };
@@ -1350,35 +1350,14 @@ compose_sample_action(const struct xbridge *xbridge,
 }
 
 static void
-compose_sflow_cookie(const struct xbridge *xbridge, ovs_be16 vlan_tci,
-                     odp_port_t odp_port, unsigned int n_outputs,
+compose_sflow_cookie(ovs_be16 vlan_tci,
+                     ofp_port_t ofp_port, unsigned int n_outputs,
                      union user_action_cookie *cookie)
 {
-    int ifindex;
-
     cookie->type = USER_ACTION_COOKIE_SFLOW;
     cookie->sflow.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->sflow.output = 0x40000000 | 256;
-        break;
-
-    case 1:
-        ifindex = dpif_sflow_odp_port_to_ifindex(xbridge->sflow, odp_port);
-        if (ifindex) {
-            cookie->sflow.output = ifindex;
-            break;
-        }
-        /* Fall through. */
-    default:
-        /* 0x80000000 means "multiple output ports. */
-        cookie->sflow.output = 0x80000000 | n_outputs;
-        break;
-    }
+    cookie->sflow.n_outputs = n_outputs;
+    cookie->sflow.output_port = (n_outputs == 0) ? 0 : ofp_port;
 }
 
 /* Compose SAMPLE action for sFlow bridge sampling. */
@@ -1386,7 +1365,7 @@ static size_t
 compose_sflow_action(const struct xbridge *xbridge,
                      struct ofpbuf *odp_actions,
                      const struct flow *flow,
-                     odp_port_t odp_port)
+                     ofp_port_t ofp_port)
 {
     uint32_t probability;
     union user_action_cookie cookie;
@@ -1396,8 +1375,11 @@ compose_sflow_action(const struct xbridge *xbridge,
     }
 
     probability = dpif_sflow_get_probability(xbridge->sflow);
-    compose_sflow_cookie(xbridge, htons(0), odp_port,
-                         odp_port == ODPP_NONE ? 0 : 1, &cookie);
+    // maybe we should be putting something like flow->out_port.ofp_port in here?
+    // -- and flow->in_port.ofp_port too if that's the easiest way to get the tunnel
+    // ids to the sFlow module?
+    compose_sflow_cookie(htons(0), ofp_port,
+                         ofp_port == OFPP_NONE ? 0 : 1, &cookie);
 
     return compose_sample_action(xbridge, odp_actions, flow,  probability,
                                  &cookie, sizeof cookie.sflow);
@@ -1449,8 +1431,8 @@ add_sflow_action(struct xlate_ctx *ctx)
 {
     ctx->user_cookie_offset = compose_sflow_action(ctx->xbridge,
                                                    &ctx->xout->odp_actions,
-                                                   &ctx->xin->flow, ODPP_NONE);
-    ctx->sflow_odp_port = 0;
+                                                   &ctx->xin->flow, OFPP_NONE);
+    ctx->sflow_ofp_port = 0;
     ctx->sflow_n_outputs = 0;
 }
 
@@ -1480,8 +1462,8 @@ fix_sflow_action(struct xlate_ctx *ctx)
                        sizeof cookie->sflow);
     ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW);
 
-    compose_sflow_cookie(ctx->xbridge, base->vlan_tci,
-                         ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie);
+    compose_sflow_cookie(base->vlan_tci,
+                         ctx->sflow_ofp_port, ctx->sflow_n_outputs, cookie);
 }
 
 static enum slow_path_reason
@@ -1649,7 +1631,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
                             out_port);
 
-        ctx->sflow_odp_port = odp_port;
+        ctx->sflow_ofp_port = ofp_port;
         ctx->sflow_n_outputs++;
         ctx->xout->nf_output_iface = ofp_port;
     }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 80874b8..3972a1b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1763,7 +1763,7 @@ port_construct(struct ofport *port_)
     dpif_port_destroy(&dpif_port);
 
     if (ofproto->sflow) {
-        dpif_sflow_add_port(ofproto->sflow, port_, port->odp_port);
+        dpif_sflow_add_port(ofproto->sflow, port_);
     }
 
     return 0;
@@ -1863,24 +1863,43 @@ port_reconfigured(struct ofport *port_, enum ofputil_port_config old_config)
     }
 }
 
+static bool
+dpif_sflow_port_lookup_callback(void *magic, ofp_port_t ofp_port, struct dpif_sflow_port_lookup *port_lookup)
+{
+    /* XXX do we need to acquire ofproto_mutex here?  Or should it be acquired in handle_sflow_upcall() below,
+     * so that it will not be released until after the sFlow-module has followed the pointers we are giving
+     * it here and the sflow sample has been fully processed? */
+
+    struct ofproto_dpif *ofproto = (struct ofproto_dpif *)magic;
+    struct ofport_dpif *port = get_ofp_port(ofproto, ofp_port);
+    if(port) {
+	port_lookup->ofport = &port->up;
+	port_lookup->is_tunnel = port->is_tunnel;
+	/* XXX add bundle and lacp info here */
+	return true;
+    }
+    return false;
+}
+
+
 static int
 set_sflow(struct ofproto *ofproto_,
           const struct ofproto_sflow_options *sflow_options)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct dpif_sflow *ds = ofproto->sflow;
+    struct ofport_dpif *ofport;
 
     if (sflow_options) {
         if (!ds) {
-            struct ofport_dpif *ofport;
-
-            ds = ofproto->sflow = dpif_sflow_create();
-            HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
-                dpif_sflow_add_port(ds, &ofport->up, ofport->odp_port);
-            }
-            ofproto->backer->need_revalidate = REV_RECONFIGURE;
+	    ds = ofproto->sflow = dpif_sflow_create();
+	    ofproto->backer->need_revalidate = REV_RECONFIGURE;
         }
+	dpif_sflow_set_port_lookup_callback(ds, dpif_sflow_port_lookup_callback, (void *)ofproto);
         dpif_sflow_set_options(ds, sflow_options);
+	HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
+	    dpif_sflow_add_port(ds, &ofport->up);
+	}
     } else {
         if (ds) {
             dpif_sflow_unref(ds);
@@ -3444,8 +3463,7 @@ handle_sflow_upcall(struct dpif_backer *backer,
 
     memset(&cookie, 0, sizeof cookie);
     memcpy(&cookie, nl_attr_get(upcall->userdata), sizeof cookie.sflow);
-    dpif_sflow_received(ofproto->sflow, upcall->packet, &flow,
-                        odp_in_port, &cookie);
+    dpif_sflow_received(ofproto->sflow, upcall->packet, &flow, &cookie);
 }
 
 static void
diff --git a/tests/odp.at b/tests/odp.at
index 469e120..7b5fef1 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -155,7 +155,7 @@ AT_SETUP([OVS datapath actions parsing and formatting - valid forms])
 AT_DATA([actions.txt], [dnl
 1,2,3
 userspace(pid=555666777)
-userspace(pid=6633,sFlow(vid=9,pcp=7,output=10))
+userspace(pid=6633,sFlow(vid=9,pcp=7,n_outputs=1,output_port=10))
 userspace(pid=9765,slow_path())
 userspace(pid=9765,slow_path(cfm))
 userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f))
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index f67c3ab..6be46e3 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1831,6 +1831,86 @@ IFCOUNTERS
 AT_CLEANUP
 
 
+dnl Test sFlow tunnel structures.
+AT_SETUP([ofproto-dpif - sFlow tunnel structures])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \
+                    options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \
+                    options:key=5 ofport_request=1\
+                    -- add-port br0 p2 -- set Interface p2 type=dummy \
+                    options:ifindex=1002 ofport_request=2])
+AT_DATA([flows.txt], [dnl
+actions=output:1
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
+		br0 65534/100: (dummy)
+		p1 1/1: (gre: key=5, local_ip=2.2.2.2, remote_ip=1.1.1.1)
+		p2 2/2: (dummy: ifindex=1002)
+])
+
+dnl set up sFlow logging
+dnl ON_EXIT([kill `cat test-sflow.pid`])
+AT_CHECK([test-sflow --log-file --detach --no-chdir --pidfile 0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([sflow.log])
+SFLOW_PORT=`parse_listening_port < test-sflow.log`
+ovs-appctl time/stop
+ovs-vsctl \
+   set Bridge br0 sflow=@sf -- \
+   --id=@sf create sflow targets=\"127.0.0.1:$SFLOW_PORT\" \
+     header=128 sampling=1 polling=0
+
+dnl use ofproto/trace to check the actions
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: sample(sample=100.0%,actions(userspace(pid=0,sFlow(vid=0,pcp=0,n_outputs=1,output_port=1)))),set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,tos=0x0,ttl=64,flags(df,key))),1
+])
+
+dnl use netdev-dummy/receive to send a packet that will be sampled
+AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout])
+
+dnl sleep long enough to get the sFlow datagram flushed out (may be delayed for up to 1 second)
+for i in `seq 1 30`; do
+    ovs-appctl time/warp 100
+done
+OVS_VSWITCHD_STOP
+ovs-appctl -t test-sflow exit
+
+AT_CHECK([[sort sflow.log | $EGREP 'HEADER|ERROR' | sed 's/ /\
+	/g']], [0], [dnl
+HEADER
+	dgramSeqNo=1
+	ds=127.0.0.1>2:1000
+	fsSeqNo=1
+	tunnel4_out_length=0
+	tunnel4_out_protocol=47
+	tunnel4_out_src=2.2.2.2
+	tunnel4_out_dst=1.1.1.1
+	tunnel4_out_src_port=0
+	tunnel4_out_dst_port=0
+	tunnel4_out_tcp_flags=0
+	tunnel4_out_tos=0
+	in_vlan=0
+	in_priority=0
+	out_vlan=0
+	out_priority=0
+	meanSkip=1
+	samplePool=1
+	dropEvents=0
+	in_ifindex=1002
+	in_format=0
+	out_ifindex=0
+	out_format=0
+	hdr_prot=1
+	pkt_len=64
+	stripped=4
+	hdr_len=60
+	hdr=50-54-00-00-00-07-50-54-00-00-00-05-08-00-45-04-00-28-00-00-00-00-80-06-B9-78-C0-A8-00-01-C0-A8-00-02-00-08-00-09-00-00-00-00-00-00-00-00-50-00-00-00-00-00-00-00-00-00-00-00-00-00
+])
+AT_CLEANUP
+
+
 
 dnl Test that basic NetFlow reports flow statistics correctly:
 dnl - The initial packet of a flow are correctly accounted.
diff --git a/tests/test-sflow.c b/tests/test-sflow.c
index cba01b9..bdcf3ef 100644
--- a/tests/test-sflow.c
+++ b/tests/test-sflow.c
@@ -56,6 +56,8 @@ static unixctl_cb_func test_sflow_exit;
 #define SFLOW_TAG_CTR_IFCOUNTERS 1
 #define SFLOW_TAG_PKT_HEADER 1
 #define SFLOW_TAG_PKT_SWITCH 1001
+#define SFLOW_TAG_PKT_TUNNEL4_OUT 1023
+#define SFLOW_TAG_PKT_TUNNEL4_IN 1024
 
 struct sflow_addr {
     enum {
@@ -99,6 +101,8 @@ struct sflow_xdr {
     struct {
         uint32_t HEADER;
         uint32_t SWITCH;
+	uint32_t TUNNEL4_OUT;
+	uint32_t TUNNEL4_IN;
         uint32_t IFCOUNTERS;
     } offset;
 
@@ -251,6 +255,23 @@ print_hex(const char *a, int len, char *buf, int bufLen)
     return b;
 }
 
+static void
+print_struct_ipv4(struct sflow_xdr *x, const char *prefix)
+{
+    printf(" %s_length=%"PRIu32,    prefix, sflowxdr_next(x));
+    printf(" %s_protocol=%"PRIu32,  prefix, sflowxdr_next(x));
+
+    uint32_t src = sflowxdr_next_n(x);
+    uint32_t dst = sflowxdr_next_n(x);
+    printf(" %s_src="IP_FMT,        prefix, IP_ARGS(src));
+    printf(" %s_dst="IP_FMT,        prefix, IP_ARGS(dst));
+
+    printf(" %s_src_port=%"PRIu32,  prefix, sflowxdr_next(x));
+    printf(" %s_dst_port=%"PRIu32,  prefix, sflowxdr_next(x));
+    printf(" %s_tcp_flags=%"PRIu32, prefix, sflowxdr_next(x));
+    printf(" %s_tos=%"PRIu32,       prefix, sflowxdr_next(x));
+}
+
 #define SFLOW_HEX_SCRATCH 1024
 
 static void
@@ -266,6 +287,16 @@ process_flow_sample(struct sflow_xdr *x)
                x->agentIPStr, x->dsClass, x->dsIndex);
         printf(" fsSeqNo=%"PRIu32, x->fsSeqNo);
 
+        if (x->offset.TUNNEL4_IN) {
+            sflowxdr_setc(x, x->offset.TUNNEL4_IN);
+	    print_struct_ipv4(x, "tunnel4_in");
+        }
+
+        if (x->offset.TUNNEL4_OUT) {
+            sflowxdr_setc(x, x->offset.TUNNEL4_OUT);
+	    print_struct_ipv4(x, "tunnel4_out");
+        }
+
         if (x->offset.SWITCH) {
             sflowxdr_setc(x, x->offset.SWITCH);
             printf(" in_vlan=%"PRIu32, sflowxdr_next(x));
@@ -442,6 +473,14 @@ process_datagram(struct sflow_xdr *x)
                     sflowxdr_mark_unique(x, &x->offset.SWITCH);
                     break;
 
+		case SFLOW_TAG_PKT_TUNNEL4_OUT:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL4_OUT);
+                    break;
+
+		case SFLOW_TAG_PKT_TUNNEL4_IN:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL4_IN);
+                    break;
+
                     /* Add others here... */
                 }
 
-- 
1.8.1.4




More information about the dev mailing list