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 4ED276CD1 for ; Thu, 20 Oct 2016 00:07:29 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 19 Oct 2016 15:07:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,516,1473145200"; d="scan'208";a="891811386" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by orsmga003.jf.intel.com with ESMTP; 19 Oct 2016 15:07:27 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX151.ger.corp.intel.com (163.33.192.59) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 19 Oct 2016 23:07:26 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by irsmsx155.ger.corp.intel.com ([169.254.14.176]) with mapi id 14.03.0248.002; Wed, 19 Oct 2016 23:07:25 +0100 From: "Ananyev, Konstantin" To: "Kulasek, TomaszX" , Olivier Matz , "dev@dpdk.org" CC: "thomas.monjalon@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v6 1/6] ethdev: add Tx preparation Thread-Index: AQHSJizlH9H8Y3eh90CgO2OcydhRYqCuQq+AgAGfFgCAAHlmsA== Date: Wed, 19 Oct 2016 22:07:24 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0C3652@irsmsx105.ger.corp.intel.com> References: <1476380222-9900-1-git-send-email-tomaszx.kulasek@intel.com> <1476457519-6840-1-git-send-email-tomaszx.kulasek@intel.com> <1476457519-6840-2-git-send-email-tomaszx.kulasek@intel.com> <0e011fea-ddd4-283b-61db-8a7a67b652e2@6wind.com> <3042915272161B4EB253DA4D77EB373A14F37118@IRSMSX102.ger.corp.intel.com> In-Reply-To: <3042915272161B4EB253DA4D77EB373A14F37118@IRSMSX102.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 1/6] ethdev: add Tx preparation 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: Wed, 19 Oct 2016 22:07:29 -0000 Hi guys, > > > > > +static inline int > > > +rte_phdr_cksum_fix(struct rte_mbuf *m) { > > > + struct ipv4_hdr *ipv4_hdr; > > > + struct ipv6_hdr *ipv6_hdr; > > > + struct tcp_hdr *tcp_hdr; > > > + struct udp_hdr *udp_hdr; > > > + uint64_t ol_flags =3D m->ol_flags; > > > + uint64_t inner_l3_offset =3D m->l2_len; > > > + > > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) > > > + inner_l3_offset +=3D m->outer_l2_len + m->outer_l3_len; > > > + > > > + if ((ol_flags & PKT_TX_UDP_CKSUM) =3D=3D PKT_TX_UDP_CKSUM) { > > > + if (ol_flags & PKT_TX_IPV4) { > > > + ipv4_hdr =3D rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, > > > + inner_l3_offset); > > > + > > > + if (ol_flags & PKT_TX_IP_CKSUM) > > > + ipv4_hdr->hdr_checksum =3D 0; > > > + > > > + udp_hdr =3D (struct udp_hdr *)((char *)ipv4_hdr + m- > > >l3_len); > > > + udp_hdr->dgram_cksum =3D rte_ipv4_phdr_cksum(ipv4_hdr, > > ol_flags); > > > + } else { > > > + ipv6_hdr =3D rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *, > > > + inner_l3_offset); > > > + /* non-TSO udp */ > > > + udp_hdr =3D rte_pktmbuf_mtod_offset(m, struct udp_hdr *, > > > + inner_l3_offset + m->l3_len); > > > + udp_hdr->dgram_cksum =3D rte_ipv6_phdr_cksum(ipv6_hdr, > > ol_flags); > > > + } > > > + } else if ((ol_flags & PKT_TX_TCP_CKSUM) || > > > + (ol_flags & PKT_TX_TCP_SEG)) { > > > + if (ol_flags & PKT_TX_IPV4) { > > > + ipv4_hdr =3D rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, > > > + inner_l3_offset); > > > + > > > + if (ol_flags & PKT_TX_IP_CKSUM) > > > + ipv4_hdr->hdr_checksum =3D 0; > > > + > > > + /* non-TSO tcp or TSO */ > > > + tcp_hdr =3D (struct tcp_hdr *)((char *)ipv4_hdr + m- > > >l3_len); > > > + tcp_hdr->cksum =3D rte_ipv4_phdr_cksum(ipv4_hdr, ol_flags); > > > + } else { > > > + ipv6_hdr =3D rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *, > > > + inner_l3_offset); > > > + /* non-TSO tcp or TSO */ > > > + tcp_hdr =3D rte_pktmbuf_mtod_offset(m, struct tcp_hdr *, > > > + inner_l3_offset + m->l3_len); > > > + tcp_hdr->cksum =3D rte_ipv6_phdr_cksum(ipv6_hdr, ol_flags); > > > + } > > > + } > > > + return 0; > > > +} > > > + > > > +#endif /* _RTE_PKT_H_ */ > > > > > > > The function expects that all the network headers are in the first, and > > that each of them is contiguous. > > >=20 > Yes, I see... Yes it does. I suppose it is a legitimate restriction (assumptions) for those who'd lik= e to use that function. But that's a good point and I suppose we need to state it explicitly in the= API comments. >=20 > > Also, I had an interesting remark from Stephen [1] on a similar code. > > If the mbuf is a clone, it will modify the data of the direct mbuf, whi= ch > > should be read-only. Note that it is likely to happen in a TCP stack, > > because the packet is kept locally in case it has to be retransmitted. > > Cloning a mbuf is more efficient than duplicating it. > > > > I plan to fix it in virtio code by "uncloning" the headers. > > > > [1] http://dpdk.org/ml/archives/dev/2016-October/048873.html This subject is probably a bit off-topic...=20 My position on it - it shouldn't be a PMD responsibility to make these kind= of checks. I think it should be upper layer responsibility to provide to the PMD an mb= uf that can be safely used (and in that case modified) by the driver. Let say if upper layer would like to use packet clone for TCP retransmissio= ns, It can easily overcome that problem by cloning only data part of the packet= , and putting L2/L3/L4 headers in a separate (head) mbuf and chaining it with cloned data mbuf. Konstantin=20