[ovs-dev] [PATCH v8 2/2] DSCP marking on packets egressing VIF interface
Babu Shanmugam
bschanmu at redhat.com
Wed Aug 17 04:38:02 UTC 2016
Thank Mickey for your review. My comments are inlined.
On Tuesday 16 August 2016 09:59 PM, Mickey Spiegel wrote:
> On Tue, Aug 16, 2016 at 3:55 AM, <bschanmu at redhat.com
> <mailto:bschanmu at redhat.com>> wrote:
>
> From: Babu Shanmugam <bschanmu at redhat.com
> <mailto:bschanmu at redhat.com>>
>
> ovn-northd sets 'ip.dscp' to the DSCP value
>
> Signed-off-by: Babu Shanmugam <bschanmu at redhat.com
> <mailto:bschanmu at 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 <http://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.
>
DSCP marking is done at a different table. Priority 90 flows will be in
the next table. I agree my documentation is not proper. I will correct it.
> 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?
>
You are right. DSCP marking is done through the logical flow ('ip.dscp'
set action). qos_dscp option is just copied from NB to SB like all the
other options for a VIF port.
> 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 <http://ovn.at> b/tests/ovn.at
> <http://ovn.at>
> index a3aebd9..0e935ef 100644
> --- a/tests/ovn.at <http://ovn.at>
> +++ b/tests/ovn.at <http://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 at openvswitch.org <mailto:dev at openvswitch.org>
> http://openvswitch.org/mailman/listinfo/dev
> <http://openvswitch.org/mailman/listinfo/dev>
>
>
More information about the dev
mailing list