From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 13FC3A04DB; Fri, 16 Oct 2020 11:27:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E36FB1EBFC; Fri, 16 Oct 2020 11:27:54 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 26A651EBF8 for ; Fri, 16 Oct 2020 11:27:53 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id D3C837F60F; Fri, 16 Oct 2020 12:27:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru D3C837F60F DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1602840471; bh=+sSYZh0XvPV3Ow4081OK01m3744Jj95H9PPnfCaiDlo=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=F28k+1layzJ0b/VoQ0RkcV8m5WJWa/jP55qkF4EdSg2hPoeV8EnfXAZF9eVQZ5SaM um733ee5HJ2svx/n3HVRHiadw7gRtMspILQIZGu9WN/aMlErfB1M3SYZ0ChA0B9bSf H7iIcIz069SlhjQz53RqzTevLdslpej0mzLHExbo= To: Slava Ovsiienko , "dev@dpdk.org" Cc: NBU-Contact-Thomas Monjalon , "stephen@networkplumber.org" , "ferruh.yigit@intel.com" , "olivier.matz@6wind.com" , "jerinjacobk@gmail.com" , "maxime.coquelin@redhat.com" , "david.marchand@redhat.com" References: <1602834519-8696-1-git-send-email-viacheslavo@nvidia.com> <1602834519-8696-2-git-send-email-viacheslavo@nvidia.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <6084716a-314d-5d8d-0d32-060fb682f81a@oktetlabs.ru> Date: Fri, 16 Oct 2020 12:27:51 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 1/6] ethdev: introduce Rx buffer split X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/16/20 12:15 PM, Slava Ovsiienko wrote: > Hi, Andrew > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Friday, October 16, 2020 11:52 >> To: Slava Ovsiienko ; dev@dpdk.org >> Cc: NBU-Contact-Thomas Monjalon ; >> stephen@networkplumber.org; ferruh.yigit@intel.com; >> olivier.matz@6wind.com; jerinjacobk@gmail.com; >> maxime.coquelin@redhat.com; david.marchand@redhat.com >> Subject: Re: [PATCH v8 1/6] ethdev: introduce Rx buffer split >> >> On 10/16/20 10:48 AM, Viacheslav Ovsiienko wrote: >>> The DPDK datapath in the transmit direction is very flexible. >>> An application can build the multi-segment packet and manages almost >>> all data aspects - the memory pools where segments are allocated from, >>> the segment lengths, the memory attributes like external buffers, >>> registered for DMA, etc. >>> > [snip] >>> +struct rte_eth_rxseg { >>> + union { >> >> Why not just 'union rte_eth_rxseg' ? >> >>> + /* The settings for buffer split offload. */ >>> + struct rte_eth_rxseg_split split; >> >> Pointer to a split table must be here. I.e. >> struct rte_eth_rxseg_split *split; > OK, will try to simplify with that, thanks. > >> Also it must be specified how the array is terminated. >> We need either a number of define last item condition (mp == NULL ?) > > We have one, please see: "rte_eth_rxconf->rx_nseg" A bit confusing to have it outside of union far from split but may be acceptable for the experimental API... If there are better ideas - welcome. >> >>> + /* The other features settings should be added here. */ >>> + } conf; >>> +}; >> >> >> >>> + >>> +/** >>> * A structure used to configure an RX ring of an Ethernet port. >>> */ >>> struct rte_eth_rxconf { >>> @@ -977,6 +998,46 @@ struct rte_eth_rxconf { >>> uint16_t rx_free_thresh; /**< Drives the freeing of RX descriptors. */ >>> uint8_t rx_drop_en; /**< Drop packets if no descriptors are available. >> */ >>> + struct rte_eth_rxseg *rx_seg; >> >> It must not be a pointer. It looks really strange this way taking into account >> that it is a union in fact. >> Also, why is it put here in the middle of exsiting structure? >> IMHO it should be added after offlaods. > OK, agree, will move. > >> >>> /** >>> * Per-queue Rx offloads to be set using DEV_RX_OFFLOAD_* flags. >>> * Only offloads set on rx_queue_offload_capa or rx_offload_capa @@ >>> -1260,6 +1321,7 @@ struct rte_eth_conf { >>> #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 >>> #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 >>> #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 >>> +#define RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT 0x00100000 >>> >>> #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ >>> DEV_RX_OFFLOAD_UDP_CKSUM | \ >>> @@ -1376,6 +1438,17 @@ struct rte_eth_switch_info { }; >>> >>> /** >>> + * Ethernet device Rx buffer segmentation capabilities. >>> + */ >>> +__extension__ >>> +struct rte_eth_rxseg_capa { >>> + uint16_t max_seg; /**< Maximum amount of segments to split. */ >> >> May be 'max_segs' to avoid confusing vs maximum segment length. >> > OK, max_nseg would be more appropriate. Agreed. > >>> + uint16_t multi_pools:1; /**< Supports receiving to multiple pools.*/ >>> + uint16_t offset_allowed:1; /**< Supports buffer offsets. */ >>> + uint16_t offset_align_log2:4; /**< Required offset alignment. */ >> >> 4 bits are even insufficient to specify cache-line alignment. >> IMHO at least 8 bits are required. > > 4 bits seems to be quite sufficient. It is a log2, tells how many lsbs in offset should be zeroes. > 2^15 is 32K, it covers all reasonable alignments for uint16_t type. > >> >> Consider to put 32 width bit-fields at start of the structure. >> Than, max_segs (16), offset_align_log2 (8), plus reserved (8). > OK, np. > > With best regards, Slava >