* [PATCH 1/4] ipv4: streamline ipmr_new_tunnel
2020-05-14 14:50 use symbol_get to create the magic ipv4/ipv6 tunnels Christoph Hellwig
@ 2020-05-14 14:50 ` Christoph Hellwig
2020-05-14 14:50 ` [PATCH 2/4] ipv4: consolidate the VIFF_TUNNEL handling in ipmr_new_tunnel Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-14 14:50 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel
Reduce a few level of indentation to simplify the function.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/ipv4/ipmr.c | 73 ++++++++++++++++++++++++-------------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 9cf83cc85e4ad..84541c601cfab 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -469,50 +469,49 @@ static bool ipmr_init_vif_indev(const struct net_device *dev)
static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
{
- struct net_device *dev;
+ mm_segment_t oldfs = get_fs();
+ struct net_device *tunnel_dev, *new_dev;
+ struct ip_tunnel_parm p = { };
+ struct ifreq ifr;
+ int err;
- dev = __dev_get_by_name(net, "tunl0");
+ tunnel_dev = __dev_get_by_name(net, "tunl0");
+ if (!tunnel_dev)
+ goto out;
- if (dev) {
- const struct net_device_ops *ops = dev->netdev_ops;
- int err;
- struct ifreq ifr;
- struct ip_tunnel_parm p;
+ p.iph.daddr = v->vifc_rmt_addr.s_addr;
+ p.iph.saddr = v->vifc_lcl_addr.s_addr;
+ p.iph.version = 4;
+ p.iph.ihl = 5;
+ p.iph.protocol = IPPROTO_IPIP;
+ sprintf(p.name, "dvmrp%d", v->vifc_vifi);
+ ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
- memset(&p, 0, sizeof(p));
- p.iph.daddr = v->vifc_rmt_addr.s_addr;
- p.iph.saddr = v->vifc_lcl_addr.s_addr;
- p.iph.version = 4;
- p.iph.ihl = 5;
- p.iph.protocol = IPPROTO_IPIP;
- sprintf(p.name, "dvmrp%d", v->vifc_vifi);
- ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
+ if (!tunnel_dev->netdev_ops->ndo_do_ioctl)
+ goto out;
- if (ops->ndo_do_ioctl) {
- mm_segment_t oldfs = get_fs();
+ set_fs(KERNEL_DS);
+ err = tunnel_dev->netdev_ops->ndo_do_ioctl(tunnel_dev, &ifr,
+ SIOCADDTUNNEL);
+ set_fs(oldfs);
+ if (err)
+ goto out;
- set_fs(KERNEL_DS);
- err = ops->ndo_do_ioctl(dev, &ifr, SIOCADDTUNNEL);
- set_fs(oldfs);
- } else {
- err = -EOPNOTSUPP;
- }
- dev = NULL;
+ new_dev = __dev_get_by_name(net, p.name);
+ if (!new_dev)
+ goto out;
- if (err == 0 &&
- (dev = __dev_get_by_name(net, p.name)) != NULL) {
- dev->flags |= IFF_MULTICAST;
- if (!ipmr_init_vif_indev(dev))
- goto failure;
- if (dev_open(dev, NULL))
- goto failure;
- dev_hold(dev);
- }
- }
- return dev;
+ new_dev->flags |= IFF_MULTICAST;
+ if (!ipmr_init_vif_indev(new_dev))
+ goto out_unregister;
+ if (dev_open(new_dev, NULL))
+ goto out_unregister;
+ dev_hold(new_dev);
+ return new_dev;
-failure:
- unregister_netdevice(dev);
+out_unregister:
+ unregister_netdevice(new_dev);
+out:
return NULL;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] ipv4: consolidate the VIFF_TUNNEL handling in ipmr_new_tunnel
2020-05-14 14:50 use symbol_get to create the magic ipv4/ipv6 tunnels Christoph Hellwig
2020-05-14 14:50 ` [PATCH 1/4] ipv4: streamline ipmr_new_tunnel Christoph Hellwig
@ 2020-05-14 14:50 ` Christoph Hellwig
2020-05-14 14:51 ` [PATCH 3/4] ipv4: use symbol_get to access ipip symbols Christoph Hellwig
2020-05-14 14:51 ` [PATCH 4/4] ipv6: symbol_get to access a sit symbol Christoph Hellwig
3 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-14 14:50 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel
Also move the dev_set_allmulti call and the error handling into the
ioctl helper. This allows reusing already looked up tunnel_dev pointer
and the set up argument structure for the deletion in the error handler.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/ipv4/ipmr.c | 53 ++++++++++++-------------------------------------
1 file changed, 13 insertions(+), 40 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 84541c601cfab..6bf2a88abe86e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -419,37 +419,6 @@ static void ipmr_free_table(struct mr_table *mrt)
/* Service routines creating virtual interfaces: DVMRP tunnels and PIMREG */
-static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
-{
- struct net *net = dev_net(dev);
-
- dev_close(dev);
-
- dev = __dev_get_by_name(net, "tunl0");
- if (dev) {
- const struct net_device_ops *ops = dev->netdev_ops;
- struct ifreq ifr;
- struct ip_tunnel_parm p;
-
- memset(&p, 0, sizeof(p));
- p.iph.daddr = v->vifc_rmt_addr.s_addr;
- p.iph.saddr = v->vifc_lcl_addr.s_addr;
- p.iph.version = 4;
- p.iph.ihl = 5;
- p.iph.protocol = IPPROTO_IPIP;
- sprintf(p.name, "dvmrp%d", v->vifc_vifi);
- ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
-
- if (ops->ndo_do_ioctl) {
- mm_segment_t oldfs = get_fs();
-
- set_fs(KERNEL_DS);
- ops->ndo_do_ioctl(dev, &ifr, SIOCDELTUNNEL);
- set_fs(oldfs);
- }
- }
-}
-
/* Initialize ipmr pimreg/tunnel in_device */
static bool ipmr_init_vif_indev(const struct net_device *dev)
{
@@ -507,12 +476,22 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
if (dev_open(new_dev, NULL))
goto out_unregister;
dev_hold(new_dev);
+ err = dev_set_allmulti(new_dev, 1);
+ if (err) {
+ dev_close(new_dev);
+ set_fs(KERNEL_DS);
+ tunnel_dev->netdev_ops->ndo_do_ioctl(tunnel_dev, &ifr,
+ SIOCDELTUNNEL);
+ set_fs(oldfs);
+ dev_put(new_dev);
+ new_dev = ERR_PTR(err);
+ }
return new_dev;
out_unregister:
unregister_netdevice(new_dev);
out:
- return NULL;
+ return ERR_PTR(-ENOBUFS);
}
#if defined(CONFIG_IP_PIMSM_V1) || defined(CONFIG_IP_PIMSM_V2)
@@ -864,14 +843,8 @@ static int vif_add(struct net *net, struct mr_table *mrt,
break;
case VIFF_TUNNEL:
dev = ipmr_new_tunnel(net, vifc);
- if (!dev)
- return -ENOBUFS;
- err = dev_set_allmulti(dev, 1);
- if (err) {
- ipmr_del_tunnel(dev, vifc);
- dev_put(dev);
- return err;
- }
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
break;
case VIFF_USE_IFINDEX:
case 0:
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] ipv4: use symbol_get to access ipip symbols
2020-05-14 14:50 use symbol_get to create the magic ipv4/ipv6 tunnels Christoph Hellwig
2020-05-14 14:50 ` [PATCH 1/4] ipv4: streamline ipmr_new_tunnel Christoph Hellwig
2020-05-14 14:50 ` [PATCH 2/4] ipv4: consolidate the VIFF_TUNNEL handling in ipmr_new_tunnel Christoph Hellwig
@ 2020-05-14 14:51 ` Christoph Hellwig
2020-05-14 14:51 ` [PATCH 4/4] ipv6: symbol_get to access a sit symbol Christoph Hellwig
3 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-14 14:51 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel
Instead of going through the ioctl handler from kernel space, use
symbol_get to access the ip_tunnel_ioctl directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
net/ipv4/ipmr.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 6bf2a88abe86e..3780ab694c574 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -438,10 +438,9 @@ static bool ipmr_init_vif_indev(const struct net_device *dev)
static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
{
- mm_segment_t oldfs = get_fs();
+ int (*tunnel_ctl)(struct net_device *, struct ip_tunnel_parm *, int);
struct net_device *tunnel_dev, *new_dev;
struct ip_tunnel_parm p = { };
- struct ifreq ifr;
int err;
tunnel_dev = __dev_get_by_name(net, "tunl0");
@@ -454,21 +453,17 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
p.iph.ihl = 5;
p.iph.protocol = IPPROTO_IPIP;
sprintf(p.name, "dvmrp%d", v->vifc_vifi);
- ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
- if (!tunnel_dev->netdev_ops->ndo_do_ioctl)
- goto out;
+ tunnel_ctl = symbol_get(ip_tunnel_ioctl);
+ if (!tunnel_ctl)
+ return ERR_PTR(-ENOBUFS);
- set_fs(KERNEL_DS);
- err = tunnel_dev->netdev_ops->ndo_do_ioctl(tunnel_dev, &ifr,
- SIOCADDTUNNEL);
- set_fs(oldfs);
- if (err)
- goto out;
+ if (tunnel_ctl(tunnel_dev, &p, SIOCADDTUNNEL))
+ goto out_symbol_put;
new_dev = __dev_get_by_name(net, p.name);
if (!new_dev)
- goto out;
+ goto out_symbol_put;
new_dev->flags |= IFF_MULTICAST;
if (!ipmr_init_vif_indev(new_dev))
@@ -479,17 +474,18 @@ static struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
err = dev_set_allmulti(new_dev, 1);
if (err) {
dev_close(new_dev);
- set_fs(KERNEL_DS);
- tunnel_dev->netdev_ops->ndo_do_ioctl(tunnel_dev, &ifr,
- SIOCDELTUNNEL);
- set_fs(oldfs);
+ tunnel_ctl(tunnel_dev, &p, SIOCDELTUNNEL);
dev_put(new_dev);
new_dev = ERR_PTR(err);
}
+
+ symbol_put(ip_tunnel_ioctl);
return new_dev;
out_unregister:
unregister_netdevice(new_dev);
+out_symbol_put:
+ symbol_put(ipmr_new_tunnel);
out:
return ERR_PTR(-ENOBUFS);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] ipv6: symbol_get to access a sit symbol
2020-05-14 14:50 use symbol_get to create the magic ipv4/ipv6 tunnels Christoph Hellwig
` (2 preceding siblings ...)
2020-05-14 14:51 ` [PATCH 3/4] ipv4: use symbol_get to access ipip symbols Christoph Hellwig
@ 2020-05-14 14:51 ` Christoph Hellwig
2020-05-15 0:53 ` David Miller
3 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-14 14:51 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel
Instead of going through the ioctl handler from kernel space, use
symbol_get to the newly factored out ipip6_set_dstaddr helper, bypassing
addrconf.c entirely.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/net/addrconf.h | 1 -
include/net/ipv6.h | 2 ++
net/ipv6/addrconf.c | 66 ------------------------------------------
net/ipv6/af_inet6.c | 20 ++++++++++++-
net/ipv6/sit.c | 41 ++++++++++++++++++++++++++
5 files changed, 62 insertions(+), 68 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index fdb07105384ca..569eb03ae2440 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -76,7 +76,6 @@ void addrconf_cleanup(void);
int addrconf_add_ifaddr(struct net *net, void __user *arg);
int addrconf_del_ifaddr(struct net *net, void __user *arg);
-int addrconf_set_dstaddr(struct net *net, void __user *arg);
int ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
const struct net_device *dev, int strict);
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 955badd1e8ffc..1b983f32c87ce 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1080,6 +1080,8 @@ struct in6_addr *fl6_update_dst(struct flowi6 *fl6,
const struct ipv6_txoptions *opt,
struct in6_addr *orig);
+int ipip6_set_dstaddr(struct net *net, void __user *arg);
+
/*
* socket options (ipv6_sockglue.c)
*/
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index fd885f06c4ed6..02186f00f91c5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2783,72 +2783,6 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
in6_dev_put(in6_dev);
}
-/*
- * Set destination address.
- * Special case for SIT interfaces where we create a new "virtual"
- * device.
- */
-int addrconf_set_dstaddr(struct net *net, void __user *arg)
-{
- struct in6_ifreq ireq;
- struct net_device *dev;
- int err = -EINVAL;
-
- rtnl_lock();
-
- err = -EFAULT;
- if (copy_from_user(&ireq, arg, sizeof(struct in6_ifreq)))
- goto err_exit;
-
- dev = __dev_get_by_index(net, ireq.ifr6_ifindex);
-
- err = -ENODEV;
- if (!dev)
- goto err_exit;
-
-#if IS_ENABLED(CONFIG_IPV6_SIT)
- if (dev->type == ARPHRD_SIT) {
- const struct net_device_ops *ops = dev->netdev_ops;
- struct ifreq ifr;
- struct ip_tunnel_parm p;
-
- err = -EADDRNOTAVAIL;
- if (!(ipv6_addr_type(&ireq.ifr6_addr) & IPV6_ADDR_COMPATv4))
- goto err_exit;
-
- memset(&p, 0, sizeof(p));
- p.iph.daddr = ireq.ifr6_addr.s6_addr32[3];
- p.iph.saddr = 0;
- p.iph.version = 4;
- p.iph.ihl = 5;
- p.iph.protocol = IPPROTO_IPV6;
- p.iph.ttl = 64;
- ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
-
- if (ops->ndo_do_ioctl) {
- mm_segment_t oldfs = get_fs();
-
- set_fs(KERNEL_DS);
- err = ops->ndo_do_ioctl(dev, &ifr, SIOCADDTUNNEL);
- set_fs(oldfs);
- } else
- err = -EOPNOTSUPP;
-
- if (err == 0) {
- err = -ENOBUFS;
- dev = __dev_get_by_name(net, p.name);
- if (!dev)
- goto err_exit;
- err = dev_open(dev, NULL);
- }
- }
-#endif
-
-err_exit:
- rtnl_unlock();
- return err;
-}
-
static int ipv6_mc_config(struct sock *sk, bool join,
const struct in6_addr *addr, int ifindex)
{
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 345baa0a754f4..3ec9734c7bb11 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -538,6 +538,19 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
}
EXPORT_SYMBOL(inet6_getname);
+static int inet6_ioctl_set_dstaddr(struct net *net, void __user *arg)
+{
+ int (*set_dstaddr)(struct net *, void __user *);
+ int err;
+
+ set_dstaddr = symbol_get(ipip6_set_dstaddr);
+ if (!set_dstaddr)
+ return -EOPNOTSUPP;
+ err = set_dstaddr(net, arg);
+ symbol_put(ipip6_set_dstaddr);
+ return err;
+}
+
int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
{
struct sock *sk = sock->sk;
@@ -554,7 +567,12 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
case SIOCDIFADDR:
return addrconf_del_ifaddr(net, (void __user *) arg);
case SIOCSIFDSTADDR:
- return addrconf_set_dstaddr(net, (void __user *) arg);
+ /* Special case for SIT interfaces where we create a new
+ * "virtual" device.
+ */
+ if (!IS_ENABLED(CONFIG_IPV6_SIT))
+ return -ENODEV;
+ return inet6_ioctl_set_dstaddr(net, (void __user *) arg);
default:
if (!sk->sk_prot->ioctl)
return -ENOIOCTLCMD;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 98954830c40ba..cb2cfa297f72e 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -274,6 +274,47 @@ static struct ip_tunnel *ipip6_tunnel_locate(struct net *net,
return NULL;
}
+int ipip6_set_dstaddr(struct net *net, void __user *arg)
+{
+ struct ip_tunnel_parm p = { };
+ struct in6_ifreq ireq;
+ struct net_device *tunnel_dev, *new_dev;
+ int err;
+
+ rtnl_lock();
+ err = -EFAULT;
+ if (copy_from_user(&ireq, arg, sizeof(struct in6_ifreq)))
+ goto out_unlock;
+
+ err = -ENODEV;
+ tunnel_dev = __dev_get_by_index(net, ireq.ifr6_ifindex);
+ if (!tunnel_dev || tunnel_dev->type != ARPHRD_SIT)
+ goto out_unlock;
+
+ err = -EADDRNOTAVAIL;
+ if (!(ipv6_addr_type(&ireq.ifr6_addr) & IPV6_ADDR_COMPATv4))
+ goto out_unlock;
+
+ p.iph.daddr = ireq.ifr6_addr.s6_addr32[3];
+ p.iph.version = 4;
+ p.iph.ihl = 5;
+ p.iph.protocol = IPPROTO_IPV6;
+ p.iph.ttl = 64;
+ p.iph.frag_off |= htons(IP_DF);
+
+ err = -ENOBUFS;
+ if (!ipip6_tunnel_locate(dev_net(tunnel_dev), &p, true))
+ goto out_unlock;
+ new_dev = __dev_get_by_name(net, p.name);
+ if (!new_dev)
+ goto out_unlock;
+ err = dev_open(new_dev, NULL);
+out_unlock:
+ rtnl_unlock();
+ return err;
+}
+EXPORT_SYMBOL_GPL(ipip6_set_dstaddr);
+
#define for_each_prl_rcu(start) \
for (prl = rcu_dereference(start); \
prl; \
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ipv6: symbol_get to access a sit symbol
2020-05-14 14:51 ` [PATCH 4/4] ipv6: symbol_get to access a sit symbol Christoph Hellwig
@ 2020-05-15 0:53 ` David Miller
2020-05-15 6:33 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2020-05-15 0:53 UTC (permalink / raw)
To: hch; +Cc: kuba, kuznet, yoshfuji, netdev, linux-kernel
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 14 May 2020 16:51:01 +0200
> Instead of going through the ioctl handler from kernel space, use
> symbol_get to the newly factored out ipip6_set_dstaddr helper, bypassing
> addrconf.c entirely.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
...
> - memset(&p, 0, sizeof(p));
> - p.iph.daddr = ireq.ifr6_addr.s6_addr32[3];
> - p.iph.saddr = 0;
> - p.iph.version = 4;
> - p.iph.ihl = 5;
> - p.iph.protocol = IPPROTO_IPV6;
> - p.iph.ttl = 64;
> - ifr.ifr_ifru.ifru_data = (__force void __user *)&p;
> -
> - if (ops->ndo_do_ioctl) {
> - mm_segment_t oldfs = get_fs();
> -
> - set_fs(KERNEL_DS);
> - err = ops->ndo_do_ioctl(dev, &ifr, SIOCADDTUNNEL);
> - set_fs(oldfs);
> - } else
> - err = -EOPNOTSUPP;
...
> + p.iph.daddr = ireq.ifr6_addr.s6_addr32[3];
> + p.iph.version = 4;
> + p.iph.ihl = 5;
> + p.iph.protocol = IPPROTO_IPV6;
> + p.iph.ttl = 64;
> + p.iph.frag_off |= htons(IP_DF);
> +
> + err = -ENOBUFS;
> + if (!ipip6_tunnel_locate(dev_net(tunnel_dev), &p, true))
> + goto out_unlock;
You're not undoing one, but two levels of abstraction here.
Is this "ipip6_tunnel_locate()" call part of the SIT ioctl implementation?
Where did it come from? Why are ->ndo_do_ioctl() implementations no longer
allowed from here?
Honestly, this feels like a bit much.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ipv6: symbol_get to access a sit symbol
2020-05-15 0:53 ` David Miller
@ 2020-05-15 6:33 ` Christoph Hellwig
2020-05-16 20:55 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-15 6:33 UTC (permalink / raw)
To: David Miller; +Cc: hch, kuba, kuznet, yoshfuji, netdev, linux-kernel
On Thu, May 14, 2020 at 05:53:55PM -0700, David Miller wrote:
> You're not undoing one, but two levels of abstraction here.
>
> Is this "ipip6_tunnel_locate()" call part of the SIT ioctl implementation?
Yes. Take a look at the convoluted case handling the
SIOCADDTUNNEL and SIOCCHGTUNNEL commands in ipip6_tunnel_ioctl.
> Where did it come from? Why are ->ndo_do_ioctl() implementations no longer
> allowed from here?
The problem is that we feed kernel pointers to it, which requires
set_fs address space overrides that I plan to kill off entirely.
> Honestly, this feels like a bit much.
My initial plan was to add a ->tunnel_ctl method to the net_device_ops,
and lift the copy_{to,from}_user for SIOCADDTUNNEL, SIOCCHGTUNNEL,
SIOCDELTUNNEL and maybe SIOCGETTUNNEL to net/socket.c. But that turned
out to have two problems:
- first these ioctls names use SIOCDEVPRIVATE range, that can also
be implemented by other drivers
- the ip_tunnel_parm struture is only used by the ipv4 tunneling
drivers (including sit), the "real" ipv6 tunnels use a
ip6_tnl_parm or ip6_tnl_parm structure instead
But if you don't like the symbol_get approach, I could do the
tunnel_ctl operation, just for the іpv4-ish tunnels, and only for
the kernel callers.
---end quoted text---
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ipv6: symbol_get to access a sit symbol
2020-05-15 6:33 ` Christoph Hellwig
@ 2020-05-16 20:55 ` David Miller
2020-05-18 6:30 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2020-05-16 20:55 UTC (permalink / raw)
To: hch; +Cc: kuba, kuznet, yoshfuji, netdev, linux-kernel
From: Christoph Hellwig <hch@lst.de>
Date: Fri, 15 May 2020 08:33:24 +0200
> My initial plan was to add a ->tunnel_ctl method to the net_device_ops,
> and lift the copy_{to,from}_user for SIOCADDTUNNEL, SIOCCHGTUNNEL,
> SIOCDELTUNNEL and maybe SIOCGETTUNNEL to net/socket.c. But that turned
> out to have two problems:
>
> - first these ioctls names use SIOCDEVPRIVATE range, that can also
> be implemented by other drivers
> - the ip_tunnel_parm struture is only used by the ipv4 tunneling
> drivers (including sit), the "real" ipv6 tunnels use a
> ip6_tnl_parm or ip6_tnl_parm structure instead
Yes, this is the core of the problem, the user provided data's type
is unknown until we are very deep in the call chains.
I wonder if there is some clever way to propagate this size value
"up"?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] ipv6: symbol_get to access a sit symbol
2020-05-16 20:55 ` David Miller
@ 2020-05-18 6:30 ` Christoph Hellwig
2020-05-19 0:36 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2020-05-18 6:30 UTC (permalink / raw)
To: David Miller; +Cc: hch, kuba, kuznet, yoshfuji, netdev, linux-kernel
On Sat, May 16, 2020 at 01:55:48PM -0700, David Miller wrote:
> From: Christoph Hellwig <hch@lst.de>
> Date: Fri, 15 May 2020 08:33:24 +0200
>
> > My initial plan was to add a ->tunnel_ctl method to the net_device_ops,
> > and lift the copy_{to,from}_user for SIOCADDTUNNEL, SIOCCHGTUNNEL,
> > SIOCDELTUNNEL and maybe SIOCGETTUNNEL to net/socket.c. But that turned
> > out to have two problems:
> >
> > - first these ioctls names use SIOCDEVPRIVATE range, that can also
> > be implemented by other drivers
> > - the ip_tunnel_parm struture is only used by the ipv4 tunneling
> > drivers (including sit), the "real" ipv6 tunnels use a
> > ip6_tnl_parm or ip6_tnl_parm structure instead
>
> Yes, this is the core of the problem, the user provided data's type
> is unknown until we are very deep in the call chains.
>
> I wonder if there is some clever way to propagate this size value
> "up"?
As far as I can tell the only information vectors is the net_device
structure or its op vector. But even then we have the problem that
other devices use the SIOCDEVPRIVATE range for something else.
I'll look into implenenting the tunnel_ctl method just for kernel
callers (plus maybe a generic helper for the ioctl), and we'll see if
you like that better.
^ permalink raw reply [flat|nested] 10+ messages in thread