Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, September 29, 2016 13:09 > To: Kulasek, TomaszX <tomaszx.kulasek at intel.com>; dev at dpdk.org > Subject: RE: [PATCH v3 5/6] ixgbe: add Tx preparation > > Hi Tomasz, > > > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek at intel.com> > > ---
... > > +*/ > > +uint16_t > > +ixgbe_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t > > +nb_pkts) { > > + int i, ret; > > + struct rte_mbuf *m; > > + struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue; > > + > > + for (i = 0; i < nb_pkts; i++) { > > + m = tx_pkts[i]; > > + > > + /** > > + * Check if packet meets requirements for number of > segments > > + * > > + * NOTE: for ixgbe it's always (40 - WTHRESH) for both TSO > and non-TSO > > + */ > > + > > + if (m->nb_segs > IXGBE_TX_MAX_SEG - txq->wthresh) { > > + rte_errno = -EINVAL; > > + return i; > > + } > > + > > + if (m->ol_flags & IXGBE_TX_OFFLOAD_NOTSUP_MASK) { > > + rte_errno = -EINVAL; > > + return i; > > + } > > + > > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG > > + ret = rte_validate_tx_offload(m); > > + if (ret != 0) { > > + rte_errno = ret; > > + return i; > > + } > > +#endif > > + ret = rte_phdr_cksum_fix(m); > > + if (ret != 0) { > > + rte_errno = ret; > > + return i; > > + } > > + } > > + > > + return i; > > +} > > + > > +/* ixgbe simple path as well as vector TX doesn't support tx offloads > > +*/ uint16_t ixgbe_prep_pkts_simple(void *tx_queue __rte_unused, > > +struct rte_mbuf **tx_pkts, > > + uint16_t nb_pkts) > > +{ > > + int i; > > + struct rte_mbuf *m; > > + uint64_t ol_flags; > > + > > + for (i = 0; i < nb_pkts; i++) { > > + m = tx_pkts[i]; > > + ol_flags = m->ol_flags; > > + > > + /* simple tx path doesn't support multi-segments */ > > + if (m->nb_segs != 1) { > > + rte_errno = -EINVAL; > > + return i; > > + } > > + > > + /* For simple path (simple and vector) no tx offloads are > supported */ > > + if (ol_flags & PKT_TX_OFFLOAD_MASK) { > > + rte_errno = -EINVAL; > > + return i; > > + } > > + } > > + > > + return i; > > +} > > Just thought about it once again: > As now inside rte_eth_tx_prep() we do now: > + > + if (!dev->tx_pkt_prep) > + return nb_pkts; > > Then there might be a better approach to set > dev->tx_pkt_prep = NULL > for simple and vector TX functions? > > After all, prep_simple() does nothing but returns an error if conditions are > not met. > And if simple TX was already selected, then that means that user deliberately > disabled all HW TX offloads in favor of faster TX and there is no point to > slow > him down with extra checks here. > Same for i40e and fm10k. > What is your opinion? > > Konstantin > Yes, if performance is a key, and, while the limitations of vector/simple path are quite well documented, these additional checks are a bit overzealous. We may assume that to made tx offloads working, we need to configure driver in a right way, and this is a configuration issue if something doesn't work. I will remove it. Tomasz