On 1/10/2019 4:58 PM, Tiago Lam wrote:
Given that multi-segment mbufs might be sent between interfaces that
support different capabilities, and may even support different layouts
of mbufs, outgoing packets should be validated before sent on the egress
interface. Thus, netdev_dpdk_eth_tx_burst() now calls DPDK's
rte_eth_tx_prepare() function, if and only multi-segments is enbaled, in
order to validate the following (taken from the DPDK documentation), on
a device specific manner:
- Check if packet meets devices requirements for tx offloads.
- Check limitations about number of segments.
- Check additional requirements when debug is enabled.
- Update and/or reset required checksums when tx offload is set for
packet.
Thanks Tiago comments below.
Signed-off-by: Tiago Lam <tiago....@intel.com>
---
lib/netdev-dpdk.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d6114ee..77d04fc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2029,6 +2029,10 @@ netdev_dpdk_rxq_dealloc(struct netdev_rxq *rxq)
/* Tries to transmit 'pkts' to txq 'qid' of device 'dev'. Takes ownership of
* 'pkts', even in case of failure.
+ * In case multi-segment mbufs / TSO is being used, it also prepares. In such
+ * cases, only the prepared packets will be sent to Tx burst, meaning that if
+ * an invalid packet appears in 'pkts'[3] only the validated packets in indices
+ * 0, 1 and 2 will be sent.
*
* Returns the number of packets that weren't transmitted. */
static inline int
@@ -2036,11 +2040,24 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int
qid,
struct rte_mbuf **pkts, int cnt)
{
uint32_t nb_tx = 0;
+ uint16_t nb_prep = cnt;
- while (nb_tx != cnt) {
+ if (dpdk_multi_segment_mbufs) {
As this feature would be experimental and not enabled, by default this
would be false.
Would it make sense to deal with the default non multiseg case first?
Extra checks in the datapath can be costly and I'd like to minimize any
impact for the non multi seg traffic which is the default use case
currently. Possibly checking for !dpdk_multi_segment_mbufs first? Or
possibly the use of OVS_LIKELY/OVS_UNLIKELY. Have you given thought to this?
+ /* Validate the burst of packets for Tx. */
+ nb_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
So one of the gotchas here is that rte_eth_tx_prepare is experimental
although I didn't see any compilation issues (warnings etc.) and it
builds OK with travis.
Possibly a comment above it to explain it's currently experimental and
subject to change in DPDK would be useful, can be removed once the api
is non experimental in DPDK. It would be one of the conditions to remove
the experimental tag for TSO in OVS DPDK I suspect in the future.
Ian
+ if (nb_prep != cnt) {
+ VLOG_WARN_RL(&rl, "%s: Preparing packet tx burst failed (%u/%u "
+ "packets valid): %s", dev->up.name, nb_prep, cnt,
+ rte_strerror(rte_errno));
+ }
+ }
+
+ /* Tx the validated burst of packets only. */
+ while (nb_tx != nb_prep) {
uint32_t ret;
- ret = rte_eth_tx_burst(dev->port_id, qid, pkts + nb_tx, cnt - nb_tx);
+ ret = rte_eth_tx_burst(dev->port_id, qid, pkts + nb_tx,
+ nb_prep - nb_tx);
if (!ret) {
break;
}
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
