Re: [patch net-next 1/8] ynl-gen-c.py: fix rendering of validate field

[Jiri Pirko <jiri@resnulli.us>]

Tue, Aug 01, 2023 at 08:25:30PM CEST, kuba@kernel.org wrote:
>On Tue,  1 Aug 2023 16:19:00 +0200 Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> For split ops, do and dump has different value in validate field. Fix
>> the rendering so for do op, only "strict" is filled out and for dump op,
>> "strict" is prefixed by "dump-".
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>>  tools/net/ynl/ynl-gen-c.py | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
>> index 650be9b8b693..1c36d0c935da 100755
>> --- a/tools/net/ynl/ynl-gen-c.py
>> +++ b/tools/net/ynl/ynl-gen-c.py
>> @@ -1988,9 +1988,17 @@ def print_kernel_op_table(family, cw):
>>                  cw.block_start()
>>                  members = [('cmd', op.enum_name)]
>>                  if 'dont-validate' in op:
>> +                    dont_validate = []
>> +                    for x in op['dont-validate']:
>> +                        if op_mode == 'do' and x == 'dump':
>> +                            continue
>> +                        if op_mode == "dump" and x == 'strict':
>> +                            x = 'dump-' + x
>> +                        dont_validate.append(x)
>> +
>>                      members.append(('validate',
>>                                      ' | '.join([c_upper('genl-dont-validate-' + x)
>> -                                                for x in op['dont-validate']])), )
>> +                                                for x in dont_validate])), )
>>                  name = c_lower(f"{family.name}-nl-{op_name}-{op_mode}it")
>>                  if 'pre' in op[op_mode]:
>>                      members.append((cb_names[op_mode]['pre'], c_lower(op[op_mode]['pre'])))
>
>I was hoping we can delete GENL_DONT_VALIDATE_DUMP_STRICT
>but there is one cmd (TIPC_NL_LINK_GET) which
>sets GENL_DONT_VALIDATE_STRICT and nothing about the dump.

I need GENL_DONT_VALIDATE_STRICT for devlink dump selectors as well. I
don't want to break existing user that may pass garbage attributes.

>
>To express something like that we should add dump-strict as
>an allowed flag explicitly rather than doing the auto-prepending

Yeah, that was an option. But strict means GENL_DONT_VALIDATE_STRICT
for do and GENL_DONT_VALIDATE_DUMP_STRICT for dump. That is why I
decided to go this way.

I will add dump-strict if you prefer it, I don't care.

>-- 
>pw-bot: cr