Consider the case of stateful Firewall for N-S traffic: PUBLIC---S1-(S1-R1)---------(R1-S1)-R1 -------- S2 ---- VM1
Configuration: ovn-nbctl pg-add pg_dgw ovn-nbctl pg-set-ports pg_dgw S1-R1 ovn-nbctl acl-add pg_dgw from-lport 2000 "inport == @pg_dgw && ip4 && icmp4" allow-related ovn-nbctl acl-add pg_dgw from-lport 1000 "inport == @pg_dgw && ip4" drop ovn-nbctl acl-add pg_dgw to-lport 1000 "outport == @pg_dgw && ip4" drop ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1 enable_router_port_acl=true VM1 pings external network. Through this patch[1], the ovn-controller assigned a CT zone ID to the localnet LSP but not the dgw LSP. This caused ACL failures: ICMP reply packets from external networks performed CT lookups in the wrong zone, couldn't match established connections, and were incorrectly dropped. This commit enables CT zone allocation for patch ports that correspond to router gateway ports when enable_router_port_acl=true is set. Changes: - northd: Add enable-router-port-acl option to southbound port binding - binding: Handle patch port CT zone requirements in local_lports - controller: Add/Delete CT zone for patch ports enabled/disabled ACL [1]https://github.com/ovn-org/ovn/commit/5ae7d2cb60a50541e88e8f5c74a669e2aa7acdda Reported-at: https://github.com/ovn-org/ovn/issues/264 Signed-off-by: Xie Liu <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> (cherry picked from commit 4ff608e9179ed85dd7031e820be7d03d479ef6b2) --- controller/binding.c | 29 ++++++++ controller/ovn-controller.c | 24 ++++--- northd/northd.c | 5 ++ tests/system-ovn.at | 138 ++++++++++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 10 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 674c00c3c..a6b142353 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -783,6 +783,16 @@ update_active_pb_ras_pd(const struct sbrec_port_binding *pb, } } +/* Check the patch port needs CT zone based on enable_router_port_acl. + * Returns true if the patch port should be added to local_lports for + * CT zone allocation. + */ +static bool +patch_port_needs_ct_zone(const struct sbrec_port_binding *pb) +{ + return smap_get_bool(&pb->options, "enable_router_port_acl", false); +} + static bool is_ext_id_changed(const struct smap *a, const struct smap *b, const char *key); static struct local_binding *local_binding_create( @@ -2233,6 +2243,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) case LP_PATCH: update_related_lport(pb, b_ctx_out); consider_patch_port_for_local_datapaths(pb, b_ctx_in, b_ctx_out); + /* Check if this patch port needs CT zone allocation and add it + * to local_lports if required. */ + if (patch_port_needs_ct_zone(pb)) { + update_local_lports(pb->logical_port, b_ctx_out); + } break; case LP_VTEP: @@ -3139,6 +3154,20 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in, update_related_lport(pb, b_ctx_out); handled = consider_patch_port_for_local_datapaths(pb, b_ctx_in, b_ctx_out); + /* Handle dynamic changes to enable_router_port_acl option. + * Check CT zone requirement if options updated. */ + if (sbrec_port_binding_is_updated(pb, + SBREC_PORT_BINDING_COL_OPTIONS)) { + tracked_datapath_lport_add(pb, TRACKED_RESOURCE_UPDATED, + b_ctx_out->tracked_dp_bindings); + if (patch_port_needs_ct_zone(pb)) { + update_local_lports(pb->logical_port, b_ctx_out); + } else { + /* Option was removed, remove from local_lports to + * release CT zone */ + remove_local_lports(pb->logical_port, b_ctx_out); + } + } break; case LP_VTEP: diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 4904ddb73..1b4bac138 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2434,20 +2434,24 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) if (strcmp(t_lport->pb->type, "") && strcmp(t_lport->pb->type, "localport") && strcmp(t_lport->pb->type, "l3gateway") - && strcmp(t_lport->pb->type, "localnet")) { + && strcmp(t_lport->pb->type, "localnet") + && strcmp(t_lport->pb->type, "patch")) { /* We allocate zone-id's only to VIF, localport, l3gateway, - * and localnet lports. */ - if (sbrec_port_binding_is_updated(t_lport->pb, - SBREC_PORT_BINDING_COL_TYPE)) { - updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, - t_lport->pb, - false, &scan_start, - min_ct_zone, max_ct_zone); - } - + * localnet and patch (enabled ACL) lports. */ continue; } + /* For patch ports without ACL flag, delete the ct_zone. */ + if (!strcmp(t_lport->pb->type, "patch") && + !smap_get_bool(&t_lport->pb->options, + "enable_router_port_acl", false)) { + updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, + t_lport->pb, + false, &scan_start, + min_ct_zone, + max_ct_zone); + continue; + } bool port_updated = t_lport->tracked_type == TRACKED_RESOURCE_NEW || t_lport->tracked_type == TRACKED_RESOURCE_UPDATED; diff --git a/northd/northd.c b/northd/northd.c index cb41a8132..320400c0e 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3464,6 +3464,11 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn, } else { smap_add(&new, "peer", router_port); } + /* Add enable_router_port_acl option for CT zone alloc + * to patch ports that need ACL processing. */ + if (op->enable_router_port_acl) { + smap_add(&new, "enable_router_port_acl", "true"); + } } if (chassis) { smap_add(&new, "l3gateway-chassis", chassis); diff --git a/tests/system-ovn.at b/tests/system-ovn.at index c5ffc088b..c7c106084 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -17408,3 +17408,141 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d /.*terminating with signal 15.*/d"]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([Distributed gw port CT zone allocation]) +AT_SKIP_IF([test $HAVE_NC = no]) +CHECK_CONNTRACK() + +ovn_start + +# Topo: vm1 -- ls -- lr -- ext (with ACLs on ext_lr) + +OVS_TRAFFIC_VSWITCHD_START() +ADD_BR([br-int]) +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone]) + +# Set external-ids in br-int needed for ovn-controller. +check ovs-vsctl \ + -- set Open_vSwitch . external-ids:system-id=hv1 \ + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true \ + -- set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext + +start_daemon ovn-controller + +# Create logical switches: internal and external networks. +check ovn-nbctl ls-add ls +check ovn-nbctl ls-add ext + +# Add localnet port to external switch for physical connectivity. +check ovn-nbctl lsp-add-localnet-port ext ln_port phynet + +# Add VM port to internal switch. +check ovn-nbctl lsp-add ls vm1 \ + -- lsp-set-addresses vm1 "00:00:00:00:00:11 192.168.100.11" + +# Create logical_router_ports: lr_ls and lr_ext. +check ovn-nbctl lr-add lr +check ovn-nbctl lrp-add lr lr_ls 00:00:00:00:01:01 192.168.100.1/24 +check ovn-nbctl lrp-add lr lr_ext 00:00:00:00:01:02 172.16.0.1/24 +check ovn-nbctl lrp-set-gateway-chassis lr_ext hv1 + +# Create peer logical_switch_ports: ls_lr and ext_lr. +check ovn-nbctl lsp-add ls ls_lr \ + -- lsp-set-type ls_lr router \ + -- lsp-set-addresses ls_lr router \ + -- lsp-set-options ls_lr router-port=lr_ls + +check ovn-nbctl lsp-add ext ext_lr \ + -- lsp-set-type ext_lr router \ + -- lsp-set-addresses ext_lr router \ + -- lsp-set-options ext_lr router-port=lr_ext enable_router_port_acl=true + +# Create port group for external gateway ACLs. +check ovn-nbctl pg-add gw_pg ext_lr + +# Add ACL rule to enable connection tracking for external gateway traffic +# allowing ICMP from vm1 to external. +check ovn-nbctl acl-add gw_pg to-lport 2001 "inport == @gw_pg && ip4 && icmp4" allow-related +check ovn-nbctl acl-add gw_pg to-lport 1001 "inport == @gw_pg && ip4" drop +check ovn-nbctl acl-add gw_pg to-lport 1001 "outport == @gw_pg && ip4" drop + +# Create namespace vm1. +ADD_NAMESPACES(vm1) +ADD_VETH(vm1, vm1, br-int, "192.168.100.11/24", "00:00:00:00:00:11", \ + "192.168.100.1") + +# Create namespace external. +ADD_NAMESPACES(external) +ADD_VETH(external, external, br-ext, "172.16.0.100/24", "00:00:00:00:01:10", \ + "172.16.0.1") + +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +# Check that ext_lr has enable_router_port_acl option. +AT_CHECK([ovn-sbctl get Port_Binding ext_lr options:enable_router_port_acl], [0], [dnl +"true" +]) + +# Check that ls_lr does not have enable_router_port_acl option. +AT_CHECK([ovn-sbctl --columns options list Port_Binding ls_lr | grep -q enable_router_port_acl], [1], []) + +# Check allocated CT zone for ext_lr. +AT_CHECK([ovn-appctl -t ovn-controller ct-zone-list | sed "s/ [[0-9]]*/ ??/"], [0], [stdout]) +AT_CHECK([grep "ext_lr" stdout], [0], [dnl +ext_lr ?? +]) + +# Check no CT zone for ls_lr. +AT_CHECK([grep "ls_lr" stdout], [1], []) + +# Generate ICMP traffic to create conntrack entries. +NS_CHECK_EXEC([vm1], [ping -q -c 2 -W 1 172.16.0.100 | FORMAT_PING], \ +[0], [dnl +2 packets transmitted, 2 received, 0% packet loss, time 0ms +]) + +# Check that ICMP conntrack entries exist. +zone_id=$(ovn-appctl -t ovn-controller ct-zone-list | grep ext_lr | cut -d ' ' -f2) +AT_CHECK_UNQUOTED([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.0.100) | grep "zone=$zone_id"], [0], [dnl +icmp,orig=(src=192.168.100.11,dst=172.16.0.100,id=<cleared>,type=8,code=0),reply=(src=172.16.0.100,dst=192.168.100.11,id=<cleared>,type=0,code=0),zone=$zone_id +]) + +# Test TCP traffic - should be blocked by ACLs. +NETNS_DAEMONIZE([external], [nc -l -k 172.16.0.100 80], [external.pid]) +NS_CHECK_EXEC([vm1], [nc -z 172.16.0.100 80], [1]) + +# Remove enable_router_port_acl and verify CT zone release. +check ovn-nbctl lsp-set-options ext_lr router-port=lr_ext +check ovn-nbctl --wait=sb sync +AT_CHECK([ovn-appctl -t ovn-controller ct-zone-list | grep "ext_lr"], [1], []) + +# Re-enable enable_router_port_acl and verify CT zone reallocation. +check ovn-nbctl lsp-set-options ext_lr router-port=lr_ext enable_router_port_acl=true +check ovn-nbctl --wait=sb sync +AT_CHECK([ovn-appctl -t ovn-controller ct-zone-list | sed "s/ [[0-9]]*/ ??/"], [0], [stdout]) +AT_CHECK([grep "ext_lr" stdout], [0], [dnl +ext_lr ?? +]) + +# Test ICMP traffic still works after dynamic ACL changes +NS_CHECK_EXEC([vm1], [ping -q -c 2 -W 1 172.16.0.100 | FORMAT_PING], \ +[0], [dnl +2 packets transmitted, 2 received, 0% packet loss, time 0ms +]) + +OVS_APP_EXIT_AND_WAIT([ovn-controller]) + +OVN_CLEANUP_NORTHD + +as +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d +/connection dropped.*/d"]) + +AT_CLEANUP +]) +]) -- 2.27.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
