DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: "Ye, Xiaolong" <xiaolong.ye@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"olivier.matz@6wind.com" <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v8] net/ice: optimize protocol extraction by dynamic mbuf API
Date: Thu, 7 Nov 2019 10:38:27 +0000
Message-ID: <E3B9F2FDCB65864C82CD632F23D8AB8773D8FA36@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20191107090838.GE100445@intel.com>

Hi Xiaolong,

> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Thursday, November 7, 2019 17:09
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; olivier.matz@6wind.com
> Subject: Re: [PATCH v8] net/ice: optimize protocol extraction by dynamic mbuf API
> 
> Hi, Haiyue
> 
> On 11/07, Haiyue Wang wrote:
> >The original design is to use rte_mbuf::udata64 to save the metadata of
> >protocol extraction which has network protocol data fields and type, a
> >private API is used to decode this metadata.
> >
> >Use the dynamic mbuf field and flags to register the needed fields in
> >mbuf, to avoid overwriting 'rte_mbuf::udata64' if the application uses
> >it. It only needs 4B size to save the protocol extraction data, and its
> >type and validity is indicated by related bit in 'rte_mbuf::ol_flags'.
> >
> >Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >---
> >v8: - Cleanup the doxgen, rename the dynamic filed and flags with ice prefix.
> >
> >v7: - Change the log level from ERR to DEBUG for a successful registration,
> >      ERR is used to develop firstly.
> >
> >v6: - Don't need to register all flags when if one dev_args is set, register
> >      it as required.
> >
> >v5: - Remove the '_OL/_ol' in dynamic mbuf flag to make the variable clean.
> >
> >v4: - Include the 'rte_pmd_ice.h' header in ICE source as needed, reduce
> >      its compile scope.
> >
> >v3: - Use the conventions name "rte_net_<pmd>_dynfield_<name>" since it
> >      is defined in a in PMD.
> >    - Add helpers API for easiy access.
> >
> >v2: - disable the protocol extraction if failed to register some ol_flags
> >    - rewrite the commit message
> >
> > doc/api/doxy-api-index.md               |   1 +
> > doc/api/doxy-api.conf.in                |   1 +
> > doc/guides/nics/ice.rst                 |  14 +-
> > drivers/net/ice/ice_ethdev.c            |  84 ++++++-
> > drivers/net/ice/ice_ethdev.h            |   9 +
> > drivers/net/ice/ice_rxtx.c              |  88 ++++---
> > drivers/net/ice/ice_rxtx.h              |   1 -
> > drivers/net/ice/rte_pmd_ice.h           | 312 +++++++++++++++---------
> > drivers/net/ice/rte_pmd_ice_version.map |  12 +
> > 9 files changed, 369 insertions(+), 153 deletions(-)
> >
> 
> [snip]
> 
> >
> >-  The extraction will be copied into the lower 32 bit of ``rte_mbuf::udata64``.
> >+  The extraction metadata will be copied into the registered dynamic mbuf field
> >+  with, and the related dynamic mbuf flags is set.
> 
> Above 'with' is redundant?

Yes, will fix next version.

> 
> >
> >   .. table:: Protocol extraction : ``vlan``
> >
> >@@ -175,10 +176,11 @@ Runtime Config Options
> >
> >   TCPHDR2 - Reserved
> >
> >-  Use ``get_proto_xtr_flds(struct rte_mbuf *mb)`` to access the protocol
> >-  extraction, do not use ``rte_mbuf::udata64`` directly.
> >+  Use ``rte_net_ice_dynf_proto_xtr_metadata_get`` to access the protocol
> >+  extraction metadata, and use ``PKT_RX_DYNF_PROTO_XTR_*`` to get the
> >+  metadata type of ``struct rte_mbuf::ol_flags``.
> >
> >-  The ``dump_proto_xtr_flds(struct rte_mbuf *mb)`` routine shows how to
> >+  The ``rte_net_ice_dump_proto_xtr_metadata`` routine shows how to
> >   access the protocol extraction result in ``struct rte_mbuf``.
> >
> 
> [snip]
> 
> >+
> >+	PMD_DRV_LOG(DEBUG,
> >+		    "Protocol extraction metadata offset in mbuf is : %d",
> >+		    offset);
> >+	rte_net_ice_dynfield_proto_xtr_metadata = offset;
> 
> Seems rte_net_ice_dump_proto_xtr_metadata_off is a better name judging from
> its real meaning.
> 

Make sense, will change it.

> >+
> >+	for (i = 0; i < RTE_DIM(ice_proto_xtr_ol_flag_params); i++) {
> >+		ol_flag = &ice_proto_xtr_ol_flag_params[i];
> >+
> >+		if (!ol_flag->required)
> 
> [snip]
> 
> >+static void
> >+ice_rxd_to_proto_xtr(struct rte_mbuf *mb,
> >+		     volatile struct ice_32b_rx_flex_desc_comms *desc)
> >+{
> >+	uint16_t stat_err = rte_le_to_cpu_16(desc->status_error1);
> >+	uint32_t metadata;
> >+	uint64_t ol_flag;
> >+
> >+	if (unlikely(!(stat_err & ICE_RX_PROTO_XTR_VALID)))
> >+		return;
> >+
> >+	ol_flag = ice_rxdid_to_proto_xtr_ol_flag(desc->rxdid);
> 
> ol_flag here is obtained through offset which is returned by
> rte_mbuf_dynflag_register, will it have any chance to conflict with
> existing offload flags such as PKT_RX_VLAN, PKT_RX_VLAN_STRIPPED, ...?
> 

This API will choose between "PKT_FIRST_FREE" and "PKT_LAST_FREE",
not conflict. ;-)

> >+	if (unlikely(!ol_flag))
> >+		return;
> >+
> >+	mb->ol_flags |= ol_flag;
> >+
> 
> [snip]
> 
> >--- a/drivers/net/ice/rte_pmd_ice_version.map
> >+++ b/drivers/net/ice/rte_pmd_ice_version.map
> >@@ -2,3 +2,15 @@ DPDK_19.02 {
> >
> > 	local: *;
> > };
> >+
> >+EXPERIMENTAL {
> >+	global:
> >+
> >+	# added in 19.11
> >+	rte_net_ice_dynfield_proto_xtr_metadata;
> >+	rte_net_ice_dynflag_proto_xtr_vlan;
> >+	rte_net_ice_dynflag_proto_xtr_ipv4;
> >+	rte_net_ice_dynflag_proto_xtr_ipv6;
> >+	rte_net_ice_dynflag_proto_xtr_ipv6_flow;
> >+	rte_net_ice_dynflag_proto_xtr_tcp;
> 
> Why put the variable other than the experimental api in the map?
> 

The experimental API is static and inline, not global, so no need
to be put here. But the above variables are global, so need to be
put here for compiling shared.

Please refer to: git log -p lib/librte_ethdev/rte_ethdev_version.map

> Thanks,
> Xiaolong
> 
> >+};
> >--
> >2.17.1
> >

  reply	other threads:[~2019-11-07 10:38 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-26 17:44 [dpdk-dev] [PATCH v1] net/ice: use dynamic mbuf API to handle protocol extraction Haiyue Wang
2019-10-29  7:34 ` [dpdk-dev] [PATCH v2] net/ice: optimize protocol extraction by dynamic mbuf API Haiyue Wang
2019-10-30 16:56   ` Olivier Matz
2019-10-31  1:16     ` Wang, Haiyue
2019-11-01  3:17   ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2019-11-01 12:53     ` [dpdk-dev] [PATCH v4] " Haiyue Wang
2019-11-01 16:19       ` [dpdk-dev] [PATCH v5] " Haiyue Wang
2019-11-05  1:19         ` [dpdk-dev] [PATCH v6] " Haiyue Wang
2019-11-05  1:26           ` [dpdk-dev] [PATCH v7] " Haiyue Wang
2019-11-07  5:35           ` [dpdk-dev] [PATCH v8] " Haiyue Wang
2019-11-07  9:08             ` Ye Xiaolong
2019-11-07 10:38               ` Wang, Haiyue [this message]
2019-11-07 10:44           ` [dpdk-dev] [PATCH v9] " Haiyue Wang
2019-11-08  2:35             ` Ye Xiaolong
2019-11-08 12:34             ` Thomas Monjalon
2019-11-08 14:08               ` Wang, Haiyue
2019-11-08 14:39                 ` Thomas Monjalon
2019-11-08 15:04                   ` Wang, Haiyue
2019-11-08 15:03               ` Ye Xiaolong
2019-11-08 12:55             ` Thomas Monjalon
2019-11-08 14:01               ` Wang, Haiyue
2019-11-08 14:38                 ` Thomas Monjalon
2019-11-08 15:44           ` [dpdk-dev] [PATCH v10 0/2] " Haiyue Wang
2019-11-08 15:44             ` [dpdk-dev] [PATCH v10 1/2] " Haiyue Wang
2019-11-08 15:44             ` [dpdk-dev] [PATCH v10 2/2] doc: add the ice PMD doxygen Haiyue Wang
2019-11-08 15:58             ` [dpdk-dev] [PATCH v10 0/2] net/ice: optimize protocol extraction by dynamic mbuf API Thomas Monjalon
2019-11-09  1:31               ` Ye Xiaolong

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=E3B9F2FDCB65864C82CD632F23D8AB8773D8FA36@shsmsx102.ccr.corp.intel.com \
    --to=haiyue.wang@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=xiaolong.ye@intel.com \
    /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 patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/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 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git