Thinking about this a little more, I could make the debug interface set a single static 'enable_ufid' variable, then have a helper function like so:
+static bool +udpif_use_ufid(struct udpif *udpif) +{ + return enable_ufid && ofproto_dpif_get_enable_ufid(udpif->backer); +} I don't feel strongly about it, though this is perhaps a little more intuitive. On 18 December 2014 at 16:42, Joe Stringer <joestrin...@nicira.com> wrote: > Almost; theoretically not all udpifs will have the same backer, each > of which may or may not support UFID. So, it's still useful to track > this per-udpif. (One example is right now if you add a userspace > datapath and a kernel datapath, the userspace will support UFID but > the kernel will not.) > > The rest I think can be implemented by just performing the check > before setting udpif->enable_ufid to true in the debug interface, > incremental below: > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index f0cd4cc..46e6ce0 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -2087,11 +2087,20 @@ upcall_unixctl_enable_ufid(struct unixctl_conn > *conn, int argc OVS_UNUSED, > const char *argv[] OVS_UNUSED, void *aux > OVS_UNUSED) > { > struct udpif *udpif; > + bool error = false; > > LIST_FOR_EACH (udpif, list_node, &all_udpifs) { > - atomic_store(&udpif->enable_ufid, true); > + if (ofproto_dpif_get_enable_ufid(udpif->backer)) { > + atomic_store(&udpif->enable_ufid, true); > + } else { > + error = true; > + } > + } > + if (error) { > + unixctl_command_reply_error(conn, "One or more datapaths > don't support UFID"); > + } else { > + unixctl_command_reply(conn, "Datapath dumping tersely using > UFID enabled"); > } > - unixctl_command_reply(conn, "Datapath dumping tersely using UFID > enabled"); > } > > /* Set the flow limit. > > On 18 December 2014 at 16:15, Andy Zhou <az...@nicira.com> wrote: >> It seems we can do away with udpif->enable_ufid field. udpif has >> access to the backer, so it can know >> about the datapath capability. >> >> The debug interface just need to control another global 'enable' >> variable. ufid can only be use if both conditions >> are true. This will also remove the possibility of turning on udpif >> when the datapath does not support it. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev