netdev - Re: [PATCH] ucc_geth: Move freeing of TX packets to NAPI context.

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: <OF6D5FFE1D.1843C7FE-ONC1257589.00328D91-C1257589.00336B8B@transmode.se>
Date:	Mon, 30 Mar 2009 11:21:39 +0200
From:	Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>
To:	Li Yang <leoli@...escale.com>
Cc:	avorontsov@...mvista.com,
	linuxppc-dev Development <linuxppc-dev@...abs.org>,
	Netdev <netdev@...r.kernel.org>, pku.leo@...il.com
Subject: Re: [PATCH] ucc_geth: Move freeing of TX packets to NAPI context.

pku.leo@...il.com wrote on 30/03/2009 10:34:47:
> 
> On Thu, Mar 26, 2009 at 1:51 AM, Joakim Tjernlund
> <Joakim.Tjernlund@...nsmode.se> wrote:
> > Anton Vorontsov <avorontsov@...mvista.com> wrote on 25/03/2009 
15:25:40:
> >> On Wed, Mar 25, 2009 at 02:30:49PM +0100, Joakim Tjernlund wrote:
> >> > >>From 1c2f23b1f37f4818c0fd0217b93eb38ab6564840 Mon Sep 17 00:00:00
> > 2001
> >> > From: Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>
> >> > Date: Tue, 24 Mar 2009 10:19:27 +0100
> >> > Subject: [PATCH] ucc_geth: Move freeing of TX packets to NAPI 
context.
> >> >  Also increase NAPI weight somewhat.
> >> >  This will make the system alot more responsive while
> >> >  ping flooding the ucc_geth ethernet interaface.
> >>
> >> Some time ago I've tried a similar thing for this driver, but during
> >> tcp (or udp I don't quite remember) netperf tests I was getting tx
> >> watchdog timeouts after ~2-5 minutes of work. I was testing with a
> >> gigabit and 100 Mbit link, with 100 Mbit link the issue was not
> >> reproducible.
> >>
> >> Though, I recalling I was doing a bit more than your patch: I was
> >> also clearing the TX events in the ucce register before calling
> >> ucc_geth_tx, that way I was trying to avoid stale interrupts. That
> >> helped to increase an overall performance (not only responsiveness),
> >> but as I said my approach didn't pass the tests.
> >>
> >> I don't really think that your patch may cause this, but can you
> >> try netperf w/ this patch applied anyway? And see if it really
> >> doesn't cause any issues under stress?
> >
> > Does the line(in ucc_geth_tx()) look OK to you:
> >        if ((bd == ugeth->txBd[txQ]) && (netif_queue_stopped(dev) == 
0))
> >                        break;
> >
> > Sure does look fishy to me.
> 
> There are two cases when txBd=ConfBd: the BD ring is full or empty.
> The condition used here ensures that it is the empty case.  Because in
> hard_start_xmit, the queue will be stopped when the BD ring is full.
> Maybe some comment is needed here.

But how do you know that the queue hasn't been stopped by someone else 
than
the driver? 
If it is stopped by higher layers, the if stmt will fail.

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