On Wed, Aug 17, 2022 at 11:24 PM Shmulik Ladkani
<shmulik@xxxxxxxxxxxxxxxx> wrote:
>
> Currently, attaching BPF_PROG_TYPE_FLOW_DISSECTOR programs completely
> replaces the flow-dissector logic with custom dissection logic.
> This forces implementors to write programs that handle dissection for
> any flows expected in the namespace.
>
> It makes sense for flow-dissector bpf programs to just augment the
> dissector with custom logic (e.g. dissecting certain flows or custom
> protocols), while enjoying the broad capabilities of the standard
> dissector for any other traffic.
>
> Introduce BPF_FLOW_DISSECTOR_CONTINUE retcode. Flow-dissector bpf
> programs may return this to indicate no dissection was made, and
> fallback to the standard dissector is requested.
Some historic perspective: the original goal was to explicitly not
fallback to the c code.
It seems like it should be fine with this extra return code.
But let's also extend tools/testing/selftests/bpf/progs/bpf_flow.c
with a case that exercises this new return code?
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@xxxxxxxxx>
> ---
> include/uapi/linux/bpf.h | 5 +++++
> net/core/flow_dissector.c | 3 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 7bf9ba1329be..6d6654da7cef 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5836,6 +5836,11 @@ enum bpf_ret_code {
> * represented by BPF_REDIRECT above).
> */
> BPF_LWT_REROUTE = 128,
> + /* BPF_FLOW_DISSECTOR_CONTINUE: used by BPF_PROG_TYPE_FLOW_DISSECTOR
> + * to indicate that no custom dissection was performed, and
> + * fallback to standard dissector is requested.
> + */
> + BPF_FLOW_DISSECTOR_CONTINUE = 129,
> };
Is it too late to also amend verifier's check_return_code to allow
only a small subset of return types for flow-disccestor program type?
> struct bpf_sock {
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index a01817fb4ef4..990429c69ccd 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -1022,11 +1022,14 @@ bool __skb_flow_dissect(const struct net *net,
> prog = READ_ONCE(run_array->items[0].prog);
> result = bpf_flow_dissect(prog, &ctx, n_proto, nhoff,
> hlen, flags);
> + if (result == BPF_FLOW_DISSECTOR_CONTINUE)
> + goto dissect_continue;
> __skb_flow_bpf_to_target(&flow_keys, flow_dissector,
> target_container);
> rcu_read_unlock();
> return result == BPF_OK;
> }
> +dissect_continue:
> rcu_read_unlock();
> }
>
> --
> 2.37.1
>
