Hi Ondrej, What do you think about splitting sk_setup() into different flavours? See the example patch attached. This approach may be more beneficial, because it does not count on different internal socket types, but uses direct logic of calling specific initialization function depending how the socket is obtained. Regards, Alexander On Mon, Jul 29, 2024 at 4:25 AM Ondrej Zajicek <santiago@crfreenet.org> wrote:
Hi
Thanks for the patch. I agree that inheriting the iface-bind is the only way accept() on a VRF-bound scoket would make sense. I guess this ineriting behavior is true also for other socket properties, Therefore, sk_setup() likely does plenty of useless steps for sockets associated with incoming TCP connections.
I would prefer the conditional set, you can just check (s->type == SK_TCP), (Listening socket has type SK_TCP_PASSIVE, while connecting socket has type SK_TCP_ACTIVE at sk_setup() time).
Also, you noted that this is for BIRD running as non-root with no specific capability. Do you have some experiences and suggestions about running BIRD this way? I guess there are many other syscalls in BIRD that are CAP_NET_ADMIN privileged, like setsockopt(SO_DONTROUTE).
On Sat, Jul 27, 2024 at 02:18:20PM +0200, Christian Svensson via Bird-users wrote:
Hi Alexander,
On Sat, Jul 27, 2024 at 2:05 PM Alexander Zubkov <green@qrator.net> wrote:
I wonder if it is necessary at all to set a vrf on an accepted connection? It seems to me that setting or checking vrf should be avoided instead for an accepted connection. What do you think?
Indeed, this is what I set out to do in the beginning and is, if you boil this patch down, the actual implication when using VRFs.
The reason I chose to implement the patch as a get+set rather than a conditional set was that the existing code structure assumes that sk_setup is called on multiple types of sockets and I wasn't sure exactly how to guard for specifically sockets that are connected. In addition I tried to find a reference in the kernel to where exactly it inherits the bound interface when a new socket is created from accept() but I could not. It is evident from my experiments that it is inherited, and that is the only way accept() on a VRF bind() would make sense. Doing a get+set seems like the least risky change that I felt safe to propose.
That said, if you believe it is better implemented as a conditional and are able to nudge me how you'd want a check for the particular socket type to look, I'd be happy to do a v2 patch.
Regards,
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) "To err is human -- to blame it on a computer is even more so."