[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230720231904.GA31372@breakpoint.cc>
Date: Fri, 21 Jul 2023 01:19:04 +0200
From: Florian Westphal <fw@...len.de>
To: Daniel Xu <dxu@...uu.xyz>
Cc: daniel@...earbox.net, kadlec@...filter.org, ast@...nel.org,
pablo@...filter.org, kuba@...nel.org, davem@...emloft.net,
andrii@...nel.org, edumazet@...gle.com, pabeni@...hat.com,
fw@...len.de, alexei.starovoitov@...il.com, martin.lau@...ux.dev,
song@...nel.org, yhs@...com, john.fastabend@...il.com,
kpsingh@...nel.org, sdf@...gle.com, haoluo@...gle.com,
jolsa@...nel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
coreteam@...filter.org, netdev@...r.kernel.org, dsahern@...nel.org
Subject: Re: [PATCH bpf-next v5 2/5] netfilter: bpf: Support
BPF_F_NETFILTER_IP_DEFRAG in netfilter link
Daniel Xu <dxu@...uu.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.
> + 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);
Other than those nits this lgtm, thanks!
Powered by blists - more mailing lists