[PATCH] lib: Update ce-mask to uint64_t

[David Ahern]

On 12/18/15 1:16 AM, Thomas Haller wrote:
> Hi David,
>
>
> On Wed, 2015-12-16 at 13:11 -0800, David Ahern wrote:
>> lib/route/link.c already defines 32 attributes which fills the
>> current
>> uint32_t used for ce_mask. To accommodate more attributes the mask
>> needs
>> to be expanded. This patch updates the definition to uint64_t and
>> fixes
>> callers and implementations to consistently use uint64_t data type.
>
>
>>   extern void *			rtnl_tc_data(struct rtnl_tc *);
>> diff --git a/include/netlink/object.h b/include/netlink/object.h
>> index a95feda7b4db..81a1578bb22c 100644
>> --- a/include/netlink/object.h
>> +++ b/include/netlink/object.h
>> @@ -41,7 +41,7 @@ extern void			nl_object_dump(s
>> truct nl_object *,
>>   extern void			nl_object_dump_buf(struct
>> nl_object *, char *, size_t);
>>   extern int			nl_object_identical(struct
>> nl_object *,
>>   						    struct nl_object
>> *);
>> -extern uint32_t			nl_object_diff(struct
>> nl_object *,
>> +extern uint64_t			nl_object_diff(struct
>> nl_object *,
>>
>
> This changes public API. Cannot do that. Maybe add a nl_object_diff64()
> -- as ugly as it is...

That crossed my mind but I was not sure how to handle the existing API 
-- a way to indicate that there are more than 32 attributes and a higher 
one differed.

It also struck me as odd that the return is a bitmask with a bit per 
attribute that differed. How can that be used since the attributes are 
hidden inside of the code?

> Too bad we already used:
> #define LINK_ATTR_LINK_NETNSID  (1 << 31)
>
> I missed that. Otherwise we could reserve the highest bit for ~there
> are some other flags above 32 bit~.
>
>
> like:
>
> uint32_t nl_object_diff(struct nl_object *obj1, ... obj2)
> {
>      uint64_t diff;
>
>      diff = nl_object_diff64 (obj1, obj2);
>
>      return (diff & ~((uint64_t 0xFFFFFFFF))
>             ? (uint32_t) diff | (1 << 31)
>             : (uint32_t) diff;
> }
>
>
> Maybe we just change LINK_ATTR_LINK_NETNSID to be (1 << 32) and do it
> anyway? The actual numeric values of LINK_ATTR_* are anyway private
> API.

Ok, that's an idea - mark the high bit as 'other';


>
> Otherwise this patch changes ABI for internal API in a somewhat crucial
> way. I guess that's OK and no user in the wild depends on that,
> right(??)

No way to really know. I'll spin a v2 with the above.