| Message ID | 1471438966-20440-2-git-send-email-i.maximets@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Daniele Di Proietto |
| Headers | show |
> 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.maximets@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? > + 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
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.maximets@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
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; + 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]); } }
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. Fixes: 0072e931b207 ("netdev-dpdk: add support for jumbo frames") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- lib/netdev-dpdk.c | 52 +++++++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 23 deletions(-)