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
