subject:"Re\: \[devel\] \[PATCH 1\/1\] mds\: Unset flow control env var \[#3109\]"

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

2019-10-31 Thread Minh Hon Chau


Hi Vu,

Please find comments below.

Thanks

Minh

On 31/10/19 6:15 pm, Nguyen Minh Vu wrote:

Hi Minh,

Ack with minor comments.

Regards, Vu

On 10/31/19 11:55 AM, Minh Chau wrote:

Patch unsets MDS_TIPC_FCTRL_ENABLED, MDS_TIPC_FCTRL_ACKTIMEOUT,
and MDS_TIPC_FCTRL_ACKSIZE to prevent child process inheritance.
---
  src/mds/mds_dt_tipc.c | 39 +--
  1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e7a7b48..096e4ca 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -167,6 +167,8 @@ uint32_t mdtm_global_frag_num;
    const unsigned int MAX_RECV_THRESHOLD = 30;
  uint8_t gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+int gl_mds_fctrl_acksize = -1;
+int gl_mds_fctrl_ackto = -1;
[Vu] Should we declare these ones as static variables if they are only 
used in this file ?

[M]: Yes, make them static
    static bool get_tipc_port_id(int sock, struct tipc_portid* 
port_id) {

  struct sockaddr_tipc addr;
@@ -347,32 +349,49 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, 
uint32_t *mds_tipc_ref)

  if ((ptr = getenv("MDS_TIPC_FCTRL_ENABLED")) != NULL) {
  if (atoi(ptr) == 1) {
  gl_mds_pro_ver = MDS_PROT_FCTRL;
-    int ackto = -1;
-    int acksize = -1;
  if ((ptr = getenv("MDS_TIPC_FCTRL_ACKTIMEOUT")) != NULL) {
-    ackto = atoi(ptr);
-    if (ackto == 0) {
+    gl_mds_fctrl_ackto = atoi(ptr);
+    if (gl_mds_fctrl_ackto == 0) {
  syslog(LOG_ERR, "MDTM:TIPC Invalid "
  "MDS_TIPC_FCTRL_ACKTIMEOUT, using 
default value");

-    ackto = -1;
+    gl_mds_fctrl_ackto = -1;
  }
  }
  if ((ptr = getenv("MDS_TIPC_FCTRL_ACKSIZE")) != NULL) {
-    acksize = atoi(ptr);
-    if (acksize == 0) {
+    gl_mds_fctrl_acksize = atoi(ptr);
+    if (gl_mds_fctrl_acksize == 0) {
  syslog(LOG_ERR, "MDTM:TIPC Invalid "
  "MDS_TIPC_FCTRL_ACKSIZE, using default 
value");

-    acksize = -1;
+    gl_mds_fctrl_acksize = -1;
  }
  }
-    mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, optval,
-    ackto, acksize, tipc_mcast_enabled);
+    /* unset the env var to prevent child process 
inheritance */

+    if (unsetenv("MDS_TIPC_FCTRL_ENABLED") != 0) {
+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ENABLED");

+    }
+    if (gl_mds_fctrl_ackto != -1 &&
+    unsetenv("MDS_TIPC_FCTRL_ACKTIMEOUT") != 0) {
+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKTIMEOUT");

+    }
+    if (gl_mds_fctrl_acksize != -1 &&
+    unsetenv("MDS_TIPC_FCTRL_ACKSIZE") != 0) {
+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKSIZE");

+    }
  } else {
+    gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
[Vu] This line may be not necessary as the default value of 
gl_mds_pro_ver is `MDS_PROT | MDS_VERSION`.
[M] It may be invalid value by setenv() in the scenario you suggested: 
Init/Finalize/Init with setenv(invalid value).

  syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
  "MDS_TIPC_FCTRL_ENABLED");
  }
  }
  +    if (gl_mds_pro_ver == MDS_PROT_FCTRL) {
+    mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, optval,
+    gl_mds_fctrl_ackto, gl_mds_fctrl_acksize, 
tipc_mcast_enabled);

+    }
+
  /* Create a task to receive the events and data */
  if (mdtm_create_rcv_task(tipc_cb.hdle_mdtm) != NCSCC_RC_SUCCESS) {
  syslog(LOG_ERR,






___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

2019-10-31 Thread Nguyen Minh Vu


Hi Minh,

Ack with minor comments.

Regards, Vu

On 10/31/19 11:55 AM, Minh Chau wrote:

Patch unsets MDS_TIPC_FCTRL_ENABLED, MDS_TIPC_FCTRL_ACKTIMEOUT,
and MDS_TIPC_FCTRL_ACKSIZE to prevent child process inheritance.
---
  src/mds/mds_dt_tipc.c | 39 +--
  1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e7a7b48..096e4ca 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -167,6 +167,8 @@ uint32_t mdtm_global_frag_num;
  
  const unsigned int MAX_RECV_THRESHOLD = 30;

  uint8_t gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
+int gl_mds_fctrl_acksize = -1;
+int gl_mds_fctrl_ackto = -1;
[Vu] Should we declare these ones as static variables if they are only 
used in this file ?
  
  static bool get_tipc_port_id(int sock, struct tipc_portid* port_id) {

struct sockaddr_tipc addr;
@@ -347,32 +349,49 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, uint32_t 
*mds_tipc_ref)
if ((ptr = getenv("MDS_TIPC_FCTRL_ENABLED")) != NULL) {
if (atoi(ptr) == 1) {
gl_mds_pro_ver = MDS_PROT_FCTRL;
-   int ackto = -1;
-   int acksize = -1;
if ((ptr = getenv("MDS_TIPC_FCTRL_ACKTIMEOUT")) != 
NULL) {
-   ackto = atoi(ptr);
-   if (ackto == 0) {
+   gl_mds_fctrl_ackto = atoi(ptr);
+   if (gl_mds_fctrl_ackto == 0) {
syslog(LOG_ERR, "MDTM:TIPC Invalid "
"MDS_TIPC_FCTRL_ACKTIMEOUT, 
using default value");
-   ackto = -1;
+   gl_mds_fctrl_ackto = -1;
}
}
if ((ptr = getenv("MDS_TIPC_FCTRL_ACKSIZE")) != NULL) {
-   acksize = atoi(ptr);
-   if (acksize == 0) {
+   gl_mds_fctrl_acksize = atoi(ptr);
+   if (gl_mds_fctrl_acksize == 0) {
syslog(LOG_ERR, "MDTM:TIPC Invalid "
"MDS_TIPC_FCTRL_ACKSIZE, 
using default value");
-   acksize = -1;
+   gl_mds_fctrl_acksize = -1;
}
}
-   mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, 
optval,
-   ackto, acksize, tipc_mcast_enabled);
+   /* unset the env var to prevent child process 
inheritance */
+   if (unsetenv("MDS_TIPC_FCTRL_ENABLED") != 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ENABLED");
+   }
+   if (gl_mds_fctrl_ackto != -1 &&
+   unsetenv("MDS_TIPC_FCTRL_ACKTIMEOUT") != 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKTIMEOUT");
+   }
+   if (gl_mds_fctrl_acksize != -1 &&
+   unsetenv("MDS_TIPC_FCTRL_ACKSIZE") != 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKSIZE");
+   }
} else {
+   gl_mds_pro_ver = MDS_PROT | MDS_VERSION;
[Vu] This line may be not necessary as the default value of 
gl_mds_pro_ver is `MDS_PROT | MDS_VERSION`.

syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
"MDS_TIPC_FCTRL_ENABLED");
}
}
  
+	if (gl_mds_pro_ver == MDS_PROT_FCTRL) {

+   mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, optval,
+   gl_mds_fctrl_ackto, gl_mds_fctrl_acksize, 
tipc_mcast_enabled);
+   }
+
/* Create a task to receive the events and data */
if (mdtm_create_rcv_task(tipc_cb.hdle_mdtm) != NCSCC_RC_SUCCESS) {
syslog(LOG_ERR,




___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

2019-10-30 Thread Minh Hon Chau


Hi Vu,

I think users will lose the value set in env var if users repeat 
Init/Finalize/Init. I have sent out V2.


Thanks

Minh

On 31/10/19 2:33 pm, Nguyen Minh Vu wrote:

Hi Minh,

Ack with one question.

What happens if user does following sequence:
1) Init service handle
--> Have this env variable set, then unset later on.
2) Finalize the handle

3) Init service handle
--> I am not sure if previous unset has any affects to tipc flow 
control from this point

e.g. has tipc flow control been disabled from previous unset?

Regards, Vu

On 10/31/19 5:32 AM, Minh Chau wrote:

Patch unsets MDS_TIPC_FCTRL_ENABLED, MDS_TIPC_FCTRL_ACKTIMEOUT,
and MDS_TIPC_FCTRL_ACKSIZE to prevent child process inheritance.
---
  src/mds/mds_dt_tipc.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e7a7b48..12b275d 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -367,6 +367,19 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, uint32_t 
*mds_tipc_ref)

  }
  mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, 
optval,

  ackto, acksize, tipc_mcast_enabled);
+    /* unset the env var to prevent child process 
inheritance */

+    if (unsetenv("MDS_TIPC_FCTRL_ENABLED") != 0) {
+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ENABLED");

+    }
+    if (ackto != -1 && unsetenv("MDS_TIPC_FCTRL_ACKTIMEOUT") 
!= 0) {

+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKTIMEOUT");

+    }
+    if (acksize != -1 && unsetenv("MDS_TIPC_FCTRL_ACKSIZE") 
!= 0) {

+    syslog(LOG_ERR,
+    "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKSIZE");

+    }
  } else {
  syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
  "MDS_TIPC_FCTRL_ENABLED");






___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

2019-10-30 Thread Nguyen Minh Vu


Hi Minh,

Ack with one question.

What happens if user does following sequence:
1) Init service handle
--> Have this env variable set, then unset later on.
2) Finalize the handle

3) Init service handle
--> I am not sure if previous unset has any affects to tipc flow control 
from this point

e.g. has tipc flow control been disabled from previous unset?

Regards, Vu

On 10/31/19 5:32 AM, Minh Chau wrote:

Patch unsets MDS_TIPC_FCTRL_ENABLED, MDS_TIPC_FCTRL_ACKTIMEOUT,
and MDS_TIPC_FCTRL_ACKSIZE to prevent child process inheritance.
---
  src/mds/mds_dt_tipc.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index e7a7b48..12b275d 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -367,6 +367,19 @@ uint32_t mdtm_tipc_init(NODE_ID nodeid, uint32_t 
*mds_tipc_ref)
}
mds_tipc_fctrl_initialize(tipc_cb.BSRsock, port_id, 
optval,
ackto, acksize, tipc_mcast_enabled);
+   /* unset the env var to prevent child process 
inheritance */
+   if (unsetenv("MDS_TIPC_FCTRL_ENABLED") != 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ENABLED");
+   }
+   if (ackto != -1 && 
unsetenv("MDS_TIPC_FCTRL_ACKTIMEOUT") != 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKTIMEOUT");
+   }
+   if (acksize != -1 && unsetenv("MDS_TIPC_FCTRL_ACKSIZE") 
!= 0) {
+   syslog(LOG_ERR,
+   "MDTM:TIPC Failed to unset 
MDS_TIPC_FCTRL_ACKSIZE");
+   }
} else {
syslog(LOG_ERR, "MDTM:TIPC Invalid value of"
"MDS_TIPC_FCTRL_ENABLED");




___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

Re: [devel] [PATCH 1/1] mds: Unset flow control env var [#3109]

4 matches

Site Navigation

Mail list logo

Footer information