From: Ilia Lin <[email protected]>
Add an API that returns true, if the net_dev_init was already called,
and the driver was initialized.
Some early drivers, that are initialized during the subsys_initcall
may try accessing the net_dev or NAPI APIs before the net_dev_init,
and will encounter a kernel bug. This API provides a way to handle
this and manage by deferring or by other way.
Signed-off-by: Ilia Lin <[email protected]>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 98d290c..d17d364 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2600,6 +2600,8 @@ enum netdev_cmd {
};
const char *netdev_cmd_to_name(enum netdev_cmd cmd);
+bool is_net_dev_initialized(void);
+
int register_netdevice_notifier(struct notifier_block *nb);
int unregister_netdevice_notifier(struct notifier_block *nb);
int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 316349f..1b50488 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1793,6 +1793,23 @@ static void call_netdevice_unregister_net_notifiers(struct notifier_block *nb,
static int dev_boot_phase = 1;
/**
+ * is_net_dev_initialized - check, whether the net_dev was
+ * initialized
+ *
+ * Returns true, if the net_dev_init was already called, and
+ * the driver is initialized.
+ *
+ * This is useful for early drivers trying to call net_dev and
+ * NAPI APIs
+ */
+
+bool is_net_dev_initialized(void)
+{
+ return !(bool)dev_boot_phase;
+}
+EXPORT_SYMBOL(is_net_dev_initialized);
+
+/**
* register_netdevice_notifier - register a network notifier block
* @nb: notifier
*
On Sun, Jul 26, 2020 at 10:37:54PM +0300, Ilia Lin wrote:
> From: Ilia Lin <[email protected]>
>
> Add an API that returns true, if the net_dev_init was already called,
> and the driver was initialized.
>
> Some early drivers, that are initialized during the subsys_initcall
> may try accessing the net_dev or NAPI APIs before the net_dev_init,
> and will encounter a kernel bug. This API provides a way to handle
> this and manage by deferring or by other way.
Hi Ilia
You need to include a user of this new API.
I also have to wonder why a network device driver is being probed the
subsys_initcall.
Andrew
From: Andrew Lunn <[email protected]>
Date: Sun, 26 Jul 2020 21:45:28 +0200
> I also have to wonder why a network device driver is being probed the
> subsys_initcall.
This makes me wonder how this interface could even be useful. The
only way to fix the problem is to change when the device is probed,
which would mean changing which initcall it uses. So at run time,
this information can't do much.
Hi Andrew and David,
Thank you for your comments!
The client driver is still work in progress, but it can be seen here:
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_api.c#n3842
For HW performance reasons, it has to be in subsys_initcall.
Here is the register_netdev call:
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c#n2497
And it is going to be in the subsys_initcall as well.
Thanks,
Ilia
On Mon, Jul 27, 2020 at 8:32 PM David Miller <[email protected]> wrote:
>
> From: Andrew Lunn <[email protected]>
> Date: Sun, 26 Jul 2020 21:45:28 +0200
>
> > I also have to wonder why a network device driver is being probed the
> > subsys_initcall.
>
> This makes me wonder how this interface could even be useful. The
> only way to fix the problem is to change when the device is probed,
> which would mean changing which initcall it uses. So at run time,
> this information can't do much.
Hi Andrew and David,
Thank you for your comments!
The client driver is still work in progress, but it can be seen here:
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_api.c#n3842
For HW performance reasons, it has to be in subsys_initcall.
Here is the register_netdev call:
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_v3/rmnet_ipa.c#n2497
And it is going to be in the subsys_initcall as well.
Thanks,
Ilia
On Mon, Jul 27, 2020 at 8:32 PM David Miller <[email protected]> wrote:
>
> From: Andrew Lunn <[email protected]>
> Date: Sun, 26 Jul 2020 21:45:28 +0200
>
> > I also have to wonder why a network device driver is being probed the
> > subsys_initcall.
>
> This makes me wonder how this interface could even be useful. The
> only way to fix the problem is to change when the device is probed,
> which would mean changing which initcall it uses. So at run time,
> this information can't do much.
On Tue, Aug 04, 2020 at 08:47:18PM +0300, Ilia Lin wrote:
> Hi Andrew and David,
Hi Ilia
Please don't top post.
>
> Thank you for your comments!
>
> The client driver is still work in progress, but it can be seen here:
> https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_api.c#n3842
>
> For HW performance reasons, it has to be in subsys_initcall.
Well, until the user of this new API is ready, we will not accept the
patch.
You also need to explain "For HW performance reasons". Why is this
driver special that it can do things which no over driver does?
And you should really be working on net-next, not this dead kernel
version, if you want to get merged into mainline.
Network drivers do not belong is drivers/platform. There is also ready
a drivers/net/ipa, so i assume you will move there.
Andrew
> > Well, until the user of this new API is ready, we will not accept the
> > patch.
> OK, but once we submit the change in the driver, is it good to go?
No. You really do need to explain why it is needed, and why it is
safe.
> > You also need to explain "For HW performance reasons". Why is this
> > driver special that it can do things which no over driver does?
> There are very strict KPI requirements. E.g. automotive rear mirror
> must be online 3 sec since boot.
Which does not explain why this driver is special. What you really
should be thinking about is having the required drivers for this use
case built in, and the rest as modules. Get your time critical parts
running, and then load the rest of the driver moduless and kick off
additional user space in a second phase.
You are breaking all sorts of assumptions by loading network drivers
before the stack is ready. You need to expect all sorts of nasty bugs.
If this was just in your vendor kernel, we would not care too much, it
is your problem to solve. But by doing this in mainline, you are
setting a precedent for others to do it as well. And then we really do
need to care about the broken assumptions. I doubt we are ready to
allow this.
Andrew
My comments inline.
Thanks,
Ilia Lin
On Tue, Aug 4, 2020 at 10:24 PM Andrew Lunn <[email protected]> wrote:
>
> On Tue, Aug 04, 2020 at 08:47:18PM +0300, Ilia Lin wrote:
> > Hi Andrew and David,
>
> Hi Ilia
>
> Please don't top post.
>
> >
> > Thank you for your comments!
> >
> > The client driver is still work in progress, but it can be seen here:
> > https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/platform/msm/ipa/ipa_api.c#n3842
> >
> > For HW performance reasons, it has to be in subsys_initcall.
>
> Well, until the user of this new API is ready, we will not accept the
> patch.
OK, but once we submit the change in the driver, is it good to go?
>
> You also need to explain "For HW performance reasons". Why is this
> driver special that it can do things which no over driver does?
There are very strict KPI requirements. E.g. automotive rear mirror
must be online 3 sec since boot.
>
> And you should really be working on net-next, not this dead kernel
> version, if you want to get merged into mainline.
Of course, the upstream submition will be done on the mainline. I just
gave an example of an existing product driver.
>
> Network drivers do not belong is drivers/platform. There is also ready
> a drivers/net/ipa, so i assume you will move there.
Sure, the driver in the drivers/net/ipa is the one that will be
updated in the future.
>
> Andrew