| Message ID | PAXPR04MB9139F27885A571C2D834CAFFE3C12@PAXPR04MB9139.eurprd04.prod.outlook.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hostapd: Rename event handling functions in hostapd for | expand |
On Thu, Jun 13, 2024 at 06:32:01AM +0000, Hui Bai wrote: > On Zephyr, both wpa_supplicant and hostapd are supported. One compilation error was found due to function name conflict. > Both wpa_supplicant and hostapd has its own global event and event handlers with same name: > wpa_supplicant_event > wpa_supplicant_event_global > > To fix the compilation error, rename above functions in hostapd for Zephyr as below: > hostapd_event > hostapd_event_global This does not look like something that would really work at all in hostap.git, i.e., this is based on something that has other changes and as such, I'm not sure why this particular change should be in hostap.git. Those functions have the same name for a reason, i.e., those are the functions that are called from the driver interface code for either hostapd or wpa_supplicant. If there is need to make this work with both hostapd and wpa_supplicant somehow linked into a single binary, the driver wrappers would need changes. For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and wpa_supplicant_event_global as function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git.
Hi Jouni Malinen, Currently on Zephyr, we are using Monolithic build, which means both wpa_supplicant and hostapd files will be built into one single binary. I didn't quite get your idea of using function pointers to the driver interface. But based on my understanding, even I take your suggestions, the redefinition build error will still occurred with Monolithic build. Regards, Hui Bai -----Original Message----- From: Jouni Malinen <j@w1.fi> Sent: Saturday, August 10, 2024 3:49 PM To: Hui Bai <hui.bai@nxp.com> Cc: hostap@lists.infradead.org Subject: [EXT] Re: [PATCH] hostapd: Rename event handling functions in hostapd for Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button On Thu, Jun 13, 2024 at 06:32:01AM +0000, Hui Bai wrote: > On Zephyr, both wpa_supplicant and hostapd are supported. One compilation error was found due to function name conflict. > Both wpa_supplicant and hostapd has its own global event and event handlers with same name: > wpa_supplicant_event > wpa_supplicant_event_global > > To fix the compilation error, rename above functions in hostapd for Zephyr as below: > hostapd_event > hostapd_event_global This does not look like something that would really work at all in hostap.git, i.e., this is based on something that has other changes and as such, I'm not sure why this particular change should be in hostap.git. Those functions have the same name for a reason, i.e., those are the functions that are called from the driver interface code for either hostapd or wpa_supplicant. If there is need to make this work with both hostapd and wpa_supplicant somehow linked into a single binary, the driver wrappers would need changes. For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and wpa_supplicant_event_global as function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git. -- Jouni Malinen PGP id EFC895FA
On Mon, Aug 12, 2024 at 3:39 PM Hui Bai <hui.bai@nxp.com> wrote: > > Hi Jouni Malinen, > > Currently on Zephyr, we are using Monolithic build, which means both wpa_supplicant and hostapd files will be built into one single binary. > I didn't quite get your idea of using function pointers to the driver interface. But based on my understanding, even I take your suggestions, the redefinition build error will still occurred with Monolithic build. > > Regards, > Hui Bai > > -----Original Message----- > From: Jouni Malinen <j@w1.fi> > Sent: Saturday, August 10, 2024 3:49 PM > To: Hui Bai <hui.bai@nxp.com> > Cc: hostap@lists.infradead.org > Subject: [EXT] Re: [PATCH] hostapd: Rename event handling functions in hostapd for > > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button > > > On Thu, Jun 13, 2024 at 06:32:01AM +0000, Hui Bai wrote: > > On Zephyr, both wpa_supplicant and hostapd are supported. One compilation error was found due to function name conflict. > > Both wpa_supplicant and hostapd has its own global event and event handlers with same name: > > wpa_supplicant_event > > wpa_supplicant_event_global > > > > To fix the compilation error, rename above functions in hostapd for Zephyr as below: > > hostapd_event > > hostapd_event_global > > This does not look like something that would really work at all in hostap.git, i.e., this is based on something that has other changes and as such, I'm not sure why this particular change should be in hostap.git. > > Those functions have the same name for a reason, i.e., those are the functions that are called from the driver interface code for either hostapd or wpa_supplicant. If there is need to make this work with both hostapd and wpa_supplicant somehow linked into a single binary, the driver wrappers would need changes. For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and wpa_supplicant_event_global as function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git. > somehow linked into a single binary, This and a couple of other patches related to single binary using both hostap and wpa_supplicant are in the context of an embedded OS (ZephyrRTOS) where there are only threads and no processes. So, hostapd and WPA supplicant run in separate threads and belong to the same binary and we see conflicts in that case. > For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and > wpa_supplicant_event_global as function pointers to the driver interface and using those function > pointers instead of direct calls. That is something that I could consider applying to hostap.git. I guess what Jouni is proposing is something like below, where we can register both a global and per-interface event callback and both AP and STA interfaces can register their own callback using per-interface callbacks. diff --git a/src/drivers/driver.h b/src/drivers/driver.h index f39ea6560..64a6c169c 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -2550,6 +2550,9 @@ enum nested_attr { NESTED_ATTR_UNSPECIFIED = 2, }; + +typedef void (*wpa_event_callback)(void *ctx, enum wpa_event_type event, + union wpa_event_data *data); /** * struct wpa_driver_ops - Driver interface API definition * @@ -2937,7 +2940,7 @@ struct wpa_driver_ops { * use init2() function instead of init() to get the pointer to global * data available to per-interface initializer. */ - void * (*global_init)(void *ctx); + void * (*global_init)(void *ctx, wpa_event_callback cb); /** * global_deinit - Global driver deinitialization @@ -2959,7 +2962,8 @@ struct wpa_driver_ops { * This function can be used instead of init() if the driver wrapper * uses global data. */ - void * (*init2)(void *ctx, const char *ifname, void *global_priv); + void * (*init2)(void *ctx, const char *ifname, void *global_priv, + wpa_event_callback cb); /** * get_interfaces - Get information about available interfaces
> Those functions have the same name for a reason, i.e., those are the functions that are called from the driver interface code for either hostapd or wpa_supplicant. If there is need to make this work with both hostapd and wpa_supplicant somehow linked into a single binary, the driver wrappers would need changes. For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and wpa_supplicant_event_global as function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git. Hi Jouni, Thanks for the suggestion. RTOS platform is using wpa_supplicant and hostapd in monolithic image. Are you planning to support this case? We read your suggestion and it will be a huge change containing all the driver wrappers. As hostap maintainer, you may be more proper to add this feature's framework. And we can implement driver zephyr to support this feature. Is that okay to you? Best regards, Fengming Ye -----Original Message----- From: Krishna Chaitanya <chaitanya.mgit@gmail.com> Sent: Tuesday, August 13, 2024 5:17 PM To: Hui Bai <hui.bai@nxp.com> Cc: Jouni Malinen <j@w1.fi>; hostap@lists.infradead.org Subject: Re: [EXT] Re: [PATCH] hostapd: Rename event handling functions in hostapd for Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button On Mon, Aug 12, 2024 at 3:39 PM Hui Bai <hui.bai@nxp.com> wrote: > > Hi Jouni Malinen, > > Currently on Zephyr, we are using Monolithic build, which means both wpa_supplicant and hostapd files will be built into one single binary. > I didn't quite get your idea of using function pointers to the driver interface. But based on my understanding, even I take your suggestions, the redefinition build error will still occurred with Monolithic build. > > Regards, > Hui Bai > > -----Original Message----- > From: Jouni Malinen <j@w1.fi> > Sent: Saturday, August 10, 2024 3:49 PM > To: Hui Bai <hui.bai@nxp.com> > Cc: hostap@lists.infradead.org > Subject: [EXT] Re: [PATCH] hostapd: Rename event handling functions in > hostapd for > > Caution: This is an external email. Please take care when clicking > links or opening attachments. When in doubt, report the message using > the 'Report this email' button > > > On Thu, Jun 13, 2024 at 06:32:01AM +0000, Hui Bai wrote: > > On Zephyr, both wpa_supplicant and hostapd are supported. One compilation error was found due to function name conflict. > > Both wpa_supplicant and hostapd has its own global event and event handlers with same name: > > wpa_supplicant_event > > wpa_supplicant_event_global > > > > To fix the compilation error, rename above functions in hostapd for Zephyr as below: > > hostapd_event > > hostapd_event_global > > This does not look like something that would really work at all in hostap.git, i.e., this is based on something that has other changes and as such, I'm not sure why this particular change should be in hostap.git. > > Those functions have the same name for a reason, i.e., those are the functions that are called from the driver interface code for either hostapd or wpa_supplicant. If there is need to make this work with both hostapd and wpa_supplicant somehow linked into a single binary, the driver wrappers would need changes. For that, the cleaner way of updating the design would be by registering wpa_supplicant_event and wpa_supplicant_event_global as function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git. > somehow linked into a single binary, This and a couple of other patches related to single binary using both hostap and wpa_supplicant are in the context of an embedded OS (ZephyrRTOS) where there are only threads and no processes. So, hostapd and WPA supplicant run in separate threads and belong to the same binary and we see conflicts in that case. > For that, the cleaner way of updating the design would be by > registering wpa_supplicant_event and wpa_supplicant_event_global as > function pointers to the driver interface and using those function pointers instead of direct calls. That is something that I could consider applying to hostap.git. I guess what Jouni is proposing is something like below, where we can register both a global and per-interface event callback and both AP and STA interfaces can register their own callback using per-interface callbacks. diff --git a/src/drivers/driver.h b/src/drivers/driver.h index f39ea6560..64a6c169c 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -2550,6 +2550,9 @@ enum nested_attr { NESTED_ATTR_UNSPECIFIED = 2, }; + +typedef void (*wpa_event_callback)(void *ctx, enum wpa_event_type event, + union wpa_event_data *data); /** * struct wpa_driver_ops - Driver interface API definition * @@ -2937,7 +2940,7 @@ struct wpa_driver_ops { * use init2() function instead of init() to get the pointer to global * data available to per-interface initializer. */ - void * (*global_init)(void *ctx); + void * (*global_init)(void *ctx, wpa_event_callback cb); /** * global_deinit - Global driver deinitialization @@ -2959,7 +2962,8 @@ struct wpa_driver_ops { * This function can be used instead of init() if the driver wrapper * uses global data. */ - void * (*init2)(void *ctx, const char *ifname, void *global_priv); + void * (*init2)(void *ctx, const char *ifname, void *global_priv, + wpa_event_callback cb); /** * get_interfaces - Get information about available interfaces
diff --git a/src/ap/drv_callbacks.c b/src/ap/drv_callbacks.c index ad06e6a3f..29836cc73 100644 --- a/src/ap/drv_callbacks.c +++ b/src/ap/drv_callbacks.c @@ -2409,8 +2409,13 @@ static void hostapd_event_color_change(struct hostapd_data *hapd, bool success) #endif /* CONFIG_IEEE80211AX */ +#ifdef __ZEPHYR__ +void hostapd_event(void *ctx, enum wpa_event_type event, + union wpa_event_data *data) +#else void wpa_supplicant_event(void *ctx, enum wpa_event_type event, union wpa_event_data *data) +#endif { struct hostapd_data *hapd = ctx; struct sta_info *sta; @@ -2761,8 +2766,13 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, } +#ifdef __ZEPHYR__ +void hostapd_event_global(void *ctx, enum wpa_event_type event, + union wpa_event_data *data) +#else void wpa_supplicant_event_global(void *ctx, enum wpa_event_type event, union wpa_event_data *data) +#endif { struct hapd_interfaces *interfaces = ctx; struct hostapd_data *hapd; diff --git a/src/drivers/driver.h b/src/drivers/driver.h index 204694dd4..d86bf403a 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -6846,6 +6846,21 @@ union wpa_event_data { void wpa_supplicant_event(void *ctx, enum wpa_event_type event, union wpa_event_data *data); +#ifdef __ZEPHYR__ +/** + * hostapd_event - Report a driver event for hostapd + * @ctx: Context pointer (wpa_s); this is the ctx variable registered + * with struct wpa_driver_ops::init() + * @event: event type (defined above) + * @data: possible extra data for the event + * + * Driver wrapper code should call this function whenever an event is received + * from the driver. + */ +void hostapd_event(void *ctx, enum wpa_event_type event, + union wpa_event_data *data); +#endif + /** * wpa_supplicant_event_global - Report a driver event for wpa_supplicant * @ctx: Context pointer (wpa_s); this is the ctx variable registered @@ -6859,6 +6874,21 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event, void wpa_supplicant_event_global(void *ctx, enum wpa_event_type event, union wpa_event_data *data); +#ifdef __ZEPHYR__ +/** + * hostapd_event_global - Report a driver event for hostapd + * @ctx: Context pointer (wpa_s); this is the ctx variable registered + * with struct wpa_driver_ops::init() + * @event: event type (defined above) + * @data: possible extra data for the event + * + * Same as wpa_supplicant_event(), but we search for the interface in + * wpa_global. + */ +void hostapd_event_global(void *ctx, enum wpa_event_type event, + union wpa_event_data *data); +#endif + /* * The following inline functions are provided for convenience to simplify * event indication for some of the common events.
On Zephyr, both wpa_supplicant and hostapd are supported. One compilation error was found due to function name conflict. Both wpa_supplicant and hostapd has its own global event and event handlers with same name: wpa_supplicant_event wpa_supplicant_event_global To fix the compilation error, rename above functions in hostapd for Zephyr as below: hostapd_event hostapd_event_global Signed-off-by: Hui Bai <mailto:hui.bai@nxp.com> --- src/ap/drv_callbacks.c | 10 ++++++++++ src/drivers/driver.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+)