| Message ID | da9757e4cf34444792f586f33c9d481d@aphydexm01f.ap.qualcomm.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jun 30, 2016 at 9:29 AM, Undekari, Sunil Dutt <usdutt@qti.qualcomm.com> wrote: > I did discuss with Jouni earlier for a similar fix and his response was "Reserving an entry for WILDCARD_SSID_SCAN in this manner would be problematic since it would not allow even a single specific SSID to be scanned for with drivers that support only a single SSID per request.". We can restrict proposed patch only for cases when max_ssid > 1. > I agree with this and IMO , the number of hidden network ID's provided via the control interface should ensure to have a groom for this "wildcard" scan ssid entry ( the number should be one less than the max_size advertised by the driver). > This also asks for a new control interface command in wpa_supplicant to get the max_size supported by driver. > I don't see any other option , if the hidden_ssids have to be provided through the control interface. > > Regards, > Sunil > > > > -----Original Message----- > From: Hostap [mailto:hostap-bounces@lists.infradead.org] On Behalf Of Roshan Pius > Sent: Thursday, June 30, 2016 5:25 AM > To: hostap@lists.infradead.org > Cc: Roshan Pius <rpius@google.com> > Subject: [PATCH] Dont exceed scan ssid max size advertised by driver > > Currently |wpa_set_scan_ssids| fully exhausts > |wpa_driver_scan_params.ssid| list when hidden network ID's > are provided via the control interface. This results in us exceeding the max size for the list advertised by the driver when we add the "wildcard" scan ssid entry. So, ensure that we leave space for one more scan ssid entry in the list when we exit out of |wpa_set_scan_ssids|. > > Signed-off-by: Roshan Pius <rpius@google.com> > --- > wpa_supplicant/scan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c index 7a52826..8a29522 100644 > --- a/wpa_supplicant/scan.c > +++ b/wpa_supplicant/scan.c > @@ -593,7 +593,7 @@ static void wpa_set_scan_ssids(struct wpa_supplicant *wpa_s, > if (j < params->num_ssids) > continue; /* already in the list */ > > - if (params->num_ssids + 1 > max_ssids) { > + if (params->num_ssids + 1 >= max_ssids) { > wpa_printf(MSG_DEBUG, > "Over max scan SSIDs for manual request"); > break; > -- > 2.8.0.rc3.226.g39d4020 > > > _______________________________________________ > Hostap mailing list > Hostap@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/hostap > > _______________________________________________ > Hostap mailing list > Hostap@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/hostap
On Thu, Jun 30, 2016 at 10:21:38AM -0700, Dmitry Shmidt wrote: > On Thu, Jun 30, 2016 at 9:29 AM, Undekari, Sunil Dutt > <usdutt@qti.qualcomm.com> wrote: > > I did discuss with Jouni earlier for a similar fix and his response was "Reserving an entry for WILDCARD_SSID_SCAN in this manner would be problematic since it would not allow even a single specific SSID to be scanned for with drivers that support only a single SSID per request.". > > We can restrict proposed patch only for cases when max_ssid > 1. That's probably fine, but it should be noted that reserving a slot for the wildcard SSID in all scan requests would be pretty heavy extra latency for drivers that support only a small number of SSIDs (say, 2). Anyway, I'm not aware of such driver, so I think I'd accept such a patch with the understanding that most drivers support something closer to 16 SSIDs where the impact would be minimal.
On Thu, Jun 30, 2016 at 3:07 PM, Jouni Malinen <j@w1.fi> wrote: > > > That's probably fine, but it should be noted that reserving a slot for > the wildcard SSID in all scan requests would be pretty heavy extra > latency for drivers that support only a small number of SSIDs (say, 2). Do you expect the latency to be significant for drivers supporting a max of 4?
On Thu, Jul 07, 2016 at 02:14:41PM -0400, Jeffery Miller wrote: > On Thu, Jun 30, 2016 at 3:07 PM, Jouni Malinen <j@w1.fi> wrote: > > That's probably fine, but it should be noted that reserving a slot for > > the wildcard SSID in all scan requests would be pretty heavy extra > > latency for drivers that support only a small number of SSIDs (say, 2). > > Do you expect the latency to be significant for drivers supporting a max of 4? That depends on the use case.. If there is a huge number of specific SSIDs to scan for, reducing the maximum number of SSIDs from 4 to 3 per scan request has significantly more impact than moving from 16 to 15 would.. That said, I don't think this is really that significant an impact in practice even with a limit of 4.
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c index 7a52826..8a29522 100644 --- a/wpa_supplicant/scan.c +++ b/wpa_supplicant/scan.c @@ -593,7 +593,7 @@ static void wpa_set_scan_ssids(struct wpa_supplicant *wpa_s, if (j < params->num_ssids) continue; /* already in the list */ - if (params->num_ssids + 1 > max_ssids) { + if (params->num_ssids + 1 >= max_ssids) { wpa_printf(MSG_DEBUG, "Over max scan SSIDs for manual request"); break;