From 5f6575852f4eac742665cd20e55d28782820e8c9 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Tue, 26 Nov 2024 17:48:11 +0000 Subject: [PATCH 1/2] fix global namespace disable when we change a sysctl external to bpftune, the aim is to disable the auto-tuning for the associated namespace. however in the global ns we were disabling tuning for global and non-global namespaces via bpftuner_fini(). Ensure we disable global namespace only, and that we only send events for tuners that are enabled and - if namespaced - if the namespace is enabled also. Reported by: Roger Knobbe (https://github.com/rknobbe) Signed-off-by: Alan Maguire --- src/Makefile | 2 +- src/libbpftune.c | 53 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/Makefile b/src/Makefile index e655613..ca27b14 100644 --- a/src/Makefile +++ b/src/Makefile @@ -166,7 +166,7 @@ $(TUNER_LIBS): $(OPATH)libbpftune.so $(TUNER_OBJS) $(CC) $(CFLAGS) -shared -o $(@) $(patsubst $(OPATH)%.so,%.c,$(@)) \ $(LDLIBS) -lbpftune $(LDFLAGS) -$(OPATH)libbpftune.o: probe.skel.h probe.skel.legacy.h probe.skel.nobtf.h +$(OPATH)libbpftune.o: probe.skel.h probe.skel.legacy.h probe.skel.nobtf.h libbpftune.c $(QUIET_CC)$(CC) $(CFLAGS) -c libbpftune.c -o $@ $(OPATH)bpftune.o: $(OPATH)libbpftune.so diff --git a/src/libbpftune.c b/src/libbpftune.c index f75981b..ec28ee9 100644 --- a/src/libbpftune.c +++ b/src/libbpftune.c @@ -646,6 +646,21 @@ void bpftuner_bpf_fini(struct bpftuner *tuner) static struct bpftuner *bpftune_tuners[BPFTUNE_MAX_TUNERS]; +static unsigned long global_netns_cookie; + +static void bpftune_global_netns_init(void) +{ + unsigned long cookie = 0; + + if (global_netns_cookie || !netns_cookie_supported) + return; + if (!bpftune_netns_info(getpid(), NULL, &cookie)) { + global_netns_cookie = cookie; + bpftune_log(LOG_DEBUG, "global netns cookie is %ld\n", + global_netns_cookie); + } +} + /* add a tuner to the list of tuners, or replace existing inactive tuner. * If successful, call init(). */ @@ -703,6 +718,12 @@ struct bpftuner *bpftuner_init(const char *path) free(tuner); return NULL; } + if (!global_netns_cookie) + bpftune_global_netns_init(); + if (global_netns_cookie) { + tuner->netns.netns_cookie = global_netns_cookie; + tuner->netns.state = BPFTUNE_ACTIVE; + } tuner->id = bpftune_num_tuners; tuner->state = BPFTUNE_ACTIVE; bpftune_tuners[bpftune_num_tuners++] = tuner; @@ -711,8 +732,6 @@ struct bpftuner *bpftuner_init(const char *path) return tuner; } -static unsigned long global_netns_cookie; - static void bpftuner_scenario_log(struct bpftuner *tuner, unsigned int tunable, unsigned int scenario, int netns_fd, bool summary, @@ -761,6 +780,7 @@ void bpftune_set_learning_rate(unsigned short rate) static int bpftune_ringbuf_event_read(void *ctx, void *data, size_t size) { + const char *status = "skipped due to inactive tuner/netns"; struct bpftune_event *event = data; struct bpftuner *tuner; @@ -777,16 +797,22 @@ static int bpftune_ringbuf_event_read(void *ctx, void *data, size_t size) bpftune_log(LOG_ERR, "no tuner for id %d\n", event->tuner_id); return 0; } + /* only send events to active tuners/netns */ + if (tuner->state == BPFTUNE_ACTIVE) { + struct bpftuner_netns *netns = bpftuner_netns_from_cookie(event->tuner_id, event->netns_cookie); + + if (!netns || netns->state != BPFTUNE_MANUAL) { + tuner->event_handler(tuner, event, ctx); + status = "sent"; + } + } bpftune_log(LOG_DEBUG, - "event scenario [%d] for tuner %s[%d] netns %ld (%s)\n", + "event scenario [%d] for tuner %s[%d] netns %lu (%s) %s\n", event->scenario_id, tuner->name, tuner->id, event->netns_cookie, event->netns_cookie && event->netns_cookie != global_netns_cookie ? - "non-global netns" : "global netns"); - /* only send events to active tuners */ - if (tuner->state == BPFTUNE_ACTIVE) - tuner->event_handler(tuner, event, ctx); - + "non-global netns" : "global netns", + status); return 0; } @@ -1330,7 +1356,7 @@ int bpftune_netns_info(int pid, int *fd, unsigned long *cookie) static int bpftune_netns_find(unsigned long cookie) { - unsigned long netns_cookie; + unsigned long netns_cookie = 0; struct bpftuner *t; struct mntent *ent; FILE *mounts; @@ -1446,11 +1472,8 @@ int bpftune_netns_init_all(void) if (!netns_cookie_supported) return 0; - if (!bpftune_netns_info(getpid(), NULL, &cookie)) { - global_netns_cookie = cookie; - bpftune_log(LOG_DEBUG, "global netns cookie is %ld\n", - global_netns_cookie); - } + bpftune_global_netns_init(); + return bpftune_netns_find(0); } @@ -1480,7 +1503,7 @@ void bpftuner_netns_fini(struct bpftuner *tuner, unsigned long cookie, enum bpft { struct bpftuner_netns *netns, *prev = NULL; - if (cookie == 0 || cookie == global_netns_cookie) { + if (cookie == 0 || (cookie == global_netns_cookie && !netns_cookie_supported)) { bpftuner_fini(tuner, state); return; } From b893c8b3730b1e913496c609a8fc7128323e5ca2 Mon Sep 17 00:00:00 2001 From: Alan Maguire Date: Tue, 26 Nov 2024 20:54:34 +0000 Subject: [PATCH 2/2] sysctl_tuner: show command associated with process that sets sysctl this will help diagnose the cause of sysctl changes that disable auto-tuning. Signed-off-by: Alan Maguire --- src/sysctl_tuner.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/sysctl_tuner.c b/src/sysctl_tuner.c index fd9cffd..3d64d71 100644 --- a/src/sysctl_tuner.c +++ b/src/sysctl_tuner.c @@ -52,6 +52,22 @@ void fini(struct bpftuner *tuner) bpftuner_bpf_fini(tuner); } +static const char *pid2cmd(int pid, char *cmd, size_t cmdsize) +{ + char cmdline[64]; + FILE *fp; + + snprintf(cmdline, sizeof(cmdline) - 1, "/proc/%d/cmdline", pid); + fp = fopen(cmdline, "r"); + if (fp) { + fgets(cmd, cmdsize - 1, fp); + fclose(fp); + } + if (strlen(cmd) == 0) + strncpy(cmd, "?", 2); + return cmd; +} + void event_handler(struct bpftuner *tuner, struct bpftune_event *event, __attribute__((unused))void *ctx) { @@ -80,9 +96,13 @@ void event_handler(struct bpftuner *tuner, struct bpftune_event *event, * gc_thresh3 in neigh table tuner for example. */ if (strstr(path, event->str)) { + char cmd[1024] = {}; + bpftune_log(BPFTUNE_LOG_LEVEL, - "user (pid %ld) modified sysctl '%s' that tuner '%s' uses; disabling '%s' for namespace cookie %ld\n", - event->pid, event->str, t->name, t->name, + "pid %ld, cmd '%s' modified sysctl '%s' that tuner '%s' uses; disabling '%s' for namespace cookie %ld\n", + event->pid, + pid2cmd(event->pid, cmd, sizeof(cmd)), + event->str, t->name, t->name, event->netns_cookie); bpftuner_netns_fini(t, event->netns_cookie, BPFTUNE_MANUAL); break;