[patch net-next] tools: ynl-gen: lift type requirement for attribute subsets

[patch net-next] tools: ynl-gen: lift type requirement for attribute subsets

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

In case an attribute is used in a subset, the type has to be currently
specified. As the attribute is already defined in the original set, this
is a redundant information in yaml file, moreover, may lead to
inconsistencies.

Example:
attribute-sets:
    ...
    name: pin
    enum-name: dpll_a_pin
    attributes:
      ...
      -
        name: parent-id
        type: u32
      ...
  -
    name: pin-parent-device
    subset-of: pin
    attributes:
      -
        name: parent-id
        type: u32             <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Remove the requirement from schema files to specify the "type" and add
check and bail out if "type" is not set.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 Documentation/netlink/genetlink-c.yaml      | 2 +-
 Documentation/netlink/genetlink-legacy.yaml | 2 +-
 Documentation/netlink/genetlink.yaml        | 2 +-
 Documentation/netlink/netlink-raw.yaml      | 2 +-
 tools/net/ynl/ynl-gen-c.py                  | 2 ++
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/netlink/genetlink-c.yaml b/Documentation/netlink/genetlink-c.yaml
index 9806c44f604c..80d8aa2708c5 100644
--- a/Documentation/netlink/genetlink-c.yaml
+++ b/Documentation/netlink/genetlink-c.yaml
@@ -142,7 +142,7 @@ properties:
           type: array
           items:
             type: object
-            required: [ name, type ]
+            required: [ name ]
             additionalProperties: False
             properties:
               name:
diff --git a/Documentation/netlink/genetlink-legacy.yaml b/Documentation/netlink/genetlink-legacy.yaml
index 12a0a045605d..2a21aae525a4 100644
--- a/Documentation/netlink/genetlink-legacy.yaml
+++ b/Documentation/netlink/genetlink-legacy.yaml
@@ -180,7 +180,7 @@ properties:
           type: array
           items:
             type: object
-            required: [ name, type ]
+            required: [ name ]
             additionalProperties: False
             properties:
               name:
diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
index 3d338c48bf21..9e8354f80466 100644
--- a/Documentation/netlink/genetlink.yaml
+++ b/Documentation/netlink/genetlink.yaml
@@ -115,7 +115,7 @@ properties:
           type: array
           items:
             type: object
-            required: [ name, type ]
+            required: [ name ]
             additionalProperties: False
             properties:
               name:
diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml
index 896797876414..9aeb64b27ada 100644
--- a/Documentation/netlink/netlink-raw.yaml
+++ b/Documentation/netlink/netlink-raw.yaml
@@ -187,7 +187,7 @@ properties:
           type: array
           items:
             type: object
-            required: [ name, type ]
+            required: [ name ]
             additionalProperties: False
             properties:
               name:
diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py
index 897af958cee8..b378412a9d7a 100755
--- a/tools/net/ynl/ynl-gen-c.py
+++ b/tools/net/ynl/ynl-gen-c.py
@@ -723,6 +723,8 @@ class AttrSet(SpecAttrSet):
             self.c_name = ''
 
     def new_attr(self, elem, value):
+        if 'type' not in elem:
+            raise Exception(f"Type has to be set for attribute {elem['name']}")
         if elem['type'] in scalars:
             t = TypeScalar(self.family, self, elem, value)
         elif elem['type'] == 'unused':
-- 
2.41.0

Re: [patch net-next] tools: ynl-gen: lift type requirement for attribute subsets

[Paolo Abeni]

On Tue, 2023-09-19 at 16:21 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> In case an attribute is used in a subset, the type has to be currently
> specified. As the attribute is already defined in the original set, this
> is a redundant information in yaml file, moreover, may lead to
> inconsistencies.
> 
> Example:
> attribute-sets:
>     ...
>     name: pin
>     enum-name: dpll_a_pin
>     attributes:
>       ...
>       -
>         name: parent-id
>         type: u32
>       ...
>   -
>     name: pin-parent-device
>     subset-of: pin
>     attributes:
>       -
>         name: parent-id
>         type: u32             <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
> 
> Remove the requirement from schema files to specify the "type" and add
> check and bail out if "type" is not set.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

What about updating accordingly the existing specs? They are used as
references, I think it would be better if the info there would be
consistent.

I think the tool still allows writing something alike:

    attributes:
      ...
      -
        name: parent-id
        type: u32
      ...
  -
    name: pin-parent-device
    subset-of: pin
    attributes:
      -
        name: parent-id
        type: string

(mismatching types). What about adding an explicit test to prevent
specifying again the types for already defined attributes?

Cheers,

Paolo

Re: [patch net-next] tools: ynl-gen: lift type requirement for attribute subsets

[Jiri Pirko]

Thu, Sep 28, 2023 at 12:15:22PM CEST, pabeni@redhat.com wrote:
>On Tue, 2023-09-19 at 16:21 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> In case an attribute is used in a subset, the type has to be currently
>> specified. As the attribute is already defined in the original set, this
>> is a redundant information in yaml file, moreover, may lead to
>> inconsistencies.
>> 
>> Example:
>> attribute-sets:
>>     ...
>>     name: pin
>>     enum-name: dpll_a_pin
>>     attributes:
>>       ...
>>       -
>>         name: parent-id
>>         type: u32
>>       ...
>>   -
>>     name: pin-parent-device
>>     subset-of: pin
>>     attributes:
>>       -
>>         name: parent-id
>>         type: u32             <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>> 
>> Remove the requirement from schema files to specify the "type" and add
>> check and bail out if "type" is not set.
>> 
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>What about updating accordingly the existing specs? They are used as
>references, I think it would be better if the info there would be
>consistent.

Okay, will check that out. I will try to update some.


>
>I think the tool still allows writing something alike:
>
>    attributes:
>      ...
>      -
>        name: parent-id
>        type: u32
>      ...
>  -
>    name: pin-parent-device
>    subset-of: pin
>    attributes:
>      -
>        name: parent-id
>        type: string
>
>(mismatching types). What about adding an explicit test to prevent
>specifying again the types for already defined attributes?

Good point. Will check.


>
>Cheers,
>
>Paolo
>