Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 iproute2 8/9] man pages: Add tc-ife to Makefile
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Jamal Hadi Salim <jhs@mojatatu.com>

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 man/man8/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/Makefile b/man/man8/Makefile
index 9213769..4ad96ce 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -16,7 +16,7 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 rtpr.8 ss.
 	tc-basic.8 tc-cgroup.8 tc-flow.8 tc-flower.8 tc-fw.8 tc-route.8 \
 	tc-tcindex.8 tc-u32.8 tc-matchall.8 \
 	tc-connmark.8 tc-csum.8 tc-mirred.8 tc-nat.8 tc-pedit.8 tc-police.8 \
-	tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8 \
+	tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8  tc-ife.8 \
 	devlink.8 devlink-dev.8 devlink-monitor.8 devlink-port.8 devlink-sb.8
 
 all: $(TARGETS)
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 iproute2 4/9] actions ife: Introduce encoding and decoding of tcindex metadata
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Jamal Hadi Salim <jhs@mojatatu.com>

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/linux/tc_act/tc_ife.h |  3 ++-
 tc/m_ife.c                    | 29 +++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/include/linux/tc_act/tc_ife.h b/include/linux/tc_act/tc_ife.h
index 4ece02a..cd18360 100644
--- a/include/linux/tc_act/tc_ife.h
+++ b/include/linux/tc_act/tc_ife.h
@@ -32,8 +32,9 @@ enum {
 #define IFE_META_HASHID 2
 #define	IFE_META_PRIO 3
 #define	IFE_META_QMAP 4
+#define	IFE_META_TCINDEX 5
 /*Can be overridden at runtime by module option*/
-#define	__IFE_META_MAX 5
+#define	__IFE_META_MAX 6
 #define IFE_META_MAX (__IFE_META_MAX - 1)
 
 #endif
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 862461b..e6f6153 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -67,6 +67,8 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 	__u32 ife_prio_v = 0;
 	__u32 ife_mark = 0;
 	__u32 ife_mark_v = 0;
+	__u16 ife_tcindex = 0;
+	__u16 ife_tcindex_v = 0;
 	char *daddr = NULL;
 	char *saddr = NULL;
 
@@ -89,6 +91,8 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 				ife_mark = IFE_META_SKBMARK;
 			} else if (matches(*argv, "prio") == 0) {
 				ife_prio = IFE_META_PRIO;
+			} else if (matches(*argv, "tcindex") == 0) {
+				ife_prio = IFE_META_TCINDEX;
 			} else {
 				fprintf(stderr, "Illegal meta define <%s>\n",
 					*argv);
@@ -106,6 +110,11 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 				if (get_u32(&ife_prio_v, *argv, 0))
 					invarg("ife prio val is invalid",
 					       *argv);
+			} else if (matches(*argv, "tcindex") == 0) {
+				NEXT_ARG();
+				if (get_u16(&ife_tcindex_v, *argv, 0))
+					invarg("ife tcindex val is invalid",
+					       *argv);
 			} else {
 				fprintf(stderr, "Illegal meta use type <%s>\n",
 					*argv);
@@ -196,6 +205,13 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 		else
 			addattr_l(n, MAX_MSG, IFE_META_PRIO, NULL, 0);
 	}
+	if (ife_tcindex || ife_tcindex_v) {
+		if (ife_tcindex_v)
+			addattr_l(n, MAX_MSG, IFE_META_TCINDEX, &ife_tcindex_v,
+				  2);
+		else
+			addattr_l(n, MAX_MSG, IFE_META_TCINDEX, NULL, 0);
+	}
 
 	tail2->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail2;
 
@@ -213,7 +229,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 	struct rtattr *tb[TCA_IFE_MAX + 1];
 	__u16 ife_type = 0;
 	__u32 mmark = 0;
-	__u32 mhash = 0;
+	__u16 mtcindex = 0;
 	__u32 mprio = 0;
 	int has_optional = 0;
 	SPRINT_BUF(b2);
@@ -258,13 +274,14 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 				fprintf(f, "allow mark ");
 		}
 
-		if (metalist[IFE_META_HASHID]) {
-			len = RTA_PAYLOAD(metalist[IFE_META_HASHID]);
+		if (metalist[IFE_META_TCINDEX]) {
+			len = RTA_PAYLOAD(metalist[IFE_META_TCINDEX]);
 			if (len) {
-				mhash = rta_getattr_u32(metalist[IFE_META_HASHID]);
-				fprintf(f, "use hash %u ", mhash);
+				mtcindex =
+					rta_getattr_u16(metalist[IFE_META_TCINDEX]);
+				fprintf(f, "use tcindex %d ", mtcindex);
 			} else
-				fprintf(f, "allow hash ");
+				fprintf(f, "allow tcindex ");
 		}
 
 		if (metalist[IFE_META_PRIO]) {
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 iproute2 1/9] ife action: allow specifying index in hex
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Roman Mashak <mrv@mojatatu.com>

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/m_ife.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index 0219760..a5a7516 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -152,7 +152,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 	if (argc) {
 		if (matches(*argv, "index") == 0) {
 			NEXT_ARG();
-			if (get_u32(&p.index, *argv, 10)) {
+			if (get_u32(&p.index, *argv, 0)) {
 				fprintf(stderr, "ife: Illegal \"index\"\n");
 				return -1;
 			}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 iproute2 5/9] action gact: list pipe as a valid action
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Craig Dillabaugh <cdillaba@mojatatu.com>

Signed-off-by: Craig Dillabaugh <cdillaba@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/m_gact.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tc/m_gact.c b/tc/m_gact.c
index 2bfd9a7..dc04b9f 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -45,7 +45,7 @@ explain(void)
 #ifdef CONFIG_GACT_PROB
 	fprintf(stderr, "Usage: ... gact <ACTION> [RAND] [INDEX]\n");
 	fprintf(stderr,
-		"Where: \tACTION := reclassify | drop | continue | pass\n"
+		"Where: \tACTION := reclassify | drop | continue | pass | pipe\n"
 			"\tRAND := random <RANDTYPE> <ACTION> <VAL>\n"
 			"\tRANDTYPE := netrand | determ\n"
 			"\tVAL : = value not exceeding 10000\n"
@@ -54,7 +54,7 @@ explain(void)
 #else
 	fprintf(stderr, "Usage: ... gact <ACTION> [INDEX]\n");
 	fprintf(stderr,
-		"Where: \tACTION := reclassify | drop | continue | pass\n"
+		"Where: \tACTION := reclassify | drop | continue | pass | pipe\n"
 		"\tINDEX := index value used\n"
 		"\n");
 #endif
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 iproute2 3/9] ife: improve help text
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Roman Mashak <mrv@mojatatu.com>

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/m_ife.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index 588bad7..862461b 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -29,12 +29,13 @@
 static void ife_explain(void)
 {
 	fprintf(stderr,
-		"Usage:... ife {decode|encode} {ALLOW|USE} [dst DMAC] [src SMAC] [type TYPE] [CONTROL] [index INDEX]\n");
+		"Usage:... ife {decode|encode} [{ALLOW|USE} ATTR] [dst DMAC] [src SMAC] [type TYPE] [CONTROL] [index INDEX]\n");
 	fprintf(stderr,
 		"\tALLOW := Encode direction. Allows encoding specified metadata\n"
 		"\t\t e.g \"allow mark\"\n"
 		"\tUSE := Encode direction. Enforce Static encoding of specified metadata\n"
 		"\t\t e.g \"use mark 0x12\"\n"
+		"\tATTR := mark (32-bit), prio (32-bit), tcindex (16-bit)\n"
 		"\tDMAC := 6 byte Destination MAC address to encode\n"
 		"\tSMAC := optional 6 byte Source MAC address to encode\n"
 		"\tTYPE := optional 16 bit ethertype to encode\n"
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 iproute2 9/9] man pages: add man page for skbmod action
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Lucas Bates <lucasb@mojatatu.com>

Signed-off-by: Lucas Bates <lucasb@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 man/man8/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/Makefile b/man/man8/Makefile
index 4ad96ce..de6f249 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -16,7 +16,7 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 rtpr.8 ss.
 	tc-basic.8 tc-cgroup.8 tc-flow.8 tc-flower.8 tc-fw.8 tc-route.8 \
 	tc-tcindex.8 tc-u32.8 tc-matchall.8 \
 	tc-connmark.8 tc-csum.8 tc-mirred.8 tc-nat.8 tc-pedit.8 tc-police.8 \
-	tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8  tc-ife.8 \
+	tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8  tc-ife.8 tc-skbmod.8 \
 	devlink.8 devlink-dev.8 devlink-monitor.8 devlink-port.8 devlink-sb.8
 
 all: $(TARGETS)
-- 
1.9.1

^ permalink raw reply related

* RE: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: David Laight @ 2016-10-11 10:50 UTC (permalink / raw)
  To: 'Tom Herbert', davem@davemloft.net,
	netdev@vger.kernel.org, saeedm@mellanox.com
  Cc: kernel-team@fb.com
In-Reply-To: <20161011042222.2366145-1-tom@herbertland.com>

From: Tom Herbert
> Sent: 11 October 2016 05:22
...
> Fix is to create MLX5_SET64_VCHK that takes an additional argument
> that is a constant. There are two callers of MLX5_SET64 that are
> trying to get a variable offset, change those to call MLX5_SET64_VCHK
> passing pas[0] as the argument to use in the offset check.

I think I'd separate the array index instead.
Something like:

#define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
	BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
	__MLX5_SET64(typ, p, fld[ndx], v); \
} while (0)

	David

^ permalink raw reply

* [PATCH v2 iproute2 6/9] actions:  add skbmod action
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Jamal Hadi Salim <jhs@mojatatu.com>

This action is intended to be an upgrade from a usability perspective
from pedit (as well as operational debugability).
Compare this:

sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
u32 match ip protocol 1 0xff flowid 1:2 \
action pedit munge offset -14 u8 set 0x02 \
    munge offset -13 u8 set 0x15 \
    munge offset -12 u8 set 0x15 \
    munge offset -11 u8 set 0x15 \
    munge offset -10 u16 set 0x1515 \
    pipe

to:

sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
u32 match ip protocol 1 0xff flowid 1:2 \
action skbmod dmac 02:15:15:15:15:15

Or worse, try to debug a policy with destination mac, source mac and
etherype. Then make that a hundred rules and you'll get my point.

The most important ethernet use case at the moment is when redirecting or
mirroring packets to a remote machine. The dst mac address needs a re-write
so that it doesn't get dropped or confuse an interconnecting (learning) switch
or dropped by a target machine (which looks at the dst mac).

In the future common use cases on pedit can be migrated to this action
(as an example different fields in ip v4/6, transports like tcp/udp/sctp
etc). For this first cut, this allows modifying basic ethernet header.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/m_skbmod.c | 260 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 260 insertions(+)
 create mode 100644 tc/m_skbmod.c

diff --git a/tc/m_skbmod.c b/tc/m_skbmod.c
new file mode 100644
index 0000000..b7f4765
--- /dev/null
+++ b/tc/m_skbmod.c
@@ -0,0 +1,260 @@
+/*
+ * m_skbmod.c	skb modifier action module
+ *
+ *		This program is free software; you can distribute 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:  J Hadi Salim (jhs@mojatatu.com)
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include <linux/netdevice.h>
+
+#include "rt_names.h"
+#include "utils.h"
+#include "tc_util.h"
+#include <linux/tc_act/tc_skbmod.h>
+
+static void skbmod_explain(void)
+{
+	fprintf(stderr,
+		"Usage:... skbmod {[set <SETTABLE>] [swap <SWAPABLE>]} [CONTROL] [index INDEX]\n");
+	fprintf(stderr, "where SETTABLE is: [dmac DMAC] [smac SMAC] [etype ETYPE] \n");
+	fprintf(stderr, "where SWAPABLE is: \"mac\" to swap mac addresses\n");
+	fprintf(stderr, "note: \"swap mac\" is done after any outstanding D/SMAC change\n");
+	fprintf(stderr,
+		"\tDMAC := 6 byte Destination MAC address\n"
+		"\tSMAC := optional 6 byte Source MAC address\n"
+		"\tETYPE := optional 16 bit ethertype\n"
+		"\tCONTROL := reclassify|pipe|drop|continue|ok\n"
+		"\tINDEX := skbmod index value to use\n");
+}
+
+static void skbmod_usage(void)
+{
+	skbmod_explain();
+	exit(-1);
+}
+
+static int parse_skbmod(struct action_util *a, int *argc_p, char ***argv_p,
+			int tca_id, struct nlmsghdr *n)
+{
+	int argc = *argc_p;
+	char **argv = *argv_p;
+	int ok = 0;
+	struct tc_skbmod p;
+	struct rtattr *tail;
+	char dbuf[ETH_ALEN];
+	char sbuf[ETH_ALEN];
+	__u16 skbmod_etype = 0;
+	char *daddr = NULL;
+	char *saddr = NULL;
+
+	memset(&p, 0, sizeof(p));
+	p.action = TC_ACT_PIPE;	/* good default */
+
+	if (argc <= 0)
+		return -1;
+
+	while (argc > 0) {
+		if (matches(*argv, "skbmod") == 0) {
+			NEXT_ARG();
+			continue;
+		} else if (matches(*argv, "swap") == 0) {
+			NEXT_ARG();
+			continue;
+		} else if (matches(*argv, "mac") == 0) {
+			p.flags |= SKBMOD_F_SWAPMAC;
+			ok += 1;
+		} else if (matches(*argv, "set") == 0) {
+			NEXT_ARG();
+			continue;
+		} else if (matches(*argv, "etype") == 0) {
+			NEXT_ARG();
+			if (get_u16(&skbmod_etype, *argv, 0))
+				invarg("ethertype is invalid", *argv);
+			fprintf(stderr, "skbmod etype 0x%x\n", skbmod_etype);
+			p.flags |= SKBMOD_F_ETYPE;
+			ok += 1;
+		} else if (matches(*argv, "dmac") == 0) {
+			NEXT_ARG();
+			daddr = *argv;
+			if (sscanf(daddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+				   dbuf, dbuf + 1, dbuf + 2,
+				   dbuf + 3, dbuf + 4, dbuf + 5) != 6) {
+				fprintf(stderr, "Invalid dst mac address %s\n",
+					daddr);
+				return -1;
+			}
+			p.flags |= SKBMOD_F_DMAC;
+			fprintf(stderr, "dst MAC address <%s>\n", daddr);
+			ok += 1;
+
+		} else if (matches(*argv, "smac") == 0) {
+			NEXT_ARG();
+			saddr = *argv;
+			if (sscanf(saddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+				   sbuf, sbuf + 1, sbuf + 2,
+				   sbuf + 3, sbuf + 4, sbuf + 5) != 6) {
+				fprintf(stderr, "Invalid smac address %s\n",
+					saddr);
+				return -1;
+			}
+			p.flags |= SKBMOD_F_SMAC;
+			fprintf(stderr, "src MAC address <%s>\n", saddr);
+			ok += 1;
+		} else if (matches(*argv, "help") == 0) {
+			skbmod_usage();
+		} else {
+			break;
+		}
+
+		argc--;
+		argv++;
+	}
+
+	if (argc) {
+		if (matches(*argv, "reclassify") == 0) {
+			p.action = TC_ACT_RECLASSIFY;
+			argc--;
+			argv++;
+		} else if (matches(*argv, "pipe") == 0) {
+			p.action = TC_ACT_PIPE;
+			argc--;
+			argv++;
+		} else if (matches(*argv, "drop") == 0 ||
+			   matches(*argv, "shot") == 0) {
+			p.action = TC_ACT_SHOT;
+			argc--;
+			argv++;
+		} else if (matches(*argv, "continue") == 0) {
+			p.action = TC_ACT_UNSPEC;
+			argc--;
+			argv++;
+		} else if (matches(*argv, "pass") == 0 ||
+			   matches(*argv, "ok") == 0) {
+			p.action = TC_ACT_OK;
+			argc--;
+			argv++;
+		}
+	}
+
+	if (argc) {
+		if (matches(*argv, "index") == 0) {
+			NEXT_ARG();
+			if (get_u32(&p.index, *argv, 0)) {
+				fprintf(stderr, "skbmod: Illegal \"index\"\n");
+				return -1;
+			}
+			ok++;
+			argc--;
+			argv++;
+		}
+	}
+
+	if (!ok) {
+		fprintf(stderr, "skbmod requires at least one option\n");
+		skbmod_usage();
+	}
+
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	addattr_l(n, MAX_MSG, TCA_SKBMOD_PARMS, &p, sizeof(p));
+
+	if (daddr)
+		addattr_l(n, MAX_MSG, TCA_SKBMOD_DMAC, dbuf, ETH_ALEN);
+	if (skbmod_etype)
+		addattr16(n, MAX_MSG, TCA_SKBMOD_ETYPE, skbmod_etype);
+	if (saddr)
+		addattr_l(n, MAX_MSG, TCA_SKBMOD_SMAC, sbuf, ETH_ALEN);
+
+	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+
+	*argc_p = argc;
+	*argv_p = argv;
+	return 0;
+}
+
+static int print_skbmod(struct action_util *au, FILE *f, struct rtattr *arg)
+{
+	struct tc_skbmod *p = NULL;
+	struct rtattr *tb[TCA_SKBMOD_MAX + 1];
+	__u16 skbmod_etype = 0;
+	int has_optional = 0;
+	SPRINT_BUF(b1);
+	SPRINT_BUF(b2);
+
+	if (arg == NULL)
+		return -1;
+
+	parse_rtattr_nested(tb, TCA_SKBMOD_MAX, arg);
+
+	if (tb[TCA_SKBMOD_PARMS] == NULL) {
+		fprintf(f, "[NULL skbmod parameters]");
+		return -1;
+	}
+
+	p = RTA_DATA(tb[TCA_SKBMOD_PARMS]);
+
+	fprintf(f, "skbmod action %s ", action_n2a(p->action));
+
+	if (tb[TCA_SKBMOD_ETYPE]) {
+		skbmod_etype = rta_getattr_u16(tb[TCA_SKBMOD_ETYPE]);
+		has_optional = 1;
+		fprintf(f, "set etype 0x%X ", skbmod_etype);
+	}
+
+	if (has_optional)
+		fprintf(f, "\n\t ");
+
+	if (tb[TCA_SKBMOD_DMAC]) {
+		has_optional = 1;
+		fprintf(f, "set dmac %s ",
+			ll_addr_n2a(RTA_DATA(tb[TCA_SKBMOD_DMAC]),
+				    RTA_PAYLOAD(tb[TCA_SKBMOD_DMAC]), 0, b1,
+				    sizeof(b1)));
+
+	}
+
+	if (tb[TCA_SKBMOD_SMAC]) {
+		has_optional = 1;
+		fprintf(f, "set smac %s ",
+			ll_addr_n2a(RTA_DATA(tb[TCA_SKBMOD_SMAC]),
+				    RTA_PAYLOAD(tb[TCA_SKBMOD_SMAC]), 0, b2,
+				    sizeof(b2)));
+	}
+
+	if (p->flags & SKBMOD_F_SWAPMAC)
+		fprintf(f, "swap mac ");
+
+	fprintf(f, "\n\t index %d ref %d bind %d", p->index, p->refcnt,
+		p->bindcnt);
+	if (show_stats) {
+		if (tb[TCA_SKBMOD_TM]) {
+			struct tcf_t *tm = RTA_DATA(tb[TCA_SKBMOD_TM]);
+
+			print_tm(f, tm);
+		}
+	}
+
+	fprintf(f, "\n");
+
+	return 0;
+}
+
+struct action_util skbmod_action_util = {
+	.id = "skbmod",
+	.parse_aopt = parse_skbmod,
+	.print_aopt = print_skbmod,
+};
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Saeed Mahameed @ 2016-10-11 11:57 UTC (permalink / raw)
  To: David Laight
  Cc: Tom Herbert, davem@davemloft.net, netdev@vger.kernel.org,
	saeedm@mellanox.com, kernel-team@fb.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB01E62DC@AcuExch.aculab.com>

On Tue, Oct 11, 2016 at 7:50 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Tom Herbert
>> Sent: 11 October 2016 05:22
> ...
>> Fix is to create MLX5_SET64_VCHK that takes an additional argument
>> that is a constant. There are two callers of MLX5_SET64 that are
>> trying to get a variable offset, change those to call MLX5_SET64_VCHK
>> passing pas[0] as the argument to use in the offset check.
>
> I think I'd separate the array index instead.
> Something like:
>
> #define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
>         BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
>         __MLX5_SET64(typ, p, fld[ndx], v); \
> } while (0)
>
>         David

Yes, I think this looks more natural, but instead MLX5_SET64_INDEXED,
I prefer to have 2 macros
MLX5_SET64(typ, p, fld, v) and MLX5_ARRAY_SET64(typ, p, fld, idx, v).

Tom, do you want me to fix it ?

Thanks,
Saeed.

^ permalink raw reply

* [PATCH v2 iproute2 2/9] ife: print prio, mark and hash as unsigned
From: Jamal Hadi Salim @ 2016-10-11 11:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, mrv, cdillaba, lucasb, Jamal Hadi Salim
In-Reply-To: <1476183648-25850-1-git-send-email-jhs@emojatatu.com>

From: Roman Mashak <mrv@mojatatu.com>

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 tc/m_ife.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tc/m_ife.c b/tc/m_ife.c
index a5a7516..588bad7 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -252,7 +252,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 			len = RTA_PAYLOAD(metalist[IFE_META_SKBMARK]);
 			if (len) {
 				mmark = rta_getattr_u32(metalist[IFE_META_SKBMARK]);
-				fprintf(f, "use mark %d ", mmark);
+				fprintf(f, "use mark %u ", mmark);
 			} else
 				fprintf(f, "allow mark ");
 		}
@@ -261,7 +261,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 			len = RTA_PAYLOAD(metalist[IFE_META_HASHID]);
 			if (len) {
 				mhash = rta_getattr_u32(metalist[IFE_META_HASHID]);
-				fprintf(f, "use hash %d ", mhash);
+				fprintf(f, "use hash %u ", mhash);
 			} else
 				fprintf(f, "allow hash ");
 		}
@@ -270,7 +270,7 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg)
 			len = RTA_PAYLOAD(metalist[IFE_META_PRIO]);
 			if (len) {
 				mprio = rta_getattr_u32(metalist[IFE_META_PRIO]);
-				fprintf(f, "use prio %d ", mprio);
+				fprintf(f, "use prio %u ", mprio);
 			} else
 				fprintf(f, "allow prio ");
 		}
-- 
1.9.1

^ permalink raw reply related

* [PATCH] drivers/ptp: Fix kernel memory disclosure
From: Vlad Tsyrklevich @ 2016-10-11 13:02 UTC (permalink / raw)
  To: netdev; +Cc: richardcochran, Vlad Tsyrklevich

The reserved field precise_offset->rsv is not cleared before being
copied to user space, leaking kernel stack memory. Clear the struct
before it's copied.

Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>

---
 drivers/ptp/ptp_chardev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index d637c93..58a97d4 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -193,6 +193,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
 		if (err)
 			break;
 
+		memset(&precise_offset, 0, sizeof(precise_offset));
 		ts = ktime_to_timespec64(xtstamp.device);
 		precise_offset.device.sec = ts.tv_sec;
 		precise_offset.device.nsec = ts.tv_nsec;
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH net-next v10 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.
From: Andrew Lunn @ 2016-10-11 13:09 UTC (permalink / raw)
  To: Allan W. Nielsen; +Cc: netdev, f.fainelli, raju.lakkaraju
In-Reply-To: <1476108825-14613-2-git-send-email-allan.nielsen@microsemi.com>

On Mon, Oct 10, 2016 at 04:13:45PM +0200, Allan W. Nielsen wrote:
> Edge-Rate cleanup include the following:
> - Updated device tree bindings documentation for edge-rate
> - The edge-rate is now specified as a "slowdown", meaning that it is now
>   being specified as positive values instead of negative (both
>   documentation and implementation wise).
> - Only explicitly documented values for "vsc8531,vddmac" and
>   "vsc8531,edge-slowdown" are accepted by the device driver.
> - Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
> - Read/validate devicetree settings in probe instead of init
> 
> Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
> Signed-off-by: Raju Lakkaraju <raju.lakkaraju@microsemi.com>

Hi Raju, Allan

Thanks for keeping at this.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: slab corruption with current -git
From: Michal Kubecek @ 2016-10-11 13:17 UTC (permalink / raw)
  To: David Miller
  Cc: torvalds, aconole, fw, viro, akpm, axboe, tytso, cl, pablo,
	linux-kernel, linux-fsdevel, netdev, netfilter-devel
In-Reply-To: <20161010.042401.637964142015887598.davem@davemloft.net>

On Mon, Oct 10, 2016 at 04:24:01AM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sun, 09 Oct 2016 23:57:45 -0400 (EDT)
> 
> This means that the netns is possibly getting freed up before we
> unregister the netfilter hooks.

Sounds a bit like the issue discussed here:

  https://marc.info/?l=netfilter-devel&m=146980917627262&w=2

Could it be (partly) the same race condition?

Michal Kubecek

^ permalink raw reply

* [PATCH 1/1] vxlan: insert ipv6 macro
From: zyjzyj2000 @ 2016-10-11  8:23 UTC (permalink / raw)
  To: netdev, pabeni, daniel, pshelar, aduyck, hannes, jbenc, davem,
	zyjzyj2000

From: Zhu Yanjun <zyjzyj2000@gmail.com>

The source code is related with ipv6. As such, it is better to insert
ipv6 macro.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/vxlan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index e7d1668..9af6600 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2647,15 +2647,15 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
 	int err;
 
 	memset(&udp_conf, 0, sizeof(udp_conf));
-
+#if IS_ENABLED(CONFIG_IPV6)
 	if (ipv6) {
 		udp_conf.family = AF_INET6;
 		udp_conf.use_udp6_rx_checksums =
 		    !(flags & VXLAN_F_UDP_ZERO_CSUM6_RX);
 		udp_conf.ipv6_v6only = 1;
-	} else {
+	} else 
+#endif
 		udp_conf.family = AF_INET;
-	}
 
 	udp_conf.local_udp_port = port;
 
-- 
2.7.4

^ permalink raw reply related

* Re: slab corruption with current -git
From: Aaron Conole @ 2016-10-11 13:55 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: David Miller, torvalds, fw, viro, akpm, axboe, tytso, cl, pablo,
	linux-kernel, linux-fsdevel, netdev, netfilter-devel
In-Reply-To: <20161011131709.GB24682@unicorn.suse.cz>

Michal Kubecek <mkubecek@suse.cz> writes:

> On Mon, Oct 10, 2016 at 04:24:01AM -0400, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Sun, 09 Oct 2016 23:57:45 -0400 (EDT)
>> 
>> This means that the netns is possibly getting freed up before we
>> unregister the netfilter hooks.
>
> Sounds a bit like the issue discussed here:
>
>   https://marc.info/?l=netfilter-devel&m=146980917627262&w=2
>
> Could it be (partly) the same race condition?

It looks like it's possible.  It appears that there could be a
long-standing race between these.  I'll look into it more carefully, and
discuss with Pablo and Florian when they're situated from netdev
conference.

-Aaron

^ permalink raw reply

* Hello !
From: Benoît QUEMIN @ 2016-10-11 14:01 UTC (permalink / raw)
  To: netdev; +Cc: Ghada AOUICHAOUI
In-Reply-To: <20161011134029.5369933.40547.6729@telecom-bretagne.eu>

‎
We have seen your message and are glad to know you :)
We love your work and hope to hear from you soon <3
Xx

^ permalink raw reply

* Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Chris Rorvick @ 2016-10-11 14:09 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Luca Coelho, Intel Linux Wireless, Emmanuel Grumbach,
	Johannes Berg, Kalle Valo, Oren Givon, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <1476180668.17022.21.camel@tiscali.nl>

Hi Luca,

I didn't receive your email so I'll try to respond via Paul's.

On Tue, Oct 11, 2016 at 5:11 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
>> This is not coming from the NIC itself, but from the platform's ACPI
>> tables.  Can you tell us which platform you are using?

Interesting.  I'm running a Dell XPS 13 9350.  I replaced the
factory-provided Broadcom card with an AC 8260.  I can update the
commit log to reflect this.

>> There are other things that look a bit inconsistent in this code...
>> I'll try to find the official ACPI table definitions for this entries
>> to make sure it's correct.
>
> When I looked into this error, some time ago, I searched around a bit
> for documentation on this splx stuff. Sadly, commit bcb079a14d75
> ("iwlwifi: pcie: retrieve and parse ACPI power limitations") provides
> very few clues and my searches turned up nothing useful. So a pointer
> or two would be really appreciated.

Ditto.

>> If this is really bothering you, I guess I could apply this patch for
>> now.  But as I said, this is not solving the actual problem.
>
> Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't
> imply one needs to act on this message, while warn does imply that
> action is needed.

Agreed.  I still think making this a warning is appropriate, but it
seems pretty clear this is not an error.  This has nothing to do with
how much it bothers me.  An error tells the user something needs to be
fixed, but in this case the interface is working fine.  Making it a
warning with an improved message will result in fewer people wasting
their time.

Thanks!

Chris

^ permalink raw reply

* Re: [PATCH 1/1] vxlan: insert ipv6 macro
From: Jiri Benc @ 2016-10-11 14:06 UTC (permalink / raw)
  To: zyjzyj2000; +Cc: netdev, pabeni, daniel, pshelar, aduyck, hannes, davem
In-Reply-To: <1476174211-9358-1-git-send-email-zyjzyj2000@gmail.com>

On Tue, 11 Oct 2016 16:23:31 +0800, zyjzyj2000@gmail.com wrote:
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2647,15 +2647,15 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
>  	int err;
>  
>  	memset(&udp_conf, 0, sizeof(udp_conf));
> -
> +#if IS_ENABLED(CONFIG_IPV6)
>  	if (ipv6) {
>  		udp_conf.family = AF_INET6;
>  		udp_conf.use_udp6_rx_checksums =
>  		    !(flags & VXLAN_F_UDP_ZERO_CSUM6_RX);
>  		udp_conf.ipv6_v6only = 1;
> -	} else {
> +	} else 
> +#endif
>  		udp_conf.family = AF_INET;
> -	}

Zhu Yanjun, before posting patches such as the previous ones or
this one, please test whether they make any difference. In this case,
try to compile the code with IPv6 disabled before and after this patch,
disassemble and compare the results. You'll see that this patch is
pointless.

It's pretty obvious from the code but to be really sure, I've just
quickly built the vxlan module with IPv6 disabled. And indeed, as
expected, the compiler just inlined everything into vxlan_open. The
whole chain vxlan_open -> vxlan_sock_add -> __vxlan_sock_add (note that
there's only a single caller of __vxlan_sock_add with IPv6 disabled) ->
vxlan_socket_create -> vxlan_create_sock is inlined.

It also means the code in the "if (ipv6)" branch is completely
eliminated by the compiler even without ugly #ifdefs.

 Jiri

^ permalink raw reply

* Re: [PATCH] iwlwifi: pcie: reduce "unsupported splx" to a warning
From: Chris Rorvick @ 2016-10-11 14:27 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Luca Coelho, Intel Linux Wireless, Emmanuel Grumbach,
	Johannes Berg, Kalle Valo, Oren Givon, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <CAEUsAPYDPCP0NBaTmy4xqFJhAS4xFQYinoNTknxFT23ef0dk4w@mail.gmail.com>

On Tue, Oct 11, 2016 at 9:09 AM, Chris Rorvick <chris@rorvick.com> wrote:
> I didn't receive your email so I'll try to respond via Paul's.

>>> If this is really bothering you, I guess I could apply this patch for
>>> now.  But as I said, this is not solving the actual problem.
>>
>> Bikeshedding: I think IWL_INFO() is more appropriate, as info doesn't
>> imply one needs to act on this message, while warn does imply that
>> action is needed.
>
> Agreed.  I still think making this a warning is appropriate, but it
> seems pretty clear this is not an error.  This has nothing to do with
> how much it bothers me.  An error tells the user something needs to be
> fixed, but in this case the interface is working fine.  Making it a
> warning with an improved message will result in fewer people wasting
> their time.

I found your original email on lkml.org... should have looked there in
the first place!  Yes, if there is a fix for the underlying issue then
that is obviously preferred.  When I investigated this I saw several
reports spanning at least a few distros and kernel versions with at
least some concluding "this is normal".

Again, thanks!

Chris

^ permalink raw reply

* Re: [RFC] net: phy: smsc: Disable auto-negotiation on startup
From: Jeremy Linton @ 2016-10-11 14:32 UTC (permalink / raw)
  To: Kyle Roeschley, f.fainelli; +Cc: netdev, linux-kernel
In-Reply-To: <20161010174123.21436-1-kyle.roeschley@ni.com>

On 10/10/2016 12:41 PM, Kyle Roeschley wrote:
> Because the SMSC PHY completes auto-negotiation before the driver is
> ready to handle interrupts, the PHY state machine never realizes that we
> have a link. Clear the ANENABLE bit on initialization, which lets
> genphy_config_aneg do its thing when that code is hit later.
>
> While this patch does fix the problem we see (no link on boot without
> re-plugging the cable), it seems like the generic PHY code should be
> able to handle auto-negotiation completing before interrupts are
> enabled. Submitted as an RFC in the hopes that someone has an idea as to
> how that could be done.

Hi,

	Which smsc chip/driver? Maybe assuring the device interrupts are 
enabled before the phy is started is a solution?

	The whole problem sounds similar to what was recently happening in the 
smsc911x driver, but AFAIK that driver is basically only polling at this 
point so connecting the phy before the interrupts are enabled shouldn't 
be a problem.




>
> This fix is copied from commit 99f81afc139c ("phy: micrel: Disable auto
> negotiation on startup").
>
> Signed-off-by: Kyle Roeschley <kyle.roeschley@ni.com>
> ---
>  drivers/net/phy/smsc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index b62c4aa..8de8011 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -62,6 +62,16 @@ static int smsc_phy_config_init(struct phy_device *phydev)
>  			return rc;
>  	}
>
> +	if (phy_interrupt_is_valid(phydev)) {
> +		rc = phy_read(phydev, MII_BMCR);
> +		if (rc < 0)
> +			return rc;
> +
> +		rc = phy_write(phydev, MII_BMCR, rc & ~BMCR_ANENABLE);
> +		if (rc < 0)
> +			return rc;
> +	}
> +
>  	return smsc_phy_ack_interrupt(phydev);
>  }
>
>

^ permalink raw reply

* Re: [PATCH] doc: fix wrongly referencing dev->skb_mark
From: Ryota Ozaki @ 2016-10-11 15:47 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev
In-Reply-To: <20161010134943.GA8090@splinter>

On Mon, Oct 10, 2016 at 10:49 PM, Ido Schimmel <idosch@idosch.org> wrote:
> Hi,
>
> On Mon, Oct 10, 2016 at 08:15:39PM +0900, Ryota Ozaki wrote:
>> Section "Flooding L2 domain" says, to avoid duplicated flooding, if
>> skb->offload_fwd_mark is matched with dev->skb_mark, the kernel will
>> drop the packet. However, the relevant code in __dev_queue_xmit
>> compares skb->offload_fwd_mark with dev->offload_fwd_mark, not
>> dev->skb_mark. I guess the text is wrong.
>>
>> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
>> ---
>>  Documentation/networking/switchdev.txt | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
>> index 31c3911..d4124a0 100644
>> --- a/Documentation/networking/switchdev.txt
>> +++ b/Documentation/networking/switchdev.txt
>> @@ -286,8 +286,8 @@ otherwise there will be duplicate packets on the wire.
>>  To avoid duplicate packets, the device/driver should mark a packet as already
>>  forwarded using skb->offload_fwd_mark.  The same mark is set on the device
>>  ports in the domain using dev->offload_fwd_mark.  If the skb->offload_fwd_mark
>> -is non-zero and matches the forwarding egress port's dev->skb_mark, the kernel
>> -will drop the skb right before transmit on the egress port, with the
>> +is non-zero and matches the forwarding egress port's dev->offload_fwd_mark,
>> +the kernel will drop the skb right before transmit on the egress port, with the
>
> I think your tree isn't up to date. The flooding mechanism (and this
> document) were modified in commit 6bc506b4fb06 ("bridge: switchdev: Add
> forward mark support for stacked devices")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6bc506b4fb065eac3d89ca1ce37082e174493d9e

Oh, right. I forgot to rebase my branch after the 4.9 merge period
and saw the file as of 4.8.

>
> Also, in the future, please specify to which tree (net, net-next) your
> patch should go. See:
> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
> (under "How do I indicate which tree (net vs. net-next) my patch
> should be in?").

Thanks for the pointer. I'll follow the rule next time.

Thanks,
  ozaki-r

>
> Thanks!
>
>>  understanding that the device already forwarded the packet on same egress port.
>>  The driver can use switchdev_port_fwd_mark_set() to set a globally unique mark
>>  for port's dev->offload_fwd_mark, based on the port's parent ID (switch ID) and
>> --
>> 2.7.4
>>

^ permalink raw reply

* [PATCHv1 net] xen-netback: fix guest Rx stall detection (after guest Rx refactor)
From: David Vrabel @ 2016-10-11 15:48 UTC (permalink / raw)
  To: netdev; +Cc: David Vrabel, xen-devel, Paul Durrant, Wei Liu

If a VIF has been ready for rx_stall_timeout (60s by default) and an
Rx ring is drained of all requests an Rx stall will be incorrectly
detected.  When this occurs and the guest Rx queue is empty, the Rx
ring's event index will not be set and the frontend will not raise an
event when new requests are placed on the ring, permanently stalling
the VIF.

This is a regression introduced by eb1723a29b9a7 (xen-netback:
refactor guest rx).

Fix this by reinstating the setting of queue->last_rx_time when
placing a packet onto the guest Rx ring.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
index 8e9ade6..d69f2a9 100644
--- a/drivers/net/xen-netback/rx.c
+++ b/drivers/net/xen-netback/rx.c
@@ -425,6 +425,8 @@ void xenvif_rx_skb(struct xenvif_queue *queue)
 
 	xenvif_rx_next_skb(queue, &pkt);
 
+	queue->last_rx_time = jiffies;
+
 	do {
 		struct xen_netif_rx_request *req;
 		struct xen_netif_rx_response *rsp;
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH kernel v2] PCI: Enable access to custom VPD for Chelsio devices (cxgb3)
From: Alexander Duyck @ 2016-10-11 15:49 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Bjorn Helgaas, Netdev, Santosh Raspatur,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <6c5b40df-700f-bc6f-998c-9ac0106f3165@ozlabs.ru>

On Mon, Oct 10, 2016 at 9:08 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 11/10/16 02:23, Alexander Duyck wrote:
>> On Wed, Sep 28, 2016 at 10:21 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>> There is at least one Chelsio 10Gb card which uses VPD area to store
>>> some custom blocks (example below). However pci_vpd_size() returns
>>> the length of the first block only assuming that there can be only
>>> one VPD "End Tag" and VFIO blocks access beyond that offset
>>> (since 4e1a63555) which leads to the situation when the guest "cxgb3"
>>> driver fails to probe the device. The host system does not have this
>>> problem as the drives accesses the config space directly without
>>> pci_read_vpd()/...
>>>
>>> This adds a quirk to override the VPD size to a bigger value.
>>> The maximum size is taken from EEPROMSIZE in
>>> drivers/net/ethernet/chelsio/cxgb3/common.h. We do not read the tag
>>> as the cxgb3 driver does as the driver supports writing to EEPROM/VPD
>>> and when it writes, it only checks for 8192 bytes boundary. The quirk
>>> is registerted for all devices supported by the cxgb3 driver.
>>>
>>> This adds a quirk to the PCI layer (not to the cxgb3 driver) as
>>> the cxgb3 driver itself accesses VPD directly and the problem only exists
>>> with the vfio-pci driver (when cxgb3 is not running on the host and
>>> may not be even loaded) which blocks accesses beyond the first block
>>> of VPD data. However vfio-pci itself does not have quirks mechanism so
>>> we add it to PCI.
>>>
>>> Tested on:
>>> Ethernet controller [0200]: Chelsio Communications Inc T310 10GbE Single Port Adapter [1425:0030]
>>>
>>> This is its VPD:
>>> 0000 Large item 42 bytes; name 0x2 Identifier String
>>>         b'10 Gigabit Ethernet-SR PCI Express Adapter'
>>>         #00 [EC] len=7: b'D76809 '
>>>         #0a [FN] len=7: b'46K7897'
>>>         #14 [PN] len=7: b'46K7897'
>>>         #1e [MN] len=4: b'1037'
>>>         #25 [FC] len=4: b'5769'
>>>         #2c [SN] len=12: b'YL102035603V'
>>>         #3b [NA] len=12: b'00145E992ED1'
>>>
>>> 0c00 Large item 16 bytes; name 0x2 Identifier String
>>>         b'S310E-SR-X      '
>>> 0c13 Large item 234 bytes; name 0x10
>>>         #00 [PN] len=16: b'TBD             '
>>>         #13 [EC] len=16: b'110107730D2     '
>>>         #26 [SN] len=16: b'97YL102035603V  '
>>>         #39 [NA] len=12: b'00145E992ED1'
>>>         #48 [V0] len=6: b'175000'
>>>         #51 [V1] len=6: b'266666'
>>>         #5a [V2] len=6: b'266666'
>>>         #63 [V3] len=6: b'2000  '
>>>         #6c [V4] len=2: b'1 '
>>>         #71 [V5] len=6: b'c2    '
>>>         #7a [V6] len=6: b'0     '
>>>         #83 [V7] len=2: b'1 '
>>>         #88 [V8] len=2: b'0 '
>>>         #8d [V9] len=2: b'0 '
>>>         #92 [VA] len=2: b'0 '
>>>         #97 [RV] len=80: b's\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
>>> 0d00 Large item 252 bytes; name 0x11
>>>         #00 [VC] len=16: b'122310_1222 dp  '
>>>         #13 [VD] len=16: b'610-0001-00 H1\x00\x00'
>>>         #26 [VE] len=16: b'122310_1353 fp  '
>>>         #39 [VF] len=16: b'610-0001-00 H1\x00\x00'
>>>         #4c [RW] len=173: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
>>> 0dff Small item 0 bytes; name 0xf End Tag
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v2:
>>> * used pci_set_vpd_size() helper
>>> * added explicit list of IDs from cxgb3 driver
>>> * added a note in the commit log why the quirk is not in cxgb3
>>> ---
>>>  drivers/pci/quirks.c | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 44e0ff3..b22fce5 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -3243,6 +3243,28 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
>>>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
>>>                         quirk_thunderbolt_hotplug_msi);
>>>
>>> +static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
>>> +{
>>> +       if (!dev->vpd)
>>> +               return;
>>> +
>>> +       pci_set_vpd_size(dev, max_t(unsigned int, dev->vpd->len, 8192));
>>
>> What is the point of the max_t?  From what I can tell you aren't
>> writing 8192, you will always end up writing 32K since that is the
>> starting value for dev->vpd->len assuming there have yet to be any
>> reads.
>
> At this stage dev->vpd->len is always 32k? I thought here VPD was scanned
> already, I'll double check.

It only gets updated when an actual read is performed.  You cannot
rely on that occurring before your workaround.

>
>>
>> What you may want to do instead is just modify the pci_vpd_size
>> function you can use that in your quirk, and modify it so that you can
>> pass an offset  Then you could just start it with an offset of 0x0c00
>> and have it read to get the exact size of the region covered in this
>> second block of the VPD.
>
> Sorry, I am totally missing the point. The device allows writing to it, the
> driver claims it is 8192, we can be pretty sure that accessing anything
> between 0 and 8191 won't break the device (which was the initial point of
> limiting VPD access), why do this scan? The format can be actually not
> exactly as PCI VPD and probably there is some extension which I failed to
> parse without knowing it (there are non zero bytes after the end of the
> second block).

If you want to use a fixed value of 8192 you could go with that.
Otherwise if you are wanting a bit more dynamic setup and you happen
to know that the block you need to access starts at 0x0c00 you could
just use the pci_vpd_size function with a modification to support
offset to get the length value.

- Alex

^ permalink raw reply

* RE: [PATCHv1 net] xen-netback: fix guest Rx stall detection (after guest Rx refactor)
From: Paul Durrant @ 2016-10-11 15:54 UTC (permalink / raw)
  To: David Vrabel, netdev@vger.kernel.org
  Cc: David Vrabel, xen-devel@lists.xenproject.org, Wei Liu
In-Reply-To: <1476200907-16085-1-git-send-email-david.vrabel@citrix.com>

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@citrix.com]
> Sent: 11 October 2016 16:48
> To: netdev@vger.kernel.org
> Cc: David Vrabel <david.vrabel@citrix.com>; xen-devel@lists.xenproject.org;
> Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Subject: [PATCHv1 net] xen-netback: fix guest Rx stall detection (after guest
> Rx refactor)
> 
> If a VIF has been ready for rx_stall_timeout (60s by default) and an
> Rx ring is drained of all requests an Rx stall will be incorrectly
> detected.  When this occurs and the guest Rx queue is empty, the Rx
> ring's event index will not be set and the frontend will not raise an
> event when new requests are placed on the ring, permanently stalling
> the VIF.
> 
> This is a regression introduced by eb1723a29b9a7 (xen-netback:
> refactor guest rx).
> 
> Fix this by reinstating the setting of queue->last_rx_time when
> placing a packet onto the guest Rx ring.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
>  drivers/net/xen-netback/rx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
> index 8e9ade6..d69f2a9 100644
> --- a/drivers/net/xen-netback/rx.c
> +++ b/drivers/net/xen-netback/rx.c
> @@ -425,6 +425,8 @@ void xenvif_rx_skb(struct xenvif_queue *queue)
> 
>  	xenvif_rx_next_skb(queue, &pkt);
> 
> +	queue->last_rx_time = jiffies;
> +
>  	do {
>  		struct xen_netif_rx_request *req;
>  		struct xen_netif_rx_response *rsp;
> --
> 2.1.4

^ permalink raw reply

* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Tom Herbert @ 2016-10-11 15:46 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: David Laight, davem@davemloft.net, netdev@vger.kernel.org,
	saeedm@mellanox.com, kernel-team@fb.com
In-Reply-To: <CALzJLG_9Yr=jCPix2Oisxf22CLHxtYYyXejYxkBi-A85kZ_Shw@mail.gmail.com>

On Tue, Oct 11, 2016 at 4:57 AM, Saeed Mahameed
<saeedm@dev.mellanox.co.il> wrote:
> On Tue, Oct 11, 2016 at 7:50 PM, David Laight <David.Laight@aculab.com> wrote:
>> From: Tom Herbert
>>> Sent: 11 October 2016 05:22
>> ...
>>> Fix is to create MLX5_SET64_VCHK that takes an additional argument
>>> that is a constant. There are two callers of MLX5_SET64 that are
>>> trying to get a variable offset, change those to call MLX5_SET64_VCHK
>>> passing pas[0] as the argument to use in the offset check.
>>
>> I think I'd separate the array index instead.
>> Something like:
>>
>> #define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
>>         BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
>>         __MLX5_SET64(typ, p, fld[ndx], v); \
>> } while (0)
>>
>>         David
>
> Yes, I think this looks more natural, but instead MLX5_SET64_INDEXED,
> I prefer to have 2 macros
> MLX5_SET64(typ, p, fld, v) and MLX5_ARRAY_SET64(typ, p, fld, idx, v).
>
> Tom, do you want me to fix it ?
>
Please do.

> Thanks,
> Saeed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox