From cd35c954234eb8773192ea8a503f4fcf76e2aa83 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Tue, 21 May 2019 11:27:14 -0700 Subject: [PATCH 01/11] man: fix macaddr section of ip-link The formatting of setting mac address was confusing. Break lines and fix highlighting. Signed-off-by: Stephen Hemminger --- man/man8/ip-link.8.in | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index d035a5c9..883d8807 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -154,10 +154,15 @@ ip-link \- network device configuration .br .RB "[ " addrgenmode " { " eui64 " | " none " | " stable_secret " | " random " } ]" .br -.RB "[ " macaddr " { " flush " | { " add " | " del " } " -.IR MACADDR " | set [ " -.IR MACADDR " [ " -.IR MACADDR " [ ... ] ] ] } ]" +.RB "[ " macaddr +.RI "[ " MACADDR " ]" +.br +.in +10 +.RB "[ { " flush " | " add " | " del " } " +.IR MACADDR " ]" +.br +.RB "[ " set +.IR MACADDR " ] ]" .br .ti -8 From 6eccf7ecdb010a90e5271942748ef4338ddb61ae Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Mon, 20 May 2019 11:56:52 +0200 Subject: [PATCH 02/11] m_mirred: don't bail if the control action is missing The mirred act admits an optional control action, defaulting to TC_ACT_PIPE. The parsing code currently emits an error message if the control action is not provided on the command line, even if the command itself completes with no error. This change shuts down the error message, using the appropriate parsing helper. Fixes: e67aba559581 ("tc: actions: add helpers to parse and print control actions") Signed-off-by: Paolo Abeni Signed-off-by: Stephen Hemminger --- tc/m_mirred.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tc/m_mirred.c b/tc/m_mirred.c index c7f7318b..23ba638a 100644 --- a/tc/m_mirred.c +++ b/tc/m_mirred.c @@ -202,7 +202,8 @@ parse_direction(struct action_util *a, int *argc_p, char ***argv_p, if (p.eaction == TCA_EGRESS_MIRROR || p.eaction == TCA_INGRESS_MIRROR) - parse_action_control(&argc, &argv, &p.action, false); + parse_action_control_dflt(&argc, &argv, &p.action, false, + TC_ACT_PIPE); if (argc) { if (iok && matches(*argv, "index") == 0) { From 1bb38f6c5e46d33bc983541a814178690646ae46 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Fri, 24 May 2019 15:51:06 -0700 Subject: [PATCH 03/11] uapi: minor upstream btf.h header change Signed-off-by: Stephen Hemminger --- include/uapi/linux/btf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h index 73eba2e5..8ef8001d 100644 --- a/include/uapi/linux/btf.h +++ b/include/uapi/linux/btf.h @@ -83,7 +83,7 @@ struct btf_type { * is the 32 bits arrangement: */ #define BTF_INT_ENCODING(VAL) (((VAL) & 0x0f000000) >> 24) -#define BTF_INT_OFFSET(VAL) (((VAL & 0x00ff0000)) >> 16) +#define BTF_INT_OFFSET(VAL) (((VAL) & 0x00ff0000) >> 16) #define BTF_INT_BITS(VAL) ((VAL) & 0x000000ff) /* Attributes stored in the BTF_INT_ENCODING */ From 757837230a654d39623d0b90882b695a2facd107 Mon Sep 17 00:00:00 2001 From: Nicolas Dichtel Date: Fri, 24 May 2019 10:59:10 +0200 Subject: [PATCH 04/11] lib: suppress error msg when filling the cache Before the patch: $ ip netns add foo $ ip link add name veth1 address 2a:a5:5c:b9:52:89 type veth peer name veth2 address 2a:a5:5c:b9:53:90 netns foo RTNETLINK answers: No such device RTNETLINK answers: No such device But the command was successful. This may break script. Let's remove those error messages. Fixes: 55870dfe7f8b ("Improve batch and dump times by caching link lookups") Reported-by: Philippe Guibert Signed-off-by: Nicolas Dichtel Signed-off-by: Stephen Hemminger --- lib/ll_map.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ll_map.c b/lib/ll_map.c index 2d7b65dc..e0ed54bf 100644 --- a/lib/ll_map.c +++ b/lib/ll_map.c @@ -177,7 +177,7 @@ static int ll_link_get(const char *name, int index) addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name, strlen(name) + 1); - if (rtnl_talk(&rth, &req.n, &answer) < 0) + if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0) goto out; /* add entry to cache */ From 767b6fd620ddc4319a121595f953313b2e9789ff Mon Sep 17 00:00:00 2001 From: Lukasz Czapnik Date: Mon, 27 May 2019 23:03:49 +0200 Subject: [PATCH 05/11] tc: flower: fix port value truncation sscanf truncates read port values silently without any error. As sscanf man says: (...) sscanf() conform to C89 and C99 and POSIX.1-2001. These standards do not specify the ERANGE error. Replace sscanf with safer get_be16 that returns error when value is out of range. Example: tc filter add dev eth0 protocol ip parent ffff: prio 1 flower ip_proto tcp dst_port 70000 hw_tc 1 Would result in filter for port 4464 without any warning. Fixes: 8930840e678b ("tc: flower: Classify packets based port ranges") Signed-off-by: Lukasz Czapnik Signed-off-by: Stephen Hemminger --- tc/f_flower.c | 48 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/tc/f_flower.c b/tc/f_flower.c index 9659e894..e2420d92 100644 --- a/tc/f_flower.c +++ b/tc/f_flower.c @@ -493,23 +493,40 @@ static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type, return 0; } +/* parse range args in format 10-20 */ +static int parse_range(char *str, __be16 *min, __be16 *max) +{ + char *sep; + + sep = strchr(str, '-'); + if (sep) { + *sep = '\0'; + + if (get_be16(min, str, 10)) + return -1; + + if (get_be16(max, sep + 1, 10)) + return -1; + } else { + if (get_be16(min, str, 10)) + return -1; + } + return 0; +} + static int flower_parse_port(char *str, __u8 ip_proto, enum flower_endpoint endpoint, struct nlmsghdr *n) { - __u16 min, max; + __be16 min = 0; + __be16 max = 0; int ret; - ret = sscanf(str, "%hu-%hu", &min, &max); + ret = parse_range(str, &min, &max); + if (ret) + return -1; - if (ret == 1) { - int type; - - type = flower_port_attr_type(ip_proto, endpoint); - if (type < 0) - return -1; - addattr16(n, MAX_MSG, type, htons(min)); - } else if (ret == 2) { + if (min && max) { __be16 min_port_type, max_port_type; if (max <= min) { @@ -520,8 +537,15 @@ static int flower_parse_port(char *str, __u8 ip_proto, &min_port_type, &max_port_type)) return -1; - addattr16(n, MAX_MSG, min_port_type, htons(min)); - addattr16(n, MAX_MSG, max_port_type, htons(max)); + addattr16(n, MAX_MSG, min_port_type, min); + addattr16(n, MAX_MSG, max_port_type, max); + } else if (min && !max) { + int type; + + type = flower_port_attr_type(ip_proto, endpoint); + if (type < 0) + return -1; + addattr16(n, MAX_MSG, type, min); } else { return -1; } From a9661b8b0fd787711fdfc8299afc2c1725bae131 Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Wed, 29 May 2019 20:52:42 +0300 Subject: [PATCH 06/11] bridge: mdb: restore text output format While I fixed the mdb json output, I did overlook the text output. This patch returns the original text output format: dev port grp Example (old format, restored by this patch): dev br0 port eth8 grp 239.1.1.11 temp Example (changed format after the commit below): 23: br0 eth8 239.1.1.11 temp We had some reports of failing scripts which were parsing the output. Also the old format matches the bridge mdb command syntax which makes it easier to build commands out of the output. Fixes: c7c1a1ef51ae ("bridge: colorize output and use JSON print library") Signed-off-by: Nikolay Aleksandrov Signed-off-by: Stephen Hemminger --- bridge/mdb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bridge/mdb.c b/bridge/mdb.c index 59aa1764..eadc6212 100644 --- a/bridge/mdb.c +++ b/bridge/mdb.c @@ -132,21 +132,21 @@ static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e, open_json_object(NULL); - print_int(PRINT_ANY, "index", "%u: ", ifindex); - print_color_string(PRINT_ANY, COLOR_IFNAME, "dev", "%s ", dev); - print_string(PRINT_ANY, "port", " %s ", + print_int(PRINT_JSON, "index", NULL, ifindex); + print_color_string(PRINT_ANY, COLOR_IFNAME, "dev", "dev %s", dev); + print_string(PRINT_ANY, "port", " port %s", ll_index_to_name(e->ifindex)); print_color_string(PRINT_ANY, ifa_family_color(af), - "grp", " %s ", + "grp", " grp %s", inet_ntop(af, src, abuf, sizeof(abuf))); - print_string(PRINT_ANY, "state", " %s ", + print_string(PRINT_ANY, "state", " %s", (e->state & MDB_PERMANENT) ? "permanent" : "temp"); open_json_array(PRINT_JSON, "flags"); if (e->flags & MDB_FLAGS_OFFLOAD) - print_string(PRINT_ANY, NULL, "%s ", "offload"); + print_string(PRINT_ANY, NULL, " %s", "offload"); close_json_array(PRINT_JSON, NULL); if (e->vid) From c442234858a6bf3ee954f5e482dac9486ece5f17 Mon Sep 17 00:00:00 2001 From: Nicolas Dichtel Date: Wed, 29 May 2019 16:42:10 +0200 Subject: [PATCH 07/11] iplink: don't try to get ll addr len when creating an iface It will obviously fail. This is a follow up of the commit 757837230a65 ("lib: suppress error msg when filling the cache"). Suggested-by: David Ahern Signed-off-by: Nicolas Dichtel Signed-off-by: Stephen Hemminger --- ip/iplink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ip/iplink.c b/ip/iplink.c index 7952cb2b..d275efa9 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -945,7 +945,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type) else if (!strcmp(name, dev)) name = dev; - if (dev && addr_len) { + if (dev && addr_len && + !(req->n.nlmsg_flags & NLM_F_CREATE)) { int halen = nl_get_ll_addr_len(dev); if (halen >= 0 && halen != addr_len) { From 4ae441e3d17e129ecf422ce5f65348fb1930cf9f Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Fri, 31 May 2019 14:12:15 +0200 Subject: [PATCH 08/11] man: tc-skbedit.8: document 'inheritdsfield' while at it, fix missing square bracket near 'ptype' and a typo in the action description (it's -> its). Signed-off-by: Davide Caratti Acked-by: Andrea Claudi Signed-off-by: Stephen Hemminger --- man/man8/tc-skbedit.8 | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/man/man8/tc-skbedit.8 b/man/man8/tc-skbedit.8 index 003f05c9..24591982 100644 --- a/man/man8/tc-skbedit.8 +++ b/man/man8/tc-skbedit.8 @@ -10,9 +10,10 @@ skbedit - SKB editing action .B priority .IR PRIORITY " ] [" .B mark -.IR MARK " ]" +.IR MARK " ] [" .B ptype -.IR PTYPE " ]" +.IR PTYPE " ] [" +.BR inheritdsfield " ]" .SH DESCRIPTION The .B skbedit @@ -22,7 +23,7 @@ action, which in turn allows to change parts of the packet data itself. The most unique feature of .B skbedit -is it's ability to decide over which queue of an interface with multiple +is its ability to decide over which queue of an interface with multiple transmit queues the packet is to be sent out. The number of available transmit queues is reflected by sysfs entries within .I /sys/class/net//queues @@ -61,6 +62,12 @@ needing to allow ingressing packets with the wrong MAC address but correct IP address. .I PTYPE is one of: host, otherhost, broadcast, multicast +.TP +.BI inheritdsfield +Override the packet classification decision, and any value specified with +.BR priority ", " +using the information stored in the Differentiated Services Field of the +IPv6/IPv4 header (RFC2474). .SH SEE ALSO .BR tc (8), .BR tc-pedit (8) From 2cc10ce81d22676a3eff02dd57dd524828489449 Mon Sep 17 00:00:00 2001 From: Parav Pandit Date: Thu, 6 Jun 2019 06:49:19 -0500 Subject: [PATCH 09/11] devlink: Increase bus, device buffer size to 64 bytes Device name on mdev bus is 36 characters long which follow standard uuid RFC 4122. This is probably the longest name that a kernel will return for a device. Hence increase the buffer size to 64 bytes. Acked-by: Jiri Pirko Signed-off-by: Parav Pandit Signed-off-by: Stephen Hemminger --- devlink/devlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devlink/devlink.c b/devlink/devlink.c index 436935f8..559f624e 100644 --- a/devlink/devlink.c +++ b/devlink/devlink.c @@ -1523,7 +1523,7 @@ static void __pr_out_handle_start(struct dl *dl, struct nlattr **tb, { const char *bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]); const char *dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]); - char buf[32]; + char buf[64]; sprintf(buf, "%s/%s", bus_name, dev_name); @@ -1616,7 +1616,7 @@ static void __pr_out_port_handle_start(struct dl *dl, const char *bus_name, uint32_t port_index, bool try_nice, bool array) { - static char buf[32]; + static char buf[64]; char *ifname = NULL; if (dl->no_nice_names || !try_nice || From fa495889731430ebf3f4d054cec221b9a8eb483e Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Thu, 6 Jun 2019 17:32:09 -0400 Subject: [PATCH 10/11] tc: Fix binding of gact action by index. The following operation fails: % sudo tc actions add action pipe index 1 % sudo tc filter add dev lo parent ffff: \ protocol ip pref 10 u32 match ip src 127.0.0.2 \ flowid 1:10 action gact index 1 Bad action type index Usage: ... gact [RAND] [INDEX] Where: ACTION := reclassify | drop | continue | pass | pipe | goto chain | jump RAND := random RANDTYPE := netrand | determ VAL : = value not exceeding 10000 JUMP_COUNT := Absolute jump from start of action list INDEX := index value used However, passing a control action of gact rule during filter binding works: % sudo tc filter add dev lo parent ffff: \ protocol ip pref 10 u32 match ip src 127.0.0.2 \ flowid 1:10 action gact pipe index 1 Binding by reference, i.e. by index, has to consistently work with any tc action. Since tc is sensitive to the order of keywords passed on the command line, we can teach gact to skip parsing arguments as soon as it sees 'gact' followed by 'index' keyword. Signed-off-by: Roman Mashak Signed-off-by: Stephen Hemminger --- tc/m_gact.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tc/m_gact.c b/tc/m_gact.c index a0a3c33d..5b781e16 100644 --- a/tc/m_gact.c +++ b/tc/m_gact.c @@ -89,6 +89,9 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p, if (!matches(*argv, "gact")) NEXT_ARG_FWD(); + /* we're binding existing gact action to filter by index. */ + if (!matches(*argv, "index")) + goto skip_args; if (parse_action_control(&argc, &argv, &p.action, false)) usage(); /* does not return */ @@ -133,6 +136,7 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p, if (argc > 0) { if (matches(*argv, "index") == 0) { +skip_args: NEXT_ARG(); if (get_u32(&p.index, *argv, 10)) { fprintf(stderr, "Illegal \"index\"\n"); From 0ee4d17954f665276e9216c0ec7650ff74b0709d Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Wed, 5 Jun 2019 00:30:16 +0200 Subject: [PATCH 11/11] tc: simple: don't hardcode the control action the following TDC test case: b776 - Replace simple action with invalid goto chain control checks if the kernel correctly validates the 'goto chain' control action, when it is specified in 'act_simple' rules. The test systematically fails because the control action is hardcoded in parse_simple(), i.e. it is not parsed by command line arguments, so its value is constantly TC_ACT_PIPE. Because of that, the following command: # tc action add action simple sdata "test" drop index 7 installs an 'act_simple' rule that never drops packets, and whose 'index' is the first IDR available, plus an 'act_gact' rule with 'index' equal to 7, that drops packets. Use parse_action_control_dflt(), like we did on many other TC actions, to make the control action configurable also with 'act_simple'. The expected results of test b776 are summarized below: iproute2 v kernel->| 5.1-rc2 (and previous) | 5.1-rc3 (and subsequent) ------------------+-------------------------+------------------------- 5.1.0 | FAIL (bad IDR) | FAIL (bad IDR) 5.1.0(patched) | FAIL (no rule/bad sdata)| PASS Changes since v1: - reword commit message, thanks Stephen Hemminger Fixes: 087f46ee4ebd ("tc: introduce simple action") CC: Andrea Claudi CC: Marcelo Ricardo Leitner Signed-off-by: Davide Caratti Signed-off-by: Stephen Hemminger --- tc/m_simple.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tc/m_simple.c b/tc/m_simple.c index 886606f9..e3c8a60f 100644 --- a/tc/m_simple.c +++ b/tc/m_simple.c @@ -119,6 +119,9 @@ parse_simple(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, } } + parse_action_control_dflt(&argc, &argv, &sel.action, false, + TC_ACT_PIPE); + if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); @@ -144,8 +147,6 @@ parse_simple(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, return -1; } - sel.action = TC_ACT_PIPE; - tail = addattr_nest(n, MAX_MSG, tca_id); addattr_l(n, MAX_MSG, TCA_DEF_PARMS, &sel, sizeof(sel)); if (simpdata)