> On Feb 13, 2019, at 3:50 AM, Andrew Rybchenko <arybche...@solarflare.com> > wrote: > > Ping. > > Do 2 weeks without reply mean that it looks good and I should send non-RCF > version?
Just send the non-RFC patch as it seems most do not even look at the RFC’s anyway. > > Andrew. > > On 1/29/19 11:49 AM, Andrew Rybchenko wrote: >> rte_validate_tx_offload() is used in Tx prepare callbacks >> (RTE_LIBRTE_ETHDEV_DEBUG only) to check Tx offloads consistency. >> Requirement that packet headers should not be fragmented is not >> documented and unclear where it comes from except >> rte_net_intel_cksum_prepare() functions which relies on it. >> >> It could be NIC vendor specific driver or hardware limitation, but, >> if so, it should be documented and checked in corresponding Tx >> prepare callbacks. >> >> Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com> >> --- >> May be the check should be done in rte_net_intel_cksum_prepare() >> under RTE_LIBRTE_ETHDEV_DEBUG only. Mainly I'd like to get feedback >> on where the limitation comes from and idea to remove it from >> rte_validate_tx_offload(). >> >> lib/librte_mbuf/rte_mbuf.h | 12 ------------ >> lib/librte_net/rte_net.h | 17 +++++++++++++++++ >> 2 files changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h >> index a7f67023a..14a3b472b 100644 >> --- a/lib/librte_mbuf/rte_mbuf.h >> +++ b/lib/librte_mbuf/rte_mbuf.h >> @@ -2257,23 +2257,11 @@ static inline int >> rte_validate_tx_offload(const struct rte_mbuf *m) >> { >> uint64_t ol_flags = m->ol_flags; >> - uint64_t inner_l3_offset = m->l2_len; >> /* Does packet set any of available offloads? */ >> if (!(ol_flags & PKT_TX_OFFLOAD_MASK)) >> return 0; >> - if (ol_flags & PKT_TX_OUTER_IP_CKSUM) >> - /* NB: elaborating the addition like this instead of using >> - * += gives the result uint64_t type instead of int, >> - * avoiding compiler warnings on gcc 8.1 at least */ >> - inner_l3_offset = inner_l3_offset + m->outer_l2_len + >> - m->outer_l3_len; >> - >> - /* Headers are fragmented */ >> - if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len) >> - return -ENOTSUP; >> - >> /* IP checksum can be counted only for IPv4 packet */ >> if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6)) >> return -EINVAL; >> diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h >> index e59760a0a..bd75aea8e 100644 >> --- a/lib/librte_net/rte_net.h >> +++ b/lib/librte_net/rte_net.h >> @@ -118,10 +118,27 @@ rte_net_intel_cksum_flags_prepare(struct rte_mbuf *m, >> uint64_t ol_flags) >> struct udp_hdr *udp_hdr; >> uint64_t inner_l3_offset = m->l2_len; >> + /* >> + * Does packet set any of available offloads? >> + * Mainly it is required to avoid fragmented headers check if >> + * no offloads are requested. >> + */ >> + if (!(ol_flags & PKT_TX_OFFLOAD_MASK)) >> + return 0; >> + >> if ((ol_flags & PKT_TX_OUTER_IP_CKSUM) || >> (ol_flags & PKT_TX_OUTER_IPV6)) >> inner_l3_offset += m->outer_l2_len + m->outer_l3_len; >> + /* >> + * Check if headers are fragmented. >> + * The check could be less strict depending on which offloads are >> + * requested and headers to be used, but let's keep it simple. >> + */ >> + if (unlikely(rte_pktmbuf_data_len(m) < >> + inner_l3_offset + m->l3_len + m->l4_len)) >> + return -ENOTSUP; >> + >> if (ol_flags & PKT_TX_IPV4) { >> ipv4_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *, >> inner_l3_offset); > Regards, Keith