by Nathan Chancellor

[permalink] [raw]

Subject: Re: [PATCHv7 5/6] nl80211: Add netlink attribute to configure TID specific tx rate

Hi Tamizh,

On Sat, Nov 02, 2019 at 11:37:49AM +0530, Tamizh chelvam wrote:
> Introduce NL80211_TID_CONFIG_ATTR_TX_RATES in nl80211_tid_attr_config
> to accept data TID specific TX bitrate configuration
> through NL80211_CMD_SET_TID_CONFIG command. TID for which the
> this configuration is to be applied is passed in
> NL80211_TID_CONFIG_ATTR_TID attribute. TX bitrate mask values passed
> in NL80211_ATTR_TX_RATES attribute and NL80211_TID_CONFIG_ATTR_TX_RATES
> attribute will have types of the TX rate should be applied. This uses
> nl80211_parse_tx_bitrate_mask to validate and calculate the bitrate
> mask. When the user-space wants this configuration peer specific
> rather than being applied for all the connected stations,
> MAC address of the peer can be passed in NL80211_ATTR_MAC attribute.
>
> Driver supporting this feature should advertise
> NL80211_EXT_FEATURE_PER_TID_TX_BITRATE_MASK and supporting per-STA data TID
> TX bitrate configuration should advertise
> NL80211_EXT_FEATURE_PER_STA_TX_BITRATE_MASK.
>
> Signed-off-by: Tamizh chelvam <[email protected]>

Below is a report from the 0day team doing clang builds for us, please
take a look at it when you get a chance. Mixing enums like this usually
means the code is not using the enums properly; if it is intentional,
consider explicitly casting the enum to eliminate the warning.

Cheers,
Nathan

On Sun, Nov 03, 2019 at 11:14:10AM +0800, kbuild test robot wrote:
> CC: [email protected]
> In-Reply-To: <[email protected]>
> References: <[email protected]>
> TO: Tamizh chelvam <[email protected]>
> CC: [email protected]
> CC: [email protected], Tamizh chelvam <[email protected]>
>
> Hi Tamizh,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on mac80211-next/master]
> [also build test WARNING on v5.4-rc5 next-20191031]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Tamizh-chelvam/nl80211-New-netlink-command-for-TID-specific-configuration/20191103-092741
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> config: arm64-defconfig (attached as .config)
> compiler: clang version 10.0.0 (git://gitmirror/llvm_project 97725707f488d3f00f94d4c4f2d9e193c302b23c)
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm64
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> net/wireless/nl80211.c:13912:11: warning: implicit conversion from enumeration type 'enum nl80211_tid_config_attr' to different enumeration type 'enum nl80211_attrs' [-Wenum-conversion]
> attr = NL80211_TID_CONFIG_ATTR_TX_RATES;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 warning generated.
>
> vim +13912 net/wireless/nl80211.c
>
> 13805
> 13806 static int parse_tid_conf(struct cfg80211_registered_device *rdev,
> 13807 struct nlattr *attrs[],
> 13808 struct ieee80211_tid_cfg *tid_conf,
> 13809 struct genl_info *info, const u8 *peer)
> 13810 {
> 13811 struct netlink_ext_ack *extack = info->extack;
> 13812 int err;
> 13813
> 13814 tid_conf->tid = nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_TID]);
> 13815 if (attrs[NL80211_TID_CONFIG_ATTR_NOACK]) {
> 13816 err = nl80211_check_tid_config_support(rdev, extack, peer,
> 13817 attrs[NL80211_TID_CONFIG_ATTR_NOACK],
> 13818 NL80211_EXT_FEATURE_PER_TID_NOACK_CONFIG,
> 13819 NL80211_EXT_FEATURE_PER_STA_NOACK_CONFIG);
> 13820 if (err)
> 13821 return err;
> 13822
> 13823 tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_NOACK;
> 13824 tid_conf->noack =
> 13825 nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_NOACK]);
> 13826 }
> 13827
> 13828 if (nla_get_flag(attrs[NL80211_TID_CONFIG_ATTR_RETRY])) {
> 13829 err = nl80211_check_tid_config_support(rdev, extack, peer,
> 13830 attrs[NL80211_TID_CONFIG_ATTR_RETRY],
> 13831 NL80211_EXT_FEATURE_PER_TID_RETRY_CONFIG,
> 13832 NL80211_EXT_FEATURE_PER_STA_RETRY_CONFIG);
> 13833 if (err)
> 13834 return err;
> 13835
> 13836 tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_RETRY;
> 13837 if (attrs[NL80211_TID_CONFIG_ATTR_RETRY_SHORT]) {
> 13838 tid_conf->retry_short =
> 13839 nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RETRY_SHORT]);
> 13840 if (tid_conf->retry_short >
> 13841 rdev->wiphy.max_data_retry_count)
> 13842 return -EINVAL;
> 13843 } else {
> 13844 /* Use driver default retry count or VIF specific
> 13845 * retry count
> 13846 */
> 13847 tid_conf->retry_short = -1;
> 13848 }
> 13849
> 13850 if (attrs[NL80211_TID_CONFIG_ATTR_RETRY_LONG]) {
> 13851 tid_conf->retry_long =
> 13852 nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RETRY_LONG]);
> 13853 if (tid_conf->retry_long >
> 13854 rdev->wiphy.max_data_retry_count)
> 13855 return -EINVAL;
> 13856 } else {
> 13857 /* Use driver default retry count or VIF specific
> 13858 * retry count
> 13859 */
> 13860 tid_conf->retry_long = -1;
> 13861 }
> 13862 }
> 13863
> 13864 if (attrs[NL80211_TID_CONFIG_ATTR_AMPDU_CTRL]) {
> 13865 err = nl80211_check_tid_config_support(rdev, extack, peer,
> 13866 attrs[NL80211_TID_CONFIG_ATTR_AMPDU_CTRL],
> 13867 NL80211_EXT_FEATURE_PER_TID_AMPDU_CTRL,
> 13868 NL80211_EXT_FEATURE_PER_STA_AMPDU_CTRL);
> 13869 if (err)
> 13870 return err;
> 13871
> 13872 tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_AMPDU;
> 13873 tid_conf->ampdu =
> 13874 nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_AMPDU_CTRL]);
> 13875 }
> 13876
> 13877 if (attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]) {
> 13878 err = nl80211_check_tid_config_support(rdev, extack, peer,
> 13879 attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL],
> 13880 NL80211_EXT_FEATURE_PER_TID_RTSCTS_CTRL,
> 13881 NL80211_EXT_FEATURE_PER_STA_RTSCTS_CTRL);
> 13882 if (err)
> 13883 return err;
> 13884
> 13885 tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_RTSCTS;
> 13886 tid_conf->rtscts =
> 13887 nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]);
> 13888 }
> 13889
> 13890 if (attrs[NL80211_TID_CONFIG_ATTR_TX_RATES_TYPE]) {
> 13891 int idx;
> 13892 enum nl80211_attrs attr;
> 13893
> 13894 err = nl80211_check_tid_config_support(rdev, extack, peer,
> 13895 attrs[NL80211_TID_CONFIG_ATTR_TX_RATES_TYPE],
> 13896 NL80211_EXT_FEATURE_PER_TID_TX_BITRATE_MASK,
> 13897 NL80211_EXT_FEATURE_PER_STA_TX_BITRATE_MASK);
> 13898 if (err)
> 13899 return err;
> 13900
> 13901 idx = NL80211_TID_CONFIG_ATTR_TX_RATES_TYPE;
> 13902 tid_conf->txrate_type = nla_get_u8(attrs[idx]);
> 13903
> 13904 tid_conf->tid_conf_mask |= IEEE80211_TID_CONF_TX_BITRATE;
> 13905 if (tid_conf->txrate_type != NL80211_TX_RATE_AUTOMATIC) {
> 13906 tid_conf->mask =
> 13907 kzalloc(sizeof(struct cfg80211_bitrate_mask),
> 13908 GFP_KERNEL);
> 13909 if (!tid_conf->mask)
> 13910 return -ENOMEM;
> 13911
> 13912 attr = NL80211_TID_CONFIG_ATTR_TX_RATES;
> 13913 err = nl80211_parse_tx_bitrate_mask(info, attrs, attr,
> 13914 tid_conf->mask);
> 13915 if (err) {
> 13916 kfree(tid_conf->mask);
> 13917 return err;
> 13918 }
> 13919 } else {
> 13920 tid_conf->mask = NULL;
> 13921 }
> 13922 }
> 13923
> 13924 return 0;
> 13925 }
> 13926
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation