* Strange CPU load when flushing route cache (kernel 2.6.31.6)
From: robert @ 2009-11-23 15:07 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, Linux Kernel Network Hackers, Robert Olsson
In-Reply-To: <1258970332.29747.262.camel@jdb-workstation>
Jesper Dangaard Brouer writes:
> I have observed a strange route cache behaviour when I upgraded some
> of my production Linux routers (1Gbit/s tg3) to kernel 2.6.31.6 (from
> kernel 2.6.25.7).
>
> Every time the route cache is flushed I get a CPU spike (in softirq)
> with a tail. I have attached some graphs that illustrate the issue
> (hope vger.kernel.org will allow these attachments...)
Nice plots. Yes had the same problem long time. Packets were dropped on
even moderately loaded machines and the network managers were complaining.
Also the are some router benchmarks (RFC??) that estimates the forwarding
performance to the level when the first packet drop occurs. One can of course
discuss such test but it's there...
IMO is best to have he GC "inlined" with the creation of new flows and avoid
periodic tasks in this aspect.
Also I tried with something I called "active" garbage collection. The idea
was to get hints from TCP-FIN etc when to remove stale entries to take a
more pro-active approach. I think this was mentioned in the TRASH-paper.
If you only do routing you might try to disable the route cache.
Cheers
--ro
^ permalink raw reply
* RE: [RFC PATCH 1/4] net: Add support to netdev ops for changing hardware queue MAC and VLAN filters
From: Ben Hutchings @ 2009-11-23 13:22 UTC (permalink / raw)
To: Williams, Mitch A
Cc: Kirsher, Jeffrey T, davem@davemloft.net, shemminger@vyatta.com,
netdev@vger.kernel.org, gospo@redhat.com
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FA6A200AD@rrsmsx501.amr.corp.intel.com>
On Thu, 2009-11-19 at 12:34 -0700, Williams, Mitch A wrote:
> >From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> >Sent: Thursday, November 19, 2009 10:59 AM
>
> >> Please explain specifically what you perceive to be the difference
> >between:
> >>
> >> $ ip link set eth1 queue 1 mac <blah>
> >> $ ip link set eth1 queue 1 vlan <foo>
> >>
> >> and
> >>
> >> $ ip link set eth1 queue 1 mac <blah> vlan <foo>
> >>
> >> The two filter types are, in my mind, completely orthogonal. You can
> >> have one, or the other, or both, or neither. What do we gain by
> >> glomming both options on one command line? And is this worth the
> >> tradeoff of more complex code?
> >
> >I think you need to state clearly what semantics you are now proposing
> >before I can make any judgement on them.
> >
>
> OK, now I'm really confused, Ben. It seems that we are both asking
> each other the same question.
>
> What I'm proposing is really the same as we have now for single-queue
> devices:
>
> - A MAC filter is enabled by default. If you want to change the MAC
> address, you use a tool (ip or ifconfig) to change it.
>
> - A VLAN filter is not enabled by default. If you want to filter on
> VLANs, then you load the 8021q module, and enable a filter.
This doesn't seem quite the same to me, but I'll not argue this.
> The MAC filter is configured completely separately from the VLAN
> filter. Either one can be changed without affecting the other one and,
> in fact, you use two different tools to do these operations.
>
> For SR-IOV VF devices, my proposed changes implement exactly the same
> thing. You use one command to change the MAC address. You use
> another command to change the VLAN filter. Changing one does not
> affect the other.
>
> In this case, we use the same tool for both operations, but they're
> still separate operations.
This makes some sense, and I accept that it's rather different from
filtering for delivery to the host.
> N.B.
> The major difference in VLAN filtering is that this implementation
> allows the VF to participate in only one VLAN, and the filter is
> applied independently of the VF driver. So you can put a specific VM
> on a VLAN without its knowledge. If you want the VM to have more
> intelligent VLAN filtering, you don't use this filter, and you load
> 8021q in the VM and set your filters there.
How does this interact with use of multiple queues within a single
function? Are the specified queue numbers really interpreted as RX
queue indices or as function numbers?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: Strange CPU load when flushing route cache (kernel 2.6.31.6)
From: Eric Dumazet @ 2009-11-23 13:25 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Linux Kernel Network Hackers, Robert Olsson
In-Reply-To: <1258979320.29747.270.camel@jdb-workstation>
Jesper Dangaard Brouer a écrit :
> On Mon, 2009-11-23 at 11:29 +0100, Eric Dumazet wrote:
>> Sure, after a flush, we have to rebuild the cache, so extra work is expected.
>
> But the old 2.6.25.7 do NOT show this behavior... That is the real
> issue...
Previous kernels were crashing, because flush was immediate and not deferred
as today.
During flush, we were dropping enormous amounts of packets.
Now, its possible to have setups with equilibrium and no packet loss,
because we smoothtly invalidate cache entries.
> I did the cache flushing due to some historical issues, that I think you
> did a fix for... Guess I can drop the flushing and see if the garbage
> collection can keep up...
Yes it can. Unless your route cache settings are not optimal.
>
>> Do you run a 2G/2G User/Kernel split kernel ?
>
> Not sure, how do I check?
grep LowTotal /proc/meminfo
or
dmesg | grep LOWMEM
913MB LOWMEM available. (standard 3G/1G User/Kernel split)
^ permalink raw reply
* Re: A generic kernel compatibilty code
From: Ben Hutchings @ 2009-11-23 13:26 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <43e72e890911201307g2a1f280aie223ed4fd270aad@mail.gmail.com>
On Fri, 2009-11-20 at 13:07 -0800, Luis R. Rodriguez wrote:
> On Fri, Nov 20, 2009 at 1:00 PM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> > On Fri, 2009-11-20 at 12:45 -0800, Luis R. Rodriguez wrote:
> >> Everyone and their mother reinvents the wheel when it comes to
> >> backporting kernel modules. It a painful job and it seems to me an
> >> alternative is possible. If we can write generic compatibilty code for
> >> a new routine introduced on the next kernel how about just merging it
> >> to the kernel under some generic compat module. This would be
> >> completey ignored by everyone using the stable kernel but can be
> >> copied by anyone doing backport work.
> >>
> >> So I'm thinking something as simple as a generic compat/comat.ko with
> >> compat-2.6.32.[ch] files.
> >>
> >> We've already backported everything needed for wireless drivers under
> >> compat-wireless under this format down to even 2.6.25.
> > [...]
> >
> > If you think 2.6.25 is old then I don't think you understand the scale
> > of the problem.
> >
> > OEMs still expect us to support RHEL 4 (2.6.9) and SLES 9 (2.6.5) though
> > the latter will probably be dropped soon. Some other vendors apparently
> > still need to support even 2.4 kernels!
>
> Heh understood. Well shouldn't this help with that then? Sure I'd love
> to see the Enteprise Linux releases on 2.6.31 but that's not going to
> happen right? Shouldn't this help then?
You'd really have to ask the 'enterprise' vendors whether they'd be
interested in working on some sort of shared forward-compat library. If
the library is to include a module rather than being statically linked
into each module that needs it then there can only be one instance in
the system.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 0/7] via-velocity performance fixes
From: Simon Kagstrom @ 2009-11-23 13:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, davem, davej, romieu
In-Reply-To: <20091120090348.1e51e29e@nehalam>
Hi Stephen and David!
Thanks for the comments, I'll address these in the next version.
On Fri, 20 Nov 2009 09:03:48 -0800
Stephen Hemminger <shemminger@vyatta.com> wrote:
> > 1. Correct setting of skipped checksums (unsure about this). The
> > mainline driver sets CHECKSUM_UNNECESSARY if this is an IP packet
> > except if the TCP checksum is NOT ok.
> >
> > The VIA driver sets CHECKSUM_UNNECESSARY if this is an UDP/TCP
> > packet except if the TCP checksum is not OK. The patch selects the
> > VIA behavior.
>
> The mainline driver is already doing the correct thing.
> The VIA driver would send packet
> with known bad checksum up the stack with CHECKSUM_UNNECESSARY.
> What is supposed to happen is:
>
> Checksum good: set CHECKSUM_UNNECESSARY
> bad: leave CHECKSUM_NONE
Yes, I see now that mainline is correct and I'll remove the patch from
the upcoming version 2 of the series. However, I don't think the driver
would send bad packets upwards - it would rather be more conservative.
What the current driver does is
ip_summed = CHECKSUM_NONE;
if (hardware sees that this is an IP packet):
if (hw IP checksum is OK):
if (hw sees that this is a TCP/UDP packet):
if (hw TCP/UDP checksum is NOT OK):
return # Returning CHECKSUM_NONE
ip_summed = CHECKSUM_UNNECESSARY
while the VIA driver does
ip_summed = CHECKSUM_NONE;
if (hardware sees that this is an IP packet):
if (hw IP checksum is OK):
if (hw sees that this is a TCP/UDP packet):
if (hw TCP/UDP checksum is NOT OK):
return # Returning CHECKSUM_NONE
ip_summed = CHECKSUM_UNNECESSARY
i.e., it will return CHECKSUM_NONE also for non-TCP/UDP IP packets.
// Simon
^ permalink raw reply
* Re: Strange CPU load when flushing route cache (kernel 2.6.31.6)
From: Jesper Dangaard Brouer @ 2009-11-23 13:48 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jesper Dangaard Brouer, Linux Kernel Network Hackers,
Robert Olsson
In-Reply-To: <4B0A8D41.6010608@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1870 bytes --]
On Mon, 23 Nov 2009, Eric Dumazet wrote:
> Jesper Dangaard Brouer a écrit :
>> On Mon, 2009-11-23 at 11:29 +0100, Eric Dumazet wrote:
>
>>> Sure, after a flush, we have to rebuild the cache, so extra work is expected.
>>
>> But the old 2.6.25.7 do NOT show this behavior... That is the real
>> issue...
>
> Previous kernels were crashing, because flush was immediate and not deferred
> as today.
>
> During flush, we were dropping enormous amounts of packets.
Ahh... Now I remember that was why I was flushing the cache so often. If
I flushed the route cache before it got too big then it was not a
problem with packet drops occuring.
> Now, its possible to have setups with equilibrium and no packet loss,
> because we smoothtly invalidate cache entries.
Which is a good thing :-)
>> I did the cache flushing due to some historical issues, that I think you
>> did a fix for... Guess I can drop the flushing and see if the garbage
>> collection can keep up...
>
> Yes it can. Unless your route cache settings are not optimal.
I'll adjust my flushing interval, or disable it and monitor it.
>>> Do you run a 2G/2G User/Kernel split kernel ?
>>
>> Not sure, how do I check?
>
> grep LowTotal /proc/meminfo
Yes, guess I'm using User/Kernel split.
grep LowTotal /proc/meminfo
LowTotal: 747080 kB
What does that mean? Is it bad? What should I run on a 32-bit
system/kernel?
Can you recommend any other /proc/sys/ tuning options?
Does my kernel boot option rhash_entries=262143 make sense anymore?
Or do we adjust the hash bucket size dynamically these days?
Cheers,
Jesper Brouer
--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
^ permalink raw reply
* Re: Strange CPU load when flushing route cache (kernel 2.6.31.6)
From: Eric Dumazet @ 2009-11-23 14:03 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Jesper Dangaard Brouer, Linux Kernel Network Hackers,
Robert Olsson
In-Reply-To: <Pine.LNX.4.64.0911231437020.24347@ask.diku.dk>
> grep LowTotal /proc/meminfo
> LowTotal: 747080 kB
>
> What does that mean? Is it bad? What should I run on a 32-bit
> system/kernel?
If you have more than 1GB of physical ram, and use your machine as a router, you might
compile a 2GB/2GB User/Kernel kernel, to get twice available RAM for kernel
and more IP route entries (if needed)
make menuconfig
--> Processor type and features
--> Memory split
--> 2G/2G
> Can you recommend any other /proc/sys/ tuning options?
Really hard to say without exact context of use :)
>
> Does my kernel boot option rhash_entries=262143 make sense anymore?
> Or do we adjust the hash bucket size dynamically these days?
>
Its not dynamic yet.
^ permalink raw reply
* Re: ixgbe question
From: Eric Dumazet @ 2009-11-23 14:05 UTC (permalink / raw)
To: Waskiewicz Jr, Peter P; +Cc: Linux Netdev List
In-Reply-To: <4B0A65E0.7060403@gmail.com>
Eric Dumazet a écrit :
> Waskiewicz Jr, Peter P a écrit :
>> On Mon, 23 Nov 2009, Eric Dumazet wrote:
>>
>>> Hi Peter
>>>
>>> I tried a pktgen stress on 82599EB card and could not split RX load on multiple cpus.
>>>
>>> Setup is :
>>>
>>> One 82599 card with fiber0 looped to fiber1, 10Gb link mode.
>>> machine is a HPDL380 G6 with dual quadcore E5530 @2.4GHz (16 logical cpus)
>> Can you specify kernel version and driver version?
>
>
> Well, I forgot to mention I am only working with net-next-2.6 tree.
>
> Ubuntu 9.10 kernel (Fedora Core 12 installer was not able to recognize disks on this machine :( )
>
> ixgbe: Intel(R) 10 Gigabit PCI Express Network Driver - version 2.0.44-k2
>
>
I tried with several pktgen threads, no success so far.
Only one cpu handles all interrupts and ksoftirq enters
a mode with no escape to splitted mode.
To get real multi queue and uncontended handling, I had to force :
echo 1 >`echo /proc/irq/*/fiber1-TxRx-0/../smp_affinity`
echo 2 >`echo /proc/irq/*/fiber1-TxRx-1/../smp_affinity`
echo 4 >`echo /proc/irq/*/fiber1-TxRx-2/../smp_affinity`
echo 8 >`echo /proc/irq/*/fiber1-TxRx-3/../smp_affinity`
echo 10 >`echo /proc/irq/*/fiber1-TxRx-4/../smp_affinity`
echo 20 >`echo /proc/irq/*/fiber1-TxRx-5/../smp_affinity`
echo 40 >`echo /proc/irq/*/fiber1-TxRx-6/../smp_affinity`
echo 80 >`echo /proc/irq/*/fiber1-TxRx-7/../smp_affinity`
echo 100 >`echo /proc/irq/*/fiber1-TxRx-8/../smp_affinity`
echo 200 >`echo /proc/irq/*/fiber1-TxRx-9/../smp_affinity`
echo 400 >`echo /proc/irq/*/fiber1-TxRx-10/../smp_affinity`
echo 800 >`echo /proc/irq/*/fiber1-TxRx-11/../smp_affinity`
echo 1000 >`echo /proc/irq/*/fiber1-TxRx-12/../smp_affinity`
echo 2000 >`echo /proc/irq/*/fiber1-TxRx-13/../smp_affinity`
echo 4000 >`echo /proc/irq/*/fiber1-TxRx-14/../smp_affinity`
echo 8000 >`echo /proc/irq/*/fiber1-TxRx-15/../smp_affinity`
Probably problem comes from fact that when ksoftirqd runs and
RX queues are not depleted, no hardware interrupts is sent,
and NAPI contexts stay sticked on one cpu forever ?
^ permalink raw reply
* Re: ixgbe question
From: Jesper Dangaard Brouer @ 2009-11-23 14:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Peter P Waskiewicz Jr, Linux Netdev List
In-Reply-To: <4B0A6218.9040303@gmail.com>
On Mon, 23 Nov 2009, Eric Dumazet wrote:
> I tried a pktgen stress on 82599EB card and could not split RX load on multiple cpus.
>
> Setup is :
>
> One 82599 card with fiber0 looped to fiber1, 10Gb link mode.
> machine is a HPDL380 G6 with dual quadcore E5530 @2.4GHz (16 logical cpus)
>
> I use one pktgen thread sending to fiber0 one many dst IP, and checked that fiber1
> was using many RX queues :
How is your smp_affinity mask's set?
grep . /proc/irq/*/fiber1-*/../smp_affinity
> But only one CPU (CPU1) had a softirq running, 100%, and many frames were dropped
Just a hint, I use 'ethtool -S fiber1' to see how the packets gets
distributed across the rx and tx queues.
> CLONE_SKB="clone_skb 15"
Be careful with to high clone, as my experience is it will send a burst of
clone_skb packets before the packet gets randomized again.
> pgset "dst_min 192.168.0.2"
> pgset "dst_max 192.168.0.250"
> pgset "src_min 192.168.0.1"
> pgset "src_max 192.168.0.1"
> pgset "dst_mac 00:1b:21:4a:fe:55"
To get a packets randomized across RX queues, I used:
echo "- Random UDP source port min:$min - max:$max"
pgset "flag UDPSRC_RND"
pgset "udp_src_min $min"
pgset "udp_src_max $max"
Ahh.. I think you are missing:
pgset "flag IPDST_RND"
Cheers,
Jesper Brouer
--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
^ permalink raw reply
* [PATCH v2 0/7] via-velocity performance fixes
From: Simon Kagstrom @ 2009-11-23 14:27 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091120160633.77b7aee0@marrow.netinsight.se>
Hi again!
This is version two of the series posted here:
http://marc.info/?l=linux-netdev&m=125873023117769&w=2
The new version tries to integrate Stephen and Davids comments, and
also some general improvements which I've done. There are 7 patches:
1. See to it that data is 64-byte aligned (same as before)
2. Add ethtool interrupt coalescing support. This is the old "adaptive
interrupt suppression" patch, which is now implemented through the
ethtool interface. Both tx/rx frames and tx/rx time can be
controlled. The module parameters have been removed.
The default is everything off (i.e., as before).
3. NAPI support for via-velocity (same as before, but rebased)
4. Change DMA length default (same as before, rebased)
5. Re-enable scatter-gather support. This is also implemented through
ethtool, and therefore needs to be turned on. It's not always a win,
but for some netperf runs it improves stuff.
6. Set tx checksum from ethtool instead of a module parameter (new
patch)
7. Bump the version.
To get the same performance results as before, you now need to setup
interrupt supression with
ethtool -C eth-swa rx-usecs 20 tx-usecs 100 tx-frames 31 rx-frames 31
(which are the defaults from the VIA driver)
// Simon
^ permalink raw reply
* [PATCH v2 1/7] via-velocity: Correct 64-byte alignment for rx buffers
From: Simon Kagstrom @ 2009-11-23 14:30 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
(From the VIA driver). The current code does not guarantee 64-byte
alignment since it simply does
int add = skb->data & 63;
skb->data += add;
(via skb_reserve). So for example, if the skb->data address would be
0x10, this would result in 32-byte alignment (0x10 + 0x10).
Correct by adding
64 - (skb->data & 63)
instead.
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index e04e5be..b6cf3b5 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -1483,7 +1483,8 @@ static int velocity_alloc_rx_buf(struct velocity_info *vptr, int idx)
* Do the gymnastics to get the buffer head for data at
* 64byte alignment.
*/
- skb_reserve(rd_info->skb, (unsigned long) rd_info->skb->data & 63);
+ skb_reserve(rd_info->skb,
+ 64 - ((unsigned long) rd_info->skb->data & 63));
rd_info->skb_dma = pci_map_single(vptr->pdev, rd_info->skb->data,
vptr->rx.buf_sz, PCI_DMA_FROMDEVICE);
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 2/7] via-velocity: Add ethtool interrupt coalescing support
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
(Partially from the upstream VIA driver). Tweaking the number of
frames-per-interrupt and timer-until-interrupt can reduce the amount of
CPU work quite a lot.
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 160 +++++++++++++++++++++++++++++++++++++++++++-
drivers/net/via-velocity.h | 7 ++-
2 files changed, 165 insertions(+), 2 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index b6cf3b5..85ee898 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -1259,6 +1259,66 @@ static void mii_init(struct velocity_info *vptr, u32 mii_status)
}
}
+/**
+ * setup_queue_timers - Setup interrupt timers
+ *
+ * Setup interrupt frequency during suppression (timeout if the frame
+ * count isn't filled).
+ */
+static void setup_queue_timers(struct velocity_info *vptr)
+{
+ /* Only for newer revisions */
+ if (vptr->rev_id >= REV_ID_VT3216_A0) {
+ u8 txqueue_timer = 0;
+ u8 rxqueue_timer = 0;
+
+ if (vptr->mii_status & (VELOCITY_SPEED_1000 |
+ VELOCITY_SPEED_100)) {
+ txqueue_timer = vptr->options.txqueue_timer;
+ rxqueue_timer = vptr->options.rxqueue_timer;
+ }
+
+ writeb(txqueue_timer, &vptr->mac_regs->TQETMR);
+ writeb(rxqueue_timer, &vptr->mac_regs->RQETMR);
+ }
+}
+/**
+ * setup_adaptive_interrupts - Setup interrupt suppression
+ *
+ * @vptr velocity adapter
+ *
+ * The velocity is able to suppress interrupt during high interrupt load.
+ * This function turns on that feature.
+ */
+static void setup_adaptive_interrupts(struct velocity_info *vptr)
+{
+ struct mac_regs __iomem *regs = vptr->mac_regs;
+ u16 tx_intsup = vptr->options.tx_intsup;
+ u16 rx_intsup = vptr->options.rx_intsup;
+
+ /* Setup default interrupt mask (will be changed below) */
+ vptr->int_mask = INT_MASK_DEF;
+
+ /* Set Tx Interrupt Suppression Threshold */
+ writeb(CAMCR_PS0, ®s->CAMCR);
+ if (tx_intsup != 0) {
+ vptr->int_mask &= ~(ISR_PTXI | ISR_PTX0I | ISR_PTX1I |
+ ISR_PTX2I | ISR_PTX3I);
+ writew(tx_intsup, ®s->ISRCTL);
+ } else
+ writew(ISRCTL_TSUPDIS, ®s->ISRCTL);
+
+ /* Set Rx Interrupt Suppression Threshold */
+ writeb(CAMCR_PS1, ®s->CAMCR);
+ if (rx_intsup != 0) {
+ vptr->int_mask &= ~ISR_PRXI;
+ writew(rx_intsup, ®s->ISRCTL);
+ } else
+ writew(ISRCTL_RSUPDIS, ®s->ISRCTL);
+
+ /* Select page to interrupt hold timer */
+ writeb(0, ®s->CAMCR);
+}
/**
* velocity_init_registers - initialise MAC registers
@@ -1345,7 +1405,7 @@ static void velocity_init_registers(struct velocity_info *vptr,
*/
enable_mii_autopoll(regs);
- vptr->int_mask = INT_MASK_DEF;
+ setup_adaptive_interrupts(vptr);
writel(vptr->rx.pool_dma, ®s->RDBaseLo);
writew(vptr->options.numrx - 1, ®s->RDCSize);
@@ -1802,6 +1862,8 @@ static void velocity_error(struct velocity_info *vptr, int status)
BYTE_REG_BITS_OFF(TESTCFG_HBDIS, ®s->TESTCFG);
else
BYTE_REG_BITS_ON(TESTCFG_HBDIS, ®s->TESTCFG);
+
+ setup_queue_timers(vptr);
}
/*
* Get link status from PHYSR0
@@ -3223,6 +3285,100 @@ static void velocity_set_msglevel(struct net_device *dev, u32 value)
msglevel = value;
}
+static int get_pending_timer_val(int val)
+{
+ int mult_bits = val >> 6;
+ int mult = 1;
+
+ switch (mult_bits)
+ {
+ case 1:
+ mult = 4; break;
+ case 2:
+ mult = 16; break;
+ case 3:
+ mult = 64; break;
+ case 0:
+ default:
+ break;
+ }
+
+ return (val & 0x3f) * mult;
+}
+
+static void set_pending_timer_val(int *val, u32 us)
+{
+ u8 mult = 0;
+ u8 shift = 0;
+
+ if (us >= 0x3f) {
+ mult = 1; /* mult with 4 */
+ shift = 2;
+ }
+ if (us >= 0x3f * 4) {
+ mult = 2; /* mult with 16 */
+ shift = 4;
+ }
+ if (us >= 0x3f * 16) {
+ mult = 3; /* mult with 64 */
+ shift = 6;
+ }
+
+ *val = (mult << 6) | ((us >> shift) & 0x3f);
+}
+
+
+static int velocity_get_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ecmd)
+{
+ struct velocity_info *vptr = netdev_priv(dev);
+
+ ecmd->tx_max_coalesced_frames = vptr->options.tx_intsup;
+ ecmd->rx_max_coalesced_frames = vptr->options.rx_intsup;
+
+ ecmd->rx_coalesce_usecs = get_pending_timer_val(vptr->options.rxqueue_timer);
+ ecmd->tx_coalesce_usecs = get_pending_timer_val(vptr->options.txqueue_timer);
+
+ return 0;
+}
+
+static int velocity_set_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ecmd)
+{
+ struct velocity_info *vptr = netdev_priv(dev);
+ int max_us = 0x3f * 64;
+
+ /* 6 bits of */
+ if (ecmd->tx_coalesce_usecs > max_us)
+ return -EINVAL;
+ if (ecmd->rx_coalesce_usecs > max_us)
+ return -EINVAL;
+
+ if (ecmd->tx_max_coalesced_frames > 0xff)
+ return -EINVAL;
+ if (ecmd->rx_max_coalesced_frames > 0xff)
+ return -EINVAL;
+
+ vptr->options.rx_intsup = ecmd->rx_max_coalesced_frames;
+ vptr->options.tx_intsup = ecmd->tx_max_coalesced_frames;
+
+ set_pending_timer_val(&vptr->options.rxqueue_timer,
+ ecmd->rx_coalesce_usecs);
+ set_pending_timer_val(&vptr->options.txqueue_timer,
+ ecmd->tx_coalesce_usecs);
+
+ /* Setup the interrupt suppression and queue timers */
+ mac_disable_int(vptr->mac_regs);
+ setup_adaptive_interrupts(vptr);
+ setup_queue_timers(vptr);
+
+ mac_write_int_mask(vptr->int_mask, vptr->mac_regs);
+ mac_clear_isr(vptr->mac_regs);
+ mac_enable_int(vptr->mac_regs);
+
+ return 0;
+}
+
static const struct ethtool_ops velocity_ethtool_ops = {
.get_settings = velocity_get_settings,
.set_settings = velocity_set_settings,
@@ -3232,6 +3388,8 @@ static const struct ethtool_ops velocity_ethtool_ops = {
.get_msglevel = velocity_get_msglevel,
.set_msglevel = velocity_set_msglevel,
.get_link = velocity_get_link,
+ .get_coalesce = velocity_get_coalesce,
+ .set_coalesce = velocity_set_coalesce,
.begin = velocity_ethtool_up,
.complete = velocity_ethtool_down
};
diff --git a/drivers/net/via-velocity.h b/drivers/net/via-velocity.h
index 2f00c13..6091946 100644
--- a/drivers/net/via-velocity.h
+++ b/drivers/net/via-velocity.h
@@ -1005,7 +1005,8 @@ struct mac_regs {
volatile __le32 RDBaseLo; /* 0x38 */
volatile __le16 RDIdx; /* 0x3C */
- volatile __le16 reserved_3E;
+ volatile u8 TQETMR; /* 0x3E, VT3216 and above only */
+ volatile u8 RQETMR; /* 0x3F, VT3216 and above only */
volatile __le32 TDBaseLo[4]; /* 0x40 */
@@ -1491,6 +1492,10 @@ struct velocity_opt {
int rx_bandwidth_hi;
int rx_bandwidth_lo;
int rx_bandwidth_en;
+ int rxqueue_timer;
+ int txqueue_timer;
+ int tx_intsup;
+ int rx_intsup;
u32 flags;
};
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 3/7] via-velocity: Implement NAPI support
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
This patch adds NAPI support for VIA velocity. The new velocity_poll
function also pairs tx/rx handling twice which improves perforamance on
some workloads (e.g., netperf UDP_STREAM) significantly (that part is
from the VIA driver).
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 81 +++++++++++++++++++++++++-------------------
drivers/net/via-velocity.h | 3 ++
2 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 85ee898..668cdf6 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -354,12 +354,6 @@ VELOCITY_PARAM(ValPktLen, "Receiving or Drop invalid 802.3 frame");
*/
VELOCITY_PARAM(wol_opts, "Wake On Lan options");
-#define INT_WORKS_DEF 20
-#define INT_WORKS_MIN 10
-#define INT_WORKS_MAX 64
-
-VELOCITY_PARAM(int_works, "Number of packets per interrupt services");
-
static int rx_copybreak = 200;
module_param(rx_copybreak, int, 0644);
MODULE_PARM_DESC(rx_copybreak, "Copy breakpoint for copy-only-tiny-frames");
@@ -516,7 +510,6 @@ static void __devinit velocity_get_options(struct velocity_opt *opts, int index,
velocity_set_bool_opt(&opts->flags, ValPktLen[index], VAL_PKT_LEN_DEF, VELOCITY_FLAGS_VAL_PKT_LEN, "ValPktLen", devname);
velocity_set_int_opt((int *) &opts->spd_dpx, speed_duplex[index], MED_LNK_MIN, MED_LNK_MAX, MED_LNK_DEF, "Media link mode", devname);
velocity_set_int_opt((int *) &opts->wol_opts, wol_opts[index], WOL_OPT_MIN, WOL_OPT_MAX, WOL_OPT_DEF, "Wake On Lan options", devname);
- velocity_set_int_opt((int *) &opts->int_works, int_works[index], INT_WORKS_MIN, INT_WORKS_MAX, INT_WORKS_DEF, "Interrupt service works", devname);
opts->numrx = (opts->numrx & ~3);
}
@@ -2123,13 +2116,14 @@ static int velocity_receive_frame(struct velocity_info *vptr, int idx)
* any received packets from the receive queue. Hand the ring
* slots back to the adapter for reuse.
*/
-static int velocity_rx_srv(struct velocity_info *vptr, int status)
+static int velocity_rx_srv(struct velocity_info *vptr, int status,
+ int budget_left)
{
struct net_device_stats *stats = &vptr->dev->stats;
int rd_curr = vptr->rx.curr;
int works = 0;
- do {
+ while (works < budget_left) {
struct rx_desc *rd = vptr->rx.ring + rd_curr;
if (!vptr->rx.info[rd_curr].skb)
@@ -2160,7 +2154,8 @@ static int velocity_rx_srv(struct velocity_info *vptr, int status)
rd_curr++;
if (rd_curr >= vptr->options.numrx)
rd_curr = 0;
- } while (++works <= 15);
+ works++;
+ }
vptr->rx.curr = rd_curr;
@@ -2171,6 +2166,40 @@ static int velocity_rx_srv(struct velocity_info *vptr, int status)
return works;
}
+static int velocity_poll(struct napi_struct *napi, int budget)
+{
+ struct velocity_info *vptr = container_of(napi,
+ struct velocity_info, napi);
+ unsigned int rx_done;
+ u32 isr_status;
+
+ spin_lock(&vptr->lock);
+ isr_status = mac_read_isr(vptr->mac_regs);
+
+ /* Ack the interrupt */
+ mac_write_isr(vptr->mac_regs, isr_status);
+ if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
+ velocity_error(vptr, isr_status);
+
+ /*
+ * Do rx and tx twice for performance (taken from the VIA
+ * out-of-tree driver).
+ */
+ rx_done = velocity_rx_srv(vptr, isr_status, budget / 2);
+ velocity_tx_srv(vptr, isr_status);
+ rx_done += velocity_rx_srv(vptr, isr_status, budget - rx_done);
+ velocity_tx_srv(vptr, isr_status);
+
+ spin_unlock(&vptr->lock);
+
+ /* If budget not fully consumed, exit the polling mode */
+ if (rx_done < budget) {
+ napi_complete(napi);
+ mac_enable_int(vptr->mac_regs);
+ }
+
+ return rx_done;
+}
/**
* velocity_intr - interrupt callback
@@ -2187,8 +2216,6 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance)
struct net_device *dev = dev_instance;
struct velocity_info *vptr = netdev_priv(dev);
u32 isr_status;
- int max_count = 0;
-
spin_lock(&vptr->lock);
isr_status = mac_read_isr(vptr->mac_regs);
@@ -2199,32 +2226,13 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance)
return IRQ_NONE;
}
- mac_disable_int(vptr->mac_regs);
-
- /*
- * Keep processing the ISR until we have completed
- * processing and the isr_status becomes zero
- */
-
- while (isr_status != 0) {
- mac_write_isr(vptr->mac_regs, isr_status);
- if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
- velocity_error(vptr, isr_status);
- if (isr_status & (ISR_PRXI | ISR_PPRXI))
- max_count += velocity_rx_srv(vptr, isr_status);
- if (isr_status & (ISR_PTXI | ISR_PPTXI))
- max_count += velocity_tx_srv(vptr, isr_status);
- isr_status = mac_read_isr(vptr->mac_regs);
- if (max_count > vptr->options.int_works) {
- printk(KERN_WARNING "%s: excessive work at interrupt.\n",
- dev->name);
- max_count = 0;
- }
+ if (likely(napi_schedule_prep(&vptr->napi))) {
+ mac_disable_int(vptr->mac_regs);
+ __napi_schedule(&vptr->napi);
}
spin_unlock(&vptr->lock);
- mac_enable_int(vptr->mac_regs);
- return IRQ_HANDLED;
+ return IRQ_HANDLED;
}
/**
@@ -2264,6 +2272,7 @@ static int velocity_open(struct net_device *dev)
mac_enable_int(vptr->mac_regs);
netif_start_queue(dev);
+ napi_enable(&vptr->napi);
vptr->flags |= VELOCITY_FLAGS_OPENED;
out:
return ret;
@@ -2499,6 +2508,7 @@ static int velocity_close(struct net_device *dev)
{
struct velocity_info *vptr = netdev_priv(dev);
+ napi_disable(&vptr->napi);
netif_stop_queue(dev);
velocity_shutdown(vptr);
@@ -2818,6 +2828,7 @@ static int __devinit velocity_found1(struct pci_dev *pdev, const struct pci_devi
dev->irq = pdev->irq;
dev->netdev_ops = &velocity_netdev_ops;
dev->ethtool_ops = &velocity_ethtool_ops;
+ netif_napi_add(dev, &vptr->napi, velocity_poll, VELOCITY_NAPI_WEIGHT);
dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER |
NETIF_F_HW_VLAN_RX;
diff --git a/drivers/net/via-velocity.h b/drivers/net/via-velocity.h
index 6091946..22bfea4 100644
--- a/drivers/net/via-velocity.h
+++ b/drivers/net/via-velocity.h
@@ -32,6 +32,7 @@
#define VELOCITY_VERSION "1.14"
#define VELOCITY_IO_SIZE 256
+#define VELOCITY_NAPI_WEIGHT 64
#define PKT_BUF_SZ 1540
@@ -1564,6 +1565,8 @@ struct velocity_info {
u32 ticks;
u8 rev_id;
+
+ struct napi_struct napi;
};
/**
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 4/7] via-velocity: Change DMA_LENGTH_DEF (from the VIA driver)
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
The VIA driver has changed the default for the DMA_LENGTH_DEF parameter.
Together with adaptive interrupt supression and NAPI support, this
improves performance quite a bit
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 668cdf6..06a6d80 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -275,7 +275,7 @@ VELOCITY_PARAM(rx_thresh, "Receive fifo threshold");
#define DMA_LENGTH_MIN 0
#define DMA_LENGTH_MAX 7
-#define DMA_LENGTH_DEF 0
+#define DMA_LENGTH_DEF 6
/* DMA_length[] is used for controlling the DMA length
0: 8 DWORDs
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 5/7] via-velocity: Re-enable transmit scatter-gather support
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
The velocity hardware can handle up to 7 memory segments. This can be
turned on and off via ethtool.
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 88 +++++++++++++++++++++++++++++---------------
1 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 06a6d80..92b41fc 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -9,7 +9,6 @@
*
* TODO
* rx_copybreak/alignment
- * Scatter gather
* More testing
*
* The changes are (c) Copyright 2004, Red Hat Inc. <alan@lxorguk.ukuu.org.uk>
@@ -1656,12 +1655,10 @@ out:
*/
static int velocity_init_td_ring(struct velocity_info *vptr)
{
- dma_addr_t curr;
int j;
/* Init the TD ring entries */
for (j = 0; j < vptr->tx.numq; j++) {
- curr = vptr->tx.pool_dma[j];
vptr->tx.infos[j] = kcalloc(vptr->options.numtx,
sizeof(struct velocity_td_info),
@@ -1727,21 +1724,27 @@ err_free_dma_rings_0:
* Release an transmit buffer. If the buffer was preallocated then
* recycle it, if not then unmap the buffer.
*/
-static void velocity_free_tx_buf(struct velocity_info *vptr, struct velocity_td_info *tdinfo)
+static void velocity_free_tx_buf(struct velocity_info *vptr,
+ struct velocity_td_info *tdinfo, struct tx_desc *td)
{
struct sk_buff *skb = tdinfo->skb;
- int i;
- int pktlen;
/*
* Don't unmap the pre-allocated tx_bufs
*/
if (tdinfo->skb_dma) {
+ int i;
- pktlen = max_t(unsigned int, skb->len, ETH_ZLEN);
for (i = 0; i < tdinfo->nskb_dma; i++) {
- pci_unmap_single(vptr->pdev, tdinfo->skb_dma[i], pktlen, PCI_DMA_TODEVICE);
- tdinfo->skb_dma[i] = 0;
+ size_t pktlen = max_t(size_t, skb->len, ETH_ZLEN);
+
+ /* For scatter-gather */
+ if (skb_shinfo(skb)->nr_frags > 0)
+ pktlen = max_t(size_t, pktlen,
+ td->td_buf[i].size & ~TD_QUEUE);
+
+ pci_unmap_single(vptr->pdev, tdinfo->skb_dma[i],
+ le16_to_cpu(pktlen), PCI_DMA_TODEVICE);
}
}
dev_kfree_skb_irq(skb);
@@ -1943,7 +1946,7 @@ static int velocity_tx_srv(struct velocity_info *vptr, u32 status)
stats->tx_packets++;
stats->tx_bytes += tdinfo->skb->len;
}
- velocity_free_tx_buf(vptr, tdinfo);
+ velocity_free_tx_buf(vptr, tdinfo, td);
vptr->tx.used[qnum]--;
}
vptr->tx.tail[qnum] = idx;
@@ -2543,14 +2546,27 @@ static netdev_tx_t velocity_xmit(struct sk_buff *skb,
struct velocity_td_info *tdinfo;
unsigned long flags;
int pktlen;
- __le16 len;
- int index;
+ int index, prev;
+ int i = 0;
if (skb_padto(skb, ETH_ZLEN))
goto out;
- pktlen = max_t(unsigned int, skb->len, ETH_ZLEN);
- len = cpu_to_le16(pktlen);
+ /* The hardware can handle at most 7 memory segments, so merge
+ * the skb if there are more */
+ if (skb_shinfo(skb)->nr_frags > 6 && __skb_linearize(skb)) {
+ kfree_skb(skb);
+ return 0;
+ }
+ /* If it's still above 6 we can't do anything */
+ if (skb_shinfo(skb)->nr_frags > 6) {
+ dev_err(&vptr->pdev->dev,
+ "via-velocity: more than 6 frags, can't send.\n");
+ return 0;
+ }
+ pktlen = skb_shinfo(skb)->nr_frags == 0 ?
+ max_t(unsigned int, skb->len, ETH_ZLEN) :
+ skb_headlen(skb);
spin_lock_irqsave(&vptr->lock, flags);
@@ -2567,11 +2583,24 @@ static netdev_tx_t velocity_xmit(struct sk_buff *skb,
*/
tdinfo->skb = skb;
tdinfo->skb_dma[0] = pci_map_single(vptr->pdev, skb->data, pktlen, PCI_DMA_TODEVICE);
- td_ptr->tdesc0.len = len;
+ td_ptr->tdesc0.len = cpu_to_le16(pktlen);
td_ptr->td_buf[0].pa_low = cpu_to_le32(tdinfo->skb_dma[0]);
td_ptr->td_buf[0].pa_high = 0;
- td_ptr->td_buf[0].size = len;
- tdinfo->nskb_dma = 1;
+ td_ptr->td_buf[0].size = cpu_to_le16(pktlen);
+
+ /* Handle fragments */
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+ tdinfo->skb_dma[i + 1] = pci_map_page(vptr->pdev, frag->page,
+ frag->page_offset, frag->size,
+ PCI_DMA_TODEVICE);
+
+ td_ptr->td_buf[i + 1].pa_low = cpu_to_le32(tdinfo->skb_dma[i + 1]);
+ td_ptr->td_buf[i + 1].pa_high = 0;
+ td_ptr->td_buf[i + 1].size = cpu_to_le16(frag->size);
+ }
+ tdinfo->nskb_dma = i + 1;
td_ptr->tdesc1.cmd = TCPLS_NORMAL + (tdinfo->nskb_dma + 1) * 16;
@@ -2592,23 +2621,21 @@ static netdev_tx_t velocity_xmit(struct sk_buff *skb,
td_ptr->tdesc1.TCR |= (TCR0_UDPCK);
td_ptr->tdesc1.TCR |= TCR0_IPCK;
}
- {
- int prev = index - 1;
+ prev = index - 1;
+ if (prev < 0)
+ prev = vptr->options.numtx - 1;
+ td_ptr->tdesc0.len |= OWNED_BY_NIC;
+ vptr->tx.used[qnum]++;
+ vptr->tx.curr[qnum] = (index + 1) % vptr->options.numtx;
- if (prev < 0)
- prev = vptr->options.numtx - 1;
- td_ptr->tdesc0.len |= OWNED_BY_NIC;
- vptr->tx.used[qnum]++;
- vptr->tx.curr[qnum] = (index + 1) % vptr->options.numtx;
+ if (AVAIL_TD(vptr, qnum) < 1)
+ netif_stop_queue(dev);
- if (AVAIL_TD(vptr, qnum) < 1)
- netif_stop_queue(dev);
+ td_ptr = &(vptr->tx.rings[qnum][prev]);
+ td_ptr->td_buf[0].size |= TD_QUEUE;
+ mac_tx_queue_wake(vptr->mac_regs, qnum);
- td_ptr = &(vptr->tx.rings[qnum][prev]);
- td_ptr->td_buf[0].size |= TD_QUEUE;
- mac_tx_queue_wake(vptr->mac_regs, qnum);
- }
dev->trans_start = jiffies;
spin_unlock_irqrestore(&vptr->lock, flags);
out:
@@ -3398,6 +3425,7 @@ static const struct ethtool_ops velocity_ethtool_ops = {
.set_wol = velocity_ethtool_set_wol,
.get_msglevel = velocity_get_msglevel,
.set_msglevel = velocity_set_msglevel,
+ .set_sg = ethtool_op_set_sg,
.get_link = velocity_get_link,
.get_coalesce = velocity_get_coalesce,
.set_coalesce = velocity_set_coalesce,
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 6/7] via-velocity: Set tx checksum from ethtool instead of module parameter
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
Defaults to on (as before).
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.c | 18 ++++--------------
drivers/net/via-velocity.h | 1 -
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 92b41fc..4af16ba 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -297,14 +297,6 @@ VELOCITY_PARAM(DMA_length, "DMA length");
*/
VELOCITY_PARAM(IP_byte_align, "Enable IP header dword aligned");
-#define TX_CSUM_DEF 1
-/* txcsum_offload[] is used for setting the checksum offload ability of NIC.
- (We only support RX checksum offload now)
- 0: disable csum_offload[checksum offload
- 1: enable checksum offload. (Default)
-*/
-VELOCITY_PARAM(txcsum_offload, "Enable transmit packet checksum offload");
-
#define FLOW_CNTL_DEF 1
#define FLOW_CNTL_MIN 1
#define FLOW_CNTL_MAX 5
@@ -503,7 +495,6 @@ static void __devinit velocity_get_options(struct velocity_opt *opts, int index,
velocity_set_int_opt(&opts->numrx, RxDescriptors[index], RX_DESC_MIN, RX_DESC_MAX, RX_DESC_DEF, "RxDescriptors", devname);
velocity_set_int_opt(&opts->numtx, TxDescriptors[index], TX_DESC_MIN, TX_DESC_MAX, TX_DESC_DEF, "TxDescriptors", devname);
- velocity_set_bool_opt(&opts->flags, txcsum_offload[index], TX_CSUM_DEF, VELOCITY_FLAGS_TX_CSUM, "txcsum_offload", devname);
velocity_set_int_opt(&opts->flow_cntl, flow_control[index], FLOW_CNTL_MIN, FLOW_CNTL_MAX, FLOW_CNTL_DEF, "flow_control", devname);
velocity_set_bool_opt(&opts->flags, IP_byte_align[index], IP_ALIG_DEF, VELOCITY_FLAGS_IP_ALIGN, "IP_byte_align", devname);
velocity_set_bool_opt(&opts->flags, ValPktLen[index], VAL_PKT_LEN_DEF, VELOCITY_FLAGS_VAL_PKT_LEN, "ValPktLen", devname);
@@ -2612,7 +2603,7 @@ static netdev_tx_t velocity_xmit(struct sk_buff *skb,
/*
* Handle hardware checksum
*/
- if ((vptr->flags & VELOCITY_FLAGS_TX_CSUM)
+ if ( (dev->features & NETIF_F_IP_CSUM)
&& (skb->ip_summed == CHECKSUM_PARTIAL)) {
const struct iphdr *ip = ip_hdr(skb);
if (ip->protocol == IPPROTO_TCP)
@@ -2858,10 +2849,7 @@ static int __devinit velocity_found1(struct pci_dev *pdev, const struct pci_devi
netif_napi_add(dev, &vptr->napi, velocity_poll, VELOCITY_NAPI_WEIGHT);
dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER |
- NETIF_F_HW_VLAN_RX;
-
- if (vptr->flags & VELOCITY_FLAGS_TX_CSUM)
- dev->features |= NETIF_F_IP_CSUM;
+ NETIF_F_HW_VLAN_RX | NETIF_F_IP_CSUM;
ret = register_netdev(dev);
if (ret < 0)
@@ -3421,6 +3409,8 @@ static const struct ethtool_ops velocity_ethtool_ops = {
.get_settings = velocity_get_settings,
.set_settings = velocity_set_settings,
.get_drvinfo = velocity_get_drvinfo,
+ .set_tx_csum = ethtool_op_set_tx_csum,
+ .get_tx_csum = ethtool_op_get_tx_csum,
.get_wol = velocity_ethtool_get_wol,
.set_wol = velocity_ethtool_set_wol,
.get_msglevel = velocity_get_msglevel,
diff --git a/drivers/net/via-velocity.h b/drivers/net/via-velocity.h
index 22bfea4..cf10801 100644
--- a/drivers/net/via-velocity.h
+++ b/drivers/net/via-velocity.h
@@ -1423,7 +1423,6 @@ enum velocity_msg_level {
*/
#define VELOCITY_FLAGS_TAGGING 0x00000001UL
-#define VELOCITY_FLAGS_TX_CSUM 0x00000002UL
#define VELOCITY_FLAGS_RX_CSUM 0x00000004UL
#define VELOCITY_FLAGS_IP_ALIGN 0x00000008UL
#define VELOCITY_FLAGS_VAL_PKT_LEN 0x00000010UL
--
1.6.0.4
^ permalink raw reply related
* [PATCH v2 7/7] via-velocity: Bump version
From: Simon Kagstrom @ 2009-11-23 14:31 UTC (permalink / raw)
To: netdev; +Cc: davem, davej, shemminger, romieu
In-Reply-To: <20091123152724.2871cf4d@marrow.netinsight.se>
Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
drivers/net/via-velocity.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/via-velocity.h b/drivers/net/via-velocity.h
index cf10801..6b5fc51 100644
--- a/drivers/net/via-velocity.h
+++ b/drivers/net/via-velocity.h
@@ -29,7 +29,7 @@
#define VELOCITY_NAME "via-velocity"
#define VELOCITY_FULL_DRV_NAM "VIA Networking Velocity Family Gigabit Ethernet Adapter Driver"
-#define VELOCITY_VERSION "1.14"
+#define VELOCITY_VERSION "1.15"
#define VELOCITY_IO_SIZE 256
#define VELOCITY_NAPI_WEIGHT 64
--
1.6.0.4
^ permalink raw reply related
* Re: ixgbe question
From: Eric Dumazet @ 2009-11-23 14:38 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Peter P Waskiewicz Jr, Linux Netdev List
In-Reply-To: <Pine.LNX.4.64.0911231500060.24347@ask.diku.dk>
Jesper Dangaard Brouer a écrit :
> How is your smp_affinity mask's set?
>
> grep . /proc/irq/*/fiber1-*/../smp_affinity
First, I tried default affinities (ffff)
Then I tried irqbalance... no more success.
Driver seems to try to handle all queues on one cpu on low trafic,
and possibly dynamically switches to a multi-cpu mode,
but as all interrupts are masked, we stay in
a NAPI context handling all queues.
And we let one cpu in flood/drops mode.
>
>
>> But only one CPU (CPU1) had a softirq running, 100%, and many frames
>> were dropped
>
> Just a hint, I use 'ethtool -S fiber1' to see how the packets gets
> distributed across the rx and tx queues.
They are correctly distributed
rx_queue_0_packets: 14119644
rx_queue_0_bytes: 847178640
rx_queue_1_packets: 14126315
rx_queue_1_bytes: 847578900
rx_queue_2_packets: 14115249
rx_queue_2_bytes: 846914940
rx_queue_3_packets: 14118146
rx_queue_3_bytes: 847088760
rx_queue_4_packets: 14130869
rx_queue_4_bytes: 847853268
rx_queue_5_packets: 14112239
rx_queue_5_bytes: 846734340
rx_queue_6_packets: 14128425
rx_queue_6_bytes: 847705500
rx_queue_7_packets: 14110587
rx_queue_7_bytes: 846635220
rx_queue_8_packets: 14117350
rx_queue_8_bytes: 847041000
rx_queue_9_packets: 14125992
rx_queue_9_bytes: 847559520
rx_queue_10_packets: 14121732
rx_queue_10_bytes: 847303920
rx_queue_11_packets: 14120997
rx_queue_11_bytes: 847259820
rx_queue_12_packets: 14125576
rx_queue_12_bytes: 847535854
rx_queue_13_packets: 14118512
rx_queue_13_bytes: 847110720
rx_queue_14_packets: 14118348
rx_queue_14_bytes: 847100880
rx_queue_15_packets: 14118647
rx_queue_15_bytes: 847118820
>
>
>
>> CLONE_SKB="clone_skb 15"
>
> Be careful with to high clone, as my experience is it will send a burst
> of clone_skb packets before the packet gets randomized again.
Yes, but 15 should be ok with 10Gb link :)
Thanks
^ permalink raw reply
* [patch]USB autosuspend for cdc-ether
From: Oliver Neukum @ 2009-11-23 15:21 UTC (permalink / raw)
To: David Brownell, stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
David S. Miller, linux-usb-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
"Torgny Johansson" <torgny
Hi David,
here's a new, tested version of autosuspend for cdc-ether with
a few glitches fixed. I think it addresses everything you mentioned.
Could you provide some feedback about what's wrong with it or
an acknowledgement?
Regards
Oliver
Signed-off-by: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
commit cb0634eee6f96fb24044c961d7dffefed7b6457e
Author: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Date: Mon Nov 23 16:04:45 2009 +0100
usb:cdc-ether:implement power management for online devices
This implements USB autosuspend for cdc-ether devices that
support remote wakeup while they are online. For reception
remote wakeup and the last busy functionality of usbcore
is used.
Transmission is done with an anchor for deferred transmission
and the async pm methods of usbcore.
For devices that don't do remote wakeup power management while
offline is provided for.
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 21e1ba1..ed55024 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -411,6 +411,12 @@ static int cdc_bind(struct usbnet *dev, struct usb_interface *intf)
return 0;
}
+static int cdc_manage_power(struct usbnet *dev, int on)
+{
+ dev->intf->needs_remote_wakeup = on;
+ return 0;
+}
+
static const struct driver_info cdc_info = {
.description = "CDC Ethernet Device",
.flags = FLAG_ETHER,
@@ -418,6 +424,7 @@ static const struct driver_info cdc_info = {
.bind = cdc_bind,
.unbind = usbnet_cdc_unbind,
.status = cdc_status,
+ .manage_power = cdc_manage_power,
};
/*-------------------------------------------------------------------------*/
@@ -610,6 +617,7 @@ static struct usb_driver cdc_driver = {
.disconnect = usbnet_disconnect,
.suspend = usbnet_suspend,
.resume = usbnet_resume,
+ .supports_autosuspend = 1,
};
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ca5ca5a..4c2d902 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -353,7 +353,8 @@ static void rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags)
if (netif_running (dev->net)
&& netif_device_present (dev->net)
- && !test_bit (EVENT_RX_HALT, &dev->flags)) {
+ && !test_bit (EVENT_RX_HALT, &dev->flags)
+ && !test_bit (EVENT_DEV_ASLEEP, &dev->flags)) {
switch (retval = usb_submit_urb (urb, GFP_ATOMIC)) {
case -EPIPE:
usbnet_defer_kevent (dev, EVENT_RX_HALT);
@@ -611,15 +612,39 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
/*-------------------------------------------------------------------------*/
// precondition: never called in_interrupt
+static void usbnet_terminate_urbs(struct usbnet *dev)
+{
+ DECLARE_WAIT_QUEUE_HEAD_ONSTACK (unlink_wakeup);
+ DECLARE_WAITQUEUE (wait, current);
+ int temp;
+
+ /* ensure there are no more active urbs */
+ add_wait_queue(&unlink_wakeup, &wait);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ dev->wait = &unlink_wakeup;
+ temp = unlink_urbs(dev, &dev->txq) +
+ unlink_urbs(dev, &dev->rxq);
+
+ /* maybe wait for deletions to finish. */
+ while (!skb_queue_empty(&dev->rxq)
+ && !skb_queue_empty(&dev->txq)
+ && !skb_queue_empty(&dev->done)) {
+ schedule_timeout(UNLINK_TIMEOUT_MS);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (netif_msg_ifdown(dev))
+ devdbg(dev, "waited for %d urb completions",
+ temp);
+ }
+ set_current_state(TASK_RUNNING);
+ dev->wait = NULL;
+ remove_wait_queue(&unlink_wakeup, &wait);
+}
int usbnet_stop (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
struct driver_info *info = dev->driver_info;
- int temp;
int retval;
- DECLARE_WAIT_QUEUE_HEAD_ONSTACK (unlink_wakeup);
- DECLARE_WAITQUEUE (wait, current);
netif_stop_queue (net);
@@ -641,25 +666,8 @@ int usbnet_stop (struct net_device *net)
info->description);
}
- if (!(info->flags & FLAG_AVOID_UNLINK_URBS)) {
- /* ensure there are no more active urbs */
- add_wait_queue(&unlink_wakeup, &wait);
- dev->wait = &unlink_wakeup;
- temp = unlink_urbs(dev, &dev->txq) +
- unlink_urbs(dev, &dev->rxq);
-
- /* maybe wait for deletions to finish. */
- while (!skb_queue_empty(&dev->rxq)
- && !skb_queue_empty(&dev->txq)
- && !skb_queue_empty(&dev->done)) {
- msleep(UNLINK_TIMEOUT_MS);
- if (netif_msg_ifdown(dev))
- devdbg(dev, "waited for %d urb completions",
- temp);
- }
- dev->wait = NULL;
- remove_wait_queue(&unlink_wakeup, &wait);
- }
+ if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
+ usbnet_terminate_urbs(dev);
usb_kill_urb(dev->interrupt);
@@ -672,7 +680,10 @@ int usbnet_stop (struct net_device *net)
dev->flags = 0;
del_timer_sync (&dev->delay);
tasklet_kill (&dev->bh);
- usb_autopm_put_interface(dev->intf);
+ if (info->manage_power)
+ info->manage_power(dev, 0);
+ else
+ usb_autopm_put_interface(dev->intf);
return 0;
}
@@ -753,6 +764,12 @@ int usbnet_open (struct net_device *net)
// delay posting reads until we're fully open
tasklet_schedule (&dev->bh);
+ if (info->manage_power) {
+ retval = info->manage_power(dev, 1);
+ if (retval < 0)
+ goto done;
+ usb_autopm_put_interface(dev->intf);
+ }
return retval;
done:
usb_autopm_put_interface(dev->intf);
@@ -881,11 +898,16 @@ kevent (struct work_struct *work)
/* usb_clear_halt() needs a thread context */
if (test_bit (EVENT_TX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->txq);
+ status = usb_autopm_get_interface(dev->intf);
+ if (status < 0)
+ goto fail_pipe;
status = usb_clear_halt (dev->udev, dev->out);
+ usb_autopm_put_interface(dev->intf);
if (status < 0
&& status != -EPIPE
&& status != -ESHUTDOWN) {
if (netif_msg_tx_err (dev))
+fail_pipe:
deverr (dev, "can't clear tx halt, status %d",
status);
} else {
@@ -896,11 +918,16 @@ kevent (struct work_struct *work)
}
if (test_bit (EVENT_RX_HALT, &dev->flags)) {
unlink_urbs (dev, &dev->rxq);
+ status = usb_autopm_get_interface(dev->intf);
+ if (status < 0)
+ goto fail_halt;
status = usb_clear_halt (dev->udev, dev->in);
+ usb_autopm_put_interface(dev->intf);
if (status < 0
&& status != -EPIPE
&& status != -ESHUTDOWN) {
if (netif_msg_rx_err (dev))
+fail_halt:
deverr (dev, "can't clear rx halt, status %d",
status);
} else {
@@ -919,7 +946,12 @@ kevent (struct work_struct *work)
clear_bit (EVENT_RX_MEMORY, &dev->flags);
if (urb != NULL) {
clear_bit (EVENT_RX_MEMORY, &dev->flags);
+ status = usb_autopm_get_interface(dev->intf);
+ if (status < 0)
+ goto fail_lowmem;
rx_submit (dev, urb, GFP_KERNEL);
+ usb_autopm_put_interface(dev->intf);
+fail_lowmem:
tasklet_schedule (&dev->bh);
}
}
@@ -929,7 +961,11 @@ kevent (struct work_struct *work)
int retval = 0;
clear_bit (EVENT_LINK_RESET, &dev->flags);
+ status = usb_autopm_get_interface(dev->intf);
+ if (status < 0)
+ goto skip_reset;
if(info->link_reset && (retval = info->link_reset(dev)) < 0) {
+skip_reset:
devinfo(dev, "link reset failed (%d) usbnet usb-%s-%s, %s",
retval,
dev->udev->bus->bus_name, dev->udev->devpath,
@@ -958,7 +994,7 @@ static void tx_complete (struct urb *urb)
switch (urb->status) {
case -EPIPE:
- usbnet_defer_kevent (dev, EVENT_TX_HALT);
+ usbnet_defer_kevent (dev, EVENT_TX_HALT);
break;
/* software-driven interface shutdown */
@@ -971,6 +1007,7 @@ static void tx_complete (struct urb *urb)
case -EPROTO:
case -ETIME:
case -EILSEQ:
+ usb_mark_last_busy(dev->udev);
if (!timer_pending (&dev->delay)) {
mod_timer (&dev->delay,
jiffies + THROTTLE_JIFFIES);
@@ -987,6 +1024,7 @@ static void tx_complete (struct urb *urb)
}
}
+ usb_autopm_put_interface_async(dev->intf);
urb->dev = NULL;
entry->state = tx_done;
defer_bh(dev, skb, &dev->txq);
@@ -1058,13 +1096,33 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
}
spin_lock_irqsave (&dev->txq.lock, flags);
+ retval = usb_autopm_get_interface_async(dev->intf);
+ if (retval < 0) {
+ spin_unlock_irqrestore (&dev->txq.lock, flags);
+ goto drop;
+ }
+
+#ifdef CONFIG_PM
+ /* if this triggers the device is still a sleep */
+ if (test_bit(EVENT_DEV_ASLEEP, &dev->flags)) {
+ /* transmission will be done in resume */
+ usb_anchor_urb(urb, &dev->deferred);
+ /* no use to process more packets */
+ netif_stop_queue(net);
+ spin_unlock_irqrestore(&dev->txq.lock, flags);
+ devdbg(dev, "Delaying transmission for resumption");
+ goto deferred;
+ }
+#endif
switch ((retval = usb_submit_urb (urb, GFP_ATOMIC))) {
case -EPIPE:
netif_stop_queue (net);
usbnet_defer_kevent (dev, EVENT_TX_HALT);
+ usb_autopm_put_interface_async(dev->intf);
break;
default:
+ usb_autopm_put_interface_async(dev->intf);
if (netif_msg_tx_err (dev))
devdbg (dev, "tx: submit urb err %d", retval);
break;
@@ -1088,6 +1146,7 @@ drop:
devdbg (dev, "> tx, len %d, type 0x%x",
length, skb->protocol);
}
+deferred:
return NETDEV_TX_OK;
}
EXPORT_SYMBOL_GPL(usbnet_start_xmit);
@@ -1255,6 +1314,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
dev->bh.func = usbnet_bh;
dev->bh.data = (unsigned long) dev;
INIT_WORK (&dev->kevent, kevent);
+ init_usb_anchor(&dev->deferred);
dev->delay.function = usbnet_bh;
dev->delay.data = (unsigned long) dev;
init_timer (&dev->delay);
@@ -1363,13 +1423,23 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
struct usbnet *dev = usb_get_intfdata(intf);
if (!dev->suspend_count++) {
+ spin_lock_irq(&dev->txq.lock);
+ /* don't autosuspend while transmitting */
+ if (dev->txq.qlen && (message.event & PM_EVENT_AUTO)) {
+ spin_unlock_irq(&dev->txq.lock);
+ return -EBUSY;
+ } else {
+ set_bit(EVENT_DEV_ASLEEP, &dev->flags);
+ spin_unlock_irq(&dev->txq.lock);
+ }
/*
* accelerate emptying of the rx and queues, to avoid
* having everything error out.
*/
netif_device_detach (dev->net);
- (void) unlink_urbs (dev, &dev->rxq);
- (void) unlink_urbs (dev, &dev->txq);
+ usbnet_terminate_urbs(dev);
+ usb_kill_urb(dev->interrupt);
+
/*
* reattach so runtime management can use and
* wake the device
@@ -1383,10 +1453,34 @@ EXPORT_SYMBOL_GPL(usbnet_suspend);
int usbnet_resume (struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
+ struct sk_buff *skb;
+ struct urb *res;
+ int retval;
+
+ if (!--dev->suspend_count) {
+ spin_lock_irq(&dev->txq.lock);
+ while ((res = usb_get_from_anchor(&dev->deferred))) {
+
+ printk(KERN_INFO"Called %s with delayed data\n", __func__);
+ skb = (struct sk_buff *)res->context;
+ retval = usb_submit_urb(res, GFP_ATOMIC);
+ if (retval < 0) {
+ dev_kfree_skb_any(skb);
+ usb_free_urb(res);
+ usb_autopm_put_interface_async(dev->intf);
+ } else {
+ dev->net->trans_start = jiffies;
+ __skb_queue_tail (&dev->txq, skb);
+ }
+ }
- if (!--dev->suspend_count)
+ smp_mb();
+ clear_bit(EVENT_DEV_ASLEEP, &dev->flags);
+ spin_unlock_irq(&dev->txq.lock);
+ if (!(dev->txq.qlen >= TX_QLEN(dev)))
+ netif_start_queue(dev->net);
tasklet_schedule (&dev->bh);
-
+ }
return 0;
}
EXPORT_SYMBOL_GPL(usbnet_resume);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f814730..e75ba0e 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -55,6 +55,7 @@ struct usbnet {
struct sk_buff_head done;
struct sk_buff_head rxq_pause;
struct urb *interrupt;
+ struct usb_anchor deferred;
struct tasklet_struct bh;
struct work_struct kevent;
@@ -65,6 +66,8 @@ struct usbnet {
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
# define EVENT_RX_PAUSED 5
+# define EVENT_DEV_WAKING 6
+# define EVENT_DEV_ASLEEP 7
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
@@ -107,6 +110,9 @@ struct driver_info {
/* see if peer is connected ... can sleep */
int (*check_connect)(struct usbnet *);
+ /* (dis)activate runtime power management */
+ int (*manage_power)(struct usbnet *, int);
+
/* for status polling */
void (*status)(struct usbnet *, struct urb *);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] pktgen: Fix device name compares
From: Eric Dumazet @ 2009-11-23 15:22 UTC (permalink / raw)
To: robert; +Cc: David S. Miller, Linux Netdev List, Robert Olsson
In-Reply-To: <19210.40927.13011.176740@gargle.gargle.HOWL>
robert@herjulf.net a écrit :
> Ok. So the duplicate test got wrong when the multiqueue stuff was
> added.
>
> Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>
>
> Cheers
Thanks Robert
Here is an updated version because the netdev unregister event also needed some changes.
[PATCH] pktgen: Fix device name compares
Commit e6fce5b916cd7f7f7 (pktgen: multiqueue etc.) tried to relax
the pktgen restriction of one device per kernel thread, adding a '@'
tag to device names.
Problem is we dont perform check on full pktgen device name.
This allows adding many time same 'device' to pktgen thread
pgset "add_device eth0@0"
one session later :
pgset "add_device eth0@0"
(This doesnt find previous device)
This consumes ~1.5 MBytes of vmalloc memory per round and also triggers
this warning :
[ 673.186380] proc_dir_entry 'pktgen/eth0@0' already registered
[ 673.186383] Modules linked in: pktgen ixgbe ehci_hcd psmouse mdio mousedev evdev [last unloaded: pktgen]
[ 673.186406] Pid: 6219, comm: bash Tainted: G W 2.6.32-rc7-03302-g41cec6f-dirty #16
[ 673.186410] Call Trace:
[ 673.186417] [<ffffffff8104a29b>] warn_slowpath_common+0x7b/0xc0
[ 673.186422] [<ffffffff8104a341>] warn_slowpath_fmt+0x41/0x50
[ 673.186426] [<ffffffff8114e789>] proc_register+0x109/0x210
[ 673.186433] [<ffffffff8100bf2e>] ? apic_timer_interrupt+0xe/0x20
[ 673.186438] [<ffffffff8114e905>] proc_create_data+0x75/0xd0
[ 673.186444] [<ffffffffa006ad38>] pktgen_thread_write+0x568/0x640 [pktgen]
[ 673.186449] [<ffffffffa006a7d0>] ? pktgen_thread_write+0x0/0x640 [pktgen]
[ 673.186453] [<ffffffff81149144>] proc_reg_write+0x84/0xc0
[ 673.186458] [<ffffffff810f5a58>] vfs_write+0xb8/0x180
[ 673.186463] [<ffffffff810f5c11>] sys_write+0x51/0x90
[ 673.186468] [<ffffffff8100b51b>] system_call_fastpath+0x16/0x1b
[ 673.186470] ---[ end trace ccbb991b0a8d994d ]---
Solution to this problem is to use a odevname field (includes @ tag and suffix),
instead of using netdevice name.
This also permits clean unloading of NIC drivers.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/pktgen.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index d38470a..e23f494 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -364,6 +364,7 @@ struct pktgen_dev {
* device name (not when the inject is
* started as it used to do.)
*/
+ char odevname[32];
struct flow_state *flows;
unsigned cflows; /* Concurrent flows (config) */
unsigned lflow; /* Flow length (config) */
@@ -427,7 +428,7 @@ static const char version[] =
static int pktgen_remove_device(struct pktgen_thread *t, struct pktgen_dev *i);
static int pktgen_add_device(struct pktgen_thread *t, const char *ifname);
static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
- const char *ifname);
+ const char *ifname, bool exact);
static int pktgen_device_event(struct notifier_block *, unsigned long, void *);
static void pktgen_run_all_threads(void);
static void pktgen_reset_all_threads(void);
@@ -529,7 +530,7 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
seq_printf(seq,
" frags: %d delay: %llu clone_skb: %d ifname: %s\n",
pkt_dev->nfrags, (unsigned long long) pkt_dev->delay,
- pkt_dev->clone_skb, pkt_dev->odev->name);
+ pkt_dev->clone_skb, pkt_dev->odevname);
seq_printf(seq, " flows: %u flowlen: %u\n", pkt_dev->cflows,
pkt_dev->lflow);
@@ -1689,13 +1690,13 @@ static int pktgen_thread_show(struct seq_file *seq, void *v)
if_lock(t);
list_for_each_entry(pkt_dev, &t->if_list, list)
if (pkt_dev->running)
- seq_printf(seq, "%s ", pkt_dev->odev->name);
+ seq_printf(seq, "%s ", pkt_dev->odevname);
seq_printf(seq, "\nStopped: ");
list_for_each_entry(pkt_dev, &t->if_list, list)
if (!pkt_dev->running)
- seq_printf(seq, "%s ", pkt_dev->odev->name);
+ seq_printf(seq, "%s ", pkt_dev->odevname);
if (t->result[0])
seq_printf(seq, "\nResult: %s\n", t->result);
@@ -1818,9 +1819,10 @@ static struct pktgen_dev *__pktgen_NN_threads(const char *ifname, int remove)
{
struct pktgen_thread *t;
struct pktgen_dev *pkt_dev = NULL;
+ bool exact = (remove == FIND);
list_for_each_entry(t, &pktgen_threads, th_list) {
- pkt_dev = pktgen_find_dev(t, ifname);
+ pkt_dev = pktgen_find_dev(t, ifname, exact);
if (pkt_dev) {
if (remove) {
if_lock(t);
@@ -1995,7 +1997,7 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
"queue_map_min (zero-based) (%d) exceeds valid range "
"[0 - %d] for (%d) queues on %s, resetting\n",
pkt_dev->queue_map_min, (ntxq ?: 1) - 1, ntxq,
- pkt_dev->odev->name);
+ pkt_dev->odevname);
pkt_dev->queue_map_min = ntxq - 1;
}
if (pkt_dev->queue_map_max >= ntxq) {
@@ -2003,7 +2005,7 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
"queue_map_max (zero-based) (%d) exceeds valid range "
"[0 - %d] for (%d) queues on %s, resetting\n",
pkt_dev->queue_map_max, (ntxq ?: 1) - 1, ntxq,
- pkt_dev->odev->name);
+ pkt_dev->odevname);
pkt_dev->queue_map_max = ntxq - 1;
}
@@ -3263,7 +3265,7 @@ static int pktgen_stop_device(struct pktgen_dev *pkt_dev)
if (!pkt_dev->running) {
printk(KERN_WARNING "pktgen: interface: %s is already "
- "stopped\n", pkt_dev->odev->name);
+ "stopped\n", pkt_dev->odevname);
return -EINVAL;
}
@@ -3467,7 +3469,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
default: /* Drivers are not supposed to return other values! */
if (net_ratelimit())
pr_info("pktgen: %s xmit error: %d\n",
- odev->name, ret);
+ pkt_dev->odevname, ret);
pkt_dev->errors++;
/* fallthru */
case NETDEV_TX_LOCKED:
@@ -3570,13 +3572,18 @@ static int pktgen_thread_worker(void *arg)
}
static struct pktgen_dev *pktgen_find_dev(struct pktgen_thread *t,
- const char *ifname)
+ const char *ifname, bool exact)
{
struct pktgen_dev *p, *pkt_dev = NULL;
+ size_t len = strlen(ifname);
if_lock(t);
list_for_each_entry(p, &t->if_list, list)
- if (strncmp(p->odev->name, ifname, IFNAMSIZ) == 0) {
+ if (strncmp(p->odevname, ifname, len) == 0) {
+ if (p->odevname[len]) {
+ if (exact || p->odevname[len] != '@')
+ continue;
+ }
pkt_dev = p;
break;
}
@@ -3632,6 +3639,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
if (!pkt_dev)
return -ENOMEM;
+ strcpy(pkt_dev->odevname, ifname);
pkt_dev->flows = vmalloc(MAX_CFLOWS * sizeof(struct flow_state));
if (pkt_dev->flows == NULL) {
kfree(pkt_dev);
^ permalink raw reply related
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Shirley Ma @ 2009-11-23 16:07 UTC (permalink / raw)
To: Rusty Russell
Cc: Eric Dumazet, Michael S. Tsirkin, Avi Kivity, netdev, kvm,
linux-kernel
In-Reply-To: <200911231138.30755.rusty@rustcorp.com.au>
On Mon, 2009-11-23 at 11:38 +1030, Rusty Russell wrote:
> How about:
> struct page *end;
>
> /* Find end of list, sew whole thing into vi->pages. */
> for (end = page; end->private; end = (struct page
> *)end->private);
> end->private = (unsigned long)vi->pages;
> vi->pages = page;
Yes, this looks nicer.
> > +void virtio_free_pages(void *buf)
>
> This is a terrible name; it's specific to virtio_net, plus it should
> be
> static. Just "free_pages" should be sufficient here I think.
>
> This smells a lot like a for loop to me?
>
> struct page *i, *next;
>
> for (i = buf; next; i = next) {
> next = (struct page *)i->private;
> __free_pages(i, 0);
> }
Agree, will change it.
> > +static int set_skb_frags(struct sk_buff *skb, struct page *page,
> > + int offset, int len)
>
> A better name might be "skb_add_frag()"?
OK.
> > + skb = (struct sk_buff *)buf;
> This cast is unnecessary, but a comment would be nice:
> /* Simple case: the pointer is the 1514-byte skb */
>
> > + } else {
Without this cast there is a compile warning.
> And a matching comment here:
>
> /* The pointer is a chain of pages. */
>
OK.
> > + if (unlikely(!skb)) {
> > + dev->stats.rx_dropped++;
>
> Does this mean we leak all those pages? Shouldn't we give_pages()
> here?
Yes, it should call give_pages here.
> > + offset = hdr_len + 6;
>
> Really? I can't see where the current driver does this 6 byte offset.
> There
> should be a header, then the packet...
> Ah, I see below that you set things up this way. The better way to do
> this
> is to create a new structure to use in various places.
>
> struct padded_vnet_hdr {
> struct virtio_net_hdr hdr;
> /* This padding makes our packet 16 byte aligned */
> char padding[6];
> };
>
> However, I question whether making it 16 byte is the right thing: the
> ethernet header is 14 bytes long, so don't we want 8 bytes of padding?
Because in QEMU it requires 10 bytes header in a separately, so one page
is used to share between virtio_net_hdr header which is 10 bytes head
and rest of data. So I put 6 bytes offset here between two buffers. I
didn't look at the reason why a seperate buf is used for virtio_net_hdr
in QEMU.
> > + }
>
> I think you can move the memcpy outside the if, like so:
>
> memcpy(&hdr, p, hdr_len);
I didn't move it out, because num_buf = hdr->mhdr.num_buffers;
> > + if (!len)
> > + give_pages(vi, page);
> > + else {
> > + len = set_skb_frags(skb, page, copy + offset,
> len);
> > + /* process big packets */
> > + while (len > 0) {
> > + page = (struct page *)page->private;
> > + if (!page)
> > + break;
> > + len = set_skb_frags(skb, page, 0,
> len);
> > + }
> > + if (page && page->private)
> > + give_pages(vi, (struct page
> *)page->private);
> > }
>
> I can't help but think this statement should be one loop somehow.
> Something
> like:
>
> offset += copy;
>
> while (len) {
> len = skb_add_frag(skb, page, offset, len);
> page = (struct page *)page->private;
> offset = 0;
> }
> if (page)
> give_pages(vi, page);
I was little bit worried about qemu messed up len (i saw code in
mergeable buffer checking len > PAGE_SIZE), so I put page checking
inside. I will change it outside if checking len is enough.
> > -static bool try_fill_recv_maxbufs(struct virtnet_info *vi, gfp_t
> gfp)
> > +/* Returns false if we couldn't fill entirely (OOM). */
> > +static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
>
> The result of trying to merge all the cases here is messy. I'd split
> it
> inside the loop into: add_recvbuf_small(vi, gfp), add_recvbuf_big(vi,
> gfp)
> and add_recvbuf_mergeable(vi, gfp).
>
> It'll also be easier for me to review each case then, as well as
> getting
> rid of the big packets if we decide to do that later. You could even
> do
> that split as a separate patch, before this one.
Ok, will separate it.
> destroy_buf should really be called destroy_bufs(). And I don't think
> you
> use the vi->recv skb list in this case at all, so the loop which
> purges those
> should be under an "else {" of this branch.
Yes.
> This new parameter should be introduced as a separate patch. It
> should also be
> called destroy_bufs. It also returns an unsigned int. I would call
> the callback
> something a little more informative, such as "destroy"; here and in
> the header.
Ok.
> ret is a bad name for this; how about buf?
Sure.
> Overall, the patch looks good. But it would have been nicer if it
> were
> split into several parts: cleanups, new infrastructure, then the
> actual
> allocation change.
I will try to separate this patch.
Thanks
Shirley
^ permalink raw reply
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Shirley Ma @ 2009-11-23 16:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eric Dumazet, Avi Kivity, Rusty Russell, netdev, kvm,
linux-kernel
In-Reply-To: <20091123094323.GA3748@redhat.com>
On Mon, 2009-11-23 at 11:43 +0200, Michael S. Tsirkin wrote:
> should be !npage->private
> and nesting is too deep here:
> this is cleaner in a give_a_page subroutine
> as it was.
This will be addressed with Rusty's comment.
> > + /* use private to chain big packets */
>
> packets? or pages?
Will change it to chain pages for big packets
> > + p->private = (unsigned long)0;
>
> the comment is not really helpful:
> you say you use private to chain but 0 does not
> chain anything. You also do not need the cast to long?
Ok.
> > + if (len > (PAGE_SIZE - f->page_offset))
>
> brackets around math are not needed.
OK.
> typo
>
> > + * header and data */
Got it.
> please think of a way to get rid of magic constants like 6 and 2
> here and elsewhere.
Will do.
> replace MAX_SKB_FRAGS + 2 with something symbolic? We have it in 2 palces now.
> And comment.
Ok, I can change it.
> terrible goto based loop
> move stuff into subfunction, it will be much
> more manageable, and convert this to a simple
> for loop.
Will change it to different functions based on Rusty's comment.
> and here it is MAX_SKB_FRAGS + 1
I think I should use MAX_SKB_FRAGS + 2 instead. Now I only use
MAX_SKB_FRAGS + 1 but allocated + 2.
> > + page->private = (unsigned long)first_page;
> > + first_page = page;
> > + if (--i == 1) {
>
> this is pretty hairy ... has to be this way?
> What you are trying to do here
> is fill buffer with pages, in a loop, with first one
> using a partial page, and then add it.
> Is that it?
Yes.
> So please code this in a straight forward manner.
> it should be as simple as:
> offset = XXX
> for (i = 0; i < MAX_SKB_FRAGS + 2; ++i) {
>
> sg_set_buf(sg + i, p + offset, PAGE_SIZE - offset);
> offset = 0;
>
> }
Ok, looks more neat.
> space around +
> sg + 1 here is same as &sg[i] in fact?
Ok.
> callback -> destructor?
Ok.
I will integrate these comments with Rusty's and resubmit the patch set.
Thanks
Shirley
^ permalink raw reply
* Re: A generic kernel compatibilty code
From: Luis R. Rodriguez @ 2009-11-23 16:20 UTC (permalink / raw)
To: Ben Hutchings; +Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <1258982793.2845.13.camel@achroite.uk.solarflarecom.com>
On Mon, Nov 23, 2009 at 5:26 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Fri, 2009-11-20 at 13:07 -0800, Luis R. Rodriguez wrote:
>> On Fri, Nov 20, 2009 at 1:00 PM, Ben Hutchings
>> <bhutchings@solarflare.com> wrote:
>> > On Fri, 2009-11-20 at 12:45 -0800, Luis R. Rodriguez wrote:
>> >> Everyone and their mother reinvents the wheel when it comes to
>> >> backporting kernel modules. It a painful job and it seems to me an
>> >> alternative is possible. If we can write generic compatibilty code for
>> >> a new routine introduced on the next kernel how about just merging it
>> >> to the kernel under some generic compat module. This would be
>> >> completey ignored by everyone using the stable kernel but can be
>> >> copied by anyone doing backport work.
>> >>
>> >> So I'm thinking something as simple as a generic compat/comat.ko with
>> >> compat-2.6.32.[ch] files.
>> >>
>> >> We've already backported everything needed for wireless drivers under
>> >> compat-wireless under this format down to even 2.6.25.
>> > [...]
>> >
>> > If you think 2.6.25 is old then I don't think you understand the scale
>> > of the problem.
>> >
>> > OEMs still expect us to support RHEL 4 (2.6.9) and SLES 9 (2.6.5) though
>> > the latter will probably be dropped soon. Some other vendors apparently
>> > still need to support even 2.4 kernels!
>>
>> Heh understood. Well shouldn't this help with that then? Sure I'd love
>> to see the Enteprise Linux releases on 2.6.31 but that's not going to
>> happen right? Shouldn't this help then?
>
> You'd really have to ask the 'enterprise' vendors whether they'd be
> interested in working on some sort of shared forward-compat library.
OK will do thanks.
> If the library is to include a module rather than being statically linked
> into each module that needs it then there can only be one instance in
> the system.
Sure, that's the idea.
Luis
^ permalink raw reply
* Re: ixgbe question
From: robert @ 2009-11-23 18:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jesper Dangaard Brouer, Peter P Waskiewicz Jr, Linux Netdev List
In-Reply-To: <4B0A9E4E.9010804@gmail.com>
Eric Dumazet writes:
> Jesper Dangaard Brouer a écrit :
>
> > How is your smp_affinity mask's set?
> >
> > grep . /proc/irq/*/fiber1-*/../smp_affinity
>
Weird... set clone_skb to 1 to be sure and vary dst or something so
the HW classifier selects different queues and with proper RX affinty.
You should see in /proc/net/softnet_stat something like:
012a7bb9 00000000 000000ae 00000000 00000000 00000000 00000000 00000000 00000000
01288d4c 00000000 00000049 00000000 00000000 00000000 00000000 00000000 00000000
0128fe28 00000000 00000043 00000000 00000000 00000000 00000000 00000000 00000000
01295387 00000000 00000047 00000000 00000000 00000000 00000000 00000000 00000000
0129a722 00000000 0000004a 00000000 00000000 00000000 00000000 00000000 00000000
0128c5e4 00000000 00000046 00000000 00000000 00000000 00000000 00000000 00000000
0128f718 00000000 00000043 00000000 00000000 00000000 00000000 00000000 00000000
012993e3 00000000 0000004a 00000000 00000000 00000000 00000000 00000000 00000000
Or something is...
Cheers
--ro
^ permalink raw reply
* Re: [PATCH 0/7] via-velocity performance fixes
From: Stephen Hemminger @ 2009-11-23 16:27 UTC (permalink / raw)
To: Simon Kagstrom; +Cc: netdev, davem, davej, romieu
In-Reply-To: <20091123143922.273d13f9@marrow.netinsight.se>
On Mon, 23 Nov 2009 14:39:22 +0100
Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:
> while the VIA driver does
>
> ip_summed = CHECKSUM_NONE;
> if (hardware sees that this is an IP packet):
> if (hw IP checksum is OK):
> if (hw sees that this is a TCP/UDP packet):
> if (hw TCP/UDP checksum is NOT OK):
> return # Returning CHECKSUM_NONE
> ip_summed = CHECKSUM_UNNECESSARY
>
> i.e., it will return CHECKSUM_NONE also for non-TCP/UDP IP packets.
That is correct assuming hardware can only checksum TCP/UDP.
--
^ 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