On Thu, Jan 16, 2014 at 09:46:17PM +0100, Arturo Borrero Gonzalez wrote: > On 16 January 2014 19:05, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > The kernel will complain if we pass invalid combinations, I don't want > > to have this early validation code in the library. > > > > The problem is that as far as I've tested, the kernel unconditionally > returns 'dir' [0]. Then I think we have to fix the kernel, it should not dump an attribute that we don't need. I can see that nft is currently not using the direction at all, so such change should not break anything. > If we print in XML/JSON the data obtained from the kernel, <dir> is > also printed, while it may be totally undesirable (for example, for a > latter parsing of that XML/JSON meant to be resended to the kernel). > I think we need this check, in libnftables or nft. > > I don't see the point of allowing such a disruptive combination of attributes. I agree those combinations don't make sense, but let just the kernel bail out when we pass invalid combinations. Otherwise, the library makes internal decisions that we simply cannot change as we'll have 3rd party userspace application relying on it. And we may want to extend the kernel behaviour in some way that may clash with old libraries. Really, we have to avoid this, it's just troubles in the long run. On the other hand, we should also make sure that the information that we get from the kernel can be reinjected without troubles. So if you note similar issues, please report them. > We already have similar checks in other objects to disallow invalid > combinations, see [1] [2]. > > What do you think? > > > > > Not related to this patch, but better I prefer if you use: > > nft_rule_expr_set_u8(...) instead of these two lines above. > > > > I agree. But I think it would be better if all ops are of the same kind. Agreed. > So I will patch all non-shorcuts ops like this all around libnftables, > unless you say otherwise, before this patch. That will be a large patch, I guess. I prefer if you hold on with that cleanup until we have released the first version which is coming soon. Please, focus on fixes at this stage. -- 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
