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 3634CA0093; Thu, 21 Apr 2022 12:28:08 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D406640042; Thu, 21 Apr 2022 12:28:07 +0200 (CEST) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by mails.dpdk.org (Postfix) with ESMTP id EB50540040 for ; Thu, 21 Apr 2022 12:28:06 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id C402F3202135; Thu, 21 Apr 2022 06:28:02 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 21 Apr 2022 06:28:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1650536882; x= 1650623282; bh=d30N/AheYtPxvAQuFibBeE3OFp+Rg/D+QSZLKtUQz7g=; b=U yoFil4upWiio8JozbdZDqzPlMj0YRBSxzUqu0hYrTcLEycRLYXoYvq82/4nOu5U3 MQs3yiGhrh+DH51S2QVZcuGTan4QLz2DG9H3I99MZ16jePcIDWD5kGOYKvmS2IXs wfqXT/Xg/UMSDtaiAEjWsx9wXVkOFgFG3kSqL2Tyl8oJ7A90Lpkipw5n/32kAJnj 3DcKrRgJ17qajSBCW6zXm4NOZkxMHtphEVviKXGR/dbnPorHyVacLiD5U88DvbV9 nF0TZppZ32cPKn7Gwhr4rWv3nVcGt9SFSO+sHWqNXEqesnsvyvywF4US3pEAYClj rBHcNooFHkyaMJEss78Pw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1650536882; x=1650623282; bh=d30N/AheYtPxv AQuFibBeE3OFp+Rg/D+QSZLKtUQz7g=; b=TbaFwrBoHMO+Q8QzxhyqYyXwb0pY7 rLP73L18+LCoDYXnChb0X9/RZXICUFwPR+kSfArJLG9EBbTNniCLWYeXZ/wiwagW A3SbQNnRU4SGSm0SCcLGFA/iujqRXnNpfbvtulQMf8zqvyky4UbOgdsucDW1jQ65 aJ4GTEMJTbNo1Y/eUlqDhQusLXXsdZIRieyaZtukjfwXL42FW3rGdeO5TJRBc1hS 5AYtJ3uqHxdUG7LmVZaKYl002C6LiMBV6dx5BnLB+HBSY4YAFQS2JT1FxsAl4XY9 U8Rlo6bLc16Yj+Y+bMZ5TM6JRzOrTPZxQc2lceuL/lQbwwX6UE1rmJ4KQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrtddvgddvjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddtieek gfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 21 Apr 2022 06:27:59 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko , "Wu, WenxuanX" , "Li, Xiaoyun" , "Singh, Aman Deep" , "Zhang, Yuying" , "Zhang, Qi Z" , dev@dpdk.org Cc: "dev@dpdk.org" , "stephen@networkplumber.org" , "mb@smartsharesystems.com" , "viacheslavo@nvidia.com" , "Yu, Ping" , "Wang, YuanX" , "david.marchand@redhat.com" , Ferruh Yigit , "Ding, Xuan" Subject: Re: [v4 1/3] ethdev: introduce protocol type based header split Date: Thu, 21 Apr 2022 12:27:58 +0200 Message-ID: <3350018.QJadu78ljV@thomas> In-Reply-To: References: <20220303060136.36427-1-xuan.ding@intel.com> <253f6a27-0daa-d708-79e6-607228dda044@oktetlabs.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 12/04/2022 18:15, Ding, Xuan: > From: Andrew Rybchenko > > On 4/2/22 13:41, wenxuanx.wu@intel.com wrote: > > > From: Xuan Ding > > > > > > Header split consists of splitting a received packet into two separate > > > regions based on the packet content. The split happens after the > > > packet header and before the packet payload. Splitting is usually > > > between the packet header that can be posted to a dedicated buffer and > > > the packet payload that can be posted to a different buffer. > > > > > > Currently, Rx buffer split supports length and offset based packet split. > > > Although header split is a subset of buffer split, configuring buffer > > > split based on length is not suitable for NICs that do split based on > > > header protocol types. Because tunneling makes the conversion from > > > length to protocol type impossible. > > > > > > This patch extends the current buffer split to support protocol type > > > and offset based header split. A new proto field is introduced in the > > > rte_eth_rxseg_split structure reserved field to specify header > > > protocol type. With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT > > > enabled and protocol type configured, PMD will split the ingress > > > packets into two separate regions. Currently, both inner and outer > > > L2/L3/L4 level header split can be supported. > > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time > > ago to substitute bit-field header_split in struct rte_eth_rxmode. It allows to > > enable header split offload with the header size controlled using > > split_hdr_size in the same structure. > > > > Right now I see no single PMD which actually supports > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. > > Many examples and test apps initialize the field to 0 explicitly. The most of > > drivers simply ignore split_hdr_size since the offload is not advertised, but > > some double-check that its value is 0. > > > > I think that it means that the field should be removed on the next LTS, and I'd > > say, together with the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload bit. > > > > We should not redefine the offload meaning. > > Yes, you are right. No single PMD supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT now. > Previously, I used this flag is to distinguish buffer split and header split. > The former supports multi-segments split by length and offset. > The later supports two segments split by proto and offset. > At this level, header split is a subset of buffer split. > > Since we shouldn't redefine the meaning of this offload, > I will use the RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT flag. > The existence of tunnel needs to define a proto field in buffer split, > because some PMDs do not support split based on length and offset. Before doing anything, the first patch of this series should make the current status clearer. Example, this line does not explain what it does: uint16_t split_hdr_size; /**< hdr buf size (header_split enabled).*/ And header_split has been removed in ab3ce1e0c193 ("ethdev: remove old offload API") If RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is not needed, let's add a comment to start a deprecation. > > > For example, let's suppose we configured the Rx queue with the > > > following segments: > > > seg0 - pool0, off0=2B > > > seg1 - pool1, off1=128B > > > > Corresponding feature is named Rx buffer split. > > Does it mean that protocol type based header split requires Rx buffer split > > feature to be supported? > > Protocol type based header split does not requires Rx buffer split. > In previous design, the header split and buffer split are exclusive. > Because we only configure one split offload for one RX queue. Things must be made clear and documented. > > > With header split type configured with RTE_ETH_RX_HEADER_SPLIT_UDP, > > > the packet consists of MAC_IP_UDP_PAYLOAD will be split like following: > > > seg0 - udp header @ RTE_PKTMBUF_HEADROOM + 2 in mbuf from > > pool0 > > > seg1 - payload @ 128 in mbuf from pool1 > > > > Is it always outermost UDP? Does it require both UDP over IPv4 and UDP over > > IPv6 to be supported? What will happen if only one is supported? How > > application can find out which protocol stack are supported? > > Both inner and outer UDP are considered. > Current design does not distinguish UDP over IPv4 or IPv6. > If we want to support granularity like only IPv4 or IPv6 supported, > user need add more configurations. > > If application want to find out which protocol stack is supported, > one way I think is to expose the protocol stack supported by the driver through dev_info. > Any thoughts are welcomed :) [...] > > > + uint16_t reserved; /**< Reserved field. */ > > > > As far as I can see the structure is experimental. So, it should not be the > > problem to extend it, but it is a really good question raised by Stephen in RFC > > v1 discussion. > > Shouldn't we require that all reserved fields are initialized to zero and > > ignored on processing? Frankly speaking I always thought so, but failed to > > find the place were it is documented. > > Yes, it can be documented. By default is should be zero, and we can configure > it to enable protocol type based buffer split. > > > @Thomas, @David, @Ferruh? Yes that's very important to have a clear state of the reserved fields. A value must be set and documented.