| Message ID | 20181107142209.8965-1-chrubis@suse.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] lib: Add support for test tags | expand |
On Wed, Nov 7, 2018 at 10:22 PM, Cyril Hrubis <chrubis@suse.cz> wrote: > This is just proof of concept of moving some of the test metadata into > more structured form. If implemented it will move some information from > comments in the test source code to an array to the tst_test structure. I have to say, this is really a good proposal to LTP especially for that regressions, we sometimes spend a lot of time to dig into test failures and read code comments to find the reason/clue, if now testcase can print out useful info when trigger the issue, that'd be more friendly to LTP runner. Another thought come to my mind is, can we build a correlation for one case which have many alias? e.g cve-2017-15274 == add_key02. If LTP framework has finished cve-2017-15274 test then run add_key02 in next, just skip and mark it as the same result with cve-2017-15274 show. > > It's not finished and certainly not set into a stone, this patch is > mainly intended to start a discussion. > > The newly introduced test tags are generic name-value pairs that can > hold test metadata, the intended use for now is to store kernel commit > hashes for kernel reproducers as well as CVE ids. The mechanism is > however choosen to be very generic so that it's easy to add basically > any information later on. > > As it is the main purpose is to print hints for a test failures. If a > test that has been written as a kernel reproducer fails it prints nice > URL pointing to a kernel commit that may be missing. The key point is, we'd better set a strict limitation to print hints only for known/expected failures. Because if the failure is a new bz/issue for a system or testcase, that messages will be useless and even misleading. > > This commit also adds adds the -q test flag, that can be used to query > test information, which includes these tags, but is not limited to them. > > The main inteded use for the query operation is to export test metadata > and constraints to the test execution system. The long term goal for > this would be parallel test execution as for this case the test runner > would need to know which global system resources is the test using to > avoid unexpected failures. > > So far it exposes only if test needs root and if block device is needed > for the test, but I would expect that we will need a few more tags for > various resources, one that comes to my mind would be "test is using > SystemV SHM" for that we can do something as add a "constraint" tag with > value "SysV SHM" or anything else that would be fitting. Another would > be "Test is changing system wide clocks", etc. Sounds good. But why only special( needs root or block-device) test can use that. Maybe we could just define a generic struct for the tag, about the real contents, test author can fill what they want to highlight.
Hi! > > This is just proof of concept of moving some of the test metadata into > > more structured form. If implemented it will move some information from > > comments in the test source code to an array to the tst_test structure. > > I have to say, this is really a good proposal to LTP especially for > that regressions, we sometimes spend a lot of time to dig into test > failures and read code comments to find the reason/clue, if now > testcase can print out useful info when trigger the issue, that'd be > more friendly to LTP runner. This is exactly the reason I looked into the problem. > Another thought come to my mind is, can we build a correlation for one > case which have many alias? e.g cve-2017-15274 == add_key02. If LTP > framework has finished cve-2017-15274 test then run add_key02 in next, > just skip and mark it as the same result with cve-2017-15274 show. Well the way how we track testcases is something that should be probably rethinked in the future. The runtest files are far from optimal, maybe we can build something based on tags in the future. > > It's not finished and certainly not set into a stone, this patch is > > mainly intended to start a discussion. > > > > The newly introduced test tags are generic name-value pairs that can > > hold test metadata, the intended use for now is to store kernel commit > > hashes for kernel reproducers as well as CVE ids. The mechanism is > > however choosen to be very generic so that it's easy to add basically > > any information later on. > > > > As it is the main purpose is to print hints for a test failures. If a > > test that has been written as a kernel reproducer fails it prints nice > > URL pointing to a kernel commit that may be missing. > > The key point is, we'd better set a strict limitation to print hints > only for known/expected failures. Because if the failure is a new > bz/issue for a system or testcase, that messages will be useless and > even misleading. Well there is always possibility for this to be misleading, once regression has been reintroduced into the kernel, it will point to a patch that has been already merged. What do you have in mind? I couldn't figure out anything better than this. I.e. if test has failed and there is reproducer tag, print the hint. Maybe we can reword the hint in a way that it's clear that there are other possible reason for the failure. > > This commit also adds adds the -q test flag, that can be used to query > > test information, which includes these tags, but is not limited to them. > > > > The main inteded use for the query operation is to export test metadata > > and constraints to the test execution system. The long term goal for > > this would be parallel test execution as for this case the test runner > > would need to know which global system resources is the test using to > > avoid unexpected failures. > > > > So far it exposes only if test needs root and if block device is needed > > for the test, but I would expect that we will need a few more tags for > > various resources, one that comes to my mind would be "test is using > > SystemV SHM" for that we can do something as add a "constraint" tag with > > value "SysV SHM" or anything else that would be fitting. Another would > > be "Test is changing system wide clocks", etc. > > Sounds good. > > But why only special( needs root or block-device) test can use that. For this patch I only picked two attributes from the tst_test structure that are relevant for the test execution, we will probably want to export a few more later on. > Maybe we could just define a generic struct for the tag, about the > real contents, test author can fill what they want to highlight. You lost me there, what exactly do you have in mind here?
Hi, ----- Original Message ----- > This is just proof of concept of moving some of the test metadata into > more structured form. If implemented it will move some information from > comments in the test source code to an array to the tst_test structure. > > It's not finished and certainly not set into a stone, this patch is > mainly intended to start a discussion. > > The newly introduced test tags are generic name-value pairs that can > hold test metadata, the intended use for now is to store kernel commit > hashes for kernel reproducers as well as CVE ids. The mechanism is > however choosen to be very generic so that it's easy to add basically > any information later on. > > As it is the main purpose is to print hints for a test failures. If a > test that has been written as a kernel reproducer fails it prints nice > URL pointing to a kernel commit that may be missing. > > Example output: > > tst_test.c:1145: INFO: Timeout per run is 0h 05m 00s > add_key02.c:98: FAIL: unexpected error with key type 'asymmetric': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'cifs.idmap': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'cifs.spnego': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'pkcs7_test': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'rxrpc': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'rxrpc_s': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'user': EINVAL > add_key02.c:98: FAIL: unexpected error with key type 'logon': EINVAL > > HINT: This is a regression test for linux kernel, see commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c > > HINT: This test also tests for CVE-2017-15274, see: > > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15274 > > Summary: > passed 0 > failed 8 > skipped 0 > warnings 0 > > This commit also adds adds the -q test flag, that can be used to query > test information, which includes these tags, but is not limited to them. > > The main inteded use for the query operation is to export test metadata > and constraints to the test execution system. The long term goal for > this would be parallel test execution as for this case the test runner > would need to know which global system resources is the test using to > avoid unexpected failures. > > So far it exposes only if test needs root and if block device is needed > for the test, but I would expect that we will need a few more tags for > various resources, one that comes to my mind would be "test is using > SystemV SHM" for that we can do something as add a "constraint" tag with > value "SysV SHM" or anything else that would be fitting. Another would > be "Test is changing system wide clocks", etc. > > So far the output from the query operation looks as: > > sh$ ./add_key02 -q > { > "root": false > "blk_device": false > "linux-git": "5649645d725c" > "CVE": "2017-15274" > } So, only way to get metadata for tools is to run the test with -q on supported target? (since I assume when TST_TEST_TCONF branch hits, there won't be any data). Would there be any benefit to having metadata in a (json) file from the start? Negative would be likely duplicating things like "needs_root". Positive is we could check for errors/typos during build time. Regards, Jan > > The format is meant to be machine-parseable, but it is certainly not > final as there are no consumers for the data yet. > > Another nice feature of having this information in the test runner is > that we can include the URL pointing to a kernel git commit fixing the > issue even for kernel reproducers that crashed the kernel under test and > failed to write out any logs. Which is not that important I guess, but > still nice to have.
> -----Original Message----- > From: Cyril Hrubis on Thursday, November 08, 2018 4:52 AM > > Hi! > > > This is just proof of concept of moving some of the test metadata into > > > more structured form. If implemented it will move some information > from > > > comments in the test source code to an array to the tst_test structure. > > > > I have to say, this is really a good proposal to LTP especially for > > that regressions, we sometimes spend a lot of time to dig into test > > failures and read code comments to find the reason/clue, if now > > testcase can print out useful info when trigger the issue, that'd be > > more friendly to LTP runner. > > This is exactly the reason I looked into the problem. I'll go on record as really liking this proposal as well, for the same reasons given. We created a mechanism in Fuego to address this issue, but it would be much better to see it addressed upstream. I'll describe below our "solution" so you can compare it's features with the proposed features. Note that it is only partially implemented and not populated with much information yet (only a few testcases have the files described). > > > Another thought come to my mind is, can we build a correlation for one > > case which have many alias? e.g cve-2017-15274 == add_key02. If LTP > > framework has finished cve-2017-15274 test then run add_key02 in next, > > just skip and mark it as the same result with cve-2017-15274 show. > > Well the way how we track testcases is something that should be probably > rethinked in the future. The runtest files are far from optimal, maybe > we can build something based on tags in the future. I'm a big proponent of having each testcase have a unique identifier, to solve this problem. There were a few slides in the ATS presentation about what I call 'tguids' (Testcase Globally Unique Identifiers), but I didn't have time to get into the rationale for these at the summit. At one Linaro Connect where I presented this idea, Neil Williams gave some good feedback, and pointed out some problems with the idea, but I think it would be good to discuss this concept on the list. I'll try to start a discussion thread on tguids and UTI (uniform testcase identifiers) sometime in the future, to discuss some of the issues. > > > > It's not finished and certainly not set into a stone, this patch is > > > mainly intended to start a discussion. > > > > > > The newly introduced test tags are generic name-value pairs that can > > > hold test metadata, the intended use for now is to store kernel commit > > > hashes for kernel reproducers as well as CVE ids. The mechanism is > > > however choosen to be very generic so that it's easy to add basically > > > any information later on. > > > > > > As it is the main purpose is to print hints for a test failures. If a > > > test that has been written as a kernel reproducer fails it prints nice > > > URL pointing to a kernel commit that may be missing. That's really great. > > > This commit also adds adds the -q test flag, that can be used to query > > > test information, which includes these tags, but is not limited to them. > > > > > > The main inteded use for the query operation is to export test metadata > > > and constraints to the test execution system. The long term goal for > > > this would be parallel test execution as for this case the test runner > > > would need to know which global system resources is the test using to > > > avoid unexpected failures. > > > > > > So far it exposes only if test needs root and if block device is needed > > > for the test, but I would expect that we will need a few more tags for > > > various resources, one that comes to my mind would be "test is using > > > SystemV SHM" for that we can do something as add a "constraint" tag > with > > > value "SysV SHM" or anything else that would be fitting. Another would > > > be "Test is changing system wide clocks", etc. It sounds like you will be preserving test metadata with two different uses: 1) dependencies required for the test to execute 2) possible explanations for test failure There might be a value in keeping these distinct. I can think of some other use categories that meta-data might fall into. One would be: 3) things that need to be (can be) adjusted on the target in order for a test to run (this is different from something that straight-up blocks a test from being able to run on the target) Overall, I think it would be useful to clarify the category and expected handling for the different meta-data that is defined around tests. It also might be good to share different systems constraint/dependency mechanisms and phrasing, for more commonality between systems and easier understanding by users. But that's independent of this hinting thing you're talking about. ---- Here's how we solved the problem of allowing users to share information with each other about testcases, in Fuego. For each test, there is (or can be) a documentation directory, where reStructuredText documents can be placed to describe testcases. It is expected that the directory would be sparse, and that only the "problematical" testcases would have this documentation. The overall idea is to prevent users from having to research failures by digging through code, if someone else had already done that and posted the information. Here is our document for the testcase we call: "Functional.LTP.syscalls.add_key02" The file is between ---------------------------- lines, with additional explanation (this e-mail) below it: File: fuego-core/engine/tests/Functional.LTP/docs/Functional.LTP.syscalls.add_key02.ftmp ---------------------------------------------------- =========== Description =========== Obtained from addkey02.c DESCRIPTION: Test that the add_key() syscall correctly handles a NULL payload with nonzero length. Specifically, it should fail with EFAULT rather than oopsing the kernel with a NULL pointer dereference or failing with EINVAL, as it did before (depending on the key type). This is a regression test for commit 5649645d725c ("KEYS: fix dereferencing NULL payload with nonzero length"). Note that none of the key types that exhibited the NULL pointer dereference are guaranteed to be built into the kernel, so we just test as many as we can, in the hope of catching one. We also test with the "user" key type for good measure, although it was one of the types that failed with EINVAL rather than dereferencing NULL. This has been assigned CVE-2017-15274. Other notes: Commit 5649645d725c appears to have been included since the kernel 4.12. ==== Tags ==== * kernel, syscall, addkey ========= Resources ========= * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c73df4302428ee4e02c869248b4c5 ======= Results ======= Running on a PC (64 bits) using Debian Jessie (kernel 3.16): add_key02.c:81: CONF: kernel doesn't support key type 'asymmetric' add_key02.c:81: CONF: kernel doesn't support key type 'cifs.idmap' add_key02.c:81: CONF: kernel doesn't support key type 'cifs.spnego' add_key02.c:81: CONF: kernel doesn't support key type 'pkcs7_test' add_key02.c:81: CONF: kernel doesn't support key type 'rxrpc' add_key02.c:81: CONF: kernel doesn't support key type 'rxrpc_s' add_key02.c:96: FAIL: unexpected error with key type 'user': EINVAL add_key02.c:96: FAIL: unexpected error with key type 'logon': EINVAL The kernel should have returned an EFAULT error, not EINVAL: Ref: https://github.com/linux-test-project/ltp/issues/182 .. fuego_result_list:: ====== Status ====== .. fuego_status:: ===== Notes ===== ---------------------------------------------------- So, a few more observations on this... The format is rst, with some Sphinx macros. This allows the system to replace the macros with data from the current system (from a set of runs). The macros were not parameterized yet, but the intent was to add parameters to the macros so that a report generated with this file would include a data over a specific time period, or with specific attributes (e.g. only the failures), and indicating what meta-data fields from the test runs to include. Thus, Fuego end-users could customize the output from these using external settings. This was intended to allow us to populate the results interface with nice friendly documents with additional data. This puts the information into a human-readable form, with tables with recent results, but IMHO this doesn't lend itself to additional automation, the way your more-structured tag system does. I could envision in your system a mechanism that went back to the source and did a check using git to see if the kernel included the commit or not, and if so flagging this as a regression. That would be a really neat additional level of results diagnosis/analysis, that could be automated with your system. In any event - that's what we're doing now in Fuego to solve what I think is the same problem. -- Tim P.S. If you want to see additional testcase documentation files in Fuego for LTP, please see: https://bitbucket.org/tbird20d/fuego-core/src/bf8c28cab5ec2dde5552ed2ff1e6fe2e0abf9582/engine/tests/Functional.LTP/docs/ We don't have a lot of them yet, but they show the general pattern of what we were trying for.
> -----Original Message----- > From: Jan Stancek > > Hi, > > ----- Original Message ----- > > This is just proof of concept of moving some of the test metadata into > > more structured form. If implemented it will move some information from > > comments in the test source code to an array to the tst_test structure. > > > > It's not finished and certainly not set into a stone, this patch is > > mainly intended to start a discussion. > > > > The newly introduced test tags are generic name-value pairs that can > > hold test metadata, the intended use for now is to store kernel commit > > hashes for kernel reproducers as well as CVE ids. The mechanism is > > however choosen to be very generic so that it's easy to add basically > > any information later on. > > > > As it is the main purpose is to print hints for a test failures. If a > > test that has been written as a kernel reproducer fails it prints nice > > URL pointing to a kernel commit that may be missing. > > > > Example output: > > > > tst_test.c:1145: INFO: Timeout per run is 0h 05m 00s > > add_key02.c:98: FAIL: unexpected error with key type 'asymmetric': > EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'cifs.idmap': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'cifs.spnego': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'pkcs7_test': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'rxrpc': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'rxrpc_s': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'user': EINVAL > > add_key02.c:98: FAIL: unexpected error with key type 'logon': EINVAL > > > > HINT: This is a regression test for linux kernel, see commit: > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i > d=5649645d725c > > > > HINT: This test also tests for CVE-2017-15274, see: > > > > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15274 > > > > Summary: > > passed 0 > > failed 8 > > skipped 0 > > warnings 0 > > > > This commit also adds adds the -q test flag, that can be used to query > > test information, which includes these tags, but is not limited to them. > > > > The main inteded use for the query operation is to export test metadata > > and constraints to the test execution system. The long term goal for > > this would be parallel test execution as for this case the test runner > > would need to know which global system resources is the test using to > > avoid unexpected failures. > > > > So far it exposes only if test needs root and if block device is needed > > for the test, but I would expect that we will need a few more tags for > > various resources, one that comes to my mind would be "test is using > > SystemV SHM" for that we can do something as add a "constraint" tag with > > value "SysV SHM" or anything else that would be fitting. Another would > > be "Test is changing system wide clocks", etc. > > > > So far the output from the query operation looks as: > > > > sh$ ./add_key02 -q > > { > > "root": false > > "blk_device": false > > "linux-git": "5649645d725c" > > "CVE": "2017-15274" > > } > > So, only way to get metadata for tools is to run the test > with -q on supported target? (since I assume when > TST_TEST_TCONF branch hits, there won't be any data). > > Would there be any benefit to having metadata in a (json) > file from the start? Negative would be likely duplicating > things like "needs_root". Positive is we could check for > errors/typos during build time. I'd like to see a way to extract this data without having to run the test as well. That way test frameworks using LTP could extract the data and handle test scheduling, test dependencies, etc. without having to build or execute the code. In the case of Fuego, test dependencies are analyzed on a separate machine from the one running the test. Also, we try to process some dependencies prior to building the test. One mechanism for storing the data would be a separate json file, but you could also embed the information in the source using some regularized markup (json or something simpler) that could be handled both in-source (for your -q operation), or by an external scanner/converter. -- Tim
Hi! > > So, only way to get metadata for tools is to run the test > > with -q on supported target? (since I assume when > > TST_TEST_TCONF branch hits, there won't be any data). > > > > Would there be any benefit to having metadata in a (json) > > file from the start? Negative would be likely duplicating > > things like "needs_root". Positive is we could check for > > errors/typos during build time. > > I'd like to see a way to extract this data without having to > run the test as well. That way test frameworks using LTP > could extract the data and handle test scheduling, > test dependencies, etc. without having to build or execute > the code. In the case of Fuego, test dependencies are analyzed > on a separate machine from the one running the test. Also, > we try to process some dependencies prior to building the test. What I had in mind was some additional database build step where all tests would be executed with -q parameter on the target machine which would create a big structured file with all the metadata. So first thing before any test would be executed would be a script that would check if there is a database file or not and and if there is none it would build it, then we can get the database from the target machine and the test runner can make use of that. This process has to be either part of the LTP build or has to be executed before first testrun anyways, sice there is no other way to keep the data in sync with the binaries. > One mechanism for storing the data would be a separate json > file, but you could also embed the information in the source > using some regularized markup (json or something simpler) > that could be handled both in-source (for your -q operation), > or by an external scanner/converter. Actually I've started to think that this may be the answer, if we manage to encode the metadata into C string as well as into some structured form we can have test description as well as some of the metadata available both at the runtime as well as in a format that could be parsed without compiling the test. But there are drawbacks, I do not think that it's sane to encode if test requires root or not, or if it needs a block device in some kind of markup text. I think that it's mutch better when it's encoded in the tst_test structure as it is now.
Hi! > > > Another thought come to my mind is, can we build a correlation for one > > > case which have many alias? e.g cve-2017-15274 == add_key02. If LTP > > > framework has finished cve-2017-15274 test then run add_key02 in next, > > > just skip and mark it as the same result with cve-2017-15274 show. > > > > Well the way how we track testcases is something that should be probably > > rethinked in the future. The runtest files are far from optimal, maybe > > we can build something based on tags in the future. > > I'm a big proponent of having each testcase have a unique identifier, to > solve this problem. There were a few slides in the ATS presentation about > what I call 'tguids' (Testcase Globally Unique Identifiers), but I didn't have > time to get into the rationale for these at the summit. > > At one Linaro Connect where I presented this idea, Neil Williams gave > some good feedback, and pointed out some problems with the idea, > but I think it would be good to discuss this concept on the list. > > I'll try to start a discussion thread on tguids and UTI (uniform testcase > identifiers) sometime in the future, to discuss some of the issues. That's really great, thanks for looking into this :-). > > > > This commit also adds adds the -q test flag, that can be used to query > > > > test information, which includes these tags, but is not limited to them. > > > > > > > > The main inteded use for the query operation is to export test metadata > > > > and constraints to the test execution system. The long term goal for > > > > this would be parallel test execution as for this case the test runner > > > > would need to know which global system resources is the test using to > > > > avoid unexpected failures. > > > > > > > > So far it exposes only if test needs root and if block device is needed > > > > for the test, but I would expect that we will need a few more tags for > > > > various resources, one that comes to my mind would be "test is using > > > > SystemV SHM" for that we can do something as add a "constraint" tag > > with > > > > value "SysV SHM" or anything else that would be fitting. Another would > > > > be "Test is changing system wide clocks", etc. > > It sounds like you will be preserving test metadata with two different uses: > 1) dependencies required for the test to execute > 2) possible explanations for test failure > > There might be a value in keeping these distinct. It's a bit more complicated than this in LTP we have basically three sets: 1) test dependencies that could be derived from the test structure (these are the needs_root, needs_device, but also needs mkfs.foo or others) 2) test dependencies that has to be specified explicitely (test is doing something with global resources, SysV IPC, Wall clock) 3) test metadata that explain the test (these are test description, kernel commmit ids, CVEs, etc.) Now 2 and 3 is not completely distinct, since "test is testing SysV IPC" is both constraint which means that such test most of the time cannot be executed in parallel, but it's also test documentation. Also if possible I would like to avoid specifying 1) in 2) again, which most likely means that we have either compile and run the code or do some hackery around grepping the test source. > I can think of some other use categories that meta-data > might fall into. One would be: > 3) things that need to be (can be) adjusted on the target in > order for a test to run (this is different from something > that straight-up blocks a test from being able to run on the target) Ah, right, and then there are parameters that can be tuned to provide diferent test variants. > Overall, I think it would be useful to clarify the category and > expected handling for the different meta-data that is defined > around tests. Hmm, but still in the end we want to propagate these parameters from the tests to the automated framework so that we can make use of them when results are produced right? > It also might be good to share different systems constraint/dependency > mechanisms and phrasing, for more commonality between systems and > easier understanding by users. But that's independent of this hinting > thing you're talking about. Sure. I do prefer to work on actuall implementation though :-). > ---- > Here's how we solved the problem of allowing users to share > information with each other about testcases, in Fuego. > > For each test, there is (or can be) a documentation directory, where > reStructuredText documents can be placed to describe testcases. It is > expected that the directory would be sparse, and that only the > "problematical" testcases would have this documentation. > > The overall idea is to prevent users from having to research > failures by digging through code, if someone else had already > done that and posted the information. > > Here is our document for the testcase we call: "Functional.LTP.syscalls.add_key02" > The file is between ---------------------------- lines, with additional explanation (this e-mail) > below it: > > File: fuego-core/engine/tests/Functional.LTP/docs/Functional.LTP.syscalls.add_key02.ftmp > ---------------------------------------------------- > =========== > Description > =========== > > Obtained from addkey02.c DESCRIPTION: > Test that the add_key() syscall correctly handles a NULL payload with nonzero > length. Specifically, it should fail with EFAULT rather than oopsing the > kernel with a NULL pointer dereference or failing with EINVAL, as it did > before (depending on the key type). This is a regression test for commit > 5649645d725c ("KEYS: fix dereferencing NULL payload with nonzero length"). > > Note that none of the key types that exhibited the NULL pointer dereference > are guaranteed to be built into the kernel, so we just test as many as we > can, in the hope of catching one. We also test with the "user" key type for > good measure, although it was one of the types that failed with EINVAL rather > than dereferencing NULL. > > This has been assigned CVE-2017-15274. > > Other notes: > Commit 5649645d725c appears to have been included since the kernel 4.12. > > ==== > Tags > ==== > > * kernel, syscall, addkey This is exactly what I was thinking about when I was speaking about tagging the tests. Another thing that comes to my mind is that we could also define tag groups, for instance key_management would group consiting of add_key, request_key, keyctl, ... Then group called security would include key_management and possibly some crypto stuff. Once the testcases are tagged like this we can filter out tests based on the area they are testing and we do not need to maintain different runtest files anymore. > ========= > Resources > ========= > > * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c73df4302428ee4e02c869248b4c5 > > ======= > Results > ======= > > Running on a PC (64 bits) using Debian Jessie (kernel 3.16): > add_key02.c:81: CONF: kernel doesn't support key type 'asymmetric' > add_key02.c:81: CONF: kernel doesn't support key type 'cifs.idmap' > add_key02.c:81: CONF: kernel doesn't support key type 'cifs.spnego' > add_key02.c:81: CONF: kernel doesn't support key type 'pkcs7_test' > add_key02.c:81: CONF: kernel doesn't support key type 'rxrpc' > add_key02.c:81: CONF: kernel doesn't support key type 'rxrpc_s' > add_key02.c:96: FAIL: unexpected error with key type 'user': EINVAL > add_key02.c:96: FAIL: unexpected error with key type 'logon': EINVAL > > The kernel should have returned an EFAULT error, not EINVAL: > Ref: https://github.com/linux-test-project/ltp/issues/182 > > .. fuego_result_list:: > > ====== > Status > ====== > > .. fuego_status:: > > ===== > Notes > ===== > ---------------------------------------------------- > > So, a few more observations on this... > The format is rst, with some Sphinx macros. This allows > the system to replace the macros with data from the current system > (from a set of runs). The macros were not parameterized yet, but > the intent was to add parameters to the macros so that a report > generated with this file would include a data over a specific time > period, or with specific attributes (e.g. only the failures), and indicating > what meta-data fields from the test runs to include. Thus, Fuego > end-users could customize the output from these using external > settings. This was intended to allow us to populate the results > interface with nice friendly documents with additional data. > > This puts the information into a human-readable form, with > tables with recent results, but IMHO this doesn't lend itself to > additional automation, the way your more-structured tag system > does. I could envision in your system a mechanism that went back > to the source and did a check using git to see if the kernel included > the commit or not, and if so flagging this as a regression. That would > be a really neat additional level of results diagnosis/analysis, that > could be automated with your system. > > In any event - that's what we're doing now in Fuego to solve what > I think is the same problem. > -- Tim > > P.S. If you want to see additional testcase documentation files in Fuego > for LTP, please see: > https://bitbucket.org/tbird20d/fuego-core/src/bf8c28cab5ec2dde5552ed2ff1e6fe2e0abf9582/engine/tests/Functional.LTP/docs/ > We don't have a lot of them yet, but they show the general pattern of > what we were trying for. It does look nice. I had something like this in my mind when I was talking about well defined format for test descriptions and rst looks like a good format for that, maybe we should just adapt it here.
> -----Original Message----- > From: Cyril Hrubis ... > > > > > This commit also adds adds the -q test flag, that can be used to query > > > > > test information, which includes these tags, but is not limited to them. > > > > > > > > > > The main inteded use for the query operation is to export test > metadata > > > > > and constraints to the test execution system. The long term goal for > > > > > this would be parallel test execution as for this case the test runner > > > > > would need to know which global system resources is the test using > to > > > > > avoid unexpected failures. I don't think I was paying enough attention when I read through this earlier. This is very interesting, and different from the external dependencies I've focused on in my analysis of dependencies. But it's certainly as important as what I've thought about in Fuego. > > > > > > > > > > So far it exposes only if test needs root and if block device is needed > > > > > for the test, but I would expect that we will need a few more tags for > > > > > various resources, one that comes to my mind would be "test is using > > > > > SystemV SHM" for that we can do something as add a "constraint" tag > > > with > > > > > value "SysV SHM" or anything else that would be fitting. Another > would > > > > > be "Test is changing system wide clocks", etc. > > > > It sounds like you will be preserving test metadata with two different uses: > > 1) dependencies required for the test to execute > > 2) possible explanations for test failure > > > > There might be a value in keeping these distinct. > > It's a bit more complicated than this in LTP we have basically three sets: > > 1) test dependencies that could be derived from the test structure > (these are the needs_root, needs_device, but also needs mkfs.foo or > others) > > 2) test dependencies that has to be specified explicitely > (test is doing something with global resources, SysV IPC, Wall > clock) Ahhh. This turned on a light bulb in my head, and was a new thought for me, that really blew my mind. The reason is that Fuego runs only one test at a time on the target, so any test automatically has exclusive access (from a test standpoint) to on-target resources. This is a result of how Fuego uses Jenkins to schedule testing jobs. I've been focused on off-target resource allocation (like the netperf server, or off-board power measurement hardware). It's important for tests that work in the same "lab" to be able to reserve these exclusively to avoid contention and perturbation of the test results. And this is a difficult problem because there are no standards whatsoever for doing this. This ends up being a test scheduling issue (and possibly a deadlock avoidance/resolution issue). But this dependency you mention is focused on on-target resources, to which a test might also need exclusive access. This also presents a test scheduling issue. A few questions: Can LTP run multiple tests simultaneously? I seem to recall something about ltp-pan running multiple tests at the same time (for stress testing). Does LTP have a mechanism to "claim" or "reserve" a resource on the system under test? Does LTP have a mechanism to schedule tests? That is, to check for resource availability and hold off test execution until a resource becomes available? Thanks. I really appreciate that you posted this to the automated-testing list so we could think about it and the problems it's solving for you. -- Tim
> -----Original Message----- > From: Cyril Hrubis > > Hi! > > > So, only way to get metadata for tools is to run the test > > > with -q on supported target? (since I assume when > > > TST_TEST_TCONF branch hits, there won't be any data). > > > > > > Would there be any benefit to having metadata in a (json) > > > file from the start? Negative would be likely duplicating > > > things like "needs_root". Positive is we could check for > > > errors/typos during build time. > > > > I'd like to see a way to extract this data without having to > > run the test as well. That way test frameworks using LTP > > could extract the data and handle test scheduling, > > test dependencies, etc. without having to build or execute > > the code. In the case of Fuego, test dependencies are analyzed > > on a separate machine from the one running the test. Also, > > we try to process some dependencies prior to building the test. > > What I had in mind was some additional database build step where > all tests would be executed with -q parameter on the target machine > which would create a big structured file with all the metadata. So first > thing before any test would be executed would be a script that would > check if there is a database file or not and and if there is none it > would build it, then we can get the database from the target machine > and the test runner can make use of that. > > This process has to be either part of the LTP build or has to be > executed before first testrun anyways, sice there is no other way > to keep the data in sync with the binaries. OK - I think we could make that work in Fuego. Is any of the information you are encoding architecture-specific? It might be convenient for Fuego to build LTP on x86_64 to extract this data, and then build the actual test binaries for the target architecture in a separate step. But that wouldn't work if the x86_64 binaries gave different '-q' results than the (say) ARM binaries. > > > One mechanism for storing the data would be a separate json > > file, but you could also embed the information in the source > > using some regularized markup (json or something simpler) > > that could be handled both in-source (for your -q operation), > > or by an external scanner/converter. > > Actually I've started to think that this may be the answer, if we manage > to encode the metadata into C string as well as into some structured > form we can have test description as well as some of the metadata > available both at the runtime as well as in a format that could be > parsed without compiling the test. > > But there are drawbacks, I do not think that it's sane to encode if test > requires root or not, or if it needs a block device in some kind of > markup text. I think that it's mutch better when it's encoded in the > tst_test structure as it is now. If there's a standard way of expressing this that can be reliably grepp'ed, I don't think everything needs to be in a structured form. As long as it's not too difficult to write a parser, and there are some conventions that naturally keep the data parsable, I think having the metadata in C strings is fine. For example, "grep needs_root -R * | grep -v Binary" shows a list which is pretty clear. Maybe it's missing some instances, due to a program setting setting this field in a weird way, but I kind of doubt it. (This field is usually declared statically like this, right? It would be harder to parse if needs_root is assigned at runtime.) -- Tim
Hi! > > > It sounds like you will be preserving test metadata with two different uses: > > > 1) dependencies required for the test to execute > > > 2) possible explanations for test failure > > > > > > There might be a value in keeping these distinct. > > > > It's a bit more complicated than this in LTP we have basically three sets: > > > > 1) test dependencies that could be derived from the test structure > > (these are the needs_root, needs_device, but also needs mkfs.foo or > > others) > > > > 2) test dependencies that has to be specified explicitely > > (test is doing something with global resources, SysV IPC, Wall > > clock) > > Ahhh. This turned on a light bulb in my head, and was a new thought > for me, that really blew my mind. > > The reason is that Fuego runs only one test at a time on the target, so > any test automatically has exclusive access (from a test standpoint) > to on-target resources. This is a result of how Fuego uses Jenkins > to schedule testing jobs. > > I've been focused on off-target resource allocation (like the netperf > server, or off-board power measurement hardware). It's important > for tests that work in the same "lab" to be able to reserve these > exclusively to avoid contention and perturbation of the test results. > And this is a difficult problem because there are no standards whatsoever > for doing this. This ends up being a test scheduling issue (and possibly > a deadlock avoidance/resolution issue). > > But this dependency you mention is focused on on-target resources, to > which a test might also need exclusive access. This also presents a > test scheduling issue. > > A few questions: > > Can LTP run multiple tests simultaneously? I seem to recall something > about ltp-pan running multiple tests at the same time (for stress testing). Not at the moment, but given that even embedded hardware have multicore CPUs these days it's a waste of resources and even more on server grade hardware with many cores. There is subset of tests that will fail if the machine is under load for example, which also prevents us from runnig the testsuite with some kind of backgroud CPU load and under memory pressure, which is just another reason why we need annotations like this. > Does LTP have a mechanism to "claim" or "reserve" a resource > on the system under test? There is none, which is the reason I started to look into this. My take on the problem is that each test would export this information to the testrunner so that the testrunner can make informed decisions. It could be as simple as a sinle bit of data that tells the testrunner not to run the tests in parallel, but I would like to be a bit more granular from the start. For instance we can run a few tests that require loop device in parallel as well, no need to limit this. > Does LTP have a mechanism to schedule tests? That is, to check for > resource availability and hold off test execution until a resource > becomes available? Not yet, this is what I'm currently looking into. But I do not see any complications down the road, once the testrunner has the information, we just need to refrain from running conflicting tests while saturating the CPU usage with running NCPU+1 test at a time or so. > Thanks. I really appreciate that you posted this to the automated-testing > list so we could think about it and the problems it's solving for you. And I do appreciate that there is some discussion around this :-).
Hi! > > > I'd like to see a way to extract this data without having to > > > run the test as well. That way test frameworks using LTP > > > could extract the data and handle test scheduling, > > > test dependencies, etc. without having to build or execute > > > the code. In the case of Fuego, test dependencies are analyzed > > > on a separate machine from the one running the test. Also, > > > we try to process some dependencies prior to building the test. > > > > What I had in mind was some additional database build step where > > all tests would be executed with -q parameter on the target machine > > which would create a big structured file with all the metadata. So first > > thing before any test would be executed would be a script that would > > check if there is a database file or not and and if there is none it > > would build it, then we can get the database from the target machine > > and the test runner can make use of that. > > > > This process has to be either part of the LTP build or has to be > > executed before first testrun anyways, sice there is no other way > > to keep the data in sync with the binaries. > > OK - I think we could make that work in Fuego. > > Is any of the information you are encoding architecture-specific? As it is test are disabled on missing library headers on compile time and in rare case based on architecture. In that case the tst_test structure is ifdefed out and the test library only gets a dummy one which means that there will be no data. I guess that I will have to look into that. > It might be convenient for Fuego to build LTP on x86_64 to extract > this data, and then build the actual test binaries for the target architecture > in a separate step. But that wouldn't work if the x86_64 > binaries gave different '-q' results than the (say) ARM binaries. Agreed, the cross compilation would become much more complex if we required to run target binaries. > > > One mechanism for storing the data would be a separate json > > > file, but you could also embed the information in the source > > > using some regularized markup (json or something simpler) > > > that could be handled both in-source (for your -q operation), > > > or by an external scanner/converter. > > > > Actually I've started to think that this may be the answer, if we manage > > to encode the metadata into C string as well as into some structured > > form we can have test description as well as some of the metadata > > available both at the runtime as well as in a format that could be > > parsed without compiling the test. > > > > But there are drawbacks, I do not think that it's sane to encode if test > > requires root or not, or if it needs a block device in some kind of > > markup text. I think that it's mutch better when it's encoded in the > > tst_test structure as it is now. > If there's a standard way of expressing this that can be reliably grepp'ed, > I don't think everything needs to be in a structured form. > As long as it's not too difficult to write a parser, and there are > some conventions that naturally keep the data parsable, I think having > the metadata in C strings is fine. > > For example, "grep needs_root -R * | grep -v Binary" shows a list which > is pretty clear. Maybe it's missing some instances, due to a program setting > setting this field in a weird way, but I kind of doubt it. > (This field is usually declared statically like this, right? > It would be harder to parse if needs_root is assigned at runtime.) In the new library it's all static data passed in the test library. The old LTP API was a mess of random functions, so at some point I've decided to rewrite it so that we specify most of the test information in a form of a constant data. However so far we managed to covert about 30% of tests to the new API and converting the rest will take a few more years...
diff --git a/include/tst_test.h b/include/tst_test.h index 2ebf746eb..7af540854 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -110,6 +110,11 @@ int tst_parse_int(const char *str, int *val, int min, int max); int tst_parse_long(const char *str, long *val, long min, long max); int tst_parse_float(const char *str, float *val, float min, float max); +struct tst_tag { + const char *name; + const char *value; +}; + struct tst_test { /* number of tests available in test() function */ unsigned int tcnt; @@ -182,6 +187,11 @@ struct tst_test { * before setup and restore after cleanup */ const char * const *save_restore; + + /* + * {NULL, NULL} terminated array of tags. + */ + const struct tst_tag *tags; }; /* diff --git a/lib/tst_test.c b/lib/tst_test.c index 661fbbfce..976f67135 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -406,6 +406,7 @@ static struct option { {"h", "-h Prints this help"}, {"i:", "-i n Execute test n times"}, {"I:", "-I x Execute test for n seconds"}, + {"q", "-q Queries test information"}, {"C:", "-C ARG Run child process with ARG arguments (used internally)"}, }; @@ -423,6 +424,25 @@ static void print_help(void) fprintf(stderr, "%s\n", tst_test->options[i].help); } +static void print_test_info(void) +{ + unsigned int i; + const struct tst_tag *tags = tst_test->tags; + int needs_device = tst_test->needs_device || tst_test->needs_rofs; + + printf("{\n"); + + printf(" \"root\": %s\n", tst_test->needs_root ? "true" : "false"); + printf(" \"blk_device\": %s\n", needs_device ? "true" : "false"); + + if (tags) { + for (i = 0; tags[i].name; i++) + printf(" \"%s\": \"%s\"\n", tags[i].name, tags[i].value); + } + + printf("}\n"); +} + static void check_option_collision(void) { unsigned int i, j; @@ -505,6 +525,10 @@ static void parse_opts(int argc, char *argv[]) case 'I': duration = atof(optarg); break; + case 'q': + print_test_info(); + exit(0); + break; case 'C': #ifdef UCLINUX child_args = optarg; @@ -583,26 +607,64 @@ int tst_parse_float(const char *str, float *val, float min, float max) return 0; } +#define LINUX_GIT_URL "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=" +#define CVE_DB_URL "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-" + +static void print_colored(const char *str) +{ + if (tst_color_enabled(STDOUT_FILENO)) + printf("%s%s%s", ANSI_COLOR_YELLOW, str, ANSI_COLOR_RESET); + else + printf("%s", str); +} + +static void print_failure_hints(void) +{ + unsigned int i; + const struct tst_tag *tags = tst_test->tags; + + if (!tags) + return; + + for (i = 0; tags[i].name; i++) { + if (!strcmp(tags[i].name, "linux-git")) { + printf("\n"); + print_colored("HINT: "); + printf("This is a regression test for linux kernel, see commit:\n\n" + LINUX_GIT_URL "%s\n", tags[i].value); + } + + if (!strcmp(tags[i].name, "CVE")) { + printf("\n"); + print_colored("HINT: "); + printf("This test also tests for CVE-%s, see:\n\n" + CVE_DB_URL "%s\n", tags[i].value, tags[i].value); + } + } +} + static void do_exit(int ret) { if (results) { - printf("\nSummary:\n"); - printf("passed %d\n", results->passed); - printf("failed %d\n", results->failed); - printf("skipped %d\n", results->skipped); - printf("warnings %d\n", results->warnings); - if (results->passed && ret == TCONF) ret = 0; - if (results->failed) + if (results->failed) { ret |= TFAIL; + print_failure_hints(); + } if (results->skipped && !results->passed) ret |= TCONF; if (results->warnings) ret |= TWARN; + + printf("\nSummary:\n"); + printf("passed %d\n", results->passed); + printf("failed %d\n", results->failed); + printf("skipped %d\n", results->skipped); + printf("warnings %d\n", results->warnings); } do_cleanup(); @@ -777,6 +839,17 @@ static void do_setup(int argc, char *argv[]) if (tst_test->sample) tst_test = tst_timer_test_setup(tst_test); + if (tst_test->format_device) + tst_test->needs_device = 1; + + if (tst_test->mount_device) { + tst_test->needs_device = 1; + tst_test->format_device = 1; + } + + if (tst_test->all_filesystems) + tst_test->needs_device = 1; + parse_opts(argc, argv); if (tst_test->needs_root && geteuid() != 0) @@ -794,17 +867,6 @@ static void do_setup(int argc, char *argv[]) tst_brk(TCONF, "%s driver not available", name); } - if (tst_test->format_device) - tst_test->needs_device = 1; - - if (tst_test->mount_device) { - tst_test->needs_device = 1; - tst_test->format_device = 1; - } - - if (tst_test->all_filesystems) - tst_test->needs_device = 1; - setup_ipc(); if (needs_tmpdir() && !tst_tmpdir_created()) diff --git a/testcases/kernel/syscalls/add_key/add_key02.c b/testcases/kernel/syscalls/add_key/add_key02.c index 6d19ff2d8..9e23d9eb5 100644 --- a/testcases/kernel/syscalls/add_key/add_key02.c +++ b/testcases/kernel/syscalls/add_key/add_key02.c @@ -70,6 +70,8 @@ static void verify_add_key(unsigned int i) return; } + TST_ERR = EINVAL; + if (TST_ERR == EFAULT) { tst_res(TPASS, "received expected EFAULT with key type '%s'", tcases[i].type); @@ -99,4 +101,9 @@ static void verify_add_key(unsigned int i) static struct tst_test test = { .tcnt = ARRAY_SIZE(tcases), .test = verify_add_key, + .tags = (const struct tst_tag[]) { + {"linux-git", "5649645d725c"}, + {"CVE", "2017-15274"}, + {NULL, NULL} + } };
This is just proof of concept of moving some of the test metadata into more structured form. If implemented it will move some information from comments in the test source code to an array to the tst_test structure. It's not finished and certainly not set into a stone, this patch is mainly intended to start a discussion. The newly introduced test tags are generic name-value pairs that can hold test metadata, the intended use for now is to store kernel commit hashes for kernel reproducers as well as CVE ids. The mechanism is however choosen to be very generic so that it's easy to add basically any information later on. As it is the main purpose is to print hints for a test failures. If a test that has been written as a kernel reproducer fails it prints nice URL pointing to a kernel commit that may be missing. Example output: tst_test.c:1145: INFO: Timeout per run is 0h 05m 00s add_key02.c:98: FAIL: unexpected error with key type 'asymmetric': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'cifs.idmap': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'cifs.spnego': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'pkcs7_test': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'rxrpc': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'rxrpc_s': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'user': EINVAL add_key02.c:98: FAIL: unexpected error with key type 'logon': EINVAL HINT: This is a regression test for linux kernel, see commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c HINT: This test also tests for CVE-2017-15274, see: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15274 Summary: passed 0 failed 8 skipped 0 warnings 0 This commit also adds adds the -q test flag, that can be used to query test information, which includes these tags, but is not limited to them. The main inteded use for the query operation is to export test metadata and constraints to the test execution system. The long term goal for this would be parallel test execution as for this case the test runner would need to know which global system resources is the test using to avoid unexpected failures. So far it exposes only if test needs root and if block device is needed for the test, but I would expect that we will need a few more tags for various resources, one that comes to my mind would be "test is using SystemV SHM" for that we can do something as add a "constraint" tag with value "SysV SHM" or anything else that would be fitting. Another would be "Test is changing system wide clocks", etc. So far the output from the query operation looks as: sh$ ./add_key02 -q { "root": false "blk_device": false "linux-git": "5649645d725c" "CVE": "2017-15274" } The format is meant to be machine-parseable, but it is certainly not final as there are no consumers for the data yet. Another nice feature of having this information in the test runner is that we can include the URL pointing to a kernel git commit fixing the issue even for kernel reproducers that crashed the kernel under test and failed to write out any logs. Which is not that important I guess, but still nice to have. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> CC: automated-testing@yoctoproject.org --- include/tst_test.h | 10 +++ lib/tst_test.c | 98 ++++++++++++++++++++++----- testcases/kernel/syscalls/add_key/add_key02.c | 7 ++ 3 files changed, 97 insertions(+), 18 deletions(-)