Re: [v2 PATCH 1/6] netfilter: Add new netlink NFQA_CFG_FAIL_OPEN

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

On Tue, May 08, 2012 at 03:13:54PM +0530, Krishna Kumar wrote:
> Define a new netlink message: NFQA_CFG_FAIL_OPEN
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> Signed-off-by: Vivek Kashyap <vivk@us.ibm.com>
> Signed-off-by: Sridhar Samudrala <samudrala@us.ibm.com>
> ---
>  include/linux/netfilter/nfnetlink_queue.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff -ruNp org/include/linux/netfilter/nfnetlink_queue.h new/include/linux/netfilter/nfnetlink_queue.h
> --- org/include/linux/netfilter/nfnetlink_queue.h	2012-05-08 09:16:41.969050136 +0530
> +++ new/include/linux/netfilter/nfnetlink_queue.h	2012-05-08 09:39:10.334761077 +0530
> @@ -84,6 +84,7 @@ enum nfqnl_attr_config {
>  	NFQA_CFG_CMD,			/* nfqnl_msg_config_cmd */
>  	NFQA_CFG_PARAMS,		/* nfqnl_msg_config_params */
>  	NFQA_CFG_QUEUE_MAXLEN,		/* __u32 */
> +	NFQA_CFG_FAIL_OPEN,		/* __u8 */
>  	__NFQA_CFG_MAX
>  };
>  #define NFQA_CFG_MAX (__NFQA_CFG_MAX-1)

The patch logic that you use is not correct. One new feature, one
patch, please.

In case it gets big, then you can split changes following this logic:
patches that prepare the new feature first, then the patches that
contain the feature.

In this case, you have to put everything into one single patch.

Please, send one single patch for this new feature.

Moreover, rename NFQA_CFG_FAIL_OPEN to NFAQ_CFG_FLAGS and declare:

#define NFAQ_CFG_F_FAIL_OPEN  (1 << 0)

Then, check for that flag in the code to enable the fail open
behaviour.

I have another feature here that will use those flags for another
option.