Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net V2] vhost_net: correctly check tx avail during rx busy polling
From: David Miller @ 2017-09-05 21:48 UTC (permalink / raw)
  To: jasowang; +Cc: mst, netdev, kvm, virtualization, linux-kernel
In-Reply-To: <1504574525-4711-1-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Tue,  5 Sep 2017 09:22:05 +0800

> We check tx avail through vhost_enable_notify() in the past which is
> wrong since it only checks whether or not guest has filled more
> available buffer since last avail idx synchronization which was just
> done by vhost_vq_avail_empty() before. What we really want is checking
> pending buffers in the avail ring. Fix this by calling
> vhost_vq_avail_empty() instead.
> 
> This issue could be noticed by doing netperf TCP_RR benchmark as
> client from guest (but not host). With this fix, TCP_RR from guest to
> localhost restores from 1375.91 trans per sec to 55235.28 trans per
> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).
> 
> Fixes: 030881372460 ("vhost_net: basic polling support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next] rxrpc: Make service connection lookup always check for retry
From: David Miller @ 2017-09-05 21:41 UTC (permalink / raw)
  To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <150453530801.20359.17059994007095255872.stgit@warthog.procyon.org.uk>

From: David Howells <dhowells@redhat.com>
Date: Mon, 04 Sep 2017 15:28:28 +0100

> When an RxRPC service packet comes in, the target connection is looked up
> by an rb-tree search under RCU and a read-locked seqlock; the seqlock retry
> check is, however, currently skipped if we got a match, but probably
> shouldn't be in case the connection we found gets replaced whilst we're
> doing a search.
> 
> Make the lookup procedure always go through need_seqretry(), even if the
> lookup was successful.  This makes sure we always pick up on a write-lock
> event.
> 
> On the other hand, since we don't take a ref on the object, but rely on RCU
> to prevent its destruction after dropping the seqlock, I'm not sure this is
> necessary.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Applied, thanks David.

^ permalink raw reply

* Re: [PATCH v3 net-next 0/2] Use correct sk->sk_prot for IPV6
From: David Miller @ 2017-09-05 21:38 UTC (permalink / raw)
  To: ilyal; +Cc: netdev, davejwatson, aviadye
In-Reply-To: <1504520041-55634-1-git-send-email-ilyal@mellanox.com>

From: Ilya Lesokhin <ilyal@mellanox.com>
Date: Mon,  4 Sep 2017 13:13:59 +0300

> The tls ulp overrides sk->prot with a new tls specific proto structs.            
> The tls specific structs were previously based on the ipv4 specific              
> tcp_prot sturct.                                                                 
> As a result, attaching the tls ulp to an ipv6 tcp socket replaced                
> some ipv6 callback with the ipv4 equivalents.                                    
>                                                                                  
> This patch adds ipv6 tls proto structs and uses them when                        
> attached to ipv6 sockets. 
> 
> Changed since v2: 
> - Dropped patch to fix IPV6_ADDRFORM setsockopt
> There was some disagreement about the correct way of fixinig it,
> and this series does not depend on it.
> 
> Changes since v1:                                                                
> - TLS now dependes on IPV6                                                       
> This fixes complication issues when TLS is built-in and IPV6 is a module.        
> The downside should be small as it is unlikely that there are kernel TLS         
> users who can't afford to include IPV6 in thier kernel.                          

This is not acceptable.

Forcing such a huge piece of infrastructure like ipv6 to be statically
built just because someone wants to enable TLS is not going to pass.

Every distrubtion out there will enable all features, including TLS,
so effectively you are forcing every distribution to build ipv6
non-modules.

Sorry, you will have to fix this in a way that allows TLS and IPV6
to be modular.

^ permalink raw reply

* Re: [PATCH net-next v7] net: stmmac: Delete dead code for MDIO registration
From: David Miller @ 2017-09-05 21:36 UTC (permalink / raw)
  To: romain.perier
  Cc: peppe.cavallaro, alexandre.torgue, andrew, f.fainelli, netdev,
	linux-kernel
In-Reply-To: <20170904084136.21490-1-romain.perier@collabora.com>

From: Romain Perier <romain.perier@collabora.com>
Date: Mon,  4 Sep 2017 10:41:36 +0200

> This code is no longer used, the logging function was changed by commit
> fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register").
> It was previously showing information about the type of the IRQ, if it's
> polled, ignored or a normal interrupt. As we don't want information loss,
> I have moved this code to phy_attached_print().
> 
> Fixes: fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register")
> Signed-off-by: Romain Perier <romain.perier@collabora.com>

Applied.

^ permalink raw reply

* Re: [net v2] gianfar: Fix Tx flow control deactivation
From: David Miller @ 2017-09-05 21:35 UTC (permalink / raw)
  To: claudiu.manoil; +Cc: netdev
In-Reply-To: <1504511128-25294-1-git-send-email-claudiu.manoil@nxp.com>

From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: Mon, 4 Sep 2017 10:45:28 +0300

> The wrong register is checked for the Tx flow control bit,
> it should have been maccfg1 not maccfg2.
> This went unnoticed for so long probably because the impact is
> hardly visible, not to mention the tangled code from adjust_link().
> First, link flow control (i.e. handling of Rx/Tx link level pause frames)
> is disabled by default (needs to be enabled via 'ethtool -A').
> Secondly, maccfg2 always returns 0 for tx_flow_oldval (except for a few
> old boards), which results in Tx flow control remaining always on
> once activated.
> 
> Fixes: 45b679c9a3ccd9e34f28e6ec677b812a860eb8eb ("gianfar: Implement PAUSE frame generation support")
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>

Please do not put an empty line between Fixes: and other tags.  All tags
are equal and should be in one uninterrupted block of text.

Applied, and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: Ignore MPS_TX_INT_CAUSE[Bubble] for T6
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, leedom
In-Reply-To: <1504504534-22115-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon,  4 Sep 2017 11:25:34 +0530

> MPS_TX_INT_CAUSE[Bubble] is a normal condition for T6, hence
> ignore this interrupt for T6.
> 
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: Fix pause frame count in t4_get_port_stats
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, leedom
In-Reply-To: <1504504056-21956-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon,  4 Sep 2017 11:17:36 +0530

> MPS_STAT_CTL[CountPauseStatTx] and MPS_STAT_CTL[CountPauseStatRx]
> only control whether or not Pause Frames will be counted as part
> of the 64-Byte Tx/Rx Frame counters.  These bits do not control
> whether Pause Frames are counted in the Total Tx/Rx Frames/Bytes
> counters.
> 
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] cxgb4: fix memory leak
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
  To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1504503988-21903-1-git-send-email-ganeshgr@chelsio.com>

From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon,  4 Sep 2017 11:16:28 +0530

> do not reuse the loop counter which is used iterate over
> the ports, so that sched_tbl will be freed for all the ports.
> 
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] tun: rename generic_xdp to skb_xdp
From: David Miller @ 2017-09-05 21:26 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, mst, daniel
In-Reply-To: <1504496169-31190-2-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Mon,  4 Sep 2017 11:36:09 +0800

> Rename "generic_xdp" to "skb_xdp" to avoid confusing it with the
> generic XDP which will be done at netif_receive_skb().
> 
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tun: reserve extra headroom only when XDP is set
From: David Miller @ 2017-09-05 21:26 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, mst, kubakici
In-Reply-To: <1504496169-31190-1-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Mon,  4 Sep 2017 11:36:08 +0800

> We reserve headroom unconditionally which could cause unnecessary
> stress on socket memory accounting because of increased trusesize. Fix
> this by only reserve extra headroom when XDP is set.
> 
> Cc: Jakub Kicinski <kubakici@wp.pl>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: cpsw: Don't handle SIOC[GS]HWTSTAMP when CPTS is disabled
From: Grygorii Strashko @ 2017-09-05 21:25 UTC (permalink / raw)
  To: Richard Cochran, David Miller; +Cc: stefan.sorensen, netdev, linux-omap
In-Reply-To: <20170831074807.3dbbt6qhqxdquipa@localhost>

Hi

On 08/31/2017 02:48 AM, Richard Cochran wrote:
> On Wed, Aug 30, 2017 at 02:47:45PM -0700, David Miller wrote:
>> It should not be required to disable a Kconfig option just to get PHY
>> timestamping to work properly.
> 
> Well, if the MAC driver handles the ioctl and enables time stamping,
> then the PHY driver's time stamping remains disabled.  We don't have a
> way to choose PHY time stamping at run time.
>   
>> Rather, if the CPTS code returns -EOPNOTSUPP we should try to
>> fallthrough to the PHY library based methods.
> 
> I agree that it would be better for the core (rather than the
> individual drivers) to handle this case.

I'd like to clarify one thing here - what is the preferable time-stamping
device: PHY over MAC, or MAC over PHY? 
my understanding it's PHY and ethtool_get_ts_info() seems already implemented this way.

> 
> There are a few callers of .ndo_do_ioctl to consider.  Besides
> dev_ifsioc() there is at least vlan_dev_ioctl() that needs to handle
> the EOPNOTSUPP.

Sry, I've not tried to do solution in Net core, but below patch updates CPSW
driver to selected PHY time-stamping over MAC if supported without disabling CPTS in Kconfig
(at least it's expected to fix it) - not tested as I do not have HW with dp83640 phy.

-----------------------------------------------------------------------
>From 51347692087732320f2f5615030f5f36ed3c7724 Mon Sep 17 00:00:00 2001
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Tue, 5 Sep 2017 15:24:44 -0500
Subject: [PATCH] net: ethernet: cpsw: allow phy timestamping over mac

Allow phy timestamping to be used over mac timestamping if supported.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 21 +++++++++++++++++----
 drivers/net/ethernet/ti/cpts.c |  2 ++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 95ac926..8831cb9 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -291,6 +291,10 @@ struct cpsw_ss_regs {
 #define CPSW_MAX_BLKS_TX_SHIFT		4
 #define CPSW_MAX_BLKS_RX		5
 
+#define HAS_PHY_TXTSTAMP(p) ((p) && (p)->drv && (p)->drv->txtstamp)
+#define HAS_PHY_TSTAMP(p) ((p) && (p)->drv && \
+	((p)->drv->rxtstamp || (p)->drv->rxtstamp))
+
 struct cpsw_host_regs {
 	u32	max_blks;
 	u32	blk_cnt;
@@ -1600,6 +1604,8 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
 	struct cpsw_common *cpsw = priv->cpsw;
+	int slave_no = cpsw_slave_index(cpsw, priv);
+	struct phy_device *phy = cpsw->slaves[slave_no].phy;
 	struct cpts *cpts = cpsw->cpts;
 	struct netdev_queue *txq;
 	struct cpdma_chan *txch;
@@ -1611,8 +1617,9 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 		return NET_XMIT_DROP;
 	}
 
-	if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
-	    cpts_is_tx_enabled(cpts) && cpts_can_timestamp(cpts, skb))
+	if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+	    cpts_can_timestamp(cpts, skb) &&
+	    (cpts_is_tx_enabled(cpts) || HAS_PHY_TXTSTAMP(phy)))
 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 
 	q_idx = skb_get_queue_mapping(skb);
@@ -1810,20 +1817,26 @@ static int cpsw_ndo_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
 	struct cpsw_priv *priv = netdev_priv(dev);
 	struct cpsw_common *cpsw = priv->cpsw;
 	int slave_no = cpsw_slave_index(cpsw, priv);
+	struct phy_device *phy = cpsw->slaves[slave_no].phy;
+
 
 	if (!netif_running(dev))
 		return -EINVAL;
 
 	switch (cmd) {
 	case SIOCSHWTSTAMP:
+		if (HAS_PHY_TSTAMP(phy))
+			break;
 		return cpsw_hwtstamp_set(dev, req);
 	case SIOCGHWTSTAMP:
+		if (HAS_PHY_TSTAMP(phy))
+			break;
 		return cpsw_hwtstamp_get(dev, req);
 	}
 
-	if (!cpsw->slaves[slave_no].phy)
+	if (!phy)
 		return -EOPNOTSUPP;
-	return phy_mii_ioctl(cpsw->slaves[slave_no].phy, req, cmd);
+	return phy_mii_ioctl(phy, req, cmd);
 }
 
 static void cpsw_ndo_tx_timeout(struct net_device *ndev)
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index c2121d2..f257f54 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -421,6 +421,8 @@ void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
 	u64 ns;
 	struct skb_shared_hwtstamps ssh;
 
+	if (!cpts->rx_enable)
+		return;
 	if (!(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
 		return;
 	ns = cpts_find_ts(cpts, skb, CPTS_EV_TX);
-- 
2.10.1


-- 
regards,
-grygorii

^ permalink raw reply related

* Re: [PATCH 6/10] ixgbe: Use ARRAY_SIZE macro
From: David Miller @ 2017-09-05 21:22 UTC (permalink / raw)
  To: joe; +Cc: thomas, netdev, linux-kernel
In-Reply-To: <1504641678.28899.12.camel@perches.com>

From: Joe Perches <joe@perches.com>
Date: Tue, 05 Sep 2017 13:01:18 -0700

> On Tue, 2017-09-05 at 21:45 +0200, Thomas Meyer wrote:
>> On Tue, Sep 05, 2017 at 11:50:44AM -0700, David Miller wrote:
>> > From: Thomas Meyer <thomas@m3y3r.de>
>> > Date: Sun, 03 Sep 2017 14:19:31 +0200
>> > 
>> > > Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
>> > > yourself.
>> > > Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
>> > > 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
>> > > /ARRAY_SIZE(\1)/g' and manual check/verification.
>> > > 
>> > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>> > 
>> > This should be submitted to the Intel ethernet driver maintainers.
>> 
>> Hi,
>> 
>> my script checks the output of get_maintainer scripts and only sends to "open
>> list" entries.
>> 
>> The intel-wired-lan@lists.osuosl.org is moderated, so that's why the patch
>> wasn't send there.
>> 
>> Strangely the lists for nouveau@lists.freedesktop.org and
>> intel-gvt-dev@lists.freedesktop.org appears as open lists in the MAINTAINERS
>> file but seems to be also moderated lists... At least I got some reply that my
>> message awaits approval. Maybe an update to the MAINTAINERS file is missing
>> here?
>> 
>> I may drop above check in my script and send to all mailing lists that
>> get_maintainer.pl will return.
> 
> There's a difference between moderated and subscriber-only
> entries in MAINTAINERS.
> 
> get_maintainers will by default list moderated lists and
> not show subscriber-only lists unless using the -s switch.

Furthermore, nothing prevented you from CC:'ing the maintainer,
Jeff Kirscher.

^ permalink raw reply

* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Tom Herbert @ 2017-09-05 21:13 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
	Linux Netdev List
In-Reply-To: <87fuc1q7en.fsf@stressinduktion.org>

> The situation with encapsulation is even more complicated:
>
> We are basically only interested in the UDP/vxlan/Ethernet/IP/UDP
> constellation. If we do the fragmentation inside the vxlan tunnel and
> carry over the skb hash to all resulting UDP/vxlan packets source ports,
> we are fine and reordering on the receiver NIC won't happen in this
> case. If the fragmentation happens on the outer UDP header, this will
> result in reordering of the inner L2 flow. Unfortunately this depends on
> how the vxlan tunnel was set up, how other devices do that and (I
> believe so) on the kernel version.
>
This really isn't that complicated. The assumption that an IP network
always delivers packets in order is simply wrong. The inventors of
VXLAN must have know full well that when you use IP, packets can and
eventually will be delivered out of order. This isn't just because of
fragmentation, there are many other reasons that packets can be
delivered OOO. This also must have been known when IP/GRE and any
other protocol that carries L2 over IP was invented. If OOO is an
issue for these protocols then they need to be fixed-- this is not a
concern with IP protocol nor the stack.

Tom

^ permalink raw reply

* Re: [PATCH net-next 2/4] ip/options: explicitly provide net ns to __ip_options_echo()
From: Paolo Abeni @ 2017-09-05 21:03 UTC (permalink / raw)
  To: Eric Dumazet, David S. Miller; +Cc: netdev, Eric Dumazet, Hannes Frederic Sowa
In-Reply-To: <1504631927.15310.47.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, 2017-09-05 at 10:18 -0700, Eric Dumazet wrote:
> On Thu, 2017-08-03 at 18:07 +0200, Paolo Abeni wrote:
> > __ip_options_echo() uses the current network namespace, and
> > currently retrives it via skb->dst->dev.
> > 
> > This commit adds an explicit 'net' argument to __ip_options_echo()
> > and update all the call sites to provide it, usually via a simpler
> > sock_net().
> > 
> > After this change, __ip_options_echo() no more needs to access
> > skb->dst and we can drop a couple of hack to preserve such
> > info in the rx path.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> 
> David, Paolo
> 
> This commit (91ed1e666a4ea2e260452a7d7d311ac5ae852cba "ip/options:
> explicitly provide net ns to __ip_options_echo()")
> 
> needs to be backported to linux-4.13 stable version to avoid these kind
> of crashes [1]
> 
> This is because of MSG_PEEK operation, hitting skb_consume_udp() while
> skb is still in receive queue.
> 
> Next read() finding again the skb then can see a NULL skb->dst
> 
> Thanks !
> 
> [1]
> general protection fault: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 3017 Comm: syzkaller446772 Not tainted 4.13.0+ #68
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> task: ffff8801cd0a4380 task.stack: ffff8801cc498000
> RIP: 0010:__ip_options_echo+0xea8/0x1430 net/ipv4/ip_options.c:143
> RSP: 0018:ffff8801cc49f628 EFLAGS: 00010246
> RAX: dffffc0000000000 RBX: ffff8801cc49f928 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000004
> RBP: ffff8801cc49f6b8 R08: ffff8801cc49f936 R09: ffffed0039893f28
> R10: 0000000000000003 R11: ffffed0039893f27 R12: ffff8801cc49f918
> R13: ffff8801ccbcf36c R14: 000000000000000f R15: 0000000000000018
> FS:  0000000000979880(0000) GS:ffff8801db200000(0000)
> knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000200c0ff0 CR3: 00000001cc4ed000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  ip_options_echo include/net/ip.h:574 [inline]
>  ip_cmsg_recv_retopts net/ipv4/ip_sockglue.c:91 [inline]
>  ip_cmsg_recv_offset+0xa17/0x1280 net/ipv4/ip_sockglue.c:207
>  udp_recvmsg+0xe0b/0x1260 net/ipv4/udp.c:1641
>  inet_recvmsg+0x14c/0x5f0 net/ipv4/af_inet.c:793
>  sock_recvmsg_nosec net/socket.c:792 [inline]
>  sock_recvmsg+0xc9/0x110 net/socket.c:799
>  SYSC_recvfrom+0x2dc/0x570 net/socket.c:1788
>  SyS_recvfrom+0x40/0x50 net/socket.c:1760
>  entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x444c89
> RSP: 002b:00007ffd80c788e8 EFLAGS: 00000286 ORIG_RAX: 000000000000002d
> RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 0000000000444c89
> RDX: 0000000000000000 RSI: 0000000020bc0000 RDI: 0000000000000004
> RBP: 0000000000000082 R08: 00000000200c0ff0 R09: 0000000000000010
> R10: 0000000000000000 R11: 0000000000000286 R12: 0000000000402390
> R13: 0000000000402420 R14: 0000000000000000 R15: 0000000000000000
> Code: f6 c1 01 0f 85 a5 01 00 00 48 89 4d b8 e8 31 e9 6b fd 48 8b 4d b8
> 48 b8 00 00 00 00 00 fc ff df 48 83 e1 fe 48 89 ca 48 c1 ea 03 <80> 3c
> 02 00 0f 85 41 02 00 00 48 8b 09 48 b8 00 00 00 00 00 fc 
> RIP: __ip_options_echo+0xea8/0x1430 net/ipv4/ip_options.c:143 RSP:
> ffff8801cc49f628
> ---[ end trace b30d95b284222843 ]---
> Kernel panic - not syncing: Fatal exception

Thank you Eric for the report! 

Darn me, I seriously messed-up with the stateless consume.

I think we can have similar issues pith ipsec/secpath and MSG_PEEK,
even if with less catastropthic outcome.

What about the following, which should cover both cases? (only compile
tested, I'll test it tomorrow morning my time)
---
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d67a8182e5eb..63df75ae70ee 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -885,7 +885,7 @@ void kfree_skb(struct sk_buff *skb);
 void kfree_skb_list(struct sk_buff *segs);
 void skb_tx_error(struct sk_buff *skb);
 void consume_skb(struct sk_buff *skb);
-void consume_stateless_skb(struct sk_buff *skb);
+void __consume_stateless_skb(struct sk_buff *skb);
 void  __kfree_skb(struct sk_buff *skb);
 extern struct kmem_cache *skbuff_head_cache;
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e07556606284..f2411a8744d7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -753,14 +753,11 @@ EXPORT_SYMBOL(consume_skb);
  *	consume_stateless_skb - free an skbuff, assuming it is stateless
  *	@skb: buffer to free
  *
- *	Works like consume_skb(), but this variant assumes that all the head
- *	states have been already dropped.
+ *	Alike consume_skb(), but this variant assumes that all the head
+ *	states have been already dropped and usage count is one
  */
-void consume_stateless_skb(struct sk_buff *skb)
+void __consume_stateless_skb(struct sk_buff *skb)
 {
-	if (!skb_unref(skb))
-		return;
-
 	trace_consume_skb(skb);
 	if (likely(skb->head))
 		skb_release_data(skb);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 62344804baae..979e4d8526ba 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1386,12 +1386,15 @@ void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len)
 		unlock_sock_fast(sk, slow);
 	}
 
+	if (!skb_unref(skb))
+		return;
+
 	/* In the more common cases we cleared the head states previously,
 	 * see __udp_queue_rcv_skb().
 	 */
 	if (unlikely(udp_skb_has_head_state(skb)))
 		skb_release_head_state(skb);
-	consume_stateless_skb(skb);
+	__consume_stateless_skb(skb);
 }
 EXPORT_SYMBOL_GPL(skb_consume_udp);
 

^ permalink raw reply related

* [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Kardonik Michael @ 2017-09-05 20:27 UTC (permalink / raw)
  To: leoyang.li, michael.kardonik, Jeff Kirsher, Jesse Brandeburg,
	Shannon Nelson, Carolyn Wyborny, Don Skidmore, Matthew Vick,
	John Ronciak, Mitch Williams, intel-wired-lan, netdev,
	linux-kernel
  Cc: Matthew Tan

Calls to udelay are not preemtable by userspace so userspace
applications experience a large (~200us) latency when running on core0. 
Instead usleep_range can be used to be more friendly to userspace
since it is preemtable. This is due to udelay using busy-wait loops
while usleep_rang uses hrtimers instead. It is recommended to use
udelay when the delay is <10us since at that precision overhead of
usleep_range hrtimer setup causes issues. However, the replaced calls
are for 50us and 100us so this should not be not an issue.
There is no open bug that this patch is fixing, but we see a good
boost in zero loss performance of specific user space application 
(dpdk l3fwd) when this patch is applied: we get from 32% of 10Gb line 
to 49%.

Signed-off-by: Matthew Tan <matthew.tan_1@nxp.com>
Signed-off-by: Michael Kardonik <michael.kardonik@nxp.com>

---
 drivers/net/ethernet/intel/e1000e/phy.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index de13aea..e318fdc 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -158,7 +158,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
 	 * the lower time out
 	 */
 	for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
-		udelay(50);
+		usleep_range(50, 60);
 		mdic = er32(MDIC);
 		if (mdic & E1000_MDIC_READY)
 			break;
@@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 *data)
 	 * reading duplicate data in the next MDIC transaction.
 	 */
 	if (hw->mac.type == e1000_pch2lan)
-		udelay(100);
+		usleep_range(100, 110);
 
 	return 0;
 }
@@ -222,7 +222,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
 	 * the lower time out
 	 */
 	for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
-		udelay(50);
+		usleep_range(50, 60);
 		mdic = er32(MDIC);
 		if (mdic & E1000_MDIC_READY)
 			break;
@@ -246,7 +246,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32 offset, u16 data)
 	 * reading duplicate data in the next MDIC transaction.
 	 */
 	if (hw->mac.type == e1000_pch2lan)
-		udelay(100);
+		usleep_range(100, 110);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 2/2] b43legacy: fix unitialized reads of ret by initializing the array to zero
From: Michael Büsch @ 2017-09-05 20:18 UTC (permalink / raw)
  To: Colin King
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kalle Valo,
	Larry Finger
In-Reply-To: <20170905181658.23893-1-colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1561 bytes --]

On Tue,  5 Sep 2017 19:16:58 +0100
Colin King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:

> From: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> 
> The u8 char array ret is not being initialized and elements outside
> the range start to end contain just garbage values from the stack.
> This results in a later scan of the array to read potentially
> uninitialized values.  Fix this by initializing the array to zero.
> This seems to have been an issue since the very first commit.
> 
> Detected by CoverityScan CID#139653 ("Uninitialized scalar variable")
> 
> Signed-off-by: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  drivers/net/wireless/broadcom/b43legacy/radio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/b43legacy/radio.c b/drivers/net/wireless/broadcom/b43legacy/radio.c
> index 9501420340a9..eab1c9387846 100644
> --- a/drivers/net/wireless/broadcom/b43legacy/radio.c
> +++ b/drivers/net/wireless/broadcom/b43legacy/radio.c
> @@ -280,7 +280,7 @@ u8 b43legacy_radio_aci_detect(struct b43legacy_wldev *dev, u8 channel)
>  u8 b43legacy_radio_aci_scan(struct b43legacy_wldev *dev)
>  {
>  	struct b43legacy_phy *phy = &dev->phy;
> -	u8 ret[13];
> +	u8 ret[13] = { 0 };
>  	unsigned int channel = phy->channel;
>  	unsigned int i;
>  	unsigned int j;


This fix seems to be correct.
Thanks for finding and fixing the issue.

Reviewed-by: Michael Buesch <m@bues.ch>



-- 
Michael

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 179 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply

* Re: [PATCH 1/2] b43: fix unitialized reads of ret by initializing the array to zero
From: Michael Büsch @ 2017-09-05 20:18 UTC (permalink / raw)
  To: Colin King
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kalle Valo
In-Reply-To: <20170905181550.23839-1-colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1512 bytes --]

On Tue,  5 Sep 2017 19:15:50 +0100
Colin King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:

> From: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> 
> The u8 char array ret is not being initialized and elements outside
> the range start to end contain just garbage values from the stack.
> This results in a later scan of the array to read potentially
> uninitialized values.  Fix this by initializing the array to zero.
> This seems to have been an issue since the very first commit.
> 
> Detected by CoverityScan CID#139652 ("Uninitialized scalar variable")
> 
> Signed-off-by: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  drivers/net/wireless/broadcom/b43/phy_g.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/b43/phy_g.c b/drivers/net/wireless/broadcom/b43/phy_g.c
> index 822dcaa8ace6..f59c02166462 100644
> --- a/drivers/net/wireless/broadcom/b43/phy_g.c
> +++ b/drivers/net/wireless/broadcom/b43/phy_g.c
> @@ -2297,7 +2297,7 @@ static u8 b43_gphy_aci_detect(struct b43_wldev *dev, u8 channel)
>  static u8 b43_gphy_aci_scan(struct b43_wldev *dev)
>  {
>  	struct b43_phy *phy = &dev->phy;
> -	u8 ret[13];
> +	u8 ret[13] = { 0 };
>  	unsigned int channel = phy->channel;
>  	unsigned int i, j, start, end;
>  


This fix seems to be correct.
Thanks for finding and fixing the issue.

Reviewed-by: Michael Buesch <m@bues.ch>


-- 
Michael

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 179 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply

* Re: [PATCH 6/10] ixgbe: Use ARRAY_SIZE macro
From: Joe Perches @ 2017-09-05 20:01 UTC (permalink / raw)
  To: Thomas Meyer, David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20170905194419.pdwrkqdad6tbq7hs@olymp>

On Tue, 2017-09-05 at 21:45 +0200, Thomas Meyer wrote:
> On Tue, Sep 05, 2017 at 11:50:44AM -0700, David Miller wrote:
> > From: Thomas Meyer <thomas@m3y3r.de>
> > Date: Sun, 03 Sep 2017 14:19:31 +0200
> > 
> > > Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> > > yourself.
> > > Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> > > 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> > > /ARRAY_SIZE(\1)/g' and manual check/verification.
> > > 
> > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> > 
> > This should be submitted to the Intel ethernet driver maintainers.
> 
> Hi,
> 
> my script checks the output of get_maintainer scripts and only sends to "open
> list" entries.
> 
> The intel-wired-lan@lists.osuosl.org is moderated, so that's why the patch
> wasn't send there.
> 
> Strangely the lists for nouveau@lists.freedesktop.org and
> intel-gvt-dev@lists.freedesktop.org appears as open lists in the MAINTAINERS
> file but seems to be also moderated lists... At least I got some reply that my
> message awaits approval. Maybe an update to the MAINTAINERS file is missing
> here?
> 
> I may drop above check in my script and send to all mailing lists that
> get_maintainer.pl will return.

There's a difference between moderated and subscriber-only
entries in MAINTAINERS.

get_maintainers will by default list moderated lists and
not show subscriber-only lists unless using the -s switch.

^ permalink raw reply

* [PATCH][next v2] rocker: fix kcalloc parameter order
From: Zahari Doychev @ 2017-09-05 19:49 UTC (permalink / raw)
  To: jiri, netdev; +Cc: Zahari Doychev

The function calls to kcalloc use wrong parameter order and incorrect flags
values. GFP_KERNEL is used instead of flags now and the order is corrected.

The change was done using the following coccinelle script:

@@
expression E1,E2;
type T;
@@

-kcalloc(E1, E2, sizeof(T))
+kcalloc(E2, sizeof(T), GFP_KERNEL)

Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
---
 drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index da4e26b53a52..0653b70723a3 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1177,7 +1177,7 @@ static int ofdpa_group_l2_fan_out(struct ofdpa_port *ofdpa_port,
 	entry->group_id = group_id;
 	entry->group_count = group_count;
 
-	entry->group_ids = kcalloc(flags, group_count, sizeof(u32));
+	entry->group_ids = kcalloc(group_count, sizeof(u32), GFP_KERNEL);
 	if (!entry->group_ids) {
 		kfree(entry);
 		return -ENOMEM;
@@ -1456,7 +1456,7 @@ static int ofdpa_port_vlan_flood_group(struct ofdpa_port *ofdpa_port,
 	int err = 0;
 	int i;
 
-	group_ids = kcalloc(flags, port_count, sizeof(u32));
+	group_ids = kcalloc(port_count, sizeof(u32), GFP_KERNEL);
 	if (!group_ids)
 		return -ENOMEM;
 
-- 
2.13.0

^ permalink raw reply related

* Re: [PATCH 6/10] ixgbe: Use ARRAY_SIZE macro
From: Thomas Meyer @ 2017-09-05 19:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20170905.115044.551456011251065741.davem@davemloft.net>

On Tue, Sep 05, 2017 at 11:50:44AM -0700, David Miller wrote:
> From: Thomas Meyer <thomas@m3y3r.de>
> Date: Sun, 03 Sep 2017 14:19:31 +0200
> 
> > Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> > yourself.
> > Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> > 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> > /ARRAY_SIZE(\1)/g' and manual check/verification.
> > 
> > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> 
> This should be submitted to the Intel ethernet driver maintainers.
Hi,

my script checks the output of get_maintainer scripts and only sends to "open
list" entries.

The intel-wired-lan@lists.osuosl.org is moderated, so that's why the patch
wasn't send there.

Strangely the lists for nouveau@lists.freedesktop.org and
intel-gvt-dev@lists.freedesktop.org appears as open lists in the MAINTAINERS
file but seems to be also moderated lists... At least I got some reply that my
message awaits approval. Maybe an update to the MAINTAINERS file is missing
here?

I may drop above check in my script and send to all mailing lists that
get_maintainer.pl will return.

> 
> Thank you.

^ permalink raw reply

* Re: 915f3e3f76 ("mac80211_hwsim: Replace bogus hrtimer clockid"): BUG: kernel reboot-without-warning in test stage
From: Thomas Gleixner @ 2017-09-05 19:30 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: kernel test robot, LKP, linux-kernel, netdev, linux-wireless, wfg
In-Reply-To: <20170905164103.tkrhxzca4b5ulps2@breakpoint.cc>

On Tue, 5 Sep 2017, Sebastian Andrzej Siewior wrote:

> On 2017-09-05 09:12:40 [+0200], Thomas Gleixner wrote:
> > Sorry, no. That bisect is completely bogus. The commit in question merily
> > replaces the unsupported clockid with a valid one.
> 
> The bisect is correct. It just has problems to express itself properly. So
> the table says:
> 
> | WARNING:at_kernel/time/hrtimer.c:#hrtimer_init  | 7   |     |    |   |                                                                                
> | BUG:kernel_reboot-without-warning_in_test_stage | 0   | 12  | 6  | 6 |                                                                                
> 
> which means _before_ your commit it counted a warning in hrtimer_init()
> (an unsupported clock id was used). With your commit, the warning was
> gone and I *think* the userland then printed
> "BUG:kernel_reboot-without-warning_in_test_stage" because it had no
> warning.
> It seems that the bot learned to live with that warning which was around
> for more than three years. Now that you removed it, it seems to be a
> mistake to do so because nobody complained about it so far.

Thanks for the translation. I'll never learn to decode these reports.

       tglx

^ permalink raw reply

* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Hannes Frederic Sowa @ 2017-09-05 19:20 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
	Linux Netdev List
In-Reply-To: <CALx6S345jR6UtAGju9TaPxAOMgkBG2RZ+eMWz80ix1joSmQCog@mail.gmail.com>

Hi Tom,

Tom Herbert <tom@herbertland.com> writes:

> On Tue, Sep 5, 2017 at 4:14 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> Tom Herbert <tom@herbertland.com> writes:
>>
>>> There is absolutely no requirement in IP that packets are delivered in
>>> order-- there never has been and there never will be! If the ULP, like
>>> Ethernet encapsulation, requires in order deliver then it needs to
>>> implement that itself like TCP, GRE, and other protocols ensure that
>>> with sequence numbers and reassembly. All of these hoops we do make
>>> sure that packets always follow the same path and are always in order
>>> are done for benefit of middlebox devices like stateful firewalls that
>>> have force us to adopt their concept of network architecture-- in the
>>> long run this is self-defeating and kills our ability to innovate.
>>>
>>> I'm not saying that we shouldn't consider legacy devices, but we
>>> should scrutinize new development or solutions that perpetuate
>>> incorrect design or bad assumptions.
>>
>> So configure RSS per port and ensure no fragments are send to those
>> ports. This is possible and rather easy to do. It solves the problem
>> with legacy software and it spreads out packets for your applications.
>>
>> It is not perfect but it is working and solves both problems.
>>
> Hannes,
>
> I don't see how that solves anything. The purpose of RSS is to
> distribute the load of protocol packets across queues. This needs to
> work for UDP applications. For instance, if I were building a QUIC
> server I'd want the sort of flow distribution that a TCP server would
> give. You can't do that by configuring a few ports in the device.

I seriously am with you and I think you are on the right track. I would
also much rather like to see fragmentation *not* happening and would
like to see hardware logic *not* parsing deep down into packets and
protocols. We can agree on that!

Albeit I don't understand the problem with my approach:

If you know that you are running a QUIC server and know your environment
(safe against reordering), run `ethtool -N ethX rx-flow-hash udp4 sdfn'
on the box and you would get spreading of UDP packets based on the UDP
src+dst port numbers to the rx queues. Otherwise only source and
destination addresses are being considered, which is the safe default
(maybe this is sometimes enough).

Intel network cards warn you if you actually switch the mode:

-- >8 --
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c:                                  "enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
drivers/net/ethernet/intel/igb/igb_ethtool.c:                           "enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c:                       e_warn(drv, "enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
-- 8< --

I argue that we can't change the default.

Furthermore I tried to argue that in a cloud where you don't know which
applications you run within the encapsulated networks, it is very hard
to predict which protocols can be considered safe against those
effects. I agree, that most of the time, it probably should not
matter. But given the rule about kernel backwards compatibility I do
have doubts that this change would go unnoticed.

I am the devil's advocate here and argue it is on us to show that is
safe before we change the semantics.

> If I were to suggest any HW change it would be to not do DPI on
> fragments (MF or offset is set). This ensures that all packets of the
> fragment train get hashed to the same queue and is on fact what RPS
> has been doing for years without any complaints.

The same UDP flow could consist out of fragments and non-fragmented
packets. Even if hardware would act according to what you wrote,
reordering is very much possible within the same UDP flow.

In case of QUIC, as far as I know, it uses destination ports 80 and
443. Like we already notify NICs about vxlan and geneve port numbers, we
could selectively enable RSS for UDP ports on those destination port
numbers to tell the NIC it won't receive fragments on those ports and
thus do RSS. The same way the NIC could be notified if reordering on
this port is possible. This could be done for example via a lightweight
setsockopt.

The situation with encapsulation is even more complicated:

We are basically only interested in the UDP/vxlan/Ethernet/IP/UDP
constellation. If we do the fragmentation inside the vxlan tunnel and
carry over the skb hash to all resulting UDP/vxlan packets source ports,
we are fine and reordering on the receiver NIC won't happen in this
case. If the fragmentation happens on the outer UDP header, this will
result in reordering of the inner L2 flow. Unfortunately this depends on
how the vxlan tunnel was set up, how other devices do that and (I
believe so) on the kernel version.

Given the fact that we tell the NICs the receiving port of vxlan tunnels
anyway (and I only can assume) they are doing RSS on that, we probably
have reordering anyway on those tunnels. So maybe you are right and it
doesn't matter in the end for encapsulated packets, by accident.

> But even before I'd make that recommendation, I'd really like
> understand what the problem actually is. The only thing I can garner
> from this discussion and the Intel patch is that when fragments are
> received OOO that is perceived as a problem. But the by the protocol
> specification clearly says this is not a problem. So the questions
> are: who is negatively affected by this? Is this a problem because
> some artificial test that checks for everything to be in order is now
> failing? Is this affecting real users? Is this an issue in the stack
> or really with some implementation outside of the stack? If it is an
> implementation outside of the stack, then are we just bandaid'ing over
> someone else's incorrect implementation by patching the kernel (like
> would have be the case if we change the kernel to interoperate with
> Facebook's switch that couldn't handle OOO in twstate).

I agree with that and I would also love to hear more feedback on
this. Unfortunately I don't know.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCH net-next v2 RESEND 0/4] net: dsa: Allow switch drivers to indicate number of TX queues
From: David Miller @ 2017-09-05 18:53 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <20170904032703.4593-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun,  3 Sep 2017 20:26:59 -0700

> This patch series extracts the parts of the patch set that are likely not to be
> controversial and actually bringing multi-queue support to DSA-created network
> devices.
> 
> With these patches, we can now use sch_multiq as documented under
> Documentation/networking/multique.txt and let applications dedice the switch
> port output queue they want to use. Currently only Broadcom tags utilize that
> information.
> 
> Resending based on David's feedback regarding the patches not in patchwork.
> 
> Changes in v2:
> - use a proper define for the number of TX queues in bcm_sf2.c (Andrew)
> 
> Changes from RFC:
> 
> - dropped the ability to configure RX queues since we don't do anything with
>   those just yet
> - dropped the patches that dealt with binding the DSA slave network devices
>   queues with their master network devices queues this will be worked on
>   separately.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] bridge: switchdev: Use an helper to clear forward mark
From: David Miller @ 2017-09-05 18:51 UTC (permalink / raw)
  To: idosch; +Cc: netdev, stephen, nikolay, yotamg, bridge, mlxsw
In-Reply-To: <20170903144413.21946-1-idosch@mellanox.com>

From: Ido Schimmel <idosch@mellanox.com>
Date: Sun,  3 Sep 2017 17:44:13 +0300

> Instead of using ifdef in the C file.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Suggested-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Tested-by: Yotam Gigi <yotamg@mellanox.com>

Applied.

^ permalink raw reply

* Re: [PATCH 7/10] net/mlx4_core: Use ARRAY_SIZE macro
From: David Miller @ 2017-09-05 18:50 UTC (permalink / raw)
  To: thomas-VsYtu1Qij5c
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1504439110050-1961876957-7-diffsplit-thomas-VsYtu1Qij5c@public.gmane.org>

From: Thomas Meyer <thomas-VsYtu1Qij5c@public.gmane.org>
Date: Sun, 03 Sep 2017 14:19:31 +0200

> Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> yourself.
> Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> /ARRAY_SIZE(\1)/g' and manual check/verification.
> 
> Signed-off-by: Thomas Meyer <thomas-VsYtu1Qij5c@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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