On Fri, Jul 21, 2023 at 01:19:04AM +0200, Florian Westphal wrote:
> Daniel Xu <dxu@xxxxxxxxx> wrote:
> > + const struct nf_defrag_hook __maybe_unused *hook;
> > +
> > + switch (link->hook_ops.pf) {
> > +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV4)
> > + case NFPROTO_IPV4:
> > + hook = get_proto_defrag_hook(link, nf_defrag_v4_hook, "nf_defrag_ipv4");
> > + if (IS_ERR(hook))
> > + return PTR_ERR(hook);
> > +
> > + link->defrag_hook = hook;
> > + return 0;
> > +#endif
> > +#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
> > + case NFPROTO_IPV6:
> > + hook = get_proto_defrag_hook(link, nf_defrag_v6_hook, "nf_defrag_ipv6");
> > + if (IS_ERR(hook))
> > + return PTR_ERR(hook);
> > +
> > + link->defrag_hook = hook;
> > + return 0;
> > +#endif
> > + default:
> > + return -EAFNOSUPPORT;
> > + }
> > +}
> > +
> > +static void bpf_nf_disable_defrag(struct bpf_nf_link *link)
> > +{
> > + const struct nf_defrag_hook *hook = link->defrag_hook;
> > +
> > + if (!hook)
> > + return;
> > + hook->disable(link->net);
> > + module_put(hook->owner);
> > +}
> > +
> > static void bpf_nf_link_release(struct bpf_link *link)
> > {
> > struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
> > @@ -37,6 +119,8 @@ static void bpf_nf_link_release(struct bpf_link *link)
> > */
> > if (!cmpxchg(&nf_link->dead, 0, 1))
> > nf_unregister_net_hook(nf_link->net, &nf_link->hook_ops);
> > +
> > + bpf_nf_disable_defrag(nf_link);
> > }
>
> I suspect this needs to be within the cmpxchg() branch to avoid
> multiple ->disable() calls.
Ah, good catch.
>
> > + if (attr->link_create.netfilter.flags & BPF_F_NETFILTER_IP_DEFRAG) {
> > + err = bpf_nf_enable_defrag(link);
> > + if (err) {
> > + bpf_link_cleanup(&link_primer);
> > + return err;
> > + }
> > + }
> > +
> > err = nf_register_net_hook(net, &link->hook_ops);
> > if (err) {
> bpf_nf_disable_defrag(link);
Ack. Did not see that bpf_link_cleanup() sets link->prog to NULL which
disables bpf_link_ops:release() on the release codepath.
>
> Other than those nits this lgtm, thanks!
>
Thanks for reviewing!
Daniel
