Hi!
On 25.11.2013 22:40, [email protected] wrote:
Hi!
+
+}
+
+static void client_run(void)
+{
+ void *res = NULL;
+ long clnt_time = 0;
+
+ int i;
+ for (i = 0; i < clients_num; ++i)
+ pthread_join(thread_ids[i], &res);
+
+ threads_num = 0;
+
+ gettimeofday(&tv_client_end, NULL);
The gettimeofday time measurement may break if someone happen to change
the time while the test was running (i.e. ntp client daemon).
clock_gettime(CLOCK_MONOTONIC, &tp)
should be better.
The clock_gettime manual says that CLOCK_MONOTONIC_RAW similar to
_MONOTONIC, but is not subject to NTP adjustments. So CLOCK_MONOTONIC
can be adjusted, doesn't it?
+ clnt_time = (tv_client_end.tv_sec - tv_client_start.tv_sec) * 1000000 +
+ tv_client_end.tv_usec - tv_client_start.tv_usec;
+
+ tst_resm(TINFO, "total time '%ld' ms", clnt_time / 1000);
+
+ /* ask server to terminate */
+ int cfd;
+ client_msg[0] = start_fin_byte;
+ if (!client_connect_send(&cfd)) {
+ shutdown(cfd, SHUT_WR);
+ SAFE_CLOSE(NULL, cfd);
+ }
I would be happier if you have passed the client_connect_send()
arguments as arguments instead of using global variables (it would be
easier to follow the codepath). Moreover I would make the function to
return the filedescriptor directly, it returns int anyway...
OK
+ /* the script tcp_fastopen_run.sh will remove it */
+ SAFE_FILE_PRINTF(cleanup, rpath, "%ld", clnt_time / 1000);
+}
+
+static void client_cleanup(void)
+{
+ void *res = NULL;
+ int i;
+ for (i = 0; i < threads_num; ++i)
+ pthread_join(thread_ids[i], &res);
+
+ free(thread_ids);
+
+ if (remote_addrinfo)
+ freeaddrinfo(remote_addrinfo);
+}
+
+
+void make_server_reply(char **send_msg, const uint16_t *send_msg_size,
+ const char *send_msg_byte)
+{
+ *send_msg = SAFE_MALLOC(NULL, *send_msg_size);
+ memset(*send_msg, *send_msg_byte, *send_msg_size - 1);
+
+ (*send_msg)[0] = start_byte;
+
+ (*send_msg)[*send_msg_size - 1] = end_byte;
What about simply returning pointer to the newly allocated buffer
instead of the ugly pointer to an array?
Oh, yes thanks. It will be much nicer and simpler.
+ tst_resm(TFAIL, "recv failed, sock '%d'", client_fd);
+ break;
+ }
+
+ offset += recv_len;
+
+ if (recv_msg[offset - 1] != end_byte) {
+ tst_resm(TINFO, "msg is not complete, continue recv");
+ continue;
+ }
+
+ if (recv_msg[0] == start_fin_byte)
+ tst_brkm(TBROK, cleanup, "client asks to terminate...");
Why is this TBROK?
Thought about all in one function, which prints description messages,
calls cleanup function and exit the program.
Also looking int the tst_res() the T_exitval |= ttype_result; is not
guarded by locks, so in very unlikely case the value may be rewritten by
a tst_resm() for example. (Two threads enters tst_res and each of them
gets the value, modifies it and saves it and the result depends on the
order of these operations)
And the problem is if we add a locks there, all tests would need to be
compiled with -lpthread, which is someting I do not want to do. :(
One solution would be not using the tst_interface in this program at all
as it's executed by the shell script that prints the test messages and
returns exit value... And the same would simplify the propagation of the
result that is currently hacked around via the tfo_result file. You
could have simply printed the value into the stdout. But on the other
hand this will be more work on the testcase.
What about built in gcc functions for atomic memory access like
|__sync_fetch_and_or()?|
+
+
+ while (bind(sfd, local_addrinfo->ai_addr,
+ local_addrinfo->ai_addrlen) == -1) {
+ sleep(1);
I do not really like sleep(1) in testcases in most of the cases it slows
down testruns and it adds up to minutes and hours for thousands of
testcases... What about trying ten times in a second with
usleep(100000); or similar?
OK
...
+ check_opt("a", aarg, &clients_num, 1);
+ check_opt("r", rarg, &client_max_requests, 1);
+ check_opt("R", Rarg, &server_max_requests, 1);
+ check_opt("n", narg, &client_msg_size, 1);
+
+ check_opt("b", barg, &client_byte, 0);
+ check_msg_byte(&client_byte);
+
+ check_opt("N", Narg, &server_msg_size, 1);
+
+ check_opt("B", Barg, &server_byte, 0);
+ check_msg_byte(&server_byte);
Does chaning the message content bytes actually adds up some value or
not? Do you expect that test outcome would be different with different
values? If not, I would remove this options, the test is complex enough
itself.
True, I will set some constant.
+
+run_remote_cmd "[ ! -e $tdir ] && mkdir -p $tdir"
+
+if [ "$use_ssh" = 1 ]; then
+ scp -q ${tdir}tcp_fastopen $user_name@$remote_addr:$tdir
+else
+ rcp ${tdir}tcp_fastopen $user_name@$remote_addr:$tdir
+fi
Eh, you copy the test binary to the remote machine and then execute it?
The rest of the network testcases expects that ltp is installed both on
the client and the server machine and while I don't like that it expects
that the LTP install directory is /opt/ltp it's far more robust than
this. For example x86_64 machine is on one end and i386 on the other one
will not work this way.
OK, I see it now, will remove it.
------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing
conversations that shape the rapidly evolving mobile landscape. Sign up now.
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
