Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
Hi Simon, ip link set dev $iface master bat0 This doesn't work. eth0 is not added to batman. To further test, I've added it via batctl. I'm not sure..but I think this does not work because the set master command does not use ioctl, but rtnl only. Since in this kernel we don't have rtnl for these operations (this is why we have this hacky patch) I did not expect this command to work. ip link set dev $iface nomaster has no effect same as before. Even more interesting are combinations of those: * batctl if add $iface some OGMS here ip link set dev $iface nomaster ip link del dev bat0 * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here batctl if del $iface Some of these combinations are tested above, but I'd suggest we should fix the general case first. :) what would you like to fix? :) We have just implemented a feature and now we don't have any userspace tool which can exploit it! :-D Simon, Thanks a lot for testing! Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgpAaf5z_WLY1.pgp Description: PGP signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
On Wed, Feb 20, 2013 at 02:26:25PM +0100, Antonio Quartulli wrote: Hi Simon, ip link set dev $iface master bat0 This doesn't work. eth0 is not added to batman. To further test, I've added it via batctl. I'm not sure..but I think this does not work because the set master command does not use ioctl, but rtnl only. Since in this kernel we don't have rtnl for these operations (this is why we have this hacky patch) I did not expect this command to work. OK Even more interesting are combinations of those: * batctl if add $iface some OGMS here ip link set dev $iface nomaster ip link del dev bat0 * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here batctl if del $iface Some of these combinations are tested above, but I'd suggest we should fix the general case first. :) what would you like to fix? :) We have just implemented a feature and now we don't have any userspace tool which can exploit it! :-D Well, if we don't expect this command to work then it's fine. Personally I don't care as long as batctl works. :) If there are any other things to test, please let me know. Cheers, Simon signature.asc Description: Digital signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
On Wed, Feb 20, 2013 at 03:14:13PM +0100, Simon Wunderlich wrote: On Wed, Feb 20, 2013 at 02:26:25PM +0100, Antonio Quartulli wrote: Hi Simon, ip link set dev $iface master bat0 This doesn't work. eth0 is not added to batman. To further test, I've added it via batctl. I'm not sure..but I think this does not work because the set master command does not use ioctl, but rtnl only. Since in this kernel we don't have rtnl for these operations (this is why we have this hacky patch) I did not expect this command to work. OK Even more interesting are combinations of those: * batctl if add $iface some OGMS here ip link set dev $iface nomaster ip link del dev bat0 * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here batctl if del $iface Some of these combinations are tested above, but I'd suggest we should fix the general case first. :) what would you like to fix? :) We have just implemented a feature and now we don't have any userspace tool which can exploit it! :-D Well, if we don't expect this command to work then it's fine. I just double-checked. iplink handles the master subcommand by sending a rtnl request with the IFLA_MASTER flasg set, nothing else. Unluckily, this flag is not supported in 2.6.32. Personally I don't care as long as batctl works. :) yeah, that's the important point. And batman-adv is still working with this patch. If there are any other things to test, please let me know. Nothing on my side. Thanks again :) Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgpMSF2rJfKtj.pgp Description: PGP signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
On Wed, Feb 20, 2013 at 03:27:27PM +0100, Antonio Quartulli wrote: Nothing on my side. Thanks again :) I just realised that the ioctl commands this code is handling are for a bridge interface. Since batman-adv does not have its own set of ioctls, then the ndo_do_ioctl() callback should simply return EOPNOTSUPP. I'm going to send v2. This will not affect any tested behaviour. Cheers, -- Antonio Quartulli ..each of us alone is worth nothing.. Ernesto Che Guevara pgppnZvPh99Nd.pgp Description: PGP signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
On Tue, Feb 19, 2013 at 09:08:44AM +0800, Marek Lindner wrote: Hi Simon, I've tested this (just test, no code review) in my 2.6.32 qemu (Debian squeeze, iproute from wheezy). It generally loads and batctl if add/del seems to work, but there is something weird going on with bridge interaction. After calling the following 3 commands: brctl addbr br0 batctl if add eth0 ip link set dev eth0 master br0 I would expect that eth0 moved from bat0 to br0 (or some error is reported). I feel some confusion here. It could very well be that 2.6.32 did not yet support this kind of interface transfer. You could repeat your test without any of the later patches (just go back to the last stable release) to see what happens. It is very likely that our patches have nothing to do with that. you are right - I've checked 2013.0.0, same behaviour - and now that you mention it, this is all completely independent from batman-adv/netlink. Sorry for reporting stuff like that ... :) Also it seems that batctl (2010.0.0, from debian stable) reports all devices, even if they are not added to batman-adv. It reports them as not in use. This confused me, haven't seen that on other systems (unused devices are just skipped ...). After upding to the wheezy version of batctl (2012.1.0), this effect was gone. Anyway, I've tested a little more and still I see some issues: These are the tests we should do in order to confirm the compat patch in question: * batctl if add $iface some OGMS here batctl if del $iface This works fine, I see OGMs, and the interface is removed cleanly. * ip link add dev bat0 type batadv This works fine. ip link set dev $iface master bat0 This doesn't work. eth0 is not added to batman. To further test, I've added it via batctl. some OGMS here yes (bat0 created with ip, eth0 added using batctl) ip link set dev $iface nomaster has no effect ip link del dev bat0 works. Even more interesting are combinations of those: * batctl if add $iface some OGMS here ip link set dev $iface nomaster ip link del dev bat0 * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here batctl if del $iface Some of these combinations are tested above, but I'd suggest we should fix the general case first. :) Cheers, Simon signature.asc Description: Digital signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
Hey Antonio,
I've tested this (just test, no code review) in my 2.6.32 qemu (Debian squeeze,
iproute from wheezy).
It generally loads and batctl if add/del seems to work, but there is something
weird going on with
bridge interaction. After calling the following 3 commands:
brctl addbr br0
batctl if add eth0
ip link set dev eth0 master br0
I would expect that eth0 moved from bat0 to br0 (or some error is reported).
But what I see is:
root@debian:~# batctl if
eth0: active
br0: not in use
root@debian:~# brctl show
bridge name bridge id STP enabled interfaces
br0 8000. no
Even more weird, after doing batctl if del eth0 or batctl if del br0, I don't
get any output, but
both devices remain in bat0 (according to batctl if). But I do get some output
that it's trying
to remove eth0:
[ 102.788930] batman_adv: bat0: Interface deactivated: eth0
[ 102.788935] batman_adv: bat0: Removing interface: eth0
Note that I don't have this behaviour on my host machine (3.2.0, Debian
unstable kernel).
So I can't ack this now, although I don't know if the problem is in this patch,
in the
current batman-adv/netlink implementation or somewhere in 2.6.32 kernel release.
Cheers,
Simon
On Sun, Feb 17, 2013 at 03:43:04PM +0100, Antonio Quartulli wrote:
This patches enable the new rtnl ops to compile on kernels starting from
2.6.32
Signed-off-by: Antonio Quartulli or...@autistici.org
---
Could not be uglier.
*Compile tested only* because linux-2.6.32 does not compile on my laptop
compat.h | 40
1 file changed, 40 insertions(+)
diff --git a/compat.h b/compat.h
index 885e551..de22e2b 100644
--- a/compat.h
+++ b/compat.h
@@ -71,6 +71,9 @@ static inline void batadv_this_cpu_add(uint64_t *count_ptr,
size_t count)
put_cpu();
}
+#define batadv_softif_destroy_netlink(dev, head)
batadv_softif_destroy_netlink(dev)
+#define unregister_netdevice_queue(dev, head) unregister_netdevice(dev)
+
#endif /* KERNEL_VERSION(2, 6, 33) */
@@ -132,6 +135,43 @@ static inline int batadv_param_set_copystring(const char
*val,
#define kstrtoul strict_strtoul
#define kstrtol strict_strtol
+#define batadv_softif_slave_add(x, y) \
+batadv_softif_slave_add(struct net_device *dev, struct ifreq *rq, int cmd)\
+{\
+ return 0;\
+}\
+static int __batadv_softif_slave_add(x, y)
+
+#define batadv_softif_slave_del(x, y) \
+__batadv_softif_slave_del(struct net_device *dev, struct net_device
*slave_dev);\
+static int batadv_softif_slave_del(struct net_device *dev, struct ifreq *rq,
int cmd)\
+{\
+ struct batadv_priv *bat_priv = netdev_priv(dev);\
+ struct net_device *slave_dev;\
+ int ret;\
+ if (!capable(CAP_NET_ADMIN))\
+ return -EPERM;\
+ slave_dev = dev_get_by_index(init_net, rq-ifr_ifindex);\
+ if (!dev)\
+ return -EINVAL;\
+ switch(cmd) {\
+ case SIOCBRADDIF:\
+ ret = __batadv_softif_slave_add(bat_priv-soft_iface,
slave_dev);\
+ case SIOCBRDELIF:\
+ ret = __batadv_softif_slave_del(bat_priv-soft_iface,
slave_dev);\
+ default:\
+ pr_debug(batman-adv does not support ioctl 0x%x\n, cmd);\
+ return -EOPNOTSUPP;\
+ }\
+\
+ dev_put(slave_dev);\
+ return ret;\
+}\
+static int __batadv_softif_slave_del(x, y)
+
+#define ndo_add_slave ndo_do_ioctl
+#define ndo_del_slave ndo_do_ioctl
+
#endif /* KERNEL_VERSION(2, 6, 39) */
--
1.8.1.2
signature.asc
Description: Digital signature
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
Hi Simon, I've tested this (just test, no code review) in my 2.6.32 qemu (Debian squeeze, iproute from wheezy). It generally loads and batctl if add/del seems to work, but there is something weird going on with bridge interaction. After calling the following 3 commands: brctl addbr br0 batctl if add eth0 ip link set dev eth0 master br0 I would expect that eth0 moved from bat0 to br0 (or some error is reported). I feel some confusion here. It could very well be that 2.6.32 did not yet support this kind of interface transfer. You could repeat your test without any of the later patches (just go back to the last stable release) to see what happens. It is very likely that our patches have nothing to do with that. These are the tests we should do in order to confirm the compat patch in question: * batctl if add $iface some OGMS here batctl if del $iface * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here ip link set dev $iface nomaster ip link del dev bat0 Even more interesting are combinations of those: * batctl if add $iface some OGMS here ip link set dev $iface nomaster ip link del dev bat0 * ip link add dev bat0 type batadv ip link set dev $iface master bat0 some OGMS here batctl if del $iface Thanks for testing! Cheers, Marek
[B.A.T.M.A.N.] [PATCH] batman-adv: add compat support for the new netlink ops
This patches enable the new rtnl ops to compile on kernels starting from 2.6.32
Signed-off-by: Antonio Quartulli or...@autistici.org
---
Could not be uglier.
*Compile tested only* because linux-2.6.32 does not compile on my laptop
compat.h | 40
1 file changed, 40 insertions(+)
diff --git a/compat.h b/compat.h
index 885e551..de22e2b 100644
--- a/compat.h
+++ b/compat.h
@@ -71,6 +71,9 @@ static inline void batadv_this_cpu_add(uint64_t *count_ptr,
size_t count)
put_cpu();
}
+#define batadv_softif_destroy_netlink(dev, head)
batadv_softif_destroy_netlink(dev)
+#define unregister_netdevice_queue(dev, head) unregister_netdevice(dev)
+
#endif /* KERNEL_VERSION(2, 6, 33) */
@@ -132,6 +135,43 @@ static inline int batadv_param_set_copystring(const char
*val,
#define kstrtoul strict_strtoul
#define kstrtol strict_strtol
+#define batadv_softif_slave_add(x, y) \
+batadv_softif_slave_add(struct net_device *dev, struct ifreq *rq, int cmd)\
+{\
+ return 0;\
+}\
+static int __batadv_softif_slave_add(x, y)
+
+#define batadv_softif_slave_del(x, y) \
+__batadv_softif_slave_del(struct net_device *dev, struct net_device
*slave_dev);\
+static int batadv_softif_slave_del(struct net_device *dev, struct ifreq *rq,
int cmd)\
+{\
+ struct batadv_priv *bat_priv = netdev_priv(dev);\
+ struct net_device *slave_dev;\
+ int ret;\
+ if (!capable(CAP_NET_ADMIN))\
+ return -EPERM;\
+ slave_dev = dev_get_by_index(init_net, rq-ifr_ifindex);\
+ if (!dev)\
+ return -EINVAL;\
+ switch(cmd) {\
+ case SIOCBRADDIF:\
+ ret = __batadv_softif_slave_add(bat_priv-soft_iface,
slave_dev);\
+ case SIOCBRDELIF:\
+ ret = __batadv_softif_slave_del(bat_priv-soft_iface,
slave_dev);\
+ default:\
+ pr_debug(batman-adv does not support ioctl 0x%x\n, cmd);\
+ return -EOPNOTSUPP;\
+ }\
+\
+ dev_put(slave_dev);\
+ return ret;\
+}\
+static int __batadv_softif_slave_del(x, y)
+
+#define ndo_add_slave ndo_do_ioctl
+#define ndo_del_slave ndo_do_ioctl
+
#endif /* KERNEL_VERSION(2, 6, 39) */
--
1.8.1.2
