git seemed to want to make this a series of 2 patches, and I don't know how to
argue with git. Hope that's OK.
Neil
Change sFlow packet sampling to better reflect the
per-bridge sampling that is really happening, rather than modeling it as
per-interface sampling. This fixes an aliasing problem that occurred when
tunnel traffic was sampled by multiple bridges. The sFlow counter samples
remain unchanged, reporting only on interfaces that have ifIndex numbers.
---
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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
