Re: [PATCH 1/7] netfilter: use a linked list of loggers.

[Eric Leblond <eric@inl.fr>]

Hi,

Patrick McHardy a écrit :
> Eric Leblond wrote:
>> This patch modifies nf_log to use a linked list of loggers for each
>
>> ---
>>  include/net/netfilter/nf_log.h |   10 +++-

>> +#define NFLOGGER_NAME_LEN        12
> 
> Just wondering what this limit is based on, I can't seem to
> fine anything requiring a limit at all since the names are
> const char *.

Ok, good remark. This is not needed here but I was forced to keep a name
maximum length due to the maxlen part of sysctl.

>>  static const struct nf_logger *nf_loggers[NFPROTO_NUMPROTO]
>> __read_mostly;
>> -static DEFINE_MUTEX(nf_log_mutex);
>> +static struct list_head nf_loggers_l[NFPROTO_NUMPROTO] __read_mostly;
>> +static DEFINE_SPINLOCK(nf_log_lock);
> 
> I don't understand how locking is supposed to work now. You change
> the mutex to a spinlock and use it with _bh everywhere, yet I don't
> see a single spot thats called outside of process context and the
> RCU assignments are still present.

I've simplified the code to have the loggers list protected by a trivial
mutex and I only use RCU for the used function array.

> The users of nf_log_register are also not converted simultaneously,
> so this patch breaks bisection.

I've reordered and combined my patches to avoid this.

Thanks a lot for all your remarks, the updated patchset will follow as
reply to this mail.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/
--
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