| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260202194946.64555356@kernel.org>
Date: Mon, 2 Feb 2026 19:49:46 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Tariq Toukan <tariqt@...dia.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Donald Hunter <donald.hunter@...il.com>, Jiri Pirko
<jiri@...nulli.us>, Jonathan Corbet <corbet@....net>, Saeed Mahameed
<saeedm@...dia.com>, "Leon Romanovsky" <leon@...nel.org>, Mark Bloch
<mbloch@...dia.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-rdma@...r.kernel.org>, Gal Pressman <gal@...dia.com>, Moshe Shemesh
<moshe@...dia.com>, Carolina Jubran <cjubran@...dia.com>, Cosmin Ratiu
<cratiu@...dia.com>, Jiri Pirko <jiri@...dia.com>, Randy Dunlap
<rdunlap@...radead.org>, Simon Horman <horms@...nel.org>, Krzysztof
Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH net-next V7 02/14] devlink: introduce shared devlink
instance for PFs on same chip
On Wed, 28 Jan 2026 13:25:32 +0200 Tariq Toukan wrote:
> From: Jiri Pirko <jiri@...dia.com>
>
> Multiple PFs may reside on the same physical chip, running a single
> firmware. Some of the resources and configurations may be shared among
> these PFs. Currently, there is no good object to pin the configuration
> knobs on.
>
> Introduce a shared devlink instance, instantiated upon probe of the
> first PF and removed during remove of the last PF. The shared devlink
> instance is backed by a faux device, as there is no PCI device related
> to it. The implementation uses reference counting to manage the
> lifecycle: each PF that probes calls devlink_shd_get() to get or create
> the shared instance, and calls devlink_shd_put() when it removes. The
> shared instance is automatically destroyed when the last PF removes.
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index cb839e0435a1..c453faec8ebf 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1644,6 +1644,12 @@ void devlink_register(struct devlink *devlink);
> void devlink_unregister(struct devlink *devlink);
> void devlink_free(struct devlink *devlink);
>
> +struct devlink *devlink_shd_get(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size);
> +void devlink_shd_put(struct devlink *devlink);
> +void *devlink_shd_get_priv(struct devlink *devlink);
Would Cosmin or someone else be willing to take on co-maintainership
of this API (including reviews of other drivers using it)?
We could add a maintainers entry with:
K: devlink_shd_
So y'all get CCed.
> +#include <linux/device/faux.h>
> +#include <net/devlink.h>
> +/* This structure represents a shared devlink instance,
> + * there is one created per identifier (e.g., serial number).
> + */
> +struct devlink_shd {
> + struct list_head list; /* Node in shd list */
> + const char *id; /* Identifier string (e.g., serial number) */
Why does this have to be a string? The identifier should be irrelevant,
and if something like serial number exists it can be reported in dev
info for the shared instance?
> + struct faux_device *faux_dev; /* Related faux device */
> + refcount_t refcount; /* Reference count */
> + char priv[] __aligned(NETDEV_ALIGN); /* Driver private data */
size member annotated with __counted_by() is missing here
> +};
> +static struct devlink_shd *devlink_shd_create(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size)
> +{
> + struct faux_device *faux_dev;
> + struct devlink_shd *shd;
> + struct devlink *devlink;
> +
> + /* Create faux device - probe will be called synchronously */
> + faux_dev = faux_device_create(id, NULL, NULL);
> + if (!faux_dev)
> + return NULL;
> +
> + devlink = devlink_alloc(ops, sizeof(struct devlink_shd) + priv_size,
> + &faux_dev->dev);
> + if (!devlink)
> + goto err_devlink_alloc;
error labels should be named after the target not the source in new code
> + shd = devlink_priv(devlink);
> +
> + shd->id = kstrdup(id, GFP_KERNEL);
> + if (!shd->id)
> + goto err_kstrdup_id;
> + shd->faux_dev = faux_dev;
> + refcount_set(&shd->refcount, 1);
> +
> + devl_lock(devlink);
> + devl_register(devlink);
> + devl_unlock(devlink);
> +
> + list_add_tail(&shd->list, &shd_list);
> +
> + return shd;
> +
> +err_kstrdup_id:
> + devlink_free(devlink);
> +
> +err_devlink_alloc:
> + faux_device_destroy(faux_dev);
> + return NULL;
> +}
> +struct devlink *devlink_shd_get(const char *id,
> + const struct devlink_ops *ops,
> + size_t priv_size)
> +{
> + struct devlink_shd *shd;
> +
> + if (WARN_ON(!id || !ops))
> + return NULL;
Seems a little too defensive to check input attrs against NULL.
Let the kernel crash if someone is foolish enough..
> + mutex_lock(&shd_mutex);
> +
> + shd = devlink_shd_lookup(id);
> + if (!shd)
> + shd = devlink_shd_create(id, ops, priv_size);
> + else
> + refcount_inc(&shd->refcount);
> +
> + mutex_unlock(&shd_mutex);
> + return shd ? priv_to_devlink(shd) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(devlink_shd_get);
> +
> +/**
> + * devlink_shd_put - Release a reference on a shared devlink instance
> + * @devlink: Shared devlink instance
> + *
> + * Release a reference on a shared devlink instance obtained via
> + * devlink_shd_get().
> + */
> +void devlink_shd_put(struct devlink *devlink)
> +{
> + struct devlink_shd *shd;
> +
> + if (WARN_ON(!devlink))
> + return;
ditto
> + mutex_lock(&shd_mutex);
> + shd = devlink_priv(devlink);
> + if (refcount_dec_and_test(&shd->refcount))
> + devlink_shd_destroy(shd);
> + mutex_unlock(&shd_mutex);
> +}
> +EXPORT_SYMBOL_GPL(devlink_shd_put);
Powered by blists - more mailing lists