| Message ID | 20141108213434.27991.30805.stgit@nfdev.cica.es |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
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@gmail.com> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); + 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); } if (fw->arp.arpro != 0) { add_payload(r, offsetof(struct arphdr, ar_pro), 2); - add_cmp_u16(r, fw->arp.arpro, NFT_CMP_EQ); + + if (fw->arp.invflags & ARPT_INV_ARPPRO) + op = NFT_CMP_NEQ; + else + op = NFT_CMP_EQ; + + add_cmp_u16(r, fw->arp.arpro, op); } if (fw->arp.arhln != 0) @@ -190,12 +203,24 @@ static int nft_arp_add(struct nft_rule *r, void *data) if (fw->arp.arpop != 0) { add_payload(r, offsetof(struct arphdr, ar_op), 2); - add_cmp_u16(r, fw->arp.arpop, NFT_CMP_EQ); + + if (fw->arp.invflags & ARPT_INV_ARPOP) + op = NFT_CMP_NEQ; + else + op = NFT_CMP_EQ; + + add_cmp_u16(r, fw->arp.arpop, op); } if (fw->arp.src_devaddr.addr[0] != '\0') { add_payload(r, sizeof(struct arphdr), fw->arp.arhln); - add_cmp_ptr(r, NFT_CMP_EQ, fw->arp.src_devaddr.addr, fw->arp.arhln); + + if (fw->arp.invflags & ARPT_INV_SRCDEVADDR) + op = NFT_CMP_NEQ; + else + op = NFT_CMP_EQ; + + add_cmp_ptr(r, op, fw->arp.src_devaddr.addr, fw->arp.arhln); } if (fw->arp.src.s_addr != 0) { @@ -206,7 +231,13 @@ static int nft_arp_add(struct nft_rule *r, void *data) if (fw->arp.tgt_devaddr.addr[0] != '\0') { add_payload(r, sizeof(struct arphdr) + fw->arp.arhln + 4, fw->arp.arhln); - add_cmp_ptr(r, NFT_CMP_EQ, fw->arp.tgt_devaddr.addr, fw->arp.arhln); + + if (fw->arp.invflags & ARPT_INV_TGTDEVADDR) + op = NFT_CMP_NEQ; + else + op = NFT_CMP_EQ; + + add_cmp_ptr(r, op, fw->arp.tgt_devaddr.addr, fw->arp.arhln); } if (fw->arp.tgt.s_addr != 0) {
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@gmail.com> --- NOTES: This patch is for the master branch of iptables tree. Compile-tested only. Please comment. iptables/nft-arp.c | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html