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

[Neil McKee]

At first glance it looks fine.   It's good that you can get away with just the atomic_inc().   If you push it then I will find time to test it tomorrow.

Neil


On Jan 20, 2010, at 1:55 PM, Ben Pfaff wrote:

> On Mon, Jan 11, 2010 at 12:53:28PM -0800, Neil McKee wrote:
>> 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.
> 
> OK.  This makes sense.
> 
> I've revised your patch a bit and sent it out as a separate followup to
> your email.  How does it look?  If you're happy with it, I'll test it
> slightly and push it to the branch.