[PATCH bpf-next 00/16] net: Add support for XDP in egress path

[PATCH bpf-next 00/16] net: Add support for XDP in egress path

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

This series adds support for XDP in the egress path by introducing
a new XDP attachment type, BPF_XDP_EGRESS, and adding a UAPI to
if_link.h for attaching the program to a netdevice and reporting
the program. bpf programs can be run on all packets in the Tx path -
skbs or redirected xdp frames. The intent is to emulate the current
RX path for XDP as much as possible to maintain consistency and
symmetry in the 2 paths with their APIs.

This is a missing primitive for XDP allowing solutions to build small,
targeted programs properly distributed in the networking path allowing,
for example, an egress firewall/ACL/traffic verification or packet
manipulation and encapping an entire ethernet frame whether it is
locally generated traffic, forwarded via the slow path (ie., full
stack processing) or xdp redirected frames.

Nothing about running a program in the Tx path requires driver specific
resources like the Rx path has. Thus, programs can be run in core
code and attached to the net_device struct similar to skb mode. The
existing XDP_FLAGS_*_MODE are not relevant at the moment, so none can
be set in the attach. XDP_FLAGS_HW_MODE can be used in the future
(e.g., the work on offloading programs from a VM).

The locations chosen to run the egress program - __netdev_start_xmit
before the call to ndo_start_xmit and bq_xmit_all before invoking
ndo_xdp_xmit - allow follow on patch sets to handle tx queueing and
setting the queue index if multi-queue with consistency in handling
both packet formats.

A few of the patches trace back to work done on offloading programs
from a VM by Jason Wang and Prashant Bole.

v1:
- add selftests
- flip the order of xdp generic patches as requested by Toke
- fixed the count arg to do_xdp_egress_frame - Toke
- remove meta data invalidate in __xdp_egress_frame - Toke
- fixed data_hard_start in __xdp_egress_frame - Jesper
- refactored convert_to_xdp_frame to reuse buf to frame code - Jesper
- added missed refactoring patch when generating patch set
- updates to sample program

RFC v5:
- updated cover letter
- moved running of ebpf program to from ndo_{start,xdp}_xmit to core
  code. Dropped all tun and vhost related changes.
- added egress support to bpftool

RFC v4:
- updated cover letter
- patches related to code movement between tuntap, headers and vhost
  are dropped; previous RFC ran the XDP program in vhost context vs
  this set which runs them before queueing to vhost. As a part of this
  moved invocation of egress program to tun_net_xmit and tun_xdp_xmit.
- renamed do_xdp_generic to do_xdp_generic_rx to emphasize is called
  in the Rx path; added rx argument to do_xdp_generic_core since it
  is used for both directions and needs to know which queue values to
  set in xdp_buff

RFC v3:
- reworked the patches - splitting patch 1 from RFC v2 into 3, combining
  patch 2 from RFC v2 into the first 3, combining patches 6 and 7 from
  RFC v2 into 1 since both did a trivial rename and export. Reordered
  the patches such that kernel changes are first followed by libbpf and
  an enhancement to a sample.

- moved small xdp related helper functions from tun.c to tun.h to make
  tun_ptr_free usable from the tap code. This is needed to handle the
  case of tap builtin and tun built as a module.

- pkt_ptrs added to `struct tun_file` and passed to tun_consume_packets
  rather than declaring pkts as an array on the stack.

RFC v2:
- New XDP attachment type: Jesper, Toke and Alexei discussed whether
  to introduce a new program type. Since this set adds a way to attach
  regular XDP program to the tx path, as per Alexei's suggestion, a
  new attachment type BPF_XDP_EGRESS is introduced.

- libbpf API changes:
  Alexei had suggested _opts() style of API extension. Considering it
  two new libbpf APIs are introduced which are equivalent to existing
  APIs. New ones can be extended easily. Please see individual patches
  for details. xdp1 sample program is modified to use new APIs.

- tun: Some patches from previous set are removed as they are
  irrelevant in this series. They will in introduced later.


David Ahern (16):
  net: Refactor convert_to_xdp_frame
  net: Move handling of IFLA_XDP attribute out of do_setlink
  net: Add XDP setup and query commands for Tx programs
  net: Add BPF_XDP_EGRESS as a bpf_attach_type
  xdp: Add xdp_txq_info to xdp_buff
  net: Add IFLA_XDP_EGRESS for XDP programs in the egress path
  net: Rename do_xdp_generic to do_xdp_generic_rx
  net: rename netif_receive_generic_xdp to do_generic_xdp_core
  net: set XDP egress program on netdevice
  net: Support xdp in the Tx path for packets as an skb
  net: Support xdp in the Tx path for xdp_frames
  libbpf: Add egress XDP support
  bpftool: Add support for XDP egress
  selftest: Add test for xdp_egress
  selftest: Add xdp_egress attach tests
  samples/bpf: add XDP egress support to xdp1

 drivers/net/tun.c                             |   4 +-
 include/linux/netdevice.h                     |  23 +-
 include/net/xdp.h                             |  35 ++-
 include/uapi/linux/bpf.h                      |   3 +
 include/uapi/linux/if_link.h                  |   3 +
 kernel/bpf/devmap.c                           |  19 +-
 net/core/dev.c                                | 251 ++++++++++++++----
 net/core/filter.c                             |  23 ++
 net/core/rtnetlink.c                          | 180 ++++++++++---
 samples/bpf/xdp1_user.c                       |  39 ++-
 tools/bpf/bpftool/main.h                      |   2 +-
 tools/bpf/bpftool/net.c                       |  48 +++-
 tools/bpf/bpftool/netlink_dumper.c            |  12 +-
 tools/bpf/bpftool/prog.c                      |   2 +-
 tools/include/uapi/linux/bpf.h                |   3 +
 tools/include/uapi/linux/if_link.h            |   3 +
 tools/lib/bpf/libbpf.c                        |   2 +
 tools/lib/bpf/libbpf.h                        |   9 +-
 tools/lib/bpf/libbpf.map                      |   2 +
 tools/lib/bpf/netlink.c                       |  63 ++++-
 tools/testing/selftests/bpf/Makefile          |   1 +
 .../bpf/prog_tests/xdp_egress_attach.c        |  62 +++++
 .../selftests/bpf/progs/test_xdp_egress.c     |  12 +
 .../bpf/progs/test_xdp_egress_fail.c          |  16 ++
 tools/testing/selftests/bpf/progs/xdp_drop.c  |  25 ++
 .../testing/selftests/bpf/test_xdp_egress.sh  | 159 +++++++++++
 26 files changed, 861 insertions(+), 140 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_egress_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_egress.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_egress_fail.c
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_drop.c
 create mode 100755 tools/testing/selftests/bpf/test_xdp_egress.sh

-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 01/16] net: Refactor convert_to_xdp_frame

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Move the guts of convert_to_xdp_frame to a new helper, update_xdp_frame
so it can be reused in a later patch.

Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/net/xdp.h | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 40c6d3398458..779313862073 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -93,32 +93,42 @@ static inline void xdp_scrub_frame(struct xdp_frame *frame)
 
 struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
 
-/* Convert xdp_buff to xdp_frame */
 static inline
-struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
+bool update_xdp_frame(struct xdp_buff *xdp, struct xdp_frame *xdp_frame)
 {
-	struct xdp_frame *xdp_frame;
 	int metasize;
 	int headroom;
 
-	if (xdp->rxq->mem.type == MEM_TYPE_ZERO_COPY)
-		return xdp_convert_zc_to_xdp_frame(xdp);
-
 	/* Assure headroom is available for storing info */
 	headroom = xdp->data - xdp->data_hard_start;
 	metasize = xdp->data - xdp->data_meta;
 	metasize = metasize > 0 ? metasize : 0;
 	if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
-		return NULL;
-
-	/* Store info in top of packet */
-	xdp_frame = xdp->data_hard_start;
+		return false;
 
 	xdp_frame->data = xdp->data;
 	xdp_frame->len  = xdp->data_end - xdp->data;
 	xdp_frame->headroom = headroom - sizeof(*xdp_frame);
 	xdp_frame->metasize = metasize;
 
+	return true;
+}
+
+/* Convert xdp_buff to xdp_frame */
+static inline
+struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
+{
+	struct xdp_frame *xdp_frame;
+
+	if (xdp->rxq->mem.type == MEM_TYPE_ZERO_COPY)
+		return xdp_convert_zc_to_xdp_frame(xdp);
+
+	/* Store info in top of packet */
+	xdp_frame = xdp->data_hard_start;
+
+	if (unlikely(!update_xdp_frame(xdp, xdp_frame)))
+		return NULL;
+
 	/* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
 	xdp_frame->mem = xdp->rxq->mem;
 
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 02/16] net: Move handling of IFLA_XDP attribute out of do_setlink

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Later patch re-uses XDP attributes. Move processing of IFLA_XDP
to a helper. Code move only; no functional change intended.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 net/core/rtnetlink.c | 94 +++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 709ebbf8ab5b..97e47c292333 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2480,6 +2480,53 @@ static int do_set_master(struct net_device *dev, int ifindex,
 #define DO_SETLINK_MODIFIED	0x01
 /* notify flag means notify + modified. */
 #define DO_SETLINK_NOTIFY	0x03
+
+static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
+			  int *status, struct netlink_ext_ack *extack)
+{
+	struct nlattr *xdp[IFLA_XDP_MAX + 1];
+	u32 xdp_flags = 0;
+	int err;
+
+	err = nla_parse_nested_deprecated(xdp, IFLA_XDP_MAX, tb,
+					  ifla_xdp_policy, NULL);
+	if (err < 0)
+		return err;
+
+	if (xdp[IFLA_XDP_ATTACHED] || xdp[IFLA_XDP_PROG_ID])
+		goto out_einval;
+
+	if (xdp[IFLA_XDP_FLAGS]) {
+		xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
+		if (xdp_flags & ~XDP_FLAGS_MASK)
+			goto out_einval;
+		if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1)
+			goto out_einval;
+	}
+
+	if (xdp[IFLA_XDP_FD]) {
+		int expected_fd = -1;
+
+		if (xdp_flags & XDP_FLAGS_REPLACE) {
+			if (!xdp[IFLA_XDP_EXPECTED_FD])
+				goto out_einval;
+			expected_fd = nla_get_s32(xdp[IFLA_XDP_EXPECTED_FD]);
+		}
+
+		err = dev_change_xdp_fd(dev, extack,
+					nla_get_s32(xdp[IFLA_XDP_FD]),
+					expected_fd, xdp_flags);
+		if (err)
+			return err;
+
+		*status |= DO_SETLINK_NOTIFY;
+	}
+
+	return 0;
+out_einval:
+	return -EINVAL;
+}
+
 static int do_setlink(const struct sk_buff *skb,
 		      struct net_device *dev, struct ifinfomsg *ifm,
 		      struct netlink_ext_ack *extack,
@@ -2774,52 +2821,9 @@ static int do_setlink(const struct sk_buff *skb,
 	}
 
 	if (tb[IFLA_XDP]) {
-		struct nlattr *xdp[IFLA_XDP_MAX + 1];
-		u32 xdp_flags = 0;
-
-		err = nla_parse_nested_deprecated(xdp, IFLA_XDP_MAX,
-						  tb[IFLA_XDP],
-						  ifla_xdp_policy, NULL);
-		if (err < 0)
-			goto errout;
-
-		if (xdp[IFLA_XDP_ATTACHED] || xdp[IFLA_XDP_PROG_ID]) {
-			err = -EINVAL;
+		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, extack);
+		if (err)
 			goto errout;
-		}
-
-		if (xdp[IFLA_XDP_FLAGS]) {
-			xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
-			if (xdp_flags & ~XDP_FLAGS_MASK) {
-				err = -EINVAL;
-				goto errout;
-			}
-			if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1) {
-				err = -EINVAL;
-				goto errout;
-			}
-		}
-
-		if (xdp[IFLA_XDP_FD]) {
-			int expected_fd = -1;
-
-			if (xdp_flags & XDP_FLAGS_REPLACE) {
-				if (!xdp[IFLA_XDP_EXPECTED_FD]) {
-					err = -EINVAL;
-					goto errout;
-				}
-				expected_fd =
-					nla_get_s32(xdp[IFLA_XDP_EXPECTED_FD]);
-			}
-
-			err = dev_change_xdp_fd(dev, extack,
-						nla_get_s32(xdp[IFLA_XDP_FD]),
-						expected_fd,
-						xdp_flags);
-			if (err)
-				goto errout;
-			status |= DO_SETLINK_NOTIFY;
-		}
 	}
 
 errout:
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 03/16] net: Add XDP setup and query commands for Tx programs

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add new netdev commands, XDP_SETUP_PROG_EGRESS and
XDP_QUERY_PROG_EGRESS, to query and setup egress programs.

Update dev_change_xdp_fd and dev_xdp_install to take an egress argument.
If egress bool is set, then use XDP_SETUP_PROG_EGRESS in dev_xdp_install
and XDP_QUERY_PROG_EGRESS in dev_change_xdp_fd.

Update dev_xdp_uninstall to query for XDP_QUERY_PROG_EGRESS and if a
program is installed call dev_xdp_install with the egress argument set
to true.

This enables existing infrastructure to be used for XDP programs in
the egress path.

Signed-off-by: David Ahern <dahern@digitalocean.com>
Co-developed-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
 include/linux/netdevice.h |  4 +++-
 net/core/dev.c            | 34 +++++++++++++++++++++++-----------
 net/core/rtnetlink.c      |  2 +-
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 130a668049ab..d0bb9e09660a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -871,8 +871,10 @@ enum bpf_netdev_command {
 	 */
 	XDP_SETUP_PROG,
 	XDP_SETUP_PROG_HW,
+	XDP_SETUP_PROG_EGRESS,
 	XDP_QUERY_PROG,
 	XDP_QUERY_PROG_HW,
+	XDP_QUERY_PROG_EGRESS,
 	/* BPF program for offload callbacks, invoked at program load time. */
 	BPF_OFFLOAD_MAP_ALLOC,
 	BPF_OFFLOAD_MAP_FREE,
@@ -3777,7 +3779,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 
 typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
 int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
-		      int fd, int expected_fd, u32 flags);
+		      int fd, int expected_fd, u32 flags, bool egress);
 u32 __dev_xdp_query(struct net_device *dev, bpf_op_t xdp_op,
 		    enum bpf_netdev_command cmd);
 int xdp_umem_query(struct net_device *dev, u16 queue_id);
diff --git a/net/core/dev.c b/net/core/dev.c
index 522288177bbd..97180458e7cb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8590,7 +8590,7 @@ u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
 
 static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 			   struct netlink_ext_ack *extack, u32 flags,
-			   struct bpf_prog *prog)
+			   struct bpf_prog *prog, bool egress)
 {
 	bool non_hw = !(flags & XDP_FLAGS_HW_MODE);
 	struct bpf_prog *prev_prog = NULL;
@@ -8598,8 +8598,10 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 	int err;
 
 	if (non_hw) {
-		prev_prog = bpf_prog_by_id(__dev_xdp_query(dev, bpf_op,
-							   XDP_QUERY_PROG));
+		enum bpf_netdev_command cmd;
+
+		cmd = egress ? XDP_QUERY_PROG_EGRESS : XDP_QUERY_PROG;
+		prev_prog = bpf_prog_by_id(__dev_xdp_query(dev, bpf_op, cmd));
 		if (IS_ERR(prev_prog))
 			prev_prog = NULL;
 	}
@@ -8608,7 +8610,7 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 	if (flags & XDP_FLAGS_HW_MODE)
 		xdp.command = XDP_SETUP_PROG_HW;
 	else
-		xdp.command = XDP_SETUP_PROG;
+		xdp.command = egress ? XDP_SETUP_PROG_EGRESS : XDP_SETUP_PROG;
 	xdp.extack = extack;
 	xdp.flags = flags;
 	xdp.prog = prog;
@@ -8629,7 +8631,12 @@ static void dev_xdp_uninstall(struct net_device *dev)
 	bpf_op_t ndo_bpf;
 
 	/* Remove generic XDP */
-	WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL, 0, NULL));
+	WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL, 0, NULL,
+				false));
+
+	/* Remove XDP egress */
+	WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL, 0, NULL,
+				true));
 
 	/* Remove from the driver */
 	ndo_bpf = dev->netdev_ops->ndo_bpf;
@@ -8641,14 +8648,14 @@ static void dev_xdp_uninstall(struct net_device *dev)
 	WARN_ON(ndo_bpf(dev, &xdp));
 	if (xdp.prog_id)
 		WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
-					NULL));
+					NULL, false));
 
 	/* Remove HW offload */
 	memset(&xdp, 0, sizeof(xdp));
 	xdp.command = XDP_QUERY_PROG_HW;
 	if (!ndo_bpf(dev, &xdp) && xdp.prog_id)
 		WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
-					NULL));
+					NULL, false));
 }
 
 /**
@@ -8662,7 +8669,7 @@ static void dev_xdp_uninstall(struct net_device *dev)
  *	Set or clear a bpf program for a device
  */
 int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
-		      int fd, int expected_fd, u32 flags)
+		      int fd, int expected_fd, u32 flags, bool egress)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
 	enum bpf_netdev_command query;
@@ -8675,7 +8682,11 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 	ASSERT_RTNL();
 
 	offload = flags & XDP_FLAGS_HW_MODE;
-	query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+	if (egress)
+		query = XDP_QUERY_PROG_EGRESS;
+	else
+		query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+
 
 	bpf_op = bpf_chk = ops->ndo_bpf;
 	if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
@@ -8705,7 +8716,8 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		}
 	}
 	if (fd >= 0) {
-		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
+		if (!offload && !egress &&
+		    __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
 			NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
 			return -EEXIST;
 		}
@@ -8737,7 +8749,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		prog = NULL;
 	}
 
-	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
+	err = dev_xdp_install(dev, bpf_op, extack, flags, prog, egress);
 	if (err < 0 && prog)
 		bpf_prog_put(prog);
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 97e47c292333..dc44af16226a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2515,7 +2515,7 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
 
 		err = dev_change_xdp_fd(dev, extack,
 					nla_get_s32(xdp[IFLA_XDP_FD]),
-					expected_fd, xdp_flags);
+					expected_fd, xdp_flags, false);
 		if (err)
 			return err;
 
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add new bpf_attach_type, BPF_XDP_EGRESS, for BPF programs attached
at the XDP layer, but the egress path.

Since egress path will not have ingress_ifindex and rx_queue_index
set, update xdp_is_valid_access to block access to these entries in
the xdp context when a program is attached to egress path.

Update dev_change_xdp_fd to verify expected_attach_type for a program
is BPF_XDP_EGRESS if egress argument is set.

The next patch adds support for the egress ifindex.

Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/uapi/linux/bpf.h       |  1 +
 net/core/dev.c                 | 11 +++++++++++
 net/core/filter.c              |  8 ++++++++
 tools/include/uapi/linux/bpf.h |  1 +
 4 files changed, 21 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 2e29a671d67e..a9d384998e8b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -215,6 +215,7 @@ enum bpf_attach_type {
 	BPF_TRACE_FEXIT,
 	BPF_MODIFY_RETURN,
 	BPF_LSM_MAC,
+	BPF_XDP_EGRESS,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 97180458e7cb..e8a62bdb395b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8732,6 +8732,17 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
+		if (egress && prog->expected_attach_type != BPF_XDP_EGRESS) {
+			NL_SET_ERR_MSG(extack, "XDP program in Tx path must use BPF_XDP_EGRESS attach type");
+			bpf_prog_put(prog);
+			return -EINVAL;
+		}
+		if (!egress && prog->expected_attach_type == BPF_XDP_EGRESS) {
+			NL_SET_ERR_MSG(extack, "XDP program in Rx path can not use BPF_XDP_EGRESS attach type");
+			bpf_prog_put(prog);
+			return -EINVAL;
+		}
+
 		if (!offload && bpf_prog_is_dev_bound(prog->aux)) {
 			NL_SET_ERR_MSG(extack, "using device-bound program without HW_MODE flag is not supported");
 			bpf_prog_put(prog);
diff --git a/net/core/filter.c b/net/core/filter.c
index 7d6ceaa54d21..bcb56448f336 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6935,6 +6935,14 @@ static bool xdp_is_valid_access(int off, int size,
 				const struct bpf_prog *prog,
 				struct bpf_insn_access_aux *info)
 {
+	if (prog->expected_attach_type == BPF_XDP_EGRESS) {
+		switch (off) {
+		case offsetof(struct xdp_md, ingress_ifindex):
+		case offsetof(struct xdp_md, rx_queue_index):
+			return false;
+		}
+	}
+
 	if (type == BPF_WRITE) {
 		if (bpf_prog_is_dev_bound(prog->aux)) {
 			switch (off) {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 2e29a671d67e..a9d384998e8b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -215,6 +215,7 @@ enum bpf_attach_type {
 	BPF_TRACE_FEXIT,
 	BPF_MODIFY_RETURN,
 	BPF_LSM_MAC,
+	BPF_XDP_EGRESS,
 	__MAX_BPF_ATTACH_TYPE
 };
 
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 05/16] xdp: Add xdp_txq_info to xdp_buff

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add xdp_txq_info as the Tx counterpart to xdp_rxq_info. At the
moment only the device is added. Other fields (queue_index)
can be added as use cases arise.

From a UAPI perspective, add egress_ifindex to xdp context.

Update the verifier to reject accesses to egress_ifindex by
rx programs.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/net/xdp.h              |  5 +++++
 include/uapi/linux/bpf.h       |  2 ++
 net/core/filter.c              | 15 +++++++++++++++
 tools/include/uapi/linux/bpf.h |  2 ++
 4 files changed, 24 insertions(+)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 779313862073..7e4ca99def77 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -63,6 +63,10 @@ struct xdp_rxq_info {
 	struct xdp_mem_info mem;
 } ____cacheline_aligned; /* perf critical, avoid false-sharing */
 
+struct xdp_txq_info {
+	struct net_device *dev;
+};
+
 struct xdp_buff {
 	void *data;
 	void *data_end;
@@ -70,6 +74,7 @@ struct xdp_buff {
 	void *data_hard_start;
 	unsigned long handle;
 	struct xdp_rxq_info *rxq;
+	struct xdp_txq_info *txq;
 };
 
 struct xdp_frame {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a9d384998e8b..35e3aab97dd4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3487,6 +3487,8 @@ struct xdp_md {
 	/* Below access go through struct xdp_rxq_info */
 	__u32 ingress_ifindex; /* rxq->dev->ifindex */
 	__u32 rx_queue_index;  /* rxq->queue_index  */
+
+	__u32 egress_ifindex;  /* txq->dev->ifindex */
 };
 
 enum sk_action {
diff --git a/net/core/filter.c b/net/core/filter.c
index bcb56448f336..273653f44cb0 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6941,6 +6941,11 @@ static bool xdp_is_valid_access(int off, int size,
 		case offsetof(struct xdp_md, rx_queue_index):
 			return false;
 		}
+	} else {
+		switch (off) {
+		case offsetof(struct xdp_md, egress_ifindex):
+			return false;
+		}
 	}
 
 	if (type == BPF_WRITE) {
@@ -7890,6 +7895,16 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
 				      offsetof(struct xdp_rxq_info,
 					       queue_index));
 		break;
+	case offsetof(struct xdp_md, egress_ifindex):
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_buff, txq),
+				      si->dst_reg, si->src_reg,
+				      offsetof(struct xdp_buff, txq));
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_txq_info, dev),
+				      si->dst_reg, si->dst_reg,
+				      offsetof(struct xdp_txq_info, dev));
+		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+				      offsetof(struct net_device, ifindex));
+		break;
 	}
 
 	return insn - insn_buf;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a9d384998e8b..35e3aab97dd4 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3487,6 +3487,8 @@ struct xdp_md {
 	/* Below access go through struct xdp_rxq_info */
 	__u32 ingress_ifindex; /* rxq->dev->ifindex */
 	__u32 rx_queue_index;  /* rxq->queue_index  */
+
+	__u32 egress_ifindex;  /* txq->dev->ifindex */
 };
 
 enum sk_action {
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 06/16] net: Add IFLA_XDP_EGRESS for XDP programs in the egress path

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Running programs in the egress path, on skbs or xdp_frames, does not
require driver specific resources like Rx path. Accordingly, the
programs can be run in core code, so add xdp_egress_prog to net_device
to hold a reference to an attached program.

For UAPI, add IFLA_XDP_EGRESS to if_link.h to specify egress programs,
add a new attach flag, XDP_ATTACHED_EGRESS_CORE, to denote the
attach point is at the core level (as opposed to driver or hardware)
and add IFLA_XDP_EGRESS_CORE_PROG_ID for reporting the program id.

Add egress argument to do_setlink_xdp to denote processing of
IFLA_XDP_EGRESS versus IFLA_XDP, and add a check that none of the
existing modes (SKB, DRV or HW) are set since those modes are not
valid. The expectation is that XDP_FLAGS_HW_MODE will be used later
(e.g., offloading guest programs).

Add rtnl_xdp_egress_fill and helpers as the egress counterpart to the
existing rtnl_xdp_fill.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/linux/netdevice.h          |  1 +
 include/uapi/linux/if_link.h       |  3 +
 net/core/rtnetlink.c               | 96 ++++++++++++++++++++++++++++--
 tools/include/uapi/linux/if_link.h |  3 +
 4 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d0bb9e09660a..3133247681fd 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1995,6 +1995,7 @@ struct net_device {
 	unsigned int		real_num_rx_queues;
 
 	struct bpf_prog __rcu	*xdp_prog;
+	struct bpf_prog __rcu	*xdp_egress_prog;
 	unsigned long		gro_flush_timeout;
 	rx_handler_func_t __rcu	*rx_handler;
 	void __rcu		*rx_handler_data;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 127c704eeba9..b3c6cb2f0f0a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -170,6 +170,7 @@ enum {
 	IFLA_PROP_LIST,
 	IFLA_ALT_IFNAME, /* Alternative ifname */
 	IFLA_PERM_ADDRESS,
+	IFLA_XDP_EGRESS, /* nested attribute with 1 or more IFLA_XDP_ attrs */
 	__IFLA_MAX
 };
 
@@ -988,6 +989,7 @@ enum {
 	XDP_ATTACHED_SKB,
 	XDP_ATTACHED_HW,
 	XDP_ATTACHED_MULTI,
+	XDP_ATTACHED_EGRESS_CORE,
 };
 
 enum {
@@ -1000,6 +1002,7 @@ enum {
 	IFLA_XDP_SKB_PROG_ID,
 	IFLA_XDP_HW_PROG_ID,
 	IFLA_XDP_EXPECTED_FD,
+	IFLA_XDP_EGRESS_CORE_PROG_ID,
 	__IFLA_XDP_MAX,
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dc44af16226a..e9bc5cee06c8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1030,7 +1030,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */
 	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
 	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
-	       + rtnl_xdp_size() /* IFLA_XDP */
+	       + rtnl_xdp_size() * 2 /* IFLA_XDP and IFLA_XDP_EGRESS */
 	       + nla_total_size(4)  /* IFLA_EVENT */
 	       + nla_total_size(4)  /* IFLA_NEW_NETNSID */
 	       + nla_total_size(4)  /* IFLA_NEW_IFINDEX */
@@ -1395,6 +1395,42 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static u32 rtnl_xdp_egress_prog(struct net_device *dev)
+{
+	const struct bpf_prog *prog;
+
+	ASSERT_RTNL();
+
+	prog = rtnl_dereference(dev->xdp_egress_prog);
+	if (!prog)
+		return 0;
+	return prog->aux->id;
+}
+
+static int rtnl_xdp_egress_report(struct sk_buff *skb, struct net_device *dev,
+				  u32 *prog_id, u8 *mode, u8 tgt_mode, u32 attr,
+				  u32 (*get_prog_id)(struct net_device *dev))
+{
+	u32 curr_id;
+	int err;
+
+	curr_id = get_prog_id(dev);
+	if (!curr_id)
+		return 0;
+
+	*prog_id = curr_id;
+	err = nla_put_u32(skb, attr, curr_id);
+	if (err)
+		return err;
+
+	if (*mode != XDP_ATTACHED_NONE)
+		*mode = XDP_ATTACHED_MULTI;
+	else
+		*mode = tgt_mode;
+
+	return 0;
+}
+
 static u32 rtnl_xdp_prog_skb(struct net_device *dev)
 {
 	const struct bpf_prog *generic_xdp_prog;
@@ -1486,6 +1522,42 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 	return err;
 }
 
+static int rtnl_xdp_egress_fill(struct sk_buff *skb, struct net_device *dev)
+{
+	u8 mode = XDP_ATTACHED_NONE;
+	struct nlattr *xdp;
+	u32 prog_id = 0;
+	int err;
+
+	xdp = nla_nest_start_noflag(skb, IFLA_XDP_EGRESS);
+	if (!xdp)
+		return -EMSGSIZE;
+
+	err = rtnl_xdp_egress_report(skb, dev, &prog_id, &mode,
+				     XDP_ATTACHED_EGRESS_CORE,
+				     IFLA_XDP_EGRESS_CORE_PROG_ID,
+				     rtnl_xdp_egress_prog);
+	if (err)
+		goto err_cancel;
+
+	err = nla_put_u8(skb, IFLA_XDP_ATTACHED, mode);
+	if (err)
+		goto err_cancel;
+
+	if (prog_id && mode != XDP_ATTACHED_MULTI) {
+		err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
+		if (err)
+			goto err_cancel;
+	}
+
+	nla_nest_end(skb, xdp);
+	return 0;
+
+err_cancel:
+	nla_nest_cancel(skb, xdp);
+	return err;
+}
+
 static u32 rtnl_get_event(unsigned long event)
 {
 	u32 rtnl_event_type = IFLA_EVENT_NONE;
@@ -1743,6 +1815,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
 	if (rtnl_xdp_fill(skb, dev))
 		goto nla_put_failure;
 
+	if (rtnl_xdp_egress_fill(skb, dev))
+		goto nla_put_failure;
+
 	if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
 		if (rtnl_link_fill(skb, dev) < 0)
 			goto nla_put_failure;
@@ -1827,6 +1902,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_ALT_IFNAME]	= { .type = NLA_STRING,
 				    .len = ALTIFNAMSIZ - 1 },
 	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
+	[IFLA_XDP_EGRESS]	= { .type = NLA_NESTED },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -2482,7 +2558,8 @@ static int do_set_master(struct net_device *dev, int ifindex,
 #define DO_SETLINK_NOTIFY	0x03
 
 static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
-			  int *status, struct netlink_ext_ack *extack)
+			  int *status, bool egress,
+			  struct netlink_ext_ack *extack)
 {
 	struct nlattr *xdp[IFLA_XDP_MAX + 1];
 	u32 xdp_flags = 0;
@@ -2498,6 +2575,10 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
 
 	if (xdp[IFLA_XDP_FLAGS]) {
 		xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
+		if (egress && xdp_flags & XDP_FLAGS_MODES) {
+			NL_SET_ERR_MSG(extack, "XDP_FLAGS_MODES not valid for egress");
+			goto out_einval;
+		}
 		if (xdp_flags & ~XDP_FLAGS_MASK)
 			goto out_einval;
 		if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1)
@@ -2515,7 +2596,7 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
 
 		err = dev_change_xdp_fd(dev, extack,
 					nla_get_s32(xdp[IFLA_XDP_FD]),
-					expected_fd, xdp_flags, false);
+					expected_fd, xdp_flags, egress);
 		if (err)
 			return err;
 
@@ -2821,7 +2902,14 @@ static int do_setlink(const struct sk_buff *skb,
 	}
 
 	if (tb[IFLA_XDP]) {
-		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, extack);
+		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, false, extack);
+		if (err)
+			goto errout;
+	}
+
+	if (tb[IFLA_XDP_EGRESS]) {
+		err = do_setlink_xdp(dev, tb[IFLA_XDP_EGRESS], &status, true,
+				     extack);
 		if (err)
 			goto errout;
 	}
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index ca6665ea758a..f9e665aa836a 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -170,6 +170,7 @@ enum {
 	IFLA_PROP_LIST,
 	IFLA_ALT_IFNAME, /* Alternative ifname */
 	IFLA_PERM_ADDRESS,
+	IFLA_XDP_EGRESS, /* nested attribute with 1 or more IFLA_XDP_ attrs */
 	__IFLA_MAX
 };
 
@@ -976,6 +977,7 @@ enum {
 	XDP_ATTACHED_SKB,
 	XDP_ATTACHED_HW,
 	XDP_ATTACHED_MULTI,
+	XDP_ATTACHED_EGRESS_CORE,
 };
 
 enum {
@@ -988,6 +990,7 @@ enum {
 	IFLA_XDP_SKB_PROG_ID,
 	IFLA_XDP_HW_PROG_ID,
 	IFLA_XDP_EXPECTED_FD,
+	IFLA_XDP_EGRESS_CORE_PROG_ID,
 	__IFLA_XDP_MAX,
 };
 
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 07/16] net: Rename do_xdp_generic to do_xdp_generic_rx

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Rename do_xdp_generic to do_xdp_generic_rx to emphasize its use in the
Rx path.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 drivers/net/tun.c         | 4 ++--
 include/linux/netdevice.h | 2 +-
 net/core/dev.c            | 7 ++++---
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 44889eba1dbc..efe655d27661 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1898,7 +1898,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		rcu_read_lock();
 		xdp_prog = rcu_dereference(tun->xdp_prog);
 		if (xdp_prog) {
-			ret = do_xdp_generic(xdp_prog, skb);
+			ret = do_xdp_generic_rx(xdp_prog, skb);
 			if (ret != XDP_PASS) {
 				rcu_read_unlock();
 				local_bh_enable();
@@ -2463,7 +2463,7 @@ static int tun_xdp_one(struct tun_struct *tun,
 	skb_record_rx_queue(skb, tfile->queue_index);
 
 	if (skb_xdp) {
-		err = do_xdp_generic(xdp_prog, skb);
+		err = do_xdp_generic_rx(xdp_prog, skb);
 		if (err != XDP_PASS)
 			goto out;
 	}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3133247681fd..2649f2b36858 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3711,7 +3711,7 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
 }
 
 void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
-int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb);
+int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb);
 int netif_rx(struct sk_buff *skb);
 int netif_rx_ni(struct sk_buff *skb);
 int netif_receive_skb(struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index e8a62bdb395b..18b93d34633c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4642,7 +4642,7 @@ void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog)
 
 static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
 
-int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
+int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 {
 	if (xdp_prog) {
 		struct xdp_buff xdp;
@@ -4670,7 +4670,7 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 	kfree_skb(skb);
 	return XDP_DROP;
 }
-EXPORT_SYMBOL_GPL(do_xdp_generic);
+EXPORT_SYMBOL_GPL(do_xdp_generic_rx);
 
 static int netif_rx_internal(struct sk_buff *skb)
 {
@@ -5020,7 +5020,8 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
 		int ret2;
 
 		preempt_disable();
-		ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);
+		ret2 = do_xdp_generic_rx(rcu_dereference(skb->dev->xdp_prog),
+					 skb);
 		preempt_enable();
 
 		if (ret2 != XDP_PASS)
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 08/16] net: rename netif_receive_generic_xdp to do_generic_xdp_core

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

In skb generic path, we need a way to run XDP program on skb but
to have customized handling of XDP actions. netif_receive_generic_xdp
will be more helpful in such cases than do_xdp_generic.

This patch prepares netif_receive_generic_xdp() to be used as general
purpose function for running xdp programs on skbs by renaming it to
do_xdp_generic_core, moving skb_is_redirected and rxq settings as well
as XDP return code checks to the callers.

This allows this core function to be used from both Rx and Tx paths
with rxq and txq set based on context.

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 net/core/dev.c | 52 ++++++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 18b93d34633c..046455c54b03 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4501,25 +4501,17 @@ static struct netdev_rx_queue *netif_get_rxqueue(struct sk_buff *skb)
 	return rxqueue;
 }
 
-static u32 netif_receive_generic_xdp(struct sk_buff *skb,
-				     struct xdp_buff *xdp,
-				     struct bpf_prog *xdp_prog)
+static u32 do_xdp_generic_core(struct sk_buff *skb, struct xdp_buff *xdp,
+			       struct bpf_prog *xdp_prog)
 {
-	struct netdev_rx_queue *rxqueue;
 	void *orig_data, *orig_data_end;
-	u32 metalen, act = XDP_DROP;
 	__be16 orig_eth_type;
 	struct ethhdr *eth;
+	u32 metalen, act;
 	bool orig_bcast;
 	int hlen, off;
 	u32 mac_len;
 
-	/* Reinjected packets coming from act_mirred or similar should
-	 * not get XDP generic processing.
-	 */
-	if (skb_is_redirected(skb))
-		return XDP_PASS;
-
 	/* XDP packets must be linear and must have sufficient headroom
 	 * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
 	 * native XDP provides, thus we need to do it here as well.
@@ -4535,9 +4527,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
 		if (pskb_expand_head(skb,
 				     hroom > 0 ? ALIGN(hroom, NET_SKB_PAD) : 0,
 				     troom > 0 ? troom + 128 : 0, GFP_ATOMIC))
-			goto do_drop;
+			return XDP_DROP;
 		if (skb_linearize(skb))
-			goto do_drop;
+			return XDP_DROP;
 	}
 
 	/* The XDP program wants to see the packet starting at the MAC
@@ -4555,9 +4547,6 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
 	orig_bcast = is_multicast_ether_addr_64bits(eth->h_dest);
 	orig_eth_type = eth->h_proto;
 
-	rxqueue = netif_get_rxqueue(skb);
-	xdp->rxq = &rxqueue->xdp_rxq;
-
 	act = bpf_prog_run_xdp(xdp_prog, xdp);
 
 	/* check if bpf_xdp_adjust_head was used */
@@ -4600,16 +4589,6 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
 		if (metalen)
 			skb_metadata_set(skb, metalen);
 		break;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-		/* fall through */
-	case XDP_ABORTED:
-		trace_xdp_exception(skb->dev, xdp_prog, act);
-		/* fall through */
-	case XDP_DROP:
-	do_drop:
-		kfree_skb(skb);
-		break;
 	}
 
 	return act;
@@ -4644,12 +4623,22 @@ static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
 
 int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 {
+	/* Reinjected packets coming from act_mirred or similar should
+	 * not get XDP generic processing.
+	 */
+	if (skb_is_redirected(skb))
+		return XDP_PASS;
+
 	if (xdp_prog) {
+		struct netdev_rx_queue *rxqueue;
 		struct xdp_buff xdp;
 		u32 act;
 		int err;
 
-		act = netif_receive_generic_xdp(skb, &xdp, xdp_prog);
+		rxqueue = netif_get_rxqueue(skb);
+		xdp.rxq = &rxqueue->xdp_rxq;
+
+		act = do_xdp_generic_core(skb, &xdp, xdp_prog);
 		if (act != XDP_PASS) {
 			switch (act) {
 			case XDP_REDIRECT:
@@ -4661,6 +4650,15 @@ int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 			case XDP_TX:
 				generic_xdp_tx(skb, xdp_prog);
 				break;
+			default:
+				bpf_warn_invalid_xdp_action(act);
+				/* fall through */
+			case XDP_ABORTED:
+				trace_xdp_exception(skb->dev, xdp_prog, act);
+				/* fall through */
+			case XDP_DROP:
+				kfree_skb(skb);
+				break;
 			}
 			return XDP_DROP;
 		}
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 09/16] net: set XDP egress program on netdevice

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

This patch adds a way to set tx path XDP program on a net_device
by handling XDP_SETUP_PROG_EGRESS and XDP_QUERY_PROG_EGRESS in
generic_xdp_install handler. New static key is added to signal
when an egress program has been installed.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/linux/netdevice.h |  2 ++
 net/core/dev.c            | 44 ++++++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2649f2b36858..0c89996a6bec 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -750,6 +750,8 @@ struct netdev_rx_queue {
 #endif
 } ____cacheline_aligned_in_smp;
 
+extern struct static_key_false xdp_egress_needed_key;
+
 /*
  * RX queue sysfs structures and functions.
  */
diff --git a/net/core/dev.c b/net/core/dev.c
index 046455c54b03..6718048c3448 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4620,6 +4620,7 @@ void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog)
 }
 
 static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
+DEFINE_STATIC_KEY_FALSE(xdp_egress_needed_key);
 
 int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 {
@@ -5335,12 +5336,12 @@ static void __netif_receive_skb_list(struct list_head *head)
 
 static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 {
-	struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
-	struct bpf_prog *new = xdp->prog;
+	struct bpf_prog *old, *new = xdp->prog;
 	int ret = 0;
 
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
+		old = rtnl_dereference(dev->xdp_prog);
 		rcu_assign_pointer(dev->xdp_prog, new);
 		if (old)
 			bpf_prog_put(old);
@@ -5353,11 +5354,25 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 			dev_disable_gro_hw(dev);
 		}
 		break;
+	case XDP_SETUP_PROG_EGRESS:
+		old = rtnl_dereference(dev->xdp_egress_prog);
+		rcu_assign_pointer(dev->xdp_egress_prog, new);
+		if (old)
+			bpf_prog_put(old);
 
+		if (old && !new)
+			static_branch_dec(&xdp_egress_needed_key);
+		else if (new && !old)
+			static_branch_inc(&xdp_egress_needed_key);
+		break;
 	case XDP_QUERY_PROG:
+		old = rtnl_dereference(dev->xdp_prog);
+		xdp->prog_id = old ? old->aux->id : 0;
+		break;
+	case XDP_QUERY_PROG_EGRESS:
+		old = rtnl_dereference(dev->xdp_egress_prog);
 		xdp->prog_id = old ? old->aux->id : 0;
 		break;
-
 	default:
 		ret = -EINVAL;
 		break;
@@ -8681,21 +8696,22 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 	ASSERT_RTNL();
 
 	offload = flags & XDP_FLAGS_HW_MODE;
-	if (egress)
+	if (egress) {
 		query = XDP_QUERY_PROG_EGRESS;
-	else
+		bpf_op = bpf_chk = generic_xdp_install;
+	} else {
 		query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
 
-
-	bpf_op = bpf_chk = ops->ndo_bpf;
-	if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
-		NL_SET_ERR_MSG(extack, "underlying driver does not support XDP in native mode");
-		return -EOPNOTSUPP;
+		bpf_op = bpf_chk = ops->ndo_bpf;
+		if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
+			NL_SET_ERR_MSG(extack, "underlying driver does not support XDP in native mode");
+			return -EOPNOTSUPP;
+		}
+		if (!bpf_op || (flags & XDP_FLAGS_SKB_MODE))
+			bpf_op = generic_xdp_install;
+		if (bpf_op == bpf_chk)
+			bpf_chk = generic_xdp_install;
 	}
-	if (!bpf_op || (flags & XDP_FLAGS_SKB_MODE))
-		bpf_op = generic_xdp_install;
-	if (bpf_op == bpf_chk)
-		bpf_chk = generic_xdp_install;
 
 	prog_id = __dev_xdp_query(dev, bpf_op, query);
 	if (flags & XDP_FLAGS_REPLACE) {
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 10/16] net: Support xdp in the Tx path for packets as an skb

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add support to run Tx path program on packets about to hit the
ndo_start_xmit function for a device. Only XDP_DROP and XDP_PASS
are supported now. Conceptually, XDP_REDIRECT for this path can
work the same as it does for the Rx path, but that support is left
for a follow on series.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/linux/netdevice.h | 11 +++++++++
 net/core/dev.c            | 52 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0c89996a6bec..39e1b42c042f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3714,6 +3714,7 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
 
 void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
 int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb);
+u32 do_xdp_egress_skb(struct net_device *dev, struct sk_buff *skb);
 int netif_rx(struct sk_buff *skb);
 int netif_rx_ni(struct sk_buff *skb);
 int netif_receive_skb(struct sk_buff *skb);
@@ -4534,6 +4535,16 @@ static inline netdev_tx_t __netdev_start_xmit(const struct net_device_ops *ops,
 					      struct sk_buff *skb, struct net_device *dev,
 					      bool more)
 {
+	if (static_branch_unlikely(&xdp_egress_needed_key)) {
+		u32 act;
+
+		rcu_read_lock();
+		act = do_xdp_egress_skb(dev, skb);
+		rcu_read_unlock();
+		if (act == XDP_DROP)
+			return NET_XMIT_DROP;
+	}
+
 	__this_cpu_write(softnet_data.xmit.more, more);
 	return ops->ndo_start_xmit(skb, dev);
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 6718048c3448..d47e88beeeca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4620,7 +4620,6 @@ void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog)
 }
 
 static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
-DEFINE_STATIC_KEY_FALSE(xdp_egress_needed_key);
 
 int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 {
@@ -4671,6 +4670,57 @@ int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(do_xdp_generic_rx);
 
+DEFINE_STATIC_KEY_FALSE(xdp_egress_needed_key);
+EXPORT_SYMBOL_GPL(xdp_egress_needed_key);
+
+static u32 handle_xdp_egress_act(u32 act, struct net_device *dev,
+				 struct bpf_prog *xdp_prog)
+{
+	switch (act) {
+	case XDP_DROP:
+		/* fall through */
+	case XDP_PASS:
+		break;
+	case XDP_TX:
+		/* fall through */
+	case XDP_REDIRECT:
+		/* fall through */
+	default:
+		bpf_warn_invalid_xdp_action(act);
+		/* fall through */
+	case XDP_ABORTED:
+		trace_xdp_exception(dev, xdp_prog, act);
+		act = XDP_DROP;
+		break;
+	}
+
+	return act;
+}
+
+u32 do_xdp_egress_skb(struct net_device *dev, struct sk_buff *skb)
+{
+	struct bpf_prog *xdp_prog;
+	u32 act = XDP_PASS;
+
+	xdp_prog = rcu_dereference(dev->xdp_egress_prog);
+	if (xdp_prog) {
+		struct xdp_txq_info txq = { .dev = dev };
+		struct xdp_buff xdp;
+
+		xdp.txq = &txq;
+		act = do_xdp_generic_core(skb, &xdp, xdp_prog);
+		act = handle_xdp_egress_act(act, dev, xdp_prog);
+		if (act == XDP_DROP) {
+			atomic_long_inc(&dev->tx_dropped);
+			skb_tx_error(skb);
+			kfree_skb(skb);
+		}
+	}
+
+	return act;
+}
+EXPORT_SYMBOL_GPL(do_xdp_egress_skb);
+
 static int netif_rx_internal(struct sk_buff *skb)
 {
 	int ret;
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 11/16] net: Support xdp in the Tx path for xdp_frames

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add support to run Tx path program on xdp_frames by adding a hook to
bq_xmit_all before xdp_frames are passed to ndo_xdp_xmit for the device.

If an xdp_frame is dropped by the program, it is removed from the
xdp_frames array with subsequent entries moved up.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 include/linux/netdevice.h |  3 ++
 kernel/bpf/devmap.c       | 19 +++++++++----
 net/core/dev.c            | 59 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 39e1b42c042f..d18d68ddf604 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3715,6 +3715,9 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
 void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
 int do_xdp_generic_rx(struct bpf_prog *xdp_prog, struct sk_buff *skb);
 u32 do_xdp_egress_skb(struct net_device *dev, struct sk_buff *skb);
+unsigned int do_xdp_egress_frame(struct net_device *dev,
+				 struct xdp_frame **frames,
+				 unsigned int count);
 int netif_rx(struct sk_buff *skb);
 int netif_rx_ni(struct sk_buff *skb);
 int netif_receive_skb(struct sk_buff *skb);
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 58bdca5d978a..c038c8c3ccdb 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -322,24 +322,33 @@ static int bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 {
 	struct net_device *dev = bq->dev;
 	int sent = 0, drops = 0, err = 0;
+	unsigned int count = bq->count;
 	int i;
 
-	if (unlikely(!bq->count))
+	if (unlikely(!count))
 		return 0;
 
-	for (i = 0; i < bq->count; i++) {
+	for (i = 0; i < count; i++) {
 		struct xdp_frame *xdpf = bq->q[i];
 
 		prefetch(xdpf);
 	}
 
-	sent = dev->netdev_ops->ndo_xdp_xmit(dev, bq->count, bq->q, flags);
+	if (static_branch_unlikely(&xdp_egress_needed_key)) {
+		count = do_xdp_egress_frame(dev, bq->q, count);
+		drops += bq->count - count;
+		/* all frames consumed by the xdp program? */
+		if (!count)
+			goto out;
+	}
+
+	sent = dev->netdev_ops->ndo_xdp_xmit(dev, count, bq->q, flags);
 	if (sent < 0) {
 		err = sent;
 		sent = 0;
 		goto error;
 	}
-	drops = bq->count - sent;
+	drops += count - sent;
 out:
 	bq->count = 0;
 
@@ -351,7 +360,7 @@ static int bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags)
 	/* If ndo_xdp_xmit fails with an errno, no frames have been
 	 * xmit'ed and it's our responsibility to them free all.
 	 */
-	for (i = 0; i < bq->count; i++) {
+	for (i = 0; i < count; i++) {
 		struct xdp_frame *xdpf = bq->q[i];
 
 		xdp_return_frame_rx_napi(xdpf);
diff --git a/net/core/dev.c b/net/core/dev.c
index d47e88beeeca..60fb1884e4ef 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4721,6 +4721,65 @@ u32 do_xdp_egress_skb(struct net_device *dev, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(do_xdp_egress_skb);
 
+static u32 __xdp_egress_frame(struct net_device *dev,
+			      struct bpf_prog *xdp_prog,
+			      struct xdp_frame *xdp_frame,
+			      struct xdp_txq_info *txq)
+{
+	struct xdp_buff xdp;
+	u32 act;
+
+	xdp.data_hard_start = xdp_frame->data - xdp_frame->headroom
+			      - sizeof(*xdp_frame);
+	xdp.data = xdp_frame->data;
+	xdp.data_end = xdp.data + xdp_frame->len;
+	xdp.data_meta = xdp.data - xdp_frame->metasize;
+	xdp.txq = txq;
+
+	act = bpf_prog_run_xdp(xdp_prog, &xdp);
+	act = handle_xdp_egress_act(act, dev, xdp_prog);
+
+	/* if not dropping frame, readjust pointers in case
+	 * program made changes to the buffer
+	 */
+	if (act != XDP_DROP) {
+		if (unlikely(!update_xdp_frame(&xdp, xdp_frame)))
+			return XDP_DROP;
+	}
+
+	return act;
+}
+
+unsigned int do_xdp_egress_frame(struct net_device *dev,
+				 struct xdp_frame **frames,
+				 unsigned int count)
+{
+	struct bpf_prog *xdp_prog;
+
+	xdp_prog = rcu_dereference(dev->xdp_egress_prog);
+	if (xdp_prog) {
+		struct xdp_txq_info txq = { .dev = dev };
+		unsigned int i, j;
+		u32 act;
+
+		for (i = 0, j = 0; i < count; i++) {
+			struct xdp_frame *frame = frames[i];
+
+			act = __xdp_egress_frame(dev, xdp_prog, frame, &txq);
+			if (act == XDP_DROP) {
+				xdp_return_frame_rx_napi(frame);
+				continue;
+			}
+
+			frames[j] = frame;
+			j++;
+		}
+		count = j;
+	}
+
+	return count;
+}
+
 static int netif_rx_internal(struct sk_buff *skb)
 {
 	int ret;
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 12/16] libbpf: Add egress XDP support

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Patch adds egress XDP support in libbpf.

New section name hint, xdp_egress, is added to set expected attach
type at program load. Programs can use xdp_egress as the prefix in
the SEC statement to load the program with the BPF_XDP_EGRESS
attach type set.

egress is added to bpf_xdp_set_link_opts to specify egress type for
use with bpf_set_link_xdp_fd_opts. Update library side to check
for flag and set nla_type to IFLA_XDP_EGRESS.

Add egress version of bpf_get_link_xdp* info and id apis with core
code refactored to handle both rx and tx paths.

Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Co-developed-by: David Ahern <dahern@digitalocean.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 tools/lib/bpf/libbpf.c   |  2 ++
 tools/lib/bpf/libbpf.h   |  9 +++++-
 tools/lib/bpf/libbpf.map |  2 ++
 tools/lib/bpf/netlink.c  | 63 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8f480e29a6b0..32fc970495d9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6366,6 +6366,8 @@ static const struct bpf_sec_def section_defs[] = {
 		.is_attach_btf = true,
 		.expected_attach_type = BPF_LSM_MAC,
 		.attach_fn = attach_lsm),
+	BPF_EAPROG_SEC("xdp_egress",		BPF_PROG_TYPE_XDP,
+						BPF_XDP_EGRESS),
 	BPF_PROG_SEC("xdp",			BPF_PROG_TYPE_XDP),
 	BPF_PROG_SEC("perf_event",		BPF_PROG_TYPE_PERF_EVENT),
 	BPF_PROG_SEC("lwt_in",			BPF_PROG_TYPE_LWT_IN),
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index f1dacecb1619..3feb1242f78e 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -453,14 +453,16 @@ struct xdp_link_info {
 	__u32 drv_prog_id;
 	__u32 hw_prog_id;
 	__u32 skb_prog_id;
+	__u32 egress_core_prog_id;
 	__u8 attach_mode;
 };
 
 struct bpf_xdp_set_link_opts {
 	size_t sz;
 	int old_fd;
+	__u8  egress;
 };
-#define bpf_xdp_set_link_opts__last_field old_fd
+#define bpf_xdp_set_link_opts__last_field egress
 
 LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
@@ -468,6 +470,11 @@ LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
 LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
 LIBBPF_API int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 				     size_t info_size, __u32 flags);
+LIBBPF_API int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id,
+					  __u32 flags);
+LIBBPF_API int bpf_get_link_xdp_egress_info(int ifindex,
+					    struct xdp_link_info *info,
+					    size_t info_size, __u32 flags);
 
 struct perf_buffer;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index bb8831605b25..51576c8a02fe 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -253,4 +253,6 @@ LIBBPF_0.0.8 {
 		bpf_program__set_attach_target;
 		bpf_program__set_lsm;
 		bpf_set_link_xdp_fd_opts;
+		bpf_get_link_xdp_egress_id;
+		bpf_get_link_xdp_egress_info;
 } LIBBPF_0.0.7;
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 0b709fd10bba..342acacf7cda 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -28,6 +28,7 @@ typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, libbpf_dump_nlmsg_t,
 struct xdp_id_md {
 	int ifindex;
 	__u32 flags;
+	__u16 nla_type;
 	struct xdp_link_info info;
 };
 
@@ -133,7 +134,7 @@ static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
 }
 
 static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
-					 __u32 flags)
+					 __u32 flags, __u16 nla_type)
 {
 	int sock, seq = 0, ret;
 	struct nlattr *nla, *nla_xdp;
@@ -160,7 +161,7 @@ static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
 	/* started nested attribute for XDP */
 	nla = (struct nlattr *)(((char *)&req)
 				+ NLMSG_ALIGN(req.nh.nlmsg_len));
-	nla->nla_type = NLA_F_NESTED | IFLA_XDP;
+	nla->nla_type = NLA_F_NESTED | nla_type;
 	nla->nla_len = NLA_HDRLEN;
 
 	/* add XDP fd */
@@ -203,6 +204,7 @@ static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
 int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
 			     const struct bpf_xdp_set_link_opts *opts)
 {
+	__u16 nla_type = IFLA_XDP;
 	int old_fd = -1;
 
 	if (!OPTS_VALID(opts, bpf_xdp_set_link_opts))
@@ -213,14 +215,22 @@ int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
 		flags |= XDP_FLAGS_REPLACE;
 	}
 
+	if (OPTS_HAS(opts, egress)) {
+		__u8 egress = OPTS_GET(opts, egress, 0);
+
+		if (egress)
+			nla_type = IFLA_XDP_EGRESS;
+	}
+
 	return __bpf_set_link_xdp_fd_replace(ifindex, fd,
 					     old_fd,
-					     flags);
+					     flags,
+					     nla_type);
 }
 
 int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
 {
-	return __bpf_set_link_xdp_fd_replace(ifindex, fd, 0, flags);
+	return __bpf_set_link_xdp_fd_replace(ifindex, fd, 0, flags, IFLA_XDP);
 }
 
 static int __dump_link_nlmsg(struct nlmsghdr *nlh,
@@ -243,15 +253,16 @@ static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb)
 	struct nlattr *xdp_tb[IFLA_XDP_MAX + 1];
 	struct xdp_id_md *xdp_id = cookie;
 	struct ifinfomsg *ifinfo = msg;
+	__u16 atype = xdp_id->nla_type;
 	int ret;
 
 	if (xdp_id->ifindex && xdp_id->ifindex != ifinfo->ifi_index)
 		return 0;
 
-	if (!tb[IFLA_XDP])
+	if (!tb[atype])
 		return 0;
 
-	ret = libbpf_nla_parse_nested(xdp_tb, IFLA_XDP_MAX, tb[IFLA_XDP], NULL);
+	ret = libbpf_nla_parse_nested(xdp_tb, IFLA_XDP_MAX, tb[atype], NULL);
 	if (ret)
 		return ret;
 
@@ -280,11 +291,16 @@ static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb)
 		xdp_id->info.hw_prog_id = libbpf_nla_getattr_u32(
 			xdp_tb[IFLA_XDP_HW_PROG_ID]);
 
+	if (xdp_tb[IFLA_XDP_EGRESS_CORE_PROG_ID])
+		xdp_id->info.egress_core_prog_id = libbpf_nla_getattr_u32(
+			xdp_tb[IFLA_XDP_EGRESS_CORE_PROG_ID]);
+
 	return 0;
 }
 
-int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
-			  size_t info_size, __u32 flags)
+static int __bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
+				   size_t info_size, __u32 flags,
+				   __u16 nla_type)
 {
 	struct xdp_id_md xdp_id = {};
 	int sock, ret;
@@ -306,6 +322,7 @@ int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 
 	xdp_id.ifindex = ifindex;
 	xdp_id.flags = flags;
+	xdp_id.nla_type = nla_type;
 
 	ret = libbpf_nl_get_link(sock, nl_pid, get_xdp_info, &xdp_id);
 	if (!ret) {
@@ -319,6 +336,20 @@ int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 	return ret;
 }
 
+int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
+			  size_t info_size, __u32 flags)
+{
+	return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
+				       IFLA_XDP);
+}
+
+int bpf_get_link_xdp_egress_info(int ifindex, struct xdp_link_info *info,
+				 size_t info_size, __u32 flags)
+{
+	return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
+				       IFLA_XDP_EGRESS);
+}
+
 static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
 {
 	if (info->attach_mode != XDP_ATTACHED_MULTI && !flags)
@@ -345,6 +376,22 @@ int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
 	return ret;
 }
 
+int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id, __u32 flags)
+{
+	struct xdp_link_info info;
+	int ret;
+
+	/* egress path does not support SKB, DRV or HW mode */
+	if (flags & XDP_FLAGS_MODES)
+		return -EINVAL;
+
+	ret = bpf_get_link_xdp_egress_info(ifindex, &info, sizeof(info), flags);
+	if (!ret)
+		*prog_id = get_xdp_id(&info, flags);
+
+	return ret;
+}
+
 int libbpf_nl_get_link(int sock, unsigned int nl_pid,
 		       libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
 {
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 13/16] bpftool: Add support for XDP egress

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add NET_ATTACH_TYPE_XDP_EGRESS and update attach_type_strings to
allow a user to specify 'xdp_egress' as the attach or detach point.

libbpf handles egress config via bpf_set_link_xdp_fd_opts, so
update do_attach_detach_xdp to use it. Specifically, the new API
requires old_fd to be set based on any currently loaded program,
so use bpf_get_link_xdp_id and bpf_get_link_xdp_egress_id to get
an fd to any existing program.

Update 'net show' command to dump egress programs.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 tools/bpf/bpftool/main.h           |  2 +-
 tools/bpf/bpftool/net.c            | 48 +++++++++++++++++++++++++++---
 tools/bpf/bpftool/netlink_dumper.c | 12 ++++++--
 tools/bpf/bpftool/prog.c           |  2 +-
 4 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 86f14ce26fd7..cbc0cc2257eb 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -230,7 +230,7 @@ void btf_dump_linfo_json(const struct btf *btf,
 struct nlattr;
 struct ifinfomsg;
 struct tcmsg;
-int do_xdp_dump(struct ifinfomsg *ifinfo, struct nlattr **tb);
+int do_xdp_dump(struct ifinfomsg *ifinfo, struct nlattr **tb, bool egress);
 int do_filter_dump(struct tcmsg *ifinfo, struct nlattr **tb, const char *kind,
 		   const char *devname, int ifindex);
 
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index c5e3895b7c8b..d09272a53734 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -32,6 +32,7 @@ struct bpf_netdev_t {
 	int	used_len;
 	int	array_len;
 	int	filter_idx;
+	bool	egress;
 };
 
 struct tc_kind_handle {
@@ -61,6 +62,7 @@ enum net_attach_type {
 	NET_ATTACH_TYPE_XDP_GENERIC,
 	NET_ATTACH_TYPE_XDP_DRIVER,
 	NET_ATTACH_TYPE_XDP_OFFLOAD,
+	NET_ATTACH_TYPE_XDP_EGRESS,
 };
 
 static const char * const attach_type_strings[] = {
@@ -68,6 +70,7 @@ static const char * const attach_type_strings[] = {
 	[NET_ATTACH_TYPE_XDP_GENERIC]	= "xdpgeneric",
 	[NET_ATTACH_TYPE_XDP_DRIVER]	= "xdpdrv",
 	[NET_ATTACH_TYPE_XDP_OFFLOAD]	= "xdpoffload",
+	[NET_ATTACH_TYPE_XDP_EGRESS]	= "xdp_egress",
 };
 
 const size_t net_attach_type_size = ARRAY_SIZE(attach_type_strings);
@@ -111,7 +114,7 @@ static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb)
 			 : "");
 	netinfo->used_len++;
 
-	return do_xdp_dump(ifinfo, tb);
+	return do_xdp_dump(ifinfo, tb, netinfo->egress);
 }
 
 static int dump_class_qdisc_nlmsg(void *cookie, void *msg, struct nlattr **tb)
@@ -276,10 +279,20 @@ static int net_parse_dev(int *argc, char ***argv)
 static int do_attach_detach_xdp(int progfd, enum net_attach_type attach_type,
 				int ifindex, bool overwrite)
 {
-	__u32 flags = 0;
+	struct bpf_xdp_set_link_opts opts;
+	__u32 flags = 0, id = 0;
+	int rc;
+
+	memset(&opts, 0, sizeof(opts));
+	opts.sz = sizeof(opts);
+	opts.old_fd = -1;
 
 	if (!overwrite)
 		flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+
+	if (attach_type == NET_ATTACH_TYPE_XDP_EGRESS)
+		opts.egress = 1;
+
 	if (attach_type == NET_ATTACH_TYPE_XDP_GENERIC)
 		flags |= XDP_FLAGS_SKB_MODE;
 	if (attach_type == NET_ATTACH_TYPE_XDP_DRIVER)
@@ -287,7 +300,25 @@ static int do_attach_detach_xdp(int progfd, enum net_attach_type attach_type,
 	if (attach_type == NET_ATTACH_TYPE_XDP_OFFLOAD)
 		flags |= XDP_FLAGS_HW_MODE;
 
-	return bpf_set_link_xdp_fd(ifindex, progfd, flags);
+	if (opts.egress)
+		rc = bpf_get_link_xdp_egress_id(ifindex, &id, flags);
+	else
+		rc = bpf_get_link_xdp_id(ifindex, &id, flags);
+
+	if (rc) {
+		p_err("Failed to get existing prog id for device");
+		return rc;
+	}
+
+	if (id)
+		opts.old_fd = bpf_prog_get_fd_by_id(id);
+
+	rc = bpf_set_link_xdp_fd_opts(ifindex, progfd, flags, &opts);
+
+	if (opts.old_fd != -1)
+		close(opts.old_fd);
+
+	return rc;
 }
 
 static int do_attach(int argc, char **argv)
@@ -411,6 +442,7 @@ static int do_show(int argc, char **argv)
 	dev_array.used_len = 0;
 	dev_array.array_len = 0;
 	dev_array.filter_idx = filter_idx;
+	dev_array.egress = 0;
 
 	if (json_output)
 		jsonw_start_array(json_wtr);
@@ -419,6 +451,14 @@ static int do_show(int argc, char **argv)
 	ret = libbpf_nl_get_link(sock, nl_pid, dump_link_nlmsg, &dev_array);
 	NET_END_ARRAY("\n");
 
+	if (!ret) {
+		dev_array.egress = true;
+		NET_START_ARRAY("xdp_egress", "%s:\n");
+		ret = libbpf_nl_get_link(sock, nl_pid, dump_link_nlmsg,
+					 &dev_array);
+		NET_END_ARRAY("\n");
+	}
+
 	if (!ret) {
 		NET_START_ARRAY("tc", "%s:\n");
 		for (i = 0; i < dev_array.used_len; i++) {
@@ -464,7 +504,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_PROGRAM "\n"
-		"       ATTACH_TYPE := { xdp | xdpgeneric | xdpdrv | xdpoffload }\n"
+		"       ATTACH_TYPE := { xdp | xdpgeneric | xdpdrv | xdpoffload | xdp_egress}\n"
 		"\n"
 		"Note: Only xdp and tc attachments are supported now.\n"
 		"      For progs attached to cgroups, use \"bpftool cgroup\"\n"
diff --git a/tools/bpf/bpftool/netlink_dumper.c b/tools/bpf/bpftool/netlink_dumper.c
index 5f65140b003b..e4a2b6f8e50b 100644
--- a/tools/bpf/bpftool/netlink_dumper.c
+++ b/tools/bpf/bpftool/netlink_dumper.c
@@ -55,6 +55,7 @@ static int do_xdp_dump_one(struct nlattr *attr, unsigned int ifindex,
 		xdp_dump_prog_id(tb, IFLA_XDP_SKB_PROG_ID, "generic", true);
 		xdp_dump_prog_id(tb, IFLA_XDP_DRV_PROG_ID, "driver", true);
 		xdp_dump_prog_id(tb, IFLA_XDP_HW_PROG_ID, "offload", true);
+		xdp_dump_prog_id(tb, IFLA_XDP_EGRESS_CORE_PROG_ID, "core", true);
 		if (json_output)
 			jsonw_end_array(json_wtr);
 	} else if (mode == XDP_ATTACHED_DRV) {
@@ -63,18 +64,23 @@ static int do_xdp_dump_one(struct nlattr *attr, unsigned int ifindex,
 		xdp_dump_prog_id(tb, IFLA_XDP_PROG_ID, "generic", false);
 	} else if (mode == XDP_ATTACHED_HW) {
 		xdp_dump_prog_id(tb, IFLA_XDP_PROG_ID, "offload", false);
+	} else if (mode == XDP_ATTACHED_EGRESS_CORE) {
+		xdp_dump_prog_id(tb, IFLA_XDP_EGRESS_CORE_PROG_ID, "core",
+				 false);
 	}
 
 	NET_END_OBJECT_FINAL;
 	return 0;
 }
 
-int do_xdp_dump(struct ifinfomsg *ifinfo, struct nlattr **tb)
+int do_xdp_dump(struct ifinfomsg *ifinfo, struct nlattr **tb, bool egress)
 {
-	if (!tb[IFLA_XDP])
+	__u16 atype = egress ? IFLA_XDP_EGRESS : IFLA_XDP;
+
+	if (!tb[atype])
 		return 0;
 
-	return do_xdp_dump_one(tb[IFLA_XDP], ifinfo->ifi_index,
+	return do_xdp_dump_one(tb[atype], ifinfo->ifi_index,
 			       libbpf_nla_getattr_str(tb[IFLA_IFNAME]));
 }
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index f6a5974a7b0a..64695ccdcf9d 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2014,7 +2014,7 @@ static int do_help(int argc, char **argv)
 		"                 cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
 		"                 cgroup/sendmsg4 | cgroup/sendmsg6 | cgroup/recvmsg4 |\n"
 		"                 cgroup/recvmsg6 | cgroup/getsockopt | cgroup/setsockopt |\n"
-		"                 struct_ops | fentry | fexit | freplace }\n"
+		"                 struct_ops | fentry | fexit | freplace | xdp_egress }\n"
 		"       ATTACH_TYPE := { msg_verdict | stream_verdict | stream_parser |\n"
 		"                        flow_dissector }\n"
 		"       METRIC := { cycles | instructions | l1d_loads | llc_misses }\n"
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 14/16] selftest: Add test for xdp_egress

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add selftest for xdp_egress. Add xdp_drop program to veth connecting
a namespace to drop packets and break connectivity.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 tools/testing/selftests/bpf/Makefile          |   1 +
 tools/testing/selftests/bpf/progs/xdp_drop.c  |  25 +++
 .../testing/selftests/bpf/test_xdp_egress.sh  | 159 ++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/xdp_drop.c
 create mode 100755 tools/testing/selftests/bpf/test_xdp_egress.sh

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 7729892e0b04..5dae18ebac13 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -50,6 +50,7 @@ TEST_PROGS := test_kmod.sh \
 	test_xdp_redirect.sh \
 	test_xdp_meta.sh \
 	test_xdp_veth.sh \
+	test_xdp_egress.sh \
 	test_offload.py \
 	test_sock_addr.sh \
 	test_tunnel.sh \
diff --git a/tools/testing/selftests/bpf/progs/xdp_drop.c b/tools/testing/selftests/bpf/progs/xdp_drop.c
new file mode 100644
index 000000000000..cffabc53a5e1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_drop.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("drop")
+int xdp_drop(struct xdp_md *ctx)
+{
+	void *data_end = (void *)(long)ctx->data_end;
+	void *data = (void *)(long)ctx->data;
+	struct ethhdr *eth = data;
+	void *nh;
+
+	nh = data + sizeof(*eth);
+	if (nh > data_end)
+		return XDP_DROP;
+
+	if (eth->h_proto == 0x0008)
+		return XDP_DROP;
+
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_xdp_egress.sh b/tools/testing/selftests/bpf/test_xdp_egress.sh
new file mode 100755
index 000000000000..64cc9a8486a6
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_egress.sh
@@ -0,0 +1,159 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# XDP egress tests.
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+TESTNAME=xdp_egress
+BPF_FS=$(awk '$3 == "bpf" {print $2; exit}' /proc/mounts)
+
+ret=0
+
+################################################################################
+#
+log_test()
+{
+	local rc=$1
+	local expected=$2
+	local msg="$3"
+
+	if [ ${rc} -eq ${expected} ]; then
+		printf "TEST: %-60s  [ OK ]\n" "${msg}"
+	else
+		ret=1
+		printf "TEST: %-60s  [FAIL]\n" "${msg}"
+	fi
+}
+
+################################################################################
+# create namespaces and connect them
+
+create_ns()
+{
+	local ns=$1
+	local addr=$2
+	local addr6=$3
+
+	ip netns add ${ns}
+
+	ip -netns ${ns} link set lo up
+	ip -netns ${ns} addr add dev lo ${addr}
+	ip -netns ${ns} -6 addr add dev lo ${addr6}
+
+	ip -netns ${ns} ro add unreachable default metric 8192
+	ip -netns ${ns} -6 ro add unreachable default metric 8192
+
+	ip netns exec ${ns} sysctl -qw net.ipv4.ip_forward=1
+	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.keep_addr_on_down=1
+	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.forwarding=1
+	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.forwarding=1
+	ip netns exec ${ns} sysctl -qw net.ipv6.conf.all.accept_dad=0
+}
+
+connect_ns()
+{
+	local ns1=$1
+	local ns1_dev=$2
+	local ns1_addr=$3
+	local ns1_addr6=$4
+	local ns2=$5
+	local ns2_dev=$6
+	local ns2_addr=$7
+	local ns2_addr6=$8
+	local ns1arg
+	local ns2arg
+
+	if [ -n "${ns1}" ]; then
+		ns1arg="-netns ${ns1}"
+	fi
+	if [ -n "${ns2}" ]; then
+		ns2arg="-netns ${ns2}"
+	fi
+
+	ip ${ns1arg} li add ${ns1_dev} type veth peer name tmp
+	ip ${ns1arg} li set ${ns1_dev} up
+	ip ${ns1arg} li set tmp netns ${ns2} name ${ns2_dev}
+	ip ${ns2arg} li set ${ns2_dev} up
+
+	ip ${ns1arg} addr add dev ${ns1_dev} ${ns1_addr}
+	ip ${ns2arg} addr add dev ${ns2_dev} ${ns2_addr}
+
+	ip ${ns1arg} addr add dev ${ns1_dev} ${ns1_addr6} nodad
+	ip ${ns2arg} addr add dev ${ns2_dev} ${ns2_addr6} nodad
+}
+
+################################################################################
+#
+
+setup()
+{
+	create_ns host 172.16.101.1/32 2001:db8:101::1/128
+	connect_ns "" veth-host 172.16.1.1/24 2001:db8:1::1/64 host eth0 172.16.1.2/24 2001:db8:1::2/64
+	ip ro add 172.16.101.1 via 172.16.1.2
+	ip -6 ro add 2001:db8:101::1 via 2001:db8:1::2
+	ping -c1 -w1 172.16.101.1 >/dev/null 2>&1
+	ping -c1 -w1 2001:db8:101::1 >/dev/null 2>&1
+}
+
+cleanup()
+{
+	ip li del veth-host 2>/dev/null
+	ip netns del host 2>/dev/null
+	rm -f $BPF_FS/test_$TESTNAME
+}
+
+################################################################################
+# main
+
+if [ $(id -u) -ne 0 ]; then
+	echo "selftests: $TESTNAME [SKIP] Need root privileges"
+	exit $ksft_skip
+fi
+
+if ! ip link set dev lo xdp off > /dev/null 2>&1; then
+	echo "selftests: $TESTNAME [SKIP] Could not run test without the ip xdp support"
+	exit $ksft_skip
+fi
+
+if [ -z "$BPF_FS" ]; then
+	echo "selftests: $TESTNAME [SKIP] Could not run test without bpffs mounted"
+	exit $ksft_skip
+fi
+
+if ! bpftool version > /dev/null 2>&1; then
+	echo "selftests: $TESTNAME [SKIP] Could not run test without bpftool"
+	exit $ksft_skip
+fi
+
+cleanup
+trap cleanup EXIT
+
+set -e
+setup
+set +e
+
+bpftool prog load xdp_drop.o $BPF_FS/test_$TESTNAME type xdp_egress
+ID=$(bpftool prog show name xdp_drop | awk '$4 == "xdp_drop" {print $1}')
+
+# attach egress program
+bpftool net attach xdp_egress id ${ID/:/} dev veth-host
+ping -c1 -w1 172.16.101.1 >/dev/null 2>&1
+log_test $? 1 "IPv4 connectivity disabled by xdp_egress"
+ping -c1 -w1 2001:db8:101::1 >/dev/null 2>&1
+log_test $? 0 "IPv6 connectivity not disabled by egress drop program"
+
+# detach program should restore connectivity
+bpftool net detach xdp_egress dev veth-host
+ping -c1 -w1 172.16.101.1 >/dev/null 2>&1
+log_test $? 0 "IPv4 connectivity restored"
+
+# cleanup on delete
+ip netns exec host bpftool net attach xdp_egress id ${ID/:/} dev eth0
+bpftool net attach xdp_egress id ${ID/:/} dev veth-host
+ip li del veth-host
+rm -f $BPF_FS/test_$TESTNAME
+bpftool prog show name xdp_drop
+
+exit $ret
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 15/16] selftest: Add xdp_egress attach tests

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

Add xdp_egress attach tests:
1. verify egress programs cannot access ingress entries in xdp context
2. verify ability to load, attach, and detach xdp egress to a device.

Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 .../bpf/prog_tests/xdp_egress_attach.c        | 62 +++++++++++++++++++
 .../selftests/bpf/progs/test_xdp_egress.c     | 12 ++++
 .../bpf/progs/test_xdp_egress_fail.c          | 16 +++++
 3 files changed, 90 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_egress_attach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_egress.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_egress_fail.c

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_egress_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_egress_attach.c
new file mode 100644
index 000000000000..a8727f82a29d
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_egress_attach.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+#define IFINDEX_LO 1
+
+void test_xdp_egress_attach(void)
+{
+	struct bpf_prog_load_attr attr = {
+		.prog_type = BPF_PROG_TYPE_XDP,
+		.expected_attach_type = BPF_XDP_EGRESS,
+	};
+	struct bpf_xdp_set_link_opts opts;
+	struct bpf_prog_info info = {};
+	__u32 id, len = sizeof(info);
+	struct bpf_object *obj;
+	__u32 duration = 0;
+	int err, fd = -1;
+
+	memset(&opts, 0, sizeof(opts));
+	opts.sz = sizeof(opts);
+
+	/* should fail - accesses rx queue info */
+	attr.file = "./test_xdp_egress_fail.o",
+	err = bpf_prog_load_xattr(&attr, &obj, &fd);
+	if (CHECK(err == 0 && fd >= 0, "xdp_egress with rx failed to load",
+		 "load of xdp_egress with rx succeeded instead of failed"))
+		return;
+
+	attr.file = "./test_xdp_egress.o",
+	err = bpf_prog_load_xattr(&attr, &obj, &fd);
+	if (CHECK_FAIL(err))
+		return;
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &len);
+	if (CHECK_FAIL(err))
+		goto out_close;
+
+	opts.old_fd = -1;
+	opts.egress = 1;
+	err = bpf_set_link_xdp_fd_opts(IFINDEX_LO, fd, 0, &opts);
+	if (CHECK(err, "xdp attach", "xdp attach failed"))
+		goto out_close;
+
+	err = bpf_get_link_xdp_egress_id(IFINDEX_LO, &id, 0);
+	if (CHECK(err || id != info.id, "id_check",
+		  "loaded prog id %u != id %u, err %d", info.id, id, err))
+		goto out;
+
+out:
+	opts.old_fd = bpf_prog_get_fd_by_id(id);
+	err = bpf_set_link_xdp_fd_opts(IFINDEX_LO, -1, 0, &opts);
+	if (CHECK(err, "xdp detach", "xdp detach failed"))
+		goto out_close;
+
+	err = bpf_get_link_xdp_egress_id(IFINDEX_LO, &id, 0);
+	if (CHECK(err || id, "id_check",
+		  "failed to detach program %u", id))
+		goto out;
+
+out_close:
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_egress.c b/tools/testing/selftests/bpf/progs/test_xdp_egress.c
new file mode 100644
index 000000000000..0477e8537b7f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_egress.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("xdp_egress")
+int xdp_egress_good(struct xdp_md *ctx)
+{
+	__u32 idx = ctx->egress_ifindex;
+
+	return idx == 1 ? XDP_DROP : XDP_PASS;
+}
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_egress_fail.c b/tools/testing/selftests/bpf/progs/test_xdp_egress_fail.c
new file mode 100644
index 000000000000..76b47b1d3bc3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_xdp_egress_fail.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("xdp_egress")
+int xdp_egress_fail(struct xdp_md *ctx)
+{
+	__u32 rxq = ctx->rx_queue_index;
+	__u32 idx = ctx->ingress_ifindex;
+
+	if (idx == 1)
+		return XDP_DROP;
+
+	return rxq ? XDP_DROP : XDP_PASS;
+}
-- 
2.21.1 (Apple Git-122.3)

[PATCH bpf-next 16/16] samples/bpf: add XDP egress support to xdp1

[David Ahern]

From: David Ahern <dahern@digitalocean.com>

xdp1 and xdp2 now accept -E flag to set XDP program in the egress
path.

Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
Signed-off-by: David Ahern <dahern@digitalocean.com>
---
 samples/bpf/xdp1_user.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index c447ad9e3a1d..9f79bd537763 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -20,22 +20,37 @@
 
 static int ifindex;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static struct bpf_xdp_set_link_opts opts;
 static __u32 prog_id;
 
 static void int_exit(int sig)
 {
 	__u32 curr_prog_id = 0;
+	int rc;
 
-	if (bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags)) {
-		printf("bpf_get_link_xdp_id failed\n");
+	if (opts.egress)
+		rc = bpf_get_link_xdp_egress_id(ifindex, &curr_prog_id, xdp_flags);
+	else
+		rc = bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+
+	if (rc) {
+		printf("Failed to get existing prog id for device");
 		exit(1);
 	}
+
+	if (curr_prog_id)
+		opts.old_fd = bpf_prog_get_fd_by_id(curr_prog_id);
+
 	if (prog_id == curr_prog_id)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		bpf_set_link_xdp_fd_opts(ifindex, -1, xdp_flags, &opts);
 	else if (!curr_prog_id)
 		printf("couldn't find a prog id on a given interface\n");
 	else
 		printf("program on interface changed, not removing\n");
+
+	if (opts.old_fd >= 0)
+		close(opts.old_fd);
+
 	exit(0);
 }
 
@@ -73,7 +88,8 @@ static void usage(const char *prog)
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
 		"    -N    enforce native mode\n"
-		"    -F    force loading prog\n",
+		"    -F    force loading prog\n"
+		"    -E	   egress path program\n",
 		prog);
 }
 
@@ -83,15 +99,20 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_xdp_set_link_opts opts;
 	struct bpf_prog_info info = {};
 	__u32 info_len = sizeof(info);
-	const char *optstr = "FSN";
+	const char *optstr = "FSNE";
 	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
 	int err;
 
+	memset(&opts, 0, sizeof(opts));
+	opts.sz = sizeof(opts);
+	opts.old_fd = -1;
+
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
 		case 'S':
@@ -103,13 +124,17 @@ int main(int argc, char **argv)
 		case 'F':
 			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
 			break;
+		case 'E':
+			opts.egress = true;
+			prog_load_attr.expected_attach_type = BPF_XDP_EGRESS;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
 		}
 	}
 
-	if (!(xdp_flags & XDP_FLAGS_SKB_MODE))
+	if (!(xdp_flags & XDP_FLAGS_SKB_MODE) && !opts.egress)
 		xdp_flags |= XDP_FLAGS_DRV_MODE;
 
 	if (optind == argc) {
@@ -149,7 +174,7 @@ int main(int argc, char **argv)
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
-	if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd_opts(ifindex, prog_fd, xdp_flags, &opts) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
-- 
2.21.1 (Apple Git-122.3)

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@kernel.org> writes:

> From: David Ahern <dahern@digitalocean.com>
>
> Add new bpf_attach_type, BPF_XDP_EGRESS, for BPF programs attached
> at the XDP layer, but the egress path.
>
> Since egress path will not have ingress_ifindex and rx_queue_index
> set, update xdp_is_valid_access to block access to these entries in
> the xdp context when a program is attached to egress path.
>
> Update dev_change_xdp_fd to verify expected_attach_type for a program
> is BPF_XDP_EGRESS if egress argument is set.
>
> The next patch adds support for the egress ifindex.
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> Signed-off-by: David Ahern <dahern@digitalocean.com>
> ---
>  include/uapi/linux/bpf.h       |  1 +
>  net/core/dev.c                 | 11 +++++++++++
>  net/core/filter.c              |  8 ++++++++
>  tools/include/uapi/linux/bpf.h |  1 +
>  4 files changed, 21 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 2e29a671d67e..a9d384998e8b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -215,6 +215,7 @@ enum bpf_attach_type {
>  	BPF_TRACE_FEXIT,
>  	BPF_MODIFY_RETURN,
>  	BPF_LSM_MAC,
> +	BPF_XDP_EGRESS,
>  	__MAX_BPF_ATTACH_TYPE
>  };
>  
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 97180458e7cb..e8a62bdb395b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -8732,6 +8732,17 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>  		if (IS_ERR(prog))
>  			return PTR_ERR(prog);
>  
> +		if (egress && prog->expected_attach_type != BPF_XDP_EGRESS) {
> +			NL_SET_ERR_MSG(extack, "XDP program in Tx path must use BPF_XDP_EGRESS attach type");
> +			bpf_prog_put(prog);
> +			return -EINVAL;
> +		}
> +		if (!egress && prog->expected_attach_type == BPF_XDP_EGRESS) {
> +			NL_SET_ERR_MSG(extack, "XDP program in Rx path can not use BPF_XDP_EGRESS attach type");
> +			bpf_prog_put(prog);
> +			return -EINVAL;
> +		}
> +
>  		if (!offload && bpf_prog_is_dev_bound(prog->aux)) {
>  			NL_SET_ERR_MSG(extack, "using device-bound program without HW_MODE flag is not supported");
>  			bpf_prog_put(prog);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 7d6ceaa54d21..bcb56448f336 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6935,6 +6935,14 @@ static bool xdp_is_valid_access(int off, int size,
>  				const struct bpf_prog *prog,
>  				struct bpf_insn_access_aux *info)
>  {
> +	if (prog->expected_attach_type == BPF_XDP_EGRESS) {
> +		switch (off) {
> +		case offsetof(struct xdp_md, ingress_ifindex):
> +		case offsetof(struct xdp_md, rx_queue_index):
> +			return false;
> +		}
> +	}

As I pointed out on the RFC patch, I'm concerned whether this will work
right with freplace programs attaching to XDP programs. It may just be
that I'm missing something, but in that case please explain why it
works? :)

-Toke

Re: [PATCH bpf-next 06/16] net: Add IFLA_XDP_EGRESS for XDP programs in the egress path

[Toke Høiland-Jørgensen]

David Ahern <dsahern@kernel.org> writes:

> From: David Ahern <dahern@digitalocean.com>
>
> Running programs in the egress path, on skbs or xdp_frames, does not
> require driver specific resources like Rx path. Accordingly, the
> programs can be run in core code, so add xdp_egress_prog to net_device
> to hold a reference to an attached program.
>
> For UAPI, add IFLA_XDP_EGRESS to if_link.h to specify egress programs,
> add a new attach flag, XDP_ATTACHED_EGRESS_CORE, to denote the
> attach point is at the core level (as opposed to driver or hardware)
> and add IFLA_XDP_EGRESS_CORE_PROG_ID for reporting the program id.
>
> Add egress argument to do_setlink_xdp to denote processing of
> IFLA_XDP_EGRESS versus IFLA_XDP, and add a check that none of the
> existing modes (SKB, DRV or HW) are set since those modes are not
> valid. The expectation is that XDP_FLAGS_HW_MODE will be used later
> (e.g., offloading guest programs).
>
> Add rtnl_xdp_egress_fill and helpers as the egress counterpart to the
> existing rtnl_xdp_fill.
>
> Signed-off-by: David Ahern <dahern@digitalocean.com>
> ---
>  include/linux/netdevice.h          |  1 +
>  include/uapi/linux/if_link.h       |  3 +
>  net/core/rtnetlink.c               | 96 ++++++++++++++++++++++++++++--
>  tools/include/uapi/linux/if_link.h |  3 +
>  4 files changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d0bb9e09660a..3133247681fd 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1995,6 +1995,7 @@ struct net_device {
>  	unsigned int		real_num_rx_queues;
>  
>  	struct bpf_prog __rcu	*xdp_prog;
> +	struct bpf_prog __rcu	*xdp_egress_prog;
>  	unsigned long		gro_flush_timeout;
>  	rx_handler_func_t __rcu	*rx_handler;
>  	void __rcu		*rx_handler_data;
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 127c704eeba9..b3c6cb2f0f0a 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -170,6 +170,7 @@ enum {
>  	IFLA_PROP_LIST,
>  	IFLA_ALT_IFNAME, /* Alternative ifname */
>  	IFLA_PERM_ADDRESS,
> +	IFLA_XDP_EGRESS, /* nested attribute with 1 or more IFLA_XDP_ attrs */
>  	__IFLA_MAX
>  };
>  
> @@ -988,6 +989,7 @@ enum {
>  	XDP_ATTACHED_SKB,
>  	XDP_ATTACHED_HW,
>  	XDP_ATTACHED_MULTI,
> +	XDP_ATTACHED_EGRESS_CORE,
>  };
>  
>  enum {
> @@ -1000,6 +1002,7 @@ enum {
>  	IFLA_XDP_SKB_PROG_ID,
>  	IFLA_XDP_HW_PROG_ID,
>  	IFLA_XDP_EXPECTED_FD,
> +	IFLA_XDP_EGRESS_CORE_PROG_ID,
>  	__IFLA_XDP_MAX,
>  };
>  
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index dc44af16226a..e9bc5cee06c8 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1030,7 +1030,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */
>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
>  	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
> -	       + rtnl_xdp_size() /* IFLA_XDP */
> +	       + rtnl_xdp_size() * 2 /* IFLA_XDP and IFLA_XDP_EGRESS */
>  	       + nla_total_size(4)  /* IFLA_EVENT */
>  	       + nla_total_size(4)  /* IFLA_NEW_NETNSID */
>  	       + nla_total_size(4)  /* IFLA_NEW_IFINDEX */
> @@ -1395,6 +1395,42 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
>  	return 0;
>  }
>  
> +static u32 rtnl_xdp_egress_prog(struct net_device *dev)
> +{
> +	const struct bpf_prog *prog;
> +
> +	ASSERT_RTNL();
> +
> +	prog = rtnl_dereference(dev->xdp_egress_prog);
> +	if (!prog)
> +		return 0;
> +	return prog->aux->id;
> +}
> +
> +static int rtnl_xdp_egress_report(struct sk_buff *skb, struct net_device *dev,
> +				  u32 *prog_id, u8 *mode, u8 tgt_mode, u32 attr,
> +				  u32 (*get_prog_id)(struct net_device *dev))
> +{
> +	u32 curr_id;
> +	int err;
> +
> +	curr_id = get_prog_id(dev);
> +	if (!curr_id)
> +		return 0;
> +
> +	*prog_id = curr_id;
> +	err = nla_put_u32(skb, attr, curr_id);
> +	if (err)
> +		return err;
> +
> +	if (*mode != XDP_ATTACHED_NONE)
> +		*mode = XDP_ATTACHED_MULTI;
> +	else
> +		*mode = tgt_mode;
> +
> +	return 0;
> +}
> +
>  static u32 rtnl_xdp_prog_skb(struct net_device *dev)
>  {
>  	const struct bpf_prog *generic_xdp_prog;
> @@ -1486,6 +1522,42 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
>  	return err;
>  }
>  
> +static int rtnl_xdp_egress_fill(struct sk_buff *skb, struct net_device *dev)
> +{
> +	u8 mode = XDP_ATTACHED_NONE;
> +	struct nlattr *xdp;
> +	u32 prog_id = 0;
> +	int err;
> +
> +	xdp = nla_nest_start_noflag(skb, IFLA_XDP_EGRESS);
> +	if (!xdp)
> +		return -EMSGSIZE;
> +
> +	err = rtnl_xdp_egress_report(skb, dev, &prog_id, &mode,
> +				     XDP_ATTACHED_EGRESS_CORE,
> +				     IFLA_XDP_EGRESS_CORE_PROG_ID,
> +				     rtnl_xdp_egress_prog);
> +	if (err)
> +		goto err_cancel;
> +
> +	err = nla_put_u8(skb, IFLA_XDP_ATTACHED, mode);
> +	if (err)
> +		goto err_cancel;
> +
> +	if (prog_id && mode != XDP_ATTACHED_MULTI) {
> +		err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
> +		if (err)
> +			goto err_cancel;
> +	}
> +
> +	nla_nest_end(skb, xdp);
> +	return 0;
> +
> +err_cancel:
> +	nla_nest_cancel(skb, xdp);
> +	return err;
> +}
> +
>  static u32 rtnl_get_event(unsigned long event)
>  {
>  	u32 rtnl_event_type = IFLA_EVENT_NONE;
> @@ -1743,6 +1815,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
>  	if (rtnl_xdp_fill(skb, dev))
>  		goto nla_put_failure;
>  
> +	if (rtnl_xdp_egress_fill(skb, dev))
> +		goto nla_put_failure;
> +
>  	if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
>  		if (rtnl_link_fill(skb, dev) < 0)
>  			goto nla_put_failure;
> @@ -1827,6 +1902,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>  	[IFLA_ALT_IFNAME]	= { .type = NLA_STRING,
>  				    .len = ALTIFNAMSIZ - 1 },
>  	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
> +	[IFLA_XDP_EGRESS]	= { .type = NLA_NESTED },
>  };
>  
>  static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
> @@ -2482,7 +2558,8 @@ static int do_set_master(struct net_device *dev, int ifindex,
>  #define DO_SETLINK_NOTIFY	0x03
>  
>  static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
> -			  int *status, struct netlink_ext_ack *extack)
> +			  int *status, bool egress,
> +			  struct netlink_ext_ack *extack)
>  {
>  	struct nlattr *xdp[IFLA_XDP_MAX + 1];
>  	u32 xdp_flags = 0;
> @@ -2498,6 +2575,10 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>  
>  	if (xdp[IFLA_XDP_FLAGS]) {
>  		xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
> +		if (egress && xdp_flags & XDP_FLAGS_MODES) {
> +			NL_SET_ERR_MSG(extack, "XDP_FLAGS_MODES not valid for egress");
> +			goto out_einval;
> +		}
>  		if (xdp_flags & ~XDP_FLAGS_MASK)
>  			goto out_einval;
>  		if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1)
> @@ -2515,7 +2596,7 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>  
>  		err = dev_change_xdp_fd(dev, extack,
>  					nla_get_s32(xdp[IFLA_XDP_FD]),
> -					expected_fd, xdp_flags, false);
> +					expected_fd, xdp_flags, egress);
>  		if (err)
>  			return err;
>  
> @@ -2821,7 +2902,14 @@ static int do_setlink(const struct sk_buff *skb,
>  	}
>  
>  	if (tb[IFLA_XDP]) {
> -		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, extack);
> +		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, false, extack);
> +		if (err)
> +			goto errout;
> +	}
> +
> +	if (tb[IFLA_XDP_EGRESS]) {
> +		err = do_setlink_xdp(dev, tb[IFLA_XDP_EGRESS], &status, true,
> +				     extack);
>  		if (err)
>  			goto errout;
>  	}

This means that IFLA_XDP and IFLA_XDP_EGRESS can be present in the same
netlink message, right? But then installation of the RX program could
succeed, but userspace would still get an error if the egress program
installation fails? That is probably not good?

Since I don't think we can atomically make sure both operations fail or
succeed, maybe it's better to disallow both entries being present in the
same netlink message?

-Toke

Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

[Toke Høiland-Jørgensen]

David Ahern <dsahern@kernel.org> writes:

> From: David Ahern <dahern@digitalocean.com>
>
> Patch adds egress XDP support in libbpf.
>
> New section name hint, xdp_egress, is added to set expected attach
> type at program load. Programs can use xdp_egress as the prefix in
> the SEC statement to load the program with the BPF_XDP_EGRESS
> attach type set.
>
> egress is added to bpf_xdp_set_link_opts to specify egress type for
> use with bpf_set_link_xdp_fd_opts. Update library side to check
> for flag and set nla_type to IFLA_XDP_EGRESS.
>
> Add egress version of bpf_get_link_xdp* info and id apis with core
> code refactored to handle both rx and tx paths.
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> Co-developed-by: David Ahern <dahern@digitalocean.com>
> Signed-off-by: David Ahern <dahern@digitalocean.com>
> ---
>  tools/lib/bpf/libbpf.c   |  2 ++
>  tools/lib/bpf/libbpf.h   |  9 +++++-
>  tools/lib/bpf/libbpf.map |  2 ++
>  tools/lib/bpf/netlink.c  | 63 +++++++++++++++++++++++++++++++++++-----
>  4 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8f480e29a6b0..32fc970495d9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -6366,6 +6366,8 @@ static const struct bpf_sec_def section_defs[] = {
>  		.is_attach_btf = true,
>  		.expected_attach_type = BPF_LSM_MAC,
>  		.attach_fn = attach_lsm),
> +	BPF_EAPROG_SEC("xdp_egress",		BPF_PROG_TYPE_XDP,
> +						BPF_XDP_EGRESS),
>  	BPF_PROG_SEC("xdp",			BPF_PROG_TYPE_XDP),
>  	BPF_PROG_SEC("perf_event",		BPF_PROG_TYPE_PERF_EVENT),
>  	BPF_PROG_SEC("lwt_in",			BPF_PROG_TYPE_LWT_IN),
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index f1dacecb1619..3feb1242f78e 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -453,14 +453,16 @@ struct xdp_link_info {
>  	__u32 drv_prog_id;
>  	__u32 hw_prog_id;
>  	__u32 skb_prog_id;
> +	__u32 egress_core_prog_id;
>  	__u8 attach_mode;
>  };
>  
>  struct bpf_xdp_set_link_opts {
>  	size_t sz;
>  	int old_fd;
> +	__u8  egress;
>  };
> -#define bpf_xdp_set_link_opts__last_field old_fd
> +#define bpf_xdp_set_link_opts__last_field egress
>  
>  LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
>  LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
> @@ -468,6 +470,11 @@ LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
>  LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
>  LIBBPF_API int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
>  				     size_t info_size, __u32 flags);
> +LIBBPF_API int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id,
> +					  __u32 flags);
> +LIBBPF_API int bpf_get_link_xdp_egress_info(int ifindex,
> +					    struct xdp_link_info *info,
> +					    size_t info_size, __u32 flags);

Isn't the kernel returning both program types in the same message when
dumping an interface? So do we really need a separate getter instead of
just populating xdp_link_info with the egress ID in the existing getter?

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/21/20 4:14 AM, Toke Høiland-Jørgensen wrote:
> As I pointed out on the RFC patch, I'm concerned whether this will work
> right with freplace programs attaching to XDP programs. It may just be
> that I'm missing something, but in that case please explain why it
> works? :)

expected_attach_type is not unique to XDP. freplace is not unique to
XDP. IF there is a problem, it is not unique to XDP, and any
enhancements needed to freplace functionality will not be unique to XDP.

Re: [PATCH bpf-next 06/16] net: Add IFLA_XDP_EGRESS for XDP programs in the egress path

[David Ahern]

On 4/21/20 4:17 AM, Toke Høiland-Jørgensen wrote:
> David Ahern <dsahern@kernel.org> writes:
> 
>> From: David Ahern <dahern@digitalocean.com>
>>
>> Running programs in the egress path, on skbs or xdp_frames, does not
>> require driver specific resources like Rx path. Accordingly, the
>> programs can be run in core code, so add xdp_egress_prog to net_device
>> to hold a reference to an attached program.
>>
>> For UAPI, add IFLA_XDP_EGRESS to if_link.h to specify egress programs,
>> add a new attach flag, XDP_ATTACHED_EGRESS_CORE, to denote the
>> attach point is at the core level (as opposed to driver or hardware)
>> and add IFLA_XDP_EGRESS_CORE_PROG_ID for reporting the program id.
>>
>> Add egress argument to do_setlink_xdp to denote processing of
>> IFLA_XDP_EGRESS versus IFLA_XDP, and add a check that none of the
>> existing modes (SKB, DRV or HW) are set since those modes are not
>> valid. The expectation is that XDP_FLAGS_HW_MODE will be used later
>> (e.g., offloading guest programs).
>>
>> Add rtnl_xdp_egress_fill and helpers as the egress counterpart to the
>> existing rtnl_xdp_fill.
>>
>> Signed-off-by: David Ahern <dahern@digitalocean.com>
>> ---
>>  include/linux/netdevice.h          |  1 +
>>  include/uapi/linux/if_link.h       |  3 +
>>  net/core/rtnetlink.c               | 96 ++++++++++++++++++++++++++++--
>>  tools/include/uapi/linux/if_link.h |  3 +
>>  4 files changed, 99 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index d0bb9e09660a..3133247681fd 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1995,6 +1995,7 @@ struct net_device {
>>  	unsigned int		real_num_rx_queues;
>>  
>>  	struct bpf_prog __rcu	*xdp_prog;
>> +	struct bpf_prog __rcu	*xdp_egress_prog;
>>  	unsigned long		gro_flush_timeout;
>>  	rx_handler_func_t __rcu	*rx_handler;
>>  	void __rcu		*rx_handler_data;
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index 127c704eeba9..b3c6cb2f0f0a 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -170,6 +170,7 @@ enum {
>>  	IFLA_PROP_LIST,
>>  	IFLA_ALT_IFNAME, /* Alternative ifname */
>>  	IFLA_PERM_ADDRESS,
>> +	IFLA_XDP_EGRESS, /* nested attribute with 1 or more IFLA_XDP_ attrs */
>>  	__IFLA_MAX
>>  };
>>  
>> @@ -988,6 +989,7 @@ enum {
>>  	XDP_ATTACHED_SKB,
>>  	XDP_ATTACHED_HW,
>>  	XDP_ATTACHED_MULTI,
>> +	XDP_ATTACHED_EGRESS_CORE,
>>  };
>>  
>>  enum {
>> @@ -1000,6 +1002,7 @@ enum {
>>  	IFLA_XDP_SKB_PROG_ID,
>>  	IFLA_XDP_HW_PROG_ID,
>>  	IFLA_XDP_EXPECTED_FD,
>> +	IFLA_XDP_EGRESS_CORE_PROG_ID,
>>  	__IFLA_XDP_MAX,
>>  };
>>  
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index dc44af16226a..e9bc5cee06c8 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1030,7 +1030,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
>>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */
>>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
>>  	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
>> -	       + rtnl_xdp_size() /* IFLA_XDP */
>> +	       + rtnl_xdp_size() * 2 /* IFLA_XDP and IFLA_XDP_EGRESS */
>>  	       + nla_total_size(4)  /* IFLA_EVENT */
>>  	       + nla_total_size(4)  /* IFLA_NEW_NETNSID */
>>  	       + nla_total_size(4)  /* IFLA_NEW_IFINDEX */
>> @@ -1395,6 +1395,42 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
>>  	return 0;
>>  }
>>  
>> +static u32 rtnl_xdp_egress_prog(struct net_device *dev)
>> +{
>> +	const struct bpf_prog *prog;
>> +
>> +	ASSERT_RTNL();
>> +
>> +	prog = rtnl_dereference(dev->xdp_egress_prog);
>> +	if (!prog)
>> +		return 0;
>> +	return prog->aux->id;
>> +}
>> +
>> +static int rtnl_xdp_egress_report(struct sk_buff *skb, struct net_device *dev,
>> +				  u32 *prog_id, u8 *mode, u8 tgt_mode, u32 attr,
>> +				  u32 (*get_prog_id)(struct net_device *dev))
>> +{
>> +	u32 curr_id;
>> +	int err;
>> +
>> +	curr_id = get_prog_id(dev);
>> +	if (!curr_id)
>> +		return 0;
>> +
>> +	*prog_id = curr_id;
>> +	err = nla_put_u32(skb, attr, curr_id);
>> +	if (err)
>> +		return err;
>> +
>> +	if (*mode != XDP_ATTACHED_NONE)
>> +		*mode = XDP_ATTACHED_MULTI;
>> +	else
>> +		*mode = tgt_mode;
>> +
>> +	return 0;
>> +}
>> +
>>  static u32 rtnl_xdp_prog_skb(struct net_device *dev)
>>  {
>>  	const struct bpf_prog *generic_xdp_prog;
>> @@ -1486,6 +1522,42 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
>>  	return err;
>>  }
>>  
>> +static int rtnl_xdp_egress_fill(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +	u8 mode = XDP_ATTACHED_NONE;
>> +	struct nlattr *xdp;
>> +	u32 prog_id = 0;
>> +	int err;
>> +
>> +	xdp = nla_nest_start_noflag(skb, IFLA_XDP_EGRESS);
>> +	if (!xdp)
>> +		return -EMSGSIZE;
>> +
>> +	err = rtnl_xdp_egress_report(skb, dev, &prog_id, &mode,
>> +				     XDP_ATTACHED_EGRESS_CORE,
>> +				     IFLA_XDP_EGRESS_CORE_PROG_ID,
>> +				     rtnl_xdp_egress_prog);
>> +	if (err)
>> +		goto err_cancel;
>> +
>> +	err = nla_put_u8(skb, IFLA_XDP_ATTACHED, mode);
>> +	if (err)
>> +		goto err_cancel;
>> +
>> +	if (prog_id && mode != XDP_ATTACHED_MULTI) {
>> +		err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
>> +		if (err)
>> +			goto err_cancel;
>> +	}
>> +
>> +	nla_nest_end(skb, xdp);
>> +	return 0;
>> +
>> +err_cancel:
>> +	nla_nest_cancel(skb, xdp);
>> +	return err;
>> +}
>> +
>>  static u32 rtnl_get_event(unsigned long event)
>>  {
>>  	u32 rtnl_event_type = IFLA_EVENT_NONE;
>> @@ -1743,6 +1815,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
>>  	if (rtnl_xdp_fill(skb, dev))
>>  		goto nla_put_failure;
>>  
>> +	if (rtnl_xdp_egress_fill(skb, dev))
>> +		goto nla_put_failure;
>> +
>>  	if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
>>  		if (rtnl_link_fill(skb, dev) < 0)
>>  			goto nla_put_failure;
>> @@ -1827,6 +1902,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>>  	[IFLA_ALT_IFNAME]	= { .type = NLA_STRING,
>>  				    .len = ALTIFNAMSIZ - 1 },
>>  	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
>> +	[IFLA_XDP_EGRESS]	= { .type = NLA_NESTED },
>>  };
>>  
>>  static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>> @@ -2482,7 +2558,8 @@ static int do_set_master(struct net_device *dev, int ifindex,
>>  #define DO_SETLINK_NOTIFY	0x03
>>  
>>  static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>> -			  int *status, struct netlink_ext_ack *extack)
>> +			  int *status, bool egress,
>> +			  struct netlink_ext_ack *extack)
>>  {
>>  	struct nlattr *xdp[IFLA_XDP_MAX + 1];
>>  	u32 xdp_flags = 0;
>> @@ -2498,6 +2575,10 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>>  
>>  	if (xdp[IFLA_XDP_FLAGS]) {
>>  		xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
>> +		if (egress && xdp_flags & XDP_FLAGS_MODES) {
>> +			NL_SET_ERR_MSG(extack, "XDP_FLAGS_MODES not valid for egress");
>> +			goto out_einval;
>> +		}
>>  		if (xdp_flags & ~XDP_FLAGS_MASK)
>>  			goto out_einval;
>>  		if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1)
>> @@ -2515,7 +2596,7 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>>  
>>  		err = dev_change_xdp_fd(dev, extack,
>>  					nla_get_s32(xdp[IFLA_XDP_FD]),
>> -					expected_fd, xdp_flags, false);
>> +					expected_fd, xdp_flags, egress);
>>  		if (err)
>>  			return err;
>>  
>> @@ -2821,7 +2902,14 @@ static int do_setlink(const struct sk_buff *skb,
>>  	}
>>  
>>  	if (tb[IFLA_XDP]) {
>> -		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, extack);
>> +		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, false, extack);
>> +		if (err)
>> +			goto errout;
>> +	}
>> +
>> +	if (tb[IFLA_XDP_EGRESS]) {
>> +		err = do_setlink_xdp(dev, tb[IFLA_XDP_EGRESS], &status, true,
>> +				     extack);
>>  		if (err)
>>  			goto errout;
>>  	}
> 
> This means that IFLA_XDP and IFLA_XDP_EGRESS can be present in the same
> netlink message, right? But then installation of the RX program could
> succeed, but userspace would still get an error if the egress program
> installation fails? That is probably not good?

That's a good catch.

> 
> Since I don't think we can atomically make sure both operations fail or
> succeed, maybe it's better to disallow both entries being present in the
> same netlink message?
> 

I think so since there is no way to undo the setlink for IFLA_XDP if
IFLA_XDP_EGRESS fails - exisitng program references are gone.

Although, existing do_setlink leaves the device in an incomplete state
on any failure...

Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

[David Ahern]

On 4/21/20 4:20 AM, Toke Høiland-Jørgensen wrote:
> Isn't the kernel returning both program types in the same message when
> dumping an interface? So do we really need a separate getter instead of
> just populating xdp_link_info with the egress ID in the existing getter?

That might work with some refactoring of get_xdp_info. I'll take a look.

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/21/20 4:14 AM, Toke Høiland-Jørgensen wrote:
>> As I pointed out on the RFC patch, I'm concerned whether this will work
>> right with freplace programs attaching to XDP programs. It may just be
>> that I'm missing something, but in that case please explain why it
>> works? :)
>
> expected_attach_type is not unique to XDP. freplace is not unique to
> XDP. IF there is a problem, it is not unique to XDP, and any
> enhancements needed to freplace functionality will not be unique to XDP.

Still needs to be fixed, though :)

Also, at least looking through all the is_valid_access functions in
filter.c, they all seem to "fail safe". I.e., specific
expected_attach_type values can permit the program access to additional
ranges. In which case an freplace program that doesn't have the right
attach type will just be rejected if it tries to access such a field.
Whereas here you're *disallowing* something based on a particular
expected_attach_type, so you can end up with an egress program that
should have been rejected by the verifier but isn't because it's missing
the attach_type.

-Toke

Re: [PATCH bpf-next 06/16] net: Add IFLA_XDP_EGRESS for XDP programs in the egress path

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/21/20 4:17 AM, Toke Høiland-Jørgensen wrote:
>> David Ahern <dsahern@kernel.org> writes:
>> 
>>> From: David Ahern <dahern@digitalocean.com>
>>>
>>> Running programs in the egress path, on skbs or xdp_frames, does not
>>> require driver specific resources like Rx path. Accordingly, the
>>> programs can be run in core code, so add xdp_egress_prog to net_device
>>> to hold a reference to an attached program.
>>>
>>> For UAPI, add IFLA_XDP_EGRESS to if_link.h to specify egress programs,
>>> add a new attach flag, XDP_ATTACHED_EGRESS_CORE, to denote the
>>> attach point is at the core level (as opposed to driver or hardware)
>>> and add IFLA_XDP_EGRESS_CORE_PROG_ID for reporting the program id.
>>>
>>> Add egress argument to do_setlink_xdp to denote processing of
>>> IFLA_XDP_EGRESS versus IFLA_XDP, and add a check that none of the
>>> existing modes (SKB, DRV or HW) are set since those modes are not
>>> valid. The expectation is that XDP_FLAGS_HW_MODE will be used later
>>> (e.g., offloading guest programs).
>>>
>>> Add rtnl_xdp_egress_fill and helpers as the egress counterpart to the
>>> existing rtnl_xdp_fill.
>>>
>>> Signed-off-by: David Ahern <dahern@digitalocean.com>
>>> ---
>>>  include/linux/netdevice.h          |  1 +
>>>  include/uapi/linux/if_link.h       |  3 +
>>>  net/core/rtnetlink.c               | 96 ++++++++++++++++++++++++++++--
>>>  tools/include/uapi/linux/if_link.h |  3 +
>>>  4 files changed, 99 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index d0bb9e09660a..3133247681fd 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -1995,6 +1995,7 @@ struct net_device {
>>>  	unsigned int		real_num_rx_queues;
>>>  
>>>  	struct bpf_prog __rcu	*xdp_prog;
>>> +	struct bpf_prog __rcu	*xdp_egress_prog;
>>>  	unsigned long		gro_flush_timeout;
>>>  	rx_handler_func_t __rcu	*rx_handler;
>>>  	void __rcu		*rx_handler_data;
>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>> index 127c704eeba9..b3c6cb2f0f0a 100644
>>> --- a/include/uapi/linux/if_link.h
>>> +++ b/include/uapi/linux/if_link.h
>>> @@ -170,6 +170,7 @@ enum {
>>>  	IFLA_PROP_LIST,
>>>  	IFLA_ALT_IFNAME, /* Alternative ifname */
>>>  	IFLA_PERM_ADDRESS,
>>> +	IFLA_XDP_EGRESS, /* nested attribute with 1 or more IFLA_XDP_ attrs */
>>>  	__IFLA_MAX
>>>  };
>>>  
>>> @@ -988,6 +989,7 @@ enum {
>>>  	XDP_ATTACHED_SKB,
>>>  	XDP_ATTACHED_HW,
>>>  	XDP_ATTACHED_MULTI,
>>> +	XDP_ATTACHED_EGRESS_CORE,
>>>  };
>>>  
>>>  enum {
>>> @@ -1000,6 +1002,7 @@ enum {
>>>  	IFLA_XDP_SKB_PROG_ID,
>>>  	IFLA_XDP_HW_PROG_ID,
>>>  	IFLA_XDP_EXPECTED_FD,
>>> +	IFLA_XDP_EGRESS_CORE_PROG_ID,
>>>  	__IFLA_XDP_MAX,
>>>  };
>>>  
>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>>> index dc44af16226a..e9bc5cee06c8 100644
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -1030,7 +1030,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
>>>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */
>>>  	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
>>>  	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
>>> -	       + rtnl_xdp_size() /* IFLA_XDP */
>>> +	       + rtnl_xdp_size() * 2 /* IFLA_XDP and IFLA_XDP_EGRESS */
>>>  	       + nla_total_size(4)  /* IFLA_EVENT */
>>>  	       + nla_total_size(4)  /* IFLA_NEW_NETNSID */
>>>  	       + nla_total_size(4)  /* IFLA_NEW_IFINDEX */
>>> @@ -1395,6 +1395,42 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
>>>  	return 0;
>>>  }
>>>  
>>> +static u32 rtnl_xdp_egress_prog(struct net_device *dev)
>>> +{
>>> +	const struct bpf_prog *prog;
>>> +
>>> +	ASSERT_RTNL();
>>> +
>>> +	prog = rtnl_dereference(dev->xdp_egress_prog);
>>> +	if (!prog)
>>> +		return 0;
>>> +	return prog->aux->id;
>>> +}
>>> +
>>> +static int rtnl_xdp_egress_report(struct sk_buff *skb, struct net_device *dev,
>>> +				  u32 *prog_id, u8 *mode, u8 tgt_mode, u32 attr,
>>> +				  u32 (*get_prog_id)(struct net_device *dev))
>>> +{
>>> +	u32 curr_id;
>>> +	int err;
>>> +
>>> +	curr_id = get_prog_id(dev);
>>> +	if (!curr_id)
>>> +		return 0;
>>> +
>>> +	*prog_id = curr_id;
>>> +	err = nla_put_u32(skb, attr, curr_id);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	if (*mode != XDP_ATTACHED_NONE)
>>> +		*mode = XDP_ATTACHED_MULTI;
>>> +	else
>>> +		*mode = tgt_mode;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static u32 rtnl_xdp_prog_skb(struct net_device *dev)
>>>  {
>>>  	const struct bpf_prog *generic_xdp_prog;
>>> @@ -1486,6 +1522,42 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
>>>  	return err;
>>>  }
>>>  
>>> +static int rtnl_xdp_egress_fill(struct sk_buff *skb, struct net_device *dev)
>>> +{
>>> +	u8 mode = XDP_ATTACHED_NONE;
>>> +	struct nlattr *xdp;
>>> +	u32 prog_id = 0;
>>> +	int err;
>>> +
>>> +	xdp = nla_nest_start_noflag(skb, IFLA_XDP_EGRESS);
>>> +	if (!xdp)
>>> +		return -EMSGSIZE;
>>> +
>>> +	err = rtnl_xdp_egress_report(skb, dev, &prog_id, &mode,
>>> +				     XDP_ATTACHED_EGRESS_CORE,
>>> +				     IFLA_XDP_EGRESS_CORE_PROG_ID,
>>> +				     rtnl_xdp_egress_prog);
>>> +	if (err)
>>> +		goto err_cancel;
>>> +
>>> +	err = nla_put_u8(skb, IFLA_XDP_ATTACHED, mode);
>>> +	if (err)
>>> +		goto err_cancel;
>>> +
>>> +	if (prog_id && mode != XDP_ATTACHED_MULTI) {
>>> +		err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
>>> +		if (err)
>>> +			goto err_cancel;
>>> +	}
>>> +
>>> +	nla_nest_end(skb, xdp);
>>> +	return 0;
>>> +
>>> +err_cancel:
>>> +	nla_nest_cancel(skb, xdp);
>>> +	return err;
>>> +}
>>> +
>>>  static u32 rtnl_get_event(unsigned long event)
>>>  {
>>>  	u32 rtnl_event_type = IFLA_EVENT_NONE;
>>> @@ -1743,6 +1815,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
>>>  	if (rtnl_xdp_fill(skb, dev))
>>>  		goto nla_put_failure;
>>>  
>>> +	if (rtnl_xdp_egress_fill(skb, dev))
>>> +		goto nla_put_failure;
>>> +
>>>  	if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
>>>  		if (rtnl_link_fill(skb, dev) < 0)
>>>  			goto nla_put_failure;
>>> @@ -1827,6 +1902,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>>>  	[IFLA_ALT_IFNAME]	= { .type = NLA_STRING,
>>>  				    .len = ALTIFNAMSIZ - 1 },
>>>  	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
>>> +	[IFLA_XDP_EGRESS]	= { .type = NLA_NESTED },
>>>  };
>>>  
>>>  static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>>> @@ -2482,7 +2558,8 @@ static int do_set_master(struct net_device *dev, int ifindex,
>>>  #define DO_SETLINK_NOTIFY	0x03
>>>  
>>>  static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>>> -			  int *status, struct netlink_ext_ack *extack)
>>> +			  int *status, bool egress,
>>> +			  struct netlink_ext_ack *extack)
>>>  {
>>>  	struct nlattr *xdp[IFLA_XDP_MAX + 1];
>>>  	u32 xdp_flags = 0;
>>> @@ -2498,6 +2575,10 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>>>  
>>>  	if (xdp[IFLA_XDP_FLAGS]) {
>>>  		xdp_flags = nla_get_u32(xdp[IFLA_XDP_FLAGS]);
>>> +		if (egress && xdp_flags & XDP_FLAGS_MODES) {
>>> +			NL_SET_ERR_MSG(extack, "XDP_FLAGS_MODES not valid for egress");
>>> +			goto out_einval;
>>> +		}
>>>  		if (xdp_flags & ~XDP_FLAGS_MASK)
>>>  			goto out_einval;
>>>  		if (hweight32(xdp_flags & XDP_FLAGS_MODES) > 1)
>>> @@ -2515,7 +2596,7 @@ static int do_setlink_xdp(struct net_device *dev, struct nlattr *tb,
>>>  
>>>  		err = dev_change_xdp_fd(dev, extack,
>>>  					nla_get_s32(xdp[IFLA_XDP_FD]),
>>> -					expected_fd, xdp_flags, false);
>>> +					expected_fd, xdp_flags, egress);
>>>  		if (err)
>>>  			return err;
>>>  
>>> @@ -2821,7 +2902,14 @@ static int do_setlink(const struct sk_buff *skb,
>>>  	}
>>>  
>>>  	if (tb[IFLA_XDP]) {
>>> -		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, extack);
>>> +		err = do_setlink_xdp(dev, tb[IFLA_XDP], &status, false, extack);
>>> +		if (err)
>>> +			goto errout;
>>> +	}
>>> +
>>> +	if (tb[IFLA_XDP_EGRESS]) {
>>> +		err = do_setlink_xdp(dev, tb[IFLA_XDP_EGRESS], &status, true,
>>> +				     extack);
>>>  		if (err)
>>>  			goto errout;
>>>  	}
>> 
>> This means that IFLA_XDP and IFLA_XDP_EGRESS can be present in the same
>> netlink message, right? But then installation of the RX program could
>> succeed, but userspace would still get an error if the egress program
>> installation fails? That is probably not good?
>
> That's a good catch.
>
>> 
>> Since I don't think we can atomically make sure both operations fail or
>> succeed, maybe it's better to disallow both entries being present in the
>> same netlink message?
>> 
>
> I think so since there is no way to undo the setlink for IFLA_XDP if
> IFLA_XDP_EGRESS fails - exisitng program references are gone.

Yeah, and also even if we could undo it, there would be a window where
the new program could process packets before the revert. I'm fine with
just rejecting the combination; I guess the most common thing would be
to set these separately anyway.

> Although, existing do_setlink leaves the device in an incomplete state
> on any failure...

Hmm, ideally we should fix that as well (if possible)? Probably out of
scope for this patch series, though :)

-Toke

Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/21/20 4:20 AM, Toke Høiland-Jørgensen wrote:
>> Isn't the kernel returning both program types in the same message when
>> dumping an interface? So do we really need a separate getter instead of
>> just populating xdp_link_info with the egress ID in the existing getter?
>
> That might work with some refactoring of get_xdp_info. I'll take a look.

Great, thanks! :)

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/21/20 7:25 AM, Toke Høiland-Jørgensen wrote:
> David Ahern <dsahern@gmail.com> writes:
> 
>> On 4/21/20 4:14 AM, Toke Høiland-Jørgensen wrote:
>>> As I pointed out on the RFC patch, I'm concerned whether this will work
>>> right with freplace programs attaching to XDP programs. It may just be
>>> that I'm missing something, but in that case please explain why it
>>> works? :)
>>
>> expected_attach_type is not unique to XDP. freplace is not unique to
>> XDP. IF there is a problem, it is not unique to XDP, and any
>> enhancements needed to freplace functionality will not be unique to XDP.
> 
> Still needs to be fixed, though :)

one problem at a time. I have a long list of items that are directly
relevant to what I want to do.

> 
> Also, at least looking through all the is_valid_access functions in
> filter.c, they all seem to "fail safe". I.e., specific
> expected_attach_type values can permit the program access to additional
> ranges. In which case an freplace program that doesn't have the right
> attach type will just be rejected if it tries to access such a field.
> Whereas here you're *disallowing* something based on a particular
> expected_attach_type, so you can end up with an egress program that
> should have been rejected by the verifier but isn't because it's missing
> the attach_type.

There are 6 existing valid access checks on expected_attach_type doing
the exact same thing - validating access based on attach type.

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/21/20 7:25 AM, Toke Høiland-Jørgensen wrote:
>> David Ahern <dsahern@gmail.com> writes:
>> 
>>> On 4/21/20 4:14 AM, Toke Høiland-Jørgensen wrote:
>>>> As I pointed out on the RFC patch, I'm concerned whether this will work
>>>> right with freplace programs attaching to XDP programs. It may just be
>>>> that I'm missing something, but in that case please explain why it
>>>> works? :)
>>>
>>> expected_attach_type is not unique to XDP. freplace is not unique to
>>> XDP. IF there is a problem, it is not unique to XDP, and any
>>> enhancements needed to freplace functionality will not be unique to XDP.
>> 
>> Still needs to be fixed, though :)
>
> one problem at a time. I have a long list of items that are directly
> relevant to what I want to do.

Not saying a fix to freplace *has* to be part of this series; just
saying that I would be more comfortable if that was fixed before we
merge this.

>> Also, at least looking through all the is_valid_access functions in
>> filter.c, they all seem to "fail safe". I.e., specific
>> expected_attach_type values can permit the program access to additional
>> ranges. In which case an freplace program that doesn't have the right
>> attach type will just be rejected if it tries to access such a field.
>> Whereas here you're *disallowing* something based on a particular
>> expected_attach_type, so you can end up with an egress program that
>> should have been rejected by the verifier but isn't because it's missing
>> the attach_type.
>
> There are 6 existing valid access checks on expected_attach_type doing
> the exact same thing - validating access based on attach type.

See my point about default black/white listing, though. You are adding a
new restriction to an existing program type based on this, so surely we
should make sure this restriction actually sticks, no?

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/22/20 5:21 AM, Toke Høiland-Jørgensen wrote:
> Not saying a fix to freplace *has* to be part of this series; just
> saying that I would be more comfortable if that was fixed before we
> merge this.

You don't have a test case that fails, you just think there is a problem
based on code analysis. Have you taken this presumption up with the
author of the freplace code?

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/22/20 5:21 AM, Toke Høiland-Jørgensen wrote:
>> Not saying a fix to freplace *has* to be part of this series; just
>> saying that I would be more comfortable if that was fixed before we
>> merge this.
>
> You don't have a test case that fails, you just think there is a problem
> based on code analysis. Have you taken this presumption up with the
> author of the freplace code?

Well this is a mailing list, isn't it? So that's what I'm doing;
hopefully Alexei will chime in... Otherwise I guess I can produce a
testcase tomorrow.

And as I said in the beginning, I'm perfectly happy to be told why I'm
wrong; but so far you have just been arguing that I'm out of scope ;)

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
> And as I said in the beginning, I'm perfectly happy to be told why I'm
> wrong; but so far you have just been arguing that I'm out of scope ;)

you are arguing about a suspected bug with existing code that is no way
touched or modified by this patch set, so yes it is out of scope.

The proper way to discuss a bug in existing code is a thread focused on
that topic, not buried inside comments on a patch set.

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
>> And as I said in the beginning, I'm perfectly happy to be told why I'm
>> wrong; but so far you have just been arguing that I'm out of scope ;)
>
> you are arguing about a suspected bug with existing code that is no way
> touched or modified by this patch set, so yes it is out of scope.

Your patch is relying on the (potentially buggy) behaviour, so I don't
think it's out of scope to mention it in this context.

> The proper way to discuss a bug in existing code is a thread focused on
> that topic, not buried inside comments on a patch set.

I'm fine with starting a new thread; will do (but as I said, tomorrow).

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/22/20 9:51 AM, Toke Høiland-Jørgensen wrote:
> Your patch is relying on the (potentially buggy) behaviour, so I don't
> think it's out of scope to mention it in this context.

This is getting ridiculous. I am in no way, shape, or form relying on
freplace.

If you think there is a problem with existing code, then you prove it.
Don't ask someone else to stop what they are doing and prove you right
or wrong.

I guess we'll see tomorrow the outcome of your investigations.

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Alexei Starovoitov]

On Wed, Apr 22, 2020 at 05:51:36PM +0200, Toke Høiland-Jørgensen wrote:
> David Ahern <dsahern@gmail.com> writes:
> 
> > On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
> >> And as I said in the beginning, I'm perfectly happy to be told why I'm
> >> wrong; but so far you have just been arguing that I'm out of scope ;)
> >
> > you are arguing about a suspected bug with existing code that is no way
> > touched or modified by this patch set, so yes it is out of scope.
> 
> Your patch is relying on the (potentially buggy) behaviour, so I don't
> think it's out of scope to mention it in this context.

Sorry for slow reply.
I'm swamped with other things atm.

Looks like there is indeed a bug in prog_type_ext handling code that
is doing
env->ops = bpf_verifier_ops[tgt_prog->type];
I'm not sure whether the verifier can simply add:
prog->expected_attach_type = tgt_prog->expected_attach_type;
and be done with it.
Likely yes, since expected_attach_type must be zero at that point
that is enforced by bpf_prog_load_check_attach().
So I suspect it's a single line fix.
A selftest to prove or disprove is necessary, of course.

Thanks Toke for bringing it to my attention.

Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

[Andrii Nakryiko]

On Mon, Apr 20, 2020 at 1:02 PM David Ahern <dsahern@kernel.org> wrote:
>
> From: David Ahern <dahern@digitalocean.com>
>
> Patch adds egress XDP support in libbpf.
>
> New section name hint, xdp_egress, is added to set expected attach
> type at program load. Programs can use xdp_egress as the prefix in
> the SEC statement to load the program with the BPF_XDP_EGRESS
> attach type set.
>
> egress is added to bpf_xdp_set_link_opts to specify egress type for
> use with bpf_set_link_xdp_fd_opts. Update library side to check
> for flag and set nla_type to IFLA_XDP_EGRESS.
>
> Add egress version of bpf_get_link_xdp* info and id apis with core
> code refactored to handle both rx and tx paths.
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> Co-developed-by: David Ahern <dahern@digitalocean.com>
> Signed-off-by: David Ahern <dahern@digitalocean.com>
> ---
>  tools/lib/bpf/libbpf.c   |  2 ++
>  tools/lib/bpf/libbpf.h   |  9 +++++-
>  tools/lib/bpf/libbpf.map |  2 ++
>  tools/lib/bpf/netlink.c  | 63 +++++++++++++++++++++++++++++++++++-----
>  4 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8f480e29a6b0..32fc970495d9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -6366,6 +6366,8 @@ static const struct bpf_sec_def section_defs[] = {
>                 .is_attach_btf = true,
>                 .expected_attach_type = BPF_LSM_MAC,
>                 .attach_fn = attach_lsm),
> +       BPF_EAPROG_SEC("xdp_egress",            BPF_PROG_TYPE_XDP,
> +                                               BPF_XDP_EGRESS),
>         BPF_PROG_SEC("xdp",                     BPF_PROG_TYPE_XDP),
>         BPF_PROG_SEC("perf_event",              BPF_PROG_TYPE_PERF_EVENT),
>         BPF_PROG_SEC("lwt_in",                  BPF_PROG_TYPE_LWT_IN),
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index f1dacecb1619..3feb1242f78e 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -453,14 +453,16 @@ struct xdp_link_info {
>         __u32 drv_prog_id;
>         __u32 hw_prog_id;
>         __u32 skb_prog_id;
> +       __u32 egress_core_prog_id;

This changes layout of struct xdp_link_info in ABI-breaking way. New
fields have to be added at the end.

>         __u8 attach_mode;
>  };
>
>  struct bpf_xdp_set_link_opts {
>         size_t sz;
>         int old_fd;
> +       __u8  egress;

Is this a true/false field? If yes, why not bool then?

>  };
> -#define bpf_xdp_set_link_opts__last_field old_fd
> +#define bpf_xdp_set_link_opts__last_field egress
>
>  LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
>  LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
> @@ -468,6 +470,11 @@ LIBBPF_API int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
>  LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
>  LIBBPF_API int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
>                                      size_t info_size, __u32 flags);
> +LIBBPF_API int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id,
> +                                         __u32 flags);
> +LIBBPF_API int bpf_get_link_xdp_egress_info(int ifindex,
> +                                           struct xdp_link_info *info,
> +                                           size_t info_size, __u32 flags);
>
>  struct perf_buffer;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index bb8831605b25..51576c8a02fe 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -253,4 +253,6 @@ LIBBPF_0.0.8 {
>                 bpf_program__set_attach_target;
>                 bpf_program__set_lsm;
>                 bpf_set_link_xdp_fd_opts;
> +               bpf_get_link_xdp_egress_id;
> +               bpf_get_link_xdp_egress_info;

This should go into 0.0.9 section, 0.0.8 is sealed now.

>  } LIBBPF_0.0.7;

[...]

> @@ -203,6 +204,7 @@ static int __bpf_set_link_xdp_fd_replace(int ifindex, int fd, int old_fd,
>  int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
>                              const struct bpf_xdp_set_link_opts *opts)
>  {
> +       __u16 nla_type = IFLA_XDP;
>         int old_fd = -1;
>
>         if (!OPTS_VALID(opts, bpf_xdp_set_link_opts))
> @@ -213,14 +215,22 @@ int bpf_set_link_xdp_fd_opts(int ifindex, int fd, __u32 flags,
>                 flags |= XDP_FLAGS_REPLACE;
>         }
>
> +       if (OPTS_HAS(opts, egress)) {

I don't think you need to check OPTS_HAS here, just OPTS_GET with
proper default would work.

> +               __u8 egress = OPTS_GET(opts, egress, 0);
> +
> +               if (egress)
> +                       nla_type = IFLA_XDP_EGRESS;
> +       }
> +
>         return __bpf_set_link_xdp_fd_replace(ifindex, fd,
>                                              old_fd,
> -                                            flags);
> +                                            flags,

nit: old_fd, flags fit on the same line as ifindex, fd, not sure why
there are so many lines for this?

> +                                            nla_type);
>  }
>
>  int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
>  {
> -       return __bpf_set_link_xdp_fd_replace(ifindex, fd, 0, flags);
> +       return __bpf_set_link_xdp_fd_replace(ifindex, fd, 0, flags, IFLA_XDP);
>  }
>

[...]

>
> +int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
> +                         size_t info_size, __u32 flags)
> +{
> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
> +                                      IFLA_XDP);
> +}
> +
> +int bpf_get_link_xdp_egress_info(int ifindex, struct xdp_link_info *info,
> +                                size_t info_size, __u32 flags)
> +{
> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
> +                                      IFLA_XDP_EGRESS);
> +}
> +
>  static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
>  {
>         if (info->attach_mode != XDP_ATTACHED_MULTI && !flags)
> @@ -345,6 +376,22 @@ int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
>         return ret;
>  }
>
> +int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id, __u32 flags)

Is bpf_get_link_xdp_egress_id() even needed? This is a special case of
bpf_get_link_xdp_egress_info(), I don't think we have to add another
API to support it specifically.

Also, just curious, would it be better to have a generalized
XDP/XDP_EGRESS xdp_info() functions instead of two separate ones?
Could there be some third variant of XDP program later, whatever that
might be?

> +{
> +       struct xdp_link_info info;
> +       int ret;
> +
> +       /* egress path does not support SKB, DRV or HW mode */
> +       if (flags & XDP_FLAGS_MODES)
> +               return -EINVAL;
> +
> +       ret = bpf_get_link_xdp_egress_info(ifindex, &info, sizeof(info), flags);
> +       if (!ret)
> +               *prog_id = get_xdp_id(&info, flags);
> +
> +       return ret;
> +}
> +
>  int libbpf_nl_get_link(int sock, unsigned int nl_pid,
>                        libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
>  {
> --
> 2.21.1 (Apple Git-122.3)
>

Re: [PATCH bpf-next 12/16] libbpf: Add egress XDP support

[David Ahern]

On 4/22/20 7:19 PM, Andrii Nakryiko wrote:
>>
>> +int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
>> +                         size_t info_size, __u32 flags)
>> +{
>> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
>> +                                      IFLA_XDP);
>> +}
>> +
>> +int bpf_get_link_xdp_egress_info(int ifindex, struct xdp_link_info *info,
>> +                                size_t info_size, __u32 flags)
>> +{
>> +       return __bpf_get_link_xdp_info(ifindex, info, info_size, flags,
>> +                                      IFLA_XDP_EGRESS);
>> +}
>> +
>>  static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
>>  {
>>         if (info->attach_mode != XDP_ATTACHED_MULTI && !flags)
>> @@ -345,6 +376,22 @@ int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
>>         return ret;
>>  }
>>
>> +int bpf_get_link_xdp_egress_id(int ifindex, __u32 *prog_id, __u32 flags)
> 
> Is bpf_get_link_xdp_egress_id() even needed? This is a special case of
> bpf_get_link_xdp_egress_info(), I don't think we have to add another
> API to support it specifically.
> 
> Also, just curious, would it be better to have a generalized
> XDP/XDP_EGRESS xdp_info() functions instead of two separate ones?

Toke mentioned that too, and I have removed bpf_get_link_xdp_egress_info.

bpf_get_link_xdp_egress_id is needed. The flags argument is a direct
pass through for the kernel uapi and correlates to XDP_FLAGS that are
passed to the kernel. There is no harm or overhead in exporting symbols.
Having a simple, easy to use API for applications is important to making
a library user friendly which is key to its adoption.

Will fix the other comments.

Re: [PATCH bpf-next 13/16] bpftool: Add support for XDP egress

[Quentin Monnet]

2020-04-20 14:00 UTC-0600 ~ David Ahern <dsahern@kernel.org>
> From: David Ahern <dahern@digitalocean.com>
> 
> Add NET_ATTACH_TYPE_XDP_EGRESS and update attach_type_strings to
> allow a user to specify 'xdp_egress' as the attach or detach point.
> 
> libbpf handles egress config via bpf_set_link_xdp_fd_opts, so
> update do_attach_detach_xdp to use it. Specifically, the new API
> requires old_fd to be set based on any currently loaded program,
> so use bpf_get_link_xdp_id and bpf_get_link_xdp_egress_id to get
> an fd to any existing program.
> 
> Update 'net show' command to dump egress programs.
> 
> Signed-off-by: David Ahern <dahern@digitalocean.com>
> ---

Thanks for the bpftool update. Can you please also update the man pages
(bpftool-prog, bpftool-net) and bash completion?

Quentin

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

David Ahern <dsahern@gmail.com> writes:

> On 4/22/20 9:51 AM, Toke Høiland-Jørgensen wrote:
>> Your patch is relying on the (potentially buggy) behaviour, so I don't
>> think it's out of scope to mention it in this context.
>
> This is getting ridiculous. I am in no way, shape, or form relying on
> freplace.

No, I meant that you're relying on 'expected_attach_type'.

> I guess we'll see tomorrow the outcome of your investigations.

Try this (when running a kernel with your XDP egress patches applied,
obviously):

$ git clone --recurse-submodules -b xdp-egress https://github.com/xdp-project/xdp-tools
$ cd xdp-tools && make && cd xdp-loader
$ sudo ./xdp-loader load --egress eth0 xdp-ctx-test.o xdp-ctx-test.o

Leads to an instant kernel crash (well, assuming there's any traffic on
the device, I suppose):

[  804.417699] general protection fault, probably for non-canonical address 0x10a18210d14ba9: 0000 [#1] SMP PTI
[  804.427518] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.7.0-rc1+ #204
[  804.434053] Hardware name: LENOVO 30B3005DMT/102F, BIOS S00KT56A 01/15/2018
[  804.441012] RIP: 0010:bpf_prog_3525d0165d4bca62_xdp_drop_func+0x1d/0xc98
[  804.447704] Code: cc cc cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 48 81 ec 00 00 00 00 53 41 55 41 56 41 57 6a 00 48 8b 7f 28 <48> 8b 7f 00 8b bf 00 01 00 00 b8 01 00 00 00 48 85 ff 74 05 b8 02
[  804.466442] RSP: 0018:ffffb177c007ca38 EFLAGS: 00010286
[  804.471662] RAX: 000000000000dd86 RBX: ffffb177c007cb10 RCX: ffff939cde4ae102
[  804.478782] RDX: ffffffffc0bcb680 RSI: ffffb177c016b038 RDI: 0010a18210d14ba9
[  804.485903] RBP: ffffb177c007ca60 R08: ffff939cde4ae184 R09: 0000000000000001
[  804.493188] R10: ffff939d1c8028c0 R11: ffff939d1c3acf00 R12: ffffb177c016b000
[  804.500307] R13: 0000000000000002 R14: ffffb177c0594000 R15: ffff939cde4ae102
[  804.507430] FS:  0000000000000000(0000) GS:ffff939d1fc40000(0000) knlGS:0000000000000000
[  804.515510] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  804.521245] CR2: 00007f6705642640 CR3: 000000074860a002 CR4: 00000000003606e0
[  804.528374] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  804.535502] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  804.542629] Call Trace:
[  804.545076]  <IRQ>
[  804.547096]  bpf_prog_79f73a00e07a6bab_F+0x38/0x980
[  804.551974]  do_xdp_generic_core+0x12b/0x320
[  804.556366]  do_xdp_egress_skb+0x52/0x110
[  804.560376]  dev_hard_start_xmit+0x107/0x220
[  804.564644]  sch_direct_xmit+0xec/0x230
[  804.568481]  __qdisc_run+0x140/0x550
[  804.572058]  __dev_queue_xmit+0x4b3/0x780
[  804.576071]  ip6_finish_output2+0x248/0x5b0
[  804.580254]  ip6_output+0x73/0x120
[  804.583656]  ? __ip6_finish_output+0x100/0x100
[  804.588105]  mld_sendpack+0x1c1/0x230
[  804.591766]  mld_ifc_timer_expire+0x1ab/0x310
[  804.596122]  ? ip6_mc_leave_src+0x90/0x90
[  804.600134]  call_timer_fn+0x2b/0x140
[  804.603796]  __run_timers.part.0+0x16f/0x270
[  804.608068]  ? timerqueue_add+0x96/0xb0
[  804.611902]  ? enqueue_hrtimer+0x36/0x90
[  804.615827]  run_timer_softirq+0x26/0x50
[  804.619877]  __do_softirq+0xe1/0x2fe
[  804.623453]  irq_exit+0xa6/0xb0
[  804.626595]  smp_apic_timer_interrupt+0x68/0x130
[  804.631210]  apic_timer_interrupt+0xf/0x20
[  804.635308]  </IRQ>
[  804.637414] RIP: 0010:cpuidle_enter_state+0xc3/0x400
[  804.642377] Code: e8 b2 02 8b ff 80 7c 24 0f 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 0a 03 00 00 31 ff e8 64 14 91 ff fb 66 0f 1f 44 00 00 <45> 85 ed 0f 88 52 02 00 00 49 63 d5 4c 2b 64 24 10 48 8d 04 52 48
[  804.661114] RSP: 0018:ffffb177c00ffe60 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
[  804.668674] RAX: ffff939d1fc6c8c0 RBX: ffff939d1fc76708 RCX: 000000000000001f
[  804.675801] RDX: 0000000000000000 RSI: 0000000023a34d57 RDI: 0000000000000000
[  804.683035] RBP: ffffffff896e6760 R08: 000000bb4b078647 R09: 0000000000000498
[  804.690163] R10: 0000000000000ae9 R11: ffff939d1fc6ba44 R12: 000000bb4b078647
[  804.697283] R13: 0000000000000004 R14: 0000000000000004 R15: ffff939d1fc76708
[  804.704409]  cpuidle_enter+0x29/0x40
[  804.707987]  do_idle+0x1ff/0x290
[  804.711216]  cpu_startup_entry+0x19/0x20
[  804.715139]  start_secondary+0x161/0x1c0
[  804.719063]  secondary_startup_64+0xa4/0xb0
[  804.723246] Modules linked in: xt_nat mlx5_ib xt_CHECKSUM iptable_mangle xt_MASQUERADE iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_tcpudp bridge stp llc iptable_filter binfmt_misc nls_iso8859_1 ib_uverbs intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi irqbypass mlx5_core snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep snd_hda_core crct10dif_pclmul crc32_pclmul snd_pcm uas ghash_clmulni_intel usb_storage snd_timer wmi_bmof e1000e snd mei_me mei lpc_ich soundcore pata_acpi squashfs mac_hid ib_iser rdma_cm configfs iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 raid10 raid456 libcrc32c async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 multipath linear nouveau video i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm mxm_wmi aesni_intel
[  804.723288]  glue_helper crypto_simd cryptd ahci libahci wmi
[  804.816671] ---[ end trace d70f476e5883fde3 ]---
[  804.866476] RIP: 0010:bpf_prog_3525d0165d4bca62_xdp_drop_func+0x1d/0xc98
[  804.873269] Code: cc cc cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 48 81 ec 00 00 00 00 53 41 55 41 56 41 57 6a 00 48 8b 7f 28 <48> 8b 7f 00 8b bf 00 01 00 00 b8 01 00 00 00 48 85 ff 74 05 b8 02
[  804.892010] RSP: 0018:ffffb177c007ca38 EFLAGS: 00010286
[  804.897236] RAX: 000000000000dd86 RBX: ffffb177c007cb10 RCX: ffff939cde4ae102
[  804.904362] RDX: ffffffffc0bcb680 RSI: ffffb177c016b038 RDI: 0010a18210d14ba9
[  804.911491] RBP: ffffb177c007ca60 R08: ffff939cde4ae184 R09: 0000000000000001
[  804.918622] R10: ffff939d1c8028c0 R11: ffff939d1c3acf00 R12: ffffb177c016b000
[  804.925750] R13: 0000000000000002 R14: ffffb177c0594000 R15: ffff939cde4ae102
[  804.932879] FS:  0000000000000000(0000) GS:ffff939d1fc40000(0000) knlGS:0000000000000000
[  804.941135] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  804.946876] CR2: 00007f6705642640 CR3: 000000074860a002 CR4: 00000000003606e0
[  804.954006] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  804.961134] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  804.968265] Kernel panic - not syncing: Fatal exception in interrupt
[  804.974648] Kernel Offset: 0x7000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)


The XDP program being loaded (as a multi-prog with freplace) is simply
this:

SEC("xdp_ctx_test")
int xdp_drop_func(struct xdp_md *ctx)
{
        if (ctx->ingress_ifindex > 0)
                return XDP_PASS;

        return XDP_DROP;
}

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Wed, Apr 22, 2020 at 05:51:36PM +0200, Toke Høiland-Jørgensen wrote:
>> David Ahern <dsahern@gmail.com> writes:
>> 
>> > On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
>> >> And as I said in the beginning, I'm perfectly happy to be told why I'm
>> >> wrong; but so far you have just been arguing that I'm out of scope ;)
>> >
>> > you are arguing about a suspected bug with existing code that is no way
>> > touched or modified by this patch set, so yes it is out of scope.
>> 
>> Your patch is relying on the (potentially buggy) behaviour, so I don't
>> think it's out of scope to mention it in this context.
>
> Sorry for slow reply.
> I'm swamped with other things atm.
>
> Looks like there is indeed a bug in prog_type_ext handling code that
> is doing
> env->ops = bpf_verifier_ops[tgt_prog->type];
> I'm not sure whether the verifier can simply add:
> prog->expected_attach_type = tgt_prog->expected_attach_type;
> and be done with it.
> Likely yes, since expected_attach_type must be zero at that point
> that is enforced by bpf_prog_load_check_attach().
> So I suspect it's a single line fix.

Not quite: the check in bpf_tracing_prog_attach() that enforces
prog->expected_attach_type==0 also needs to go. So 5 lines :)

With those two changes, I do seem to get the correct behaviour for XDP
multiprogs in both ingress and egress mode (with David's patch set).

> A selftest to prove or disprove is necessary, of course.

I'll see if I can't hook this into the existing freplace selftest and
send a proper patch. I'm not really sure how to check for any hidden
side effects; in particular I'm a bit twitchy about removing that check
in bpf_tracing_prog_attach() - presumably it is there for a reason? But
I guess I'll send a patch and you can take a look.

> Thanks Toke for bringing it to my attention.

You're quite welcome! And thanks for the hint that the fix was that
simple; I feared something more invasive was needed.

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Alexei Starovoitov]

On Thu, Apr 23, 2020 at 9:40 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Wed, Apr 22, 2020 at 05:51:36PM +0200, Toke Høiland-Jørgensen wrote:
> >> David Ahern <dsahern@gmail.com> writes:
> >>
> >> > On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
> >> >> And as I said in the beginning, I'm perfectly happy to be told why I'm
> >> >> wrong; but so far you have just been arguing that I'm out of scope ;)
> >> >
> >> > you are arguing about a suspected bug with existing code that is no way
> >> > touched or modified by this patch set, so yes it is out of scope.
> >>
> >> Your patch is relying on the (potentially buggy) behaviour, so I don't
> >> think it's out of scope to mention it in this context.
> >
> > Sorry for slow reply.
> > I'm swamped with other things atm.
> >
> > Looks like there is indeed a bug in prog_type_ext handling code that
> > is doing
> > env->ops = bpf_verifier_ops[tgt_prog->type];
> > I'm not sure whether the verifier can simply add:
> > prog->expected_attach_type = tgt_prog->expected_attach_type;
> > and be done with it.
> > Likely yes, since expected_attach_type must be zero at that point
> > that is enforced by bpf_prog_load_check_attach().
> > So I suspect it's a single line fix.
>
> Not quite: the check in bpf_tracing_prog_attach() that enforces
> prog->expected_attach_type==0 also needs to go. So 5 lines :)

prog_ext's expected_attach_type needs to stay zero.
It needs to be inherited from tgt prog. Hence one line:
prog->expected_attach_type = tgt_prog->expected_attach_type;

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Apr 23, 2020 at 9:40 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>> > On Wed, Apr 22, 2020 at 05:51:36PM +0200, Toke Høiland-Jørgensen wrote:
>> >> David Ahern <dsahern@gmail.com> writes:
>> >>
>> >> > On 4/22/20 9:27 AM, Toke Høiland-Jørgensen wrote:
>> >> >> And as I said in the beginning, I'm perfectly happy to be told why I'm
>> >> >> wrong; but so far you have just been arguing that I'm out of scope ;)
>> >> >
>> >> > you are arguing about a suspected bug with existing code that is no way
>> >> > touched or modified by this patch set, so yes it is out of scope.
>> >>
>> >> Your patch is relying on the (potentially buggy) behaviour, so I don't
>> >> think it's out of scope to mention it in this context.
>> >
>> > Sorry for slow reply.
>> > I'm swamped with other things atm.
>> >
>> > Looks like there is indeed a bug in prog_type_ext handling code that
>> > is doing
>> > env->ops = bpf_verifier_ops[tgt_prog->type];
>> > I'm not sure whether the verifier can simply add:
>> > prog->expected_attach_type = tgt_prog->expected_attach_type;
>> > and be done with it.
>> > Likely yes, since expected_attach_type must be zero at that point
>> > that is enforced by bpf_prog_load_check_attach().
>> > So I suspect it's a single line fix.
>>
>> Not quite: the check in bpf_tracing_prog_attach() that enforces
>> prog->expected_attach_type==0 also needs to go. So 5 lines :)
>
> prog_ext's expected_attach_type needs to stay zero.
> It needs to be inherited from tgt prog. Hence one line:
> prog->expected_attach_type = tgt_prog->expected_attach_type;

Not sure I follow you here? I ended up with the patch below - without
the first hunk I can't attach freplace funcs to an xdp egress prog
(since the expected_attach_type will have been propagated from
verification time), and so that check will fail. Or am I missing
something?

-Toke



diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d85f37239540..40c3103c7233 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2381,10 +2381,6 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
                }
                break;
        case BPF_PROG_TYPE_EXT:
-               if (prog->expected_attach_type != 0) {
-                       err = -EINVAL;
-                       goto out_put_prog;
-               }
                break;
        case BPF_PROG_TYPE_LSM:
                if (prog->expected_attach_type != BPF_LSM_MAC) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 513d9c545176..41c31773a3c4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10485,6 +10485,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                                return -EINVAL;
                        }
                        env->ops = bpf_verifier_ops[tgt_prog->type];
+                       prog->expected_attach_type = tgt_prog->expected_attach_type;
                }
                if (!tgt_prog->jited) {
                        verbose(env, "Can attach to only JITed progs\n");

Re: [PATCH bpf-next 13/16] bpftool: Add support for XDP egress

[David Ahern]

On 4/23/20 4:43 AM, Quentin Monnet wrote:
> 
> Thanks for the bpftool update. Can you please also update the man pages
> (bpftool-prog, bpftool-net) and bash completion?
> 

will do

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Alexei Starovoitov]

On Thu, Apr 23, 2020 at 07:05:42PM +0200, Toke Høiland-Jørgensen wrote:
> >> >
> >> > Looks like there is indeed a bug in prog_type_ext handling code that
> >> > is doing
> >> > env->ops = bpf_verifier_ops[tgt_prog->type];
> >> > I'm not sure whether the verifier can simply add:
> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
> >> > and be done with it.
> >> > Likely yes, since expected_attach_type must be zero at that point
> >> > that is enforced by bpf_prog_load_check_attach().
> >> > So I suspect it's a single line fix.
> >>
> >> Not quite: the check in bpf_tracing_prog_attach() that enforces
> >> prog->expected_attach_type==0 also needs to go. So 5 lines :)
> >
> > prog_ext's expected_attach_type needs to stay zero.
> > It needs to be inherited from tgt prog. Hence one line:
> > prog->expected_attach_type = tgt_prog->expected_attach_type;
> 
> Not sure I follow you here? I ended up with the patch below - without
> the first hunk I can't attach freplace funcs to an xdp egress prog
> (since the expected_attach_type will have been propagated from
> verification time), and so that check will fail. Or am I missing
> something?
> 
> -Toke
> 
> 
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index d85f37239540..40c3103c7233 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2381,10 +2381,6 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
>                 }
>                 break;
>         case BPF_PROG_TYPE_EXT:
> -               if (prog->expected_attach_type != 0) {
> -                       err = -EINVAL;
> -                       goto out_put_prog;
> -               }
>                 break;

ahh. that extra check.
I think it's better to keep it for extra safety.
Here all expected_attach_type have clear checks depending on prog_type.
There is no other place where it's that obvious.
The verifier does similar thing earlier, but it's not that clear.
I think the better fix would to set expected_attach_type = 0 for PROG_TYPE_EXT
at the end of do_check, since we're overriding this field temporarily
during verification.

>         case BPF_PROG_TYPE_LSM:
>                 if (prog->expected_attach_type != BPF_LSM_MAC) {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 513d9c545176..41c31773a3c4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10485,6 +10485,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
>                                 return -EINVAL;
>                         }
>                         env->ops = bpf_verifier_ops[tgt_prog->type];
> +                       prog->expected_attach_type = tgt_prog->expected_attach_type;

In that sense it's like 'env->expected_attach_type'.

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Thu, Apr 23, 2020 at 07:05:42PM +0200, Toke Høiland-Jørgensen wrote:
>> >> >
>> >> > Looks like there is indeed a bug in prog_type_ext handling code that
>> >> > is doing
>> >> > env->ops = bpf_verifier_ops[tgt_prog->type];
>> >> > I'm not sure whether the verifier can simply add:
>> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
>> >> > and be done with it.
>> >> > Likely yes, since expected_attach_type must be zero at that point
>> >> > that is enforced by bpf_prog_load_check_attach().
>> >> > So I suspect it's a single line fix.
>> >>
>> >> Not quite: the check in bpf_tracing_prog_attach() that enforces
>> >> prog->expected_attach_type==0 also needs to go. So 5 lines :)
>> >
>> > prog_ext's expected_attach_type needs to stay zero.
>> > It needs to be inherited from tgt prog. Hence one line:
>> > prog->expected_attach_type = tgt_prog->expected_attach_type;
>> 
>> Not sure I follow you here? I ended up with the patch below - without
>> the first hunk I can't attach freplace funcs to an xdp egress prog
>> (since the expected_attach_type will have been propagated from
>> verification time), and so that check will fail. Or am I missing
>> something?
>> 
>> -Toke
>> 
>> 
>> 
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index d85f37239540..40c3103c7233 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -2381,10 +2381,6 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
>>                 }
>>                 break;
>>         case BPF_PROG_TYPE_EXT:
>> -               if (prog->expected_attach_type != 0) {
>> -                       err = -EINVAL;
>> -                       goto out_put_prog;
>> -               }
>>                 break;
>
> ahh. that extra check.
> I think it's better to keep it for extra safety.
> Here all expected_attach_type have clear checks depending on prog_type.
> There is no other place where it's that obvious.
> The verifier does similar thing earlier, but it's not that clear.
> I think the better fix would to set expected_attach_type = 0 for PROG_TYPE_EXT
> at the end of do_check, since we're overriding this field temporarily
> during verification.

OK, sure, can do. I do agree it's better to keep the check. I'll send a
proper patch tomorrow, then.

As far as a adding a selftest for this, I think the most natural thing
would be to add it on top of David's tests for xdp_egress, since that's
what hit this - would you be OK with that? And if so, should I send the
main patch straight away and hold off on the selftest, or should I split
them, or hold off on the whole thing?

-Toke

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Alexei Starovoitov]

On Fri, Apr 24, 2020 at 01:49:03AM +0200, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> 
> > On Thu, Apr 23, 2020 at 07:05:42PM +0200, Toke Høiland-Jørgensen wrote:
> >> >> >
> >> >> > Looks like there is indeed a bug in prog_type_ext handling code that
> >> >> > is doing
> >> >> > env->ops = bpf_verifier_ops[tgt_prog->type];
> >> >> > I'm not sure whether the verifier can simply add:
> >> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
> >> >> > and be done with it.
> >> >> > Likely yes, since expected_attach_type must be zero at that point
> >> >> > that is enforced by bpf_prog_load_check_attach().
> >> >> > So I suspect it's a single line fix.
> >> >>
> >> >> Not quite: the check in bpf_tracing_prog_attach() that enforces
> >> >> prog->expected_attach_type==0 also needs to go. So 5 lines :)
> >> >
> >> > prog_ext's expected_attach_type needs to stay zero.
> >> > It needs to be inherited from tgt prog. Hence one line:
> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
> >> 
> >> Not sure I follow you here? I ended up with the patch below - without
> >> the first hunk I can't attach freplace funcs to an xdp egress prog
> >> (since the expected_attach_type will have been propagated from
> >> verification time), and so that check will fail. Or am I missing
> >> something?
> >> 
> >> -Toke
> >> 
> >> 
> >> 
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index d85f37239540..40c3103c7233 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -2381,10 +2381,6 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
> >>                 }
> >>                 break;
> >>         case BPF_PROG_TYPE_EXT:
> >> -               if (prog->expected_attach_type != 0) {
> >> -                       err = -EINVAL;
> >> -                       goto out_put_prog;
> >> -               }
> >>                 break;
> >
> > ahh. that extra check.
> > I think it's better to keep it for extra safety.
> > Here all expected_attach_type have clear checks depending on prog_type.
> > There is no other place where it's that obvious.
> > The verifier does similar thing earlier, but it's not that clear.
> > I think the better fix would to set expected_attach_type = 0 for PROG_TYPE_EXT
> > at the end of do_check, since we're overriding this field temporarily
> > during verification.
> 
> OK, sure, can do. I do agree it's better to keep the check. I'll send a
> proper patch tomorrow, then.
> 
> As far as a adding a selftest for this, I think the most natural thing
> would be to add it on top of David's tests for xdp_egress, since that's
> what hit this - would you be OK with that? And if so, should I send the
> main patch straight away and hold off on the selftest, or should I split
> them, or hold off on the whole thing?

I think the issue is not related to xdp egress.
Hence I'd like to push the fix along with selftest into bpf tree.
The selftest can be:
void noinline do_bind((struct bpf_sock_addr *ctx)
{
  struct sockaddr_in sa = {};

  bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa));
  return 0;
}
SEC("cgroup/connect4")
int connect_v4_prog(struct bpf_sock_addr *ctx)
{
  return do_bind(ctx);
}

and freplace would replace do_bind() with do_new_bind()
that also calls bpf_bind().
I think without the fix freplace will fail to load, because
availability of bpf_bind() depends on correct prog->expected_attach_type.

I haven't looked at the crash you mentioned in the other email related
to xdp egress set. That could be different issue. I hope it's the same thing :)

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[David Ahern]

On 4/23/20 6:53 PM, Alexei Starovoitov wrote:
> 
> I think the issue is not related to xdp egress.

It isn't; that has been my point all along.

> Hence I'd like to push the fix along with selftest into bpf tree.
> The selftest can be:
> void noinline do_bind((struct bpf_sock_addr *ctx)
> {
>   struct sockaddr_in sa = {};
> 
>   bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa));
>   return 0;
> }
> SEC("cgroup/connect4")
> int connect_v4_prog(struct bpf_sock_addr *ctx)
> {
>   return do_bind(ctx);
> }
> 
> and freplace would replace do_bind() with do_new_bind()
> that also calls bpf_bind().
> I think without the fix freplace will fail to load, because
> availability of bpf_bind() depends on correct prog->expected_attach_type.
> 
> I haven't looked at the crash you mentioned in the other email related
> to xdp egress set. That could be different issue. I hope it's the same thing :)
> 

it is. The replaced program is accessing ingress_ifindex from xdp egress
context, and Rx stuff is not set (access is blocked by verifier).

Re: [PATCH bpf-next 04/16] net: Add BPF_XDP_EGRESS as a bpf_attach_type

[Toke Høiland-Jørgensen]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Apr 24, 2020 at 01:49:03AM +0200, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> 
>> > On Thu, Apr 23, 2020 at 07:05:42PM +0200, Toke Høiland-Jørgensen wrote:
>> >> >> >
>> >> >> > Looks like there is indeed a bug in prog_type_ext handling code that
>> >> >> > is doing
>> >> >> > env->ops = bpf_verifier_ops[tgt_prog->type];
>> >> >> > I'm not sure whether the verifier can simply add:
>> >> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
>> >> >> > and be done with it.
>> >> >> > Likely yes, since expected_attach_type must be zero at that point
>> >> >> > that is enforced by bpf_prog_load_check_attach().
>> >> >> > So I suspect it's a single line fix.
>> >> >>
>> >> >> Not quite: the check in bpf_tracing_prog_attach() that enforces
>> >> >> prog->expected_attach_type==0 also needs to go. So 5 lines :)
>> >> >
>> >> > prog_ext's expected_attach_type needs to stay zero.
>> >> > It needs to be inherited from tgt prog. Hence one line:
>> >> > prog->expected_attach_type = tgt_prog->expected_attach_type;
>> >> 
>> >> Not sure I follow you here? I ended up with the patch below - without
>> >> the first hunk I can't attach freplace funcs to an xdp egress prog
>> >> (since the expected_attach_type will have been propagated from
>> >> verification time), and so that check will fail. Or am I missing
>> >> something?
>> >> 
>> >> -Toke
>> >> 
>> >> 
>> >> 
>> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> >> index d85f37239540..40c3103c7233 100644
>> >> --- a/kernel/bpf/syscall.c
>> >> +++ b/kernel/bpf/syscall.c
>> >> @@ -2381,10 +2381,6 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog)
>> >>                 }
>> >>                 break;
>> >>         case BPF_PROG_TYPE_EXT:
>> >> -               if (prog->expected_attach_type != 0) {
>> >> -                       err = -EINVAL;
>> >> -                       goto out_put_prog;
>> >> -               }
>> >>                 break;
>> >
>> > ahh. that extra check.
>> > I think it's better to keep it for extra safety.
>> > Here all expected_attach_type have clear checks depending on prog_type.
>> > There is no other place where it's that obvious.
>> > The verifier does similar thing earlier, but it's not that clear.
>> > I think the better fix would to set expected_attach_type = 0 for PROG_TYPE_EXT
>> > at the end of do_check, since we're overriding this field temporarily
>> > during verification.
>> 
>> OK, sure, can do. I do agree it's better to keep the check. I'll send a
>> proper patch tomorrow, then.
>> 
>> As far as a adding a selftest for this, I think the most natural thing
>> would be to add it on top of David's tests for xdp_egress, since that's
>> what hit this - would you be OK with that? And if so, should I send the
>> main patch straight away and hold off on the selftest, or should I split
>> them, or hold off on the whole thing?
>
> I think the issue is not related to xdp egress.
> Hence I'd like to push the fix along with selftest into bpf tree.
> The selftest can be:
> void noinline do_bind((struct bpf_sock_addr *ctx)
> {
>   struct sockaddr_in sa = {};
>
>   bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa));
>   return 0;
> }
> SEC("cgroup/connect4")
> int connect_v4_prog(struct bpf_sock_addr *ctx)
> {
>   return do_bind(ctx);
> }
>
> and freplace would replace do_bind() with do_new_bind()
> that also calls bpf_bind().
> I think without the fix freplace will fail to load, because
> availability of bpf_bind() depends on correct
> prog->expected_attach_type.

Right, I'll give this a shot, thanks :)

> I haven't looked at the crash you mentioned in the other email related
> to xdp egress set. That could be different issue. I hope it's the same
> thing :)

Yeah, it is.

-Toke