PATCH: Error message if set memlock=infinite failed during bpf load

Executing ip vrf exec <vrfname> command sometimes fails with:

bpf: Failed to load program: Operation not permitted

This error message might be misleading because the underlying reason can be
that memlock limit is to small.

It is already implemented to set memlock to infinite, but without
error handling.

With this patch at least a warning is printed out to inform the user
what might be the root cause.


Signed-off-by: Stefan Majer <stefan.majer@gmail.com>

  fprintf(stderr, "Continuing without mounted eBPF fs. Too old kernel?\n");

On 4/1/20 12:57 AM, Stefan Majer wrote:
> Executing ip vrf exec <vrfname> command sometimes fails with:
> 
> bpf: Failed to load program: Operation not permitted
> 
> This error message might be misleading because the underlying reason can be
> that memlock limit is to small.
> 
> It is already implemented to set memlock to infinite, but without
> error handling.
> 
> With this patch at least a warning is printed out to inform the user
> what might be the root cause.
> 
> 
> Signed-off-by: Stefan Majer <stefan.majer@gmail.com>
> 
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 10cf9bf4..210830d9 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1416,8 +1416,8 @@ static void bpf_init_env(void)
>   .rlim_max = RLIM_INFINITY,
>   };
> 
> - /* Don't bother in case we fail! */
> - setrlimit(RLIMIT_MEMLOCK, &limit);
> + if (!setrlimit(RLIMIT_MEMLOCK, &limit))
> + fprintf(stderr, "Continue without setting ulimit memlock=infinity.
> Error:%s\n", strerror(errno));
> 
>   if (!bpf_get_work_dir(BPF_PROG_TYPE_UNSPEC))
>   fprintf(stderr, "Continuing without mounted eBPF fs. Too old kernel?\n");
> 

bpf_init_env is not called for 'ip vrf exec'.

Since other bpf code raises the limit it would be consistent for 'ip vrf
exec' to do the same. I know this limit has been a pain for some users.

Hi David,

i thought is was my poor C knowledge that i was unable to get the
point where bpf_init_env is called from ip vrf, but thanks.

So should we also do:

diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index b9a43675..16d19621 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -256,6 +256,8 @@ static int prog_load(int idx)
                BPF_EXIT_INSN(),
        };

+       bpf_init_env();
+
        return bpf_prog_load(BPF_PROG_TYPE_CGROUP_SOCK, prog, sizeof(prog),
                             "GPL", bpf_log_buf, sizeof(bpf_log_buf));
 }
diff --git a/lib/bpf.c b/lib/bpf.c
index 10cf9bf4..210830d9 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1416,8 +1416,8 @@ static void bpf_init_env(void)
                .rlim_max = RLIM_INFINITY,
        };

-       /* Don't bother in case we fail! */
-       setrlimit(RLIMIT_MEMLOCK, &limit);
+       if (!setrlimit(RLIMIT_MEMLOCK, &limit))
+               fprintf(stderr, "Continue without setting ulimit
memlock=infinity. Error:%s\n", strerror(errno));

        if (!bpf_get_work_dir(BPF_PROG_TYPE_UNSPEC))
                fprintf(stderr, "Continuing without mounted eBPF fs.
Too old kernel?\n");

Greetings
Stefan

On Wed, Apr 1, 2020 at 9:57 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 4/1/20 12:57 AM, Stefan Majer wrote:
> > Executing ip vrf exec <vrfname> command sometimes fails with:
> >
> > bpf: Failed to load program: Operation not permitted
> >
> > This error message might be misleading because the underlying reason can be
> > that memlock limit is to small.
> >
> > It is already implemented to set memlock to infinite, but without
> > error handling.
> >
> > With this patch at least a warning is printed out to inform the user
> > what might be the root cause.
> >
> >
> > Signed-off-by: Stefan Majer <stefan.majer@gmail.com>
> >
> > diff --git a/lib/bpf.c b/lib/bpf.c
> > index 10cf9bf4..210830d9 100644
> > --- a/lib/bpf.c
> > +++ b/lib/bpf.c
> > @@ -1416,8 +1416,8 @@ static void bpf_init_env(void)
> >   .rlim_max = RLIM_INFINITY,
> >   };
> >
> > - /* Don't bother in case we fail! */
> > - setrlimit(RLIMIT_MEMLOCK, &limit);
> > + if (!setrlimit(RLIMIT_MEMLOCK, &limit))
> > + fprintf(stderr, "Continue without setting ulimit memlock=infinity.
> > Error:%s\n", strerror(errno));
> >
> >   if (!bpf_get_work_dir(BPF_PROG_TYPE_UNSPEC))
> >   fprintf(stderr, "Continuing without mounted eBPF fs. Too old kernel?\n");
> >
>
> bpf_init_env is not called for 'ip vrf exec'.
>
> Since other bpf code raises the limit it would be consistent for 'ip vrf
> exec' to do the same. I know this limit has been a pain for some users.