On Mon, Nov 26, 2018 at 03:04:19PM +0530, Sarada Prasanna Garnayak wrote:
> The nl80211_get_aid is used to receive a new AID from the kernel
> driver, which may in turn ask it from the FW, and that from the HW.
> This AID is tied to SID and will need to be freed eventually.
>
> This nl80211_free_aid is used to release an AID back to the kernel
> driver, which may release it to the FW, and that to the HW.
> diff --git a/src/common/intel-ltq-vendor.h b/src/common/intel-ltq-vendor.h
> @@ -21,6 +21,12 @@
> #ifndef INTEL_LTQ_VENDOR_H
> #define INTEL_LTQ_VENDOR_H
>
> +#include "ap/hostapd.h"
> +#include "ap/ap_drv_ops.h"
> +#include "l2_packet/l2_packet.h"
> +#include "drivers/driver_nl80211.h"
> +#include "ap/sta_info.h"
src/common/* should not include src/ap/*.h to maintain cleaner
interfaces between the components (src/ap being specific to AP
functionality). Similarly, there should be nothing nl80211-specific
implementation in src/common so that drivers/driver_nl80211.h must not
be included here. Furthermore, these includes get even more difficult
once this intel-ltq-vendor.h file gets included into
src/drivers/driver_nl80211.c in patch 6/6. src/drivers/* are not allowed
to include any src/ap header files.
>
> +#ifdef CONFIG_DRIVER_NL80211_INTEL_LTQ
> +static inline void ap_sta_remove_in_other_bss_now(struct hostapd_data *hapd,
> + const u8 *addr)
> +{
> + struct hostapd_iface *iface = hapd->iface;
> + size_t i;
...
Please do not implement long functions as static inline in a header
file. These should be in a .c file somewhere. In particular, when
dereferencing struct hostapd_data or hostapd_iface, the only acceptable
directory for these functions would be src/ap.
> +static inline int nl80211_get_aid(void *priv, u16 *aid, const u8 *addr)
> +{
> + int res = 0;
> + struct wpabuf *rsp_aid;
> + int aid_size = sizeof(u16);
> + struct i802_bss *bss = priv;
> + struct hostapd_data *hapd = bss->drv->ctx;
This combination is going to be very difficult to accept.. There should
be no mixing of core AP specific functionality (src/ap/*) and driver
interface specific functionality (that nl80211_ prefix in the function
sounds like that).
> + ap_sta_remove_in_other_bss_now(hapd, addr);
So that is clear src/ap/* stuff.
> + res = nl80211_vendor_cmd(priv, OUI_INTEL_LTQ,
> + INTEL_LTQ_NL80211_VENDOR_SUBCMD_GET_AID,
> + addr, ETH_ALEN, (struct wpabuf *) rsp_aid);
While this needs to be in src/drivers.
The core hostapd implementation outside src/drivers needs to be generic
and not have driver interface specific details like this.
src/drivers/driver.h defines the interface for performing driver
interface specific operations.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
