lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 5 Oct 2021 09:13:18 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Ido Schimmel <idosch@...dia.com>,
        Ingo Molnar <mingo@...hat.com>, Jiri Pirko <jiri@...dia.com>,
        linux-kernel@...r.kernel.org, linux-rdma@...r.kernel.org,
        mlxsw@...dia.com, Moshe Shemesh <moshe@...dia.com>,
        netdev@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>,
        Salil Mehta <salil.mehta@...wei.com>,
        Shay Drory <shayd@...dia.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Tariq Toukan <tariqt@...dia.com>,
        Yisen Zhuang <yisen.zhuang@...wei.com>
Subject: Re: [PATCH net-next v2 1/5] devlink: Reduce struct devlink exposure

On Mon, Oct 04, 2021 at 04:38:08PM -0700, Jakub Kicinski wrote:
> On Sun,  3 Oct 2021 21:12:02 +0300 Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> > 
> > The declaration of struct devlink in general header provokes the
> > situation where internal fields can be accidentally used by the driver
> > authors. In order to reduce such possible situations, let's reduce the
> > namespace exposure of struct devlink.
> > 
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> 
> 100% subjective but every time I decided to hide a structure definition
> like this I came to regret it later. The fact there is only one minor
> infraction in drivers poking at members seems to prove this is not in
> fact needed.

Yes, it is subjective, my experience is completely opposite :). Every
time the internals were exposed, they were abused.

IMHO, the one user that poked into the struct devlink internals is a pure
luck together with lack of devlink adoption outside of the netdev which
limited number of devlink API users. The more devlink will be used, the
more creative usage will be.

For example, ionic had internal logic based on internal devlink_port state:
 * c2255ff47768 ("ionic: cleanly release devlink instance")
 * d7907a2b1a3b ("devlink: Remove duplicated registration check")

However, this patch was written not because of having right software
abstraction, but because of the next patch, where I needed to have
declaration of "struct devlink_ops" before struct devlink itself.

Without this patch, I would need to heavily reshuffle include/net/devlink.h
to have structs declarations written in different order. So a lot of
churn for something that needs to be fixed anyway (in my opinion).

Thanks

Powered by blists - more mailing lists