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

Reply via email to