| Message ID | 20170725102748.501-1-sysugaozhenyu@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
ping.... Thanks Zhenyu Gao 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: > 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@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 ea17b97..bb8bd8f 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -258,7 +258,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 */ > @@ -1438,7 +1438,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; > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm > *meter, > } > cnt++; > } else { > - rte_pktmbuf_free(pkt); > + if (may_steal) { > + rte_pktmbuf_free(pkt); > + } > } > } > > @@ -1463,12 +1466,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; > @@ -1572,7 +1576,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; > } > > @@ -1609,7 +1613,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; > } > > @@ -1627,13 +1631,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); > } > > @@ -1707,7 +1711,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 { > @@ -1753,10 +1757,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++) { > @@ -1785,21 +1801,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; > @@ -1852,7 +1867,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); > @@ -3061,13 +3076,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 > >
Hi Ian Are you interested in reviewing this ? Thanks Darrell -----Original Message----- From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu <sysugaozhenyu@gmail.com> Date: Monday, August 7, 2017 at 6:36 PM To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf ping.... Thanks Zhenyu Gao 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: > 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@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 ea17b97..bb8bd8f 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -258,7 +258,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 */ > @@ -1438,7 +1438,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; > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm > *meter, > } > cnt++; > } else { > - rte_pktmbuf_free(pkt); > + if (may_steal) { > + rte_pktmbuf_free(pkt); > + } > } > } > > @@ -1463,12 +1466,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; > @@ -1572,7 +1576,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; > } > > @@ -1609,7 +1613,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; > } > > @@ -1627,13 +1631,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); > } > > @@ -1707,7 +1711,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 { > @@ -1753,10 +1757,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++) { > @@ -1785,21 +1801,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; > @@ -1852,7 +1867,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); > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F-jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e=
Hi Darrell, I am, I've had a cursory look over it already but was planning to do a deeper dive later this week as I have a few concerns about the approach. Ian > -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com] > Sent: Tuesday, August 8, 2017 3:07 AM > To: Gao Zhenyu <sysugaozhenyu@gmail.com>; Ben Pfaff <blp@ovn.org>; > Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev > <dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > Hi Ian > > Are you interested in reviewing this ? > > Thanks Darrell > > -----Original Message----- > From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu > <sysugaozhenyu@gmail.com> > Date: Monday, August 7, 2017 at 6:36 PM > To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" > <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > ping.... > > Thanks > Zhenyu Gao > > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: > > > 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@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 ea17b97..bb8bd8f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -258,7 +258,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 */ > > @@ -1438,7 +1438,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; > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm > > *meter, > > } > > cnt++; > > } else { > > - rte_pktmbuf_free(pkt); > > + if (may_steal) { > > + rte_pktmbuf_free(pkt); > > + } > > } > > } > > > > @@ -1463,12 +1466,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; > > @@ -1572,7 +1576,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; > > } > > > > @@ -1609,7 +1613,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; > > } > > > > @@ -1627,13 +1631,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); > > } > > > > @@ -1707,7 +1711,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 { > > @@ -1753,10 +1757,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++) { > > @@ -1785,21 +1801,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; > > @@ -1852,7 +1867,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); > > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= >
Thanks for the review. Please let me know if you have any concern on it. :) Thanks Zhenyu Gao 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com>: > Hi Darrell, > > I am, I've had a cursory look over it already but was planning to do a > deeper dive later this week as I have a few concerns about the approach. > > Ian > > > -----Original Message----- > > From: Darrell Ball [mailto:dball@vmware.com] > > Sent: Tuesday, August 8, 2017 3:07 AM > > To: Gao Zhenyu <sysugaozhenyu@gmail.com>; Ben Pfaff <blp@ovn.org>; > > Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev > > <dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com> > > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking > before > > copying to mbuf > > > > Hi Ian > > > > Are you interested in reviewing this ? > > > > Thanks Darrell > > > > -----Original Message----- > > From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu > > <sysugaozhenyu@gmail.com> > > Date: Monday, August 7, 2017 at 6:36 PM > > To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" > > <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> > > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking > before > > copying to mbuf > > > > ping.... > > > > Thanks > > Zhenyu Gao > > > > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: > > > > > 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@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 ea17b97..bb8bd8f 100644 > > > --- a/lib/netdev-dpdk.c > > > +++ b/lib/netdev-dpdk.c > > > @@ -258,7 +258,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 */ > > > @@ -1438,7 +1438,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; > > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct > rte_meter_srtcm > > > *meter, > > > } > > > cnt++; > > > } else { > > > - rte_pktmbuf_free(pkt); > > > + if (may_steal) { > > > + rte_pktmbuf_free(pkt); > > > + } > > > } > > > } > > > > > > @@ -1463,12 +1466,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; > > > @@ -1572,7 +1576,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; > > > } > > > > > > @@ -1609,7 +1613,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; > > > } > > > > > > @@ -1627,13 +1631,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); > > > } > > > > > > @@ -1707,7 +1711,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 { > > > @@ -1753,10 +1757,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++) { > > > @@ -1785,21 +1801,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; > > > @@ -1852,7 +1867,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); > > > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- > > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= > > > >
Hi Ian, This patch is pending for a long time. May I know if I should revise it? Thanks Zhenyu Gao 2017-08-08 17:34 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com>: > Thanks for the review. Please let me know if you have any concern on it. :) > > Thanks > Zhenyu Gao > > 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com>: > >> Hi Darrell, >> >> I am, I've had a cursory look over it already but was planning to do a >> deeper dive later this week as I have a few concerns about the approach. >> >> Ian >> >> > -----Original Message----- >> > From: Darrell Ball [mailto:dball@vmware.com] >> > Sent: Tuesday, August 8, 2017 3:07 AM >> > To: Gao Zhenyu <sysugaozhenyu@gmail.com>; Ben Pfaff <blp@ovn.org>; >> > Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev >> > <dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com> >> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking >> before >> > copying to mbuf >> > >> > Hi Ian >> > >> > Are you interested in reviewing this ? >> > >> > Thanks Darrell >> > >> > -----Original Message----- >> > From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu >> > <sysugaozhenyu@gmail.com> >> > Date: Monday, August 7, 2017 at 6:36 PM >> > To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" >> > <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> >> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking >> before >> > copying to mbuf >> > >> > ping.... >> > >> > Thanks >> > Zhenyu Gao >> > >> > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: >> > >> > > 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@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 ea17b97..bb8bd8f 100644 >> > > --- a/lib/netdev-dpdk.c >> > > +++ b/lib/netdev-dpdk.c >> > > @@ -258,7 +258,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 */ >> > > @@ -1438,7 +1438,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; >> > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct >> rte_meter_srtcm >> > > *meter, >> > > } >> > > cnt++; >> > > } else { >> > > - rte_pktmbuf_free(pkt); >> > > + if (may_steal) { >> > > + rte_pktmbuf_free(pkt); >> > > + } >> > > } >> > > } >> > > >> > > @@ -1463,12 +1466,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; >> > > @@ -1572,7 +1576,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; >> > > } >> > > >> > > @@ -1609,7 +1613,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; >> > > } >> > > >> > > @@ -1627,13 +1631,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); >> > > } >> > > >> > > @@ -1707,7 +1711,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 { >> > > @@ -1753,10 +1757,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++) { >> > > @@ -1785,21 +1801,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; >> > > @@ -1852,7 +1867,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); >> > > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- >> > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= >> > >> >> >
ping... Comments are welcome. Thanks Zhenyu Gao 2017-08-18 9:28 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com>: > Hi Ian, > > This patch is pending for a long time. May I know if I should revise it? > > > Thanks > Zhenyu Gao > > 2017-08-08 17:34 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com>: > >> Thanks for the review. Please let me know if you have any concern on it. >> :) >> >> Thanks >> Zhenyu Gao >> >> 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com>: >> >>> Hi Darrell, >>> >>> I am, I've had a cursory look over it already but was planning to do a >>> deeper dive later this week as I have a few concerns about the approach. >>> >>> Ian >>> >>> > -----Original Message----- >>> > From: Darrell Ball [mailto:dball@vmware.com] >>> > Sent: Tuesday, August 8, 2017 3:07 AM >>> > To: Gao Zhenyu <sysugaozhenyu@gmail.com>; Ben Pfaff <blp@ovn.org>; >>> > Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev >>> > <dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com> >>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking >>> before >>> > copying to mbuf >>> > >>> > Hi Ian >>> > >>> > Are you interested in reviewing this ? >>> > >>> > Thanks Darrell >>> > >>> > -----Original Message----- >>> > From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu >>> > <sysugaozhenyu@gmail.com> >>> > Date: Monday, August 7, 2017 at 6:36 PM >>> > To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" >>> > <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> >>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking >>> before >>> > copying to mbuf >>> > >>> > ping.... >>> > >>> > Thanks >>> > Zhenyu Gao >>> > >>> > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: >>> > >>> > > 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@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 ea17b97..bb8bd8f 100644 >>> > > --- a/lib/netdev-dpdk.c >>> > > +++ b/lib/netdev-dpdk.c >>> > > @@ -258,7 +258,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 >>> */ >>> > > @@ -1438,7 +1438,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; >>> > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct >>> rte_meter_srtcm >>> > > *meter, >>> > > } >>> > > cnt++; >>> > > } else { >>> > > - rte_pktmbuf_free(pkt); >>> > > + if (may_steal) { >>> > > + rte_pktmbuf_free(pkt); >>> > > + } >>> > > } >>> > > } >>> > > >>> > > @@ -1463,12 +1466,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; >>> > > @@ -1572,7 +1576,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; >>> > > } >>> > > >>> > > @@ -1609,7 +1613,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; >>> > > } >>> > > >>> > > @@ -1627,13 +1631,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); >>> > > } >>> > > >>> > > @@ -1707,7 +1711,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 { >>> > > @@ -1753,10 +1757,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++) { >>> > > @@ -1785,21 +1801,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; >>> > > @@ -1852,7 +1867,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); >>> > > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- >>> > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= >>> > >>> >>> >> >
Hi Zhenyu, Sincere apologies for the delay, I’ll be reviewing the patch this week and hope to have feedback by Friday for you. If you could rebase that would be helpful for testing. Thanks Ian From: Gao Zhenyu [mailto:sysugaozhenyu@gmail.com] Sent: Friday, August 18, 2017 2:28 AM To: Stokes, Ian <ian.stokes@intel.com> Cc: Darrell Ball <dball@vmware.com>; Ben Pfaff <blp@ovn.org>; Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf Hi Ian, This patch is pending for a long time. May I know if I should revise it? Thanks Zhenyu Gao 2017-08-08 17:34 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>: Thanks for the review. Please let me know if you have any concern on it. :) Thanks Zhenyu Gao 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>>: Hi Darrell, I am, I've had a cursory look over it already but was planning to do a deeper dive later this week as I have a few concerns about the approach. Ian > -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com<mailto:dball@vmware.com>] > Sent: Tuesday, August 8, 2017 3:07 AM > To: Gao Zhenyu <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>; Ben Pfaff <blp@ovn.org<mailto:blp@ovn.org>>; > Chandran, Sugesh <sugesh.chandran@intel.com<mailto:sugesh.chandran@intel.com>>; ovs dev > <dev@openvswitch.org<mailto:dev@openvswitch.org>>; Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > Hi Ian > > Are you interested in reviewing this ? > > Thanks Darrell > > -----Original Message----- > From: <ovs-dev-bounces@openvswitch.org<mailto:ovs-dev-bounces@openvswitch.org>> on behalf of Gao Zhenyu > <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>> > Date: Monday, August 7, 2017 at 6:36 PM > To: Ben Pfaff <blp@ovn.org<mailto:blp@ovn.org>>, "Chandran, Sugesh" > <sugesh.chandran@intel.com<mailto:sugesh.chandran@intel.com>>, ovs dev <dev@openvswitch.org<mailto:dev@openvswitch.org>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > ping.... > > Thanks > Zhenyu Gao > > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>: > > > 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@gmail.com<mailto:sysugaozhenyu@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 ea17b97..bb8bd8f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -258,7 +258,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 */ > > @@ -1438,7 +1438,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; > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm > > *meter, > > } > > cnt++; > > } else { > > - rte_pktmbuf_free(pkt); > > + if (may_steal) { > > + rte_pktmbuf_free(pkt); > > + } > > } > > } > > > > @@ -1463,12 +1466,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; > > @@ -1572,7 +1576,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; > > } > > > > @@ -1609,7 +1613,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; > > } > > > > @@ -1627,13 +1631,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); > > } > > > > @@ -1707,7 +1711,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 { > > @@ -1753,10 +1757,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++) { > > @@ -1785,21 +1801,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; > > @@ -1852,7 +1867,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); > > @@ -3061,13 +3076,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@openvswitch.org<mailto:dev@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= >
Hi Ian, Please take a look at v2 https://patchwork.ozlabs.org/patch/807059/ :) No code change, just rebase to latest master. Thanks Zhenyu Gao 2017-08-28 21:12 GMT+08:00 Stokes, Ian <ian.stokes@intel.com>: > Hi Zhenyu, > > > > Sincere apologies for the delay, I’ll be reviewing the patch this week and > hope to have feedback by Friday for you. If you could rebase that would be > helpful for testing. > > > > Thanks > > Ian > > > > *From:* Gao Zhenyu [mailto:sysugaozhenyu@gmail.com] > *Sent:* Friday, August 18, 2017 2:28 AM > *To:* Stokes, Ian <ian.stokes@intel.com> > *Cc:* Darrell Ball <dball@vmware.com>; Ben Pfaff <blp@ovn.org>; Chandran, > Sugesh <sugesh.chandran@intel.com>; ovs dev <dev@openvswitch.org> > > *Subject:* Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking > before copying to mbuf > > > > Hi Ian, > > This patch is pending for a long time. May I know if I should revise it? > > Thanks > > Zhenyu Gao > > > > 2017-08-08 17:34 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com>: > > Thanks for the review. Please let me know if you have any concern on it. :) > > Thanks > > Zhenyu Gao > > > > 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com>: > > Hi Darrell, > > I am, I've had a cursory look over it already but was planning to do a > deeper dive later this week as I have a few concerns about the approach. > > Ian > > > > -----Original Message----- > > From: Darrell Ball [mailto:dball@vmware.com] > > Sent: Tuesday, August 8, 2017 3:07 AM > > To: Gao Zhenyu <sysugaozhenyu@gmail.com>; Ben Pfaff <blp@ovn.org>; > > Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev > > <dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com> > > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking > before > > copying to mbuf > > > > Hi Ian > > > > Are you interested in reviewing this ? > > > > Thanks Darrell > > > > -----Original Message----- > > From: <ovs-dev-bounces@openvswitch.org> on behalf of Gao Zhenyu > > <sysugaozhenyu@gmail.com> > > Date: Monday, August 7, 2017 at 6:36 PM > > To: Ben Pfaff <blp@ovn.org>, "Chandran, Sugesh" > > <sugesh.chandran@intel.com>, ovs dev <dev@openvswitch.org> > > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking > before > > copying to mbuf > > > > ping.... > > > > Thanks > > Zhenyu Gao > > > > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com>: > > > > > 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@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 ea17b97..bb8bd8f 100644 > > > --- a/lib/netdev-dpdk.c > > > +++ b/lib/netdev-dpdk.c > > > @@ -258,7 +258,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 */ > > > @@ -1438,7 +1438,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; > > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct > rte_meter_srtcm > > > *meter, > > > } > > > cnt++; > > > } else { > > > - rte_pktmbuf_free(pkt); > > > + if (may_steal) { > > > + rte_pktmbuf_free(pkt); > > > + } > > > } > > > } > > > > > > @@ -1463,12 +1466,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; > > > @@ -1572,7 +1576,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; > > > } > > > > > > @@ -1609,7 +1613,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; > > > } > > > > > > @@ -1627,13 +1631,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); > > > } > > > > > > @@ -1707,7 +1711,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 { > > > @@ -1753,10 +1757,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++) { > > > @@ -1785,21 +1801,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; > > > @@ -1852,7 +1867,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); > > > @@ -3061,13 +3076,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@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- > > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= > > > > > > >
Thanks for rebasing Zhenyu, Will look at this version for testing and review this week. Ian From: Gao Zhenyu [mailto:sysugaozhenyu@gmail.com] Sent: Tuesday, August 29, 2017 1:33 PM To: Stokes, Ian <ian.stokes@intel.com> Cc: Darrell Ball <dball@vmware.com>; Ben Pfaff <blp@ovn.org>; Chandran, Sugesh <sugesh.chandran@intel.com>; ovs dev <dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf Hi Ian, Please take a look at v2 https://patchwork.ozlabs.org/patch/807059/ :) No code change, just rebase to latest master. Thanks Zhenyu Gao 2017-08-28 21:12 GMT+08:00 Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>>: Hi Zhenyu, Sincere apologies for the delay, I’ll be reviewing the patch this week and hope to have feedback by Friday for you. If you could rebase that would be helpful for testing. Thanks Ian From: Gao Zhenyu [mailto:sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>] Sent: Friday, August 18, 2017 2:28 AM To: Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>> Cc: Darrell Ball <dball@vmware.com<mailto:dball@vmware.com>>; Ben Pfaff <blp@ovn.org<mailto:blp@ovn.org>>; Chandran, Sugesh <sugesh.chandran@intel.com<mailto:sugesh.chandran@intel.com>>; ovs dev <dev@openvswitch.org<mailto:dev@openvswitch.org>> Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf Hi Ian, This patch is pending for a long time. May I know if I should revise it? Thanks Zhenyu Gao 2017-08-08 17:34 GMT+08:00 Gao Zhenyu <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>: Thanks for the review. Please let me know if you have any concern on it. :) Thanks Zhenyu Gao 2017-08-08 17:08 GMT+08:00 Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>>: Hi Darrell, I am, I've had a cursory look over it already but was planning to do a deeper dive later this week as I have a few concerns about the approach. Ian > -----Original Message----- > From: Darrell Ball [mailto:dball@vmware.com<mailto:dball@vmware.com>] > Sent: Tuesday, August 8, 2017 3:07 AM > To: Gao Zhenyu <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>; Ben Pfaff <blp@ovn.org<mailto:blp@ovn.org>>; > Chandran, Sugesh <sugesh.chandran@intel.com<mailto:sugesh.chandran@intel.com>>; ovs dev > <dev@openvswitch.org<mailto:dev@openvswitch.org>>; Stokes, Ian <ian.stokes@intel.com<mailto:ian.stokes@intel.com>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > Hi Ian > > Are you interested in reviewing this ? > > Thanks Darrell > > -----Original Message----- > From: <ovs-dev-bounces@openvswitch.org<mailto:ovs-dev-bounces@openvswitch.org>> on behalf of Gao Zhenyu > <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>> > Date: Monday, August 7, 2017 at 6:36 PM > To: Ben Pfaff <blp@ovn.org<mailto:blp@ovn.org>>, "Chandran, Sugesh" > <sugesh.chandran@intel.com<mailto:sugesh.chandran@intel.com>>, ovs dev <dev@openvswitch.org<mailto:dev@openvswitch.org>> > Subject: Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before > copying to mbuf > > ping.... > > Thanks > Zhenyu Gao > > 2017-07-25 18:27 GMT+08:00 Zhenyu Gao <sysugaozhenyu@gmail.com<mailto:sysugaozhenyu@gmail.com>>: > > > 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@gmail.com<mailto:sysugaozhenyu@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 ea17b97..bb8bd8f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -258,7 +258,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 */ > > @@ -1438,7 +1438,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; > > @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm > > *meter, > > } > > cnt++; > > } else { > > - rte_pktmbuf_free(pkt); > > + if (may_steal) { > > + rte_pktmbuf_free(pkt); > > + } > > } > > } > > > > @@ -1463,12 +1466,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; > > @@ -1572,7 +1576,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; > > } > > > > @@ -1609,7 +1613,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; > > } > > > > @@ -1627,13 +1631,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); > > } > > > > @@ -1707,7 +1711,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 { > > @@ -1753,10 +1757,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++) { > > @@ -1785,21 +1801,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; > > @@ -1852,7 +1867,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); > > @@ -3061,13 +3076,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@openvswitch.org<mailto:dev@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=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F- > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e= >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ea17b97..bb8bd8f 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -258,7 +258,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 */ @@ -1438,7 +1438,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; @@ -1454,7 +1455,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, } cnt++; } else { - rte_pktmbuf_free(pkt); + if (may_steal) { + rte_pktmbuf_free(pkt); + } } } @@ -1463,12 +1466,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; @@ -1572,7 +1576,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; } @@ -1609,7 +1613,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; } @@ -1627,13 +1631,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); } @@ -1707,7 +1711,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 { @@ -1753,10 +1757,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++) { @@ -1785,21 +1801,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; @@ -1852,7 +1867,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); @@ -3061,13 +3076,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; }
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@gmail.com> --- lib/netdev-dpdk.c | 55 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 19 deletions(-)