From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [RFC 00/16] enhance checksum offload API Date: Wed, 21 Jan 2015 17:01:49 -0800 Message-ID: <20150121170149.157da6ed@urahara> References: <1421883395-27235-1-git-send-email-olivier.matz@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Olivier Matz Return-path: In-Reply-To: <1421883395-27235-1-git-send-email-olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" On Thu, 22 Jan 2015 00:36:19 +0100 Olivier Matz wrote: > The goal of this series is to clarify and simplify the mbuf offload API. > Several issues are solved: > > - simplify the definitions of PKT_TX_IP_CKSUM and PKT_TX_IPV4, each > flag has now only one meaning. No impact on the code. > > - add a feature flag for OUTER_IP_CHECKSUM (from Jijiang's patches) > > - remove the PKT_TX_UDP_TUNNEL_PKT flag: it is useless from an API point > of view. It was added because i40e need this info for some reason. We > have 3 solutions: > > - remove the flag and adapt the driver to the API (the choice I made > for this series). > > - remove the flag and stop advertising OUTER_IP_CHECKSUM in i40e > > - keep this flag, penalizing performance of drivers that do not > require the flag. It would also mean that drivers won't support > outer IP checksum for all tunnel types, but only for the tunnel > types having a flag. > > - a side effect of this API clarification is that there is only one > way for doing one operation. If the hardware has several ways to > do the same operation, a choice has to be made in the driver. > > The patch adds new tunnel types to testpmd csum forward engine. > Note: the i40e patches should be carefully checked by i40e experts. > > [1] http://dpdk.org/ml/archives/dev/2015-January/011127.html If you are doing this could you invert the meaning of the checksum flags? Right now the flags are fine for Intel hardware but are useless for devices that have less checksum support. It would work better if instead of two states: * Checksum known bad => PKT_RX_L4_CKSUM_BAD == 1 * Checksum (maybe) good => PKT_RX_L4_CKSUM_BAD == 0 The bit was changed to only flag good checksum: * Checksum known good => PKT_RX_L4_CKSUM_GOOD == 1 * Checksum status unknown => PKT_RX_L4_CKSUM_GOOD == 0 That way code code fallback to software checksum if hardware was incapable of handling the packet. It does mean packets with bad checksums get checked twice, but thats ok.