[RFC] net: core: devlink: add port_params_ops for devlink port parameters altering

2021-04-09 Thread Oleksandr Mazur
I'd like to discuss a possibility of handling devlink port parameters
with devlink port pointer supplied.

Current design makes it impossible to distinguish which port's parameter
should get altered (set) or retrieved (get) whenever there's a single
parameter registered within a few ports.

This patch aims to show how this can be changed:
  - introduce structure port_params_ops that has callbacks for get/set/validate;
  - if devlink has registered port_params_ops, then upon every devlink
port parameter get/set call invoke port parameters callback

Signed-off-by: Oleksandr Mazur 
---
 drivers/net/netdevsim/dev.c   | 46 +++
 drivers/net/netdevsim/netdevsim.h |  1 +
 include/net/devlink.h | 11 
 net/core/devlink.c| 16 ++-
 4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6189a4c0d39e..4f9a3104ca46 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -39,6 +39,11 @@ static struct dentry *nsim_dev_ddir;
 
 #define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
 
+static int nsim_dev_port_param_set(struct devlink_port *port, u32 id,
+  struct devlink_param_gset_ctx *ctx);
+static int nsim_dev_port_param_get(struct devlink_port *port, u32 id,
+  struct devlink_param_gset_ctx *ctx);
+
 static int
 nsim_dev_take_snapshot(struct devlink *devlink,
   const struct devlink_region_ops *ops,
@@ -339,6 +344,7 @@ static int nsim_dev_resources_register(struct devlink 
*devlink)
 enum nsim_devlink_param_id {
NSIM_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
NSIM_DEVLINK_PARAM_ID_TEST1,
+   NSIM_DEVLINK_PARAM_ID_TEST2,
 };
 
 static const struct devlink_param nsim_devlink_params[] = {
@@ -349,6 +355,10 @@ static const struct devlink_param nsim_devlink_params[] = {
 "test1", DEVLINK_PARAM_TYPE_BOOL,
 BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
 NULL, NULL, NULL),
+   DEVLINK_PARAM_DRIVER(NSIM_DEVLINK_PARAM_ID_TEST2,
+"test1", DEVLINK_PARAM_TYPE_U32,
+BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
+NULL, NULL, NULL),
 };
 
 static void nsim_devlink_set_params_init_values(struct nsim_dev *nsim_dev,
@@ -892,6 +902,11 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink 
*devlink,
return 0;
 }
 
+static const struct devlink_port_param_ops nsim_dev_port_param_ops = {
+   .get = nsim_dev_port_param_get,
+   .set = nsim_dev_port_param_set,
+};
+
 static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT 
|
 
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
@@ -905,6 +920,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_group_set = nsim_dev_devlink_trap_group_set,
.trap_policer_set = nsim_dev_devlink_trap_policer_set,
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
+   .port_param_ops = _dev_port_param_ops,
 };
 
 #define NSIM_DEV_MAX_MACS_DEFAULT 32
@@ -1239,6 +1255,36 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
return err;
 }
 
+static int nsim_dev_port_param_get(struct devlink_port *port, u32 id,
+  struct devlink_param_gset_ctx *ctx)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(port->devlink);
+   struct nsim_dev_port *nsim_port =
+   __nsim_dev_port_lookup(nsim_dev, port->index);
+
+   if (id == NSIM_DEVLINK_PARAM_ID_TEST2) {
+   ctx->val.vu32 = nsim_port->test_parameter_value;
+   return 0;
+   }
+
+   return -EINVAL;
+}
+
+static int nsim_dev_port_param_set(struct devlink_port *port, u32 id,
+  struct devlink_param_gset_ctx *ctx)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(port->devlink);
+   struct nsim_dev_port *nsim_port =
+   __nsim_dev_port_lookup(nsim_dev, port->index);
+
+   if (id == NSIM_DEVLINK_PARAM_ID_TEST2) {
+   nsim_port->test_parameter_value = ctx->val.vu32;
+   return 0;
+   }
+
+   return -EINVAL;
+}
+
 int nsim_dev_init(void)
 {
nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL);
diff --git a/drivers/net/netdevsim/netdevsim.h 
b/drivers/net/netdevsim/netdevsim.h
index 7ff24e03577b..4f5fc491c8d6 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -203,6 +203,7 @@ struct nsim_dev_port {
unsigned int port_index;
struct dentry *ddir;
struct netdevsim *ns;
+   u32 test_parameter_value;
 };
 
 struct nsim_dev {
diff --git a/include/net/devlink.

Re: [PATCH iproute2-next V4] devlink: add support for port params get/set

2021-02-11 Thread Oleksandr Mazur
On 2/9/21 3:31 AM, Oleksandr Mazur wrote:
> Add implementation for the port parameters
> getting/setting.
> Add bash completion for port param.
> Add man description for port param.
> 
> Signed-off-by: Oleksandr Mazur 
> ---

> applied to iproute2-next.

> In the future, please add example commands - get/set/show, and the show
> commands should have examples for normal and json.

Thanks;
And yes - sorry; i missed it when uploaded the 4-th patch version (was present 
in V3)...

[PATCH iproute2-next V4] devlink: add support for port params get/set

2021-02-09 Thread Oleksandr Mazur
Add implementation for the port parameters
getting/setting.
Add bash completion for port param.
Add man description for port param.

Signed-off-by: Oleksandr Mazur 
---
V4:
1) Rebase on top of master;
2) Add missed space in after else-if-clause bracket;
V3:
1) Add usage example;
2) Remove stray newline in code;
V2:
1) Add bash completion for port param;
2) Add man decsription / examples for port param;

 bash-completion/devlink |  55 
 devlink/devlink.c   | 274 +++-
 man/man8/devlink-port.8 |  65 ++
 3 files changed, 388 insertions(+), 6 deletions(-)

diff --git a/bash-completion/devlink b/bash-completion/devlink
index 7395b504..361be9fe 100644
--- a/bash-completion/devlink
+++ b/bash-completion/devlink
@@ -319,6 +319,57 @@ _devlink_port_split()
 esac
 }
 
+# Completion for devlink port param set
+_devlink_port_param_set()
+{
+case $cword in
+7)
+COMPREPLY=( $( compgen -W "value" -- "$cur" ) )
+return
+;;
+8)
+# String argument
+return
+;;
+9)
+COMPREPLY=( $( compgen -W "cmode" -- "$cur" ) )
+return
+;;
+10)
+COMPREPLY=( $( compgen -W "runtime driverinit permanent" -- \
+"$cur" ) )
+return
+;;
+esac
+}
+
+# Completion for devlink port param
+_devlink_port_param()
+{
+case "$cword" in
+3)
+COMPREPLY=( $( compgen -W "show set" -- "$cur" ) )
+return
+;;
+4)
+_devlink_direct_complete "port"
+return
+;;
+5)
+COMPREPLY=( $( compgen -W "name" -- "$cur" ) )
+return
+;;
+6)
+_devlink_direct_complete "param_name"
+return
+;;
+esac
+
+if [[ "${words[3]}" == "set" ]]; then
+_devlink_port_param_set
+fi
+}
+
 # Completion for devlink port
 _devlink_port()
 {
@@ -331,6 +382,10 @@ _devlink_port()
 _devlink_port_split
 return
 ;;
+param)
+_devlink_port_param
+return
+;;
 show|unsplit)
 if [[ $cword -eq 3 ]]; then
 _devlink_direct_complete "port"
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 10398f77..c6e85ff9 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2808,7 +2808,8 @@ static void pr_out_param_value(struct dl *dl, const char 
*nla_name,
}
 }
 
-static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array)
+static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array,
+bool is_port_param)
 {
struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {};
struct nlattr *param_value_attr;
@@ -2825,9 +2826,15 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
return;
 
if (array)
-   pr_out_handle_start_arr(dl, tb);
+   if (is_port_param)
+   pr_out_port_handle_start_arr(dl, tb, false);
+   else
+   pr_out_handle_start_arr(dl, tb);
else
-   __pr_out_handle_start(dl, tb, true, false);
+   if (is_port_param)
+   pr_out_port_handle_start(dl, tb, false);
+   else
+   __pr_out_handle_start(dl, tb, true, false);
 
nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]);
 
@@ -2847,7 +2854,10 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
pr_out_entry_end(dl);
}
pr_out_array_end(dl);
-   pr_out_handle_end(dl);
+   if (is_port_param)
+   pr_out_port_handle_end(dl);
+   else
+   pr_out_handle_end(dl);
 }
 
 static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data)
@@ -2860,7 +2870,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr 
*nlh, void *data)
if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
!tb[DEVLINK_ATTR_PARAM])
return MNL_CB_ERROR;
-   pr_out_param(dl, tb, true);
+   pr_out_param(dl, tb, true, false);
return MNL_CB_OK;
 }
 
@@ -3058,6 +3068,21 @@ err_param_value_parse:
return err;
 }
 
+static int cmd_port_param_show_cb(const struct nlmsghdr *nlh, void *data)
+{
+   struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+   struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+   struct dl *dl = data;
+
+   mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+   if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
+   !tb[DEVLINK_ATTR_PORT_INDEX] || !tb[DEV

[RFC v5 net-next] net: core: devlink: add 'dropped' stats field for traps

2021-02-04 Thread Oleksandr Mazur
Whenever query statistics is issued for trap, devlink subsystem
would also fill-in statistics 'dropped' field. This field indicates
the number of packets HW dropped and failed to report to the device driver,
and thus - to the devlink subsystem itself.
In case if device driver didn't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Add trap_drop_counter_get callback implementation to the netdevsim.
Add new test cases for netdevsim, to test both the callback
functionality, as well as drop statistics alteration check.

Signed-off-by: Oleksandr Mazur 
---
V5:
1) Add missed static specifier for function in netdevsim/dev.c
V4:
1) Change commit description / subject.
2) Change 'dropped' statistics fill condition from
   'trap_drop_counter_get is registered' && 'trap action == drop'
   to
   'trap_drop_counter_get is registered'.
3) Fix statistics fill condition used for wrong stats attribute:
   DEVLINK_ATTR_STATS_RX_PACKETS -> DEVLINK_ATTR_STATS_RX_DROPPED
V3:
1) Mark subject as RFC instead of PATCH.
V2:
1) Change commit description / subject.
2) Remove HARD_DROP action.
3) Remove devlink UAPI changes.
4) Rename hard statistics get callback to be 'trap_drop_counter_get'
5) Make callback get called for existing trap action - DROP:
   whenever statistics for trap with DROP action is queried,
   devlink subsystem would call-in callback to get stats from HW;
6) Add changes to the netdevsim support implemented changes
   (as well as changes to make it possible to test netdevsim with
these changes).
7) Add new test cases to the netdevsim's kselftests to test new
   changes provided with this patchset;

Test-results:
# selftests: drivers/net/netdevsim: devlink_trap.sh
# TEST: Initialization[ OK ]
# TEST: Trap action   [ OK ]
# TEST: Trap metadata [ OK ]
# TEST: Non-existing trap [ OK ]
# TEST: Non-existing trap action  [ OK ]
# TEST: Trap statistics   [ OK ]
# TEST: Trap group action [ OK ]
# TEST: Non-existing trap group   [ OK ]
# TEST: Trap group statistics [ OK ]
# TEST: Trap policer  [ OK ]
# TEST: Trap policer binding  [ OK ]
# TEST: Port delete   [ OK ]
# TEST: Device delete [ OK ]
ok 1 selftests: drivers/net/netdevsim: devlink_trap.sh
---
 drivers/net/netdevsim/dev.c   | 22 
 drivers/net/netdevsim/netdevsim.h |  1 +
 include/net/devlink.h | 10 
 net/core/devlink.c| 53 +--
 .../drivers/net/netdevsim/devlink_trap.sh | 10 
 .../selftests/net/forwarding/devlink_lib.sh   | 26 +
 6 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 816af1f55e2c..d83a53bd0930 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -231,6 +231,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
_dev->fail_trap_policer_counter_get);
+   debugfs_create_bool("fail_trap_counter_get", 0600,
+   nsim_dev->ddir,
+   _dev->fail_trap_counter_get);
nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
 }
@@ -416,6 +419,7 @@ struct nsim_trap_data {
struct delayed_work trap_report_dw;
struct nsim_trap_item *trap_items_arr;
u64 *trap_policers_cnt_arr;
+   u64 trap_pkt_cnt;
struct nsim_dev *nsim_dev;
spinlock_t trap_lock;   /* Protects trap_items_arr */
 };
@@ -892,6 +896,23 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink 
*devlink,
return 0;
 }
 
+static int
+nsim_dev_devlink_trap_hw_counter_get(struct devlink *devlink,
+const struct devlink_trap *trap,
+u64 *p_drops)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(devlink);
+   u64 *cnt;
+
+   if (nsim_dev->fail_trap_counter_get)
+   return -EINVAL;
+
+   cnt = _dev->trap_data->trap_pkt_cnt;
+   *p_drops = (*cnt)++;
+
+   return 0;
+}
+
 static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_up

[RFC v4 net-next] net: core: devlink: add 'dropped' stats field for traps

2021-02-02 Thread Oleksandr Mazur
Whenever query statistics is issued for trap, devlink subsystem
would also fill-in statistics 'dropped' field. This field indicates
the number of packets HW dropped and failed to report to the device driver,
and thus - to the devlink subsystem itself.
In case if device driver didn't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Add trap_drop_counter_get callback implementation to the netdevsim.
Add new test cases for netdevsim, to test both the callback
functionality, as well as drop statistics alteration check.

Signed-off-by: Oleksandr Mazur 
---
V4:
1) Change commit description / subject.
2) Change 'dropped' statistics fill condition from
   'trap_drop_counter_get is registered' && 'trap action == drop'
   to
   'trap_drop_counter_get is registered'.
3) Fix statistics fill condition used for wrong stats attribute:
   DEVLINK_ATTR_STATS_RX_PACKETS -> DEVLINK_ATTR_STATS_RX_DROPPED
V3:
1) Mark subject as RFC instead of PATCH.
V2:
1) Change commit description / subject.
2) Remove HARD_DROP action.
3) Remove devlink UAPI changes.
4) Rename hard statistics get callback to be 'trap_drop_counter_get'
5) Make callback get called for existing trap action - DROP:
   whenever statistics for trap with DROP action is queried,
   devlink subsystem would call-in callback to get stats from HW;
6) Add changes to the netdevsim support implemented changes
   (as well as changes to make it possible to test netdevsim with
these changes).
7) Add new test cases to the netdevsim's kselftests to test new
   changes provided with this patchset;

Test-results:
# selftests: drivers/net/netdevsim: devlink_trap.sh
# TEST: Initialization[ OK ]
# TEST: Trap action   [ OK ]
# TEST: Trap metadata [ OK ]
# TEST: Non-existing trap [ OK ]
# TEST: Non-existing trap action  [ OK ]
# TEST: Trap statistics   [ OK ]
# TEST: Trap group action [ OK ]
# TEST: Non-existing trap group   [ OK ]
# TEST: Trap group statistics [ OK ]
# TEST: Trap policer  [ OK ]
# TEST: Trap policer binding  [ OK ]
# TEST: Port delete   [ OK ]
# TEST: Device delete [ OK ]
ok 1 selftests: drivers/net/netdevsim: devlink_trap.sh
---
 drivers/net/netdevsim/dev.c   | 21 
 drivers/net/netdevsim/netdevsim.h |  1 +
 include/net/devlink.h | 10 
 net/core/devlink.c| 53 +--
 .../drivers/net/netdevsim/devlink_trap.sh | 10 
 .../selftests/net/forwarding/devlink_lib.sh   | 26 +
 6 files changed, 117 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 816af1f55e2c..8cdd3541be0e 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -231,6 +231,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
_dev->fail_trap_policer_counter_get);
+   debugfs_create_bool("fail_trap_counter_get", 0600,
+   nsim_dev->ddir,
+   _dev->fail_trap_counter_get);
nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
 }
@@ -416,6 +419,7 @@ struct nsim_trap_data {
struct delayed_work trap_report_dw;
struct nsim_trap_item *trap_items_arr;
u64 *trap_policers_cnt_arr;
+   u64 trap_pkt_cnt;
struct nsim_dev *nsim_dev;
spinlock_t trap_lock;   /* Protects trap_items_arr */
 };
@@ -892,6 +896,22 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink 
*devlink,
return 0;
 }
 
+int nsim_dev_devlink_trap_hw_counter_get(struct devlink *devlink,
+const struct devlink_trap *trap,
+u64 *p_drops)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(devlink);
+   u64 *cnt;
+
+   if (nsim_dev->fail_trap_counter_get)
+   return -EINVAL;
+
+   cnt = _dev->trap_data->trap_pkt_cnt;
+   *p_drops = (*cnt)++;
+
+   return 0;
+}
+
 static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT 
|
 

[PATCH iproute2-next V3] devlink: add support for port params get/set

2021-02-02 Thread Oleksandr Mazur
Add implementation for the port parameters
getting/setting.
Add bash completion for port param.
Add man description for port param.

Example:
$ devlink dev param set netdevsim/netdevsim0/0 name test_port_parameter value 
false cmode runtime

$ devlink port param show netdevsim/netdevsim0/0 name test_port_parameter
netdevsim/netdevsim0/0:
  name test_port_parameter type driver-specific
values:
  cmode runtime value false

$ devlink port  -jp param show netdevsim/netdevsim0/0 name test_port_parameter
{
"param": {
"netdevsim/netdevsim0/0": [ {
"name": "test_port_parameter",
"type": "driver-specific",
"values": [ {
"cmode": "runtime",
    "value": false
} ]
} ]
}
}

Signed-off-by: Oleksandr Mazur 
---
V3:
1) Add usage example;
2) Remove stray newline in code;
V2:
1) Add bash completion for port param;
2) Add man decsription / examples for port param;

 bash-completion/devlink |  55 
 devlink/devlink.c   | 274 +++-
 man/man8/devlink-port.8 |  65 ++
 3 files changed, 388 insertions(+), 6 deletions(-)

diff --git a/bash-completion/devlink b/bash-completion/devlink
index 7395b504..361be9fe 100644
--- a/bash-completion/devlink
+++ b/bash-completion/devlink
@@ -319,6 +319,57 @@ _devlink_port_split()
 esac
 }
 
+# Completion for devlink port param set
+_devlink_port_param_set()
+{
+case $cword in
+7)
+COMPREPLY=( $( compgen -W "value" -- "$cur" ) )
+return
+;;
+8)
+# String argument
+return
+;;
+9)
+COMPREPLY=( $( compgen -W "cmode" -- "$cur" ) )
+return
+;;
+10)
+COMPREPLY=( $( compgen -W "runtime driverinit permanent" -- \
+"$cur" ) )
+return
+;;
+esac
+}
+
+# Completion for devlink port param
+_devlink_port_param()
+{
+case "$cword" in
+3)
+COMPREPLY=( $( compgen -W "show set" -- "$cur" ) )
+return
+;;
+4)
+_devlink_direct_complete "port"
+return
+;;
+5)
+COMPREPLY=( $( compgen -W "name" -- "$cur" ) )
+return
+;;
+6)
+_devlink_direct_complete "param_name"
+return
+;;
+esac
+
+if [[ "${words[3]}" == "set" ]]; then
+_devlink_port_param_set
+fi
+}
+
 # Completion for devlink port
 _devlink_port()
 {
@@ -331,6 +382,10 @@ _devlink_port()
 _devlink_port_split
 return
 ;;
+param)
+_devlink_port_param
+return
+;;
 show|unsplit)
 if [[ $cword -eq 3 ]]; then
 _devlink_direct_complete "port"
diff --git a/devlink/devlink.c b/devlink/devlink.c
index a2e06644..1984ddbb 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2706,7 +2706,8 @@ static void pr_out_param_value(struct dl *dl, const char 
*nla_name,
}
 }
 
-static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array)
+static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array,
+bool is_port_param)
 {
struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {};
struct nlattr *param_value_attr;
@@ -2723,9 +2724,15 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
return;
 
if (array)
-   pr_out_handle_start_arr(dl, tb);
+   if (is_port_param)
+   pr_out_port_handle_start_arr(dl, tb, false);
+   else
+   pr_out_handle_start_arr(dl, tb);
else
-   __pr_out_handle_start(dl, tb, true, false);
+   if (is_port_param)
+   pr_out_port_handle_start(dl, tb, false);
+   else
+   __pr_out_handle_start(dl, tb, true, false);
 
nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]);
 
@@ -2745,7 +2752,10 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
pr_out_entry_end(dl);
}
pr_out_array_end(dl);
-   pr_out_handle_end(dl);
+   if (is_port_param)
+   pr_out_port_handle_end(dl);
+   else
+   pr_out_handle_end(dl);
 }
 
 static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data)
@@ -2758,7 +2768,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr 
*nlh, void *data)
if (!tb[DEVLINK_ATTR_BUS_NAME] |

Re: [RFC v3 net-next] net: core: devlink: add 'dropped' stats field for DROP trap action

2021-02-01 Thread Oleksandr Mazur

On Fri, 29 Jan 2021 11:15:43 + Oleksandr Mazur wrote:
> > >Thinking about it again - if the action can be changed wouldn't it 
> > >be best for the user to actually get a "HW condition hit" counter,
> >> which would increment regardless of SW config (incl. policers)?  
> >
> > >Otherwise if admin logs onto the box and temporarily enables a trap 
> >> for debug this count would disappear.  
>> 
>> But still this counter makes sense only for 'drop' action.

>Okay, well, "dropped while trap was disabled" seems a lot less useful
>of a definition than "number of times this trap would trigger" but if
>that's all the HW can provide then it is what it is.

>Does the HW also count packets dropped because of overload / overflow
>or some other event, or purely dropped because disabled?

Hw starts counting traffic (hw drops) only when action has been explicitly set 
to be 'DROP';

Re: [RFC v3 net-next] net: core: devlink: add 'dropped' stats field for DROP trap action

2021-01-26 Thread Oleksandr Mazur
On Mon, 25 Jan 2021 14:38:56 +0200 Oleksandr Mazur wrote:
> + if (trap_item->action == DEVLINK_TRAP_ACTION_DROP &&
> + devlink->ops->trap_drop_counter_get) {
> + err = devlink->ops->trap_drop_counter_get(devlink,
> +   trap_item->trap,
> +   );
> + if (err)
> + return err;
> + }

> Why only report this counter when action is set to drop?

> Thinking about it again - if the action can be changed wouldn't it 
> be best for the user to actually get a "HW condition hit" counter,
> which would increment regardless of SW config (incl. policers)?

> Otherwise if admin logs onto the box and temporarily enables a trap 
> for debug this count would disappear.

Okay, so should it become like generic HW counter - trap_hw_counter_get?

[PATCH v2 net-next] net: core: devlink: add 'dropped' stats field for DROP trap action

2021-01-25 Thread Oleksandr Mazur
Whenever query statistics is issued for trap with DROP action,
devlink subsystem would also fill-in statistics 'dropped' field.
In case if device driver did't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Add trap_drop_counter_get callback implementation to the netdevsim.
Add new test cases for netdevsim, to test both the callback
functionality, as well as drop statistics alteration check.

Signed-off-by: Oleksandr Mazur 
---
V2:
1) Change commit description / subject.
2) Remove HARD_DROP action.
3) Remove devlink UAPI changes.
4) Rename hard statistics get callback to be 'trap_drop_counter_get'
5) Make callback get called for existing trap action - DROP:
   whenever statistics for trap with DROP action is queried,
   devlink subsystem would call-in callback to get stats from HW;
6) Add changes to the netdevsim support implemented changes
   (as well as changes to make it possible to test netdevsim with
these changes).
7) Add new test cases to the netdevsim's kselftests to test new
   changes provided with this patchset;

Test-results:
# selftests: drivers/net/netdevsim: devlink_trap.sh
# TEST: Initialization[ OK ]
# TEST: Trap action   [ OK ]
# TEST: Trap metadata [ OK ]
# TEST: Non-existing trap [ OK ]
# TEST: Non-existing trap action  [ OK ]
# TEST: Trap statistics   [ OK ]
# TEST: Trap group action [ OK ]
# TEST: Non-existing trap group   [ OK ]
# TEST: Trap group statistics [ OK ]
# TEST: Trap policer  [ OK ]
# TEST: Trap policer binding  [ OK ]
# TEST: Port delete   [ OK ]
# TEST: Device delete [ OK ]
ok 1 selftests: drivers/net/netdevsim: devlink_trap.sh


 drivers/net/netdevsim/dev.c   | 21 +++
 drivers/net/netdevsim/netdevsim.h |  1 +
 include/net/devlink.h | 10 
 net/core/devlink.c| 55 +--
 .../drivers/net/netdevsim/devlink_trap.sh | 10 
 .../selftests/net/forwarding/devlink_lib.sh   | 26 +
 6 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 816af1f55e2c..1fc8c7a2a1e3 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -231,6 +231,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
_dev->fail_trap_policer_counter_get);
+   debugfs_create_bool("fail_trap_drop_counter_get", 0600,
+   nsim_dev->ddir,
+   _dev->fail_trap_drop_counter_get);
nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
 }
@@ -416,6 +419,7 @@ struct nsim_trap_data {
struct delayed_work trap_report_dw;
struct nsim_trap_item *trap_items_arr;
u64 *trap_policers_cnt_arr;
+   u64 trap_hard_drop_cnt;
struct nsim_dev *nsim_dev;
spinlock_t trap_lock;   /* Protects trap_items_arr */
 };
@@ -892,6 +896,22 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink 
*devlink,
return 0;
 }
 
+int nsim_dev_devlink_trap_drop_counter_get(struct devlink *devlink,
+  const struct devlink_trap *trap,
+  u64 *p_drops)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(devlink);
+   u64 *cnt;
+
+   if (nsim_dev->fail_trap_drop_counter_get)
+   return -EINVAL;
+
+   cnt = _dev->trap_data->trap_hard_drop_cnt;
+   *p_drops = (*cnt)++;
+
+   return 0;
+}
+
 static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT 
|
 
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
@@ -905,6 +925,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_group_set = nsim_dev_devlink_trap_group_set,
.trap_policer_set = nsim_dev_devlink_trap_policer_set,
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
+   .trap_drop_counter_get = nsim_dev_devlink_trap_drop_counter_get,
 };
 
 #define NSIM_DEV_MAX_MACS_DEFAULT 32
diff --git a/drivers/net/netdevsim/netdevsim.h 
b/drivers/net/netdevsim/netdevsim.

Re: [PATCH net-next] net: core: devlink: add new trap action HARD_DROP

2021-01-25 Thread Oleksandr Mazur
Thu, Jan 21, 2021 at 06:36:05PM CET, k...@kernel.org wrote:
>On Thu, 21 Jan 2021 14:21:52 +0200 Ido Schimmel wrote:
>> On Thu, Jan 21, 2021 at 01:29:37PM +0200, Oleksandr Mazur wrote:
>> > Add new trap action HARD_DROP, which can be used by the
>> > drivers to register traps, where it's impossible to get
>> > packet reported to the devlink subsystem by the device
>> > driver, because it's impossible to retrieve dropped packet
>> > from the device itself.
>> > In order to use this action, driver must also register
>> > additional devlink operation - callback that is used
>> > to retrieve number of packets that have been dropped by
>> > the device.  
>> 
>> Are these global statistics about number of packets the hardware dropped
>> for a specific reason or are these per-port statistics?
>> 
>> It's a creative use of devlink-trap interface, but I think it makes
>> sense. Better to re-use an existing interface than creating yet another
>> one.
>
>Not sure if I agree, if we can't trap why is it a trap?
>It's just a counter.

>+1
Device might be unable to trap only the 'DROP' packets, and this information 
should be transparent for the user.

I agree on the statement, that new action might be an overhead.
I could continue on with the solution Ido Schimmel proposed: since no new 
action would be needed and no UAPI changes are required, i could simply do the 
dropped statistics (additional field) output added upon trap stats queiring.
(In case if driver registerd callback, of course; and do so only for DROP 
actions)

[RFC v3 net-next] net: core: devlink: add 'dropped' stats field for DROP trap action

2021-01-25 Thread Oleksandr Mazur
Whenever query statistics is issued for trap with DROP action,
devlink subsystem would also fill-in statistics 'dropped' field.
In case if device driver did't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Add trap_drop_counter_get callback implementation to the netdevsim.
Add new test cases for netdevsim, to test both the callback
functionality, as well as drop statistics alteration check.

Signed-off-by: Oleksandr Mazur 
---
V3:
1) Mark subject as RFC instead of PATCH.
V2:
1) Change commit description / subject.
2) Remove HARD_DROP action.
3) Remove devlink UAPI changes.
4) Rename hard statistics get callback to be 'trap_drop_counter_get'
5) Make callback get called for existing trap action - DROP:
   whenever statistics for trap with DROP action is queried,
   devlink subsystem would call-in callback to get stats from HW;
6) Add changes to the netdevsim support implemented changes
   (as well as changes to make it possible to test netdevsim with
these changes).
7) Add new test cases to the netdevsim's kselftests to test new
   changes provided with this patchset;

Test-results:
# selftests: drivers/net/netdevsim: devlink_trap.sh
# TEST: Initialization[ OK ]
# TEST: Trap action   [ OK ]
# TEST: Trap metadata [ OK ]
# TEST: Non-existing trap [ OK ]
# TEST: Non-existing trap action  [ OK ]
# TEST: Trap statistics   [ OK ]
# TEST: Trap group action [ OK ]
# TEST: Non-existing trap group   [ OK ]
# TEST: Trap group statistics [ OK ]
# TEST: Trap policer  [ OK ]
# TEST: Trap policer binding  [ OK ]
# TEST: Port delete   [ OK ]
# TEST: Device delete [ OK ]
ok 1 selftests: drivers/net/netdevsim: devlink_trap.sh


 drivers/net/netdevsim/dev.c   | 21 +++
 drivers/net/netdevsim/netdevsim.h |  1 +
 include/net/devlink.h | 10 
 net/core/devlink.c| 55 +--
 .../drivers/net/netdevsim/devlink_trap.sh | 10 
 .../selftests/net/forwarding/devlink_lib.sh   | 26 +
 6 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 816af1f55e2c..1fc8c7a2a1e3 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -231,6 +231,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
_dev->fail_trap_policer_counter_get);
+   debugfs_create_bool("fail_trap_drop_counter_get", 0600,
+   nsim_dev->ddir,
+   _dev->fail_trap_drop_counter_get);
nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
 }
@@ -416,6 +419,7 @@ struct nsim_trap_data {
struct delayed_work trap_report_dw;
struct nsim_trap_item *trap_items_arr;
u64 *trap_policers_cnt_arr;
+   u64 trap_hard_drop_cnt;
struct nsim_dev *nsim_dev;
spinlock_t trap_lock;   /* Protects trap_items_arr */
 };
@@ -892,6 +896,22 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink 
*devlink,
return 0;
 }
 
+int nsim_dev_devlink_trap_drop_counter_get(struct devlink *devlink,
+  const struct devlink_trap *trap,
+  u64 *p_drops)
+{
+   struct nsim_dev *nsim_dev = devlink_priv(devlink);
+   u64 *cnt;
+
+   if (nsim_dev->fail_trap_drop_counter_get)
+   return -EINVAL;
+
+   cnt = _dev->trap_data->trap_hard_drop_cnt;
+   *p_drops = (*cnt)++;
+
+   return 0;
+}
+
 static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT 
|
 
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
@@ -905,6 +925,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_group_set = nsim_dev_devlink_trap_group_set,
.trap_policer_set = nsim_dev_devlink_trap_policer_set,
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
+   .trap_drop_counter_get = nsim_dev_devlink_trap_drop_counter_get,
 };
 
 #define NSIM_DEV_MAX_MACS_DEFAULT 32
diff --git a/drivers/net/netde

[PATCH iproute-next v2] devlink: add support for port params get/set

2021-01-25 Thread Oleksandr Mazur
Add implementation for the port parameters getting/setting.
Add bash completion for port param.
Add man description for port param.

Signed-off-by: Oleksandr Mazur 
---
V2:
1) Add bash completion for port param;
2) Add man decsription / examples for port param;

 bash-completion/devlink |  55 
 devlink/devlink.c   | 275 +++-
 man/man8/devlink-port.8 |  65 ++
 3 files changed, 389 insertions(+), 6 deletions(-)

diff --git a/bash-completion/devlink b/bash-completion/devlink
index 7395b504..361be9fe 100644
--- a/bash-completion/devlink
+++ b/bash-completion/devlink
@@ -319,6 +319,57 @@ _devlink_port_split()
 esac
 }
 
+# Completion for devlink port param set
+_devlink_port_param_set()
+{
+case $cword in
+7)
+COMPREPLY=( $( compgen -W "value" -- "$cur" ) )
+return
+;;
+8)
+# String argument
+return
+;;
+9)
+COMPREPLY=( $( compgen -W "cmode" -- "$cur" ) )
+return
+;;
+10)
+COMPREPLY=( $( compgen -W "runtime driverinit permanent" -- \
+"$cur" ) )
+return
+;;
+esac
+}
+
+# Completion for devlink port param
+_devlink_port_param()
+{
+case "$cword" in
+3)
+COMPREPLY=( $( compgen -W "show set" -- "$cur" ) )
+return
+;;
+4)
+_devlink_direct_complete "port"
+return
+;;
+5)
+COMPREPLY=( $( compgen -W "name" -- "$cur" ) )
+return
+;;
+6)
+_devlink_direct_complete "param_name"
+return
+;;
+esac
+
+if [[ "${words[3]}" == "set" ]]; then
+_devlink_port_param_set
+fi
+}
+
 # Completion for devlink port
 _devlink_port()
 {
@@ -331,6 +382,10 @@ _devlink_port()
 _devlink_port_split
 return
 ;;
+param)
+_devlink_port_param
+return
+;;
 show|unsplit)
 if [[ $cword -eq 3 ]]; then
 _devlink_direct_complete "port"
diff --git a/devlink/devlink.c b/devlink/devlink.c
index a2e06644..0fc1d4f0 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2706,7 +2706,8 @@ static void pr_out_param_value(struct dl *dl, const char 
*nla_name,
}
 }
 
-static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array)
+static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array,
+bool is_port_param)
 {
struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {};
struct nlattr *param_value_attr;
@@ -2714,6 +2715,7 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
int nla_type;
int err;
 
+
err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_PARAM], attr_cb, nla_param);
if (err != MNL_CB_OK)
return;
@@ -2723,9 +2725,15 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
return;
 
if (array)
-   pr_out_handle_start_arr(dl, tb);
+   if (is_port_param)
+   pr_out_port_handle_start_arr(dl, tb, false);
+   else
+   pr_out_handle_start_arr(dl, tb);
else
-   __pr_out_handle_start(dl, tb, true, false);
+   if (is_port_param)
+   pr_out_port_handle_start(dl, tb, false);
+   else
+   __pr_out_handle_start(dl, tb, true, false);
 
nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]);
 
@@ -2745,7 +2753,10 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
pr_out_entry_end(dl);
}
pr_out_array_end(dl);
-   pr_out_handle_end(dl);
+   if (is_port_param)
+   pr_out_port_handle_end(dl);
+   else
+   pr_out_handle_end(dl);
 }
 
 static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data)
@@ -2758,7 +2769,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr 
*nlh, void *data)
if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
!tb[DEVLINK_ATTR_PARAM])
return MNL_CB_ERROR;
-   pr_out_param(dl, tb, true);
+   pr_out_param(dl, tb, true, false);
return MNL_CB_OK;
 }
 
@@ -2956,6 +2967,21 @@ err_param_value_parse:
return err;
 }
 
+static int cmd_port_param_show_cb(const struct nlmsghdr *nlh, void *data)
+{
+   struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+   struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+   struct dl *dl = data;
+
+   mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+  

Re: [PATCH net-next] net: core: devlink: add new trap action HARD_DROP

2021-01-22 Thread Oleksandr Mazur
On Thu, 21 Jan 2021 14:21:52 +0200 Ido Schimmel wrote:
> On Thu, Jan 21, 2021 at 01:29:37PM +0200, Oleksandr Mazur wrote:
> > Add new trap action HARD_DROP, which can be used by the
> > drivers to register traps, where it's impossible to get
> > packet reported to the devlink subsystem by the device
> > driver, because it's impossible to retrieve dropped packet
> > from the device itself.
> > In order to use this action, driver must also register
> > additional devlink operation - callback that is used
> > to retrieve number of packets that have been dropped by
> > the device.  
> 
> Are these global statistics about number of packets the hardware dropped
> for a specific reason or are these per-port statistics?
> 
> It's a creative use of devlink-trap interface, but I think it makes
> sense. Better to re-use an existing interface than creating yet another
> one.

> Not sure if I agree, if we can't trap why is it a trap?
> It's just a counter.

It's just another ACTION for trap item. Action however can be switched, e.g. 
from HARD_DROP to MIRROR.

The thing is to be able to configure specific trap to be dropped, and provide a 
way for the device to report back how many packets have been dropped.
If device is able to report the packet itself, then devlink would be in charge 
of counting. If not, there should be a way to retrieve these statistics from 
the devlink.

Re: [PATCH net-next] net: core: devlink: add new trap action HARD_DROP

2021-01-22 Thread Oleksandr Mazur
From: Ido Schimmel 
Sent: Thursday, January 21, 2021 2:21 PM
To: Oleksandr Mazur 
Cc: net...@vger.kernel.org ; j...@nvidia.com 
; da...@davemloft.net ; 
linux-kernel@vger.kernel.org ; k...@kernel.org 

Subject: Re: [PATCH net-next] net: core: devlink: add new trap action HARD_DROP 
 
On Thu, Jan 21, 2021 at 01:29:37PM +0200, Oleksandr Mazur wrote:
>> Add new trap action HARD_DROP, which can be used by the
>> drivers to register traps, where it's impossible to get
>> packet reported to the devlink subsystem by the device
>> driver, because it's impossible to retrieve dropped packet
>> from the device itself.
>> In order to use this action, driver must also register
>> additional devlink operation - callback that is used
>> to retrieve number of packets that have been dropped by
>> the device.

>Are these global statistics about number of packets the hardware dropped
> for a specific reason or are these per-port statistics?

Global statistics. Basically, it’s the DROP action, with the only difference 
that device might be unable to post the packet to the devlink subsystem.
Also, as this is an action, it could also be altered: e.g. changed to ‘mirror’ 
or else.

> Anyway, this patch really needs to be marked as "RFC" since we cannot
> add infrastructure without anyone using it.

Will do.
Also, should I make a V2 patch, that will already hold the RFC tag and the 
changes (which include the commentaries fixes)?

> Additionally, the documentation
> (Documentation/networking/devlink/devlink-trap.rst) needs to be updated,
> netdevsim needs to be patched and the test over netdevsim
> (tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh) needs to
> be extended to cover the new functionality.

Okay. Will do.

>> @@ -9876,6 +9915,9 @@ void devlink_trap_report(struct devlink *devlink, 
>> struct sk_buff *skb,
>>  {
>>struct devlink_trap_item *trap_item = trap_ctx;
>> 
>> + if (trap_item->action == DEVLINK_TRAP_ACTION_HARD_DROP)
>> + return;

>How can this happen?

My bad. Will get removed in V2.

[PATCH iproute2-next] devlink: add support for port params get/set

2021-01-21 Thread Oleksandr Mazur
Add implementation for the port (named) parameters
getting/setting.
Kernel-side already has implemented devlink port params
get / set commands handling.

Signed-off-by: Oleksandr Mazur 
---
 devlink/devlink.c | 275 +-
 1 file changed, 269 insertions(+), 6 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 77185f7c..6be81e92 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2708,7 +2708,8 @@ static void pr_out_param_value(struct dl *dl, const char 
*nla_name,
}
 }
 
-static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array)
+static void pr_out_param(struct dl *dl, struct nlattr **tb, bool array,
+bool is_port_param)
 {
struct nlattr *nla_param[DEVLINK_ATTR_MAX + 1] = {};
struct nlattr *param_value_attr;
@@ -2716,6 +2717,7 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
int nla_type;
int err;
 
+
err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_PARAM], attr_cb, nla_param);
if (err != MNL_CB_OK)
return;
@@ -2725,9 +2727,15 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
return;
 
if (array)
-   pr_out_handle_start_arr(dl, tb);
+   if (is_port_param)
+   pr_out_port_handle_start_arr(dl, tb, false);
+   else
+   pr_out_handle_start_arr(dl, tb);
else
-   __pr_out_handle_start(dl, tb, true, false);
+   if (is_port_param)
+   pr_out_port_handle_start(dl, tb, false);
+   else
+   __pr_out_handle_start(dl, tb, true, false);
 
nla_type = mnl_attr_get_u8(nla_param[DEVLINK_ATTR_PARAM_TYPE]);
 
@@ -2747,7 +2755,10 @@ static void pr_out_param(struct dl *dl, struct nlattr 
**tb, bool array)
pr_out_entry_end(dl);
}
pr_out_array_end(dl);
-   pr_out_handle_end(dl);
+   if (is_port_param)
+   pr_out_port_handle_end(dl);
+   else
+   pr_out_handle_end(dl);
 }
 
 static int cmd_dev_param_show_cb(const struct nlmsghdr *nlh, void *data)
@@ -2760,7 +2771,7 @@ static int cmd_dev_param_show_cb(const struct nlmsghdr 
*nlh, void *data)
if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
!tb[DEVLINK_ATTR_PARAM])
return MNL_CB_ERROR;
-   pr_out_param(dl, tb, true);
+   pr_out_param(dl, tb, true, false);
return MNL_CB_OK;
 }
 
@@ -2958,6 +2969,21 @@ err_param_value_parse:
return err;
 }
 
+static int cmd_port_param_show_cb(const struct nlmsghdr *nlh, void *data)
+{
+   struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+   struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+   struct dl *dl = data;
+
+   mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+   if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
+   !tb[DEVLINK_ATTR_PORT_INDEX] || !tb[DEVLINK_ATTR_PARAM])
+   return MNL_CB_ERROR;
+
+   pr_out_param(dl, tb, true, true);
+   return MNL_CB_OK;
+}
+
 static int cmd_dev_param_show(struct dl *dl)
 {
uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
@@ -3703,6 +3729,8 @@ static void cmd_port_help(void)
pr_err("   devlink port split DEV/PORT_INDEX count COUNT\n");
pr_err("   devlink port unsplit DEV/PORT_INDEX\n");
pr_err("   devlink port function set DEV/PORT_INDEX [ hw_addr ADDR 
]\n");
+   pr_err("   devlink port param set DEV/PORT_INDEX name PARAMETER 
value VALUE cmode { permanent | driverinit | runtime }\n");
+   pr_err("   devlink port param show [DEV/PORT_INDEX name 
PARAMETER]\n");
pr_err("   devlink port health show [ DEV/PORT_INDEX reporter 
REPORTER_NAME ]\n");
 }
 
@@ -3937,6 +3965,31 @@ static int cmd_port_unsplit(struct dl *dl)
return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
 }
 
+static int cmd_port_param_show(struct dl *dl)
+{
+   uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
+   struct nlmsghdr *nlh;
+   int err;
+
+   if (dl_argc(dl) == 0)
+   flags |= NLM_F_DUMP;
+
+   nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_PORT_PARAM_GET, flags);
+
+   if (dl_argc(dl) > 0) {
+   err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLEP |
+   DL_OPT_PARAM_NAME, 0);
+   if (err)
+   return err;
+   }
+
+   pr_out_section_start(dl, "param");
+   err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_port_param_show_cb, dl);
+   pr_out_section_end(dl);
+
+   return err;
+}
+
 static void cmd_port_function_help(void)
 {
pr_err("Usage: devlink port function set DEV/PORT_INDEX [ hw_addr ADDR 
]\n");
@@ -3956,6 +4009,205 @@ sta

[PATCH net-next] net: core: devlink: add new trap action HARD_DROP

2021-01-21 Thread Oleksandr Mazur
Add new trap action HARD_DROP, which can be used by the
drivers to register traps, where it's impossible to get
packet reported to the devlink subsystem by the device
driver, because it's impossible to retrieve dropped packet
from the device itself.
In order to use this action, driver must also register
additional devlink operation - callback that is used
to retrieve number of packets that have been dropped by
the device.

Signed-off-by: Oleksandr Mazur 
---
 include/net/devlink.h| 10 
 include/uapi/linux/devlink.h |  4 
 net/core/devlink.c   | 44 +++-
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index f466819cc477..6811a614f6fd 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1294,6 +1294,16 @@ struct devlink_ops {
 const struct devlink_trap_group *group,
 enum devlink_trap_action action,
 struct netlink_ext_ack *extack);
+   /**
+* @trap_hard_drop_counter_get: Trap hard drop counter get function.
+*
+* Should be used by device drivers to report number of packets dropped
+* by the underlying device, that have been dropped because device
+* failed to pass the trapped packet.
+*/
+   int (*trap_hard_drop_counter_get)(struct devlink *devlink,
+ const struct devlink_trap *trap,
+ u64 *p_drops);
/**
 * @trap_policer_init: Trap policer initialization function.
 *
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index cf89c318f2ac..9247d9c7db03 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -261,12 +261,16 @@ enum {
  * enum devlink_trap_action - Packet trap action.
  * @DEVLINK_TRAP_ACTION_DROP: Packet is dropped by the device and a copy is not
  *sent to the CPU.
+ * @DEVLINK_TRAP_ACTION_HARD_DROP: Packet was dropped by the underlying device,
+ * and device cannot report packet to devlink
+ * (or inject it into the kernel RX path).
  * @DEVLINK_TRAP_ACTION_TRAP: The sole copy of the packet is sent to the CPU.
  * @DEVLINK_TRAP_ACTION_MIRROR: Packet is forwarded by the device and a copy is
  *  sent to the CPU.
  */
 enum devlink_trap_action {
DEVLINK_TRAP_ACTION_DROP,
+   DEVLINK_TRAP_ACTION_HARD_DROP,
DEVLINK_TRAP_ACTION_TRAP,
DEVLINK_TRAP_ACTION_MIRROR,
 };
diff --git a/net/core/devlink.c b/net/core/devlink.c
index ee828e4b1007..5a06e00429e1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6732,6 +6732,7 @@ devlink_trap_action_get_from_info(struct genl_info *info,
val = nla_get_u8(info->attrs[DEVLINK_ATTR_TRAP_ACTION]);
switch (val) {
case DEVLINK_TRAP_ACTION_DROP:
+   case DEVLINK_TRAP_ACTION_HARD_DROP:
case DEVLINK_TRAP_ACTION_TRAP:
case DEVLINK_TRAP_ACTION_MIRROR:
*p_trap_action = val;
@@ -6820,6 +6821,37 @@ static int devlink_trap_stats_put(struct sk_buff *msg,
return -EMSGSIZE;
 }
 
+static int
+devlink_trap_hard_drop_stats_put(struct sk_buff *msg,
+struct devlink *devlink,
+const struct devlink_trap_item *trap_item)
+{
+   struct nlattr *attr;
+   u64 drops;
+   int err;
+
+   err = devlink->ops->trap_hard_drop_counter_get(devlink, trap_item->trap,
+  );
+   if (err)
+   return err;
+
+   attr = nla_nest_start(msg, DEVLINK_ATTR_STATS);
+   if (!attr)
+   return -EMSGSIZE;
+
+   if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_DROPPED, drops,
+ DEVLINK_ATTR_PAD))
+   goto nla_put_failure;
+
+   nla_nest_end(msg, attr);
+
+   return 0;
+
+nla_put_failure:
+   nla_nest_cancel(msg, attr);
+   return -EMSGSIZE;
+}
+
 static int devlink_nl_trap_fill(struct sk_buff *msg, struct devlink *devlink,
const struct devlink_trap_item *trap_item,
enum devlink_command cmd, u32 portid, u32 seq,
@@ -6857,7 +6889,10 @@ static int devlink_nl_trap_fill(struct sk_buff *msg, 
struct devlink *devlink,
if (err)
goto nla_put_failure;
 
-   err = devlink_trap_stats_put(msg, trap_item->stats);
+   if (trap_item->action == DEVLINK_TRAP_ACTION_HARD_DROP)
+   err = devlink_trap_hard_drop_stats_put(msg, devlink, trap_item);
+   else
+   err = devlink_trap_stats_put(msg, trap_item->stats);
if (err)
goto nla_put_failure;
 
@@ -9697,6 +9732,10 @@ devlink_trap_re

[PATCH iproute2-next] devlink: add support for HARD_DROP trap action

2021-01-21 Thread Oleksandr Mazur
Add support for new HARD_DROP action, which is used for
trap hard drop statistics retrival. It's used whenever
device is unable to report trapped packet to the devlink
subsystem, and thus device could only state how many
packets have been dropped, without passing a packet
to the devlink subsystem to get track of traffic statistics.

Signed-off-by: Oleksandr Mazur 
---
 devlink/devlink.c| 4 
 include/uapi/linux/devlink.h | 4 
 2 files changed, 8 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index a2e06644..77185f7c 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1335,6 +1335,8 @@ static int trap_action_get(const char *actionstr,
 {
if (strcmp(actionstr, "drop") == 0) {
*p_action = DEVLINK_TRAP_ACTION_DROP;
+   } else if (strcmp(actionstr, "hard_drop") == 0) {
+   *p_action = DEVLINK_TRAP_ACTION_HARD_DROP;
} else if (strcmp(actionstr, "trap") == 0) {
*p_action = DEVLINK_TRAP_ACTION_TRAP;
} else if (strcmp(actionstr, "mirror") == 0) {
@@ -7726,6 +7728,8 @@ static const char *trap_action_name(uint8_t action)
switch (action) {
case DEVLINK_TRAP_ACTION_DROP:
return "drop";
+   case DEVLINK_TRAP_ACTION_HARD_DROP:
+   return "hard_drop";
case DEVLINK_TRAP_ACTION_TRAP:
return "trap";
case DEVLINK_TRAP_ACTION_MIRROR:
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 1414acee..ecee2541 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -261,12 +261,16 @@ enum {
  * enum devlink_trap_action - Packet trap action.
  * @DEVLINK_TRAP_ACTION_DROP: Packet is dropped by the device and a copy is not
  *sent to the CPU.
+ * @DEVLINK_TRAP_ACTION_HARD_DROP: Packet was dropped by the underlying device,
+ * and device cannot report packet to devlink
+ * (or inject it into the kernel RX path).
  * @DEVLINK_TRAP_ACTION_TRAP: The sole copy of the packet is sent to the CPU.
  * @DEVLINK_TRAP_ACTION_MIRROR: Packet is forwarded by the device and a copy is
  *  sent to the CPU.
  */
 enum devlink_trap_action {
DEVLINK_TRAP_ACTION_DROP,
+   DEVLINK_TRAP_ACTION_HARD_DROP,
DEVLINK_TRAP_ACTION_TRAP,
DEVLINK_TRAP_ACTION_MIRROR,
 };
-- 
2.17.1