On Thu, Aug 19, 2010 at 11:35 AM, Changli Gao <xiaosuo@xxxxxxxxx> wrote:
> On Thu, Aug 19, 2010 at 11:21 PM, Mathieu Desnoyers
> <mathieu.desnoyers@xxxxxxxxxx> wrote:
>> * Changli Gao (xiaosuo@xxxxxxxxx) wrote:
>>> Since we don't change the tuple in the original direction, we can save it
>>> in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
>>> use.
>>>
>>> __hash_conntrack() is split into two steps: ____hash_conntrack() is used
>>> to get the raw hash, and __hash_bucket() is used to get the bucket id.
>>>
>>> In SYN-flood case, early_drop() doesn't need to recompute the hash again.
>>>
>>> Signed-off-by: Changli Gao <xiaosuo@xxxxxxxxx>
>>> ---
>>> v2: use cmpxchg() to save 2 variables.
>>> net/netfilter/nf_conntrack_core.c | 114 ++++++++++++++++++++++++++------------
>>> 1 file changed, 79 insertions(+), 35 deletions(-)
>>> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
>>> index df3eedb..0e02205 100644
>>> --- a/net/netfilter/nf_conntrack_core.c
>>> +++ b/net/netfilter/nf_conntrack_core.c
>>> @@ -65,14 +65,20 @@ EXPORT_SYMBOL_GPL(nf_conntrack_max);
>>> DEFINE_PER_CPU(struct nf_conn, nf_conntrack_untracked);
>>> EXPORT_PER_CPU_SYMBOL(nf_conntrack_untracked);
>>>
>>> -static int nf_conntrack_hash_rnd_initted;
>>> -static unsigned int nf_conntrack_hash_rnd;
>>> -
>>> -static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
>>> - u16 zone, unsigned int size, unsigned int rnd)
>>> +static u32 ____hash_conntrack(const struct nf_conntrack_tuple *tuple, u16 zone)
>>> {
>>> unsigned int n;
>>> u_int32_t h;
>>> + static unsigned int rnd;
>>
>> Why are you putting a hidden static variable here ? It should probably
>> be declared outside of the function scope.
>
> It is only used in this function.
Looks like it is only initialized once to rnd then it should go a "init".
>
>>
>>> +
>>> + if (unlikely(!rnd)) {
>>> + unsigned int rand;
>>> +
>>> + get_random_bytes(&rand, sizeof(rand));
>>> + if (!rand)
>>> + rand = 1;
>>> + cmpxchg(&rnd, 0, rand);
>>
>> This really belongs to a "init()" function, not in a dynamic check in
>> the __hash_conntrack hot path. If you do that a init time, then you
>> don't even need the cmpxchg.
>>
>> Or maybe I completely misunderstand you goal here; maybe you are really
>> trying to re-calculate random bytes. But more explanation is called for,
>> because I really don't see where the value is brought back to 0.
>>
>
> In fact, I don't know the reason clearly. This code is derived from
> the older one. Maybe there isn't enough entropy when initializing.
>
> --
> Regards,
> Changli Gao(xiaosuo@xxxxxxxxx)
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
yao
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
