[Click] [PATCH] ToDevice/linuxmodule: Transmit skb with dev_queue_xmit()

[Joonwoo Park]

Hi Eddie,

Thanks for picking up this.  I was totally forgetting about this.
It's my pleasure as usual :)

Joonwoo

On Fri, Jul 8, 2011 at 11:38 AM, Eddie Kohler <kohler at cs.ucla.edu> wrote:
> Hi Joonwoo,
>
> Thanks, as usual, for this fix!  It is applied.
>
> Best,
> Eddie
>
>
> On 2/23/11 11:10 PM, Joonwoo Park wrote:
>>
>> Hi Eddie/All,
>>
>> I think ToDevice() needs to use dev_queue_xmit() to update
>> txq->trans_start and avoid dev_watchdog's wrong netdev resetting.
>> Please review patch below.
>>
>> Thanks,
>> Joonwoo
>>
>>> From 77d8981ae8735c7ae08a87d9c334eb230e946010 Mon Sep 17 00:00:00 2001
>>
>> From: Joonwoo Park<joonwpark81 at gmail.com>
>> Date: Wed, 23 Feb 2011 22:49:46 -0800
>> Subject: [PATCH] ToDevice/linuxmodule: Transmit skb with dev_queue_xmit()
>>
>> If ToDevice sends skbs fast enough so TX desc of netdev is full,
>> device driver can request stop queuing by calling netif_stop_queue().
>> In that case dev_watchdog() will think netdev's TX is hung because
>> txq->trans_start is never updated by ToDevice().
>> To update txq->trans_start, ToDevice needs to use dev_queue_xmit()
>>
>> This fixes periodic watchdog timeout:
>> [  101.895978] ------------[ cut here ]------------
>> [  101.897175] WARNING: at net/sched/sch_generic.c:266
>> dev_watchdog+0x1d6/0x288()
>> [  101.898728] Hardware name: Bochs
>> [  101.899389] NETDEV WATCHDOG: eth1 (e1000e): transmit queue 0 timed out
>> [  101.900768] Modules linked in: click proclikefs e1000e
>> [  101.902532] Pid: 552, comm: kclick Not tainted 2.6.37+ #35
>> [  101.904416] Call Trace:
>> [  101.905264]<IRQ>   [<ffffffff81082946>] warn_slowpath_common+0x80/0x98
>> [  101.907012]  [<ffffffff810829f2>] warn_slowpath_fmt+0x41/0x43
>> [  101.908146]  [<ffffffff8143aeee>] dev_watchdog+0x1d6/0x288
>> [  101.909225]  [<ffffffff810c5a8e>] ? handle_edge_irq+0x187/0x195
>> [  101.910370]  [<ffffffff814218e5>] ? __napi_complete+0x28/0x31
>> [  101.911739]  [<ffffffff8108f726>] run_timer_softirq+0x20e/0x300
>> [  101.913106]  [<ffffffff8143ad18>] ? dev_watchdog+0x0/0x288
>> [  101.914498]  [<ffffffff81088c26>] __do_softirq+0xe1/0x1b7
>> [  101.915462]  [<ffffffff8103fb9c>] call_softirq+0x1c/0x30
>> [  101.916659]  [<ffffffff81041359>] do_softirq+0x61/0xbf
>> [  101.917657]  [<ffffffff810885ca>] irq_exit+0x36/0x38
>> [  101.919697]  [<ffffffff8105b13b>] smp_apic_timer_interrupt+0x88/0x96
>> [  101.920563]  [<ffffffff8103f653>] apic_timer_interrupt+0x13/0x20
>>
>> Signed-off-by: Joonwoo Park<joonwpark81 at gmail.com>
>> ---
>>  elements/linuxmodule/todevice.cc |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/elements/linuxmodule/todevice.cc
>> b/elements/linuxmodule/todevice.cc
>> index dbef920..5985871 100644
>> --- a/elements/linuxmodule/todevice.cc
>> +++ b/elements/linuxmodule/todevice.cc
>> @@ -503,8 +503,7 @@ ToDevice::queue_packet(Packet *p, struct netdev_queue
>> *txq)
>>  #endif
>>
>>  #if LINUX_VERSION_CODE>= KERNEL_VERSION(2, 6, 30)
>> -    // XXX should we call dev_hard_start_xmit???  Probably
>> -    ret = dev->netdev_ops->ndo_start_xmit(skb1, dev);
>> +    ret = dev_queue_xmit(skb1);
>>  #else
>>      ret = dev->hard_start_xmit(skb1, dev);
>>  #endif
>