From: Michael Breuer <mbreuer@majjas•com>
To: Stephen Hemminger <shemminger@vyatta•com>
Cc: Jarek Poplawski <jarkao2@gmail•com>,
David Miller <davem@davemloft•net>,
mikem@ring3k•org, flyboy@gmail•com, rjw@sisk•pl,
netdev@vger•kernel.org
Subject: Re: [PATCH] sky2: safer transmit ring cleaning (v4)
Date: Thu, 14 Jan 2010 18:51:11 -0500 [thread overview]
Message-ID: <4B4FADEF.6020408@majjas.com> (raw)
In-Reply-To: <20100114095254.5cf7faa0@nehalam>
On 1/14/2010 12:52 PM, Stephen Hemminger wrote:
> On Thu, 14 Jan 2010 10:14:45 +0000
> Jarek Poplawski<jarkao2@gmail•com> wrote:
>
>
>> This makes it safe, but it still resembles the "short term fix"
>> according do David's opinion.
>>
>> This change seems to affect dev->stats too. Since they are not
>> updated in sky2_tx_clean(). Btw, I hope "&" is some optimization
>> because it's less readable than "&&".
>>
> Stats don't matter for packets flushed during device reset.
>
> The& is because in the most common case device is up,
> and we don't want the additional conditional branch.
>
I've been looking at what might explain the dhcp stuff - as well as the
dropped packets only when there's an extra hop. I came across one path
that seems suspect - although I'm really not familiar with the network
stack code... that said, I'm wondering about neigh_compat_output (and
eth_rebuild_header and arp_find). If I'm following things correctly (or
perhaps mostly correctly), the only time anything goes this route (pun
intentional) is when the packet was routed to this box. I'm guessing
that bridging makes this more likely. So my dhcp stuff would all be
going through here, as would the smb stuff that seemed flaky. The race
I'm seeing (maybe) is that when the arp table is being rebuilt, there's
a possibility that arp_find frees the skb. There's some other locking
and stuff going on that seems maybe races with sky2.c in places on both
the rx and tx path. I *think* it's right from looking at it, but test
results suggest otherwise. Aside from the potential race, I think
there's also a corner case where neigh_compat_output can return either
with or without freeing the skb depending on the return from
dev_hard_header... this may also be part of the race.
Maybe I've missed something... but as far as I can see, this is just
about the only difference in code path taken between stuff that is
working and stuff that is occasionally not.
next prev parent reply other threads:[~2010-01-14 23:51 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-10 22:27 2.6.33-rc3-git3: Reported regressions from 2.6.32 Rafael J. Wysocki
2010-01-10 22:32 ` [Bug #14925] sky2 panic under load Rafael J. Wysocki
2010-01-11 0:36 ` Berck E. Nash
2010-01-11 13:26 ` Jarek Poplawski
2010-01-11 19:32 ` Rafael J. Wysocki
2010-01-11 20:31 ` Jarek Poplawski
2010-01-11 20:50 ` Rafael J. Wysocki
2010-01-11 21:02 ` Berck E. Nash
2010-01-11 21:47 ` Jarek Poplawski
2010-01-11 14:03 ` Mike McCormack
2010-01-11 16:45 ` Stephen Hemminger
2010-01-11 22:07 ` Jarek Poplawski
2010-01-12 0:14 ` David Miller
2010-01-12 7:50 ` Jarek Poplawski
2010-01-12 8:08 ` David Miller
2010-01-12 8:56 ` Jarek Poplawski
2010-01-12 9:42 ` David Miller
2010-01-12 10:31 ` Jarek Poplawski
2010-01-12 10:56 ` David Miller
2010-01-12 11:04 ` Jarek Poplawski
2010-01-12 15:39 ` Stephen Hemminger
2010-01-12 16:15 ` [PATCH] sky2: safer transmit ring cleaning Stephen Hemminger
2010-01-12 16:32 ` Michael Breuer
2010-01-12 17:02 ` Stephen Hemminger
2010-01-12 18:04 ` Jarek Poplawski
2010-01-12 18:13 ` Stephen Hemminger
2010-01-12 18:24 ` Jarek Poplawski
2010-01-12 18:49 ` [PATCH] sky2: safer transmit ring cleaning (v2) Stephen Hemminger
2010-01-12 19:16 ` Jarek Poplawski
2010-01-12 19:23 ` Stephen Hemminger
2010-01-12 19:50 ` Jarek Poplawski
2010-01-13 1:23 ` Stephen Hemminger
2010-01-12 19:34 ` Michael Breuer
2010-01-12 18:35 ` [PATCH] sky2: safer transmit ring cleaning Michael Breuer
2010-01-12 18:42 ` Michael Breuer
2010-01-12 20:31 ` Michael Breuer
2010-01-13 4:10 ` [PATCH] sky2: safer transmit ring cleaning (v3) Stephen Hemminger
2010-01-13 4:31 ` Michael Breuer
2010-01-13 7:35 ` Jarek Poplawski
2010-01-13 16:04 ` Michael Breuer
2010-01-14 3:41 ` [PATCH] sky2: safer transmit ring cleaning (v4) Stephen Hemminger
2010-01-14 10:14 ` Jarek Poplawski
2010-01-14 11:16 ` Jarek Poplawski
2010-01-14 11:20 ` David Miller
2010-01-14 11:26 ` Jarek Poplawski
2010-01-14 13:19 ` Mike McCormack
2010-01-14 15:43 ` Michael Breuer
2010-01-14 16:46 ` Michael Breuer
2010-01-14 17:51 ` Stephen Hemminger
2010-01-14 17:52 ` Stephen Hemminger
2010-01-14 23:51 ` Michael Breuer [this message]
2010-01-16 18:35 ` sky2 DHCPOFFER packet loss under load (Was Re: [PATCH] sky2: safer transmit ring cleaning (v4)) Michael Breuer
2010-01-14 15:46 ` [PATCH] sky2: safer transmit ring cleaning (v4) Michael Breuer
2010-01-11 22:31 ` [Bug #14925] sky2 panic under load Jarek Poplawski
2010-01-11 16:00 ` 2.6.33-rc3-git3: Reported regressions from 2.6.32 Luis R. Rodriguez
2010-01-11 21:47 ` Nick Bowler
2010-01-11 22:10 ` Rafael J. Wysocki
[not found] ` <omZ8aBZReyD.A.rz.COlSLB@chimera>
2010-01-28 23:18 ` [PATCH net-2.6] cdc_ether: Partially revert "usbnet: Set link down initially ..." Ben Hutchings
2010-01-29 5:37 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B4FADEF.6020408@majjas.com \
--to=mbreuer@majjas$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=flyboy@gmail$(echo .)com \
--cc=jarkao2@gmail$(echo .)com \
--cc=mikem@ring3k$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rjw@sisk$(echo .)pl \
--cc=shemminger@vyatta$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox