[PATCH nft v3 0/6] add and check dump files for JSON in tests/shell

[PATCH nft v3 0/6] add and check dump files for JSON in tests/shell

[Thomas Haller]

Changes since v2:

- rebased on master, and generated new tests.
- reword commit messages.
- add "json-sanitize-ruleset.sh" script as internal helper (this will
  also be used by other tests, that want to compare the output of
  `nft -j list ruleset`).
- bugfix "test-wrapper.sh"

Thomas Haller (6):
  json: fix use after free in table_flags_json()
  tests/shell: check and generate JSON dump files
  tests/shell: add JSON dump files
  tools: simplify error handling in "check-tree.sh" by adding
    msg_err()/msg_warn()
  tools: check more strictly for bash shebang in "check-tree.sh"
  tools: check for consistency of .json-nft dumps in "check-tree.sh"

 src/json.c                                    |   2 +-
 tests/shell/helpers/json-sanitize-ruleset.sh  |  23 +++
 tests/shell/helpers/test-wrapper.sh           | 143 +++++++++++++-----
 tests/shell/run-tests.sh                      |  11 +-
 .../bitwise/dumps/0040mark_binop_0.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_1.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_2.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_3.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_4.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_5.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_6.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_7.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_8.json-nft   |   1 +
 .../bitwise/dumps/0040mark_binop_9.json-nft   |   1 +
 .../bogons/dumps/assert_failures.json-nft     |   1 +
 .../dumps/0001_cache_handling_0.json-nft      |   1 +
 .../cache/dumps/0002_interval_0.json-nft      |   1 +
 .../cache/dumps/0003_cache_update_0.json-nft  |   1 +
 .../cache/dumps/0004_cache_update_0.json-nft  |   1 +
 .../dumps/0005_cache_chain_flush.json-nft     |   1 +
 .../dumps/0006_cache_table_flush.json-nft     |   1 +
 .../dumps/0007_echo_cache_init_0.json-nft     |   1 +
 .../dumps/0008_delete_by_handle_0.json-nft    |   1 +
 ...0009_delete_by_handle_incorrect_0.json-nft |   1 +
 .../cache/dumps/0011_index_0.json-nft         |   1 +
 .../chains/dumps/0001jumps_0.json-nft         |   1 +
 .../chains/dumps/0002jumps_1.json-nft         |   1 +
 .../chains/dumps/0003jump_loop_1.json-nft     |   1 +
 .../chains/dumps/0004busy_1.json-nft          |   1 +
 .../chains/dumps/0005busy_map_1.json-nft      |   1 +
 .../chains/dumps/0006masquerade_0.json-nft    |   1 +
 .../chains/dumps/0007masquerade_1.json-nft    |   1 +
 .../dumps/0008masquerade_jump_1.json-nft      |   1 +
 .../dumps/0009masquerade_jump_1.json-nft      |   1 +
 .../dumps/0010endless_jump_loop_1.json-nft    |   1 +
 .../chains/dumps/0013rename_0.json-nft        |   1 +
 .../chains/dumps/0014rename_0.json-nft        |   1 +
 .../dumps/0015check_jump_loop_1.json-nft      |   1 +
 .../chains/dumps/0016delete_handle_0.json-nft |   1 +
 .../dumps/0017masquerade_jump_1.json-nft      |   1 +
 .../dumps/0018check_jump_loop_1.json-nft      |   1 +
 .../dumps/0019masquerade_jump_1.json-nft      |   1 +
 .../chains/dumps/0020depth_1.json-nft         |   1 +
 .../chains/dumps/0021prio_0.json-nft          |   1 +
 .../chains/dumps/0022prio_dummy_1.json-nft    |   1 +
 .../dumps/0023prio_inet_srcnat_1.json-nft     |   1 +
 .../dumps/0024prio_inet_dstnat_1.json-nft     |   1 +
 .../chains/dumps/0025prio_arp_1.json-nft      |   1 +
 .../chains/dumps/0026prio_netdev_1.json-nft   |   1 +
 .../dumps/0027prio_bridge_dstnat_1.json-nft   |   1 +
 .../dumps/0028prio_bridge_out_1.json-nft      |   1 +
 .../dumps/0029prio_bridge_srcnat_1.json-nft   |   1 +
 .../chains/dumps/0030create_0.json-nft        |   1 +
 .../dumps/0031priority_variable_0.json-nft    |   1 +
 .../dumps/0032priority_variable_0.json-nft    |   1 +
 .../dumps/0033priority_variable_1.json-nft    |   1 +
 .../dumps/0034priority_variable_1.json-nft    |   1 +
 .../dumps/0035policy_variable_0.json-nft      |   1 +
 .../dumps/0036policy_variable_0.json-nft      |   1 +
 .../dumps/0037policy_variable_1.json-nft      |   1 +
 .../dumps/0038policy_variable_1.json-nft      |   1 +
 .../dumps/0039negative_priority_0.json-nft    |   1 +
 .../dumps/0042chain_variable_0.json-nft       |   1 +
 .../chains/dumps/0043chain_ingress_0.json-nft |   1 +
 .../chains/dumps/0044chain_destroy_0.json-nft |   1 +
 .../chains/dumps/netdev_chain_0.json-nft      |   1 +
 .../dumps/netdev_chain_autoremove.json-nft    |   1 +
 .../comments/dumps/comments_0.json-nft        |   1 +
 .../flowtable/dumps/0001flowtable_0.json-nft  |   1 +
 .../dumps/0002create_flowtable_0.json-nft     |   1 +
 .../dumps/0003add_after_flush_0.json-nft      |   1 +
 .../dumps/0004delete_after_add_0.json-nft     |   1 +
 .../dumps/0005delete_in_use_1.json-nft        |   1 +
 .../flowtable/dumps/0006segfault_0.json-nft   |   1 +
 .../flowtable/dumps/0007prio_0.json-nft       |   1 +
 .../flowtable/dumps/0008prio_1.json-nft       |   1 +
 .../dumps/0009deleteafterflush_0.json-nft     |   1 +
 .../dumps/0010delete_handle_0.json-nft        |   1 +
 .../dumps/0011deleteafterflush_0.json-nft     |   1 +
 .../dumps/0012flowtable_variable_0.json-nft   |   1 +
 .../dumps/0013addafterdelete_0.json-nft       |   1 +
 .../dumps/0014addafterdelete_0.json-nft       |   1 +
 .../flowtable/dumps/0015destroy_0.json-nft    |   1 +
 .../include/dumps/0001absolute_0.json-nft     |   1 +
 .../include/dumps/0002relative_0.json-nft     |   1 +
 .../include/dumps/0003includepath_0.json-nft  |   1 +
 .../include/dumps/0004endlessloop_1.json-nft  |   1 +
 .../include/dumps/0005glob_empty_0.json-nft   |   1 +
 .../include/dumps/0006glob_single_0.json-nft  |   1 +
 .../include/dumps/0007glob_double_0.json-nft  |   1 +
 .../dumps/0008glob_nofile_wildcard_0.json-nft |   1 +
 .../include/dumps/0009glob_nofile_1.json-nft  |   1 +
 .../dumps/0010glob_broken_file_1.json-nft     |   1 +
 .../dumps/0011glob_dependency_0.json-nft      |   1 +
 .../dumps/0012glob_dependency_1.json-nft      |   1 +
 .../include/dumps/0013glob_dotfile_0.json-nft |   1 +
 ...nput_descriptors_included_files_0.json-nft |   1 +
 .../dumps/0014glob_directory_0.json-nft       |   1 +
 .../dumps/0015doubleincludepath_0.json-nft    |   1 +
 .../include/dumps/0016maxdepth_0.json-nft     |   1 +
 .../0017glob_more_than_maxdepth_1.json-nft    |   1 +
 .../dumps/0018include_error_0.json-nft        |   1 +
 .../dumps/0019include_error_0.json-nft        |   1 +
 .../dumps/0020include_chain_0.json-nft        |   1 +
 .../json/dumps/0001set_statements_0.json-nft  |   1 +
 .../json/dumps/0002table_map_0.json-nft       |   1 +
 .../dumps/0003json_schema_version_0.json-nft  |   1 +
 .../dumps/0004json_schema_version_1.json-nft  |   1 +
 .../json/dumps/0005secmark_objref_0.json-nft  |   1 +
 .../json/dumps/0006obj_comment_0.json-nft     |   1 +
 .../testcases/json/dumps/netdev.json-nft      |   1 +
 .../listing/dumps/0001ruleset_0.json-nft      |   1 +
 .../listing/dumps/0002ruleset_0.json-nft      |   1 +
 .../listing/dumps/0003table_0.json-nft        |   1 +
 .../listing/dumps/0004table_0.json-nft        |   1 +
 .../listing/dumps/0005ruleset_ip_0.json-nft   |   1 +
 .../listing/dumps/0006ruleset_ip6_0.json-nft  |   1 +
 .../listing/dumps/0007ruleset_inet_0.json-nft |   1 +
 .../listing/dumps/0008ruleset_arp_0.json-nft  |   1 +
 .../dumps/0009ruleset_bridge_0.json-nft       |   1 +
 .../listing/dumps/0010sets_0.json-nft         |   1 +
 .../listing/dumps/0011sets_0.json-nft         |   1 +
 .../listing/dumps/0012sets_0.json-nft         |   1 +
 .../listing/dumps/0014objects_0.json-nft      |   1 +
 .../listing/dumps/0015dynamic_0.json-nft      |   1 +
 .../listing/dumps/0016anonymous_0.json-nft    |   1 +
 .../listing/dumps/0017objects_0.json-nft      |   1 +
 .../listing/dumps/0018data_0.json-nft         |   1 +
 .../listing/dumps/0019set_0.json-nft          |   1 +
 .../listing/dumps/0020flowtable_0.json-nft    |   1 +
 .../dumps/0021ruleset_json_terse_0.json-nft   |   1 +
 .../listing/dumps/0022terse_0.json-nft        |   1 +
 .../0003map_add_many_elements_0.json-nft      |   1 +
 ...5interval_map_add_many_elements_0.json-nft |   1 +
 .../dumps/0006interval_map_overlap_0.json-nft |   1 +
 .../dumps/0007named_ifname_dtype_0.json-nft   |   1 +
 .../dumps/0008interval_map_delete_0.json-nft  |   1 +
 .../testcases/maps/dumps/0009vmap_0.json-nft  |   1 +
 .../maps/dumps/0012map_concat_0.json-nft      |   1 +
 .../testcases/maps/dumps/0013map_0.json-nft   |   1 +
 .../maps/dumps/0014destroy_0.json-nft         |   1 +
 .../maps/dumps/0016map_leak_0.json-nft        |   1 +
 .../maps/dumps/0017_map_variable_0.json-nft   |   1 +
 .../dumps/0018map_leak_timeout_0.json-nft     |   1 +
 .../maps/dumps/anon_objmap_concat.json-nft    |   1 +
 .../maps/dumps/anonymous_snat_map_0.json-nft  |   1 +
 .../maps/dumps/different_map_types_1.json-nft |   1 +
 .../map_catchall_double_deactivate.json-nft   |   1 +
 .../maps/dumps/map_with_flags_0.json-nft      |   1 +
 .../maps/dumps/named_snat_map_0.json-nft      |   1 +
 .../dumps/typeof_maps_add_delete.json-nft     |   1 +
 .../maps/dumps/typeof_maps_update_0.json-nft  |   1 +
 .../netns/dumps/0001nft-f_0.json-nft          |   1 +
 .../netns/dumps/0002loosecommands_0.json-nft  |   1 +
 .../testcases/netns/dumps/0003many_0.json-nft |   1 +
 .../nft-f/dumps/0001define_slash_0.json-nft   |   1 +
 .../nft-f/dumps/0002rollback_rule_0.json-nft  |   1 +
 .../nft-f/dumps/0003rollback_jump_0.json-nft  |   1 +
 .../nft-f/dumps/0004rollback_set_0.json-nft   |   1 +
 .../nft-f/dumps/0005rollback_map_0.json-nft   |   1 +
 .../nft-f/dumps/0006action_object_0.json-nft  |   1 +
 .../0007action_object_set_segfault_1.json-nft |   1 +
 .../nft-f/dumps/0008split_tables_0.json-nft   |   1 +
 .../nft-f/dumps/0009variable_0.json-nft       |   1 +
 .../nft-f/dumps/0010variable_0.json-nft       |   1 +
 .../nft-f/dumps/0013defines_1.json-nft        |   1 +
 .../nft-f/dumps/0014defines_1.json-nft        |   1 +
 .../nft-f/dumps/0015defines_1.json-nft        |   1 +
 .../nft-f/dumps/0016redefines_1.json-nft      |   1 +
 .../dumps/0018ct_expectation_obj_0.json-nft   |   1 +
 .../nft-f/dumps/0018jump_variable_0.json-nft  |   1 +
 .../nft-f/dumps/0019jump_variable_1.json-nft  |   1 +
 .../nft-f/dumps/0020jump_variable_1.json-nft  |   1 +
 .../nft-f/dumps/0021list_ruleset_0.json-nft   |   1 +
 .../nft-f/dumps/0022variables_0.json-nft      |   1 +
 .../nft-f/dumps/0023check_1.json-nft          |   1 +
 .../nft-f/dumps/0025empty_dynset_0.json-nft   |   1 +
 .../nft-f/dumps/0026listing_0.json-nft        |   1 +
 .../nft-f/dumps/0027split_chains_0.json-nft   |   1 +
 .../dumps/0028variable_cmdline_0.json-nft     |   1 +
 .../nft-f/dumps/0029split_file_0.json-nft     |   1 +
 .../nft-f/dumps/0030variable_reuse_0.json-nft |   1 +
 .../nft-f/dumps/0031vmap_string_0.json-nft    |   1 +
 .../nft-f/dumps/0032pknock_0.json-nft         |   1 +
 .../nft-i/dumps/0001define_0.json-nft         |   1 +
 .../dumps/dependency_kill.json-nft            |   1 +
 .../optimizations/dumps/merge_nat.json-nft    |   1 +
 .../dumps/merge_nat_concat.json-nft           |   1 +
 .../optimizations/dumps/merge_reject.json-nft |   1 +
 .../optimizations/dumps/merge_stmts.json-nft  |   1 +
 .../dumps/merge_stmts_concat.json-nft         |   1 +
 .../dumps/merge_stmts_concat_vmap.json-nft    |   1 +
 .../dumps/merge_stmts_vmap.json-nft           |   1 +
 .../dumps/merge_vmap_raw.json-nft             |   1 +
 .../optimizations/dumps/merge_vmaps.json-nft  |   1 +
 .../dumps/not_mergeable.json-nft              |   1 +
 .../optimizations/dumps/ruleset.json-nft      |   1 +
 .../dumps/single_anon_set_expr.json-nft       |   1 +
 .../optimizations/dumps/skip_merge.json-nft   |   1 +
 .../optimizations/dumps/skip_non_eq.json-nft  |   1 +
 .../dumps/skip_unsupported.json-nft           |   1 +
 .../optimizations/dumps/variables.json-nft    |   1 +
 .../optionals/dumps/comments_0.json-nft       |   1 +
 .../optionals/dumps/comments_chain_0.json-nft |   1 +
 .../dumps/comments_handles_0.json-nft         |   1 +
 .../dumps/comments_objects_dup_0.json-nft     |   1 +
 .../optionals/dumps/comments_table_0.json-nft |   1 +
 .../dumps/delete_object_handles_0.json-nft    |   1 +
 .../optionals/dumps/handles_0.json-nft        |   1 +
 .../optionals/dumps/handles_1.json-nft        |   1 +
 .../optionals/dumps/log_prefix_0.json-nft     |   1 +
 .../dumps/update_object_handles_0.json-nft    |   1 +
 .../owner/dumps/0001-flowtable-uaf.json-nft   |   1 +
 .../testcases/parsing/dumps/describe.json-nft |   1 +
 .../parsing/dumps/large_rule_pipe.json-nft    |   1 +
 .../testcases/parsing/dumps/log.json-nft      |   1 +
 .../testcases/parsing/dumps/octal.json-nft    |   1 +
 .../dumps/0001addinsertposition_0.json-nft    |   1 +
 .../dumps/0002addinsertlocation_1.json-nft    |   1 +
 .../dumps/0003insert_0.json-nft               |   1 +
 .../dumps/0004replace_0.json-nft              |   1 +
 .../dumps/0005replace_1.json-nft              |   1 +
 .../dumps/0006replace_1.json-nft              |   1 +
 .../dumps/0007delete_0.json-nft               |   1 +
 .../dumps/0008delete_1.json-nft               |   1 +
 .../dumps/0009delete_1.json-nft               |   1 +
 .../dumps/0010replace_0.json-nft              |   1 +
 .../dumps/0011reset_0.json-nft                |   1 +
 .../dumps/0012destroy_0.json-nft              |   1 +
 .../sets/dumps/0001named_interval_0.json-nft  |   1 +
 .../0002named_interval_automerging_0.json-nft |   1 +
 ...0003named_interval_missing_flag_0.json-nft |   1 +
 .../0004named_interval_shadow_0.json-nft      |   1 +
 .../0005named_interval_shadow_0.json-nft      |   1 +
 .../sets/dumps/0006create_set_0.json-nft      |   1 +
 .../sets/dumps/0007create_element_0.json-nft  |   1 +
 .../dumps/0008comments_interval_0.json-nft    |   1 +
 .../dumps/0009comments_timeout_0.json-nft     |   1 +
 .../sets/dumps/0010comments_0.json-nft        |   1 +
 .../0012add_delete_many_elements_0.json-nft   |   1 +
 .../0013add_delete_many_elements_0.json-nft   |   1 +
 ...014malformed_set_is_not_defined_0.json-nft |   1 +
 .../sets/dumps/0015rulesetflush_0.json-nft    |   1 +
 .../sets/dumps/0016element_leak_0.json-nft    |   1 +
 .../sets/dumps/0017add_after_flush_0.json-nft |   1 +
 .../sets/dumps/0018set_check_size_1.json-nft  |   1 +
 .../sets/dumps/0019set_check_size_0.json-nft  |   1 +
 .../sets/dumps/0020comments_0.json-nft        |   1 +
 .../sets/dumps/0021nesting_0.json-nft         |   1 +
 .../dumps/0022type_selective_flush_0.json-nft |   1 +
 .../0023incomplete_add_set_command_0.json-nft |   1 +
 .../sets/dumps/0025anonymous_set_0.json-nft   |   1 +
 .../sets/dumps/0026named_limit_0.json-nft     |   1 +
 .../sets/dumps/0027ipv6_maps_ipv4_0.json-nft  |   1 +
 .../sets/dumps/0028autoselect_0.json-nft      |   1 +
 .../sets/dumps/0028delete_handle_0.json-nft   |   1 +
 .../dumps/0032restore_set_simple_0.json-nft   |   1 +
 .../dumps/0033add_set_simple_flat_0.json-nft  |   1 +
 .../sets/dumps/0034get_element_0.json-nft     |   1 +
 .../0035add_set_elements_flat_0.json-nft      |   1 +
 .../0037_set_with_inet_service_0.json-nft     |   1 +
 .../sets/dumps/0038meter_list_0.json-nft      |   1 +
 .../sets/dumps/0039delete_interval_0.json-nft |   1 +
 .../0040get_host_endian_elements_0.json-nft   |   1 +
 .../sets/dumps/0041interval_0.json-nft        |   1 +
 .../sets/dumps/0042update_set_0.json-nft      |   1 +
 .../dumps/0043concatenated_ranges_0.json-nft  |   1 +
 .../dumps/0043concatenated_ranges_1.json-nft  |   1 +
 .../dumps/0044interval_overlap_1.json-nft     |   1 +
 .../dumps/0045concat_ipv4_service.json-nft    |   1 +
 .../sets/dumps/0046netmap_0.json-nft          |   1 +
 .../sets/dumps/0048set_counters_0.json-nft    |   1 +
 .../sets/dumps/0049set_define_0.json-nft      |   1 +
 .../sets/dumps/0050set_define_1.json-nft      |   1 +
 .../dumps/0051set_interval_counter_0.json-nft |   1 +
 .../sets/dumps/0052overlap_0.json-nft         |   1 +
 .../testcases/sets/dumps/0053echo_0.json-nft  |   1 +
 .../sets/dumps/0054comments_set_0.json-nft    |   1 +
 .../sets/dumps/0055tcpflags_0.json-nft        |   1 +
 .../sets/dumps/0056dynamic_limit_0.json-nft   |   1 +
 .../dumps/0057set_create_fails_0.json-nft     |   1 +
 .../dumps/0058_setupdate_timeout_0.json-nft   |   1 +
 .../dumps/0059set_update_multistmt_0.json-nft |   1 +
 .../sets/dumps/0060set_multistmt_0.json-nft   |   1 +
 .../sets/dumps/0060set_multistmt_1.json-nft   |   1 +
 .../dumps/0061anonymous_automerge_0.json-nft  |   1 +
 .../sets/dumps/0062set_connlimit_0.json-nft   |   1 +
 .../sets/dumps/0063set_catchall_0.json-nft    |   1 +
 .../sets/dumps/0064map_catchall_0.json-nft    |   1 +
 .../dumps/0065_icmp_postprocessing.json-nft   |   1 +
 .../sets/dumps/0069interval_merge_0.json-nft  |   1 +
 .../0071unclosed_prefix_interval_0.json-nft   |   1 +
 .../sets/dumps/0072destroy_0.json-nft         |   1 +
 .../sets/dumps/0073flat_interval_set.json-nft |   1 +
 .../dumps/0074nested_interval_set.json-nft    |   1 +
 .../sets/dumps/collapse_elem_0.json-nft       |   1 +
 .../sets/dumps/concat_interval_0.json-nft     |   1 +
 .../sets/dumps/dynset_missing.json-nft        |   1 +
 .../testcases/sets/dumps/errors_0.json-nft    |   1 +
 .../sets/dumps/exact_overlap_0.json-nft       |   1 +
 .../testcases/sets/dumps/inner_0.json-nft     |   1 +
 .../testcases/sets/dumps/set_eval_0.json-nft  |   1 +
 .../sets/dumps/type_set_symbol.json-nft       |   1 +
 .../transactions/dumps/0001table_0.json-nft   |   1 +
 .../transactions/dumps/0002table_0.json-nft   |   1 +
 .../transactions/dumps/0003table_0.json-nft   |   1 +
 .../transactions/dumps/0010chain_0.json-nft   |   1 +
 .../transactions/dumps/0011chain_0.json-nft   |   1 +
 .../transactions/dumps/0012chain_0.json-nft   |   1 +
 .../transactions/dumps/0013chain_0.json-nft   |   1 +
 .../transactions/dumps/0014chain_1.json-nft   |   1 +
 .../transactions/dumps/0015chain_0.json-nft   |   1 +
 .../transactions/dumps/0020rule_0.json-nft    |   1 +
 .../transactions/dumps/0021rule_0.json-nft    |   1 +
 .../transactions/dumps/0022rule_1.json-nft    |   1 +
 .../transactions/dumps/0023rule_1.json-nft    |   1 +
 .../transactions/dumps/0024rule_0.json-nft    |   1 +
 .../transactions/dumps/0025rule_0.json-nft    |   1 +
 .../transactions/dumps/0030set_0.json-nft     |   1 +
 .../transactions/dumps/0031set_0.json-nft     |   1 +
 .../transactions/dumps/0032set_0.json-nft     |   1 +
 .../transactions/dumps/0033set_0.json-nft     |   1 +
 .../transactions/dumps/0034set_0.json-nft     |   1 +
 .../transactions/dumps/0035set_0.json-nft     |   1 +
 .../transactions/dumps/0036set_1.json-nft     |   1 +
 .../transactions/dumps/0037set_0.json-nft     |   1 +
 .../transactions/dumps/0038set_0.json-nft     |   1 +
 .../transactions/dumps/0039set_0.json-nft     |   1 +
 .../transactions/dumps/0040set_0.json-nft     |   1 +
 .../dumps/0041nat_restore_0.json-nft          |   1 +
 .../dumps/0042_stateful_expr_0.json-nft       |   1 +
 .../transactions/dumps/0043set_1.json-nft     |   1 +
 .../transactions/dumps/0044rule_0.json-nft    |   1 +
 .../dumps/0045anon-unbind_0.json-nft          |   1 +
 .../transactions/dumps/0046set_0.json-nft     |   1 +
 .../transactions/dumps/0047set_0.json-nft     |   1 +
 .../transactions/dumps/0048helpers_0.json-nft |   1 +
 .../transactions/dumps/0049huge_0.json-nft    |   1 +
 .../transactions/dumps/0050rule_1.json-nft    |   1 +
 .../transactions/dumps/30s-stress.json-nft    |   1 +
 .../dumps/anon_chain_loop.json-nft            |   1 +
 .../dumps/bad_expression.json-nft             |   1 +
 .../transactions/dumps/table_onoff.json-nft   |   1 +
 tools/check-tree.sh                           |  63 +++++---
 344 files changed, 523 insertions(+), 58 deletions(-)
 create mode 100755 tests/shell/helpers/json-sanitize-ruleset.sh
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_0.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_1.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_2.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_3.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_4.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_5.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_6.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_7.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_8.json-nft
 create mode 100644 tests/shell/testcases/bitwise/dumps/0040mark_binop_9.json-nft
 create mode 100644 tests/shell/testcases/bogons/dumps/assert_failures.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0001_cache_handling_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0002_interval_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0003_cache_update_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0004_cache_update_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0005_cache_chain_flush.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0006_cache_table_flush.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0007_echo_cache_init_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0008_delete_by_handle_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0009_delete_by_handle_incorrect_0.json-nft
 create mode 100644 tests/shell/testcases/cache/dumps/0011_index_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0001jumps_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0002jumps_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0003jump_loop_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0004busy_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0005busy_map_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0006masquerade_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0007masquerade_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0008masquerade_jump_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0009masquerade_jump_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0010endless_jump_loop_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0013rename_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0014rename_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0015check_jump_loop_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0016delete_handle_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0017masquerade_jump_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0018check_jump_loop_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0019masquerade_jump_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0020depth_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0021prio_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0022prio_dummy_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0023prio_inet_srcnat_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0024prio_inet_dstnat_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0025prio_arp_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0026prio_netdev_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0027prio_bridge_dstnat_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0028prio_bridge_out_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0029prio_bridge_srcnat_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0030create_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0031priority_variable_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0032priority_variable_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0033priority_variable_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0034priority_variable_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0035policy_variable_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0036policy_variable_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0037policy_variable_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0038policy_variable_1.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0039negative_priority_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0042chain_variable_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0043chain_ingress_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044chain_destroy_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/netdev_chain_0.json-nft
 create mode 100644 tests/shell/testcases/chains/dumps/netdev_chain_autoremove.json-nft
 create mode 100644 tests/shell/testcases/comments/dumps/comments_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0001flowtable_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0002create_flowtable_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0003add_after_flush_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0004delete_after_add_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0005delete_in_use_1.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0006segfault_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0007prio_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0008prio_1.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0009deleteafterflush_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0010delete_handle_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0011deleteafterflush_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0012flowtable_variable_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0013addafterdelete_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0014addafterdelete_0.json-nft
 create mode 100644 tests/shell/testcases/flowtable/dumps/0015destroy_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0001absolute_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0002relative_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0003includepath_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0004endlessloop_1.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0005glob_empty_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0006glob_single_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0007glob_double_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0008glob_nofile_wildcard_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0009glob_nofile_1.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0010glob_broken_file_1.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0011glob_dependency_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0012glob_dependency_1.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0013glob_dotfile_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0013input_descriptors_included_files_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0014glob_directory_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0015doubleincludepath_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0016maxdepth_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0017glob_more_than_maxdepth_1.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0018include_error_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0019include_error_0.json-nft
 create mode 100644 tests/shell/testcases/include/dumps/0020include_chain_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0001set_statements_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0002table_map_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0003json_schema_version_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0004json_schema_version_1.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0005secmark_objref_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0006obj_comment_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/netdev.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0001ruleset_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0002ruleset_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0003table_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0004table_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0005ruleset_ip_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0006ruleset_ip6_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0007ruleset_inet_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0008ruleset_arp_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0009ruleset_bridge_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0010sets_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0011sets_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0012sets_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0014objects_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0015dynamic_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0016anonymous_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0017objects_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0018data_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0019set_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0020flowtable_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0021ruleset_json_terse_0.json-nft
 create mode 100644 tests/shell/testcases/listing/dumps/0022terse_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0003map_add_many_elements_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0005interval_map_add_many_elements_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0006interval_map_overlap_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0007named_ifname_dtype_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0008interval_map_delete_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0009vmap_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0012map_concat_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0013map_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0014destroy_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0016map_leak_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0017_map_variable_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0018map_leak_timeout_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/anon_objmap_concat.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/anonymous_snat_map_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/different_map_types_1.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/map_catchall_double_deactivate.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/map_with_flags_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/named_snat_map_0.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/typeof_maps_add_delete.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/typeof_maps_update_0.json-nft
 create mode 100644 tests/shell/testcases/netns/dumps/0001nft-f_0.json-nft
 create mode 100644 tests/shell/testcases/netns/dumps/0002loosecommands_0.json-nft
 create mode 100644 tests/shell/testcases/netns/dumps/0003many_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0001define_slash_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0002rollback_rule_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0003rollback_jump_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0004rollback_set_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0005rollback_map_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0006action_object_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0007action_object_set_segfault_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0008split_tables_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0009variable_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0010variable_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0013defines_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0014defines_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0015defines_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0016redefines_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0018ct_expectation_obj_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0018jump_variable_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0019jump_variable_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0020jump_variable_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0021list_ruleset_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0022variables_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0023check_1.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0025empty_dynset_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0026listing_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0027split_chains_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0028variable_cmdline_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0029split_file_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0030variable_reuse_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0031vmap_string_0.json-nft
 create mode 100644 tests/shell/testcases/nft-f/dumps/0032pknock_0.json-nft
 create mode 100644 tests/shell/testcases/nft-i/dumps/0001define_0.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/dependency_kill.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_nat.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_nat_concat.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_reject.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_stmts.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_stmts_concat.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_stmts_concat_vmap.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_vmap_raw.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/merge_vmaps.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/not_mergeable.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/ruleset.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/single_anon_set_expr.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_merge.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_non_eq.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/skip_unsupported.json-nft
 create mode 100644 tests/shell/testcases/optimizations/dumps/variables.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_chain_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_handles_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_objects_dup_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/comments_table_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/delete_object_handles_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/handles_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/handles_1.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/log_prefix_0.json-nft
 create mode 100644 tests/shell/testcases/optionals/dumps/update_object_handles_0.json-nft
 create mode 100644 tests/shell/testcases/owner/dumps/0001-flowtable-uaf.json-nft
 create mode 100644 tests/shell/testcases/parsing/dumps/describe.json-nft
 create mode 100644 tests/shell/testcases/parsing/dumps/large_rule_pipe.json-nft
 create mode 100644 tests/shell/testcases/parsing/dumps/log.json-nft
 create mode 100644 tests/shell/testcases/parsing/dumps/octal.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0001addinsertposition_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0002addinsertlocation_1.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0003insert_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0004replace_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0005replace_1.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0006replace_1.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0007delete_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0008delete_1.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0009delete_1.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0010replace_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0011reset_0.json-nft
 create mode 100644 tests/shell/testcases/rule_management/dumps/0012destroy_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0001named_interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0002named_interval_automerging_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0003named_interval_missing_flag_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0004named_interval_shadow_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0005named_interval_shadow_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0006create_set_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0007create_element_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0008comments_interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0009comments_timeout_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0010comments_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0012add_delete_many_elements_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0013add_delete_many_elements_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0014malformed_set_is_not_defined_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0015rulesetflush_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0016element_leak_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0017add_after_flush_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0018set_check_size_1.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0019set_check_size_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0020comments_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0021nesting_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0022type_selective_flush_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0023incomplete_add_set_command_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0025anonymous_set_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0026named_limit_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0027ipv6_maps_ipv4_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0028autoselect_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0028delete_handle_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0032restore_set_simple_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0033add_set_simple_flat_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0034get_element_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0035add_set_elements_flat_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0037_set_with_inet_service_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0038meter_list_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0039delete_interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0040get_host_endian_elements_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0041interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0042update_set_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0043concatenated_ranges_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0043concatenated_ranges_1.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0044interval_overlap_1.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0045concat_ipv4_service.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0046netmap_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0048set_counters_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0049set_define_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0050set_define_1.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0051set_interval_counter_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0052overlap_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0053echo_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0054comments_set_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0056dynamic_limit_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0057set_create_fails_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0058_setupdate_timeout_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0059set_update_multistmt_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0060set_multistmt_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0060set_multistmt_1.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0061anonymous_automerge_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0062set_connlimit_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0063set_catchall_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0064map_catchall_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0065_icmp_postprocessing.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0069interval_merge_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0071unclosed_prefix_interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0072destroy_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0073flat_interval_set.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/0074nested_interval_set.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/collapse_elem_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/concat_interval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/dynset_missing.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/errors_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/exact_overlap_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/inner_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/set_eval_0.json-nft
 create mode 100644 tests/shell/testcases/sets/dumps/type_set_symbol.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0001table_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0002table_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0003table_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0010chain_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0011chain_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0012chain_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0013chain_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0014chain_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0015chain_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0020rule_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0021rule_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0022rule_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0023rule_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0024rule_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0025rule_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0030set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0031set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0032set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0033set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0034set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0035set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0036set_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0037set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0038set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0039set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0040set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0041nat_restore_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0042_stateful_expr_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0043set_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0044rule_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0045anon-unbind_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0046set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0047set_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0048helpers_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0049huge_0.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/0050rule_1.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/30s-stress.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/anon_chain_loop.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/bad_expression.json-nft
 create mode 100644 tests/shell/testcases/transactions/dumps/table_onoff.json-nft

-- 
2.41.0

[PATCH nft v3 1/6] json: fix use after free in table_flags_json()

[Thomas Haller]

Add `$NFT -j list ruleset` to the end of "tests/shell/testcases/transactions/table_onoff".
Then valgrind will find this issue:

  $ make -j && ./tests/shell/run-tests.sh tests/shell/testcases/transactions/table_onoff -V

Gives:

  ==286== Invalid read of size 4
  ==286==    at 0x49B0261: do_dump (dump.c:211)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B04F7: do_dump (dump.c:273)
  ==286==    by 0x49B08B8: do_dump (dump.c:378)
  ==286==    by 0x49B0E84: json_dump_callback (dump.c:465)
  ==286==    by 0x48AF22A: do_command_list_json (json.c:2016)
  ==286==    by 0x48732F1: do_command_list (rule.c:2335)
  ==286==    by 0x48737F5: do_command (rule.c:2605)
  ==286==    by 0x48A867D: nft_netlink (libnftables.c:42)
  ==286==    by 0x48A92B1: nft_run_cmd_from_buffer (libnftables.c:597)
  ==286==    by 0x402CBA: main (main.c:533)

Fixes: e70354f53e9f ("libnftables: Implement JSON output support")
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 src/json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/json.c b/src/json.c
index 23bd247221d3..81328ab3a4e4 100644
--- a/src/json.c
+++ b/src/json.c
@@ -496,7 +496,7 @@ static json_t *table_flags_json(const struct table *table)
 		json_decref(root);
 		return NULL;
 	case 1:
-		json_unpack(root, "[o]", &tmp);
+		json_unpack(root, "[O]", &tmp);
 		json_decref(root);
 		root = tmp;
 		break;
-- 
2.41.0

[PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

The rules after a successful test are good opportunity to test
`nft -j list ruleset` and `nft -j --check`. This quite possibly touches
code paths that are not hit by other tests yet.

The only downside is the increase of the test runtime (which seems
negligible, given the benefits of increasing test coverage).

Future commits will generate and commit those ".json-nft" dump files.

"DUMPGEN=y" will, like before, regenerate only the existing
"*.{nodump,nft,json-nft}" files (unless a test has none of the 3 files,
in which case they are all generated and the user is suggested to commit
the correct ones). Now also "DUMPGEN=all" is honored, that will generate
all 3 files, regardless of whether they already existed. That is useful
if you start out with a test that only has a .nft file, and then you
want to generate a .json-nft file too.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/json-sanitize-ruleset.sh |  23 +++
 tests/shell/helpers/test-wrapper.sh          | 143 ++++++++++++++-----
 tests/shell/run-tests.sh                     |  11 +-
 3 files changed, 138 insertions(+), 39 deletions(-)
 create mode 100755 tests/shell/helpers/json-sanitize-ruleset.sh

diff --git a/tests/shell/helpers/json-sanitize-ruleset.sh b/tests/shell/helpers/json-sanitize-ruleset.sh
new file mode 100755
index 000000000000..270a6107e0aa
--- /dev/null
+++ b/tests/shell/helpers/json-sanitize-ruleset.sh
@@ -0,0 +1,23 @@
+#!/bin/bash -e
+
+die() {
+	printf "%s\n" "$*"
+	exit 1
+}
+
+do_sed() {
+	sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' "$@"
+}
+
+if [ "$#" = 0 ] ; then
+	do_sed
+	exit $?
+fi
+
+for f ; do
+	test -f "$f" || die "$0: file \"$f\" does not exist"
+done
+
+for f ; do
+	do_sed -i "$f" || die "$0: \`sed -i\` failed for \"$f\""
+done
diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index b74c56168768..62414d0db074 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -15,6 +15,16 @@ array_contains() {
 	return 1
 }
 
+show_file() {
+	local filename="$1"
+	shift
+	local msg="$*"
+
+	printf '%s\n>>>>\n' "$msg"
+	cat "$filename"
+	printf "<<<<\n"
+}
+
 TEST="$1"
 TESTBASE="$(basename "$TEST")"
 TESTDIR="$(dirname "$TEST")"
@@ -109,55 +119,108 @@ if [ "$rc_test" -eq 0 ] ; then
 	"${CMD[@]}" &>> "$NFT_TEST_TESTTMPDIR/testout.log" || rc_test=$?
 fi
 
-$NFT list ruleset > "$NFT_TEST_TESTTMPDIR/ruleset-after"
+rc_chkdump=0
+rc=0
+$NFT list ruleset > "$NFT_TEST_TESTTMPDIR/ruleset-after" 2> "$NFT_TEST_TESTTMPDIR/chkdump" || rc=$?
+if [ "$rc" -ne 0 -o -s "$NFT_TEST_TESTTMPDIR/chkdump" ] ; then
+	show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT list ruleset\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+	rc_chkdump=1
+fi
+if [ "$NFT_TEST_HAVE_json" != n ] ; then
+	rc=0
+	$NFT -j list ruleset > "$NFT_TEST_TESTTMPDIR/ruleset-after.json" 2> "$NFT_TEST_TESTTMPDIR/chkdump" || rc=$?
+
+	# Workaround known bug in stmt_print_json(), due to
+	# "chain_stmt_ops.json" being NULL. This spams stderr.
+	sed -i '/^warning: stmt ops chain have no json callback$/d' "$NFT_TEST_TESTTMPDIR/chkdump"
+
+	if [ "$rc" -ne 0 -o -s "$NFT_TEST_TESTTMPDIR/chkdump" ] ; then
+		show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j list ruleset\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+		rc_chkdump=1
+	fi
+	# Normalize the version number from the JSON output. Otherwise, we'd
+	# have to regenerate the .json-nft files upon release.
+	"$NFT_TEST_BASEDIR/helpers/json-sanitize-ruleset.sh" "$NFT_TEST_TESTTMPDIR/ruleset-after.json"
+fi
 
 read tainted_after < /proc/sys/kernel/tainted
 
 DUMPPATH="$TESTDIR/dumps"
 DUMPFILE="$DUMPPATH/$TESTBASE.nft"
+JDUMPFILE="$DUMPPATH/$TESTBASE.json-nft"
 NODUMPFILE="$DUMPPATH/$TESTBASE.nodump"
 
-dump_written=
-
-# The caller can request a re-geneating of the dumps, by setting
-# DUMPGEN=y.
-#
-# This only will happen if the command completed with success.
+# The caller can request a re-geneating of the .nft, .nodump, .json-nft dump files
+# by setting DUMPGEN=y. In that case, only the existing files will be regenerated
+# (unless all three files are missing, in which case all of them are generated).
 #
-# It also will only happen for tests, that have a "$DUMPPATH" directory. There
-# might be tests, that don't want to have dumps created. The existence of the
-# directory controls that. Tests that have a "$NODUMPFILE" file, don't get a dump generated.
-if [ "$rc_test" -eq 0 -a "$DUMPGEN" = y -a -d "$DUMPPATH" -a ! -f "$NODUMPFILE" ] ; then
+# By setting DUMPGEN=all, all 3 files are always regenerated.
+dump_written=n
+if [ "$rc_test" -eq 0 -a '(' "$DUMPGEN" = all -o "$DUMPGEN" = y ')' ] ; then
 	dump_written=y
-	if [ ! -f "$DUMPFILE" ] ; then
-		# No dumpfile exists yet. We generate both a .nft and a .nodump
-		# file. The user can pick which one to commit to git.
+	if [ ! -d "$DUMPPATH" ] ; then
+		mkdir "$DUMPPATH"
+	fi
+	if [ "$DUMPGEN" = all ] ; then
+		gen_nodumpfile=y
+		gen_dumpfile=y
+		gen_jdumpfile=y
+	else
+		# by default, only regenerate the files that we already have on disk.
+		gen_nodumpfile=n
+		gen_dumpfile=n
+		gen_jdumpfile=n
+		test -f "$DUMPFILE"  && gen_dumpfile=y
+		test -f "$JDUMPFILE" && gen_jdumpfile=y
+		test -f "$NODUMPFILE" && gen_nodumpfile=y
+		if [ "$gen_dumpfile" != y -a "$gen_jdumpfile" != y -a "$gen_nodumpfile" != y ] ; then
+			# Except, if no files exist. Them generate all files.
+			gen_dumpfile=y
+			gen_jdumpfile=y
+			gen_nodumpfile=y
+		fi
+	fi
+	if [ "$gen_nodumpfile" = y ] ; then
 		: > "$NODUMPFILE"
 	fi
-	cat "$NFT_TEST_TESTTMPDIR/ruleset-after" > "$DUMPFILE"
+	if [ "$gen_dumpfile" = y ] ; then
+		cat "$NFT_TEST_TESTTMPDIR/ruleset-after" > "$DUMPFILE"
+	fi
+	if [ "$NFT_TEST_HAVE_json" != n -a "$gen_jdumpfile" = y ] ; then
+		cat "$NFT_TEST_TESTTMPDIR/ruleset-after.json" > "$JDUMPFILE"
+	fi
 fi
 
 rc_dump=0
-if [ "$rc_test" -ne 77 -a -f "$DUMPFILE" ] ; then
-	if [ "$dump_written" != y ] ; then
+if [ "$rc_test" -ne 77 -a "$dump_written" != y ] ; then
+	if [ -f "$DUMPFILE" ] ; then
 		if ! $DIFF -u "$DUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff" ; then
+			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff" "Failed \`$DIFF -u \"$DUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
 			rc_dump=1
 		else
 			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff"
 		fi
 	fi
-fi
-if [ "$rc_dump" -ne 0 ] ; then
-	echo "$DUMPFILE" > "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+	if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
+		if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
+			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+			rc_dump=1
+		else
+			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff.json"
+		fi
+	fi
 fi
 
-rc_chkdump=0
 # check that a flush after the test succeeds. We anyway need a clean ruleset
 # for the `nft --check` next.
-$NFT flush ruleset &> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" || rc_chkdump=1
+rc=0
+$NFT flush ruleset &> "$NFT_TEST_TESTTMPDIR/chkdump" || rc=1
+if [ "$rc" = 1 -o -s "$NFT_TEST_TESTTMPDIR/chkdump" ] ; then
+	show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT flush ruleset\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+	rc_chkdump=1
+fi
+# For the dumpfiles, call `nft --check` to possibly cover new code paths.
 if [ -f "$DUMPFILE" ] ; then
-	# We have a dumpfile. Call `nft --check` to possibly cover new code
-	# paths.
 	if [ "$rc_test" -eq 77 ] ; then
 		# The test was skipped. Possibly we don't have the required
 		# features to process this file. Ignore any output and exit
@@ -165,20 +228,30 @@ if [ -f "$DUMPFILE" ] ; then
 		# issue we hope to find).
 		$NFT --check -f "$DUMPFILE" &>/dev/null || :
 	else
-		$NFT --check -f "$DUMPFILE" &>> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" || rc_chkdump=1
+		fail=n
+		$NFT --check -f "$DUMPFILE" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
+		if [ "$fail" = y ] ; then
+			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT --check -f \"$DUMPFILE\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+			rc_chkdump=1
+		fi
+		rm -f "$NFT_TEST_TESTTMPDIR/chkdump"
 	fi
 fi
-if [ -s "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump" ] ; then
-	# Non-empty output? That is wrong.
-	rc_chkdump=1
-elif [ "$rc_chkdump" -eq 0 ] ; then
-	rm -rf "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
-fi
-if [ "$rc_chkdump" -ne 0 ] ; then
-	# Ensure we don't have empty output files. Always write something, so
-	# that `grep ^ -R` lists the file.
-	echo -e "<<<<<\n\nCalling \`nft --check\` (or \`nft flush ruleset\`) failed for \"$DUMPFILE\"" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
+	if [ "$rc_test" -eq 77 ] ; then
+		$NFT -j --check -f "$JDUMPFILE" &>/dev/null || :
+	else
+		fail=n
+		$NFT -j --check -f "$JDUMPFILE" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
+		if [ "$fail" = y ] ; then
+			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$JDUMPFILE\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+			rc_chkdump=1
+		fi
+	fi
 fi
+rm -f "$NFT_TEST_TESTTMPDIR/chkdump"
 
 rc_valgrind=0
 [ -f "$NFT_TEST_TESTTMPDIR/rc-failed-valgrind" ] && rc_valgrind=1
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 27a0ec43042a..3cde97b7ea17 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -184,9 +184,10 @@ usage() {
 	echo " VERBOSE=*|y   : Enable verbose output."
 	echo " NFT_TEST_VERBOSE_TEST=*|y: if true, enable verbose output for tests. For bash scripts, this means"
 	echo "                 to pass \"-x\" to the interpreter."
-	echo " DUMPGEN=*|y   : Regenerate dump files. Dump files are only recreated if the"
-	echo "                 test completes successfully and the \"dumps\" directory for the"
-	echo "                 test exits."
+	echo " DUMPGEN=*|y|all : Regenerate dump files \".{nft,json-nft,nodump}\". \"DUMPGEN=y\" only regenerates existing"
+	echo "                 files, unless the test has no files (then all three files are generated, and you need to"
+	echo "                 choose which to keep). With \"DUMPGEN=all\" all 3 files are regenerated, regardless"
+	echo "                 whether they already exist."
 	echo " VALGRIND=*|y  : Run \$NFT in valgrind."
 	echo " KMEMLEAK=*|y  : Check for kernel memleaks."
 	echo " NFT_TEST_HAS_REALROOT=*|y : To indicate whether the test has real root permissions."
@@ -279,7 +280,9 @@ _NFT_TEST_JOBS_DEFAULT="$(( _NFT_TEST_JOBS_DEFAULT + (_NFT_TEST_JOBS_DEFAULT + 1
 
 VERBOSE="$(bool_y "$VERBOSE")"
 NFT_TEST_VERBOSE_TEST="$(bool_y "$NFT_TEST_VERBOSE_TEST")"
-DUMPGEN="$(bool_y "$DUMPGEN")"
+if [ "$DUMPGEN" != "all" ] ; then
+	DUMPGEN="$(bool_y "$DUMPGEN")"
+fi
 VALGRIND="$(bool_y "$VALGRIND")"
 KMEMLEAK="$(bool_y "$KMEMLEAK")"
 NFT_TEST_KEEP_LOGS="$(bool_y "$NFT_TEST_KEEP_LOGS")"
-- 
2.41.0

[PATCH nft v3 4/6] tools: simplify error handling in "check-tree.sh" by adding msg_err()/msg_warn()

[Thomas Haller]

msg_err() also sets EXIT_CODE=, so we don't have to duplicate this.

Also add msg_warn() to print non-fatal warnings. Will be used in the
future. As "check-tree.sh" tests the consistency of the source tree, a
warning only makes sense to point something out that really should be
fixed, but is not yet.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tools/check-tree.sh | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/tools/check-tree.sh b/tools/check-tree.sh
index e3ddf8bdea58..b16d37c4651b 100755
--- a/tools/check-tree.sh
+++ b/tools/check-tree.sh
@@ -25,6 +25,15 @@ cd "$(dirname "$0")/.."
 
 EXIT_CODE=0
 
+msg_err() {
+	printf "ERR:  %s\n" "$*"
+	EXIT_CODE=1
+}
+
+msg_warn() {
+	printf "WARN: %s\n" "$*"
+}
+
 ##############################################################################
 
 check_shell_dumps() {
@@ -37,8 +46,7 @@ check_shell_dumps() {
 	local nodump_name
 
 	if [ ! -d "$dir/dumps/" ] ; then
-		echo "\"$TEST\" has no \"$dir/dumps/\" directory"
-		EXIT_CODE=1
+		msg_err "\"$TEST\" has no \"$dir/dumps/\" directory"
 		return 0
 	fi
 
@@ -49,34 +57,31 @@ check_shell_dumps() {
 	[ -f "$nodump_name" ] && has_nodump=1
 
 	if [ "$has_nft" != 1 -a "$has_nodump" != 1 ] ; then
-		echo "\"$TEST\" has no \"$dir/dumps/$base.{nft,nodump}\" file"
-		EXIT_CODE=1
+		msg_err "\"$TEST\" has no \"$dir/dumps/$base.{nft,nodump}\" file"
 	elif [ "$has_nft" == 1 -a "$has_nodump" == 1 ] ; then
-		echo "\"$TEST\" has both \"$dir/dumps/$base.{nft,nodump}\" files"
-		EXIT_CODE=1
+		msg_err "\"$TEST\" has both \"$dir/dumps/$base.{nft,nodump}\" files"
 	elif [ "$has_nodump" == 1 -a -s "$nodump_name" ] ; then
-		echo "\"$TEST\" has a non-empty \"$dir/dumps/$base.nodump\" file"
-		EXIT_CODE=1
+		msg_err "\"$TEST\" has a non-empty \"$dir/dumps/$base.nodump\" file"
 	fi
 }
 
 SHELL_TESTS=( $(find "tests/shell/testcases/" -type f -executable | sort) )
 
 if [ "${#SHELL_TESTS[@]}" -eq 0 ] ; then
-	echo "No executable tests under \"tests/shell/testcases/\" found"
-	EXIT_CODE=1
+	msg_err "No executable tests under \"tests/shell/testcases/\" found"
 fi
 for t in "${SHELL_TESTS[@]}" ; do
 	check_shell_dumps "$t"
-	head -n 1 "$t" |grep -q  '^#!/bin/sh' && echo "$t uses sh instead of bash" && EXIT_CODE=1
+	if head -n 1 "$t" |grep -q  '^#!/bin/sh' ; then
+		msg_err "$t uses #!/bin/sh instead of /bin/bash"
+	fi
 done
 
 ##############################################################################
 
 SHELL_TESTS2=( $(./tests/shell/run-tests.sh --list-tests) )
 if [ "${SHELL_TESTS[*]}" != "${SHELL_TESTS2[*]}" ] ; then
-	echo "\`./tests/shell/run-tests.sh --list-tests\` does not list the expected tests"
-	EXIT_CODE=1
+	msg_err "\`./tests/shell/run-tests.sh --list-tests\` does not list the expected tests"
 fi
 
 ##############################################################################
@@ -85,8 +90,7 @@ F=( $(find tests/shell/testcases/ -type f | grep '^tests/shell/testcases/[^/]\+/
 IGNORED_FILES=( tests/shell/testcases/bogons/nft-f/* )
 for f in "${F[@]}" ; do
 	if ! array_contains "$f" "${SHELL_TESTS[@]}" "${IGNORED_FILES[@]}" ; then
-		echo "Unexpected file \"$f\""
-		EXIT_CODE=1
+		msg_err "Unexpected file \"$f\""
 	fi
 done
 
@@ -97,8 +101,7 @@ FILES=( $(find "tests/shell/testcases/" -type f | sed -n 's#\(tests/shell/testca
 for f in "${FILES[@]}" ; do
 	f2="$(echo "$f" | sed -n 's#\(tests/shell/testcases\(/.*\)\?/\)dumps/\(.*\)\.\(nft\|nodump\)$#\1\3#p')"
 	if ! array_contains "$f2" "${SHELL_TESTS[@]}" ; then
-		echo "\"$f\" has no test \"$f2\""
-		EXIT_CODE=1
+		msg_err "\"$f\" has no test \"$f2\""
 	fi
 done
 
-- 
2.41.0

[PATCH nft v3 5/6] tools: check more strictly for bash shebang in "check-tree.sh"

[Thomas Haller]

There is no problem in principle to allow any executable/shebang. However,
it also not clear why we would want to use anything except bash. Unless
we have a good use case, check and reject anything else.

Also not that `./tests/shell/run-tests.sh -x` only works if the shebang
is either exactly "#!/bin/bash" or "#!/bin/bash -e". While it probably
could be made work with other shebangs, the simpler thing is to just use
bash consistently.

Just check that they are all bash scripts. If there ever is a use-case,
we can always adjust this check.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tools/check-tree.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/check-tree.sh b/tools/check-tree.sh
index b16d37c4651b..4be874fcd85e 100755
--- a/tools/check-tree.sh
+++ b/tools/check-tree.sh
@@ -72,8 +72,13 @@ if [ "${#SHELL_TESTS[@]}" -eq 0 ] ; then
 fi
 for t in "${SHELL_TESTS[@]}" ; do
 	check_shell_dumps "$t"
-	if head -n 1 "$t" |grep -q  '^#!/bin/sh' ; then
-		msg_err "$t uses #!/bin/sh instead of /bin/bash"
+	if ! ( head -n 1 "$t" | grep -q '^#!/bin/bash\( -e\)\?$' ) ; then
+		# Currently all tests only use bash as shebang. That also
+		# works with `./tests/shell/run-tests.sh -x`.
+		#
+		# We could allow other shebangs, but for now there is no need.
+		# Unless you have a good reason, create a bash script.
+		msg_err "$t should use either \"#!/bin/bash\" or \"#!/bin/bash -e\" as shebang"
 	fi
 done
 
-- 
2.41.0

[PATCH nft v3 6/6] tools: check for consistency of .json-nft dumps in "check-tree.sh"

[Thomas Haller]

Add checks for the newly introduced .json-nft dump files.

Optimally, every test that has a .nft dump should also have a .json-nft
dump, and vice versa.

However, currently some JSON tests fail to validate, and are missing.
Only flag those missing files as warning, without failing the script.
The reason to warn about this, is that we really should fix those tests,
and having a annoying warning increases the pressure and makes it
discoverable.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tools/check-tree.sh | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/check-tree.sh b/tools/check-tree.sh
index 4be874fcd85e..e358c957857e 100755
--- a/tools/check-tree.sh
+++ b/tools/check-tree.sh
@@ -41,6 +41,7 @@ check_shell_dumps() {
 	local base="$(basename "$TEST")"
 	local dir="$(dirname "$TEST")"
 	local has_nft=0
+	local has_jnft=0
 	local has_nodump=0
 	local nft_name
 	local nodump_name
@@ -51,9 +52,11 @@ check_shell_dumps() {
 	fi
 
 	nft_name="$dir/dumps/$base.nft"
+	jnft_name="$dir/dumps/$base.json-nft"
 	nodump_name="$dir/dumps/$base.nodump"
 
 	[ -f "$nft_name" ] && has_nft=1
+	[ -f "$jnft_name" ] && has_jnft=1
 	[ -f "$nodump_name" ] && has_nodump=1
 
 	if [ "$has_nft" != 1 -a "$has_nodump" != 1 ] ; then
@@ -63,6 +66,22 @@ check_shell_dumps() {
 	elif [ "$has_nodump" == 1 -a -s "$nodump_name" ] ; then
 		msg_err "\"$TEST\" has a non-empty \"$dir/dumps/$base.nodump\" file"
 	fi
+	if [ "$has_jnft" = 1 -a "$has_nft" != 1 ] ; then
+		msg_err "\"$TEST\" has a JSON dump file \"$jnft_name\" but lacks a dump \"$nft_name\""
+	fi
+	if [ "$has_nft" = 1 -a "$has_jnft" != 1 ] ; then
+		# it's currently known that `nft -j --check` cannot parse all dumped rulesets.
+		# Accept having no JSON dump file.
+		#
+		# This should be fixed. Currently this is only a warning.
+		msg_warn "\"$TEST\" has a dump file \"$nft_name\" but lacks a JSON dump \"$jnft_name\""
+	fi
+
+	if [ "$has_jnft" = 1 ] && command -v jq &>/dev/null ; then
+		if ! jq empty < "$jnft_name" &>/dev/null ; then
+			msg_err "\"$TEST\" has a JSON dump file \"$jnft_name\" that does not validate with \`jq empty < \"$jnft_name\"\`"
+		fi
+	fi
 }
 
 SHELL_TESTS=( $(find "tests/shell/testcases/" -type f -executable | sort) )
@@ -91,7 +110,7 @@ fi
 
 ##############################################################################
 #
-F=( $(find tests/shell/testcases/ -type f | grep '^tests/shell/testcases/[^/]\+/dumps/[^/]\+\.\(nft\|nodump\)$' -v | sort) )
+F=( $(find tests/shell/testcases/ -type f | grep '^tests/shell/testcases/[^/]\+/dumps/[^/]\+\.\(json-nft\|nft\|nodump\)$' -v | sort) )
 IGNORED_FILES=( tests/shell/testcases/bogons/nft-f/* )
 for f in "${F[@]}" ; do
 	if ! array_contains "$f" "${SHELL_TESTS[@]}" "${IGNORED_FILES[@]}" ; then
-- 
2.41.0

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Thomas Haller <thaller@redhat.com> wrote:
> The rules after a successful test are good opportunity to test
> `nft -j list ruleset` and `nft -j --check`. This quite possibly touches
> code paths that are not hit by other tests yet.

This series looks good to me, I'll apply it in the next few hours if
noone else takes any action by then.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 09:24:27AM +0100, Florian Westphal wrote:
> Thomas Haller <thaller@redhat.com> wrote:
> > The rules after a successful test are good opportunity to test
> > `nft -j list ruleset` and `nft -j --check`. This quite possibly touches
> > code paths that are not hit by other tests yet.
> 
> This series looks good to me, I'll apply it in the next few hours if
> noone else takes any action by then.

Just a question, patch 3 is missing in patchwork. I guess it is too
big.

My understanding is that this performs the json tests if nft comes with
json support.

I wanted to give this a run, description says a few tests are failing.
Last time we talked it is chain binding support, then there is a good
number of tests that are going to fail (or there is a mechanism to
temporarily disable json tests for this without losing coverage?).

What is the current output from tests? I wanted to make this run
myself so I don't need to ask.

I am asking all this because I am finishing backports for older stable
kernels while this is also going on.

Re: [PATCH nft v3 1/6] json: fix use after free in table_flags_json()

[Pablo Neira Ayuso]

On Tue, Nov 14, 2023 at 04:29:25PM +0100, Thomas Haller wrote:
> Add `$NFT -j list ruleset` to the end of "tests/shell/testcases/transactions/table_onoff".
> Then valgrind will find this issue:
> 
>   $ make -j && ./tests/shell/run-tests.sh tests/shell/testcases/transactions/table_onoff -V
> 
> Gives:
> 
>   ==286== Invalid read of size 4
>   ==286==    at 0x49B0261: do_dump (dump.c:211)
>   ==286==    by 0x49B08B8: do_dump (dump.c:378)
>   ==286==    by 0x49B08B8: do_dump (dump.c:378)
>   ==286==    by 0x49B04F7: do_dump (dump.c:273)
>   ==286==    by 0x49B08B8: do_dump (dump.c:378)
>   ==286==    by 0x49B0E84: json_dump_callback (dump.c:465)
>   ==286==    by 0x48AF22A: do_command_list_json (json.c:2016)
>   ==286==    by 0x48732F1: do_command_list (rule.c:2335)
>   ==286==    by 0x48737F5: do_command (rule.c:2605)
>   ==286==    by 0x48A867D: nft_netlink (libnftables.c:42)
>   ==286==    by 0x48A92B1: nft_run_cmd_from_buffer (libnftables.c:597)
>   ==286==    by 0x402CBA: main (main.c:533)

I have applied this. This fix does not need to wait the entire series.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Nov 15, 2023 at 09:24:27AM +0100, Florian Westphal wrote:
> > Thomas Haller <thaller@redhat.com> wrote:
> > > The rules after a successful test are good opportunity to test
> > > `nft -j list ruleset` and `nft -j --check`. This quite possibly touches
> > > code paths that are not hit by other tests yet.
> > 
> > This series looks good to me, I'll apply it in the next few hours if
> > noone else takes any action by then.
> 
> Just a question, patch 3 is missing in patchwork. I guess it is too
> big.
> 
> My understanding is that this performs the json tests if nft comes with
> json support.
> 
> I wanted to give this a run, description says a few tests are failing.

... but it says that no dump files are added for the failing test cases.

I'll double check this of course before pushing this out.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 11:01:01AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Nov 15, 2023 at 09:24:27AM +0100, Florian Westphal wrote:
> > > Thomas Haller <thaller@redhat.com> wrote:
> > > > The rules after a successful test are good opportunity to test
> > > > `nft -j list ruleset` and `nft -j --check`. This quite possibly touches
> > > > code paths that are not hit by other tests yet.
> > > 
> > > This series looks good to me, I'll apply it in the next few hours if
> > > noone else takes any action by then.
> > 
> > Just a question, patch 3 is missing in patchwork. I guess it is too
> > big.
> > 
> > My understanding is that this performs the json tests if nft comes with
> > json support.
> > 
> > I wanted to give this a run, description says a few tests are failing.
> 
> ... but it says that no dump files are added for the failing test cases.

OK. Then json it is skipped in that case, that is fine.

> I'll double check this of course before pushing this out.

Then, please also disable json dump by now for:

        sets/sets_with_ifnames

because I am currently figuring out how to detach pipapo support from
it without losing coverage.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> because I am currently figuring out how to detach pipapo support from
> it without losing coverage.

Is it worth the pain?  I'd probably lean towards skipping the test
entirely, splitting it in two is possible but we'd end up with quite
some duplicated scripting.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Wed, 2023-11-15 at 10:54 +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 15, 2023 at 09:24:27AM +0100, Florian Westphal wrote:
> > Thomas Haller <thaller@redhat.com> wrote:
> > > The rules after a successful test are good opportunity to test
> > > `nft -j list ruleset` and `nft -j --check`. This quite possibly
> > > touches
> > > code paths that are not hit by other tests yet.
> > 
> > This series looks good to me, I'll apply it in the next few hours
> > if
> > noone else takes any action by then.
> 
> Just a question, patch 3 is missing in patchwork. I guess it is too
> big.


Yes, it's not on the list, as it's too large. I CC-ed you and Florian
on patch 2/6.

You can also find it here:
https://gitlab.freedesktop.org/thaller/nftables/-/commit/b0edc64d005510b8c3db8a8ebe496a8296271bf4.patch

> 
> My understanding is that this performs the json tests if nft comes
> with
> json support.

right.

> 
> I wanted to give this a run, description says a few tests are
> failing.

For tests that fail, the patch 2/6 does not add a `.json-nft` file.

If you get any failure, that's wrong. Then the corresponding .json-nft
should be excluded from patch 2/6.

> Last time we talked it is chain binding support, then there is a good
> number of tests that are going to fail (or there is a mechanism to
> temporarily disable json tests for this without losing coverage?).

If chain binding support is missing, then that is detected via the
common mechanism (NFT_TEST_HAVE_chain_binding=n) and the test will be
marked as SKIPPED.

SKIPPED tests don't get their .nft dump checked. The same for .json-nft
files.

(of course, it also honors NFT_TEST_HAVE_json=n to skip the check).

> 
> What is the current output from tests? I wanted to make this run
> myself so I don't need to ask.

it's the same as with .nft files.

If the .nft/.json-nft dump does not match, the test fails with [DUMP
FAIL]. As always, you can find the result data in /tmp.

I find it most useful to run

  grep --color=always ^ -a -R /tmp/nft-test.latest.*/ | less -R

but whatever works for you.


Also, the test wrapper will call `nft --check -f x.nft` and `nft -j --
check -f x.json-nft` after tests. When those fail, you'll see [CHK
DUMP].

You can see that with:

  DUMPGEN=all ./tests/shell/run-tests.sh tests/shell/testcases/chains/0011endless_jump_loop_1



Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 11:10:20AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > because I am currently figuring out how to detach pipapo support from
> > it without losing coverage.
> 
> Is it worth the pain?  I'd probably lean towards skipping the test
> entirely, splitting it in two is possible but we'd end up with quite
> some duplicated scripting.

I can skip it by now, that is easy, I was just trying not to reduce
coverage.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> I can skip it by now, that is easy, I was just trying not to reduce
> coverage.

Sure, I understand that, but OTOH I think there are limitations
as to what we should provide for, in this case the work/benefit ratio
is quite bad...

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 11:31:12AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I can skip it by now, that is easy, I was just trying not to reduce
> > coverage.
> 
> Sure, I understand that, but OTOH I think there are limitations
> as to what we should provide for, in this case the work/benefit ratio
> is quite bad...

It can be done later, that is what the commit description said.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 11:01:01AM +0100, Florian Westphal wrote:
[...]
> I'll double check this of course before pushing this out.

OK, then all has been clarified and this can follow its route to git.

I will follow up with my pending patches for tests/shell and older
kernels, with my last patch 5/4 it is done for kernel 5.4.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Nov 15, 2023 at 11:01:01AM +0100, Florian Westphal wrote:
> [...]
> > I'll double check this of course before pushing this out.
> 
> OK, then all has been clarified and this can follow its route to git.
> 
> I will follow up with my pending patches for tests/shell and older
> kernels, with my last patch 5/4 it is done for kernel 5.4.

I"ve pushed the series out.  Note that I modified two dump files
to account for "src: expand create commands", which did modify
two tests in the mean time.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Wed, Nov 15, 2023 at 01:21:05PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Nov 15, 2023 at 11:01:01AM +0100, Florian Westphal wrote:
> > [...]
> > > I'll double check this of course before pushing this out.
> > 
> > OK, then all has been clarified and this can follow its route to git.
> > 
> > I will follow up with my pending patches for tests/shell and older
> > kernels, with my last patch 5/4 it is done for kernel 5.4.
> 
> I"ve pushed the series out.  Note that I modified two dump files
> to account for "src: expand create commands", which did modify
> two tests in the mean time.

I see _lots_ of DUMP FAIL with kernel 5.4

I: [SKIPPED]      1/387 testcases/flowtable/0015destroy_0
W: [DUMP FAIL]    2/387 testcases/optimizations/merge_stmts_concat_vmap
I: [OK]           3/387 testcases/chains/0037policy_variable_1
I: [OK]           4/387 testcases/transactions/bad_expression
W: [DUMP FAIL]    5/387 testcases/sets/collapse_elem_0
I: [OK]           6/387 testcases/include/0009glob_nofile_1
W: [DUMP FAIL]    7/387 testcases/sets/0049set_define_0
W: [DUMP FAIL]    8/387 testcases/rule_management/0005replace_1
W: [DUMP FAIL]    9/387 testcases/flowtable/0003add_after_flush_0
W: [DUMP FAIL]   10/387 testcases/sets/0017add_after_flush_0
I: [OK]          11/387 testcases/chains/0011endless_jump_loop_1
I: [SKIPPED]     12/387 testcases/maps/0014destroy_0
I: [SKIPPED]     13/387 testcases/sets/0060set_multistmt_0
W: [DUMP FAIL]   14/387 testcases/cache/0004_cache_update_0
I: [OK]          15/387 testcases/transactions/0036set_1
W: [DUMP FAIL]   16/387 testcases/nft-f/0002rollback_rule_0
W: [DUMP FAIL]   17/387 testcases/chains/0022prio_dummy_1
W: [DUMP FAIL]   18/387 testcases/nft-f/0032pknock_0
I: [SKIPPED]     19/387 testcases/maps/0017_map_variable_0
I: [OK]          20/387 testcases/optionals/comments_objects_dup_0
I: [OK]          21/387 testcases/transactions/0014chain_1
W: [DUMP FAIL]   22/387 testcases/chains/0026prio_netdev_1
W: [DUMP FAIL]   23/387 testcases/optionals/handles_1
W: [DUMP FAIL]   24/387 testcases/include/0007glob_double_0
W: [DUMP FAIL]   25/387 testcases/transactions/0048helpers_0
W: [DUMP FAIL]   26/387 testcases/listing/0021ruleset_json_terse_0
W: [DUMP FAIL]   27/387 testcases/listing/0022terse_0
W: [DUMP FAIL]   28/387 testcases/chains/0017masquerade_jump_1
W: [DUMP FAIL]   29/387 testcases/sets/0016element_leak_0
...

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 15, 2023 at 01:21:05PM +0100, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Wed, Nov 15, 2023 at 11:01:01AM +0100, Florian Westphal wrote:
> > > [...]
> > > > I'll double check this of course before pushing this out.
> > > 
> > > OK, then all has been clarified and this can follow its route to
> > > git.
> > > 
> > > I will follow up with my pending patches for tests/shell and
> > > older
> > > kernels, with my last patch 5/4 it is done for kernel 5.4.
> > 
> > I"ve pushed the series out.  Note that I modified two dump files
> > to account for "src: expand create commands", which did modify
> > two tests in the mean time.
> 
> I see _lots_ of DUMP FAIL with kernel 5.4

Hi,

Could you provide more details?

For example,

    make -j && ./tests/shell/run-tests.sh tests/shell/testcases/include/0007glob_double_0 -x
    grep ^ -a -R /tmp/nft-test.latest.*/


Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

Hi Thomas,

On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
[...]
> > I see _lots_ of DUMP FAIL with kernel 5.4
> 
> Hi,
> 
> Could you provide more details?
> 
> For example,
> 
>     make -j && ./tests/shell/run-tests.sh tests/shell/testcases/include/0007glob_double_0 -x
>     grep ^ -a -R /tmp/nft-test.latest.*/

# cat [...]/ruleset-diff.json
--- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15 13:27:20.272084254 +0100
+++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-include-0007glob_double_0.1/ruleset-after.json      2023-11-16 17:06:18.332535411 +0100
@@ -1 +1 @@
-{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip", "name": "y", "handle": 2}}]}
+{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip", "name": "y", "handle": 159}}]}

It seems that handles are a problem in this diff.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Thu, 2023-11-16 at 17:10 +0100, Pablo Neira Ayuso wrote:
> It seems that handles are a problem in this diff.


Hi,


It's a bit surprising that this happens. Do you use unshare (the
default), to create a separate netns for each test? For me, those
handles then grow incrementally from 1. 


A possible workaround would be the following (and a `DUMPGEN=y` run)



diff --git c/tests/shell/helpers/json-sanitize-ruleset.sh w/tests/shell/helpers/json-sanitize-ruleset.sh
index 270a6107e0aa..22a57d72110e 100755
--- c/tests/shell/helpers/json-sanitize-ruleset.sh
+++ w/tests/shell/helpers/json-sanitize-ruleset.sh
@@ -6,7 +6,12 @@ die() {
 }
 
 do_sed() {
-	sed '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' "$@"
+	# Also normalize all "handle". Optimally, those handles would be stable
+	# and reproducible. However, they are not.
+	sed \
+		-e '1s/\({"nftables": \[{"metainfo": {"version": "\)[0-9.]\+\(", "release_name": "\)[^"]\+\(", "\)/\1VERSION\2RELEASE_NAME\3/' \
+		-e '1s/"handle": [0-9]\+\>/"handle": 1/g' \
+		"$@"
 }
 
 if [ "$#" = 0 ] ; then

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Thu, 2023-11-16 at 17:49 +0100, Thomas Haller wrote:
> On Thu, 2023-11-16 at 17:10 +0100, Pablo Neira Ayuso wrote:
> > It seems that handles are a problem in this diff.
> 
> A possible workaround would be the following (and a `DUMPGEN=y` run)

actually, I think first should be understood why the handles are not
stable. And whether they are stable on recent kernels (as I think they
are supposed to be -- provided the test runs in a new netns).


Also, it seems that `nft -j list ruleset` has a bug and does not honor
(the lack of) the `--handle` option.


Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Thomas,
> 
> On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> [...]
> > > I see _lots_ of DUMP FAIL with kernel 5.4
> > 
> > Hi,
> > 
> > Could you provide more details?
> > 
> > For example,
> > 
> >     make -j && ./tests/shell/run-tests.sh tests/shell/testcases/include/0007glob_double_0 -x
> >     grep ^ -a -R /tmp/nft-test.latest.*/
> 
> # cat [...]/ruleset-diff.json
> --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15 13:27:20.272084254 +0100
> +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-include-0007glob_double_0.1/ruleset-after.json      2023-11-16 17:06:18.332535411 +0100
> @@ -1 +1 @@
> -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip", "name": "y", "handle": 2}}]}
> +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip", "name": "y", "handle": 159}}]}
> 
> It seems that handles are a problem in this diff.

Are you running tests with -s option?

In that case, modules are removed after each test.

I suspect its because we can then hit -EAGAIN mid-transaction
because module is missing (again), then replay logic does its
thing.

But the handle generator isn't transaction aware,
so it has advanced vs. the aborted partial transaction.

I'm not sure what to do here.

One the one hand those rmmods are plain stupid, but on the other
hand this adds partial coverage for the rmmod path.

We could make the handle counter transaction aware to
"fix" this on kernel side; it should not be too much code.

What do you think?

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Thomas,
> > 
> > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > 
> > > Hi,
> > > 
> > > Could you provide more details?
> > > 
> > > For example,
> > > 
> > >     make -j && ./tests/shell/run-tests.sh tests/shell/testcases/include/0007glob_double_0 -x
> > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > 
> > # cat [...]/ruleset-diff.json
> > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15 13:27:20.272084254 +0100
> > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-include-0007glob_double_0.1/ruleset-after.json      2023-11-16 17:06:18.332535411 +0100
> > @@ -1 +1 @@
> > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip", "name": "y", "handle": 2}}]}
> > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip", "name": "y", "handle": 159}}]}
> > 
> > It seems that handles are a problem in this diff.
> 
> Are you running tests with -s option?
> 
> In that case, modules are removed after each test.
> 
> I suspect its because we can then hit -EAGAIN mid-transaction
> because module is missing (again), then replay logic does its
> thing.
> 
> But the handle generator isn't transaction aware,
> so it has advanced vs. the aborted partial transaction.
> 
> I'm not sure what to do here.
> 
> One the one hand those rmmods are plain stupid, but on the other
> hand this adds partial coverage for the rmmod path.
> 
> We could make the handle counter transaction aware to
> "fix" this on kernel side; it should not be too much code.
> 
> What do you think?

Oh, wait, on older kernels the entire handle counter is global,
so "unshare -n" has no effect on it.

But the rmmod scenario explained above happens as well, this
"breaks" json dumps on centos stream 9, which does have the
pernet handle generator fix backported.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Fri, Nov 17, 2023 at 12:00:24AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Thomas,
> > 
> > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > 
> > > Hi,
> > > 
> > > Could you provide more details?
> > > 
> > > For example,
> > > 
> > >     make -j && ./tests/shell/run-tests.sh tests/shell/testcases/include/0007glob_double_0 -x
> > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > 
> > # cat [...]/ruleset-diff.json
> > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15 13:27:20.272084254 +0100
> > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-include-0007glob_double_0.1/ruleset-after.json      2023-11-16 17:06:18.332535411 +0100
> > @@ -1 +1 @@
> > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip", "name": "y", "handle": 2}}]}
> > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name": "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family": "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip", "name": "y", "handle": 159}}]}
> > 
> > It seems that handles are a problem in this diff.
> 
> Are you running tests with -s option?

This is plain run with no options.

> In that case, modules are removed after each test.
> 
> I suspect its because we can then hit -EAGAIN mid-transaction
> because module is missing (again), then replay logic does its
> thing.
> 
> But the handle generator isn't transaction aware,
> so it has advanced vs. the aborted partial transaction.
> 
> I'm not sure what to do here.
> 
> One the one hand those rmmods are plain stupid, but on the other
> hand this adds partial coverage for the rmmod path.
> 
> We could make the handle counter transaction aware to
> "fix" this on kernel side; it should not be too much code.
> 
> What do you think?

I don't think this needs a kernel fix.

The kernel is free to allocate handle, the guarantee is that they are
unique. How this handles are allocated could change in the future.
There is no way userspace can forecast how handles are allocated.

Phil made some code to skip comparing handles in tests/py that I
remember to deal with this.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Thomas,
> > 
> > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > [...]
> > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > 
> > > Hi,
> > > 
> > > Could you provide more details?
> > > 
> > > For example,
> > > 
> > >     make -j && ./tests/shell/run-tests.sh
> > > tests/shell/testcases/include/0007glob_double_0 -x
> > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > 
> > # cat [...]/ruleset-diff.json
> > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15
> > 13:27:20.272084254 +0100
> > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > include-0007glob_double_0.1/ruleset-after.json      2023-11-16
> > 17:06:18.332535411 +0100
> > @@ -1 +1 @@
> > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > "name": "y", "handle": 2}}]}
> > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip",
> > "name": "y", "handle": 159}}]}
> > 
> > It seems that handles are a problem in this diff.
> 
> Are you running tests with -s option?
> 
> In that case, modules are removed after each test.
> 
> I suspect its because we can then hit -EAGAIN mid-transaction
> because module is missing (again), then replay logic does its
> thing.
> 
> But the handle generator isn't transaction aware,
> so it has advanced vs. the aborted partial transaction.

> I'm not sure what to do here.

a combination of:

a) make an effort, that kernel behavior is consistent and reproducible.
Stable output seems important to me, and the automatic loading of a
kernel module should not make a difference. This is IMO a bug.

b) let `nft -j list ruleset` honor (the lack of) `--handle` option and
not print those handles. That bugfix would change behavior, so maybe
instead add a "--no-handle" option for `nft -j` dumps. 

c) sanitize the output with the sed command (my other mail).



This also means, that the .json-nft dumps won't work, if you run
without `unshare`. IMO, the mode without unshare should not be
supported anymore. But if it's deemed important, then it requires b) or
c) or detect the case and skip the diffs with .json-nft.


Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Pablo Neira Ayuso]

On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Hi Thomas,
> > > 
> > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > > [...]
> > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > 
> > > > Hi,
> > > > 
> > > > Could you provide more details?
> > > > 
> > > > For example,
> > > > 
> > > >     make -j && ./tests/shell/run-tests.sh
> > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > 
> > > # cat [...]/ruleset-diff.json
> > > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15
> > > 13:27:20.272084254 +0100
> > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > > include-0007glob_double_0.1/ruleset-after.json      2023-11-16
> > > 17:06:18.332535411 +0100
> > > @@ -1 +1 @@
> > > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > > "name": "y", "handle": 2}}]}
> > > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > > "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip",
> > > "name": "y", "handle": 159}}]}
> > > 
> > > It seems that handles are a problem in this diff.
> > 
> > Are you running tests with -s option?
> > 
> > In that case, modules are removed after each test.
> > 
> > I suspect its because we can then hit -EAGAIN mid-transaction
> > because module is missing (again), then replay logic does its
> > thing.
> > 
> > But the handle generator isn't transaction aware,
> > so it has advanced vs. the aborted partial transaction.
> 
> > I'm not sure what to do here.
> 
> a combination of:
> 
> a) make an effort, that kernel behavior is consistent and reproducible.
> Stable output seems important to me, and the automatic loading of a
> kernel module should not make a difference. This is IMO a bug.

This is not a bug in the kernel. The kernel guarantees that the handle
is unique, but the handle allocation strategy is up to the kernel.
Userspace cannot forecast what handle will get, such thing might lead
to easy to break assumptions from userspace.

> b) let `nft -j list ruleset` honor (the lack of) `--handle` option and
> not print those handles. That bugfix would change behavior, so maybe
> instead add a "--no-handle" option for `nft -j` dumps.

Will honoring -a/--handle break firewalld? I think it is the main user
of the JSON API. That might help disentangle if this makes sense or
not and what the chances of breaking third party applications are.

I'd prefer not to see a --no-handle that will only work for JSON and
that is only useful for this test infrastructure (noone else asked for
this).

> c) sanitize the output with the sed command (my other mail).
>
> This also means, that the .json-nft dumps won't work, if you run
> without `unshare`. IMO, the mode without unshare should not be
> supported anymore. But if it's deemed important, then it requires b) or
> c) or detect the case and skip the diffs with .json-nft.

a) is no-go (kernel update to make test infrastructure or to allow
userspace application to make fragile assumptions on how handles are
allocated is not correct).

b) needs to evaluated, you maintain firewalld, let us know.

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

> 
> 
> Will honoring -a/--handle break firewalld?

firewalld doesn't use the nft command line, only libnftables (via
py/src/nftables.py).

However, on the libnftables API the same problem happens. Namely, that
the nft output by default does not show handles, and you have to opt-in
via NFT_CTX_OUTPUT_HANDLE. On the other hand, the JSON output always
outputs handles. Starting to honor a lack of NFT_CTX_OUTPUT_HANDLE with
JSON output is an obvious change in behavior (well, or rather a
bugfix).


The good new is, that firewalld wouldn't care about that either,
because since forever it calls Nftables.set_handle_output(True) and
always sets NFT_CTX_OUTPUT_HANDLE.



Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Phil Sutter]

On Fri, Nov 17, 2023 at 05:36:23PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> > On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Hi Thomas,
> > > > 
> > > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller wrote:
> > > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > > > [...]
> > > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > Could you provide more details?
> > > > > 
> > > > > For example,
> > > > > 
> > > > >     make -j && ./tests/shell/run-tests.sh
> > > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > > 
> > > > # cat [...]/ruleset-diff.json
> > > > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-11-15
> > > > 13:27:20.272084254 +0100
> > > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > > > include-0007glob_double_0.1/ruleset-after.json      2023-11-16
> > > > 17:06:18.332535411 +0100
> > > > @@ -1 +1 @@
> > > > -{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > > > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > > > "name": "y", "handle": 2}}]}
> > > > +{"nftables": [{"metainfo": {"version": "VERSION", "release_name":
> > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table": {"family":
> > > > "ip", "name": "x", "handle": 158}}, {"table": {"family": "ip",
> > > > "name": "y", "handle": 159}}]}
> > > > 
> > > > It seems that handles are a problem in this diff.
> > > 
> > > Are you running tests with -s option?
> > > 
> > > In that case, modules are removed after each test.
> > > 
> > > I suspect its because we can then hit -EAGAIN mid-transaction
> > > because module is missing (again), then replay logic does its
> > > thing.
> > > 
> > > But the handle generator isn't transaction aware,
> > > so it has advanced vs. the aborted partial transaction.
> > 
> > > I'm not sure what to do here.
> > 
> > a combination of:
> > 
> > a) make an effort, that kernel behavior is consistent and reproducible.
> > Stable output seems important to me, and the automatic loading of a
> > kernel module should not make a difference. This is IMO a bug.
> 
> This is not a bug in the kernel. The kernel guarantees that the handle
> is unique, but the handle allocation strategy is up to the kernel.
> Userspace cannot forecast what handle will get, such thing might lead
> to easy to break assumptions from userspace.
> 
> > b) let `nft -j list ruleset` honor (the lack of) `--handle` option and
> > not print those handles. That bugfix would change behavior, so maybe
> > instead add a "--no-handle" option for `nft -j` dumps.


> 
> Will honoring -a/--handle break firewalld? I think it is the main user
> of the JSON API. That might help disentangle if this makes sense or
> not and what the chances of breaking third party applications are.
> 
> I'd prefer not to see a --no-handle that will only work for JSON and
> that is only useful for this test infrastructure (noone else asked for
> this).
> 
> > c) sanitize the output with the sed command (my other mail).
> >
> > This also means, that the .json-nft dumps won't work, if you run
> > without `unshare`. IMO, the mode without unshare should not be
> > supported anymore. But if it's deemed important, then it requires b) or
> > c) or detect the case and skip the diffs with .json-nft.

What is the problem without unshare? Looking at your patch, it seems
possible to drop the handle attributes in json-sanitize-ruleset.sh.

> a) is no-go (kernel update to make test infrastructure or to allow
> userspace application to make fragile assumptions on how handles are
> allocated is not correct).
> 
> b) needs to evaluated, you maintain firewalld, let us know.

Given the inherent importance of the handle value for ruleset
manipulations, I assume *any* application will need to be updated to
pass --handle (or the libnftables-equivalent) to remain functional.

Cheers, Phil

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Fri, 2023-11-17 at 17:57 +0100, Phil Sutter wrote:
> On Fri, Nov 17, 2023 at 05:36:23PM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> > > On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > Hi Thomas,
> > > > > 
> > > > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller
> > > > > wrote:
> > > > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > > > > [...]
> > > > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > Could you provide more details?
> > > > > > 
> > > > > > For example,
> > > > > > 
> > > > > >     make -j && ./tests/shell/run-tests.sh
> > > > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > > > 
> > > > > # cat [...]/ruleset-diff.json
> > > > > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-
> > > > > 11-15
> > > > > 13:27:20.272084254 +0100
> > > > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > > > > include-0007glob_double_0.1/ruleset-after.json      2023-11-
> > > > > 16
> > > > > 17:06:18.332535411 +0100
> > > > > @@ -1 +1 @@
> > > > > -{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > "release_name":
> > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > {"family":
> > > > > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > > > > "name": "y", "handle": 2}}]}
> > > > > +{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > "release_name":
> > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > {"family":
> > > > > "ip", "name": "x", "handle": 158}}, {"table": {"family":
> > > > > "ip",
> > > > > "name": "y", "handle": 159}}]}
> > > > > 
> > > > > It seems that handles are a problem in this diff.
> > > > 
> > > > Are you running tests with -s option?
> > > > 
> > > > In that case, modules are removed after each test.
> > > > 
> > > > I suspect its because we can then hit -EAGAIN mid-transaction
> > > > because module is missing (again), then replay logic does its
> > > > thing.
> > > > 
> > > > But the handle generator isn't transaction aware,
> > > > so it has advanced vs. the aborted partial transaction.
> > > 
> > > > I'm not sure what to do here.
> > > 
> > > a combination of:
> > > 
> > > a) make an effort, that kernel behavior is consistent and
> > > reproducible.
> > > Stable output seems important to me, and the automatic loading of
> > > a
> > > kernel module should not make a difference. This is IMO a bug.
> > 
> > This is not a bug in the kernel. The kernel guarantees that the
> > handle
> > is unique, but the handle allocation strategy is up to the kernel.
> > Userspace cannot forecast what handle will get, such thing might
> > lead
> > to easy to break assumptions from userspace.
> > 
> > > b) let `nft -j list ruleset` honor (the lack of) `--handle`
> > > option and
> > > not print those handles. That bugfix would change behavior, so
> > > maybe
> > > instead add a "--no-handle" option for `nft -j` dumps.
> 
> 
> > 
> > Will honoring -a/--handle break firewalld? I think it is the main
> > user
> > of the JSON API. That might help disentangle if this makes sense or
> > not and what the chances of breaking third party applications are.
> > 
> > I'd prefer not to see a --no-handle that will only work for JSON
> > and
> > that is only useful for this test infrastructure (noone else asked
> > for
> > this).
> > 
> > > c) sanitize the output with the sed command (my other mail).
> > > 
> > > This also means, that the .json-nft dumps won't work, if you run
> > > without `unshare`. IMO, the mode without unshare should not be
> > > supported anymore. But if it's deemed important, then it requires
> > > b) or
> > > c) or detect the case and skip the diffs with .json-nft.
> 
> What is the problem without unshare? Looking at your patch, it seems
> possible to drop the handle attributes in json-sanitize-ruleset.sh.

Yes, (b) would suffice. I said "or" :)

No further problem, but without-unshare seems not a useful thing to
support. The test-run takes significantly longer, interferes with the
caller's netns and requires CAP_NET_ADMIN.

> 
> > a) is no-go (kernel update to make test infrastructure or to allow
> > userspace application to make fragile assumptions on how handles
> > are
> > allocated is not correct).
> > 
> > b) needs to evaluated, you maintain firewalld, let us know.
> 
> Given the inherent importance of the handle value for ruleset
> manipulations, I assume *any* application will need to be updated to
> pass --handle (or the libnftables-equivalent) to remain functional.

Right. So a "--no-handle" / NFT_CTX_OUTPUT_NO_HANDLE flag for JSON
output?



Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Phil Sutter]

On Fri, Nov 17, 2023 at 06:06:16PM +0100, Thomas Haller wrote:
> On Fri, 2023-11-17 at 17:57 +0100, Phil Sutter wrote:
> > On Fri, Nov 17, 2023 at 05:36:23PM +0100, Pablo Neira Ayuso wrote:
> > > On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> > > > On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > > Hi Thomas,
> > > > > > 
> > > > > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller
> > > > > > wrote:
> > > > > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso wrote:
> > > > > > [...]
> > > > > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > Could you provide more details?
> > > > > > > 
> > > > > > > For example,
> > > > > > > 
> > > > > > >     make -j && ./tests/shell/run-tests.sh
> > > > > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > > > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > > > > 
> > > > > > # cat [...]/ruleset-diff.json
> > > > > > --- testcases/include/dumps/0007glob_double_0.json-nft  2023-
> > > > > > 11-15
> > > > > > 13:27:20.272084254 +0100
> > > > > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-testcases-
> > > > > > include-0007glob_double_0.1/ruleset-after.json      2023-11-
> > > > > > 16
> > > > > > 17:06:18.332535411 +0100
> > > > > > @@ -1 +1 @@
> > > > > > -{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > "release_name":
> > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > {"family":
> > > > > > "ip", "name": "x", "handle": 1}}, {"table": {"family": "ip",
> > > > > > "name": "y", "handle": 2}}]}
> > > > > > +{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > "release_name":
> > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > {"family":
> > > > > > "ip", "name": "x", "handle": 158}}, {"table": {"family":
> > > > > > "ip",
> > > > > > "name": "y", "handle": 159}}]}
> > > > > > 
> > > > > > It seems that handles are a problem in this diff.
> > > > > 
> > > > > Are you running tests with -s option?
> > > > > 
> > > > > In that case, modules are removed after each test.
> > > > > 
> > > > > I suspect its because we can then hit -EAGAIN mid-transaction
> > > > > because module is missing (again), then replay logic does its
> > > > > thing.
> > > > > 
> > > > > But the handle generator isn't transaction aware,
> > > > > so it has advanced vs. the aborted partial transaction.
> > > > 
> > > > > I'm not sure what to do here.
> > > > 
> > > > a combination of:
> > > > 
> > > > a) make an effort, that kernel behavior is consistent and
> > > > reproducible.
> > > > Stable output seems important to me, and the automatic loading of
> > > > a
> > > > kernel module should not make a difference. This is IMO a bug.
> > > 
> > > This is not a bug in the kernel. The kernel guarantees that the
> > > handle
> > > is unique, but the handle allocation strategy is up to the kernel.
> > > Userspace cannot forecast what handle will get, such thing might
> > > lead
> > > to easy to break assumptions from userspace.
> > > 
> > > > b) let `nft -j list ruleset` honor (the lack of) `--handle`
> > > > option and
> > > > not print those handles. That bugfix would change behavior, so
> > > > maybe
> > > > instead add a "--no-handle" option for `nft -j` dumps.
> > 
> > 
> > > 
> > > Will honoring -a/--handle break firewalld? I think it is the main
> > > user
> > > of the JSON API. That might help disentangle if this makes sense or
> > > not and what the chances of breaking third party applications are.
> > > 
> > > I'd prefer not to see a --no-handle that will only work for JSON
> > > and
> > > that is only useful for this test infrastructure (noone else asked
> > > for
> > > this).
> > > 
> > > > c) sanitize the output with the sed command (my other mail).
> > > > 
> > > > This also means, that the .json-nft dumps won't work, if you run
> > > > without `unshare`. IMO, the mode without unshare should not be
> > > > supported anymore. But if it's deemed important, then it requires
> > > > b) or
> > > > c) or detect the case and skip the diffs with .json-nft.
> > 
> > What is the problem without unshare? Looking at your patch, it seems
> > possible to drop the handle attributes in json-sanitize-ruleset.sh.
> 
> Yes, (b) would suffice. I said "or" :)
> 
> No further problem, but without-unshare seems not a useful thing to
> support. The test-run takes significantly longer, interferes with the
> caller's netns and requires CAP_NET_ADMIN.

No, I was wondering why with option (c) "This also means, that the
.json-nft dumps won't work, if you run without `unshare`."

Because I vote for that option. ;)

> > 
> > > a) is no-go (kernel update to make test infrastructure or to allow
> > > userspace application to make fragile assumptions on how handles
> > > are
> > > allocated is not correct).
> > > 
> > > b) needs to evaluated, you maintain firewalld, let us know.
> > 
> > Given the inherent importance of the handle value for ruleset
> > manipulations, I assume *any* application will need to be updated to
> > pass --handle (or the libnftables-equivalent) to remain functional.
> 
> Right. So a "--no-handle" / NFT_CTX_OUTPUT_NO_HANDLE flag for JSON
> output?

Should not be needed. IIUC, the test infrastructure you're about to
introduce sanitizes the JSON output already anyway, right?

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Thomas Haller]

On Fri, 2023-11-17 at 18:11 +0100, Phil Sutter wrote:
> On Fri, Nov 17, 2023 at 06:06:16PM +0100, Thomas Haller wrote:
> > On Fri, 2023-11-17 at 17:57 +0100, Phil Sutter wrote:
> > > On Fri, Nov 17, 2023 at 05:36:23PM +0100, Pablo Neira Ayuso
> > > wrote:
> > > > On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> > > > > On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > > > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > > > Hi Thomas,
> > > > > > > 
> > > > > > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller
> > > > > > > wrote:
> > > > > > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso
> > > > > > > > wrote:
> > > > > > > [...]
> > > > > > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > Could you provide more details?
> > > > > > > > 
> > > > > > > > For example,
> > > > > > > > 
> > > > > > > >     make -j && ./tests/shell/run-tests.sh
> > > > > > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > > > > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > > > > > 
> > > > > > > # cat [...]/ruleset-diff.json
> > > > > > > --- testcases/include/dumps/0007glob_double_0.json-nft 
> > > > > > > 2023-
> > > > > > > 11-15
> > > > > > > 13:27:20.272084254 +0100
> > > > > > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-
> > > > > > > testcases-
> > > > > > > include-0007glob_double_0.1/ruleset-after.json      2023-
> > > > > > > 11-
> > > > > > > 16
> > > > > > > 17:06:18.332535411 +0100
> > > > > > > @@ -1 +1 @@
> > > > > > > -{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > > "release_name":
> > > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > > {"family":
> > > > > > > "ip", "name": "x", "handle": 1}}, {"table": {"family":
> > > > > > > "ip",
> > > > > > > "name": "y", "handle": 2}}]}
> > > > > > > +{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > > "release_name":
> > > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > > {"family":
> > > > > > > "ip", "name": "x", "handle": 158}}, {"table": {"family":
> > > > > > > "ip",
> > > > > > > "name": "y", "handle": 159}}]}
> > > > > > > 
> > > > > > > It seems that handles are a problem in this diff.
> > > > > > 
> > > > > > Are you running tests with -s option?
> > > > > > 
> > > > > > In that case, modules are removed after each test.
> > > > > > 
> > > > > > I suspect its because we can then hit -EAGAIN mid-
> > > > > > transaction
> > > > > > because module is missing (again), then replay logic does
> > > > > > its
> > > > > > thing.
> > > > > > 
> > > > > > But the handle generator isn't transaction aware,
> > > > > > so it has advanced vs. the aborted partial transaction.
> > > > > 
> > > > > > I'm not sure what to do here.
> > > > > 
> > > > > a combination of:
> > > > > 
> > > > > a) make an effort, that kernel behavior is consistent and
> > > > > reproducible.
> > > > > Stable output seems important to me, and the automatic
> > > > > loading of
> > > > > a
> > > > > kernel module should not make a difference. This is IMO a
> > > > > bug.
> > > > 
> > > > This is not a bug in the kernel. The kernel guarantees that the
> > > > handle
> > > > is unique, but the handle allocation strategy is up to the
> > > > kernel.
> > > > Userspace cannot forecast what handle will get, such thing
> > > > might
> > > > lead
> > > > to easy to break assumptions from userspace.
> > > > 
> > > > > b) let `nft -j list ruleset` honor (the lack of) `--handle`
> > > > > option and
> > > > > not print those handles. That bugfix would change behavior,
> > > > > so
> > > > > maybe
> > > > > instead add a "--no-handle" option for `nft -j` dumps.
> > > 
> > > 
> > > > 
> > > > Will honoring -a/--handle break firewalld? I think it is the
> > > > main
> > > > user
> > > > of the JSON API. That might help disentangle if this makes
> > > > sense or
> > > > not and what the chances of breaking third party applications
> > > > are.
> > > > 
> > > > I'd prefer not to see a --no-handle that will only work for
> > > > JSON
> > > > and
> > > > that is only useful for this test infrastructure (noone else
> > > > asked
> > > > for
> > > > this).
> > > > 
> > > > > c) sanitize the output with the sed command (my other mail).
> > > > > 
> > > > > This also means, that the .json-nft dumps won't work, if you
> > > > > run
> > > > > without `unshare`. IMO, the mode without unshare should not
> > > > > be
> > > > > supported anymore. But if it's deemed important, then it
> > > > > requires
> > > > > b) or
> > > > > c) or detect the case and skip the diffs with .json-nft.
> > > 
> > > What is the problem without unshare? Looking at your patch, it
> > > seems
> > > possible to drop the handle attributes in json-sanitize-
> > > ruleset.sh.
> > 
> > Yes, (b) would suffice. I said "or" :)
> > 
> > No further problem, but without-unshare seems not a useful thing to
> > support. The test-run takes significantly longer, interferes with
> > the
> > caller's netns and requires CAP_NET_ADMIN.
> 
> No, I was wondering why with option (c) "This also means, that the
> .json-nft dumps won't work, if you run without `unshare`."
> 
> Because I vote for that option. ;)

Yes, sorry. I got confused with my own numbering :)

I meant also c)


> > > 
> > > > a) is no-go (kernel update to make test infrastructure or to
> > > > allow
> > > > userspace application to make fragile assumptions on how
> > > > handles
> > > > are
> > > > allocated is not correct).
> > > > 
> > > > b) needs to evaluated, you maintain firewalld, let us know.
> > > 
> > > Given the inherent importance of the handle value for ruleset
> > > manipulations, I assume *any* application will need to be updated
> > > to
> > > pass --handle (or the libnftables-equivalent) to remain
> > > functional.
> > 
> > Right. So a "--no-handle" / NFT_CTX_OUTPUT_NO_HANDLE flag for JSON
> > output?
> 
> Should not be needed. IIUC, the test infrastructure you're about to
> introduce sanitizes the JSON output already anyway, right?

Right. c) alone may very well suffice.

I just sent a patch to that amount.


I still think that `nft -j` ignoring the lack of "--no-handle" /
NFT_CTX_OUTPUT_NO_HANDLE is a bug. At the very last a documentation
bug.


Thomas

Re: [PATCH nft v3 2/6] tests/shell: check and generate JSON dump files

[Phil Sutter]

On Fri, Nov 17, 2023 at 06:23:35PM +0100, Thomas Haller wrote:
> On Fri, 2023-11-17 at 18:11 +0100, Phil Sutter wrote:
> > On Fri, Nov 17, 2023 at 06:06:16PM +0100, Thomas Haller wrote:
> > > On Fri, 2023-11-17 at 17:57 +0100, Phil Sutter wrote:
> > > > On Fri, Nov 17, 2023 at 05:36:23PM +0100, Pablo Neira Ayuso
> > > > wrote:
> > > > > On Fri, Nov 17, 2023 at 05:16:02PM +0100, Thomas Haller wrote:
> > > > > > On Fri, 2023-11-17 at 00:00 +0100, Florian Westphal wrote:
> > > > > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > > > > Hi Thomas,
> > > > > > > > 
> > > > > > > > On Wed, Nov 15, 2023 at 01:36:40PM +0100, Thomas Haller
> > > > > > > > wrote:
> > > > > > > > > On Wed, 2023-11-15 at 13:30 +0100, Pablo Neira Ayuso
> > > > > > > > > wrote:
> > > > > > > > [...]
> > > > > > > > > > I see _lots_ of DUMP FAIL with kernel 5.4
> > > > > > > > > 
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > Could you provide more details?
> > > > > > > > > 
> > > > > > > > > For example,
> > > > > > > > > 
> > > > > > > > >     make -j && ./tests/shell/run-tests.sh
> > > > > > > > > tests/shell/testcases/include/0007glob_double_0 -x
> > > > > > > > >     grep ^ -a -R /tmp/nft-test.latest.*/
> > > > > > > > 
> > > > > > > > # cat [...]/ruleset-diff.json
> > > > > > > > --- testcases/include/dumps/0007glob_double_0.json-nft 
> > > > > > > > 2023-
> > > > > > > > 11-15
> > > > > > > > 13:27:20.272084254 +0100
> > > > > > > > +++ /tmp/nft-test.20231116-170617.584.lrZzMy/test-
> > > > > > > > testcases-
> > > > > > > > include-0007glob_double_0.1/ruleset-after.json      2023-
> > > > > > > > 11-
> > > > > > > > 16
> > > > > > > > 17:06:18.332535411 +0100
> > > > > > > > @@ -1 +1 @@
> > > > > > > > -{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > > > "release_name":
> > > > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > > > {"family":
> > > > > > > > "ip", "name": "x", "handle": 1}}, {"table": {"family":
> > > > > > > > "ip",
> > > > > > > > "name": "y", "handle": 2}}]}
> > > > > > > > +{"nftables": [{"metainfo": {"version": "VERSION",
> > > > > > > > "release_name":
> > > > > > > > "RELEASE_NAME", "json_schema_version": 1}}, {"table":
> > > > > > > > {"family":
> > > > > > > > "ip", "name": "x", "handle": 158}}, {"table": {"family":
> > > > > > > > "ip",
> > > > > > > > "name": "y", "handle": 159}}]}
> > > > > > > > 
> > > > > > > > It seems that handles are a problem in this diff.
> > > > > > > 
> > > > > > > Are you running tests with -s option?
> > > > > > > 
> > > > > > > In that case, modules are removed after each test.
> > > > > > > 
> > > > > > > I suspect its because we can then hit -EAGAIN mid-
> > > > > > > transaction
> > > > > > > because module is missing (again), then replay logic does
> > > > > > > its
> > > > > > > thing.
> > > > > > > 
> > > > > > > But the handle generator isn't transaction aware,
> > > > > > > so it has advanced vs. the aborted partial transaction.
> > > > > > 
> > > > > > > I'm not sure what to do here.
> > > > > > 
> > > > > > a combination of:
> > > > > > 
> > > > > > a) make an effort, that kernel behavior is consistent and
> > > > > > reproducible.
> > > > > > Stable output seems important to me, and the automatic
> > > > > > loading of
> > > > > > a
> > > > > > kernel module should not make a difference. This is IMO a
> > > > > > bug.
> > > > > 
> > > > > This is not a bug in the kernel. The kernel guarantees that the
> > > > > handle
> > > > > is unique, but the handle allocation strategy is up to the
> > > > > kernel.
> > > > > Userspace cannot forecast what handle will get, such thing
> > > > > might
> > > > > lead
> > > > > to easy to break assumptions from userspace.
> > > > > 
> > > > > > b) let `nft -j list ruleset` honor (the lack of) `--handle`
> > > > > > option and
> > > > > > not print those handles. That bugfix would change behavior,
> > > > > > so
> > > > > > maybe
> > > > > > instead add a "--no-handle" option for `nft -j` dumps.
> > > > 
> > > > 
> > > > > 
> > > > > Will honoring -a/--handle break firewalld? I think it is the
> > > > > main
> > > > > user
> > > > > of the JSON API. That might help disentangle if this makes
> > > > > sense or
> > > > > not and what the chances of breaking third party applications
> > > > > are.
> > > > > 
> > > > > I'd prefer not to see a --no-handle that will only work for
> > > > > JSON
> > > > > and
> > > > > that is only useful for this test infrastructure (noone else
> > > > > asked
> > > > > for
> > > > > this).
> > > > > 
> > > > > > c) sanitize the output with the sed command (my other mail).
> > > > > > 
> > > > > > This also means, that the .json-nft dumps won't work, if you
> > > > > > run
> > > > > > without `unshare`. IMO, the mode without unshare should not
> > > > > > be
> > > > > > supported anymore. But if it's deemed important, then it
> > > > > > requires
> > > > > > b) or
> > > > > > c) or detect the case and skip the diffs with .json-nft.
> > > > 
> > > > What is the problem without unshare? Looking at your patch, it
> > > > seems
> > > > possible to drop the handle attributes in json-sanitize-
> > > > ruleset.sh.
> > > 
> > > Yes, (b) would suffice. I said "or" :)
> > > 
> > > No further problem, but without-unshare seems not a useful thing to
> > > support. The test-run takes significantly longer, interferes with
> > > the
> > > caller's netns and requires CAP_NET_ADMIN.
> > 
> > No, I was wondering why with option (c) "This also means, that the
> > .json-nft dumps won't work, if you run without `unshare`."
> > 
> > Because I vote for that option. ;)
> 
> Yes, sorry. I got confused with my own numbering :)
> 
> I meant also c)
> 
> 
> > > > 
> > > > > a) is no-go (kernel update to make test infrastructure or to
> > > > > allow
> > > > > userspace application to make fragile assumptions on how
> > > > > handles
> > > > > are
> > > > > allocated is not correct).
> > > > > 
> > > > > b) needs to evaluated, you maintain firewalld, let us know.
> > > > 
> > > > Given the inherent importance of the handle value for ruleset
> > > > manipulations, I assume *any* application will need to be updated
> > > > to
> > > > pass --handle (or the libnftables-equivalent) to remain
> > > > functional.
> > > 
> > > Right. So a "--no-handle" / NFT_CTX_OUTPUT_NO_HANDLE flag for JSON
> > > output?
> > 
> > Should not be needed. IIUC, the test infrastructure you're about to
> > introduce sanitizes the JSON output already anyway, right?
> 
> Right. c) alone may very well suffice.
> 
> I just sent a patch to that amount.
> 
> 
> I still think that `nft -j` ignoring the lack of "--no-handle" /
> NFT_CTX_OUTPUT_NO_HANDLE is a bug. At the very last a documentation
> bug.

It is per design. Same with --numeric. JSON formatting is meant for
programmatic consumption, no point in increasing readability. I don't
see a reason why one would not want the handle attribute included in
dumps apart from your use-case and there is a solution at hand. See for
instance how nft-test.py strips the handle attribute when comparing JSON
output against the record or creating *.json.got files for missing
records.

Cheers, Phil