Hi bro.Minh, I will update and send out V3.
Best Regards, ThuanTr -----Original Message----- From: Minh Hon Chau <minh.c...@dektech.com.au> Sent: Wednesday, September 25, 2019 9:00 AM To: thuan.tran <thuan.t...@dektech.com.au>; gary....@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1/1] mds: optimize mdstest suite 27 [#3087] Hi Thuan, Some comments: - a few warnings for >80 chars line - Need to free(msg) that is returned from each MDS callback - Another minor comment below Thanks Minh On 24/9/19 1:10 pm, thuan.tran wrote: > - Just allocate a small buffer instead of huge buffer > --- > src/mds/apitest/mdstipc_api.c | 116 +++++++++++++++------------------- > 1 file changed, 52 insertions(+), 64 deletions(-) > > diff --git a/src/mds/apitest/mdstipc_api.c > b/src/mds/apitest/mdstipc_api.c index 805728464..33e7d6c12 100644 > --- a/src/mds/apitest/mdstipc_api.c > +++ b/src/mds/apitest/mdstipc_api.c > @@ -13105,10 +13105,14 @@ void tet_create_default_PWE_VDEST_tp() > test_validate(FAIL, 0); > } > > -void tet_sender(char *send_buff, uint32_t buff_len, int msg_count) > +void tet_sender(uint32_t msg_count, uint32_t msg_size) > { > int live = 100; // sender live max 100s > TET_MDS_MSG *mesg; > + if (msg_size > TET_MSG_SIZE_MIN) { > + printf("\nSender: msg_size cannot bigger than > TET_MSG_SIZE_MIN\n"); > + exit(1); > + } > mesg = (TET_MDS_MSG *)malloc(sizeof(TET_MDS_MSG)); > memset(mesg, 0, sizeof(TET_MDS_MSG)); > > @@ -13134,7 +13138,7 @@ void tet_sender(char *send_buff, uint32_t buff_len, > int msg_count) > exit(1); > } > > - while(!gl_tet_adest.svc[0].svcevt[0].dest && live-- > 0) { > + while (!gl_tet_adest.svc[0].svcevt[0].dest && live-- > 0) { > printf("\nSender is waiting for receiver UP\n"); > sleep(1); > } > @@ -13147,11 +13151,11 @@ void tet_sender(char *send_buff, uint32_t buff_len, > int msg_count) > // otherwise, receiver won't detect loss message > sleep(1); > > - uint32_t offset = 0; > - uint32_t msg_len = buff_len / msg_count; > - for (int i = 1; i <= msg_count; i++) { > - memcpy(mesg->send_data, &send_buff[offset], msg_len); > - mesg->send_len = msg_len; > + for (uint32_t i = 1; i <= msg_count; i++) { > + /* to verify received correct order */ > + memset(mesg->send_data, 'X', msg_size); > + sprintf(mesg->send_data, "%u", i); > + mesg->send_len = msg_size; > if (mds_just_send(gl_tet_adest.mds_pwe1_hdl, > NCSMDS_SVC_ID_INTERNAL_MIN, > NCSMDS_SVC_ID_EXTERNAL_MIN, > @@ -13163,23 +13167,25 @@ void tet_sender(char *send_buff, uint32_t buff_len, > int msg_count) > } else { > printf("\nSender SENT message %d successfully\n", i); > } > - offset += msg_len; > } > free(mesg); > - while(live-- > 0) { > + while (live-- > 0) { > // Keep sender alive for retransmission > sleep(1); > } > } > > -bool tet_receiver(char *expected_buff, uint32_t buff_len, int > msg_count) > +bool tet_receiver(uint32_t msg_count, uint32_t msg_size) > { > - int ret = 1; > + if (msg_size > TET_MSG_SIZE_MIN) { > + printf("\nReceiver: msg_size cannot bigger than > TET_MSG_SIZE_MIN\n"); > + return 1; > + } > printf("\nStarted Receiver (pid:%d) svc_id=%d\n", > (int)getpid(), NCSMDS_SVC_ID_EXTERNAL_MIN); > if (adest_get_handle() != NCSCC_RC_SUCCESS) { > printf("\nReceiver FAIL to get adest handle\n"); > - return ret; > + return 1; > } > > sleep(1); //Let sender subscribe before receiver install @@ > -13197,14 +13203,12 @@ bool tet_receiver(char *expected_buff, uint32_t > buff_len, int msg_count) > exit(1); > } > > - char *received_buff = malloc(buff_len); > - memset(received_buff, 0, buff_len); > - uint32_t offset = 0; > + char *expected_buff = malloc(msg_size); > struct pollfd sel; > - int counter = 0; > + uint32_t counter = 0; > sel.fd = m_GET_FD_FROM_SEL_OBJ(gl_tet_adest.svc[0].sel_obj); > sel.events = POLLIN; > - while(counter < msg_count) { > + while (counter < msg_count) { > int ret = osaf_poll(&sel, 1, 10000); > if (ret > 0) { > gl_rcvdmsginfo.msg = NULL; > @@ -13214,11 +13218,23 @@ bool tet_receiver(char *expected_buff, uint32_t > buff_len, int msg_count) > printf("\nReceiver FAIL to retrieve message\n"); > break; > } > - TET_MDS_MSG *msg = (TET_MDS_MSG*)gl_rcvdmsginfo.msg; > + TET_MDS_MSG *msg = (TET_MDS_MSG *)gl_rcvdmsginfo.msg; > if (msg != NULL) { > - memcpy(&received_buff[offset],msg->recvd_data, > msg->recvd_len); > - offset += msg->recvd_len; > counter++; > + memset(expected_buff, 'X', msg_size); [M] I think you can move the above memset(expected_buff,...) before the while (counter,...) loop, since it constantly set 'X' to the @expected_buff > + sprintf(expected_buff, "%u", counter); > + if (memcmp(msg->recvd_data, expected_buff, > msg_size) != 0) { > + printf("\nReceiver: Received buffer is > not as expected\n"); > + printf("\nReceiver: Dump buffer to > /var/log/opensaf/\n"); > + FILE *fp = > fopen("/var/log/opensaf/mdstest_received_buff", "wb"); > + fwrite(msg->recvd_data, sizeof(char), > msg_size, fp); > + fclose(fp); > + fp = > fopen("/var/log/opensaf/mdstest_expect_buff", "wb"); > + fwrite(expected_buff, sizeof(char), > msg_size, fp); > + fclose(fp); > + free(expected_buff); > + return 1; > + } > } > } else { > printf("\nReceiver got %d messages\n", counter); @@ > -13227,43 > +13243,25 @@ bool tet_receiver(char *expected_buff, uint32_t buff_len, int > msg_count) > } > } > > - printf("\nReceiver verify received buffer is same as expected\n"); > - if (offset == buff_len) { > - if (memcmp(received_buff, expected_buff, buff_len) != 0) { > - printf("\nReceiver: Received buffer is not as > expected\n"); > - printf("\nReceiver: Dump buffer to /var/log/opensaf/ > \n"); > - FILE *fp = > fopen("/var/log/opensaf/mdstest_received_buff","wb"); > - fwrite(received_buff, sizeof(char), buff_len, fp); > - fclose(fp); > - fp = fopen("/var/log/opensaf/mdstest_expect_buff","wb"); > - fwrite(expected_buff, sizeof(char), buff_len, fp); > - fclose(fp); > - } else { > - ret = 0; > - } > - } else { > - printf("\nReceiver: Total received length=%d differ expected > %d\n", offset, buff_len); > + printf("\nReceiver verify received number message is same as > expected\n"); > + if (counter != msg_count) { > + printf("\nReceiver: Total received msg=%d differ expected > %d\n", counter, msg_count); > + free(expected_buff); > + return 1; > } > - free(received_buff); > + > printf("\nEnd Receiver (pid:%d) svc_id=%d\n", > (int)getpid(), NCSMDS_SVC_ID_EXTERNAL_MIN); > - return ret; > + free(expected_buff); > + return 0; > } > > void tet_overload_tp_1(void) > { > int pid = 0; > int FAIL = 0; > - uint32_t buff_len = 100000000; > - int msg_count = 2000; > - int msg_size = buff_len/msg_count; > - char *buff = malloc(buff_len); > - > - for (unsigned int i = 0; i < msg_count; i++) { > - /* to verify received correct order */ > - memset(buff+(i*msg_size), 'X', msg_size); > - sprintf(buff+(i*msg_size), "%u", i+1); > - } > + uint32_t msg_count = 2000; > + uint32_t msg_size = 50000; > > printf("\nTest Case 1: Receiver wait for drop normal message retransmit > and receive all messages in order\n"); > > /*-------------------------------------------------------------------- > */ @@ -13272,13 +13270,13 @@ void tet_overload_tp_1(void) > /* child as sender */ > setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1); > mds_startup(); > - tet_sender(buff, buff_len, msg_count); > + tet_sender(msg_count, msg_size); > mds_shutdown(); > } else if (pid > 0) { > /* parent as receiver */ > setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1); > mds_startup(); > - FAIL = tet_receiver(buff, buff_len, msg_count); > + FAIL = tet_receiver(msg_count, msg_size); > printf("\nReceiver finish, kill Sender\n"); > kill(pid, SIGKILL); > mds_shutdown(); > @@ -13288,7 +13286,6 @@ void tet_overload_tp_1(void) > FAIL = 1; > } > > - free(buff); > test_validate(FAIL, 0); > } > > @@ -13296,16 +13293,8 @@ void tet_overload_tp_2(void) > { > int pid = 0; > int FAIL = 0; > - uint32_t buff_len = 100000000; > - int msg_count = 1000; > - int msg_size = buff_len/msg_count; > - char *buff = malloc(buff_len); > - > - for (unsigned int i = 0; i < msg_count; i++) { > - /* to verify received correct order */ > - memset(buff+(i*msg_size), 'X', msg_size); > - sprintf(buff+(i*msg_size), "%u", i+1); > - } > + uint32_t msg_count = 1000; > + uint32_t msg_size = 100000; > > printf("\nTest Case 2: Receiver wait for drop fragment message > retransmit and receive all messages in order\n"); > > /*-------------------------------------------------------------------- > */ @@ -13314,13 +13303,13 @@ void tet_overload_tp_2(void) > /* child as sender */ > setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1); > mds_startup(); > - tet_sender(buff, buff_len, msg_count); > + tet_sender(msg_count, msg_size); > mds_shutdown(); > } else if (pid > 0) { > /* parent as receiver */ > setenv("MDS_TIPC_FCTRL_ENABLED", "1", 1); > mds_startup(); > - FAIL = tet_receiver(buff, buff_len, msg_count); > + FAIL = tet_receiver(msg_count, msg_size); > kill(pid, SIGKILL); > mds_shutdown(); > } else { > @@ -13329,7 +13318,6 @@ void tet_overload_tp_2(void) > FAIL = 1; > } > > - free(buff); > test_validate(FAIL, 0); > } > _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel