In the asynchronous API definition and some drivers, the rte_flow_item spec value may not be calculated by the driver due to the reason of speed of light rule insertion rate and sometimes the input parameters will be copied and changed internally.
After copying, the spec and last will be protected by the keyword const and cannot be changed in the code itself. And also the driver needs some extra memory to do the calculation and extra conditions to understand the length of each item spec. This is not efficient. To solve the issue and support usage of the following fix, a new OP was introduced to calculate the spec and last values after applying the mask inline. Signed-off-by: Bing Zhao <[email protected]> Acked-by: Dariusz Sosnowski <[email protected]> --- v3: - add test code - fix the issue found by AI v4: reabse on top of the main v5: handle some items separately and add test for them v6: resend the correct version instead of v5 with AI defects resolved --- app/test/test_ethdev_api.c | 123 +++++++++++++++++++++++++++++++++++++ lib/ethdev/rte_flow.c | 55 +++++++++++++++-- lib/ethdev/rte_flow.h | 13 ++++ 3 files changed, 185 insertions(+), 6 deletions(-) diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c index 00d6a5c614..ba3e785425 100644 --- a/app/test/test_ethdev_api.c +++ b/app/test/test_ethdev_api.c @@ -2,8 +2,11 @@ * Copyright (C) 2023, Advanced Micro Devices, Inc. */ +#include <string.h> + #include <rte_log.h> #include <rte_ethdev.h> +#include <rte_flow.h> #include <rte_test.h> #include "test.h" @@ -15,6 +18,125 @@ #define NUM_MBUF 1024 #define MBUF_CACHE_SIZE 256 +static int32_t +ethdev_api_flow_conv_pattern_masked(void) +{ + const struct rte_flow_item_eth spec = { + .hdr.dst_addr.addr_bytes = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }, + .hdr.src_addr.addr_bytes = { 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f }, + .hdr.ether_type = RTE_BE16(0x1234), + }; + const struct rte_flow_item_eth last = { + .hdr.dst_addr.addr_bytes = { 0x11, 0x12, 0x13, 0x14, 0x15, 0x16 }, + .hdr.src_addr.addr_bytes = { 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f }, + .hdr.ether_type = RTE_BE16(0x5678), + }; + const struct rte_flow_item_eth mask = { + .hdr.dst_addr.addr_bytes = { 0xff, 0xff, 0x00, 0x00, 0xff, 0xff }, + .hdr.src_addr.addr_bytes = { 0xff, 0x00, 0xff, 0x00, 0xff, 0x00 }, + .hdr.ether_type = RTE_BE16(0xffff), + }; + const uint8_t raw_pattern[] = { 0xaa, 0xbb, 0xcc, 0xdd }; + const struct rte_flow_item_raw raw_spec = { + .relative = 1, + .search = 1, + .offset = 8, + .limit = 4, + .length = sizeof(raw_pattern), + .pattern = raw_pattern, + }; + const struct rte_flow_item_raw raw_mask = { + .relative = 1, + .search = 0, + .reserved = 0x3fffffff, + .offset = 0xffffffff, + .limit = 0xffff, + .length = 0xffff, + .pattern = NULL, + }; + const struct rte_flow_item pattern[] = { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &spec, + .last = &last, + .mask = &mask, + }, + { + .type = RTE_FLOW_ITEM_TYPE_RAW, + .spec = &raw_spec, + .mask = &raw_mask, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + union { + struct rte_flow_item item; + struct rte_flow_item_eth eth; + struct rte_flow_item_raw raw; + double align; + uint8_t buf[256]; + } dst; + const struct rte_flow_item *item; + const struct rte_flow_item_eth *conv_spec; + const struct rte_flow_item_eth *conv_last; + const struct rte_flow_item_raw *conv_raw_spec; + int ret; + + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, NULL, 0, pattern, NULL); + TEST_ASSERT(ret > 0, "Masked pattern conversion size query failed"); + TEST_ASSERT((size_t)ret <= sizeof(dst.buf), + "Masked pattern conversion needs too much storage"); + + memset(&dst, 0, sizeof(dst)); + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, dst.buf, + sizeof(dst.buf), pattern, NULL); + TEST_ASSERT(ret > 0, "Masked pattern conversion failed"); + + item = (const struct rte_flow_item *)dst.buf; + conv_spec = item[0].spec; + conv_last = item[0].last; + TEST_ASSERT_NOT_NULL(conv_spec, "Converted spec must be set"); + TEST_ASSERT_NOT_NULL(conv_last, "Converted last must be set"); + + TEST_ASSERT_EQUAL(conv_spec->hdr.dst_addr.addr_bytes[0], 0x01, + "Masked spec dst byte 0 mismatch"); + TEST_ASSERT_EQUAL(conv_spec->hdr.dst_addr.addr_bytes[2], 0x00, + "Masked spec dst byte 2 mismatch"); + TEST_ASSERT_EQUAL(conv_spec->hdr.src_addr.addr_bytes[1], 0x00, + "Masked spec src byte 1 mismatch"); + TEST_ASSERT_EQUAL(conv_spec->hdr.ether_type, RTE_BE16(0x1234), + "Masked spec ether type mismatch"); + TEST_ASSERT_EQUAL(conv_last->hdr.dst_addr.addr_bytes[0], 0x11, + "Masked last dst byte 0 mismatch"); + TEST_ASSERT_EQUAL(conv_last->hdr.dst_addr.addr_bytes[2], 0x00, + "Masked last dst byte 2 mismatch"); + TEST_ASSERT_EQUAL(conv_last->hdr.src_addr.addr_bytes[1], 0x00, + "Masked last src byte 1 mismatch"); + TEST_ASSERT_EQUAL(conv_last->hdr.ether_type, RTE_BE16(0x5678), + "Masked last ether type mismatch"); + + conv_raw_spec = item[1].spec; + TEST_ASSERT_NOT_NULL(conv_raw_spec, "Converted RAW spec must be set"); + TEST_ASSERT_NOT_NULL(conv_raw_spec->pattern, + "Converted RAW pattern pointer must be set"); + TEST_ASSERT(conv_raw_spec->pattern != raw_pattern, + "Converted RAW pattern must be deep-copied"); + TEST_ASSERT_EQUAL(conv_raw_spec->relative, 1, + "Masked RAW relative mismatch"); + TEST_ASSERT_EQUAL(conv_raw_spec->search, 0, + "Masked RAW search mismatch"); + TEST_ASSERT_EQUAL(conv_raw_spec->offset, raw_spec.offset, + "Masked RAW offset mismatch"); + TEST_ASSERT_EQUAL(conv_raw_spec->limit, raw_spec.limit, + "Masked RAW limit mismatch"); + TEST_ASSERT_EQUAL(conv_raw_spec->length, raw_spec.length, + "Masked RAW length mismatch"); + TEST_ASSERT_BUFFERS_ARE_EQUAL(conv_raw_spec->pattern, raw_pattern, + sizeof(raw_pattern), + "Converted RAW pattern mismatch"); + + return TEST_SUCCESS; +} + static int32_t ethdev_api_queue_status(void) { @@ -167,6 +289,7 @@ static struct unit_test_suite ethdev_api_testsuite = { .setup = NULL, .teardown = NULL, .unit_test_cases = { + TEST_CASE(ethdev_api_flow_conv_pattern_masked), TEST_CASE(ethdev_api_queue_status), /* TODO: Add deferred_start queue status test */ TEST_CASES_END() /**< NULL terminate unit test array */ diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index ca2f85c3fa..e4efcbf10f 100644 --- a/lib/ethdev/rte_flow.c +++ b/lib/ethdev/rte_flow.c @@ -175,6 +175,23 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = { MK_FLOW_ITEM(COMPARE, sizeof(struct rte_flow_item_compare)), }; +static inline size_t +rte_flow_conv_item_mask_size(const struct rte_flow_item *item) +{ + if ((int)item->type < 0) + return sizeof(void *); + switch (item->type) { + case RTE_FLOW_ITEM_TYPE_RAW: + return offsetof(struct rte_flow_item_raw, pattern); + case RTE_FLOW_ITEM_TYPE_GENEVE_OPT: + return offsetof(struct rte_flow_item_geneve_opt, data); + default: + if (rte_flow_desc_item[item->type].desc_fn != NULL) + return 0; + return rte_flow_desc_item[item->type].size; + } +} + /** Generate flow_action[] entry. */ #define MK_FLOW_ACTION(t, s) \ [RTE_FLOW_ACTION_TYPE_ ## t] = { \ @@ -797,6 +814,8 @@ rte_flow_conv_action_conf(void *buf, const size_t size, * RTE_FLOW_ITEM_TYPE_END is encountered. * @param[out] error * Perform verbose error reporting if not NULL. + * @param[in] with_mask + * If true, @p src mask will be applied to spec and last. * * @return * A positive value representing the number of bytes needed to store @@ -809,12 +828,13 @@ rte_flow_conv_pattern(struct rte_flow_item *dst, const size_t size, const struct rte_flow_item *src, unsigned int num, + bool with_mask, struct rte_flow_error *error) { uintptr_t data = (uintptr_t)dst; size_t off; size_t ret; - unsigned int i; + unsigned int i, j; for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) { /** @@ -838,15 +858,27 @@ rte_flow_conv_pattern(struct rte_flow_item *dst, src -= num; dst -= num; do { + uint8_t *c_spec = NULL, *c_last = NULL; + const uint8_t *mask = src->mask; + size_t item_mask_size = mask ? rte_flow_conv_item_mask_size(src) : 0; + if (src->spec) { off = RTE_ALIGN_CEIL(off, sizeof(double)); ret = rte_flow_conv_item_spec ((void *)(data + off), size > off ? size - off : 0, src, RTE_FLOW_CONV_ITEM_SPEC); - if (size && size >= off + ret) + if (size && size >= off + ret) { dst->spec = (void *)(data + off); + c_spec = (uint8_t *)(data + off); + } off += ret; + if (with_mask && c_spec && mask) { + size_t mask_size = RTE_MIN(ret, item_mask_size); + + for (j = 0; j < mask_size; j++) + c_spec[j] &= mask[j]; + } } if (src->last) { @@ -855,9 +887,17 @@ rte_flow_conv_pattern(struct rte_flow_item *dst, ((void *)(data + off), size > off ? size - off : 0, src, RTE_FLOW_CONV_ITEM_LAST); - if (size && size >= off + ret) + if (size && size >= off + ret) { dst->last = (void *)(data + off); + c_last = (uint8_t *)(data + off); + } off += ret; + if (with_mask && c_last && mask) { + size_t mask_size = RTE_MIN(ret, item_mask_size); + + for (j = 0; j < mask_size; j++) + c_last[j] &= mask[j]; + } } if (src->mask) { off = RTE_ALIGN_CEIL(off, sizeof(double)); @@ -1004,7 +1044,7 @@ rte_flow_conv_rule(struct rte_flow_conv_rule *dst, off = RTE_ALIGN_CEIL(off, sizeof(double)); ret = rte_flow_conv_pattern((void *)((uintptr_t)dst + off), size > off ? size - off : 0, - src->pattern_ro, 0, error); + src->pattern_ro, 0, false, error); if (ret < 0) return ret; if (size && size >= off + (size_t)ret) @@ -1104,7 +1144,7 @@ rte_flow_conv(enum rte_flow_conv_op op, ret = sizeof(*attr); break; case RTE_FLOW_CONV_OP_ITEM: - ret = rte_flow_conv_pattern(dst, size, src, 1, error); + ret = rte_flow_conv_pattern(dst, size, src, 1, false, error); break; case RTE_FLOW_CONV_OP_ITEM_MASK: item = src; @@ -1119,7 +1159,7 @@ rte_flow_conv(enum rte_flow_conv_op op, ret = rte_flow_conv_actions(dst, size, src, 1, error); break; case RTE_FLOW_CONV_OP_PATTERN: - ret = rte_flow_conv_pattern(dst, size, src, 0, error); + ret = rte_flow_conv_pattern(dst, size, src, 0, false, error); break; case RTE_FLOW_CONV_OP_ACTIONS: ret = rte_flow_conv_actions(dst, size, src, 0, error); @@ -1139,6 +1179,9 @@ rte_flow_conv(enum rte_flow_conv_op op, case RTE_FLOW_CONV_OP_ACTION_NAME_PTR: ret = rte_flow_conv_name(1, 1, dst, size, src, error); break; + case RTE_FLOW_CONV_OP_PATTERN_MASKED: + ret = rte_flow_conv_pattern(dst, size, src, 0, true, error); + break; default: ret = rte_flow_error_set (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index f864578f80..2261d1ea06 100644 --- a/lib/ethdev/rte_flow.h +++ b/lib/ethdev/rte_flow.h @@ -4544,6 +4544,19 @@ enum rte_flow_conv_op { * @code const char ** @endcode */ RTE_FLOW_CONV_OP_ACTION_NAME_PTR, + + /** + * Convert an entire pattern. + * + * Duplicates all pattern items at once, applying each source item mask + * to its copied specification and range. + * + * - @p src type: + * @code const struct rte_flow_item * @endcode + * - @p dst type: + * @code struct rte_flow_item * @endcode + */ + RTE_FLOW_CONV_OP_PATTERN_MASKED, }; /** -- 2.43.0
