Re: [PATCH bpf-next v9 03/11] selftests/bpf: Tolerate BPF and skeleton generation failures

["Ricardo B. Marlière" <rbm@suse.com>]

On Wed Apr 29, 2026 at 12:13 PM -03, bot+bpf-ci wrote:
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index cc6ee7a2df93..b104c687dcf0 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>
> [ ... ]
>
>> @@ -607,47 +611,81 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:				\
>>  					  $$($$<-$2-CFLAGS),$(TRUNNER_BINARY))
>>
>>  $(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
>> -	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
>> -	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked1.o) $$<
>> -	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked2.o) $$(<:.o=.linked1.o)
>> -	$(Q)$$(BPFTOOL) gen object $$(<:.o=.linked3.o) $$(<:.o=.linked2.o)
>> -	$(Q)diff $$(<:.o=.linked2.o) $$(<:.o=.linked3.o)
>> -	$(Q)$$(BPFTOOL) gen skeleton $$(<:.o=.linked3.o) name $$(notdir $$(<:.bpf.o=)) > $$@
>> -	$(Q)$$(BPFTOOL) gen subskeleton $$(<:.o=.linked3.o) name $$(notdir $$(<:.bpf.o=)) > $$(@:.skel.h=.subskel.h)
>> -	$(Q)rm -f $$(<:.o=.linked1.o) $$(<:.o=.linked2.o) $$(<:.o=.linked3.o)
>> +	$(Q)$(if $(PERMISSIVE),if [ ! -f $$< ]; then			\
>> +		$$(RM) $$@ $$(@:.skel.h=.subskel.h);			\
>> +		printf '  %-12s %s\n' 'SKIP-SKEL' '$$(notdir $$@)' 1>&2; \
>> +		exit 0;							\
>> +	fi;)								\
>> +	printf '  %-12s %s\n' 'GEN-SKEL' '[$(TRUNNER_BINARY)] $$(notdir $$@)' 1>&2; \
>> +	$$(BPFTOOL) gen object $$(<:.o=.linked1.o) $$< &&		\
>> +	$$(BPFTOOL) gen object $$(<:.o=.linked2.o) $$(<:.o=.linked1.o) && \
>> +	$$(BPFTOOL) gen object $$(<:.o=.linked3.o) $$(<:.o=.linked2.o) && \
>> +	diff $$(<:.o=.linked2.o) $$(<:.o=.linked3.o) &&		\
>> +	$$(BPFTOOL) gen skeleton $$(<:.o=.linked3.o) name $$(notdir $$(<:.bpf.o=)) > $$@ && \
>> +	$$(BPFTOOL) gen subskeleton $$(<:.o=.linked3.o) name $$(notdir $$(<:.bpf.o=)) > $$(@:.skel.h=.subskel.h) $(if $(PERMISSIVE),|| { \
>> +		$$(RM) $$@ $$(@:.skel.h=.subskel.h); \
>> +		printf '  %-12s %s\n' 'SKIP-SKEL' '$$(notdir $$@)' 1>&2; \
>> +	}) && \
>> +	rm -f $$(<:.o=.linked1.o) $$(<:.o=.linked2.o) $$(<:.o=.linked3.o)
>
> Does the permissive mode error handler only apply to the final command?
>
> The TRUNNER_BPF_SKELS rule chains six commands together with &&:
>
>     $(BPFTOOL) gen object $(<:.o=.linked1.o) $< &&
>     $(BPFTOOL) gen object $(<:.o=.linked2.o) $(<:.o=.linked1.o) &&
>     $(BPFTOOL) gen object $(<:.o=.linked3.o) $(<:.o=.linked2.o) &&
>     diff $(<:.o=.linked2.o) $(<:.o=.linked3.o) &&
>     $(BPFTOOL) gen skeleton ... > $@ &&
>     $(BPFTOOL) gen subskeleton ... > $(@:.skel.h=.subskel.h) || { ... }
>
> If any of the first five commands fail, the && chain breaks and the
> recipe exits immediately with an error status, causing Make to abort the
> build. The error handler (|| { $(RM) ...; printf 'SKIP-SKEL' ...; })
> only catches failures from the final gen subskeleton command.

This is false. POSIX shell && and || have equal precedence and are
left-associative [1].

A failure of any command in the && chain causes the chain's exit status to
be non-zero, which the || handler then catches. Make recipes are invoked as
/bin/sh -c 'recipe' without -e, so the chain doesn't abort early.

Example:
  $ false && echo B && echo C || echo HANDLER
  HANDLER
  $ echo A && false && echo C || echo HANDLER
  A
  HANDLER
  $ echo A && echo B && false || echo HANDLER
  A
  B
  HANDLER

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_03

Thanks,

>
> This appears to contradict the commit message, which states the intent is
> to "continue with remaining tests" when generation fails. The same pattern
> affects TRUNNER_BPF_LSKELS, TRUNNER_BPF_LSKELS_SIGNED, and
> TRUNNER_BPF_SKELS_LINKED.
>
> To fully implement permissive mode, would it make sense to attach the
> error handler to the entire pipeline by wrapping the command sequence in
> a subshell?
>
>     ( cmd1 && cmd2 && ... ) || { error_handler }
>
>
> ---
> 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
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25115689001