[ovs-dev] [PATCH] Simplify kernel sFlow implementation

[Ben Pfaff]

On Tue, Aug 09, 2011 at 10:46:38AM -0700, pravin shelar wrote:
>  This patch is first step toward generic/simple sampling
>  action which will be completely independent of sflow.
>  Following patch cleanup sampling upcall by striping it down to
>  minimum essential data that userspace can not figure out for
>  itself.
> 
> Signed-off by: Pravin Shelar <pshelar at nicira.com>

(That's "-by", not " by".)

I like the general approach here--the sflow support is a big wart--but I
think that the userspace interface changes represent a regression.
Before, by passing the actions down to userspace, there was always a
consistent view of those actions.  With this change (as comments in the
code make clear) the actions might have changed or the facet might even
have been deleted.

A really good fix would probably require being able to somehow keep
old copies of flows that have changed around for a while in userspace,
link them to the sflow messages, and free them when they are known not
to be needed anymore.  I think that's probably feasible, but in fact I
think that we can avoid regression with something simpler.

Suppose we keep the "sample pool" netlink option that this patch
removes, and add a way for userspace to set a small amount of user data
on a flow, and pass that along as the "user data" netlink option.  64
bits of user data should be plenty of space to store what the userspace
sflow implementation actually needs (16 bits of final VLAN TCI plus 32
bits for sFlow's idea of output port).  Plus, those same 64 bits would
be more than enough for the sequence number that the "really good fix"
mentioned in the previous paragraph would probably need.

With the current patch, the obvious place to put this user data would be
in the flow itself.  Another approach (that you have probably already
considered) is to make sFlow sampling a flow action, in which case the
user data could be that action's argument.

Thanks,

Ben.