All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] pull request for net: batman-adv 2022-03-02
@ 2022-03-02 16:30 ` Simon Wunderlich
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Wunderlich @ 2022-03-02 16:30 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich

Hi David, hi Jakub,

here are some bugfixes for batman-adv which we would like to have integrated into net.

Please pull or let me know of any problem!

There might be a small merge conflict between the net and net-next pull
requests which I both send today, specifically those two commits:

* (batadv/net) 690bb6fb64f5 ("batman-adv: Request iflink once in batadv-on-batadv check")
* (batadv/net-next) 6ee3c393eeb7 ("batman-adv: Demote batadv-on-batadv skip error message")

They should be resolved like this: 

--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -167,13 +167,7 @@
 		return false;
 
 	/* recurse over the parent device */
-<<<<<<<
-	parent_dev = __dev_get_by_index((struct net *)parent_net,
-					dev_get_iflink(net_dev));
-=======
 	parent_dev = __dev_get_by_index((struct net *)parent_net, iflink);
-	/* if we got a NULL parent_dev there is something broken.. */
->>>>>>>
 	if (!parent_dev) {
 		pr_warn("Cannot find parent device. Skipping batadv-on-batadv check for %s\n",
 			net_dev->name);

Thank you,
      Simon

The following changes since commit e783362eb54cd99b2cac8b3a9aeac942e6f6ac07:

  Linux 5.17-rc1 (2022-01-23 10:12:53 +0200)

are available in the Git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batadv-net-pullrequest-20220302

for you to fetch changes up to 6c1f41afc1dbe59d9d3c8bb0d80b749c119aa334:

  batman-adv: Don't expect inter-netns unique iflink indices (2022-03-02 09:24:55 +0100)

----------------------------------------------------------------
Here are some batman-adv bugfixes:

 - Remove redundant iflink requests, by Sven Eckelmann (2 patches)

 - Don't expect inter-netns unique iflink indices, by Sven Eckelmann

----------------------------------------------------------------
Sven Eckelmann (3):
      batman-adv: Request iflink once in batadv-on-batadv check
      batman-adv: Request iflink once in batadv_get_real_netdevice
      batman-adv: Don't expect inter-netns unique iflink indices

 net/batman-adv/hard-interface.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 0/3] pull request for net: batman-adv 2022-03-02
@ 2022-03-02 16:30 ` Simon Wunderlich
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Wunderlich @ 2022-03-02 16:30 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, b.a.t.m.a.n

Hi David, hi Jakub,

here are some bugfixes for batman-adv which we would like to have integrated into net.

Please pull or let me know of any problem!

There might be a small merge conflict between the net and net-next pull
requests which I both send today, specifically those two commits:

* (batadv/net) 690bb6fb64f5 ("batman-adv: Request iflink once in batadv-on-batadv check")
* (batadv/net-next) 6ee3c393eeb7 ("batman-adv: Demote batadv-on-batadv skip error message")

They should be resolved like this: 

--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -167,13 +167,7 @@
 		return false;
 
 	/* recurse over the parent device */
-<<<<<<<
-	parent_dev = __dev_get_by_index((struct net *)parent_net,
-					dev_get_iflink(net_dev));
-=======
 	parent_dev = __dev_get_by_index((struct net *)parent_net, iflink);
-	/* if we got a NULL parent_dev there is something broken.. */
->>>>>>>
 	if (!parent_dev) {
 		pr_warn("Cannot find parent device. Skipping batadv-on-batadv check for %s\n",
 			net_dev->name);

Thank you,
      Simon

The following changes since commit e783362eb54cd99b2cac8b3a9aeac942e6f6ac07:

  Linux 5.17-rc1 (2022-01-23 10:12:53 +0200)

are available in the Git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batadv-net-pullrequest-20220302

for you to fetch changes up to 6c1f41afc1dbe59d9d3c8bb0d80b749c119aa334:

  batman-adv: Don't expect inter-netns unique iflink indices (2022-03-02 09:24:55 +0100)

----------------------------------------------------------------
Here are some batman-adv bugfixes:

 - Remove redundant iflink requests, by Sven Eckelmann (2 patches)

 - Don't expect inter-netns unique iflink indices, by Sven Eckelmann

----------------------------------------------------------------
Sven Eckelmann (3):
      batman-adv: Request iflink once in batadv-on-batadv check
      batman-adv: Request iflink once in batadv_get_real_netdevice
      batman-adv: Don't expect inter-netns unique iflink indices

 net/batman-adv/hard-interface.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] batman-adv: Request iflink once in batadv-on-batadv check
  2022-03-02 16:30 ` Simon Wunderlich
  (?)
@ 2022-03-02 16:30 ` Simon Wunderlich
  2022-03-03  6:00   ` patchwork-bot+netdevbpf
  -1 siblings, 1 reply; 6+ messages in thread
From: Simon Wunderlich @ 2022-03-02 16:30 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

There is no need to call dev_get_iflink multiple times for the same
net_device in batadv_is_on_batman_iface. And since some of the
.ndo_get_iflink callbacks are dynamic (for example via RCUs like in
vxcan_get_iflink), it could easily happen that the returned values are not
stable. The pre-checks before __dev_get_by_index are then of course bogus.

Fixes: b7eddd0b3950 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/hard-interface.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 8a2b78f9c4b2..35aa1122043b 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -149,22 +149,23 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 	struct net *net = dev_net(net_dev);
 	struct net_device *parent_dev;
 	struct net *parent_net;
+	int iflink;
 	bool ret;
 
 	/* check if this is a batman-adv mesh interface */
 	if (batadv_softif_is_valid(net_dev))
 		return true;
 
+	iflink = dev_get_iflink(net_dev);
+
 	/* no more parents..stop recursion */
-	if (dev_get_iflink(net_dev) == 0 ||
-	    dev_get_iflink(net_dev) == net_dev->ifindex)
+	if (iflink == 0 || iflink == net_dev->ifindex)
 		return false;
 
 	parent_net = batadv_getlink_net(net_dev, net);
 
 	/* recurse over the parent device */
-	parent_dev = __dev_get_by_index((struct net *)parent_net,
-					dev_get_iflink(net_dev));
+	parent_dev = __dev_get_by_index((struct net *)parent_net, iflink);
 	/* if we got a NULL parent_dev there is something broken.. */
 	if (!parent_dev) {
 		pr_err("Cannot find parent device\n");
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] batman-adv: Request iflink once in batadv_get_real_netdevice
  2022-03-02 16:30 ` Simon Wunderlich
  (?)
  (?)
@ 2022-03-02 16:30 ` Simon Wunderlich
  -1 siblings, 0 replies; 6+ messages in thread
From: Simon Wunderlich @ 2022-03-02 16:30 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

There is no need to call dev_get_iflink multiple times for the same
net_device in batadv_get_real_netdevice. And since some of the
ndo_get_iflink callbacks are dynamic (for example via RCUs like in
vxcan_get_iflink), it could easily happen that the returned values are not
stable. The pre-checks before __dev_get_by_index are then of course bogus.

Fixes: 5ed4a460a1d3 ("batman-adv: additional checks for virtual interfaces on top of WiFi")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/hard-interface.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 35aa1122043b..e2760cfce190 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -215,14 +215,16 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev)
 	struct net_device *real_netdev = NULL;
 	struct net *real_net;
 	struct net *net;
-	int ifindex;
+	int iflink;
 
 	ASSERT_RTNL();
 
 	if (!netdev)
 		return NULL;
 
-	if (netdev->ifindex == dev_get_iflink(netdev)) {
+	iflink = dev_get_iflink(netdev);
+
+	if (netdev->ifindex == iflink) {
 		dev_hold(netdev);
 		return netdev;
 	}
@@ -232,9 +234,8 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev)
 		goto out;
 
 	net = dev_net(hard_iface->soft_iface);
-	ifindex = dev_get_iflink(netdev);
 	real_net = batadv_getlink_net(netdev, net);
-	real_netdev = dev_get_by_index(real_net, ifindex);
+	real_netdev = dev_get_by_index(real_net, iflink);
 
 out:
 	batadv_hardif_put(hard_iface);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] batman-adv: Don't expect inter-netns unique iflink indices
  2022-03-02 16:30 ` Simon Wunderlich
                   ` (2 preceding siblings ...)
  (?)
@ 2022-03-02 16:30 ` Simon Wunderlich
  -1 siblings, 0 replies; 6+ messages in thread
From: Simon Wunderlich @ 2022-03-02 16:30 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Sabrina Dubroca, Simon Wunderlich

From: Sven Eckelmann <sven@narfation.org>

The ifindex doesn't have to be unique for multiple network namespaces on
the same machine.

  $ ip netns add test1
  $ ip -net test1 link add dummy1 type dummy
  $ ip netns add test2
  $ ip -net test2 link add dummy2 type dummy

  $ ip -net test1 link show dev dummy1
  6: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
      link/ether 96:81:55:1e:dd:85 brd ff:ff:ff:ff:ff:ff
  $ ip -net test2 link show dev dummy2
  6: dummy2: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
      link/ether 5a:3c:af:35:07:c3 brd ff:ff:ff:ff:ff:ff

But the batman-adv code to walk through the various layers of virtual
interfaces uses this assumption because dev_get_iflink handles it
internally and doesn't return the actual netns of the iflink. And
dev_get_iflink only documents the situation where ifindex == iflink for
physical devices.

But only checking for dev->netdev_ops->ndo_get_iflink is also not an option
because ipoib_get_iflink implements it even when it sometimes returns an
iflink != ifindex and sometimes iflink == ifindex. The caller must
therefore make sure itself to check both netns and iflink + ifindex for
equality. Only when they are equal, a "physical" interface was detected
which should stop the traversal. On the other hand, vxcan_get_iflink can
also return 0 in case there was currently no valid peer. In this case, it
is still necessary to stop.

Fixes: b7eddd0b3950 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface")
Fixes: 5ed4a460a1d3 ("batman-adv: additional checks for virtual interfaces on top of WiFi")
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/hard-interface.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index e2760cfce190..35fadb924849 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -157,13 +157,15 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 		return true;
 
 	iflink = dev_get_iflink(net_dev);
-
-	/* no more parents..stop recursion */
-	if (iflink == 0 || iflink == net_dev->ifindex)
+	if (iflink == 0)
 		return false;
 
 	parent_net = batadv_getlink_net(net_dev, net);
 
+	/* iflink to itself, most likely physical device */
+	if (net == parent_net && iflink == net_dev->ifindex)
+		return false;
+
 	/* recurse over the parent device */
 	parent_dev = __dev_get_by_index((struct net *)parent_net, iflink);
 	/* if we got a NULL parent_dev there is something broken.. */
@@ -223,8 +225,7 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev)
 		return NULL;
 
 	iflink = dev_get_iflink(netdev);
-
-	if (netdev->ifindex == iflink) {
+	if (iflink == 0) {
 		dev_hold(netdev);
 		return netdev;
 	}
@@ -235,6 +236,14 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev)
 
 	net = dev_net(hard_iface->soft_iface);
 	real_net = batadv_getlink_net(netdev, net);
+
+	/* iflink to itself, most likely physical device */
+	if (net == real_net && netdev->ifindex == iflink) {
+		real_netdev = netdev;
+		dev_hold(real_netdev);
+		goto out;
+	}
+
 	real_netdev = dev_get_by_index(real_net, iflink);
 
 out:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] batman-adv: Request iflink once in batadv-on-batadv check
  2022-03-02 16:30 ` [PATCH 1/3] batman-adv: Request iflink once in batadv-on-batadv check Simon Wunderlich
@ 2022-03-03  6:00   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-03-03  6:00 UTC (permalink / raw)
  To: Simon Wunderlich; +Cc: davem, kuba, netdev, b.a.t.m.a.n, sven

Hello:

This series was applied to netdev/net.git (master)
by Simon Wunderlich <sw@simonwunderlich.de>:

On Wed,  2 Mar 2022 17:30:47 +0100 you wrote:
> From: Sven Eckelmann <sven@narfation.org>
> 
> There is no need to call dev_get_iflink multiple times for the same
> net_device in batadv_is_on_batman_iface. And since some of the
> .ndo_get_iflink callbacks are dynamic (for example via RCUs like in
> vxcan_get_iflink), it could easily happen that the returned values are not
> stable. The pre-checks before __dev_get_by_index are then of course bogus.
> 
> [...]

Here is the summary with links:
  - [1/3] batman-adv: Request iflink once in batadv-on-batadv check
    https://git.kernel.org/netdev/net/c/690bb6fb64f5
  - [2/3] batman-adv: Request iflink once in batadv_get_real_netdevice
    https://git.kernel.org/netdev/net/c/6116ba09423f
  - [3/3] batman-adv: Don't expect inter-netns unique iflink indices
    https://git.kernel.org/netdev/net/c/6c1f41afc1db

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-03  6:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-02 16:30 [PATCH 0/3] pull request for net: batman-adv 2022-03-02 Simon Wunderlich
2022-03-02 16:30 ` Simon Wunderlich
2022-03-02 16:30 ` [PATCH 1/3] batman-adv: Request iflink once in batadv-on-batadv check Simon Wunderlich
2022-03-03  6:00   ` patchwork-bot+netdevbpf
2022-03-02 16:30 ` [PATCH 2/3] batman-adv: Request iflink once in batadv_get_real_netdevice Simon Wunderlich
2022-03-02 16:30 ` [PATCH 3/3] batman-adv: Don't expect inter-netns unique iflink indices Simon Wunderlich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.