PATCH: IO: Avoid calling SO_BINDTODEVICE if not needed

[Alexander Zubkov]

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 at 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 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."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bird-sk_setup.patch
Type: text/x-patch
Size: 1976 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20240730/84d7528b/attachment.bin>