Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 1/2] ip: add helpers to process in-order fragments faster.
From: Peter Oskolkov @ 2018-08-11 20:27 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: Peter Oskolkov, Eric Dumazet, Florian Westphal
In-Reply-To: <20180811202725.242498-1-posk@google.com>

This patch introduces several helper functions/macros that will be
used in the follow-up patch. No runtime changes yet.

The new logic (fully implemented in the second patch) is as follows:

* Nodes in the rb-tree will now contain not single fragments, but lists
  of consecutive fragments ("runs").

* At each point in time, the current "active" run at the tail is
  maintained/tracked. Fragments that arrive in-order, adjacent
  to the previous tail fragment, are added to this tail run without
  triggering the re-balancing of the rb-tree.

* If a fragment arrives out of order with the offset _before_ the tail run,
  it is inserted into the rb-tree as a single fragment.

* If a fragment arrives after the current tail fragment (with a gap),
  it starts a new "tail" run, as is inserted into the rb-tree
  at the end as the head of the new run.

skb->cb is used to store additional information
needed here (suggested by Eric Dumazet).

Reported-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>

---
 include/net/inet_frag.h |  6 ++++
 net/ipv4/ip_fragment.c  | 73 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index b86d14528188..1662cbc0b46b 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -57,7 +57,9 @@ struct frag_v6_compare_key {
  * @lock: spinlock protecting this frag
  * @refcnt: reference count of the queue
  * @fragments: received fragments head
+ * @rb_fragments: received fragments rb-tree root
  * @fragments_tail: received fragments tail
+ * @last_run_head: the head of the last "run". see ip_fragment.c
  * @stamp: timestamp of the last received fragment
  * @len: total length of the original datagram
  * @meat: length of received fragments so far
@@ -78,6 +80,7 @@ struct inet_frag_queue {
 	struct sk_buff		*fragments;  /* Used in IPv6. */
 	struct rb_root		rb_fragments; /* Used in IPv4. */
 	struct sk_buff		*fragments_tail;
+	struct sk_buff		*last_run_head;
 	ktime_t			stamp;
 	int			len;
 	int			meat;
@@ -113,6 +116,9 @@ void inet_frag_kill(struct inet_frag_queue *q);
 void inet_frag_destroy(struct inet_frag_queue *q);
 struct inet_frag_queue *inet_frag_find(struct netns_frags *nf, void *key);
 
+/* Free all skbs in the queue; return the sum of their truesizes. */
+unsigned int inet_frag_rbtree_purge(struct rb_root *root);
+
 static inline void inet_frag_put(struct inet_frag_queue *q)
 {
 	if (refcount_dec_and_test(&q->refcnt))
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 7cb7ed761d8c..26ace9d2d976 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -57,6 +57,57 @@
  */
 static const char ip_frag_cache_name[] = "ip4-frags";
 
+/* Use skb->cb to track consecutive/adjacent fragments coming at
+ * the end of the queue. Nodes in the rb-tree queue will
+ * contain "runs" of one or more adjacent fragments.
+ *
+ * Invariants:
+ * - next_frag is NULL at the tail of a "run";
+ * - the head of a "run" has the sum of all fragment lengths in frag_run_len.
+ */
+struct ipfrag_skb_cb {
+	struct inet_skb_parm	h;
+	struct sk_buff		*next_frag;
+	int			frag_run_len;
+};
+
+#define FRAG_CB(skb)		((struct ipfrag_skb_cb *)((skb)->cb))
+
+static void ip4_frag_init_run(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(struct ipfrag_skb_cb) > sizeof(skb->cb));
+
+	FRAG_CB(skb)->next_frag = NULL;
+	FRAG_CB(skb)->frag_run_len = skb->len;
+}
+
+/* Append skb to the last "run". */
+static void ip4_frag_append_to_last_run(struct inet_frag_queue *q,
+					struct sk_buff *skb)
+{
+	RB_CLEAR_NODE(&skb->rbnode);
+	FRAG_CB(skb)->next_frag = NULL;
+
+	FRAG_CB(q->last_run_head)->frag_run_len += skb->len;
+	FRAG_CB(q->fragments_tail)->next_frag = skb;
+	q->fragments_tail = skb;
+}
+
+/* Create a new "run" with the skb. */
+static void ip4_frag_create_run(struct inet_frag_queue *q, struct sk_buff *skb)
+{
+	if (q->last_run_head)
+		rb_link_node(&skb->rbnode, &q->last_run_head->rbnode,
+			     &q->last_run_head->rbnode.rb_right);
+	else
+		rb_link_node(&skb->rbnode, NULL, &q->rb_fragments.rb_node);
+	rb_insert_color(&skb->rbnode, &q->rb_fragments);
+
+	ip4_frag_init_run(skb);
+	q->fragments_tail = skb;
+	q->last_run_head = skb;
+}
+
 /* Describe an entry in the "incomplete datagrams" queue. */
 struct ipq {
 	struct inet_frag_queue q;
@@ -654,6 +705,28 @@ struct sk_buff *ip_check_defrag(struct net *net, struct sk_buff *skb, u32 user)
 }
 EXPORT_SYMBOL(ip_check_defrag);
 
+unsigned int inet_frag_rbtree_purge(struct rb_root *root)
+{
+	struct rb_node *p = rb_first(root);
+	unsigned int sum = 0;
+
+	while (p) {
+		struct sk_buff *skb = rb_entry(p, struct sk_buff, rbnode);
+
+		p = rb_next(p);
+		rb_erase(&skb->rbnode, root);
+		while (skb) {
+			struct sk_buff *next = FRAG_CB(skb)->next_frag;
+
+			sum += skb->truesize;
+			kfree_skb(skb);
+			skb = next;
+		}
+	}
+	return sum;
+}
+EXPORT_SYMBOL(inet_frag_rbtree_purge);
+
 #ifdef CONFIG_SYSCTL
 static int dist_min;
 
-- 
2.18.0.597.ga71716f1ad-goog

^ permalink raw reply related

* [PATCH net-next v2 0/2] ip: faster in-order IP fragments
From: Peter Oskolkov @ 2018-08-11 20:27 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: Peter Oskolkov

Added "Signed-off-by" in v2.

Peter Oskolkov (2):
  ip: add helpers to process in-order fragments faster.
  ip: process in-order fragments efficiently

 include/net/inet_frag.h  |   6 ++
 net/ipv4/inet_fragment.c |   2 +-
 net/ipv4/ip_fragment.c   | 183 ++++++++++++++++++++++++++++++---------
 3 files changed, 149 insertions(+), 42 deletions(-)

-- 
2.18.0.597.ga71716f1ad-goog

^ permalink raw reply

* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: David Miller @ 2018-08-11 19:06 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: echaudro, netdev, jhs, xiyou.wangcong, jiri, simon.horman
In-Reply-To: <20180809202608.6b816326@cakuba.netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 9 Aug 2018 20:26:08 -0700

> It is not immediately clear why this is needed.  The memory and
> updating two sets of counters won't come for free, so perhaps a
> stronger justification than troubleshooting is due? :S
> 
> Netdev has counters for fallback vs forwarded traffic, so you'd know
> that traffic hits the SW datapath, plus the rules which are in_hw will
> most likely not match as of today for flower (assuming correctness).
> 
> I'm slightly concerned about potential performance impact, would you
> be able to share some numbers for non-trivial number of flows (100k
> active?)?

Agreed, features used for diagnostics cannot have a harmful penalty for
fast path performance.

^ permalink raw reply

* Re: [PATCH v2 net-next 0/2] virtio_net: Expand affinity to arbitrary numbers of cpu and vq
From: David Miller @ 2018-08-11 19:03 UTC (permalink / raw)
  To: caleb.raitto
  Cc: herbert, mst, arei.gonglei, jasowang, netdev, linux-crypto,
	caraitto
In-Reply-To: <20180810011828.199888-1-caleb.raitto@gmail.com>

From: Caleb Raitto <caleb.raitto@gmail.com>
Date: Thu,  9 Aug 2018 18:18:27 -0700

> Virtio-net tries to pin each virtual queue rx and tx interrupt to a cpu if
> there are as many queues as cpus.
> 
> Expand this heuristic to configure a reasonable affinity setting also
> when the number of cpus != the number of virtual queues.
> 
> Patch 1 allows vqs to take an affinity mask with more than 1 cpu.
> Patch 2 generalizes the algorithm in virtnet_set_affinity beyond
> the case where #cpus == #vqs.
> 
> v2 changes:
> Renamed "virtio_net: Make vp_set_vq_affinity() take a mask." to
> "virtio: Make vp_set_vq_affinity() take a mask."
> 
> Tested:
 ...

Yes, this is so much better than the NAPI module parameter thing!

I'll apply this, and we can adjust/tweak the logic if necessary
on top of these changes.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: David Miller @ 2018-08-11 18:54 UTC (permalink / raw)
  To: doronrk; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180809224344.GA48089@doronrk-mbp.dhcp.thefacebook.com>

From: Doron Roberts-Kedes <doronrk@fb.com>
Date: Thu, 9 Aug 2018 15:43:44 -0700

> Taking a step back, is there an existing solution for what this function
> is trying to do? I was surprised to find that there did not seem to
> exist a function for determining the number of scatterlist elements
> required to map an skb without COW. 

The reason is that we usually never need to "map" an SKB on receive,
and on transmit the SKB geometry is normalized by the destination
device features which means no frag_list.

And the other existing receive path doing SW crypto, IPSEC, always
COWs the data so get the number of SG array entries needed from
skb_cow_data().

Frag lists on RX should be pretty rare.  They occur when GRO
segmentation encouters poorly arranged incoming SKBs.  For example
this happens if the incoming frames use lots of tiny SKB page frags,
and therefore prevent accumulation into the page frag array of the
head GRO skb.

So yes it can happen, and we have to account for it.

So I guess you really do need to accomodate this situation.  Why
don't you try to rearrange your new function with some likely()
and unlikely() tests so that the straight code path optimizes for
the non-frag-list case?

Thanks!

^ permalink raw reply

* Re: [PATCH v1 net-next] lan743x: lan743x: Add PTP support
From: David Miller @ 2018-08-11 18:47 UTC (permalink / raw)
  To: Bryan.Whitehead; +Cc: netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1533843370-26315-1-git-send-email-Bryan.Whitehead@microchip.com>

From: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Date: Thu, 9 Aug 2018 15:36:10 -0400

> PTP support includes:
>     Ingress, and egress timestamping.
>     One step timestamping available.
>     PTP clock support.
>     Periodic output support.
> 
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>

Applied to net-next, thank you.

^ permalink raw reply

* Re: [PATCH net-next 1/1] tc: Update README and add config
From: David Miller @ 2018-08-11 19:21 UTC (permalink / raw)
  To: kleib; +Cc: netdev, jhs, xiyou.wangcong, jiri, lucasb
In-Reply-To: <1533910181-17214-1-git-send-email-kleib@mojatatu.com>

From: Keara Leibovitz <kleib@mojatatu.com>
Date: Fri, 10 Aug 2018 10:09:41 -0400

> Updated README.
> 
> Added config file that contains the minimum required features enabled to
> run the tests currently present in the kernel.
> This must be updated when new unittests are created and require their own
> modules.
> 
> Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next 1/2] ip: add helpers to process in-order fragments faster.
From: David Miller @ 2018-08-11 19:22 UTC (permalink / raw)
  To: posk; +Cc: netdev, edumazet, fw
In-Reply-To: <20180810162246.46805-1-posk@google.com>

From: Peter Oskolkov <posk@google.com>
Date: Fri, 10 Aug 2018 16:22:45 +0000

> This patch introduces several helper functions/macros that will be
> used in the follow-up patch. No runtime changes yet.
> 
> The new logic (fully implemented in the second patch) is as follows:
> 
> * Nodes in the rb-tree will now contain not single fragments, but lists
>   of consecutive fragments ("runs").
> 
> * At each point in time, the current "active" run at the tail is
>   maintained/tracked. Fragments that arrive in-order, adjacent
>   to the previous tail fragment, are added to this tail run without
>   triggering the re-balancing of the rb-tree.
> 
> * If a fragment arrives out of order with the offset _before_ the tail run,
>   it is inserted into the rb-tree as a single fragment.
> 
> * If a fragment arrives after the current tail fragment (with a gap),
>   it starts a new "tail" run, as is inserted into the rb-tree
>   at the end as the head of the new run.
> 
> skb->cb is used to store additional information
> needed here (suggested by Eric Dumazet).
> 
> Reported-by: Willem de Bruijn <willemb@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Cc: Florian Westphal <fw@strlen.de>

You need to post these changes with proper Signed-off-by: tags.

^ permalink raw reply

* Re: [PATCH net-next 0/4] new mechanism to ACK immediately
From: David Miller @ 2018-08-11 18:41 UTC (permalink / raw)
  To: ycheng; +Cc: edumazet, netdev, ncardwell, brakmo, weiwan
In-Reply-To: <20180809163812.58365-1-ycheng@google.com>

From: Yuchung Cheng <ycheng@google.com>
Date: Thu,  9 Aug 2018 09:38:08 -0700

> This patch is a follow-up feature improvement to the recent fixes on
> the performance issues in ECN (delayed) ACKs. Many of the fixes use
> tcp_enter_quickack_mode routine to force immediate ACKs. However the
> routine also reset tracking interactive session. This is not ideal
> because these immediate ACKs are required by protocol specifics
> unrelated to the interactiveness nature of the application.
> 
> This patch set introduces a new flag to send a one-time immediate ACK
> without changing the status of interactive session tracking. With this
> patch set the immediate ACKs are generated upon these protocol states:
> 
> 1) When a hole is repaired
> 2) When CE status changes between subsequent data packets received
> 3) When a data packet carries CWR flag

This looks really nice, series applied.

Thanks!

^ permalink raw reply

* Re: [PATCH v1 net-next] lan743x: lan743x: Add PTP support
From: Richard Cochran @ 2018-08-11 19:28 UTC (permalink / raw)
  To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver
In-Reply-To: <1533843370-26315-1-git-send-email-Bryan.Whitehead@microchip.com>

On Thu, Aug 09, 2018 at 03:36:10PM -0400, Bryan Whitehead wrote:
> +static int lan743x_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
> +{
> +	struct lan743x_ptp *ptp =
> +		container_of(ptpci, struct lan743x_ptp, ptp_clock_info);
> +	struct lan743x_adapter *adapter =
> +		container_of(ptp, struct lan743x_adapter, ptp);
> +	u32 lan743x_rate_adj = 0;
> +	bool positive = true;
> +	u64 u64_delta = 0;
> +
> +	if ((scaled_ppm < (-LAN743X_PTP_MAX_FINE_ADJ_IN_SCALED_PPM)) ||
> +	    scaled_ppm > LAN743X_PTP_MAX_FINE_ADJ_IN_SCALED_PPM) {
> +		return -EINVAL;

Maybe ERANGE is better here.

> +	}
> +	if (scaled_ppm > 0) {
> +		u64_delta = (u64)scaled_ppm;
> +		positive = true;
> +	} else {
> +		u64_delta = (u64)(-scaled_ppm);
> +		positive = false;
> +	}
> +	u64_delta = (u64_delta << 19);
> +	lan743x_rate_adj = div_u64(u64_delta, 1000000);
> +
> +	if (positive)
> +		lan743x_rate_adj |= PTP_CLOCK_RATE_ADJ_DIR_;
> +
> +	lan743x_csr_write(adapter, PTP_CLOCK_RATE_ADJ,
> +			  lan743x_rate_adj);
> +
> +	return 0;
> +}
> +
> +static int lan743x_ptpci_adjfreq(struct ptp_clock_info *ptpci, s32 delta_ppb)
> +{

Since you have adjfine, you can delete adjfreq.  The core PTP code
does this in ptp_clock.c:

		if (ops->adjfine)
			err = ops->adjfine(ops, tx->freq);
		else
			err = ops->adjfreq(ops, ppb);

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next 0/8] l2tp: rework pppol2tp ioctl handling
From: David Miller @ 2018-08-11 19:19 UTC (permalink / raw)
  To: g.nault; +Cc: netdev, jchapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 10 Aug 2018 13:21:54 +0200

> The current ioctl() handling code can be simplified. It tests for
> non-relevant conditions and uselessly holds sockets. Once useless
> code is removed, it becomes even simpler to let pppol2tp_ioctl() handle
> commands directly, rather than dispatch them to pppol2tp_tunnel_ioctl()
> or pppol2tp_session_ioctl(). That is the approach taken by this series.
> 
> Patch #1 and #2 define helper functions aimed at simplifying the rest
> of the patch set.
> 
> Patch #3 drops useless tests in pppol2p_ioctl() and avoid holding a
> refcount on the socket.
> 
> Patches #4, #5 and #6 are the core of the series. They let
> pppol2tp_ioctl() handle all ioctls and drop the tunnel and session
> specific functions.
> 
> Then patch #6 brings a little bit of consolidation.
> 
> Finally, patch #7 takes advantage of the simplified code to make
> pppol2tp sockets compatible with dev_ioctl(). Certainly not a killer
> feature, but it is trivial and it is always nice to see l2tp getting
> better integration with the rest of the stack.

Very nice cleanups.

Let's leave the -ENOSYS stuff there for now, changing error return
codes seems to always break something :-/

Series applied, thanks!

^ permalink raw reply

* Re: [PATCH net-next v2 00/15] Remove rtnl lock dependency from all action implementations
From: David Miller @ 2018-08-11 19:38 UTC (permalink / raw)
  To: vladbu
  Cc: netdev, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
	edumazet, keescook, marcelo.leitner
In-Reply-To: <1533923515-5664-1-git-send-email-vladbu@mellanox.com>

From: Vlad Buslov <vladbu@mellanox.com>
Date: Fri, 10 Aug 2018 20:51:40 +0300

> The goal of this change is to update specific actions APIs that access
> action private state directly, in order to be independent from external
> locking. General approach is to re-use existing tcf_lock spinlock (used
> by some action implementation to synchronize control path with data
> path) to protect action private state from concurrent modification. If
> action has rcu-protected pointer, tcf spinlock is used to protect its
> update code, instead of relying on rtnl lock.
> 
> Some actions need to determine rtnl mutex status in order to release it.
> For example, ife action can load additional kernel modules(meta ops) and
> must make sure that no locks are held during module load. In such cases
> 'rtnl_held' argument is used to conditionally release rtnl mutex.
 ...

I like these changes, nice work.  If there are any bugs or whatever, we
can fix them on top.

Series applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH v3 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: David Miller @ 2018-08-11 19:36 UTC (permalink / raw)
  To: khorenko
  Cc: marcelo.leitner, oleg.babin, netdev, linux-sctp, vyasevich,
	nhorman, lucien.xin, aryabinin
In-Reply-To: <20180810171143.21592-1-khorenko@virtuozzo.com>

From: Konstantin Khorenko <khorenko@virtuozzo.com>
Date: Fri, 10 Aug 2018 20:11:41 +0300

> Each SCTP association can have up to 65535 input and output streams.
> For each stream type an array of sctp_stream_in or sctp_stream_out
> structures is allocated using kmalloc_array() function. This function
> allocates physically contiguous memory regions, so this can lead
> to allocation of memory regions of very high order, i.e.:
> 
>   sizeof(struct sctp_stream_out) == 24,
>   ((65535 * 24) / 4096) == 383 memory pages (4096 byte per page),
>   which means 9th memory order.
> 
> This can lead to a memory allocation failures on the systems
> under a memory stress.
> 
> We actually do not need these arrays of memory to be physically
> contiguous. Possible simple solution would be to use kvmalloc()
> instread of kmalloc() as kvmalloc() can allocate physically scattered
> pages if contiguous pages are not available. But the problem
> is that the allocation can happed in a softirq context with
> GFP_ATOMIC flag set, and kvmalloc() cannot be used in this scenario.
> 
> So the other possible solution is to use flexible arrays instead of
> contiguios arrays of memory so that the memory would be allocated
> on a per-page basis.
> 
> This patchset replaces kvmalloc() with flex_array usage.
> It consists of two parts:
> 
>   * First patch is preparatory - it mechanically wraps all direct
>     access to assoc->stream.out[] and assoc->stream.in[] arrays
>     with SCTP_SO() and SCTP_SI() wrappers so that later a direct
>     array access could be easily changed to an access to a
>     flex_array (or any other possible alternative).
>   * Second patch replaces kmalloc_array() with flex_array usage.

Looks good, series applied, thanks!

^ permalink raw reply

* Re: [net-next, PATCH 0/2 v2] netsec driver improvements
From: David Miller @ 2018-08-11 19:11 UTC (permalink / raw)
  To: ilias.apalodimas
  Cc: netdev, jaswinder.singh, ard.biesheuvel, masami.hiramatsu, arnd
In-Reply-To: <1533881559-18589-1-git-send-email-ilias.apalodimas@linaro.org>

From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: Fri, 10 Aug 2018 09:12:37 +0300

> This patchset introduces some improvements on socionext netsec driver. 
>  - patch 1/2, avoids unneeded MMIO reads on the Rx path 
>  - patch 2/2, is adjusting the numbers of descriptors used
> 
> Changes since v1:
>  - Move dma_rmb() to protect descriptor accesses until the device 
>  has updated the NETSEC_RX_PKT_OWN_FIELD bit

Series applied.

^ permalink raw reply

* Re: [PATCH] bonding: avoid repeated display of same link status change
From: David Miller @ 2018-08-11 16:41 UTC (permalink / raw)
  To: rama.nichanamatlu; +Cc: netdev
In-Reply-To: <4d0ff757-ea60-2dae-cc66-45d3fe2dcca0@oracle.com>

From: rama nichanamatlu <rama.nichanamatlu@oracle.com>
Date: Thu, 9 Aug 2018 14:12:57 -0700

> From 9927a1c2a632d9479a80c63b7d9fda59ea8374bc Mon Sep 17 00:00:00 2001
> From: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
> Date: Tue, 31 Jul 2018 07:09:52 -0700
> Subject: [PATCH] bonding: avoid repeated display of same link status
> change
> 
> When link status change needs to be committed and rtnl lock couldn't
> be
> taken, avoid redisplay of same link status change message.
> 
> Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>

Your email client has mis-formatted your patch.

Also, for a boolean set of values you should use 'bool' not 'u8'.

^ permalink raw reply

* Re: [PATCH net-next] vxge: remove set but not used variable 'req_out','status' and 'ret'
From: David Miller @ 2018-08-11 19:06 UTC (permalink / raw)
  To: yuehaibing
  Cc: jdmason, keescook, colin.king, linux-kernel, netdev, gustavo,
	bhelgaas
In-Reply-To: <20180810060837.14756-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 10 Aug 2018 14:08:37 +0800

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/neterion/vxge/vxge-config.c:1097:6: warning:
>  variable 'ret' set but not used [-Wunused-but-set-variable]
> drivers/net/ethernet/neterion/vxge/vxge-config.c:2263:6: warning:
>  variable 'req_out' set but not used [-Wunused-but-set-variable]
> drivers/net/ethernet/neterion/vxge/vxge-config.c:2262:22: warning:
>  variable 'status' set but not used [-Wunused-but-set-variable]
> drivers/net/ethernet/neterion/vxge/vxge-config.c:2360:22: warning:
>  variable 'status' set but not used [-Wunused-but-set-variable]
>   enum vxge_hw_status status = VXGE_HW_OK;
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Stefan Schmidt @ 2018-08-11 18:30 UTC (permalink / raw)
  To: Alan Cox, Jian-Hong Pan
  Cc: mkubecek, Konstantin Böhm, shess, pieter.robyns, contact,
	Xue Liu, Ken Yu, Michael Röder, Rob Herring, lora,
	Alexander Graf, Jan Jongboom, Janus Piwek, Jon Ortego, devicetree,
	Jiri Pirko, Hasnain Virk, Daniele Comel, Marcel Holtmann,
	linux-spi, Mark Brown, Dollar Chen, Brian Ray, linux-arm-kernel,
	Matthias Brugger, Ben Whitten
In-Reply-To: <20180810165711.59bf26f7@alans-desktop>

Hello.

On 08/10/2018 05:57 PM, Alan Cox wrote:
> 
> Long term yes I think Alexander is right the inevitable fate of all
> networks is to become a link layer in order to transmit IP frames 8)

There should be a niche for both at the same time. LoRaWAN is relevant
right now and we should aim for making it possible to run a native Linux
gateway for it.

As for IP for long range low power there is the static context header
compression draft to adapt IPv6 to the characteristics for some use
cases of such networks. Implementing it within the existing 6lwopan
subsystem should be possible.

https://tools.ietf.org/html/draft-ietf-lpwan-ipv6-static-context-hc-16

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH net-next] wimax: usb-tx: mark expected switch fall-through
From: David Miller @ 2018-08-11 18:30 UTC (permalink / raw)
  To: gustavo; +Cc: inaky.perez-gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <20180809154720.GA17920@embeddedor.com>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 9 Aug 2018 10:47:20 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that in this particular case, I placed the "fall through"
> annotation at the bottom of the case, which is what GCC is expecting
> to find.
> 
> Addresses-Coverity-ID: 115075 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] wimax: usb-fw: mark expected switch fall-through
From: David Miller @ 2018-08-11 18:30 UTC (permalink / raw)
  To: gustavo; +Cc: inaky.perez-gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <20180809153944.GA16703@embeddedor.com>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 9 Aug 2018 10:39:44 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that in this particular case, I placed the "fall through"
> annotation at the bottom of the case, which is what GCC is expecting
> to find.
> 
> Addresses-Coverity-ID: 1369529 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: dp83640: Mark expected switch fall-throughs
From: David Miller @ 2018-08-11 18:28 UTC (permalink / raw)
  To: gustavo; +Cc: richardcochran, andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <20180809150824.GA15116@embeddedor.com>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 9 Aug 2018 10:08:24 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Notice that in this particular case, I replaced the code comment at the
> top of the switch statement with a proper "fall through" annotation for
> each case, which is what GCC is expecting to find.
> 
> Addresses-Coverity-ID: 1056542 ("Missing break in switch")
> Addresses-Coverity-ID: 1339579 ("Missing break in switch")
> Addresses-Coverity-ID: 1369526 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] rxrpc: remove redundant static int 'zero'
From: David Miller @ 2018-08-11 18:25 UTC (permalink / raw)
  To: colin.king; +Cc: dhowells, linux-afs, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180809110049.32497-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Thu,  9 Aug 2018 12:00:49 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> The static int 'zero' is defined but is never used hence it is
> redundant and can be removed. The use of this variable was removed
> with commit a158bdd3247b ("rxrpc: Fix call timeouts").
> 
> Cleans up clang warning:
> warning: 'zero' defined but not used [-Wunused-const-variable=]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] drivers/net/usb/r8152: remove the unneeded variable "ret" in rtl8152_system_suspend
From: David Miller @ 2018-08-11 18:23 UTC (permalink / raw)
  To: zhongjiang; +Cc: gustavo, netdev, linux-kernel
In-Reply-To: <1533778753-38089-1-git-send-email-zhongjiang@huawei.com>

From: zhong jiang <zhongjiang@huawei.com>
Date: Thu, 9 Aug 2018 09:39:13 +0800

> rtl8152_system_suspend defines the variable "ret", but it is not modified
> after initialization. So just remove it.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>

Applied.

^ permalink raw reply

* [PATCH 2/2] 9p: Add refcount to p9_req_t
From: Tomas Bortoli @ 2018-08-11 14:42 UTC (permalink / raw)
  To: asmadeus, ericvh, rminnich, lucho
  Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller,
	Tomas Bortoli, Dominique Martinet
In-Reply-To: <20180811144254.23665-1-tomasbortoli@gmail.com>

To avoid use-after-free(s), use a refcount to keep track of the
usable references to any instantiated struct p9_req_t.

This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
wrappers to kref_put(), kref_get() and kref_get_unless_zero().
These are used by the client and the transports to keep track of
valid requests' references.

p9_free_req() is added back and used as callback by kref_put().

Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
kmem_cache_free() will not be reused for another type until the rcu
synchronisation period is over, so an address gotten under rcu read
lock is safe to inc_ref() without corrupting random memory while
the lock is held.

Co-developed-by: Dominique Martinet <dominique.martinet@cea.fr>
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
---
 include/net/9p/client.h | 14 +++++++++++++
 net/9p/client.c         | 54 +++++++++++++++++++++++++++++++++++++++++++------
 net/9p/trans_fd.c       | 11 +++++++++-
 net/9p/trans_rdma.c     |  1 +
 4 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index 735f3979d559..947a570307a6 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -94,6 +94,7 @@ enum p9_req_status_t {
 struct p9_req_t {
 	int status;
 	int t_err;
+	struct kref refcount;
 	wait_queue_head_t wq;
 	struct p9_fcall tc;
 	struct p9_fcall rc;
@@ -233,6 +234,19 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status);
 int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *fl);
 void p9_fcall_fini(struct p9_fcall *fc);
 struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
+
+static inline void p9_req_get(struct p9_req_t *r)
+{
+	kref_get(&r->refcount);
+}
+
+static inline int p9_req_try_get(struct p9_req_t *r)
+{
+	return kref_get_unless_zero(&r->refcount);
+}
+
+int p9_req_put(struct p9_req_t *r);
+
 void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status);
 
 int p9_parse_header(struct p9_fcall *, int32_t *, int8_t *, int16_t *, int);
diff --git a/net/9p/client.c b/net/9p/client.c
index 7942c0bfcc5b..83f2f0aadc14 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -310,6 +310,18 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
 	if (tag < 0)
 		goto free;
 
+	/*	Init ref to two because in the general case there is one ref
+	 *	that is put asynchronously by a writer thread, one ref
+	 *	temporarily given by p9_tag_lookup and put by p9_client_cb
+	 *	in the recv thread, and one ref put by p9_remove_tag in the
+	 *	main thread. The only exception is virtio that does not use
+	 *	p9_tag_lookup but does not have a writer thread either
+	 *	(the write happens synchronously in the request/zc_request
+	 *	callback), so p9_client_cb eats the second ref there
+	 *	as the pointer is duplicated directly by virtqueue_add_sgs()
+	 */
+	refcount_set(&req->refcount.refcount, 2);
+
 	return req;
 
 free:
@@ -333,10 +345,21 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
 	struct p9_req_t *req;
 
 	rcu_read_lock();
+again:
 	req = idr_find(&c->reqs, tag);
-	/* There's no refcount on the req; a malicious server could cause
-	 * us to dereference a NULL pointer
-	 */
+	if (req) {
+		/* We have to be careful with the req found under rcu_read_lock
+		 * Thanks to SLAB_TYPESAFE_BY_RCU we can safely try to get the
+		 * ref again without corrupting other data, then check again
+		 * that the tag matches once we have the ref
+		 */
+		if (!p9_req_try_get(req))
+			goto again;
+		if (req->tc.tag != tag) {
+			p9_req_put(req);
+			goto again;
+		}
+	}
 	rcu_read_unlock();
 
 	return req;
@@ -350,7 +373,7 @@ EXPORT_SYMBOL(p9_tag_lookup);
  *
  * Context: Any context.
  */
-static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
+static int p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
 {
 	unsigned long flags;
 	u16 tag = r->tc.tag;
@@ -359,11 +382,23 @@ static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
 	spin_lock_irqsave(&c->lock, flags);
 	idr_remove(&c->reqs, tag);
 	spin_unlock_irqrestore(&c->lock, flags);
+	return p9_req_put(r);
+}
+
+static void p9_req_free(struct kref *ref)
+{
+	struct p9_req_t *r = container_of(ref, struct p9_req_t, refcount);
 	p9_fcall_fini(&r->tc);
 	p9_fcall_fini(&r->rc);
 	kmem_cache_free(p9_req_cache, r);
 }
 
+int p9_req_put(struct p9_req_t *r)
+{
+	return kref_put(&r->refcount, p9_req_free);
+}
+EXPORT_SYMBOL(p9_req_put);
+
 /**
  * p9_tag_cleanup - cleans up tags structure and reclaims resources
  * @c:  v9fs client struct
@@ -379,7 +414,9 @@ static void p9_tag_cleanup(struct p9_client *c)
 	rcu_read_lock();
 	idr_for_each_entry(&c->reqs, req, id) {
 		pr_info("Tag %d still in use\n", id);
-		p9_tag_remove(c, req);
+		if (p9_tag_remove(c, req) == 0)
+			pr_warn("Packet with tag %d has still references",
+				req->tc.tag);
 	}
 	rcu_read_unlock();
 }
@@ -403,6 +440,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
 
 	wake_up(&req->wq);
 	p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag);
+	p9_req_put(req);
 }
 EXPORT_SYMBOL(p9_client_cb);
 
@@ -682,6 +720,8 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
 	return req;
 reterr:
 	p9_tag_remove(c, req);
+	/* We have to put also the 2nd reference as it won't be used */
+	p9_req_put(req);
 	return ERR_PTR(err);
 }
 
@@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
 
 	err = c->trans_mod->request(c, req);
 	if (err < 0) {
+		/* write won't happen */
+		p9_req_put(req);
 		if (err != -ERESTARTSYS && err != -EFAULT)
 			c->status = Disconnected;
 		goto recalc_sigpending;
@@ -2241,7 +2283,7 @@ EXPORT_SYMBOL(p9_client_readlink);
 
 int __init p9_client_init(void)
 {
-	p9_req_cache = KMEM_CACHE(p9_req_t, 0);
+	p9_req_cache = KMEM_CACHE(p9_req_t, SLAB_TYPESAFE_BY_RCU);
 	return p9_req_cache ? 0 : -ENOMEM;
 }
 
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 20f46f13fe83..686e24e355d0 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -132,6 +132,7 @@ struct p9_conn {
 	struct list_head req_list;
 	struct list_head unsent_req_list;
 	struct p9_req_t *req;
+	struct p9_req_t *wreq;
 	char tmp_buf[7];
 	struct p9_fcall rc;
 	int wpos;
@@ -383,6 +384,7 @@ static void p9_read_work(struct work_struct *work)
 		m->rc.sdata = NULL;
 		m->rc.offset = 0;
 		m->rc.capacity = 0;
+		p9_req_put(m->req);
 		m->req = NULL;
 	}
 
@@ -472,6 +474,8 @@ static void p9_write_work(struct work_struct *work)
 		m->wbuf = req->tc.sdata;
 		m->wsize = req->tc.size;
 		m->wpos = 0;
+		p9_req_get(req);
+		m->wreq = req;
 		spin_unlock(&m->client->lock);
 	}
 
@@ -492,8 +496,11 @@ static void p9_write_work(struct work_struct *work)
 	}
 
 	m->wpos += err;
-	if (m->wpos == m->wsize)
+	if (m->wpos == m->wsize) {
 		m->wpos = m->wsize = 0;
+		p9_req_put(m->wreq);
+		m->wreq = NULL;
+	}
 
 end_clear:
 	clear_bit(Wworksched, &m->wsched);
@@ -694,6 +701,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
 	if (req->status == REQ_STATUS_UNSENT) {
 		list_del(&req->req_list);
 		req->status = REQ_STATUS_FLSHD;
+		p9_req_put(req);
 		ret = 0;
 	}
 	spin_unlock(&client->lock);
@@ -711,6 +719,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
 	spin_lock(&client->lock);
 	list_del(&req->req_list);
 	spin_unlock(&client->lock);
+	p9_req_put(req);
 
 	return 0;
 }
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index c60655c90c9e..8cff368a11e3 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -365,6 +365,7 @@ send_done(struct ib_cq *cq, struct ib_wc *wc)
 			    c->busa, c->req->tc.size,
 			    DMA_TO_DEVICE);
 	up(&rdma->sq_sem);
+	p9_req_put(c->req);
 	kfree(c);
 }
 
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] xen/netfront: don't cache skb_shinfo()
From: David Miller @ 2018-08-11 16:42 UTC (permalink / raw)
  To: jgross; +Cc: linux-kernel, xen-devel, netdev, boris.ostrovsky, stable
In-Reply-To: <20180809144216.18856-1-jgross@suse.com>

From: Juergen Gross <jgross@suse.com>
Date: Thu,  9 Aug 2018 16:42:16 +0200

> skb_shinfo() can change when calling __pskb_pull_tail(): Don't cache
> its return value.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Juergen Gross <jgross@suse.com>

Applied.

^ permalink raw reply

* Re: [PATCH 0/2] net: ethernet: ti: cpsw: fix runtime pm while add/del reserved vid
From: David Miller @ 2018-08-11 16:39 UTC (permalink / raw)
  To: ivan.khoronzhuk; +Cc: grygorii.strashko, linux-omap, netdev, linux-kernel
In-Reply-To: <20180810124709.25089-1-ivan.khoronzhuk@linaro.org>

From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Fri, 10 Aug 2018 15:47:07 +0300

> Here 2 not critical fixes for:
> - vlan ale table leak while error if deleting vlan (simplifies next fix)
> - runtime pm while try to set reserved vlan

Series applied, thank you.

^ 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