Re: [PATCH net-next v5 06/10] bnxt: Add devlink support for config get/set
On Mon, Oct 30, 2017 at 1:35 PM, Jiri Pirko wrote:
> Mon, Oct 30, 2017 at 03:46:12PM CET, steven.l...@broadcom.com wrote:
>>Implements get and set of configuration parameters using new devlink
>>config get/set API. Parameters themselves defined in later patches.
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 258
>> +-
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
>> 2 files changed, 263 insertions(+), 12 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>>b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>>index 402fa32..deb24e0 100644
>
> [...]
>
>>+static int bnxt_dl_perm_config_set(struct devlink *devlink,
>>+ enum devlink_perm_config_param param,
>>+ enum devlink_perm_config_type type,
>>+ union devlink_perm_config_value *value,
>>+ bool *restart_reqd)
>>+{
>>+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
>>+ struct bnxt_drv_cfgparam *entry = NULL;
>>+ u32 value_int;
>>+ u32 bytesize;
>>+ int idx = 0;
>>+ int ret = 0;
>>+ u16 val16;
>>+ u8 val8;
>>+ int i;
>>+
>>+ *restart_reqd = 0;
>>+
>>+ /* Find parameter in table */
>>+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
>>+ if (param == bnxt_drv_cfgparam_list[i].param) {
>>+ entry = &bnxt_drv_cfgparam_list[i];
>>+ break;
>>+ }
>>+ }
>>+
>>+ /* Not found */
>>+ if (!entry)
>>+ return -EINVAL;
>>+
>>+ /* Check to see if this func type can access variable */
>>+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
>>+ return -EOPNOTSUPP;
>>+ if (BNXT_VF(bp) && !(entry->func & BNXT_DRV_VF))
>>+ return -EOPNOTSUPP;
>>+
>>+ /* If parameter is per port or function, compute index */
>>+ if (entry->appl == BNXT_DRV_APPL_PORT) {
>>+ idx = bp->pf.port_id;
>>+ } else if (entry->appl == BNXT_DRV_APPL_FUNCTION) {
>>+ if (BNXT_PF(bp))
>>+ idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
>> }
>>
>>+ bytesize = roundup(entry->bitlength, BITS_PER_BYTE) / BITS_PER_BYTE;
>>+
>>+ /* Type expected by device may or may not be the same as type passed
>>+ * in from devlink API. So first convert to an interval u32 value,
>>+ * then to type needed by device.
>>+ */
>>+ if (type == DEVLINK_PERM_CONFIG_TYPE_U8) {
>>+ value_int = value->value8;
>>+ } else if (type == DEVLINK_PERM_CONFIG_TYPE_U16) {
>>+ value_int = value->value16;
>>+ } else if (type == DEVLINK_PERM_CONFIG_TYPE_U32) {
>>+ value_int = value->value32;
>>+ } else {
>>+ /* Unsupported type */
>>+ return -EINVAL;
>>+ }
>>+
>>+ if (bytesize == 1) {
>>+ val8 = value_int;
>
>>+ ret = bnxt_nvm_write(bp, entry->nvm_param, idx, &val8,
>>+ entry->bitlength);
>>+ } else if (bytesize == 2) {
>>+ val16 = value_int;
>>+ ret = bnxt_nvm_write(bp, entry->nvm_param, idx, &val16,
>>+ entry->bitlength);
>>+ } else {
>>+ ret = bnxt_nvm_write(bp, entry->nvm_param, idx, &value_int,
>>+ entry->bitlength);
>>+ }
>>+
>>+ /* Restart required for all nvm parameter writes */
>>+ *restart_reqd = 1;
>>+
>>+ return ret;
>>+}
>
> I don't like this swissknife approach for setters and getters. It's
> messy, and hard to read. There should be clean get/set pair function
> per parameter defined in array.
The advantage of the swiss kinfe approach is that you don't have a
large number of almost-identical functions (i.e. any "set" that
operates on a u32 will be doing essentially the exact same thing) in
the driver, which could lead to code bloat and also make it harder to
catch errors (since any bug that may creep in during the copy/paste to
make so many functions will only be caught when that specific
parameter
Re: [PATCH net-next v5 02/10] devlink: Adding SR-IOV enablement perm config param
On Mon, Oct 30, 2017 at 1:20 PM, Jiri Pirko wrote:
> Mon, Oct 30, 2017 at 03:46:08PM CET, steven.l...@broadcom.com wrote:
>>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>>parameter; this parameter controls whether the device
>>advertises the SR-IOV PCI capability. Value is permanent, so
>>becomes the new default value for this device.
>>
>> DEVLINK_PERM_CONFIG_DISABLE = Disable SR-IOV
>> DEVLINK_PERM_CONFIG_ENABLE = Enable SR-IOV
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/uapi/linux/devlink.h | 18 +-
>> net/core/devlink.c | 1 +
>> 2 files changed, 18 insertions(+), 1 deletion(-)
>>
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index 55e35f2..1c5d4ae 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -256,8 +256,24 @@ enum devlink_dpipe_header_id {
>> DEVLINK_DPIPE_HEADER_IPV6,
>> };
>>
>>-/* Permanent config parameters */
>>+/* Permanent config parameter enable/disable
>>+ * DEVLINK_PERM_CONFIG_DISABLE = disable a permanent config parameter
>>+ * DEVLINK_PERM_CONFIG_ENBALE = enable a permanent config parameter
>>+ */
>>+enum devlink_perm_config_enabled {
>>+ DEVLINK_PERM_CONFIG_DISABLE,
>>+ DEVLINK_PERM_CONFIG_ENABLE,
>>+};
>
>
> Basically, this is "bool"
>
> Why don't you use some own enum instead of NLA_U*. Like team does for
> example:
>
> enum team_option_type {
> TEAM_OPTION_TYPE_U32,
> TEAM_OPTION_TYPE_STRING,
> TEAM_OPTION_TYPE_BINARY,
> TEAM_OPTION_TYPE_BOOL,
> TEAM_OPTION_TYPE_S32,
> };
>
>
I had added enum devlink_perm_config_type in v5 based on your comments
in v4 - I will add BOOL if it helps clarity.
>
>>+
>>+/* Permanent config parameters:
>>+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED: Configures whether SR-IOV PCI
>>capability
>>+ * is advertised by the device.
>>+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
>>+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
>>+ */
>> enum devlink_perm_config_param {
>>+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>
>
> Please align "ENABLED" vs "ENABLE".
> The rest of devlink code uses "ENABLED"
>
Ok.
>
>>+
>> __DEVLINK_PERM_CONFIG_MAX,
>> DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
>> };
>>diff --git a/net/core/devlink.c b/net/core/devlink.c
>>index 5e77408..b4d43c29 100644
>>--- a/net/core/devlink.c
>>+++ b/net/core/devlink.c
>>@@ -1569,6 +1569,7 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
>>sk_buff *skb,
>> static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
>>
>> static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
>>+ [DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
>> };
>>
>> static int devlink_nl_single_param_get(struct sk_buff *msg,
>>--
>>2.7.4
>>
Re: [PATCH net-next v5 01/10] devlink: Add permanent config parameter get/set operations
On Mon, Oct 30, 2017 at 1:03 PM, Jiri Pirko wrote:
> Mon, Oct 30, 2017 at 03:46:07PM CET, steven.l...@broadcom.com wrote:
>>Add support for permanent config parameter get/set commands. Used
>>for persistent device configuration parameters.
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>
> In general, I don't like how you use netlink. Please see the rest of
> this code. We should do things in the common way. More info inlined.
>
>
> [...]
>
>>
>>+/* Permanent config parameters */
>>+enum devlink_perm_config_param {
>>+ __DEVLINK_PERM_CONFIG_MAX,
>>+ DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
>>+};
>>+
>>+/* Permanent config types */
>>+enum devlink_perm_config_type {
>>+ DEVLINK_PERM_CONFIG_TYPE_UNSPEC,
>
> You should not need this. Type should be always specified.
This was an attempt to align enum devlink_perm_config_type with the
NLA_* enums in netlink.h, which include the unspecified type. I will
remove the unspecified enum, but it seems like having multiple type
enums that are not aligned with each other could be problematic.
>
>
>>+ DEVLINK_PERM_CONFIG_TYPE_U8,
>>+ DEVLINK_PERM_CONFIG_TYPE_U16,
>>+ DEVLINK_PERM_CONFIG_TYPE_U32,
>>+};
>
> This definitelly should not be in uapi header.
>
>
>>+
>>+/* Permanent config value */
>>+union devlink_perm_config_value {
>>+ __u8value8;
>>+ __u16 value16;
>>+ __u32 value32;
>>+};
>
> This definitelly should not be in uapi header.
Ok, will move these.
>
>
>
>>+
>> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>
> [...]
>
>
>>+static int devlink_nl_config_get_fill(struct sk_buff *msg,
>>+struct devlink *devlink,
>>+enum devlink_command cmd,
>>+struct genl_info *info)
>>+{
>>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>>+ enum devlink_perm_config_param param;
>>+ enum devlink_perm_config_type type;
>>+ struct nlattr *cfgparam_attr;
>>+ struct nlattr *attr;
>>+ void *hdr;
>>+ int err;
>>+ int rem;
>>+
>>+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
>>+&devlink_nl_family, 0, cmd);
>>+ if (!hdr)
>>+ return -EMSGSIZE;
>>+
>>+ err = devlink_nl_put_handle(msg, devlink);
>>+ if (err)
>>+ goto nla_put_failure;
>>+
>>+ if (!info->attrs[DEVLINK_ATTR_PERM_CONFIGS]) {
>>+ err = -EINVAL;
>>+ goto nla_put_failure;
>>+ }
>>+
>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
>>+
>>+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS],
>
>
> Okay. So I have to know what is in kernel in order to get the value.
>
> We need a dump operation. In fact, why can't we just have dump operation?
Ok, I can add a dump operation.
>
>
>
>>+ rem) {
>>+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
>>+ devlink_nl_policy, NULL);
>>+ if (err)
>>+ goto nla_nest_failure;
>>+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
>>+ !tb[DEVLINK_ATTR_PERM_CONFIG_TYPE])
>>+ continue;
>>+
>>+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
>>+ type = nla_get_u8(tb[DEVLINK_ATTR_PERM_CONFIG_TYPE]);
>
> So to get parameter, I have to specify a type? What if I specify wrong
> type? That is wrong. The type should be read attr for get.
For set, seems like the user would need to know and set the type. So,
I was just doing the same for get(). But I can make the type a read
attribute for get.
>
> You should have a param -> type table and always use that.
>
>
>>+ if (param > DEVLINK_PERM_CONFIG_MAX ||
>>+ type > NLA_TYPE_MAX) {
>>+ continue;
>>+ }
>>+ /* Note if single_param_get fails, that param won't be in
>>+ * response msg, so caller will know which param(s) failed to
>>+ * get.
>>+ */
>>+ devlink_nl_single_param_get(msg, devlink, param, type);
>>+ }
>>+
>>+ nla_nest_end(msg, cfgparam_attr);
>>+
>>+ genlmsg_end(msg, hdr);
>>+ return
[PATCH net-next v5 06/10] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API. Parameters themselves defined in later patches.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 258 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
2 files changed, 263 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 402fa32..deb24e0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,26 +14,260 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
-#ifdef CONFIG_BNXT_SRIOV
- .eswitch_mode_set = bnxt_dl_eswitch_mode_set,
- .eswitch_mode_get = bnxt_dl_eswitch_mode_get,
-#endif /* CONFIG_BNXT_SRIOV */
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
};
-int bnxt_dl_register(struct bnxt *bp)
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
{
- struct devlink *dl;
+ struct hwrm_nvm_get_variable_input req = {0};
+ dma_addr_t dest_data_dma_addr;
+ void *dest_data_addr = NULL;
+ int bytesize;
int rc;
- if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
- return 0;
+ bytesize = roundup(size, BITS_PER_BYTE) / BITS_PER_BYTE;
+ dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &dest_data_dma_addr, GFP_KERNEL);
+ if (!dest_data_addr)
+ return -ENOMEM;
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
+ req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ memcpy(buf, dest_data_addr, bytesize);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, dest_data_addr,
+ dest_data_dma_addr);
+
+ return rc;
+}
+
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
+ const void *buf, int size)
+{
+ struct hwrm_nvm_set_variable_input req = {0};
+ dma_addr_t src_data_dma_addr;
+ void *src_data_addr = NULL;
+ int bytesize;
+ int rc;
+
+ bytesize = roundup(size, BITS_PER_BYTE) / BITS_PER_BYTE;
+
+ src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &src_data_dma_addr, GFP_KERNEL);
+ if (!src_data_addr)
+ return -ENOMEM;
+
+ memcpy(src_data_addr, buf, bytesize);
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+ req.src_data_addr = cpu_to_le64(src_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, src_data_addr,
+ src_data_dma_addr);
- if (bp->hwrm_spec_code < 0x10803) {
- netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch
SWITCHDEV mode.\n");
- return -ENOTSUPP;
+ return 0;
+}
+
+static int bnxt_dl_perm_config_set(struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ enum devlink_perm_config_type type,
+ union devlink_perm_config_value *value,
+ bool *restart_reqd)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ struct bnxt_drv_cfgparam *entry = NULL;
+ u32 value_int;
+ u32 bytesize;
+ int idx = 0;
+ int ret = 0;
+ u16 val16;
+ u8 val8;
+ int i;
+
+ *restart_reqd = 0;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (!entry)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+
[PATCH net-next v5 10/10] bnxt: Adding num MSI-X vectors per VF perm config param
Adding permanent config parameter for number MSI-X vectors
per VF, using devlink API for get/set operation.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index ec0f57f..0ea8d95 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -22,6 +22,8 @@
* # of VFs per PF in SR-IOV mode
* DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
* Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF:
+ * # of MSI-X vectors per VF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
@@ -30,6 +32,8 @@ struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
BNXT_DRV_APPL_FUNCTION, 8, 404},
{DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v5 05/10] devlink: Adding num MSI-X vectors per VF perm config param
Adding DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value defines number of MSI-X vectors allocated per VF.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6936c0e..fe56a02 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -276,11 +276,15 @@ enum devlink_perm_config_enabled {
* DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT: Configures the # of MSI-X vectors
* advertised via PCI capability for this device.
* Value = Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF: Configures the # of MSI-X vectors
+ * advertised via PCI capability for each VF on this device.
+ * # of MSI-X vectors per VF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT,
+ DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 78bb949..f1f16c2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1572,6 +1572,7 @@ static const u8
devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
[DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v5 07/10] bnxt: Adding SR-IOV enablement permanent cfg param
Adding permanent config parameter for SR-IOV enablement, using
devlink API for get/set operation.
DEVLINK_PERM_CONFIG_DISABLE = SR-IOV disabled
DEVLINK_PERM_CONFIG_ENABLE = SR-IOV enabled
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index deb24e0..a2a4973 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,7 +14,14 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
+/* Permanent config parameters from devlink.h:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ */
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v5 09/10] bnxt: Adding max PF MSI-X vectors perm config param
Adding permanent config parameter for maximum number of PF
MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 218d18d..ec0f57f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -20,12 +20,16 @@
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
* # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
+ * Max # of MSI-X vectors per PF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
{DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v5 01/10] devlink: Add permanent config parameter get/set operations
Add support for permanent config parameter get/set commands. Used
for persistent device configuration parameters.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 9 ++
include/uapi/linux/devlink.h | 33 +
net/core/devlink.c | 293 +++
3 files changed, 335 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..437b6ab 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,15 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*perm_config_get)(struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ enum devlink_perm_config_type type,
+ union devlink_perm_config_value *value);
+ int (*perm_config_set)(struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ enum devlink_perm_config_type type,
+ union devlink_perm_config_value *value,
+ bool *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..55e35f2 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,10 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ /* Permanent device config get/set */
+ DEVLINK_CMD_PERM_CONFIG_GET,
+ DEVLINK_CMD_PERM_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +206,14 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CONFIGS, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_TYPE, /* u8 */
+ DEVLINK_ATTR_PERM_CONFIG_VALUE, /* dynamic */
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* flag */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
@@ -244,4 +256,25 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
+/* Permanent config parameters */
+enum devlink_perm_config_param {
+ __DEVLINK_PERM_CONFIG_MAX,
+ DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
+};
+
+/* Permanent config types */
+enum devlink_perm_config_type {
+ DEVLINK_PERM_CONFIG_TYPE_UNSPEC,
+ DEVLINK_PERM_CONFIG_TYPE_U8,
+ DEVLINK_PERM_CONFIG_TYPE_U16,
+ DEVLINK_PERM_CONFIG_TYPE_U32,
+};
+
+/* Permanent config value */
+union devlink_perm_config_value {
+ __u8value8;
+ __u16 value16;
+ __u32 value32;
+};
+
#endif /* _UAPI_LINUX_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7d430c1..5e77408 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,283 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+};
+
+static int devlink_nl_single_param_get(struct sk_buff *msg,
+ struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ enum devlink_perm_config_type type)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ union devlink_perm_config_value value;
+ struct nlattr *param_attr;
+ int err;
+
+ err = ops->perm_config_get(devlink, param, type, &value);
+ if (err)
+ return err;
+
+ param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ if (!param_attr)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param) ||
+ nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_TYPE, type))
+ goto nest_err;
+
+ switch (type) {
+ case DEVLINK_PERM_CONFIG_TYPE_U8:
+ if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
+ value.value8))
+ goto nest_err;
+ break;
+ case DEVLINK_PERM_CONFIG_TYPE_U16:
+ if (nla_put_u16(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
+ val
[PATCH net-next v5 08/10] bnxt: Adding num VFs per PF perm config param
Adding permanent config parameter for number of VFs per PF,
using devlink API for get/set operation.
Value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index a2a4973..218d18d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -18,10 +18,14 @@
* DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
* DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
+ * # of VFs per PF in SR-IOV mode
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
+ {DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v5 00/10] Adding permanent config get/set to devlink
Changes since v4: * Created / used enum for permanent config param / types. * Use bool and NLA_FLAG for restart_required indication. * Move and reworded comments and git commit messagess for clarity and removed double-spaces after sentences. -- Adds a devlink command for getting & setting permanent / persistent device configuration parameters, and enumerates the parameters as nested devlink attributes. Steve Lin (10): devlink: Add permanent config parameter get/set operations devlink: Adding SR-IOV enablement perm config param devlink: Adding num VFs per PF permanent config param devlink: Adding max PF MSI-X vectors perm config param devlink: Adding num MSI-X vectors per VF perm config param bnxt: Add devlink support for config get/set bnxt: Adding SR-IOV enablement permanent cfg param bnxt: Adding num VFs per PF perm config param bnxt: Adding max PF MSI-X vectors perm config param bnxt: Adding num MSI-X vectors per VF perm config param drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 277 +++- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++ include/net/devlink.h | 9 + include/uapi/linux/devlink.h | 61 + net/core/devlink.c| 297 ++ 5 files changed, 649 insertions(+), 12 deletions(-) -- 2.7.4
[PATCH net-next v5 03/10] devlink: Adding num VFs per PF permanent config param
Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
parameter. Value is permanent, so becomes the new default
value for this device.
The value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 1c5d4ae..4e0b8b7 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -270,9 +270,13 @@ enum devlink_perm_config_enabled {
* is advertised by the device.
* DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF: Configures the number of total/maximum
+ * VFs per PF available on device.
+ * Value: # of VFs per PF in SR-IOV mode
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+ DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index b4d43c29..f421cfb 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1570,6 +1570,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
+ [DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v5 04/10] devlink: Adding max PF MSI-X vectors perm config param
Adding DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value sets the maximum number of PF MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 4e0b8b7..6936c0e 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -273,10 +273,14 @@ enum devlink_perm_config_enabled {
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF: Configures the number of total/maximum
* VFs per PF available on device.
* Value: # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT: Configures the # of MSI-X vectors
+ * advertised via PCI capability for this device.
+ * Value = Max # of MSI-X vectors per PF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
+ DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f421cfb..78bb949 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1571,6 +1571,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v5 02/10] devlink: Adding SR-IOV enablement perm config param
Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
parameter; this parameter controls whether the device
advertises the SR-IOV PCI capability. Value is permanent, so
becomes the new default value for this device.
DEVLINK_PERM_CONFIG_DISABLE = Disable SR-IOV
DEVLINK_PERM_CONFIG_ENABLE = Enable SR-IOV
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 18 +-
net/core/devlink.c | 1 +
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 55e35f2..1c5d4ae 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -256,8 +256,24 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
-/* Permanent config parameters */
+/* Permanent config parameter enable/disable
+ * DEVLINK_PERM_CONFIG_DISABLE = disable a permanent config parameter
+ * DEVLINK_PERM_CONFIG_ENBALE = enable a permanent config parameter
+ */
+enum devlink_perm_config_enabled {
+ DEVLINK_PERM_CONFIG_DISABLE,
+ DEVLINK_PERM_CONFIG_ENABLE,
+};
+
+/* Permanent config parameters:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED: Configures whether SR-IOV PCI capability
+ * is advertised by the device.
+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ */
enum devlink_perm_config_param {
+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
};
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 5e77408..b4d43c29 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1569,6 +1569,7 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff
*skb,
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+ [DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
Re: [PATCH net-next v4 02/10] devlink: Adding SR-IOV enablement perm config param
On Fri, Oct 27, 2017 at 5:06 PM, Jiri Pirko wrote:
> Fri, Oct 27, 2017 at 10:54:06PM CEST, steven.l...@broadcom.com wrote:
>>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>>parameter. Value is permanent, so becomes the new default
>
> Avoid the double space.
Ok.
>
>
>>value for this device.
>>
>> DEVLINK_PERM_CONFIG_DISABLE = Disable SR-IOV
>> DEVLINK_PERM_CONFIG_ENABLE = Enable SR-IOV
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/uapi/linux/devlink.h | 14 +-
>> net/core/devlink.c | 1 +
>> 2 files changed, 14 insertions(+), 1 deletion(-)
>>
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index b3a0b2a..9a41f6e 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -256,8 +256,20 @@ enum devlink_dpipe_header_id {
>> DEVLINK_DPIPE_HEADER_IPV6,
>> };
>>
>>-/* Permanent config parameters */
>>+enum devlink_perm_config_enabled {
>>+ DEVLINK_PERM_CONFIG_DISABLE,
>>+ DEVLINK_PERM_CONFIG_ENABLE,
>>+};
>>+
>>+/* Permanent config parameters:
>>+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED: Configures whether SR-IOV PCI
>>capability
>>+ * provided by device.
>
> I don't understand the sentense :/
It means that this parameter controls whether the device advertises
SR-IOV PCI capability -- perhaps I should say "advertised" rather than
"provided"? Ok, will do that...
>
>
>>+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
>>+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
>
> These comments should be at the enum values, not here.
Ok.
>
>
>>+ */
>> enum devlink_perm_config_param {
>>+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>>+
>> __DEVLINK_PERM_CONFIG_MAX,
>> DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
>> };
>>diff --git a/net/core/devlink.c b/net/core/devlink.c
>>index a7fa7cc..395c93c 100644
>>--- a/net/core/devlink.c
>>+++ b/net/core/devlink.c
>>@@ -1569,6 +1569,7 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
>>sk_buff *skb,
>> static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
>>
>> static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
>>+ [DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
>> };
>>
>> static int devlink_nl_single_param_get(struct sk_buff *msg,
>>--
>>2.7.4
>>
Re: [PATCH net-next v4 01/10] devlink: Add permanent config parameter get/set operations
On Fri, Oct 27, 2017 at 5:04 PM, Jiri Pirko wrote:
> Fri, Oct 27, 2017 at 10:54:05PM CEST, steven.l...@broadcom.com wrote:
>>Add support for permanent config parameter get/set commands. Used
>>for persistent device configuration parameters.
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/net/devlink.h| 7 ++
>> include/uapi/linux/devlink.h | 25
>> net/core/devlink.c | 287
>> +++
>> 3 files changed, 319 insertions(+)
>>
>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>index b9654e1..c7dd912 100644
>>--- a/include/net/devlink.h
>>+++ b/include/net/devlink.h
>>@@ -270,6 +270,13 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>+ int (*perm_config_get)(struct devlink *devlink,
>>+ enum devlink_perm_config_param param, u8 type,
>>+ union devlink_perm_config_value *value);
>>+ int (*perm_config_set)(struct devlink *devlink,
>>+ enum devlink_perm_config_param param, u8 type,
>>+ union devlink_perm_config_value *value,
>>+ u8 *restart_reqd);
>
> type should be enum and restart_reqd should be bool.
Throughout devlink/netlink code, NLA_* types are not a named enum;
they're defined like this in netlink.h:
enum {
NLA_UNSPEC,
NLA_U8,
NLA_U16,
NLA_U32,
additional types follow
};
and are, I believe referred to as u8 or u16 in the code. I could
define a new enum specifically for the config get/set types, but that
seems duplicative with the NLA_* types already defined - is that what
you're suggesting?
I will change restart_reqd to be bool.
>
> [...]
>
>
>>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>>+ struct devlink *devlink,
>>+ u32 param, u8 type,
>>+ union devlink_perm_config_value *value)
>>+{
>>+ const struct devlink_ops *ops = devlink->ops;
>>+ struct nlattr *cfgparam_attr;
>>+ u8 need_restart;
>>+ int err;
>>+
>>+ /* Now set parameter */
>>+ err = ops->perm_config_set(devlink, param, type, value, &need_restart);
>>+ if (err)
>>+ return err;
>>+
>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>>+ /* Update restart reqd - if any param needs restart, should be set */
>>+ if (need_restart) {
>>+ err = nla_put_u8(msg,
>>+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
>
> Why don't you just put this flag always? Otherwise it could be NLA_FLAG
Ok, I will change to NLA_FLAG.
Thanks,
Steve
[PATCH net-next v4 06/10] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API. Parameters themselves defined in later patches.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 258 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
2 files changed, 263 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 402fa32..8a6037f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,26 +14,260 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
-#ifdef CONFIG_BNXT_SRIOV
- .eswitch_mode_set = bnxt_dl_eswitch_mode_set,
- .eswitch_mode_get = bnxt_dl_eswitch_mode_get,
-#endif /* CONFIG_BNXT_SRIOV */
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
};
-int bnxt_dl_register(struct bnxt *bp)
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
{
- struct devlink *dl;
+ struct hwrm_nvm_get_variable_input req = {0};
+ dma_addr_t dest_data_dma_addr;
+ void *dest_data_addr = NULL;
+ int bytesize;
int rc;
- if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
- return 0;
+ bytesize = roundup(size, BITS_PER_BYTE) / BITS_PER_BYTE;
+ dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &dest_data_dma_addr, GFP_KERNEL);
+ if (!dest_data_addr)
+ return -ENOMEM;
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
+ req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ memcpy(buf, dest_data_addr, bytesize);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, dest_data_addr,
+ dest_data_dma_addr);
+
+ return rc;
+}
+
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
+ const void *buf, int size)
+{
+ struct hwrm_nvm_set_variable_input req = {0};
+ dma_addr_t src_data_dma_addr;
+ void *src_data_addr = NULL;
+ int bytesize;
+ int rc;
+
+ bytesize = roundup(size, BITS_PER_BYTE) / BITS_PER_BYTE;
+
+ src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &src_data_dma_addr, GFP_KERNEL);
+ if (!src_data_addr)
+ return -ENOMEM;
+
+ memcpy(src_data_addr, buf, bytesize);
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+ req.src_data_addr = cpu_to_le64(src_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, src_data_addr,
+ src_data_dma_addr);
- if (bp->hwrm_spec_code < 0x10803) {
- netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch
SWITCHDEV mode.\n");
- return -ENOTSUPP;
+ return 0;
+}
+
+static int bnxt_dl_perm_config_set(struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ u8 type,
+ union devlink_perm_config_value *value,
+ u8 *restart_reqd)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ struct bnxt_drv_cfgparam *entry = NULL;
+ u32 value_int;
+ u32 bytesize;
+ int idx = 0;
+ int ret = 0;
+ u16 val16;
+ u8 val8;
+ int i;
+
+ *restart_reqd = 0;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (!entry)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+ if (BNXT_VF(bp) &
[PATCH net-next v4 01/10] devlink: Add permanent config parameter get/set operations
Add support for permanent config parameter get/set commands. Used
for persistent device configuration parameters.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 7 ++
include/uapi/linux/devlink.h | 25
net/core/devlink.c | 287 +++
3 files changed, 319 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..c7dd912 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,13 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*perm_config_get)(struct devlink *devlink,
+ enum devlink_perm_config_param param, u8 type,
+ union devlink_perm_config_value *value);
+ int (*perm_config_set)(struct devlink *devlink,
+ enum devlink_perm_config_param param, u8 type,
+ union devlink_perm_config_value *value,
+ u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..b3a0b2a 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,10 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ /* Permanent device config get/set */
+ DEVLINK_CMD_PERM_CONFIG_GET,
+ DEVLINK_CMD_PERM_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +206,14 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CONFIGS, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_TYPE, /* u8 */
+ DEVLINK_ATTR_PERM_CONFIG_VALUE, /* dynamic */
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
@@ -244,4 +256,17 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
+/* Permanent config parameters */
+enum devlink_perm_config_param {
+ __DEVLINK_PERM_CONFIG_MAX,
+ DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
+};
+
+/* Permanent config value */
+union devlink_perm_config_value {
+ __u8value8;
+ __u16 value16;
+ __u32 value32;
+};
+
#endif /* _UAPI_LINUX_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7d430c1..a7fa7cc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,277 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+};
+
+static int devlink_nl_single_param_get(struct sk_buff *msg,
+ struct devlink *devlink,
+ u32 param, u8 type)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ union devlink_perm_config_value value;
+ struct nlattr *param_attr;
+ int err;
+
+ err = ops->perm_config_get(devlink, param, type, &value);
+ if (err)
+ return err;
+
+ param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ if (!param_attr)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param) ||
+ nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_TYPE, type))
+ goto nest_err;
+
+ switch (type) {
+ case NLA_U8:
+ if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
+ value.value8))
+ goto nest_err;
+ break;
+ case NLA_U16:
+ if (nla_put_u16(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
+ value.value16))
+ goto nest_err;
+ break;
+ case NLA_U32:
+ if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
+ value.value32))
+ goto nest_err;
+ break;
+ }
+ nla_nest_end(msg, param_attr);
+
+ return err;
+
+nest_err:
+ nla_nest_cancel(msg, param_attr);
+ return -EMSGSIZE;
+}
+
+static int devlink_nl_config_get_fill(struct sk
[PATCH net-next v4 05/10] devlink: Adding num MSI-X vectors per VF perm config param
Adding DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value defines number of MSI-X vectors allocated per VF.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2fd0265..5a0 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -272,11 +272,15 @@ enum devlink_perm_config_enabled {
* DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT: Configures # MSI-X vectors
* advertised via PCI capability for this device.
* Value = Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF: Configures # MSI-X vectors
+ * advertised via PCI capability for each VF on this device.
+ * # of MSI-X vectors per VF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT,
+ DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f970d03..226d151 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1572,6 +1572,7 @@ static const u8
devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
[DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v4 07/10] bnxt: Adding SR-IOV enablement permanent cfg param
Adding permanent config parameter for SR-IOV enablement, using
devlink API for get/set operation.
DEVLINK_PERM_CONFIG_DISABLE = SR-IOV disabled
DEVLINK_PERM_CONFIG_ENABLE = SR-IOV enabled
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 8a6037f..9f168a8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,7 +14,14 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
+/* Permanent config parameters from devlink.h:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ */
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v4 10/10] bnxt: Adding num MSI-X vectors per VF perm config param
Adding permanent config parameter for number MSI-X vectors
per VF, using devlink API for get/set operation.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 047f09e..555e098 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -22,6 +22,8 @@
* # of VFs per PF in SR-IOV mode
* DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
* Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF:
+ * # of MSI-X vectors per VF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
@@ -30,6 +32,8 @@ struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
BNXT_DRV_APPL_FUNCTION, 8, 404},
{DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_PERM_CONFIG_NUM_MSIX_VECT_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v4 08/10] bnxt: Adding num VFs per PF perm config param
Adding permanent config parameter for number of VFs per PF,
using devlink API for get/set operation.
Value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 9f168a8..0812221 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -18,10 +18,14 @@
* DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
* DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
+ * # of VFs per PF in SR-IOV mode
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
+ {DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v4 09/10] bnxt: Adding max PF MSI-X vectors perm config param
Adding permanent config parameter for maximum number of PF
MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 0812221..047f09e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -20,12 +20,16 @@
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
* # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
+ * Max # of MSI-X vectors per PF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
{DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v4 04/10] devlink: Adding max PF MSI-X vectors perm config param
Adding DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value sets the maximum number of PF MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2c714fb..2fd0265 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -269,10 +269,14 @@ enum devlink_perm_config_enabled {
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF: Configures number of total/maximum VFs
* per PF available on device.
* Value: # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT: Configures # MSI-X vectors
+ * advertised via PCI capability for this device.
+ * Value = Max # of MSI-X vectors per PF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
+ DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4b1fff3..f970d03 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1571,6 +1571,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_NUM_PF_MSIX_VECT] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v4 03/10] devlink: Adding num VFs per PF permanent config param
Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
parameter. Value is permanent, so becomes the new default
value for this device.
The value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 4
net/core/devlink.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 9a41f6e..2c714fb 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -266,9 +266,13 @@ enum devlink_perm_config_enabled {
* provided by device.
* DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
* DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF: Configures number of total/maximum VFs
+ * per PF available on device.
+ * Value: # of VFs per PF in SR-IOV mode
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+ DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 395c93c..4b1fff3 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1570,6 +1570,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
+ [DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v4 00/10] Adding permanent config get/set to devlink
Changes since v3: * Using union instead of void * to pass parameter values to/from drivers. * Updated parameter comments / naming and added enum for enable/disable to add clarity. * Various code cleanup in bnxt: - change hwrm call to use mutex protected version - use roundup() macro - remove unnecessary error messages - change mechanism to check for parameter-not-found - cleanup merge w/ switchdev eswitch devlink functions - rebased with Michael Chan's recent bnxt changes in net-next. Suggested changes not implemented: * re: val8 = val32 "Don't you need explicit castings for these kind of assignments to prevent warnings?" -- No, I don't think explicit castings are necessary here; I don't get any compiler warnings (w/ gcc 5.4.0) -- Adds a devlink command for getting & setting permanent / persistent device configuration parameters, and enumerates the parameters as nested devlink attributes. Steve Lin (10): devlink: Add permanent config parameter get/set operations devlink: Adding SR-IOV enablement perm config param devlink: Adding num VFs per PF permanent config param devlink: Adding max PF MSI-X vectors perm config param devlink: Adding num MSI-X vectors per VF perm config param bnxt: Add devlink support for config get/set bnxt: Adding SR-IOV enablement permanent cfg param bnxt: Adding num VFs per PF perm config param bnxt: Adding max PF MSI-X vectors perm config param bnxt: Adding num MSI-X vectors per VF perm config param drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 277 +++- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++ include/net/devlink.h | 7 + include/uapi/linux/devlink.h | 49 net/core/devlink.c| 291 ++ 5 files changed, 629 insertions(+), 12 deletions(-) -- 2.7.4
[PATCH net-next v4 02/10] devlink: Adding SR-IOV enablement perm config param
Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
parameter. Value is permanent, so becomes the new default
value for this device.
DEVLINK_PERM_CONFIG_DISABLE = Disable SR-IOV
DEVLINK_PERM_CONFIG_ENABLE = Enable SR-IOV
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 14 +-
net/core/devlink.c | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index b3a0b2a..9a41f6e 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -256,8 +256,20 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
-/* Permanent config parameters */
+enum devlink_perm_config_enabled {
+ DEVLINK_PERM_CONFIG_DISABLE,
+ DEVLINK_PERM_CONFIG_ENABLE,
+};
+
+/* Permanent config parameters:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED: Configures whether SR-IOV PCI capability
+ * provided by device.
+ * DEVLINK_PERM_CONFIG_DISABLE = disable SR-IOV
+ * DEVLINK_PERM_CONFIG_ENABLE = enable SR-IOV
+ */
enum devlink_perm_config_param {
+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
};
diff --git a/net/core/devlink.c b/net/core/devlink.c
index a7fa7cc..395c93c 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1569,6 +1569,7 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff
*skb,
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+ [DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
Re: [PATCH net-next v3 01/10] devlink: Add permanent config parameter get/set operations
On Tue, Oct 24, 2017 at 5:22 PM, Yuval Mintz wrote:
>> Add support for permanent config parameter get/set commands. Used
>> for persistent device configuration parameters.
>>
> ...
>> + int (*perm_config_get)(struct devlink *devlink,
>> +enum devlink_perm_config_param param, u8
>> type,
>> +void *value);
>> + int (*perm_config_set)(struct devlink *devlink,
>> +enum devlink_perm_config_param param, u8
>> type,
>> +void *value, u8 *restart_reqd);
>> };
>> +static int devlink_nl_single_param_get(struct sk_buff *msg,
>> +struct devlink *devlink,
>> +u32 param, u8 type)
>> +{
>> + const struct devlink_ops *ops = devlink->ops;
>> + struct nlattr *param_attr;
>> + void *value;
>> + u32 val;
>> + int err;
>> +
>> + /* Allocate buffer for parameter value */
>> + switch (type) {
>> + case NLA_U8:
>> + value = kmalloc(sizeof(u8), GFP_KERNEL);
>> + break;
>> + case NLA_U16:
>> + value = kmalloc(sizeof(u16), GFP_KERNEL);
>> + break;
>> + case NLA_U32:
>> + value = kmalloc(sizeof(u32), GFP_KERNEL);
>> + break;
>> + default:
>> + return -EINVAL; /* Unsupported Type */
>> + }
>> +
>> + if (!value)
>> + return -ENOMEM;
>> +
>> + err = ops->perm_config_get(devlink, param, type, value);
>> + if (err)
>> + return err;
>
> I suspect this logic might be risky - its dependent on the driver to cast the
> 'value' into the proper type or else, E.g., the following switch might break
> for BE platforms.
> Is there any reason to have the devlink <-> driver API be based on void*
> and not on some typed data [of sufficient size]?
> ...
>> + switch (type) {
>> + case NLA_U8:
>> + val = *((u8 *)value);
>> + if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
>> val))
>> + goto nest_err;
>> + break;
>> + case NLA_U16:
>> + val = *((u16 *)value);
>> + if (nla_put_u16(msg,
>> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
>> + goto nest_err;
>> + break;
>> + case NLA_U32:
>> + val = *((u32 *)value);
>> + if (nla_put_u32(msg,
>> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
>> + goto nest_err;
>> + break;
>> + }
Why might this break on a BE system? It's not as though driver will
be compiled LE and kernel BE or vice versa - as long as driver and
kernel are same endian-ness, I would think it should be okay?
In general, the issue is that the parameter could be any of the
netlink types (per Jiri's suggestion to the previous version of this
patch). So, we allocate some space, tell the driver the type we're
expecting (the type argument to the perm_config_get() function), and
yes, we rely on the driver to write something of the type we request
to the pointer we provide. Are you suggesting defining a union of
U8/U16/U32, and passing a pointer to that for the driver to fill in?
The issue is that whatever the types we support now, we want future
parameters to be able to be of arbitrary types. Defining the
interface to use the void pointer means that some future parameter can
be of some other type, without having to update all the drivers using
this API...
Or did I misunderstand your suggestion?
Re: [PATCH net-next v3 06/10] bnxt: Add devlink support for config get/set
On Tue, Oct 24, 2017 at 4:52 PM, Michael Chan wrote:
> On Tue, Oct 24, 2017 at 1:12 PM, Steve Lin wrote:
>> Implements get and set of configuration parameters using new devlink
>> config get/set API. Parameters themselves defined in later patches.
>>
>> Signed-off-by: Steve Lin
>> Acked-by: Andy Gospodarek
>> ---
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 262
>> +-
>> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
>> drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +
>> 3 files changed, 373 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> index f3f6aa8..81ab77e 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> @@ -14,11 +14,261 @@
>> #include "bnxt_vfr.h"
>> #include "bnxt_devlink.h"
>>
>> -static const struct devlink_ops bnxt_dl_ops = {
>> +struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
>> +};
>> +
>> +#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
>> +
>> +static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
>> +void *buf, int size)
>> +{
>> + struct hwrm_nvm_get_variable_input req = {0};
>> + dma_addr_t dest_data_dma_addr;
>> + void *dest_data_addr = NULL;
>> + int bytesize;
>> + int rc;
>> +
>> + bytesize = (size + 7) / BITS_PER_BYTE;
>> + dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
>> + &dest_data_dma_addr, GFP_KERNEL);
>> + if (!dest_data_addr) {
>> + netdev_err(bp->dev, "dma_alloc_coherent failure\n");
>> + return -ENOMEM;
>> + }
>> +
>> + bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
>> + req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
>> + req.data_len = cpu_to_le16(size);
>> + req.option_num = cpu_to_le16(nvm_param);
>> + req.index_0 = cpu_to_le16(idx);
>> + if (idx != 0)
>> + req.dimensions = cpu_to_le16(1);
>> +
>> + rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
>
> This won't have the proper mutex protection. You should call
> hwrm_send_message() instead.
Ok, thanks, I'll fix that.
[PATCH net-next v3 01/10] devlink: Add permanent config parameter get/set operations
Add support for permanent config parameter get/set commands. Used
for persistent device configuration parameters.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 6 +
include/uapi/linux/devlink.h | 18 +++
net/core/devlink.c | 295 +++
3 files changed, 319 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..eb86031 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,12 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*perm_config_get)(struct devlink *devlink,
+ enum devlink_perm_config_param param, u8 type,
+ void *value);
+ int (*perm_config_set)(struct devlink *devlink,
+ enum devlink_perm_config_param param, u8 type,
+ void *value, u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..28ea961 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,10 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ /* Permanent device config get/set */
+ DEVLINK_CMD_PERM_CONFIG_GET,
+ DEVLINK_CMD_PERM_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +206,14 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CONFIGS, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_TYPE, /* u8 */
+ DEVLINK_ATTR_PERM_CONFIG_VALUE, /* dynamic */
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
@@ -244,4 +256,10 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
+/* Permanent config parameters */
+enum devlink_perm_config_param {
+ __DEVLINK_PERM_CONFIG_MAX,
+ DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
+};
+
#endif /* _UAPI_LINUX_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7d430c1..4deb4da 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,285 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+};
+
+static int devlink_nl_single_param_get(struct sk_buff *msg,
+ struct devlink *devlink,
+ u32 param, u8 type)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ struct nlattr *param_attr;
+ void *value;
+ u32 val;
+ int err;
+
+ /* Allocate buffer for parameter value */
+ switch (type) {
+ case NLA_U8:
+ value = kmalloc(sizeof(u8), GFP_KERNEL);
+ break;
+ case NLA_U16:
+ value = kmalloc(sizeof(u16), GFP_KERNEL);
+ break;
+ case NLA_U32:
+ value = kmalloc(sizeof(u32), GFP_KERNEL);
+ break;
+ default:
+ return -EINVAL; /* Unsupported Type */
+ }
+
+ if (!value)
+ return -ENOMEM;
+
+ err = ops->perm_config_get(devlink, param, type, value);
+ if (err)
+ return err;
+
+ param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ if (!param_attr)
+ goto nonest_err;
+
+ if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param) ||
+ nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_TYPE, type))
+ goto nest_err;
+
+ switch (type) {
+ case NLA_U8:
+ val = *((u8 *)value);
+ if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
+ goto nest_err;
+ break;
+ case NLA_U16:
+ val = *((u16 *)value);
+ if (nla_put_u16(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
+ goto nest_err;
+ break;
+ case NLA_U32:
+ val = *((u32 *)value);
+ if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
+
[PATCH net-next v3 05/10] devlink: Adding num MSI-X vectors per VF perm config param
Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value defines number of MSI-X vectors allocated per VF.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 3 +++
net/core/devlink.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5877ff9..1b76e8f 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -264,11 +264,14 @@ enum devlink_dpipe_header_id {
* # of VFs per PF in SR-IOV mode
* DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
* Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF:
+ * # of MSI-X vectors per VF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
+ DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 284e167..07d64da 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1572,6 +1572,7 @@ static const u8
devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
[DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v3 07/10] bnxt: Adding SR-IOV enablement permanent cfg param
Adding permanent config parameter for SR-IOV enablement, using
devlink API for get/set operation.
0 = SR-IOV disabled
1 = SR-IOV enabled
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 81ab77e..6eb80c1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,7 +14,14 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
+/* Permanent config parameters from devlink.h:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
+ * 0 = disable SR-IOV
+ * 1 = enable SR-IOV
+ */
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v3 06/10] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API. Parameters themselves defined in later patches.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 262 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +
3 files changed, 373 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index f3f6aa8..81ab77e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,11 +14,261 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+};
+
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
+{
+ struct hwrm_nvm_get_variable_input req = {0};
+ dma_addr_t dest_data_dma_addr;
+ void *dest_data_addr = NULL;
+ int bytesize;
+ int rc;
+
+ bytesize = (size + 7) / BITS_PER_BYTE;
+ dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &dest_data_dma_addr, GFP_KERNEL);
+ if (!dest_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
+ req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ memcpy(buf, dest_data_addr, bytesize);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, dest_data_addr,
+ dest_data_dma_addr);
+
+ return rc;
+}
+
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
+ const void *buf, int size)
+{
+ struct hwrm_nvm_set_variable_input req = {0};
+ dma_addr_t src_data_dma_addr;
+ void *src_data_addr = NULL;
+ int bytesize;
+ int rc;
+
+ bytesize = (size + 7) / BITS_PER_BYTE;
+
+ src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &src_data_dma_addr, GFP_KERNEL);
+ if (!src_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ memcpy(src_data_addr, buf, bytesize);
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+ req.src_data_addr = cpu_to_le64(src_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, src_data_addr,
+ src_data_dma_addr);
+
+ return 0;
+}
+
+static int bnxt_dl_perm_config_set(struct devlink *devlink,
+ enum devlink_perm_config_param param,
+ u8 type, void *value, u8 *restart_reqd)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ struct bnxt_drv_cfgparam *entry;
+ int idx = 0;
+ int ret = 0;
+ u32 bytesize;
+ u32 val32;
+ u16 val16;
+ u8 val8;
+ int i;
+
+ *restart_reqd = 0;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (i == BNXT_NUM_DRV_CFGPARAM)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+ if (BNXT_VF(bp) && !(entry->func & BNXT_DRV_VF))
+ return -EOPNOTSUPP;
+
+ /* If parameter is per port or function, compute index */
+ if (entry->appl == BNXT_DRV_APPL_PORT) {
+ idx = bp->pf.port_id;
+ } else if (entry->appl == BNXT_DRV_APPL_FUNCTION) {
+ if (BNXT_PF(bp))
+ idx = bp->pf.fw_fid - BNXT_FIRST_PF
[PATCH net-next v3 00/10] Adding permanent config get/set to devlink
Changes since v2: * Removed references to "NVRAM" in comments / commits. * Add parameter descriptions to header file. * Split bnxt patch into infrastructure, then one patch for each new parameter. * Cleaned up goofs (unused parameters leftover from v1) * Used enum rather than u32 in prototype for perm_config_get() and _set(). * Defined DEVLINK_ATTR_PERM_CONFIG_TYPE so future parameters can use arbitrary types (not just U32 and smaller). * Reverse Christmas tree local variable definitions. * No longer return original/previous value of parameter in response to set. * Check parameter within enum limits before using it as array index. I have NOT implemented the following suggested changes: * Have driver report what parameters and parameter options it supports. Could be done in future patch by defining new devlink op. * Support parameters spread across multi-part netlink messages. See discussion on list - this doesn't seem necessary even for devices with large number of parameters. * Support specifying per-VF config, if the VFs don't have pci b/d/f associated with them. See discussion on list; if/when this support is required, could add DEVLINK_ATTR_PERM_CONFIG_VF_INDEX to describe, without breaking UAPI. * Rolling back previously set parameters in a collection of sets, when one fails. Instead, we report back to user which sets were successful, so they know which were set and which weren't, and can decide how to proceed. -- Adds a devlink command for getting & setting permanent / persistent device configuration parameters, and enumerates the parameters as nested devlink attributes. bnxt driver patches makes use of these new devlink cmds. Steve Lin (10): devlink: Add permanent config parameter get/set operations devlink: Adding SR-IOV enablement perm config param devlink: Adding num VFs per PF permanent config param devlink: Adding max PF MSI-X vectors perm config param devlink: Adding num MSI-X vectors per VF perm config param bnxt: Add devlink support for config get/set bnxt: Adding SR-IOV enablement permanent cfg param bnxt: Adding num VFs per PF perm config param bnxt: Adding max PF MSI-X vectors perm config param bnxt: Adding num MSI-X vectors per VF perm config param drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 281 +++- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 include/net/devlink.h | 6 + include/uapi/linux/devlink.h | 33 +++ net/core/devlink.c| 299 ++ 6 files changed, 730 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH net-next v3 02/10] devlink: Adding SR-IOV enablement perm config param
Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
parameter. Value is permanent, so becomes the new default
value for this device.
0 = Disable SR-IOV
1 = Enable SR-IOV
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 8 +++-
net/core/devlink.c | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 28ea961..ed520e7 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -256,8 +256,14 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
-/* Permanent config parameters */
+/* Permanent config parameters:
+ * DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
+ * 0 = disable SR-IOV
+ * 1 = enable SR-IOV
+ */
enum devlink_perm_config_param {
+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
};
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4deb4da..58ba715 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1569,6 +1569,7 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff
*skb,
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
+ [DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v3 09/10] bnxt: Adding max PF MSI-X vectors perm config param
Adding permanent config parameter for maximum number of PF
MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 55913c4..c6e670c 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -20,12 +20,16 @@
* 1 = enable SR-IOV
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
* # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
+ * Max # of MSI-X vectors per PF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
{DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v3 03/10] devlink: Adding num VFs per PF permanent config param
Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
parameter. Value is permanent, so becomes the new default
value for this device.
The value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 3 +++
net/core/devlink.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index ed520e7..db512c5 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -260,9 +260,12 @@ enum devlink_dpipe_header_id {
* DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
* 0 = disable SR-IOV
* 1 = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
+ * # of VFs per PF in SR-IOV mode
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+ DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 58ba715..18f2600 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1570,6 +1570,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
+ [DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v3 04/10] devlink: Adding max PF MSI-X vectors perm config param
Adding DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT permanent config
parameter. Value is permanent, so becomes the new default value
for this device.
Value sets the maximum number of PF MSI-X vectors.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 3 +++
net/core/devlink.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index db512c5..5877ff9 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -262,10 +262,13 @@ enum devlink_dpipe_header_id {
* 1 = enable SR-IOV
* DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
* # of VFs per PF in SR-IOV mode
+ * DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
+ * Max # of MSI-X vectors per PF
*/
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
+ DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
__DEVLINK_PERM_CONFIG_MAX,
DEVLINK_PERM_CONFIG_MAX = __DEVLINK_PERM_CONFIG_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 18f2600..284e167 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1571,6 +1571,7 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
static const u8 devlink_perm_cfg_param_types[DEVLINK_PERM_CONFIG_MAX + 1] = {
[DEVLINK_PERM_CONFIG_SRIOV_ENABLED] = NLA_U8,
[DEVLINK_PERM_CONFIG_NUM_VF_PER_PF] = NLA_U32,
+ [DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT] = NLA_U32,
};
static int devlink_nl_single_param_get(struct sk_buff *msg,
--
2.7.4
[PATCH net-next v3 10/10] bnxt: Adding num MSI-X vectors per VF perm config param
Adding permanent config parameter for number MSI-X vectors
per VF, using devlink API for get/set operation.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index c6e670c..620a207 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -22,6 +22,8 @@
* # of VFs per PF in SR-IOV mode
* DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT:
* Max # of MSI-X vectors per PF
+ * DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF:
+ * # of MSI-X vectors per VF
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
@@ -30,6 +32,8 @@ struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
BNXT_DRV_APPL_FUNCTION, 8, 404},
{DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
[PATCH net-next v3 08/10] bnxt: Adding num VFs per PF perm config param
Adding permanent config parameter for number of VFs per PF,
using devlink API for get/set operation.
Value sets the number of VFs per PF in SR-IOV mode.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 6eb80c1..55913c4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -18,10 +18,14 @@
* DEVLINK_PERM_CONFIG_SRIOV_ENABLED:
* 0 = disable SR-IOV
* 1 = enable SR-IOV
+ * DEVLINK_PERM_CONFIG_NUM_VF_PER_PF:
+ * # of VFs per PF in SR-IOV mode
*/
struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
{DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
BNXT_DRV_APPL_SHARED, 1, 401},
+ {DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
};
#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
--
2.7.4
Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
On Mon, Oct 23, 2017 at 10:37 AM, Yuval Mintz wrote: >> >> On Fri, Oct 20, 2017 at 10:10 AM, Jiri Pirko wrote: >> >> > Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.l...@broadcom.com wrote: >> >> >>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko wrote: >> >> >>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuv...@mellanox.com wrote: >> >> > Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF >> >> permanent >> >> > config >> >> > parameter. Defines number of MSI-X vectors allocated per VF. >> >> > Value is permanent (stored in NVRAM), so becomes the new >> default >> >> > value for this device. >> >> >> >> Sounds like you're having this enforce the same configuration for all >> >> child VFs. >> >> >>> >> >> >>> Yeah, this sounds like per-port config. >> >> >>> >> >> >> >> >> >>Well, it gets a little tricky here. I assume some cards handle this >> >> >>per-port. Other cards might handle this per PF, where PF may not >> >> >>always correspond 1:1 with a port. And some cards maybe just allow a >> >> >>single value for this parameter for the entire card, covering all >> >> >>ports/PFs. >> >> >> >> >> >>To keep things simple and as general as possible, it made sense to set >> >> >>all parameters on a per-PCI device level. As I mentioned in my >> >> >>cover-letter, the devices most likely to use these proposed commands >> >> >>do not have a single "whole asic" PCI b/d/f with internal mechanism >> >> >>for accessing ports - most expose each port (and each function on each >> >> >>port) as a separate PCI b/d/f, with no separate "whole asic" PCI >> >> >>b/d/f. That's how the BCM cards work, and I think that's how the >> MLNX >> >> >>cards work, and others that would be likely to use these cmds. >> >> >> >> >> >>So, to summarize, you direct the command to the PCI b/d/f you want >> to >> >> >>target. Does this make sense? >> >> > >> >> > So you plan to have 1 devlink instance for each vf? Not sure that >> >> > does sound right to me :/ >> >> > >> >> >> >> For the commands proposed in this patchset, AFAIK they all apply on a >> >> per-PF or broader, i.e. per-port or whole-card, granularity, since >> >> they affect permanent config that applies at boot-up. So, no, the VFs >> >> don't really come into play here. >> > >> > Regardless of whether you're planning on having VFs as devlink instances, >> > the actual attribute question remains - >> > you're proposing an attribute that forces all VFs to have the same value. >> > This probably suits your PCI core limitations but other vendors might have >> > a different capability set, and accepting this design limitation now would >> > muck all future extension attempts of such attributes. >> > >> > I think VF configurations should be planned in advance for supporting a >> > per-VF Configuration whenever it's possible - even if not required >> [/possible] >> > by the one pushing the new attribute. >> > >> >> The commands being added in this patch are for permanent (i.e. NVRAM) >> config - essentially setting the new default values for various >> features of the device at boot-up. At that initialization time, no >> VFs are yet instantiated. >> >> So my perspective was, in general (not just for our specific device / >> design), it doesn't seem like permanent config parameters would be set >> on individual VFs. That was what my previous comment was trying to >> convey. > > That's an odd assumption; Why should you assume there's some device > that allows configuring persistent behavior for all VFs but think no other > would set the same on a per-VF basis? > >> If that assumption is wrong, though, and there is some device that has >> NVRAM config that is set per-VF, I assume the user would instantiate >> the VF and then call the devlink API on the pci device corresponding >> to the VF they with to affect, and I think the model proposed still >> works. > > What would be the purpose of re-configuring a value reflected in the > PCI device for an already instantiated VF? > >> Are you suggesting adding a mechanism to set NVRAM parameters on a >> per-VF basis, without instantiating the VF first? I would prefer not >> adding such a mechanism unless/until there's a use case for it. > > The thing is that you're suggesting a new UAPI; We don't have the leisure > of pushing a partial implementation and changing it later on. I hope we're not talking past each other because I'm not sure we're saying the same thing. But if you have a device which has NVRAM config on an individual VF basis, and you want to be able to get/set that configuration without instantiating the VF first (i.e. without a PCI device to operate on), then one way to handle this is with a new attribute, DEVLINK_ATTR_PERM_CONFIG_VF_INDEX, for example. It could be sent in the nested DEVLINK_ATTR_PERM_CONFIG attribute, along with the existing DEVLINK_ATTR_PERM_CONFIG_PARAMETER and _VALUE, to indicate a specific VF within the PF that you are targeting. That seems like the type of thing that could be add
Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
On Sat, Oct 21, 2017 at 10:12 AM, Yuval Mintz wrote: >> On Thu, Oct 19, 2017 at 4:21 PM, Yuval Mintz >> wrote: >> >> Subject: [PATCH net-next v2 1/6] devlink: Add permanent config >> parameter >> >> get/set operations >> >> >> >> Add support for permanent config parameter get/set commands. Used >> >> for parameters held in NVRAM, persistent device configuration. >> > >> > Given some of the attributes aren't Boolean, what about an API that >> > allows the user to learn of supported values per option? >> > Otherwise only way for configuring some of them would be trial & error. >> >> Interesting suggestion. There's a couple of places where this could >> be a factor. (1) When a user wants to know what values are >> defined/available in the API, and (2) When the user wants to know what >> values are supported by a specific driver/device. >> >> The intention for (1) is to push that into userspace. The userspace >> devlink tool patches I am working on (not yet submitted) essentially >> mirror the config parameters and their options, with string "keywords" >> associated with each parameter and option, since it's the userspace >> app that will be parsing the command line strings and converting to >> API enums. So the userspace app can provide the list of >> parameters/options it supports, which could be a subset of what's >> available in the API. >> >> For (2), currently there is no mechanism other than trial/error as you >> suggest (up to driver to either return an error or else make use of >> the value specified by the user). We could contemplate adding such a >> mechanism, but it's a little complicated as some options take a range >> (i.e. # of VFs per PF for example), and others may take one of a set >> of enumerated values (pre-boot link speed for example). >> >> To clarify, are you suggesting some mechanism to allow a driver to >> report which parameters and options it supports (case (2))? Or are >> you suggesting something in the kernel API to handle case (1) above? > > I was thinking of (2). And I agree it would take some effort. I don't disagree that this could be a useful addition. But, it seems like this could be added as a follow on patch. I think many/most users of this permanent device config API probably already know what capabilities their device offers, and if they are wrong, the driver can indicate invalid config options. Nothing in the patchset prevents future work to add a new devlink operation to query the driver for supported options. Thoughts? >> > Isn't it possible that a response for a single request for multiple ATTRs >> > wouldn't fit in a single message? >> > >> >> Hmm... Given the small size and relatively small total number of these >> attributes, even when additional drivers add their own, I think it's > > We probably have a different idea about 'small'. > Didn’t your *initial* series attempt to add 35 attributes at once? > Yes, and I'd still like to get those ~35 parameters in eventually! :) But even if there were 100 parameters, and all 100 were being get or set at once, if you assume 20 bytes per parameter (4 bytes each for DEVLINK_ATTR_CONFIG, DEVLINK_ATTR_CONFIG_PARAMETER, DEVLINK_ATTR_CONFIG_VALUE, 8 for nesting), that's ~2000 bytes in the message. A little overhead, too, of course, but still less than half a typical netlink message size of 4KB. So, at some point - if a device with 250 parameters comes along, say, all of which are set/get at once - this could be a problem, but it's not clear if this is a realistic scenario to prepare for?
Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
On Sat, Oct 21, 2017 at 9:59 AM, Yuval Mintz wrote: >> On Fri, Oct 20, 2017 at 10:10 AM, Jiri Pirko wrote: >> > Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.l...@broadcom.com wrote: >> >>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko wrote: >> >>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuv...@mellanox.com wrote: >> > Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF >> permanent >> > config >> > parameter. Defines number of MSI-X vectors allocated per VF. >> > Value is permanent (stored in NVRAM), so becomes the new default >> > value for this device. >> >> Sounds like you're having this enforce the same configuration for all >> child VFs. >> >>> >> >>> Yeah, this sounds like per-port config. >> >>> >> >> >> >>Well, it gets a little tricky here. I assume some cards handle this >> >>per-port. Other cards might handle this per PF, where PF may not >> >>always correspond 1:1 with a port. And some cards maybe just allow a >> >>single value for this parameter for the entire card, covering all >> >>ports/PFs. >> >> >> >>To keep things simple and as general as possible, it made sense to set >> >>all parameters on a per-PCI device level. As I mentioned in my >> >>cover-letter, the devices most likely to use these proposed commands >> >>do not have a single "whole asic" PCI b/d/f with internal mechanism >> >>for accessing ports - most expose each port (and each function on each >> >>port) as a separate PCI b/d/f, with no separate "whole asic" PCI >> >>b/d/f. That's how the BCM cards work, and I think that's how the MLNX >> >>cards work, and others that would be likely to use these cmds. >> >> >> >>So, to summarize, you direct the command to the PCI b/d/f you want to >> >>target. Does this make sense? >> > >> > So you plan to have 1 devlink instance for each vf? Not sure that >> > does sound right to me :/ >> > >> >> For the commands proposed in this patchset, AFAIK they all apply on a >> per-PF or broader, i.e. per-port or whole-card, granularity, since >> they affect permanent config that applies at boot-up. So, no, the VFs >> don't really come into play here. > > Regardless of whether you're planning on having VFs as devlink instances, > the actual attribute question remains - > you're proposing an attribute that forces all VFs to have the same value. > This probably suits your PCI core limitations but other vendors might have > a different capability set, and accepting this design limitation now would > muck all future extension attempts of such attributes. > > I think VF configurations should be planned in advance for supporting a > per-VF Configuration whenever it's possible - even if not required [/possible] > by the one pushing the new attribute. > The commands being added in this patch are for permanent (i.e. NVRAM) config - essentially setting the new default values for various features of the device at boot-up. At that initialization time, no VFs are yet instantiated. So my perspective was, in general (not just for our specific device / design), it doesn't seem like permanent config parameters would be set on individual VFs. That was what my previous comment was trying to convey. If that assumption is wrong, though, and there is some device that has NVRAM config that is set per-VF, I assume the user would instantiate the VF and then call the devlink API on the pci device corresponding to the VF they with to affect, and I think the model proposed still works. Are you suggesting adding a mechanism to set NVRAM parameters on a per-VF basis, without instantiating the VF first? I would prefer not adding such a mechanism unless/until there's a use case for it.
Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
On Sat, Oct 21, 2017 at 5:24 AM, Jiri Pirko wrote:
> Fri, Oct 20, 2017 at 05:13:39PM CEST, steven.l...@broadcom.com wrote:
>>On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko wrote:
>>> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.l...@broadcom.com wrote:
>>>>Add support for permanent config parameter get/set commands. Used
>>>>for parameters held in NVRAM, persistent device configuration.
>>>>
>>>>Signed-off-by: Steve Lin
>>>>Acked-by: Andy Gospodarek
>>>>---
>>>> include/net/devlink.h| 3 +
>>>> include/uapi/linux/devlink.h | 11 ++
>>>> net/core/devlink.c | 234
>>>> +++
>>>> 3 files changed, 248 insertions(+)
>>>>
>>>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>>>index b9654e1..bd64623 100644
>>>>--- a/include/net/devlink.h
>>>>+++ b/include/net/devlink.h
>>>>@@ -270,6 +270,9 @@ struct devlink_ops {
>>>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>>>> inline_mode);
>>>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>>>> *p_encap_mode);
>>>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
>>>> encap_mode);
>>>>+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32
>>>>*value);
>>>>+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
>>>
>>> Please use enum instead of "u32 param". Also, what would happen if the
>>> value is >u32, like string for example? I believe we need to take it into
>>> the consideration for the UAPI sake.
>>>
>>>
>>
>>Using enum instead of u32 param: ok, will do in v3, thanks.
>>
>>Value > u32: In the RFC and v1 versions of the patch, each parameter
>>was its own attribute, so could have its own type (u32, string,
>>whatever). In v2, trying to move to nested parameters w/ parameter
>>being an enum, as requested, it seems to mean that the parameter value
>>now must be defined as a specific type, so I went with u32.
>
> Why? I have to be missing something. In the nest all is same as outside
> of the nest.
>
> Also, please see team_nl_cmd_options_set() where something similar is
> done, for multiple option types.
>
>
Okay, I can implement similar to that driver. In the current
devlink.c/devlink.h, all attributes have specific types, and aren't
dynamic types like the "data" attribute in the team driver, so I had
thought having specific types defined for each attribute was required
in devlink.
Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko wrote:
> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.l...@broadcom.com wrote:
>>Add support for permanent config parameter get/set commands. Used
>>for parameters held in NVRAM, persistent device configuration.
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/net/devlink.h| 3 +
>> include/uapi/linux/devlink.h | 11 ++
>> net/core/devlink.c | 234
>> +++
>> 3 files changed, 248 insertions(+)
>>
>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>index b9654e1..bd64623 100644
>>--- a/include/net/devlink.h
>>+++ b/include/net/devlink.h
>>@@ -270,6 +270,9 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
>>+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
>
> Please use enum instead of "u32 param". Also, what would happen if the
> value is >u32, like string for example? I believe we need to take it into
> the consideration for the UAPI sake.
>
>
Using enum instead of u32 param: ok, will do in v3, thanks.
Value > u32: In the RFC and v1 versions of the patch, each parameter
was its own attribute, so could have its own type (u32, string,
whatever). In v2, trying to move to nested parameters w/ parameter
being an enum, as requested, it seems to mean that the parameter value
now must be defined as a specific type, so I went with u32.
If we need to support strings or other types > u32, then the
perm_config_value attribute will not be a fixed type, so can't be
policy checked. Or, I could go back to non-nested as in RFC/v1 case
and have each parameter with its own type.
> [...]
>
>
>>+
>>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>>+ struct devlink *devlink,
>>+ u32 param, u32 value)
>>+{
>>+ u32 orig_value;
>>+ u8 need_restart;
>>+ int err;
>>+ const struct devlink_ops *ops = devlink->ops;
>>+ struct nlattr *cfgparam_attr;
>
> Reverse christmas tree please (this applies to all functions)
Will do in v3, thanks.
>
>
>>+
>>+ /* First get current value of parameter */
>>+ err = ops->perm_config_get(devlink, param, &orig_value);
>
> I'm missing why this is needed.
>
>
>>+ if (err)
>>+ return err;
>>+
>>+ /* Now set parameter */
>>+ err = ops->perm_config_set(devlink, param, value, &need_restart);
>>+ if (err)
>>+ return err;
>>+
>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>>+ /* Update restart reqd - if any param needs restart, should be set */
>>+ if (need_restart)
>>+ err = nla_put_u8(msg,
>>+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
>>+
>>+ /* Since set was successful, write attr back to msg with orig val */
>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, orig_value);
>
> Why to write it back?
In a response to the "RFC" version of this patch, you wrote: "Also,
we need to expose to the user the original value (currently being
used) and the new one (to be used after driver re-instatiation)".
I understood that to mean that we need to return the current/original
value of the parameter (and the user knows the new value, since they
are setting it).
If I mis-interpreted that comment, then I'm happy to remove returning
the original value to the user; it wasn't in there originally.
>
>
>>+
>>+ nla_nest_end(msg, cfgparam_attr);
>>+
>>+ return 0;
>>+}
>>+
>>+static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
>>+ struct genl_info *info)
>>+{
>>+ struct devlink *devlink = info->user_ptr[0];
>>+ struct sk_buff *msg;
>>+ void *hdr;
>>+ struct nlattr *attr;
>>+ int rem;
>>+ int err;
>>+ u8 restart_reqd = 0;
>>+ struct nlattr *cfgparam_attr;
>>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>>+ u32 param;
>>
Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
On Fri, Oct 20, 2017 at 10:10 AM, Jiri Pirko wrote: > Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.l...@broadcom.com wrote: >>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko wrote: >>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuv...@mellanox.com wrote: > Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent > config > parameter. Defines number of MSI-X vectors allocated per VF. > Value is permanent (stored in NVRAM), so becomes the new default > value for this device. Sounds like you're having this enforce the same configuration for all child VFs. >>> >>> Yeah, this sounds like per-port config. >>> >> >>Well, it gets a little tricky here. I assume some cards handle this >>per-port. Other cards might handle this per PF, where PF may not >>always correspond 1:1 with a port. And some cards maybe just allow a >>single value for this parameter for the entire card, covering all >>ports/PFs. >> >>To keep things simple and as general as possible, it made sense to set >>all parameters on a per-PCI device level. As I mentioned in my >>cover-letter, the devices most likely to use these proposed commands >>do not have a single "whole asic" PCI b/d/f with internal mechanism >>for accessing ports - most expose each port (and each function on each >>port) as a separate PCI b/d/f, with no separate "whole asic" PCI >>b/d/f. That's how the BCM cards work, and I think that's how the MLNX >>cards work, and others that would be likely to use these cmds. >> >>So, to summarize, you direct the command to the PCI b/d/f you want to >>target. Does this make sense? > > So you plan to have 1 devlink instance for each vf? Not sure that > does sound right to me :/ > For the commands proposed in this patchset, AFAIK they all apply on a per-PF or broader, i.e. per-port or whole-card, granularity, since they affect permanent config that applies at boot-up. So, no, the VFs don't really come into play here.
Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko wrote: > Thu, Oct 19, 2017 at 10:32:21PM CEST, yuv...@mellanox.com wrote: >>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent >>> config >>> parameter. Defines number of MSI-X vectors allocated per VF. >>> Value is permanent (stored in NVRAM), so becomes the new default >>> value for this device. >> >>Sounds like you're having this enforce the same configuration for all child >>VFs. > > Yeah, this sounds like per-port config. > Well, it gets a little tricky here. I assume some cards handle this per-port. Other cards might handle this per PF, where PF may not always correspond 1:1 with a port. And some cards maybe just allow a single value for this parameter for the entire card, covering all ports/PFs. To keep things simple and as general as possible, it made sense to set all parameters on a per-PCI device level. As I mentioned in my cover-letter, the devices most likely to use these proposed commands do not have a single "whole asic" PCI b/d/f with internal mechanism for accessing ports - most expose each port (and each function on each port) as a separate PCI b/d/f, with no separate "whole asic" PCI b/d/f. That's how the BCM cards work, and I think that's how the MLNX cards work, and others that would be likely to use these cmds. So, to summarize, you direct the command to the PCI b/d/f you want to target. Does this make sense?
Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
On Thu, Oct 19, 2017 at 4:21 PM, Yuval Mintz wrote:
>> Subject: [PATCH net-next v2 1/6] devlink: Add permanent config parameter
>> get/set operations
>>
>> Add support for permanent config parameter get/set commands. Used
>> for parameters held in NVRAM, persistent device configuration.
>
> Given some of the attributes aren't Boolean, what about an API that
> allows the user to learn of supported values per option?
> Otherwise only way for configuring some of them would be trial & error.
Interesting suggestion. There's a couple of places where this could
be a factor. (1) When a user wants to know what values are
defined/available in the API, and (2) When the user wants to know what
values are supported by a specific driver/device.
The intention for (1) is to push that into userspace. The userspace
devlink tool patches I am working on (not yet submitted) essentially
mirror the config parameters and their options, with string "keywords"
associated with each parameter and option, since it's the userspace
app that will be parsing the command line strings and converting to
API enums. So the userspace app can provide the list of
parameters/options it supports, which could be a subset of what's
available in the API.
For (2), currently there is no mechanism other than trial/error as you
suggest (up to driver to either return an error or else make use of
the value specified by the user). We could contemplate adding such a
mechanism, but it's a little complicated as some options take a range
(i.e. # of VFs per PF for example), and others may take one of a set
of enumerated values (pre-boot link speed for example).
To clarify, are you suggesting some mechanism to allow a driver to
report which parameters and options it supports (case (2))? Or are
you suggesting something in the kernel API to handle case (1) above?
>
>>
>> Signed-off-by: Steve Lin
>> Acked-by: Andy Gospodarek
>> ---
>> include/net/devlink.h| 3 +
>> include/uapi/linux/devlink.h | 11 ++
>> net/core/devlink.c | 234
>> +++
>> 3 files changed, 248 insertions(+)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index b9654e1..bd64623 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -270,6 +270,9 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
>> encap_mode);
>> + int (*perm_config_get)(struct devlink *devlink, u32 param, u32
>> *value);
>> + int (*perm_config_set)(struct devlink *devlink, u32 param, u32
>> value,
>> +u8 *restart_reqd);
>> };
>>
>> static inline void *devlink_priv(struct devlink *devlink)
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 0cbca96..47cc584 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -70,6 +70,10 @@ enum devlink_command {
>> DEVLINK_CMD_DPIPE_HEADERS_GET,
>> DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
>>
>> + /* Permanent (NVRAM) device config get/set */
>> + DEVLINK_CMD_PERM_CONFIG_GET,
>> + DEVLINK_CMD_PERM_CONFIG_SET,
>> +
>> /* add new commands above here */
>> __DEVLINK_CMD_MAX,
>> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
>> @@ -202,6 +206,13 @@ enum devlink_attr {
>>
>> DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
>>
>> + /* Permanent Configuration Parameters */
>> + DEVLINK_ATTR_PERM_CONFIGS, /* nested */
>> + DEVLINK_ATTR_PERM_CONFIG, /* nested */
>> + DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
>> + DEVLINK_ATTR_PERM_CONFIG_VALUE, /*
>> u32 */
>> + DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
>> +
>> /* 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 7d430c1..c2cc7c6 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -1566,6 +1566,224 @@ static int
>> devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
>> return 0;
>> }
>>
>> +static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
>> +
>> +static int devlink_nl_single_param_get(struct sk_buff *msg,
>
Re: [PATCH net-next v2 6/6] bnxt: Add devlink support for config get/set
On Thu, Oct 19, 2017 at 3:35 PM, Jiri Pirko wrote: > Thu, Oct 19, 2017 at 09:17:10PM CEST, steven.l...@broadcom.com wrote: >>Implements get and set of configuration parameters using new devlink >>config get/set API. > > Please split this patch too. One to introduce the infra, one per each > config option. Ok, will do in v3, thanks.
Re: [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param
On Thu, Oct 19, 2017 at 3:33 PM, Jiri Pirko wrote:
> Thu, Oct 19, 2017 at 09:17:06PM CEST, steven.l...@broadcom.com wrote:
>>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>>parameter. If value is 1, SR-IOV is enabled. If value is 0,
>>SR-IOV is disabled on this device. Value is permanent (stored
>>in NVRAM), so becomes the new default value for this device.
>>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/uapi/linux/devlink.h | 5 +
>> 1 file changed, 5 insertions(+)
>>
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index 47cc584..2640203 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
>> DEVLINK_DPIPE_HEADER_IPV6,
>> };
>>
>>+/* Permanent (NVRAM) config parameters */
>
> We need the decription here in the header as well. Commit message alone
> is no good for this.
>
> Also, there should not be mention of "NVRAM". It is up to the device
> implementation where is stores the value.
>
>
Will do, I'll add descriptions in the header file and reword as
requested. Thanks!
[PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
Add support for permanent config parameter get/set commands. Used
for parameters held in NVRAM, persistent device configuration.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 3 +
include/uapi/linux/devlink.h | 11 ++
net/core/devlink.c | 234 +++
3 files changed, 248 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..bd64623 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,9 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
+ u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..47cc584 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,10 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ /* Permanent (NVRAM) device config get/set */
+ DEVLINK_CMD_PERM_CONFIG_GET,
+ DEVLINK_CMD_PERM_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +206,13 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CONFIGS, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_VALUE, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
+
/* 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 7d430c1..c2cc7c6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,224 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static int devlink_nl_single_param_get(struct sk_buff *msg,
+ struct devlink *devlink,
+ uint32_t param)
+{
+ u32 value;
+ int err;
+ const struct devlink_ops *ops = devlink->ops;
+ struct nlattr *param_attr;
+
+ err = ops->perm_config_get(devlink, param, &value);
+ if (err)
+ return err;
+
+ param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
+ nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, value);
+ nla_nest_end(msg, param_attr);
+
+ return 0;
+}
+
+static int devlink_nl_config_get_fill(struct sk_buff *msg,
+ struct devlink *devlink,
+ enum devlink_command cmd,
+ struct genl_info *info)
+{
+ void *hdr;
+ int err;
+ struct nlattr *attr;
+ int param_count = 0;
+ struct nlattr *cfgparam_attr;
+ int rem;
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
+ u32 param;
+
+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
+ &devlink_nl_family, 0, cmd);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto nla_msg_failure;
+ }
+
+ err = devlink_nl_put_handle(msg, devlink);
+ if (err)
+ goto nla_put_failure;
+
+ if (!info->attrs[DEVLINK_ATTR_PERM_CONFIGS]) {
+ /* No configuration parameters */
+ goto nla_put_failure;
+ }
+
+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
+
+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS],
+ rem) {
+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
+ devlink_nl_policy, NULL);
+ if (err)
+ goto nla_nest_failure;
+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER])
+ continue;
+
+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
+ err = devlink_nl_single_param_get(msg, devlink, param);
+ if (err)
+ goto nla_nest_failure;
+ param_count++;
+ }
+
+ nla_nest_end(msg, cfgparam_at
[PATCH net-next v2 4/6] devlink: Adding max PF MSI-X vectors NVRAM config param
Adding DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT permanent config
parameter. Sets the maximum number of PF MSI-X (Message
Signaled Interrupts) vectors. Value is permanent (stored in
NVRAM), so becomes the new default value for this device.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 09231e1..8ad6c63 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -259,6 +259,7 @@ enum devlink_dpipe_header_id {
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
+ DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
[PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param
Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
parameter. If value is 1, SR-IOV is enabled. If value is 0,
SR-IOV is disabled on this device. Value is permanent (stored
in NVRAM), so becomes the new default value for this device.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 47cc584..2640203 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
+/* Permanent (NVRAM) config parameters */
+enum devlink_perm_config_param {
+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+};
+
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
[PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent config
parameter. Defines number of MSI-X vectors allocated per VF.
Value is permanent (stored in NVRAM), so becomes the new default
value for this device.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 8ad6c63..ef163b6 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -260,6 +260,7 @@ enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
+ DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
[PATCH net-next v2 0/6] Adding permanent config get/set to devlink
Changes since v1, based on the excellent feedback received: * Implemented nested parameters correctly this time, I think. * Submitting config get/set infrastructure separately from the parameters themselves, and then submitting just the first four parameters as separate patches. Once this approach is accepted, I will add additional parameters, taking into account comments received on them. * Changed devlink_nl_sing_param_get/set to use _single_. * Tried to make clear that all params using this command are permanent / NVRAM settings, not transient. * Split out the reorganization of bnxt driver to separate patch, submitted to net-next earlier today. * One non-change: The devices this change affects don't typically have a separate 'asic' pci b/d/f versus per-port b/d/f; they just have (typically multiple) b/d/f entities for each function on the device. So, doesn't seem to me like splitting these parameters into port vs. device params works here. Adds a devlink command for getting & setting permanent (persistent / NVRAM) device configuration parameters, and enumerates the parameters as nested devlink attributes. bnxt driver patch makes use of these new devlink cmds. Steve Lin (6): devlink: Add permanent config parameter get/set operations devlink: Adding SR-IOV enablement NVRAM config param devlink: Adding num VFs per PF NVRAM config param devlink: Adding max PF MSI-X vectors NVRAM config param devlink: Adding num MSI-X vectors per VF NVRAM config param bnxt: Add devlink support for config get/set drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 245 +- drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 + include/net/devlink.h | 3 + include/uapi/linux/devlink.h | 19 ++ net/core/devlink.c| 234 + 6 files changed, 612 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH net-next v2 3/6] devlink: Adding num VFs per PF NVRAM config param
Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
parameter, which sets the number of VFs per PF in SR-IOV
mode. Value is permanent (stored in NVRAM), so becomes the
new default value for this device.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2640203..09231e1 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -258,6 +258,7 @@ enum devlink_dpipe_header_id {
/* Permanent (NVRAM) config parameters */
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+ DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
[PATCH net-next v2 6/6] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 245 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +
3 files changed, 356 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index f3f6aa8..88a1f1d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,11 +14,244 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
+ {DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
+};
+
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
+{
+ struct hwrm_nvm_get_variable_input req = {0};
+ void *dest_data_addr = NULL;
+ dma_addr_t dest_data_dma_addr;
+ int rc;
+ int bytesize;
+
+ bytesize = (size + 7) / 8;
+ dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &dest_data_dma_addr, GFP_KERNEL);
+ if (!dest_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
+ req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ memcpy(buf, dest_data_addr, bytesize);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, dest_data_addr,
+ dest_data_dma_addr);
+
+ return rc;
+}
+
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
+ const void *buf, int size)
+{
+ struct hwrm_nvm_set_variable_input req = {0};
+ void *src_data_addr = NULL;
+ dma_addr_t src_data_dma_addr;
+ int rc;
+ int bytesize;
+
+ bytesize = (size + 7) / 8;
+
+ src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &src_data_dma_addr, GFP_KERNEL);
+ if (!src_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ memcpy(src_data_addr, buf, bytesize);
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+ req.src_data_addr = cpu_to_le64(src_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, src_data_addr,
+ src_data_dma_addr);
+
+ return 0;
+}
+
+static int bnxt_dl_perm_config_set(struct devlink *devlink,
+ u32 param, u32 value, u8 *restart_reqd)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ int i;
+ int idx = 0;
+ void *data;
+ int ret = 0;
+ u32 bytesize;
+ struct bnxt_drv_cfgparam *entry;
+
+ *restart_reqd = 0;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (i == BNXT_NUM_DRV_CFGPARAM)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+ if (BNXT_VF(bp) && !(entry->func & BNXT_DRV_VF))
+ return -EOPNOTSUPP;
+
+ /* If parameter is per port or function, compute index
Re: [PATCH 2/7] devlink: Adding NPAR permanent config parameters
On Thu, Oct 19, 2017 at 6:39 AM, Yuval Mintz wrote: >> DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID: 1 to use >> BW_RESERVATION setting, 0 to ignore. >> > ... >> DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID: 1 to use BW_LIMIT >> setting, 0 to ignore. > > While it probably ties to different fields in your NVM layout why would the > user > require specific attributes for these? Why not have values in the actual > attributes indicating of this status? Hi Yuval, Does having the separate valid flag present any difficulties? There are lots of implementation options here (a limit or reservation value of 0 could mean invalid, or we could define (1 << 31) to be a valid flag when setting the value, etc.), and I'm not necessarily tied to doing it this way, but it seemed a straightforward way to represent the validity of the other field. Thanks again, Steve
Re: [PATCH 4/7] devlink: Adding perm config of link settings
On Thu, Oct 19, 2017 at 2:07 AM, Yuval Mintz wrote:
>> +enum devlink_autoneg_protocol {
>> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_BAM,
>> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_CONSORTIUM,
>> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY,
>> + DEVLINK_AUTONEG_PROTOCOL_BAM, /* Broadcom
>> Autoneg Mode */
>> + DEVLINK_AUTONEG_PROTOCOL_CONSORTIUM,/*
>> Consortium Autoneg Mode */
>> +};
>
> Wouldn't adding BAM as a 'generic' mode of operation be like adding
> non-consortium speeds to ethtool API?
> [I profess ignorance in this area; For all I know it can be a widely accepted
> industry standard]
>
Yuval, I'm glad to get input from other NIC vendors. The high-level
goal of this effort is to allow users of various vendors' NICs to be
able to configure these types of NVRAM/permanent/default settings
using an inbox tool, rather than the collection of vendor-specific
tools that is the status quo.
In order to provide that functionality, it seems like the
vendor-specific parameters and also the vendor-specific settings of
common parameters both need to be supported in this manner.
Ideally there will be much overlap in both the set of parameters
available as well as the options for each parameter, but in the real
world, there will always be differences between vendors and even
between different devices (drivers) from the same vendor. Despite
that reality, I think there is still great benefit in having a common
inbox tool that users can use for device configuration of this type.
It just means that not all drivers will support all parameters, nor
all options for each parameter that they do support.
Thanks,
Steve
[PATCH net-next] bnxt: Move generic devlink code to new file
Moving generic devlink code (registration) out of VF-R code
into new bnxt_devlink file, in preparation for future work
to add additional devlink functionality to bnxt.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 65 +++
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 39 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 53 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 37 ++---
6 files changed, 112 insertions(+), 85 deletions(-)
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
diff --git a/drivers/net/ethernet/broadcom/bnxt/Makefile
b/drivers/net/ethernet/broadcom/bnxt/Makefile
index 457201f..59c8ec9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/Makefile
+++ b/drivers/net/ethernet/broadcom/bnxt/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_BNXT) += bnxt_en.o
-bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o bnxt_devlink.o
bnxt_en-$(CONFIG_BNXT_FLOWER_OFFLOAD) += bnxt_tc.o
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5ba4993..52cc38d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -61,6 +61,7 @@
#include "bnxt_xdp.h"
#include "bnxt_vfr.h"
#include "bnxt_tc.h"
+#include "bnxt_devlink.h"
#define BNXT_TX_TIMEOUT(5 * HZ)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
new file mode 100644
index 000..f3f6aa8
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -0,0 +1,65 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include
+#include
+#include "bnxt_hsi.h"
+#include "bnxt.h"
+#include "bnxt_vfr.h"
+#include "bnxt_devlink.h"
+
+static const struct devlink_ops bnxt_dl_ops = {
+#ifdef CONFIG_BNXT_SRIOV
+ .eswitch_mode_set = bnxt_dl_eswitch_mode_set,
+ .eswitch_mode_get = bnxt_dl_eswitch_mode_get,
+#endif /* CONFIG_BNXT_SRIOV */
+};
+
+int bnxt_dl_register(struct bnxt *bp)
+{
+ struct devlink *dl;
+ int rc;
+
+ if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
+ return 0;
+
+ if (bp->hwrm_spec_code < 0x10800) {
+ netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch
SWITCHDEV mode.\n");
+ return -ENOTSUPP;
+ }
+
+ dl = devlink_alloc(&bnxt_dl_ops, sizeof(struct bnxt_dl));
+ if (!dl) {
+ netdev_warn(bp->dev, "devlink_alloc failed");
+ return -ENOMEM;
+ }
+
+ bnxt_link_bp_to_dl(bp, dl);
+ bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
+ rc = devlink_register(dl, &bp->pdev->dev);
+ if (rc) {
+ bnxt_link_bp_to_dl(bp, NULL);
+ devlink_free(dl);
+ netdev_warn(bp->dev, "devlink_register failed. rc=%d", rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+void bnxt_dl_unregister(struct bnxt *bp)
+{
+ struct devlink *dl = bp->dl;
+
+ if (!dl)
+ return;
+
+ devlink_unregister(dl);
+ devlink_free(dl);
+}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
new file mode 100644
index 000..e92a35d
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -0,0 +1,39 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef BNXT_DEVLINK_H
+#define BNXT_DEVLINK_H
+
+/* Struct to hold housekeeping info needed by devlink interface */
+struct bnxt_dl {
+ struct bnxt *bp;/* back ptr to the controlling dev */
+};
+
+static inline struct bnxt *bnxt_get_bp_from_dl(struct devlink *dl)
+{
+ return ((struct bnxt_dl *)devlink_priv(dl))->bp;
+}
+
+/* To clear devlink pointer from bp, pass NULL dl */
+static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
+{
+ bp->dl = dl;
+
+ /* add a back pointer in dl to bp */
+
Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
On Wed, Oct 18, 2017 at 11:22 AM, Or Gerlitz wrote:
> On Tue, Oct 17, 2017 at 11:44 PM, Steve Lin
> wrote:
>>
>> Add support for permanent config parameter get/set commands. Used
>> for parameters held in NVRAM, persistent device configuration.
>> The config_get() and config_set() operations operate as expected, but
>> note that the driver implementation of the config_set() operation can
>> indicate whether a restart is necessary for the setting to take
>> effect. This indication of a necessary restart is passed via the
>> DEVLINK_ATTR_PERM_CFG_RESTART_REQUIRED attribute.
>>
>> First set of parameters defined are PCI SR-IOV and per-VF
>> configuration:
>>
>> DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED: Enable SR-IOV capability.
>> DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF: Maximum number of VFs per PF, in
>> SR-IOV mode.
>> DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT: Maximum number of
>> MSI-X vectors assigned per PF.
>> DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF: Number of MSI-X vectors
>> allocated per VF.
>>
>> Signed-off-by: Steve Lin
>> Acked-by: Andy Gospodarek
>> ---
>> include/net/devlink.h| 4 +
>> include/uapi/linux/devlink.h | 20
>> net/core/devlink.c | 266
>> +++
>> 3 files changed, 290 insertions(+)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index b9654e1..952966c 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -270,6 +270,10 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
>> encap_mode);
>> + int (*config_get)(struct devlink *devlink, enum devlink_attr attr,
>> + u32 *value);
>> + int (*config_set)(struct devlink *devlink, enum devlink_attr attr,
>> + u32 value, u8 *restart_reqd);
>> };
>>
>> static inline void *devlink_priv(struct devlink *devlink)
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 0cbca96..34de44d 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -70,6 +70,9 @@ enum devlink_command {
>> DEVLINK_CMD_DPIPE_HEADERS_GET,
>> DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
>>
>> + DEVLINK_CMD_CONFIG_GET,
>> + DEVLINK_CMD_CONFIG_SET,
>> +
>> /* add new commands above here */
>> __DEVLINK_CMD_MAX,
>> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
>> @@ -202,6 +205,23 @@ enum devlink_attr {
>>
>> DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
>>
>> + /* Permanent Configuration Parameters */
>> + DEVLINK_ATTR_PERM_CFG, /* nested */
>> +
>> + /* When config doesn't take effect until next reboot (config
>> +* just changed NVM which isn't read until boot, for example),
>> +* this attribute should be set by the driver.
>> +*/
>> + DEVLINK_ATTR_PERM_CFG_RESTART_REQUIRED, /* u8 */
>> + DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED,/* u8 */
>> + DEVLINK_ATTR_PERM_CFG_FIRST = DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED,
>
>
> ??? can you explain / document this one?
I attempted to explain in the patch commit message, but I will try to
add more detail when I resubmit, with each attribute in a separate
patch.
>
> I would join Jiri's request to have patch #1 not defining any attributes,
> review will be much
> easier and robust.
Ok, I'll do that when I resubmit.
>
> Talking on attributes... what is your plan for vendor specific attributes?
>
They will be added just like common attributes, any given driver
doesn't have to support all attributes.
Steve
Re: [PATCH 4/7] devlink: Adding perm config of link settings
On Wed, Oct 18, 2017 at 9:01 AM, Jiri Pirko wrote: > Wed, Oct 18, 2017 at 02:39:42PM CEST, steven.l...@broadcom.com wrote: >>On Wed, Oct 18, 2017 at 3:31 AM, Jiri Pirko wrote: >>> >>> You need to split the config option to those that are per-port and to >>> those that are per-asic. For each family, you have to use ither >>> devlink_port of devlink handle. Also, you need to split into those that are >>> permanent and to those who are teporary (until reset). I think you might >>> need some flags for that. >>> >> >>All these are permanent; none are temporary - that's (partially) why >>we consider these to be devlink/device parameters rather than a >>netdev/ethtool thing, since they take effect after the next reset and >>before any drivers are loaded (i.e. the card uses these parameters for >>its default/startup configuration). > > Understood. But I think that this iface should be capable to serve the > options of non-permanent as well. Or this could be 2 separate interfaces > with 2 separate cmd pair. Thoughts? > I would prefer to keep this command for permanent config only, and use a separate command for transient configuration. I think that transient device configuration should be tackled in the separate discussion that was started in the RFC version of this patchset, related to moving ethtool ops to devlink/netlink. I think that's a separate topic that requires a little more thought and discussion, but really isn't that related to this current patchset for permanent device configuration. What do you think? Steve
Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
On Wed, Oct 18, 2017 at 8:58 AM, Jiri Pirko wrote: > Wed, Oct 18, 2017 at 02:39:09PM CEST, steven.l...@broadcom.com wrote: >>On Wed, Oct 18, 2017 at 3:11 AM, Jiri Pirko wrote: >>> Tue, Oct 17, 2017 at 10:44:23PM CEST, steven.l...@broadcom.com wrote: >>> Steve. As I originally requested, could you please split this to: >>> 1) single patch adding config get/set commands, without any config >>> attributes >>> 2) single patch per config attribute - please don't add them in bulk. >>>We also need very strict description for every single attribute so >>>other vendors know what it is and can re-use it. There is need to >>>avoid duplication here. Also, please send just few attribites in the >>>first run, not like 40 you are sending now. Impossible to review. >> >>I broke the patch set up into functional blocks of attributes, in >>order to avoid having ~40 patches of just a couple lines each. But, I >>will split further for each individual attribute, and just submit a >>few initially, per your request. >> >>> >>> Also, why didn't you put it into nested attribute we were discussing? >>> >> >>I thought I did :) , using the DPIPE_HEADERS nested attribute as an >>example. I'll reach out to you off-list to understand what I'm >>missing. > > I missed that. But you need a separate attr enum as well. > I did have this as the nested attr enum in the original patch: /* Permanent Configuration Parameters */ DEVLINK_ATTR_PERM_CFG, /* nested */ However, I only used the nested construct in the response from kernel to userspace, not in the request from userspace to kernel. (This was based on looking at the various DPIPE_* nested attributes as examples.) Thinking about it after seeing your comment, I'm thinking I should also use the nested attribute construct in the original request from userspace to kernel as well, although I didn't see any previous examples of this in devlink. So I'll plan to use nesting in that direction as well. Thanks, Steve
Re: [PATCH 4/7] devlink: Adding perm config of link settings
On Wed, Oct 18, 2017 at 3:31 AM, Jiri Pirko wrote: > > You need to split the config option to those that are per-port and to > those that are per-asic. For each family, you have to use ither > devlink_port of devlink handle. Also, you need to split into those that are > permanent and to those who are teporary (until reset). I think you might > need some flags for that. > All these are permanent; none are temporary - that's (partially) why we consider these to be devlink/device parameters rather than a netdev/ethtool thing, since they take effect after the next reset and before any drivers are loaded (i.e. the card uses these parameters for its default/startup configuration). I will take a closer look at splitting these between per-port and per-asic. Thanks, Steve
Re: [PATCH 6/7] bnxt: Move generic devlink code to new file
On Wed, Oct 18, 2017 at 3:33 AM, Jiri Pirko wrote: > Tue, Oct 17, 2017 at 10:44:28PM CEST, steven.l...@broadcom.com wrote: >>Moving generic devlink code (registration) out of VR-R code >>into new bnxt_devlink file. > > You can send this patch separatelly and let it be applied before the > patchset. Ok, will do. Thanks again for all the feedback, Jiri. Steve
Re: [PATCH 1/7] devlink: Add permanent config parameter get/set operations
On Wed, Oct 18, 2017 at 3:11 AM, Jiri Pirko wrote: > Tue, Oct 17, 2017 at 10:44:23PM CEST, steven.l...@broadcom.com wrote: > Steve. As I originally requested, could you please split this to: > 1) single patch adding config get/set commands, without any config attributes > 2) single patch per config attribute - please don't add them in bulk. >We also need very strict description for every single attribute so >other vendors know what it is and can re-use it. There is need to >avoid duplication here. Also, please send just few attribites in the >first run, not like 40 you are sending now. Impossible to review. I broke the patch set up into functional blocks of attributes, in order to avoid having ~40 patches of just a couple lines each. But, I will split further for each individual attribute, and just submit a few initially, per your request. > > Also, why didn't you put it into nested attribute we were discussing? > I thought I did :) , using the DPIPE_HEADERS nested attribute as an example. I'll reach out to you off-list to understand what I'm missing. >> >>+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1]; >>+ >>+static int devlink_nl_sing_param_get(struct sk_buff *msg, > > I was wondering what song it will sing :) Just add "le", it's just 2 > chars :) > Will do, thanks. ;) Steve
Re: [PATCH 0/7] Adding permanent config get/set to devlink
My apologies - this patchset was intended for net-next; I forgot to add that to the subject line, though. Steve On Tue, Oct 17, 2017 at 4:44 PM, Steve Lin wrote: > DIFFERENCES FROM RFC: > Implemented most of the changes suggested by Jiri and others. > Thanks for the valuable feedback! > > Adds a devlink command for getting & setting permanent > (persistent / NVRAM) device configuration parameters, and > enumerates the parameters as nested devlink attributes. > > bnxt driver patches make use of these new devlink cmds/ > attributes. > > Steve Lin (7): > devlink: Add permanent config parameter get/set operations > devlink: Adding NPAR permanent config parameters > devlink: Adding high level dev perm config params > devlink: Adding perm config of link settings > devlink: Adding pre-boot permanent config parameters > bnxt: Move generic devlink code to new file > bnxt: Add devlink support for config get/set > > drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + > drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 363 > ++ > drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 56 > drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 ++ > drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 53 +--- > drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 37 +-- > include/net/devlink.h | 4 + > include/uapi/linux/devlink.h | 113 +++ > net/core/devlink.c| 300 ++ > 10 files changed, 944 insertions(+), 85 deletions(-) > create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c > create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h > > -- > 2.7.4 >
[PATCH 1/7] devlink: Add permanent config parameter get/set operations
Add support for permanent config parameter get/set commands. Used
for parameters held in NVRAM, persistent device configuration.
The config_get() and config_set() operations operate as expected, but
note that the driver implementation of the config_set() operation can
indicate whether a restart is necessary for the setting to take
effect. This indication of a necessary restart is passed via the
DEVLINK_ATTR_PERM_CFG_RESTART_REQUIRED attribute.
First set of parameters defined are PCI SR-IOV and per-VF
configuration:
DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED: Enable SR-IOV capability.
DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF: Maximum number of VFs per PF, in
SR-IOV mode.
DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT: Maximum number of
MSI-X vectors assigned per PF.
DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF: Number of MSI-X vectors
allocated per VF.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 4 +
include/uapi/linux/devlink.h | 20
net/core/devlink.c | 266 +++
3 files changed, 290 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..952966c 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,10 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*config_get)(struct devlink *devlink, enum devlink_attr attr,
+ u32 *value);
+ int (*config_set)(struct devlink *devlink, enum devlink_attr attr,
+ u32 value, u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..34de44d 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,9 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ DEVLINK_CMD_CONFIG_GET,
+ DEVLINK_CMD_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +205,23 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CFG, /* nested */
+
+ /* When config doesn't take effect until next reboot (config
+* just changed NVM which isn't read until boot, for example),
+* this attribute should be set by the driver.
+*/
+ DEVLINK_ATTR_PERM_CFG_RESTART_REQUIRED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED,/* u8 */
+ DEVLINK_ATTR_PERM_CFG_FIRST = DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED,
+ DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF, /* u32 */
+
+ /* Add new permanent config parameters above here */
+ DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF,
+
/* 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 7d430c1..427a65e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,254 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static int devlink_nl_sing_param_get(struct sk_buff *msg,
+struct devlink *devlink,
+enum devlink_attr attr)
+{
+ struct nla_policy policy;
+ u32 value;
+ int err;
+ const struct devlink_ops *ops = devlink->ops;
+
+ policy = devlink_nl_policy[attr];
+ err = ops->config_get(devlink, attr, &value);
+ if (err)
+ return err;
+
+ switch (policy.type) {
+ case NLA_U8:
+ err = nla_put_u8(msg, attr, value);
+ break;
+ case NLA_U16:
+ err = nla_put_u16(msg, attr, value);
+ break;
+ case NLA_U32:
+ err = nla_put_u32(msg, attr, value);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int devlink_nl_config_get_fill(struct sk_buff *msg,
+ struct devlink *devlink,
+ enum devlink_command cmd,
+ struct genl_info *info)
+{
+ const struct devlink_ops *ops = devlink->ops;
+ void *hdr;
+ int err;
+ enum devlink_attr attr;
+
[PATCH 5/7] devlink: Adding pre-boot permanent config parameters
Extending DEVLINK_ATTR_PERM_CFG (permanent/NVRAM device configuration)
to include some pre-boot device configuration settings:
DEVLINK_ATTR_PERM_CFG_MBA_ENABLED: 1 to enable Multiple Boot
Agent (BMA), 0 to disable.
DEVLINK_ATTR_PERM_CFG_MBA_BOOT_TYPE: Controls mechanism MBA will
use to insert itself into the list of devices recognized by the
BIOS; use enum devlink_mba_boot_type.
DEVLINK_ATTR_PERM_CFG_MBA_DELAY_TIME: Controls how long MBA
banner display and ability to enter MBA setup will persist
during initialization, in seconds.
DEVLINK_ATTR_PERM_CFG_MBA_SETUP_HOT_KEY: Configures which hot-key
will be used to enter MBA setup; use enum devlink_mba_setup_hot_key.
DEVLINK_ATTR_PERM_CFG_MBA_HIDE_SETUP_PROMPT: 1 to enable hiding
of 'enter setup' prompt during initialization, 0 to disable.
DEVLINK_ATTR_PERM_CFG_MBA_BOOT_RETRY_COUNT: MBA retries booting
this number of times, if it fails initially.
DEVLINK_ATTR_PERM_CFG_MBA_VLAN_ENABLED: 1 to enable using VLAN
when executing MBA host software (PXE/iSCSI), 0 to disable.
DEVLINK_ATTR_PERM_CFG_MBA_VLAN_TAG: The 16 bit VLAN tag to use
if MBA_VLAN_ENABLED is set.
DEVLINK_ATTR_PERM_CFG_MBA_BOOT_PROTOCOL: Selects MBA boot
protocol; use enum devlink_mba_boot_protocol.
DEVLINK_ATTR_PERM_CFG_MBA_LINK_SPEED: Configured link speed
while executing MBA host software (PXI/iSCSI); use enum
devlink_mba_link_speed.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 39 ++-
net/core/devlink.c | 10 ++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2e1c006..609784a 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -162,6 +162,33 @@ enum devlink_pre_os_link_speed {
DEVLINK_PRE_OS_LINK_SPEED_100M = 0xf,
};
+enum devlink_mba_boot_type {
+ DEVLINK_MBA_BOOT_TYPE_AUTO_DETECT,
+ DEVLINK_MBA_BOOT_TYPE_BBS, /* BIOS Boot Specification */
+ DEVLINK_MBA_BOOT_TYPE_INTR18, /* Hook interrupt 0x18 */
+ DEVLINK_MBA_BOOT_TYPE_INTR19, /* Hook interrupt 0x19 */
+};
+
+enum devlink_mba_setup_hot_key {
+ DEVLINK_MBA_SETUP_HOT_KEY_CTRL_S,
+ DEVLINK_MBA_SETUP_HOT_KEY_CTRL_B,
+};
+
+enum devlink_mba_boot_protocol {
+ DEVLINK_MBA_BOOT_PROTOCOL_PXE,
+ DEVLINK_MBA_BOOT_PROTOCOL_ISCSI,
+ DEVLINK_MBA_BOOT_PROTOCOL_NONE = 0x7,
+};
+
+enum devlink_mba_link_speed {
+ DEVLINK_MBA_LINK_SPEED_AUTONEG,
+ DEVLINK_MBA_LINK_SPEED_1G,
+ DEVLINK_MBA_LINK_SPEED_10G,
+ DEVLINK_MBA_LINK_SPEED_25G,
+ DEVLINK_MBA_LINK_SPEED_40G,
+ DEVLINK_MBA_LINK_SPEED_50G,
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -274,9 +301,19 @@ enum devlink_attr {
DEVLINK_ATTR_PERM_CFG_PHY_SELECT, /* u8 */
DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0, /* u32 */
DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_MBA_BOOT_TYPE,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_DELAY_TIME, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_SETUP_HOT_KEY,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_HIDE_SETUP_PROMPT,/* u8 */
+ DEVLINK_ATTR_PERM_CFG_MBA_BOOT_RETRY_COUNT, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_VLAN_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_MBA_VLAN_TAG, /* u16 */
+ DEVLINK_ATTR_PERM_CFG_MBA_BOOT_PROTOCOL,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_MBA_LINK_SPEED, /* u32 */
/* Add new permanent config parameters above here */
- DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3,
+ DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_MBA_LINK_SPEED,
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 80a2a50..2eaa566 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2567,6 +2567,16 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_PERM_CFG_PHY_SELECT] = { .type = NLA_U8 },
[DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0] = { .type = NLA_U32 },
[DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_BOOT_TYPE] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_DELAY_TIME] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_SETUP_HOT_KEY] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_HIDE_SETUP_PROMPT] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_BOOT_RETRY_COUNT] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MBA_VLAN_ENABLED] = {
[PATCH 6/7] bnxt: Move generic devlink code to new file
Moving generic devlink code (registration) out of VR-R code
into new bnxt_devlink file.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 65 +++
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 39 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 53 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 37 ++---
6 files changed, 112 insertions(+), 85 deletions(-)
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
diff --git a/drivers/net/ethernet/broadcom/bnxt/Makefile
b/drivers/net/ethernet/broadcom/bnxt/Makefile
index 457201f..59c8ec9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/Makefile
+++ b/drivers/net/ethernet/broadcom/bnxt/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_BNXT) += bnxt_en.o
-bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o bnxt_devlink.o
bnxt_en-$(CONFIG_BNXT_FLOWER_OFFLOAD) += bnxt_tc.o
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5ba4993..52cc38d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -61,6 +61,7 @@
#include "bnxt_xdp.h"
#include "bnxt_vfr.h"
#include "bnxt_tc.h"
+#include "bnxt_devlink.h"
#define BNXT_TX_TIMEOUT(5 * HZ)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
new file mode 100644
index 000..f3f6aa8
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -0,0 +1,65 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include
+#include
+#include "bnxt_hsi.h"
+#include "bnxt.h"
+#include "bnxt_vfr.h"
+#include "bnxt_devlink.h"
+
+static const struct devlink_ops bnxt_dl_ops = {
+#ifdef CONFIG_BNXT_SRIOV
+ .eswitch_mode_set = bnxt_dl_eswitch_mode_set,
+ .eswitch_mode_get = bnxt_dl_eswitch_mode_get,
+#endif /* CONFIG_BNXT_SRIOV */
+};
+
+int bnxt_dl_register(struct bnxt *bp)
+{
+ struct devlink *dl;
+ int rc;
+
+ if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
+ return 0;
+
+ if (bp->hwrm_spec_code < 0x10800) {
+ netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch
SWITCHDEV mode.\n");
+ return -ENOTSUPP;
+ }
+
+ dl = devlink_alloc(&bnxt_dl_ops, sizeof(struct bnxt_dl));
+ if (!dl) {
+ netdev_warn(bp->dev, "devlink_alloc failed");
+ return -ENOMEM;
+ }
+
+ bnxt_link_bp_to_dl(bp, dl);
+ bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
+ rc = devlink_register(dl, &bp->pdev->dev);
+ if (rc) {
+ bnxt_link_bp_to_dl(bp, NULL);
+ devlink_free(dl);
+ netdev_warn(bp->dev, "devlink_register failed. rc=%d", rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+void bnxt_dl_unregister(struct bnxt *bp)
+{
+ struct devlink *dl = bp->dl;
+
+ if (!dl)
+ return;
+
+ devlink_unregister(dl);
+ devlink_free(dl);
+}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
new file mode 100644
index 000..e92a35d
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -0,0 +1,39 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef BNXT_DEVLINK_H
+#define BNXT_DEVLINK_H
+
+/* Struct to hold housekeeping info needed by devlink interface */
+struct bnxt_dl {
+ struct bnxt *bp;/* back ptr to the controlling dev */
+};
+
+static inline struct bnxt *bnxt_get_bp_from_dl(struct devlink *dl)
+{
+ return ((struct bnxt_dl *)devlink_priv(dl))->bp;
+}
+
+/* To clear devlink pointer from bp, pass NULL dl */
+static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
+{
+ bp->dl = dl;
+
+ /* add a back pointer in dl to bp */
+ if (dl) {
+ struct bnxt_dl *bp_dl = devlink_priv(dl);
+
+
[PATCH 7/7] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 310 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +++
3 files changed, 421 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index f3f6aa8..e247cae 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,11 +14,309 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_ATTR_PERM_CFG_IGNORE_ARI_CAPABILITY, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 164},
+ {DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 166},
+ {DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_BRIDGE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 269},
+ {DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 270},
+ {DEVLINK_ATTR_PERM_CFG_SECURE_NIC_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 162},
+ {DEVLINK_ATTR_PERM_CFG_PHY_SELECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 329},
+ {DEVLINK_ATTR_PERM_CFG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
+
+ {DEVLINK_ATTR_PERM_CFG_MBA_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 351},
+ {DEVLINK_ATTR_PERM_CFG_MBA_BOOT_TYPE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 2, 352},
+ {DEVLINK_ATTR_PERM_CFG_MBA_DELAY_TIME, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 4, 353},
+ {DEVLINK_ATTR_PERM_CFG_MBA_SETUP_HOT_KEY, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 354},
+ {DEVLINK_ATTR_PERM_CFG_MBA_HIDE_SETUP_PROMPT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 355},
+ {DEVLINK_ATTR_PERM_CFG_MBA_VLAN_TAG, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 16, 357},
+ {DEVLINK_ATTR_PERM_CFG_MBA_VLAN_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 358},
+ {DEVLINK_ATTR_PERM_CFG_MBA_LINK_SPEED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 4, 359},
+ {DEVLINK_ATTR_PERM_CFG_MBA_BOOT_RETRY_COUNT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 3, 360},
+ {DEVLINK_ATTR_PERM_CFG_MBA_BOOT_PROTOCOL, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 3, 361},
+ {DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 501},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 502},
+ {DEVLINK_ATTR_PERM_CFG_RDMA_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 506},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_BW_IN_PERCENT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 507},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 508},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 509},
+
+ {DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 152},
+ {DEVLINK_ATTR_PERM_CFG_DCBX_MODE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 155},
+ {DEVLINK_ATTR_PERM_CFG_MULTIFUNC_MODE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 5, 157},
+ {DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 205},
+ {DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 208},
+ {DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 210},
+ {DEVLINK_ATTR_PERM_CFG_MEDIA_AUTO_DETECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 213},
+ {DEVLINK_ATTR_PERM_CFG_AUTONEG_PROTOCOL, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 8, 312},
+ {DEVLINK_ATTR_PERM_CFG_NPAR_NUM_PARTITIONS_PER_PORT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 8, 503},
+};
+
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
+{
+ struct hwrm_nvm_get_var
[PATCH 3/7] devlink: Adding high level dev perm config params
Extending DEVLINK_ATTR_PERM_CFG (permanent/NVRAM device configuration)
to include some high level device configuration settings:
DEVLINK_ATTR_PERM_CFG_DCBX_MODE: Data Center Bridging Exchange
(DCBX) protocol mode; use enum devlink_dcbx_mode.
DEVLINK_ATTR_PERM_CFG_RDMA_ENABLED: 1 to enable RDMA, 0 to disable.
DEVLINK_ATTR_PERM_CFG_MULTIFUNC_MODE: Configure multi-function
mode; use devlink_multifunc_mode.
DEVLINK_ATTR_PERM_CFG_SECURE_NIC_ENABLED: 1 to enable Secure NIC
functionality, 0 to disable.
DEVLINK_ATTR_PERM_CFG_IGNORE_ARI_CAPABILITY: 1 to ignore ARI
(Alternate Routing ID) interpretation, 0 to honor ARI.
DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_BRIDGE_ENABLED: 1 to enable
Link Layer Data Protocol (LLDP) on Nearest Bridge, 0 to
disable.
DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED: 1 to
enable Link Layer Data Protocol (LLDP) on Non Two Port MAC
Relay (non-TPMR) Bridge, 0 to disable.
DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED: 1 to enable Power
Management Events (PME) functionality, 0 to disable.
DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED: 1 to enable
Magic Packet Wake on Lan using ACPI pattern, 0 to disable.
DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED: 1 to enable Energy
Efficient Ethernet (EEE), 0 to disable.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 27 ++-
net/core/devlink.c | 12
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 21cfb37..4a9eafd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -127,6 +127,21 @@ enum devlink_eswitch_encap_mode {
DEVLINK_ESWITCH_ENCAP_MODE_BASIC,
};
+enum devlink_dcbx_mode {
+ DEVLINK_DCBX_MODE_DISABLED,
+ DEVLINK_DCBX_MODE_IEEE,
+ DEVLINK_DCBX_MODE_CEE,
+ DEVLINK_DCBX_MODE_IEEE_CEE,
+};
+
+enum devlink_multifunc_mode {
+ DEVLINK_MULTIFUNC_MODE_ALLOWED, /* Ext switch activates MF */
+ DEVLINK_MULTIFUNC_MODE_FORCE_SINGFUNC,
+ DEVLINK_MULTIFUNC_MODE_NPAR10, /* NPAR 1.0 */
+ DEVLINK_MULTIFUNC_MODE_NPAR15, /* NPAR 1.5 */
+ DEVLINK_MULTIFUNC_MODE_NPAR20, /* NPAR 2.0 */
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -224,9 +239,19 @@ enum devlink_attr {
DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID,/* u8 */
DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT,/* u32 */
DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_DCBX_MODE,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_RDMA_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_MULTIFUNC_MODE, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_SECURE_NIC_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_IGNORE_ARI_CAPABILITY,/* u8 */
+ DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_BRIDGE_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED, /* u8 */
/* Add new permanent config parameters above here */
- DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID,
+ DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED,
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 76bb6d4..d611154 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2550,6 +2550,18 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID] = { .type = NLA_U8 },
[DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT] = { .type = NLA_U32 },
[DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_DCBX_MODE] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_RDMA_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_MULTIFUNC_MODE] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_SECURE_NIC_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_IGNORE_ARI_CAPABILITY] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_BRIDGE_ENABLED] = {
+ .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED] = {
+ .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED] = { .type = NLA_U8 },
};
static const struct genl_ops devlink_nl_ops[] = {
--
2.7.4
[PATCH 0/7] Adding permanent config get/set to devlink
DIFFERENCES FROM RFC: Implemented most of the changes suggested by Jiri and others. Thanks for the valuable feedback! Adds a devlink command for getting & setting permanent (persistent / NVRAM) device configuration parameters, and enumerates the parameters as nested devlink attributes. bnxt driver patches make use of these new devlink cmds/ attributes. Steve Lin (7): devlink: Add permanent config parameter get/set operations devlink: Adding NPAR permanent config parameters devlink: Adding high level dev perm config params devlink: Adding perm config of link settings devlink: Adding pre-boot permanent config parameters bnxt: Move generic devlink code to new file bnxt: Add devlink support for config get/set drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 363 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 56 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 53 +--- drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 37 +-- include/net/devlink.h | 4 + include/uapi/linux/devlink.h | 113 +++ net/core/devlink.c| 300 ++ 10 files changed, 944 insertions(+), 85 deletions(-) create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h -- 2.7.4
[PATCH 4/7] devlink: Adding perm config of link settings
Extending DEVLINK_ATTR_PERM_CFG (permanent/NVRAM device configuration)
to include persistent configuration of device link settings:
DEVLINK_ATTR_PERM_CFG_AUTONEG_PROTOCOL: Configure default autoneg
protocol; use enum devlink_autoneg_protocol.
DEVLINK_ATTR_PERM_CFG_MEDIA_AUTO_DETECT: Configure default
auto-detection of attached media connector (1 = enable, 0 =
disable).
DEVLINK_ATTR_PERM_CFG_PHY_SELECT: Configure default external PHY
selection (0 = PHY 0, 1 = PHY 1).
DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0: Configure default
pre-OS link speed in full power (D0) state; use enum
devlink_pre_os_link_speed.
DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3: Configure default
pre-OS link speed in sleep (D3) state; use enum
devlink_pre_os_link_speed.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 27 ++-
net/core/devlink.c | 5 +
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 4a9eafd..2e1c006 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -142,6 +142,26 @@ enum devlink_multifunc_mode {
DEVLINK_MULTIFUNC_MODE_NPAR20, /* NPAR 2.0 */
};
+enum devlink_autoneg_protocol {
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_BAM,
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_CONSORTIUM,
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY,
+ DEVLINK_AUTONEG_PROTOCOL_BAM, /* Broadcom Autoneg Mode */
+ DEVLINK_AUTONEG_PROTOCOL_CONSORTIUM,/* Consortium Autoneg Mode */
+};
+
+enum devlink_pre_os_link_speed {
+ DEVLINK_PRE_OS_LINK_SPEED_AUTONEG,
+ DEVLINK_PRE_OS_LINK_SPEED_1G,
+ DEVLINK_PRE_OS_LINK_SPEED_10G,
+ DEVLINK_PRE_OS_LINK_SPEED_25G,
+ DEVLINK_PRE_OS_LINK_SPEED_40G,
+ DEVLINK_PRE_OS_LINK_SPEED_50G,
+ DEVLINK_PRE_OS_LINK_SPEED_100G,
+ DEVLINK_PRE_OS_LINK_SPEED_5G = 0xe,
+ DEVLINK_PRE_OS_LINK_SPEED_100M = 0xf,
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -249,9 +269,14 @@ enum devlink_attr {
DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED, /* u8 */
DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED, /* u8 */
DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_AUTONEG_PROTOCOL, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_MEDIA_AUTO_DETECT,/* u8 */
+ DEVLINK_ATTR_PERM_CFG_PHY_SELECT, /* u8 */
+ DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3, /* u32 */
/* Add new permanent config parameters above here */
- DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED,
+ DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3,
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index d611154..80a2a50 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2562,6 +2562,11 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_PERM_CFG_PME_CAPABILITY_ENABLED] = { .type = NLA_U8 },
[DEVLINK_ATTR_PERM_CFG_MAGIC_PACKET_WOL_ENABLED] = { .type = NLA_U8 },
[DEVLINK_ATTR_PERM_CFG_EEE_PWR_SAVE_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_AUTONEG_PROTOCOL] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_MEDIA_AUTO_DETECT] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_PHY_SELECT] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D0] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_PRE_OS_LINK_SPEED_D3] = { .type = NLA_U32 },
};
static const struct genl_ops devlink_nl_ops[] = {
--
2.7.4
[PATCH 2/7] devlink: Adding NPAR permanent config parameters
Extending DEVLINK_ATTR_PERM_CFG (permanent/NVRAM device configuration)
to include NPAR settings:
DEVLINK_ATTR_PERM_CFG_NPAR_NUM_PARTITIONS_PER_PORT: Number of NIC
Partitions (NPAR) per port.
DEVLINK_ATTR_PERM_CFG_NPAR_BW_IN_PERCENT: 1 if BW_RESERVATION and
BW_LIMIT is in percent; /0 if BW_RESERVATION and BW_LIMIT is in
100 Mbps units.
DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION: Configures NPAR bandwidth
or weight reservation, in percent or 100 Mbps units, depending on
BW_IN_PERCENT.
DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID: 1 to use
BW_RESERVATION setting, 0 to ignore.
DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT: Configures NPAR bandwidth or
weight limit, in percent or 100 Mbps units, depending on
BW_IN_PERCENT.
DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID: 1 to use BW_LIMIT
setting, 0 to ignore.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/uapi/linux/devlink.h | 8 +++-
net/core/devlink.c | 7 +++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 34de44d..21cfb37 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -218,9 +218,15 @@ enum devlink_attr {
DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF,/* u32 */
DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT, /* u32 */
DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_NUM_PARTITIONS_PER_PORT, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_BW_IN_PERCENT, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION, /* u32 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID,/* u8 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT,/* u32 */
+ DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID, /* u8 */
/* Add new permanent config parameters above here */
- DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF,
+ DEVLINK_ATTR_PERM_CFG_LAST = DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID,
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 427a65e..76bb6d4 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2543,6 +2543,13 @@ static const struct nla_policy
devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_PERM_CFG_NUM_VF_PER_PF] = { .type = NLA_U32 },
[DEVLINK_ATTR_PERM_CFG_MAX_NUM_PF_MSIX_VECT] = { .type = NLA_U32 },
[DEVLINK_ATTR_PERM_CFG_MSIX_VECTORS_PER_VF] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_NUM_PARTITIONS_PER_PORT] = {
+ .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_BW_IN_PERCENT] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID] = { .type = NLA_U8 },
};
static const struct genl_ops devlink_nl_ops[] = {
--
2.7.4
Re: [RFC 0/3] Adding config get/set to devlink
On Thu, Oct 12, 2017 at 3:20 PM, Florian Fainelli wrote: > On 10/12/2017 12:06 PM, David Miller wrote: >> From: Florian Fainelli >> Date: Thu, 12 Oct 2017 08:43:59 -0700 >> >>> Once we move ethtool (or however we name its successor) over to >>> netlink there is an opportunity for accessing objects that do and do >>> not have a netdevice representor today (e.g: management ports on >>> switches) with the same interface, and devlink could be used for >>> that. >> >> That is an interesting angle for including this in devlink. >> >> I'm not so sure what to do about this. >> >> One suggestion is that devlink is used for getting ethtool stats for >> objects lacking netdev representor's, and a new genetlink family is >> used for netdev based ethtool. > > Right, I was also thinking along those lines that we we would have a new > generic netlink family for ethtool to support ethtool over netlink. > >> >> I think it's important that we don't expand the scope of devlink >> beyond what it was originally designed for. > > It seems to me like devlink is well defined in what it is not for: it is > not meant to be used for any object that is/has a net_device, but it is > not well defined for what it can offer to these non network devices. For > instance, we have a tremendous amount of operations that are extremely > specific to its single user(s) such as mlx5 and mlxsw. > > For instance, I am not sure how the buffer reservation scheme can be > generalized, and this is always the tricky part with a single user > facility in that you try to generalize the best you can based on the HW > you know. This is not a criticism or meant to be anything negative, this > just happens to be the case, and we did not have anything better. > > So maybe the first thing is to clarify what devlink operations can and > should be and what they are absolutely not allowed to cover. We should > also clarify whether a generic set/get that Steven is proposing is > something that we tolerate, or whether there should be specific function > pointers implemented for each attribute, which would be more in line > with what has been done thus far. Hi Florian, Some of this is subjective, of course, but just to clarify, it did seem like implementing a new devlink_op function pointer per attribute might be more consistent with what's been done so far. But for code reuse purposes - i.e. to avoid replicating essentially the same function for each of the 30+ config attributes - I elected to just implement a single generic get and set devlink_op. Steve
Re: [RFC 0/3] Adding config get/set to devlink
Hi Roopa, The attributes added in this patchset are not really the same type as ethtool - these are more device configuration type attributes. The speeds you saw, for example, affect the pre-OS [i.e. PXE boot time] configuration for a port, and aren't run-time speed changes on a given netdev like ethtool configures. As Jiri mentioned, I will add some comments to better describe each of the attributes. So I don't think there's much duplication here with ethtool. That said, there also shouldn't be anything in the patchset that would preclude some future migration of ethtool settings to using devlink or rtnetlink API. Steve On Thu, Oct 12, 2017 at 10:35 AM, Roopa Prabhu wrote: > On Thu, Oct 12, 2017 at 6:34 AM, Steve Lin wrote: >> Adds a devlink command for getting & setting device configuration >> parameters, and enumerates a bunch of those parameters as devlink >> attributes. Also introduces an attribute that can be set by a >> driver to indicate that the config change doesn't take effect >> until the next restart (as in the case of the bnxt driver changes >> in this patchset, for which all the configuration changes affect NVM >> only, and aren't loaded until the next restart.) >> >> bnxt driver patches make use of these new devlink cmds/attributes. >> >> Steve Lin (3): >> devlink: Add config parameter get/set operations >> bnxt: Move generic devlink code to new file >> bnxt: Add devlink support for config get/set >> > > Is the goal here to move all ethtool operations to devlink (I saw some > attrs related to speed etc). ?. > We do need to move ethtool attrs to netlink and devlink is a good > place (and of-course leave the current ethtool api around for backward > compatibility).
Re: [RFC 1/3] devlink: Add config parameter get/set operations
Jiri,
Thanks for your feedback below and in your other response. I will
make some changes to address those issues and resubmit.
Thanks again!
Steve
On Thu, Oct 12, 2017 at 10:03 AM, Jiri Pirko wrote:
> Thu, Oct 12, 2017 at 03:34:20PM CEST, steven.l...@broadcom.com wrote:
>>Add support for config parameter get/set commands. Initially used by
>>bnxt driver, but other drivers can use the same, or new, attributes.
>>The config_get() and config_set() operations operate as expected, but
>>note that the driver implementation of the config_set() operation can
>>indicate whether a restart is necessary for the setting to take
>>effect.
>>
>
> First of all, I like this approach.
>
> I would like to see this patch split into:
> 1) config-options infrastructure introduction
> 2) specific config options introductions - would be best to have it
>per-option. We need to make sure every option is very well described
>and explained usecases. This is needed in order vendors to share
> attributes among drivers.
>
> More nits inlined.
>
>
>>Signed-off-by: Steve Lin
>>Acked-by: Andy Gospodarek
>>---
>> include/net/devlink.h| 4 +
>> include/uapi/linux/devlink.h | 108 ++
>> net/core/devlink.c | 207
>> +++
>> 3 files changed, 319 insertions(+)
>>
>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>index b9654e1..952966c 100644
>>--- a/include/net/devlink.h
>>+++ b/include/net/devlink.h
>>@@ -270,6 +270,10 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>+ int (*config_get)(struct devlink *devlink, enum devlink_attr attr,
>>+u32 *value);
>>+ int (*config_set)(struct devlink *devlink, enum devlink_attr attr,
>>+u32 value, u8 *restart_reqd);
>> };
>>
>> static inline void *devlink_priv(struct devlink *devlink)
>>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>index 0cbca96..e959716 100644
>>--- a/include/uapi/linux/devlink.h
>>+++ b/include/uapi/linux/devlink.h
>>@@ -70,6 +70,9 @@ enum devlink_command {
>> DEVLINK_CMD_DPIPE_HEADERS_GET,
>> DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
>>
>>+ DEVLINK_CMD_CONFIG_GET,
>>+ DEVLINK_CMD_CONFIG_SET,
>>+
>> /* add new commands above here */
>> __DEVLINK_CMD_MAX,
>> DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
>>@@ -124,6 +127,68 @@ enum devlink_eswitch_encap_mode {
>> DEVLINK_ESWITCH_ENCAP_MODE_BASIC,
>> };
>>
>>+enum devlink_dcbx_mode {
>>+ DEVLINK_DCBX_MODE_DISABLED,
>>+ DEVLINK_DCBX_MODE_IEEE,
>>+ DEVLINK_DCBX_MODE_CEE,
>>+ DEVLINK_DCBX_MODE_IEEE_CEE,
>>+};
>>+
>>+enum devlink_multifunc_mode {
>>+ DEVLINK_MULTIFUNC_MODE_ALLOWED, /* Ext switch activates MF */
>>+ DEVLINK_MULTIFUNC_MODE_FORCE_SINGFUNC,
>>+ DEVLINK_MULTIFUNC_MODE_NPAR10, /* NPAR 1.0 */
>>+ DEVLINK_MULTIFUNC_MODE_NPAR15, /* NPAR 1.5 */
>>+ DEVLINK_MULTIFUNC_MODE_NPAR20, /* NPAR 2.0 */
>>+};
>>+
>>+enum devlink_autoneg_protocol {
>>+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_BAM,
>>+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_CONSORTIUM,
>>+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY,
>>+ DEVLINK_AUTONEG_PROTOCOL_BAM, /* Broadcom Autoneg Mode */
>>+ DEVLINK_AUTONEG_PROTOCOL_CONSORTIUM,/* Consortium Autoneg Mode */
>>+};
>>+
>>+enum devlink_pre_os_link_speed {
>>+ DEVLINK_PRE_OS_LINK_SPEED_AUTONEG,
>>+ DEVLINK_PRE_OS_LINK_SPEED_1G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_10G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_25G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_40G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_50G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_100G,
>>+ DEVLINK_PRE_OS_LINK_SPEED_5G = 0xe,
>>+ DEVLINK_PRE_OS_LINK_SPEED_100M = 0xf,
>>+};
>>+
>>+enum devlink_mba_boot_type {
>>+ DEVLINK_MBA_BOOT_TYPE_AUTO_DETECT,
>>+ DEVLINK_MBA_BOOT_TYPE_BBS, /* BIOS Boot Specification */
>>+ DEVLINK_MBA_BOOT_TYPE_INTR18, /* Hook interrupt 0x18 */
>>+ DEVLINK_MBA_BOOT_TYPE_INTR19, /* Hook interrupt 0x19 */
>>+};
>>+
>>+enum devlink_mba_setup_hot_key
[RFC 1/3] devlink: Add config parameter get/set operations
Add support for config parameter get/set commands. Initially used by
bnxt driver, but other drivers can use the same, or new, attributes.
The config_get() and config_set() operations operate as expected, but
note that the driver implementation of the config_set() operation can
indicate whether a restart is necessary for the setting to take
effect.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
include/net/devlink.h| 4 +
include/uapi/linux/devlink.h | 108 ++
net/core/devlink.c | 207 +++
3 files changed, 319 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..952966c 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,10 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
*p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*config_get)(struct devlink *devlink, enum devlink_attr attr,
+ u32 *value);
+ int (*config_set)(struct devlink *devlink, enum devlink_attr attr,
+ u32 value, u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..e959716 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,9 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ DEVLINK_CMD_CONFIG_GET,
+ DEVLINK_CMD_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -124,6 +127,68 @@ enum devlink_eswitch_encap_mode {
DEVLINK_ESWITCH_ENCAP_MODE_BASIC,
};
+enum devlink_dcbx_mode {
+ DEVLINK_DCBX_MODE_DISABLED,
+ DEVLINK_DCBX_MODE_IEEE,
+ DEVLINK_DCBX_MODE_CEE,
+ DEVLINK_DCBX_MODE_IEEE_CEE,
+};
+
+enum devlink_multifunc_mode {
+ DEVLINK_MULTIFUNC_MODE_ALLOWED, /* Ext switch activates MF */
+ DEVLINK_MULTIFUNC_MODE_FORCE_SINGFUNC,
+ DEVLINK_MULTIFUNC_MODE_NPAR10, /* NPAR 1.0 */
+ DEVLINK_MULTIFUNC_MODE_NPAR15, /* NPAR 1.5 */
+ DEVLINK_MULTIFUNC_MODE_NPAR20, /* NPAR 2.0 */
+};
+
+enum devlink_autoneg_protocol {
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_BAM,
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_CONSORTIUM,
+ DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY,
+ DEVLINK_AUTONEG_PROTOCOL_BAM, /* Broadcom Autoneg Mode */
+ DEVLINK_AUTONEG_PROTOCOL_CONSORTIUM,/* Consortium Autoneg Mode */
+};
+
+enum devlink_pre_os_link_speed {
+ DEVLINK_PRE_OS_LINK_SPEED_AUTONEG,
+ DEVLINK_PRE_OS_LINK_SPEED_1G,
+ DEVLINK_PRE_OS_LINK_SPEED_10G,
+ DEVLINK_PRE_OS_LINK_SPEED_25G,
+ DEVLINK_PRE_OS_LINK_SPEED_40G,
+ DEVLINK_PRE_OS_LINK_SPEED_50G,
+ DEVLINK_PRE_OS_LINK_SPEED_100G,
+ DEVLINK_PRE_OS_LINK_SPEED_5G = 0xe,
+ DEVLINK_PRE_OS_LINK_SPEED_100M = 0xf,
+};
+
+enum devlink_mba_boot_type {
+ DEVLINK_MBA_BOOT_TYPE_AUTO_DETECT,
+ DEVLINK_MBA_BOOT_TYPE_BBS, /* BIOS Boot Specification */
+ DEVLINK_MBA_BOOT_TYPE_INTR18, /* Hook interrupt 0x18 */
+ DEVLINK_MBA_BOOT_TYPE_INTR19, /* Hook interrupt 0x19 */
+};
+
+enum devlink_mba_setup_hot_key {
+ DEVLINK_MBA_SETUP_HOT_KEY_CTRL_S,
+ DEVLINK_MBA_SETUP_HOT_KEY_CTRL_B,
+};
+
+enum devlink_mba_boot_protocol {
+ DEVLINK_MBA_BOOT_PROTOCOL_PXE,
+ DEVLINK_MBA_BOOT_PROTOCOL_ISCSI,
+ DEVLINK_MBA_BOOT_PROTOCOL_NONE = 0x7,
+};
+
+enum devlink_mba_link_speed {
+ DEVLINK_MBA_LINK_SPEED_AUTONEG,
+ DEVLINK_MBA_LINK_SPEED_1G,
+ DEVLINK_MBA_LINK_SPEED_10G,
+ DEVLINK_MBA_LINK_SPEED_25G,
+ DEVLINK_MBA_LINK_SPEED_40G,
+ DEVLINK_MBA_LINK_SPEED_50G,
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -202,6 +267,49 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE,/* u8 */
+ /* Configuration Parameters */
+ DEVLINK_ATTR_SRIOV_ENABLED, /* u8 */
+ DEVLINK_ATTR_NUM_VF_PER_PF, /* u32 */
+ DEVLINK_ATTR_MAX_NUM_PF_MSIX_VECT, /* u32 */
+ DEVLINK_ATTR_MSIX_VECTORS_PER_VF, /* u32 */
+ DEVLINK_ATTR_NPAR_NUM_PARTITIONS_PER_PORT, /* u32 */
+ DEVLINK_ATTR_NPAR_BW_IN_PERCENT,/* u8 */
+ DEVLINK_ATTR_NPAR_BW_RESERVATION, /* u8 */
+ DEVLINK_ATTR_NPAR_BW_RESERVATION_VALID, /* u8 */
+ DEVLINK_ATTR_NPAR_BW_LIMIT, /* u8 */
+ DEVLINK_ATTR_NPAR_BW_LIMIT_VALID, /* u8 */
+ DEVLINK_ATTR_DCBX_MODE,
[RFC 0/3] Adding config get/set to devlink
Adds a devlink command for getting & setting device configuration parameters, and enumerates a bunch of those parameters as devlink attributes. Also introduces an attribute that can be set by a driver to indicate that the config change doesn't take effect until the next restart (as in the case of the bnxt driver changes in this patchset, for which all the configuration changes affect NVM only, and aren't loaded until the next restart.) bnxt driver patches make use of these new devlink cmds/attributes. Steve Lin (3): devlink: Add config parameter get/set operations bnxt: Move generic devlink code to new file bnxt: Add devlink support for config get/set drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 403 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 56 +++ drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 ++ drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 97 +- drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 35 +- include/net/devlink.h | 4 + include/uapi/linux/devlink.h | 108 ++ net/core/devlink.c| 207 +++ 10 files changed, 882 insertions(+), 131 deletions(-) create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h -- 2.7.4
[RFC 2/3] bnxt: Move generic devlink code to new file
Moving generic devlink code (registration, etc.) out of VF-R code
into new bnxt_devlink file.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/Makefile | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 109 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 39
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 97 +--
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.h | 35 +--
6 files changed, 152 insertions(+), 131 deletions(-)
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
diff --git a/drivers/net/ethernet/broadcom/bnxt/Makefile
b/drivers/net/ethernet/broadcom/bnxt/Makefile
index 457201f..59c8ec9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/Makefile
+++ b/drivers/net/ethernet/broadcom/bnxt/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_BNXT) += bnxt_en.o
-bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
bnxt_xdp.o bnxt_vfr.o bnxt_devlink.o
bnxt_en-$(CONFIG_BNXT_FLOWER_OFFLOAD) += bnxt_tc.o
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5ba4993..52cc38d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -61,6 +61,7 @@
#include "bnxt_xdp.h"
#include "bnxt_vfr.h"
#include "bnxt_tc.h"
+#include "bnxt_devlink.h"
#define BNXT_TX_TIMEOUT(5 * HZ)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
new file mode 100644
index 000..d159cce
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -0,0 +1,109 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include
+#include
+#include "bnxt_hsi.h"
+#include "bnxt.h"
+#include "bnxt_vfr.h"
+#include "bnxt_devlink.h"
+
+static int bnxt_dl_eswitch_mode_get(struct devlink *devlink, u16 *mode)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+
+ *mode = bp->eswitch_mode;
+ return 0;
+}
+
+static int bnxt_dl_eswitch_mode_set(struct devlink *devlink, u16 mode)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ int rc = 0;
+
+ mutex_lock(&bp->sriov_lock);
+ if (bp->eswitch_mode == mode) {
+ netdev_info(bp->dev, "already in %s eswitch mode",
+ mode == DEVLINK_ESWITCH_MODE_LEGACY ?
+ "legacy" : "switchdev");
+ rc = -EINVAL;
+ goto done;
+ }
+
+ switch (mode) {
+ case DEVLINK_ESWITCH_MODE_LEGACY:
+ bnxt_vf_reps_destroy(bp);
+ break;
+
+ case DEVLINK_ESWITCH_MODE_SWITCHDEV:
+ if (pci_num_vf(bp->pdev) == 0) {
+ netdev_info(bp->dev,
+ "Enable VFs before setting switchdev mode");
+ rc = -EPERM;
+ goto done;
+ }
+ rc = bnxt_vf_reps_create(bp);
+ break;
+
+ default:
+ rc = -EINVAL;
+ goto done;
+ }
+done:
+ mutex_unlock(&bp->sriov_lock);
+ return rc;
+}
+
+static const struct devlink_ops bnxt_dl_ops = {
+ .eswitch_mode_set = bnxt_dl_eswitch_mode_set,
+ .eswitch_mode_get = bnxt_dl_eswitch_mode_get,
+};
+
+int bnxt_dl_register(struct bnxt *bp)
+{
+ struct devlink *dl;
+ int rc;
+
+ if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
+ return 0;
+
+ if (bp->hwrm_spec_code < 0x10800) {
+ netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch
SWITCHDEV mode.\n");
+ return -ENOTSUPP;
+ }
+
+ dl = devlink_alloc(&bnxt_dl_ops, sizeof(struct bnxt_dl));
+ if (!dl) {
+ netdev_warn(bp->dev, "devlink_alloc failed");
+ return -ENOMEM;
+ }
+
+ bnxt_link_bp_to_dl(bp, dl);
+ bp->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
+ rc = devlink_register(dl, &bp->pdev->dev);
+ if (rc) {
+ bnxt_link_bp_to_dl(bp, NULL);
+ devlink_free(dl);
+ netdev_warn(bp->dev, "devlink_register failed. rc=%d", rc);
+ return rc;
+ }
+
+ return 0;
+}
[RFC 3/3] bnxt: Add devlink support for config get/set
Implements get and set of configuration parameters using new devlink
config get/set API.
Signed-off-by: Steve Lin
Acked-by: Andy Gospodarek
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 306 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +++
3 files changed, 417 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index d159cce..32319d5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,6 +14,83 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_ATTR_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_ATTR_IGNORE_ARI_CAPABILITY, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 164},
+ {DEVLINK_ATTR_PME_CAPABILITY_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 166},
+ {DEVLINK_ATTR_LLDP_NEAREST_BRIDGE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 269},
+ {DEVLINK_ATTR_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 270},
+ {DEVLINK_ATTR_SECURE_NIC_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 162},
+ {DEVLINK_ATTR_PHY_SELECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 329},
+ {DEVLINK_ATTR_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
+
+ {DEVLINK_ATTR_MBA_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 351},
+ {DEVLINK_ATTR_MBA_BOOT_TYPE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 2, 352},
+ {DEVLINK_ATTR_MBA_DELAY_TIME, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 4, 353},
+ {DEVLINK_ATTR_MBA_SETUP_HOT_KEY, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 354},
+ {DEVLINK_ATTR_MBA_HIDE_SETUP_PROMPT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 355},
+ {DEVLINK_ATTR_MBA_VLAN_TAG, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 16, 357},
+ {DEVLINK_ATTR_MBA_VLAN_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 358},
+ {DEVLINK_ATTR_MBA_LINK_SPEED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 4, 359},
+ {DEVLINK_ATTR_MBA_BOOT_RETRY_COUNT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 3, 360},
+ {DEVLINK_ATTR_MBA_BOOT_PROTOCOL, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 3, 361},
+ {DEVLINK_ATTR_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_ATTR_MSIX_VECTORS_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
+ {DEVLINK_ATTR_NPAR_BW_RESERVATION, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 501},
+ {DEVLINK_ATTR_NPAR_BW_LIMIT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 502},
+ {DEVLINK_ATTR_RDMA_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 506},
+ {DEVLINK_ATTR_NPAR_BW_IN_PERCENT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 507},
+ {DEVLINK_ATTR_NPAR_BW_RESERVATION_VALID, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 508},
+ {DEVLINK_ATTR_NPAR_BW_LIMIT_VALID, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 1, 509},
+
+ {DEVLINK_ATTR_MAGIC_PACKET_WOL_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 152},
+ {DEVLINK_ATTR_DCBX_MODE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 155},
+ {DEVLINK_ATTR_MULTIFUNC_MODE, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 5, 157},
+ {DEVLINK_ATTR_PRE_OS_LINK_SPEED_D0, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 205},
+ {DEVLINK_ATTR_EEE_PWR_SAVE_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 208},
+ {DEVLINK_ATTR_PRE_OS_LINK_SPEED_D3, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 4, 210},
+ {DEVLINK_ATTR_MEDIA_AUTO_DETECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 1, 213},
+ {DEVLINK_ATTR_AUTONEG_PROTOCOL, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 8, 312},
+ {DEVLINK_ATTR_NPAR_NUM_PARTITIONS_PER_PORT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_PORT, 8, 503},
+};
+
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
static int bnxt_dl_eswitch_mode_get(struct devlink *devlink, u16 *mode)
{
struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
@@ -60,9 +137,226 @@ static int bnxt_dl_eswitch_mode_set(struct devlink
*devlink, u16 mode)
return rc;
}
-static const struct devlink_ops bnxt_dl_ops = {
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+void *buf, int size)
+{
+ struct hwrm_nvm_get_variable_input req = {0};
+ void *dest_data_addr = NULL
[PATCH] net: ethernet: bgmac: Allow MAC address to be specified in DTB
Allows the BCMA version of the bgmac driver to obtain MAC address
from the device tree. If no MAC address is specified there, then
the previous behavior (obtaining MAC address from SPROM) is
used.
Signed-off-by: Steve Lin
---
drivers/net/ethernet/broadcom/bgmac-bcma.c | 39 ++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c
b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index cf15b7e..6322594 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "bgmac.h"
static inline bool bgmac_is_bcm4707_family(struct bcma_device *core)
@@ -114,7 +115,7 @@ static int bgmac_probe(struct bcma_device *core)
struct ssb_sprom *sprom = &core->bus->sprom;
struct mii_bus *mii_bus;
struct bgmac *bgmac;
- u8 *mac;
+ const u8 *mac = NULL;
int err;
bgmac = bgmac_alloc(&core->dev);
@@ -127,21 +128,27 @@ static int bgmac_probe(struct bcma_device *core)
bcma_set_drvdata(core, bgmac);
- switch (core->core_unit) {
- case 0:
- mac = sprom->et0mac;
- break;
- case 1:
- mac = sprom->et1mac;
- break;
- case 2:
- mac = sprom->et2mac;
- break;
- default:
- dev_err(bgmac->dev, "Unsupported core_unit %d\n",
- core->core_unit);
- err = -ENOTSUPP;
- goto err;
+ if (bgmac->dev->of_node)
+ mac = of_get_mac_address(bgmac->dev->of_node);
+
+ /* If no MAC address assigned via device tree, check SPROM */
+ if (!mac) {
+ switch (core->core_unit) {
+ case 0:
+ mac = sprom->et0mac;
+ break;
+ case 1:
+ mac = sprom->et1mac;
+ break;
+ case 2:
+ mac = sprom->et2mac;
+ break;
+ default:
+ dev_err(bgmac->dev, "Unsupported core_unit %d\n",
+ core->core_unit);
+ err = -ENOTSUPP;
+ goto err;
+ }
}
ether_addr_copy(bgmac->net_dev->dev_addr, mac);
--
2.1.0
