From: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> To: Ido Goshen <Ido@cgstowernetworks.com>, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "users@dpdk.org" <users@dpdk.org>, "dev@dpdk.org" <dev@dpdk.org> Subject: Re: Does ACL support field size of 8 bytes? Date: Sun, 15 May 2022 21:53:42 +0100 Message-ID: <59727248-59ac-ab74-5ac3-e6eb7163647e@yandex.ru> (raw) In-Reply-To: <AS4PR09MB5525DD1B32B8C0AAA2AB0C17D6C89@AS4PR09MB5525.eurprd09.prod.outlook.com> 11/05/2022 15:28, Ido Goshen пишет: > > >> -----Original Message----- >> From: Ananyev, Konstantin <konstantin.ananyev@intel.com> >> Sent: Tuesday, 26 April 2022 20:57 >> To: Ido Goshen <Ido@cgstowernetworks.com>; users@dpdk.org; >> dev@dpdk.org >> Cc: Konstantin Ananyev <konstantin.v.ananyev@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 My concern was it is sort of awkward in terms of input_field value for rules with 8B long. But sure, if you believe it is useful, then let's try to keep it. I submitted v2, there is no change in the library itself, just updated the test script to cover new case. If you'll have a chance, please add 'tested-by:' tag to it. > > 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 I kept RTE_ACL_MAX_FIELDS, but internally had to increase max number of input fields up to 2 * RTE_ACL_MAX_FIELDS, to cover the situation when all fields are 8B long. Thanks Konstantin
next prev parent reply other threads:[~2022-05-15 20:53 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-13 13:55 Ido Goshen 2022-04-26 17:56 ` Ananyev, Konstantin 2022-04-26 17:58 ` Fwd: " Konstantin Ananyev 2022-05-11 14:28 ` Ido Goshen 2022-05-15 20:53 ` Konstantin Ananyev [this message] 2022-05-16 6:28 ` Ido Goshen 2022-05-17 23:43 ` Konstantin Ananyev
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=59727248-59ac-ab74-5ac3-e6eb7163647e@yandex.ru \ --to=konstantin.v.ananyev@yandex.ru \ --cc=Ido@cgstowernetworks.com \ --cc=dev@dpdk.org \ --cc=konstantin.ananyev@intel.com \ --cc=users@dpdk.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK usage discussions This inbox may be cloned and mirrored by anyone: git clone --mirror http://inbox.dpdk.org/users/0 users/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 users users/ http://inbox.dpdk.org/users \ users@dpdk.org public-inbox-index users Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.users AGPL code for this site: git clone https://public-inbox.org/public-inbox.git