netdev - Re: [PATCH 2/9] pasemi_mac: Clean TX ring in poll

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] [day] [month] [year] [list]

Message-ID: <20070703203131.GA26181@lixom.net>
Date:	Tue, 3 Jul 2007 15:31:31 -0500
From:	olof@...om.net (Olof Johansson)
To:	Jeff Garzik <jgarzik@...ox.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 2/9] pasemi_mac: Clean TX ring in poll

On Mon, Jul 02, 2007 at 08:34:22AM -0400, Jeff Garzik wrote:
> >Index: netdev-2.6/drivers/net/pasemi_mac.c
> >===================================================================
> >--- netdev-2.6.orig/drivers/net/pasemi_mac.c
> >+++ netdev-2.6/drivers/net/pasemi_mac.c
> >@@ -1052,6 +1052,7 @@ static int pasemi_mac_poll(struct net_de
> > 	int pkts, limit = min(*budget, dev->quota);
> > 	struct pasemi_mac *mac = netdev_priv(dev);
> > 
> >+	pasemi_mac_clean_tx(mac);
> > 	pkts = pasemi_mac_clean_rx(mac, limit);
> 
> Why is this needed?  Is your TX interrupt mitigated or delayed somehow?

On some level hardware, we only have the possibility of taking the
TX interrupt after a certain number of packets, and no way of doing a
(hw) timer-based one (i.e.  after no packets have been sent for x ms,
interrupt). Taking an interrupt for every TX packet isn't an interesting
proposal, and doing it solely based on count > 1 can leave stale packets
there for a long time when the network is otherwise idle.

So, I would need to do a software based timer. The poll function seemed like
a good enough solution for now though, since it's called periodically even
when the NAPI mode is such that interrupts are enabled.

> In general, drivers should only clean TX in one place.  Doing so in 
> multiple places tends to indicate a bug somewhere.

Yes, I wish I could just do it in the interrupt handler. I could do it
with a dedicated timer if you prefer, but I'd still need to have two
ways of entering the tx clean function.

-Olof
-
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