Skip to content

selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage - #13389

Open
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf-next_basefrom
series/1149350=>bpf-next
Open

selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage#13389
kernel-patches-daemon-bpf[bot] wants to merge 1 commit into
bpf-next_basefrom
series/1149350=>bpf-next

Conversation

@kernel-patches-daemon-bpf

Copy link
Copy Markdown

Pull request for series with
subject: selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage
version: 3
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: d19862d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-review-bot

Copy link
Copy Markdown
commit 13acc020a68a79fa42ce134695e01654e17c1edf
Author: Sean Young <sean@mess.org>

selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage

This commit converts the lirc_mode2 selftest from the old
TEST_PROGS/TEST_GEN_PROGS_EXTENDED style (shell wrapper +
standalone user program) to the prog_tests framework, and adds
coverage for attach/detach/query API edge cases.

> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 986a6389186b..b815bf0d8877 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -17,7 +17,6 @@ test_verifier_log
>  feature
>  urandom_read
>  test_sockmap
> -test_lirc_mode2_user
>  flow_dissector_load
>  test_tcpnotify_user
>  test_libbpf

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/lirc_mode2.c b/tools/testing/selftests/bpf/prog_tests/lirc_mode2.c
> new file mode 100644
> index 000000000000..7b7352dbcd82
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/lirc_mode2.c
> @@ -0,0 +1,334 @@

[ ... ]

> +// This test works by sending IR over rc-loopback, so the IR is processed by
> +// BPF and then decoded into scancodes. The lirc chardev must be the one
> +// associated with rc-loopback, see the output of ir-keytable(1).

This isn't a bug, but now that find_loopback_devices() discovers the
nodes itself, should the ir-keytable(1) sentence be reworded (or
dropped) so the header does not describe a manual step that no longer
exists?

> +//
> +// The following CONFIG options must be enabled for the test to succeed:
> +// CONFIG_RC_CORE=y
> +// CONFIG_BPF_LIRC_MODE2=y
> +// CONFIG_RC_LOOPBACK=y
> +// CONFIG_LIRC=y
> +
> +#include <linux/input.h>
> +#include <linux/lirc.h>

This isn't a bug, but is <linux/lirc.h> still needed in
prog_tests/lirc_mode2.c, or can it be dropped now that the raw-IR
values are written as plain ints?

> +#include <glob.h>
> +#include <limits.h>
> +#include <poll.h>
> +#include <test_progs.h>
> +#include "lirc_mode2.skel.h"
> +
> +/*
> + * Read the DEVNAME= line out of the first uevent file that matches
> + * pattern, and turn it into a /dev/<name> path.
> + */
> +static bool find_devname(const char *pattern, char *path, size_t path_sz)
> +{
> +	glob_t gl = {};
> +	bool found = false;
> +	FILE *f;
> +
> +	if (glob(pattern, 0, NULL, &gl) || gl.gl_pathc == 0)
> +		goto out;
> +
> +	f = fopen(gl.gl_pathv[0], "r");
> +	if (!f)
> +		goto out;
> +
> +	char line[256];

This isn't a bug, but could `char line[256]` move up with the other
locals at the top of find_devname(), to match the rest of the function?

> +
> +	while (fgets(line, sizeof(line), f)) {
> +		char *val;
> +
> +		if (strncmp(line, "DEVNAME=", 8))
> +			continue;
> +
> +		val = line + 8;
> +		val[strcspn(val, "\n")] = '\0';
> +		snprintf(path, path_sz, "/dev/%s", val);
> +		found = true;
> +		break;
> +	}
> +
> +	fclose(f);
> +out:
> +	globfree(&gl);
> +	return found;
> +}

[ ... ]

> commit 13acc020a68a79fa42ce134695e01654e17c1edf
> Author: Sean Young <sean@mess.org>
>
>     selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage
>
>     While porting, extend coverage of the BPF_LIRC_MODE2 attach/detach/
>     query API:
>
>      - bpf_prog_attach() with invalid flags is rejected with -EINVAL
>        and does not attach the program
>      - bpf_prog_query() with invalid flags is rejected with -EINVAL
>        without disturbing existing attachments
>      - bpf_prog_query() reports the correct program id, not just count,
>        at each step, via bpf_prog_get_info_by_fd()
>      - a lirc chardev can hold more than one attached program: load a
>        second, independent instance, attach it alongside the first,
>        confirm both are reported by bpf_prog_query(), then detach it
>        without disturbing the first program's attachment
>      - detaching an already-detached program consistently fails with
>        -ENOENT, for both the first and second program

This isn't a bug, but could the changelog open with why the test is
being moved under test_progs, before listing the extra coverage that
was added while porting?

The changelog body starts at the secondary half of the change and
enumerates the new assertions, but never states the primary change or
its motivation. The conversion itself - deleting test_lirc_mode2.sh
plus test_lirc_mode2_user.c and the standalone TEST_PROGS/
TEST_GEN_PROGS_EXTENDED wiring, and re-adding the test as a prog_tests
entry with a skeleton - is what most of the diff does, and a reader
learns only what was added on top of it, not why running under
test_progs is preferable (single runner, DENYLIST/CI integration, no
separate binary to install).



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: selftests/bpf: convert lirc_mode2 to prog_tests and extend coverage
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32422213902

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 4191e49
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 4954de7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: e40f61a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 669e4fa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: a284ed4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 5e289c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: d83fba2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: ce36e38
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 05ea1b6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 1555de3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

@kernel-patches-daemon-bpf

Copy link
Copy Markdown
Author

Upstream branch: 48b69cc
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1149350
version: 3

While porting, extend coverage of the BPF_LIRC_MODE2 attach/detach/
query API:

 - bpf_prog_attach() with invalid flags is rejected with -EINVAL
   and does not attach the program
 - bpf_prog_query() with invalid flags is rejected with -EINVAL
   without disturbing existing attachments
 - bpf_prog_query() reports the correct program id, not just count,
   at each step, via bpf_prog_get_info_by_fd()
 - a lirc chardev can hold more than one attached program: load a
   second, independent instance, attach it alongside the first,
   confirm both are reported by bpf_prog_query(), then detach it
   without disturbing the first program's attachment
 - detaching an already-detached program consistently fails with
   -ENOENT, for both the first and second program

Signed-off-by: Sean Young <sean@mess.org>
Assisted-by: Claude:claude-sonnet-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant