On 18.08.2016 15:25, Stokes, Ian wrote: >> This patch introduces function 'netdev_dpdk_filter_packet_len()' which is >> intended to find and remove all packets with 'pkt_len > max_packet_len' >> from the Tx batch. >> >> It fixes inaccurate counting of 'tx_bytes' in vHost case if there was >> dropped packets and allows to simplify send function. >> > > Thanks for the patch Ilya, minor comment below. > >> Fixes: 0072e931b207 ("netdev-dpdk: add support for jumbo frames") >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >> --- >> lib/netdev-dpdk.c | 52 +++++++++++++++++++++++++++++--------------------- >> -- >> 1 file changed, 29 insertions(+), 23 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index e5f2cdd..bd374d0 >> 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -1435,6 +1435,28 @@ netdev_dpdk_qos_run__(struct netdev_dpdk *dev, >> struct rte_mbuf **pkts, >> return cnt; >> } >> >> +static int >> +netdev_dpdk_filter_packet_len(struct netdev_dpdk *dev, struct rte_mbuf >> **pkts, >> + int pkt_cnt) { >> + int i = 0; >> + int cnt = 0; >> + struct rte_mbuf *pkt; >> + >> + for (i = 0; i < pkt_cnt; i++) { >> + pkt = pkts[i]; >> + if (OVS_UNLIKELY(pkt->pkt_len > dev->max_packet_len)) { >> + VLOG_WARN_RL(&rl, "%s: Too big size %" PRIu32 " >> max_packet_len %d", >> + dev->up.name, pkt->pkt_len, dev- >>> max_packet_len); >> + rte_pktmbuf_free(pkt); >> + } else if (i != cnt++) { >> + pkts[cnt - 1] = pkt; >> + } >> + } >> + >> + return cnt; >> +} >> + >> static inline void >> netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats, >> struct dp_packet **packets, @@ - >> 1459,8 +1481,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, >> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); >> struct rte_mbuf **cur_pkts = (struct rte_mbuf **) pkts; >> unsigned int total_pkts = cnt; >> - unsigned int qos_pkts = 0; >> - unsigned int mtu_dropped = 0; >> + unsigned int dropped = 0; >> int i, retries = 0; >> >> qid = dev->tx_q[qid % netdev->n_txq].map; @@ -1477,50 +1498,35 @@ >> __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, >> >> /* Check has QoS has been configured for the netdev */ >> cnt = netdev_dpdk_qos_run__(dev, cur_pkts, cnt); >> - qos_pkts = total_pkts - cnt; > > I think it would be better to call netdev_dpdk_filter_packet_len() before > netdev_dpdk_qos_run__ above. > If the packet is too large for the netdev it will be dropped inevitably, we > should avoid the overhead of QoS before dropping it, thoughts?
Sounds reasonable. I'll send v2. >> + cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt); >> + dropped = total_pkts - cnt; >> >> do { >> int vhost_qid = qid * VIRTIO_QNUM + VIRTIO_RXQ; >> unsigned int tx_pkts; >> - unsigned int try_tx_pkts = cnt; >> >> - for (i = 0; i < cnt; i++) { >> - if (cur_pkts[i]->pkt_len > dev->max_packet_len) { >> - try_tx_pkts = i; >> - break; >> - } >> - } >> - if (!try_tx_pkts) { >> - cur_pkts++; >> - mtu_dropped++; >> - cnt--; >> - continue; >> - } >> tx_pkts = rte_vhost_enqueue_burst(netdev_dpdk_get_vid(dev), >> - vhost_qid, cur_pkts, >> try_tx_pkts); >> + vhost_qid, cur_pkts, cnt); >> if (OVS_LIKELY(tx_pkts)) { >> /* Packets have been sent.*/ >> cnt -= tx_pkts; >> /* Prepare for possible retry.*/ >> cur_pkts = &cur_pkts[tx_pkts]; >> - if (tx_pkts != try_tx_pkts) { >> - retries++; >> - } >> } else { >> /* No packets sent - do not retry.*/ >> break; >> } >> - } while (cnt && (retries <= VHOST_ENQ_RETRY_NUM)); >> + } while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); >> >> rte_spinlock_unlock(&dev->tx_q[qid].tx_lock); >> >> rte_spinlock_lock(&dev->stats_lock); >> netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts, >> - cnt + mtu_dropped + qos_pkts); >> + cnt + dropped); >> rte_spinlock_unlock(&dev->stats_lock); >> >> out: >> - for (i = 0; i < total_pkts - qos_pkts; i++) { >> + for (i = 0; i < total_pkts - dropped; i++) { >> dp_packet_delete(pkts[i]); >> } >> } >> -- >> 2.7.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev