Finally getting back to this. Can we do it one piece at a time? For example, attached is a patch that just adds counters to the LACP module and a call to retrieve them.
Neil ------ Neil McKee InMon Corp. http://www.inmon.com On Mon, Jan 13, 2014 at 8:29 AM, Neil McKee <neil.mc...@inmon.com> wrote: > Thanks for the detailed explanation. You should teach a class :) I'll > rework the patch. > > Neil > > > > ------ > Neil McKee > InMon Corp. > http://www.inmon.com > > > On Thu, Jan 9, 2014 at 11:43 AM, Ben Pfaff <b...@nicira.com> wrote: >> >> On Fri, Jan 03, 2014 at 09:39:00PM -0800, Neil McKee wrote: >> > On Mon, Dec 30, 2013 at 4:08 PM, Ben Pfaff <b...@nicira.com> wrote: >> > > I'm not a fan of callbacks because of the surprising inversion of >> > > control that they cause, and especially not of layered callbacks like >> > > the one that this introduces. Instead of calling back into >> > > dpif_sflow_port_lookup_callback() from sflow_agent_get_counters(), I >> > > think that it would be better to provide the necessary information in >> > > the call to dpif_sflow_add_port(). >> > > >> > > >> > I wasn't expecting you to say that. The purpose of the callback is >> > really just to access the state held within ofproto-dpif, but in an >> > encapsulated way so that the exposure is limited and controlled. >> > The alternatives were (1) to make those private structure fields >> > visible to the ofproto-dpif-sflow module so it could just read them >> > directly, or (2) to copy all the relevant state into a separate >> > hash-table just for ofproto-dpif-sflow to use when processing packet >> > samples. It sounds like you prefer (2)? To understand why I think >> > I just need to know what you mean by "surprising inversion of control". >> > Are you referring to the possibility of mutex contention/deadlock >> > if the data-structures within ofproto-dpif are interrogated from >> > within the packet-processing path (i.e. when processing an >> > sFlow packet sample)? >> >> Let me step back and explain, since my phrasing didn't help. I'm >> going to explain one of my own views of program structure. Most of it >> is opinion, not fact, so bear with me a little. >> >> The call stack in a well-structured large program usually follows a >> tree structure from one logical module (often, one source file) to >> another. Often, you can consider one module to be "higher level" or >> "lower level" than another based on the rule that higher level modules >> call into lower level modules, but not vice versa. Sometimes calls go >> both ways, and in my experience that is undesirable because it makes >> the structure of the program harder to understand. Direct calls are >> one thing, because it is easy enough to trace through them with "grep" >> and other tools, but indirect calls through callbacks are harder to >> understand. Actually, there are at least two classes of callbacks: >> callbacks that you pass into a function for that function to call, and >> callbacks that you store somewhere in data owned by a module that in >> theory any function in that module *could* call (even if it doesn't >> currently). The latter are harder to understand. >> >> My personal idea of nightmarish tangles of callbacks is GObject and in >> particular GTK+. Everything is a callback and it's very difficult to >> figure out exactly when one of them could get called. You end up >> writing bad code because you can't figure it out. (GTK+ has other >> problems too, that's just one of them.) >> >> Looking at the callback in question again, it's going to be difficult >> to maintain for a reason that you basically called out in the comment, >> that is, locking. In most places in OVS we are able to use Clang >> thread safety annotations to statically check (to some basic level of >> safety, anyhow) that locks are held or not held at various places. >> But those checks can't flow through callback functions: >> >> /* XXX do we need to acquire ofproto_mutex here? Or should it be >> acquired in handle_sflow_upcall() below, >> * so that it will not be released until after the sFlow-module has >> followed the pointers we are giving >> * it here and the sflow sample has been fully processed? */ >> >> So I'd prefer, if we can avoid it, to use some interface more direct >> than a callback. > >
0001-Add-lacp-couters-and-a-call-to-retrieve-them.patch
Description: Binary data
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
