Hi, Yes, you are right, it makes another unneeded level of indirection. Because sk_setup() is called from only 2 paces now - sk_open() and sk_passive_connected(). Those are already specific initialization functions. And there is no reason to add additional layer, when VRF code could be moved directly to sk_open(). But the fix is there already. And in some sence the bird's socket type is a non-explicit argument to sk_setup(), that idetifies how socket is obtained. I only have doubts if SK_UNIX should be there too, because it is also obtained by accept(). But those are fs-bound, and I'm not sure that VRFs work for them at all somehow. On Tue, Jul 30, 2024 at 5:03 PM Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Tue, Jul 30, 2024 at 12:33:30AM +0200, Alexander Zubkov wrote:
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.
Hi
That is a good point, but in some way sk_setup() is already the common function, while the specific code (like bind() to address and associated calls) are directly in sk_open(). Adding another layer of indirection would complicate it even more and would force reordering of syscalls if some of these are moved from generic to specific.
I thought about just adding an argument to sk_setup() based on how the socked is obtained, so it can do the conditional operations internally, but finally i chose the minimal change:
https://gitlab.nic.cz/labs/bird/-/commit/df22b3140cad6bd8742dce16e6a1b342d4a...
(I do not like how this code is structured anyways, especially split of syscalls between sk_open() and sk_setup(), so perhaps this will be restructured later in some more sensible manner.)
-- 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."