[ovs-dev] [PATCH v2 0/4] netdev-dpdk: Support the multi-queue QoS for dpdk datapath

Stokes, Ian ian.stokes at intel.com
Tue Oct 17 08:52:30 UTC 2017

> This patch adds similar QoS construction with kernel datapath.
> Original command as below:
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
>     --id=@newqos create qos type=egress-policer \
>     other-config:cir=46000000 other-config:cbs=2048`
> New command as below:
> $ ovs-vsctl set port vhost-user0 qos=@newqos -- \
>     --id=@newqos create qos type=egress-policer \
>     other-config:cir=46000000 other-config:cbs=2048 \
>     queues:123=@q123 queues:234=@q234 -- \
>     --id=@q123 create queue \
>     other-config:cir=12800000 other-config:cbs=2048 -- \
>     --id=@q234 create queue \
>     other-config:cir=25600000 other-config:cbs=2048`
> Through new command, QoS and multi-queue will to be set, then use OpenFlow
> to direct packet to queues:
> $ ovs-ofctl add-flow br0 in_port=5,actions=set_queue:123,normal
> $ ovs-ofctl add-flow br0 in_port=6,actions=set_queue:234,normal
> Finally, show QoS and its queues information through command as below:
> $ ovs-appctl -t ovs-vswitchd qos/show vhost-user0
> I respined the patches based on review feedback at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336141.html

Thanks for the rework zhaozhanxu.

I did a rough high level review of the patchset and have a few comments for each patch.

I'd like to do some testing next but there's been some churn on master since the previous submission, if you could rebase to master for a v.3 and address the comments I've made I'll proceed with more in depth testing.

Just one observation for all the patches. You need to address the commit message style and abide by what is expected in OVS. Usually it's as follows

[PATCH <n>/<m>]: indicates that this is the nth of a series of m patches. It helps reviewers to read patches in the correct order. You may omit this prefix if you are sending only one patch.

<area>: indicates the area of the Open vSwitch to which the change applies (often the name of a source file or a directory). You may omit it if the change crosses multiple distinct pieces of code.

<summary>: briefly describes the change. Use the the imperative form, e.g. "Force SNAT for multiple gateway routers." or "Fix daemon exit for bad datapaths or flows." Try to keep the summary short, about 50 characters wide.

There are multiple examples of this for netdev-dpdk and docs in the commit message logs you can follow.

> Patch 1: Add multi-queue QoS configuration.
> Patch 2: Add multi-queue QoS function.
> Patch 3: Add QoS queues information to show.
> Patch 4: Modify QoS configuration command.
> --
> V1->V2:
> 1. modified some code style error for every patch noticed by Ian.
> 2. modified some patch error for patch 1-3 noticed for by Ian.
> 3. modified documents error for patch 4.
> zhaozhanxu (4):
>   netdev-dpdk.c: Support the multi-queue QoS configuration for dpdk
>     datapath
>   netdev-dpdk.c: Support multi-queue QoS rate limitation function for
>     dpdk datapath
>   netdev-dpdk.c: Support to show multi-queue qos info for dpdk datapath
>   dpdk.rst: Modify QoS configure documents for ovs-dpdk
>  Documentation/howto/dpdk.rst |  14 +-
>  lib/netdev-dpdk.c            | 387
> +++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 389 insertions(+), 12 deletions(-)
> --
> 2.7.4
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

More information about the dev mailing list