netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Donald Hunter <donald.hunter@gmail.com>
To: Alessandro Marcolini <alessandromarcolini99@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	 davem@davemloft.net, edumazet@google.com,  pabeni@redhat.com,
	 sdf@google.com, chuck.lever@oracle.com,  lorenzo@kernel.org,
	 jacob.e.keller@intel.com, jiri@resnulli.us,
	 netdev@vger.kernel.org
Subject: Re: [PATCH v2 net-next 3/3] tools: ynl: add support for encoding multi-attr
Date: Fri, 02 Feb 2024 11:42:28 +0000	[thread overview]
Message-ID: <m2frybum6z.fsf@gmail.com> (raw)
In-Reply-To: <2b3ec0f1-303d-4e0c-92de-5d0430470c33@gmail.com> (Alessandro Marcolini's message of "Fri, 2 Feb 2024 12:38:11 +0100")

Alessandro Marcolini <alessandromarcolini99@gmail.com> writes:

> On 2/2/24 02:24, Jakub Kicinski wrote:
>> I think you're trying to handle this at the wrong level. The main
>> message can also contain multi-attr, so looping inside nests won't
>> cut it.
>>
>> Early in the function check if attr.is_multi and isinstance(value,
>> list), and if so do:
>>
>> 	attr_payload = b''
>> 	for subvalue in value:
>> 		attr_payload += self._add_attr(space, name, subvalue,
>> 					       search_attrs) 
>> 	return attr_payload
>>
>> IOW all you need to do is recursively call _add_attr() with the
>> subvalues stripped. You don't have to descend into a nest.
>
> I (wrongly) supposed that multi-attr attributes were always inside a nest (that's because I've
> only experimented with the tc spec). That's also because I (mistakenly, again) thought that the
> syntax for specifying a multi-attr would be:
> "parent-attr":[{multi-attr:{values}}, {multi-attr: {values}}, ... ]
> Instead of:
> "optional-parent-attr": {"multi-attr": [{values in multi-attr}, ...]}
>
> By reading the docs [1]:
> "multi-attr (arrays)
> Boolean property signifying that the attribute may be present multiple times. Allowing an
> attribute to repeat is the recommended way of implementing arrays (no extra nesting)."
>
> I understood that the syntax should be the former (I was thinking of an array containing all the
> multi-attr attributes, and not only their values), albeit really verbose and not that readable.
>
> I've now made the changes as you suggested and tested it, it works as expected!
> I'll post a v3 soon, thanks for your review :)
>
> [1] https://docs.kernel.org/userspace-api/netlink/specs.html#multi-attr-arrays

Yes, if your input matches the ynl output then you should be good:

"sched-entry-list": {
 "entry": [
  {
   "index": 0,
   "cmd": 0,
   "gate-mask": 1,
   "interval": 500000
  },
  {
   "index": 1,
   "cmd": 0,
   "gate-mask": 1,
   "interval": 500000
  }
 ]
}

  reply	other threads:[~2024-02-02 11:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-01 15:12 [PATCH v2 net-next 0/3] Add support for encoding multi-attr to ynl Alessandro Marcolini
2024-02-01 15:12 ` [PATCH v2 net-next 1/3] tools: ynl: correct typo and docstring Alessandro Marcolini
2024-02-01 15:12 ` [PATCH v2 net-next 2/3] doc: netlink: specs: tc: add multi-attr to tc-taprio-sched-entry Alessandro Marcolini
2024-02-01 15:48   ` Donald Hunter
2024-02-01 15:12 ` [PATCH v2 net-next 3/3] tools: ynl: add support for encoding multi-attr Alessandro Marcolini
2024-02-02  1:24   ` Jakub Kicinski
2024-02-02 11:38     ` Alessandro Marcolini
2024-02-02 11:42       ` Donald Hunter [this message]
2024-02-02 13:55         ` Alessandro Marcolini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2frybum6z.fsf@gmail.com \
    --to=donald.hunter@gmail.com \
    --cc=alessandromarcolini99@gmail.com \
    --cc=chuck.lever@oracle.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).