On Fri, Oct 1, 2021 at 3:04 PM Joanne Koong <joannekoong@xxxxxx> wrote:
>
> This patch enables XDP programs to use the bpf_load_hdr_opt helper
> function to load header options.
>
> The upper 16 bits of "flags" is used to denote the offset to the tcp
> header. No other flags are, at this time, used by XDP programs.
> In the future, more flags can be included to support other types of
> header options.
>
> Much of the logic for loading header options can be shared between
> sockops and xdp programs. In net/core/filter.c, this common shared
> logic is refactored into a separate function both sockops and xdp
> use.
>
> Signed-off-by: Joanne Koong <joannekoong@xxxxxx>
Looks good over all.
Acked-by: Song Liu <songliubraving@xxxxxx>
Just a nitpick below.
> ---
> include/uapi/linux/bpf.h | 26 ++++++----
> net/core/filter.c | 88 ++++++++++++++++++++++++++--------
> tools/include/uapi/linux/bpf.h | 26 ++++++----
> 3 files changed, 103 insertions(+), 37 deletions(-)
>
[...]
> +
> +BPF_CALL_4(bpf_xdp_load_hdr_opt, struct xdp_buff *, xdp,
> + void *, search_res, u32, len, u64, flags)
> +{
> + const void *op, *opend;
> + struct tcphdr *th;
> +
> + /* The upper 16 bits of flags contain the offset to the tcp header.
> + * No other bits should be set.
> + */
> + if (flags & 0xffffffffffff)
Maybe use (1ULL << BPF_LOAD_HDR_OPT_TCP_OFFSET_SHIFT) - 1
> + return -EINVAL;
> +
> + th = xdp->data + (flags >> BPF_LOAD_HDR_OPT_TCP_OFFSET_SHIFT);
> + op = (void *)th + sizeof(struct tcphdr);
> + if (unlikely(op > xdp->data_end))
> + return -EINVAL;
[...]
