Re: [PATCH bpf-next v2 1/2] bpf: Add ipip6 and ip6ip decap support for bpf_skb_adjust_room()

[Willem de Bruijn <willemb@xxxxxxxxxx>]

On Wed, Jan 11, 2023 at 3:01 AM Ziyang Xuan
<william.xuanziyang@xxxxxxxxxx> wrote:
>
> Add ipip6 and ip6ip decap support for bpf_skb_adjust_room().
> Main use case is for using cls_bpf on ingress hook to decapsulate
> IPv4 over IPv6 and IPv6 over IPv4 tunnel packets.
>
> Add two new flags BPF_F_ADJ_ROOM_DECAP_L3_IPV{4,6} to indicate the
> new IP header version after decapsulating the outer IP header.
>
> Suggested-by: Willem de Bruijn <willemb@xxxxxxxxxx>
> Signed-off-by: Ziyang Xuan <william.xuanziyang@xxxxxxxxxx>
> ---
>  include/uapi/linux/bpf.h       |  8 ++++++++
>  net/core/filter.c              | 26 +++++++++++++++++++++++++-
>  tools/include/uapi/linux/bpf.h |  8 ++++++++
>  3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 464ca3f01fe7..dde1c2ea1c84 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2644,6 +2644,12 @@ union bpf_attr {
>   *               Use with BPF_F_ADJ_ROOM_ENCAP_L2 flag to further specify the
>   *               L2 type as Ethernet.
>   *
> + *              * **BPF_F_ADJ_ROOM_DECAP_L3_IPV4**,
> + *                **BPF_F_ADJ_ROOM_DECAP_L3_IPV6**:
> + *                Indicate the new IP header version after decapsulating the
> + *                outer IP header. Mainly used in scenarios that the inner and
> + *                outer IP versions are different.
> + *

Nit (only since I have another comment below)

Indicate -> Set
[Mainly used .. that] -> [Used when]

>         if (skb_is_gso(skb)) {
>                 struct skb_shared_info *shinfo = skb_shinfo(skb);
>
> @@ -3596,6 +3609,10 @@ BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
>         if (unlikely(proto != htons(ETH_P_IP) &&
>                      proto != htons(ETH_P_IPV6)))
>                 return -ENOTSUPP;
> +       if (unlikely((shrink && ((flags & BPF_F_ADJ_ROOM_DECAP_L3_MASK) ==
> +                     BPF_F_ADJ_ROOM_DECAP_L3_MASK)) || (!shrink &&
> +                     flags & BPF_F_ADJ_ROOM_DECAP_L3_MASK)))
> +               return -EINVAL;
>
>         off = skb_mac_header_len(skb);
>         switch (mode) {
> @@ -3608,6 +3625,13 @@ BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
>                 return -ENOTSUPP;
>         }
>
> +       if (shrink) {
> +               if (flags & BPF_F_ADJ_ROOM_DECAP_L3_IPV6)
> +                       len_min = sizeof(struct ipv6hdr);
> +               else if (flags & BPF_F_ADJ_ROOM_DECAP_L3_IPV4)
> +                       len_min = sizeof(struct iphdr);
> +       }
> +

How about combining this branch with the above:

  if (flags & BPF_F_ADJ_ROOM_DECAP_L3_MASK) {
    if (!shrink)
      return -EINVAL;

    switch (flags & BPF_F_ADJ_ROOM_DECAP_L3_MASK) {
      case BPF_F_ADJ_ROOM_DECAP_L3_IPV4:
        len_min = sizeof(struct iphdr);
        break;
      case BPF_F_ADJ_ROOM_DECAP_L3_IPV6:
        len_min = sizeof(struct ipv6hdr);
        break;
      default:
        return -EINVAL;
    }