Netdev List
 help / color / mirror / Atom feed
* [PATCH 27/28] thunderbolt: Make rest of the logging to happen at debug level
From: Mika Westerberg @ 2019-01-29 15:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner,
	David S . Miller, Mika Westerberg, Andy Shevchenko, netdev
In-Reply-To: <20190129150143.12681-1-mika.westerberg@linux.intel.com>

Now that the driver can handle every possible tunnel types there is no
point to log everything as info level so turn these to happen at debug
level instead.

While at it remove duplicated tunnel activation log message
(tb_tunnel_activate() calls tb_tunnel_restart() which print the same
message).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/ctl.c    |  2 +-
 drivers/thunderbolt/icm.c    |  2 +-
 drivers/thunderbolt/path.c   | 30 +++++++++++++++---------------
 drivers/thunderbolt/switch.c | 19 +++++++++----------
 drivers/thunderbolt/tb.c     | 11 +++++------
 drivers/thunderbolt/tunnel.c | 10 ++++------
 6 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 73b386de4d15..2427d73be731 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -720,7 +720,7 @@ int tb_cfg_error(struct tb_ctl *ctl, u64 route, u32 port,
 		.port = port,
 		.error = error,
 	};
-	tb_ctl_info(ctl, "resetting error on %llx:%x.\n", route, port);
+	tb_ctl_dbg(ctl, "resetting error on %llx:%x.\n", route, port);
 	return tb_ctl_tx(ctl, &pkg, sizeof(pkg), TB_CFG_PKG_ERROR);
 }
 
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index e28a4255d56a..c44906fac2a4 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -1559,7 +1559,7 @@ static int icm_firmware_start(struct tb *tb, struct tb_nhi *nhi)
 	if (val & REG_FW_STS_ICM_EN)
 		return 0;
 
-	dev_info(&nhi->pdev->dev, "starting ICM firmware\n");
+	dev_dbg(&nhi->pdev->dev, "starting ICM firmware\n");
 
 	ret = icm_firmware_reset(tb, nhi);
 	if (ret)
diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index afdb667fcc0d..1aefdef403ef 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -354,12 +354,12 @@ void tb_path_deactivate(struct tb_path *path)
 		tb_WARN(path->tb, "trying to deactivate an inactive path\n");
 		return;
 	}
-	tb_info(path->tb,
-		"deactivating path from %llx:%x to %llx:%x\n",
-		tb_route(path->hops[0].in_port->sw),
-		path->hops[0].in_port->port,
-		tb_route(path->hops[path->path_length - 1].out_port->sw),
-		path->hops[path->path_length - 1].out_port->port);
+	tb_dbg(path->tb,
+	       "deactivating path from %llx:%x to %llx:%x\n",
+	       tb_route(path->hops[0].in_port->sw),
+	       path->hops[0].in_port->port,
+	       tb_route(path->hops[path->path_length - 1].out_port->sw),
+	       path->hops[path->path_length - 1].out_port->port);
 	__tb_path_deactivate_hops(path, 0);
 	__tb_path_deallocate_nfc(path, 0);
 	path->activated = false;
@@ -382,12 +382,12 @@ int tb_path_activate(struct tb_path *path)
 		return -EINVAL;
 	}
 
-	tb_info(path->tb,
-		"activating path from %llx:%x to %llx:%x\n",
-		tb_route(path->hops[0].in_port->sw),
-		path->hops[0].in_port->port,
-		tb_route(path->hops[path->path_length - 1].out_port->sw),
-		path->hops[path->path_length - 1].out_port->port);
+	tb_dbg(path->tb,
+	       "activating path from %llx:%x to %llx:%x\n",
+	       tb_route(path->hops[0].in_port->sw),
+	       path->hops[0].in_port->port,
+	       tb_route(path->hops[path->path_length - 1].out_port->sw),
+	       path->hops[path->path_length - 1].out_port->port);
 
 	/* Clear counters. */
 	for (i = path->path_length - 1; i >= 0; i--) {
@@ -438,8 +438,8 @@ int tb_path_activate(struct tb_path *path)
 					    & out_mask;
 		hop.unknown3 = 0;
 
-		tb_port_info(path->hops[i].in_port, "Writing hop %d, index %d",
-			     i, path->hops[i].in_hop_index);
+		tb_port_dbg(path->hops[i].in_port, "Writing hop %d, index %d",
+			    i, path->hops[i].in_hop_index);
 		tb_dump_hop(path->hops[i].in_port, &hop);
 		res = tb_port_write(path->hops[i].in_port, &hop, TB_CFG_HOPS,
 				    2 * path->hops[i].in_hop_index, 2);
@@ -450,7 +450,7 @@ int tb_path_activate(struct tb_path *path)
 		}
 	}
 	path->activated = true;
-	tb_info(path->tb, "path activation complete\n");
+	tb_dbg(path->tb, "path activation complete\n");
 	return 0;
 err:
 	tb_WARN(path->tb, "path activation failed\n");
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index f5ebe8bcaeb0..af0ec45129c8 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -500,23 +500,22 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged)
 		if (state < 0)
 			return state;
 		if (state == TB_PORT_DISABLED) {
-			tb_port_info(port, "is disabled (state: 0)\n");
+			tb_port_dbg(port, "is disabled (state: 0)\n");
 			return 0;
 		}
 		if (state == TB_PORT_UNPLUGGED) {
 			if (wait_if_unplugged) {
 				/* used during resume */
-				tb_port_info(port,
-					     "is unplugged (state: 7), retrying...\n");
+				tb_port_dbg(port,
+					    "is unplugged (state: 7), retrying...\n");
 				msleep(100);
 				continue;
 			}
-			tb_port_info(port, "is unplugged (state: 7)\n");
+			tb_port_dbg(port, "is unplugged (state: 7)\n");
 			return 0;
 		}
 		if (state == TB_PORT_UP) {
-			tb_port_info(port,
-				     "is connected, link is up (state: 2)\n");
+			tb_port_dbg(port, "is connected, link is up (state: 2)\n");
 			return 1;
 		}
 
@@ -524,9 +523,9 @@ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged)
 		 * After plug-in the state is TB_PORT_CONNECTING. Give it some
 		 * time.
 		 */
-		tb_port_info(port,
-			     "is connected, link is not up (state: %d), retrying...\n",
-			     state);
+		tb_port_dbg(port,
+			    "is connected, link is not up (state: %d), retrying...\n",
+			    state);
 		msleep(100);
 	}
 	tb_port_warn(port,
@@ -592,7 +591,7 @@ int tb_port_set_initial_credits(struct tb_port *port, u32 credits)
 int tb_port_clear_counter(struct tb_port *port, int counter)
 {
 	u32 zero[3] = { 0, 0, 0 };
-	tb_port_info(port, "clearing counter %d\n", counter);
+	tb_port_dbg(port, "clearing counter %d\n", counter);
 	return tb_port_write(port, zero, TB_CFG_COUNTERS, 3 * counter, 3);
 }
 
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 645798eb0a77..4e0a5d7f4e64 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -514,18 +514,17 @@ static void tb_handle_hotplug(struct work_struct *work)
 		} else if (tb_port_is_dpout(port)) {
 			tb_teardown_dp(tb, port);
 		} else {
-			tb_port_info(port,
-				     "got unplug event for disconnected port, ignoring\n");
+			tb_port_dbg(port,
+				   "got unplug event for disconnected port, ignoring\n");
 		}
 	} else if (port->remote) {
-		tb_port_info(port,
-			     "got plug event for connected port, ignoring\n");
+		tb_port_dbg(port, "got plug event for connected port, ignoring\n");
 	} else {
 		if (tb_port_is_null(port)) {
-			tb_port_info(port, "hotplug: scanning\n");
+			tb_port_dbg(port, "hotplug: scanning\n");
 			tb_scan_port(port);
 			if (!port->remote)
-				tb_port_info(port, "hotplug: no switch found\n");
+				tb_port_dbg(port, "hotplug: no switch found\n");
 		} else if (tb_port_is_dpout(port)) {
 			tb_tunnel_dp(tb, port);
 		}
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index fad18e84d475..fae6defc21d1 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -47,8 +47,8 @@ static const char * const tb_tunnel_names[] = { "PCI", "DP", "DMA" };
 	__TB_TUNNEL_PRINT(tb_WARN, tunnel, fmt, ##arg)
 #define tb_tunnel_warn(tunnel, fmt, arg...) \
 	__TB_TUNNEL_PRINT(tb_warn, tunnel, fmt, ##arg)
-#define tb_tunnel_info(tunnel, fmt, arg...) \
-	__TB_TUNNEL_PRINT(tb_info, tunnel, fmt, ##arg)
+#define tb_tunnel_dbg(tunnel, fmt, arg...) \
+	__TB_TUNNEL_PRINT(tb_dbg, tunnel, fmt, ##arg)
 
 static struct tb_tunnel *tb_tunnel_alloc(struct tb *tb, size_t npaths,
 					 enum tb_tunnel_type type)
@@ -610,7 +610,7 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)
 {
 	int res, i;
 
-	tb_tunnel_info(tunnel, "activating\n");
+	tb_tunnel_dbg(tunnel, "activating\n");
 
 	/* Make sure all paths are properly disabled before enable them again */
 	for (i = 0; i < tunnel->npaths; i++) {
@@ -650,8 +650,6 @@ int tb_tunnel_activate(struct tb_tunnel *tunnel)
 {
 	int i;
 
-	tb_tunnel_info(tunnel, "activating\n");
-
 	for (i = 0; i < tunnel->npaths; i++) {
 		if (tunnel->paths[i]->activated) {
 			tb_tunnel_WARN(tunnel,
@@ -671,7 +669,7 @@ void tb_tunnel_deactivate(struct tb_tunnel *tunnel)
 {
 	int i;
 
-	tb_tunnel_info(tunnel, "deactivating\n");
+	tb_tunnel_dbg(tunnel, "deactivating\n");
 
 	if (tunnel->activate)
 		tunnel->activate(tunnel, false);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 03/28] thunderbolt: Enable TMU access when accessing port space on legacy devices
From: Mika Westerberg @ 2019-01-29 15:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner,
	David S . Miller, Mika Westerberg, Andy Shevchenko, netdev
In-Reply-To: <20190129150143.12681-1-mika.westerberg@linux.intel.com>

Light Ridge and Eagle Ridge both need to have TMU access enabled before
port space can be fully accessed so make sure it happens on those. This
allows us to get rid of the offset quirk in tb_port_find_cap().

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/cap.c | 73 ++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 17 deletions(-)

diff --git a/drivers/thunderbolt/cap.c b/drivers/thunderbolt/cap.c
index 9553305c63ea..94a507f12b71 100644
--- a/drivers/thunderbolt/cap.c
+++ b/drivers/thunderbolt/cap.c
@@ -22,28 +22,43 @@ struct tb_cap_any {
 	};
 } __packed;
 
-/**
- * tb_port_find_cap() - Find port capability
- * @port: Port to find the capability for
- * @cap: Capability to look
- *
- * Returns offset to start of capability or %-ENOENT if no such
- * capability was found. Negative errno is returned if there was an
- * error.
- */
-int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+static int tb_port_enable_tmu(struct tb_port *port, bool enable)
 {
-	u32 offset;
+	struct tb_switch *sw = port->sw;
+	u32 value, offset;
+	int ret;
 
 	/*
-	 * DP out adapters claim to implement TMU capability but in
-	 * reality they do not so we hard code the adapter specific
-	 * capability offset here.
+	 * Legacy devices need to have TMU access enabled before port
+	 * space can be fully accessed.
 	 */
-	if (port->config.type == TB_TYPE_DP_HDMI_OUT)
-		offset = 0x39;
+	switch (sw->config.device_id) {
+	case PCI_DEVICE_ID_INTEL_LIGHT_RIDGE:
+		offset = 0x26;
+		break;
+	case PCI_DEVICE_ID_INTEL_EAGLE_RIDGE:
+		offset = 0x2a;
+		break;
+
+	default:
+		return 0;
+	}
+
+	ret = tb_sw_read(sw, &value, TB_CFG_SWITCH, offset, 1);
+	if (ret)
+		return ret;
+
+	if (enable)
+		value |= BIT(20);
 	else
-		offset = 0x1;
+		value &= ~BIT(20);
+
+	return tb_sw_write(sw, &value, TB_CFG_SWITCH, offset, 1);
+}
+
+static int __tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+{
+	u32 offset = 1;
 
 	do {
 		struct tb_cap_any header;
@@ -62,6 +77,30 @@ int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
 	return -ENOENT;
 }
 
+/**
+ * tb_port_find_cap() - Find port capability
+ * @port: Port to find the capability for
+ * @cap: Capability to look
+ *
+ * Returns offset to start of capability or %-ENOENT if no such
+ * capability was found. Negative errno is returned if there was an
+ * error.
+ */
+int tb_port_find_cap(struct tb_port *port, enum tb_port_cap cap)
+{
+	int ret;
+
+	ret = tb_port_enable_tmu(port, true);
+	if (ret)
+		return ret;
+
+	ret = __tb_port_find_cap(port, cap);
+
+	tb_port_enable_tmu(port, false);
+
+	return ret;
+}
+
 static int tb_switch_find_cap(struct tb_switch *sw, enum tb_switch_cap cap)
 {
 	int offset = sw->config.first_cap_offset;
-- 
2.20.1


^ permalink raw reply related

* [PATCH iproute2-next v2] netns: add subcommand to attach an existing network namespace
From: Matteo Croce @ 2019-01-29 15:01 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern, Stephen Hemminger, Andrea Claudi

ip tracks namespaces with dummy files in /var/run/netns/, but can't see
namespaces created with other tools.
Creating the dummy file and bind mounting the correct procfs entry will
make ip aware of that namespace.
Add an ip netns subcommand to automate this task.

Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 ip/ipnetns.c        | 62 ++++++++++++++++++++++++++++++++++-----------
 man/man8/ip-netns.8 | 10 ++++++++
 2 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 03879b49..430d8844 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -28,6 +28,7 @@ static int usage(void)
 {
 	fprintf(stderr, "Usage: ip netns list\n");
 	fprintf(stderr, "       ip netns add NAME\n");
+	fprintf(stderr, "       ip netns attach NAME PID\n");
 	fprintf(stderr, "       ip netns set NAME NETNSID\n");
 	fprintf(stderr, "       ip [-all] netns delete [NAME]\n");
 	fprintf(stderr, "       ip netns identify [PID]\n");
@@ -632,24 +633,40 @@ static int create_netns_dir(void)
 	return 0;
 }
 
-static int netns_add(int argc, char **argv)
+static int netns_add(int argc, char **argv, bool create)
 {
 	/* This function creates a new network namespace and
 	 * a new mount namespace and bind them into a well known
 	 * location in the filesystem based on the name provided.
 	 *
+	 * If create is true, a new namespace will be created,
+	 * otherwise an existing one will be attached to the file.
+	 *
 	 * The mount namespace is created so that any necessary
 	 * userspace tweaks like remounting /sys, or bind mounting
-	 * a new /etc/resolv.conf can be shared between uers.
+	 * a new /etc/resolv.conf can be shared between users.
 	 */
-	char netns_path[PATH_MAX];
+	char netns_path[PATH_MAX], proc_path[PATH_MAX];
 	const char *name;
+	pid_t pid;
 	int fd;
 	int made_netns_run_dir_mount = 0;
 
-	if (argc < 1) {
-		fprintf(stderr, "No netns name specified\n");
-		return -1;
+	if (create) {
+		if (argc < 1) {
+			fprintf(stderr, "No netns name specified\n");
+			return -1;
+		}
+	} else {
+		if (argc < 2) {
+			fprintf(stderr, "No netns name and PID specified\n");
+			return -1;
+		}
+
+		if (get_s32(&pid, argv[1], 0) || !pid) {
+			fprintf(stderr, "Invalid PID: %s\n", argv[1]);
+			return -1;
+		}
 	}
 	name = argv[0];
 
@@ -689,21 +706,33 @@ static int netns_add(int argc, char **argv)
 		return -1;
 	}
 	close(fd);
-	if (unshare(CLONE_NEWNET) < 0) {
-		fprintf(stderr, "Failed to create a new network namespace \"%s\": %s\n",
-			name, strerror(errno));
-		goto out_delete;
+
+	if (create) {
+		if (unshare(CLONE_NEWNET) < 0) {
+			fprintf(stderr, "Failed to create a new network namespace \"%s\": %s\n",
+				name, strerror(errno));
+			goto out_delete;
+		}
+
+		strcpy(proc_path, "/proc/self/ns/net");
+	} else {
+		snprintf(proc_path, sizeof(proc_path), "/proc/%d/ns/net", pid);
 	}
 
 	/* Bind the netns last so I can watch for it */
-	if (mount("/proc/self/ns/net", netns_path, "none", MS_BIND, NULL) < 0) {
-		fprintf(stderr, "Bind /proc/self/ns/net -> %s failed: %s\n",
-			netns_path, strerror(errno));
+	if (mount(proc_path, netns_path, "none", MS_BIND, NULL) < 0) {
+		fprintf(stderr, "Bind %s -> %s failed: %s\n",
+			proc_path, netns_path, strerror(errno));
 		goto out_delete;
 	}
 	return 0;
 out_delete:
-	netns_delete(argc, argv);
+	if (create) {
+		netns_delete(argc, argv);
+	} else if (unlink(netns_path) < 0) {
+		fprintf(stderr, "Cannot remove namespace file \"%s\": %s\n",
+			netns_path, strerror(errno));
+	}
 	return -1;
 }
 
@@ -846,7 +875,7 @@ int do_netns(int argc, char **argv)
 		return usage();
 
 	if (matches(*argv, "add") == 0)
-		return netns_add(argc-1, argv+1);
+		return netns_add(argc-1, argv+1, true);
 
 	if (matches(*argv, "set") == 0)
 		return netns_set(argc-1, argv+1);
@@ -866,6 +895,9 @@ int do_netns(int argc, char **argv)
 	if (matches(*argv, "monitor") == 0)
 		return netns_monitor(argc-1, argv+1);
 
+	if (matches(*argv, "attach") == 0)
+		return netns_add(argc-1, argv+1, false);
+
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip netns help\".\n", *argv);
 	exit(-1);
 }
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index d539f18b..39a10e76 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -19,6 +19,10 @@ ip-netns \- process network namespace management
 .B ip netns add
 .I NETNSNAME
 
+.ti -8
+.B ip netns attach
+.I NETNSNAME PID
+
 .ti -8
 .B ip [-all] netns del
 .RI "[ " NETNSNAME " ]"
@@ -89,6 +93,12 @@ This command displays all of the network namespaces in /var/run/netns
 If NAME is available in /var/run/netns/ this command creates a new
 network namespace and assigns NAME.
 
+.TP
+.B ip netns attach NAME PID - create a new named network namespace
+.sp
+If NAME is available in /var/run/netns/ this command attaches the network
+namespace of the process PID to NAME as if it were created with ip netns.
+
 .TP
 .B ip [-all] netns delete [ NAME ] - delete the name of a network namespace(s)
 .sp
-- 
2.20.1


^ permalink raw reply related

* Re: marvell 6190 NAT performance
From: Andrew Lunn @ 2019-01-29 14:56 UTC (permalink / raw)
  To: Marek Behún; +Cc: Florian Fainelli, netdev, Russell King
In-Reply-To: <20190129152734.5bf998ea@dellmb.labs.office.nic.cz>

On Tue, Jan 29, 2019 at 03:27:34PM +0100, Marek Behún wrote:
> Hi Florian,
> I've made a screenshot of perf top when doing the NAT throughput test
> without the switch (which too doesn't work on 1000mbps as I thought,
> but on ~680 mbps). What do you think about the result?
> 
> http://blackhole.sk/~kabel/tmp/a3700_nat_perf.png

Hi Marek

This is plain text, you can just cut/paste it into the email.

What you actually want to do is a side by side comparison of this and
the case where it does go through the switch. What are the big
changes?

	Andrew

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: Save switch rules
From: Andrew Lunn @ 2019-01-29 14:51 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Florian Fainelli, Vivien Didelot, David S. Miller, netdev,
	linux-kernel, Thomas Petazzoni, Gregory Clement, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai
In-Reply-To: <20190129100117.5ef6774c@xps13>

On Tue, Jan 29, 2019 at 10:01:17AM +0100, Miquel Raynal wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> wrote on Mon, 28 Jan 2019 18:42:46 +0100:
> 
> > On Mon, Jan 28, 2019 at 04:57:49PM +0100, Miquel Raynal wrote:
> > > Hi Andrew,
> > > 
> > > Thanks for helping!
> > > 
> > > Andrew Lunn <andrew@lunn.ch> wrote on Mon, 28 Jan 2019 15:44:17 +0100:
> > >   
> > > > > I don't see where VLAN and bridge information are cached, can you point
> > > > > me to the relevant locations?    
> > > > 
> > > > Miquèl
> > > > 
> > > > The bridge should have all that information. You need to ask it to
> > > > enumerate the current configuration and replay it to the switch.
> > > > 
> > > > There might be something in the Mellanox driver you can copy? But i've
> > > > not looked, i'm just guessing.  
> > > 
> > > I am still searching but so far I did not find a mechanism reading the
> > > configuration of the bridge out of a 'net' object. Indeed there are
> > > multiple lists with the configuration but they are all 'mellanox'
> > > objects, they do not belong to the core.  
> > 
> > Hi Miquèl
> > 
> > Look at how iproute2 works. How does the bridge command enumerate the
> > fdb and mdb's? How does bridge vlan show work? bridge link show? See
> > if you can use this infrastructure within the kernel.
> 
> Thanks!
> 
> > 
> > > > We also need to think about how we are going to test this. There is a
> > > > lot of state information in a switch. So we are going to need some
> > > > pretty good tests to show we have recreated all of it.  
> > > 
> > > My understanding of all this is rather short, until know I used what
> > > you proposed in the v1 of this series but I am all ears if I need to
> > > add anything to my test list.  
> > 
> > What you probably need is a generic DSA test suite, with a number of
> > hardware devices, with different generations of mv88e6xxx devices, and
> > ideally different sf2, kzs, etc switches. Setup a configuration and
> > test is works correctly. Suspend, resume, and test is still works. And
> > you probably need to go through a number of cycles of suspend/resume.
> > And you are going to need to maintain that for a number of years,
> > testing every release, to see what breaks as we add new features and
> > new devices.
> 
> I am very sorry but I kind of disagree with the above proposal. Usually
> contributors try to write the best solution with the help of the
> community, test on the hardware they have in hand and propose the
> changes. I cannot bond on a 10-years involvement in testing several
> switches over the releases.

Hi Miquèl

I was trying to point out this is a very hard subject to tackle. And
to do it right is not going to be a few patches. It needs a lot of
work, and a lot of testing, and it needs ongoing work because the
mv88e6xxx driver is not complete, there are more features to add,
which are going to need suspend/resume support adding.

> Today, there is no S2RAM support for switches. First, I proposed to add
> suspend/resume callbacks to the mv88e6xxx driver - just enough to avoid
> crashing the kernel.

Then i would suggest the mv88e6xxx refuses the suspend. Actually that
probably is the first correct step. We don't have suspend support, so
stop the suspend happening, so preventing the kernel crash.

Having to maintain the mv88e6xxx, i don't want a suspend which might
work in the simplest configuration, but fails badly for more complex
configurations. Before accepting any patches, i want a good feeling it
works correctly. I would be willing to accept support and testing on
one Marvell family of switches, but again, i want to know it is well
tested. And i want to know somebody is going to stay around and look
after the support as the switch driver develops new features, which
are going to need suspend/resume support.

If you are only willing to consider a limited number of features, you
need to track if the switch is still within those limited set of
features, and refuse the suspend if not.

> > There also needs to be some though put into what happens when the
> > network changes while the switch is suspended. A port looses its link,
> > a port comes up, an SFP module is ejected, and SFP module is
> > inserted. The PTP grand master moves, etc. I hope the usual mechanisms
> > just work, but it all needs testing.
> 
> Is this really specific to switches? I know it is an issue and I
> understand you would prefer not to support S2RAM at all rather than
> addressing part of it, but isn't it better to support the simplest
> situation first, than supporting nothing at all?

Worst case scenario, you induce a loop in your network, and a
broadcast storm takes down the whole network. It is unlikely, but it
is very disruptive if it does happen. It is also the sort of situation
which is probably not going to get tested, making it more likely to
actually happen. And this is specific to switches. A single network
card cannot do this, you need two ports to form a loop.

     Andrew


^ permalink raw reply

* [PATCH net] net: 8139cp: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-01-29 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, yang.wei9

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in cp_tx() when skb xmit
done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/realtek/8139cp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 44f6e48..4f910c4 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -691,7 +691,7 @@ static void cp_tx (struct cp_private *cp)
 			}
 			bytes_compl += skb->len;
 			pkts_compl++;
-			dev_kfree_skb_irq(skb);
+			dev_consume_skb_irq(skb);
 		}
 
 		cp->tx_skb[tx_tail] = NULL;
-- 
2.7.4



^ permalink raw reply related

* Re: marvell 6190 NAT performance
From: Marek Behún @ 2019-01-29 14:27 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Andrew Lunn, netdev, Russell King
In-Reply-To: <ea329d29-36eb-0eec-666f-1cc12940b16f@gmail.com>

Hi Florian,
I've made a screenshot of perf top when doing the NAT throughput test
without the switch (which too doesn't work on 1000mbps as I thought,
but on ~680 mbps). What do you think about the result?

http://blackhole.sk/~kabel/tmp/a3700_nat_perf.png

Marek

On Thu, 24 Jan 2019 13:31:24 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 12:26 PM, Marek Behun wrote:
> > Hello,
> > 
> > I am encountering strange performance issue when benchmarking NAT
> > performance on Armada 3720 with Marvell 88e6190 switch.
> > 
> > Download speed (from internet, via Armada 3720 NAT, via switch to
> > LAN device) is ~750mbps and the CPU running on 100% (mostly in
> > ksoftirq). Upload speed is ~250mbps.
> > 
> > When the LAN device is connected to A3720 directly (via SFP), the
> > speeds are both ~1000mbps.  
> 
> OK and that presumably uses the second Ethernet MAC on the SoC right?
> 
> > 
> > I realize that packing/unpacking packets with Marvell header for the
> > switch takes some time, but is such a performance drop expected?  
> 
> If you run perf top/record you would be able to see that pretty
> quickly, I would not think that processing of the Marvell DSA tag
> would incur such a high penalty though since the packets are already
> hot in D$ by the time we get to mangle them for the DSA network
> devices.
> 
> How about pure (non-NAT) IP routing? How about just bridging between
> WAN and LAN?
> 
> > 
> > This was tested with 5.0.0-rc2 and also 4.14.
> > 
> > Thank you.
> > 
> > Marek
> >   
> 


^ permalink raw reply

* [PATCH net] ixgbe: fix potential RX buffer starvation for AF_XDP
From: Magnus Karlsson @ 2019-01-29 14:03 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, intel-wired-lan; +Cc: netdev

When the RX rings are created they are also populated with buffers so
that packets can be received. Usually these are kernel buffers, but
for AF_XDP in zero-copy mode, these are user-space buffers and in this
case the application might not have sent down any buffers to the
driver at this point. And if no buffers are allocated at ring creation
time, no packets can be received and no interupts will be generated so
the napi poll function that allocates buffers to the rings will never
get executed.

To recitfy this, we kick the NAPI context of any queue with an
attached AF_XDP zero-copy socket in two places in the code. Once after
an XDP program has loaded and once after the umem is registered.  This
take care of both cases: XDP program gets loaded first then AF_XDP
socket is created, and the reverse, AF_XDP socket is created first,
then XDP program is loaded.

Fixes: d0bcacd0a130 ("ixgbe: add AF_XDP zero-copy Rx support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 12 +++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c  | 12 ++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff818..017c930 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10225,6 +10225,7 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	int i, frame_size = dev->mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 	struct bpf_prog *old_prog;
+	bool need_reset;
 
 	if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
 		return -EINVAL;
@@ -10247,9 +10248,10 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 		return -ENOMEM;
 
 	old_prog = xchg(&adapter->xdp_prog, prog);
+	need_reset = (!!prog != !!old_prog);
 
 	/* If transitioning XDP modes reconfigure rings */
-	if (!!prog != !!old_prog) {
+	if (need_reset) {
 		int err = ixgbe_setup_tc(dev, adapter->hw_tcs);
 
 		if (err) {
@@ -10265,6 +10267,14 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	if (old_prog)
 		bpf_prog_put(old_prog);
 
+	/* Kick start the NAPI context if there is an AF_XDP socket open
+	 * on that queue id. This so that receiving will start.
+	 */
+	if (need_reset && prog)
+		for (i = 0; i < adapter->num_rx_queues; i++)
+			if (adapter->xdp_ring[i]->xsk_umem)
+				(void)ixgbe_xsk_async_xmit(adapter->netdev, i);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 65c3e2c..654ae92 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -144,11 +144,19 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
 		ixgbe_txrx_ring_disable(adapter, qid);
 
 	err = ixgbe_add_xsk_umem(adapter, umem, qid);
+	if (err)
+		return err;
 
-	if (if_running)
+	if (if_running) {
 		ixgbe_txrx_ring_enable(adapter, qid);
 
-	return err;
+		/* Kick start the NAPI context so that receiving will start */
+		err = ixgbe_xsk_async_xmit(adapter->netdev, qid);
+		if (err)
+			return err;
+	}
+
+	return 0;
 }
 
 static int ixgbe_xsk_umem_disable(struct ixgbe_adapter *adapter, u16 qid)
-- 
2.7.4


^ permalink raw reply related

* [PATCH net] i40e: fix potential RX buffer starvation for AF_XDP
From: Magnus Karlsson @ 2019-01-29 14:03 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, intel-wired-lan; +Cc: netdev

When the RX rings are created they are also populated with buffers
so that packets can be received. Usually these are kernel buffers,
but for AF_XDP in zero-copy mode, these are user-space buffers and
in this case the application might not have sent down any buffers
to the driver at this point. And if no buffers are allocated at ring
creation time, no packets can be received and no interupts will be
generated so the napi poll function that allocates buffers to the
rings will never get executed.

To rectify this, we kick the NAPI context of any queue with an
attached AF_XDP zero-copy socket in two places in the code. Once
after an XDP program has loaded and once after the umem is registered.
This take care of both cases: XDP program gets loaded first then AF_XDP
socket is created, and the reverse, AF_XDP socket is created first,
then XDP program is loaded.

Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 13 ++++++++++++-
 drivers/net/ethernet/intel/i40e/i40e_xsk.c  |  5 +++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index f52e2c4..3a0990d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3289,8 +3289,11 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	     i40e_alloc_rx_buffers_zc(ring, I40E_DESC_UNUSED(ring)) :
 	     !i40e_alloc_rx_buffers(ring, I40E_DESC_UNUSED(ring));
 	if (!ok) {
+		/* Log this in case the user has forgotten to give the kernel
+		 * any buffers, even later in the application.
+		 */
 		dev_info(&vsi->back->pdev->dev,
-			 "Failed allocate some buffers on %sRx ring %d (pf_q %d)\n",
+			 "Failed to allocate some buffers on %sRx ring %d (pf_q %d)\n",
 			 ring->xsk_umem ? "UMEM enabled " : "",
 			 ring->queue_index, pf_q);
 	}
@@ -11895,6 +11898,14 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
 	if (old_prog)
 		bpf_prog_put(old_prog);
 
+	/* Kick start the NAPI context if there is an AF_XDP socket open
+	 * on that queue id. This so that receiving will start.
+	 */
+	if (need_reset && prog)
+		for (i = 0; i < vsi->num_queue_pairs; i++)
+			if (vsi->xdp_rings[i]->xsk_umem)
+				(void)i40e_xsk_async_xmit(vsi->netdev, i);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 870cf654..3827f16 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -183,6 +183,11 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
 		err = i40e_queue_pair_enable(vsi, qid);
 		if (err)
 			return err;
+
+		/* Kick start the NAPI context so that receiving will start */
+		err = i40e_xsk_async_xmit(vsi->netdev, qid);
+		if (err)
+			return err;
 	}
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net V2] MAINTAINERS: Add entry for XDP (eXpress Data Path)
From: Jesper Dangaard Brouer @ 2019-01-29 13:22 UTC (permalink / raw)
  To: netdev
  Cc: Jakub Kicinski, John Fastabend, Alexei Starovoitov,
	Jesper Dangaard Brouer, Daniel Borkmann, David S. Miller

Add multiple people as maintainers for XDP, sorted alphabetically.

XDP is also tied to driver level support and code, but we cannot add all
drivers to the list. Instead K: and N: match on 'xdp' in hope to catch some
of those changes in drivers.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 MAINTAINERS |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a0245fd1b09a..a4ebc0068400 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16686,6 +16686,24 @@ T:	git git://linuxtv.org/media_tree.git
 S:	Maintained
 F:	drivers/media/tuners/tuner-xc2028.*
 
+XDP (eXpress Data Path)
+M:	Alexei Starovoitov <ast@kernel.org>
+M:	Daniel Borkmann <daniel@iogearbox.net>
+M:	David S. Miller <davem@davemloft.net>
+M:	Jakub Kicinski <jakub.kicinski@netronome.com>
+M:	Jesper Dangaard Brouer <hawk@kernel.org>
+M:	John Fastabend <john.fastabend@gmail.com>
+L:	netdev@vger.kernel.org
+L:	xdp-newbies@vger.kernel.org
+S:	Supported
+F:	net/core/xdp.c
+F:	include/net/xdp.h
+F:	kernel/bpf/devmap.c
+F:	kernel/bpf/cpumap.c
+F:	include/trace/events/xdp.h
+K:	xdp
+N:	xdp
+
 XDP SOCKETS (AF_XDP)
 M:	Björn Töpel <bjorn.topel@intel.com>
 M:	Magnus Karlsson <magnus.karlsson@intel.com>


^ permalink raw reply related

* Re: [PATCH v2] i40e: replace switch-statement to speed-up retpoline-enabled builds
From: Björn Töpel @ 2019-01-29 13:17 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: intel-wired-lan, Björn Töpel, Paul Menzel,
	Jesper Dangaard Brouer, Karlsson, Magnus, Magnus Karlsson, Netdev,
	Alexei Starovoitov, David Miller
In-Reply-To: <5650fdb7-5b86-9c7d-c112-1ff5ee7812b7@iogearbox.net>

Den tis 29 jan. 2019 kl 12:17 skrev Daniel Borkmann <daniel@iogearbox.net>:
>
> On 01/29/2019 10:57 AM, bjorn.topel@gmail.com wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > GCC will generate jump tables for switch-statements with more than 5
> > case statements. An entry into the jump table is an indirect call,
> > which means that for CONFIG_RETPOLINE builds, this is rather
> > expensive.
> >
> > This commit replaces the switch-statement that acts on the XDP program
> > result with an if-clause.
> >
> > The if-clause was also refactored into a common function that can be
> > used by AF_XDP zero-copy and non-zero-copy code.
> >
> > Performance prior this patch:
> > $ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
> > Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
> > XDP stats       CPU     pps         issue-pps
> > XDP-RX CPU      20      18983018    0
> > XDP-RX CPU      total   18983018
> >
> > RXQ stats       RXQ:CPU pps         issue-pps
> > rx_queue_index   20:20  18983012    0
> > rx_queue_index   20:sum 18983012
> >
> > $ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
> >  sock0@enp134s0f0:20 rxdrop
> >                 pps         pkts        2.00
> > rx              14,641,496  144,751,092
> > tx              0           0
> >
> > And after:
> > $ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
> > Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
> > XDP stats       CPU     pps         issue-pps
> > XDP-RX CPU      20      24000986    0
> > XDP-RX CPU      total   24000986
> >
> > RXQ stats       RXQ:CPU pps         issue-pps
> > rx_queue_index   20:20  24000985    0
> > rx_queue_index   20:sum 24000985
> >
> >   +26%
> >
> > $ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
> >  sock0@enp134s0f0:20 rxdrop
> >                 pps         pkts        2.00
> > rx              17,623,578  163,503,263
> > tx              0           0
> >
> >   +20%
> >
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
> Looks good. Given the performance improvements, wondering in general whether
> it would make sense to raise the default limit for generating jump tables if
> we have CONFIG_RETPOLINE enabled; as in:
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 9c5a67d..33495a9 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -217,6 +217,8 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
>  # Avoid indirect branches in kernel to deal with Spectre
>  ifdef CONFIG_RETPOLINE
>    KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
> +  # Avoid generating slow indirect jumps for small number of switch cases
> +  KBUILD_CFLAGS += --param case-values-threshold=12

Yes, it might make sense to raise it. All XDP capable drivers use a
switch to act on the action.

The default GCC for x86-64 is 5; I'm curious why you're suggesting 12,
I'd pick 17. ;-P


Björn

>  endif
>
>  archscripts: scripts_basic
>
> That would likely bloat the kernel a bit also in slow-path places where it
> would not be needed, but it would generically catch majority of cases. I'll
> run some experiments later today (but in any case that should not block this
> patch here).
>
> Cheers,
> Daniel

^ permalink raw reply

* Re: [RFC net-next 1/4] net: Reserve protocol identifiers for EnOcean
From: Alexander Aring @ 2019-01-29 12:57 UTC (permalink / raw)
  To: Andreas Färber
  Cc: linux-lpwan, linux-wpan, Alexander Aring, Stefan Schmidt, netdev,
	linux-kernel, support
In-Reply-To: <20190129050130.10932-2-afaerber@suse.de>

Hi,

On Tue, Jan 29, 2019 at 06:01:27AM +0100, Andreas Färber wrote:
> EnOcean wireless technology is based on ASK (ERP1) and FSK (ERP2) modulations
> for sub-GHz and on IEEE 802.15.4 for 2.4 GHz.
> 

I am not sure what you try to do here. If I see that correctly you
want to add for some special protocol vendor specific transceiver which
is underneath an 802.15.4 transceiver a new ARPHRD type and even more
for each modulation what it supports?

If it's a 802.15.4 transceiver why not using the 802.15.4 subsystem?

For me it sounds more like a HardMAC transceiver driver for doing the
vendor protocol. The different modulations is part of a 802.15.4 phy
device class. Similar like in wireless.

- Alex

^ permalink raw reply

* Re: [PATCH net-next] wireless: fix typos
From: Kalle Valo @ 2019-01-29 12:49 UTC (permalink / raw)
  To: Matteo Croce
  Cc: linux-wireless, netdev, Arend van Spriel, Franky Lin,
	Hante Meuleman, David S . Miller, Johannes Berg
In-Reply-To: <87va2ioaee.fsf@kamboji.qca.qualcomm.com>

Kalle Valo <kvalo@codeaurora.org> writes:

> Matteo Croce <mcroce@redhat.com> writes:
>
>> Fix spelling mistakes in brcm80211 and cfg80211: "lenght" -> "length".
>> The typos are also in the special comment blocks which
>> translates to documentation.
>>
>> Signed-off-by: Matteo Croce <mcroce@redhat.com>
>> ---
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h     | 2 +-
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 +-
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h     | 2 +-
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c      | 2 +-
>>  include/net/cfg80211.h                                      | 2 +-
>
> cfg80211 changes go through mac80211-next. Can you please split these in
> two patches, cfg80211 and brcmfmac?

Strange, my previous comment didn't make it to patchwork. Let's try
again :)

https://patchwork.kernel.org/patch/10773337/

-- 
Kalle Valo

^ permalink raw reply

* VSOCK performance
From: Stefano Garzarella @ 2019-01-29 11:39 UTC (permalink / raw)
  To: jiangyiwen
  Cc: Michael Tsirkin, Stefan Hajnoczi, Jason Wang, netdev,
	qemu devel list

Hi Yiwen,
I'm currently interested on how to improve the VSOCK performance and I
read your discussions with Jason, Michael, and Stefan on both
linux-netdev and qemu-devel mailing lists.

Are you still working on it?

Reading the discussions I understood that batching can help us a lot
to increase the VSOCK performance (especially for guest->host
direction).

As a first step, I added VSOCK support to iperf3 [1] in order to
measure the current performance of VSOCK (without any modification):

              Single socket
Guest->Host        ~950MB/s
Host->Guest       ~2700MB/s

They are slightly different from your results but I think that depends
on the hardware.
My current setup is the following:
HW: Lenovo T480s (i7-8650U, DDR4 2133 Mhz)
OS (guest/host): Fedora 29 (Linux 4.20.4-200.fc29.x86_64)
Guest app: iperf3 --vsock -s
Host app:  iperf3 --vsock -c $GUEST_CID -l 64K


Just to double check these values, can you share with me the tool that you used?


Thanks,
Stefano

#1 https://github.com/esnet/iperf/pull/840

-- 
Stefano Garzarella
Software Engineer @ Red Hat

^ permalink raw reply

* Re: [PATCHv2 net] sctp: check and update stream->out_curr when allocating stream_out
From: Marcelo Ricardo Leitner @ 2019-01-29 12:05 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <8a5df8eb5cc41dfc3d08e294147b9729bbe90aa0.1543473776.git.lucien.xin@gmail.com>

On Thu, Nov 29, 2018 at 02:42:56PM +0800, Xin Long wrote:
> Now when using stream reconfig to add out streams, stream->out
> will get re-allocated, and all old streams' information will
> be copied to the new ones and the old ones will be freed.
> 
> So without stream->out_curr updated, next time when trying to
> send from stream->out_curr stream, a panic would be caused.
> 
> This patch is to check and update stream->out_curr when
> allocating stream_out.
> 
> v1->v2:
>   - define fa_index() to get elem index from stream->out_curr.
> 
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Reported-by: Ying Xu <yinxu@redhat.com>
> Reported-by: syzbot+e33a3a138267ca119c7d@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

We are sort of mixing things up here. We have a bug on SCTP stack that
triggers panics. As good practices recommends, the code should be as
generic as possible and the SCTP-only was dropped in favor of a more
generic one, fixing rhashtables instead. Okay. But then we discovered
rhashtables are going away and we are now waiting on a restructing
to fix the panic. That's not good, especially because it cannot and
should not be backported into -stable trees.

That said, we should not wait for the restructuring to _implicitly_
fix the bug. We should pursuit both fixes here:
- Apply this patch, to fix SCTP stack and allow it to be easily
  backportable.
- Apply the generic fix, which is the restructuring, whenever it
  actually lands.

Thoughts?

Thanks,
Marcelo

^ permalink raw reply

* Q: is it possible to use macvtap with lowerdev hotplug?
From: Mike Rapoport @ 2019-01-29 11:52 UTC (permalink / raw)
  To: netdev, virtualization

Hi,

I have a setup in which VMs are connected with macvtap to the network. The
macvtap interfaces are attached to a physical NIC.

There is a question if it is possible to hot-unplug - hot-plug the physical
NIC and retain the VMs connectivity to the network (of course with some
hiccup for the unplug-plug time).

As far as I could tell from the macvtap/macvlan code, once lowerdev is gone
there is nothing that can be done to reattach macvlan/macvtap to another
lowerdev.

My question is there something fundamental that prevents it?

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* Re: WoL broken in r8169.c since kernel 4.19
From: Marc Haber @ 2019-01-29 11:52 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: netdev@vger.kernel.org
In-Reply-To: <48876bf2-19ff-3777-90d1-dfbfe26d01c7@gmail.com>

On Mon, Jan 28, 2019 at 10:21:50PM +0100, Heiner Kallweit wrote:
> And I wonder why the following didn't work for you, you said it makes
> no difference. Could you try again the following in addition to the
> latest debug output statement?

This time it says
Jan 29 12:34:59 fan kernel: [  141.397307] may wakeup? 1
Jan 29 12:46:39 fan kernel: [  819.665683] may wakeup? 1

while the machine needed a manual wakeup at 12:34 as usual. The log
entry is only written after resume, first suspend was at 09:05 and the
machine was sleeping for three hours.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany    |  lose things."    Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature |  How to make an American Quilt | Fax: *49 6224 1600421

^ permalink raw reply

* [PATCH 4.19 077/103] net: sun: cassini: Cleanup license conflict
From: Greg Kroah-Hartman @ 2019-01-29 11:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Shannon Nelson,
	Zhu Yanjun, David S. Miller, netdev, Shannon Nelson
In-Reply-To: <20190129113159.567154026@linuxfoundation.org>

4.19-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Thomas Gleixner <tglx@linutronix.de>

commit 56cb4e5034998b5522a657957321ca64ca2ea0a0 upstream.

The recent addition of SPDX license identifiers to the files in
drivers/net/ethernet/sun created a licensing conflict.

The cassini driver files contain a proper license notice:

  * 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.

but the SPDX change added:

   SPDX-License-Identifier: GPL-2.0

So the file got tagged GPL v2 only while in fact it is licensed under GPL
v2 or later.

It's nice that people care about the SPDX tags, but they need to be more
careful about it. Not everything under (the) sun belongs to ...

Fix up the SPDX identifier and remove the boiler plate text as it is
redundant.

Fixes: c861ef83d771 ("sun: Add SPDX license tags to Sun network drivers")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Shannon Nelson <shannon.nelson@oracle.com>
Cc: Zhu Yanjun <yanjun.zhu@oracle.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: stable@vger.kernel.org
Acked-by: Shannon Nelson <shannon.lee.nelson@gmail.com>
Reviewed-by: Zhu Yanjun <yanjun.zhu@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/net/ethernet/sun/cassini.c |   15 +--------------
 drivers/net/ethernet/sun/cassini.h |   15 +--------------
 2 files changed, 2 insertions(+), 28 deletions(-)

--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1,22 +1,9 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0+
 /* cassini.c: Sun Microsystems Cassini(+) ethernet driver.
  *
  * Copyright (C) 2004 Sun Microsystems Inc.
  * Copyright (C) 2003 Adrian Sun (asun@darksunrising.com)
  *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
  * This driver uses the sungem driver (c) David Miller
  * (davem@redhat.com) as its basis.
  *
--- a/drivers/net/ethernet/sun/cassini.h
+++ b/drivers/net/ethernet/sun/cassini.h
@@ -1,23 +1,10 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: GPL-2.0+ */
 /* $Id: cassini.h,v 1.16 2004/08/17 21:15:16 zaumen Exp $
  * cassini.h: Definitions for Sun Microsystems Cassini(+) ethernet driver.
  *
  * Copyright (C) 2004 Sun Microsystems Inc.
  * Copyright (c) 2003 Adrian Sun (asun@darksunrising.com)
  *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
  * vendor id: 0x108E (Sun Microsystems, Inc.)
  * device id: 0xabba (Cassini)
  * revision ids: 0x01 = Cassini



^ permalink raw reply

* [PATCH 4.20 085/117] net: sun: cassini: Cleanup license conflict
From: Greg Kroah-Hartman @ 2019-01-29 11:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Shannon Nelson,
	Zhu Yanjun, David S. Miller, netdev, Shannon Nelson
In-Reply-To: <20190129113207.477505932@linuxfoundation.org>

4.20-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Thomas Gleixner <tglx@linutronix.de>

commit 56cb4e5034998b5522a657957321ca64ca2ea0a0 upstream.

The recent addition of SPDX license identifiers to the files in
drivers/net/ethernet/sun created a licensing conflict.

The cassini driver files contain a proper license notice:

  * 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.

but the SPDX change added:

   SPDX-License-Identifier: GPL-2.0

So the file got tagged GPL v2 only while in fact it is licensed under GPL
v2 or later.

It's nice that people care about the SPDX tags, but they need to be more
careful about it. Not everything under (the) sun belongs to ...

Fix up the SPDX identifier and remove the boiler plate text as it is
redundant.

Fixes: c861ef83d771 ("sun: Add SPDX license tags to Sun network drivers")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Shannon Nelson <shannon.nelson@oracle.com>
Cc: Zhu Yanjun <yanjun.zhu@oracle.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: stable@vger.kernel.org
Acked-by: Shannon Nelson <shannon.lee.nelson@gmail.com>
Reviewed-by: Zhu Yanjun <yanjun.zhu@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/net/ethernet/sun/cassini.c |   15 +--------------
 drivers/net/ethernet/sun/cassini.h |   15 +--------------
 2 files changed, 2 insertions(+), 28 deletions(-)

--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1,22 +1,9 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0+
 /* cassini.c: Sun Microsystems Cassini(+) ethernet driver.
  *
  * Copyright (C) 2004 Sun Microsystems Inc.
  * Copyright (C) 2003 Adrian Sun (asun@darksunrising.com)
  *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
  * This driver uses the sungem driver (c) David Miller
  * (davem@redhat.com) as its basis.
  *
--- a/drivers/net/ethernet/sun/cassini.h
+++ b/drivers/net/ethernet/sun/cassini.h
@@ -1,23 +1,10 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: GPL-2.0+ */
 /* $Id: cassini.h,v 1.16 2004/08/17 21:15:16 zaumen Exp $
  * cassini.h: Definitions for Sun Microsystems Cassini(+) ethernet driver.
  *
  * Copyright (C) 2004 Sun Microsystems Inc.
  * Copyright (c) 2003 Adrian Sun (asun@darksunrising.com)
  *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
  * vendor id: 0x108E (Sun Microsystems, Inc.)
  * device id: 0xabba (Cassini)
  * revision ids: 0x01 = Cassini



^ permalink raw reply

* Re: [PATCH bpf-next v4 5/7] samples/bpf: Add a "force" flag to XDP samples
From: Jesper Dangaard Brouer @ 2019-01-29 11:34 UTC (permalink / raw)
  To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer
In-Reply-To: <20190129090000.3c34b873@redhat.com>

On Tue, 29 Jan 2019 09:00:00 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> On Mon, 28 Jan 2019 20:16:11 +0100
> Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> 
> > From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > 
> > Make xdp samples consistent with iproute2 behavior and set the
> > XDP_FLAGS_UPDATE_IF_NOEXIST by default when setting the xdp program on
> > interface. Provide an option for user to force the program loading,
> > which as a result will not include the mentioned flag in
> > bpf_set_link_xdp_fd call.  
> 
> I like the idea, but what is the error message users get after this
> change?

$ sudo ./xdp1 mlx5p1 &
[1] 9768

$ sudo ./xdp1 mlx5p1 
link set xdp fd failed

This error message is a little too generic to be a good user experience.
The kernel (in dev_change_xdp_fd) will return errno -EBUSY (-16), but
we don't use or report the return value in these sample programs.

If my QA see this error message, I will still get an error report
bugzilla that I need to spend time on investigating.  Can we please
improve this error message? 

If you are really cool you get inspired by (or use) libbpf_strerror()
code avail in tools/lib/bpf/libbpf_errno.c.  Default strerror(EBUSY)
will return "Device or resource busy", but maybe we can do slightly
better and report something more meaningful for this XDP context.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH v2] i40e: replace switch-statement to speed-up retpoline-enabled builds
From: Daniel Borkmann @ 2019-01-29 11:17 UTC (permalink / raw)
  To: bjorn.topel, intel-wired-lan
  Cc: Björn Töpel, pmenzel, brouer, magnus.karlsson,
	magnus.karlsson, netdev, alexei.starovoitov, davem
In-Reply-To: <20190129095754.9390-1-bjorn.topel@gmail.com>

On 01/29/2019 10:57 AM, bjorn.topel@gmail.com wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> GCC will generate jump tables for switch-statements with more than 5
> case statements. An entry into the jump table is an indirect call,
> which means that for CONFIG_RETPOLINE builds, this is rather
> expensive.
> 
> This commit replaces the switch-statement that acts on the XDP program
> result with an if-clause.
> 
> The if-clause was also refactored into a common function that can be
> used by AF_XDP zero-copy and non-zero-copy code.
> 
> Performance prior this patch:
> $ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
> Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      20      18983018    0
> XDP-RX CPU      total   18983018
> 
> RXQ stats       RXQ:CPU pps         issue-pps
> rx_queue_index   20:20  18983012    0
> rx_queue_index   20:sum 18983012
> 
> $ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
>  sock0@enp134s0f0:20 rxdrop
>                 pps         pkts        2.00
> rx              14,641,496  144,751,092
> tx              0           0
> 
> And after:
> $ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
> Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      20      24000986    0
> XDP-RX CPU      total   24000986
> 
> RXQ stats       RXQ:CPU pps         issue-pps
> rx_queue_index   20:20  24000985    0
> rx_queue_index   20:sum 24000985
> 
>   +26%
> 
> $ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
>  sock0@enp134s0f0:20 rxdrop
>                 pps         pkts        2.00
> rx              17,623,578  163,503,263
> tx              0           0
> 
>   +20%
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Looks good. Given the performance improvements, wondering in general whether
it would make sense to raise the default limit for generating jump tables if
we have CONFIG_RETPOLINE enabled; as in:

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 9c5a67d..33495a9 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -217,6 +217,8 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_RETPOLINE
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
+  # Avoid generating slow indirect jumps for small number of switch cases
+  KBUILD_CFLAGS += --param case-values-threshold=12
 endif

 archscripts: scripts_basic

That would likely bloat the kernel a bit also in slow-path places where it
would not be needed, but it would generically catch majority of cases. I'll
run some experiments later today (but in any case that should not block this
patch here).

Cheers,
Daniel

^ permalink raw reply related

* Re: [PATCH 0/7] sh_eth: implement simple RX checksum offload
From: Sergei Shtylyov @ 2019-01-29 11:06 UTC (permalink / raw)
  To: Heiner Kallweit, netdev, David S. Miller; +Cc: linux-renesas-soc, linux-sh
In-Reply-To: <62dbdf72-4e3f-6c3e-901f-5e8f6de8dec3@gmail.com>

Hello!

On 01/27/2019 08:52 PM, Heiner Kallweit wrote:

>> Here's a set of 7 patches against DaveM's 'net-next.git' repo. I'm implemeting
>> the simple RX checksum offload (like was done for the 'ravb' driver by Simon
>> Horman); it was only tested on the R8A77980 SoC, the other SoCs should just
>> work (according to their manuals)...
>>
>> [1/7] sh_eth: rename sh_eth_cpu_data::hw_checksum
>> [2/7] sh_eth: RX checksum offload support
>> [3/7] sh_eth: offload RX checksum on R7S72100
>> [4/7] sh_eth: offload RX checksum on R8A7740
>> [5/7] sh_eth: offload RX checksum on R8A77980
>> [6/7] sh_eth: offload RX checksum on SH7734
>> [7/7] sh_eth: offload RX checksum on SH7763
>>
>> MBR, Sergei
>>
> Hi Sergei,
> 
> the formatting of the patch series isn't in line with the netdev standards.
> See here: https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html
> 
> - cover letter isn't generated by git

   Sorry, I don't use git for development. However, I fail to see where this
is requested in the above FAQ.

>  
> - That's not ok
> --- *net-next.orig/*drivers/net/ethernet/renesas/sh_eth.h
> +++ *net-next/*drivers/net/ethernet/renesas/sh_eth.h
> 
> - patches miss net / net-next annotation

   I noted the applicable repo in the cover letter. I know I should use
the subject... but I just keep forgetting about this requirement. :-)
 
> Heiner

MBR, Sergei

^ permalink raw reply

* Re: [PATCH 3/7] sh_eth: offload RX checksum on R7S72100
From: Sergei Shtylyov @ 2019-01-29 10:37 UTC (permalink / raw)
  To: Simon Horman; +Cc: netdev, David S. Miller, linux-renesas-soc, linux-sh
In-Reply-To: <20190129080004.blc3jrt5y2j7dauo@verge.net.au>

Hello!

On 01/29/2019 11:00 AM, Simon Horman wrote:

>>>> The RZ/A1H (R7S721000) SoC manual describes the Ether MAC's RX checksum
>>>> offload the same way as it's implemented in the EtherAVB MACs...
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> Regarding this and the remaining patches in this series,
>>> which add rx-csum offload support in the sh_eth driver for
>>> various SoCs: has this been tested?
>>
>>    As I said, I've only tested it on R8A77980.

   And still hoping Geert would be able to test on R8A7740.

> 
> Thanks, I missed that.
> 
> As you may have guessed the implication of my question is that
> IMHO it would be best only to add this feature to SoCs where
> it has been tested.

   You don't trust the manuals? :-)

MBR, Sergei

^ permalink raw reply

* [PATCH net-next] MAINTAINERS: update cxgb4 and cxgb3 maintainer
From: Arjun Vynipadath @ 2019-01-29 10:08 UTC (permalink / raw)
  To: netdev, davem; +Cc: nirranjan, indranil, dt, leedom, Arjun Vynipadath

Vishal Kulkarni will be the new maintainer for Chelsio cxgb3/cxgb4
drivers.

Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a0245fd..071f1f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4122,7 +4122,7 @@ S:	Maintained
 F:	drivers/media/dvb-frontends/cxd2820r*
 
 CXGB3 ETHERNET DRIVER (CXGB3)
-M:	Arjun Vynipadath <arjun@chelsio.com>
+M:	Vishal Kulkarni <vishal@chelsio.com>
 L:	netdev@vger.kernel.org
 W:	http://www.chelsio.com
 S:	Supported
@@ -4151,7 +4151,7 @@ S:	Supported
 F:	drivers/crypto/chelsio
 
 CXGB4 ETHERNET DRIVER (CXGB4)
-M:	Arjun Vynipadath <arjun@chelsio.com>
+M:	Vishal Kulkarni <vishal@chelsio.com>
 L:	netdev@vger.kernel.org
 W:	http://www.chelsio.com
 S:	Supported
-- 
2.9.5


^ permalink raw reply related

* [PATCH v2] i40e: replace switch-statement to speed-up retpoline-enabled builds
From: bjorn.topel @ 2019-01-29  9:57 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Björn Töpel, pmenzel, brouer, magnus.karlsson,
	magnus.karlsson, netdev

From: Björn Töpel <bjorn.topel@intel.com>

GCC will generate jump tables for switch-statements with more than 5
case statements. An entry into the jump table is an indirect call,
which means that for CONFIG_RETPOLINE builds, this is rather
expensive.

This commit replaces the switch-statement that acts on the XDP program
result with an if-clause.

The if-clause was also refactored into a common function that can be
used by AF_XDP zero-copy and non-zero-copy code.

Performance prior this patch:
$ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      18983018    0
XDP-RX CPU      total   18983018

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  18983012    0
rx_queue_index   20:sum 18983012

$ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
 sock0@enp134s0f0:20 rxdrop
                pps         pkts        2.00
rx              14,641,496  144,751,092
tx              0           0

And after:
$ sudo ./xdp_rxq_info --dev enp134s0f0 --action XDP_DROP
Running XDP on dev:enp134s0f0 (ifindex:7) action:XDP_DROP options:no_touch
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      20      24000986    0
XDP-RX CPU      total   24000986

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index   20:20  24000985    0
rx_queue_index   20:sum 24000985

  +26%

$ sudo ./xdpsock -i enp134s0f0 -q 20 -n 2 -z -r
 sock0@enp134s0f0:20 rxdrop
                pps         pkts        2.00
rx              17,623,578  163,503,263
tx              0           0

  +20%

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
v1->v2: 

* Fixed build error on alpha "error: implicit declaration of function
  'unlikely'; did you mean 'inline'? " (kbuild test robot)

* Improved commit message (Paul Menzel)
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 32 ++++---------------
 .../ethernet/intel/i40e/i40e_txrx_common.h    | 27 ++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_xsk.c    | 24 ++------------
 3 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index a7e14e98889f..4f530427ce61 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2,7 +2,7 @@
 /* Copyright(c) 2013 - 2018 Intel Corporation. */
 
 #include <linux/prefetch.h>
-#include <linux/bpf_trace.h>
+#include <linux/compiler.h>
 #include <net/xdp.h>
 #include "i40e.h"
 #include "i40e_trace.h"
@@ -2195,41 +2195,23 @@ int i40e_xmit_xdp_tx_ring(struct xdp_buff *xdp, struct i40e_ring *xdp_ring)
 static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
 				    struct xdp_buff *xdp)
 {
-	int err, result = I40E_XDP_PASS;
-	struct i40e_ring *xdp_ring;
 	struct bpf_prog *xdp_prog;
+	int result;
 	u32 act;
 
 	rcu_read_lock();
 	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
 
-	if (!xdp_prog)
+	if (!xdp_prog) {
+		result = I40E_XDP_PASS;
 		goto xdp_out;
+	}
 
 	prefetchw(xdp->data_hard_start); /* xdp_frame write */
 
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
-	switch (act) {
-	case XDP_PASS:
-		break;
-	case XDP_TX:
-		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
-		result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
-		break;
-	case XDP_REDIRECT:
-		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
-		result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
-		break;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-		/* fall through */
-	case XDP_ABORTED:
-		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
-		/* fall through -- handle aborts by dropping packet */
-	case XDP_DROP:
-		result = I40E_XDP_CONSUMED;
-		break;
-	}
+	i40e_xdp_do_action(act, &result, rx_ring, xdp, xdp_prog);
+
 xdp_out:
 	rcu_read_unlock();
 	return ERR_PTR(-result);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
index 8af0e99c6c0d..8cc4d8365f9e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
@@ -4,6 +4,8 @@
 #ifndef I40E_TXRX_COMMON_
 #define I40E_TXRX_COMMON_
 
+#include <linux/bpf_trace.h>
+
 void i40e_fd_handle_status(struct i40e_ring *rx_ring,
 			   union i40e_rx_desc *rx_desc, u8 prog_id);
 int i40e_xmit_xdp_tx_ring(struct xdp_buff *xdp, struct i40e_ring *xdp_ring);
@@ -88,4 +90,29 @@ void i40e_xsk_clean_rx_ring(struct i40e_ring *rx_ring);
 void i40e_xsk_clean_tx_ring(struct i40e_ring *tx_ring);
 bool i40e_xsk_any_rx_ring_enabled(struct i40e_vsi *vsi);
 
+static inline void i40e_xdp_do_action(u32 act, int *result,
+				      struct i40e_ring *rx_ring,
+				      struct xdp_buff *xdp,
+				      struct bpf_prog *xdp_prog)
+{
+	struct i40e_ring *xdp_ring;
+	int err;
+
+	if (act == XDP_TX) {
+		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
+		*result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
+	} else if (act == XDP_REDIRECT) {
+		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
+		*result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
+	} else if (act == XDP_PASS) {
+		*result = I40E_XDP_PASS;
+	} else if (act == XDP_DROP) {
+		*result = I40E_XDP_CONSUMED;
+	} else {
+		if (act != XDP_ABORTED)
+			bpf_warn_invalid_xdp_action(act);
+		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
+		*result = I40E_XDP_CONSUMED;
+	}
+}
 #endif /* I40E_TXRX_COMMON_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 96d849460d9b..c9d58f49f7a2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -210,9 +210,8 @@ int i40e_xsk_umem_setup(struct i40e_vsi *vsi, struct xdp_umem *umem,
  **/
 static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp)
 {
-	int err, result = I40E_XDP_PASS;
-	struct i40e_ring *xdp_ring;
 	struct bpf_prog *xdp_prog;
+	int result;
 	u32 act;
 
 	rcu_read_lock();
@@ -222,26 +221,7 @@ static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct xdp_buff *xdp)
 	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
 	xdp->handle += xdp->data - xdp->data_hard_start;
-	switch (act) {
-	case XDP_PASS:
-		break;
-	case XDP_TX:
-		xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
-		result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
-		break;
-	case XDP_REDIRECT:
-		err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
-		result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
-		break;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-	case XDP_ABORTED:
-		trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
-		/* fallthrough -- handle aborts by dropping packet */
-	case XDP_DROP:
-		result = I40E_XDP_CONSUMED;
-		break;
-	}
+	i40e_xdp_do_action(act, &result, rx_ring, xdp, xdp_prog);
 	rcu_read_unlock();
 	return result;
 }
-- 
2.19.1


^ permalink raw reply related


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