On Tue, May 26, 2020 at 7:06 PM Dumitru Ceara <[email protected]> wrote:
> On 5/1/20 8:48 PM, [email protected] wrote: > > From: Numan Siddique <[email protected]> > > > > 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 <[email protected]> > > 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. > > Thanks Dumitru for the review. I've a patch almost ready which tunnels the pkt metadata. I need to handle one issue. I'll submit v2 with that patch after fixing that issue. Thanks Numan 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 > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
