* [PATCH v3 iproute2-next 03/11] devlink: Fix boolean JSON print
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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 b51380298f8a..e5aca7b955cc 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 v3 iproute2-next 07/11] devlink: Add devlink health diagnose command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 187 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index ff1d2fcf97bb..ce3b427a3670 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"
@@ -5761,6 +5762,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;
@@ -5918,6 +6101,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)
@@ -5932,6 +6116,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));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 05/11] devlink: Add devlink health show command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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. Add 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>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 184 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 60f20b636500..a53db32952e8 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,
@@ -5736,11 +5761,166 @@ 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;
+
+ 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");
}
@@ -5773,6 +5953,9 @@ 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 v3 iproute2-next 00/11] Add support for devlink health
From: Aya Levin @ 2019-02-24 12:46 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.
v3:
-Add blank row that was wrongly added patch #4
-Rephrase commit message patch #5
-Some refactoring in patch #5
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 | 725 ++++++++++++++++++++++++++++++++++++++--------
man/man8/devlink-health.8 | 197 +++++++++++++
man/man8/devlink.8 | 7 +-
3 files changed, 815 insertions(+), 114 deletions(-)
create mode 100644 man/man8/devlink-health.8
--
2.14.1
^ permalink raw reply
* [PATCH v3 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index e5aca7b955cc..60f20b636500 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1636,6 +1636,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 v3 iproute2-next 09/11] devlink: Add devlink health dump clear command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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 328d4e092688..a10c5957f1cc 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5762,6 +5762,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;
@@ -6108,6 +6125,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)
@@ -6130,6 +6148,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);
}
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 08/11] devlink: Add devlink health dump show command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-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 ce3b427a3670..328d4e092688 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5939,6 +5939,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);
@@ -6102,6 +6107,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)
@@ -6119,6 +6125,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));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
For quite some time, I wondered why the PCI mwifiex device built in my
Chromebook was unable to use the good old legacy interrupts. But as MSIs
were working fine, I never really bothered investigating. I finally had a
look, and the result isn't very pretty.
On this machine (rk3399-based kevin), the wake-up interrupt is described as
such:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wakeup-source;
};
};
Note how the interrupt is part of the properties directly attached to the
PCI node. And yet, this interrupt has nothing to do with a PCI legacy
interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
altogether (Yay for the broken design!). This is in total violation of the
IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
specifiers describe the PCI device interrupts, and must obey the
INT-{A,B,C,D} mapping. Oops!
The net effect of the above is that Linux tries to do something vaguely
sensible, and uses the same interrupt for both the wake-up widget and the
PCI device. This doesn't work for two reasons: (1) the wake-up widget grabs
the interrupt in exclusive mode, and (2) the PCI interrupt is still routed
to the RC, leading to a screaming interrupt. This simply cannot work.
To sort out this mess, we need to lift the confusion between the two
interrupts. This is done by extending the DT binding to allow the wake-up
interrupt to be described in a 'wake-up' subnode, sidestepping the issue
completely. On my Chromebook, it now looks like this:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wake-up {
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
wakeup-source;
};
};
};
The driver is then updated to look for this subnode first, and fallback to
the original, broken behaviour (spitting out a warning in the offending
configuration).
For good measure, there are two additional patches:
- The wake-up interrupt requesting is horribly racy, and could lead to
unpredictable behaviours. Let's fix that properly.
- A final patch implementing the above transformation for the whole
RK3399-based Chromebook range, which all use the same broken
configuration.
With all that, I finally have PCI legacy interrupts working with the mwifiex
driver on my Chromebook.
[1] http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
Marc Zyngier (4):
dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a
separate node
mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling
it too late
arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own
subnode
.../bindings/net/wireless/marvell-8xxx.txt | 23 ++++++++++++++++++-
.../dts/rockchip/rk3399-gru-chromebook.dtsi | 8 ++++---
drivers/net/wireless/marvell/mwifiex/main.c | 13 +++++++++--
3 files changed, 38 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 4/4] arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own subnode
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
In order to get PCIe legacy interrupts working on gru-based Chromebooks,
let's move the wake-up interrupt out of the way and into its own
subnode. This ensures that this interrupt specifier will not be
mistaken as a PCI interrupt.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
index c400be64170e..61fff688770c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
@@ -310,11 +310,13 @@ ap_i2c_tp: &i2c5 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
- interrupt-parent = <&gpio0>;
- interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
- wakeup-source;
+ wake-up {
+ interrupt-parent = <&gpio0>;
+ interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ };
};
};
--
2.20.1
^ permalink raw reply related
* [PATCH 3/4] mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling it too late
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
The mwifiex driver makes unsafe assumptions about the state of the
wake-up interrupt. It requests it and only then disable it. Of
course, the interrupt may be screaming for whatever reason at that
time, and the handler will then be called without the interrupt
having been registered with the PM/wakeup subsystem. Oops.
The right way to handle this kind of situation is to flag the
interrupt with IRQ_NOAUTOEN before requesting it. It will then
stay disabled until someone (the wake-up subsystem) enables it.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 2105c2b7c627..82cf35e50579 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1610,6 +1610,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
"wake-up interrupt outside 'wake-up' subnode of %pOF\n",
adapter->dt_node);
+ irq_set_status_flags(adapter->irq_wakeup, IRQ_NOAUTOEN);
ret = devm_request_irq(dev, adapter->irq_wakeup,
mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
"wifi_wake", adapter);
@@ -1619,7 +1620,6 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
goto err_exit;
}
- disable_irq(adapter->irq_wakeup);
if (device_init_wakeup(dev, true)) {
dev_err(dev, "fail to init wakeup for mwifiex\n");
goto err_exit;
--
2.20.1
^ permalink raw reply related
* [PATCH 2/4] mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
Encoding the wake-up interrupt as part of the PCI DT node is completely
broken, as it violates the most basic rules of PCI description in OF:
the interrupts described in such node are supposed to apply to the
PCI device, and not to some non-PCI stuff on the side.
In such a configuration, both the PCI device and the wake-up widget
end-up trying to share an interrupt. Of course, this doesn't work:
The PCI device can only generate interrupts through the root port,
while the wake-up widget uses sideband signaling that bypasses PCI
altogether. Clearly, this was never tested.
So let's first try and obtain the wake-up interrupt from a 'wake-up'
subnode, and fallback to the main DT node otherwise. This ensures
that old DTs will carry on working as bad as before (with the added
warning to let the user know about the situation), and new DT will
enjoy legacy interrupts in case MSIs are unavailable.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 20cee5c397fb..2105c2b7c627 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1590,17 +1590,26 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
{
int ret;
struct device *dev = adapter->dev;
+ struct device_node *wup_node;
if (!dev->of_node)
goto err_exit;
adapter->dt_node = dev->of_node;
- adapter->irq_wakeup = irq_of_parse_and_map(adapter->dt_node, 0);
+ wup_node = of_get_child_by_name(adapter->dt_node, "wake-up");
+ if (!wup_node)
+ wup_node = adapter->dt_node;
+ adapter->irq_wakeup = irq_of_parse_and_map(wup_node, 0);
if (!adapter->irq_wakeup) {
dev_dbg(dev, "fail to parse irq_wakeup from device tree\n");
goto err_exit;
}
+ if (dev_is_pci(dev) && adapter->dt_node == wup_node)
+ dev_warn(dev,
+ "wake-up interrupt outside 'wake-up' subnode of %pOF\n",
+ adapter->dt_node);
+
ret = devm_request_irq(dev, adapter->irq_wakeup,
mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
"wifi_wake", adapter);
--
2.20.1
^ permalink raw reply related
* [PATCH 1/4] dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a separate node
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
The DT binding for the PCI version of the Marvell 8xxx wifi devices
is pretty broken (the fact that a PCI device requires a DT binding
is quite telling on its own).
The binding allows the description of a wake-up interrupt as a sideband
signal, allowing the wifi device to wake-up the system. So far, so good.
Until you realise that placing an interrupt in the PCI device DT node
has a specific meaning, and applies to the actual PCI function, and
not some random sideband stuff. This is of course in total violation
of the OF specification (IEEE Std 1275-1994), but hey, who cares.
Let's thus change the binding to be somewhat compatible with the spec,
by placing the wake-up interrupt in a subnode called "wake-up". This
still is an optional property, but a recommended one for PCI devices.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
.../bindings/net/wireless/marvell-8xxx.txt | 23 ++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
index 9bf9bbac16e2..f9340ca37047 100644
--- a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
+++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
@@ -33,10 +33,16 @@ Optional properties:
this interrupt number. during system suspend, the irq will be enabled
so that the wifi chip can wakeup host platform under certain condition.
during system resume, the irq will be disabled to make sure
- unnecessary interrupt is not received.
+ unnecessary interrupt is not received. For PCI devices, it
+ is recommended that this property is placed in a "wake-up"
+ sub-node in order to limit the ambiguity with PCI legacy
+ interrupts.
- vmmc-supply: a phandle of a regulator, supplying VCC to the card
- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc-pwrseq-*"
for documentation of MMC power sequence bindings.
+ - wake-up: a subnode containing the interrupt specifier for a wake-up
+ interrupt. This is the recommended configuration for PCI
+ devices.
Example:
@@ -66,3 +72,18 @@ so that firmware can wakeup host using this device side pin.
marvell,wakeup-pin = <3>;
};
};
+
+&pci_rootport {
+ mvl_wifi: wifi@0,0 {
+ compatible = "pci1b4b,2b42";
+ reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
+ 0x83010000 0x0 0x00100000 0x0 0x00100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_host_wake_l>;
+ wake-up {
+ interrupt-parent = <&gpio0>;
+ interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ };
+ };
+};
--
2.20.1
^ permalink raw reply related
* Re: [RFC v1 12/19] RDMA/irdma: Implement device supported verb APIs
From: Gal Pressman @ 2019-02-24 14:35 UTC (permalink / raw)
To: Shiraz Saleem, dledford, jgg, davem
Cc: linux-rdma, netdev, mustafa.ismail, jeffrey.t.kirsher,
Yossi Leybovich
In-Reply-To: <20190215171107.6464-13-shiraz.saleem@intel.com>
On 15-Feb-19 19:10, Shiraz Saleem wrote:
> /**
> * irdma_dealloc_ucontext - deallocate the user context data structure
> * @context: user context created during alloc
> */
> static int irdma_dealloc_ucontext(struct ib_ucontext *context)
> {
> struct irdma_ucontext *ucontext = to_ucontext(context);
> unsigned long flags;
>
> spin_lock_irqsave(&ucontext->cq_reg_mem_list_lock, flags);
> if (!list_empty(&ucontext->cq_reg_mem_list)) {
> spin_unlock_irqrestore(&ucontext->cq_reg_mem_list_lock, flags);
> return -EBUSY;
> }
> spin_unlock_irqrestore(&ucontext->cq_reg_mem_list_lock, flags);
>
> spin_lock_irqsave(&ucontext->qp_reg_mem_list_lock, flags);
> if (!list_empty(&ucontext->qp_reg_mem_list)) {
> spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);
> return -EBUSY;
Drivers are not permitted to fail dealloc_ucontext.
> }
> spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);
> kfree(ucontext);
>
> return 0;
> }
> +/**> + * irdma_disassociate_ucontext - Disassociate user context> + * @context: ib user context> + */> +static void irdma_disassociate_ucontext(struct ib_ucontext *context)
> +{
> +}
What's the motivation for a nop callback (over not implementing the
function)?
> +/**
> + * irdma_alloc_pd - allocate protection domain
> + * @pd: PD pointer
> + * @context: user context created during alloc
> + * @udata: user data
> + */
> +static int irdma_alloc_pd(struct ib_pd *pd,
> + struct ib_ucontext *context,
> + struct ib_udata *udata)
> +{
> + struct irdma_pd *iwpd = to_iwpd(pd);
> + struct irdma_device *iwdev = to_iwdev(pd->device);
> + struct irdma_sc_dev *dev = &iwdev->rf->sc_dev;
> + struct irdma_pci_f *rf = iwdev->rf;
> + struct irdma_alloc_pd_resp uresp = {};
> + struct irdma_sc_pd *sc_pd;
> + struct irdma_ucontext *ucontext;
> + u32 pd_id = 0;
> + int err;
> +
> + if (iwdev->closing)
> + return -ENODEV;
> +
> + err = irdma_alloc_rsrc(rf, rf->allocated_pds, rf->max_pd, &pd_id,
> + &rf->next_pd);
> + if (err)
> + return err;
> +
> + sc_pd = &iwpd->sc_pd;
> + if (context) {
I think this should be 'if (udata)', this applies to many other places in this driver.
> + ucontext = to_ucontext(context);
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
> + uresp.pd_id = pd_id;
> + if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
> + err = -EFAULT;
> + goto error;
> + }
> + } else {
> + dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
> + }
> +
> + irdma_add_pdusecount(iwpd);
> +
> + return 0;
> +error:
> + irdma_free_rsrc(rf, rf->allocated_pds, pd_id);
> +
> + return err;
> +}
> +/**
> + * irdma_create_qp - create qp
> + * @ibpd: ptr of pd
> + * @init_attr: attributes for qp
> + * @udata: user data for create qp
> + */
> +static struct ib_qp *irdma_create_qp(struct ib_pd *ibpd,
> + struct ib_qp_init_attr *init_attr,
> + struct ib_udata *udata)
> +{
> + struct irdma_pd *iwpd = to_iwpd(ibpd);
> + struct irdma_device *iwdev = to_iwdev(ibpd->device);
> + struct irdma_pci_f *rf = iwdev->rf;
> + struct irdma_cqp *iwcqp = &rf->cqp;
> + struct irdma_qp *iwqp;
> + struct irdma_ucontext *ucontext;
> + struct irdma_create_qp_req req;
> + struct irdma_create_qp_resp uresp = {};
> + struct i40iw_create_qp_resp uresp_gen1 = {};
> + u32 qp_num = 0;
> + void *mem;
> + enum irdma_status_code ret;
> + int err_code = 0;
> + int sq_size;
> + int rq_size;
> + struct irdma_sc_qp *qp;
> + struct irdma_sc_dev *dev = &rf->sc_dev;
> + struct irdma_qp_init_info init_info = {};
> + struct irdma_create_qp_info *qp_info;
> + struct irdma_cqp_request *cqp_request;
> + struct cqp_cmds_info *cqp_info;
> + struct irdma_qp_host_ctx_info *ctx_info;
> + struct irdma_iwarp_offload_info *iwarp_info;
> + struct irdma_roce_offload_info *roce_info;
> + struct irdma_udp_offload_info *udp_info;
> + unsigned long flags;
> +
> + if (iwdev->closing)
> + return ERR_PTR(-ENODEV);
> +
> + if (init_attr->create_flags)
> + return ERR_PTR(-EINVAL);
> +
> + if (init_attr->cap.max_inline_data > dev->hw_attrs.max_hw_inline)
> + init_attr->cap.max_inline_data = dev->hw_attrs.max_hw_inline;
> +
> + if (init_attr->cap.max_send_sge > dev->hw_attrs.max_hw_wq_frags)
> + init_attr->cap.max_send_sge = dev->hw_attrs.max_hw_wq_frags;
> +
> + if (init_attr->cap.max_recv_sge > dev->hw_attrs.max_hw_wq_frags)
> + init_attr->cap.max_recv_sge = dev->hw_attrs.max_hw_wq_frags;
AFAIK, you can change the requested values to be greater than or equal to
the values requested. I don't think you can change them to something smaller.
> +
> + sq_size = init_attr->cap.max_send_wr;
> + rq_size = init_attr->cap.max_recv_wr;
> +
> + init_info.vsi = &iwdev->vsi;
> + init_info.qp_uk_init_info.hw_attrs = &dev->hw_attrs;
> + init_info.qp_uk_init_info.sq_size = sq_size;
> + init_info.qp_uk_init_info.rq_size = rq_size;
> + init_info.qp_uk_init_info.max_sq_frag_cnt = init_attr->cap.max_send_sge;
> + init_info.qp_uk_init_info.max_rq_frag_cnt = init_attr->cap.max_recv_sge;
> + init_info.qp_uk_init_info.max_inline_data = init_attr->cap.max_inline_data;
> +
> + mem = kzalloc(sizeof(*iwqp), GFP_KERNEL);
> + if (!mem)
> + return ERR_PTR(-ENOMEM);
> +
> + iwqp = (struct irdma_qp *)mem;
> + iwqp->allocated_buf = mem;
'allocated_buf' feels redundant. Why is iwqp not sufficient?
> + qp = &iwqp->sc_qp;
> + qp->back_qp = (void *)iwqp;
> + qp->push_idx = IRDMA_INVALID_PUSH_PAGE_INDEX;
> +
> + if (irdma_allocate_dma_mem(dev->hw,
> + &iwqp->q2_ctx_mem,
> + IRDMA_Q2_BUF_SIZE + IRDMA_QP_CTX_SIZE,
> + 256)) {
> + err_code = -ENOMEM;
> + goto error;
> + }
> +
> + init_info.q2 = iwqp->q2_ctx_mem.va;
> + init_info.q2_pa = iwqp->q2_ctx_mem.pa;
> + init_info.host_ctx = (void *)init_info.q2 + IRDMA_Q2_BUF_SIZE;
> + init_info.host_ctx_pa = init_info.q2_pa + IRDMA_Q2_BUF_SIZE;
> +
> + if (init_attr->qp_type == IB_QPT_GSI && rf->sc_dev.is_pf)
> + qp_num = 1;
> + else
> + err_code = irdma_alloc_rsrc(rf, rf->allocated_qps, rf->max_qp,
> + &qp_num, &rf->next_qp);
> + if (err_code)
> + goto error;
> +
> + iwqp->iwdev = iwdev;
> + iwqp->iwpd = iwpd;
> + if (init_attr->qp_type == IB_QPT_GSI && !rf->sc_dev.is_pf)
> + iwqp->ibqp.qp_num = 1;
> + else
> + iwqp->ibqp.qp_num = qp_num;
> +
> + qp = &iwqp->sc_qp;
> + iwqp->iwscq = to_iwcq(init_attr->send_cq);
> + iwqp->iwrcq = to_iwcq(init_attr->recv_cq);
> + iwqp->host_ctx.va = init_info.host_ctx;
> + iwqp->host_ctx.pa = init_info.host_ctx_pa;
> + iwqp->host_ctx.size = IRDMA_QP_CTX_SIZE;
> +
> + init_info.pd = &iwpd->sc_pd;
> + init_info.qp_uk_init_info.qp_id = iwqp->ibqp.qp_num;
> + if (!rdma_protocol_roce(&iwdev->iwibdev->ibdev, 1))
> + init_info.qp_uk_init_info.first_sq_wq = 1;
> + iwqp->ctx_info.qp_compl_ctx = (uintptr_t)qp;
> + init_waitqueue_head(&iwqp->waitq);
> + init_waitqueue_head(&iwqp->mod_qp_waitq);
> +
> + if (rdma_protocol_roce(&iwdev->iwibdev->ibdev, 1)) {
> + if (init_attr->qp_type != IB_QPT_RC &&
> + init_attr->qp_type != IB_QPT_UD &&
> + init_attr->qp_type != IB_QPT_GSI) {
> + err_code = -EINVAL;
> + goto error;
> + }
> + } else {
> + if (init_attr->qp_type != IB_QPT_RC) {
> + err_code = -EINVAL;
> + goto error;
> + }
> + }
> +
> + if (iwdev->push_mode)
> + irdma_alloc_push_page(rf, qp);
> +
> + if (udata) {
> + err_code = ib_copy_from_udata(&req, udata, sizeof(req));
Perhaps ib_copy_from_udata(&req, udata, min(sizeof(req), udata->inlen)?
Applies to other call sites of ib_copy_from/to_udata as well.
> + if (err_code) {
> + irdma_debug(dev, IRDMA_DEBUG_ERR,
> + "ib_copy_from_data fail\n");
> + goto error;
> + }
> +
> + iwqp->ctx_info.qp_compl_ctx = req.user_compl_ctx;
> + iwqp->user_mode = 1;
> + ucontext = to_ucontext(ibpd->uobject->context);
> + if (req.user_wqe_bufs) {
> + spin_lock_irqsave(&ucontext->qp_reg_mem_list_lock, flags);
> + iwqp->iwpbl = irdma_get_pbl((unsigned long)req.user_wqe_bufs,
> + &ucontext->qp_reg_mem_list);
> + spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);
> +
> + if (!iwqp->iwpbl) {
> + err_code = -ENODATA;
> + irdma_debug(dev, IRDMA_DEBUG_ERR,
> + "no pbl info\n");
> + goto error;
> + }
> + }
> + err_code = irdma_setup_virt_qp(iwdev, iwqp, &init_info);
> + } else {
> + err_code = irdma_setup_kmode_qp(iwdev, iwqp, &init_info);
> + }
> +
> + if (err_code) {
> + irdma_debug(dev, IRDMA_DEBUG_ERR, "setup qp failed\n");
> + goto error;
> + }
> +
> + if (rdma_protocol_roce(&iwdev->iwibdev->ibdev, 1)) {
> + if (init_attr->qp_type == IB_QPT_RC) {
> + init_info.type = IRDMA_QP_TYPE_ROCE_RC;
> + init_info.qp_uk_init_info.qp_caps =
> + IRDMA_SEND_WITH_IMM | IRDMA_WRITE_WITH_IMM | IRDMA_ROCE;
> + } else {
> + init_info.type = IRDMA_QP_TYPE_ROCE_UD;
> + init_info.qp_uk_init_info.qp_caps = IRDMA_SEND_WITH_IMM | IRDMA_ROCE;
> + }
> + } else {
> + init_info.type = IRDMA_QP_TYPE_IWARP;
> + init_info.qp_uk_init_info.qp_caps = IRDMA_WRITE_WITH_IMM;
> + }
> +
> + ret = dev->iw_priv_qp_ops->qp_init(qp, &init_info);
> + if (ret) {
> + err_code = -EPROTO;
> + irdma_debug(dev, IRDMA_DEBUG_ERR, "qp_init fail\n");
> + goto error;
> + }
> +
> + ctx_info = &iwqp->ctx_info;
> + if (rdma_protocol_roce(&iwdev->iwibdev->ibdev, 1)) {
> + iwqp->ctx_info.roce_info = &iwqp->roce_info;
> + iwqp->ctx_info.udp_info = &iwqp->udp_info;
> + udp_info = &iwqp->udp_info;
> + udp_info->snd_mss = irdma_roce_mtu(iwdev->vsi.mtu);
> + udp_info->cwnd = 0x400;
> + udp_info->src_port = 0xc000;
> + udp_info->dst_port = ROCE_V2_UDP_DPORT;
> + roce_info = &iwqp->roce_info;
> + ether_addr_copy(roce_info->mac_addr, iwdev->netdev->dev_addr);
> +
> + if (init_attr->qp_type == IB_QPT_GSI && !rf->sc_dev.is_pf)
> + roce_info->is_qp1 = true;
> + roce_info->rd_en = true;
> + roce_info->wr_rdresp_en = true;
> + roce_info->dctcp_en = iwdev->dctcp_en;
> + roce_info->ecn_en = iwdev->ecn_en;
> + roce_info->dcqcn_en = iwdev->roce_dcqcn_en;
> + roce_info->timely_en = iwdev->roce_timely_en;
> +
> + roce_info->ack_credits = 0x1E;
> + roce_info->ird_size = IRDMA_MAX_ENCODED_IRD_SIZE;
> + roce_info->ord_size = dev->hw_attrs.max_hw_ord;
> +
> + if (!iwqp->user_mode) {
> + roce_info->priv_mode_en = true;
> + roce_info->fast_reg_en = true;
> + roce_info->udprivcq_en = true;
> + }
> + roce_info->roce_tver = 0;
> + } else {
> + iwqp->ctx_info.iwarp_info = &iwqp->iwarp_info;
> + iwarp_info = &iwqp->iwarp_info;
> + ether_addr_copy(iwarp_info->mac_addr, iwdev->netdev->dev_addr);
> + iwarp_info->rd_en = true;
> + iwarp_info->wr_rdresp_en = true;
> + iwarp_info->ib_rd_en = true;
> + if (!iwqp->user_mode) {
> + iwarp_info->priv_mode_en = true;
> + iwarp_info->fast_reg_en = true;
> + }
> + iwarp_info->ddp_ver = 1;
> + iwarp_info->rdmap_ver = 1;
> + ctx_info->iwarp_info_valid = true;
> + }
> +
> + ctx_info->send_cq_num = iwqp->iwscq->sc_cq.cq_uk.cq_id;
> + ctx_info->rcv_cq_num = iwqp->iwrcq->sc_cq.cq_uk.cq_id;
> + if (qp->push_idx == IRDMA_INVALID_PUSH_PAGE_INDEX) {
> + ctx_info->push_mode_en = false;
> + } else {
> + ctx_info->push_mode_en = true;
> + ctx_info->push_idx = qp->push_idx;
> + }
> +
> + if (rdma_protocol_roce(&iwdev->iwibdev->ibdev, 1)) {
> + ret =
> + dev->iw_priv_qp_ops->qp_setctx_roce(&iwqp->sc_qp,
> + iwqp->host_ctx.va,
> + ctx_info);
> + } else {
> + ret = dev->iw_priv_qp_ops->qp_setctx(&iwqp->sc_qp,
> + iwqp->host_ctx.va,
> + ctx_info);
> + ctx_info->iwarp_info_valid = false;
> + }
> +
> + cqp_request = irdma_get_cqp_request(iwcqp, true);
> + if (!cqp_request) {
> + err_code = -ENOMEM;
> + goto error;
> + }
> +
> + cqp_info = &cqp_request->info;
> + qp_info = &cqp_request->info.in.u.qp_create.info;
> + memset(qp_info, 0, sizeof(*qp_info));
> + qp_info->mac_valid = true;
> + qp_info->cq_num_valid = true;
> + qp_info->next_iwarp_state = IRDMA_QP_STATE_IDLE;
> +
> + cqp_info->cqp_cmd = IRDMA_OP_QP_CREATE;
> + cqp_info->post_sq = 1;
> + cqp_info->in.u.qp_create.qp = qp;
> + cqp_info->in.u.qp_create.scratch = (uintptr_t)cqp_request;
> + ret = irdma_handle_cqp_op(rf, cqp_request);
> + if (ret) {
> + irdma_debug(dev, IRDMA_DEBUG_ERR, "CQP-OP QP create fail");
> + err_code = -ENOMEM;
> + goto error;
> + }
> +
> + irdma_add_ref(&iwqp->ibqp);
> + spin_lock_init(&iwqp->lock);
> + spin_lock_init(&iwqp->sc_qp.pfpdu.lock);
> + iwqp->sig_all = (init_attr->sq_sig_type == IB_SIGNAL_ALL_WR) ? 1 : 0;
> + rf->qp_table[qp_num] = iwqp;
> + irdma_add_pdusecount(iwqp->iwpd);
> + irdma_add_devusecount(iwdev);
> + if (udata) {
> + if (iwdev->rf->sc_dev.hw_attrs.hw_rev == IRDMA_GEN_1) {
> + uresp_gen1.lsmm = 1;
> + uresp_gen1.actual_sq_size = sq_size;
> + uresp_gen1.actual_rq_size = rq_size;
> + uresp_gen1.qp_id = qp_num;
> + uresp_gen1.push_idx = qp->push_idx;
> + uresp_gen1.lsmm = 1;
> + err_code = ib_copy_to_udata(udata, &uresp_gen1, sizeof(uresp_gen1));
> + } else {
> + if (rdma_protocol_iwarp(&iwdev->iwibdev->ibdev, 1))
> + uresp.lsmm = 1;
> + uresp.actual_sq_size = sq_size;
> + uresp.actual_rq_size = rq_size;
> + uresp.qp_id = qp_num;
> + uresp.push_idx = qp->push_idx;
> + uresp.qp_caps = qp->qp_uk.qp_caps;
> +
> + err_code = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> + }
> + if (err_code) {
> + irdma_debug(dev, IRDMA_DEBUG_ERR, "copy_to_udata failed\n");
> + irdma_destroy_qp(&iwqp->ibqp);
> + return ERR_PTR(err_code);
> + }
> + }
> + init_completion(&iwqp->sq_drained);
> + init_completion(&iwqp->rq_drained);
> + return &iwqp->ibqp;
> +
> +error:
> + irdma_free_qp_rsrc(iwdev, iwqp, qp_num);
> +
> + return ERR_PTR(err_code);
> +}
> +
> +/**
> + * irdma_query - query qp attributes
> + * @ibqp: qp pointer
> + * @attr: attributes pointer
> + * @attr_mask: Not used
> + * @init_attr: qp attributes to return
> + */
> +static int irdma_query_qp(struct ib_qp *ibqp,
> + struct ib_qp_attr *attr,
> + int attr_mask,
> + struct ib_qp_init_attr *init_attr)
> +{
> + struct irdma_qp *iwqp = to_iwqp(ibqp);
> + struct irdma_sc_qp *qp = &iwqp->sc_qp;
> +
> + attr->qp_state = iwqp->ibqp_state;
> + attr->cur_qp_state = iwqp->ibqp_state;
> + attr->qp_access_flags = 0;
> + attr->cap.max_send_wr = qp->qp_uk.sq_size - 1;
> + attr->cap.max_recv_wr = qp->qp_uk.rq_size - 1;
Why -1?
> + attr->cap.max_inline_data = qp->qp_uk.max_inline_data;
> + attr->cap.max_send_sge = qp->qp_uk.max_sq_frag_cnt;
> + attr->cap.max_recv_sge = qp->qp_uk.max_rq_frag_cnt;
> + attr->qkey = iwqp->roce_info.qkey;
> +
> + init_attr->event_handler = iwqp->ibqp.event_handler;
> + init_attr->qp_context = iwqp->ibqp.qp_context;
> + init_attr->send_cq = iwqp->ibqp.send_cq;
> + init_attr->recv_cq = iwqp->ibqp.recv_cq;
> + init_attr->srq = iwqp->ibqp.srq;
> + init_attr->cap = attr->cap;
> +
> + return 0;
> +}
> +
> +/**
> + * irdma_destroy_cq - destroy cq
> + * @ib_cq: cq pointer
> + */
> +static int irdma_destroy_cq(struct ib_cq *ib_cq)
> +{
> + struct irdma_cq *iwcq;
> + struct irdma_device *iwdev;
> + struct irdma_sc_cq *cq;
> +
> + if (!ib_cq) {
> + irdma_pr_err("ib_cq == NULL\n");
> + return 0;
> + }
Is this really needed? Which caller can pass NULL pointer?
> +
> + iwcq = to_iwcq(ib_cq);
> + iwdev = to_iwdev(ib_cq->device);
> + cq = &iwcq->sc_cq;
> + irdma_cq_wq_destroy(iwdev->rf, cq);
> + cq_free_rsrc(iwdev->rf, iwcq);
> + kfree(iwcq);
> + irdma_rem_devusecount(iwdev);
> +
> + return 0;
> +}
> +> +/**
> + * irdma_create_stag - create random stag
> + * @iwdev: iwarp device
> + */
> +static u32 irdma_create_stag(struct irdma_device *iwdev)
> +{
> + u32 stag = 0;
> + u32 stag_index = 0;
> + u32 next_stag_index;
> + u32 driver_key;
> + u32 random;
> + u8 consumer_key;
> + int ret;
> +
> + get_random_bytes(&random, sizeof(random));
> + consumer_key = (u8)random;
> +
> + driver_key = random & ~iwdev->rf->mr_stagmask;
> + next_stag_index = (random & iwdev->rf->mr_stagmask) >> 8;
> + next_stag_index %= iwdev->rf->max_mr;
> +
> + ret = irdma_alloc_rsrc(iwdev->rf,
> + iwdev->rf->allocated_mrs, iwdev->rf->max_mr,
> + &stag_index, &next_stag_index);
> + if (!ret) {
> + stag = stag_index << IRDMA_CQPSQ_STAG_IDX_S;
> + stag |= driver_key;
> + stag += (u32)consumer_key;
> + irdma_add_devusecount(iwdev);
> + }
This is confusing IMHO, better to test for 'if (ret)' and keep the main flow
unindented.
> + return stag;
> +}
> +
> +/**
> + * board_id_show
> + */
> +static ssize_t board_id_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%.*s\n", 32, "IRDMA Board ID");
That doesn't add much information.
> +}
> +
> +static DEVICE_ATTR_RO(hw_rev);
> +static DEVICE_ATTR_RO(hca_type);
> +static DEVICE_ATTR_RO(board_id);
> +
> +static struct attribute *irdma_dev_attributes[] = {
> + &dev_attr_hw_rev.attr,
> + &dev_attr_hca_type.attr,
> + &dev_attr_board_id.attr,
> + NULL
> +};
> +
> +static const struct attribute_group irdma_attr_group = {
> + .attrs = irdma_dev_attributes,
> +};
> +
> +/**
> + * irdma_modify_port Modify port properties
> + * @ibdev: device pointer from stack
> + * @port: port number
> + * @port_modify_mask: mask for port modifications
> + * @props: port properties
> + */
> +static int irdma_modify_port(struct ib_device *ibdev,
> + u8 port,
> + int port_modify_mask,
> + struct ib_port_modify *props)
> +{
> + return 0;
> +}
Same question as disacossiate_ucontext.
> +
> +/**
> + * irdma_query_gid_roce - Query port GID for Roce
> + * @ibdev: device pointer from stack
> + * @port: port number
> + * @index: Entry index
> + * @gid: Global ID
> + */
> +static int irdma_query_gid_roce(struct ib_device *ibdev,
> + u8 port,
> + int index,
> + union ib_gid *gid)
> +{
> + int ret;
> +
> + ret = rdma_query_gid(ibdev, port, index, gid);
> + if (ret == -EAGAIN) {
I can't see a path where rdma_query_gid returns -EAGAIN.
> + memcpy(gid, &zgid, sizeof(*gid));
> + return 0;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * irdma_create_ah - create address handle
> + * @ibpd: ptr to protection domain
> + * @ah_attr: address handle attributes
'ah_attr' -> 'attr', missing flags and udata.
> + *
> + * returns a pointer to an address handle
> + */
> +static struct ib_ah *irdma_create_ah(struct ib_pd *ibpd,
> + struct rdma_ah_attr *attr,
> + u32 flags,
> + struct ib_udata *udata)
> +{
> + struct irdma_pd *pd = to_iwpd(ibpd);
> + struct irdma_ah *ah;
> + const struct ib_gid_attr *sgid_attr;
> + struct irdma_device *iwdev = to_iwdev(ibpd->device);
> + struct irdma_pci_f *rf = iwdev->rf;
> + struct irdma_sc_ah *sc_ah;
> + u32 ah_id = 0;
> + struct irdma_ah_info *ah_info;
> + struct irdma_create_ah_resp uresp;
> + union {
> + struct sockaddr saddr;
> + struct sockaddr_in saddr_in;
> + struct sockaddr_in6 saddr_in6;
> + } sgid_addr, dgid_addr;
> + int err;
> + u8 dmac[ETH_ALEN];
> +
> + err = irdma_alloc_rsrc(rf, rf->allocated_ahs,
> + rf->max_ah, &ah_id, &rf->next_ah);
> + if (err)
> + return ERR_PTR(err);
> +
> + ah = kzalloc(sizeof(*ah), GFP_ATOMIC);
> + if (!ah) {
> + irdma_free_rsrc(rf, rf->allocated_ahs, ah_id);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + ah->pd = pd;
> + sc_ah = &ah->sc_ah;
> + sc_ah->ah_info.ah_idx = ah_id;
> + sc_ah->ah_info.vsi = &iwdev->vsi;
> + iwdev->rf->sc_dev.iw_uda_ops->init_ah(&rf->sc_dev, sc_ah);
> + ah->sgid_index = attr->grh.sgid_index;
> + sgid_attr = attr->grh.sgid_attr;
> + memcpy(&ah->dgid, &attr->grh.dgid, sizeof(ah->dgid));
> + rdma_gid2ip(&sgid_addr.saddr, &sgid_attr->gid);
> + rdma_gid2ip(&dgid_addr.saddr, &attr->grh.dgid);
> + ah->av.attrs = *attr;
> + ah->av.net_type = rdma_gid_attr_network_type(sgid_attr);
> + ah->av.sgid_addr.saddr = sgid_addr.saddr;
> + ah->av.dgid_addr.saddr = dgid_addr.saddr;
> + ah_info = &sc_ah->ah_info;
> + ah_info->ah = sc_ah;
> + ah_info->ah_idx = ah_id;
> + ah_info->pd_idx = pd->sc_pd.pd_id;
> + ether_addr_copy(ah_info->mac_addr, iwdev->netdev->dev_addr);
> + if (attr->ah_flags & IB_AH_GRH) {
> + ah_info->flow_label = attr->grh.flow_label;
> + ah_info->hop_ttl = attr->grh.hop_limit;
> + ah_info->tc_tos = attr->grh.traffic_class;
> + }
> +
> + ether_addr_copy(dmac, attr->roce.dmac);
> + if (rdma_gid_attr_network_type(sgid_attr) == RDMA_NETWORK_IPV4) {
> + ah_info->ipv4_valid = true;
> + ah_info->dest_ip_addr[0] =
> + ntohl(dgid_addr.saddr_in.sin_addr.s_addr);
> + ah_info->src_ip_addr[0] =
> + ntohl(sgid_addr.saddr_in.sin_addr.s_addr);
> + ah_info->do_lpbk = irdma_ipv4_is_lpb(ah_info->src_ip_addr[0],
> + ah_info->dest_ip_addr[0]);
> + if (ipv4_is_multicast(dgid_addr.saddr_in.sin_addr.s_addr))
> + irdma_mcast_mac(ah_info->dest_ip_addr, dmac, true);
> + } else {
> + irdma_copy_ip_ntohl(ah_info->dest_ip_addr,
> + dgid_addr.saddr_in6.sin6_addr.in6_u.u6_addr32);
> + irdma_copy_ip_ntohl(ah_info->src_ip_addr,
> + sgid_addr.saddr_in6.sin6_addr.in6_u.u6_addr32);
> + ah_info->do_lpbk = irdma_ipv6_is_lpb(ah_info->src_ip_addr,
> + ah_info->dest_ip_addr);
> + if (rdma_is_multicast_addr(&dgid_addr.saddr_in6.sin6_addr))
> + irdma_mcast_mac(ah_info->dest_ip_addr, dmac, false);
> + }
> + if (sgid_attr->ndev && is_vlan_dev(sgid_attr->ndev))
> + ah_info->vlan_tag = vlan_dev_vlan_id(sgid_attr->ndev);
> + else
> + ah_info->vlan_tag = IRDMA_NO_VLAN;
> +
> + ah_info->dst_arpindex = irdma_add_arp(iwdev->rf, ah_info->dest_ip_addr,
> + ah_info->ipv4_valid, dmac);
> +
> + if (ah_info->dst_arpindex == -1) {
> + err = -EINVAL;
> + goto error;
> + }
> +
> + if (ah_info->vlan_tag != 0xFFFF)
> + ah_info->insert_vlan_tag = true;
> +
> + err = irdma_ah_cqp_op(iwdev->rf, sc_ah, IRDMA_OP_AH_CREATE,
> + flags & RDMA_CREATE_AH_SLEEPABLE,
> + irdma_gsi_ud_qp_ah_cb, sc_ah);
> + if (err) {
> + irdma_debug(&rf->sc_dev, IRDMA_DEBUG_ERR,
> + "CQP-OP Create AH fail");
> + goto error;
> + }
> +
> + if (!(flags & RDMA_CREATE_AH_SLEEPABLE)) {
> + int cnt = CQP_COMPL_WAIT_TIME_MS * CQP_TIMEOUT_THRESHOLD;
> +
> + do {
> + irdma_cqp_ce_handler(rf, &rf->ccq.sc_cq);
> + mdelay(1);
> + } while (!sc_ah->ah_info.ah_valid && --cnt);
> +
> + if (!cnt) {
> + irdma_debug(&rf->sc_dev, IRDMA_DEBUG_ERR,
> + "CQP create AH timed out");
> + err = -ETIMEDOUT;
> + goto error;
> + }
> + }
> +
> + irdma_add_pdusecount(pd);
> + if (udata) {
> + uresp.ah_id = ah->sc_ah.ah_info.ah_idx;
> + err = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> + }
> + return &ah->ibah;
> +
> +error:
> + kfree(ah);
> + irdma_free_rsrc(iwdev->rf, iwdev->rf->allocated_ahs, ah_id);
> +
> + return ERR_PTR(err);
> +}
> +
> +/**
> + * irdma_destroy_ah - Destroy address handle
> + * @ah: pointer to address handle
Missing flags.
> + */
> +static int irdma_destroy_ah(struct ib_ah *ibah, u32 flags)
> +{
> + struct irdma_device *iwdev = to_iwdev(ibah->device);
> + struct irdma_ah *ah = to_iwah(ibah);
> + int err;
> +
> + if (!ah->sc_ah.ah_info.ah_valid)
> + return -EINVAL;
> +
> + err = irdma_ah_cqp_op(iwdev->rf, &ah->sc_ah, IRDMA_OP_AH_DESTROY,
> + flags & RDMA_DESTROY_AH_SLEEPABLE,
> + irdma_destroy_ah_cb, ah);
> + if (!err)
> + return 0;
Why are the rest of the cleanups only in case of error?
> +
> + irdma_free_rsrc(iwdev->rf, iwdev->rf->allocated_ahs,
> + ah->sc_ah.ah_info.ah_idx);
> + irdma_rem_pdusecount(ah->pd, iwdev);
> + kfree(ah);
> +
> + return 0;
> +}
> +
> +static __be64 irdma_mac_to_guid(struct net_device *ndev)
> +{
> + unsigned char *mac = ndev->dev_addr;
> + __be64 guid;
> + unsigned char *dst = (unsigned char *)&guid;
> +
> + dst[0] = mac[0] ^ 2;
> + dst[1] = mac[1];
> + dst[2] = mac[2];
> + dst[3] = 0xff;
> + dst[4] = 0xfe;
> + dst[5] = mac[3];
> + dst[6] = mac[4];
> + dst[7] = mac[5];
> +
> + return guid;
> +}
There's a variant of this function in irdma, bnxt_re, ocrdma and qedr.
Maybe it's time to provide it in common code?
^ permalink raw reply
* Re: [PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for Generic Mux controls
From: Andrew Lunn @ 2019-02-24 14:57 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Leo Li, Peter Rosin, Florian Fainelli, Heiner Kallweit,
netdev@vger.kernel.org
In-Reply-To: <20190224141338.29907-2-pankaj.bansal@nxp.com>
> +static int mdio_mux_multiplexer_switch_fn(int current_child, int desired_child,
> + void *data)
> +{
> + struct platform_device *pdev;
> + struct mdio_mux_multiplexer_state *s;
> + int ret = 0;
> +
> + pdev = (struct platform_device *)data;
> + s = (struct mdio_mux_multiplexer_state *)platform_get_drvdata(pdev);
platform_get_drvdata() returns a void *. So you don't need the cast.
> + if (current_child ^ desired_child) {
> + if (current_child != -1)
> + ret = mux_control_deselect(s->muxc);
> + if (ret)
> + return ret;
Please use {} here, so you check the error when current_child != -1.
> +
> + ret = mux_control_select(s->muxc, desired_child);
> + if (!ret)
> + dev_dbg(&pdev->dev, "%s %d -> %d\n", __func__,
> + current_child, desired_child);
> + }
> +
> + return ret;
> +}
> +
> +static int mdio_mux_multiplexer_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mdio_mux_multiplexer_state *s;
> + int ret = 0;
> +
> + s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> +
> + s->muxc = devm_mux_control_get(dev, NULL);
> + if (IS_ERR(s->muxc)) {
> + ret = PTR_ERR(s->muxc);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Failed to get mux: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, s);
> +
> + ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
> + mdio_mux_multiplexer_switch_fn, &s->mux_handle,
> + pdev, NULL);
> +
> + return ret;
> +}
> +
> +static int mdio_mux_multiplexer_remove(struct platform_device *pdev)
> +{
> + struct mdio_mux_multiplexer_state *s = platform_get_drvdata(pdev);
> +
> + mdio_mux_uninit(s->mux_handle);
I've never used the multiplexer framework before. But i think you need
a call to mux_control_deselect() here in order to unlock the
multiplexer. Without that, it will deadlock when you reload the
module.
Andrew
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: net: Add bindings for mdio mux consumers
From: Andrew Lunn @ 2019-02-24 15:00 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: <20190224141338.29907-1-pankaj.bansal@nxp.com>
On Sun, Feb 24, 2019 at 08:49:21AM +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.
>
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Simon Huelck @ 2019-02-24 15:00 UTC (permalink / raw)
To: Jerome Brunet, Jose Abreu, Martin Blumenstingl
Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
Emiliano Ingrassia, netdev
In-Reply-To: <d7de65c614ee788152300f6d3799fd537b438496.camel@baylibre.com>
Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
> On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
>> Hi,
>>
>>
>>
>> this was changed recently, with a patch for the EEE stuff , see here:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
> Hu, I was not aware this finally went through. Good !
> As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
>
> The realtek PHY has EEE enabled by default. Having this enabled generates a
> lot of (Low Power) Interrupts.
>
> Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
> eventually miss an IRQ and the interface would just die. Unfortunately, it was
> not that easy find out.
>
> 2 years ago, we just noticed that disabling EEE would make the failure go
> away. Forcing this EEE feature off through DT was merely a work around.
>
> Now that the real cause of the problem is known, there is no reason to keep
> this hack around.
>
> Whether EEE adds a performance penality and why, is another topic.
> As Jose pointed out, you can disable EEE at runtime, using ethtool.
>
> Jerome
>
Hi,
i tested the latest patches of **next-20190222, there were some stmmac
improvements.**
**
**
**For the topic i got , the performance stayed identical.**
**C:\Users\Simon\Downloads\iperf3.6_64bit\iperf3.6_64bit>iperf3.exe -c
10.10.11.1 -i1
warning: Ignoring nonsense TCP MSS 0
Connecting to host 10.10.11.1, port 5201
[ 5] local 10.10.11.100 port 50830 connected to 10.10.11.1 port 5201
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 78.9 MBytes 661 Mbits/sec
[ 5] 1.00-2.00 sec 79.1 MBytes 664 Mbits/sec
[ 5] 2.00-3.00 sec 79.4 MBytes 666 Mbits/sec
[ 5] 3.00-4.00 sec 34.4 MBytes 288 Mbits/sec
[ 5] 4.00-5.00 sec 16.1 MBytes 135 Mbits/sec
[ 5] 5.00-6.00 sec 15.8 MBytes 132 Mbits/sec
[ 5] 6.00-7.00 sec 14.2 MBytes 120 Mbits/sec
[ 5] 7.00-8.00 sec 15.6 MBytes 131 Mbits/sec
[ 5] 8.00-9.00 sec 14.9 MBytes 125 Mbits/sec
[ 5] 9.00-10.00 sec 15.0 MBytes 126 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate
[ 5] 0.00-10.00 sec 363 MBytes 305 Mbits/sec sender
[ 5] 0.00-10.04 sec 363 MBytes 303 Mbits/sec
receiver
**
**its clearly visible when i activated the other stream for getting
duplex load ... The highest rate also stays alot under the possible
930MBits that i have seen earlier with 4.14.
**
**
**
**the parallel stream reached around 450Mbits , which almost sums up to
660Mbits. This is what i meant when i said that duplex might be broken.
**
**
**
**Connecting to host 10.10.11.100, port 5201
[ 5] local 10.10.11.1 port 38658 connected to 10.10.11.100 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 62.9 MBytes 528 Mbits/sec 0 65.6 KBytes
[ 5] 1.00-2.00 sec 56.9 MBytes 477 Mbits/sec 0 65.6 KBytes
[ 5] 2.00-3.00 sec 55.9 MBytes 469 Mbits/sec 0 65.6 KBytes
[ 5] 3.00-4.00 sec 53.0 MBytes 445 Mbits/sec 0 65.6 KBytes
[ 5] 4.00-5.00 sec 54.3 MBytes 455 Mbits/sec 0 65.6 KBytes
[ 5] 5.00-6.00 sec 54.8 MBytes 460 Mbits/sec 0 65.6 KBytes
[ 5] 6.00-7.00 sec 45.3 MBytes 380 Mbits/sec 0 65.6 KBytes
[ 5] 7.00-8.00 sec 51.2 MBytes 429 Mbits/sec 0 65.6 KBytes
[ 5] 8.00-9.00 sec 56.1 MBytes 470 Mbits/sec 0 65.6 KBytes
[ 5] 9.00-10.00 sec 55.3 MBytes 464 Mbits/sec 0 65.6 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.00 sec 546 MBytes 458 Mbits/sec 0 sender
[ 5] 0.00-10.00 sec 545 MBytes 457 Mbits/sec
receiver**
**
**
**regards,**
**Simon
**
**
**
^ permalink raw reply
* Re: [RFC v1 04/19] RDMA/irdma: Add driver framework definitions
From: Gal Pressman @ 2019-02-24 15:02 UTC (permalink / raw)
To: Shiraz Saleem, dledford, jgg, davem
Cc: linux-rdma, netdev, mustafa.ismail, jeffrey.t.kirsher
In-Reply-To: <20190215171107.6464-5-shiraz.saleem@intel.com>
On 15-Feb-19 19:10, Shiraz Saleem wrote:
> +/* client interface functions */
> +static const struct i40e_client_ops i40e_ops = {
> + .open = i40iw_open,
> + .close = i40iw_close,
> + .l2_param_change = i40iw_l2param_change,
> + .virtchnl_receive = NULL,
> + .vf_reset = NULL,
> + .vf_enable = NULL,
> + .vf_capable = NULL
NULL assignments are redundant.
> +};
> +
> diff --git a/drivers/infiniband/hw/irdma/irdma_if.c b/drivers/infiniband/hw/irdma/irdma_if.c
> new file mode 100644
> index 0000000..f7b89e9
> --- /dev/null
> +++ b/drivers/infiniband/hw/irdma/irdma_if.c
> @@ -0,0 +1,430 @@
> +// SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB
> +/* Copyright (c) 2019, Intel Corporation. */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <ice_idc.h>
> +#include "main.h"
> +#include "ws.h"
> +#include "icrdma_hw.h"
> +
> +void irdma_add_dev_ref(struct irdma_sc_dev *dev)
> +{
> + try_module_get(THIS_MODULE);
> +}
> +
> +void irdma_put_dev_ref(struct irdma_sc_dev *dev)
> +{
> + module_put(THIS_MODULE);
> +}
What are these used for?
> +
> +/**
> + * irdma_find_iwdev - find a vsi device given a name
> + * @name: name of iwdev
> + */
Can't find uses of this function as well.
> +struct irdma_device *irdma_find_iwdev(const char *name)
> +{
> + struct irdma_handler *hdl;
> + struct list_head *pos;
> + struct list_head *tmp;
> + struct irdma_device *iwdev;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&irdma_handler_lock, flags);
> + list_for_each_entry(hdl, &irdma_handlers, list) {
> + list_for_each_safe(pos, tmp, &hdl->rf.vsi_dev_list) {
> + iwdev = container_of(pos, struct irdma_device, list);
> + if (!strcmp(name, iwdev->iwibdev->ibdev.name)) {
> + spin_unlock_irqrestore(&irdma_handler_lock,
> + flags);
> + return iwdev;
> + }
> + }
> + }
> + spin_unlock_irqrestore(&irdma_handler_lock, flags);
> +
> + return NULL;
> +}
> +
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Simon Huelck @ 2019-02-24 15:02 UTC (permalink / raw)
To: Jerome Brunet, Jose Abreu, Martin Blumenstingl
Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
Emiliano Ingrassia, netdev
In-Reply-To: <8ec64936-c8fa-1f0e-68bf-2ad1d6e8f5d9@gmx.de>
Am 24.02.2019 um 16:00 schrieb Simon Huelck:
> Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
>> On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
>>> Hi,
>>>
>>>
>>>
>>> this was changed recently, with a patch for the EEE stuff , see here:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
>> Hu, I was not aware this finally went through. Good !
>> As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
>>
>> The realtek PHY has EEE enabled by default. Having this enabled generates a
>> lot of (Low Power) Interrupts.
>>
>> Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
>> eventually miss an IRQ and the interface would just die. Unfortunately, it was
>> not that easy find out.
>>
>> 2 years ago, we just noticed that disabling EEE would make the failure go
>> away. Forcing this EEE feature off through DT was merely a work around.
>>
>> Now that the real cause of the problem is known, there is no reason to keep
>> this hack around.
>>
>> Whether EEE adds a performance penality and why, is another topic.
>> As Jose pointed out, you can disable EEE at runtime, using ethtool.
>>
>> Jerome
>>
> Hi,
>
>
> i tested the latest patches of **next-20190222, there were some stmmac
> improvements.**
>
> **
> **
>
> **For the topic i got , the performance stayed identical.**
>
> **C:\Users\Simon\Downloads\iperf3.6_64bit\iperf3.6_64bit>iperf3.exe -c
> 10.10.11.1 -i1
> warning: Ignoring nonsense TCP MSS 0
> Connecting to host 10.10.11.1, port 5201
> [ 5] local 10.10.11.100 port 50830 connected to 10.10.11.1 port 5201
> [ ID] Interval Transfer Bitrate
> [ 5] 0.00-1.00 sec 78.9 MBytes 661 Mbits/sec
> [ 5] 1.00-2.00 sec 79.1 MBytes 664 Mbits/sec
> [ 5] 2.00-3.00 sec 79.4 MBytes 666 Mbits/sec
> [ 5] 3.00-4.00 sec 34.4 MBytes 288 Mbits/sec
> [ 5] 4.00-5.00 sec 16.1 MBytes 135 Mbits/sec
> [ 5] 5.00-6.00 sec 15.8 MBytes 132 Mbits/sec
> [ 5] 6.00-7.00 sec 14.2 MBytes 120 Mbits/sec
> [ 5] 7.00-8.00 sec 15.6 MBytes 131 Mbits/sec
> [ 5] 8.00-9.00 sec 14.9 MBytes 125 Mbits/sec
> [ 5] 9.00-10.00 sec 15.0 MBytes 126 Mbits/sec
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval Transfer Bitrate
> [ 5] 0.00-10.00 sec 363 MBytes 305 Mbits/sec sender
> [ 5] 0.00-10.04 sec 363 MBytes 303 Mbits/sec
> receiver
>
> **
>
> **its clearly visible when i activated the other stream for getting
> duplex load ... The highest rate also stays alot under the possible
> 930MBits that i have seen earlier with 4.14.
> **
>
> **
> **
>
> **the parallel stream reached around 450Mbits , which almost sums up to
> 660Mbits. This is what i meant when i said that duplex might be broken.
> **
>
> **
> **
>
> **Connecting to host 10.10.11.100, port 5201
> [ 5] local 10.10.11.1 port 38658 connected to 10.10.11.100 port 5201
> [ ID] Interval Transfer Bitrate Retr Cwnd
> [ 5] 0.00-1.00 sec 62.9 MBytes 528 Mbits/sec 0 65.6 KBytes
> [ 5] 1.00-2.00 sec 56.9 MBytes 477 Mbits/sec 0 65.6 KBytes
> [ 5] 2.00-3.00 sec 55.9 MBytes 469 Mbits/sec 0 65.6 KBytes
> [ 5] 3.00-4.00 sec 53.0 MBytes 445 Mbits/sec 0 65.6 KBytes
> [ 5] 4.00-5.00 sec 54.3 MBytes 455 Mbits/sec 0 65.6 KBytes
> [ 5] 5.00-6.00 sec 54.8 MBytes 460 Mbits/sec 0 65.6 KBytes
> [ 5] 6.00-7.00 sec 45.3 MBytes 380 Mbits/sec 0 65.6 KBytes
> [ 5] 7.00-8.00 sec 51.2 MBytes 429 Mbits/sec 0 65.6 KBytes
> [ 5] 8.00-9.00 sec 56.1 MBytes 470 Mbits/sec 0 65.6 KBytes
> [ 5] 9.00-10.00 sec 55.3 MBytes 464 Mbits/sec 0 65.6 KBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval Transfer Bitrate Retr
> [ 5] 0.00-10.00 sec 546 MBytes 458 Mbits/sec 0 sender
> [ 5] 0.00-10.00 sec 545 MBytes 457 Mbits/sec
> receiver**
>
> **
> **
>
> **regards,**
>
> **Simon
> **
>
> **
> **
>
Hi,
and another thing that i recognized , TX from my odroid stays around 450
Mbits ( is not affected by RX). but RX suffers alot from the TX stuff (
the max sum of RX/TX seems to be the magical 650MBits)
regards,
Simon
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-24 15:04 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Russell King - ARM Linux admin, Florian Fainelli,
netdev@vger.kernel.org
In-Reply-To: <6dd3f82a-eea0-c9b4-8dd6-f2f313578b1f@gmail.com>
> I think what's not correct is that phydev->autoneg is set
> (by phy_device_create) for a fixed link.
Fixed-link tries to emulate auto-neg:
bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
Maybe it needs better emulation of auto-neg?
Andrew
^ permalink raw reply
* [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Tariq Toukan @ 2019-02-24 15:08 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev, Eran Ben Elisha, Aya Levin, Tariq Toukan
From: Aya Levin <ayal@mellanox.com>
Introduce 50Gbps per lane link modes and 200Gbps speed, update print
functions and initialization functions accordingly.
In addition, update related man page accordingly.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
ethtool-copy.h | 19 ++++++++++++++++++-
ethtool.8.in | 15 +++++++++++++++
ethtool.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/ethtool-copy.h b/ethtool-copy.h
index 6bfbb85f9402..de459900b2d3 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -1455,15 +1455,31 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_FEC_NONE_BIT = 49,
ETHTOOL_LINK_MODE_FEC_RS_BIT = 50,
ETHTOOL_LINK_MODE_FEC_BASER_BIT = 51,
+ ETHTOOL_LINK_MODE_50000baseKR_Full_BIT = 52,
+ ETHTOOL_LINK_MODE_50000baseSR_Full_BIT = 53,
+ ETHTOOL_LINK_MODE_50000baseCR_Full_BIT = 54,
+ ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT = 55,
+ ETHTOOL_LINK_MODE_50000baseDR_Full_BIT = 56,
+ ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT = 57,
+ ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT = 58,
+ ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT = 59,
+ ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT = 60,
+ ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT = 61,
+ ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT = 62,
+ ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT = 63,
+ ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT = 64,
+ ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT = 65,
+ ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66,
/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
* 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
* macro for bits > 31. The only way to use indices > 31 is to
* use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
*/
+ ETHTOOL_LINK_MODE_LAST,
__ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
+ = ETHTOOL_LINK_MODE_LAST - 1,
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
@@ -1571,6 +1587,7 @@ enum ethtool_link_mode_bit_indices {
#define SPEED_50000 50000
#define SPEED_56000 56000
#define SPEED_100000 100000
+#define SPEED_200000 200000
#define SPEED_UNKNOWN -1
diff --git a/ethtool.8.in b/ethtool.8.in
index 5a26cff5fb33..64ce0711ad5f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -650,6 +650,11 @@ lB l lB.
0x400000000 50000baseCR2 Full
0x800000000 50000baseKR2 Full
0x10000000000 50000baseSR2 Full
+0x10000000000000 50000baseKR Full
+0x20000000000000 50000baseSR Full
+0x40000000000000 50000baseCR Full
+0x80000000000000 50000baseLR_ER_FR Full
+0x100000000000000 50000baseDR Full
0x8000000 56000baseKR4 Full
0x10000000 56000baseCR4 Full
0x20000000 56000baseSR4 Full
@@ -658,6 +663,16 @@ lB l lB.
0x2000000000 100000baseSR4 Full
0x4000000000 100000baseCR4 Full
0x8000000000 100000baseLR4_ER4 Full
+0x200000000000000 100000baseKR2 Full
+0x400000000000000 100000baseSR2 Full
+0x800000000000000 100000baseCR2 Full
+0x1000000000000000 100000baseLR2_ER2_FR2 Full
+0x2000000000000000 100000baseDR2 Full
+0x4000000000000000 200000baseKR4 Full
+0x8000000000000000 200000baseSR4 Full
+0x10000000000000000 200000baseLR4_ER4_FR4 Full
+0x20000000000000000 200000baseDR4 Full
+0x40000000000000000 200000baseCR4 Full
.TE
.TP
.BI phyad \ N
diff --git a/ethtool.c b/ethtool.c
index fb4c0886ca84..1a23be818ec1 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -530,6 +530,21 @@ static void init_global_link_mode_masks(void)
ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+ ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+ ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+ ETHTOOL_LINK_MODE_50000baseDR_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT,
+ ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT,
+ ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT,
+ ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT,
+ ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
};
static const enum ethtool_link_mode_bit_indices
additional_advertised_flags_bits[] = {
@@ -689,6 +704,36 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
"2500baseT/Full" },
{ 0, ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
"5000baseT/Full" },
+ { 0, ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+ "50000baseKR/Full" },
+ { 0, ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+ "50000baseSR/Full" },
+ { 0, ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+ "50000baseCR/Full" },
+ { 0, ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+ "50000baseLR_ER_FR/Full" },
+ { 0, ETHTOOL_LINK_MODE_50000baseDR_Full_BIT,
+ "50000baseDR/Full" },
+ { 0, ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT,
+ "100000baseKR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT,
+ "100000baseSR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+ "100000baseCR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
+ "100000baseLR2_ER2_FR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT,
+ "100000baseDR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT,
+ "200000baseKR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT,
+ "200000baseSR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT,
+ "200000baseLR4_ER4_FR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT,
+ "200000baseDR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
+ "200000baseCR4/Full" },
};
int indent;
int did1, new_line_pend, i;
@@ -2958,6 +3003,13 @@ static int do_sset(struct cmd_context *ctx)
else if (speed_wanted == SPEED_10000 &&
duplex_wanted == DUPLEX_FULL)
adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
+ else if (speed_wanted == SPEED_20000 &&
+ duplex_wanted == DUPLEX_FULL)
+ adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
+ ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
if (adv_bit >= 0) {
advertising_wanted = mask_advertising_wanted;
--
1.8.3.1
^ permalink raw reply related
* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 15:15 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Heiner Kallweit, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190224150403.GF26626@lunn.ch>
On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
> > I think what's not correct is that phydev->autoneg is set
> > (by phy_device_create) for a fixed link.
>
> Fixed-link tries to emulate auto-neg:
>
> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>
> Maybe it needs better emulation of auto-neg?
Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
(BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
so according to 802.3-2015, we should not be setting 1.5
(BMSR_ANEGCOMPLETE).
However, swphy does try to emulate autonegotiation - we do have cases
where swphy is used in situations where the speed and duplex are not
fixed. It returns an emulated link partner advertisement for the
current speed, which would suggest that we should set BMCR_ANENABLE.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH iproute2-next v2 00/19] Export object IDs to users
From: David Ahern @ 2019-02-24 15:16 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger,
Steve Wise
In-Reply-To: <20190223091528.8509-1-leon@kernel.org>
On 2/23/19 4:15 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog:
> v1->v2:
> * Fixed commit messages
> * Added Steve's ROB
> v0->v1:
> * Rebased to latest iproute2-next
> * Added latest rdma_netlink.h and updated commit message to point
> to kernel SHA commit.
>
> This series adds ability to present and query all known to rdmatool
> object by their respective, unique IDs (e.g. pdn. mrn, cqn e.t.c).
> All objects which have "parent" object has this information too.
>
applied to iproute2-next. Thanks, Leon.
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-24 15:28 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn
Cc: Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190224151555.xsopwkuicsop65rw@shell.armlinux.org.uk>
On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
> On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
>>> I think what's not correct is that phydev->autoneg is set
>>> (by phy_device_create) for a fixed link.
>>
>> Fixed-link tries to emulate auto-neg:
>>
>> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>>
>> Maybe it needs better emulation of auto-neg?
>
> Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
> (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
> so according to 802.3-2015, we should not be setting 1.5
> (BMSR_ANEGCOMPLETE).
>
> However, swphy does try to emulate autonegotiation - we do have cases
> where swphy is used in situations where the speed and duplex are not
> fixed. It returns an emulated link partner advertisement for the
> current speed, which would suggest that we should set BMCR_ANENABLE.
>
If we emulate auto-neg, then it's not needed to set the speed bits
in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
don't support forced mode. So we may have to go with auto-neg.
To avoid the original issue it should be sufficient to copy
supported -> advertising at a suited place.
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 15:31 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Heiner Kallweit, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190223234235.GA26626@lunn.ch>
On Sun, Feb 24, 2019 at 12:42:35AM +0100, Andrew Lunn wrote:
> Looking forward, at some point we are going to have to make fixed-link
> support higher speeds. That probably means we need a swphy-c45 which
> emulates the standard registers for 2.5G, 5G and 10G. At that point
> genphy will not work...
Do we _need_ to emulate Clause 45 PHYs? Today, the MII interface does
not work with Clause 45 PHYs, so there is no userspace accessing these
PHYs out there. I have a patch that adds support for the MII ioctls
which gives me the ability to poke around in the 88x3310 for
investigatory / debug purposes.
However, I don't see the point of adding what would be very complex
Clause 45 support (we'd have to emulate the PMA/PMD, PCS and AN as a
minimum) when we have the ethtool API, and then we have the issue
that not all advertisement modes are standardised in Clause 45 -
there are some missing.
As I understand it, the swphy for fixed-links only exists because we
have existing tooling that expects Clause 22 PHYs to exist. This will
break even if we were to add Clause 45 support as many bits in the
first 16 registers have different meanings.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: No traffic with Marvell switch and latest linux-next
From: Russell King - ARM Linux admin @ 2019-02-24 15:34 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <403607a0-16d8-15b6-f0aa-b8b13793d401@gmail.com>
On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
> >>> I think what's not correct is that phydev->autoneg is set
> >>> (by phy_device_create) for a fixed link.
> >>
> >> Fixed-link tries to emulate auto-neg:
> >>
> >> bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
> >>
> >> Maybe it needs better emulation of auto-neg?
> >
> > Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
> > (BMSR_ANEGCAPABLE). In any case, 0.12 (BMCR_ANENABLE) is not set,
> > so according to 802.3-2015, we should not be setting 1.5
> > (BMSR_ANEGCOMPLETE).
> >
> > However, swphy does try to emulate autonegotiation - we do have cases
> > where swphy is used in situations where the speed and duplex are not
> > fixed. It returns an emulated link partner advertisement for the
> > current speed, which would suggest that we should set BMCR_ANENABLE.
> >
> If we emulate auto-neg, then it's not needed to set the speed bits
> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
> don't support forced mode. So we may have to go with auto-neg.
Sure.
> To avoid the original issue it should be sufficient to copy
> supported -> advertising at a suited place.
Why bother - the software PHY emulation is an emulation to allow
existing userspace that pre-dates the ethtool API to get some link
parameters. If we augment the PHY emulation in non-standard ways,
userspace will need to be updated to handle those non-standard
ways. If userspace needs to be updated, why not just bite the
bullet and update to ethtool APIs rather than adding more
complication through an emulation layer?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox