Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/6] virtio_net: Add ethtool stat items
From: Michael S. Tsirkin @ 2018-07-25  9:40 UTC (permalink / raw)
  To: Toshiaki Makita
  Cc: Jason Wang, David S. Miller, Toshiaki Makita, netdev,
	virtualization
In-Reply-To: <20180723143609.2242-1-toshiaki.makita1@gmail.com>

On Mon, Jul 23, 2018 at 11:36:03PM +0900, Toshiaki Makita wrote:
> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> 
> Add some ethtool stat items useful for performance analysis.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Series:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Patch 1 seems appropriate for stable, even though it's minor.

> Toshiaki Makita (6):
>   virtio_net: Fix incosistent received bytes counter
>   virtio_net: Use temporary storage for accounting rx stats
>   virtio_net: Make drop counter per-queue
>   virtio_net: Factor out the logic to determine xdp sq
>   virtio_net: Add XDP related stats
>   virtio_net: Add kick stats
> 
>  drivers/net/virtio_net.c | 221 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 158 insertions(+), 63 deletions(-)
> 
> -- 
> 2.14.3

^ permalink raw reply

* UDP GRO without Merging
From: Gauvain Roussel-Tarbouriech @ 2018-07-25  9:53 UTC (permalink / raw)
  To: netdev

Subject:

Hello Netdev,

I am working on WireGuard as part of Google Summer of Code and Jason and
I are working on adding GRO to WireGuard, on the udp_tunnel side of
things. The goal is to inform udp_tunnel’s gro_receive that certain
packets are part of the same flow. Then sometime later, we’d like to
have a list of those same-flow packets appear in gro_complete. As far as
I can tell, the API seems well suited for merging packets in
gro_receive, such that gro_complete only then gets one single merged
packet. However, we need to receive the entire list of same-flow packets
in gro_complete, unmerged. Is this possible with the present APIs?

The goal of this, by the way, is to have more efficient clumping in
multi-core packet decryption, similar to what’s done using GSO on the
send/encryption side. See Jason’s netdevconf presentation for details on
this: https://www.wireguard.com/papers/wireguard-netdev22.pdf (section
“Generic Segmentation Offload Batching”).

Thanks,
-G

^ permalink raw reply

* [PATCH net-next] bnxt_en: combine 'else if' and 'else' into single branche
From: YueHaibing @ 2018-07-25 11:14 UTC (permalink / raw)
  To: davem, michael.chan; +Cc: linux-kernel, netdev, vasundhara-v.volam, YueHaibing

The else-if branch and else branch set mac_ok to true similarly,
so combine the two into single else branch.

Aslo add comments to explain the two conditions, which
from Michael Chan and Vasundhara Volam.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index a649108..f560845 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -956,9 +956,13 @@ static int bnxt_vf_validate_set_mac(struct bnxt *bp, struct bnxt_vf_info *vf)
 	} else if (is_valid_ether_addr(vf->vf_mac_addr)) {
 		if (ether_addr_equal((const u8 *)req->l2_addr, vf->vf_mac_addr))
 			mac_ok = true;
-	} else if (bp->hwrm_spec_code < 0x10202) {
-		mac_ok = true;
 	} else {
+		/* There are two cases:
+		 * 1.If firmware spec < 0x10202,VF MAC address is not forwarded
+		 *   to the PF and so it doesn't have to match
+		 * 2.Allow VF to modify it's own MAC when PF has not assigned a
+		 *   valid MAC address and firmware spec >= 0x10202
+		 */
 		mac_ok = true;
 	}
 	if (mac_ok)
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH net-next v3 5/5] act_mirred: use TC_ACT_REINJECT when possible
From: Paolo Abeni @ 2018-07-25 10:14 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
	Daniel Borkmann, Marcelo Ricardo Leitner, Eyal Birger,
	David Miller
In-Reply-To: <CAM_iQpUiH5gdPp_=GKhP7N8Eu2VVjmGfR6hvXQoaKwyot0JkjQ@mail.gmail.com>

On Tue, 2018-07-24 at 14:15 -0700, Cong Wang wrote:
> On Tue, Jul 24, 2018 at 1:07 PM Paolo Abeni <pabeni@redhat.com> wrote:
> > +
> > +               /* let's the caller reinject the packet, if possible */
> > +               if (skb_at_tc_ingress(skb)) {
> > +                       res->ingress = want_ingress;
> > +                       res->qstats = this_cpu_ptr(m->common.cpu_qstats);
> > +                       return TC_ACT_REINJECT;
> > +               }
> 
> Looks good to me, but here we no longer return user-specified
> return value here, I am sure it is safe for TC_ACT_STOLEN, but
> I am not sure if it is safe for other values, like TC_ACT_RECLASSIFY.

I can make it safer, using the no clone path only if tcf_action is
TC_ACT_STOLEN. That will still cover the relevant use-cases.

Will do that in v4.

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: add End.DT6 action to bpf_lwt_seg6_action helper
From: Mathieu Xhonneux @ 2018-07-25 10:21 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Martin KaFai Lau, netdev, Alexei Starovoitov
In-Reply-To: <3219cdfd-a6b5-8db7-a397-891e6d017d89@iogearbox.net>

Indeed, I missed this one. Thanks, sending a v2.

2018-07-25 5:40 GMT+00:00 Daniel Borkmann <daniel@iogearbox.net>:
> On 07/24/2018 07:14 PM, Martin KaFai Lau wrote:
>> On Tue, Jul 24, 2018 at 04:59:54PM +0000, Mathieu Xhonneux wrote:
>>> The seg6local LWT provides the End.DT6 action, which allows to
>>> decapsulate an outer IPv6 header containing a Segment Routing Header
>>> (SRH), full specification is available here:
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dfilsfils-2Dspring-2Dsrv6-2Dnetwork-2Dprogramming-2D05&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=c61PGnhPMmCUcL5lpyBsxOmsBU2mU5KFY0-Ioo-pBC4&s=mzShtRc5ofzfknAuqoehbGN1ifA17aKihiVLJVfkuZ8&e=
>>>
>>> This patch adds this action now to the seg6local BPF
>>> interface. Since it is not mandatory that the inner IPv6 header also
>>> contains a SRH, seg6_bpf_srh_state has been extended with a pointer to
>>> a possible SRH of the outermost IPv6 header. This helps assessing if the
>>> validation must be triggered or not, and avoids some calls to
>>> ipv6_find_hdr.
>>>
>>> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
> [...]
>>> +
>>>  static int input_action_end_bpf(struct sk_buff *skb,
>>>                              struct seg6_local_lwt *slwt)
>>>  {
>>>      struct seg6_bpf_srh_state *srh_state =
>>>              this_cpu_ptr(&seg6_bpf_srh_states);
>>> -    struct seg6_bpf_srh_state local_srh_state;
>>>      struct ipv6_sr_hdr *srh;
>>> -    int srhoff = 0;
>>>      int ret;
>>>
>>>      srh = get_and_validate_srh(skb);
>>> @@ -478,6 +499,7 @@ static int input_action_end_bpf(struct sk_buff *skb,
>>>       * which is also accessed by the bpf_lwt_seg6_* helpers
>>>       */
>>>      preempt_disable();
>>> +    srh_state->srh = srh;
>>>      srh_state->hdrlen = srh->hdrlen << 3;
>>>      srh_state->valid = 1;
>>>
>>> @@ -486,9 +508,6 @@ static int input_action_end_bpf(struct sk_buff *skb,
>>>      ret = bpf_prog_run_save_cb(slwt->bpf.prog, skb);
>>>      rcu_read_unlock();
>>>
>>> -    local_srh_state = *srh_state;
>>> -    preempt_enable();
>>> -
>>>      switch (ret) {
>>>      case BPF_OK:
>>>      case BPF_REDIRECT:
>>> @@ -500,24 +519,17 @@ static int input_action_end_bpf(struct sk_buff *skb,
>>>              goto drop;
>>>      }
>>>
>>> -    if (unlikely((local_srh_state.hdrlen & 7) != 0))
>>> -            goto drop;
>>> -
>>> -    if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
>>> -            goto drop;
>>> -    srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>>> -    srh->hdrlen = (u8)(local_srh_state.hdrlen >> 3);
>>> -
>>> -    if (!local_srh_state.valid &&
>>> -        unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
>>> +    if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
>>>              goto drop;
>>>
>>> +    preempt_enable();
>>>      if (ret != BPF_REDIRECT)
>>>              seg6_lookup_nexthop(skb, NULL, 0);
>>>
>>>      return dst_input(skb);
>>>
>>>  drop:
>>> +    preempt_enable();
>> For this drop case at the beginning of this function:
>>
>>       srh = get_and_validate_srh(skb);
>>       if (!srh)
>>               goto drop;
>>
>> preempt_disable() was not called yet?
>
> Agree, this is buggy.

^ permalink raw reply

* [PATCH bpf-next v2] bpf: add End.DT6 action to bpf_lwt_seg6_action helper
From: Mathieu Xhonneux @ 2018-07-25 12:36 UTC (permalink / raw)
  To: netdev; +Cc: daniel, alexei.starovoitov

The seg6local LWT provides the End.DT6 action, which allows to
decapsulate an outer IPv6 header containing a Segment Routing Header
(SRH), full specification is available here:

https://tools.ietf.org/html/draft-filsfils-spring-srv6-network-programming-05

This patch adds this action now to the seg6local BPF
interface. Since it is not mandatory that the inner IPv6 header also
contains a SRH, seg6_bpf_srh_state has been extended with a pointer to
a possible SRH of the outermost IPv6 header. This helps assessing if the
validation must be triggered or not, and avoids some calls to
ipv6_find_hdr.

v2: - changed true/false -> 1/0
    - preempt_enable no longer called in first conditional block

Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
---
 include/net/seg6_local.h |  4 ++-
 net/core/filter.c        | 83 +++++++++++++++++++++++++++++++++---------------
 net/ipv6/seg6_local.c    | 48 ++++++++++++++++++----------
 3 files changed, 91 insertions(+), 44 deletions(-)

diff --git a/include/net/seg6_local.h b/include/net/seg6_local.h
index 661fd5b4d3e0..08359e2d8b35 100644
--- a/include/net/seg6_local.h
+++ b/include/net/seg6_local.h
@@ -21,10 +21,12 @@
 
 extern int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
 			       u32 tbl_id);
+extern bool seg6_bpf_has_valid_srh(struct sk_buff *skb);
 
 struct seg6_bpf_srh_state {
-	bool valid;
+	struct ipv6_sr_hdr *srh;
 	u16 hdrlen;
+	bool valid;
 };
 
 DECLARE_PER_CPU(struct seg6_bpf_srh_state, seg6_bpf_srh_states);
diff --git a/net/core/filter.c b/net/core/filter.c
index 104d560946da..2cdea7d05063 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4542,14 +4542,13 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
+	struct ipv6_sr_hdr *srh = srh_state->srh;
 	void *srh_tlvs, *srh_end, *ptr;
-	struct ipv6_sr_hdr *srh;
 	int srhoff = 0;
 
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+	if (srh == NULL)
 		return -EINVAL;
 
-	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 	srh_tlvs = (void *)((char *)srh + ((srh->first_segment + 1) << 4));
 	srh_end = (void *)((char *)srh + sizeof(*srh) + srh_state->hdrlen);
 
@@ -4562,6 +4561,9 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
 
 	if (unlikely(bpf_try_make_writable(skb, offset + len)))
 		return -EFAULT;
+	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+		return -EINVAL;
+	srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 
 	memcpy(skb->data + offset, from, len);
 	return 0;
@@ -4577,52 +4579,79 @@ static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = {
 	.arg4_type	= ARG_CONST_SIZE
 };
 
-BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
-	   u32, action, void *, param, u32, param_len)
+static void bpf_update_srh_state(struct sk_buff *skb)
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
-	struct ipv6_sr_hdr *srh;
 	int srhoff = 0;
-	int err;
-
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
-		return -EINVAL;
-	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
-
-	if (!srh_state->valid) {
-		if (unlikely((srh_state->hdrlen & 7) != 0))
-			return -EBADMSG;
-
-		srh->hdrlen = (u8)(srh_state->hdrlen >> 3);
-		if (unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
-			return -EBADMSG;
 
+	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0) {
+		srh_state->srh = NULL;
+	} else {
+		srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
+		srh_state->hdrlen = srh_state->srh->hdrlen << 3;
 		srh_state->valid = 1;
 	}
+}
+
+BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
+	   u32, action, void *, param, u32, param_len)
+{
+	struct seg6_bpf_srh_state *srh_state =
+		this_cpu_ptr(&seg6_bpf_srh_states);
+	int hdroff = 0;
+	int err;
 
 	switch (action) {
 	case SEG6_LOCAL_ACTION_END_X:
+		if (!seg6_bpf_has_valid_srh(skb))
+			return -EBADMSG;
 		if (param_len != sizeof(struct in6_addr))
 			return -EINVAL;
 		return seg6_lookup_nexthop(skb, (struct in6_addr *)param, 0);
 	case SEG6_LOCAL_ACTION_END_T:
+		if (!seg6_bpf_has_valid_srh(skb))
+			return -EBADMSG;
+		if (param_len != sizeof(int))
+			return -EINVAL;
+		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
+	case SEG6_LOCAL_ACTION_END_DT6:
+		if (!seg6_bpf_has_valid_srh(skb))
+			return -EBADMSG;
 		if (param_len != sizeof(int))
 			return -EINVAL;
+
+		// find inner IPv6 header, pull outer IPv6 header
+		if (ipv6_find_hdr(skb, &hdroff, IPPROTO_IPV6, NULL, NULL) < 0)
+			return -EBADMSG;
+		if (!pskb_pull(skb, hdroff))
+			return -EBADMSG;
+
+		skb_postpull_rcsum(skb, skb_network_header(skb), hdroff);
+		skb_reset_network_header(skb);
+		skb_reset_transport_header(skb);
+		skb->encapsulation = 0;
+
+		bpf_compute_data_pointers(skb);
+		bpf_update_srh_state(skb);
 		return seg6_lookup_nexthop(skb, NULL, *(int *)param);
 	case SEG6_LOCAL_ACTION_END_B6:
+		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
+			return -EBADMSG;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
 					  param, param_len);
 		if (!err)
-			srh_state->hdrlen =
-				((struct ipv6_sr_hdr *)param)->hdrlen << 3;
+			bpf_update_srh_state(skb);
+
 		return err;
 	case SEG6_LOCAL_ACTION_END_B6_ENCAP:
+		if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
+			return -EBADMSG;
 		err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
 					  param, param_len);
 		if (!err)
-			srh_state->hdrlen =
-				((struct ipv6_sr_hdr *)param)->hdrlen << 3;
+			bpf_update_srh_state(skb);
+
 		return err;
 	default:
 		return -EINVAL;
@@ -4644,15 +4673,14 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
+	struct ipv6_sr_hdr *srh = srh_state->srh;
 	void *srh_end, *srh_tlvs, *ptr;
-	struct ipv6_sr_hdr *srh;
 	struct ipv6hdr *hdr;
 	int srhoff = 0;
 	int ret;
 
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+	if (unlikely(srh == NULL))
 		return -EINVAL;
-	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 
 	srh_tlvs = (void *)((unsigned char *)srh + sizeof(*srh) +
 			((srh->first_segment + 1) << 4));
@@ -4682,6 +4710,9 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 	hdr = (struct ipv6hdr *)skb->data;
 	hdr->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 
+	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
+		return -EINVAL;
+	srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 	srh_state->hdrlen += len;
 	srh_state->valid = 0;
 	return 0;
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index e1025b493a18..01f5d6c17f3f 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -459,25 +459,49 @@ static int input_action_end_b6_encap(struct sk_buff *skb,
 
 DEFINE_PER_CPU(struct seg6_bpf_srh_state, seg6_bpf_srh_states);
 
+bool seg6_bpf_has_valid_srh(struct sk_buff *skb)
+{
+	struct seg6_bpf_srh_state *srh_state =
+		this_cpu_ptr(&seg6_bpf_srh_states);
+	struct ipv6_sr_hdr *srh = srh_state->srh;
+
+	if (unlikely(srh == NULL))
+		return 0;
+
+	if (unlikely(!srh_state->valid)) {
+		if ((srh_state->hdrlen & 7) != 0)
+			return 0;
+
+		srh->hdrlen = (u8)(srh_state->hdrlen >> 3);
+		if (!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3))
+			return 0;
+
+		srh_state->valid = 1;
+	}
+
+	return 1;
+}
+
 static int input_action_end_bpf(struct sk_buff *skb,
 				struct seg6_local_lwt *slwt)
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
-	struct seg6_bpf_srh_state local_srh_state;
 	struct ipv6_sr_hdr *srh;
-	int srhoff = 0;
 	int ret;
 
 	srh = get_and_validate_srh(skb);
-	if (!srh)
-		goto drop;
+	if (!srh) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
 	advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
 
 	/* preempt_disable is needed to protect the per-CPU buffer srh_state,
 	 * which is also accessed by the bpf_lwt_seg6_* helpers
 	 */
 	preempt_disable();
+	srh_state->srh = srh;
 	srh_state->hdrlen = srh->hdrlen << 3;
 	srh_state->valid = 1;
 
@@ -486,9 +510,6 @@ static int input_action_end_bpf(struct sk_buff *skb,
 	ret = bpf_prog_run_save_cb(slwt->bpf.prog, skb);
 	rcu_read_unlock();
 
-	local_srh_state = *srh_state;
-	preempt_enable();
-
 	switch (ret) {
 	case BPF_OK:
 	case BPF_REDIRECT:
@@ -500,24 +521,17 @@ static int input_action_end_bpf(struct sk_buff *skb,
 		goto drop;
 	}
 
-	if (unlikely((local_srh_state.hdrlen & 7) != 0))
-		goto drop;
-
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
-		goto drop;
-	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
-	srh->hdrlen = (u8)(local_srh_state.hdrlen >> 3);
-
-	if (!local_srh_state.valid &&
-	    unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
+	if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
 		goto drop;
 
+	preempt_enable();
 	if (ret != BPF_REDIRECT)
 		seg6_lookup_nexthop(skb, NULL, 0);
 
 	return dst_input(skb);
 
 drop:
+	preempt_enable();
 	kfree_skb(skb);
 	return -EINVAL;
 }
-- 
2.16.1

^ permalink raw reply related

* Re: UDP GRO without Merging
From: Steffen Klassert @ 2018-07-25 10:52 UTC (permalink / raw)
  To: Gauvain Roussel-Tarbouriech; +Cc: netdev
In-Reply-To: <03922960-2b6e-181f-041b-aa636b3c9d98@anarchist.pw>

On Wed, Jul 25, 2018 at 11:53:45AM +0200, Gauvain Roussel-Tarbouriech wrote:
> Subject:
> 
> Hello Netdev,
> 
> I am working on WireGuard as part of Google Summer of Code and Jason and
> I are working on adding GRO to WireGuard, on the udp_tunnel side of
> things. The goal is to inform udp_tunnel’s gro_receive that certain
> packets are part of the same flow. Then sometime later, we’d like to
> have a list of those same-flow packets appear in gro_complete. As far as
> I can tell, the API seems well suited for merging packets in
> gro_receive, such that gro_complete only then gets one single merged
> packet. However, we need to receive the entire list of same-flow packets
> in gro_complete, unmerged. Is this possible with the present APIs?

We recently posted a patchset that does this GRO packet chaining.
Not sure if this is exactly what you need, but you can have a look here:

https://www.spinics.net/lists/netdev/msg508093.html

This is implemented as a part of a forward fastpath, but
could be also moved generic code if needed.

^ permalink raw reply

* [net-next v1] net/ipv6: allow any source address for sendmsg pktinfo with ip_nonlocal_bind
From: Vincent Bernat @ 2018-07-25 11:19 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	Tom Herbert
  Cc: Vincent Bernat

When freebind feature is set of an IPv6 socket, any source address can
be used when sending UDP datagrams using IPv6 PKTINFO ancillary
message. Global non-local bind feature was added in commit
35a256fee52c ("ipv6: Nonlocal bind") for IPv6. This commit also allows
IPv6 source address spoofing when non-local bind feature is enabled.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 net/ipv6/datagram.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 201306b9b5ea..c46936563b15 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -800,7 +800,8 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
 
 			if (addr_type != IPV6_ADDR_ANY) {
 				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
-				if (!(inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
+				if (!(net->ipv6.sysctl.ip_nonlocal_bind ||
+				      inet_sk(sk)->freebind || inet_sk(sk)->transparent) &&
 				    !ipv6_chk_addr_and_flags(net, &src_info->ipi6_addr,
 							     dev, !strict, 0,
 							     IFA_F_TENTATIVE) &&
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH RFC/RFT net-next 00/17] net: Convert neighbor tables to per-namespace
From: Eric W. Biederman @ 2018-07-25 12:33 UTC (permalink / raw)
  To: Cong Wang
  Cc: David Ahern, David Miller, Linux Kernel Network Developers,
	nikita.leshchenko, Roopa Prabhu, Stephen Hemminger, Ido Schimmel,
	Jiri Pirko, Saeed Mahameed, Alexander Aring, linux-wpan,
	NetFilter, LKML
In-Reply-To: <CAM_iQpUc-H2kZCBevePs6g-8qKyJkUhdd1ZDSQgYsCG0ChfC2w@mail.gmail.com>

Cong Wang <xiyou.wangcong@gmail.com> writes:

> On Tue, Jul 24, 2018 at 8:14 AM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 7/19/18 11:12 AM, Cong Wang wrote:
>> > On Thu, Jul 19, 2018 at 9:16 AM David Ahern <dsahern@gmail.com> wrote:
>> >>
>> >> Chatting with Nikolay about this and he brought up a good corollary - ip
>> >> fragmentation. It really is a similar problem in that memory is consumed
>> >> as a result of packets received from an external entity. The ipfrag
>> >> sysctls are per namespace with a limit that non-init_net namespaces can
>> >> not set high_thresh > the current value of init_net. Potential memory
>> >> consumed by fragments scales with the number of namespaces which is the
>> >> primary concern with making neighbor tables per namespace.
>> >
>> > Nothing new, already discussed:
>> > https://marc.info/?l=linux-netdev&m=140391416215988&w=2
>> >
>> > :)
>> >
>>
>> Neighbor tables, bridge fdbs, vxlan fdbs and ip fragments all consume
>> local memory resources due to received packets. bridge and vxlan fdb's
>> are fairly straightforward analogs to neighbor entries; they are per
>> device with no limits on the number of entries. Fragments have memory
>> limits per namespace. So neighbor tables are the only ones with this
>> strict limitation and concern on memory consumption.
>>
>> I get the impression there is no longer a strong resistance against
>> moving the tables to per namespace, but deciding what is the right
>> approach to handle backwards compatibility. Correct? Changing the
>> accounting is inevitably going to be noticeable to some use case(s), but
>> with sysctl settings it is a simple runtime update once the user knows
>> to make the change.
>
> This question definitely should go to Eric Biederman who was against
> my proposal.
>
> Let's add Eric into CC.

Given that the entries are per device and the devices are per-namespace,
semantically neighbours are already kept in a per-namespace manner.  So
this is all about making the code not honoring global resource limits.
Making the code not honor gc_thresh3.

Skimming through the code today the default for gc_thresh3 is 1024.
Which means that we limit the neighbour tables to 1024 entries per
protocol type.

There are some pretty compelling reasons especially with ipv4 to keep
the subnet size down.  Arp storms are a real thing.

I don't know off the top of my head what the reasons for limiting the
neighbour table sizes.  I would be much more comfortable with a patchset
like this if we did some research and figured out the reasons why
we have a global limit.  Then changed the code to remove those limits.

When the limits are gone.  When the code can support large subnets
without tuning.  We we don't have to worry about someone scanning an all
addresses in an ipv6 subnet and causing a DOS on working machines.
I think it is completely appropriate to look to see if something per
network namespace needs to happen.

So please let's address the limits, not the fact that some specific
corner case ran into them.

If we are going to neuter gc_thresh3 let's go as far as removing it
entirely.  If we are going to make the neighbour table per something
let's make it per network device.  If we can afford the multiple hash
tables then a hash table per device is better.   Perhaps we want to move
to rhash tables while we look at this, instead of an old hand grown
version of resizable hash table.

Unless I misread something all your patchset did is reshuffle code and
data structures so that gc_thresh3 does not apply accross namespaces.
That does not feel like it really fixes anything.  That just lies to
people.

Further unless I misread something you are increasing the number of
timers to 3 per namespace.  If I create create a thousand network
namespaces that feels like it will hurt system performance overall.

Eric

^ permalink raw reply

* Re: [PATCH v6 10/18] x86/power/64: Remove VLA usage
From: Rafael J. Wysocki @ 2018-07-25 11:32 UTC (permalink / raw)
  To: Kees Cook
  Cc: Herbert Xu, Arnd Bergmann, Eric Biggers, Gustavo A. R. Silva,
	Alasdair Kergon, Rabin Vincent, Tim Chen, Rafael J. Wysocki,
	Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Philipp Reisner, Lars Ellenberg,
	Jens Axboe, Giovanni Cabiddu, Mike Snitzer, Paul Mackerras,
	Greg Kroah-Hartman <gregk
In-Reply-To: <20180724164936.37477-11-keescook@chromium.org>

On Tue, Jul 24, 2018 at 6:49 PM, Kees Cook <keescook@chromium.org> wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> removes the discouraged use of AHASH_REQUEST_ON_STACK by switching to
> shash directly and allocating the descriptor in heap memory (which should
> be fine: the tfm has already been allocated there too).
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>

I think I can queue this up if there are no objections from others.

Do you want me to do that?

> ---
>  arch/x86/power/hibernate_64.c | 36 ++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/power/hibernate_64.c b/arch/x86/power/hibernate_64.c
> index 67ccf64c8bd8..f8e3b668d20b 100644
> --- a/arch/x86/power/hibernate_64.c
> +++ b/arch/x86/power/hibernate_64.c
> @@ -233,29 +233,35 @@ struct restore_data_record {
>   */
>  static int get_e820_md5(struct e820_table *table, void *buf)
>  {
> -       struct scatterlist sg;
> -       struct crypto_ahash *tfm;
> +       struct crypto_shash *tfm;
> +       struct shash_desc *desc;
>         int size;
>         int ret = 0;
>
> -       tfm = crypto_alloc_ahash("md5", 0, CRYPTO_ALG_ASYNC);
> +       tfm = crypto_alloc_shash("md5", 0, 0);
>         if (IS_ERR(tfm))
>                 return -ENOMEM;
>
> -       {
> -               AHASH_REQUEST_ON_STACK(req, tfm);
> -               size = offsetof(struct e820_table, entries) + sizeof(struct e820_entry) * table->nr_entries;
> -               ahash_request_set_tfm(req, tfm);
> -               sg_init_one(&sg, (u8 *)table, size);
> -               ahash_request_set_callback(req, 0, NULL, NULL);
> -               ahash_request_set_crypt(req, &sg, buf, size);
> -
> -               if (crypto_ahash_digest(req))
> -                       ret = -EINVAL;
> -               ahash_request_zero(req);
> +       desc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(tfm),
> +                      GFP_KERNEL);
> +       if (!desc) {
> +               ret = -ENOMEM;
> +               goto free_tfm;
>         }
> -       crypto_free_ahash(tfm);
>
> +       desc->tfm = tfm;
> +       desc->flags = 0;
> +
> +       size = offsetof(struct e820_table, entries) +
> +               sizeof(struct e820_entry) * table->nr_entries;
> +
> +       if (crypto_shash_digest(desc, (u8 *)table, size, buf))
> +               ret = -EINVAL;
> +
> +       kzfree(desc);
> +
> +free_tfm:
> +       crypto_free_shash(tfm);
>         return ret;
>  }
>
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH PATCH net-next 07/18] ceph: fix whitespace
From: Ilya Dryomov @ 2018-07-25 12:59 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
	rminnich-4OHPYypu0djtX7QSmKvirg, lucho-OnYtXJJ0/fesTnJN9+BGXg,
	ralf-6z/3iImG2C8G8FEW9MqTrA, jreuter-K7Hl1MveuGQ,
	pablo-Cap9r6Oaw4JrovVCs/uTlw,
	kadlec-K40Dz/62t/MgiyqX0sVFJYdd74u8MsAO,
	fw-HFFVJYpyMKqzQB+pC5nmwQ, alex.aring-Re5JQEeQqe8AvxtiuMwx3w,
	stefan-OrPQZGeq07wqhVmZOOOmNx2eb7JE58TQ,
	kuznet-v/Mj1YrvjDBInbfyfbPRSQ, yoshfuji-VfPWfsRibaP+Ru+s062T9g,
	johannes-cdvu00un1VgdHxzADdlk8Q, jhs-jkUAjuhPggJWk0Htik3J/w,
	xiyou.wangcong-Re5JQEeQqe8AvxtiuMwx3w,
	jiri-rHqAuBHg3fBzbRFIqnYvSA, Vlad Yasevich,
	nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w,
	trond.myklebust-F/q8l9xzQnoyLce1RVWEUA,
	anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
	steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ, Herbert Xu, netdev,
	v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-hams-u79uwXL29TY76Z2rM5mHXA, Ceph Development,
	linux-decnet-user-5NWGOfrQmneRv+LV9MX5uuRhgaa4a2kL
In-Reply-To: <20180724192918.31165-8-sthemmin-0li6OtcxBFHby3iVrkZq2A@public.gmane.org>

On Tue, Jul 24, 2018 at 9:31 PM Stephen Hemminger
<stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org> wrote:
>
> Remove blank lines at end of file and trailing whitespace.
>
> Signed-off-by: Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>
> ---
>  net/ceph/Kconfig      | 1 -
>  net/ceph/Makefile     | 1 -
>  net/ceph/auth_none.c  | 1 -
>  net/ceph/auth_none.h  | 1 -
>  net/ceph/auth_x.c     | 2 --
>  net/ceph/auth_x.h     | 1 -
>  net/ceph/mon_client.c | 2 +-
>  net/ceph/pagevec.c    | 1 -
>  8 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/net/ceph/Kconfig b/net/ceph/Kconfig
> index f8cceb99e732..cd2d5b9301a1 100644
> --- a/net/ceph/Kconfig
> +++ b/net/ceph/Kconfig
> @@ -41,4 +41,3 @@ config CEPH_LIB_USE_DNS_RESOLVER
>           Documentation/networking/dns_resolver.txt
>
>           If unsure, say N.
> -
> diff --git a/net/ceph/Makefile b/net/ceph/Makefile
> index 12bf49772d24..db09defe27d0 100644
> --- a/net/ceph/Makefile
> +++ b/net/ceph/Makefile
> @@ -15,4 +15,3 @@ libceph-y := ceph_common.o messenger.o msgpool.o buffer.o pagelist.o \
>         auth_x.o \
>         ceph_fs.o ceph_strings.o ceph_hash.o \
>         pagevec.o snapshot.o string_table.o
> -
> diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c
> index 41d2a0c72236..edb7042479ed 100644
> --- a/net/ceph/auth_none.c
> +++ b/net/ceph/auth_none.c
> @@ -142,4 +142,3 @@ int ceph_auth_none_init(struct ceph_auth_client *ac)
>         ac->ops = &ceph_auth_none_ops;
>         return 0;
>  }
> -
> diff --git a/net/ceph/auth_none.h b/net/ceph/auth_none.h
> index 860ed9875791..4158f064302e 100644
> --- a/net/ceph/auth_none.h
> +++ b/net/ceph/auth_none.h
> @@ -26,4 +26,3 @@ struct ceph_auth_none_info {
>  int ceph_auth_none_init(struct ceph_auth_client *ac);
>
>  #endif
> -
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index 2f4a1baf5f52..32c7f5c4b1a6 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -823,5 +823,3 @@ int ceph_x_init(struct ceph_auth_client *ac)
>  out:
>         return ret;
>  }
> -
> -
> diff --git a/net/ceph/auth_x.h b/net/ceph/auth_x.h
> index 454cb54568af..a71c4c282b57 100644
> --- a/net/ceph/auth_x.h
> +++ b/net/ceph/auth_x.h
> @@ -52,4 +52,3 @@ struct ceph_x_info {
>  int ceph_x_init(struct ceph_auth_client *ac);
>
>  #endif
> -
> diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
> index d7a7a2330ef7..18deb3d889c4 100644
> --- a/net/ceph/mon_client.c
> +++ b/net/ceph/mon_client.c
> @@ -1249,7 +1249,7 @@ static void dispatch(struct ceph_connection *con, struct ceph_msg *msg)
>                 if (monc->client->extra_mon_dispatch &&
>                     monc->client->extra_mon_dispatch(monc->client, msg) == 0)
>                         break;
> -
> +
>                 pr_err("received unknown message type %d %s\n", type,
>                        ceph_msg_type_name(type));
>         }
> diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
> index e560d3975f41..d3736f5bffec 100644
> --- a/net/ceph/pagevec.c
> +++ b/net/ceph/pagevec.c
> @@ -197,4 +197,3 @@ void ceph_zero_page_vector_range(int off, int len, struct page **pages)
>         }
>  }
>  EXPORT_SYMBOL(ceph_zero_page_vector_range);
> -

Applied.

Thanks,

                Ilya

^ permalink raw reply

* Re: [PATCH net-next v3 5/5] act_mirred: use TC_ACT_REINJECT when possible
From: Jamal Hadi Salim @ 2018-07-25 11:50 UTC (permalink / raw)
  To: Cong Wang, Paolo Abeni
  Cc: Linux Kernel Network Developers, Jiri Pirko, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David Miller
In-Reply-To: <CAM_iQpUiH5gdPp_=GKhP7N8Eu2VVjmGfR6hvXQoaKwyot0JkjQ@mail.gmail.com>

On 24/07/18 05:15 PM, Cong Wang wrote:
> On Tue, Jul 24, 2018 at 1:07 PM Paolo Abeni <pabeni@redhat.com> wrote:
>> +
>> +               /* let's the caller reinject the packet, if possible */
>> +               if (skb_at_tc_ingress(skb)) {
>> +                       res->ingress = want_ingress;
>> +                       res->qstats = this_cpu_ptr(m->common.cpu_qstats);
>> +                       return TC_ACT_REINJECT;
>> +               }
> 
> Looks good to me, but here we no longer return user-specified
> return value here, I am sure it is safe for TC_ACT_STOLEN, but
> I am not sure if it is safe for other values, like TC_ACT_RECLASSIFY.
> 
> Jamal, is there any use case of returning !TC_ACT_STOLEN for
> ingress redirections?

I cant think of one off top of my head.
There maybe a future use case where it is not so - maybe just allow
to return the user programmed action? that value will always be
TC_ACT_STOLEN if the rule was specified via iproute2/tc.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH net-next v3 1/5] tc/act: user space can't use TC_ACT_REDIRECT directly
From: Jamal Hadi Salim @ 2018-07-25 11:55 UTC (permalink / raw)
  To: Paolo Abeni, netdev
  Cc: Cong Wang, Jiri Pirko, Daniel Borkmann, Marcelo Ricardo Leitner,
	Eyal Birger, David S. Miller
In-Reply-To: <82c5852909788fef3c7b5c29d6ad8c90b60c7170.1532437050.git.pabeni@redhat.com>

On 24/07/18 04:06 PM, Paolo Abeni wrote:

> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -895,6 +895,11 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>   		}
>   	}
>   
> +	if (a->tcfa_action == TC_ACT_REDIRECT) {
> +		net_warn_ratelimited("TC_ACT_REDIRECT can't be used directly");
> +		a->tcfa_action = TC_ACT_UNSPEC;
> +	}
> +

Why not just reject the rule instead of changing the code underneath the
user?

cheers,
jamal

^ permalink raw reply

* [PATCH net-next] tcp: add SNMP counter for the number of packets pruned from ofo queue
From: Yafang Shao @ 2018-07-25 13:06 UTC (permalink / raw)
  To: davem, edumazet; +Cc: netdev, linux-kernel, Yafang Shao

LINUX_MIB_OFOPRUNED is used to count how many times ofo queue is pruned,
but sometimes we want to know how many packets are pruned from this queue,
that could help us to track the dropped packets.

As LINUX_MIB_OFOPRUNED is a useful event for us, so I introduce a new
SNMP counter LINUX_MIB_OFOPRUNEDROP, which could be showed in netstat as
OfoPruneDrop.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/uapi/linux/snmp.h | 1 +
 net/ipv4/proc.c           | 1 +
 net/ipv4/tcp_input.c      | 1 +
 3 files changed, 3 insertions(+)

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index e5ebc83..c996fba 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -172,6 +172,7 @@ enum
 	LINUX_MIB_PRUNECALLED,			/* PruneCalled */
 	LINUX_MIB_RCVPRUNED,			/* RcvPruned */
 	LINUX_MIB_OFOPRUNED,			/* OfoPruned */
+	LINUX_MIB_OFOPRUNEDROP,			/* OfoPruneDrop */
 	LINUX_MIB_OUTOFWINDOWICMPS,		/* OutOfWindowIcmps */
 	LINUX_MIB_LOCKDROPPEDICMPS,		/* LockDroppedIcmps */
 	LINUX_MIB_ARPFILTER,			/* ArpFilter */
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index b46e4cf..c718295 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -180,6 +180,7 @@ static int sockstat_seq_show(struct seq_file *seq, void *v)
 	SNMP_MIB_ITEM("PruneCalled", LINUX_MIB_PRUNECALLED),
 	SNMP_MIB_ITEM("RcvPruned", LINUX_MIB_RCVPRUNED),
 	SNMP_MIB_ITEM("OfoPruned", LINUX_MIB_OFOPRUNED),
+	SNMP_MIB_ITEM("OfoPruneDrop", LINUX_MIB_OFOPRUNEDROP),
 	SNMP_MIB_ITEM("OutOfWindowIcmps", LINUX_MIB_OUTOFWINDOWICMPS),
 	SNMP_MIB_ITEM("LockDroppedIcmps", LINUX_MIB_LOCKDROPPEDICMPS),
 	SNMP_MIB_ITEM("ArpFilter", LINUX_MIB_ARPFILTER),
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 91dbb9a..5267121 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4980,6 +4980,7 @@ static bool tcp_prune_ofo_queue(struct sock *sk)
 	do {
 		prev = rb_prev(node);
 		rb_erase(node, &tp->out_of_order_queue);
+		NET_INC_STATS(sock_net(sk), LINUX_MIB_OFOPRUNEDROP);
 		tcp_drop(sk, rb_to_skb(node));
 		sk_mem_reclaim(sk);
 		if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next v3 0/5] TC: refactor act_mirred packets re-injection
From: Jiri Pirko @ 2018-07-25 11:53 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David S. Miller
In-Reply-To: <cover.1532437050.git.pabeni@redhat.com>

Tue, Jul 24, 2018 at 10:06:38PM CEST, pabeni@redhat.com wrote:
>This series is aimed at improving the act_mirred redirect performances.
>Such action is used by OVS to represent TC S/W flows, and it's current largest
>bottle-neck is the need for a skb_clone() for each packet.
>
>The first 3 patches introduce some cleanup and safeguards to allow extending 
>tca_result - we will use it to store RCU protected redirect information - and
>introduce a clear separation between user-space accessible tcfa_action
>value and internal values accessible only by the kernel.
>Then a new tcfa_action value is introduced: TC_ACT_REINJECT, similar to
>TC_ACT_REDIRECT, but preserving the mirred semantic. Such value is not
>accessible from user-space.
>The last patch exploits the newly introduced infrastructure in the act_mirred
>action, to avoid a skb_clone, when possible.
>
>Overall this the above gives a ~10% performance improvement in forwarding tput,
>when using the TC S/W datapath.
>
>v1 -> v2:
> - preserve the rcu lock in act_bpf
> - add and use a new action value to reinject the packets, preserving the mirred
>   semantic
>
>v2 -> v3:
> - renamed to new action as TC_ACT_REINJECT
> - TC_ACT_REINJECT is not exposed to user-space
>
>Paolo Abeni (5):
>  tc/act: user space can't use TC_ACT_REDIRECT directly
>  net/sched: user-space can't set unknown tcfa_action values
>  tc/act: remove unneeded RCU lock in action callback
>  net/tc: introduce TC_ACT_REINJECT.
>  act_mirred: use TC_ACT_REINJECT when possible

Could you please send the userspace iproute2 patch and list some
examples of usage along with the next patchset version?

Also, I would like you to do selftest script to test this and have it as
a part of this patchset.

Thanks!


>
> include/net/act_api.h        |  2 +-
> include/net/pkt_cls.h        |  3 +++
> include/net/sch_generic.h    | 21 ++++++++++++++++++++
> include/uapi/linux/pkt_cls.h |  6 ++++--
> net/core/dev.c               |  6 +++++-
> net/sched/act_api.c          | 15 +++++++++++++-
> net/sched/act_csum.c         | 12 +++---------
> net/sched/act_ife.c          |  5 +----
> net/sched/act_mirred.c       | 38 ++++++++++++++++++++++++------------
> net/sched/act_sample.c       |  4 +---
> net/sched/act_skbedit.c      | 10 +++-------
> net/sched/act_skbmod.c       | 21 +++++++++-----------
> net/sched/act_tunnel_key.c   |  6 +-----
> net/sched/act_vlan.c         | 19 +++++++-----------
> 14 files changed, 98 insertions(+), 70 deletions(-)
>
>-- 
>2.17.1
>

^ permalink raw reply

* Re: [PATCH net-next v3 1/5] tc/act: user space can't use TC_ACT_REDIRECT directly
From: Jiri Pirko @ 2018-07-25 11:56 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David S. Miller
In-Reply-To: <82c5852909788fef3c7b5c29d6ad8c90b60c7170.1532437050.git.pabeni@redhat.com>

Tue, Jul 24, 2018 at 10:06:39PM CEST, pabeni@redhat.com wrote:
>Only cls_bpf and act_bpf can safely use such value. If a generic
>action is configured by user space to return TC_ACT_REDIRECT,
>the usually visible behavior is passing the skb up the stack - as
>for unknown action, but, with complex configuration, more random
>results can be obtained.
>
>This patch forcefully converts TC_ACT_REDIRECT to TC_ACT_UNSPEC
>at action init time, making the kernel behavior more consistent.
>
>v1 -> v3: use TC_ACT_UNSPEC instead of a newly definied act value
>
>Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>---
> net/sched/act_api.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>index 148a89ab789b..24b5534967fe 100644
>--- a/net/sched/act_api.c
>+++ b/net/sched/act_api.c
>@@ -895,6 +895,11 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> 		}
> 	}
> 
>+	if (a->tcfa_action == TC_ACT_REDIRECT) {
>+		net_warn_ratelimited("TC_ACT_REDIRECT can't be used directly");

Can't you push this warning through extack?

But, wouldn't it be more appropriate to fail here? User is passing
invalid configuration....


>+		a->tcfa_action = TC_ACT_UNSPEC;
>+	}
>+
> 	return a;
> 
> err_mod:
>-- 
>2.17.1
>

^ permalink raw reply

* Re: [PATCH net-next v3 3/5] tc/act: remove unneeded RCU lock in action callback
From: Jamal Hadi Salim @ 2018-07-25 11:59 UTC (permalink / raw)
  To: Paolo Abeni, netdev
  Cc: Cong Wang, Jiri Pirko, Daniel Borkmann, Marcelo Ricardo Leitner,
	Eyal Birger, David S. Miller
In-Reply-To: <b7bda0b303bed5bad1535bd917ea99789cd1db9e.1532437050.git.pabeni@redhat.com>

On 24/07/18 04:06 PM, Paolo Abeni wrote:
> Each lockless action currently does its own RCU locking in ->act().
> This is allows using plain RCU accessor, even if the context
> is really RCU BH.
> 
> This change drops the per action RCU lock, replace the accessors
> with _bh variant, cleans up a bit the surronding code and documents
> the RCU status in the relevant header.
> No functional nor performance change is intended.
> 
> The goal of this patch is clarifying that the RCU critical section
> used by the tc actions extends up to the classifier's caller.
> 

This and 2/5 seems to stand on their own merit.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH 1/4] treewide: convert ISO_8859-1 text comments to utf-8
From: Arnd Bergmann @ 2018-07-25 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Joe Perches, Samuel Ortiz, David S. Miller, Rob Herring,
	Michael Ellerman, Jonathan Cameron, linux-wireless, Networking,
	DTML, Linux Kernel Mailing List, Linux ARM,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linuxppc-dev,
	linux-iio, Linux PM list, lvs-devel, netfilter-devel, coreteam
In-Reply-To: <20180724175531.75276cf4e539124aa9e27177@linux-foundation.org>

tools/perf/tests/.gitignore:
                            LLVM byte-codes, uncompressed
On Wed, Jul 25, 2018 at 2:55 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 24 Jul 2018 17:13:20 -0700 Joe Perches <joe@perches.com> wrote:
>
>> On Tue, 2018-07-24 at 14:00 -0700, Andrew Morton wrote:
>> > On Tue, 24 Jul 2018 13:13:25 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>> > > Almost all files in the kernel are either plain text or UTF-8
>> > > encoded. A couple however are ISO_8859-1, usually just a few
>> > > characters in a C comments, for historic reasons.
>> > > This converts them all to UTF-8 for consistency.
>> []
>> > Will we be getting a checkpatch rule to keep things this way?
>>
>> How would that be done?
>
> I'm using this, seems to work.
>
>         if ! file $p | grep -q -P ", ASCII text|, UTF-8 Unicode text"
>         then
>                 echo $p: weird charset
>         fi

There are a couple of files that my version of 'find' incorrectly identified as
something completely different, like:

Documentation/devicetree/bindings/pinctrl/pinctrl-sx150x.txt:
            SemOne archive data
Documentation/devicetree/bindings/rtc/epson,rtc7301.txt:
            Microsoft Document Imaging Format
Documentation/filesystems/nfs/pnfs-block-server.txt:
            PPMN archive data
arch/arm/boot/dts/bcm283x-rpi-usb-host.dtsi:
        Sendmail frozen configuration  - version = "host";
Documentation/networking/segmentation-offloads.txt:
        StuffIt Deluxe Segment (data) : gmentation Offloads in the
Linux Networking Stack
arch/sparc/include/asm/visasm.h:                              SAS 7+
arch/xtensa/kernel/setup.c:                                         ,
init=0x454c, stat=0x090a, dev=0x2009, bas=0x2020
drivers/cpufreq/powernow-k8.c:
TI-XX Graphing Calculator (FLASH)
tools/testing/selftests/net/forwarding/tc_shblocks.sh:
                            Minix filesystem, V2 (big endian)
tools/perf/tests/.gitignore:
                            LLVM byte-codes, uncompressed

All of the above seem to be valid ASCII or UTF-8 files, so the check
above will lead
to false-positives, but it may be good enough as they are the
exception, and may be
bugs in 'file'.

Not sure if we need to worry about 'file' not being installed.

       Arnd

^ permalink raw reply

* Re: [PATCH net-next v6 1/4] net: vhost: lock the vqs one by one
From: Tonghao Zhang @ 2018-07-25 12:05 UTC (permalink / raw)
  To: mst
  Cc: jasowang, makita.toshiaki, virtualization,
	Linux Kernel Network Developers
In-Reply-To: <20180722182448-mutt-send-email-mst@kernel.org>

On Sun, Jul 22, 2018 at 11:26 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Jul 21, 2018 at 11:03:59AM -0700, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > This patch changes the way that lock all vqs
> > at the same, to lock them one by one. It will
> > be used for next patch to avoid the deadlock.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  drivers/vhost/vhost.c | 24 +++++++-----------------
> >  1 file changed, 7 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index a502f1a..a1c06e7 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -294,8 +294,11 @@ static void vhost_vq_meta_reset(struct vhost_dev *d)
> >  {
> >       int i;
> >
> > -     for (i = 0; i < d->nvqs; ++i)
> > +     for (i = 0; i < d->nvqs; ++i) {
> > +             mutex_lock(&d->vqs[i]->mutex);
> >               __vhost_vq_meta_reset(d->vqs[i]);
> > +             mutex_unlock(&d->vqs[i]->mutex);
> > +     }
> >  }
> >
> >  static void vhost_vq_reset(struct vhost_dev *dev,
> > @@ -890,20 +893,6 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
> >  #define vhost_get_used(vq, x, ptr) \
> >       vhost_get_user(vq, x, ptr, VHOST_ADDR_USED)
> >
> > -static void vhost_dev_lock_vqs(struct vhost_dev *d)
> > -{
> > -     int i = 0;
> > -     for (i = 0; i < d->nvqs; ++i)
> > -             mutex_lock_nested(&d->vqs[i]->mutex, i);
> > -}
> > -
> > -static void vhost_dev_unlock_vqs(struct vhost_dev *d)
> > -{
> > -     int i = 0;
> > -     for (i = 0; i < d->nvqs; ++i)
> > -             mutex_unlock(&d->vqs[i]->mutex);
> > -}
> > -
> >  static int vhost_new_umem_range(struct vhost_umem *umem,
> >                               u64 start, u64 size, u64 end,
> >                               u64 userspace_addr, int perm)
> > @@ -953,7 +942,10 @@ static void vhost_iotlb_notify_vq(struct vhost_dev *d,
> >               if (msg->iova <= vq_msg->iova &&
> >                   msg->iova + msg->size - 1 > vq_msg->iova &&
> >                   vq_msg->type == VHOST_IOTLB_MISS) {
> > +                     mutex_lock(&node->vq->mutex);
> >                       vhost_poll_queue(&node->vq->poll);
> > +                     mutex_unlock(&node->vq->mutex);
> > +
> >                       list_del(&node->node);
> >                       kfree(node);
> >               }
> > @@ -985,7 +977,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
> >       int ret = 0;
> >
> >       mutex_lock(&dev->mutex);
> > -     vhost_dev_lock_vqs(dev);
> >       switch (msg->type) {
> >       case VHOST_IOTLB_UPDATE:
> >               if (!dev->iotlb) {
> > @@ -1019,7 +1010,6 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
> >               break;
> >       }
> >
> > -     vhost_dev_unlock_vqs(dev);
> >       mutex_unlock(&dev->mutex);
> >
> >       return ret;
>
> I do prefer the finer-grained locking but I remember we
> discussed something like this in the past and Jason saw issues
> with such a locking.
This change is suggested by Jason. Should I send new version because
the patch 3 is changed.

> Jason?
>
> > --
> > 1.8.3.1

^ permalink raw reply

* Re: [PATCH net-next v3 0/5] TC: refactor act_mirred packets re-injection
From: Paolo Abeni @ 2018-07-25 12:07 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David S. Miller
In-Reply-To: <20180725115345.GC2164@nanopsycho>

Hi,

On Wed, 2018-07-25 at 13:53 +0200, Jiri Pirko wrote:
> Could you please send the userspace iproute2 patch and list some
> examples of usage along with the next patchset version?

There are no iproute2 patches, as TC_ACT_REINJECT is not accessible
from user-space: act_mirred will use such value when possible.

User-space can have any notion of the above only via the perf tool
and/or performances tests ;)

An use case is the OVS TC S/W datapath. openvswitch alreday leverage
this when configured with:

other_config:hw-offload=true
other_config:tc-policy=none

> Also, I would like you to do selftest script to test this and have it as
> a part of this patchset.

As said, there is no user-space change. AFAICS the current act_mirred
self tests already cover this. Do you have other things in mind?

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT.
From: Jamal Hadi Salim @ 2018-07-25 12:16 UTC (permalink / raw)
  To: Paolo Abeni, netdev
  Cc: Cong Wang, Jiri Pirko, Daniel Borkmann, Marcelo Ricardo Leitner,
	Eyal Birger, David S. Miller, Shmulik Ladkani
In-Reply-To: <3c20787be0fd5d64728ffed46ae0a7dff10d7e05.1532437050.git.pabeni@redhat.com>

+Cc Shmulik

Paolo - please also run the tdc tests (and add anymore if you
feel they dont do coverage to your changes)

On 24/07/18 04:06 PM, Paolo Abeni wrote:
> This is similar TC_ACT_REDIRECT, but with a slightly different
> semantic:
> - on ingress the mirred skbs are passed to the target device
> network stack without any additional check not scrubbing.
> - the rcu-protected stats provided via the tcf_result struct
>    are updated on error conditions.
> 
> This new tcfa_action value is not exposed to the user-space
> and can be used only internally by clsact.
> 
> v1 -> v2: do not touch TC_ACT_REDIRECT code path, introduce
>   a new action type instead
> 
> v2 -> v3:
>   - rename the new action value TC_ACT_REINJECT, update the
>     helper accordingly
>   - take care of uncloned reinjected packets in XDP generic
>     hook
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>   include/net/pkt_cls.h     |  3 +++
>   include/net/sch_generic.h | 19 +++++++++++++++++++
>   net/core/dev.c            |  6 +++++-
>   3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 2081e4219f81..36ccfe2a303a 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -7,6 +7,9 @@
>   #include <net/sch_generic.h>
>   #include <net/act_api.h>
>   
> +/* TC action not accessible from user space */
> +#define TC_ACT_REINJECT		(TC_ACT_VALUE_MAX + 1)

Lets say in the future we add a new opcode.
Will old kernel, new iproute2 (new value) work?
Maybe use a negative number below -1.
I am honestly still unclear about introducing this new
code.
Could you not use the result code to carry sufficient
info to indicate the intent?

cheers,
jamal

> +
>   /* Basic packet classifier frontend definitions. */
>   
>   struct tcf_walker {
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 056dc1083aa3..95e81a70f549 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -235,6 +235,12 @@ struct tcf_result {
>   			u32		classid;
>   		};
>   		const struct tcf_proto *goto_tp;
> +
> +		/* used by the TC_ACT_REINJECT action */
> +		struct {
> +			bool		ingress;
> +			struct gnet_stats_queue *qstats;
> +		};
>   	};
>   };
>   
> @@ -1091,4 +1097,17 @@ void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
>   void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
>   			  struct mini_Qdisc __rcu **p_miniq);
>   
> +static inline void skb_tc_reinject(struct sk_buff *skb, struct tcf_result *res)
> +{
> +	struct gnet_stats_queue *stats = res->qstats;
> +	int ret;
> +
> +	if (res->ingress)
> +		ret = netif_receive_skb(skb);
> +	else
> +		ret = dev_queue_xmit(skb);
> +	if (ret && stats)
> +		qstats_overlimit_inc(res->qstats);
> +}
> +
>   #endif
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 14a748ee8cc9..826ec74fe1d9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4252,7 +4252,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
>   	/* Reinjected packets coming from act_mirred or similar should
>   	 * not get XDP generic processing.
>   	 */
> -	if (skb_cloned(skb))
> +	if (skb_cloned(skb) || skb->tc_redirected)
>   		return XDP_PASS;
>   
>   	/* XDP packets must be linear and must have sufficient headroom
> @@ -4602,6 +4602,10 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>   		__skb_push(skb, skb->mac_len);
>   		skb_do_redirect(skb);
>   		return NULL;
> +	case TC_ACT_REINJECT:
> +		/* this does not scrub the packet, and updates stats on error */
> +		skb_tc_reinject(skb, &cl_res);
> +		return NULL;
>   	default:
>   		break;
>   	}
> 

^ permalink raw reply

* Re: [BUG BISECT] NFSv4 client fails on Flush Journal to Persistent Storage
From: Krzysztof Kozlowski @ 2018-07-25 13:27 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Trond Myklebust, sudeep.holla@arm.com, Trond Myklebust,
	Anna Schumaker, J. Bruce Fields, Jeff Layton, David S. Miller,
	linux-nfs, netdev, linux-kernel,
	linux-samsung-soc@vger.kernel.org
In-Reply-To: <784782B0-A387-4D3B-A517-E85D851162B8@oracle.com>

On 18 June 2018 at 18:20, Chuck Lever <chuck.lever@oracle.com> wrote:
>
> The extra serialization appears to have a reproducible performance
> impact on RDMA, which no longer takes the reserve_lock when allocating
> a slot.
>
> I could put an xprt_alloc_xid call in xprt_alloc_slot, but that would
> only work for socket-based transports. Would it be OK if RDMA had its
> own XID allocation mechanism?

Hi,

On recent next the issue appeared again. My boards with NFSv4 root
timeout on 80% of boots. This time my NFS server is faster - Pi3 B+
:).

Is this know? Should I start long bisect or maybe you can point me to
possible causes?

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH net-next v3 0/5] TC: refactor act_mirred packets re-injection
From: Jiri Pirko @ 2018-07-25 12:17 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David S. Miller
In-Reply-To: <168de66002c665d817e7ad5f3137a7db42b0bec9.camel@redhat.com>

Wed, Jul 25, 2018 at 02:07:15PM CEST, pabeni@redhat.com wrote:
>Hi,
>
>On Wed, 2018-07-25 at 13:53 +0200, Jiri Pirko wrote:
>> Could you please send the userspace iproute2 patch and list some
>> examples of usage along with the next patchset version?
>
>There are no iproute2 patches, as TC_ACT_REINJECT is not accessible
>from user-space: act_mirred will use such value when possible.

Okay.

>
>User-space can have any notion of the above only via the perf tool
>and/or performances tests ;)
>
>An use case is the OVS TC S/W datapath. openvswitch alreday leverage
>this when configured with:
>
>other_config:hw-offload=true
>other_config:tc-policy=none
>
>> Also, I would like you to do selftest script to test this and have it as
>> a part of this patchset.
>
>As said, there is no user-space change. AFAICS the current act_mirred
>self tests already cover this. Do you have other things in mind?

Sounds allright. Thanks for the explanation!


>
>Cheers,
>
>Paolo
>

^ permalink raw reply

* Re: [PATCH net-next] tcp: add SNMP counter for the number of packets pruned from ofo queue
From: Eric Dumazet @ 2018-07-25 13:33 UTC (permalink / raw)
  To: Yafang Shao, davem, edumazet; +Cc: netdev, linux-kernel
In-Reply-To: <1532524010-11855-1-git-send-email-laoar.shao@gmail.com>



On 07/25/2018 06:06 AM, Yafang Shao wrote:
> LINUX_MIB_OFOPRUNED is used to count how many times ofo queue is pruned,
> but sometimes we want to know how many packets are pruned from this queue,
> that could help us to track the dropped packets.
> 
> As LINUX_MIB_OFOPRUNED is a useful event for us, so I introduce a new
> SNMP counter LINUX_MIB_OFOPRUNEDROP, which could be showed in netstat as
> OfoPruneDrop.


Okay, but why tracking number of skbs that are removed ?

Skb can contain many segments (because of GRO and TCP coalescing)

So your claim of tracking dropped packets is ill defined.

Also I prefer having net tree being merged into net-next, since your patch would
add a merge conflict.

^ permalink raw reply

* Re: [PATCH net-next v3 4/5] net/tc: introduce TC_ACT_REINJECT.
From: Jamal Hadi Salim @ 2018-07-25 12:27 UTC (permalink / raw)
  To: Paolo Abeni, Cong Wang
  Cc: Linux Kernel Network Developers, Jiri Pirko, Daniel Borkmann,
	Marcelo Ricardo Leitner, Eyal Birger, David Miller
In-Reply-To: <6a9fd3c8f861c203c7e12a2a2e477796c5e093d5.camel@redhat.com>

On 25/07/18 04:29 AM, Paolo Abeni wrote:
> On Tue, 2018-07-24 at 13:50 -0700, Cong Wang wrote:

[..]
>>> I fail to understand why overlimit is increased in your case
>>> here. I guess you want to increase 'drops' instead.
>>
>> Hmm, actually the current mirred code increases overlimit too.
>> But I still don't think it makes sense.
> 
> Yep, I chose to increment 'overlimits' to preserve the current mirred
> semantic.
> 
> AFAICS, that was first introduced with:
> 
> commit 8919bc13e8d92c5b082c5c0321567383a071f5bc
> Author: Jamal Hadi Salim <jhs@mojatatu.com>
> Date:   Mon Aug 15 05:25:40 2011 +0000
> 
>      net_sched: fix port mirror/redirect stats reporting
> 
> Likely increasing 'drops' would be "better", but I'm unsure we can
> change this established behavior without affecting some user.
> 

Those changes were there from the beginning (above patch did
not introduce them).
IIRC, the reason was to distinguish between policy intended
drops and drops because of errors.

cheers,
jamal

^ 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