Re: [PATCH nf-next v3 5/9] netfilter: nft_tunnel: support NFT_TUNNEL_SRC/DST_IP match

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

On Thu, Aug 01, 2019 at 10:01:22PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> Add new two NFT_TUNNEL_SRC/DST_IP match in nft_tunnel
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> v3: no change
> 
>  include/uapi/linux/netfilter/nf_tables.h |  2 ++
>  net/netfilter/nft_tunnel.c               | 46 +++++++++++++++++++++++++-------
>  2 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 82abaa1..173690a 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -1765,6 +1765,8 @@ enum nft_tunnel_key_attributes {
>  enum nft_tunnel_keys {
>  	NFT_TUNNEL_PATH,
>  	NFT_TUNNEL_ID,
> +	NFT_TUNNEL_SRC_IP,
> +	NFT_TUNNEL_DST_IP,
>  	__NFT_TUNNEL_MAX
>  };
>  #define NFT_TUNNEL_MAX	(__NFT_TUNNEL_MAX - 1)
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index 3d4c2ae..e218163 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -18,6 +18,18 @@ struct nft_tunnel {
>  	enum nft_tunnel_mode	mode:8;
>  };
>  
> +bool nft_tunnel_mode_validate(enum nft_tunnel_mode priv_mode, u8 tun_mode)
> +{
> +	if (priv_mode == NFT_TUNNEL_MODE_NONE ||
> +	    (priv_mode == NFT_TUNNEL_MODE_RX &&
> +	     !(tun_mode & IP_TUNNEL_INFO_TX)) ||
> +	    (priv_mode == NFT_TUNNEL_MODE_TX &&
> +	     (tun_mode & IP_TUNNEL_INFO_TX)))
> +		return true;
> +
> +	return false;
> +}

Make an initial patch to add nft_tunnel_mode_validate().

>  static void nft_tunnel_get_eval(const struct nft_expr *expr,
>  				struct nft_regs *regs,
>  				const struct nft_pktinfo *pkt)
> @@ -34,11 +46,7 @@ static void nft_tunnel_get_eval(const struct nft_expr *expr,
>  			nft_reg_store8(dest, false);
>  			return;
>  		}
> -		if (priv->mode == NFT_TUNNEL_MODE_NONE ||
> -		    (priv->mode == NFT_TUNNEL_MODE_RX &&
> -		     !(tun_info->mode & IP_TUNNEL_INFO_TX)) ||
> -		    (priv->mode == NFT_TUNNEL_MODE_TX &&
> -		     (tun_info->mode & IP_TUNNEL_INFO_TX)))
> +		if (nft_tunnel_mode_validate(priv->mode, tun_info->mode))
>  			nft_reg_store8(dest, true);
>  		else
>  			nft_reg_store8(dest, false);
[...]
> +	case NFT_TUNNEL_DST_IP:
> +		if (!tun_info) {
> +			regs->verdict.code = NFT_BREAK;
> +			return;
> +		}
> +		if (nft_tunnel_mode_validate(priv->mode, tun_info->mode))
> +			*dest = ntohl(tun_info->key.u.ipv4.dst);

No need to convert this from network to host endianess.

> +		else
> +			regs->verdict.code = NFT_BREAK;
> +		break;
>  	default:
>  		WARN_ON(1);
>  		regs->verdict.code = NFT_BREAK;
> @@ -86,6 +110,8 @@ static int nft_tunnel_get_init(const struct nft_ctx *ctx,
>  		len = sizeof(u8);
>  		break;
>  	case NFT_TUNNEL_ID:
> +	case NFT_TUNNEL_SRC_IP:
> +	case NFT_TUNNEL_DST_IP:

Missing policy updates, ie. nft_tunnel_key_policy.

I would take an initial patchset with two patches to add support for
this to the tunnel extension.

IPv6 is missing though, you could add it too to this patchset so this
becomes a patchset compose of three patches, I'd suggest.