[ovs-dev] [PATCH v2] netdev-dpdk: Execute QoS Checking before copying to mbuf
Gao Zhenyu
sysugaozhenyu at gmail.com
Wed Sep 6 06:39:35 UTC 2017
Thanks Derrell, it makes sence to me and I think the code path is more
clear.
Thanks
Zhenyu Gao
2017-09-06 0:24 GMT+08:00 Darrell Ball <dball at vmware.com>:
> Hi Gao
>
> How about the following incremental ?
>
> Thanks Darrell
>
>
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 4808d38..648d719 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1843,64 +1843,58 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
> struct dp_packet_batch *batch)
> #endif
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> struct rte_mbuf *pkts[PKT_ARRAY_SIZE];
> - int txcnt_eth = batch->count;
> - int dropped = 0;
> - int newcnt = 0;
> - int i;
> + uint32_t cnt = batch->count;
> + uint32_t dropped = 0;
>
> if (dev->type != DPDK_DEV_VHOST) {
> /* Check if QoS has been configured for this netdev. */
> - txcnt_eth = netdev_dpdk_qos_run(dev,
> - (struct rte_mbuf
> **)batch->packets,
> - txcnt_eth, false);
> - if (txcnt_eth == 0) {
> - dropped = batch->count;
> - goto out;
> - }
> + cnt = netdev_dpdk_qos_run(dev, (struct rte_mbuf **)
> batch->packets,
> + cnt, false);
> + dropped += batch->count - cnt;
> }
>
> dp_packet_batch_apply_cutlen(batch);
>
> - for (i = 0; i < batch->count; i++) {
> - int size = dp_packet_size(batch->packets[i]);
> + uint32_t txcnt = 0;
> +
> + for (uint32_t i = 0; i < cnt; i++) {
> +
> + uint32_t size = dp_packet_size(batch->packets[i]);
>
> if (OVS_UNLIKELY(size > dev->max_packet_len)) {
> - VLOG_WARN_RL(&rl, "Too big size %d max_packet_len %d",
> - (int) size, dev->max_packet_len);
> + VLOG_WARN_RL(&rl, "Too big size %u max_packet_len %d",
> + size, dev->max_packet_len);
>
> dropped++;
> continue;
> }
>
> - pkts[newcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
> + pkts[txcnt] = rte_pktmbuf_alloc(dev->dpdk_mp->mp);
>
> - if (!pkts[newcnt]) {
> - dropped += batch->count - i;
> + if (!pkts[txcnt]) {
> + dropped += cnt - i;
> break;
> }
>
> /* We have to do a copy for now */
> - memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),
> + memcpy(rte_pktmbuf_mtod(pkts[txcnt], void *),
> dp_packet_data(batch->packets[i]), size);
>
> - rte_pktmbuf_data_len(pkts[newcnt]) = size;
> - rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
> + rte_pktmbuf_data_len(pkts[txcnt]) = size;
> + rte_pktmbuf_pkt_len(pkts[txcnt]) = size;
>
> - newcnt++;
> - if (dev->type != DPDK_DEV_VHOST && newcnt >= txcnt_eth) {
> - dropped += batch->count - i - 1;
> - break;
> - }
> + txcnt++;
> }
>
> - if (dev->type == DPDK_DEV_VHOST) {
> - __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
> - newcnt);
> - } else {
> - dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, newcnt);
> + if (OVS_LIKELY(txcnt)) {
> + if (dev->type == DPDK_DEV_VHOST) {
> + __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **)
> pkts,
> + txcnt);
> + } else {
> + dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt);
> + }
> }
>
> -out:
> if (OVS_UNLIKELY(dropped)) {
> rte_spinlock_lock(&dev->stats_lock);
> dev->stats.tx_dropped += dropped;
>
>
> //////////////////////////////////////////////////////////////
>
>
> On 8/29/17, 4:39 AM, "ovs-dev-bounces at openvswitch.org on behalf of Zhenyu
> Gao" <ovs-dev-bounces at openvswitch.org on behalf of sysugaozhenyu at gmail.com>
> wrote:
>
> In dpdk_do_tx_copy function, all packets were copied to mbuf first,
> but QoS checking may drop some of them.
> Move the QoS checking in front of copying data to mbuf, it helps to
> reduce useless copy.
>
> Signed-off-by: Zhenyu Gao <sysugaozhenyu at gmail.com>
> ---
> lib/netdev-dpdk.c | 55 ++++++++++++++++++++++++++++++
> ++++++-------------------
> 1 file changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 1aaf6f7..50874b8 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -279,7 +279,7 @@ struct dpdk_qos_ops {
> * For all QoS implementations it should always be non-null.
> */
> int (*qos_run)(struct qos_conf *qos_conf, struct rte_mbuf **pkts,
> - int pkt_cnt);
> + int pkt_cnt, bool may_steal);
> };
>
> /* dpdk_qos_ops for each type of user space QoS implementation */
> @@ -1501,7 +1501,8 @@ netdev_dpdk_policer_pkt_handle(struct
> rte_meter_srtcm *meter,
>
> static int
> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
> - struct rte_mbuf **pkts, int pkt_cnt)
> + struct rte_mbuf **pkts, int pkt_cnt,
> + bool may_steal)
> {
> int i = 0;
> int cnt = 0;
> @@ -1517,7 +1518,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm
> *meter,
> }
> cnt++;
> } else {
> - rte_pktmbuf_free(pkt);
> + if (may_steal) {
> + rte_pktmbuf_free(pkt);
> + }
> }
> }
>
> @@ -1526,12 +1529,13 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm
> *meter,
>
> static int
> ingress_policer_run(struct ingress_policer *policer, struct rte_mbuf
> **pkts,
> - int pkt_cnt)
> + int pkt_cnt, bool may_steal)
> {
> int cnt = 0;
>
> rte_spinlock_lock(&policer->policer_lock);
> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts,
> pkt_cnt);
> + cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts,
> + pkt_cnt, may_steal);
> rte_spinlock_unlock(&policer->policer_lock);
>
> return cnt;
> @@ -1635,7 +1639,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq
> *rxq,
> dropped = nb_rx;
> nb_rx = ingress_policer_run(policer,
> (struct rte_mbuf **)
> batch->packets,
> - nb_rx);
> + nb_rx, true);
> dropped -= nb_rx;
> }
>
> @@ -1672,7 +1676,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq,
> struct dp_packet_batch *batch)
> dropped = nb_rx;
> nb_rx = ingress_policer_run(policer,
> (struct rte_mbuf **)
> batch->packets,
> - nb_rx);
> + nb_rx, true);
> dropped -= nb_rx;
> }
>
> @@ -1690,13 +1694,13 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq,
> struct dp_packet_batch *batch)
>
> static inline int
> netdev_dpdk_qos_run(struct netdev_dpdk *dev, struct rte_mbuf **pkts,
> - int cnt)
> + int cnt, bool may_steal)
> {
> struct qos_conf *qos_conf = ovsrcu_get(struct qos_conf *,
> &dev->qos_conf);
>
> if (qos_conf) {
> rte_spinlock_lock(&qos_conf->lock);
> - cnt = qos_conf->ops->qos_run(qos_conf, pkts, cnt);
> + cnt = qos_conf->ops->qos_run(qos_conf, pkts, cnt, may_steal);
> rte_spinlock_unlock(&qos_conf->lock);
> }
>
> @@ -1770,7 +1774,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
> int qid,
>
> cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt);
> /* Check has QoS has been configured for the netdev */
> - cnt = netdev_dpdk_qos_run(dev, cur_pkts, cnt);
> + cnt = netdev_dpdk_qos_run(dev, cur_pkts, cnt, true);
> dropped = total_pkts - cnt;
>
> do {
> @@ -1816,10 +1820,22 @@ dpdk_do_tx_copy(struct netdev *netdev, int
> qid, struct dp_packet_batch *batch)
> #endif
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> struct rte_mbuf *pkts[PKT_ARRAY_SIZE];
> + int txcnt_eth = batch->count;
> int dropped = 0;
> int newcnt = 0;
> int i;
>
> + if (dev->type != DPDK_DEV_VHOST) {
> + /* Check if QoS has been configured for this netdev. */
> + txcnt_eth = netdev_dpdk_qos_run(dev,
> + (struct rte_mbuf
> **)batch->packets,
> + txcnt_eth, false);
> + if (txcnt_eth == 0) {
> + dropped = batch->count;
> + goto out;
> + }
> + }
> +
> dp_packet_batch_apply_cutlen(batch);
>
> for (i = 0; i < batch->count; i++) {
> @@ -1848,21 +1864,20 @@ dpdk_do_tx_copy(struct netdev *netdev, int
> qid, struct dp_packet_batch *batch)
> rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
>
> newcnt++;
> + if (dev->type != DPDK_DEV_VHOST && newcnt >= txcnt_eth) {
> + dropped += batch->count - i - 1;
> + break;
> + }
> }
>
> if (dev->type == DPDK_DEV_VHOST) {
> __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **)
> pkts,
> newcnt);
> } else {
> - unsigned int qos_pkts = newcnt;
> -
> - /* Check if QoS has been configured for this netdev. */
> - newcnt = netdev_dpdk_qos_run(dev, pkts, newcnt);
> -
> - dropped += qos_pkts - newcnt;
> dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, newcnt);
> }
>
> +out:
> if (OVS_UNLIKELY(dropped)) {
> rte_spinlock_lock(&dev->stats_lock);
> dev->stats.tx_dropped += dropped;
> @@ -1915,7 +1930,7 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int
> qid,
> dp_packet_batch_apply_cutlen(batch);
>
> cnt = netdev_dpdk_filter_packet_len(dev, pkts, cnt);
> - cnt = netdev_dpdk_qos_run(dev, pkts, cnt);
> + cnt = netdev_dpdk_qos_run(dev, pkts, cnt, true);
> dropped = batch->count - cnt;
>
> dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, cnt);
> @@ -3132,13 +3147,15 @@ egress_policer_qos_is_equal(const struct
> qos_conf *conf,
> }
>
> static int
> -egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int
> pkt_cnt)
> +egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int
> pkt_cnt,
> + bool may_steal)
> {
> int cnt = 0;
> struct egress_policer *policer =
> CONTAINER_OF(conf, struct egress_policer, qos_conf);
>
> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts,
> pkt_cnt);
> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts,
> + pkt_cnt, may_steal);
>
> return cnt;
> }
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.
> openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=
> uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=
> EZzkW98aZhxGnYEEmB3u5e3vaFA90qSr3aIM1t-ggFY&s=
> fHRBBnkb29ujBAr27aGj1MCWkVzjfKqmJz3R9wEg99o&e=
>
>
>
>
>
More information about the dev
mailing list