Changeset View
Standalone View
sys/net/vnet.c
Show First 20 Lines • Show All 219 Lines • ▼ Show 20 Lines | |||||
SDT_PROBE_DEFINE1(vnet, functions, vnet_destroy, return, | SDT_PROBE_DEFINE1(vnet, functions, vnet_destroy, return, | ||||
"int"); | "int"); | ||||
#ifdef DDB | #ifdef DDB | ||||
static void db_show_vnet_print_vs(struct vnet_sysinit *, int); | static void db_show_vnet_print_vs(struct vnet_sysinit *, int); | ||||
#endif | #endif | ||||
/* | /* | ||||
* Check if VNET is not in startup nor shutdown phase. | |||||
*/ | |||||
bool | |||||
vnet_is_ready(struct vnet *vnet) | |||||
{ | |||||
return (vnet->vnet_state <= SI_SUB_VNET || | |||||
vnet->vnet_state >= SI_SUB_VNET_DONE); | |||||
} | |||||
bz: No matter that the outcome of the naming/state discussion will be, can this please be moved… | |||||
/* | |||||
* Allocate a virtual network stack. | * Allocate a virtual network stack. | ||||
*/ | */ | ||||
struct vnet * | struct vnet * | ||||
vnet_alloc(void) | vnet_alloc(void) | ||||
{ | { | ||||
struct vnet *vnet; | struct vnet *vnet; | ||||
SDT_PROBE1(vnet, functions, vnet_alloc, entry, __LINE__); | SDT_PROBE1(vnet, functions, vnet_alloc, entry, __LINE__); | ||||
▲ Show 20 Lines • Show All 89 Lines • ▼ Show 20 Lines | vnet0_init(void *arg __unused) | ||||
curvnet = prison0.pr_vnet = vnet0 = vnet_alloc(); | curvnet = prison0.pr_vnet = vnet0 = vnet_alloc(); | ||||
} | } | ||||
SYSINIT(vnet0_init, SI_SUB_VNET, SI_ORDER_FIRST, vnet0_init, NULL); | SYSINIT(vnet0_init, SI_SUB_VNET, SI_ORDER_FIRST, vnet0_init, NULL); | ||||
static void | static void | ||||
vnet_init_done(void *unused __unused) | vnet_init_done(void *unused __unused) | ||||
{ | { | ||||
curvnet = NULL; | curvnet = NULL; | ||||
Done Inline ActionsI seem to remember you had a NULL panic somewhere a long time ago; however this should not be needed as vnet0_init() calls vnet_alloc() for the default network stack and that in turn calls vnet_sysinit() which will update the vnet_state for every VNET_SYSINIT and with that vnet_sysinit_done should set the state to SI_SUB_VNET_DONE. Can you remind me how you ended up with a curvnet/vnet0 without state set? bz: I seem to remember you had a NULL panic somewhere a long time ago; however this should not be… | |||||
Done Inline ActionsThe NULL panic was because I wan't checking the VNET state in the other patch (D19622), which eventually leaded up to this patch. After inspecting the code I believe the chunk above can be removed. You are right. hselasky: The NULL panic was because I wan't checking the VNET state in the other patch (D19622), which… | |||||
Done Inline ActionsAfter an actual check, it appears the sysinit for setting the done, has not been registered yet. So the vnet_state is actually 0, for the curvnet at this point :-) hselasky: After an actual check, it appears the sysinit for setting the done, has not been registered yet. | |||||
Done Inline ActionsThat's eaxtly the problem I am trying to describe. So you are using something in the frag code (the other change) which panics because you are no up yet? That's an ordering issue then if I understand that correctly. Please don't fix the symtom, we need to fix the problem or over the next months a lot of similar things will show up again... bz: That's eaxtly the problem I am trying to describe. So you are using something in the frag code… | |||||
Done Inline ActionsHi, In the old code both SI_SUB_VNET_DONE and 0 would be treated as running state. So this bug has been hidden for a while! shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET && ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0; The problem is like this: vnet_state is only updated inside vnet_sysinit() and vnet_sysuninit(). But for vnet0, there are no VNET SYSINIT's registered when vnet_sysinit() is called, so the state never gets set. However when vnet_register_sysinit() is called and run the VNET's done sysinit is executed, but it does not update the vnet_state :-( Is my approach to this OK? Get rid of vnet_state and set the running variable from the done callback function itself. Add a similar unload callback function aswell. hselasky: Hi,
In the old code both SI_SUB_VNET_DONE and 0 would be treated as running state. So this bug… | |||||
Done Inline ActionsThe panic happened when I assumed that all VNET's have reached SI_SUB_VNET_DONE. This manifested itself when destroying vnets, with the other patch applied. hselasky: The panic happened when I assumed that all VNET's have reached SI_SUB_VNET_DONE. This… | |||||
} | } | ||||
SYSINIT(vnet_init_done, SI_SUB_VNET_DONE, SI_ORDER_ANY, vnet_init_done, | SYSINIT(vnet_init_done, SI_SUB_VNET_DONE, SI_ORDER_ANY, vnet_init_done, | ||||
NULL); | NULL); | ||||
/* | /* | ||||
* Once on boot, initialize the modspace freelist to entirely cover modspace. | * Once on boot, initialize the modspace freelist to entirely cover modspace. | ||||
*/ | */ | ||||
static void | static void | ||||
Show All 16 Lines | vnet_sysinit_done(void *unused __unused) | ||||
return; | return; | ||||
} | } | ||||
VNET_SYSINIT(vnet_sysinit_done, SI_SUB_VNET_DONE, SI_ORDER_ANY, | VNET_SYSINIT(vnet_sysinit_done, SI_SUB_VNET_DONE, SI_ORDER_ANY, | ||||
vnet_sysinit_done, NULL); | vnet_sysinit_done, NULL); | ||||
/* | /* | ||||
* When a module is loaded and requires storage for a virtualized global | * When a module is loaded and requires storage for a virtualized global | ||||
* variable, allocate space from the modspace free list. This interface | * variable, allocate space from the modspace free list. This interface | ||||
Not Done Inline ActionsCould be a single-line comment. bz: Could be a single-line comment. | |||||
* should be used only by the kernel linker. | * should be used only by the kernel linker. | ||||
*/ | */ | ||||
Not Done Inline ActionsI am still considerung if calling it "vnet_is_up()" would be more clear than "running". bz: I am still considerung if calling it "vnet_is_up()" would be more clear than "running". | |||||
void * | void * | ||||
vnet_data_alloc(int size) | vnet_data_alloc(int size) | ||||
{ | { | ||||
struct vnet_data_free *df; | struct vnet_data_free *df; | ||||
void *s; | void *s; | ||||
Not Done Inline ActionsDuring startup and shutdown of a vnet the vnet is not on the vnet list anymore. However it does exist ouside these boundries. Could you, (a) for documentation purposes, and (b) to make sure no bogus code calls into this (or rather the calling functions) add the vnet_state > SI_SUB_VNET to the KASSERT as well again? Getting VNETs "stable" was hard and tightening checks wherever possible helps to keep it that way. bz: During startup and shutdown of a vnet the vnet is not on the vnet list anymore. However it… | |||||
s = NULL; | s = NULL; | ||||
size = roundup2(size, sizeof(void *)); | size = roundup2(size, sizeof(void *)); | ||||
sx_xlock(&vnet_data_free_lock); | sx_xlock(&vnet_data_free_lock); | ||||
TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { | TAILQ_FOREACH(df, &vnet_data_free_head, vnd_link) { | ||||
if (df->vnd_len < size) | if (df->vnd_len < size) | ||||
continue; | continue; | ||||
if (df->vnd_len == size) { | if (df->vnd_len == size) { | ||||
s = (void *)df->vnd_start; | s = (void *)df->vnd_start; | ||||
▲ Show 20 Lines • Show All 423 Lines • Show Last 20 Lines |
No matter that the outcome of the naming/state discussion will be, can this please be moved under vnet_sysinit_done() before the comment for vnet_data_alloc()?