Re: [PATCH v4 bpf-next] Small BPF verifier log improvements

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

Hi Mykola,

On 2/24/22 1:37 AM, Mykola Lysenko wrote:
> In particular:
> 1) remove output of inv for scalars in print_verifier_state
> 2) replace inv with scalar in verifier error messages
> 3) remove _value suffixes for umin/umax/s32_min/etc (except map_value)
> 4) remove output of id=0
> 5) remove output of ref_obj_id=0
>
> Signed-off-by: Mykola Lysenko <mykolal@xxxxxx>

Thanks for helping to improve the verifier output. Small comment below:

[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/align.c b/tools/testing/selftests/bpf/prog_tests/align.c
> index 0ee29e11eaee..210dc6b4a169 100644
> --- a/tools/testing/selftests/bpf/prog_tests/align.c
> +++ b/tools/testing/selftests/bpf/prog_tests/align.c
> @@ -39,13 +39,13 @@ static struct bpf_align_test tests[] = {
>   		},
>   		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
>   		.matches = {
> -			{0, "R1=ctx(id=0,off=0,imm=0)"},
> +			{0, "R1=ctx(off=0,imm=0)"},
>   			{0, "R10=fp0"},
> -			{0, "R3_w=inv2"},
> -			{1, "R3_w=inv4"},
> -			{2, "R3_w=inv8"},
> -			{3, "R3_w=inv16"},
> -			{4, "R3_w=inv32"},
> +			{0, "R3_w=2"},
> +			{1, "R3_w=4"},
> +			{2, "R3_w=8"},
> +			{3, "R3_w=16"},
> +			{4, "R3_w=32"},

Ack, definitely better compared to the state today. :)

[...]
> @@ -161,19 +161,19 @@ static struct bpf_align_test tests[] = {
>   		},
>   		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
>   		.matches = {
> -			{6, "R0_w=pkt(id=0,off=8,r=8,imm=0)"},
> -			{6, "R3_w=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"},
> -			{7, "R3_w=inv(id=0,umax_value=510,var_off=(0x0; 0x1fe))"},
> -			{8, "R3_w=inv(id=0,umax_value=1020,var_off=(0x0; 0x3fc))"},
> -			{9, "R3_w=inv(id=0,umax_value=2040,var_off=(0x0; 0x7f8))"},
> -			{10, "R3_w=inv(id=0,umax_value=4080,var_off=(0x0; 0xff0))"},
> -			{12, "R3_w=pkt_end(id=0,off=0,imm=0)"},
> -			{17, "R4_w=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"},
> -			{18, "R4_w=inv(id=0,umax_value=8160,var_off=(0x0; 0x1fe0))"},
> -			{19, "R4_w=inv(id=0,umax_value=4080,var_off=(0x0; 0xff0))"},
> -			{20, "R4_w=inv(id=0,umax_value=2040,var_off=(0x0; 0x7f8))"},
> -			{21, "R4_w=inv(id=0,umax_value=1020,var_off=(0x0; 0x3fc))"},
> -			{22, "R4_w=inv(id=0,umax_value=510,var_off=(0x0; 0x1fe))"},
> +			{6, "R0_w=pkt(off=8,r=8,imm=0)"},
> +			{6, "R3_w=(umax=255,var_off=(0x0; 0xff))"},
> +			{7, "R3_w=(umax=510,var_off=(0x0; 0x1fe))"},
> +			{8, "R3_w=(umax=1020,var_off=(0x0; 0x3fc))"},
> +			{9, "R3_w=(umax=2040,var_off=(0x0; 0x7f8))"},
> +			{10, "R3_w=(umax=4080,var_off=(0x0; 0xff0))"},
> +			{12, "R3_w=pkt_end(off=0,imm=0)"},
> +			{17, "R4_w=(umax=255,var_off=(0x0; 0xff))"},
> +			{18, "R4_w=(umax=8160,var_off=(0x0; 0x1fe0))"},
> +			{19, "R4_w=(umax=4080,var_off=(0x0; 0xff0))"},
> +			{20, "R4_w=(umax=2040,var_off=(0x0; 0x7f8))"},
> +			{21, "R4_w=(umax=1020,var_off=(0x0; 0x3fc))"},
> +			{22, "R4_w=(umax=510,var_off=(0x0; 0x1fe))"},
>   		},
>   	},
>   	{

However, not printing any type info here is imho more confusing. For debugging /
troubleshooting knowing that the register type is inv or scalar would be helpful.
Fwiw, scalar is probably a better fit, although longer..

Thanks,
Daniel