From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8EC45A00BE; Sun, 15 May 2022 22:53:48 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2B1EA40A7A; Sun, 15 May 2022 22:53:48 +0200 (CEST) Received: from forward500o.mail.yandex.net (forward500o.mail.yandex.net [37.140.190.195]) by mails.dpdk.org (Postfix) with ESMTP id C455440A79; Sun, 15 May 2022 22:53:46 +0200 (CEST) Received: from vla3-424b09af403e.qloud-c.yandex.net (vla3-424b09af403e.qloud-c.yandex.net [IPv6:2a02:6b8:c15:3511:0:640:424b:9af]) by forward500o.mail.yandex.net (Yandex) with ESMTP id 44F0594183E; Sun, 15 May 2022 23:53:46 +0300 (MSK) Received: from vla3-23c3b031fed5.qloud-c.yandex.net (vla3-23c3b031fed5.qloud-c.yandex.net [2a02:6b8:c15:2582:0:640:23c3:b031]) by vla3-424b09af403e.qloud-c.yandex.net (mxback/Yandex) with ESMTP id CM4CgcT9W4-rjg4KqIL; Sun, 15 May 2022 23:53:46 +0300 X-Yandex-Fwd: 2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1652648026; bh=ygbm9ndeAJKVknWI6lZCxM2HHg4BdjMn+dqztOFSouQ=; h=In-Reply-To:From:Subject:References:Date:Message-ID:To; b=wbblTDIg7pXTz+DpafHRskPk9HHcoyjmHmQfKg3p0LbuYfT3kiqsRj7feen/+tGtt 8BDIRk8aEDH5zK8kk+pc5rZL6/dgkzzxdw4QvFF55VcT6pOb5gek6ySYoV/QpozBzK soo+7ITI74Yce1fOdu+k0sfN8Jii4qnYqg0xoUn8= Authentication-Results: vla3-424b09af403e.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Received: by vla3-23c3b031fed5.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id 1jShmREGWm-riO81gFs; Sun, 15 May 2022 23:53:45 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) Message-ID: <59727248-59ac-ab74-5ac3-e6eb7163647e@yandex.ru> Date: Sun, 15 May 2022 21:53:42 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: Does ACL support field size of 8 bytes? Content-Language: en-US To: Ido Goshen , "Ananyev, Konstantin" , "users@dpdk.org" , "dev@dpdk.org" References: From: Konstantin Ananyev In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 11/05/2022 15:28, Ido Goshen пишет: > > >> -----Original Message----- >> From: Ananyev, Konstantin >> Sent: Tuesday, 26 April 2022 20:57 >> To: Ido Goshen ; users@dpdk.org; >> dev@dpdk.org >> Cc: Konstantin Ananyev >> 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