Add a simple tool that demonstrates adding a flower filter with two VLAN push actions. This example can be invoked as: # ./tools/samples/tc-filter-add p2 # tc -j -p filter show dev p2 ingress pref 2211 [ { "protocol": "802.1Q", "kind": "flower", "chain": 0 },{ "protocol": "802.1Q", "kind": "flower", "chain": 0, "options": { "handle": 1, "keys": { "num_of_vlans": 3, "vlan_id": 255, "vlan_prio": 5 }, "not_in_hw": true, "actions": [ { "order": 1, "kind": "vlan", "vlan_action": "push", "id": 255, "control_action": { "type": "pass" }, "index": 5, "ref": 1, "bind": 1 },{ "order": 2, "kind": "vlan", "vlan_action": "push", "id": 555, "control_action": { "type": "pass" }, "index": 6, "ref": 1, "bind": 1 } ] } } ] This shows the filter with two VLAN push actions, verifying that tc action attributes are handled correctly. 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 | 92 +++++++++++++++++++++++++++ 3 files changed, 94 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..b9c6f30f2a30 --- /dev/null +++ b/tools/net/ynl/samples/tc-filter-add.c @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "tc-user.h" + +int main(int argc, char **argv) +{ + struct tc_newtfilter_req *req; + struct tc_act_attrs *acts; + struct tc_vlan p = { + .v_action = TCA_VLAN_ACT_PUSH + }; + __u16 flags = NLM_F_EXCL | NLM_F_CREATE; + 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; + } + + req = tc_newtfilter_req_alloc(); + if (!req) { + fprintf(stderr, "tc_newtfilter_req_alloc failed\n"); + goto err_destroy; + } + memset(req, 0, sizeof(*req)); + + acts = tc_act_attrs_alloc(2); + if (!acts) { + fprintf(stderr, "tc_act_attrs_alloc\n"); + goto err_act; + } + memset(acts, 0, sizeof(*acts)); + + 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((2211 << 16), htons(0x8100)); + 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)); + + if (tc_newtfilter(ys, req)) + fprintf(stderr, "YNL: %s\n", ys->err.msg); + + tc_newtfilter_req_free(req); + ynl_sock_destroy(ys); + return 0; + +err_act: + tc_newtfilter_req_free(req); +err_destroy: + ynl_sock_destroy(ys); + return 2; +} -- 2.51.0 The memory belonging to tx_buf and rx_buf in ynl_sock is not initialized after allocation. This commit ensures the entire allocated memory is set to zero. When asan is enabled, uninitialized bytes may contain poison values. This can cause failures e.g. when doing ynl_attr_put_str then poisoned bytes appear after the null terminator. As a result, tc filter addition may fail. Signed-off-by: Zahari Doychev --- tools/net/ynl/lib/ynl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c index 2bcd781111d7..16a4815d6a49 100644 --- a/tools/net/ynl/lib/ynl.c +++ b/tools/net/ynl/lib/ynl.c @@ -744,7 +744,7 @@ ynl_sock_create(const struct ynl_family *yf, struct ynl_error *yse) ys = malloc(sizeof(*ys) + 2 * YNL_SOCKET_BUFFER_SIZE); if (!ys) return NULL; - memset(ys, 0, sizeof(*ys)); + memset(ys, 0, sizeof(*ys) + 2 * YNL_SOCKET_BUFFER_SIZE); ys->family = yf; ys->tx_buf = &ys->raw_buf[0]; -- 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 --- 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 The Linux tc actions expect that the action order starts from index one. To accommodate this, add a start-index property to the ynl spec for indexed arrays. This property allows the starting index to be specified, ensuring compatibility with consumers that require a non-zero-based index. For example if we have "start_index = 1" then we get the following diff. ynl_attr_put_str(nlh, TCA_FLOWER_INDEV, obj->indev); 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); Signed-off-by: Zahari Doychev --- Documentation/netlink/netlink-raw.yaml | 13 +++++++++++++ Documentation/netlink/specs/tc.yaml | 7 +++++++ tools/net/ynl/pyynl/lib/nlspec.py | 1 + tools/net/ynl/pyynl/ynl_gen_c.py | 6 +++++- 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Documentation/netlink/netlink-raw.yaml b/Documentation/netlink/netlink-raw.yaml index 246fa07bccf6..aafb7cb16beb 100644 --- a/Documentation/netlink/netlink-raw.yaml +++ b/Documentation/netlink/netlink-raw.yaml @@ -260,6 +260,9 @@ properties: Sometimes, however, both forms are necessary, in which case header contains the enum form while specific attributes may request to convert the values into a bitfield. type: boolean + start-index: + description: For indexed arrays the first index value. + type: integer checks: description: Kernel input validation. type: object @@ -308,6 +311,16 @@ properties: type: string # End netlink-raw + # allow start index only for indexed arrays + if: + properties: + type: + const: indexed-array + then: {} + else: + not: + required: [ start-index ] + # Make sure name-prefix does not appear in subsets (subsets inherit naming) dependencies: name-prefix: diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml index b398f7a46dae..459aa51059ec 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 + start-index: 1 - name: police type: nest @@ -2303,6 +2304,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: police type: nest @@ -2493,6 +2495,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: key-eth-dst type: binary @@ -3020,6 +3023,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: mask type: u32 @@ -3180,6 +3184,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: flags type: u32 @@ -3566,6 +3571,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: taprio-attrs name-prefix: tca-taprio-attr- @@ -3798,6 +3804,7 @@ attribute-sets: type: indexed-array sub-type: nest nested-attributes: act-attrs + start-index: 1 - name: indev type: string diff --git a/tools/net/ynl/pyynl/lib/nlspec.py b/tools/net/ynl/pyynl/lib/nlspec.py index 85c17fe01e35..08660602da9d 100644 --- a/tools/net/ynl/pyynl/lib/nlspec.py +++ b/tools/net/ynl/pyynl/lib/nlspec.py @@ -181,6 +181,7 @@ class SpecAttr(SpecElement): self.display_hint = yaml.get('display-hint') self.sub_message = yaml.get('sub-message') self.selector = yaml.get('selector') + self.start_index = yaml.get('start-index', 0) self.is_auto_scalar = self.type == "sint" or self.type == "uint" diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index aadeb3abcad8..698d6089a856 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -852,7 +852,11 @@ 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, " + f"i{f' + {self.start_index}' if self.start_index > 0 else ''}, " + f"&{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