[RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Chetan Loke]

Hello All,

I meant to 'CC' netdev earlier(http://lkml.org/lkml/2010/7/15/334).
Please 'CC' me.

LKML Post:
http://kerneltrap.org/mailarchive/linux-kernel/2010/7/12/4592938


This proposal will provide the ability to shoot an
(early?/prep-time?)'ioctl' via an 'ethX'
agnostic naming scheme.

Requirement:
R1)Ability to address NICs/interfaces using a mac-addr in ioctls. This
is required because we don't have a consistent naming scheme for
Ethernet devices.Asking customers and/or field-engineers to change
udev rules and
other config files is not feasible.

Existing pain-points:
P1) ioctl needs either i) if-name or ii) if-index before we can invoke
bind() etc.This works fine if you know your configuration and it is not going
to change.However,if we hot-add a NIC and if you have adapters from multiple
vendors(think:driver load order) then upon a reboot,the 'eth'
interfaces can be re-mapped.

Existing work-around(s):
W1) user-apps scan /sys/class/net/ethX/address nodes, grep the hw-addrs
till they find a hwaddr-match and then internally create a hwaddr-ethX
mapping table.
W2) change udev-70..persistent rules file and 'rename' the interfaces
according to your needs.
  W2.1) If renaming were to even succeed then none of the existing
drivers re-register their msix-vectors.
  NETDEV_RENAME(or _CHANGE ) handler in the driver does not tear down
the interrupts etc.Some of the sample msix-vectors are as follows : ethX-rx-0,
ethX-rx-1 ... ethX-rx-N
  So if the interface is renamed then how do we measure/correlate the
interrupt-count?

But there is no programmatic way of deriving the 'ethX' name. I got a
few offline replies to the above post, asking me to continue using W1)
from above.Sorry but that was an ugly hack. Also why not replace the
get-ioctls to a 'sys' read everywhere?? ;).

Solution/Proposal:
S1)   Introduce a new ioctl(SIOCGHWADDR_TO_IFNAMEINDEX_MAP[or pick your
name])
S1.1) Enhance dev_ioctl to handle this new case.
S1.2  Re-use for_each_netdev_rcu::is_etherdev_addr(this will iterate
through dev_addrs). By using the above for_each loop we don't need to
re-invent the
wheel.

Input(ifr->hw_addr) : output -> if_name and if_index if ifr->hw_addr is
found.

This way an app can first shoot down an ioctl(sock_fd,
SIOCGHWADDR_TO_IFNAMEINDEX_MAP,ifr), where ifr.ifr_hwaddr is populated
w/ the mac_addr whose mapping you would like.
Then once the if_name and if_index is known, using other ioctls is easy.


Please review the proposal and the sample code below. If this is not a
good approach and if there is a simple workaround then please let me
know.


Regards
Chetan Loke


----------------------------------------------------------

Sample code(PS- I used a quick and dirty driver to demonstrate the
concept rather than modifying the kernel)


Copyright NetScout Systems
Chetan Loke <loke.c@alumni.neu.edu>

struct foo {
      char name[IFNAMSIZ];
      int  index;
};

/* shamelessly copied from compare_etherdev */
/* eventually is_etherdev_equal will be called by dev_ioctl */
int ntct_is_etherdev_equal(u16 *a,u16 *b) {
      return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0;
}

/* eventually enhance dev_ioctl */
int _do_ioctl(unsigned long arg) {

      struct foo my_foo;
      struct net_device *dev;
      int ret=0;
      int found=0;
      int i=0;

      /* eventually sent via
ioctl(sock_fd)->SIOCG_HWADDR_TO_NAMEIDX_MAP and ifr->hw_addr */
      unsigned char mac_addr[]={0x00,0x50,0x56,0xBB,0x52,0xF7};

      /* eventually use rcu_read_lock(); */
      read_lock(&dev_base_lock);

      /* 2.6.31 doesn't have this defined. eventually use
for_each_netdev_rcu. */
      for_each_netdev(&init_net, dev) {
              dev_hold(dev);

              /* eventually use is_etherdev_addr(addr1,addr2) */
              ret = ntct_is_etherdev_equal((u16 *)dev->dev_addr,(u16*)mac_addr);
              if (ret) {
                      printk("<%s> Found
eth-if:%sifindex:%d\n",__func__,dev->name,dev->ifindex);
                      printk("Mac:");
                      for (i=0;i<ETH_ALEN;i++)
                              printk("%02x%c",(unsigned
char)dev->dev_addr[i],((i < 5)? ':':' '));
                      printk("\n");
                      strcpy(my_foo.name,dev->name);
                      my_foo.index=dev->ifindex;
                      dev_put(dev);
                      found=1;
                      break;
              }
              dev_put(dev);
      }

      /* eventually use rcu_read_unlock(); */
      read_unlock(&dev_base_lock);

      if (!found) {
              printk("<%s> hwaddr<->name mapping not found\n",__func__);
              return -EINVAL;
      }

      return copy_to_user((char *)arg,&my_foo,sizeof(struct foo)) ? -EFAULT : 0;
}

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Ben Hutchings]

On Fri, 2010-07-16 at 09:18 -0400, Chetan Loke wrote:
[...]
> Requirement:
> R1)Ability to address NICs/interfaces using a mac-addr in ioctls. This
> is required because we don't have a consistent naming scheme for
> Ethernet devices.Asking customers and/or field-engineers to change
> udev rules and
> other config files is not feasible.

I don't know why they would need to change those.  It might be useful to
have a simple userland tool that will look up a device name by MAC
address.  But it's not like a MAC address is any more user-friendly than
a net device name!

> Existing pain-points:
> P1) ioctl needs either i) if-name or ii) if-index before we can invoke
> bind() etc.This works fine if you know your configuration and it is not going
> to change.However,if we hot-add a NIC and if you have adapters from multiple
> vendors(think:driver load order) then upon a reboot,the 'eth'
> interfaces can be re-mapped.

As you well know, udev makes the name/MAC-address association
persistent, so this is no longer a problem.

[...]
>   W2.1) If renaming were to even succeed then none of the existing
> drivers re-register their msix-vectors.

There is no need to do that, since the IRQ handler names are not copied
but are held by the driver.  So the driver only needs to rewrite the
names when the device is renamed.  However that does require registering
a netdev notifier.  It might be worth adding a netdev operation for
renaming, to make this easier for driver maintainers.

[...]
> But there is no programmatic way of deriving the 'ethX' name.
[...]

Deriving it from what?  Are you aiming to determine what the name of a
device will be before the physical device is installed?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Stephen Hemminger]

On Fri, 16 Jul 2010 18:10:05 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Fri, 2010-07-16 at 09:18 -0400, Chetan Loke wrote:
> [...]
> > Requirement:
> > R1)Ability to address NICs/interfaces using a mac-addr in ioctls. This
> > is required because we don't have a consistent naming scheme for
> > Ethernet devices.Asking customers and/or field-engineers to change
> > udev rules and
> > other config files is not feasible.
> 
> I don't know why they would need to change those.  It might be useful to
> have a simple userland tool that will look up a device name by MAC
> address.  But it's not like a MAC address is any more user-friendly than
> a net device name!
> 
> > Existing pain-points:
> > P1) ioctl needs either i) if-name or ii) if-index before we can invoke
> > bind() etc.This works fine if you know your configuration and it is not going
> > to change.However,if we hot-add a NIC and if you have adapters from multiple
> > vendors(think:driver load order) then upon a reboot,the 'eth'
> > interfaces can be re-mapped.
> 
> As you well know, udev makes the name/MAC-address association
> persistent, so this is no longer a problem.
> 
> [...]
> >   W2.1) If renaming were to even succeed then none of the existing
> > drivers re-register their msix-vectors.
> 
> There is no need to do that, since the IRQ handler names are not copied
> but are held by the driver.  So the driver only needs to rewrite the
> names when the device is renamed.  However that does require registering
> a netdev notifier.  It might be worth adding a netdev operation for
> renaming, to make this easier for driver maintainers.
> 
> [...]
> > But there is no programmatic way of deriving the 'ethX' name.
> [...]
> 
> Deriving it from what?  Are you aiming to determine what the name of a
> device will be before the physical device is installed?
> 
> Ben.
> 

The additional API is not needed. It is trivial to find address for device
and do reverse mapping. Either with ioctl's or /sys/class/net/XXX/addr

RE: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Loke, Chetan]

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: July 16, 2010 2:04 PM
 
> The additional API is not needed. It is trivial to find address for
> device and do reverse mapping. Either with ioctl's 
Sorry, I might have missed it. But which ioctl would that be?

> or /sys/class/net/XXX/addr
So, is reading /sys/ nodes preferred over get-calls?

regards
Chetan Loke

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Stephen Hemminger]

On Fri, 16 Jul 2010 14:12:24 -0400
"Loke, Chetan" <Chetan.Loke@netscout.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: July 16, 2010 2:04 PM
>  
> > The additional API is not needed. It is trivial to find address for
> > device and do reverse mapping. Either with ioctl's 
> Sorry, I might have missed it. But which ioctl would that be?

Simple way:
   Use SIOCGIFCONF to get list of interfaces
   Use SIOCGIFHWADDR to read device addresss

If you want to handle the case where device address is set
by bonding or other protocols, use ETHTOOL_GPERMADDR to read
the original ethernet address.

> > or /sys/class/net/XXX/addr
> So, is reading /sys/ nodes preferred over get-calls?

No difference

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Chetan Loke]

On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 16 Jul 2010 14:12:24 -0400
> "Loke, Chetan" <Chetan.Loke@netscout.com> wrote:
>
>> > -----Original Message-----
>> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
>> > Sent: July 16, 2010 2:04 PM
>>
>> > The additional API is not needed. It is trivial to find address for
>> > device and do reverse mapping. Either with ioctl's
>> Sorry, I might have missed it. But which ioctl would that be?
>
> Simple way:
>   Use SIOCGIFCONF to get list of interfaces
>   Use SIOCGIFHWADDR to read device addresss
>
And interfaces that don't have IP's?

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[David Miller]

From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 15:29:06 -0400

> On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
>> Simple way:
>>   Use SIOCGIFCONF to get list of interfaces
>>   Use SIOCGIFHWADDR to read device addresss
>>
> And interfaces that don't have IP's?

There is no requirement that an interface have configured IP addresses
in order to use those ioctl()'s.

You simply create an arbitrary socket, and use the resulting 'fd' to
run the ioctl's mentioned to fetch information about any and all
interfaces which exist in the system.

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Chetan Loke]

On Fri, Jul 16, 2010 at 3:33 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 15:29:06 -0400
>
>> On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
>> <shemminger@vyatta.com> wrote:
>>> Simple way:
>>>   Use SIOCGIFCONF to get list of interfaces
>>>   Use SIOCGIFHWADDR to read device addresss
>>>
>> And interfaces that don't have IP's?
>
> There is no requirement that an interface have configured IP addresses
> in order to use those ioctl()'s.
>
> You simply create an arbitrary socket, and use the resulting 'fd' to
> run the ioctl's mentioned to fetch information about any and all
> interfaces which exist in the system.
>

Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
that interfaces that didn't have an IP weren't returned.

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[David Miller]

From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 15:35:48 -0400

> Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
> that interfaces that didn't have an IP weren't returned.

If you use the correct set of ioctl()'s it will, just as
"/sbin/ifconfig -a" lists all interfaces regardless of whether they
have IP addresses.

Run strace on 'ifconfig', see what it does :-)

And then you also have the option of using netlink as well.
This is how "ip l l" lists all interfaces, also regardless of
configuration.

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Chetan Loke]

On Fri, Jul 16, 2010 at 3:40 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 15:35:48 -0400
>
>> Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
>> that interfaces that didn't have an IP weren't returned.
>
> If you use the correct set of ioctl()'s it will, just as
> "/sbin/ifconfig -a" lists all interfaces regardless of whether they
> have IP addresses.
>
> Run strace on 'ifconfig', see what it does :-)

I grep the /sys nodes in my existing code already ;) and I didn't want
to do that.If we plop the single for-each-netdev loop then wouldn't it
be easier?

>
> And then you also have the option of using netlink as well.
> This is how "ip l l" lists all interfaces, also regardless of
> configuration.
>

So all user space apps should replicate code when all of that can be
replaced at the cost of a single for-loop in a non fast-path code
within the kernel?

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[David Miller]

From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 16:02:23 -0400

> So all user space apps should replicate code when all of that can be
> replaced at the cost of a single for-loop in a non fast-path code
> within the kernel?

When there is a way to do something already, we don't add "yet another"
way to do it.  The kernel already suffers from API masterbation as-is.

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Chetan Loke]

On Fri, Jul 16, 2010 at 4:04 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 16:02:23 -0400
>
>> So all user space apps should replicate code when all of that can be
>> replaced at the cost of a single for-loop in a non fast-path code
>> within the kernel?
>
> When there is a way to do something already, we don't add "yet another"
> way to do it.  The kernel already suffers from API masterbation as-is.
>

Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if the
ifindex is not found ? May be then in the else clause we could search
using the hw-addr before bailing out ;)? This way we can avoid adding
a new API. Still not a good idea?

Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[David Miller]

From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 16:15:44 -0400

> Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if the
> ifindex is not found ? May be then in the else clause we could search
> using the hw-addr before bailing out ;)? This way we can avoid adding
> a new API. Still not a good idea?

I'm really not going to consider this seriously, sorry.  I've
made my position pretty clear, and gave sufficient reasons for
it.

Myself and others have also shown you several ways to fetch the
information you need, and of them netlink is probably the most
efficient, but the others work just as well.

The new interface proposals are just not a good idea.

RE: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping

[Loke, Chetan]

Ooops... I got the 'CC' list this time



On Fri, Jul 16, 2010 at 4:33 PM, David Miller <davem@davemloft.net>
wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 16:24:16 -0400
>
>> Thanks for your time guys.
>
> You dropped everything from the CC: list, so nobody other
> than me saw this. :)
>
>

On Fri, Jul 16, 2010 at 4:24 PM, Chetan Loke <chetanloke@gmail.com>
wrote:
> On Fri, Jul 16, 2010 at 4:18 PM, David Miller <davem@davemloft.net>
wrote:
>> From: Chetan Loke <chetanloke@gmail.com>
>> Date: Fri, 16 Jul 2010 16:15:44 -0400
>>
>>> Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if
the
>>> ifindex is not found ? May be then in the else clause we could
search
>>> using the hw-addr before bailing out ;)? This way we can avoid
adding
>>> a new API. Still not a good idea?
>>
>> I'm really not going to consider this seriously, sorry.  I've
>> made my position pretty clear, and gave sufficient reasons for
>> it.
>>
>> Myself and others have also shown you several ways to fetch the
>> information you need, and of them netlink is probably the most
>> efficient, but the others work just as well.
>>
>> The new interface proposals are just not a good idea.
>>
>
> No problem Dave. I will patch our internal iso and add dev_getbyhwaddr
> in the else clause.
>
> Thanks for your time guys.
>
> regards
> Chetan Loke
>