On Fri, Jul 30, 2021 at 7:14 AM Jakub Sitnicki jakub@cloudflare.com wrote:
On Thu, Jul 29, 2021 at 11:23 PM CEST, Jiang Wang wrote:
Previously, sockmap for AF_UNIX protocol only supports dgram type. This patch add unix stream type support, which is similar to unix_dgram_proto. To support sockmap, dgram and stream cannot share the same unix_proto anymore, because they have different implementations, such as unhash for stream type (which will remove closed or disconnected sockets from the map), so rename unix_proto to unix_dgram_proto and add a new unix_stream_proto.
Also implement stream related sockmap functions. And add dgram key words to those dgram specific functions.
Signed-off-by: Jiang Wang jiang.wang@bytedance.com Reviewed-by: Cong Wang cong.wang@bytedance.com
It seems that with commit c63829182c37 ("af_unix: Implement ->psock_update_sk_prot()") we have enabled inserting dgram, stream, and seqpacket UNIX sockets into sockmap.
After all, in ->map_update_elem we only check if sk->sk_prot->psock_update_sk_prot is set (sock_map_sk_is_suitable).
Excellent point. I should check the sock type in unix_bpf_update_proto(), and will send a fix.
Socket can be in listening, established or disconnected (TCP_CLOSE) state, that is before bind+listen/connect, or after connect(AF_UNSPEC).
For connection-oriented socket types (stream, seqpacket) there's not much you can do with disconnected sockets. I think we should limit the allowed states to listening and established for UNIX domain, as we do for TCP.
I think we should use ->unhash() to remove those connection-oriented sockets, like TCP.
AFAIU we also seem to be already allowing redirect to connected stream (and dgram, and seqpacket) UNIX sockets. sock_map_redirect_allowed() checks only if a socket is in TCP_ESTABLISHED state for anything else than TCP. Not sure what it leads to, though.
The goal is to keep all stream sockets like TCP, which only allows established ones to stay in sockmap. For dgram, any socket state is allowed to add to map but only established ones are allowed to redirect.
BTW, we do not have any intention to support Unix seqpacket socket or any seqpacket.
Thanks.