Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Wed, Jul 20, 2016 at 08:02:03PM +0530, bscha...@redhat.com wrote: > ovn-northd processes the list of Port_Bindings and hashes the list of > queues per chassis. When it finds a port with qos_parameters and without > a queue_id, it allocates a free queue for the chassis that this port belongs. > The queue_id information is stored in the options field of Port_binding table. > Adds an action set_queue to the ingress table 0 of the logical flows > which will be translated to openflow set_queue by ovn-controller > > ovn-controller opens the netdev corresponding to the tunnel interface's > status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for > each SB port_binding that has queue_id set, it allocates a queue with the > qos_parameters of that port. It also frees up unused queues. > > This patch replaces the older approach of policing > > Signed-off-by: Babu Shanmugam I suggest folding in the following changes. Notably, set_queue(0); was documented but didn't work because QDISC_MIN_QUEUE_ID was 1. This series has no tests. I think that at least the DSCP marking feature could be tested with the existing OVN testing infrastructure. Will you work on that? Thanks, Ben. --8<--cut here-->8-- diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h index c0fde28..230115f 100644 --- a/ovn/lib/actions.h +++ b/ovn/lib/actions.h @@ -24,7 +24,11 @@ #include "openvswitch/dynamic-string.h" #include "util.h" -#define QDISC_MIN_QUEUE_ID 1 +/* Valid arguments to set_queue() action. + * + * QDISC_MIN_QUEUE_ID is the default queue, so user-defined queues should + * start at QDISC_MIN_QUEUE_ID+1. */ +#define QDISC_MIN_QUEUE_ID 0 #define QDISC_MAX_QUEUE_ID 0xf000 struct expr; diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 4eef2d9..dc803d2 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -309,7 +309,7 @@ chassis_queueid_in_use(const struct hmap *set, const char *chassis, static uint32_t allocate_chassis_queueid(struct hmap *set, const char *chassis) { -for (uint32_t queue_id = QDISC_MIN_QUEUE_ID; +for (uint32_t queue_id = QDISC_MIN_QUEUE_ID + 1; queue_id <= QDISC_MAX_QUEUE_ID; queue_id++) { if (!chassis_queueid_in_use(set, chassis, queue_id)) { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 5e6f9c3..057a4f1 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -1768,10 +1768,10 @@ tcp.flags = RST; interface, in bits. - + Indicates the queue number on the physical device. This is same as the -queue_id used in OpenFlow in struct ofp_action_enqueue. Value should -be in the range of 1 to 61440. +queue_id used in OpenFlow in struct ofp_action_enqueue. diff --git a/tests/ovn.at b/tests/ovn.at index 86efcf5..84ee2c1 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -655,6 +655,11 @@ reg1[0] = put_dhcp_opts(offerip=1.2.3.4, domain=1.2.3.4); => DHCP option domain # na na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport = inport; inport = ""; /* Allow sending out inport. */ output; }; => actions=controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.00.19.00.10.00.01.1c.04.00.00.00.00.00.00.00.00.00.19.00.10.00.00.00.02.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00), prereqs=nd +# set_queue +set_queue(0); => actions=set_queue:0, prereqs=1 +set_queue(61440); => actions=set_queue:61440, prereqs=1 +set_queue(65535); => Queue ID 65535 for set_queue is not in valid range 0 to 61440. + # Contradictionary prerequisites (allowed but not useful): ip4.src = ip6.src[0..31]; => actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd ip4.src <-> ip6.src[0..31]; => actions=push:NXM_NX_IPV6_SRC[0..31],push:NXM_OF_IP_SRC[],pop:NXM_NX_IPV6_SRC[0..31],pop:NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: >ovn-northd processes the list of Port_Bindings and hashes the list of >queues per chassis. When it finds a port with qos_parameters and without >a queue_id, it allocates a free queue for the chassis that this port belongs. >The queue_id information is stored in the options field of Port_binding table. >Adds an action set_queue to the ingress table 0 of the logical flows >which will be translated to openflow set_queue by ovn-controller > >ovn-controller opens the netdev corresponding to the tunnel interface's >status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for >each SB port_binding that has queue_id set, it allocates a queue with the >qos_parameters of that port. It also frees up unused queues. > >This patch replaces the older approach of policing > >Signed-off-by: Babu Shanmugam I suggest folding in the following changes. Notably, set_queue(0); was documented but didn't work because QDISC_MIN_QUEUE_ID was 1. Thanks for the review, Ben This series has no tests. I think that at least the DSCP marking feature could be tested with the existing OVN testing infrastructure. Will you work on that? Sure. I will work on it. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: >ovn-northd processes the list of Port_Bindings and hashes the list of >queues per chassis. When it finds a port with qos_parameters and without >a queue_id, it allocates a free queue for the chassis that this port belongs. >The queue_id information is stored in the options field of Port_binding table. >Adds an action set_queue to the ingress table 0 of the logical flows >which will be translated to openflow set_queue by ovn-controller > >ovn-controller opens the netdev corresponding to the tunnel interface's >status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for >each SB port_binding that has queue_id set, it allocates a queue with the >qos_parameters of that port. It also frees up unused queues. > >This patch replaces the older approach of policing > >Signed-off-by: Babu Shanmugam I suggest folding in the following changes. Notably, set_queue(0); was documented but didn't work because QDISC_MIN_QUEUE_ID was 1. This series has no tests. I think that at least the DSCP marking feature could be tested with the existing OVN testing infrastructure. Will you work on that? Ben, I am trying to write a test case with the following sequence to test the dscp marking. - Add two ports (lp1, lp2) through ovn-nbctl and assign addresses - Setup the vswitch db with external_ids:iface-id for the logical ports created above - Set options:qos_dscp on one logical port (lp1) through ovn-nbctl - ofproto/trace a simple ip packet originating from lp1, and check if the final flow has the DSCP bit set or not ofproto/trace is tracing out correctly only when qos_dscp is not set. When qos_dscp is set, the controller adds includes an additional action "0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it drops the packet after it passes through table 0. Do you have any suggestion on how to overcome this? Thank you, Babu ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote: > > > On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: > >On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: > >>>ovn-northd processes the list of Port_Bindings and hashes the list of > >>>queues per chassis. When it finds a port with qos_parameters and without > >>>a queue_id, it allocates a free queue for the chassis that this port > >>>belongs. > >>>The queue_id information is stored in the options field of Port_binding > >>>table. > >>>Adds an action set_queue to the ingress table 0 of the logical flows > >>>which will be translated to openflow set_queue by ovn-controller > >>> > >>>ovn-controller opens the netdev corresponding to the tunnel interface's > >>>status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for > >>>each SB port_binding that has queue_id set, it allocates a queue with the > >>>qos_parameters of that port. It also frees up unused queues. > >>> > >>>This patch replaces the older approach of policing > >>> > >>>Signed-off-by: Babu Shanmugam > >I suggest folding in the following changes. Notably, set_queue(0); was > >documented but didn't work because QDISC_MIN_QUEUE_ID was 1. > > > >This series has no tests. I think that at least the DSCP marking > >feature could be tested with the existing OVN testing infrastructure. > >Will you work on that? > Ben, > I am trying to write a test case with the following sequence to test the > dscp marking. > - Add two ports (lp1, lp2) through ovn-nbctl and assign addresses > - Setup the vswitch db with external_ids:iface-id for the logical ports > created above > - Set options:qos_dscp on one logical port (lp1) through ovn-nbctl > - ofproto/trace a simple ip packet originating from lp1, and check if the > final flow has the DSCP bit set or not > > ofproto/trace is tracing out correctly only when qos_dscp is not set. When > qos_dscp is set, the controller adds includes an additional action > "0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it drops > the packet after it passes through table 0. > > Do you have any suggestion on how to overcome this? Hmm, that's odd. What's the whole trace output look like? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Monday 01 August 2016 09:14 PM, Ben Pfaff wrote: On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote: On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: ovn-northd processes the list of Port_Bindings and hashes the list of queues per chassis. When it finds a port with qos_parameters and without a queue_id, it allocates a free queue for the chassis that this port belongs. The queue_id information is stored in the options field of Port_binding table. Adds an action set_queue to the ingress table 0 of the logical flows which will be translated to openflow set_queue by ovn-controller ovn-controller opens the netdev corresponding to the tunnel interface's status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for each SB port_binding that has queue_id set, it allocates a queue with the qos_parameters of that port. It also frees up unused queues. This patch replaces the older approach of policing Signed-off-by: Babu Shanmugam I suggest folding in the following changes. Notably, set_queue(0); was documented but didn't work because QDISC_MIN_QUEUE_ID was 1. This series has no tests. I think that at least the DSCP marking feature could be tested with the existing OVN testing infrastructure. Will you work on that? Ben, I am trying to write a test case with the following sequence to test the dscp marking. - Add two ports (lp1, lp2) through ovn-nbctl and assign addresses - Setup the vswitch db with external_ids:iface-id for the logical ports created above - Set options:qos_dscp on one logical port (lp1) through ovn-nbctl - ofproto/trace a simple ip packet originating from lp1, and check if the final flow has the DSCP bit set or not ofproto/trace is tracing out correctly only when qos_dscp is not set. When qos_dscp is set, the controller adds includes an additional action "0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it drops the packet after it passes through table 0. Do you have any suggestion on how to overcome this? Hmm, that's odd. What's the whole trace output look like? It looks like the following $ ovs-appctl ofproto/trace br-int 'in_port=1,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01' -generate Bridge: br-int Flow: in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Rule: table=0 cookie=0 priority=100,in_port=1 OpenFlow actions=set_field:0x1->reg13,set_field:0x1->metadata,set_field:0x1->reg14,resubmit(,16) Resubmitted flow: reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Resubmitted regs: reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 reg5=0x0 reg6=0x0 reg7=0x0 reg8=0x0 reg9=0x0 reg10=0x0 reg11=0x0 reg12=0x0 reg13=0x1 reg14=0x1 reg15=0x0 Resubmitted odp: drop Resubmitted megaflow: recirc_id=0,reg13=0,reg14=0,metadata=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x Rule: table=254 cookie=0 priority=0,reg0=0x2 OpenFlow actions=drop Final flow: reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Megaflow: recirc_id=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x Datapath actions: drop -- Babu ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Monday 01 August 2016 10:09 PM, Babu Shanmugam wrote: On Monday 01 August 2016 09:14 PM, Ben Pfaff wrote: On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote: On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: ovn-northd processes the list of Port_Bindings and hashes the list of queues per chassis. When it finds a port with qos_parameters and without a queue_id, it allocates a free queue for the chassis that this port belongs. The queue_id information is stored in the options field of Port_binding table. Adds an action set_queue to the ingress table 0 of the logical flows which will be translated to openflow set_queue by ovn-controller ovn-controller opens the netdev corresponding to the tunnel interface's status:tunnel_egress_iface value and configures a HTB qdisc on it. Then for each SB port_binding that has queue_id set, it allocates a queue with the qos_parameters of that port. It also frees up unused queues. This patch replaces the older approach of policing Signed-off-by: Babu Shanmugam I suggest folding in the following changes. Notably, set_queue(0); was documented but didn't work because QDISC_MIN_QUEUE_ID was 1. This series has no tests. I think that at least the DSCP marking feature could be tested with the existing OVN testing infrastructure. Will you work on that? Ben, I am trying to write a test case with the following sequence to test the dscp marking. - Add two ports (lp1, lp2) through ovn-nbctl and assign addresses - Setup the vswitch db with external_ids:iface-id for the logical ports created above - Set options:qos_dscp on one logical port (lp1) through ovn-nbctl - ofproto/trace a simple ip packet originating from lp1, and check if the final flow has the DSCP bit set or not ofproto/trace is tracing out correctly only when qos_dscp is not set. When qos_dscp is set, the controller adds includes an additional action "0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it drops the packet after it passes through table 0. Do you have any suggestion on how to overcome this? Hmm, that's odd. What's the whole trace output look like? It looks like the following $ ovs-appctl ofproto/trace br-int 'in_port=1,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01' -generate Bridge: br-int Flow: in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Rule: table=0 cookie=0 priority=100,in_port=1 OpenFlow actions=set_field:0x1->reg13,set_field:0x1->metadata,set_field:0x1->reg14,resubmit(,16) Resubmitted flow: reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Resubmitted regs: reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 reg5=0x0 reg6=0x0 reg7=0x0 reg8=0x0 reg9=0x0 reg10=0x0 reg11=0x0 reg12=0x0 reg13=0x1 reg14=0x1 reg15=0x0 Resubmitted odp: drop Resubmitted megaflow: recirc_id=0,reg13=0,reg14=0,metadata=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x Rule: table=254 cookie=0 priority=0,reg0=0x2 OpenFlow actions=drop Final flow: reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x Megaflow: recirc_id=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x Datapath actions: drop Turns out that it was due to a bug in the DSCP marking code. The DSCP marking was done at L2 stage without any additional flows to handle the L2 packets itself. So any L3 packet gets dropped at this stage. I moved the DSCP marking logic to L3 pipeline with additional tests and submitted the patches for review. Sorry for the trouble, Babu -- Babu ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v6 1/2] Check and allocate free qdisc queue id for ports with qos parameters
On Tue, Aug 02, 2016 at 03:06:26PM +0530, Babu Shanmugam wrote: > > > On Monday 01 August 2016 10:09 PM, Babu Shanmugam wrote: > > > > > >On Monday 01 August 2016 09:14 PM, Ben Pfaff wrote: > >>On Mon, Aug 01, 2016 at 07:03:14PM +0530, Babu Shanmugam wrote: > >>> > >>>On Thursday 28 July 2016 01:48 AM, Ben Pfaff wrote: > On Wed, Jul 20, 2016 at 08:02:03PM +0530,bscha...@redhat.com wrote: > >>ovn-northd processes the list of Port_Bindings and hashes the > >>list of > >>queues per chassis. When it finds a port with qos_parameters and > >>without > >>a queue_id, it allocates a free queue for the chassis that this > >>port belongs. > >>The queue_id information is stored in the options field of > >>Port_binding table. > >>Adds an action set_queue to the ingress table 0 of the logical flows > >>which will be translated to openflow set_queue by ovn-controller > >> > >>ovn-controller opens the netdev corresponding to the tunnel > >>interface's > >>status:tunnel_egress_iface value and configures a HTB qdisc on > >>it. Then for > >>each SB port_binding that has queue_id set, it allocates a queue > >>with the > >>qos_parameters of that port. It also frees up unused queues. > >> > >>This patch replaces the older approach of policing > >> > >>Signed-off-by: Babu Shanmugam > I suggest folding in the following changes. Notably, set_queue(0); > was > documented but didn't work because QDISC_MIN_QUEUE_ID was 1. > > This series has no tests. I think that at least the DSCP marking > feature could be tested with the existing OVN testing infrastructure. > Will you work on that? > >>>Ben, > >>>I am trying to write a test case with the following sequence to test > >>>the > >>>dscp marking. > >>>- Add two ports (lp1, lp2) through ovn-nbctl and assign addresses > >>>- Setup the vswitch db with external_ids:iface-id for the logical ports > >>>created above > >>>- Set options:qos_dscp on one logical port (lp1) through ovn-nbctl > >>>- ofproto/trace a simple ip packet originating from lp1, and check if > >>>the > >>>final flow has the DSCP bit set or not > >>> > >>>ofproto/trace is tracing out correctly only when qos_dscp is not set. > >>>When > >>>qos_dscp is set, the controller adds includes an additional action > >>>"0x22->OXM_OF_IP_DSCP[]" in table 16. With this, ofproto/trace says it > >>>drops > >>>the packet after it passes through table 0. > >>> > >>>Do you have any suggestion on how to overcome this? > >>Hmm, that's odd. What's the whole trace output look like? > >It looks like the following > > > >$ ovs-appctl ofproto/trace br-int > >'in_port=1,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01' -generate > >Bridge: br-int > >Flow: > >in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x > > > >Rule: table=0 cookie=0 priority=100,in_port=1 > >OpenFlow > >actions=set_field:0x1->reg13,set_field:0x1->metadata,set_field:0x1->reg14,resubmit(,16) > > > >Resubmitted flow: > > reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x > >Resubmitted regs: reg0=0x0 reg1=0x0 reg2=0x0 reg3=0x0 reg4=0x0 > >reg5=0x0 reg6=0x0 reg7=0x0 reg8=0x0 reg9=0x0 reg10=0x0 reg11=0x0 reg12=0x0 > >reg13=0x1 reg14=0x1 reg15=0x0 > >Resubmitted odp: drop > >Resubmitted megaflow: > > recirc_id=0,reg13=0,reg14=0,metadata=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x > >Rule: table=254 cookie=0 priority=0,reg0=0x2 > >OpenFlow actions=drop > > > >Final flow: > >reg13=0x1,reg14=0x1,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:02,dl_dst=f0:00:00:00:00:01,dl_type=0x > >Megaflow: > >recirc_id=0,in_port=1,vlan_tci=0x/0x1000,dl_src=f0:00:00:00:00:02,dl_type=0x > >Datapath actions: drop > > > > Turns out that it was due to a bug in the DSCP marking code. The DSCP > marking was done at L2 stage without any additional flows to handle the L2 > packets itself. So any L3 packet gets dropped at this stage. > I moved the DSCP marking logic to L3 pipeline with additional tests and > submitted the patches for review. I'm glad you figured it out. I found the trace surprising and had decided to look again later. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev