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 16508A04DC; Mon, 26 Oct 2020 16:21:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B680B1E2B; Mon, 26 Oct 2020 16:21:05 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 9EA8D1D9E for ; Mon, 26 Oct 2020 16:21:04 +0100 (CET) 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 4E5A67F60F; Mon, 26 Oct 2020 18:21:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 4E5A67F60F DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1603725663; bh=FGtKLreybt3Xw68VQDQ3LevFeYfQ/5CjLlvJK3Gyyr8=; h=Subject:From:To:Cc:References:Date:In-Reply-To; b=X/JI1HvGMFB0oq0K9Y2L0+kU42GLnlXqmHogvDQGmm1DD+koJChIMzbogSGXDoNWb 0P2/van7qmgW/haW2vF81W/mojQXZ5Y7PihoDuU+tvz6mqU2CPIIW1Rj7zN2dvfAcl KpEROnbvZY++DU0kgccw+NCJB59xDcnAjS92mRnI= From: Andrew Rybchenko To: Thomas Monjalon , dev@dpdk.org Cc: ferruh.yigit@intel.com, david.marchand@redhat.com, bruce.richardson@intel.com, olivier.matz@6wind.com, akhil.goyal@nxp.com, Yong Wang References: <20201026052105.1561859-1-thomas@monjalon.net> <20201026052105.1561859-10-thomas@monjalon.net> Organization: OKTET Labs Message-ID: Date: Mon, 26 Oct 2020 18:21:03 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 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 09/15] net/vmxnet3: switch MSS hint to dynamic mbuf field 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/26/20 6:14 PM, Andrew Rybchenko wrote: > On 10/26/20 8:20 AM, Thomas Monjalon wrote: >> The segment count, used for MSS guess, >> was stored in the deprecated mbuf field udata64. >> It is moved to a dynamic field in order to allow removal of udata64. >> >> Signed-off-by: Thomas Monjalon >> --- >> drivers/net/vmxnet3/vmxnet3_ethdev.c | 15 +++++++++++++++ >> drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 ++ >> drivers/net/vmxnet3/vmxnet3_rxtx.c | 10 ++++++---- >> 3 files changed, 23 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c >> index 6920ab568c..eeb1152b61 100644 >> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c >> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c >> @@ -59,6 +59,8 @@ >> DEV_RX_OFFLOAD_JUMBO_FRAME | \ >> DEV_RX_OFFLOAD_RSS_HASH) >> >> +int vmxnet3_segs_dynfield_offset; >> + >> static int eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev); >> static int eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev); >> static int vmxnet3_dev_configure(struct rte_eth_dev *dev); >> @@ -233,6 +235,11 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev) >> struct vmxnet3_hw *hw = eth_dev->data->dev_private; >> uint32_t mac_hi, mac_lo, ver; >> struct rte_eth_link link; >> + static const struct rte_mbuf_dynfield vmxnet3_segs_dynfield_desc = { >> + .name = "rte_net_vmxnet3_dynfield_segs", >> + .size = sizeof(uint8_t), >> + .align = __alignof__(uint8_t), >> + }; >> >> PMD_INIT_FUNC_TRACE(); >> >> @@ -242,6 +249,14 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev) >> eth_dev->tx_pkt_prepare = vmxnet3_prep_pkts; >> pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); >> >> + /* extra mbuf field is required to guess MSS */ >> + vmxnet3_segs_dynfield_offset = >> + rte_mbuf_dynfield_register(&vmxnet3_segs_dynfield_desc); >> + if (vmxnet3_segs_dynfield_offset < 0) { >> + PMD_INIT_LOG(ERR, "Cannot register mbuf field."); >> + return -rte_errno; >> + } >> + >> /* >> * for secondary processes, we don't initialize any further as primary >> * has already done this work. >> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h >> index dd685b02b7..5730e94d50 100644 >> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h >> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h >> @@ -193,4 +193,6 @@ uint16_t vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, >> uint16_t vmxnet3_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, >> uint16_t nb_pkts); >> >> +extern int vmxnet3_segs_dynfield_offset; >> + >> #endif /* _VMXNET3_ETHDEV_H_ */ >> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c >> index e10f9ee870..6655622f94 100644 >> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c >> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c >> @@ -674,6 +674,7 @@ vmxnet3_guess_mss(struct vmxnet3_hw *hw, const Vmxnet3_RxCompDesc *rcd, >> struct rte_ipv6_hdr *ipv6_hdr; >> struct rte_tcp_hdr *tcp_hdr; >> char *ptr; >> + uint8_t segs; >> >> RTE_ASSERT(rcd->tcp); >> >> @@ -710,9 +711,9 @@ vmxnet3_guess_mss(struct vmxnet3_hw *hw, const Vmxnet3_RxCompDesc *rcd, >> tcp_hdr = (struct rte_tcp_hdr *)(ptr + hlen); >> hlen += (tcp_hdr->data_off & 0xf0) >> 2; >> >> - if (rxm->udata64 > 1) >> - return (rte_pktmbuf_pkt_len(rxm) - hlen + >> - rxm->udata64 - 1) / rxm->udata64; >> + segs = *RTE_MBUF_DYNFIELD(rxm, vmxnet3_segs_dynfield_offset, uint8_t *); >> + if (segs > 1) >> + return (rte_pktmbuf_pkt_len(rxm) - hlen + segs - 1) / segs; >> else >> return hw->mtu - hlen + sizeof(struct rte_ether_hdr); >> } >> @@ -737,7 +738,8 @@ vmxnet3_rx_offload(struct vmxnet3_hw *hw, const Vmxnet3_RxCompDesc *rcd, >> (const Vmxnet3_RxCompDescExt *)rcd; >> >> rxm->tso_segsz = rcde->mss; >> - rxm->udata64 = rcde->segCnt; >> + *RTE_MBUF_DYNFIELD(rxm, vmxnet3_segs_dynfield_offset, >> + uint8_t *) = rcde->segCnt; >> ol_flags |= PKT_RX_LRO; >> } >> } else { /* Offloads set in eop */ >> > > I think it should be a rule of thumb to introduce helper > macro to access a dynamic field (as you do in few of > previous patches). > > It should be just nearby declaration of the the offset > variable. > May be inline function is even better since, IMHO, if you both ways are possible, inline function is the right choice. In this particular case inline function will not add value from type safety point of view, but it is still better as an example to follow. In some case inline function could be used as a place to put build assertion to check size.