Re: [ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-10-05 Thread Babu Shanmugam 


On Wednesday 05 October 2016 12:05 AM, Ben Pfaff wrote:

On Wed, Sep 07, 2016 at 11:40:12AM +0530,bscha...@redhat.com  wrote:

>From: Babu Shanmugam
>
>This patch adds support for marking qos on IP packets based on arbitrary
>match criteria for a logical switch.
>
>Signed-off-by: Babu Shanmugam
>Suggested-by: Mickey Spiegel
>Acked-by: Mickey Spiegel

I spent a little time with this, but I couldn't make the test pass.  The
first issue seems to be that MFF_LOG_CT_ZONE (in reg13) is
nondeterministic: sometimes I see 0x1, sometimes 0x2.  That's not a
problem with this patch, so I folded in the appended incremental, which
also makes sure that the logical port numbers get assigned in the
expected order, by adding --wait and sync.  But even with that, I get
the following failure, so i think that some more work is needed here.

Thanks,

Ben.


Ben, I rebased the patch and applied your changes. I could not see any 
failures in any test case. I have published a v14 which is the rebased 
version with your changes. Kindly try it and let me know if you are 
still seeing failures.


Thank you,
Babu
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-10-04 Thread Babu Shanmugam 



On Wednesday 05 October 2016 12:05 AM, Ben Pfaff wrote:

On Wed, Sep 07, 2016 at 11:40:12AM +0530,bscha...@redhat.com  wrote:

>From: Babu Shanmugam
>
>This patch adds support for marking qos on IP packets based on arbitrary
>match criteria for a logical switch.
>
>Signed-off-by: Babu Shanmugam
>Suggested-by: Mickey Spiegel
>Acked-by: Mickey Spiegel

I spent a little time with this, but I couldn't make the test pass.  The
first issue seems to be that MFF_LOG_CT_ZONE (in reg13) is
nondeterministic: sometimes I see 0x1, sometimes 0x2.  That's not a
problem with this patch, so I folded in the appended incremental, which
also makes sure that the logical port numbers get assigned in the
expected order, by adding --wait and sync.  But even with that, I get
the following failure, so i think that some more work is needed here.

Thanks,

Ben.

Thank you for reviewing Ben. I will have a look at the test cases.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-10-04 Thread Ben Pfaff 
On Wed, Sep 07, 2016 at 11:40:12AM +0530, bscha...@redhat.com wrote:
> From: Babu Shanmugam 
> 
> This patch adds support for marking qos on IP packets based on arbitrary
> match criteria for a logical switch.
> 
> Signed-off-by: Babu Shanmugam 
> Suggested-by: Mickey Spiegel 
> Acked-by: Mickey Spiegel 

I spent a little time with this, but I couldn't make the test pass.  The
first issue seems to be that MFF_LOG_CT_ZONE (in reg13) is
nondeterministic: sometimes I see 0x1, sometimes 0x2.  That's not a
problem with this patch, so I folded in the appended incremental, which
also makes sure that the logical port numbers get assigned in the
expected order, by adding --wait and sync.  But even with that, I get
the following failure, so i think that some more work is needed here.

Thanks,

Ben.

../../tests/ovn.at:5408: get_final_flow
--- -   2016-10-04 11:30:59.714524682 -0700
+++ /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2262/stdout   
2016-10-04 11:30:59.712152388 -0700
@@ -1,2 +1,2 @@
-Final flow: 
ip,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=192,nw_ecn=0,nw_ttl=0
+Final flow: 
ip,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0

--8<--cut here-->8--

diff --git a/tests/ovn.at b/tests/ovn.at
index f17ba26..12fe54d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5370,12 +5370,13 @@ ovn_start
 # Configure the Northbound database
 ovn-nbctl ls-add lsw0
 
-ovn-nbctl lsp-add lsw0 lp1
-ovn-nbctl lsp-add lsw0 lp2
+ovn-nbctl --wait=sb lsp-add lsw0 lp1
+ovn-nbctl --wait=sb lsp-add lsw0 lp2
 ovn-nbctl lsp-set-addresses lp1 f0:00:00:00:00:01
 ovn-nbctl lsp-set-addresses lp2 f0:00:00:00:00:02
 ovn-nbctl lsp-set-port-security lp1 f0:00:00:00:00:01
 ovn-nbctl lsp-set-port-security lp2 f0:00:00:00:00:02
+ovn-nbctl --wait=sb sync
 net_add n1
 sim_add hv
 as hv
@@ -5384,37 +5385,43 @@ ovn_attach n1 br-phys 192.168.0.1
 ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1 
options:tx_pcap=vif1-tx.pcap options:rxq_pcap=vif1-rx.pcap ofport-request=1
 ovs-vsctl add-port br-int vif2 -- set Interface vif2 external-ids:iface-id=lp2 
options:tx_pcap=vif2-tx.pcap options:rxq_pcap=vif2-rx.pcap ofport-request=2
 
+# Extracts the final flow from ofproto/trace output,
+# removing the irrelevant MFF_LOG_CT_ZONE (reg13) value.
+get_final_flow() {
+sed -n "/Final flow:/s/reg13=[[^,]]*,//p" stdout
+}
+
 # check at L2
 AT_CHECK([ovs-appctl ofproto/trace br-int 
'in_port=1,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02' -generate], [0], 
[stdout])
-AT_CHECK([grep "Final flow:" stdout], [0],[Final flow: 
reg13=0x2,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,dl_type=0x
+AT_CHECK([get_final_flow], [0],[Final flow: 
reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,dl_type=0x
 ])
 
 # check at L3 without dscp marking
 AT_CHECK([ovs-appctl ofproto/trace br-int 
'in_port=1,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,dl_type=0x800,nw_src=1.1.1.1,nw_dst=1.1.1.2'
 -generate], [0], [stdout])
-AT_CHECK([grep "Final flow:" stdout], [0],[Final flow: 
ip,reg13=0x2,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
+AT_CHECK([get_final_flow], [0],[Final flow: 
ip,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
 ])
 
 # Mark DSCP with a valid value
 qos_id=$(ovn-nbctl -- --id=@lp1-qos create QoS priority=100 action=dscp=48 
match="inport\=\=\"lp1\"" direction="from-lport" -- set Logical_Switch lsw0 
qos_rules=@lp1-qos)
 AT_CHECK([ovs-appctl ofproto/trace br-int 
'in_port=1,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,dl_type=0x800,nw_src=1.1.1.1,nw_dst=1.1.1.2'
 -generate], [0], [stdout])
-AT_CHECK([grep "Final flow:" stdout], [0],[Final flow: 
ip,reg13=0x2,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=192,nw_ecn=0,nw_ttl=0
+AT_CHECK([get_final_flow], [0],[Final flow: 
ip,reg14=0x1,reg15=0x2,metadata=0x1,in_port=1,vlan_tci=0x,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=1.1.1.1,nw_dst=1.1.1.2,nw_proto=0,nw_tos=192,nw_ecn=0,nw_ttl=0
 ])
 
 # Update the DSCP marking
 ovn-nbctl set QoS $qos_id action=dscp=63
 AT_CHECK([ovs-appctl ofproto/trace br-int

[ovs-dev] [PATCH v13 2/2] DSCP marking on packets

2016-09-07 Thread bschanmu 
From: Babu Shanmugam 

This patch adds support for marking qos on IP packets based on arbitrary
match criteria for a logical switch.

Signed-off-by: Babu Shanmugam 
Suggested-by: Mickey Spiegel 
Acked-by: Mickey Spiegel 
---
 ovn/lib/logical-fields.c|  2 +-
 ovn/northd/ovn-northd.8.xml | 47 --
 ovn/northd/ovn-northd.c | 80 ++---
 ovn/ovn-nb.ovsschema| 26 +--
 ovn/ovn-nb.xml  | 57 
 ovn/utilities/ovn-nbctl.c   |  5 +++
 tests/ovn.at| 57 
 7 files changed, 234 insertions(+), 40 deletions(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae..068c000 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -134,7 +134,7 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd");
 expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6");
 expr_symtab_add_field(symtab, "ip.proto", MFF_IP_PROTO, "ip", true);
-expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP, "ip", false);
+expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP_SHIFTED, "ip", false);
 expr_symtab_add_field(symtab, "ip.ecn", MFF_IP_ECN, "ip", false);
 expr_symtab_add_field(symtab, "ip.ttl", MFF_IP_TTL, "ip", false);
 
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 3448370..25b8564 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -362,7 +362,27 @@
   
 
 
-Ingress Table 7: LB
+Ingress Table 7: from-lport QoS marking
+
+
+  Logical flows in this table closely reproduce those in the
+  QoS table in the OVN_Northbound database
+  for the from-lport direction.
+
+
+
+  
+For every qos_rules for every logical switch a flow will be added at
+priorities mentioned in the QoS table.
+  
+
+  
+One priority-0 fallback flow that matches all packets and advances to
+the next table.
+  
+
+
+Ingress Table 8: LB
 
 
   It contains a priority-0 flow that simply moves traffic to the next
@@ -375,7 +395,7 @@
   connection.)
 
 
-Ingress Table 8: Stateful
+Ingress Table 9: Stateful
 
 
   
@@ -412,7 +432,7 @@
   
 
 
-Ingress Table 9: ARP/ND responder
+Ingress Table 10: ARP/ND responder
 
 
   This table implements ARP/ND responder for known IPs.  It contains these
@@ -484,7 +504,7 @@ nd_na {
   
 
 
-Ingress Table 10: DHCP option processing
+Ingress Table 11: DHCP option processing
 
 
   This table adds the DHCPv4 options to a DHCPv4 packet from the
@@ -544,7 +564,7 @@ next;
   
 
 
-Ingress Table 11: DHCP responses
+Ingress Table 12: DHCP responses
 
 
   This table implements DHCP responder for the DHCP replies generated by
@@ -626,7 +646,7 @@ output;
   
 
 
-Ingress Table 12: Destination Lookup
+Ingress Table 13 Destination Lookup
 
 
   This table implements switching behavior.  It contains these logical
@@ -693,7 +713,14 @@ output;
   to-lport ACLs.
 
 
-Egress Table 5: Stateful
+Egress Table 5: to-lport QoS marking
+
+
+  This is similar to ingress table QoS marking except for
+  to-lport qos rules.
+
+
+Egress Table 6: Stateful
 
 
   This is similar to ingress table Stateful except that
@@ -704,10 +731,10 @@ output;
   Also a priority 34000 logical flow is added for each logical port which
   has DHCPv4 options defined to allow the DHCPv4 reply packet and which has
   DHCPv6 options defined to allow the DHCPv6 reply packet from the
-  Ingress Table 11: DHCP responses.
+  Ingress Table 12: DHCP responses.
 
 
-Egress Table 6: Egress Port Security - IP
+Egress Table 7: Egress Port Security - IP
 
 
   This is similar to the port security logic in table
@@ -717,7 +744,7 @@ output;
   ip4.src and ip6.src
 
 
-Egress Table 7: Egress Port Security - L2
+Egress Table 8: Egress Port Security - L2
 
 
   This is similar to the ingress port security logic in ingress table
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 9ce2af9..838783a 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -93,21 +93,22 @@ enum ovn_datapath_type {
  * form the stage's full name, e.g. S_SWITCH_IN_PORT_SEC_L2,
  * S_ROUTER_OUT_DELIVERY. */
 enum ovn_stage {
-#define PIPELINE_STAGES   \
-/* Logical switch ingress stages. */  \
-PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_L2,0, "ls_in_port_sec_l2") \
-PIPELINE_STAGE(SWITCH, IN,  PORT_SEC_IP,1, "ls_in_port_sec_ip") \
-PIPELINE_STAGE(SWITCH,