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

[Jesse Gross]

On Aug 19, 2011 7:20 AM, "Neil McKee" <neil.mckee at inmon.com> wrote:
>
>
> On Aug 18, 2011, at 4:21 AM, Jesse Gross wrote:
>
>> On Aug 18, 2011 2:51 AM, "Pravin Shelar" <pshelar at nicira.com> wrote:
>> >
>> > On Tue, Aug 16, 2011 at 11:17 PM, Jesse Gross <jesse at nicira.com> wrote:
>> > > On Mon, Aug 15, 2011 at 8:34 PM, pravin shelar <pshelar at nicira.com>
wrote:
>> > >>   This patch makes sampling support more generic/simple by adding
>> > >>  Datapath SAMPLING action. When sampling is turned on, a SAMPLING
>> > >>  action is added to all flows for given bridge.
>> > >>   This patch cleanup sampling upcall by striping it down to minimum
>> > >>  essential that userspace can not figure out for itself. Now sample
>> > >>  packet is sent along with actions checksum. So that sflow module
can
>> > >>  make sure it has consistent actions.
>> > >
>> > > I'm not sure that this does much to simplify or genericize sampling.
>> > > For the sample action itself, I was expecting something that could be
>> > > used to execute any set of actions with a given probability but this
>> > > is still tied towards sending packets to userspace with a fixed
>> > > probability across the datapath.  This would truly be generic and the
>> > > word "sFlow" would not appear in the kernel at all but could be
>> > > implemented entirely in userspace based on these primitives.  Does
>> > > this make sense to you?
>> >
>> > I think patch is first step toward that.
>> >
>> > My idea is to have sampling action with arguments like probability(P)
>> > and action(A). When probability(P) is hit, passed action (A) could be
>> > executed on that packet. But i dont like to generalize when there is
>> > only one use case at present where we need to assign probability. it
>> > just keeps things simple.
>>
>> I mentioned this on the phone but just for everybody else's benefit:
>>
>> My main goal here is to come up with an interface that is as close to
final as we can get it. While in general I agree with incremental work, in
this case I think we need to go the full way in regards to the interface.
>>
>>
>
> Yes.  It would a shame to only get half way and risk breaking the current
solution without quite nailing the more generic API either.
>
> So can we enumerate some of the use cases?
> Sampled-RSPAN?
> Random-forwarding for load-balancing?
>
> If you have multiple consumers for samples,  all with different
sampling-probabilities,  then obviously it's important that they can operate
efficiently and independently without treading on each others toes.   For
example,  you might only have to generate a random number once,  but then
several different actions might test it against different probability
thresholds.   Would "generate-a-random-number" now become an action too?
 (gosh,  the actions-list is starting to look like a FORTH program - not
that there's anything wrong with that!).  You would also have to be prepared
for the same packet to get sampled by different actions and copied onto
multiple channels to user-space.  You might want some of those channels to
be higher priority than others.   But I'm sure you've already thought of all
these ramifications.

These are all good points and I'm certainly not going to claim to have
thought through every single one of them or that the solution will be
perfectly extensible.  However, I actually think that it's an argument in
favor of doing it this way because we'll have a bunch of simple building
blocks rather than larger monolithic implementations of features.
Essentially, once the interface is locked down we can add new things but
never remove the old. It is less painful to do this if the units are
smaller.

Here's an example with your point about priority on the userspace channels:

Currently there's no priority when sending packets to userspace - it's first
come, first serve. Let's say that in the future, after we've locked down the
interface, we want to add that priority. In the current model, we'd have to
add something to sFlow and to every other channel that sends things to
userspace. After this change, we could add the priority argument to just the
userspace action and everything automatically picks it up. In this situation
I think the internal interfaces to each of these components are flexible
enough to be extended without creating entirely new versions. However, if
that wasn't the case then we just end up with a new userspace-priority
action instead of an entirely new sFlow interface and new interfaces for
everything else that sends packets to userspace.

> I'm still not persuaded that there's anything wrong with copying the
actions to user-space as you do now.  It can continue to be an annotation
that always goes along with any sampled packets that are going there.
 Stuffing that other 64-bit field into every rule -- just for sFlow -- seems
counter to the spirit of this streamlining effort.

The main issue that I have with sending the actions to userspace is that it
really shouldn't be necessary as userspace should already know the actions
since it controls them.  Given that, I'd like to eliminate it because an
interface that doesn't exist is an interface that we won't get stuck with
later on.

Obviously it isn't strictly true that userspace knows what actions were set
due to the asynchronous nature of updates.  But to resolve this, a userspace
controlled tag seems the simplest to me and can actually be more convienient
than the full actions list. Some examples that were mention before are where
userspace has more contextual information or where actions are flipping back
and forth.  You also need to do something like this anyways for the send to
controller OpenFlow action - the action list by itself is not sufficient.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20110819/6a5dd35e/attachment-0003.html>