2022-11-28 04:28:53

by Vincent MAILHOL

[permalink] [raw]
Subject: [PATCH net-next v4 0/3] net: devlink: return the driver name in devlink_nl_info_fill

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

The goal of this series is to have the devlink core to report this
information instead of the drivers.

The first patch fulfills the actual goal of this series: modify
devlink core to report the driver name and clean-up all drivers. Both
have to be done in an atomic change to avoid attribute duplication.

The second patch removes the devlink_info_driver_name_put() function
to prevent future drivers from reporting the driver name themselves.

Finally, the third and last patch allows the core to call
devlink_nl_info_fill() even if the devlink_ops::info_get() callback is
NULL. This allows to do further more clean up in the drivers.
---
* Changelog *

v3 -> v4

* Ido pointed out that the mlxsw did not need to be fixed:
https://lore.kernel.org/netdev/Y4ONgD4dAj8yU2%2F+@shredder/
Remove the first two patches from the series.

v2 -> v3

* [PATCH 3/5] remove the call to devlink_info_driver_name_put() in
mlxsw driver as well (this was missing in v2, making the build
fail... sorry for the noise).

* add additional people in CC as pointed by netdev patchwork CI:
https://patchwork.kernel.org/project/netdevbpf/list/?series=699451

* use the "Link:" prefix before URL to silence checkpatch's line
length warning.


RFC v1 -> v2

* drop the RFC tag

* big rework following the discussion on RFC:
https://lore.kernel.org/netdev/[email protected]/
Went from one patch to a series of five patches:

* drop the idea to report the USB serial number following Greg's
comment:
https://lore.kernel.org/linux-usb/Y3+VfNdt%[email protected]/


Vincent Mailhol (3):
net: devlink: let the core report the driver name instead of the
drivers
net: devlink: remove devlink_info_driver_name_put()
net: devlink: make the devlink_ops::info_get() callback optional

.../marvell/octeontx2/otx2_cpt_devlink.c | 4 ---
drivers/net/dsa/hirschmann/hellcreek.c | 5 ---
drivers/net/dsa/mv88e6xxx/devlink.c | 5 ---
drivers/net/dsa/sja1105/sja1105_devlink.c | 12 ++-----
.../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4 ---
.../freescale/dpaa2/dpaa2-eth-devlink.c | 11 +-----
.../ethernet/fungible/funeth/funeth_devlink.c | 7 ----
.../hisilicon/hns3/hns3pf/hclge_devlink.c | 5 ---
.../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 5 ---
drivers/net/ethernet/intel/ice/ice_devlink.c | 6 ----
.../marvell/octeontx2/af/rvu_devlink.c | 7 ----
.../marvell/octeontx2/nic/otx2_devlink.c | 15 --------
.../marvell/prestera/prestera_devlink.c | 5 ---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 4 ---
drivers/net/ethernet/mellanox/mlxsw/core.c | 5 ---
.../net/ethernet/netronome/nfp/nfp_devlink.c | 4 ---
.../ethernet/pensando/ionic/ionic_devlink.c | 4 ---
drivers/net/ethernet/qlogic/qed/qed_devlink.c | 4 ---
drivers/net/netdevsim/dev.c | 3 --
drivers/ptp/ptp_ocp.c | 4 ---
include/net/devlink.h | 2 --
net/core/devlink.c | 35 ++++++++++++-------
22 files changed, 26 insertions(+), 130 deletions(-)

--
2.25.1


2022-11-28 04:29:39

by Vincent MAILHOL

[permalink] [raw]
Subject: [PATCH net-next v4 1/3] net: devlink: let the core report the driver name instead of the drivers

The driver name is available in device_driver::name. Right now,
drivers still have to report this piece of information themselves in
their devlink_ops::info_get callback function.

In order to factorize code, make devlink_nl_info_fill() add the driver
name attribute.

nla_put() does not check if an attribute already exists and
unconditionally reserves new space, c.f. [1]. To avoid attribute
duplication, clean-up all the drivers which are currently reporting
the driver name in their callback.

[1] __nla_put() from lib/nlattr.c
Link: https://elixir.bootlin.com/linux/v6.0/source/lib/nlattr.c#L993

Signed-off-by: Vincent Mailhol <[email protected]>
---
.../crypto/marvell/octeontx2/otx2_cpt_devlink.c | 4 ----
drivers/net/dsa/hirschmann/hellcreek.c | 5 -----
drivers/net/dsa/mv88e6xxx/devlink.c | 5 -----
drivers/net/dsa/sja1105/sja1105_devlink.c | 12 +++---------
.../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4 ----
.../freescale/dpaa2/dpaa2-eth-devlink.c | 11 +----------
.../ethernet/fungible/funeth/funeth_devlink.c | 2 +-
.../hisilicon/hns3/hns3pf/hclge_devlink.c | 5 -----
.../hisilicon/hns3/hns3vf/hclgevf_devlink.c | 5 -----
drivers/net/ethernet/intel/ice/ice_devlink.c | 6 ------
.../ethernet/marvell/octeontx2/af/rvu_devlink.c | 2 +-
.../marvell/octeontx2/nic/otx2_devlink.c | 9 +--------
.../marvell/prestera/prestera_devlink.c | 5 -----
.../net/ethernet/mellanox/mlx5/core/devlink.c | 4 ----
drivers/net/ethernet/mellanox/mlxsw/core.c | 5 -----
.../net/ethernet/netronome/nfp/nfp_devlink.c | 4 ----
.../net/ethernet/pensando/ionic/ionic_devlink.c | 4 ----
drivers/net/ethernet/qlogic/qed/qed_devlink.c | 4 ----
drivers/net/netdevsim/dev.c | 3 ---
drivers/ptp/ptp_ocp.c | 4 ----
net/core/devlink.c | 17 +++++++++++++++++
21 files changed, 24 insertions(+), 96 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
index 7503f6b18ac5..a2aba0b0d68a 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
@@ -76,10 +76,6 @@ static int otx2_cpt_devlink_info_get(struct devlink *dl,
struct otx2_cptpf_dev *cptpf = cpt_dl->cptpf;
int err;

- err = devlink_info_driver_name_put(req, "rvu_cptpf");
- if (err)
- return err;
-
err = otx2_cpt_dl_info_firmware_version_put(req, cptpf->eng_grps.grp,
"fw.ae", OTX2_CPT_AE_TYPES);
if (err)
diff --git a/drivers/net/dsa/hirschmann/hellcreek.c b/drivers/net/dsa/hirschmann/hellcreek.c
index 951f7935c872..595a548bb0a8 100644
--- a/drivers/net/dsa/hirschmann/hellcreek.c
+++ b/drivers/net/dsa/hirschmann/hellcreek.c
@@ -1176,11 +1176,6 @@ static int hellcreek_devlink_info_get(struct dsa_switch *ds,
struct netlink_ext_ack *extack)
{
struct hellcreek *hellcreek = ds->priv;
- int ret;
-
- ret = devlink_info_driver_name_put(req, "hellcreek");
- if (ret)
- return ret;

return devlink_info_version_fixed_put(req,
DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
diff --git a/drivers/net/dsa/mv88e6xxx/devlink.c b/drivers/net/dsa/mv88e6xxx/devlink.c
index 1266eabee086..a08dab75e0c0 100644
--- a/drivers/net/dsa/mv88e6xxx/devlink.c
+++ b/drivers/net/dsa/mv88e6xxx/devlink.c
@@ -821,11 +821,6 @@ int mv88e6xxx_devlink_info_get(struct dsa_switch *ds,
struct netlink_ext_ack *extack)
{
struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- err = devlink_info_driver_name_put(req, "mv88e6xxx");
- if (err)
- return err;

return devlink_info_version_fixed_put(req,
DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
diff --git a/drivers/net/dsa/sja1105/sja1105_devlink.c b/drivers/net/dsa/sja1105/sja1105_devlink.c
index 10c6fea1227f..da532614f34a 100644
--- a/drivers/net/dsa/sja1105/sja1105_devlink.c
+++ b/drivers/net/dsa/sja1105/sja1105_devlink.c
@@ -120,16 +120,10 @@ int sja1105_devlink_info_get(struct dsa_switch *ds,
struct netlink_ext_ack *extack)
{
struct sja1105_private *priv = ds->priv;
- int rc;
-
- rc = devlink_info_driver_name_put(req, "sja1105");
- if (rc)
- return rc;

- rc = devlink_info_version_fixed_put(req,
- DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
- priv->info->name);
- return rc;
+ return devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+ priv->info->name);
}

int sja1105_devlink_setup(struct dsa_switch *ds)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a6f788f6294..26913dc816d3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -892,10 +892,6 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
u32 ver = 0;
int rc;

- rc = devlink_info_driver_name_put(req, DRV_MODULE_NAME);
- if (rc)
- return rc;
-
if (BNXT_PF(bp) && (bp->flags & BNXT_FLAG_DSN_VALID)) {
sprintf(buf, "%02X-%02X-%02X-%02X-%02X-%02X-%02X-%02X",
bp->dsn[7], bp->dsn[6], bp->dsn[5], bp->dsn[4],
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 5c6dd3029e2f..76f808d38066 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
@@ -37,18 +37,9 @@ static int dpaa2_eth_dl_info_get(struct devlink *devlink,
struct dpaa2_eth_devlink_priv *dl_priv = devlink_priv(devlink);
struct dpaa2_eth_priv *priv = dl_priv->dpaa2_priv;
char buf[10];
- int err;
-
- err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (err)
- return err;

scnprintf(buf, 10, "%d.%d", priv->dpni_ver_major, priv->dpni_ver_minor);
- err = devlink_info_version_running_put(req, "dpni", buf);
- if (err)
- return err;
-
- return 0;
+ return devlink_info_version_running_put(req, "dpni", buf);
}

static struct dpaa2_eth_trap_item *
diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index d50c222948b4..6668375edff6 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
@@ -6,7 +6,7 @@
static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
- return devlink_info_driver_name_put(req, KBUILD_MODNAME);
+ return 0;
}

static const struct devlink_ops fun_dl_ops = {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
index 4c441e6a5082..3d3b69605423 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_devlink.c
@@ -13,11 +13,6 @@ static int hclge_devlink_info_get(struct devlink *devlink,
struct hclge_devlink_priv *priv = devlink_priv(devlink);
char version_str[HCLGE_DEVLINK_FW_STRING_LEN];
struct hclge_dev *hdev = priv->hdev;
- int ret;
-
- ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (ret)
- return ret;

snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
index fdc19868b818..a6c3c5e8f0ab 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_devlink.c
@@ -13,11 +13,6 @@ static int hclgevf_devlink_info_get(struct devlink *devlink,
struct hclgevf_devlink_priv *priv = devlink_priv(devlink);
char version_str[HCLGEVF_DEVLINK_FW_STRING_LEN];
struct hclgevf_dev *hdev = priv->hdev;
- int ret;
-
- ret = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (ret)
- return ret;

snprintf(version_str, sizeof(version_str), "%lu.%lu.%lu.%lu",
hnae3_get_field(hdev->fw_version, HNAE3_FW_VERSION_BYTE3_MASK,
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 1d638216484d..ba74977e75dc 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -311,12 +311,6 @@ static int ice_devlink_info_get(struct devlink *devlink,
}
}

- err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (err) {
- NL_SET_ERR_MSG_MOD(extack, "Unable to set driver name");
- goto out_free_ctx;
- }
-
ice_info_get_dsn(pf, ctx);

err = devlink_info_serial_number_put(req, ctx->buf);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 88dee589cb21..f15439d26d21 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1550,7 +1550,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
- return devlink_info_driver_name_put(req, DRV_NAME);
+ return 0;
}

static const struct devlink_ops rvu_devlink_ops = {
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 777a27047c8e..5cc6416cf1a6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -77,18 +77,11 @@ static const struct devlink_param otx2_dl_params[] = {
otx2_dl_mcam_count_validate),
};

-/* Devlink OPs */
static int otx2_devlink_info_get(struct devlink *devlink,
struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
- struct otx2_devlink *otx2_dl = devlink_priv(devlink);
- struct otx2_nic *pfvf = otx2_dl->pfvf;
-
- if (is_otx2_vf(pfvf->pcifunc))
- return devlink_info_driver_name_put(req, "rvu_nicvf");
-
- return devlink_info_driver_name_put(req, "rvu_nicpf");
+ return 0;
}

static const struct devlink_ops otx2_devlink_ops = {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 84ad05c9f12d..2a4c9df4eb79 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -355,11 +355,6 @@ static int prestera_dl_info_get(struct devlink *dl,
{
struct prestera_switch *sw = devlink_priv(dl);
char buf[16];
- int err;
-
- err = devlink_info_driver_name_put(req, PRESTERA_DRV_NAME);
- if (err)
- return err;

snprintf(buf, sizeof(buf), "%d.%d.%d",
sw->dev->fw_rev.maj,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index cc2ae427dcb0..751bc4a9edcf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -46,10 +46,6 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
u32 running_fw, stored_fw;
int err;

- err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (err)
- return err;
-
err = devlink_info_version_fixed_put(req, "fw.psid", dev->board_id);
if (err)
return err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a83f6bc30072..a0a06e2eff82 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1459,11 +1459,6 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
char buf[32];
int err;

- err = devlink_info_driver_name_put(req,
- mlxsw_core->bus_info->device_kind);
- if (err)
- return err;
-
mlxsw_reg_mgir_pack(mgir_pl);
err = mlxsw_reg_query(mlxsw_core, MLXSW_REG(mgir), mgir_pl);
if (err)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 8bfd48d50ef0..4c601ff09cd3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -239,10 +239,6 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
char *buf = NULL;
int err;

- err = devlink_info_driver_name_put(req, "nfp");
- if (err)
- return err;
-
vendor = nfp_hwinfo_lookup(pf->hwinfo, "assembly.vendor");
part = nfp_hwinfo_lookup(pf->hwinfo, "assembly.partno");
sn = nfp_hwinfo_lookup(pf->hwinfo, "assembly.serial");
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 567f778433e2..e6ff757895ab 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -26,10 +26,6 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
char buf[16];
int err = 0;

- err = devlink_info_driver_name_put(req, IONIC_DRV_NAME);
- if (err)
- return err;
-
err = devlink_info_version_running_put(req,
DEVLINK_INFO_VERSION_GENERIC_FW,
idev->dev_info.fw_version);
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index 6bb4e165b592..922c47797af6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -162,10 +162,6 @@ static int qed_devlink_info_get(struct devlink *devlink,

dev_info = &cdev->common_dev_info;

- err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (err)
- return err;
-
memcpy(buf, cdev->hwfns[0].hw_info.part_num, sizeof(cdev->hwfns[0].hw_info.part_num));
buf[sizeof(cdev->hwfns[0].hw_info.part_num)] = 0;

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index e14686594a71..b962fc8e1397 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -994,9 +994,6 @@ static int nsim_dev_info_get(struct devlink *devlink,
{
int err;

- err = devlink_info_driver_name_put(req, DRV_NAME);
- if (err)
- return err;
err = devlink_info_version_stored_put_ext(req, "fw.mgmt", "10.20.30",
DEVLINK_INFO_VERSION_TYPE_COMPONENT);
if (err)
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 154d58cbd9ce..4bbaccd543ad 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -1647,10 +1647,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
char buf[32];
int err;

- err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
- if (err)
- return err;
-
fw_image = bp->fw_loader ? "loader" : "fw";
sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
err = devlink_info_version_running_put(req, fw_image, buf);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index cea154ddce7a..6478135d9ba1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6749,11 +6749,24 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req,
}
EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext);

+static int devlink_nl_driver_info_get(struct device_driver *drv,
+ struct devlink_info_req *req)
+{
+ if (!drv)
+ return 0;
+
+ if (drv->name[0])
+ return devlink_info_driver_name_put(req, drv->name);
+
+ return 0;
+}
+
static int
devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd, u32 portid,
u32 seq, int flags, struct netlink_ext_ack *extack)
{
+ struct device *dev = devlink_to_dev(devlink);
struct devlink_info_req req = {};
void *hdr;
int err;
@@ -6771,6 +6784,10 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
if (err)
goto err_cancel_msg;

+ err = devlink_nl_driver_info_get(dev->driver, &req);
+ if (err)
+ goto err_cancel_msg;
+
genlmsg_end(msg, hdr);
return 0;

--
2.25.1

2022-11-28 04:31:54

by Vincent MAILHOL

[permalink] [raw]
Subject: [PATCH net-next v4 2/3] net: devlink: remove devlink_info_driver_name_put()

Now that the core sets the driver name attribute, drivers are not
supposed to call devlink_info_driver_name_put() anymore. Remove it.

Signed-off-by: Vincent Mailhol <[email protected]>
---
include/net/devlink.h | 2 --
net/core/devlink.c | 11 ++---------
2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 074a79b8933f..52d5fb67e9b8 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1746,8 +1746,6 @@ int devlink_region_snapshot_create(struct devlink_region *region,
u8 *data, u32 snapshot_id);
int devlink_info_serial_number_put(struct devlink_info_req *req,
const char *sn);
-int devlink_info_driver_name_put(struct devlink_info_req *req,
- const char *name);
int devlink_info_board_serial_number_put(struct devlink_info_req *req,
const char *bsn);

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6478135d9ba1..3babc16eeb6b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6633,14 +6633,6 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
return err;
}

-int devlink_info_driver_name_put(struct devlink_info_req *req, const char *name)
-{
- if (!req->msg)
- return 0;
- return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME, name);
-}
-EXPORT_SYMBOL_GPL(devlink_info_driver_name_put);
-
int devlink_info_serial_number_put(struct devlink_info_req *req, const char *sn)
{
if (!req->msg)
@@ -6756,7 +6748,8 @@ static int devlink_nl_driver_info_get(struct device_driver *drv,
return 0;

if (drv->name[0])
- return devlink_info_driver_name_put(req, drv->name);
+ return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRIVER_NAME,
+ drv->name);

return 0;
}
--
2.25.1

2022-11-28 04:33:39

by Vincent MAILHOL

[permalink] [raw]
Subject: [PATCH net-next v4 3/3] net: devlink: make the devlink_ops::info_get() callback optional

Some drivers only reported the driver name in their
devlink_ops::info_get() callback. Now that the core provides this
information, the callback became empty. For such drivers, just
removing the callback would prevent the core from executing
devlink_nl_info_fill() meaning that "devlink dev info" would not
return anything.

Make the callback function optional by executing
devlink_nl_info_fill() even if devlink_ops::info_get() is NULL.

Remove all the empty devlink_ops::info_get() functions from the
drivers.

N.B.: the drivers with devlink support which previously did not
implement devlink_ops::info_get() will now also be able to report
the driver name.

Signed-off-by: Vincent Mailhol <[email protected]>
---
.../net/ethernet/fungible/funeth/funeth_devlink.c | 7 -------
.../net/ethernet/marvell/octeontx2/af/rvu_devlink.c | 7 -------
.../ethernet/marvell/octeontx2/nic/otx2_devlink.c | 8 --------
net/core/devlink.c | 13 ++++++-------
4 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
index 6668375edff6..4fbeb3fd71a8 100644
--- a/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
+++ b/drivers/net/ethernet/fungible/funeth/funeth_devlink.c
@@ -3,14 +3,7 @@
#include "funeth.h"
#include "funeth_devlink.h"

-static int fun_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
- struct netlink_ext_ack *extack)
-{
- return 0;
-}
-
static const struct devlink_ops fun_dl_ops = {
- .info_get = fun_dl_info_get,
};

struct devlink *fun_devlink_alloc(struct device *dev)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index f15439d26d21..bda1a6fa2ec4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1547,14 +1547,7 @@ static int rvu_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
return 0;
}

-static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
- struct netlink_ext_ack *extack)
-{
- return 0;
-}
-
static const struct devlink_ops rvu_devlink_ops = {
- .info_get = rvu_devlink_info_get,
.eswitch_mode_get = rvu_devlink_eswitch_mode_get,
.eswitch_mode_set = rvu_devlink_eswitch_mode_set,
};
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 5cc6416cf1a6..63ef7c41d18d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -77,15 +77,7 @@ static const struct devlink_param otx2_dl_params[] = {
otx2_dl_mcam_count_validate),
};

-static int otx2_devlink_info_get(struct devlink *devlink,
- struct devlink_info_req *req,
- struct netlink_ext_ack *extack)
-{
- return 0;
-}
-
static const struct devlink_ops otx2_devlink_ops = {
- .info_get = otx2_devlink_info_get,
};

int otx2_register_dl(struct otx2_nic *pfvf)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3babc16eeb6b..817d978bb729 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6773,9 +6773,11 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
goto err_cancel_msg;

req.msg = msg;
- err = devlink->ops->info_get(devlink, &req, extack);
- if (err)
- goto err_cancel_msg;
+ if (devlink->ops->info_get) {
+ err = devlink->ops->info_get(devlink, &req, extack);
+ if (err)
+ goto err_cancel_msg;
+ }

err = devlink_nl_driver_info_get(dev->driver, &req);
if (err)
@@ -6796,9 +6798,6 @@ static int devlink_nl_cmd_info_get_doit(struct sk_buff *skb,
struct sk_buff *msg;
int err;

- if (!devlink->ops->info_get)
- return -EOPNOTSUPP;
-
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -6824,7 +6823,7 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
int err = 0;

devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
- if (idx < start || !devlink->ops->info_get)
+ if (idx < start)
goto inc;

devl_lock(devlink);
--
2.25.1

2022-11-28 12:02:24

by Ido Schimmel

[permalink] [raw]
Subject: Re: [PATCH net-next v4 1/3] net: devlink: let the core report the driver name instead of the drivers

On Mon, Nov 28, 2022 at 01:15:43PM +0900, Vincent Mailhol wrote:
> The driver name is available in device_driver::name. Right now,
> drivers still have to report this piece of information themselves in
> their devlink_ops::info_get callback function.
>
> In order to factorize code, make devlink_nl_info_fill() add the driver
> name attribute.
>
> nla_put() does not check if an attribute already exists and
> unconditionally reserves new space, c.f. [1]. To avoid attribute
> duplication, clean-up all the drivers which are currently reporting
> the driver name in their callback.
>
> [1] __nla_put() from lib/nlattr.c
> Link: https://elixir.bootlin.com/linux/v6.0/source/lib/nlattr.c#L993
>
> Signed-off-by: Vincent Mailhol <[email protected]>

For mlxsw:

Tested-by: Ido Schimmel <[email protected]>

I don't see a difference in "devlink dev info" output before and after
the patchset. Tested with mlxsw_spectrum and mlxsw_minimal.

2022-11-28 18:48:30

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v4 3/3] net: devlink: make the devlink_ops::info_get() callback optional

On Mon, 28 Nov 2022 13:15:45 +0900 Vincent Mailhol wrote:
> Some drivers only reported the driver name in their
> devlink_ops::info_get() callback. Now that the core provides this
> information, the callback became empty. For such drivers, just
> removing the callback would prevent the core from executing
> devlink_nl_info_fill() meaning that "devlink dev info" would not
> return anything.
>
> Make the callback function optional by executing
> devlink_nl_info_fill() even if devlink_ops::info_get() is NULL.
>
> Remove all the empty devlink_ops::info_get() functions from the
> drivers.
>
> N.B.: the drivers with devlink support which previously did not
> implement devlink_ops::info_get() will now also be able to report
> the driver name.

These should be two separate patches, tho.
Please don't post v5 in-reply-to.

Otherwise looks good. Kinda risky but good :)