Re: [PATCH RFC v2 bpf-next 1/3] bpf: add bpf_link support for BPF_NETFILTER programs

[Daniel Xu <dxu@dxuuu.xyz>]

Hi Florian, Stan,

On Fri, Mar 03, 2023 at 01:27:52AM +0100, Florian Westphal wrote:
> Stanislav Fomichev <sdf@google.com> wrote:
> > On 03/02, Florian Westphal wrote:
> > > +			struct {
> > > +				__u32		pf;
> > > +				__u32		hooknum;
> > > +				__s32		prio;
> > > +			} netfilter;
> > 
> > For recent tc BPF program extensions, we've discussed that it might be
> > better
> > to have an option to attach program before/after another one in the chain.
> > So the API essentially would receive a before/after flag + fd/id of the
> >
> > Should we do something similar here? See [0] for the original
> > discussion.
> > 
> > 0: https://lore.kernel.org/bpf/YzzWDqAmN5DRTupQ@google.com/
> 
> Thanks for the pointer, I will have a look.
> 
> The above exposes the "prio" of netfilter hooks, so someone
> that needs their hook to run early on, say, before netfilters
> nat engine, could just use INT_MIN.
> 
> We could -- for nf bpf -- make the bpf_link fail if a hook
> with the same priority already exists to avoid the "undefined
> behaviour" here (same prio means register order decides what
> hook function runs first ...).
> 
> This could be relevant if you have e.g. one bpf program collecting
> statistics vs. one doing drops.
> 
> I'll dig though the thread and would try to mimic the tc link
> mechanism as close as possible.

While I think the direction the TC link discussion took is totally fine,
TC has the advantage (IIUC) of being a somewhat isolated hook. Meaning
it does not make sense for a user to mix priority values && before/after
semantics.

Netfilter is different in that there is by default modules active with
fixed priority values. So mixing in before/after semantics here could
get confusing.

Thanks,
Daniel