glusterfs.git/rpc/rpc-lib/src/rpcsvc.c, branch v3.8.3

glusterd: add defence mechanism to avoid brick port clashes

2016-05-10T07:11:33+00:00

Intro:
Currently glusterd maintain the portmap registry which contains ports that
are free to use between 49152 - 65535, this registry is initialized
once, and updated accordingly as an then when glusterd sees they are been
used.

Glusterd first checks for a port within the portmap registry and gets a FREE
port marked in it, then checks if that port is currently free using a connect()
function then passes it to brick process which have to bind on it.

Problem:
We see that there is a time gap between glusterd checking the port with
connect() and brick process actually binding on it. In this time gap it could
be so possible that any process would have occupied this port because of which
brick will fail to bind and exit.

Case 1:
To avoid the gluster client process occupying the port supplied by glusterd :

we have separated the client port map range with brick port map range more @
http://review.gluster.org/#/c/13998/

Case 2: (Handled by this patch)
To avoid the other foreign process occupying the port supplied by glusterd :

To handle above situation this patch implements a mechanism to return EADDRINUSE
error code to glusterd, upon which a new port is allocated and try to restart
the brick process with the newly allocated port.

Note: Incase of glusterd restarts i.e. runner_run_nowait() there is no way to
handle Case 2, becuase runner_run_nowait() will not wait to get the return/exit
code of the executed command (brick process). Hence as of now in such case,
we cannot know with what error the brick has failed to connect.

This patch also fix the runner_end() to perform some cleanup w.r.t
return values.

Backport of:
> Change-Id: Iec52e7f5d87ce938d173f8ef16aa77fd573f2c5e
> BUG: 1322805
> Signed-off-by: Prasanna Kumar Kalever 
> Reviewed-on: http://review.gluster.org/14043
> Tested-by: Prasanna Kumar Kalever 
> Reviewed-by: Atin Mukherjee 
> Smoke: Gluster Build System 
> NetBSD-regression: NetBSD Build System 
> CentOS-regression: Gluster Build System 
> Reviewed-by: Raghavendra G 

Change-Id: Id7d8351a0082b44310177e714edc0571ad0f7195
BUG: 1333711
Signed-off-by: Prasanna Kumar Kalever 
Reviewed-on: http://review.gluster.org/14235
Tested-by: Prasanna Kumar Kalever 
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
CentOS-regression: Gluster Build System 
Reviewed-by: Atin Mukherjee

rpc : build_prog_details should iterate program list inside critical section

2016-02-22T18:07:05+00:00

While I was analyzing a glusterd crash from free_prog_details, a code
walkthrough detects that we iterate over the rpc svc program list without been
inside the criticial section. This opens up a possibility of a crash when there
is a concurrent writer updating the same list. Solution is to read the list
inside lock.

Change-Id: Ib4b4b0022a9535e139cd3c00574aab23f07aa9d2
BUG: 1306560
Signed-off-by: Atin Mukherjee 
Reviewed-on: http://review.gluster.org/13428
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
Reviewed-by: Niels de Vos 
CentOS-regression: Gluster Build System 
Reviewed-by: Raghavendra G

all: reduce "inline" usage

2015-09-01T11:55:15+00:00

There are three kinds of inline functions: plain inline, extern inline,
and static inline.  All three have been removed from .c files, except
those in "contrib" which aren't our problem.  Inlines in .h files, which
are overwhelmingly "static inline" already, have generally been left
alone.  Over time we should be able to "lower" these into .c files, but
that has to be done in a case-by-case fashion requiring more manual
effort.  This part was easy to do automatically without (as far as I can
tell) any ill effect.

In the process, several pieces of dead code were flagged by the
compiler, and were removed.

Change-Id: I56a5e614735c9e0a6ee420dab949eac22e25c155
BUG: 1245331
Signed-off-by: Jeff Darcy 
Reviewed-on: http://review.gluster.org/11769
Tested-by: NetBSD Build System 
Reviewed-by: Dan Lambright 
Reviewed-by: Kaleb KEITHLEY 
Reviewed-by: Niels de Vos 
Reviewed-by: Raghavendra Bhat 
Reviewed-by: Venky Shankar

rpc: check for unprivileged port should start at 1024

2015-07-30T10:19:45+00:00

The current check for unprivileged port starts beyond 1024
i.e. port > 1024
The actual check should start at 1024
i.e. port >= 1024

Change-Id: I78aff3025891e3e78ca6a9a670c89571752157df
BUG: 1247930
Signed-off-by: Milind Changire 
Reviewed-on: http://review.gluster.org/11788
Tested-by: NetBSD Build System 
Reviewed-by: Raghavendra G

rpc: fix binding brick issue while bind-insecure is enabled

2015-07-27T04:31:39+00:00

problem:

When bind-insecure is turned on (which is the default now), it may happen
that brick is not able to bind to port assigned by Glusterd for example
49192-49195...

It seems to occur because the rpc_clnt connections are binding to ports in
the same range. so brick fails to bind to a port which is already used by
someone else

solution:

fix for now is to  make rpc_clnt to get port numbers from 65535 in a descending
order, as a result port clash is minimized

other fixes:

previously rdma binds to port >= 1024 if it cannot find a free port < 1024,
even when bind insecure was turned off(ref to commit '0e3fd04e'), this patch
add's a check for bind-insecure in gf_rdma_client_bind function

This patch also re-enable bind-insecure and allow insecure by default which was
reverted (ref: commit cef1720) previously

Change-Id: Ia1cfa93c5454e2ae0ff57813689b75de282ebd07
BUG: 1238661
Signed-off-by: Prasanna Kumar Kalever 
Reviewed-on: http://review.gluster.org/11512
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G

rpcsvc: add request's address in log

2015-07-13T17:59:27+00:00

This helps in identifying the remote endpoint whose request arrived at
an non-privileged port.

Change-Id: Ie58155e00229cfcbd0ca5221d79e2e90efb3e2b3
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/11631
Reviewed-by: Atin Mukherjee 
Tested-by: NetBSD Build System 
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G

Revert "rpc: By default set allow-insecure, bind-insecure to on"

2015-07-02T15:44:38+00:00

This reverts commit 5bf6522562990f7490d9fc226d58f19891bfb7a4.

This patch introduced a regression where client no longer binds to 
privileged port. This is causing lots of regressions. Hence reverting 
this patch for now and will be resent after suitable modifications.

Change-Id: Id697013ca6474e9c192e8f58c8179522fa5d397e
Reviewed-on: http://review.gluster.org/11507
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G 
Tested-by: Raghavendra G

rpc: By default set allow-insecure, bind-insecure to on

2015-06-30T08:58:13+00:00

since we now use SSL (Secure Sockets Layer) for the security issues, the patch
changes the default setting to allow connections/requests from non-privilaged
ports by setting allow-insecure and bind-insecure to 1

Also added bind functionality for insecure binding which can select from
available local ports dynamically

BUG: 1232658
Change-Id: I927e112223f33611452093e38cd846a0b9347e57
Signed-off-by: Pranith Kumar K 
Signed-off-by: Prasanna Kumar Kalever 
Reviewed-on: http://review.gluster.org/11039
Tested-by: NetBSD Build System 
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G

build: do not #include "config.h" in each file

2015-05-29T12:43:17+00:00

Instead of including config.h in each file, and have the additional
config.h included from the compiler commandline (-include option).

When a .c file tests for a certain #define, and config.h was not
included, incorrect assumtions were made. With this change, it can not
happen again.

BUG: 1222319
Change-Id: I4f9097b8740b81ecfe8b218d52ca50361f74cb64
Signed-off-by: Niels de Vos 
Reviewed-on: http://review.gluster.org/10808
Tested-by: Gluster Build System 
Tested-by: NetBSD Build System
Reviewed-by: Kaleb KEITHLEY 
Reviewed-by: Pranith Kumar Karampuri

rpc: Maintain separate xlator pointer in 'rpcsvc_state'

2015-05-04T11:08:26+00:00

The structure 'rpcsvc_state', which maintains rpc server
state had no separate pointer to track the translator.
It was using the mydata pointer itself. So callers were
forced to send xlator pointer as mydata which is opaque
(void pointer) by function prototype.

'rpcsvc_register_init' is setting svc->mydata with xlator
pointer. 'rpcsvc_register_notify' is overwriting svc->mydata
with mydata pointer. And rpc interprets svc->mydata as
xlator pointer internally. If someone passes non xlator
structure pointer to rpcsvc_register_notify as libgfchangelog
currently does, it might corrupt mydata. So interpreting opaque
mydata as xlator pointer is incorrect as it is caller's choice
to send mydata as any type of data to 'rpcsvc_register_notify'.

Maintaining two different pointers in 'rpcsvc_state' for xlator
and mydata solves the issue.

Change-Id: I7874933fefc68f3fe01d44f92016a8e4e9768378
BUG: 1215161
Signed-off-by: Kotresh HR 
Reviewed-on: http://review.gluster.org/10366
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G 
Tested-by: Raghavendra G