From b436222457d89c8bfd8cae90b8bbf65b114adbb9 Mon Sep 17 00:00:00 2001 From: Numan Siddique Date: Tue, 26 Nov 2024 18:31:53 -0500 Subject: [PATCH] Skip only OVN DNS responder packets from OUT_ACL. Signed-off-by: Numan Siddique --- controller/chassis.c | 8 +++++++ controller/pinctrl.c | 27 ++++++++++++++++++++++ include/ovn/features.h | 1 + include/ovn/logical-fields.h | 1 + lib/logical-fields.c | 3 +++ northd/en-global-config.c | 10 ++++++++ northd/en-global-config.h | 1 + northd/northd.c | 8 +++++-- tests/ovn-northd.at | 45 ++++++++++++++++++++++++++++++++++++ 9 files changed, 102 insertions(+), 2 deletions(-) diff --git a/controller/chassis.c b/controller/chassis.c index ee839084ae..e4290836d9 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -391,6 +391,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg, smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS, ovs_cfg->sample_with_regs ? "true" : "false"); smap_replace(config, OVN_FEATURE_CT_NEXT_ZONE, "true"); + smap_replace(config, OVN_FEATURE_FROM_CTRL_FLAG, "true"); } /* @@ -556,6 +557,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg, return true; } + if (!smap_get_bool(&chassis_rec->other_config, + OVN_FEATURE_FROM_CTRL_FLAG, + false)) { + return true; + } + return false; } @@ -714,6 +721,7 @@ update_supported_sset(struct sset *supported) sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS); sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE); + sset_add(supported, OVN_FEATURE_FROM_CTRL_FLAG); } static void diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3fb7e2fd7f..659eb8f407 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -389,6 +389,8 @@ static void pinctrl_handle_put_fdb(const struct flow *md, const struct flow *headers) OVS_REQUIRES(pinctrl_mutex); +static void set_from_ctrl_flag_in_pkt_metadata(struct ofputil_packet_in *); + COVERAGE_DEFINE(pinctrl_drop_put_mac_binding); COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map); COVERAGE_DEFINE(pinctrl_drop_controller_event); @@ -3586,6 +3588,10 @@ pinctrl_handle_dns_lookup( union mf_subvalue sv; sv.u8_val = success; mf_write_subfield(&dst, &sv, &pin->flow_metadata); + + /* Indicate that this packet is from ovn-controller. */ + set_from_ctrl_flag_in_pkt_metadata(pin); + } queue_msg(swconn, ofputil_encode_resume(pin, continuation, proto)); dp_packet_uninit(pkt_out_ptr); @@ -8883,3 +8889,24 @@ pinctrl_handle_put_fdb(const struct flow *md, const struct flow *headers) fdb_add(&put_fdbs, fdb_data, timestamp); notify_pinctrl_main(); } + +/* This function sets the register bit 'MLF_FROM_CTRL_BIT' + * in the register 'MFF_LOG_FLAGS' to indicate that this packet + * is generated/sent by ovn-controller. + * ovn-northd can add logical flows to match on "flags.from_ctrl". + */ +static void +set_from_ctrl_flag_in_pkt_metadata(struct ofputil_packet_in *pin) +{ + const struct mf_field *f = mf_from_id(MFF_LOG_FLAGS); + + struct mf_subfield dst = { + .field = f, + .ofs = MLF_FROM_CTRL_BIT, + .n_bits = 1, + }; + + union mf_subvalue sv; + sv.u8_val = 1; + mf_write_subfield(&dst, &sv, &pin->flow_metadata); +} diff --git a/include/ovn/features.h b/include/ovn/features.h index 3566ab60ff..a0151ea52e 100644 --- a/include/ovn/features.h +++ b/include/ovn/features.h @@ -31,6 +31,7 @@ #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" #define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers" #define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone" +#define OVN_FEATURE_FROM_CTRL_FLAG "from-ctrl-flag" /* OVS datapath supported features. Based on availability OVN might generate * different types of openflows. diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index d563e044cb..1ac75f0237 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -86,6 +86,7 @@ enum mff_log_flags_bits { MLF_LOCALNET_BIT = 15, MLF_RX_FROM_TUNNEL_BIT = 16, MLF_ICMP_SNAT_BIT = 17, + MLF_FROM_CTRL_BIT = 18, }; /* MFF_LOG_FLAGS_REG flag assignments */ diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 5a8b53f2b6..f49a0a79db 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -140,6 +140,9 @@ ovn_init_symtab(struct shash *symtab) snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_RX_FROM_TUNNEL_BIT); expr_symtab_add_subfield(symtab, "flags.tunnel_rx", NULL, flags_str); + snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_FROM_CTRL_BIT); + expr_symtab_add_subfield(symtab, "flags.from_ctrl", NULL, flags_str); + /* Connection tracking state. */ expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false, WR_CT_COMMIT); diff --git a/northd/en-global-config.c b/northd/en-global-config.c index fff2aaa169..dbfceac1c3 100644 --- a/northd/en-global-config.c +++ b/northd/en-global-config.c @@ -383,6 +383,7 @@ northd_enable_all_features(struct ed_type_global_config *data) .ct_commit_to_zone = true, .sample_with_reg = true, .ct_next_zone = true, + .from_ctrl_flag = true, }; } @@ -462,6 +463,15 @@ build_chassis_features(const struct sbrec_chassis_table *sbrec_chassis_table, chassis_features->ct_next_zone) { chassis_features->ct_next_zone = false; } + + bool from_ctrl_flag = + smap_get_bool(&chassis->other_config, + OVN_FEATURE_FROM_CTRL_FLAG, + false); + if (!from_ctrl_flag && + chassis_features->from_ctrl_flag) { + chassis_features->from_ctrl_flag = false; + } } } diff --git a/northd/en-global-config.h b/northd/en-global-config.h index 7678105424..4b4f2f9c7f 100644 --- a/northd/en-global-config.h +++ b/northd/en-global-config.h @@ -21,6 +21,7 @@ struct chassis_features { bool ct_commit_to_zone; bool sample_with_reg; bool ct_next_zone; + bool from_ctrl_flag; }; struct global_config_tracked_data { diff --git a/northd/northd.c b/northd/northd.c index 2aa6c0958d..fef0879f70 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -7694,9 +7694,13 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, has_stateful ? REGBIT_ACL_VERDICT_ALLOW" = 1; " "ct_commit; next;" : REGBIT_ACL_VERDICT_ALLOW" = 1; next;"; + + const char *dns_match = + features->from_ctrl_flag ? "flags.from_ctrl && udp.src == 53" + : "udp.src == 53"; ovn_lflow_add( - lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000, "udp.src == 53", - dns_actions, lflow_ref); + lflows, od, S_SWITCH_OUT_ACL_EVAL, 34000, + dns_match, dns_actions, lflow_ref); } if (ls_stateful_rec->has_acls || ls_stateful_rec->has_lb_vip) { diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index e3b7b0cb5f..d8331495d1 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -14234,3 +14234,48 @@ AT_CHECK([grep "lr_in_dnat" lr1flows | ovn_strip_lflows | grep "30.0.0.1"], [0], AT_CLEANUP ]) + + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Check OVN DNS flows for from-ctrl-flag chassis feature flag ]) +ovn_start + +AS_BOX([chassis 1]) +check ovn-sbctl chassis-add hv1 geneve 127.0.0.1 \ + -- set chassis hv1 other_config:from-ctrl-flag=true + +AS_BOX([chassis 2]) +check ovn-sbctl chassis-add hv2 geneve 127.0.0.2 \ + -- set chassis hv2 other_config:from-ctrl-flag=true + +check ovn-nbctl ls-add sw0 +DNS1=$(ovn-nbctl create DNS records={vm1=10.0.0.10}) +check ovn-nbctl set Logical_Switch sw0 dns_records="$DNS1" +check ovn-nbctl --wait=sb sync + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep "ls_out_acl_eval" sw0flows | ovn_strip_lflows | grep "udp.src"], [0], [dnl + table=??(ls_out_acl_eval ), priority=34000, match=(flags.from_ctrl && udp.src == 53), action=(reg8[[16]] = 1; next;) +]) + +check ovn-sbctl set chassis hv2 other_config:from-ctrl-flag=false +check ovn-nbctl --wait=sb sync + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep "ls_out_acl_eval" sw0flows | ovn_strip_lflows | grep "udp.src"], [0], [dnl + table=??(ls_out_acl_eval ), priority=34000, match=(udp.src == 53), action=(reg8[[16]] = 1; next;) +]) + +check ovn-nbctl --wait=sb clear DNS $DNS1 records +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep "ls_out_acl_eval" sw0flows | ovn_strip_lflows | grep "udp.src"], [1], [dnl +]) + +AT_CLEANUP +])