[ovs-dev] [PATCH 1/3] odp-util: Refactor ovs_u128 handling functions.

[Joe Stringer]

Thanks for review,

On 29 May 2015 at 13:22, Ben Pfaff <blp at nicira.com> wrote:
> On Mon, Apr 13, 2015 at 05:56:15PM -0700, Joe Stringer wrote:
>> Signed-off-by: Joe Stringer <joestringer at nicira.com>
>
> The code in scan_u128() looks wrong to me: I don't see anything that
> makes the second call to ovs_scan(), to get the mask, skip past the
> value, e.g. by passing s + n to the second ovs_scan() or by advancing s
> with s += n.

You're right, I also used this for conn_label but I didn't attempt to
mask the connlabel. Also, mask doesn't make sense for UFID so it
wasn't tested there.

> I have another idea too: these issues for ovs_u128 are quite similar to
> the issues for UUIDs, which are also exactly 128 bits long and already
> have support for parsing, formatting, and so on, and furthermore have a
> distinctive format that is easily identifiable.  Have you considered
> using UUIDs as the representation for ufids?

I don't think I'd recognized that UUID was even present. I think it
makes sense to reuse the same for UUID and UFID.

My other aim was to make this usable by conn_label, which also implies
optional mask. Perhaps it's appropriate to morph this function into a
wrapper for the UUID parsing/formatting, with support for masks.
(Should *all* 128-bit values be printed like UUIDs, or just ID-like
information?)