Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf
Thanks for rebasing Zhenyu,
Will look at this version for testing and review this week.
Ian
From: Gao Zhenyu [mailto:sysugaozhe...@gmail.com]
Sent: Tuesday, August 29, 2017 1:33 PM
To: Stokes, Ian
Cc: Darrell Ball ; Ben Pfaff ; Chandran, Sugesh
; ovs dev
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
mailto: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<mailto:sysugaozhe...@gmail.com>]
Sent: Friday, August 18, 2017 2:28 AM
To: Stokes, Ian mailto:ian.sto...@intel.com>>
Cc: Darrell Ball mailto:db...@vmware.com>>; Ben Pfaff
mailto:b...@ovn.org>>; Chandran, Sugesh
mailto:sugesh.chand...@intel.com>>; ovs dev
mailto: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
mailto: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
mailto: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<mailto:db...@vmware.com>]
> Sent: Tuesday, August 8, 2017 3:07 AM
> To: Gao Zhenyu mailto:sysugaozhe...@gmail.com>>; Ben
> Pfaff mailto:b...@ovn.org>>;
> Chandran, Sugesh
> mailto:sugesh.chand...@intel.com>>; ovs dev
> mailto:d...@openvswitch.org>>; Stokes, Ian
> mailto: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:
> mailto:ovs-dev-boun...@openvswitch.org>> on
> behalf of Gao Zhenyu
> mailto:sysugaozhe...@gmail.com>>
> Date: Monday, August 7, 2017 at 6:36 PM
> To: Ben Pfaff mailto:b...@ovn.org>>, "Chandran, Sugesh"
> mailto:sugesh.chand...@intel.com>>, ovs dev
> mailto: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
> mailto: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
> mailto: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 {
>
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 :
> 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
> *Cc:* Darrell Ball ; Ben Pfaff ; Chandran,
> Sugesh ; ovs dev
>
> *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 :
>
> 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 :
>
> 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 ; Ben Pfaff ;
> > Chandran, Sugesh ; ovs dev
> > ; Stokes, Ian
> > 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: on behalf of Gao Zhenyu
> >
> > Date: Monday, August 7, 2017 at 6:36 PM
> > To: Ben Pfaff , "Chandran, Sugesh"
> > , ovs dev
> > 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 :
> >
> > > 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
> > > ---
> > > 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 ingre
Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf
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
Cc: Darrell Ball ; Ben Pfaff ; Chandran, Sugesh
; ovs dev
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
mailto: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
mailto: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<mailto:db...@vmware.com>]
> Sent: Tuesday, August 8, 2017 3:07 AM
> To: Gao Zhenyu mailto:sysugaozhe...@gmail.com>>; Ben
> Pfaff mailto:b...@ovn.org>>;
> Chandran, Sugesh
> mailto:sugesh.chand...@intel.com>>; ovs dev
> mailto:d...@openvswitch.org>>; Stokes, Ian
> mailto: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:
> mailto:ovs-dev-boun...@openvswitch.org>> on
> behalf of Gao Zhenyu
> mailto:sysugaozhe...@gmail.com>>
> Date: Monday, August 7, 2017 at 6:36 PM
> To: Ben Pfaff mailto:b...@ovn.org>>, "Chandran, Sugesh"
> mailto:sugesh.chand...@intel.com>>, ovs dev
> mailto: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
> mailto: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
> mailto: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
Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf
ping... Comments are welcome.
Thanks
Zhenyu Gao
2017-08-18 9:28 GMT+08:00 Gao Zhenyu :
> 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 :
>
>> 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 :
>>
>>> 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 ; Ben Pfaff ;
>>> > Chandran, Sugesh ; ovs dev
>>> > ; Stokes, Ian
>>> > 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: on behalf of Gao Zhenyu
>>> >
>>> > Date: Monday, August 7, 2017 at 6:36 PM
>>> > To: Ben Pfaff , "Chandran, Sugesh"
>>> > , ovs dev
>>> > 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 :
>>> >
>>> > > 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
>>> > > ---
>>> > > 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,
>>> > >
>>
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 :
> 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 :
>
>> 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 ; Ben Pfaff ;
>> > Chandran, Sugesh ; ovs dev
>> > ; Stokes, Ian
>> > 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: on behalf of Gao Zhenyu
>> >
>> > Date: Monday, August 7, 2017 at 6:36 PM
>> > To: Ben Pfaff , "Chandran, Sugesh"
>> > , ovs dev
>> > 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 :
>> >
>> > > 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
>> > > ---
>> > > 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);
&
Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf
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 :
> 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 ; Ben Pfaff ;
> > Chandran, Sugesh ; ovs dev
> > ; Stokes, Ian
> > 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: on behalf of Gao Zhenyu
> >
> > Date: Monday, August 7, 2017 at 6:36 PM
> > To: Ben Pfaff , "Chandran, Sugesh"
> > , ovs dev
> > 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 :
> >
> > > 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
> > > ---
> > > 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-
> >
Re: [ovs-dev] [PATCH v1] netdev-dpdk: Execute QoS Checking before copying to mbuf
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 ; Ben Pfaff ;
> Chandran, Sugesh ; ovs dev
> ; Stokes, Ian
> 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: on behalf of Gao Zhenyu
>
> Date: Monday, August 7, 2017 at 6:36 PM
> To: Ben Pfaff , "Chandran, Sugesh"
> , ovs dev
> 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 :
>
> > 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
> > ---
> > 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;
> >
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: on behalf of Gao Zhenyu
Date: Monday, August 7, 2017 at 6:36 PM
To: Ben Pfaff , "Chandran, Sugesh" ,
ovs dev
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 :
> 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
> ---
> 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 con
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 :
> 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
> ---
> 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
