Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: xiangxia.m.yue @ 2019-02-21  2:14 UTC (permalink / raw)
  To: saeedm, gerlitz.or; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1550715283-23579-1-git-send-email-xiangxia.m.yue@gmail.com>

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

If we try to offload decapsulation actions to VFs hw, we get the log [1].
It's not friendly, because the kind of net device is null, and we don't
know what '0' means.

[1] "mlx5_core 0000:05:01.2 vf_0: decapsulation offload is not supported for  net device (0)"

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index bdcc5e7..6911e0a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -620,8 +620,10 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
 						headers_c, headers_v);
 	} else {
 		netdev_warn(priv->netdev,
-			    "decapsulation offload is not supported for %s net device (%d)\n",
-			    mlx5e_netdev_kind(filter_dev), tunnel_type);
+			    "decapsulation offload is not supported for %s (kind: \"%s\")\n",
+			    netdev_name(filter_dev),
+			    mlx5e_netdev_kind(filter_dev));
+
 		return -EOPNOTSUPP;
 	}
 	return err;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next 3/5] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
From: xiangxia.m.yue @ 2019-02-21  2:14 UTC (permalink / raw)
  To: saeedm, gerlitz.or; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1550715283-23579-1-git-send-email-xiangxia.m.yue@gmail.com>

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch is a little improvement. Simplify the parse_tc_fdb_actions().

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index c4359f1..bbe2374 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2476,13 +2476,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
 
 static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
 				struct flow_action *flow_action,
-				struct mlx5e_tc_flow_parse_attr *parse_attr,
 				struct mlx5e_tc_flow *flow,
 				struct netlink_ext_ack *extack)
 {
 	struct pedit_headers_action hdrs[2] = {};
 	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
 	struct mlx5_esw_flow_attr *attr = flow->esw_attr;
+	struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
 	const struct ip_tunnel_info *info = NULL;
 	const struct flow_action_entry *act;
@@ -2797,7 +2797,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
 	if (err)
 		goto err_free;
 
-	err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
+	err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
 	if (err)
 		goto err_free;
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next 4/5] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
From: xiangxia.m.yue @ 2019-02-21  2:14 UTC (permalink / raw)
  To: saeedm, gerlitz.or; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1550715283-23579-1-git-send-email-xiangxia.m.yue@gmail.com>

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch deletes unnecessary setting of the esw_attr->parse_attr
to parse_attr in parse_tc_fdb_actions() because it is already done
by the mlx5e_flow_esw_attr_init() function.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index bbe2374..2d8ffdb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2567,7 +2567,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
 					out_dev->ifindex;
 				parse_attr->tun_info[attr->out_count] = *info;
 				encap = false;
-				attr->parse_attr = parse_attr;
 				attr->dests[attr->out_count].flags |=
 					MLX5_ESW_DEST_ENCAP;
 				attr->out_count++;
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next 5/5] net/mlx5e: Support enable/disable VFs link state on switchdev mode
From: xiangxia.m.yue @ 2019-02-21  2:14 UTC (permalink / raw)
  To: saeedm, gerlitz.or; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1550715283-23579-1-git-send-email-xiangxia.m.yue@gmail.com>

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

This patch allow users to enable/disable VFs link state
on switchdev mode.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 71c65cc..9f8761f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -1063,5 +1063,6 @@ netdev_features_t mlx5e_features_check(struct sk_buff *skb,
 int mlx5e_set_vf_rate(struct net_device *dev, int vf, int min_tx_rate, int max_tx_rate);
 int mlx5e_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi);
 int mlx5e_get_vf_stats(struct net_device *dev, int vf, struct ifla_vf_stats *vf_stats);
+int mlx5e_set_vf_link_state(struct net_device *dev, int vf, int link_state);
 #endif
 #endif /* __MLX5_EN_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 878b346..f7475ed 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3956,7 +3956,7 @@ static int mlx5_ifla_link2vport(u8 ifla_link)
 	return MLX5_VPORT_ADMIN_STATE_AUTO;
 }
 
-static int mlx5e_set_vf_link_state(struct net_device *dev, int vf,
+int mlx5e_set_vf_link_state(struct net_device *dev, int vf,
 				   int link_state)
 {
 	struct mlx5e_priv *priv = netdev_priv(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 287d48e..d270552 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -1316,6 +1316,7 @@ static int mlx5e_uplink_rep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan
 	.ndo_set_vf_rate         = mlx5e_set_vf_rate,
 	.ndo_get_vf_config       = mlx5e_get_vf_config,
 	.ndo_get_vf_stats        = mlx5e_get_vf_stats,
+	.ndo_set_vf_link_state   = mlx5e_set_vf_link_state,
 	.ndo_set_vf_vlan         = mlx5e_uplink_rep_set_vf_vlan,
 	.ndo_get_port_parent_id	 = mlx5e_rep_get_port_parent_id,
 };
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 00/11] Add support for devlink health
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin

This series adds support for devlink health commands:
 devlink health show     [ DEV reporter REPORTER_NAME ]
 devlink health recover    DEV reporter REPORTER_NAME
 devlink health diagnose   DEV reporter REPORTER_NAME
 devlink health dump show  DEV reporter REPORTER_NAME
 devlink health dump clear DEV reporter REPORTER_NAME
 devlink health set        DEV reporter REPORTER_NAME { grace_period | auto_recover } { msec | boolean }

The first patch refactors the validation of input parameters, which
grow way too long. Second and third patches fix bugs that were
discovered during the devlink health development. The forth patch adds
helper functions which enable output of value and labels separately.
Patches 5-10 add the devlink health functionality by command, the last
is the man page.

Changelog:
v2:
-Add patch #4.
-Separate patch "Add support for devlink health" into patches (5-10)
by command.
-Patch #1 Changed function's name dl_args_finding_required_validate
and a small refactor.
-Modify show command's output.

Note: this series (patch 0005 and on) can be applied after aligning
include/uapi/linux/devlink.h with its corresponding kernel's version. 

Aya Levin (11):
  devlink: Refactor validation of finding required arguments
  devlink: Fix print of uint64_t
  devlink: Fix boolean JSON print
  devlink: Add helper functions for name and value separately
  devlink: Add devlink health show command
  devlink: Add devlink health recover command
  devlink: Add devlink health diagnose command
  devlink: Add devlink health dump show command
  devlink: Add devlink health dump clear command
  devlink: Add devlink health set command
  devlink: Add devlink-health man page

 devlink/devlink.c         | 729 ++++++++++++++++++++++++++++++++++++++--------
 man/man8/devlink-health.8 | 197 +++++++++++++
 man/man8/devlink.8        |   7 +-
 3 files changed, 818 insertions(+), 115 deletions(-)
 create mode 100644 man/man8/devlink-health.8

-- 
2.14.1


^ permalink raw reply

* [PATCH v2 iproute2-next 10/11] devlink: Add devlink health set command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink set command which enables the user to configure parameters
related to the devlink health mechanism per reporter.
1) grace_period [msec] time interval between auto recoveries.
2) auto_recover [true/false] whether the devlink should execute automatic
recover on error.
Please note that this command fails (not supported) when the reporter
doesn't support a recovery method.
This patch also introduce a helper function to retrieve a boolean value
as an input parameter.
Example:
$ devlink health set pci/0000:00:09.0 reporter tx grace_period 3500
$ devlink health set pci/0000:00:09.0 reporter tx auto_recover false

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index f36bbeca5641..8834c1c5f517 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -207,6 +207,8 @@ static void ifname_map_free(struct ifname_map *ifname_map)
 #define DL_OPT_REGION_ADDRESS		BIT(23)
 #define DL_OPT_REGION_LENGTH		BIT(24)
 #define DL_OPT_HEALTH_REPORTER_NAME	BIT(25)
+#define DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD	BIT(26)
+#define DL_OPT_HEALTH_REPORTER_AUTO_RECOVER	BIT(27)
 
 struct dl_opts {
 	uint32_t present; /* flags of present items */
@@ -239,6 +241,8 @@ struct dl_opts {
 	uint64_t region_address;
 	uint64_t region_length;
 	const char *reporter_name;
+	uint64_t reporter_graceful_period;
+	bool reporter_auto_recover;
 };
 
 struct dl {
@@ -837,6 +841,24 @@ static int dl_argv_uint16_t(struct dl *dl, uint16_t *p_val)
 	return 0;
 }
 
+static int dl_argv_bool(struct dl *dl, bool *p_val)
+{
+	char *str = dl_argv_next(dl);
+	int err;
+
+	if (!str) {
+		pr_err("Boolean argument expected\n");
+		return -EINVAL;
+	}
+
+	err = strtobool(str, p_val);
+	if (err) {
+		pr_err("\"%s\" is not a valid boolean value\n", str);
+		return err;
+	}
+	return 0;
+}
+
 static int dl_argv_str(struct dl *dl, const char **p_str)
 {
 	const char *str = dl_argv_next(dl);
@@ -1252,6 +1274,21 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 			if (err)
 				return err;
 			o_found |= DL_OPT_HEALTH_REPORTER_NAME;
+		} else if (dl_argv_match(dl, "grace_period") &&
+			   (o_all & DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD)) {
+			dl_arg_inc(dl);
+			err = dl_argv_uint64_t(dl,
+					       &opts->reporter_graceful_period);
+			if (err)
+				return err;
+			o_found |= DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD;
+		} else if (dl_argv_match(dl, "auto_recover") &&
+			(o_all & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)) {
+			dl_arg_inc(dl);
+			err = dl_argv_bool(dl, &opts->reporter_auto_recover);
+			if (err)
+				return err;
+			o_found |= DL_OPT_HEALTH_REPORTER_AUTO_RECOVER;
 		} else {
 			pr_err("Unknown option \"%s\"\n", dl_argv(dl));
 			return -EINVAL;
@@ -1352,6 +1389,14 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 	if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
 		mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME,
 				  opts->reporter_name);
+	if (opts->present & DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD)
+		mnl_attr_put_u64(nlh,
+				 DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD,
+				 opts->reporter_graceful_period);
+	if (opts->present & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)
+		mnl_attr_put_u8(nlh, DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,
+				opts->reporter_auto_recover);
+
 }
 
 static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
@@ -5765,6 +5810,23 @@ static int cmd_region(struct dl *dl)
 	return -ENOENT;
 }
 
+static int cmd_health_set_params(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_SET,
+			       NLM_F_REQUEST | NLM_F_ACK);
+	err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME,
+			    DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD |
+			    DL_OPT_HEALTH_REPORTER_AUTO_RECOVER);
+	if (err)
+		return err;
+
+	dl_opts_put(nlh, dl);
+	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
 static int cmd_health_dump_clear(struct dl *dl)
 {
 	struct nlmsghdr *nlh;
@@ -6130,6 +6192,7 @@ static void cmd_health_help(void)
 	pr_err("       devlink health diagnose DEV reporter REPORTER_NAME\n");
 	pr_err("       devlink health dump show DEV reporter REPORTER_NAME\n");
 	pr_err("       devlink health dump clear DEV reporter REPORTER_NAME\n");
+	pr_err("       devlink health set DEV reporter REPORTER_NAME { grace_period | auto_recover } { msec | boolean }\n");
 }
 
 static int cmd_health(struct dl *dl)
@@ -6156,6 +6219,9 @@ static int cmd_health(struct dl *dl)
 			dl_arg_inc(dl);
 			return cmd_health_dump_clear(dl);
 		}
+	} else if (dl_argv_match(dl, "set")) {
+		dl_arg_inc(dl);
+		return cmd_health_set_params(dl);
 	}
 
 	pr_err("Command \"%s\" not found\n", dl_argv(dl));
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 06/11] devlink: Add devlink health recover command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink health recover command which enables the user to initiate a
recovery on a reporter (if a recovery cb was supplied by the reporter).
This operation will increment the recoveries counter displayed in the
show command.
Example:
$ devlink health recover pci/0000:00:09.0 reporter tx

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 5685f1e5113a..3ec0ef4b95b2 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5764,6 +5764,23 @@ static int cmd_region(struct dl *dl)
 	return -ENOENT;
 }
 
+static int cmd_health_recover(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
+			       NLM_F_REQUEST | NLM_F_ACK);
+
+	err = dl_argv_parse_put(nlh, dl,
+				DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+	if (err)
+		return err;
+
+	dl_opts_put(nlh, dl);
+	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
 enum devlink_health_reporter_state {
 	DEVLINK_HEALTH_REPORTER_STATE_HEALTHY,
 	DEVLINK_HEALTH_REPORTER_STATE_ERROR,
@@ -5904,6 +5921,7 @@ static int cmd_health_show(struct dl *dl)
 static void cmd_health_help(void)
 {
 	pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
+	pr_err("       devlink health recover DEV reporter REPORTER_NAME\n");
 }
 
 static int cmd_health(struct dl *dl)
@@ -5915,6 +5933,9 @@ static int cmd_health(struct dl *dl)
 		   dl_argv_match(dl, "list") || dl_no_arg(dl)) {
 		dl_arg_inc(dl);
 		return cmd_health_show(dl);
+	} else if (dl_argv_match(dl, "recover")) {
+		dl_arg_inc(dl);
+		return cmd_health_recover(dl);
 	}
 
 	pr_err("Command \"%s\" not found\n", dl_argv(dl));
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 03/11] devlink: Fix boolean JSON print
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

This patch removes the inverted commas from boolean values in JSON
format: true/false instead of "true"/"false".

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 2c921f3811fc..b073ae020d52 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1603,10 +1603,10 @@ static void pr_out_str(struct dl *dl, const char *name, const char *val)
 
 static void pr_out_bool(struct dl *dl, const char *name, bool val)
 {
-	if (val)
-		pr_out_str(dl, name, "true");
+	if (dl->json_output)
+		jsonw_bool_field(dl->jw, name, val);
 	else
-		pr_out_str(dl, name, "false");
+		pr_out_str(dl, name, val ? "true" : "false");
 }
 
 static void pr_out_uint(struct dl *dl, const char *name, unsigned int val)
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 08/11] devlink: Add devlink health dump show command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink dump show command which displays the last saved dump.
Devlink health saves a single dump. If a dump is not already stored
by the devlink for this reporter, devlink generates a new dump. The dump
can be generated automatically when a reporter reports on an
error or manually by user's request.
The dump's output is defined by the reporter. The command uses the
infra structure for flexible format output introduced in previous patch.
Example:
$ devlink health dump show pci/0000:00:09.0 reporter tx

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index ef6a72f0b04e..627f8e819aa5 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5942,6 +5942,11 @@ static int cmd_health_object_common(struct dl *dl, uint8_t cmd)
 	return err;
 }
 
+static int cmd_health_dump_show(struct dl *dl)
+{
+	return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET);
+}
+
 static int cmd_health_diagnose(struct dl *dl)
 {
 	return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE);
@@ -6106,6 +6111,7 @@ static void cmd_health_help(void)
 	pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
 	pr_err("       devlink health recover DEV reporter REPORTER_NAME\n");
 	pr_err("       devlink health diagnose DEV reporter REPORTER_NAME\n");
+	pr_err("       devlink health dump show DEV reporter REPORTER_NAME\n");
 }
 
 static int cmd_health(struct dl *dl)
@@ -6123,6 +6129,12 @@ static int cmd_health(struct dl *dl)
 	} else if (dl_argv_match(dl, "diagnose")) {
 		dl_arg_inc(dl);
 		return cmd_health_diagnose(dl);
+	} else if (dl_argv_match(dl, "dump")) {
+		dl_arg_inc(dl);
+		if (dl_argv_match(dl, "show")) {
+			dl_arg_inc(dl);
+			return cmd_health_dump_show(dl);
+		}
 	}
 
 	pr_err("Command \"%s\" not found\n", dl_argv(dl));
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 09/11] devlink: Add devlink health dump clear command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink dump clear command which deletes the last saved dump file.
Clearing the last saved dump enables a new dump file to be saved.
Example:
$ devlink health dump clear pci/0000:00:09.0 reporter tx

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 627f8e819aa5..f36bbeca5641 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5765,6 +5765,23 @@ static int cmd_region(struct dl *dl)
 	return -ENOENT;
 }
 
+static int cmd_health_dump_clear(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
+			       NLM_F_REQUEST | NLM_F_ACK);
+
+	err = dl_argv_parse_put(nlh, dl,
+				DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+	if (err)
+		return err;
+
+	dl_opts_put(nlh, dl);
+	return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
 static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
 {
 	uint8_t *data;
@@ -6112,6 +6129,7 @@ static void cmd_health_help(void)
 	pr_err("       devlink health recover DEV reporter REPORTER_NAME\n");
 	pr_err("       devlink health diagnose DEV reporter REPORTER_NAME\n");
 	pr_err("       devlink health dump show DEV reporter REPORTER_NAME\n");
+	pr_err("       devlink health dump clear DEV reporter REPORTER_NAME\n");
 }
 
 static int cmd_health(struct dl *dl)
@@ -6134,6 +6152,9 @@ static int cmd_health(struct dl *dl)
 		if (dl_argv_match(dl, "show")) {
 			dl_arg_inc(dl);
 			return cmd_health_dump_show(dl);
+		} else if (dl_argv_match(dl, "clear")) {
+			dl_arg_inc(dl);
+			return cmd_health_dump_clear(dl);
 		}
 	}
 
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 11/11] devlink: Add devlink-health man page
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add a man page describing devlink health's command set. Also add a
reference link from devlink main man page.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 man/man8/devlink-health.8 | 197 ++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/devlink.8        |   7 +-
 2 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 man/man8/devlink-health.8

diff --git a/man/man8/devlink-health.8 b/man/man8/devlink-health.8
new file mode 100644
index 000000000000..7ed0ae4534dc
--- /dev/null
+++ b/man/man8/devlink-health.8
@@ -0,0 +1,197 @@
+.TH DEVLINK\-HEALTH 8 "20 Feb 2019" "iproute2" "Linux"
+.SH NAME
+devlink-health \- devlink health reporting and recovery
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+.ti -8
+.B devlink
+.RI "[ " OPTIONS " ]"
+.B health
+.RI  " { " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.IR OPTIONS " := { "
+\fB\-V\fR[\fIersion\fR] }
+
+.ti -8
+.BR "devlink health show"
+.RI "[ " DEV ""
+.B reporter
+.RI ""REPORTER " ] "
+
+.ti -8
+.BR "devlink health recover"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health diagnose"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health dump show"
+.RI "" DEV ""
+.B  reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health dump clear"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health set"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+.RI " { "
+.B grace_period | auto_recover
+.RI " } { "
+.RI "" msec ""
+.RI "|"
+.RI "" boolean ""
+.RI " } "
+.ti -8
+.B devlink health help
+
+.SH "DESCRIPTION"
+.SS devlink health show - Show status and configuration on all supported reporters on all devlink devices.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health recover - Initiate a recovery operation on a reporter.
+This action performs a recovery and increases the recoveries counter on success.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health diagnose - Retrieve diagnostics data on a reporter.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health dump show - Display the last saved dump.
+
+.PD 0
+.P
+devlink health saves a single dump per reporter. If an dump is
+.P
+not already stored by the Devlink, this command will generate a new
+.P
+dump. The dump can be generated either automatically when a
+.P
+reporter reports on an error or manually at the user's request.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health dump clear - Delete the saved dump.
+Deleting the saved dump enables a generation of a new dump on
+.PD 0
+.P
+the next "devlink health dump show" command.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health set - Enable the user to configure:
+.PD 0
+1) grace_period [msec] - Time interval between consecutive auto recoveries.
+.P
+2) auto_recover [true/false] - Indicates whether the devlink should execute automatic recover on error.
+.P
+Please note that this command is not supported on a reporter which
+doesn't support a recovery method.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SH "EXAMPLES"
+.PP
+devlink health show
+.RS 4
+List status and configuration of available reporters on devices.
+.RE
+.PP
+devlink health recover pci/0000:00:09.0 reporter tx
+.RS 4
+Initiate recovery on tx reporter registered on pci/0000:00:09.0.
+.RE
+.PP
+devlink health diagnose pci/0000:00:09.0 reporter tx
+.RS 4
+List diagnostics data on the specified device and reporter.
+.RE
+.PP
+devlink health dump show pci/0000:00:09.0 reporter tx
+.RS 4
+Display the last saved dump on the specified device and reporter.
+.RE
+.PP
+devlink health dump clear pci/0000:00:09.0 reporter tx
+.RS 4
+Delete saved dump on the specified device and reporter.
+.RE
+.PP
+devlink health set pci/0000:00:09.0 reporter tx grace_period 3500
+.RS 4
+Set time interval between auto recoveries to minimum of 3500 msec on
+the specified device and reporter.
+.RE
+.PP
+devlink health set pci/0000:00:09.0 reporter tx auto_recover false
+.RS 4
+Turn off auto recovery on the specified device and reporter.
+.RE
+.SH SEE ALSO
+.BR devlink (8),
+.BR devlink-dev (8),
+.BR devlink-port (8),
+.BR devlink-param (8),
+.BR devlink-region (8),
+.br
+
+.SH AUTHOR
+Aya Levin <ayal@mellanox.com>
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index 8d527e7e1d60..13d4dcd908b3 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -7,7 +7,7 @@ devlink \- Devlink tool
 .in +8
 .ti -8
 .B devlink
-.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource | region " } { " COMMAND " | "
+.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource | region | health " } { " COMMAND " | "
 .BR help " }"
 .sp
 
@@ -78,6 +78,10 @@ Turn on verbose output.
 .B region
 - devlink address region access
 
+.TP
+.B health
+- devlink reporting and recovery
+
 .SS
 .I COMMAND
 
@@ -109,6 +113,7 @@ Exit status is 0 if command was successful or a positive integer upon failure.
 .BR devlink-sb (8),
 .BR devlink-resource (8),
 .BR devlink-region (8),
+.BR devlink-health (8),
 .br
 
 .SH REPORTING BUGS
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add a new helper functions which outputs only values (without name
label) for different types: boolean, uint, uint64, string and binary.
In addition add a helper function which prints only the name label.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
---
 devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index b073ae020d52..5d69c4f24f29 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
 		pr_out("\n");
 }
 
-
 static void pr_out_str(struct dl *dl, const char *name, const char *val)
 {
 	if (dl->json_output) {
@@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
 	}
 }
 
+static void pr_out_bool_value(struct dl *dl, bool value)
+{
+	if (dl->json_output)
+		jsonw_bool(dl->jw, value);
+	else
+		pr_out(" %s", value ? "true" : "false");
+}
+
+static void pr_out_uint_value(struct dl *dl, unsigned int value)
+{
+	if (dl->json_output)
+		jsonw_uint(dl->jw, value);
+	else
+		pr_out(" %u", value);
+}
+
+static void pr_out_uint64_value(struct dl *dl, uint64_t value)
+{
+	if (dl->json_output)
+		jsonw_u64(dl->jw, value);
+	else
+		pr_out(" %lu", value);
+}
+
+static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
+{
+	int i = 1;
+
+	if (dl->json_output)
+		jsonw_start_array(dl->jw);
+	else
+		pr_out("\n");
+
+	while (i < len) {
+		if (dl->json_output) {
+			jsonw_printf(dl->jw, "%d", data[i]);
+		} else {
+			pr_out(" %02x", data[i]);
+			if (!(i % 16))
+				pr_out("\n");
+		}
+		i++;
+	}
+	if (dl->json_output)
+		jsonw_end_array(dl->jw);
+	else if ((i - 1) % 16)
+		pr_out("\n");
+}
+
+static void pr_out_str_value(struct dl *dl, const char *value)
+{
+	if (dl->json_output)
+		jsonw_string(dl->jw, value);
+	else
+		pr_out(" %s", value);
+}
+
+static void pr_out_name(struct dl *dl, const char *name)
+{
+	if (dl->json_output)
+		jsonw_name(dl->jw, name);
+	else
+		pr_out(" %s:", name);
+}
+
 static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
 {
 	if (dl->json_output) {
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 05/11] devlink: Add devlink health show command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink health show command which displays status and configuration
info on a specific reporter on a device or dump the info on all
reporters on all devices. The patch also contains helper functions to
display status and dump's time stamp.
Example:
$ devlink health show pci/0000:00:09.0 reporter tx
pci/0000:00:09.0:
 name tx
  state healthy error 0 recover 1 last_dump_date 2019-02-14 last_dump_time 10:10:10 grace_period 600 auto_recover true
$ devlink health show pci/0000:00:09.0 reporter tx -jp
{
 "health":{
  "pci/0000:00:0a.0":[
     {
     "name":"tx",
     "state":"healthy",
     "error":0,
     "recover":1,
     "last_dump_date":"2019-Feb-14",
     "last_dump_time":"10:10:10",
     "grace_period":600,
     "auto_recover":true
    }
  ]
}

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
---
 devlink/devlink.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 187 insertions(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 5d69c4f24f29..5685f1e5113a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -22,6 +22,7 @@
 #include <linux/devlink.h>
 #include <libmnl/libmnl.h>
 #include <netinet/ether.h>
+#include <sys/sysinfo.h>
 
 #include "SNAPSHOT.h"
 #include "list.h"
@@ -41,6 +42,10 @@
 #define PARAM_CMODE_PERMANENT_STR "permanent"
 #define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
 
+#define HEALTH_REPORTER_STATE_HEALTHY_STR "healthy"
+#define HEALTH_REPORTER_STATE_ERROR_STR "error"
+#define HEALTH_REPORTER_TIMESTAMP_FMT_LEN 80
+
 static int g_new_line_count;
 
 #define pr_err(args...) fprintf(stderr, ##args)
@@ -200,6 +205,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
 #define DL_OPT_REGION_SNAPSHOT_ID	BIT(22)
 #define DL_OPT_REGION_ADDRESS		BIT(23)
 #define DL_OPT_REGION_LENGTH		BIT(24)
+#define DL_OPT_HEALTH_REPORTER_NAME	BIT(25)
 
 struct dl_opts {
 	uint32_t present; /* flags of present items */
@@ -231,6 +237,7 @@ struct dl_opts {
 	uint32_t region_snapshot_id;
 	uint64_t region_address;
 	uint64_t region_length;
+	const char *reporter_name;
 };
 
 struct dl {
@@ -391,6 +398,13 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_INFO_VERSION_STORED] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_INFO_VERSION_NAME] = MNL_TYPE_STRING,
 	[DEVLINK_ATTR_INFO_VERSION_VALUE] = MNL_TYPE_STRING,
+	[DEVLINK_ATTR_HEALTH_REPORTER] = MNL_TYPE_NESTED,
+	[DEVLINK_ATTR_HEALTH_REPORTER_NAME] = MNL_TYPE_STRING,
+	[DEVLINK_ATTR_HEALTH_REPORTER_STATE] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] = MNL_TYPE_U64,
+	[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] = MNL_TYPE_U64,
+	[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS] = MNL_TYPE_U64,
+	[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = MNL_TYPE_U64,
 };
 
 static int attr_cb(const struct nlattr *attr, void *data)
@@ -976,6 +990,7 @@ static const struct dl_args_metadata dl_args_required[] = {
 	{DL_OPT_REGION_SNAPSHOT_ID,   "Region snapshot id expected."},
 	{DL_OPT_REGION_ADDRESS,	      "Region address value expected."},
 	{DL_OPT_REGION_LENGTH,	      "Region length value expected."},
+	{DL_OPT_HEALTH_REPORTER_NAME, "Reporter's name is expected."},
 };
 
 static int dl_args_finding_required_validate(uint32_t o_required,
@@ -1229,6 +1244,13 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 			if (err)
 				return err;
 			o_found |= DL_OPT_REGION_LENGTH;
+		} else if (dl_argv_match(dl, "reporter") &&
+			   (o_all & DL_OPT_HEALTH_REPORTER_NAME)) {
+			dl_arg_inc(dl);
+			err = dl_argv_str(dl, &opts->reporter_name);
+			if (err)
+				return err;
+			o_found |= DL_OPT_HEALTH_REPORTER_NAME;
 		} else {
 			pr_err("Unknown option \"%s\"\n", dl_argv(dl));
 			return -EINVAL;
@@ -1326,6 +1348,9 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 	if (opts->present & DL_OPT_REGION_LENGTH)
 		mnl_attr_put_u64(nlh, DEVLINK_ATTR_REGION_CHUNK_LEN,
 				 opts->region_length);
+	if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
+		mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME,
+				  opts->reporter_name);
 }
 
 static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
@@ -5739,11 +5764,168 @@ static int cmd_region(struct dl *dl)
 	return -ENOENT;
 }
 
+enum devlink_health_reporter_state {
+	DEVLINK_HEALTH_REPORTER_STATE_HEALTHY,
+	DEVLINK_HEALTH_REPORTER_STATE_ERROR,
+};
+
+static const char *health_state_name(uint8_t state)
+{
+	switch (state) {
+	case DEVLINK_HEALTH_REPORTER_STATE_HEALTHY:
+		return HEALTH_REPORTER_STATE_HEALTHY_STR;
+	case DEVLINK_HEALTH_REPORTER_STATE_ERROR:
+		return HEALTH_REPORTER_STATE_ERROR_STR;
+	default:
+		return "<unknown state>";
+	}
+}
+
+static void format_logtime(uint64_t time_ms, char *ts_date, char *ts_time)
+{
+	struct sysinfo s_info;
+	struct tm *info;
+	time_t now, sec;
+	int err;
+
+	time(&now);
+	info = localtime(&now);
+	err = sysinfo(&s_info);
+	if (err)
+		goto out;
+	/* Subtract uptime in sec from now yields the time of system
+	 * uptime. To this, add time_ms which is the amount of
+	 * milliseconds elapsed between uptime and the dump taken.
+	 */
+	sec = now - s_info.uptime + time_ms / 1000;
+	info = localtime(&sec);
+out:
+	strftime(ts_date, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%Y-%m-%d", info);
+	strftime(ts_time, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%H:%M:%S", info);
+}
+
+static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
+{
+	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+	enum devlink_health_reporter_state state;
+	const struct nlattr *attr;
+	uint64_t time_ms;
+	int err;
+
+	state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;
+
+	err = mnl_attr_parse_nested(tb_health[DEVLINK_ATTR_HEALTH_REPORTER],
+				    attr_cb, tb);
+	if (err != MNL_CB_OK)
+		return;
+
+	if (!tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME] ||
+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] ||
+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] ||
+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE])
+		return;
+
+	pr_out_handle_start_arr(dl, tb_health);
+
+	pr_out_str(dl, "name",
+		   mnl_attr_get_str(tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
+	if (!dl->json_output) {
+		__pr_out_newline();
+		__pr_out_indent_inc();
+	}
+	state = mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE]);
+	pr_out_str(dl, "state", health_state_name(state));
+	pr_out_u64(dl, "error",
+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
+	pr_out_u64(dl, "recover",
+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
+	if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
+		char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+		char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+		attr = tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
+		time_ms = mnl_attr_get_u64(attr);
+		format_logtime(time_ms, dump_date, dump_time);
+
+		pr_out_str(dl, "last_dump_date", dump_date);
+		pr_out_str(dl, "last_dump_time", dump_time);
+	}
+	if (tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
+		pr_out_u64(dl, "grace_period",
+			   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]));
+	if (tb[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER])
+		pr_out_bool(dl, "auto_recover",
+			    mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER]));
+
+	__pr_out_indent_dec();
+	pr_out_handle_end(dl);
+}
+
+static int cmd_health_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_HEALTH_REPORTER])
+		return MNL_CB_ERROR;
+
+	pr_out_health(dl, tb);
+
+	return MNL_CB_OK;
+}
+
+static int cmd_health_show(struct dl *dl)
+{
+	struct nlmsghdr *nlh;
+	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
+	int err;
+
+	if (dl_argc(dl) == 0)
+		flags |= NLM_F_DUMP;
+	nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_GET,
+			       flags);
+
+	if (dl_argc(dl) > 0) {
+		err = dl_argv_parse_put(nlh, dl,
+					DL_OPT_HANDLE |
+					DL_OPT_HEALTH_REPORTER_NAME, 0);
+		if (err)
+			return err;
+	}
+	pr_out_section_start(dl, "health");
+
+	err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_health_show_cb, dl);
+	pr_out_section_end(dl);
+	return err;
+}
+
+static void cmd_health_help(void)
+{
+	pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
+}
+
+static int cmd_health(struct dl *dl)
+{
+	if (dl_argv_match(dl, "help")) {
+		cmd_health_help();
+		return 0;
+	} else if (dl_argv_match(dl, "show") ||
+		   dl_argv_match(dl, "list") || dl_no_arg(dl)) {
+		dl_arg_inc(dl);
+		return cmd_health_show(dl);
+	}
+
+	pr_err("Command \"%s\" not found\n", dl_argv(dl));
+	return -ENOENT;
+}
+
 static void help(void)
 {
 	pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
 	       "       devlink [ -f[orce] ] -b[atch] filename\n"
-	       "where  OBJECT := { dev | port | sb | monitor | dpipe | resource | region }\n"
+	       "where  OBJECT := { dev | port | sb | monitor | dpipe | resource | region | health }\n"
 	       "       OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] }\n");
 }
 
@@ -5776,7 +5958,11 @@ static int dl_cmd(struct dl *dl, int argc, char **argv)
 	} else if (dl_argv_match(dl, "region")) {
 		dl_arg_inc(dl);
 		return cmd_region(dl);
+	} else if (dl_argv_match(dl, "health")) {
+		dl_arg_inc(dl);
+		return cmd_health(dl);
 	}
+
 	pr_err("Object \"%s\" not found\n", dl_argv(dl));
 	return -ENOENT;
 }
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 07/11] devlink: Add devlink health diagnose command
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Add devlink health diagnose command: enabling retrieval of diagnostics data
by the user on a reporter on a device. The command's output is a
free text defined by the reporter.

This patch also introduces an infra structure for flexible format
output. This allow the  command to display different data fields
according to the reporter.
Example:
$ devlink health diagnose pci/0000:00:0a.0 reporter tx
SQs:
  sqn: 4403 HW state: 1 stopped: false
  sqn: 4408 HW state: 1 stopped: false
  sqn: 4413 HW state: 1 stopped: false
  sqn: 4418 HW state: 1 stopped: false
  sqn: 4423 HW state: 1 stopped: false

$ devlink health diagnose pci/0000:00:0a.0 reporter tx -jp
{
 "SQs":[
      {
       "sqn":4403,
       "HW state":1,
       "stopped":false
     },
      {
       "sqn":4408,
       "HW state":1,
       "stopped":false
     },
      {
       "sqn":4413,
       "HW state":1,
       "stopped":false
     },
      {
       "sqn":4418,
       "HW state":1,
       "stopped":false
     },
      {
       "sqn":4423,
       "HW state":1,
       "stopped":false
     }
   ]
}

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
---
 devlink/devlink.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 187 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 3ec0ef4b95b2..ef6a72f0b04e 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -23,6 +23,7 @@
 #include <libmnl/libmnl.h>
 #include <netinet/ether.h>
 #include <sys/sysinfo.h>
+#include <sys/queue.h>
 
 #include "SNAPSHOT.h"
 #include "list.h"
@@ -5764,6 +5765,188 @@ static int cmd_region(struct dl *dl)
 	return -ENOENT;
 }
 
+static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
+{
+	uint8_t *data;
+	uint32_t len;
+
+	switch (type) {
+	case MNL_TYPE_FLAG:
+		pr_out_bool_value(dl, mnl_attr_get_u8(nl_data));
+		break;
+	case MNL_TYPE_U8:
+		pr_out_uint_value(dl, mnl_attr_get_u8(nl_data));
+		break;
+	case MNL_TYPE_U16:
+		pr_out_uint_value(dl, mnl_attr_get_u16(nl_data));
+		break;
+	case MNL_TYPE_U32:
+		pr_out_uint_value(dl, mnl_attr_get_u32(nl_data));
+		break;
+	case MNL_TYPE_U64:
+		pr_out_uint64_value(dl, mnl_attr_get_u64(nl_data));
+		break;
+	case MNL_TYPE_NUL_STRING:
+		pr_out_str_value(dl, mnl_attr_get_str(nl_data));
+		break;
+	case MNL_TYPE_BINARY:
+		len = mnl_attr_get_payload_len(nl_data);
+		data = mnl_attr_get_payload(nl_data);
+		pr_out_binary_value(dl, data, len);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return MNL_CB_OK;
+}
+
+struct nest_qentry {
+	int attr_type;
+	TAILQ_ENTRY(nest_qentry) nest_entries;
+};
+
+struct fmsg_cb_data {
+	struct dl *dl;
+	uint8_t value_type;
+	TAILQ_HEAD(, nest_qentry) qhead;
+};
+
+static int cmd_fmsg_nest_queue(struct fmsg_cb_data *fmsg_data,
+			       uint8_t *attr_value, bool insert)
+{
+	struct nest_qentry *entry = NULL;
+
+	if (insert) {
+		entry = malloc(sizeof(struct nest_qentry));
+		if (!entry)
+			return -ENOMEM;
+
+		entry->attr_type = *attr_value;
+		TAILQ_INSERT_HEAD(&fmsg_data->qhead, entry, nest_entries);
+	} else {
+		if (TAILQ_EMPTY(&fmsg_data->qhead))
+			return MNL_CB_ERROR;
+		entry = TAILQ_FIRST(&fmsg_data->qhead);
+		*attr_value = entry->attr_type;
+		TAILQ_REMOVE(&fmsg_data->qhead, entry, nest_entries);
+		free(entry);
+	}
+	return MNL_CB_OK;
+}
+
+static int cmd_fmsg_nest(struct fmsg_cb_data *fmsg_data, uint8_t nest_value,
+			 bool start)
+{
+	struct dl *dl = fmsg_data->dl;
+	uint8_t value = nest_value;
+	int err;
+
+	err = cmd_fmsg_nest_queue(fmsg_data, &value, start);
+	if (err != MNL_CB_OK)
+		return err;
+
+	switch (value) {
+	case DEVLINK_ATTR_FMSG_OBJ_NEST_START:
+		if (start)
+			pr_out_entry_start(dl);
+		else
+			pr_out_entry_end(dl);
+		break;
+	case DEVLINK_ATTR_FMSG_PAIR_NEST_START:
+		break;
+	case DEVLINK_ATTR_FMSG_ARR_NEST_START:
+		if (dl->json_output) {
+			if (start)
+				jsonw_start_array(dl->jw);
+			else
+				jsonw_end_array(dl->jw);
+		} else {
+			if (start) {
+				__pr_out_newline();
+				__pr_out_indent_inc();
+			} else {
+				__pr_out_indent_dec();
+			}
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+	return MNL_CB_OK;
+}
+
+static int cmd_fmsg_object_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+	struct fmsg_cb_data *fmsg_data = data;
+	struct dl *dl = fmsg_data->dl;
+	struct nlattr *nla_object;
+	int attr_type;
+	int err;
+
+	mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+	if (!tb[DEVLINK_ATTR_FMSG])
+		return MNL_CB_ERROR;
+
+	mnl_attr_for_each_nested(nla_object, tb[DEVLINK_ATTR_FMSG]) {
+		attr_type = mnl_attr_get_type(nla_object);
+		switch (attr_type) {
+		case DEVLINK_ATTR_FMSG_OBJ_NEST_START:
+		case DEVLINK_ATTR_FMSG_PAIR_NEST_START:
+		case DEVLINK_ATTR_FMSG_ARR_NEST_START:
+			err = cmd_fmsg_nest(fmsg_data, attr_type, true);
+			if (err != MNL_CB_OK)
+				return err;
+			break;
+		case DEVLINK_ATTR_FMSG_NEST_END:
+			err = cmd_fmsg_nest(fmsg_data, attr_type, false);
+			if (err != MNL_CB_OK)
+				return err;
+			break;
+		case DEVLINK_ATTR_FMSG_OBJ_NAME:
+			pr_out_name(dl, mnl_attr_get_str(nla_object));
+			break;
+		case DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE:
+			fmsg_data->value_type = mnl_attr_get_u8(nla_object);
+			break;
+		case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA:
+			err = fmsg_value_show(dl, fmsg_data->value_type,
+					      nla_object);
+			if (err != MNL_CB_OK)
+				return err;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return MNL_CB_OK;
+}
+
+static int cmd_health_object_common(struct dl *dl, uint8_t cmd)
+{
+	struct fmsg_cb_data data;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = mnlg_msg_prepare(dl->nlg, cmd,  NLM_F_REQUEST | NLM_F_ACK);
+
+	err = dl_argv_parse_put(nlh, dl,
+				DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+	if (err)
+		return err;
+
+	data.dl = dl;
+	TAILQ_INIT(&data.qhead);
+	err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_fmsg_object_cb, &data);
+	return err;
+}
+
+static int cmd_health_diagnose(struct dl *dl)
+{
+	return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE);
+}
+
 static int cmd_health_recover(struct dl *dl)
 {
 	struct nlmsghdr *nlh;
@@ -5922,6 +6105,7 @@ static void cmd_health_help(void)
 {
 	pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
 	pr_err("       devlink health recover DEV reporter REPORTER_NAME\n");
+	pr_err("       devlink health diagnose DEV reporter REPORTER_NAME\n");
 }
 
 static int cmd_health(struct dl *dl)
@@ -5936,6 +6120,9 @@ static int cmd_health(struct dl *dl)
 	} else if (dl_argv_match(dl, "recover")) {
 		dl_arg_inc(dl);
 		return cmd_health_recover(dl);
+	} else if (dl_argv_match(dl, "diagnose")) {
+		dl_arg_inc(dl);
+		return cmd_health_diagnose(dl);
 	}
 
 	pr_err("Command \"%s\" not found\n", dl_argv(dl));
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 02/11] devlink: Fix print of uint64_t
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

This patch prints uint64_t with its corresponding format and avoid implicit
cast to uint32_t.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index fbaf9c9fabda..2c921f3811fc 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1626,7 +1626,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
 	if (val == (uint64_t) -1)
 		return pr_out_str(dl, name, "unlimited");
 
-	return pr_out_uint(dl, name, val);
+	if (dl->json_output) {
+		jsonw_u64_field(dl->jw, name, val);
+	} else {
+		if (g_indent_newline)
+			pr_out("%s %lu", name, val);
+		else
+			pr_out(" %s %lu", name, val);
+	}
 }
 
 static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
-- 
2.14.1


^ permalink raw reply related

* [PATCH v2 iproute2-next 01/11] devlink: Refactor validation of finding required arguments
From: Aya Levin @ 2019-02-21 13:42 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1550756567-18227-1-git-send-email-ayal@mellanox.com>

Introducing argument's metadata structure matching a bitmap flag per
required argument and an error message if missing. Using this static
array to refactor validation of finding required arguments in devlink
command line and to ease further maintenance.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 devlink/devlink.c | 153 ++++++++++++++++--------------------------------------
 1 file changed, 45 insertions(+), 108 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index d823512a4030..fbaf9c9fabda 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -39,6 +39,7 @@
 #define PARAM_CMODE_RUNTIME_STR "runtime"
 #define PARAM_CMODE_DRIVERINIT_STR "driverinit"
 #define PARAM_CMODE_PERMANENT_STR "permanent"
+#define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
 
 static int g_new_line_count;
 
@@ -950,6 +951,49 @@ static int param_cmode_get(const char *cmodestr,
 	return 0;
 }
 
+struct dl_args_metadata {
+	uint32_t o_flag;
+	char err_msg[DL_ARGS_REQUIRED_MAX_ERR_LEN];
+};
+
+static const struct dl_args_metadata dl_args_required[] = {
+	{DL_OPT_PORT_TYPE,	      "Port type not set."},
+	{DL_OPT_PORT_COUNT,	      "Port split count option expected."},
+	{DL_OPT_SB_POOL,	      "Pool index option expected."},
+	{DL_OPT_SB_SIZE,	      "Pool size option expected."},
+	{DL_OPT_SB_TYPE,	      "Pool type option expected."},
+	{DL_OPT_SB_THTYPE,	      "Pool threshold type option expected."},
+	{DL_OPT_SB_TH,		      "Threshold option expected."},
+	{DL_OPT_SB_TC,		      "TC index option expected."},
+	{DL_OPT_ESWITCH_MODE,	      "E-Switch mode option expected."},
+	{DL_OPT_ESWITCH_INLINE_MODE,  "E-Switch inline-mode option expected."},
+	{DL_OPT_DPIPE_TABLE_NAME,     "Dpipe table name expected."},
+	{DL_OPT_DPIPE_TABLE_COUNTERS, "Dpipe table counter state expected."},
+	{DL_OPT_ESWITCH_ENCAP_MODE,   "E-Switch encapsulation option expected."},
+	{DL_OPT_PARAM_NAME,	      "Parameter name expected."},
+	{DL_OPT_PARAM_VALUE,	      "Value to set expected."},
+	{DL_OPT_PARAM_CMODE,	      "Configuration mode expected."},
+	{DL_OPT_REGION_SNAPSHOT_ID,   "Region snapshot id expected."},
+	{DL_OPT_REGION_ADDRESS,	      "Region address value expected."},
+	{DL_OPT_REGION_LENGTH,	      "Region length value expected."},
+};
+
+static int dl_args_finding_required_validate(uint32_t o_required,
+					     uint32_t o_found)
+{
+	uint32_t o_flag;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dl_args_required); i++) {
+		o_flag = dl_args_required[i].o_flag;
+		if ((o_required & o_flag) && !(o_found & o_flag)) {
+			pr_err("%s\n", dl_args_required[i].err_msg);
+			return -EINVAL;
+		}
+	}
+	return 0;
+}
+
 static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 			 uint32_t o_optional)
 {
@@ -1198,114 +1242,7 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 		opts->present |= DL_OPT_SB;
 	}
 
-	if ((o_required & DL_OPT_PORT_TYPE) && !(o_found & DL_OPT_PORT_TYPE)) {
-		pr_err("Port type option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_PORT_COUNT) &&
-	    !(o_found & DL_OPT_PORT_COUNT)) {
-		pr_err("Port split count option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_POOL) && !(o_found & DL_OPT_SB_POOL)) {
-		pr_err("Pool index option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_SIZE) && !(o_found & DL_OPT_SB_SIZE)) {
-		pr_err("Pool size option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_TYPE) && !(o_found & DL_OPT_SB_TYPE)) {
-		pr_err("Pool type option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_THTYPE) && !(o_found & DL_OPT_SB_THTYPE)) {
-		pr_err("Pool threshold type option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_TH) && !(o_found & DL_OPT_SB_TH)) {
-		pr_err("Threshold option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_SB_TC) && !(o_found & DL_OPT_SB_TC)) {
-		pr_err("TC index option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_ESWITCH_MODE) &&
-	    !(o_found & DL_OPT_ESWITCH_MODE)) {
-		pr_err("E-Switch mode option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_ESWITCH_INLINE_MODE) &&
-	    !(o_found & DL_OPT_ESWITCH_INLINE_MODE)) {
-		pr_err("E-Switch inline-mode option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_DPIPE_TABLE_NAME) &&
-	    !(o_found & DL_OPT_DPIPE_TABLE_NAME)) {
-		pr_err("Dpipe table name expected\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_DPIPE_TABLE_COUNTERS) &&
-	    !(o_found & DL_OPT_DPIPE_TABLE_COUNTERS)) {
-		pr_err("Dpipe table counter state expected\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_ESWITCH_ENCAP_MODE) &&
-	    !(o_found & DL_OPT_ESWITCH_ENCAP_MODE)) {
-		pr_err("E-Switch encapsulation option expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_PARAM_NAME) &&
-	    !(o_found & DL_OPT_PARAM_NAME)) {
-		pr_err("Parameter name expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_PARAM_VALUE) &&
-	    !(o_found & DL_OPT_PARAM_VALUE)) {
-		pr_err("Value to set expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_PARAM_CMODE) &&
-	    !(o_found & DL_OPT_PARAM_CMODE)) {
-		pr_err("Configuration mode expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_REGION_SNAPSHOT_ID) &&
-	    !(o_found & DL_OPT_REGION_SNAPSHOT_ID)) {
-		pr_err("Region snapshot id expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_REGION_ADDRESS) &&
-	    !(o_found & DL_OPT_REGION_ADDRESS)) {
-		pr_err("Region address value expected.\n");
-		return -EINVAL;
-	}
-
-	if ((o_required & DL_OPT_REGION_LENGTH) &&
-	    !(o_found & DL_OPT_REGION_LENGTH)) {
-		pr_err("Region length value expected.\n");
-		return -EINVAL;
-	}
-
-	return 0;
+	return dl_args_finding_required_validate(o_required, o_found);
 }
 
 static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
-- 
2.14.1


^ permalink raw reply related

* Re: [PATCH] dt-bindings: Add bindings for mdio mux consumers
From: Andrew Lunn @ 2019-02-21 13:45 UTC (permalink / raw)
  To: Pankaj Bansal
  Cc: Leo Li, Peter Rosin, Florian Fainelli, Heiner Kallweit,
	Rob Herring, Mark Rutland, devicetree@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <20190221182349.4057-1-pankaj.bansal@nxp.com>

On Thu, Feb 21, 2019 at 12:59:19PM +0000, Pankaj Bansal wrote:
> When we use the bindings defined in Documentation/devicetree/bindings/mux
> to define mdio mux in producer and consumer terms, it results in two
> devices. one is mux producer and other is mux consumer.
> 
> Add the bindings needed for Mdio mux consumer devices.

Hi Pankaj

Please thread this binding patch with the code which implements the
binding. The two should be reviewed together.

Thanks
	Andrew

^ permalink raw reply

* Re: [bonding] 86838ad7bd: BUG:sleeping_function_called_from_invalid_context_at_kernel/locking/mutex.c
From: Kefeng Wang @ 2019-02-21 13:54 UTC (permalink / raw)
  To: kernel test robot
  Cc: netdev, Willem de Bruijn, David S . Miller, Jay Vosburgh,
	Veaceslav Falico, Eric Dumazet, weiyongjun1, lkp
In-Reply-To: <20190221133259.74etmwq7gadltqew@inn2.lkp.intel.com>

Please ignore this patch, this is unacceptable, thanks.

On 2019/2/21 21:32, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-8):
>
> commit: 86838ad7bde0d7741b272670d94d535580173d95 ("[RFC PATCH] bonding: use mutex lock in bond_get_stats()")
> url: https://github.com/0day-ci/linux/commits/Kefeng-Wang/bonding-use-mutex-lock-in-bond_get_stats/20190216-030058
>
>
> in testcase: rcutorture
> with following parameters:
>
> 	runtime: 300s
> 	test: cpuhotplug
> 	torture_type: srcu
>
> test-description: rcutorture is rcutorture kernel module load/unload test.
> test-url: https://www.kernel.org/doc/Documentation/RCU/torture.txt
>
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 1G
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> +-----------------------------------------------------------------------------+------------+------------+
> |                                                                             | f9bcc9f3ee | 86838ad7bd |
> +-----------------------------------------------------------------------------+------------+------------+
> | boot_successes                                                              | 50         | 24         |
> | boot_failures                                                               | 10         | 41         |
> | invoked_oom-killer:gfp_mask=0x                                              | 1          |            |
> | Mem-Info                                                                    | 1          |            |
> | Out_of_memory:Kill_process                                                  | 1          |            |
> | BUG:kernel_in_stage                                                         | 6          | 8          |
> | BUG:kernel_reboot-without-warning_in_early-boot_stage                       | 2          |            |
> | BUG:kernel_hang_in_test_stage                                               | 1          | 1          |
> | BUG:sleeping_function_called_from_invalid_context_at_kernel/locking/mutex.c | 0          | 32         |
> +-----------------------------------------------------------------------------+------------+------------+
>
>
>
> [   17.153431] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:908
> [   17.157556] in_atomic(): 0, irqs_disabled(): 0, pid: 599, name: dropbearkey
> [   17.159141] 2 locks held by dropbearkey/599:
> [   17.159779]  #0: (____ptrval____) (&p->lock){+.+.}, at: seq_read+0x41/0x3f0
> [   17.160802]  #1: (____ptrval____) (rcu_read_lock){....}, at: dev_seq_start+0x5/0x110
> [   17.161928] Preemption disabled at:
> [   17.161935] [<ffffffff818eafe0>] __mutex_lock+0x50/0x980
> [   17.163150] CPU: 1 PID: 599 Comm: dropbearkey Not tainted 5.0.0-rc5-00320-g86838ad #1
> [   17.164213] Call Trace:
> [   17.164571]  dump_stack+0x67/0x90
> [   17.165051]  ___might_sleep.cold.14+0xf0/0x106
> [   17.165675]  ? bond_get_stats+0x3b/0x260
> [   17.166232]  __mutex_lock+0x46/0x980
> [   17.166743]  ? kvm_clock_read+0x14/0x30
> [   17.167288]  ? validate_chain+0xb5/0xad0
> [   17.167936]  ? number+0x2fc/0x340
> [   17.168416]  ? bond_get_stats+0x3b/0x260
> [   17.168966]  bond_get_stats+0x3b/0x260
> [   17.169500]  ? seq_vprintf+0x30/0x50
> [   17.170007]  ? seq_printf+0x43/0x50
> [   17.170498]  ? validate_chain+0xb5/0xad0
> [   17.171136]  ? dev_seq_printf_stats+0x9a/0xb0
> [   17.171736]  dev_get_stats+0x55/0xb0
> [   17.172221]  dev_seq_printf_stats+0x1f/0xb0
> [   17.172817]  dev_seq_show+0x10/0x30
> [   17.173289]  seq_read+0x2cd/0x3f0
> [   17.173777]  proc_reg_read+0x36/0x60
> [   17.174288]  __vfs_read+0x23/0x160
> [   17.174779]  ? _copy_to_user+0x5e/0x70
> [   17.175312]  ? entry_INT80_compat+0x7c/0x90
> [   17.175903]  vfs_read+0xa6/0x140
> [   17.176366]  ksys_read+0x42/0xa0
> [   17.176834]  do_int80_syscall_32+0x4f/0x150
> [   17.177418]  entry_INT80_compat+0x84/0x90
> LKP: HOSTNAME vm-snb-openwrt-ia32-524, MAC , kernel 5.0.0-rc5-00320-g86838ad 1, serial console /dev/ttyS0
> [   18.404764] Kernel tests: Boot OK!
> [   18.404764] LKP: waiting for network...
> [   19.256982] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
> [   19.271235] _warn_unseeded_randomness: 1329 callbacks suppressed
> [   19.271244] random: get_random_u64 called from arch_pick_mmap_layout+0x65/0x170 with crng_init=0
> [   19.273507] random: get_random_u64 called from arch_pick_mmap_layout+0x10e/0x170 with crng_init=0
> [   19.274790] random: get_random_u64 called from load_elf_binary+0x87d/0x1120 with crng_init=0
> [   20.407748] /lkp/lkp/src/bin/run-lkp
> [   20.407748] RESULT_ROOT=/result/rcutorture/300s-cpuhotplug-srcu/vm-snb-openwrt-ia32/openwrt-i386-2016-03-16.cgz/x86_64-randconfig-s4-02200400/gcc-8/86838ad7bde0d7741b272670d94d535580173d95/3
> [   20.407748] job=/lkp/jobs/scheduled/vm-snb-openwrt-ia32-524/rcutorture-300s-cpuhotplug-srcu-openwrt-i386-2016-03-16.cgz-20190220-89648-gjivxf-3.yaml
> [   20.407748] run-job /lkp/jobs/scheduled/vm-snb-openwrt-ia32-524/rcutorture-300s-cpuhotplug-srcu-openwrt-i386-2016-03-16.cgz-20190220-89648-gjivxf-3.yaml
> [   20.407748] /bin/busybox wget -q http://inn:80/~lkp/cgi-bin/lkp-jobfile-append-var?job_file=/lkp/jobs/scheduled/vm-snb-openwrt-ia32-524/rcutorture-300s-cpuhotplug-srcu-openwrt-i386-2016-03-16.cgz-20190220-89648-gjivxf-3.yaml&job_state=running -O /dev/null
> [   20.407748] target ucode: 
> [   20.416008] wget: can't connect to remote host (192.168.1.1): Network is unreachable
> [   20.416008] /lkp/lkp/src/monitors/heartbeat: line 9: vmstat: not found
> [   20.416008] /lkp/lkp/src/monitors/heartbeat: exec: line 23: vmstat: not found
> [   20.678602] _warn_unseeded_randomness: 431 callbacks suppressed
> [   20.678610] random: get_random_u64 called from arch_pick_mmap_layout+0x65/0x170 with crng_init=0
> [   20.681816] random: get_random_u64 called from arch_pick_mmap_layout+0x10e/0x170 with crng_init=0
> [   20.683622] random: get_random_u64 called from load_elf_binary+0x87d/0x1120 with crng_init=0
> [   21.418765] BusyBox v1.23.2 (2016-01-02 14:31:17 CET) multi-call binary.
> [   21.418765] 
> [   21.418765] Usage: time [-v] PROG ARGS
> [   21.418765] 
> [   21.418765] Run PROG, display resource usage when it exits
> [   21.418765] 
> [   21.418765] 	-v	Verbose
> [   21.418765] 
> [   21.418765] failed to load rcutorture module, try to enable CONFIG_RCU_TORTURE_TEST and build rcutorture.ko
> [   21.418765] wget: can't connect to remote host (192.168.1.1): Network is unreachable
> [   21.423344] 2019-02-20 06:29:25 modprobe rcutorture onoff_interval=3 onoff_holdoff=30 torture_type=srcu
> [   21.423344] /bin/busybox wget -q http://inn:80/~lkp/cgi-bin/lkp-jobfile-append-var?job_file=/lkp/jobs/scheduled/vm-snb-openwrt-ia32-524/rcutorture-300s-cpuhotplug-srcu-openwrt-i386-2016-03-16.cgz-20190220-89648-gjivxf-3.yaml&job_state=post_run -O /dev/null
> [   21.818029] _warn_unseeded_randomness: 210 callbacks suppressed
> [   21.818037] random: get_random_u64 called from arch_pick_mmap_layout+0x65/0x170 with crng_init=0
> [   21.821714] random: get_random_u64 called from arch_pick_mmap_layout+0x10e/0x170 with crng_init=0
> [   21.823918] random: get_random_u64 called from load_elf_binary+0x87d/0x1120 with crng_init=0
> [   22.190264] e1000: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
> [   22.444763] kill 804 cat /proc/kmsg 
> [   22.444763] wait for background processes: 812 810 oom-killer meminfo
>
> Elapsed time: 20
>
> qemu-img create -f qcow2 disk-vm-snb-openwrt-ia32-524-0 256G
> qemu-img create -f qcow2 disk-vm-snb-openwrt-ia32-524-1 256G
>
> kvm=(
> 	qemu-system-x86_64
> 	-enable-kvm
> 	-cpu SandyBridge
> 	-kernel $kernel
> 	-initrd initrd-vm-snb-openwrt-ia32-524
> 	-m 1024
> 	-smp 2
> 	-device e1000,netdev=net0
> 	-netdev user,id=net0
> 	-boot order=nc
> 	-no-reboot
> 	-watchdog i6300esb
> 	-watchdog-action debug
> 	-rtc base=localtime
> 	-drive file=disk-vm-snb-openwrt-ia32-524-0,media=disk,if=virtio
> 	-drive file=disk-vm-snb-openwrt-ia32-524-1,media=disk,if=virtio
> 	-serial stdio
> 	-display none
> 	-monitor null
> )
>
>
> To reproduce:
>
>         git clone https://github.com/intel/lkp-tests.git
>         cd lkp-tests
>         bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
>
>
>
> Thanks,
> Rong Chen


^ permalink raw reply

* Re: Handling an Extra Signal at PHY Reset
From: Andrew Lunn @ 2019-02-21 14:04 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: Florian Fainelli, Heiner Kallweit, netdev, Thomas Petazzoni,
	Mylène Josserand
In-Reply-To: <efd27592a4f4e76fb5016d1ba1314d4e29003db3.camel@bootlin.com>

> I can also confirm that it does not prevent contacting the PHY on the
> MDIO bus, contrary to what I have stated previously.

O.K, so wrong voltage does not matter, you can still probe the PHY.

Ignore the switch. Use a pin hog to set the GPIO to the disabled
state. And set the voltage using the PHY register during probe().

    Andrew
 

^ permalink raw reply

* [PATCH net-next 1/1] qed: Read device port count from the shmem
From: Sudarsana Reddy Kalluru @ 2019-02-21 14:03 UTC (permalink / raw)
  To: davem; +Cc: netdev, aelior, mkalderon

Read port count from the shared memory instead of driver deriving this
value. This change simplifies the driver implementation and also avoids
any dependencies for finding the port-count.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed.h     |  6 +-
 drivers/net/ethernet/qlogic/qed/qed_dev.c | 95 ++++++++++++-------------------
 drivers/net/ethernet/qlogic/qed/qed_l2.c  |  4 +-
 drivers/net/ethernet/qlogic/qed/qed_mcp.h |  4 --
 drivers/net/ethernet/qlogic/qed/qed_ptp.c |  2 +-
 5 files changed, 44 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h
index 2d21c94..43a57ec 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -753,6 +753,7 @@ struct qed_dev {
 #define CHIP_BOND_ID_SHIFT              0
 
 	u8				num_engines;
+	u8				num_ports;
 	u8				num_ports_in_engine;
 	u8				num_funcs_in_port;
 
@@ -892,7 +893,6 @@ void qed_configure_vp_wfq_on_link_change(struct qed_dev *cdev,
 
 void qed_clean_wfq_db(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
 int qed_device_num_engines(struct qed_dev *cdev);
-int qed_device_get_port_id(struct qed_dev *cdev);
 void qed_set_fw_mac_addr(__le16 *fw_msb,
 			 __le16 *fw_mid, __le16 *fw_lsb, u8 *mac);
 
@@ -939,6 +939,10 @@ void qed_db_recovery_execute(struct qed_hwfn *p_hwfn,
 	writel((u32)val, (void __iomem *)((u8 __iomem *)\
 					  (cdev->doorbells) + (db_addr)))
 
+#define MFW_PORT(_p_hwfn)       ((_p_hwfn)->abs_pf_id %			  \
+				  qed_device_num_ports((_p_hwfn)->cdev))
+int qed_device_num_ports(struct qed_dev *cdev);
+
 /* Prototypes */
 int qed_fill_dev_info(struct qed_dev *cdev,
 		      struct qed_dev_info *dev_info);
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index e2cbd77..9df8c4b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -3269,55 +3269,43 @@ static void qed_get_num_funcs(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 		   p_hwfn->enabled_func_idx, p_hwfn->num_funcs_on_engine);
 }
 
-static void qed_hw_info_port_num_bb(struct qed_hwfn *p_hwfn,
-				    struct qed_ptt *p_ptt)
-{
-	u32 port_mode;
-
-	port_mode = qed_rd(p_hwfn, p_ptt, CNIG_REG_NW_PORT_MODE_BB);
-
-	if (port_mode < 3) {
-		p_hwfn->cdev->num_ports_in_engine = 1;
-	} else if (port_mode <= 5) {
-		p_hwfn->cdev->num_ports_in_engine = 2;
-	} else {
-		DP_NOTICE(p_hwfn, "PORT MODE: %d not supported\n",
-			  p_hwfn->cdev->num_ports_in_engine);
-
-		/* Default num_ports_in_engine to something */
-		p_hwfn->cdev->num_ports_in_engine = 1;
-	}
-}
-
-static void qed_hw_info_port_num_ah(struct qed_hwfn *p_hwfn,
-				    struct qed_ptt *p_ptt)
+static void qed_hw_info_port_num(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 {
-	u32 port;
-	int i;
-
-	p_hwfn->cdev->num_ports_in_engine = 0;
+	u32 addr, global_offsize, global_addr, port_mode;
+	struct qed_dev *cdev = p_hwfn->cdev;
 
-	for (i = 0; i < MAX_NUM_PORTS_K2; i++) {
-		port = qed_rd(p_hwfn, p_ptt,
-			      CNIG_REG_NIG_PORT0_CONF_K2 + (i * 4));
-		if (port & 1)
-			p_hwfn->cdev->num_ports_in_engine++;
+	/* In CMT there is always only one port */
+	if (cdev->num_hwfns > 1) {
+		cdev->num_ports_in_engine = 1;
+		cdev->num_ports = 1;
+		return;
 	}
 
-	if (!p_hwfn->cdev->num_ports_in_engine) {
-		DP_NOTICE(p_hwfn, "All NIG ports are inactive\n");
-
-		/* Default num_ports_in_engine to something */
-		p_hwfn->cdev->num_ports_in_engine = 1;
+	/* Determine the number of ports per engine */
+	port_mode = qed_rd(p_hwfn, p_ptt, MISC_REG_PORT_MODE);
+	switch (port_mode) {
+	case 0x0:
+		cdev->num_ports_in_engine = 1;
+		break;
+	case 0x1:
+		cdev->num_ports_in_engine = 2;
+		break;
+	case 0x2:
+		cdev->num_ports_in_engine = 4;
+		break;
+	default:
+		DP_NOTICE(p_hwfn, "Unknown port mode 0x%08x\n", port_mode);
+		cdev->num_ports_in_engine = 1;	/* Default to something */
+		break;
 	}
-}
 
-static void qed_hw_info_port_num(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
-{
-	if (QED_IS_BB(p_hwfn->cdev))
-		qed_hw_info_port_num_bb(p_hwfn, p_ptt);
-	else
-		qed_hw_info_port_num_ah(p_hwfn, p_ptt);
+	/* Get the total number of ports of the device */
+	addr = SECTION_OFFSIZE_ADDR(p_hwfn->mcp_info->public_base,
+				    PUBLIC_GLOBAL);
+	global_offsize = qed_rd(p_hwfn, p_ptt, addr);
+	global_addr = SECTION_ADDR(global_offsize, 0);
+	addr = global_addr + offsetof(struct public_global, max_ports);
+	cdev->num_ports = (u8)qed_rd(p_hwfn, p_ptt, addr);
 }
 
 static void qed_get_eee_caps(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
@@ -3355,7 +3343,8 @@ static void qed_get_eee_caps(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 			return rc;
 	}
 
-	qed_hw_info_port_num(p_hwfn, p_ptt);
+	if (IS_LEAD_HWFN(p_hwfn))
+		qed_hw_info_port_num(p_hwfn, p_ptt);
 
 	qed_mcp_get_capabilities(p_hwfn, p_ptt);
 
@@ -4760,23 +4749,9 @@ void qed_clean_wfq_db(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 	       sizeof(*p_hwfn->qm_info.wfq_data) * p_hwfn->qm_info.num_vports);
 }
 
-int qed_device_num_engines(struct qed_dev *cdev)
-{
-	return QED_IS_BB(cdev) ? 2 : 1;
-}
-
-static int qed_device_num_ports(struct qed_dev *cdev)
-{
-	/* in CMT always only one port */
-	if (cdev->num_hwfns > 1)
-		return 1;
-
-	return cdev->num_ports_in_engine * qed_device_num_engines(cdev);
-}
-
-int qed_device_get_port_id(struct qed_dev *cdev)
+int qed_device_num_ports(struct qed_dev *cdev)
 {
-	return (QED_LEADING_HWFN(cdev)->abs_pf_id) % qed_device_num_ports(cdev);
+	return cdev->num_ports;
 }
 
 void qed_set_fw_mac_addr(__le16 *fw_msb,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index 58be1c4..5764172 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -1898,6 +1898,7 @@ static void _qed_get_vport_stats(struct qed_dev *cdev,
 		struct qed_hwfn *p_hwfn = &cdev->hwfns[i];
 		struct qed_ptt *p_ptt = IS_PF(cdev) ? qed_ptt_acquire(p_hwfn)
 						    :  NULL;
+		bool b_get_port_stats;
 
 		if (IS_PF(cdev)) {
 			/* The main vport index is relative first */
@@ -1912,8 +1913,9 @@ static void _qed_get_vport_stats(struct qed_dev *cdev,
 			continue;
 		}
 
+		b_get_port_stats = IS_PF(cdev) && IS_LEAD_HWFN(p_hwfn);
 		__qed_get_vport_stats(p_hwfn, p_ptt, stats, fw_vport,
-				      IS_PF(cdev) ? true : false);
+				      b_get_port_stats);
 
 out:
 		if (IS_PF(cdev) && p_ptt)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 2799e67..261c1a3 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -691,10 +691,6 @@ int qed_mcp_bist_nvm_get_image_att(struct qed_hwfn *p_hwfn,
 					    rel_pfid)
 #define MCP_PF_ID(p_hwfn) MCP_PF_ID_BY_REL(p_hwfn, (p_hwfn)->rel_pf_id)
 
-#define MFW_PORT(_p_hwfn)       ((_p_hwfn)->abs_pf_id %			  \
-				 ((_p_hwfn)->cdev->num_ports_in_engine * \
-				  qed_device_num_engines((_p_hwfn)->cdev)))
-
 struct qed_mcp_info {
 	/* List for mailbox commands which were sent and wait for a response */
 	struct list_head			cmd_list;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
index 5a90d69..1302b30 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
@@ -47,7 +47,7 @@
 
 static enum qed_resc_lock qed_ptcdev_to_resc(struct qed_hwfn *p_hwfn)
 {
-	switch (qed_device_get_port_id(p_hwfn->cdev)) {
+	switch (MFW_PORT(p_hwfn)) {
 	case 0:
 		return QED_RESC_LOCK_PTP_PORT0;
 	case 1:
-- 
1.8.3.1


^ permalink raw reply related

* RE: [PATCH] dt-bindings: Add bindings for mdio mux consumers
From: Pankaj Bansal @ 2019-02-21 14:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Leo Li, Peter Rosin, Florian Fainelli, Heiner Kallweit,
	Rob Herring, Mark Rutland, devicetree@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <20190221134504.GE5653@lunn.ch>



> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, 21 February, 2019 07:15 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: Leo Li <leoyang.li@nxp.com>; Peter Rosin <peda@axentia.se>; Florian
> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> devicetree@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH] dt-bindings: Add bindings for mdio mux consumers
> 
> On Thu, Feb 21, 2019 at 12:59:19PM +0000, Pankaj Bansal wrote:
> > When we use the bindings defined in
> > Documentation/devicetree/bindings/mux
> > to define mdio mux in producer and consumer terms, it results in two
> > devices. one is mux producer and other is mux consumer.
> >
> > Add the bindings needed for Mdio mux consumer devices.
> 
> Hi Pankaj
> 
> Please thread this binding patch with the code which implements the binding.
> The two should be reviewed together.

OK. If you have any comments about this patch, then please let me know.
I will incorporate these when I send both patches together.

> 
> Thanks
> 	Andrew

^ permalink raw reply

* RE: [PATCH] drivers: net: phy: mdio-mux: Add support for Generic Mux controls
From: Pankaj Bansal @ 2019-02-21 14:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Leo Li, Peter Rosin, Florian Fainelli, Heiner Kallweit,
	netdev@vger.kernel.org
In-Reply-To: <20190221134213.GD5653@lunn.ch>



> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, 21 February, 2019 07:12 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: Leo Li <leoyang.li@nxp.com>; Peter Rosin <peda@axentia.se>; Florian
> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>;
> netdev@vger.kernel.org
> Subject: Re: [PATCH] drivers: net: phy: mdio-mux: Add support for Generic Mux
> controls
> 
> On Thu, Feb 21, 2019 at 01:00:44PM +0000, Pankaj Bansal wrote:
> > Add support for Generic Mux controls, when Mdio mux node is a consumer
> > of mux produced by some other device.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > ---
> >  drivers/net/phy/Kconfig    |  1 +
> >  drivers/net/phy/mdio-mux.c | 81 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 82 insertions(+)
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index
> > 3d187cd50eb0..e0e6b2b33d6d 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -47,6 +47,7 @@ config MDIO_BITBANG
> >  config MDIO_BUS_MUX
> >  	tristate
> >  	depends on OF_MDIO
> > +	select MULTIPLEXER
> >  	help
> >  	  This module provides a driver framework for MDIO bus
> >  	  multiplexers which connect one of several child MDIO busses
> 
> Hi Pankaj
> 
> Please add a MDIO_BUS_MUX_MULTIPLEXER and put all the code into mdio-
> mux-multiplexer.c

Isn't MUX short for MULTIPLEXER ? wouldn't this be more confusing ?
I have kept the mux APIs code unchanged, so as to not disturb the existing drivers that use these APIs.

> 
> > +static const struct of_device_id mdio_mux_match[] = {
> > +	{ .compatible = "mdio-mux", },
> 
> mdio-mux-multiplexer

Again, isn't mux short for multiplexer? wouldn't this be more confusing ?

> 
> Thanks
> 	Andrew


^ permalink raw reply

* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Jiri Pirko @ 2019-02-21 14:08 UTC (permalink / raw)
  To: Aya Levin; +Cc: David Ahern, netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha
In-Reply-To: <1550756567-18227-5-git-send-email-ayal@mellanox.com>

Thu, Feb 21, 2019 at 02:42:40PM CET, ayal@mellanox.com wrote:
>Add a new helper functions which outputs only values (without name
>label) for different types: boolean, uint, uint64, string and binary.
>In addition add a helper function which prints only the name label.
>
>Signed-off-by: Aya Levin <ayal@mellanox.com>
>Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>---
> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
>diff --git a/devlink/devlink.c b/devlink/devlink.c
>index b073ae020d52..5d69c4f24f29 100644
>--- a/devlink/devlink.c
>+++ b/devlink/devlink.c
>@@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
> 		pr_out("\n");
> }
> 
>-

Please avoid changes like this.


[...]

^ permalink raw reply

* Re: stmmac / meson8b-dwmac
From: Jerome Brunet @ 2019-02-21 14:21 UTC (permalink / raw)
  To: Simon Huelck, Jose Abreu, Martin Blumenstingl
  Cc: linux-amlogic, netdev, alexandre.torgue, Emiliano Ingrassia,
	Gpeppe.cavallaro
In-Reply-To: <244d7c74-e0ca-a9c7-f4b0-3de7bec4024b@gmx.de>

On Tue, 2019-02-19 at 20:41 +0100, Simon Huelck wrote:
> Am 19.02.2019 um 09:47 schrieb Jose Abreu:
> > Hi Simon,
> > 
> > On 2/18/2019 6:05 PM, Simon Huelck wrote:
> > > disabling EEE doesnt help ( did it via the entry in the .dtb / .dts ),
> > > the results are the same. I can confirm the LPI counters are zero or one
> > > only after the test.....
> > It's interesting to see that you have a lot of RX packets but few
> > RX interrupts. This can either be due to mis-configuration of
> > interrupt flags or RX Watchdog.
> > 
> > 1. For interrupt flags you should be using LEVEL triggered IRQ.
> > 2. For RX Watchdog you can try disabling it by adding the
> > following in your platform wrapper driver (which will be
> > "dwmac-meson8b.c", at probe):
> > 	"plat_data->force_sf_dma_mode = 1;" and
> > 	"plat_data->riwt_off = 1;"
> > 
> > Thanks,
> > Jose Miguel Abreu
> 
> Hi,
> 
> 
> are you aware that odroid c2 dts is using level irq ?

The odroid-c2 (meson-gxl) is using level triggered irq, indeed.
For a reason that still eludes me, The odroid-c1 (meson8b) isn't.

This is confusing since both boards have been mentionned in this thread.

> 
> at which numbers do you estimage that RX irqs are low in numbers ?
> 
> 
> regards,
> 
> Simon
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic



^ permalink raw reply

* Re: [PATCH v2 iproute2-next 05/11] devlink: Add devlink health show command
From: Jiri Pirko @ 2019-02-21 14:25 UTC (permalink / raw)
  To: Aya Levin; +Cc: David Ahern, netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha
In-Reply-To: <1550756567-18227-6-git-send-email-ayal@mellanox.com>

Thu, Feb 21, 2019 at 02:42:41PM CET, ayal@mellanox.com wrote:
>Add devlink health show command which displays status and configuration
>info on a specific reporter on a device or dump the info on all
>reporters on all devices. The patch also contains helper functions to

What "patch"? Please format the description as you would command the
codebase what to do.


>display status and dump's time stamp.

[...]


>+static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
>+{
>+	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>+	enum devlink_health_reporter_state state;
>+	const struct nlattr *attr;
>+	uint64_t time_ms;
>+	int err;
>+
>+	state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;

Pointless assignment.


>+
>+	err = mnl_attr_parse_nested(tb_health[DEVLINK_ATTR_HEALTH_REPORTER],
>+				    attr_cb, tb);
>+	if (err != MNL_CB_OK)
>+		return;
>+
>+	if (!tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] ||
>+	    !tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE])
>+		return;
>+
>+	pr_out_handle_start_arr(dl, tb_health);
>+
>+	pr_out_str(dl, "name",
>+		   mnl_attr_get_str(tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
>+	if (!dl->json_output) {
>+		__pr_out_newline();
>+		__pr_out_indent_inc();
>+	}
>+	state = mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE]);
>+	pr_out_str(dl, "state", health_state_name(state));
>+	pr_out_u64(dl, "error",
>+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
>+	pr_out_u64(dl, "recover",
>+		   mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
>+	if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
>+		char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
>+		char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];

Newline here please.


>+		attr = tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
>+		time_ms = mnl_attr_get_u64(attr);
>+		format_logtime(time_ms, dump_date, dump_time);
>+
>+		pr_out_str(dl, "last_dump_date", dump_date);
>+		pr_out_str(dl, "last_dump_time", dump_time);
>+	}

[...]


>@@ -5776,7 +5958,11 @@ static int dl_cmd(struct dl *dl, int argc, char **argv)
> 	} else if (dl_argv_match(dl, "region")) {
> 		dl_arg_inc(dl);
> 		return cmd_region(dl);
>+	} else if (dl_argv_match(dl, "health")) {
>+		dl_arg_inc(dl);
>+		return cmd_health(dl);
> 	}
>+

Please avoid newlines like this. Unrelated to the patch.


> 	pr_err("Object \"%s\" not found\n", dl_argv(dl));
> 	return -ENOENT;
> }
>-- 
>2.14.1
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox