* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: YOSHIFUJI Hideaki @ 2010-03-25 14:14 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Patrick McHardy, Shan Wei, YOSHIFUJI Hideaki, David Miller,
Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
netfilter-devel, YOSHIFUJI Hideaki
In-Reply-To: <alpine.DEB.2.00.1003251004200.21825@blackhole.kfki.hu>
(2010/03/25 18:23), Jozsef Kadlecsik wrote:
>> First of all. in "sane" setup, people should configure according
>> to their own requirements. They may or may not want send back
>> icmp packet. And, even if the core is to send icmp back, the
>> state would be correctly assigned.
>
> I meant the state of the fragmented packets. If we let the uncompleted
> fragments to enter conntrack, as far as I see their state will be INVALID.
> Or should we add an exception and set their state to UNTRACKED in
> conntrack?
Got it. INVALID seems fine to me so far
while further consideration might be needed.
>> For my conclusion, first option is just to drop
>> uncompleted fragments as we do today. Second option
>> would be to forward them to the next process so that
>> core code could send ICMPv6 etc. or, we could have
>> new code to send ICMPV6_TIME_EXCEED in REJECT target.
>> In longer term, I think it is better to introduce
>> per-exthdr hooks.
>
> I agree with your conclusion too, except a few question.
>
> It is unclear for me how can you forward the packets to the next process:
> above you pointed out that in defrag/reassembly before conntrack we do not
> know yet whether the packets are destined to the host or not. So again,
> how would you let through the fragments on conntrack then?
>
> I don't know how could the REJECT target help in any way.
Argh, more explanation.
If you unconditionally send ICMPv6, behavior would be
broken.
If you do really want to send ICMP in netfilter,
you could pretend additional rules in filter table.
For example, ICMP to be sent back only if other exthdrs
do not exist, and other packets to be silently dropped.
This cannot be our clean/final/full answer, so I said it'd
be better to introduce per-exthdr hooks in longer term.
Regards,
--yoshfuji
^ permalink raw reply
* Re: [PATCH] NETLABEL: Fix an RCU warning
From: Paul Moore @ 2010-03-25 14:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Howells, netdev, Paul E. McKenney
In-Reply-To: <1269523940.3626.37.camel@edumazet-laptop>
On Thursday 25 March 2010 09:32:20 am Eric Dumazet wrote:
> Le jeudi 25 mars 2010 à 11:37 +0000, David Howells a écrit :
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Sorry this is not the right fix.
> > >
> > > Fix is to change the dereference check to take into account the lock
> > > owned here.
> >
> > Then the comments on netlbl_unlhsh_hash(), netlbl_unlhsh_search_iface(),
> > netlbl_unlhsh_search_iface_def() and netlbl_unlhsh_add_iface() are all
> > wrong,
> >
> > for all of them say:
> > * The caller is responsible for calling the rcu_read_[un]lock()
> > * functions.
> >
> > Furthermore, netlabel_unlhsh_add() and netlabel_unlhsh_remove() _do_ wrap
> > the calls to those functions in rcu_read_lock'd sections.
>
> Current code is probably fine.
> Comments are not up to date (as many other comments BTW)
>
> Only the dereference check is bad, as it assumes the rcu_read lock is
> held.
>
> Its not the case, we own a spinlock.
>
> You suggest adding a surrounding rcu lock, but this surrounding lock
> adds overhead on normal kernels, to correct checker warnings only.
The rcu_dereference call, and the outdated comments for that matter, are
likely artifacts from an early patch. When the code was originally being
developed I had assumed that you _always_ needed to hold the RCU read lock
when doing RCU operations, regardless of if you were already holding the
associated spinlock. Thankfully, Paul McKenney was able to review the code
and helped me to understand RCU a bit better, unfortunately, it looks like I
still missed a few spots :/
Give me a day or two to go through the comments again and do some sanity
checks and I'll send out a patch. However, if you've already got something
that is tested and ready to go don't let me stand in your way.
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-25 13:48 UTC (permalink / raw)
To: Eric Dumazet
Cc: Sergey Senozhatsky, Oleg Nesterov, David Miller, Francois Romieu,
netdev
In-Reply-To: <1269523180.3626.25.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 532 bytes --]
On (03/25/10 14:19), Eric Dumazet wrote:
> > Eric, I think you can put Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > on the cumulative patch as I don't see any errors with pktgen over LAN testing.
> >
>
> Hi
>
> Thanks for the report. Problem is I am not confident enough on this
> driver.
> I believe François Romieu is the one that can possibly push a patch ?
>
Thanks.
Hm. François' patch introduced new 'problems'/'stack traces' with pktgen testing...
François, any comments?
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH] NETLABEL: Fix an RCU warning
From: Eric Dumazet @ 2010-03-25 13:32 UTC (permalink / raw)
To: David Howells; +Cc: paul.moore, netdev, Paul E. McKenney
In-Reply-To: <6522.1269517044@redhat.com>
Le jeudi 25 mars 2010 à 11:37 +0000, David Howells a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > Sorry this is not the right fix.
> >
> > Fix is to change the dereference check to take into account the lock
> > owned here.
>
> Then the comments on netlbl_unlhsh_hash(), netlbl_unlhsh_search_iface(),
> netlbl_unlhsh_search_iface_def() and netlbl_unlhsh_add_iface() are all wrong,
> for all of them say:
>
> * The caller is responsible for calling the rcu_read_[un]lock()
> * functions.
>
> Furthermore, netlabel_unlhsh_add() and netlabel_unlhsh_remove() _do_ wrap the
> calls to those functions in rcu_read_lock'd sections.
Current code is probably fine.
Comments are not up to date (as many other comments BTW)
Only the dereference check is bad, as it assumes the rcu_read lock is
held.
Its not the case, we own a spinlock.
You suggest adding a surrounding rcu lock, but this surrounding lock
adds overhead on normal kernels, to correct checker warnings only.
If a mutex was protecting existing code, instead of a spinlock, then
adding rcu_read_lock() would be no correct anyway (existing code would
not be allowed to call a might_sleep function)
Please take a look at rcu_dereference_check() in :
__sk_free() (file net/core/sock.c)
__in6_dev_get() (file include/net/addrconf.h)
rcu_dereference_check_fdtable (file include/linux/fdtable.h)
task_subsys_state() (file include/linux/cgroup.h)
rcu_dereference_check_sched_domain (file kernel/sched.c)
...
for examples of proper checks.
Yes, its more difficult, but its the right thing.
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Eric Dumazet @ 2010-03-25 13:19 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <20100325113038.GA3471@swordfish.minsk.epam.com>
Le jeudi 25 mars 2010 à 13:30 +0200, Sergey Senozhatsky a écrit :
> Hello,
>
> On (03/16/10 19:48), Eric Dumazet wrote:
> [..]
> > OK thanks for the report, this rtl8169_reset_task() seems pretty buggy,
> > or multiple invocation...
> >
> > Did you tried removing the rtl8169_schedule_work() call from
> > rtl8169_rx_interrupt() ?
> >
> > Maybe the reset is not necessary at all in case of fifo overflow..
> >
> > Cumulative patch :
> >
> [..]
>
> Eric, I think you can put Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> on the cumulative patch as I don't see any errors with pktgen over LAN testing.
>
Hi
Thanks for the report. Problem is I am not confident enough on this
driver.
I believe François Romieu is the one that can possibly push a patch ?
Thanks
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Restart rt_intern_hash after emergency rebuild (v2)
From: Neil Horman @ 2010-03-25 13:17 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Eric Dumazet, Linux Netdev List
In-Reply-To: <4BAB15FA.20108@openvz.org>
On Thu, Mar 25, 2010 at 10:51:22AM +0300, Pavel Emelyanov wrote:
> The the rebuild changes the genid which in turn is used at
> the hash calculation. Thus if we don't restart and go on with
> inserting the rt will happen in wrong chain.
>
> (Fixed Neil's comment about the index passed into the rt_intern_hash)
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Thanks! I think this is pretty reasonable.
Reviewed-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: debugging kernel during packet drops
From: Jan Engelhardt @ 2010-03-25 12:42 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, Jorrit Kronjee, netfilter-devel, netdev
In-Reply-To: <4BAB3C8B.3030104@trash.net>
On Thursday 2010-03-25 11:35, Patrick McHardy wrote:
>Eric Dumazet wrote:
>> Here is patch I cooked for xt_hashlimit (on top of net-next-2.6) to make
>> it use RCU and scale better in your case (allowing several concurrent
>> cpus once RPS is activated), but also on more general cases.
>>
>> [PATCH] xt_hashlimit: RCU conversion
>
>This clashes with some recent cleanups in nf-next-2.6.git. I'm
>also expecting a patch from Jan to remove the old v0 revision
>very soon (probably today).
I was waiting for you to process
http://marc.info/?l=netfilter-devel&m=126936212932683&w=2
which is a prerequisite for the hashlimit v0 removal.
(I expected that you might miss some mails due to trash.net
having been down yesterday. No biggie.)
^ permalink raw reply
* Re: [PATCH] NETLABEL: Fix an RCU warning
From: David Howells @ 2010-03-25 11:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: dhowells, paul.moore, netdev
In-Reply-To: <1269516484.3626.21.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Sorry this is not the right fix.
>
> Fix is to change the dereference check to take into account the lock
> owned here.
Then the comments on netlbl_unlhsh_hash(), netlbl_unlhsh_search_iface(),
netlbl_unlhsh_search_iface_def() and netlbl_unlhsh_add_iface() are all wrong,
for all of them say:
* The caller is responsible for calling the rcu_read_[un]lock()
* functions.
Furthermore, netlabel_unlhsh_add() and netlabel_unlhsh_remove() _do_ wrap the
calls to those functions in rcu_read_lock'd sections.
David
^ permalink raw reply
* Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
From: Sergey Senozhatsky @ 2010-03-25 11:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Oleg Nesterov, David Miller, Francois Romieu, netdev
In-Reply-To: <1268765284.2932.17.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 561 bytes --]
Hello,
On (03/16/10 19:48), Eric Dumazet wrote:
[..]
> OK thanks for the report, this rtl8169_reset_task() seems pretty buggy,
> or multiple invocation...
>
> Did you tried removing the rtl8169_schedule_work() call from
> rtl8169_rx_interrupt() ?
>
> Maybe the reset is not necessary at all in case of fifo overflow..
>
> Cumulative patch :
>
[..]
Eric, I think you can put Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
on the cumulative patch as I don't see any errors with pktgen over LAN testing.
Thanks.
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: [PATCH] NETLABEL: Fix an RCU warning
From: Eric Dumazet @ 2010-03-25 11:28 UTC (permalink / raw)
To: David Howells; +Cc: paul.moore, netdev
In-Reply-To: <20100325110621.5348.32020.stgit@warthog.procyon.org.uk>
Le jeudi 25 mars 2010 à 11:06 +0000, David Howells a écrit :
> Fix an RCU warning in the netlabel code due to missing rcu read locking around
> an rcu_dereference() in netlbl_unlhsh_hash() when called from
> netlbl_unlhsh_netdev_handler():
>
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> net/netlabel/netlabel_unlabeled.c:246 invoked rcu_dereference_check() without protection!
>
> other info that might help us debug this:
>
>
> rcu_scheduler_active = 1, debug_locks = 0
> 2 locks held by ip/5108:
> #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff812c4a36>] rtnl_lock+0x12/0x14
> #1: (netlbl_unlhsh_lock){+.+...}, at: [<ffffffff8134daa4>] netlbl_unlhsh_netdev_handler+0x1e/0x86
>
> stack backtrace:
> Pid: 5108, comm: ip Not tainted 2.6.34-rc2-cachefs #114
> Call Trace:
> [<ffffffff8105121f>] lockdep_rcu_dereference+0xaa/0xb2
> [<ffffffff8134d781>] netlbl_unlhsh_hash+0x3e/0x50
> [<ffffffff8134d7a1>] netlbl_unlhsh_search_iface+0xe/0x84
> [<ffffffff8134daaf>] netlbl_unlhsh_netdev_handler+0x29/0x86
> [<ffffffff81048362>] notifier_call_chain+0x32/0x5e
> [<ffffffff810483fe>] raw_notifier_call_chain+0xf/0x11
> [<ffffffff812ba924>] call_netdevice_notifiers+0x16/0x18
> [<ffffffff812bac22>] __dev_notify_flags+0x37/0x5b
> [<ffffffff812bac8c>] dev_change_flags+0x46/0x52
> [<ffffffff812c41af>] do_setlink+0x250/0x3cd
> [<ffffffff812c4ee8>] rtnl_newlink+0x2b6/0x49d
> [<ffffffff812c4cdd>] ? rtnl_newlink+0xab/0x49d
> [<ffffffff812c4c17>] rtnetlink_rcv_msg+0x1b7/0x1d2
> [<ffffffff812c4a60>] ? rtnetlink_rcv_msg+0x0/0x1d2
> [<ffffffff812cc1dc>] netlink_rcv_skb+0x3e/0x8f
> [<ffffffff812c4a59>] rtnetlink_rcv+0x21/0x28
> [<ffffffff812cbefe>] netlink_unicast+0x218/0x28f
> [<ffffffff812cc76e>] netlink_sendmsg+0x26b/0x27a
> [<ffffffff812a9f1d>] sock_sendmsg+0xd4/0xf5
> [<ffffffff81096dca>] ? might_fault+0x4e/0x9e
> [<ffffffff81096dca>] ? might_fault+0x4e/0x9e
> [<ffffffff81096e13>] ? might_fault+0x97/0x9e
> [<ffffffff81096dca>] ? might_fault+0x4e/0x9e
> [<ffffffff812b4122>] ? verify_iovec+0x59/0x97
> [<ffffffff812aa1d9>] sys_sendmsg+0x209/0x273
> [<ffffffff810976dc>] ? __do_fault+0x395/0x3cd
> [<ffffffff8109928f>] ? handle_mm_fault+0x324/0x69d
> [<ffffffff81051e4e>] ? trace_hardirqs_on_caller+0x10c/0x130
> [<ffffffff81074972>] ? audit_syscall_entry+0x17d/0x1b0
> [<ffffffff81364154>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> net/netlabel/netlabel_unlabeled.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 852d9d7..7ea64e4 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -799,6 +799,7 @@ static int netlbl_unlhsh_netdev_handler(struct notifier_block *this,
>
> /* XXX - should this be a check for NETDEV_DOWN or _UNREGISTER? */
> if (event == NETDEV_DOWN) {
> + rcu_read_lock();
> spin_lock(&netlbl_unlhsh_lock);
> iface = netlbl_unlhsh_search_iface(dev->ifindex);
> if (iface != NULL && iface->valid) {
> @@ -807,6 +808,7 @@ static int netlbl_unlhsh_netdev_handler(struct notifier_block *this,
> } else
> iface = NULL;
> spin_unlock(&netlbl_unlhsh_lock);
> + rcu_read_unlock();
> }
>
> if (iface != NULL)
>
> --
Sorry this is not the right fix.
Fix is to change the dereference check to take into account the lock
owned here.
^ permalink raw reply
* [PATCH] NETLABEL: Fix an RCU warning
From: David Howells @ 2010-03-25 11:06 UTC (permalink / raw)
To: paul.moore; +Cc: netdev, David Howells
Fix an RCU warning in the netlabel code due to missing rcu read locking around
an rcu_dereference() in netlbl_unlhsh_hash() when called from
netlbl_unlhsh_netdev_handler():
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
net/netlabel/netlabel_unlabeled.c:246 invoked rcu_dereference_check() without protection!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
2 locks held by ip/5108:
#0: (rtnl_mutex){+.+.+.}, at: [<ffffffff812c4a36>] rtnl_lock+0x12/0x14
#1: (netlbl_unlhsh_lock){+.+...}, at: [<ffffffff8134daa4>] netlbl_unlhsh_netdev_handler+0x1e/0x86
stack backtrace:
Pid: 5108, comm: ip Not tainted 2.6.34-rc2-cachefs #114
Call Trace:
[<ffffffff8105121f>] lockdep_rcu_dereference+0xaa/0xb2
[<ffffffff8134d781>] netlbl_unlhsh_hash+0x3e/0x50
[<ffffffff8134d7a1>] netlbl_unlhsh_search_iface+0xe/0x84
[<ffffffff8134daaf>] netlbl_unlhsh_netdev_handler+0x29/0x86
[<ffffffff81048362>] notifier_call_chain+0x32/0x5e
[<ffffffff810483fe>] raw_notifier_call_chain+0xf/0x11
[<ffffffff812ba924>] call_netdevice_notifiers+0x16/0x18
[<ffffffff812bac22>] __dev_notify_flags+0x37/0x5b
[<ffffffff812bac8c>] dev_change_flags+0x46/0x52
[<ffffffff812c41af>] do_setlink+0x250/0x3cd
[<ffffffff812c4ee8>] rtnl_newlink+0x2b6/0x49d
[<ffffffff812c4cdd>] ? rtnl_newlink+0xab/0x49d
[<ffffffff812c4c17>] rtnetlink_rcv_msg+0x1b7/0x1d2
[<ffffffff812c4a60>] ? rtnetlink_rcv_msg+0x0/0x1d2
[<ffffffff812cc1dc>] netlink_rcv_skb+0x3e/0x8f
[<ffffffff812c4a59>] rtnetlink_rcv+0x21/0x28
[<ffffffff812cbefe>] netlink_unicast+0x218/0x28f
[<ffffffff812cc76e>] netlink_sendmsg+0x26b/0x27a
[<ffffffff812a9f1d>] sock_sendmsg+0xd4/0xf5
[<ffffffff81096dca>] ? might_fault+0x4e/0x9e
[<ffffffff81096dca>] ? might_fault+0x4e/0x9e
[<ffffffff81096e13>] ? might_fault+0x97/0x9e
[<ffffffff81096dca>] ? might_fault+0x4e/0x9e
[<ffffffff812b4122>] ? verify_iovec+0x59/0x97
[<ffffffff812aa1d9>] sys_sendmsg+0x209/0x273
[<ffffffff810976dc>] ? __do_fault+0x395/0x3cd
[<ffffffff8109928f>] ? handle_mm_fault+0x324/0x69d
[<ffffffff81051e4e>] ? trace_hardirqs_on_caller+0x10c/0x130
[<ffffffff81074972>] ? audit_syscall_entry+0x17d/0x1b0
[<ffffffff81364154>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/netlabel/netlabel_unlabeled.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 852d9d7..7ea64e4 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -799,6 +799,7 @@ static int netlbl_unlhsh_netdev_handler(struct notifier_block *this,
/* XXX - should this be a check for NETDEV_DOWN or _UNREGISTER? */
if (event == NETDEV_DOWN) {
+ rcu_read_lock();
spin_lock(&netlbl_unlhsh_lock);
iface = netlbl_unlhsh_search_iface(dev->ifindex);
if (iface != NULL && iface->valid) {
@@ -807,6 +808,7 @@ static int netlbl_unlhsh_netdev_handler(struct notifier_block *this,
} else
iface = NULL;
spin_unlock(&netlbl_unlhsh_lock);
+ rcu_read_unlock();
}
if (iface != NULL)
^ permalink raw reply related
* Re: debugging kernel during packet drops
From: Eric Dumazet @ 2010-03-25 11:02 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jorrit Kronjee, netfilter-devel, netdev
In-Reply-To: <4BAB3C8B.3030104@trash.net>
Le jeudi 25 mars 2010 à 11:35 +0100, Patrick McHardy a écrit :
> Eric Dumazet wrote:
> > Here is patch I cooked for xt_hashlimit (on top of net-next-2.6) to make
> > it use RCU and scale better in your case (allowing several concurrent
> > cpus once RPS is activated), but also on more general cases.
> >
> > [PATCH] xt_hashlimit: RCU conversion
> >
> > xt_hashlimit uses a central lock per hash table and suffers from
> > contention on some workloads.
> >
> > After RCU conversion, central lock is only used when a writer wants to
> > add or delete an entry. For 'readers', updating an existing entry, they
> > use an individual lock per entry.
>
> This clashes with some recent cleanups in nf-next-2.6.git. I'm
> also expecting a patch from Jan to remove the old v0 revision
> very soon (probably today). Please rediff once I've pushed that out.
Sure, this patch was mainly for Jorrit tests (and he uses net-next-2.6),
I will submit several patches for mainline :)
Thanks
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: debugging kernel during packet drops
From: Patrick McHardy @ 2010-03-25 10:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jorrit Kronjee, netfilter-devel, netdev
In-Reply-To: <1269509574.3626.9.camel@edumazet-laptop>
Eric Dumazet wrote:
> Here is patch I cooked for xt_hashlimit (on top of net-next-2.6) to make
> it use RCU and scale better in your case (allowing several concurrent
> cpus once RPS is activated), but also on more general cases.
>
> [PATCH] xt_hashlimit: RCU conversion
>
> xt_hashlimit uses a central lock per hash table and suffers from
> contention on some workloads.
>
> After RCU conversion, central lock is only used when a writer wants to
> add or delete an entry. For 'readers', updating an existing entry, they
> use an individual lock per entry.
This clashes with some recent cleanups in nf-next-2.6.git. I'm
also expecting a patch from Jan to remove the old v0 revision
very soon (probably today). Please rediff once I've pushed that out.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Patrick McHardy @ 2010-03-25 10:25 UTC (permalink / raw)
To: YOSHIFUJI Hideaki
Cc: Jozsef Kadlecsik, Shan Wei, YOSHIFUJI Hideaki, David Miller,
Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
netfilter-devel
In-Reply-To: <4BAAE496.8000701@linux-ipv6.org>
YOSHIFUJI Hideaki wrote:
> (2010/03/24 5:10), Jozsef Kadlecsik wrote:
>
>> On Wed, 24 Mar 2010, YOSHIFUJI Hideaki wrote:
>>
>>>> In this case without conntrack, IPv6 would send an ICMPv6 message,
>>>> so in my opinion the transparent thing to do would be to still send
>>>> them. Of course only if reassembly is done on an end host.
>>>
>>> Well, no. conntrack should just forward even uncompleted fragments
>>> to next process (e.g. core ipv6 code), and then the core would send
>>> ICMP error back. ICMP should be sent by the core ipv6 code according
>>> to decision of itself, not according to netfilter.
>>
>> But what state could be associated by conntrack to the uncompleted
>> fragments but the INVALID state? In consequence, in any sane setup, the
>> uncompleted fragments will be dropped silently by a filter table rule
>> and no ICMP error message will be sent back.
>>
>> Therefore I think iff the destination of the fragments is the host
>> itself, then conntrack should generate an ICMP error message. But that
>> error message must be processed by conntrack to set its state and then
>> the fate of the generated packet can be decided by a rule.
>
> Well.... no.
>
> First of all. in "sane" setup, people should configure according
> to their own requirements. They may or may not want send back
> icmp packet. And, even if the core is to send icmp back, the
> state would be correctly assigned.
>
> We cannot (and should not) do something "cleaver" (excluding
> packet drops) in conntrack in PRE_ROUTING chain.
>
> One reason is that in PRE_ROUTING context, we can NOT determine
> if the address we see in the IP header is really the final
> destination. The overall situation is the same even if the
> routing entry corresponding the "current" destination points
> the node itself, or even if the node is configured as host.
Agreed, that is a problem.
> It might seems that we could do something in the "filter"
> table in LOCAL_IN, FORWARD or LOCAL_OUT (after routing header
> process).
>
> But well, we unfortunately cannot do this (at least
> automatically) because even in LOCAL_IN, the final
> destination has not been decided until we process all
> of extension headers.
>
> Sending ICMP in netfilter (especially in conntrack) is too
> patchy, and is not right. If we do the right thing (and
> I believe we should do so), I'd propose to have hooks
> around handlers inside ip6_input_finish().
>
> ...I remember that I was thinking about this before.
>
> For my conclusion, first option is just to drop
> uncompleted fragments as we do today. Second option
> would be to forward them to the next process so that
> core code could send ICMPv6 etc. or, we could have
> new code to send ICMPV6_TIME_EXCEED in REJECT target.
> In longer term, I think it is better to introduce
> per-exthdr hooks.
We'd need something that allows to process the incomplete fragments
long enough so they can actually reach the IPv6 core (people usually
don't allow incoming fragments when using conntrack).
But for now you've convinced me that this patch is wrong.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Patrick McHardy @ 2010-03-25 10:20 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Shan Wei, Pascal Hambourg, YOSHIFUJI Hideaki, David Miller,
Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
netfilter-devel
In-Reply-To: <alpine.DEB.2.00.1003251028580.21825@blackhole.kfki.hu>
[-- Attachment #1: Type: text/plain, Size: 719 bytes --]
Jozsef Kadlecsik wrote:
> Why the order of the raw table and defragmentation is reversed for IPv6?
>
> That makes impossible to use the NOTRACK target in IPv6: for example if
> someone enters
>
> ip6tables -t raw -A PREROUTING -p tcp --dport 80 -j NOTRACK
>
> and if we receive fragmented packets then the first fragment will be
> untracked and thus skip nf_ct_frag6_gather (and conntrack), while all
> subsequent fragments enter nf_ct_frag6_gather and reassembly will never
> successfully be finished.
>
> IMHO this is a bug and should be fixed. Patrick, please consider applying
> the patch below.
Indeed. I've applied your patch with a minor fixup (attached) to
apply cleanly to the current tree, thanks.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1657 bytes --]
commit 9c13886665c43600bd0af4b38e33c654e648e078
Author: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Thu Mar 25 11:17:26 2010 +0100
netfilter: ip6table_raw: fix table priority
The order of the IPv6 raw table is currently reversed, that makes impossible
to use the NOTRACK target in IPv6: for example if someone enters
ip6tables -t raw -A PREROUTING -p tcp --dport 80 -j NOTRACK
and if we receive fragmented packets then the first fragment will be
untracked and thus skip nf_ct_frag6_gather (and conntrack), while all
subsequent fragments enter nf_ct_frag6_gather and reassembly will never
successfully be finished.
Singed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
index d654873..1f7e300 100644
--- a/include/linux/netfilter_ipv6.h
+++ b/include/linux/netfilter_ipv6.h
@@ -59,6 +59,7 @@
enum nf_ip6_hook_priorities {
NF_IP6_PRI_FIRST = INT_MIN,
NF_IP6_PRI_CONNTRACK_DEFRAG = -400,
+ NF_IP6_PRI_RAW = -300,
NF_IP6_PRI_SELINUX_FIRST = -225,
NF_IP6_PRI_CONNTRACK = -200,
NF_IP6_PRI_MANGLE = -150,
diff --git a/net/ipv6/netfilter/ip6table_raw.c b/net/ipv6/netfilter/ip6table_raw.c
index aef31a2..b9cf7cd 100644
--- a/net/ipv6/netfilter/ip6table_raw.c
+++ b/net/ipv6/netfilter/ip6table_raw.c
@@ -13,7 +13,7 @@ static const struct xt_table packet_raw = {
.valid_hooks = RAW_VALID_HOOKS,
.me = THIS_MODULE,
.af = NFPROTO_IPV6,
- .priority = NF_IP6_PRI_FIRST,
+ .priority = NF_IP6_PRI_RAW,
};
/* The work comes in here from netfilter.c. */
^ permalink raw reply related
* Re: [PATCH] xt_hashlimit: dl_seq_stop() fix
From: Eric Dumazet @ 2010-03-25 10:07 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Netfilter Development Mailinglist
In-Reply-To: <4BAB3494.4030402@trash.net>
Le jeudi 25 mars 2010 à 11:01 +0100, Patrick McHardy a écrit :
> Eric Dumazet wrote:
> > If dl_seq_start() memory allocation fails, we crash later in
> > dl_seq_stop(), trying to kfree(ERR_PTR(-ENOMEM))
>
> Seems pretty counter intuitive to me that ->stop is invoked after
> ->start returned an error.
Yes, I feel your pain :)
At least ->next is not invoked :)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Jozsef Kadlecsik @ 2010-03-25 10:07 UTC (permalink / raw)
To: Shan Wei
Cc: Pascal Hambourg, YOSHIFUJI Hideaki, Patrick McHardy, David Miller,
Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
netfilter-devel
In-Reply-To: <4BAB2954.9010608@cn.fujitsu.com>
On Thu, 25 Mar 2010, Shan Wei wrote:
> Pascal Hambourg wrote, at 03/25/2010 04:38 PM:
> >
> > Jozsef Kadlecsik a ?crit :
> >> On Wed, 24 Mar 2010, YOSHIFUJI Hideaki wrote:
> >>
> >>>> In this case without conntrack, IPv6 would send an ICMPv6 message,
> >>>> so in my opinion the transparent thing to do would be to still send
> >>>> them. Of course only if reassembly is done on an end host.
> >>> Well, no. conntrack should just forward even uncompleted fragments
> >>> to next process (e.g. core ipv6 code), and then the core would send
> >>> ICMP error back. ICMP should be sent by the core ipv6 code according
> >>> to decision of itself, not according to netfilter.
> >> But what state could be associated by conntrack to the uncompleted
> >> fragments but the INVALID state? In consequence, in any sane setup, the
> >> uncompleted fragments will be dropped silently by a filter table rule
> >> and no ICMP error message will be sent back.
> >
> > AFAIK, in the IPv4 stack the reassembly takes place before the INPUT
> > chains (NF_IP_LOCAL_IN hook). Is it different in the IPv6 stack ?
>
> Yes, they are different.
>
> In IPv4 stack?for an end host, ip_local_deliver() reassemble
> fragments before LOCAL_IN hook .
>
> But in IPv6 stack, ip6_input_finish() handles fragment extension headers
> and try to reassemble them *after* LOCAL_IN hook.
But we are discussing netfilter and (de)fragmentation: what should happen
when the packet reassembly in netfilter times out and the destination is
the host itself.
In IPv4 the very first subsystem is ipv4_conntrack_defrag, called from
NF_INET_PRE_ROUTING. Then comes the raw table and after that conntrack.
In IPv6 the very first is the raw table, then comes ipv6_defrag and then
conntrack.
Why the order of the raw table and defragmentation is reversed for IPv6?
That makes impossible to use the NOTRACK target in IPv6: for example if
someone enters
ip6tables -t raw -A PREROUTING -p tcp --dport 80 -j NOTRACK
and if we receive fragmented packets then the first fragment will be
untracked and thus skip nf_ct_frag6_gather (and conntrack), while all
subsequent fragments enter nf_ct_frag6_gather and reassembly will never
successfully be finished.
IMHO this is a bug and should be fixed. Patrick, please consider applying
the patch below.
Singed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
index d654873..1f7e300 100644
--- a/include/linux/netfilter_ipv6.h
+++ b/include/linux/netfilter_ipv6.h
@@ -59,6 +59,7 @@
enum nf_ip6_hook_priorities {
NF_IP6_PRI_FIRST = INT_MIN,
NF_IP6_PRI_CONNTRACK_DEFRAG = -400,
+ NF_IP6_PRI_RAW = -300,
NF_IP6_PRI_SELINUX_FIRST = -225,
NF_IP6_PRI_CONNTRACK = -200,
NF_IP6_PRI_MANGLE = -150,
diff --git a/net/ipv6/netfilter/ip6table_raw.c b/net/ipv6/netfilter/ip6table_raw.c
index ed1a118..3d8c6f0 100644
--- a/net/ipv6/netfilter/ip6table_raw.c
+++ b/net/ipv6/netfilter/ip6table_raw.c
@@ -70,14 +70,14 @@ static struct nf_hook_ops ip6t_ops[] __read_mostly = {
.hook = ip6t_pre_routing_hook,
.pf = NFPROTO_IPV6,
.hooknum = NF_INET_PRE_ROUTING,
- .priority = NF_IP6_PRI_FIRST,
+ .priority = NF_IP6_PRI_RAW,
.owner = THIS_MODULE,
},
{
.hook = ip6t_local_out_hook,
.pf = NFPROTO_IPV6,
.hooknum = NF_INET_LOCAL_OUT,
- .priority = NF_IP6_PRI_FIRST,
+ .priority = NF_IP6_PRI_RAW,
.owner = THIS_MODULE,
},
};
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply related
* Re: [PATCH] xt_hashlimit: dl_seq_stop() fix
From: Patrick McHardy @ 2010-03-25 10:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Netfilter Development Mailinglist
In-Reply-To: <1269463664.2849.9.camel@edumazet-laptop>
Eric Dumazet wrote:
> If dl_seq_start() memory allocation fails, we crash later in
> dl_seq_stop(), trying to kfree(ERR_PTR(-ENOMEM))
Seems pretty counter intuitive to me that ->stop is invoked after
->start returned an error.
Applied, thanks.
^ permalink raw reply
* Re: why does the tcp_time_wait() call tcp_done(sk)?
From: Shan Wei @ 2010-03-25 10:01 UTC (permalink / raw)
To: 杨硕; +Cc: netdev
In-Reply-To: <ffe582101003242024o23ddbdc9w7aaffde382c3da8b@mail.gmail.com>
杨硕 wrote, at 03/25/2010 11:24 AM:
> that means the connection has been closed?
> Does kernel think like that???
> we can use "STATE_TRACE" to see FIN_WAIT_2--->CLOSE rather than
> FIN_WAIT_2-->TIME_WAIT-->CLOSE.
There are three functions to call tcp_time_wait() with FIN_WAIT_2.
Can you provide the fully function call stack?
tcp_close tcp_rcv_state_process
\ |
\ |
\ |
-|/ \|/
tcp_time_wait
/|\
|
|
tcp_keepalive_timer
--
Best Regards
-----
Shan Wei
^ permalink raw reply
* Re: debugging kernel during packet drops
From: Eric Dumazet @ 2010-03-25 9:32 UTC (permalink / raw)
To: Jorrit Kronjee; +Cc: netfilter-devel, netdev, Patrick McHardy
In-Reply-To: <1269447674.3213.64.camel@edumazet-laptop>
Le mercredi 24 mars 2010 à 17:22 +0100, Eric Dumazet a écrit :
> Le mercredi 24 mars 2010 à 16:20 +0100, Jorrit Kronjee a écrit :
> >
> > I hope this helps! Is there anything special I need to do to use RPS?
> >
>
> Sure this helps a lot !
>
> You might try RPS by doing :
>
> echo f >/sys/class/net/eth3/queues/rx-0/rps_cpus
>
> (But you'll also need a new xt_hashlimit module to make it more
> scalable, I can work on this this week if necessary)
>
Here is patch I cooked for xt_hashlimit (on top of net-next-2.6) to make
it use RCU and scale better in your case (allowing several concurrent
cpus once RPS is activated), but also on more general cases.
[PATCH] xt_hashlimit: RCU conversion
xt_hashlimit uses a central lock per hash table and suffers from
contention on some workloads.
After RCU conversion, central lock is only used when a writer wants to
add or delete an entry. For 'readers', updating an existing entry, they
use an individual lock per entry.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/netfilter/xt_hashlimit.c | 290 +++++++++++++++++----------------
1 file changed, 151 insertions(+), 139 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 9e9c489..a4f1b69 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -41,6 +41,54 @@ MODULE_DESCRIPTION("Xtables: per hash-bucket rate-limit match");
MODULE_ALIAS("ipt_hashlimit");
MODULE_ALIAS("ip6t_hashlimit");
+/* The algorithm used is the Simple Token Bucket Filter (TBF)
+ * see net/sched/sch_tbf.c in the linux source tree
+ */
+
+/* Rusty: This is my (non-mathematically-inclined) understanding of
+ this algorithm. The `average rate' in jiffies becomes your initial
+ amount of credit `credit' and the most credit you can ever have
+ `credit_cap'. The `peak rate' becomes the cost of passing the
+ test, `cost'.
+
+ `prev' tracks the last packet hit: you gain one credit per jiffy.
+ If you get credit balance more than this, the extra credit is
+ discarded. Every time the match passes, you lose `cost' credits;
+ if you don't have that many, the test fails.
+
+ See Alexey's formal explanation in net/sched/sch_tbf.c.
+
+ To get the maximum range, we multiply by this factor (ie. you get N
+ credits per jiffy). We want to allow a rate as low as 1 per day
+ (slowest userspace tool allows), which means
+ CREDITS_PER_JIFFY*HZ*60*60*24 < 2^32 ie.
+*/
+#define MAX_CPJ (0xFFFFFFFF / (HZ*60*60*24))
+
+/* Repeated shift and or gives us all 1s, final shift and add 1 gives
+ * us the power of 2 below the theoretical max, so GCC simply does a
+ * shift. */
+#define _POW2_BELOW2(x) ((x)|((x)>>1))
+#define _POW2_BELOW4(x) (_POW2_BELOW2(x)|_POW2_BELOW2((x)>>2))
+#define _POW2_BELOW8(x) (_POW2_BELOW4(x)|_POW2_BELOW4((x)>>4))
+#define _POW2_BELOW16(x) (_POW2_BELOW8(x)|_POW2_BELOW8((x)>>8))
+#define _POW2_BELOW32(x) (_POW2_BELOW16(x)|_POW2_BELOW16((x)>>16))
+#define POW2_BELOW32(x) ((_POW2_BELOW32(x)>>1) + 1)
+
+#define CREDITS_PER_JIFFY POW2_BELOW32(MAX_CPJ)
+
+/* Precision saver. */
+static inline u_int32_t
+user2credits(u_int32_t user)
+{
+ /* If multiplying would overflow... */
+ if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
+ /* Divide first. */
+ return (user / XT_HASHLIMIT_SCALE) * HZ * CREDITS_PER_JIFFY;
+
+ return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE;
+}
+
struct hashlimit_net {
struct hlist_head htables;
struct proc_dir_entry *ipt_hashlimit;
@@ -80,12 +128,14 @@ struct dsthash_ent {
struct dsthash_dst dst;
/* modified structure members in the end */
+ spinlock_t lock;
unsigned long expires; /* precalculated expiry time */
struct {
unsigned long prev; /* last modification */
u_int32_t credit;
- u_int32_t credit_cap, cost;
+ u_int32_t cost;
} rateinfo;
+ struct rcu_head rcu;
};
struct xt_hashlimit_htable {
@@ -95,10 +145,12 @@ struct xt_hashlimit_htable {
bool rnd_initialized;
struct hashlimit_cfg1 cfg; /* config */
+ u_int32_t credit_cap;
/* used internally */
- spinlock_t lock; /* lock for list_head */
u_int32_t rnd; /* random seed for hash */
+
+ spinlock_t hlock; /* lock for writes to hash[]/count/rnd */
unsigned int count; /* number entries in table */
struct timer_list timer; /* timer for gc */
@@ -142,55 +194,27 @@ dsthash_find(const struct xt_hashlimit_htable *ht,
u_int32_t hash = hash_dst(ht, dst);
if (!hlist_empty(&ht->hash[hash])) {
- hlist_for_each_entry(ent, pos, &ht->hash[hash], node)
- if (dst_cmp(ent, dst))
+ hlist_for_each_entry_rcu(ent, pos, &ht->hash[hash], node)
+ if (dst_cmp(ent, dst)) {
+ spin_lock(&ent->lock);
return ent;
+ }
}
return NULL;
}
-/* allocate dsthash_ent, initialize dst, put in htable and lock it */
-static struct dsthash_ent *
-dsthash_alloc_init(struct xt_hashlimit_htable *ht,
- const struct dsthash_dst *dst)
-{
- struct dsthash_ent *ent;
-
- /* initialize hash with random val at the time we allocate
- * the first hashtable entry */
- if (!ht->rnd_initialized) {
- get_random_bytes(&ht->rnd, sizeof(ht->rnd));
- ht->rnd_initialized = true;
- }
-
- if (ht->cfg.max && ht->count >= ht->cfg.max) {
- /* FIXME: do something. question is what.. */
- if (net_ratelimit())
- printk(KERN_WARNING
- "xt_hashlimit: max count of %u reached\n",
- ht->cfg.max);
- return NULL;
- }
- ent = kmem_cache_alloc(hashlimit_cachep, GFP_ATOMIC);
- if (!ent) {
- if (net_ratelimit())
- printk(KERN_ERR
- "xt_hashlimit: can't allocate dsthash_ent\n");
- return NULL;
- }
- memcpy(&ent->dst, dst, sizeof(ent->dst));
+static void dsthash_free_rcu(struct rcu_head *head)
+{
+ struct dsthash_ent *ent = container_of(head, struct dsthash_ent, rcu);
- hlist_add_head(&ent->node, &ht->hash[hash_dst(ht, dst)]);
- ht->count++;
- return ent;
+ kmem_cache_free(hashlimit_cachep, ent);
}
-static inline void
-dsthash_free(struct xt_hashlimit_htable *ht, struct dsthash_ent *ent)
+static void dsthash_free(struct xt_hashlimit_htable *ht, struct dsthash_ent *ent)
{
- hlist_del(&ent->node);
- kmem_cache_free(hashlimit_cachep, ent);
+ hlist_del_rcu(&ent->node);
+ call_rcu_bh(&ent->rcu, dsthash_free_rcu);
ht->count--;
}
static void htable_gc(unsigned long htlong);
@@ -243,11 +267,12 @@ static int htable_create_v0(struct net *net, struct xt_hashlimit_info *minfo, u_
for (i = 0; i < hinfo->cfg.size; i++)
INIT_HLIST_HEAD(&hinfo->hash[i]);
+ hinfo->credit_cap = user2credits(hinfo->cfg.avg * hinfo->cfg.burst);
hinfo->use = 1;
hinfo->count = 0;
hinfo->family = family;
hinfo->rnd_initialized = false;
- spin_lock_init(&hinfo->lock);
+ spin_lock_init(&hinfo->hlock);
hinfo->pde = proc_create_data(minfo->name, 0,
(family == NFPROTO_IPV4) ?
hashlimit_net->ipt_hashlimit : hashlimit_net->ip6t_hashlimit,
@@ -305,11 +330,12 @@ static int htable_create(struct net *net, struct xt_hashlimit_mtinfo1 *minfo,
for (i = 0; i < hinfo->cfg.size; i++)
INIT_HLIST_HEAD(&hinfo->hash[i]);
+ hinfo->credit_cap = user2credits(hinfo->cfg.avg * hinfo->cfg.burst);
hinfo->use = 1;
hinfo->count = 0;
hinfo->family = family;
hinfo->rnd_initialized = false;
- spin_lock_init(&hinfo->lock);
+ spin_lock_init(&hinfo->hlock);
hinfo->pde = proc_create_data(minfo->name, 0,
(family == NFPROTO_IPV4) ?
@@ -349,7 +375,7 @@ static void htable_selective_cleanup(struct xt_hashlimit_htable *ht,
unsigned int i;
/* lock hash table and iterate over it */
- spin_lock_bh(&ht->lock);
+ spin_lock_bh(&ht->hlock);
for (i = 0; i < ht->cfg.size; i++) {
struct dsthash_ent *dh;
struct hlist_node *pos, *n;
@@ -358,7 +384,7 @@ static void htable_selective_cleanup(struct xt_hashlimit_htable *ht,
dsthash_free(ht, dh);
}
}
- spin_unlock_bh(&ht->lock);
+ spin_unlock_bh(&ht->hlock);
}
/* hash table garbage collector, run by timer */
@@ -417,59 +443,12 @@ static void htable_put(struct xt_hashlimit_htable *hinfo)
mutex_unlock(&hashlimit_mutex);
}
-/* The algorithm used is the Simple Token Bucket Filter (TBF)
- * see net/sched/sch_tbf.c in the linux source tree
- */
-
-/* Rusty: This is my (non-mathematically-inclined) understanding of
- this algorithm. The `average rate' in jiffies becomes your initial
- amount of credit `credit' and the most credit you can ever have
- `credit_cap'. The `peak rate' becomes the cost of passing the
- test, `cost'.
-
- `prev' tracks the last packet hit: you gain one credit per jiffy.
- If you get credit balance more than this, the extra credit is
- discarded. Every time the match passes, you lose `cost' credits;
- if you don't have that many, the test fails.
-
- See Alexey's formal explanation in net/sched/sch_tbf.c.
-
- To get the maximum range, we multiply by this factor (ie. you get N
- credits per jiffy). We want to allow a rate as low as 1 per day
- (slowest userspace tool allows), which means
- CREDITS_PER_JIFFY*HZ*60*60*24 < 2^32 ie.
-*/
-#define MAX_CPJ (0xFFFFFFFF / (HZ*60*60*24))
-
-/* Repeated shift and or gives us all 1s, final shift and add 1 gives
- * us the power of 2 below the theoretical max, so GCC simply does a
- * shift. */
-#define _POW2_BELOW2(x) ((x)|((x)>>1))
-#define _POW2_BELOW4(x) (_POW2_BELOW2(x)|_POW2_BELOW2((x)>>2))
-#define _POW2_BELOW8(x) (_POW2_BELOW4(x)|_POW2_BELOW4((x)>>4))
-#define _POW2_BELOW16(x) (_POW2_BELOW8(x)|_POW2_BELOW8((x)>>8))
-#define _POW2_BELOW32(x) (_POW2_BELOW16(x)|_POW2_BELOW16((x)>>16))
-#define POW2_BELOW32(x) ((_POW2_BELOW32(x)>>1) + 1)
-
-#define CREDITS_PER_JIFFY POW2_BELOW32(MAX_CPJ)
-
-/* Precision saver. */
-static inline u_int32_t
-user2credits(u_int32_t user)
-{
- /* If multiplying would overflow... */
- if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
- /* Divide first. */
- return (user / XT_HASHLIMIT_SCALE) * HZ * CREDITS_PER_JIFFY;
-
- return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE;
-}
-
-static inline void rateinfo_recalc(struct dsthash_ent *dh, unsigned long now)
+static inline void rateinfo_recalc(struct dsthash_ent *dh, unsigned long now,
+ struct xt_hashlimit_htable *hinfo)
{
dh->rateinfo.credit += (now - dh->rateinfo.prev) * CREDITS_PER_JIFFY;
- if (dh->rateinfo.credit > dh->rateinfo.credit_cap)
- dh->rateinfo.credit = dh->rateinfo.credit_cap;
+ if (dh->rateinfo.credit > hinfo->credit_cap)
+ dh->rateinfo.credit = hinfo->credit_cap;
dh->rateinfo.prev = now;
}
@@ -563,9 +542,7 @@ hashlimit_init_dst(const struct xt_hashlimit_htable *hinfo,
&_ports);
break;
default:
- _ports[0] = _ports[1] = 0;
- ports = _ports;
- break;
+ return 0;
}
if (!ports)
return -1;
@@ -576,6 +553,48 @@ hashlimit_init_dst(const struct xt_hashlimit_htable *hinfo,
return 0;
}
+/* allocate dsthash_ent, initialize dst, put in htable and lock it */
+static struct dsthash_ent *
+dsthash_alloc_init(struct xt_hashlimit_htable *ht,
+ const struct dsthash_dst *dst)
+{
+ struct dsthash_ent *ent = NULL;
+
+ spin_lock(&ht->hlock);
+ /* initialize hash with random val at the time we allocate
+ * the first hashtable entry */
+ if (unlikely(!ht->rnd_initialized)) {
+ get_random_bytes(&ht->rnd, sizeof(ht->rnd));
+ ht->rnd_initialized = true;
+ }
+
+ if (ht->cfg.max && ht->count >= ht->cfg.max) {
+ /* FIXME: do something. question is what.. */
+ if (net_ratelimit())
+ printk(KERN_WARNING
+ "xt_hashlimit: max count of %u reached\n",
+ ht->cfg.max);
+ } else
+ ent = kmem_cache_alloc(hashlimit_cachep, GFP_ATOMIC);
+ if (!ent) {
+ if (net_ratelimit())
+ pr_err("xt_hashlimit: can't allocate dsthash_ent\n");
+ } else {
+ memcpy(&ent->dst, dst, sizeof(ent->dst));
+ spin_lock_init(&ent->lock);
+ ent->expires = jiffies + msecs_to_jiffies(ht->cfg.expire);
+ ent->rateinfo.prev = jiffies;
+ ent->rateinfo.credit = ht->credit_cap;
+ ent->rateinfo.cost = user2credits(ht->cfg.avg);
+ spin_lock(&ent->lock);
+
+ hlist_add_head_rcu(&ent->node, &ht->hash[hash_dst(ht, dst)]);
+ ht->count++;
+ }
+ spin_unlock(&ht->hlock);
+ return ent;
+}
+
static bool
hashlimit_mt_v0(const struct sk_buff *skb, const struct xt_match_param *par)
{
@@ -588,38 +607,30 @@ hashlimit_mt_v0(const struct sk_buff *skb, const struct xt_match_param *par)
if (hashlimit_init_dst(hinfo, &dst, skb, par->thoff) < 0)
goto hotdrop;
- spin_lock_bh(&hinfo->lock);
+ rcu_read_lock_bh();
dh = dsthash_find(hinfo, &dst);
if (!dh) {
dh = dsthash_alloc_init(hinfo, &dst);
if (!dh) {
- spin_unlock_bh(&hinfo->lock);
+ rcu_read_unlock_bh();
goto hotdrop;
}
-
- dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
- dh->rateinfo.prev = jiffies;
- dh->rateinfo.credit = user2credits(hinfo->cfg.avg *
- hinfo->cfg.burst);
- dh->rateinfo.credit_cap = user2credits(hinfo->cfg.avg *
- hinfo->cfg.burst);
- dh->rateinfo.cost = user2credits(hinfo->cfg.avg);
} else {
/* update expiration timeout */
dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
- rateinfo_recalc(dh, now);
+ rateinfo_recalc(dh, now, hinfo);
}
if (dh->rateinfo.credit >= dh->rateinfo.cost) {
/* We're underlimit. */
dh->rateinfo.credit -= dh->rateinfo.cost;
- spin_unlock_bh(&hinfo->lock);
+ spin_unlock(&dh->lock);
+ rcu_read_unlock_bh();
return true;
}
- spin_unlock_bh(&hinfo->lock);
-
- /* default case: we're overlimit, thus don't match */
+ spin_unlock(&dh->lock);
+ rcu_read_unlock_bh();
return false;
hotdrop:
@@ -639,36 +650,31 @@ hashlimit_mt(const struct sk_buff *skb, const struct xt_match_param *par)
if (hashlimit_init_dst(hinfo, &dst, skb, par->thoff) < 0)
goto hotdrop;
- spin_lock_bh(&hinfo->lock);
+ rcu_read_lock_bh();
dh = dsthash_find(hinfo, &dst);
if (dh == NULL) {
dh = dsthash_alloc_init(hinfo, &dst);
if (dh == NULL) {
- spin_unlock_bh(&hinfo->lock);
+ rcu_read_unlock_bh();
goto hotdrop;
}
- dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
- dh->rateinfo.prev = jiffies;
- dh->rateinfo.credit = user2credits(hinfo->cfg.avg *
- hinfo->cfg.burst);
- dh->rateinfo.credit_cap = user2credits(hinfo->cfg.avg *
- hinfo->cfg.burst);
- dh->rateinfo.cost = user2credits(hinfo->cfg.avg);
} else {
/* update expiration timeout */
dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
- rateinfo_recalc(dh, now);
+ rateinfo_recalc(dh, now, hinfo);
}
if (dh->rateinfo.credit >= dh->rateinfo.cost) {
/* below the limit */
dh->rateinfo.credit -= dh->rateinfo.cost;
- spin_unlock_bh(&hinfo->lock);
+ spin_unlock(&dh->lock);
+ rcu_read_unlock_bh();
return !(info->cfg.mode & XT_HASHLIMIT_INVERT);
}
- spin_unlock_bh(&hinfo->lock);
+ spin_unlock(&dh->lock);
+ rcu_read_unlock_bh();
/* default match is underlimit - so over the limit, we need to invert */
return info->cfg.mode & XT_HASHLIMIT_INVERT;
@@ -843,12 +849,12 @@ static struct xt_match hashlimit_mt_reg[] __read_mostly = {
/* PROC stuff */
static void *dl_seq_start(struct seq_file *s, loff_t *pos)
- __acquires(htable->lock)
+ __acquires(htable->hlock)
{
struct xt_hashlimit_htable *htable = s->private;
unsigned int *bucket;
- spin_lock_bh(&htable->lock);
+ spin_lock_bh(&htable->hlock);
if (*pos >= htable->cfg.size)
return NULL;
@@ -874,46 +880,52 @@ static void *dl_seq_next(struct seq_file *s, void *v, loff_t *pos)
}
static void dl_seq_stop(struct seq_file *s, void *v)
- __releases(htable->lock)
+ __releases(htable->hlock)
{
struct xt_hashlimit_htable *htable = s->private;
unsigned int *bucket = (unsigned int *)v;
kfree(bucket);
- spin_unlock_bh(&htable->lock);
+ spin_unlock_bh(&htable->hlock);
}
static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
- struct seq_file *s)
+ struct seq_file *s, struct xt_hashlimit_htable *htable)
{
+ int res = 0;
+
+ spin_lock(&ent->lock);
/* recalculate to show accurate numbers */
- rateinfo_recalc(ent, jiffies);
+ rateinfo_recalc(ent, jiffies, htable);
switch (family) {
case NFPROTO_IPV4:
- return seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
+ res = seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
(long)(ent->expires - jiffies)/HZ,
&ent->dst.ip.src,
ntohs(ent->dst.src_port),
&ent->dst.ip.dst,
ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.credit, htable->credit_cap,
ent->rateinfo.cost);
+ break;
#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
case NFPROTO_IPV6:
- return seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
+ res = seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
(long)(ent->expires - jiffies)/HZ,
&ent->dst.ip6.src,
ntohs(ent->dst.src_port),
&ent->dst.ip6.dst,
ntohs(ent->dst.dst_port),
- ent->rateinfo.credit, ent->rateinfo.credit_cap,
+ ent->rateinfo.credit, htable->credit_cap,
ent->rateinfo.cost);
+ break;
#endif
default:
BUG();
- return 0;
}
+ spin_unlock(&ent->lock);
+ return res;
}
static int dl_seq_show(struct seq_file *s, void *v)
@@ -925,7 +937,7 @@ static int dl_seq_show(struct seq_file *s, void *v)
if (!hlist_empty(&htable->hash[*bucket])) {
hlist_for_each_entry(ent, pos, &htable->hash[*bucket], node)
- if (dl_seq_real_show(ent, htable->family, s))
+ if (dl_seq_real_show(ent, htable->family, s, htable))
return -1;
}
return 0;
@@ -1037,9 +1049,9 @@ err1:
static void __exit hashlimit_mt_exit(void)
{
- kmem_cache_destroy(hashlimit_cachep);
xt_unregister_matches(hashlimit_mt_reg, ARRAY_SIZE(hashlimit_mt_reg));
unregister_pernet_subsys(&hashlimit_net_ops);
+ kmem_cache_destroy(hashlimit_cachep);
}
module_init(hashlimit_mt_init);
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH RFC 2/2] xfrm: cache bundles instead of policies for outgoing flows
From: Timo Teras @ 2010-03-25 9:24 UTC (permalink / raw)
To: netdev; +Cc: Herbert Xu, Timo Teras
In-Reply-To: <1269509091-6440-1-git-send-email-timo.teras@iki.fi>
__xfrm_lookup() is called for each packet transmitted out of
system. The xfrm_find_bundle() does a linear search which can
kill system performance depending on how many bundles are
required per policy.
This modifies __xfrm_lookup() to store bundles directly in
the flow cache. If we did not get a hit, we just create a new
bundle instead of doing slow search. This means that we can now
get multiple xfrm_dst's for same flow (on per-cpu basis).
Signed-off-by: Timo Teras <timo.teras@iki.fi>
---
include/net/flow.h | 6 +-
include/net/xfrm.h | 10 +-
net/core/flow.c | 4 +-
net/ipv4/xfrm4_policy.c | 22 --
net/ipv6/xfrm6_policy.c | 31 ---
net/xfrm/xfrm_policy.c | 690 ++++++++++++++++++++++++-----------------------
6 files changed, 360 insertions(+), 403 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 68fea54..9264c82 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -93,12 +93,12 @@ struct flow_cache_entry_ops {
};
typedef struct flow_cache_entry_ops **(*flow_resolve_t)(
- struct net *net, struct flowi *key, u16 family,
- u8 dir, struct flow_cache_entry_ops **old_ops);
+ struct net *net, struct flowi *key, u16 family, u8 dir,
+ struct flow_cache_entry_ops **old_ops, void *ctx);
extern struct flow_cache_entry_ops **flow_cache_lookup(
struct net *net, struct flowi *key, u16 family,
- u8 dir, flow_resolve_t resolver);
+ u8 dir, flow_resolve_t resolver, void *ctx);
extern void flow_cache_flush(void);
extern atomic_t flow_cache_genid;
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index cb8934b..6ce593f 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -267,7 +267,6 @@ struct xfrm_policy_afinfo {
xfrm_address_t *saddr,
xfrm_address_t *daddr);
int (*get_saddr)(struct net *net, xfrm_address_t *saddr, xfrm_address_t *daddr);
- struct dst_entry *(*find_bundle)(struct flowi *fl, struct xfrm_policy *policy);
void (*decode_session)(struct sk_buff *skb,
struct flowi *fl,
int reverse);
@@ -483,13 +482,13 @@ struct xfrm_policy {
struct timer_list timer;
struct flow_cache_entry_ops *fc_ops;
+ atomic_t genid;
u32 priority;
u32 index;
struct xfrm_mark mark;
struct xfrm_selector selector;
struct xfrm_lifetime_cfg lft;
struct xfrm_lifetime_cur curlft;
- struct dst_entry *bundles;
struct xfrm_policy_walk_entry walk;
u8 type;
u8 action;
@@ -879,11 +878,15 @@ struct xfrm_dst {
struct rt6_info rt6;
} u;
struct dst_entry *route;
+ struct flow_cache_entry_ops *fc_ops;
+ struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
+ int num_pols, num_xfrms;
#ifdef CONFIG_XFRM_SUB_POLICY
struct flowi *origin;
struct xfrm_selector *partner;
#endif
- u32 genid;
+ u32 xfrm_genid;
+ u32 policy_genid;
u32 route_mtu_cached;
u32 child_mtu_cached;
u32 route_cookie;
@@ -893,6 +896,7 @@ struct xfrm_dst {
#ifdef CONFIG_XFRM
static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
{
+ xfrm_pols_put(xdst->pols, xdst->num_pols);
dst_release(xdst->route);
if (likely(xdst->u.dst.xfrm))
xfrm_state_put(xdst->u.dst.xfrm);
diff --git a/net/core/flow.c b/net/core/flow.c
index dfbf3c9..d3b9a22 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -166,7 +166,7 @@ static int flow_key_compare(struct flowi *key1, struct flowi *key2)
struct flow_cache_entry_ops **flow_cache_lookup(
struct net *net, struct flowi *key, u16 family, u8 dir,
- flow_resolve_t resolver)
+ flow_resolve_t resolver, void *ctx)
{
struct flow_cache_entry *fle, **head;
struct flow_cache_entry_ops **ops;
@@ -232,7 +232,7 @@ struct flow_cache_entry_ops **flow_cache_lookup(
}
nocache:
- ops = resolver(net, key, family, dir, ops);
+ ops = resolver(net, key, family, dir, ops, ctx);
if (fle) {
fle->genid = atomic_read(&flow_cache_genid);
if (IS_ERR(ops)) {
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index e4a1483..1705476 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -59,27 +59,6 @@ static int xfrm4_get_saddr(struct net *net,
return 0;
}
-static struct dst_entry *
-__xfrm4_find_bundle(struct flowi *fl, struct xfrm_policy *policy)
-{
- struct dst_entry *dst;
-
- read_lock_bh(&policy->lock);
- for (dst = policy->bundles; dst; dst = dst->next) {
- struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
- if (xdst->u.rt.fl.oif == fl->oif && /*XXX*/
- xdst->u.rt.fl.fl4_dst == fl->fl4_dst &&
- xdst->u.rt.fl.fl4_src == fl->fl4_src &&
- xdst->u.rt.fl.fl4_tos == fl->fl4_tos &&
- xfrm_bundle_ok(policy, xdst, fl, AF_INET, 0)) {
- dst_clone(dst);
- break;
- }
- }
- read_unlock_bh(&policy->lock);
- return dst;
-}
-
static int xfrm4_get_tos(struct flowi *fl)
{
return fl->fl4_tos;
@@ -259,7 +238,6 @@ static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
.dst_ops = &xfrm4_dst_ops,
.dst_lookup = xfrm4_dst_lookup,
.get_saddr = xfrm4_get_saddr,
- .find_bundle = __xfrm4_find_bundle,
.decode_session = _decode_session4,
.get_tos = xfrm4_get_tos,
.init_path = xfrm4_init_path,
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index ae18165..8c452fd 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -67,36 +67,6 @@ static int xfrm6_get_saddr(struct net *net,
return 0;
}
-static struct dst_entry *
-__xfrm6_find_bundle(struct flowi *fl, struct xfrm_policy *policy)
-{
- struct dst_entry *dst;
-
- /* Still not clear if we should set fl->fl6_{src,dst}... */
- read_lock_bh(&policy->lock);
- for (dst = policy->bundles; dst; dst = dst->next) {
- struct xfrm_dst *xdst = (struct xfrm_dst*)dst;
- struct in6_addr fl_dst_prefix, fl_src_prefix;
-
- ipv6_addr_prefix(&fl_dst_prefix,
- &fl->fl6_dst,
- xdst->u.rt6.rt6i_dst.plen);
- ipv6_addr_prefix(&fl_src_prefix,
- &fl->fl6_src,
- xdst->u.rt6.rt6i_src.plen);
- if (ipv6_addr_equal(&xdst->u.rt6.rt6i_dst.addr, &fl_dst_prefix) &&
- ipv6_addr_equal(&xdst->u.rt6.rt6i_src.addr, &fl_src_prefix) &&
- xfrm_bundle_ok(policy, xdst, fl, AF_INET6,
- (xdst->u.rt6.rt6i_dst.plen != 128 ||
- xdst->u.rt6.rt6i_src.plen != 128))) {
- dst_clone(dst);
- break;
- }
- }
- read_unlock_bh(&policy->lock);
- return dst;
-}
-
static int xfrm6_get_tos(struct flowi *fl)
{
return 0;
@@ -291,7 +261,6 @@ static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
.dst_ops = &xfrm6_dst_ops,
.dst_lookup = xfrm6_dst_lookup,
.get_saddr = xfrm6_get_saddr,
- .find_bundle = __xfrm6_find_bundle,
.decode_session = _decode_session6,
.get_tos = xfrm6_get_tos,
.init_path = xfrm6_init_path,
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index a0fa804..281c621 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -50,6 +50,7 @@ static DEFINE_SPINLOCK(xfrm_policy_gc_lock);
static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
static void xfrm_init_pmtu(struct dst_entry *dst);
+static int stale_bundle(struct dst_entry *dst);
static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
int dir);
@@ -272,8 +273,6 @@ void xfrm_policy_destroy(struct xfrm_policy *policy)
{
BUG_ON(!policy->walk.dead);
- BUG_ON(policy->bundles);
-
if (del_timer(&policy->timer))
BUG();
@@ -284,12 +283,7 @@ EXPORT_SYMBOL(xfrm_policy_destroy);
static void xfrm_policy_gc_kill(struct xfrm_policy *policy)
{
- struct dst_entry *dst;
-
- while ((dst = policy->bundles) != NULL) {
- policy->bundles = dst->next;
- dst_free(dst);
- }
+ atomic_inc(&policy->genid);
if (del_timer(&policy->timer))
atomic_dec(&policy->refcnt);
@@ -577,7 +571,6 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
struct xfrm_policy *delpol;
struct hlist_head *chain;
struct hlist_node *entry, *newpos;
- struct dst_entry *gc_list;
u32 mark = policy->mark.v & policy->mark.m;
write_lock_bh(&xfrm_policy_lock);
@@ -628,33 +621,11 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
schedule_work(&net->xfrm.policy_hash_work);
read_lock_bh(&xfrm_policy_lock);
- gc_list = NULL;
entry = &policy->bydst;
- hlist_for_each_entry_continue(policy, entry, bydst) {
- struct dst_entry *dst;
-
- write_lock(&policy->lock);
- dst = policy->bundles;
- if (dst) {
- struct dst_entry *tail = dst;
- while (tail->next)
- tail = tail->next;
- tail->next = gc_list;
- gc_list = dst;
-
- policy->bundles = NULL;
- }
- write_unlock(&policy->lock);
- }
+ hlist_for_each_entry_continue(policy, entry, bydst)
+ atomic_inc(&policy->genid);
read_unlock_bh(&xfrm_policy_lock);
- while (gc_list) {
- struct dst_entry *dst = gc_list;
-
- gc_list = dst->next;
- dst_free(dst);
- }
-
return 0;
}
EXPORT_SYMBOL(xfrm_policy_insert);
@@ -1006,30 +977,35 @@ fail:
return ret;
}
+static struct xfrm_policy *__xfrm_policy_lookup(
+ struct net *net, struct flowi *fl,
+ u16 family, u8 dir)
+{
+#ifdef CONFIG_XFRM_SUB_POLICY
+ struct xfrm_policy *pol;
+
+ pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_SUB, fl, family, dir);
+ if (pol != NULL)
+ return pol;
+#endif
+ return xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN, fl, family, dir);
+}
+
static struct flow_cache_entry_ops **xfrm_policy_lookup(
struct net *net, struct flowi *fl, u16 family,
- u8 dir, struct flow_cache_entry_ops **old_ops)
+ u8 dir, struct flow_cache_entry_ops **old_ops, void *ctx)
{
struct xfrm_policy *pol;
if (old_ops)
xfrm_pol_put(container_of(old_ops, struct xfrm_policy, fc_ops));
-#ifdef CONFIG_XFRM_SUB_POLICY
- pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_SUB, fl, family, dir);
+ pol = __xfrm_policy_lookup(net, fl, family, dir);
if (IS_ERR(pol))
return (void *) pol;
- if (pol)
- goto found;
-#endif
- pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN, fl, family, dir);
- if (IS_ERR(pol))
- return (void *) pol;
- if (pol)
- goto found;
- return NULL;
+ if (!pol)
+ return NULL;
-found:
/* Resolver returns two references:
* one for cache and one for caller of flow_cache_lookup() */
xfrm_pol_hold(pol);
@@ -1318,18 +1294,6 @@ xfrm_tmpl_resolve(struct xfrm_policy **pols, int npols, struct flowi *fl,
* still valid.
*/
-static struct dst_entry *
-xfrm_find_bundle(struct flowi *fl, struct xfrm_policy *policy, unsigned short family)
-{
- struct dst_entry *x;
- struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family);
- if (unlikely(afinfo == NULL))
- return ERR_PTR(-EINVAL);
- x = afinfo->find_bundle(fl, policy);
- xfrm_policy_put_afinfo(afinfo);
- return x;
-}
-
static inline int xfrm_get_tos(struct flowi *fl, int family)
{
struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family);
@@ -1345,6 +1309,41 @@ static inline int xfrm_get_tos(struct flowi *fl, int family)
return tos;
}
+static struct flow_cache_entry_ops **xfrm_bundle_get_fce(
+ struct flow_cache_entry_ops **ops)
+{
+ struct xfrm_dst *xdst = container_of(ops, struct xfrm_dst, fc_ops);
+ struct dst_entry *dst = (struct dst_entry*) xdst;
+
+ if (xdst->route == NULL) {
+ /* Dummy bundle - if it has xfrms we were not
+ * able to build bundle as template resolution failed.
+ * It means we need to try again resolving. */
+ if (xdst->num_xfrms > 0)
+ return NULL;
+ } else {
+ /* Real bundle */
+ if (stale_bundle(dst))
+ return NULL;
+ }
+
+ dst_hold(dst);
+ return ops;
+}
+
+static void xfrm_bundle_delete_fce(struct flow_cache_entry_ops **ops)
+{
+ struct xfrm_dst *xdst = container_of(ops, struct xfrm_dst, fc_ops);
+ struct dst_entry *dst = (struct dst_entry*) xdst;
+
+ dst_free(dst);
+}
+
+static struct flow_cache_entry_ops xfrm_bundle_fc_ops __read_mostly = {
+ .get = xfrm_bundle_get_fce,
+ .delete = xfrm_bundle_delete_fce,
+};
+
static inline struct xfrm_dst *xfrm_alloc_dst(struct net *net, int family)
{
struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family);
@@ -1367,9 +1366,10 @@ static inline struct xfrm_dst *xfrm_alloc_dst(struct net *net, int family)
BUG();
}
xdst = dst_alloc(dst_ops) ?: ERR_PTR(-ENOBUFS);
-
xfrm_policy_put_afinfo(afinfo);
+ xdst->fc_ops = &xfrm_bundle_fc_ops;
+
return xdst;
}
@@ -1407,6 +1407,7 @@ static inline int xfrm_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
return err;
}
+
/* Allocate chain of dst_entry's, attach known xfrm's, calculate
* all the metrics... Shortly, bundle a bundle.
*/
@@ -1470,7 +1471,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
dst_hold(dst);
dst1->xfrm = xfrm[i];
- xdst->genid = xfrm[i]->genid;
+ xdst->xfrm_genid = xfrm[i]->genid;
dst1->obsolete = -1;
dst1->flags |= DST_HOST;
@@ -1563,7 +1564,189 @@ xfrm_dst_update_origin(struct dst_entry *dst, struct flowi *fl)
#endif
}
-static int stale_bundle(struct dst_entry *dst);
+static int xfrm_resolve_policies(struct xfrm_policy *main_policy,
+ struct flowi *fl, u16 family,
+ struct xfrm_policy **pols,
+ int *num_pols, int *num_xfrms)
+{
+ int i;
+
+ if (!main_policy) {
+ *num_pols = 0;
+ *num_xfrms = 0;
+ return 0;
+ }
+ if (IS_ERR(main_policy))
+ return PTR_ERR(main_policy);
+
+ pols[0] = main_policy;
+ *num_pols = 1;
+ *num_xfrms = pols[0]->xfrm_nr;
+
+#ifdef CONFIG_XFRM_SUB_POLICY
+ if (pols[0] && pols[0]->action == XFRM_POLICY_ALLOW &&
+ pols[0]->type != XFRM_POLICY_TYPE_MAIN) {
+ pols[1] = xfrm_policy_lookup_bytype(xp_net(main_policy),
+ XFRM_POLICY_TYPE_MAIN,
+ fl, family,
+ XFRM_POLICY_OUT);
+ if (pols[1]) {
+ if (IS_ERR(pols[1])) {
+ xfrm_pols_put(pols, *num_pols);
+ return PTR_ERR(pols[1]);
+ }
+ (*num_pols) ++;
+ (*num_xfrms) += pols[1]->xfrm_nr;
+ }
+ }
+#endif
+ for (i = 0; i < *num_pols; i++) {
+ if (pols[i]->action != XFRM_POLICY_ALLOW) {
+ *num_xfrms = -1;
+ break;
+ }
+ }
+
+ return 0;
+
+}
+
+static struct xfrm_dst *xfrm_resolve_and_create_bundle(
+ struct xfrm_policy **pols, int num_pols,
+ struct flowi *fl, u16 family, struct dst_entry *dst_orig)
+{
+ struct net *net = xp_net(pols[0]);
+ struct xfrm_state *xfrm[XFRM_MAX_DEPTH];
+ struct dst_entry *dst;
+ struct xfrm_dst *xdst;
+ int err;
+
+ /* Try to instantiate a bundle */
+ err = xfrm_tmpl_resolve(pols, num_pols, fl, xfrm, family);
+ if (err < 0) {
+ if (err != -EAGAIN)
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
+ return ERR_PTR(err);
+ }
+
+ dst = xfrm_bundle_create(pols[0], xfrm, err, fl, dst_orig);
+ if (IS_ERR(dst)) {
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLEGENERROR);
+ return (void*) dst;
+ }
+
+ xdst = (struct xfrm_dst *)dst;
+ xdst->num_xfrms = err;
+ if (num_pols > 1)
+ err = xfrm_dst_update_parent(dst, &pols[1]->selector);
+ else
+ err = xfrm_dst_update_origin(dst, fl);
+ if (unlikely(err)) {
+ dst_free(dst);
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
+ return ERR_PTR(err);
+ }
+
+ xdst->num_pols = num_pols;
+ memcpy(xdst->pols, pols, sizeof(struct xfrm_policy*) * num_pols);
+ xdst->policy_genid = atomic_read(&pols[0]->genid);
+
+ return xdst;
+}
+
+static struct flow_cache_entry_ops **xfrm_bundle_lookup(
+ struct net *net, struct flowi *fl, u16 family, u8 dir,
+ struct flow_cache_entry_ops **old_ops, void *ctx)
+{
+ struct dst_entry *dst_orig = (struct dst_entry *)ctx;
+ struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
+ struct xfrm_dst *xdst, *new_xdst;
+ int num_pols = 0, num_xfrms = 0, i, err, pol_dead;
+
+ /* Check if the policies from old bundle are usable */
+ xdst = NULL;
+ if (old_ops) {
+ xdst = container_of(old_ops, struct xfrm_dst, fc_ops);
+ num_pols = xdst->num_pols;
+ num_xfrms = xdst->num_xfrms;
+ pol_dead = 0;
+ for (i = 0; i < num_pols; i++) {
+ pols[i] = xdst->pols[i];
+ read_lock_bh(&pols[i]->lock);
+ pol_dead |= pols[i]->walk.dead;
+ read_unlock_bh(&pols[i]->lock);
+ }
+ if (pol_dead) {
+ dst_free((struct dst_entry*) xdst);
+ xdst = NULL;
+ num_pols = 0;
+ num_xfrms = 0;
+ old_ops = NULL;
+ }
+ }
+
+ /* Resolve policies to use if we couldn't get them from
+ * previous cache entry */
+ if (xdst == NULL) {
+ pols[0] = __xfrm_policy_lookup(net, fl, family, dir);
+ err = xfrm_resolve_policies(pols[0], fl, family,
+ pols, &num_pols, &num_xfrms);
+ if (err < 0)
+ goto inc_error;
+ if (num_pols == 0)
+ return NULL;
+ if (num_xfrms <= 0)
+ goto make_dummy_bundle;
+ }
+
+ new_xdst = xfrm_resolve_and_create_bundle(pols, num_pols, fl, family, dst_orig);
+ if (IS_ERR(new_xdst)) {
+ err = PTR_ERR(new_xdst);
+ if (err != -EAGAIN)
+ goto error;
+ if (old_ops == NULL)
+ goto make_dummy_bundle;
+ dst_hold((struct dst_entry*) xdst);
+ return old_ops;
+ }
+
+ /* Kill the previous bundle */
+ if (xdst) {
+ /* The policies were stolen for newly generated bundle */
+ xdst->num_pols = 0;
+ dst_free((struct dst_entry*)xdst);
+ }
+
+ /* Flow cache does not have reference, it dst_free()'s,
+ * but we do need to return one reference for original caller */
+ dst_hold((struct dst_entry*)new_xdst);
+ return &new_xdst->fc_ops;
+
+make_dummy_bundle:
+ /* We found policies, but there's no bundles to instantiate:
+ * either because the policy blocks, has no transformations or
+ * we could not build template (no xfrm_states).*/
+ xdst = xfrm_alloc_dst(net, family);
+ if (IS_ERR(xdst)) {
+ xfrm_pols_put(pols, num_pols);
+ return (void*) xdst;
+ }
+ xdst->num_pols = num_pols;
+ xdst->num_xfrms = num_xfrms;
+ memcpy(xdst->pols, pols, sizeof(struct xfrm_policy*) * num_pols);
+
+ dst_hold((struct dst_entry*)xdst);
+ return &xdst->fc_ops;
+
+inc_error:
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
+error:
+ if (xdst != NULL)
+ dst_free((struct dst_entry*) xdst);
+ else
+ xfrm_pols_put(pols, num_pols);
+ return ERR_PTR(err);
+}
/* Main function: finds/creates a bundle for given flow.
*
@@ -1573,251 +1756,138 @@ static int stale_bundle(struct dst_entry *dst);
int __xfrm_lookup(struct net *net, struct dst_entry **dst_p, struct flowi *fl,
struct sock *sk, int flags)
{
- struct xfrm_policy *policy;
- struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
- int npols;
- int pol_dead;
- int xfrm_nr;
- int pi;
- struct xfrm_state *xfrm[XFRM_MAX_DEPTH];
- struct dst_entry *dst, *dst_orig = *dst_p;
- int nx = 0;
- int err;
- u32 genid;
- u16 family;
+ struct flow_cache_entry_ops **ops;
+ struct xfrm_dst *xdst;
+ struct xfrm_policy *pol;
+ struct dst_entry *dst, *dst_orig = *dst_p, *route;
+ u16 family = dst_orig->ops->family;
u8 dir = policy_to_flow_dir(XFRM_POLICY_OUT);
+ int err, num_pols, num_xfrms;
restart:
- genid = atomic_read(&flow_cache_genid);
- policy = NULL;
- for (pi = 0; pi < ARRAY_SIZE(pols); pi++)
- pols[pi] = NULL;
- npols = 0;
- pol_dead = 0;
- xfrm_nr = 0;
+ dst = NULL;
+ xdst = NULL;
+ route = NULL;
if (sk && sk->sk_policy[XFRM_POLICY_OUT]) {
- policy = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl);
- err = PTR_ERR(policy);
- if (IS_ERR(policy)) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
+ struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
+
+ pol = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl);
+ err = xfrm_resolve_policies(pol, fl, family,
+ pols, &num_pols, &num_xfrms);
+ if (err < 0)
goto dropdst;
+
+ if (num_pols) {
+ if (num_xfrms <= 0)
+ goto no_transform;
+
+ xdst = xfrm_resolve_and_create_bundle(
+ pols, num_pols, fl,
+ family, dst_orig);
+ if (IS_ERR(xdst)) {
+ xfrm_pols_put(pols, num_pols);
+ err = PTR_ERR(xdst);
+ goto dropdst;
+ }
+ route = xdst->route;
}
}
- if (!policy) {
- struct flow_cache_entry_ops **ops;
-
+ if (xdst == NULL) {
/* To accelerate a bit... */
if ((dst_orig->flags & DST_NOXFRM) ||
!net->xfrm.policy_count[XFRM_POLICY_OUT])
goto nopol;
- ops = flow_cache_lookup(net, fl, dst_orig->ops->family,
- dir, xfrm_policy_lookup);
- err = PTR_ERR(ops);
+ ops = flow_cache_lookup(net, fl, family, dir,
+ xfrm_bundle_lookup, dst_orig);
+ if (ops == NULL)
+ goto nopol;
if (IS_ERR(ops)) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
+ err = PTR_ERR(ops);
goto dropdst;
}
- if (ops)
- policy = container_of(ops, struct xfrm_policy, fc_ops);
- else
- policy = NULL;
- }
+ xdst = container_of(ops, struct xfrm_dst, fc_ops);
+
+ num_pols = xdst->num_pols;
+ num_xfrms = xdst->num_xfrms;
+ pol = xdst->pols[0];
+ route = xdst->route;
+ }
+
+ dst = (struct dst_entry *) xdst;
+ if (route == NULL && num_xfrms > 0) {
+ dst_release(dst);
+
+ /* The only case when xfrm_bundle_lookup() returns a
+ * bundle with null route, is when the template could
+ * not be resolved. It means policies are there, but
+ * bundle could not be created, since we don't yet
+ * have the xfrm_state's. We need to wait for KM to
+ * negotiate new SA's or bail out with error.*/
+ if (net->xfrm.sysctl_larval_drop) {
+ /* EREMOTE tells the caller to generate
+ * a one-shot blackhole route. */
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
+ return -EREMOTE;
+ }
+ if (flags & XFRM_LOOKUP_WAIT) {
+ DECLARE_WAITQUEUE(wait, current);
- if (!policy)
- goto nopol;
+ add_wait_queue(&net->xfrm.km_waitq, &wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&net->xfrm.km_waitq, &wait);
- family = dst_orig->ops->family;
- pols[0] = policy;
- npols ++;
- xfrm_nr += pols[0]->xfrm_nr;
+ if (!signal_pending(current))
+ goto restart;
- err = -ENOENT;
- if ((flags & XFRM_LOOKUP_ICMP) && !(policy->flags & XFRM_POLICY_ICMP))
+ err = -ERESTART;
+ } else
+ err = -EAGAIN;
+
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
goto error;
+ }
- policy->curlft.use_time = get_seconds();
+no_transform:
+ if (num_pols == 0)
+ goto nopol;
- switch (policy->action) {
- default:
- case XFRM_POLICY_BLOCK:
+ if ((flags & XFRM_LOOKUP_ICMP) &&
+ !(pol->flags & XFRM_POLICY_ICMP)) {
+ err = -ENOENT;
+ goto error;
+ }
+
+ pol->curlft.use_time = get_seconds();
+ if (num_xfrms < 0) {
/* Prohibit the flow */
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLBLOCK);
err = -EPERM;
goto error;
-
- case XFRM_POLICY_ALLOW:
-#ifndef CONFIG_XFRM_SUB_POLICY
- if (policy->xfrm_nr == 0) {
- /* Flow passes not transformed. */
- xfrm_pol_put(policy);
- return 0;
- }
-#endif
-
- /* Try to find matching bundle.
- *
- * LATER: help from flow cache. It is optional, this
- * is required only for output policy.
- */
- dst = xfrm_find_bundle(fl, policy, family);
- if (IS_ERR(dst)) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
- err = PTR_ERR(dst);
- goto error;
- }
-
- if (dst)
- break;
-
-#ifdef CONFIG_XFRM_SUB_POLICY
- if (pols[0]->type != XFRM_POLICY_TYPE_MAIN) {
- pols[1] = xfrm_policy_lookup_bytype(net,
- XFRM_POLICY_TYPE_MAIN,
- fl, family,
- XFRM_POLICY_OUT);
- if (pols[1]) {
- if (IS_ERR(pols[1])) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
- err = PTR_ERR(pols[1]);
- goto error;
- }
- if (pols[1]->action == XFRM_POLICY_BLOCK) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLBLOCK);
- err = -EPERM;
- goto error;
- }
- npols ++;
- xfrm_nr += pols[1]->xfrm_nr;
- }
- }
-
- /*
- * Because neither flowi nor bundle information knows about
- * transformation template size. On more than one policy usage
- * we can realize whether all of them is bypass or not after
- * they are searched. See above not-transformed bypass
- * is surrounded by non-sub policy configuration, too.
- */
- if (xfrm_nr == 0) {
- /* Flow passes not transformed. */
- xfrm_pols_put(pols, npols);
- return 0;
- }
-
-#endif
- nx = xfrm_tmpl_resolve(pols, npols, fl, xfrm, family);
-
- if (unlikely(nx<0)) {
- err = nx;
- if (err == -EAGAIN && net->xfrm.sysctl_larval_drop) {
- /* EREMOTE tells the caller to generate
- * a one-shot blackhole route.
- */
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
- xfrm_pol_put(policy);
- return -EREMOTE;
- }
- if (err == -EAGAIN && (flags & XFRM_LOOKUP_WAIT)) {
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue(&net->xfrm.km_waitq, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
- schedule();
- set_current_state(TASK_RUNNING);
- remove_wait_queue(&net->xfrm.km_waitq, &wait);
-
- nx = xfrm_tmpl_resolve(pols, npols, fl, xfrm, family);
-
- if (nx == -EAGAIN && signal_pending(current)) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
- err = -ERESTART;
- goto error;
- }
- if (nx == -EAGAIN ||
- genid != atomic_read(&flow_cache_genid)) {
- xfrm_pols_put(pols, npols);
- goto restart;
- }
- err = nx;
- }
- if (err < 0) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
- goto error;
- }
- }
- if (nx == 0) {
- /* Flow passes not transformed. */
- xfrm_pols_put(pols, npols);
- return 0;
- }
-
- dst = xfrm_bundle_create(policy, xfrm, nx, fl, dst_orig);
- err = PTR_ERR(dst);
- if (IS_ERR(dst)) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLEGENERROR);
- goto error;
- }
-
- for (pi = 0; pi < npols; pi++) {
- read_lock_bh(&pols[pi]->lock);
- pol_dead |= pols[pi]->walk.dead;
- read_unlock_bh(&pols[pi]->lock);
- }
-
- write_lock_bh(&policy->lock);
- if (unlikely(pol_dead || stale_bundle(dst))) {
- /* Wow! While we worked on resolving, this
- * policy has gone. Retry. It is not paranoia,
- * we just cannot enlist new bundle to dead object.
- * We can't enlist stable bundles either.
- */
- write_unlock_bh(&policy->lock);
- dst_free(dst);
-
- if (pol_dead)
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLDEAD);
- else
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
- err = -EHOSTUNREACH;
- goto error;
- }
-
- if (npols > 1)
- err = xfrm_dst_update_parent(dst, &pols[1]->selector);
- else
- err = xfrm_dst_update_origin(dst, fl);
- if (unlikely(err)) {
- write_unlock_bh(&policy->lock);
- dst_free(dst);
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
- goto error;
- }
-
- dst->next = policy->bundles;
- policy->bundles = dst;
- dst_hold(dst);
- write_unlock_bh(&policy->lock);
+ } else if (num_xfrms > 0) {
+ /* Flow transformed */
+ *dst_p = dst;
+ dst_release(dst_orig);
+ } else {
+ /* Flow passes untransformed */
+ dst_release(dst);
}
- *dst_p = dst;
- dst_release(dst_orig);
- xfrm_pols_put(pols, npols);
return 0;
+nopol:
+ if (!(flags & XFRM_LOOKUP_ICMP))
+ return 0;
+ err = -ENOENT;
error:
- xfrm_pols_put(pols, npols);
+ dst_release(dst);
dropdst:
dst_release(dst_orig);
*dst_p = NULL;
return err;
-
-nopol:
- err = -ENOENT;
- if (flags & XFRM_LOOKUP_ICMP)
- goto dropdst;
- return 0;
}
EXPORT_SYMBOL(__xfrm_lookup);
@@ -1980,7 +2050,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
struct flow_cache_entry_ops **ops;
ops = flow_cache_lookup(net, &fl, family, fl_dir,
- xfrm_policy_lookup);
+ xfrm_policy_lookup, NULL);
if (IS_ERR(ops))
pol = (void *) ops;
else if (ops)
@@ -2169,69 +2239,9 @@ static struct dst_entry *xfrm_negative_advice(struct dst_entry *dst)
return dst;
}
-static void prune_one_bundle(struct xfrm_policy *pol, int (*func)(struct dst_entry *), struct dst_entry **gc_list_p)
-{
- struct dst_entry *dst, **dstp;
-
- write_lock(&pol->lock);
- dstp = &pol->bundles;
- while ((dst=*dstp) != NULL) {
- if (func(dst)) {
- *dstp = dst->next;
- dst->next = *gc_list_p;
- *gc_list_p = dst;
- } else {
- dstp = &dst->next;
- }
- }
- write_unlock(&pol->lock);
-}
-
-static void xfrm_prune_bundles(struct net *net, int (*func)(struct dst_entry *))
-{
- struct dst_entry *gc_list = NULL;
- int dir;
-
- read_lock_bh(&xfrm_policy_lock);
- for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) {
- struct xfrm_policy *pol;
- struct hlist_node *entry;
- struct hlist_head *table;
- int i;
-
- hlist_for_each_entry(pol, entry,
- &net->xfrm.policy_inexact[dir], bydst)
- prune_one_bundle(pol, func, &gc_list);
-
- table = net->xfrm.policy_bydst[dir].table;
- for (i = net->xfrm.policy_bydst[dir].hmask; i >= 0; i--) {
- hlist_for_each_entry(pol, entry, table + i, bydst)
- prune_one_bundle(pol, func, &gc_list);
- }
- }
- read_unlock_bh(&xfrm_policy_lock);
-
- while (gc_list) {
- struct dst_entry *dst = gc_list;
- gc_list = dst->next;
- dst_free(dst);
- }
-}
-
-static int unused_bundle(struct dst_entry *dst)
-{
- return !atomic_read(&dst->__refcnt);
-}
-
static void __xfrm_garbage_collect(struct net *net)
{
- xfrm_prune_bundles(net, unused_bundle);
-}
-
-static int xfrm_flush_bundles(struct net *net)
-{
- xfrm_prune_bundles(net, stale_bundle);
- return 0;
+ flow_cache_flush();
}
static void xfrm_init_pmtu(struct dst_entry *dst)
@@ -2291,7 +2301,9 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *first,
return 0;
if (dst->xfrm->km.state != XFRM_STATE_VALID)
return 0;
- if (xdst->genid != dst->xfrm->genid)
+ if (xdst->xfrm_genid != dst->xfrm->genid)
+ return 0;
+ if (xdst->policy_genid != atomic_read(&xdst->pols[0]->genid))
return 0;
if (strict && fl &&
@@ -2452,11 +2464,9 @@ static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
{
- struct net_device *dev = ptr;
-
switch (event) {
case NETDEV_DOWN:
- xfrm_flush_bundles(dev_net(dev));
+ flow_cache_flush();
}
return NOTIFY_DONE;
}
@@ -2788,7 +2798,6 @@ static int xfrm_policy_migrate(struct xfrm_policy *pol,
struct xfrm_migrate *m, int num_migrate)
{
struct xfrm_migrate *mp;
- struct dst_entry *dst;
int i, j, n = 0;
write_lock_bh(&pol->lock);
@@ -2813,10 +2822,7 @@ static int xfrm_policy_migrate(struct xfrm_policy *pol,
sizeof(pol->xfrm_vec[i].saddr));
pol->xfrm_vec[i].encap_family = mp->new_family;
/* flush bundles */
- while ((dst = pol->bundles) != NULL) {
- pol->bundles = dst->next;
- dst_free(dst);
- }
+ atomic_inc(&pol->genid);
}
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH RFC 1/2] flow: virtualize get and entry deletion methods
From: Timo Teras @ 2010-03-25 9:24 UTC (permalink / raw)
To: netdev; +Cc: Herbert Xu, Timo Teras
In-Reply-To: <1269509091-6440-1-git-send-email-timo.teras@iki.fi>
This allows to validate the cached object before returning it.
It also allows to destruct object properly, if the last reference
was held in flow cache. This is also a prepartion for caching
bundles in the flow cache.
In return for virtualizing the methods, we save on:
- not having to regenerate the whole flow cache on policy removal:
each flow matching a killed policy gets refreshed as the getter
function notices it smartly.
- we do not have to call flow_cache_flush from policy gc, since the
flow cache now properly deletes the object if it had any references
This also means the flow cache entry deletion does more work. If
it's too slow now, may have to implement delayed deletion of flow
cache entries. But this is a save because this enables immediate
deletion of policies and bundles.
Signed-off-by: Timo Teras <timo.teras@iki.fi>
---
include/net/flow.h | 17 ++++++--
include/net/xfrm.h | 2 +
net/core/flow.c | 102 ++++++++++++++++++++++++----------------------
net/xfrm/xfrm_policy.c | 105 +++++++++++++++++++++++++++++++-----------------
4 files changed, 136 insertions(+), 90 deletions(-)
diff --git a/include/net/flow.h b/include/net/flow.h
index 809970b..68fea54 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -86,11 +86,20 @@ struct flowi {
struct net;
struct sock;
-typedef int (*flow_resolve_t)(struct net *net, struct flowi *key, u16 family,
- u8 dir, void **objp, atomic_t **obj_refp);
-extern void *flow_cache_lookup(struct net *net, struct flowi *key, u16 family,
- u8 dir, flow_resolve_t resolver);
+struct flow_cache_entry_ops {
+ struct flow_cache_entry_ops ** (*get)(struct flow_cache_entry_ops **);
+ void (*delete)(struct flow_cache_entry_ops **);
+};
+
+typedef struct flow_cache_entry_ops **(*flow_resolve_t)(
+ struct net *net, struct flowi *key, u16 family,
+ u8 dir, struct flow_cache_entry_ops **old_ops);
+
+extern struct flow_cache_entry_ops **flow_cache_lookup(
+ struct net *net, struct flowi *key, u16 family,
+ u8 dir, flow_resolve_t resolver);
+
extern void flow_cache_flush(void);
extern atomic_t flow_cache_genid;
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index d74e080..cb8934b 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -19,6 +19,7 @@
#include <net/route.h>
#include <net/ipv6.h>
#include <net/ip6_fib.h>
+#include <net/flow.h>
#include <linux/interrupt.h>
@@ -481,6 +482,7 @@ struct xfrm_policy {
atomic_t refcnt;
struct timer_list timer;
+ struct flow_cache_entry_ops *fc_ops;
u32 priority;
u32 index;
struct xfrm_mark mark;
diff --git a/net/core/flow.c b/net/core/flow.c
index 9601587..dfbf3c9 100644
--- a/net/core/flow.c
+++ b/net/core/flow.c
@@ -26,13 +26,12 @@
#include <linux/security.h>
struct flow_cache_entry {
- struct flow_cache_entry *next;
- u16 family;
- u8 dir;
- u32 genid;
- struct flowi key;
- void *object;
- atomic_t *object_ref;
+ struct flow_cache_entry *next;
+ u16 family;
+ u8 dir;
+ u32 genid;
+ struct flowi key;
+ struct flow_cache_entry_ops **ops;
};
atomic_t flow_cache_genid = ATOMIC_INIT(0);
@@ -86,8 +85,8 @@ static void flow_cache_new_hashrnd(unsigned long arg)
static void flow_entry_kill(int cpu, struct flow_cache_entry *fle)
{
- if (fle->object)
- atomic_dec(fle->object_ref);
+ if (fle->ops)
+ (*fle->ops)->delete(fle->ops);
kmem_cache_free(flow_cachep, fle);
flow_count(cpu)--;
}
@@ -165,10 +164,12 @@ static int flow_key_compare(struct flowi *key1, struct flowi *key2)
return 0;
}
-void *flow_cache_lookup(struct net *net, struct flowi *key, u16 family, u8 dir,
- flow_resolve_t resolver)
+struct flow_cache_entry_ops **flow_cache_lookup(
+ struct net *net, struct flowi *key, u16 family, u8 dir,
+ flow_resolve_t resolver)
{
struct flow_cache_entry *fle, **head;
+ struct flow_cache_entry_ops **ops;
unsigned int hash;
int cpu;
@@ -176,6 +177,8 @@ void *flow_cache_lookup(struct net *net, struct flowi *key, u16 family, u8 dir,
cpu = smp_processor_id();
fle = NULL;
+ ops = NULL;
+
/* Packet really early in init? Making flow_cache_init a
* pre-smp initcall would solve this. --RR */
if (!flow_table(cpu))
@@ -187,26 +190,35 @@ void *flow_cache_lookup(struct net *net, struct flowi *key, u16 family, u8 dir,
head = &flow_table(cpu)[hash];
for (fle = *head; fle; fle = fle->next) {
- if (fle->family == family &&
- fle->dir == dir &&
- flow_key_compare(key, &fle->key) == 0) {
- if (fle->genid == atomic_read(&flow_cache_genid)) {
- void *ret = fle->object;
-
- if (ret)
- atomic_inc(fle->object_ref);
- local_bh_enable();
-
- return ret;
+ if (fle->family != family ||
+ fle->dir != dir ||
+ flow_key_compare(key, &fle->key) != 0)
+ continue;
+
+ ops = fle->ops;
+ if (fle->genid == atomic_read(&flow_cache_genid)) {
+ if (ops) {
+ ops = (*ops)->get(ops);
+ if (ops) {
+ local_bh_enable();
+ return ops;
+ }
+ ops = fle->ops;
}
- break;
+ } else {
+ if (ops)
+ (*ops)->delete(ops);
+ fle->ops = NULL;
+ ops = NULL;
}
+ break;
}
if (!fle) {
if (flow_count(cpu) > flow_hwm)
flow_cache_shrink(cpu);
+ ops = NULL;
fle = kmem_cache_alloc(flow_cachep, GFP_ATOMIC);
if (fle) {
fle->next = *head;
@@ -214,36 +226,28 @@ void *flow_cache_lookup(struct net *net, struct flowi *key, u16 family, u8 dir,
fle->family = family;
fle->dir = dir;
memcpy(&fle->key, key, sizeof(*key));
- fle->object = NULL;
+ fle->ops = NULL;
flow_count(cpu)++;
}
}
nocache:
- {
- int err;
- void *obj;
- atomic_t *obj_ref;
-
- err = resolver(net, key, family, dir, &obj, &obj_ref);
-
- if (fle && !err) {
- fle->genid = atomic_read(&flow_cache_genid);
-
- if (fle->object)
- atomic_dec(fle->object_ref);
-
- fle->object = obj;
- fle->object_ref = obj_ref;
- if (obj)
- atomic_inc(fle->object_ref);
+ ops = resolver(net, key, family, dir, ops);
+ if (fle) {
+ fle->genid = atomic_read(&flow_cache_genid);
+ if (IS_ERR(ops)) {
+ fle->genid--;
+ fle->ops = NULL;
+ } else {
+ fle->ops = ops;
}
- local_bh_enable();
-
- if (err)
- obj = ERR_PTR(err);
- return obj;
+ } else {
+ if (ops && !IS_ERR(ops))
+ (*ops)->delete(ops);
}
+ local_bh_enable();
+
+ return ops;
}
static void flow_cache_flush_tasklet(unsigned long data)
@@ -260,11 +264,11 @@ static void flow_cache_flush_tasklet(unsigned long data)
for (; fle; fle = fle->next) {
unsigned genid = atomic_read(&flow_cache_genid);
- if (!fle->object || fle->genid == genid)
+ if (!fle->ops || fle->genid == genid)
continue;
- fle->object = NULL;
- atomic_dec(fle->object_ref);
+ (*fle->ops)->delete(fle->ops);
+ fle->ops = NULL;
}
}
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 843e066..a0fa804 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -216,6 +216,30 @@ expired:
xfrm_pol_put(xp);
}
+static struct flow_cache_entry_ops **xfrm_policy_get_fce(
+ struct flow_cache_entry_ops **ops)
+{
+ struct xfrm_policy *pol = container_of(ops, struct xfrm_policy, fc_ops);
+
+ read_lock(&pol->lock);
+ if (pol->walk.dead)
+ ops = NULL;
+ else
+ xfrm_pol_hold(pol);
+ read_unlock(&pol->lock);
+
+ return ops;
+}
+
+static void xfrm_policy_delete_fce(struct flow_cache_entry_ops **ops)
+{
+ xfrm_pol_put(container_of(ops, struct xfrm_policy, fc_ops));
+}
+
+static struct flow_cache_entry_ops xfrm_policy_fc_ops __read_mostly = {
+ .get = xfrm_policy_get_fce,
+ .delete = xfrm_policy_delete_fce,
+};
/* Allocate xfrm_policy. Not used here, it is supposed to be used by pfkeyv2
* SPD calls.
@@ -236,6 +260,7 @@ struct xfrm_policy *xfrm_policy_alloc(struct net *net, gfp_t gfp)
atomic_set(&policy->refcnt, 1);
setup_timer(&policy->timer, xfrm_policy_timer,
(unsigned long)policy);
+ policy->fc_ops = &xfrm_policy_fc_ops;
}
return policy;
}
@@ -269,9 +294,6 @@ static void xfrm_policy_gc_kill(struct xfrm_policy *policy)
if (del_timer(&policy->timer))
atomic_dec(&policy->refcnt);
- if (atomic_read(&policy->refcnt) > 1)
- flow_cache_flush();
-
xfrm_pol_put(policy);
}
@@ -671,10 +693,8 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u8 type,
}
write_unlock_bh(&xfrm_policy_lock);
- if (ret && delete) {
- atomic_inc(&flow_cache_genid);
+ if (ret && delete)
xfrm_policy_kill(ret);
- }
return ret;
}
EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
@@ -713,10 +733,8 @@ struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u8 type,
}
write_unlock_bh(&xfrm_policy_lock);
- if (ret && delete) {
- atomic_inc(&flow_cache_genid);
+ if (ret && delete)
xfrm_policy_kill(ret);
- }
return ret;
}
EXPORT_SYMBOL(xfrm_policy_byid);
@@ -835,7 +853,6 @@ int xfrm_policy_flush(struct net *net, u8 type, struct xfrm_audit *audit_info)
}
if (!cnt)
err = -ESRCH;
- atomic_inc(&flow_cache_genid);
out:
write_unlock_bh(&xfrm_policy_lock);
return err;
@@ -989,32 +1006,35 @@ fail:
return ret;
}
-static int xfrm_policy_lookup(struct net *net, struct flowi *fl, u16 family,
- u8 dir, void **objp, atomic_t **obj_refp)
+static struct flow_cache_entry_ops **xfrm_policy_lookup(
+ struct net *net, struct flowi *fl, u16 family,
+ u8 dir, struct flow_cache_entry_ops **old_ops)
{
struct xfrm_policy *pol;
- int err = 0;
+
+ if (old_ops)
+ xfrm_pol_put(container_of(old_ops, struct xfrm_policy, fc_ops));
#ifdef CONFIG_XFRM_SUB_POLICY
pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_SUB, fl, family, dir);
- if (IS_ERR(pol)) {
- err = PTR_ERR(pol);
- pol = NULL;
- }
- if (pol || err)
- goto end;
+ if (IS_ERR(pol))
+ return (void *) pol;
+ if (pol)
+ goto found;
#endif
pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN, fl, family, dir);
- if (IS_ERR(pol)) {
- err = PTR_ERR(pol);
- pol = NULL;
- }
-#ifdef CONFIG_XFRM_SUB_POLICY
-end:
-#endif
- if ((*objp = (void *) pol) != NULL)
- *obj_refp = &pol->refcnt;
- return err;
+ if (IS_ERR(pol))
+ return (void *) pol;
+ if (pol)
+ goto found;
+ return NULL;
+
+found:
+ /* Resolver returns two references:
+ * one for cache and one for caller of flow_cache_lookup() */
+ xfrm_pol_hold(pol);
+
+ return &pol->fc_ops;
}
static inline int policy_to_flow_dir(int dir)
@@ -1104,8 +1124,6 @@ int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
pol = __xfrm_policy_unlink(pol, dir);
write_unlock_bh(&xfrm_policy_lock);
if (pol) {
- if (dir < XFRM_POLICY_MAX)
- atomic_inc(&flow_cache_genid);
xfrm_policy_kill(pol);
return 0;
}
@@ -1588,18 +1606,24 @@ restart:
}
if (!policy) {
+ struct flow_cache_entry_ops **ops;
+
/* To accelerate a bit... */
if ((dst_orig->flags & DST_NOXFRM) ||
!net->xfrm.policy_count[XFRM_POLICY_OUT])
goto nopol;
- policy = flow_cache_lookup(net, fl, dst_orig->ops->family,
- dir, xfrm_policy_lookup);
- err = PTR_ERR(policy);
- if (IS_ERR(policy)) {
+ ops = flow_cache_lookup(net, fl, dst_orig->ops->family,
+ dir, xfrm_policy_lookup);
+ err = PTR_ERR(ops);
+ if (IS_ERR(ops)) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
goto dropdst;
}
+ if (ops)
+ policy = container_of(ops, struct xfrm_policy, fc_ops);
+ else
+ policy = NULL;
}
if (!policy)
@@ -1952,9 +1976,16 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
}
}
- if (!pol)
- pol = flow_cache_lookup(net, &fl, family, fl_dir,
+ if (!pol) {
+ struct flow_cache_entry_ops **ops;
+
+ ops = flow_cache_lookup(net, &fl, family, fl_dir,
xfrm_policy_lookup);
+ if (IS_ERR(ops))
+ pol = (void *) ops;
+ else if (ops)
+ pol = container_of(ops, struct xfrm_policy, fc_ops);
+ }
if (IS_ERR(pol)) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
--
1.6.3.3
^ permalink raw reply related
* [PATCH RFC 0/2] caching bundles instead of policies
From: Timo Teras @ 2010-03-25 9:24 UTC (permalink / raw)
To: netdev; +Cc: Herbert Xu, Timo Teras
This is not yet intended for commit (see below), but rather a
snapshot of what I'm working with currently. And to get some feedback
on the direction where I'm going.
The first patch changes flow cache reference counting fundamentally.
It no longer assumes that the cached objects are GC'd later, but instead
calls the virtualized delete method to delete (or drop it's reference).
This does now mean that ->delete() call can be slowish. The plan is to
add ->check() which is called when ever the cache is flushed, add all
deleted entries to GC list, and have the main flush routine delete the
GC list entries.
I wanted to suggest this because:
- xfrm_policy_put() is currently never guaranteed to be fast, instead
it can always result to slow path. Only the flow cache wanted to have
fast free when bh is disabled, and this was made possible with the
policy GC ensuring that cache is flushed before policies are freed.
- now that we can cache bundles or policies, it makes sense to have
more selective flushing; otherwise we lose some of the speed ups.
This also means that flushing gets faster, and is needed very rarely
(sensible points are GC'ing bundles and when interface goes down)
- now we don't have to do two periodic/delayed GC's: one for bundles,
and one for policies. instead we can have central code for that in
the flow cache. this also means that the flow cache hash is the
owner of bundle, and when the flow cache entry is expired the bundle
is dst_free()'d (which we would want to do anyway). no need to keep
bundles in separate global (or policy specific) list
Does this sound acceptable approach?
What the current patch set is missing:
- delayed deletion of flow cache objects
- doing check() on flush for each object
- removing the policy GC
- some of the other flow cache improvements from my original patch
Also, we might want to cache dummy bundles in xfrm_check_policy().
The reason is that if we matched sub policy originally, we always have
to do O(n) search for the main policy.
Timo Teras (2):
flow: virtualize get and entry deletion methods
xfrm: cache bundles instead of policies for outgoing flows
include/net/flow.h | 17 +-
include/net/xfrm.h | 12 +-
net/core/flow.c | 102 ++++---
net/ipv4/xfrm4_policy.c | 22 --
net/ipv6/xfrm6_policy.c | 31 --
net/xfrm/xfrm_policy.c | 755 +++++++++++++++++++++++++----------------------
6 files changed, 471 insertions(+), 468 deletions(-)
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Jozsef Kadlecsik @ 2010-03-25 9:23 UTC (permalink / raw)
To: YOSHIFUJI Hideaki
Cc: Patrick McHardy, Shan Wei, YOSHIFUJI Hideaki, David Miller,
Alexey Dobriyan, Yasuyuki KOZAKAI, netdev@vger.kernel.org,
netfilter-devel
In-Reply-To: <4BAAE496.8000701@linux-ipv6.org>
On Thu, 25 Mar 2010, YOSHIFUJI Hideaki wrote:
> (2010/03/24 5:10), Jozsef Kadlecsik wrote:
>
> > On Wed, 24 Mar 2010, YOSHIFUJI Hideaki wrote:
> >
> > > > In this case without conntrack, IPv6 would send an ICMPv6 message,
> > > > so in my opinion the transparent thing to do would be to still send
> > > > them. Of course only if reassembly is done on an end host.
> > >
> > > Well, no. conntrack should just forward even uncompleted fragments
> > > to next process (e.g. core ipv6 code), and then the core would send
> > > ICMP error back. ICMP should be sent by the core ipv6 code according
> > > to decision of itself, not according to netfilter.
> >
> > But what state could be associated by conntrack to the uncompleted
> > fragments but the INVALID state? In consequence, in any sane setup, the
> > uncompleted fragments will be dropped silently by a filter table rule
> > and no ICMP error message will be sent back.
> >
> > Therefore I think iff the destination of the fragments is the host
> > itself, then conntrack should generate an ICMP error message. But that
> > error message must be processed by conntrack to set its state and then
> > the fate of the generated packet can be decided by a rule.
>
> Well.... no.
>
> First of all. in "sane" setup, people should configure according
> to their own requirements. They may or may not want send back
> icmp packet. And, even if the core is to send icmp back, the
> state would be correctly assigned.
I meant the state of the fragmented packets. If we let the uncompleted
fragments to enter conntrack, as far as I see their state will be INVALID.
Or should we add an exception and set their state to UNTRACKED in
conntrack?
> We cannot (and should not) do something "cleaver" (excluding
> packet drops) in conntrack in PRE_ROUTING chain.
Actually, I have to agree with you.
> One reason is that in PRE_ROUTING context, we can NOT determine
> if the address we see in the IP header is really the final
> destination. The overall situation is the same even if the
> routing entry corresponding the "current" destination points
> the node itself, or even if the node is configured as host.
>
> It might seems that we could do something in the "filter"
> table in LOCAL_IN, FORWARD or LOCAL_OUT (after routing header
> process).
>
> But well, we unfortunately cannot do this (at least
> automatically) because even in LOCAL_IN, the final
> destination has not been decided until we process all
> of extension headers.
>
> Sending ICMP in netfilter (especially in conntrack) is too
> patchy, and is not right. If we do the right thing (and
> I believe we should do so), I'd propose to have hooks
> around handlers inside ip6_input_finish().
>
> ...I remember that I was thinking about this before.
>
> For my conclusion, first option is just to drop
> uncompleted fragments as we do today. Second option
> would be to forward them to the next process so that
> core code could send ICMPv6 etc. or, we could have
> new code to send ICMPV6_TIME_EXCEED in REJECT target.
> In longer term, I think it is better to introduce
> per-exthdr hooks.
I agree with your conclusion too, except a few question.
It is unclear for me how can you forward the packets to the next process:
above you pointed out that in defrag/reassembly before conntrack we do not
know yet whether the packets are destined to the host or not. So again,
how would you let through the fragments on conntrack then?
I don't know how could the REJECT target help in any way.
This is not a simple case at all and I have to think that the "best" way
just to drop the packets as we currently do.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7 v2]IPv6:netfilter: defragment
From: Shan Wei @ 2010-03-25 9:13 UTC (permalink / raw)
To: Pascal Hambourg
Cc: Jozsef Kadlecsik, YOSHIFUJI Hideaki, Patrick McHardy,
David Miller, Alexey Dobriyan, Yasuyuki KOZAKAI,
netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <4BAB2121.2030503@plouf.fr.eu.org>
Pascal Hambourg wrote, at 03/25/2010 04:38 PM:
> Hello,
>
> Jozsef Kadlecsik a écrit :
>> On Wed, 24 Mar 2010, YOSHIFUJI Hideaki wrote:
>>
>>>> In this case without conntrack, IPv6 would send an ICMPv6 message,
>>>> so in my opinion the transparent thing to do would be to still send
>>>> them. Of course only if reassembly is done on an end host.
>>> Well, no. conntrack should just forward even uncompleted fragments
>>> to next process (e.g. core ipv6 code), and then the core would send
>>> ICMP error back. ICMP should be sent by the core ipv6 code according
>>> to decision of itself, not according to netfilter.
>> But what state could be associated by conntrack to the uncompleted
>> fragments but the INVALID state? In consequence, in any sane setup, the
>> uncompleted fragments will be dropped silently by a filter table rule
>> and no ICMP error message will be sent back.
>
> AFAIK, in the IPv4 stack the reassembly takes place before the INPUT
> chains (NF_IP_LOCAL_IN hook). Is it different in the IPv6 stack ?
Yes, they are different.
In IPv4 stack，for an end host, ip_local_deliver() reassemble
fragments before LOCAL_IN hook .
But in IPv6 stack, ip6_input_finish() handles fragment extension headers
and try to reassemble them *after* LOCAL_IN hook.
--
Best Regards
-----
Shan Wei
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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