Netdev Archive on
help / color / mirror / Atom feed
From: Jakub Kicinski <>
To: Leon Romanovsky <>
Cc: "David S . Miller" <>,
	Guangbin Huang <>,
	Ido Schimmel <>, Jiri Pirko <>,,
	Michael Guralnik <>,, Saeed Mahameed <>,
	Salil Mehta <>,
	Tariq Toukan <>,
	Yisen Zhuang <>,
	Yufeng Mo <>
Subject: Re: [PATCH net-next 0/5] Move devlink_register to be near devlink_reload_enable
Date: Wed, 11 Aug 2021 07:18:17 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <YRPYMKxPHUkegEhj@unreal>

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.

  parent reply	other threads:[~2021-08-11 14:18 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 [this message]
2021-08-11 14:36           ` Leon Romanovsky
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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH net-next 0/5] Move devlink_register to be near devlink_reload_enable' \

* 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).