DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 67AA320869
From:   Tamizh Chelvam Raja <tamizhr@qti.qualcomm.com>
To:     Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>,
        Tamizh chelvam <tamizhr@codeaurora.org>
CC:     "johannes@sipsolutions.net" <johannes@sipsolutions.net>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH 1/3] cfg80211: Add support to configure
 station specific RSSI threshold for AP mode
Thread-Topic: [EXTERNAL] Re: [PATCH 1/3] cfg80211: Add support to configure
 station specific RSSI threshold for AP mode
Thread-Index: AQHUZUOD4Uovi4zEWEunRvcK1EQvyKUhxzJw
Date:   Tue, 16 Oct 2018 12:47:33 +0000
Message-ID: <da28e67ec88a488ab62a88d0899657f3@aphydexm01f.ap.qualcomm.com>
References: <1539626250-769-1-git-send-email-tamizhr@codeaurora.org>
 <1539626250-769-2-git-send-email-tamizhr@codeaurora.org>
 <20181016112831.bglm53ydrktjy7i5@bars>
In-Reply-To: <20181016112831.bglm53ydrktjy7i5@bars>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

>> +static int nl80211_set_sta_mon(struct sk_buff *skb, struct genl_info=20
>> +*info) {
>> +       struct cfg80211_registered_device *rdev =3D info->user_ptr[0];
>> +       struct net_device *dev =3D info->user_ptr[1];
>> +       struct nlattr *attrs[NL80211_ATTR_CQM_MAX + 1];
>> +       bool fixed_thold =3D false;
>> +       struct nlattr *sta_mon;
>> +       u8 *addr =3D NULL;
>> +       int err;
>> +
>> +       if (!wiphy_ext_feature_isset(&rdev->wiphy,
>> +                                    NL80211_EXT_FEATURE_STA_MON_RSSI_CO=
NFIG))
>> +               return -EOPNOTSUPP;
>> +
>> +       sta_mon =3D info->attrs[NL80211_ATTR_CQM];
>> +       if (!sta_mon || !(info->attrs[NL80211_ATTR_MAC]))
>> +               return -EINVAL;
>> +
>> +       err =3D nla_parse_nested(attrs, NL80211_ATTR_CQM_MAX, sta_mon,
>> +                              nl80211_attr_cqm_policy, info->extack);
>> +       if (err)
>> +               return err;
>> +
>> +       addr =3D nla_data(info->attrs[NL80211_ATTR_MAC]);
>> +       fixed_thold =3D
>> +              =20
>> + nla_get_flag(info->attrs[NL80211_ATTR_STA_MON_FIXED_THOLD]);
>> +
>> +       if (attrs[NL80211_ATTR_CQM_RSSI_THOLD] &&
>> +           attrs[NL80211_ATTR_CQM_RSSI_HYST]) {
>> +               const s32 *thresholds =3D
>> +                       nla_data(attrs[NL80211_ATTR_CQM_RSSI_THOLD]);
>> +               int len =3D nla_len(attrs[NL80211_ATTR_CQM_RSSI_THOLD]);
>> +               u32 hysteresis =3D=20
>> + nla_get_u32(attrs[NL80211_ATTR_CQM_RSSI_HYST]);
>> +
>> +               if (len % 4)
>> +                       return -EINVAL;
>> +
>> +               err =3D nl80211_validate_rssi_tholds(thresholds, len / 4=
);
>> +               if (err)
>> +                       return err;
>> +
>> +               return rdev_set_sta_mon_rssi_config(rdev, dev, addr, thr=
esholds,
>> +                                                   hysteresis, len / 4,
>> +                                                   fixed_thold);
>> +       }
>> +
>> +       return -EINVAL;
>> +}

>IIUC there is a noticeable overlap between this new command and existing n=
l80211_set_cqm_rssi command. Is there any reason why >nl80211_set_cqm_rss c=
an not be adapted for AP case ?

[Tamizh] This new command for AP mode introduced as per the previous discus=
sion.=20

>If there is a reason to handle AP case separately, then it looks like it m=
akes sense to add explicit check for supported iftype in this new command. =
>Besides, it looks like there is no generic way to handle disabling of RSSI=
 monitoring in the new command.
>As a result, we may end up in multiple driver specific implementations.

[Tamizh] thanks for pointing me out. I'll add the ifmode check in the next =
patchset version.

Thanks,
Tamizh.