On 01/10/2018 05:25 PM, Daniel Borkmann wrote:
> On 01/10/2018 07:39 PM, John Fastabend wrote:
>> sockmap sample program takes arguments from cmd line but it reads them
>> in using offsets into the array. Because we want to add more arguments
>> in the future lets do proper argument handling.
>>
>> Also refactor code to pull apart sock init and ping/pong test. This
>> allows us to add new tests in the future.
>>
>> Signed-off-by: John Fastabend <john.fastab...@gmail.com>
>> ---
>> samples/sockmap/sockmap_user.c | 142
>> +++++++++++++++++++++++++++++-----------
>> 1 file changed, 103 insertions(+), 39 deletions(-)
> [...]
>>
>> /* Accept Connecrtions */
>> @@ -149,23 +177,32 @@ static int sockmap_test_sockets(int rate, int dot)
>> goto out;
>> }
>>
>> - max_fd = p2;
>> - timeout.tv_sec = 10;
>> - timeout.tv_usec = 0;
>> -
>> printf("connected sockets: c1 <-> p1, c2 <-> p2\n");
>> printf("cgroups binding: c1(%i) <-> s1(%i) - - - c2(%i) <-> s2(%i)\n",
>> c1, s1, c2, s2);
>> +out:
>> + return err;
>
> Maybe rather than setting err and goto out where we now just return
> err anyway, return from those places directly.
>
Perhaps but how about doing this in another patch. This
patch is not changing the goto err pattern. I can send
a follow up.
>> +}
>> +
>> +static int forever_ping_pong(int rate, int verbose)
>> +{
>> + struct timeval timeout;
>> + char buf[1024] = {0};
>> + int sc;
>> +
>> + timeout.tv_sec = 10;
>> + timeout.tv_usec = 0;
>>
>> /* Ping/Pong data from client to server */
>> sc = send(c1, buf, sizeof(buf), 0);
>> if (sc < 0) {
>> perror("send failed()\n");
>> - goto out;
>> + return sc;
>> }
>>
>> do {
>> - int s, rc, i;
>> + int s, rc, i, max_fd = p2;
>> + fd_set w;
>>
>> /* FD sets */
>> FD_ZERO(&w);
> [...]
>> - err = sockmap_test_sockets(rate, dot);
>> + err = sockmap_init_sockets();
>> if (err) {
>> fprintf(stderr, "ERROR: test socket failed: %d\n", err);
>> - return err;
>> + goto out;
>> }
>> - return 0;
>> +
>> + err = forever_ping_pong(rate, verbose);
>> +out:
>> + close(s1);
>> + close(s2);
>> + close(p1);
>> + close(p2);
>> + close(c1);
>> + close(c2);
>> + return err;
>> }
>>
>> void running_handler(int a)
>> {
>> running = 0;
>> + printf("\n");
>
> Do we need this out of the sighandler instead of e.g. main loop when
> we break out?
Not really let me just remove it. I'll do
another patch with documentation and fixup
error messages so we don't have this issue.
I agree its a bit clumsy.
