[ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf

Gao Zhenyu sysugaozhenyu at gmail.com
Fri Aug 18 01:28:26 UTC 2017


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 at 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 at 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 at vmware.com]
>> > Sent: Tuesday, August 8, 2017 3:07 AM
>> > To: Gao Zhenyu <sysugaozhenyu at gmail.com>; Ben Pfaff <blp at ovn.org>;
>> > Chandran, Sugesh <sugesh.chandran at intel.com>; ovs dev
>> > <dev at openvswitch.org>; Stokes, Ian <ian.stokes at 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 at openvswitch.org> on behalf of Gao Zhenyu
>> > <sysugaozhenyu at gmail.com>
>> > Date: Monday, August 7, 2017 at 6:36 PM
>> > To: Ben Pfaff <blp at ovn.org>, "Chandran, Sugesh"
>> > <sugesh.chandran at intel.com>, ovs dev <dev at 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 at 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 at gmail.com>
>> >     > ---
>> >     >  lib/netdev-dpdk.c | 55 ++++++++++++++++++++++++++++++
>> >     > ++++++-------------------
>> >     >  1 file changed, 36 insertions(+), 19 deletions(-)
>> >     >
>> >     > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> >     > index 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 at openvswitch.org
>> >     https://urldefense.proofpoint.com/v2/url?u=https-
>> > 3A__mail.openvswitch.org_mailman_listinfo_ovs-
>> > 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> > uZnsw&m=roZOnbVuoZgmZiJ5Yl6u9BT9dmjoSQBFIScSFBX2pTE&s=F-
>> > jBBvlqt9jYLPkVbaiqJoGKjt3zYp1sTA1LtUK6AJ0&e=
>> >
>>
>>
>


More information about the dev mailing list