On Sat, Jun 22, 2019 at 7:19 PM Toke Høiland-Jørgensen <t...@redhat.com> wrote:
>
> When using the bpf_redirect_map() helper to redirect packets from XDP, the 
> eBPF
> program cannot currently know whether the redirect will succeed, which makes 
> it
> impossible to gracefully handle errors. To properly fix this will probably
> require deeper changes to the way TX resources are allocated, but one thing 
> that
> is fairly straight forward to fix is to allow lookups into devmaps, so 
> programs
> can at least know when a redirect is *guaranteed* to fail because there is no
> entry in the map. Currently, programs work around this by keeping a shadow map
> of another type which indicates whether a map index is valid.
>
> This series contains two changes that are complementary ways to fix this 
> issue:
>
> - Moving the map lookup into the bpf_redirect_map() helper (and caching the
>   result), so the helper can return an error if no value is found in the map.
>   This includes a refactoring of the devmap and cpumap code to not care about
>   the index on enqueue.
>
> - Allowing regular lookups into devmaps from eBPF programs, using the 
> read-only
>   flag to make sure they don't change the values.
>
> The performance impact of the series is negligible, in the sense that I cannot
> measure it because the variance between test runs is higher than the 
> difference
> pre/post series.
>
> Changelog:
>
> v5:
>   - Rebase on latest bpf-next.
>   - Update documentation for bpf_redirect_map() with the new meaning of flags.
>
> v4:
>   - Fix a few nits from Andrii
>   - Lose the #defines in bpf.h and just compare the flags argument directly to
>     XDP_TX in bpf_xdp_redirect_map().
>
> v3:
>   - Adopt Jonathan's idea of using the lower two bits of the flag value as the
>     return code.
>   - Always do the lookup, and cache the result for use in xdp_do_redirect(); 
> to
>     achieve this, refactor the devmap and cpumap code to get rid the bitmap 
> for
>     selecting which devices to flush.
>
> v2:
>   - For patch 1, make it clear that the change works for any map type.
>   - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the return
>     value read-only.
>
> ---
>
> Toke Høiland-Jørgensen (3):
>       devmap/cpumap: Use flush list instead of bitmap
>       bpf_xdp_redirect_map: Perform map lookup in eBPF helper
>       devmap: Allow map lookups from eBPF
>
>
>  include/linux/filter.h   |    1
>  include/uapi/linux/bpf.h |    7 ++-
>  kernel/bpf/cpumap.c      |  106 ++++++++++++++++++++-----------------------
>  kernel/bpf/devmap.c      |  113 
> ++++++++++++++++++++++------------------------
>  kernel/bpf/verifier.c    |    7 +--
>  net/core/filter.c        |   29 +++++-------
>  6 files changed, 123 insertions(+), 140 deletions(-)
>


Looks like you forgot to add my Acked-by's for your patches?

Reply via email to