[PATCH 4/4] hostapd: Use channel switch fallback on error

[Michal Kazior]

On 25 June 2014 15:52, Otcheretianski, Andrei
<andrei.otcheretianski at intel.com> wrote:
> Hi,
> See my inline comments.
>
> Andrei
[...]
>> +
>> +static void hostapd_event_iface_unavailable(struct hostapd_data *hapd)
>> +{
>> +     wpa_printf(MSG_DEBUG, "Interface is unavailable -- stopped");
>
> check csa_in_progress here maybe before calling to fallback?

Makes sense considering your other suggestions.


[...]
>>
>> +     iface->csa_freq = settings->freq_params.freq;
>> +     iface->csa_channel = settings->freq_params.channel;
>> +     iface->csa_secondary_channel =
>> +settings->freq_params.sec_channel_offset;
>> +
>> +     if (settings->freq_params.center_freq1)
>> +             iface->csa_vht_oper_centr_freq_seg0_idx =
>> +                     36 + (settings->freq_params.center_freq1 - 5180) / 5;
>> +
>> +     if (settings->freq_params.center_freq2)
>> +             iface->csa_vht_oper_centr_freq_seg1_idx =
>> +                     36 + (settings->freq_params.center_freq2 - 5180) / 5;
>> +
>
> You have the entire freq_params inside hostapd_data.. Why do you need to copy?

I believe cs_freq_params wasn't around when I originally made this
patch. And then I failed to update this accordingly.


>>       ret = hostapd_drv_switch_channel(hapd, settings);
>>       free_beacon_data(&settings->beacon_csa);
>>       free_beacon_data(&settings->beacon_after);
>> @@ -2382,4 +2396,31 @@ int hostapd_switch_channel(struct hostapd_data
>> *hapd,
>>       return 0;
>>  }
>>
>> +void hostapd_switch_channel_fallback(struct hostapd_data *hapd) {
>
> I think this function should receive hostapd_iface as param, otherwise it's quite confusing that all the iface is restarted.

I agree.


>
>> +     struct hostapd_iface *iface = hapd->iface;
>> +     int i;
>> +
>> +     if (!hapd->csa_in_progress)
>> +             return;
>
> And then this if should be outside

I agree.


>
>> +
>> +     for (i = 0; i < iface->num_bss; i++) {
>> +             iface->bss[i]->csa_in_progress = 0;
>> +             iface->bss[i]->cs_freq_params.freq = 0;
>
> hostapd_cleanup_cs_params(iface->bss[i])

You're right.


>> +     }
>> +
>> +     wpa_printf(MSG_WARNING, "CSA failed, trying fallback by toggling
>> +interface");
>> +
>> +     iface->freq = iface->csa_freq;
>> +     iface->conf->channel = iface->csa_channel;
>> +     iface->conf->secondary_channel = iface->csa_secondary_channel;
>> +     iface->conf->vht_oper_centr_freq_seg0_idx =
>> +             iface->csa_vht_oper_centr_freq_seg0_idx;
>> +     iface->conf->vht_oper_centr_freq_seg1_idx =
>> +             iface->csa_vht_oper_centr_freq_seg1_idx;
>
> It's possible to get all this info from hostapd_data->csa_freq_params and pass it as argument to this function.

Good idea.


Thanks for your review. I'll re-spin later :-)


Micha?