* [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
@ 2025-04-22 21:14 Lorenzo Bianconi
2025-04-22 22:49 ` Stanislav Fomichev
0 siblings, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2025-04-22 21:14 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer
Cc: bpf, netdev, Lorenzo Bianconi
In the current implementation if the program is bounded to a specific
device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
or CPUMAP even if the program is not attached to the map entry. This
seems in contrast with the explanation available in
bpf_prog_map_compatible routine. Fix the issue taking into account
even the attach program type and allow XDP dev bounded program to
perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
or BPF_XDP_CPUMAP.
Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
kernel/bpf/core.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
return 0;
}
+static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
+ const struct bpf_prog *prog)
+{
+ if (!bpf_prog_is_dev_bound(prog->aux))
+ return true;
+
+ if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
+ return false;
+
+ if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
+ prog->expected_attach_type != BPF_XDP_DEVMAP)
+ return true;
+
+ if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
+ prog->expected_attach_type != BPF_XDP_CPUMAP)
+ return true;
+
+ return false;
+}
+
bool bpf_prog_map_compatible(struct bpf_map *map,
const struct bpf_prog *fp)
{
@@ -2373,7 +2393,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
* in the case of devmap and cpumap). Until device checks
* are implemented, prohibit adding dev-bound programs to program maps.
*/
- if (bpf_prog_is_dev_bound(aux))
+ if (!bpf_prog_dev_bound_map_compatible(map, fp))
return false;
spin_lock(&map->owner.lock);
---
base-commit: 5709be4c35ba760b001733939e20069de033a697
change-id: 20250422-xdp-prog-bound-fix-9f30f3e134aa
Best regards,
--
Lorenzo Bianconi <lorenzo@kernel.org>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-22 21:14 [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps Lorenzo Bianconi
@ 2025-04-22 22:49 ` Stanislav Fomichev
2025-04-23 0:03 ` Jakub Kicinski
2025-04-23 10:11 ` Lorenzo Bianconi
0 siblings, 2 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2025-04-22 22:49 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, bpf,
netdev
On 04/22, Lorenzo Bianconi wrote:
> In the current implementation if the program is bounded to a specific
> device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
> or CPUMAP even if the program is not attached to the map entry. This
> seems in contrast with the explanation available in
> bpf_prog_map_compatible routine. Fix the issue taking into account
> even the attach program type and allow XDP dev bounded program to
> perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
> or BPF_XDP_CPUMAP.
>
> Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> kernel/bpf/core.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> return 0;
> }
>
> +static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
> + const struct bpf_prog *prog)
> +{
> + if (!bpf_prog_is_dev_bound(prog->aux))
> + return true;
> +
> + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
> + return false;
[..]
> + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> + prog->expected_attach_type != BPF_XDP_DEVMAP)
> + return true;
> +
> + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> + prog->expected_attach_type != BPF_XDP_CPUMAP)
> + return true;
Not sure I understand, what does it mean exactly? That it's ok to add
a dev-bound program to the dev/cpumap if the program itself is gonna
be attached only to the real device? Can you expand more on the specific
use-case?
The existing check makes sure that the dev-bound programs run only in the
contexts that have hw descriptors. devmap and cpumap don't satisfy
this constraint afaiu.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-22 22:49 ` Stanislav Fomichev
@ 2025-04-23 0:03 ` Jakub Kicinski
2025-04-23 10:11 ` Lorenzo Bianconi
1 sibling, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2025-04-23 0:03 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Lorenzo Bianconi, Alexei Starovoitov, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
Stanislav Fomichev, Hao Luo, Jiri Olsa, David S. Miller,
Jesper Dangaard Brouer, bpf, netdev
On Tue, 22 Apr 2025 15:49:52 -0700 Stanislav Fomichev wrote:
> > + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> > + prog->expected_attach_type != BPF_XDP_DEVMAP)
> > + return true;
> > +
> > + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> > + prog->expected_attach_type != BPF_XDP_CPUMAP)
> > + return true;
>
> Not sure I understand, what does it mean exactly? That it's ok to add
> a dev-bound program to the dev/cpumap if the program itself is gonna
> be attached only to the real device? Can you expand more on the specific
> use-case?
And an upstream offload which supports it..
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-22 22:49 ` Stanislav Fomichev
2025-04-23 0:03 ` Jakub Kicinski
@ 2025-04-23 10:11 ` Lorenzo Bianconi
2025-04-23 14:33 ` Stanislav Fomichev
1 sibling, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2025-04-23 10:11 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, bpf,
netdev
[-- Attachment #1: Type: text/plain, Size: 4077 bytes --]
On Apr 22, Stanislav Fomichev wrote:
> On 04/22, Lorenzo Bianconi wrote:
> > In the current implementation if the program is bounded to a specific
> > device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
> > or CPUMAP even if the program is not attached to the map entry. This
> > seems in contrast with the explanation available in
> > bpf_prog_map_compatible routine. Fix the issue taking into account
> > even the attach program type and allow XDP dev bounded program to
> > perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
> > or BPF_XDP_CPUMAP.
> >
> > Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > kernel/bpf/core.c | 22 +++++++++++++++++++++-
> > 1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> > return 0;
> > }
> >
> > +static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
> > + const struct bpf_prog *prog)
> > +{
> > + if (!bpf_prog_is_dev_bound(prog->aux))
> > + return true;
> > +
> > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
> > + return false;
>
> [..]
>
> > + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> > + prog->expected_attach_type != BPF_XDP_DEVMAP)
> > + return true;
> > +
> > + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> > + prog->expected_attach_type != BPF_XDP_CPUMAP)
> > + return true;
>
> Not sure I understand, what does it mean exactly? That it's ok to add
> a dev-bound program to the dev/cpumap if the program itself is gonna
> be attached only to the real device? Can you expand more on the specific
> use-case?
>
> The existing check makes sure that the dev-bound programs run only in the
> contexts that have hw descriptors. devmap and cpumap don't satisfy
> this constraint afaiu.
My use-case is to use a hw-metadata kfunc like bpf_xdp_metadata_rx_timestamp()
to read hw timestamp from the NIC and then redirect the xdp_buff into a DEVMP
(please note there are no programs attached to any DEVMAP entries):
extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
__u64 *timestamp) __ksym;
struct {
__uint(type, BPF_MAP_TYPE_DEVMAP);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(struct bpf_devmap_val));
__uint(max_entries, 1);
} dev_map SEC(".maps");
SEC("xdp")
int xdp_meta_redirect(struct xdp_md *ctx)
{
__u64 timestamp;
...
bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
...
return bpf_redirect_map(&dev_map, ctx->rx_queue_index, XDP_PASS);
}
According to my understanding this is feasible just if the "xdp_meta_redirect"
program is bounded to a device otherwise the program is reject with the following
error at load time:
libbpf: prog 'xdp_meta_redirect': BPF program load failed: -EINVAL
libbpf: prog 'xdp_meta_redirect': -- BEGIN PROG LOAD LOG --
metadata kfuncs require device-bound program
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
-- END PROG LOAD LOG --
in order to fix it:
...
index = if_nametoindex(DEV);
bpf_program__set_ifindex(prog, index);
bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
...
Doing so the program load still fails for the check in bpf_prog_map_compatible():
bool bpf_prog_map_compatible()
{
...
if (bpf_prog_is_dev_bound(aux))
return false;
...
In other words, a dev-bound XDP program can't interact with a DEVMAP (or
CPUMAP) even if it is not attached to a map entry.
I think if the XDP program is just running in the driver NAPI context
it should be doable to use a hw-metada kfunc and perform a redirect into
a DEVMAP or CPUMAP, right? Am I missing something?
Regards,
Lorenzo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-23 10:11 ` Lorenzo Bianconi
@ 2025-04-23 14:33 ` Stanislav Fomichev
2025-04-23 14:54 ` Lorenzo Bianconi
0 siblings, 1 reply; 7+ messages in thread
From: Stanislav Fomichev @ 2025-04-23 14:33 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, bpf,
netdev
On 04/23, Lorenzo Bianconi wrote:
> On Apr 22, Stanislav Fomichev wrote:
> > On 04/22, Lorenzo Bianconi wrote:
> > > In the current implementation if the program is bounded to a specific
> > > device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
> > > or CPUMAP even if the program is not attached to the map entry. This
> > > seems in contrast with the explanation available in
> > > bpf_prog_map_compatible routine. Fix the issue taking into account
> > > even the attach program type and allow XDP dev bounded program to
> > > perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
> > > or BPF_XDP_CPUMAP.
> > >
> > > Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > kernel/bpf/core.c | 22 +++++++++++++++++++++-
> > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
> > > --- a/kernel/bpf/core.c
> > > +++ b/kernel/bpf/core.c
> > > @@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> > > return 0;
> > > }
> > >
> > > +static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
> > > + const struct bpf_prog *prog)
> > > +{
> > > + if (!bpf_prog_is_dev_bound(prog->aux))
> > > + return true;
> > > +
> > > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
> > > + return false;
> >
> > [..]
> >
> > > + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> > > + prog->expected_attach_type != BPF_XDP_DEVMAP)
> > > + return true;
> > > +
> > > + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> > > + prog->expected_attach_type != BPF_XDP_CPUMAP)
> > > + return true;
> >
> > Not sure I understand, what does it mean exactly? That it's ok to add
> > a dev-bound program to the dev/cpumap if the program itself is gonna
> > be attached only to the real device? Can you expand more on the specific
> > use-case?
> >
> > The existing check makes sure that the dev-bound programs run only in the
> > contexts that have hw descriptors. devmap and cpumap don't satisfy
> > this constraint afaiu.
>
> My use-case is to use a hw-metadata kfunc like bpf_xdp_metadata_rx_timestamp()
> to read hw timestamp from the NIC and then redirect the xdp_buff into a DEVMP
> (please note there are no programs attached to any DEVMAP entries):
>
> extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
> __u64 *timestamp) __ksym;
>
> struct {
> __uint(type, BPF_MAP_TYPE_DEVMAP);
> __uint(key_size, sizeof(__u32));
> __uint(value_size, sizeof(struct bpf_devmap_val));
> __uint(max_entries, 1);
> } dev_map SEC(".maps");
>
> SEC("xdp")
> int xdp_meta_redirect(struct xdp_md *ctx)
> {
> __u64 timestamp;
>
> ...
> bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
> ...
>
> return bpf_redirect_map(&dev_map, ctx->rx_queue_index, XDP_PASS);
> }
>
> According to my understanding this is feasible just if the "xdp_meta_redirect"
> program is bounded to a device otherwise the program is reject with the following
> error at load time:
>
> libbpf: prog 'xdp_meta_redirect': BPF program load failed: -EINVAL
> libbpf: prog 'xdp_meta_redirect': -- BEGIN PROG LOAD LOG --
> metadata kfuncs require device-bound program
> processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0
> -- END PROG LOAD LOG --
>
> in order to fix it:
>
> ...
> index = if_nametoindex(DEV);
> bpf_program__set_ifindex(prog, index);
> bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
> ...
>
> Doing so the program load still fails for the check in bpf_prog_map_compatible():
>
> bool bpf_prog_map_compatible()
> {
> ...
> if (bpf_prog_is_dev_bound(aux))
> return false;
> ...
[..]
> In other words, a dev-bound XDP program can't interact with a DEVMAP (or
> CPUMAP) even if it is not attached to a map entry.
> I think if the XDP program is just running in the driver NAPI context
> it should be doable to use a hw-metada kfunc and perform a redirect into
> a DEVMAP or CPUMAP, right? Am I missing something?
Thanks for the info! Yes, that should work. I wonder if you hit
bpf_prog_select_runtime->bpf_check_tail_call->bpf_prog_map_compatible
path? Looks like we should not do bpf_prog_is_dev_bound in that case (the rest
of the bpf_prog_map_compatible callers should).
When doing a follow up, can you also extend tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
to cover these conditions? (redirect to empty map -> nop, adding
dev-bound program to devmap is einval).
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-23 14:33 ` Stanislav Fomichev
@ 2025-04-23 14:54 ` Lorenzo Bianconi
2025-04-23 15:09 ` Stanislav Fomichev
0 siblings, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2025-04-23 14:54 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, bpf,
netdev
[-- Attachment #1: Type: text/plain, Size: 5577 bytes --]
> On 04/23, Lorenzo Bianconi wrote:
> > On Apr 22, Stanislav Fomichev wrote:
> > > On 04/22, Lorenzo Bianconi wrote:
> > > > In the current implementation if the program is bounded to a specific
> > > > device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
> > > > or CPUMAP even if the program is not attached to the map entry. This
> > > > seems in contrast with the explanation available in
> > > > bpf_prog_map_compatible routine. Fix the issue taking into account
> > > > even the attach program type and allow XDP dev bounded program to
> > > > perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
> > > > or BPF_XDP_CPUMAP.
> > > >
> > > > Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > kernel/bpf/core.c | 22 +++++++++++++++++++++-
> > > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > > index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
> > > > --- a/kernel/bpf/core.c
> > > > +++ b/kernel/bpf/core.c
> > > > @@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> > > > return 0;
> > > > }
> > > >
> > > > +static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
> > > > + const struct bpf_prog *prog)
> > > > +{
> > > > + if (!bpf_prog_is_dev_bound(prog->aux))
> > > > + return true;
> > > > +
> > > > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
> > > > + return false;
> > >
> > > [..]
> > >
> > > > + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> > > > + prog->expected_attach_type != BPF_XDP_DEVMAP)
> > > > + return true;
> > > > +
> > > > + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> > > > + prog->expected_attach_type != BPF_XDP_CPUMAP)
> > > > + return true;
> > >
> > > Not sure I understand, what does it mean exactly? That it's ok to add
> > > a dev-bound program to the dev/cpumap if the program itself is gonna
> > > be attached only to the real device? Can you expand more on the specific
> > > use-case?
> > >
> > > The existing check makes sure that the dev-bound programs run only in the
> > > contexts that have hw descriptors. devmap and cpumap don't satisfy
> > > this constraint afaiu.
> >
> > My use-case is to use a hw-metadata kfunc like bpf_xdp_metadata_rx_timestamp()
> > to read hw timestamp from the NIC and then redirect the xdp_buff into a DEVMP
> > (please note there are no programs attached to any DEVMAP entries):
> >
> > extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
> > __u64 *timestamp) __ksym;
> >
> > struct {
> > __uint(type, BPF_MAP_TYPE_DEVMAP);
> > __uint(key_size, sizeof(__u32));
> > __uint(value_size, sizeof(struct bpf_devmap_val));
> > __uint(max_entries, 1);
> > } dev_map SEC(".maps");
> >
> > SEC("xdp")
> > int xdp_meta_redirect(struct xdp_md *ctx)
> > {
> > __u64 timestamp;
> >
> > ...
> > bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
> > ...
> >
> > return bpf_redirect_map(&dev_map, ctx->rx_queue_index, XDP_PASS);
> > }
> >
> > According to my understanding this is feasible just if the "xdp_meta_redirect"
> > program is bounded to a device otherwise the program is reject with the following
> > error at load time:
> >
> > libbpf: prog 'xdp_meta_redirect': BPF program load failed: -EINVAL
> > libbpf: prog 'xdp_meta_redirect': -- BEGIN PROG LOAD LOG --
> > metadata kfuncs require device-bound program
> > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
> > -- END PROG LOAD LOG --
> >
> > in order to fix it:
> >
> > ...
> > index = if_nametoindex(DEV);
> > bpf_program__set_ifindex(prog, index);
> > bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
> > ...
> >
> > Doing so the program load still fails for the check in bpf_prog_map_compatible():
> >
> > bool bpf_prog_map_compatible()
> > {
> > ...
> > if (bpf_prog_is_dev_bound(aux))
> > return false;
> > ...
>
> [..]
>
> > In other words, a dev-bound XDP program can't interact with a DEVMAP (or
> > CPUMAP) even if it is not attached to a map entry.
> > I think if the XDP program is just running in the driver NAPI context
> > it should be doable to use a hw-metada kfunc and perform a redirect into
> > a DEVMAP or CPUMAP, right? Am I missing something?
>
> Thanks for the info! Yes, that should work. I wonder if you hit
> bpf_prog_select_runtime->bpf_check_tail_call->bpf_prog_map_compatible
> path? Looks like we should not do bpf_prog_is_dev_bound in that case (the rest
> of the bpf_prog_map_compatible callers should).
yes, the issue occurs at the program load time when we run
bpf_prog_map_compatible() following the call path you pointed out:
bpf_prog_select_runtime() -> bpf_check_tail_call() -> bpf_prog_map_compatible()
Do you mean we should get rid of the bpf_prog_is_dev_bound() check in
bpf_prog_map_compatible() and move it in the bpf_prog_map_compatible() callers
instead? In particular:
- __cpu_map_load_bpf_program()
- __dev_map_alloc_node()
- prog_fd_array_get_ptr()
>
> When doing a follow up, can you also extend tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> to cover these conditions? (redirect to empty map -> nop, adding
> dev-bound program to devmap is einval).
ack, I will do it in v2.
Regards,
Lorenzo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps
2025-04-23 14:54 ` Lorenzo Bianconi
@ 2025-04-23 15:09 ` Stanislav Fomichev
0 siblings, 0 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2025-04-23 15:09 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, bpf,
netdev
On 04/23, Lorenzo Bianconi wrote:
> > On 04/23, Lorenzo Bianconi wrote:
> > > On Apr 22, Stanislav Fomichev wrote:
> > > > On 04/22, Lorenzo Bianconi wrote:
> > > > > In the current implementation if the program is bounded to a specific
> > > > > device, it will not be possible to perform XDP_REDIRECT into a DEVMAP
> > > > > or CPUMAP even if the program is not attached to the map entry. This
> > > > > seems in contrast with the explanation available in
> > > > > bpf_prog_map_compatible routine. Fix the issue taking into account
> > > > > even the attach program type and allow XDP dev bounded program to
> > > > > perform XDP_REDIRECT into maps if the attach type is not BPF_XDP_DEVMAP
> > > > > or BPF_XDP_CPUMAP.
> > > > >
> > > > > Fixes: 3d76a4d3d4e59 ("bpf: XDP metadata RX kfuncs")
> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > ---
> > > > > kernel/bpf/core.c | 22 +++++++++++++++++++++-
> > > > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > > > index ba6b6118cf504041278d05417c4212d57be6fca0..a33175efffc377edbfe281397017eb467bfbcce9 100644
> > > > > --- a/kernel/bpf/core.c
> > > > > +++ b/kernel/bpf/core.c
> > > > > @@ -2358,6 +2358,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static bool bpf_prog_dev_bound_map_compatible(struct bpf_map *map,
> > > > > + const struct bpf_prog *prog)
> > > > > +{
> > > > > + if (!bpf_prog_is_dev_bound(prog->aux))
> > > > > + return true;
> > > > > +
> > > > > + if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY)
> > > > > + return false;
> > > >
> > > > [..]
> > > >
> > > > > + if (map->map_type == BPF_MAP_TYPE_DEVMAP &&
> > > > > + prog->expected_attach_type != BPF_XDP_DEVMAP)
> > > > > + return true;
> > > > > +
> > > > > + if (map->map_type == BPF_MAP_TYPE_CPUMAP &&
> > > > > + prog->expected_attach_type != BPF_XDP_CPUMAP)
> > > > > + return true;
> > > >
> > > > Not sure I understand, what does it mean exactly? That it's ok to add
> > > > a dev-bound program to the dev/cpumap if the program itself is gonna
> > > > be attached only to the real device? Can you expand more on the specific
> > > > use-case?
> > > >
> > > > The existing check makes sure that the dev-bound programs run only in the
> > > > contexts that have hw descriptors. devmap and cpumap don't satisfy
> > > > this constraint afaiu.
> > >
> > > My use-case is to use a hw-metadata kfunc like bpf_xdp_metadata_rx_timestamp()
> > > to read hw timestamp from the NIC and then redirect the xdp_buff into a DEVMP
> > > (please note there are no programs attached to any DEVMAP entries):
> > >
> > > extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx,
> > > __u64 *timestamp) __ksym;
> > >
> > > struct {
> > > __uint(type, BPF_MAP_TYPE_DEVMAP);
> > > __uint(key_size, sizeof(__u32));
> > > __uint(value_size, sizeof(struct bpf_devmap_val));
> > > __uint(max_entries, 1);
> > > } dev_map SEC(".maps");
> > >
> > > SEC("xdp")
> > > int xdp_meta_redirect(struct xdp_md *ctx)
> > > {
> > > __u64 timestamp;
> > >
> > > ...
> > > bpf_xdp_metadata_rx_timestamp(ctx, ×tamp);
> > > ...
> > >
> > > return bpf_redirect_map(&dev_map, ctx->rx_queue_index, XDP_PASS);
> > > }
> > >
> > > According to my understanding this is feasible just if the "xdp_meta_redirect"
> > > program is bounded to a device otherwise the program is reject with the following
> > > error at load time:
> > >
> > > libbpf: prog 'xdp_meta_redirect': BPF program load failed: -EINVAL
> > > libbpf: prog 'xdp_meta_redirect': -- BEGIN PROG LOAD LOG --
> > > metadata kfuncs require device-bound program
> > > processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > > peak_states 0 mark_read 0
> > > -- END PROG LOAD LOG --
> > >
> > > in order to fix it:
> > >
> > > ...
> > > index = if_nametoindex(DEV);
> > > bpf_program__set_ifindex(prog, index);
> > > bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
> > > ...
> > >
> > > Doing so the program load still fails for the check in bpf_prog_map_compatible():
> > >
> > > bool bpf_prog_map_compatible()
> > > {
> > > ...
> > > if (bpf_prog_is_dev_bound(aux))
> > > return false;
> > > ...
> >
> > [..]
> >
> > > In other words, a dev-bound XDP program can't interact with a DEVMAP (or
> > > CPUMAP) even if it is not attached to a map entry.
> > > I think if the XDP program is just running in the driver NAPI context
> > > it should be doable to use a hw-metada kfunc and perform a redirect into
> > > a DEVMAP or CPUMAP, right? Am I missing something?
> >
> > Thanks for the info! Yes, that should work. I wonder if you hit
> > bpf_prog_select_runtime->bpf_check_tail_call->bpf_prog_map_compatible
> > path? Looks like we should not do bpf_prog_is_dev_bound in that case (the rest
> > of the bpf_prog_map_compatible callers should).
>
> yes, the issue occurs at the program load time when we run
> bpf_prog_map_compatible() following the call path you pointed out:
>
> bpf_prog_select_runtime() -> bpf_check_tail_call() -> bpf_prog_map_compatible()
>
> Do you mean we should get rid of the bpf_prog_is_dev_bound() check in
> bpf_prog_map_compatible() and move it in the bpf_prog_map_compatible() callers
> instead? In particular:
>
> - __cpu_map_load_bpf_program()
> - __dev_map_alloc_node()
> - prog_fd_array_get_ptr()
Maybe move existing bpf_prog_map_compatible parts (except is_dev_bound)
into some new bpf_prog_map_compatible_type helper?
bpf_prog_map_compatible() {
if (bpf_prog_is_dev_bound)
return false;
return bpf_prog_map_compatible_type(...);
}
And make bpf_check_tail_call call new bpf_prog_map_compatible_type. Not
sure about the naming though (as usual)..
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-23 15:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-22 21:14 [PATCH bpf-next] bpf: Allow XDP dev bounded program to perform XDP_REDIRECT into maps Lorenzo Bianconi
2025-04-22 22:49 ` Stanislav Fomichev
2025-04-23 0:03 ` Jakub Kicinski
2025-04-23 10:11 ` Lorenzo Bianconi
2025-04-23 14:33 ` Stanislav Fomichev
2025-04-23 14:54 ` Lorenzo Bianconi
2025-04-23 15:09 ` Stanislav Fomichev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).