[ovs-dev] [PATCH V5 0/2] Do not rewrite fields with the same values as matched

[Eli Britstein]

On 2/25/2019 7:31 PM, Ben Pfaff wrote:
> On Sun, Feb 24, 2019 at 09:45:21AM +0000, Eli Britstein wrote:
>> On 2/22/2019 6:42 PM, Ben Pfaff wrote:
>>> On Sun, Feb 17, 2019 at 09:18:46AM +0000, Eli Britstein wrote:
>>>> This patch set avoids unnecessary rewrite actions to fields with the
>>>> same values as matched on.
>>>>
>>>> Patch 1 is a pre-step of generating ovs key fields macros
>>>> Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly
>>> Thanks for the revision.
>>>
>>> Do you foresee other uses of OVS_KEY_FIELD in the future?  As is,
>>> there's a lot of duplication here from the numerous declarations like
>>>
>>>       struct ovs_key_field_properties ovs_key_nd_extensions_properties[] = {
>>> #define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_nd_extensions, name), sizeof(type)},
>>>           OVS_KEY_ND_EXTENSIONS_FIELDS
>>>           {0, 0}
>>> #undef OVS_KEY_FIELD
>>>       };
>>>
>>> If this is the only currently foreseen use, it would be better to have
>>> the code generator just generate the declarations directly instead of
>>> forcing these later duplications.
>> Please note the definitions are not exactly duplicated - each define
>> refers to a different struct. Currently I don't know what other uses
>> will be needed/done in the future. However, I think we should remain
>> with more generic and flexible code, that will allow easier use of those
>> macros in the future rather than specific macros.
> What uses do you foresee in the future?

I don't foresee the future. I just think we should keep the code as 
generic as possible, so future implementations are easier to reuse the 
same infrastructure.

Just for example, I can think of a log/debug implementation to print a 
struct:

suppose there is a function: void print_hex(char *buf, int len)

and you want to print a struct ovs_key_ipv4 ip:

#define OVS_KEY_FIELD(type, name) print_hex(((char*)ip)+offsetof(struct 
ovs_key_ipv4, name), sizeof(type))

OVS_KEY_IPV4_FIELDS - will print all the fields, and you can also add a 
title etc.