* [PATCH lnfct 0/2] fix - src: add support for IPv6 NAT
@ 2017-02-28 4:53 Ken-ichirou MATSUZAWA
2017-02-28 4:55 ` [PATCH lnfct 1/2] conntrack: fix missing break Ken-ichirou MATSUZAWA
2017-02-28 5:00 ` [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition Ken-ichirou MATSUZAWA
0 siblings, 2 replies; 10+ messages in thread
From: Ken-ichirou MATSUZAWA @ 2017-02-28 4:53 UTC (permalink / raw)
To: The netfilter developer mailinglist
Hi,
This patch series fixes commit:
73ad642ba462d0992e1903012eee4ebfec89ed69
f5e51ad64d9e5597e8880b652abe261585c2563d
Conditions in getobjopt_is_snat() / getobjopt_is_dnat() has
been changed in the commit above and fixed by this, but I do
not understand proper way to check a nfct content is correct.
Please review it.
Thanks,
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH lnfct 1/2] conntrack: fix missing break 2017-02-28 4:53 [PATCH lnfct 0/2] fix - src: add support for IPv6 NAT Ken-ichirou MATSUZAWA @ 2017-02-28 4:55 ` Ken-ichirou MATSUZAWA 2017-02-28 10:47 ` Pablo Neira Ayuso 2017-02-28 5:00 ` [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition Ken-ichirou MATSUZAWA 1 sibling, 1 reply; 10+ messages in thread From: Ken-ichirou MATSUZAWA @ 2017-02-28 4:55 UTC (permalink / raw) To: The netfilter developer mailinglist Signed-off-by Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> --- src/conntrack/objopt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c index 119a83a..fb43d6c 100644 --- a/src/conntrack/objopt.c +++ b/src/conntrack/objopt.c @@ -81,6 +81,7 @@ static void setobjopt_undo_dnat(struct nf_conntrack *ct) ct->dnat.max_ip.v4 = ct->dnat.min_ip.v4; ct->repl.src.v4 = ct->head.orig.dst.v4; set_bit(ATTR_DNAT_IPV4, ct->head.set); + break; case AF_INET6: memcpy(&ct->dnat.min_ip.v6, &ct->repl.src.v6, sizeof(struct in6_addr)); -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 1/2] conntrack: fix missing break 2017-02-28 4:55 ` [PATCH lnfct 1/2] conntrack: fix missing break Ken-ichirou MATSUZAWA @ 2017-02-28 10:47 ` Pablo Neira Ayuso 0 siblings, 0 replies; 10+ messages in thread From: Pablo Neira Ayuso @ 2017-02-28 10:47 UTC (permalink / raw) To: Ken-ichirou MATSUZAWA; +Cc: The netfilter developer mailinglist Applied, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 4:53 [PATCH lnfct 0/2] fix - src: add support for IPv6 NAT Ken-ichirou MATSUZAWA 2017-02-28 4:55 ` [PATCH lnfct 1/2] conntrack: fix missing break Ken-ichirou MATSUZAWA @ 2017-02-28 5:00 ` Ken-ichirou MATSUZAWA 2017-02-28 10:47 ` Pablo Neira Ayuso 1 sibling, 1 reply; 10+ messages in thread From: Ken-ichirou MATSUZAWA @ 2017-02-28 5:00 UTC (permalink / raw) To: The netfilter developer mailinglist >From 9e8aa4ed079b526faf190b69a2c1032f22776602 Mon Sep 17 00:00:00 2001 From: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> Date: Tue, 28 Feb 2017 11:34:29 +0900 Subject: [PATCH 2/2] conntrack: revert getobjopt_is_nat condition Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> --- src/conntrack/objopt.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c index fb43d6c..1581480 100644 --- a/src/conntrack/objopt.c +++ b/src/conntrack/objopt.c @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int option) static int getobjopt_is_snat(const struct nf_conntrack *ct) { - if (!(test_bit(ATTR_STATUS, ct->head.set))) - return 0; - - if (!(ct->status & IPS_SRC_NAT_DONE)) + if (test_bit(ATTR_STATUS, ct->head.set) && + !(ct->status & IPS_SRC_NAT_DONE)) return 0; switch (ct->head.orig.l3protonum) { @@ -166,10 +164,8 @@ static int getobjopt_is_snat(const struct nf_conntrack *ct) static int getobjopt_is_dnat(const struct nf_conntrack *ct) { - if (!(test_bit(ATTR_STATUS, ct->head.set))) - return 0; - - if (!(ct->status & IPS_DST_NAT_DONE)) + if (test_bit(ATTR_STATUS, ct->head.set) && + !(ct->status & IPS_DST_NAT_DONE)) return 0; switch (ct->head.orig.l3protonum) { -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 5:00 ` [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition Ken-ichirou MATSUZAWA @ 2017-02-28 10:47 ` Pablo Neira Ayuso 2017-02-28 11:44 ` Ken-ichirou MATSUZAWA 0 siblings, 1 reply; 10+ messages in thread From: Pablo Neira Ayuso @ 2017-02-28 10:47 UTC (permalink / raw) To: Ken-ichirou MATSUZAWA; +Cc: The netfilter developer mailinglist Hi Ken-ichirou, On Tue, Feb 28, 2017 at 02:00:41PM +0900, Ken-ichirou MATSUZAWA wrote: > From 9e8aa4ed079b526faf190b69a2c1032f22776602 Mon Sep 17 00:00:00 2001 > From: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> > Date: Tue, 28 Feb 2017 11:34:29 +0900 > Subject: [PATCH 2/2] conntrack: revert getobjopt_is_nat condition > > Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> > --- > src/conntrack/objopt.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c > index fb43d6c..1581480 100644 > --- a/src/conntrack/objopt.c > +++ b/src/conntrack/objopt.c > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int option) > > static int getobjopt_is_snat(const struct nf_conntrack *ct) > { > - if (!(test_bit(ATTR_STATUS, ct->head.set))) > - return 0; > - > - if (!(ct->status & IPS_SRC_NAT_DONE)) > + if (test_bit(ATTR_STATUS, ct->head.set) && > + !(ct->status & IPS_SRC_NAT_DONE)) However, if ATTR_STATUS is not set, we keep checking ahead. What are you trying to fix? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 10:47 ` Pablo Neira Ayuso @ 2017-02-28 11:44 ` Ken-ichirou MATSUZAWA 2017-02-28 11:48 ` Pablo Neira Ayuso 0 siblings, 1 reply; 10+ messages in thread From: Ken-ichirou MATSUZAWA @ 2017-02-28 11:44 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: The netfilter developer mailinglist Hi, Pablo On Tue, Feb 28, 2017 at 11:47:25AM +0100, Pablo Neira Ayuso wrote: > > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c > > index fb43d6c..1581480 100644 > > --- a/src/conntrack/objopt.c > > +++ b/src/conntrack/objopt.c > > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int option) > > > > static int getobjopt_is_snat(const struct nf_conntrack *ct) > > { > > - if (!(test_bit(ATTR_STATUS, ct->head.set))) > > - return 0; > > - > > - if (!(ct->status & IPS_SRC_NAT_DONE)) > > + if (test_bit(ATTR_STATUS, ct->head.set) && > > + !(ct->status & IPS_SRC_NAT_DONE)) > > However, if ATTR_STATUS is not set, we keep checking ahead. What are > you trying to fix? It was: - return ((test_bit(ATTR_STATUS, ct->head.set) ? - ct->status & IPS_SRC_NAT_DONE : 1) && - ct->repl.dst.v4 != - ct->head.orig.src.v4); I thought it keeps checking even ATTR_STATUS is not set. But it's ok not to apply, returning false in case of ATTR_STATUS is not set. Thanks, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 11:44 ` Ken-ichirou MATSUZAWA @ 2017-02-28 11:48 ` Pablo Neira Ayuso 2017-02-28 22:29 ` Ken-ichirou MATSUZAWA 0 siblings, 1 reply; 10+ messages in thread From: Pablo Neira Ayuso @ 2017-02-28 11:48 UTC (permalink / raw) To: Ken-ichirou MATSUZAWA; +Cc: The netfilter developer mailinglist On Tue, Feb 28, 2017 at 08:44:53PM +0900, Ken-ichirou MATSUZAWA wrote: > Hi, Pablo > > On Tue, Feb 28, 2017 at 11:47:25AM +0100, Pablo Neira Ayuso wrote: > > > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c > > > index fb43d6c..1581480 100644 > > > --- a/src/conntrack/objopt.c > > > +++ b/src/conntrack/objopt.c > > > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int option) > > > > > > static int getobjopt_is_snat(const struct nf_conntrack *ct) > > > { > > > - if (!(test_bit(ATTR_STATUS, ct->head.set))) > > > - return 0; > > > - > > > - if (!(ct->status & IPS_SRC_NAT_DONE)) > > > + if (test_bit(ATTR_STATUS, ct->head.set) && > > > + !(ct->status & IPS_SRC_NAT_DONE)) > > > > However, if ATTR_STATUS is not set, we keep checking ahead. What are > > you trying to fix? > > It was: > > - return ((test_bit(ATTR_STATUS, ct->head.set) ? > - ct->status & IPS_SRC_NAT_DONE : 1) && > - ct->repl.dst.v4 != > - ct->head.orig.src.v4); > > I thought it keeps checking even ATTR_STATUS is not set. > But it's ok not to apply, returning false in case of > ATTR_STATUS is not set. Ah, I see. static int getobjopt_is_snat(const struct nf_conntrack *ct) { if (!(test_bit(ATTR_STATUS, ct->head.set))) return 0; if (!(ct->status & IPS_SRC_NAT_DONE)) return 0; switch (ct->head.orig.l3protonum) { case AF_INET: return ct->repl.dst.v4 != ct->head.orig.src.v4; case AF_INET6: if (memcmp(&ct->repl.dst.v6, &ct->head.orig.src.v6, sizeof(struct in6_addr)) != 0) return 1; else return 0; default: return 0; } } So you want to check if the addresses mismatch, so we infer from there if there is NAT or not when status bits are not available. Are you trying to catch up some case in netlink event specifically? Thanks for explaining. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 11:48 ` Pablo Neira Ayuso @ 2017-02-28 22:29 ` Ken-ichirou MATSUZAWA 2017-03-01 16:28 ` Pablo Neira Ayuso 0 siblings, 1 reply; 10+ messages in thread From: Ken-ichirou MATSUZAWA @ 2017-02-28 22:29 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: The netfilter developer mailinglist Hi, Pablo On Tue, Feb 28, 2017 at 12:48:09PM +0100, Pablo Neira Ayuso wrote: > So you want to check if the addresses mismatch, so we infer from there > if there is NAT or not when status bits are not available. > > Are you trying to catch up some case in netlink event specifically? It's nothing. My skimping test for lnfct binding for another languate which set only ATTR_REPL_IPV4DST then get NFCT_GOPT_IS_NAT was success, but it fails now. Thanks, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-02-28 22:29 ` Ken-ichirou MATSUZAWA @ 2017-03-01 16:28 ` Pablo Neira Ayuso 2017-03-03 12:18 ` Pablo Neira Ayuso 0 siblings, 1 reply; 10+ messages in thread From: Pablo Neira Ayuso @ 2017-03-01 16:28 UTC (permalink / raw) To: Ken-ichirou MATSUZAWA; +Cc: The netfilter developer mailinglist On Wed, Mar 01, 2017 at 07:29:33AM +0900, Ken-ichirou MATSUZAWA wrote: > Hi, Pablo > > On Tue, Feb 28, 2017 at 12:48:09PM +0100, Pablo Neira Ayuso wrote: > > So you want to check if the addresses mismatch, so we infer from there > > if there is NAT or not when status bits are not available. > > > > Are you trying to catch up some case in netlink event specifically? > > It's nothing. My skimping test for lnfct binding for another languate > which set only ATTR_REPL_IPV4DST then get NFCT_GOPT_IS_NAT was success, > but it fails now. I see, we should restore the original behaviour, I think your patch is correct then. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition 2017-03-01 16:28 ` Pablo Neira Ayuso @ 2017-03-03 12:18 ` Pablo Neira Ayuso 0 siblings, 0 replies; 10+ messages in thread From: Pablo Neira Ayuso @ 2017-03-03 12:18 UTC (permalink / raw) To: Ken-ichirou MATSUZAWA; +Cc: The netfilter developer mailinglist On Wed, Mar 01, 2017 at 05:28:02PM +0100, Pablo Neira Ayuso wrote: > On Wed, Mar 01, 2017 at 07:29:33AM +0900, Ken-ichirou MATSUZAWA wrote: > > Hi, Pablo > > > > On Tue, Feb 28, 2017 at 12:48:09PM +0100, Pablo Neira Ayuso wrote: > > > So you want to check if the addresses mismatch, so we infer from there > > > if there is NAT or not when status bits are not available. > > > > > > Are you trying to catch up some case in netlink event specifically? > > > > It's nothing. My skimping test for lnfct binding for another languate > > which set only ATTR_REPL_IPV4DST then get NFCT_GOPT_IS_NAT was success, > > but it fails now. > > I see, we should restore the original behaviour, I think your patch is > correct then. JFYI: I have applied this patch, thanks for explaining. BTW, it would be great if you can add some short description to your follow up patch. If English is a concern, don't bother much about it, we're non-native speaker mostly :) Thanks! ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-03-03 15:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-28 4:53 [PATCH lnfct 0/2] fix - src: add support for IPv6 NAT Ken-ichirou MATSUZAWA 2017-02-28 4:55 ` [PATCH lnfct 1/2] conntrack: fix missing break Ken-ichirou MATSUZAWA 2017-02-28 10:47 ` Pablo Neira Ayuso 2017-02-28 5:00 ` [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition Ken-ichirou MATSUZAWA 2017-02-28 10:47 ` Pablo Neira Ayuso 2017-02-28 11:44 ` Ken-ichirou MATSUZAWA 2017-02-28 11:48 ` Pablo Neira Ayuso 2017-02-28 22:29 ` Ken-ichirou MATSUZAWA 2017-03-01 16:28 ` Pablo Neira Ayuso 2017-03-03 12:18 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).