Re: [PATCH v3 nf-next 1/2] netfilter: x_tables: wait until old table isn't used anymore

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 11, 2017 at 10:48 AM, Florian Westphal <fw@xxxxxxxxx> wrote:
> Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>> On Wed, Oct 11, 2017 at 7:26 AM, Florian Westphal <fw@xxxxxxxxx> wrote:
>> > xt_replace_table relies on table replacement counter retrieval (which
>> > uses xt_recseq to synchronize pcpu counters).
>> >
>> > This is fine, however with large rule set get_counters() can take
>> > a very long time -- it needs to synchronize all counters because
>> > it has to assume concurrent modifications can occur.
>> >
>> > Make xt_replace_table synchronize by itself by waiting until all cpus
>> > had an even seqcount.
>> >
>> > This allows a followup patch to copy the counters of the old ruleset
>> > without any synchonization after xt_replace_table has completed.
>> >
>> > Cc: Dan Williams <dcbw@xxxxxxxxxx>
>> > Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
>> > Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
>> > ---
>> >  v3: check for 'seq is uneven' OR 'has changed' since
>> >  last check. Its fine if seq is uneven iff its a different
>> >  sequence number than the initial one.
>> >
>> >  v2: fix Erics email address
>> >  net/netfilter/x_tables.c | 18 +++++++++++++++---
>> >  1 file changed, 15 insertions(+), 3 deletions(-)
>> >
>> >
>>
>> Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
>>
>> But it seems we need an extra smp_wmb() after
>>
>>         smp_wmb();
>>         table->private = newinfo;
>> +      smp_wmb();
>>
>> Otherwise we have no guarantee other cpus actually see the new ->private value.
>
> Seems to be unrelated to this change, so I will submit
> a separate patch for nf.git that adds this.

This is related to this change, please read the comment before the
local_bh_enable9)

        /*
         * Even though table entries have now been swapped, other CPU's
         * may still be using the old entries. This is okay, because
         * resynchronization happens because of the locking done
         * during the get_counters() routine.
         */


Since your new code happens right after table->private  = newinfo ;
the smp_wmb() is required.
--
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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux