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
