linux-kernel - Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Aug 2022 16:48:43 +0300
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Arınç ÜNAL <arinc.unal@...nc9.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Frank Wunderlich <frank-w@...lic-files.de>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Sander Vanheule <sander@...nheule.net>,
        René van Dorst <opensource@...rst.com>,
        Daniel Golle <daniel@...rotopia.org>, erkin.bozoglu@...ont.com,
        Sergio Paracuellos <sergio.paracuellos@...il.com>
Cc:     netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update
 json-schema

On 12/08/2022 16:41, Arınç ÜNAL wrote:
> On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
>> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
>>> On 12/08/2022 01:09, Arınç ÜNAL wrote:
>>>>>> -patternProperties:
>>>>>> -  "^(ethernet-)?ports$":
>>>>>> -    type: object
>>>>>
>>>>> Actually four patches...
>>>>>
>>>>> I don't find this change explained in commit msg. What is more, it looks
>>>>> incorrect. All properties and patternProperties should be explained in
>>>>> top-level part.
>>>>>
>>>>> Defining such properties (with big piece of YAML) in each if:then: is no
>>>>> readable.
>>>>
>>>> I can't figure out another way. I need to require certain properties for
>>>> a compatible string AND certain enum/const for certain properties which
>>>> are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
>>>> the compatible string.
>>>
>>> requiring properties is not equal to defining them and nothing stops you
>>> from defining all properties top-level and requiring them in
>>> allOf:if:then:patternProperties.
>>>
>>>
>>>> If I put allOf:if:then under patternProperties, I can't do the latter.
>>>
>>> You can.
> 
> Am I supposed to do something like this:
> 
> patternProperties:
>    "^(ethernet-)?ports$":
>      type: object
> 
>      patternProperties:
>        "^(ethernet-)?port@[0-9]+$":
>          type: object
>          description: Ethernet switch ports
> 
>          unevaluatedProperties: false
> 
>          properties:
>            reg:
>              description:
>                Port address described must be 5 or 6 for CPU port and
>                from 0 to 5 for user ports.
> 
>          allOf:
>            - $ref: dsa-port.yaml#
>            - if:
>                properties:
>                  label:
>                    items:
>                      - const: cpu
>              then:
>                allOf:
>                  - if:
>                      properties:

Not really, this is absolutely unreadable.

Usually the way it is handled is:

patternProperties:
   "^(ethernet-)?ports$":
     type: object

     patternProperties:
       "^(ethernet-)?port@[0-9]+$":
         type: object
         description: Ethernet switch ports
         unevaluatedProperties: false
         ... regular stuff follows

allOf:
 - if:
     properties:
       compatible:
         .....
   then:
     patternProperties:
       "^(ethernet-)?ports$":
         patternProperties:
           "^(ethernet-)?port@[0-9]+$":
             properties:
               reg:
                 const: 5


I admit that it is still difficult to parse, which could justify
splitting to separate schema. Anyway the point of my comment was to
define all properties in top level, not in allOf.

allOf should be used to constrain these properties.

Best regards,
Krzysztof