> -----Original Message----- > From: Ananyev, Konstantin <konstantin.anan...@intel.com> > Sent: Tuesday, 26 April 2022 20:57 > To: Ido Goshen <i...@cgstowernetworks.com>; us...@dpdk.org; > dev@dpdk.org > Cc: Konstantin Ananyev <konstantin.v.anan...@yandex.ru> > Subject: RE: Does ACL support field size of 8 bytes? > > > Hi Ido, > > > I've lots of good experience with ACL but can't make it work with u64 > > values I know it can be split to 2xu32 fields, but it makes it more > > complex to use and a wastes double number of fields (we hit the > > RTE_ACL_MAX_FIELDS 64 limit) > > Wow, that's a lot of fields...
[idog] We provide a general purpose packet-broker that covers wide range of l2-l4 protocols + tunnels + some app level metadata. Though in most cases they won't be used simultaneously and many fields may end up being don't-care (e.g. mask=0) it's easier to code a static rte_acl_field_def struct that covers all the options then constructing it dynamically on each user configuration change > > According to the documentation and rte_acl.h fields size can be 8[idog] > > ... > > Should it work? > > Did anyone try it successfully and/or can share an example? > > You are right: though it is formally supported, we do not test it, and AFAIK > no- > one used it till now. > As we do group fields by 4B long chunks anyway, 8B field is sort of awkward > and > confusing. > To be honest, I don't even remember what was the rationale beyond introducing > it at first place. > Anyway, just submitted patches that should fix 8B field support (at least it > works > for me now): > https://patches.dpdk.org/project/dpdk/list/?series=22676 > Please give it a try. [idog] The patch works great for me. Thanx! > In long term it probably would be good to hear from you and other users, > should > we keep 8B support at all, or might be it would be easier just to abandon it. > Thanks > Konstantin [idog] I find the 8B option very useful: 1. It's easier and more natural to use for long size fields e.g. part of how it simplifies our MAC rules code @@ -231,48 +231,34 @@ struct rte_acl_field_def acl_fields[] = { { .type = RTE_ACL_FIELD_TYPE_BITMASK, - .size = sizeof(uint32_t), - .field_index = FIELD_MAC_SRC_4MSB, + .size = sizeof(uint64_t), + .field_index = FIELD_MAC_SRC, .input_index = INPUT_INDEX_GROUP_2, .offset = offsetof(struct acl_data, mac_src), }, - { - .type = RTE_ACL_FIELD_TYPE_BITMASK, - .size = sizeof(uint16_t), - .field_index = FIELD_MAC_SRC_2LSB, - .input_index = INPUT_INDEX_GROUP_3, - .offset = offsetof(struct acl_data, mac_src) + sizeof(uint32_t), - }, . . . +static int get_mac_val(const char *in, uint64_t *mac) { - static const size_t MAC_4MSB_SIZE = sizeof(uint32_t); - static const size_t MAC_2LSB_SIZE = sizeof(uint16_t); uint32_t i = 0; uint8_t octet = 0; char dlm = ':'; - - for (i = 0; i < MAC_4MSB_SIZE; i++) - { - GET_CB_FIELD(in, octet, 16, UINT8_MAX, dlm); - ((uint8_t*)mac4msb)[MAC_4MSB_SIZE - 1 - i] = octet; - } - for (i = 0; i < MAC_2LSB_SIZE; i++) + *mac = 0; + for (i = 0; i < RTE_ETHER_ADDR_LEN; i++) { - if (i == MAC_2LSB_SIZE - 1) + if (i == RTE_ETHER_ADDR_LEN - 1) dlm = 0; GET_CB_FIELD(in, octet, 16, UINT8_MAX, dlm); - ((uint8_t*)mac2lsb)[MAC_2LSB_SIZE - 1 - i] = octet; + ((uint8_t*)mac)[RTE_ETHER_ADDR_LEN + 1 - i] = octet; } return 0; } It' even much more significant for RTE_ACL_FIELD_TYPE_RANGE that may require breaking a single U64 range to 3 U32 based rules 2. It may save acl fields Alternative is to increase RTE_ACL_MAX_FIELDS (maybe expose it to rte_config.h) As long as the "64" doesn't stand for some algorithmic/performance reason
