On Tue, Nov 17, 2020 at 02:56:36PM +0000, Daniel T. Lee wrote:
> Under the samples/bpf directory, similar tracing helpers are
> fragmented around. To keep consistent of tracing programs, this commit
> moves the helper and define locations to increase the reuse of each
> helper function.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@xxxxxxxxx>
>
> ---
> samples/bpf/Makefile | 2 +-
> samples/bpf/hbm.c | 51 ++++-----------------
> tools/testing/selftests/bpf/trace_helpers.c | 33 ++++++++++++-
> tools/testing/selftests/bpf/trace_helpers.h | 3 ++
> 4 files changed, 45 insertions(+), 44 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index aeebf5d12f32..3e83cd5ca1c2 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -110,7 +110,7 @@ xdp_fwd-objs := xdp_fwd_user.o
> task_fd_query-objs := bpf_load.o task_fd_query_user.o $(TRACE_HELPERS)
> xdp_sample_pkts-objs := xdp_sample_pkts_user.o $(TRACE_HELPERS)
> ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS)
> -hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
> +hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS) $(TRACE_HELPERS)
>
> # Tell kbuild to always build the programs
> always-y := $(tprogs-y)
> diff --git a/samples/bpf/hbm.c b/samples/bpf/hbm.c
> index 400e741a56eb..b9f9f771dd81 100644
> --- a/samples/bpf/hbm.c
> +++ b/samples/bpf/hbm.c
> @@ -48,6 +48,7 @@
>
> #include "bpf_load.h"
> #include "bpf_rlimit.h"
> +#include "trace_helpers.h"
> #include "cgroup_helpers.h"
> #include "hbm.h"
> #include "bpf_util.h"
> @@ -65,51 +66,12 @@ bool no_cn_flag;
> bool edt_flag;
>
> static void Usage(void);
> -static void read_trace_pipe2(void);
> static void do_error(char *msg, bool errno_flag);
>
> -#define DEBUGFS "/sys/kernel/debug/tracing/"
> -
> struct bpf_object *obj;
> int bpfprog_fd;
> int cgroup_storage_fd;
>
> -static void read_trace_pipe2(void)
> -{
> - int trace_fd;
> - FILE *outf;
> - char *outFname = "hbm_out.log";
> -
> - trace_fd = open(DEBUGFS "trace_pipe", O_RDONLY, 0);
> - if (trace_fd < 0) {
> - printf("Error opening trace_pipe\n");
> - return;
> - }
> -
> -// Future support of ingress
> -// if (!outFlag)
> -// outFname = "hbm_in.log";
> - outf = fopen(outFname, "w");
> -
> - if (outf == NULL)
> - printf("Error creating %s\n", outFname);
> -
> - while (1) {
> - static char buf[4097];
> - ssize_t sz;
> -
> - sz = read(trace_fd, buf, sizeof(buf) - 1);
> - if (sz > 0) {
> - buf[sz] = 0;
> - puts(buf);
> - if (outf != NULL) {
> - fprintf(outf, "%s\n", buf);
> - fflush(outf);
> - }
> - }
> - }
> -}
> -
> static void do_error(char *msg, bool errno_flag)
> {
> if (errno_flag)
> @@ -392,8 +354,15 @@ static int run_bpf_prog(char *prog, int cg_id)
> fclose(fout);
> }
>
> - if (debugFlag)
> - read_trace_pipe2();
> + if (debugFlag) {
> + char *out_fname = "hbm_out.log";
> + /* Future support of ingress */
> + // if (!outFlag)
> + // out_fname = "hbm_in.log";
> +
> + read_trace_pipe2(out_fname);
> + }
> +
> return rc;
> err:
> rc = 1;
> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index 1bbd1d9830c8..b7c184e109e8 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c
> @@ -11,8 +11,6 @@
> #include <sys/mman.h>
> #include "trace_helpers.h"
>
> -#define DEBUGFS "/sys/kernel/debug/tracing/"
Is this change needed?
> -
> #define MAX_SYMS 300000
> static struct ksym syms[MAX_SYMS];
> static int sym_cnt;
> @@ -136,3 +134,34 @@ void read_trace_pipe(void)
> }
> }
> }
> +
> +void read_trace_pipe2(char *filename)
> +{
> + int trace_fd;
> + FILE *outf;
> +
> + trace_fd = open(DEBUGFS "trace_pipe", O_RDONLY, 0);
> + if (trace_fd < 0) {
> + printf("Error opening trace_pipe\n");
> + return;
> + }
> +
> + outf = fopen(filename, "w");
> + if (!outf)
> + printf("Error creating %s\n", filename);
> +
> + while (1) {
> + static char buf[4096];
> + ssize_t sz;
> +
> + sz = read(trace_fd, buf, sizeof(buf) - 1);
> + if (sz > 0) {
> + buf[sz] = 0;
> + puts(buf);
> + if (outf) {
> + fprintf(outf, "%s\n", buf);
> + fflush(outf);
> + }
> + }
> + }
It needs a fclose().
IIUC, this function will never return. I am not sure
this is something that should be made available to selftests.
