> -----Original Message----- > From: Kulasek, TomaszX > Sent: Monday, October 24, 2016 1:49 PM > To: Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org > Cc: olivier.matz at 6wind.com > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation > > Hi Konstantin, > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Monday, October 24, 2016 14:15 > > To: Kulasek, TomaszX <tomaszx.kulasek at intel.com>; dev at dpdk.org > > Cc: olivier.matz at 6wind.com > > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation > > > > Hi Tomasz, > > > > [...] > > > > > > > +/** > > > + * Fix pseudo header checksum > > > + * > > > + * This function fixes pseudo header checksum for TSO and non-TSO > > > +tcp/udp in > > > + * provided mbufs packet data. > > > + * > > > + * - for non-TSO tcp/udp packets full pseudo-header checksum is counted > > and set > > > + * in packet data, > > > + * - for TSO the IP payload length is not included in pseudo header. > > > + * > > > + * This function expects that used headers are in the first data > > > +segment of > > > + * mbuf, and are not fragmented. > > > + * > > > + * @param m > > > + * The packet mbuf to be validated. > > > + * @return > > > + * 0 if checksum is initialized properly > > > + */ > > > +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 = m->ol_flags; > > > + uint64_t inner_l3_offset = m->l2_len; > > > + > > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) > > > + inner_l3_offset += m->outer_l2_len + m->outer_l3_len; > > > + > > > + /* headers are fragmented */ > > > + if (unlikely(rte_pktmbuf_data_len(m) >= inner_l3_offset + m->l3_len > > + > > > + m->l4_len)) > > > > Might be better to move that check into rte_validate_tx_offload(), so it > > would be called only when TX_DEBUG is on. > > While unfragmented headers are not general requirements for Tx offloads, and > this requirement is for this particular implementation, > maybe for performance reasons will be better to keep it here, and just add > #if DEBUG to leave rte_validate_tx_offload more generic.
Hmm and what is the advantage to pollute that code with more ifdefs? Again, why unfragmented headers are not general requirements? As long as DPDK pseudo-headear csum calculation routines can't handle fragmented case, it pretty much is a general requirement, no? Konstantin > > > Another thing, shouldn't it be: > > if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len) ? > > Yes, it should. > > > Konstantin > > > > Tomasz