Applied to master and branch-1.11, thanks. Do we need this on
branch-1.10 also?
On Tue, Apr 30, 2013 at 10:38:53PM -0700, Neil Mckee wrote:
> 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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
