2024-02-21 19:10:09

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs

On Wed, 21 Feb 2024 07:57:28 -0800 Joe Damato wrote:
> Greetings:
>
> The netdev netlink APIs currently provide the ifindex of a device
> associated with the NIC queue or NAPI when the netlink API is used. In
> order for user applications to map this back to a human readable device
> name, user applications must issue a subsequent ioctl (SIOCGIFNAME) in
> order to map an ifindex back to a device name.

To be clear, if_indextoname() is doing it, right? I wanted to be sure
the concern is really number of syscalls, not the difficulty in getting
the name.

> This patch set adds ifname to the API so that when queue or NAPI
> information is retrieved, the human readable string is included. The netdev
> netlink YAML spec has been updated to include this field, as well.
>
> This saves the subsequent call to ioctl and makes the netlink information
> more user friendly. Applications might use this information in conjunction
> with SO_INCOMING_NAPI_ID to map NAPI IDs to specific NICs with application
> specific configuration (e.g. NUMA zone and CPU layout information).

For context, the reason why I left the names out is that they can change
at any moment, but primarily because there are also altnames now:

2: eth0:
[...]
altname enp2s0np0

Most of the APIs try to accept altnames as well as the "main" name.
If we propagate the name we'll step back into the rtnetlink naming
mess :(


2024-02-21 19:21:38

by Joe Damato

[permalink] [raw]
Subject: Re: [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs

On Wed, Feb 21, 2024 at 11:09:52AM -0800, Jakub Kicinski wrote:
> On Wed, 21 Feb 2024 07:57:28 -0800 Joe Damato wrote:
> > Greetings:
> >
> > The netdev netlink APIs currently provide the ifindex of a device
> > associated with the NIC queue or NAPI when the netlink API is used. In
> > order for user applications to map this back to a human readable device
> > name, user applications must issue a subsequent ioctl (SIOCGIFNAME) in
> > order to map an ifindex back to a device name.
>
> To be clear, if_indextoname() is doing it, right? I wanted to be sure
> the concern is really number of syscalls, not the difficulty in getting
> the name.

It seemed a bit odd to me to require the user to hit different APIs -- one
to get the ifindex and then another to get the name. I didn't realize you
had intentionally left the name out, though.

> > This patch set adds ifname to the API so that when queue or NAPI
> > information is retrieved, the human readable string is included. The netdev
> > netlink YAML spec has been updated to include this field, as well.
> >
> > This saves the subsequent call to ioctl and makes the netlink information
> > more user friendly. Applications might use this information in conjunction
> > with SO_INCOMING_NAPI_ID to map NAPI IDs to specific NICs with application
> > specific configuration (e.g. NUMA zone and CPU layout information).
>
> For context, the reason why I left the names out is that they can change
> at any moment, but primarily because there are also altnames now:
>
> 2: eth0:
> [...]
> altname enp2s0np0
>
> Most of the APIs try to accept altnames as well as the "main" name.
> If we propagate the name we'll step back into the rtnetlink naming
> mess :(

OK, I see. I didn't realize this was a thing. I suppose what you are saying
is that we wouldn't want to expose names at all and stick with ifindexes
only, is that right?

2024-02-21 19:27:04

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs

On Wed, 21 Feb 2024 11:21:23 -0800 Joe Damato wrote:
> > For context, the reason why I left the names out is that they can change
> > at any moment, but primarily because there are also altnames now:
> >
> > 2: eth0:
> > [...]
> > altname enp2s0np0
> >
> > Most of the APIs try to accept altnames as well as the "main" name.
> > If we propagate the name we'll step back into the rtnetlink naming
> > mess :(
>
> OK, I see. I didn't realize this was a thing. I suppose what you are saying
> is that we wouldn't want to expose names at all and stick with ifindexes
> only, is that right?

If you think it's a major usability improvement I can be convinced,
but yes, leaving the names out initially was indeed intentional.

2024-02-21 19:30:06

by Joe Damato

[permalink] [raw]
Subject: Re: [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs

On Wed, Feb 21, 2024 at 11:26:44AM -0800, Jakub Kicinski wrote:
> On Wed, 21 Feb 2024 11:21:23 -0800 Joe Damato wrote:
> > > For context, the reason why I left the names out is that they can change
> > > at any moment, but primarily because there are also altnames now:
> > >
> > > 2: eth0:
> > > [...]
> > > altname enp2s0np0
> > >
> > > Most of the APIs try to accept altnames as well as the "main" name.
> > > If we propagate the name we'll step back into the rtnetlink naming
> > > mess :(
> >
> > OK, I see. I didn't realize this was a thing. I suppose what you are saying
> > is that we wouldn't want to expose names at all and stick with ifindexes
> > only, is that right?
>
> If you think it's a major usability improvement I can be convinced,
> but yes, leaving the names out initially was indeed intentional.

Well... it is useful to me, but I think I'm only one user and the side
effects of adding this might have painful results in the future so after
your comment I think it might be best left out.

Sorry for the noise.