* Re: [Fastboot] Re: Re: tg3: issue for reboot/kexec
From: Eric W. Biederman @ 2005-06-30 23:52 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, fastboot, linux-pm
In-Reply-To: <20050630233354.GA22287@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 2031 bytes --]
Greg KH <greg@kroah.com> writes:
> On Thu, Jun 30, 2005 at 05:21:45PM -0600, Eric W. Biederman wrote:
>> However I have gotten feedback a couple of times that
>> driver writers tend to prefer using reboot notifiers. In part
>> because shutdown functions don't exist for non-pci devices.
>
> That's a very lame excuse. All busses should have shutdown functions.
> And any device that is just bypassing all of the existing bus logic is
> still tying into the driver core directly (which is a bad thing by
> itself, but that's a different matter.) And there's a shutdown method
> there too.
>
> So there is no excuse to not use it. Please, if they complain, point
> them to me :)
Ok.
Then there is still my complaint and device_shutdown doesn't get called
on module removal which means it really doesn't get implemented. Perhaps
with kexec now being in the mainline kernel this will get better.
Currently I have the following patch outstanding against the e1000
driver because on reboot on some boxes it card revisions
it places the card into a sleep state the driver initialization
routing cannot get the card out of.
And yes the e1000 is bad and is using a reboot_notifier.
Eric
e1000_main.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
diff -uNr linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c
--- linux-2.4.29-kexec-apic-virtwire-on-shutdownx86_64/drivers/net/e1000/e1000_main.c Tue Feb 15 14:17:09 2005
+++ linux-2.4.29-e1000-no-poweroff-on-reboot/drivers/net/e1000/e1000_main.c Wed Feb 16 05:49:00 2005
@@ -2777,7 +2777,7 @@
case SYS_POWER_OFF:
while((pdev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, pdev))) {
if(pci_dev_driver(pdev) == &e1000_driver)
- e1000_suspend(pdev, 3);
+ e1000_suspend(pdev, (event == SYS_DOWN)?0:3);
}
}
return NOTIFY_DONE;
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [Fastboot] Re: Re: tg3: issue for reboot/kexec
From: Greg KH @ 2005-06-30 23:33 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev, fastboot, linux-pm
In-Reply-To: <m18y0rl9bq.fsf@ebiederm.dsl.xmission.com>
[-- Attachment #1: Type: text/plain, Size: 646 bytes --]
On Thu, Jun 30, 2005 at 05:21:45PM -0600, Eric W. Biederman wrote:
> However I have gotten feedback a couple of times that
> driver writers tend to prefer using reboot notifiers. In part
> because shutdown functions don't exist for non-pci devices.
That's a very lame excuse. All busses should have shutdown functions.
And any device that is just bypassing all of the existing bus logic is
still tying into the driver core directly (which is a bad thing by
itself, but that's a different matter.) And there's a shutdown method
there too.
So there is no excuse to not use it. Please, if they complain, point
them to me :)
thanks,
greg k-h
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [Fastboot] Re: Re: tg3: issue for reboot/kexec
From: Eric W. Biederman @ 2005-06-30 23:21 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, fastboot, linux-pm
In-Reply-To: <20050625033328.GC3934@kroah.com>
[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]
Greg KH <greg@kroah.com> writes:
> On Fri, Jun 24, 2005 at 05:46:52PM -0700, randy_dunlap wrote:
>> On Fri, 24 Jun 2005 17:02:17 -0700 Haren Myneni wrote:
>>
>> |
>> | Hello,
>> | The kexec boot on power blade (JS20) is not successfull without
>> | doing 'ifdown <ethernet device>'. Based on my initial look in tg3 code,
>> | the driver does not have shutdown nor reboot notifier code unlike in
>> | other drivers. Hence, I added the the following patch (based on e1000)
>> | and it is working. Is it OK to include this patch? If not, any help to
>> | track down the issue would be appreciated.
>> |
>> | My system is having the following cards:
>> | 0000:11:01.0 Ethernet controller: Broadcom Corporation NetXtreme
>> | BCM5704S Gigabit Ethernet (rev 03)
>> | 0000:11:01.1 Ethernet controller: Broadcom Corporation NetXtreme
>> | BCM5704S Gigabit Ethernet (rev 03)
>> |
>> | lspci -n -s 0000:11:01
>> | 0000:11:01.0 Class 0200: 14e4:16a8 (rev 03)
>> | 0000:11:01.1 Class 0200: 14e4:16a8 (rev 03)
>>
>> [adding fastboot & linux-pm mailing lists]
>>
>> There was just a SCSI driver patch that tried to use a reboot
>> notifier on shutdown and the patch got this comment:
>>
>> Subject: Re: [PATCH] 2.6 aacraid: shutdown notification
>> Message-ID: <42BC6D6F.5090402@us.ibm.com>
>> From: Mark Haverkamp <markh@osdl.org>
>>
>> and reply From: Brian King <brking@us.ibm.com>:
>> | The pci_driver->shutdown method should be used instead of registering
>> | a reboot notifier.
>>
>> so is there a good reason that network drivers should use the
>> reboot notifier instead of pci_driver->shutdown,
>> or should we be converting drivers to use pci_driver->shutdown
>> instead?
>> My quick look favors pci_driver->shutdown.
A couple stray bits of feedback. Downing the interface is generally
a user space requirement like unmounting filesystems.
/sbin/kexec and /sbin/reboot both perform this operation so I don't
see how this bug was hit.
I am wondering here if the problem was on on going DMA transaction
or something else. If the problem was not an on going DMA transaction
then this problem is going to crop back up when people start
messing with the kexec on panic, because it indicates the
driver initialization is not robust.
> Yes, don't add a reboot notifier, use the shutdown function instead.
Sounds right to me.
However I have gotten feedback a couple of times that
driver writers tend to prefer using reboot notifiers. In part
because shutdown functions don't exist for non-pci devices.
Eric
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* はじめまして。
From: 今井玲子 @ 2005-06-30 23:15 UTC (permalink / raw)
To: netdev
こんばんわ♪
はずかしくってちょっと書きづらいんだけど、
すごいＨ☆なサイトをがあるの知ってますか？
有名な人がいっぱいで隠し撮りやいろんな種類の動画が盛りだくさん♪
芸能人のもあるから驚きだよ！
とりあえずサンプル見てみよ☆
http://the-premium-movie.com
^ permalink raw reply
* Re: -mm -> 2.6.13 merge status (dropped patches?)
From: Andrew Morton @ 2005-06-30 23:14 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: netdev, davem, jgarzik, linux-kernel
In-Reply-To: <200506301605_MC3-1-A320-D9D6@compuserve.com>
Chuck Ebbert <76306.1226@compuserve.com> wrote:
>
> On Mon, 20 Jun 2005 at 23:54:58 -0700, Andrew Morton wrote:
>
> > This summarises my current thinking on various patches which are presently
> > in -mm. I cover large things and small-but-controversial things. Anything
> > which isn't covered here (and that's a lot of material) is probably a "will
> > merge", unless it obviously isn't.
>
> What happened to:
>
> remove-last_rx-update-from-loopback-device.patch
Davem rejected it. hm, I can't find the email trail - it was cc'ed to
netdev@oss.sgi.com mid-March, but nothing seems to have been sent back.
> It was dropped in 2.6.12-rc1-mm4 with a status of "merged" but it's not
> in either 2.6.12 or 2.6.13-rc1.
>
> And in general, who is tracking whether things are being lost?
Things don't get lost. I have a habit of misremembering the disposition of
patches when preparing -mm release summaries.
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Thomas Graf @ 2005-06-30 22:08 UTC (permalink / raw)
To: David S. Miller; +Cc: laforge, netdev, netfilter-devel, hadi, pablo, kaber
In-Reply-To: <20050630.144942.74731532.davem@davemloft.net>
* David S. Miller <20050630.144942.74731532.davem@davemloft.net> 2005-06-30 14:49
> From: Thomas Graf <tgraf@suug.ch>
> Date: Thu, 30 Jun 2005 23:34:59 +0200
>
> > This is only true if one attribute is added. What I really meant
> > is something like em_text_dump() in em_text.c.
>
> Speaking of em_text.c, when one loads ematch rules into that
> module it does a printk(). I assume that's a thinko and should
> be removed?
Yes, it's a debugging leftover but I'll keep it in until rc3 so if
problems appear I have some debugging information. Given that is acceptable.
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: David S. Miller @ 2005-06-30 22:08 UTC (permalink / raw)
To: tgraf; +Cc: laforge, netdev, netfilter-devel, hadi, pablo, kaber
In-Reply-To: <20050630220814.GP16076@postel.suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Fri, 1 Jul 2005 00:08:14 +0200
> Yes, it's a debugging leftover but I'll keep it in until rc3 so if
> problems appear I have some debugging information. Given that is acceptable.
No problem.
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: David S. Miller @ 2005-06-30 21:49 UTC (permalink / raw)
To: tgraf; +Cc: laforge, netdev, netfilter-devel, hadi, pablo, kaber
In-Reply-To: <20050630213459.GO16076@postel.suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Thu, 30 Jun 2005 23:34:59 +0200
> This is only true if one attribute is added. What I really meant
> is something like em_text_dump() in em_text.c.
Speaking of em_text.c, when one loads ematch rules into that
module it does a printk(). I assume that's a thinko and should
be removed?
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Thomas Graf @ 2005-06-30 21:34 UTC (permalink / raw)
To: Patrick McHardy
Cc: Harald Welte, netdev, Jamal Hadi Salim,
Netfilter Development Mailinglist, Pablo Neira
In-Reply-To: <42C46374.5000407@trash.net>
* Patrick McHardy <42C46374.5000407@trash.net> 2005-06-30 23:26
> Thomas Graf wrote:
> > * Patrick McHardy <42C3F35B.50805@trash.net> 2005-06-30 15:27
> >
> >>I think for nested attributes error handling should happen on the
> >>outer level. Just trimming on the inner level would leave a
> >>half-finished nested attribute.
> >
> > I can agree with this, the question that remains is: do we want
> > to trim in functions where no nesting is done at all? i.e. things
> > like the generic network statistics dumping interface.
>
> Trimming isn't required in this case since the length is know and
> checked in advance. An error should still be propagated back of
> course so potential outer levels can do trimming.
This is only true if one attribute is added. What I really meant
is something like em_text_dump() in em_text.c. Assuming we rework
areas like this one in order to avoid structs on the stack, we'll
have a lot of such cases where we might hit the limit while not
really knowing about the start of the nested tlv.
Anyways, I think this is a minor issue and we can safely ignore it
and delegate the trimming responsibility to the underlying layer
which "started" the nested tlv.
The gnet_stats interface was a bad example since it doesn't have
such a case.
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Patrick McHardy @ 2005-06-30 21:26 UTC (permalink / raw)
To: Thomas Graf
Cc: Harald Welte, netdev, Jamal Hadi Salim,
Netfilter Development Mailinglist, Pablo Neira
In-Reply-To: <20050630180238.GM16076@postel.suug.ch>
Thomas Graf wrote:
> * Patrick McHardy <42C3F35B.50805@trash.net> 2005-06-30 15:27
>
>>I think for nested attributes error handling should happen on the
>>outer level. Just trimming on the inner level would leave a
>>half-finished nested attribute.
>
> I can agree with this, the question that remains is: do we want
> to trim in functions where no nesting is done at all? i.e. things
> like the generic network statistics dumping interface.
Trimming isn't required in this case since the length is know and
checked in advance. An error should still be propagated back of
course so potential outer levels can do trimming.
Regards
Patrick
^ permalink raw reply
* Re: -mm -> 2.6.13 merge status (dropped patches?)
From: Chuck Ebbert @ 2005-06-30 20:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-netdev, David S. Miller, Jeff Garzik, linux-kernel
On Mon, 20 Jun 2005 at 23:54:58 -0700, Andrew Morton wrote:
> This summarises my current thinking on various patches which are presently
> in -mm. I cover large things and small-but-controversial things. Anything
> which isn't covered here (and that's a lot of material) is probably a "will
> merge", unless it obviously isn't.
What happened to:
remove-last_rx-update-from-loopback-device.patch
It was dropped in 2.6.12-rc1-mm4 with a status of "merged" but it's not
in either 2.6.12 or 2.6.13-rc1.
And in general, who is tracking whether things are being lost?
--
Chuck
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Thomas Graf @ 2005-06-30 18:02 UTC (permalink / raw)
To: Patrick McHardy
Cc: Harald Welte, netdev, Jamal Hadi Salim,
Netfilter Development Mailinglist, Pablo Neira
In-Reply-To: <42C3F35B.50805@trash.net>
* Patrick McHardy <42C3F35B.50805@trash.net> 2005-06-30 15:27
> Thomas Graf wrote:
> > Good point, after all, I think structs have been used too often
> > and we now suffer from backwards compatbility issues. The
> > points likely to be argueable are things like trim responsibility
> > in the error handling, i.e. delegate it down to the function
> > which also created the header or trim on every level. Other than
> > that the whole thing should be pretty straight forward.
>
> I think for nested attributes error handling should happen on the
> outer level. Just trimming on the inner level would leave a
> half-finished nested attribute.
I can agree with this, the question that remains is: do we want
to trim in functions where no nesting is done at all? i.e. things
like the generic network statistics dumping interface.
^ permalink raw reply
* こりゃひどい！！
From: 妹とSEX @ 2005-06-30 16:00 UTC (permalink / raw)
To: netdev
中出ししない約束での妹とのSEX
あまりの気持ちよさに、抜く前に暴発してしまい、
気持ちよがっていた妹がいきなり豹変し、
兄を蹴り回してます。
http://keepintouchhoney.com/mc/?88020190458024240
^ permalink raw reply
* Re: [RFC/PATCH 0/12] Updates & bug fixes for iseries_veth network driver
From: Jeff Garzik @ 2005-06-30 14:41 UTC (permalink / raw)
To: michael; +Cc: PPC64-dev, netdev, LKML
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
Michael Ellerman wrote:
> Hi y'all,
>
> The following is a series of patches for the iseries_veth driver.
>
> They're not ready for merging yet, as we need to do more extensive testing.
> However any feedback you have will be greatly appreciated.
Note, make sure to CC me, and also the new netdev list
(netdev@vger.kernel.org).
Jeff
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Patrick McHardy @ 2005-06-30 13:27 UTC (permalink / raw)
To: Thomas Graf
Cc: Harald Welte, netdev, Jamal Hadi Salim,
Netfilter Development Mailinglist, Pablo Neira
In-Reply-To: <20050630120302.GI16076@postel.suug.ch>
Thomas Graf wrote:
> Good point, after all, I think structs have been used too often
> and we now suffer from backwards compatbility issues. The
> points likely to be argueable are things like trim responsibility
> in the error handling, i.e. delegate it down to the function
> which also created the header or trim on every level. Other than
> that the whole thing should be pretty straight forward.
I think for nested attributes error handling should happen on the
outer level. Just trimming on the inner level would leave a
half-finished nested attribute.
> What do you think about naming it nlattr?
Sounds good.
Regards
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] updates for [nf|ct]netlink and event API
From: Thomas Graf @ 2005-06-30 12:03 UTC (permalink / raw)
To: Patrick McHardy
Cc: Harald Welte, netdev, Jamal Hadi Salim,
Netfilter Development Mailinglist, Pablo Neira
In-Reply-To: <42C350A1.1030602@trash.net>
* Patrick McHardy <42C350A1.1030602@trash.net> 2005-06-30 03:53
> Thomas Graf wrote:
> > At some point we should introduce a "generic" attribute
> > architecture for all netlink families. The connector
> > stuff will need it as well, so maybe the time has come
> > to actually do it.
>
> I agree, most of the macros are just copied without modification
> anyway. One more thing I would like to change is the excessive
> use of RTA_PUT with structures on the stack. Using __RTA_PUT
> and putting together these structures in-place would be much
> nicer. nfnetlink has copied this part of rtnetlink.
Good point, after all, I think structs have been used too often
and we now suffer from backwards compatbility issues. The
points likely to be argueable are things like trim responsibility
in the error handling, i.e. delegate it down to the function
which also created the header or trim on every level. Other than
that the whole thing should be pretty straight forward.
What do you think about naming it nlattr?
^ permalink raw reply
* [PATCH 11/12] iseries_veth: Add a per-connection ack timer
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
Currently the iseries_veth driver contravenes the specification in
Documentation/networking/driver.txt, in that if packets are not acked by
the other LPAR they will sit around forever.
This patch adds a per-connection timer which fires if we've had no acks for
five seconds. This is superior to the generic TX timer because it catches
the case of a small number of packets being sent and never acked.
---
drivers/net/iseries_veth.c | 75 +++++++++++++++++++++++++++++++++++++++++----
1 files changed, 69 insertions(+), 6 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -132,6 +132,11 @@ struct veth_lpar_connection {
struct kref refcount;
struct timer_list ack_timer;
+ struct timer_list reset_timer;
+ unsigned int reset_timeout;
+ unsigned long last_contact;
+ int outstanding_tx;
+
spinlock_t lock;
unsigned long state;
HvLpInstanceId src_inst;
@@ -171,7 +176,8 @@ static int veth_start_xmit(struct sk_buf
static void veth_recycle_msg(struct veth_lpar_connection *, struct veth_msg *);
static void veth_flush_pending(struct veth_lpar_connection *cnx);
static void veth_receive(struct veth_lpar_connection *, struct VethLpEvent *);
-static void veth_timed_ack(unsigned long connectionPtr);
+static void veth_timed_ack(unsigned long ptr);
+static void veth_timed_reset(unsigned long ptr);
/*
* Utility functions
@@ -353,7 +359,7 @@ static void veth_handle_int(struct VethL
HvLpIndex rlp = event->base_event.xSourceLp;
struct veth_lpar_connection *cnx = veth_cnx[rlp];
unsigned long flags;
- int i;
+ int i, acked = 0;
BUG_ON(! cnx);
@@ -367,13 +373,22 @@ static void veth_handle_int(struct VethL
break;
case VethEventTypeFramesAck:
spin_lock_irqsave(&cnx->lock, flags);
+
for (i = 0; i < VETH_MAX_ACKS_PER_MSG; ++i) {
u16 msgnum = event->u.frames_ack_data.token[i];
- if (msgnum < VETH_NUMBUFFERS)
+ if (msgnum < VETH_NUMBUFFERS) {
veth_recycle_msg(cnx, cnx->msgs + msgnum);
+ cnx->outstanding_tx--;
+ acked++;
+ }
}
+
+ if (acked > 0)
+ cnx->last_contact = jiffies;
+
spin_unlock_irqrestore(&cnx->lock, flags);
+
veth_flush_pending(cnx);
break;
case VethEventTypeFrames:
@@ -447,8 +462,6 @@ static void veth_statemachine(void *p)
restart:
if (cnx->state & VETH_STATE_RESET) {
- int i;
-
if (cnx->state & VETH_STATE_OPEN)
HvCallEvent_closeLpEventPath(cnx->remote_lp,
HvLpEvent_Type_VirtualLan);
@@ -467,15 +480,20 @@ static void veth_statemachine(void *p)
| VETH_STATE_SENTCAPACK | VETH_STATE_READY);
/* Clean up any leftover messages */
- if (cnx->msgs)
+ if (cnx->msgs) {
+ int i;
for (i = 0; i < VETH_NUMBUFFERS; ++i)
veth_recycle_msg(cnx, cnx->msgs + i);
+ }
+ cnx->outstanding_tx = 0;
/* Drop the lock so we can do stuff that might sleep or
* take other locks. */
spin_unlock_irq(&cnx->lock);
del_timer_sync(&cnx->ack_timer);
+ del_timer_sync(&cnx->reset_timer);
+
veth_flush_pending(cnx);
spin_lock_irq(&cnx->lock);
@@ -628,9 +646,16 @@ static int veth_init_connection(u8 rlp)
cnx->remote_lp = rlp;
spin_lock_init(&cnx->lock);
INIT_WORK(&cnx->statemachine_wq, veth_statemachine, cnx);
+
init_timer(&cnx->ack_timer);
cnx->ack_timer.function = veth_timed_ack;
cnx->ack_timer.data = (unsigned long) cnx;
+
+ init_timer(&cnx->reset_timer);
+ cnx->reset_timer.function = veth_timed_reset;
+ cnx->reset_timer.data = (unsigned long) cnx;
+ cnx->reset_timeout = 5 * HZ * (VETH_ACKTIMEOUT / 1000000);
+
memset(&cnx->pending_acks, 0xff, sizeof (cnx->pending_acks));
veth_cnx[rlp] = cnx;
@@ -937,6 +962,13 @@ static int veth_transmit_to_one(struct s
if (rc != HvLpEvent_Rc_Good)
goto recycle_and_drop;
+ /* If the timer's not already running, start it now. */
+ if (0 == cnx->outstanding_tx)
+ mod_timer(&cnx->reset_timer, jiffies + cnx->reset_timeout);
+
+ cnx->last_contact = jiffies;
+ cnx->outstanding_tx++;
+
spin_unlock_irqrestore(&cnx->lock, flags);
return 0;
@@ -1081,6 +1113,37 @@ static void veth_flush_pending(struct ve
}
}
+static void veth_timed_reset(unsigned long ptr)
+{
+ struct veth_lpar_connection *cnx = (struct veth_lpar_connection *)ptr;
+ unsigned long trigger_time, flags;
+
+ /* FIXME is it possible this fires after veth_stop_connection()?
+ * That would reschedule the statemachine for 5 seconds and probably
+ * execute it after the module's been unloaded. Hmm. */
+
+ spin_lock_irqsave(&cnx->lock, flags);
+
+ if (cnx->outstanding_tx > 0) {
+ trigger_time = cnx->last_contact + cnx->reset_timeout;
+
+ if (trigger_time < jiffies) {
+ cnx->state |= VETH_STATE_RESET;
+ veth_kick_statemachine(cnx);
+ veth_error("%d packets not acked by LPAR %d within %d "
+ "seconds, resetting.\n",
+ cnx->outstanding_tx, cnx->remote_lp,
+ cnx->reset_timeout / HZ);
+ } else {
+ /* Reschedule the timer */
+ trigger_time = jiffies + cnx->reset_timeout;
+ mod_timer(&cnx->reset_timer, trigger_time);
+ }
+ }
+
+ spin_unlock_irqrestore(&cnx->lock, flags);
+}
+
/*
* Rx path
*/
^ permalink raw reply
* [PATCH 12/12] iseries_veth: Simplify full-queue handling
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
The iseries_veth driver may have multiple netdevices sending packets over
a single connection to another LPAR. If the bandwidth to the other LPAR is
exceeded all the netdevices must have their queue's stopped.
The current code achieves this by queueing one incoming skb on the
per-netdevice port structure. When the connection is able to send more packets
it flushes the queued packet for all netdevices and restarts their queues.
This arrangement makes less sense now that we have per-connection TX timers,
rather than the per-netdevice generic TX timer.
The new code simply detects when one of the connections is full, and stops
the queue of all associated netdevices. Then when a packet is acked on that
connection (ie. there is space again) all the queues are woken up.
---
drivers/net/iseries_veth.c | 108 ++++++++++++++++++++++++++-------------------
1 files changed, 64 insertions(+), 44 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -158,10 +158,11 @@ struct veth_port {
u64 mac_addr;
HvLpIndexMap lpar_map;
- spinlock_t pending_gate;
- struct sk_buff *pending_skb;
- HvLpIndexMap pending_lpmask;
+ /* queue_lock protects the stopped_map and dev's queue. */
+ spinlock_t queue_lock;
+ HvLpIndexMap stopped_map;
+ /* mcast_gate protects promiscuous, num_mcast & mcast_addr. */
rwlock_t mcast_gate;
int promiscuous;
int num_mcast;
@@ -174,7 +175,8 @@ static struct net_device *veth_dev[HVMAX
static int veth_start_xmit(struct sk_buff *skb, struct net_device *dev);
static void veth_recycle_msg(struct veth_lpar_connection *, struct veth_msg *);
-static void veth_flush_pending(struct veth_lpar_connection *cnx);
+static void veth_wake_queues(struct veth_lpar_connection *cnx);
+static void veth_stop_queues(struct veth_lpar_connection *cnx);
static void veth_receive(struct veth_lpar_connection *, struct VethLpEvent *);
static void veth_timed_ack(unsigned long ptr);
static void veth_timed_reset(unsigned long ptr);
@@ -216,6 +218,12 @@ static inline struct veth_msg *veth_stac
return msg;
}
+/* You must hold the connection's lock when you call this function. */
+static inline int veth_stack_is_empty(struct veth_lpar_connection *cnx)
+{
+ return cnx->msg_stack_head == NULL;
+}
+
static inline HvLpEvent_Rc
veth_signalevent(struct veth_lpar_connection *cnx, u16 subtype,
HvLpEvent_AckInd ackind, HvLpEvent_AckType acktype,
@@ -384,12 +392,12 @@ static void veth_handle_int(struct VethL
}
}
- if (acked > 0)
+ if (acked > 0) {
cnx->last_contact = jiffies;
+ veth_wake_queues(cnx);
+ }
spin_unlock_irqrestore(&cnx->lock, flags);
-
- veth_flush_pending(cnx);
break;
case VethEventTypeFrames:
veth_receive(cnx, event);
@@ -485,7 +493,9 @@ static void veth_statemachine(void *p)
for (i = 0; i < VETH_NUMBUFFERS; ++i)
veth_recycle_msg(cnx, cnx->msgs + i);
}
+
cnx->outstanding_tx = 0;
+ veth_wake_queues(cnx);
/* Drop the lock so we can do stuff that might sleep or
* take other locks. */
@@ -494,8 +504,6 @@ static void veth_statemachine(void *p)
del_timer_sync(&cnx->ack_timer);
del_timer_sync(&cnx->reset_timer);
- veth_flush_pending(cnx);
-
spin_lock_irq(&cnx->lock);
if (cnx->state & VETH_STATE_RESET)
@@ -852,8 +860,9 @@ static struct net_device * __init veth_p
port = (struct veth_port *) dev->priv;
- spin_lock_init(&port->pending_gate);
+ spin_lock_init(&port->queue_lock);
rwlock_init(&port->mcast_gate);
+ port->stopped_map = 0;
for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
HvLpVirtualLanIndexMap map;
@@ -969,6 +978,9 @@ static int veth_transmit_to_one(struct s
cnx->last_contact = jiffies;
cnx->outstanding_tx++;
+ if (veth_stack_is_empty(cnx))
+ veth_stop_queues(cnx);
+
spin_unlock_irqrestore(&cnx->lock, flags);
return 0;
@@ -1012,7 +1024,6 @@ static int veth_start_xmit(struct sk_buf
{
unsigned char *frame = skb->data;
struct veth_port *port = (struct veth_port *) dev->priv;
- unsigned long flags;
HvLpIndexMap lpmask;
if (! (frame[0] & 0x01)) {
@@ -1029,27 +1040,9 @@ static int veth_start_xmit(struct sk_buf
lpmask = port->lpar_map;
}
- spin_lock_irqsave(&port->pending_gate, flags);
-
- lpmask = veth_transmit_to_many(skb, lpmask, dev);
+ veth_transmit_to_many(skb, lpmask, dev);
- if (! lpmask) {
- dev_kfree_skb(skb);
- } else {
- if (port->pending_skb) {
- veth_error("%s: TX while skb was pending!\n",
- dev->name);
- dev_kfree_skb(skb);
- spin_unlock_irqrestore(&port->pending_gate, flags);
- return 1;
- }
-
- port->pending_skb = skb;
- port->pending_lpmask = lpmask;
- netif_stop_queue(dev);
- }
-
- spin_unlock_irqrestore(&port->pending_gate, flags);
+ dev_kfree_skb(skb);
return 0;
}
@@ -1081,9 +1074,10 @@ static void veth_recycle_msg(struct veth
}
}
-static void veth_flush_pending(struct veth_lpar_connection *cnx)
+static void veth_wake_queues(struct veth_lpar_connection *cnx)
{
int i;
+
for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++) {
struct net_device *dev = veth_dev[i];
struct veth_port *port;
@@ -1097,19 +1091,45 @@ static void veth_flush_pending(struct ve
if (! (port->lpar_map & (1<<cnx->remote_lp)))
continue;
- spin_lock_irqsave(&port->pending_gate, flags);
- if (port->pending_skb) {
- port->pending_lpmask =
- veth_transmit_to_many(port->pending_skb,
- port->pending_lpmask,
- dev);
- if (! port->pending_lpmask) {
- dev_kfree_skb_any(port->pending_skb);
- port->pending_skb = NULL;
- netif_wake_queue(dev);
- }
+ spin_lock_irqsave(&port->queue_lock, flags);
+
+ port->stopped_map &= ~(1 << cnx->remote_lp);
+
+ if (0 == port->stopped_map && netif_queue_stopped(dev)) {
+ veth_debug("cnx %d: woke queue for %s.\n",
+ cnx->remote_lp, dev->name);
+ netif_wake_queue(dev);
}
- spin_unlock_irqrestore(&port->pending_gate, flags);
+ spin_unlock_irqrestore(&port->queue_lock, flags);
+ }
+}
+
+static void veth_stop_queues(struct veth_lpar_connection *cnx)
+{
+ int i;
+
+ for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++) {
+ struct net_device *dev = veth_dev[i];
+ struct veth_port *port;
+
+ if (! dev)
+ continue;
+
+ port = (struct veth_port *)dev->priv;
+
+ /* If this cnx is not on the vlan for this port, continue */
+ if (! (port->lpar_map & (1 << cnx->remote_lp)))
+ continue;
+
+ spin_lock(&port->queue_lock);
+
+ netif_stop_queue(dev);
+ port->stopped_map |= (1 << cnx->remote_lp);
+
+ veth_debug("cnx %d: stopped queue for %s, map = 0x%x.\n",
+ cnx->remote_lp, dev->name, port->stopped_map);
+
+ spin_unlock(&port->queue_lock);
}
}
^ permalink raw reply
* [PATCH 2/12] iseries_veth: Cleanup error and debug messages
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
This patch:
* converts uses of veth_printk() to veth_debug()/veth_error()
* makes terminology consistent, ie. always refer to LPAR not lpar
* be consistent about printing return codes as %d not %x
* make printf formats fit in 80 columns
---
drivers/net/iseries_veth.c | 87 ++++++++++++++++++++++-----------------------
1 files changed, 43 insertions(+), 44 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -287,7 +287,7 @@ static void veth_take_cap(struct veth_lp
HvLpEvent_Type_VirtualLan);
if (cnx->state & VETH_STATE_GOTCAPS) {
- veth_error("Received a second capabilities from lpar %d\n",
+ veth_error("Received a second capabilities from LPAR %d.\n",
cnx->remote_lp);
event->base_event.xRc = HvLpEvent_Rc_BufferNotAvailable;
HvCallEvent_ackLpEvent((struct HvLpEvent *) event);
@@ -306,7 +306,7 @@ static void veth_take_cap_ack(struct vet
spin_lock_irqsave(&cnx->lock, flags);
if (cnx->state & VETH_STATE_GOTCAPACK) {
- veth_error("Received a second capabilities ack from lpar %d\n",
+ veth_error("Received a second capabilities ack from LPAR %d.\n",
cnx->remote_lp);
} else {
memcpy(&cnx->cap_ack_event, event,
@@ -323,8 +323,7 @@ static void veth_take_monitor_ack(struct
unsigned long flags;
spin_lock_irqsave(&cnx->lock, flags);
- veth_printk(KERN_DEBUG, "Monitor ack returned for lpar %d\n",
- cnx->remote_lp);
+ veth_debug("cnx %d: lost connection.\n", cnx->remote_lp);
cnx->state |= VETH_STATE_RESET;
veth_kick_statemachine(cnx);
spin_unlock_irqrestore(&cnx->lock, flags);
@@ -345,8 +344,8 @@ static void veth_handle_ack(struct VethL
veth_take_monitor_ack(cnx, event);
break;
default:
- veth_error("Unknown ack type %d from lpar %d\n",
- event->base_event.xSubtype, rlp);
+ veth_error("Unknown ack type %d from LPAR %d.\n",
+ event->base_event.xSubtype, rlp);
};
}
@@ -382,8 +381,8 @@ static void veth_handle_int(struct VethL
veth_receive(cnx, event);
break;
default:
- veth_error("Unknown interrupt type %d from lpar %d\n",
- event->base_event.xSubtype, rlp);
+ veth_error("Unknown interrupt type %d from LPAR %d.\n",
+ event->base_event.xSubtype, rlp);
};
}
@@ -409,8 +408,8 @@ static int veth_process_caps(struct veth
|| (remote_caps->ack_threshold > VETH_MAX_ACKS_PER_MSG)
|| (remote_caps->ack_threshold == 0)
|| (cnx->ack_timeout == 0) ) {
- veth_error("Received incompatible capabilities from lpar %d\n",
- cnx->remote_lp);
+ veth_error("Received incompatible capabilities from LPAR %d.\n",
+ cnx->remote_lp);
return HvLpEvent_Rc_InvalidSubtypeData;
}
@@ -427,8 +426,8 @@ static int veth_process_caps(struct veth
cnx->num_ack_events += num;
if (cnx->num_ack_events < num_acks_needed) {
- veth_error("Couldn't allocate enough ack events for lpar %d\n",
- cnx->remote_lp);
+ veth_error("Couldn't allocate enough ack events "
+ "for LPAR %d.\n", cnx->remote_lp);
return HvLpEvent_Rc_BufferNotAvailable;
}
@@ -507,9 +506,8 @@ static void veth_statemachine(void *p)
} else {
if ( (rc != HvLpEvent_Rc_PartitionDead)
&& (rc != HvLpEvent_Rc_PathClosed) )
- veth_error("Error sending monitor to "
- "lpar %d, rc=%x\n",
- rlp, (int) rc);
+ veth_error("Error sending monitor to LPAR %d, "
+ "rc = %d\n", rlp, rc);
/* Oh well, hope we get a cap from the other
* end and do better when that kicks us */
@@ -532,9 +530,9 @@ static void veth_statemachine(void *p)
} else {
if ( (rc != HvLpEvent_Rc_PartitionDead)
&& (rc != HvLpEvent_Rc_PathClosed) )
- veth_error("Error sending caps to "
- "lpar %d, rc=%x\n",
- rlp, (int) rc);
+ veth_error("Error sending caps to LPAR %d, "
+ "rc = %d\n", rlp, rc);
+
/* Oh well, hope we get a cap from the other
* end and do better when that kicks us */
goto out;
@@ -574,10 +572,8 @@ static void veth_statemachine(void *p)
add_timer(&cnx->ack_timer);
cnx->state |= VETH_STATE_READY;
} else {
- veth_printk(KERN_ERR, "Caps rejected (rc=%d) by "
- "lpar %d\n",
- cnx->cap_ack_event.base_event.xRc,
- rlp);
+ veth_error("Caps rejected by LPAR %d, rc = %d\n",
+ rlp, cnx->cap_ack_event.base_event.xRc);
goto cant_cope;
}
}
@@ -590,8 +586,8 @@ static void veth_statemachine(void *p)
/* FIXME: we get here if something happens we really can't
* cope with. The link will never work once we get here, and
* all we can do is not lock the rest of the system up */
- veth_error("Badness on connection to lpar %d (state=%04lx) "
- " - shutting down\n", rlp, cnx->state);
+ veth_error("Unrecoverable error on connection to LPAR %d, shutting down"
+ " (state = 0x%04lx)\n", rlp, cnx->state);
cnx->state |= VETH_STATE_SHUTDOWN;
spin_unlock_irq(&cnx->lock);
}
@@ -623,7 +619,7 @@ static int veth_init_connection(u8 rlp)
msgs = kmalloc(VETH_NUMBUFFERS * sizeof(struct veth_msg), GFP_KERNEL);
if (! msgs) {
- veth_error("Can't allocate buffers for lpar %d\n", rlp);
+ veth_error("Can't allocate buffers for LPAR %d.\n", rlp);
return -ENOMEM;
}
@@ -639,8 +635,7 @@ static int veth_init_connection(u8 rlp)
cnx->num_events = veth_allocate_events(rlp, 2 + VETH_NUMBUFFERS);
if (cnx->num_events < (2 + VETH_NUMBUFFERS)) {
- veth_error("Can't allocate events for lpar %d, only got %d\n",
- rlp, cnx->num_events);
+ veth_error("Can't allocate enough events for LPAR %d.\n", rlp);
return -ENOMEM;
}
@@ -898,15 +893,17 @@ static struct net_device * __init veth_p
rc = register_netdev(dev);
if (rc != 0) {
- veth_printk(KERN_ERR,
- "Failed to register ethernet device for vlan %d\n",
- vlan);
+ veth_error("Failed registering net device for vlan%d.\n", vlan);
free_netdev(dev);
return NULL;
}
- veth_printk(KERN_DEBUG, "%s attached to iSeries vlan %d (lpar_map=0x%04x)\n",
- dev->name, vlan, port->lpar_map);
+ veth_info("%s attached to iSeries vlan %d.\n", dev->name, vlan);
+
+ for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
+ if (port->lpar_map & (1 << i))
+ veth_info("%s connected to LPAR %d.\n", dev->name, i);
+ }
return dev;
}
@@ -1039,7 +1036,7 @@ static int veth_start_xmit(struct sk_buf
dev_kfree_skb(skb);
} else {
if (port->pending_skb) {
- veth_error("%s: Tx while skb was pending!\n",
+ veth_error("%s: TX while skb was pending!\n",
dev->name);
dev_kfree_skb(skb);
spin_unlock_irqrestore(&port->pending_gate, flags);
@@ -1075,10 +1072,10 @@ static void veth_recycle_msg(struct veth
memset(&msg->data, 0, sizeof(msg->data));
veth_stack_push(cnx, msg);
- } else
- if (cnx->state & VETH_STATE_OPEN)
- veth_error("Bogus frames ack from lpar %d (#%d)\n",
- cnx->remote_lp, msg->token);
+ } else if (cnx->state & VETH_STATE_OPEN) {
+ veth_error("Non-pending frame (# %d) acked by LPAR %d.\n",
+ cnx->remote_lp, msg->token);
+ }
}
static void veth_flush_pending(struct veth_lpar_connection *cnx)
@@ -1188,8 +1185,8 @@ static void veth_flush_acks(struct veth_
0, &cnx->pending_acks);
if (rc != HvLpEvent_Rc_Good)
- veth_error("Error 0x%x acking frames from lpar %d!\n",
- (unsigned)rc, cnx->remote_lp);
+ veth_error("Failed acking frames from LPAR %d, rc = %d\n",
+ cnx->remote_lp, (int)rc);
cnx->num_pending_acks = 0;
memset(&cnx->pending_acks, 0xff, sizeof(cnx->pending_acks));
@@ -1225,9 +1222,10 @@ static void veth_receive(struct veth_lpa
/* make sure that we have at least 1 EOF entry in the
* remaining entries */
if (! (senddata->eofmask >> (startchunk + VETH_EOF_SHIFT))) {
- veth_error("missing EOF frag in event "
- "eofmask=0x%x startchunk=%d\n",
- (unsigned) senddata->eofmask, startchunk);
+ veth_error("Missing EOF fragment in event "
+ "eofmask = 0x%x startchunk = %d\n",
+ (unsigned)senddata->eofmask,
+ startchunk);
break;
}
@@ -1246,8 +1244,9 @@ static void veth_receive(struct veth_lpa
/* nchunks == # of chunks in this frame */
if ((length - ETH_HLEN) > VETH_MAX_MTU) {
- veth_error("Received oversize frame from lpar %d "
- "(length=%d)\n", cnx->remote_lp, length);
+ veth_error("Received oversize frame from LPAR %d "
+ "(length = %d)\n",
+ cnx->remote_lp, length);
continue;
}
^ permalink raw reply
* [PATCH 7/12] iseries_veth: Remove redundant message stack lock
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
The iseries_veth driver keeps a stack of messages for each connection
and a lock to protect the stack. However there is also a per-connection lock
which makes the message stack redundant.
Remove the message stack lock and document the fact that callers of the
stack-manipulation functions must hold the connection's lock.
---
drivers/net/iseries_veth.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -143,7 +143,6 @@ struct veth_lpar_connection {
struct VethCapData remote_caps;
u32 ack_timeout;
- spinlock_t msg_stack_lock;
struct veth_msg *msg_stack_head;
};
@@ -190,27 +189,23 @@ static void veth_timed_ack(unsigned long
#define veth_debug(fmt, args...) do {} while (0)
#endif
+/* You must hold the connection's lock when you call this function. */
static inline void veth_stack_push(struct veth_lpar_connection *cnx,
struct veth_msg *msg)
{
- unsigned long flags;
-
- spin_lock_irqsave(&cnx->msg_stack_lock, flags);
msg->next = cnx->msg_stack_head;
cnx->msg_stack_head = msg;
- spin_unlock_irqrestore(&cnx->msg_stack_lock, flags);
}
+/* You must hold the connection's lock when you call this function. */
static inline struct veth_msg *veth_stack_pop(struct veth_lpar_connection *cnx)
{
- unsigned long flags;
struct veth_msg *msg;
- spin_lock_irqsave(&cnx->msg_stack_lock, flags);
msg = cnx->msg_stack_head;
if (msg)
cnx->msg_stack_head = cnx->msg_stack_head->next;
- spin_unlock_irqrestore(&cnx->msg_stack_lock, flags);
+
return msg;
}
@@ -643,7 +638,6 @@ static int veth_init_connection(u8 rlp)
cnx->msgs = msgs;
memset(msgs, 0, VETH_NUMBUFFERS * sizeof(struct veth_msg));
- spin_lock_init(&cnx->msg_stack_lock);
for (i = 0; i < VETH_NUMBUFFERS; i++) {
msgs[i].token = i;
^ permalink raw reply
* [PATCH 9/12] iseries_veth: Use ref counts to track lifecycle of connection structs
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
The iseries_veth driver can attach to multiple vlans, which correspond to
multiple net devices. However there is only 1 connection between each LPAR,
so the connection structure may be shared by multiple net devices.
This makes module removal messy, because we can't deallocate the connections
until we know there are no net devices still using them. The solution is to
use ref counts on the connections, so we can delete them (actually stop) as
soon as the ref count hits zero.
This patch fixes (part of) a bug we were seeing with IPv6 sending probes to
a dead LPAR, which would then hang us forever due to leftover skbs.
This patch has the (minor?) side effect that we only start negotiating a
connection with LPARs which are on one of our vlans. The previous behaviour
was to start negotiation with all LPARs unconditionally, will have the think
about that one.
---
drivers/net/iseries_veth.c | 89 ++++++++++++++++++++++++++++++---------------
1 files changed, 61 insertions(+), 28 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -129,6 +129,7 @@ struct veth_lpar_connection {
int num_events;
struct VethCapData local_caps;
+ struct kref refcount;
struct timer_list ack_timer;
spinlock_t lock;
@@ -620,6 +621,10 @@ static int veth_init_connection(u8 rlp)
return -ENOMEM;
memset(cnx, 0, sizeof(*cnx));
+ /* This gets us 1 reference, which is held on behalf of the driver
+ * infrastructure. It's released at module unload. */
+ kref_init(&cnx->refcount);
+
cnx->remote_lp = rlp;
spin_lock_init(&cnx->lock);
INIT_WORK(&cnx->statemachine_wq, veth_statemachine, cnx);
@@ -658,12 +663,10 @@ static int veth_init_connection(u8 rlp)
return 0;
}
-static void veth_stop_connection(u8 rlp)
+static void veth_stop_connection(struct kref *ref)
{
- struct veth_lpar_connection *cnx = veth_cnx[rlp];
-
- if (! cnx)
- return;
+ struct veth_lpar_connection *cnx;
+ cnx = container_of(ref, struct veth_lpar_connection, refcount);
spin_lock_irq(&cnx->lock);
cnx->state |= VETH_STATE_RESET | VETH_STATE_SHUTDOWN;
@@ -1352,15 +1355,31 @@ static void veth_timed_ack(unsigned long
static int veth_remove(struct vio_dev *vdev)
{
- int i = vdev->unit_address;
+ struct veth_lpar_connection *cnx;
struct net_device *dev;
+ struct veth_port *port;
+ int i;
- dev = veth_dev[i];
- if (dev != NULL) {
- veth_dev[i] = NULL;
- unregister_netdev(dev);
- free_netdev(dev);
+ dev = veth_dev[vdev->unit_address];
+
+ if (! dev)
+ return 0;
+
+ port = netdev_priv(dev);
+
+ for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
+ cnx = veth_cnx[i];
+
+ if (cnx && (port->lpar_map & (1 << i))) {
+ /* Drop our reference to connections on our VLAN */
+ kref_put(&cnx->refcount, veth_stop_connection);
+ }
}
+
+ veth_dev[vdev->unit_address] = NULL;
+ unregister_netdev(dev);
+ free_netdev(dev);
+
return 0;
}
@@ -1368,6 +1387,7 @@ static int veth_probe(struct vio_dev *vd
{
int i = vdev->unit_address;
struct net_device *dev;
+ struct veth_port *port;
dev = veth_probe_one(i, &vdev->dev);
if (dev == NULL) {
@@ -1376,11 +1396,19 @@ static int veth_probe(struct vio_dev *vd
}
veth_dev[i] = dev;
- /* Start the state machine on each connection, to commence
- * link negotiation */
- for (i = 0; i < HVMAXARCHITECTEDLPS; i++)
- if (veth_cnx[i])
+ port = (struct veth_port*)netdev_priv(dev);
+
+ /* Start the state machine on each connection on this vlan. If we're
+ * the first dev to do so this will commence link negotiation */
+ for (i = 0; i < HVMAXARCHITECTEDLPS; i++) {
+ if (! (port->lpar_map & (1 << i)))
+ continue;
+
+ if (veth_cnx[i]) {
+ kref_get(&(veth_cnx[i]->refcount));
veth_kick_statemachine(veth_cnx[i]);
+ }
+ }
return 0;
}
@@ -1409,26 +1437,31 @@ static struct vio_driver veth_driver = {
void __exit veth_module_cleanup(void)
{
int i;
+ struct veth_lpar_connection *cnx;
- /* Stop the queues first to stop any new packets being sent. */
- for (i = 0; i < HVMAXARCHITECTEDVIRTUALLANS; i++)
- if (veth_dev[i])
- netif_stop_queue(veth_dev[i]);
+ /* Drop the driver's references to the connections. */
+ for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) {
+ cnx = veth_cnx[i];
- /* Stop the connections before we unregister the driver. This
- * ensures there's no skbs lying around holding the device open. */
- for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
- veth_stop_connection(i);
+ if (cnx) {
+ kref_put(&cnx->refcount, veth_stop_connection);
+ }
+ }
- HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan);
+ /* Unregister the driver, which will close all the netdevs and stop
+ * the connections when they're no longer referenced. */
+ vio_unregister_driver(&veth_driver);
- /* Hypervisor callbacks may have scheduled more work while we
- * were stoping connections. Now that we've disconnected from
- * the hypervisor make sure everything's finished. */
+ /* Make sure each connection's state machine has run to completion. */
flush_scheduled_work();
- vio_unregister_driver(&veth_driver);
+ /* Disconnect our "irq" to stop events coming from the Hypervisor. */
+ HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan);
+
+ /* Make sure any work queued from Hypervisor callbacks is finished. */
+ flush_scheduled_work();
+ /* Deallocate everything. */
for (i = 0; i < HVMAXARCHITECTEDLPS; ++i)
veth_destroy_connection(i);
^ permalink raw reply
* [PATCH 3/12] iseries_veth: Make init_connection() & destroy_connection() symmetrical
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
This patch makes veth_init_connection() and veth_destroy_connection()
symmetrical in that they allocate/deallocate the same data.
Currently if there's an error while initialising connections (ie. ENOMEM)
we call veth_module_cleanup(), however this will oops because we call
driver_unregister() before we've called driver_register(). I've never seen
this actually happen though.
So instead we explicitly call veth_destroy_connection() in a reverse
loop for the connections we've successfully initialised.
---
drivers/net/iseries_veth.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -664,6 +664,14 @@ static void veth_stop_connection(u8 rlp)
* been deleted by the state machine, just want to make sure
* its not running any more */
del_timer_sync(&cnx->ack_timer);
+}
+
+static void veth_destroy_connection(u8 rlp)
+{
+ struct veth_lpar_connection *cnx = veth_cnx[rlp];
+
+ if (! cnx)
+ return;
if (cnx->num_events > 0)
mf_deallocate_lp_events(cnx->remote_lp,
@@ -675,14 +683,6 @@ static void veth_stop_connection(u8 rlp)
HvLpEvent_Type_VirtualLan,
cnx->num_ack_events,
NULL, NULL);
-}
-
-static void veth_destroy_connection(u8 rlp)
-{
- struct veth_lpar_connection *cnx = veth_cnx[rlp];
-
- if (! cnx)
- return;
kfree(cnx->msgs);
kfree(cnx);
@@ -1424,15 +1424,15 @@ module_exit(veth_module_cleanup);
int __init veth_module_init(void)
{
- int i;
- int rc;
+ int i, rc;
this_lp = HvLpConfig_getLpIndex_outline();
for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) {
rc = veth_init_connection(i);
if (rc != 0) {
- veth_module_cleanup();
+ for (; i >= 0; i--)
+ veth_destroy_connection(i);
return rc;
}
}
^ permalink raw reply
* [PATCH 4/12] iseries_veth: Remove a FIXME WRT deletion of the ack_timer
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
The iseries_veth driver has a timer which we use to send acks. When the
connection is reset or stopped we need to delete the timer.
Currently we only call del_timer() when resetting a connection, which means
the timer might run again while the connection is being re-setup. As it turns
out that's ok, because the flags the timer consults have been reset.
It's cleaner though to call del_timer_sync() once we've dropped the lock,
although the timer may still run between us dropping the lock and calling
del_timer_sync(), but as above that's ok.
---
drivers/net/iseries_veth.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -450,13 +450,15 @@ static void veth_statemachine(void *p)
if (cnx->state & VETH_STATE_RESET) {
int i;
- del_timer(&cnx->ack_timer);
-
if (cnx->state & VETH_STATE_OPEN)
HvCallEvent_closeLpEventPath(cnx->remote_lp,
HvLpEvent_Type_VirtualLan);
- /* reset ack data */
+ /*
+ * Reset ack data. This prevents the ack_timer actually
+ * doing anything, even if it runs one more time when
+ * we drop the lock below.
+ */
memset(&cnx->pending_acks, 0xff, sizeof (cnx->pending_acks));
cnx->num_pending_acks = 0;
@@ -469,9 +471,16 @@ static void veth_statemachine(void *p)
if (cnx->msgs)
for (i = 0; i < VETH_NUMBUFFERS; ++i)
veth_recycle_msg(cnx, cnx->msgs + i);
+
+ /* Drop the lock so we can do stuff that might sleep or
+ * take other locks. */
spin_unlock_irq(&cnx->lock);
+
+ del_timer_sync(&cnx->ack_timer);
veth_flush_pending(cnx);
+
spin_lock_irq(&cnx->lock);
+
if (cnx->state & VETH_STATE_RESET)
goto restart;
}
@@ -658,12 +667,8 @@ static void veth_stop_connection(u8 rlp)
veth_kick_statemachine(cnx);
spin_unlock_irq(&cnx->lock);
+ /* Wait for the state machine to run. */
flush_scheduled_work();
-
- /* FIXME: not sure if this is necessary - will already have
- * been deleted by the state machine, just want to make sure
- * its not running any more */
- del_timer_sync(&cnx->ack_timer);
}
static void veth_destroy_connection(u8 rlp)
^ permalink raw reply
* [PATCH 1/12] iseries_veth: Make error messages more user friendly, and add a debug macro
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
Currently the iseries_veth driver prints the file name and line number in its
error messages. This isn't very useful for most users, so just print
"iseries_veth: message" instead.
Also add a veth_debug() and veth_info() macro to replace the current
veth_printk().
---
drivers/net/iseries_veth.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -79,6 +79,8 @@
#include <asm/iommu.h>
#include <asm/vio.h>
+#define DEBUG 1
+
#include "iseries_veth.h"
MODULE_AUTHOR("Kyle Lucke <klucke@us.ibm.com>");
@@ -176,11 +178,18 @@ static void veth_timed_ack(unsigned long
* Utility functions
*/
-#define veth_printk(prio, fmt, args...) \
- printk(prio "%s: " fmt, __FILE__, ## args)
+#define veth_info(fmt, args...) \
+ printk(KERN_INFO "iseries_veth: " fmt, ## args)
#define veth_error(fmt, args...) \
- printk(KERN_ERR "(%s:%3.3d) ERROR: " fmt, __FILE__, __LINE__ , ## args)
+ printk(KERN_ERR "iseries_veth: Error: " fmt, ## args)
+
+#ifdef DEBUG
+#define veth_debug(fmt, args...) \
+ printk(KERN_DEBUG "iseries_veth: " fmt, ## args)
+#else
+#define veth_debug(fmt, args...) do {} while (0)
+#endif
static inline void veth_stack_push(struct veth_lpar_connection *cnx,
struct veth_msg *msg)
^ permalink raw reply
* [PATCH 5/12] iseries_veth: Try to avoid pathological reset behaviour
From: Michael Ellerman @ 2005-06-30 10:20 UTC (permalink / raw)
To: linuxppc64-dev, netdev, linux-kernel
In-Reply-To: <200506302016.55125.michael@ellerman.id.au>
The iseries_veth driver contains a state machine which is used to manage
how connections are setup and neogotiated between LPARs.
If one side of a connection resets for some reason, the two LPARs can get
stuck in a race to re-setup the connection. This can lead to the connection
being declared dead by one or both ends. In practice this happens ~8/10 times
a connection is reset, although it's rare for connections to be reset.
(an example here: http://michael.ellerman.id.au/files/misc/veth-trace.html)
The core of the problem is that the end that resets the connection doesn't
wait for the other end to become aware of the reset. So the resetting end
starts setting the connection back up, and then receives a reset from the
other end (which is the response to the initial reset). And so on.
We're severely limited in what we can do to fix this. The protocol between
LPARs is essentially fixed, as we have to interoperate with both OS/400
and old Linux drivers. Which also means we need a fix that only changes the
code on one end.
The only fix I've found given that, is to just blindly sleep for a bit when
resetting the connection, in the hope that the other end will get itself
sorted. Needless to say I'd love it if someone has a better idea.
This does work, I've so far been unable to get it to break, whereas without
the fix a reset of one end will lead to a dead connection ~8/10 times.
---
drivers/net/iseries_veth.c | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
Index: veth-dev/drivers/net/iseries_veth.c
===================================================================
--- veth-dev.orig/drivers/net/iseries_veth.c
+++ veth-dev/drivers/net/iseries_veth.c
@@ -324,8 +324,12 @@ static void veth_take_monitor_ack(struct
spin_lock_irqsave(&cnx->lock, flags);
veth_debug("cnx %d: lost connection.\n", cnx->remote_lp);
- cnx->state |= VETH_STATE_RESET;
- veth_kick_statemachine(cnx);
+ /* Avoid kicking the statemachine once we're shutdown.
+ * It's unnecessary and it could break veth_stop_connection(). */
+ if (! (cnx->state & VETH_STATE_SHUTDOWN)) {
+ cnx->state |= VETH_STATE_RESET;
+ veth_kick_statemachine(cnx);
+ }
spin_unlock_irqrestore(&cnx->lock, flags);
}
@@ -483,6 +487,12 @@ static void veth_statemachine(void *p)
if (cnx->state & VETH_STATE_RESET)
goto restart;
+
+ /* Hack, wait for the other end to reset itself. */
+ if (! (cnx->state & VETH_STATE_SHUTDOWN)) {
+ schedule_delayed_work(&cnx->statemachine_wq, 5 * HZ);
+ goto out;
+ }
}
if (cnx->state & VETH_STATE_SHUTDOWN)
@@ -667,6 +677,15 @@ static void veth_stop_connection(u8 rlp)
veth_kick_statemachine(cnx);
spin_unlock_irq(&cnx->lock);
+ /* There's a slim chance the reset code has just queued the
+ * statemachine to run in five seconds. If so we need to cancel
+ * that and requeue the work to run now. */
+ if (cancel_delayed_work(&cnx->statemachine_wq)) {
+ spin_lock_irq(&cnx->lock);
+ veth_kick_statemachine(cnx);
+ spin_unlock_irq(&cnx->lock);
+ }
+
/* Wait for the state machine to run. */
flush_scheduled_work();
}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox