[PATCH V4 4/5] mac80211: add support for BSS coloring

[John Crispin]

On 14.08.20 10:41, Karthikeyan periyasamy wrote:
>> The CCA (color change announcement) is very similar to how CSA works 
>> where
>> we have an IE that includes a counter. When the counter hits 0, the new
>> color is applied via an updated beacon.
>>
>> This patch makes the CSA counter functionality reusable, rather than
>> implementing it again. This also allows for future reuse incase support
>> for other counter IEs gets added.
>>
>> Signed-off-by: John Crispin <john at phrozen.org>
>> ---
>>  include/net/mac80211.h     |  28 +++++
>>  net/mac80211/cfg.c         | 227 +++++++++++++++++++++++++++++++++++--
>>  net/mac80211/ieee80211_i.h |  12 ++
>>  net/mac80211/iface.c       |   3 +
>>  net/mac80211/tx.c          |  24 ++--
>>  5 files changed, 276 insertions(+), 18 deletions(-)
> ...
>>
>> +static int ieee80211_cca_finalize(struct ieee80211_sub_if_data *sdata)
>> +{
>> +    struct ieee80211_local *local = sdata->local;
>> +    u32 changed = 0;
>> +    int err;
>> +
>> +    sdata_assert_lock(sdata);
>> +    lockdep_assert_held(&local->mtx);
>> +
>> +    sdata->vif.cca_active = false;
>> +
>> +    err = ieee80211_set_after_cca_beacon(sdata, &changed);
>> +    if (err) {
>> +        cfg80211_color_change_aborted_notify(sdata->dev);
>> +        return err;
>> +    }
>> +
>> +    sdata->vif.bss_conf.he_bss_color.color = sdata->vif.cca_color;
>> +    sdata->vif.bss_conf.he_bss_color.enabled = 1;
>> +    changed |= BSS_CHANGED_HE_BSS_COLOR;
>> +
>
> Why we are not updating the bss color information in he_oper.params.
> IMHO remove the redundant information of bss color in the "struct 
> ieee80211_bss_conf" by removing
> the he_bss_color since he_oper.params holds more information in u32 
> itself.
> What do you think?
>
the he_oper field is const, so we cannot easily update it. In an AP 
setup the color value will only be used once during start_ap() and 
updated via dedicated bss_color/bss_config change notification.

in STA morde the value is derived from the remote beacon rx event (which 
is why we treat it as const.

generally the bss_config change is only related to the color and hence 
should not do a cross layer thing with the he_oper mode.

     John


>> + ieee80211_bss_info_change_notify(sdata, changed);
>> +
>> +    cfg80211_color_change_notify(sdata->dev);
>> +
>> +    return 0;
>> +}
>> +
>> +void ieee80211_cca_finalize_work(struct work_struct *work)
>> +{
>> +    struct ieee80211_sub_if_data *sdata =
>> +        container_of(work, struct ieee80211_sub_if_data,
>> +                 cca_finalize_work);
>> +    struct ieee80211_local *local = sdata->local;
>> +
>> +    sdata_lock(sdata);
>> +    mutex_lock(&local->mtx);
>> +
>> +    /* AP might have been stopped while waiting for the lock. */
>> +    if (!sdata->vif.cca_active)
>> +        goto unlock;
>> +
>> +    if (!ieee80211_sdata_running(sdata))
>> +        goto unlock;
>> +
>> +    ieee80211_cca_finalize(sdata);
>> +
>> +unlock:
>> +    mutex_unlock(&local->mtx);
>> +    sdata_unlock(sdata);
>> +}
>> +
>> +void ieee80211_cca_finish(struct ieee80211_vif *vif)
>> +{
>> +    struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
>> +
>> +    ieee80211_queue_work(&sdata->local->hw,
>> +                 &sdata->cca_finalize_work);
>> +}
>> +EXPORT_SYMBOL_GPL(ieee80211_cca_finish);
>> +
>> +void
>> +ieeee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
>> +                       u64 color_bitmap)
>> +{
>> +    struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
>> +
>> +    if (sdata->vif.cca_active || sdata->vif.csa_active)
>> +        return;
>> +
>> +    cfg80211_obss_color_collision_notify(sdata->dev, color_bitmap);
>> +}
>> +EXPORT_SYMBOL_GPL(ieeee80211_obss_color_collision_notify);
>> +
>> +static int
>> +__ieee80211_color_change(struct wiphy *wiphy, struct net_device *dev,
>> +             struct cfg80211_color_change_settings *params)
>> +{
>> +    struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>> +    struct ieee80211_local *local = sdata->local;
>> +    u32 changed = 0;
>> +    int err;
>> +
>> +    sdata_assert_lock(sdata);
>> +    lockdep_assert_held(&local->mtx);
>> +
>> +    /* don't allow another color change if one is already active or 
>> if csa
>> +     * is active
>> +     */
>> +    if (sdata->vif.cca_active || sdata->vif.csa_active)
>> +        return -EBUSY;
>> +
>> +    err = ieee80211_set_cca_beacon(sdata, params, &changed);
>> +    if (err)
>> +        return err;
>> +
>> +    sdata->vif.cca_active = true;
>> +    sdata->vif.cca_color = params->color;
>> +
>> +    cfg80211_color_change_started_notify(sdata->dev, params->count);
>> +
>> +    if (changed) {
>> +        sdata->vif.bss_conf.he_bss_color.enabled = 0;
>> +        changed |= BSS_CHANGED_HE_BSS_COLOR;
>
> same here
>
>> + ieee80211_bss_info_change_notify(sdata, changed);
>> +    } else {
>> +        /* if the beacon didn't change, we can finalize immediately */
>> +        ieee80211_cca_finalize(sdata);
>> +    }
>> +
>> +    return 0;
>> +}
>
> Thanks
> Karthikeyan P