Patrick McHardy <kaber@xxxxxxxxx> wrote: > On 25.04, Florian Westphal wrote: > > Patrick McHardy <kaber@xxxxxxxxx> wrote: > > > On 21.04, Florian Westphal wrote: > > > > Pablo suggested to re-use the immediate attributes already used by > > > > nft_immediate, nft_bitwise and nft_cmp to re-use as much code as > > > > possible. > > > > > > > > Just add new NFTA_CT_IMM that contains nested data attributes. > > > > We can then use nft_data_init and nft_data_dump for this as well. > > > > > > What's the argument against using immediate and a register? > > > > http://marc.info/?l=netfilter-devel&m=145800804914781&w=2 > > > > <quote> > > > However, with nft, the input is just a register with arbitrary runtime > > > content. > > > > > > We therefore ask for the upper ceiling we currently have, which is > > > enough room to store 128 bits. > > > > We can probably allow passing the label value as attribute to the > > nft_ct expression so you don't have to use the upper ceiling. Patrick > > suggested something similar for nft_ct set helper support. > > </unqote> > > Helpers are somewhat special because we need to load the modules and get > a reference to the helper structure, so we need the context of what the > immediate will be used for. I'd certainly prefer not to use immediates > since that means we'll need a single rule per assignment. Especially with > helpers it seems a lot nicer to simply use a map. Okay, fair enough. > The alternative to internally handling it would be to some propagating > validation to immediates / sets which invoke the actual user of the data. > So in the case of helpers, we could replace the name by references to > the helper structures and reverse this during dumping. > > Regarding connlabels this doesn't really apply though. We expect userspace > to create a reasonable ruleset and anything that does not cause critical > errors is validated in userspace. Yes. So we have three choices here (pseudo-code) memcpy(ct->labels, regs->data[priv->sreg], sizeof(reg)); vs. set_bit(priv->imm, ct->labels); The latter is what the iptables module does, I do not mind if we go for #1 (treat the label area just like an 128bit register and replace it completely with whatever is in the source register). My only problem is that Pablo suggested #2 whereas you recommend #1. I don't want to resubmit until there is consensus as to what the preferred solution is. We could go for a 3rd alternative, namely: u16 bit = regs->data[priv->sreg]; set_bit(bit, ct->labels); i.e. have userspace place the _bit_ that we want to set in the source register. If we go for sreg that would be my favored solution. The only drawback vs #1 is that get and set work differently (get places all labels into dreg, set expects bit to set). (We also need to validate at eval time but thats not a problem in this case). -- 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
