On Wed, Aug 10, 2011 at 12:42 PM, Ben Pfaff <b...@nicira.com> wrote: > 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 <pshe...@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.
I had pretty much exactly the same thoughts. The rough goal that I was thinking of (although certainly without all of the details fleshed out) is to have a sample action which takes a probability and sublist of actions which is executed according to the probability. The existing sFlow support would then be replaced in the kernel by prepending sample(probability, action:userspace(identifier)) to to the normal list of actions. Of course, it could also be used to mirror to a fraction of traffic, etc. Before this can be done, there are two things that need to be removed or cleaned up: the action list and the sample pool. I think this patch was intended to be a step towards the former but it still needs a little work. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
