Hi Nguyen,
In that function, some osafassert(0) is used when fail calloc(), why don't just
ignore as your idea?
Just wonder one point:
Agent is a thread of SMFD, it crash mean SMFD crash (node reboot?), is there
any rollback trigger after that?
If it cannot, then I should handle posting fail as your comment.
Best Regards,
Thuan
-----Original Message-----
From: Nguyen Luu <nguyen.tk....@dektech.com.au>
Sent: Thursday, July 5, 2018 8:00 PM
To: Tran Thuan <thuan.t...@dektech.com.au>; lennart.l...@ericsson.com; Gary Lee
<gary....@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [devel] [PATCH 1/1] smf: Increase cbk count before post the evt to
client [#2882]
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
