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.sto...@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:sysugaozhe...@gmail.com] > *Sent:* Friday, August 18, 2017 2:28 AM > *To:* Stokes, Ian <ian.sto...@intel.com> > *Cc:* Darrell Ball <db...@vmware.com>; Ben Pfaff <b...@ovn.org>; Chandran, > Sugesh <sugesh.chand...@intel.com>; ovs dev <d...@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 <sysugaozhe...@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.sto...@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:db...@vmware.com] > > Sent: Tuesday, August 8, 2017 3:07 AM > > To: Gao Zhenyu <sysugaozhe...@gmail.com>; Ben Pfaff <b...@ovn.org>; > > Chandran, Sugesh <sugesh.chand...@intel.com>; ovs dev > > <d...@openvswitch.org>; Stokes, Ian <ian.sto...@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-boun...@openvswitch.org> on behalf of Gao Zhenyu > > <sysugaozhe...@gmail.com> > > Date: Monday, August 7, 2017 at 6:36 PM > > To: Ben Pfaff <b...@ovn.org>, "Chandran, Sugesh" > > <sugesh.chand...@intel.com>, ovs dev <d...@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 <sysugaozhe...@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 <sysugaozhe...@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 > > d...@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= > > > > > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
