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.
