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]> --- v3: - add test code - fix the issue found by AI --- app/test/test_ethdev_api.c | 76 ++++++++++++++++++++++++++ doc/guides/rel_notes/release_26_07.rst | 6 ++ lib/ethdev/rte_flow.c | 46 ++++++++++++++-- lib/ethdev/rte_flow.h | 13 +++++ 4 files changed, 135 insertions(+), 6 deletions(-) diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c index 76afd0345c..5cae1cdc1d 100644 --- a/app/test/test_ethdev_api.c +++ b/app/test/test_ethdev_api.c @@ -4,6 +4,7 @@ #include <rte_log.h> #include <rte_ethdev.h> +#include <rte_flow.h> #include <rte_test.h> #include "test.h" @@ -15,6 +16,80 @@ #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 struct rte_flow_item pattern[] = { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &spec, + .last = &last, + .mask = &mask, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + union { + struct rte_flow_item item; + struct rte_flow_item_eth eth; + double align; + uint8_t raw[256]; + } dst; + const struct rte_flow_item *item; + const struct rte_flow_item_eth *conv_spec; + const struct rte_flow_item_eth *conv_last; + 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.raw), + "Masked pattern conversion needs too much storage"); + + memset(&dst, 0, sizeof(dst)); + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, dst.raw, + sizeof(dst.raw), pattern, NULL); + TEST_ASSERT(ret > 0, "Masked pattern conversion failed"); + + item = (const struct rte_flow_item *)dst.raw; + 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"); + + return TEST_SUCCESS; +} + static int32_t ethdev_api_queue_status(void) { @@ -167,6 +242,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/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst index 8b4f8401e2..46a83cfea5 100644 --- a/doc/guides/rel_notes/release_26_07.rst +++ b/doc/guides/rel_notes/release_26_07.rst @@ -116,6 +116,12 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= +* ethdev: Added masked pattern conversion. + + Added ``RTE_FLOW_CONV_OP_PATTERN_MASKED`` to ``rte_flow_conv()`` + to copy an entire pattern while applying each item's mask to its + ``spec`` and ``last`` fields. + ABI Changes ----------- diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index 7a51b667cf..7cf9f6f6f3 100644 --- a/lib/ethdev/rte_flow.c +++ b/lib/ethdev/rte_flow.c @@ -178,6 +178,14 @@ 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 rte_flow_desc_item[item->type].size; + return sizeof(void *); +} + /** Generate flow_action[] entry. */ #define MK_FLOW_ACTION(t, s) \ [RTE_FLOW_ACTION_TYPE_ ## t] = { \ @@ -835,6 +843,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 @@ -847,12 +857,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) { /** @@ -876,15 +887,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) { @@ -893,9 +916,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)); @@ -1042,7 +1073,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) @@ -1143,7 +1174,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; @@ -1158,7 +1189,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); @@ -1178,6 +1209,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 ba3bcc89a3..f3102f4bb0 100644 --- a/lib/ethdev/rte_flow.h +++ b/lib/ethdev/rte_flow.h @@ -4558,6 +4558,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 @p mask to @p spec + * and @p last. + * + * - @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.34.1
