net device renaming 2-step, IFNAMSIZ limit

net device renaming 2-step, IFNAMSIZ limit

[Matt Domsch]

Once again, IFNAMSIZ bites me.

I see this in the udev debug logs when using biosdevname (which I
finally got working with SR-IOV devices tonight):

renamed network interface eth60 to eth60-pci2#0_60
renamed network interface eth60-pci2#0_60 to pci2#0_60

So, it worked, however, note that the rename happens in 2 steps, the
middle step of which uses a name that's dangerously close to
IFNAMSIZ, in fact it is 15 chars there.

In my testing, I'm doing:
 modprobe ixgbe max_vfsc

which generates ethN..(N+127) interfaces (yes, 128 new interfaces) on
this dual-port card.  Combine that with the longer names that
biosdevname is suggesting:

 pciNN#PP_VVV

which is itself already 12 chars if all the fields are used to their
maximum length, and the concatenation of <oldname>-<newname> is way
past IFNAMSIZ.

I need a solution in which the intermediate name doesn't exceed 15
characters, and is guaranteed to be unique, as there may be lots of
udev instances running in parallel trying to do the same thing.

Ideas?

(note, vconfig can create VLANs for interfaces, which needs another 5
characters appended on the end, but that's for another day).

Thanks,
Matt

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: net device renaming 2-step, IFNAMSIZ limit

[Piter PUNK]

Matt Domsch wrote:
> I see this in the udev debug logs when using biosdevname (which I
> finally got working with SR-IOV devices tonight):
>
> renamed network interface eth60 to eth60-pci2#0_60
> renamed network interface eth60-pci2#0_60 to pci2#0_60
>
> So, it worked, however, note that the rename happens in 2 steps, the
> middle step of which uses a name that's dangerously close to
> IFNAMSIZ, in fact it is 15 chars there.
>
> <...>
>
> I need a solution in which the intermediate name doesn't exceed 15
> characters, and is guaranteed to be unique, as there may be lots of
> udev instances running in parallel trying to do the same thing.
>
> Ideas?

As we talk on #udev, there is a patch that uses hash32 function inside 
libudev to create
the intermediate name. We use the "oldname-newname" to create the hash. 
With that,
exceeding IFNAMSIZ isn't a problem:

-------cut here--------

diff --git a/udev/udev-event.c b/udev/udev-event.c
index 0648735..fc4aae0 100644
--- a/udev/udev-event.c
+++ b/udev/udev-event.c
@@ -462,6 +462,7 @@ static void rename_netif_kernel_log(struct ifreq ifr)
  static int rename_netif(struct udev_event *event)
  {
         struct udev_device *dev = event->dev;
+       char iftmp[IFNAMSIZ*2+1];
         int sk;
         struct ifreq ifr;
         int loop;
@@ -492,7 +493,8 @@ static int rename_netif(struct udev_event *event)
                 goto out;

         /* free our own name, another process may wait for us */
-       util_strscpyl(ifr.ifr_newname, IFNAMSIZ, 
udev_device_get_sysname(dev), "-", event->name, NULL);
+       util_strscpyl(iftmp, IFNAMSIZ*2+1, udev_device_get_sysname(dev), 
"-", event->name, NULL);
+       util_strscpyl(ifr.ifr_newname, IFNAMSIZ, "eth_%X", 
util_string_hash32(iftmp), NULL);
         err = ioctl(sk, SIOCSIFNAME, &ifr);
         if (err < 0) {
                 err = -errno;

-------cut here--------

Ideas and fixes are welcome,

Piter PUNK

Re: net device renaming 2-step, IFNAMSIZ limit

[Matt Domsch]

On Tue, Dec 07, 2010 at 12:45:04AM -0200, Piter PUNK wrote:
> As we talk on #udev, there is a patch that uses hash32 function inside 
> libudev to create
> the intermediate name. We use the "oldname-newname" to create the hash. 
> With that,
> exceeding IFNAMSIZ isn't a problem:

Thanks piterpunk.

I built udev on a Fedora 14 box, with this patch, and it works.
However, I don't see udev doing the 2-step anymore, perhaps that was
just a figment of the loglevel.  Anyhow, it looks right, and doesn't
blow up.

Thanks,
Matt

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: net device renaming 2-step, IFNAMSIZ limit

[Matt Domsch]

On Tue, Dec 07, 2010 at 12:45:04AM -0200, Piter PUNK wrote:
> Matt Domsch wrote:
> >I see this in the udev debug logs when using biosdevname (which I
> >finally got working with SR-IOV devices tonight):
> >
> >renamed network interface eth60 to eth60-pci2#0_60
> >renamed network interface eth60-pci2#0_60 to pci2#0_60
> >
> >So, it worked, however, note that the rename happens in 2 steps, the
> >middle step of which uses a name that's dangerously close to
> >IFNAMSIZ, in fact it is 15 chars there.
> >
> ><...>
> >
> >I need a solution in which the intermediate name doesn't exceed 15
> >characters, and is guaranteed to be unique, as there may be lots of
> >udev instances running in parallel trying to do the same thing.
> >
> >Ideas?
> 
> As we talk on #udev, there is a patch that uses hash32 function inside 
> libudev to create
> the intermediate name. We use the "oldname-newname" to create the hash. 
> With that,
> exceeding IFNAMSIZ isn't a problem:


Piter's patch works for me, and I need this in udev fairly quickly
please.  Some kind folks from IBM have been testing biosdevname with
quad-port Intel SR-IOV-capable devices, and instantiating all the VFs,
can easily have >100 eth* devices created in the kernel
instantaneously.  That bumps step 1 in the 2-step rename operation
over IFNAMSIZ  (eth100-pci2#3_62 is 16 chars).

Please apply.  I'm re-pasting from the original email for convenience.

Tested-by: Matt Domsch <Matt_Domsch@dell.com>

Thanks,
Matt


-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

diff --git a/udev/udev-event.c b/udev/udev-event.c
index 0648735..5387c00 100644
--- a/udev/udev-event.c
+++ b/udev/udev-event.c
@@ -462,6 +462,7 @@ static void rename_netif_kernel_log(struct ifreq ifr)
 static int rename_netif(struct udev_event *event)
 {
 	struct udev_device *dev = event->dev;
+	char iftmp[IFNAMSIZ];
 	int sk;
 	struct ifreq ifr;
 	int loop;
@@ -492,7 +493,8 @@ static int rename_netif(struct udev_event *event)
 		goto out;
 
 	/* free our own name, another process may wait for us */
-	util_strscpyl(ifr.ifr_newname, IFNAMSIZ, udev_device_get_sysname(dev), "-", event->name, NULL);
+	util_strscpyl(iftmp, IFNAMSIZ, udev_device_get_sysname(dev), "-", event->name, NULL);
+	sprintf(ifr.ifr_newname, "eth_%X",util_string_hash32(iftmp));
 	err = ioctl(sk, SIOCSIFNAME, &ifr);
 	if (err < 0) {
 		err = -errno;

Re: net device renaming 2-step, IFNAMSIZ limit

[Kay Sievers]

On Sat, Jan 29, 2011 at 16:23, Matt Domsch <Matt_Domsch@dell.com> wrote:
> On Tue, Dec 07, 2010 at 12:45:04AM -0200, Piter PUNK wrote:
>> Matt Domsch wrote:

> I need this in udev fairly quickly
> please.  Some kind folks from IBM have been testing biosdevname with
> quad-port Intel SR-IOV-capable devices, and instantiating all the VFs,
> can easily have >100 eth* devices created in the kernel
> instantaneously.  That bumps step 1 in the 2-step rename operation
> over IFNAMSIZ  (eth100-pci2#3_62 is 16 chars).

What is all this about?

If biosdevname ever needs a two-step renaming, there is something
wrong. Two-step renaming happens only for conflicting names. We do not
want to support renaming in conflicting namespaces anymore.

Also, we can not use hashes without checking for conflicts, it's
unlikely to happen, but we just don't do such hacks. Just use the
ifindex or whatever fits to be correct.

Kay

Re: net device renaming 2-step, IFNAMSIZ limit

[Scott James Remnant]

Indeed, speaking as the guy who wrote the two-step naming, the point
was to make it possible to *swap* eth0 and eth1.

If you're renaming from eth0 -> e1 and eth1 -> e0, you don't need the
two steps, since the destination names cannot possible conflict.

Scott

On Tue, Feb 8, 2011 at 6:42 AM, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Sat, Jan 29, 2011 at 16:23, Matt Domsch <Matt_Domsch@dell.com> wrote:
>> On Tue, Dec 07, 2010 at 12:45:04AM -0200, Piter PUNK wrote:
>>> Matt Domsch wrote:
>
>> I need this in udev fairly quickly
>> please.  Some kind folks from IBM have been testing biosdevname with
>> quad-port Intel SR-IOV-capable devices, and instantiating all the VFs,
>> can easily have >100 eth* devices created in the kernel
>> instantaneously.  That bumps step 1 in the 2-step rename operation
>> over IFNAMSIZ  (eth100-pci2#3_62 is 16 chars).
>
> What is all this about?
>
> If biosdevname ever needs a two-step renaming, there is something
> wrong. Two-step renaming happens only for conflicting names. We do not
> want to support renaming in conflicting namespaces anymore.
>
> Also, we can not use hashes without checking for conflicts, it's
> unlikely to happen, but we just don't do such hacks. Just use the
> ifindex or whatever fits to be correct.
>
> Kay
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: net device renaming 2-step, IFNAMSIZ limit

[Matt Domsch]

> On Tue, Feb 8, 2011 at 6:42 AM, Kay Sievers <kay.sievers@vrfy.org> wrote:
> > If biosdevname ever needs a two-step renaming, there is something
> > wrong. Two-step renaming happens only for conflicting names. We do not
> > want to support renaming in conflicting namespaces anymore.

biosdevname by default uses --policy=physical, which uses the emN and
pci<slot>p<port>_<vf> convention.  I believed (incorrectly)
that all renames were 2-step, thus going from eth100 -> pci4p1_63
could overflow.  Since only renames in the same space use the 2-step
process, then this isn't as much of a concern.

biosdevname does have --policy=all_ethN, for those who really love
their ethN convention.  It's not default, and it's not in the udev
rules file to use it, but it would be an option.  As long
as we don't have >10k devices, we're ok.  


> > Also, we can not use hashes without checking for conflicts, it's
> > unlikely to happen, but we just don't do such hacks. Just use the
> > ifindex or whatever fits to be correct.

I do like the idea of using ifindex here instead of a hash, just
because we allow any arbitrary-length name to be used in a rename
rule.  biosdevname itself won't exceed the length, but if someone had
a rule that would move from eth12345 to eth67890, it would trigger.

Thanks,
Matt

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO