I learned how to squash patches...
Change sFlow model to reflect per-bridge sampling. Before we were presenting a
separate
sFlow data-source (sampler) for each ifIndex-interface, and it was causing
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 <neil.mc...@inmon.com>
---
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 8ea9693..0d1f2b9 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..9ad0eaf 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 3ae3532..4306d40 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1782,7 +1782,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 8aa7fbe..8d29184 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 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
