Re: [PATCH v10] cfg80211: Provision to allow the support for different beacon intervals

2016-10-13 Thread Johannes Berg
On Wed, 2016-10-12 at 18:26 +0530, Purushottam Kushwaha wrote:
> This commit provides a mechanism for the host drivers to advertise
> the
> support for different beacon intervals among the respective interface
> combinations in a group, through beacon_int_min_gcd (u32).
> This beacon_int_min_gcd will be compared against GCD of all beaconing
> interfaces of matching combinations.

Applied. I made some more changes, in particular:

> @@ -3120,7 +3130,7 @@ struct ieee80211_iface_limit {
>   *  };
>   *
>   *
> - * 2. Allow #{AP, P2P-GO} <= 8, channels = 1, 8 total:
> + * 2. Allow #{AP, P2P-GO} <= 8, BI min gcd = 10, channels = 1, 8
> total:
>   *
>   *  struct ieee80211_iface_limit limits2[] = {
>   *   { .max = 8, .types = BIT(NL80211_IFTYPE_AP) |
> @@ -3131,6 +3141,7 @@ struct ieee80211_iface_limit {
>   *   .n_limits = ARRAY_SIZE(limits2),
>   *   .max_interfaces = 8,
>   *   .num_different_channels = 1,
> + *   .beacon_int_min_gcd = 10,
>   *  };
>   *

I removed this, because it would conflict with the other documentation
changes in this area going through the doc tree. I didn't think this
was important enough in the example to break the merge later.

>   list_for_each_entry(wdev, >wiphy.wdev_list, list) {
>   if (!wdev->beacon_interval)
>   continue;
>   if (wdev->beacon_interval != beacon_int) {
> - res = -EINVAL;
> + params.diff_bi = true;
> + /* Get the GCD */
> + bi_prev = wdev->beacon_interval;
> + while (bi_prev != 0) {
> + tmp_bi = bi_prev;
> + bi_prev = params.beacon_gcd %
> bi_prev;
> + params.beacon_gcd = tmp_bi;
> + }

I changed that a bit, moving the variables in.

>   break;

and, more importantly, I removed this break - we need to look at all
interfaces now, not break after the first one with different BI.

>   }
>   }
>  
> - return res;
> + res = cfg80211_check_combinations(>wiphy, );
> + return (res < 0) ? res : 0;

removed the res variable too

johannes


Re: [PATCH v10] cfg80211: Provision to allow the support for different beacon intervals

2016-10-13 Thread Johannes Berg

> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -421,6 +421,8 @@ static int brcmf_vif_change_validate(struct
> brcmf_cfg80211_info *cfg,
>   .num_different_channels = 1,
>   .radar_detect = 0,
>   .iftype_num = {0},
> + .beacon_gcd = 0,
> + .diff_bi = false,
>   };
>  
>   list_for_each_entry(pos, >vif_list, list)
> @@ -446,6 +448,8 @@ static int brcmf_vif_add_validate(struct
> brcmf_cfg80211_info *cfg,
>   .num_different_channels = 1,
>   .radar_detect = 0,
>   .iftype_num = {0},
> + .beacon_gcd = 0,
> + .diff_bi = false,
>   };

You don't need this now, since the default is 0/false, so we don't have
to touch this file. I already removed some of the other useless
initializers in the previous patch, and will edit this out as well - no
need to resend.
 
>   list_for_each_entry(pos, >vif_list, list)
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 21bbe44..f0bcd55 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -784,11 +784,15 @@ struct cfg80211_csa_settings {
>   * @iftype_num: array with the numbers of interfaces of each
> interface
>   *   type.  The index is the interface type as specified in
> 
>   *   nl80211_iftype.
> + * @beacon_gcd: a value specifying GCD of all beaconing interfaces.
> + * @diff_bi: a flag which denotes beacon intervals are different or
> same.

I think I'll rename both to beacon_* or *_bi, but I don't really like
the difference in naming here.

>   */
>  struct iface_combination_params {
>   int num_different_channels;
>   u8 radar_detect;
>   int iftype_num[NUM_NL80211_IFTYPES];
> + u32 beacon_gcd;
> + bool diff_bi;
>  };
>  
>  /**
> @@ -3100,6 +3104,12 @@ struct ieee80211_iface_limit {
>   *   only in special cases.
>   * @radar_detect_widths: bitmap of channel widths supported for
> radar detection
>   * @radar_detect_regions: bitmap of regions supported for radar
> detection
> + * @beacon_int_min_gcd: This interface combination supports
> different
> + *   beacon intervals.
> + *   = 0 - all beacon intervals for different interface must be
> same.
> + *   > 0 - any beacon interval for the interface part of this
> combination AND
> + *     *GCD* of all beacon intervals from beaconing
> interfaces of this
> + *     combination must be greator or equal to this value.

typo: greater - will also fix

>   * With this structure the driver can describe which interface
>   * combinations it supports concurrently.
> @@ -3120,7 +3130,7 @@ struct ieee80211_iface_limit {
>   *  };
>   *
>   *
> - * 2. Allow #{AP, P2P-GO} <= 8, channels = 1, 8 total:
> + * 2. Allow #{AP, P2P-GO} <= 8, BI min gcd = 10, channels = 1, 8
> total:
>   *
>   *  struct ieee80211_iface_limit limits2[] = {
>   *   { .max = 8, .types = BIT(NL80211_IFTYPE_AP) |
> @@ -3131,6 +3141,7 @@ struct ieee80211_iface_limit {
>   *   .n_limits = ARRAY_SIZE(limits2),
>   *   .max_interfaces = 8,
>   *   .num_different_channels = 1,
> + *   .beacon_int_min_gcd = 10,
>   *  };
>   *
>   *
> @@ -3158,6 +3169,7 @@ struct ieee80211_iface_combination {
>   bool beacon_int_infra_match;
>   u8 radar_detect_widths;
>   u8 radar_detect_regions;
> + u32 beacon_int_min_gcd;
>  };
>  
>  struct ieee80211_txrx_stypes {
> diff --git a/include/uapi/linux/nl80211.h
> b/include/uapi/linux/nl80211.h
> index 56368e9..3c19cca 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -4280,6 +4280,9 @@ enum nl80211_iface_limit_attrs {
>   *   of supported channel widths for radar detection.
>   * @NL80211_IFACE_COMB_RADAR_DETECT_REGIONS: u32 attribute
> containing the bitmap
>   *   of supported regulatory regions for radar detection.
> + * @NL80211_IFACE_COMB_BI_MIN_GCD: u32 attribute specifying the
> minimum GCD of
> + *   different beacon intervals supported by all the interface
> combinations
> + *   in this group (if not present, all beacon interval must
> match).

beacon intervals

>   * @NUM_NL80211_IFACE_COMB: number of attributes
>   * @MAX_NL80211_IFACE_COMB: highest attribute number
>   *
> @@ -4287,8 +4290,8 @@ enum nl80211_iface_limit_attrs {
>   *   limits = [ #{STA} <= 1, #{AP} <= 1 ], matching BI,
> channels = 1, max = 2
>   *   => allows an AP and a STA that must match BIs
>   *
> - *   numbers = [ #{AP, P2P-GO} <= 8 ], channels = 1, max = 8
> - *   => allows 8 of AP/GO
> + *   numbers = [ #{AP, P2P-GO} <= 8 ], BI min gcd, channels =
> 1, max = 8,
> + *   => allows 8 of AP/GO that can have BI gcd >= min gcd
>   *
>   *   numbers = [ #{STA} <= 2 ], channels = 2, max = 2
>   *   => allows two STAs on different channels
> @@ -4314,6 +4317,7 @@ enum nl80211_if_combination_attrs {
>   NL80211_IFACE_COMB_NUM_CHANNELS,
>   NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
>