Message ID | 1451219771-10978-1-git-send-email-ilan.peer@intel.com |
---|---|
State | Accepted |
Headers | show |
On Sun, Dec 27, 2015 at 02:36:11PM +0200, Ilan Peer wrote: > As a dedicated P2P Device interface does not have a network > interface associated with it, trying to call linux_iface_up() > on it would always fail so this call can be skipped for > such an interface. > > Getting interface nlmode can be done only after bss->wdev_id is > set, so move this call to wpa_driver_nl80211_finish_drv_init(), > and do it only in case the nlmode != NL80211_IFTYPE_P2P_DEVICE. Does this need to be done after the wpa_driver_nl80211_capa() call here? > @@ -2243,6 +2240,11 @@ wpa_driver_nl80211_finish_drv_init(struct wpa_driver_nl80211_data *drv, > if (wpa_driver_nl80211_capa(drv)) > return -1; > > + if (first && > + nl80211_get_ifmode(bss) != NL80211_IFTYPE_P2P_DEVICE && > + linux_iface_up(drv->global->ioctl_sock, bss->ifname) > 0) > + drv->start_iface_up = 1; The sequence used here would be different for fetching driver capabilities compared to what was used previously (i.e., the interface would now be down while it was up previously). I'd rather avoid such a change here (i.e., reorder these two calls with wpa_driver_nl80211_finish_drv_init()) unless this nl80211_get_ifmode() part is not expected to work here without the driver capabilities fetched. If this specific sequence is needed, I'd like to get better understand on whether this could break any existing use cases (i.e., whether there is a driver that cannot return correct capabilities without the interface being UP first). If I understood correctly, this new patch does not really fix anything other than maybe making the debug log cleaner (and well, if it does fix something, the commit log should be modified to say so).
> -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Monday, December 28, 2015 16:05 > To: Peer, Ilan > Cc: hostap@lists.infradead.org > Subject: Re: [PATCH] nl80211: Don't call linux_iface_up() for a dedicated P2P > Device > > On Sun, Dec 27, 2015 at 02:36:11PM +0200, Ilan Peer wrote: > > As a dedicated P2P Device interface does not have a network interface > > associated with it, trying to call linux_iface_up() on it would always > > fail so this call can be skipped for such an interface. > > > > Getting interface nlmode can be done only after bss->wdev_id is set, > > so move this call to wpa_driver_nl80211_finish_drv_init(), > > and do it only in case the nlmode != NL80211_IFTYPE_P2P_DEVICE. > > Does this need to be done after the wpa_driver_nl80211_capa() call here? No. The important thing is to have wdev_id set, so it can be used in nl80211_cmd_msg(). > > > @@ -2243,6 +2240,11 @@ wpa_driver_nl80211_finish_drv_init(struct > wpa_driver_nl80211_data *drv, > > if (wpa_driver_nl80211_capa(drv)) > > return -1; > > > > + if (first && > > + nl80211_get_ifmode(bss) != NL80211_IFTYPE_P2P_DEVICE && > > + linux_iface_up(drv->global->ioctl_sock, bss->ifname) > 0) > > + drv->start_iface_up = 1; > > The sequence used here would be different for fetching driver capabilities > compared to what was used previously (i.e., the interface would now be down > while it was up previously). I'd rather avoid such a change here (i.e., reorder > these two calls with > wpa_driver_nl80211_finish_drv_init()) unless this nl80211_get_ifmode() part > is not expected to work here without the driver capabilities fetched. If this This should still work as expected as AFAIU the only calls command that are used before it this flow are ones that do not require the interface to be up. But as stated above this call can be moved before querying the capabilities. > specific sequence is needed, I'd like to get better understand on whether this > could break any existing use cases (i.e., whether there is a driver that cannot > return correct capabilities without the interface being UP first). If I > understood correctly, this new patch does not really fix anything other than > maybe making the debug log cleaner (and well, if it does fix something, the > commit log should be modified to say so). This is only log cleanup. Should I send a fixed patch that move the lines before querying the capabilities? Thanks, Ilan.
On Mon, Dec 28, 2015 at 02:47:55PM +0000, Peer, Ilan wrote: > > From: Jouni Malinen [mailto:j@w1.fi] > > Does this need to be done after the wpa_driver_nl80211_capa() call here? > > No. The important thing is to have wdev_id set, so it can be used in nl80211_cmd_msg(). OK, thanks. > Should I send a fixed patch that move the lines before querying the capabilities? No need, I'll move it when applying the current patch.
On Sun, Dec 27, 2015 at 02:36:11PM +0200, Ilan Peer wrote: > As a dedicated P2P Device interface does not have a network > interface associated with it, trying to call linux_iface_up() > on it would always fail so this call can be skipped for > such an interface. > > Getting interface nlmode can be done only after bss->wdev_id is > set, so move this call to wpa_driver_nl80211_finish_drv_init(), > and do it only in case the nlmode != NL80211_IFTYPE_P2P_DEVICE. Thanks, applied.
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index ed5e4a8..b3d56b2 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -1735,9 +1735,6 @@ static void * wpa_driver_nl80211_drv_init(void *ctx, const char *ifname, if (nl80211_init_bss(bss)) goto failed; - if (linux_iface_up(drv->global->ioctl_sock, ifname) > 0) - drv->start_iface_up = 1; - if (wpa_driver_nl80211_finish_drv_init(drv, set_addr, 1, driver_params)) goto failed; @@ -2243,6 +2240,11 @@ wpa_driver_nl80211_finish_drv_init(struct wpa_driver_nl80211_data *drv, if (wpa_driver_nl80211_capa(drv)) return -1; + if (first && + nl80211_get_ifmode(bss) != NL80211_IFTYPE_P2P_DEVICE && + linux_iface_up(drv->global->ioctl_sock, bss->ifname) > 0) + drv->start_iface_up = 1; + if (driver_params && nl80211_set_param(bss, driver_params) < 0) return -1;
As a dedicated P2P Device interface does not have a network interface associated with it, trying to call linux_iface_up() on it would always fail so this call can be skipped for such an interface. Getting interface nlmode can be done only after bss->wdev_id is set, so move this call to wpa_driver_nl80211_finish_drv_init(), and do it only in case the nlmode != NL80211_IFTYPE_P2P_DEVICE. Signed-off-by: Ilan Peer <ilan.peer@intel.com> --- src/drivers/driver_nl80211.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)