Hi Thuan,
I think it's not a good idea and also not that critical to crash the SMF
client just because of the agent failing to send a callback message. It
also depends on whether the client expects SMF to wait for its response
on callback in order to proceed with the upgrade or not, which behavior
is specified by the callback timeout value in the campaign file.
With the current handling, SMF will not wait for the client response on
callback if the timeout is '0', and will just continue executing the
campaign; otherwise it will wait for as long as the specified timeout
and will fail the campaign if not receiving the callback response within
that period. A failed campaign and an error message in the log should be
good enough for SMF to communicate the failure in this situation. It's
not that severe to call abort() and generate core dump in this case.
Perhaps Lennart and Gary can have further comments/ideas on this solution.
Thanks,
Nguyen
On 7/5/2018 4:11 PM, Tran Thuan wrote:
Hi Nguyen,
Thanks for your comments. I agree with general comment.
About the patch (new flow), when posting to the client mailbox fail,
osafassert(0) is called.
I think the posting fail is critical problem since agent will miss a callback.
Also avoid smfnd is keeping waiting for agent response for that callback.
Please correct if I was wrong.
Best Regards,
Thuan (UFO – Unique FBI Opensaf)
CoreMW Maintenance, DEK VietNam
-----Original Message-----
From: Nguyen Luu <nguyen.tk....@dektech.com.au>
Sent: Wednesday, July 4, 2018 5:08 PM
To: thuan.tran <thuan.t...@dektech.com.au>; lennart.l...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] smf: Increase cbk count before post the evt to
client [#2882]
Hi,
One comment from me for the patch: With the new flow, the matched hdl should be
removed from the cbk_list if posting the message to the client mailbox fails.
A general comment: The original issue happened because of a race condition in
which the smfa_cb was used in different threads without proper use of mutex.
This patch may solve the issue, but the agent code should be improved to use
the mutex better, to avoid any other hidden race conditions. Perhaps a future
enhancement.
Thanks,
Nguyen
On 6/26/2018 11:32 AM, thuan.tran wrote:
Sometimes, callback agent dispatch and fail at saSmfReponse() because
cbk list is empty, agent by somehow handle evt before increase cbk
count. To avoid this, increase cbk count before post the evt.
---
src/smf/agent/smfa_utils.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/smf/agent/smfa_utils.c b/src/smf/agent/smfa_utils.c
index fb31a9a..917b6b8 100644
--- a/src/smf/agent/smfa_utils.c
+++ b/src/smf/agent/smfa_utils.c
@@ -615,8 +615,8 @@ SMFA_CBK_HDL_LIST *smfa_inv_hdl_add(SaInvocationT inv_id,
SaSmfHandleT hdl)
}
/***************************************************************************
-@brief : Match the filter. If matches, post the evt to the client MBX
- and increment the cbk count of the the corresponding hdl node.
+@brief : Match the filter. If matches, increment the cbk count of
+ the corresponding hdl node and post the evt to the client MBX.
If for a client, more than one scope matches, then those many
no of evts are posted to the MBX.
@param[in] : client_info - For which filter match to be performed.
@@ -694,27 +694,29 @@ uint32_t smfa_cbk_filter_match(SMFA_CLIENT_INFO
*client_info,
&cbk_evt->object_name),
&evt->evt.cbk_evt.object_name);
- if (m_NCS_IPC_SEND(&client_info->cbk_mbx,
- (NCSCONTEXT)evt,
- NCS_IPC_PRIORITY_NORMAL)) {
- /* Increment the cbk count.*/
- if (NULL != hdl_list) {
- /* There are two scope id
- * matching for the same hdl.*/
- } else {
- /* First scope id matching for
- * this hdl.*/
- hdl_list = smfa_inv_hdl_add(
- cbk_evt->inv_id,
- client_info->client_hdl);
- }
- hdl_list->cnt++;
- rc = NCSCC_RC_SUCCESS;
+ /* Increment the cbk count.*/
+ if (NULL != hdl_list) {
+ /* There are two scope id
+ * matching for the same hdl.*/
} else {
+ /* First scope id matching for
+ * this hdl.*/
+ hdl_list = smfa_inv_hdl_add(
+ cbk_evt->inv_id,
+ client_info->client_hdl);
+ }
+ hdl_list->cnt++;
+ rc = NCSCC_RC_SUCCESS;
+ if (m_NCS_IPC_SEND(
+ &client_info->cbk_mbx,
+ (NCSCONTEXT)evt,
+ NCS_IPC_PRIORITY_NORMAL)
+ != NCSCC_RC_SUCCESS) {
LOG_ER(
"SMFA: Posting to MBX failed. hdl: %llu,
scoe_id: %u",
client_info->client_hdl,
cbk_evt->scope_id);
+ osafassert(0);
}
/* If one of the filter matches then go to the
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
