[PATCH v4 2/3] hostap: set driver probe-response template

[Arik Nemtsov]

On Sat, Dec 10, 2011 at 21:18, Jouni Malinen <j at w1.fi> wrote:
> On Thu, Nov 10, 2011 at 11:33:24AM +0200, Arik Nemtsov wrote:
>> Configure a probe response template for drivers that support it. The
>> template is updated when the beacon is updated.
>>
>> The probe response template is propagated to kernel via the set_ap()
>> callback.
>
> This was a bit difficult patch to review - it would have helped if it
> had been split in multiple parts. I ended up dropping parts that did not
> seem to be related and/or needed (splitting of ieee802_11_set_beacon()
> into pieces and renaming of some of the functions. In addition, I
> committed this in smaller parts to make it easier to review and address
> conflicts (this area is quite difficult between number of trees). Please

That's a good idea.

> let me know if I missed something from the patch that should really have
> went in.

The small refactor of the the gen_probe_resp (to remove the alloc
function) and the addition of hostapd_probe_resp_offloads() look ok.

>
>> diff --git a/src/ap/ap_list.c b/src/ap/ap_list.c
>> @@ -326,7 +326,7 @@ void ap_list_process_beacon(struct hostapd_iface *iface,
>> - ? ? ? ? ? ? ieee802_11_set_beacons(iface);
>> + ? ? ? ? ? ? ieee802_11_set_beacons_and_probes(iface);
>
> I dropped all renames like this as mentioned above..

Ok

>
>> diff --git a/src/ap/beacon.c b/src/ap/beacon.c
>> @@ -186,18 +186,133 @@ static u8 * hostapd_eid_wpa(struct hostapd_data *hapd, u8 *eid, size_t len)
>
>> +static size_t hostapd_gen_probe_resp(struct hostapd_data *hapd,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct ieee80211_mgmt *req,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ieee80211_mgmt *resp,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t resp_basic_len,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int is_p2p)
>> +{
>> + ? ? u8 *pos, *epos;
>> + ? ? struct sta_info *sta = NULL;
>
>> + ? ? ? ? ? ? host_to_le16(hostapd_own_capab_info(hapd, sta, 1));
>
> That's a bug... sta was hardcoded to NULL after this change. I changed
> the caller to pass in the sta pointer instead.

Right. Thanks for caching that.

>
>> -void ieee802_11_set_beacon(struct hostapd_data *hapd)
>> +static int hostapd_alloc_beacon(struct hostapd_data *hapd,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? u8 **head, u8 **tail)
>
> These changes of splitting ieee802_11_set_beacon() did not seem to
> really do anything, so I dropped them. This would have just added
> likelihood of conflicts between different trees.

The rename was just for clarity.

>
>> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
>> @@ -1444,6 +1457,20 @@ struct wpa_driver_ops {
>> ? ? ? int (*set_ap)(void *priv, struct wpa_driver_ap_params *params);
>>
>> ? ? ? /**
>> + ? ? ?* set_probe_resp - Set Probe Response frame template
>> + ? ? ?* @priv: Private driver interface data
>> + ? ? ?* @resp: Probe Response template
>> + ? ? ?* @resp_len: Length of the resp buffer in octets
>> + ? ? ?* Returns: 0 on success, -1 on failure
>> + ? ? ?*
>> + ? ? ?* This function is used to configure a Probe Response template for the
>> + ? ? ?* driver in AP mode. The driver is responsible for building the full
>> + ? ? ?* Probe Response frame by setting the destination address to that of
>> + ? ? ?* the desired recipient.
>> + ? ? ?*/
>> + ? ? int (*set_probe_resp)(void *priv, const u8 *resp, size_t resp_len);
>
> I assume this was just forgotten here - I dropped it.

Yea it's a remnant from a previous commit.

Arik