[PATCH] loopback: Force LOOPBACK_IFINDEX for registration

Now with commit 9c7dafb (net: Allow to create links with
given ifindex) support registration of network devices
with specific ifindex is added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>
---
 drivers/net/loopback.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 3061249..d7233aa 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c

@@ -210,12 +210,13 @@ static __net_init int loopback_net_init(struct net *net)
 	if (!dev)
 		goto out;
 
+	dev->ifindex = LOOPBACK_IFINDEX;
+
 	dev_net_set(dev, net);
 	err = register_netdev(dev);
 	if (err)
 		goto out_free_netdev;
 
-	BUG_ON(dev->ifindex != LOOPBACK_IFINDEX);
 	net->loopback_dev = dev;
 	return 0;
 

-- 
1.8.3.1

Hello!

On 6/16/2017 3:10 PM, Serhey Popovych wrote:

Now with commit 9c7dafb (net: Allow to create links with

   The commit citing style is standardized: 12-digit SHA1 and the summary enclosed in ("").

given ifindex) support registration of network devices

   Support for.

with specific ifindex is added.

   Was added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>

[...]

MBR, Sergei

From: Serhey Popovych <redacted>
Date: Fri, 16 Jun 2017 15:10:03 +0300

Now with commit 9c7dafb (net: Allow to create links with
given ifindex) support registration of network devices
with specific ifindex is added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>

Is the BUG_ON() triggering, if so why?

It looks to me that unless there is a bug, this assignment
is unnecessary.

From: Serhey Popovych <redacted>
Date: Fri, 16 Jun 2017 15:10:03 +0300

quoted

Now with commit 9c7dafb (net: Allow to create links with
given ifindex) support registration of network devices
with specific ifindex is added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>

Is the BUG_ON() triggering, if so why?

It looks to me that unless there is a bug, this assignment
is unnecessary.

Okay, get your position, thanks!

-- 
Thanks, Serhey

quoted

From: Serhey Popovych <redacted>
Date: Fri, 16 Jun 2017 15:10:03 +0300

quoted

Now with commit 9c7dafb (net: Allow to create links with
given ifindex) support registration of network devices
with specific ifindex is added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>

Is the BUG_ON() triggering, if so why?

It looks to me that unless there is a bug, this assignment
is unnecessary.

Okay, get your position, thanks!

Sorry for raising again, but my objectives following:

BUG_ON() might be compiled out when CONFIG_BUG=n.
That's open possibility in buggy setups to have
"lo" with ->ifindex other than LOOPBACK_IFINDEX.

By forcing register_netdevice() via setting
ifindex = LOOPBACK_IFINDEX we address CONFIG_BUG=n
case.

We can leave BUG_ON(), but add ifindex = LOOPBACK_IFINDEX
to ensure register_netdevice() operates properly.

What do you think?

-- 
Thanks, Serhey

From: Serhey Popovych <redacted>
Date: Wed, 21 Jun 2017 12:35:12 +0300

quoted

quoted

From: Serhey Popovych <redacted>
Date: Fri, 16 Jun 2017 15:10:03 +0300

quoted

Now with commit 9c7dafb (net: Allow to create links with
given ifindex) support registration of network devices
with specific ifindex is added.

We can force loopback network device index before call to
register_netdev() to ensure we always configure it with
LOOPBACK_IFINDEX.

Kill BUG_ON() since system can continue without network
namespace failed in loopback init path, unless it is
init_net namespace where we panic() anyway.

Signed-off-by: Serhey Popovych <redacted>

Is the BUG_ON() triggering, if so why?

It looks to me that unless there is a bug, this assignment
is unnecessary.

Okay, get your position, thanks!

Sorry for raising again, but my objectives following:

BUG_ON() might be compiled out when CONFIG_BUG=n.
That's open possibility in buggy setups to have
"lo" with ->ifindex other than LOOPBACK_IFINDEX.

By forcing register_netdevice() via setting
ifindex = LOOPBACK_IFINDEX we address CONFIG_BUG=n
case.

We can leave BUG_ON(), but add ifindex = LOOPBACK_IFINDEX
to ensure register_netdevice() operates properly.

What do you think?

I don't understand what you are trying to achieve.

BUG_ON() is an assertion.

It checks that an invariant holds.

Whether the check is enabled or not has no bearing on the
invariant.

There is no reason to make the assignment if it is supposed
to be set properly already, period.  The assertion states
that it is supposed to be set properly when we get to this
code.