[ovs-dev] [PATCH] Initial implementation of sFlow.

[Neil McKee]

The containment rule is that a given sflow-datasource (sampler or poller) should be scoped within only one sflow-agent (or sub-agent).   So the issue arrises when you have two switches/datapaths defined on the same host being managed with the same IP address:  each switch is a separate sub-agent,  so they can run independently (e.g. with their own sequence numbers) but they can't both claim to speak for the same sflow-datasource.   Specifically, they can't both represent the <ifindex>:0 data-source.   This containment rule is necessary so that the sFlow collector can scale and combine the results accurately.

One option would be to stick with the <ifindex>:0 data-source but elevate it to be global across all bridges,  with a global sample_pool and a global sflow_agent.  Not tempting.  Better to go the other way and allow each interface to have it's own sampler, just as it already has it's own poller.   The ifIndex numbers are globally unique across all switches/datapaths on the host,  so the containment is now clean.  Datasource <ifindex>:5 might be on one switch,  whille <ifindex>:7 can be on another.   Other benefits are that 1) you can support the option of overriding the default sampling-rate on an interface-by-interface basis,  and 2) this is how most sFlow implementations are coded,  so there will be no surprises or interoperability issues with any sFlow collectors out there.

As you can see from the patch I sent you,  it dropped in quite cleanly in ofproto-sflow.c.  The part I expect you to examine more carefully is in the datapath,  where now you are asked to maintain a separate sample_pool for each input port.  I wasn't sure of the best way to get a pointer back to the input port structure there.

A sample_pool counter like this might end up tracking pretty closely to the interface counters (inbound unicasts + inbound multicasts + inbound broadcasts on that port) but it's hard to be sure, so it's safer to just accumulate your own counter right at the point where you know for sure that the packet was real, made it this far,  and was eligible for sampling.

One question that came up here --  is there already some synchronization that prevents multiple CPUs from processing packets from the same input port in parallel (to avoid reordering)?  If so,  maybe you don't even need per-cpu counters? 

Regards,
Neil



We have been using the <ifIndex>:0 sampler to represent the whole

On Jan 11, 2010, at 12:02 PM, Ben Pfaff wrote:

> On Thu, Jan 07, 2010 at 04:08:40PM -0800, Neil McKee wrote:
>>> +    /* Add a single sampler to represent the whole switch (special <ifIndex>:0
>>> 
>>> The "whole switch" is a bridge/datapath correct?  This sounds to me
>>> like it's representing the entire physical box, which I don't think
>>> is true.
>> 
>> Oh, you are right.  If there were multiple bridges all reporting on
>> the same <ifIndex>:0 datasource using the same agent IP address then
>> it would violate the containment rules.  After some discussion here it
>> seems that the cleanest solution would be to leave most things exactly
>> where they are, but make the following tweaks:
>> 
>> 1. maintain separate sample-pool counters for each port/cpu rather
>> than just one per datapath/cpu.  In other words, move the "sflow_pool"
>> counter out of dp_stats_percpu and into net_bridge_port (in
>> datapath.h).  If you increment the pool for the ingress port in each
>> case, then it should be almost the same number of instructions, right?
>> 2.  pick the relevant sflow_pool counter in actions.c:sflow_sample()
>> 3.  in ofproto-sflow.c, add a separate sampler object for each
>> ifindex, exactly as you do with ofproto_sflow_add_poller().
>> 4.  when a sample is passed to ofproto_sflow_received(), make sure you
>> give it to the sampler that represents that ingress port.
>> 
>> I'm sorry I didn't see this coming before.  On the bright side,
>> however, this would make it easier to support a different
>> sampling-rate for each interface, if you found that you wanted to do
>> that in the future.  It would also make your implementation look
>> exactly like the majority that exist in the wild, so interoperability
>> with any sFlow collector would be pretty much guaranteed.
> 
> Hi Neil.
> 
> Thanks for the patch (in your later email).  But I'd like to understand
> the situation better before I apply it.
> 
> Are you (and Justin) saying that distinguishing datapaths based on the
> sub-agent ID is insufficient from an sFlow perspective?  Can you explain
> why?  I do not understand yet.

----
Neil McKee
InMon Corp.
http://www.inmon.com