diff mbox series

[net-next,v2,2/2] selftests/bpf: activate the OP_NE login in range_cond()

Message ID 20231212131031.3088661-3-menglong8.dong@gmail.com
State New
Headers
Series bpf: support to trace BPF_JNE |

Commit Message

Menglong Dong Dec. 12, 2023, 1:10 p.m. UTC 
  The edge range checking for the registers is supported by the verifier
now, so we can activate the extended login in
tools/testing/selftests/bpf/prog_tests/reg_bounds.c/range_cond() to test
such logic.

Signed-off-by: Menglong Dong <menglong8.dong@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/reg_bounds.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Eduard Zingerman Dec. 12, 2023, 11:37 p.m. UTC | #1 
On Tue, 2023-12-12 at 21:10 +0800, Menglong Dong wrote:
> The edge range checking for the registers is supported by the verifier
> now, so we can activate the extended login in
> tools/testing/selftests/bpf/prog_tests/reg_bounds.c/range_cond() to test
> such logic.
> 
> Signed-off-by: Menglong Dong <menglong8.dong@gmail.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/reg_bounds.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> index 0c9abd279e18..49d8d4bafe99 100644
> --- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> +++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> @@ -590,12 +590,7 @@ static void range_cond(enum num_t t, struct range x, struct range y,
>  		*newy = range(t, max_t(t, x.a, y.a), min_t(t, x.b, y.b));
>  		break;
>  	case OP_NE:
> -		/* generic case, can't derive more information */
> -		*newx = range(t, x.a, x.b);
> -		*newy = range(t, y.a, y.b);
> -		break;
> -
> -		/* below extended logic is not supported by verifier just yet */
> +		/* below logic is supported by the verifier now */
>  		if (x.a == x.b && x.a == y.a) {
>  			/* X is a constant matching left side of Y */
>  			*newx = range(t, x.a, x.b);

I think that some crafted tests have to be added.
Note that reg_bounds only runs a subset of tests during CI
(controlled by variable SLOW_TESTS).
By default only randomized and crafted tests are run.
It appears to me that probability of randomly generating specific
ranges explored by this series is quite low.
Menglong Dong Dec. 13, 2023, 2:35 a.m. UTC | #2 
Hello,

On Wed, Dec 13, 2023 at 7:37 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2023-12-12 at 21:10 +0800, Menglong Dong wrote:
> > The edge range checking for the registers is supported by the verifier
> > now, so we can activate the extended login in
> > tools/testing/selftests/bpf/prog_tests/reg_bounds.c/range_cond() to test
> > such logic.
> >
> > Signed-off-by: Menglong Dong <menglong8.dong@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/reg_bounds.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > index 0c9abd279e18..49d8d4bafe99 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
> > @@ -590,12 +590,7 @@ static void range_cond(enum num_t t, struct range x, struct range y,
> >               *newy = range(t, max_t(t, x.a, y.a), min_t(t, x.b, y.b));
> >               break;
> >       case OP_NE:
> > -             /* generic case, can't derive more information */
> > -             *newx = range(t, x.a, x.b);
> > -             *newy = range(t, y.a, y.b);
> > -             break;
> > -
> > -             /* below extended logic is not supported by verifier just yet */
> > +             /* below logic is supported by the verifier now */
> >               if (x.a == x.b && x.a == y.a) {
> >                       /* X is a constant matching left side of Y */
> >                       *newx = range(t, x.a, x.b);
>
> I think that some crafted tests have to be added.
> Note that reg_bounds only runs a subset of tests during CI
> (controlled by variable SLOW_TESTS).
> By default only randomized and crafted tests are run.
> It appears to me that probability of randomly generating specific
> ranges explored by this series is quite low.

You are right, I'll add some cases to the "crafted_cases" for
this logic.

Thanks!
Menglong Dong
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
index 0c9abd279e18..49d8d4bafe99 100644
--- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
+++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c
@@ -590,12 +590,7 @@  static void range_cond(enum num_t t, struct range x, struct range y,
 		*newy = range(t, max_t(t, x.a, y.a), min_t(t, x.b, y.b));
 		break;
 	case OP_NE:
-		/* generic case, can't derive more information */
-		*newx = range(t, x.a, x.b);
-		*newy = range(t, y.a, y.b);
-		break;
-
-		/* below extended logic is not supported by verifier just yet */
+		/* below logic is supported by the verifier now */
 		if (x.a == x.b && x.a == y.a) {
 			/* X is a constant matching left side of Y */
 			*newx = range(t, x.a, x.b);