Re: [PATCH v2] extensions: libxt_mark: Add translation to nft

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

On Sun, Dec 06, 2015 at 09:02:14AM +0530, Shivani Bhardwaj wrote:
> Add translation of the metainformation mark to nft.

Much better, but I still need some changes.

Could you include in the description the commands and the result of
testing this, eg.

$ iptables-xlate -I INPUT -m mark --mark 20
nft add rule filter INPUT mark 20

Several examples like that would be good covering all possible cases,
just to make sure you're testing all possible branches in the code
(even if this kind of manual way, there are better ways to cover-test
this in an automated way but this simple testing is fine by now).

More comments below.

> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
> Changes in v2:
> 	Fix syntax according to nft and remove trailing whitespaces
> 
>  extensions/libxt_mark.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/extensions/libxt_mark.c b/extensions/libxt_mark.c
> index 7f8c995..b0c06c2 100644
> --- a/extensions/libxt_mark.c
> +++ b/extensions/libxt_mark.c
> @@ -75,7 +75,7 @@ mark_print(const void *ip, const struct xt_entry_match *match, int numeric)
>  
>  	if (info->invert)
>  		printf(" !");
> -	
> +
>  	print_mark(info->mark, info->mask);
>  }
>  
> @@ -97,11 +97,69 @@ mark_save(const void *ip, const struct xt_entry_match *match)
>  
>  	if (info->invert)
>  		printf(" !");
> -	
> +
>  	printf(" --mark");
>  	print_mark(info->mark, info->mask);
>  }
>  
> +static void
> +print_mark_xlate(struct xt_buf *buf,
> +		 unsigned int mark, unsigned int mask)
> +{
> +        if (mask != 0xffffffffU)
> +                xt_buf_add(buf, " & x%x == x%x", mark, mask);
> +        else
> +                xt_buf_add(buf, " & x%x", mark);
> +}
> +
> +static void
> +mark_mt_xlate_print(const struct xt_entry_match *match,
> +		    struct xt_buf *buf, int numeric)
> +{
> +	const struct xt_mark_mtinfo1 *info = (const void *)match->data;
> +
> +	if (info->invert)
> +		xt_buf_add(buf, " !=");
> +	print_mark_xlate(buf, info->mark, info->mask);
> +}
> +
> +static int
> +mark_mt_xlate(const struct xt_entry_match *match,
> +	      struct xt_buf *buf, int numeric)
> +{
> +	const struct xt_mark_mtinfo1 *info = (const void *)match->data;
> +
> +	xt_buf_add(buf, "ct mark %s", info->invert ? " !=" : "");
> +        print_mark_xlate(buf, info->mark, info->mask);
> +        xt_buf_add(buf, " ");
   ^^^^^^^^

This indentation is not correct.

> +
> +        return 1;
> +}
> +
> +static void
> +mark_xlate_print(const struct xt_entry_match *match,
> +		 struct xt_buf *buf, int numeric)
> +{
> +	const struct xt_mark_info *info = (const void *)match->data;
> +
> +        if (info->invert)
> +                xt_buf_add(buf, " !=");
> +        print_mark_xlate(buf, info->mark, info->mask);
        ^^^

Same here.

Thanks.