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
