[PATCH net-next 1/2] netfilter: nf_ct_expect: nf_ct_expect_related_report(): Return zero on success,

[PATCH net-next 1/2] netfilter: nf_ct_expect: nf_ct_expect_related_report(): Return zero on success,

[Jarno Rajahalme]

Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
returns void") inadvertently changed the successful return value of
nf_ct_expect_related_report() from 0 to 1, which caused openvswitch
conntrack integration fail in FTP test cases.

Fix this by always returning zero on the success code path.

Fixes: 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert() returns void")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
 net/netfilter/nf_conntrack_expect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index e19a697..d6ace69 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -467,7 +467,7 @@ int nf_ct_expect_related_report(struct nf_conntrack_expect *expect,
 
 	spin_unlock_bh(&nf_conntrack_expect_lock);
 	nf_ct_expect_event_report(IPEXP_NEW, expect, portid, report);
-	return ret;
+	return 0;
 out:
 	spin_unlock_bh(&nf_conntrack_expect_lock);
 	return ret;
-- 
2.1.4

[PATCH net-next 2/2] netfilter: nf_ct_expect: Change __nf_ct_expect_check() return value.

[Jarno Rajahalme]

Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
returns void") inadvertently changed the successful return value of
nf_ct_expect_related_report() from 0 to 1 due to
__nf_ct_expect_check() returning 1 on success.  Prevent this
regression in the future by changing the return value of
__nf_ct_expect_check() to 0 on success.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
 net/netfilter/nf_conntrack_expect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index d6ace69..4b2e1fb 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -410,7 +410,7 @@ static inline int __nf_ct_expect_check(struct nf_conntrack_expect *expect)
 	struct net *net = nf_ct_exp_net(expect);
 	struct hlist_node *next;
 	unsigned int h;
-	int ret = 1;
+	int ret = 0;
 
 	if (!master_help) {
 		ret = -ESHUTDOWN;
@@ -460,7 +460,7 @@ int nf_ct_expect_related_report(struct nf_conntrack_expect *expect,
 
 	spin_lock_bh(&nf_conntrack_expect_lock);
 	ret = __nf_ct_expect_check(expect);
-	if (ret <= 0)
+	if (ret < 0)
 		goto out;
 
 	nf_ct_expect_insert(expect);
-- 
2.1.4

Re: [PATCH net-next 1/2] netfilter: nf_ct_expect: nf_ct_expect_related_report(): Return zero on success,

[Joe Stringer]

On 21 February 2017 at 15:25, Jarno Rajahalme <jarno@ovn.org> wrote:
> Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
> returns void") inadvertently changed the successful return value of
> nf_ct_expect_related_report() from 0 to 1, which caused openvswitch
> conntrack integration fail in FTP test cases.

I can't tell at a glance given all the netlink callback stuff but I
suspect this broke expectation insertion from netlink too.

> Fix this by always returning zero on the success code path.
>
> Fixes: 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert() returns void")
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Acked-by: Joe Stringer <joe@ovn.org>

Re: [PATCH net-next 2/2] netfilter: nf_ct_expect: Change __nf_ct_expect_check() return value.

[Joe Stringer]

On 21 February 2017 at 15:25, Jarno Rajahalme <jarno@ovn.org> wrote:
> Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
> returns void") inadvertently changed the successful return value of
> nf_ct_expect_related_report() from 0 to 1 due to
> __nf_ct_expect_check() returning 1 on success.  Prevent this
> regression in the future by changing the return value of
> __nf_ct_expect_check() to 0 on success.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---

It seems like other functions in this file return 0 as successful
condition, so from that perspective this seems more in line with
existing code.

Acked-by: Joe Stringer <joe@ovn.org>

Re: [PATCH net-next 1/2] netfilter: nf_ct_expect: nf_ct_expect_related_report(): Return zero on success,

[Pablo Neira Ayuso]

On Tue, Feb 21, 2017 at 03:25:21PM -0800, Jarno Rajahalme wrote:
> Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
> returns void") inadvertently changed the successful return value of
> nf_ct_expect_related_report() from 0 to 1, which caused openvswitch
> conntrack integration fail in FTP test cases.
> 
> Fix this by always returning zero on the success code path.

Please, Cc netfilter-devel for your netfilter patches.

I can pick these patches via nf.git tree.

Thank you.

Re: [PATCH net-next 2/2] netfilter: nf_ct_expect: Change __nf_ct_expect_check() return value.

[Pablo Neira Ayuso]

On Tue, Feb 21, 2017 at 03:25:22PM -0800, Jarno Rajahalme wrote:
> Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()
> returns void") inadvertently changed the successful return value of
> nf_ct_expect_related_report() from 0 to 1 due to
> __nf_ct_expect_check() returning 1 on success.  Prevent this
> regression in the future by changing the return value of
> __nf_ct_expect_check() to 0 on success.

Same thing, please Cc netfilter-devel. Thanks.

Re: [PATCH net-next 1/2] netfilter: nf_ct_expect: nf_ct_expect_related_report(): Return zero on success,

[Sergei Shtylyov]

Hello!

On 2/22/2017 2:25 AM, Jarno Rajahalme wrote:

> Commit 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert()

    Need at least 12 hex digits.

> returns void") inadvertently changed the successful return value of
> nf_ct_expect_related_report() from 0 to 1, which caused openvswitch
> conntrack integration fail in FTP test cases.
>
> Fix this by always returning zero on the success code path.
>
> Fixes: 4dee62b1 ("netfilter: nf_ct_expect: nf_ct_expect_insert() returns void")
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
[...]

MBR, Sergei