* [PATCH v4 iproute2-next 05/11] devlink: Add devlink health show command
From: Aya Levin @ 2019-02-28 12:12 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-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 c3f874564309..37eb2054ef06 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)
@@ -202,6 +207,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
#define DL_OPT_REGION_LENGTH BIT(24)
#define DL_OPT_FLASH_FILE_NAME BIT(25)
#define DL_OPT_FLASH_COMPONENT BIT(26)
+#define DL_OPT_HEALTH_REPORTER_NAME BIT(27)
struct dl_opts {
uint32_t present; /* flags of present items */
@@ -235,6 +241,7 @@ struct dl_opts {
uint64_t region_length;
const char *flash_file_name;
const char *flash_component;
+ const char *reporter_name;
};
struct dl {
@@ -395,6 +402,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)
@@ -980,6 +994,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,
@@ -1247,6 +1262,13 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
if (err)
return err;
o_found |= DL_OPT_FLASH_COMPONENT;
+ } 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;
@@ -1350,6 +1372,9 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
if (opts->present & DL_OPT_FLASH_COMPONENT)
mnl_attr_put_strz(nlh, DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,
opts->flash_component);
+ 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,
@@ -5790,11 +5815,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");
}
@@ -5827,6 +6007,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 v4 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Aya Levin @ 2019-02-28 12:12 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-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 4b7e8ec2c88a..c3f874564309 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1661,6 +1661,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 v4 iproute2-next 07/11] devlink: Add devlink health diagnose command
From: Aya Levin @ 2019-02-28 12:13 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-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 c77f80c87adc..8490fffdd0a6 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"
@@ -5815,6 +5816,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;
@@ -5972,6 +6155,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)
@@ -5986,6 +6170,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 v4 iproute2-next 01/11] devlink: Refactor validation of finding required arguments
From: Aya Levin @ 2019-02-28 12:12 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-1-git-send-email-ayal@mellanox.com>
Introducing argument's metadata structure matching a bitmap flag per
required argument and an error message if missing. Using this static
array to refactor validation of finding required arguments in devlink
command line and to ease further maintenance.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 153 ++++++++++++++++--------------------------------------
1 file changed, 45 insertions(+), 108 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 5c6cac1f76dd..a774b196b7fd 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -39,6 +39,7 @@
#define PARAM_CMODE_RUNTIME_STR "runtime"
#define PARAM_CMODE_DRIVERINIT_STR "driverinit"
#define PARAM_CMODE_PERMANENT_STR "permanent"
+#define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
static int g_new_line_count;
@@ -954,6 +955,49 @@ static int param_cmode_get(const char *cmodestr,
return 0;
}
+struct dl_args_metadata {
+ uint32_t o_flag;
+ char err_msg[DL_ARGS_REQUIRED_MAX_ERR_LEN];
+};
+
+static const struct dl_args_metadata dl_args_required[] = {
+ {DL_OPT_PORT_TYPE, "Port type not set."},
+ {DL_OPT_PORT_COUNT, "Port split count option expected."},
+ {DL_OPT_SB_POOL, "Pool index option expected."},
+ {DL_OPT_SB_SIZE, "Pool size option expected."},
+ {DL_OPT_SB_TYPE, "Pool type option expected."},
+ {DL_OPT_SB_THTYPE, "Pool threshold type option expected."},
+ {DL_OPT_SB_TH, "Threshold option expected."},
+ {DL_OPT_SB_TC, "TC index option expected."},
+ {DL_OPT_ESWITCH_MODE, "E-Switch mode option expected."},
+ {DL_OPT_ESWITCH_INLINE_MODE, "E-Switch inline-mode option expected."},
+ {DL_OPT_DPIPE_TABLE_NAME, "Dpipe table name expected."},
+ {DL_OPT_DPIPE_TABLE_COUNTERS, "Dpipe table counter state expected."},
+ {DL_OPT_ESWITCH_ENCAP_MODE, "E-Switch encapsulation option expected."},
+ {DL_OPT_PARAM_NAME, "Parameter name expected."},
+ {DL_OPT_PARAM_VALUE, "Value to set expected."},
+ {DL_OPT_PARAM_CMODE, "Configuration mode expected."},
+ {DL_OPT_REGION_SNAPSHOT_ID, "Region snapshot id expected."},
+ {DL_OPT_REGION_ADDRESS, "Region address value expected."},
+ {DL_OPT_REGION_LENGTH, "Region length value expected."},
+};
+
+static int dl_args_finding_required_validate(uint32_t o_required,
+ uint32_t o_found)
+{
+ uint32_t o_flag;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(dl_args_required); i++) {
+ o_flag = dl_args_required[i].o_flag;
+ if ((o_required & o_flag) && !(o_found & o_flag)) {
+ pr_err("%s\n", dl_args_required[i].err_msg);
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
static int dl_argv_parse(struct dl *dl, uint32_t o_required,
uint32_t o_optional)
{
@@ -1216,114 +1260,7 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
opts->present |= DL_OPT_SB;
}
- if ((o_required & DL_OPT_PORT_TYPE) && !(o_found & DL_OPT_PORT_TYPE)) {
- pr_err("Port type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PORT_COUNT) &&
- !(o_found & DL_OPT_PORT_COUNT)) {
- pr_err("Port split count option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_POOL) && !(o_found & DL_OPT_SB_POOL)) {
- pr_err("Pool index option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_SIZE) && !(o_found & DL_OPT_SB_SIZE)) {
- pr_err("Pool size option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TYPE) && !(o_found & DL_OPT_SB_TYPE)) {
- pr_err("Pool type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_THTYPE) && !(o_found & DL_OPT_SB_THTYPE)) {
- pr_err("Pool threshold type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TH) && !(o_found & DL_OPT_SB_TH)) {
- pr_err("Threshold option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TC) && !(o_found & DL_OPT_SB_TC)) {
- pr_err("TC index option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_MODE) &&
- !(o_found & DL_OPT_ESWITCH_MODE)) {
- pr_err("E-Switch mode option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_INLINE_MODE) &&
- !(o_found & DL_OPT_ESWITCH_INLINE_MODE)) {
- pr_err("E-Switch inline-mode option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_DPIPE_TABLE_NAME) &&
- !(o_found & DL_OPT_DPIPE_TABLE_NAME)) {
- pr_err("Dpipe table name expected\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_DPIPE_TABLE_COUNTERS) &&
- !(o_found & DL_OPT_DPIPE_TABLE_COUNTERS)) {
- pr_err("Dpipe table counter state expected\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_ENCAP_MODE) &&
- !(o_found & DL_OPT_ESWITCH_ENCAP_MODE)) {
- pr_err("E-Switch encapsulation option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_NAME) &&
- !(o_found & DL_OPT_PARAM_NAME)) {
- pr_err("Parameter name expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_VALUE) &&
- !(o_found & DL_OPT_PARAM_VALUE)) {
- pr_err("Value to set expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_CMODE) &&
- !(o_found & DL_OPT_PARAM_CMODE)) {
- pr_err("Configuration mode expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_SNAPSHOT_ID) &&
- !(o_found & DL_OPT_REGION_SNAPSHOT_ID)) {
- pr_err("Region snapshot id expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_ADDRESS) &&
- !(o_found & DL_OPT_REGION_ADDRESS)) {
- pr_err("Region address value expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_LENGTH) &&
- !(o_found & DL_OPT_REGION_LENGTH)) {
- pr_err("Region length value expected.\n");
- return -EINVAL;
- }
-
- return 0;
+ return dl_args_finding_required_validate(o_required, o_found);
}
static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
--
2.14.1
^ permalink raw reply related
* [PATCH v4 iproute2-next 08/11] devlink: Add devlink health dump show command
From: Aya Levin @ 2019-02-28 12:13 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-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 8490fffdd0a6..30076d25199b 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5993,6 +5993,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);
@@ -6156,6 +6161,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)
@@ -6173,6 +6179,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 v4 iproute2-next 03/11] devlink: Fix boolean JSON print
From: Aya Levin @ 2019-02-28 12:12 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551355984-15752-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 8a28b5d5c26f..4b7e8ec2c88a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1628,10 +1628,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
* Re: [PATCH RFC v3 4/5] sctp: Make sctp_enqueue_event tak an skb list.
From: Neil Horman @ 2019-02-28 12:23 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: David Miller, netdev, lucien.xin
In-Reply-To: <20190228021958.GA13343@localhost.localdomain>
On Wed, Feb 27, 2019 at 11:19:58PM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Feb 27, 2019 at 05:00:24PM -0800, David Miller wrote:
> >
> > Pass this, instead of an event. Then everything trickles down and we
> > always have events a non-empty list.
> >
> > Then we needs a list creating stub to place into .enqueue_event for sctp_stream_interleave_1.
> >
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> > net/sctp/stream_interleave.c | 44 +++++++++++++++++++++++++++---------
> > 1 file changed, 33 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> > index b6b251b8b3cf..0bc3d9329d9a 100644
> > --- a/net/sctp/stream_interleave.c
> > +++ b/net/sctp/stream_interleave.c
> ...
> > @@ -866,11 +867,15 @@ static int sctp_ulpevent_idata(struct sctp_ulpq *ulpq,
>
> More context:
> if (!(event->msg_flags & SCTP_DATA_UNORDERED)) {
> event = sctp_intl_reasm(ulpq, event); [1]
> if (event && event->msg_flags & MSG_EOR) { [2]
> skb_queue_head_init(&temp);
> __skb_queue_tail(&temp, sctp_event2skb(event));
>
> event = sctp_intl_order(ulpq, event);
> > }
> > } else {
> > event = sctp_intl_reasm_uo(ulpq, event);
> > + if (event) {
> > + skb_queue_head_init(&temp);
> > + __skb_queue_tail(&temp, sctp_event2skb(event));
> > + }
> > }
> >
> > if (event) {
> > event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
> > - sctp_enqueue_event(ulpq, event);
> > + sctp_enqueue_event(ulpq, &temp);
>
> [1] can return an event without MSG_EOR (a partial delivery), which
> would skip the condition on [2] and cause temp to not be initialized
> by here. Same applies to sctp_ulpq_tail_data().
>
I agree, it seems we canjust drop the msg_flags check and just key off of event
being non-null, no?
Neil
> It's the only thing I noticed on the series. Will test it tomorrow.
>
> > }
> >
> > return event_eor;
> ...
>
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Masami Hiramatsu @ 2019-02-28 12:29 UTC (permalink / raw)
To: Joel Fernandes
Cc: Alexei Starovoitov, Linus Torvalds, Masami Hiramatsu,
Steven Rostedt, Andy Lutomirski, will.deacon,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski, daniel,
netdev, bpf
In-Reply-To: <20190226152447.GA99670@google.com>
On Tue, 26 Feb 2019 10:24:47 -0500
Joel Fernandes <joel@joelfernandes.org> wrote:
> On Fri, Feb 22, 2019 at 11:27:05AM -0800, Alexei Starovoitov wrote:
> > On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> > >
> > > Then we should still probably fix up "__probe_kernel_read()" to not
> > > allow user accesses. The easiest way to do that is actually likely to
> > > use the "unsafe_get_user()" functions *without* doing a
> > > uaccess_begin(), which will mean that modern CPU's will simply fault
> > > on a kernel access to user space.
> >
> > On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> > and users pass both user and kernel addresses into it and expect
> > that the helper will actually try to read from that address.
>
> Slightly related and FWIW, BCC's eBPF-based opensnoop tool [1] installs a
> kprobe on do_sys_open to monitor calls to the open syscall globally.
>
> do_sys_open() has prototype:
>
> long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode);
>
> This causes a "blank" filename to be displayed by opensnoop when I run it on
> my Pixel 3 (arm64), possibly because this is a user pointer. However, it
> works fine on x86-64.
>
> So it seems to me that on arm64, reading user pointers directly still doesn't
> work even if there is a distinction between user/kernel addresses. In that
> case reading the user pointer using user accessors (possibly using
> bpf_probe_user_read helper) should be needed to fix this issue (as Yonghong
> also privately discussed with me).
OK, it sounds like the same issue. Please add a bpf_user_read() and use it
for __user pointer.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] xen-netback: fix occasional leak of grant ref mappings under memory pressure
From: Wei Liu @ 2019-02-28 12:37 UTC (permalink / raw)
To: Paul Durrant
Cc: Wei Liu, Igor Druzhinin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org,
davem@davemloft.net
In-Reply-To: <0aa1850892c94d6ba7543bf322734614@AMSPEX02CL02.citrite.net>
On Thu, Feb 28, 2019 at 12:07:07PM +0000, Paul Durrant wrote:
> Yes, I meant kfree_skb(nskb).
>
In that case I think your patch looks fine.
Wei.
^ permalink raw reply
* [PATCH net-next] net: sched: pie: avoid slow division in drop probability decay
From: Leslie Monis @ 2019-02-28 12:36 UTC (permalink / raw)
To: davem; +Cc: netdev, tahiliani, dave.taht, Leslie Monis
As per RFC 8033, it is sufficient for the drop probability
decay factor to have a value of (1 - 1/64) instead of 98%.
This avoids the need to do slow division.
Suggested-by: David Laight <David.Laight@aculab.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
net/sched/sch_pie.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index f93cfe034c72..1cc0c7b74aa3 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -429,7 +429,8 @@ static void calculate_probability(struct Qdisc *sch)
*/
if (qdelay == 0 && qdelay_old == 0 && update_prob)
- q->vars.prob = 98 * div_u64(q->vars.prob, 100);
+ /* Reduce drop probability to 98.4% */
+ q->vars.prob -= q->vars.prob / 64u;
q->vars.qdelay = qdelay;
q->vars.qlen_old = qlen;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2] appletalk: Fix use-after-free in atalk_proc_exit
From: kbuild test robot @ 2019-02-28 12:38 UTC (permalink / raw)
To: Yue Haibing
Cc: kbuild-all, davem, joe, gregkh, linux-kernel, netdev, YueHaibing
In-Reply-To: <20190228023033.26248-1-yuehaibing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
Hi YueHaibing,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on v5.0-rc8 next-20190228]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yue-Haibing/appletalk-Fix-use-after-free-in-atalk_proc_exit/20190228-195802
config: i386-randconfig-x003-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
>> WARNING: net/appletalk/appletalk.o(.init.text+0x104): Section mismatch in reference from the function init_module() to the function .exit.text:atalk_proc_exit()
The function __init init_module() references
a function __exit atalk_proc_exit().
This is often seen when error handling in the init function
uses functionality in the exit path.
The fix is often to remove the __exit annotation of
atalk_proc_exit() so it may be used outside an exit section.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28071 bytes --]
^ permalink raw reply
* [PATCH v2] xen-netback: fix occasional leak of grant ref mappings under memory pressure
From: Igor Druzhinin @ 2019-02-28 12:48 UTC (permalink / raw)
To: xen-devel, netdev, linux-kernel
Cc: wei.liu2, paul.durrant, davem, Igor Druzhinin
Zero-copy callback flag is not yet set on frag list skb at the moment
xenvif_handle_frag_list() returns -ENOMEM. This eventually results in
leaking grant ref mappings since xenvif_zerocopy_callback() is never
called for these fragments. Those eventually build up and cause Xen
to kill Dom0 as the slots get reused for new mappings:
"d0v0 Attempt to implicitly unmap a granted PTE c010000329fce005"
That behavior is observed under certain workloads where sudden spikes
of page cache writes coexist with active atomic skb allocations from
network traffic. Additionally, rework the logic to deal with frag_list
deallocation in a single place.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
drivers/net/xen-netback/netback.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 80aae3a..f09948b 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1072,11 +1072,6 @@ static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *s
skb_frag_size_set(&frags[i], len);
}
- /* Copied all the bits from the frag list -- free it. */
- skb_frag_list_init(skb);
- xenvif_skb_zerocopy_prepare(queue, nskb);
- kfree_skb(nskb);
-
/* Release all the original (foreign) frags. */
for (f = 0; f < skb_shinfo(skb)->nr_frags; f++)
skb_frag_unref(skb, f);
@@ -1145,6 +1140,8 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
xenvif_fill_frags(queue, skb);
if (unlikely(skb_has_frag_list(skb))) {
+ struct sk_buff *nskb = skb_shinfo(skb)->frag_list;
+ xenvif_skb_zerocopy_prepare(queue, nskb);
if (xenvif_handle_frag_list(queue, skb)) {
if (net_ratelimit())
netdev_err(queue->vif->dev,
@@ -1153,6 +1150,9 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
kfree_skb(skb);
continue;
}
+ /* Copied all the bits from the frag list -- free it. */
+ skb_frag_list_init(skb);
+ kfree_skb(nskb);
}
skb->dev = queue->vif->dev;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] xen-netback: fix occasional leak of grant ref mappings under memory pressure
From: Wei Liu @ 2019-02-28 12:52 UTC (permalink / raw)
To: Igor Druzhinin
Cc: xen-devel, netdev, linux-kernel, wei.liu2, paul.durrant, davem
In-Reply-To: <1551358083-14227-1-git-send-email-igor.druzhinin@citrix.com>
On Thu, Feb 28, 2019 at 12:48:03PM +0000, Igor Druzhinin wrote:
> Zero-copy callback flag is not yet set on frag list skb at the moment
> xenvif_handle_frag_list() returns -ENOMEM. This eventually results in
> leaking grant ref mappings since xenvif_zerocopy_callback() is never
> called for these fragments. Those eventually build up and cause Xen
> to kill Dom0 as the slots get reused for new mappings:
>
> "d0v0 Attempt to implicitly unmap a granted PTE c010000329fce005"
>
> That behavior is observed under certain workloads where sudden spikes
> of page cache writes coexist with active atomic skb allocations from
> network traffic. Additionally, rework the logic to deal with frag_list
> deallocation in a single place.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 80aae3a..f09948b 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1072,11 +1072,6 @@ static int xenvif_handle_frag_list(struct xenvif_queue *queue, struct sk_buff *s
> skb_frag_size_set(&frags[i], len);
> }
>
> - /* Copied all the bits from the frag list -- free it. */
> - skb_frag_list_init(skb);
> - xenvif_skb_zerocopy_prepare(queue, nskb);
> - kfree_skb(nskb);
> -
> /* Release all the original (foreign) frags. */
> for (f = 0; f < skb_shinfo(skb)->nr_frags; f++)
> skb_frag_unref(skb, f);
> @@ -1145,6 +1140,8 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
> xenvif_fill_frags(queue, skb);
>
> if (unlikely(skb_has_frag_list(skb))) {
> + struct sk_buff *nskb = skb_shinfo(skb)->frag_list;
> + xenvif_skb_zerocopy_prepare(queue, nskb);
> if (xenvif_handle_frag_list(queue, skb)) {
> if (net_ratelimit())
> netdev_err(queue->vif->dev,
> @@ -1153,6 +1150,9 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
> kfree_skb(skb);
> continue;
> }
> + /* Copied all the bits from the frag list -- free it. */
> + skb_frag_list_init(skb);
> + kfree_skb(nskb);
> }
>
> skb->dev = queue->vif->dev;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH RFC v3 4/5] sctp: Make sctp_enqueue_event tak an skb list.
From: Marcelo Ricardo Leitner @ 2019-02-28 13:17 UTC (permalink / raw)
To: Neil Horman; +Cc: David Miller, netdev, lucien.xin
In-Reply-To: <20190228122356.GA26023@hmswarspite.think-freely.org>
On Thu, Feb 28, 2019 at 07:23:56AM -0500, Neil Horman wrote:
> On Wed, Feb 27, 2019 at 11:19:58PM -0300, Marcelo Ricardo Leitner wrote:
> > On Wed, Feb 27, 2019 at 05:00:24PM -0800, David Miller wrote:
> > >
> > > Pass this, instead of an event. Then everything trickles down and we
> > > always have events a non-empty list.
> > >
> > > Then we needs a list creating stub to place into .enqueue_event for sctp_stream_interleave_1.
> > >
> > > Signed-off-by: David S. Miller <davem@davemloft.net>
> > > ---
> > > net/sctp/stream_interleave.c | 44 +++++++++++++++++++++++++++---------
> > > 1 file changed, 33 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> > > index b6b251b8b3cf..0bc3d9329d9a 100644
> > > --- a/net/sctp/stream_interleave.c
> > > +++ b/net/sctp/stream_interleave.c
> > ...
> > > @@ -866,11 +867,15 @@ static int sctp_ulpevent_idata(struct sctp_ulpq *ulpq,
> >
> > More context:
> > if (!(event->msg_flags & SCTP_DATA_UNORDERED)) {
> > event = sctp_intl_reasm(ulpq, event); [1]
> > if (event && event->msg_flags & MSG_EOR) { [2]
> > skb_queue_head_init(&temp);
> > __skb_queue_tail(&temp, sctp_event2skb(event));
> >
> > event = sctp_intl_order(ulpq, event);
> > > }
> > > } else {
> > > event = sctp_intl_reasm_uo(ulpq, event);
> > > + if (event) {
> > > + skb_queue_head_init(&temp);
> > > + __skb_queue_tail(&temp, sctp_event2skb(event));
> > > + }
> > > }
> > >
> > > if (event) {
> > > event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
> > > - sctp_enqueue_event(ulpq, event);
> > > + sctp_enqueue_event(ulpq, &temp);
> >
> > [1] can return an event without MSG_EOR (a partial delivery), which
> > would skip the condition on [2] and cause temp to not be initialized
> > by here. Same applies to sctp_ulpq_tail_data().
> >
> I agree, it seems we canjust drop the msg_flags check and just key off of event
> being non-null, no?
Yes. Like:
if (!(event->msg_flags & SCTP_DATA_UNORDERED)) {
event = sctp_intl_reasm(ulpq, event);
if (event) {
skb_queue_head_init(&temp);
__skb_queue_tail(&temp, sctp_event2skb(event));
if (event->msg_flags & MSG_EOR)
event = sctp_intl_order(ulpq, event);
}
Or some other clever way to handle it and reduce amount of the 'if
(event)' checks that we have in there. Maybe 'if (!event) goto out;'
helps.
Marcelo
>
> Neil
>
> > It's the only thing I noticed on the series. Will test it tomorrow.
> >
> > > }
> > >
> > > return event_eor;
> > ...
> >
^ permalink raw reply
* [PATCH net-next 2 0/3]
From: Toke Høiland-Jørgensen @ 2019-02-28 13:23 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
---
Toke Høiland-Jørgensen (3):
xdp: Refactor devmap code in preparation for subsequent additions
xdp: Always use a devmap for XDP_REDIRECT to a device
xdp: Add devmap_idx map type for looking up devices by ifindex
include/linux/bpf.h | 46 ++
include/linux/bpf_types.h | 1
include/linux/filter.h | 2
include/net/net_namespace.h | 2
include/net/netns/xdp.h | 11 +
include/trace/events/xdp.h | 3
include/uapi/linux/bpf.h | 1
kernel/bpf/devmap.c | 609 +++++++++++++++++++++++++++----
kernel/bpf/syscall.c | 27 +
kernel/bpf/verifier.c | 14 +
net/core/dev.c | 59 +++
net/core/filter.c | 69 +---
tools/bpf/bpftool/map.c | 1
tools/include/uapi/linux/bpf.h | 1
tools/lib/bpf/libbpf_probes.c | 1
tools/testing/selftests/bpf/test_maps.c | 16 +
16 files changed, 720 insertions(+), 143 deletions(-)
^ permalink raw reply
* [PATCH net-next 2 3/3] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-28 13:23 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136022499.3174.12765993257926981358.stgit@alrua-x1>
A common pattern when using xdp_redirect_map() is to create a device map
where the lookup key is simply ifindex. Because device maps are arrays,
this leaves holes in the map, and the map has to be sized to fit the
largest ifindex, regardless of how many devices actually are actually
needed in the map.
This patch adds a second type of device map where the key is interpreted as
an ifindex and looked up using a hashmap, instead of being used as an array
index. This leads to maps being densely packed, so they can be smaller.
The default maps used by xdp_redirect() are changed to use the new map
type, which means that xdp_redirect() is no longer limited to ifindex < 64,
but instead to 64 total simultaneous interfaces per network namespace. This
also provides an easy way to compare the performance of devmap and
devmap_idx:
xdp_redirect_map (devmap): 8394560 pkt/s
xdp_redirect (devmap_idx): 8179480 pkt/s
Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 12 +-
include/linux/bpf_types.h | 1
include/trace/events/xdp.h | 3
include/uapi/linux/bpf.h | 1
kernel/bpf/devmap.c | 232 ++++++++++++++++++++++++++++++-
kernel/bpf/verifier.c | 2
net/core/filter.c | 11 +
tools/bpf/bpftool/map.c | 1
tools/include/uapi/linux/bpf.h | 1
tools/lib/bpf/libbpf_probes.c | 1
tools/testing/selftests/bpf/test_maps.c | 16 ++
11 files changed, 265 insertions(+), 16 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b0f28989ccd7..532cae85de4e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -614,12 +614,13 @@ struct xdp_buff;
struct sk_buff;
struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
+struct bpf_dtab_netdev *__dev_map_idx_lookup_elem(struct bpf_map *map, u32 key);
struct bpf_map *__dev_map_get_default_map(struct net_device *dev);
int dev_map_ensure_default_map(struct net *net);
void dev_map_put_default_map(struct net *net);
int dev_map_inc_redirect_count(void);
void dev_map_dec_redirect_count(void);
-void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
+void __dev_map_insert_ctx(struct bpf_map *map, struct bpf_dtab_netdev *dst);
void __dev_map_flush(struct bpf_map *map);
int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
struct net_device *dev_rx);
@@ -705,6 +706,12 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
return NULL;
}
+static inline struct net_device *__dev_map_idx_lookup_elem(struct bpf_map *map,
+ u32 key)
+{
+ return NULL;
+}
+
static inline struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
{
return NULL;
@@ -727,7 +734,8 @@ static inline void dev_map_dec_redirect_count(void)
{
}
-static inline void __dev_map_insert_ctx(struct bpf_map *map, u32 index)
+static inline void __dev_map_insert_ctx(struct bpf_map *map,
+ struct bpf_dtab_netdev *dst)
{
}
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 08bf2f1fe553..374c013ca243 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -59,6 +59,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
#ifdef CONFIG_NET
BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
+BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_IDX, dev_map_idx_ops)
#if defined(CONFIG_BPF_STREAM_PARSER)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index e95cb86b65cf..fcf006d49f67 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -147,7 +147,8 @@ struct _bpf_dtab_netdev {
#define devmap_ifindex(fwd, map) \
(!fwd ? 0 : \
- ((map->map_type == BPF_MAP_TYPE_DEVMAP) ? \
+ ((map->map_type == BPF_MAP_TYPE_DEVMAP || \
+ map->map_type == BPF_MAP_TYPE_DEVMAP_IDX) ? \
((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0))
#define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..2cdd384f2cbc 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -132,6 +132,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
+ BPF_MAP_TYPE_DEVMAP_IDX,
};
/* Note that tracing related programs such as
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index e55707e62b60..95b276104dcd 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -46,6 +46,12 @@
* notifier hook walks the map we know that new dev references can not be
* added by the user because core infrastructure ensures dev_get_by_index()
* calls will fail at this point.
+ *
+ * The devmap_idx type is a map type which interprets keys as ifindexes and
+ * indexes these using a hashmap. This allows maps that use ifindex as key to be
+ * densely packed instead of having holes in the lookup array for unused
+ * ifindexes. The setup and packet enqueue/send code is shared between the two
+ * types of devmap; only the lookup and insertion is different.
*/
#include <linux/bpf.h>
#include <net/xdp.h>
@@ -67,6 +73,8 @@ struct xdp_bulk_queue {
struct bpf_dtab_netdev {
struct net_device *dev; /* must be first member, due to tracepoint */
+ unsigned int ifindex;
+ struct hlist_node index_hlist;
struct bpf_dtab *dtab;
unsigned int bit;
struct xdp_bulk_queue __percpu *bulkq;
@@ -79,12 +87,30 @@ struct bpf_dtab {
unsigned long __percpu *flush_needed;
struct list_head list;
struct rcu_head rcu;
+
+ /* these are only used for DEVMAP_IDX type maps */
+ unsigned long *bits_used;
+ struct hlist_head *dev_index_head;
+ spinlock_t index_lock;
};
static DEFINE_SPINLOCK(dev_map_lock);
static LIST_HEAD(dev_map_list);
static atomic_t global_redirect_use = {};
+static struct hlist_head *dev_map_create_hash(void)
+{
+ int i;
+ struct hlist_head *hash;
+
+ hash = kmalloc_array(NETDEV_HASHENTRIES, sizeof(*hash), GFP_KERNEL);
+ if (hash != NULL)
+ for (i = 0; i < NETDEV_HASHENTRIES; i++)
+ INIT_HLIST_HEAD(&hash[i]);
+
+ return hash;
+}
+
static u64 dev_map_bitmap_size(const union bpf_attr *attr)
{
return BITS_TO_LONGS((u64) attr->max_entries) * sizeof(unsigned long);
@@ -101,6 +127,11 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
/* make sure page count doesn't overflow */
cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
cost += dev_map_bitmap_size(attr) * num_possible_cpus();
+
+ if (attr->map_type == BPF_MAP_TYPE_DEVMAP_IDX)
+ cost += dev_map_bitmap_size(attr) +
+ sizeof(struct hlist_head) * NETDEV_HASHENTRIES;
+
if (cost >= U32_MAX - PAGE_SIZE)
return -EINVAL;
@@ -126,8 +157,25 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
if (!dtab->netdev_map)
goto err_map;
+ if (attr->map_type == BPF_MAP_TYPE_DEVMAP_IDX) {
+ dtab->bits_used = kzalloc(dev_map_bitmap_size(attr),
+ GFP_KERNEL);
+ if (!dtab->bits_used)
+ goto err_bitmap;
+
+ dtab->dev_index_head = dev_map_create_hash();
+ if (!dtab->dev_index_head)
+ goto err_idx;
+
+ spin_lock_init(&dtab->index_lock);
+ }
+
return 0;
+ err_idx:
+ kfree(dtab->bits_used);
+ err_bitmap:
+ bpf_map_area_free(dtab->netdev_map);
err_map:
free_percpu(dtab->flush_needed);
err_alloc:
@@ -192,6 +240,8 @@ static void __dev_map_free(struct rcu_head *rcu)
kfree(dev);
}
+ kfree(dtab->dev_index_head);
+ kfree(dtab->bits_used);
free_percpu(dtab->flush_needed);
bpf_map_area_free(dtab->netdev_map);
kfree(dtab);
@@ -234,12 +284,76 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
return 0;
}
-void __dev_map_insert_ctx(struct bpf_map *map, u32 bit)
+static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
+ int ifindex)
+{
+ return &dtab->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
+}
+
+struct bpf_dtab_netdev *__dev_map_idx_lookup_elem(struct bpf_map *map, u32 key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct hlist_head *head = dev_map_index_hash(dtab, key);
+ struct bpf_dtab_netdev *dev;
+
+ hlist_for_each_entry_rcu(dev, head, index_hlist)
+ if (dev->ifindex == key)
+ return dev;
+
+ return NULL;
+}
+
+static int dev_map_idx_get_next_key(struct bpf_map *map, void *key,
+ void *next_key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ u32 ifindex, *next = next_key;
+ struct bpf_dtab_netdev *dev, *next_dev;
+ struct hlist_head *head;
+ int i = 0;
+
+ if (!key)
+ goto find_first;
+
+ ifindex = *(u32 *)key;
+
+ dev = __dev_map_idx_lookup_elem(map, ifindex);
+ if (!dev)
+ goto find_first;
+
+ next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&dev->index_hlist)),
+ struct bpf_dtab_netdev, index_hlist);
+
+ if (next_dev) {
+ *next = next_dev->ifindex;
+ return 0;
+ }
+
+ i = ifindex & (NETDEV_HASHENTRIES - 1);
+ i++;
+
+ find_first:
+ for (; i < NETDEV_HASHENTRIES; i++) {
+ head = dev_map_index_hash(dtab, i);
+
+ next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
+ struct bpf_dtab_netdev,
+ index_hlist);
+ if (next_dev) {
+ *next = next_dev->ifindex;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+void __dev_map_insert_ctx(struct bpf_map *map, struct bpf_dtab_netdev *dst)
{
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
unsigned long *bitmap = this_cpu_ptr(dtab->flush_needed);
- __set_bit(bit, bitmap);
+ __set_bit(dst->bit, bitmap);
}
static int bq_xmit_all(struct bpf_dtab_netdev *obj,
@@ -409,9 +523,16 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
{
struct bpf_dtab_netdev *obj = __dev_map_lookup_elem(map, *(u32 *)key);
- struct net_device *dev = obj ? obj->dev : NULL;
- return dev ? &dev->ifindex : NULL;
+ return obj ? &obj->ifindex : NULL;
+}
+
+static void *dev_map_idx_lookup_elem(struct bpf_map *map, void *key)
+{
+ struct bpf_dtab_netdev *obj = __dev_map_idx_lookup_elem(map,
+ *(u32 *)key);
+
+ return obj ? &obj->ifindex : NULL;
}
static void dev_map_flush_old(struct bpf_dtab_netdev *dev)
@@ -466,6 +587,43 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
return 0;
}
+static int dev_map_idx_delete_elem(struct bpf_map *map, void *key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *old_dev;
+ int k = *(u32 *)key;
+
+ old_dev = __dev_map_idx_lookup_elem(map, k);
+ if (!old_dev)
+ return 0;
+
+ spin_lock(&dtab->index_lock);
+ hlist_del_rcu(&old_dev->index_hlist);
+ spin_unlock(&dtab->index_lock);
+
+ xchg(&dtab->netdev_map[old_dev->bit], NULL);
+ clear_bit_unlock(old_dev->bit, dtab->bits_used);
+ call_rcu(&old_dev->rcu, __dev_map_entry_free);
+ return 0;
+}
+
+static bool __dev_map_find_bit(struct bpf_dtab *dtab, unsigned int *bit)
+{
+ unsigned int b = 0;
+
+ retry:
+ b = find_next_zero_bit(dtab->bits_used, dtab->map.max_entries, b);
+
+ if (b >= dtab->map.max_entries)
+ return false;
+
+ if (test_and_set_bit_lock(b, dtab->bits_used))
+ goto retry;
+
+ *bit = b;
+ return true;
+}
+
static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
struct bpf_dtab *dtab,
u32 ifindex,
@@ -492,6 +650,7 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
return ERR_PTR(-EINVAL);
}
+ dev->ifindex = dev->dev->ifindex;
dev->bit = bit;
dev->dtab = dtab;
@@ -539,6 +698,49 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
map, key, value, map_flags);
}
+static int __dev_map_idx_update_elem(struct net *net, struct bpf_map *map,
+ void *key, void *value, u64 map_flags)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *dev, *old_dev;
+ u32 idx = *(u32 *)key;
+ u32 val = *(u32 *)value;
+ u32 bit;
+
+ if (idx != val)
+ return -EINVAL;
+ if (unlikely(map_flags > BPF_EXIST))
+ return -EINVAL;
+
+ old_dev = __dev_map_idx_lookup_elem(map, idx);
+ if (old_dev) {
+ if (map_flags & BPF_NOEXIST)
+ return -EEXIST;
+ else
+ return 0;
+ }
+
+ if (!__dev_map_find_bit(dtab, &bit))
+ return -ENOSPC;
+ dev = __dev_map_alloc_node(net, dtab, idx, bit);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ xchg(&dtab->netdev_map[bit], dev);
+ spin_lock(&dtab->index_lock);
+ hlist_add_head_rcu(&dev->index_hlist,
+ dev_map_index_hash(dtab, dev->ifindex));
+ spin_unlock(&dtab->index_lock);
+ return 0;
+}
+
+static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
+ u64 map_flags)
+{
+ return __dev_map_idx_update_elem(current->nsproxy->net_ns,
+ map, key, value, map_flags);
+}
+
const struct bpf_map_ops dev_map_ops = {
.map_alloc = dev_map_alloc,
.map_free = dev_map_free,
@@ -549,6 +751,16 @@ const struct bpf_map_ops dev_map_ops = {
.map_check_btf = map_check_no_btf,
};
+const struct bpf_map_ops dev_map_idx_ops = {
+ .map_alloc = dev_map_alloc,
+ .map_free = dev_map_free,
+ .map_get_next_key = dev_map_idx_get_next_key,
+ .map_lookup_elem = dev_map_idx_lookup_elem,
+ .map_update_elem = dev_map_idx_update_elem,
+ .map_delete_elem = dev_map_idx_delete_elem,
+ .map_check_btf = map_check_no_btf,
+};
+
static inline struct net *bpf_default_map_to_net(struct bpf_dtab_container *cont)
{
struct netns_xdp *xdp = container_of(cont, struct netns_xdp, default_map);
@@ -583,8 +795,8 @@ void dev_map_put_default_map(struct net *net)
static int __init_default_map(struct bpf_dtab_container *cont)
{
struct net *net = bpf_default_map_to_net(cont);
+ int size = DEV_MAP_DEFAULT_SIZE, i = 0;
struct bpf_dtab *dtab, *old_dtab;
- int size = DEV_MAP_DEFAULT_SIZE;
struct net_device *netdev;
union bpf_attr attr = {};
u32 idx;
@@ -596,7 +808,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
return 0;
for_each_netdev(net, netdev)
- if (netdev->ifindex >= size)
+ if (++i >= size)
size <<= 1;
old_dtab = rcu_dereference(cont->dtab);
@@ -607,7 +819,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
if (!dtab)
return -ENOMEM;
- attr.map_type = BPF_MAP_TYPE_DEVMAP;
+ attr.map_type = BPF_MAP_TYPE_DEVMAP_IDX;
attr.max_entries = size;
attr.value_size = 4;
attr.key_size = 4;
@@ -620,7 +832,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
for_each_netdev(net, netdev) {
idx = netdev->ifindex;
- err = __dev_map_update_elem(net, &dtab->map, &idx, &idx, 0);
+ err = __dev_map_idx_update_elem(net, &dtab->map, &idx, &idx, 0);
if (err) {
__dev_map_free(&dtab->rcu);
return err;
@@ -741,8 +953,8 @@ static int dev_map_notification(struct notifier_block *notifier,
rcu_read_lock();
dtab = rcu_dereference(net->xdp.default_map.dtab);
if (dtab) {
- err = __dev_map_update_elem(net, &dtab->map,
- &idx, &idx, 0);
+ err = __dev_map_idx_update_elem(net, &dtab->map,
+ &idx, &idx, 0);
if (err == -E2BIG) {
spin_lock(&dev_map_lock);
err = __init_default_map(&net->xdp.default_map);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f1b2f01e7ca1..5bdcb3c23014 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2576,6 +2576,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
* for now.
*/
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
if (func_id != BPF_FUNC_redirect_map)
goto error;
break;
@@ -2648,6 +2649,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
break;
case BPF_FUNC_redirect_map:
if (map->map_type != BPF_MAP_TYPE_DEVMAP &&
+ map->map_type != BPF_MAP_TYPE_DEVMAP_IDX &&
map->map_type != BPF_MAP_TYPE_CPUMAP &&
map->map_type != BPF_MAP_TYPE_XSKMAP)
goto error;
diff --git a/net/core/filter.c b/net/core/filter.c
index be02ea103d05..59f2e188e5ed 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3345,13 +3345,14 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
int err;
switch (map->map_type) {
- case BPF_MAP_TYPE_DEVMAP: {
+ case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX: {
struct bpf_dtab_netdev *dst = fwd;
err = dev_map_enqueue(dst, xdp, dev_rx);
if (unlikely(err))
return err;
- __dev_map_insert_ctx(map, index);
+ __dev_map_insert_ctx(map, dst);
break;
}
case BPF_MAP_TYPE_CPUMAP: {
@@ -3384,6 +3385,7 @@ void xdp_do_flush_map(void)
if (map) {
switch (map->map_type) {
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
__dev_map_flush(map);
break;
case BPF_MAP_TYPE_CPUMAP:
@@ -3404,6 +3406,8 @@ static inline void *__xdp_map_lookup_elem(struct bpf_map *map, u32 index)
switch (map->map_type) {
case BPF_MAP_TYPE_DEVMAP:
return __dev_map_lookup_elem(map, index);
+ case BPF_MAP_TYPE_DEVMAP_IDX:
+ return __dev_map_idx_lookup_elem(map, index);
case BPF_MAP_TYPE_CPUMAP:
return __cpu_map_lookup_elem(map, index);
case BPF_MAP_TYPE_XSKMAP:
@@ -3494,7 +3498,8 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
goto err;
}
- if (map->map_type == BPF_MAP_TYPE_DEVMAP) {
+ if (map->map_type == BPF_MAP_TYPE_DEVMAP ||
+ map->map_type == BPF_MAP_TYPE_DEVMAP_IDX) {
struct bpf_dtab_netdev *dst = fwd;
err = dev_map_generic_redirect(dst, skb, xdp_prog);
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e0c650d91784..0864ce33df94 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -37,6 +37,7 @@ const char * const map_type_name[] = {
[BPF_MAP_TYPE_ARRAY_OF_MAPS] = "array_of_maps",
[BPF_MAP_TYPE_HASH_OF_MAPS] = "hash_of_maps",
[BPF_MAP_TYPE_DEVMAP] = "devmap",
+ [BPF_MAP_TYPE_DEVMAP_IDX] = "devmap_idx",
[BPF_MAP_TYPE_SOCKMAP] = "sockmap",
[BPF_MAP_TYPE_CPUMAP] = "cpumap",
[BPF_MAP_TYPE_XSKMAP] = "xskmap",
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..2cdd384f2cbc 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -132,6 +132,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
+ BPF_MAP_TYPE_DEVMAP_IDX,
};
/* Note that tracing related programs such as
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 8c3a1c04dcb2..b87b760a1355 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -172,6 +172,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
case BPF_MAP_TYPE_ARRAY_OF_MAPS:
case BPF_MAP_TYPE_HASH_OF_MAPS:
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
case BPF_MAP_TYPE_SOCKMAP:
case BPF_MAP_TYPE_CPUMAP:
case BPF_MAP_TYPE_XSKMAP:
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 3c627771f965..63681e4647f9 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -519,6 +519,21 @@ static void test_devmap(unsigned int task, void *data)
close(fd);
}
+static void test_devmap_idx(unsigned int task, void *data)
+{
+ int fd;
+ __u32 key, value;
+
+ fd = bpf_create_map(BPF_MAP_TYPE_DEVMAP_IDX, sizeof(key), sizeof(value),
+ 2, 0);
+ if (fd < 0) {
+ printf("Failed to create devmap_idx '%s'!\n", strerror(errno));
+ exit(1);
+ }
+
+ close(fd);
+}
+
static void test_queuemap(unsigned int task, void *data)
{
const int MAP_SIZE = 32;
@@ -1686,6 +1701,7 @@ static void run_all_tests(void)
test_arraymap_percpu_many_keys();
test_devmap(0, NULL);
+ test_devmap_idx(0, NULL);
test_sockmap(0, NULL);
test_map_large();
^ permalink raw reply related
* [PATCH net-next 2 2/3] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-28 13:23 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136022499.3174.12765993257926981358.stgit@alrua-x1>
An XDP program can redirect packets between interfaces using either the
xdp_redirect() helper or the xdp_redirect_map() helper. Apart from the
flexibility of updating maps from userspace, the redirect_map() helper also
uses the map structure to batch packets, which results in a significant
(around 50%) performance boost. However, the xdp_redirect() API is simpler
if one just wants to redirect to another interface, which means people tend
to use this interface and then wonder why they getter worse performance
than expected.
This patch seeks to close this performance difference between the two APIs.
It achieves this by changing xdp_redirect() to use a hidden devmap for
looking up destination interfaces, thus gaining the batching benefit with
no visible difference from the user API point of view.
A hidden per-namespace map is allocated when an XDP program that uses the
non-map xdp_redirect() helper is first loaded. This map is populated with
all available interfaces in its namespace, and kept up to date as
interfaces come and go. Once allocated, the map is kept around until the
namespace is removed.
The hidden map uses the ifindex as map key, which means they are limited to
ifindexes smaller than the map size of 64. A later patch introduces a new
map type to lift this restriction.
Performance numbers:
Before patch:
xdp_redirect: 5426035 pkt/s
xdp_redirect_map: 8412754 pkt/s
After patch:
xdp_redirect: 8314702 pkt/s
xdp_redirect_map: 8411854 pkt/s
This corresponds to a 53% increase in xdp_redirect performance, or a
reduction in per-packet processing time by 64 nanoseconds.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 34 +++++++
include/linux/filter.h | 2
include/net/net_namespace.h | 2
include/net/netns/xdp.h | 11 ++
kernel/bpf/devmap.c | 214 +++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/syscall.c | 27 ++++-
kernel/bpf/verifier.c | 12 ++
net/core/dev.c | 59 ++++++++++++
net/core/filter.c | 58 +-----------
9 files changed, 353 insertions(+), 66 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..b0f28989ccd7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -25,6 +25,7 @@ struct sock;
struct seq_file;
struct btf;
struct btf_type;
+struct net;
/* map is generic key/value storage optionally accesible by eBPF programs */
struct bpf_map_ops {
@@ -533,6 +534,7 @@ extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
extern const struct bpf_verifier_ops xdp_analyzer_ops;
struct bpf_prog *bpf_prog_get(u32 ufd);
+struct bpf_prog *bpf_prog_get_by_id(u32 id);
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
bool attach_drv);
struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
@@ -612,6 +614,11 @@ struct xdp_buff;
struct sk_buff;
struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
+struct bpf_map *__dev_map_get_default_map(struct net_device *dev);
+int dev_map_ensure_default_map(struct net *net);
+void dev_map_put_default_map(struct net *net);
+int dev_map_inc_redirect_count(void);
+void dev_map_dec_redirect_count(void);
void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
void __dev_map_flush(struct bpf_map *map);
int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
@@ -641,6 +648,11 @@ static inline struct bpf_prog *bpf_prog_get(u32 ufd)
return ERR_PTR(-EOPNOTSUPP);
}
+static inline struct bpf_prog *bpf_prog_get_by_id(u32 id)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
static inline struct bpf_prog *bpf_prog_get_type_dev(u32 ufd,
enum bpf_prog_type type,
bool attach_drv)
@@ -693,6 +705,28 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
return NULL;
}
+static inline struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
+{
+ return NULL;
+}
+
+static inline int dev_map_ensure_default_map(struct net *net)
+{
+ return 0;
+}
+
+static inline void dev_map_put_default_map(struct net *net)
+{
+}
+
+static inline int dev_map_inc_redirect_count(void)
+{
+}
+
+static inline void dev_map_dec_redirect_count(void)
+{
+}
+
static inline void __dev_map_insert_ctx(struct bpf_map *map, u32 index)
{
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 95e2d7ebdf21..dd6bbbab32f7 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -507,6 +507,8 @@ struct bpf_prog {
gpl_compatible:1, /* Is filter GPL compatible? */
cb_access:1, /* Is control block accessed? */
dst_needed:1, /* Do we need dst entry? */
+ redirect_needed:1, /* Does program need access to xdp_redirect? */
+ redirect_used:1, /* Does program use xdp_redirect? */
blinded:1, /* Was blinded */
is_func:1, /* program is a bpf function */
kprobe_override:1, /* Do we override a kprobe? */
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index a68ced28d8f4..6706ecc25d8f 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -162,7 +162,7 @@ struct net {
#if IS_ENABLED(CONFIG_CAN)
struct netns_can can;
#endif
-#ifdef CONFIG_XDP_SOCKETS
+#ifdef CONFIG_BPF_SYSCALL
struct netns_xdp xdp;
#endif
struct sock *diag_nlsk;
diff --git a/include/net/netns/xdp.h b/include/net/netns/xdp.h
index e5734261ba0a..4935dfe1cf43 100644
--- a/include/net/netns/xdp.h
+++ b/include/net/netns/xdp.h
@@ -4,10 +4,21 @@
#include <linux/rculist.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>
+
+struct bpf_dtab;
+
+struct bpf_dtab_container {
+ struct bpf_dtab __rcu *dtab;
+ atomic_t refcnt;
+};
struct netns_xdp {
+#ifdef CONFIG_XDP_SOCKETS
struct mutex lock;
struct hlist_head list;
+#endif
+ struct bpf_dtab_container default_map;
};
#endif /* __NETNS_XDP_H__ */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 1037fc08c504..e55707e62b60 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -56,6 +56,9 @@
(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
#define DEV_MAP_BULK_SIZE 16
+#define DEV_MAP_DEFAULT_SIZE 8
+#define BPF_MAX_REFCNT 32768
+
struct xdp_bulk_queue {
struct xdp_frame *q[DEV_MAP_BULK_SIZE];
struct net_device *dev_rx;
@@ -80,6 +83,7 @@ struct bpf_dtab {
static DEFINE_SPINLOCK(dev_map_lock);
static LIST_HEAD(dev_map_list);
+static atomic_t global_redirect_use = {};
static u64 dev_map_bitmap_size(const union bpf_attr *attr)
{
@@ -332,6 +336,18 @@ struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key)
return obj;
}
+/* This is only being called from xdp_do_redirect() if the xdp_redirect helper
+ * is used; the default map is allocated on XDP program load if the helper is
+ * used, so will always be available at this point.
+ */
+struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
+{
+ struct net *net = dev_net(dev);
+ struct bpf_dtab *dtab = rcu_dereference(net->xdp.default_map.dtab);
+
+ return dtab ? &dtab->map : NULL;
+}
+
/* Runs under RCU-read-side, plus in softirq under NAPI protection.
* Thus, safe percpu variable access.
*/
@@ -533,14 +549,210 @@ const struct bpf_map_ops dev_map_ops = {
.map_check_btf = map_check_no_btf,
};
+static inline struct net *bpf_default_map_to_net(struct bpf_dtab_container *cont)
+{
+ struct netns_xdp *xdp = container_of(cont, struct netns_xdp, default_map);
+
+ return container_of(xdp, struct net, xdp);
+}
+
+static void __dev_map_release_default_map(struct bpf_dtab_container *cont)
+{
+ struct bpf_dtab *dtab = NULL;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ dtab = rcu_dereference(cont->dtab);
+ if (dtab) {
+ list_del_rcu(&dtab->list);
+ rcu_assign_pointer(cont->dtab, NULL);
+ bpf_clear_redirect_map(&dtab->map);
+ call_rcu(&dtab->rcu, __dev_map_free);
+ }
+}
+
+void dev_map_put_default_map(struct net *net)
+{
+ if (atomic_dec_and_test(&net->xdp.default_map.refcnt)) {
+ spin_lock(&dev_map_lock);
+ __dev_map_release_default_map(&net->xdp.default_map);
+ spin_unlock(&dev_map_lock);
+ }
+}
+
+static int __init_default_map(struct bpf_dtab_container *cont)
+{
+ struct net *net = bpf_default_map_to_net(cont);
+ struct bpf_dtab *dtab, *old_dtab;
+ int size = DEV_MAP_DEFAULT_SIZE;
+ struct net_device *netdev;
+ union bpf_attr attr = {};
+ u32 idx;
+ int err;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ if (!atomic_read(&global_redirect_use))
+ return 0;
+
+ for_each_netdev(net, netdev)
+ if (netdev->ifindex >= size)
+ size <<= 1;
+
+ old_dtab = rcu_dereference(cont->dtab);
+ if (old_dtab && old_dtab->map.max_entries == size)
+ return 0;
+
+ dtab = kzalloc(sizeof(*dtab), GFP_USER);
+ if (!dtab)
+ return -ENOMEM;
+
+ attr.map_type = BPF_MAP_TYPE_DEVMAP;
+ attr.max_entries = size;
+ attr.value_size = 4;
+ attr.key_size = 4;
+
+ err = dev_map_init_map(dtab, &attr, false);
+ if (err) {
+ kfree(dtab);
+ return err;
+ }
+
+ for_each_netdev(net, netdev) {
+ idx = netdev->ifindex;
+ err = __dev_map_update_elem(net, &dtab->map, &idx, &idx, 0);
+ if (err) {
+ __dev_map_free(&dtab->rcu);
+ return err;
+ }
+ }
+
+ rcu_assign_pointer(cont->dtab, dtab);
+ list_add_tail_rcu(&dtab->list, &dev_map_list);
+
+ if (old_dtab) {
+ list_del_rcu(&old_dtab->list);
+ bpf_clear_redirect_map(&old_dtab->map);
+ call_rcu(&old_dtab->rcu, __dev_map_free);
+ }
+
+ return 0;
+}
+
+static int maybe_inc_refcnt(atomic_t *v)
+{
+ int refcnt;
+
+ refcnt = atomic_inc_return(v);
+ if (refcnt > BPF_MAX_REFCNT) {
+ atomic_dec(v);
+ return -EBUSY;
+ }
+
+ return refcnt;
+}
+
+int dev_map_ensure_default_map(struct net *net)
+{
+ int refcnt, err = 0;
+
+ refcnt = maybe_inc_refcnt(&net->xdp.default_map.refcnt);
+ if (refcnt < 0)
+ return refcnt;
+
+ if (refcnt == 1) {
+ spin_lock(&dev_map_lock);
+ err = __init_default_map(&net->xdp.default_map);
+ spin_unlock(&dev_map_lock);
+ }
+
+ return err;
+}
+
+static void __dev_map_dec_redirect_count(void)
+{
+ struct net *net;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ if (atomic_dec_and_test(&global_redirect_use))
+ for_each_net_rcu(net)
+ __dev_map_release_default_map(&net->xdp.default_map);
+}
+
+void dev_map_dec_redirect_count(void)
+{
+ spin_lock(&dev_map_lock);
+ __dev_map_dec_redirect_count();
+ spin_unlock(&dev_map_lock);
+}
+
+static int __dev_map_init_redirect_use(void)
+{
+ struct net *net;
+ int err;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ for_each_net_rcu(net) {
+ if (atomic_read(&net->xdp.default_map.refcnt)) {
+ err = __init_default_map(&net->xdp.default_map);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+int dev_map_inc_redirect_count(void)
+{
+ int refcnt, err = 0;
+
+ spin_lock(&dev_map_lock);
+ refcnt = maybe_inc_refcnt(&global_redirect_use);
+ if (refcnt < 0) {
+ err = refcnt;
+ goto out;
+ }
+
+ if (refcnt == 1)
+ err = __dev_map_init_redirect_use();
+
+ if (err)
+ __dev_map_dec_redirect_count();
+
+ out:
+ spin_unlock(&dev_map_lock);
+ return err;
+}
+
static int dev_map_notification(struct notifier_block *notifier,
ulong event, void *ptr)
{
struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
+ struct net *net = dev_net(netdev);
+ u32 idx = netdev->ifindex;
struct bpf_dtab *dtab;
- int i;
+ int i, err;
switch (event) {
+ case NETDEV_REGISTER:
+ rcu_read_lock();
+ dtab = rcu_dereference(net->xdp.default_map.dtab);
+ if (dtab) {
+ err = __dev_map_update_elem(net, &dtab->map,
+ &idx, &idx, 0);
+ if (err == -E2BIG) {
+ spin_lock(&dev_map_lock);
+ err = __init_default_map(&net->xdp.default_map);
+ if (err)
+ net_warn_ratelimited("Unable to re-allocate default map, xdp_redirect() may fail on some ifindexes\n");
+ spin_unlock(&dev_map_lock);
+ }
+ }
+ rcu_read_unlock();
+ break;
case NETDEV_UNREGISTER:
/* This rcu_read_lock/unlock pair is needed because
* dev_map_list is an RCU list AND to ensure a delete
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ec7c552af76b..fd1b76f5da2d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1265,6 +1265,9 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
kvfree(prog->aux->func_info);
bpf_prog_free_linfo(prog);
+ if (prog->redirect_used)
+ dev_map_dec_redirect_count();
+
call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
}
}
@@ -1962,6 +1965,21 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD prog_id
+struct bpf_prog *bpf_prog_get_by_id(u32 id)
+{
+ struct bpf_prog *prog;
+
+ spin_lock_bh(&prog_idr_lock);
+ prog = idr_find(&prog_idr, id);
+ if (prog)
+ prog = bpf_prog_inc_not_zero(prog);
+ else
+ prog = ERR_PTR(-ENOENT);
+ spin_unlock_bh(&prog_idr_lock);
+
+ return prog;
+}
+
static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
{
struct bpf_prog *prog;
@@ -1974,14 +1992,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- spin_lock_bh(&prog_idr_lock);
- prog = idr_find(&prog_idr, id);
- if (prog)
- prog = bpf_prog_inc_not_zero(prog);
- else
- prog = ERR_PTR(-ENOENT);
- spin_unlock_bh(&prog_idr_lock);
-
+ prog = bpf_prog_get_by_id(id);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1b9496c41383..f1b2f01e7ca1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7609,6 +7609,17 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
prog->dst_needed = 1;
if (insn->imm == BPF_FUNC_get_prandom_u32)
bpf_user_rnd_init_once();
+ if (insn->imm == BPF_FUNC_redirect) {
+ prog->redirect_needed = true;
+ if (!prog->redirect_used) {
+ int err = dev_map_inc_redirect_count();
+
+ if (err)
+ return err;
+ prog->redirect_used = true;
+ }
+ }
+
if (insn->imm == BPF_FUNC_override_return)
prog->kprobe_override = 1;
if (insn->imm == BPF_FUNC_tail_call) {
@@ -7618,6 +7629,7 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
* the program array.
*/
prog->cb_access = 1;
+ prog->redirect_needed = true;
env->prog->aux->stack_depth = MAX_BPF_STACK;
env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b67f2aa59dd..1df20d529026 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7990,6 +7990,21 @@ u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
return xdp.prog_id;
}
+static struct bpf_prog *dev_xdp_get_prog(struct net_device *dev)
+{
+ struct bpf_prog *prog;
+ u32 prog_id;
+
+ prog_id = __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf, XDP_QUERY_PROG);
+ if (prog_id) {
+ prog = bpf_prog_get_by_id(prog_id);
+ if (!IS_ERR(prog))
+ return prog;
+ }
+
+ return NULL;
+}
+
static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
struct netlink_ext_ack *extack, u32 flags,
struct bpf_prog *prog)
@@ -8024,9 +8039,18 @@ static void dev_xdp_uninstall(struct net_device *dev)
memset(&xdp, 0, sizeof(xdp));
xdp.command = XDP_QUERY_PROG;
WARN_ON(ndo_bpf(dev, &xdp));
- if (xdp.prog_id)
+ if (xdp.prog_id) {
+ struct bpf_prog *prog = bpf_prog_get_by_id(xdp.prog_id);
+
+ if (!IS_ERR(prog)) {
+ if (prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+ bpf_prog_put(prog);
+ }
+
WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
NULL));
+ }
/* Remove HW offload */
memset(&xdp, 0, sizeof(xdp));
@@ -8091,6 +8115,23 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
bpf_prog_put(prog);
return -EINVAL;
}
+
+ if (!offload && bpf_op == ops->ndo_bpf &&
+ prog->redirect_needed) {
+ err = dev_map_ensure_default_map(dev_net(dev));
+ if (err) {
+ NL_SET_ERR_MSG(extack, "unable to allocate default map for xdp_redirect()");
+ return err;
+ }
+ }
+ } else {
+ struct bpf_prog *old_prog = dev_xdp_get_prog(dev);
+
+ if (old_prog) {
+ if (old_prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+ bpf_prog_put(old_prog);
+ }
}
err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
@@ -9333,6 +9374,7 @@ EXPORT_SYMBOL(unregister_netdev);
int dev_change_net_namespace(struct net_device *dev, struct net *net, const char *pat)
{
int err, new_nsid, new_ifindex;
+ struct bpf_prog *prog = NULL;
ASSERT_RTNL();
@@ -9350,6 +9392,15 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
if (net_eq(dev_net(dev), net))
goto out;
+ prog = dev_xdp_get_prog(dev);
+ if (prog) {
+ if (prog->redirect_needed)
+ err = dev_map_ensure_default_map(net);
+
+ if (err)
+ goto out;
+ }
+
/* Pick the destination device name, and ensure
* we can use it in the destination network namespace.
*/
@@ -9388,6 +9439,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
+ if (prog && prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+
new_nsid = peernet2id_alloc(dev_net(dev), net);
/* If there is an ifindex conflict assign a new one */
if (__dev_get_by_index(net, dev->ifindex))
@@ -9435,6 +9489,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
synchronize_net();
err = 0;
out:
+ if (prog)
+ bpf_prog_put(prog);
+
return err;
}
EXPORT_SYMBOL_GPL(dev_change_net_namespace);
diff --git a/net/core/filter.c b/net/core/filter.c
index 5132c054c981..be02ea103d05 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3337,58 +3337,6 @@ static const struct bpf_func_proto bpf_xdp_adjust_meta_proto = {
.arg2_type = ARG_ANYTHING,
};
-static int __bpf_tx_xdp(struct net_device *dev,
- struct bpf_map *map,
- struct xdp_buff *xdp,
- u32 index)
-{
- struct xdp_frame *xdpf;
- int err, sent;
-
- if (!dev->netdev_ops->ndo_xdp_xmit) {
- return -EOPNOTSUPP;
- }
-
- err = xdp_ok_fwd_dev(dev, xdp->data_end - xdp->data);
- if (unlikely(err))
- return err;
-
- xdpf = convert_to_xdp_frame(xdp);
- if (unlikely(!xdpf))
- return -EOVERFLOW;
-
- sent = dev->netdev_ops->ndo_xdp_xmit(dev, 1, &xdpf, XDP_XMIT_FLUSH);
- if (sent <= 0)
- return sent;
- return 0;
-}
-
-static noinline int
-xdp_do_redirect_slow(struct net_device *dev, struct xdp_buff *xdp,
- struct bpf_prog *xdp_prog, struct bpf_redirect_info *ri)
-{
- struct net_device *fwd;
- u32 index = ri->ifindex;
- int err;
-
- fwd = dev_get_by_index_rcu(dev_net(dev), index);
- ri->ifindex = 0;
- if (unlikely(!fwd)) {
- err = -EINVAL;
- goto err;
- }
-
- err = __bpf_tx_xdp(fwd, NULL, xdp, 0);
- if (unlikely(err))
- goto err;
-
- _trace_xdp_redirect(dev, xdp_prog, index);
- return 0;
-err:
- _trace_xdp_redirect_err(dev, xdp_prog, index, err);
- return err;
-}
-
static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
struct bpf_map *map,
struct xdp_buff *xdp,
@@ -3519,10 +3467,10 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct bpf_map *map = READ_ONCE(ri->map);
- if (likely(map))
- return xdp_do_redirect_map(dev, xdp, xdp_prog, map, ri);
+ if (unlikely(!map))
+ map = __dev_map_get_default_map(dev);
- return xdp_do_redirect_slow(dev, xdp, xdp_prog, ri);
+ return xdp_do_redirect_map(dev, xdp, xdp_prog, map, ri);
}
EXPORT_SYMBOL_GPL(xdp_do_redirect);
^ permalink raw reply related
* [PATCH net-next 2 1/3] xdp: Refactor devmap code in preparation for subsequent additions
From: Toke Høiland-Jørgensen @ 2019-02-28 13:23 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136022499.3174.12765993257926981358.stgit@alrua-x1>
The subsequent commits introducing default maps and a hash-based ifindex
devmap require a bit of refactoring of the devmap code. Perform this first
so the subsequent commits become easier to read.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
kernel/bpf/devmap.c | 177 +++++++++++++++++++++++++++++++--------------------
1 file changed, 109 insertions(+), 68 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 191b79948424..1037fc08c504 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -75,6 +75,7 @@ struct bpf_dtab {
struct bpf_dtab_netdev **netdev_map;
unsigned long __percpu *flush_needed;
struct list_head list;
+ struct rcu_head rcu;
};
static DEFINE_SPINLOCK(dev_map_lock);
@@ -85,23 +86,11 @@ static u64 dev_map_bitmap_size(const union bpf_attr *attr)
return BITS_TO_LONGS((u64) attr->max_entries) * sizeof(unsigned long);
}
-static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
+static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
+ bool check_memlock)
{
- struct bpf_dtab *dtab;
- int err = -EINVAL;
u64 cost;
-
- if (!capable(CAP_NET_ADMIN))
- return ERR_PTR(-EPERM);
-
- /* check sanity of attributes */
- if (attr->max_entries == 0 || attr->key_size != 4 ||
- attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
- return ERR_PTR(-EINVAL);
-
- dtab = kzalloc(sizeof(*dtab), GFP_USER);
- if (!dtab)
- return ERR_PTR(-ENOMEM);
+ int err;
bpf_map_init_from_attr(&dtab->map, attr);
@@ -109,60 +98,72 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
cost += dev_map_bitmap_size(attr) * num_possible_cpus();
if (cost >= U32_MAX - PAGE_SIZE)
- goto free_dtab;
+ return -EINVAL;
dtab->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
- /* if map size is larger than memlock limit, reject it early */
- err = bpf_map_precharge_memlock(dtab->map.pages);
- if (err)
- goto free_dtab;
-
- err = -ENOMEM;
+ if (check_memlock) {
+ /* if map size is larger than memlock limit, reject it early */
+ err = bpf_map_precharge_memlock(dtab->map.pages);
+ if (err)
+ return -EINVAL;
+ }
/* A per cpu bitfield with a bit per possible net device */
dtab->flush_needed = __alloc_percpu_gfp(dev_map_bitmap_size(attr),
__alignof__(unsigned long),
GFP_KERNEL | __GFP_NOWARN);
if (!dtab->flush_needed)
- goto free_dtab;
+ goto err_alloc;
dtab->netdev_map = bpf_map_area_alloc(dtab->map.max_entries *
sizeof(struct bpf_dtab_netdev *),
dtab->map.numa_node);
if (!dtab->netdev_map)
- goto free_dtab;
+ goto err_map;
- spin_lock(&dev_map_lock);
- list_add_tail_rcu(&dtab->list, &dev_map_list);
- spin_unlock(&dev_map_lock);
+ return 0;
- return &dtab->map;
-free_dtab:
+ err_map:
free_percpu(dtab->flush_needed);
- kfree(dtab);
- return ERR_PTR(err);
+ err_alloc:
+ return -ENOMEM;
}
-static void dev_map_free(struct bpf_map *map)
+static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
{
- struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
- int i, cpu;
+ struct bpf_dtab *dtab;
+ int err;
- /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
- * so the programs (can be more than one that used this map) were
- * disconnected from events. Wait for outstanding critical sections in
- * these programs to complete. The rcu critical section only guarantees
- * no further reads against netdev_map. It does __not__ ensure pending
- * flush operations (if any) are complete.
- */
+ if (!capable(CAP_NET_ADMIN))
+ return ERR_PTR(-EPERM);
+
+ /* check sanity of attributes */
+ if (attr->max_entries == 0 || attr->key_size != 4 ||
+ attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
+ return ERR_PTR(-EINVAL);
+
+ dtab = kzalloc(sizeof(*dtab), GFP_USER);
+ if (!dtab)
+ return ERR_PTR(-ENOMEM);
+
+ err = dev_map_init_map(dtab, attr, true);
+ if (err) {
+ kfree(dtab);
+ return ERR_PTR(err);
+ }
spin_lock(&dev_map_lock);
- list_del_rcu(&dtab->list);
+ list_add_tail_rcu(&dtab->list, &dev_map_list);
spin_unlock(&dev_map_lock);
- bpf_clear_redirect_map(map);
- synchronize_rcu();
+ return &dtab->map;
+}
+
+static void __dev_map_free(struct rcu_head *rcu)
+{
+ struct bpf_dtab *dtab = container_of(rcu, struct bpf_dtab, rcu);
+ int i, cpu;
/* To ensure all pending flush operations have completed wait for flush
* bitmap to indicate all flush_needed bits to be zero on _all_ cpus.
@@ -192,6 +193,26 @@ static void dev_map_free(struct bpf_map *map)
kfree(dtab);
}
+static void dev_map_free(struct bpf_map *map)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+
+ /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+ * so the programs (can be more than one that used this map) were
+ * disconnected from events. Wait for outstanding critical sections in
+ * these programs to complete. The rcu critical section only guarantees
+ * no further reads against netdev_map. It does __not__ ensure pending
+ * flush operations (if any) are complete.
+ */
+
+ spin_lock(&dev_map_lock);
+ list_del_rcu(&dtab->list);
+ spin_unlock(&dev_map_lock);
+
+ bpf_clear_redirect_map(map);
+ call_rcu(&dtab->rcu, __dev_map_free);
+}
+
static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
{
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
@@ -429,12 +450,42 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
return 0;
}
-static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
- u64 map_flags)
+static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
+ struct bpf_dtab *dtab,
+ u32 ifindex,
+ unsigned int bit)
{
- struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
- struct net *net = current->nsproxy->net_ns;
gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
+ struct bpf_dtab_netdev *dev;
+
+ dev = kmalloc_node(sizeof(*dev), gfp, dtab->map.numa_node);
+ if (!dev)
+ return ERR_PTR(-ENOMEM);
+
+ dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
+ sizeof(void *), gfp);
+ if (!dev->bulkq) {
+ kfree(dev);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ dev->dev = dev_get_by_index(net, ifindex);
+ if (!dev->dev) {
+ free_percpu(dev->bulkq);
+ kfree(dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ dev->bit = bit;
+ dev->dtab = dtab;
+
+ return dev;
+}
+
+static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
+ void *key, void *value, u64 map_flags)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
struct bpf_dtab_netdev *dev, *old_dev;
u32 i = *(u32 *)key;
u32 ifindex = *(u32 *)value;
@@ -449,26 +500,9 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
if (!ifindex) {
dev = NULL;
} else {
- dev = kmalloc_node(sizeof(*dev), gfp, map->numa_node);
- if (!dev)
- return -ENOMEM;
-
- dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
- sizeof(void *), gfp);
- if (!dev->bulkq) {
- kfree(dev);
- return -ENOMEM;
- }
-
- dev->dev = dev_get_by_index(net, ifindex);
- if (!dev->dev) {
- free_percpu(dev->bulkq);
- kfree(dev);
- return -EINVAL;
- }
-
- dev->bit = i;
- dev->dtab = dtab;
+ dev = __dev_map_alloc_node(net, dtab, ifindex, i);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
}
/* Use call_rcu() here to ensure rcu critical sections have completed
@@ -482,6 +516,13 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
return 0;
}
+static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
+ u64 map_flags)
+{
+ return __dev_map_update_elem(current->nsproxy->net_ns,
+ map, key, value, map_flags);
+}
+
const struct bpf_map_ops dev_map_ops = {
.map_alloc = dev_map_alloc,
.map_free = dev_map_free,
^ permalink raw reply related
* [PATCH net-next 04/15] net: mvpp2: a port can be disabled even if we use the link IRQ
From: Antoine Tenart @ 2019-02-28 13:21 UTC (permalink / raw)
To: davem, linux
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190228132128.30154-1-antoine.tenart@bootlin.com>
We had a check in the mvpp2_mac_link_down() function (called by phylink)
to avoid disabling the port when link interrupts are used. It turned out
the interrupt can still be used with the port disabled. We can thus
remove this check.
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 37d3c97c0b9b..71a3fdb76b12 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4739,13 +4739,6 @@ static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode,
mvpp2_egress_disable(port);
mvpp2_ingress_disable(port);
- /* When using link interrupts to notify phylink of a MAC state change,
- * we do not want the port to be disabled (we want to receive further
- * interrupts, to be notified when the port will have a link later).
- */
- if (!port->has_phy)
- return;
-
mvpp2_port_disable(port);
}
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 14/15] net: mvpp2: set the XPCS and MPCS in reset when not used
From: Antoine Tenart @ 2019-02-28 13:21 UTC (permalink / raw)
To: davem, linux
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190228132128.30154-1-antoine.tenart@bootlin.com>
This patch sets both the XPCS and MPCS blocks in reset when they aren't
used. This is done both at boot time and when reconfiguring a port mode.
The advantage now is that only the PCS used is set out of reset when the
port is configured (10GKR uses the MCPS while RXAUI uses the XPCS).
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 1 +
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 62 ++++++++++++++++---
2 files changed, 55 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index c9edeac9ec01..ff0f4c503f53 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -483,6 +483,7 @@
/* XPCS registers. PPv2.2 only */
#define MVPP22_XPCS_BASE(port) (0x7400 + (port) * 0x1000)
#define MVPP22_XPCS_CFG0 0x0
+#define MVPP22_XPCS_CFG0_RESET_DIS BIT(0)
#define MVPP22_XPCS_CFG0_PCS_MODE(n) ((n) << 3)
#define MVPP22_XPCS_CFG0_ACTIVE_LANE(n) ((n) << 5)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 36e709a4e7d9..29d32cb3d52b 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1016,27 +1016,20 @@ static void mvpp22_gop_init_10gkr(struct mvpp2_port *port)
void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
u32 val;
- /* XPCS */
val = readl(xpcs + MVPP22_XPCS_CFG0);
val &= ~(MVPP22_XPCS_CFG0_PCS_MODE(0x3) |
MVPP22_XPCS_CFG0_ACTIVE_LANE(0x3));
val |= MVPP22_XPCS_CFG0_ACTIVE_LANE(2);
writel(val, xpcs + MVPP22_XPCS_CFG0);
- /* MPCS */
val = readl(mpcs + MVPP22_MPCS_CTRL);
val &= ~MVPP22_MPCS_CTRL_FWD_ERR_CONN;
writel(val, mpcs + MVPP22_MPCS_CTRL);
val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
- val &= ~(MVPP22_MPCS_CLK_RESET_DIV_RATIO(0x7) | MAC_CLK_RESET_MAC |
- MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX);
+ val &= ~MVPP22_MPCS_CLK_RESET_DIV_RATIO(0x7);
val |= MVPP22_MPCS_CLK_RESET_DIV_RATIO(1);
writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
-
- val &= ~MVPP22_MPCS_CLK_RESET_DIV_SET;
- val |= MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX;
- writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
}
static int mvpp22_gop_init(struct mvpp2_port *port)
@@ -1385,6 +1378,53 @@ static void mvpp2_mac_reset(struct mvpp2_port *port)
}
}
+static void mvpp22_pcs_reset(struct mvpp2_port *port)
+{
+ struct mvpp2 *priv = port->priv;
+ void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
+ void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
+ u32 val;
+
+ if (port->priv->hw_version != MVPP22 || port->gop_id != 0)
+ return;
+
+ val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
+ val &= ~(MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX | MAC_CLK_RESET_SD_TX);
+ val |= MVPP22_MPCS_CLK_RESET_DIV_SET;
+ writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
+
+ val = readl(xpcs + MVPP22_XPCS_CFG0);
+ writel(val & ~MVPP22_XPCS_CFG0_RESET_DIS, xpcs + MVPP22_XPCS_CFG0);
+}
+
+static void mvpp22_pcs_unreset(struct mvpp2_port *port)
+{
+ struct mvpp2 *priv = port->priv;
+ void __iomem *mpcs = priv->iface_base + MVPP22_MPCS_BASE(port->gop_id);
+ void __iomem *xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
+ u32 val;
+
+ if (port->priv->hw_version != MVPP22 || port->gop_id != 0)
+ return;
+
+ switch (port->phy_interface) {
+ case PHY_INTERFACE_MODE_10GKR:
+ val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
+ val |= MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX |
+ MAC_CLK_RESET_SD_TX;
+ val &= ~MVPP22_MPCS_CLK_RESET_DIV_SET;
+ writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
+ break;
+ case PHY_INTERFACE_MODE_XAUI:
+ case PHY_INTERFACE_MODE_RXAUI:
+ val = readl(xpcs + MVPP22_XPCS_CFG0);
+ writel(val | MVPP22_XPCS_CFG0_RESET_DIS, xpcs + MVPP22_XPCS_CFG0);
+ break;
+ default:
+ break;
+ }
+}
+
/* Change maximum receive size of the port */
static inline void mvpp2_gmac_max_rx_size_set(struct mvpp2_port *port)
{
@@ -3140,12 +3180,17 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
/* Set the GMAC & XLG MAC in reset */
mvpp2_mac_reset(port);
+ /* Set the MPCS and XPCS in reset */
+ mvpp22_pcs_reset(port);
+
/* comphy reconfiguration */
mvpp22_comphy_init(port);
/* gop reconfiguration */
mvpp22_gop_init(port);
+ mvpp22_pcs_unreset(port);
+
/* Only GOP port 0 has an XLG MAC */
if (port->gop_id == 0) {
ctrl3 = readl(port->base + MVPP22_XLG_CTRL3_REG);
@@ -4960,6 +5005,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
mvpp2_port_periodic_xon_disable(port);
mvpp2_mac_reset(port);
+ mvpp22_pcs_reset(port);
port->pcpu = alloc_percpu(struct mvpp2_port_pcpu);
if (!port->pcpu) {
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 15/15] net: mvpp2: set the GMAC, XLG MAC, XPCS and MPCS in reset when a port is down
From: Antoine Tenart @ 2019-02-28 13:21 UTC (permalink / raw)
To: davem, linux
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190228132128.30154-1-antoine.tenart@bootlin.com>
This patch adds calls in the stop() helper to ensure both MACs and
both PCS blocks are set in reset when the user manually sets a port
down. This is done so that we have the exact same block reset states at
boot time and when a port is set down.
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 29d32cb3d52b..c10fd894c86f 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3257,6 +3257,7 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
if (port->phylink)
phylink_stop(port->phylink);
+
phy_power_off(port->comphy);
}
@@ -3520,6 +3521,9 @@ static int mvpp2_stop(struct net_device *dev)
mvpp2_cleanup_rxqs(port);
mvpp2_cleanup_txqs(port);
+ mvpp2_mac_reset(port);
+ mvpp22_pcs_reset(port);
+
cancel_delayed_work_sync(&port->stats_work);
return 0;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 13/15] net: mvpp2: reset the MACs when reconfiguring a port
From: Antoine Tenart @ 2019-02-28 13:21 UTC (permalink / raw)
To: davem, linux
Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, gregory.clement, miquel.raynal, nadavh,
stefanc, ymarkman, mw
In-Reply-To: <20190228132128.30154-1-antoine.tenart@bootlin.com>
This patch makes sure both PPv2 MACs (GMAC + XLG MAC) are set in reset
while a port is reconfigured. This is done so that we make sure a MAC is
in a reset state when not used, as only one of the two will be set out
of reset after the port is configured properly.
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 13c4fcbc4269..36e709a4e7d9 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3137,6 +3137,9 @@ static void mvpp22_mode_reconfigure(struct mvpp2_port *port)
{
u32 ctrl3;
+ /* Set the GMAC & XLG MAC in reset */
+ mvpp2_mac_reset(port);
+
/* comphy reconfiguration */
mvpp22_comphy_init(port);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 2/3] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-28 13:25 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136028377.3381.2072266362746015640.stgit@alrua-x1>
An XDP program can redirect packets between interfaces using either the
xdp_redirect() helper or the xdp_redirect_map() helper. Apart from the
flexibility of updating maps from userspace, the redirect_map() helper also
uses the map structure to batch packets, which results in a significant
(around 50%) performance boost. However, the xdp_redirect() API is simpler
if one just wants to redirect to another interface, which means people tend
to use this interface and then wonder why they getter worse performance
than expected.
This patch seeks to close this performance difference between the two APIs.
It achieves this by changing xdp_redirect() to use a hidden devmap for
looking up destination interfaces, thus gaining the batching benefit with
no visible difference from the user API point of view.
A hidden per-namespace map is allocated when an XDP program that uses the
non-map xdp_redirect() helper is first loaded. This map is populated with
all available interfaces in its namespace, and kept up to date as
interfaces come and go. Once allocated, the map is kept around until the
namespace is removed.
The hidden map uses the ifindex as map key, which means they are limited to
ifindexes smaller than the map size of 64. A later patch introduces a new
map type to lift this restriction.
Performance numbers:
Before patch:
xdp_redirect: 5426035 pkt/s
xdp_redirect_map: 8412754 pkt/s
After patch:
xdp_redirect: 8314702 pkt/s
xdp_redirect_map: 8411854 pkt/s
This corresponds to a 53% increase in xdp_redirect performance, or a
reduction in per-packet processing time by 64 nanoseconds.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 34 +++++++
include/linux/filter.h | 2
include/net/net_namespace.h | 2
include/net/netns/xdp.h | 11 ++
kernel/bpf/devmap.c | 214 +++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/syscall.c | 27 ++++-
kernel/bpf/verifier.c | 12 ++
net/core/dev.c | 59 ++++++++++++
net/core/filter.c | 58 +-----------
9 files changed, 353 insertions(+), 66 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..b0f28989ccd7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -25,6 +25,7 @@ struct sock;
struct seq_file;
struct btf;
struct btf_type;
+struct net;
/* map is generic key/value storage optionally accesible by eBPF programs */
struct bpf_map_ops {
@@ -533,6 +534,7 @@ extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
extern const struct bpf_verifier_ops xdp_analyzer_ops;
struct bpf_prog *bpf_prog_get(u32 ufd);
+struct bpf_prog *bpf_prog_get_by_id(u32 id);
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
bool attach_drv);
struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
@@ -612,6 +614,11 @@ struct xdp_buff;
struct sk_buff;
struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
+struct bpf_map *__dev_map_get_default_map(struct net_device *dev);
+int dev_map_ensure_default_map(struct net *net);
+void dev_map_put_default_map(struct net *net);
+int dev_map_inc_redirect_count(void);
+void dev_map_dec_redirect_count(void);
void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
void __dev_map_flush(struct bpf_map *map);
int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
@@ -641,6 +648,11 @@ static inline struct bpf_prog *bpf_prog_get(u32 ufd)
return ERR_PTR(-EOPNOTSUPP);
}
+static inline struct bpf_prog *bpf_prog_get_by_id(u32 id)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
static inline struct bpf_prog *bpf_prog_get_type_dev(u32 ufd,
enum bpf_prog_type type,
bool attach_drv)
@@ -693,6 +705,28 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
return NULL;
}
+static inline struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
+{
+ return NULL;
+}
+
+static inline int dev_map_ensure_default_map(struct net *net)
+{
+ return 0;
+}
+
+static inline void dev_map_put_default_map(struct net *net)
+{
+}
+
+static inline int dev_map_inc_redirect_count(void)
+{
+}
+
+static inline void dev_map_dec_redirect_count(void)
+{
+}
+
static inline void __dev_map_insert_ctx(struct bpf_map *map, u32 index)
{
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 95e2d7ebdf21..dd6bbbab32f7 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -507,6 +507,8 @@ struct bpf_prog {
gpl_compatible:1, /* Is filter GPL compatible? */
cb_access:1, /* Is control block accessed? */
dst_needed:1, /* Do we need dst entry? */
+ redirect_needed:1, /* Does program need access to xdp_redirect? */
+ redirect_used:1, /* Does program use xdp_redirect? */
blinded:1, /* Was blinded */
is_func:1, /* program is a bpf function */
kprobe_override:1, /* Do we override a kprobe? */
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index a68ced28d8f4..6706ecc25d8f 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -162,7 +162,7 @@ struct net {
#if IS_ENABLED(CONFIG_CAN)
struct netns_can can;
#endif
-#ifdef CONFIG_XDP_SOCKETS
+#ifdef CONFIG_BPF_SYSCALL
struct netns_xdp xdp;
#endif
struct sock *diag_nlsk;
diff --git a/include/net/netns/xdp.h b/include/net/netns/xdp.h
index e5734261ba0a..4935dfe1cf43 100644
--- a/include/net/netns/xdp.h
+++ b/include/net/netns/xdp.h
@@ -4,10 +4,21 @@
#include <linux/rculist.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>
+
+struct bpf_dtab;
+
+struct bpf_dtab_container {
+ struct bpf_dtab __rcu *dtab;
+ atomic_t refcnt;
+};
struct netns_xdp {
+#ifdef CONFIG_XDP_SOCKETS
struct mutex lock;
struct hlist_head list;
+#endif
+ struct bpf_dtab_container default_map;
};
#endif /* __NETNS_XDP_H__ */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 1037fc08c504..e55707e62b60 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -56,6 +56,9 @@
(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
#define DEV_MAP_BULK_SIZE 16
+#define DEV_MAP_DEFAULT_SIZE 8
+#define BPF_MAX_REFCNT 32768
+
struct xdp_bulk_queue {
struct xdp_frame *q[DEV_MAP_BULK_SIZE];
struct net_device *dev_rx;
@@ -80,6 +83,7 @@ struct bpf_dtab {
static DEFINE_SPINLOCK(dev_map_lock);
static LIST_HEAD(dev_map_list);
+static atomic_t global_redirect_use = {};
static u64 dev_map_bitmap_size(const union bpf_attr *attr)
{
@@ -332,6 +336,18 @@ struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key)
return obj;
}
+/* This is only being called from xdp_do_redirect() if the xdp_redirect helper
+ * is used; the default map is allocated on XDP program load if the helper is
+ * used, so will always be available at this point.
+ */
+struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
+{
+ struct net *net = dev_net(dev);
+ struct bpf_dtab *dtab = rcu_dereference(net->xdp.default_map.dtab);
+
+ return dtab ? &dtab->map : NULL;
+}
+
/* Runs under RCU-read-side, plus in softirq under NAPI protection.
* Thus, safe percpu variable access.
*/
@@ -533,14 +549,210 @@ const struct bpf_map_ops dev_map_ops = {
.map_check_btf = map_check_no_btf,
};
+static inline struct net *bpf_default_map_to_net(struct bpf_dtab_container *cont)
+{
+ struct netns_xdp *xdp = container_of(cont, struct netns_xdp, default_map);
+
+ return container_of(xdp, struct net, xdp);
+}
+
+static void __dev_map_release_default_map(struct bpf_dtab_container *cont)
+{
+ struct bpf_dtab *dtab = NULL;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ dtab = rcu_dereference(cont->dtab);
+ if (dtab) {
+ list_del_rcu(&dtab->list);
+ rcu_assign_pointer(cont->dtab, NULL);
+ bpf_clear_redirect_map(&dtab->map);
+ call_rcu(&dtab->rcu, __dev_map_free);
+ }
+}
+
+void dev_map_put_default_map(struct net *net)
+{
+ if (atomic_dec_and_test(&net->xdp.default_map.refcnt)) {
+ spin_lock(&dev_map_lock);
+ __dev_map_release_default_map(&net->xdp.default_map);
+ spin_unlock(&dev_map_lock);
+ }
+}
+
+static int __init_default_map(struct bpf_dtab_container *cont)
+{
+ struct net *net = bpf_default_map_to_net(cont);
+ struct bpf_dtab *dtab, *old_dtab;
+ int size = DEV_MAP_DEFAULT_SIZE;
+ struct net_device *netdev;
+ union bpf_attr attr = {};
+ u32 idx;
+ int err;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ if (!atomic_read(&global_redirect_use))
+ return 0;
+
+ for_each_netdev(net, netdev)
+ if (netdev->ifindex >= size)
+ size <<= 1;
+
+ old_dtab = rcu_dereference(cont->dtab);
+ if (old_dtab && old_dtab->map.max_entries == size)
+ return 0;
+
+ dtab = kzalloc(sizeof(*dtab), GFP_USER);
+ if (!dtab)
+ return -ENOMEM;
+
+ attr.map_type = BPF_MAP_TYPE_DEVMAP;
+ attr.max_entries = size;
+ attr.value_size = 4;
+ attr.key_size = 4;
+
+ err = dev_map_init_map(dtab, &attr, false);
+ if (err) {
+ kfree(dtab);
+ return err;
+ }
+
+ for_each_netdev(net, netdev) {
+ idx = netdev->ifindex;
+ err = __dev_map_update_elem(net, &dtab->map, &idx, &idx, 0);
+ if (err) {
+ __dev_map_free(&dtab->rcu);
+ return err;
+ }
+ }
+
+ rcu_assign_pointer(cont->dtab, dtab);
+ list_add_tail_rcu(&dtab->list, &dev_map_list);
+
+ if (old_dtab) {
+ list_del_rcu(&old_dtab->list);
+ bpf_clear_redirect_map(&old_dtab->map);
+ call_rcu(&old_dtab->rcu, __dev_map_free);
+ }
+
+ return 0;
+}
+
+static int maybe_inc_refcnt(atomic_t *v)
+{
+ int refcnt;
+
+ refcnt = atomic_inc_return(v);
+ if (refcnt > BPF_MAX_REFCNT) {
+ atomic_dec(v);
+ return -EBUSY;
+ }
+
+ return refcnt;
+}
+
+int dev_map_ensure_default_map(struct net *net)
+{
+ int refcnt, err = 0;
+
+ refcnt = maybe_inc_refcnt(&net->xdp.default_map.refcnt);
+ if (refcnt < 0)
+ return refcnt;
+
+ if (refcnt == 1) {
+ spin_lock(&dev_map_lock);
+ err = __init_default_map(&net->xdp.default_map);
+ spin_unlock(&dev_map_lock);
+ }
+
+ return err;
+}
+
+static void __dev_map_dec_redirect_count(void)
+{
+ struct net *net;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ if (atomic_dec_and_test(&global_redirect_use))
+ for_each_net_rcu(net)
+ __dev_map_release_default_map(&net->xdp.default_map);
+}
+
+void dev_map_dec_redirect_count(void)
+{
+ spin_lock(&dev_map_lock);
+ __dev_map_dec_redirect_count();
+ spin_unlock(&dev_map_lock);
+}
+
+static int __dev_map_init_redirect_use(void)
+{
+ struct net *net;
+ int err;
+
+ lockdep_assert_held(&dev_map_lock);
+
+ for_each_net_rcu(net) {
+ if (atomic_read(&net->xdp.default_map.refcnt)) {
+ err = __init_default_map(&net->xdp.default_map);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+int dev_map_inc_redirect_count(void)
+{
+ int refcnt, err = 0;
+
+ spin_lock(&dev_map_lock);
+ refcnt = maybe_inc_refcnt(&global_redirect_use);
+ if (refcnt < 0) {
+ err = refcnt;
+ goto out;
+ }
+
+ if (refcnt == 1)
+ err = __dev_map_init_redirect_use();
+
+ if (err)
+ __dev_map_dec_redirect_count();
+
+ out:
+ spin_unlock(&dev_map_lock);
+ return err;
+}
+
static int dev_map_notification(struct notifier_block *notifier,
ulong event, void *ptr)
{
struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
+ struct net *net = dev_net(netdev);
+ u32 idx = netdev->ifindex;
struct bpf_dtab *dtab;
- int i;
+ int i, err;
switch (event) {
+ case NETDEV_REGISTER:
+ rcu_read_lock();
+ dtab = rcu_dereference(net->xdp.default_map.dtab);
+ if (dtab) {
+ err = __dev_map_update_elem(net, &dtab->map,
+ &idx, &idx, 0);
+ if (err == -E2BIG) {
+ spin_lock(&dev_map_lock);
+ err = __init_default_map(&net->xdp.default_map);
+ if (err)
+ net_warn_ratelimited("Unable to re-allocate default map, xdp_redirect() may fail on some ifindexes\n");
+ spin_unlock(&dev_map_lock);
+ }
+ }
+ rcu_read_unlock();
+ break;
case NETDEV_UNREGISTER:
/* This rcu_read_lock/unlock pair is needed because
* dev_map_list is an RCU list AND to ensure a delete
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ec7c552af76b..fd1b76f5da2d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1265,6 +1265,9 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
kvfree(prog->aux->func_info);
bpf_prog_free_linfo(prog);
+ if (prog->redirect_used)
+ dev_map_dec_redirect_count();
+
call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
}
}
@@ -1962,6 +1965,21 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
#define BPF_PROG_GET_FD_BY_ID_LAST_FIELD prog_id
+struct bpf_prog *bpf_prog_get_by_id(u32 id)
+{
+ struct bpf_prog *prog;
+
+ spin_lock_bh(&prog_idr_lock);
+ prog = idr_find(&prog_idr, id);
+ if (prog)
+ prog = bpf_prog_inc_not_zero(prog);
+ else
+ prog = ERR_PTR(-ENOENT);
+ spin_unlock_bh(&prog_idr_lock);
+
+ return prog;
+}
+
static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
{
struct bpf_prog *prog;
@@ -1974,14 +1992,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- spin_lock_bh(&prog_idr_lock);
- prog = idr_find(&prog_idr, id);
- if (prog)
- prog = bpf_prog_inc_not_zero(prog);
- else
- prog = ERR_PTR(-ENOENT);
- spin_unlock_bh(&prog_idr_lock);
-
+ prog = bpf_prog_get_by_id(id);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1b9496c41383..f1b2f01e7ca1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7609,6 +7609,17 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
prog->dst_needed = 1;
if (insn->imm == BPF_FUNC_get_prandom_u32)
bpf_user_rnd_init_once();
+ if (insn->imm == BPF_FUNC_redirect) {
+ prog->redirect_needed = true;
+ if (!prog->redirect_used) {
+ int err = dev_map_inc_redirect_count();
+
+ if (err)
+ return err;
+ prog->redirect_used = true;
+ }
+ }
+
if (insn->imm == BPF_FUNC_override_return)
prog->kprobe_override = 1;
if (insn->imm == BPF_FUNC_tail_call) {
@@ -7618,6 +7629,7 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
* the program array.
*/
prog->cb_access = 1;
+ prog->redirect_needed = true;
env->prog->aux->stack_depth = MAX_BPF_STACK;
env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b67f2aa59dd..1df20d529026 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7990,6 +7990,21 @@ u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
return xdp.prog_id;
}
+static struct bpf_prog *dev_xdp_get_prog(struct net_device *dev)
+{
+ struct bpf_prog *prog;
+ u32 prog_id;
+
+ prog_id = __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf, XDP_QUERY_PROG);
+ if (prog_id) {
+ prog = bpf_prog_get_by_id(prog_id);
+ if (!IS_ERR(prog))
+ return prog;
+ }
+
+ return NULL;
+}
+
static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
struct netlink_ext_ack *extack, u32 flags,
struct bpf_prog *prog)
@@ -8024,9 +8039,18 @@ static void dev_xdp_uninstall(struct net_device *dev)
memset(&xdp, 0, sizeof(xdp));
xdp.command = XDP_QUERY_PROG;
WARN_ON(ndo_bpf(dev, &xdp));
- if (xdp.prog_id)
+ if (xdp.prog_id) {
+ struct bpf_prog *prog = bpf_prog_get_by_id(xdp.prog_id);
+
+ if (!IS_ERR(prog)) {
+ if (prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+ bpf_prog_put(prog);
+ }
+
WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
NULL));
+ }
/* Remove HW offload */
memset(&xdp, 0, sizeof(xdp));
@@ -8091,6 +8115,23 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
bpf_prog_put(prog);
return -EINVAL;
}
+
+ if (!offload && bpf_op == ops->ndo_bpf &&
+ prog->redirect_needed) {
+ err = dev_map_ensure_default_map(dev_net(dev));
+ if (err) {
+ NL_SET_ERR_MSG(extack, "unable to allocate default map for xdp_redirect()");
+ return err;
+ }
+ }
+ } else {
+ struct bpf_prog *old_prog = dev_xdp_get_prog(dev);
+
+ if (old_prog) {
+ if (old_prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+ bpf_prog_put(old_prog);
+ }
}
err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
@@ -9333,6 +9374,7 @@ EXPORT_SYMBOL(unregister_netdev);
int dev_change_net_namespace(struct net_device *dev, struct net *net, const char *pat)
{
int err, new_nsid, new_ifindex;
+ struct bpf_prog *prog = NULL;
ASSERT_RTNL();
@@ -9350,6 +9392,15 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
if (net_eq(dev_net(dev), net))
goto out;
+ prog = dev_xdp_get_prog(dev);
+ if (prog) {
+ if (prog->redirect_needed)
+ err = dev_map_ensure_default_map(net);
+
+ if (err)
+ goto out;
+ }
+
/* Pick the destination device name, and ensure
* we can use it in the destination network namespace.
*/
@@ -9388,6 +9439,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
+ if (prog && prog->redirect_needed)
+ dev_map_put_default_map(dev_net(dev));
+
new_nsid = peernet2id_alloc(dev_net(dev), net);
/* If there is an ifindex conflict assign a new one */
if (__dev_get_by_index(net, dev->ifindex))
@@ -9435,6 +9489,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
synchronize_net();
err = 0;
out:
+ if (prog)
+ bpf_prog_put(prog);
+
return err;
}
EXPORT_SYMBOL_GPL(dev_change_net_namespace);
diff --git a/net/core/filter.c b/net/core/filter.c
index 5132c054c981..be02ea103d05 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3337,58 +3337,6 @@ static const struct bpf_func_proto bpf_xdp_adjust_meta_proto = {
.arg2_type = ARG_ANYTHING,
};
-static int __bpf_tx_xdp(struct net_device *dev,
- struct bpf_map *map,
- struct xdp_buff *xdp,
- u32 index)
-{
- struct xdp_frame *xdpf;
- int err, sent;
-
- if (!dev->netdev_ops->ndo_xdp_xmit) {
- return -EOPNOTSUPP;
- }
-
- err = xdp_ok_fwd_dev(dev, xdp->data_end - xdp->data);
- if (unlikely(err))
- return err;
-
- xdpf = convert_to_xdp_frame(xdp);
- if (unlikely(!xdpf))
- return -EOVERFLOW;
-
- sent = dev->netdev_ops->ndo_xdp_xmit(dev, 1, &xdpf, XDP_XMIT_FLUSH);
- if (sent <= 0)
- return sent;
- return 0;
-}
-
-static noinline int
-xdp_do_redirect_slow(struct net_device *dev, struct xdp_buff *xdp,
- struct bpf_prog *xdp_prog, struct bpf_redirect_info *ri)
-{
- struct net_device *fwd;
- u32 index = ri->ifindex;
- int err;
-
- fwd = dev_get_by_index_rcu(dev_net(dev), index);
- ri->ifindex = 0;
- if (unlikely(!fwd)) {
- err = -EINVAL;
- goto err;
- }
-
- err = __bpf_tx_xdp(fwd, NULL, xdp, 0);
- if (unlikely(err))
- goto err;
-
- _trace_xdp_redirect(dev, xdp_prog, index);
- return 0;
-err:
- _trace_xdp_redirect_err(dev, xdp_prog, index, err);
- return err;
-}
-
static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
struct bpf_map *map,
struct xdp_buff *xdp,
@@ -3519,10 +3467,10 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct bpf_map *map = READ_ONCE(ri->map);
- if (likely(map))
- return xdp_do_redirect_map(dev, xdp, xdp_prog, map, ri);
+ if (unlikely(!map))
+ map = __dev_map_get_default_map(dev);
- return xdp_do_redirect_slow(dev, xdp, xdp_prog, ri);
+ return xdp_do_redirect_map(dev, xdp, xdp_prog, map, ri);
}
EXPORT_SYMBOL_GPL(xdp_do_redirect);
^ permalink raw reply related
* [PATCH net-next v2 1/3] xdp: Refactor devmap code in preparation for subsequent additions
From: Toke Høiland-Jørgensen @ 2019-02-28 13:25 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136028377.3381.2072266362746015640.stgit@alrua-x1>
The subsequent commits introducing default maps and a hash-based ifindex
devmap require a bit of refactoring of the devmap code. Perform this first
so the subsequent commits become easier to read.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
kernel/bpf/devmap.c | 177 +++++++++++++++++++++++++++++++--------------------
1 file changed, 109 insertions(+), 68 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 191b79948424..1037fc08c504 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -75,6 +75,7 @@ struct bpf_dtab {
struct bpf_dtab_netdev **netdev_map;
unsigned long __percpu *flush_needed;
struct list_head list;
+ struct rcu_head rcu;
};
static DEFINE_SPINLOCK(dev_map_lock);
@@ -85,23 +86,11 @@ static u64 dev_map_bitmap_size(const union bpf_attr *attr)
return BITS_TO_LONGS((u64) attr->max_entries) * sizeof(unsigned long);
}
-static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
+static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
+ bool check_memlock)
{
- struct bpf_dtab *dtab;
- int err = -EINVAL;
u64 cost;
-
- if (!capable(CAP_NET_ADMIN))
- return ERR_PTR(-EPERM);
-
- /* check sanity of attributes */
- if (attr->max_entries == 0 || attr->key_size != 4 ||
- attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
- return ERR_PTR(-EINVAL);
-
- dtab = kzalloc(sizeof(*dtab), GFP_USER);
- if (!dtab)
- return ERR_PTR(-ENOMEM);
+ int err;
bpf_map_init_from_attr(&dtab->map, attr);
@@ -109,60 +98,72 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
cost += dev_map_bitmap_size(attr) * num_possible_cpus();
if (cost >= U32_MAX - PAGE_SIZE)
- goto free_dtab;
+ return -EINVAL;
dtab->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
- /* if map size is larger than memlock limit, reject it early */
- err = bpf_map_precharge_memlock(dtab->map.pages);
- if (err)
- goto free_dtab;
-
- err = -ENOMEM;
+ if (check_memlock) {
+ /* if map size is larger than memlock limit, reject it early */
+ err = bpf_map_precharge_memlock(dtab->map.pages);
+ if (err)
+ return -EINVAL;
+ }
/* A per cpu bitfield with a bit per possible net device */
dtab->flush_needed = __alloc_percpu_gfp(dev_map_bitmap_size(attr),
__alignof__(unsigned long),
GFP_KERNEL | __GFP_NOWARN);
if (!dtab->flush_needed)
- goto free_dtab;
+ goto err_alloc;
dtab->netdev_map = bpf_map_area_alloc(dtab->map.max_entries *
sizeof(struct bpf_dtab_netdev *),
dtab->map.numa_node);
if (!dtab->netdev_map)
- goto free_dtab;
+ goto err_map;
- spin_lock(&dev_map_lock);
- list_add_tail_rcu(&dtab->list, &dev_map_list);
- spin_unlock(&dev_map_lock);
+ return 0;
- return &dtab->map;
-free_dtab:
+ err_map:
free_percpu(dtab->flush_needed);
- kfree(dtab);
- return ERR_PTR(err);
+ err_alloc:
+ return -ENOMEM;
}
-static void dev_map_free(struct bpf_map *map)
+static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
{
- struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
- int i, cpu;
+ struct bpf_dtab *dtab;
+ int err;
- /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
- * so the programs (can be more than one that used this map) were
- * disconnected from events. Wait for outstanding critical sections in
- * these programs to complete. The rcu critical section only guarantees
- * no further reads against netdev_map. It does __not__ ensure pending
- * flush operations (if any) are complete.
- */
+ if (!capable(CAP_NET_ADMIN))
+ return ERR_PTR(-EPERM);
+
+ /* check sanity of attributes */
+ if (attr->max_entries == 0 || attr->key_size != 4 ||
+ attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
+ return ERR_PTR(-EINVAL);
+
+ dtab = kzalloc(sizeof(*dtab), GFP_USER);
+ if (!dtab)
+ return ERR_PTR(-ENOMEM);
+
+ err = dev_map_init_map(dtab, attr, true);
+ if (err) {
+ kfree(dtab);
+ return ERR_PTR(err);
+ }
spin_lock(&dev_map_lock);
- list_del_rcu(&dtab->list);
+ list_add_tail_rcu(&dtab->list, &dev_map_list);
spin_unlock(&dev_map_lock);
- bpf_clear_redirect_map(map);
- synchronize_rcu();
+ return &dtab->map;
+}
+
+static void __dev_map_free(struct rcu_head *rcu)
+{
+ struct bpf_dtab *dtab = container_of(rcu, struct bpf_dtab, rcu);
+ int i, cpu;
/* To ensure all pending flush operations have completed wait for flush
* bitmap to indicate all flush_needed bits to be zero on _all_ cpus.
@@ -192,6 +193,26 @@ static void dev_map_free(struct bpf_map *map)
kfree(dtab);
}
+static void dev_map_free(struct bpf_map *map)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+
+ /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+ * so the programs (can be more than one that used this map) were
+ * disconnected from events. Wait for outstanding critical sections in
+ * these programs to complete. The rcu critical section only guarantees
+ * no further reads against netdev_map. It does __not__ ensure pending
+ * flush operations (if any) are complete.
+ */
+
+ spin_lock(&dev_map_lock);
+ list_del_rcu(&dtab->list);
+ spin_unlock(&dev_map_lock);
+
+ bpf_clear_redirect_map(map);
+ call_rcu(&dtab->rcu, __dev_map_free);
+}
+
static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
{
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
@@ -429,12 +450,42 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
return 0;
}
-static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
- u64 map_flags)
+static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
+ struct bpf_dtab *dtab,
+ u32 ifindex,
+ unsigned int bit)
{
- struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
- struct net *net = current->nsproxy->net_ns;
gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
+ struct bpf_dtab_netdev *dev;
+
+ dev = kmalloc_node(sizeof(*dev), gfp, dtab->map.numa_node);
+ if (!dev)
+ return ERR_PTR(-ENOMEM);
+
+ dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
+ sizeof(void *), gfp);
+ if (!dev->bulkq) {
+ kfree(dev);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ dev->dev = dev_get_by_index(net, ifindex);
+ if (!dev->dev) {
+ free_percpu(dev->bulkq);
+ kfree(dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ dev->bit = bit;
+ dev->dtab = dtab;
+
+ return dev;
+}
+
+static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
+ void *key, void *value, u64 map_flags)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
struct bpf_dtab_netdev *dev, *old_dev;
u32 i = *(u32 *)key;
u32 ifindex = *(u32 *)value;
@@ -449,26 +500,9 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
if (!ifindex) {
dev = NULL;
} else {
- dev = kmalloc_node(sizeof(*dev), gfp, map->numa_node);
- if (!dev)
- return -ENOMEM;
-
- dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
- sizeof(void *), gfp);
- if (!dev->bulkq) {
- kfree(dev);
- return -ENOMEM;
- }
-
- dev->dev = dev_get_by_index(net, ifindex);
- if (!dev->dev) {
- free_percpu(dev->bulkq);
- kfree(dev);
- return -EINVAL;
- }
-
- dev->bit = i;
- dev->dtab = dtab;
+ dev = __dev_map_alloc_node(net, dtab, ifindex, i);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
}
/* Use call_rcu() here to ensure rcu critical sections have completed
@@ -482,6 +516,13 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
return 0;
}
+static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
+ u64 map_flags)
+{
+ return __dev_map_update_elem(current->nsproxy->net_ns,
+ map, key, value, map_flags);
+}
+
const struct bpf_map_ops dev_map_ops = {
.map_alloc = dev_map_alloc,
.map_free = dev_map_free,
^ permalink raw reply related
* [PATCH net-next v2 3/3] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-28 13:25 UTC (permalink / raw)
To: David Miller
Cc: netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, Jakub Kicinski
In-Reply-To: <155136028377.3381.2072266362746015640.stgit@alrua-x1>
A common pattern when using xdp_redirect_map() is to create a device map
where the lookup key is simply ifindex. Because device maps are arrays,
this leaves holes in the map, and the map has to be sized to fit the
largest ifindex, regardless of how many devices actually are actually
needed in the map.
This patch adds a second type of device map where the key is interpreted as
an ifindex and looked up using a hashmap, instead of being used as an array
index. This leads to maps being densely packed, so they can be smaller.
The default maps used by xdp_redirect() are changed to use the new map
type, which means that xdp_redirect() is no longer limited to ifindex < 64,
but instead to 64 total simultaneous interfaces per network namespace. This
also provides an easy way to compare the performance of devmap and
devmap_idx:
xdp_redirect_map (devmap): 8394560 pkt/s
xdp_redirect (devmap_idx): 8179480 pkt/s
Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 12 +-
include/linux/bpf_types.h | 1
include/trace/events/xdp.h | 3
include/uapi/linux/bpf.h | 1
kernel/bpf/devmap.c | 232 ++++++++++++++++++++++++++++++-
kernel/bpf/verifier.c | 2
net/core/filter.c | 11 +
tools/bpf/bpftool/map.c | 1
tools/include/uapi/linux/bpf.h | 1
tools/lib/bpf/libbpf_probes.c | 1
tools/testing/selftests/bpf/test_maps.c | 16 ++
11 files changed, 265 insertions(+), 16 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b0f28989ccd7..532cae85de4e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -614,12 +614,13 @@ struct xdp_buff;
struct sk_buff;
struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
+struct bpf_dtab_netdev *__dev_map_idx_lookup_elem(struct bpf_map *map, u32 key);
struct bpf_map *__dev_map_get_default_map(struct net_device *dev);
int dev_map_ensure_default_map(struct net *net);
void dev_map_put_default_map(struct net *net);
int dev_map_inc_redirect_count(void);
void dev_map_dec_redirect_count(void);
-void __dev_map_insert_ctx(struct bpf_map *map, u32 index);
+void __dev_map_insert_ctx(struct bpf_map *map, struct bpf_dtab_netdev *dst);
void __dev_map_flush(struct bpf_map *map);
int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
struct net_device *dev_rx);
@@ -705,6 +706,12 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
return NULL;
}
+static inline struct net_device *__dev_map_idx_lookup_elem(struct bpf_map *map,
+ u32 key)
+{
+ return NULL;
+}
+
static inline struct bpf_map *__dev_map_get_default_map(struct net_device *dev)
{
return NULL;
@@ -727,7 +734,8 @@ static inline void dev_map_dec_redirect_count(void)
{
}
-static inline void __dev_map_insert_ctx(struct bpf_map *map, u32 index)
+static inline void __dev_map_insert_ctx(struct bpf_map *map,
+ struct bpf_dtab_netdev *dst)
{
}
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 08bf2f1fe553..374c013ca243 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -59,6 +59,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
#ifdef CONFIG_NET
BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
+BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_IDX, dev_map_idx_ops)
#if defined(CONFIG_BPF_STREAM_PARSER)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index e95cb86b65cf..fcf006d49f67 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -147,7 +147,8 @@ struct _bpf_dtab_netdev {
#define devmap_ifindex(fwd, map) \
(!fwd ? 0 : \
- ((map->map_type == BPF_MAP_TYPE_DEVMAP) ? \
+ ((map->map_type == BPF_MAP_TYPE_DEVMAP || \
+ map->map_type == BPF_MAP_TYPE_DEVMAP_IDX) ? \
((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0))
#define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..2cdd384f2cbc 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -132,6 +132,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
+ BPF_MAP_TYPE_DEVMAP_IDX,
};
/* Note that tracing related programs such as
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index e55707e62b60..95b276104dcd 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -46,6 +46,12 @@
* notifier hook walks the map we know that new dev references can not be
* added by the user because core infrastructure ensures dev_get_by_index()
* calls will fail at this point.
+ *
+ * The devmap_idx type is a map type which interprets keys as ifindexes and
+ * indexes these using a hashmap. This allows maps that use ifindex as key to be
+ * densely packed instead of having holes in the lookup array for unused
+ * ifindexes. The setup and packet enqueue/send code is shared between the two
+ * types of devmap; only the lookup and insertion is different.
*/
#include <linux/bpf.h>
#include <net/xdp.h>
@@ -67,6 +73,8 @@ struct xdp_bulk_queue {
struct bpf_dtab_netdev {
struct net_device *dev; /* must be first member, due to tracepoint */
+ unsigned int ifindex;
+ struct hlist_node index_hlist;
struct bpf_dtab *dtab;
unsigned int bit;
struct xdp_bulk_queue __percpu *bulkq;
@@ -79,12 +87,30 @@ struct bpf_dtab {
unsigned long __percpu *flush_needed;
struct list_head list;
struct rcu_head rcu;
+
+ /* these are only used for DEVMAP_IDX type maps */
+ unsigned long *bits_used;
+ struct hlist_head *dev_index_head;
+ spinlock_t index_lock;
};
static DEFINE_SPINLOCK(dev_map_lock);
static LIST_HEAD(dev_map_list);
static atomic_t global_redirect_use = {};
+static struct hlist_head *dev_map_create_hash(void)
+{
+ int i;
+ struct hlist_head *hash;
+
+ hash = kmalloc_array(NETDEV_HASHENTRIES, sizeof(*hash), GFP_KERNEL);
+ if (hash != NULL)
+ for (i = 0; i < NETDEV_HASHENTRIES; i++)
+ INIT_HLIST_HEAD(&hash[i]);
+
+ return hash;
+}
+
static u64 dev_map_bitmap_size(const union bpf_attr *attr)
{
return BITS_TO_LONGS((u64) attr->max_entries) * sizeof(unsigned long);
@@ -101,6 +127,11 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
/* make sure page count doesn't overflow */
cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
cost += dev_map_bitmap_size(attr) * num_possible_cpus();
+
+ if (attr->map_type == BPF_MAP_TYPE_DEVMAP_IDX)
+ cost += dev_map_bitmap_size(attr) +
+ sizeof(struct hlist_head) * NETDEV_HASHENTRIES;
+
if (cost >= U32_MAX - PAGE_SIZE)
return -EINVAL;
@@ -126,8 +157,25 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
if (!dtab->netdev_map)
goto err_map;
+ if (attr->map_type == BPF_MAP_TYPE_DEVMAP_IDX) {
+ dtab->bits_used = kzalloc(dev_map_bitmap_size(attr),
+ GFP_KERNEL);
+ if (!dtab->bits_used)
+ goto err_bitmap;
+
+ dtab->dev_index_head = dev_map_create_hash();
+ if (!dtab->dev_index_head)
+ goto err_idx;
+
+ spin_lock_init(&dtab->index_lock);
+ }
+
return 0;
+ err_idx:
+ kfree(dtab->bits_used);
+ err_bitmap:
+ bpf_map_area_free(dtab->netdev_map);
err_map:
free_percpu(dtab->flush_needed);
err_alloc:
@@ -192,6 +240,8 @@ static void __dev_map_free(struct rcu_head *rcu)
kfree(dev);
}
+ kfree(dtab->dev_index_head);
+ kfree(dtab->bits_used);
free_percpu(dtab->flush_needed);
bpf_map_area_free(dtab->netdev_map);
kfree(dtab);
@@ -234,12 +284,76 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
return 0;
}
-void __dev_map_insert_ctx(struct bpf_map *map, u32 bit)
+static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
+ int ifindex)
+{
+ return &dtab->dev_index_head[ifindex & (NETDEV_HASHENTRIES - 1)];
+}
+
+struct bpf_dtab_netdev *__dev_map_idx_lookup_elem(struct bpf_map *map, u32 key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct hlist_head *head = dev_map_index_hash(dtab, key);
+ struct bpf_dtab_netdev *dev;
+
+ hlist_for_each_entry_rcu(dev, head, index_hlist)
+ if (dev->ifindex == key)
+ return dev;
+
+ return NULL;
+}
+
+static int dev_map_idx_get_next_key(struct bpf_map *map, void *key,
+ void *next_key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ u32 ifindex, *next = next_key;
+ struct bpf_dtab_netdev *dev, *next_dev;
+ struct hlist_head *head;
+ int i = 0;
+
+ if (!key)
+ goto find_first;
+
+ ifindex = *(u32 *)key;
+
+ dev = __dev_map_idx_lookup_elem(map, ifindex);
+ if (!dev)
+ goto find_first;
+
+ next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&dev->index_hlist)),
+ struct bpf_dtab_netdev, index_hlist);
+
+ if (next_dev) {
+ *next = next_dev->ifindex;
+ return 0;
+ }
+
+ i = ifindex & (NETDEV_HASHENTRIES - 1);
+ i++;
+
+ find_first:
+ for (; i < NETDEV_HASHENTRIES; i++) {
+ head = dev_map_index_hash(dtab, i);
+
+ next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
+ struct bpf_dtab_netdev,
+ index_hlist);
+ if (next_dev) {
+ *next = next_dev->ifindex;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+void __dev_map_insert_ctx(struct bpf_map *map, struct bpf_dtab_netdev *dst)
{
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
unsigned long *bitmap = this_cpu_ptr(dtab->flush_needed);
- __set_bit(bit, bitmap);
+ __set_bit(dst->bit, bitmap);
}
static int bq_xmit_all(struct bpf_dtab_netdev *obj,
@@ -409,9 +523,16 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
{
struct bpf_dtab_netdev *obj = __dev_map_lookup_elem(map, *(u32 *)key);
- struct net_device *dev = obj ? obj->dev : NULL;
- return dev ? &dev->ifindex : NULL;
+ return obj ? &obj->ifindex : NULL;
+}
+
+static void *dev_map_idx_lookup_elem(struct bpf_map *map, void *key)
+{
+ struct bpf_dtab_netdev *obj = __dev_map_idx_lookup_elem(map,
+ *(u32 *)key);
+
+ return obj ? &obj->ifindex : NULL;
}
static void dev_map_flush_old(struct bpf_dtab_netdev *dev)
@@ -466,6 +587,43 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
return 0;
}
+static int dev_map_idx_delete_elem(struct bpf_map *map, void *key)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *old_dev;
+ int k = *(u32 *)key;
+
+ old_dev = __dev_map_idx_lookup_elem(map, k);
+ if (!old_dev)
+ return 0;
+
+ spin_lock(&dtab->index_lock);
+ hlist_del_rcu(&old_dev->index_hlist);
+ spin_unlock(&dtab->index_lock);
+
+ xchg(&dtab->netdev_map[old_dev->bit], NULL);
+ clear_bit_unlock(old_dev->bit, dtab->bits_used);
+ call_rcu(&old_dev->rcu, __dev_map_entry_free);
+ return 0;
+}
+
+static bool __dev_map_find_bit(struct bpf_dtab *dtab, unsigned int *bit)
+{
+ unsigned int b = 0;
+
+ retry:
+ b = find_next_zero_bit(dtab->bits_used, dtab->map.max_entries, b);
+
+ if (b >= dtab->map.max_entries)
+ return false;
+
+ if (test_and_set_bit_lock(b, dtab->bits_used))
+ goto retry;
+
+ *bit = b;
+ return true;
+}
+
static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
struct bpf_dtab *dtab,
u32 ifindex,
@@ -492,6 +650,7 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
return ERR_PTR(-EINVAL);
}
+ dev->ifindex = dev->dev->ifindex;
dev->bit = bit;
dev->dtab = dtab;
@@ -539,6 +698,49 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
map, key, value, map_flags);
}
+static int __dev_map_idx_update_elem(struct net *net, struct bpf_map *map,
+ void *key, void *value, u64 map_flags)
+{
+ struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
+ struct bpf_dtab_netdev *dev, *old_dev;
+ u32 idx = *(u32 *)key;
+ u32 val = *(u32 *)value;
+ u32 bit;
+
+ if (idx != val)
+ return -EINVAL;
+ if (unlikely(map_flags > BPF_EXIST))
+ return -EINVAL;
+
+ old_dev = __dev_map_idx_lookup_elem(map, idx);
+ if (old_dev) {
+ if (map_flags & BPF_NOEXIST)
+ return -EEXIST;
+ else
+ return 0;
+ }
+
+ if (!__dev_map_find_bit(dtab, &bit))
+ return -ENOSPC;
+ dev = __dev_map_alloc_node(net, dtab, idx, bit);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ xchg(&dtab->netdev_map[bit], dev);
+ spin_lock(&dtab->index_lock);
+ hlist_add_head_rcu(&dev->index_hlist,
+ dev_map_index_hash(dtab, dev->ifindex));
+ spin_unlock(&dtab->index_lock);
+ return 0;
+}
+
+static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
+ u64 map_flags)
+{
+ return __dev_map_idx_update_elem(current->nsproxy->net_ns,
+ map, key, value, map_flags);
+}
+
const struct bpf_map_ops dev_map_ops = {
.map_alloc = dev_map_alloc,
.map_free = dev_map_free,
@@ -549,6 +751,16 @@ const struct bpf_map_ops dev_map_ops = {
.map_check_btf = map_check_no_btf,
};
+const struct bpf_map_ops dev_map_idx_ops = {
+ .map_alloc = dev_map_alloc,
+ .map_free = dev_map_free,
+ .map_get_next_key = dev_map_idx_get_next_key,
+ .map_lookup_elem = dev_map_idx_lookup_elem,
+ .map_update_elem = dev_map_idx_update_elem,
+ .map_delete_elem = dev_map_idx_delete_elem,
+ .map_check_btf = map_check_no_btf,
+};
+
static inline struct net *bpf_default_map_to_net(struct bpf_dtab_container *cont)
{
struct netns_xdp *xdp = container_of(cont, struct netns_xdp, default_map);
@@ -583,8 +795,8 @@ void dev_map_put_default_map(struct net *net)
static int __init_default_map(struct bpf_dtab_container *cont)
{
struct net *net = bpf_default_map_to_net(cont);
+ int size = DEV_MAP_DEFAULT_SIZE, i = 0;
struct bpf_dtab *dtab, *old_dtab;
- int size = DEV_MAP_DEFAULT_SIZE;
struct net_device *netdev;
union bpf_attr attr = {};
u32 idx;
@@ -596,7 +808,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
return 0;
for_each_netdev(net, netdev)
- if (netdev->ifindex >= size)
+ if (++i >= size)
size <<= 1;
old_dtab = rcu_dereference(cont->dtab);
@@ -607,7 +819,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
if (!dtab)
return -ENOMEM;
- attr.map_type = BPF_MAP_TYPE_DEVMAP;
+ attr.map_type = BPF_MAP_TYPE_DEVMAP_IDX;
attr.max_entries = size;
attr.value_size = 4;
attr.key_size = 4;
@@ -620,7 +832,7 @@ static int __init_default_map(struct bpf_dtab_container *cont)
for_each_netdev(net, netdev) {
idx = netdev->ifindex;
- err = __dev_map_update_elem(net, &dtab->map, &idx, &idx, 0);
+ err = __dev_map_idx_update_elem(net, &dtab->map, &idx, &idx, 0);
if (err) {
__dev_map_free(&dtab->rcu);
return err;
@@ -741,8 +953,8 @@ static int dev_map_notification(struct notifier_block *notifier,
rcu_read_lock();
dtab = rcu_dereference(net->xdp.default_map.dtab);
if (dtab) {
- err = __dev_map_update_elem(net, &dtab->map,
- &idx, &idx, 0);
+ err = __dev_map_idx_update_elem(net, &dtab->map,
+ &idx, &idx, 0);
if (err == -E2BIG) {
spin_lock(&dev_map_lock);
err = __init_default_map(&net->xdp.default_map);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f1b2f01e7ca1..5bdcb3c23014 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2576,6 +2576,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
* for now.
*/
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
if (func_id != BPF_FUNC_redirect_map)
goto error;
break;
@@ -2648,6 +2649,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
break;
case BPF_FUNC_redirect_map:
if (map->map_type != BPF_MAP_TYPE_DEVMAP &&
+ map->map_type != BPF_MAP_TYPE_DEVMAP_IDX &&
map->map_type != BPF_MAP_TYPE_CPUMAP &&
map->map_type != BPF_MAP_TYPE_XSKMAP)
goto error;
diff --git a/net/core/filter.c b/net/core/filter.c
index be02ea103d05..59f2e188e5ed 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3345,13 +3345,14 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
int err;
switch (map->map_type) {
- case BPF_MAP_TYPE_DEVMAP: {
+ case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX: {
struct bpf_dtab_netdev *dst = fwd;
err = dev_map_enqueue(dst, xdp, dev_rx);
if (unlikely(err))
return err;
- __dev_map_insert_ctx(map, index);
+ __dev_map_insert_ctx(map, dst);
break;
}
case BPF_MAP_TYPE_CPUMAP: {
@@ -3384,6 +3385,7 @@ void xdp_do_flush_map(void)
if (map) {
switch (map->map_type) {
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
__dev_map_flush(map);
break;
case BPF_MAP_TYPE_CPUMAP:
@@ -3404,6 +3406,8 @@ static inline void *__xdp_map_lookup_elem(struct bpf_map *map, u32 index)
switch (map->map_type) {
case BPF_MAP_TYPE_DEVMAP:
return __dev_map_lookup_elem(map, index);
+ case BPF_MAP_TYPE_DEVMAP_IDX:
+ return __dev_map_idx_lookup_elem(map, index);
case BPF_MAP_TYPE_CPUMAP:
return __cpu_map_lookup_elem(map, index);
case BPF_MAP_TYPE_XSKMAP:
@@ -3494,7 +3498,8 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
goto err;
}
- if (map->map_type == BPF_MAP_TYPE_DEVMAP) {
+ if (map->map_type == BPF_MAP_TYPE_DEVMAP ||
+ map->map_type == BPF_MAP_TYPE_DEVMAP_IDX) {
struct bpf_dtab_netdev *dst = fwd;
err = dev_map_generic_redirect(dst, skb, xdp_prog);
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e0c650d91784..0864ce33df94 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -37,6 +37,7 @@ const char * const map_type_name[] = {
[BPF_MAP_TYPE_ARRAY_OF_MAPS] = "array_of_maps",
[BPF_MAP_TYPE_HASH_OF_MAPS] = "hash_of_maps",
[BPF_MAP_TYPE_DEVMAP] = "devmap",
+ [BPF_MAP_TYPE_DEVMAP_IDX] = "devmap_idx",
[BPF_MAP_TYPE_SOCKMAP] = "sockmap",
[BPF_MAP_TYPE_CPUMAP] = "cpumap",
[BPF_MAP_TYPE_XSKMAP] = "xskmap",
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..2cdd384f2cbc 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -132,6 +132,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
BPF_MAP_TYPE_QUEUE,
BPF_MAP_TYPE_STACK,
+ BPF_MAP_TYPE_DEVMAP_IDX,
};
/* Note that tracing related programs such as
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 8c3a1c04dcb2..b87b760a1355 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -172,6 +172,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
case BPF_MAP_TYPE_ARRAY_OF_MAPS:
case BPF_MAP_TYPE_HASH_OF_MAPS:
case BPF_MAP_TYPE_DEVMAP:
+ case BPF_MAP_TYPE_DEVMAP_IDX:
case BPF_MAP_TYPE_SOCKMAP:
case BPF_MAP_TYPE_CPUMAP:
case BPF_MAP_TYPE_XSKMAP:
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 3c627771f965..63681e4647f9 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -519,6 +519,21 @@ static void test_devmap(unsigned int task, void *data)
close(fd);
}
+static void test_devmap_idx(unsigned int task, void *data)
+{
+ int fd;
+ __u32 key, value;
+
+ fd = bpf_create_map(BPF_MAP_TYPE_DEVMAP_IDX, sizeof(key), sizeof(value),
+ 2, 0);
+ if (fd < 0) {
+ printf("Failed to create devmap_idx '%s'!\n", strerror(errno));
+ exit(1);
+ }
+
+ close(fd);
+}
+
static void test_queuemap(unsigned int task, void *data)
{
const int MAP_SIZE = 32;
@@ -1686,6 +1701,7 @@ static void run_all_tests(void)
test_arraymap_percpu_many_keys();
test_devmap(0, NULL);
+ test_devmap_idx(0, NULL);
test_sockmap(0, NULL);
test_map_large();
^ permalink raw reply related
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