Hi,
I added some note for my better understanding, but I have some questions
at the end on the testing part.
Sorry if they seem irrelevant or too picky, still learning.
On 18 Jun, num...@ovn.org wrote:
> + expr_symtab_add_field(symtab, "pkt.mark", MFF_PKT_MARK, NULL, false);
> +
Note to self: this part maps the mnemonic string with the meta flow
field in OVS as described in include/openvswitch/meta-flow.h .
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
Note to self: the pkt.mark is enabled for both allow and reroute
policies
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
Note to self: even if a value is interpreted as integer in the schema, all
values
are saved as strings (HEX compatibility ?)
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 9acb48ae5..f31ab9bbf 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -965,6 +965,22 @@ ip.ttl--;
> ip.ttl
> Syntax error at end of input expecting `--'.
Missing a section index here ? # Packet mark
> +pkt.mark=1;
> + formats as pkt.mark = 1;
> + encodes as set_field:0x1->pkt_mark
> +
> +pkt.mark = 1000;
> + encodes as set_field:0x3e8->pkt_mark
> +
> +pkt.mark;
> + Syntax error at `pkt.mark' expecting action.
> +
> +pkt.mark = foo;
> + Syntax error at `foo' expecting field name.
> +
> +pkt.mark = "foo";
> + Integer field pkt.mark is not compatible with string constant.
> +
> # load balancing.
> ct_lb;
> encodes as ct(table=19,zone=NXM_NX_REG13[0..15],nat)
> @@ -19898,3 +19914,154 @@ OVS_WAIT_UNTIL([test 0 = $(ovn-sbctl show | grep
> Port_Binding -c)], [0])
>
> OVN_CLEANUP([hv1])
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- Logical router policy packet marking])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=sw0-port1 \
> + options:tx_pcap=hv1/vif1-tx.pcap \
> + options:rxq_pcap=hv1/vif1-rx.pcap \
> + ofport-request=1
> +ovs-vsctl -- add-port br-int hv1-vif2 -- \
> + set interface hv1-vif2 external-ids:iface-id=sw0-port2 \
> + options:tx_pcap=hv1/vif2-tx.pcap \
> + options:rxq_pcap=hv1/vif2-rx.pcap \
> + ofport-request=2
> +
> +as hv1 ovs-vsctl set Open_vSwitch .
> external-ids:ovn-bridge-mappings=public:br-phys
> +
> +ovn-nbctl ls-add sw0
> +ovn-nbctl lsp-add sw0 sw0-port1
> +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:03 10.0.0.3"
> +ovn-nbctl lsp-set-port-security sw0-port1 "50:54:00:00:00:03 10.0.0.3"
really nitpicking here .... 50:54:00 is a range reserved to cisco ....
local test should use a locally administered range :P
More seriously, is there a convention for the usage of differnt OUIs ?,
I see a mix of cisco, dataindustrier, not sure if different ranges have
different meanings in the tests.
> +
> +ovn-nbctl lsp-add sw0 sw0-port2
> +ovn-nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:04 10.0.0.4"
> +
> +ovn-nbctl lr-add lr0
> +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> +ovn-nbctl lsp-add sw0 sw0-lr0
> +ovn-nbctl lsp-set-type sw0-lr0 router
> +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
You are creating a router port with a mac, then creating a switch port
ion sw0 to connect to it, with the same mac.
Is there a reason for using the same mac ?
Maybe it doesn't really matter in OVS implementation, and the router
will probably never send a packet to the switch itself, but I would
imagine problems when a host sends a packet for the router and the
switch assumes it's for itself and doesn't forward it to other ports.
> +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
> +
> +ovn-nbctl ls-add public
> +ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
> +ovn-nbctl lsp-add public public-lr0
> +ovn-nbctl lsp-set-type public-lr0 router
> +ovn-nbctl lsp-set-addresses public-lr0 router
> +ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
> +
> +# localnet port
> +ovn-nbctl lsp-add public ln-public
> +ovn-nbctl lsp-set-type ln-public localnet
> +ovn-nbctl lsp-set-addresses ln-public unknown
> +ovn-nbctl lsp-set-options ln-public network_name=public
> +
> +ovn-nbctl lrp-set-gateway-chassis lr0-public hv1 20
> +ovn-nbctl lr-nat-add lr0 snat 172.168.0.100 10.0.0.0/24
> +lr0_dp_uuid=$(ovn-sbctl --bare --columns _uuid list datapath_binding lr0)
> +ovn-sbctl create mac_binding datapath=$lr0_dp_uuid ip=172.168.0.120 \
> +logical_port=lr0-public mac="10\:54\:00\:00\:00\:03"
> +
> +ovn-nbctl --wait=hv sync
> +
> +# Add logical router polcy and set pkt_mark on it.
nit: policy
> +ovn-nbctl lr-policy-add lr0 2000 "ip4.src == 10.0.0.3" allow
> +ovn-nbctl lr-policy-add lr0 1000 "ip4.src == 10.0.0.4" allow
> +
> +pol1=$(ovn-nbctl --bare --columns _uuid find logical_router_policy
> priority=2000)
> +pol2=$(ovn-nbctl --bare --columns _uuid find logical_router_policy
> priority=1000)
I don't see pol2 used anywhere in the following steps. 10.0.0.4 sources
(to which pol2 would be attached to ) are used the verify the absence of
marks. And a different mark replaces the first in the same policy pol1
in later tests.
> +ovn-nbctl set logical_router_policy $pol1 options:pkt_mark=100
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-int table=19 | \
> + grep "load:0x64->NXM_NX_PKT_MARK" -c)
> +])
> +
> +AT_DATA([flows.txt], [dnl
> +table=0, priority=0 actions=NORMAL
> +table=0, priority=100, pkt_mark=0x64 actions=drop
> +table=0, priority=100, pkt_mark=0x2 actions=drop
> +])
> +
> +AT_CHECK([as hv1 ovs-ofctl --protocols=OpenFlow13 add-flows br-phys
> flows.txt])
> +
> +ip_to_hex() {
> + printf "%02x%02x%02x%02x" "$@"
> +}
> +
> +send_ipv4_pkt() {
> + local hv=$1 inport=$2 eth_src=$3 eth_dst=$4
> + local ip_src=$5 ip_dst=$6
> +
> packet=${eth_dst}${eth_src}08004500001c0000000040110000${ip_src}${ip_dst}0035111100080000
> + as $hv ovs-appctl netdev-dummy/receive ${inport} ${packet}
> +}
> +
> +send_ipv4_pkt hv1 hv1-vif1 505400000003 00000000ff01 \
> + $(ip_to_hex 10 0 0 3) $(ip_to_hex 172 168 0 120)
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=100 | grep "priority=100,pkt_mark=0x64" | \
The regex on the first grep is contained in the regex for the second.
Do you need both for some reason ?
> + grep "n_packets=1" -c)
> +])
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=0 | \
> + grep "n_packets=0" -c)
> +])
> +
> +# Send the pkt from sw0-port2. Packet should not be marked.
> +send_ipv4_pkt hv1 hv1-vif2 505400000004 00000000ff01 \
> + $(ip_to_hex 10 0 0 4) $(ip_to_hex 172 168 0 120)
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=0 | \
> + grep "n_packets=1" -c)
> +])
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=100 | grep "priority=100,pkt_mark=0x64" | \
Same for the grep here.
> + grep "n_packets=1" -c)
> +])
> +
> +
> +ovn-nbctl set logical_router_policy $pol1 options:pkt_mark=2
> +send_ipv4_pkt hv1 hv1-vif1 505400000003 00000000ff01 \
> + $(ip_to_hex 10 0 0 3) $(ip_to_hex 172 168 0 120)
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-int table=19 | \
> + grep "load:0x2->NXM_NX_PKT_MARK" -c)
> +])
> +
> +OVS_WAIT_UNTIL([
> + test 0 -eq $(as hv1 ovs-ofctl dump-flows br-int table=19 | \
> + grep "load:0x64->NXM_NX_PKT_MARK" -c)
> +])
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=100 | grep "priority=100,pkt_mark=0x2" | \
Same for the grep here.
> + grep "n_packets=1" -c)
> +])
> +
> +OVS_WAIT_UNTIL([
> + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> + grep priority=0 | \
> + grep "n_packets=1" -c)
> +])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
Is there any need to add a test for the reroute policy case ?
Or a test for seeing the pkt.mark option not present after removing the
policy/options:pkt_mark ?
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
