netdev - Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+mtBx9DWop_HYU9H4X-9nuFZw8epJhqy9FPArz6evSY0=9Ohg@mail.gmail.com>
Date:	Tue, 9 Aug 2011 11:28:43 -0700
From:	Tom Herbert <therbert@...gle.com>
To:	Stephen Hemminger <shemminger@...tta.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits

> Ok. then we need to fix all NAPI drivers at once, and get more
> drivers converted over to NAPI. It is really annoying how there
> is just too much variation among network drivers.

Agreed this variation has created a mess and makes it difficult to add
this sort of functionality.  But I'm not sure what the "fix" would be.

For instance...

It seems like there are three primary points points where drivers
interact with stack to stop or wakeup queue:

1) On start-xmit, availability is checked and queue is stopped if no
space available queue is stopped.  Some drivers report an error if
there was no availability and queue had not been stopped as expected.
2) At the end of start-xmit availability is checked and queue is
stopped if necessary.
3) At end of completion processing, availability is checked and queue
is waked up if necessary.

Each of these are several lines of code in each driver, and seem to be
begging for a unified abstraction.  For instance in tg3 instead of:

if (unlikely(netif_tx_queue_stopped(txq) &&
                     (tg3_tx_avail(tnapi) > TG3_TX_WAKEUP_THRESH(tnapi)))) {
                __netif_tx_lock(txq, smp_processor_id());
                if (netif_tx_queue_stopped(txq) &&
                    (tg3_tx_avail(tnapi) > TG3_TX_WAKEUP_THRESH(tnapi)))
                        netif_tx_wake_queue(txq);
                __netif_tx_unlock(txq);
        }

we probably would want something like:

netif_tx_queue_complete_check(txq, pkts, bytes, tg3_tx_avail(tnapi) >
TG3_TX_WAKEUP_THRESH(tnapi))

which would do all the work to check for available space, wakeup the
queue, statistic, bql, etc...

I actually did consider abstracting these out as part of bql, but even
though drivers implemented these in similar ways, variations in each
driver might make introducing a unified abstraction nontrivial (like
where drivers need to insert memory barriers and variations in
locking).
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html