2004-03-04 01:02:18

by Hanna Linder

Subject: [PATCH 2.6] Patch to hook up PPP to simple class sysfs support

Here is a small patch to add PPP support to /sys/class.

Please consider for inclusion.

Thanks.

Hanna
----

diff -Nrup -Xdontdiff linux-2.6.3/drivers/net/ppp_generic.c linux-2.6.3p/drivers/net/ppp_generic.c
--- linux-2.6.3/drivers/net/ppp_generic.c 2004-02-17 19:59:31.000000000 -0800
+++ linux-2.6.3p/drivers/net/ppp_generic.c 2004-03-03 15:14:07.000000000 -0800
@@ -45,6 +45,7 @@
#include <linux/smp_lock.h>
#include <linux/rwsem.h>
#include <linux/stddef.h>
+#include <linux/device.h>
#include <net/slhc_vj.h>
#include <asm/atomic.h>

@@ -271,6 +272,8 @@ static int ppp_connect_channel(struct ch
static int ppp_disconnect_channel(struct channel *pch);
static void ppp_destroy_channel(struct channel *pch);

+static struct class_simple *ppp_class;
+
/* Translates a PPP protocol number to a NP index (NP == network protocol) */
static inline int proto_to_npindex(int proto)
{
@@ -799,10 +802,14 @@ static int __init ppp_init(void)
printk(KERN_INFO "PPP generic driver version " PPP_VERSION "\n");
err = register_chrdev(PPP_MAJOR, "ppp", &ppp_device_fops);
if (!err) {
+ ppp_class = class_simple_create(THIS_MODULE, "ppp");
+ class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
- if (err)
+ if (err) {
unregister_chrdev(PPP_MAJOR, "ppp");
+ class_simple_device_remove(MKDEV(PPP_MAJOR,0));
+ }
}

if (err)
@@ -2540,6 +2547,7 @@ static void __exit ppp_cleanup(void)
if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
printk(KERN_ERR "PPP: failed to unregister PPP device\n");
devfs_remove("ppp");
+ class_simple_device_remove(MKDEV(PPP_MAJOR, 0));
}

/*

2004-03-04 03:55:44

by Chris Wright

Subject: Re: [PATCH 2.6] Patch to hook up PPP to simple class sysfs support

* Hanna Linder ([email protected]) wrote:
> + ppp_class = class_simple_create(THIS_MODULE, "ppp");
> + class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");

What happens if that class_simple_create() fails? Actually,
class_simple_device_add could fail too, but doesn't seem anybody is
checking for that.

> err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
> S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
> - if (err)
> + if (err) {
> unregister_chrdev(PPP_MAJOR, "ppp");
> + class_simple_device_remove(MKDEV(PPP_MAJOR,0));
> + }

need to destroy the class on error path to avoid leak.

> @@ -2540,6 +2547,7 @@ static void __exit ppp_cleanup(void)
> if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
> printk(KERN_ERR "PPP: failed to unregister PPP device\n");
> devfs_remove("ppp");
> + class_simple_device_remove(MKDEV(PPP_MAJOR, 0));

ditto. this will leak and would cause oops on reload of module.

something like below.

thanks,
-chris

===== drivers/net/ppp_generic.c 1.43 vs edited =====
--- 1.43/drivers/net/ppp_generic.c Wed Feb 18 19:42:37 2004
+++ edited/drivers/net/ppp_generic.c Wed Mar 3 19:08:24 2004
@@ -45,6 +45,7 @@
#include <linux/smp_lock.h>
#include <linux/rwsem.h>
#include <linux/stddef.h>
+#include <linux/device.h>
#include <net/slhc_vj.h>
#include <asm/atomic.h>

@@ -271,6 +272,8 @@
static int ppp_disconnect_channel(struct channel *pch);
static void ppp_destroy_channel(struct channel *pch);

+static struct class_simple *ppp_class;
+
/* Translates a PPP protocol number to a NP index (NP == network protocol) */
static inline int proto_to_npindex(int proto)
{
@@ -804,15 +807,29 @@
printk(KERN_INFO "PPP generic driver version " PPP_VERSION "\n");
err = register_chrdev(PPP_MAJOR, "ppp", &ppp_device_fops);
if (!err) {
+ ppp_class = class_simple_create(THIS_MODULE, "ppp");
+ if (IS_ERR(ppp_class)) {
+ err = PTR_ERR(ppp_class);
+ goto out_chrdev;
+ }
+ class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
if (err)
- unregister_chrdev(PPP_MAJOR, "ppp");
+ goto out_class;
}

+out:
if (err)
printk(KERN_ERR "failed to register PPP device (%d)\n", err);
return err;
+
+out_class:
+ class_simple_device_remove(MKDEV(PPP_MAJOR,0));
+ class_simple_destroy(ppp_class);
+out_chrdev:
+ unregister_chrdev(PPP_MAJOR, "ppp");
+ goto out;
}

/*
@@ -2545,6 +2562,8 @@
if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
printk(KERN_ERR "PPP: failed to unregister PPP device\n");
devfs_remove("ppp");
+ class_simple_device_remove(MKDEV(PPP_MAJOR, 0));
+ class_simple_destroy(ppp_class);
}

/*

2004-03-04 05:16:41

by Jeff Garzik

Subject: Re: [PATCH 2.6] Patch to hook up PPP to simple class sysfs support

On Wed, Mar 03, 2004 at 05:03:26PM -0800, Hanna Linder wrote:
>
> Here is a small patch to add PPP support to /sys/class.
>
> Please consider for inclusion.
>
> Thanks.
>
> Hanna
> ----
>
> diff -Nrup -Xdontdiff linux-2.6.3/drivers/net/ppp_generic.c
> linux-2.6.3p/drivers/net/ppp_generic.c
> --- linux-2.6.3/drivers/net/ppp_generic.c 2004-02-17
> 19:59:31.000000000 -0800
> +++ linux-2.6.3p/drivers/net/ppp_generic.c 2004-03-03

Please CC the PPP maintainer (paulus) and the network developers
([email protected]) when modifying PPP.

Jeff

2004-03-05 02:04:42

by Hanna Linder

Subject: Re: [PATCH 2.6] Patch to hook up PPP to simple class sysfs support

Thanks Chris. Your changes fixed the reload oops I was seeing.

root@w-hlinder2 root]# modprobe ppp_generic
[root@w-hlinder2 root]# tree /sys/class/ppp
/sys/class/ppp
`-- ppp
`-- dev

1 directory, 1 file
[root@w-hlinder2 root]# rmmod ppp_generic
[root@w-hlinder2 root]# tree /sys/class/ppp
/sys/class/ppp [error opening dir]

0 directories, 0 files
[root@w-hlinder2 root]# modprobe ppp_generic
[root@w-hlinder2 root]# tree /sys/class/ppp
/sys/class/ppp
`-- ppp
`-- dev

1 directory, 1 file

Please consider the patch below for inclusion.

Thanks.


--On Wednesday, March 03, 2004 07:55:39 PM -0800 Chris Wright <[email protected]> wrote:

>
> something like below.
>
> thanks,
> -chris
>
> ===== drivers/net/ppp_generic.c 1.43 vs edited =====
> --- 1.43/drivers/net/ppp_generic.c Wed Feb 18 19:42:37 2004
> +++ edited/drivers/net/ppp_generic.c Wed Mar 3 19:08:24 2004
> @@ -45,6 +45,7 @@
> #include <linux/smp_lock.h>
> #include <linux/rwsem.h>
> #include <linux/stddef.h>
> +#include <linux/device.h>
> #include <net/slhc_vj.h>
> #include <asm/atomic.h>
>
> @@ -271,6 +272,8 @@
> static int ppp_disconnect_channel(struct channel *pch);
> static void ppp_destroy_channel(struct channel *pch);
>
> +static struct class_simple *ppp_class;
> +
> /* Translates a PPP protocol number to a NP index (NP == network protocol) */
> static inline int proto_to_npindex(int proto)
> {
> @@ -804,15 +807,29 @@
> printk(KERN_INFO "PPP generic driver version " PPP_VERSION "\n");
> err = register_chrdev(PPP_MAJOR, "ppp", &ppp_device_fops);
> if (!err) {
> + ppp_class = class_simple_create(THIS_MODULE, "ppp");
> + if (IS_ERR(ppp_class)) {
> + err = PTR_ERR(ppp_class);
> + goto out_chrdev;
> + }
> + class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
> err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
> S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
> if (err)
> - unregister_chrdev(PPP_MAJOR, "ppp");
> + goto out_class;
> }
>
> +out:
> if (err)
> printk(KERN_ERR "failed to register PPP device (%d)\n", err);
> return err;
> +
> +out_class:
> + class_simple_device_remove(MKDEV(PPP_MAJOR,0));
> + class_simple_destroy(ppp_class);
> +out_chrdev:
> + unregister_chrdev(PPP_MAJOR, "ppp");
> + goto out;
> }
>
> /*
> @@ -2545,6 +2562,8 @@
> if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
> printk(KERN_ERR "PPP: failed to unregister PPP device\n");
> devfs_remove("ppp");
> + class_simple_device_remove(MKDEV(PPP_MAJOR, 0));
> + class_simple_destroy(ppp_class);
> }
>
> /*
>

2004-03-11 01:45:32

by Greg KH

Subject: Re: [PATCH 2.6] Patch to hook up PPP to simple class sysfs support

On Wed, Mar 03, 2004 at 07:55:39PM -0800, Chris Wright wrote:
> * Hanna Linder ([email protected]) wrote:
> > + ppp_class = class_simple_create(THIS_MODULE, "ppp");
> > + class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
>
> What happens if that class_simple_create() fails? Actually,
> class_simple_device_add could fail too, but doesn't seem anybody is
> checking for that.
>
> > err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
> > S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
> > - if (err)
> > + if (err) {
> > unregister_chrdev(PPP_MAJOR, "ppp");
> > + class_simple_device_remove(MKDEV(PPP_MAJOR,0));
> > + }
>
> need to destroy the class on error path to avoid leak.
>
> > @@ -2540,6 +2547,7 @@ static void __exit ppp_cleanup(void)
> > if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
> > printk(KERN_ERR "PPP: failed to unregister PPP device\n");
> > devfs_remove("ppp");
> > + class_simple_device_remove(MKDEV(PPP_MAJOR, 0));
>
> ditto. this will leak and would cause oops on reload of module.
>
> something like below.

Applied, thanks.

greg k-h