Skip to content

Commit

Permalink
Skip only OVN DNS responder packets from OUT_ACL.
Browse files Browse the repository at this point in the history
Signed-off-by: Numan Siddique <[email protected]>
  • Loading branch information
numansiddique committed Nov 27, 2024
1 parent 3e177f7 commit b436222
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 2 deletions.
8 changes: 8 additions & 0 deletions controller/chassis.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

/*
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions controller/pinctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
1 change: 1 addition & 0 deletions include/ovn/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions include/ovn/logical-fields.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
3 changes: 3 additions & 0 deletions lib/logical-fields.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions northd/en-global-config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down Expand Up @@ -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;
}
}
}

Expand Down
1 change: 1 addition & 0 deletions northd/en-global-config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
45 changes: 45 additions & 0 deletions tests/ovn-northd.at
Original file line number Diff line number Diff line change
Expand Up @@ -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
])

0 comments on commit b436222

Please sign in to comment.