On Fri, Nov 16, 2012 at 12:31:09PM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > On Thu, Nov 15, 2012 at 04:55:12PM +0100, Florian Westphal wrote: > > > Changes since RFCv2: > > > - make it a variable-size extension and remove dynamic > > > reallocation of the label array > > > - add ctnetlink support for receiving/setting labels > > > - limit to 128 instead of 1k labels due to limited extension > > > space (128 is more than enough for now, so this is no problem). > > > > At a quick glance I like this round, you got this simplified :-). > > > > My only concern is that (if I'm not missing anything) it > > inconditionally add the extension even if we don't need it. > > Its conditional: > > words = ACCESS_ONCE(net->ct.label_words); > if (words == 0 || WARN_ON_ONCE(words > 8)) > return NULL; > cl_ext = nf_ct_ext_add_length(ct, NF_CT_EXT_LABELS, words * sizeof(long),.. > > xt_connlabel increases "label_words" to a non-zero > value based on the highest bit it saw so far. So e.g. when bit 32 is > requested, a 32-bit machine kernel will set words to 2, and > a 64 bit kernel to 1, thus turning the extension on for new connections. > > The only drawback is that when you add a connlabel rule for bits 127, > then for bit 1, then delete the first rule again words will be larger > than one. > > But i think this ok (I don't want to add per-bit refcountr :-) ) I see. Let me give a more look in-depth, in any case this version seems reasonable to be included mainstream :-). Regards. -- 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