lkml.org 
[lkml]   [2023]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH bpf-next v5 2/5] netfilter: bpf: Support BPF_F_NETFILTER_IP_DEFRAG in netfilter link
    On Fri, Jul 21, 2023 at 01:19:04AM +0200, Florian Westphal wrote:
    > Daniel Xu <dxu@dxuuu.xyz> 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

    \
     
     \ /
      Last update: 2023-07-21 22:04    [W:2.589 / U:0.792 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site