Netdev List
 help / color / mirror / Atom feed
* [PATCH v4 bpf-next 2/6] bpf: Hooks for sys_sendmsg
From: Andrey Ignatov @ 2018-05-25 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, davem, kafai, ast, daniel, kernel-team
In-Reply-To: <cover.1527263217.git.rdna@fb.com>

In addition to already existing BPF hooks for sys_bind and sys_connect,
the patch provides new hooks for sys_sendmsg.

It leverages existing BPF program type `BPF_PROG_TYPE_CGROUP_SOCK_ADDR`
that provides access to socket itlself (properties like family, type,
protocol) and user-passed `struct sockaddr *` so that BPF program can
override destination IP and port for system calls such as sendto(2) or
sendmsg(2) and/or assign source IP to the socket.

The hooks are implemented as two new attach types:
`BPF_CGROUP_UDP4_SENDMSG` and `BPF_CGROUP_UDP6_SENDMSG` for UDPv4 and
UDPv6 correspondingly.

UDPv4 and UDPv6 separate attach types for same reason as sys_bind and
sys_connect hooks, i.e. to prevent reading from / writing to e.g.
user_ip6 fields when user passes sockaddr_in since it'd be out-of-bound.

The difference with already existing hooks is sys_sendmsg are
implemented only for unconnected UDP.

For TCP it doesn't make sense to change user-provided `struct sockaddr *`
at sendto(2)/sendmsg(2) time since socket either was already connected
and has source/destination set or wasn't connected and call to
sendto(2)/sendmsg(2) would lead to ENOTCONN anyway.

Connected UDP is already handled by sys_connect hooks that can override
source/destination at connect time and use fast-path later, i.e. these
hooks don't affect UDP fast-path.

Rewriting source IP is implemented differently than that in sys_connect
hooks. When sys_sendmsg is used with unconnected UDP it doesn't work to
just bind socket to desired local IP address since source IP can be set
on per-packet basis by using ancillary data (cmsg(3)). So no matter if
socket is bound or not, source IP has to be rewritten on every call to
sys_sendmsg.

To do so two new fields are added to UAPI `struct bpf_sock_addr`;
* `msg_src_ip4` to set source IPv4 for UDPv4;
* `msg_src_ip6` to set source IPv6 for UDPv6.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf-cgroup.h | 23 +++++++++++++++++------
 include/linux/filter.h     |  1 +
 include/uapi/linux/bpf.h   |  8 ++++++++
 kernel/bpf/cgroup.c        | 11 ++++++++++-
 kernel/bpf/syscall.c       |  8 ++++++++
 net/core/filter.c          | 39 +++++++++++++++++++++++++++++++++++++++
 net/ipv4/udp.c             | 20 ++++++++++++++++++--
 net/ipv6/udp.c             | 24 ++++++++++++++++++++++++
 8 files changed, 125 insertions(+), 9 deletions(-)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index de8e89a..975fb4c 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -66,7 +66,8 @@ int __cgroup_bpf_run_filter_sk(struct sock *sk,
 
 int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 				      struct sockaddr *uaddr,
-				      enum bpf_attach_type type);
+				      enum bpf_attach_type type,
+				      void *t_ctx);
 
 int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
 				     struct bpf_sock_ops_kern *sock_ops,
@@ -120,16 +121,18 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
 ({									       \
 	int __ret = 0;							       \
 	if (cgroup_bpf_enabled)						       \
-		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, type);    \
+		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, type,     \
+							  NULL);	       \
 	__ret;								       \
 })
 
-#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, type)			       \
+#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, type, t_ctx)		       \
 ({									       \
 	int __ret = 0;							       \
 	if (cgroup_bpf_enabled)	{					       \
 		lock_sock(sk);						       \
-		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, type);    \
+		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, type,     \
+							  t_ctx);	       \
 		release_sock(sk);					       \
 	}								       \
 	__ret;								       \
@@ -151,10 +154,16 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
 	BPF_CGROUP_RUN_SA_PROG(sk, uaddr, BPF_CGROUP_INET6_CONNECT)
 
 #define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_INET4_CONNECT)
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_INET4_CONNECT, NULL)
 
 #define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_INET6_CONNECT)
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_INET6_CONNECT, NULL)
+
+#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, t_ctx)		       \
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_UDP4_SENDMSG, t_ctx)
+
+#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, t_ctx)		       \
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, BPF_CGROUP_UDP6_SENDMSG, t_ctx)
 
 #define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops)				       \
 ({									       \
@@ -198,6 +207,8 @@ static inline int cgroup_bpf_inherit(struct cgroup *cgrp) { return 0; }
 #define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, t_ctx) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, t_ctx) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type,major,minor,access) ({ 0; })
 
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d358d18..d90abda 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1010,6 +1010,7 @@ struct bpf_sock_addr_kern {
 	 * only two (src and dst) are available at convert_ctx_access time
 	 */
 	u64 tmp_reg;
+	void *t_ctx;	/* Attach type specific context. */
 };
 
 struct bpf_sock_ops_kern {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9b8c6e3..cc68787 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -160,6 +160,8 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET6_CONNECT,
 	BPF_CGROUP_INET4_POST_BIND,
 	BPF_CGROUP_INET6_POST_BIND,
+	BPF_CGROUP_UDP4_SENDMSG,
+	BPF_CGROUP_UDP6_SENDMSG,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -2363,6 +2365,12 @@ struct bpf_sock_addr {
 	__u32 family;		/* Allows 4-byte read, but no write */
 	__u32 type;		/* Allows 4-byte read, but no write */
 	__u32 protocol;		/* Allows 4-byte read, but no write */
+	__u32 msg_src_ip4;	/* Allows 1,2,4-byte read an 4-byte write.
+				 * Stored in network byte order.
+				 */
+	__u32 msg_src_ip6[4];	/* Allows 1,2,4-byte read an 4-byte write.
+				 * Stored in network byte order.
+				 */
 };
 
 /* User bpf_sock_ops struct to access socket values and specify request ops
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 43171a0..f7c00bd 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -500,6 +500,7 @@ EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
  * @sk: sock struct that will use sockaddr
  * @uaddr: sockaddr struct provided by user
  * @type: The type of program to be exectuted
+ * @t_ctx: Pointer to attach type specific context
  *
  * socket is expected to be of type INET or INET6.
  *
@@ -508,12 +509,15 @@ EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
  */
 int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 				      struct sockaddr *uaddr,
-				      enum bpf_attach_type type)
+				      enum bpf_attach_type type,
+				      void *t_ctx)
 {
 	struct bpf_sock_addr_kern ctx = {
 		.sk = sk,
 		.uaddr = uaddr,
+		.t_ctx = t_ctx,
 	};
+	struct sockaddr_storage unspec;
 	struct cgroup *cgrp;
 	int ret;
 
@@ -523,6 +527,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 	if (sk->sk_family != AF_INET && sk->sk_family != AF_INET6)
 		return 0;
 
+	if (!ctx.uaddr) {
+		memset(&unspec, 0, sizeof(unspec));
+		ctx.uaddr = (struct sockaddr *)&unspec;
+	}
+
 	cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
 	ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], &ctx, BPF_PROG_RUN);
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 388d4fe..e254526 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1249,6 +1249,8 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
 		case BPF_CGROUP_INET6_BIND:
 		case BPF_CGROUP_INET4_CONNECT:
 		case BPF_CGROUP_INET6_CONNECT:
+		case BPF_CGROUP_UDP4_SENDMSG:
+		case BPF_CGROUP_UDP6_SENDMSG:
 			return 0;
 		default:
 			return -EINVAL;
@@ -1565,6 +1567,8 @@ static int bpf_prog_attach(const union bpf_attr *attr)
 	case BPF_CGROUP_INET6_BIND:
 	case BPF_CGROUP_INET4_CONNECT:
 	case BPF_CGROUP_INET6_CONNECT:
+	case BPF_CGROUP_UDP4_SENDMSG:
+	case BPF_CGROUP_UDP6_SENDMSG:
 		ptype = BPF_PROG_TYPE_CGROUP_SOCK_ADDR;
 		break;
 	case BPF_CGROUP_SOCK_OPS:
@@ -1635,6 +1639,8 @@ static int bpf_prog_detach(const union bpf_attr *attr)
 	case BPF_CGROUP_INET6_BIND:
 	case BPF_CGROUP_INET4_CONNECT:
 	case BPF_CGROUP_INET6_CONNECT:
+	case BPF_CGROUP_UDP4_SENDMSG:
+	case BPF_CGROUP_UDP6_SENDMSG:
 		ptype = BPF_PROG_TYPE_CGROUP_SOCK_ADDR;
 		break;
 	case BPF_CGROUP_SOCK_OPS:
@@ -1692,6 +1698,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
 	case BPF_CGROUP_INET6_POST_BIND:
 	case BPF_CGROUP_INET4_CONNECT:
 	case BPF_CGROUP_INET6_CONNECT:
+	case BPF_CGROUP_UDP4_SENDMSG:
+	case BPF_CGROUP_UDP6_SENDMSG:
 	case BPF_CGROUP_SOCK_OPS:
 	case BPF_CGROUP_DEVICE:
 		break;
diff --git a/net/core/filter.c b/net/core/filter.c
index acf1f4f..24e6ce8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5299,6 +5299,7 @@ static bool sock_addr_is_valid_access(int off, int size,
 		switch (prog->expected_attach_type) {
 		case BPF_CGROUP_INET4_BIND:
 		case BPF_CGROUP_INET4_CONNECT:
+		case BPF_CGROUP_UDP4_SENDMSG:
 			break;
 		default:
 			return false;
@@ -5308,6 +5309,24 @@ static bool sock_addr_is_valid_access(int off, int size,
 		switch (prog->expected_attach_type) {
 		case BPF_CGROUP_INET6_BIND:
 		case BPF_CGROUP_INET6_CONNECT:
+		case BPF_CGROUP_UDP6_SENDMSG:
+			break;
+		default:
+			return false;
+		}
+		break;
+	case bpf_ctx_range(struct bpf_sock_addr, msg_src_ip4):
+		switch (prog->expected_attach_type) {
+		case BPF_CGROUP_UDP4_SENDMSG:
+			break;
+		default:
+			return false;
+		}
+		break;
+	case bpf_ctx_range_till(struct bpf_sock_addr, msg_src_ip6[0],
+				msg_src_ip6[3]):
+		switch (prog->expected_attach_type) {
+		case BPF_CGROUP_UDP6_SENDMSG:
 			break;
 		default:
 			return false;
@@ -5318,6 +5337,9 @@ static bool sock_addr_is_valid_access(int off, int size,
 	switch (off) {
 	case bpf_ctx_range(struct bpf_sock_addr, user_ip4):
 	case bpf_ctx_range_till(struct bpf_sock_addr, user_ip6[0], user_ip6[3]):
+	case bpf_ctx_range(struct bpf_sock_addr, msg_src_ip4):
+	case bpf_ctx_range_till(struct bpf_sock_addr, msg_src_ip6[0],
+				msg_src_ip6[3]):
 		/* Only narrow read access allowed for now. */
 		if (type == BPF_READ) {
 			bpf_ctx_record_field_size(info, size_default);
@@ -6072,6 +6094,23 @@ static u32 sock_addr_convert_ctx_access(enum bpf_access_type type,
 		*insn++ = BPF_ALU32_IMM(BPF_RSH, si->dst_reg,
 					SK_FL_PROTO_SHIFT);
 		break;
+
+	case offsetof(struct bpf_sock_addr, msg_src_ip4):
+		/* Treat t_ctx as struct in_addr for msg_src_ip4. */
+		SOCK_ADDR_LOAD_OR_STORE_NESTED_FIELD_SIZE_OFF(
+			struct bpf_sock_addr_kern, struct in_addr, t_ctx,
+			s_addr, BPF_SIZE(si->code), 0, tmp_reg);
+		break;
+
+	case bpf_ctx_range_till(struct bpf_sock_addr, msg_src_ip6[0],
+				msg_src_ip6[3]):
+		off = si->off;
+		off -= offsetof(struct bpf_sock_addr, msg_src_ip6[0]);
+		/* Treat t_ctx as struct in6_addr for msg_src_ip6. */
+		SOCK_ADDR_LOAD_OR_STORE_NESTED_FIELD_SIZE_OFF(
+			struct bpf_sock_addr_kern, struct in6_addr, t_ctx,
+			s6_addr32[0], BPF_SIZE(si->code), off, tmp_reg);
+		break;
 	}
 
 	return insn - insn_buf;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d71f1f3..3c27d00 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -901,6 +901,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct udp_sock *up = udp_sk(sk);
+	DECLARE_SOCKADDR(struct sockaddr_in *, usin, msg->msg_name);
 	struct flowi4 fl4_stack;
 	struct flowi4 *fl4;
 	int ulen = len;
@@ -955,8 +956,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	/*
 	 *	Get and verify the address.
 	 */
-	if (msg->msg_name) {
-		DECLARE_SOCKADDR(struct sockaddr_in *, usin, msg->msg_name);
+	if (usin) {
 		if (msg->msg_namelen < sizeof(*usin))
 			return -EINVAL;
 		if (usin->sin_family != AF_INET) {
@@ -1010,6 +1010,22 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		rcu_read_unlock();
 	}
 
+	if (cgroup_bpf_enabled && !connected) {
+		err = BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk,
+					    (struct sockaddr *)usin, &ipc.addr);
+		if (err)
+			goto out_free;
+		if (usin) {
+			if (usin->sin_port == 0) {
+				/* BPF program set invalid port. Reject it. */
+				err = -EINVAL;
+				goto out_free;
+			}
+			daddr = usin->sin_addr.s_addr;
+			dport = usin->sin_port;
+		}
+	}
+
 	saddr = ipc.addr;
 	ipc.addr = faddr = daddr;
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 426c9d2..9f729a7 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1316,6 +1316,29 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		fl6.saddr = np->saddr;
 	fl6.fl6_sport = inet->inet_sport;
 
+	if (cgroup_bpf_enabled && !connected) {
+		err = BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk,
+					   (struct sockaddr *)sin6, &fl6.saddr);
+		if (err)
+			goto out_no_dst;
+		if (sin6) {
+			if (ipv6_addr_v4mapped(&sin6->sin6_addr)) {
+				/* BPF program rewrote IPv6-only by IPv4-mapped
+				 * IPv6. It's currently unsupported.
+				 */
+				err = -ENOTSUPP;
+				goto out_no_dst;
+			}
+			if (sin6->sin6_port == 0) {
+				/* BPF program set invalid port. Reject it. */
+				err = -EINVAL;
+				goto out_no_dst;
+			}
+			fl6.fl6_dport = sin6->sin6_port;
+			fl6.daddr = sin6->sin6_addr;
+		}
+	}
+
 	final_p = fl6_update_dst(&fl6, opt, &final);
 	if (final_p)
 		connected = false;
@@ -1395,6 +1418,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 out:
 	dst_release(dst);
+out_no_dst:
 	fl6_sock_release(flowlabel);
 	txopt_put(opt_to_free);
 	if (!err)
-- 
2.9.5

^ permalink raw reply related

* [PATCH v4 bpf-next 0/6] bpf: Hooks for sys_sendmsg
From: Andrey Ignatov @ 2018-05-25 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, davem, kafai, ast, daniel, kernel-team

v3 -> v4:
* handle static key correctly for CONFIG_CGROUP_BPF=n.

v2 -> v3:
* place BPF logic under static key in udp_sendmsg, udpv6_sendmsg;
* rebase.

v1 -> v2:
* return ENOTSUPP if bpf_prog rewrote IPv6-only with IPv4-mapped IPv6;
* add test for IPv4-mapped IPv6 use-case;
* fix build for CONFIG_CGROUP_BPF=n;
* rebase.

This path set adds BPF hooks for sys_sendmsg similar to existing hooks for
sys_bind and sys_connect.

Hooks allow to override source IP (including the case when it's set via
cmsg(3)) and destination IP:port for unconnected UDP (slow path). TCP and
connected UDP (fast path) are not affected. This makes UDP support
complete: connected UDP is handled by sys_connect hooks, unconnected by
sys_sendmsg ones.

Similar to sys_connect hooks, sys_sendmsg ones can be used to make system
calls such as sendmsg(2) and sendto(2) return EPERM.

Please see patch 0002 for more details.


Andrey Ignatov (6):
  bpf: Define cgroup_bpf_enabled for CONFIG_CGROUP_BPF=n
  bpf: Hooks for sys_sendmsg
  bpf: Sync bpf.h to tools/
  libbpf: Support guessing sendmsg{4,6} progs
  selftests/bpf: Prepare test_sock_addr for extension
  selftests/bpf: Selftest for sys_sendmsg hooks

 include/linux/bpf-cgroup.h                   |   24 +-
 include/linux/filter.h                       |    1 +
 include/uapi/linux/bpf.h                     |    8 +
 kernel/bpf/cgroup.c                          |   11 +-
 kernel/bpf/syscall.c                         |    8 +
 net/core/filter.c                            |   39 +
 net/ipv4/udp.c                               |   20 +-
 net/ipv6/udp.c                               |   24 +
 tools/include/uapi/linux/bpf.h               |    8 +
 tools/lib/bpf/libbpf.c                       |    2 +
 tools/testing/selftests/bpf/Makefile         |    2 +-
 tools/testing/selftests/bpf/sendmsg4_prog.c  |   49 ++
 tools/testing/selftests/bpf/sendmsg6_prog.c  |   60 ++
 tools/testing/selftests/bpf/test_sock_addr.c | 1155 +++++++++++++++++++++-----
 14 files changed, 1215 insertions(+), 196 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/sendmsg4_prog.c
 create mode 100644 tools/testing/selftests/bpf/sendmsg6_prog.c

-- 
2.9.5

^ permalink raw reply

* [PATCH v4 bpf-next 3/6] bpf: Sync bpf.h to tools/
From: Andrey Ignatov @ 2018-05-25 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, davem, kafai, ast, daniel, kernel-team
In-Reply-To: <cover.1527263217.git.rdna@fb.com>

Sync new `BPF_CGROUP_UDP4_SENDMSG` and `BPF_CGROUP_UDP6_SENDMSG`
attach types to tools/.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/include/uapi/linux/bpf.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9b8c6e3..cc68787 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -160,6 +160,8 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET6_CONNECT,
 	BPF_CGROUP_INET4_POST_BIND,
 	BPF_CGROUP_INET6_POST_BIND,
+	BPF_CGROUP_UDP4_SENDMSG,
+	BPF_CGROUP_UDP6_SENDMSG,
 	__MAX_BPF_ATTACH_TYPE
 };
 
@@ -2363,6 +2365,12 @@ struct bpf_sock_addr {
 	__u32 family;		/* Allows 4-byte read, but no write */
 	__u32 type;		/* Allows 4-byte read, but no write */
 	__u32 protocol;		/* Allows 4-byte read, but no write */
+	__u32 msg_src_ip4;	/* Allows 1,2,4-byte read an 4-byte write.
+				 * Stored in network byte order.
+				 */
+	__u32 msg_src_ip6[4];	/* Allows 1,2,4-byte read an 4-byte write.
+				 * Stored in network byte order.
+				 */
 };
 
 /* User bpf_sock_ops struct to access socket values and specify request ops
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH net-next 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()
From: Vivien Didelot @ 2018-05-25 16:10 UTC (permalink / raw)
  To: Petr Machata, netdev, devel, bridge
  Cc: jiri, idosch, davem, razvan.stefanescu, gregkh, stephen, andrew,
	f.fainelli, nikolay
In-Reply-To: <af1bed5ce2b236544edd591b8cfb8425c2cb5ca0.1527173527.git.petrm@mellanox.com>

Hi Petr,

Petr Machata <petrm@mellanox.com> writes:

> -static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
> -			  u16 vid, u16 flags)
> +static int br_switchdev_port_obj_add(struct net_device *dev, u16 vid, u16 flags)
>  {
>  	struct switchdev_obj_port_vlan v = {
>  		.obj.orig_dev = dev,
> @@ -89,12 +88,29 @@ static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
>  		.vid_begin = vid,
>  		.vid_end = vid,
>  	};
> -	int err;
>  
> +	return switchdev_port_obj_add(dev, &v.obj);
> +}
> +
> +static int br_switchdev_port_obj_del(struct net_device *dev, u16 vid)
> +{
> +	struct switchdev_obj_port_vlan v = {
> +		.obj.orig_dev = dev,
> +		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
> +		.vid_begin = vid,
> +		.vid_end = vid,
> +	};
> +
> +	return switchdev_port_obj_del(dev, &v.obj);
> +}

Shouldn't they be br_switchdev_port_vlan_add (or similar) implemented in
net/bridge/br_switchdev.c instead, since they are VLAN specific?

Other than that, the change looks good!

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net-next 4/7] dsa: port: Ignore bridge VLAN events
From: Vivien Didelot @ 2018-05-25 16:11 UTC (permalink / raw)
  To: Petr Machata, netdev, devel, bridge
  Cc: jiri, idosch, davem, razvan.stefanescu, gregkh, stephen, andrew,
	f.fainelli, nikolay
In-Reply-To: <56beb523d39852fa30e8e90e80638b0ec917b96a.1527173527.git.petrm@mellanox.com>

Hi Petr,

Petr Machata <petrm@mellanox.com> writes:

> Ignore VLAN events where the orig_dev is the bridge device itself.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: missing headers test_lwt_seg6local
From: Y Song @ 2018-05-25 16:16 UTC (permalink / raw)
  To: Mathieu Xhonneux; +Cc: netdev, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180525112036.4768-1-m.xhonneux@gmail.com>

On Fri, May 25, 2018 at 4:20 AM, Mathieu Xhonneux <m.xhonneux@gmail.com> wrote:
> Previous patch "selftests/bpf: test for seg6local End.BPF action" lacks
> some UAPI headers in tools/.
>
> clang -I. -I./include/uapi -I../../../include/uapi -idirafter
> /usr/local/include -idirafter
> /data/users/yhs/work/llvm/build/install/lib/clang/7.0.0/include
> -idirafter /usr/include -Wno-compare-distinct-pointer-types \
>          -O2 -target bpf -emit-llvm -c test_lwt_seg6local.c -o - |      \
> llc -march=bpf -mcpu=generic  -filetype=obj -o
> [...]/net-next/tools/testing/selftests/bpf/test_lwt_seg6local.o
> test_lwt_seg6local.c:4:10: fatal error: 'linux/seg6_local.h' file not found
>          ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: Leaving directory
> `/data/users/yhs/work/net-next/tools/testing/selftests/bpf'
>
> Reported-by: Y Song <ys114321@gmail.com>
> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
> ---
>  .../selftests/bpf/include/uapi/linux/seg6.h        | 55 +++++++++++++++
>  .../selftests/bpf/include/uapi/linux/seg6_local.h  | 80 ++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6.h
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h

Thanks for fixing the issue.

Acked-by: Y Song <ys114321@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 6/7] net: bridge: Notify about bridge VLANs
From: Vivien Didelot @ 2018-05-25 16:17 UTC (permalink / raw)
  To: Petr Machata, netdev, devel, bridge
  Cc: jiri, idosch, davem, razvan.stefanescu, gregkh, stephen, andrew,
	f.fainelli, nikolay
In-Reply-To: <b0b6b46a239eb330d722cea89137ab6ec5bae6e3.1527173527.git.petrm@mellanox.com>

Hi Petr,

Petr Machata <petrm@mellanox.com> writes:

> A driver might need to react to changes in settings of brentry VLANs.
> Therefore send switchdev port notifications for these as well. Reuse
> SWITCHDEV_OBJ_ID_PORT_VLAN for this purpose. Listeners should use
> netif_is_bridge_master() on orig_dev to determine whether the
> notification is about a bridge port or a bridge.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>

> +	} else {
> +		err = br_switchdev_port_obj_add(dev, v->vid, flags);
> +		if (err && err != -EOPNOTSUPP)
> +			goto out;
>  	}

Except that br_switchdev_port_obj_add taking vid and flags arguments
seems confusing to me, the change looks good:

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net-next] bpfilter: fix a build err
From: Alexei Starovoitov @ 2018-05-25 16:19 UTC (permalink / raw)
  To: YueHaibing; +Cc: davem, ast, netdev, linux-kernel
In-Reply-To: <20180525101757.13756-1-yuehaibing@huawei.com>

On Fri, May 25, 2018 at 06:17:57PM +0800, YueHaibing wrote:
> gcc-7.3.0 report following err:
> 
>   HOSTCC  net/bpfilter/main.o
> In file included from net/bpfilter/main.c:9:0:
> ./include/uapi/linux/bpf.h:12:10: fatal error: linux/bpf_common.h: No such file or directory
>  #include <linux/bpf_common.h>
> 
> remove it by adding a include path.
> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  net/bpfilter/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
> index 2af752c..3f3cb87 100644
> --- a/net/bpfilter/Makefile
> +++ b/net/bpfilter/Makefile
> @@ -5,7 +5,7 @@
>  
>  hostprogs-y := bpfilter_umh
>  bpfilter_umh-objs := main.o
> -HOSTCFLAGS += -I. -Itools/include/
> +HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi

Strangely I don't see this error with gcc 7.3
I've tried this patch and it doesn't hurt,
but before it gets applied could you please try
the top two patches from this tree:
https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/?h=ipt_bpf
in your environment?
These two patches add the actual meat of bpfilter and I'd like
to make sure the build setup is good for everyone before
we proceed too far.

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: missing headers test_lwt_seg6local
From: Song Liu @ 2018-05-25 16:20 UTC (permalink / raw)
  To: Mathieu Xhonneux; +Cc: netdev, ys114321, daniel, alexei.starovoitov
In-Reply-To: <20180525112036.4768-1-m.xhonneux@gmail.com>

On Fri, May 25, 2018 at 4:20 AM, Mathieu Xhonneux <m.xhonneux@gmail.com> wrote:
> Previous patch "selftests/bpf: test for seg6local End.BPF action" lacks
> some UAPI headers in tools/.
>
> clang -I. -I./include/uapi -I../../../include/uapi -idirafter
> /usr/local/include -idirafter
> /data/users/yhs/work/llvm/build/install/lib/clang/7.0.0/include
> -idirafter /usr/include -Wno-compare-distinct-pointer-types \
>          -O2 -target bpf -emit-llvm -c test_lwt_seg6local.c -o - |      \
> llc -march=bpf -mcpu=generic  -filetype=obj -o
> [...]/net-next/tools/testing/selftests/bpf/test_lwt_seg6local.o
> test_lwt_seg6local.c:4:10: fatal error: 'linux/seg6_local.h' file not found
>          ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: Leaving directory
> `/data/users/yhs/work/net-next/tools/testing/selftests/bpf'
>
> Reported-by: Y Song <ys114321@gmail.com>
> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
> ---
>  .../selftests/bpf/include/uapi/linux/seg6.h        | 55 +++++++++++++++
>  .../selftests/bpf/include/uapi/linux/seg6_local.h  | 80 ++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6.h
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
>
> diff --git a/tools/testing/selftests/bpf/include/uapi/linux/seg6.h b/tools/testing/selftests/bpf/include/uapi/linux/seg6.h
> new file mode 100644
> index 000000000000..286e8d6a8e98
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/include/uapi/linux/seg6.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + *  SR-IPv6 implementation
> + *
> + *  Author:
> + *  David Lebrun <david.lebrun@uclouvain.be>
> + *
> + *
> + *  This program is free software; you can redistribute it and/or
> + *      modify it under the terms of the GNU General Public License
> + *      as published by the Free Software Foundation; either version
> + *      2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_SEG6_H
> +#define _UAPI_LINUX_SEG6_H
> +
> +#include <linux/types.h>
> +#include <linux/in6.h>         /* For struct in6_addr. */
> +
> +/*
> + * SRH
> + */
> +struct ipv6_sr_hdr {
> +       __u8    nexthdr;
> +       __u8    hdrlen;
> +       __u8    type;
> +       __u8    segments_left;
> +       __u8    first_segment; /* Represents the last_entry field of SRH */
> +       __u8    flags;
> +       __u16   tag;
> +
> +       struct in6_addr segments[0];
> +};
> +
> +#define SR6_FLAG1_PROTECTED    (1 << 6)
> +#define SR6_FLAG1_OAM          (1 << 5)
> +#define SR6_FLAG1_ALERT                (1 << 4)
> +#define SR6_FLAG1_HMAC         (1 << 3)
> +
> +#define SR6_TLV_INGRESS                1
> +#define SR6_TLV_EGRESS         2
> +#define SR6_TLV_OPAQUE         3
> +#define SR6_TLV_PADDING                4
> +#define SR6_TLV_HMAC           5
> +
> +#define sr_has_hmac(srh) ((srh)->flags & SR6_FLAG1_HMAC)
> +
> +struct sr6_tlv {
> +       __u8 type;
> +       __u8 len;
> +       __u8 data[0];
> +};
> +
> +#endif
> diff --git a/tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h b/tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
> new file mode 100644
> index 000000000000..edc138bdc56d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
> @@ -0,0 +1,80 @@
> +/*
> + *  SR-IPv6 implementation
> + *
> + *  Author:
> + *  David Lebrun <david.lebrun@uclouvain.be>
> + *
> + *
> + *  This program is free software; you can redistribute it and/or
> + *      modify it under the terms of the GNU General Public License
> + *      as published by the Free Software Foundation; either version
> + *      2 of the License, or (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_SEG6_LOCAL_H
> +#define _UAPI_LINUX_SEG6_LOCAL_H
> +
> +#include <linux/seg6.h>
> +
> +enum {
> +       SEG6_LOCAL_UNSPEC,
> +       SEG6_LOCAL_ACTION,
> +       SEG6_LOCAL_SRH,
> +       SEG6_LOCAL_TABLE,
> +       SEG6_LOCAL_NH4,
> +       SEG6_LOCAL_NH6,
> +       SEG6_LOCAL_IIF,
> +       SEG6_LOCAL_OIF,
> +       SEG6_LOCAL_BPF,
> +       __SEG6_LOCAL_MAX,
> +};
> +#define SEG6_LOCAL_MAX (__SEG6_LOCAL_MAX - 1)
> +
> +enum {
> +       SEG6_LOCAL_ACTION_UNSPEC        = 0,
> +       /* node segment */
> +       SEG6_LOCAL_ACTION_END           = 1,
> +       /* adjacency segment (IPv6 cross-connect) */
> +       SEG6_LOCAL_ACTION_END_X         = 2,
> +       /* lookup of next seg NH in table */
> +       SEG6_LOCAL_ACTION_END_T         = 3,
> +       /* decap and L2 cross-connect */
> +       SEG6_LOCAL_ACTION_END_DX2       = 4,
> +       /* decap and IPv6 cross-connect */
> +       SEG6_LOCAL_ACTION_END_DX6       = 5,
> +       /* decap and IPv4 cross-connect */
> +       SEG6_LOCAL_ACTION_END_DX4       = 6,
> +       /* decap and lookup of DA in v6 table */
> +       SEG6_LOCAL_ACTION_END_DT6       = 7,
> +       /* decap and lookup of DA in v4 table */
> +       SEG6_LOCAL_ACTION_END_DT4       = 8,
> +       /* binding segment with insertion */
> +       SEG6_LOCAL_ACTION_END_B6        = 9,
> +       /* binding segment with encapsulation */
> +       SEG6_LOCAL_ACTION_END_B6_ENCAP  = 10,
> +       /* binding segment with MPLS encap */
> +       SEG6_LOCAL_ACTION_END_BM        = 11,
> +       /* lookup last seg in table */
> +       SEG6_LOCAL_ACTION_END_S         = 12,
> +       /* forward to SR-unaware VNF with static proxy */
> +       SEG6_LOCAL_ACTION_END_AS        = 13,
> +       /* forward to SR-unaware VNF with masquerading */
> +       SEG6_LOCAL_ACTION_END_AM        = 14,
> +       /* custom BPF action */
> +       SEG6_LOCAL_ACTION_END_BPF       = 15,
> +
> +       __SEG6_LOCAL_ACTION_MAX,
> +};
> +
> +#define SEG6_LOCAL_ACTION_MAX (__SEG6_LOCAL_ACTION_MAX - 1)
> +
> +enum {
> +       SEG6_LOCAL_BPF_PROG_UNSPEC,
> +       SEG6_LOCAL_BPF_PROG,
> +       SEG6_LOCAL_BPF_PROG_NAME,
> +       __SEG6_LOCAL_BPF_PROG_MAX,
> +};
> +
> +#define SEG6_LOCAL_BPF_PROG_MAX (__SEG6_LOCAL_BPF_PROG_MAX - 1)
> +
> +#endif
> --
> 2.16.1
>

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: missing headers test_lwt_seg6local
From: Y Song @ 2018-05-25 16:22 UTC (permalink / raw)
  To: Mathieu Xhonneux; +Cc: netdev, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CAH3MdRXW_nPi9Hup_a4vMRZx0S=eTuVxO8XSpdDTFr0zzXWVHg@mail.gmail.com>

On Fri, May 25, 2018 at 9:16 AM, Y Song <ys114321@gmail.com> wrote:
> On Fri, May 25, 2018 at 4:20 AM, Mathieu Xhonneux <m.xhonneux@gmail.com> wrote:
>> Previous patch "selftests/bpf: test for seg6local End.BPF action" lacks
>> some UAPI headers in tools/.
>>
>> clang -I. -I./include/uapi -I../../../include/uapi -idirafter
>> /usr/local/include -idirafter
>> /data/users/yhs/work/llvm/build/install/lib/clang/7.0.0/include
>> -idirafter /usr/include -Wno-compare-distinct-pointer-types \
>>          -O2 -target bpf -emit-llvm -c test_lwt_seg6local.c -o - |      \
>> llc -march=bpf -mcpu=generic  -filetype=obj -o
>> [...]/net-next/tools/testing/selftests/bpf/test_lwt_seg6local.o
>> test_lwt_seg6local.c:4:10: fatal error: 'linux/seg6_local.h' file not found
>>          ^~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>> make: Leaving directory
>> `/data/users/yhs/work/net-next/tools/testing/selftests/bpf'
>>
>> Reported-by: Y Song <ys114321@gmail.com>
>> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
>> ---
>>  .../selftests/bpf/include/uapi/linux/seg6.h        | 55 +++++++++++++++
>>  .../selftests/bpf/include/uapi/linux/seg6_local.h  | 80 ++++++++++++++++++++++
>>  2 files changed, 135 insertions(+)
>>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6.h
>>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
>
> Thanks for fixing the issue.
>
> Acked-by: Y Song <ys114321@gmail.com>

Although it fixed the issue, the file is placed in
tools/testing/selftests/bpf/include/uapi/linux
directory. Considering the file is really coming from
linux/include/uapi/linux directory, should it
be placed in tools/include/uapi/linux directory instead?

^ permalink raw reply

* Re: [PATCH 1/7] core, dma-direct: add a flag 32-bit dma limits
From: Christoph Hellwig @ 2018-05-25 16:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Fenghua Yu, Tony Luck, linux-ia64-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Ingo Molnar,
	Thomas Gleixner, Christoph Hellwig
In-Reply-To: <20180525145012.GA3863-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

On Fri, May 25, 2018 at 04:50:12PM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 25, 2018 at 04:35:06PM +0200, Christoph Hellwig wrote:
> > Various PCI bridges (VIA PCI, Xilinx PCIe) limit DMA to only 32-bits
> > even if the device itself supports more.  Add a single bit flag to
> > struct device (to be moved into the dma extension once we around it)
> 
> "once we around it"?  I don't understand, sorry.

Should be "once we get around it", which in proper grammar should
probably be "once we get to it".  Anyway, the point is that right
now struct device is bloated with a lot of fields for dma/iommu
purposes and we need to clean this up.  It's been on my TODO list
for a while.

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: missing headers test_lwt_seg6local
From: Alexei Starovoitov @ 2018-05-25 16:23 UTC (permalink / raw)
  To: Mathieu Xhonneux; +Cc: netdev, ys114321, daniel
In-Reply-To: <20180525112036.4768-1-m.xhonneux@gmail.com>

On Fri, May 25, 2018 at 12:20:36PM +0100, Mathieu Xhonneux wrote:
> Previous patch "selftests/bpf: test for seg6local End.BPF action" lacks
> some UAPI headers in tools/.
> 
> clang -I. -I./include/uapi -I../../../include/uapi -idirafter
> /usr/local/include -idirafter
> /data/users/yhs/work/llvm/build/install/lib/clang/7.0.0/include
> -idirafter /usr/include -Wno-compare-distinct-pointer-types \
>          -O2 -target bpf -emit-llvm -c test_lwt_seg6local.c -o - |      \
> llc -march=bpf -mcpu=generic  -filetype=obj -o
> [...]/net-next/tools/testing/selftests/bpf/test_lwt_seg6local.o
> test_lwt_seg6local.c:4:10: fatal error: 'linux/seg6_local.h' file not found
>          ^~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> make: Leaving directory
> `/data/users/yhs/work/net-next/tools/testing/selftests/bpf'
> 
> Reported-by: Y Song <ys114321@gmail.com>
> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
> ---
>  .../selftests/bpf/include/uapi/linux/seg6.h        | 55 +++++++++++++++
>  .../selftests/bpf/include/uapi/linux/seg6_local.h  | 80 ++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6.h
>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
> 
> diff --git a/tools/testing/selftests/bpf/include/uapi/linux/seg6.h b/tools/testing/selftests/bpf/include/uapi/linux/seg6.h

hmm. why to selftest?
Shouldn't they be added to tools/include/uapi/linux/ instead?

^ permalink raw reply

* Re: [PATCH] [RFC] bpf: tracing: new helper bpf_get_current_cgroup_ino
From: Y Song @ 2018-05-25 16:28 UTC (permalink / raw)
  To: Alban Crequy
  Cc: Alexei Starovoitov, netdev, LKML, Linux Containers, cgroups,
	Tejun Heo, Iago López Galeiras
In-Reply-To: <CADZs7q4xd1CwGULvYe2-Y2aYpwhiiw3upF=mAK0ve_-jrk1yFg@mail.gmail.com>

On Fri, May 25, 2018 at 8:21 AM, Alban Crequy <alban@kinvolk.io> wrote:
> On Wed, May 23, 2018 at 4:34 AM Y Song <ys114321@gmail.com> wrote:
>
>> I did a quick prototyping and the above interface seems working fine.
>
> Thanks! I gave your kernel patch & userspace program a try and it works for
> me on cgroup-v2.
>
> Also, I found out how to get my containers to use both cgroup-v1 and
> cgroup-v2 (by enabling systemd's hybrid cgroup mode and docker's
> '--exec-opt native.cgroupdriver=systemd' option). So I should be able to
> use the BPF helper function without having to add support for all the
> cgroup-v1 hierarchies.

Great. Will submit a formal patch soon.

>
>> The kernel change:
>> ===============
>
>> [yhs@localhost bpf-next]$ git diff
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 97446bbe2ca5..669b7383fddb 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1976,7 +1976,8 @@ union bpf_attr {
>>          FN(fib_lookup),                 \
>>          FN(sock_hash_update),           \
>>          FN(msg_redirect_hash),          \
>> -       FN(sk_redirect_hash),
>> +       FN(sk_redirect_hash),           \
>> +       FN(get_current_cgroup_id),
>
>>   /* integer value in 'imm' field of BPF_CALL instruction selects which
> helper
>>    * function eBPF program intends to call
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index ce2cbbff27e4..e11e3298f911 100644
>> --- a/kernel/trace/bpf_trace.c
>> +++ b/kernel/trace/bpf_trace.c
>> @@ -493,6 +493,21 @@ static const struct bpf_func_proto
>> bpf_current_task_under_cgroup_proto = {
>>          .arg2_type      = ARG_ANYTHING,
>>   };
>
>> +BPF_CALL_0(bpf_get_current_cgroup_id)
>> +{
>> +       struct cgroup *cgrp = task_dfl_cgroup(current);
>> +       if (!cgrp)
>> +               return -EINVAL;
>> +
>> +       return cgrp->kn->id.id;
>> +}
>> +
>> +static const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
>> +       .func           = bpf_get_current_cgroup_id,
>> +       .gpl_only       = false,
>> +       .ret_type       = RET_INTEGER,
>> +};
>> +
>>   BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
>>             const void *, unsafe_ptr)
>>   {
>> @@ -563,6 +578,8 @@ tracing_func_proto(enum bpf_func_id func_id, const
>> struct bpf_prog *prog)
>>                  return &bpf_get_prandom_u32_proto;
>>          case BPF_FUNC_probe_read_str:
>>                  return &bpf_probe_read_str_proto;
>> +       case BPF_FUNC_get_current_cgroup_id:
>> +               return &bpf_get_current_cgroup_id_proto;
>>          default:
>>                  return NULL;
>>          }
>
>> The following program can be used to print out a cgroup id given a cgroup
> path.
>> [yhs@localhost cg]$ cat get_cgroup_id.c
>> #define _GNU_SOURCE
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>
>> int main(int argc, char **argv)
>> {
>>      int dirfd, err, flags, mount_id, fhsize;
>>      struct file_handle *fhp;
>>      char *pathname;
>
>>      if (argc != 2) {
>>          printf("usage: %s <cgroup_path>\n", argv[0]);
>>          return 1;
>>      }
>
>>      pathname = argv[1];
>>      dirfd = AT_FDCWD;
>>      flags = 0;
>
>>      fhsize = sizeof(*fhp);
>>      fhp = malloc(fhsize);
>>      if (!fhp)
>>          return 1;
>
>>      err = name_to_handle_at(dirfd, pathname, fhp, &mount_id, flags);
>>      if (err >= 0) {
>>          printf("error\n");
>>          return 1;
>>      }
>
>>      fhsize = sizeof(struct file_handle) + fhp->handle_bytes;
>>      fhp = realloc(fhp, fhsize);
>>      if (!fhp)
>>          return 1;
>
>>      err = name_to_handle_at(dirfd, pathname, fhp, &mount_id, flags);
>>      if (err < 0)
>>          perror("name_to_handle_at");
>>      else {
>>          int i;
>
>>          printf("dir = %s, mount_id = %d\n", pathname, mount_id);
>>          printf("handle_bytes = %d, handle_type = %d\n", fhp->handle_bytes,
>>              fhp->handle_type);
>>          if (fhp->handle_bytes != 8)
>>              return 1;
>
>>          printf("cgroup_id = 0x%llx\n", *(unsigned long long
> *)fhp->f_handle);
>>      }
>
>>      return 0;
>> }
>> [yhs@localhost cg]$
>
>> Given a cgroup path, the user can get cgroup_id and use it in their bpf
>> program for filtering purpose.
>
>> I run a simple program t.c
>>     int main() { while(1) sleep(1); return 0; }
>> in the cgroup v2 directory /home/yhs/tmp/yhs
>>     none on /home/yhs/tmp type cgroup2 (rw,relatime,seclabel)
>
>> $ ./get_cgroup_id /home/yhs/tmp/yhs
>> dir = /home/yhs/tmp/yhs, mount_id = 124
>> handle_bytes = 8, handle_type = 1
>> cgroup_id = 0x1000006b2
>
>> // the below command to get cgroup_id from the kernel for the
>> // process compiled with t.c and ran under /home/yhs/tmp/yhs:
>> $ sudo ./trace.py -p 4067 '__x64_sys_nanosleep "cgid = %llx", $cgid'
>> PID     TID     COMM            FUNC             -
>> 4067    4067    a.out           __x64_sys_nanosleep cgid = 1000006b2
>> 4067    4067    a.out           __x64_sys_nanosleep cgid = 1000006b2
>> 4067    4067    a.out           __x64_sys_nanosleep cgid = 1000006b2
>> ^C[yhs@localhost tools]$
>
>> The kernel and user space cgid matches. Will provide a
>> formal patch later.
>
>
>
>
>> On Mon, May 21, 2018 at 5:24 PM, Y Song <ys114321@gmail.com> wrote:
>> > On Mon, May 21, 2018 at 9:26 AM, Alexei Starovoitov
>> > <alexei.starovoitov@gmail.com> wrote:
>> >> On Sun, May 13, 2018 at 07:33:18PM +0200, Alban Crequy wrote:
>> >>>
>> >>> +BPF_CALL_2(bpf_get_current_cgroup_ino, u32, hierarchy, u64, flags)
>> >>> +{
>> >>> +     // TODO: pick the correct hierarchy instead of the mem
> controller
>> >>> +     struct cgroup *cgrp = task_cgroup(current, memory_cgrp_id);
>> >>> +
>> >>> +     if (unlikely(!cgrp))
>> >>> +             return -EINVAL;
>> >>> +     if (unlikely(hierarchy))
>> >>> +             return -EINVAL;
>> >>> +     if (unlikely(flags))
>> >>> +             return -EINVAL;
>> >>> +
>> >>> +     return cgrp->kn->id.ino;
>> >>
>> >> ino only is not enough to identify cgroup. It needs generation number
> too.
>> >> I don't quite see how hierarchy and flags can be used in the future.
>> >> Also why limit it to memcg?
>> >>
>> >> How about something like this instead:
>> >>
>> >> BPF_CALL_2(bpf_get_current_cgroup_id)
>> >> {
>> >>         struct cgroup *cgrp = task_dfl_cgroup(current);
>> >>
>> >>         return cgrp->kn->id.id;
>> >> }
>> >> The user space can use fhandle api to get the same 64-bit id.
>> >
>> > I think this should work. This will also be useful to bcc as user
>> > space can encode desired id
>> > in the bpf program and compared that id to the current cgroup id, so we
> can have
>> > cgroup level tracing (esp. stat collection) support. To cope with
>> > cgroup hierarchy, user can use
>> > cgroup-array based approach or explicitly compare against multiple
> cgroup id's.

^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/6] bpf: Define cgroup_bpf_enabled for CONFIG_CGROUP_BPF=n
From: Alexei Starovoitov @ 2018-05-25 16:29 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: netdev, davem, kafai, ast, daniel, kernel-team
In-Reply-To: <677e2ddff0a1ff3d19ceb897e68f86e0246526a7.1527263217.git.rdna@fb.com>

On Fri, May 25, 2018 at 08:55:22AM -0700, Andrey Ignatov wrote:
> Static key is used to enable/disable cgroup-bpf related code paths at
> run time.
> 
> Though it's not defined when cgroup-bpf is disabled at compile time,
> i.e. CONFIG_CGROUP_BPF=n, and if some code wants to use it, it has to do
> this:
> 
> 	#ifdef CONFIG_CGROUP_BPF
> 		if (cgroup_bpf_enabled) {
> 			/* ... some work ... */
> 		}
> 	#endif
> 
> This code can be simplified by setting cgroup_bpf_enabled to 0 for
> CONFIG_CGROUP_BPF=n case:
> 
> 	if (cgroup_bpf_enabled) {
> 		/* ... some work ... */
> 	}
> 
> And it aligns well with existing BPF_CGROUP_RUN_PROG_* macros that
> defined for both states of CONFIG_CGROUP_BPF.
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net-next 0/7] net: bridge: Notify about bridge VLANs
From: Vivien Didelot @ 2018-05-25 16:31 UTC (permalink / raw)
  To: Florian Fainelli, Petr Machata, netdev, devel, bridge
  Cc: jiri, idosch, davem, razvan.stefanescu, gregkh, stephen, andrew,
	nikolay
In-Reply-To: <1ea85ef2-0feb-dec0-ae25-68f4b42543b2@gmail.com>

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> Andrew, Vivien, if the following hunks get applied are we possibly
> breaking mv88e6xxx? This is the use case that is really missing IMHO at
> the moment in DSA: we cannot control the VLAN membership and attributes
> of the CPU port(s), so either we make it always tagged in every VLAN
> (not great), or we introduce the ability to target the CPU port which is
> what Petr's patches + mine do.

Your change looks good to me. mv88e6xxx programs the DSA and CPU ports'
membership as "unmodified" (i.e. "as-is", which is a Marvell feature),
so that shouldn't change the current behavior.


Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH 1/7] core, dma-direct: add a flag 32-bit dma limits
From: Greg Kroah-Hartman @ 2018-05-25 16:35 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Thomas Gleixner, Ingo Molnar, Tony Luck, Fenghua Yu, x86, iommu,
	linux-kernel, linux-ia64, netdev
In-Reply-To: <20180525162307.GB28900@lst.de>

On Fri, May 25, 2018 at 06:23:07PM +0200, Christoph Hellwig wrote:
> On Fri, May 25, 2018 at 04:50:12PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, May 25, 2018 at 04:35:06PM +0200, Christoph Hellwig wrote:
> > > Various PCI bridges (VIA PCI, Xilinx PCIe) limit DMA to only 32-bits
> > > even if the device itself supports more.  Add a single bit flag to
> > > struct device (to be moved into the dma extension once we around it)
> > 
> > "once we around it"?  I don't understand, sorry.
> 
> Should be "once we get around it", which in proper grammar should
> probably be "once we get to it".  Anyway, the point is that right
> now struct device is bloated with a lot of fields for dma/iommu
> purposes and we need to clean this up.  It's been on my TODO list
> for a while.

Ah, makes sense, that's fine with me, I'd love to see that get cleaned
up.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH bpf-next] selftests/bpf: missing headers test_lwt_seg6local
From: Daniel Borkmann @ 2018-05-25 16:39 UTC (permalink / raw)
  To: Alexei Starovoitov, Mathieu Xhonneux; +Cc: netdev, ys114321
In-Reply-To: <20180525162312.dadn2wtcp4bkf2la@ast-mbp>

On 05/25/2018 06:23 PM, Alexei Starovoitov wrote:
> On Fri, May 25, 2018 at 12:20:36PM +0100, Mathieu Xhonneux wrote:
>> Previous patch "selftests/bpf: test for seg6local End.BPF action" lacks
>> some UAPI headers in tools/.
>>
>> clang -I. -I./include/uapi -I../../../include/uapi -idirafter
>> /usr/local/include -idirafter
>> /data/users/yhs/work/llvm/build/install/lib/clang/7.0.0/include
>> -idirafter /usr/include -Wno-compare-distinct-pointer-types \
>>          -O2 -target bpf -emit-llvm -c test_lwt_seg6local.c -o - |      \
>> llc -march=bpf -mcpu=generic  -filetype=obj -o
>> [...]/net-next/tools/testing/selftests/bpf/test_lwt_seg6local.o
>> test_lwt_seg6local.c:4:10: fatal error: 'linux/seg6_local.h' file not found
>>          ^~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>> make: Leaving directory
>> `/data/users/yhs/work/net-next/tools/testing/selftests/bpf'
>>
>> Reported-by: Y Song <ys114321@gmail.com>
>> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
>> ---
>>  .../selftests/bpf/include/uapi/linux/seg6.h        | 55 +++++++++++++++
>>  .../selftests/bpf/include/uapi/linux/seg6_local.h  | 80 ++++++++++++++++++++++
>>  2 files changed, 135 insertions(+)
>>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6.h
>>  create mode 100644 tools/testing/selftests/bpf/include/uapi/linux/seg6_local.h
>>
>> diff --git a/tools/testing/selftests/bpf/include/uapi/linux/seg6.h b/tools/testing/selftests/bpf/include/uapi/linux/seg6.h
> 
> hmm. why to selftest?
> Shouldn't they be added to tools/include/uapi/linux/ instead?

Yes, should definitely go there to tools include infrastructure.

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] bpf: add boot parameters for sysctl knobs
From: Eugene Syromiatnikov @ 2018-05-25 16:50 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jesper Dangaard Brouer, netdev, linux-kernel, linux-doc,
	Kees Cook, Kai-Heng Feng, Daniel Borkmann, Alexei Starovoitov,
	Jonathan Corbet, Jiri Olsa
In-Reply-To: <20180524233449.ga664pzexrkzepfv@ast-mbp>

On Thu, May 24, 2018 at 04:34:51PM -0700, Alexei Starovoitov wrote:
> On Thu, May 24, 2018 at 09:41:08AM +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 23 May 2018 15:02:45 -0700
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > 
> > > On Wed, May 23, 2018 at 02:18:19PM +0200, Eugene Syromiatnikov wrote:
> > > > Some BPF sysctl knobs affect the loading of BPF programs, and during
> > > > system boot/init stages these sysctls are not yet configured.
> > > > A concrete example is systemd, that has implemented loading of BPF
> > > > programs.
> > > > 
> > > > Thus, to allow controlling these setting at early boot, this patch set
> > > > adds the ability to change the default setting of these sysctl knobs
> > > > as well as option to override them via a boot-time kernel parameter
> > > > (in order to avoid rebuilding kernel each time a need of changing these
> > > > defaults arises).
> > > > 
> > > > The sysctl knobs in question are kernel.unprivileged_bpf_disable,
> > > > net.core.bpf_jit_harden, and net.core.bpf_jit_kallsyms.  
> > > 
> > > - systemd is root. today it only uses cgroup-bpf progs which require root,
> > >   so disabling unpriv during boot time makes no difference to systemd.
> > >   what is the actual reason to present time?
systemd also runs a lot of code, some of which is unprivileged.

> > > - say in the future systemd wants to use so_reuseport+bpf for faster
> > >   networking. With unpriv disable during boot, it will force systemd
> > >   to do such networking from root, which will lower its security barrier.
No, it will force systemd not to use SO_REUSEPORT BPF.

> > > - bpf_jit_kallsyms sysctl has immediate effect on loaded programs.
> > >   Flipping it during the boot or right after or any time after
> > >   is the same thing. Why add such boot flag then?
Well, that one was for completeness.

> > > - jit_harden can be turned on by systemd. so turning it during the boot
> > >   will make systemd progs to be constant blinded.
> > >   Constant blinding protects kernel from unprivileged JIT spraying.
> > >   Are you worried that systemd will attack the kernel with JIT spraying?
I'm worried that systemd can be exploited for a JIT spraying attack.

Another thing I'm concerned with is that the generated code is different,
which introduces additional complication during debugging.

> > I think you are missing that, we want the ability to change these
> > defaults in-order to avoid depending on /etc/sysctl.conf settings, and
> > that the these sysctl.conf setting happen too late.
> 
> What does it mean 'happens too late' ?
> Too late for what?
> sysctl.conf has plenty of system critical knobs like
> kernel.perf_event_paranoid, kernel.core_pattern, etc
> The behavior of the host is drastically different after sysctl config
> is applied.
> 
> > For example with jit_harden, there will be a difference between the
> > loaded BPF program that got loaded at boot-time with systemd (no
> > constant blinding) and when someone reloads that systemd service after
> > /etc/sysctl.conf have been evaluated and setting bpf_jit_harden (now
> > slower due to constant blinding).   This is inconsistent behavior.
> 
> net.core.bpf_jit_harden can be flipped back and forth at run-time,
> so bpf progs before and after will be either blinded or not.
> I don't see any inconsistency.

That can't be the reason to maintain that inconsistency.

^ permalink raw reply

* Re: [PATCH net-next 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()
From: Petr Machata @ 2018-05-25 16:56 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, devel, bridge, jiri, idosch, davem, razvan.stefanescu,
	gregkh, stephen, andrew, f.fainelli, nikolay
In-Reply-To: <87d0xjzpf2.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:

> Hi Petr,
>
> Petr Machata <petrm@mellanox.com> writes:
>
>> -static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
>> -			  u16 vid, u16 flags)
>> +static int br_switchdev_port_obj_add(struct net_device *dev, u16 vid, u16 flags)
>>  {
>>  	struct switchdev_obj_port_vlan v = {
>>  		.obj.orig_dev = dev,
>> @@ -89,12 +88,29 @@ static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
>>  		.vid_begin = vid,
>>  		.vid_end = vid,
>>  	};
>> -	int err;
>>  
>> +	return switchdev_port_obj_add(dev, &v.obj);
>> +}
>> +
>> +static int br_switchdev_port_obj_del(struct net_device *dev, u16 vid)
>> +{
>> +	struct switchdev_obj_port_vlan v = {
>> +		.obj.orig_dev = dev,
>> +		.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
>> +		.vid_begin = vid,
>> +		.vid_end = vid,
>> +	};
>> +
>> +	return switchdev_port_obj_del(dev, &v.obj);
>> +}
>
> Shouldn't they be br_switchdev_port_vlan_add (or similar) implemented in
> net/bridge/br_switchdev.c instead, since they are VLAN specific?

(You mean switchdev-specific?)

This logic was in br_vlan.c before as well, so it's natural to think
about the functions as helpers of VLAN module. I can move to
br_switchdev.c if you think that's the better place.

Thanks,
Petr

^ permalink raw reply

* Re: [PATCH net-next 6/7] net: bridge: Notify about bridge VLANs
From: Petr Machata @ 2018-05-25 17:00 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, devel, bridge, jiri, idosch, davem, razvan.stefanescu,
	gregkh, stephen, andrew, f.fainelli, nikolay
In-Reply-To: <877enrzp42.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:

>> +	} else {
>> +		err = br_switchdev_port_obj_add(dev, v->vid, flags);
>> +		if (err && err != -EOPNOTSUPP)
>> +			goto out;
>>  	}
>
> Except that br_switchdev_port_obj_add taking vid and flags arguments
> seems confusing to me, the change looks good:

I'm not sure what you're aiming at. Both VID and flags are sent with the
notification, so they need to be passed on to the function somehow. Do
you have a counterproposal for the API?

Thanks,
Petr

^ permalink raw reply

* Re: [PATCH] PCI: allow drivers to limit the number of VFs to 0
From: Bjorn Helgaas @ 2018-05-25 17:01 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Bjorn Helgaas, linux-pci, netdev, Sathya Perla, Felix Manlunas,
	alexander.duyck, john.fastabend, Jacob Keller, Donald Dutile,
	oss-drivers, Christoph Hellwig, Derek Chickles, Satanand Burla,
	Raghu Vatsavayi, Ajit Khaparde, Sriharsha Basavapatna,
	Somnath Kotur, Jeff Kirsher, intel-wired-lan
In-Reply-To: <20180525140223.GA45098@bhelgaas-glaptop.roam.corp.google.com>

[+cc liquidio, benet, fm10k maintainers:

  The patch below will affect you if your driver calls
    pci_sriov_set_totalvfs(dev, 0);

  Previously that caused a subsequent pci_sriov_get_totalvfs() to return
  the totalVFs value from the SR-IOV capability.  After this patch, it will
  return 0, which has implications for VF enablement via the sysfs
  "sriov_numvfs" file.]

On Fri, May 25, 2018 at 09:02:23AM -0500, Bjorn Helgaas wrote:
> On Thu, May 24, 2018 at 06:20:15PM -0700, Jakub Kicinski wrote:
> > Hi Bjorn!
> > 
> > On Thu, 24 May 2018 18:57:48 -0500, Bjorn Helgaas wrote:
> > > On Mon, Apr 02, 2018 at 03:46:52PM -0700, Jakub Kicinski wrote:
> > > > Some user space depends on enabling sriov_totalvfs number of VFs
> > > > to not fail, e.g.:
> > > > 
> > > > $ cat .../sriov_totalvfs > .../sriov_numvfs
> > > > 
> > > > For devices which VF support depends on loaded FW we have the
> > > > pci_sriov_{g,s}et_totalvfs() API.  However, this API uses 0 as
> > > > a special "unset" value, meaning drivers can't limit sriov_totalvfs
> > > > to 0.  Remove the special values completely and simply initialize
> > > > driver_max_VFs to total_VFs.  Then always use driver_max_VFs.
> > > > Add a helper for drivers to reset the VF limit back to total.  
> > > 
> > > I still can't really make sense out of the changelog.
> > >
> > > I think part of the reason it's confusing is because there are two
> > > things going on:
> > > 
> > >   1) You want this:
> > >   
> > >        pci_sriov_set_totalvfs(dev, 0);
> > >        x = pci_sriov_get_totalvfs(dev) 
> > > 
> > >      to return 0 instead of total_VFs.  That seems to connect with
> > >      your subject line.  It means "sriov_totalvfs" in sysfs could be
> > >      0, but I don't know how that is useful (I'm sure it is; just
> > >      educate me :))
> > 
> > Let me just quote the bug report that got filed on our internal bug
> > tracker :)
> > 
> >   When testing Juju Openstack with Ubuntu 18.04, enabling SR-IOV causes
> >   errors because Juju gets the sriov_totalvfs for SR-IOV-capable device
> >   then tries to set that as the sriov_numvfs parameter.
> > 
> >   For SR-IOV incapable FW, the sriov_totalvfs parameter should be 0, 
> >   but it's set to max.  When FW is switched to flower*, the correct 
> >   sriov_totalvfs value is presented.
> > 
> > * flower is a project name
> 
> From the point of view of the PCI core (which knows nothing about
> device firmware and relies on the architected config space described
> by the PCIe spec), this sounds like an erratum: with some firmware
> installed, the device is not capable of SR-IOV, but still advertises
> an SR-IOV capability with "TotalVFs > 0".
> 
> Regardless of whether that's an erratum, we do allow PF drivers to use
> pci_sriov_set_totalvfs() to limit the number of VFs that may be
> enabled by writing to the PF's "sriov_numvfs" sysfs file.
> 
> But the current implementation does not allow a PF driver to limit VFs
> to 0, and that does seem nonsensical.
> 
> > My understanding is OpenStack uses sriov_totalvfs to determine how many
> > VFs can be enabled, looks like this is the code:
> > 
> > http://git.openstack.org/cgit/openstack/charm-neutron-openvswitch/tree/hooks/neutron_ovs_utils.py#n464
> > 
> > >   2) You're adding the pci_sriov_reset_totalvfs() interface.  I'm not
> > >      sure what you intend for this.  Is *every* driver supposed to
> > >      call it in .remove()?  Could/should this be done in the core
> > >      somehow instead of depending on every driver?
> > 
> > Good question, I was just thinking yesterday we may want to call it
> > from the core, but I don't think it's strictly necessary nor always
> > sufficient (we may reload FW without re-probing).
> > 
> > We have a device which supports different number of VFs based on the FW
> > loaded.  Some legacy FWs does not inform the driver how many VFs it can
> > support, because it supports max.  So the flow in our driver is this:
> > 
> > load_fw(dev);
> > ...
> > max_vfs = ask_fw_for_max_vfs(dev);
> > if (max_vfs >= 0)
> > 	return pci_sriov_set_totalvfs(dev, max_vfs);
> > else /* FW didn't tell us, assume max */
> > 	return pci_sriov_reset_totalvfs(dev); 
> > 
> > We also reset the max on device remove, but that's not strictly
> > necessary.
> > 
> > Other users of pci_sriov_set_totalvfs() always know the value to set
> > the total to (either always get it from FW or it's a constant).
> > 
> > If you prefer we can work out the correct max for those legacy cases in
> > the driver as well, although it seemed cleaner to just ask the core,
> > since it already has total_VFs value handy :)
> > 
> > > I'm also having a hard time connecting your user-space command example
> > > with the rest of this.  Maybe it will make more sense to me tomorrow
> > > after some coffee.
> > 
> > OpenStack assumes it will always be able to set sriov_numvfs to
> > sriov_totalvfs, see this 'if':
> > 
> > http://git.openstack.org/cgit/openstack/charm-neutron-openvswitch/tree/hooks/neutron_ovs_utils.py#n512
> 
> Thanks for educating me.  I think there are two issues here that we
> can separate.  I extracted the patch below for the first.
> 
> The second is the question of resetting driver_max_VFs.  I think we
> currently have a general issue in the core:
> 
>   - load PF driver 1
>   - driver calls pci_sriov_set_totalvfs() to reduce driver_max_VFs
>   - unload PF driver 1
>   - load PF driver 2
> 
> Now driver_max_VFs is still stuck at the lower value set by driver 1.
> I don't think that's the way this should work.
> 
> I guess this is partly a consequence of setting driver_max_VFs in
> sriov_init(), which is called before driver attach and should only
> depend on hardware characteristics, so it is related to the patch
> below.  But I think we should fix it in general, not just for
> netronome.
> 
> 
> commit 4a338bc6f94b9ad824ac944f5dfc249d6838719c
> Author: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date:   Fri May 25 08:18:34 2018 -0500
> 
>     PCI/IOV: Allow PF drivers to limit total_VFs to 0
>     
>     Some SR-IOV PF drivers implement .sriov_configure(), which allows
>     user-space to enable VFs by writing the desired number of VFs to the sysfs
>     "sriov_numvfs" file (see sriov_numvfs_store()).
>     
>     The PCI core limits the number of VFs to the TotalVFs advertised by the
>     device in its SR-IOV capability.  The PF driver can limit the number of VFs
>     to even fewer (it may have pre-allocated data structures or knowledge of
>     device limitations) by calling pci_sriov_set_totalvfs(), but previously it
>     could not limit the VFs to 0.
>     
>     Change pci_sriov_get_totalvfs() so it always respects the VF limit imposed
>     by the PF driver, even if the limit is 0.
>     
>     This sequence:
>     
>       pci_sriov_set_totalvfs(dev, 0);
>       x = pci_sriov_get_totalvfs(dev);
>     
>     previously set "x" to TotalVFs from the SR-IOV capability.  Now it will set
>     "x" to 0.
>     
>     Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 192b82898a38..d0d73dbbd5ca 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -469,6 +469,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  	iov->nres = nres;
>  	iov->ctrl = ctrl;
>  	iov->total_VFs = total;
> +	iov->driver_max_VFs = total;
>  	pci_read_config_word(dev, pos + PCI_SRIOV_VF_DID, &iov->vf_device);
>  	iov->pgsz = pgsz;
>  	iov->self = dev;
> @@ -827,10 +828,7 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
>  	if (!dev->is_physfn)
>  		return 0;
>  
> -	if (dev->sriov->driver_max_VFs)
> -		return dev->sriov->driver_max_VFs;
> -
> -	return dev->sriov->total_VFs;
> +	return dev->sriov->driver_max_VFs;
>  }
>  EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
>  

^ permalink raw reply

* Re: [PATCH v2 net-next] net: stmmac: Add PPS and Flexible PPS support
From: Richard Cochran @ 2018-05-25 17:02 UTC (permalink / raw)
  To: Jose Abreu
  Cc: netdev, David S. Miller, Joao Pinto, Vitor Soares,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <fb25946a2fee8708876ff9a56d5b20f8b9224a5a.1527262324.git.joabreu@synopsys.com>

On Fri, May 25, 2018 at 04:32:52PM +0100, Jose Abreu wrote:
> +int dwmac5_pps_config(void __iomem *ioaddr, bool enable)
> +{
> +	u32 val = readl(ioaddr + MAC_PPS_CONTROL);
> +
> +	/* There is no way to disable fixed PPS output so we just reset
> +	 * the values to make sure its in fixed PPS mode */

In that case, don't try to make this appear to be programmable.  Just
reset the registers in your probe or setup function unconditionally.

> +	val &= ~PPSx_MASK(0);
> +	val |= TRGTMODSELx(0, 0x2);
> +
> +	writel(val, ioaddr + MAC_PPS_CONTROL);
> +	return 0;
> +}
> +
> +int dwmac5_flex_pps_config(void __iomem *ioaddr, int index,
> +			   struct stmmac_pps_cfg *cfg, bool enable,
> +			   u32 sub_second_inc, u32 systime_flags)
> +{
> +	u32 tnsec = readl(ioaddr + MAC_PPSx_TARGET_TIME_NSEC(index));
> +	u32 val = readl(ioaddr + MAC_PPS_CONTROL);
> +	u64 period;
> +
> +	if (!cfg->available)
> +		return -EINVAL;
> +	if (tnsec & TRGTBUSY0)
> +		return -EBUSY;
> +	if (!sub_second_inc || !systime_flags)
> +		return -EINVAL;

Don't add tests on the arguments like this.  Instead, make sure the
caller always provides correct values.

> +
> +	val &= ~PPSx_MASK(index);
> +
> +	if (!enable) {
> +		val |= PPSCMDx(index, 0x5);
> +		writel(val, ioaddr + MAC_PPS_CONTROL);
> +		return 0;
> +	}
> +
> +	val |= PPSCMDx(index, 0x2);
> +	val |= TRGTMODSELx(index, 0x2);
> +	val |= PPSEN0;
> +
> +	writel(cfg->start.tv_sec, ioaddr + MAC_PPSx_TARGET_TIME_SEC(index));
> +
> +	if (!(systime_flags & PTP_TCR_TSCTRLSSR))
> +		cfg->start.tv_nsec = (cfg->start.tv_nsec * 1000) / 465;
> +	writel(cfg->start.tv_nsec, ioaddr + MAC_PPSx_TARGET_TIME_NSEC(index));
> +
> +	period = cfg->period.tv_sec * 1000000000;
> +	period += cfg->period.tv_nsec;

> +	struct timespec64 period;

> +
> +	do_div(period, sub_second_inc);
> +
> +	if (period <= 1)
> +		return -EINVAL;
> +
> +	writel(period - 1, ioaddr + MAC_PPSx_INTERVAL(index));
> +
> +	period >>= 1;
> +	if (period <= 1)
> +		return -EINVAL;
> +
> +	writel(period - 1, ioaddr + MAC_PPSx_WIDTH(index));
> +
> +	/* Finally, activate it */
> +	writel(val, ioaddr + MAC_PPS_CONTROL);
> +	return 0;
> +}

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 7d3a5c7..35c6d0c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -140,19 +140,50 @@ static int stmmac_set_time(struct ptp_clock_info *ptp,
>  static int stmmac_enable(struct ptp_clock_info *ptp,
>  			 struct ptp_clock_request *rq, int on)
>  {
> -	return -EOPNOTSUPP;
> +	struct stmmac_priv *priv =
> +	    container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> +	struct stmmac_pps_cfg *cfg;
> +	int ret = -EOPNOTSUPP;
> +	unsigned long flags;
> +
> +	switch (rq->type) {
> +	case PTP_CLK_REQ_PEROUT:
> +		cfg = &priv->pps[rq->perout.index];
> +
> +		cfg->start.tv_sec = rq->perout.start.sec;
> +		cfg->start.tv_nsec = rq->perout.start.nsec;
> +		cfg->period.tv_sec = rq->perout.period.sec;
> +		cfg->period.tv_nsec = rq->perout.period.nsec;
> +
> +		spin_lock_irqsave(&priv->ptp_lock, flags);
> +		ret = stmmac_flex_pps_config(priv, priv->ioaddr,
> +					     rq->perout.index, cfg, on,
> +					     priv->sub_second_inc,
> +					     priv->systime_flags);
> +		spin_unlock_irqrestore(&priv->ptp_lock, flags);
> +		break;
> +	case PTP_CLK_REQ_PPS:
> +		spin_lock_irqsave(&priv->ptp_lock, flags);
> +		ret = stmmac_pps_config(priv, priv->ioaddr, on);

This is not what PTP_CLK_REQ_PPS is for.  It only to arrange for a
trigger into the kernel's PPS sub-system.

[ Sorry that the ptp header files don't explain this in the comments.
  I should really fix that. ]

> +		spin_unlock_irqrestore(&priv->ptp_lock, flags);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
>  }
>  
>  /* structure describing a PTP hardware clock */
> -static const struct ptp_clock_info stmmac_ptp_clock_ops = {
> +static struct ptp_clock_info stmmac_ptp_clock_ops = {
>  	.owner = THIS_MODULE,
>  	.name = "stmmac_ptp_clock",
>  	.max_adj = 62500000,
>  	.n_alarm = 0,
>  	.n_ext_ts = 0,
> -	.n_per_out = 0,
> +	.n_per_out = 0, /* will be overwritten in stmmac_ptp_register */
>  	.n_pins = 0,
> -	.pps = 0,
> +	.pps = 0, /* will be overwritten in stmmac_ptp_register */

Again, this is for inputting a PPS event into the kernel's PPS subsystem.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next v12 0/5] Enable virtio_net to act as a standby for a passthru device
From: Michael S. Tsirkin @ 2018-05-25 17:19 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: stephen, davem, netdev, virtualization, virtio-dev,
	jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
	loseweigh, jiri, aaron.f.brown, anjali.singhai
In-Reply-To: <1527180917-39737-1-git-send-email-sridhar.samudrala@intel.com>

On Thu, May 24, 2018 at 09:55:12AM -0700, Sridhar Samudrala wrote:
> The main motivation for this patch is to enable cloud service providers
> to provide an accelerated datapath to virtio-net enabled VMs in a 
> transparent manner with no/minimal guest userspace changes. This also
> enables hypervisor controlled live migration to be supported with VMs that
> have direct attached SR-IOV VF devices.
> 
> Patch 1 introduces a failover module that provides a generic interface for 
> paravirtual drivers to listen for netdev register/unregister/link change
> events from pci ethernet devices with the same MAC and takeover their
> datapath. The notifier and event handling code is based on the existing
> netvsc implementation. 
> 
> Patch 2 refactors netvsc to use the registration/notification framework
> introduced by failover module.
> 
> Patch 3 introduces a net_failover driver that provides an automated
> failover mechanism to paravirtual drivers via APIs to create and destroy
> a failover master netdev and mananges a primary and standby slave netdevs
> that get registered via the generic failover infrastructure.
> 
> Patch 4 introduces a new feature bit VIRTIO_NET_F_STANDBY to virtio-net
> that can be used by hypervisor to indicate that virtio_net interface
> should act as a standby for another device with the same MAC address.
> 
> Patch 5 extends virtio_net to use alternate datapath when available and
> registered. When STANDBY feature is enabled, virtio_net driver uese the
> net_failover API to create an additional 'failover' netdev that acts as
> a master device and controls 2 slave devices.  The original virtio_net
> netdev is registered as 'standby' netdev and a passthru/vf device with
> the same MAC gets registered as 'primary' netdev. Both 'standby' and
> 'failover' netdevs are associated with the same 'pci' device.  The user
> accesses the network interface via 'failover' netdev. The 'failover'
> netdev chooses 'primary' netdev as default for transmits when it is
> available with link up and running.
> 
> As this patch series is initially focusing on usecases where hypervisor 
> fully controls the VM networking and the guest is not expected to directly 
> configure any hardware settings, it doesn't expose all the ndo/ethtool ops
> that are supported by virtio_net at this time. To support additional usecases,
> it should be possible to enable additional ops later by caching the state
> in failover netdev and replaying when the 'primary' netdev gets registered. 
>  
> At the time of live migration, the hypervisor needs to unplug the VF device
> from the guest on the source host and reset the MAC filter of the VF to
> initiate failover of datapath to virtio before starting the migration. After
> the migration is completed, the destination hypervisor sets the MAC filter
> on the VF and plugs it back to the guest to switch over to VF datapath.
> 
> This patch is based on the discussion initiated by Jesse on this thread.
> https://marc.info/?l=linux-virtualization&m=151189725224231&w=2

Series:

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


> v12:
> - Tested live migration with virtio-net/AVF(i40evf) configured in failover
>   mode while running iperf in background. Tried static ip and dhcp
>   configurations using 'network' scripts and Network Manager.
> - Build tested netvsc module.
> Updates:
> - Extended generic failover module to do common functions like setting
>   FAILOVER_SLAVE flag, registering rx-handler and linking to upper dev in
>   the generic register/unregister handlers.
>   This required adding 3 additional failover ops pre_register, pre_unregister
>   and handle_frame.  netvsc and net_failover drivers are updated to support
>   these ops.
> 
> v11:
> - Split net_failover module into 2 components.
>   1. 'failover' module that provides generic failover infrastructure
>   to register a failover instance and listen for slave events.
>   2. 'net_failover' driver that provides APIs to create/destroy upper
>   netdev and supports 3-netdev model used by virtio-net.
> - Added documentation
> 
> v10:
> - fix net_failover_open() to update failover CARRIER correctly based on
>   standby and primary states. 
> - fix net_failover_handle_frame() to handle frames received on standby
>   when primary is present.
> - replace netdev_upper_dev_link with netdev_master_upper_dev_link and
>   handle lower dev state changes.
> - fix net_failver_create() and net_failover_register() interfaces to
>   use ERR_PTR and avoid arg **
> - disable setting mac address when virtio-net in STANDBY mode
> - document exported symbols
> - added entry to MAINTAINERS file
> 
> v9:
> Select NET_FAILOVER automatically when VIRTIO_NET/HYPERV_NET 
> are enabled. (stephen)
> 
> v8:
> - Made the failover managment routines more robust by updating the feature 
>   bits/other fields in the failover netdev when slave netdevs are 
>   registered/unregistered. (mst)
> - added support for handling vlans.
> - Limited the changes in netvsc to only use the notifier/event/lookups
>   from the failover module. The slave register/unregister/link-change 
>   handlers are only updated to use the getbymac routine to get the 
>   upper netdev. There is no change in their functionality. (stephen)
> - renamed structs/function/file names to use net_failover prefix. (mst)
> 
> v7
> - Rename 'bypass/active/backup' terminology with 'failover/primary/standy'
>   (jiri, mst)
> - re-arranged dev_open() and dev_set_mtu() calls in the register routines
>   so that they don't get called for 2-netdev model. (stephen)
> - fixed select_queue() routine to do queue selection based on VF if it is
>   registered as primary. (stephen)
> -  minor bugfixes
> 
> v6 RFC:
>   Simplified virtio_net changes by moving all the ndo_ops of the 
>   bypass_netdev and create/destroy of bypass_netdev to 'bypass' module.
>   avoided 2 phase registration(driver + instances).
>   introduced IFF_BYPASS/IFF_BYPASS_SLAVE dev->priv_flags 
>   replaced mutex with a spinlock
> 
> v5 RFC:
>   Based on Jiri's comments, moved the common functionality to a 'bypass'
>   module so that the same notifier and event handlers to handle child
>   register/unregister/link change events can be shared between virtio_net
>   and netvsc.
>   Improved error handling based on Siwei's comments.
> v4:
> - Based on the review comments on the v3 version of the RFC patch and
>   Jakub's suggestion for the naming issue with 3 netdev solution,
>   proposed 3 netdev in-driver bonding solution for virtio-net.
> v3 RFC:
> - Introduced 3 netdev model and pointed out a couple of issues with
>   that model and proposed 2 netdev model to avoid these issues.
> - Removed broadcast/multicast optimization and only use virtio as
>   backup path when VF is unplugged.
> v2 RFC:
> - Changed VIRTIO_NET_F_MASTER to VIRTIO_NET_F_BACKUP (mst)
> - made a small change to the virtio-net xmit path to only use VF datapath
>   for unicasts. Broadcasts/multicasts use virtio datapath. This avoids
>   east-west broadcasts to go over the PCI link.
> - added suppport for the feature bit in qemu
> 
> Sridhar Samudrala (5):
>   net: Introduce generic failover module
>   netvsc: refactor notifier/event handling code to use the failover
>     framework
>   net: Introduce net_failover driver
>   virtio_net: Introduce VIRTIO_NET_F_STANDBY feature bit
>   virtio_net: Extend virtio to use VF datapath when available
> 
>  Documentation/networking/failover.rst     |  18 +
>  Documentation/networking/net_failover.rst | 116 +++++
>  MAINTAINERS                               |  16 +
>  drivers/net/Kconfig                       |  13 +
>  drivers/net/Makefile                      |   1 +
>  drivers/net/hyperv/Kconfig                |   1 +
>  drivers/net/hyperv/hyperv_net.h           |   2 +
>  drivers/net/hyperv/netvsc_drv.c           | 222 ++------
>  drivers/net/net_failover.c                | 836 ++++++++++++++++++++++++++++++
>  drivers/net/virtio_net.c                  |  40 +-
>  include/linux/netdevice.h                 |  16 +
>  include/net/failover.h                    |  36 ++
>  include/net/net_failover.h                |  40 ++
>  include/uapi/linux/virtio_net.h           |   3 +
>  net/Kconfig                               |  13 +
>  net/core/Makefile                         |   1 +
>  net/core/failover.c                       | 315 +++++++++++
>  17 files changed, 1522 insertions(+), 167 deletions(-)
>  create mode 100644 Documentation/networking/failover.rst
>  create mode 100644 Documentation/networking/net_failover.rst
>  create mode 100644 drivers/net/net_failover.c
>  create mode 100644 include/net/failover.h
>  create mode 100644 include/net/net_failover.h
>  create mode 100644 net/core/failover.c
> 
> -- 
> 2.14.3

^ permalink raw reply

* [PATCH bpf-next] libbpf: Install btf.h with libbpf
From: Andrey Ignatov @ 2018-05-25 17:23 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, kafai, ast, daniel, kernel-team

install_headers target should contain all headers that are part of
libbpf. Add missing btf.h

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/lib/bpf/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index f3fab4a..5390e77 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -189,6 +189,7 @@ install_headers:
 	$(call QUIET_INSTALL, headers) \
 		$(call do_install,bpf.h,$(prefix)/include/bpf,644); \
 		$(call do_install,libbpf.h,$(prefix)/include/bpf,644);
+		$(call do_install,btf.h,$(prefix)/include/bpf,644);
 
 install: install_lib
 
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH bpf-next] libbpf: Install btf.h with libbpf
From: Martin KaFai Lau @ 2018-05-25 17:33 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <20180525172313.1043567-1-rdna@fb.com>

On Fri, May 25, 2018 at 10:23:13AM -0700, Andrey Ignatov wrote:
> install_headers target should contain all headers that are part of
> libbpf. Add missing btf.h
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>

^ 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