On Tue, Sep 27, 2011 at 4:56 PM, Pravin Shelar <[email protected]> wrote: > This is incremental patch to sFlow patch posted. Fixed according to > comments from Jesse. > - Fixed use-after-free bug in sample() > - Fixed comments > - Controller generated packets at undefined in_port are not > sampled > > Signed-off-by: Pravin Shelar <[email protected]>
Thanks, these changes look good to me. Ben, do you have any further comments? With the patch as is, there are two areas where the sample pool does not match what is sampled. Both of these are planned to be addressed anyways to deal with other issues. I'm somewhat concerned about them since they are regressions from the current sFlow implementation. However, since at least one of them is also an issue with our general stats handling it's not really an sFlow issue and perhaps shouldn't hold up this patch. They are: * Packets entirely synthesized by userspace/controller can be sampled but are not counted as part of the appropriate port. Fix is to count them, which is probably the generally correct thing to do in all situations. * Protocol packets consumed by userspace are counted but cannot be sampled. Fix is to install flows for these packets instead of simply allowing them to continue missing, which also makes it easier to tell what is going on and enables giving them higher priority. * Bonus question: what about packets which are sent to the controller on a flow table miss? This is essentially a special case of the previous issue. Perhaps we should install a send-to-controller flow? This woud also avoid searching the classifier each time. Actually, I see there is a comment roughly to this effect already. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
