* [PATCH 0/2] ppp: add netlink support
@ 2015-12-23 20:04 Guillaume Nault
2015-12-23 20:04 ` [PATCH 1/2] ppp: define reusable device creation functions Guillaume Nault
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Guillaume Nault @ 2015-12-23 20:04 UTC (permalink / raw)
To: netdev; +Cc: linux-ppp, David Miller, Paul Mackerras
This series adds netlink support for creating PPP devices.
It doesn't implement any PPP specific attribute beyond IFLA_PPP_DEV_FD,
which is necessary in the PPP device model for passing control plane
packets to user space.
Guillaume Nault (2):
ppp: define reusable device creation functions
ppp: implement rtnetlink device handling
drivers/net/ppp/ppp_generic.c | 319 ++++++++++++++++++++++++++++++------------
include/uapi/linux/if_link.h | 8 ++
2 files changed, 237 insertions(+), 90 deletions(-)
--
2.6.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] ppp: define reusable device creation functions 2015-12-23 20:04 [PATCH 0/2] ppp: add netlink support Guillaume Nault @ 2015-12-23 20:04 ` Guillaume Nault 2015-12-23 20:04 ` [PATCH 2/2] ppp: implement rtnetlink device handling Guillaume Nault ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Guillaume Nault @ 2015-12-23 20:04 UTC (permalink / raw) To: netdev; +Cc: linux-ppp, David Miller, Paul Mackerras Move PPP device initialisation and registration out of ppp_create_interface(). This prepares code for device registration with rtnetlink. While there, cleanup ppp_create_interface() prototype to return an error code rather than the ppp structure. The unit parameter is now read/write so that caller can retrieve the unit identifier without accessing the new ppp structure. Signed-off-by: Guillaume Nault <g.nault@alphalink.fr> --- drivers/net/ppp/ppp_generic.c | 216 +++++++++++++++++++++++++----------------- 1 file changed, 127 insertions(+), 89 deletions(-) diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c index fc8ad00..bc24537 100644 --- a/drivers/net/ppp/ppp_generic.c +++ b/drivers/net/ppp/ppp_generic.c @@ -183,6 +183,11 @@ struct channel { #endif /* CONFIG_PPP_MULTILINK */ }; +struct ppp_config { + struct file *file; + s32 unit; +}; + /* * SMP locking issues: * Both the ppp.rlock and ppp.wlock locks protect the ppp.channels @@ -269,8 +274,7 @@ static void ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound); static void ppp_ccp_closed(struct ppp *ppp); static struct compressor *find_compressor(int type); static void ppp_get_stats(struct ppp *ppp, struct ppp_stats *st); -static struct ppp *ppp_create_interface(struct net *net, int unit, - struct file *file, int *retp); +static int ppp_create_interface(struct net *net, int *unit, struct file *file); static void init_ppp_file(struct ppp_file *pf, int kind); static void ppp_destroy_interface(struct ppp *ppp); static struct ppp *ppp_find_unit(struct ppp_net *pn, int unit); @@ -842,12 +846,13 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf, /* Create a new ppp unit */ if (get_user(unit, p)) break; - ppp = ppp_create_interface(net, unit, file, &err); - if (!ppp) + + err = ppp_create_interface(net, &unit, file); + if (err) break; - file->private_data = &ppp->file; + err = -EFAULT; - if (put_user(ppp->file.index, p)) + if (put_user(unit, p)) break; err = 0; break; @@ -949,6 +954,101 @@ static struct pernet_operations ppp_net_ops = { .size = sizeof(struct ppp_net), }; +static int ppp_unit_register(struct ppp *ppp, int unit) +{ + struct ppp_net *pn = ppp_pernet(ppp->ppp_net); + int ret; + + mutex_lock(&pn->all_ppp_mutex); + + if (unit < 0) { + ret = unit_get(&pn->units_idr, ppp); + if (ret < 0) + goto err; + } else { + /* Caller asked for a specific unit number. Fail with -EEXIST + * if unavailable. For backward compatibility, return -EEXIST + * too if idr allocation fails; this makes pppd retry without + * requesting a specific unit number. + */ + if (unit_find(&pn->units_idr, unit)) { + ret = -EEXIST; + goto err; + } + ret = unit_set(&pn->units_idr, ppp, unit); + if (ret < 0) { + ret = -EEXIST; /* rewrite error for backward compat. */ + goto err; + } + } + + ppp->file.index = ret; + + snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index); + + ret = register_netdevice(ppp->dev); + if (ret) + goto err_unit; + + atomic_inc(&ppp_unit_count); + + mutex_unlock(&pn->all_ppp_mutex); + + return 0; + +err_unit: + unit_put(&pn->units_idr, ppp->file.index); +err: + mutex_unlock(&pn->all_ppp_mutex); + + return ret; +} + +static int ppp_dev_configure(struct net *src_net, struct net_device *dev, + const struct ppp_config *conf) +{ + struct ppp *ppp = netdev_priv(dev); + struct file *file; + int indx; + int err; + + file = conf->file; + if (!file) + return -EBADF; + if (file->private_data) + return -EBUSY; + + init_ppp_file(&ppp->file, INTERFACE); + ppp->file.hdrlen = PPP_HDRLEN - 2; /* don't count proto bytes */ + ppp->owner = file; + + for (indx = 0; indx < NUM_NP; ++indx) + ppp->npmode[indx] = NPMODE_PASS; + INIT_LIST_HEAD(&ppp->channels); + spin_lock_init(&ppp->rlock); + spin_lock_init(&ppp->wlock); +#ifdef CONFIG_PPP_MULTILINK + ppp->minseq = -1; + skb_queue_head_init(&ppp->mrq); +#endif /* CONFIG_PPP_MULTILINK */ +#ifdef CONFIG_PPP_FILTER + ppp->pass_filter = NULL; + ppp->active_filter = NULL; +#endif /* CONFIG_PPP_FILTER */ + + ppp->dev = dev; + ppp->mru = PPP_MRU; + ppp->ppp_net = src_net; + + err = ppp_unit_register(ppp, conf->unit); + if (err < 0) + return err; + + file->private_data = &ppp->file; + + return 0; +} + #define PPP_MAJOR 108 /* Called at boot time if ppp is compiled into the kernel, @@ -2717,101 +2817,39 @@ ppp_get_stats(struct ppp *ppp, struct ppp_stats *st) * or if there is already a unit with the requested number. * unit = -1 means allocate a new number. */ -static struct ppp *ppp_create_interface(struct net *net, int unit, - struct file *file, int *retp) +static int ppp_create_interface(struct net *net, int *unit, struct file *file) { + struct ppp_config conf = { + .file = file, + .unit = *unit, + }; + struct net_device *dev; struct ppp *ppp; - struct ppp_net *pn; - struct net_device *dev = NULL; - int ret = -ENOMEM; - int i; + int err; dev = alloc_netdev(sizeof(struct ppp), "", NET_NAME_ENUM, ppp_setup); - if (!dev) - goto out1; - - pn = ppp_pernet(net); - - ppp = netdev_priv(dev); - ppp->dev = dev; - ppp->mru = PPP_MRU; - init_ppp_file(&ppp->file, INTERFACE); - ppp->file.hdrlen = PPP_HDRLEN - 2; /* don't count proto bytes */ - ppp->owner = file; - for (i = 0; i < NUM_NP; ++i) - ppp->npmode[i] = NPMODE_PASS; - INIT_LIST_HEAD(&ppp->channels); - spin_lock_init(&ppp->rlock); - spin_lock_init(&ppp->wlock); -#ifdef CONFIG_PPP_MULTILINK - ppp->minseq = -1; - skb_queue_head_init(&ppp->mrq); -#endif /* CONFIG_PPP_MULTILINK */ -#ifdef CONFIG_PPP_FILTER - ppp->pass_filter = NULL; - ppp->active_filter = NULL; -#endif /* CONFIG_PPP_FILTER */ + if (!dev) { + err = -ENOMEM; + goto err; + } - /* - * drum roll: don't forget to set - * the net device is belong to - */ dev_net_set(dev, net); rtnl_lock(); - mutex_lock(&pn->all_ppp_mutex); - - if (unit < 0) { - unit = unit_get(&pn->units_idr, ppp); - if (unit < 0) { - ret = unit; - goto out2; - } - } else { - ret = -EEXIST; - if (unit_find(&pn->units_idr, unit)) - goto out2; /* unit already exists */ - /* - * if caller need a specified unit number - * lets try to satisfy him, otherwise -- - * he should better ask us for new unit number - * - * NOTE: yes I know that returning EEXIST it's not - * fair but at least pppd will ask us to allocate - * new unit in this case so user is happy :) - */ - unit = unit_set(&pn->units_idr, ppp, unit); - if (unit < 0) - goto out2; - } - - /* Initialize the new ppp unit */ - ppp->file.index = unit; - sprintf(dev->name, "ppp%d", unit); - - ret = register_netdevice(dev); - if (ret != 0) { - unit_put(&pn->units_idr, unit); - netdev_err(ppp->dev, "PPP: couldn't register device %s (%d)\n", - dev->name, ret); - goto out2; - } - - ppp->ppp_net = net; - - atomic_inc(&ppp_unit_count); - mutex_unlock(&pn->all_ppp_mutex); + err = ppp_dev_configure(net, dev, &conf); rtnl_unlock(); + if (err) + goto err_dev; - *retp = 0; - return ppp; + ppp = netdev_priv(dev); + *unit = ppp->file.index; -out2: - mutex_unlock(&pn->all_ppp_mutex); + return 0; + +err_dev: free_netdev(dev); -out1: - *retp = ret; - return NULL; +err: + return err; } /* -- 2.6.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ppp: implement rtnetlink device handling 2015-12-23 20:04 [PATCH 0/2] ppp: add netlink support Guillaume Nault 2015-12-23 20:04 ` [PATCH 1/2] ppp: define reusable device creation functions Guillaume Nault @ 2015-12-23 20:04 ` Guillaume Nault 2016-01-25 11:09 ` walter harms 2015-12-23 20:09 ` [PATCH 0/2] ppp: add netlink support Guillaume Nault 2016-01-05 18:10 ` Guillaume Nault 3 siblings, 1 reply; 9+ messages in thread From: Guillaume Nault @ 2015-12-23 20:04 UTC (permalink / raw) To: netdev; +Cc: linux-ppp, David Miller, Paul Mackerras Define PPP device handler for use with rtnetlink. The only PPP specific attribute is IFLA_PPP_DEV_FD. It is mandatory and contains the file descriptor of the associated /dev/ppp instance (the file descriptor which would have been used for ioctl(PPPIOCNEWUNIT) in the ioctl-based API). PPP devices created with the rtnetlink API behave like the ones created with ioctl(PPPIOCNEWUNIT). In particular existing ioctls still apply to PPP units created with the rtnetlink API. Signed-off-by: Guillaume Nault <g.nault@alphalink.fr> --- drivers/net/ppp/ppp_generic.c | 109 ++++++++++++++++++++++++++++++++++++++++-- include/uapi/linux/if_link.h | 8 ++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c index bc24537..19476d6 100644 --- a/drivers/net/ppp/ppp_generic.c +++ b/drivers/net/ppp/ppp_generic.c @@ -46,6 +46,7 @@ #include <linux/device.h> #include <linux/mutex.h> #include <linux/slab.h> +#include <linux/file.h> #include <asm/unaligned.h> #include <net/slhc_vj.h> #include <linux/atomic.h> @@ -185,7 +186,9 @@ struct channel { struct ppp_config { struct file *file; + s32 fd; s32 unit; + bool ifname_is_set; }; /* @@ -286,6 +289,7 @@ static int unit_get(struct idr *p, void *ptr); static int unit_set(struct idr *p, void *ptr, int n); static void unit_put(struct idr *p, int n); static void *unit_find(struct idr *p, int n); +static void ppp_setup(struct net_device *dev); static const struct net_device_ops ppp_netdev_ops; @@ -954,7 +958,7 @@ static struct pernet_operations ppp_net_ops = { .size = sizeof(struct ppp_net), }; -static int ppp_unit_register(struct ppp *ppp, int unit) +static int ppp_unit_register(struct ppp *ppp, int unit, bool ifname_is_set) { struct ppp_net *pn = ppp_pernet(ppp->ppp_net); int ret; @@ -984,7 +988,8 @@ static int ppp_unit_register(struct ppp *ppp, int unit) ppp->file.index = ret; - snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index); + if (!ifname_is_set) + snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index); ret = register_netdevice(ppp->dev); if (ret) @@ -1012,7 +1017,24 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, int indx; int err; - file = conf->file; + if (conf->fd >= 0) { + file = fget(conf->fd); + if (file) { + if (file->f_op != &ppp_device_fops) { + fput(file); + return -EBADF; + } + + /* Don't hold reference on file: ppp_release() is + * responsible for safely freeing the associated + * resources upon release. So file won't go away + * from under us. + */ + fput(file); + } + } else { + file = conf->file; + } if (!file) return -EBADF; if (file->private_data) @@ -1040,7 +1062,7 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, ppp->mru = PPP_MRU; ppp->ppp_net = src_net; - err = ppp_unit_register(ppp, conf->unit); + err = ppp_unit_register(ppp, conf->unit, conf->ifname_is_set); if (err < 0) return err; @@ -1049,6 +1071,73 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, return 0; } +static const struct nla_policy ppp_nl_policy[IFLA_PPP_MAX + 1] = { + [IFLA_PPP_DEV_FD] = { .type = NLA_S32 }, +}; + +static int ppp_nl_validate(struct nlattr *tb[], struct nlattr *data[]) +{ + if (!data) + return -EINVAL; + + if (!data[IFLA_PPP_DEV_FD]) + return -EINVAL; + if (nla_get_s32(data[IFLA_PPP_DEV_FD]) < 0) + return -EBADF; + + return 0; +} + +static int ppp_nl_newlink(struct net *src_net, struct net_device *dev, + struct nlattr *tb[], struct nlattr *data[]) +{ + struct ppp_config conf = { + .file = NULL, + .unit = -1, + .ifname_is_set = true, + }; + + conf.fd = nla_get_s32(data[IFLA_PPP_DEV_FD]); + + return ppp_dev_configure(src_net, dev, &conf); +} + +static void ppp_nl_dellink(struct net_device *dev, struct list_head *head) +{ + unregister_netdevice_queue(dev, head); +} + +static size_t ppp_nl_get_size(const struct net_device *dev) +{ + return 0; +} + +static int ppp_nl_fill_info(struct sk_buff *skb, const struct net_device *dev) +{ + return 0; +} + +static struct net *ppp_nl_get_link_net(const struct net_device *dev) +{ + struct ppp *ppp = netdev_priv(dev); + + return ppp->ppp_net; +} + +static struct rtnl_link_ops ppp_link_ops __read_mostly = { + .kind = "ppp", + .maxtype = IFLA_PPP_MAX, + .policy = ppp_nl_policy, + .priv_size = sizeof(struct ppp), + .setup = ppp_setup, + .validate = ppp_nl_validate, + .newlink = ppp_nl_newlink, + .dellink = ppp_nl_dellink, + .get_size = ppp_nl_get_size, + .fill_info = ppp_nl_fill_info, + .get_link_net = ppp_nl_get_link_net, +}; + #define PPP_MAJOR 108 /* Called at boot time if ppp is compiled into the kernel, @@ -1077,11 +1166,19 @@ static int __init ppp_init(void) goto out_chrdev; } + err = rtnl_link_register(&ppp_link_ops); + if (err) { + pr_err("Failed to register RT netlink PPP handler\n"); + goto out_class; + } + /* not a big deal if we fail here :-) */ device_create(ppp_class, NULL, MKDEV(PPP_MAJOR, 0), NULL, "ppp"); return 0; +out_class: + class_destroy(ppp_class); out_chrdev: unregister_chrdev(PPP_MAJOR, "ppp"); out_net: @@ -2821,7 +2918,9 @@ static int ppp_create_interface(struct net *net, int *unit, struct file *file) { struct ppp_config conf = { .file = file, + .fd = -1, .unit = *unit, + .ifname_is_set = false, }; struct net_device *dev; struct ppp *ppp; @@ -3038,6 +3137,7 @@ static void __exit ppp_cleanup(void) /* should never happen */ if (atomic_read(&ppp_unit_count) || atomic_read(&channel_count)) pr_err("PPP: removing module but units remain!\n"); + rtnl_link_unregister(&ppp_link_ops); unregister_chrdev(PPP_MAJOR, "ppp"); device_destroy(ppp_class, MKDEV(PPP_MAJOR, 0)); class_destroy(ppp_class); @@ -3096,4 +3196,5 @@ EXPORT_SYMBOL(ppp_register_compressor); EXPORT_SYMBOL(ppp_unregister_compressor); MODULE_LICENSE("GPL"); MODULE_ALIAS_CHARDEV(PPP_MAJOR, 0); +MODULE_ALIAS_RTNL_LINK("ppp"); MODULE_ALIAS("devname:ppp"); diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index a30b780..72b0213 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -470,6 +470,14 @@ enum { }; #define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1) +/* PPP section */ +enum { + IFLA_PPP_UNSPEC, + IFLA_PPP_DEV_FD, + __IFLA_PPP_MAX, +}; +#define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) + /* Bonding section */ enum { -- 2.6.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ppp: implement rtnetlink device handling 2015-12-23 20:04 ` [PATCH 2/2] ppp: implement rtnetlink device handling Guillaume Nault @ 2016-01-25 11:09 ` walter harms 2016-01-25 15:28 ` Guillaume Nault 0 siblings, 1 reply; 9+ messages in thread From: walter harms @ 2016-01-25 11:09 UTC (permalink / raw) To: Guillaume Nault; +Cc: netdev, linux-ppp, David Miller, Paul Mackerras Am 23.12.2015 21:04, schrieb Guillaume Nault: > Define PPP device handler for use with rtnetlink. > > The only PPP specific attribute is IFLA_PPP_DEV_FD. It is mandatory and > contains the file descriptor of the associated /dev/ppp instance (the > file descriptor which would have been used for ioctl(PPPIOCNEWUNIT) in > the ioctl-based API). > > PPP devices created with the rtnetlink API behave like the ones created > with ioctl(PPPIOCNEWUNIT). In particular existing ioctls still apply to > PPP units created with the rtnetlink API. > > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr> > --- > drivers/net/ppp/ppp_generic.c | 109 ++++++++++++++++++++++++++++++++++++++++-- > include/uapi/linux/if_link.h | 8 ++++ > 2 files changed, 113 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c > index bc24537..19476d6 100644 > --- a/drivers/net/ppp/ppp_generic.c > +++ b/drivers/net/ppp/ppp_generic.c > @@ -46,6 +46,7 @@ > #include <linux/device.h> > #include <linux/mutex.h> > #include <linux/slab.h> > +#include <linux/file.h> > #include <asm/unaligned.h> > #include <net/slhc_vj.h> > #include <linux/atomic.h> > @@ -185,7 +186,9 @@ struct channel { > > struct ppp_config { > struct file *file; > + s32 fd; > s32 unit; > + bool ifname_is_set; > }; > > /* > @@ -286,6 +289,7 @@ static int unit_get(struct idr *p, void *ptr); > static int unit_set(struct idr *p, void *ptr, int n); > static void unit_put(struct idr *p, int n); > static void *unit_find(struct idr *p, int n); > +static void ppp_setup(struct net_device *dev); > > static const struct net_device_ops ppp_netdev_ops; > > @@ -954,7 +958,7 @@ static struct pernet_operations ppp_net_ops = { > .size = sizeof(struct ppp_net), > }; > > -static int ppp_unit_register(struct ppp *ppp, int unit) > +static int ppp_unit_register(struct ppp *ppp, int unit, bool ifname_is_set) > { > struct ppp_net *pn = ppp_pernet(ppp->ppp_net); > int ret; > @@ -984,7 +988,8 @@ static int ppp_unit_register(struct ppp *ppp, int unit) > > ppp->file.index = ret; > > - snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index); > + if (!ifname_is_set) > + snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index); > > ret = register_netdevice(ppp->dev); > if (ret) > @@ -1012,7 +1017,24 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, > int indx; > int err; > > - file = conf->file; > + if (conf->fd >= 0) { > + file = fget(conf->fd); > + if (file) { > + if (file->f_op != &ppp_device_fops) { > + fput(file); > + return -EBADF; > + } > + > + /* Don't hold reference on file: ppp_release() is > + * responsible for safely freeing the associated > + * resources upon release. So file won't go away > + * from under us. > + */ > + fput(file); > + } > + } else { > + file = conf->file; > + } > if (!file) > return -EBADF; I would write that a bid different to reduce indent und improve readability (note: totaly untested just reviewing) if (conf->fd < 0) { file = conf->file; if (!file) return -EBADF; } else { file = fget(conf->fd); if (!file) return -EBADF; fput(file); if (file->f_op != &ppp_device_fops) { return -EBADF; } } just my 2 cents re, wh > if (file->private_data) > @@ -1040,7 +1062,7 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, > ppp->mru = PPP_MRU; > ppp->ppp_net = src_net; > > - err = ppp_unit_register(ppp, conf->unit); > + err = ppp_unit_register(ppp, conf->unit, conf->ifname_is_set); > if (err < 0) > return err; > > @@ -1049,6 +1071,73 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, > return 0; > } > > +static const struct nla_policy ppp_nl_policy[IFLA_PPP_MAX + 1] = { > + [IFLA_PPP_DEV_FD] = { .type = NLA_S32 }, > +}; > + > +static int ppp_nl_validate(struct nlattr *tb[], struct nlattr *data[]) > +{ > + if (!data) > + return -EINVAL; > + > + if (!data[IFLA_PPP_DEV_FD]) > + return -EINVAL; > + if (nla_get_s32(data[IFLA_PPP_DEV_FD]) < 0) > + return -EBADF; > + > + return 0; > +} > + > +static int ppp_nl_newlink(struct net *src_net, struct net_device *dev, > + struct nlattr *tb[], struct nlattr *data[]) > +{ > + struct ppp_config conf = { > + .file = NULL, > + .unit = -1, > + .ifname_is_set = true, > + }; > + > + conf.fd = nla_get_s32(data[IFLA_PPP_DEV_FD]); > + > + return ppp_dev_configure(src_net, dev, &conf); > +} > + > +static void ppp_nl_dellink(struct net_device *dev, struct list_head *head) > +{ > + unregister_netdevice_queue(dev, head); > +} > + > +static size_t ppp_nl_get_size(const struct net_device *dev) > +{ > + return 0; > +} > + > +static int ppp_nl_fill_info(struct sk_buff *skb, const struct net_device *dev) > +{ > + return 0; > +} > + > +static struct net *ppp_nl_get_link_net(const struct net_device *dev) > +{ > + struct ppp *ppp = netdev_priv(dev); > + > + return ppp->ppp_net; > +} > + > +static struct rtnl_link_ops ppp_link_ops __read_mostly = { > + .kind = "ppp", > + .maxtype = IFLA_PPP_MAX, > + .policy = ppp_nl_policy, > + .priv_size = sizeof(struct ppp), > + .setup = ppp_setup, > + .validate = ppp_nl_validate, > + .newlink = ppp_nl_newlink, > + .dellink = ppp_nl_dellink, > + .get_size = ppp_nl_get_size, > + .fill_info = ppp_nl_fill_info, > + .get_link_net = ppp_nl_get_link_net, > +}; > + > #define PPP_MAJOR 108 > > /* Called at boot time if ppp is compiled into the kernel, > @@ -1077,11 +1166,19 @@ static int __init ppp_init(void) > goto out_chrdev; > } > > + err = rtnl_link_register(&ppp_link_ops); > + if (err) { > + pr_err("Failed to register RT netlink PPP handler\n"); > + goto out_class; > + } > + > /* not a big deal if we fail here :-) */ > device_create(ppp_class, NULL, MKDEV(PPP_MAJOR, 0), NULL, "ppp"); > > return 0; > > +out_class: > + class_destroy(ppp_class); > out_chrdev: > unregister_chrdev(PPP_MAJOR, "ppp"); > out_net: > @@ -2821,7 +2918,9 @@ static int ppp_create_interface(struct net *net, int *unit, struct file *file) > { > struct ppp_config conf = { > .file = file, > + .fd = -1, > .unit = *unit, > + .ifname_is_set = false, > }; > struct net_device *dev; > struct ppp *ppp; > @@ -3038,6 +3137,7 @@ static void __exit ppp_cleanup(void) > /* should never happen */ > if (atomic_read(&ppp_unit_count) || atomic_read(&channel_count)) > pr_err("PPP: removing module but units remain!\n"); > + rtnl_link_unregister(&ppp_link_ops); > unregister_chrdev(PPP_MAJOR, "ppp"); > device_destroy(ppp_class, MKDEV(PPP_MAJOR, 0)); > class_destroy(ppp_class); > @@ -3096,4 +3196,5 @@ EXPORT_SYMBOL(ppp_register_compressor); > EXPORT_SYMBOL(ppp_unregister_compressor); > MODULE_LICENSE("GPL"); > MODULE_ALIAS_CHARDEV(PPP_MAJOR, 0); > +MODULE_ALIAS_RTNL_LINK("ppp"); > MODULE_ALIAS("devname:ppp"); > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index a30b780..72b0213 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -470,6 +470,14 @@ enum { > }; > #define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1) > > +/* PPP section */ > +enum { > + IFLA_PPP_UNSPEC, > + IFLA_PPP_DEV_FD, > + __IFLA_PPP_MAX, > +}; > +#define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) > + > /* Bonding section */ > > enum { ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ppp: implement rtnetlink device handling 2016-01-25 11:09 ` walter harms @ 2016-01-25 15:28 ` Guillaume Nault 0 siblings, 0 replies; 9+ messages in thread From: Guillaume Nault @ 2016-01-25 15:28 UTC (permalink / raw) To: walter harms; +Cc: netdev, linux-ppp, David Miller, Paul Mackerras On Mon, Jan 25, 2016 at 12:09:34PM +0100, walter harms wrote: > > > Am 23.12.2015 21:04, schrieb Guillaume Nault: > > @@ -1012,7 +1017,24 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev, > > int indx; > > int err; > > > > - file = conf->file; > > + if (conf->fd >= 0) { > > + file = fget(conf->fd); > > + if (file) { > > + if (file->f_op != &ppp_device_fops) { > > + fput(file); > > + return -EBADF; > > + } > > + > > + /* Don't hold reference on file: ppp_release() is > > + * responsible for safely freeing the associated > > + * resources upon release. So file won't go away > > + * from under us. > > + */ > > + fput(file); > > + } > > + } else { > > + file = conf->file; > > + } > > if (!file) > > return -EBADF; > > > I would write that a bid different to reduce indent > und improve readability > > (note: totaly untested just reviewing) > > if (conf->fd < 0) { > file = conf->file; > if (!file) > return -EBADF; > } > else > { > file = fget(conf->fd); > if (!file) > return -EBADF; > Early return on fget() failure looks indeed simpler. > fput(file); > if (file->f_op != &ppp_device_fops) { > return -EBADF; > } > But this is wrong: we can't act on file after fput(). So we have to place fput() after the test. Thanks for your review. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ppp: add netlink support 2015-12-23 20:04 [PATCH 0/2] ppp: add netlink support Guillaume Nault 2015-12-23 20:04 ` [PATCH 1/2] ppp: define reusable device creation functions Guillaume Nault 2015-12-23 20:04 ` [PATCH 2/2] ppp: implement rtnetlink device handling Guillaume Nault @ 2015-12-23 20:09 ` Guillaume Nault 2016-01-05 18:10 ` Guillaume Nault 3 siblings, 0 replies; 9+ messages in thread From: Guillaume Nault @ 2015-12-23 20:09 UTC (permalink / raw) To: netdev; +Cc: linux-ppp, David Miller, Paul Mackerras On Wed, Dec 23, 2015 at 09:04:46PM +0100, Guillaume Nault wrote: > This series adds netlink support for creating PPP devices. > And I forgot to mention that it applies to net-next. Sorry. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ppp: add netlink support 2015-12-23 20:04 [PATCH 0/2] ppp: add netlink support Guillaume Nault ` (2 preceding siblings ...) 2015-12-23 20:09 ` [PATCH 0/2] ppp: add netlink support Guillaume Nault @ 2016-01-05 18:10 ` Guillaume Nault 2016-01-05 19:15 ` David Miller 3 siblings, 1 reply; 9+ messages in thread From: Guillaume Nault @ 2016-01-05 18:10 UTC (permalink / raw) To: netdev; +Cc: linux-ppp, David Miller, Paul Mackerras On Wed, Dec 23, 2015 at 09:04:46PM +0100, Guillaume Nault wrote: > This series adds netlink support for creating PPP devices. > Any feedback on this series? I can see that it has been marked "Deferred" in patchwork, so I'm unsure about what to do with this patch set now. Should I repost later (e.g. after the end of the merge window)? Are there any concern/issues with the series itself? Or should the idea of implementing netlink handlers for ppp devices be entirely dropped? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ppp: add netlink support 2016-01-05 18:10 ` Guillaume Nault @ 2016-01-05 19:15 ` David Miller 2016-01-05 19:20 ` Guillaume Nault 0 siblings, 1 reply; 9+ messages in thread From: David Miller @ 2016-01-05 19:15 UTC (permalink / raw) To: g.nault; +Cc: netdev, linux-ppp, paulus From: Guillaume Nault <g.nault@alphalink.fr> Date: Tue, 5 Jan 2016 19:10:20 +0100 > On Wed, Dec 23, 2015 at 09:04:46PM +0100, Guillaume Nault wrote: >> This series adds netlink support for creating PPP devices. >> > Any feedback on this series? I can see that it has been marked > "Deferred" in patchwork, so I'm unsure about what to do with this patch > set now. > Should I repost later (e.g. after the end of the merge window)? Are > there any concern/issues with the series itself? Or should the idea of > implementing netlink handlers for ppp devices be entirely dropped? Repost it again later, I want more people to see and review this series and the holidays is not a good time for that... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ppp: add netlink support 2016-01-05 19:15 ` David Miller @ 2016-01-05 19:20 ` Guillaume Nault 0 siblings, 0 replies; 9+ messages in thread From: Guillaume Nault @ 2016-01-05 19:20 UTC (permalink / raw) To: David Miller; +Cc: netdev, linux-ppp, paulus On Tue, Jan 05, 2016 at 02:15:34PM -0500, David Miller wrote: > From: Guillaume Nault <g.nault@alphalink.fr> > Date: Tue, 5 Jan 2016 19:10:20 +0100 > > > On Wed, Dec 23, 2015 at 09:04:46PM +0100, Guillaume Nault wrote: > >> This series adds netlink support for creating PPP devices. > >> > > Any feedback on this series? I can see that it has been marked > > "Deferred" in patchwork, so I'm unsure about what to do with this patch > > set now. > > Should I repost later (e.g. after the end of the merge window)? Are > > there any concern/issues with the series itself? Or should the idea of > > implementing netlink handlers for ppp devices be entirely dropped? > > Repost it again later, I want more people to see and review this > series and the holidays is not a good time for that... Sure. Thanks for the feedback. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-01-25 15:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-23 20:04 [PATCH 0/2] ppp: add netlink support Guillaume Nault 2015-12-23 20:04 ` [PATCH 1/2] ppp: define reusable device creation functions Guillaume Nault 2015-12-23 20:04 ` [PATCH 2/2] ppp: implement rtnetlink device handling Guillaume Nault 2016-01-25 11:09 ` walter harms 2016-01-25 15:28 ` Guillaume Nault 2015-12-23 20:09 ` [PATCH 0/2] ppp: add netlink support Guillaume Nault 2016-01-05 18:10 ` Guillaume Nault 2016-01-05 19:15 ` David Miller 2016-01-05 19:20 ` Guillaume Nault
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).