Re: git: a730d82378d3 - main - tcp: fix RACK and BBR when using VIMAGE enabled kernel
- In reply to: Marko Zec : "Re: git: a730d82378d3 - main - tcp: fix RACK and BBR when using VIMAGE enabled kernel"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 23 Jul 2021 05:49:45 UTC
> On 20 Jul 2021, at 18:22, Marko Zec <zec@fer.hr> wrote: > > On Tue, 20 Jul 2021 10:29:30 +0930 > Daniel O'Connor via freebsd-hackers <freebsd-hackers@freebsd.org> wrote: >> >> Not to pick on this particular commit, but does anyone know why >> CURVNET_RESTORE os not defined such that it doesn't require wrapping >> with {}? > > True, the body of CURVNET_RESTORE() macro could be redefined so that it > gets enclosed in a do {...} while (0) block, most probably without any > ill side-effects. > > One reason it was never defined in such manner is that CURVNET_ macros > were never intended to be invoked conditionally, which is also clearly > documented in VNET(9) . The other reason is my clumsiness... OK understood. I didn't even think to look for a man page, sorry! One nit pick about the man page is that 'man 9 VNET' doesn't work, you need 'man 9 vnet'. >> Looking through vnet.h I see that VNET_ASSERT, >> VNET_GLOBAL_EVENTHANDLER_REGISTER_TAG, and >> VNET_GLOBAL_EVENTHANDLER_REGISTER have a do { } while(0) wrapper but >> CURVNET_SET_QUIET, CURVNET_SET_VERBOSE, CURVNET_RESTORE, >> VNET_SYSINIT, and VNET_SYSUNINIT don't. > > CURVNET_SET macros cannot be wrapped in a separate block because they > declare a local variable which must be visible / available to the > corresponding CURVNET_RESTORE(s). Ahh right, obviously I didn't read the code very deeply :) > In this particular case, i.e. inside ctf_process_inbound_raw(), the > rule that CURVNET_SET() must not be conditionally called seems to be > circumvented by a local hack, in an attempt to handle a case when > VNET cannot be inferred from m->m_pkthdr->rcvif->if_vnet, when m == > NULL. > > Given that a non-NULL struct socket *so seem to be available as an > argument to ctf_process_inbound_raw(), perhaps it could be possible to > remove the if (no_vn == 0) hack, and instead to unconditionally invoke > CURVNET_SET(so->so_vnet) on entry to ctf_process_inbound_raw()? Thanks for the detailed explanation, much appreciated. -- Daniel O'Connor "The nice thing about standards is that there are so many of them to choose from." -- Andrew Tanenbaum