How to display IPv4 array?

How to display IPv4 array?

[Hangbin Liu]

Hi Jakub,

I plan to add bond support for Documentation/netlink/specs/rt_link.yaml. While
dealing with the attrs. I got a problem about how to show the bonding arp/ns
targets. Because the arp/ns targets are filled as an array[1]. I tried
something like:

  -
    name: linkinfo-bond-attrs
    name-prefix: ifla-bond-
    attributes:
      -
        name: arp-ip-target
        type: nest
        nested-attributes: ipv4-addr
  -
    name: ipv4-addr
    attributes:
      -
        name: addr
        type: binary
        display-hint: ipv4

But this failed with error: Exception: Space 'ipv4-addr' has no attribute with value '0'
Do you have any suggestion?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/bonding/bond_netlink.c#n670

Thanks
Hangbin

Re: How to display IPv4 array?

[Jiri Pirko]

Tue, Mar 12, 2024 at 11:08:33AM CET, liuhangbin@gmail.com wrote:
>Hi Jakub,
>
>I plan to add bond support for Documentation/netlink/specs/rt_link.yaml. While
>dealing with the attrs. I got a problem about how to show the bonding arp/ns
>targets. Because the arp/ns targets are filled as an array[1]. I tried
>something like:
>
>  -
>    name: linkinfo-bond-attrs
>    name-prefix: ifla-bond-
>    attributes:
>      -
>        name: arp-ip-target
>        type: nest
>        nested-attributes: ipv4-addr
>  -
>    name: ipv4-addr
>    attributes:
>      -
>        name: addr
>        type: binary
>        display-hint: ipv4
>
>But this failed with error: Exception: Space 'ipv4-addr' has no attribute with value '0'
>Do you have any suggestion?
>
>[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/bonding/bond_netlink.c#n670

Yeah, that's odd use of attr type, here it is an array index. I'm pretty
sure I saw this in the past on different netlink places.
I believe that is not supported with the existing ynl code.

Perhaps something like the following might work:
      -
        name: arp-ip-target
        type: binary
        display-hint: ipv4
	nested-array: true

"nested-array" would tell the parser to expect a nest that has attr
type of value of array index, "type" is the same for all array members.
The output will be the same as in case of "multi-attr", array index
ignored (I don't see what it would be good for to the user).

Other existing attrs considered:

"nested-attributes" does not make much sense for this usecase IMO as the
attr type is array index, the mapping fails.

"multi-attr" also counts with valid attr type and no nest.

Makes sense?

Re: How to display IPv4 array?

[Hangbin Liu]

Hi Jiri,
On Tue, Mar 12, 2024 at 01:12:30PM +0100, Jiri Pirko wrote:
> Tue, Mar 12, 2024 at 11:08:33AM CET, liuhangbin@gmail.com wrote:
> >Hi Jakub,
> >
> >I plan to add bond support for Documentation/netlink/specs/rt_link.yaml. While
> >dealing with the attrs. I got a problem about how to show the bonding arp/ns
> >targets. Because the arp/ns targets are filled as an array[1]. I tried
> >something like:
> >
> >  -
> >    name: linkinfo-bond-attrs
> >    name-prefix: ifla-bond-
> >    attributes:
> >      -
> >        name: arp-ip-target
> >        type: nest
> >        nested-attributes: ipv4-addr
> >  -
> >    name: ipv4-addr
> >    attributes:
> >      -
> >        name: addr
> >        type: binary
> >        display-hint: ipv4
> >
> >But this failed with error: Exception: Space 'ipv4-addr' has no attribute with value '0'
> >Do you have any suggestion?
> >
> >[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/bonding/bond_netlink.c#n670
> 
> Yeah, that's odd use of attr type, here it is an array index. I'm pretty
> sure I saw this in the past on different netlink places.
> I believe that is not supported with the existing ynl code.
> 
> Perhaps something like the following might work:
>       -
>         name: arp-ip-target
>         type: binary
>         display-hint: ipv4
> 	nested-array: true
> 
> "nested-array" would tell the parser to expect a nest that has attr
> type of value of array index, "type" is the same for all array members.
> The output will be the same as in case of "multi-attr", array index
> ignored (I don't see what it would be good for to the user).

Yes, this looks a do-able way. Although we already have a similar type
'array-nest'...

I also figured out a workaround. e.g.

  -
    name: linkinfo-bond-attrs
    name-prefix: ifla-bond-
    attributes:
      -
        name: arp-ip-target
        type: nest
        nested-attributes: ipv4-addr

  -
    name: ipv4-addr
    attributes:
      -
        name: addr0
        value: 0
        type: u32
        byte-order: big-endian
        display-hint: ipv4
      -
        name: addr1
        value: 1
        type: u32
        byte-order: big-endian
        display-hint: ipv4

With this we can show the target like:

     'arp-ip-target': {'addr0': '192.168.1.1',
                       'addr1': '192.168.1.2'},

But we need to add all BOND_MAX_ARP_TARGETS attrs. Which doesn't like a good
way. So maybe as you suggested, add a new type "nested-array".

Thanks
Hangbin

Re: How to display IPv4 array?

[Jiri Pirko]

Tue, Mar 12, 2024 at 01:37:01PM CET, liuhangbin@gmail.com wrote:
>Hi Jiri,
>On Tue, Mar 12, 2024 at 01:12:30PM +0100, Jiri Pirko wrote:
>> Tue, Mar 12, 2024 at 11:08:33AM CET, liuhangbin@gmail.com wrote:
>> >Hi Jakub,
>> >
>> >I plan to add bond support for Documentation/netlink/specs/rt_link.yaml. While
>> >dealing with the attrs. I got a problem about how to show the bonding arp/ns
>> >targets. Because the arp/ns targets are filled as an array[1]. I tried
>> >something like:
>> >
>> >  -
>> >    name: linkinfo-bond-attrs
>> >    name-prefix: ifla-bond-
>> >    attributes:
>> >      -
>> >        name: arp-ip-target
>> >        type: nest
>> >        nested-attributes: ipv4-addr
>> >  -
>> >    name: ipv4-addr
>> >    attributes:
>> >      -
>> >        name: addr
>> >        type: binary
>> >        display-hint: ipv4
>> >
>> >But this failed with error: Exception: Space 'ipv4-addr' has no attribute with value '0'
>> >Do you have any suggestion?
>> >
>> >[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/bonding/bond_netlink.c#n670
>> 
>> Yeah, that's odd use of attr type, here it is an array index. I'm pretty
>> sure I saw this in the past on different netlink places.
>> I believe that is not supported with the existing ynl code.
>> 
>> Perhaps something like the following might work:
>>       -
>>         name: arp-ip-target
>>         type: binary
>>         display-hint: ipv4
>> 	nested-array: true
>> 
>> "nested-array" would tell the parser to expect a nest that has attr
>> type of value of array index, "type" is the same for all array members.
>> The output will be the same as in case of "multi-attr", array index
>> ignored (I don't see what it would be good for to the user).
>
>Yes, this looks a do-able way. Although we already have a similar type
>'array-nest'...

That is something else. That is you have a nested attribute that holds
multiple nests of the same attr type with attributes inside. Something
completely different from what I see...


>
>I also figured out a workaround. e.g.
>
>  -
>    name: linkinfo-bond-attrs
>    name-prefix: ifla-bond-
>    attributes:
>      -
>        name: arp-ip-target
>        type: nest
>        nested-attributes: ipv4-addr
>
>  -
>    name: ipv4-addr
>    attributes:
>      -
>        name: addr0
>        value: 0
>        type: u32
>        byte-order: big-endian
>        display-hint: ipv4
>      -
>        name: addr1
>        value: 1
>        type: u32
>        byte-order: big-endian
>        display-hint: ipv4

Or, special value "any" or "all" that would match them all?


>
>With this we can show the target like:
>
>     'arp-ip-target': {'addr0': '192.168.1.1',
>                       'addr1': '192.168.1.2'},
>
>But we need to add all BOND_MAX_ARP_TARGETS attrs. Which doesn't like a good
>way. So maybe as you suggested, add a new type "nested-array".



>
>Thanks
>Hangbin

Re: How to display IPv4 array?

[Donald Hunter]

On Tue, 12 Mar 2024 at 12:12, Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Mar 12, 2024 at 11:08:33AM CET, liuhangbin@gmail.com wrote:
> >Hi Jakub,
> >
> >I plan to add bond support for Documentation/netlink/specs/rt_link.yaml. While
> >dealing with the attrs. I got a problem about how to show the bonding arp/ns
> >targets. Because the arp/ns targets are filled as an array[1]. I tried
> >something like:
> >
> >  -
> >    name: linkinfo-bond-attrs
> >    name-prefix: ifla-bond-
> >    attributes:
> >      -
> >        name: arp-ip-target
> >        type: nest
> >        nested-attributes: ipv4-addr
> >  -
> >    name: ipv4-addr
> >    attributes:
> >      -
> >        name: addr
> >        type: binary
> >        display-hint: ipv4
> >
> >But this failed with error: Exception: Space 'ipv4-addr' has no attribute with value '0'
> >Do you have any suggestion?
> >
> >[1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/drivers/net/bonding/bond_netlink.c#n670
>
> Yeah, that's odd use of attr type, here it is an array index. I'm pretty
> sure I saw this in the past on different netlink places.
> I believe that is not supported with the existing ynl code.
>
> Perhaps something like the following might work:
>       -
>         name: arp-ip-target
>         type: binary
>         display-hint: ipv4
>         nested-array: true
>
> "nested-array" would tell the parser to expect a nest that has attr
> type of value of array index, "type" is the same for all array members.
> The output will be the same as in case of "multi-attr", array index
> ignored (I don't see what it would be good for to the user).

I'd say that this construct looks more like nest-type-value, except
that it contains a value rather than a set of nested attributes. It is
documented here:

https://docs.kernel.org/userspace-api/netlink/genetlink-legacy.html#type-value

You can see an example of nest-type-value in the nlctrl.yaml in net-next:

      -
        name: policy
        type: nest-type-value
        type-value: [ policy-id, attr-id ]
        nested-attributes: policy-attrs

It has one or more indices that are stored as nest keys, followed by a
set of nested attributes. Perhaps this could be extended to support an
attribute instead of the nested-attributes ?

> Other existing attrs considered:
>
> "nested-attributes" does not make much sense for this usecase IMO as the
> attr type is array index, the mapping fails.
>
> "multi-attr" also counts with valid attr type and no nest.
>
> Makes sense?
>

Re: How to display IPv4 array?

[Jakub Kicinski]

On Tue, 12 Mar 2024 16:04:56 +0000 Donald Hunter wrote:
> > "nested-array" would tell the parser to expect a nest that has attr
> > type of value of array index, "type" is the same for all array members.
> > The output will be the same as in case of "multi-attr", array index
> > ignored (I don't see what it would be good for to the user).  
> 
> I'd say that this construct looks more like nest-type-value

type-value is sort of a decomposed array, if we have all the entries
under one nest I reckon array extension may be more appropriate.

My gut feeling is that we should generalize the array-nest type,
when I wrote the initial spec we didn't have sub-type. How about
we replace array-nest with indexed-array (good name TBD), and instead
of assuming the value is always a nest pass the type via sub-type?

For bonding probably something like:

  -
    name: linkinfo-bond-attrs
    name-prefix: ifla-bond-
    attributes:
      -
        name: arp-ip-target
        type: indexed-array
        sub-type: u32
	byte-order: big-endian

how does that sound?

exiting array-nests would change from:

 -
   name: bla
   type: array-nest
   nested-attributes: bla-attrs

to

 -
   name: bla
   type: indexed-array
   sub-type: nest
   nested-attributes: bla-attrs

But that'd mean updating all existing specs and codegen.

Re: How to display IPv4 array?

[Hangbin Liu]

On Tue, Mar 12, 2024 at 10:01:05AM -0700, Jakub Kicinski wrote:
> On Tue, 12 Mar 2024 16:04:56 +0000 Donald Hunter wrote:
> > > "nested-array" would tell the parser to expect a nest that has attr
> > > type of value of array index, "type" is the same for all array members.
> > > The output will be the same as in case of "multi-attr", array index
> > > ignored (I don't see what it would be good for to the user).  
> > 
> > I'd say that this construct looks more like nest-type-value
> 
> type-value is sort of a decomposed array, if we have all the entries
> under one nest I reckon array extension may be more appropriate.
> 
> My gut feeling is that we should generalize the array-nest type,
> when I wrote the initial spec we didn't have sub-type. How about
> we replace array-nest with indexed-array (good name TBD), and instead
> of assuming the value is always a nest pass the type via sub-type?
> 
> For bonding probably something like:
> 
>   -
>     name: linkinfo-bond-attrs
>     name-prefix: ifla-bond-
>     attributes:
>       -
>         name: arp-ip-target
>         type: indexed-array
>         sub-type: u32
> 	byte-order: big-endian
> 
> how does that sound?
> 
> exiting array-nests would change from:
> 
>  -
>    name: bla
>    type: array-nest
>    nested-attributes: bla-attrs
> 
> to
> 
>  -
>    name: bla
>    type: indexed-array
>    sub-type: nest
>    nested-attributes: bla-attrs
> 
> But that'd mean updating all existing specs and codegen.

This looks good to me.

Thanks
Hangbin