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

[Ben Pfaff]

On Sat, Jun 06, 2015 at 02:19:03PM -0700, Jarno Rajahalme wrote:
> 
> > On Jun 6, 2015, at 2:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > On Fri, May 29, 2015 at 03:09:05PM -0700, Joe Stringer wrote:
> >> 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?)
> > 
> > Sorry about my delayed response.
> > 
> > Hmm.  I see a useful likeness between UUIDs and UFIDs, because they're
> > both essentially random numbers (although a hash is somewhat different
> > from a random UUID).  I don't know enough about conn_labels to know
> > whether they're also the same kind of thing.  Do they contain structured
> > data (e.g. extracted from a flow) or are they like UUIDs and UFIDs?  If
> > conn_labels qualitatively different, then maybe they should not be
> > formatted the same way.
> 
> Conn labels is a set of typically individual bits, meaning of which depends on the system/controller configuration.

That seems quite different from UUID/UFIDs to me then, and I wouldn't
want to lump them together just because they're all the same size.