* Re: [PATCH] netfilter: xt_condition: add security capability support
From: Luciano Coelho @ 2010-08-23 18:45 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: ext Changli Gao, kaber@trash.net, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, James Morris,
linux-security-module@vger.kernel.org
In-Reply-To: <alpine.LNX.2.01.1008231628010.21000@obet.zrqbmnf.qr>
On Mon, 2010-08-23 at 16:34 +0200, ext Jan Engelhardt wrote:
> On Monday 2010-08-23 15:42, Luciano Coelho wrote:
> >> > /* Defaults, these can be overridden on the module command-line. */
> >> > static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
> >> > static unsigned int condition_uid_perms = 0;
> >> > static unsigned int condition_gid_perms = 0;
> >> > +static unsigned int condition_capabilities = CAP_NET_ADMIN;
> >> >
> >> It is strange that we set security policy in this way. Maybe the
> >> permission of the proc file is enough in this case.
> >
> >Yes, that is another way to do it. But in our device we use security
> >capabilities more extensively than normal file permissions. That's why
> >we need this.
> >
> >If this is too restrictive (ie. having CAP_NET_ADMIN) for most users, we
> >could change the default value to no capabilities needed. Then we can
> >set CAP_NET_ADMIN when loading the module.
>
> But it looks as strange as the Yama code attempt.
What is so strange about it? Is it because it's possible to set the
capability requirement from modprobe arguments? The capability check
already exists in at least in nfnetlink, where it checks for received
messages for the CAP_NET_ADMIN capability. Is it strange because we're
checking for the capability when someone tries to write to a file?
> This is the one time
> where I would personally be looking into SELinux, or perhaps SMACK if
> the former is too complex, to whether _t'ing off procfs is possible.
Yeah, but it's not up to me to decide this. We have one entire team
dedicated to figuring out how to ensure "security" in our device. It
was that team who advised us to protect this file by checking for
CAP_NET_ADMIN.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH] netfilter: xt_condition: add security capability support
From: Jan Engelhardt @ 2010-08-23 18:58 UTC (permalink / raw)
To: Luciano Coelho
Cc: ext Changli Gao, kaber@trash.net, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, James Morris,
linux-security-module@vger.kernel.org
In-Reply-To: <1282589101.7198.28.camel@powerslave>
On Monday 2010-08-23 20:45, Luciano Coelho wrote:
>> But it looks as strange as the Yama code attempt.
>
>What is so strange about it? Is it because it's possible to set the
>capability requirement from modprobe arguments? The capability check
>already exists in at least in nfnetlink, where it checks for received
>messages for the CAP_NET_ADMIN capability. Is it strange because we're
>checking for the capability when someone tries to write to a file?
It is strange that you check this capability from a module focused on
packet handling. For lack of a better example, it's as if you tried
to check the uid of the file, the latter of which is better left to
the routines in fs/.
>> This is the one time
>> where I would personally be looking into SELinux, or perhaps SMACK if
>> the former is too complex, to whether _t'ing off procfs is possible.
>
>Yeah, but it's not up to me to decide this. We have one entire team
>dedicated to figuring out how to ensure "security" in our device. It
>was that team who advised us to protect this file by checking for
>CAP_NET_ADMIN.
You can do whatever you want with your product. I am just saying this
does not look like kernel material, and I suppose it won't go well
with the maintainers up the chain either.
^ permalink raw reply
* [PATCH] tcp: make TCP quick ACK behavior modifiable
From: Hagen Paul Pfeifer @ 2010-08-23 19:00 UTC (permalink / raw)
To: netdev
Cc: Hagen Paul Pfeifer, David S. Miller, Eric Dumazet,
Ilpo Järvinen
The TCP quick ACK mechanism analyze if a connection is interactive or
not. Per default the quick ACK mechanism is enabled and ACK packets are
triggered instantly to raise the CWND fast - which is clever for
bulk data (non-interactive) flows. On the other hand interactive protocols
like HTTP, SMTP or XMPP will suffer from the quick ACK mechanism
because one additional packets is generated. A simple heuristic
detects if a connection is interactive (pingpong) and if so,
disable the quick ACK. But, the mechanism is not in the ability to
blindly guess if a connection is interactive, and so it must wait for at
least one return packet with payload.
For the server side this requires one additional packet because (packet
number 5 and 6 can be combined):
192.168.1.35.44833 > 78.47.222.210.80: Flags [S], seq 2854340018, win 5840, options [mss 1460,sackOK,TS val 4382726 ecr 0,nop,wscale 7], length 0
78.47.222.210.80 > 192.168.1.35.44833: Flags [S.], seq 719041385, ack 2854340019, win 5792, options [mss 1452,sackOK,TS val 2606891996 ecr 4382726,nop,wscale 7], length 0
192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 0
192.168.1.35.44833 > 78.47.222.210.80: Flags [P.], seq 1:682, ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 681
78.47.222.210.80 > 192.168.1.35.44833: Flags [.], ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 0
78.47.222.210.80 > 192.168.1.35.44833: Flags [.], seq 1:1441, ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 1440
192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1441, win 69, options [nop,nop,TS val 4382737 ecr 2606892002], length 0
This patch provides a sysctl interface for the administrator to globally
enable or disable TCP quick ACKs. Short lived protocols like HTTP will
save a non unimportant portion of packets!
Disable TCP Quick ACK:
$ echo 0 > /proc/sys/net/ipv4/tcp_quickack
Enable TCP Quick ACK:
$ echo 1 > /proc/sys/net/ipv4/tcp_quickack
Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/net/inet_connection_sock.h | 3 +++
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv4/tcp.c | 2 ++
3 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index b6d3b55..da2fbaf 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -170,9 +170,12 @@ static inline int inet_csk_ack_scheduled(const struct sock *sk)
return inet_csk(sk)->icsk_ack.pending & ICSK_ACK_SCHED;
}
+extern int sysctl_tcp_quickack;
+
static inline void inet_csk_delack_init(struct sock *sk)
{
memset(&inet_csk(sk)->icsk_ack, 0, sizeof(inet_csk(sk)->icsk_ack));
+ inet_csk(sk)->icsk_ack.pingpong = sysctl_tcp_quickack ? 0 : 1;
}
extern void inet_csk_delete_keepalive_timer(struct sock *sk);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index d96c1da..8923ca8 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -394,6 +394,13 @@ static struct ctl_table ipv4_table[] = {
.proc_handler = proc_dointvec
},
{
+ .procname = "tcp_quickack",
+ .data = &sysctl_tcp_quickack,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
+ {
.procname = "tcp_mem",
.data = &sysctl_tcp_mem,
.maxlen = sizeof(sysctl_tcp_mem),
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 176e11a..5161689 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -279,6 +279,8 @@
int sysctl_tcp_fin_timeout __read_mostly = TCP_FIN_TIMEOUT;
+int sysctl_tcp_quickack __read_mostly = 1;
+
struct percpu_counter tcp_orphan_count;
EXPORT_SYMBOL_GPL(tcp_orphan_count);
--
1.7.2.1.95.g3d045.dirty
^ permalink raw reply related
* Re: [PATCH] tcp: make TCP quick ACK behavior modifiable
From: Stephen Hemminger @ 2010-08-23 19:14 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: netdev, David S. Miller, Eric Dumazet, Ilpo Järvinen
In-Reply-To: <1282590037-18566-1-git-send-email-hagen@jauu.net>
On Mon, 23 Aug 2010 21:00:37 +0200
Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> The TCP quick ACK mechanism analyze if a connection is interactive or
> not. Per default the quick ACK mechanism is enabled and ACK packets are
> triggered instantly to raise the CWND fast - which is clever for
> bulk data (non-interactive) flows. On the other hand interactive protocols
> like HTTP, SMTP or XMPP will suffer from the quick ACK mechanism
> because one additional packets is generated. A simple heuristic
> detects if a connection is interactive (pingpong) and if so,
> disable the quick ACK. But, the mechanism is not in the ability to
> blindly guess if a connection is interactive, and so it must wait for at
> least one return packet with payload.
>
> For the server side this requires one additional packet because (packet
> number 5 and 6 can be combined):
>
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [S], seq 2854340018, win 5840, options [mss 1460,sackOK,TS val 4382726 ecr 0,nop,wscale 7], length 0
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [S.], seq 719041385, ack 2854340019, win 5792, options [mss 1452,sackOK,TS val 2606891996 ecr 4382726,nop,wscale 7], length 0
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 0
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [P.], seq 1:682, ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 681
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [.], ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 0
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [.], seq 1:1441, ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 1440
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1441, win 69, options [nop,nop,TS val 4382737 ecr 2606892002], length 0
>
> This patch provides a sysctl interface for the administrator to globally
> enable or disable TCP quick ACKs. Short lived protocols like HTTP will
> save a non unimportant portion of packets!
If this is configurable (still not sure about having yet more
TCP knobs). It should either be per-socket or a route metric so it can
be controlled on a per-path basis.
--
^ permalink raw reply
* Re: [Bug 16617] New: inconsistent lock state at boot
From: Andrew Morton @ 2010-08-23 19:18 UTC (permalink / raw)
To: netdev; +Cc: bugzilla-daemon, Patrick McHardy, casteyde.christian
In-Reply-To: <bug-16617-27@https.bugzilla.kernel.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Tue, 17 Aug 2010 20:05:55 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=16617
>
> Summary: inconsistent lock state at boot
> Product: Memory Management
> Version: 2.5
> Kernel Version: 2.6.36-rc1
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> AssignedTo: akpm@linux-foundation.org
> ReportedBy: casteyde.christian@free.fr
> Regression: Yes
>
>
> Kernel 2.6.36-rc1
> Athlon64 X2 4200 in 64bit mode
> Slackware64 13.1
> 1 GB RAM
>
> Since 2.6.36-rc1 with debug options activated (lock validation, kmemcheck,
> etc.), I get the following error at boot:
>
> ADDRCONF(NETDEV_UP): eth0: link is not ready
> uli526x: eth0 NIC Link is Up 100 Mbps Full duplex
> ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.36-rc1 #2
> ---------------------------------
> inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> iptables/1915 [HC0[0]:SC0[0]:HE1:SE1] takes:
> (&(&lock->lock)->rlock){+.?...}, at: [<ffffffff814d7c01>]
> get_counters+0x111/0x1c0
> {IN-SOFTIRQ-W} state was registered at:
> [<ffffffff81074e66>] __lock_acquire+0xb96/0x1c10
> [<ffffffff81075f3a>] lock_acquire+0x5a/0x70
> [<ffffffff81595996>] _raw_spin_lock+0x36/0x50
> [<ffffffff8152bfc8>] ip6t_do_table+0x88/0x4e0
> [<ffffffff8152e437>] ip6table_filter_hook+0x17/0x20
> [<ffffffff8147ef54>] nf_iterate+0x64/0xa0
> [<ffffffff8147f034>] nf_hook_slow+0xa4/0x150
> [<ffffffff815196ab>] mld_sendpack+0x28b/0x4d0
> [<ffffffff8151a10b>] mld_ifc_timer_expire+0x27b/0x310
> [<ffffffff8104fade>] run_timer_softirq+0x1ae/0x2e0
> [<ffffffff810499a0>] __do_softirq+0xc0/0x160
> [<ffffffff8100324c>] call_softirq+0x1c/0x30
> [<ffffffff81005585>] do_softirq+0x75/0xb0
> [<ffffffff810494d5>] irq_exit+0x95/0xa0
> [<ffffffff8101f5fa>] smp_apic_timer_interrupt+0x6a/0xa0
> [<ffffffff81002d13>] apic_timer_interrupt+0x13/0x20
> [<ffffffff81165c1d>] ext4_lookup+0x9d/0x110
> [<ffffffff810d8f1b>] do_lookup+0x1db/0x270
> [<ffffffff810db135>] link_path_walk+0x445/0xbe0
> [<ffffffff810dba40>] path_walk+0x60/0xe0
> [<ffffffff810dbbc3>] do_path_lookup+0x53/0x60
> [<ffffffff810dc803>] user_path_at+0x53/0xa0
> [<ffffffff810d2ed7>] vfs_fstatat+0x37/0x70
> [<ffffffff810d2f46>] vfs_stat+0x16/0x20
> [<ffffffff810d311f>] sys_newstat+0x1f/0x50
> [<ffffffff810023ab>] system_call_fastpath+0x16/0x1b
> irq event stamp: 73961
> hardirqs last enabled at (73961): [<ffffffff810497ae>]
> local_bh_enable+0x7e/0x100
> hardirqs last disabled at (73959): [<ffffffff81049761>]
> local_bh_enable+0x31/0x100
> softirqs last enabled at (73960): [<ffffffff814d7b6b>] get_counters+0x7b/0x1c0
> softirqs last disabled at (73914): [<ffffffff814d7b1e>] get_counters+0x2e/0x1c0
>
> other info that might help us debug this:
> 2 locks held by iptables/1915:
> #0: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff8149c35a>]
> ip_getsockopt+0x6a/0xb0
> #1: (&xt[i].mutex){+.+.+.}, at: [<ffffffff8148ac4e>]
> xt_find_table_lock+0x3e/0x120
>
> stack backtrace:
> Pid: 1915, comm: iptables Not tainted 2.6.36-rc1 #2
> Call Trace:
> [<ffffffff81072371>] print_usage_bug+0x181/0x1d0
> [<ffffffff810727d9>] mark_lock+0x419/0x630
> [<ffffffff81074edc>] __lock_acquire+0xc0c/0x1c10
> [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> [<ffffffff81075f3a>] lock_acquire+0x5a/0x70
> [<ffffffff814d7c01>] ? get_counters+0x111/0x1c0
> [<ffffffff81072d2d>] ? trace_hardirqs_on_caller+0x13d/0x180
> [<ffffffff81595996>] _raw_spin_lock+0x36/0x50
> [<ffffffff814d7c01>] ? get_counters+0x111/0x1c0
> [<ffffffff810497ae>] ? local_bh_enable+0x7e/0x100
> [<ffffffff814d7c01>] get_counters+0x111/0x1c0
> [<ffffffff814d7cde>] alloc_counters+0x2e/0x50
> [<ffffffff814d94cd>] ? do_ipt_get_ctl+0xfd/0x3a0
> [<ffffffff814d95b1>] do_ipt_get_ctl+0x1e1/0x3a0
> [<ffffffff81072d7d>] ? trace_hardirqs_on+0xd/0x10
> [<ffffffff814808b0>] nf_sockopt+0x90/0xa0
> [<ffffffff814808d3>] nf_getsockopt+0x13/0x20
> [<ffffffff8149c370>] ip_getsockopt+0x80/0xb0
> [<ffffffff814ba391>] raw_getsockopt+0x11/0x30
> [<ffffffff8145242f>] sock_common_getsockopt+0xf/0x20
> [<ffffffff8144f3d0>] sys_getsockopt+0x60/0xc0
> [<ffffffff810023ab>] system_call_fastpath+0x16/0x1b
>
> I haven't managed to find the faulty module.
>
^ permalink raw reply
* Re: IPVS network namespace aware ?
From: Daniel Lezcano @ 2010-08-23 19:21 UTC (permalink / raw)
To: Hans Schillstrom; +Cc: netfilter-devel, netdev
In-Reply-To: <1281099450.2518.21.camel@seasc0214.dyn.rnd.as.sw.ericsson.se>
On 08/06/2010 02:57 PM, Hans Schillstrom wrote:
> Hello
> Is there any one that have made ipvs network namespace aware ?
> There is an urgent need of it within Ericsson.
> I could not find any trace of it in any of the normal lists.
>
> I will start patching ipvs next week or give a helping hand to any
> existing work.
Hi Hans,
do you have a patchset ready (or just a prototype) ? I will be glad to
help you ;)
Thanks
-- Daniel
^ permalink raw reply
* Re: [Bug 16617] New: inconsistent lock state at boot
From: Eric Dumazet @ 2010-08-23 19:25 UTC (permalink / raw)
To: Andrew Morton
Cc: netdev, bugzilla-daemon, Patrick McHardy, casteyde.christian
In-Reply-To: <20100823121837.e246fb69.akpm@linux-foundation.org>
Le lundi 23 août 2010 à 12:18 -0700, Andrew Morton a écrit :
> (switched to email. Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Tue, 17 Aug 2010 20:05:55 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
> > https://bugzilla.kernel.org/show_bug.cgi?id=16617
> >
> > Summary: inconsistent lock state at boot
> > Product: Memory Management
> > Version: 2.5
> > Kernel Version: 2.6.36-rc1
> > Platform: All
> > OS/Version: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: normal
> > Priority: P1
> > Component: Other
> > AssignedTo: akpm@linux-foundation.org
> > ReportedBy: casteyde.christian@free.fr
> > Regression: Yes
> >
> >
> > Kernel 2.6.36-rc1
> > Athlon64 X2 4200 in 64bit mode
> > Slackware64 13.1
> > 1 GB RAM
> >
> > Since 2.6.36-rc1 with debug options activated (lock validation, kmemcheck,
> > etc.), I get the following error at boot:
> >
> > ADDRCONF(NETDEV_UP): eth0: link is not ready
> > uli526x: eth0 NIC Link is Up 100 Mbps Full duplex
> > ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> >
> > =================================
> > [ INFO: inconsistent lock state ]
> > 2.6.36-rc1 #2
> > ---------------------------------
> > inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> > iptables/1915 [HC0[0]:SC0[0]:HE1:SE1] takes:
> > (&(&lock->lock)->rlock){+.?...}, at: [<ffffffff814d7c01>]
> > get_counters+0x111/0x1c0
> > {IN-SOFTIRQ-W} state was registered at:
> > [<ffffffff81074e66>] __lock_acquire+0xb96/0x1c10
> > [<ffffffff81075f3a>] lock_acquire+0x5a/0x70
> > [<ffffffff81595996>] _raw_spin_lock+0x36/0x50
> > [<ffffffff8152bfc8>] ip6t_do_table+0x88/0x4e0
> > [<ffffffff8152e437>] ip6table_filter_hook+0x17/0x20
> > [<ffffffff8147ef54>] nf_iterate+0x64/0xa0
> > [<ffffffff8147f034>] nf_hook_slow+0xa4/0x150
> > [<ffffffff815196ab>] mld_sendpack+0x28b/0x4d0
> > [<ffffffff8151a10b>] mld_ifc_timer_expire+0x27b/0x310
> > [<ffffffff8104fade>] run_timer_softirq+0x1ae/0x2e0
> > [<ffffffff810499a0>] __do_softirq+0xc0/0x160
> > [<ffffffff8100324c>] call_softirq+0x1c/0x30
> > [<ffffffff81005585>] do_softirq+0x75/0xb0
> > [<ffffffff810494d5>] irq_exit+0x95/0xa0
> > [<ffffffff8101f5fa>] smp_apic_timer_interrupt+0x6a/0xa0
> > [<ffffffff81002d13>] apic_timer_interrupt+0x13/0x20
> > [<ffffffff81165c1d>] ext4_lookup+0x9d/0x110
> > [<ffffffff810d8f1b>] do_lookup+0x1db/0x270
> > [<ffffffff810db135>] link_path_walk+0x445/0xbe0
> > [<ffffffff810dba40>] path_walk+0x60/0xe0
> > [<ffffffff810dbbc3>] do_path_lookup+0x53/0x60
> > [<ffffffff810dc803>] user_path_at+0x53/0xa0
> > [<ffffffff810d2ed7>] vfs_fstatat+0x37/0x70
> > [<ffffffff810d2f46>] vfs_stat+0x16/0x20
> > [<ffffffff810d311f>] sys_newstat+0x1f/0x50
> > [<ffffffff810023ab>] system_call_fastpath+0x16/0x1b
> > irq event stamp: 73961
> > hardirqs last enabled at (73961): [<ffffffff810497ae>]
> > local_bh_enable+0x7e/0x100
> > hardirqs last disabled at (73959): [<ffffffff81049761>]
> > local_bh_enable+0x31/0x100
> > softirqs last enabled at (73960): [<ffffffff814d7b6b>] get_counters+0x7b/0x1c0
> > softirqs last disabled at (73914): [<ffffffff814d7b1e>] get_counters+0x2e/0x1c0
> >
> > other info that might help us debug this:
> > 2 locks held by iptables/1915:
> > #0: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff8149c35a>]
> > ip_getsockopt+0x6a/0xb0
> > #1: (&xt[i].mutex){+.+.+.}, at: [<ffffffff8148ac4e>]
> > xt_find_table_lock+0x3e/0x120
> >
> > stack backtrace:
> > Pid: 1915, comm: iptables Not tainted 2.6.36-rc1 #2
> > Call Trace:
> > [<ffffffff81072371>] print_usage_bug+0x181/0x1d0
> > [<ffffffff810727d9>] mark_lock+0x419/0x630
> > [<ffffffff81074edc>] __lock_acquire+0xc0c/0x1c10
> > [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> > [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> > [<ffffffff815957b8>] ? trace_hardirqs_off_thunk+0x3a/0x3c
> > [<ffffffff81075f3a>] lock_acquire+0x5a/0x70
> > [<ffffffff814d7c01>] ? get_counters+0x111/0x1c0
> > [<ffffffff81072d2d>] ? trace_hardirqs_on_caller+0x13d/0x180
> > [<ffffffff81595996>] _raw_spin_lock+0x36/0x50
> > [<ffffffff814d7c01>] ? get_counters+0x111/0x1c0
> > [<ffffffff810497ae>] ? local_bh_enable+0x7e/0x100
> > [<ffffffff814d7c01>] get_counters+0x111/0x1c0
> > [<ffffffff814d7cde>] alloc_counters+0x2e/0x50
> > [<ffffffff814d94cd>] ? do_ipt_get_ctl+0xfd/0x3a0
> > [<ffffffff814d95b1>] do_ipt_get_ctl+0x1e1/0x3a0
> > [<ffffffff81072d7d>] ? trace_hardirqs_on+0xd/0x10
> > [<ffffffff814808b0>] nf_sockopt+0x90/0xa0
> > [<ffffffff814808d3>] nf_getsockopt+0x13/0x20
> > [<ffffffff8149c370>] ip_getsockopt+0x80/0xb0
> > [<ffffffff814ba391>] raw_getsockopt+0x11/0x30
> > [<ffffffff8145242f>] sock_common_getsockopt+0xf/0x20
> > [<ffffffff8144f3d0>] sys_getsockopt+0x60/0xc0
> > [<ffffffff810023ab>] system_call_fastpath+0x16/0x1b
> >
> > I haven't managed to find the faulty module.
> >
Already fixed in Linus tree
commit 001389b9581c13fe5fc357a0f89234f85af4215d
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon Aug 16 10:22:10 2010 +0000
netfilter: {ip,ip6,arp}_tables: avoid lockdep false positive
After commit 24b36f019 (netfilter: {ip,ip6,arp}_tables: dont block
bottom half more than necessary), lockdep can raise a warning
because we attempt to lock a spinlock with BH enabled, while
the same lock is usually locked by another cpu in a softirq context.
Disable again BH to avoid these lockdep warnings.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Diagnosed-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [Bridge] [PATCH v2] bridge: netfilter: fix a memory leak
From: Bart De Schuymer @ 2010-08-23 19:33 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev, bridge
In-Reply-To: <1282539806-25496-1-git-send-email-xiaosuo@gmail.com>
Looks correct to me.
Signed-off-by: Bart De Schuymer <bdschuym@pandora.be>
Changli Gao schreef:
> nf_bridge_alloc() always reset the skb->nf_bridge, so we should always
> put the old one.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v2: Should init tmp->use again because memcpy() overwrite it.
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index 2c911c0..5ed00bd 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -162,8 +162,8 @@ static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb)
> if (tmp) {
> memcpy(tmp, nf_bridge, sizeof(struct nf_bridge_info));
> atomic_set(&tmp->use, 1);
> - nf_bridge_put(nf_bridge);
> }
> + nf_bridge_put(nf_bridge);
> nf_bridge = tmp;
> }
> return nf_bridge;
> _______________________________________________
> Bridge mailing list
> Bridge@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
>
>
--
Bart De Schuymer
www.artinalgorithms.be
^ permalink raw reply
* Re: [PATCH] Fix deadlock between boomerang_interrupt and boomerang_start_tx in 3c59x
From: David Miller @ 2010-08-23 19:38 UTC (permalink / raw)
To: nhorman; +Cc: netdev, klassert
In-Reply-To: <20100823183227.GB12906@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 23 Aug 2010 14:32:27 -0400
> Dave, any further thoughts here? Is the explination regarding the lack of
> lock-race satisfactory, or would you rather we just pull all the pr_* calls from
> the interrupt handler?
Sorry, I have your most recently reply in my inbox and haven't looked into
it yet, will try to do so today.
^ permalink raw reply
* Re: [PATCH] iproute2: add 64bit support to ifstat
From: Eric Dumazet @ 2010-08-23 19:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, David Miller, netdev
In-Reply-To: <20100823103333.67c5f795@nehalam>
Le lundi 23 août 2010 à 10:33 -0700, Stephen Hemminger a écrit :
> I think this breaks the wraparound detection code in this command.
>
>
OK lets fix the bug only, before adding 64bit counters capabilities.
Thanks
[PATCH] iproute2: add 64bit arches support to ifstat
ifstat assumes IFLA_STATS fields are "unsigned long", but they are
__u32. This fix is needed to let ifstat run on 64bit arches.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 5cf2e01..5b229e7 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -49,7 +49,7 @@ int npatterns;
char info_source[128];
int source_mismatch;
-#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(unsigned long))
+#define MAXS (sizeof(struct rtnl_link_stats)/sizeof(__u32))
struct ifstat_ent
{
@@ -58,7 +58,7 @@ struct ifstat_ent
int ifindex;
unsigned long long val[MAXS];
double rate[MAXS];
- unsigned long ival[MAXS];
+ __u32 ival[MAXS];
};
struct ifstat_ent *kern_db;
@@ -187,7 +187,7 @@ void load_raw_table(FILE *fp)
*next++ = 0;
if (sscanf(p, "%llu", n->val+i) != 1)
abort();
- n->ival[i] = (unsigned long)n->val[i];
+ n->ival[i] = (__u32)n->val[i];
p = next;
if (!(next = strchr(p, ' ')))
abort();
@@ -563,8 +563,6 @@ static void usage(void)
" -s, --noupdate don;t update history\n"
" -t, --interval=SECS report average over the last SECS\n"
" -V, --version output version information\n"
-" -z, --zeros show entries with zero activity\n"
-" -e, --errors show errors\n"
" -z, --zeros show entries with zero activity\n");
exit(-1);
@@ -581,8 +579,6 @@ static const struct option longopts[] = {
{ "interval", 1, 0, 't' },
{ "version", 0, 0, 'V' },
{ "zeros", 0, 0, 'z' },
- { "errors", 0, 0, 'e' },
- { "zeros", 0, 0, 'z' },
{ 0 }
};
^ permalink raw reply related
* Re: [PATCH] tcp: make TCP quick ACK behavior modifiable
From: Hagen Paul Pfeifer @ 2010-08-23 19:57 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, David S. Miller, Eric Dumazet, Ilpo Järvinen
In-Reply-To: <20100823121449.29419771@nehalam>
* Stephen Hemminger | 2010-08-23 12:14:49 [-0700]:
>If this is configurable (still not sure about having yet more
>TCP knobs). It should either be per-socket or a route metric so it can
>be controlled on a per-path basis.
Hello Stephen,
I thought about this too. But IMHO it makes no sense because interactive/bulk
characteristic do not depend on the "path". Rather it depends on application
level. Furthermore, how should an administrator configure this on a per path
basis? The administrator knows that he runs a WEB server - great . And then
SHOULD disable Quick ACK and everything is fine, think about typical server
setup.
The only remunerating alternative is to disable TCP quick ACK at all for the
_first_ server ACK. So that the standard delayed ACK is active and therefore
the mechanism can detect if the flow is interactive or not. The drawback is
that for bulk data flows the first ACK is "artificial" delayed. This is the
superior solution IMHO.
Anyway, I think that for most server setup these days (HTTP, SMTP, ...) the
TCP Quick ACK mechanism is contra-productive. Vanilla bulk data transfer
protocols are rarer these days.
Hagen
^ permalink raw reply
* Re: [PATCH] Fix deadlock between boomerang_interrupt and boomerang_start_tx in 3c59x
From: Eric Dumazet @ 2010-08-23 20:01 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev
In-Reply-To: <20100811151257.GB23317@hmsreliant.think-freely.org>
Le mercredi 11 août 2010 à 11:12 -0400, Neil Horman a écrit :
> If netconsole is in use, there is a possibility for deadlock in 3c59x between
> boomerang_interrupt and boomerang_start_xmit. Both routines take the vp->lock,
> and if netconsole is in use, a pr_* call from the boomerang_interrupt routine
> will result in the netconsole code attempting to trnasmit an skb, which can try
> to take the same spin lock, resulting in deadlock.
>
> The fix is pretty straightforward. This patch allocats a bit in the 3c59x
> private structure to indicate that its handling an interrupt. If we get into
> the transmit routine and that bit is set, we can be sure that we have recursed
> and will deadlock if we continue, so instead we just return NETDEV_TX_BUSY, so
> the stack requeues the skb to try again later.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> drivers/net/3c59x.c | 15 ++++++++++++++-
> 1 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
> index c754d88..c685a55 100644
> --- a/drivers/net/3c59x.c
> +++ b/drivers/net/3c59x.c
> @@ -633,7 +633,8 @@ struct vortex_private {
> open:1,
> medialock:1,
> must_free_region:1, /* Flag: if zero, Cardbus owns the I/O region */
> - large_frames:1; /* accept large frames */
> + large_frames:1, /* accept large frames */
> + handling_irq:1; /* private in_irq indicator */
It would be safer and faster to use a dedicated 'int' instead of a
bitfield.
^ permalink raw reply
* Re: [PATCH] iproute2: add 64bit support to ifstat
From: Stephen Hemminger @ 2010-08-23 20:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, David Miller, netdev
In-Reply-To: <1282592696.2378.193.camel@edumazet-laptop>
> @@ -563,8 +563,6 @@ static void usage(void)
> " -s, --noupdate don;t update history\n"
> " -t, --interval=SECS report average over the last SECS\n"
> " -V, --version output version information\n"
> -" -z, --zeros show entries with zero activity\n"
> -" -e, --errors show errors\n"
> " -z, --zeros show entries with zero activity\n");
>
> exit(-1);
> @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> { "interval", 1, 0, 't' },
> { "version", 0, 0, 'V' },
> { "zeros", 0, 0, 'z' },
> - { "errors", 0, 0, 'e' },
> - { "zeros", 0, 0, 'z' },
> { 0 }
> };
>
>
>
Did you mean to break --errors?
^ permalink raw reply
* [PATCH] tc: add meta match on receive hash
From: Stephen Hemminger @ 2010-08-23 20:08 UTC (permalink / raw)
To: David Miller, jamal, Thomas Graf; +Cc: netdev
Trivial extension to existing meta data match rules to allow
matching on skb receive hash value.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/include/linux/tc_ematch/tc_em_meta.h 2010-08-22 11:28:09.208550912 -0700
+++ b/include/linux/tc_ematch/tc_em_meta.h 2010-08-22 11:28:20.560321846 -0700
@@ -79,6 +79,7 @@ enum {
TCF_META_ID_SK_SENDMSG_OFF,
TCF_META_ID_SK_WRITE_PENDING,
TCF_META_ID_VLAN_TAG,
+ TCF_META_ID_RXHASH,
__TCF_META_ID_MAX
};
#define TCF_META_ID_MAX (__TCF_META_ID_MAX - 1)
--- a/net/sched/em_meta.c 2010-08-22 11:23:29.394155391 -0700
+++ b/net/sched/em_meta.c 2010-08-22 11:25:16.088029057 -0700
@@ -223,6 +223,11 @@ META_COLLECTOR(int_maclen)
dst->value = skb->mac_len;
}
+META_COLLECTOR(int_rxhash)
+{
+ dst->value = skb->rxhash;
+}
+
/**************************************************************************
* Netfilter
**************************************************************************/
@@ -541,6 +546,7 @@ static struct meta_ops __meta_ops[TCF_ME
[META_ID(SK_SENDMSG_OFF)] = META_FUNC(int_sk_sendmsg_off),
[META_ID(SK_WRITE_PENDING)] = META_FUNC(int_sk_write_pend),
[META_ID(VLAN_TAG)] = META_FUNC(int_vlan_tag),
+ [META_ID(RXHASH)] = META_FUNC(int_rxhash),
}
};
^ permalink raw reply
* Re: [PATCH] tcp: make TCP quick ACK behavior modifiable
From: Hagen Paul Pfeifer @ 2010-08-23 20:08 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, David S. Miller, Eric Dumazet, Ilpo Järvinen,
Tom Herbert
In-Reply-To: <20100823121449.29419771@nehalam>
* Stephen Hemminger | 2010-08-23 12:14:49 [-0700]:
>If this is configurable (still not sure about having yet more
>TCP knobs). It should either be per-socket or a route metric so it can
>be controlled on a per-path basis.
BTW: quick ACK's are still configurable on the socket basis. But sometimes you
had no access to the code, the maintainer is busy or whatever ..
At least google inc is too busy to disable tcp quick ack's. And I think they will
save a lot of packets! Who make a bet? ;)
IP 192.168.1.35.45632 > 209.85.135.99.80: Flags [S], seq 4207702225, win 5840, options [mss 1460,sackOK,TS val 1669166 ecr 0,nop,wscale 7], length 0
IP 209.85.135.99.80 > 192.168.1.35.45632: Flags [S.], seq 263349349, ack 4207702226, win 5672, options [mss 1430,sackOK,TS val 397205192 ecr 1669166,nop,wscale 6], length 0
IP 192.168.1.35.45632 > 209.85.135.99.80: Flags [.], ack 1, win 46, options [nop,nop,TS val 1669168 ecr 397205192], length 0
IP 192.168.1.35.45632 > 209.85.135.99.80: Flags [P.], seq 1:925, ack 1, win 46, options [nop,nop,TS val 1669168 ecr 397205192], length 924
IP 209.85.135.99.80 > 192.168.1.35.45632: Flags [.], ack 925, win 118, options [nop,nop,TS val 397205208 ecr 1669168], length 0
IP 209.85.135.99.80 > 192.168.1.35.45632: Flags [P.], seq 1:510, ack 925, win 118, options [nop,nop,TS val 397205221 ecr 1669168], length 509
Hagen
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-23 20:08 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linuxppc-dev, devicetree-discuss,
linux-arm-kernel, Krzysztof Halasa, Rodolfo Giometti,
Arnd Bergmann
In-Reply-To: <20100819055518.GA4084@riccoc20.at.omicron.at>
Sorry for the slow response here, I got busy with other work at the end
of last week.
On Thu, 2010-08-19 at 07:55 +0200, Richard Cochran wrote:
> On Wed, Aug 18, 2010 at 05:12:56PM -0700, john stultz wrote:
> > On Wed, 2010-08-18 at 09:19 +0200, Richard Cochran wrote:
> > > The timer/alarm stuff is "ancillary" and is not at all necessary. It
> > > is just a "nice to have." I will happily remove it, if it is too
> > > troubling for people.
> >
> > If there's a compelling argument for it, I'm interested to hear. But
> > again, it seems like just
> > yet-another-way-to-get-alarm/timer-functionality, so before we add an
> > extra API (or widen an existing API) I'd like to understand the need.
>
> We don't really need it, IMHO.
>
> But if we offer clockid_t CLOCK_PTP, then we get timer_settime()
> without any extra effort.
Sure. There are some clear parallels and the API seems to match, but
what I'm asking is: does it make sense from an overall API view that
application developers have to understand?
> > > I was emulating the posix interface. Instead I should use it directly.
> >
> > I'm definitely interested to see what you come up with here. I'm still
> > hesitant with adding a PTP clock_id, but extending the posix-clocks
> > interface in this way isn't unprecedented (see: CLOCK_SGI_CYCLE) I just
> > would like to make sure we don't end up with a clock_id namespace
> > littered with oddball clocks that were not well abstracted (see:
> > CLOCK_SGI_CYCLE :).
> >
> > For instance: imagine if instead of keeping the clocksource abstraction
> > internal to the timekeeping core, we exposed each clocksource to
> > userland via a clock_id. Every arch would have different ids, and each
> > arch might have multiple ids. Programming against that would be a huge
> > pain.
>
> The clockid_t CLOCK_PTP will be arch-neutral.
Sure, but are they conceptually neutral? There are other clock
synchronization algorithms out there. Will they need their own
similar-but-different clock_ids?
Look at the other clock ids and what the represent:
CLOCK_REALTIME : Wall time (possibly freq/offset corrected)
CLOCK_MONOTONIC: Monotonic time (possibly freq corrected).
CLOCK_PROCESS_CPUTIME_ID: Process cpu time.
CLOCK_THREAD_CPUTIME_ID: Thread cpu time.
CLOCK_MONOTONIC_RAW: Non freq corrected monotonic time.
CLOCK_REALTIME_COARSE: Tick granular wall time (filesystem timestamp)
CLOCK_MONOTONIC_COARSE: Tick granular monotonic time.
CLOCK_PTP that you're proposing doesn't seem to be at the same level of
abstraction. I'm not saying that this isn't the right place for it, but
can we take a step back from PTP and consider what your exposing in more
generic terms. In other words, could someone use the same
packet-timestamping hardware to implement a different non-PTP time
synchronization algorithm?
Further, if we're using PTP to synchoronize the system time, then there
shouldn't be any measurable difference between CLOCK_PTP and
CLOCK_REALTIME, no?
> > So in thinking about this, try to focus on what the new clock_id
> > provides that the other existing clockids do not? Are they at comparable
> > levels of abstraction? 15 years from now, are folks likely to still be
> > using it? Will it be maintainable? etc...
>
> Arnd convinced me that clockid_t=CLOCK_PTP is a good fit. My plan
> would be to introduce just one additional syscall:
>
> SYSCALL_DEFINE3(clock_adjtime, const clockid_t, clkid,
> int, ppb, struct timespec __user *, ts)
>
> ppb - desired frequency adjustment in parts per billion
> ts - desired time step (or jump) in <sec,nsec> to correct
> a measured offset
>
> Arguably, this syscall might be useful for other clocks, too.
So yea, obviously the syscall should not be CLOCK_PTP specific, so we
would want it to be usable against CLOCK_REALTIME.
That said, the clock_adjtime your proposing does not seem to be
sufficient for usage by NTPd. So this suggests that it is not generic
enough.
> I think the ancillary features from PTP hardware clocks should be made
> available throught the sysfs. A syscall for these would end up very
> ugly, looking like an ioctl. Also, it is hard to see how these
> features relate to the more general idea of the clockid.
This may be a good approach, but be aware that adding stuff to sysfs
requires similar scrutiny as adding a syscall.
> In contrast, sysfs attributes will fit the need nicely:
>
> 1. enable or disable pps
> 2. enable or disable external timestamps
> 3. read out external timestamp
> 4. configure period for periodic output
Things to consider here:
Do having these options really make sense?
Why would we want pps disabled? And if that does make sense, would it
be better to do so via the existing pps interface instead of adding a
new ptp specific one?
Same for the timestamps and periodic output (ie: and how do they differ
from reading or setting a timer on CLOCK_PTP?)
> > > 1. Use Case: SW timestamping
> > The way I tend to see it: PTP is just one of the many ways to sync
> > system time.
>
> > > 2. Use Case: HW timestamping for industrial control
> > These specialized applications are part of what concerns me the most.
>
> PTP was not invented to just to get a computer's system time in the
> ball park. For that, NTP is good enough. Rather, some people want to
> use their computers for tasks that require close synchronization, like
> industrial control, audio/video streaming, and many others.
>
> Are you saying that we should not support such applications?
Of course not! Just because I'm reviewing and critiquing your work does
not mean our goals are incompatible.
> > For example, I can see some parallels between things like audio
> > processing, where you have a buffer consumed by the card at a certain
> > rate. Now, the card has its own crystal it uses to time its consumption,
> > so it has its own time domain, and could drift from system time. Thus
> > you want to trigger buffer-refill interrupts off of the audio card's
> > clock, not the system time which might run the risk of being late.
> >
> > But again, we don't expose the audio hardware clock to userland in the
> > same way we expose system time.
>
> This is a good example of the poverty (in regards to time
> synchronization) of our current systems.
>
> Lets say I want to build a surround sound audio system, using a set of
> distributed computers, each host connected to one speaker. How I can
> be sure that the samples in one channel (ie one host) pass through the
> DA converter at exactly the same time?
They won't be exactly the same, but to minimize any noticeable
difference we'd need each speaker/client-system that have their system
time closely synced. Then the server-system would need to send the
channel stream and frame times to each client. The clients would then
feed the audio frames to the audio card at the designated times.
This is a little high level and generic and of course, the devil's in
the details:
1) How is the system time synchronized across systems?
2) How is the error between the system time freq and the audio cards
rate addressed?
These are things that need to be addressed, but the high-level design is
what the applications should target, because it doesn't limit them to
the specifics of the details.
By suggesting the application be designed to use CLOCK_PTP, it limits
itself to systems with CLOCK_PTP hardware, and should the application be
ported to a different distributed system that's using RADclocks or some
other synchronization method, it won't function.
What the kernel needs to provide are ways to address #1 and #2 above,
but what the kernel needs to expose to userland should be minimal and
generic.
> > Again, my knowledge in the networking stack is pretty limited. But it
> > would seem that having an interface that does something to the effect of
> > "adjust the timestamp clock on the hardware that generated it from this
> > packet by Xppb" would feel like the right level of abstraction. Its
> > closely related to SO_TIMESTAMP, option right? Would something like
> > using the setsockopt/getsockopt interface with
> > SO_TIMESTAMP_ADJUST/OFFSET/SET/etc be reasonable?
>
> The clock and its adjustment have nothing to do with a network
> socket. The current PTP hacks floating around all add private ioctls
> to the MAC driver. That is the *wrong* way to do it.
Could you clarify on *why* that is the wrong approach?
Maybe this is where some of the confusion is coming from? The subtleties
of the more generic PTP algorithm and how the existence of PTP hardware
clocks change things are not clear to me. My understanding of ptp and
the networking details around it is limited, so your expertise is
appreciated. Might you consider covering some of this via a
Documentation/ptp/overview.txt file in a future version of your patch?
Here's a summary of what I understand:
So from:
http://en.wikipedia.org/wiki/Precision_Time_Protocol#Synchronization
We see the message exchange of Sync/Delay_Req/Delay_Resp, and the
calculation of the local offset from the server (and then a frequency
adjustment over time as offsets values are accumulated).
Without the hardware clock, this all of these messages and their
corresponding timestamps are likely created by PTPd, using clock_gettime
and then adjtimex() to correct for the calculated offset or freq
adjustment. No extra interfaces are necessary, and PTPd is syncing the
system time as accurately as it can. This is how the existing ptpd
projects on the web seem to function.
Now, with PTP hardware on the system, my understanding of what you're
trying to enable with your patches is that the PTP hardware does the
timestamping on both incoming and outgoing messages. PTPd then reads the
pre-timestamped messages, calculates the offset and freq correction, and
then feeds that back into the PTP hardware via your interface. No time
correction is done at all by PTPd.
Instead, you're proposing then to have a PPS signal emitted by the PTP
hardware (via the timer interface on that hardware, if I'm understanding
correctly). This PPS signal would then be picked up by something like
NTPd which would use it to correct the system time.
Questions:
1) When the PTP hardware is doing the timestamping, what API/interface
does PTPd use to get and send the Sync/Delay_req/Delay_Resp messages?
SO_TIMESTAMPed packets from a network device seems the obvious answer,
but your comments above about with regards to my SO_TIMESTAMP_ADJ idea
suggest there's something more subtle here.
2) You've mentioned multiple PTP hardware clocks are possible, but maybe
not practically useful. How does PTPd enumerate the existing clocks, and
know which devices to listen to for Sync/Delay_Resp messages?
The issue I'm trying to address here is the interface inconsistency
between the message timestamping interface (ie: likely from a packet,
possibly multiple sources) and the proposed CLOCK_PTP interface (with
only a single clock being exposed at a time, and that being controlled
by a sysfs interface).
My concerns:
1) Again, I'm not totally comfortable exposing the PTP hardware via the
posix-clocks/timers interface. I'm not dead set against it, but it just
doesn't seem right as a top-level abstraction.
I'm curious if its possible to do the PTP hardware offset/adjustment
calculation in a module internally to the kernel? That would allow the
PPS interface to still be used to sync the system time, and not expose
additional interfaces.
2) If this is a top-level interface, I'd prefer the inconsistency
between how the timestamped messages are received and the proposed
posix_clocks/timer interface be clarified.
For example: does the networking stack need to have the source clock_id
to use for SO_TIMESTAMPing be specified?
3) I still prefer the idea of keeping the PTP hardware adjustment API
close to the existing API to enable the SO_TIMESTAMPing. I realize you
disagree, but would like to understand better why.
thanks again,
-john
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-23 20:21 UTC (permalink / raw)
To: Richard Cochran
Cc: Arnd Bergmann, netdev, linux-kernel, linuxppc-dev,
devicetree-discuss, linux-arm-kernel, Krzysztof Halasa,
Rodolfo Giometti
In-Reply-To: <20100819153847.GA10695@riccoc20.at.omicron.at>
On Thu, 2010-08-19 at 17:38 +0200, Richard Cochran wrote:
> On Thu, Aug 19, 2010 at 02:28:04PM +0200, Arnd Bergmann wrote:
> > My point was that a syscall is better than an ioctl based interface here,
> > which I definitely still think. Given that John knows much more about
> > clocks than I do, we still need to get agreement on the question that
> > he raised, which is whether we actually need to expose this clock to the
> > user or not.
> >
> > If we can find a way to sync system time accurate enough with PTP and
> > PPS, user applications may not need to see two separate clocks at all.
>
> At the very least, one user application (the PTPd) needs to see the
> PTP clock.
>
> > > SYSCALL_DEFINE3(clock_adjtime, const clockid_t, clkid,
> > > int, ppb, struct timespec __user *, ts)
> > >
> > > ppb - desired frequency adjustment in parts per billion
> > > ts - desired time step (or jump) in <sec,nsec> to correct
> > > a measured offset
> > >
> > > Arguably, this syscall might be useful for other clocks, too.
> >
> > This is a mix of adjtime and adjtimex with the addition of
> > the clkid parameter, right?
>
> Sort of, but not really. ADJTIME(3) takes an offset and slowly
> corrects the clock using a servo in the kernel, over hours.
>
> For this function, the offset passed in the 'ts' parameter will be
> immediately corrected, by jumping to the new time. This reflects the
> way that PTP works. After the first few samples, the PTPd has an
> estimate of the offset to the master and the rate difference. The PTPd
> can proceed in one of two ways.
>
> 1. If resetting the clock is not desired, then the clock is set to the
> maximum adjustment (in the right direction) until the clock time is
> close to the master's time.
>
> 2. The estimated offset is added to the current time, resulting in a
> jump in time.
>
> We need clock_adjtime(id, 0, ts) for the second case.
>
> > Have you considered passing a struct timex instead of ppb and ts?
>
> Yes, but the timex is not suitable, IMHO.
Could you expand on this?
Could we not add a adjustment mode ADJ_SETOFFSET or something that would
provide the instantaneous offset correction?
> > Is using ppb instead of the timex ppm required to get the accuracy
> > you want?
>
> That is one very good reason.
>
> Another is this: can you explain what the 20+ fields mean?
>
> Consider the field, freq. The comment says "frequency offset (scaled
> ppm)." To what is it scaled? The only way I know of to find out is to
> read the NTP code (which is fairly complex) and see what the unit
> really is meant to be. Ditto for the other fields.
>
> The timex structure reveals, AFAICT, the inner workings of the kernel
> clock servo. For PTP, we don't need or want the kernel servo. The PTPd
> has its own clock servo in user space.
You're right that the timex is a little crufty. But its legacy that we
will support indefinitely. So following the established interface helps
maintainability.
So if the clock_adjtime interface is needed, it would seem best for it
to be generic enough to support not only PTP, but also the NTP kernel
PLL.
In effect, this would make clock_adjtime(or clock_adjtimex) identical to
adjtimex, but only applicable to different CLOCK_ids. Additionally, it
would simplify the code for the CLOCK_REALTIME case as you could just
call directly into do_adjtimex().
Of course, extending adjtimex for ADJ_SETOFFSET would be needed first,
but that seems like a reasonable expansion of the interface that could
be used by more then just PTP.
thanks
-john
^ permalink raw reply
* [PATCH] ethtool: allow non-netadmin to query settings
From: Stephen Hemminger @ 2010-08-23 20:24 UTC (permalink / raw)
To: David Miller, Jeff Garzik; +Cc: netdev
The SNMP daemon uses ethtool to determine the speed of
network interfaces. This fails on Debian (and probably elsewhere)
because for security SNMP daemon runs as non-root user (snmp).
Note: A similar patch was rejected previously because of a concern about
the possibility that on some hardware querying the ethtool settings
requires access to the PHY and could slow the machine down. But the
security risk of requiring SNMP daemon (and related services)
to run as root far out weighs the risk of denial-of-service.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/core/ethtool.c 2010-08-22 11:45:34.680419224 -0700
+++ b/net/core/ethtool.c 2010-08-22 11:46:17.207548950 -0700
@@ -1423,6 +1423,7 @@ int dev_ethtool(struct net *net, struct
/* Allow some commands to be done by anyone */
switch (ethcmd) {
+ case ETHTOOL_GSET:
case ETHTOOL_GDRVINFO:
case ETHTOOL_GMSGLVL:
case ETHTOOL_GCOALESCE:
^ permalink raw reply
* Re: [PATCH] iproute2: add 64bit support to ifstat
From: Eric Dumazet @ 2010-08-23 20:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Ben Hutchings, David Miller, netdev
In-Reply-To: <20100823130630.1dcb885b@nehalam>
Le lundi 23 août 2010 à 13:06 -0700, Stephen Hemminger a écrit :
> > @@ -563,8 +563,6 @@ static void usage(void)
> > " -s, --noupdate don;t update history\n"
> > " -t, --interval=SECS report average over the last SECS\n"
> > " -V, --version output version information\n"
> > -" -z, --zeros show entries with zero activity\n"
> > -" -e, --errors show errors\n"
> > " -z, --zeros show entries with zero activity\n");
> >
> > exit(-1);
> > @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> > { "interval", 1, 0, 't' },
> > { "version", 0, 0, 'V' },
> > { "zeros", 0, 0, 'z' },
> > - { "errors", 0, 0, 'e' },
> > - { "zeros", 0, 0, 'z' },
> > { 0 }
> > };
> >
> >
> >
>
> Did you mean to break --errors?
I dont think so.
These options are already handled, unless I am wrong.
Do you want a separate patch for this trivial fix ?
^ permalink raw reply
* Re: [PATCH] Fix deadlock between boomerang_interrupt and boomerang_start_tx in 3c59x
From: Neil Horman @ 2010-08-23 20:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1282593694.2378.230.camel@edumazet-laptop>
On Mon, Aug 23, 2010 at 10:01:34PM +0200, Eric Dumazet wrote:
> Le mercredi 11 août 2010 à 11:12 -0400, Neil Horman a écrit :
> > If netconsole is in use, there is a possibility for deadlock in 3c59x between
> > boomerang_interrupt and boomerang_start_xmit. Both routines take the vp->lock,
> > and if netconsole is in use, a pr_* call from the boomerang_interrupt routine
> > will result in the netconsole code attempting to trnasmit an skb, which can try
> > to take the same spin lock, resulting in deadlock.
> >
> > The fix is pretty straightforward. This patch allocats a bit in the 3c59x
> > private structure to indicate that its handling an interrupt. If we get into
> > the transmit routine and that bit is set, we can be sure that we have recursed
> > and will deadlock if we continue, so instead we just return NETDEV_TX_BUSY, so
> > the stack requeues the skb to try again later.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > ---
> > drivers/net/3c59x.c | 15 ++++++++++++++-
> > 1 files changed, 14 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
> > index c754d88..c685a55 100644
> > --- a/drivers/net/3c59x.c
> > +++ b/drivers/net/3c59x.c
> > @@ -633,7 +633,8 @@ struct vortex_private {
> > open:1,
> > medialock:1,
> > must_free_region:1, /* Flag: if zero, Cardbus owns the I/O region */
> > - large_frames:1; /* accept large frames */
> > + large_frames:1, /* accept large frames */
> > + handling_irq:1; /* private in_irq indicator */
>
>
> It would be safer and faster to use a dedicated 'int' instead of a
> bitfield.
>
>
>
>
Faster I agree with, although I'm not sure if speed is really a big issue here,
given that this is a ancient (but fairly well used) 10/100 adapter. And we
still have space in the octet that that bitfield is living in, so I figured I'd
use that anyway.
As for safe, I'm not sure I follow you on that point. Is there something
inherently dangerous about using a bitfield in this case?
Neil
^ permalink raw reply
* Re: [PATCH] iproute2: add 64bit support to ifstat
From: Stephen Hemminger @ 2010-08-23 20:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, David Miller, netdev
In-Reply-To: <1282595311.2378.288.camel@edumazet-laptop>
On Mon, 23 Aug 2010 22:28:31 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 23 août 2010 à 13:06 -0700, Stephen Hemminger a écrit :
> > > @@ -563,8 +563,6 @@ static void usage(void)
> > > " -s, --noupdate don;t update history\n"
> > > " -t, --interval=SECS report average over the last SECS\n"
> > > " -V, --version output version information\n"
> > > -" -z, --zeros show entries with zero activity\n"
> > > -" -e, --errors show errors\n"
> > > " -z, --zeros show entries with zero activity\n");
> > >
> > > exit(-1);
> > > @@ -581,8 +579,6 @@ static const struct option longopts[] = {
> > > { "interval", 1, 0, 't' },
> > > { "version", 0, 0, 'V' },
> > > { "zeros", 0, 0, 'z' },
> > > - { "errors", 0, 0, 'e' },
> > > - { "zeros", 0, 0, 'z' },
> > > { 0 }
> > > };
> > >
> > >
> > >
> >
> > Did you mean to break --errors?
>
> I dont think so.
>
> These options are already handled, unless I am wrong.
>
> Do you want a separate patch for this trivial fix ?
I'll tweak your previous patch
--
^ permalink raw reply
* Re: [PATCH 1/1] posix clocks: introduce syscall for clock tuning.
From: john stultz @ 2010-08-23 20:41 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Richard Cochran, linux-api, netdev, linux-kernel
In-Reply-To: <201008231457.26690.arnd@arndb.de>
On Mon, 2010-08-23 at 14:57 +0200, Arnd Bergmann wrote:
> On Monday 23 August 2010, Richard Cochran wrote:
> > A new syscall is introduced that allows tuning of a POSIX clock. The
> > syscall is implemented for four architectures: arm, blackfin, powerpc,
> > and x86.
> >
> > The new syscall, clock_adjtime, takes two parameters, a frequency
> > adjustment in parts per billion, and a pointer to a struct timespec
> > containing the clock offset. If the pointer is NULL, a frequency
> > adjustment is performed. Otherwise, the clock offset is immediately
> > corrected by skipping to the new time value.
>
> It looks well-implemented, and seems to be a reasonable extension
> to the clock API. I'm looking forward to your ptp patches on top
> of this to see how it all fits together.
>
> For new syscalls, it's best to take linux-api on Cc. I also added
> John, since he participated in the discussion.
As I mentioned in the previous mail, I agree the new functionality
(adjusting the time by an offset instantaneously) is useful, but I'd
prefer it be done initially within the existing adjtimex() interface.
Then if the posix-time clock_id multiplexing version of adjtimex is
found to be necessary, the new syscall should be introduced, using the
same API (not all clock_ids need to support all the adjtimex modes, but
the new interface should be sufficient for NTPd to use).
There are some other conceptual issues this new syscall introduces:
1) While clock_adjtimex(CLOCK_REALTIME,...) would be equivalent to
adjtimex(), would clock_adjtimex(CLOCK_MONOTONIC,...) make sense?
Given CLOCK_MONOTONIC and CLOCK_REALTIME are both based off the same
notion of time, but offset from each other, any adjustment to one clock
would be reflected in the other. However, the API would make it seem
like they could be adjusted independently.
2) The same issue in #1 exists for CLOCK_REALTIME/MONOTONIC_COARSE
variants.
3) Freq steering for MONOTONIC_RAW would defeat the purpose of the
clock_id.
4) Does adjustments to CPU_TIME clock_ids make sense?
I'm guessing "no" is the right call to all of the above, but am
interested if others see it differently.
thanks
-john
^ permalink raw reply
* Re: [PATCH] tcp: make TCP quick ACK behavior modifiable
From: Eric Dumazet @ 2010-08-23 20:44 UTC (permalink / raw)
To: Hagen Paul Pfeifer; +Cc: netdev, David S. Miller, Ilpo Järvinen
In-Reply-To: <1282590037-18566-1-git-send-email-hagen@jauu.net>
Le lundi 23 août 2010 à 21:00 +0200, Hagen Paul Pfeifer a écrit :
> The TCP quick ACK mechanism analyze if a connection is interactive or
> not. Per default the quick ACK mechanism is enabled and ACK packets are
> triggered instantly to raise the CWND fast - which is clever for
> bulk data (non-interactive) flows. On the other hand interactive protocols
> like HTTP, SMTP or XMPP will suffer from the quick ACK mechanism
> because one additional packets is generated. A simple heuristic
> detects if a connection is interactive (pingpong) and if so,
> disable the quick ACK. But, the mechanism is not in the ability to
> blindly guess if a connection is interactive, and so it must wait for at
> least one return packet with payload.
>
> For the server side this requires one additional packet because (packet
> number 5 and 6 can be combined):
>
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [S], seq 2854340018, win 5840, options [mss 1460,sackOK,TS val 4382726 ecr 0,nop,wscale 7], length 0
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [S.], seq 719041385, ack 2854340019, win 5792, options [mss 1452,sackOK,TS val 2606891996 ecr 4382726,nop,wscale 7], length 0
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 0
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [P.], seq 1:682, ack 1, win 46, options [nop,nop,TS val 4382730 ecr 2606891996], length 681
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [.], ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 0
> 78.47.222.210.80 > 192.168.1.35.44833: Flags [.], seq 1:1441, ack 682, win 56, options [nop,nop,TS val 2606892002 ecr 4382730], length 1440
> 192.168.1.35.44833 > 78.47.222.210.80: Flags [.], ack 1441, win 69, options [nop,nop,TS val 4382737 ecr 2606892002], length 0
>
> This patch provides a sysctl interface for the administrator to globally
> enable or disable TCP quick ACKs. Short lived protocols like HTTP will
> save a non unimportant portion of packets!
>
> Disable TCP Quick ACK:
> $ echo 0 > /proc/sys/net/ipv4/tcp_quickack
>
> Enable TCP Quick ACK:
> $ echo 1 > /proc/sys/net/ipv4/tcp_quickack
>
> Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
> include/net/inet_connection_sock.h | 3 +++
> net/ipv4/sysctl_net_ipv4.c | 7 +++++++
> net/ipv4/tcp.c | 2 ++
> 3 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index b6d3b55..da2fbaf 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -170,9 +170,12 @@ static inline int inet_csk_ack_scheduled(const struct sock *sk)
> return inet_csk(sk)->icsk_ack.pending & ICSK_ACK_SCHED;
> }
>
> +extern int sysctl_tcp_quickack;
> +
> static inline void inet_csk_delack_init(struct sock *sk)
> {
> memset(&inet_csk(sk)->icsk_ack, 0, sizeof(inet_csk(sk)->icsk_ack));
> + inet_csk(sk)->icsk_ack.pingpong = sysctl_tcp_quickack ? 0 : 1;
What about dccp using this function ?
> }
>
> extern void inet_csk_delete_keepalive_timer(struct sock *sk);
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index d96c1da..8923ca8 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -394,6 +394,13 @@ static struct ctl_table ipv4_table[] = {
> .proc_handler = proc_dointvec
> },
> {
> + .procname = "tcp_quickack",
> + .data = &sysctl_tcp_quickack,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec
> + },
> + {
> .procname = "tcp_mem",
> .data = &sysctl_tcp_mem,
> .maxlen = sizeof(sysctl_tcp_mem),
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 176e11a..5161689 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -279,6 +279,8 @@
>
> int sysctl_tcp_fin_timeout __read_mostly = TCP_FIN_TIMEOUT;
>
> +int sysctl_tcp_quickack __read_mostly = 1;
> +
> struct percpu_counter tcp_orphan_count;
> EXPORT_SYMBOL_GPL(tcp_orphan_count);
>
So here is a new undocumented setting ? hint hint...
I thought setsockopt(TCP_QUICKACK) was already available for this
optimization ?
^ permalink raw reply
* Re: [PATCH] Fix deadlock between boomerang_interrupt and boomerang_start_tx in 3c59x
From: Eric Dumazet @ 2010-08-23 20:48 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev
In-Reply-To: <20100823202452.GC12906@hmsreliant.think-freely.org>
Le lundi 23 août 2010 à 16:24 -0400, Neil Horman a écrit :
>
> Faster I agree with, although I'm not sure if speed is really a big issue here,
> given that this is a ancient (but fairly well used) 10/100 adapter. And we
> still have space in the octet that that bitfield is living in, so I figured I'd
> use that anyway.
>
> As for safe, I'm not sure I follow you on that point. Is there something
> inherently dangerous about using a bitfield in this case?
>
A bitfield is not SMP safe.
Are you sure another cpu is not changing another bit, using a non atomic
RMW sequence, and your bit change is lost ?
Quite tricky to check I suppose, so just add an "int" ;)
^ permalink raw reply
* Re: [patch 08/20] ibmveth: Dont overallocate buffers
From: Robert Jennings @ 2010-08-23 20:49 UTC (permalink / raw)
To: Anton Blanchard; +Cc: brking, santil, netdev
In-Reply-To: <20100823001239.041525926@samba.org>
* Anton Blanchard (anton@samba.org) wrote:
> We were allocating a page, even though we always want 4k. Use kmalloc
> instead of get_zeroed_page.
I get a failure during device open when we have a 4k allocation rather
than a 64k page allocation. This is running on a Power6 LPAR without
AMS.
(drivers/net/ibmveth.c:621 ua:3000000c) ERROR: h_register_logical_lan failed with -4
(drivers/net/ibmveth.c:626 ua:3000000c) ERROR: buffer TCE:0x300 filter TCE:0x2300 rxq desc:0x8000601000004000 MAC:0x763cbdd9d30c
^ 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