On Sun, Jan 01, 2023 at 05:00:35PM +0000, Ali Farzanrad wrote:
> Hi tech@,
>
> Happy new year!
> I have some weird problems with my athn interface which drives me crazy.
>
> 1. Whenever I configure my athn as `media auto' for the first time it
> correctly detects correct media subclass, but as soon as I select exact
> same media subclass manually, it diverts to DS1 media subclass and after
> that auto will not work again (I need to reboot system or my WiFi
> provider aka my Android phone).
You are not supposed to force a specific Tx rate, unless you are
debugging a Tx-specific problem in the code. The driver will
adjust the Tx rate on demand, based on packet loss statistics.
If you force a specific Tx rate then associated devices will either see
a lot of packet loss (if the rate is too high and the device is too far
away) or very low speed (if the Tx rate is forced to a low rate).
> 2. Sometimes my athn interface will reset all of a sudden and after
> reset it diverts to DS1 media subclass again.
On 2Ghz channels the driver will start out in DS1 and adjust upwards,
and adjust the rate upwards if packet loss remains low while doing so.
> Anyway I decided to read athn related files, but I rarely understand the
> code. My primary suspect was bad dynamic allocation usage, and I found
> this:
>
> /usr/src/sys/dev/ic/ar5008.c:676: free(rxq->bf, M_DEVBUF, 0);
> /usr/src/sys/dev/ic/ar5008.c:677:
> /usr/src/sys/dev/ic/ar5008.c:678: /* Free Rx descriptors. */
>
> In this file almost after every free there is an explicit NULL
> assignment but why there is no NULL assignment after this line?
>
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/ic/ar5008.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 ar5008.c
> --- ar5008.c 27 Dec 2022 20:13:03 -0000 1.71
> +++ ar5008.c 1 Jan 2023 16:57:54 -0000
> @@ -674,6 +674,7 @@ ar5008_rx_free(struct athn_softc *sc)
> m_freem(bf->bf_m);
> }
> free(rxq->bf, M_DEVBUF, 0);
> + rxq->bf = NULL;
>
> /* Free Rx descriptors. */
> if (rxq->map != NULL) {
There is no reason to NULL out this pointer.
ar5008_rx_free() is called from either athn_detach() (via sc->ops.dma_free)
or if device attachment fails in athn_attach() (via sc->osp.dma_alloc).
In either case we're not going to be using this allocation ever again.
