Hi Tomasz, On Mon, Aug 12, 2013 at 10:54:27AM +0300, Tomasz Bursztyka wrote: > Hi Pablo, > > >I like patches from 11 to 13, that refactorization save us quite some > >code and the result in one single parsing and we also work with the > >command structure. > > > >Please, can you send me these three patches in first place? > > Not impossible but means a big rebase of this patchset. > (it currently needs translation stuff, or then it would require some > work to handle target/matches properly as original code does not in > nft_rule_to_iptables_command_state, without solving pure-nft > expressed extensions of course) > > Could you recheck in detail patches 3/16 and 4/16? > If there is no flaws in the translation engine, we might just go for > it at it is. I'm not convinced that this approach is the way to go. In the payload case, the number of instruction tuples will explode, and you will have to iterate many times to find the corresponding parser. I started a quick patch this weekend based on this, the initial idea is the following: 1) Assuming we use a common path to translate rules from nft to xt in nft_rule_to_iptables_command_state, that function invokes a parser depending on the instruction: +void nft_parse_nat(struct nft_parse_ctx *ctx, struct nft_rule_expr *e, + struct nft_rule_expr_iter *iter, int family, + struct iptables_command_state *cs) +{ + int nat_type; + struct nf_nat_range range; + uint32_t reg; + + nat_type = nft_rule_expr_get_u32(e, NFT_EXPR_NAT_TYPE); + + reg = nft_rule_expr_get_u32(e, NFT_EXPR_NAT_REG_ADDR_MIN); + if (reg) + range.min_addr.ip = ctx->registers[reg]; + + reg = nft_rule_expr_get_u32(e, NFT_EXPR_NAT_REG_ADDR_MAX); + if (reg) { + range.flags |= NF_NAT_RANGE_MAP_IPS; + range.max_addr.ip = ctx->registers[reg]; + } + + reg = nft_rule_expr_get_u32(e, NFT_EXPR_NAT_REG_PROTO_MIN); + if (reg) + range.min_proto.all = ctx->registers[reg]; + + reg = nft_rule_expr_get_u32(e, NFT_EXPR_NAT_REG_PROTO_MAX); + if (reg) { + range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED; + range.max_proto.all = ctx->registers[reg]; + } + + nft_nat_dispatcher(nat_type, family, &range); +} 2) Then, this parser generates a structure that contains what most NAT extension need. The immediate parser stores the values in the ctx->registers, so the NAT will find what it needs there. The dispatcher is specific per instruction, so we can look up for the corresponding xt extension base on some specific tuple, in this case, the family and the nat type: +static void nft_nat_dispatcher(enum nft_nat_types nat_type, int fa + struct nf_nat_range *range) +{ + struct nft_xt_nat *xt_nat; + + list_for_each_entry(xt_nat, &nft_xt_nat_list, head) { + if (xt_nat->family == family && + xt_nat->nat_type == nat_type) { + xt_nat->parse(range); + return; + } + } +} This is currently a list, but we could implement it using a hashtable in the payload case. The xt_nat->parse function is defined in the corresponding xt extension. Anyway, I really think we have to start by converting nft to ipt command state in one single patch, as your patches 11-13 do, we need it for whatever approach we decide to follow. If you don't have time to make that rebase, I'll try to find some spare time to work on it. Let me know. 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