Re: [PATCH -next v2] selftests/bpf: replace fall through comment by fallthrough pseudo-keyword

[Hou Tao <houtao@xxxxxxxxxxxxxxx>]

Hi,

On 8/1/2023 2:54 PM, Ruan Jinjie wrote:
> Replace the existing /* fall through */ comments with the
> new pseudo-keyword macro fallthrough[1].
>
> [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
>
> Signed-off-by: Ruan Jinjie <ruanjinjie@xxxxxxxxxx>
> ---
> v2:
> - Update the subject and commit message.

According to the section "How do I indicate which tree (bpf vs.
bpf-next) my patch should be applied to" in
Documentation/bpf/bpf_devel_QA.rst, the subject prefix should be [PATCH
bpf-next].
> ---
>  tools/testing/selftests/bpf/prog_tests/kfunc_call.c          | 4 ++--
>  tools/testing/selftests/bpf/progs/test_cls_redirect.c        | 2 +-
>  tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c | 2 +-
>  tools/testing/selftests/bpf/test_verifier.c                  | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> index a543742cd7bd..0fd08172965a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
> @@ -101,7 +101,7 @@ static void verify_success(struct kfunc_test_params *param)
>  	case syscall_test:
>  		topts.ctx_in = &args;
>  		topts.ctx_size_in = sizeof(args);
> -		/* fallthrough */
> +		fallthrough;
>  	case syscall_null_ctx_test:
>  		break;
>  	case tc_test:
> @@ -167,7 +167,7 @@ static void verify_fail(struct kfunc_test_params *param)
>  	case syscall_test:
>  		topts.ctx_in = &args;
>  		topts.ctx_size_in = sizeof(args);
> -		/* fallthrough */
> +		fallthrough;
>  	case syscall_null_ctx_test:
>  		break;
>  	case tc_test:
> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> index 66b304982245..f97960759558 100644
> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
> @@ -300,7 +300,7 @@ bool pkt_skip_ipv6_extension_headers(buf_t *pkt,
>  		case IPPROTO_FRAGMENT:
>  			*is_fragment = true;
>  			/* NB: We don't check that hdrlen == 0 as per spec. */
> -			/* fallthrough; */
> +			fallthrough;

The build of test_progs failed as shown below. Have you tested your
patch locally ?

progs/test_cls_redirect.c:292:4: In file included from
progs/test_cls_redirect_subprogs.cerror: :2:
use of undeclared identifier 'fallthrough'
progs/test_cls_redirect.c:292:4: error: use of undeclared identifier
'fallthrough'
                        fallthrough;
                        ^
                        fallthrough;
                        ^


>  
>  		case IPPROTO_HOPOPTS:
>  		case IPPROTO_ROUTING:
> diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
> index f41c81212ee9..54dbf307c692 100644
> --- a/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
> +++ b/tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
> @@ -204,7 +204,7 @@ static bool pkt_skip_ipv6_extension_headers(struct bpf_dynptr *dynptr, __u64 *of
>  		case IPPROTO_FRAGMENT:
>  			*is_fragment = true;
>  			/* NB: We don't check that hdrlen == 0 as per spec. */
> -			/* fallthrough; */
> +			fallthrough;
>  
>  		case IPPROTO_HOPOPTS:
>  		case IPPROTO_ROUTING:
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 31f1c935cd07..5621a4e0a1be 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -1289,7 +1289,7 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
>  				printf("Did not run the program (no permission) ");
>  				return 0;
>  			}
> -			/* fallthrough; */
> +			fallthrough;
>  		default:
>  			printf("FAIL: Unexpected bpf_prog_test_run error (%s) ",
>  				strerror(saved_errno));