From: "Daniel T. Lee" <danieltimlee@gmail.com>
To: Martin KaFai Lau <kafai@fb.com>
Cc: "Daniel Borkmann" <daniel@iogearbox.net>,
"Alexei Starovoitov" <ast@kernel.org>,
"Andrii Nakryiko" <andrii@kernel.org>, brakmo <brakmo@fb.com>,
"Jesper Dangaard Brouer" <brouer@redhat.com>,
"Andrii Nakryiko" <andrii.nakryiko@gmail.com>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"David Ahern" <dsa@cumulusnetworks.com>,
"Yonghong Song" <yhs@fb.com>,
"Toke Høiland-Jørgensen" <toke@redhat.com>,
"Ira Weiny" <ira.weiny@intel.com>, "Thomas Graf" <tgraf@suug.ch>,
"Jakub Kicinski" <kuba@kernel.org>,
"John Fastabend" <john.fastabend@gmail.com>,
bpf <bpf@vger.kernel.org>, netdev <netdev@vger.kernel.org>,
Xdp <xdp-newbies@vger.kernel.org>
Subject: Re: [PATCH bpf-next 2/9] samples: bpf: refactor hbm program with libbpf
Date: Wed, 18 Nov 2020 18:31:19 +0900 [thread overview]
Message-ID: <CAEKGpzgfVfevOi4R04_0SBznHPyXWLPoh3rkXB_E9eD_JKCc+A@mail.gmail.com> (raw)
In-Reply-To: <20201118021043.zck246i2jvbboqlu@kafai-mbp.dhcp.thefacebook.com>
On Wed, Nov 18, 2020 at 11:10 AM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Tue, Nov 17, 2020 at 02:56:37PM +0000, Daniel T. Lee wrote:
> [ ... ]
>
> > +
> > +cleanup:
> > + if (rc != 0)
> so this test can be avoided.
>
Thanks for pointing me out! I will follow this approach.
> > + bpf_object__close(obj);
> > +
> > + return rc;
> > }
> >
> > [...]
> > if (!outFlag)
> > - type = BPF_CGROUP_INET_INGRESS;
> > - if (bpf_prog_attach(bpfprog_fd, cg1, type, 0)) {
> > - printf("ERROR: bpf_prog_attach fails!\n");
> > - log_err("Attaching prog");
> > + bpf_program__set_expected_attach_type(bpf_prog, BPF_CGROUP_INET_INGRESS);
> > +
> > + link = bpf_program__attach_cgroup(bpf_prog, cg1);
> There is a difference here.
> I think the bpf_prog will be detached when link is gone (e.g. process exit)
> I am not sure it is what hbm is expected considering
> cg is not clean-up on the success case.
>
I think you're right. As I did in the third patch, I will use the
link__pin approach to prevent the link from being cleaned up when the
process exit.
> > + if (libbpf_get_error(link)) {
> > + fprintf(stderr, "ERROR: bpf_program__attach_cgroup failed\n");
> > + link = NULL;
> not needed. bpf_link__destroy() can handle err ptr.
>
Thank you for the detailed advice, but in order to make it more clear
that link is no longer used, how about keeping this approach?
> > goto err;
> > }
> > [...]
> > +
> > if (cg1)
> This test looks wrong since cg1 is a fd.
>
I'll remove unnecessary fd compare.
--
Best,
Daniel T. Lee
next prev parent reply other threads:[~2020-11-18 18:31 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 14:56 [PATCH bpf-next 0/9] bpf: remove bpf_load loader completely Daniel T. Lee
2020-11-17 14:56 ` [PATCH bpf-next 1/9] selftests: bpf: move tracing helpers to trace_helper Daniel T. Lee
2020-11-18 1:19 ` Martin KaFai Lau
2020-11-18 2:44 ` Daniel T. Lee
2020-11-18 1:58 ` Andrii Nakryiko
2020-11-18 2:54 ` Daniel T. Lee
2020-11-18 3:04 ` Andrii Nakryiko
2020-11-17 14:56 ` [PATCH bpf-next 2/9] samples: bpf: refactor hbm program with libbpf Daniel T. Lee
2020-11-18 2:10 ` Martin KaFai Lau
2020-11-18 9:31 ` Daniel T. Lee [this message]
2020-11-17 14:56 ` [PATCH bpf-next 3/9] samples: bpf: refactor test_cgrp2_sock2 " Daniel T. Lee
2020-11-18 3:02 ` Andrii Nakryiko
2020-11-18 3:21 ` Daniel T. Lee
2020-11-18 5:58 ` Martin KaFai Lau
2020-11-18 9:03 ` Daniel T. Lee
2020-11-17 14:56 ` [PATCH bpf-next 4/9] samples: bpf: refactor task_fd_query " Daniel T. Lee
2020-11-18 2:58 ` Andrii Nakryiko
2020-11-18 3:19 ` Daniel T. Lee
2020-11-18 6:15 ` Martin KaFai Lau
2020-11-17 14:56 ` [PATCH bpf-next 5/9] samples: bpf: refactor ibumad " Daniel T. Lee
2020-11-18 2:52 ` Andrii Nakryiko
2020-11-18 3:05 ` Daniel T. Lee
2020-11-18 3:10 ` Andrii Nakryiko
2020-11-18 5:04 ` Daniel T. Lee
2020-11-17 14:56 ` [PATCH bpf-next 6/9] samples: bpf: refactor test_overhead " Daniel T. Lee
2020-11-18 2:45 ` Andrii Nakryiko
2020-11-17 14:56 ` [PATCH bpf-next 7/9] samples: bpf: fix lwt_len_hist reusing previous BPF map Daniel T. Lee
2020-11-17 14:56 ` [PATCH bpf-next 8/9] samples: bpf: remove unused trace_helper and bpf_load from Makefile Daniel T. Lee
2020-11-17 14:56 ` [PATCH bpf-next 9/9] samples: bpf: remove bpf_load loader completely Daniel T. Lee
2020-11-17 16:12 ` Jesper Dangaard Brouer
2020-11-18 2:48 ` Andrii Nakryiko
2020-11-18 2:57 ` Daniel T. Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAEKGpzgfVfevOi4R04_0SBznHPyXWLPoh3rkXB_E9eD_JKCc+A@mail.gmail.com \
--to=danieltimlee@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brakmo@fb.com \
--cc=brouer@redhat.com \
--cc=daniel@iogearbox.net \
--cc=dsa@cumulusnetworks.com \
--cc=ira.weiny@intel.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kuba@kernel.org \
--cc=lorenzo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=tgraf@suug.ch \
--cc=toke@redhat.com \
--cc=xdp-newbies@vger.kernel.org \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).