Hi Minh & Gary,

As suggested its better to handle D2N_PRESENCE_SU and PG_TRACK in a separate 
Ticket

For DATA_REQUEST messages, I agree with the suggestions. I incorporated the 
suggestions and tested, its working fine.
I am sending the diff patch on top of the original patch, If you are fine I 
will push the patch

Thanks,
Ravi

----- Original Message -----
From: minh.c...@dektech.com.au
To: gary....@dektech.com.au, ravisekhar.ko...@oracle.com, 
minh-c...@users.sf.net, hans.nordeb...@ericsson.com
Cc: opensaf-devel@lists.sourceforge.net
Sent: Wednesday, October 25, 2017 11:15:58 AM GMT +05:30 Chennai, Kolkata, 
Mumbai, New Delhi
Subject: Re: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
Headless state [#2601]

Hi,

Agree with Ravi that the msg N2D_REG_SU should be queued too, otherwise 
there will be no D2N_PRESENCE_SU msg.

So the condition to queue N2D_INFO_SU_SI_ASSIGN, N2D_OPERATION_STATE and 
D2N_PRESENCE_SU should be:

cb->is_avd_down == true || cb->amfd_sync_required == true

The reason is the above 3 messages won't be processed until amfd 
finishes sync

Also agree with Gary that N2D_DATA_REQUEST should be queued with: 
cb->is_avd_down == false && cb->amfd_sync_required == true
The reason is the DATA is carried in sync_info msg. After amfd is up and 
before node_up is accepted, if the DATA_REQUEST msg is sent and rejected 
by amfd, it's still in the queue and will be resent after sync by 
avnd_diq_rec_send_buffered_msg(). However, the msgs in queue could have 
the msg_id not in ascending order.

For the PG track msg, I think we probably need to return TRY_AGAIN on 
waiting for acceptance of node_up too, because the pg track is resent 
when node_up is accepted (on stop resp_tmr timer), so the PG track may 
be duplicated if PG track is called just right after amfd is up. Also, 
it could cause the msg id out of order in the same way of DATA_REQUEST 
in this ticket too.

I can check the D2N_PRESENCE_SU and PG track and raise separated tickets 
if there're bugs (most likely)
The solution of the patch looks ok to me within Gary's comment.

Thanks,
Minh
On 25/10/17 14:58, Gary Lee wrote:
> Hi Ravi
>
> >From what I can see, the main problem is AMFND is sending messages after 
> >AMFD has come up, but before NODE_UP has been accepted by AMFD.
> If that is correct, then PG track messages, like DATA_REQUEST, should be in 
> the same situation as it’s only checking is_avd_down.
>
> uint32_t avnd_evt_ava_pg_start_evh(AVND_CB *cb, AVND_EVT *evt) {
>
>    // if headless, return TRY_AGAIN to application
>    if (cb->is_avd_down == true) {
>      LOG_NO("Director is down. Return try again for PG start.");
>       ..
>    }
>    ..
> }
>
> I think we should only buffer these messages if “is_avd_down == false && 
> amfd_sync_required == true”.
> We should be able to drop DATA_REQUEST when it’s headless, since that type of 
> information is sync’ed with AVSV_N2D_ND_CSICOMP_STATE_INFO_MSG and 
> AVSV_N2D_ND_SISU_STATE_INFO_MSG.
>
> Thanks
> Gary
>
> On 24/10/17, 11:14 pm, "Ravi Sekhar Reddy Konda" 
> <ravisekhar.ko...@oracle.com> wrote:
>
>      Hi Gary,
>      
>      There are some messages which does not required to be queued like 
> PG_TRACK & NODE_DOWN messages
>      During SC absence period, we are returning SA_AMF_ERROR_TRY_AGAIN for PG 
> Track operations
>      
>      Currently along with this Fix we are queuing the following messages
>      AVSV_N2D_INFO_SU_SI_ASSIGN_MSG
>      AVSV_N2D_OPERATION_STATE_MSG
>      AVSV_N2D_DATA_REQUEST_MSG
>      
>      only missing event is Reg SU response event (AVSV_N2D_REG_SU_MSG), I 
> think this event has to be queued.
>      
>      if you agree with this. I will update the patch by adding queuing for 
> Reg SU response event also
>      
>      
>      Thanks,
>      Ravi
>      
>      -----Original Message-----
>      From: Gary Lee [mailto:gary....@dektech.com.au]
>      Sent: Wednesday, October 18, 2017 10:57 AM
>      To: ravi-sekhar <ravisekhar.ko...@oracle.com>; 
> hans.nordeb...@ericsson.com
>      Cc: opensaf-devel@lists.sourceforge.net
>      Subject: Re: [devel] [PATCH 1/1] amf: Buffer and resend data req 
> messages in Headless state [#2601]
>      
>      Hi Ravi
>      
>      I’ve started looking at this. My initial thought is that perhaps we need 
> to queue up all messages when is_avd_down == false && amfd_sync_required == 
> true (ie. AMFD has come up but haven’t accepted node_up). What do you think?
>      
>      Will get back to you.
>      
>      /Gary
>      
>      -----Original Message-----
>      From: ravi-sekhar <ravisekhar.ko...@oracle.com>
>      Date: Tuesday, 17 October 2017 at 10:39 pm
>      To: <hans.nordeb...@ericsson.com>
>      Cc: <opensaf-devel@lists.sourceforge.net>
>      Subject: [devel] [PATCH 1/1] amf: Buffer and resend data req messages in 
> Headless state [#2601]
>      
>          ---
>           src/amf/amfnd/di.cc | 42 +++++++++++++++++++++++++++++++-----------
>           1 file changed, 31 insertions(+), 11 deletions(-)
>          
>          diff --git a/src/amf/amfnd/di.cc b/src/amf/amfnd/di.cc
>          index 2dc023c..1e0d682 100644
>          --- a/src/amf/amfnd/di.cc
>          +++ b/src/amf/amfnd/di.cc
>          @@ -998,21 +998,30 @@ uint32_t avnd_di_object_upd_send(AVND_CB *cb, 
> AVSV_PARAM_INFO *param) {
>             uint32_t rc = NCSCC_RC_SUCCESS;
>             TRACE_ENTER2("Comp '%s'", 
> osaf_extended_name_borrow(&param->name));
>           
>          -  if (cb->is_avd_down == true) {
>          -    TRACE_LEAVE2("AVD is down. %u", rc);
>          -    return rc;
>          -  }
>          -
>          -  memset(&msg, 0, sizeof(AVND_MSG));
>          -
>             /* populate the msg */
>          +  memset(&msg, 0, sizeof(AVND_MSG));
>             msg.info.avd = static_cast<AVSV_DND_MSG *>(calloc(1, 
> sizeof(AVSV_DND_MSG)));
>             msg.type = AVND_MSG_AVD;
>             msg.info.avd->msg_type = AVSV_N2D_DATA_REQUEST_MSG;
>          -  msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
>             msg.info.avd->msg_info.n2d_data_req.node_id = 
> cb->node_info.nodeId;
>             msg.info.avd->msg_info.n2d_data_req.param_info = *param;
>           
>          +  if ((cb->is_avd_down == true) || (cb->amfd_sync_required == 
> true)) {
>          +    msg.info.avd->msg_info.n2d_data_req.msg_id = 0;
>          +    if (avnd_diq_rec_add(cb, &msg) == nullptr) {
>          +      rc = NCSCC_RC_FAILURE;
>          +    }
>          +    LOG_NO(
>          +        "avnd_di_object_upd_send() deferred as AMF director is 
> offline(%d),"
>          +        " or sync is required(%d)",
>          +        cb->is_avd_down, cb->amfd_sync_required);
>          +
>          +    TRACE_LEAVE2("AVD is down. %u", rc);
>          +    return rc;
>          +  } else {
>          +    msg.info.avd->msg_info.n2d_data_req.msg_id = ++(cb->snd_msg_id);
>          +  }
>          +
>             /* send the msg to AvD */
>             rc = avnd_di_msg_send(cb, &msg);
>             if (NCSCC_RC_SUCCESS == rc) msg.info.avd = 0;
>          @@ -1515,9 +1524,20 @@ void avnd_diq_rec_send_buffered_msg(AVND_CB 
> *cb) {
>                       
> pending_rec->msg.info.avd->msg_info.n2d_opr_state.rec_rcvr
>                           .raw);
>                   ++iter;
>          -    } else {
>          -      ++iter;
>          -    }
>          +    } else if (pending_rec->msg.info.avd->msg_type == 
> AVSV_N2D_DATA_REQUEST_MSG &&
>          +               
> pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id == 0) {
>          +        pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id =
>          +              ++(cb->snd_msg_id);
>          +
>          +        LOG_NO(
>          +            "Found and resend buffered Data Req msg for SU:'%s', 
> msg_id:'%u'",
>          +            
> osaf_extended_name_borrow(&pending_rec->msg.info.avd->msg_info
>          +                                           
> .n2d_data_req.param_info.name),
>          +            
> pending_rec->msg.info.avd->msg_info.n2d_data_req.msg_id);
>          +       ++iter;
>          +     } else {
>          +       ++iter;
>          +     }
>             }
>           
>             TRACE("retransmit message to amfd");
>          --
>          1.9.1
>          
>          
>          
> ------------------------------------------------------------------------------
>          Check out the vibrant tech community on one of the world's most
>          engaging tech sites, Slashdot.org! 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__sdm.link_slashdot&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=rFCQ76TW5HZUgA7b20ApVcXgXru6mvz4fvCm1_H6w1k&m=_B8vfWmVodcwGLVWeK6LJ5kV9_6Z39lakp7QX8LKQzk&s=tpxAiV8CXUFAPD5RvnnpQZoFhOdQrBc7h-EA0c9AfZM&e=
>          _______________________________________________
>          Opensaf-devel mailing list
>          Opensaf-devel@lists.sourceforge.net
>          
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.sourceforge.net_lists_listinfo_opensaf-2Ddevel&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=rFCQ76TW5HZUgA7b20ApVcXgXru6mvz4fvCm1_H6w1k&m=_B8vfWmVodcwGLVWeK6LJ5kV9_6Z39lakp7QX8LKQzk&s=psJ9buGgwHkQZjvgL9KVS9M2C3plzno6CO0Ksv_kKFA&e=
>          
>      
>      
>      
>
>
>

Attachment: 2601_comments.patch
Description: Binary data

------------------------------------------------------------------------------
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

Reply via email to