* [PATCH for-next 3/4] devlink: fix boolean JSON print
From: Aya Levin @ 2019-02-10 18:28 UTC (permalink / raw)
To: David Ahern, netdev, David S. Miller, Jiri Pirko
Cc: Moshe Shemesh, Aya Levin, Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-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>
---
devlink/devlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 46e2e41c5dfd..a433883f1b2b 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1605,10 +1605,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 for-next 4/4] devlink: add health command support
From: Aya Levin @ 2019-02-10 18:28 UTC (permalink / raw)
To: David Ahern, netdev, David S. Miller, Jiri Pirko
Cc: Moshe Shemesh, Aya Levin, Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-1-git-send-email-ayal@mellanox.com>
This patch adds support for the following commands:
devlink health show [DEV reporter REPORTE_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 NAME VALUE
* show: Devlink health show command displays status and configuration info on
specific reporter on a device or dump the info on all reporters on all
devices.
* recover: Devlink health recover enables the user to initiate a
recovery on a reporter. This operation will increment the recoveries
counter displayed in the show command.
* diagnose: Devlink health diagnose enables the user to retrieve diagnostics data
on a reporter on a device. The command's output is a free text defined
by the reporter.
* dump show: Devlink health dump show 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.
dump output is defined by the reporter.
* dump clear: Devlink health dump clear, deletes the last saved dump file.
* set: Devlink health set, enables the user to configure:
1) grace_period [msec] time interval between auto recoveries.
2) auto_recover [true/false] whether the devlink should execute
automatic recover on error.
Examples:
$devlink health show pci/0000:00:09.0 reporter tx
pci/0000:00:09.0:
name tx
state healthy #err 0 #recover 1 last_dump_ts N/A
parameters:
grace period 600 auto_recover true
$devlink health diagnose pci/0000:00:09.0 reporter tx
SQs:
sqn: 4283 HW state: 1 stopped: false
sqn: 4288 HW state: 1 stopped: false
sqn: 4293 HW state: 1 stopped: false
sqn: 4298 HW state: 1 stopped: false
sqn: 4303 HW state: 1 stopped: false
$devlink health dump show pci/0000:00:09.0 reporter tx
TX dump data
$devlink health dump clear pci/0000:00:09.0 reporter tx
$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>
---
devlink/devlink.c | 551 ++++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/devlink.h | 23 ++
man/man8/devlink-health.8 | 176 ++++++++++++++
man/man8/devlink.8 | 7 +-
4 files changed, 755 insertions(+), 2 deletions(-)
create mode 100644 man/man8/devlink-health.8
diff --git a/devlink/devlink.c b/devlink/devlink.c
index a433883f1b2b..2ddbf389a3ea 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -22,6 +22,8 @@
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
+#include <sys/sysinfo.h>
+#include <sys/queue.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -41,6 +43,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_FORMAT_LEN 80
+
static int g_new_line_count;
#define pr_err(args...) fprintf(stderr, ##args)
@@ -200,6 +206,9 @@ 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)
+#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 */
@@ -231,6 +240,9 @@ struct dl_opts {
uint32_t region_snapshot_id;
uint64_t region_address;
uint64_t region_length;
+ const char *reporter_name;
+ uint64_t reporter_graceful_period;
+ bool reporter_auto_recover;
};
struct dl {
@@ -391,6 +403,17 @@ 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_FMSG] = MNL_TYPE_NESTED,
+ [DEVLINK_ATTR_FMSG_OBJ_NAME] = MNL_TYPE_STRING,
+ [DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE] = MNL_TYPE_U8,
+ [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] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_RECOVER] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] = MNL_TYPE_U8,
};
static int attr_cb(const struct nlattr *attr, void *data)
@@ -822,6 +845,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);
@@ -976,6 +1017,7 @@ static const struct dl_args_metadata dl_args_required[] = {
{DL_OPT_REGION_SNAPSHOT_ID, "Region snapshot id expected.\n"},
{DL_OPT_REGION_ADDRESS, "Region address value expected.\n"},
{DL_OPT_REGION_LENGTH, "Region length value expected.\n"},
+ {DL_OPT_HEALTH_REPORTER_NAME, "Reporter's name is expected.\n"},
};
static int validate_finding_required_dl_args(uint32_t o_required,
@@ -1231,6 +1273,28 @@ 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 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;
@@ -1328,6 +1392,16 @@ 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);
+ 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,
@@ -5677,11 +5751,482 @@ 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;
+ 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 health_value_show(struct dl *dl, int type, struct nlattr *nl_data)
+{
+ const char *str;
+ uint8_t *data;
+ uint32_t len;
+ uint64_t val_u64;
+ uint32_t val_u32;
+ uint16_t val_u16;
+ uint8_t val_u8;
+ bool val_bool;
+ int i;
+
+ switch (type) {
+ case MNL_TYPE_FLAG:
+ val_bool = mnl_attr_get_u8(nl_data);
+ if (dl->json_output)
+ jsonw_bool(dl->jw, !!val_bool);
+ else
+ pr_out("%s ", val_bool ? "true" : "false");
+ break;
+ case MNL_TYPE_U8:
+ val_u8 = mnl_attr_get_u8(nl_data);
+ if (dl->json_output)
+ jsonw_uint(dl->jw, val_u8);
+ else
+ pr_out("%u ", val_u8);
+ break;
+ case MNL_TYPE_U16:
+ val_u16 = mnl_attr_get_u16(nl_data);
+ if (dl->json_output)
+ jsonw_uint(dl->jw, val_u16);
+ else
+ pr_out("%u ", val_u16);
+ break;
+ case MNL_TYPE_U32:
+ val_u32 = mnl_attr_get_u32(nl_data);
+ if (dl->json_output)
+ jsonw_uint(dl->jw, val_u32);
+ else
+ pr_out("%u ", val_u32);
+ break;
+ case MNL_TYPE_U64:
+ val_u64 = mnl_attr_get_u64(nl_data);
+ if (dl->json_output)
+ jsonw_u64(dl->jw, val_u64);
+ else
+ pr_out("%lu ", val_u64);
+ break;
+ case MNL_TYPE_NUL_STRING:
+ str = mnl_attr_get_str(nl_data);
+ if (dl->json_output)
+ jsonw_string(dl->jw, str);
+ else
+ pr_out("%s ", str);
+ break;
+ case MNL_TYPE_BINARY:
+ len = mnl_attr_get_payload_len(nl_data);
+ data = mnl_attr_get_payload(nl_data);
+ i = 0;
+ while (i < len) {
+ if (dl->json_output) {
+ jsonw_printf(dl->jw, "%d", data[i]);
+ } else {
+ pr_out("%02x ", data[i]);
+ if (!(i % 15))
+ pr_out("\n");
+ }
+ i++;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+ return MNL_CB_OK;
+}
+
+struct nest_qentry {
+ int attr_type;
+
+ TAILQ_ENTRY(nest_qentry) nest_entries;
+};
+
+struct health_cb_data {
+ struct dl *dl;
+ uint8_t value_type;
+
+ TAILQ_HEAD(, nest_qentry) qhead;
+};
+
+static int cmd_health_nest_queue(struct health_cb_data *health_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(&health_data->qhead, entry, nest_entries);
+ } else {
+ if (TAILQ_EMPTY(&health_data->qhead))
+ return MNL_CB_ERROR;
+ entry = TAILQ_FIRST(&health_data->qhead);
+ *attr_value = entry->attr_type;
+ TAILQ_REMOVE(&health_data->qhead, entry, nest_entries);
+ free(entry);
+ }
+ return MNL_CB_OK;
+}
+
+static int cmd_health_nest(struct health_cb_data *health_data,
+ uint8_t nest_value, bool start)
+{
+ struct dl *dl = health_data->dl;
+ uint8_t value = nest_value;
+ int err;
+
+ err = cmd_health_nest_queue(health_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_health_object_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ struct health_cb_data *health_data = data;
+ struct dl *dl = health_data->dl;
+ struct nlattr *nla_object;
+ const char *name;
+ 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_health_nest(health_data, attr_type, true);
+ if (err != MNL_CB_OK)
+ return err;
+ break;
+ case DEVLINK_ATTR_FMSG_NEST_END:
+ err = cmd_health_nest(health_data, attr_type, false);
+ if (err != MNL_CB_OK)
+ return err;
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_NAME:
+ name = mnl_attr_get_str(nla_object);
+ if (dl->json_output)
+ jsonw_name(dl->jw, name);
+ else
+ pr_out("%s: ", name);
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE:
+ health_data->value_type = mnl_attr_get_u8(nla_object);
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA:
+ err = health_value_show(dl, health_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 nlmsghdr *nlh;
+ struct health_cb_data data;
+ 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_health_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_dump_show(struct dl *dl)
+{
+ return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET);
+}
+
+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,
+};
+
+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 *output)
+{
+ 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;
+ /* substruct uptime in sec from now
+ * add time_ms and 5 minutes factor
+ */
+ sec = now - (s_info.uptime - time_ms / 1000) + 300;
+ info = localtime(&sec);
+out:
+ strftime(output, HEALTH_REPORTER_TIMESTAMP_FORMAT_LEN,
+ "%Y-%b-%d %H:%M:%S", info);
+}
+
+static void pr_out_health(struct dl *dl, struct nlattr **tb)
+{
+ char dump_time_date[HEALTH_REPORTER_TIMESTAMP_FORMAT_LEN] = "N/A";
+ struct nlattr *hlt[DEVLINK_ATTR_MAX + 1] = {};
+ enum devlink_health_reporter_state state;
+ bool auto_recover = false;
+ const struct nlattr *attr;
+ uint64_t time_ms;
+ int err;
+
+ state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;
+
+ err = mnl_attr_parse_nested(tb[DEVLINK_ATTR_HEALTH_REPORTER], attr_cb,
+ hlt);
+ if (err != MNL_CB_OK)
+ return;
+
+ if (!hlt[DEVLINK_ATTR_HEALTH_REPORTER_NAME] ||
+ !hlt[DEVLINK_ATTR_HEALTH_REPORTER_ERR] ||
+ !hlt[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER] ||
+ !hlt[DEVLINK_ATTR_HEALTH_REPORTER_STATE] ||
+ !hlt[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] ||
+ !hlt[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
+ return;
+
+ if (hlt[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
+ attr = hlt[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
+ time_ms = mnl_attr_get_u64(attr);
+ format_logtime(time_ms, dump_time_date);
+ }
+ pr_out_handle_start_arr(dl, tb);
+
+ pr_out_str(dl, "name",
+ mnl_attr_get_str(hlt[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
+ if (!dl->json_output) {
+ __pr_out_newline();
+ __pr_out_indent_inc();
+ }
+ state = mnl_attr_get_u8(hlt[DEVLINK_ATTR_HEALTH_REPORTER_STATE]);
+ pr_out_str(dl, "state", health_state_name(state));
+ pr_out_u64(dl, "#err",
+ mnl_attr_get_u64(hlt[DEVLINK_ATTR_HEALTH_REPORTER_ERR]));
+ pr_out_u64(dl, "#recover",
+ mnl_attr_get_u64(hlt[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER]));
+ pr_out_str(dl, "last_dump_ts", dump_time_date);
+ pr_out_array_start(dl, "parameters");
+ pr_out_entry_start(dl);
+ pr_out_u64(dl, "grace_period",
+ mnl_attr_get_u64(hlt[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]));
+ auto_recover = mnl_attr_get_u8(hlt[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER]);
+ pr_out_bool(dl, "auto_recover", auto_recover);
+ pr_out_entry_end(dl);
+ pr_out_array_end(dl);
+ __pr_out_indent_dec();
+ __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);
+ if (err)
+ return err;
+ 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");
+ pr_err("Usage: devlink health recover DEV reporter REPORTER_NAME\n");
+ pr_err("Usage: devlink health diagnose DEV reporter REPORTER_NAME\n");
+ pr_err("Usage: devlink health dump show DEV reporter REPORTER_NAME\n");
+ pr_err("Usage: devlink health dump clear DEV reporter REPORTER_NAME\n");
+ pr_err("Usage: devlink health set DEV reporter REPORTER_NAME NAME VALUE\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);
+ } 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);
+ } 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);
+ } else if (dl_argv_match(dl, "clear")) {
+ 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));
+ 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");
}
@@ -5714,7 +6259,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;
}
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index d51b59a7b8ee..bec76e94bc47 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -95,6 +95,12 @@ enum devlink_command {
DEVLINK_CMD_PORT_PARAM_DEL,
DEVLINK_CMD_INFO_GET, /* can dump */
+ DEVLINK_CMD_HEALTH_REPORTER_GET,
+ DEVLINK_CMD_HEALTH_REPORTER_SET,
+ DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
+ DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
+ DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
+ DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
/* add new commands above here */
__DEVLINK_CMD_MAX,
@@ -302,6 +308,23 @@ enum devlink_attr {
DEVLINK_ATTR_SB_POOL_CELL_SIZE, /* u32 */
+ DEVLINK_ATTR_FMSG, /* nested */
+ DEVLINK_ATTR_FMSG_OBJ_NEST_START, /* flag */
+ DEVLINK_ATTR_FMSG_PAIR_NEST_START, /* flag */
+ DEVLINK_ATTR_FMSG_ARR_NEST_START, /* flag */
+ DEVLINK_ATTR_FMSG_NEST_END, /* flag */
+ DEVLINK_ATTR_FMSG_OBJ_NAME, /* string */
+ DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, /* u8 */
+ DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA, /* dynamic */
+
+ DEVLINK_ATTR_HEALTH_REPORTER, /* nested */
+ DEVLINK_ATTR_HEALTH_REPORTER_NAME, /* string */
+ DEVLINK_ATTR_HEALTH_REPORTER_STATE, /* u8 */
+ DEVLINK_ATTR_HEALTH_REPORTER_ERR, /* u64 */
+ DEVLINK_ATTR_HEALTH_REPORTER_RECOVER, /* u64 */
+ DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS, /* u64 */
+ DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD, /* u64 */
+ DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER, /* u8 */
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/man/man8/devlink-health.8 b/man/man8/devlink-health.8
new file mode 100644
index 000000000000..2824d4d1bbf1
--- /dev/null
+++ b/man/man8/devlink-health.8
@@ -0,0 +1,176 @@
+.TH DEVLINK\-HEALTH 8 "27 Dec 2018" "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 "[ "
+.RI "" DEV ""
+.BR " reporter "
+.RI ""REPORTER " ] "
+.ti -8
+.BR "devlink health recover"
+.RI "" DEV ""
+.BR "reporter"
+.RI "" REPORTER ""
+.ti -8
+.BR "devlink health diagnose"
+.RI "" DEV ""
+.BR "reporter"
+.RI "" REPORTER ""
+.ti -8
+.BR "devlink health dump show"
+.RI "" DEV ""
+.BR "reporter"
+.RI "" REPORTER ""
+.ti -8
+.BR "devlink health dump clear"
+.RI "" DEV ""
+.BR "reporter"
+.RI "" REPORTER ""
+.ti -8
+.BR "devlink health set"
+.RI "" DEV ""
+.BR "reporter"
+.RI "" REPORTER ""
+.RI "" NAME ""
+.RI "" VALUE ""
+.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 auto recoveries.
+.P
+2) auto_recover [true/false] - Indicates whether the devlink should execute automatic recover on error.
+.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
+pci/0000:00:09.0:
+ name tx
+ state healthy #err 1 #recover 1 last_dump_ts N/A
+ parameters:
+ grace period 600 auto_recover true
+.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
+.PD 0
+SQs:
+.P
+sqn: 4283 HW state: 1 stopped: 0
+.P
+sqn: 4288 HW state: 1 stopped: 0
+.P
+sqn: 4293 HW state: 1 stopped: 0
+.P
+sqn: 4298 HW state: 1 stopped: 0
+.PD
+.RE
+.PP
+devlink health dump show pci/0000:00:09.0 reporter tx
+.RS 4
+Display the last saved dump on tx reporter registered on pci/0000:00:09.0.
+.RE
+.PP
+devlink health dump clear pci/0000:00:09.0 reporter tx
+.RS 4
+Delete saved dump on tx reporter registered on pci/0000:00:09.0.
+.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
+tx reporter registered on pci/0000:00:09.0.
+.RE
+.PP
+devlink health set pci/0000:00:09.0 reporter tx auto_recover false
+.RS 4
+Turn off auto recovery on tx reporter registered on pci/0000:00:09.0.
+.RE
+.SH SEE ALSO
+.BR devlink (8),
+.BR devlink-dev (8),
+.BR devlink-port (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
* Re: tc qdisc kernel crash
From: Cong Wang @ 2019-02-10 18:30 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, 921542, Adrian
In-Reply-To: <daa17f14ff9de9dc9be4d8ab08f0a804576af669.camel@decadent.org.uk>
On Sun, Feb 10, 2019 at 7:54 AM Ben Hutchings <ben@decadent.org.uk> wrote:
>
> Control: tag -1 confirmed upstream
> Control: found -1 4.20-1~exp1
>
> Adrian (cc'd) reported (https://bugs.debian.org/921542) that a script
> using tc could trigger a kernel crash. I've simplified the script he
> provided down to:
>
> --- BEGIN ---
> #!/bin/sh -ex
>
> modprobe ifb
>
> while true; do
> tc qdisc add dev ifb0 root handle 2:0 prio bands 5
> tc qdisc add dev ifb0 parent 2:5 sfq
> tc filter add dev ifb0 parent 2:0 protocol ip prio 5 handle 0 tcindex mask 0 classid 2:5 pass_on
> tc qdisc del dev ifb0 root || true
> done
> --- END ---
>
> The crash is still reproducible in 4.20 and 5.0-rc5. KASan shows a
> use-after-free:
Thanks for the reproducer and report! I will send a fix.
^ permalink raw reply
* [iproute2-next, 0/4] Add support for devlink health
From: Aya Levin @ 2019-02-10 18:35 UTC (permalink / raw)
To: David Ahern, netdev, David S. Miller, Jiri Pirko
Cc: Moshe Shemesh, Aya Levin, Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549532202-943-1-git-send-email-eranbe@mellanox.com>
This series adds support for devlink health commands:
devlink health show [DEV reporter REPORTE_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 NAME VALUE
The first patch refactors the validation of input parameters, which
grow way too long. Patch 2 and 3 fix bugs that were discovered during
the devlink health development. Patch 4 adds the devlink health
functionality.
Aya Levin (4):
devlink: refactor validation of finding required arguments
devlink: fix print of uint64_t
devlink: fix boolean JSON print
devlink: add health command support
devlink/devlink.c | 721 ++++++++++++++++++++++++++++++++++++-------
include/uapi/linux/devlink.h | 23 ++
man/man8/devlink-health.8 | 176 +++++++++++
man/man8/devlink.8 | 7 +-
4 files changed, 813 insertions(+), 114 deletions(-)
create mode 100644 man/man8/devlink-health.8
--
2.14.1
^ permalink raw reply
* [PATCH net-next 0/3] net: phy: add and use register modifying helpers returning 1 on change
From: Heiner Kallweit @ 2019-02-10 18:56 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Add and use register modifying helpers returning 1 on change.
Heiner Kallweit (3):
net: phy: add register modifying helpers returning 1 on change
net: phy: marvell10g: fix usage of new MMD modifying helpers
net: phy: use phy_modify_changed in genphy_config_advert
drivers/net/phy/marvell10g.c | 13 ++--
drivers/net/phy/phy-core.c | 127 ++++++++++++++++++++++++++++++++---
drivers/net/phy/phy_device.c | 43 +++++-------
include/linux/phy.h | 12 +++-
4 files changed, 149 insertions(+), 46 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next 1/3] net: phy: add register modifying helpers returning 1 on change
From: Heiner Kallweit @ 2019-02-10 18:57 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>
When modifying registers there are scenarios where we need to know
whether the register content actually changed. This patch adds
new helpers to not break users of the current ones, phy_modify() etc.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-core.c | 127 ++++++++++++++++++++++++++++++++++---
include/linux/phy.h | 12 +++-
2 files changed, 128 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 7d6aad287..cdea028d1 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -531,7 +531,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
EXPORT_SYMBOL(phy_write_mmd);
/**
- * __phy_modify() - Convenience function for modifying a PHY register
+ * __phy_modify_changed() - Convenience function for modifying a PHY register
* @phydev: a pointer to a &struct phy_device
* @regnum: register number
* @mask: bit mask of bits to clear
@@ -539,16 +539,69 @@ EXPORT_SYMBOL(phy_write_mmd);
*
* Unlocked helper function which allows a PHY register to be modified as
* new register value = (old register value & ~mask) | set
+ *
+ * Returns negative errno, 0 if there was no change, and 1 in case of change
*/
-int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
+ u16 set)
{
- int ret;
+ int new, ret;
ret = __phy_read(phydev, regnum);
if (ret < 0)
return ret;
- ret = __phy_write(phydev, regnum, (ret & ~mask) | set);
+ new = (ret & ~mask) | set;
+ if (new == ret)
+ return 0;
+
+ ret = __phy_write(phydev, regnum, new);
+
+ return ret < 0 ? ret : 1;
+}
+EXPORT_SYMBOL_GPL(__phy_modify_changed);
+
+/**
+ * phy_modify_changed - Function for modifying a PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to modify
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ *
+ * Returns negative errno, 0 if there was no change, and 1 in case of change
+ */
+int phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret;
+
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ ret = __phy_modify_changed(phydev, regnum, mask, set);
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_modify_changed);
+
+/**
+ * __phy_modify - Convenience function for modifying a PHY register
+ * @phydev: the phy_device struct
+ * @regnum: register number to modify
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
+{
+ int ret;
+
+ ret = __phy_modify_changed(phydev, regnum, mask, set);
return ret < 0 ? ret : 0;
}
@@ -578,7 +631,7 @@ int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
EXPORT_SYMBOL_GPL(phy_modify);
/**
- * __phy_modify_mmd - Convenience function for modifying a register on MMD
+ * __phy_modify_mmd_changed - Function for modifying a register on MMD
* @phydev: the phy_device struct
* @devad: the MMD containing register to modify
* @regnum: register number to modify
@@ -587,17 +640,73 @@ EXPORT_SYMBOL_GPL(phy_modify);
*
* Unlocked helper function which allows a MMD register to be modified as
* new register value = (old register value & ~mask) | set
+ *
+ * Returns negative errno, 0 if there was no change, and 1 in case of change
*/
-int __phy_modify_mmd(struct phy_device *phydev, int devad, u32 regnum,
- u16 mask, u16 set)
+int __phy_modify_mmd_changed(struct phy_device *phydev, int devad, u32 regnum,
+ u16 mask, u16 set)
{
- int ret;
+ int new, ret;
ret = __phy_read_mmd(phydev, devad, regnum);
if (ret < 0)
return ret;
- ret = __phy_write_mmd(phydev, devad, regnum, (ret & ~mask) | set);
+ new = (ret & ~mask) | set;
+ if (new == ret)
+ return 0;
+
+ ret = __phy_write_mmd(phydev, devad, regnum, new);
+
+ return ret < 0 ? ret : 1;
+}
+EXPORT_SYMBOL_GPL(__phy_modify_mmd_changed);
+
+/**
+ * phy_modify_mmd_changed - Function for modifying a register on MMD
+ * @phydev: the phy_device struct
+ * @devad: the MMD containing register to modify
+ * @regnum: register number to modify
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ *
+ * Returns negative errno, 0 if there was no change, and 1 in case of change
+ */
+int phy_modify_mmd_changed(struct phy_device *phydev, int devad, u32 regnum,
+ u16 mask, u16 set)
+{
+ int ret;
+
+ mutex_lock(&phydev->mdio.bus->mdio_lock);
+ ret = __phy_modify_mmd_changed(phydev, devad, regnum, mask, set);
+ mutex_unlock(&phydev->mdio.bus->mdio_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_modify_mmd_changed);
+
+/**
+ * __phy_modify_mmd - Convenience function for modifying a register on MMD
+ * @phydev: the phy_device struct
+ * @devad: the MMD containing register to modify
+ * @regnum: register number to modify
+ * @mask: bit mask of bits to clear
+ * @set: new value of bits set in mask to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int __phy_modify_mmd(struct phy_device *phydev, int devad, u32 regnum,
+ u16 mask, u16 set)
+{
+ int ret;
+
+ ret = __phy_modify_mmd_changed(phydev, devad, regnum, mask, set);
return ret < 0 ? ret : 0;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d2ffae992..378da9a61 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -799,13 +799,21 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val);
*/
int __phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val);
+int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
+ u16 set);
+int phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
+ u16 set);
int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
int phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set);
+int __phy_modify_mmd_changed(struct phy_device *phydev, int devad, u32 regnum,
+ u16 mask, u16 set);
+int phy_modify_mmd_changed(struct phy_device *phydev, int devad, u32 regnum,
+ u16 mask, u16 set);
int __phy_modify_mmd(struct phy_device *phydev, int devad, u32 regnum,
- u16 mask, u16 set);
+ u16 mask, u16 set);
int phy_modify_mmd(struct phy_device *phydev, int devad, u32 regnum,
- u16 mask, u16 set);
+ u16 mask, u16 set);
/**
* __phy_set_bits - Convenience function for setting bits in a PHY register
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/3] net: phy: marvell10g: fix usage of new MMD modifying helpers
From: Heiner Kallweit @ 2019-02-10 18:58 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>
When replacing mv3310_modify() with phy_modify_mmd() we missed that
they behave differently, mv3310_modify() returns 1 on a changed
register value whilst phy_modify_mmd() returns 0. Fix this by replacing
phy_modify_mmd() with phy_modify_mmd_changed() where needed.
Fixes: b52c018ddccf ("net: phy: make use of new MMD accessors")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/marvell10g.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 96a79c6c7..08362dc65 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -141,10 +141,9 @@ static int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
return ret;
val = enable ? MV_V2_TEMP_CTRL_SAMPLE : MV_V2_TEMP_CTRL_DISABLE;
- ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP_CTRL,
- MV_V2_TEMP_CTRL_MASK, val);
- return ret < 0 ? ret : 0;
+ return phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP_CTRL,
+ MV_V2_TEMP_CTRL_MASK, val);
}
static void mv3310_hwmon_disable(void *data)
@@ -345,7 +344,7 @@ static int mv3310_config_aneg(struct phy_device *phydev)
linkmode_and(phydev->advertising, phydev->advertising,
phydev->supported);
- ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
ADVERTISE_ALL | ADVERTISE_100BASE4 |
ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
linkmode_adv_to_mii_adv_t(phydev->advertising));
@@ -355,7 +354,7 @@ static int mv3310_config_aneg(struct phy_device *phydev)
changed = true;
reg = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
- ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MV_AN_CTRL1000,
ADVERTISE_1000FULL | ADVERTISE_1000HALF, reg);
if (ret < 0)
return ret;
@@ -369,8 +368,8 @@ static int mv3310_config_aneg(struct phy_device *phydev)
else
reg = 0;
- ret = phy_modify_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
- MDIO_AN_10GBT_CTRL_ADV10G, reg);
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
+ MDIO_AN_10GBT_CTRL_ADV10G, reg);
if (ret < 0)
return ret;
if (ret > 0)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Heiner Kallweit @ 2019-02-10 18:59 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>
Use phy_modify_changed() to simplify the code.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 43 +++++++++++++-----------------------
1 file changed, 15 insertions(+), 28 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8573d17ec..3d14e48ae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1516,7 +1516,7 @@ EXPORT_SYMBOL(phy_reset_after_clk_enable);
static int genphy_config_advert(struct phy_device *phydev)
{
u32 advertise;
- int oldadv, adv, bmsr;
+ int bmsr, adv;
int err, changed = 0;
/* Only allow advertising what this PHY supports */
@@ -1529,22 +1529,14 @@ static int genphy_config_advert(struct phy_device *phydev)
phydev->advertising);
/* Setup standard advertisement */
- adv = phy_read(phydev, MII_ADVERTISE);
- if (adv < 0)
- return adv;
-
- oldadv = adv;
- adv &= ~(ADVERTISE_ALL | ADVERTISE_100BASE4 | ADVERTISE_PAUSE_CAP |
- ADVERTISE_PAUSE_ASYM);
- adv |= ethtool_adv_to_mii_adv_t(advertise);
-
- if (adv != oldadv) {
- err = phy_write(phydev, MII_ADVERTISE, adv);
-
- if (err < 0)
- return err;
+ err = phy_modify_changed(phydev, MII_ADVERTISE,
+ ADVERTISE_ALL | ADVERTISE_100BASE4 |
+ ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
+ ethtool_adv_to_mii_adv_t(advertise));
+ if (err < 0)
+ return err;
+ if (err > 0)
changed = 1;
- }
bmsr = phy_read(phydev, MII_BMSR);
if (bmsr < 0)
@@ -1558,25 +1550,20 @@ static int genphy_config_advert(struct phy_device *phydev)
return changed;
/* Configure gigabit if it's supported */
- adv = phy_read(phydev, MII_CTRL1000);
- if (adv < 0)
- return adv;
-
- oldadv = adv;
- adv &= ~(ADVERTISE_1000FULL | ADVERTISE_1000HALF);
-
+ adv = 0;
if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
phydev->supported) ||
linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
phydev->supported))
- adv |= ethtool_adv_to_mii_ctrl1000_t(advertise);
+ adv = ethtool_adv_to_mii_ctrl1000_t(advertise);
- if (adv != oldadv)
- changed = 1;
-
- err = phy_write(phydev, MII_CTRL1000, adv);
+ err = phy_modify_changed(phydev, MII_CTRL1000,
+ ADVERTISE_1000FULL | ADVERTISE_1000HALF,
+ adv);
if (err < 0)
return err;
+ if (err > 0)
+ changed = 1;
return changed;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get()
From: Ido Schimmel @ 2019-02-10 19:05 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, bridge@lists.linux-foundation.org,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190210175105.31629-7-f.fainelli@gmail.com>
On Sun, Feb 10, 2019 at 09:50:55AM -0800, Florian Fainelli wrote:
> Now that all switchdev drivers have been converted to checking the
> bridge port flags during the prepare phase of the
> switchdev_port_attr_set(), we can move straight to trying to set the
> desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/bridge/br_switchdev.c | 20 +++-----------------
> 1 file changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index db9e8ab96d48..939f300522c5 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -64,29 +64,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
> {
> struct switchdev_attr attr = {
> .orig_dev = p->dev,
> - .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
> + .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
> + .flags = SWITCHDEV_F_DEFER,
How does this work? You defer the operation, so the driver cannot veto
it. This is why we have SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
which is not deferred.
> + .u.brport_flags = flags,
> };
> int err;
>
> if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
> return 0;
>
> - err = switchdev_port_attr_get(p->dev, &attr);
> - if (err == -EOPNOTSUPP)
> - return 0;
> - if (err)
> - return err;
> -
> - /* Check if specific bridge flag attribute offload is supported */
> - if (!(attr.u.brport_flags_support & mask)) {
> - br_warn(p->br, "bridge flag offload is not supported %u(%s)\n",
> - (unsigned int)p->port_no, p->dev->name);
> - return -EOPNOTSUPP;
> - }
> -
> - attr.id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS;
> - attr.flags = SWITCHDEV_F_DEFER;
> - attr.u.brport_flags = flags;
> err = switchdev_port_attr_set(p->dev, &attr);
> if (err) {
> br_warn(p->br, "error setting offload flag on port %u(%s)\n",
> --
> 2.19.1
>
^ permalink raw reply
* Re: [PATCH net-next v2 06/16] net: bridge: Stop calling switchdev_port_attr_get()
From: Florian Fainelli @ 2019-02-10 19:34 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
devel@driverdev.osuosl.org, bridge@lists.linux-foundation.org,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190210190501.GA26726@splinter>
Le 2/10/19 à 11:05 AM, Ido Schimmel a écrit :
> On Sun, Feb 10, 2019 at 09:50:55AM -0800, Florian Fainelli wrote:
>> Now that all switchdev drivers have been converted to checking the
>> bridge port flags during the prepare phase of the
>> switchdev_port_attr_set(), we can move straight to trying to set the
>> desired flag through SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS.
>>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> net/bridge/br_switchdev.c | 20 +++-----------------
>> 1 file changed, 3 insertions(+), 17 deletions(-)
>>
>> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
>> index db9e8ab96d48..939f300522c5 100644
>> --- a/net/bridge/br_switchdev.c
>> +++ b/net/bridge/br_switchdev.c
>> @@ -64,29 +64,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>> {
>> struct switchdev_attr attr = {
>> .orig_dev = p->dev,
>> - .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT,
>> + .id = SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS,
>> + .flags = SWITCHDEV_F_DEFER,
>
> How does this work? You defer the operation, so the driver cannot veto
> it. This is why we have SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
> which is not deferred.
I missed that indeed, how would you feel about splitting the attribute
setting into different phases:
- checking that the attribute setting is supported (caller context, so
possibly atomic)
- allocating and committing resources (deferred context)
For pretty much any DSA driver, we will have to be in sleepable context
anyway because of MDIO, SPI, I2C, whatever transport layer.
Not sure how to best approach this now...
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: phy: add register modifying helpers returning 1 on change
From: Andrew Lunn @ 2019-02-10 19:43 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <f454fd0e-199b-992e-f47f-6118101ac6ba@gmail.com>
On Sun, Feb 10, 2019 at 07:57:56PM +0100, Heiner Kallweit wrote:
> When modifying registers there are scenarios where we need to know
> whether the register content actually changed. This patch adds
> new helpers to not break users of the current ones, phy_modify() etc.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: phy: marvell10g: fix usage of new MMD modifying helpers
From: Andrew Lunn @ 2019-02-10 19:44 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <f6705472-58c2-5bad-be0a-b44518a4e72d@gmail.com>
On Sun, Feb 10, 2019 at 07:58:49PM +0100, Heiner Kallweit wrote:
> When replacing mv3310_modify() with phy_modify_mmd() we missed that
> they behave differently, mv3310_modify() returns 1 on a changed
> register value whilst phy_modify_mmd() returns 0. Fix this by replacing
> phy_modify_mmd() with phy_modify_mmd_changed() where needed.
>
> Fixes: b52c018ddccf ("net: phy: make use of new MMD accessors")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Sorry, my bad.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Andrew Lunn @ 2019-02-10 19:46 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <affccf1b-9c2f-2c5a-df8f-43f11ca8ccdc@gmail.com>
On Sun, Feb 10, 2019 at 07:59:57PM +0100, Heiner Kallweit wrote:
> Use phy_modify_changed() to simplify the code.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* linux-next: Fixes tag needs some work in the net tree
From: Stephen Rothwell @ 2019-02-10 20:14 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List, Ursula Braun
[-- Attachment #1: Type: text/plain, Size: 286 bytes --]
Hi all,
In commit
ccc8ca9b90ac ("net/smc: fix byte_order for rx_curs_confirmed")
Fixes tag
Fixes: b8649efad879 ("net/smc: fix sender_free computation") (net-tree)
has these problem(s):
- The trailing '(net-tree)' is unexpected
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] sctp: make sctp_setsockopt_events() less strict about the option length
From: Marcelo Ricardo Leitner @ 2019-02-10 20:15 UTC (permalink / raw)
To: David Miller
Cc: julien, netdev, linux-sctp, linux-kernel, nhorman, vyasevich,
lucien.xin
In-Reply-To: <20190210124616.GG13621@localhost.localdomain>
On Sun, Feb 10, 2019 at 10:46:16AM -0200, Marcelo Ricardo Leitner wrote:
> On Sat, Feb 09, 2019 at 03:12:17PM -0800, David Miller wrote:
> > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > Date: Wed, 6 Feb 2019 18:37:54 -0200
> >
> > > On Wed, Feb 06, 2019 at 12:14:30PM -0800, Julien Gomes wrote:
> > >> Make sctp_setsockopt_events() able to accept sctp_event_subscribe
> > >> structures longer than the current definitions.
> > >>
> > >> This should prevent unjustified setsockopt() failures due to struct
> > >> sctp_event_subscribe extensions (as in 4.11 and 4.12) when using
> > >> binaries that should be compatible, but were built with later kernel
> > >> uapi headers.
> > >
> > > Not sure if we support backwards compatibility like this?
> >
> > What a complete mess we have here.
> >
> > Use new socket option numbers next time, do not change the size and/or
> > layout of existing socket options.
>
> What about reusing the same socket option, but defining a new struct?
> Say, MYSOCKOPT supports struct mysockopt, struct mysockopt2, struct
> mysockopt3...
>
> That way we have a clear definition of the user's intent.
>
> >
> > This whole thread, if you read it, is basically "if we compatability
> > this way, that breaks, and if we do compatability this other way oh
> > shit this other thing doesn't work."
> >
> > I think we really need to specifically check for the difference sizes
> > that existed one by one, clear out the part not given by the user, and
> > backport this as far back as possible in a way that in the older kernels
> > we see if the user is actually trying to use the new features and if so
> > error out.
>
> I'm afraid clearing out may not be enough, though seems it's the best
> we can do so far. If the struct is allocated but not fully initialized
> via a memset, but by setting its fields one by one, the remaining new
> fields will be left uninitinialized.
Need to clarify the "clearing out", I think it was meant differently.
It was more about on how to ensure that the 16-bytes long of the v3
supplied to a v1-only kernel is compatible with the 12-bytes long v1.
The kernel would have to check the trailing 4 bytes after v1-size and
make sure they are all zeroed in order for the old kernel to accept it
as a v1. But, as I said above, there are situations that this will not
be enough.
>
> >
> > Which, btw, is terrible behavior. Newly compiled apps should work on
> > older kernels if they don't try to use the new features, and if they
>
> One use case here is: a given distro is using kernel X and app Foo is
> built against it. Then upgrades to X+1, Foo is patched to fix an issue
> and is rebuilt against X+1. The user upgrades Foo package but for
> whatever reason, doesn't upgrade kernel or reboot the system. Here,
> Foo doesn't work anymore until the new kernel is also running.
>
> > can the ones that want to try to use the new features should be able
> > to fall back when that feature isn't available in a non-ambiguous
> > and precisely defined way.
> >
> > The fact that the use of the new feature is hidden in the new
> > structure elements is really rotten.
> >
> > This patch, at best, needs some work and definitely a longer and more
> > detailed commit message.
>
We have issues on read path too. 52ccb8e90c0a ("[SCTP]: Update
SCTP_PEER_ADDR_PARAMS socket option to the latest api draft.")
extended struct sctp_paddrparams and its getsockopt goes with:
sctp_getsockopt_peer_addr_params()
...
if (len < sizeof(struct sctp_paddrparams))
return -EINVAL;
len = sizeof(struct sctp_paddrparams);
By then, we didn't have the /uapi/ folder yet. There may other cases.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: phy: add register modifying helpers returning 1 on change
From: Florian Fainelli @ 2019-02-10 20:27 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <f454fd0e-199b-992e-f47f-6118101ac6ba@gmail.com>
Le 2/10/19 à 10:57 AM, Heiner Kallweit a écrit :
> When modifying registers there are scenarios where we need to know
> whether the register content actually changed. This patch adds
> new helpers to not break users of the current ones, phy_modify() etc.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: phy: marvell10g: fix usage of new MMD modifying helpers
From: Florian Fainelli @ 2019-02-10 20:27 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <f6705472-58c2-5bad-be0a-b44518a4e72d@gmail.com>
Le 2/10/19 à 10:58 AM, Heiner Kallweit a écrit :
> When replacing mv3310_modify() with phy_modify_mmd() we missed that
> they behave differently, mv3310_modify() returns 1 on a changed
> register value whilst phy_modify_mmd() returns 0. Fix this by replacing
> phy_modify_mmd() with phy_modify_mmd_changed() where needed.
>
> Fixes: b52c018ddccf ("net: phy: make use of new MMD accessors")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: use phy_modify_changed in genphy_config_advert
From: Florian Fainelli @ 2019-02-10 20:28 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <affccf1b-9c2f-2c5a-df8f-43f11ca8ccdc@gmail.com>
Le 2/10/19 à 10:59 AM, Heiner Kallweit a écrit :
> Use phy_modify_changed() to simplify the code.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH for-next 2/4] devlink: fix print of uint64_t
From: Stephen Hemminger @ 2019-02-10 20:34 UTC (permalink / raw)
To: Aya Levin
Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-3-git-send-email-ayal@mellanox.com>
On Sun, 10 Feb 2019 20:28:47 +0200
Aya Levin <ayal@mellanox.com> wrote:
> 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>
> Reported-by: Maria Pasechnik <mariap@mellanox.com>
> ---
> devlink/devlink.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index a05755385a49..46e2e41c5dfd 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1628,7 +1628,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)
More conditional code adds bloat and is a nuisance.
Why not use print_u64 that already exists.
I wish devlink used json_print in a more standard manner.
^ permalink raw reply
* Re: [PATCH for-next 3/4] devlink: fix boolean JSON print
From: Stephen Hemminger @ 2019-02-10 20:34 UTC (permalink / raw)
To: Aya Levin
Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-4-git-send-email-ayal@mellanox.com>
On Sun, 10 Feb 2019 20:28:48 +0200
Aya Levin <ayal@mellanox.com> wrote:
> 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>
> ---
> devlink/devlink.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/devlink/devlink.c b/devlink/devlink.c
> index 46e2e41c5dfd..a433883f1b2b 100644
> --- a/devlink/devlink.c
> +++ b/devlink/devlink.c
> @@ -1605,10 +1605,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)
There is already print_bool in json_print why is that not used?
^ permalink raw reply
* Re: [PATCH for-next 4/4] devlink: add health command support
From: Stephen Hemminger @ 2019-02-10 20:42 UTC (permalink / raw)
To: Aya Levin
Cc: David Ahern, netdev, David S. Miller, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha, Tal Alon, Ariel Almog
In-Reply-To: <1549823329-10377-5-git-send-email-ayal@mellanox.com>
On Sun, 10 Feb 2019 20:28:49 +0200
Aya Levin <ayal@mellanox.com> wrote:
> +
> +static void cmd_health_help(void)
> +{
> + pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
> + pr_err("Usage: devlink health recover DEV reporter REPORTER_NAME\n");
> + pr_err("Usage: devlink health diagnose DEV reporter REPORTER_NAME\n");
> + pr_err("Usage: devlink health dump show DEV reporter REPORTER_NAME\n");
> + pr_err("Usage: devlink health dump clear DEV reporter REPORTER_NAME\n");
> + pr_err("Usage: devlink health set DEV reporter REPORTER_NAME NAME VALUE\n");
> +}
> +
Minor nit:
I prefer that all code and outputs in iproute2 look the same for ease
of maintenance and constituency of user experience.
Why does devlink not use:
static void cmd_health_help(void)
{
fprintf(stderr, "Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
fprintf(stderr, " devlink health recover DEV reporter REPORTER_NAME\n");
fprintf(stderr, " devlink health diagnose DEV reporter REPORTER_NAME\n");
...
Or
fprintf(stderr, "Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n"
" devlink health recover DEV reporter REPORTER_NAME\n"
" devlink health diagnose DEV reporter REPORTER_NAME\n");
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] net: Remove switchdev_ops
From: Florian Fainelli @ 2019-02-10 20:44 UTC (permalink / raw)
To: netdev; +Cc: idosch, linux-kernel, devel, bridge, jiri, andrew, vivien.didelot
In-Reply-To: <20190210175105.31629-1-f.fainelli@gmail.com>
Le 2/10/19 à 9:50 AM, Florian Fainelli a écrit :
> Hi all,
>
> This patch series finishes by the removal of switchdev_ops. To get there
> we need to do a few things:
>
> - get rid of the one and only call to switchdev_port_attr_get() which is
> used to fetch the device's bridge port flags capabilities, instead we
> can just check what flags are being programmed during the prepare
> phase
>
> - once we get rid of getting switchdev port attributes we convert the
> setting of such attributes using a blocking notifier
>
> And then remove switchdev_ops completely.
>
> Please review and let me know what you think!
I am going to submit a v3 which moves the port_attr_{get,set} to a
switchdev notifier, but does not yet get rid of
switchdev_port_attr_get() entirely since there is not a clear path yet
to split the setting between non-sleeping and sleeping context.
>
> Changes in v2:
> - fixed bisectability issues in patch #15
> - added Acked-by from Jiri where necessary
> - fixed a few minor issues according to Jiri's feedback:
> - rename port_attr_event -> port_attr_set_event
> - moved SWITCHDEV_PORT_ATTR_SET closer to other blocking events
>
> Florian Fainelli (16):
> Documentation: networking: switchdev: Update port parent ID section
> mlxsw: spectrum: Check bridge flags during prepare phase
> staging: fsl-dpaa2: ethsw: Check bridge port flags during set
> net: dsa: Add setter for SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS
> rocker: Check bridge flags during prepare phase
> net: bridge: Stop calling switchdev_port_attr_get()
> net: Remove SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT
> net: Get rid of switchdev_port_attr_get()
> switchdev: Add SWITCHDEV_PORT_ATTR_SET
> rocker: Handle SWITCHDEV_PORT_ATTR_SET
> net: dsa: Handle SWITCHDEV_PORT_ATTR_SET
> mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
> net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_SET
> staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_SET
> net: switchdev: Replace port attr set SDO with a notification
> net: Remove switchdev_ops
>
> Documentation/networking/switchdev.txt | 15 ++-
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 12 ---
> .../net/ethernet/mellanox/mlxsw/spectrum.h | 2 -
> .../mellanox/mlxsw/spectrum_switchdev.c | 69 ++++----------
> drivers/net/ethernet/mscc/ocelot.c | 21 +++-
> drivers/net/ethernet/rocker/rocker_main.c | 95 ++++++++-----------
> drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 48 ++++------
> include/linux/netdevice.h | 3 -
> include/net/switchdev.h | 36 ++-----
> net/bridge/br_switchdev.c | 20 +---
> net/dsa/dsa_priv.h | 3 +
> net/dsa/port.c | 10 ++
> net/dsa/slave.c | 40 ++++----
> net/switchdev/switchdev.c | 92 +++++-------------
> 14 files changed, 170 insertions(+), 296 deletions(-)
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] net: Remove switchdev_ops
From: David Miller @ 2019-02-10 20:51 UTC (permalink / raw)
To: f.fainelli
Cc: netdev, idosch, linux-kernel, devel, bridge, jiri, andrew,
vivien.didelot
In-Reply-To: <746bce08-5b5f-70c9-efb7-33b067929198@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun, 10 Feb 2019 12:44:47 -0800
> I am going to submit a v3 which moves the port_attr_{get,set} to a
> switchdev notifier, but does not yet get rid of
> switchdev_port_attr_get() entirely since there is not a clear path yet
> to split the setting between non-sleeping and sleeping context.
Ok.
^ permalink raw reply
* [PATCH v2 bpf] bpf: fix lockdep false positive in stackmap
From: Alexei Starovoitov @ 2019-02-10 20:52 UTC (permalink / raw)
To: davem; +Cc: daniel, peterz, edumazet, longman, jannh, netdev, kernel-team
Lockdep warns about false positive:
[ 11.211460] ------------[ cut here ]------------
[ 11.211936] DEBUG_LOCKS_WARN_ON(depth <= 0)
[ 11.211985] WARNING: CPU: 0 PID: 141 at ../kernel/locking/lockdep.c:3592 lock_release+0x1ad/0x280
[ 11.213134] Modules linked in:
[ 11.214954] RIP: 0010:lock_release+0x1ad/0x280
[ 11.223508] Call Trace:
[ 11.223705] <IRQ>
[ 11.223874] ? __local_bh_enable+0x7a/0x80
[ 11.224199] up_read+0x1c/0xa0
[ 11.224446] do_up_read+0x12/0x20
[ 11.224713] irq_work_run_list+0x43/0x70
[ 11.225030] irq_work_run+0x26/0x50
[ 11.225310] smp_irq_work_interrupt+0x57/0x1f0
[ 11.225662] irq_work_interrupt+0xf/0x20
since rw_semaphore is released in a different task vs task that locked the sema.
It is expected behavior.
Fix the warning with up_read_non_owner() and rwsem_release() annotation.
Fixes: bae77c5eb5b2 ("bpf: enable stackmap with build_id in nmi context")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/stackmap.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index d43b14535827..950ab2f28922 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -44,7 +44,7 @@ static void do_up_read(struct irq_work *entry)
struct stack_map_irq_work *work;
work = container_of(entry, struct stack_map_irq_work, irq_work);
- up_read(work->sem);
+ up_read_non_owner(work->sem);
work->sem = NULL;
}
@@ -338,6 +338,12 @@ static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
} else {
work->sem = ¤t->mm->mmap_sem;
irq_work_queue(&work->irq_work);
+ /*
+ * The irq_work will release the mmap_sem with
+ * up_read_non_owner(). The rwsem_release() is called
+ * here to release the lock from lockdep's perspective.
+ */
+ rwsem_release(¤t->mm->mmap_sem.dep_map, 1, _RET_IP_);
}
}
--
2.20.0
^ permalink raw reply related
* Re: [PATCH net-next 0/3] net: phy: add and use register modifying helpers returning 1 on change
From: David Miller @ 2019-02-10 20:53 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <a9d88fc1-6ff7-5805-a7d3-7aad7391b4d8@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 10 Feb 2019 19:56:47 +0100
> Add and use register modifying helpers returning 1 on change.
Series applied, thanks.
^ 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