if_start / if_transmit handling and packet ordering - how can I guarantee it?

[PseudoCylon]

> ----------------------------------------------------------------------
>
> Message: 1
> Date: Fri, 1 Jun 2012 00:46:02 -0700
> From: Adrian Chadd <adrian at freebsd.org>
> Subject: if_start / if_transmit handling and packet ordering - how can
>        I guarantee it?
> To: freebsd-net at freebsd.org
> Cc: freebsd-wireless at freebsd.org
> Message-ID:
>        <CAJ-Vmo=58aZcWJub8uej4xjddHsBLCOeRCZ1_MXAgiA4nQsG_g at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> Ok, so now that I've mostly tried to lucidly dump what's going on-
> what do people think about holding the locks for (potentially) so
> long? I know iwn(4) holds the driver lock for as long as it can for
> _everything_, so it avoids this issue. But again, I don't really like
> the idea of holding a lock for this long.

Nether do I. Basically, this is how things go with holding a lock.

if_start()
{
        LOCK();
        for (;;) {
                add_slot();
                if (++queue_counter > MAX)
                        break;
        }
        UNLOCK();
}

_txeof() or usb_bulk_callback()
{
        LOCK();
        clear_slot();
        queue_counter--;
        UNLOCK();
        if_start();
}

When if_start() is called first time, it will loop until slots get
full. This is guaranteed because both functions want the lock, so no
slot will be cleared until if_start() exits.
When _txeof() is called, it frees one slot. Then calls if_start().
After enqueuing one frame, slots get full again. Then, _txeof() frees
one slot, if_start() adds one ...
So, the driver processes one frame at a time. The packet order is
maintained, but it seems wasting memory for queue slots.

> Does anyone else have any
> other ideas?

Maybe...
If we guarantee only one thread/process runs the if_start(), we won't
have to hold the lock for that long. i.e

if_start()
{
        if (!atomic_cmpset(&running, 0, 1))
                return;

        for(;;) {
                if (full) {
                        running = 0;
                        break;
                }
        }
}

> FWIW - I temporarily converted the ath driver to make ath_start()
> enqueue a taskqueue task, which then did all of the TX inside the
> taskqueue.

I have tried the similar thing with run(4), because I thought calling
taskqueue_enqueue() is better than calling if_start() in
usb_bulk_callback(). (if_start() is a big process.) I got extra
bandwidth (forget the actual number).

> I unfortunately
> then become very, very susceptible to scheduling latency

I had to use a private taskqueue instead of shared one to over come
the latency. Other than that, it worked well, at least under 1 ap + 1
sta both use run(4) environment.


AK