On Mon, 27 Jan 2025 at 07:58, Bjoern A. Zeeb <b...@freebsd.org> wrote:

> Hi,
>
> before more drivers are growing this (LinuxKPI is currently thus I
> noticed) beyond the two rtwn chipsets:
>
> sys/dev/rtwn/rtl8192c/r92c_rx.c:                rxs->c_phytype =
> IEEE80211_RX_FP_11B;
> sys/dev/rtwn/rtl8192c/r92c_rx.c:                rxs->c_phytype =
> IEEE80211_RX_FP_11G;
> sys/dev/rtwn/rtl8192c/r92c_rx.c:                rxs->c_phytype =
> IEEE80211_RX_FP_11NG;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:                rxs->c_phytype =
> IEEE80211_RX_FP_11B;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:
> rxs->c_phytype = IEEE80211_RX_FP_11A;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:
> rxs->c_phytype = IEEE80211_RX_FP_11G;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:
> rxs->c_phytype = IEEE80211_RX_FP_11NA;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:
> rxs->c_phytype = IEEE80211_RX_FP_11NG;
> sys/dev/rtwn/rtl8812a/r12a_rx.c:                        rxs->c_phytype =
> IEEE80211_RX_FP_11NA;
>
> These flags are kind-of redundant and along with
>
> #define IEEE80211_RX_F_CCK              0x00001000
> #define IEEE80211_RX_F_OFDM             0x00002000
>
> Then we would also have the band in c_band avail given we know
> it for the current c_phytype.
>

The reason for the split between band and phytype was because way back when
I found some NICs didn't
give us all the receive frame info, especially in offload scenarios. We may
only get back something like an
802.3 encapsulated frame with no rate info, but we know what channel it was
received on.


>
> I would suggest (as it really simplifies the logic in drivers) to do
> these conversions once centrally if we really need them for anything
> and simply report the encoding up insetad of what currently is
> c_phytype, that is legacy, HT, VHT, .. as that removes a lot of
> redundancy and confusion.
>
> And we kind-of have that already as well given we have:
> #define IEEE80211_RX_F_HT               0x00004000
> #define IEEE80211_RX_F_VHT              0x00008000
>
> So there's redundancy.  I want to understand why?  The c_phytype also
> has no notion of AC but then again we do have the VHT flag.
>

It likely doesn't have a VHT flag because we haven't added one yet. :-)


>
> (I am not even going to mention what happens if c_band, c_freq, c_ieee,
> the c_pktflags and the current c_phytype would not agree).
>

Ah, band, freq, and ieee. If you're lucky, and most of the NICs we play
with are
"normal" nowdays, there's going to be a nice line-up between frequency, mode
and ieee number.

However in the past (and maybe future, we'll see) people made custom NICs
that operated
on other frequencies. They'd take a 2GHz or 5GHz NIC and they'd add a
down/up converter
and filter so they could operate on things like 3GHz, 1.2GHz, 900MHz, etc.
If you look at our
regdomain / code you'll see some 900MHz conversion support for older NICs
in there.

In that case, ieee, freq and band don't always line up.

Then, you have the problem with half/quarter rates and the allowable
frequencies.
This is where I started trying to clean it up (eg by making c_freq khz
instead of mhz)
and life got in the way. Specifically, the older atheros NICs didn't allow
you to set arbitrary
frequencies, they had to be on very specific boundaries. So the
half/quarter channel
gaps weren't ACTUALLY what they said they were.)

(I documented the details in the radio code for the AR5212/AR9280 HAL).

But the AR9820 and later radios allow you to tune to a KHz boundary because
they used
a fractional synthesiser.

Which meant until I err, "fixed" the AR9280 RF programming code to
interoperate, they
didn't interoperate. ifconfig and all the programming said one thing, but
the spectrum analyser
said something completely different.

So, my goal here was to be able to capture if a NIC is operating on a
frequency that was "off"
from what was programmed, so we could at least diagnose what the heck was
going on.

It /may/ make sense to also capture information from later NICs that can
report when the
centre frequency is off due to doppler effects (eg if you're moving), since
some (eg rtwn)
can report the offsets. However that should be a different field.


> It's these things which in parts make net80211 sometimes hard to figure
> out because there are three ways to express something at times and it's
> not clear at all which ones would be needed (especially if not yet
> used).
>

I/we should go add some documentation around these and try to get them
consistent.
I do agree that for the default cases we should have err, 'sensible
defaults' pop up so
drivers don't have to set /everything exactly correct/.


>
> I would really love us to clean this up before it's being used for
> anything to avoid further future work.
>
> There's a lot of "nice to have for reporting" but currently dead code there
> which seems to need a bit more tought...
>
> /bz
>
> --
> Bjoern A. Zeeb                                                     r15:7
>
>

Reply via email to