All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
@ 2017-06-16 12:10 Serhey Popovych
  2017-06-17 11:21 ` Sergei Shtylyov
  2017-06-19 18:41 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Serhey Popovych @ 2017-06-16 12:10 UTC (permalink / raw)
  To: netdev

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 <serhe.popovych@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
  2017-06-16 12:10 [PATCH] loopback: Force LOOPBACK_IFINDEX for registration Serhey Popovych
@ 2017-06-17 11:21 ` Sergei Shtylyov
  2017-06-19 18:41 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2017-06-17 11:21 UTC (permalink / raw)
  To: Serhey Popovych, netdev

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 <serhe.popovych@gmail.com>
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
  2017-06-16 12:10 [PATCH] loopback: Force LOOPBACK_IFINDEX for registration Serhey Popovych
  2017-06-17 11:21 ` Sergei Shtylyov
@ 2017-06-19 18:41 ` David Miller
  2017-06-20 10:32   ` Serhey Popovych
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2017-06-19 18:41 UTC (permalink / raw)
  To: serhe.popovych; +Cc: netdev

From: Serhey Popovych <serhe.popovych@gmail.com>
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 <serhe.popovych@gmail.com>

Is the BUG_ON() triggering, if so why?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
  2017-06-19 18:41 ` David Miller
@ 2017-06-20 10:32   ` Serhey Popovych
  2017-06-21  9:35     ` Serhey Popovych
  0 siblings, 1 reply; 6+ messages in thread
From: Serhey Popovych @ 2017-06-20 10:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev


> From: Serhey Popovych <serhe.popovych@gmail.com>
> 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 <serhe.popovych@gmail.com>
> 
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
  2017-06-20 10:32   ` Serhey Popovych
@ 2017-06-21  9:35     ` Serhey Popovych
  2017-06-21 15:44       ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Serhey Popovych @ 2017-06-21  9:35 UTC (permalink / raw)
  To: David Miller; +Cc: netdev


> 
>> From: Serhey Popovych <serhe.popovych@gmail.com>
>> 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 <serhe.popovych@gmail.com>
>>
>> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] loopback: Force LOOPBACK_IFINDEX for registration
  2017-06-21  9:35     ` Serhey Popovych
@ 2017-06-21 15:44       ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-06-21 15:44 UTC (permalink / raw)
  To: serhe.popovych; +Cc: netdev

From: Serhey Popovych <serhe.popovych@gmail.com>
Date: Wed, 21 Jun 2017 12:35:12 +0300

> 
>> 
>>> From: Serhey Popovych <serhe.popovych@gmail.com>
>>> 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 <serhe.popovych@gmail.com>
>>>
>>> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-21 16:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-16 12:10 [PATCH] loopback: Force LOOPBACK_IFINDEX for registration Serhey Popovych
2017-06-17 11:21 ` Sergei Shtylyov
2017-06-19 18:41 ` David Miller
2017-06-20 10:32   ` Serhey Popovych
2017-06-21  9:35     ` Serhey Popovych
2017-06-21 15:44       ` David Miller

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.