ping... Comments are welcome. Thanks Zhenyu Gao
2017-08-18 9:28 GMT+08:00 Gao Zhenyu <[email protected]>: > 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 <[email protected]>: > >> 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 <[email protected]>: >> >>> 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:[email protected]] >>> > Sent: Tuesday, August 8, 2017 3:07 AM >>> > To: Gao Zhenyu <[email protected]>; Ben Pfaff <[email protected]>; >>> > Chandran, Sugesh <[email protected]>; ovs dev >>> > <[email protected]>; Stokes, Ian <[email protected]> >>> > 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: <[email protected]> on behalf of Gao Zhenyu >>> > <[email protected]> >>> > Date: Monday, August 7, 2017 at 6:36 PM >>> > To: Ben Pfaff <[email protected]>, "Chandran, Sugesh" >>> > <[email protected]>, ovs dev <[email protected]> >>> > 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 <[email protected]>: >>> > >>> > > 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 <[email protected]> >>> > > --- >>> > > 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 >>> > [email protected] >>> > 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
