Re: [LTP] [PATCH v2] network: tcp_fastopen: add TCP Fast Open test

[chrubis@suse.cz]

Hi!
> > 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?

Right, the CLOCK_MONOTONIC_RAW is better choice.

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

I would say this is a misuse of the API as it is...

I suppose that you can do this in the server because the return value is
not checked, but it's confusing when reading the code.

> > 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()?|

That would be easy solution, but I doubt that these are available on
all platforms and slightly older compilers. Perpahs we can add configure
checks and enable it only when it's supported.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list