* Re: [RFC PATCH net 1/2] [BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
From: Américo Wang @ 2011-10-07 13:24 UTC (permalink / raw)
To: Mitsuo Hayasaka
Cc: Jay Vosburgh, Andy Gospodarek, netdev, linux-kernel,
yrl.pp-manager.tt
In-Reply-To: <20111007124954.7089.9776.stgit@ltc219.sdl.hitachi.co.jp>
On Fri, Oct 7, 2011 at 8:49 PM, Mitsuo Hayasaka
<mitsuo.hayasaka.hu@hitachi.com> wrote:
> The bond->recv_probe is called in bond_handle_frame() when
> a packet is received, but bond_close() sets it to NULL. So,
> a panic occurs when both functions work in parallel.
>
> Why this happen:
> After null pointer check of bond->recv_probe, an sk_buff is
> duplicated and bond->recv_probe is called in bond_handle_frame.
> So, a panic occurs when bond_close() is called between the
> check and call of bond->recv_probe.
>
> Patch:
> This patch uses a local function pointer of bond->recv_probe
> in bond_handle_frame(). So, it can avoid the null pointer
> dereference.
>
Hmm, I don't doubt it can fix the problem, I am wondering if
bond->recv_probe should be protected by bond->lock...
^ permalink raw reply
* [RFC PATCH net 2/2] [BUGFIX] bonding: use flush_delayed_work_sync in bond_close
From: Mitsuo Hayasaka @ 2011-10-07 12:50 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek
Cc: netdev, linux-kernel, yrl.pp-manager.tt, Mitsuo Hayasaka,
Jay Vosburgh, Andy Gospodarek
In-Reply-To: <20111007124925.7089.27260.stgit@ltc219.sdl.hitachi.co.jp>
The bond_close() calls cancel_delayed_work() to cancel delayed works.
It, however, cannot cancel works that were already queued in workqueue.
The bond_open() initializes work->data, and proccess_one_work() refers
get_work_cwq(work)->wq->flags. The get_work_cwq() returns NULL when
work->data has been initialized. Thus, a panic occurs.
This patch uses flush_delayed_work_sync() instead of cancel_delayed_work()
in bond_close(). It cancels delayed timer and waits for work to finish
execution. So, it can avoid the null pointer dereference due to the
parallel executions of proccess_one_work() and initializing proccess
of bond_open().
Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_main.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f1baec5..cd490b7 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3504,27 +3504,27 @@ static int bond_close(struct net_device *bond_dev)
write_unlock_bh(&bond->lock);
if (bond->params.miimon) { /* link check interval, in milliseconds. */
- cancel_delayed_work(&bond->mii_work);
+ flush_delayed_work_sync(&bond->mii_work);
}
if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
- cancel_delayed_work(&bond->arp_work);
+ flush_delayed_work_sync(&bond->arp_work);
}
switch (bond->params.mode) {
case BOND_MODE_8023AD:
- cancel_delayed_work(&bond->ad_work);
+ flush_delayed_work_sync(&bond->ad_work);
break;
case BOND_MODE_TLB:
case BOND_MODE_ALB:
- cancel_delayed_work(&bond->alb_work);
+ flush_delayed_work_sync(&bond->alb_work);
break;
default:
break;
}
if (delayed_work_pending(&bond->mcast_work))
- cancel_delayed_work(&bond->mcast_work);
+ flush_delayed_work_sync(&bond->mcast_work);
if (bond_is_lb(bond)) {
/* Must be called only after all
^ permalink raw reply related
* [RFC PATCH net 1/2] [BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
From: Mitsuo Hayasaka @ 2011-10-07 12:49 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek
Cc: netdev, linux-kernel, yrl.pp-manager.tt, Mitsuo Hayasaka,
Jay Vosburgh, Andy Gospodarek
In-Reply-To: <20111007124925.7089.27260.stgit@ltc219.sdl.hitachi.co.jp>
The bond->recv_probe is called in bond_handle_frame() when
a packet is received, but bond_close() sets it to NULL. So,
a panic occurs when both functions work in parallel.
Why this happen:
After null pointer check of bond->recv_probe, an sk_buff is
duplicated and bond->recv_probe is called in bond_handle_frame.
So, a panic occurs when bond_close() is called between the
check and call of bond->recv_probe.
Patch:
This patch uses a local function pointer of bond->recv_probe
in bond_handle_frame(). So, it can avoid the null pointer
dereference.
Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_main.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6d79b78..f1baec5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1435,6 +1435,8 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
struct sk_buff *skb = *pskb;
struct slave *slave;
struct bonding *bond;
+ void (*recv_probe)(struct sk_buff *, struct bonding *,
+ struct slave *);
skb = skb_share_check(skb, GFP_ATOMIC);
if (unlikely(!skb))
@@ -1448,11 +1450,12 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
if (bond->params.arp_interval)
slave->dev->last_rx = jiffies;
- if (bond->recv_probe) {
+ recv_probe = bond->recv_probe;
+ if (recv_probe) {
struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
if (likely(nskb)) {
- bond->recv_probe(nskb, bond, slave);
+ recv_probe(nskb, bond, slave);
dev_kfree_skb(nskb);
}
}
^ permalink raw reply related
* [RFC PATCH net 0/2] bonding: fix panics for making bonding device up and down repeatedly
From: Mitsuo Hayasaka @ 2011-10-07 12:49 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek; +Cc: netdev, linux-kernel, yrl.pp-manager.tt
Hi,
I hit a kernel BUG when I made a bonding interface up and down
repeatedly, as below.
# while true; do ifconfig bond0 down; done &
# while true; do ifconfig bond0 up; done &
(panic messages are bottom of this mail)
I investigated it and found the below commit tries to address
this kind of parallel up&down problem.
a0db2dad0935e798973bb79676e722b82f177206 "bonding: properly stop
queuing work when requested"
However, above fix is not enough. I've found two different BUG
patterns.
They happens as follows:
(1) NULL pointer dereference in bond_handle_frame()
1. bond_close() sets bond->recv_probe to NULL.
2. bond_handle_frame() calls bond->recv_probe.
3. a panic happens when running bond_close() in parallel
with bond_handle_frame().
(2) NULL pointer dereference in proccess_one_work()
1. bond_close() calls cancel_delayed_work() but its last works
are not cancelled because they were already queued in workqueue.
2. bond_open() initializes work->data.
3. process_one_work() refers get_work_cwq(work)->wq->flags in
workqueue.c:1850, but get_work_cwq() returns NULL when work->data
has been initialized.
4. a panic happens when running process_one_work() in parallel with
bond_open().
Each log is shown below.
The following two patches addresses these panics in detail:
[PATCH 1/2] use local function pointer of bond->recv_probe
in bond_handle_frame
[PATCH 2/2] use flush_delayed_work_sync in bond_close
Thanks,
============== log for a panic in bond_handle_frame=========
# while true; do ifconfig bond0 down; done &
[1] 1441
# while true; do ifconfig bond0 up; done &
[2] 3383
# [ 296.794817] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 296.795761] IP: [< (null)>] (null)
[ 296.795761] PGD 79515067 PUD 375e9067 PMD 0
[ 296.795761] Oops: 0010 [#1] SMP
[ 296.795761] CPU 0
[ 296.795761] Modules linked in: nfs lockd fscache auth_rpcgss nfs_acl sunrpc bonding snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm pcspkr virtio_balloon microcode i2c_piix4 joydev snd_timer snd virtio_net i2c_core soundcore snd_page_alloc ipv6 xfs exportfs virtio_blk [last unloaded: speedstep_lib]
[ 296.795761]
[ 296.795761] Pid: 0, comm: swapper Not tainted 3.1.0-rc9+ #4 Bochs Bochs
[ 296.795761] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
[ 296.795761] RSP: 0018:ffff88007fc03d28 EFLAGS: 00010286
[ 296.795761] RAX: ffff88007b1e6500 RBX: ffff88007b1e6a00 RCX: 0000000000000000
[ 296.795761] RDX: ffff880037670e00 RSI: ffff88007b522740 RDI: ffff88007b1e6500
[ 296.795761] RBP: ffff88007fc03d50 R08: 0000000000000000 R09: 0000000180100001
[ 296.795761] R10: ffffffff81a01fd8 R11: ffffffff81a01fd8 R12: ffff88007b522740
[ 296.795761] R13: ffff880037670e00 R14: ffff88007b1e6500 R15: ffffe8ffffc02578
[ 296.795761] FS: 0000000000000000(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[ 296.795761] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 296.795761] CR2: 0000000000000000 CR3: 00000000794f9000 CR4: 00000000000006f0
[ 296.795761] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 296.795761] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 296.795761] Process swapper (pid: 0, threadinfo ffffffff81a00000, task ffffffff81a0d020)
[ 296.795761] Stack:
[ 296.795761] ffffffffa00b64d0 ffffffffa00b642e 0000000000000001 ffff880037a3f000
[ 296.795761] 0000000000000000 ffff88007fc03db0 ffffffff813e2996 000000000000002e
[ 296.795761] ffff88007b1e6a00 ffff88007fc03d90 ffffffff81b5be40 ffff88007b1e6a00
[ 296.795761] Call Trace:
[ 296.795761] <IRQ>
[ 296.795761] [<ffffffffa00b64d0>] ? bond_handle_frame+0xa2/0x160 [bonding]
[ 296.795761] [<ffffffffa00b642e>] ? skb_cow_head+0x70/0x70 [bonding]
[ 296.795761] [<ffffffff813e2996>] __netif_receive_skb+0x2c5/0x417
[ 296.795761] [<ffffffff813e5c22>] netif_receive_skb+0x6c/0x73
[ 296.795761] [<ffffffffa0155360>] virtnet_poll+0x49a/0x562 [virtio_net]
[ 296.795761] [<ffffffff813e623b>] net_rx_action+0xa9/0x1b8
[ 296.795761] [<ffffffff8105c61b>] __do_softirq+0xc9/0x1b5
[ 296.795761] [<ffffffff81014fa0>] ? sched_clock+0x9/0xd
[ 296.795761] [<ffffffff814b8c2c>] call_softirq+0x1c/0x30
[ 296.795761] [<ffffffff81010af9>] do_softirq+0x46/0x81
[ 296.795761] [<ffffffff8105c8e3>] irq_exit+0x57/0xb1
[ 296.795761] [<ffffffff814b950e>] do_IRQ+0x8e/0xa5
[ 296.795761] [<ffffffff814b062e>] common_interrupt+0x6e/0x6e
[ 296.795761] <EOI>
[ 296.795761] [<ffffffff810b27c3>] ? rcu_needs_cpu+0x11a/0x1cb
[ 296.795761] [<ffffffff8102f2f1>] ? native_safe_halt+0xb/0xd
[ 296.795761] [<ffffffff81015b32>] default_idle+0x4e/0x86
[ 296.795761] [<ffffffff8100e2ed>] cpu_idle+0xae/0xe8
[ 296.795761] [<ffffffff8148cd6e>] rest_init+0x72/0x74
[ 296.795761] [<ffffffff81b74b7d>] start_kernel+0x3ab/0x3b6
[ 296.795761] [<ffffffff81b742c4>] x86_64_start_reservations+0xaf/0xb3
[ 296.795761] [<ffffffff81b74140>] ? early_idt_handlers+0x140/0x140
[ 296.795761] [<ffffffff81b743ca>] x86_64_start_kernel+0x102/0x111
[ 296.795761] Code: Bad RIP value.
[ 296.795761] RIP [< (null)>] (null)
[ 296.795761] RSP <ffff88007fc03d28>
[ 296.795761] CR2: 0000000000000000
[ 296.846940] ---[ end trace 2d2403022cca4fe1 ]---
[ 296.847691] Kernel panic - not syncing: Fatal exception in interrupt
[ 296.848701] Pid: 0, comm: swapper Tainted: G D 3.1.0-rc9+ #4
[ 296.849717] Call Trace:
[ 296.850122] <IRQ> [<ffffffff814a6f2b>] panic+0x91/0x1a5
[ 296.851080] [<ffffffff814b13e6>] oops_end+0xb4/0xc5
[ 296.851856] [<ffffffff814a67d5>] no_context+0x203/0x212
[ 296.852710] [<ffffffff813e7601>] ? dev_queue_xmit+0x44b/0x472
[ 296.853641] [<ffffffff814a69af>] __bad_area_nosemaphore+0x1cb/0x1ec
[ 296.854654] [<ffffffffa00bc52c>] ? bond_3ad_xmit_xor+0x130/0x156 [bonding]
[ 296.855747] [<ffffffff814a69e3>] bad_area_nosemaphore+0x13/0x15
[ 296.856705] [<ffffffff814b33a3>] do_page_fault+0x1b8/0x37e
[ 296.857602] [<ffffffff811131d5>] ? virt_to_head_page+0xe/0x31
[ 296.858533] [<ffffffff81114f7a>] ? __cmpxchg_double_slab+0x2c/0x9e
[ 296.859538] [<ffffffff814a9d74>] ? __slab_alloc+0x39f/0x3b4
[ 296.860453] [<ffffffff813dcde0>] ? skb_clone+0x77/0x97
[ 296.861277] [<ffffffff8102fdbd>] ? pvclock_clocksource_read+0x48/0xb4
[ 296.862325] [<ffffffff814b08f5>] page_fault+0x25/0x30
[ 296.863148] [<ffffffffa00b64d0>] ? bond_handle_frame+0xa2/0x160 [bonding]
[ 296.864235] [<ffffffffa00b642e>] ? skb_cow_head+0x70/0x70 [bonding]
[ 296.865242] [<ffffffff813e2996>] __netif_receive_skb+0x2c5/0x417
[ 296.866218] [<ffffffff813e5c22>] netif_receive_skb+0x6c/0x73
[ 296.867134] [<ffffffffa0155360>] virtnet_poll+0x49a/0x562 [virtio_net]
[ 296.868166] [<ffffffff813e623b>] net_rx_action+0xa9/0x1b8
[ 296.869037] [<ffffffff8105c61b>] __do_softirq+0xc9/0x1b5
[ 296.869888] [<ffffffff81014fa0>] ? sched_clock+0x9/0xd
[ 296.870726] [<ffffffff814b8c2c>] call_softirq+0x1c/0x30
[ 296.871584] [<ffffffff81010af9>] do_softirq+0x46/0x81
[ 296.872402] [<ffffffff8105c8e3>] irq_exit+0x57/0xb1
[ 296.873192] [<ffffffff814b950e>] do_IRQ+0x8e/0xa5
[ 296.873940] [<ffffffff814b062e>] common_interrupt+0x6e/0x6e
[ 296.874846] <EOI> [<ffffffff810b27c3>] ? rcu_needs_cpu+0x11a/0x1cb
[ 296.875881] [<ffffffff8102f2f1>] ? native_safe_halt+0xb/0xd
[ 296.876771] [<ffffffff81015b32>] default_idle+0x4e/0x86
[ 296.877615] [<ffffffff8100e2ed>] cpu_idle+0xae/0xe8
[ 296.878412] [<ffffffff8148cd6e>] rest_init+0x72/0x74
[ 296.879215] [<ffffffff81b74b7d>] start_kernel+0x3ab/0x3b6
[ 296.880096] [<ffffffff81b742c4>] x86_64_start_reservations+0xaf/0xb3
[ 296.881189] [<ffffffff81b74140>] ? early_idt_handlers+0x140/0x140
[ 296.882169] [<ffffffff81b743ca>] x86_64_start_kernel+0x102/0x111
============== log for a panic in proccess_one_work=========
# while true; do ifconfig bond0 down; done &
[1] 1463
# while true; do ifconfig bond0 up; done &
[2] 7093
# [ 734.859163] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[ 734.862624] IP: [<ffffffff8106da2e>] process_one_work+0x104/0x2a9
[ 734.862624] PGD 0
[ 734.862624] Oops: 0000 [#1] SMP
[ 734.862624] CPU 3
[ 734.862624] Modules linked in: fuse nfs lockd fscache auth_rpcgss nfs_acl sunrpc bonding snd_hda_intel snd_hda_codec snd_hwdep joydev snd_seq snd_seq_device snd_pcm pcspkr microcode virtio_net virtio_balloon snd_timer snd soundcore snd_page_alloc i2c_piix4 i2c_core ipv6 xfs exportfs virtio_blk [last unloaded: speedstep_lib]
[ 734.862624]
[ 734.862624] Pid: 47, comm: kworker/u:1 Not tainted 3.1.0-rc9+ #4 Bochs Bochs
[ 734.862624] RIP: 0010:[<ffffffff8106da2e>] [<ffffffff8106da2e>] process_one_work+0x104/0x2a9
[ 734.862624] RSP: 0018:ffff880078f95e50 EFLAGS: 00010046
[ 734.862624] RAX: 0000000000000000 RBX: ffff880078f5af00 RCX: 0000000000010000
[ 734.862624] RDX: 0000000000010000 RSI: ffff88007bcada50 RDI: ffff88007bcad8f8
[ 734.862624] RBP: ffff880078f95ea0 R08: ffff88007bcad900 R09: ffff880076c1bf00
[ 734.862624] R10: ffff880076c1bf00 R11: ffff88007fd92e40 R12: ffffffff81cc3dc0
[ 734.862624] R13: ffff88007a81f600 R14: ffffffffa00b8759 R15: ffff88007a81f607
[ 734.862624] FS: 0000000000000000(0000) GS:ffff88007fd80000(0000) knlGS:0000000000000000
[ 734.862624] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 734.862624] CR2: 0000000000000008 CR3: 0000000001a05000 CR4: 00000000000006e0
[ 734.862624] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 734.862624] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 734.862624] Process kworker/u:1 (pid: 47, threadinfo ffff880078f94000, task ffff880078f3ae40)
[ 734.862624] Stack:
[ 734.862624] ffffffff81cc3dc8 ffff88007bcad900 0000009a7bc54a70 ffff88007bcad8f8
[ 734.862624] ffffffff81cc3dc0 ffff880078f5af00 ffffffff81cc3dc0 ffff880078f5af20
[ 734.862624] ffff880078f3ae40 ffff880078f5af20 ffff880078f95ee0 ffffffff8106e5aa
[ 734.862624] Call Trace:
[ 734.862624] [<ffffffff8106e5aa>] worker_thread+0xda/0x15d
[ 734.862624] [<ffffffff8106e4d0>] ? manage_workers+0x176/0x176
[ 734.862624] [<ffffffff810719f7>] kthread+0x84/0x8c
[ 734.862624] [<ffffffff814b8b34>] kernel_thread_helper+0x4/0x10
[ 734.862624] [<ffffffff81071973>] ? kthread_worker_fn+0x148/0x148
[ 734.862624] [<ffffffff814b8b30>] ? gs_change+0x13/0x13
[ 734.862624] Code: 8b 55 c8 48 89 42 08 48 89 42 10 41 f6 44 24 1c 10 74 31 49 8b 7c 24 08 49 8d 44 24 08 48 39 c7 74 1c 48 83 ef 08 e8 6a e0 ff ff
[ 734.862624] 8b 40 08 f6 00 10 74 0a 4c 89 e7 e8 85 e4 ff ff eb 06 41 83
[ 734.862624] RIP [<ffffffff8106da2e>] process_one_work+0x104/0x2a9
[ 734.862624] RSP <ffff880078f95e50>
[ 734.862624] CR2: 0000000000000008
[ 734.862624] ---[ end trace 7cf0d13647b8711b ]---
[ 734.997227] BUG: unable to handle kernel paging request at fffffffffffffff8
Mess[age fr om734.998114] IP:
---
Mitsuo Hayasaka (2):
[BUGFIX] bonding: use flush_delayed_work_sync in bond_close
[BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
drivers/net/bonding/bond_main.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
--
Mitsuo Hayasaka (mitsuo.hayasaka.hu@hitachi.com)
^ permalink raw reply
* Re: loopback IP alias breaks tftp?
From: Eric Dumazet @ 2011-10-07 12:23 UTC (permalink / raw)
To: Josh Boyer; +Cc: Joel Sing, Julian Anastasov, netdev
In-Reply-To: <1317989073.3207.10.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Le vendredi 07 octobre 2011 à 14:04 +0200, Eric Dumazet a écrit :
> Its a completely different problem IMHO : You describe a tftp server
> bug.
>
> Say your tftp server is multihomed with 3 different IPS :
>
> 192.168.20.21, 192.168.20.22, 192.168.20.23
>
> And tftp server listens to any address (UDP port 69) : 0.0.0.0:69
>
> When receiving a request on 192.168.20.22, it should use same source
> address, not let the system chose a "random or whatever policy" one.
>
>
>
> So I would suggest to check/fix if TFTP server uses the correct socket
> API to get both the client IP and its own IP in each UDP datagram
>
> -> setsockopt(fd, IPPROTO_IP, &on, sizeof(on))
> This permits tftp server to use the same "struct in_pktinfo" for replies, forcing a correct source address.
>
By the way, there is no need for tftp change :
Just launch several tftpd instances, and bind each them to one
particular IP.
If started from xinetd.d :
$ cat /etc/xinetd.d/tftp1
service tftp
{
socket_type = dgram
protocol = udp
wait = yes
user = root
server = /usr/sbin/in.tftpd
server_args = -s /tftpboot
disable = no
bind = 192.168.20.21
per_source = 11
cps = 100 2
flags = IPv4
}
$ cat /etc/xinetd.d/tftp2
service tftp
{
socket_type = dgram
protocol = udp
wait = yes
user = root
server = /usr/sbin/in.tftpd
server_args = -s /tftpboot
disable = no
bind = 192.168.20.22
per_source = 11
cps = 100 2
flags = IPv4
}
$ cat /etc/xinetd.d/tftp3
service tftp
{
socket_type = dgram
protocol = udp
wait = yes
user = root
server = /usr/sbin/in.tftpd
server_args = -s /tftpboot
disable = no
bind = 192.168.20.23
per_source = 11
cps = 100 2
flags = IPv4
}
^ permalink raw reply
* Re: [PATCH 1/2 net-next] r6040: invoke phy_{start,stop} when appropriate
From: Florian Fainelli @ 2011-10-07 12:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1317983087.3207.1.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Friday 07 October 2011 12:24:47 Eric Dumazet wrote:
> Le vendredi 07 octobre 2011 à 11:36 +0200, Florian Fainelli a écrit :
> > Joe reported to me that right after a bring up of a r6040 interface
> > the ethtool output had no consistent output with respect to link duplex
> > and speed. Fix this by adding a missing phy_start call in r6040_up and
> > conversely a phy_stop call in r6040_down to properly initialize phy
> > states.
> >
> > Reported-by: Joe Chou <Joe.Chou@rdc.com.tw>
> > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > ---
> > diff --git a/drivers/net/ethernet/rdc/r6040.c
> > b/drivers/net/ethernet/rdc/r6040.c index 2bbadc0..a128c3d 100644
> > --- a/drivers/net/ethernet/rdc/r6040.c
> > +++ b/drivers/net/ethernet/rdc/r6040.c
> > @@ -470,6 +470,8 @@ static void r6040_down(struct net_device *dev)
> >
> > iowrite16(adrp[0], ioaddr + MID_0L);
> > iowrite16(adrp[1], ioaddr + MID_0M);
> > iowrite16(adrp[2], ioaddr + MID_0H);
> >
> > +
> > + phy_stop(lp->phydev);
> >
> > }
> >
> > static int r6040_close(struct net_device *dev)
> >
> > @@ -727,6 +729,8 @@ static int r6040_up(struct net_device *dev)
> >
> > /* Initialize all MAC registers */
> > r6040_init_mac_regs(dev);
> >
> > + phy_start(lp->phydev);
> > +
> >
> > return 0;
> >
> > }
>
> You dont need to submit your patches twice (net and net-next)
>
> If its a bug fix, base your patch against net tree
>
> If its a new feature, against net-next
Allright, thanks for the tip. I just thought that would make it easier for
David to get this applied (especially after the drivers reorganization).
--
Florian
^ permalink raw reply
* [PATCH 1/1] ethtool: add ETHTOOL_{G,S}CHANNEL support.
From: Sucheta Chakraborty @ 2011-10-07 11:58 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1317988730-4197-1-git-send-email-sucheta.chakraborty@qlogic.com>
Used to configure number of tx/ rx/ other channels.
Reqd. man page changes are included.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
ethtool.8.in | 29 ++++++++++++++
ethtool.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 149 insertions(+), 0 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index efc6098..f9d3633 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -307,6 +307,16 @@ ethtool \- query or control network driver and hardware settings
.BN action
.BN loc
.RB ]
+.HP
+.B ethtool \-l|\-\-show\-channels
+.I ethX
+.HP
+.B ethtool \-L|\-\-set\-channels
+.I ethX
+.BN rx
+.BN tx
+.BN other
+.BN combined
.
.\" Adjust lines (i.e. full justification) and hyphenate.
.ad
@@ -754,6 +764,25 @@ lB l.
Specify the location/ID to insert the rule. This will overwrite
any rule present in that location and will not go through any
of the rule ordering process.
+.TP
+.B \-l \-\-show\-channels
+Queries the specified network device for the numbers of channels it has.
+A channel is an IRQ and the set of queues that can trigger that IRQ.
+.TP
+.B \-L \-\-set\-channels
+Changes the numbers of channels of the specified network device.
+.TP
+.BI rx \ N
+Changes the number of channels with only receive queues.
+.TP
+.BI tx \ N
+Changes the number of channels with only transmit queues.
+.TP
+.BI other \ N
+Changes the number of channels used only for other purposes e.g. link interrupts or SR-IOV co-ordination.
+.TP
+.BI combined \ N
+Changes the number of multi-purpose channels.
.SH BUGS
Not supported (in part or whole) on all network drivers.
.SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index d7d2d58..c533a48 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -80,6 +80,8 @@ static int do_gpause(int fd, struct ifreq *ifr);
static int do_spause(int fd, struct ifreq *ifr);
static int do_gring(int fd, struct ifreq *ifr);
static int do_sring(int fd, struct ifreq *ifr);
+static int do_schannels(int fd, struct ifreq *ifr);
+static int do_gchannels(int fd, struct ifreq *ifr);
static int do_gcoalesce(int fd, struct ifreq *ifr);
static int do_scoalesce(int fd, struct ifreq *ifr);
static int do_goffload(int fd, struct ifreq *ifr);
@@ -133,6 +135,8 @@ static enum {
MODE_PERMADDR,
MODE_SET_DUMP,
MODE_GET_DUMP,
+ MODE_GCHANNELS,
+ MODE_SCHANNELS
} mode = MODE_GSET;
static struct option {
@@ -266,6 +270,12 @@ static struct option {
{ "-W", "--set-dump", MODE_SET_DUMP,
"Set dump flag of the device",
" N\n"},
+ { "-l", "--show-channels", MODE_GCHANNELS, "Query Channels" },
+ { "-L", "--set-channels", MODE_SCHANNELS, "Set Channels",
+ " [ rx N ]\n"
+ " [ tx N ]\n"
+ " [ other N ]\n"
+ " [ combined N ]\n" },
{ "-h", "--help", MODE_HELP, "Show this help" },
{ NULL, "--version", MODE_VERSION, "Show version number" },
{}
@@ -331,6 +341,13 @@ static s32 ring_rx_mini_wanted = -1;
static s32 ring_rx_jumbo_wanted = -1;
static s32 ring_tx_wanted = -1;
+static struct ethtool_channels echannels;
+static int gchannels_changed;
+static s32 channels_rx_wanted = -1;
+static s32 channels_tx_wanted = -1;
+static s32 channels_other_wanted = -1;
+static s32 channels_combined_wanted = -1;
+
static struct ethtool_coalesce ecoal;
static int gcoalesce_changed = 0;
static s32 coal_stats_wanted = -1;
@@ -495,6 +512,13 @@ static struct cmdline_info cmdline_ring[] = {
{ "tx", CMDL_S32, &ring_tx_wanted, &ering.tx_pending },
};
+static struct cmdline_info cmdline_channels[] = {
+ { "rx", CMDL_S32, &channels_rx_wanted, &echannels.rx_count },
+ { "tx", CMDL_S32, &channels_tx_wanted, &echannels.tx_count },
+ { "other", CMDL_S32, &channels_other_wanted, &echannels.other_count },
+ { "combined", CMDL_S32, &channels_combined_wanted, &echannels.combined_count },
+};
+
static struct cmdline_info cmdline_coalesce[] = {
{ "adaptive-rx", CMDL_BOOL, &coal_adaptive_rx_wanted, &ecoal.use_adaptive_rx_coalesce },
{ "adaptive-tx", CMDL_BOOL, &coal_adaptive_tx_wanted, &ecoal.use_adaptive_tx_coalesce },
@@ -787,6 +811,8 @@ static void parse_cmdline(int argc, char **argp)
(mode == MODE_GCOALESCE) ||
(mode == MODE_SCOALESCE) ||
(mode == MODE_GRING) ||
+ (mode == MODE_GCHANNELS) ||
+ (mode == MODE_SCHANNELS) ||
(mode == MODE_SRING) ||
(mode == MODE_GOFFLOAD) ||
(mode == MODE_SOFFLOAD) ||
@@ -871,6 +897,14 @@ static void parse_cmdline(int argc, char **argp)
i = argc;
break;
}
+ if (mode == MODE_SCHANNELS) {
+ parse_generic_cmdline(argc, argp, i,
+ &gchannels_changed,
+ cmdline_channels,
+ ARRAY_SIZE(cmdline_ring));
+ i = argc;
+ break;
+ }
if (mode == MODE_SCOALESCE) {
parse_generic_cmdline(argc, argp, i,
&gcoalesce_changed,
@@ -1751,6 +1785,32 @@ static int dump_ring(void)
return 0;
}
+static int dump_channels(void)
+{
+ fprintf(stdout,
+ "Pre-set maximums:\n"
+ "RX: %u\n"
+ "TX: %u\n"
+ "Other: %u\n"
+ "Combined: %u\n",
+ echannels.max_rx, echannels.max_tx,
+ echannels.max_other,
+ echannels.max_combined);
+
+ fprintf(stdout,
+ "Current hardware settings:\n"
+ "RX: %u\n"
+ "TX: %u\n"
+ "Other: %u\n"
+ "Combined: %u\n",
+ echannels.rx_count, echannels.tx_count,
+ echannels.other_count,
+ echannels.combined_count);
+
+ fprintf(stdout, "\n");
+ return 0;
+}
+
static int dump_coalesce(void)
{
fprintf(stdout, "Adaptive RX: %s TX: %s\n",
@@ -1937,6 +1997,10 @@ static int doit(void)
return do_gring(fd, &ifr);
} else if (mode == MODE_SRING) {
return do_sring(fd, &ifr);
+ } else if (mode == MODE_GCHANNELS) {
+ return do_gchannels(fd, &ifr);
+ } else if (mode == MODE_SCHANNELS) {
+ return do_schannels(fd, &ifr);
} else if (mode == MODE_GOFFLOAD) {
return do_goffload(fd, &ifr);
} else if (mode == MODE_SOFFLOAD) {
@@ -2114,6 +2178,62 @@ static int do_gring(int fd, struct ifreq *ifr)
return 0;
}
+static int do_schannels(int fd, struct ifreq *ifr)
+{
+ int err, changed = 0;
+
+ echannels.cmd = ETHTOOL_GCHANNELS;
+ ifr->ifr_data = (caddr_t)&echannels;
+ err = send_ioctl(fd, ifr);
+ if (err) {
+ perror("Cannot get device channel parameters");
+ return 1;
+ }
+
+ do_generic_set(cmdline_channels, ARRAY_SIZE(cmdline_channels),
+ &changed);
+
+ if (!changed) {
+ fprintf(stderr, "no channel parameters changed, aborting\n");
+ fprintf(stderr, "current values: tx %u rx %u other %u"
+ "combined %u\n", echannels.rx_count,
+ echannels.tx_count, echannels.other_count,
+ echannels.combined_count);
+ return 1;
+ }
+
+ echannels.cmd = ETHTOOL_SCHANNELS;
+ ifr->ifr_data = (caddr_t)&echannels;
+ err = send_ioctl(fd, ifr);
+ if (err) {
+ perror("Cannot set device channel parameters");
+ return 1;
+ }
+
+ return 0;
+}
+
+static int do_gchannels(int fd, struct ifreq *ifr)
+{
+ int err;
+
+ fprintf(stdout, "Channel parameters for %s:\n", devname);
+
+ echannels.cmd = ETHTOOL_GCHANNELS;
+ ifr->ifr_data = (caddr_t)&echannels;
+ err = send_ioctl(fd, ifr);
+ if (err == 0) {
+ err = dump_channels();
+ if (err)
+ return err;
+ } else {
+ perror("Cannot get device channel parameters\n");
+ return 1;
+ }
+ return 0;
+
+}
+
static int do_gcoalesce(int fd, struct ifreq *ifr)
{
int err;
--
1.7.3.3
^ permalink raw reply related
* [PATCH 0/1] ethtool: {G, S}-CHANNEL support added
From: Sucheta Chakraborty @ 2011-10-07 11:58 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Dept_NX_Linux_NIC_Driver
Thanks Ben for reviewing and applying the patches.
I have incorporated all the changes suggested by you for this patch.
^ permalink raw reply
* Re: loopback IP alias breaks tftp?
From: Eric Dumazet @ 2011-10-07 12:04 UTC (permalink / raw)
To: Josh Boyer; +Cc: Joel Sing, Julian Anastasov, netdev
In-Reply-To: <20111007114017.GA1165@zod.bos.redhat.com>
Le vendredi 07 octobre 2011 à 07:40 -0400, Josh Boyer a écrit :
> The original bug reporter explained the loopback testcase is simply a
> trivial example to illustrate the problem. The real setup seems to be:
>
> "I have a hardware (Dell PC) booting over the network with PXE, this
> hardware try to load it system from a tftp server. If I used the real
> ip of the tftp server I have no problem, if I used an alias I have the
> problem (I have to use an alias, because I have a Cluster).
>
> What I think, the bios tftp client talk to the server with the IP alias
> but the server reply with the real IP and the client reject the reply."
>
> So in this case, because the alias is being used in a cluster, should he
> also setup the local routing table as you suggested?
>
> I apologize for my lack of depth here. I'm at the moment somewhat of a
> go-between. I do greatly appreciate the help!
Its a completely different problem IMHO : You describe a tftp server
bug.
Say your tftp server is multihomed with 3 different IPS :
192.168.20.21, 192.168.20.22, 192.168.20.23
And tftp server listens to any address (UDP port 69) : 0.0.0.0:69
When receiving a request on 192.168.20.22, it should use same source
address, not let the system chose a "random or whatever policy" one.
So I would suggest to check/fix if TFTP server uses the correct socket
API to get both the client IP and its own IP in each UDP datagram
-> setsockopt(fd, IPPROTO_IP, &on, sizeof(on))
IP_PKTINFO
Pass an IP_PKTINFO ancillary message that contains a pktinfo structure that supplies some
information about the incoming packet. This only works for datagram oriented sockets. The
argument is a flag that tells the socket whether the IP_PKTINFO message should be passed or
not. The message itself can only be sent/retrieved as control message with a packet using
recvmsg(2) or sendmsg(2).
struct in_pktinfo {
unsigned int ipi_ifindex; /* Interface index */
struct in_addr ipi_spec_dst; /* Local address */
struct in_addr ipi_addr; /* Header Destination
address */
};
ipi_ifindex is the unique index of the interface the packet was received on. ipi_spec_dst
is the local address of the packet and ipi_addr is the destination address in the packet
header. If IP_PKTINFO is passed to sendmsg(2) and ipi_spec_dst is not zero, then it is used
as the local source address for the routing table lookup and for setting up IP source route
options. When ipi_ifindex is not zero, the primary local address of the interface specified
by the index overwrites ipi_spec_dst for the routing table lookup.
This permits tftp server to use the same "struct in_pktinfo" for replies, forcing a correct source address.
^ permalink raw reply
* Re: loopback IP alias breaks tftp?
From: Josh Boyer @ 2011-10-07 11:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Joel Sing, Julian Anastasov, netdev
In-Reply-To: <1317972573.3457.55.camel@edumazet-laptop>
On Fri, Oct 07, 2011 at 09:29:33AM +0200, Eric Dumazet wrote:
> Le vendredi 07 octobre 2011 à 18:02 +1100, Joel Sing a écrit :
> > > > > We've had a report [1] of a change in behavior when trying to use an IP
> > > > > alias to tftp from a loopback device. Apparently the steps outlined in
> > > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=739534
> > >
> > > Yep. That is exactly what my git bisect said too.
> > >
> > > So now we have a change in behavior since that commit for the usecase
> > > described in the bug above, but I'm unsure if that usecase was ever
> > > really valid or if the commit had unintentional side effects.
> > >
> > > Joel (or anyone else) can you take a look and comment?
> >
> > Prior to this commit the src address in the local routing table was
> > completely ignored, so connecting to a local address always used the
> > same source and destination addresses. However, this is not what was
> > configured in the local routing table:
> >
> > $ ifconfig lo:0 127.0.0.2
> > $ ip route show table local | grep 127.0.0.2
> > local 127.0.0.2 dev lo proto kernel scope host src 127.0.0.1
> >
> > When an interface has an alias configured, the source address
> > installed in the local routing table is always the primary address for
> > the interface. The tftp use case you've described now breaks due to
> > the way that in.tftpd is determining the reply address (as documented
> > in the in.tftpd manual page). This means that it is now responding
> > from 127.0.0.1 even though the client connected to 127.0.0.2. Binding
> > inetd to a specific address will avoid this issue.
> >
> > I have not yet looked to see if there is a specific reason for the
> > source address selection, however one way of restoring the previous
> > behaviour (whilst still respecting the configured source address)
> > would be to use a default source address which matches the configured
> > address (as is done for primary addresses). I cannot immediately think
> > of a reason not to do this, but I've not gone looking at the code.
> >
> > Worst case scenario if you specifically need the original behaviour
> > then the source address can be changed in the local routing table:
> >
> > $ ip route change table local local 127.0.0.2 dev lo:0 proto kernel
> > scope host src 127.0.0.2
> > $ ip route show table local | grep 127.0.0.2
> > local 127.0.0.2 dev lo proto kernel scope host src 127.0.0.2
>
> Agreed.
>
> The "table local" bit is really the key, because following naive setup
> doesnt work.
>
> # ip addr add 127.0.0.11/8 dev lo
> # ip route add 127.0.0.11 dev lo src 127.0.0.11
>
> since 127.0.0.1 will still be the src address selected for connections.
>
> # ip ro show table local | grep 127.0.0.11
> local 127.0.0.11 dev lo proto kernel scope host src 127.0.0.1
The original bug reporter explained the loopback testcase is simply a
trivial example to illustrate the problem. The real setup seems to be:
"I have a hardware (Dell PC) booting over the network with PXE, this
hardware try to load it system from a tftp server. If I used the real
ip of the tftp server I have no problem, if I used an alias I have the
problem (I have to use an alias, because I have a Cluster).
What I think, the bios tftp client talk to the server with the IP alias
but the server reply with the real IP and the client reject the reply."
So in this case, because the alias is being used in a cluster, should he
also setup the local routing table as you suggested?
I apologize for my lack of depth here. I'm at the moment somewhat of a
go-between. I do greatly appreciate the help!
josh
^ permalink raw reply
* Re: [PATCH] IPv6: DAD from bonding iface is treated as dup address from others
From: Neil Horman @ 2011-10-07 11:10 UTC (permalink / raw)
To: Yinglin Sun
Cc: Jay Vosburgh, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <CAN17JHUkeCarKf3U-0EMEyL+RJvZGV3GSQWG7+TZFy7ZvD8jYA@mail.gmail.com>
On Thu, Oct 06, 2011 at 06:24:36PM -0700, Yinglin Sun wrote:
> On Thu, Oct 6, 2011 at 5:59 PM, Jay Vosburgh <fubar@us.ibm.com> wrote:
> > Yinglin Sun <Yinglin.Sun@emc.com> wrote:
> >
> >>On Thu, Oct 6, 2011 at 3:17 PM, Yinglin Sun <Yinglin.Sun@emc.com> wrote:
> >>>
> >>> On Thu, Oct 6, 2011 at 12:05 PM, Jay Vosburgh <fubar@us.ibm.com> wrote:
> >>> >
> >>> > Neil Horman <nhorman@tuxdriver.com> wrote:
> >>> >
> >>> > >On Wed, Oct 05, 2011 at 08:59:10PM -0700, Yinglin Sun wrote:
> >>> > >> Steps to reproduce this issue:
> >>> > >> 1. create bond0 over eth0 and eth1, set the mode to balance-xor
> >>> > >> 2. add an IPv6 address to bond0
> >>> > >> 3. DAD packet is sent out from one slave and then is looped back from
> >>> > >> the other slave. Therefore, it is treated as a duplicate address and
> >>> > >> stays tentative afterwards:
> >>> > >> kern.info:
> >>> > >> Oct 5 11:50:18 testvm1 kernel: [ 129.224353] bond0: IPv6 duplicate address 1234::1 detected!
> >
> > [...]
> >
> >>> > >Nack, This seems like it will just completely break DAD. What if theres another
> >>> > >system out there with the same mac address. A response from that system would
> >>> > >get dropped by this filter, instead of causing The local system to stop using
> >>> > >the address. What you really want to do is modify
> >>> > >bond_should_deliver_exact_match to detect this frame on the inactive slave or
> >>> > >some such, and drop the frame there.
> >>> >
> >>> > Also NACK; and adding a bit of information. The balance-xor
> >>> > mode is nominally expecting to interact with a switch whose ports are
> >>> > set for etherchannel ("static link aggregation"), in which case the
> >>> > switch will not loop the packet back around.
> >>> >
> >>> > If your switch can do etherchannel, then enable it and the
> >>> > problem should go away. If your switch cannot do this, then you may
> >>> > have other issues, because all of the multicast or broadcast packets
> >>> > going out any bonding slave will loop around to another slave. You
> >>> > could also use 802.3ad / LACP if you switch supports that.
> >>> >
> >>> > For balance-xor (or balance-rr, for that matter) mode to a
> >>> > non-etherchannel switch, it's going to be difficult, if not impossible,
> >>> > to modify bond_should_deliver_exact_match, because there are no inactive
> >>> > slaves. In this mode, bonding is expecting the switch to balance
> >>> > incoming traffic across the ports, and not deliver looped back packets
> >>> > or duplicates. There are no restrictions on what type of traffic
> >>> > (mcast, bcast, ucast) may arrive on any given port.
> >>> >
> >>> > I can't think of a way to make the non-etherchannel case work
> >>> > for balance-xor (or balance-rr) without breaking the DAD functionality
> >>> > in the case of an actual duplicate. I'm not aware of a way to
> >>> > distinguish a looped back DAD probe from an actual duplicate address
> >>> > probe elsewhere on the network.
> >>> >
> >>>
> >>> Hi Neil & Jay,
> >>>
> >>> Thanks a lot for the comments.
> >>>
> >>> The use case is to add IPv6 address on the bonding interface first,
> >>> and then set up port channel on switch. We'll hit this issue and the
> >>> new address will stay tentative and unusable after port channel is set
> >>> up on switch. This patch is for this valid use case.
> >>>
> >>> Except failover mode, all slaves are active on receiving packets, so
> >>> we are receiving such looped back DAD and the bonding driver cannot
> >>> ignore them. I cannot think of a way to distinguish if a DAD is looped
> >>> back or from someone else having the same mac address. They look the
> >>> same to the host. If there is another machine having the same mac
> >>> address, this code path gets executed if both are doing DAD at the
> >>> same time for the same IPv6 address. Maybe we should find out what the
> >>> specification defines for this case?
> >>>
> >>
> >>RFC4862 has a discussion about this issue:
> >>http://tools.ietf.org/html/rfc4862#appendix-A
> >>The better solution could be to record the number of DAD sent out. If
> >>we received more DAD packets than we sent out, there is someone else
> >>on the network who has the same mac address and sent DAD for the same
> >>IPv6 address. However, this solution doesn't work with bonding
> >>interface, since all other active slaves but the one sending out DAD
> >>will receive packet looped back. It doesn't seem there is a simple
> >>solution for this issue.
> >
> > Why are you setting up the port channel after configuring the
> > bond?
> >
> > As a possible workaround, if you have control over the setup
> > process (perhaps it's some sort of manual process), adding one slave to
> > the bond, leaving the other soon-to-be slaves down, then setting up the
> > switch, and finally adding the remaining slaves should work around the
> > issue, since if the bond has only one slave it won't see any looped
> > packets.
> >
> > Or you could bring the bond up as active-backup, then change the
> > mode to balance-xor once the switch is configured.
> >
> > Ultimately, though, the problem stems from the settings mismatch
> > between the switch and the bonding system; balance-xor is meant to
> > interoperate with etherchannel, and when the switch is not configured
> > properly, correct behavior is difficult to guarantee.
> >
>
> Jay,
>
> Thanks a lot for the suggestion.
>
> It's mainly about usability. We would like to provide customers with
> consistent IPv6 configuration procedures as IPv4. Such workarounds
> could be confusing and generate customer calls.
>
Its not a workaround, its the way it has to be done. You can't just drop dad
packets because you can't tell the difference between those that are looped back
and those that are legitimaely from other hosts, so you need to do something
like what Jay is suggesting.
> Yinglin
>
^ permalink raw reply
* Re: [PATCH 1/2 net-next] r6040: invoke phy_{start,stop} when appropriate
From: Eric Dumazet @ 2011-10-07 10:24 UTC (permalink / raw)
To: Florian Fainelli; +Cc: davem, netdev
In-Reply-To: <201110071136.22107.florian@openwrt.org>
Le vendredi 07 octobre 2011 à 11:36 +0200, Florian Fainelli a écrit :
> Joe reported to me that right after a bring up of a r6040 interface
> the ethtool output had no consistent output with respect to link duplex
> and speed. Fix this by adding a missing phy_start call in r6040_up and
> conversely a phy_stop call in r6040_down to properly initialize phy states.
>
> Reported-by: Joe Chou <Joe.Chou@rdc.com.tw>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
> index 2bbadc0..a128c3d 100644
> --- a/drivers/net/ethernet/rdc/r6040.c
> +++ b/drivers/net/ethernet/rdc/r6040.c
> @@ -470,6 +470,8 @@ static void r6040_down(struct net_device *dev)
> iowrite16(adrp[0], ioaddr + MID_0L);
> iowrite16(adrp[1], ioaddr + MID_0M);
> iowrite16(adrp[2], ioaddr + MID_0H);
> +
> + phy_stop(lp->phydev);
> }
>
> static int r6040_close(struct net_device *dev)
> @@ -727,6 +729,8 @@ static int r6040_up(struct net_device *dev)
> /* Initialize all MAC registers */
> r6040_init_mac_regs(dev);
>
> + phy_start(lp->phydev);
> +
> return 0;
> }
>
You dont need to submit your patches twice (net and net-next)
If its a bug fix, base your patch against net tree
If its a new feature, against net-next
^ permalink raw reply
* [PATCH 2/2 net] r6040: bump version to 0.28 and date to 07Oct2011.
From: Florian Fainelli @ 2011-10-07 9:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, stable
Signed-off-by: Florian Fainelli <florian@openwrt.org>
CC: stable@kernel.org # 2.6.38+
---
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index ab59538..6b21565 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -49,8 +49,8 @@
#include <asm/processor.h>
#define DRV_NAME "r6040"
-#define DRV_VERSION "0.27"
-#define DRV_RELDATE "23Feb2011"
+#define DRV_VERSION "0.28"
+#define DRV_RELDATE "07Oct2011"
/* PHY CHIP Address */
#define PHY1_ADDR 1 /* For MAC1 */
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2 net] r6040: invoke phy_{start,stop} when appropriate
From: Florian Fainelli @ 2011-10-07 9:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev, stable
Joe reported to me that right after a bring up of a r6040 interface
the ethtool output had no consistent output with respect to link duplex
and speed. Fix this by adding a missing phy_start call in r6040_up and
conversely a phy_stop call in r6040_down to properly initialize phy states.
Reported-by: Joe Chou <Joe.Chou@rdc.com.tw>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
CC: stable@kernel.org # 2.6.38+
---
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index b64fcee..ab59538 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -470,6 +470,8 @@ static void r6040_down(struct net_device *dev)
iowrite16(adrp[0], ioaddr + MID_0L);
iowrite16(adrp[1], ioaddr + MID_0M);
iowrite16(adrp[2], ioaddr + MID_0H);
+
+ phy_stop(lp->phydev);
}
static int r6040_close(struct net_device *dev)
@@ -727,6 +729,8 @@ static int r6040_up(struct net_device *dev)
/* Initialize all MAC registers */
r6040_init_mac_regs(dev);
+ phy_start(lp->phydev);
+
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/2 net-next] r6040: bump version to 0.28 and date to 07Oct2011.
From: Florian Fainelli @ 2011-10-07 9:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index a128c3d..1fc01ca 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -49,8 +49,8 @@
#include <asm/processor.h>
#define DRV_NAME "r6040"
-#define DRV_VERSION "0.27"
-#define DRV_RELDATE "23Feb2011"
+#define DRV_VERSION "0.28"
+#define DRV_RELDATE "07Oct2011"
/* PHY CHIP Address */
#define PHY1_ADDR 1 /* For MAC1 */
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2 net-next] r6040: invoke phy_{start,stop} when appropriate
From: Florian Fainelli @ 2011-10-07 9:36 UTC (permalink / raw)
To: davem; +Cc: netdev
Joe reported to me that right after a bring up of a r6040 interface
the ethtool output had no consistent output with respect to link duplex
and speed. Fix this by adding a missing phy_start call in r6040_up and
conversely a phy_stop call in r6040_down to properly initialize phy states.
Reported-by: Joe Chou <Joe.Chou@rdc.com.tw>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 2bbadc0..a128c3d 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -470,6 +470,8 @@ static void r6040_down(struct net_device *dev)
iowrite16(adrp[0], ioaddr + MID_0L);
iowrite16(adrp[1], ioaddr + MID_0M);
iowrite16(adrp[2], ioaddr + MID_0H);
+
+ phy_stop(lp->phydev);
}
static int r6040_close(struct net_device *dev)
@@ -727,6 +729,8 @@ static int r6040_up(struct net_device *dev)
/* Initialize all MAC registers */
r6040_init_mac_regs(dev);
+ phy_start(lp->phydev);
+
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v5 0/8] per-cgroup tcp buffer pressure settings
From: KAMEZAWA Hiroyuki @ 2011-10-07 8:55 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
linux-mm, kirill, avagin, devel
In-Reply-To: <4E8EB634.9090208@parallels.com>
On Fri, 7 Oct 2011 12:20:04 +0400
Glauber Costa <glommer@parallels.com> wrote:
> >
> >> So what I really mean here with "will integrate later", is that I think
> >> that we'd be better off tracking the allocations themselves at the slab
> >> level.
> >>
> >>> Can't tcp-limit-code borrows some amount of charges in batch from kmem_limit
> >>> and use it ?
> >> Sorry, I don't know what exactly do you mean. Can you clarify?
> >>
> > Now, tcp-usage is independent from kmem-usage.
> >
> > My idea is
> >
> > 1. when you account tcp usage, charge kmem, too.
>
> Absolutely.
> > Now, your work is
> > a) tcp use new xxxx bytes.
> > b) account it to tcp.uage and check tcp limit
> >
> > To ingegrate kmem,
> > a) tcp use new xxxx bytes.
> > b) account it to tcp.usage and check tcp limit
> > c) account it to kmem.usage
> >
> > ? 2 counters may be slow ?
>
> Well, the way I see it, 1 counter is slow already =)
> I honestly think we need some optimizations here. But
> that is a side issue.
>
> To begin with: The new patchset that I intend to spin
> today or Monday, depending on my progress, uses res_counters,
> as you and Kirill requested.
>
> So what makes res_counters slow IMHO, is two things:
>
> 1) interrupts are always disabled.
> 2) All is done under a lock.
>
> Now, we are starting to have resources that are billed to multiple
> counters. One simple way to work around it, is to have child counters
> that has to be accounted for as well everytime a resource is counted.
>
> Like this:
>
> 1) tcp has kmem as child. When we bill to tcp, we bill to kmem as well.
> For protocols that do memory pressure, we then don't bill kmem from
> the slab.
> 2) When kmem_independent_account is set to 0, kmem has mem as child.
>
Seems reasonable.
> >
> >
> >>> - Don't you need a stat file to indicate "tcp memory pressure works!" ?
> >>> It can be obtained already ?
> >>
> >> Not 100 % clear as well. We can query the amount of buffer used, and the
> >> amount of buffer allowed. What else do we need?
> >>
> >
> > IIUC, we can see the fact tcp.usage is near to tcp.limit but never can see it
> > got memory pressure and how many numbers of failure happens.
> > I'm sorry if I don't read codes correctly.
>
> IIUC, With res_counters being used, we get at least failcnt for free, right?
>
Right. you can get failcnt and max_usage and can have soft_limit base
implemenation at the same time.
Thank you.
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 0/8] per-cgroup tcp buffer pressure settings
From: Glauber Costa @ 2011-10-07 8:20 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
linux-mm, kirill, avagin, devel
In-Reply-To: <20111007170522.624fab3d.kamezawa.hiroyu@jp.fujitsu.com>
On 10/07/2011 12:05 PM, KAMEZAWA Hiroyuki wrote:
>
>
> Sorry for lazy answer.
Hi Kame,
Now matter how hard you try, you'll never be as lazy as I am. So that's
okay.
>
> On Wed, 5 Oct 2011 11:25:50 +0400
> Glauber Costa<glommer@parallels.com> wrote:
>
>> On 10/05/2011 04:29 AM, KAMEZAWA Hiroyuki wrote:
>>> On Tue, 4 Oct 2011 16:17:52 +0400
>>> Glauber Costa<glommer@parallels.com> wrote:
>>>
>
>>> At this stage, my concern is view of interfaces and documenation, and future plans.
>>
>> Okay. I will try to address them as well as I can.
>>
>>> * memory.independent_kmem_limit
>>> If 1, kmem_limit_in_bytes/kmem_usage_in_bytes works.
>>> If 0, kmem_limit_in_bytes/kmem_usage_in_bytes doesn't work and all kmem
>>> usages are controlled under memory.limit_in_bytes.
>>
>> Correct. For the questions below, I won't even look at the code not to
>> get misguided. Let's settle on the desired behavior, and everything that
>> deviates from it, is a bug.
>>
>>> Question:
>>> - What happens when parent/chidlren cgroup has different indepedent_kmem_limit ?
>> I think it should be forbidden. It was raised by Kirill before, and
>> IIRC, he specifically requested it to be. (Okay: Saying it now, makes me
>> realizes that the child can have set it to 1 while parent was 1. But
>> then parent sets it to 0... I don't think I am handling this case).
>>
>
> ok, please put it into TODO list ;)
Done.
>
>
>>> In future plan, kmem.usage_in_bytes should includes tcp.kmem_usage_in_bytes.
>>> And kmem.limit_in_bytes should be the limiation of sum of all kmem.xxxx.limit_in_bytes.
>>
>> I am not sure there will be others xxx.limit_in_bytes. (see below)
>>
>
> ok.
>
>
>>>
>>> Question:
>>> - Why this integration is difficult ?
>> It is not that it is difficult.
>> What happens is that there are two things taking place here:
>> One of them is allocation.
>> The other, is tcp-specific pressure thresholds. Bear with me with the
>> following example code: (from sk_stream_alloc_skb, net/ipv4/tcp.c)
>>
>> 1: skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
>> if (skb) {
>> 3: if (sk_wmem_schedule(sk, skb->truesize)) {
>> /*
>> * Make sure that we have exactly size bytes
>> * available to the caller, no more, no less.
>> */
>> skb_reserve(skb, skb_tailroom(skb) - size);
>> return skb;
>> }
>> __kfree_skb(skb);
>> } else {
>> sk->sk_prot->enter_memory_pressure(sk);
>> sk_stream_moderate_sndbuf(sk);
>> }
>>
>> In line 1, an allocation takes place. This allocs memory from the skbuff
>> slab cache.
>> But then, pressure thresholds are applied in 3. If it fails, we drop the
>> memory buffer even if the allocation succeeded.
>>
>
> Sure.
>
>
>> So this patchset, as I've stated already, cares about pressure
>> conditions only. It is enough to guarantee that no more memory will be
>> pinned that we specified, because we'll free the allocation in case
>> pressure is reached.
>>
>> There is work in progress from guys at google (and I have my very own
>> PoCs as well), to include all slab allocations in kmem.usage_in_bytes.
>>
>
> ok.
>
>
>> So what I really mean here with "will integrate later", is that I think
>> that we'd be better off tracking the allocations themselves at the slab
>> level.
>>
>>> Can't tcp-limit-code borrows some amount of charges in batch from kmem_limit
>>> and use it ?
>> Sorry, I don't know what exactly do you mean. Can you clarify?
>>
> Now, tcp-usage is independent from kmem-usage.
>
> My idea is
>
> 1. when you account tcp usage, charge kmem, too.
Absolutely.
> Now, your work is
> a) tcp use new xxxx bytes.
> b) account it to tcp.uage and check tcp limit
>
> To ingegrate kmem,
> a) tcp use new xxxx bytes.
> b) account it to tcp.usage and check tcp limit
> c) account it to kmem.usage
>
> ? 2 counters may be slow ?
Well, the way I see it, 1 counter is slow already =)
I honestly think we need some optimizations here. But
that is a side issue.
To begin with: The new patchset that I intend to spin
today or Monday, depending on my progress, uses res_counters,
as you and Kirill requested.
So what makes res_counters slow IMHO, is two things:
1) interrupts are always disabled.
2) All is done under a lock.
Now, we are starting to have resources that are billed to multiple
counters. One simple way to work around it, is to have child counters
that has to be accounted for as well everytime a resource is counted.
Like this:
1) tcp has kmem as child. When we bill to tcp, we bill to kmem as well.
For protocols that do memory pressure, we then don't bill kmem from
the slab.
2) When kmem_independent_account is set to 0, kmem has mem as child.
>
>
>>> - Don't you need a stat file to indicate "tcp memory pressure works!" ?
>>> It can be obtained already ?
>>
>> Not 100 % clear as well. We can query the amount of buffer used, and the
>> amount of buffer allowed. What else do we need?
>>
>
> IIUC, we can see the fact tcp.usage is near to tcp.limit but never can see it
> got memory pressure and how many numbers of failure happens.
> I'm sorry if I don't read codes correctly.
IIUC, With res_counters being used, we get at least failcnt for free, right?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v5 0/8] per-cgroup tcp buffer pressure settings
From: KAMEZAWA Hiroyuki @ 2011-10-07 8:05 UTC (permalink / raw)
To: Glauber Costa
Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
linux-mm, kirill, avagin, devel
In-Reply-To: <4E8C067E.6040203@parallels.com>
Sorry for lazy answer.
On Wed, 5 Oct 2011 11:25:50 +0400
Glauber Costa <glommer@parallels.com> wrote:
> On 10/05/2011 04:29 AM, KAMEZAWA Hiroyuki wrote:
> > On Tue, 4 Oct 2011 16:17:52 +0400
> > Glauber Costa<glommer@parallels.com> wrote:
> >
> > At this stage, my concern is view of interfaces and documenation, and future plans.
>
> Okay. I will try to address them as well as I can.
>
> > * memory.independent_kmem_limit
> > If 1, kmem_limit_in_bytes/kmem_usage_in_bytes works.
> > If 0, kmem_limit_in_bytes/kmem_usage_in_bytes doesn't work and all kmem
> > usages are controlled under memory.limit_in_bytes.
>
> Correct. For the questions below, I won't even look at the code not to
> get misguided. Let's settle on the desired behavior, and everything that
> deviates from it, is a bug.
>
> > Question:
> > - What happens when parent/chidlren cgroup has different indepedent_kmem_limit ?
> I think it should be forbidden. It was raised by Kirill before, and
> IIRC, he specifically requested it to be. (Okay: Saying it now, makes me
> realizes that the child can have set it to 1 while parent was 1. But
> then parent sets it to 0... I don't think I am handling this case).
>
ok, please put it into TODO list ;)
> > In future plan, kmem.usage_in_bytes should includes tcp.kmem_usage_in_bytes.
> > And kmem.limit_in_bytes should be the limiation of sum of all kmem.xxxx.limit_in_bytes.
>
> I am not sure there will be others xxx.limit_in_bytes. (see below)
>
ok.
> >
> > Question:
> > - Why this integration is difficult ?
> It is not that it is difficult.
> What happens is that there are two things taking place here:
> One of them is allocation.
> The other, is tcp-specific pressure thresholds. Bear with me with the
> following example code: (from sk_stream_alloc_skb, net/ipv4/tcp.c)
>
> 1: skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
> if (skb) {
> 3: if (sk_wmem_schedule(sk, skb->truesize)) {
> /*
> * Make sure that we have exactly size bytes
> * available to the caller, no more, no less.
> */
> skb_reserve(skb, skb_tailroom(skb) - size);
> return skb;
> }
> __kfree_skb(skb);
> } else {
> sk->sk_prot->enter_memory_pressure(sk);
> sk_stream_moderate_sndbuf(sk);
> }
>
> In line 1, an allocation takes place. This allocs memory from the skbuff
> slab cache.
> But then, pressure thresholds are applied in 3. If it fails, we drop the
> memory buffer even if the allocation succeeded.
>
Sure.
> So this patchset, as I've stated already, cares about pressure
> conditions only. It is enough to guarantee that no more memory will be
> pinned that we specified, because we'll free the allocation in case
> pressure is reached.
>
> There is work in progress from guys at google (and I have my very own
> PoCs as well), to include all slab allocations in kmem.usage_in_bytes.
>
ok.
> So what I really mean here with "will integrate later", is that I think
> that we'd be better off tracking the allocations themselves at the slab
> level.
>
> > Can't tcp-limit-code borrows some amount of charges in batch from kmem_limit
> > and use it ?
> Sorry, I don't know what exactly do you mean. Can you clarify?
>
Now, tcp-usage is independent from kmem-usage.
My idea is
1. when you account tcp usage, charge kmem, too.
Now, your work is
a) tcp use new xxxx bytes.
b) account it to tcp.uage and check tcp limit
To ingegrate kmem,
a) tcp use new xxxx bytes.
b) account it to tcp.usage and check tcp limit
c) account it to kmem.usage
? 2 counters may be slow ?
> > - Don't you need a stat file to indicate "tcp memory pressure works!" ?
> > It can be obtained already ?
>
> Not 100 % clear as well. We can query the amount of buffer used, and the
> amount of buffer allowed. What else do we need?
>
IIUC, we can see the fact tcp.usage is near to tcp.limit but never can see it
got memory pressure and how many numbers of failure happens.
I'm sorry if I don't read codes correctly.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] pch_gbe: compilation warning in pch_gbe_setup_rctl() fixed
From: Vasily Averin @ 2011-10-07 7:59 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Toshiharu Okada, Vasily Averin
From: Vasily Averin <vvs@sw.ru>
compilation warning fixed
drivers/net/pch_gbe/pch_gbe_main.c: In function ‘pch_gbe_setup_rctl’:
drivers/net/pch_gbe/pch_gbe_main.c:701:21: warning: unused variable ‘netdev’
Signed-off-by: Vasily Averin <vvs@sw.ru>
---
drivers/net/pch_gbe/pch_gbe_main.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index b8b4ba2..7659324 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -698,7 +698,6 @@ static void pch_gbe_configure_tx(struct pch_gbe_adapter *adapter)
*/
static void pch_gbe_setup_rctl(struct pch_gbe_adapter *adapter)
{
- struct net_device *netdev = adapter->netdev;
struct pch_gbe_hw *hw = &adapter->hw;
u32 rx_mode, tcpip;
--
1.7.4.1
^ permalink raw reply related
* Re: loopback IP alias breaks tftp?
From: Eric Dumazet @ 2011-10-07 7:29 UTC (permalink / raw)
To: Joel Sing; +Cc: Josh Boyer, Julian Anastasov, netdev
In-Reply-To: <CAA6X_dP_LDO0hQYpixi-DtsKVJZQEgAY5wa4y_wYpEqXVyn7TQ@mail.gmail.com>
Le vendredi 07 octobre 2011 à 18:02 +1100, Joel Sing a écrit :
> On 7 October 2011 00:23, Josh Boyer <jwboyer@redhat.com> wrote:
> >
> > On Thu, Oct 06, 2011 at 12:18:44AM +0300, Julian Anastasov wrote:
> > >
> > > Hello,
> > >
> > > On Wed, 5 Oct 2011, Josh Boyer wrote:
> > >
> > > > Hi All,
> > > >
> > > > We've had a report [1] of a change in behavior when trying to use an IP
> > > > alias to tftp from a loopback device. Apparently the steps outlined in
> > > > the bug worked in 2.6.35, and broke somewhere before 2.6.38.6.
> > > >
> > > > I can confirm the steps fail on a 3.0 based kernel and I'm trying to do
> > > > a git bisect to find the commit involved, but I thought I would send
> > > > this along to see if anyone might have an idea. (Also, I'm not really
> > > > sure how valid of a usecase this was to begin with.)
> > >
> > > What about commit 9fc3bbb4a752f108cf096d96640f3b548bbbce6c ?
> > >
> > > ipv4/route.c: respect prefsrc for local routes
> > >
> > > http://marc.info/?t=129412232500001&r=1&w=2
> > >
> > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=739534
> >
> > Yep. That is exactly what my git bisect said too.
> >
> > So now we have a change in behavior since that commit for the usecase
> > described in the bug above, but I'm unsure if that usecase was ever
> > really valid or if the commit had unintentional side effects.
> >
> > Joel (or anyone else) can you take a look and comment?
>
> Prior to this commit the src address in the local routing table was
> completely ignored, so connecting to a local address always used the
> same source and destination addresses. However, this is not what was
> configured in the local routing table:
>
> $ ifconfig lo:0 127.0.0.2
> $ ip route show table local | grep 127.0.0.2
> local 127.0.0.2 dev lo proto kernel scope host src 127.0.0.1
>
> When an interface has an alias configured, the source address
> installed in the local routing table is always the primary address for
> the interface. The tftp use case you've described now breaks due to
> the way that in.tftpd is determining the reply address (as documented
> in the in.tftpd manual page). This means that it is now responding
> from 127.0.0.1 even though the client connected to 127.0.0.2. Binding
> inetd to a specific address will avoid this issue.
>
> I have not yet looked to see if there is a specific reason for the
> source address selection, however one way of restoring the previous
> behaviour (whilst still respecting the configured source address)
> would be to use a default source address which matches the configured
> address (as is done for primary addresses). I cannot immediately think
> of a reason not to do this, but I've not gone looking at the code.
>
> Worst case scenario if you specifically need the original behaviour
> then the source address can be changed in the local routing table:
>
> $ ip route change table local local 127.0.0.2 dev lo:0 proto kernel
> scope host src 127.0.0.2
> $ ip route show table local | grep 127.0.0.2
> local 127.0.0.2 dev lo proto kernel scope host src 127.0.0.2
Agreed.
The "table local" bit is really the key, because following naive setup
doesnt work.
# ip addr add 127.0.0.11/8 dev lo
# ip route add 127.0.0.11 dev lo src 127.0.0.11
since 127.0.0.1 will still be the src address selected for connections.
# ip ro show table local | grep 127.0.0.11
local 127.0.0.11 dev lo proto kernel scope host src 127.0.0.1
^ permalink raw reply
* Re: [net-next] cs89x0: Move the driver into the Cirrus dir
From: Jeff Kirsher @ 2011-10-07 7:21 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, netdev, gospo, sassmann, Russell Nelson, Andrew Morton
In-Reply-To: <1317971926-23145-2-git-send-email-jeffrey.t.kirsher@intel.com>
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
On 10/07/2011 12:18 AM, Jeff Kirsher wrote:
> The cs89x0 driver was initial placed in the apple/ when it
> should have been placed in the cirrus/. This resolves the
> issue by moving the dirver and fixing up the respective
> Kconfig(s) and Makefile(s).
>
> Thanks to Sascha for reporting the issue.
>
> CC: Russell Nelson <nelson@crynwr.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Sorry, I did not mean to send this out a second time.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]
^ permalink raw reply
* [net-next 13/13] igb: consolidate creation of Tx buffer info and data descriptor
From: Jeff Kirsher @ 2011-10-07 7:18 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1317971926-23145-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change will combine the writes of tx_buffer_info and the Tx data
descriptors into a single function. The advantage of this is that we can
avoid needless memory reads from the buffer info struct and speed things up
by keeping the accesses to the local registers.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 8 +-
drivers/net/ethernet/intel/igb/igb_main.c | 318 ++++++++++++++++-------------
2 files changed, 184 insertions(+), 142 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index b71d186..77793a9 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -135,7 +135,6 @@ struct vf_data_storage {
#define IGB_TX_FLAGS_TSO 0x00000004
#define IGB_TX_FLAGS_IPV4 0x00000008
#define IGB_TX_FLAGS_TSTAMP 0x00000010
-#define IGB_TX_FLAGS_MAPPED_AS_PAGE 0x00000020
#define IGB_TX_FLAGS_VLAN_MASK 0xffff0000
#define IGB_TX_FLAGS_VLAN_SHIFT 16
@@ -144,13 +143,12 @@ struct vf_data_storage {
struct igb_tx_buffer {
union e1000_adv_tx_desc *next_to_watch;
unsigned long time_stamp;
- dma_addr_t dma;
- u32 length;
- u32 tx_flags;
struct sk_buff *skb;
unsigned int bytecount;
u16 gso_segs;
- u8 mapped_as_page;
+ dma_addr_t dma;
+ u32 length;
+ u32 tx_flags;
};
struct igb_rx_buffer {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index dc93d64..862dd7c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3139,29 +3139,26 @@ static void igb_free_all_tx_resources(struct igb_adapter *adapter)
igb_free_tx_resources(adapter->tx_ring[i]);
}
-void igb_unmap_and_free_tx_resource(struct igb_ring *tx_ring,
- struct igb_tx_buffer *buffer_info)
-{
- if (buffer_info->dma) {
- if (buffer_info->tx_flags & IGB_TX_FLAGS_MAPPED_AS_PAGE)
- dma_unmap_page(tx_ring->dev,
- buffer_info->dma,
- buffer_info->length,
- DMA_TO_DEVICE);
- else
- dma_unmap_single(tx_ring->dev,
- buffer_info->dma,
- buffer_info->length,
- DMA_TO_DEVICE);
- buffer_info->dma = 0;
- }
- if (buffer_info->skb) {
- dev_kfree_skb_any(buffer_info->skb);
- buffer_info->skb = NULL;
- }
- buffer_info->time_stamp = 0;
- buffer_info->length = 0;
- buffer_info->next_to_watch = NULL;
+void igb_unmap_and_free_tx_resource(struct igb_ring *ring,
+ struct igb_tx_buffer *tx_buffer)
+{
+ if (tx_buffer->skb) {
+ dev_kfree_skb_any(tx_buffer->skb);
+ if (tx_buffer->dma)
+ dma_unmap_single(ring->dev,
+ tx_buffer->dma,
+ tx_buffer->length,
+ DMA_TO_DEVICE);
+ } else if (tx_buffer->dma) {
+ dma_unmap_page(ring->dev,
+ tx_buffer->dma,
+ tx_buffer->length,
+ DMA_TO_DEVICE);
+ }
+ tx_buffer->next_to_watch = NULL;
+ tx_buffer->skb = NULL;
+ tx_buffer->dma = 0;
+ /* buffer_info must be completely set up in the transmit path */
}
/**
@@ -4138,124 +4135,153 @@ static __le32 igb_tx_olinfo_status(u32 tx_flags, unsigned int paylen,
return cpu_to_le32(olinfo_status);
}
-#define IGB_MAX_TXD_PWR 16
-#define IGB_MAX_DATA_PER_TXD (1<<IGB_MAX_TXD_PWR)
+/*
+ * The largest size we can write to the descriptor is 65535. In order to
+ * maintain a power of two alignment we have to limit ourselves to 32K.
+ */
+#define IGB_MAX_TXD_PWR 15
+#define IGB_MAX_DATA_PER_TXD (1 << IGB_MAX_TXD_PWR)
-static inline int igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
- struct igb_tx_buffer *first, u32 tx_flags)
+static void igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
+ struct igb_tx_buffer *first, u32 tx_flags,
+ const u8 hdr_len)
{
- struct igb_tx_buffer *buffer_info;
- struct device *dev = tx_ring->dev;
- unsigned int hlen = skb_headlen(skb);
- unsigned int count = 0, i;
- unsigned int f;
- u16 gso_segs = skb_shinfo(skb)->gso_segs ?: 1;
-
- i = tx_ring->next_to_use;
-
- buffer_info = &tx_ring->tx_buffer_info[i];
- BUG_ON(hlen >= IGB_MAX_DATA_PER_TXD);
- buffer_info->length = hlen;
- buffer_info->tx_flags = tx_flags;
- buffer_info->dma = dma_map_single(dev, skb->data, hlen,
- DMA_TO_DEVICE);
- if (dma_mapping_error(dev, buffer_info->dma))
+ struct igb_tx_buffer *tx_buffer_info;
+ union e1000_adv_tx_desc *tx_desc;
+ dma_addr_t dma;
+ struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
+ unsigned int data_len = skb->data_len;
+ unsigned int size = skb_headlen(skb);
+ unsigned int paylen = skb->len - hdr_len;
+ __le32 cmd_type;
+ u16 i = tx_ring->next_to_use;
+ u16 gso_segs;
+
+ if (tx_flags & IGB_TX_FLAGS_TSO)
+ gso_segs = skb_shinfo(skb)->gso_segs;
+ else
+ gso_segs = 1;
+
+ /* multiply data chunks by size of headers */
+ first->bytecount = paylen + (gso_segs * hdr_len);
+ first->gso_segs = gso_segs;
+ first->skb = skb;
+
+ tx_desc = IGB_TX_DESC(tx_ring, i);
+
+ tx_desc->read.olinfo_status =
+ igb_tx_olinfo_status(tx_flags, paylen, tx_ring);
+
+ cmd_type = igb_tx_cmd_type(tx_flags);
+
+ dma = dma_map_single(tx_ring->dev, skb->data, size, DMA_TO_DEVICE);
+ if (dma_mapping_error(tx_ring->dev, dma))
goto dma_error;
- tx_flags |= IGB_TX_FLAGS_MAPPED_AS_PAGE;
+ /* record length, and DMA address */
+ first->length = size;
+ first->dma = dma;
+ first->tx_flags = tx_flags;
+ tx_desc->read.buffer_addr = cpu_to_le64(dma);
- for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
- struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[f];
- unsigned int len = frag->size;
+ for (;;) {
+ while (unlikely(size > IGB_MAX_DATA_PER_TXD)) {
+ tx_desc->read.cmd_type_len =
+ cmd_type | cpu_to_le32(IGB_MAX_DATA_PER_TXD);
+
+ i++;
+ tx_desc++;
+ if (i == tx_ring->count) {
+ tx_desc = IGB_TX_DESC(tx_ring, 0);
+ i = 0;
+ }
+
+ dma += IGB_MAX_DATA_PER_TXD;
+ size -= IGB_MAX_DATA_PER_TXD;
+
+ tx_desc->read.olinfo_status = 0;
+ tx_desc->read.buffer_addr = cpu_to_le64(dma);
+ }
+
+ if (likely(!data_len))
+ break;
+
+ tx_desc->read.cmd_type_len = cmd_type | cpu_to_le32(size);
- count++;
i++;
- if (i == tx_ring->count)
+ tx_desc++;
+ if (i == tx_ring->count) {
+ tx_desc = IGB_TX_DESC(tx_ring, 0);
i = 0;
+ }
- buffer_info = &tx_ring->tx_buffer_info[i];
- BUG_ON(len >= IGB_MAX_DATA_PER_TXD);
- buffer_info->length = len;
- buffer_info->tx_flags = tx_flags;
- buffer_info->dma = skb_frag_dma_map(dev, frag, 0, len,
- DMA_TO_DEVICE);
- if (dma_mapping_error(dev, buffer_info->dma))
+ size = frag->size;
+ data_len -= size;
+
+ dma = skb_frag_dma_map(tx_ring->dev, frag, 0,
+ size, DMA_TO_DEVICE);
+ if (dma_mapping_error(tx_ring->dev, dma))
goto dma_error;
+ tx_buffer_info = &tx_ring->tx_buffer_info[i];
+ tx_buffer_info->length = size;
+ tx_buffer_info->dma = dma;
+
+ tx_desc->read.olinfo_status = 0;
+ tx_desc->read.buffer_addr = cpu_to_le64(dma);
+
+ frag++;
}
- buffer_info->skb = skb;
- /* multiply data chunks by size of headers */
- buffer_info->bytecount = ((gso_segs - 1) * hlen) + skb->len;
- buffer_info->gso_segs = gso_segs;
+ /* write last descriptor with RS and EOP bits */
+ cmd_type |= cpu_to_le32(size) | cpu_to_le32(IGB_TXD_DCMD);
+ tx_desc->read.cmd_type_len = cmd_type;
/* set the timestamp */
first->time_stamp = jiffies;
+ /*
+ * Force memory writes to complete before letting h/w know there
+ * are new descriptors to fetch. (Only applicable for weak-ordered
+ * memory model archs, such as IA-64).
+ *
+ * We also need this memory barrier to make certain all of the
+ * status bits have been updated before next_to_watch is written.
+ */
+ wmb();
+
/* set next_to_watch value indicating a packet is present */
- first->next_to_watch = IGB_TX_DESC(tx_ring, i);
+ first->next_to_watch = tx_desc;
- return ++count;
+ i++;
+ if (i == tx_ring->count)
+ i = 0;
-dma_error:
- dev_err(dev, "TX DMA map failed\n");
+ tx_ring->next_to_use = i;
+
+ writel(i, tx_ring->tail);
- /* clear timestamp and dma mappings for failed buffer_info mapping */
- buffer_info->dma = 0;
- buffer_info->time_stamp = 0;
- buffer_info->length = 0;
+ /* we need this if more than one processor can write to our tail
+ * at a time, it syncronizes IO on IA64/Altix systems */
+ mmiowb();
+
+ return;
+
+dma_error:
+ dev_err(tx_ring->dev, "TX DMA map failed\n");
- /* clear timestamp and dma mappings for remaining portion of packet */
- while (count--) {
+ /* clear dma mappings for failed tx_buffer_info map */
+ for (;;) {
+ tx_buffer_info = &tx_ring->tx_buffer_info[i];
+ igb_unmap_and_free_tx_resource(tx_ring, tx_buffer_info);
+ if (tx_buffer_info == first)
+ break;
if (i == 0)
i = tx_ring->count;
i--;
- buffer_info = &tx_ring->tx_buffer_info[i];
- igb_unmap_and_free_tx_resource(tx_ring, buffer_info);
}
- return 0;
-}
-
-static inline void igb_tx_queue(struct igb_ring *tx_ring,
- u32 tx_flags, int count, u32 paylen,
- u8 hdr_len)
-{
- union e1000_adv_tx_desc *tx_desc;
- struct igb_tx_buffer *buffer_info;
- __le32 olinfo_status, cmd_type;
- unsigned int i = tx_ring->next_to_use;
-
- cmd_type = igb_tx_cmd_type(tx_flags);
- olinfo_status = igb_tx_olinfo_status(tx_flags,
- paylen - hdr_len,
- tx_ring);
-
- do {
- buffer_info = &tx_ring->tx_buffer_info[i];
- tx_desc = IGB_TX_DESC(tx_ring, i);
- tx_desc->read.buffer_addr = cpu_to_le64(buffer_info->dma);
- tx_desc->read.cmd_type_len = cmd_type |
- cpu_to_le32(buffer_info->length);
- tx_desc->read.olinfo_status = olinfo_status;
- count--;
- i++;
- if (i == tx_ring->count)
- i = 0;
- } while (count > 0);
-
- tx_desc->read.cmd_type_len |= cpu_to_le32(IGB_TXD_DCMD);
- /* Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch. (Only
- * applicable for weak-ordered memory model archs,
- * such as IA-64). */
- wmb();
-
tx_ring->next_to_use = i;
- writel(i, tx_ring->tail);
- /* we need this if more than one processor can write to our tail
- * at a time, it syncronizes IO on IA64/Altix systems */
- mmiowb();
}
static int __igb_maybe_stop_tx(struct igb_ring *tx_ring, int size)
@@ -4295,7 +4321,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
struct igb_ring *tx_ring)
{
struct igb_tx_buffer *first;
- int tso, count;
+ int tso;
u32 tx_flags = 0;
__be16 protocol = vlan_get_protocol(skb);
u8 hdr_len = 0;
@@ -4335,19 +4361,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
tx_flags |= IGB_TX_FLAGS_CSUM;
}
- /*
- * count reflects descriptors mapped, if 0 or less then mapping error
- * has occurred and we need to rewind the descriptor queue
- */
- count = igb_tx_map(tx_ring, skb, first, tx_flags);
- if (!count) {
- dev_kfree_skb_any(skb);
- first->time_stamp = 0;
- tx_ring->next_to_use = first - tx_ring->tx_buffer_info;
- return NETDEV_TX_OK;
- }
-
- igb_tx_queue(tx_ring, tx_flags, count, skb->len, hdr_len);
+ igb_tx_map(tx_ring, skb, first, tx_flags, hdr_len);
/* Make sure there is space in the ring for the next send. */
igb_maybe_stop_tx(tx_ring, MAX_SKB_FRAGS + 4);
@@ -5609,17 +5623,26 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
/* clear next_to_watch to prevent false hangs */
tx_buffer->next_to_watch = NULL;
- do {
- tx_desc->wb.status = 0;
- if (likely(tx_desc == eop_desc)) {
- eop_desc = NULL;
+ /* update the statistics for this packet */
+ total_bytes += tx_buffer->bytecount;
+ total_packets += tx_buffer->gso_segs;
- total_bytes += tx_buffer->bytecount;
- total_packets += tx_buffer->gso_segs;
- igb_tx_hwtstamp(q_vector, tx_buffer);
- }
+ /* retrieve hardware timestamp */
+ igb_tx_hwtstamp(q_vector, tx_buffer);
+
+ /* free the skb */
+ dev_kfree_skb_any(tx_buffer->skb);
+ tx_buffer->skb = NULL;
+
+ /* unmap skb header data */
+ dma_unmap_single(tx_ring->dev,
+ tx_buffer->dma,
+ tx_buffer->length,
+ DMA_TO_DEVICE);
- igb_unmap_and_free_tx_resource(tx_ring, tx_buffer);
+ /* clear last DMA location and unmap remaining buffers */
+ while (tx_desc != eop_desc) {
+ tx_buffer->dma = 0;
tx_buffer++;
tx_desc++;
@@ -5629,7 +5652,28 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
tx_buffer = tx_ring->tx_buffer_info;
tx_desc = IGB_TX_DESC(tx_ring, 0);
}
- } while (eop_desc);
+
+ /* unmap any remaining paged data */
+ if (tx_buffer->dma) {
+ dma_unmap_page(tx_ring->dev,
+ tx_buffer->dma,
+ tx_buffer->length,
+ DMA_TO_DEVICE);
+ }
+ }
+
+ /* clear last DMA location */
+ tx_buffer->dma = 0;
+
+ /* move us one more past the eop_desc for start of next pkt */
+ tx_buffer++;
+ tx_desc++;
+ i++;
+ if (unlikely(!i)) {
+ i -= tx_ring->count;
+ tx_buffer = tx_ring->tx_buffer_info;
+ tx_desc = IGB_TX_DESC(tx_ring, 0);
+ }
}
i += tx_ring->count;
--
1.7.6.4
^ permalink raw reply related
* [net-next 12/13] igb: Combine all flag info fields into a single tx_flags structure
From: Jeff Kirsher @ 2011-10-07 7:18 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1317971926-23145-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to combine all of the TX flags fields into one u32
flags field so that it can be stored into the tx_buffer_info structure.
This includes the time stamp flag as well as mapped_as_page flag info.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 9 +++++++++
drivers/net/ethernet/intel/igb/igb_main.c | 24 ++++++++----------------
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 1608110..b71d186 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -130,6 +130,15 @@ struct vf_data_storage {
#define IGB_MNG_VLAN_NONE -1
+#define IGB_TX_FLAGS_CSUM 0x00000001
+#define IGB_TX_FLAGS_VLAN 0x00000002
+#define IGB_TX_FLAGS_TSO 0x00000004
+#define IGB_TX_FLAGS_IPV4 0x00000008
+#define IGB_TX_FLAGS_TSTAMP 0x00000010
+#define IGB_TX_FLAGS_MAPPED_AS_PAGE 0x00000020
+#define IGB_TX_FLAGS_VLAN_MASK 0xffff0000
+#define IGB_TX_FLAGS_VLAN_SHIFT 16
+
/* wrapper around a pointer to a socket buffer,
* so a DMA handle can be stored along with the buffer */
struct igb_tx_buffer {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 3ebeb3e..dc93d64 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3143,7 +3143,7 @@ void igb_unmap_and_free_tx_resource(struct igb_ring *tx_ring,
struct igb_tx_buffer *buffer_info)
{
if (buffer_info->dma) {
- if (buffer_info->mapped_as_page)
+ if (buffer_info->tx_flags & IGB_TX_FLAGS_MAPPED_AS_PAGE)
dma_unmap_page(tx_ring->dev,
buffer_info->dma,
buffer_info->length,
@@ -3162,7 +3162,6 @@ void igb_unmap_and_free_tx_resource(struct igb_ring *tx_ring,
buffer_info->time_stamp = 0;
buffer_info->length = 0;
buffer_info->next_to_watch = NULL;
- buffer_info->mapped_as_page = false;
}
/**
@@ -3955,14 +3954,6 @@ set_itr_now:
}
}
-#define IGB_TX_FLAGS_CSUM 0x00000001
-#define IGB_TX_FLAGS_VLAN 0x00000002
-#define IGB_TX_FLAGS_TSO 0x00000004
-#define IGB_TX_FLAGS_IPV4 0x00000008
-#define IGB_TX_FLAGS_TSTAMP 0x00000010
-#define IGB_TX_FLAGS_VLAN_MASK 0xffff0000
-#define IGB_TX_FLAGS_VLAN_SHIFT 16
-
void igb_tx_ctxtdesc(struct igb_ring *tx_ring, u32 vlan_macip_lens,
u32 type_tucmd, u32 mss_l4len_idx)
{
@@ -4151,7 +4142,7 @@ static __le32 igb_tx_olinfo_status(u32 tx_flags, unsigned int paylen,
#define IGB_MAX_DATA_PER_TXD (1<<IGB_MAX_TXD_PWR)
static inline int igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
- struct igb_tx_buffer *first)
+ struct igb_tx_buffer *first, u32 tx_flags)
{
struct igb_tx_buffer *buffer_info;
struct device *dev = tx_ring->dev;
@@ -4165,11 +4156,14 @@ static inline int igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
buffer_info = &tx_ring->tx_buffer_info[i];
BUG_ON(hlen >= IGB_MAX_DATA_PER_TXD);
buffer_info->length = hlen;
+ buffer_info->tx_flags = tx_flags;
buffer_info->dma = dma_map_single(dev, skb->data, hlen,
DMA_TO_DEVICE);
if (dma_mapping_error(dev, buffer_info->dma))
goto dma_error;
+ tx_flags |= IGB_TX_FLAGS_MAPPED_AS_PAGE;
+
for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[f];
unsigned int len = frag->size;
@@ -4182,7 +4176,7 @@ static inline int igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
buffer_info = &tx_ring->tx_buffer_info[i];
BUG_ON(len >= IGB_MAX_DATA_PER_TXD);
buffer_info->length = len;
- buffer_info->mapped_as_page = true;
+ buffer_info->tx_flags = tx_flags;
buffer_info->dma = skb_frag_dma_map(dev, frag, 0, len,
DMA_TO_DEVICE);
if (dma_mapping_error(dev, buffer_info->dma))
@@ -4191,7 +4185,6 @@ static inline int igb_tx_map(struct igb_ring *tx_ring, struct sk_buff *skb,
}
buffer_info->skb = skb;
- buffer_info->tx_flags = skb_shinfo(skb)->tx_flags;
/* multiply data chunks by size of headers */
buffer_info->bytecount = ((gso_segs - 1) * hlen) + skb->len;
buffer_info->gso_segs = gso_segs;
@@ -4211,7 +4204,6 @@ dma_error:
buffer_info->dma = 0;
buffer_info->time_stamp = 0;
buffer_info->length = 0;
- buffer_info->mapped_as_page = false;
/* clear timestamp and dma mappings for remaining portion of packet */
while (count--) {
@@ -4347,7 +4339,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
* count reflects descriptors mapped, if 0 or less then mapping error
* has occurred and we need to rewind the descriptor queue
*/
- count = igb_tx_map(tx_ring, skb, first);
+ count = igb_tx_map(tx_ring, skb, first, tx_flags);
if (!count) {
dev_kfree_skb_any(skb);
first->time_stamp = 0;
@@ -5567,7 +5559,7 @@ static void igb_tx_hwtstamp(struct igb_q_vector *q_vector,
u64 regval;
/* if skb does not support hw timestamp or TX stamp not valid exit */
- if (likely(!(buffer_info->tx_flags & SKBTX_HW_TSTAMP)) ||
+ if (likely(!(buffer_info->tx_flags & IGB_TX_FLAGS_TSTAMP)) ||
!(rd32(E1000_TSYNCTXCTL) & E1000_TSYNCTXCTL_VALID))
return;
--
1.7.6.4
^ permalink raw reply related
* [net-next 11/13] igb: Cleanup protocol handling in transmit path
From: Jeff Kirsher @ 2011-10-07 7:18 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1317971926-23145-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change is meant to cleanup the protocol handling in the transmit path
so that it correctly offloads software VLAN tagged frames.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++------------
1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 2c61ec4..3ebeb3e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3987,8 +3987,8 @@ void igb_tx_ctxtdesc(struct igb_ring *tx_ring, u32 vlan_macip_lens,
context_desc->mss_l4len_idx = cpu_to_le32(mss_l4len_idx);
}
-static inline int igb_tso(struct igb_ring *tx_ring,
- struct sk_buff *skb, u32 tx_flags, u8 *hdr_len)
+static inline int igb_tso(struct igb_ring *tx_ring, struct sk_buff *skb,
+ u32 tx_flags, __be16 protocol, u8 *hdr_len)
{
int err;
u32 vlan_macip_lens, type_tucmd;
@@ -4006,7 +4006,7 @@ static inline int igb_tso(struct igb_ring *tx_ring,
/* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
type_tucmd = E1000_ADVTXD_TUCMD_L4T_TCP;
- if (skb->protocol == htons(ETH_P_IP)) {
+ if (protocol == __constant_htons(ETH_P_IP)) {
struct iphdr *iph = ip_hdr(skb);
iph->tot_len = 0;
iph->check = 0;
@@ -4039,8 +4039,8 @@ static inline int igb_tso(struct igb_ring *tx_ring,
return 1;
}
-static inline bool igb_tx_csum(struct igb_ring *tx_ring,
- struct sk_buff *skb, u32 tx_flags)
+static inline bool igb_tx_csum(struct igb_ring *tx_ring, struct sk_buff *skb,
+ u32 tx_flags, __be16 protocol)
{
u32 vlan_macip_lens = 0;
u32 mss_l4len_idx = 0;
@@ -4051,7 +4051,7 @@ static inline bool igb_tx_csum(struct igb_ring *tx_ring,
return false;
} else {
u8 l4_hdr = 0;
- switch (skb->protocol) {
+ switch (protocol) {
case __constant_htons(ETH_P_IP):
vlan_macip_lens |= skb_network_header_len(skb);
type_tucmd |= E1000_ADVTXD_TUCMD_IPV4;
@@ -4065,7 +4065,7 @@ static inline bool igb_tx_csum(struct igb_ring *tx_ring,
if (unlikely(net_ratelimit())) {
dev_warn(tx_ring->dev,
"partial checksum but proto=%x!\n",
- skb->protocol);
+ protocol);
}
break;
}
@@ -4305,6 +4305,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
struct igb_tx_buffer *first;
int tso, count;
u32 tx_flags = 0;
+ __be16 protocol = vlan_get_protocol(skb);
u8 hdr_len = 0;
/* need: 1 descriptor per page,
@@ -4330,16 +4331,14 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
/* record the location of the first descriptor for this packet */
first = &tx_ring->tx_buffer_info[tx_ring->next_to_use];
- tso = igb_tso(tx_ring, skb, tx_flags, &hdr_len);
-
+ tso = igb_tso(tx_ring, skb, tx_flags, protocol, &hdr_len);
if (tso < 0) {
goto out_drop;
} else if (tso) {
tx_flags |= IGB_TX_FLAGS_TSO | IGB_TX_FLAGS_CSUM;
- if (skb->protocol == htons(ETH_P_IP))
+ if (protocol == htons(ETH_P_IP))
tx_flags |= IGB_TX_FLAGS_IPV4;
-
- } else if (igb_tx_csum(tx_ring, skb, tx_flags) &&
+ } else if (igb_tx_csum(tx_ring, skb, tx_flags, protocol) &&
(skb->ip_summed == CHECKSUM_PARTIAL)) {
tx_flags |= IGB_TX_FLAGS_CSUM;
}
--
1.7.6.4
^ permalink raw reply related
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