Re: [PATCH nf-next 1/3] netfilter: nfnetlink_queue: remove duplicated obsolete commands handling

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Nov 06, 2015 at 09:49:47AM +0900, Ken-ichirou MATSUZAWA wrote:
> Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
> ---
>  net/netfilter/nfnetlink_queue.c |   18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 7d81d28..f85a3d3 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -1116,21 +1116,10 @@ nfqnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
>  	struct nfgenmsg *nfmsg = nlmsg_data(nlh);
>  	u_int16_t queue_num = ntohs(nfmsg->res_id);
>  	struct nfqnl_instance *queue;
> -	struct nfqnl_msg_config_cmd *cmd = NULL;
>  	struct net *net = sock_net(ctnl);
>  	struct nfnl_queue_net *q = nfnl_queue_pernet(net);
>  	int ret = 0;
>  
> -	if (nfqa[NFQA_CFG_CMD]) {
> -		cmd = nla_data(nfqa[NFQA_CFG_CMD]);
> -
> -		/* Obsolete commands without queue context */
> -		switch (cmd->command) {
> -		case NFQNL_CFG_CMD_PF_BIND: return 0;
> -		case NFQNL_CFG_CMD_PF_UNBIND: return 0;
> -		}
> -	}
> -

Thanks for following up on this Ken-ichirou.

However, I suspect this is wrong:

commit 0360ae412d09bc6f4864c801effcb20bfd84520e
Author: Florian Westphal <fw@strlen.de>
Date:   Fri Nov 23 06:22:21 2012 +0000

    netfilter: kill support for per-af queue backends

The former behaviour before Florian's patch is that
NFQNL_CFG_CMD_PF_BIND and NFQNL_CFG_CMD_PF_UNBIND are terminal.

This code is there not to break backward compatibility, ie. old
userspace that rely on these commands.

After this patch, we inspect if there is an existing queue for this,
which was not happening before.

As I said, my concerns go in the direction of possible breakage of old
code, we shouldn't break backward.

Let me know, thanks.

>  	rcu_read_lock();
>  	queue = instance_lookup(q, queue_num);
>  	if (queue && queue->peer_portid != NETLINK_CB(skb).portid) {