Changeset View
Standalone View
sys/dev/wtap/wtap_hal/hal.c
Show First 20 Lines • Show All 67 Lines • ▼ Show 20 Lines | init_hal(struct wtap_hal *hal) | ||||
mtx_init(&hal->hal_mtx, "wtap_hal mtx", NULL, MTX_DEF | MTX_RECURSE); | mtx_init(&hal->hal_mtx, "wtap_hal mtx", NULL, MTX_DEF | MTX_RECURSE); | ||||
hal->hal_md = (struct wtap_medium *)malloc(sizeof(struct wtap_medium), | hal->hal_md = (struct wtap_medium *)malloc(sizeof(struct wtap_medium), | ||||
M_WTAP, M_NOWAIT | M_ZERO); | M_WTAP, M_NOWAIT | M_ZERO); | ||||
init_medium(hal->hal_md); | init_medium(hal->hal_md); | ||||
/* register event handler for packets */ | /* register event handler for packets */ | ||||
TASK_INIT(&hal->hal_md->tx_handler->proc, 0, hal_tx_proc, hal); | TASK_INIT(&hal->hal_md->tx_handler->proc, 0, hal_tx_proc, hal); | ||||
callout_init_mtx(&hal->hw.timer_intr, &hal->hal_mtx, 0); | |||||
lwhsu: Does it need to acquire giant lock? If no, we can change mpsafe to 1, or, maybe we can use… | |||||
Not Done Inline ActionsOK. We don't need a giant lock, so I will change mpsafe to 1, and use callout_init_mtx(9) to avoid race condition. enweiwu: OK. We don't need a giant lock, so I will change `mpsafe` to 1, and use `callout_init_mtx(9)`… | |||||
Not Done Inline Actions
Please note that there is no mpsafe parameter in callout_init_mtx(9) so you can only chose one to do. lwhsu: > OK. We don't need a giant lock, so I will change `mpsafe` to 1, and use `callout_init_mtx(9)`… | |||||
Done Inline ActionsOops, the "and" above is actually "or". enweiwu: Oops, the "and" above is actually "or". | |||||
hal->hw.timer_intr_intval = msecs_to_ticks(HAL_TIMER_INTVAL); | |||||
Not Done Inline Actionsif 50 is from spec, please make it as a definition in hal.c lwhsu: if 50 is from spec, please make it as a definition in hal.c | |||||
} | } | ||||
void | void | ||||
register_plugin(struct wtap_hal *hal, struct wtap_plugin *plugin) | register_plugin(struct wtap_hal *hal, struct wtap_plugin *plugin) | ||||
{ | { | ||||
plugin->init(plugin); | plugin->init(plugin); | ||||
hal->plugin = plugin; | hal->plugin = plugin; | ||||
Show All 16 Lines | deinit_hal(struct wtap_hal *hal) | ||||
free(hal->hal_md, M_WTAP); | free(hal->hal_md, M_WTAP); | ||||
mtx_destroy(&hal->hal_mtx); | mtx_destroy(&hal->hal_mtx); | ||||
} | } | ||||
int32_t | int32_t | ||||
new_wtap(struct wtap_hal *hal, int32_t id) | new_wtap(struct wtap_hal *hal, int32_t id) | ||||
{ | { | ||||
static const uint8_t mac_pool[64][IEEE80211_ADDR_LEN] = { | static const uint8_t mac_pool[64][IEEE80211_ADDR_LEN] = { | ||||
{0,152,154,152,150,151}, | {0,152,154,152,150,151}, | ||||
Not Done Inline ActionsUnrelated; but we should probably fix this "pool"? I don't think this is a special prefix? bz: Unrelated; but we should probably fix this "pool"? I don't think this is a special prefix? | |||||
Not Done Inline Actionsyeah ew, let's get this fixed in a subsequent commit? adrian: yeah ew, let's get this fixed in a subsequent commit? | |||||
Not Done Inline ActionsSounds good, btw, any suggestions about how to fix this? lwhsu: Sounds good, btw, any suggestions about how to fix this? | |||||
Not Done Inline ActionsHm, we should look at what ath(4) does for inspiration. :-) It will create a primary VAP based on the hardware (and in our case we can use the random generator.) For the secondary NICs it does it during VAP clone - it sets the "I'm a local mac address!" bit and then will increment the low byte by one so the hardware mask that's used is kept small (ie it doesn't have to be COMPLETELY promisc, for "anything matching this mask I'll ACK in hardware" reasons.) For this we can likely get away with supporting either random addresses per interface clone / VAP creation, and ensure we can set it manually when we create the interface. That way automated tests can be written with well known MAC addresses. adrian: Hm, we should look at what ath(4) does for inspiration. :-)
It will create a primary VAP based… | |||||
Not Done Inline ActionsAnd when using random, use the FreeBSD prefix we also use for Ethernet. There's a nice function to get one. If we try to be clever we could do FreeBSD prefix + some wtap unit/vap number thing for the last bits; that way things would be very deterministic to start from for testing as well? bz: And when using random, use the FreeBSD prefix we also use for Ethernet. There's a nice… | |||||
{0,152,154,152,150,152}, | {0,152,154,152,150,152}, | ||||
{0,152,154,152,150,153}, | {0,152,154,152,150,153}, | ||||
{0,152,154,152,150,154}, | {0,152,154,152,150,154}, | ||||
{0,152,154,152,150,155}, | {0,152,154,152,150,155}, | ||||
{0,152,154,152,150,156}, | {0,152,154,152,150,156}, | ||||
{0,152,154,152,150,157}, | {0,152,154,152,150,157}, | ||||
{0,152,154,152,150,158}, | {0,152,154,152,150,158}, | ||||
{0,152,154,152,151,151}, | {0,152,154,152,151,151}, | ||||
▲ Show 20 Lines • Show All 56 Lines • ▼ Show 20 Lines | new_wtap(struct wtap_hal *hal, int32_t id) | ||||
DWTAP_PRINTF("%s\n", __func__); | DWTAP_PRINTF("%s\n", __func__); | ||||
uint8_t const *macaddr = mac_pool[id]; | uint8_t const *macaddr = mac_pool[id]; | ||||
if(hal->hal_devs[id] != NULL){ | if(hal->hal_devs[id] != NULL){ | ||||
printf("error, wtap_id=%d already created\n", id); | printf("error, wtap_id=%d already created\n", id); | ||||
return -1; | return -1; | ||||
} | } | ||||
hal->hal_devs[id] = (struct wtap_softc *)malloc( | hal->hal_devs[id] = (struct wtap_softc *)malloc( | ||||
Not Done Inline Actionshm, malloc with M_NOWAIT can return NULL, right? Should we check for that? adrian: hm, malloc with M_NOWAIT can return NULL, right? Should we check for that? | |||||
Done Inline ActionsActually, I think we might use M_WAITOK rather than M_NOWAIT, because it's not in the interrupt context. Shall I change this flag in the next diff? enweiwu: Actually, I think we might use M_WAITOK rather than M_NOWAIT, because it's not in the interrupt… | |||||
Not Done Inline ActionsYes pleas do. bz: Yes pleas do. | |||||
sizeof(struct wtap_softc), M_WTAP, M_NOWAIT | M_ZERO); | sizeof(struct wtap_softc), M_WTAP, M_NOWAIT | M_ZERO); | ||||
hal->hal_devs[id]->sc_md = hal->hal_md; | hal->hal_devs[id]->sc_md = hal->hal_md; | ||||
hal->hal_devs[id]->id = id; | hal->hal_devs[id]->id = id; | ||||
hal->hal_devs[id]->hal = hal; | |||||
snprintf(hal->hal_devs[id]->name, sizeof(hal->hal_devs[id]->name), | snprintf(hal->hal_devs[id]->name, sizeof(hal->hal_devs[id]->name), | ||||
"wtap%d", id); | "wtap%d", id); | ||||
mtx_init(&hal->hal_devs[id]->sc_mtx, "wtap_softc mtx", NULL, | mtx_init(&hal->hal_devs[id]->sc_mtx, "wtap_softc mtx", NULL, | ||||
MTX_DEF | MTX_RECURSE); | MTX_DEF | MTX_RECURSE); | ||||
if(wtap_attach(hal->hal_devs[id], macaddr)){ | if(wtap_attach(hal->hal_devs[id], macaddr)){ | ||||
printf("%s, cant alloc new wtap\n", __func__); | printf("%s, cant alloc new wtap\n", __func__); | ||||
return -1; | return -1; | ||||
Show All 13 Lines | free_wtap(struct wtap_hal *hal, int32_t id) | ||||
} | } | ||||
if(wtap_detach(hal->hal_devs[id])) | if(wtap_detach(hal->hal_devs[id])) | ||||
printf("%s, cant alloc new wtap\n", __func__); | printf("%s, cant alloc new wtap\n", __func__); | ||||
mtx_destroy(&hal->hal_devs[id]->sc_mtx); | mtx_destroy(&hal->hal_devs[id]->sc_mtx); | ||||
free(hal->hal_devs[id], M_WTAP); | free(hal->hal_devs[id], M_WTAP); | ||||
hal->hal_devs[id] = NULL; | hal->hal_devs[id] = NULL; | ||||
return 0; | return 0; | ||||
} | |||||
void | |||||
Not Done Inline ActionsPlease follow the style of function definition as others in this file, and style(9). lwhsu: Please follow the style of function definition as others in this file, and style(9). | |||||
Done Inline ActionsSure. enweiwu: Sure. | |||||
wtap_hal_timer_intr(void *arg) | |||||
{ | |||||
struct wtap_hal *hal = arg; | |||||
uint32_t intval = hal->hw.timer_intr_intval; | |||||
hal->hw.tsf += ticks_to_msecs(intval); | |||||
callout_schedule(&hal->hw.timer_intr, intval); | |||||
} | |||||
void | |||||
wtap_hal_reset_tsf(struct wtap_hal *hal) | |||||
{ | |||||
mtx_lock(&hal->hal_mtx); | |||||
callout_stop(&hal->hw.timer_intr); | |||||
hal->hw.tsf = 0; | |||||
callout_reset(&hal->hw.timer_intr, hal->hw.timer_intr_intval, | |||||
wtap_hal_timer_intr, hal); | |||||
mtx_unlock(&hal->hal_mtx); | |||||
} | |||||
uint64_t | |||||
wtap_hal_get_tsf(struct wtap_hal *hal) | |||||
{ | |||||
return (hal->hw.tsf); | |||||
} | } |
Does it need to acquire giant lock? If no, we can change mpsafe to 1, or, maybe we can use callout_init_mtx(9) and &hal->mtx (which doesn't seem to be use in other place in wtap(4).)