PATCH: IO: Avoid calling SO_BINDTODEVICE if not needed

[Ondrej Zajicek]

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 at 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 at crfreenet.org)
"To err is human -- to blame it on a computer is even more so."