On 15 January 2015 at 13:51, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Thu, Jan 15, 2015 at 01:44:16PM +0100, Arturo Borrero Gonzalez wrote:
>> On 15 January 2015 at 13:32, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>> > On Tue, Jan 13, 2015 at 06:36:10PM +0100, Arturo Borrero Gonzalez wrote:
>> >> The commit bc543af ("ebtables-compat: fix segfault in rules w/o target")
>> >> doesn't handle all possible cases of target printing, and ACCEPT is left
>> >> behind.
>> >>
>> >> BTW, the logic of target (-j XXX) printing is a bit weird. This patch
>> >> simplifies it.
>> >>
>> >> I assume:
>> >> * cs->jumpto is only filled by nft_immediate.
>> >> * cs->target is only filled by nft_target.
>> >>
>> >> So we end with these cases:
>> >> * nft_immediate contains a 'standard' target (ACCEPT, DROP, CONTINUE, RETURN, chain)
>> >> Then cs->jumpto contains the target already. We have the rule.
>> >> * No standard target. If nft_target contains a target, try to load it.
>> >> * Neither nft_target nor nft_immediate exist. Then, assume CONTINUE.
>> >>
>> >> The printing path is then straight forward: either cs.jumpto or cs.target
>> >> contains the target.
>> >>
>> >> As there isn't support for target extensions yet, there is no way to test the
>> >> nft_target (cs.target) path.
>> >
>> > Not telling this is wrong, but I guess the resulting code to print the
>> > target has to converge to what we have in iptables-compat (see
>> > iptables/nft-ipv4.c). I mean, the handling should look similar. Could
>> > you revisit that and make sure that this and the existing code
>> > converge to the point? Thanks.
>>
>> I could try to factorize code to a common function, something like:
>> void nft_shared_rule_translate_target(char **jumpto, struct
>> xtables_target **target)
>> void nft_shared_print_target(const char *jumpto, const struct
>> xtables_target *target)
>>
>> Do you like the idea?
>
> Yes, the more we consolidate the less redundancy. Please, for the
> function names, I'd suggest a bit shorter ones: nft_set_target() and
> nft_print_target() I'd say.
Hi Pablo,
reading the code again, I believe code paths we mentioned for
iptables-compat and ebtables-compat are different enough for the code
factorization to not worth it.
However, I think that nft_rule_to_*tables_command_state() is more
suitable for the factorization, but it is a different issue.
let me know.
--
Arturo Borrero González
--
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
