[PATCH net-next 0/2] devlink enhancements

[PATCH net-next 0/2] devlink enhancements

[Kalesh A P]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

This patchset adds device capability reporting to devlink info API.
It may be useful if we expose the device capabilities to the user
through devlink info API.

Kalesh AP (2):
  devlink: add device capability reporting to devlink info API
  bnxt_en: Add device capabilities to devlink info_get cb

 Documentation/networking/devlink/devlink-info.rst |  3 +++
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 31 +++++++++++++++++++++--
 include/net/devlink.h                             |  2 ++
 include/uapi/linux/devlink.h                      |  3 +++
 net/core/devlink.c                                | 25 ++++++++++++++++++
 5 files changed, 62 insertions(+), 2 deletions(-)

-- 
2.10.1

[PATCH net-next 1/2] devlink: add device capability reporting to devlink info API

[Kalesh A P]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

It may be useful if we expose the device capabilities
to the user through devlink info API.
Add a new devlink API to allow retrieving device capabilities.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 Documentation/networking/devlink/devlink-info.rst |  3 +++
 include/net/devlink.h                             |  2 ++
 include/uapi/linux/devlink.h                      |  3 +++
 net/core/devlink.c                                | 25 +++++++++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/Documentation/networking/devlink/devlink-info.rst b/Documentation/networking/devlink/devlink-info.rst
index 7572bf6..b9b32dc 100644
--- a/Documentation/networking/devlink/devlink-info.rst
+++ b/Documentation/networking/devlink/devlink-info.rst
@@ -78,6 +78,9 @@ versions is generally discouraged - here, and via any other Linux API.
        ``stored`` versions when new software is flashed, it must not report
        them.
 
+   * - ``capabilities``
+     - Group for device capabilities.
+
 Each version can be reported at most once in each version group. Firmware
 components stored on the flash should feature in both the ``running`` and
 ``stored`` sections, if device is capable of reporting ``stored`` versions
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 08f4c61..e06d781 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1687,6 +1687,8 @@ int devlink_info_version_stored_put(struct devlink_info_req *req,
 int devlink_info_version_running_put(struct devlink_info_req *req,
 				     const char *version_name,
 				     const char *version_value);
+int devlink_info_device_capability_put(struct devlink_info_req *req,
+				       const char *capability_name);
 
 int devlink_fmsg_obj_nest_start(struct devlink_fmsg *fmsg);
 int devlink_fmsg_obj_nest_end(struct devlink_fmsg *fmsg);
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 32f53a00..f61a59ae 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -551,6 +551,9 @@ enum devlink_attr {
 	DEVLINK_ATTR_RATE_NODE_NAME,		/* string */
 	DEVLINK_ATTR_RATE_PARENT_NODE_NAME,	/* string */
 
+	DEVLINK_ATTR_INFO_DEVICE_CAPABILITY_LIST,	/* nested */
+	DEVLINK_ATTR_INFO_DEVICE_CAPABILITY_NAME,	/* string */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 8fa0153..4f1ce03 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5850,6 +5850,31 @@ int devlink_info_version_running_put(struct devlink_info_req *req,
 }
 EXPORT_SYMBOL_GPL(devlink_info_version_running_put);
 
+int devlink_info_device_capability_put(struct devlink_info_req *req,
+				       const char *capability_name)
+{
+	struct nlattr *nest;
+	int err;
+
+	nest = nla_nest_start(req->msg, DEVLINK_ATTR_INFO_DEVICE_CAPABILITY_LIST);
+	if (!nest)
+		return -EMSGSIZE;
+
+	err = nla_put_string(req->msg, DEVLINK_ATTR_INFO_DEVICE_CAPABILITY_NAME,
+			     capability_name);
+	if (err)
+		goto nla_put_failure;
+
+	nla_nest_end(req->msg, nest);
+
+	return 0;
+
+nla_put_failure:
+	nla_nest_cancel(req->msg, nest);
+	return err;
+}
+EXPORT_SYMBOL_GPL(devlink_info_device_capability_put);
+
 static int
 devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
 		     enum devlink_command cmd, u32 portid,
-- 
2.10.1

[PATCH net-next 2/2] bnxt_en: Add device capabilities to devlink info_get cb

[Kalesh A P]

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

HWRM_VER_GET and FUNC_QCAPS command response provides the support
to indicate various device capabilities.

Expose these details via the devlink info.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 31 +++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 64381be..0ae5f47 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -379,6 +379,25 @@ static int bnxt_hwrm_get_nvm_cfg_ver(struct bnxt *bp,
 	return rc;
 }
 
+static int bnxt_dl_dev_capability_info_put(struct bnxt *bp, struct devlink_info_req *req)
+{
+	int rc;
+
+	if (bp->fw_cap & BNXT_FW_CAP_HOT_RESET) {
+		rc = devlink_info_device_capability_put(req, "hot_reset");
+		if (rc)
+			return rc;
+	}
+
+	if (bp->fw_cap & BNXT_FW_CAP_ERROR_RECOVERY) {
+		rc = devlink_info_device_capability_put(req, "error_recovery");
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 static int bnxt_dl_info_put(struct bnxt *bp, struct devlink_info_req *req,
 			    enum bnxt_dl_version_type type, const char *key,
 			    char *buf)
@@ -557,8 +576,16 @@ static int bnxt_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	snprintf(roce_ver, FW_VER_STR_LEN, "%d.%d.%d.%d",
 		 nvm_dev_info.roce_fw_major, nvm_dev_info.roce_fw_minor,
 		 nvm_dev_info.roce_fw_build, nvm_dev_info.roce_fw_patch);
-	return bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
-				DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
+	rc = bnxt_dl_info_put(bp, req, BNXT_VERSION_STORED,
+			      DEVLINK_INFO_VERSION_GENERIC_FW_ROCE, roce_ver);
+	if (rc)
+		return rc;
+
+	rc = bnxt_dl_dev_capability_info_put(bp, req);
+	if (rc)
+		return rc;
+
+	return 0;
 }
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
-- 
2.10.1

Re: [PATCH net-next 0/2] devlink enhancements

[Jakub Kicinski]

On Mon,  2 Aug 2021 09:57:38 +0530 Kalesh A P wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> This patchset adds device capability reporting to devlink info API.
> It may be useful if we expose the device capabilities to the user
> through devlink info API.

Did you see the RFC Jake posted? That's way more palatable.

Operationally the API provided here is of little to no value 
to the user, and since it extends the "let the vendors dump custom
meaningless strings" paradigm present in devlink please expect 
major push back.

Re: [PATCH net-next 0/2] devlink enhancements

[Keller, Jacob E]

On 8/2/2021 9:36 AM, Jakub Kicinski wrote:
> On Mon,  2 Aug 2021 09:57:38 +0530 Kalesh A P wrote:
>> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>>
>> This patchset adds device capability reporting to devlink info API.
>> It may be useful if we expose the device capabilities to the user
>> through devlink info API.
> 
> Did you see the RFC Jake posted? That's way more palatable.
> 

FWIW, my patch is more in regards to making sure that users, tools, or
scripts, have a way to tell if a given devlink interface is supported.

This seems like a way to indicate specific device features?

> Operationally the API provided here is of little to no value 
> to the user, and since it extends the "let the vendors dump custom
> meaningless strings" paradigm present in devlink please expect 
> major push back.
> 

Right. the better approach here is to ensure that whatever user-facing
impact of these features is exposed through a standard interface. If one
doesn't exist for the capability, you will need to do work to create
such an interface.

If there's no user impact (and thus no need for a separate interface),
then why does a user need to know the capability?

Thanks,
Jake