On Tue, 2024-05-28 at 02:39 +0000, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
> > Johannes suggested that we should set hw->wiphy->retry_{long,short}
> > before ieee80211_register_hw(). So that would go in
> > _rtl_init_mac80211(). That has the added benefit of making the
> > true retry limits visible to userspace ("iw phy").
> >
> > The problem is that setting hw->wiphy->retry_{long,short} is
> > not enough. rtlwifi still gets the default retry limits of 4
> > and 7, because ieee80211_register_hw() doesn't set
> > hw->conf.long_frame_max_tx_count (and hw->conf.short_frame_max_tx_count).
> > Johannes suggested moving this code from ieee80211_alloc_hw_nm()
> > to ieee80211_register_hw():
> >
> > local->hw.conf.long_frame_max_tx_count = wiphy->retry_long;
> > local->hw.conf.short_frame_max_tx_count = wiphy->retry_short;
Right, so I suggested that because then driver changes between alloc and
register can take effect rather than being overwritten, and these are
the retry limits that are actually visible to userspace.
> > I didn't do this yet partly because I don't want to compile
> > the entire kernel, and partly because I'm not sure how to handle
> > the different retry limits for AP/IBSS mode and station mode.
> >
> > Can we change hw->wiphy->retry_{long,short} any time, not just
> > before ieee80211_register_hw()? If yes, what is even the point
> > of hw->conf.{short,long}_frame_max_tx_count ? It would be simpler
> > if we can ignore them and use hw->wiphy->retry_{long,short}
> > directly.
> >
> > What do y'all think?
>
> Logically I think you can change hw->wiphy->retry_{long,short} any time,
> because cfg80211/mac80211 seemingly just bypass the values to driver.
Not quite, I think there's some additional layer of thing in mac80211?
> But we can't know if user space has set the value, right?
You can't check from the value, but that's why I said to change the
defaults before registering etc.
> One thing is that should we honor the values set by user space?
>
> If user space has not set, driver wants to control this value by itself
> according to AP/IBSS/station modes.
> If user space has set, driver fully follows the value from user space.
> Is above the behavior you want?
Honestly, is it? I think most drivers these days probably ignore the
values from userspace for lesser reasons (e.g. iwlwifi always has a
firmware-internal limit of 16 IIRC.)
This API also comes from the dawn of wireless in Linux, and was just
ported from wireless extensions to nl80211. So I wouldn't feel too bad
about simply ignoring it either.
johannes
