Re: [devel] [PATCH 1/1] mds: Avoid message re-allocation [#3089]
Hi Minh,
Ack with comments inline.
Regards, Vu
On 11/25/19 1:12 PM, Minh Chau wrote:
The patch avoids message reallocation if enable
MDS_TIPC_FCTRL_ENABLED
---
src/mds/mds_dt_tipc.c| 27 ---
src/mds/mds_tipc_fctrl_msg.cc| 2 +-
src/mds/mds_tipc_fctrl_portid.cc | 9 +++--
3 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index fdf0da7..aa8d5c2 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -2644,7 +2644,7 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
if (req->snd_type == MDS_SENDTYPE_ACK ||
req->snd_type == MDS_SENDTYPE_RACK) {
uint8_t len = sum_mds_hdr_plus_mdtm_hdr_plus_len;
- uint8_t buffer_ack[len];
+ uint8_t* buffer_ack = calloc(1, len);
[Vu] Below this allocation, there are several error handlings, but not
free memory before returning.
Is that expected?
/* Add mds_hdr */
if (NCSCC_RC_SUCCESS !=
@@ -2667,7 +2667,11 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
m_MDS_LOG_DBG(
"MDTM:Sending message with Service Seqno=%d, TO
Dest_Tipc_id=<0x%08x:%u> ",
req->svc_seq_num, tipc_id.node, tipc_id.ref);
- return mdtm_sendto(buffer_ack, len, tipc_id);
+ status = mdtm_sendto(buffer_ack, len, tipc_id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(buffer_ack);
+ }
[Vu] Above allocation does not stick with `MDS_PROT_FCTRL` check, so if
the above condition check
gets failure, the allocated memory is leaked?
+ return status;
}
if (MDS_ENC_TYPE_FLAT == req->msg.encoding) {
@@ -2815,6 +2819,8 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
} else {
if (NCSCC_RC_SUCCESS !=
mdtm_sendto(body, len, tipc_id)) {
@@ -2824,9 +2830,12 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
+ }
}
- m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
+
return NCSCC_RC_SUCCESS;
}
} break;
@@ -2909,7 +2918,9 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
mds_free_direct_buff(
req->msg.data.buff_info.buff);
}
- free(body);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
return NCSCC_RC_SUCCESS;
} break;
@@ -3059,21 +3070,23 @@ uint32_t mdtm_frag_and_send(MDTM_SEND_REQ *req, uint32_t seq_num,
get_svc_names(req->src_svc_id),
req->src_svc_id,
get_svc_names(req->dest_svc_id),
req->dest_svc_id);
ret = mdtm_mcast_sendto(body, len_buf, req);
+ free(body);
} else {
m_MDS_LOG_DBG(
"MDTM:Sending message with Service Seqno=%d, Fragment
Seqnum=%d, frag_num=%d, TO Dest_Tipc_id=<0x%08x:%u>",
req->svc_seq_num, seq_num, frag_val,
id.node, id.ref);
ret = mdtm_sendto(body, len_buf, id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
}
if (ret != NCSCC_RC_SUCCESS) {
// Failed to send a fragmented msg, stop sending
m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
break;
}
Re: [devel] [PATCH 1/1] nid: fix unable to start UML cluster with tipc transport [#3122]
Hi Vu,
Sorry, I have comments inline.
Best Regards,
ThuanTr
-Original Message-
From: Tran Thuan
Sent: Monday, November 25, 2019 2:27 PM
To: 'Vu Minh Nguyen' ;
'thien.m.hu...@dektech.com.au'
Cc: 'opensaf-devel@lists.sourceforge.net'
Subject: RE: [PATCH 1/1] nid: fix unable to start UML cluster with tipc
transport [#3122]
Hi Vu,
ACK from me (code review).
Best Regards,
ThuanTr
-Original Message-
From: Vu Minh Nguyen
Sent: Monday, November 25, 2019 1:45 PM
To: thuan.t...@dektech.com.au; thien.m.hu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
Subject: [PATCH 1/1] nid: fix unable to start UML cluster with tipc transport
[#3122]
---
src/nid/configure_tipc.in | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/nid/configure_tipc.in b/src/nid/configure_tipc.in
index a63c97046..43ddb06e1 100644
--- a/src/nid/configure_tipc.in
+++ b/src/nid/configure_tipc.in
@@ -221,11 +221,13 @@ function tipc_duplicate_node_detect ()
function tipc_configure ()
{
echo "Inserting TIPC mdoule..."
-
-if ! test -f "$TIPC_MODULE" ; then
- modprobe tipc
+
+# Prefer using modprobe to insmod as modprobe takes care of
+# loading all dependencies if any. If any dependent module
+# has not yet loaded, insmod will get failed.
+if modprobe tipc ; then
[Thuan] ret_val=$?
RM_TIPC_MODULE="modprobe -r tipc"
-else
+else
insmod "$TIPC_MODULE"
[Thuan] ret_val=$?
RM_TIPC_MODULE="rmmod $TIPC_MODULE"
fi
ret_val=$?
[Thuan] Remove ret_val=$? here
if [ $ret_val -ne 0 ] ; then
logger -p user.err " TIPC Module could not be loaded "
exit 1
fi
--
2.17.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] nid: fix unable to start UML cluster with tipc transport [#3122]
Hi Vu,
ACK from me (code review).
Best Regards,
ThuanTr
-Original Message-
From: Vu Minh Nguyen
Sent: Monday, November 25, 2019 1:45 PM
To: thuan.t...@dektech.com.au; thien.m.hu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Vu Minh Nguyen
Subject: [PATCH 1/1] nid: fix unable to start UML cluster with tipc transport
[#3122]
---
src/nid/configure_tipc.in | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/nid/configure_tipc.in b/src/nid/configure_tipc.in
index a63c97046..43ddb06e1 100644
--- a/src/nid/configure_tipc.in
+++ b/src/nid/configure_tipc.in
@@ -221,11 +221,13 @@ function tipc_duplicate_node_detect ()
function tipc_configure ()
{
echo "Inserting TIPC mdoule..."
-
-if ! test -f "$TIPC_MODULE" ; then
- modprobe tipc
+
+# Prefer using modprobe to insmod as modprobe takes care of
+# loading all dependencies if any. If any dependent module
+# has not yet loaded, insmod will get failed.
+if modprobe tipc ; then
RM_TIPC_MODULE="modprobe -r tipc"
-else
+else
insmod "$TIPC_MODULE"
RM_TIPC_MODULE="rmmod $TIPC_MODULE"
fi
--
2.17.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] mds: Avoid message re-allocation [#3089]
Hi Minh,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Monday, November 25, 2019 1:13 PM
To: thuan.t...@dektech.com.au; gary@dektech.com.au;
vu.m.ngu...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/1] mds: Avoid message re-allocation [#3089]
The patch avoids message reallocation if enable
MDS_TIPC_FCTRL_ENABLED
---
src/mds/mds_dt_tipc.c| 27 ---
src/mds/mds_tipc_fctrl_msg.cc| 2 +-
src/mds/mds_tipc_fctrl_portid.cc | 9 +++--
3 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index fdf0da7..aa8d5c2 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -2644,7 +2644,7 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
if (req->snd_type == MDS_SENDTYPE_ACK ||
req->snd_type == MDS_SENDTYPE_RACK) {
uint8_t len = sum_mds_hdr_plus_mdtm_hdr_plus_len;
- uint8_t buffer_ack[len];
+ uint8_t* buffer_ack = calloc(1, len);
/* Add mds_hdr */
if (NCSCC_RC_SUCCESS !=
@@ -2667,7 +2667,11 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
m_MDS_LOG_DBG(
"MDTM:Sending message with Service Seqno=%d, TO
Dest_Tipc_id=<0x%08x:%u> ",
req->svc_seq_num, tipc_id.node, tipc_id.ref);
- return mdtm_sendto(buffer_ack, len, tipc_id);
+ status = mdtm_sendto(buffer_ack, len, tipc_id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(buffer_ack);
+ }
+ return status;
}
if (MDS_ENC_TYPE_FLAT == req->msg.encoding) {
@@ -2815,6 +2819,8 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
} else {
if (NCSCC_RC_SUCCESS !=
mdtm_sendto(body, len, tipc_id)) {
@@ -2824,9 +2830,12 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
+ }
}
- m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
+
return NCSCC_RC_SUCCESS;
}
} break;
@@ -2909,7 +2918,9 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
mds_free_direct_buff(
req->msg.data.buff_info.buff);
}
- free(body);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
return NCSCC_RC_SUCCESS;
} break;
@@ -3059,21 +3070,23 @@ uint32_t mdtm_frag_and_send(MDTM_SEND_REQ *req,
uint32_t seq_num,
get_svc_names(req->src_svc_id),
req->src_svc_id,
get_svc_names(req->dest_svc_id),
req->dest_svc_id);
ret = mdtm_mcast_sendto(body, len_buf, req);
+ free(body);
} else {
m_MDS_LOG_DBG(
"MDTM:Sending message with Service
Seqno=%d, Fragment Seqnum=%d, frag_num=%d, TO
Dest_Tipc_id=<0x%08x:%u>",
req->svc_seq_num, seq_num, frag_val,
id.node, id.ref);
ret = mdtm_sendto(body, len_buf, id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
}
if (ret != NCSCC_RC_SUCCESS) {
// Failed to send a fragmented msg, stop sending
m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
break;
}
[devel] [PATCH 0/1] Review Request for amfd: not accept lock-in admin op if presence msg not processed [#3121]
Summary: amfd: not accept lock-in admin op if presence msg not processed [#3121] Review request for Ticket(s): 3121 Peer Reviewer(s): Gary,Minh,Thuan Pull request to: Thuan Affected branch(es): develop Development branch: ticket-3121 Base revision: c6c7e77292d622ee042476bb0815feae51dd0cba Personal repository: git://git.code.sf.net/u/thangng/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - revision 75bc22f743793f2ffb0eb22fa294203be857d6ea Author: thang.d.nguyen Date: Mon, 25 Nov 2019 12:03:43 +0700 amfd: not accept lock-in admin op if presence msg not processed [#3121] AMFD should not accept lock-in admin op on SU if the presence msg has already sent to that SU. Complete diffstat: -- src/amf/amfd/sgproc.cc | 1 + src/amf/amfd/su.cc | 13 + src/amf/amfd/su.h | 2 ++ 3 files changed, 16 insertions(+) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - Acked from reviewer Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] amfd: not accept lock-in admin op if presence msg not processed [#3121]
AMFD should not accept lock-in admin op on SU if the presence msg
has already sent to that SU.
---
src/amf/amfd/sgproc.cc | 1 +
src/amf/amfd/su.cc | 13 +
src/amf/amfd/su.h | 2 ++
3 files changed, 16 insertions(+)
diff --git a/src/amf/amfd/sgproc.cc b/src/amf/amfd/sgproc.cc
index ddd825d44..8aeb9ec3c 100644
--- a/src/amf/amfd/sgproc.cc
+++ b/src/amf/amfd/sgproc.cc
@@ -2126,6 +2126,7 @@ uint32_t avd_sg_app_su_inst_func(AVD_CL_CB *cb, AVD_SG
*sg) {
}
} else {
if (avd_snd_presence_msg(cb, i_su, false) == NCSCC_RC_SUCCESS) {
+ i_su->is_presence_msg_processed = true;
num_try_insvc_su++;
}
}
diff --git a/src/amf/amfd/su.cc b/src/amf/amfd/su.cc
index 8c8ef9d4f..494022893 100644
--- a/src/amf/amfd/su.cc
+++ b/src/amf/amfd/su.cc
@@ -51,6 +51,7 @@ void AVD_SU::initialize() {
term_state = false;
su_switch = AVSV_SI_TOGGLE_STABLE;
su_is_external = false;
+ is_presence_msg_processed = false;
su_act_state = 0;
sg_of_su = nullptr;
su_on_node = nullptr;
@@ -810,6 +811,12 @@ void AVD_SU::set_pres_state(SaAmfPresenceStateT
pres_state) {
*/
return;
+ if ((pres_state == SA_AMF_PRESENCE_INSTANTIATED) ||
+ (pres_state == SA_AMF_PRESENCE_INSTANTIATION_FAILED) ||
+ (pres_state == SA_AMF_PRESENCE_TERMINATION_FAILED)) {
+this->is_presence_msg_processed = false;
+ }
+
osafassert(pres_state <= SA_AMF_PRESENCE_TERMINATION_FAILED);
TRACE_ENTER2("'%s' %s => %s", name.c_str(),
avd_pres_state_name[saAmfSUPresenceState],
@@ -1085,6 +1092,12 @@ void AVD_SU::lock_instantiation(SaImmOiHandleT
immoi_handle,
goto done;
}
+ if (is_presence_msg_processed == true) {
+report_admin_op_error(immoi_handle, invocation, SA_AIS_ERR_TRY_AGAIN,
+ nullptr, "'%s' instantiate not done", name.c_str());
+goto done;
+ }
+
if (list_of_susi != nullptr) {
report_admin_op_error(immoi_handle, invocation, SA_AIS_ERR_TRY_AGAIN,
nullptr, "SIs still assigned to this SU '%s'",
diff --git a/src/amf/amfd/su.h b/src/amf/amfd/su.h
index 7afc5abee..722c68b9c 100644
--- a/src/amf/amfd/su.h
+++ b/src/amf/amfd/su.h
@@ -87,6 +87,8 @@ class AVD_SU {
bool su_is_external; /* indicates if this SU is external */
+ bool is_presence_msg_processed; /* indicate inst msg sent to nd */
+
int su_act_state; // not used, kept for EDU, remove later
bool wait_for_contained_to_quiesce;
--
2.17.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for nid: fix unable to start UML cluster with tipc transport [#3122]
Summary: nid: fix unable to start UML cluster with tipc transport [#3122] Review request for Ticket(s): 3122 Peer Reviewer(s): Thuan, Thien Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3122 Base revision: c6c7e77292d622ee042476bb0815feae51dd0cba Personal repository: git://git.code.sf.net/u/winhvu/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts y SAF servicesn OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision c9e55e13f42a571a109c3d5e0484c58d3791dadb Author: Vu Minh Nguyen Date: Mon, 25 Nov 2019 13:41:43 +0700 nid: fix unable to start UML cluster with tipc transport [#3122] Complete diffstat: -- src/nid/configure_tipc.in | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Testing Commands: - Build UML cluster with tipc transport mode Testing, Expected Results: -- Cluster starts up normally Conditions of Submission: - Ack from reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 n n powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] nid: fix unable to start UML cluster with tipc transport [#3122]
---
src/nid/configure_tipc.in | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/nid/configure_tipc.in b/src/nid/configure_tipc.in
index a63c97046..43ddb06e1 100644
--- a/src/nid/configure_tipc.in
+++ b/src/nid/configure_tipc.in
@@ -221,11 +221,13 @@ function tipc_duplicate_node_detect ()
function tipc_configure ()
{
echo "Inserting TIPC mdoule..."
-
-if ! test -f "$TIPC_MODULE" ; then
- modprobe tipc
+
+# Prefer using modprobe to insmod as modprobe takes care of
+# loading all dependencies if any. If any dependent module
+# has not yet loaded, insmod will get failed.
+if modprobe tipc ; then
RM_TIPC_MODULE="modprobe -r tipc"
-else
+else
insmod "$TIPC_MODULE"
RM_TIPC_MODULE="rmmod $TIPC_MODULE"
fi
--
2.17.1
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for mds: Avoid message re-allocation [#3089]
Summary: mds: Avoid message re-allocation [#3089] Review request for Ticket(s): 3089 Peer Reviewer(s): Thuan, Gary, Vu Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3089 Base revision: c6c7e77292d622ee042476bb0815feae51dd0cba Personal repository: git://git.code.sf.net/u/minh-chau/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 18a3387b6331ec37ea47be47d1013afaee91d649 Author: Minh Chau Date: Mon, 25 Nov 2019 17:06:20 +1100 mds: Avoid message re-allocation [#3089] The patch avoids message reallocation if enable MDS_TIPC_FCTRL_ENABLED Complete diffstat: -- src/mds/mds_dt_tipc.c| 27 --- src/mds/mds_tipc_fctrl_msg.cc| 2 +- src/mds/mds_tipc_fctrl_portid.cc | 9 +++-- 3 files changed, 24 insertions(+), 14 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] mds: Avoid message re-allocation [#3089]
The patch avoids message reallocation if enable
MDS_TIPC_FCTRL_ENABLED
---
src/mds/mds_dt_tipc.c| 27 ---
src/mds/mds_tipc_fctrl_msg.cc| 2 +-
src/mds/mds_tipc_fctrl_portid.cc | 9 +++--
3 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/mds/mds_dt_tipc.c b/src/mds/mds_dt_tipc.c
index fdf0da7..aa8d5c2 100644
--- a/src/mds/mds_dt_tipc.c
+++ b/src/mds/mds_dt_tipc.c
@@ -2644,7 +2644,7 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
if (req->snd_type == MDS_SENDTYPE_ACK ||
req->snd_type == MDS_SENDTYPE_RACK) {
uint8_t len = sum_mds_hdr_plus_mdtm_hdr_plus_len;
- uint8_t buffer_ack[len];
+ uint8_t* buffer_ack = calloc(1, len);
/* Add mds_hdr */
if (NCSCC_RC_SUCCESS !=
@@ -2667,7 +2667,11 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
m_MDS_LOG_DBG(
"MDTM:Sending message with Service Seqno=%d, TO
Dest_Tipc_id=<0x%08x:%u> ",
req->svc_seq_num, tipc_id.node, tipc_id.ref);
- return mdtm_sendto(buffer_ack, len, tipc_id);
+ status = mdtm_sendto(buffer_ack, len, tipc_id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(buffer_ack);
+ }
+ return status;
}
if (MDS_ENC_TYPE_FLAT == req->msg.encoding) {
@@ -2815,6 +2819,8 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
} else {
if (NCSCC_RC_SUCCESS !=
mdtm_sendto(body, len, tipc_id)) {
@@ -2824,9 +2830,12 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
free(body);
return NCSCC_RC_FAILURE;
}
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ m_MMGR_FREE_BUFR_LIST(usrbuf);
+ free(body);
+ }
}
- m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
+
return NCSCC_RC_SUCCESS;
}
} break;
@@ -2909,7 +2918,9 @@ uint32_t mds_mdtm_send_tipc(MDTM_SEND_REQ *req)
mds_free_direct_buff(
req->msg.data.buff_info.buff);
}
- free(body);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
return NCSCC_RC_SUCCESS;
} break;
@@ -3059,21 +3070,23 @@ uint32_t mdtm_frag_and_send(MDTM_SEND_REQ *req,
uint32_t seq_num,
get_svc_names(req->src_svc_id),
req->src_svc_id,
get_svc_names(req->dest_svc_id),
req->dest_svc_id);
ret = mdtm_mcast_sendto(body, len_buf, req);
+ free(body);
} else {
m_MDS_LOG_DBG(
"MDTM:Sending message with Service
Seqno=%d, Fragment Seqnum=%d, frag_num=%d, TO Dest_Tipc_id=<0x%08x:%u>",
req->svc_seq_num, seq_num, frag_val,
id.node, id.ref);
ret = mdtm_sendto(body, len_buf, id);
+ if (gl_mds_pro_ver != MDS_PROT_FCTRL) {
+ free(body);
+ }
}
if (ret != NCSCC_RC_SUCCESS) {
// Failed to send a fragmented msg, stop sending
m_MMGR_FREE_BUFR_LIST(usrbuf);
- free(body);
break;
}
m_MMGR_REMOVE_FROM_START(, len_buf - hdr_plus);
- free(body);
len = len - (len_buf - hdr_plus);
if (len == 0)
break;
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index
Re: [devel] [PATCH 1/1] mds: Reduce mds logging [#3120]
Hi Minh,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Minh Chau
Sent: Monday, November 25, 2019 7:53 AM
To: thuan.t...@dektech.com.au; vu.m.ngu...@dektech.com.au;
gary@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net; Minh Chau
Subject: [PATCH 1/1] mds: Reduce mds logging [#3120]
The logging of broadcast/multicast is currently logged with
NOTIFY as mds does not support broadcast/multicast message,
so the logging would be helpful in some cases. However, the
mds.log may be located in nfs file system, and this logging
may cause high rate traffic towards nfs file system.
This patch moves the logging to DEBUG for broadcast/multicast
message, and for adding/removal mds service.
---
src/mds/mds_tipc_fctrl_intf.cc | 4 ++--
src/mds/mds_tipc_fctrl_portid.cc | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index dd8d80d..0e3230a 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -390,7 +390,7 @@ uint32_t mds_tipc_fctrl_portid_up(struct tipc_portid id,
uint32_t type) {
id.node, id.ref, svc_id, portid->svc_cnt_);
} else {
portid->svc_cnt_++;
-m_MDS_LOG_NOTIFY("FCTRL: Add svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
+m_MDS_LOG_DBG("FCTRL: Add svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
id.node, id.ref, svc_id, portid->svc_cnt_);
}
@@ -410,7 +410,7 @@ uint32_t mds_tipc_fctrl_portid_down(struct tipc_portid id,
uint32_t type) {
TipcPortId *portid = portid_lookup(id);
if (portid != nullptr) {
portid->svc_cnt_--;
-m_MDS_LOG_NOTIFY("FCTRL: Remove svc[node:%x, ref:%u svc_id:%u],
svc_cnt:%u",
+m_MDS_LOG_DBG("FCTRL: Remove svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
id.node, id.ref, svc_id, portid->svc_cnt_);
}
portid_map_mutex.unlock();
diff --git a/src/mds/mds_tipc_fctrl_portid.cc b/src/mds/mds_tipc_fctrl_portid.cc
index 724eb7b..316e1ba 100644
--- a/src/mds/mds_tipc_fctrl_portid.cc
+++ b/src/mds/mds_tipc_fctrl_portid.cc
@@ -298,7 +298,7 @@ uint32_t TipcPortId::ReceiveData(uint32_t mseq, uint16_t
mfrag,
}
}
if (rcving_mbcast_ == true) {
- m_MDS_LOG_NOTIFY("FCTRL: [me] <-- [node:%x, ref:%u], "
+ m_MDS_LOG_DBG("FCTRL: [me] <-- [node:%x, ref:%u], "
"RcvData[mseq:%u, mfrag:%u, fseq:%u], "
"rcvwnd[acked:%u, rcv:%u, nacked:%" PRIu64 "], "
"Ignore bcast/mcast ",
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for Reduce mds logging [#3120]
Summary: mds: Reduce mds logging [#3120] Review request for Ticket(s): 3120 Peer Reviewer(s): Thuan, Vu, Gary Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3120 Base revision: c6c7e77292d622ee042476bb0815feae51dd0cba Personal repository: git://git.code.sf.net/u/minh-chau/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision f39e30b489d6101d94d6b04bd4a4c0f5f1d5cbe6 Author: Minh Chau Date: Mon, 25 Nov 2019 11:32:33 +1100 mds: Reduce mds logging [#3120] The logging of broadcast/multicast is currently logged with NOTIFY as mds does not support broadcast/multicast message, so the logging would be helpful in some cases. However, the mds.log may be located in nfs file system, and this logging may cause high rate traffic towards nfs file system. This patch moves the logging to DEBUG for broadcast/multicast message, and for adding/removal mds service. Complete diffstat: -- src/mds/mds_tipc_fctrl_intf.cc | 4 ++-- src/mds/mds_tipc_fctrl_portid.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] mds: Reduce mds logging [#3120]
The logging of broadcast/multicast is currently logged with
NOTIFY as mds does not support broadcast/multicast message,
so the logging would be helpful in some cases. However, the
mds.log may be located in nfs file system, and this logging
may cause high rate traffic towards nfs file system.
This patch moves the logging to DEBUG for broadcast/multicast
message, and for adding/removal mds service.
---
src/mds/mds_tipc_fctrl_intf.cc | 4 ++--
src/mds/mds_tipc_fctrl_portid.cc | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index dd8d80d..0e3230a 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -390,7 +390,7 @@ uint32_t mds_tipc_fctrl_portid_up(struct tipc_portid id,
uint32_t type) {
id.node, id.ref, svc_id, portid->svc_cnt_);
} else {
portid->svc_cnt_++;
-m_MDS_LOG_NOTIFY("FCTRL: Add svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
+m_MDS_LOG_DBG("FCTRL: Add svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
id.node, id.ref, svc_id, portid->svc_cnt_);
}
@@ -410,7 +410,7 @@ uint32_t mds_tipc_fctrl_portid_down(struct tipc_portid id,
uint32_t type) {
TipcPortId *portid = portid_lookup(id);
if (portid != nullptr) {
portid->svc_cnt_--;
-m_MDS_LOG_NOTIFY("FCTRL: Remove svc[node:%x, ref:%u svc_id:%u],
svc_cnt:%u",
+m_MDS_LOG_DBG("FCTRL: Remove svc[node:%x, ref:%u svc_id:%u], svc_cnt:%u",
id.node, id.ref, svc_id, portid->svc_cnt_);
}
portid_map_mutex.unlock();
diff --git a/src/mds/mds_tipc_fctrl_portid.cc b/src/mds/mds_tipc_fctrl_portid.cc
index 724eb7b..316e1ba 100644
--- a/src/mds/mds_tipc_fctrl_portid.cc
+++ b/src/mds/mds_tipc_fctrl_portid.cc
@@ -298,7 +298,7 @@ uint32_t TipcPortId::ReceiveData(uint32_t mseq, uint16_t
mfrag,
}
}
if (rcving_mbcast_ == true) {
- m_MDS_LOG_NOTIFY("FCTRL: [me] <-- [node:%x, ref:%u], "
+ m_MDS_LOG_DBG("FCTRL: [me] <-- [node:%x, ref:%u], "
"RcvData[mseq:%u, mfrag:%u, fseq:%u], "
"rcvwnd[acked:%u, rcv:%u, nacked:%" PRIu64 "], "
"Ignore bcast/mcast ",
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
