Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 2/3] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
From: Andrey Ignatov @ 2018-11-09 18:54 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kafai, kernel-team
In-Reply-To: <cover.1541789512.git.rdna@fb.com>

Make bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers
available in programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR.

Such programs operate on sockets and have access to socket and struct
sockaddr passed by user to system calls such as sys_bind, sys_connect,
sys_sendmsg.

It's useful to be able to lookup other sockets from these programs.
E.g. sys_connect may lookup IP:port endpoint and if there is a server
socket bound to that endpoint ("server" can be defined by saddr & sport
being zero), redirect client connection to it by rewriting IP:port in
sockaddr passed to sys_connect.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index f4ae933edf61..f6ca38a7d433 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5042,6 +5042,43 @@ static const struct bpf_func_proto bpf_xdp_sk_lookup_tcp_proto = {
 	.arg4_type      = ARG_ANYTHING,
 	.arg5_type      = ARG_ANYTHING,
 };
+
+BPF_CALL_5(bpf_sock_addr_sk_lookup_tcp, struct bpf_sock_addr_kern *, ctx,
+	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+{
+	return __bpf_sk_lookup(NULL, tuple, len, sock_net(ctx->sk), 0,
+			       IPPROTO_TCP, netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = {
+	.func		= bpf_sock_addr_sk_lookup_tcp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
+BPF_CALL_5(bpf_sock_addr_sk_lookup_udp, struct bpf_sock_addr_kern *, ctx,
+	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+{
+	return __bpf_sk_lookup(NULL, tuple, len, sock_net(ctx->sk), 0,
+			       IPPROTO_UDP, netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
+	.func		= bpf_sock_addr_sk_lookup_udp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -5148,6 +5185,14 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_socket_cookie_sock_addr_proto;
 	case BPF_FUNC_get_local_storage:
 		return &bpf_get_local_storage_proto;
+#ifdef CONFIG_INET
+	case BPF_FUNC_sk_lookup_tcp:
+		return &bpf_sock_addr_sk_lookup_tcp_proto;
+	case BPF_FUNC_sk_lookup_udp:
+		return &bpf_sock_addr_sk_lookup_udp_proto;
+	case BPF_FUNC_sk_release:
+		return &bpf_sk_release_proto;
+#endif /* CONFIG_INET */
 	default:
 		return bpf_base_func_proto(func_id);
 	}
-- 
2.17.1

^ permalink raw reply related

* [PATCH bpf-next v2 0/3] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
From: Andrey Ignatov @ 2018-11-09 18:53 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kafai, kernel-team

This patch set makes bpf_sk_lookup_tcp, bpf_sk_lookup_udp and
bpf_sk_release helpers available in programs of type
BPF_PROG_TYPE_CGROUP_SOCK_ADDR.

Patch 1 is a fix for bpf_sk_lookup_udp that was already merged to bpf
(stable) tree. Here it's prerequisite for patch 3.

Patch 2 is the main patch in the set, it makes the helpers available for
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and provides more details about use-case.

Patch 3 adds selftest for new functionality.

v1->v2:
- remove "Split bpf_sk_lookup" patch since it was already split by:
  commit c8123ead13a5 ("bpf: Extend the sk_lookup() helper to XDP
  hookpoint.");
- avoid unnecessary bpf_sock_addr_sk_lookup function.


Andrey Ignatov (3):
  bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp
  bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
  selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr

 net/core/filter.c                           | 50 ++++++++++++++++--
 tools/testing/selftests/bpf/connect4_prog.c | 43 ++++++++++++----
 tools/testing/selftests/bpf/connect6_prog.c | 56 ++++++++++++++++-----
 3 files changed, 125 insertions(+), 24 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH bpf-next v2 3/3] selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr
From: Andrey Ignatov @ 2018-11-09 18:54 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kafai, kernel-team
In-Reply-To: <cover.1541789512.git.rdna@fb.com>

Use bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers from
test_sock_addr programs to make sure they're available and can lookup
and release socket properly for IPv4/IPv4, TCP/UDP.

Reading from a few fields of returned struct bpf_sock is also tested.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/connect4_prog.c | 43 ++++++++++++----
 tools/testing/selftests/bpf/connect6_prog.c | 56 ++++++++++++++++-----
 2 files changed, 78 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/bpf/connect4_prog.c b/tools/testing/selftests/bpf/connect4_prog.c
index 5a88a681d2ab..b8395f3c43e9 100644
--- a/tools/testing/selftests/bpf/connect4_prog.c
+++ b/tools/testing/selftests/bpf/connect4_prog.c
@@ -21,23 +21,48 @@ int _version SEC("version") = 1;
 SEC("cgroup/connect4")
 int connect_v4_prog(struct bpf_sock_addr *ctx)
 {
+	struct bpf_sock_tuple tuple = {};
 	struct sockaddr_in sa;
+	struct bpf_sock *sk;
+
+	/* Verify that new destination is available. */
+	memset(&tuple.ipv4.saddr, 0, sizeof(tuple.ipv4.saddr));
+	memset(&tuple.ipv4.sport, 0, sizeof(tuple.ipv4.sport));
+
+	tuple.ipv4.daddr = bpf_htonl(DST_REWRITE_IP4);
+	tuple.ipv4.dport = bpf_htons(DST_REWRITE_PORT4);
+
+	if (ctx->type != SOCK_STREAM && ctx->type != SOCK_DGRAM)
+		return 0;
+	else if (ctx->type == SOCK_STREAM)
+		sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof(tuple.ipv4), 0, 0);
+	else
+		sk = bpf_sk_lookup_udp(ctx, &tuple, sizeof(tuple.ipv4), 0, 0);
+
+	if (!sk)
+		return 0;
+
+	if (sk->src_ip4 != tuple.ipv4.daddr ||
+	    sk->src_port != DST_REWRITE_PORT4) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release(sk);
 
 	/* Rewrite destination. */
 	ctx->user_ip4 = bpf_htonl(DST_REWRITE_IP4);
 	ctx->user_port = bpf_htons(DST_REWRITE_PORT4);
 
-	if (ctx->type == SOCK_DGRAM || ctx->type == SOCK_STREAM) {
-		///* Rewrite source. */
-		memset(&sa, 0, sizeof(sa));
+	/* Rewrite source. */
+	memset(&sa, 0, sizeof(sa));
 
-		sa.sin_family = AF_INET;
-		sa.sin_port = bpf_htons(0);
-		sa.sin_addr.s_addr = bpf_htonl(SRC_REWRITE_IP4);
+	sa.sin_family = AF_INET;
+	sa.sin_port = bpf_htons(0);
+	sa.sin_addr.s_addr = bpf_htonl(SRC_REWRITE_IP4);
 
-		if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
-			return 0;
-	}
+	if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
+		return 0;
 
 	return 1;
 }
diff --git a/tools/testing/selftests/bpf/connect6_prog.c b/tools/testing/selftests/bpf/connect6_prog.c
index 8ea3f7d12dee..25f5dc7b7aa0 100644
--- a/tools/testing/selftests/bpf/connect6_prog.c
+++ b/tools/testing/selftests/bpf/connect6_prog.c
@@ -29,7 +29,41 @@ int _version SEC("version") = 1;
 SEC("cgroup/connect6")
 int connect_v6_prog(struct bpf_sock_addr *ctx)
 {
+	struct bpf_sock_tuple tuple = {};
 	struct sockaddr_in6 sa;
+	struct bpf_sock *sk;
+
+	/* Verify that new destination is available. */
+	memset(&tuple.ipv6.saddr, 0, sizeof(tuple.ipv6.saddr));
+	memset(&tuple.ipv6.sport, 0, sizeof(tuple.ipv6.sport));
+
+	tuple.ipv6.daddr[0] = bpf_htonl(DST_REWRITE_IP6_0);
+	tuple.ipv6.daddr[1] = bpf_htonl(DST_REWRITE_IP6_1);
+	tuple.ipv6.daddr[2] = bpf_htonl(DST_REWRITE_IP6_2);
+	tuple.ipv6.daddr[3] = bpf_htonl(DST_REWRITE_IP6_3);
+
+	tuple.ipv6.dport = bpf_htons(DST_REWRITE_PORT6);
+
+	if (ctx->type != SOCK_STREAM && ctx->type != SOCK_DGRAM)
+		return 0;
+	else if (ctx->type == SOCK_STREAM)
+		sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof(tuple.ipv6), 0, 0);
+	else
+		sk = bpf_sk_lookup_udp(ctx, &tuple, sizeof(tuple.ipv6), 0, 0);
+
+	if (!sk)
+		return 0;
+
+	if (sk->src_ip6[0] != tuple.ipv6.daddr[0] ||
+	    sk->src_ip6[1] != tuple.ipv6.daddr[1] ||
+	    sk->src_ip6[2] != tuple.ipv6.daddr[2] ||
+	    sk->src_ip6[3] != tuple.ipv6.daddr[3] ||
+	    sk->src_port != DST_REWRITE_PORT6) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release(sk);
 
 	/* Rewrite destination. */
 	ctx->user_ip6[0] = bpf_htonl(DST_REWRITE_IP6_0);
@@ -39,21 +73,19 @@ int connect_v6_prog(struct bpf_sock_addr *ctx)
 
 	ctx->user_port = bpf_htons(DST_REWRITE_PORT6);
 
-	if (ctx->type == SOCK_DGRAM || ctx->type == SOCK_STREAM) {
-		/* Rewrite source. */
-		memset(&sa, 0, sizeof(sa));
+	/* Rewrite source. */
+	memset(&sa, 0, sizeof(sa));
 
-		sa.sin6_family = AF_INET6;
-		sa.sin6_port = bpf_htons(0);
+	sa.sin6_family = AF_INET6;
+	sa.sin6_port = bpf_htons(0);
 
-		sa.sin6_addr.s6_addr32[0] = bpf_htonl(SRC_REWRITE_IP6_0);
-		sa.sin6_addr.s6_addr32[1] = bpf_htonl(SRC_REWRITE_IP6_1);
-		sa.sin6_addr.s6_addr32[2] = bpf_htonl(SRC_REWRITE_IP6_2);
-		sa.sin6_addr.s6_addr32[3] = bpf_htonl(SRC_REWRITE_IP6_3);
+	sa.sin6_addr.s6_addr32[0] = bpf_htonl(SRC_REWRITE_IP6_0);
+	sa.sin6_addr.s6_addr32[1] = bpf_htonl(SRC_REWRITE_IP6_1);
+	sa.sin6_addr.s6_addr32[2] = bpf_htonl(SRC_REWRITE_IP6_2);
+	sa.sin6_addr.s6_addr32[3] = bpf_htonl(SRC_REWRITE_IP6_3);
 
-		if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
-			return 0;
-	}
+	if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
+		return 0;
 
 	return 1;
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH bpf-next v2 1/3] bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp
From: Andrey Ignatov @ 2018-11-09 18:54 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, joe, kafai, kernel-team
In-Reply-To: <cover.1541789512.git.rdna@fb.com>

Lookup functions in sk_lookup have different expectations about byte
order of provided arguments.

Specifically __inet_lookup, __udp4_lib_lookup and __udp6_lib_lookup
expect dport to be in network byte order and do ntohs(dport) internally.

At the same time __inet6_lookup expects dport to be in host byte order
and correspondingly name the argument hnum.

sk_lookup works correctly with __inet_lookup, __udp4_lib_lookup and
__inet6_lookup with regard to dport. But in __udp6_lib_lookup case it
uses host instead of expected network byte order. It makes result
returned by bpf_sk_lookup_udp for IPv6 incorrect.

The patch fixes byte order of dport passed to __udp6_lib_lookup.

Originally sk_lookup properly handled UDPv6, but not TCPv6. 5ef0ae84f02a
fixes TCPv6 but breaks UDPv6.

Fixes: 5ef0ae84f02a ("bpf: Fix IPv6 dport byte-order in bpf_sk_lookup")
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 net/core/filter.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 53d50fb75ea1..f4ae933edf61 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4867,17 +4867,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
 	} else {
 		struct in6_addr *src6 = (struct in6_addr *)&tuple->ipv6.saddr;
 		struct in6_addr *dst6 = (struct in6_addr *)&tuple->ipv6.daddr;
-		u16 hnum = ntohs(tuple->ipv6.dport);
 
 		if (proto == IPPROTO_TCP)
 			sk = __inet6_lookup(net, &tcp_hashinfo, NULL, 0,
 					    src6, tuple->ipv6.sport,
-					    dst6, hnum,
+					    dst6, ntohs(tuple->ipv6.dport),
 					    dif, sdif, &refcounted);
 		else if (likely(ipv6_bpf_stub))
 			sk = ipv6_bpf_stub->udp6_lib_lookup(net,
 							    src6, tuple->ipv6.sport,
-							    dst6, hnum,
+							    dst6, tuple->ipv6.dport,
 							    dif, sdif,
 							    &udp_table, NULL);
 #endif
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next v2 2/3] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
From: Martin Lau @ 2018-11-09 19:30 UTC (permalink / raw)
  To: Andrey Ignatov
  Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	joe@wand.net.nz, Kernel Team
In-Reply-To: <53ea8fee1f9a7c0a862e489799c3fd80b5b0034a.1541789512.git.rdna@fb.com>

On Fri, Nov 09, 2018 at 10:54:01AM -0800, Andrey Ignatov wrote:
> Make bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers
> available in programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR.
> 
> Such programs operate on sockets and have access to socket and struct
> sockaddr passed by user to system calls such as sys_bind, sys_connect,
> sys_sendmsg.
> 
> It's useful to be able to lookup other sockets from these programs.
> E.g. sys_connect may lookup IP:port endpoint and if there is a server
> socket bound to that endpoint ("server" can be defined by saddr & sport
> being zero), redirect client connection to it by rewriting IP:port in
> sockaddr passed to sys_connect.
Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-09 19:43 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang

Currently netdev_rx_csum_fault() only shows a device name,
we need more information about the skb for debugging.

Sample output:

 ens3: hw csum failure
 dev features: 0x0000000000014b89
 skb len=84 data_len=0 gso_size=0 gso_type=0 ip_summed=0 csum=0, csum_complete_sw=0, csum_valid=0

Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/linux/netdevice.h |  5 +++--
 net/core/datagram.c       |  6 +++---
 net/core/dev.c            | 10 ++++++++--
 net/sunrpc/socklib.c      |  2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 857f8abf7b91..fabcd9fa6cf7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4332,9 +4332,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
 }
 
 #ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev);
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
 #else
-static inline void netdev_rx_csum_fault(struct net_device *dev)
+static inline void netdev_rx_csum_fault(struct net_device *dev,
+					struct sk_buff *skb)
 {
 }
 #endif
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 57f3a6fcfc1e..d8f4d55cd6c5 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
 	if (likely(!sum)) {
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
 		    !skb->csum_complete_sw)
-			netdev_rx_csum_fault(skb->dev);
+			netdev_rx_csum_fault(skb->dev, skb);
 	}
 	if (!skb_shared(skb))
 		skb->csum_valid = !sum;
@@ -756,7 +756,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
 	if (likely(!sum)) {
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
 		    !skb->csum_complete_sw)
-			netdev_rx_csum_fault(skb->dev);
+			netdev_rx_csum_fault(skb->dev, skb);
 	}
 
 	if (!skb_shared(skb)) {
@@ -810,7 +810,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
 
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
 		    !skb->csum_complete_sw)
-			netdev_rx_csum_fault(NULL);
+			netdev_rx_csum_fault(NULL, skb);
 	}
 	return 0;
 fault:
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ffcbdd55fa9..2b337df26117 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3091,10 +3091,16 @@ EXPORT_SYMBOL(__skb_gso_segment);
 
 /* Take action when hardware reception checksum errors are detected. */
 #ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev)
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
 {
 	if (net_ratelimit()) {
 		pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
+		if (dev)
+			pr_err("dev features: %pNF\n", &dev->features);
+		pr_err("skb len=%d data_len=%d gso_size=%d gso_type=%d ip_summed=%d csum=%x, csum_complete_sw=%d, csum_valid=%d\n",
+		       skb->len, skb->data_len, skb_shinfo(skb)->gso_size,
+		       skb_shinfo(skb)->gso_type, skb->ip_summed, skb->csum,
+		       skb->csum_complete_sw, skb->csum_valid);
 		dump_stack();
 	}
 }
@@ -5779,7 +5785,7 @@ __sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
 	if (likely(!sum)) {
 		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
 		    !skb->csum_complete_sw)
-			netdev_rx_csum_fault(skb->dev);
+			netdev_rx_csum_fault(skb->dev, skb);
 	}
 
 	NAPI_GRO_CB(skb)->csum = wsum;
diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
index 9062967575c4..7e55cfc69697 100644
--- a/net/sunrpc/socklib.c
+++ b/net/sunrpc/socklib.c
@@ -175,7 +175,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr, struct sk_buff *skb)
 		return -1;
 	if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
 	    !skb->csum_complete_sw)
-		netdev_rx_csum_fault(skb->dev);
+		netdev_rx_csum_fault(skb->dev, skb);
 	return 0;
 no_checksum:
 	if (xdr_partial_copy_from_skb(xdr, 0, &desc, xdr_skb_read_bits) < 0)
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH net] ip: hash fragments consistently
From: Eric Dumazet @ 2018-11-09 19:44 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: David S. Miller, soukjin bae
In-Reply-To: <211e391b-3537-560b-0522-9ea595848477@gmail.com>



On 07/23/2018 09:26 AM, Eric Dumazet wrote:
> 
> 
> On 07/23/2018 07:50 AM, Paolo Abeni wrote:
>> The skb hash for locally generated ip[v6] fragments belonging
>> to the same datagram can vary in several circumstances:
>> * for connected UDP[v6] sockets, the first fragment get its hash
>>   via set_owner_w()/skb_set_hash_from_sk()
>> * for unconnected IPv6 UDPv6 sockets, the first fragment can get
>>   its hash via ip6_make_flowlabel()/skb_get_hash_flowi6(), if
>>   auto_flowlabel is enabled
>>
>> For the following frags the hash is usually computed via
>> skb_get_hash().
>> The above can cause OoO for unconnected IPv6 UDPv6 socket: in that
>> scenario the egress tx queue can be selected on a per packet basis
>> via the skb hash.
>> It may also fool flow-oriented schedulers to place fragments belonging
>> to the same datagram in different flows.
>>
> 
> It also fools bond_xmit_hash(), packets of the same datagram can be sent on
> two bonding slaves instead of one, meaning adding pressure on the defrag unit
> in receiver.
> 
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> 

Also we might note that flow dissector itself is buggy as
found by Soukjin Bae ( https://patchwork.ozlabs.org/patch/994601/ )

I will send a v2 of his patch with a different changelog.

Defrag is fixed [1] but the bug in flow dissector is adding
extra work and hash inconsistencies.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=0d5b9311baf27bb545f187f12ecfd558220c607d

^ permalink raw reply

* Re: [PATCH iproute] ss: Actually print left delimiter for columns
From: Stefano Brivio @ 2018-11-09 19:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Yoann P., netdev, Phil Sutter
In-Reply-To: <20181109090546.50c15c61@xeon-e3>

On Fri, 9 Nov 2018 09:05:46 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> On Mon, 29 Oct 2018 23:04:25 +0100
> Stefano Brivio <sbrivio@redhat.com> wrote:
> 
> > While rendering columns, we use a local variable to keep track of the
> > field currently being printed, without touching current_field, which is
> > used for buffering.
> > 
> > Use the right pointer to access the left delimiter for the current column,
> > instead of always printing the left delimiter for the last buffered field,
> > which is usually an empty string.
> > 
> > This fixes an issue especially visible on narrow terminals, where some
> > columns might be displayed without separation.
> > 
> > Reported-by: YoyPa <yoann.p.public@gmail.com>
> > Fixes: 691bd854bf4a ("ss: Buffer raw fields first, then render them as a table")
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > Tested-by: YoyPa <yoann.p.public@gmail.com>  
> 
> This test broke the testsuite/ss/ssfilter.t test.
> Please fix the test to match your new output format, or I will have to revert it.

Ouch, sorry, I didn't notice that "new" test. I'll fix that by tomorrow.

-- 
Stefano

^ permalink raw reply

* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-09 19:59 UTC (permalink / raw)
  To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <1017e0cd-d8a1-ca2f-7d1d-8edad5a37eeb@gmail.com>



W dniu 09.11.2018 o 17:21, David Ahern pisze:
> On 11/9/18 3:20 AM, Paweł Staszewski wrote:
>> I just catch some weird behavior :)
>> All was working fine for about 20k packets
>>
>> Then after xdp start to forward every 10 packets
> Interesting. Any counter showing drops?
nothing that will fit

NIC statistics:
      rx_packets: 187041
      rx_bytes: 10600954
      tx_packets: 40316
      tx_bytes: 16526844
      tx_tso_packets: 797
      tx_tso_bytes: 3876084
      tx_tso_inner_packets: 0
      tx_tso_inner_bytes: 0
      tx_added_vlan_packets: 38391
      tx_nop: 2
      rx_lro_packets: 0
      rx_lro_bytes: 0
      rx_ecn_mark: 0
      rx_removed_vlan_packets: 187041
      rx_csum_unnecessary: 0
      rx_csum_none: 150011
      rx_csum_complete: 37030
      rx_csum_unnecessary_inner: 0
      rx_xdp_drop: 0
      rx_xdp_redirect: 64893
      rx_xdp_tx_xmit: 0
      rx_xdp_tx_full: 0
      rx_xdp_tx_err: 0
      rx_xdp_tx_cqe: 0
      tx_csum_none: 2468
      tx_csum_partial: 35955
      tx_csum_partial_inner: 0
      tx_queue_stopped: 0
      tx_queue_dropped: 0
      tx_xmit_more: 0
      tx_recover: 0
      tx_cqes: 38423
      tx_queue_wake: 0
      tx_udp_seg_rem: 0
      tx_cqe_err: 0
      tx_xdp_xmit: 0
      tx_xdp_full: 0
      tx_xdp_err: 0
      tx_xdp_cqes: 0
      rx_wqe_err: 0
      rx_mpwqe_filler_cqes: 0
      rx_mpwqe_filler_strides: 0
      rx_buff_alloc_err: 0
      rx_cqe_compress_blks: 0
      rx_cqe_compress_pkts: 0
      rx_page_reuse: 0
      rx_cache_reuse: 186302
      rx_cache_full: 0
      rx_cache_empty: 666768
      rx_cache_busy: 174
      rx_cache_waive: 0
      rx_congst_umr: 0
      rx_arfs_err: 0
      ch_events: 249320
      ch_poll: 249321
      ch_arm: 249001
      ch_aff_change: 0
      ch_eq_rearm: 0
      rx_out_of_buffer: 0
      rx_if_down_packets: 57
      rx_vport_unicast_packets: 142659
      rx_vport_unicast_bytes: 42706914
      tx_vport_unicast_packets: 40167
      tx_vport_unicast_bytes: 16668096
      rx_vport_multicast_packets: 39188170
      rx_vport_multicast_bytes: 3466527450
      tx_vport_multicast_packets: 58
      tx_vport_multicast_bytes: 4556
      rx_vport_broadcast_packets: 16343520
      rx_vport_broadcast_bytes: 1031334602
      tx_vport_broadcast_packets: 91
      tx_vport_broadcast_bytes: 5460
      rx_vport_rdma_unicast_packets: 0
      rx_vport_rdma_unicast_bytes: 0
      tx_vport_rdma_unicast_packets: 0
      tx_vport_rdma_unicast_bytes: 0
      rx_vport_rdma_multicast_packets: 0
      rx_vport_rdma_multicast_bytes: 0
      tx_vport_rdma_multicast_packets: 0
      tx_vport_rdma_multicast_bytes: 0
      tx_packets_phy: 40316
      rx_packets_phy: 55674361
      rx_crc_errors_phy: 0
      tx_bytes_phy: 16839376
      rx_bytes_phy: 4763267396
      tx_multicast_phy: 58
      tx_broadcast_phy: 91
      rx_multicast_phy: 39188180
      rx_broadcast_phy: 16343521
      rx_in_range_len_errors_phy: 0
      rx_out_of_range_len_phy: 0
      rx_oversize_pkts_phy: 0
      rx_symbol_err_phy: 0
      tx_mac_control_phy: 0
      rx_mac_control_phy: 0
      rx_unsupported_op_phy: 0
      rx_pause_ctrl_phy: 0
      tx_pause_ctrl_phy: 0
      rx_discards_phy: 1
      tx_discards_phy: 0
      tx_errors_phy: 0
      rx_undersize_pkts_phy: 0
      rx_fragments_phy: 0
      rx_jabbers_phy: 0
      rx_64_bytes_phy: 3792455
      rx_65_to_127_bytes_phy: 51821620
      rx_128_to_255_bytes_phy: 37669
      rx_256_to_511_bytes_phy: 1481
      rx_512_to_1023_bytes_phy: 434
      rx_1024_to_1518_bytes_phy: 694
      rx_1519_to_2047_bytes_phy: 20008
      rx_2048_to_4095_bytes_phy: 0
      rx_4096_to_8191_bytes_phy: 0
      rx_8192_to_10239_bytes_phy: 0
      link_down_events_phy: 0
      rx_pcs_symbol_err_phy: 0
      rx_corrected_bits_phy: 6
      rx_err_lane_0_phy: 0
      rx_err_lane_1_phy: 0
      rx_err_lane_2_phy: 0
      rx_err_lane_3_phy: 6
      rx_buffer_passed_thres_phy: 0
      rx_pci_signal_integrity: 0
      tx_pci_signal_integrity: 82
      outbound_pci_stalled_rd: 0
      outbound_pci_stalled_wr: 0
      outbound_pci_stalled_rd_events: 0
      outbound_pci_stalled_wr_events: 0
      rx_prio0_bytes: 4144920388
      rx_prio0_packets: 48310037
      tx_prio0_bytes: 16839376
      tx_prio0_packets: 40316
      rx_prio1_bytes: 481032
      rx_prio1_packets: 7074
      tx_prio1_bytes: 0
      tx_prio1_packets: 0
      rx_prio2_bytes: 9074194
      rx_prio2_packets: 106207
      tx_prio2_bytes: 0
      tx_prio2_packets: 0
      rx_prio3_bytes: 0
      rx_prio3_packets: 0
      tx_prio3_bytes: 0
      tx_prio3_packets: 0
      rx_prio4_bytes: 0
      rx_prio4_packets: 0
      tx_prio4_bytes: 0
      tx_prio4_packets: 0
      rx_prio5_bytes: 0
      rx_prio5_packets: 0
      tx_prio5_bytes: 0
      tx_prio5_packets: 0
      rx_prio6_bytes: 371961810
      rx_prio6_packets: 4006281
      tx_prio6_bytes: 0
      tx_prio6_packets: 0
      rx_prio7_bytes: 236830040
      rx_prio7_packets: 3244761
      tx_prio7_bytes: 0
      tx_prio7_packets: 0
      tx_pause_storm_warning_events : 0
      tx_pause_storm_error_events: 0
      module_unplug: 0
      module_bus_stuck: 0
      module_high_temp: 0
      module_bad_shorted: 0

NIC statistics:
      rx_packets: 843
      rx_bytes: 58889
      tx_packets: 324
      tx_bytes: 23324
      tx_tso_packets: 0
      tx_tso_bytes: 0
      tx_tso_inner_packets: 0
      tx_tso_inner_bytes: 0
      tx_added_vlan_packets: 293
      tx_nop: 0
      rx_lro_packets: 0
      rx_lro_bytes: 0
      rx_ecn_mark: 0
      rx_removed_vlan_packets: 843
      rx_csum_unnecessary: 0
      rx_csum_none: 190
      rx_csum_complete: 653
      rx_csum_unnecessary_inner: 0
      rx_xdp_drop: 0
      rx_xdp_redirect: 0
      rx_xdp_tx_xmit: 0
      rx_xdp_tx_full: 0
      rx_xdp_tx_err: 0
      rx_xdp_tx_cqe: 0
      tx_csum_none: 324
      tx_csum_partial: 0
      tx_csum_partial_inner: 0
      tx_queue_stopped: 0
      tx_queue_dropped: 0
      tx_xmit_more: 1
      tx_recover: 0
      tx_cqes: 323
      tx_queue_wake: 0
      tx_udp_seg_rem: 0
      tx_cqe_err: 0
      tx_xdp_xmit: 64926
      tx_xdp_full: 0
      tx_xdp_err: 0
      tx_xdp_cqes: 47958
      rx_wqe_err: 0
      rx_mpwqe_filler_cqes: 0
      rx_mpwqe_filler_strides: 0
      rx_buff_alloc_err: 0
      rx_cqe_compress_blks: 0
      rx_cqe_compress_pkts: 0
      rx_page_reuse: 0
      rx_cache_reuse: 648
      rx_cache_full: 0
      rx_cache_empty: 602112
      rx_cache_busy: 0
      rx_cache_waive: 0
      rx_congst_umr: 0
      rx_arfs_err: 0
      ch_events: 49628
      ch_poll: 49628
      ch_arm: 49626
      ch_aff_change: 0
      ch_eq_rearm: 0
      rx_out_of_buffer: 0
      rx_if_down_packets: 46
      rx_vport_unicast_packets: 5953
      rx_vport_unicast_bytes: 4927049
      tx_vport_unicast_packets: 65194
      tx_vport_unicast_bytes: 31820150
      rx_vport_multicast_packets: 37085249
      rx_vport_multicast_bytes: 2449620421
      tx_vport_multicast_packets: 55
      tx_vport_multicast_bytes: 4278
      rx_vport_broadcast_packets: 434654
      rx_vport_broadcast_bytes: 31881063
      tx_vport_broadcast_packets: 1
      tx_vport_broadcast_bytes: 60
      rx_vport_rdma_unicast_packets: 0
      rx_vport_rdma_unicast_bytes: 0
      tx_vport_rdma_unicast_packets: 0
      tx_vport_rdma_unicast_bytes: 0
      rx_vport_rdma_multicast_packets: 0
      rx_vport_rdma_multicast_bytes: 0
      tx_vport_rdma_multicast_packets: 0
      tx_vport_rdma_multicast_bytes: 0
      tx_packets_phy: 65250
      rx_packets_phy: 37525857
      rx_crc_errors_phy: 0
      tx_bytes_phy: 32085488
      rx_bytes_phy: 2636532027
      tx_multicast_phy: 55
      tx_broadcast_phy: 1
      rx_multicast_phy: 37085250
      rx_broadcast_phy: 434654
      rx_in_range_len_errors_phy: 0
      rx_out_of_range_len_phy: 0
      rx_oversize_pkts_phy: 0
      rx_symbol_err_phy: 0
      tx_mac_control_phy: 0
      rx_mac_control_phy: 0
      rx_unsupported_op_phy: 0
      rx_pause_ctrl_phy: 0
      tx_pause_ctrl_phy: 0
      rx_discards_phy: 0
      tx_discards_phy: 0
      tx_errors_phy: 0
      rx_undersize_pkts_phy: 0
      rx_fragments_phy: 0
      rx_jabbers_phy: 0
      rx_64_bytes_phy: 63346
      rx_65_to_127_bytes_phy: 37434768
      rx_128_to_255_bytes_phy: 14088
      rx_256_to_511_bytes_phy: 10461
      rx_512_to_1023_bytes_phy: 96
      rx_1024_to_1518_bytes_phy: 1933
      rx_1519_to_2047_bytes_phy: 1165
      rx_2048_to_4095_bytes_phy: 0
      rx_4096_to_8191_bytes_phy: 0
      rx_8192_to_10239_bytes_phy: 0
      link_down_events_phy: 0
      rx_pcs_symbol_err_phy: 0
      rx_corrected_bits_phy: 5
      rx_err_lane_0_phy: 1
      rx_err_lane_1_phy: 0
      rx_err_lane_2_phy: 0
      rx_err_lane_3_phy: 4
      rx_buffer_passed_thres_phy: 0
      rx_pci_signal_integrity: 0
      tx_pci_signal_integrity: 82
      outbound_pci_stalled_rd: 0
      outbound_pci_stalled_wr: 0
      outbound_pci_stalled_rd_events: 0
      outbound_pci_stalled_wr_events: 0
      rx_prio0_bytes: 23157221
      rx_prio0_packets: 195789
      tx_prio0_bytes: 32085488
      tx_prio0_packets: 65250
      rx_prio1_bytes: 0
      rx_prio1_packets: 0
      tx_prio1_bytes: 0
      tx_prio1_packets: 0
      rx_prio2_bytes: 0
      rx_prio2_packets: 0
      tx_prio2_bytes: 0
      tx_prio2_packets: 0
      rx_prio3_bytes: 23397578
      rx_prio3_packets: 343182
      tx_prio3_bytes: 0
      tx_prio3_packets: 0
      rx_prio4_bytes: 0
      rx_prio4_packets: 0
      tx_prio4_bytes: 0
      tx_prio4_packets: 0
      rx_prio5_bytes: 0
      rx_prio5_packets: 0
      tx_prio5_bytes: 0
      tx_prio5_packets: 0
      rx_prio6_bytes: 14643472
      rx_prio6_packets: 203589
      tx_prio6_bytes: 0
      tx_prio6_packets: 0
      rx_prio7_bytes: 2575333474
      rx_prio7_packets: 36783293
      tx_prio7_bytes: 0
      tx_prio7_packets: 0
      tx_pause_storm_warning_events : 0
      tx_pause_storm_error_events: 0
      module_unplug: 0
      module_bus_stuck: 0
      module_high_temp: 0
      module_bad_shorted: 0


But wondering if any offloading now can do some things that we dont want 
for xdp

currently all offloads are enabled.
  ethtool -k enp175s0f0
Features for enp175s0f0:
rx-checksumming: on
tx-checksumming: on
         tx-checksum-ipv4: on
         tx-checksum-ip-generic: off [fixed]
         tx-checksum-ipv6: on
         tx-checksum-fcoe-crc: off [fixed]
         tx-checksum-sctp: off [fixed]
scatter-gather: on
         tx-scatter-gather: on
         tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: on
         tx-tcp-segmentation: on
         tx-tcp-ecn-segmentation: off [fixed]
         tx-tcp-mangleid-segmentation: off
         tx-tcp6-segmentation: on
udp-fragmentation-offload: off
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: on
tx-vlan-offload: on
ntuple-filters: off
receive-hashing: on
highdma: on [fixed]
rx-vlan-filter: on
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: on
tx-gre-csum-segmentation: on
tx-ipxip4-segmentation: off [fixed]
tx-ipxip6-segmentation: off [fixed]
tx-udp_tnl-segmentation: on
tx-udp_tnl-csum-segmentation: on
tx-gso-partial: on
tx-sctp-segmentation: off [fixed]
tx-esp-segmentation: off [fixed]
tx-udp-segmentation: on
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off
rx-all: off
tx-vlan-stag-hw-insert: on
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: on [fixed]
l2-fwd-offload: off [fixed]
hw-tc-offload: off
esp-hw-offload: off [fixed]
esp-tx-csum-hw-offload: off [fixed]
rx-udp_tunnel-port-offload: on
tls-hw-tx-offload: off [fixed]
tls-hw-rx-offload: off [fixed]
rx-gro-hw: off [fixed]
tls-hw-record: off [fixed]



Also at the time when xdp is forwarding 1/10 frame - same problem is 
with local input/output traffic - testing server is also responding to 
1/10 icmp request



>
>
>> ping 172.16.0.2 -i 0.1
>> PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
>> 64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.12 ms
>> 64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.20 ms
>> 64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=4.85 ms
>> 64 bytes from 172.16.0.2: icmp_seq=29 ttl=64 time=4.91 ms
>> 64 bytes from 172.16.0.2: icmp_seq=38 ttl=64 time=4.85 ms
>> 64 bytes from 172.16.0.2: icmp_seq=48 ttl=64 time=5.00 ms
>> ^C
>> --- 172.16.0.2 ping statistics ---
>> 55 packets transmitted, 6 received, 89% packet loss, time 5655ms
>> rtt min/avg/max/mdev = 4.850/4.992/5.203/0.145 ms
>>
>>
>> And again after some time back to normal
>>
>>   ping 172.16.0.2 -i 0.1
>> PING 172.16.0.2 (172.16.0.2) 56(84) bytes of data.
>> 64 bytes from 172.16.0.2: icmp_seq=1 ttl=64 time=5.02 ms
>> 64 bytes from 172.16.0.2: icmp_seq=2 ttl=64 time=5.06 ms
>> 64 bytes from 172.16.0.2: icmp_seq=3 ttl=64 time=5.19 ms
>> 64 bytes from 172.16.0.2: icmp_seq=4 ttl=64 time=5.07 ms
>> 64 bytes from 172.16.0.2: icmp_seq=5 ttl=64 time=5.08 ms
>> 64 bytes from 172.16.0.2: icmp_seq=6 ttl=64 time=5.14 ms
>> 64 bytes from 172.16.0.2: icmp_seq=7 ttl=64 time=5.08 ms
>> 64 bytes from 172.16.0.2: icmp_seq=8 ttl=64 time=5.17 ms
>> 64 bytes from 172.16.0.2: icmp_seq=9 ttl=64 time=5.04 ms
>> 64 bytes from 172.16.0.2: icmp_seq=10 ttl=64 time=5.10 ms
>> 64 bytes from 172.16.0.2: icmp_seq=11 ttl=64 time=5.11 ms
>> 64 bytes from 172.16.0.2: icmp_seq=12 ttl=64 time=5.13 ms
>> 64 bytes from 172.16.0.2: icmp_seq=13 ttl=64 time=5.12 ms
>> 64 bytes from 172.16.0.2: icmp_seq=14 ttl=64 time=5.15 ms
>> 64 bytes from 172.16.0.2: icmp_seq=15 ttl=64 time=5.13 ms
>> 64 bytes from 172.16.0.2: icmp_seq=16 ttl=64 time=5.04 ms
>> 64 bytes from 172.16.0.2: icmp_seq=17 ttl=64 time=5.12 ms
>> 64 bytes from 172.16.0.2: icmp_seq=18 ttl=64 time=5.07 ms
>> 64 bytes from 172.16.0.2: icmp_seq=19 ttl=64 time=5.06 ms
>> 64 bytes from 172.16.0.2: icmp_seq=20 ttl=64 time=5.12 ms
>> 64 bytes from 172.16.0.2: icmp_seq=21 ttl=64 time=5.21 ms
>> 64 bytes from 172.16.0.2: icmp_seq=22 ttl=64 time=4.98 ms
>> ^C
>> --- 172.16.0.2 ping statistics ---
>> 22 packets transmitted, 22 received, 0% packet loss, time 2105ms
>> rtt min/avg/max/mdev = 4.988/5.104/5.210/0.089 ms
>>
>>
>> I will try to catch this with debug enabled
>>
>>
>>
>>
>>
>> Wondering also - cause xdp will bypass now vlan counters and other stuff
>> like tcpdump
> yes, xdp is before tcpdump based sockets.
>
> And the counters (vlan just being the current example) is another
> problem to be solved. The vlan net_device never sees the packet and you
> can not arbitrarily bump the counters just because the device lookups
> reference them.
Ok.

>> Is there possible to add only counters from xdp for vlans ?
>> This will help me in testing.
> I will take a look today at adding counters that you can dump using
> bpftool. It will be a temporary solution for this xdp program only.
Yes anything that can give me counters to check traffic lvls
>>
>> And also - for non lab scenario there should be possible to sniff
>> sometimes on interface :)
> Yes, sampling is another problem.
>
>
>> Soo wondering if need to attack another xdp program to interface or all
>> this can be done by one
>>
>> I think this is time where i will need to learn more about xdp :)
>>
>>
>

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Edward Cree @ 2018-11-09 20:00 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Martin KaFai Lau, Yonghong Song, Alexei Starovoitov,
	Daniel Borkmann, Network Development, Kernel Team
In-Reply-To: <20181109043507.dyemsy6va4wto67l@ast-mbp>

On 09/11/18 04:35, Alexei Starovoitov wrote:
> On Thu, Nov 08, 2018 at 10:56:55PM +0000, Edward Cree wrote:
>>  think this question of maps should be discussed in tomorrow's
>>  call, since it is when we start having other kinds of instances
> turned out most of us have a conflict, so the earliest is 1:30pm on Friday.
> still works for you?

Yep (that's 9.30pm GMT right?)

I'm assuming same bluejeans link again.

-Ed

^ permalink raw reply

* Re: [PATCH v2 net-next] net: phy: improve struct phy_device member interrupts handling
From: Andrew Lunn @ 2018-11-09 20:14 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <63a44097-c37d-eb0f-98c8-2c1fefd1f246@gmail.com>

On Fri, Nov 09, 2018 at 06:35:52PM +0100, Heiner Kallweit wrote:
> As a heritage from the very early days of phylib member interrupts is
> defined as u32 even though it's just a flag whether interrupts are
> enabled. So we can change it to a bitfield member. In addition change
> the code dealing with this member in a way that it's clear we're
> dealing with a bool value.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
From: Jakub Kicinski @ 2018-11-09 20:15 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet, guro,
	jiong.wang, bhole_prashant_q7, john.fastabend, jbenc,
	treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109162146.78019-1-sdf@google.com>

On Fri,  9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery

Thanks!  Patch 3 needs attention from maintainers but the rest LGTM!

^ permalink raw reply

* Re: [PATCH v2 net-next] net: phy: improve struct phy_device member interrupts handling
From: Florian Fainelli @ 2018-11-09 20:19 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <63a44097-c37d-eb0f-98c8-2c1fefd1f246@gmail.com>

On 11/9/18 9:35 AM, Heiner Kallweit wrote:
> As a heritage from the very early days of phylib member interrupts is
> defined as u32 even though it's just a flag whether interrupts are
> enabled. So we can change it to a bitfield member. In addition change
> the code dealing with this member in a way that it's clear we're
> dealing with a bool value.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] dpaa2-eth: defer probe on object allocate
From: Andrew Lunn @ 2018-11-09 20:39 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: netdev@vger.kernel.org, davem@davemloft.net,
	Ioana Ciocoi Radulescu
In-Reply-To: <1541777182-9135-2-git-send-email-ioana.ciornei@nxp.com>

On Fri, Nov 09, 2018 at 03:26:45PM +0000, Ioana Ciornei wrote:
> The fsl_mc_object_allocate function can fail because not all allocatable
> objects are probed by the fsl_mc_allocator at the call time. Defer the
> dpaa2-eth probe when this happens.
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v2:
>   - proper handling of IS_ERR_OR_NULL

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time
From: Arnd Bergmann @ 2018-11-09 21:06 UTC (permalink / raw)
  To: Sunil Kovvuri; +Cc: Networking, David Miller, linux-soc, Sunil Goutham
In-Reply-To: <CA+sq2CcEi4yfuwP2dN-pqOKrwdiEPBbVUGEwst1ayh6eKCpukA@mail.gmail.com>

On Fri, Nov 9, 2018 at 6:13 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Nov 9, 2018 at 4:32 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:

> >
> > Since b is aligned to four bytes, you get padding between a and b.
> > On top of that, you also get padding after c to make the size of
> > structure itself be a multiple of its alignment. For interfaces, we
> > should avoid both kinds of padding. This can be done by marking
> > members as __packed (usually I don't recommend that), by
> > changing the size of members, or by adding explicit 'reserved'
> > fields in place of the padding.
> >
> > > > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > > > use the 'pahole' tool to check for this kind of padding in the
> > > > API structures.
>
> Got your point now and agree that padding has to be avoided.
> But this is a big change and above pointed structure is not
> the only one as this applies to all structures in the file.
>
> Would it be okay if I submit a separate patch after this series
> addressing all structures ?

It depends on how you want to address it. If you want to
change the structure layout, then I think it would be better
integrated into the series as that is an incompatible interface
change. If you just want to add reserved members to make
the padding explicit, that could be a follow-up.

        Arnd

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Alexei Starovoitov @ 2018-11-09 21:14 UTC (permalink / raw)
  To: Edward Cree, Alexei Starovoitov
  Cc: Martin Lau, Yonghong Song, Daniel Borkmann, Network Development,
	Kernel Team
In-Reply-To: <fa9c91f8-3484-e141-6ade-de724d6c42f1@solarflare.com>

On 11/9/18 12:00 PM, Edward Cree wrote:
> On 09/11/18 04:35, Alexei Starovoitov wrote:
>> On Thu, Nov 08, 2018 at 10:56:55PM +0000, Edward Cree wrote:
>>>   think this question of maps should be discussed in tomorrow's
>>>   call, since it is when we start having other kinds of instances
>> turned out most of us have a conflict, so the earliest is 1:30pm on Friday.
>> still works for you?
> 
> Yep (that's 9.30pm GMT right?)
> 
> I'm assuming same bluejeans link again.

same link, but i cannot make it right now.
have to extinguish few fires.
may be at 2pm (unlikely) or 3pm (more likely) PST?

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Edward Cree @ 2018-11-09 21:28 UTC (permalink / raw)
  To: Alexei Starovoitov, Alexei Starovoitov
  Cc: Martin Lau, Yonghong Song, Daniel Borkmann, Network Development,
	Kernel Team
In-Reply-To: <78de238a-a923-ee59-601b-512f129f6c33@fb.com>

On 09/11/18 21:14, Alexei Starovoitov wrote:
> same link, but i cannot make it right now.
> have to extinguish few fires.
> may be at 2pm (unlikely) or 3pm (more likely) PST?

Yep I can do either of those, just let me know which when you can.

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Alexei Starovoitov @ 2018-11-09 21:48 UTC (permalink / raw)
  To: Edward Cree, Alexei Starovoitov
  Cc: Martin Lau, Yonghong Song, Daniel Borkmann, Network Development,
	Kernel Team
In-Reply-To: <73736f45-a7ea-b27a-3df5-bbf17eb945ac@solarflare.com>

On 11/9/18 1:28 PM, Edward Cree wrote:
> On 09/11/18 21:14, Alexei Starovoitov wrote:
>> same link, but i cannot make it right now.
>> have to extinguish few fires.
>> may be at 2pm (unlikely) or 3pm (more likely) PST?
> 
> Yep I can do either of those, just let me know which when you can.

still swamped. but see the light.
let's do 3pm



^ permalink raw reply

* Re: [PATCH net-next 0/4] Remove VLAN_TAG_PRESENT from drivers
From: Shiraz Saleem @ 2018-11-09 21:58 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: netdev, Claudiu Manoil, Faisal Latif, Pravin B Shelar, dev,
	linux-rdma
In-Reply-To: <cover.1541698641.git.mirq-linux@rere.qmqm.pl>

On Thu, Nov 08, 2018 at 06:44:46PM +0100, Michał Mirosław wrote:
> This series removes VLAN_TAG_PRESENT use from network drivers in
> preparation to removing its special meaning.
> 
> Michał Mirosław (4):
>   i40iw: remove use of VLAN_TAG_PRESENT
>   cnic: remove use of VLAN_TAG_PRESENT
>   gianfar: remove use of VLAN_TAG_PRESENT
>   OVS: remove use of VLAN_TAG_PRESENT
> 
>  drivers/infiniband/hw/i40iw/i40iw_cm.c        |  8 +++----
>

i40iw bit looks fine. Thanks!

Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>

^ permalink raw reply

* [RFC PATCH 09/10] net: hns3: Add "qos prio map" info query function
From: Salil Mehta @ 2018-11-09 22:07 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, andrew, yuvalm, leon, yisen.zhuang, lipeng321,
	mehta.salil, netdev, linux-kernel, linux-rdma, linuxarm,
	liuzhongzhu
In-Reply-To: <20181109220743.10264-1-salil.mehta@huawei.com>

From: liuzhongzhu <liuzhongzhu@huawei.com>

This patch prints qos priority map information.

debugfs command:
echo dump qos pri map > cmd

Sample Command:
root@(none)# echo dump qos pri map > cmd
hns3 0000:7d:00.0: dump qos pri map
hns3 0000:7d:00.0: vlan_to_pri: 0x0
hns3 0000:7d:00.0: pri_0_to_tc: 0x0
hns3 0000:7d:00.0: pri_1_to_tc: 0x0
hns3 0000:7d:00.0: pri_2_to_tc: 0x0
hns3 0000:7d:00.0: pri_3_to_tc: 0x0
hns3 0000:7d:00.0: pri_4_to_tc: 0x0
hns3 0000:7d:00.0: pri_5_to_tc: 0x0
hns3 0000:7d:00.0: pri_6_to_tc: 0x0
hns3 0000:7d:00.0: pri_7_to_tc: 0x0
root@(none)#

Signed-off-by: liuzhongzhu <liuzhongzhu@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c |  1 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c | 30 ++++++++++++++++++++++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.h | 13 ++++++++++
 3 files changed, 44 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 8edb4e9..6c0d237 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -134,6 +134,7 @@ static void hns3_dbg_help(struct hnae3_handle *h)
 	dev_info(&h->pdev->dev, "dump tm\n");
 	dev_info(&h->pdev->dev, "dump checksum\n");
 	dev_info(&h->pdev->dev, "dump qos pause cfg\n");
+	dev_info(&h->pdev->dev, "dump qos pri map\n");
 }
 
 static ssize_t hns3_dbg_cmd_read(struct file *filp, char __user *buffer,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index a71e9d0..b8921a8 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -364,6 +364,34 @@ static void hclge_dbg_dump_qos_pause_cfg(struct hclge_dev *hdev)
 		 pause_param->pause_trans_time);
 }
 
+static void hclge_dbg_dump_qos_pri_map(struct hclge_dev *hdev)
+{
+	struct hclge_qos_pri_map_cmd *pri_map;
+	struct hclge_desc desc;
+	int ret;
+
+	hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_PRI_TO_TC_MAPPING, true);
+
+	ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+	if (ret) {
+		dev_err(&hdev->pdev->dev,
+			"dump qos pri map fail, status is %d.\n", ret);
+		return;
+	}
+
+	pri_map = (struct hclge_qos_pri_map_cmd *)desc.data;
+	dev_info(&hdev->pdev->dev, "dump qos pri map\n");
+	dev_info(&hdev->pdev->dev, "vlan_to_pri: 0x%x\n", pri_map->vlan_pri);
+	dev_info(&hdev->pdev->dev, "pri_0_to_tc: 0x%x\n", pri_map->pri0_tc);
+	dev_info(&hdev->pdev->dev, "pri_1_to_tc: 0x%x\n", pri_map->pri1_tc);
+	dev_info(&hdev->pdev->dev, "pri_2_to_tc: 0x%x\n", pri_map->pri2_tc);
+	dev_info(&hdev->pdev->dev, "pri_3_to_tc: 0x%x\n", pri_map->pri3_tc);
+	dev_info(&hdev->pdev->dev, "pri_4_to_tc: 0x%x\n", pri_map->pri4_tc);
+	dev_info(&hdev->pdev->dev, "pri_5_to_tc: 0x%x\n", pri_map->pri5_tc);
+	dev_info(&hdev->pdev->dev, "pri_6_to_tc: 0x%x\n", pri_map->pri6_tc);
+	dev_info(&hdev->pdev->dev, "pri_7_to_tc: 0x%x\n", pri_map->pri7_tc);
+}
+
 static void hclge_dbg_fd_tcam_read(struct hclge_dev *hdev, u8 stage,
 				   bool sel_x, u32 loc)
 {
@@ -435,6 +463,8 @@ int hclge_dbg_run_cmd(struct hnae3_handle *handle, char *cmd_buf)
 		hclge_dbg_dump_checksum(hdev);
 	} else if (strncmp(cmd_buf, "dump qos pause cfg", 18) == 0) {
 		hclge_dbg_dump_qos_pause_cfg(hdev);
+	} else if (strncmp(cmd_buf, "dump qos pri map", 16) == 0) {
+		hclge_dbg_dump_qos_pri_map(hdev);
 	} else {
 		dev_info(&hdev->pdev->dev, "unknown command\n");
 		return -EINVAL;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.h
index 4ec9ced..6c839ee 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.h
@@ -8,4 +8,17 @@ struct hclge_checksum_cmd {
 	u8 outer;
 	u8 inner;
 };
+
+struct hclge_qos_pri_map_cmd {
+	u8 pri0_tc  : 4,
+	   pri1_tc  : 4;
+	u8 pri2_tc  : 4,
+	   pri3_tc  : 4;
+	u8 pri4_tc  : 4,
+	   pri5_tc  : 4;
+	u8 pri6_tc  : 4,
+	   pri7_tc  : 4;
+	u8 vlan_pri : 4,
+	   rev	    : 4;
+};
 #endif
-- 
2.7.4

^ permalink raw reply related

* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-09 22:20 UTC (permalink / raw)
  To: Saeed Mahameed, netdev@vger.kernel.org, Jesper Dangaard Brouer
In-Reply-To: <162e25c6-dae2-7e1e-75f0-9c5b22453495@itcare.pl>



W dniu 08.11.2018 o 20:12, Paweł Staszewski pisze:
> CPU load is lower than for connectx4 - but it looks like bandwidth 
> limit is the same :)
> But also after reaching 60Gbit/60Gbit
>
>  bwm-ng v0.6.1 (probing every 1.000s), press 'h' for help
>   input: /proc/net/dev type: rate
>   -         iface                   Rx Tx                Total
> ============================================================================== 
>
>          enp175s0:          45.09 Gb/s           15.09 Gb/s           
> 60.18 Gb/s
>          enp216s0:          15.14 Gb/s           45.19 Gb/s           
> 60.33 Gb/s
> ------------------------------------------------------------------------------ 
>
>             total:          60.45 Gb/s           60.48 Gb/s 120.93 Gb/s 

Today reached 65/65Gbit/s

But starting from 60Gbit/s RX / 60Gbit TX nics start to drop packets 
(with 50%CPU on all 28cores) - so still there is cpu power to use :).

So checked other stats.
softnet_stats shows average 1k squeezed per sec:
cpu      total    dropped   squeezed  collision        rps flow_limit
   0      18554          0          1          0          0 0
   1      16728          0          1          0          0 0
   2      18033          0          1          0          0 0
   3      17757          0          1          0          0 0
   4      18861          0          0          0          0 0
   5          0          0          1          0          0 0
   6          2          0          1          0          0 0
   7          0          0          1          0          0 0
   8          0          0          0          0          0 0
   9          0          0          1          0          0 0
  10          0          0          0          0          0 0
  11          0          0          1          0          0 0
  12         50          0          1          0          0 0
  13        257          0          0          0          0 0
  14 3629115363          0    3353259          0          0 0
  15  255167835          0    3138271          0          0 0
  16 4240101961          0    3036130          0          0 0
  17  599810018          0    3072169          0          0 0
  18  432796524          0    3034191          0          0 0
  19   41803906          0    3037405          0          0 0
  20  900382666          0    3112294          0          0 0
  21  620926085          0    3086009          0          0 0
  22   41861198          0    3023142          0          0 0
  23 4090425574          0    2990412          0          0 0
  24 4264870218          0    3010272          0          0 0
  25  141401811          0    3027153          0          0 0
  26  104155188          0    3051251          0          0 0
  27 4261258691          0    3039765          0          0 0
  28          4          0          1          0          0 0
  29          4          0          0          0          0 0
  30          0          0          1          0          0 0
  31          0          0          0          0          0 0
  32          3          0          1          0          0 0
  33          1          0          1          0          0 0
  34          0          0          1          0          0 0
  35          0          0          0          0          0 0
  36          0          0          1          0          0 0
  37          0          0          1          0          0 0
  38          0          0          1          0          0 0
  39          0          0          1          0          0 0
  40          0          0          0          0          0 0
  41          0          0          1          0          0 0
  42  299758202          0    3139693          0          0 0
  43 4254727979          0    3103577          0          0 0
  44 1959555543          0    2554885          0          0 0
  45 1675702723          0    2513481          0          0 0
  46 1908435503          0    2519698          0          0 0
  47 1877799710          0    2537768          0          0 0
  48 2384274076          0    2584673          0          0 0
  49 2598104878          0    2593616          0          0 0
  50 1897566829          0    2530857          0          0 0
  51 1712741629          0    2489089          0          0 0
  52 1704033648          0    2495892          0          0 0
  53 1636781820          0    2499783          0          0 0
  54 1861997734          0    2541060          0          0 0
  55 2113521616          0    2555673          0          0 0


So i rised netdev backlog and budged to rly high values
524288 for netdev_budget and same for backlog

This rised sortirqs from about 600k/sec to 800k/sec for NET_TX/NET_RX

But after this changes i have less packets drops.


Below perf top from max traffic reached:
    PerfTop:   72230 irqs/sec  kernel:99.4%  exact:  0.0% [4000Hz 
cycles],  (all, 56 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

     12.62%  [kernel]       [k] mlx5e_skb_from_cqe_mpwrq_linear
      8.44%  [kernel]       [k] mlx5e_sq_xmit
      6.69%  [kernel]       [k] build_skb
      5.21%  [kernel]       [k] fib_table_lookup
      3.54%  [kernel]       [k] memcpy_erms
      3.20%  [kernel]       [k] mlx5e_poll_rx_cq
      2.25%  [kernel]       [k] vlan_do_receive
      2.20%  [kernel]       [k] mlx5e_post_rx_mpwqes
      2.02%  [kernel]       [k] mlx5e_handle_rx_cqe_mpwrq
      1.95%  [kernel]       [k] __dev_queue_xmit
      1.83%  [kernel]       [k] dev_gro_receive
      1.79%  [kernel]       [k] tcp_gro_receive
      1.73%  [kernel]       [k] ip_finish_output2
      1.63%  [kernel]       [k] mlx5e_poll_tx_cq
      1.49%  [kernel]       [k] ipt_do_table
      1.38%  [kernel]       [k] inet_gro_receive
      1.31%  [kernel]       [k] __netif_receive_skb_core
      1.30%  [kernel]       [k] _raw_spin_lock
      1.28%  [kernel]       [k] mlx5_eq_int
      1.24%  [kernel]       [k] irq_entries_start
      1.19%  [kernel]       [k] __build_skb
      1.15%  [kernel]       [k] swiotlb_map_page
      1.02%  [kernel]       [k] vlan_dev_hard_start_xmit
      0.94%  [kernel]       [k] pfifo_fast_dequeue
      0.92%  [kernel]       [k] ip_route_input_rcu
      0.86%  [kernel]       [k] kmem_cache_alloc
      0.80%  [kernel]       [k] mlx5e_xmit
      0.79%  [kernel]       [k] dev_hard_start_xmit
      0.78%  [kernel]       [k] _raw_spin_lock_irqsave
      0.74%  [kernel]       [k] ip_forward
      0.72%  [kernel]       [k] tasklet_action_common.isra.21
      0.68%  [kernel]       [k] pfifo_fast_enqueue
      0.67%  [kernel]       [k] netif_skb_features
      0.66%  [kernel]       [k] skb_segment
      0.60%  [kernel]       [k] skb_gro_receive
      0.56%  [kernel]       [k] validate_xmit_skb.isra.142
      0.53%  [kernel]       [k] skb_release_data
      0.51%  [kernel]       [k] mlx5e_page_release
      0.51%  [kernel]       [k] ip_rcv_core.isra.20.constprop.25
      0.51%  [kernel]       [k] __qdisc_run
      0.50%  [kernel]       [k] tcp4_gro_receive
      0.49%  [kernel]       [k] page_frag_free
      0.46%  [kernel]       [k] kmem_cache_free_bulk
      0.43%  [kernel]       [k] kmem_cache_free
      0.42%  [kernel]       [k] try_to_wake_up
      0.39%  [kernel]       [k] _raw_spin_lock_irq
      0.39%  [kernel]       [k] find_busiest_group
      0.37%  [kernel]       [k] __memcpy



Remember those tests are now on two separate connectx5 connected to two 
separate pcie x16  gen 3.0

^ permalink raw reply

* Re: [RFC PATCH 01/12] dt-bindings: soc: qcom: add IPA bindings
From: Alex Elder @ 2018-11-09 22:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rob Herring, Mark Rutland, davem, Arnd Bergmann, Bjorn Andersson,
	ilias.apalodimas, netdev, devicetree, linux-arm-msm, linux-soc,
	linux-arm-kernel, Linux Kernel Mailing List, syadagir, mjavid
In-Reply-To: <CABGGiswmpmSUmg9jEW7GnNtL2uXAN7jJOqFO5kG8adq71GuZpw@mail.gmail.com>

On 11/7/18 8:59 AM, Rob Herring wrote:
> On Tue, Nov 6, 2018 at 6:33 PM Alex Elder <elder@linaro.org> wrote:
>>
>> Add the binding definitions for the "qcom,ipa" and "qcom,rmnet-ipa"
>> device tree nodes.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
>> ---
>>  .../devicetree/bindings/soc/qcom/qcom,ipa.txt | 136 ++++++++++++++++++
>>  .../bindings/soc/qcom/qcom,rmnet-ipa.txt      |  15 ++
>>  2 files changed, 151 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt
>>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt
>> new file mode 100644
>> index 000000000000..d4d3d37df029
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ipa.txt
>> @@ -0,0 +1,136 @@
>> +Qualcomm IPA (IP Accelerator) Driver
> 
> Bindings are for h/w not drivers.

OK.  I'll drop " Driver".

>> +
>> +This binding describes the Qualcomm IPA.  The IPA is capable of offloading
>> +certain network processing tasks (e.g. filtering, routing, and NAT) from
>> +the main processor.  The IPA currently serves only as a network interface,
>> +providing access to an LTE network available via a modem.
>> +
>> +The IPA sits between multiple independent "execution environments,"
>> +including the AP subsystem (APSS) and the modem.  The IPA presents
>> +a Generic Software Interface (GSI) to each execution environment.
>> +The GSI is an integral part of the IPA, but it is logically isolated
>> +and has a distinct interrupt and a separately-defined address space.
>> +
>> +    ----------   -------------   ---------
>> +    |        |   |G|       |G|   |       |
>> +    |  APSS  |===|S|  IPA  |S|===| Modem |
>> +    |        |   |I|       |I|   |       |
>> +    ----------   -------------   ---------
>> +
>> +See also:
>> +  bindings/interrupt-controller/interrupts.txt
>> +  bindings/interconnect/interconnect.txt
>> +  bindings/soc/qcom/qcom,smp2p.txt
>> +  bindings/reserved-memory/reserved-memory.txt
>> +  bindings/clock/clock-bindings.txt
>> +
>> +All properties defined below are required.
>> +
>> +- compatible:
>> +       Must be one of the following compatible strings:
>> +               "qcom,ipa-sdm845-modem_init"
>> +               "qcom,ipa-sdm845-tz_init"
> 
> Normal order is <vendor>,<soc>-<ipblock>."

I'll use "qcom,sdm845-ipa-modem-init" and "qcom,sdm845-ipa-tz-init".
(Or just "qcom,sdm845-ipa", depending on the outcome of the discussion
below.)

> Don't use '_'.

OK.

> What's the difference between these 2? It can't be detected somehow?

There is some early initialization, including loading some firmware,
that must be done by trusted code.  That can be done by either Trust
Zone or the modem.  If it's done by the modem, there is an additional
step required during initialization so the modem can tell the AP
that it has done its part, and the AP can finish IPA initialization.

There  is no way of detecting (e.g. by probing hardware) which is
in effect so we use DT.  I discussed this with Bjorn, who said that
this was a situation seen elsewhere and that using compatible strings
was the way he suggested to address it.

> This might be better expressed as a property. Then if Trustzone
> initializes things, it can just add a property.

A Boolean property to distinguish them would be fine as well, but
I would like to address this "common" problem consistently.

Bjorn, would you please weigh in?

>> +
>> +-reg:
>> +       Resources specyfing the physical address spaces of the IPA and GSI.
> 
> typo
> 
>> +
>> +-reg-names:
>> +       The names of the address space ranges defined by the "reg" property.
>> +       Must be "ipa" and "gsi".
>> +
>> +- interrupts-extended:
> 
> Use 'interrupts' here and describe what they are and the order. What
> they are connected to (and the need for interrupts-extended) is
> outside the scope of this binding.

I used interrupts-extended because there were two interrupt parents
(a "normal" interrupt controller and the interrupt controller implemented
for SMP2P input).  A paragraph here:
    bindings/interrupt-controller/interrupts.txt
recommends "interrupts-extended" in that case.

I have no objection to using just "interrupts" but can you tell me what
I misunderstood?  It seems like I need to do "interrupts-extended".

>> +       Specifies the IRQs used by the IPA.  Four cells are required,
>> +       specifying: the IPA IRQ; the GSI IRQ; the clock query interrupt
>> +       from the modem; and the "ready for stage 2 initialization"
>> +       interrupt from the modem.  The first two are hardware IRQs; the
>> +       third and fourth are SMP2P input interrupts.
>> +
>> +- interrupt-names:
>> +       The names of the interrupts defined by the "interrupts-extended"
>> +       property.  Must be "ipa", "gsi", "ipa-clock-query", and
>> +       "ipa-post-init".
> 
> Format as one per line.

Done.  And I did this throughout the file where there was more than
one name.  One per line, no comma, no "and".

>> +
>> +- clocks:
>> +       Resource that defines the IPA core clock.
>> +
>> +- clock-names:
>> +       The name used for the IPA core clock.  Must be "core".
>> +
>> +- interconnects:
>> +       Specifies the interconnects used by the IPA.  Three cells are
>> +       required, specifying:  the path from the IPA to memory; from
>> +       IPA to internal (SoC resident) memory; and between the AP
>> +       subsystem and IPA for register access.
>> +
>> +- interconnect-names:
>> +       The names of the interconnects defined by the "interconnects"
>> +       property.  Must be "memory", "imem", and "config".
>> +
>> +- qcom,smem-states
>> +       The state bits used for SMP2P output.  Two cells must be specified.
>> +       The first indicates whether the value in the second bit is valid
>> +       (1 means valid).  The second, if valid, defines whether the IPA
>> +       clock is enabled (1 means enabled).
>> +
>> +- qcom,smem-state-names
>> +       The names of the state bits used for SMP2P output.  These must be
>> +       "ipa-clock-enabled-valid" and "ipa-clock-enabled".
>> +
>> +- memory-region
>> +       A phandle for a reserved memory area that holds the firmware passed
>> +       to Trust Zone for authentication.  (Note, this is required
>> +       only for "qcom,ipa-sdm845-tz_init".)
>> +
>> += EXAMPLE
>> +
>> +The following example represents the IPA present in the SDM845 SoC.  It
>> +shows portions of the "modem-smp2p" node to indicate its relationship
>> +with the interrupts and SMEM states used by the IPA.
>> +
>> +       modem-smp2p {
>> +               compatible = "qcom,smp2p";
>> +               . . .
>> +               ipa_smp2p_out: ipa-ap-to-modem {
>> +                       qcom,entry-name = "ipa";
>> +                       #qcom,smem-state-cells = <1>;
>> +               };
>> +
>> +               ipa_smp2p_in: ipa-modem-to-ap {
>> +                       qcom,entry-name = "ipa";
>> +                       interrupt-controller;
>> +                       #interrupt-cells = <2>;
>> +               };
>> +       };
>> +
>> +       ipa@1e00000 {
> 
> ipa@1e40000

Oops.  Fixed.

>> +               compatible = "qcom,ipa-sdm845-modem_init";
>> +
>> +               reg = <0x1e40000 0x34000>,
>> +                     <0x1e04000 0x2c000>;
>> +               reg-names = "ipa",
>> +                           "gsi";
>> +
>> +               interrupts-extended = <&intc 0 311 IRQ_TYPE_LEVEL_HIGH>,
>> +                                     <&intc 0 432 IRQ_TYPE_LEVEL_HIGH>,
>> +                                     <&ipa_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
>> +                                     <&ipa_smp2p_in 1 IRQ_TYPE_EDGE_RISING>;
>> +               interrupt-names = "ipa",
>> +                                 "gsi",
>> +                                 "ipa-clock-query",
>> +                                 "ipa-post-init";
>> +
>> +               clocks = <&rpmhcc RPMH_IPA_CLK>;
>> +               clock-names = "core";
>> +
>> +               interconnects = <&qnoc MASTER_IPA &qnoc SLAVE_EBI1>,
>> +                               <&qnoc MASTER_IPA &qnoc SLAVE_IMEM>,
>> +                               <&qnoc MASTER_APPSS_PROC &qnoc SLAVE_IPA_CFG>;
>> +               interconnect-names = "memory",
>> +                                    "imem",
>> +                                    "config";
>> +
>> +               qcom,smem-states = <&ipa_smp2p_out 0>,
>> +                                  <&ipa_smp2p_out 1>;
>> +               qcom,smem-state-names = "ipa-clock-enabled-valid",
>> +                                       "ipa-clock-enabled";
>> +       };
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt
>> new file mode 100644
>> index 000000000000..3d0b2aabefc7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rmnet-ipa.txt
>> @@ -0,0 +1,15 @@
>> +Qualcomm IPA RMNet Driver
>> +
>> +This binding describes the IPA RMNet driver, which is used to
>> +represent virtual interfaces available on the modem accessed via
>> +the IPA.  Other than the compatible string there are no properties
>> +associated with this device.
> 
> Only a compatible string is a sure sign this is not a h/w device and
> you are just abusing DT to instantiate drivers. Make the IPA driver
> instantiate any sub drivers it needs.

Yeah I have been thinking this but hadn't followed through on
doing anything about it yet.  I'll remove this node entirely.
It's possible it had other properties at one time, but in the
end this  represents a soft interface and can be implemented
within the IPA driver.

Thanks a lot for the review.

					-Alex
> 
> Rob
> 

^ permalink raw reply

* [PATCH net] net: sched: cls_flower: validate nested enc_opts_policy to avoid build warning
From: Jakub Kicinski @ 2018-11-09 22:41 UTC (permalink / raw)
  To: davem; +Cc: oss-drivers, netdev, Jakub Kicinski

TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
currently contain further nested attributes, which are parsed by
hand, so the policy is never actually used.  Add the validation
anyway to avoid potential bugs when other attributes are added
and to make the attribute structure slightly more clear.  Validation
will also set extact to point to bad attribute on error.

Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_flower.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9aada2d0ef06..c6c327874abc 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -709,11 +709,23 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key,
 			  struct netlink_ext_ack *extack)
 {
 	const struct nlattr *nla_enc_key, *nla_opt_key, *nla_opt_msk = NULL;
-	int option_len, key_depth, msk_depth = 0;
+	int err, option_len, key_depth, msk_depth = 0;
+
+	err = nla_validate_nested(tb[TCA_FLOWER_KEY_ENC_OPTS],
+				  TCA_FLOWER_KEY_ENC_OPTS_MAX,
+				  enc_opts_policy, extack);
+	if (err)
+		return err;
 
 	nla_enc_key = nla_data(tb[TCA_FLOWER_KEY_ENC_OPTS]);
 
 	if (tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
+		err = nla_validate_nested(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK],
+					  TCA_FLOWER_KEY_ENC_OPTS_MAX,
+					  enc_opts_policy, extack);
+		if (err)
+			return err;
+
 		nla_opt_msk = nla_data(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]);
 		msk_depth = nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]);
 	}
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 04/23] linux/net.h: use unique identifier for each struct _ddebug
From: Rasmus Villemoes @ 2018-11-09 23:10 UTC (permalink / raw)
  To: Andrew Morton, Jason Baron; +Cc: linux-kernel, Rasmus Villemoes, netdev
In-Reply-To: <20181109231021.11658-1-linux@rasmusvillemoes.dk>

Changes on x86-64 later in this series require that all struct _ddebug
descriptors in a translation unit uses distinct identifiers. Realize
that for net_dbg_ratelimited by generating such an identifier via
__UNIQUE_ID and pass that to an extra level of macros.

No functional change.

Cc: netdev@vger.kernel.org
Acked-by: Jason Baron <jbaron@akamai.com>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/net.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index 651fca72286c..397243a25f56 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -260,7 +260,7 @@ do {								\
 #define net_info_ratelimited(fmt, ...)				\
 	net_ratelimited_function(pr_info, fmt, ##__VA_ARGS__)
 #if defined(CONFIG_DYNAMIC_DEBUG)
-#define net_dbg_ratelimited(fmt, ...)					\
+#define _net_dbg_ratelimited(descriptor, fmt, ...)			\
 do {									\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
 	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
@@ -268,6 +268,8 @@ do {									\
 		__dynamic_pr_debug(&descriptor, pr_fmt(fmt),		\
 		                   ##__VA_ARGS__);			\
 } while (0)
+#define net_dbg_ratelimited(fmt, ...)					\
+	_net_dbg_ratelimited(__UNIQUE_ID(ddebug), fmt, ##__VA_ARGS__)
 #elif defined(DEBUG)
 #define net_dbg_ratelimited(fmt, ...)				\
 	net_ratelimited_function(pr_debug, fmt, ##__VA_ARGS__)
-- 
2.19.1.6.gbde171bbf5

^ permalink raw reply related

* Re: [PATCH][net-next] net: tcp: remove BUG_ON from tcp_v4_err
From: David Miller @ 2018-11-09 23:17 UTC (permalink / raw)
  To: lirongqing; +Cc: netdev
In-Reply-To: <1541754291-8659-1-git-send-email-lirongqing@baidu.com>

From: Li RongQing <lirongqing@baidu.com>
Date: Fri,  9 Nov 2018 17:04:51 +0800

> if skb is NULL pointer, and the following access of skb's
> skb_mstamp_ns will trigger panic, which is same as BUG_ON
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Applied.

^ 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