[ovs-dev] [PATCH ovn v2] Support packet metadata marking for logical router policies.
Numan Siddique
numans at ovn.org
Thu Jun 18 16:26:00 UTC 2020
On Thu, Jun 18, 2020 at 9:50 PM Alexander Constantinescu <
aconstan at redhat.com> wrote:
> Hi Numan
>
> Thank you for the patch!
>
> I was just wondering, will this work with IPv6 too? I skimmed through the
> patch quickly and didn't see any test cases for IPv6, hence why I am
> asking. FYI: for the upcoming version of Openshift this will be a
> requirement as all networking features will need to support IPv4 and/or
> IPv6.
>
>
Thanks. Good point.
It should work. I'll add test cases for IPv6 too.
Thanks
Numan
> Best regards,
> Alexander
>
> On Thu, Jun 18, 2020 at 11:17 AM <numans at ovn.org> wrote:
>
> > From: Numan Siddique <numans at ovn.org>
> >
> > This patch adds a new column 'options' of type smap in the
> > Logical_Router_Policy table in the NB DB and supports the key 'pkt_mark'.
> > CMS can set a desired value for this key in the 'options' column. When
> this
> > router policy is applied, the packet metadata is marked with the
> specified
> > value (to the NXM_NX_PKT_MARK OVS field).
> >
> > In the case of Linux, this corresponds to struct sk_buff's "skb_mark"
> > member and this mark can be seen by the linux networking subsystem.
> > CMS can inspect this value (as an iptables rule or adding an OF flow
> > in another ovs bridge) and take appropriate action when the marked packet
> > leaves the integration bridge via the patch port.
> >
> > Requested-at: https://bugzilla.redhat.com/show_bug.cgi?id=1828933
> > Requested-by
> > <https://bugzilla.redhat.com/show_bug.cgi?id=1828933Requested-by>:
> > Alexander Constantinescu <aconstan at redhat.com>
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> > v1 -> v2
> > ---
> > * Addressed review comments from Gabriele Cerami.
> > * Documented the field pkt.mark in ovn-sb.xml.
> >
> > NEWS | 2 +
> > lib/logical-fields.c | 2 +
> > northd/ovn-northd.c | 8 ++
> > ovn-nb.ovsschema | 7 +-
> > ovn-nb.xml | 9 ++
> > ovn-sb.xml | 1 +
> > tests/ovn.at | 201 +++++++++++++++++++++++++++++++++++++++++++
> > 7 files changed, 228 insertions(+), 2 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index c6bb9b2fb..893063b01 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> > Post-v20.06.0
> > --------------------------
> > + - Added packet marking support for traffic routed with
> > + a routing policy.
> >
> > OVN v20.06.0
> > --------------------------
> > diff --git a/lib/logical-fields.c b/lib/logical-fields.c
> > index a007085b3..8ad56aa53 100644
> > --- a/lib/logical-fields.c
> > +++ b/lib/logical-fields.c
> > @@ -254,6 +254,8 @@ ovn_init_symtab(struct shash *symtab)
> > expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp",
> > false);
> > expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp",
> > false);
> >
> > + expr_symtab_add_field(symtab, "pkt.mark", MFF_PKT_MARK, NULL,
> false);
> > +
> > expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu",
> > OVN_ICMP4_FRAG_MTU);
> > }
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 6a9b097e5..369b258c0 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -7101,6 +7101,10 @@ build_routing_policy_flow(struct hmap *lflows,
> > struct ovn_datapath *od,
> > rule->priority, rule->nexthop);
> > return;
> > }
> > + uint32_t pkt_mark = smap_get_int(&rule->options, "pkt_mark", 0);
> > + if (pkt_mark) {
> > + ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark);
> > + }
> > bool is_ipv4 = strchr(rule->nexthop, '.') ? true : false;
> > ds_put_format(&actions, "%sreg0 = %s; "
> > "%sreg1 = %s; "
> > @@ -7118,6 +7122,10 @@ build_routing_policy_flow(struct hmap *lflows,
> > struct ovn_datapath *od,
> > } else if (!strcmp(rule->action, "drop")) {
> > ds_put_cstr(&actions, "drop;");
> > } else if (!strcmp(rule->action, "allow")) {
> > + uint32_t pkt_mark = smap_get_int(&rule->options, "pkt_mark", 0);
> > + if (pkt_mark) {
> > + ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark);
> > + }
> > ds_put_cstr(&actions, "next;");
> > }
> > ds_put_format(&match, "%s", rule->match);
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index a06972aa0..da9af7157 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> > {
> > "name": "OVN_Northbound",
> > - "version": "5.23.0",
> > - "cksum": "111023208 25806",
> > + "version": "5.24.0",
> > + "cksum": "1092394564 25961",
> > "tables": {
> > "NB_Global": {
> > "columns": {
> > @@ -379,6 +379,9 @@
> > "key": {"type": "string",
> > "enum": ["set", ["allow", "drop",
> > "reroute"]]}}},
> > "nexthop": {"type": {"key": "string", "min": 0, "max":
> > 1}},
> > + "options": {
> > + "type": {"key": "string", "value": "string",
> > + "min": 0, "max": "unlimited"}},
> > "external_ids": {
> > "type": {"key": "string", "value": "string",
> > "min": 0, "max": "unlimited"}}},
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 8368d5108..21fe5e773 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -2541,6 +2541,15 @@
> > </p>
> > </column>
> >
> > + <column name="options" key="pkt_mark">
> > + <p>
> > + Marks the packet with the value specified when the router policy
> > + is applied. CMS can inspect this packet marker and take some
> > decisions
> > + if desired. This value is not preserved when the packet goes out
> > of the
> > + wire.
> > + </p>
> > + </column>
> > +
> > <group title="Common Columns">
> > <column name="external_ids">
> > See <em>External IDs</em> at the beginning of this document.
> > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > index 208fb936f..00e7e7fc5 100644
> > --- a/ovn-sb.xml
> > +++ b/ovn-sb.xml
> > @@ -975,6 +975,7 @@
> > <li><code>xxreg0</code> <code>xxreg1</code></li>
> > <li><code>inport</code> <code>outport</code></li>
> > <li><code>flags.loopback</code></li>
> > + <li><code>pkt.mark</code></li>
> > <li><code>eth.src</code> <code>eth.dst</code>
> > <code>eth.type</code></li>
> > <li><code>vlan.tci</code> <code>vlan.vid</code>
> > <code>vlan.pcp</code> <code>vlan.present</code></li>
> > <li><code>ip.proto</code> <code>ip.dscp</code>
> > <code>ip.ecn</code> <code>ip.ttl</code> <code>ip.frag</code></li>
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 16b848233..69aeaec87 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -965,6 +965,23 @@ ip.ttl--;
> > ip.ttl
> > Syntax error at end of input expecting `--'.
> >
> > +# 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)
> > @@ -19904,3 +19921,187 @@ 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
> > 10.0.0.5"
> > +ovn-nbctl lsp-set-port-security sw0-port1 "50:54:00:00:00:03 10.0.0.3
> > 10.0.0.5"
> > +
> > +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 router
> > +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 <http://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-sbctl create mac_binding datapath=$lr0_dp_uuid ip=172.168.0.200 \
> > +logical_port=lr0-public mac="10\:54\:00\:00\:00\:04"
> > +
> > +ovn-nbctl --wait=hv sync
> > +
> > +# Add logical router policy and set pkt_mark on it.
> > +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
> > +ovn-nbctl lr-policy-add lr0 900 "ip4.src == 10.0.0.5" reroute
> > 172.168.0.200
> > +
> > +pol1=$(ovn-nbctl --bare --columns _uuid find logical_router_policy
> > priority=2000)
> > +pol3=$(ovn-nbctl --bare --columns _uuid find logical_router_policy
> > priority=900)
> > +
> > +ovn-nbctl set logical_router_policy $pol1 options:pkt_mark=100
> > +ovn-nbctl set logical_router_policy $pol3 options:pkt_mark=5
> > +
> > +OVS_WAIT_UNTIL([
> > + test 1 -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-int table=19 | \
> > + grep "load:0x5->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
> > +table=0, priority=100, pkt_mark=0x5 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,pkt_mark=0x64" | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +AT_CHECK([
> > + 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)
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep priority=0 | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep "priority=100,pkt_mark=0x64" | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep "priority=100,pkt_mark=0x5" | \
> > + grep "n_packets=0" -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)
> > +])
> > +
> > +AT_CHECK([
> > + test 0 -eq $(as hv1 ovs-ofctl dump-flows br-int table=19 | \
> > + grep "load:0x64->NXM_NX_PKT_MARK" -c)
> > +])
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep "priority=100,pkt_mark=0x2" | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep priority=0 | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +AT_CHECK([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep "priority=100,pkt_mark=0x5" | \
> > + grep "n_packets=0" -c)
> > +])
> > +
> > +# Send with src ip 10.0.0.5. The reroute policy should be hit
> > +# and the packet should be marked with 5.
> > +send_ipv4_pkt hv1 hv1-vif1 505400000003 00000000ff01 \
> > + $(ip_to_hex 10 0 0 5) $(ip_to_hex 172 168 0 120)
> > +
> > +ovn-sbctl dump-flows lr0 > /tmp/lr0.f
> > +OVS_WAIT_UNTIL([
> > + test 1 -eq $(as hv1 ovs-ofctl dump-flows br-phys table=0 | \
> > + grep "priority=100,pkt_mark=0x5" | \
> > + grep "n_packets=1" -c)
> > +])
> > +
> > +OVN_CLEANUP([hv1])
> > +AT_CLEANUP
> > --
> > 2.26.2
> >
> >
>
> --
>
> Best regards,
>
>
> Alexander Constantinescu
>
> Software Engineer, Openshift SDN
>
> Red Hat <https://www.redhat.com/>
>
> aconstan at redhat.com
> <https://www.redhat.com/>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list