From 743cea45434a5a3c6560a73f29632c6352874c61 Mon Sep 17 00:00:00 2001 From: Neil Mckee Date: Tue, 30 Apr 2013 22:38:53 -0700 Subject: [PATCH] Change sFlow model to reflect per-bridge sampling Until now, we were presenting a separate sFlow data-source (sampler) for each ifIndex-interface. This caused problems with samples that did not easily map to an ifIndex being aliased together and breaking the sFlow containment rules. This patch changes the model to present a single sFlow data-source for each bridge. Now we can still make all reasonable effort to map packet samples to ingress/egress ifIndex numbers, knowing that the fallback to "unknown" does not break the sFlow model. Note that interface-counter-polling is still handled the same way as before, with sFlow counter-polling data only being exported for ifIndex-interfaces. Signed-off-by: Neil Mckee Signed-off-by: Ben Pfaff --- lib/sflow.h | 7 ++++ ofproto/ofproto-dpif-sflow.c | 80 ++++++++++++++++++------------------ ofproto/ofproto-dpif.c | 6 ++- ofproto/tunnel.c | 1 - tests/ofproto-dpif.at | 48 +++++++++++----------- 5 files changed, 75 insertions(+), 67 deletions(-) diff --git a/lib/sflow.h b/lib/sflow.h index 8ea96937f..0d1f2b9cd 100644 --- a/lib/sflow.h +++ b/lib/sflow.h @@ -8,6 +8,13 @@ #ifndef SFLOW_H #define SFLOW_H 1 +typedef enum { + SFL_DSCLASS_IFINDEX = 0, + SFL_DSCLASS_VLAN = 1, + SFL_DSCLASS_PHYSICAL_ENTITY = 2, + SFL_DSCLASS_LOGICAL_ENTITY = 3 +} SFL_DSCLASS; + enum SFLAddress_type { SFLADDRESSTYPE_IP_V4 = 1, SFLADDRESSTYPE_IP_V6 = 2 diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c index 69362ab84..9ad0eaf27 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -341,39 +341,32 @@ dpif_sflow_add_poller(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) sfl_poller_set_bridgePort(poller, dsp->odp_port); } -static void -dpif_sflow_add_sampler(struct dpif_sflow *ds, struct dpif_sflow_port *dsp) -{ - SFLSampler *sampler = sfl_agent_addSampler(ds->sflow_agent, &dsp->dsi); - sfl_sampler_set_sFlowFsPacketSamplingRate(sampler, ds->options->sampling_rate); - sfl_sampler_set_sFlowFsMaximumHeaderSize(sampler, ds->options->header_len); - sfl_sampler_set_sFlowFsReceiver(sampler, RECEIVER_INDEX); -} - void dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, uint32_t odp_port) { struct dpif_sflow_port *dsp; - uint32_t ifindex; + int ifindex; dpif_sflow_del_port(ds, odp_port); - /* Add to table of ports. */ - dsp = xmalloc(sizeof *dsp); ifindex = netdev_get_ifindex(ofport->netdev); + if (ifindex <= 0) { - ifindex = (ds->sflow_agent->subId << 16) + odp_port; + /* Not an ifindex port, so do not add a cross-reference to it here */ + return; } + + /* Add to table of ports. */ + dsp = xmalloc(sizeof *dsp); dsp->ofport = ofport; dsp->odp_port = odp_port; - SFL_DS_SET(dsp->dsi, 0, ifindex, 0); + SFL_DS_SET(dsp->dsi, SFL_DSCLASS_IFINDEX, ifindex, 0); hmap_insert(&ds->ports, &dsp->hmap_node, hash_int(odp_port, 0)); - /* Add poller and sampler. */ + /* Add poller. */ if (ds->sflow_agent) { dpif_sflow_add_poller(ds, dsp); - dpif_sflow_add_sampler(ds, dsp); } } @@ -406,6 +399,9 @@ dpif_sflow_set_options(struct dpif_sflow *ds, SFLReceiver *receiver; SFLAddress agentIP; time_t now; + SFLDataSource_instance dsi; + uint32_t dsIndex; + SFLSampler *sampler; if (sset_is_empty(&options->targets) || !options->sampling_rate) { /* No point in doing any work if there are no targets or nothing to @@ -473,10 +469,20 @@ dpif_sflow_set_options(struct dpif_sflow *ds, /* Set the sampling_rate down in the datapath. */ ds->probability = MAX(1, UINT32_MAX / ds->options->sampling_rate); - /* Add samplers and pollers for the currently known ports. */ + /* Add a single sampler for the bridge. This appears as a PHYSICAL_ENTITY + because it is associated with the hypervisor, and interacts with the server + hardware directly. The sub_id is used to distinguish this sampler from + others on other bridges within the same agent. */ + dsIndex = 1000 + options->sub_id; + SFL_DS_SET(dsi, SFL_DSCLASS_PHYSICAL_ENTITY, dsIndex, 0); + sampler = sfl_agent_addSampler(ds->sflow_agent, &dsi); + sfl_sampler_set_sFlowFsPacketSamplingRate(sampler, ds->options->sampling_rate); + 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); - dpif_sflow_add_sampler(ds, dsp); } } @@ -499,43 +505,35 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet, SFLFlow_sample_element switchElem; SFLSampler *sampler; struct dpif_sflow_port *in_dsp; - struct netdev_stats stats; ovs_be16 vlan_tci; - int error; - /* Build a flow sample */ - memset(&fs, 0, sizeof fs); - - in_dsp = dpif_sflow_find_port(ds, odp_in_port); - if (!in_dsp) { + sampler = ds->sflow_agent->samplers; + if (!sampler) { return; } - fs.input = SFL_DS_INDEX(in_dsp->dsi); - error = ofproto_port_get_stats(in_dsp->ofport, &stats); - if (error) { - VLOG_WARN_RL(&rl, "netdev get-stats error %s", strerror(error)); - return; - } - fs.sample_pool = stats.rx_packets; + /* Build a flow sample. */ + memset(&fs, 0, sizeof fs); - /* We are going to give it to the sampler that represents this input port. - * By implementing "ingress-only" sampling like this we ensure that we - * never have to offer the same sample to more than one sampler. */ - sampler = sfl_agent_getSamplerByIfIndex(ds->sflow_agent, fs.input); - if (!sampler) { - VLOG_WARN_RL(&rl, "no sampler for input ifIndex (%"PRIu32")", - fs.input); - return; + /* 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); } + /* Make the assumption that the random number generator in the datapath converges + * to the configured mean, and just increment the samplePool by the configured + * sampling rate every time. */ + sampler->samplePool += sfl_sampler_get_sFlowFsPacketSamplingRate(sampler); + /* Sampled header. */ memset(&hdrElem, 0, sizeof hdrElem); hdrElem.tag = SFLFLOW_HEADER; header = &hdrElem.flowType.header; header->header_protocol = SFLHEADER_ETHERNET_ISO8023; /* The frame_length should include the Ethernet FCS (4 bytes), - but it has already been stripped, so we need to add 4 here. */ + * but it has already been stripped, so we need to add 4 here. */ header->frame_length = packet->size + 4; /* Ethernet FCS stripped off. */ header->stripped = 4; diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 6ec1c23d5..89a4668d9 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -1789,7 +1789,11 @@ port_construct(struct ofport *port_) port->carrier_seq = netdev_get_carrier_resets(netdev); if (netdev_vport_is_patch(netdev)) { - /* XXX By bailing out here, we don't do required sFlow work. */ + /* By bailing out here, we don't submit the port to the sFlow module + * to be considered for counter polling export. This is correct + * because the patch port represents an interface that sFlow considers + * to be "internal" to the switch as a whole, and therefore not an + * candidate for counter polling. */ port->odp_port = OVSP_NONE; return 0; } diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index 8aa7fbea2..8d29184f1 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -32,7 +32,6 @@ /* XXX: * - * Ability to generate metadata for packet-outs * Disallow netdevs with names like "gre64_system" to prevent collisions. */ VLOG_DEFINE_THIS_MODULE(tunnel); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index d70977374..2b9df962a 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1252,7 +1252,7 @@ AT_CHECK([[sort sflow.log | $EGREP 'HEADER|ERROR' | sed 's/ /\ /g']], [0], [dnl HEADER dgramSeqNo=1 - ds=127.0.0.1>0:1003 + ds=127.0.0.1>2:1000 fsSeqNo=1 in_vlan=0 in_priority=0 @@ -1261,7 +1261,7 @@ HEADER meanSkip=1 samplePool=1 dropEvents=0 - in_ifindex=1003 + in_ifindex=1004 in_format=0 out_ifindex=2 out_format=2 @@ -1269,10 +1269,10 @@ HEADER pkt_len=64 stripped=4 hdr_len=60 - hdr=FF-FF-FF-FF-FF-FF-50-54-00-00-00-07-08-06-00-01-08-00-06-04-00-01-50-54-00-00-00-07-C0-A8-00-01-00-00-00-00-00-00-C0-A8-00-02-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 + hdr=FF-FF-FF-FF-FF-FF-50-54-00-00-00-05-08-06-00-01-08-00-06-04-00-01-50-54-00-00-00-05-C0-A8-00-02-00-00-00-00-00-00-C0-A8-00-01-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 HEADER dgramSeqNo=1 - ds=127.0.0.1>0:1003 + ds=127.0.0.1>2:1000 fsSeqNo=2 in_vlan=0 in_priority=0 @@ -1283,16 +1283,16 @@ HEADER dropEvents=0 in_ifindex=1003 in_format=0 - out_ifindex=1004 - out_format=0 + out_ifindex=2 + out_format=2 hdr_prot=1 pkt_len=64 stripped=4 hdr_len=60 - hdr=50-54-00-00-00-05-50-54-00-00-00-07-08-00-45-00-00-1C-00-00-00-00-40-01-F9-8D-C0-A8-00-02-C0-A8-00-01-00-00-FF-FF-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 + hdr=FF-FF-FF-FF-FF-FF-50-54-00-00-00-07-08-06-00-01-08-00-06-04-00-01-50-54-00-00-00-07-C0-A8-00-01-00-00-00-00-00-00-C0-A8-00-02-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 HEADER dgramSeqNo=1 - ds=127.0.0.1>0:1003 + ds=127.0.0.1>2:1000 fsSeqNo=3 in_vlan=0 in_priority=0 @@ -1301,55 +1301,55 @@ HEADER meanSkip=1 samplePool=3 dropEvents=0 - in_ifindex=1003 + in_ifindex=1004 in_format=0 - out_ifindex=1004 + out_ifindex=1003 out_format=0 hdr_prot=1 pkt_len=64 stripped=4 hdr_len=60 - hdr=50-54-00-00-00-05-50-54-00-00-00-07-86-DD-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 + hdr=50-54-00-00-00-07-50-54-00-00-00-05-08-00-45-00-00-1C-00-00-00-00-40-01-F9-8D-C0-A8-00-01-C0-A8-00-02-08-00-F7-FF-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 HEADER dgramSeqNo=1 - ds=127.0.0.1>0:1004 - fsSeqNo=1 + ds=127.0.0.1>2:1000 + fsSeqNo=4 in_vlan=0 in_priority=0 out_vlan=0 out_priority=0 meanSkip=1 - samplePool=1 + samplePool=4 dropEvents=0 - in_ifindex=1004 + in_ifindex=1003 in_format=0 - out_ifindex=2 - out_format=2 + out_ifindex=1004 + out_format=0 hdr_prot=1 pkt_len=64 stripped=4 hdr_len=60 - hdr=FF-FF-FF-FF-FF-FF-50-54-00-00-00-05-08-06-00-01-08-00-06-04-00-01-50-54-00-00-00-05-C0-A8-00-02-00-00-00-00-00-00-C0-A8-00-01-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 + hdr=50-54-00-00-00-05-50-54-00-00-00-07-08-00-45-00-00-1C-00-00-00-00-40-01-F9-8D-C0-A8-00-02-C0-A8-00-01-00-00-FF-FF-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 HEADER dgramSeqNo=1 - ds=127.0.0.1>0:1004 - fsSeqNo=2 + ds=127.0.0.1>2:1000 + fsSeqNo=5 in_vlan=0 in_priority=0 out_vlan=0 out_priority=0 meanSkip=1 - samplePool=2 + samplePool=5 dropEvents=0 - in_ifindex=1004 + in_ifindex=1003 in_format=0 - out_ifindex=1003 + out_ifindex=1004 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-00-00-1C-00-00-00-00-40-01-F9-8D-C0-A8-00-01-C0-A8-00-02-08-00-F7-FF-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 + hdr=50-54-00-00-00-05-50-54-00-00-00-07-86-DD-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00 ]) AT_CHECK([[sort sflow.log | $EGREP 'IFCOUNTERS|ERROR' | head -6 | sed 's/ /\ -- 2.20.1