[PATCH] Fix capi devicenames

[PATCH] Fix capi devicenames

[Marc - A. Dahlhaus]

[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]

capi device names should get changed to what userspace expects.
Without this change recent udev versions can't produce working device
nodes as the capi control device node name "capi" collides with the
"capi" directory under which userspace expects to get the application
device nodes. This also fixes the Documentation.

Signed-off-by: Marc - Andre Dahlhaus <mad@wol.de>

--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -1248,10 +1248,10 @@ Your cooperation is appreciated.

  68 char	CAPI 2.0 interface
 		  0 = /dev/capi20	Control device
-		  1 = /dev/capi20.00	First CAPI 2.0 application
-		  2 = /dev/capi20.01	Second CAPI 2.0 application
+		  1 = /dev/capi/0	First CAPI 2.0 application
+		  2 = /dev/capi/1	Second CAPI 2.0 application
 		    ...
-		 20 = /dev/capi20.19	19th CAPI 2.0 application
+		 20 = /dev/capi/19	19th CAPI 2.0 application

 		ISDN CAPI 2.0 driver for use with CAPI 2.0
 		applications; currently supports the AVM B1 card.
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1363,7 +1363,7 @@ static int __init capinc_tty_init(void)
 	}
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
-	drv->name = "capi";
+	drv->name = "capi/";
 	drv->major = 0;
 	drv->minor_start = 0;
 	drv->type = TTY_DRIVER_TYPE_SERIAL;
@@ -1509,7 +1509,7 @@ static int __init capi_init(void)
 		return PTR_ERR(capi_class);
 	}

-	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
+	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi20");

 	if (capinc_tty_init() < 0) {
 		device_destroy(capi_class, MKDEV(capi_major, 0));

[-- Attachment #2: capi-name.patch --]
[-- Type: text/plain, Size: 1607 bytes --]

capi device names should get changed to what userspace expects.
Without this change recent udev versions can't produce working device
nodes as the capi control device node name "capi" collides with the
"capi" directory under which userspace expects to get the application
device nodes. This also fixes the Documentation.

Signed-off-by: Marc - Andre Dahlhaus <mad@wol.de>

--- a/Documentation/devices.txt
+++ b/Documentation/devices.txt
@@ -1248,10 +1248,10 @@ Your cooperation is appreciated.
 
  68 char	CAPI 2.0 interface
 		  0 = /dev/capi20	Control device
-		  1 = /dev/capi20.00	First CAPI 2.0 application
-		  2 = /dev/capi20.01	Second CAPI 2.0 application
+		  1 = /dev/capi/0	First CAPI 2.0 application
+		  2 = /dev/capi/1	Second CAPI 2.0 application
 		    ...
-		 20 = /dev/capi20.19	19th CAPI 2.0 application
+		 20 = /dev/capi/19	19th CAPI 2.0 application
 
 		ISDN CAPI 2.0 driver for use with CAPI 2.0
 		applications; currently supports the AVM B1 card.
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1363,7 +1363,7 @@ static int __init capinc_tty_init(void)
 	}
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
-	drv->name = "capi";
+	drv->name = "capi/";
 	drv->major = 0;
 	drv->minor_start = 0;
 	drv->type = TTY_DRIVER_TYPE_SERIAL;
@@ -1509,7 +1509,7 @@ static int __init capi_init(void)
 		return PTR_ERR(capi_class);
 	}
 
-	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
+	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi20");
 
 	if (capinc_tty_init() < 0) {
 		device_destroy(capi_class, MKDEV(capi_major, 0));

Re: [PATCH] Fix capi devicenames

[Alan Cox]

On Wed, 29 Sep 2010 21:21:23 +0200
"Marc - A. Dahlhaus" <mad@wol.de> wrote:

> capi device names should get changed to what userspace expects.

If userspace isn't following the spec (which has been the spec for many
years) then thats not our problem.

> Without this change recent udev versions can't produce working device
> nodes as the capi control device node name "capi" collides with the
> "capi" directory under which userspace expects to get the application
> device nodes. This also fixes the Documentation.

The documentation isn't broken, the documentation *is* the reference.

NAK. Fix udev.


We have a spec for a reason, if some distro didn't follow it then that's
up to them but they can sort out their own udev.

Alan

Re: [PATCH] Fix capi devicenames

[Kay Sievers]

On Thu, Sep 30, 2010 at 11:30, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Wed, 29 Sep 2010 21:21:23 +0200
> "Marc - A. Dahlhaus" <mad@wol.de> wrote:
>
>> capi device names should get changed to what userspace expects.
>
> If userspace isn't following the spec (which has been the spec for many
> years) then thats not our problem.
>
>> Without this change recent udev versions can't produce working device
>> nodes as the capi control device node name "capi" collides with the
>> "capi" directory under which userspace expects to get the application
>> device nodes. This also fixes the Documentation.
>
> The documentation isn't broken, the documentation *is* the reference.
>
> NAK. Fix udev.
>
> We have a spec for a reason, if some distro didn't follow it then that's
> up to them but they can sort out their own udev.

There is nothing really to fix in udev, it knows nothing about ISDN.
The kernel defines the device node names these day, and udev manages
only permissions and possible symlinks.

devices.txt is just wrong in some things, and the common userspace
usage pattern is much closer to be a 'standard' here than this file.
devices.txt should catch up here. The same is true for scdX vs. srX,
scdX nodes don't exist today, and srX is not at all deprecated, unlike
devices.txt states.

The kernel provided ISDN nodes never made sense for applications using
ISDN. Also the devices.txt node names, I've never seen anywhere else
than in devices.txt.

The patch looks sane to me.

Kay

Re: [PATCH] Fix capi devicenames

[Marc - A. Dahlhaus]

Am Donnerstag, den 30.09.2010, 10:30 +0100 schrieb Alan Cox:
> On Wed, 29 Sep 2010 21:21:23 +0200
> "Marc - A. Dahlhaus" <mad@wol.de> wrote:
> 
> > capi device names should get changed to what userspace expects.
> 
> If userspace isn't following the spec (which has been the spec for many
> years) then thats not our problem.

Thanks Alan,

please read further, i think i didn't wrote the whole story in the patch
declarations.

> > Without this change recent udev versions can't produce working device
> > nodes as the capi control device node name "capi" collides with the
> > "capi" directory under which userspace expects to get the application
> > device nodes. This also fixes the Documentation.
> 
> The documentation isn't broken, the documentation *is* the reference.

The Documentation doesn't even represent what is in the code right now.
kernelcapi.ko creates /dev/capi as the capi control device
and /dev/capi[0-9]+ for the applications. Documentation states the
Devices are /dev/capi20 and /dev/capi20.[0-9]+ for applications.

> NAK. Fix udev.

I don't think udev is the right place to fix this.

Udev creates symlinks and is no longer able to overwrite the name
attribute.

CCing Kay Sievers for that.

> We have a spec for a reason, if some distro didn't follow it then that's
> up to them but they can sort out their own udev.

The problem is that eg. the capi module for pppd expects the application
nodes in a capifs conform path. capifs was in charge to create
the /dev/capi/[0-9]+ nodes but was deprecated and replaced by udev.

Old udev versions contained a isdn-40.rules file to create the capifs
nodes.

There was a discussion here about where to fix the false naming of
devices. And as far as i know it was decided that the right place is to
change them inside of the kernel and not via udev. Because of this udev
removed the possibility to change devicenodes to new names without
keeping the kernels suggested devicenode name. I hope i got this right.

So IMO the documentation is wrong. And also the kernelcapi module is
wrong about that node it creates.

The ISDN4Linux userspace expects the devicenodes in the way the patch
renames them for years:

http://www.isdn4linux.de/cgi-bin/viewcvs.cgi/isdn4k-utils/capiinit/capiinit.c?revision=1.17&view=markup

I just want that it works with the latest upstream sources out of the
box and this isn't the case right now.
CCing Karsten Keil for this.

> Alan

thanks,

Marc

Re: [PATCH] Fix capi devicenames

[Kay Sievers]

On Thu, Sep 30, 2010 at 12:37, Marc - A. Dahlhaus <mad@wol.de> wrote:
> Am Donnerstag, den 30.09.2010, 10:30 +0100 schrieb Alan Cox:
>> On Wed, 29 Sep 2010 21:21:23 +0200
>> "Marc - A. Dahlhaus" <mad@wol.de> wrote:
>>
>> > capi device names should get changed to what userspace expects.
>>
>> If userspace isn't following the spec (which has been the spec for many
>> years) then thats not our problem.

>> NAK. Fix udev.
>
> I don't think udev is the right place to fix this.
>
> Udev creates symlinks and is no longer able to overwrite the name
> attribute.

Right, udev has no business here. It's the kernels responsibility since a while.

If you can't rename the kernel devices right away for some reason, you
can add naming hints to the kernel, which udev and devtmpfs will
follow, and all should work.

In any case devices.txt should be fixed, it has not much to do with
the real world.

Kay

Re: [PATCH] Fix capi devicenames

[Alan Cox]

O> devices.txt is just wrong in some things, and the common userspace
> usage pattern is much closer to be a 'standard' here than this file.
> devices.txt should catch up here. The same is true for scdX vs. srX,
> scdX nodes don't exist today, and srX is not at all deprecated, unlike
> devices.txt states.

devices.txt is the spec.

> The kernel provided ISDN nodes never made sense for applications using
> ISDN. Also the devices.txt node names, I've never seen anywhere else
> than in devices.txt.

Doesn't matter - changing them will cause regressions.

If people want to rename them in userspace then fine, but changing the
default names for device nodes is not something to do lightly. You'll
break stuff for people who do follow the spec.

So NAK, and that's with the devices@lanana.org keeper of the device name
mappings hat on.

Alan

Re: [PATCH] Fix capi devicenames

[Alan Cox]

> In any case devices.txt should be fixed, it has not much to do with
> the real world.

If some distros chose not to follow devices.txt that is their problem.

devices.txt is the specification, and its ABI.

It is fixed and the kernel behaviour is to follow it. Those who didn't
follow it, or who didn't propose a change back when it was specified in
the first place have only themselves to blame.

It isn't changing, and the ISDN code should follow the spec.

How you sort out your userspace is a problem for those who dug holes and
jumped down them.

Re: [PATCH] Fix capi devicenames

[Alan Cox]

> The Documentation doesn't even represent what is in the code right now.
> kernelcapi.ko creates /dev/capi as the capi control device
> and /dev/capi[0-9]+ for the applications. Documentation states the
> Devices are /dev/capi20 and /dev/capi20.[0-9]+ for applications.

Ok that much needs sorting out.

> Because of this udev removed the possibility to change devicenodes to new names without
> keeping the kernels suggested devicenode name. I hope i got this right.

Not my problem. Udev can do what it likes

> So IMO the documentation is wrong. And also the kernelcapi module is
> wrong about that node it creates.

The problem is that the current default naming is ABI. The fact someone
screwed it up years ago and never fixed it merely makes it worse. At the
very least we need to sort out what the naming should be and support
*both* sets of names for a while. That would I think be an acceptable
path.

> I just want that it works with the latest upstream sources out of the
> box and this isn't the case right now.

That is fine, but I also want it to work on whatever random setups people
have had for the past ten years and on other distros which may be using
the kernel default names.

If we really do have a mess of that scale here then we need to follow the
proper sequence of things for obsoleting an interface, and that is

- Document it
- Add the new one
- Wait a long time
- Remove the old one

Alan

Re: [PATCH] Fix capi devicenames

[Marc - A. Dahlhaus]

Am Donnerstag, den 30.09.2010, 12:29 +0100 schrieb Alan Cox:
> > In any case devices.txt should be fixed, it has not much to do with
> > the real world.
> 
> If some distros chose not to follow devices.txt that is their problem.
> 
> devices.txt is the specification, and its ABI.
> 
> It is fixed and the kernel behaviour is to follow it. Those who didn't
> follow it, or who didn't propose a change back when it was specified in
> the first place have only themselves to blame.
> 
> It isn't changing, and the ISDN code should follow the spec.
> 
> How you sort out your userspace is a problem for those who dug holes and
> jumped down them.

Well, i doubt that any distro or hand crafted setup out there is using
anything else than the /dev/capi20 and /dev/capi/[0-9]+ devices for capi
interaction. Because the userspace (libcapi20 & capiinit) handle just
this devices. (i checked debian/ubuntu, fedora rawhide and gentoo and
all are not using other device node names...)

<minorrant>

The only setup that could get broken by this is the setup that uses udev
to handle the devices and gives the nodes different access rights or
something.

And that already happened by the time the rules used to create the capi
devices got removed from udev and it was right to remove them from udev
IMO.

What is a specification in devices.txt worth if nobody actually cares
about it in this case because it has evolved long time ago and nobody
cared to update the devices.txt file by then?

capifs is already deprecated and scheduled for removal soon because udev
is able to replace its functionality and fedora has a patch in srpm that
removes the capifs load from capiinit.

Userspace expects the capifs device nodes.

We are stuck by now. No matter what the specification states,
no setup will work with an up to date version of linux kernel, udev and
isdn4linux userspace now.

</minorrant>

Well, in the end, i don't get it:

How could we brake an already broken thing by fixing an broken
specification and make upstream respect what any distro out there does?

I could send a patch that "fixes" the device names according to the
broken specification in devices.txt and also send a rule for udev to
create symlinks that the userspace expects.

We would get useless kernel suggested devices for capi that nobody needs
because userspace will not use them, period. A well polluted /dev is the
end if this would be the right way to fix this case. Nice, isn't it ;)

I know the rules are made out of noble intends, but IMO the rule will
defeat the logical right fix for the problem that will not brake anyones
setup and there should be exception from the rule for this one case
because the things are how they are now. The spec will not help anyone
out there who is trying to use eg. his B1-ISDN card to receive a
faximile or to provide an old school dial-in access in cases where the
internet carrier is down once the upstream versions are used in the
distros...

Marc

Re: [PATCH] Fix capi devicenames

[Olivier Galibert]

On Thu, Sep 30, 2010 at 02:50:24PM +0200, Marc - A. Dahlhaus wrote:
[...]
> The only setup that could get broken by this is the setup that uses udev
> to handle the devices and gives the nodes different access rights or
> something.
> 
> And that already happened by the time the rules used to create the capi
> devices got removed from udev and it was right to remove them from udev
> IMO.
[...]
> capifs is already deprecated and scheduled for removal soon because udev
> is able to replace its functionality and fedora has a patch in srpm that
> removes the capifs load from capiinit.
> 
> Userspace expects the capifs device nodes.
[...]

So if I understand correctly, capifs was deprecated and is soon be
removed because udev was able to replace it given a set of rules, then
after the deprecation was done the rules were deliberately removed
from udev.  And that's supposed to be a kernel issue?

Sounds... typical, actually.

  OG.

Re: [PATCH] Fix capi devicenames

[Kay Sievers]

n Thu, Sep 30, 2010 at 14:50, Marc - A. Dahlhaus <mad@wol.de> wrote:
> Am Donnerstag, den 30.09.2010, 12:29 +0100 schrieb Alan Cox:
>> > In any case devices.txt should be fixed, it has not much to do with
>> > the real world.
>>
>> If some distros chose not to follow devices.txt that is their problem.
>>
>> devices.txt is the specification, and its ABI.
>>
>> It is fixed and the kernel behaviour is to follow it. Those who didn't
>> follow it, or who didn't propose a change back when it was specified in
>> the first place have only themselves to blame.
>>
>> It isn't changing, and the ISDN code should follow the spec.
>>
>> How you sort out your userspace is a problem for those who dug holes and
>> jumped down them.
>
> Well, i doubt that any distro or hand crafted setup out there is using
> anything else than the /dev/capi20 and /dev/capi/[0-9]+ devices for capi
> interaction. Because the userspace (libcapi20 & capiinit) handle just
> this devices. (i checked debian/ubuntu, fedora rawhide and gentoo and
> all are not using other device node names...)

devices.txt is just bogus in this case and states stuff which was
never used in the real world. But you don't need to touch it. Just add
the naming hints (char *(*devnode)) to the capi class and nothing will
change, the device names in /sys and everywhere else will stay as they
are today, just devtmpfs and udev will create the right device nodes
out of the kernel supplied additional information. We did this for the
rest of the kernel the same way.

Kay

[PATCH] Fix capi devicenames v2

[Marc - A. Dahlhaus]

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]

Specification in devices.txt about capi devices is out of date as the capifs
layout for capi tty devices was never added to it.

Recent udev versions can't produce working device nodes because of a typo in
the device_create call for the capi control device.

Due to this the wrong node name of the control device "capi" collides with
the "capi" directory under which the now deprecated capifs was mounted and
created the capi tty device nodes.

This also changes the capi tty device names passed to udev to the capifs
layout which isn't mentioned in the specs.

Signed-off-by: Marc - Andre Dahlhaus <mad@wol.de>

--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1363,7 +1363,7 @@ static int __init capinc_tty_init(void)
 	}
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
-	drv->name = "capi";
+	drv->name = "capi/";
 	drv->major = 0;
 	drv->minor_start = 0;
 	drv->type = TTY_DRIVER_TYPE_SERIAL;
@@ -1509,7 +1509,7 @@ static int __init capi_init(void)
 		return PTR_ERR(capi_class);
 	}
 
-	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
+	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi20");
 
 	if (capinc_tty_init() < 0) {
 		device_destroy(capi_class, MKDEV(capi_major, 0));

[-- Attachment #2: capi-name.patch --]
[-- Type: text/x-patch, Size: 1249 bytes --]

Specification in devices.txt about capi devices is out of date as the capifs
layout for capi tty devices was never added to it.

Recent udev versions can't produce working device nodes because of a typo in
the device_create call for the capi control device.

Due to this the wrong node name of the control device "capi" collides with
the "capi" directory under which the now deprecated capifs was mounted and
created the capi tty device nodes.

This also changes the capi tty device names passed to udev to the capifs
layout which isn't mentioned in the specs.

Signed-off-by: Marc - Andre Dahlhaus <mad@wol.de>

--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -1363,7 +1363,7 @@ static int __init capinc_tty_init(void)
 	}
 	drv->owner = THIS_MODULE;
 	drv->driver_name = "capi_nc";
-	drv->name = "capi";
+	drv->name = "capi/";
 	drv->major = 0;
 	drv->minor_start = 0;
 	drv->type = TTY_DRIVER_TYPE_SERIAL;
@@ -1509,7 +1509,7 @@ static int __init capi_init(void)
 		return PTR_ERR(capi_class);
 	}
 
-	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi");
+	device_create(capi_class, NULL, MKDEV(capi_major, 0), NULL, "capi20");
 
 	if (capinc_tty_init() < 0) {
 		device_destroy(capi_class, MKDEV(capi_major, 0));

Re: [PATCH] Fix capi devicenames

[Marc - A. Dahlhaus]

Am Donnerstag, den 30.09.2010, 21:20 +0200 schrieb Kay Sievers:
--8<--
> 
> devices.txt is just bogus in this case and states stuff which was
> never used in the real world. But you don't need to touch it. Just add
> the naming hints (char *(*devnode)) to the capi class and nothing will
> change, the device names in /sys and everywhere else will stay as they
> are today, just devtmpfs and udev will create the right device nodes
> out of the kernel supplied additional information. We did this for the
> rest of the kernel the same way.

Hello Kay,

for the capi tty, if not mistaken, it looks like i need to add a
dev->devnode = 'capi/';
line but i have no clue what it needs to change the capi_class.
Looks like a larger rework of the code in capi.c to me.

It would collide in the kernel if i fix the typo for the control device
but don't change the tty device name by the same time.
(20th capi application tty would become capi20 under sysfs)

So i took the approach to ignore the devices.txt in the v2 patch and
just fix the control device node to conform to the spec and also fix the
tty node names to the ones of capifs that superseded the manualy created
tty device nodes mentioned in devices.txt

Hope this is more appropriate than my first patch.

Is there a documentation that could guide me in how to fix it in the way
you suggested, if the v2 patch also gets naked?

thanks,

Marc

Re: [PATCH] Fix capi devicenames

[Kay Sievers]

On Fri, Oct 1, 2010 at 10:36, Marc - A. Dahlhaus <mad@wol.de> wrote:
> Am Donnerstag, den 30.09.2010, 21:20 +0200 schrieb Kay Sievers:

> for the capi tty, if not mistaken, it looks like i need to add a
> dev->devnode = 'capi/';
> line but i have no clue what it needs to change the capi_class.
> Looks like a larger rework of the code in capi.c to me.

You should be able to do just do what we did for all other stuff that
lives in its own /dev sub-directory:
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=aa5ed63e96656bb246a9439e06c7b67d455a5aa1

Kay