On Wed, Aug 28, 2019 at 12:07 AM Jiri Pirko [off-list ref] wrote:

Tue, Aug 27, 2019 at 05:14:49PM CEST, roopa@cumulusnetworks.com wrote:

quoted

On Tue, Aug 27, 2019 at 2:35 AM Jiri Pirko [off-list ref] wrote:

quoted

Tue, Aug 27, 2019 at 10:22:42AM CEST, davem@davemloft.net wrote:

quoted

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 27 Aug 2019 09:08:08 +0200

quoted

Okay, so if I understand correctly, on top of separate commands for
add/del of alternative names, you suggest also get/dump to be separate
command and don't fill this up in existing newling/getlink command.

I'm not sure what to do yet.

David has a point, because the only way these ifnames are useful is
as ways to specify and choose net devices.  So based upon that I'm
slightly learning towards not using separate commands.

Well yeah, one can use it to handle existing commands instead of
IFLA_NAME.

But why does it rule out separate commands? I think it is cleaner than
to put everything in poor setlink messages :/ The fact that we would
need to add "OP" to the setlink message just feels of. Other similar
needs may show up in the future and we may endup in ridiculous messages
like:

SETLINK
  IFLA_NAME eth0
  IFLA_ATLNAME_LIST (nest)
      IFLA_ALTNAME_OP add
      IFLA_ALTNAME somereallylongname
      IFLA_ALTNAME_OP del
      IFLA_ALTNAME somereallyreallylongname
      IFLA_ALTNAME_OP add
      IFLA_ALTNAME someotherreallylongname
  IFLA_SOMETHING_ELSE_LIST (nest)
      IFLA_SOMETHING_ELSE_OP add
      ...
      IFLA_SOMETHING_ELSE_OP del
      ...
      IFLA_SOMETHING_ELSE_OP add
      ...

I don't know what to think about it. Rollbacks are going to be pure hell :/

I don't see a huge problem with the above. We need a way to solve this
anyways for other list types in the future correct ?.
The approach taken by this series will not scale if we have to add a
new msg type and header for every such list attribute in the future.

Do you have some other examples in mind? So far, this was not needed.

yes, so far it was not needed.
No other future possible examples in mind...but I wont be surprised if
we see such cases in the future.
Having a consistent API to extend a list attribute will help.

quoted

A good parallel here is bridge vlan which uses RTM_SETLINK and
RTM_DELLINK for vlan add and deletes. But it does have an advantage of
a separate
msg space under AF_BRIDGE which makes it cleaner. Maybe something
closer to that  can be made to work (possibly with a msg flag) ?.

1) Not sure if AF_BRIDGE is the right example how to do things
2) See br_vlan_info(). It is not an OP-PER-VLAN. You either add or
delete all passed info, depending on the cmd (RTM_SETLINK/RTM_DETLINK).

yes,  correct. I mentioned that because I was wondering if we can
think along the same lines for this API.
eg
(a) RTM_NEWLINK always replaces the list attribute
(b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
(c) RTM_DELLINK with NLM_F_APPEND updates the list attribute

(It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
case. I have not looked at the full dellink path if it will work
neatly..its been a busy day )

quoted

Would be good to have a consistent way to update list attributes for
future needs too.

Okay. Do you suggest to have new set of commands to handle
adding/deleting lists of items? altNames now, others (other nests) later?

Something like:

CMD SETLISTS
     IFLA_NAME eth0
     IFLA_ATLNAME_LIST (nest)
       IFLA_ALTNAME somereallylongname
       IFLA_ALTNAME somereallyreallylongname
       IFLA_ALTNAME someotherreallylongname
     IFLA_SOMETHING_ELSE_LIST (nest)
       IFLA_SOMETHING_ELSE
       IFLA_SOMETHING_ELSE
       IFLA_SOMETHING_ELSE


CMD DELLISTS
     IFLA_NAME eth0
     IFLA_ATLNAME_LIST (nest)
       IFLA_ALTNAME somereallylongname
       IFLA_ALTNAME somereallyreallylongname
       IFLA_ALTNAME someotherreallylongname
     IFLA_SOMETHING_ELSE_LIST (nest)
       IFLA_SOMETHING_ELSE
       IFLA_SOMETHING_ELSE
       IFLA_SOMETHING_ELSE

How does this sound?

This seems fine but it does introduce a new type. If we can avoid a
new msg type with a flag that would be nice (like the NLM_F_APPEND eg
above).
The reason for that is to see if we can use it else where too (eg some
random future list attribute in the route subsystem. If a flag works
then we don't have to add a RTM_NEWROUTE variant of CMD SETLISTS and
CMD DELLISTS)