From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 6C085A04B5;
	Tue, 27 Oct 2020 11:45:50 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id C08A72BDB;
	Tue, 27 Oct 2020 11:45:48 +0100 (CET)
Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com
 [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id BD80B2BD5
 for <dev@dpdk.org>; Tue, 27 Oct 2020 11:45:46 +0100 (CET)
Received: by mail-wr1-f66.google.com with SMTP id t9so1288325wrq.11
 for <dev@dpdk.org>; Tue, 27 Oct 2020 03:45:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google;
 h=date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:in-reply-to:user-agent;
 bh=RP4u4/4KQOCe84MAan553xvQach/ZDd00qC3LBVJAcU=;
 b=ZgFGiBn56t9Cm/r7Uo9uyRH1VyayUpAyGfF+KnCvShWYZ75d70pdEUk7T/ImaLsQwj
 Z3+gKIWiY+rYg3P/JlZDYv0+8NhmuS1d+jXgjuJPDBx17blED3LGLfsujxnYO6UHDaog
 l8Z5nZLSfywP0jko60vUXb+tXWl3djXMmUJ8tv1PII/pFj2h7VFx1eoxSaVjaHhUG068
 F0nT51PZNiNx9TVNKwG0w1MsIPUAZFFAcwMh64IMYP3t42tElXD9bkrX07QigEwAHYH+
 0g17NAaXx+F+ydxdw6IEpUfzj7hPGz0AQKTKw8bmjShaY/r7MdamHbWiwPRFm1ChEVjD
 /8Mg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to:user-agent;
 bh=RP4u4/4KQOCe84MAan553xvQach/ZDd00qC3LBVJAcU=;
 b=Rpm4gFz3gTGamt4HEwwvOjH+41jJZT8oAVAU7GI74aFxow5UiactDfVCUypY1qgETX
 RVkWXjkkGEhHfHP8ofP/aU1s68ToxpBG3dLADIZK6egGiKXAnUuYxIUGi2RWCej/2suU
 2M/4G142q8yz8sLOdpHnoA049OrRO+46DUD+P+uvQ9C1AUPiCHGSneL6cxuGZ+UVbrnf
 YSF5/OmtG3R+3Vz48dR+V3KkzPScpr0POVfU7p5m+pV2N1/l2v0tI0eiKlF+IRDfHPLe
 t0ufczR5+giDzbdzfR5V1b92GsWnQst0kNxpDpLpoX532RvazUU3TjtW5MEODnnmzKdg
 ZFMA==
X-Gm-Message-State: AOAM532uOmvjeAfCZyiaHIZ5z9fqlsov8cD134XKzMPYSCC4rra8VPNz
 qznI49Ph/CIhhJagondcTTRv7Q==
X-Google-Smtp-Source: ABdhPJyTdHLSHL7nhU8vmCNRX6BFJyLAoGG3wdQr84HjhsxD1E3rqb9KNBNgC5NwYyrXuxGuSO9LKA==
X-Received: by 2002:adf:ff8e:: with SMTP id j14mr2170377wrr.255.1603795546451; 
 Tue, 27 Oct 2020 03:45:46 -0700 (PDT)
Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr.
 [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0])
 by smtp.gmail.com with ESMTPSA id b18sm1480676wmj.41.2020.10.27.03.45.45
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Tue, 27 Oct 2020 03:45:45 -0700 (PDT)
Date: Tue, 27 Oct 2020 11:45:45 +0100
From: Olivier Matz <olivier.matz@6wind.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, ferruh.yigit@intel.com, david.marchand@redhat.com,
 bruce.richardson@intel.com, andrew.rybchenko@oktetlabs.ru,
 akhil.goyal@nxp.com, Yong Wang <yongwang@vmware.com>
Message-ID: <20201027104545.GR1898@platinum>
References: <20201026052105.1561859-1-thomas@monjalon.net>
 <20201026222013.2147904-1-thomas@monjalon.net>
 <20201026222013.2147904-10-thomas@monjalon.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201026222013.2147904-10-thomas@monjalon.net>
User-Agent: Mutt/1.10.1 (2018-07-13)
Subject: Re: [dpdk-dev] [PATCH v2 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Oct 26, 2020 at 11:20:07PM +0100, 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 <thomas@monjalon.net>

After seeing this commit, I wonder if we shouldn't introduce an
enhancement in the dynamic field API.

Previously, the driver used udata64 only internally, so without any
risk. The risky usages of udata64 were when the mbuf goes out of a
module.

Changing to dynamic field makes the code safe for any use, but consumes
more memory.

I wonder if we shouldn't (later) introduce a flag RTE_MBUF_DYN_F_SHARED,
or something similar, to say that this field is only used inside a
module, and that its memory can be shared with other dynamic fields.


> ---
>  drivers/net/vmxnet3/vmxnet3_ethdev.c | 15 +++++++++++++++
>  drivers/net/vmxnet3/vmxnet3_ethdev.h |  6 ++++++
>  drivers/net/vmxnet3/vmxnet3_rxtx.c   |  9 +++++----
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 6920ab568c..6e99b67b5a 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 = VMXNET3_SEGS_DYNFIELD_NAME,
> +		.size = sizeof(VMXNET3_SEGS_DYNFIELD_TYPE),
> +		.align = __alignof__(VMXNET3_SEGS_DYNFIELD_TYPE),
> +	};
>  
>  	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..e35edf07be 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> @@ -193,4 +193,10 @@ 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;
> +#define VMXNET3_SEGS_DYNFIELD_NAME "rte_net_vmxnet3_dynfield_segs"
> +#define VMXNET3_SEGS_DYNFIELD_TYPE uint8_t
> +#define VMXNET3_SEGS_DYNFIELD(mbuf) (*RTE_MBUF_DYNFIELD(rxm, \
> +		vmxnet3_segs_dynfield_offset, VMXNET3_SEGS_DYNFIELD_TYPE *))
> +
>  #endif /* _VMXNET3_ETHDEV_H_ */
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index e10f9ee870..2b0e2e6f19 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 = VMXNET3_SEGS_DYNFIELD(rxm);
> +	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,7 @@ vmxnet3_rx_offload(struct vmxnet3_hw *hw, const Vmxnet3_RxCompDesc *rcd,
>  					(const Vmxnet3_RxCompDescExt *)rcd;
>  
>  			rxm->tso_segsz = rcde->mss;
> -			rxm->udata64 = rcde->segCnt;
> +			VMXNET3_SEGS_DYNFIELD(rxm) = rcde->segCnt;
>  			ol_flags |= PKT_RX_LRO;
>  		}
>  	} else { /* Offloads set in eop */
> -- 
> 2.28.0
>