From: Stanislav Fomichev <sdf@fomichev.me>
To: Martin Lau <kafai@fb.com>
Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
David Miller <davem@davemloft.net>,
Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH bpf] bpf: net: Set sk_bpf_storage back to NULL for cloned sk
Date: Tue, 16 Jul 2019 10:32:44 -0700 [thread overview]
Message-ID: <20190716173244.GA14834@mini-arch> (raw)
In-Reply-To: <20190716054624.ea6sbbzn62grde2n@kafai-mbp>
On 07/16, Martin Lau wrote:
> On Tue, Jul 09, 2019 at 09:33:21AM -0700, Stanislav Fomichev wrote:
> > On 06/11, Martin KaFai Lau wrote:
> > > The cloned sk should not carry its parent-listener's sk_bpf_storage.
> > > This patch fixes it by setting it back to NULL.
> > Have you thought about some kind of inheritance for listener sockets'
> > storage? Suppose I have a situation where I write something
> > to listener's sk storage (directly or via recently added sockopts hooks)
> > and I want to inherit that state for a freshly established connection.
> >
> > I was looking into adding possibility to call bpf_get_listener_sock form
> > BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB callback to manually
> > copy some data form the listener socket, but I don't think
> > at this point there is any association between newly established
> > socket and the listener.
> Right, at that point, the child sk has no reference back
> to the listener's sk.
>
> After a quick look, the listener sk may not always be available
> also (e.g. the backlog processing case). Hence, adding
> the listener sk to the bpf running ctx is not obvious
> either.
>
> >
> > Thoughts/ideas?
> I think cloning the listener's bpf sk storage could be added
> to the existing sk cloning logic. It seems to be a more straight
> forward approach instead of figuring out the right place to call
> another bpf prog to clone it.
>
> Quick thoughts out of my head:
> 1. Default should be not-to-clone. Have a way (a map's flag?) to opt-in.
> 2. The listener's sk storage could be being modified while being cloned.
> One possibility is to check if the value has bpf_spin_lock.
> If there is, lock it before cloning.
Thanks for suggestion! An optional inherit/clone flag to
bpf_sk_storage_get seems like a good option. I'll try to play with it,
will probably get back with an rfc at some point.
prev parent reply other threads:[~2019-07-16 17:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-11 21:45 [PATCH bpf] bpf: net: Set sk_bpf_storage back to NULL for cloned sk Martin KaFai Lau
2019-06-12 4:46 ` Andrii Nakryiko
2019-06-12 14:48 ` Daniel Borkmann
2019-07-09 16:33 ` Stanislav Fomichev
2019-07-16 5:46 ` Martin Lau
2019-07-16 17:32 ` Stanislav Fomichev [this message]
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=20190716173244.GA14834@mini-arch \
--to=sdf@fomichev.me \
--cc=Kernel-team@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=kafai@fb.com \
--cc=netdev@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.