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 <[email protected]> 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/df22b3140cad6bd8742dce16e6a1b342d4a83f6d > > (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: [email protected]) > "To err is human -- to blame it on a computer is even more so."
