* Re: [Patch] netpoll: allow spaces in its parameter
From: Cong Wang @ 2010-03-17 6:38 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, netdev
In-Reply-To: <20100316.231434.177653499.davem@davemloft.net>
David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Wed, 17 Mar 2010 14:07:57 +0800
>
>> No, but silently accepting it as 0 is not correct, why don't
>> reject it if it is not allowed?
>
> You have to be careful even with that. For example, if two
> netconsoles are specified, seeing the space shouldn't
> kill the first netconsole specification we parsed.
>
> A warning perhaps, but outright rejection of all specifications is
> really bad behavior.
OK, I will put a warning there instead.
Thanks, David!
^ permalink raw reply
* e1000e: Fix build with CONFIG_PM disabled.
From: David Miller @ 2010-03-17 6:41 UTC (permalink / raw)
To: rjw; +Cc: netdev
I had to add the following patch to fix the build after
your changes.
Thanks.
e1000e: Fix build with CONFIG_PM disabled.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/e1000e/netdev.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 79b33c5..b96532a 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -5475,6 +5475,7 @@ static DEFINE_PCI_DEVICE_TABLE(e1000_pci_tbl) = {
};
MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
+#ifdef CONFIG_PM
static const struct dev_pm_ops e1000_pm_ops = {
.suspend = e1000_suspend,
.resume = e1000_resume,
@@ -5486,6 +5487,7 @@ static const struct dev_pm_ops e1000_pm_ops = {
.runtime_resume = e1000_runtime_resume,
.runtime_idle = e1000_idle,
};
+#endif
/* PCI Device API Driver */
static struct pci_driver e1000_driver = {
--
1.6.6.1
^ permalink raw reply related
* Re: [PATCH v7] rps: Receive Packet Steering
From: Eric Dumazet @ 2010-03-17 7:07 UTC (permalink / raw)
To: Changli Gao; +Cc: David Miller, therbert, netdev
In-Reply-To: <412e6f7f1003161854w32ed4516w2e52003097051fc7@mail.gmail.com>
Le mercredi 17 mars 2010 à 09:54 +0800, Changli Gao a écrit :
> On Wed, Mar 17, 2010 at 5:13 AM, David Miller <davem@davemloft.net> wrote:
> >
> > I'll integrate this as soon as I open up net-next-2.6
>
> It is really a good news. Now linux also can dispatch packets as
> FreeBSD does via netisr. Can we walk farer, and support weighted
> distribution?
>
May I ask why ? What would be the goal ?
If you perform too much work behalf the first cpu (the one actually
dealing with the device and dispatching packets to other cpus), you risk
bringing into its cache whole packet and consuming too many cpu cycles.
For instance, I would prefer a basic spreading, and try to reorganise
sk_buffs to that this cpu touches only one cache line in sk_buff, and
read one cache line in packet data to compute rxhash.
Then some NIC could compute rxhash themselves and provide it in their
rx_desc. Of course multiqueue support is much better and current
hardware prefer to implement the real thing.
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-17 7:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Oleg Nesterov, David Miller, Ingo Molnar, Francois Romieu,
Peter Zijlstra, netdev, linux-kernel
In-Reply-To: <1268765284.2932.17.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 7388 bytes --]
Hello,
cumulative patch:
[ 155.337373] ------------[ cut here ]------------
[ 155.337386] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
[ 155.337390] Hardware name: F3JC
[ 155.337394] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
[ 155.337397] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel snd_hda_codec
asus_laptop sparse_keymap mmc_core led_class snd_pcm snd_timer snd_page_alloc psmouse rng_core snd soundcore sg i2c_i801 evdev serio_raw r8169 mii usbhid hid uhci_hcd ehci_hcd
sr_mod cdrom sd_mod usbcore ata_piix
[ 155.337468] Pid: 7, comm: ksoftirqd/1 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
[ 155.337472] Call Trace:
[ 155.337481] [<c102e293>] warn_slowpath_common+0x65/0x7c
[ 155.337506] [<c126ac34>] ? dev_watchdog+0xc1/0x125
[ 155.337512] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
[ 155.337517] [<c126ac34>] dev_watchdog+0xc1/0x125
[ 155.337525] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 155.337530] [<c1036b51>] run_timer_softirq+0x176/0x1eb
[ 155.337536] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 155.337566] [<c126ab73>] ? dev_watchdog+0x0/0x125
[ 155.337576] [<c1032d39>] __do_softirq+0x8d/0x117
[ 155.337667] [<c1032dee>] do_softirq+0x2b/0x43
[ 155.337729] [<c1032fc1>] run_ksoftirqd+0x71/0x140
[ 155.337745] [<c1032f50>] ? run_ksoftirqd+0x0/0x140
[ 155.337810] [<c103f60e>] kthread+0x6a/0x6f
[ 155.337832] [<c103f5a4>] ? kthread+0x0/0x6f
[ 155.337903] [<c1002e42>] kernel_thread_helper+0x6/0x1a
[ 155.337907] ---[ end trace a22d306b065d4a68 ]---
[ 155.350902] r8169 0000:02:00.0: eth0: link up
[ 167.350892] r8169 0000:02:00.0: eth0: link up
Sergey
On (03/16/10 19:48), Eric Dumazet wrote:
> > Hello,
> > Got it right now.
> >
> > System completely froze. Even SysRq didn't work.
> > /*spin_lock deadlock?*/
> >
> > NOTE: I'm losing network constantly with pktgen tests
>
> yes, every 12 seconds there is a reset because of fifo overflow
>
> > [24208.010980] r8169 0000:02:00.0: eth0: link up
> > [24220.010980] r8169 0000:02:00.0: eth0: link up
> > [24232.011030] r8169 0000:02:00.0: eth0: link up
> > [24340.010980] r8169 0000:02:00.0: eth0: link up
> > [24352.010966] r8169 0000:02:00.0: eth0: link up
> > [24364.010966] r8169 0000:02:00.0: eth0: link up
> > [24376.010964] r8169 0000:02:00.0: eth0: link up
> > [24388.010961] r8169 0000:02:00.0: eth0: link up
> > [24400.010959] r8169 0000:02:00.0: eth0: link up
> > [24412.010963] r8169 0000:02:00.0: eth0: link up
> >
> >
> > Traces:
> > [24600.625078] INFO: task events/0:9 blocked for more than 120 seconds.
> > [24600.625083] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > [24600.625087] events/0 D 00001636 0 9 2 0x00000000
> > [24600.625096] f7085ebc 00000046 a87e9490 00001636 c1617cc0 c1617cc0 c1617cc0 c1617cc0
> > [24600.625109] f707c250 c1617cc0 c1617cc0 00000000 00000000 00000000 00000000 f707bfc0
> > [24600.625122] c14745c8 c14745c8 f707bfc0 00000202 f7085efc c12c6449 00000000 00085edc
> > [24600.625135] Call Trace:
> > [24600.625148] [<c12c6449>] __mutex_lock_common+0x233/0x3af
> > [24600.625155] [<c12c65ff>] mutex_lock_nested+0x12/0x15
> > [24600.625163] [<c1265e0f>] ? rtnl_lock+0xf/0x11
> > [24600.625168] [<c1265e0f>] rtnl_lock+0xf/0x11
> > [24600.625183] [<f9174acd>] rtl8169_reset_task+0x16/0xee [r8169]
> > [24600.625191] [<c103c887>] worker_thread+0x161/0x233
> > [24600.625196] [<c103c845>] ? worker_thread+0x11f/0x233
> > [24600.625205] [<f9174ab7>] ? rtl8169_reset_task+0x0/0xee [r8169]
> > [24600.625214] [<c103f9f1>] ? autoremove_wake_function+0x0/0x2f
> > [24600.625220] [<c103c726>] ? worker_thread+0x0/0x233
> > [24600.625225] [<c103f6ce>] kthread+0x6a/0x6f
> > [24600.625232] [<c103f664>] ? kthread+0x0/0x6f
> > [24600.625238] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> > [24600.625242] INFO: lockdep is turned off.
> > [24600.625259] INFO: task X:3176 blocked for more than 120 seconds.
> > [24600.625262] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > [24600.625266] X D 00000000 0 3176 3175 0x00400004
> > [24600.625273] f6435a10 00003046 00025709 00000000 c1617cc0 c1617cc0 c1617cc0 c1617cc0
> > [24600.625286] f60897d0 c1617c ...
> > /*THE REST IS LOST*/
> >
> >
>
> OK thanks for the report, this rtl8169_reset_task() seems pretty buggy,
> or multiple invocation...
>
> Did you tried removing the rtl8169_schedule_work() call from
> rtl8169_rx_interrupt() ?
>
> Maybe the reset is not necessary at all in case of fifo overflow..
>
> Cumulative patch :
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 9d3ebf3..d6ef4dd 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1038,14 +1038,14 @@ static void rtl8169_vlan_rx_register(struct net_device *dev,
> }
>
> static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> - struct sk_buff *skb)
> + struct sk_buff *skb, int polling)
> {
> u32 opts2 = le32_to_cpu(desc->opts2);
> struct vlan_group *vlgrp = tp->vlgrp;
> int ret;
>
> if (vlgrp && (opts2 & RxVlanTag)) {
> - vlan_hwaccel_receive_skb(skb, vlgrp, swab16(opts2 & 0xffff));
> + __vlan_hwaccel_rx(skb, vlgrp, swab16(opts2 & 0xffff), polling);
> ret = 0;
> } else
> ret = -1;
> @@ -1062,7 +1062,7 @@ static inline u32 rtl8169_tx_vlan_tag(struct rtl8169_private *tp,
> }
>
> static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> - struct sk_buff *skb)
> + struct sk_buff *skb, int polling)
> {
> return -1;
> }
> @@ -4429,12 +4429,20 @@ out:
> return done;
> }
>
> +/*
> + * Warning : rtl8169_rx_interrupt() might be called :
> + * 1) from NAPI (softirq) context
> + * (polling = 1 : we should call netif_receive_skb())
> + * 2) from process context (rtl8169_reset_task())
> + * (polling = 0 : we must call netif_rx() instead)
> + */
> static int rtl8169_rx_interrupt(struct net_device *dev,
> struct rtl8169_private *tp,
> void __iomem *ioaddr, u32 budget)
> {
> unsigned int cur_rx, rx_left;
> unsigned int delta, count;
> + int polling = (budget != ~(u32)0) ? 1 : 0;
>
> cur_rx = tp->cur_rx;
> rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx;
> @@ -4459,7 +4467,6 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> if (status & RxCRC)
> dev->stats.rx_crc_errors++;
> if (status & RxFOVF) {
> - rtl8169_schedule_work(dev, rtl8169_reset_task);
> dev->stats.rx_fifo_errors++;
> }
> rtl8169_mark_to_asic(desc, tp->rx_buf_sz);
> @@ -4496,8 +4503,12 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> skb_put(skb, pkt_size);
> skb->protocol = eth_type_trans(skb, dev);
>
> - if (rtl8169_rx_vlan_skb(tp, desc, skb) < 0)
> - netif_receive_skb(skb);
> + if (rtl8169_rx_vlan_skb(tp, desc, skb, polling) < 0) {
> + if (likely(polling))
> + netif_receive_skb(skb);
> + else
> + netif_rx(skb);
> + }
>
> dev->stats.rx_bytes += pkt_size;
> dev->stats.rx_packets++;
>
>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Eric Dumazet @ 2010-03-17 7:37 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <20100317072539.GA3579@swordfish>
trimming some cc
Le mercredi 17 mars 2010 à 09:25 +0200, Sergey Senozhatsky a écrit :
> Hello,
>
> cumulative patch:
>
> [ 155.337373] ------------[ cut here ]------------
> [ 155.337386] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> [ 155.337390] Hardware name: F3JC
> [ 155.337394] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> [ 155.337397] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel snd_hda_codec
> asus_laptop sparse_keymap mmc_core led_class snd_pcm snd_timer snd_page_alloc psmouse rng_core snd soundcore sg i2c_i801 evdev serio_raw r8169 mii usbhid hid uhci_hcd ehci_hcd
> sr_mod cdrom sd_mod usbcore ata_piix
> [ 155.337468] Pid: 7, comm: ksoftirqd/1 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
> [ 155.337472] Call Trace:
> [ 155.337481] [<c102e293>] warn_slowpath_common+0x65/0x7c
> [ 155.337506] [<c126ac34>] ? dev_watchdog+0xc1/0x125
> [ 155.337512] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
> [ 155.337517] [<c126ac34>] dev_watchdog+0xc1/0x125
> [ 155.337525] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> [ 155.337530] [<c1036b51>] run_timer_softirq+0x176/0x1eb
> [ 155.337536] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> [ 155.337566] [<c126ab73>] ? dev_watchdog+0x0/0x125
> [ 155.337576] [<c1032d39>] __do_softirq+0x8d/0x117
> [ 155.337667] [<c1032dee>] do_softirq+0x2b/0x43
> [ 155.337729] [<c1032fc1>] run_ksoftirqd+0x71/0x140
> [ 155.337745] [<c1032f50>] ? run_ksoftirqd+0x0/0x140
> [ 155.337810] [<c103f60e>] kthread+0x6a/0x6f
> [ 155.337832] [<c103f5a4>] ? kthread+0x0/0x6f
> [ 155.337903] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> [ 155.337907] ---[ end trace a22d306b065d4a68 ]---
> [ 155.350902] r8169 0000:02:00.0: eth0: link up
> [ 167.350892] r8169 0000:02:00.0: eth0: link up
>
>
>
On receiver ?
I suspect lot of work is needed on this driver to make it working, but I
dont have a machine with said adapter.
Are you in 100 Mb full duplex mode ?
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-17 7:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <1268811437.2932.66.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 3064 bytes --]
On (03/17/10 08:37), Eric Dumazet wrote:
> trimming some cc
>
> Le mercredi 17 mars 2010 à 09:25 +0200, Sergey Senozhatsky a écrit :
> > Hello,
> >
> > cumulative patch:
> >
> > [ 155.337373] ------------[ cut here ]------------
> > [ 155.337386] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> > [ 155.337390] Hardware name: F3JC
> > [ 155.337394] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> > [ 155.337397] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel snd_hda_codec
> > asus_laptop sparse_keymap mmc_core led_class snd_pcm snd_timer snd_page_alloc psmouse rng_core snd soundcore sg i2c_i801 evdev serio_raw r8169 mii usbhid hid uhci_hcd ehci_hcd
> > sr_mod cdrom sd_mod usbcore ata_piix
> > [ 155.337468] Pid: 7, comm: ksoftirqd/1 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
> > [ 155.337472] Call Trace:
> > [ 155.337481] [<c102e293>] warn_slowpath_common+0x65/0x7c
> > [ 155.337506] [<c126ac34>] ? dev_watchdog+0xc1/0x125
> > [ 155.337512] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
> > [ 155.337517] [<c126ac34>] dev_watchdog+0xc1/0x125
> > [ 155.337525] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> > [ 155.337530] [<c1036b51>] run_timer_softirq+0x176/0x1eb
> > [ 155.337536] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> > [ 155.337566] [<c126ab73>] ? dev_watchdog+0x0/0x125
> > [ 155.337576] [<c1032d39>] __do_softirq+0x8d/0x117
> > [ 155.337667] [<c1032dee>] do_softirq+0x2b/0x43
> > [ 155.337729] [<c1032fc1>] run_ksoftirqd+0x71/0x140
> > [ 155.337745] [<c1032f50>] ? run_ksoftirqd+0x0/0x140
> > [ 155.337810] [<c103f60e>] kthread+0x6a/0x6f
> > [ 155.337832] [<c103f5a4>] ? kthread+0x0/0x6f
> > [ 155.337903] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> > [ 155.337907] ---[ end trace a22d306b065d4a68 ]---
> > [ 155.350902] r8169 0000:02:00.0: eth0: link up
> > [ 167.350892] r8169 0000:02:00.0: eth0: link up
> >
> >
> >
>
> On receiver ?
>
Localhost pollution (both sender and receiver are localhost). In 2 hours I will
try to pktgen over LAN.
> I suspect lot of work is needed on this driver to make it working, but I
> dont have a machine with said adapter.
>
> Are you in 100 Mb full duplex mode ?
>
ethtool eth0
Settings for eth0:
Supported ports: [ TP MII ]
Supported link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Supports auto-negotiation: Yes
Advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Half 1000baseT/Full
Advertised auto-negotiation: Yes
Speed: 100Mb/s
Duplex: Full
Port: MII
PHYAD: 0
Transceiver: internal
Auto-negotiation: on
Supports Wake-on: pumbg
Wake-on: g
Current message level: 0x00000033 (51)
Link detected: yes
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH v7] rps: Receive Packet Steering
From: Changli Gao @ 2010-03-17 7:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, therbert, netdev
In-Reply-To: <1268809673.2932.62.camel@edumazet-laptop>
On Wed, Mar 17, 2010 at 3:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 17 mars 2010 à 09:54 +0800, Changli Gao a écrit :
>> On Wed, Mar 17, 2010 at 5:13 AM, David Miller <davem@davemloft.net> wrote:
>> >
>> > I'll integrate this as soon as I open up net-next-2.6
>>
>> It is really a good news. Now linux also can dispatch packets as
>> FreeBSD does via netisr. Can we walk farer, and support weighted
>> distribution?
>>
>
> May I ask why ? What would be the goal ?
>
For example, I have a firewall with dual core CPU, and use core 0 for
IRQ and dispatching. If I use both core 0 and core 1 for the left
processing, core 0 will be overloaded, and if I use core 1 for the
left processing, core 0 will be light load. In order to take full of
advantage of hardware, I need weighted the packet distribution.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
From: Yegor Yefremov @ 2010-03-17 9:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1268754720.3094.55.camel@edumazet-laptop>
On Tue, Mar 16, 2010 at 4:52 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 16 mars 2010 à 16:42 +0100, Yegor Yefremov a écrit :
>> KS8695: update ksp->next_rx_desc_read at the end of rx loop
>>
>> There is no need to adjust the next rx descriptor after each packet,
>> so do it only once at the end of the routine.
>>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>
>> Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
>> ===================================================================
>> --- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
>> +++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
>> @@ -538,12 +538,13 @@ rx_finished:
>> */
>> last_rx_processed = buff_n;
>> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
>> - /*And note which RX descriptor we last did */
>> - if (likely(last_rx_processed != -1))
>> - ksp->next_rx_desc_read =
>> - (last_rx_processed + 1) &
>> - MAX_RX_DESC_MASK;
>> }
>> +
>> + /*And note which RX descriptor we last did */
>> + if (likely(last_rx_processed != -1))
>> + ksp->next_rx_desc_read =
>> + (last_rx_processed + 1) & MAX_RX_DESC_MASK;
>> +
>
> Very strange to see all these computations...
>
> At this point, why not use
>
> if (likely(last_rx_processed != -1))
> ksp->next_rx_desc_read = buff_n;
>
> or even get rid of last_rx_processed completely and do :
>
>
> diff --git a/drivers/net/arm/ks8695net.c b/drivers/net/arm/ks8695net.c
> index a1d4188..ac6ab04 100644
> --- a/drivers/net/arm/ks8695net.c
> +++ b/drivers/net/arm/ks8695net.c
> @@ -461,7 +461,6 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
> int buff_n;
> u32 flags;
> int pktlen;
> - int last_rx_processed = -1;
> int received = 0;
>
> buff_n = ksp->next_rx_desc_read;
> @@ -533,17 +532,9 @@ rx_failure:
> ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
> rx_finished:
> received++;
> - /* And note this as processed so we can start
> - * from here next time
> - */
> - last_rx_processed = buff_n;
> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
> - /*And note which RX descriptor we last did */
> - if (likely(last_rx_processed != -1))
> - ksp->next_rx_desc_read =
> - (last_rx_processed + 1) &
> - MAX_RX_DESC_MASK;
> }
> + ksp->next_rx_desc_read = buff_n;
> /* And refill the buffers */
> ks8695_refill_rxbuffers(ksp);
Thank you for reviewing. Great idea. This simplifies the driver a
little. I tested your patch and everything is working as before. I
also added your signed-off-by and made some beatifications.
Yegor
^ permalink raw reply
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
From: Yegor Yefremov @ 2010-03-17 9:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <f69abfc31003170239k2d64aa8bgbe3f3ae59a074a05@mail.gmail.com>
KS8695: update ksp->next_rx_desc_read at the end of rx loop
There is no need to adjust the next rx descriptor after each packet,
so do it only once at the end of the routine.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
===================================================================
--- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
+++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
@@ -449,11 +449,10 @@ ks8695_rx_irq(int irq, void *dev_id)
}
/**
- * ks8695_rx - Receive packets called by NAPI poll method
+ * ks8695_rx - Receive packets called by NAPI poll method
* @ksp: Private data for the KS8695 Ethernet
- * @budget: The max packets would be receive
+ * @budget: Number of packets allowed to process
*/
-
static int ks8695_rx(struct ks8695_priv *ksp, int budget)
{
struct net_device *ndev = ksp->ndev;
@@ -461,7 +460,6 @@ static int ks8695_rx(struct ks8695_priv
int buff_n;
u32 flags;
int pktlen;
- int last_rx_processed = -1;
int received = 0;
buff_n = ksp->next_rx_desc_read;
@@ -471,6 +469,7 @@ static int ks8695_rx(struct ks8695_priv
cpu_to_le32(RDES_OWN)))) {
rmb();
flags = le32_to_cpu(ksp->rx_ring[buff_n].status);
+
/* Found an SKB which we own, this means we
* received a packet
*/
@@ -533,23 +532,18 @@ rx_failure:
ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
rx_finished:
received++;
- /* And note this as processed so we can start
- * from here next time
- */
- last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+
+ /* And note which RX descriptor we last did */
+ ksp->next_rx_desc_read = buff_n;
+
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
- /* Kick the RX DMA engine, in case it became
- * suspended */
+ /* Kick the RX DMA engine, in case it became suspended */
ks8695_writereg(ksp, KS8695_DRSC, 0);
+
return received;
}
^ permalink raw reply
* RE: [PATCH v1 2/3] Provides multiple submits and asynchronous notifications.
From: Xin, Xiaohui @ 2010-03-17 9:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, jdike@addtoit.com
In-Reply-To: <20100316113316.GA7137@redhat.com>
>> Michael,
>> I don't use the kiocb comes from the sendmsg/recvmsg,
> >since I have embeded the kiocb in page_info structure,
> >and allocate it when page_info allocated.
>So what I suggested was that vhost allocates and tracks the iocbs, and
>passes them to your device with sendmsg/ recvmsg calls. This way your
>device won't need to share structures and locking strategy with vhost:
>you get an iocb, handle it, invoke a callback to notify vhost about
>completion.
>This also gets rid of the 'receiver' callback
I'm not sure receiver callback can be removed here:
The patch describes a work flow like this:
netif_receive_skb() gets the packet, it does nothing but just queue the skb
and wakeup the handle_rx() of vhost. handle_rx() then calls the receiver callback
to deal with skb and and get the necessary notify info into a list, vhost owns the
list and in the same handle_rx() context use it to complete.
We use "receiver" callback here is because only handle_rx() is waked up from
netif_receive_skb(), and we need mp device context to deal with the skb and
notify info attached to it. We also have some lock in the callback function.
If I remove the receiver callback, I can only deal with the skb and notify
info in netif_receive_skb(), but this function is in an interrupt context,
which I think lock is not allowed there. But I cannot remove the lock there.
>> Please have a review and thanks for the instruction
>> for replying email which helps me a lot.
>>
> >Thanks,
> >Xiaohui
> >
> > drivers/vhost/net.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++--
>> drivers/vhost/vhost.h | 12 ++++
>> 2 files changed, 166 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >index 22d5fef..5483848 100644
> >--- a/drivers/vhost/net.c
> >+++ b/drivers/vhost/net.c
> >@@ -17,11 +17,13 @@
> > #include <linux/workqueue.h>
> > #include <linux/rcupdate.h>
> > #include <linux/file.h>
> >+#include <linux/aio.h>
> >
> > #include <linux/net.h>
> > #include <linux/if_packet.h>
> > #include <linux/if_arp.h>
> > #include <linux/if_tun.h>
> >+#include <linux/mpassthru.h>
> >
> > #include <net/sock.h>
> >
> >@@ -91,6 +93,12 @@ static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> > net->tx_poll_state = VHOST_NET_POLL_STARTED;
> > }
> >
> >+static void handle_async_rx_events_notify(struct vhost_net *net,
> >+ struct vhost_virtqueue *vq);
> >+
> >+static void handle_async_tx_events_notify(struct vhost_net *net,
> >+ struct vhost_virtqueue *vq);
> >+
>A couple of style comments:
>
>- It's better to arrange functions in such order that forward declarations
>aren't necessary. Since we don't have recursion, this should always be
>possible.
>- continuation lines should be idented at least at the position of '('
>on the previous line.
Thanks. I'd correct that.
>> /* Expects to be always run from workqueue - which acts as
>> * read-size critical section for our kind of RCU. */
>> static void handle_tx(struct vhost_net *net)
>> @@ -124,6 +132,8 @@ static void handle_tx(struct vhost_net *net)
>> tx_poll_stop(net);
>> hdr_size = vq->hdr_size;
>>
>> + handle_async_tx_events_notify(net, vq);
> >+
>> for (;;) {
>> head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
>> ARRAY_SIZE(vq->iov),
> >@@ -151,6 +161,12 @@ static void handle_tx(struct vhost_net *net)
>> /* Skip header. TODO: support TSO. */
>> s = move_iovec_hdr(vq->iov, vq->hdr, hdr_size, out);
>> msg.msg_iovlen = out;
> >+
> >+ if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> >+ vq->head = head;
> >+ msg.msg_control = (void *)vq;
>So here a device gets a pointer to vhost_virtqueue structure. If it gets
>an iocb and invokes a callback, it would not care about vhost internals.
>> + }
>> +
>> len = iov_length(vq->iov, out);
>> /* Sanity check */
>> if (!len) {
>> @@ -166,6 +182,10 @@ static void handle_tx(struct vhost_net *net)
>> tx_poll_start(net, sock);
>> break;
>> }
>> +
>> + if (vq->link_state == VHOST_VQ_LINK_ASYNC)
>> + continue;
>>+
>> if (err != len)
>> pr_err("Truncated TX packet: "
>> " len %d != %zd\n", err, len);
>> @@ -177,6 +197,8 @@ static void handle_tx(struct vhost_net *net)
>> }
>> }
>>
>> + handle_async_tx_events_notify(net, vq);
>> +
>> mutex_unlock(&vq->mutex);
>> unuse_mm(net->dev.mm);
>> }
>>@@ -206,7 +228,8 @@ static void handle_rx(struct vhost_net *net)
>> int err;
>> size_t hdr_size;
>> struct socket *sock = rcu_dereference(vq->private_data);
>> - if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
>> + if (!sock || (skb_queue_empty(&sock->sk->sk_receive_queue) &&
>> + vq->link_state == VHOST_VQ_LINK_SYNC))
>> return;
>>
>> use_mm(net->dev.mm);
>> @@ -214,9 +237,18 @@ static void handle_rx(struct vhost_net *net)
>> vhost_disable_notify(vq);
>> hdr_size = vq->hdr_size;
>>
>> - vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) ?
>> + /* In async cases, for write logging, the simple way is to get
>> + * the log info always, and really logging is decided later.
>>+ * Thus, when logging enabled, we can get log, and when logging
>> + * disabled, we can get log disabled accordingly.
>> + */
>> +
>This adds overhead and might be one of the reasons
>your patch does not perform that well. A better way
>would be to flush outstanding requests or reread the vq
>when logging is enabled.
Since the guest may submit a lot of buffers and h/w have already used them
to allocate host skb, it's difficult to know how many and which one is the
outstanding request, it's not just only inside in notifier list or sk->receive_queue.
But what does reread mean?
> + vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) |
> + (vq->link_state == VHOST_VQ_LINK_ASYNC) ?
> vq->log : NULL;
>
> + handle_async_rx_events_notify(net, vq);
> +
> for (;;) {
> head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> ARRAY_SIZE(vq->iov),
> @@ -245,6 +277,11 @@ static void handle_rx(struct vhost_net *net)
> s = move_iovec_hdr(vq->iov, vq->hdr, hdr_size, in);
> msg.msg_iovlen = in;
> len = iov_length(vq->iov, in);
> + if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> + vq->head = head;
> + vq->_log = log;
> + msg.msg_control = (void *)vq;
> + }
> /* Sanity check */
> if (!len) {
> vq_err(vq, "Unexpected header len for RX: "
> @@ -259,6 +296,10 @@ static void handle_rx(struct vhost_net *net)
> vhost_discard_vq_desc(vq);
> break;
> }
> +
> + if (vq->link_state == VHOST_VQ_LINK_ASYNC)
> + continue;
> +
> /* TODO: Should check and handle checksum. */
> if (err > len) {
> pr_err("Discarded truncated rx packet: "
> @@ -284,10 +325,85 @@ static void handle_rx(struct vhost_net *net)
> }
> }
>
> + handle_async_rx_events_notify(net, vq);
> +
> mutex_unlock(&vq->mutex);
> unuse_mm(net->dev.mm);
> }
>
> +struct kiocb *notify_dequeue(struct vhost_virtqueue *vq)
> +{
> + struct kiocb *iocb = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&vq->notify_lock, flags);
> + if (!list_empty(&vq->notifier)) {
> + iocb = list_first_entry(&vq->notifier,
> + struct kiocb, ki_list);
> + list_del(&iocb->ki_list);
> + }
> + spin_unlock_irqrestore(&vq->notify_lock, flags);
> + return iocb;
> +}
> +
> +static void handle_async_rx_events_notify(struct vhost_net *net,
> + struct vhost_virtqueue *vq)
> +{
> + struct kiocb *iocb = NULL;
> + struct vhost_log *vq_log = NULL;
> + int rx_total_len = 0;
> + int log, size;
> +
> + if (vq->link_state != VHOST_VQ_LINK_ASYNC)
> + return;
> + if (vq != &net->dev.vqs[VHOST_NET_VQ_RX])
> + return;
> +
> + if (vq->receiver)
> + vq->receiver(vq);
> + vq_log = unlikely(vhost_has_feature(
> + &net->dev, VHOST_F_LOG_ALL)) ? vq->log : NULL;
> + while ((iocb = notify_dequeue(vq)) != NULL) {
> + vhost_add_used_and_signal(&net->dev, vq,
> + iocb->ki_pos, iocb->ki_nbytes);
> + log = (int)iocb->ki_user_data;
> + size = iocb->ki_nbytes;
> + rx_total_len += iocb->ki_nbytes;
> + if (iocb->ki_dtor)
> + iocb->ki_dtor(iocb);
> + if (unlikely(vq_log))
> + vhost_log_write(vq, vq_log, log, size);
> + if (unlikely(rx_total_len >= VHOST_NET_WEIGHT)) {
> + vhost_poll_queue(&vq->poll);
> + break;
> + }
> + }
> +}
> +
> +static void handle_async_tx_events_notify(struct vhost_net *net,
> + struct vhost_virtqueue *vq)
> +{
> + struct kiocb *iocb = NULL;
> + int tx_total_len = 0;
> +
> + if (vq->link_state != VHOST_VQ_LINK_ASYNC)
> + return;
> + if (vq != &net->dev.vqs[VHOST_NET_VQ_TX])
> + return;
> +
Hard to see why the second check would be necessary
> + while ((iocb = notify_dequeue(vq)) != NULL) {
> + vhost_add_used_and_signal(&net->dev, vq,
> + iocb->ki_pos, 0);
> + tx_total_len += iocb->ki_nbytes;
> + if (iocb->ki_dtor)
> + iocb->ki_dtor(iocb);
> + if (unlikely(tx_total_len >= VHOST_NET_WEIGHT)) {
> + vhost_poll_queue(&vq->poll);
> + break;
> + }
> + }
> +}
> +
> static void handle_tx_kick(struct work_struct *work)
> {
> struct vhost_virtqueue *vq;
> @@ -462,7 +578,19 @@ static struct socket *get_tun_socket(int fd)
> return sock;
> }
>
> -static struct socket *get_socket(int fd)
> +static struct socket *get_mp_socket(int fd)
> +{
> + struct file *file = fget(fd);
> + struct socket *sock;
> + if (!file)
> + return ERR_PTR(-EBADF);
> + sock = mp_get_socket(file);
> + if (IS_ERR(sock))
> + fput(file);
> + return sock;
> +}
> +
> +static struct socket *get_socket(struct vhost_virtqueue *vq, int fd)
> {
> struct socket *sock;
> if (fd == -1)
> @@ -473,9 +601,26 @@ static struct socket *get_socket(int fd)
> sock = get_tun_socket(fd);
> if (!IS_ERR(sock))
> return sock;
> + sock = get_mp_socket(fd);
> + if (!IS_ERR(sock)) {
> + vq->link_state = VHOST_VQ_LINK_ASYNC;
> + return sock;
> + }
> return ERR_PTR(-ENOTSOCK);
> }
>
> +static void vhost_init_link_state(struct vhost_net *n, int index)
> +{
> + struct vhost_virtqueue *vq = n->vqs + index;
> +
> + WARN_ON(!mutex_is_locked(&vq->mutex));
> + if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> + vq->receiver = NULL;
> + INIT_LIST_HEAD(&vq->notifier);
> + spin_lock_init(&vq->notify_lock);
> + }
> +}
> +
> static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> {
> struct socket *sock, *oldsock;
> @@ -493,12 +638,15 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> }
> vq = n->vqs + index;
> mutex_lock(&vq->mutex);
> - sock = get_socket(fd);
> + vq->link_state = VHOST_VQ_LINK_SYNC;
> + sock = get_socket(vq, fd);
> if (IS_ERR(sock)) {
> r = PTR_ERR(sock);
> goto err;
> }
>
> + vhost_init_link_state(n, index);
> +
> /* start polling new socket */
> oldsock = vq->private_data;
> if (sock == oldsock)
> @@ -507,8 +655,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> vhost_net_disable_vq(n, vq);
> rcu_assign_pointer(vq->private_data, sock);
> vhost_net_enable_vq(n, vq);
> - mutex_unlock(&vq->mutex);
> done:
> + mutex_unlock(&vq->mutex);
> mutex_unlock(&n->dev.mutex);
> if (oldsock) {
> vhost_net_flush_vq(n, index);
> @@ -516,6 +664,7 @@ done:
> }
> return r;
> err:
> + mutex_unlock(&vq->mutex);
> mutex_unlock(&n->dev.mutex);
> return r;
> }
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index d1f0453..297af1c 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -43,6 +43,11 @@ struct vhost_log {
> u64 len;
> };
>
> +enum vhost_vq_link_state {
> + VHOST_VQ_LINK_SYNC = 0,
> + VHOST_VQ_LINK_ASYNC = 1,
> +};
> +
> /* The virtqueue structure describes a queue attached to a device. */
> struct vhost_virtqueue {
> struct vhost_dev *dev;
> @@ -96,6 +101,13 @@ struct vhost_virtqueue {
> /* Log write descriptors */
> void __user *log_base;
> struct vhost_log log[VHOST_NET_MAX_SG];
> + /*Differiate async socket for 0-copy from normal*/
> + enum vhost_vq_link_state link_state;
> + int head;
> + int _log;
> + struct list_head notifier;
> + spinlock_t notify_lock;
> + void (*receiver)(struct vhost_virtqueue *);
> };
>
> struct vhost_dev {
> --
> 1.5.4.4
^ permalink raw reply
* [Patch] netpoll: warn when there are spaces in parameters
From: Amerigo Wang @ 2010-03-17 10:12 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Amerigo Wang, David Miller
Currently, if we leave spaces before dst port,
netconsole will silently accept it as 0. Warn about this.
Also, when spaces appear in other places, make them
visible in error messages.
Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: David Miller <davem@davemloft.net>
---
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 7aa6972..6df1863 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -614,7 +614,7 @@ void netpoll_print_options(struct netpoll *np)
np->name, np->local_port);
printk(KERN_INFO "%s: local IP %pI4\n",
np->name, &np->local_ip);
- printk(KERN_INFO "%s: interface %s\n",
+ printk(KERN_INFO "%s: interface '%s'\n",
np->name, np->dev_name);
printk(KERN_INFO "%s: remote port %d\n",
np->name, np->remote_port);
@@ -661,6 +661,9 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
if ((delim = strchr(cur, '@')) == NULL)
goto parse_failed;
*delim = 0;
+ if (*cur == ' ' || *cur == '\t')
+ printk(KERN_INFO "%s: warning: white spaces"
+ "are not allowed.\n", np->name);
np->remote_port = simple_strtol(cur, NULL, 10);
cur = delim;
}
@@ -708,7 +711,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
return 0;
parse_failed:
- printk(KERN_INFO "%s: couldn't parse config at %s!\n",
+ printk(KERN_INFO "%s: couldn't parse config at '%s'!\n",
np->name, cur);
return -1;
}
^ permalink raw reply related
* [PATCH net-2.6 1/3] jme: Fix VLAN memory leak
From: cooldavid @ 2010-03-17 10:09 UTC (permalink / raw)
To: David Miller
Cc: Guo-Fu Tseng, linux-netdev, Ethan Hsiao, Devinchiu, Hsiu-Che Chao,
stable
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Fix memory leak while receiving 8021q tagged packet which is not
registered by user.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Cc: stable@kernel.org
---
drivers/net/jme.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 0f31497..cfc7b98 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -946,6 +946,8 @@ jme_alloc_and_feed_skb(struct jme_adapter *jme, int idx)
jme->jme_vlan_rx(skb, jme->vlgrp,
le16_to_cpu(rxdesc->descwb.vlan));
NET_STAT(jme).rx_bytes += 4;
+ } else {
+ dev_kfree_skb(skb);
}
} else {
jme->jme_rx(skb);
--
1.6.4.4
^ permalink raw reply related
* [PATCH net-2.6 2/3] jme: Protect vlgrp structure by pause RX actions.
From: cooldavid @ 2010-03-17 10:09 UTC (permalink / raw)
To: David Miller
Cc: Guo-Fu Tseng, linux-netdev, Ethan Hsiao, Devinchiu, Hsiu-Che Chao,
stable
In-Reply-To: <1268820571-2092-1-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Temporary stop the RX IRQ, and disable (sync) tasklet or napi.
And restore it after finished the vlgrp pointer assignment.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Cc: stable@kernel.org
---
drivers/net/jme.c | 33 +++++++++++++++++++++++++++++++++
1 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index cfc7b98..c0b59a5 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2083,12 +2083,45 @@ jme_tx_timeout(struct net_device *netdev)
jme_reset_link(jme);
}
+static inline void jme_pause_rx(struct jme_adapter *jme)
+{
+ atomic_dec(&jme->link_changing);
+
+ jme_set_rx_pcc(jme, PCC_OFF);
+ if (test_bit(JME_FLAG_POLL, &jme->flags)) {
+ JME_NAPI_DISABLE(jme);
+ } else {
+ tasklet_disable(&jme->rxclean_task);
+ tasklet_disable(&jme->rxempty_task);
+ }
+}
+
+static inline void jme_resume_rx(struct jme_adapter *jme)
+{
+ struct dynpcc_info *dpi = &(jme->dpi);
+
+ if (test_bit(JME_FLAG_POLL, &jme->flags)) {
+ JME_NAPI_ENABLE(jme);
+ } else {
+ tasklet_hi_enable(&jme->rxclean_task);
+ tasklet_hi_enable(&jme->rxempty_task);
+ }
+ dpi->cur = PCC_P1;
+ dpi->attempt = PCC_P1;
+ dpi->cnt = 0;
+ jme_set_rx_pcc(jme, PCC_P1);
+
+ atomic_inc(&jme->link_changing);
+}
+
static void
jme_vlan_rx_register(struct net_device *netdev, struct vlan_group *grp)
{
struct jme_adapter *jme = netdev_priv(netdev);
+ jme_pause_rx(jme);
jme->vlgrp = grp;
+ jme_resume_rx(jme);
}
static void
--
1.6.4.4
^ permalink raw reply related
* [PATCH net-2.6 3/3] jme: Advance driver version number
From: cooldavid @ 2010-03-17 10:09 UTC (permalink / raw)
To: David Miller
Cc: Guo-Fu Tseng, linux-netdev, Ethan Hsiao, Devinchiu, Hsiu-Che Chao
In-Reply-To: <1268820571-2092-2-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Advance driver version number after some bug fix.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/jme.h b/drivers/net/jme.h
index c19db91..07ad3a4 100644
--- a/drivers/net/jme.h
+++ b/drivers/net/jme.h
@@ -25,7 +25,7 @@
#define __JME_H_INCLUDED__
#define DRV_NAME "jme"
-#define DRV_VERSION "1.0.5"
+#define DRV_VERSION "1.0.6"
#define PFX DRV_NAME ": "
#define PCI_DEVICE_ID_JMICRON_JMC250 0x0250
--
1.6.4.4
^ permalink raw reply related
* Re: [PATCH v1 2/3] Provides multiple submits and asynchronous notifications.
From: Michael S. Tsirkin @ 2010-03-17 10:27 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, jdike@addtoit.com
In-Reply-To: <97F6D3BD476C464182C1B7BABF0B0AF5C16AB126@shzsmsx502.ccr.corp.intel.com>
On Wed, Mar 17, 2010 at 05:48:10PM +0800, Xin, Xiaohui wrote:
> >> Michael,
> >> I don't use the kiocb comes from the sendmsg/recvmsg,
> > >since I have embeded the kiocb in page_info structure,
> > >and allocate it when page_info allocated.
>
> >So what I suggested was that vhost allocates and tracks the iocbs, and
> >passes them to your device with sendmsg/ recvmsg calls. This way your
> >device won't need to share structures and locking strategy with vhost:
> >you get an iocb, handle it, invoke a callback to notify vhost about
> >completion.
>
> >This also gets rid of the 'receiver' callback
>
> I'm not sure receiver callback can be removed here:
> The patch describes a work flow like this:
> netif_receive_skb() gets the packet, it does nothing but just queue the skb
> and wakeup the handle_rx() of vhost. handle_rx() then calls the receiver callback
> to deal with skb and and get the necessary notify info into a list, vhost owns the
> list and in the same handle_rx() context use it to complete.
>
> We use "receiver" callback here is because only handle_rx() is waked up from
> netif_receive_skb(), and we need mp device context to deal with the skb and
> notify info attached to it. We also have some lock in the callback function.
>
> If I remove the receiver callback, I can only deal with the skb and notify
> info in netif_receive_skb(), but this function is in an interrupt context,
> which I think lock is not allowed there. But I cannot remove the lock there.
>
The basic idea is that vhost passes iocb to recvmsg and backend
completes the iocb to signal that data is ready. That completion could
be in interrupt context and so we need to switch to workqueue to handle
the event, it is true, but the code to do this would live in vhost.c or
net.c.
With this structure your device won't depend on
vhost, and can go under drivers/net/, opening up possibility
to use it for zero copy without vhost in the future.
> >> Please have a review and thanks for the instruction
> >> for replying email which helps me a lot.
> >>
> > >Thanks,
> > >Xiaohui
> > >
> > > drivers/vhost/net.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++--
> >> drivers/vhost/vhost.h | 12 ++++
> >> 2 files changed, 166 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > >index 22d5fef..5483848 100644
> > >--- a/drivers/vhost/net.c
> > >+++ b/drivers/vhost/net.c
> > >@@ -17,11 +17,13 @@
> > > #include <linux/workqueue.h>
> > > #include <linux/rcupdate.h>
> > > #include <linux/file.h>
> > >+#include <linux/aio.h>
> > >
> > > #include <linux/net.h>
> > > #include <linux/if_packet.h>
> > > #include <linux/if_arp.h>
> > > #include <linux/if_tun.h>
> > >+#include <linux/mpassthru.h>
> > >
> > > #include <net/sock.h>
> > >
> > >@@ -91,6 +93,12 @@ static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> > > net->tx_poll_state = VHOST_NET_POLL_STARTED;
> > > }
> > >
> > >+static void handle_async_rx_events_notify(struct vhost_net *net,
> > >+ struct vhost_virtqueue *vq);
> > >+
> > >+static void handle_async_tx_events_notify(struct vhost_net *net,
> > >+ struct vhost_virtqueue *vq);
> > >+
>
> >A couple of style comments:
> >
> >- It's better to arrange functions in such order that forward declarations
> >aren't necessary. Since we don't have recursion, this should always be
> >possible.
>
> >- continuation lines should be idented at least at the position of '('
> >on the previous line.
>
> Thanks. I'd correct that.
>
> >> /* Expects to be always run from workqueue - which acts as
> >> * read-size critical section for our kind of RCU. */
> >> static void handle_tx(struct vhost_net *net)
> >> @@ -124,6 +132,8 @@ static void handle_tx(struct vhost_net *net)
> >> tx_poll_stop(net);
> >> hdr_size = vq->hdr_size;
> >>
> >> + handle_async_tx_events_notify(net, vq);
> > >+
> >> for (;;) {
> >> head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> >> ARRAY_SIZE(vq->iov),
> > >@@ -151,6 +161,12 @@ static void handle_tx(struct vhost_net *net)
> >> /* Skip header. TODO: support TSO. */
> >> s = move_iovec_hdr(vq->iov, vq->hdr, hdr_size, out);
> >> msg.msg_iovlen = out;
> > >+
> > >+ if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> > >+ vq->head = head;
> > >+ msg.msg_control = (void *)vq;
>
> >So here a device gets a pointer to vhost_virtqueue structure. If it gets
> >an iocb and invokes a callback, it would not care about vhost internals.
>
> >> + }
> >> +
> >> len = iov_length(vq->iov, out);
> >> /* Sanity check */
> >> if (!len) {
> >> @@ -166,6 +182,10 @@ static void handle_tx(struct vhost_net *net)
> >> tx_poll_start(net, sock);
> >> break;
> >> }
> >> +
> >> + if (vq->link_state == VHOST_VQ_LINK_ASYNC)
> >> + continue;
> >>+
> >> if (err != len)
> >> pr_err("Truncated TX packet: "
> >> " len %d != %zd\n", err, len);
> >> @@ -177,6 +197,8 @@ static void handle_tx(struct vhost_net *net)
> >> }
> >> }
> >>
> >> + handle_async_tx_events_notify(net, vq);
> >> +
> >> mutex_unlock(&vq->mutex);
> >> unuse_mm(net->dev.mm);
> >> }
> >>@@ -206,7 +228,8 @@ static void handle_rx(struct vhost_net *net)
> >> int err;
> >> size_t hdr_size;
> >> struct socket *sock = rcu_dereference(vq->private_data);
> >> - if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> >> + if (!sock || (skb_queue_empty(&sock->sk->sk_receive_queue) &&
> >> + vq->link_state == VHOST_VQ_LINK_SYNC))
> >> return;
> >>
> >> use_mm(net->dev.mm);
> >> @@ -214,9 +237,18 @@ static void handle_rx(struct vhost_net *net)
> >> vhost_disable_notify(vq);
> >> hdr_size = vq->hdr_size;
> >>
> >> - vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) ?
> >> + /* In async cases, for write logging, the simple way is to get
> >> + * the log info always, and really logging is decided later.
> >>+ * Thus, when logging enabled, we can get log, and when logging
> >> + * disabled, we can get log disabled accordingly.
> >> + */
> >> +
>
>
> >This adds overhead and might be one of the reasons
> >your patch does not perform that well. A better way
> >would be to flush outstanding requests or reread the vq
> >when logging is enabled.
>
> Since the guest may submit a lot of buffers and h/w have already used them
> to allocate host skb, it's difficult to know how many and which one is the
> outstanding request, it's not just only inside in notifier list or sk->receive_queue.
Well, that was just a thought. I guess there needs to be some way to
recover outstanding requests at least for cleanup when device is closed?
Maybe we could put in a special request marked "flush" and wait until it
completes?
> But what does reread mean?
If we want to know the physical address of the iovec, we can look in the
virtqueue to find it. I do this in one go when building up the iovec
now, but logged mode is not a common case so it is not a must.
> > + vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) |
> > + (vq->link_state == VHOST_VQ_LINK_ASYNC) ?
> > vq->log : NULL;
> >
> > + handle_async_rx_events_notify(net, vq);
> > +
> > for (;;) {
> > head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> > ARRAY_SIZE(vq->iov),
> > @@ -245,6 +277,11 @@ static void handle_rx(struct vhost_net *net)
> > s = move_iovec_hdr(vq->iov, vq->hdr, hdr_size, in);
> > msg.msg_iovlen = in;
> > len = iov_length(vq->iov, in);
> > + if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> > + vq->head = head;
> > + vq->_log = log;
> > + msg.msg_control = (void *)vq;
> > + }
> > /* Sanity check */
> > if (!len) {
> > vq_err(vq, "Unexpected header len for RX: "
> > @@ -259,6 +296,10 @@ static void handle_rx(struct vhost_net *net)
> > vhost_discard_vq_desc(vq);
> > break;
> > }
> > +
> > + if (vq->link_state == VHOST_VQ_LINK_ASYNC)
> > + continue;
> > +
> > /* TODO: Should check and handle checksum. */
> > if (err > len) {
> > pr_err("Discarded truncated rx packet: "
> > @@ -284,10 +325,85 @@ static void handle_rx(struct vhost_net *net)
> > }
> > }
> >
> > + handle_async_rx_events_notify(net, vq);
> > +
> > mutex_unlock(&vq->mutex);
> > unuse_mm(net->dev.mm);
> > }
> >
> > +struct kiocb *notify_dequeue(struct vhost_virtqueue *vq)
> > +{
> > + struct kiocb *iocb = NULL;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&vq->notify_lock, flags);
> > + if (!list_empty(&vq->notifier)) {
> > + iocb = list_first_entry(&vq->notifier,
> > + struct kiocb, ki_list);
> > + list_del(&iocb->ki_list);
> > + }
> > + spin_unlock_irqrestore(&vq->notify_lock, flags);
> > + return iocb;
> > +}
> > +
> > +static void handle_async_rx_events_notify(struct vhost_net *net,
> > + struct vhost_virtqueue *vq)
> > +{
> > + struct kiocb *iocb = NULL;
> > + struct vhost_log *vq_log = NULL;
> > + int rx_total_len = 0;
> > + int log, size;
> > +
> > + if (vq->link_state != VHOST_VQ_LINK_ASYNC)
> > + return;
> > + if (vq != &net->dev.vqs[VHOST_NET_VQ_RX])
> > + return;
> > +
> > + if (vq->receiver)
> > + vq->receiver(vq);
> > + vq_log = unlikely(vhost_has_feature(
> > + &net->dev, VHOST_F_LOG_ALL)) ? vq->log : NULL;
> > + while ((iocb = notify_dequeue(vq)) != NULL) {
> > + vhost_add_used_and_signal(&net->dev, vq,
> > + iocb->ki_pos, iocb->ki_nbytes);
> > + log = (int)iocb->ki_user_data;
> > + size = iocb->ki_nbytes;
> > + rx_total_len += iocb->ki_nbytes;
> > + if (iocb->ki_dtor)
> > + iocb->ki_dtor(iocb);
> > + if (unlikely(vq_log))
> > + vhost_log_write(vq, vq_log, log, size);
> > + if (unlikely(rx_total_len >= VHOST_NET_WEIGHT)) {
> > + vhost_poll_queue(&vq->poll);
> > + break;
> > + }
> > + }
> > +}
> > +
> > +static void handle_async_tx_events_notify(struct vhost_net *net,
> > + struct vhost_virtqueue *vq)
> > +{
> > + struct kiocb *iocb = NULL;
> > + int tx_total_len = 0;
> > +
> > + if (vq->link_state != VHOST_VQ_LINK_ASYNC)
> > + return;
> > + if (vq != &net->dev.vqs[VHOST_NET_VQ_TX])
> > + return;
> > +
>
> Hard to see why the second check would be necessary
>
> > + while ((iocb = notify_dequeue(vq)) != NULL) {
> > + vhost_add_used_and_signal(&net->dev, vq,
> > + iocb->ki_pos, 0);
> > + tx_total_len += iocb->ki_nbytes;
> > + if (iocb->ki_dtor)
> > + iocb->ki_dtor(iocb);
> > + if (unlikely(tx_total_len >= VHOST_NET_WEIGHT)) {
> > + vhost_poll_queue(&vq->poll);
> > + break;
> > + }
> > + }
> > +}
> > +
> > static void handle_tx_kick(struct work_struct *work)
> > {
> > struct vhost_virtqueue *vq;
> > @@ -462,7 +578,19 @@ static struct socket *get_tun_socket(int fd)
> > return sock;
> > }
> >
> > -static struct socket *get_socket(int fd)
> > +static struct socket *get_mp_socket(int fd)
> > +{
> > + struct file *file = fget(fd);
> > + struct socket *sock;
> > + if (!file)
> > + return ERR_PTR(-EBADF);
> > + sock = mp_get_socket(file);
> > + if (IS_ERR(sock))
> > + fput(file);
> > + return sock;
> > +}
> > +
> > +static struct socket *get_socket(struct vhost_virtqueue *vq, int fd)
> > {
> > struct socket *sock;
> > if (fd == -1)
> > @@ -473,9 +601,26 @@ static struct socket *get_socket(int fd)
> > sock = get_tun_socket(fd);
> > if (!IS_ERR(sock))
> > return sock;
> > + sock = get_mp_socket(fd);
> > + if (!IS_ERR(sock)) {
> > + vq->link_state = VHOST_VQ_LINK_ASYNC;
> > + return sock;
> > + }
> > return ERR_PTR(-ENOTSOCK);
> > }
> >
> > +static void vhost_init_link_state(struct vhost_net *n, int index)
> > +{
> > + struct vhost_virtqueue *vq = n->vqs + index;
> > +
> > + WARN_ON(!mutex_is_locked(&vq->mutex));
> > + if (vq->link_state == VHOST_VQ_LINK_ASYNC) {
> > + vq->receiver = NULL;
> > + INIT_LIST_HEAD(&vq->notifier);
> > + spin_lock_init(&vq->notify_lock);
> > + }
> > +}
> > +
> > static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > {
> > struct socket *sock, *oldsock;
> > @@ -493,12 +638,15 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > }
> > vq = n->vqs + index;
> > mutex_lock(&vq->mutex);
> > - sock = get_socket(fd);
> > + vq->link_state = VHOST_VQ_LINK_SYNC;
> > + sock = get_socket(vq, fd);
> > if (IS_ERR(sock)) {
> > r = PTR_ERR(sock);
> > goto err;
> > }
> >
> > + vhost_init_link_state(n, index);
> > +
> > /* start polling new socket */
> > oldsock = vq->private_data;
> > if (sock == oldsock)
> > @@ -507,8 +655,8 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> > vhost_net_disable_vq(n, vq);
> > rcu_assign_pointer(vq->private_data, sock);
> > vhost_net_enable_vq(n, vq);
> > - mutex_unlock(&vq->mutex);
> > done:
> > + mutex_unlock(&vq->mutex);
> > mutex_unlock(&n->dev.mutex);
> > if (oldsock) {
> > vhost_net_flush_vq(n, index);
> > @@ -516,6 +664,7 @@ done:
> > }
> > return r;
> > err:
> > + mutex_unlock(&vq->mutex);
> > mutex_unlock(&n->dev.mutex);
> > return r;
> > }
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index d1f0453..297af1c 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -43,6 +43,11 @@ struct vhost_log {
> > u64 len;
> > };
> >
> > +enum vhost_vq_link_state {
> > + VHOST_VQ_LINK_SYNC = 0,
> > + VHOST_VQ_LINK_ASYNC = 1,
> > +};
> > +
> > /* The virtqueue structure describes a queue attached to a device. */
> > struct vhost_virtqueue {
> > struct vhost_dev *dev;
> > @@ -96,6 +101,13 @@ struct vhost_virtqueue {
> > /* Log write descriptors */
> > void __user *log_base;
> > struct vhost_log log[VHOST_NET_MAX_SG];
> > + /*Differiate async socket for 0-copy from normal*/
> > + enum vhost_vq_link_state link_state;
> > + int head;
> > + int _log;
> > + struct list_head notifier;
> > + spinlock_t notify_lock;
> > + void (*receiver)(struct vhost_virtqueue *);
> > };
> >
> > struct vhost_dev {
> > --
> > 1.5.4.4
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-17 10:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <1268811437.2932.66.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 6036 bytes --]
Hello,
We did pktgen over LAN testing for several hours:
iftop
19.1Mb 38.1Mb 57.2Mb 76.3Mb 95.4Mb
└──────────────────────┴──────────────────────┴───────────────────────┴──────────────────────┴───────────────────────
xxxxxxxx0007t2 => xxxxxxxxxxxxxx.xxxxx.xxxx.xxx 0b 3.60Kb 4.28Kb
<= 92.7Mb 92.7Mb 92.5Mb
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
TX: cumm: 2.50MB peak: 57.7Kb rates: 7.71Kb 5.97Kb 10.7Kb
RX: 28.0GB 92.9Mb 92.7Mb 92.8Mb 92.6Mb
TOTAL: 28.0GB 92.9Mb 92.7Mb 92.8Mb 92.6M
Without any problems.
As soon as I switched back to localhost "pollution":
[ 7343.999279] ------------[ cut here ]------------
[ 7343.999292] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
[ 7343.999295] Hardware name: F3JC
[ 7343.999298] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
[ 7343.999301] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel mmc_core
asus_laptop sparse_keymap snd_hda_codec led_class snd_pcm rng_core sg psmouse snd_timer snd_page_alloc i2c_i801 evdev snd soundcore serio_raw r8169 mii usbhid hid uhci_hcd
ehci_hcd sr_mod cdrom sd_mod usbcore ata_piix
[ 7343.999361] Pid: 4801, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
[ 7343.999364] Call Trace:
[ 7343.999372] [<c102e293>] warn_slowpath_common+0x65/0x7c
[ 7343.999378] [<c126ac34>] ? dev_watchdog+0xc1/0x125
[ 7343.999383] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
[ 7343.999388] [<c126ac34>] dev_watchdog+0xc1/0x125
[ 7343.999395] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 7343.999401] [<c1036b51>] run_timer_softirq+0x176/0x1eb
[ 7343.999406] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 7343.999411] [<c126ab73>] ? dev_watchdog+0x0/0x125
[ 7343.999417] [<c1032d39>] __do_softirq+0x8d/0x117
[ 7343.999422] [<c1032dee>] do_softirq+0x2b/0x43
[ 7343.999426] [<c1032f13>] irq_exit+0x38/0x75
[ 7343.999433] [<c1014e75>] smp_apic_timer_interrupt+0x66/0x74
[ 7343.999438] [<c12c770a>] apic_timer_interrupt+0x36/0x3c
[ 7343.999447] [<f80878df>] ? pktgen_thread_worker+0x5c4/0x631 [pktgen]
[ 7343.999454] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
[ 7343.999459] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
[ 7343.999465] [<f808731b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
[ 7343.999470] [<c103f60e>] kthread+0x6a/0x6f
[ 7343.999476] [<c103f5a4>] ? kthread+0x0/0x6f
[ 7343.999481] [<c1002e42>] kernel_thread_helper+0x6/0x1a
[ 7343.999485] ---[ end trace a22d306b065d4a68 ]---
[ 7344.012654] r8169 0000:02:00.0: eth0: link up
[ 7356.012657] r8169 0000:02:00.0: eth0: link up
[ 7368.013545] r8169 0000:02:00.0: eth0: link up
Sergey
On (03/17/10 08:37), Eric Dumazet wrote:
> trimming some cc
>
> Le mercredi 17 mars 2010 à 09:25 +0200, Sergey Senozhatsky a écrit :
> > Hello,
> >
> > cumulative patch:
> >
> > [ 155.337373] ------------[ cut here ]------------
> > [ 155.337386] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> > [ 155.337390] Hardware name: F3JC
> > [ 155.337394] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> > [ 155.337397] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel snd_hda_codec
> > asus_laptop sparse_keymap mmc_core led_class snd_pcm snd_timer snd_page_alloc psmouse rng_core snd soundcore sg i2c_i801 evdev serio_raw r8169 mii usbhid hid uhci_hcd ehci_hcd
> > sr_mod cdrom sd_mod usbcore ata_piix
> > [ 155.337468] Pid: 7, comm: ksoftirqd/1 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
> > [ 155.337472] Call Trace:
> > [ 155.337481] [<c102e293>] warn_slowpath_common+0x65/0x7c
> > [ 155.337506] [<c126ac34>] ? dev_watchdog+0xc1/0x125
> > [ 155.337512] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
> > [ 155.337517] [<c126ac34>] dev_watchdog+0xc1/0x125
> > [ 155.337525] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> > [ 155.337530] [<c1036b51>] run_timer_softirq+0x176/0x1eb
> > [ 155.337536] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> > [ 155.337566] [<c126ab73>] ? dev_watchdog+0x0/0x125
> > [ 155.337576] [<c1032d39>] __do_softirq+0x8d/0x117
> > [ 155.337667] [<c1032dee>] do_softirq+0x2b/0x43
> > [ 155.337729] [<c1032fc1>] run_ksoftirqd+0x71/0x140
> > [ 155.337745] [<c1032f50>] ? run_ksoftirqd+0x0/0x140
> > [ 155.337810] [<c103f60e>] kthread+0x6a/0x6f
> > [ 155.337832] [<c103f5a4>] ? kthread+0x0/0x6f
> > [ 155.337903] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> > [ 155.337907] ---[ end trace a22d306b065d4a68 ]---
> > [ 155.350902] r8169 0000:02:00.0: eth0: link up
> > [ 167.350892] r8169 0000:02:00.0: eth0: link up
> >
> >
> >
>
> On receiver ?
>
> I suspect lot of work is needed on this driver to make it working, but I
> dont have a machine with said adapter.
>
> Are you in 100 Mb full duplex mode ?
>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [Patch] netpoll: warn when there are spaces in parameters
From: Frans Pop @ 2010-03-17 10:58 UTC (permalink / raw)
To: Amerigo Wang; +Cc: linux-kernel, netdev, amwang, davem
In-Reply-To: <20100317101635.5948.23485.sendpatchset@localhost.localdomain>
Amerigo Wang wrote:
> + if (*cur == ' ' || *cur == '\t')
> + printk(KERN_INFO "%s: warning: white spaces"
> + "are not allowed.\n", np->name);
>
The term "white spaces" not correct. Please change the message to
"whitespace is not allowed".
Also, it's normally not necessary to close kernel messages with a period;
most kernel messages do not have a closing period. Messages are not
sentences and the periods only help increase the kernel size.
Thanks,
FJP
^ permalink raw reply
* RE: [PATCH net-next-2.6 v6 08/12] net-caif: add CAIF socket implementation
From: Sjur BRENDELAND @ 2010-03-17 11:04 UTC (permalink / raw)
To: Marcel Holtmann
Cc: netdev@vger.kernel.org, davem@davemloft.net, Daniel MARTENSSON,
kaber@trash.net, stefano.babic@babic.homelinux.org,
randy.dunlap@oracle.com
In-Reply-To: <1268805781.2700.46.camel@localhost.localdomain>
Hi Marcel,
>> >> + * The sock->type specifies the socket type to use. The CAIF
>> >> socket is + * a packet stream in the sence that it is packet based.
>> >> + * CAIF trusts the reliability of the link, no resending is
>> >> implemented. + */ + if (sock->type != SOCK_SEQPACKET)
>> >> + return -ESOCKTNOSUPPORT;
>> >
>> > we came to an interesting detail here when testing with a STE modem.
>> > Why is this SEQPACKET and not a STREAM.
>>
>> The reason is that CAIF provides different services not just AT,
>> and some of them are really packet oriented such as Utility links and
>> Video. It would not be right to provide a stream based solution in this case.
>>
>> > Especially with the AT
>> > command channels it is kinda weird that you have an MTU. The AT
>> > specification doesn't really have any defined behavior when using a
>> > sequential packet transport. It is more a stream based socket.
>>
>> Yes I see your point. In order to limit the effort and simplify
>> caif_socket we ended up implementing only SEQPACKET.
>
>I think we need to split this. Use SEQPACKET for packet based services
>and have STREAM for AT command channel.
>So while this might cause internally some more code. It would take away
>the complexity from userspace to turn it into a stream. And that is what
>the userspace is expecting.
Ok Marcel, I'll look into supporting both STREAM and SEQPACKET.
BR/Sjur
^ permalink raw reply
* Re: tcp_reordering as 0 possible?
From: Ilpo Järvinen @ 2010-03-17 11:38 UTC (permalink / raw)
To: raj ravi; +Cc: netdev
In-Reply-To: <de0a9dcb1003150725h367d0548j35037441fdb2f5df@mail.gmail.com>
On Mon, 15 Mar 2010, raj ravi wrote:
> what is the behaviour in TCP stack if I set tcp_reordering as 0.
> So , sender will start retransmission without waiting for any duplicate ACK ?
> /proc/sys/net/ipv4/tcp_reordering - Please clarify.
> The default value is 3 which means it waits until 3 duplicate ack's
> arrive and then start retransmission.
>
> "The TCP sender should use the fast retransmit algorithm to detect and
> repair loss based on incoming duplicate ACKs. After the arrival of 3
> duplicate ACKs (4 identical ACKs without the arrival of any other
> intervening packet), TCP performs a retransmission of what appears to
> be the missing segment, without waiting for the retransmission timer
> to expire."
Depends on other things quite much but for a typical transfer you'd still
need one duplicate ACK to trigger actual recovery. However, in general
root is not (always) forbidden to set non-sensical values for sysctls.
--
i.
^ permalink raw reply
* [PATCH] tcp: Fix tcp_mark_head_lost() with packets == 0
From: Lennart Schulte @ 2010-03-17 12:16 UTC (permalink / raw)
To: netdev; +Cc: ilpo.jarvinen, Lennart Schulte, Arnd Hannemann
A packet is marked as lost in case packets == 0, although nothing should be done.
This results in a too early retransmitted packet during recovery in some cases.
This small patch fixes this issue by returning immediately.
Signed-off-by: Lennart Schulte <lennart.schulte@nets.rwth-aachen.de>
Signed-off-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
---
net/ipv4/tcp_input.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 788851c..c096a42 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2511,6 +2511,9 @@ static void tcp_mark_head_lost(struct sock *sk, int packets)
int err;
unsigned int mss;
+ if (packets == 0)
+ return;
+
WARN_ON(packets > tp->packets_out);
if (tp->lost_skb_hint) {
skb = tp->lost_skb_hint;
--
1.6.4.1
^ permalink raw reply related
* Re: tcp_reordering as 0 possible?
From: raj ravi @ 2010-03-17 12:23 UTC (permalink / raw)
To: netdev; +Cc: Ilpo Järvinen
In-Reply-To: <alpine.DEB.2.00.1003171333250.19416@melkinpaasi.cs.helsinki.fi>
On Wed, Mar 17, 2010 at 5:08 PM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> On Mon, 15 Mar 2010, raj ravi wrote:
>
>> what is the behaviour in TCP stack if I set tcp_reordering as 0.
>> So , sender will start retransmission without waiting for any duplicate ACK ?
>> /proc/sys/net/ipv4/tcp_reordering - Please clarify.
>> The default value is 3 which means it waits until 3 duplicate ack's
>> arrive and then start retransmission.
>>
>> "The TCP sender should use the fast retransmit algorithm to detect and
>> repair loss based on incoming duplicate ACKs. After the arrival of 3
>> duplicate ACKs (4 identical ACKs without the arrival of any other
>> intervening packet), TCP performs a retransmission of what appears to
>> be the missing segment, without waiting for the retransmission timer
>> to expire."
>
> Depends on other things quite much but for a typical transfer you'd still
> need one duplicate ACK to trigger actual recovery. However, in general
> root is not (always) forbidden to set non-sensical values for sysctls.
>
> --
> i.
>
Hmm....Is that mean setting the value as 0 is non-sensical ?
OR
After setting the value as 0, TCP Stack doesn't expect any drops to
occur, so that there won't be any recovery required and if any drops
occur it leads to chaos from application point of view as it expects
all the packets ...correct?
Actually I set the value as 0 and tried running firefox with few URLs
...but my machine Crashed !
I think this is expected as This lead to chaos in the stack...
But If the same value is tried between two machines connected
together directly running iperf , There wont be any issues.
Thx,
Kavi
^ permalink raw reply
* Re: [PATCH] drivers/net/wimax/i2400m/fw.c fix possible double free
From: Darren Jenkins @ 2010-03-17 12:40 UTC (permalink / raw)
To: David Miller
Cc: inaky.perez-gonzalez, linux-wimax, kernel-janitors, cindy.h.kao,
dirk.j.brandewie, wimax, netdev, Linux Kernel Mailing List
On Wed, Mar 17, 2010 at 8:14 AM, David Miller <davem@davemloft.net>
wrote:
> Therefore the krealloc() failure handling in this driver should NULL
> out i2400m->fw_hdrs and that will fix the double kfree problem as well
> as trap any stray references.
Yes that is a much better Idea. Thanks for the advice.
It also fixes the i2400m_barker_db problem that I didn't notice before.
Fix double free on krealloc() failure by zeroing pointer
coverity CID: 13455
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
---
drivers/net/wimax/i2400m/fw.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index 25c24f0..9f3b594 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -232,8 +232,9 @@ int i2400m_zrealloc_2x(void **ptr, size_t *_count, size_t el_size,
*_count = new_count;
*ptr = nptr;
return 0;
- } else
- return -ENOMEM;
+ }
+ *ptr = NULL;
+ return -ENOMEM;
}
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-17 13:07 UTC (permalink / raw)
To: Timo Teras; +Cc: netdev
In-Reply-To: <1268655610-7845-1-git-send-email-timo.teras@iki.fi>
On Mon, Mar 15, 2010 at 02:20:10PM +0200, Timo Teras wrote:
> Instead of doing O(n) xfrm_find_bundle() call per-packet, cache
> the previous lookup results in flow cache. The flow cache is
> updated to be per-netns and more generic.
This only works well if the traffic doesn't switch bundles much.
But if that were the case then the number of bundles is likely
to be small anyway.
IOW I think if we're doing this then we should go the whole
distance and directly cache bundles instead of policies in the
flow cache.
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: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Eric Dumazet @ 2010-03-17 13:54 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <20100317105856.GB3322@swordfish>
Le mercredi 17 mars 2010 à 12:58 +0200, Sergey Senozhatsky a écrit :
> Hello,
>
> We did pktgen over LAN testing for several hours:
>
> iftop
> 19.1Mb 38.1Mb 57.2Mb 76.3Mb 95.4Mb
> └──────────────────────┴──────────────────────┴───────────────────────┴──────────────────────┴───────────────────────
> xxxxxxxx0007t2 => xxxxxxxxxxxxxx.xxxxx.xxxx.xxx 0b 3.60Kb 4.28Kb
> <= 92.7Mb 92.7Mb 92.5Mb
>
> ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
> TX: cumm: 2.50MB peak: 57.7Kb rates: 7.71Kb 5.97Kb 10.7Kb
> RX: 28.0GB 92.9Mb 92.7Mb 92.8Mb 92.6Mb
> TOTAL: 28.0GB 92.9Mb 92.7Mb 92.8Mb 92.6M
>
> Without any problems.
>
>
> As soon as I switched back to localhost "pollution":
>
> [ 7343.999279] ------------[ cut here ]------------
> [ 7343.999292] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
> [ 7343.999295] Hardware name: F3JC
> [ 7343.999298] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
> [ 7343.999301] Modules linked in: pktgen ppp_async crc_ccitt ipv6 ppp_generic slhc snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek sdhci_pci sdhci snd_hda_intel mmc_core
> asus_laptop sparse_keymap snd_hda_codec led_class snd_pcm rng_core sg psmouse snd_timer snd_page_alloc i2c_i801 evdev snd soundcore serio_raw r8169 mii usbhid hid uhci_hcd
> ehci_hcd sr_mod cdrom sd_mod usbcore ata_piix
> [ 7343.999361] Pid: 4801, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #47
> [ 7343.999364] Call Trace:
> [ 7343.999372] [<c102e293>] warn_slowpath_common+0x65/0x7c
> [ 7343.999378] [<c126ac34>] ? dev_watchdog+0xc1/0x125
> [ 7343.999383] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
> [ 7343.999388] [<c126ac34>] dev_watchdog+0xc1/0x125
> [ 7343.999395] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> [ 7343.999401] [<c1036b51>] run_timer_softirq+0x176/0x1eb
> [ 7343.999406] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
> [ 7343.999411] [<c126ab73>] ? dev_watchdog+0x0/0x125
> [ 7343.999417] [<c1032d39>] __do_softirq+0x8d/0x117
> [ 7343.999422] [<c1032dee>] do_softirq+0x2b/0x43
> [ 7343.999426] [<c1032f13>] irq_exit+0x38/0x75
> [ 7343.999433] [<c1014e75>] smp_apic_timer_interrupt+0x66/0x74
> [ 7343.999438] [<c12c770a>] apic_timer_interrupt+0x36/0x3c
> [ 7343.999447] [<f80878df>] ? pktgen_thread_worker+0x5c4/0x631 [pktgen]
> [ 7343.999454] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
> [ 7343.999459] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
> [ 7343.999465] [<f808731b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
> [ 7343.999470] [<c103f60e>] kthread+0x6a/0x6f
> [ 7343.999476] [<c103f5a4>] ? kthread+0x0/0x6f
> [ 7343.999481] [<c1002e42>] kernel_thread_helper+0x6/0x1a
> [ 7343.999485] ---[ end trace a22d306b065d4a68 ]---
> [ 7344.012654] r8169 0000:02:00.0: eth0: link up
> [ 7356.012657] r8169 0000:02:00.0: eth0: link up
> [ 7368.013545] r8169 0000:02:00.0: eth0: link up
>
>
What do you exactly mean by 'localhost pollution' ?
^ permalink raw reply
* Re: [PATCH v7] rps: Receive Packet Steering
From: Eric Dumazet @ 2010-03-17 14:09 UTC (permalink / raw)
To: Changli Gao; +Cc: David Miller, therbert, netdev
In-Reply-To: <412e6f7f1003170059r1f0fa4cfrbe8b3f22102ee9d9@mail.gmail.com>
Le mercredi 17 mars 2010 à 15:59 +0800, Changli Gao a écrit :
> On Wed, Mar 17, 2010 at 3:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le mercredi 17 mars 2010 à 09:54 +0800, Changli Gao a écrit :
> >> On Wed, Mar 17, 2010 at 5:13 AM, David Miller <davem@davemloft.net> wrote:
> >> >
> >> > I'll integrate this as soon as I open up net-next-2.6
> >>
> >> It is really a good news. Now linux also can dispatch packets as
> >> FreeBSD does via netisr. Can we walk farer, and support weighted
> >> distribution?
> >>
> >
> > May I ask why ? What would be the goal ?
> >
>
> For example, I have a firewall with dual core CPU, and use core 0 for
> IRQ and dispatching. If I use both core 0 and core 1 for the left
> processing, core 0 will be overloaded, and if I use core 1 for the
> left processing, core 0 will be light load. In order to take full of
> advantage of hardware, I need weighted the packet distribution.
I would not use RPS at all, weighted or not, unless your firewall must
perform heavy duty work (l7 or complex rules)
If the firewall setup is expensive, then IRQ processing has minor cost,
and RPS fits the bill (cpu handling IRQ will be a bit more loaded than
its buddy).
RPS is a win when _some_ TCP/UDP processing occurs, and we try to do
this processing on a cpu that will also run user space thread with data
in cpu cache (if process scheduler does a good job)
Not much applicable for routers...
Anyway, current sysfs RPS interface exposes
a /sys/class/net/eth0/queues/rx-0/rps_cpus bitmap,
I guess we could expose another file,
/sys/class/net/eth0/queues/rx-0/rps_map
to give different weight to cpus :
echo "0 1 0 1 0 1 1 1 1 1" >/sys/class/net/eth0/queues/rx-0/rps_map
cpu0 would get 30% of the postprocessing load, cpu1 70%
Using /sys/class/net/eth0/queues/rx-0/rps_cpus interface would give an
equal weight to each cpu :
# echo "0 1 0 1 0 1 1 1 1 1" >/sys/class/net/eth0/queues/rx-0/rps_map
# cat /sys/class/net/eth0/queues/rx-0/rps_cpus
3
# cat /sys/class/net/eth0/queues/rx-0/rps_map
0 1 0 1 0 1 1 1 1 1
# echo 3 >/sys/class/net/eth0/queues/rx-0/rps_cpus
# cat /sys/class/net/eth0/queues/rx-0/rps_map
0 1
^ 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