All of lore.kernel.org
 help / color / mirror / Atom feed
From: chrubis@suse.cz
To: Jan Stancek <jstancek@redhat.com>
Cc: ltp-list@lists.sourceforge.net
Subject: Re: [LTP] [PATCH] new testcase: kmsg01
Date: Mon, 17 Jun 2013 17:13:59 +0200	[thread overview]
Message-ID: <20130617151358.GA2053@rei> (raw)
In-Reply-To: <7258543eb9e994c30bac3c3b9547178c1a1f88f7.1370958327.git.jstancek@redhat.com>

Hi!
> diff --git a/runtest/logging b/runtest/logging
> new file mode 100644
> index 0000000..47a667e
> --- /dev/null
> +++ b/runtest/logging
> @@ -0,0 +1 @@
> +kmsg01 kmsg01

I don't think that adding a bunch of new runtest files with one entry is
good way to go. What about adding a kernel runtest file (kernel_misc or
so) and put all the kernel tests that does not match more general group
there?

> --- /dev/null
> +++ b/testcases/kernel/logging/kmsg/kmsg01.c
> @@ -0,0 +1,513 @@
> +/*
> + * Copyright (C) 2013 Linux Test Project
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + *
> + * Further, this software is distributed without any warranty that it
> + * is free of the rightful claim of any third person regarding
> + * infringement or the like.  Any license provided herein, whether
> + * implied or otherwise, applies only to this software file.  Patent
> + * licenses, if any, provided herein do not apply to combinations of
> + * this program with other software, or any other product whatsoever.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +/*
> + * Test /dev/kmsg based on kernel doc: Documentation/ABI/testing/dev-kmsg
> + * - read() blocks
> + * - non-blocking read() fails with EAGAIN
> + * - partial read fails (buffer smaller than message)
> + * - can write to /dev/kmsg and message seqno grows
> + * - first read() after open() returns same message
> + * - if messages get overwritten, read() returns -EPIPE
> + * - device supports SEEK_SET, SEEK_END, SEEK_DATA
> + */
> +#define _GNU_SOURCE
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <sys/wait.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include "config.h"
> +#include "test.h"
> +#include "usctest.h"
> +#include "safe_macros.h"
> +#include "linux_syscall_numbers.h"
> +
> +#define MSG_PREFIX "LTP KMSG "

Perhaps we should include exact test name in the prefix so it's clear
which test created them.

Something as "LTP kmsg01 "

> +#define MAX_MSGSIZE 4096
> +#define NUM_READ_MSGS 3
> +#define NUM_OVERWRITE_MSGS 1024
> +#define READ_TIMEOUT 5
> +
> +char *TCID = "kmsg01";
> +static void setup(void);
> +static void cleanup(void);
> +
> +/*
> + * inject_msg - write message to /dev/kmsg
> + * @msg: null-terminated message to inject, should end with \n
> + *
> + * RETURNS:
> + *   0 on success
> + *  -1 on failure, errno reflects write() errno
> + */
> +static int inject_msg(const char *msg)
> +{
> +	int f;
> +	f = open("/dev/kmsg", O_RDWR);
> +	if (f < 0)
> +		tst_brkm(TBROK | TERRNO, cleanup, "failed to open /dev/kmsg");
> +	TEST(write(f, msg, strlen(msg)));
> +	close(f);

SAFE_CLOSE() ?

> +	errno = TEST_ERRNO;
> +	return TEST_RETURN;
> +}
> +
> +/*
> + * find_msg - find message in kernel ring buffer
> + * @fd:           fd to use, if < 0 function opens /dev/kmsg itself
> + * @text_to_find: substring to look for in messages
> + * @buf:          buf to store found message
> + * @bufsize:      size of 'buf'
> + * @first:        1 - return first matching message
> + *                0 - return last matching message
> + * RETURNS:
> + *   0 on success
> + *  -1 on failure
> + */
> +static int find_msg(int fd, const char *text_to_find, char *buf, int bufsize,
> +	int first)
> +{
> +	int f, msg_found = 0;
> +	char *msg;
> +
> +	msg = SAFE_MALLOC(cleanup, bufsize);

Is the bufsize expected to be too large to be declared on stack as
msg[bufsize] ?

> +	if (fd < 0) {
> +		f = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
> +		if (f < 0)
> +			tst_brkm(TBROK, cleanup, "failed to open /dev/kmsg");
> +	} else {
> +		f = fd;
> +	}
> +
> +	while (1) {
> +		TEST(read(f, msg, bufsize));
> +		if (TEST_RETURN < 0) {
> +			if (TEST_ERRNO == EAGAIN)
> +				/* there are no more messages */
> +				break;
> +			else
> +				tst_brkm(TBROK|TTERRNO, cleanup,
> +					"failed to read /dev/kmsg");
> +		} else if (TEST_RETURN < bufsize) {
> +			/* lines from kmsg are not NULL terminated */
> +			msg[TEST_RETURN] = '\0';
> +			if (strstr(msg, text_to_find) != NULL) {
> +				memcpy(buf, msg, TEST_RETURN+1);
> +				msg_found = 1;
> +				if (first)
> +					break;
> +			}
> +		}
> +	};
> +	free(msg);
> +	if (fd < 0)
> +		close(f);
> +
> +	if (msg_found)
> +		return 0;
> +	else
> +		return -1;
> +}
> +
> +static int get_msg_fields(const char *msg, unsigned long *prio,
> +	unsigned long *seqno)
> +{
> +	unsigned long s, p;
> +	if (sscanf(msg, "%lu,%lu,", &p, &s) == 2) {
> +		if (prio)
> +			*prio = p;
> +		if (seqno)
> +			*seqno = s;
> +		return 0;
> +	} else {
> +		return 1;
> +	}
> +}
> +
> +/*
> + * timed_read - if possible reads from fd or times out
> + * @fd:           fd to read from
> + * @timeout_sec:  timeout in seconds
> + *
> + * RETURNS:
> + *   read bytes on successful read
> + *  -1 on read() error, errno reflects read() errno
> + *  -2 on timeout
> + */
> +static int timed_read(int fd, int timeout_sec)
> +{
> +	int ret, tmp;
> +	struct timeval timeout;
> +	fd_set read_fds;
> +
> +	FD_ZERO(&read_fds);
> +	FD_SET(fd, &read_fds);
> +	timeout.tv_sec = timeout_sec;
> +	timeout.tv_usec = 0;
> +
> +	ret = select(fd + 1, &read_fds, 0, 0, &timeout);
> +	switch (ret) {
> +	case -1:
> +		tst_brkm(TBROK|TERRNO, cleanup, "select failed");
> +	case 0:
> +		/* select timed out */
> +		return -2;
> +	}
> +
> +	ret = read(fd, &tmp, 1);
> +	return ret;

return read(fd, &tmp, 1) ? :)

> +}
> +
> +/*
> + * timed_read_file - reads file until it reaches end of file,
> + *                   read fails or times out.
> + * @fd:           fd to read from
> + * @timeout_sec:  timeout in seconds for every read attempt
> + *
> + * RETURNS:
> + *     0 on read reaching eof
> + *    -1 on read error, errno reflects read() errno
> + *    -2 on timeout
> + */
> +static int timed_read_file(int fd, int timeout_sec)
> +{
> +	int child, status;
> +	int pipefd[2];
> +	char msg[MAX_MSGSIZE];
> +
> +	if (pipe(pipefd) != 0)
> +		tst_brkm(TBROK|TERRNO, cleanup, "pipe failed");
> +
> +	child = fork();
> +	switch (child) {
> +	case -1:
> +		tst_brkm(TBROK|TERRNO, cleanup, "failed to fork");
> +	case 0:
> +		/* child does all the reading and keeps writing to
> +		 * pipe to let parent know that it didn't block */
> +		close(pipefd[0]);
> +		do {
> +			write(pipefd[1], "", 1);
> +			TEST(read(fd, msg, MAX_MSGSIZE));
> +		} while (TEST_RETURN > 0);
> +
> +		close(pipefd[1]);
> +		if (TEST_RETURN == -1)
> +			exit(TEST_ERRNO);
> +		exit(0);
> +	}
> +	close(pipefd[1]);
> +
> +	/* parent reads pipe until it reaches eof or until read times out */
> +	do {
> +		TEST(timed_read(pipefd[0], timeout_sec));
> +	} while (TEST_RETURN > 0);
> +	close(pipefd[0]);
> +
> +	if (TEST_RETURN == -2)
> +		/* child is blocked, kill it */
> +		kill(child, SIGTERM);

I would put the comment above the if, so that the one line block
underneath has really one line.

> +	if (waitpid(child, &status, 0) == -1)
> +		tst_brkm(TBROK | TERRNO, cleanup, "waitpid");
> +	if (WIFEXITED(status)) {
> +		if (WEXITSTATUS(status) == 0) {
> +			return 0;
> +		} else {
> +			errno = WEXITSTATUS(status);
> +			return -1;
> +		}
> +	}
> +	return -2;
> +}

...

> +static void test_read_returns_first_message(void)
> +{
> +	char msg[NUM_READ_MSGS][MAX_MSGSIZE];
> +	int msgs_match = 1;
> +	int i;
> +
> +	tst_resm(TINFO, "TEST: mult. readers will get same first message");
> +	for (i = 0; i < NUM_READ_MSGS; i++) {
> +		if (find_msg(-1, "", msg[i], MAX_MSGSIZE, 1) != 0)
> +			tst_resm(TFAIL, "failed to find any message");
> +	}
> +
> +	for (i = 0; i < NUM_READ_MSGS-1; i++)
> +		if (strlen(msg[i]) > 0 && strcmp(msg[i], msg[i+1]) != 0)
> +			msgs_match = 0;
> +	if (msgs_match)
> +		tst_resm(TPASS, "all readers got same message on first read");
> +	else
> +		tst_resm(TFAIL, "readers got different messages");

Is here a chance that the log would be overwritten by a kernel writing
urelated message? We do fill the log with a bunch of messages (126)
before this test, how much of the kernel log is filled by this? Looking
in the kernel .config I have LOG_BUF_SHIFT=18 which is 256KB which
should be large enough, but it could be set as low as 12 which is 4KB.

> +}
> +
> +static void test_messages_overwritten(void)
> +{
> +	int i, fd;
> +	char msg[MAX_MSGSIZE];
> +	unsigned long first_seqno, seqno;
> +	char filler_str[] = MSG_PREFIX"FILLER MESSAGE TO OVERWRITE OTHERS\n";
> +
> +	tst_resm(TINFO, "TEST: read returns EPIPE when messages get "
> +		"overwritten");
> +
> +	fd = open("/dev/kmsg", O_RDWR);
> +	if (fd < 0)
> +		tst_brkm(TBROK|TERRNO, cleanup, "failed to open /dev/kmsg");
> +
> +	if (find_msg(-1, "", msg, MAX_MSGSIZE, 1) == 0
> +		&& get_msg_fields(msg, NULL, &first_seqno) == 0) {
> +		tst_resm(TINFO, "first seqno: %lu", first_seqno);
> +	} else {
> +		tst_brkm(TBROK, cleanup, "failed to get first seq. number");
> +	}
> +
> +	while (1) {
> +		if (find_msg(-1, "", msg, MAX_MSGSIZE, 1) != 0
> +				|| get_msg_fields(msg, NULL, &seqno) != 0) {
> +			tst_resm(TFAIL, "failed to get first seq. number");
> +			break;
> +		}
> +		if (first_seqno != seqno) {
> +			/* first message was overwritten */
> +			tst_resm(TINFO, "first seqno now: %lu", seqno);
> +			break;
> +		}
> +		for (i = 0; i < NUM_OVERWRITE_MSGS; i++) {
> +			if (inject_msg(filler_str) == -1)
> +				tst_brkm(TBROK|TERRNO, cleanup,
> +					"failed write to /dev/kmsg");
> +		}
> +	}
> +
> +	TEST(read(fd, msg, sizeof(msg)));
> +	if (TEST_RETURN == -1 && TEST_ERRNO == EPIPE)
> +		tst_resm(TPASS, "read failed with EPIPE as expected");
> +	else
> +		tst_resm(TFAIL|TTERRNO, "read returned: %ld", TEST_RETURN);
> +	close(fd);
> +}
> +
> +static void test_seek(void)
> +{
> +	int fd;
> +	char msg[MAX_MSGSIZE], msg2[MAX_MSGSIZE];
> +
> +	tst_resm(TINFO, "TEST: seek");
> +	fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK);
> +	if (fd < 0)
> +		tst_brkm(TBROK|TERRNO, cleanup, "failed to open /dev/kmsg");
> +
> +	/* read() after SEEK_SET 0 returns same (first) message */
> +	TEST(read(fd, msg, sizeof(msg)));
> +	if (TEST_RETURN == -1)
> +		tst_brkm(TBROK|TTERRNO, cleanup, "failed to read /dev/kmsg");
> +	msg[TEST_RETURN] = '\0';
> +
> +	if (lseek(fd, 0, SEEK_SET) == -1)
> +		tst_resm(TFAIL|TERRNO, "SEEK_SET 0 failed");
> +
> +	TEST(read(fd, msg2, sizeof(msg2)));
> +	if (TEST_RETURN == -1)
> +		tst_brkm(TBROK|TTERRNO, cleanup, "failed to read /dev/kmsg");
> +	msg2[TEST_RETURN] = '\0';
> +
> +	if (strcmp(msg, msg2) != 0)
> +		tst_resm(TFAIL, "SEEK_SET 0\nmsg1: %s\n, msg2: %s", msg, msg2);
> +	else
> +		tst_resm(TPASS, "SEEK_SET 0");
> +
> +	/* messages after SEEK_END 0 shouldn't contain MSG_PREFIX */
> +	if (lseek(fd, 0, SEEK_END) == -1)
> +		tst_resm(TFAIL|TERRNO, "lseek SEEK_END 0 failed");
> +	if (find_msg(fd, MSG_PREFIX, msg, sizeof(msg), 0) != 0)
> +		tst_resm(TPASS, "SEEK_END 0");
> +	else
> +		tst_resm(TFAIL, "SEEK_END 0 found: %s", msg);
> +
> +#ifdef SEEK_DATA
> +	/* messages after SEEK_DATA 0 shouldn't contain MSG_PREFIX */
> +	if (__NR_syslog != __LTP__NR_INVALID_SYSCALL) {

What about we add ltp_syscall_defined() or similar so that we don't have
to use the double underscore internal indentificator in test code?

Or use the ltp_syscall() that does that automatically and exits the test
with TCONF if not available.

> +		/* clear ring buffer */
> +		if (syscall(__NR_syslog, 5, NULL, 0) == -1)
> +			tst_brkm(TBROK|TERRNO, cleanup, "syslog clear failed");
> +		if (lseek(fd, 0, SEEK_DATA) == -1)
> +			tst_resm(TFAIL|TERRNO, "lseek SEEK_DATA 0 failed");
> +		if (find_msg(fd, MSG_PREFIX, msg, sizeof(msg), 0) != 0)
> +			tst_resm(TPASS, "SEEK_DATA 0");
> +		else
> +			tst_resm(TFAIL, "SEEK_DATA 0 found: %s", msg);
> +	}
> +#endif
> +	close(fd);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int lc;
> +	char *msg;
> +
> +	msg = parse_opts(argc, argv, NULL, NULL);
> +	if (msg != NULL)
> +		tst_brkm(TBROK, tst_exit, "OPTION PARSING ERROR - %s", msg);
> +
> +	setup();
> +	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +		/* run test_inject first so log isn't empty for other tests */
> +		test_inject();
> +		test_read_nonblock();
> +		test_read_block();
> +		test_partial_read();
> +		test_read_returns_first_message();
> +		test_messages_overwritten();
> +		test_seek();
> +	}
> +	cleanup();
> +	tst_exit();
> +}
> +
> +static void setup(void)
> +{
> +	tst_require_root(NULL);
> +	if (tst_kvercmp(3, 5, 0) < 0)
> +		tst_brkm(TCONF, NULL, "This test requires kernel"
> +			" >= 3.5.0");
> +	srand(getpid());
> +	TEST_PAUSE;
> +}
> +
> +static void cleanup(void)
> +{
> +	TEST_CLEANUP;
> +}

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

  reply	other threads:[~2013-06-17 15:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 13:47 [LTP] [PATCH] new testcase: kmsg01 Jan Stancek
2013-06-17 15:13 ` chrubis [this message]
     [not found]   ` <790434361.3815720.1371483737505.JavaMail.root@redhat.com>
2013-06-17 16:21     ` chrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130617151358.GA2053@rei \
    --to=chrubis@suse.cz \
    --cc=jstancek@redhat.com \
    --cc=ltp-list@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.