netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples
@ 2020-10-26 23:36 Toke Høiland-Jørgensen
  2020-10-27  3:52 ` Andrii Nakryiko
  2020-10-27  7:14 ` Jesper Dangaard Brouer
  0 siblings, 2 replies; 4+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-10-26 23:36 UTC (permalink / raw)
  To: daniel, ast; +Cc: Toke Høiland-Jørgensen, bpf, netdev, brouer

The memlock rlimit is a notorious source of failure for BPF programs. Most
of the samples just set it to infinity, but a few used a lower limit. The
problem with unconditionally setting a lower limit is that this will also
override the limit if the system-wide setting is *higher* than the limit
being set, which can lead to failures on systems that lock a lot of memory,
but set 'ulimit -l' to unlimited before running a sample.

One fix for this is to only conditionally set the limit if the current
limit is lower, but it is simpler to just unify all the samples and have
them all set the limit to infinity.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 samples/bpf/task_fd_query_user.c    | 2 +-
 samples/bpf/tracex2_user.c          | 2 +-
 samples/bpf/tracex3_user.c          | 2 +-
 samples/bpf/xdp_redirect_cpu_user.c | 2 +-
 samples/bpf/xdp_rxq_info_user.c     | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/task_fd_query_user.c b/samples/bpf/task_fd_query_user.c
index 4a74531dc403..b68bd2f8fdc9 100644
--- a/samples/bpf/task_fd_query_user.c
+++ b/samples/bpf/task_fd_query_user.c
@@ -290,7 +290,7 @@ static int test_debug_fs_uprobe(char *binary_path, long offset, bool is_return)
 
 int main(int argc, char **argv)
 {
-	struct rlimit r = {1024*1024, RLIM_INFINITY};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	extern char __executable_start;
 	char filename[256], buf[256];
 	__u64 uprobe_file_offset;
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
index 3e36b3e4e3ef..3d6eab711d23 100644
--- a/samples/bpf/tracex2_user.c
+++ b/samples/bpf/tracex2_user.c
@@ -116,7 +116,7 @@ static void int_exit(int sig)
 
 int main(int ac, char **argv)
 {
-	struct rlimit r = {1024*1024, RLIM_INFINITY};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	long key, next_key, value;
 	struct bpf_link *links[2];
 	struct bpf_program *prog;
diff --git a/samples/bpf/tracex3_user.c b/samples/bpf/tracex3_user.c
index 70e987775c15..83e0fecbb01a 100644
--- a/samples/bpf/tracex3_user.c
+++ b/samples/bpf/tracex3_user.c
@@ -107,7 +107,7 @@ static void print_hist(int fd)
 
 int main(int ac, char **argv)
 {
-	struct rlimit r = {1024*1024, RLIM_INFINITY};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_link *links[2];
 	struct bpf_program *prog;
 	struct bpf_object *obj;
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 6fb8dbde62c5..f78cb18319aa 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -765,7 +765,7 @@ static int load_cpumap_prog(char *file_name, char *prog_name,
 
 int main(int argc, char **argv)
 {
-	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	char *prog_name = "xdp_cpu_map5_lb_hash_ip_pairs";
 	char *mprog_filename = "xdp_redirect_kern.o";
 	char *redir_interface = NULL, *redir_map = NULL;
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index caa4e7ffcfc7..93fa1bc54f13 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -450,7 +450,7 @@ static void stats_poll(int interval, int action, __u32 cfg_opt)
 int main(int argc, char **argv)
 {
 	__u32 cfg_options= NO_TOUCH ; /* Default: Don't touch packet memory */
-	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples
  2020-10-26 23:36 [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples Toke Høiland-Jørgensen
@ 2020-10-27  3:52 ` Andrii Nakryiko
  2020-10-27  7:14 ` Jesper Dangaard Brouer
  1 sibling, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2020-10-27  3:52 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Networking,
	Jesper Dangaard Brouer

On Mon, Oct 26, 2020 at 5:10 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> The memlock rlimit is a notorious source of failure for BPF programs. Most
> of the samples just set it to infinity, but a few used a lower limit. The
> problem with unconditionally setting a lower limit is that this will also
> override the limit if the system-wide setting is *higher* than the limit
> being set, which can lead to failures on systems that lock a lot of memory,
> but set 'ulimit -l' to unlimited before running a sample.
>
> One fix for this is to only conditionally set the limit if the current
> limit is lower, but it is simpler to just unify all the samples and have
> them all set the limit to infinity.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  samples/bpf/task_fd_query_user.c    | 2 +-
>  samples/bpf/tracex2_user.c          | 2 +-
>  samples/bpf/tracex3_user.c          | 2 +-
>  samples/bpf/xdp_redirect_cpu_user.c | 2 +-
>  samples/bpf/xdp_rxq_info_user.c     | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/samples/bpf/task_fd_query_user.c b/samples/bpf/task_fd_query_user.c
> index 4a74531dc403..b68bd2f8fdc9 100644
> --- a/samples/bpf/task_fd_query_user.c
> +++ b/samples/bpf/task_fd_query_user.c
> @@ -290,7 +290,7 @@ static int test_debug_fs_uprobe(char *binary_path, long offset, bool is_return)
>
>  int main(int argc, char **argv)
>  {
> -       struct rlimit r = {1024*1024, RLIM_INFINITY};
> +       struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>         extern char __executable_start;
>         char filename[256], buf[256];
>         __u64 uprobe_file_offset;
> diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
> index 3e36b3e4e3ef..3d6eab711d23 100644
> --- a/samples/bpf/tracex2_user.c
> +++ b/samples/bpf/tracex2_user.c
> @@ -116,7 +116,7 @@ static void int_exit(int sig)
>
>  int main(int ac, char **argv)
>  {
> -       struct rlimit r = {1024*1024, RLIM_INFINITY};
> +       struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>         long key, next_key, value;
>         struct bpf_link *links[2];
>         struct bpf_program *prog;
> diff --git a/samples/bpf/tracex3_user.c b/samples/bpf/tracex3_user.c
> index 70e987775c15..83e0fecbb01a 100644
> --- a/samples/bpf/tracex3_user.c
> +++ b/samples/bpf/tracex3_user.c
> @@ -107,7 +107,7 @@ static void print_hist(int fd)
>
>  int main(int ac, char **argv)
>  {
> -       struct rlimit r = {1024*1024, RLIM_INFINITY};
> +       struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>         struct bpf_link *links[2];
>         struct bpf_program *prog;
>         struct bpf_object *obj;
> diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> index 6fb8dbde62c5..f78cb18319aa 100644
> --- a/samples/bpf/xdp_redirect_cpu_user.c
> +++ b/samples/bpf/xdp_redirect_cpu_user.c
> @@ -765,7 +765,7 @@ static int load_cpumap_prog(char *file_name, char *prog_name,
>
>  int main(int argc, char **argv)
>  {
> -       struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
> +       struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>         char *prog_name = "xdp_cpu_map5_lb_hash_ip_pairs";
>         char *mprog_filename = "xdp_redirect_kern.o";
>         char *redir_interface = NULL, *redir_map = NULL;
> diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> index caa4e7ffcfc7..93fa1bc54f13 100644
> --- a/samples/bpf/xdp_rxq_info_user.c
> +++ b/samples/bpf/xdp_rxq_info_user.c
> @@ -450,7 +450,7 @@ static void stats_poll(int interval, int action, __u32 cfg_opt)
>  int main(int argc, char **argv)
>  {
>         __u32 cfg_options= NO_TOUCH ; /* Default: Don't touch packet memory */
> -       struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
> +       struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>         struct bpf_prog_load_attr prog_load_attr = {
>                 .prog_type      = BPF_PROG_TYPE_XDP,
>         };
> --
> 2.29.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples
  2020-10-26 23:36 [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples Toke Høiland-Jørgensen
  2020-10-27  3:52 ` Andrii Nakryiko
@ 2020-10-27  7:14 ` Jesper Dangaard Brouer
  2020-10-27 17:00   ` Roman Gushchin
  1 sibling, 1 reply; 4+ messages in thread
From: Jesper Dangaard Brouer @ 2020-10-27  7:14 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: daniel, ast, bpf, netdev, brouer, Roman Gushchin, kernel-team

On Tue, 27 Oct 2020 00:36:23 +0100
Toke Høiland-Jørgensen <toke@redhat.com> wrote:

> The memlock rlimit is a notorious source of failure for BPF programs. Most
> of the samples just set it to infinity, but a few used a lower limit. The
> problem with unconditionally setting a lower limit is that this will also
> override the limit if the system-wide setting is *higher* than the limit
> being set, which can lead to failures on systems that lock a lot of memory,
> but set 'ulimit -l' to unlimited before running a sample.
> 
> One fix for this is to only conditionally set the limit if the current
> limit is lower, but it is simpler to just unify all the samples and have
> them all set the limit to infinity.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

This change basically disable the memlock rlimit system. And this
disable method is becoming standard in more and more BPF programs.
IMHO using the system-wide memlock rlimit doesn't make sense for BPF.

I'm still ACKing the patch, as this seems the only way forward, to
ignore and in-practice not use the memlock rlimit.

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>


I saw some patches on the list (from Facebook) with a new system for
policy limiting memory usage per BPF program or was it mem-cgroup, but
I don't think that was ever merged... I would really like to see
something replace (and remove) this memlock rlimit dependency. Anyone
knows what happened to that effort?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples
  2020-10-27  7:14 ` Jesper Dangaard Brouer
@ 2020-10-27 17:00   ` Roman Gushchin
  0 siblings, 0 replies; 4+ messages in thread
From: Roman Gushchin @ 2020-10-27 17:00 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Toke Høiland-Jørgensen, daniel, ast, bpf, netdev, kernel-team

On Tue, Oct 27, 2020 at 08:14:40AM +0100, Jesper Dangaard Brouer wrote:
> On Tue, 27 Oct 2020 00:36:23 +0100
> Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> 
> > The memlock rlimit is a notorious source of failure for BPF programs. Most
> > of the samples just set it to infinity, but a few used a lower limit. The
> > problem with unconditionally setting a lower limit is that this will also
> > override the limit if the system-wide setting is *higher* than the limit
> > being set, which can lead to failures on systems that lock a lot of memory,
> > but set 'ulimit -l' to unlimited before running a sample.
> > 
> > One fix for this is to only conditionally set the limit if the current
> > limit is lower, but it is simpler to just unify all the samples and have
> > them all set the limit to infinity.
> > 
> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> 
> This change basically disable the memlock rlimit system. And this
> disable method is becoming standard in more and more BPF programs.
> IMHO using the system-wide memlock rlimit doesn't make sense for BPF.

Hi Jesper,

+1

> 
> I'm still ACKing the patch, as this seems the only way forward, to
> ignore and in-practice not use the memlock rlimit.
> 
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 
> 
> I saw some patches on the list (from Facebook) with a new system for
> policy limiting memory usage per BPF program or was it mem-cgroup, but
> I don't think that was ever merged... I would really like to see
> something replace (and remove) this memlock rlimit dependency. Anyone
> knows what happened to that effort?

I'm working on it.

It required some heavy changes on the mm side: accounting of the percpu memory,
which required a framework for accounting of arbitrary non page-sized objects,
support of accounting from an interrupt context and some manipulations with
page flags in order to allow accounted vmallocs to be mapped to userspace.

It's mostly done with the last part expected to reach linux-next in few days.
Then I'll rebase and repost the bpf part.

Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-10-27 17:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 23:36 [PATCH bpf] samples/bpf: Set rlimit for memlock to infinity in all samples Toke Høiland-Jørgensen
2020-10-27  3:52 ` Andrii Nakryiko
2020-10-27  7:14 ` Jesper Dangaard Brouer
2020-10-27 17:00   ` Roman Gushchin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).