* Re: [iproute PATCH 51/51] lib/bpf: Check return value of write()
From: Daniel Borkmann @ 2017-08-15 13:00 UTC (permalink / raw)
To: David Laight, 'Phil Sutter', Stephen Hemminger
Cc: netdev@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0055BA8@AcuExch.aculab.com>
On 08/15/2017 02:31 PM, David Laight wrote:
[...]
> WTF is this code doing anyway?
> write() is a system call, fflush() writes out any data buffered in the
> stdio stream.
> If there was anything buffered you'd want to output it earlier.
> Otherwise if it is going to use fflush() it should be using fwrite().
>
> I presume the function is allowed to write to stderr - since in general
> library functions shouldn't assume fd 0/1/2 or stdin/out/err are valid.
> There is a lot of code out there that does close(0); close(1); close(2);
> but leaves stdout/err valid. Call printf() instead of sprint() and eventually
> 10k of data gets written somewhere rather unexpected.
>
> If it is a copy loop, what is wrong with the last byte of buff[].
> It is valid for write() to return a partial length - the code should
> probably loop until all the data is accepted (or error).
Just send a patch if you really care; would have probably been faster
than typing up your email. ;) Thank you!
^ permalink raw reply
* [PATCH v4 iproute2 3/7] rdma: Add link object
From: Leon Romanovsky @ 2017-08-15 13:00 UTC (permalink / raw)
To: Doug Ledford, Stephen Hemminger
Cc: linux-rdma, Leon Romanovsky, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-1-leonro@mellanox.com>
Link (port) object represent struct ib_port to the user space.
Link properties:
* Port capabilities
* IB subnet prefix
* LID, SM_LID and LMC
* Port state
* Physical state
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/Makefile | 2 +-
rdma/link.c | 274 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
rdma/rdma.c | 3 +-
rdma/utils.c | 5 ++
4 files changed, 282 insertions(+), 2 deletions(-)
create mode 100644 rdma/link.c
diff --git a/rdma/Makefile b/rdma/Makefile
index 123d7ac5..1a9e4b1a 100644
--- a/rdma/Makefile
+++ b/rdma/Makefile
@@ -2,7 +2,7 @@ include ../Config
ifeq ($(HAVE_MNL),y)
-RDMA_OBJ = rdma.o utils.o dev.o
+RDMA_OBJ = rdma.o utils.o dev.o link.o
TARGETS=rdma
CFLAGS += $(shell $(PKG_CONFIG) libmnl --cflags)
diff --git a/rdma/link.c b/rdma/link.c
new file mode 100644
index 00000000..51858965
--- /dev/null
+++ b/rdma/link.c
@@ -0,0 +1,274 @@
+/*
+ * link.c RDMA tool
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Leon Romanovsky <leonro@mellanox.com>
+ */
+
+#include "rdma.h"
+
+static int link_help(struct rd *rd)
+{
+ pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename);
+ return 0;
+}
+
+static const char *caps_to_str(uint32_t idx)
+{
+ uint64_t cap = 1 << idx;
+
+ switch (cap) {
+ case RDMA_PORT_SM: return "SM";
+ case RDMA_PORT_NOTICE: return "NOTICE";
+ case RDMA_PORT_TRAP: return "TRAP";
+ case RDMA_PORT_OPT_IPD: return "OPT_IPD";
+ case RDMA_PORT_AUTO_MIGR: return "AUTO_MIG";
+ case RDMA_PORT_SL_MAP: return "SL_MAP";
+ case RDMA_PORT_MKEY_NVRAM: return "MKEY_NVRAM";
+ case RDMA_PORT_PKEY_NVRAM: return "PKEY_NVRAM";
+ case RDMA_PORT_LED_INFO: return "LED_INFO";
+ case RDMA_PORT_SM_DISABLED: return "SM_DISABLED";
+ case RDMA_PORT_SYS_IMAGE_GUID: return "SYS_IMAGE_GUID";
+ case RDMA_PORT_PKEY_SW_EXT_PORT_TRAP: return "PKEY_SW_EXT_PORT_TRAP";
+ case RDMA_PORT_EXTENDED_SPEEDS: return "EXTENDED_SPEEDS";
+ case RDMA_PORT_CM: return "CM";
+ case RDMA_PORT_SNMP_TUNNEL: return "SNMP_TUNNEL";
+ case RDMA_PORT_REINIT: return "REINIT";
+ case RDMA_PORT_DEVICE_MGMT: return "DEVICE_MGMT";
+ case RDMA_PORT_VENDOR_CLASS: return "VENDOR_CLASS";
+ case RDMA_PORT_DR_NOTICE: return "PORT_DR_NOTICE";
+ case RDMA_PORT_CAP_MASK_NOTICE: return "CAP_MASK_NOTICE";
+ case RDMA_PORT_BOOT_MGMT: return "BOOT_MGMT";
+ case RDMA_PORT_LINK_LATENCY: return "LINK_LATENCY";
+ case RDMA_PORT_CLIENT_REG: return "CLIENT_REG";
+ case RDMA_PORT_IP_BASED_GIDS: return "IP_BASED_GIDS";
+ default: return "UKNOWN";
+ }
+};
+
+static void link_print_caps(struct nlattr **tb)
+{
+ uint64_t caps;
+ uint32_t idx;
+
+ if (!tb[RDMA_NLDEV_ATTR_CAP_FLAGS])
+ return;
+
+ caps = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_CAP_FLAGS]);
+
+ pr_out("\n caps: <");
+ for (idx = 0; caps; idx++) {
+ if (caps & 0x1) {
+ pr_out("%s", caps_to_str(idx));
+ if (caps >> 0x1)
+ pr_out(", ");
+ }
+ caps >>= 0x1;
+ }
+
+ pr_out(">");
+}
+
+static void link_print_subnet_prefix(struct nlattr **tb)
+{
+ uint64_t subnet_prefix;
+
+ if (!tb[RDMA_NLDEV_ATTR_SUBNET_PREFIX])
+ return;
+
+ subnet_prefix = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_SUBNET_PREFIX]);
+ rd_print_u64("subnet_prefix", subnet_prefix);
+}
+
+static void link_print_lid(struct nlattr **tb)
+{
+ if (!tb[RDMA_NLDEV_ATTR_LID])
+ return;
+
+ pr_out("lid %u ",
+ mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_LID]));
+}
+
+static void link_print_sm_lid(struct nlattr **tb)
+{
+ if (!tb[RDMA_NLDEV_ATTR_SM_LID])
+ return;
+
+ pr_out("sm_lid %u ",
+ mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_SM_LID]));
+}
+
+static void link_print_lmc(struct nlattr **tb)
+{
+ if (!tb[RDMA_NLDEV_ATTR_LMC])
+ return;
+
+ pr_out("lmc %u ", mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_LMC]));
+}
+
+static const char *link_state_to_str(uint8_t link_state)
+{
+ switch (link_state) {
+ case RDMA_LINK_STATE_NOP: return "NOP";
+ case RDMA_LINK_STATE_DOWN: return "DOWN";
+ case RDMA_LINK_STATE_INIT: return "INIT";
+ case RDMA_LINK_STATE_ARMED: return "ARMED";
+ case RDMA_LINK_STATE_ACTIVE: return "ACTIVE";
+ case RDMA_LINK_STATE_ACTIVE_DEFER: return "ACTIVE_DEFER";
+ default: return "UKNOWN";
+ }
+};
+
+static void link_print_state(struct nlattr **tb)
+{
+ uint8_t state;
+
+ if (!tb[RDMA_NLDEV_ATTR_PORT_STATE])
+ return;
+
+ state = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_PORT_STATE]);
+ pr_out("state %s ", link_state_to_str(state));
+}
+
+static const char *phys_state_to_str(uint8_t phys_state)
+{
+ switch (phys_state) {
+ case RDMA_LINK_PHYS_STATE_SLEEP: return "SLEEP";
+ case RDMA_LINK_PHYS_STATE_POLLING: return "POLLING";
+ case RDMA_LINK_PHYS_STATE_DISABLED: return "DISABLED";
+ case RDMA_LINK_PHYS_STATE_PORT_CONFIGURATION_TRAINING: return "ARMED";
+ case RDMA_LINK_PHYS_STATE_LINK_UP: return "LINK_UP";
+ case RDMA_LINK_PHYS_STATE_LINK_ERROR_RECOVER:
+ return "LINK_ERROR_RECOVER";
+ case RDMA_LINK_PHYS_STATE_LINK_PHY_TEST: return "PHY_TEST";
+ default: return "UKNOWN";
+ }
+};
+
+static void link_print_phys_state(struct nlattr **tb)
+{
+ uint8_t phys_state;
+
+ if (!tb[RDMA_NLDEV_ATTR_PORT_PHYS_STATE])
+ return;
+
+ phys_state = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_PORT_PHYS_STATE]);
+ pr_out("physical_state %s ", phys_state_to_str(phys_state));
+}
+
+static int link_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+ struct rd *rd = data;
+
+ mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+ if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] || !tb[RDMA_NLDEV_ATTR_DEV_NAME])
+ return MNL_CB_ERROR;
+
+ if (!tb[RDMA_NLDEV_ATTR_PORT_INDEX]) {
+ pr_err("This tool doesn't support switches yet\n");
+ return MNL_CB_ERROR;
+ }
+
+ pr_out("%u/%u: %s/%u: ",
+ mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]),
+ mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]),
+ mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]),
+ mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]));
+ link_print_subnet_prefix(tb);
+ link_print_lid(tb);
+ link_print_sm_lid(tb);
+ link_print_lmc(tb);
+ link_print_state(tb);
+ link_print_phys_state(tb);
+ if (rd->show_details)
+ link_print_caps(tb);
+
+ pr_out("\n");
+ return MNL_CB_OK;
+}
+
+static int link_no_args(struct rd *rd)
+{
+ uint32_t seq;
+ int ret;
+
+ rd_prepare_msg(rd, RDMA_NLDEV_CMD_PORT_GET, &seq,
+ (NLM_F_REQUEST | NLM_F_ACK));
+ mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
+ mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_PORT_INDEX, rd->port_idx);
+ ret = rd_send_msg(rd);
+ if (ret)
+ return ret;
+
+ return rd_recv_msg(rd, link_parse_cb, rd, seq);
+}
+
+static int link_one_show(struct rd *rd)
+{
+ const struct rd_cmd cmds[] = {
+ { NULL, link_no_args},
+ { 0 }
+ };
+
+ return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+static int link_show(struct rd *rd)
+{
+ struct dev_map *dev_map;
+ uint32_t port;
+ int ret;
+
+ if (rd_no_arg(rd)) {
+ list_for_each_entry(dev_map, &rd->dev_map_list, list) {
+ rd->dev_idx = dev_map->idx;
+ for (port = 1; port < dev_map->num_ports + 1; port++) {
+ rd->port_idx = port;
+ ret = link_one_show(rd);
+ if (ret)
+ return ret;
+ }
+ }
+
+ } else {
+ dev_map = dev_map_lookup(rd, true);
+ port = get_port_from_argv(rd);
+ if (!dev_map || port > dev_map->num_ports) {
+ pr_err("Wrong device name\n");
+ return -ENOENT;
+ }
+ rd_arg_inc(rd);
+ rd->dev_idx = dev_map->idx;
+ rd->port_idx = port ? : 1;
+ for (; rd->port_idx < dev_map->num_ports + 1; rd->port_idx++) {
+ ret = link_one_show(rd);
+ if (ret)
+ return ret;
+ if (port)
+ /*
+ * We got request to show link for devname
+ * with port index.
+ */
+ break;
+ }
+ }
+ return 0;
+}
+
+int cmd_link(struct rd *rd)
+{
+ const struct rd_cmd cmds[] = {
+ { NULL, link_show },
+ { "show", link_show },
+ { "list", link_show },
+ { "help", link_help },
+ { 0 }
+ };
+
+ return rd_exec_cmd(rd, cmds, "link command");
+}
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 9c2bdc8f..74c09e8b 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -15,7 +15,7 @@
static void help(char *name)
{
pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
- "where OBJECT := { dev | help }\n"
+ "where OBJECT := { dev | link | help }\n"
" OPTIONS := { -V[ersion] | -d[etails]}\n", name);
}
@@ -31,6 +31,7 @@ static int rd_cmd(struct rd *rd)
{ NULL, cmd_help },
{ "help", cmd_help },
{ "dev", cmd_dev },
+ { "link", cmd_link },
{ 0 }
};
diff --git a/rdma/utils.c b/rdma/utils.c
index 0e32eefe..91d05271 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -107,6 +107,11 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_FW_VERSION] = MNL_TYPE_NUL_STRING,
[RDMA_NLDEV_ATTR_NODE_GUID] = MNL_TYPE_U64,
[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = MNL_TYPE_U64,
+ [RDMA_NLDEV_ATTR_LID] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_SM_LID] = MNL_TYPE_U32,
+ [RDMA_NLDEV_ATTR_LMC] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_PORT_STATE] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_PORT_PHYS_STATE] = MNL_TYPE_U8,
[RDMA_NLDEV_ATTR_DEV_NODE_TYPE] = MNL_TYPE_U8,
};
--
2.14.0
^ permalink raw reply related
* [PATCH v4 iproute2 1/7] rdma: Add basic infrastructure for RDMA tool
From: Leon Romanovsky @ 2017-08-15 13:00 UTC (permalink / raw)
To: Doug Ledford, Stephen Hemminger
Cc: linux-rdma, Leon Romanovsky, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-1-leonro@mellanox.com>
RDMA devices are cross-functional devices from one side,
but very tailored for the specific markets from another.
Such diversity caused to spread of RDMA related configuration
across various tools, e.g. devlink, ip, ethtool, ib specific and
vendor specific solutions.
This patch adds ability to fill device and port information
by reading RDMA netlink.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
Makefile | 2 +-
rdma/.gitignore | 1 +
rdma/Makefile | 22 ++++++
rdma/rdma.c | 116 ++++++++++++++++++++++++++++++
rdma/rdma.h | 74 +++++++++++++++++++
rdma/utils.c | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 431 insertions(+), 1 deletion(-)
create mode 100644 rdma/.gitignore
create mode 100644 rdma/Makefile
create mode 100644 rdma/rdma.c
create mode 100644 rdma/rdma.h
create mode 100644 rdma/utils.c
diff --git a/Makefile b/Makefile
index 1f88f7f5..dbb4a4af 100644
--- a/Makefile
+++ b/Makefile
@@ -49,7 +49,7 @@ WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2
CFLAGS := $(WFLAGS) $(CCOPTS) -I../include $(DEFINES) $(CFLAGS)
YACCFLAGS = -d -t -v
-SUBDIRS=lib ip tc bridge misc netem genl tipc devlink man
+SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma man
LIBNETLINK=../lib/libnetlink.a ../lib/libutil.a
LDLIBS += $(LIBNETLINK)
diff --git a/rdma/.gitignore b/rdma/.gitignore
new file mode 100644
index 00000000..51fb172b
--- /dev/null
+++ b/rdma/.gitignore
@@ -0,0 +1 @@
+rdma
diff --git a/rdma/Makefile b/rdma/Makefile
new file mode 100644
index 00000000..64da2142
--- /dev/null
+++ b/rdma/Makefile
@@ -0,0 +1,22 @@
+include ../Config
+
+ifeq ($(HAVE_MNL),y)
+
+RDMA_OBJ = rdma.o utils.o
+
+TARGETS=rdma
+CFLAGS += $(shell $(PKG_CONFIG) libmnl --cflags)
+LDLIBS += $(shell $(PKG_CONFIG) libmnl --libs)
+
+endif
+
+all: $(TARGETS) $(LIBS)
+
+rdma: $(RDMA_OBJ) $(LIBS)
+ $(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
+
+install: all
+ install -m 0755 $(TARGETS) $(DESTDIR)$(SBINDIR)
+
+clean:
+ rm -f $(RDMA_OBJ) $(TARGETS)
diff --git a/rdma/rdma.c b/rdma/rdma.c
new file mode 100644
index 00000000..d850e396
--- /dev/null
+++ b/rdma/rdma.c
@@ -0,0 +1,116 @@
+/*
+ * rdma.c RDMA tool
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Leon Romanovsky <leonro@mellanox.com>
+ */
+
+#include "rdma.h"
+#include "SNAPSHOT.h"
+
+static void help(char *name)
+{
+ pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
+ "where OBJECT := { help }\n"
+ " OPTIONS := { -V[ersion] | -d[etails]}\n", name);
+}
+
+static int cmd_help(struct rd *rd)
+{
+ help(rd->filename);
+ return 0;
+}
+
+static int rd_cmd(struct rd *rd)
+{
+ const struct rd_cmd cmds[] = {
+ { NULL, cmd_help },
+ { "help", cmd_help },
+ { 0 }
+ };
+
+ return rd_exec_cmd(rd, cmds, "object");
+}
+
+static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
+{
+ uint32_t seq;
+ int ret;
+
+ rd->filename = filename;
+ rd->argc = argc;
+ rd->argv = argv;
+ INIT_LIST_HEAD(&rd->dev_map_list);
+ rd->buff = malloc(MNL_SOCKET_BUFFER_SIZE);
+ if (!rd->buff)
+ return -ENOMEM;
+
+ rd_prepare_msg(rd, RDMA_NLDEV_CMD_GET,
+ &seq, (NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP));
+ ret = rd_send_msg(rd);
+ if (ret)
+ return ret;
+
+ return rd_recv_msg(rd, rd_dev_init_cb, rd, seq);
+}
+
+static void rd_free(struct rd *rd)
+{
+ free(rd->buff);
+ rd_free_devmap(rd);
+}
+
+int main(int argc, char **argv)
+{
+ static const struct option long_options[] = {
+ { "version", no_argument, NULL, 'V' },
+ { "help", no_argument, NULL, 'h' },
+ { "details", no_argument, NULL, 'd' },
+ { NULL, 0, NULL, 0 }
+ };
+ bool show_details = false;
+ char *filename;
+ struct rd rd;
+ int opt;
+ int err;
+
+ filename = basename(argv[0]);
+
+ while ((opt = getopt_long(argc, argv, "Vhd",
+ long_options, NULL)) >= 0) {
+ switch (opt) {
+ case 'V':
+ printf("%s utility, iproute2-ss%s\n",
+ filename, SNAPSHOT);
+ return EXIT_SUCCESS;
+ case 'd':
+ show_details = true;
+ break;
+ case 'h':
+ help(filename);
+ return EXIT_SUCCESS;
+ default:
+ pr_err("Unknown option.\n");
+ help(filename);
+ return EXIT_FAILURE;
+ }
+ }
+
+ argc -= optind;
+ argv += optind;
+
+ err = rd_init(&rd, argc, argv, filename);
+ if (err)
+ goto out;
+
+ rd.show_details = show_details;
+ err = rd_cmd(&rd);
+out:
+ /* Always cleanup */
+ rd_free(&rd);
+ return err ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/rdma/rdma.h b/rdma/rdma.h
new file mode 100644
index 00000000..361888b8
--- /dev/null
+++ b/rdma/rdma.h
@@ -0,0 +1,74 @@
+/*
+ * rdma.c RDMA tool
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Leon Romanovsky <leonro@mellanox.com>
+ */
+#ifndef _RDMA_TOOL_H_
+#define _RDMA_TOOL_H_
+
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <getopt.h>
+#include <libmnl/libmnl.h>
+#include <rdma/rdma_netlink.h>
+#include <time.h>
+#include <rdma/ib_user_verbs.h>
+#include <rdma/rdma_netlink.h>
+#include <rdma/rdma.h>
+
+#include "list.h"
+
+#define pr_err(args...) fprintf(stderr, ##args)
+#define pr_out(args...) fprintf(stdout, ##args)
+
+struct dev_map {
+ struct list_head list;
+ char *dev_name;
+ uint32_t num_ports;
+ uint32_t idx;
+};
+
+struct rd {
+ int argc;
+ char **argv;
+ char *filename;
+ bool show_details;
+ struct list_head dev_map_list;
+ struct mnl_socket *nl;
+ struct nlmsghdr *nlh;
+ char *buff;
+};
+
+struct rd_cmd {
+ const char *cmd;
+ int (*func)(struct rd *rd);
+};
+
+/*
+ * Parser interface
+ */
+bool rd_no_arg(struct rd *rd);
+void rd_arg_inc(struct rd *rd);
+
+int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
+
+/*
+ * Device manipulation
+ */
+void rd_free_devmap(struct rd *rd);
+
+/*
+ * Netlink
+ */
+int rd_send_msg(struct rd *rd);
+int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
+void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
+int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
+int rd_attr_cb(const struct nlattr *attr, void *data);
+#endif /* _RDMA_TOOL_H_ */
diff --git a/rdma/utils.c b/rdma/utils.c
new file mode 100644
index 00000000..9bd7418f
--- /dev/null
+++ b/rdma/utils.c
@@ -0,0 +1,217 @@
+/*
+ * utils.c RDMA tool
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Leon Romanovsky <leonro@mellanox.com>
+ */
+
+#include "rdma.h"
+
+static int rd_argc(struct rd *rd)
+{
+ return rd->argc;
+}
+
+static char *rd_argv(struct rd *rd)
+{
+ if (!rd_argc(rd))
+ return NULL;
+ return *rd->argv;
+}
+
+static int strcmpx(const char *str1, const char *str2)
+{
+ if (strlen(str1) > strlen(str2))
+ return -1;
+ return strncmp(str1, str2, strlen(str1));
+}
+
+static bool rd_argv_match(struct rd *rd, const char *pattern)
+{
+ if (!rd_argc(rd))
+ return false;
+ return strcmpx(rd_argv(rd), pattern) == 0;
+}
+
+void rd_arg_inc(struct rd *rd)
+{
+ if (!rd_argc(rd))
+ return;
+ rd->argc--;
+ rd->argv++;
+}
+
+bool rd_no_arg(struct rd *rd)
+{
+ return rd_argc(rd) == 0;
+}
+
+static struct dev_map *dev_map_alloc(const char *dev_name)
+{
+ struct dev_map *dev_map;
+
+ dev_map = calloc(1, sizeof(*dev_map));
+ if (!dev_map)
+ return NULL;
+ dev_map->dev_name = strdup(dev_name);
+
+ return dev_map;
+}
+
+static void dev_map_free(struct dev_map *dev_map)
+{
+ if (!dev_map)
+ return;
+
+ free(dev_map->dev_name);
+ free(dev_map);
+}
+
+static void dev_map_cleanup(struct rd *rd)
+{
+ struct dev_map *dev_map, *tmp;
+
+ list_for_each_entry_safe(dev_map, tmp,
+ &rd->dev_map_list, list) {
+ list_del(&dev_map->list);
+ dev_map_free(dev_map);
+ }
+}
+
+static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
+ [RDMA_NLDEV_ATTR_DEV_NAME] = MNL_TYPE_NUL_STRING,
+ [RDMA_NLDEV_ATTR_PORT_INDEX] = MNL_TYPE_U32,
+};
+
+int rd_attr_cb(const struct nlattr *attr, void *data)
+{
+ const struct nlattr **tb = data;
+ int type;
+
+ if (mnl_attr_type_valid(attr, RDMA_NLDEV_ATTR_MAX) < 0)
+ return MNL_CB_ERROR;
+
+ type = mnl_attr_get_type(attr);
+
+ if (mnl_attr_validate(attr, nldev_policy[type]) < 0)
+ return MNL_CB_ERROR;
+
+ tb[type] = attr;
+ return MNL_CB_OK;
+}
+
+int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+ struct dev_map *dev_map;
+ struct rd *rd = data;
+ const char *dev_name;
+
+ mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+ if (!tb[RDMA_NLDEV_ATTR_DEV_NAME] || !tb[RDMA_NLDEV_ATTR_DEV_INDEX])
+ return MNL_CB_ERROR;
+ if (!tb[RDMA_NLDEV_ATTR_PORT_INDEX]) {
+ pr_err("This tool doesn't support switches yet\n");
+ return MNL_CB_ERROR;
+ }
+
+ dev_name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+
+ dev_map = dev_map_alloc(dev_name);
+ if (!dev_map)
+ /* The main function will cleanup the allocations */
+ return MNL_CB_ERROR;
+ list_add_tail(&dev_map->list, &rd->dev_map_list);
+
+ dev_map->num_ports = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]);
+ dev_map->idx = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+ return MNL_CB_OK;
+}
+
+void rd_free_devmap(struct rd *rd)
+{
+ if (!rd)
+ return;
+ dev_map_cleanup(rd);
+}
+
+int rd_exec_cmd(struct rd *rd, const struct rd_cmd *cmds, const char *str)
+{
+ const struct rd_cmd *c;
+
+ /* First argument in objs table is default variant */
+ if (rd_no_arg(rd))
+ return cmds->func(rd);
+
+ for (c = cmds + 1; c->cmd; ++c) {
+ if (rd_argv_match(rd, c->cmd)) {
+ /* Move to next argument */
+ rd_arg_inc(rd);
+ return c->func(rd);
+ }
+ }
+
+ pr_err("Unknown %s '%s'.\n", str, rd_argv(rd));
+ return 0;
+}
+
+void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags)
+{
+ *seq = time(NULL);
+
+ rd->nlh = mnl_nlmsg_put_header(rd->buff);
+ rd->nlh->nlmsg_type = RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, cmd);
+ rd->nlh->nlmsg_seq = *seq;
+ rd->nlh->nlmsg_flags = flags;
+}
+
+int rd_send_msg(struct rd *rd)
+{
+ int ret;
+
+ rd->nl = mnl_socket_open(NETLINK_RDMA);
+ if (!rd->nl) {
+ pr_err("Failed to open NETLINK_RDMA socket\n");
+ return -ENODEV;
+ }
+
+ ret = mnl_socket_bind(rd->nl, 0, MNL_SOCKET_AUTOPID);
+ if (ret < 0) {
+ pr_err("Failed to bind socket with err %d\n", ret);
+ goto err;
+ }
+
+ ret = mnl_socket_sendto(rd->nl, rd->nlh, rd->nlh->nlmsg_len);
+ if (ret < 0) {
+ pr_err("Failed to send to socket with err %d\n", ret);
+ goto err;
+ }
+ return 0;
+
+err:
+ mnl_socket_close(rd->nl);
+ return ret;
+}
+
+int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
+{
+ int ret;
+ unsigned int portid;
+ char buf[MNL_SOCKET_BUFFER_SIZE];
+
+ portid = mnl_socket_get_portid(rd->nl);
+ do {
+ ret = mnl_socket_recvfrom(rd->nl, buf, sizeof(buf));
+ if (ret <= 0)
+ break;
+
+ ret = mnl_cb_run(buf, ret, seq, portid, callback, data);
+ } while (ret > 0);
+
+ mnl_socket_close(rd->nl);
+ return ret;
+}
--
2.14.0
^ permalink raw reply related
* [PATCH v4 iproute2 5/7] rdma: Add json and pretty outputs
From: Leon Romanovsky @ 2017-08-15 13:00 UTC (permalink / raw)
To: Doug Ledford, Stephen Hemminger
Cc: linux-rdma, Leon Romanovsky, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-1-leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.c | 31 ++++++++++++++++++++++++++++---
rdma/rdma.h | 4 ++++
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 74c09e8b..f9f4f2a2 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -16,7 +16,7 @@ static void help(char *name)
{
pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
"where OBJECT := { dev | link | help }\n"
- " OPTIONS := { -V[ersion] | -d[etails]}\n", name);
+ " OPTIONS := { -V[ersion] | -d[etails] | -j[son] | -p[retty]}\n", name);
}
static int cmd_help(struct rd *rd)
@@ -47,6 +47,16 @@ static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
rd->argc = argc;
rd->argv = argv;
INIT_LIST_HEAD(&rd->dev_map_list);
+
+ if (rd->json_output) {
+ rd->jw = jsonw_new(stdout);
+ if (!rd->jw) {
+ pr_err("Failed to create JSON writer\n");
+ return -ENOMEM;
+ }
+ jsonw_pretty(rd->jw, rd->pretty_output);
+ }
+
rd->buff = malloc(MNL_SOCKET_BUFFER_SIZE);
if (!rd->buff)
return -ENOMEM;
@@ -62,6 +72,8 @@ static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
static void rd_free(struct rd *rd)
{
+ if (rd->json_output)
+ jsonw_destroy(&rd->jw);
free(rd->buff);
rd_free_devmap(rd);
}
@@ -71,10 +83,14 @@ int main(int argc, char **argv)
static const struct option long_options[] = {
{ "version", no_argument, NULL, 'V' },
{ "help", no_argument, NULL, 'h' },
+ { "json", no_argument, NULL, 'j' },
+ { "pretty", no_argument, NULL, 'p' },
{ "details", no_argument, NULL, 'd' },
{ NULL, 0, NULL, 0 }
};
+ bool pretty_output = false;
bool show_details = false;
+ bool json_output = false;
char *filename;
struct rd rd;
int opt;
@@ -82,16 +98,22 @@ int main(int argc, char **argv)
filename = basename(argv[0]);
- while ((opt = getopt_long(argc, argv, "Vhd",
+ while ((opt = getopt_long(argc, argv, "Vhdpj",
long_options, NULL)) >= 0) {
switch (opt) {
case 'V':
printf("%s utility, iproute2-ss%s\n",
filename, SNAPSHOT);
return EXIT_SUCCESS;
+ case 'p':
+ pretty_output = true;
+ break;
case 'd':
show_details = true;
break;
+ case 'j':
+ json_output = true;
+ break;
case 'h':
help(filename);
return EXIT_SUCCESS;
@@ -105,11 +127,14 @@ int main(int argc, char **argv)
argc -= optind;
argv += optind;
+ rd.show_details = show_details;
+ rd.json_output = json_output;
+ rd.pretty_output = pretty_output;
+
err = rd_init(&rd, argc, argv, filename);
if (err)
goto out;
- rd.show_details = show_details;
err = rd_cmd(&rd);
out:
/* Always cleanup */
diff --git a/rdma/rdma.h b/rdma/rdma.h
index b9d75d29..620ec3db 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -23,6 +23,7 @@
#include <rdma/rdma.h>
#include "list.h"
+#include "json_writer.h"
#define pr_err(args...) fprintf(stderr, ##args)
#define pr_out(args...) fprintf(stdout, ##args)
@@ -45,6 +46,9 @@ struct rd {
struct mnl_socket *nl;
struct nlmsghdr *nlh;
char *buff;
+ json_writer_t *jw;
+ bool json_output;
+ bool pretty_output;
};
struct rd_cmd {
--
2.14.0
^ permalink raw reply related
* [PATCH v4 iproute2 6/7] rdma: Implement json output for dev object
From: Leon Romanovsky @ 2017-08-15 13:00 UTC (permalink / raw)
To: Doug Ledford, Stephen Hemminger
Cc: linux-rdma, Leon Romanovsky, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-1-leonro@mellanox.com>
The example output for machine with two devices
root@mtr-leonro:~# rdma dev -j -p
[{
"ifindex": 1,
"ifname": "mlx5_0",
"node_type": "ca",
"fw": "2.8.9999",
"node_guid": "5254:00c0:fe12:3457",
"sys_image_guid": 5254:00c0:fe12:3457",
"caps": [ "BAD_PKEY_CNTR", "BAD_QKEY_CNTR", "CHANGE_PHY_POR",
"PORT_ACTIVE_EVENT", "SYS_IMAGE_GUID", "RC_RNR_NAK_GEN",
"MEM_WINDOW", "UD_IP_CSUM", "UD_TSO", "XRC",
"MEM_MGT_EXTENSIONS", "BLOCK_MULTICAST_LOOPBACK",
"MEM_WINDOW_TYPE_2B", "RAW_IP_CSUM",
"MANAGED_FLOW_STEERING", "RESIZE_MAX_WR" ]
},{
"ifindex": 2,
"ifname": mlx5_1,
"node_type": "ca",
"fw": "2.8.9999",
"node_guid": "5254:00c0:fe12:3458",
"sys_image_guid": "5254:00c0:fe12:3458",
"caps": [ "BAD_PKEY_CNTR", "BAD_QKEY_CNTR", "CHANGE_PHY_POR",
"PORT_ACTIVE_EVENT", "SYS_IMAGE_GUID", "RC_RNR_NAK_GEN",
"MEM_WINDOW", "UD_IP_CSUM", "UD_TSO", "XRC",
"MEM_MGT_EXTENSIONS", "BLOCK_MULTICAST_LOOPBACK",
"MEM_WINDOW_TYPE_2B", "RAW_IP_CSUM",
"MANAGED_FLOW_STEERING", "RESIZE_MAX_WR" ]
}
]
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/dev.c | 110 +++++++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 82 insertions(+), 28 deletions(-)
diff --git a/rdma/dev.c b/rdma/dev.c
index e984f805..621c4808 100644
--- a/rdma/dev.c
+++ b/rdma/dev.c
@@ -61,7 +61,7 @@ static const char *dev_caps_to_str(uint32_t idx)
}
}
-static void dev_print_caps(struct nlattr **tb)
+static void dev_print_caps(struct rd *rd, struct nlattr **tb)
{
uint64_t caps;
uint32_t idx;
@@ -71,48 +71,78 @@ static void dev_print_caps(struct nlattr **tb)
caps = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_CAP_FLAGS]);
- pr_out("\n caps: <");
+ if (rd->json_output) {
+ jsonw_name(rd->jw, "caps");
+ jsonw_start_array(rd->jw);
+ } else {
+ pr_out("\n caps: <");
+ }
for (idx = 0; caps; idx++) {
if (caps & 0x1) {
- pr_out("%s", dev_caps_to_str(idx));
- if (caps >> 0x1)
- pr_out(", ");
+ if (rd->json_output) {
+ jsonw_string(rd->jw, dev_caps_to_str(idx));
+ } else {
+ pr_out("%s", dev_caps_to_str(idx));
+ if (caps >> 0x1)
+ pr_out(", ");
+ }
}
caps >>= 0x1;
}
- pr_out(">");
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
+ else
+ pr_out(">");
}
-static void dev_print_fw(struct nlattr **tb)
+static void dev_print_fw(struct rd *rd, struct nlattr **tb)
{
+ const char *str;
if (!tb[RDMA_NLDEV_ATTR_FW_VERSION])
return;
- pr_out("fw %s ",
- mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_FW_VERSION]));
+ str = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_FW_VERSION]);
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "fw", str);
+ else
+ pr_out("fw %s ", str);
}
-static void dev_print_node_guid(struct nlattr **tb)
+static void dev_print_node_guid(struct rd *rd, struct nlattr **tb)
{
uint64_t node_guid;
+ uint16_t vp[4];
+ char str[32];
if (!tb[RDMA_NLDEV_ATTR_NODE_GUID])
return;
node_guid = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_NODE_GUID]);
- rd_print_u64("node_guid", node_guid);
+ memcpy(vp, &node_guid, sizeof(uint64_t));
+ snprintf(str, 32, "%04x:%04x:%04x:%04x", vp[3], vp[2], vp[1], vp[0]);
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "node_guid", str);
+ else
+ pr_out("node_guid %s ", str);
}
-static void dev_print_sys_image_guid(struct nlattr **tb)
+static void dev_print_sys_image_guid(struct rd *rd, struct nlattr **tb)
{
uint64_t sys_image_guid;
+ uint16_t vp[4];
+ char str[32];
if (!tb[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID])
return;
sys_image_guid = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID]);
- rd_print_u64("sys_image_guid", sys_image_guid);
+ memcpy(vp, &sys_image_guid, sizeof(uint64_t));
+ snprintf(str, 32, "%04x:%04x:%04x:%04x", vp[3], vp[2], vp[1], vp[0]);
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "sys_image_guid", str);
+ else
+ pr_out("sys_image_guid %s ", str);
}
static const char *node_type_to_str(uint8_t node_type)
@@ -128,37 +158,51 @@ static const char *node_type_to_str(uint8_t node_type)
}
}
-static void dev_print_node_type(struct nlattr **tb)
+static void dev_print_node_type(struct rd *rd, struct nlattr **tb)
{
+ const char *node_str;
uint8_t node_type;
if (!tb[RDMA_NLDEV_ATTR_DEV_NODE_TYPE])
return;
node_type = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_DEV_NODE_TYPE]);
- pr_out("node_type %s ", node_type_to_str(node_type));
+ node_str = node_type_to_str(node_type);
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "node_type", node_str);
+ else
+ pr_out("node_type %s ", node_str);
}
static int dev_parse_cb(const struct nlmsghdr *nlh, void *data)
{
struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
struct rd *rd = data;
+ const char *name;
+ uint32_t idx;
mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] || !tb[RDMA_NLDEV_ATTR_DEV_NAME])
return MNL_CB_ERROR;
- pr_out("%u: %s: ",
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]),
- mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]));
- dev_print_node_type(tb);
- dev_print_fw(tb);
- dev_print_node_guid(tb);
- dev_print_sys_image_guid(tb);
+ idx = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+ name = mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]);
+ if (rd->json_output) {
+ jsonw_uint_field(rd->jw, "ifindex", idx);
+ jsonw_string_field(rd->jw, "ifname", name);
+ } else {
+ pr_out("%u: %s: ", idx, name);
+ }
+
+ dev_print_node_type(rd, tb);
+ dev_print_fw(rd, tb);
+ dev_print_node_guid(rd, tb);
+ dev_print_sys_image_guid(rd, tb);
if (rd->show_details)
- dev_print_caps(tb);
+ dev_print_caps(rd, tb);
- pr_out("\n");
+ if (!rd->json_output)
+ pr_out("\n");
return MNL_CB_OK;
}
@@ -174,7 +218,12 @@ static int dev_no_args(struct rd *rd)
if (ret)
return ret;
- return rd_recv_msg(rd, dev_parse_cb, rd, seq);
+ if (rd->json_output)
+ jsonw_start_object(rd->jw);
+ ret = rd_recv_msg(rd, dev_parse_cb, rd, seq);
+ if (rd->json_output)
+ jsonw_end_object(rd->jw);
+ return ret;
}
static int dev_one_show(struct rd *rd)
@@ -192,24 +241,29 @@ static int dev_show(struct rd *rd)
struct dev_map *dev_map;
int ret = 0;
+ if (rd->json_output)
+ jsonw_start_array(rd->jw);
if (rd_no_arg(rd)) {
list_for_each_entry(dev_map, &rd->dev_map_list, list) {
rd->dev_idx = dev_map->idx;
ret = dev_one_show(rd);
if (ret)
- return ret;
+ goto out;
}
-
} else {
dev_map = dev_map_lookup(rd, false);
if (!dev_map) {
pr_err("Wrong device name\n");
- return -ENOENT;
+ ret = -ENOENT;
+ goto out;
}
rd_arg_inc(rd);
rd->dev_idx = dev_map->idx;
ret = dev_one_show(rd);
}
+out:
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
return ret;
}
--
2.14.0
^ permalink raw reply related
* [PATCH v4 iproute2 7/7] rdma: Add json output to link object
From: Leon Romanovsky @ 2017-08-15 13:00 UTC (permalink / raw)
To: Doug Ledford, Stephen Hemminger
Cc: linux-rdma, Leon Romanovsky, Dennis Dalessandro, Jason Gunthorpe,
Jiri Pirko, Ariel Almog, Linux Netdev
In-Reply-To: <20170815130020.29509-1-leonro@mellanox.com>
An example for the JSON output for two devices system.
root@mtr-leonro:~# rdma link -d -p -j
[{
"ifindex": 1,
"port": 1,
"ifname": "mlx5_0/1",
"subnet_prefix": "fe80:0000:0000:0000",
"lid": 13399,
"sm_lid": 49151,
"lmc": 0,
"state": "ACTIVE",
"physical_state": "LINK_UP",
"caps": ["AUTO_MIG"
]
},{
"ifindex": 2,
"port": 1,
"ifname": "mlx5_1/1",
"subnet_prefix": "fe80:0000:0000:0000",
"lid": 13400,
"sm_lid": 49151,
"lmc": 0,
"state": "ACTIVE",
"physical_state": "LINK_UP",
"caps": ["AUTO_MIG"
]
}
]
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/link.c | 144 +++++++++++++++++++++++++++++++++++++++++++----------------
rdma/rdma.h | 1 -
rdma/utils.c | 8 ----
3 files changed, 105 insertions(+), 48 deletions(-)
diff --git a/rdma/link.c b/rdma/link.c
index 51858965..b3316e7e 100644
--- a/rdma/link.c
+++ b/rdma/link.c
@@ -50,7 +50,7 @@ static const char *caps_to_str(uint32_t idx)
}
};
-static void link_print_caps(struct nlattr **tb)
+static void link_print_caps(struct rd *rd, struct nlattr **tb)
{
uint64_t caps;
uint32_t idx;
@@ -60,54 +60,89 @@ static void link_print_caps(struct nlattr **tb)
caps = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_CAP_FLAGS]);
- pr_out("\n caps: <");
+ if (rd->json_output) {
+ jsonw_name(rd->jw, "caps");
+ jsonw_start_array(rd->jw);
+ } else {
+ pr_out("\n caps: <");
+ }
for (idx = 0; caps; idx++) {
if (caps & 0x1) {
- pr_out("%s", caps_to_str(idx));
- if (caps >> 0x1)
- pr_out(", ");
+ if (rd->json_output) {
+ jsonw_string(rd->jw, caps_to_str(idx));
+ } else {
+ pr_out("%s", caps_to_str(idx));
+ if (caps >> 0x1)
+ pr_out(", ");
+ }
}
caps >>= 0x1;
}
- pr_out(">");
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
+ else
+ pr_out(">");
}
-static void link_print_subnet_prefix(struct nlattr **tb)
+static void link_print_subnet_prefix(struct rd *rd, struct nlattr **tb)
{
uint64_t subnet_prefix;
+ uint16_t vp[4];
+ char str[32];
if (!tb[RDMA_NLDEV_ATTR_SUBNET_PREFIX])
return;
subnet_prefix = mnl_attr_get_u64(tb[RDMA_NLDEV_ATTR_SUBNET_PREFIX]);
- rd_print_u64("subnet_prefix", subnet_prefix);
+ memcpy(vp, &subnet_prefix, sizeof(uint64_t));
+ snprintf(str, 32, "%04x:%04x:%04x:%04x", vp[3], vp[2], vp[1], vp[0]);
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "subnet_prefix", str);
+ else
+ pr_out("subnet_prefix %s ", str);
}
-static void link_print_lid(struct nlattr **tb)
+static void link_print_lid(struct rd *rd, struct nlattr **tb)
{
+ uint32_t lid;
+
if (!tb[RDMA_NLDEV_ATTR_LID])
return;
- pr_out("lid %u ",
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_LID]));
+ lid = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_LID]);
+ if (rd->json_output)
+ jsonw_uint_field(rd->jw, "lid", lid);
+ else
+ pr_out("lid %u ", lid);
}
-static void link_print_sm_lid(struct nlattr **tb)
+static void link_print_sm_lid(struct rd *rd, struct nlattr **tb)
{
+ uint32_t sm_lid;
+
if (!tb[RDMA_NLDEV_ATTR_SM_LID])
return;
- pr_out("sm_lid %u ",
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_SM_LID]));
+ sm_lid = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_SM_LID]);
+ if (rd->json_output)
+ jsonw_uint_field(rd->jw, "sm_lid", sm_lid);
+ else
+ pr_out("sm_lid %u ", sm_lid);
}
-static void link_print_lmc(struct nlattr **tb)
+static void link_print_lmc(struct rd *rd, struct nlattr **tb)
{
+ uint8_t lmc;
+
if (!tb[RDMA_NLDEV_ATTR_LMC])
return;
- pr_out("lmc %u ", mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_LMC]));
+ lmc = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_LMC]);
+ if (rd->json_output)
+ jsonw_uint_field(rd->jw, "lmc", lmc);
+ else
+ pr_out("lmc %u ", lmc);
}
static const char *link_state_to_str(uint8_t link_state)
@@ -123,7 +158,7 @@ static const char *link_state_to_str(uint8_t link_state)
}
};
-static void link_print_state(struct nlattr **tb)
+static void link_print_state(struct rd *rd, struct nlattr **tb)
{
uint8_t state;
@@ -131,7 +166,10 @@ static void link_print_state(struct nlattr **tb)
return;
state = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_PORT_STATE]);
- pr_out("state %s ", link_state_to_str(state));
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "state", link_state_to_str(state));
+ else
+ pr_out("state %s ", link_state_to_str(state));
}
static const char *phys_state_to_str(uint8_t phys_state)
@@ -149,7 +187,7 @@ static const char *phys_state_to_str(uint8_t phys_state)
}
};
-static void link_print_phys_state(struct nlattr **tb)
+static void link_print_phys_state(struct rd *rd, struct nlattr **tb)
{
uint8_t phys_state;
@@ -157,13 +195,19 @@ static void link_print_phys_state(struct nlattr **tb)
return;
phys_state = mnl_attr_get_u8(tb[RDMA_NLDEV_ATTR_PORT_PHYS_STATE]);
- pr_out("physical_state %s ", phys_state_to_str(phys_state));
+ if (rd->json_output)
+ jsonw_string_field(rd->jw, "physical_state",
+ phys_state_to_str(phys_state));
+ else
+ pr_out("physical_state %s ", phys_state_to_str(phys_state));
}
static int link_parse_cb(const struct nlmsghdr *nlh, void *data)
{
struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
struct rd *rd = data;
+ uint32_t port, idx;
+ char name[32];
mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
if (!tb[RDMA_NLDEV_ATTR_DEV_INDEX] || !tb[RDMA_NLDEV_ATTR_DEV_NAME])
@@ -174,21 +218,31 @@ static int link_parse_cb(const struct nlmsghdr *nlh, void *data)
return MNL_CB_ERROR;
}
- pr_out("%u/%u: %s/%u: ",
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]),
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]),
- mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]),
- mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]));
- link_print_subnet_prefix(tb);
- link_print_lid(tb);
- link_print_sm_lid(tb);
- link_print_lmc(tb);
- link_print_state(tb);
- link_print_phys_state(tb);
+ idx = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
+ port = mnl_attr_get_u32(tb[RDMA_NLDEV_ATTR_PORT_INDEX]);
+ snprintf(name, 32, "%s/%u",
+ mnl_attr_get_str(tb[RDMA_NLDEV_ATTR_DEV_NAME]), port);
+
+ if (rd->json_output) {
+ jsonw_uint_field(rd->jw, "ifindex", idx);
+ jsonw_uint_field(rd->jw, "port", port);
+ jsonw_string_field(rd->jw, "ifname", name);
+
+ } else {
+ pr_out("%u/%u: %s: ", idx, port, name);
+ }
+
+ link_print_subnet_prefix(rd, tb);
+ link_print_lid(rd, tb);
+ link_print_sm_lid(rd, tb);
+ link_print_lmc(rd, tb);
+ link_print_state(rd, tb);
+ link_print_phys_state(rd, tb);
if (rd->show_details)
- link_print_caps(tb);
+ link_print_caps(rd, tb);
- pr_out("\n");
+ if (!rd->json_output)
+ pr_out("\n");
return MNL_CB_OK;
}
@@ -205,7 +259,12 @@ static int link_no_args(struct rd *rd)
if (ret)
return ret;
- return rd_recv_msg(rd, link_parse_cb, rd, seq);
+ if (rd->json_output)
+ jsonw_start_object(rd->jw);
+ ret = rd_recv_msg(rd, link_parse_cb, rd, seq);
+ if (rd->json_output)
+ jsonw_end_object(rd->jw);
+ return ret;
}
static int link_one_show(struct rd *rd)
@@ -222,8 +281,10 @@ static int link_show(struct rd *rd)
{
struct dev_map *dev_map;
uint32_t port;
- int ret;
+ int ret = 0;
+ if (rd->json_output)
+ jsonw_start_array(rd->jw);
if (rd_no_arg(rd)) {
list_for_each_entry(dev_map, &rd->dev_map_list, list) {
rd->dev_idx = dev_map->idx;
@@ -231,7 +292,7 @@ static int link_show(struct rd *rd)
rd->port_idx = port;
ret = link_one_show(rd);
if (ret)
- return ret;
+ goto out;
}
}
@@ -240,7 +301,8 @@ static int link_show(struct rd *rd)
port = get_port_from_argv(rd);
if (!dev_map || port > dev_map->num_ports) {
pr_err("Wrong device name\n");
- return -ENOENT;
+ ret = -ENOENT;
+ goto out;
}
rd_arg_inc(rd);
rd->dev_idx = dev_map->idx;
@@ -248,7 +310,7 @@ static int link_show(struct rd *rd)
for (; rd->port_idx < dev_map->num_ports + 1; rd->port_idx++) {
ret = link_one_show(rd);
if (ret)
- return ret;
+ goto out;
if (port)
/*
* We got request to show link for devname
@@ -257,7 +319,11 @@ static int link_show(struct rd *rd)
break;
}
}
- return 0;
+
+out:
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
+ return ret;
}
int cmd_link(struct rd *rd)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 620ec3db..4a9391c1 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -65,7 +65,6 @@ void rd_arg_inc(struct rd *rd);
char *rd_argv(struct rd *rd);
uint32_t get_port_from_argv(struct rd *rd);
-void rd_print_u64(char *name, uint64_t val);
/*
* Commands interface
*/
diff --git a/rdma/utils.c b/rdma/utils.c
index 91d05271..eb4377cf 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -59,14 +59,6 @@ uint32_t get_port_from_argv(struct rd *rd)
return slash ? atoi(slash + 1) : 0;
}
-void rd_print_u64(char *name, uint64_t val)
-{
- uint16_t vp[4];
-
- memcpy(vp, &val, sizeof(uint64_t));
- pr_out("%s %04x:%04x:%04x:%04x ", name, vp[3], vp[2], vp[1], vp[0]);
-}
-
static struct dev_map *dev_map_alloc(const char *dev_name)
{
struct dev_map *dev_map;
--
2.14.0
^ permalink raw reply related
* [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile
From: mohamedalrshah @ 2017-08-15 13:08 UTC (permalink / raw)
To: davem
Cc: netdev, torvalds, linux-kernel, Mohamed A . Alrshah,
Mohamed Othman, Borhanuddin Ali, Zurina Hanapi
This commit implements a new TCP congestion control algorithm, namely Agile-SD.
A detailed description of Agile-SD is published in the following 2 articles:
[1] "Agile-SD: a Linux-based TCP congestion control algorithm for supporting high-speed and short-distance networks",
Alrshah, M.A., Othman, M., Ali, B. and Hanapi, Z.M. Journal of Network and Computer Applications, Vol. 55, pages.181-190, May-June 2015.
[2] "Modeling the Throughput of the Linux-Based Agile-SD Transmission Control Protocol",
Alrshah, M.A., Othman, M., Ali, B. and Hanapi, Z.M.
IEEE Access journal, Vol. 4, pages.9724-9732, Jan 2017.
The Internet has predominantly used Reno or CUBIC, relying on packet loss as the
signal to slow down. While this worked well for many years, these congestion control
algorithms are unfortunately unable to present an acceptable bandwidth utilization
over today's networks. On today's Internet, data losses are very common and the
existing congestion control algorithms are very sensitive to packet losses causing
unwanted delay and retransmission of data. Also, the existing congestion control
algorithms shows long epochs which affect the general performance of TCP.
Agile-SD has significantly increased throughput, reduced sensitivity to packet loss,
and improved the ability to work with small and large buffers. Agile-SD shrinks the
needed epoch time to recover after data loss, which improves throughput up to 50% in
many cases, see [1,2].
Agile-SD is a Congestion Control Algorithm for High-speed Networks. Agile-SD is a
loss-based and RTT-independent TCP congestion control algorithm designed to support
high-speed networks. Agile-SD requires only changes on the sender side, not in the
network or the receiver side. So, it can be incrementally deployed on today's Internet
and/or in datacenters. Agile-SD introduces the Agility Factor Mechanism (AFM), which
allows Agile-SD to deal with small buffer sizes while reducing its sensitivity to packet
loss. Due to the use of this mechanism, Agile-SD improves the throughput of TCP up to 50%
compared to Cubic-TCP and Compound-TCP in many cases, especially when the buffer is small
and the data loss is common. Its performance was evaluated using simulation and testbed
to measure the average throughput, loss ratio, and fairness.
Our long-term goal is to improve the congestion control algorithms used on the Internet.
We are hopeful that Agile-SD can help advance the efforts toward this goal, and motivate
the community to do further research.
Signed-off-by: Mohamed A. Alrshah <mohamed.a.alrshah@ieee.org>
Signed-off-by: Mohamed Othman <mothman@upm.edu.my>
Signed-off-by: Borhanuddin Ali <borhan@upm.edu.my>
Signed-off-by: Zurina Hanapi <zurinamh@upm.edu.my>
---
net/ipv4/Kconfig | 15 ++++
net/ipv4/Makefile | 1 +
net/ipv4/tcp_agilesd.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 209 insertions(+)
create mode 100755 net/ipv4/tcp_agilesd.c
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 91a2557..474f72c 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -677,6 +677,17 @@ config TCP_CONG_BBR
bufferbloat, policers, or AQM schemes that do not provide a delay
signal. It requires the fq ("Fair Queue") pacing packet scheduler.
+config TCP_CONG_AGILESD
+ tristate "Agile-SD Congestion control"
+ default n
+ ---help---
+
+ This is version 1.0 of Agile-SD TCP. It is a sender-side only.
+ It contributes the Agility Factor (AF) to shorten the epoch time
+ and to make TCP independent from RTT. AF reduces the sensitivity
+ to packet losses, which in turn Agile-SD to achieve better throughput
+ over high-speed networks.
+
choice
prompt "Default TCP congestion control"
default DEFAULT_CUBIC
@@ -713,6 +724,9 @@ choice
config DEFAULT_BBR
bool "BBR" if TCP_CONG_BBR=y
+
+ config DEFAULT_AGILESD
+ bool "AGILESD" if TCP_CONG_AGILESD=y
config DEFAULT_RENO
bool "Reno"
@@ -738,6 +752,7 @@ config DEFAULT_TCP_CONG
default "dctcp" if DEFAULT_DCTCP
default "cdg" if DEFAULT_CDG
default "bbr" if DEFAULT_BBR
+ default "agilesd" if DEFAULT_AGILESD
default "cubic"
config TCP_MD5SIG
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index f83de23..33d398b 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_INET_UDP_DIAG) += udp_diag.o
obj-$(CONFIG_INET_RAW_DIAG) += raw_diag.o
obj-$(CONFIG_NET_TCPPROBE) += tcp_probe.o
obj-$(CONFIG_TCP_CONG_BBR) += tcp_bbr.o
+obj-$(CONFIG_TCP_CONG_AGILESD) += tcp_agilesd.o
obj-$(CONFIG_TCP_CONG_BIC) += tcp_bic.o
obj-$(CONFIG_TCP_CONG_CDG) += tcp_cdg.o
obj-$(CONFIG_TCP_CONG_CUBIC) += tcp_cubic.o
diff --git a/net/ipv4/tcp_agilesd.c b/net/ipv4/tcp_agilesd.c
new file mode 100755
index 0000000..5de4779
--- /dev/null
+++ b/net/ipv4/tcp_agilesd.c
@@ -0,0 +1,193 @@
+/* Agile-SD is a Loss-Based Congestion Control Algorithm for High-speed Networks.
+ * Agile-SD is a new loss-based and RTT-independent TCP congestion control algorithm
+ * designed to support high-speed networks. It introduces the Agility Factor Mechanism (AFM),
+ * which allows Agile-SD to deal with small buffer sizes while reducing its sensitivity to
+ * packet loss. Due to the use of this mechanism, Agile-SD improves the throughput of TCP
+ * up to 50% compared to Cubic-TCP and Compound-TCP in many cases, especially when the buffer
+ * is small and the data loss is common. Its performance was evaluated using simulation and
+ * testbed to measure the average throughput, loss ratio, and fairness.
+ *
+ * Agile-SD is described in detail in:
+ * "Agile-SD: a Linux-based TCP congestion control algorithm for supporting high-speed and short-distance networks",
+ * Alrshah, M.A., Othman, M., Ali, B. and Hanapi, Z.M.
+ * Journal of Network and Computer Applications, Vol. 55, pages.181-190, May-June 2015.
+ *
+ * Moreover, Agile-SD performance has been mathematically proven in:
+ * "Modeling the Throughput of the Linux-Based Agile-SD Transmission Control Protocol",
+ * Alrshah, M.A., Othman, M., Ali, B. and Hanapi, Z.M.
+ * IEEE Access journal, Vol. 4, pages.9724-9732, Jan 2017.
+ */
+
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/math64.h>
+#include <net/tcp.h>
+
+#define SCALE 1000 /* Scale factor to avoid fractions */
+#define Double_SCALE 1000000 /* Double_SCALE must be equal to SCALE^2 */
+#define beta 900 /* beta for multiplicative decrease */
+
+/* Agile-SD Parameters */
+struct agilesdtcp {
+ u32 loss_cwnd; /* congestion window at last loss.*/
+ u32 frac_tracer; /* This is to trace the fractions of the increment.*/
+ u32 degraded_loss_cwnd; /* loss_cwnd after degradation.*/
+ enum dystate{SS=0, CA=1} agilesd_tcp_status;
+};
+
+/* To reset the parameters if needed*/
+static inline void agilesdtcp_reset(struct sock *sk)
+{
+
+}
+
+/* This function is called after the first acknowledgment is received and before the congestion
+ * control algorithm will be called for the first time. If the congestion control algorithm has
+ * private data, it should initialize its private date here. */
+static void agilesdtcp_init(struct sock *sk)
+{
+ struct agilesdtcp *ca = inet_csk_ca(sk);
+
+ /* The value of initial_ssthresh parameter is not used here, thus, snd_ssthresh is initialized by a large value.*/
+ tcp_sk(sk)->snd_ssthresh = 0x7fffffff;
+
+ ca->loss_cwnd = 0;
+ ca->frac_tracer = 0;
+ ca->agilesd_tcp_status = SS;
+}
+
+/* This function is called whenever an ack is received and the congestion window can be increased.
+ * This is equivalent to opencwnd in tcp.cc.
+ * ack is the number of bytes that are acknowledged in the latest acknowledgment;
+ * rtt is the the rtt measured by the latest acknowledgment;
+ * in_flight is the packet in flight before the latest acknowledgment;
+ * good_ack is an indicator whether the current situation is normal (no duplicate ack, no loss and no SACK). */
+static void agilesdtcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct agilesdtcp *ca = inet_csk_ca(sk);
+ u32 inc_factor;
+ u32 ca_inc;
+ u32 current_gap, total_gap;
+ /* The value of inc_factor is limited by lower_fl and upper_fl.
+ * The lower_fl must be always = 1. The greater the upper_fl the higher the aggressiveness.
+ * But, if upper_fl set to 1, Agile-SD will work exactly as newreno.
+ * We have already designed an equation to calculate the optimum upper_fl based on the given beta.
+ * This equation will be revealed once its article is published*/
+ u32 lower_fl = 1 * SCALE;
+ u32 upper_fl = 3 * SCALE;
+
+ if (!tcp_is_cwnd_limited(sk)) return;
+
+ if (tp->snd_cwnd < tp->snd_ssthresh){
+ ca->agilesd_tcp_status = SS;
+ tcp_slow_start(tp, in_flight);
+ }
+ else {
+ ca->agilesd_tcp_status = CA;
+
+ if (ca->loss_cwnd > ca->degraded_loss_cwnd)
+ total_gap = ca->loss_cwnd - ca->degraded_loss_cwnd;
+ else
+ total_gap = 1;
+
+ if (ca->loss_cwnd > tp->snd_cwnd)
+ current_gap = ca->loss_cwnd - tp->snd_cwnd;
+ else
+ current_gap = 0;
+
+ inc_factor = min(max(((upper_fl * current_gap) / total_gap), lower_fl), upper_fl);
+
+ ca_inc = ((inc_factor * SCALE) / tp->snd_cwnd); /* SCALE is used to avoid fractions*/
+
+ ca->frac_tracer += ca_inc; /* This in order to take the fraction increase into account */
+ if (ca->frac_tracer >= Double_SCALE) /* To take factor scale into account */
+ {
+ tp->snd_cwnd += 1;
+ ca->frac_tracer -= Double_SCALE;
+ }
+ }
+}
+
+/* This function is called when the TCP flow detects a loss.
+ * It returns the slow start threshold of a flow, after a packet loss is detected. */
+static u32 agilesdtcp_recalc_ssthresh(struct sock *sk)
+{
+ const struct tcp_sock *tp = tcp_sk(sk);
+ struct agilesdtcp *ca = inet_csk_ca(sk);
+
+ ca->loss_cwnd = tp->snd_cwnd;
+
+ if (ca->agilesd_tcp_status == CA)
+ ca->degraded_loss_cwnd = max((tp->snd_cwnd * beta) / SCALE, 2U);
+ else
+ ca->degraded_loss_cwnd = max((tp->snd_cwnd * beta) / SCALE, 2U);
+
+ ca->frac_tracer = 0;
+
+ return ca->degraded_loss_cwnd;
+}
+
+static u32 agilesdtcp_undo_cwnd(struct sock *sk)
+{
+ const struct tcp_sock *tp = tcp_sk(sk);
+ const struct agilesdtcp *ca = inet_csk_ca(sk);
+ return max(tp->snd_cwnd, ca->loss_cwnd);
+}
+
+/* This function is called when the congestion state of the TCP is changed.
+ * newstate is the state code for the state that TCP is going to be in.
+ * The possible states are listed below:
+ * The current congestion control state, which can be one of the followings:
+ * TCP_CA_Open: normal state
+ * TCP_CA_Recovery: Loss Recovery after a Fast Transmission
+ * TCP_CA_Loss: Loss Recovery after a Timeout
+ * (The following two states are not effective in TCP-Linux but is effective in Linux)
+ * TCP_CA_Disorder: duplicate packets detected, but haven't reach the threshold. So TCP shall assume that packet reordering is happening.
+ * TCP_CA_CWR: the state that congestion window is decreasing (after local congestion in NIC, or ECN and etc).
+ * It is to notify the congestion control algorithm and is used by some
+ * algorithms which turn off their special control during loss recovery. */
+static void agilesdtcp_state(struct sock *sk, u8 new_state)
+{
+ if (new_state == TCP_CA_Loss)
+ agilesdtcp_reset(inet_csk_ca(sk));
+}
+
+/* This function is called when there is an acknowledgment that acknowledges some new packets.
+ * num_acked is the number of packets that are acknowledged by this acknowledgments. */
+static void agilesdtcp_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
+{
+
+}
+
+static struct tcp_congestion_ops agilesdtcp __read_mostly = {
+ .init = agilesdtcp_init,
+ .ssthresh = agilesdtcp_recalc_ssthresh,
+ .cong_avoid = agilesdtcp_cong_avoid,
+ .set_state = agilesdtcp_state,
+ .undo_cwnd = agilesdtcp_undo_cwnd,
+ .pkts_acked = agilesdtcp_acked,
+ .owner = THIS_MODULE,
+ .name = "agilesd",
+};
+
+static int __init agilesdtcp_register(void)
+{
+ BUILD_BUG_ON(sizeof(struct agilesdtcp) > ICSK_CA_PRIV_SIZE);
+ return tcp_register_congestion_control(&agilesdtcp);
+}
+
+static void __exit agilesdtcp_unregister(void)
+{
+ tcp_unregister_congestion_control(&agilesdtcp);
+}
+
+module_init(agilesdtcp_register);
+module_exit(agilesdtcp_unregister);
+
+MODULE_AUTHOR("Mohamed A. Alrshah <mohamed.a.alrshah@ieee.org>");
+MODULE_AUTHOR("Mohamed Othman");
+MODULE_AUTHOR("Borhanuddin Ali");
+MODULE_AUTHOR("Zurina Hanapi");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_DESCRIPTION("Agile-SD is a Loss-Based Congestion Control Algorithm for High-speed Networks");
--
2.12.3
^ permalink raw reply related
* RE: [PATCH v2 net-next 1/3] ipv6: Prevent unexpected sk->sk_prot changes
From: Boris Pismenny @ 2017-08-15 13:08 UTC (permalink / raw)
To: Eric Dumazet, Ilya Lesokhin
Cc: netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
Aviad Yehezkel
In-Reply-To: <1502798357.4936.63.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Tuesday, August 15, 2017 14:59
> To: Ilya Lesokhin <ilyal@mellanox.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; davejwatson@fb.com;
> Aviad Yehezkel <aviadye@mellanox.com>; Boris Pismenny
> <borisp@mellanox.com>
> Subject: Re: [PATCH v2 net-next 1/3] ipv6: Prevent unexpected sk->sk_prot
> changes
>
> On Tue, 2017-08-15 at 14:08 +0300, Ilya Lesokhin wrote:
> > With this patch IPV6 code ensure that only sockets with the
> > expected sk->sk_prot are converted to IPV4.
> >
> > Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> > ---
> > net/ipv6/ipv6_sockglue.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> > index 02d795f..318cd344 100644
> > --- a/net/ipv6/ipv6_sockglue.c
> > +++ b/net/ipv6/ipv6_sockglue.c
> > @@ -174,6 +174,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int
> level, int optname,
> > if (val == PF_INET) {
> > struct ipv6_txoptions *opt;
> > struct sk_buff *pktopt;
> > + struct proto *expected_prot;
> >
> > if (sk->sk_type == SOCK_RAW)
> > break;
> > @@ -199,6 +200,17 @@ static int do_ipv6_setsockopt(struct sock *sk, int
> level, int optname,
> > break;
> > }
> >
> > + if (sk->sk_protocol == IPPROTO_TCP &&
> > + sk->sk_prot != &tcpv6_prot)
> > + break;
> > +
> > + expected_prot = &udpv6_prot;
> > + if (sk->sk_protocol == IPPROTO_UDPLITE)
> > + expected_prot = &udplitev6_prot;
> > +
> > + if (sk->sk_prot != expected_prot)
> > + break;
> > +
> > fl6_free_socklist(sk);
> > __ipv6_sock_mc_close(sk);
> >
>
> I am afraid I do not understand this patch at all.
>
> Direct references to tcpv6_prot, udpv6_prot, and udplitev6_prot in
> net/ipv6/ipv6_sockglue.c looks completely broken.
>
> Please provide something cleaner, maybe by adding a new method
> (implementation would then be provided in TCP / UDP code )
>
>
The IPV6_ADDRFORM socket option assumes that when
(sk->sk_protocol == IPPROTO_TCP)
then the sk_proto is set to tcpv6_prot and it replaces it with tcp_prot.
This patch ensures that the IPV6_ADDRFORM socket option doesn't replace
the socket's sk_prot to tcp when it is not expected. For example, TLS sockets
also replace sk_prot, and we need to prevent IPV6_ADDRFORM from
overriding these.
Are you suggesting that each socket protocol will provide a method that
converts it from IPv6 to IPv4?
^ permalink raw reply
* Possible race in c4.ko
From: Anton Volkov @ 2017-08-15 13:22 UTC (permalink / raw)
To: isdn, davem, calle; +Cc: netdev, linux-kernel, ldv-project, Alexey Khoroshilov
Hello.
While searching for races in the Linux kernel I've come across
"drivers/isdn/hardware/avm/c4.ko" module. Here is a question that I came
up with while analyzing results. Lines are given using the info from
Linux v4.12.
Consider the following case:
Thread 1: Thread 2:
c4_probe
->c4_add_card
request_irq()
c4_interrupt
->c4_handle_interrupt
->c4_handle_rx
card->cardnr = ... cidx = f(card->cardnr)
(c4.c: line 1227) (c4.c: line 526)
if (cidx >= card->nlogcontr) cidx = 0;
ctrl = &card->ctrlinfo[cidx].capi_ctrl
card->cardnr is 0 until it is initialized in c4_add_card(). If at the
moment of read access in c4_handle_rx() it is still 0, cidx may then be
assigned an undesirable value and wrong controller may handle messages.
Is this case feasible from your point of view?
Thank you for your time.
-- Anton Volkov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: avolkov@ispras.ru
^ permalink raw reply
* [PATCH] net_sched: reset pointers to tcf blocks in classful qdiscs' destructors
From: Konstantin Khlebnikov @ 2017-08-15 13:35 UTC (permalink / raw)
To: netdev, David S. Miller, Cong Wang; +Cc: Jiri Pirko, Jamal Hadi Salim
Traffic filters could keep direct pointers to classes in classful qdisc,
thus qdisc destruction first removes all filters before freeing classes.
Class destruction methods also tries to free attached filters but now
this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
cannot be called second time.
This patch set class->block to NULL after first tcf_block_put() and
turn second call into no-op.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")
---
net/sched/sch_atm.c | 4 +++-
net/sched/sch_cbq.c | 4 +++-
net/sched/sch_hfsc.c | 4 +++-
net/sched/sch_htb.c | 4 +++-
4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 572fe2584e48..c403c87aff7a 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -572,8 +572,10 @@ static void atm_tc_destroy(struct Qdisc *sch)
struct atm_flow_data *flow, *tmp;
pr_debug("atm_tc_destroy(sch %p,[qdisc %p])\n", sch, p);
- list_for_each_entry(flow, &p->flows, list)
+ list_for_each_entry(flow, &p->flows, list) {
tcf_block_put(flow->block);
+ flow->block = NULL;
+ }
list_for_each_entry_safe(flow, tmp, &p->flows, list) {
if (flow->ref > 1)
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 481036f6b54e..780db43300b1 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1431,8 +1431,10 @@ static void cbq_destroy(struct Qdisc *sch)
* be bound to classes which have been destroyed already. --TGR '04
*/
for (h = 0; h < q->clhash.hashsize; h++) {
- hlist_for_each_entry(cl, &q->clhash.hash[h], common.hnode)
+ hlist_for_each_entry(cl, &q->clhash.hash[h], common.hnode) {
tcf_block_put(cl->block);
+ cl->block = NULL;
+ }
}
for (h = 0; h < q->clhash.hashsize; h++) {
hlist_for_each_entry_safe(cl, next, &q->clhash.hash[h],
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 3ad02bbe6903..fd15200f8627 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1530,8 +1530,10 @@ hfsc_destroy_qdisc(struct Qdisc *sch)
unsigned int i;
for (i = 0; i < q->clhash.hashsize; i++) {
- hlist_for_each_entry(cl, &q->clhash.hash[i], cl_common.hnode)
+ hlist_for_each_entry(cl, &q->clhash.hash[i], cl_common.hnode) {
tcf_block_put(cl->block);
+ cl->block = NULL;
+ }
}
for (i = 0; i < q->clhash.hashsize; i++) {
hlist_for_each_entry_safe(cl, next, &q->clhash.hash[i],
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 203286ab4427..5d65ec5207e9 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1258,8 +1258,10 @@ static void htb_destroy(struct Qdisc *sch)
tcf_block_put(q->block);
for (i = 0; i < q->clhash.hashsize; i++) {
- hlist_for_each_entry(cl, &q->clhash.hash[i], common.hnode)
+ hlist_for_each_entry(cl, &q->clhash.hash[i], common.hnode) {
tcf_block_put(cl->block);
+ cl->block = NULL;
+ }
}
for (i = 0; i < q->clhash.hashsize; i++) {
hlist_for_each_entry_safe(cl, next, &q->clhash.hash[i],
^ permalink raw reply related
* [PATCH 1/2] net_sched: call qlen_notify only if child qdisc is empty
From: Konstantin Khlebnikov @ 2017-08-15 13:39 UTC (permalink / raw)
To: netdev, David S. Miller, Cong Wang
Cc: Jiri Kosina, Eric Dumazet, Jamal Hadi Salim
This callback is used for deactivating class in parent qdisc.
This is cheaper to test queue length right here.
Also this allows to catch draining screwed backlog and prevent
second deactivation of already inactive parent class which will
crash kernel for sure. Kernel with print warning at destruction
of child qdisc where no packets but backlog is not zero.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
net/sched/sch_api.c | 10 +++++++++-
net/sched/sch_cbq.c | 3 +--
net/sched/sch_drr.c | 3 +--
net/sched/sch_hfsc.c | 6 ++----
net/sched/sch_htb.c | 3 +--
net/sched/sch_qfq.c | 3 +--
6 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index bd24a550e0f9..18da45c0769c 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -752,6 +752,7 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n,
const struct Qdisc_class_ops *cops;
unsigned long cl;
u32 parentid;
+ bool notify;
int drops;
if (n == 0 && len == 0)
@@ -764,6 +765,13 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n,
if (sch->flags & TCQ_F_NOPARENT)
break;
+ /* Notify parent qdisc only if child qdisc becomes empty.
+ *
+ * If child was empty even before update then backlog
+ * counter is screwed and we skip notification because
+ * parent class is already passive.
+ */
+ notify = !sch->q.qlen && !WARN_ON_ONCE(!n);
/* TODO: perform the search on a per txq basis */
sch = qdisc_lookup(qdisc_dev(sch), TC_H_MAJ(parentid));
if (sch == NULL) {
@@ -771,7 +779,7 @@ void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n,
break;
}
cops = sch->ops->cl_ops;
- if (cops->qlen_notify) {
+ if (notify && cops->qlen_notify) {
cl = cops->get(sch, parentid);
cops->qlen_notify(sch, cl);
cops->put(sch, cl);
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 780db43300b1..1bdb0106f342 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1385,8 +1385,7 @@ static void cbq_qlen_notify(struct Qdisc *sch, unsigned long arg)
{
struct cbq_class *cl = (struct cbq_class *)arg;
- if (cl->q->q.qlen == 0)
- cbq_deactivate_class(cl);
+ cbq_deactivate_class(cl);
}
static unsigned long cbq_get(struct Qdisc *sch, u32 classid)
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index a413dc1c2098..1d2f6235dfcf 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -246,8 +246,7 @@ static void drr_qlen_notify(struct Qdisc *csh, unsigned long arg)
{
struct drr_class *cl = (struct drr_class *)arg;
- if (cl->qdisc->q.qlen == 0)
- list_del(&cl->alist);
+ list_del(&cl->alist);
}
static int drr_dump_class(struct Qdisc *sch, unsigned long arg,
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index fd15200f8627..14c99870cdb6 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1221,10 +1221,8 @@ hfsc_qlen_notify(struct Qdisc *sch, unsigned long arg)
{
struct hfsc_class *cl = (struct hfsc_class *)arg;
- if (cl->qdisc->q.qlen == 0) {
- update_vf(cl, 0, 0);
- set_passive(cl);
- }
+ update_vf(cl, 0, 0);
+ set_passive(cl);
}
static unsigned long
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 5d65ec5207e9..dcf3c85e1f4f 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1186,8 +1186,7 @@ static void htb_qlen_notify(struct Qdisc *sch, unsigned long arg)
{
struct htb_class *cl = (struct htb_class *)arg;
- if (cl->un.leaf.q->q.qlen == 0)
- htb_deactivate(qdisc_priv(sch), cl);
+ htb_deactivate(qdisc_priv(sch), cl);
}
static unsigned long htb_get(struct Qdisc *sch, u32 classid)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 0e16dfda0bd7..9caa959f91e1 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1428,8 +1428,7 @@ static void qfq_qlen_notify(struct Qdisc *sch, unsigned long arg)
struct qfq_sched *q = qdisc_priv(sch);
struct qfq_class *cl = (struct qfq_class *)arg;
- if (cl->qdisc->q.qlen == 0)
- qfq_deactivate_class(q, cl);
+ qfq_deactivate_class(q, cl);
}
static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
^ permalink raw reply related
* [PATCH net-next] dsa: fix flow disector null pointer
From: Craig Gallek @ 2017-08-15 13:43 UTC (permalink / raw)
To: John Crispin, Andrew Lunn, David S . Miller; +Cc: Dmitry Vyukov, netdev
From: Craig Gallek <kraig@google.com>
A recent change to fix up DSA device behavior made the assumption that
all skbs passing through the flow disector will be associated with a
device. This does not appear to be a safe assumption. Syzkaller found
the crash below by attaching a BPF socket filter that tries to find the
payload offset of a packet passing between two unix sockets.
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 2940 Comm: syzkaller872007 Not tainted 4.13.0-rc4-next-20170811 #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
task: ffff8801d1b425c0 task.stack: ffff8801d0bc0000
RIP: 0010:__skb_flow_dissect+0xdcd/0x3ae0 net/core/flow_dissector.c:445
RSP: 0018:ffff8801d0bc7340 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000060 RSI: ffffffff856dc080 RDI: 0000000000000300
RBP: ffff8801d0bc7870 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000008 R11: ffffed003a178f1e R12: 0000000000000000
R13: 0000000000000000 R14: ffffffff856dc080 R15: ffff8801ce223140
FS: 00000000016ed880(0000) GS:ffff8801dc000000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020008000 CR3: 00000001ce22d000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
skb_flow_dissect_flow_keys include/linux/skbuff.h:1176 [inline]
skb_get_poff+0x9a/0x1a0 net/core/flow_dissector.c:1079
______skb_get_pay_offset net/core/filter.c:114 [inline]
__skb_get_pay_offset+0x15/0x20 net/core/filter.c:112
Code: 80 3c 02 00 44 89 6d 10 0f 85 44 2b 00 00 4d 8b 67 20 48 b8 00 00 00 00 00 fc ff df 49 8d bc 24 00 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 13 2b 00 00 4d 8b a4 24 00 03 00 00 4d 85 e4
RIP: __skb_flow_dissect+0xdcd/0x3ae0 net/core/flow_dissector.c:445 RSP: ffff8801d0bc7340
Fixes: 43e665287f93 ("net-next: dsa: fix flow dissection")
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Craig Gallek <kraig@google.com>
---
net/core/flow_dissector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 79b9c06c83ad..e2eaa1ff948d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -442,7 +442,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
nhoff = skb_network_offset(skb);
hlen = skb_headlen(skb);
#if IS_ENABLED(CONFIG_NET_DSA)
- if (unlikely(netdev_uses_dsa(skb->dev))) {
+ if (unlikely(skb->dev && netdev_uses_dsa(skb->dev))) {
const struct dsa_device_ops *ops;
int offset;
--
2.14.0.434.g98096fd7a8-goog
^ permalink raw reply related
* [PATCH] net_sched/sfq: update hierarchical backlog when drop packet
From: Konstantin Khlebnikov @ 2017-08-15 13:37 UTC (permalink / raw)
To: netdev, David S. Miller, Cong Wang; +Cc: Jamal Hadi Salim
When sfq_enqueue() drops head packet or packet from another queue it
have to update backlog at upper qdiscs too.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too")
---
net/sched/sch_sfq.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index f80ea2cc5f1f..82469ef9655e 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -437,6 +437,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
qdisc_drop(head, sch, to_free);
slot_queue_add(slot, skb);
+ qdisc_tree_reduce_backlog(sch, 0, delta);
return NET_XMIT_CN;
}
@@ -468,8 +469,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
/* Return Congestion Notification only if we dropped a packet
* from this flow.
*/
- if (qlen != slot->qlen)
+ if (qlen != slot->qlen) {
+ qdisc_tree_reduce_backlog(sch, 0, dropped - qdisc_pkt_len(skb));
return NET_XMIT_CN;
+ }
/* As we dropped a packet, better let upper stack know this */
qdisc_tree_reduce_backlog(sch, 1, dropped);
^ permalink raw reply related
* [PATCH] net_sched: remove warning from qdisc_hash_add
From: Konstantin Khlebnikov @ 2017-08-15 13:39 UTC (permalink / raw)
To: netdev, David S. Miller, Cong Wang
Cc: Jiri Kosina, Eric Dumazet, Jamal Hadi Salim
It was added in commit e57a784d8cae ("pkt_sched: set root qdisc
before change() in attach_default_qdiscs()") to hide duplicates
from "tc qdisc show" for incative deivices.
After 59cc1f61f ("net: sched: convert qdisc linked list to hashtable")
it triggered when classful qdisc is added to inactive device because
default qdiscs are added before switching root qdisc.
Anyway after commit ea3274695353 ("net: sched: avoid duplicates in
qdisc dump") duplicates are filtered right in dumper.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
net/sched/sch_api.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 18da45c0769c..2d2cf539668c 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -286,9 +286,6 @@ static struct Qdisc *qdisc_match_from_root(struct Qdisc *root, u32 handle)
void qdisc_hash_add(struct Qdisc *q, bool invisible)
{
if ((q->parent != TC_H_ROOT) && !(q->flags & TCQ_F_INGRESS)) {
- struct Qdisc *root = qdisc_dev(q)->qdisc;
-
- WARN_ON_ONCE(root == &noop_qdisc);
ASSERT_RTNL();
hash_add_rcu(qdisc_dev(q)->qdisc_hash, &q->hash, q->handle);
if (invisible)
^ permalink raw reply related
* [PATCH 2/2] net_sched/hfsc: opencode trivial set_active() and set_passive()
From: Konstantin Khlebnikov @ 2017-08-15 13:40 UTC (permalink / raw)
To: netdev, David S. Miller, Cong Wang
Cc: Jiri Kosina, Eric Dumazet, Jamal Hadi Salim
In-Reply-To: <150280439968.717891.6476652519937001709.stgit@buzz>
Any move comment abount update_vf() into right place.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
net/sched/sch_hfsc.c | 45 ++++++++++++++++-----------------------------
1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 14c99870cdb6..15f09cb9f1ff 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -829,28 +829,6 @@ update_vf(struct hfsc_class *cl, unsigned int len, u64 cur_time)
}
}
-static void
-set_active(struct hfsc_class *cl, unsigned int len)
-{
- if (cl->cl_flags & HFSC_RSC)
- init_ed(cl, len);
- if (cl->cl_flags & HFSC_FSC)
- init_vf(cl, len);
-
-}
-
-static void
-set_passive(struct hfsc_class *cl)
-{
- if (cl->cl_flags & HFSC_RSC)
- eltree_remove(cl);
-
- /*
- * vttree is now handled in update_vf() so that update_vf(cl, 0, 0)
- * needs to be called explicitly to remove a class from vttree.
- */
-}
-
static unsigned int
qdisc_peek_len(struct Qdisc *sch)
{
@@ -1221,8 +1199,12 @@ hfsc_qlen_notify(struct Qdisc *sch, unsigned long arg)
{
struct hfsc_class *cl = (struct hfsc_class *)arg;
+ /* vttree is now handled in update_vf() so that update_vf(cl, 0, 0)
+ * needs to be called explicitly to remove a class from vttree.
+ */
update_vf(cl, 0, 0);
- set_passive(cl);
+ if (cl->cl_flags & HFSC_RSC)
+ eltree_remove(cl);
}
static unsigned long
@@ -1583,7 +1565,12 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
}
if (cl->qdisc->q.qlen == 1) {
- set_active(cl, qdisc_pkt_len(skb));
+ unsigned int len = qdisc_pkt_len(skb);
+
+ if (cl->cl_flags & HFSC_RSC)
+ init_ed(cl, len);
+ if (cl->cl_flags & HFSC_FSC)
+ init_vf(cl, len);
/*
* If this is the first packet, isolate the head so an eventual
* head drop before the first dequeue operation has no chance
@@ -1647,18 +1634,18 @@ hfsc_dequeue(struct Qdisc *sch)
if (realtime)
cl->cl_cumul += qdisc_pkt_len(skb);
- if (cl->qdisc->q.qlen != 0) {
- if (cl->cl_flags & HFSC_RSC) {
+ if (cl->cl_flags & HFSC_RSC) {
+ if (cl->qdisc->q.qlen != 0) {
/* update ed */
next_len = qdisc_peek_len(cl->qdisc);
if (realtime)
update_ed(cl, next_len);
else
update_d(cl, next_len);
+ } else {
+ /* the class becomes passive */
+ eltree_remove(cl);
}
- } else {
- /* the class becomes passive */
- set_passive(cl);
}
qdisc_bstats_update(sch, skb);
^ permalink raw reply related
* Re: [PATCH] net/sched: reset block pointer in tcf_block_put()
From: Konstantin Khlebnikov @ 2017-08-15 13:48 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jiri Pirko, David S. Miller,
Jamal Hadi Salim
In-Reply-To: <CAM_iQpX2=Fg8JN1c8qnLPae34hsYEG7xUKZ-RoppBnVoBDb+6g@mail.gmail.com>
On 15.08.2017 00:15, Cong Wang wrote:
> On Mon, Aug 14, 2017 at 5:59 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>>
>> This should work, I suppose.
>>
>> But this approach requires careful review for all qdisc, mine is completely
>> mechanical.
>
> Well, we don't have many classful qdisc's. Your patch actually
> touches more qdisc's than mine, because you change an API, so
> it is slightly harder to backport. ;)
>
Ok, I've fixed this right in qdiscs: [PATCH] net_sched: reset pointers to tcf blocks in classful qdiscs' destructors
^ permalink raw reply
* Re: [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile
From: Neal Cardwell @ 2017-08-15 13:51 UTC (permalink / raw)
To: mohamedalrshah
Cc: David Miller, Netdev, Linus Torvalds, LKML, Mohamed Othman,
Borhanuddin Ali, Zurina Hanapi
In-Reply-To: <20170815130806.25168-1-mohamed.a.alrshah@ieee.org>
On Tue, Aug 15, 2017 at 9:08 AM, mohamedalrshah
<mohamed.a.alrshah@ieee.org> wrote:
> +
> +/* Agile-SD Parameters */
> +struct agilesdtcp {
> + u32 loss_cwnd; /* congestion window at last loss.*/
Please rebase your change on top of the latest net-next changes and
update this module to use the latest approach from the recent commit:
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=f1722a1be19dc38e0a4b282d4e6e6ec5e1b11a67
tcp: consolidate congestion control undo functions
Specifically:
- reference tp->prior_cwnd instead of ca->loss_cwnd
- remove the ca->loss_cwnd field
- have the .undo_cwnd field reference tcp_reno_undo_cwnd
> + u32 frac_tracer; /* This is to trace the fractions of the increment.*/
> + u32 degraded_loss_cwnd; /* loss_cwnd after degradation.*/
> + enum dystate{SS=0, CA=1} agilesd_tcp_status;
Per Linux style, please define the enum separately before declaring
the variable of that type, and format the enum using Linux style. Also
please use a longer, more specific name, to avoid name collisions. I'd
suggest:
enum dystate {
AGILE_SD_SS = 0, /* comment ... */
AGILE_SD_CA = 1, /* comment ... */
};
> +};
> +
> +/* To reset the parameters if needed*/
> +static inline void agilesdtcp_reset(struct sock *sk)
> +{
> +
> +}
> +
> +/* This function is called after the first acknowledgment is received and before the congestion
> + * control algorithm will be called for the first time. If the congestion control algorithm has
> + * private data, it should initialize its private date here. */
Multi-line comments should end with the trailing */ on a line by
itself. Here and elsewhere.
Please read:
https://www.kernel.org/doc/html/v4.10/process/coding-style.html
Please check the style of patches before submitting with the following
script in the Linux kernel tree:
scripts/checkpatch.pl
> +static void agilesdtcp_init(struct sock *sk)
> +{
> + struct agilesdtcp *ca = inet_csk_ca(sk);
> +
> + /* The value of initial_ssthresh parameter is not used here, thus, snd_ssthresh is initialized by a large value.*/
> + tcp_sk(sk)->snd_ssthresh = 0x7fffffff;
> +
> + ca->loss_cwnd = 0;
> + ca->frac_tracer = 0;
> + ca->agilesd_tcp_status = SS;
> +}
> +
> +/* This function is called whenever an ack is received and the congestion window can be increased.
> + * This is equivalent to opencwnd in tcp.cc.
> + * ack is the number of bytes that are acknowledged in the latest acknowledgment;
> + * rtt is the the rtt measured by the latest acknowledgment;
> + * in_flight is the packet in flight before the latest acknowledgment;
> + * good_ack is an indicator whether the current situation is normal (no duplicate ack, no loss and no SACK). */
> +static void agilesdtcp_cong_avoid(struct sock *sk, u32 ack, u32 in_flight)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct agilesdtcp *ca = inet_csk_ca(sk);
> + u32 inc_factor;
> + u32 ca_inc;
> + u32 current_gap, total_gap;
For coding style, please order local variable declarations from
longest to shortest line, also know as Reverse Christmas Tree Format.
> + /* The value of inc_factor is limited by lower_fl and upper_fl.
> + * The lower_fl must be always = 1. The greater the upper_fl the higher the aggressiveness.
> + * But, if upper_fl set to 1, Agile-SD will work exactly as newreno.
> + * We have already designed an equation to calculate the optimum upper_fl based on the given beta.
> + * This equation will be revealed once its article is published*/
> + u32 lower_fl = 1 * SCALE;
> + u32 upper_fl = 3 * SCALE;
> +
> + if (!tcp_is_cwnd_limited(sk)) return;
Please put this return (or any if/else body) on a line by itself.
> +
> + if (tp->snd_cwnd < tp->snd_ssthresh){
Need a space between ) and {.
> + ca->agilesd_tcp_status = SS;
> + tcp_slow_start(tp, in_flight);
> + }
> + else {
The else needs to go on the same line as the closing brace.
> + inc_factor = min(max(((upper_fl * current_gap) / total_gap), lower_fl), upper_fl);
Please use the existing kernel helper macro for this:
#define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
> +
> + ca_inc = ((inc_factor * SCALE) / tp->snd_cwnd); /* SCALE is used to avoid fractions*/
> +
> + ca->frac_tracer += ca_inc; /* This in order to take the fraction increase into account */
> + if (ca->frac_tracer >= Double_SCALE) /* To take factor scale into account */
> + {
The opening brace goes on the previous line.
> +/* This function is called when the TCP flow detects a loss.
> + * It returns the slow start threshold of a flow, after a packet loss is detected. */
Trailing */ style issue again.
> +static u32 agilesdtcp_recalc_ssthresh(struct sock *sk)
> +{
> + const struct tcp_sock *tp = tcp_sk(sk);
> + struct agilesdtcp *ca = inet_csk_ca(sk);
> +
> + ca->loss_cwnd = tp->snd_cwnd;
> +
> + if (ca->agilesd_tcp_status == CA)
> + ca->degraded_loss_cwnd = max((tp->snd_cwnd * beta) / SCALE, 2U);
> + else
> + ca->degraded_loss_cwnd = max((tp->snd_cwnd * beta) / SCALE, 2U);
These two branches of the if/else look the same? Can they be condensed
to a single line?
Thanks,
neal
^ permalink raw reply
* [PATCH v2 net-next] bpf/verifier: track liveness for pruning
From: Edward Cree @ 2017-08-15 13:53 UTC (permalink / raw)
To: davem, Alexei Starovoitov, Alexei Starovoitov, Daniel Borkmann
Cc: netdev, linux-kernel, iovisor-dev
State of a register doesn't matter if it wasn't read in reaching an exit;
a write screens off all reads downstream of it from all explored_states
upstream of it.
This allows us to prune many more branches; here are some processed insn
counts for some Cilium programs:
Program before after
bpf_lb_opt_-DLB_L3.o 6515 3361
bpf_lb_opt_-DLB_L4.o 8976 5176
bpf_lb_opt_-DUNKNOWN.o 2960 1137
bpf_lxc_opt_-DDROP_ALL.o 95412 48537
bpf_lxc_opt_-DUNKNOWN.o 141706 78718
bpf_netdev.o 24251 17995
bpf_overlay.o 10999 9385
The runtime is also improved; here are 'time' results in ms:
Program before after
bpf_lb_opt_-DLB_L3.o 24 6
bpf_lb_opt_-DLB_L4.o 26 11
bpf_lb_opt_-DUNKNOWN.o 11 2
bpf_lxc_opt_-DDROP_ALL.o 1288 139
bpf_lxc_opt_-DUNKNOWN.o 1768 234
bpf_netdev.o 62 31
bpf_overlay.o 15 13
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
v2: update liveness in LD_ABS|IND, as pointed out by Daniel Borkmann. The
numbers are mostly unchanged; bpf_lxc_opt_-DUNKNOWN.o dropped about 300
insns and 20ms, while bpf_lxc_opt_-DDROP_ALL.o (despite not changing its
#insns) also dropped 13ms.
include/linux/bpf_verifier.h | 11 ++-
kernel/bpf/verifier.c | 188 +++++++++++++++++++++++++++++++++----------
2 files changed, 156 insertions(+), 43 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c61c3033..91d07ef 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -21,6 +21,12 @@
*/
#define BPF_MAX_VAR_SIZ INT_MAX
+enum bpf_reg_liveness {
+ REG_LIVE_NONE = 0, /* reg hasn't been read or written this branch */
+ REG_LIVE_READ, /* reg was read, so we're sensitive to initial value */
+ REG_LIVE_WRITTEN, /* reg was written first, screening off later reads */
+};
+
struct bpf_reg_state {
enum bpf_reg_type type;
union {
@@ -40,7 +46,7 @@ struct bpf_reg_state {
* came from, when one is tested for != NULL.
*/
u32 id;
- /* These five fields must be last. See states_equal() */
+ /* Ordering of fields matters. See states_equal() */
/* For scalar types (SCALAR_VALUE), this represents our knowledge of
* the actual value.
* For pointer types, this represents the variable part of the offset
@@ -57,6 +63,8 @@ struct bpf_reg_state {
s64 smax_value; /* maximum possible (s64)value */
u64 umin_value; /* minimum possible (u64)value */
u64 umax_value; /* maximum possible (u64)value */
+ /* This field must be last, for states_equal() reasons. */
+ enum bpf_reg_liveness live;
};
enum bpf_stack_slot_type {
@@ -74,6 +82,7 @@ struct bpf_verifier_state {
struct bpf_reg_state regs[MAX_BPF_REG];
u8 stack_slot_type[MAX_BPF_STACK];
struct bpf_reg_state spilled_regs[MAX_BPF_STACK / BPF_REG_SIZE];
+ struct bpf_verifier_state *parent;
};
/* linked list of verifier states used to prune search */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ecc590e..3affb8d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -629,8 +629,10 @@ static void init_reg_state(struct bpf_reg_state *regs)
{
int i;
- for (i = 0; i < MAX_BPF_REG; i++)
+ for (i = 0; i < MAX_BPF_REG; i++) {
mark_reg_not_init(regs, i);
+ regs[i].live = REG_LIVE_NONE;
+ }
/* frame pointer */
regs[BPF_REG_FP].type = PTR_TO_STACK;
@@ -647,9 +649,26 @@ enum reg_arg_type {
DST_OP_NO_MARK /* same as above, check only, don't mark */
};
-static int check_reg_arg(struct bpf_reg_state *regs, u32 regno,
+static void mark_reg_read(const struct bpf_verifier_state *state, u32 regno)
+{
+ struct bpf_verifier_state *parent = state->parent;
+
+ while (parent) {
+ /* if read wasn't screened by an earlier write ... */
+ if (state->regs[regno].live & REG_LIVE_WRITTEN)
+ break;
+ /* ... then we depend on parent's value */
+ parent->regs[regno].live |= REG_LIVE_READ;
+ state = parent;
+ parent = state->parent;
+ }
+}
+
+static int check_reg_arg(struct bpf_verifier_env *env, u32 regno,
enum reg_arg_type t)
{
+ struct bpf_reg_state *regs = env->cur_state.regs;
+
if (regno >= MAX_BPF_REG) {
verbose("R%d is invalid\n", regno);
return -EINVAL;
@@ -661,12 +680,14 @@ static int check_reg_arg(struct bpf_reg_state *regs, u32 regno,
verbose("R%d !read_ok\n", regno);
return -EACCES;
}
+ mark_reg_read(&env->cur_state, regno);
} else {
/* check whether register used as dest operand can be written to */
if (regno == BPF_REG_FP) {
verbose("frame pointer is read only\n");
return -EACCES;
}
+ regs[regno].live |= REG_LIVE_WRITTEN;
if (t == DST_OP)
mark_reg_unknown(regs, regno);
}
@@ -695,7 +716,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
static int check_stack_write(struct bpf_verifier_state *state, int off,
int size, int value_regno)
{
- int i;
+ int i, spi = (MAX_BPF_STACK + off) / BPF_REG_SIZE;
/* caller checked that off % size == 0 and -MAX_BPF_STACK <= off < 0,
* so it's aligned access and [off, off + size) are within stack limits
*/
@@ -710,15 +731,14 @@ static int check_stack_write(struct bpf_verifier_state *state, int off,
}
/* save register state */
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE] =
- state->regs[value_regno];
+ state->spilled_regs[spi] = state->regs[value_regno];
+ state->spilled_regs[spi].live |= REG_LIVE_WRITTEN;
for (i = 0; i < BPF_REG_SIZE; i++)
state->stack_slot_type[MAX_BPF_STACK + off + i] = STACK_SPILL;
} else {
/* regular write of data into stack */
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE] =
- (struct bpf_reg_state) {};
+ state->spilled_regs[spi] = (struct bpf_reg_state) {};
for (i = 0; i < size; i++)
state->stack_slot_type[MAX_BPF_STACK + off + i] = STACK_MISC;
@@ -726,11 +746,26 @@ static int check_stack_write(struct bpf_verifier_state *state, int off,
return 0;
}
+static void mark_stack_slot_read(const struct bpf_verifier_state *state, int slot)
+{
+ struct bpf_verifier_state *parent = state->parent;
+
+ while (parent) {
+ /* if read wasn't screened by an earlier write ... */
+ if (state->spilled_regs[slot].live & REG_LIVE_WRITTEN)
+ break;
+ /* ... then we depend on parent's value */
+ parent->spilled_regs[slot].live |= REG_LIVE_READ;
+ state = parent;
+ parent = state->parent;
+ }
+}
+
static int check_stack_read(struct bpf_verifier_state *state, int off, int size,
int value_regno)
{
u8 *slot_type;
- int i;
+ int i, spi;
slot_type = &state->stack_slot_type[MAX_BPF_STACK + off];
@@ -746,10 +781,13 @@ static int check_stack_read(struct bpf_verifier_state *state, int off, int size,
}
}
- if (value_regno >= 0)
+ spi = (MAX_BPF_STACK + off) / BPF_REG_SIZE;
+
+ if (value_regno >= 0) {
/* restore register state from stack */
- state->regs[value_regno] =
- state->spilled_regs[(MAX_BPF_STACK + off) / BPF_REG_SIZE];
+ state->regs[value_regno] = state->spilled_regs[spi];
+ mark_stack_slot_read(state, spi);
+ }
return 0;
} else {
for (i = 0; i < size; i++) {
@@ -1167,7 +1205,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_insn *insn)
{
- struct bpf_reg_state *regs = env->cur_state.regs;
int err;
if ((BPF_SIZE(insn->code) != BPF_W && BPF_SIZE(insn->code) != BPF_DW) ||
@@ -1177,12 +1214,12 @@ static int check_xadd(struct bpf_verifier_env *env, int insn_idx, struct bpf_ins
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -1297,10 +1334,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
if (arg_type == ARG_DONTCARE)
return 0;
- if (type == NOT_INIT) {
- verbose("R%d !read_ok\n", regno);
- return -EACCES;
- }
+ err = check_reg_arg(env, regno, SRC_OP);
+ if (err)
+ return err;
if (arg_type == ARG_ANYTHING) {
if (is_pointer_value(env, regno)) {
@@ -1639,10 +1675,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
}
/* reset caller saved regs */
- for (i = 0; i < CALLER_SAVED_REGS; i++)
+ for (i = 0; i < CALLER_SAVED_REGS; i++) {
mark_reg_not_init(regs, caller_saved[i]);
+ check_reg_arg(env, i, DST_OP_NO_MARK);
+ }
/* update return register */
+ check_reg_arg(env, BPF_REG_0, DST_OP_NO_MARK);
if (fn->ret_type == RET_INTEGER) {
/* sets type to SCALAR_VALUE */
mark_reg_unknown(regs, BPF_REG_0);
@@ -2250,7 +2289,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2261,7 +2300,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2274,7 +2313,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
} else {
@@ -2285,7 +2324,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2328,7 +2367,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EINVAL;
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
} else {
@@ -2339,7 +2378,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2360,7 +2399,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP_NO_MARK);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP_NO_MARK);
if (err)
return err;
@@ -2717,7 +2756,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
@@ -2734,7 +2773,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -2851,7 +2890,7 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EINVAL;
}
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP);
if (err)
return err;
@@ -2917,7 +2956,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
/* check whether implicit source operand (register R6) is readable */
- err = check_reg_arg(regs, BPF_REG_6, SRC_OP);
+ err = check_reg_arg(env, BPF_REG_6, SRC_OP);
if (err)
return err;
@@ -2928,17 +2967,20 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
if (mode == BPF_IND) {
/* check explicit source operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
}
/* reset caller saved regs to unreadable */
- for (i = 0; i < CALLER_SAVED_REGS; i++)
+ for (i = 0; i < CALLER_SAVED_REGS; i++) {
mark_reg_not_init(regs, caller_saved[i]);
+ check_reg_arg(env, i, DST_OP_NO_MARK);
+ }
/* mark destination R0 register as readable, since it contains
- * the value fetched from the packet
+ * the value fetched from the packet.
+ * Already marked as written above.
*/
mark_reg_unknown(regs, BPF_REG_0);
return 0;
@@ -3194,7 +3236,11 @@ static bool regsafe(struct bpf_reg_state *rold,
struct bpf_reg_state *rcur,
bool varlen_map_access, struct idpair *idmap)
{
- if (memcmp(rold, rcur, sizeof(*rold)) == 0)
+ if (!(rold->live & REG_LIVE_READ))
+ /* explored state didn't use this */
+ return true;
+
+ if (memcmp(rold, rcur, offsetof(struct bpf_reg_state, live)) == 0)
return true;
if (rold->type == NOT_INIT)
@@ -3372,10 +3418,56 @@ static bool states_equal(struct bpf_verifier_env *env,
return ret;
}
+static bool do_propagate_liveness(const struct bpf_verifier_state *state,
+ struct bpf_verifier_state *parent)
+{
+ bool touched = false; /* any changes made? */
+ int i;
+
+ if (!parent)
+ return touched;
+ /* Propagate read liveness of registers... */
+ BUILD_BUG_ON(BPF_REG_FP + 1 != MAX_BPF_REG);
+ /* We don't need to worry about FP liveness because it's read-only */
+ for (i = 0; i < BPF_REG_FP; i++) {
+ if (parent->regs[i].live & REG_LIVE_READ)
+ continue;
+ if (state->regs[i].live == REG_LIVE_READ) {
+ parent->regs[i].live |= REG_LIVE_READ;
+ touched = true;
+ }
+ }
+ /* ... and stack slots */
+ for (i = 0; i < MAX_BPF_STACK / BPF_REG_SIZE; i++) {
+ if (parent->stack_slot_type[i * BPF_REG_SIZE] != STACK_SPILL)
+ continue;
+ if (state->stack_slot_type[i * BPF_REG_SIZE] != STACK_SPILL)
+ continue;
+ if (parent->spilled_regs[i].live & REG_LIVE_READ)
+ continue;
+ if (state->spilled_regs[i].live == REG_LIVE_READ) {
+ parent->regs[i].live |= REG_LIVE_READ;
+ touched = true;
+ }
+ }
+ return touched;
+}
+
+static void propagate_liveness(const struct bpf_verifier_state *state,
+ struct bpf_verifier_state *parent)
+{
+ while (do_propagate_liveness(state, parent)) {
+ /* Something changed, so we need to feed those changes onward */
+ state = parent;
+ parent = state->parent;
+ }
+}
+
static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
{
struct bpf_verifier_state_list *new_sl;
struct bpf_verifier_state_list *sl;
+ int i;
sl = env->explored_states[insn_idx];
if (!sl)
@@ -3385,11 +3477,14 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
return 0;
while (sl != STATE_LIST_MARK) {
- if (states_equal(env, &sl->state, &env->cur_state))
+ if (states_equal(env, &sl->state, &env->cur_state)) {
/* reached equivalent register/stack state,
- * prune the search
+ * prune the search.
+ * Registers read by the continuation are read by us.
*/
+ propagate_liveness(&sl->state, &env->cur_state);
return 1;
+ }
sl = sl->next;
}
@@ -3407,6 +3502,14 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
memcpy(&new_sl->state, &env->cur_state, sizeof(env->cur_state));
new_sl->next = env->explored_states[insn_idx];
env->explored_states[insn_idx] = new_sl;
+ /* connect new state to parentage chain */
+ env->cur_state.parent = &new_sl->state;
+ /* clear liveness marks in current state */
+ for (i = 0; i < BPF_REG_FP; i++)
+ env->cur_state.regs[i].live = REG_LIVE_NONE;
+ for (i = 0; i < MAX_BPF_STACK / BPF_REG_SIZE; i++)
+ if (env->cur_state.stack_slot_type[i * BPF_REG_SIZE] == STACK_SPILL)
+ env->cur_state.spilled_regs[i].live = REG_LIVE_NONE;
return 0;
}
@@ -3430,6 +3533,7 @@ static int do_check(struct bpf_verifier_env *env)
bool do_print_state = false;
init_reg_state(regs);
+ state->parent = NULL;
insn_idx = 0;
env->varlen_map_value_access = false;
for (;;) {
@@ -3500,11 +3604,11 @@ static int do_check(struct bpf_verifier_env *env)
/* check for reserved fields is already done */
/* check src operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
- err = check_reg_arg(regs, insn->dst_reg, DST_OP_NO_MARK);
+ err = check_reg_arg(env, insn->dst_reg, DST_OP_NO_MARK);
if (err)
return err;
@@ -3554,11 +3658,11 @@ static int do_check(struct bpf_verifier_env *env)
}
/* check src1 operand */
- err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ err = check_reg_arg(env, insn->src_reg, SRC_OP);
if (err)
return err;
/* check src2 operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -3589,7 +3693,7 @@ static int do_check(struct bpf_verifier_env *env)
return -EINVAL;
}
/* check src operand */
- err = check_reg_arg(regs, insn->dst_reg, SRC_OP);
+ err = check_reg_arg(env, insn->dst_reg, SRC_OP);
if (err)
return err;
@@ -3643,7 +3747,7 @@ static int do_check(struct bpf_verifier_env *env)
* of bpf_exit, which means that program wrote
* something into it earlier
*/
- err = check_reg_arg(regs, BPF_REG_0, SRC_OP);
+ err = check_reg_arg(env, BPF_REG_0, SRC_OP);
if (err)
return err;
^ permalink raw reply related
* [PATCH net] sfc: don't try and read ef10 data on non-ef10 NIC
From: Bert Kenward @ 2017-08-15 13:55 UTC (permalink / raw)
To: Dave Miller; +Cc: netdev, linux-net-drivers, Stefano Brivio
The MAC stats command takes a port ID, which doesn't exist on
pre-ef10 NICs (5000- and 6000- series). This is extracted from the
NIC specific data; we misinterpret this as the ef10 data structure,
causing us to read potentially unallocated data. With a KASAN kernel
this can cause errors with:
BUG: KASAN: slab-out-of-bounds in efx_mcdi_mac_stats
Fixes: 0a2ab4d988d7 ("sfc: set the port-id when calling MC_CMD_MAC_STATS")
Reported-by: Stefano Brivio <sbrivio@redhat.com>
Tested-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Bert Kenward <bkenward@solarflare.com>
---
drivers/net/ethernet/sfc/mcdi_port.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/mcdi_port.c b/drivers/net/ethernet/sfc/mcdi_port.c
index c905971c5f3a..990a63d7fcb7 100644
--- a/drivers/net/ethernet/sfc/mcdi_port.c
+++ b/drivers/net/ethernet/sfc/mcdi_port.c
@@ -938,7 +938,6 @@ enum efx_stats_action {
static int efx_mcdi_mac_stats(struct efx_nic *efx,
enum efx_stats_action action, int clear)
{
- struct efx_ef10_nic_data *nic_data = efx->nic_data;
MCDI_DECLARE_BUF(inbuf, MC_CMD_MAC_STATS_IN_LEN);
int rc;
int change = action == EFX_STATS_PULL ? 0 : 1;
@@ -960,7 +959,12 @@ static int efx_mcdi_mac_stats(struct efx_nic *efx,
MAC_STATS_IN_PERIODIC_NOEVENT, 1,
MAC_STATS_IN_PERIOD_MS, period);
MCDI_SET_DWORD(inbuf, MAC_STATS_IN_DMA_LEN, dma_len);
- MCDI_SET_DWORD(inbuf, MAC_STATS_IN_PORT_ID, nic_data->vport_id);
+
+ if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0) {
+ struct efx_ef10_nic_data *nic_data = efx->nic_data;
+
+ MCDI_SET_DWORD(inbuf, MAC_STATS_IN_PORT_ID, nic_data->vport_id);
+ }
rc = efx_mcdi_rpc_quiet(efx, MC_CMD_MAC_STATS, inbuf, sizeof(inbuf),
NULL, 0, NULL);
--
2.7.5
^ permalink raw reply related
* Re: [PATCH net-next] dsa: fix flow disector null pointer
From: Andrew Lunn @ 2017-08-15 13:56 UTC (permalink / raw)
To: Craig Gallek; +Cc: John Crispin, David S . Miller, Dmitry Vyukov, netdev
In-Reply-To: <20170815134340.169914-1-kraigatgoog@gmail.com>
On Tue, Aug 15, 2017 at 09:43:40AM -0400, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> A recent change to fix up DSA device behavior made the assumption that
> all skbs passing through the flow disector will be associated with a
> device. This does not appear to be a safe assumption. Syzkaller found
> the crash below by attaching a BPF socket filter that tries to find the
> payload offset of a packet passing between two unix sockets.
>
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 2940 Comm: syzkaller872007 Not tainted 4.13.0-rc4-next-20170811 #1
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: ffff8801d1b425c0 task.stack: ffff8801d0bc0000
> RIP: 0010:__skb_flow_dissect+0xdcd/0x3ae0 net/core/flow_dissector.c:445
> RSP: 0018:ffff8801d0bc7340 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000060 RSI: ffffffff856dc080 RDI: 0000000000000300
> RBP: ffff8801d0bc7870 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000008 R11: ffffed003a178f1e R12: 0000000000000000
> R13: 0000000000000000 R14: ffffffff856dc080 R15: ffff8801ce223140
> FS: 00000000016ed880(0000) GS:ffff8801dc000000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020008000 CR3: 00000001ce22d000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> skb_flow_dissect_flow_keys include/linux/skbuff.h:1176 [inline]
> skb_get_poff+0x9a/0x1a0 net/core/flow_dissector.c:1079
> ______skb_get_pay_offset net/core/filter.c:114 [inline]
> __skb_get_pay_offset+0x15/0x20 net/core/filter.c:112
> Code: 80 3c 02 00 44 89 6d 10 0f 85 44 2b 00 00 4d 8b 67 20 48 b8 00 00 00 00 00 fc ff df 49 8d bc 24 00 03 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 13 2b 00 00 4d 8b a4 24 00 03 00 00 4d 85 e4
> RIP: __skb_flow_dissect+0xdcd/0x3ae0 net/core/flow_dissector.c:445 RSP: ffff8801d0bc7340
>
> Fixes: 43e665287f93 ("net-next: dsa: fix flow dissection")
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Craig Gallek <kraig@google.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v11 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
From: Eric Dumazet @ 2017-08-15 13:58 UTC (permalink / raw)
To: David Miller
Cc: dingtianhong, leedom, ashok.raj, bhelgaas, helgaas, werner,
ganeshgr, asit.k.mallick, patrick.j.cramer, Suravee.Suthikulpanit,
Bob.Shaw, l.stach, amira, gabriele.paoloni, David.Laight,
jeffrey.t.kirsher, catalin.marinas, will.deacon, mark.rutland,
robin.murphy, alexander.duyck, linux-arm-kernel, netdev,
linux-pci, linux-kernel, linuxarm
In-Reply-To: <20170814.221512.1430356980828045690.davem@davemloft.net>
On Mon, 2017-08-14 at 22:15 -0700, David Miller wrote:
> From: Ding Tianhong <dingtianhong@huawei.com>
> Date: Tue, 15 Aug 2017 11:23:22 +0800
>
> > Some devices have problems with Transaction Layer Packets with the Relaxed
> > Ordering Attribute set. This patch set adds a new PCIe Device Flag,
> > PCI_DEV_FLAGS_NO_RELAXED_ORDERING, a set of PCI Quirks to catch some known
> > devices with Relaxed Ordering issues, and a use of this new flag by the
> > cxgb4 driver to avoid using Relaxed Ordering with problematic Root Complex
> > Ports.
> ...
>
> Series applied, thanks.
I got a NULL deref in pci_find_pcie_root_port()
Was it expected ?
This local hack seems to fix the issue.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af0cc3456dc1b48b1325c06c5edd2ca8cc22a640..cfd8eb5a3d0ba8347d44952ffab28d9c761044d3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -522,7 +522,7 @@ struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
bridge = pci_upstream_bridge(bridge);
}
- if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
+ if (highest_pcie_bridge && pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
return NULL;
return highest_pcie_bridge;
^ permalink raw reply related
* Re: [PATCH] net_sched: reset pointers to tcf blocks in classful qdiscs' destructors
From: Jiri Pirko @ 2017-08-15 14:00 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: netdev, David S. Miller, Cong Wang, Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <150280412126.717517.16274927544808177918.stgit@buzz>
Tue, Aug 15, 2017 at 03:35:21PM CEST, khlebnikov@yandex-team.ru wrote:
>Traffic filters could keep direct pointers to classes in classful qdisc,
>thus qdisc destruction first removes all filters before freeing classes.
>Class destruction methods also tries to free attached filters but now
>this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
>cannot be called second time.
>
>This patch set class->block to NULL after first tcf_block_put() and
>turn second call into no-op.
>
>Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")
Acked-by: Jiri Pirko <jiri@mellanox.com>
Thanks!
^ permalink raw reply
* Re: [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile
From: Neal Cardwell @ 2017-08-15 14:01 UTC (permalink / raw)
To: mohamedalrshah
Cc: David Miller, Netdev, Linus Torvalds, LKML, Mohamed Othman,
Borhanuddin Ali, Zurina Hanapi
In-Reply-To: <20170815130806.25168-1-mohamed.a.alrshah@ieee.org>
On Tue, Aug 15, 2017 at 9:08 AM, mohamedalrshah
<mohamed.a.alrshah@ieee.org> wrote:
> This commit implements a new TCP congestion control algorithm, namely Agile-SD.
Also, please use a summary line for your patch that is more in keeping
with Linux style, using spaces rather than dashes, and leading with
tcp: or tcp_agile_sd:. For example:
tcp: add Agile SD TCP congestion control module
And please read:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
Thanks,
neal
^ permalink raw reply
* Re: [PATCH v11 0/5] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
From: Eric Dumazet @ 2017-08-15 14:03 UTC (permalink / raw)
To: David Miller
Cc: dingtianhong, leedom, ashok.raj, bhelgaas, helgaas, werner,
ganeshgr, asit.k.mallick, patrick.j.cramer, Suravee.Suthikulpanit,
Bob.Shaw, l.stach, amira, gabriele.paoloni, David.Laight,
jeffrey.t.kirsher, catalin.marinas, will.deacon, mark.rutland,
robin.murphy, alexander.duyck, linux-arm-kernel, netdev,
linux-pci, linux-kernel, linuxarm
In-Reply-To: <1502805534.4936.73.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2017-08-15 at 06:58 -0700, Eric Dumazet wrote:
> On Mon, 2017-08-14 at 22:15 -0700, David Miller wrote:
> > From: Ding Tianhong <dingtianhong@huawei.com>
> > Date: Tue, 15 Aug 2017 11:23:22 +0800
> >
> > > Some devices have problems with Transaction Layer Packets with the Relaxed
> > > Ordering Attribute set. This patch set adds a new PCIe Device Flag,
> > > PCI_DEV_FLAGS_NO_RELAXED_ORDERING, a set of PCI Quirks to catch some known
> > > devices with Relaxed Ordering issues, and a use of this new flag by the
> > > cxgb4 driver to avoid using Relaxed Ordering with problematic Root Complex
> > > Ports.
> > ...
> >
> > Series applied, thanks.
>
> I got a NULL deref in pci_find_pcie_root_port()
>
This was :
[ 4.241029] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
[ 4.247001] IP: pci_find_pcie_root_port+0x62/0x80
[ 4.253011] PGD 0
[ 4.253011] P4D 0
[ 4.253011]
[ 4.258013] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 4.262015] Modules linked in:
[ 4.265005] CPU: 31 PID: 1 Comm: swapper/0 Not tainted 4.13.0-dbx-DEV #316
[ 4.271002] Hardware name: Intel RML,PCH/Iota_QC_19, BIOS 2.40.0 06/22/2016
[ 4.279002] task: ffffa2ee38cfa040 task.stack: ffffa51ec0004000
[ 4.285001] RIP: 0010:pci_find_pcie_root_port+0x62/0x80
[ 4.290012] RSP: 0000:ffffa51ec0007ab8 EFLAGS: 00010246
[ 4.295003] RAX: 0000000000000000 RBX: ffffa2ee36bae000 RCX: 0000000000000006
[ 4.303002] RDX: 000000000000081c RSI: ffffa2ee38cfa8c8 RDI: ffffa2ee36bae000
[ 4.310013] RBP: ffffa51ec0007b58 R08: 0000000000000001 R09: 0000000000000000
[ 4.317001] R10: 0000000000000000 R11: 0000000000000000 R12: ffffa51ec0007ad0
[ 4.324005] R13: ffffa2ee36bae098 R14: 0000000000000002 R15: ffffa2ee37204818
[ 4.331002] FS: 0000000000000000(0000) GS:ffffa2ee3fcc0000(0000) knlGS:0000000000000000
[ 4.339002] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4.345001] CR2: 0000000000000050 CR3: 000000401000f000 CR4: 00000000001406e0
[ 4.351002] Call Trace:
[ 4.354012] ? pci_configure_device+0x19f/0x570
[ 4.359002] ? pci_conf1_read+0xb8/0xf0
[ 4.363002] ? raw_pci_read+0x23/0x40
[ 4.366011] ? pci_read+0x2c/0x30
[ 4.370014] ? pci_read_config_word+0x67/0x70
[ 4.374012] pci_device_add+0x28/0x230
[ 4.378012] ? pci_vpd_f0_read+0x50/0x80
[ 4.382014] pci_scan_single_device+0x96/0xc0
[ 4.386012] pci_scan_slot+0x79/0xf0
[ 4.389001] pci_scan_child_bus+0x31/0x180
[ 4.394014] acpi_pci_root_create+0x1c6/0x240
[ 4.398013] pci_acpi_scan_root+0x15f/0x1b0
[ 4.402012] acpi_pci_root_add+0x2e6/0x400
[ 4.406012] ? acpi_evaluate_integer+0x37/0x60
[ 4.411002] acpi_bus_attach+0xdf/0x200
[ 4.415002] acpi_bus_attach+0x6a/0x200
[ 4.418014] acpi_bus_attach+0x6a/0x200
[ 4.422013] acpi_bus_scan+0x38/0x70
[ 4.426011] acpi_scan_init+0x10c/0x271
[ 4.429001] acpi_init+0x2fa/0x348
[ 4.433004] ? acpi_sleep_proc_init+0x2d/0x2d
[ 4.437001] do_one_initcall+0x43/0x169
[ 4.441001] kernel_init_freeable+0x1d0/0x258
[ 4.445003] ? rest_init+0xe0/0xe0
[ 4.449001] kernel_init+0xe/0x150
[ 4.451002] ret_from_fork+0x27/0x40
[ 4.457004] Code: 85 d2 74 27 80 7a 4a 00 74 21 48 89 d0 48 89 c2 f6 80 1b 09 00 00 10 74 07 48 8b 90 a0 0a 00 00 48 8b 52 10 48 83 7a 10 00 75 d0 <0f> b7 50 50 5d 81 e2 f0 00 00 00 83 fa 40 ba 00 00 00 00 48 0f
[ 4.474012] RIP: pci_find_pcie_root_port+0x62/0x80 RSP: ffffa51ec0007ab8
[ 4.481004] CR2: 0000000000000050
[ 4.484001] ---[ end trace 6f9be6a057581199 ]---
[ 4.488001] Kernel panic - not syncing: Fatal exception
[ 4.494013] Rebooting in 10 seconds..
[ 4.494013] ACPI MEMORY or I/O RESET_REG.
>
> This local hack seems to fix the issue.
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index af0cc3456dc1b48b1325c06c5edd2ca8cc22a640..cfd8eb5a3d0ba8347d44952ffab28d9c761044d3 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -522,7 +522,7 @@ struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
> bridge = pci_upstream_bridge(bridge);
> }
>
> - if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> + if (highest_pcie_bridge && pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> return NULL;
>
> return highest_pcie_bridge;
^ permalink raw reply
* Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for reset packet
From: Tom Herbert @ 2017-08-15 14:08 UTC (permalink / raw)
To: Shaohua Li; +Cc: Linux Kernel Network Developers, David S. Miller, Shaohua Li
In-Reply-To: <20170815025224.trjgcp3z6i3cgfy2@kernel.org>
On Mon, Aug 14, 2017 at 7:52 PM, Shaohua Li <shli@kernel.org> wrote:
> On Fri, Aug 11, 2017 at 06:00:20PM -0700, Tom Herbert wrote:
>> On Thu, Aug 10, 2017 at 12:13 PM, Shaohua Li <shli@kernel.org> wrote:
>> > On Thu, Aug 10, 2017 at 11:30:51AM -0700, Tom Herbert wrote:
>> >> On Thu, Aug 10, 2017 at 9:30 AM, Shaohua Li <shli@kernel.org> wrote:
>> >> > On Wed, Aug 09, 2017 at 09:40:08AM -0700, Tom Herbert wrote:
>> >> >> On Mon, Jul 31, 2017 at 3:19 PM, Shaohua Li <shli@kernel.org> wrote:
>> >> >> > From: Shaohua Li <shli@fb.com>
>> >> >> >
>> >> >> > Please see below tcpdump output:
>> >> >> > 21:00:48.109122 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [S], cksum 0x0529 (incorrect -> 0xf56c), seq 3282214508, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 0,nop,wscale 7], length 0
>> >> >> > 21:00:48.109381 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 40) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [S.], cksum 0x0529 (incorrect -> 0x49ad), seq 1923801573, ack 3282214509, win 43690, options [mss 65476,sackOK,TS val 2500903437 ecr 2500903437,nop,wscale 7], length 0
>> >> >> > 21:00:48.109548 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1bdf), seq 1, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0
>> >> >> > 21:00:48.109823 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 62) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x053f (incorrect -> 0xb8b1), seq 1:31, ack 1, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 30
>> >> >> > 21:00:48.109910 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [.], cksum 0x0521 (incorrect -> 0x1bc1), seq 1, ack 31, win 342, options [nop,nop,TS val 2500903437 ecr 2500903437], length 0
>> >> >> > 21:00:48.110043 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [P.], cksum 0x0539 (incorrect -> 0xb726), seq 1:25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 24
>> >> >> > 21:00:48.110173 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba7), seq 31, ack 25, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0
>> >> >> > 21:00:48.110211 IP6 (flowlabel 0xd827f, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [F.], cksum 0x0521 (incorrect -> 0x1ba7), seq 25, ack 31, win 342, options [nop,nop,TS val 2500903438 ecr 2500903437], length 0
>> >> >> > 21:00:48.151099 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 32) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [.], cksum 0x0521 (incorrect -> 0x1ba6), seq 31, ack 26, win 342, options [nop,nop,TS val 2500903438 ecr 2500903438], length 0
>> >> >> > 21:00:49.110524 IP6 (flowlabel 0x43304, hlim 64, next-header TCP (6) payload length: 56) fec0::5054:ff:fe12:3456.55804 > fec0::5054:ff:fe12:3456.5555: Flags [P.], cksum 0x0539 (incorrect -> 0xb324), seq 31:55, ack 26, win 342, options [nop,nop,TS val 2500904438 ecr 2500903438], length 24
>> >> >> > 21:00:49.110637 IP6 (flowlabel 0xb34d5, hlim 64, next-header TCP (6) payload length: 20) fec0::5054:ff:fe12:3456.5555 > fec0::5054:ff:fe12:3456.55804: Flags [R], cksum 0x0515 (incorrect -> 0x668c), seq 1923801599, win 0, length 0
>> >> >> >
>> >> >> > The flowlabel of reset packet (0xb34d5) and flowlabel of normal packet
>> >> >> > (0xd827f) are different. This causes our router doesn't correctly close tcp
>> >> >> > connection. The patches try to fix the issue.
>> >> >> >
>> >> >> Shaohua,
>> >> >>
>> >> >> Can you give some more detail about what the router doesn't close the
>> >> >> TCP connection means? I'm guessing the problem is either: 1) the
>> >> >> router is maintaining connection state that includes the flow label in
>> >> >> a connection tuple. 2) some router in the path is maintaining
>> >> >> connection state, but when the flow label changes the flow's packet
>> >> >> are routed through a different router that doesn't have a state for
>> >> >> the flow it drops the packet. #1 should be easily fix in the router,
>> >> >> flow labels cannot be used as state. #2 is the known problem that
>> >> >> stateful firewalls have killed our ability to use multihoming.
>> >> >
>> >> > The #2 is exactly the problem we saw.
>> >> >
>> >> >> Another consideration is that sk_txhash is also used in routing
>> >> >> decisions by the local host (flow label is normally derived from
>> >> >> txhash). If you want to ensure that connections are routed
>> >> >> consistently for timewait state you might need sk_txhash saved also.
>> >> >
>> >> > As far as I understood, we don't use sk_txhash for routing selection. The code
>> >> > does routing selection with flowlabel user configured, at that time we don't
>> >> > derive fl6.flowlabel from skb->hash (which is from sk_txhash). The code always
>> >> > does routing selection first and then uses ip6_make_flowlabel to build packet
>> >> > data where we derive flowlabel from skb->hash.
>> >> >
>> >> That is assuming one particular use case. Generally, if you want to
>> >> ensure all packets for a flow take the same path you'll need tx_hash
>> >> and make it persistent (disable flow bender). For instance, if you
>> >> were doing UDP encapsulation like in VXLAN the UDP source port
>> >> selection is unaffected by saved flow label for the lifetime of the
>> >> flow. So we would still hit #2 in that case and the stateful device
>> >> doesn't see whole flow. It might be just as easy to move tx_hash in
>> >> skc_common so that it's available in TW state for this purpose. Then
>> >> when moving to TW state just copy the tx_hash.
>> >
>> > Hi Tom,
>> >
>> > My original implementation is to add a tx_hash in tw sock, we then copy sock's
>> > tx_hash to the tw tx_hash. This does makes things simplier. One concern from
>> > Eric is this will increase the size of tw sock. If we move tx_hash to
>> > skc_common, all sock size will increase, is this acceptable?
>>
>> I think that can only be measured by how critical it is to
>> persistently route all packets the same exact way for every
>> connection. Page one of the IP book clearly states that IP packets can
>> be dropped, duplicated, or received out of order. Received OOO implies
>> that packet for the same flow are allowed to take different paths. The
>> requirement that packets for the same flow must always take the same
>> path through the network was created by stateful middleboxes-- it's
>> not inherent in the architecture of IP networking. Unfortunately,
>> we're seeing this become more and more of a problem as more devices
>> are multi-homed (like smart phones) and these network requirement
>> cripple our ability to take advantage of features like that.
>>
>> Personally, I wish the middleboxes fix the problem they created, but I
>> suppose we need to be pragmatic at least in the short term.
>
> Hmm, I still hesitate to add a new field in skc_common. Fixing current problem
> looks propriate in current stage. I'd defer fixing the generic issue till it's
> necessary.
>
Shaohua,
An alternative would be to not initialize sk_txhash, but instead defer
hash computation to use flow dissector in the TX path when the hash is
needed (to get flow label, src port for UDP encap, route for
multipath, etc.). At the first hash computation in TX path the result
in sk_txhash. In TW state there is no socket so flow dissector is
always used but that should yield the same hash. No extra fields would
be needed and additional cost is negligible.
Tom
> Thanks,
> Shaohua
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox