| Message ID | 155966185069.9084.1926498690478259792.stgit@alrua-x1 |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | xdp: Allow lookup into devmaps before redirect | expand |
On Tue, 04 Jun 2019 17:24:10 +0200 Toke Høiland-Jørgensen <toke@redhat.com> wrote: > The bpf_redirect_map() helper used by XDP programs doesn't return any > indication of whether it can successfully redirect to the map index it was > given. Instead, BPF programs have to track this themselves, leading to > programs using duplicate maps to track which entries are populated in the > devmap. > > This adds a flag to the XDP version of the bpf_redirect_map() helper, which > makes the helper do a lookup in the map when called, and return XDP_PASS if > there is no value at the provided index. This enables two use cases: To Jonathan Lemon, notice this approach of adding a flag to the helper call, it actually also works for your use-case of XSK AF_XDP maps. > - A BPF program can check the return code from the helper call and react if > it is XDP_PASS (by, for instance, redirecting out a different interface). > > - Programs that just return the value of the bpf_redirect() call will > automatically fall back to the regular networking stack, simplifying > programs that (for instance) build a router with the fib_lookup() helper. > > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > include/uapi/linux/bpf.h | 8 ++++++++ > net/core/filter.c | 10 +++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 7c6aef253173..4c41482b7604 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -3098,6 +3098,14 @@ enum xdp_action { > XDP_REDIRECT, > }; > > +/* Flags for bpf_xdp_redirect_map helper */ > + > +/* If set, the help will check if the entry exists in the map and return > + * XDP_PASS if it doesn't. > + */ > +#define XDP_REDIRECT_PASS_ON_INVALID BIT(0) > +#define XDP_REDIRECT_ALL_FLAGS XDP_REDIRECT_PASS_ON_INVALID > + > /* user accessible metadata for XDP packet hook > * new fields must be added to the end of this structure > */ > diff --git a/net/core/filter.c b/net/core/filter.c > index 55bfc941d17a..dfab8478f66c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -3755,9 +3755,17 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, > { > struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info); > > - if (unlikely(flags)) > + if (unlikely(flags & ~XDP_REDIRECT_ALL_FLAGS)) > return XDP_ABORTED; > > + if (flags & XDP_REDIRECT_PASS_ON_INVALID) { > + struct net_device *fwd; It is slightly misguiding that '*fwd' is a 'struct net_device', as the __xdp_map_lookup_elem() call works for all the supported redirect-map types. People should realize that this patch is a general approach for all the redirect-map types. > + > + fwd = __xdp_map_lookup_elem(map, ifindex); > + if (unlikely(!fwd)) > + return XDP_PASS; > + } > + > ri->ifindex = ifindex; > ri->flags = flags; > WRITE_ONCE(ri->map, map);
On 5 Jun 2019, at 3:39, Jesper Dangaard Brouer wrote: > On Tue, 04 Jun 2019 17:24:10 +0200 > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> The bpf_redirect_map() helper used by XDP programs doesn't return any >> indication of whether it can successfully redirect to the map index >> it was >> given. Instead, BPF programs have to track this themselves, leading >> to >> programs using duplicate maps to track which entries are populated in >> the >> devmap. >> >> This adds a flag to the XDP version of the bpf_redirect_map() helper, >> which >> makes the helper do a lookup in the map when called, and return >> XDP_PASS if >> there is no value at the provided index. This enables two use cases: > > To Jonathan Lemon, notice this approach of adding a flag to the helper > call, it actually also works for your use-case of XSK AF_XDP maps.c Hmm, yes, that should work also. I have a patch which returns a XDP_SOCK type from the xskmap. This could be used with a new helper for redirection directly to a socket (instead of looking the socket up a second time). While flexible, the downside is that this won't apply to devmaps.
Jesper Dangaard Brouer <brouer@redhat.com> writes: > On Tue, 04 Jun 2019 17:24:10 +0200 > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> The bpf_redirect_map() helper used by XDP programs doesn't return any >> indication of whether it can successfully redirect to the map index it was >> given. Instead, BPF programs have to track this themselves, leading to >> programs using duplicate maps to track which entries are populated in the >> devmap. >> >> This adds a flag to the XDP version of the bpf_redirect_map() helper, which >> makes the helper do a lookup in the map when called, and return XDP_PASS if >> there is no value at the provided index. This enables two use cases: > > To Jonathan Lemon, notice this approach of adding a flag to the helper > call, it actually also works for your use-case of XSK AF_XDP maps. > >> - A BPF program can check the return code from the helper call and react if >> it is XDP_PASS (by, for instance, redirecting out a different interface). >> >> - Programs that just return the value of the bpf_redirect() call will >> automatically fall back to the regular networking stack, simplifying >> programs that (for instance) build a router with the fib_lookup() helper. >> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> include/uapi/linux/bpf.h | 8 ++++++++ >> net/core/filter.c | 10 +++++++++- >> 2 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h >> index 7c6aef253173..4c41482b7604 100644 >> --- a/include/uapi/linux/bpf.h >> +++ b/include/uapi/linux/bpf.h >> @@ -3098,6 +3098,14 @@ enum xdp_action { >> XDP_REDIRECT, >> }; >> >> +/* Flags for bpf_xdp_redirect_map helper */ >> + >> +/* If set, the help will check if the entry exists in the map and return >> + * XDP_PASS if it doesn't. >> + */ >> +#define XDP_REDIRECT_PASS_ON_INVALID BIT(0) >> +#define XDP_REDIRECT_ALL_FLAGS XDP_REDIRECT_PASS_ON_INVALID >> + >> /* user accessible metadata for XDP packet hook >> * new fields must be added to the end of this structure >> */ >> diff --git a/net/core/filter.c b/net/core/filter.c >> index 55bfc941d17a..dfab8478f66c 100644 >> --- a/net/core/filter.c >> +++ b/net/core/filter.c >> @@ -3755,9 +3755,17 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, >> { >> struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info); >> >> - if (unlikely(flags)) >> + if (unlikely(flags & ~XDP_REDIRECT_ALL_FLAGS)) >> return XDP_ABORTED; >> >> + if (flags & XDP_REDIRECT_PASS_ON_INVALID) { >> + struct net_device *fwd; > > It is slightly misguiding that '*fwd' is a 'struct net_device', as the > __xdp_map_lookup_elem() call works for all the supported redirect-map > types. > > People should realize that this patch is a general approach for all the > redirect-map types. Good point, will fix! :) -Toke
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 7c6aef253173..4c41482b7604 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -3098,6 +3098,14 @@ enum xdp_action { XDP_REDIRECT, }; +/* Flags for bpf_xdp_redirect_map helper */ + +/* If set, the help will check if the entry exists in the map and return + * XDP_PASS if it doesn't. + */ +#define XDP_REDIRECT_PASS_ON_INVALID BIT(0) +#define XDP_REDIRECT_ALL_FLAGS XDP_REDIRECT_PASS_ON_INVALID + /* user accessible metadata for XDP packet hook * new fields must be added to the end of this structure */ diff --git a/net/core/filter.c b/net/core/filter.c index 55bfc941d17a..dfab8478f66c 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3755,9 +3755,17 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, { struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info); - if (unlikely(flags)) + if (unlikely(flags & ~XDP_REDIRECT_ALL_FLAGS)) return XDP_ABORTED; + if (flags & XDP_REDIRECT_PASS_ON_INVALID) { + struct net_device *fwd; + + fwd = __xdp_map_lookup_elem(map, ifindex); + if (unlikely(!fwd)) + return XDP_PASS; + } + ri->ifindex = ifindex; ri->flags = flags; WRITE_ONCE(ri->map, map);
The bpf_redirect_map() helper used by XDP programs doesn't return any indication of whether it can successfully redirect to the map index it was given. Instead, BPF programs have to track this themselves, leading to programs using duplicate maps to track which entries are populated in the devmap. This adds a flag to the XDP version of the bpf_redirect_map() helper, which makes the helper do a lookup in the map when called, and return XDP_PASS if there is no value at the provided index. This enables two use cases: - A BPF program can check the return code from the helper call and react if it is XDP_PASS (by, for instance, redirecting out a different interface). - Programs that just return the value of the bpf_redirect() call will automatically fall back to the regular networking stack, simplifying programs that (for instance) build a router with the fib_lookup() helper. Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- include/uapi/linux/bpf.h | 8 ++++++++ net/core/filter.c | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)