* Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
[not found] ` <1394547394-3910-4-git-send-email-luciano.coelho@intel.com>
@ 2023-01-09 9:39 ` Wen Gong
2023-01-09 10:05 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Wen Gong @ 2023-01-09 9:39 UTC (permalink / raw)
To: Luciano Coelho, linux-wireless
Cc: johannes, michal.kazior, sw, andrei.otcheretianski, eliad, ath11k
On 3/11/2014 10:16 PM, Luciano Coelho wrote:
> ...
> +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
> + const struct cfg80211_chan_def *chandef,
> + enum ieee80211_chanctx_mode chanmode,
> + u8 radar_detect)
> +{
> + struct ieee80211_local *local = sdata->local;
> + struct ieee80211_sub_if_data *sdata_iter;
> + enum nl80211_iftype iftype = sdata->wdev.iftype;
> + int num[NUM_NL80211_IFTYPES];
> + struct ieee80211_chanctx *ctx;
> + int num_different_channels = 1;
> + int total = 1;
> +
> + lockdep_assert_held(&local->chanctx_mtx);
> +
> + if (WARN_ON(hweight32(radar_detect) > 1))
> + return -EINVAL;
> +
> + if (WARN_ON(chanmode == IEEE80211_CHANCTX_SHARED && !chandef->chan))
> + return -EINVAL;
> +
> + if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
> + return -EINVAL;
> +
> + /* Always allow software iftypes */
> + if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
> + if (radar_detect)
> + return -EINVAL;
> + return 0;
> + }
> +
> + memset(num, 0, sizeof(num));
> +
> + if (iftype != NL80211_IFTYPE_UNSPECIFIED)
> + num[iftype] = 1;
> +
> + list_for_each_entry(ctx, &local->chanctx_list, list) {
> + if (ctx->conf.radar_enabled)
> + radar_detect |= BIT(ctx->conf.def.width);
> + if (ctx->mode == IEEE80211_CHANCTX_EXCLUSIVE) {
> + num_different_channels++;
> + continue;
> + }
> + if ((chanmode == IEEE80211_CHANCTX_SHARED) &&
> + cfg80211_chandef_compatible(chandef,
> + &ctx->conf.def))
> + continue;
> + num_different_channels++;
> + }
> +
> + list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> + struct wireless_dev *wdev_iter;
> +
> + wdev_iter = &sdata_iter->wdev;
> +
> + if (sdata_iter == sdata ||
> + rcu_access_pointer(sdata_iter->vif.chanctx_conf) == NULL ||
> + local->hw.wiphy->software_iftypes & BIT(wdev_iter->iftype))
> + continue;
> +
> + num[wdev_iter->iftype]++;
> + total++;
> + }
> +
> + if (total == 1 && !radar_detect)
> + return 0;
> +
should also check with cfg80211_check_combinations() when total == 1 and
num_different_channels > 1 ?
When MLO is enabled, it could have 2 channels for one ieee80211_sub_if_data.
> + return cfg80211_check_combinations(local->hw.wiphy,
> + num_different_channels,
> + radar_detect, num);
> +}
> ...
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
2023-01-09 9:39 ` [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211 Wen Gong
@ 2023-01-09 10:05 ` Johannes Berg
2023-01-10 7:23 ` Wen Gong
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2023-01-09 10:05 UTC (permalink / raw)
To: Wen Gong, Luciano Coelho, linux-wireless
Cc: michal.kazior, sw, andrei.otcheretianski, eliad, ath11k
On Mon, 2023-01-09 at 17:39 +0800, Wen Gong wrote:
> On 3/11/2014 10:16 PM, Luciano Coelho wrote:
> > ...
> > +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
> > + const struct cfg80211_chan_def *chandef,
> > + enum ieee80211_chanctx_mode chanmode,
> > + u8 radar_detect)
> > +{
> > + struct ieee80211_local *local = sdata->local;
> > + struct ieee80211_sub_if_data *sdata_iter;
> > + enum nl80211_iftype iftype = sdata->wdev.iftype;
> > + int num[NUM_NL80211_IFTYPES];
> > + struct ieee80211_chanctx *ctx;
> > + int num_different_channels = 1;
> > + int total = 1;
> > +
> > + lockdep_assert_held(&local->chanctx_mtx);
> > +
> > + if (WARN_ON(hweight32(radar_detect) > 1))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(chanmode == IEEE80211_CHANCTX_SHARED && !chandef->chan))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
> > + return -EINVAL;
> > +
> > + /* Always allow software iftypes */
> > + if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
> > + if (radar_detect)
> > + return -EINVAL;
> > + return 0;
> > + }
> > +
> > + memset(num, 0, sizeof(num));
> > +
> > + if (iftype != NL80211_IFTYPE_UNSPECIFIED)
> > + num[iftype] = 1;
> > +
> > + list_for_each_entry(ctx, &local->chanctx_list, list) {
> > + if (ctx->conf.radar_enabled)
> > + radar_detect |= BIT(ctx->conf.def.width);
> > + if (ctx->mode == IEEE80211_CHANCTX_EXCLUSIVE) {
> > + num_different_channels++;
> > + continue;
> > + }
> > + if ((chanmode == IEEE80211_CHANCTX_SHARED) &&
> > + cfg80211_chandef_compatible(chandef,
> > + &ctx->conf.def))
> > + continue;
> > + num_different_channels++;
> > + }
> > +
> > + list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> > + struct wireless_dev *wdev_iter;
> > +
> > + wdev_iter = &sdata_iter->wdev;
> > +
> > + if (sdata_iter == sdata ||
> > + rcu_access_pointer(sdata_iter->vif.chanctx_conf) == NULL ||
> > + local->hw.wiphy->software_iftypes & BIT(wdev_iter->iftype))
> > + continue;
> > +
> > + num[wdev_iter->iftype]++;
> > + total++;
> > + }
> > +
> > + if (total == 1 && !radar_detect)
> > + return 0;
> > +
>
> should also check with cfg80211_check_combinations() when total == 1 and
> num_different_channels > 1 ?
>
> When MLO is enabled, it could have 2 channels for one ieee80211_sub_if_data.
>
Heh. You're commenting on a patch from 2014, well before MLO :-)
Not sure what happens in the code now?
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
2023-01-09 10:05 ` Johannes Berg
@ 2023-01-10 7:23 ` Wen Gong
2023-02-14 14:41 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Wen Gong @ 2023-01-10 7:23 UTC (permalink / raw)
To: Johannes Berg, Luciano Coelho, linux-wireless
Cc: michal.kazior, sw, andrei.otcheretianski, eliad, ath11k
On 1/9/2023 6:05 PM, Johannes Berg wrote:
> On Mon, 2023-01-09 at 17:39 +0800, Wen Gong wrote:
>> On 3/11/2014 10:16 PM, Luciano Coelho wrote:
>>> ...
>>> +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>>> + const struct cfg80211_chan_def *chandef,
>>> + enum ieee80211_chanctx_mode chanmode,
>>> + u8 radar_detect)
...
>>> +
>>> + if (total == 1 && !radar_detect)
>>> + return 0;
>>> +
>> should also check with cfg80211_check_combinations() when total == 1 and
>> num_different_channels > 1 ?
>>
>> When MLO is enabled, it could have 2 channels for one ieee80211_sub_if_data.
>>
> Heh. You're commenting on a patch from 2014, well before MLO :-)
>
> Not sure what happens in the code now?
>
> johannes
Yes, it is 2014. Each interface only has one channel at 2014.
I did not hit issue for the code.
the story is like this:
When station interface and p2p device interface both running.
the station connect to MLO AP which has 2 links.
The ieee80211_link_use_channel()/ieee80211_check_combinations() call
cfg80211_check_combinations()
for the channel1 and channel2 because total==2.
When only station interface is running, not called for channel1/channel2
because total==1.
That is the little thing I hit.
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
2023-01-10 7:23 ` Wen Gong
@ 2023-02-14 14:41 ` Johannes Berg
2023-02-15 1:56 ` Wen Gong
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2023-02-14 14:41 UTC (permalink / raw)
To: Wen Gong, Luciano Coelho, linux-wireless
Cc: michal.kazior, sw, andrei.otcheretianski, eliad, ath11k
On Tue, 2023-01-10 at 15:23 +0800, Wen Gong wrote:
> On 1/9/2023 6:05 PM, Johannes Berg wrote:
> > On Mon, 2023-01-09 at 17:39 +0800, Wen Gong wrote:
> > > On 3/11/2014 10:16 PM, Luciano Coelho wrote:
> > > > ...
> > > > +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
> > > > + const struct cfg80211_chan_def *chandef,
> > > > + enum ieee80211_chanctx_mode chanmode,
> > > > + u8 radar_detect)
> ...
> > > > +
> > > > + if (total == 1 && !radar_detect)
> > > > + return 0;
> > > > +
> > > should also check with cfg80211_check_combinations() when total == 1 and
> > > num_different_channels > 1 ?
> > >
> > > When MLO is enabled, it could have 2 channels for one ieee80211_sub_if_data.
> > >
> > Heh. You're commenting on a patch from 2014, well before MLO :-)
> >
> > Not sure what happens in the code now?
> >
> > johannes
> Yes, it is 2014. Each interface only has one channel at 2014.
> I did not hit issue for the code.
>
> the story is like this:
> When station interface and p2p device interface both running.
> the station connect to MLO AP which has 2 links.
> The ieee80211_link_use_channel()/ieee80211_check_combinations() call
> cfg80211_check_combinations()
> for the channel1 and channel2 because total==2.
> When only station interface is running, not called for channel1/channel2
> because total==1.
> That is the little thing I hit.
>
So ... I guess you found a bug? Not sure what I'm supposed to say here.
Send a fix?
johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211
2023-02-14 14:41 ` Johannes Berg
@ 2023-02-15 1:56 ` Wen Gong
0 siblings, 0 replies; 5+ messages in thread
From: Wen Gong @ 2023-02-15 1:56 UTC (permalink / raw)
To: Johannes Berg, Luciano Coelho, linux-wireless
Cc: michal.kazior, sw, andrei.otcheretianski, eliad, ath11k
On 2/14/2023 10:41 PM, Johannes Berg wrote:
> On Tue, 2023-01-10 at 15:23 +0800, Wen Gong wrote:
>> On 1/9/2023 6:05 PM, Johannes Berg wrote:
>>> On Mon, 2023-01-09 at 17:39 +0800, Wen Gong wrote:
>>>> On 3/11/2014 10:16 PM, Luciano Coelho wrote:
>>>>> ...
>>>>> +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>>>>> + const struct cfg80211_chan_def *chandef,
>>>>> + enum ieee80211_chanctx_mode chanmode,
>>>>> + u8 radar_detect)
>> ...
>>>>> +
>>>>> + if (total == 1 && !radar_detect)
>>>>> + return 0;
>>>>> +
>>>> should also check with cfg80211_check_combinations() when total == 1 and
>>>> num_different_channels > 1 ?
>>>>
>>>> When MLO is enabled, it could have 2 channels for one ieee80211_sub_if_data.
>>>>
>>> Heh. You're commenting on a patch from 2014, well before MLO :-)
>>>
>>> Not sure what happens in the code now?
>>>
>>> johannes
>> Yes, it is 2014. Each interface only has one channel at 2014.
>> I did not hit issue for the code.
>>
>> the story is like this:
>> When station interface and p2p device interface both running.
>> the station connect to MLO AP which has 2 links.
>> The ieee80211_link_use_channel()/ieee80211_check_combinations() call
>> cfg80211_check_combinations()
>> for the channel1 and channel2 because total==2.
>> When only station interface is running, not called for channel1/channel2
>> because total==1.
>> That is the little thing I hit.
>>
> So ... I guess you found a bug? Not sure what I'm supposed to say here.
> Send a fix?
I consider it NOT a bug. It is only the check is not strict here.
>
> johannes
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-15 1:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1394547394-3910-1-git-send-email-luciano.coelho@intel.com>
[not found] ` <1394547394-3910-4-git-send-email-luciano.coelho@intel.com>
2023-01-09 9:39 ` [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211 Wen Gong
2023-01-09 10:05 ` Johannes Berg
2023-01-10 7:23 ` Wen Gong
2023-02-14 14:41 ` Johannes Berg
2023-02-15 1:56 ` Wen Gong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).