On Sat, Nov 08, 2014 at 10:35:49PM +0100, Arturo Borrero Gonzalez wrote:
> Inversion of matches is failing because NFT_CMP_EQ is used unconditionally.
>
> The family agnostic functions don't need this fix, because arp inv flags
> are translated to ipt inv flags, and these flags are well handled there.
>
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@xxxxxxxxx>
> ---
> NOTES: This patch is for the master branch of iptables tree.
> Compile-tested only. Please comment.
Please, no compile-tested patches, submit RFC or tested stuff.
> iptables/nft-arp.c | 41 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
> index f45ad0f..cb3623d 100644
> --- a/iptables/nft-arp.c
> +++ b/iptables/nft-arp.c
> @@ -164,6 +164,7 @@ static int nft_arp_add(struct nft_rule *r, void *data)
> struct arptables_command_state *cs = data;
> struct arpt_entry *fw = &cs->fw;
> uint8_t flags = arpt_to_ipt_flags(fw->arp.invflags);
I prefer if you kill arpt_to_ipt_flags(), so add_iniface() uses
native NFT_CMP_{EQ,NEQ} as third parameter. Thus, add_iniface() will
be consistent with other existing add_cmp_*() functions.
This will require some extra code in nft-ipv4 and nft-ipv6 though, but
I'd like to avoid this arpt to ipt flags conversion.
> + uint32_t op = NFT_CMP_EQ;
> int ret = 0;
>
> if (fw->arp.iniface[0] != '\0')
> @@ -174,12 +175,24 @@ static int nft_arp_add(struct nft_rule *r, void *data)
>
> if (fw->arp.arhrd != 0) {
> add_payload(r, offsetof(struct arphdr, ar_hrd), 2);
> - add_cmp_u16(r, fw->arp.arhrd, NFT_CMP_EQ);
> +
> + if (fw->arp.invflags & ARPT_INV_ARPHRD)
> + op = NFT_CMP_NEQ;
> + else
> + op = NFT_CMP_EQ;
> +
> + add_cmp_u16(r, fw->arp.arhrd, op);
This is a good example of what I would like to see in the family
specific nft code, use this pattern consistently in this code. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
