Move tunnel object-specific printing into its own function to reduce indenting and obj_print_json() function size. While at it, move declaration of 'geneve' variable on top of the function. Older compilers complain about the declaration inside a switch-case. Fixes: 3a957f8f1ff1e ("tunnel: add tunnel object and statement json support") Signed-off-by: Phil Sutter --- src/json.c | 115 +++++++++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 53 deletions(-) diff --git a/src/json.c b/src/json.c index 7312215dede45..a4927c1ae05c9 100644 --- a/src/json.c +++ b/src/json.c @@ -400,6 +400,67 @@ static json_t *tunnel_erspan_print_json(const struct obj *obj) return tunnel; } +static json_t *tunnel_obj_print_json(struct output_ctx *octx, + const struct obj *obj) +{ + struct tunnel_geneve *geneve; + json_t *tmp, *opts; + + tmp = json_pack("{s:i, s:o, s:o, s:i, s:i, s:i, s:i}", + "id", obj->tunnel.id, + obj->tunnel.src->dtype->type == TYPE_IPADDR ? "src-ipv4" : "src-ipv6", + expr_print_json(obj->tunnel.src, octx), + obj->tunnel.dst->dtype->type == TYPE_IPADDR ? "dst-ipv4" : "dst-ipv6", + expr_print_json(obj->tunnel.dst, octx), + "sport", obj->tunnel.sport, + "dport", obj->tunnel.dport, + "tos", obj->tunnel.tos, + "ttl", obj->tunnel.ttl); + + switch (obj->tunnel.type) { + case TUNNEL_UNSPEC: + break; + case TUNNEL_ERSPAN: + json_object_set_new(tmp, "type", json_string("erspan")); + json_object_set_new(tmp, "tunnel", + tunnel_erspan_print_json(obj)); + break; + case TUNNEL_VXLAN: + json_object_set_new(tmp, "type", json_string("vxlan")); + json_object_set_new(tmp, "tunnel", + json_pack("{s:i}", + "gbp", + obj->tunnel.vxlan.gbp)); + break; + case TUNNEL_GENEVE: + opts = json_array(); + + list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) { + char data_str[256]; + json_t *opt; + int offset; + + data_str[0] = '0'; + data_str[1] = 'x'; + offset = 2; + for (uint32_t i = 0; i < geneve->data_len; i++) + offset += snprintf(data_str + offset, + 3, "%x", geneve->data[i]); + + opt = json_pack("{s:i, s:i, s:s}", + "class", geneve->geneve_class, + "opt-type", geneve->type, + "data", data_str); + json_array_append_new(opts, opt); + } + + json_object_set_new(tmp, "type", json_string("geneve")); + json_object_set_new(tmp, "tunnel", opts); + break; + } + return tmp; +} + static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj, bool delete) { @@ -519,59 +580,7 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj, json_decref(tmp); break; case NFT_OBJECT_TUNNEL: - tmp = json_pack("{s:i, s:o, s:o, s:i, s:i, s:i, s:i}", - "id", obj->tunnel.id, - obj->tunnel.src->dtype->type == TYPE_IPADDR ? "src-ipv4" : "src-ipv6", - expr_print_json(obj->tunnel.src, octx), - obj->tunnel.dst->dtype->type == TYPE_IPADDR ? "dst-ipv4" : "dst-ipv6", - expr_print_json(obj->tunnel.dst, octx), - "sport", obj->tunnel.sport, - "dport", obj->tunnel.dport, - "tos", obj->tunnel.tos, - "ttl", obj->tunnel.ttl); - - switch (obj->tunnel.type) { - case TUNNEL_UNSPEC: - break; - case TUNNEL_ERSPAN: - json_object_set_new(tmp, "type", json_string("erspan")); - json_object_set_new(tmp, "tunnel", - tunnel_erspan_print_json(obj)); - break; - case TUNNEL_VXLAN: - json_object_set_new(tmp, "type", json_string("vxlan")); - json_object_set_new(tmp, "tunnel", - json_pack("{s:i}", - "gbp", - obj->tunnel.vxlan.gbp)); - break; - case TUNNEL_GENEVE: - struct tunnel_geneve *geneve; - json_t *opts = json_array(); - - list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) { - char data_str[256]; - json_t *opt; - int offset; - - data_str[0] = '0'; - data_str[1] = 'x'; - offset = 2; - for (uint32_t i = 0; i < geneve->data_len; i++) - offset += snprintf(data_str + offset, - 3, "%x", geneve->data[i]); - - opt = json_pack("{s:i, s:i, s:s}", - "class", geneve->geneve_class, - "opt-type", geneve->type, - "data", data_str); - json_array_append_new(opts, opt); - } - - json_object_set_new(tmp, "type", json_string("geneve")); - json_object_set_new(tmp, "tunnel", opts); - break; - } + tmp = tunnel_obj_print_json(octx, obj); json_object_update(root, tmp); json_decref(tmp); break; -- 2.54.0 Move 'add tunnel' command argument parsing into its own function to reduce size of json_parse_cmd_add_object(). While at it, move the misc variable declarations on top of the function. Older compilers complain about the declaration inside a switch-case. Fixes: 3a957f8f1ff1e ("tunnel: add tunnel object and statement json support") Signed-off-by: Phil Sutter --- src/parser_json.c | 144 ++++++++++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 68 deletions(-) diff --git a/src/parser_json.c b/src/parser_json.c index f04772a022a0b..bff077d27e6d5 100644 --- a/src/parser_json.c +++ b/src/parser_json.c @@ -3703,6 +3703,81 @@ static int json_parse_tunnel_src_and_dst(struct json_ctx *ctx, return 0; } +static int json_parse_tunnel(struct json_ctx *ctx, + json_t *root, struct obj *obj) +{ + struct tunnel_geneve *geneve; + json_t *tmp_json; + const char *tmp; + json_t *value; + size_t index; + int i, j; + + if (json_parse_tunnel_src_and_dst(ctx, root, obj)) + return 1; + + json_unpack(root, "{s:i}", "id", &obj->tunnel.id); + json_unpack(root, "{s:i}", "sport", &i); + obj->tunnel.sport = i; + json_unpack(root, "{s:i}", "dport", &i); + obj->tunnel.sport = i; + json_unpack(root, "{s:i}", "ttl", &i); + obj->tunnel.ttl = i; + json_unpack(root, "{s:i}", "tos", &i); + obj->tunnel.tos = i; + json_unpack(root, "{s:s}", "type", &tmp); + + obj->tunnel.type = json_parse_tunnel_type(ctx, tmp); + switch (obj->tunnel.type) { + case TUNNEL_UNSPEC: + break; + case TUNNEL_ERSPAN: + return json_parse_tunnel_erspan(ctx, root, obj); + case TUNNEL_VXLAN: + if (json_unpack_err(ctx, root, + "{s:o}", "tunnel", &tmp_json)) + return 1; + + json_unpack(tmp_json, "{s:i}", + "gbp", &obj->tunnel.vxlan.gbp); + break; + case TUNNEL_GENEVE: + if (json_unpack_err(ctx, root, + "{s:o}", "tunnel", &tmp_json)) + return 1; + + json_array_foreach(tmp_json, index, value) { + geneve = xmalloc(sizeof(struct tunnel_geneve)); + if (!geneve) + memory_allocation_error(); + + if (json_unpack_err(ctx, value, "{s:i, s:i, s:s}", + "class", &i, + "opt-type", &j, + "data", &tmp)) { + free(geneve); + return 1; + } + geneve->geneve_class = i; + geneve->type = j; + + if (tunnel_geneve_data_str2array(tmp, + geneve->data, + &geneve->data_len)) { + free(geneve); + return 1; + } + + if (index == 0) + init_list_head(&obj->tunnel.geneve_opts); + + list_add_tail(&geneve->list, &obj->tunnel.geneve_opts); + } + break; + } + return 0; +} + static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx, json_t *root, enum cmd_ops op, enum cmd_obj cmd_obj) @@ -3711,7 +3786,6 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx, uint32_t l3proto = NFPROTO_UNSPEC; int inv = 0, flags = 0, i, j; struct handle h = { 0 }; - json_t *tmp_json; struct obj *obj; if (json_unpack_err(ctx, root, "{s:s, s:s}", @@ -3902,74 +3976,8 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx, case NFT_OBJECT_TUNNEL: cmd_obj = CMD_OBJ_TUNNEL; obj->type = NFT_OBJECT_TUNNEL; - - if (json_parse_tunnel_src_and_dst(ctx, root, obj)) + if (json_parse_tunnel(ctx, root, obj)) goto err_free_obj; - - json_unpack(root, "{s:i}", "id", &obj->tunnel.id); - json_unpack(root, "{s:i}", "sport", &i); - obj->tunnel.sport = i; - json_unpack(root, "{s:i}", "dport", &i); - obj->tunnel.sport = i; - json_unpack(root, "{s:i}", "ttl", &i); - obj->tunnel.ttl = i; - json_unpack(root, "{s:i}", "tos", &i); - obj->tunnel.tos = i; - json_unpack(root, "{s:s}", "type", &tmp); - - obj->tunnel.type = json_parse_tunnel_type(ctx, tmp); - switch (obj->tunnel.type) { - case TUNNEL_UNSPEC: - break; - case TUNNEL_ERSPAN: - if (json_parse_tunnel_erspan(ctx, root, obj)) - goto err_free_obj; - break; - case TUNNEL_VXLAN: - if (json_unpack_err(ctx, root, - "{s:o}", "tunnel", &tmp_json)) - goto err_free_obj; - - json_unpack(tmp_json, "{s:i}", - "gbp", &obj->tunnel.vxlan.gbp); - break; - case TUNNEL_GENEVE: - json_t *value; - size_t index; - - if (json_unpack_err(ctx, root, - "{s:o}", "tunnel", &tmp_json)) - goto err_free_obj; - - json_array_foreach(tmp_json, index, value) { - struct tunnel_geneve *geneve = xmalloc(sizeof(struct tunnel_geneve)); - if (!geneve) - memory_allocation_error(); - - if (json_unpack_err(ctx, value, "{s:i, s:i, s:s}", - "class", &i, - "opt-type", &j, - "data", &tmp)) { - free(geneve); - goto err_free_obj; - } - geneve->geneve_class = i; - geneve->type = j; - - if (tunnel_geneve_data_str2array(tmp, - geneve->data, - &geneve->data_len)) { - free(geneve); - goto err_free_obj; - } - - if (index == 0) - init_list_head(&obj->tunnel.geneve_opts); - - list_add_tail(&geneve->list, &obj->tunnel.geneve_opts); - } - break; - } break; default: BUG("Invalid CMD '%d'", cmd_obj); -- 2.54.0 All tunnel types print the same heading, merge the duplicate code. Signed-off-by: Phil Sutter --- src/rule.c | 64 ++++++++++++++---------------------------------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/src/rule.c b/src/rule.c index c32e08319d149..6a6d8f880d7b9 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1833,10 +1833,14 @@ int tunnel_geneve_data_str2array(const char *hexstr, return 0; } -static void obj_print_comment(const struct obj *obj, - struct print_fmt_options *opts, - struct output_ctx *octx) +static void obj_print_header(const struct obj *obj, + struct print_fmt_options *opts, + struct output_ctx *octx) { + nft_print(octx, " %s {", obj->handle.obj.name); + if (nft_output_handle(octx)) + nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); + if (obj->comment) nft_print(octx, "%s%s%scomment \"%s\"", opts->nl, opts->tab, opts->tab, @@ -1849,11 +1853,7 @@ static void obj_print_data(const struct obj *obj, { switch (obj->type) { case NFT_OBJECT_COUNTER: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); if (nft_output_stateless(octx)) nft_print(octx, "%s", opts->nl); else @@ -1865,11 +1865,7 @@ static void obj_print_data(const struct obj *obj, const char *data_unit; uint64_t bytes; - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab); data_unit = get_rate(obj->quota.bytes, &bytes); nft_print(octx, "%s%" PRIu64 " %s", @@ -1884,20 +1880,12 @@ static void obj_print_data(const struct obj *obj, } break; case NFT_OBJECT_SECMARK: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab); nft_print(octx, "\"%s\"%s", obj->secmark.ctx, opts->nl); break; case NFT_OBJECT_CT_HELPER: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s", opts->nl); nft_print(octx, "%s%stype \"%s\" protocol ", opts->tab, opts->tab, obj->ct_helper.name); @@ -1909,11 +1897,7 @@ static void obj_print_data(const struct obj *obj, opts->stmt_separator); break; case NFT_OBJECT_CT_TIMEOUT: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s", opts->nl); nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab); print_proto_name_proto(obj->ct_timeout.l4proto, octx); @@ -1926,11 +1910,7 @@ static void obj_print_data(const struct obj *obj, obj->ct_timeout.timeout, opts, octx); break; case NFT_OBJECT_CT_EXPECT: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s", opts->nl); nft_print(octx, "%s%sprotocol ", opts->tab, opts->tab); print_proto_name_proto(obj->ct_expect.l4proto, octx); @@ -1956,11 +1936,7 @@ static void obj_print_data(const struct obj *obj, const char *data_unit; uint64_t rate; - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab); switch (obj->limit.type) { case NFT_LIMIT_PKTS: @@ -1994,11 +1970,7 @@ static void obj_print_data(const struct obj *obj, const char *sack_str = synproxy_sack_to_str(flags); const char *ts_str = synproxy_timestamp_to_str(flags); - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); if (flags & NF_SYNPROXY_OPT_MSS) { nft_print(octx, "%s%s%s", opts->nl, opts->tab, opts->tab); @@ -2016,11 +1988,7 @@ static void obj_print_data(const struct obj *obj, } break; case NFT_OBJECT_TUNNEL: - nft_print(octx, " %s {", obj->handle.obj.name); - if (nft_output_handle(octx)) - nft_print(octx, " # handle %" PRIu64, obj->handle.handle.id); - - obj_print_comment(obj, opts, octx); + obj_print_header(obj, opts, octx); nft_print(octx, "%s%s%sid %u", opts->nl, opts->tab, opts->tab, obj->tunnel.id); -- 2.54.0 Move tunnel object printing into its own function to reduce size of obj_print_data(). While at it, merge closing brace printing which also fixes broken syntax in case of unexpected tunnel type. Also move declaration of 'geneve' variable on top of the function. Older compilers complain about the declaration inside a switch-case. Signed-off-by: Phil Sutter --- src/rule.c | 210 +++++++++++++++++++++++++++-------------------------- 1 file changed, 106 insertions(+), 104 deletions(-) diff --git a/src/rule.c b/src/rule.c index 6a6d8f880d7b9..579e65ba19271 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1847,6 +1847,111 @@ static void obj_print_header(const struct obj *obj, obj->comment); } +static void tunnel_obj_print_data(const struct obj *obj, + struct print_fmt_options *opts, + struct output_ctx *octx) +{ + struct tunnel_geneve *geneve; + + obj_print_header(obj, opts, octx); + + nft_print(octx, "%s%s%sid %u", + opts->nl, opts->tab, opts->tab, obj->tunnel.id); + + if (obj->tunnel.src) { + if (obj->tunnel.src->len == 32) { + nft_print(octx, "%s%s%sip saddr ", + opts->nl, opts->tab, opts->tab); + expr_print(obj->tunnel.src, octx); + } else if (obj->tunnel.src->len == 128) { + nft_print(octx, "%s%s%sip6 saddr ", + opts->nl, opts->tab, opts->tab); + expr_print(obj->tunnel.src, octx); + } + } + if (obj->tunnel.dst) { + if (obj->tunnel.dst->len == 32) { + nft_print(octx, "%s%s%sip daddr ", + opts->nl, opts->tab, opts->tab); + expr_print(obj->tunnel.dst, octx); + } else if (obj->tunnel.dst->len == 128) { + nft_print(octx, "%s%s%sip6 daddr ", + opts->nl, opts->tab, opts->tab); + expr_print(obj->tunnel.dst, octx); + } + } + if (obj->tunnel.sport) { + nft_print(octx, "%s%s%ssport %u", + opts->nl, opts->tab, opts->tab, + obj->tunnel.sport); + } + if (obj->tunnel.dport) { + nft_print(octx, "%s%s%sdport %u", + opts->nl, opts->tab, opts->tab, + obj->tunnel.dport); + } + if (obj->tunnel.tos) { + nft_print(octx, "%s%s%stos %u", + opts->nl, opts->tab, opts->tab, + obj->tunnel.tos); + } + if (obj->tunnel.ttl) { + nft_print(octx, "%s%s%sttl %u", + opts->nl, opts->tab, opts->tab, + obj->tunnel.ttl); + } + switch (obj->tunnel.type) { + case TUNNEL_ERSPAN: + nft_print(octx, "%s%s%serspan {", + opts->nl, opts->tab, opts->tab); + nft_print(octx, "%s%s%s%sversion %u", + opts->nl, opts->tab, opts->tab, opts->tab, + obj->tunnel.erspan.version); + if (obj->tunnel.erspan.version == 1) { + nft_print(octx, "%s%s%s%sindex %u", + opts->nl, opts->tab, opts->tab, opts->tab, + obj->tunnel.erspan.v1.index); + } else { + nft_print(octx, "%s%s%s%sdirection %s", + opts->nl, opts->tab, opts->tab, opts->tab, + obj->tunnel.erspan.v2.direction ? "egress" + : "ingress"); + nft_print(octx, "%s%s%s%sid %u", + opts->nl, opts->tab, opts->tab, opts->tab, + obj->tunnel.erspan.v2.hwid); + } + break; + case TUNNEL_VXLAN: + nft_print(octx, "%s%s%svxlan {", + opts->nl, opts->tab, opts->tab); + nft_print(octx, "%s%s%s%sgbp %u", + opts->nl, opts->tab, opts->tab, opts->tab, + obj->tunnel.vxlan.gbp); + break; + case TUNNEL_GENEVE: + nft_print(octx, "%s%s%sgeneve {", opts->nl, opts->tab, opts->tab); + list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) { + char data_str[256]; + int offset = 0; + + for (uint32_t i = 0; i < geneve->data_len; i++) { + offset += snprintf(data_str + offset, + geneve->data_len, + "%x", + geneve->data[i]); + } + nft_print(octx, "%s%s%s%sclass 0x%x opt-type 0x%x data \"0x%s\"", + opts->nl, opts->tab, opts->tab, opts->tab, + geneve->geneve_class, geneve->type, data_str); + + } + break; + default: + break; + } + nft_print(octx, "%s%s%s}", opts->nl, opts->tab, opts->tab); +} + static void obj_print_data(const struct obj *obj, struct print_fmt_options *opts, struct output_ctx *octx) @@ -1988,110 +2093,7 @@ static void obj_print_data(const struct obj *obj, } break; case NFT_OBJECT_TUNNEL: - obj_print_header(obj, opts, octx); - - nft_print(octx, "%s%s%sid %u", - opts->nl, opts->tab, opts->tab, obj->tunnel.id); - - if (obj->tunnel.src) { - if (obj->tunnel.src->len == 32) { - nft_print(octx, "%s%s%sip saddr ", - opts->nl, opts->tab, opts->tab); - expr_print(obj->tunnel.src, octx); - } else if (obj->tunnel.src->len == 128) { - nft_print(octx, "%s%s%sip6 saddr ", - opts->nl, opts->tab, opts->tab); - expr_print(obj->tunnel.src, octx); - } - } - if (obj->tunnel.dst) { - if (obj->tunnel.dst->len == 32) { - nft_print(octx, "%s%s%sip daddr ", - opts->nl, opts->tab, opts->tab); - expr_print(obj->tunnel.dst, octx); - } else if (obj->tunnel.dst->len == 128) { - nft_print(octx, "%s%s%sip6 daddr ", - opts->nl, opts->tab, opts->tab); - expr_print(obj->tunnel.dst, octx); - } - } - if (obj->tunnel.sport) { - nft_print(octx, "%s%s%ssport %u", - opts->nl, opts->tab, opts->tab, - obj->tunnel.sport); - } - if (obj->tunnel.dport) { - nft_print(octx, "%s%s%sdport %u", - opts->nl, opts->tab, opts->tab, - obj->tunnel.dport); - } - if (obj->tunnel.tos) { - nft_print(octx, "%s%s%stos %u", - opts->nl, opts->tab, opts->tab, - obj->tunnel.tos); - } - if (obj->tunnel.ttl) { - nft_print(octx, "%s%s%sttl %u", - opts->nl, opts->tab, opts->tab, - obj->tunnel.ttl); - } - switch (obj->tunnel.type) { - case TUNNEL_ERSPAN: - nft_print(octx, "%s%s%serspan {", - opts->nl, opts->tab, opts->tab); - nft_print(octx, "%s%s%s%sversion %u", - opts->nl, opts->tab, opts->tab, opts->tab, - obj->tunnel.erspan.version); - if (obj->tunnel.erspan.version == 1) { - nft_print(octx, "%s%s%s%sindex %u", - opts->nl, opts->tab, opts->tab, opts->tab, - obj->tunnel.erspan.v1.index); - } else { - nft_print(octx, "%s%s%s%sdirection %s", - opts->nl, opts->tab, opts->tab, opts->tab, - obj->tunnel.erspan.v2.direction ? "egress" - : "ingress"); - nft_print(octx, "%s%s%s%sid %u", - opts->nl, opts->tab, opts->tab, opts->tab, - obj->tunnel.erspan.v2.hwid); - } - nft_print(octx, "%s%s%s}", - opts->nl, opts->tab, opts->tab); - break; - case TUNNEL_VXLAN: - nft_print(octx, "%s%s%svxlan {", - opts->nl, opts->tab, opts->tab); - nft_print(octx, "%s%s%s%sgbp %u", - opts->nl, opts->tab, opts->tab, opts->tab, - obj->tunnel.vxlan.gbp); - nft_print(octx, "%s%s%s}", - opts->nl, opts->tab, opts->tab); - break; - case TUNNEL_GENEVE: - struct tunnel_geneve *geneve; - - nft_print(octx, "%s%s%sgeneve {", opts->nl, opts->tab, opts->tab); - list_for_each_entry(geneve, &obj->tunnel.geneve_opts, list) { - char data_str[256]; - int offset = 0; - - for (uint32_t i = 0; i < geneve->data_len; i++) { - offset += snprintf(data_str + offset, - geneve->data_len, - "%x", - geneve->data[i]); - } - nft_print(octx, "%s%s%s%sclass 0x%x opt-type 0x%x data \"0x%s\"", - opts->nl, opts->tab, opts->tab, opts->tab, - geneve->geneve_class, geneve->type, data_str); - - } - nft_print(octx, "%s%s%s}", opts->nl, opts->tab, opts->tab); - break; - default: - break; - } - + tunnel_obj_print_data(obj, opts, octx); nft_print(octx, "%s", opts->stmt_separator); break; default: -- 2.54.0 Older compilers reject this, so fix up the remaining two cases after previous patches. Fixes: 59f03bf14835f ("tunnel: add geneve support") Signed-off-by: Phil Sutter --- src/mnl.c | 3 +-- src/netlink.c | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mnl.c b/src/mnl.c index b9efd3cfd3cea..364773c5642c6 100644 --- a/src/mnl.c +++ b/src/mnl.c @@ -1544,6 +1544,7 @@ static void obj_tunnel_add_opts(struct nftnl_obj *nlo, struct tunnel *tunnel) { struct nftnl_tunnel_opts *opts; struct nftnl_tunnel_opt *opt; + struct tunnel_geneve *geneve; switch (tunnel->type) { case TUNNEL_ERSPAN: @@ -1594,8 +1595,6 @@ static void obj_tunnel_add_opts(struct nftnl_obj *nlo, struct tunnel *tunnel) nftnl_obj_set_data(nlo, NFTNL_OBJ_TUNNEL_OPTS, &opts, sizeof(struct nftnl_tunnel_opts *)); break; case TUNNEL_GENEVE: - struct tunnel_geneve *geneve; - opts = nftnl_tunnel_opts_alloc(NFTNL_TUNNEL_TYPE_GENEVE); if (!opts) memory_allocation_error(); diff --git a/src/netlink.c b/src/netlink.c index 5e59cb7b2d8f2..5c263f39791f1 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -1905,7 +1905,9 @@ static int obj_parse_udata_cb(const struct nftnl_udata *attr, void *data) static int tunnel_parse_opt_cb(struct nftnl_tunnel_opt *opt, void *data) { + struct tunnel_geneve *geneve; struct obj *obj = data; + const void *gnv_data; switch (nftnl_tunnel_opt_get_type(opt)) { case NFTNL_TUNNEL_TYPE_ERSPAN: @@ -1939,9 +1941,6 @@ static int tunnel_parse_opt_cb(struct nftnl_tunnel_opt *opt, void *data) { } break; case NFTNL_TUNNEL_TYPE_GENEVE: - struct tunnel_geneve *geneve; - const void *data; - if (!obj->tunnel.type) { init_list_head(&obj->tunnel.geneve_opts); obj->tunnel.type = TUNNEL_GENEVE; @@ -1958,11 +1957,11 @@ static int tunnel_parse_opt_cb(struct nftnl_tunnel_opt *opt, void *data) { geneve->geneve_class = nftnl_tunnel_opt_get_u16(opt, NFTNL_TUNNEL_GENEVE_CLASS); if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_GENEVE_DATA)) { - data = nftnl_tunnel_opt_get_data(opt, NFTNL_TUNNEL_GENEVE_DATA, + gnv_data = nftnl_tunnel_opt_get_data(opt, NFTNL_TUNNEL_GENEVE_DATA, &geneve->data_len); - if (!data) + if (!gnv_data) return -1; - memcpy(&geneve->data, data, geneve->data_len); + memcpy(&geneve->data, gnv_data, geneve->data_len); } list_add_tail(&geneve->list, &obj->tunnel.geneve_opts); -- 2.54.0 All the nftnl_obj_get_u*() and nftnl_tunnel_opt_get_u*() functions return 0 if the attribute is not present. Since 'obj' is zeroed upon allocation and no unions are used within per object or tunnel type data, assigning that value won't change behaviour. Keep the check before calling nftnl_obj_tunnel_opts_foreach though. It looks like that function does not check tun->tun_opts before dereferencing it. Also make sure obj->tunnel.{src,dst} is not initialized twice (once for IPv4 and once for IPv6) which happens if merely the _is_set() check is removed. Let netlink_obj_tunnel_parse_addr() choose between two attributes to use and so assign just once to each of the fields. Fixes: 35d9c77c57452 ("src: add tunnel template support") Signed-off-by: Phil Sutter --- src/netlink.c | 106 ++++++++++++++++++++------------------------------ 1 file changed, 42 insertions(+), 64 deletions(-) diff --git a/src/netlink.c b/src/netlink.c index 5c263f39791f1..57f4b7c0edea1 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -1842,7 +1842,7 @@ void netlink_dump_obj(struct nftnl_obj *nln, struct netlink_ctx *ctx) static struct in6_addr all_zeroes; static struct expr * -netlink_obj_tunnel_parse_addr(struct nftnl_obj *nlo, int attr) +netlink_obj_tunnel_parse_addr(struct nftnl_obj *nlo, int attr, int alt_attr) { struct nft_data_delinearize nld; const struct datatype *dtype; @@ -1850,6 +1850,13 @@ netlink_obj_tunnel_parse_addr(struct nftnl_obj *nlo, int attr) struct expr *expr; uint32_t addr; + if (!nftnl_obj_is_set(nlo, attr)) { + if (!nftnl_obj_is_set(nlo, alt_attr)) + return NULL; + else + attr = alt_attr; + } + memset(&nld, 0, sizeof(nld)); switch (attr) { @@ -1912,33 +1919,23 @@ static int tunnel_parse_opt_cb(struct nftnl_tunnel_opt *opt, void *data) { switch (nftnl_tunnel_opt_get_type(opt)) { case NFTNL_TUNNEL_TYPE_ERSPAN: obj->tunnel.type = TUNNEL_ERSPAN; - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_ERSPAN_VERSION)) { - obj->tunnel.erspan.version = - nftnl_tunnel_opt_get_u32(opt, - NFTNL_TUNNEL_ERSPAN_VERSION); - } - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_ERSPAN_V1_INDEX)) { - obj->tunnel.erspan.v1.index = - nftnl_tunnel_opt_get_u32(opt, - NFTNL_TUNNEL_ERSPAN_V1_INDEX); - } - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_ERSPAN_V2_HWID)) { - obj->tunnel.erspan.v2.hwid = - nftnl_tunnel_opt_get_u8(opt, - NFTNL_TUNNEL_ERSPAN_V2_HWID); - } - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_ERSPAN_V2_DIR)) { - obj->tunnel.erspan.v2.direction = - nftnl_tunnel_opt_get_u8(opt, - NFTNL_TUNNEL_ERSPAN_V2_DIR); - } + obj->tunnel.erspan.version = + nftnl_tunnel_opt_get_u32(opt, + NFTNL_TUNNEL_ERSPAN_VERSION); + obj->tunnel.erspan.v1.index = + nftnl_tunnel_opt_get_u32(opt, + NFTNL_TUNNEL_ERSPAN_V1_INDEX); + obj->tunnel.erspan.v2.hwid = + nftnl_tunnel_opt_get_u8(opt, + NFTNL_TUNNEL_ERSPAN_V2_HWID); + obj->tunnel.erspan.v2.direction = + nftnl_tunnel_opt_get_u8(opt, + NFTNL_TUNNEL_ERSPAN_V2_DIR); break; case NFTNL_TUNNEL_TYPE_VXLAN: obj->tunnel.type = TUNNEL_VXLAN; - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_VXLAN_GBP)) { - obj->tunnel.type = TUNNEL_VXLAN; - obj->tunnel.vxlan.gbp = nftnl_tunnel_opt_get_u32(opt, NFTNL_TUNNEL_VXLAN_GBP); - } + obj->tunnel.vxlan.gbp = + nftnl_tunnel_opt_get_u32(opt, NFTNL_TUNNEL_VXLAN_GBP); break; case NFTNL_TUNNEL_TYPE_GENEVE: if (!obj->tunnel.type) { @@ -1950,11 +1947,11 @@ static int tunnel_parse_opt_cb(struct nftnl_tunnel_opt *opt, void *data) { if (!geneve) memory_allocation_error(); - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_GENEVE_TYPE)) - geneve->type = nftnl_tunnel_opt_get_u8(opt, NFTNL_TUNNEL_GENEVE_TYPE); - - if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_GENEVE_CLASS)) - geneve->geneve_class = nftnl_tunnel_opt_get_u16(opt, NFTNL_TUNNEL_GENEVE_CLASS); + geneve->type = + nftnl_tunnel_opt_get_u8(opt, NFTNL_TUNNEL_GENEVE_TYPE); + geneve->geneve_class = + nftnl_tunnel_opt_get_u16(opt, + NFTNL_TUNNEL_GENEVE_CLASS); if (nftnl_tunnel_opt_get_flags(opt) & (1 << NFTNL_TUNNEL_GENEVE_DATA)) { gnv_data = nftnl_tunnel_opt_get_data(opt, NFTNL_TUNNEL_GENEVE_DATA, @@ -2069,40 +2066,21 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx, nftnl_obj_get_u32(nlo, NFTNL_OBJ_SYNPROXY_FLAGS); break; case NFT_OBJECT_TUNNEL: - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_ID)) - obj->tunnel.id = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TUNNEL_ID); - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_SPORT)) { - obj->tunnel.sport = - nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_SPORT); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_DPORT)) { - obj->tunnel.dport = - nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_DPORT); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_TOS)) { - obj->tunnel.tos = - nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_TOS); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_TTL)) { - obj->tunnel.ttl = - nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_TTL); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_IPV4_SRC)) { - obj->tunnel.src = - netlink_obj_tunnel_parse_addr(nlo, NFTNL_OBJ_TUNNEL_IPV4_SRC); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_IPV4_DST)) { - obj->tunnel.dst = - netlink_obj_tunnel_parse_addr(nlo, NFTNL_OBJ_TUNNEL_IPV4_DST); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_IPV6_SRC)) { - obj->tunnel.src = - netlink_obj_tunnel_parse_addr(nlo, NFTNL_OBJ_TUNNEL_IPV6_SRC); - } - if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_IPV6_DST)) { - obj->tunnel.dst = - netlink_obj_tunnel_parse_addr(nlo, NFTNL_OBJ_TUNNEL_IPV6_DST); - } + obj->tunnel.id = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TUNNEL_ID); + obj->tunnel.sport = + nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_SPORT); + obj->tunnel.dport = + nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_DPORT); + obj->tunnel.tos = nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_TOS); + obj->tunnel.ttl = nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_TTL); + obj->tunnel.src = + netlink_obj_tunnel_parse_addr(nlo, + NFTNL_OBJ_TUNNEL_IPV6_SRC, + NFTNL_OBJ_TUNNEL_IPV4_SRC); + obj->tunnel.dst = + netlink_obj_tunnel_parse_addr(nlo, + NFTNL_OBJ_TUNNEL_IPV6_DST, + NFTNL_OBJ_TUNNEL_IPV4_DST); if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_OPTS)) { nftnl_obj_tunnel_opts_foreach(nlo, tunnel_parse_opt_cb, obj); } -- 2.54.0