Phil Sutter <phil@xxxxxx> wrote:
> Hook into new devices if their name matches the hook spec.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> net/netfilter/nft_chain_filter.c | 40 +++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
> index 2507e3beac5c..ec44c27a9d91 100644
> --- a/net/netfilter/nft_chain_filter.c
> +++ b/net/netfilter/nft_chain_filter.c
> @@ -326,14 +326,37 @@ static void nft_netdev_event(unsigned long event, struct net_device *dev,
> struct nft_hook *hook;
>
> list_for_each_entry(hook, &basechain->hook_list, list) {
> - ops = nft_hook_find_ops(hook, dev);
> - if (!ops)
> - continue;
> + switch (event) {
> + case NETDEV_UNREGISTER:
> + ops = nft_hook_find_ops(hook, dev);
> + if (!ops)
> + continue;
>
> - if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
> - nf_unregister_net_hook(ctx->net, ops);
> - list_del(&ops->list);
> - kfree(ops);
> + if (!(ctx->chain->table->flags & NFT_TABLE_F_DORMANT))
> + nf_unregister_net_hook(ctx->net, ops);
> + list_del(&ops->list);
> + kfree(ops);
This needs to use kfree_rcu() + list_del_rcu, nf_unregister_net_hook
only stops new packets from executing for dev, it doesn't stop new
packets.
Or is this guaranteed by UNREGISTER event already?
If so, please add a comment.
> + break;
> + case NETDEV_REGISTER:
> + if (strcmp(hook->ifname, dev->name))
> + continue;
> + ops = kzalloc(sizeof(struct nf_hook_ops),
> + GFP_KERNEL_ACCOUNT);
ops = kmemdup(&basechain->ops, .. ?
> + if (ops) {
> + memcpy(ops, &basechain->ops, sizeof(*ops));
> + ops->dev = dev;
> + }
> + if (ops &&
> + (ctx->chain->table->flags & NFT_TABLE_F_DORMANT ||
> + !nf_register_net_hook(dev_net(dev), ops))) {
> + list_add_tail(&ops->list, &hook->ops_list);
> + break;
> + }
> + printk(KERN_ERR "chain %s: Can't hook into device %s\n",
> + ctx->chain->name, dev->name);
I think its better to -ENOMEM and veto the netdevice register request in this case.
I also think this needs extra handling for NETDEV_CHANGENAME rather than
the 'treat as UNREG+REG' trick.
Else we may unregister and then fail to re-register which leaves the
device without the registered hook op.
