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 <[email protected]> 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 <[email protected]> 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
