Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: ethernet: mediatek: get out of potential invalid pointer access
From: David Miller @ 2016-09-22 12:23 UTC (permalink / raw)
  To: sean.wang
  Cc: john, nbd, netdev, linux-kernel, linux-mediatek, keyhaede, objelf
In-Reply-To: <1474533856-22753-1-git-send-email-sean.wang@mediatek.com>

From: <sean.wang@mediatek.com>
Date: Thu, 22 Sep 2016 16:44:16 +0800

> From: Sean Wang <sean.wang@mediatek.com>
> 
> Potential dangerous invalid pointer might be accessed if
> the error happens when couple phy_device to net_device so
> cleanup the error path.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>

Applied.

^ permalink raw reply

* Re: [PATCH 0/2] BQL support and fix for a regression issue
From: David Miller @ 2016-09-22 12:25 UTC (permalink / raw)
  To: sunil.kovvuri; +Cc: netdev, linux-kernel, linux-arm-kernel, sgoutham
In-Reply-To: <1474535121-13958-1-git-send-email-sunil.kovvuri@gmail.com>

From: sunil.kovvuri@gmail.com
Date: Thu, 22 Sep 2016 14:35:19 +0530

> From: Sunil Goutham <sgoutham@cavium.com>
> 
> These patches add byte queue limit support and also fixes a regression
> issue introduced by commit
> 'net: thunderx: Use netdev's name for naming VF's interrupts'

The correct way to refer to commits when you fix a bug is
to use the "Fixes: " tag right before the signoffs and ACKs.

The correct form is:

Fixes: $SHA1_ID ("Commit header line.")

You'll see this in many other netdev commits.

^ permalink raw reply

* Re: [PATCH resend 2] xen-netback: switch to threaded irq for control ring
From: David Miller @ 2016-09-22 12:26 UTC (permalink / raw)
  To: jgross; +Cc: xen-devel, netdev, linux-kernel, wei.liu2
In-Reply-To: <1474535185-15734-1-git-send-email-jgross@suse.com>

From: Juergen Gross <jgross@suse.com>
Date: Thu, 22 Sep 2016 11:06:25 +0200

> Instead of open coding it use the threaded irq mechanism in
> xen-netback.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH] ipvlan: fix building without netfilter
From: David Miller @ 2016-09-22 12:32 UTC (permalink / raw)
  To: arnd; +Cc: maheshb, dsa, edumazet, netdev, linux-kernel
In-Reply-To: <20160922094130.2154824-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 22 Sep 2016 11:40:52 +0200

> The new l3s mode in ipvlan relies on netfilter interfaces, but
> the ipvlan driver can be configured when CONFIG_NETFILTER is disabled,
> leading to a build error:
> 
> drivers/net/ipvlan/ipvlan.h:132:22: error: 'struct nf_hook_state' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
> drivers/net/ipvlan/ipvlan_main.c:14:27: error: array type has incomplete element type 'struct nf_hook_ops'
> ...
> 
> This adds a forward declaration for struct nf_hook_state, and hides
> the newly added l3s code in an #ifdef.
> 
> Fixes: 4fbae7d83c98 ("ipvlan: Introduce l3s mode")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I'm pretty sure I applied a Kconfig patch that added the
necessary dependency.

^ permalink raw reply

* pull-request: can 2016-09-22
From: Marc Kleine-Budde @ 2016-09-22 12:42 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request of one patch for the upcoming linux-4.8 release.

The patch by Sergei Miroshnichenko fixes a potential deadlock in the generic
CAN device code that cann occour after a bus-off.

regards,
Marc

---

The following changes since commit 7e32b44361abc77fbc01f2b97b045c405b2583e5:

  tcp: properly account Fast Open SYN-ACK retrans (2016-09-22 03:33:01 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.8-20160922

for you to fetch changes up to 9abefcb1aaa58b9d5aa40a8bb12c87d02415e4c8:

  can: dev: fix deadlock reported after bus-off (2016-09-22 10:01:21 +0200)

----------------------------------------------------------------
linux-can-fixes-for-4.8-20160922

----------------------------------------------------------------
Sergei Miroshnichenko (1):
      can: dev: fix deadlock reported after bus-off

 drivers/net/can/dev.c   | 27 +++++++++++++++++----------
 include/linux/can/dev.h |  3 ++-
 2 files changed, 19 insertions(+), 11 deletions(-)


^ permalink raw reply

* [PATCH] can: dev: fix deadlock reported after bus-off
From: Marc Kleine-Budde @ 2016-09-22 12:42 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Sergei Miroshnichenko, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20160922124222.31598-1-mkl@pengutronix.de>

From: Sergei Miroshnichenko <sergeimir@emcraft.com>

A timer was used to restart after the bus-off state, leading to a
relatively large can_restart() executed in an interrupt context,
which in turn sets up pinctrl. When this happens during system boot,
there is a high probability of grabbing the pinctrl_list_mutex,
which is locked already by the probe() of other device, making the
kernel suspect a deadlock condition [1].

To resolve this issue, the restart_timer is replaced by a delayed
work.

[1] https://github.com/victronenergy/venus/issues/24

Signed-off-by: Sergei Miroshnichenko <sergeimir@emcraft.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c   | 27 +++++++++++++++++----------
 include/linux/can/dev.h |  3 ++-
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index e21f7cc5ae4d..8d6208c0b400 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
+#include <linux/workqueue.h>
 #include <linux/can.h>
 #include <linux/can/dev.h>
 #include <linux/can/skb.h>
@@ -501,9 +502,8 @@ EXPORT_SYMBOL_GPL(can_free_echo_skb);
 /*
  * CAN device restart for bus-off recovery
  */
-static void can_restart(unsigned long data)
+static void can_restart(struct net_device *dev)
 {
-	struct net_device *dev = (struct net_device *)data;
 	struct can_priv *priv = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	struct sk_buff *skb;
@@ -543,6 +543,14 @@ restart:
 		netdev_err(dev, "Error %d during restart", err);
 }
 
+static void can_restart_work(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct can_priv *priv = container_of(dwork, struct can_priv, restart_work);
+
+	can_restart(priv->dev);
+}
+
 int can_restart_now(struct net_device *dev)
 {
 	struct can_priv *priv = netdev_priv(dev);
@@ -556,8 +564,8 @@ int can_restart_now(struct net_device *dev)
 	if (priv->state != CAN_STATE_BUS_OFF)
 		return -EBUSY;
 
-	/* Runs as soon as possible in the timer context */
-	mod_timer(&priv->restart_timer, jiffies);
+	cancel_delayed_work_sync(&priv->restart_work);
+	can_restart(dev);
 
 	return 0;
 }
@@ -578,8 +586,8 @@ void can_bus_off(struct net_device *dev)
 	netif_carrier_off(dev);
 
 	if (priv->restart_ms)
-		mod_timer(&priv->restart_timer,
-			  jiffies + (priv->restart_ms * HZ) / 1000);
+		schedule_delayed_work(&priv->restart_work,
+				      msecs_to_jiffies(priv->restart_ms));
 }
 EXPORT_SYMBOL_GPL(can_bus_off);
 
@@ -688,6 +696,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
 		return NULL;
 
 	priv = netdev_priv(dev);
+	priv->dev = dev;
 
 	if (echo_skb_max) {
 		priv->echo_skb_max = echo_skb_max;
@@ -697,7 +706,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
 
 	priv->state = CAN_STATE_STOPPED;
 
-	init_timer(&priv->restart_timer);
+	INIT_DELAYED_WORK(&priv->restart_work, can_restart_work);
 
 	return dev;
 }
@@ -778,8 +787,6 @@ int open_candev(struct net_device *dev)
 	if (!netif_carrier_ok(dev))
 		netif_carrier_on(dev);
 
-	setup_timer(&priv->restart_timer, can_restart, (unsigned long)dev);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(open_candev);
@@ -794,7 +801,7 @@ void close_candev(struct net_device *dev)
 {
 	struct can_priv *priv = netdev_priv(dev);
 
-	del_timer_sync(&priv->restart_timer);
+	cancel_delayed_work_sync(&priv->restart_work);
 	can_flush_echo_skb(dev);
 }
 EXPORT_SYMBOL_GPL(close_candev);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 5261751f6bd4..5f5270941ba0 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -32,6 +32,7 @@ enum can_mode {
  * CAN common private data
  */
 struct can_priv {
+	struct net_device *dev;
 	struct can_device_stats can_stats;
 
 	struct can_bittiming bittiming, data_bittiming;
@@ -47,7 +48,7 @@ struct can_priv {
 	u32 ctrlmode_static;	/* static enabled options for driver/hardware */
 
 	int restart_ms;
-	struct timer_list restart_timer;
+	struct delayed_work restart_work;
 
 	int (*do_set_bittiming)(struct net_device *dev);
 	int (*do_set_data_bittiming)(struct net_device *dev);
-- 
2.9.3


^ permalink raw reply related

* Re: [patch net-next 5/6] switchdev: remove FIB offload infrastructure
From: Jiri Pirko @ 2016-09-22 12:49 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa,
	nikolay, linville, andy, f.fainelli, dsa, jhs, vivien.didelot,
	andrew, ivecera, kaber, john
In-Reply-To: <20160922072519.GB13287@splinter>

Thu, Sep 22, 2016 at 09:25:19AM CEST, idosch@idosch.org wrote:
>On Wed, Sep 21, 2016 at 01:53:13PM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Since this is now taken care of by FIB notifier, remove the code, with
>> all unused dependencies.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>[...]
>
>> -static struct net_device *switchdev_get_dev_by_nhs(struct fib_info *fi)
>> -{
>> -	struct switchdev_attr attr = {
>> -		.id = SWITCHDEV_ATTR_ID_PORT_PARENT_ID,
>> -	};
>> -	struct switchdev_attr prev_attr;
>> -	struct net_device *dev = NULL;
>> -	int nhsel;
>> -
>> -	ASSERT_RTNL();
>> -
>> -	/* For this route, all nexthop devs must be on the same switch. */
>> -
>> -	for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
>> -		const struct fib_nh *nh = &fi->fib_nh[nhsel];
>> -
>> -		if (!nh->nh_dev)
>> -			return NULL;
>> -
>> -		dev = switchdev_get_lowest_dev(nh->nh_dev);
>> -		if (!dev)
>> -			return NULL;
>> -
>> -		attr.orig_dev = dev;
>> -		if (switchdev_port_attr_get(dev, &attr))
>> -			return NULL;
>> -
>> -		if (nhsel > 0 &&
>> -		    !netdev_phys_item_id_same(&prev_attr.u.ppid, &attr.u.ppid))
>> -				return NULL;
>> -
>> -		prev_attr = attr;
>> -	}
>> -
>> -	return dev;
>> -}
>
>[...]
>
>> -int switchdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi,
>> -			   u8 tos, u8 type, u32 nlflags, u32 tb_id)
>> -{
>> -	struct switchdev_obj_ipv4_fib ipv4_fib = {
>> -		.obj.id = SWITCHDEV_OBJ_ID_IPV4_FIB,
>> -		.dst = dst,
>> -		.dst_len = dst_len,
>> -		.fi = fi,
>> -		.tos = tos,
>> -		.type = type,
>> -		.nlflags = nlflags,
>> -		.tb_id = tb_id,
>> -	};
>> -	struct net_device *dev;
>> -	int err = 0;
>> -
>> -	/* Don't offload route if using custom ip rules or if
>> -	 * IPv4 FIB offloading has been disabled completely.
>> -	 */
>> -
>> -#ifdef CONFIG_IP_MULTIPLE_TABLES
>> -	if (fi->fib_net->ipv4.fib_has_custom_rules)
>> -		return 0;
>> -#endif
>> -
>> -	if (fi->fib_net->ipv4.fib_offload_disabled)
>> -		return 0;
>> -
>> -	dev = switchdev_get_dev_by_nhs(fi);
>
>Since this is now removed I believe we should perform this check inside
>the drivers. For mlxsw we can simply iterate over the nexthops and make
>sure each has a RIF.

rocker does not support multiple nexthops. For mlxsw, we can you what
you suggest. Will add it.


>
>> -	if (!dev)
>> -		return 0;
>> -
>> -	ipv4_fib.obj.orig_dev = dev;
>> -	err = switchdev_port_obj_add(dev, &ipv4_fib.obj);
>> -	if (!err)
>> -		fib_info_offload_inc(fi);
>> -
>> -	return err == -EOPNOTSUPP ? 0 : err;
>> -}

^ permalink raw reply

* Re: [PATCH] ipvlan: fix building without netfilter
From: Arnd Bergmann @ 2016-09-22 12:58 UTC (permalink / raw)
  To: David Miller; +Cc: maheshb, dsa, edumazet, netdev, linux-kernel
In-Reply-To: <20160922.083205.2034005377472668654.davem@davemloft.net>

On Thursday, September 22, 2016 8:32:05 AM CEST David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Thu, 22 Sep 2016 11:40:52 +0200
> 
> > The new l3s mode in ipvlan relies on netfilter interfaces, but
> > the ipvlan driver can be configured when CONFIG_NETFILTER is disabled,
> > leading to a build error:
> > 
> > drivers/net/ipvlan/ipvlan.h:132:22: error: 'struct nf_hook_state' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
> > drivers/net/ipvlan/ipvlan_main.c:14:27: error: array type has incomplete element type 'struct nf_hook_ops'
> > ...
> > 
> > This adds a forward declaration for struct nf_hook_state, and hides
> > the newly added l3s code in an #ifdef.
> > 
> > Fixes: 4fbae7d83c98 ("ipvlan: Introduce l3s mode")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> I'm pretty sure I applied a Kconfig patch that added the
> necessary dependency.

Yes, I see the fix cf714ac147e0 ("ipvlan: Fix dependency issue") now, and
can confirm that today's linux-next works without my patch, thanks!

	Arnd

^ permalink raw reply

* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Eric Dumazet @ 2016-09-22 13:03 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <1474542209-28409-1-git-send-email-hannes@stressinduktion.org>

On Thu, 2016-09-22 at 13:03 +0200, Hannes Frederic Sowa wrote:
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/core/net_namespace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 2c2eb1b629b11d..a2ace299f28355 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -758,9 +758,11 @@ static int __init net_ns_init(void)
>  
>  	register_pernet_subsys(&net_ns_ops);
>  
> +	rtnl_lock();
>  	rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, NULL);
>  	rtnl_register(PF_UNSPEC, RTM_GETNSID, rtnl_net_getid, rtnl_net_dumpid,
>  		      NULL);
> +	rtnl_unlock();
>  
>  	return 0;
>  }

Hi Hannes

Why is this needed here, and not in other places ?

Hint : A changelog always help reviewers and future bug hunting.

Thanks.

^ permalink raw reply

* [PATCH net] i40e: disable MSI-X interrupts if we cannot reserve enough vectors
From: Guilherme G. Piccoli @ 2016-09-22 13:03 UTC (permalink / raw)
  To: intel-wired-lan; +Cc: jeffrey.t.kirsher, netdev, gpiccoli

If we fail on allocating enough MSI-X interrupts, we should disable
them since they were previously enabled in this point of code.

Not disabling them can lead to WARN_ON() being triggered and subsequent
failure in enabling MSI as a fallback; the below message was shown without
this patch while we played with interrupt allocation in i40e driver:

[ 21.461346] sysfs: cannot create duplicate filename '/devices/pci0007:00/0007:00:00.0/0007:01:00.3/msi_irqs'
[ 21.461459] ------------[ cut here ]------------
[ 21.461514] WARNING: CPU: 64 PID: 1155 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x88/0xc0

Also, we noticed that without this patch, if we modprobe the module without
enough MSI-X interrupts (triggering the above warning), unload the module
and re-load it again, we got a crash on the system.

Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d0b3a1b..f8ebe78 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -7721,6 +7721,7 @@ static int i40e_init_msix(struct i40e_pf *pf)
 		pf->flags &= ~I40E_FLAG_MSIX_ENABLED;
 		kfree(pf->msix_entries);
 		pf->msix_entries = NULL;
+		pci_disable_msix(pf->pdev);
 		return -ENODEV;
 
 	} else if (v_actual == I40E_MIN_MSIX) {
-- 
2.1.0

^ permalink raw reply related

* [PATCH net 1/2] act_ife: Fix external mac header on encode
From: Yotam Gigi @ 2016-09-22 12:55 UTC (permalink / raw)
  To: jhs, davem, netdev; +Cc: Yotam Gigi
In-Reply-To: <1474548926-22815-1-git-send-email-yotamg@mellanox.com>

On ife encode side, external mac header is copied from the original packet
and may be overridden if the user requests. Before, the mac header copy
was done from memory region that might not be accessible anymore, as
skb_cow_head might free it and copy the packet. This led to random values
in the external mac header once the values were not set by user.

This fix takes the internal mac header from the packet, after the call to
skb_cow_head.

Fixes: ef6980b6becb ("net sched: introduce IFE action")
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
---
 net/sched/act_ife.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 27b19ca..7f71a3d 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -758,8 +758,6 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 		return TC_ACT_SHOT;
 	}
 
-	iethh = eth_hdr(skb);
-
 	err = skb_cow_head(skb, reserve);
 	if (unlikely(err)) {
 		ife->tcf_qstats.drops++;
@@ -768,6 +766,7 @@ static int tcf_ife_encode(struct sk_buff *skb, const struct tc_action *a,
 	}
 
 	__skb_push(skb, total_push);
+	iethh = (struct ethhdr *)(skb->data + hdrm);
 	memcpy(skb->data, iethh, skb->mac_len);
 	skb_reset_mac_header(skb);
 	skboff += skb->mac_len;
-- 
2.4.11

^ permalink raw reply related

* Re: [PATCH 0/2] BQL support and fix for a regression issue
From: Eric Dumazet @ 2016-09-22 13:12 UTC (permalink / raw)
  To: David Miller
  Cc: sunil.kovvuri, netdev, linux-kernel, linux-arm-kernel, sgoutham
In-Reply-To: <20160922.082540.992436214283504080.davem@davemloft.net>

On Thu, 2016-09-22 at 08:25 -0400, David Miller wrote:
> From: sunil.kovvuri@gmail.com
> Date: Thu, 22 Sep 2016 14:35:19 +0530
> 
> > From: Sunil Goutham <sgoutham@cavium.com>
> > 
> > These patches add byte queue limit support and also fixes a regression
> > issue introduced by commit
> > 'net: thunderx: Use netdev's name for naming VF's interrupts'
> 
> The correct way to refer to commits when you fix a bug is
> to use the "Fixes: " tag right before the signoffs and ACKs.
> 
> The correct form is:
> 
> Fixes: $SHA1_ID ("Commit header line.")
> 
> You'll see this in many other netdev commits.

Thanks a lot David for this enforcement, it really helps bug tracking
and backports

Note that the $SHA1_ID should be truncated to 12 first chars.

Refer to Documentation/SubmittingPatches around line 191 for more
details.

^ permalink raw reply

* Re: [PATCH RFC 1/3] xdp: Infrastructure to generalize XDP
From: Jesper Dangaard Brouer @ 2016-09-22 13:14 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Thomas Graf, David S. Miller, Linux Kernel Network Developers,
	Kernel Team, Tariq Toukan, Brenden Blanco, Alexei Starovoitov,
	Eric Dumazet, brouer
In-Reply-To: <20160921215658.2c61ed5e@redhat.com>

On Wed, 21 Sep 2016 21:56:58 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> > > I'm not opposed to running non-BPF code at XDP. I'm against adding
> > > a linked list of hook consumers.  
> 
> I also worry about the performance impact of a linked list.  We should
> simple benchmark it instead of discussing it! ;-)

(Note, there are some stability issue with this RFC patchset, when
removing the xdp program, that I had to workaround/patch)


I've started benchmarking this and I only see added cost of 2.89ns from
these patches, at these crazy speeds it does correspond to -485Kpps.

 I was really expecting to see a higher cost of this approach.

I tested this on two different machines. One was suppose to work with
DDIO, but I could not get DDIO working on that machine (result in max
12.7Mpps drop).  Even-though the mlx5 card does work with DDIO.  Even
removed the mlx5 and used same slot but no luck.   (A side-note: Also
measured a 16ns performance difference between which PCIe slot I'm
using).

The reason I wanted to benchmark this on a DDIO machine is, that I'm
suspecting that the added cost, could be hiding behind the cache miss.

Well, I'm running out-of-time benchmarking this stuff, I must prepare
for my Network Performance Workshop ;-)


(A side-note: my skylake motherboard also had a PCI slot, so I found an
old e1000 NIC in my garage, and it worked!)
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH net-next 0/4] act_mirred: Ingress actions support
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
	Shmulik Ladkani

This patch series implements action mirred 'ingress' actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.

This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.

Shmulik Ladkani (4):
  net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
  net/sched: act_mirred: Refactor detection whether dev needs xmit at
    mac header
  net/sched: tc_mirred: Rename public predicates
    'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
  net/sched: act_mirred: Implement ingress actions

 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c  |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  4 +-
 .../net/ethernet/netronome/nfp/nfp_net_offload.c   |  2 +-
 include/net/tc_act/tc_mirred.h                     |  6 +-
 net/sched/act_mirred.c                             | 80 ++++++++++++++++------
 7 files changed, 69 insertions(+), 29 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
	Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>

From: Shmulik Ladkani <shmulik.ladkani@gmail.com>

'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
egress to the target device (if action is performed at ingress).

Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
the target device.
This allows to decouple the attribute from the action to be taken.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
 include/net/tc_act/tc_mirred.h |  2 +-
 net/sched/act_mirred.c         | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 62770ad..5275158 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -8,7 +8,7 @@ struct tcf_mirred {
 	struct tc_action	common;
 	int			tcfm_eaction;
 	int			tcfm_ifindex;
-	int			tcfm_ok_push;
+	int			tcfm_mac_header_xmit;
 	struct net_device __rcu	*tcfm_dev;
 	struct list_head	tcfm_list;
 };
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 667dc38..7b03b13 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -63,7 +63,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	struct tc_mirred *parm;
 	struct tcf_mirred *m;
 	struct net_device *dev;
-	int ret, ok_push = 0;
+	int ret, mac_header_xmit = 0;
 	bool exists = false;
 
 	if (nla == NULL)
@@ -102,10 +102,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 		case ARPHRD_IPGRE:
 		case ARPHRD_VOID:
 		case ARPHRD_NONE:
-			ok_push = 0;
+			mac_header_xmit = 0;
 			break;
 		default:
-			ok_push = 1;
+			mac_header_xmit = 1;
 			break;
 		}
 	} else {
@@ -136,7 +136,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 			dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
 		dev_hold(dev);
 		rcu_assign_pointer(m->tcfm_dev, dev);
-		m->tcfm_ok_push = ok_push;
+		m->tcfm_mac_header_xmit = mac_header_xmit;
 	}
 
 	if (ret == ACT_P_CREATED) {
@@ -181,7 +181,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		goto out;
 
 	if (!(at & AT_EGRESS)) {
-		if (m->tcfm_ok_push)
+		if (m->tcfm_mac_header_xmit)
 			skb_push_rcsum(skb2, skb->mac_len);
 	}
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
	Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>

From: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Move detection logic that tests whether device expects skb data to point
to mac_header upon xmit into a function.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
 net/sched/act_mirred.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7b03b13..28629d3 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -54,6 +54,20 @@ static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = {
 static int mirred_net_id;
 static struct tc_action_ops act_mirred_ops;
 
+static int dev_is_mac_header_xmit(const struct net_device *dev)
+{
+	switch (dev->type) {
+	case ARPHRD_TUNNEL:
+	case ARPHRD_TUNNEL6:
+	case ARPHRD_SIT:
+	case ARPHRD_IPGRE:
+	case ARPHRD_VOID:
+	case ARPHRD_NONE:
+		return 0;
+	}
+	return 1;
+}
+
 static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 			   struct nlattr *est, struct tc_action **a, int ovr,
 			   int bind)
@@ -95,19 +109,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 				tcf_hash_release(*a, bind);
 			return -ENODEV;
 		}
-		switch (dev->type) {
-		case ARPHRD_TUNNEL:
-		case ARPHRD_TUNNEL6:
-		case ARPHRD_SIT:
-		case ARPHRD_IPGRE:
-		case ARPHRD_VOID:
-		case ARPHRD_NONE:
-			mac_header_xmit = 0;
-			break;
-		default:
-			mac_header_xmit = 1;
-			break;
-		}
+		mac_header_xmit = dev_is_mac_header_xmit(dev);
 	} else {
 		dev = NULL;
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 3/4] net/sched: tc_mirred: Rename public predicates 'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
	Shmulik Ladkani, Hariprasad S, Jeff Kirsher, Saeed Mahameed,
	Jiri Pirko, Ido Schimmel, Jakub Kicinski
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>

From: Shmulik Ladkani <shmulik.ladkani@gmail.com>

These accessors are used in various drivers that support tc offloading,
to detect properties of a given 'tc_action'.

'is_tcf_mirred_redirect' tests that the action is TCA_EGRESS_REDIR.
'is_tcf_mirred_mirror' tests that the action is TCA_EGRESS_MIRROR.

As a prep towards supporting INGRESS redir/mirror, rename these
predicates to reflect their true meaning:
  s/is_tcf_mirred_redirect/is_tcf_mirred_egress_redirect/
  s/is_tcf_mirred_mirror/is_tcf_mirred_egress_mirror/

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Ido Schimmel <idosch@mellanox.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c    | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c        | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c      | 2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c       | 4 +++-
 drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
 include/net/tc_act/tc_mirred.h                       | 4 ++--
 6 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index 49d2deb..52af62e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -113,7 +113,7 @@ static int fill_action_fields(struct adapter *adap,
 		}
 
 		/* Re-direct to specified port in hardware. */
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			struct net_device *n_dev;
 			unsigned int i, index;
 			bool found = false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d76bc1a..ba03da2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8425,7 +8425,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
 		}
 
 		/* Redirect to a VF or a offloaded macvlan */
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			int ifindex = tcf_mirred_ifindex(a);
 
 			err = handle_redirect_action(adapter, ifindex, queue,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 22cfc4a..7301dff 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -383,7 +383,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
 			continue;
 		}
 
-		if (is_tcf_mirred_redirect(a)) {
+		if (is_tcf_mirred_egress_redirect(a)) {
 			int ifindex = tcf_mirred_ifindex(a);
 			struct net_device *out_dev;
 			struct mlx5e_priv *out_priv;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 80f27b5..c5bac29 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1237,8 +1237,10 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	tcf_exts_to_list(cls->exts, &actions);
 	list_for_each_entry(a, &actions, list) {
-		if (!is_tcf_mirred_mirror(a) || protocol != htons(ETH_P_ALL))
+		if (!is_tcf_mirred_egress_mirror(a) ||
+		    protocol != htons(ETH_P_ALL)) {
 			return -ENOTSUPP;
+		}
 
 		err = mlxsw_sp_port_add_cls_matchall_mirror(mlxsw_sp_port, cls,
 							    a, ingress);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
index 43f42f8..2b1fa527 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
@@ -128,7 +128,7 @@ nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
 		if (is_tcf_gact_shot(a))
 			return NN_ACT_TC_DROP;
 
-		if (is_tcf_mirred_redirect(a) &&
+		if (is_tcf_mirred_egress_redirect(a) &&
 		    tcf_mirred_ifindex(a) == nn->netdev->ifindex)
 			return NN_ACT_TC_REDIR;
 	}
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 5275158..7b72c6b 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -14,7 +14,7 @@ struct tcf_mirred {
 };
 #define to_mirred(a) ((struct tcf_mirred *)a)
 
-static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_redirect(const struct tc_action *a)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	if (a->ops && a->ops->type == TCA_ACT_MIRRED)
@@ -23,7 +23,7 @@ static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
 	return false;
 }
 
-static inline bool is_tcf_mirred_mirror(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_mirror(const struct tc_action *a)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	if (a->ops && a->ops->type == TCA_ACT_MIRRED)
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
	Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>

From: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Up until now, 'action mirred' supported only egress actions (either
TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).

This patch implements the corresponding ingress actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.

This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
---
 Was wondering, whether netif_receive_skb or dev_forward_skb should be
 used for the rx bouncing. Used netif_receive_skb as in ifb device.

 net/sched/act_mirred.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 28629d3..942120e 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -33,6 +33,25 @@
 static LIST_HEAD(mirred_list);
 static DEFINE_SPINLOCK(mirred_list_lock);
 
+static bool tcf_mirred_is_act_redirect(int action)
+{
+	return action == TCA_EGRESS_REDIR || action == TCA_INGRESS_REDIR;
+}
+
+static u32 tcf_mirred_act_direction(int action)
+{
+	switch (action) {
+	case TCA_EGRESS_REDIR:
+	case TCA_EGRESS_MIRROR:
+		return AT_EGRESS;
+	case TCA_INGRESS_REDIR:
+	case TCA_INGRESS_MIRROR:
+		return AT_INGRESS;
+	default:
+		BUG();
+	}
+}
+
 static void tcf_mirred_release(struct tc_action *a, int bind)
 {
 	struct tcf_mirred *m = to_mirred(a);
@@ -96,6 +115,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	switch (parm->eaction) {
 	case TCA_EGRESS_MIRROR:
 	case TCA_EGRESS_REDIR:
+	case TCA_INGRESS_REDIR:
+	case TCA_INGRESS_MIRROR:
 		break;
 	default:
 		if (exists)
@@ -157,7 +178,8 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 	struct tcf_mirred *m = to_mirred(a);
 	struct net_device *dev;
 	struct sk_buff *skb2;
-	int retval, err;
+	int retval, err = 0;
+	int mac_len;
 	u32 at;
 
 	tcf_lastuse_update(&m->tcf_tm);
@@ -182,23 +204,37 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 	if (!skb2)
 		goto out;
 
-	if (!(at & AT_EGRESS)) {
-		if (m->tcfm_mac_header_xmit)
+	/* If action's target direction differs than filter's direction,
+	 * and devices expect a mac header on xmit, then mac push/pull is
+	 * needed.
+	 */
+	if (at != tcf_mirred_act_direction(m->tcfm_eaction) &&
+	    m->tcfm_mac_header_xmit) {
+		if (at & AT_EGRESS) {
+			/* caught at egress, act ingress: pull mac */
+			mac_len = skb_network_header(skb) - skb_mac_header(skb);
+			skb_pull_rcsum(skb2, mac_len);
+		} else {
+			/* caught at ingress, act egress: push mac */
 			skb_push_rcsum(skb2, skb->mac_len);
+		}
 	}
 
 	/* mirror is always swallowed */
-	if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+	if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
 		skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
 
 	skb2->skb_iif = skb->dev->ifindex;
 	skb2->dev = dev;
-	err = dev_queue_xmit(skb2);
+	if (tcf_mirred_act_direction(m->tcfm_eaction) & AT_EGRESS)
+		err = dev_queue_xmit(skb2);
+	else
+		netif_receive_skb(skb2);
 
 	if (err) {
 out:
 		qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
-		if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+		if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
 			retval = TC_ACT_SHOT;
 	}
 	rcu_read_unlock();
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC v2 06/12] qedr: Add support for QP verbs
From: Leon Romanovsky @ 2016-09-22 13:32 UTC (permalink / raw)
  To: Amrani, Ram
  Cc: davem@davemloft.net, dledford@redhat.com, Elior, Ariel,
	Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
	linux-rdma@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <SN1PR07MB2207C1EC2C5C86C607D0255AF8F60@SN1PR07MB2207.namprd07.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Wed, Sep 21, 2016 at 02:15:39PM +0000, Amrani, Ram wrote:
> > >  include/uapi/rdma/providers/qedr-abi.h     |   35 +
> >
> > Please be aware that the last version for ABI header files doesn't have "provider"
> > name in directory path (include/uapi/rdma/qedr-abi.h)
>
> OK. Note that I was using https://www.spinics.net/lists/linux-rdma/msg40414.html that dates to 7 days ago and it contains the "providers" folder.
> Can you point to the most recent patch?

http://marc.info/?l=linux-rdma&m=147419583317034&w=2

I'll post new version soon. It will add mthca driver to the list of
moved drivers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Hannes Frederic Sowa @ 2016-09-22 13:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1474549384.23058.94.camel@edumazet-glaptop3.roam.corp.google.com>

On 22.09.2016 15:03, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 13:03 +0200, Hannes Frederic Sowa wrote:
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>>  net/core/net_namespace.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index 2c2eb1b629b11d..a2ace299f28355 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -758,9 +758,11 @@ static int __init net_ns_init(void)
>>  
>>  	register_pernet_subsys(&net_ns_ops);
>>  
>> +	rtnl_lock();
>>  	rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, NULL);
>>  	rtnl_register(PF_UNSPEC, RTM_GETNSID, rtnl_net_getid, rtnl_net_dumpid,
>>  		      NULL);
>> +	rtnl_unlock();
>>  
>>  	return 0;
>>  }
> 
> Hi Hannes
> 
> Why is this needed here, and not in other places ?

I found this during working on the file and actually saw no live issues
(belonged to another series which I just split up).

I don't think it is a big issue but wanted the writes to the
rtnl_msg_handlers array to be strictly serialized. I was working on
adding this to other places, too. Maybe better for net-next even?

Theoretically we would need to add a memory barriers to make sure we
don't publish uninitialized memory into the array if concurrent readers
of the array want to find their function pointers.

> Hint : A changelog always help reviewers and future bug hunting.

I will add that to v2.

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH RFC 1/3] xdp: Infrastructure to generalize XDP
From: Eric Dumazet @ 2016-09-22 14:46 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Tom Herbert, Thomas Graf, David S. Miller,
	Linux Kernel Network Developers, Kernel Team, Tariq Toukan,
	Brenden Blanco, Alexei Starovoitov
In-Reply-To: <20160922151403.24648381@redhat.com>

On Thu, 2016-09-22 at 15:14 +0200, Jesper Dangaard Brouer wrote:
> On Wed, 21 Sep 2016 21:56:58 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> 
> > > > I'm not opposed to running non-BPF code at XDP. I'm against adding
> > > > a linked list of hook consumers.  
> > 
> > I also worry about the performance impact of a linked list.  We should
> > simple benchmark it instead of discussing it! ;-)
> 
> (Note, there are some stability issue with this RFC patchset, when
> removing the xdp program, that I had to workaround/patch)
> 
> 
> I've started benchmarking this and I only see added cost of 2.89ns from
> these patches, at these crazy speeds it does correspond to -485Kpps.

I claim the methodology is too biased.

At full speed, all the extra code is hot in caches, and your core has
full access to memory bus anyway. Even branch predictor has fresh
information.

Now, in a mixed workload, where all cores compete to access L2/L3 and
RAM, things might be very different.

Testing icache/dcache pressure is not a matter of measuring how many
Kpps you add or remove on a hot path.

A latency test, when other cpus are busy reading/writing all over
memory, and your caches are cold, would be useful.

^ permalink raw reply

* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Eric Dumazet @ 2016-09-22 14:48 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <4c1b9aba-e6e8-babc-1bbe-aef2bc0bca53@stressinduktion.org>

On Thu, 2016-09-22 at 15:41 +0200, Hannes Frederic Sowa wrote:

> I found this during working on the file and actually saw no live issues
> (belonged to another series which I just split up).
> 
> I don't think it is a big issue but wanted the writes to the
> rtnl_msg_handlers array to be strictly serialized. I was working on
> adding this to other places, too. Maybe better for net-next even?

Sure, and you probably could enforce an ASSERT_RTNL() in rtnl_register()
to catch all offenders.

^ permalink raw reply

* [PATCH net-next] sfc: check async completer is !NULL before calling
From: Bert Kenward @ 2016-09-22 14:47 UTC (permalink / raw)
  To: Dave Miller; +Cc: netdev, Solarflare Linux Maintainers

Add a NULL check before calling asynchronous MCDI completion functions
during device removal.

Fixes: 7014d7f6 ("sfc: allow asynchronous MCDI without completion function")
Signed-off-by: Bert Kenward <bkenward@solarflare.com>
---
 drivers/net/ethernet/sfc/mcdi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
index 9fbc12a..2415209 100644
--- a/drivers/net/ethernet/sfc/mcdi.c
+++ b/drivers/net/ethernet/sfc/mcdi.c
@@ -1156,7 +1156,8 @@ void efx_mcdi_flush_async(struct efx_nic *efx)
 	 * acquired locks in the wrong order.
 	 */
 	list_for_each_entry_safe(async, next, &mcdi->async_list, list) {
-		async->complete(efx, async->cookie, -ENETDOWN, NULL, 0);
+		if (async->complete)
+			async->complete(efx, async->cookie, -ENETDOWN, NULL, 0);
 		list_del(&async->list);
 		kfree(async);
 	}
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Eric Dumazet @ 2016-09-22 14:54 UTC (permalink / raw)
  To: Shmulik Ladkani
  Cc: David S. Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
	netdev, Shmulik Ladkani
In-Reply-To: <1474550512-7552-5-git-send-email-shmulik.ladkani@gmail.com>

On Thu, 2016-09-22 at 16:21 +0300, Shmulik Ladkani wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> 
> Up until now, 'action mirred' supported only egress actions (either
> TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
> 
> This patch implements the corresponding ingress actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
> 
> This allows attaching filters whose target is to hand matching skbs into
> the rx processing of a specified device.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  Was wondering, whether netif_receive_skb or dev_forward_skb should be
>  used for the rx bouncing. Used netif_receive_skb as in ifb device.

Hmm... we probably need to apply the full rcu protection before this
patch.

https://patchwork.ozlabs.org/patch/667680/

^ permalink raw reply

* Re: [patch net-next 3/6] mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls
From: David Ahern @ 2016-09-22 14:58 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa, nikolay,
	linville, andy, f.fainelli, jhs, vivien.didelot, andrew, ivecera,
	kaber, john
In-Reply-To: <1474458794-5512-4-git-send-email-jiri@resnulli.us>

On 9/21/16 5:53 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Until now, in order to offload a FIB entry to HW we use switchdev op.
> However that has limits. Mainly in case we need to make the HW aware of
> all route prefixes configured in kernel. HW needs to know those in order
> to properly trap appropriate packets and pass the to kernel to do
> the forwarding. Abort mechanism is now handled within the mlxsw driver.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |   9 +-
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 386 ++++++++++++---------
>  .../ethernet/mellanox/mlxsw/spectrum_switchdev.c   |   9 -
>  3 files changed, 216 insertions(+), 188 deletions(-)
> 
> 


Make the move to fib notifiers and the removal of switchdev code separate patches. It will make the fib notifier change easier to follow.

^ 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