From 091603c4a443eab272c54e509cb1d2663b432da2 Mon Sep 17 00:00:00 2001 From: Neels Janosch Hofmeyr Date: Fri, 3 Feb 2023 02:15:36 +0100 Subject: [PATCH] deprecate cfg 'nft rule tunmap append' Subsequent patch will refactor the tunmap nft ruleset. Instead of adapting the 'tunmap append' feature to the new ruleset, rather drop this feature entirely. The 'nft rule tunmap append' was intended for enabling 'trace' in the nft ruleset. However, the same can be achieved via the nft cmdline tool. For example: sudo nft 'add chain filter trace_chain { type filter hook prerouting priority -301; }' sudo nft 'add rule filter trace_chain meta nftrace set 1' Related: SYS#6327 SYS#6264 Change-Id: I1ae36f2f520217254c81fd765d27333ff0f457b2 --- include/osmocom/upf/upf.h | 3 --- include/osmocom/upf/upf_nft.h | 5 ++-- src/osmo-upf/up_gtp_action.c | 2 +- src/osmo-upf/upf.c | 1 - src/osmo-upf/upf_nft.c | 30 ++++++++-------------- src/osmo-upf/upf_vty.c | 44 ++++++++------------------------ tests/nft-rule.vty | 47 +++-------------------------------- tests/upf.vty | 18 ++++---------- 8 files changed, 33 insertions(+), 117 deletions(-) diff --git a/include/osmocom/upf/upf.h b/include/osmocom/upf/upf.h index c25acca..ef7aa2b 100644 --- a/include/osmocom/upf/upf.h +++ b/include/osmocom/upf/upf.h @@ -112,9 +112,6 @@ struct g_upf { char *table_name; int priority; uint32_t next_id_state; - - /* list of struct string_listitem */ - struct llist_head rule_append; } nft; struct llist_head netinst; diff --git a/include/osmocom/upf/upf_nft.h b/include/osmocom/upf/upf_nft.h index 8333877..4cdcb51 100644 --- a/include/osmocom/upf/upf_nft.h +++ b/include/osmocom/upf/upf_nft.h @@ -49,7 +49,6 @@ struct upf_nft_tunmap_desc { int upf_nft_init(); int upf_nft_free(); -char *upf_nft_tunmap_get_ruleset_str(void *ctx, struct upf_nft_tunmap_desc *tunmap, - const struct llist_head *rule_append); -int upf_nft_tunmap_create(struct upf_nft_tunmap_desc *tunmap, const struct llist_head *rule_append); +char *upf_nft_tunmap_get_ruleset_str(void *ctx, struct upf_nft_tunmap_desc *tunmap); +int upf_nft_tunmap_create(struct upf_nft_tunmap_desc *tunmap); int upf_nft_tunmap_delete(struct upf_nft_tunmap_desc *tunmap); diff --git a/src/osmo-upf/up_gtp_action.c b/src/osmo-upf/up_gtp_action.c index 3b3da86..238d652 100644 --- a/src/osmo-upf/up_gtp_action.c +++ b/src/osmo-upf/up_gtp_action.c @@ -135,7 +135,7 @@ static int up_gtp_action_enable_disable(struct up_gtp_action *a, bool enable) return -ENOENT; } if (enable) - rc = upf_nft_tunmap_create(&a->tunmap, &g_upf->nft.rule_append); + rc = upf_nft_tunmap_create(&a->tunmap); else rc = upf_nft_tunmap_delete(&a->tunmap); if (rc) { diff --git a/src/osmo-upf/upf.c b/src/osmo-upf/upf.c index 1476270..0a84799 100644 --- a/src/osmo-upf/upf.c +++ b/src/osmo-upf/upf.c @@ -61,7 +61,6 @@ void g_upf_alloc(void *ctx) INIT_LLIST_HEAD(&g_upf->gtp.vty_cfg.devs); INIT_LLIST_HEAD(&g_upf->gtp.devs); - INIT_LLIST_HEAD(&g_upf->nft.rule_append); INIT_LLIST_HEAD(&g_upf->netinst); } diff --git a/src/osmo-upf/upf_nft.c b/src/osmo-upf/upf_nft.c index d1a83ff..f30e30c 100644 --- a/src/osmo-upf/upf_nft.c +++ b/src/osmo-upf/upf_nft.c @@ -126,8 +126,7 @@ struct upf_nft_args { static int tunmap_single_direction(char *buf, size_t buflen, const struct upf_nft_args *args, const struct upf_nft_args_peer *from_peer, - const struct upf_nft_args_peer *to_peer, - const struct llist_head *rule_append) + const struct upf_nft_args_peer *to_peer) { struct osmo_strbuf sb = { .buf = buf, .len = buflen }; @@ -156,20 +155,13 @@ static int tunmap_single_direction(char *buf, size_t buflen, OSMO_STRBUF_PRINTF(sb, " counter"); - if (rule_append) { - struct string_listitem *i; - llist_for_each_entry(i, rule_append, entry) - OSMO_STRBUF_PRINTF(sb, " %s", i->str); - } - OSMO_STRBUF_PRINTF(sb, " accept"); OSMO_STRBUF_PRINTF(sb, ";\n"); return sb.chars_needed; } -static int upf_nft_ruleset_tunmap_create_buf(char *buf, size_t buflen, const struct upf_nft_args *args, - const struct llist_head *rule_append) +static int upf_nft_ruleset_tunmap_create_buf(char *buf, size_t buflen, const struct upf_nft_args *args) { struct osmo_strbuf sb = { .buf = buf, .len = buflen }; @@ -178,17 +170,16 @@ static int upf_nft_ruleset_tunmap_create_buf(char *buf, size_t buflen, const str args->table_name, args->chain_id, args->priority); /* Forwarding from peer_a to peer_b */ - OSMO_STRBUF_APPEND(sb, tunmap_single_direction, args, &args->peer_a, &args->peer_b, rule_append); + OSMO_STRBUF_APPEND(sb, tunmap_single_direction, args, &args->peer_a, &args->peer_b); /* And from peer_b to peer_a */ - OSMO_STRBUF_APPEND(sb, tunmap_single_direction, args, &args->peer_b, &args->peer_a, rule_append); + OSMO_STRBUF_APPEND(sb, tunmap_single_direction, args, &args->peer_b, &args->peer_a); return sb.chars_needed; } -static char *upf_nft_ruleset_tunmap_create_c(void *ctx, const struct upf_nft_args *args, - const struct llist_head *rule_append) +static char *upf_nft_ruleset_tunmap_create_c(void *ctx, const struct upf_nft_args *args) { - OSMO_NAME_C_IMPL(ctx, 512, "ERROR", upf_nft_ruleset_tunmap_create_buf, args, rule_append) + OSMO_NAME_C_IMPL(ctx, 512, "ERROR", upf_nft_ruleset_tunmap_create_buf, args) } static int upf_nft_ruleset_tunmap_delete_buf(char *buf, size_t buflen, const struct upf_nft_args *args) @@ -230,8 +221,7 @@ static void upf_nft_args_from_tunmap_desc(struct upf_nft_args *args, const struc }; } -char *upf_nft_tunmap_get_ruleset_str(void *ctx, struct upf_nft_tunmap_desc *tunmap, - const struct llist_head *rule_append) +char *upf_nft_tunmap_get_ruleset_str(void *ctx, struct upf_nft_tunmap_desc *tunmap) { struct upf_nft_args args; @@ -244,12 +234,12 @@ char *upf_nft_tunmap_get_ruleset_str(void *ctx, struct upf_nft_tunmap_desc *tunm } upf_nft_args_from_tunmap_desc(&args, tunmap); - return upf_nft_ruleset_tunmap_create_c(ctx, &args, rule_append); + return upf_nft_ruleset_tunmap_create_c(ctx, &args); } -int upf_nft_tunmap_create(struct upf_nft_tunmap_desc *tunmap, const struct llist_head *rule_append) +int upf_nft_tunmap_create(struct upf_nft_tunmap_desc *tunmap) { - return upf_nft_run(upf_nft_tunmap_get_ruleset_str(OTC_SELECT, tunmap, rule_append)); + return upf_nft_run(upf_nft_tunmap_get_ruleset_str(OTC_SELECT, tunmap)); } int upf_nft_tunmap_delete(struct upf_nft_tunmap_desc *tunmap) diff --git a/src/osmo-upf/upf_vty.c b/src/osmo-upf/upf_vty.c index afea28d..436d91f 100644 --- a/src/osmo-upf/upf_vty.c +++ b/src/osmo-upf/upf_vty.c @@ -222,8 +222,6 @@ DEFUN_CMD_ELEMENT(cfg_tunmap, cfg_nft_cmd, "nft", TUNMAP_NODE_STR, CMD_ATTR_HIDD static int config_write_tunmap(struct vty *vty) { - struct string_listitem *i; - vty_out(vty, "tunmap%s", VTY_NEWLINE); if (g_upf->nft.mockup) @@ -232,8 +230,6 @@ static int config_write_tunmap(struct vty *vty) if (g_upf->nft.table_name && strcmp(g_upf->nft.table_name, "osmo-upf")) vty_out(vty, " table-name %s%s", g_upf->nft.table_name, VTY_NEWLINE); - llist_for_each_entry(i, &g_upf->nft.rule_append, entry) - vty_out(vty, " nft-rule tunmap append %s%s", i->str, VTY_NEWLINE); return CMD_SUCCESS; } @@ -268,47 +264,29 @@ DEFUN(cfg_tunmap_table_name, cfg_tunmap_table_name_cmd, #define NFT_RULE_STR "nftables rule specifics\n" #define TUNMAP_STR "GTP tunmap use case (a.k.a. forwarding between two GTP tunnels)\n" +#define TUNMAP_APPEND_STR "'tunmap append' feature is no longer available.\n" -DEFUN(cfg_tunmap_nft_rule_append, cfg_tunmap_nft_rule_append_cmd, +DEFUN_DEPRECATED(cfg_tunmap_nft_rule_append, cfg_tunmap_nft_rule_append_cmd, "nft-rule tunmap append .NFT_RULE", - NFT_RULE_STR TUNMAP_STR - "To each tunmap nft rule, append the given text. For example, 'nft-rule append meta nftrace set 1' adds the nftables" - " instructions 'meta nftrace set 1' to each of the tunmap rules passed to netfilter. This command is cumulative," - " multiple 'nft-rule append' will append all of the items.\n" - "The text to append\n") + NFT_RULE_STR TUNMAP_STR TUNMAP_APPEND_STR TUNMAP_APPEND_STR) { - struct string_listitem *i; - i = talloc_zero(g_upf, struct string_listitem); - i->str = argv_concat(argv, argc, 0); - talloc_steal(i, i->str); - llist_add_tail(&i->entry, &g_upf->nft.rule_append); + vty_out(vty, "%% deprecated config option: 'nft-rule tunmap append'%s", VTY_NEWLINE); return CMD_SUCCESS; } -DEFUN(cfg_tunmap_no_nft_rule_append, cfg_tunmap_no_nft_rule_append_cmd, +DEFUN_DEPRECATED(cfg_tunmap_no_nft_rule_append, cfg_tunmap_no_nft_rule_append_cmd, "no nft-rule tunmap append", - NO_STR NFT_RULE_STR TUNMAP_STR - "Drop all additions to tunmap nft rules added by earlier 'nft-rule append' config\n") + NO_STR NFT_RULE_STR TUNMAP_STR TUNMAP_APPEND_STR TUNMAP_APPEND_STR) { - struct string_listitem *i; - while ((i = llist_first_entry_or_null(&g_upf->nft.rule_append, struct string_listitem, entry))) { - llist_del(&i->entry); - talloc_free(i); - } + vty_out(vty, "%% deprecated config option: 'no nft-rule tunmap append'%s", VTY_NEWLINE); return CMD_SUCCESS; } -DEFUN(show_nft_rule_append, show_nft_rule_append_cmd, +DEFUN_DEPRECATED(show_nft_rule_append, show_nft_rule_append_cmd, "show nft-rule tunmap append", - SHOW_STR NFT_RULE_STR TUNMAP_STR "List all tunmap 'nft-rule append' entries\n") + SHOW_STR NFT_RULE_STR TUNMAP_STR TUNMAP_APPEND_STR) { - struct string_listitem *i; - if (llist_empty(&g_upf->nft.rule_append)) { - vty_out(vty, " no nft-rule tunmap append%s", VTY_NEWLINE); - return CMD_SUCCESS; - } - llist_for_each_entry(i, &g_upf->nft.rule_append, entry) - vty_out(vty, " nft-rule tunmap append %s%s", i->str, VTY_NEWLINE); + vty_out(vty, "%% deprecated config option: 'show nft-rule tunmap append'%s", VTY_NEWLINE); return CMD_SUCCESS; } @@ -342,7 +320,7 @@ DEFUN(show_nft_rule_tunmap_example, show_nft_rule_tunmap_example_cmd, osmo_sockaddr_str_from_str2(&str, "3.3.3.3"); osmo_sockaddr_str_to_sockaddr(&str, &d.core.gtp_remote_addr.u.sas); - vty_out(vty, "%s%s", upf_nft_tunmap_get_ruleset_str(OTC_SELECT, &d, &g_upf->nft.rule_append), VTY_NEWLINE); + vty_out(vty, "%s%s", upf_nft_tunmap_get_ruleset_str(OTC_SELECT, &d), VTY_NEWLINE); return CMD_SUCCESS; } diff --git a/tests/nft-rule.vty b/tests/nft-rule.vty index c52ef0e..ec719ab 100644 --- a/tests/nft-rule.vty +++ b/tests/nft-rule.vty @@ -2,53 +2,14 @@ OsmoUPF> enable OsmoUPF# configure terminal OsmoUPF(config)# tunmap -OsmoUPF(config-tunmap)# show nft-rule tunmap append - no nft-rule tunmap append OsmoUPF(config-tunmap)# show nft-rule tunmap example add chain inet osmo-upf tunmap123 { type filter hook prerouting priority -300; } add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.1 @ih,32,32 0x00000201 ip saddr set 2.2.2.3 ip daddr set 3.3.3.3 @ih,32,32 set 0x00000302 counter accept; add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.3 @ih,32,32 0x00000203 ip saddr set 2.2.2.1 ip daddr set 1.1.1.1 @ih,32,32 set 0x00000102 counter accept; +OsmoUPF(config-tunmap)# show nft-rule tunmap append +% deprecated config option: 'show nft-rule tunmap append' OsmoUPF(config-tunmap)# nft-rule tunmap append meta nftrace set 1 -OsmoUPF(config-tunmap)# show nft-rule tunmap append - nft-rule tunmap append meta nftrace set 1 -OsmoUPF(config-tunmap)# show nft-rule tunmap example -add chain inet osmo-upf tunmap123 { type filter hook prerouting priority -300; } -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.1 @ih,32,32 0x00000201 ip saddr set 2.2.2.3 ip daddr set 3.3.3.3 @ih,32,32 set 0x00000302 counter meta nftrace set 1 accept; -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.3 @ih,32,32 0x00000203 ip saddr set 2.2.2.1 ip daddr set 1.1.1.1 @ih,32,32 set 0x00000102 counter meta nftrace set 1 accept; - -OsmoUPF(config-tunmap)# nft-rule tunmap append foo -OsmoUPF(config-tunmap)# show nft-rule tunmap append - nft-rule tunmap append meta nftrace set 1 - nft-rule tunmap append foo -OsmoUPF(config-tunmap)# show nft-rule tunmap example -add chain inet osmo-upf tunmap123 { type filter hook prerouting priority -300; } -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.1 @ih,32,32 0x00000201 ip saddr set 2.2.2.3 ip daddr set 3.3.3.3 @ih,32,32 set 0x00000302 counter meta nftrace set 1 foo accept; -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.3 @ih,32,32 0x00000203 ip saddr set 2.2.2.1 ip daddr set 1.1.1.1 @ih,32,32 set 0x00000102 counter meta nftrace set 1 foo accept; - -OsmoUPF(config-tunmap)# nft-rule tunmap append bar -OsmoUPF(config-tunmap)# show nft-rule tunmap append - nft-rule tunmap append meta nftrace set 1 - nft-rule tunmap append foo - nft-rule tunmap append bar -OsmoUPF(config-tunmap)# show nft-rule tunmap example -add chain inet osmo-upf tunmap123 { type filter hook prerouting priority -300; } -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.1 @ih,32,32 0x00000201 ip saddr set 2.2.2.3 ip daddr set 3.3.3.3 @ih,32,32 set 0x00000302 counter meta nftrace set 1 foo bar accept; -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.3 @ih,32,32 0x00000203 ip saddr set 2.2.2.1 ip daddr set 1.1.1.1 @ih,32,32 set 0x00000102 counter meta nftrace set 1 foo bar accept; - -OsmoUPF(config-tunmap)# show running-config -... -tunmap -... - nft-rule tunmap append meta nftrace set 1 - nft-rule tunmap append foo - nft-rule tunmap append bar -... - +% deprecated config option: 'nft-rule tunmap append' OsmoUPF(config-tunmap)# no nft-rule tunmap append -OsmoUPF(config-tunmap)# show nft-rule tunmap append - no nft-rule tunmap append -OsmoUPF(config-tunmap)# show nft-rule tunmap example -add chain inet osmo-upf tunmap123 { type filter hook prerouting priority -300; } -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.1 @ih,32,32 0x00000201 ip saddr set 2.2.2.3 ip daddr set 3.3.3.3 @ih,32,32 set 0x00000302 counter accept; -add rule inet osmo-upf tunmap123 meta l4proto udp ip daddr 2.2.2.3 @ih,32,32 0x00000203 ip saddr set 2.2.2.1 ip daddr set 1.1.1.1 @ih,32,32 set 0x00000102 counter accept; +% deprecated config option: 'no nft-rule tunmap append' diff --git a/tests/upf.vty b/tests/upf.vty index bfaa0cc..6c278c9 100644 --- a/tests/upf.vty +++ b/tests/upf.vty @@ -56,9 +56,6 @@ OsmoUPF(config-tunmap)# list mockup no mockup table-name TABLE_NAME - nft-rule tunmap append .NFT_RULE - no nft-rule tunmap append - show nft-rule tunmap append show nft-rule tunmap example OsmoUPF(config-tunmap)# exit @@ -68,16 +65,12 @@ OsmoUPF(config-tunmap)# list mockup no mockup table-name TABLE_NAME - nft-rule tunmap append .NFT_RULE - no nft-rule tunmap append - show nft-rule tunmap append show nft-rule tunmap example OsmoUPF(config-tunmap)# mockup? mockup don't actually send rulesets to nftables, just return success OsmoUPF(config-tunmap)# no ? - mockup operate nftables rulesets normally - nft-rule nftables rule specifics + mockup operate nftables rulesets normally OsmoUPF(config-tunmap)# table-name? table-name Set the nft inet table name to create and place GTP tunnel forwarding chains in (as in 'nft add table inet foo'). If multiple instances of osmo-upf are running on the same system, each osmo-upf must have its own table name. Otherwise the names of created forwarding chains will collide. The default table name is "osmo-upf". @@ -85,13 +78,13 @@ OsmoUPF(config-tunmap)# table-name ? TABLE_NAME nft inet table name OsmoUPF(config-tunmap)# nft-rule? - nft-rule nftables rule specifics +% There is no matched command. OsmoUPF(config-tunmap)# nft-rule ? - tunmap GTP tunmap use case (a.k.a. forwarding between two GTP tunnels) +% There is no matched command. OsmoUPF(config-tunmap)# nft-rule tunmap ? - append To each tunmap nft rule, append the given text. For example, 'nft-rule append meta nftrace set 1' adds the nftables instructions 'meta nftrace set 1' to each of the tunmap rules passed to netfilter. This command is cumulative, multiple 'nft-rule append' will append all of the items. +% There is no matched command. OsmoUPF(config-tunmap)# nft-rule tunmap append ? - NFT_RULE The text to append +% There is no matched command. OsmoUPF(config-tunmap)# show? show Show running system information @@ -102,5 +95,4 @@ OsmoUPF(config-tunmap)# show ? OsmoUPF(config-tunmap)# show nft-rule ? tunmap GTP tunmap use case (a.k.a. forwarding between two GTP tunnels) OsmoUPF(config-tunmap)# show nft-rule tunmap ? - append List all tunmap 'nft-rule append' entries example Print a complete nftables ruleset for a tunmap filled with example IP addresses and TEIDs