[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Tristan Ye]

Currently, o2net_accept_one() is allowed to accept a connection from
listening node itself, such a fake connection will not be successfully
established due to no handshake detected afterwards, and later end up
with triggering connecting worker in a loop.

We're going to fix this by treating such connection request as 'invalid',
since we've got no chance of requesting connection from a node to itself
in a OCFS2 cluster.

The fix doesn't hurt user's scan for o2net-listener, it always gets a
successful connection from userpace.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
Acked-by: Sunil Mushran <sunil.mushran@oracle.com>
---
 fs/ocfs2/cluster/tcp.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index aa75ca3..1361997 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -1759,6 +1759,7 @@ static int o2net_accept_one(struct socket *sock)
 	struct sockaddr_in sin;
 	struct socket *new_sock = NULL;
 	struct o2nm_node *node = NULL;
+	struct o2nm_node *local_node = NULL;
 	struct o2net_sock_container *sc = NULL;
 	struct o2net_node *nn;
 
@@ -1796,11 +1797,15 @@ static int o2net_accept_one(struct socket *sock)
 		goto out;
 	}
 
-	if (o2nm_this_node() > node->nd_num) {
-		mlog(ML_NOTICE, "unexpected connect attempted from a lower "
-		     "numbered node '%s' at " "%pI4:%d with num %u\n",
-		     node->nd_name, &sin.sin_addr.s_addr,
-		     ntohs(sin.sin_port), node->nd_num);
+	if (o2nm_this_node() >= node->nd_num) {
+		local_node = o2nm_get_node_by_num(o2nm_this_node());
+		mlog(ML_NOTICE, "unexpected connect attempt seen at node '%s' ("
+		     "%u, %pI4:%d) from node '%s' (%u, %pI4:%d)\n",
+		     local_node->nd_name, local_node->nd_num,
+		     &(local_node->nd_ipv4_address),
+		     ntohs(local_node->nd_ipv4_port),
+		     node->nd_name, node->nd_num, &sin.sin_addr.s_addr,
+		     ntohs(sin.sin_port));
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1857,6 +1862,8 @@ out:
 		sock_release(new_sock);
 	if (node)
 		o2nm_node_put(node);
+	if (local_node)
+		o2nm_node_put(local_node);
 	if (sc)
 		sc_put(sc);
 	return ret;
-- 
1.5.5

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Sunil Mushran]

Acked

On Aug 1, 2010, at 7:00 PM, Tristan Ye <tristan.ye@oracle.com> wrote:

> Currently, o2net_accept_one() is allowed to accept a connection from
> listening node itself, such a fake connection will not be successfully
> established due to no handshake detected afterwards, and later end up
> with triggering connecting worker in a loop.
> 
> We're going to fix this by treating such connection request as 'invalid',
> since we've got no chance of requesting connection from a node to itself
> in a OCFS2 cluster.
> 
> The fix doesn't hurt user's scan for o2net-listener, it always gets a
> successful connection from userpace.
> 
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> Acked-by: Sunil Mushran <sunil.mushran@oracle.com>
> ---
> fs/ocfs2/cluster/tcp.c |   17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index aa75ca3..1361997 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -1759,6 +1759,7 @@ static int o2net_accept_one(struct socket *sock)
>    struct sockaddr_in sin;
>    struct socket *new_sock = NULL;
>    struct o2nm_node *node = NULL;
> +    struct o2nm_node *local_node = NULL;
>    struct o2net_sock_container *sc = NULL;
>    struct o2net_node *nn;
> 
> @@ -1796,11 +1797,15 @@ static int o2net_accept_one(struct socket *sock)
>        goto out;
>    }
> 
> -    if (o2nm_this_node() > node->nd_num) {
> -        mlog(ML_NOTICE, "unexpected connect attempted from a lower "
> -             "numbered node '%s' at " "%pI4:%d with num %u\n",
> -             node->nd_name, &sin.sin_addr.s_addr,
> -             ntohs(sin.sin_port), node->nd_num);
> +    if (o2nm_this_node() >= node->nd_num) {
> +        local_node = o2nm_get_node_by_num(o2nm_this_node());
> +        mlog(ML_NOTICE, "unexpected connect attempt seen at node '%s' ("
> +             "%u, %pI4:%d) from node '%s' (%u, %pI4:%d)\n",
> +             local_node->nd_name, local_node->nd_num,
> +             &(local_node->nd_ipv4_address),
> +             ntohs(local_node->nd_ipv4_port),
> +             node->nd_name, node->nd_num, &sin.sin_addr.s_addr,
> +             ntohs(sin.sin_port));
>        ret = -EINVAL;
>        goto out;
>    }
> @@ -1857,6 +1862,8 @@ out:
>        sock_release(new_sock);
>    if (node)
>        o2nm_node_put(node);
> +    if (local_node)
> +        o2nm_node_put(local_node);
>    if (sc)
>        sc_put(sc);
>    return ret;
> -- 
> 1.5.5
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Joel Becker]

On Mon, Aug 02, 2010 at 10:00:26AM +0800, Tristan Ye wrote:
> Currently, o2net_accept_one() is allowed to accept a connection from
> listening node itself, such a fake connection will not be successfully
> established due to no handshake detected afterwards, and later end up
> with triggering connecting worker in a loop.
> 
> We're going to fix this by treating such connection request as 'invalid',
> since we've got no chance of requesting connection from a node to itself
> in a OCFS2 cluster.
> 
> The fix doesn't hurt user's scan for o2net-listener, it always gets a
> successful connection from userpace.
> 
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> Acked-by: Sunil Mushran <sunil.mushran@oracle.com>

This patch is now in the fixes branch of ocfs2.git.

Joel

-- 

"There are only two ways to live your life. One is as though nothing
 is a miracle. The other is as though everything is a miracle."
        - Albert Einstein

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Sunil Mushran]

You can tighten it a little more. You can add my Acked-by
to the next patch.

On 07/30/2010 01:25 AM, Tristan Ye wrote:
> Currently, o2net_accept_one() is allowed to accept a connection from
> listening node itself, such a fake connection will not be successfully
> established due to no handshake detected afterwards, and later end up
> with triggering connecting worker in a loop.
>
> We're going to fix this by treating such connection request as 'invalid',
> since we've got no chance of requesting connection from a node to itself
> in a OCFS2 cluster.
>
> The fix doesn't hurt user's scan for o2net-listener, it always gets a
> successful connection from userpace.
>
> Signed-off-by: Tristan Ye<tristan.ye@oracle.com>
> ---
>   fs/ocfs2/cluster/tcp.c |   16 +++++++++++-----
>   1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index aa75ca3..7a17e91 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -1759,6 +1759,7 @@ static int o2net_accept_one(struct socket *sock)
>   	struct sockaddr_in sin;
>   	struct socket *new_sock = NULL;
>   	struct o2nm_node *node = NULL;
> +	struct o2nm_node *local_node = o2nm_get_node_by_num(o2nm_this_node());
>    
+	struct o2nm_node *local_node = NULL;


>   	struct o2net_sock_container *sc = NULL;
>   	struct o2net_node *nn;
>
> @@ -1796,11 +1797,14 @@ static int o2net_accept_one(struct socket *sock)
>   		goto out;
>   	}
>
> -	if (o2nm_this_node()>  node->nd_num) {
> -		mlog(ML_NOTICE, "unexpected connect attempted from a lower "
> -		     "numbered node '%s' at " "%pI4:%d with num %u\n",
> -		     node->nd_name,&sin.sin_addr.s_addr,
> -		     ntohs(sin.sin_port), node->nd_num);
> +	if (o2nm_this_node()>= node->nd_num) {
>    

+		   local_node = o2nm_get_node_by_num(o2nm_this_node());


> +		mlog(ML_NOTICE, "unexpected connect attempt seen at node '%s' ("
> +		     "%u, %pI4:%d) from node '%s' (%u, %pI4:%d)\n",
> +		     local_node->nd_name, local_node->nd_num,
> +		&(local_node->nd_ipv4_address),
> +		     ntohs(local_node->nd_ipv4_port),
> +		     node->nd_name, node->nd_num,&sin.sin_addr.s_addr,
> +		     ntohs(sin.sin_port));
>   		ret = -EINVAL;
>   		goto out;
>   	}
> @@ -1857,6 +1861,8 @@ out:
>   		sock_release(new_sock);
>   	if (node)
>   		o2nm_node_put(node);
> +	if (local_node)
> +		o2nm_node_put(local_node);
>   	if (sc)
>   		sc_put(sc);
>   	return ret;
>    

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Tristan Ye]

Currently, o2net_accept_one() is allowed to accept a connection from
listening node itself, such a fake connection will not be successfully
established due to no handshake detected afterwards, and later end up
with triggering connecting worker in a loop.

We're going to fix this by treating such connection request as 'invalid',
since we've got no chance of requesting connection from a node to itself
in a OCFS2 cluster.

The fix doesn't hurt user's scan for o2net-listener, it always gets a
successful connection from userpace.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
---
 fs/ocfs2/cluster/tcp.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index aa75ca3..7a17e91 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -1759,6 +1759,7 @@ static int o2net_accept_one(struct socket *sock)
 	struct sockaddr_in sin;
 	struct socket *new_sock = NULL;
 	struct o2nm_node *node = NULL;
+	struct o2nm_node *local_node = o2nm_get_node_by_num(o2nm_this_node());
 	struct o2net_sock_container *sc = NULL;
 	struct o2net_node *nn;
 
@@ -1796,11 +1797,14 @@ static int o2net_accept_one(struct socket *sock)
 		goto out;
 	}
 
-	if (o2nm_this_node() > node->nd_num) {
-		mlog(ML_NOTICE, "unexpected connect attempted from a lower "
-		     "numbered node '%s' at " "%pI4:%d with num %u\n",
-		     node->nd_name, &sin.sin_addr.s_addr,
-		     ntohs(sin.sin_port), node->nd_num);
+	if (o2nm_this_node() >= node->nd_num) {
+		mlog(ML_NOTICE, "unexpected connect attempt seen at node '%s' ("
+		     "%u, %pI4:%d) from node '%s' (%u, %pI4:%d)\n",
+		     local_node->nd_name, local_node->nd_num,
+		     &(local_node->nd_ipv4_address),
+		     ntohs(local_node->nd_ipv4_port),
+		     node->nd_name, node->nd_num, &sin.sin_addr.s_addr,
+		     ntohs(sin.sin_port));
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1857,6 +1861,8 @@ out:
 		sock_release(new_sock);
 	if (node)
 		o2nm_node_put(node);
+	if (local_node)
+		o2nm_node_put(local_node);
 	if (sc)
 		sc_put(sc);
 	return ret;
-- 
1.5.5

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[tristan]

Sunil Mushran wrote:
> unexpected connect attempt seen at node x (num, ip:port) from node y (num, ip:port). 
>
> The above reads better, don't you think. Lower or same are software bugs. Not a user error. 

That makes sense.

>
> Btw, one test you could do is race nc with mount. Run nc in a loop while mounting the first device. nc should ping a node that has it mounted. Test with higher to lower and reverse. 

Yeah, to stress and race nc with mount is a good way to test o2net's 
listening function.

>
> On Jul 29, 2010, at 4:04 AM, Tristan Ye <tristan.ye@oracle.com> wrote:
>
>> Currently, o2net_accept_one() is allowed to accept a connection from
>> listening node itself, such a fake connection will not be successfully
>> established due to no handshake detected afterwards, and later end up
>> with triggering connecting worker in a loop.
>>
>> We're going to fix this by treating such connection request as 'invalid',
>> since we've got no chance of requesting connection from a node to itself
>> in a OCFS2 cluster.
>>
>> The fix doesn't hurt user's scan for o2net-listener, it always gets a
>> successful connection from userpace.
>>
>> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
>> ---
>> fs/ocfs2/cluster/tcp.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
>> index aa75ca3..7e4e451 100644
>> --- a/fs/ocfs2/cluster/tcp.c
>> +++ b/fs/ocfs2/cluster/tcp.c
>> @@ -1796,9 +1796,9 @@ static int o2net_accept_one(struct socket *sock)
>>        goto out;
>>    }
>>
>> -    if (o2nm_this_node() > node->nd_num) {
>> +    if (o2nm_this_node() >= node->nd_num) {
>>        mlog(ML_NOTICE, "unexpected connect attempted from a lower "
>> -             "numbered node '%s' at " "%pI4:%d with num %u\n",
>> +             "or same numbered node '%s' at " "%pI4:%d with num %u\n",
>>             node->nd_name, &sin.sin_addr.s_addr,
>>             ntohs(sin.sin_port), node->nd_num);
>>        ret = -EINVAL;
>> -- 
>> 1.5.5
>>
>>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Sunil Mushran]

unexpected connect attempt seen at node x (num, ip:port) from node y (num, ip:port). 

The above reads better, don't you think. Lower or same are software bugs. Not a user error. 

Btw, one test you could do is race nc with mount. Run nc in a loop while mounting the first device. nc should ping a node that has it mounted. Test with higher to lower and reverse. 

On Jul 29, 2010, at 4:04 AM, Tristan Ye <tristan.ye@oracle.com> wrote:

> Currently, o2net_accept_one() is allowed to accept a connection from
> listening node itself, such a fake connection will not be successfully
> established due to no handshake detected afterwards, and later end up
> with triggering connecting worker in a loop.
> 
> We're going to fix this by treating such connection request as 'invalid',
> since we've got no chance of requesting connection from a node to itself
> in a OCFS2 cluster.
> 
> The fix doesn't hurt user's scan for o2net-listener, it always gets a
> successful connection from userpace.
> 
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> ---
> fs/ocfs2/cluster/tcp.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index aa75ca3..7e4e451 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -1796,9 +1796,9 @@ static int o2net_accept_one(struct socket *sock)
>        goto out;
>    }
> 
> -    if (o2nm_this_node() > node->nd_num) {
> +    if (o2nm_this_node() >= node->nd_num) {
>        mlog(ML_NOTICE, "unexpected connect attempted from a lower "
> -             "numbered node '%s' at " "%pI4:%d with num %u\n",
> +             "or same numbered node '%s' at " "%pI4:%d with num %u\n",
>             node->nd_name, &sin.sin_addr.s_addr,
>             ntohs(sin.sin_port), node->nd_num);
>        ret = -EINVAL;
> -- 
> 1.5.5
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[Ocfs2-devel] [PATCH 1/1] O2net: Disallow o2net accept connection request from itself.

[Tristan Ye]

Currently, o2net_accept_one() is allowed to accept a connection from
listening node itself, such a fake connection will not be successfully
established due to no handshake detected afterwards, and later end up
with triggering connecting worker in a loop.

We're going to fix this by treating such connection request as 'invalid',
since we've got no chance of requesting connection from a node to itself
in a OCFS2 cluster.

The fix doesn't hurt user's scan for o2net-listener, it always gets a
successful connection from userpace.

Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
---
 fs/ocfs2/cluster/tcp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index aa75ca3..7e4e451 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -1796,9 +1796,9 @@ static int o2net_accept_one(struct socket *sock)
 		goto out;
 	}
 
-	if (o2nm_this_node() > node->nd_num) {
+	if (o2nm_this_node() >= node->nd_num) {
 		mlog(ML_NOTICE, "unexpected connect attempted from a lower "
-		     "numbered node '%s' at " "%pI4:%d with num %u\n",
+		     "or same numbered node '%s' at " "%pI4:%d with num %u\n",
 		     node->nd_name, &sin.sin_addr.s_addr,
 		     ntohs(sin.sin_port), node->nd_num);
 		ret = -EINVAL;
-- 
1.5.5