From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v2 1/4] ethdev: add Rx offload outer UDP checksum definition Date: Wed, 3 Oct 2018 23:44:13 +0530 Message-ID: <20181003181412.GA8662@jerin> References: <20180913134707.23698-1-jerin.jacob@caviumnetworks.com> <20181002192451.19119-1-jerin.jacob@caviumnetworks.com> <20181003075712.GA2003@jerin> <20181003171252.GA3193@jerin> <209397d1-f1ee-befb-1593-5adb58045bc5@solarflare.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Wenzhuo Lu , Jingjing Wu , Bernard Iremonger , John McNamara , Marko Kovacevic , Thomas Monjalon , Ferruh Yigit , Olivier Matz , dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin" To: Andrew Rybchenko Return-path: Received: from NAM04-BN3-obe.outbound.protection.outlook.com (mail-eopbgr680050.outbound.protection.outlook.com [40.107.68.50]) by dpdk.org (Postfix) with ESMTP id 2F8244F9B for ; Wed, 3 Oct 2018 20:14:36 +0200 (CEST) Content-Disposition: inline In-Reply-To: <209397d1-f1ee-befb-1593-5adb58045bc5@solarflare.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Wed, 3 Oct 2018 21:00:37 +0300 > From: Andrew Rybchenko > To: Jerin Jacob > CC: Wenzhuo Lu , Jingjing Wu , > Bernard Iremonger , John McNamara > , Marko Kovacevic , > Thomas Monjalon , Ferruh Yigit > , Olivier Matz , > dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin" > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP > checksum definition > User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 > Thunderbird/52.9.1 > > On 03.10.2018 20:12, Jerin Jacob wrote: > > -----Original Message----- > > > Date: Wed, 3 Oct 2018 13:27:13 +0530 > > > From: Jerin Jacob > > > To: Andrew Rybchenko > > > CC: Wenzhuo Lu , Jingjing Wu , > > > Bernard Iremonger , John McNamara > > > , Marko Kovacevic , > > > Thomas Monjalon , Ferruh Yigit > > > , Olivier Matz , > > > dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin" > > > > > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP > > > checksum definition > > > User-Agent: Mutt/1.10.1 (2018-07-13) > > > > > > External Email > > > > > > -----Original Message----- > > > > Date: Wed, 3 Oct 2018 10:34:52 +0300 > > > > From: Andrew Rybchenko > > > > To: Jerin Jacob , Wenzhuo Lu > > > > , Jingjing Wu , Bernard > > > > Iremonger , John McNamara > > > > , Marko Kovacevic , > > > > Thomas Monjalon , Ferruh Yigit > > > > , Olivier Matz > > > > CC: dev@dpdk.org, shahafs@mellanox.com, "Ananyev, Konstantin" > > > > > > > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: add Rx offload outer UDP > > > > checksum definition > > > > User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 > > > > Thunderbird/60.0 > > > > > > > > > > > > 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. > Is it specified/mentioned somewhere? I don't know. It it not directly related to this change set, Olivier may know additional details. > > Andrew.