[ovs-dev] [PATCH ovn] Support packet metadata marking for logical port traffic.

Dumitru Ceara dceara at redhat.com
Tue May 26 13:35:14 UTC 2020


On 5/1/20 8:48 PM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> This patch adds a new column 'mark' in the Logical_Switch_Port
> table in the NB DB. CMS can set a desired value in this column
> and whenever a packet is received from the VIF of the logical port,
> the NXM_NX_PKT_MARK field is set with the value specified by CMS.
> 
> 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.
> 
> Signed-off-by: Numan Siddique <numans at ovn.org>

Hi Numan,

This looks good to me but the fact that it's limited to the local
chassis seems a bit restrictive and would create confusion because users
might expect the mark to be set when the packet leaves OVN (even if the
packet was tunelled).

Furthermore, maintaining the mark across tunnels would make OVN more
flexible as we could allow users to create ACLs that match on the pkt
mark in both ingress/egress pipelines.

This would also probably avoid the need for src_port_group_id [0][1] for
ovn-k8s.

Regards,
Dumitru

[0]
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049932.html
[1]
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049888.html

> ---
>  NEWS                  |  2 +
>  controller/physical.c |  5 +++
>  northd/ovn-northd.c   |  3 ++
>  ovn-nb.ovsschema      |  9 ++++-
>  ovn-nb.xml            |  9 +++++
>  ovn-sb.ovsschema      |  9 ++++-
>  ovn-sb.xml            |  9 +++++
>  tests/ovn.at          | 94 +++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 136 insertions(+), 4 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 765b79f1c..d2b33b7bc 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -13,6 +13,8 @@ OVN v20.03.0 - xx xxx xxxx
>     - Added support for MLD Snooping and MLD Querier.
>     - Added support for ECMP routes in OVN router.
>     - Added IPv6 Prefix Delegation support in OVN.
> +   - Added packet marking support for traffic originating from
> +     the logical port VIFs.
>  
>     - OVN Interconnection:
>       * Support for L3 interconnection of multiple OVN deployments with tunnels
> diff --git a/controller/physical.c b/controller/physical.c
> index 144aeb7bd..24d431a75 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1131,6 +1131,11 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
>  
>          load_logical_ingress_metadata(binding, &zone_ids, ofpacts_p);
>  
> +        /* If Mark is set, set the pkt_mark. */
> +        if (binding->n_mark) {
> +            put_load(binding->mark[0], MFF_PKT_MARK, 0, 32, ofpacts_p);
> +        }
> +
>          /* Resubmit to first logical ingress pipeline table. */
>          put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
>          ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e31794c4b..b049c47ea 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3057,6 +3057,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>          }
>          sbrec_port_binding_set_external_ids(op->sb, &ids);
>          smap_destroy(&ids);
> +
> +        sbrec_port_binding_set_mark(op->sb, op->nbsp->mark, op->nbsp->n_mark);
>      }
>      int64_t tnl_key = op_get_requested_tnl_key(op);
>      if (tnl_key && tnl_key != op->sb->tunnel_key) {
> @@ -11841,6 +11843,7 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
> +    add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mark);
>      add_column_noalert(ovnsb_idl_loop.idl,
>                         &sbrec_port_binding_col_nat_addresses);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index b2a046571..72cc5c5fe 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Northbound",
> -    "version": "5.22.0",
> -    "cksum": "384164739 25476",
> +    "version": "5.23.0",
> +    "cksum": "3915944754 25731",
>      "tables": {
>          "NB_Global": {
>              "columns": {
> @@ -114,6 +114,11 @@
>                                       "refType": "strong"},
>                               "min": 0,
>                               "max": 1}},
> +                "mark": {
> +                     "type": {"key": {"type": "integer",
> +                                      "minInteger": 1,
> +                                      "maxInteger": 4294967295},
> +                              "min": 0, "max": 1}},
>                  "external_ids": {
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}}},
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index af15c550a..d2a4a0db3 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1254,6 +1254,15 @@
>        column is ignored.
>      </column>
>  
> +    <column name="mark">
> +      <p>
> +        The packet received from the VIF of the logical switch port is
> +        marked with the value defined here. CMS can inspect this packet
> +        metadata marker and take some decisions if desired. This value
> +        is not preserved when the packet goes out of the wire.
> +      </p>
> +    </column>
> +
>      <group title="Naming">
>        <column name="external_ids" key="neutron:port_name">
>          <p>
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index d89f8dbbb..ed4dbd3a8 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
> -    "version": "2.7.0",
> -    "cksum": "4286723485 21693",
> +    "version": "2.8.0",
> +    "cksum": "3650018191 21948",
>      "tables": {
>          "SB_Global": {
>              "columns": {
> @@ -189,6 +189,11 @@
>                  "nat_addresses": {"type": {"key": "string",
>                                             "min": 0,
>                                             "max": "unlimited"}},
> +                "mark": {
> +                     "type": {"key": {"type": "integer",
> +                                      "minInteger": 1,
> +                                      "maxInteger": 4294967295},
> +                              "min": 0, "max": 1}},
>                  "external_ids": {"type": {"key": "string",
>                                   "value": "string",
>                                   "min": 0,
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 3aa7cd4da..1b79b853e 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2582,6 +2582,15 @@ tcp.flags = RST;
>          </p>
>        </column>
>  
> +      <column name="mark">
> +        <p>
> +          The packet received from the VIF of the logical switch port is
> +          marked with the value defined here. CMS can inspect this packet
> +          metadata marker and take some decisions if desired. This value
> +          is not preserved when the packet goes out of the wire.
> +        </p>
> +      </column>
> +
>        <column name="mac">
>          <p>
>            The Ethernet address or addresses used as a source address on the
> diff --git a/tests/ovn.at b/tests/ovn.at
> index c04cd06d2..a3f9f4706 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -19147,3 +19147,97 @@ OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected])
>  
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- 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
> +
> +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"
> +
> +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
> +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
> +
> +AT_CHECK([
> +    test 0 -eq $(as hv1 ovs-ofctl dump-flows br-int table=0 | \
> +    grep in_port=1 | grep NXM_NX_PKT_MARK -c)])
> +
> +# Set mark on sw0-port1
> +ovn-nbctl set logical_switch_port sw0-port1 mark=100
> +
> +OVS_WAIT_UNTIL([
> +    test 1 -eq $(as hv1 ovs-ofctl dump-flows br-int table=0 | \
> +    grep in_port=1 | 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
> +])
> +
> +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" | \
> +    grep "n_packets=1" -c)
> +])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> 



More information about the dev mailing list