From dbe0f7e82ecf993e306643a0fa9e98cd86a203c9 Mon Sep 17 00:00:00 2001 From: vbnogueira <105989644+vbnogueira@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:36:42 -0300 Subject: [PATCH] Small TC fixes (#5002) * Fix action parameter declaration in p4tc_filter_fields in tc_may_override cases In cases where the tc_may_override annotation is associate with an action that has a parameter with a tc_type annotation, the compiler is emitting the parameters declaration in p4tc_filter_fields incorrectly. For example in tc_may_override_example_04: struct p4tc_filter_fields { __u32 pipeid; __u32 handle; __u32 classid; __u32 chain; __u32 blockid; __be16 proto; __u16 prio; ipv4 ipv4_tbl_1_next_hop_ipv4addr; }; "ipv4" is not a valid C type, so this won't compile Also, in the case of bit types that are not equivalent to a standard C type (u8, u16, u32, u64), the compiler is emitting the type incorrectly. Let's say, in the previous example, that the type for ipv4addr was bit<33>, the compiler would've emitted the following: struct p4tc_filter_fields { __u32 pipeid; __u32 handle; __u32 classid; __u32 chain; __u32 blockid; __be16 proto; __u16 prio; __u33 ipv4_tbl_1_next_hop_ipv4addr; }; This commit fixes this by converting tc_type annotated action parameters to their corresponding C type and converts any non C standard bit type into a u8 array rounded up to the nearest byte Signed-off-by: Victor Nogueira * Use profile_id instead of aging_ms In struct p4tc_table_entry_create_bpf_params__local we've adopted profile_id instead of aging_ms. This commits updates the field name in the generated output Signed-off-by: Victor Nogueira * Update pna.h Add missing kfuncs and remove some deprecated ones Signed-off-by: Victor Nogueira --------- Signed-off-by: Victor Nogueira --- backends/tc/ebpfCodeGen.cpp | 10 ++--- backends/tc/runtime/pna.h | 45 ++++++++++++------- backends/tc/tc.def | 30 ++++++++----- .../set_entry_timer_example_control_blocks.c | 4 +- .../tc_may_override_example_04_parser.h | 2 +- .../tc_may_override_example_05_parser.h | 2 +- .../tc_may_override_example_06_parser.h | 4 +- 7 files changed, 58 insertions(+), 39 deletions(-) diff --git a/backends/tc/ebpfCodeGen.cpp b/backends/tc/ebpfCodeGen.cpp index 37df9d92b1..00d6d66733 100644 --- a/backends/tc/ebpfCodeGen.cpp +++ b/backends/tc/ebpfCodeGen.cpp @@ -134,9 +134,9 @@ void PNAEbpfGenerator::emitP4TCActionParam(EBPF::CodeBuilder *builder) const { for (auto param : table->defaultMissActionParams) { cstring paramName = param->paramDetail->getParamName(); cstring placeholder = tblName + "_" + defaultActionName + "_" + paramName; - cstring typeName = param->paramDetail->getParamType(); + cstring paramDecl = param->paramDetail->getParamDecl(placeholder); builder->emitIndent(); - builder->appendFormat("%v %v", typeName, placeholder); + builder->append(paramDecl); builder->endOfStatement(true); } } @@ -149,9 +149,9 @@ void PNAEbpfGenerator::emitP4TCActionParam(EBPF::CodeBuilder *builder) const { for (auto param : table->defaultHitActionParams) { cstring paramName = param->paramDetail->getParamName(); cstring placeholder = tblName + "_" + defaultActionName + "_" + paramName; - cstring typeName = param->paramDetail->getParamType(); + cstring paramDecl = param->paramDetail->getParamDecl(placeholder); builder->emitIndent(); - builder->appendFormat("%v %v", typeName, placeholder); + builder->append(paramDecl); builder->endOfStatement(true); } } @@ -1679,7 +1679,7 @@ void ControlBodyTranslatorPNA::processFunction(const P4::ExternFunction *functio builder->appendFormat(" .tblid = %d,", tblId); builder->newline(); builder->emitIndent(); - builder->append(" .aging_ms = "); + builder->append(" .profile_id = "); for (auto a : *function->expr->arguments) { visit(a); } diff --git a/backends/tc/runtime/pna.h b/backends/tc/runtime/pna.h index 9e7519166b..ae87f54526 100644 --- a/backends/tc/runtime/pna.h +++ b/backends/tc/runtime/pna.h @@ -101,7 +101,7 @@ struct clone_session_entry { struct p4tc_table_entry_act_bpf_params__local { u32 pipeid; u32 tblid; -} __attribute__((preserve_access_index)); +}; struct __attribute__((__packed__)) p4tc_table_entry_act_bpf { u32 act_id; @@ -160,6 +160,19 @@ xdp_p4tc_entry_create_on_miss(struct xdp_md *xdp_ctx, const u32 params__sz, void *key, const u32 key__sz) __ksym; +/* No mapping to PNA, but are useful utilities */ +extern int +bpf_p4tc_entry_update(struct __sk_buff *skb_ctx, + struct p4tc_table_entry_create_bpf_params__local *params, + const u32 params__sz, + void *key, const u32 key__sz) __ksym; + +extern int +xdp_p4tc_entry_update(struct xdp_md *xdp_ctx, + struct p4tc_table_entry_create_bpf_params__local *params, + const u32 params__sz, + void *key, const u32 key__sz) __ksym; + /* No mapping to PNA, but are useful utilities */ extern int bpf_p4tc_entry_delete(struct __sk_buff *skb_ctx, @@ -200,33 +213,33 @@ struct p4tc_ext_bpf_val { /* Equivalent to PNA indirect counters */ extern int bpf_p4tc_extern_count_pktsnbytes(struct __sk_buff *skb_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz, void *key, const u32 key__sz) __ksym; + struct p4tc_ext_bpf_params *params, + const u32 params__sz, void *key, const u32 key__sz) __ksym; extern int bpf_p4tc_extern_count_pkts(struct __sk_buff *skb_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz, void *key, const u32 key__sz) __ksym; + struct p4tc_ext_bpf_params *params, + const u32 params__sz, void *key, const u32 key__sz) __ksym; extern int bpf_p4tc_extern_count_bytes(struct __sk_buff *skb_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz, void *key, const u32 key__sz) __ksym; + struct p4tc_ext_bpf_params *params, + const u32 params__sz, void *key, const u32 key__sz) __ksym; extern int -xdp_p4tc_extern_indirect_count_pktsnbytes(struct xdp_md *xdp_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz) __ksym; +xdp_p4tc_extern_count_pktsnbytes(struct xdp_md *xdp_ctx, + struct p4tc_ext_bpf_params *params, + const u32 params__sz) __ksym; extern int -xdp_p4tc_extern_indirect_count_pktsonly(struct xdp_md *xdp_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz) __ksym; +xdp_p4tc_extern_count_pkts(struct xdp_md *xdp_ctx, + struct p4tc_ext_bpf_params *params, + const u32 params__sz) __ksym; extern int -xdp_p4tc_extern_indirect_count_bytesonly(struct xdp_md *xdp_ctx, - struct p4tc_ext_bpf_params *params, - const u32 params__sz) __ksym; +xdp_p4tc_extern_count_bytes(struct xdp_md *xdp_ctx, + struct p4tc_ext_bpf_params *params, + const u32 params__sz) __ksym; extern int bpf_p4tc_extern_meter_bytes_color(struct __sk_buff *skb_ctx, struct p4tc_ext_bpf_params *params, diff --git a/backends/tc/tc.def b/backends/tc/tc.def index 20820e4edb..2fa27cc1c4 100644 --- a/backends/tc/tc.def +++ b/backends/tc/tc.def @@ -77,35 +77,41 @@ class TCActionParam { unsigned getDirection() const { return direction; } - cstring getParamType() const { - std::string paramType = ""; + cstring getParamDecl(cstring placeholderName) const { + std::string paramDecl = ""; switch(dataType) { case TC::BIT_TYPE : - paramType = absl::StrCat("__u", bitSize); + if (bitSize == 8 || bitSize == 16 || bitSize == 32 || bitSize == 64) { + paramDecl = absl::StrCat("__u", bitSize, " ", placeholderName); + } else { + unsigned byteSize = bitSize / 8 + (bitSize % 8 != 0); + + paramDecl = absl::StrCat("__u8 ", placeholderName, "[", byteSize, "]"); + } break; case TC::DEV_TYPE : - paramType = "dev"; + paramDecl = absl::StrCat("__u32 ", placeholderName); break; case TC::MACADDR_TYPE : - paramType = "macaddr"; + paramDecl = absl::StrCat("__u8 ", placeholderName, "[6]"); break; case TC::IPV4_TYPE : - paramType = "ipv4"; + paramDecl = absl::StrCat("__u32 ", placeholderName); break; case TC::IPV6_TYPE : - paramType = "ipv6"; + paramDecl = absl::StrCat("__u8 ", placeholderName, "[16]"); break; case TC::BE16_TYPE : - paramType = "__be16"; + paramDecl = absl::StrCat("__be16 ", placeholderName); break; case TC::BE32_TYPE : - paramType = "__be32"; + paramDecl = absl::StrCat("__be32 ", placeholderName); break; case TC::BE64_TYPE : - paramType = "__be64"; + paramDecl = absl::StrCat("__be64 ", placeholderName); break; - } - return paramType; + } + return paramDecl; } toString { std::string tcActionParam = absl::StrCat("\n\tparam ", paramName, " type "); diff --git a/testdata/p4tc_samples_outputs/set_entry_timer_example_control_blocks.c b/testdata/p4tc_samples_outputs/set_entry_timer_example_control_blocks.c index d802d6f655..07eebe4dd1 100644 --- a/testdata/p4tc_samples_outputs/set_entry_timer_example_control_blocks.c +++ b/testdata/p4tc_samples_outputs/set_entry_timer_example_control_blocks.c @@ -114,7 +114,7 @@ if (/* hdr->ipv4.isValid() */ struct p4tc_table_entry_create_bpf_params__local update_params = { .pipeid = p4tc_filter_fields.pipeid, .tblid = 1, - .aging_ms = 2 + .profile_id = 2 }; bpf_p4tc_entry_update(skb, &update_params, sizeof(params), &key, sizeof(key)); } @@ -169,7 +169,7 @@ if (/* hdr->ipv4.isValid() */ struct p4tc_table_entry_create_bpf_params__local update_params = { .pipeid = p4tc_filter_fields.pipeid, .tblid = 2, - .aging_ms = 2 + .profile_id = 2 }; bpf_p4tc_entry_update(skb, &update_params, sizeof(params), &key, sizeof(key)); } diff --git a/testdata/p4tc_samples_outputs/tc_may_override_example_04_parser.h b/testdata/p4tc_samples_outputs/tc_may_override_example_04_parser.h index 9d708e2b8f..06f3e6e418 100644 --- a/testdata/p4tc_samples_outputs/tc_may_override_example_04_parser.h +++ b/testdata/p4tc_samples_outputs/tc_may_override_example_04_parser.h @@ -54,7 +54,7 @@ struct p4tc_filter_fields { __u32 blockid; __be16 proto; __u16 prio; - ipv4 ipv4_tbl_1_next_hop_ipv4addr; + __u32 ipv4_tbl_1_next_hop_ipv4addr; }; REGISTER_START() diff --git a/testdata/p4tc_samples_outputs/tc_may_override_example_05_parser.h b/testdata/p4tc_samples_outputs/tc_may_override_example_05_parser.h index 239070ea8a..74b1b65a62 100644 --- a/testdata/p4tc_samples_outputs/tc_may_override_example_05_parser.h +++ b/testdata/p4tc_samples_outputs/tc_may_override_example_05_parser.h @@ -54,7 +54,7 @@ struct p4tc_filter_fields { __u32 blockid; __be16 proto; __u16 prio; - dev ipv4_tbl_1_next_hop_vport; + __u32 ipv4_tbl_1_next_hop_vport; }; REGISTER_START() diff --git a/testdata/p4tc_samples_outputs/tc_may_override_example_06_parser.h b/testdata/p4tc_samples_outputs/tc_may_override_example_06_parser.h index f1ae281121..8f8fb971a5 100644 --- a/testdata/p4tc_samples_outputs/tc_may_override_example_06_parser.h +++ b/testdata/p4tc_samples_outputs/tc_may_override_example_06_parser.h @@ -54,8 +54,8 @@ struct p4tc_filter_fields { __u32 blockid; __be16 proto; __u16 prio; - dev ipv4_tbl_1_next_hop_vport; - dev ipv4_tbl_2_next_hop_vport; + __u32 ipv4_tbl_1_next_hop_vport; + __u32 ipv4_tbl_2_next_hop_vport; }; REGISTER_START()