Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "David S . Miller" <davem@davemloft.net>,
Guangbin Huang <huangguangbin2@huawei.com>,
Ido Schimmel <idosch@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
linux-kernel@vger.kernel.org,
Michael Guralnik <michaelgur@mellanox.com>,
netdev@vger.kernel.org, Saeed Mahameed <saeedm@nvidia.com>,
Salil Mehta <salil.mehta@huawei.com>,
Tariq Toukan <tariqt@nvidia.com>,
Yisen Zhuang <yisen.zhuang@huawei.com>,
Yufeng Mo <moyufeng@huawei.com>
Subject: Re: [PATCH net-next 0/5] Move devlink_register to be near devlink_reload_enable
Date: Wed, 11 Aug 2021 17:36:36 +0300 [thread overview]
Message-ID: <YRPgdCG4pFtsAS0D@unreal> (raw)
In-Reply-To: <20210811071817.4af5ab34@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Wed, Aug 11, 2021 at 07:18:17AM -0700, Jakub Kicinski wrote:
> On Wed, 11 Aug 2021 17:01:20 +0300 Leon Romanovsky wrote:
> > > > Not really, they will register but won't be accessible from the user space.
> > > > The only difference is the location of "[dev,new] ..." notification.
> > >
> > > Is that because of mlx5's use of auxdev, or locking? I don't see
> > > anything that should prevent the port notification from coming out.
> >
> > And it is ok, kernel can (and does) send notifications, because we left
> > devlink_ops assignment to be in devlink_alloc(). It ensures that all
> > flows that worked before will continue to work without too much changes.
> >
> > > I think the notifications need to get straightened out, we can't notify
> > > about sub-objects until the object is registered, since they are
> > > inaccessible.
> >
> > I'm not sure about that. You present the case where kernel and user
> > space races against each other and historically kernel doesn't protect
> > from such flows.
> >
> > For example, you can randomly remove and add kernel modules. At some
> > point of time, you will get "missing symbols errors", just because
> > one module tries to load and it depends on already removed one.
>
> Sure. But there is a difference between an error because another
> actor did something conflicting, asynchronously, and API which by design
> sends notifications which can't be acted upon until later point in time,
> because kernel sent them too early.
>
> > We must protect kernel and this is what I do. User shouldn't access
> > devlink instance before he sees "dev name" notification.
>
> Which is a new rule, and therefore a uAPI change..
>
> > Of course, we can move various iterators to devlink_register(), but it
> > will make code much complex, because we have objects that can be
> > registered at any time (IMHO. trap is one of them) and I will need to
> > implement notification logic that separate objects that were created
> > before devlink_register and after.
>
> I appreciate it's a PITA but it is the downside of a solution where
> registration of co-dependent objects exposed via devlink is reordered
> in the kernel.
No problem, I will rewrite notification logic to be queue-based mechanism.
Thanks
next prev parent reply other threads:[~2021-08-11 14:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-10 13:37 Leon Romanovsky
2021-08-10 13:37 ` [PATCH net-next 1/5] net: hns3: remove always exist devlink pointer check Leon Romanovsky
2021-08-10 13:37 ` [PATCH net-next 2/5] net/mlx4: Move devlink_register to be the last initialization command Leon Romanovsky
2021-08-10 13:37 ` [PATCH net-next 3/5] mlxsw: core: Refactor code to publish devlink ops when device is ready Leon Romanovsky
2021-08-10 13:37 ` [PATCH net-next 4/5] net/mlx5: Accept devlink user input after driver initialization complete Leon Romanovsky
2021-08-10 13:37 ` [PATCH net-next 5/5] netdevsim: Delay user access till probe is finished Leon Romanovsky
2021-08-10 23:53 ` [PATCH net-next 0/5] Move devlink_register to be near devlink_reload_enable Jakub Kicinski
2021-08-11 6:10 ` Leon Romanovsky
2021-08-11 13:27 ` Jakub Kicinski
2021-08-11 14:01 ` Leon Romanovsky
2021-08-11 14:15 ` Leon Romanovsky
2021-08-11 14:18 ` Jakub Kicinski
2021-08-11 14:36 ` Leon Romanovsky [this message]
2021-08-12 4:10 ` Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YRPgdCG4pFtsAS0D@unreal \
--to=leon@kernel.org \
--cc=davem@davemloft.net \
--cc=huangguangbin2@huawei.com \
--cc=idosch@nvidia.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michaelgur@mellanox.com \
--cc=moyufeng@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=saeedm@nvidia.com \
--cc=salil.mehta@huawei.com \
--cc=tariqt@nvidia.com \
--cc=yisen.zhuang@huawei.com \
--subject='Re: [PATCH net-next 0/5] Move devlink_register to be near devlink_reload_enable' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).