Adjust sFlow packet samplers to better reflect the underlying per-bridge sampling that is really happening, rather than modeling it as per-interface sampling. This fixes an aliasing issue: tunnel traffic sampled on different bridges was all being reported against the same sFlow datasource.
The sFlow counter samples remain unchanged, reporting only on interfaces that have valid ifIndex numbers. See definition of SFlowDataSource in http://sflow.org/sflow_version_5.txt. Signed-off-by: Neil McKee <neil.mc...@inmon.com> --- lib/sflow.h | 7 ++++ ofproto/ofproto-dpif-sflow.c | 76 +++++++++++++++++++++----------------------- tests/ofproto-dpif.at | 48 ++++++++++++++-------------- 3 files changed, 68 insertions(+), 63 deletions(-) diff --git a/lib/sflow.h b/lib/sflow.h index 8ea9693..4a222d2 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 69362ab..6521d10 100644 --- a/ofproto/ofproto-dpif-sflow.c +++ b/ofproto/ofproto-dpif-sflow.c @@ -341,15 +341,6 @@ 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) @@ -359,21 +350,23 @@ dpif_sflow_add_port(struct dpif_sflow *ds, struct ofport *ofport, 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; + + if ((int32_t)ifindex <= 0) { + /* 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,35 +505,27 @@ 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; + + sampler = ds->sflow_agent->samplers; + if(!sampler) { + return; + } /* 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) { - return; + if(in_dsp) { + fs.input = SFL_DS_INDEX(in_dsp->dsi); } - 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; - - /* 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; - } + /* 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); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index b1d2f26..b813fd0 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/ /\ -- 1.8.1.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
