On Tue, Aug 16, 2016 at 3:55 AM, <bscha...@redhat.com> wrote:
> From: Babu Shanmugam <bscha...@redhat.com>
>
> ovn-northd sets 'ip.dscp' to the DSCP value
>
> Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
> ---
> ovn/lib/logical-fields.c | 2 +-
> ovn/northd/ovn-northd.8.xml | 5 ++++
> ovn/northd/ovn-northd.c | 13 ++++++++
> ovn/ovn-nb.xml | 6 ++++
> ovn/ovn-sb.xml | 5 ++++
> tests/ovn.at | 73 ++++++++++++++++++++++++++++++
> +++++++++++++++
> 6 files changed, 103 insertions(+), 1 deletion(-)
>
> 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 1cf6b6e..8f203b3 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -155,6 +155,11 @@
>
> <ul>
> <li>
> + Priority 100 flow to mark DSCP, only when the port has a
> valid DSCP
> + setting
> + </li>
> +
> + <li>
>
By adding a priority 100 flow that matches only on inport, you just
clobbered
the main port security functionality at priority 90 which restricts traffic
to certain
combinations of inport, MAC, and IP addresses. This really should be in a
separate pipeline stage where it does not affect other features.
In the long run, this should be more like ACL, allowing matches on arbitrary
fields (not just inport), but with the action being set DSCP rather than
allow or drop.
> Priority 90 flow to allow IPv4 traffic if it has IPv4
> addresses
> which match the <code>inport</code>, valid
> <code>eth.src</code>
> and valid <code>ip4.src</code> address(es).
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 91e1687..f6f9afe 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1739,6 +1739,7 @@ build_port_security_nd(struct ovn_port *op, struct
> hmap *lflows)
> *
> * - If the port security has IPv4 addresses or IPv6 addresses or both
> * - Priority 80 flow to drop all IPv4 and IPv6 traffic
> + * - Priority 100 flow to set dscp, if DSCP setting is in port options
> */
> static void
> build_port_security_ip(enum ovn_pipeline pipeline, struct ovn_port *op,
> @@ -1748,6 +1749,18 @@ build_port_security_ip(enum ovn_pipeline pipeline,
> struct ovn_port *op,
> enum ovn_stage stage;
> if (pipeline == P_IN) {
> port_direction = "inport";
> + const char *dscp = smap_get(&op->sb->options, "qos_dscp");
> + if (dscp) {
> + struct ds dscp_actions = DS_EMPTY_INITIALIZER;
> + struct ds dscp_match = DS_EMPTY_INITIALIZER;
> +
> + ds_put_format(&dscp_match, "inport == %s", op->json_key);
> + ds_put_format(&dscp_actions, "ip.dscp = %s; next;", dscp);
> + ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_IP, 100,
> + ds_cstr(&dscp_match), ds_cstr(&dscp_actions));
> + ds_destroy(&dscp_match);
> + ds_destroy(&dscp_actions);
> + }
> stage = S_SWITCH_IN_PORT_SEC_IP;
> } else {
> port_direction = "outport";
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 76309f8..b56b304 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -291,6 +291,12 @@
> If set, indicates the maximum burst size for data sent from this
> interface, in bits.
> </column>
> +
> + <column name="options" key="qos_dscp">
> + If set, indicates the DSCP code to be marked on the packets
> ingressing
> + the switch from the VIF interface. Value should be in the range
> of
> + 0 to 63 (inclusive).
> + </column>
> </group>
> </group>
>
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index 0d5ddb4..36fce59 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1879,6 +1879,11 @@ tcp.flags = RST;
> interface, in bits.
> </column>
>
> + <column name="options" key="qos_dscp">
> + If set, indicates the DSCP code to be marked on the packets
> egressing
> + the VIF interface. Value should be in the range of 0 to 63
> (inclusive).
> + </column>
> +
>
I don't see any code that does anything with this value.
Is your intent still to mark DSCP on egress, or only on ingress?
Is this just here because options get copied from NB to SB for nbsp of type
other than router?
Mickey
<column name="options" key="qdisc_queue_id"
> type='{"type": "integer", "minInteger": 1, "maxInteger":
> 61440}'>
> Indicates the queue number on the physical device. This is same
> as the
> diff --git a/tests/ovn.at b/tests/ovn.at
> index a3aebd9..0e935ef 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4813,3 +4813,76 @@ OVS_WAIT_UNTIL([
>
> OVN_CLEANUP([hv1])
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- DSCP marking - check NB to SB DB transfer])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +
> +# Configure the Northbound database
> +ovn-nbctl ls-add lsw0
> +
> +ovn-nbctl lsp-add lsw0 lp1
> +ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:01 1.1.1.1"
> +
> +ovn-nbctl set Logical_Switch_Port lp1 options:mark_dscp=34
> +AT_CHECK([ovn-sbctl get Port_Binding lp1 options:mark_dscp], [0], ["34"
> +])
> +AT_CLEANUP
> +
> +AT_SETUP([ovn -- DSCP marking check])
> +AT_KEYWORDS([ovn])
> +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 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
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl add-br br-phys
> +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
> +
> +# 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=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:
> 02,dl_type=0x0000
> +])
> +
> +# 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=0x0000,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
> +ovn-nbctl set Logical_Switch_Port lp1 options:qos_dscp=48
> +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=0x0000,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 Logical_Switch_Port lp1 options:qos_dscp=63
> +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=0x0000,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=
> 252,nw_ecn=0,nw_ttl=0
> +])
> +
> +# Mark DSCP with invalid value
> +ovn-nbctl set Logical_Switch_Port lp1 options:qos_dscp=64
> +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=0x0000,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
> +])
> +
> +# Disable DSCP marking
> +ovn-nbctl clear Logical_Switch_Port lp1 options
> +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=0x0000,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
> +])
> +
> +OVN_CLEANUP([hv])
> +AT_CLEANUP
> --
> 2.5.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
