* Re: [GIT PULL] SCTP bug fixes for net-2.6
From: Vlad Yasevich @ 2007-12-12 15:30 UTC (permalink / raw)
To: Jijo Chacko; +Cc: netdev, herbert, lksctp-developers
In-Reply-To: <b7f21edd0712112229l6219c835w466d8bf43390cf3a@mail.gmail.com>
Jijo Chacko wrote:
>
> Hello experts,
>
> [ sorry if my questions are irrelevent to any one of you ].
>
> I am trying to develop a frame work, where i transport all HTTP traffic
> over SCTP. On user space, i can do this easily over SCTP, BSD-style
> sockets. If i am to write a kernel module(say in-kernel web
> client/server) using kernel version of SCTP interfaces, what are the
> available calls now supported ? I see in-kernel implementation like
> NFS/RPC make kernel version of UDP and TCP _sendmsg, i don't find
> similar interfaces for SCTP..
Look at fs/dlm/lowcomms.c
All you need to do is use IPPROTO_SCTP when creating a socket in
the kernel and then I believe you can use all the same calls as TCP/UDP.
-vlad
^ permalink raw reply
* Re: [RFC] net: napi fix
From: David Miller @ 2007-12-12 15:22 UTC (permalink / raw)
To: joonwpark81; +Cc: shemminger, netdev, linux-kernel, jgarzik, baum, andy
In-Reply-To: <b25c3fa70712112205l2e778bacx66470606272b199@mail.gmail.com>
From: "Joonwoo Park" <joonwpark81@gmail.com>
Date: Wed, 12 Dec 2007 15:05:26 +0900
> Could you explain how it fix the problem?
> IMHO I think your patch cannot solve the problem.
> The drivers can call netif_rx_complete and net_rx_action can do
> list_move_tail also.
Stephen is confused about what the bug is in these drivers,
that's all.
^ permalink raw reply
* Re: [RFC] net: napi fix
From: David Miller @ 2007-12-12 15:21 UTC (permalink / raw)
To: shemminger; +Cc: joonwpark81, netdev, linux-kernel, jgarzik, baum, andy
In-Reply-To: <20071211214634.4dedb274@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Tue, 11 Dec 2007 21:46:34 -0800
> Isn't this a better fix for all drivers, rather than peppering every
> driver with the special case. This is how the logic worked up until
> 2.6.24.
Stephen this is not the problem.
The problem is that the driver is doing a NAPI completion and
re-enabling chip interrupts with work_done == weight, and that is
illegal.
^ permalink raw reply
* Re: [PATCH 6/7] [NETDEV]: tehuti Fix possible causing oops of net_rx_action
From: David Miller @ 2007-12-12 15:20 UTC (permalink / raw)
To: shemminger; +Cc: joonwpark81, netdev, linux-kernel, jgarzik, baum, andy
In-Reply-To: <20071211213939.122de1db@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Tue, 11 Dec 2007 21:39:39 -0800
> On Wed, 12 Dec 2007 13:01:27 +0900
> "Joonwoo Park" <joonwpark81@gmail.com> wrote:
>
> > [NETDEV]: tehuti Fix possible causing oops of net_rx_action
> >
> > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
> > ---
> > drivers/net/tehuti.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/tehuti.c b/drivers/net/tehuti.c
> > index 21230c9..955e749 100644
> > --- a/drivers/net/tehuti.c
> > +++ b/drivers/net/tehuti.c
> > @@ -305,6 +305,8 @@ static int bdx_poll(struct napi_struct *napi, int budget)
> >
> > netif_rx_complete(dev, napi);
> > bdx_enable_interrupts(priv);
> > + if (unlikely(work_done == napi->weight))
> > + return work_done - 1;
> > }
> > return work_done;
> > }
>
> A better fix would be not going over budget in the first place.
That's not the problem.
They are not going over the budget, rather, they are hitting
the budget yet doing netif_rx_complete() as well which is
illegal.
Unless you strictly process less than "weight" packets, you must
not netif_rx_complete() and re-enable chip interrupts.
I can't believe people are trying to fix this bug like this.
^ permalink raw reply
* Re: [PATCH 2/2] ucc_geth: use rx-clock-name and tx-clock-name device tree properties
From: Timur Tabi @ 2007-12-12 15:19 UTC (permalink / raw)
To: galak; +Cc: netdev, linuxppc-dev
In-Reply-To: <1196716685975-git-send-email-timur@freescale.com>
Timur Tabi wrote:
> Updates the ucc_geth device driver to check the new rx-clock-name and
> tx-clock-name properties first. If present, it uses the new function
> qe_clock_source() to obtain the clock source. Otherwise, it checks the
> deprecated rx-clock and tx-clock properties.
>
> Update the device trees for 832x, 836x, and 8568 to contain the new property
> names only.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
If there are no objections, I'd like this patch to be pulled into 2.6.25. Thanks.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 1/2] qe: add function qe_clock_source()
From: Timur Tabi @ 2007-12-12 15:19 UTC (permalink / raw)
To: galak; +Cc: netdev, linuxppc-dev
In-Reply-To: <1196716680381-git-send-email-timur@freescale.com>
Timur Tabi wrote:
> Add function qe_clock_source() which takes a string containing the name of a
> QE clock source (as is typically found in device trees) and returns the
> matching enum qe_clock value.
>
> Update booting-without-of.txt to indicate that the UCC properties rx-clock
> and tx-clock are deprecated and replaced with rx-clock-name and tx-clock-name,
> which use strings instead of numbers to indicate QE clock sources.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
If there are no objections, I'd like this patch to be pulled into 2.6.25. Thanks.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 6/7] : tehuti Fix possible causing oops of net_rx_action
From: David Miller @ 2007-12-12 15:18 UTC (permalink / raw)
To: joonwpark81; +Cc: netdev, linux-kernel, jgarzik, baum, andy
In-Reply-To: <002901c83c73$b089b6e0$9c94fea9@jason>
From: "Joonwoo Park" <joonwpark81@gmail.com>
Date: Wed, 12 Dec 2007 13:01:27 +0900
> @@ -305,6 +305,8 @@ static int bdx_poll(struct napi_struct *napi, int budget)
>
> netif_rx_complete(dev, napi);
> bdx_enable_interrupts(priv);
> + if (unlikely(work_done == napi->weight))
> + return work_done - 1;
> }
> return work_done;
> }
Any time your trying to make a caller "happy" by adjusting
a return value forcefully, it's a hack.
And I stated this in another reply about this issue.
Please do not fix the problem this way.
The correct way to fix this is, if we did process a full
"weight" or work, we should not netif_rx_complete() and
we should not re-enable chip interrupts.
Instead we should return the true "work_done" value and
allow the caller to thus poll us one more time.
^ permalink raw reply
* Re: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
From: David Miller @ 2007-12-12 15:12 UTC (permalink / raw)
To: jesse.brandeburg
Cc: joonwpark81, netdev, linux-kernel, herbert, auke-jan.h.kok
In-Reply-To: <36D9DB17C6DE9E40B059440DB8D95F5203FBAF94@orsmsx418.amr.corp.intel.com>
From: "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
Date: Tue, 11 Dec 2007 16:38:37 -0800
> @@ -3933,6 +3933,10 @@ quit_polling:
> e1000_set_itr(adapter);
> netif_rx_complete(poll_dev, napi);
> e1000_irq_enable(adapter);
> + if (work_done == weight)
> + return work_done - 1;
> + else
> + return work_done;
Don't do this.
If you processed "weight" worth of packets, return that
exact value and do not netif_rx_complete() and do not
re-enable interrupts.
That is the only correct fix.
^ permalink raw reply
* Re: [RFC PATCH net-2.6.25 uncompilable] [TCP]: Avoid breaking GSOed skbs when SACKed one-by-one
From: David Miller @ 2007-12-12 15:11 UTC (permalink / raw)
To: lachlan.andrew; +Cc: ilpo.jarvinen, netdev, quetchen
In-Reply-To: <aa7d2c6d0712111614k2832f51axa8bc9736ac7ec796@mail.gmail.com>
From: "Lachlan Andrew" <lachlan.andrew@gmail.com>
Date: Tue, 11 Dec 2007 16:14:36 -0800
> This thread started because TCP processing interferes with RTT
> estimation. This problem would be eliminated if time-stamping were
> done as soon as the packet comes off the NIC.
We don't do that because such timestamping is too expensive.
It used to be the case that we did this, but we stopped doing
that a long time ago.
On x86 for example, timestamping can involve touching a slow
I/O device to read the timestamp. We do not want to do that
for every packet.
Also, we timestamp differently for TCP, the global high
resolution timestamp is overkill for this purpose.
Really, this is a silly idea and would only be a bandaid
for the problem at hand, that TCP input processing is
too expensive in certain circumstances.
^ permalink raw reply
* Re: DM9000_IRQ_FLAGS
From: Daniel Mack @ 2007-12-12 15:01 UTC (permalink / raw)
To: Remy Bohmer; +Cc: netdev
In-Reply-To: <3efb10970712120646ub8f5047ub333b74658bab44d@mail.gmail.com>
Hi,
On Dec 12, 2007, at 3:46 PM, Remy Bohmer wrote:
> The DM9000 adapter is connected to a GPIO line on the at91sam9261-ek
> board, the board-specific code has a generic handler for every GPIO
> interrupt. So, the Device driver __does not need to know__ the type of
> interrupt, It just installs a handler by request_irq().
Ok.
>> resource->flags = IORESOURCE_IRQ | IRQT_RISING;
>>
> These types of flags are never meant to be compatible, and thus they
> shall not be stored in the same flags variable. The same is valid for
> the flags IRQF_* and IRQT* flags, so request_irq() is also not the
> place to do this.
Ah, so I was trapped by faulty code doing that. I thought as the
type of interrupt is also a detail information about the resource,
this is the place to put is. However, it also works with
set_irq_type().
Thanks for sorting that out :)
Best regards,
Daniel
^ permalink raw reply
* Re: [PATCH 7/7] : myri10ge Fix possible causing oops of net_rx_action
From: Joonwoo Park @ 2007-12-12 14:48 UTC (permalink / raw)
To: subrata; +Cc: netdev, linux-kernel, jgarzik, help
In-Reply-To: <1197448257.4710.31.camel@subratamodak.linux.ibm.com>
2007/12/12, Subrata Modak <subrata@linux.vnet.ibm.com>:
> Would you like to submit the Driver Test Cases to LTP ? Or, if you have
> your Driver test cases written in C, we can port to LTP and have it
> inside the LTP package.
> You can also check out the following links for more info, and can always
> ping me for anything:
> http://ltp.cvs.sourceforge.net/ltp/ltp/testcases/kernel/device-drivers/,
> http://ltp.sourceforge.net/documentation/how-to/ltp.php?
>
> Regards--
> Subrata
> (LTP Maintainer)
>
Subrata,
I can't sure it's possible since my test was needed some hardware
stuffs but I'll try it in near future and contact you.
Thanks.
Joonwoo
^ permalink raw reply
* Re: DM9000_IRQ_FLAGS
From: Remy Bohmer @ 2007-12-12 14:46 UTC (permalink / raw)
To: Daniel Mack; +Cc: netdev
In-Reply-To: <20071212134153.GA21510@buzzloop.caiaq.de>
Hello Daniel,
> However, request_irq() is called with IRQF_SHARED only, so neither
> IRQT_RISING nor IRQT_FALLING is set and the value defaults to
> IRQT_NOEDGE. How can you get IRQs?
The DM9000 adapter is connected to a GPIO line on the at91sam9261-ek
board, the board-specific code has a generic handler for every GPIO
interrupt. So, the Device driver __does not need to know__ the type of
interrupt, It just installs a handler by request_irq().
So, these flags are dependant on the board where the driver is
installed, and thus everything you want to define fixed in the driver
is not generic and thus wrong.
This is why a NACK your patch.
> Actually, the best way to go is to let the platform resources flags
> decide about that with something like
>
> resource->flags = IORESOURCE_IRQ | IRQT_RISING;
>
These types of flags are never meant to be compatible, and thus they
shall not be stored in the same flags variable. The same is valid for
the flags IRQF_* and IRQT* flags, so request_irq() is also not the
place to do this.
You should look at the routine set_irq_type() in kernel/irq/chip.c.
This routine is supposed to be used for the IRQT_RISING type of flags
Just do a grep of IRQT_RISING in the arch/arm tree and you will find
enough examples.
Kind Regards,
Remy
^ permalink raw reply
* [PATCH] Remove unused define from loopback driver
From: Pavel Emelyanov @ 2007-12-12 14:44 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The LOOPBACK_OVERHEAD is not used in this file at all.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 662b8d1..3d1c743 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -64,8 +64,6 @@ struct pcpu_lstats {
unsigned long bytes;
};
-#define LOOPBACK_OVERHEAD (128 + MAX_HEADER + 16 + 16)
-
/* KISS: just allocate small chunks and copy bits.
*
* So, in fact, this is documentation, explaining what we expect
^ permalink raw reply related
* Re: [PATCH 1/2] [net/wireless/iwlwifi] : iwlwifi 3945 Fix raceconditional panic.
From: Joonwoo Park @ 2007-12-12 14:41 UTC (permalink / raw)
To: Andrew Morton
Cc: Zhu Yi, ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20071212034325.4a022320.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2007/12/12, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>:
>
> Did drivers/net/wireless/iwlwifi/iwl4965-base.c get fixed in a simlar
> fashion?
>
Hi Andrew,
Both of them was applied to mainline.
commit 3ae6a054553ee8b7f74bf7de8904022b26705778
Author: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu Nov 29 10:43:16 2007 +0900
iwlwifi 4965 Fix race conditional panic.
Signed-off-by: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
commit e47eb6ad41e8fc4c2696665512b70d1fd4cf3f22
Author: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu Nov 29 10:42:49 2007 +0900
iwlwifi 3945 Fix race conditional panic.
Signed-off-by: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: John W. Linville <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Thanks,
Joonwoo
^ permalink raw reply
* Re: [Bug 9542] BUG: bad unlock balance detected!
From: Herbert Xu @ 2007-12-12 14:20 UTC (permalink / raw)
To: Andrew Morton; +Cc: netdev, bugme-daemon, Stephen Hemminger, olel, Jay Vosburgh
In-Reply-To: <20071211140614.7c1738a5.akpm@linux-foundation.org>
On Tue, Dec 11, 2007 at 10:06:14PM +0000, Andrew Morton wrote:
>
> > events/0/9 just changed the state of lock:
> > (&mc->mca_lock){-+..}, at: [<c0412602>] mld_ifc_timer_expire+0x130/0x1fb
> > but this lock took another, soft-read-irq-unsafe lock in the past:
> > (&bond->lock){-.--}
> >
> > and interrupts could create inverse lock ordering between them.
Yes this is a bug in the bonding driver.
It's assuming that bond->lock is only ever held for writing in
process context. Unfortunately our current set_multicast_list
interface violates this constraint.
Now I do have a TODO item to fix set_multicast_list to not do
that, but until that happens, bond->lock should always turn BH
off, even on read lock.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [kvm-devel] [virtio-net][PATCH] Don't arm tx hrtimer with a constant 500us each transmit
From: Dor Laor @ 2007-12-12 14:17 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: kvm-devel, Rusty Russell, netdev, virtualization
In-Reply-To: <200712121419.50400.borntraeger@de.ibm.com>
Christian Borntraeger wrote:
>
> Am Mittwoch, 12. Dezember 2007 schrieb Dor Laor:
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -406,10 +405,10 @@ again:
>
> Hmm, while I agree in general with the patch, I fail to find the proper
> version of virtio_net where this patch applies. I tried kvm.git and
> linux-2.6.git from kernel.org. Can you give me a pointer to the repository
> where you work on virtio?
>
Sorry for that, I added some debug prints of my one.
Here it is: *git clone
git*://kvm.*qumranet*.com/home/*dor*/src/linux-2.6-nv use branch 'virtio'.
BTW: what git repository do you use?
This patch improves my tx performance from 720Mbps to 900Mbps.
Dor
>
> Christian
>
^ permalink raw reply
* Re: [Bugme-new] [Bug 9543] New: RTNL: assertion failed at net/ipv6/addrconf.c (2164)/RTNL: assertion failed at net/ipv4/devinet.c (1055)
From: Herbert Xu @ 2007-12-12 13:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: olel, bugme-daemon, shemminger, davem, netdev, fubar
In-Reply-To: <20071211034603.cbcc4762.akpm@linux-foundation.org>
Andrew Morton <akpm@linux-foundation.org> wrote:
>
> From: Andrew Morton <akpm@linux-foundation.org>
>
> Remove stray rtnl_unlock().
>
> Addresses http://bugzilla.kernel.org/show_bug.cgi?id=9542
Adnrew, please cc Jay Vosburgh <fubar@us.ibm.com> on bonding
issues.
> diff -puN drivers/net/bonding/bond_sysfs.c~bonding-locking-fix drivers/net/bonding/bond_sysfs.c
> --- a/drivers/net/bonding/bond_sysfs.c~bonding-locking-fix
> +++ a/drivers/net/bonding/bond_sysfs.c
> @@ -1111,8 +1111,6 @@ static ssize_t bonding_store_primary(str
> out:
> write_unlock_bh(&bond->lock);
>
> - rtnl_unlock();
> -
Looking at the changeset that added this perhaps the intention
is to hold the lock? If so we should add an rtnl_lock to the start
of the function.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] [IPSEC]: Add populate from packet (PFP) support
From: Herbert Xu @ 2007-12-12 13:52 UTC (permalink / raw)
To: Tyler Hicks; +Cc: linux netdev, David Miller, Joy Latten
In-Reply-To: <20071212032352.GA12890@hestia>
On Tue, Dec 11, 2007 at 07:23:52PM -0800, Tyler Hicks wrote:
> RFC 4301 requires us to associate each SPD entry with a set of flags to
> determine how to assign the selector values when creating a new SAD entry.
>
> Each selector in the new xfrm_state can either be assigned using the
> corresponding selector in the xfrm_policy or with the corresponding value
> in the flowi. Prior to this patch, the fields in the flowi were always
> used.
>
> Signed-off-by: Tyler Hicks <tyhicks@ou.edu>
Thanks for the patch Tyler!
I think the kernel is fine as it is. What we're doing is generating
the most specific selector possible for the larval SA and which lets
the KM do whatever it wants.
What RFC 4301 is asking for is for the mature SAs to have their
selectors either populated from the policy or the packet.
So for PFP the KM should fill out its SA selector according to its
PFP flags. In other words we don't need PFP flags in the kernel
at all.
> + if (pol->flags & XFRM_POLICY_PFP_SPORT) {
> + x->sel.sport = xfrm_flowi_sport(fl);
> + x->sel.sport_mask = htons(0xffff);
> + } else {
> + x->sel.sport = pol->selector.sport;
> + x->sel.sport = pol->selector.sport_mask;
There's a typo here.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: DM9000_IRQ_FLAGS
From: Daniel Mack @ 2007-12-12 13:41 UTC (permalink / raw)
To: Remy Bohmer; +Cc: netdev
In-Reply-To: <3efb10970712111231r29f9fba1l3528d805d1009c52@mail.gmail.com>
Hi Remy,
On Tue, Dec 11, 2007 at 09:31:03PM +0100, Remy Bohmer wrote:
> This controller is also used on many other boards, like the e.g. Atmel
> AT91sam9261-ek board. On that board on both the rising _and_ falling
> edge an interrupt is generated.
However, request_irq() is called with IRQF_SHARED only, so neither
IRQT_RISING nor IRQT_FALLING is set and the value defaults to
IRQT_NOEDGE. How can you get IRQs?
> I can test tomorrow if this patch leaves this board in tact, but
> should the board-specific code not add this flag if it is required ?
> By modifying this driver you will interfere the behavior of other
> boards, and I do not know if there any level triggered types used.
Actually, the best way to go is to let the platform resources flags
decide about that with something like
resource->flags = IORESOURCE_IRQ | IRQT_RISING;
but the dm9000 does not care about them at all. Changing that would also
imply modifications to all board support code.
Regards,
Daniel
^ permalink raw reply
* Re: [kvm-devel] [virtio-net][PATCH] Don't arm tx hrtimer with a constant 500us each transmit
From: Christian Borntraeger @ 2007-12-12 13:19 UTC (permalink / raw)
To: kvm-devel, dor.laor; +Cc: Rusty Russell, netdev, virtualization
In-Reply-To: <475FD9E8.1060109@qumranet.com>
Am Mittwoch, 12. Dezember 2007 schrieb Dor Laor:
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -406,10 +405,10 @@ again:
Hmm, while I agree in general with the patch, I fail to find the proper
version of virtio_net where this patch applies. I tried kvm.git and
linux-2.6.git from kernel.org. Can you give me a pointer to the repository
where you work on virtio?
Christian
^ permalink raw reply
* RE: [PATCH] Increase virtual FIFOs in ucc_geth.
From: Joakim Tjernlund @ 2007-12-12 13:08 UTC (permalink / raw)
To: Li Yang; +Cc: netdev
In-Reply-To: <989B956029373F45A0B8AF029708189001B4D908@zch01exm26.fsl.freescale.net>
On Tue, 2007-12-11 at 19:51 +0800, Li Yang wrote:
> > -----Original Message-----
> > From: Joakim Tjernlund [mailto:joakim.tjernlund@transmode.se]
> > Sent: Tuesday, December 11, 2007 6:58 PM
> > To: Li Yang
> > Cc: netdev@vger.kernel.org
> > Subject: RE: [PATCH] Increase virtual FIFOs in ucc_geth.
> >
> >
> > On Tue, 2007-12-11 at 11:11 +0100, Joakim Tjernlund wrote:
> > > On Tue, 2007-12-11 at 17:49 +0800, Li Yang wrote:
> > > > > -----Original Message-----
> > > > > From: Joakim Tjernlund [mailto:Joakim.Tjernlund@transmode.se]
> > > > > Sent: Tuesday, December 11, 2007 2:46 AM
> > > > > To: Li Yang-r58472 <LeoLi@freescale.com> Netdev
> > > > > Cc: Joakim Tjernlund
> > > > > Subject: [PATCH] Increase virtual FIFOs in ucc_geth.
> > > > >
> > > > > Increase UCC_GETH_URFS_INIT to 1152 and
> > > > > UCC_GETH_UTFS_INIT to 896 to avoid HW Overrun/Underrun.
> > > >
> > > > Please be noted that these values are only used for
> > 10/100Mbps speed.
> > > > Did you get Overrun in 10/100M mode?
> > >
> > > I get both TX Underrun and RX overrun in 100Mbps, FD, just
> > > by running a tftp transfer. It feels like the URFET and/or URSFET
> > > isn't working. Why I don't know. CPU is MPC832x
> > >
> > > Jocke
> >
> > I am a bit confused how the RBMR and TBMR is supposed to work. In
> > ucc_get there is:
> > out_be32(&ugeth->p_tx_glbl_pram->tstate, ((u32)
> > function_code) << 24);
> > ugeth->p_rx_glbl_pram->rstate = function_code;
> > First, should not the rx part look the same as tx?
>
> To be consist with the chip RM, type for tstate is u32 and type for
> rstate is u8. Personally I don't think that it will be different to
> access tstate as u8, but it will be more readable to be the same as
> manual. Well, rstate probably should be changed to use IO accessor too.
>
> > Does programing rstate/tstate replace RBMR and TBMR?
>
> RBMR and TBMR? These are for other protocols.
>
> - Leo
Noticed that the number of threads(numThreadsTx and numThreadsRx) is set
to 4. That is a good value for GBE, but the manual thinks 1 is good for
10/100 Mbps. How does 4 threads affect the FIFO for 100 Mbps? I can't
test ATM as the system is busy with other things.
Jocke
^ permalink raw reply
* [virtio-net][PATCH] Don't arm tx hrtimer with a constant 500us each transmit
From: Dor Laor @ 2007-12-12 12:54 UTC (permalink / raw)
To: Rusty Russell, kvm-devel, netdev, virtualization
commit 763769621d271d92204ed27552d75448587c1ac0
Author: Dor Laor <dor.laor@qumranet.com>
Date: Wed Dec 12 14:52:00 2007 +0200
[virtio-net][PATCH] Don't arm tx hrtimer with a constant 50us each
transmit
The current start_xmit sets 500us hrtimer to kick the host.
The problem is that if another xmit happens before the timer was
fired then
the first xmit will have to wait additional 500us.
This patch does not re-arm the timer if there is existing one.
This will shorten the latency for tx.
Signed-off-by: Dor Laor <dor.laor@qumranet.com>
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7b051d5..8bb17d1
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -406,10 +405,10 @@ again:
virtio_debug(vdebug, "%s: before calling kick %d\n",
__FUNCTION__, __LINE__);
vi->svq->vq_ops->kick(vi->svq);
vi->out_num = 0;
- } else {
- vi->stats.hrtimer_starts++;
- hrtimer_start(&vi->tx_timer, ktime_set(0,500000),
- HRTIMER_MODE_REL);
+ } else if (!hrtimer_is_queued(&vi->tx_timer)) {
+ vi->stats.hrtimer_starts++;
+ hrtimer_start(&vi->tx_timer, ktime_set(0,500000),
+ HRTIMER_MODE_REL);
}
return 0;
}
^ permalink raw reply related
* Re: [PATCH] iproute2: support dotted-quad netmask notation.
From: Andreas Henriksson @ 2007-12-12 11:55 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20071211171406.57389dba@freepuppy.rosehill>
On Tue, Dec 11, 2007 at 05:14:06PM -0800, Stephen Hemminger wrote:
> On Sun, 09 Dec 2007 18:10:22 +0100
> Andreas Henriksson <andreas@fatal.se> wrote:
> > I think both previous patches where broken on big-endian platforms.
> > Here's an updated patch again. I'm very sorry for the inconvenience!
[...]
> > + *val=0;
> > + for (mask = ntohl(addr.data[0]); mask; mask <<= 1)
> > + (*val)++;
[...]
>
> applied
>
Just to make sure.... It looks on git.kernel.org like you applied the wrong
patch. (Maybe you just haven't pushed out the latest changes there yet.)
Please double-check that you actually applied the latest version (which is
the one in the mail you replied "applied" to, important part quoted above).
Additionally, there's still a couple of trivial patches pending in
the "patches" branch of
git://git.debian.org/git/collab-maint/pkg-iproute.git
Please see the original thread[1], where Patrick McHarding had some concerns
about one of the patches. It's exactly the same changes you made in commit
660818498d0f5a3f52c05355a3e82c23f670fcc1 [2] though, so I don't really see the
problem. I have an additional patch[3] available, that makes MAX_ROUNDS
configurable which Patrick requested.
Please comment on the way forward there....
[1]: http://www.spinics.net/lists/netdev/msg44800.html
[2]: Where the comment seems to be wrong about "Limit ip route flush...",
since it's actually "ip neigh flush" that's being modified.
[3]: I have a slightly updated patch, but it's basically the same as
http://www.spinics.net/lists/netdev/msg45080.html
Will send updated version if the patches it's based on goes in.
--
Regards,
Andreas Henriksson
^ permalink raw reply
* Re: [PATCH 1/2] [net/wireless/iwlwifi] : iwlwifi 3945 Fix raceconditional panic.
From: Andrew Morton @ 2007-12-12 11:43 UTC (permalink / raw)
To: Joonwoo Park
Cc: 'Zhu Yi', ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <016401c83229$3302d250$9c94fea9@jason>
On Thu, 29 Nov 2007 10:42:49 +0900 "Joonwoo Park" <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2007/11/29, Zhu Yi <yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>:
> >
> > Good catch. But it will be better if you add it into
> > iwl_cancel_deferred_work().
> >
>
> Thanks.
> I agree with you.
> Actually, I considered it, but I was afraid of side effect.
> Anyway, I'm attaching a new one.
>
> Thanks.
> Joonwoo
>
> [net/wireless/iwlwifi] : iwlwifi 3945 Fix race conditional panic.
>
> Signed-off-by: Joonwoo Park <joonwpark81-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 465da4f..e51e872 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -8270,6 +8270,7 @@ static void iwl_cancel_deferred_work(struct iwl_priv *priv)
> {
> iwl_hw_cancel_deferred_work(priv);
>
> + cancel_delayed_work_sync(&priv->init_alive_start);
> cancel_delayed_work(&priv->scan_check);
> cancel_delayed_work(&priv->alive_start);
> cancel_delayed_work(&priv->post_associate);
Did drivers/net/wireless/iwlwifi/iwl4965-base.c get fixed in a simlar
fashion?
^ permalink raw reply
* Re: [kvm-devel] [PATCH resent] virtio_net: Fix stalled inbound trafficon early packets
From: Christian Borntraeger @ 2007-12-12 11:39 UTC (permalink / raw)
To: Rusty Russell; +Cc: dor.laor, kvm-devel, netdev, virtualization
In-Reply-To: <200712121254.13865.rusty@rustcorp.com.au>
Am Mittwoch, 12. Dezember 2007 schrieb Rusty Russell:
> On Wednesday 12 December 2007 00:16:12 Christian Borntraeger wrote:
> > That would also work. We already have VRING_AVAIL_F_NO_INTERRUPT in
> > virtio_ring.c - maybe we can use that. Its hidden in callback and
> > restart handling, what about adding an explicit startup?
>
> Yes, I debated whether to make this a separate hook or not; the current
> method reduces the number of function calls without having two ways of
> disabling callbacks.
>
> In this case, simply starting devices with callbacks disabled and
> renaming 'restart' to 'enable' (or something) and calling it at the
> beginning is probably sufficient?
So you suggest something like the following patch? It seems to work but
there is still a theoretical race at startup. Therefore, I tend to agree
with Dor that a separate hook seems prefereable, so I am not fully sure if
the patch is the final solution:
ps: Its ok to answer that after your vacation.
---
drivers/block/virtio_blk.c | 3 ++-
drivers/net/virtio_net.c | 5 ++++-
drivers/virtio/virtio_ring.c | 9 ++++-----
include/linux/virtio.h | 4 ++--
4 files changed, 12 insertions(+), 9 deletions(-)
Index: kvm/drivers/virtio/virtio_ring.c
===================================================================
--- kvm.orig/drivers/virtio/virtio_ring.c
+++ kvm/drivers/virtio/virtio_ring.c
@@ -220,7 +220,7 @@ static void *vring_get_buf(struct virtqu
return ret;
}
-static bool vring_restart(struct virtqueue *_vq)
+static bool vring_enable(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -264,7 +264,7 @@ static struct virtqueue_ops vring_vq_ops
.add_buf = vring_add_buf,
.get_buf = vring_get_buf,
.kick = vring_kick,
- .restart = vring_restart,
+ .enable = vring_enable,
.shutdown = vring_shutdown,
};
@@ -299,9 +299,8 @@ struct virtqueue *vring_new_virtqueue(un
vq->in_use = false;
#endif
- /* No callback? Tell other side not to bother us. */
- if (!callback)
- vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
+ /* disable interrupts until we enable them */
+ vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
/* Put everything in free lists. */
vq->num_free = num;
Index: kvm/include/linux/virtio.h
===================================================================
--- kvm.orig/include/linux/virtio.h
+++ kvm/include/linux/virtio.h
@@ -41,7 +41,7 @@ struct virtqueue
* vq: the struct virtqueue we're talking about.
* len: the length written into the buffer
* Returns NULL or the "data" token handed to add_buf.
- * @restart: restart callbacks after callback returned false.
+ * @enable: restart callbacks after callback returned false.
* vq: the struct virtqueue we're talking about.
* This returns "false" (and doesn't re-enable) if there are pending
* buffers in the queue, to avoid a race.
@@ -65,7 +65,7 @@ struct virtqueue_ops {
void *(*get_buf)(struct virtqueue *vq, unsigned int *len);
- bool (*restart)(struct virtqueue *vq);
+ bool (*enable)(struct virtqueue *vq);
void (*shutdown)(struct virtqueue *vq);
};
Index: kvm/drivers/net/virtio_net.c
===================================================================
--- kvm.orig/drivers/net/virtio_net.c
+++ kvm/drivers/net/virtio_net.c
@@ -201,7 +201,7 @@ again:
/* Out of packets? */
if (received < budget) {
netif_rx_complete(vi->dev, napi);
- if (unlikely(!vi->rvq->vq_ops->restart(vi->rvq))
+ if (unlikely(!vi->rvq->vq_ops->enable(vi->rvq))
&& netif_rx_reschedule(vi->dev, napi))
goto again;
}
@@ -292,6 +292,9 @@ static int virtnet_open(struct net_devic
return -ENOMEM;
napi_enable(&vi->napi);
+
+ vi->rvq->vq_ops->enable(vi->rvq);
+ vi->svq->vq_ops->enable(vi->svq);
return 0;
}
Index: kvm/drivers/block/virtio_blk.c
===================================================================
--- kvm.orig/drivers/block/virtio_blk.c
+++ kvm/drivers/block/virtio_blk.c
@@ -183,7 +183,8 @@ static int virtblk_probe(struct virtio_d
err = PTR_ERR(vblk->vq);
goto out_free_vblk;
}
-
+ /* enable interrupts */
+ vblk->vq->vq_ops->enable(vblk->vq);
vblk->pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
if (!vblk->pool) {
err = -ENOMEM;
^ 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