From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id B6DE46CD5 for ; Thu, 13 Oct 2016 16:16:43 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP; 13 Oct 2016 07:16:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,340,1473145200"; d="scan'208";a="1053340533" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 13 Oct 2016 07:15:59 -0700 Date: Thu, 13 Oct 2016 22:16:54 +0800 From: Yuanhan Liu To: Olivier MATZ Cc: dev@dpdk.org, konstantin.ananyev@intel.com, sugesh.chandran@intel.com, bruce.richardson@intel.com, jianfeng.tan@intel.com, helin.zhang@intel.com, adrien.mazarguil@6wind.com, stephen@networkplumber.org, dprovan@bivio.net, xiao.w.wang@intel.com Message-ID: <20161013141654.GW16751@yliu-dev.sh.intel.com> References: <1475485223-30566-1-git-send-email-olivier.matz@6wind.com> <1475485223-30566-13-git-send-email-olivier.matz@6wind.com> <20161013081839.GT16751@yliu-dev.sh.intel.com> <57FF9409.2090205@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57FF9409.2090205@6wind.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 12/12] virtio: add Tso support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Oct 2016 14:16:44 -0000 On Thu, Oct 13, 2016 at 04:02:49PM +0200, Olivier MATZ wrote: > > > On 10/13/2016 10:18 AM, Yuanhan Liu wrote: > >On Mon, Oct 03, 2016 at 11:00:23AM +0200, Olivier Matz wrote: > >>+/* When doing TSO, the IP length is not included in the pseudo header > >>+ * checksum of the packet given to the PMD, but for virtio it is > >>+ * expected. > >>+ */ > >>+static void > >>+virtio_tso_fix_cksum(struct rte_mbuf *m) > >>+{ > >>+ /* common case: header is not fragmented */ > >>+ if (likely(rte_pktmbuf_data_len(m) >= m->l2_len + m->l3_len + > >>+ m->l4_len)) { > >... > >>+ /* replace it in the packet */ > >>+ th->cksum = new_cksum; > >>+ } else { > >... > >>+ /* replace it in the packet */ > >>+ *rte_pktmbuf_mtod_offset(m, uint8_t *, > >>+ m->l2_len + m->l3_len + 16) = new_cksum.u8[0]; > >>+ *rte_pktmbuf_mtod_offset(m, uint8_t *, > >>+ m->l2_len + m->l3_len + 17) = new_cksum.u8[1]; > >>+ } > > > >The tcp header will always be in the mbuf, right? Otherwise, you can't > >update the cksum field here. What's the point of introducing the "else > >clause" then? > > Sorry, I don't see the problem you're pointing out here. > > What I want to solve here is to support the cases where the mbuf is > segmented in the middle of the network header (which is probably a rare > case). How it's gonna segmented? > > In the "else" part, I only access the mbuf byte by byte using the > rte_pktmbuf_mtod_offset() accessor. An alternative would have been to copy > the header in a linear buffer, fix the checksum, then copy it again in the > packet, but there is no mbuf helpers to do these copies for now. In the "else" clause, the ip header is still in the mbuf, right? Why do you have to access it the way like: ip_version = *rte_pktmbuf_mtod_offset(m, uint8_t *, m->l2_len) >> 4; Why can't you just use iph = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, m->l2_len); iph->version_ihl ....; Sorry, I'm just a bit lost. --yliu