[Openvpn-devel,net,v3,7/9] ovpn: disable IPv4 redirects on MP interfaces
Commit Message
From: Antonio Quartulli <antonio@openvpn.net>
ovpn_mp_alloc() tried to disable SEND_REDIRECTS on a multipeer
interface, but it runs from ovpn_net_init() (->ndo_init), which
register_netdevice() invokes before the NETDEV_REGISTER notifier
chain. The IPv4 in_device is only created when that notifier reaches
inetdev_event() -> inetdev_init(), so __in_dev_get_rtnl() always
returned NULL at ndo_init time and the whole redirect-disabling block
(both the per-device and the per-netns IPV4_DEVCONF_ALL write) was
dead. MP interfaces therefore kept emitting ICMP redirects.
Disabling redirects only once is not enough either: the IPv4
in_device is destroyed and recreated when the interface is moved to a
different network namespace (NETDEV_UNREGISTER/NETDEV_REGISTER), and
the newly created in_device inherits the destination namespace
defaults, silently re-enabling SEND_REDIRECTS.
Disable redirects from ovpn_net_open() (->ndo_open) instead: it runs
every time the interface is brought up, including after the in_device
has been recreated, so the setting is always re-applied. This mirrors
what wireguard does in wg_open(). RTNL is held on the ndo_open() path,
so __in_dev_get_rtnl() is safe.
Fixes: 05003b408c20 ("ovpn: implement multi-peer support")
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
drivers/net/ovpn/main.c | 50 ++++++++++++++++++++++++++++-------------
1 file changed, 35 insertions(+), 15 deletions(-)
@@ -35,25 +35,11 @@ static void ovpn_priv_free(struct net_device *net)
static int ovpn_mp_alloc(struct ovpn_priv *ovpn)
{
- struct in_device *dev_v4;
int i;
if (ovpn->mode != OVPN_MODE_MP)
return 0;
- dev_v4 = __in_dev_get_rtnl(ovpn->dev);
- if (dev_v4) {
- /* disable redirects as Linux gets confused by ovpn
- * handling same-LAN routing.
- * This happens because a multipeer interface is used as
- * relay point between hosts in the same subnet, while
- * in a classic LAN this would not be needed because the
- * two hosts would be able to talk directly.
- */
- IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
- IPV4_DEVCONF_ALL(dev_net(ovpn->dev), SEND_REDIRECTS) = false;
- }
-
/* the peer container is fairly large, therefore we allocate it only in
* MP mode
*/
@@ -97,9 +83,38 @@ static void ovpn_net_uninit(struct net_device *dev)
gro_cells_destroy(&ovpn->gro_cells);
}
+static int ovpn_net_open(struct net_device *dev)
+{
+ struct ovpn_priv *ovpn = netdev_priv(dev);
+ struct in_device *dev_v4;
+
+ /* the IPv4 in_device (and thus its config) is recreated whenever the
+ * interface is moved to a new netns, so redirects must be disabled on
+ * every bring-up rather than once at creation time, otherwise the
+ * setting is silently lost after such a move
+ */
+ if (ovpn->mode == OVPN_MODE_MP) {
+ dev_v4 = __in_dev_get_rtnl(dev);
+ if (dev_v4) {
+ /* disable redirects as Linux gets confused by ovpn
+ * handling same-LAN routing.
+ * This happens because a multipeer interface is used as
+ * relay point between hosts in the same subnet, while
+ * in a classic LAN this would not be needed because the
+ * two hosts would be able to talk directly.
+ */
+ IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
+ IPV4_DEVCONF_ALL(dev_net(dev), SEND_REDIRECTS) = false;
+ }
+ }
+
+ return 0;
+}
+
static const struct net_device_ops ovpn_netdev_ops = {
.ndo_init = ovpn_net_init,
.ndo_uninit = ovpn_net_uninit,
+ .ndo_open = ovpn_net_open,
.ndo_start_xmit = ovpn_net_xmit,
};
@@ -183,6 +198,7 @@ static int ovpn_newlink(struct net_device *dev,
struct ovpn_priv *ovpn = netdev_priv(dev);
struct nlattr **data = params->data;
enum ovpn_mode mode = OVPN_MODE_P2P;
+ int ret;
if (data && data[IFLA_OVPN_MODE]) {
mode = nla_get_u8(data[IFLA_OVPN_MODE]);
@@ -207,7 +223,11 @@ static int ovpn_newlink(struct net_device *dev,
else
netif_carrier_off(dev);
- return register_netdevice(dev);
+ ret = register_netdevice(dev);
+ if (ret < 0)
+ return ret;
+
+ return 0;
}
static int ovpn_fill_info(struct sk_buff *skb, const struct net_device *dev)