BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Stanislav Fomichev <sdf@google.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [RPC PATCH bpf-next] bpf: implement new BPF_CGROUP_INET_SOCK_POST_CONNECT
Date: Fri, 15 Jan 2021 09:41:06 -0800	[thread overview]
Message-ID: <f3661770-65dc-05f2-a580-199580eb0b74@fb.com> (raw)
In-Reply-To: <CAKH8qBuZ0iLAiuqi=65RBiQ=Vhi3qkitPzj0b7U=XuiH_4TuLA@mail.gmail.com>



On 1/15/21 8:39 AM, Stanislav Fomichev wrote:
> On Thu, Jan 14, 2021 at 8:27 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 1/14/21 7:59 PM, Alexei Starovoitov wrote:
>>> On Thu, Jan 14, 2021 at 7:51 PM Stanislav Fomichev <sdf@google.com> wrote:
>>>>>>
>>>>>>           lock_sock(sock->sk);
>>>>>>           err = __inet_stream_connect(sock, uaddr, addr_len, flags, 0);
>>>>>
>>>>> Similarly here, attaching fexit to __inet_stream_connect would execute
>>>>> your BPF program at exactly the same time (and then you can check for
>>>>> err value).
>>>>>
>>>>> Or the point here is to have a more "stable" BPF program type?
>>>> Good suggestion, I can try to play with it, I think it should give me
>>>> all the info I need (I only need sock).
>>>> But yeah, I'd rather prefer a stable interface against stable
>>>> __sk_buff, but maybe fexit will also work.
>>>
>>> Maybe we can add an extension to fentry/fexit that are cgroup scoped?
>>> I think this will solve many such cases.
>>
>> Currently, google is pushing LTO build of the kernel. If this happens,
>> it is possible one global function in one file (say a.c) might be
>> inlined into another file (say b.c). So in this particular case,
>> if the global function is inlined, fentry/fexit approach might be
>> missing some cases? We could mark certain *special purpose* function
>> as non-inline, but not sure whether this is scalable or not.
> For this particular case I don't think it matters, right?
> I'd like to fexit ip4_datagram_connect which is exported symbol,
> it's accessed via proto->connect and there is no way it's
> gonna be inlined. Unless our indirect call macros give clang
> a hint :-/

You are right. It is called through indirect call and by default
compiler won't be able to do inlining. One possibility is profile
guided optimization which often profiles indirect call as well.
They may find the indirect call has one call happening in say
80% and will special case that one and may do inlining. not sure.
I guess kernel build in general is not that advanced. But just
keep in mind that this could happen in distant future.

> 
> I'm in general a bit concerned about using tracing calls for stuff
> like that and depending on the non-uapi, but it's probably
> time to give it a try and see how co-re works :-)

you can filter out based on cgroup id in bpf program. I guess
fexit should work in your use case.

  reply	other threads:[~2021-01-15 17:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15  0:17 [RPC PATCH bpf-next] bpf: implement new BPF_CGROUP_INET_SOCK_POST_CONNECT Stanislav Fomichev
2021-01-15  2:38 ` Andrii Nakryiko
2021-01-15  3:50   ` Stanislav Fomichev
2021-01-15  3:59     ` Alexei Starovoitov
2021-01-15  4:27       ` Yonghong Song
2021-01-15 16:39         ` Stanislav Fomichev
2021-01-15 17:41           ` Yonghong Song [this message]
2021-01-15  6:55 ` kernel test robot
2021-01-15 13:38 ` kernel test robot

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=f3661770-65dc-05f2-a580-199580eb0b74@fb.com \
    --to=yhs@fb.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.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