diff mbox series

[bpf-next,6/7] bpf: devmap: check XDP features in bpf_map_update_elem and __xdp_enqueue

Message ID acc9460e6e29dfe02cf474735277e196b500d2ef.1674234430.git.lorenzo@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series xdp: introduce xdp-feature support | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 23 this patch: 23
netdev/cc_maintainers warning 8 maintainers not CCed: kpsingh@kernel.org haoluo@google.com song@kernel.org yhs@fb.com martin.lau@linux.dev sdf@google.com john.fastabend@gmail.com jolsa@kernel.org
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 23 this patch: 23
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 76 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 fail Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 fail Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-8 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-9 success Logs for set-matrix

Commit Message

Lorenzo Bianconi Jan. 20, 2023, 5:16 p.m. UTC 
When we update devmap element, the net_device whose ifindex is specified
in map value must support ndo_xdp_xmit callback, which is indicated by
the presence of XDP_F_REDIRECT_TARGET feature. Let's check for
this feature and return an error if device cannot be used as a redirect
target.

Moreover check the device support xdp non-linear frame in __xdp_enqueue
and is_valid_dst routines. This patch allows to perfrom XDP_REDIRECT on
non-linear xdp buffers.

Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 kernel/bpf/devmap.c | 25 +++++++++++++++++++++----
 net/core/filter.c   | 13 +++++--------
 2 files changed, 26 insertions(+), 12 deletions(-)

Comments

Martin KaFai Lau Jan. 21, 2023, 2:02 a.m. UTC | #1 
On 1/20/23 9:16 AM, Lorenzo Bianconi wrote:
> ---
>   kernel/bpf/devmap.c | 25 +++++++++++++++++++++----
>   net/core/filter.c   | 13 +++++--------
>   2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index d01e4c55b376..69ceecc792df 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -474,7 +474,11 @@ static inline int __xdp_enqueue(struct net_device *dev, struct xdp_frame *xdpf,
>   {
>   	int err;
>   
> -	if (!dev->netdev_ops->ndo_xdp_xmit)
> +	if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))

The current "dev->netdev_ops->ndo_xdp_xmit" check is self explaining.
Any plan to put some document for the NETDEV_XDP_ACT_* values?
Lorenzo Bianconi Jan. 22, 2023, 12:13 p.m. UTC | #2 
> On 1/20/23 9:16 AM, Lorenzo Bianconi wrote:
> > ---
> >   kernel/bpf/devmap.c | 25 +++++++++++++++++++++----
> >   net/core/filter.c   | 13 +++++--------
> >   2 files changed, 26 insertions(+), 12 deletions(-)
> > 
> > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> > index d01e4c55b376..69ceecc792df 100644
> > --- a/kernel/bpf/devmap.c
> > +++ b/kernel/bpf/devmap.c
> > @@ -474,7 +474,11 @@ static inline int __xdp_enqueue(struct net_device *dev, struct xdp_frame *xdpf,
> >   {
> >   	int err;
> > -	if (!dev->netdev_ops->ndo_xdp_xmit)
> > +	if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))
> 
> The current "dev->netdev_ops->ndo_xdp_xmit" check is self explaining.
> Any plan to put some document for the NETDEV_XDP_ACT_* values?
> 

I am not a yaml description expert but I guess we can xdp features description
in Documentation/netlink/specs/netdev.yaml.

@Jakub: what do you think?

Regards,
Lorenzo
Jakub Kicinski Jan. 23, 2023, 8:09 p.m. UTC | #3 
On Sun, 22 Jan 2023 13:13:41 +0100 Lorenzo Bianconi wrote:
> > > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> > > index d01e4c55b376..69ceecc792df 100644
> > > --- a/kernel/bpf/devmap.c
> > > +++ b/kernel/bpf/devmap.c
> > > @@ -474,7 +474,11 @@ static inline int __xdp_enqueue(struct net_device *dev, struct xdp_frame *xdpf,
> > >   {
> > >   	int err;
> > > -	if (!dev->netdev_ops->ndo_xdp_xmit)
> > > +	if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))  
> > 
> > The current "dev->netdev_ops->ndo_xdp_xmit" check is self explaining.
> > Any plan to put some document for the NETDEV_XDP_ACT_* values?
> >   
> 
> I am not a yaml description expert but I guess we can xdp features description
> in Documentation/netlink/specs/netdev.yaml.
> 
> @Jakub: what do you think?

I've added the ability to document enums recently, so you may need
to rebase. But it should work and render the documentation as kdoc 
in the uAPI header (hopefully in a not-too-ugly way).

Example of YAML:
https://github.com/kuba-moo/ynl/blob/dpll/Documentation/netlink/specs/dpll.yaml#L27-L46

I've also talked to the iproute2-py maintainer about generating
documentation directly from YAML to Sphinx/htmldocs, hopefully 
that will happen, too. It would be good to have a few families 
to work with before we start that work, tho.
Lorenzo Bianconi Jan. 23, 2023, 11:29 p.m. UTC | #4 
On Jan 23, Jakub Kicinski wrote:
> On Sun, 22 Jan 2023 13:13:41 +0100 Lorenzo Bianconi wrote:
> > > > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> > > > index d01e4c55b376..69ceecc792df 100644
> > > > --- a/kernel/bpf/devmap.c
> > > > +++ b/kernel/bpf/devmap.c
> > > > @@ -474,7 +474,11 @@ static inline int __xdp_enqueue(struct net_device *dev, struct xdp_frame *xdpf,
> > > >   {
> > > >   	int err;
> > > > -	if (!dev->netdev_ops->ndo_xdp_xmit)
> > > > +	if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))  
> > > 
> > > The current "dev->netdev_ops->ndo_xdp_xmit" check is self explaining.
> > > Any plan to put some document for the NETDEV_XDP_ACT_* values?
> > >   
> > 
> > I am not a yaml description expert but I guess we can xdp features description
> > in Documentation/netlink/specs/netdev.yaml.
> > 
> > @Jakub: what do you think?
> 
> I've added the ability to document enums recently, so you may need
> to rebase. But it should work and render the documentation as kdoc 
> in the uAPI header (hopefully in a not-too-ugly way).
> 
> Example of YAML:
> https://github.com/kuba-moo/ynl/blob/dpll/Documentation/netlink/specs/dpll.yaml#L27-L46

ack, it works properly I guess, I got the following kdoc in the uAPI:

/**
 * enum netdev_xdp_act
 * @NETDEV_XDP_ACT_BASIC: XDP feautues set supported by all drivers
 *   (XDP_ABORTED, XDP_DROP, XDP_PASS, XDP_TX)
 * @NETDEV_XDP_ACT_REDIRECT: The netdev supports XDP_REDIRECT
 * @NETDEV_XDP_ACT_NDO_XMIT: This feature informs if netdev implements
 *   ndo_xdp_xmit callback.
 * @NETDEV_XDP_ACT_XSK_ZEROCOPY: This feature informs if netdev supports AF_XDP
 *   in zero copy mode.
 * @NETDEV_XDP_ACT_HW_OFFLOAD: This feature informs if netdev supports XDP hw
 *   oflloading.
 * @NETDEV_XDP_ACT_RX_SG: This feature informs if netdev implements non-linear
 *   XDP buffer support in the driver napi callback.
 * @NETDEV_XDP_ACT_NDO_XMIT_SG: This feature informs if netdev implements
 *   non-linear XDP buffer support in ndo_xdp_xmit callback.
 */
enum netdev_xdp_act {
        NETDEV_XDP_ACT_BASIC,
        NETDEV_XDP_ACT_REDIRECT,
        NETDEV_XDP_ACT_NDO_XMIT,
        NETDEV_XDP_ACT_XSK_ZEROCOPY,
        NETDEV_XDP_ACT_HW_OFFLOAD,
        NETDEV_XDP_ACT_RX_SG,
        NETDEV_XDP_ACT_NDO_XMIT_SG,
};

Regards,
Lorenzo

> 
> I've also talked to the iproute2-py maintainer about generating
> documentation directly from YAML to Sphinx/htmldocs, hopefully 
> that will happen, too. It would be good to have a few families 
> to work with before we start that work, tho.
diff mbox series

Patch

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index d01e4c55b376..69ceecc792df 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -474,7 +474,11 @@  static inline int __xdp_enqueue(struct net_device *dev, struct xdp_frame *xdpf,
 {
 	int err;
 
-	if (!dev->netdev_ops->ndo_xdp_xmit)
+	if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))
+		return -EOPNOTSUPP;
+
+	if (unlikely(!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT_SG) &&
+		     xdp_frame_has_frags(xdpf)))
 		return -EOPNOTSUPP;
 
 	err = xdp_ok_fwd_dev(dev, xdp_get_frame_len(xdpf));
@@ -532,8 +536,14 @@  int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_frame *xdpf,
 
 static bool is_valid_dst(struct bpf_dtab_netdev *obj, struct xdp_frame *xdpf)
 {
-	if (!obj ||
-	    !obj->dev->netdev_ops->ndo_xdp_xmit)
+	if (!obj)
+		return false;
+
+	if (!(obj->dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))
+		return false;
+
+	if (unlikely(!(obj->dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT_SG) &&
+		     xdp_frame_has_frags(xdpf)))
 		return false;
 
 	if (xdp_ok_fwd_dev(obj->dev, xdp_get_frame_len(xdpf)))
@@ -843,6 +853,7 @@  static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 {
 	struct bpf_prog *prog = NULL;
 	struct bpf_dtab_netdev *dev;
+	int ret = -EINVAL;
 
 	dev = bpf_map_kmalloc_node(&dtab->map, sizeof(*dev),
 				   GFP_NOWAIT | __GFP_NOWARN,
@@ -854,6 +865,12 @@  static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 	if (!dev->dev)
 		goto err_out;
 
+	/* Check if net_device can be used as a redirect target */
+	if (!(READ_ONCE(dev->dev->xdp_features) & NETDEV_XDP_ACT_NDO_XMIT)) {
+		ret = -EOPNOTSUPP;
+		goto err_put_dev;
+	}
+
 	if (val->bpf_prog.fd > 0) {
 		prog = bpf_prog_get_type_dev(val->bpf_prog.fd,
 					     BPF_PROG_TYPE_XDP, false);
@@ -882,7 +899,7 @@  static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 	dev_put(dev->dev);
 err_out:
 	kfree(dev);
-	return ERR_PTR(-EINVAL);
+	return ERR_PTR(ret);
 }
 
 static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
diff --git a/net/core/filter.c b/net/core/filter.c
index b4547a2c02f4..c1e12e8502e9 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4314,16 +4314,13 @@  int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	enum bpf_map_type map_type = ri->map_type;
 
-	/* XDP_REDIRECT is not fully supported yet for xdp frags since
-	 * not all XDP capable drivers can map non-linear xdp_frame in
-	 * ndo_xdp_xmit.
-	 */
-	if (unlikely(xdp_buff_has_frags(xdp) &&
-		     map_type != BPF_MAP_TYPE_CPUMAP))
-		return -EOPNOTSUPP;
+	if (map_type == BPF_MAP_TYPE_XSKMAP) {
+		/* XDP_REDIRECT is not supported AF_XDP yet. */
+		if (unlikely(xdp_buff_has_frags(xdp)))
+			return -EOPNOTSUPP;
 
-	if (map_type == BPF_MAP_TYPE_XSKMAP)
 		return __xdp_do_redirect_xsk(ri, dev, xdp, xdp_prog);
+	}
 
 	return __xdp_do_redirect_frame(ri, dev, xdp_convert_buff_to_frame(xdp),
 				       xdp_prog);