Changeset View
Standalone View
sys/net/vnet.c
Show First 20 Lines • Show All 227 Lines • ▼ Show 20 Lines | |||||
* 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__); | ||||
vnet = malloc(sizeof(struct vnet), M_VNET, M_WAITOK | M_ZERO); | vnet = malloc(sizeof(struct vnet), M_VNET, M_WAITOK | M_ZERO); | ||||
bz: No matter that the outcome of the naming/state discussion will be, can this please be moved… | |||||
vnet->vnet_magic_n = VNET_MAGIC_N; | vnet->vnet_magic_n = VNET_MAGIC_N; | ||||
vnet->vnet_state = 0; | vnet->vnet_state = 0; | ||||
SDT_PROBE2(vnet, functions, vnet_alloc, alloc, __LINE__, vnet); | SDT_PROBE2(vnet, functions, vnet_alloc, alloc, __LINE__, vnet); | ||||
/* | /* | ||||
* Allocate storage for virtualized global variables and copy in | * Allocate storage for virtualized global variables and copy in | ||||
* initial values form our 'master' copy. | * initial values form our 'master' copy. | ||||
*/ | */ | ||||
▲ Show 20 Lines • Show All 80 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->vnet_state = SI_SUB_VNET_DONE; | |||||
bzUnsubmitted 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… | |||||
hselaskyAuthorUnsubmitted 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… | |||||
hselaskyAuthorUnsubmitted 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. | |||||
bzUnsubmitted 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… | |||||
hselaskyAuthorUnsubmitted 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… | |||||
hselaskyAuthorUnsubmitted 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… | |||||
curvnet = NULL; | curvnet = NULL; | ||||
} | } | ||||
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. | ||||
*/ | */ | ||||
Show All 14 Lines | |||||
static void | static void | ||||
vnet_sysinit_done(void *unused __unused) | 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); | ||||
/* | |||||
* Check if VNET is running. | |||||
*/ | |||||
bzUnsubmitted Not Done Inline ActionsCould be a single-line comment. bz: Could be a single-line comment. | |||||
bool | |||||
vnet_is_running(struct vnet *vnet) | |||||
bzUnsubmitted 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". | |||||
{ | |||||
KASSERT(vnet->vnet_state <= SI_SUB_VNET_DONE, | |||||
("Invalid VNET state 0x%08x > 0x%08x", | |||||
vnet->vnet_state, SI_SUB_VNET_DONE)); | |||||
return (vnet->vnet_state == SI_SUB_VNET_DONE); | |||||
bzUnsubmitted 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… | |||||
} | |||||
/* | /* | ||||
* 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 | ||||
* should be used only by the kernel linker. | * should be used only by the kernel linker. | ||||
*/ | */ | ||||
void * | void * | ||||
vnet_data_alloc(int size) | vnet_data_alloc(int size) | ||||
▲ Show 20 Lines • Show All 435 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()?