Add a sample tool demonstrating how to add, dump, and delete a flower filter with two VLAN push actions. The example can be invoked as: # ./tc-filter-add p2 flower pref 2211 flower vlan_id: 255 action order: 1 vlan push id 255 action order: 2 vlan push id 555 This verifies correct handling of tc action attributes for multiple VLAN push actions. Signed-off-by: Zahari Doychev --- tools/net/ynl/Makefile.deps | 1 + tools/net/ynl/samples/.gitignore | 1 + tools/net/ynl/samples/tc-filter-add.c | 308 ++++++++++++++++++++++++++ 3 files changed, 310 insertions(+) create mode 100644 tools/net/ynl/samples/tc-filter-add.c diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps index 865fd2e8519e..96c390af060e 100644 --- a/tools/net/ynl/Makefile.deps +++ b/tools/net/ynl/Makefile.deps @@ -47,4 +47,5 @@ CFLAGS_tc:= $(call get_hdr_inc,__LINUX_RTNETLINK_H,rtnetlink.h) \ $(call get_hdr_inc,_TC_MIRRED_H,tc_act/tc_mirred.h) \ $(call get_hdr_inc,_TC_SKBEDIT_H,tc_act/tc_skbedit.h) \ $(call get_hdr_inc,_TC_TUNNEL_KEY_H,tc_act/tc_tunnel_key.h) +CFLAGS_tc-filter-add:=$(CFLAGS_tc) CFLAGS_tcp_metrics:=$(call get_hdr_inc,_LINUX_TCP_METRICS_H,tcp_metrics.h) diff --git a/tools/net/ynl/samples/.gitignore b/tools/net/ynl/samples/.gitignore index 7f5fca7682d7..05087ee323ba 100644 --- a/tools/net/ynl/samples/.gitignore +++ b/tools/net/ynl/samples/.gitignore @@ -7,3 +7,4 @@ rt-addr rt-link rt-route tc +tc-filter-add diff --git a/tools/net/ynl/samples/tc-filter-add.c b/tools/net/ynl/samples/tc-filter-add.c new file mode 100644 index 000000000000..297f8151ca86 --- /dev/null +++ b/tools/net/ynl/samples/tc-filter-add.c @@ -0,0 +1,308 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "tc-user.h" + +#define TC_HANDLE (0xFFFF << 16) + +const char *vlan_act_name(struct tc_vlan *p) +{ + switch (p->v_action) { + case TCA_VLAN_ACT_POP: + return "pop"; + case TCA_VLAN_ACT_PUSH: + return "push"; + case TCA_VLAN_ACT_MODIFY: + return "modify"; + default: + break; + } + + return "not supported"; +} + +const char *gact_act_name(struct tc_gact *p) +{ + switch (p->action) { + case TC_ACT_SHOT: + return "drop"; + case TC_ACT_OK: + return "ok"; + case TC_ACT_PIPE: + return "pipe"; + default: + break; + } + + return "not supported"; +} + +static void print_vlan(struct tc_act_vlan_attrs *vlan) +{ + printf("%s ", vlan_act_name(vlan->parms)); + if (vlan->_present.push_vlan_id) + printf("id %u ", vlan->push_vlan_id); + if (vlan->_present.push_vlan_protocol) + printf("protocol %x ", ntohs(vlan->push_vlan_protocol)); + if (vlan->_present.push_vlan_priority) + printf("priority %u ", vlan->push_vlan_priority); +} + +static void print_gact(struct tc_act_gact_attrs *gact) +{ + struct tc_gact *p = gact->parms; + + printf("%s ", gact_act_name(p)); +} + +static void flower_print(struct tc_flower_attrs *flower, const char *kind) +{ + struct tc_act_attrs *a; + unsigned int i; + + printf("%s ", kind); + + if (flower->_present.key_vlan_id) + printf("vlan_id: %u\n", flower->key_vlan_id); + + for (i = 0; i < flower->_count.act; i++) { + a = &flower->act[i]; + printf("action order: %i %s ", i + 1, a->kind); + if (a->options._present.vlan) + print_vlan(&a->options.vlan); + else if (a->options._present.gact) + print_gact(&a->options.gact); + printf("\n"); + } + printf("\n"); +} + +static void tc_filter_print(struct tc_gettfilter_rsp *f) +{ + struct tc_options_msg *opt = &f->options; + + if (opt->_present.flower) + flower_print(&opt->flower, f->kind); + else if (f->_len.kind) + printf("%s pref %u\n", f->kind, (f->_hdr.tcm_info >> 16)); +} + +static int tc_filter_add(struct ynl_sock *ys, int ifi) +{ + struct tc_newtfilter_req *req; + struct tc_act_attrs *acts; + struct tc_vlan p = { .v_action = TCA_VLAN_ACT_PUSH }; + __u16 flags = NLM_F_REQUEST | NLM_F_EXCL | NLM_F_CREATE; + int ret; + + req = tc_newtfilter_req_alloc(); + if (!req) { + fprintf(stderr, "tc_newtfilter_req_alloc failed\n"); + return -1; + } + memset(req, 0, sizeof(*req)); + + acts = tc_act_attrs_alloc(2); + if (!acts) { + fprintf(stderr, "tc_act_attrs_alloc\n"); + tc_newtfilter_req_free(req); + return -1; + } + memset(acts, 0, sizeof(*acts) * 2); + + req->_hdr.tcm_ifindex = ifi; + req->_hdr.tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS); + req->_hdr.tcm_info = TC_H_MAKE(1 << 16, htons(ETH_P_8021Q)); + req->chain = 0; + + tc_newtfilter_req_set_nlflags(req, flags); + tc_newtfilter_req_set_kind(req, "flower"); + tc_newtfilter_req_set_options_flower_key_vlan_id(req, 255); + tc_newtfilter_req_set_options_flower_key_vlan_prio(req, 5); + tc_newtfilter_req_set_options_flower_key_num_of_vlans(req, 3); + + __tc_newtfilter_req_set_options_flower_act(req, acts, 2); + + tc_act_attrs_set_kind(&acts[0], "vlan"); + tc_act_attrs_set_options_vlan_parms(&acts[0], &p, sizeof(p)); + tc_act_attrs_set_options_vlan_push_vlan_id(&acts[0], 255); + tc_act_attrs_set_kind(&acts[1], "vlan"); + tc_act_attrs_set_options_vlan_parms(&acts[1], &p, sizeof(p)); + tc_act_attrs_set_options_vlan_push_vlan_id(&acts[1], 555); + + tc_newtfilter_req_set_options_flower_flags(req, 0); + tc_newtfilter_req_set_options_flower_key_eth_type(req, htons(0x8100)); + + ret = tc_newtfilter(ys, req); + if (ret) + fprintf(stderr, "tc_newtfilter: %s\n", ys->err.msg); + + tc_newtfilter_req_free(req); + + return ret; +} + +static int tc_filter_show(struct ynl_sock *ys, int ifi) +{ + struct tc_gettfilter_req_dump *req; + struct tc_gettfilter_list *rsp; + + req = tc_gettfilter_req_dump_alloc(); + if (!req) { + fprintf(stderr, "tc_gettfilter_req_dump_alloc failed\n"); + return -1; + } + memset(req, 0, sizeof(*req)); + + req->_hdr.tcm_ifindex = ifi; + req->_hdr.tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS); + req->_present.chain = 1; + req->chain = 0; + + rsp = tc_gettfilter_dump(ys, req); + tc_gettfilter_req_dump_free(req); + if (!rsp) { + fprintf(stderr, "YNL: %s\n", ys->err.msg); + return -1; + } + + if (ynl_dump_empty(rsp)) + fprintf(stderr, "Error: no filters reported\n"); + else + ynl_dump_foreach(rsp, flt) tc_filter_print(flt); + + tc_gettfilter_list_free(rsp); + + return 0; +} + +static int tc_filter_del(struct ynl_sock *ys, int ifi) +{ + struct tc_deltfilter_req *req; + __u16 flags = NLM_F_REQUEST; + int ret; + + req = tc_deltfilter_req_alloc(); + if (!req) { + fprintf(stderr, "tc_deltfilter_req_alloc failedq\n"); + return -1; + } + memset(req, 0, sizeof(*req)); + + req->_hdr.tcm_ifindex = ifi; + req->_hdr.tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS); + req->_hdr.tcm_info = TC_H_MAKE(1 << 16, htons(ETH_P_8021Q)); + tc_deltfilter_req_set_nlflags(req, flags); + + ret = tc_deltfilter(ys, req); + if (ret) + fprintf(stderr, "tc_deltfilter failed: %s\n", ys->err.msg); + + tc_deltfilter_req_free(req); + + return ret; +} + +static int tc_clsact_add(struct ynl_sock *ys, int ifi) +{ + struct tc_newqdisc_req *req; + __u16 flags = NLM_F_REQUEST | NLM_F_EXCL | NLM_F_CREATE; + int ret; + + req = tc_newqdisc_req_alloc(); + if (!req) { + fprintf(stderr, "tc_newqdisc_req_alloc failed\n"); + return -1; + } + memset(req, 0, sizeof(*req)); + + req->_hdr.tcm_ifindex = ifi; + req->_hdr.tcm_parent = TC_H_CLSACT; + req->_hdr.tcm_handle = TC_HANDLE; + tc_newqdisc_req_set_nlflags(req, flags); + tc_newqdisc_req_set_kind(req, "clsact"); + + ret = tc_newqdisc(ys, req); + if (ret) + fprintf(stderr, "tc_newqdisc failed: %s\n", ys->err.msg); + + tc_newqdisc_req_free(req); + + return ret; +} + +static int tc_clsact_del(struct ynl_sock *ys, int ifi) +{ + struct tc_delqdisc_req *req; + __u16 flags = NLM_F_REQUEST; + int ret; + + req = tc_delqdisc_req_alloc(); + if (!req) { + fprintf(stderr, "tc_delqdisc_req_alloc failed\n"); + return -1; + } + memset(req, 0, sizeof(*req)); + + req->_hdr.tcm_ifindex = ifi; + req->_hdr.tcm_parent = TC_H_CLSACT; + req->_hdr.tcm_handle = TC_HANDLE; + tc_delqdisc_req_set_nlflags(req, flags); + + ret = tc_delqdisc(ys, req); + if (ret) + fprintf(stderr, "tc_delqdisc failed: %s\n", ys->err.msg); + + tc_delqdisc_req_free(req); + + return ret; +} + +int main(int argc, char **argv) +{ + struct ynl_error yerr; + struct ynl_sock *ys; + int ifi; + + if (argc < 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + ifi = if_nametoindex(argv[1]); + if (!ifi) { + perror("if_nametoindex"); + return 1; + } + + ys = ynl_sock_create(&ynl_tc_family, &yerr); + if (!ys) { + fprintf(stderr, "YNL: %s\n", yerr.msg); + return 1; + } + + if (tc_clsact_add(ys, ifi)) + goto err_destroy; + + if (tc_filter_add(ys, ifi)) + goto err_destroy; + + if (tc_filter_show(ys, ifi)) + goto err_destroy; + + tc_filter_del(ys, ifi); + tc_clsact_del(ys, ifi); + +err_destroy: + ynl_sock_destroy(ys); + return 2; +} -- 2.51.0 When freeing indexed arrays, the corresponding free function should be called for each entry of the indexed array. For example, for for 'struct tc_act_attrs' 'tc_act_attrs_free(...)' needs to be called for each entry. Previously, memory leaks were reported when enabling the ASAN analyzer. ================================================================= ==874==ERROR: LeakSanitizer: detected memory leaks Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f221fd20cb5 in malloc ./debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67 #1 0x55c98db048af in tc_act_attrs_set_options_vlan_parms ../generated/tc-user.h:2813 #2 0x55c98db048af in main ./linux/tools/net/ynl/samples/tc-filter-add.c:71 Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f221fd20cb5 in malloc ./debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67 #1 0x55c98db04a93 in tc_act_attrs_set_options_vlan_parms ../generated/tc-user.h:2813 #2 0x55c98db04a93 in main ./linux/tools/net/ynl/samples/tc-filter-add.c:74 Direct leak of 10 byte(s) in 2 object(s) allocated from: #0 0x7f221fd20cb5 in malloc ./debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67 #1 0x55c98db0527d in tc_act_attrs_set_kind ../generated/tc-user.h:1622 SUMMARY: AddressSanitizer: 58 byte(s) leaked in 4 allocation(s). The following diff illustrates the changes introduced compared to the previous version of the code. void tc_flower_attrs_free(struct tc_flower_attrs *obj) { + unsigned int i; + free(obj->indev); + for (i = 0; i < obj->_count.act; i++) + tc_act_attrs_free(&obj->act[i]); free(obj->act); free(obj->key_eth_dst); free(obj->key_eth_dst_mask); Signed-off-by: Zahari Doychev Reviewed-by: Jakub Kicinski --- tools/net/ynl/pyynl/ynl_gen_c.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 58086b101057..aadeb3abcad8 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -861,6 +861,18 @@ class TypeIndexedArray(Type): return [f"{member} = {self.c_name};", f"{presence} = n_{self.c_name};"] + def free_needs_iter(self): + return self.sub_type == 'nest' + + def _free_lines(self, ri, var, ref): + lines = [] + if self.sub_type == 'nest': + lines += [ + f"for (i = 0; i < {var}->{ref}_count.{self.c_name}; i++)", + f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name}[i]);', + ] + lines += f"free({var}->{ref}{self.c_name});", + return lines class TypeNestTypeValue(Type): def _complex_member_type(self, ri): -- 2.51.0 Linux tc actions expect the action order to start from index one. To accommodate this, update the code generation so array indexing begins at 1 for tc actions. This results in the following change: array = ynl_attr_nest_start(nlh, TCA_FLOWER_ACT); for (i = 0; i < obj->_count.act; i++) - tc_act_attrs_put(nlh, i, &obj->act[i]); + tc_act_attrs_put(nlh, i + 1, &obj->act[i]); ynl_attr_nest_end(nlh, array); This change does not impact other indexed array attributes at the moment, as analyzed in [1]. [1]: https://lore.kernel.org/netdev/20251022182701.250897-1-ast@fiberby.net/ Signed-off-by: Zahari Doychev --- Documentation/netlink/specs/tc.yaml | 8 ++++++++ tools/net/ynl/pyynl/ynl_gen_c.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml index b398f7a46dae..3e3da477dd5d 100644 --- a/Documentation/netlink/specs/tc.yaml +++ b/Documentation/netlink/specs/tc.yaml @@ -2044,6 +2044,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: police type: nest @@ -2064,6 +2065,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: police type: nest @@ -2303,6 +2305,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: police type: nest @@ -2493,6 +2496,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: key-eth-dst type: binary @@ -3020,6 +3024,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: mask type: u32 @@ -3180,6 +3185,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: flags type: u32 @@ -3566,6 +3572,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: taprio-attrs name-prefix: tca-taprio-attr- @@ -3798,6 +3805,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + doc: Index 0 is ignored and array starts from index 1. - name: indev type: string diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index aadeb3abcad8..d01ef8fa5497 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -852,7 +852,7 @@ class TypeIndexedArray(Type): ri.cw.p(f"ynl_attr_put(nlh, i, {var}->{self.c_name}[i], {self.checks['exact-len']});") elif self.sub_type == 'nest': ri.cw.p(f'for (i = 0; i < {var}->_count.{self.c_name}; i++)') - ri.cw.p(f"{self.nested_render_name}_put(nlh, i, &{var}->{self.c_name}[i]);") + ri.cw.p(f"{self.nested_render_name}_put(nlh, i + 1, &{var}->{self.c_name}[i]);") else: raise Exception(f"Put for IndexedArray sub-type {self.attr['sub-type']} not supported, yet") ri.cw.p('ynl_attr_nest_end(nlh, array);') -- 2.51.0