From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 7C8591B137 for ; Wed, 3 Oct 2018 22:08:17 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 1EC6321B7F; Wed, 3 Oct 2018 16:08:17 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 03 Oct 2018 16:08:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=vwMqqm4tXRES8JFjhmUFvwDSX4D58R3KWDhAUdcgk/I=; b=Aa8mr7Im+UU/ ukInvHJxvKG6SVHVMp0IJOpo5Vsi8vI95g1oTsRL6w+AOIwqYhYQVtoEiTayOhWn 7uKvaop6b79NRVLpNLYaJh6+q+infzln88hl+PL5Gxky/5HTn7bvZWHRZUgLqTXv jj8gBvdnpYx+AW+vC5Cwv8aUZ9yplUY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=vwMqqm4tXRES8JFjhmUFvwDSX4D58R3KWDhAUdcgk /I=; b=ItEklGkU/wOVpVAZlkVjo71VYxGegkSXIMTfGpmNZ1QWwg9iXLIeKsKNj MuoLsLC2nWP5FKTd3glyWsgvkh54dSrX6DjOCqoyYBcHe/wG+yRAKiAL5wZ4sMil 685vdejRhsgwOUo8FbQLY0nRcvqZR3gRVjSUkBk9Md7rX3D3SPQ95dVRZQi8gi4/ TLydvZ8K4nFdGKeHfujNAbQ9NVJfApi7Yy9lW5h6NOdebwq3eM0z7ZdnzQksufAh 70RVm4Z5azx/3Lt8/ZMToNaJPsgmXOha9ajg+LVkHDLpfitJ8uUIpjAjGe81HImE h7ezSYoXxA74UIMAWmDnbADtKA18g== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 20D61E4A43; Wed, 3 Oct 2018 16:08:14 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko , Jerin Jacob Cc: Wenzhuo Lu , Jingjing Wu , Bernard Iremonger , John McNamara , Marko Kovacevic , Ferruh Yigit , Olivier Matz , dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin" Date: Wed, 03 Oct 2018 22:08:13 +0200 Message-ID: <1780274.oBxpks6VXQ@xps> In-Reply-To: <8195dbcb-4b05-b47b-ebdf-2a8c36fa2974@solarflare.com> References: <20180913134707.23698-1-jerin.jacob@caviumnetworks.com> <20181003181412.GA8662@jerin> <8195dbcb-4b05-b47b-ebdf-2a8c36fa2974@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition 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: , X-List-Received-Date: Wed, 03 Oct 2018 20:08:17 -0000 03/10/2018 21:47, Andrew Rybchenko: > On 03.10.2018 21:14, Jerin Jacob wrote: > > From: Andrew Rybchenko > >>> From: Jerin Jacob > >>>> From: Andrew Rybchenko > >>>>> On 10/2/18 10:24 PM, Jerin Jacob wrote: > >>>>> > >>>>> Introduced DEV_RX_OFFLOAD_OUTER_UDP_CKSUM Rx offload flag and > >>>>> PKT_RX_EL4_CKSUM_BAD mbuf ol_flags to detect outer UDP checksum > >>>>> failure. > >>>>> > >>>>> - To use hardware Rx outer UDP checksum offload, the user needs to > >>>>> configure DEV_RX_OFFLOAD_OUTER_UDP_CKSUM offload flags in slowpath. > >>>>> > >>>>> - Driver updates the PKT_RX_EL4_CKSUM_BAD mbuf ol_flag on checksum failure > >>>>> similar to the outer L3 PKT_RX_EIP_CKSUM_BAD flag. > >>>>> > >>>>> Signed-off-by: Jerin Jacob > >>>>> > >>>>> 1. I'm not sure that it is OK that mbuf and ethdev changes go in one patch. > >>>>> It seems typically mbuf changes go separately and mbuf changes should > >>>>> be applied to main dpdk repo. > >>>> I don't have strong opinion on this. If there are no other objection, I > >>>> will split the patch further as mbuf and ethdev as you pointed out. > >>>> > >>>>> 2. I'd like to see thought why single bit is used for outer L2 checksum when > >>>>> 2 bits (UNKNOWN, BAD, GOOD, NONE) are used for PKT_RX_L4_CKSUM. > >>>>> May be it is OK, but it would be useful to state explicitly why it is decided > >>>>> to go this way. > >>>> I am following the scheme similar to OUTER IP checksum where we have only > >>>> one bit filed(PKT_RX_EIP_CKSUM_BAD). I will mention in the git commit. > >>>> > >>>> > >>>>> 3. PKT_RX_L4_CKSUM_MASK description says nothing if it is inner or outer. > >>>>> May be it is not directly related to changeset, but I think it would be really > >>>>> useful to clarify it. > >>>> I will update the comment. > >>> Hi Andrew, > >>> > >>> I looked at the other definitions in mbuf.h, according the documentation, > >>> If nothing is mentioned it is treated as inner if the packet is > >>> tunneled else it is outer most. So I would like avoid confusion by > >>> adding "inner" in the exiting PKT_RX_L4_CKSUM_MASK comment. > >>> Technically it is not correct to say "inner" if the packet is not > >>> tunneled. So I am untouching the exiting comment. > >>> > >> Yes, it is incorrect to say that it is inner. How does application find > >> how to treat PKT_RX_L4_CKSUM (inner or outer)? > >> Should it rely on packet type provided in mbuf? > > AFAIK, Finding is it a tunneled packet or not is through ptype or SW has > > to parse the packet. For example, testpmd chooses later method using > > "csum parse-tunnel on " to detect the presence of the tunnel. > > SW parsing of the packet cannot help, since app should be sure > that HW has classified the packet as tunneled and provided information > about inner and outer checksum checks. > > >> Is it specified/mentioned somewhere? > > I don't know. It it not directly related to this change set, Olivier may know > > additional details. > > I disagree. You're adding one more offload flag. Yes, it simply follows > existing RX_EIP_CKSUM_BAD pattern. But, IMHO, RX_EIP_CKSUM_BAD > has many open questions. Why should these open questions be preserved > here? It is similar to the code with a bug which is cloned once again with > the bug :) > > If everyone else is fine with the description of Rx checksum offloads and > it is only me who is unhappy with it - no problem. I agree we must better define these checksum flags. Olivier, please could you give your opinion here?