| Message ID | 149372831091.22268.12527267276516941574.stgit@firesoul |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, May 02, 2017 at 02:31:50PM +0200, Jesper Dangaard Brouer wrote: > Needed to adjust max locked memory RLIMIT_MEMLOCK for testing these bpf samples > as these are using more and larger maps than can fit in distro default 64Kbytes limit. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> ... > + struct rlimit r = {1024*1024, RLIM_INFINITY}; ... > + struct rlimit r = {1024*1024, RLIM_INFINITY}; why magic numbers? All other samples do struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; > + if (setrlimit(RLIMIT_MEMLOCK, &r)) { > + perror("setrlimit(RLIMIT_MEMLOCK)"); ip_tunnel.c test does: perror("setrlimit(RLIMIT_MEMLOCK, RLIM_INFINITY)"); Few others do: assert(!setrlimit(RLIMIT_MEMLOCK, &r)); and the rest just: setrlimit(RLIMIT_MEMLOCK, &r); We probalby need to move this to a helper. > + struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; here it's consistent :) > + if (setrlimit(RLIMIT_MEMLOCK, &r)) { > + perror("setrlimit(RLIMIT_MEMLOCK, RLIM_INFINITY)"); but with different perror ? Let's do a common helper for all?
On Tue, 2 May 2017 17:53:16 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Tue, May 02, 2017 at 02:31:50PM +0200, Jesper Dangaard Brouer wrote: > > Needed to adjust max locked memory RLIMIT_MEMLOCK for testing these bpf samples > > as these are using more and larger maps than can fit in distro default 64Kbytes limit. > > > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > ... > > + struct rlimit r = {1024*1024, RLIM_INFINITY}; > ... > > + struct rlimit r = {1024*1024, RLIM_INFINITY}; > > why magic numbers? > All other samples do > struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; I just wanted to provide some examples showing that it is possible to set some reasonable limit. The RLIM_INFINITY setting is basically just disabling the kernels memory limit checks, and it is sort of a bad coding pattern (that people will copy) as the two example programs does not need much. > > + if (setrlimit(RLIMIT_MEMLOCK, &r)) { > > + perror("setrlimit(RLIMIT_MEMLOCK)"); > > ip_tunnel.c test does: > perror("setrlimit(RLIMIT_MEMLOCK, RLIM_INFINITY)"); > Few others do: > assert(!setrlimit(RLIMIT_MEMLOCK, &r)); > and the rest just: > setrlimit(RLIMIT_MEMLOCK, &r); > > We probalby need to move this to a helper. > > > + struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; > > here it's consistent :) > > > + if (setrlimit(RLIMIT_MEMLOCK, &r)) { > > + perror("setrlimit(RLIMIT_MEMLOCK, RLIM_INFINITY)"); > > but with different perror ? > Let's do a common helper for all? Sure, it makes sense to streamline this into a helper, just not in this patchset ;-) Lets do that later... And I would argue that this helper should allow users to specify some expected/reasonable memory usage size, as the kernel side checks would then provide some value, instead of being effectively disabled. I can easily imagine someone increasing a _kern.c hash map max size to 100 million, without realizing that this can OOM the machine.
From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Tue, 2 May 2017 17:53:16 -0700 > On Tue, May 02, 2017 at 02:31:50PM +0200, Jesper Dangaard Brouer wrote: >> Needed to adjust max locked memory RLIMIT_MEMLOCK for testing these bpf samples >> as these are using more and larger maps than can fit in distro default 64Kbytes limit. >> >> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > ... >> + struct rlimit r = {1024*1024, RLIM_INFINITY}; > ... >> + struct rlimit r = {1024*1024, RLIM_INFINITY}; > > why magic numbers? > All other samples do > struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; Let's not do that. People run these tests often as root, so the safer we make running these test the better. A weird magic limit is better than none at all.
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c index ded9804c5034..7fee0f1ba9a3 100644 --- a/samples/bpf/tracex2_user.c +++ b/samples/bpf/tracex2_user.c @@ -4,6 +4,7 @@ #include <signal.h> #include <linux/bpf.h> #include <string.h> +#include <sys/resource.h> #include "libbpf.h" #include "bpf_load.h" @@ -112,6 +113,7 @@ static void int_exit(int sig) int main(int ac, char **argv) { + struct rlimit r = {1024*1024, RLIM_INFINITY}; char filename[256]; long key, next_key, value; FILE *f; @@ -119,6 +121,11 @@ int main(int ac, char **argv) snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); + if (setrlimit(RLIMIT_MEMLOCK, &r)) { + perror("setrlimit(RLIMIT_MEMLOCK)"); + return 1; + } + signal(SIGINT, int_exit); /* start 'ping' in the background to have some kfree_skb events */ diff --git a/samples/bpf/tracex3_user.c b/samples/bpf/tracex3_user.c index 8f7d199d5945..fe372239d505 100644 --- a/samples/bpf/tracex3_user.c +++ b/samples/bpf/tracex3_user.c @@ -11,6 +11,7 @@ #include <stdbool.h> #include <string.h> #include <linux/bpf.h> +#include <sys/resource.h> #include "libbpf.h" #include "bpf_load.h" @@ -112,11 +113,17 @@ static void print_hist(int fd) int main(int ac, char **argv) { + struct rlimit r = {1024*1024, RLIM_INFINITY}; char filename[256]; int i; snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); + if (setrlimit(RLIMIT_MEMLOCK, &r)) { + perror("setrlimit(RLIMIT_MEMLOCK)"); + return 1; + } + if (load_bpf_file(filename)) { printf("%s", bpf_log_buf); return 1; diff --git a/samples/bpf/tracex4_user.c b/samples/bpf/tracex4_user.c index 03449f773cb1..22c644f1f4c3 100644 --- a/samples/bpf/tracex4_user.c +++ b/samples/bpf/tracex4_user.c @@ -12,6 +12,8 @@ #include <string.h> #include <time.h> #include <linux/bpf.h> +#include <sys/resource.h> + #include "libbpf.h" #include "bpf_load.h" @@ -50,11 +52,17 @@ static void print_old_objects(int fd) int main(int ac, char **argv) { + struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY}; char filename[256]; int i; snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); + if (setrlimit(RLIMIT_MEMLOCK, &r)) { + perror("setrlimit(RLIMIT_MEMLOCK, RLIM_INFINITY)"); + return 1; + } + if (load_bpf_file(filename)) { printf("%s", bpf_log_buf); return 1;
Needed to adjust max locked memory RLIMIT_MEMLOCK for testing these bpf samples as these are using more and larger maps than can fit in distro default 64Kbytes limit. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- samples/bpf/tracex2_user.c | 7 +++++++ samples/bpf/tracex3_user.c | 7 +++++++ samples/bpf/tracex4_user.c | 8 ++++++++ 3 files changed, 22 insertions(+)