On 11/23/25 12:11 PM, Alexandra Rukomoinikova wrote:
> From: Aleksandr Smirnov <[email protected]>
>
> The northd creates flows that restrict processing ARP requests:
> If ARP request comes from lswitch that has either localnet or
> l2gateway ports and router's attachment port is L3 dgw port such
> ARP will be allowed only on resident chassis.
>
> However, same lswitch could also have 'normal' aka virtual lports.
> For these lports restrictions described above should not be applied.
>
> The fix moves is_chassis_resident check from lrouter to lswitch level.
> The residence check is only applied for ARP requests that were
> originated from localnet/l2gateway ports. If such check is succeeded
> ARP request is allowed otherwise ARP request packet is dropped.
> For ARP requests coming from virtual ports no residence restrictions
> are applied.
>
> Co-authored-by: Alexandra Rukomoinikova <[email protected]>
> Signed-off-by: Aleksandr Smirnov <[email protected]>
> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
> ---
> v4 --> v5:
> 1) Addressed Dumitru's comments:
> - Eliminated O(n) search by implementing uuid based lookup within ls_arp
> nodes
> - Retained ls_index in ls_arp_record structure for exclusive use in
> northd.c - It seems that this is the best option for searching for a datapass
>
> 2) Optimized ls_arp_record creation:
> - Previous approach: Iterated through all NATs for each ls_arp_record
> - New approach: Iterate through logical switch router ports to get
> required NAT records - i think it should reduce number of iterations
>
> 3) Restricted ls_arp_record creation scope: Records now created only for
> logical switches with physical ports - there shouldn't be many of them
>
> 4) Added ovn-northd documentation and ovn-northd tests for lflow functionality
Hi Alexandra, Aleksandr,
Thanks for the new revision!
> ---
> lib/stopwatch-names.h | 1 +
> northd/automake.mk | 2 +
> northd/en-lflow.c | 29 ++++
> northd/en-lflow.h | 2 +
> northd/en-ls-arp.c | 355 +++++++++++++++++++++++++++++++++++++++
> northd/en-ls-arp.h | 86 ++++++++++
> northd/inc-proc-northd.c | 8 +
> northd/northd.c | 191 +++++++++++++++++++--
> northd/northd.h | 10 ++
> northd/ovn-northd.8.xml | 16 ++
> northd/ovn-northd.c | 1 +
> tests/ovn-northd.at | 79 ++++++++-
> tests/ovn.at | 153 +++++++++++++++++
> 13 files changed, 918 insertions(+), 15 deletions(-)
> create mode 100644 northd/en-ls-arp.c
> create mode 100644 northd/en-ls-arp.h
>
> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> index f3a931c40..b912e813c 100644
> --- a/lib/stopwatch-names.h
> +++ b/lib/stopwatch-names.h
> @@ -34,6 +34,7 @@
> #define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
> #define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
> #define LS_STATEFUL_RUN_STOPWATCH_NAME "ls_stateful"
> +#define LS_ARP_RUN_STOPWATCH_NAME "ls_arp"
> #define ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME "advertised_route_sync"
> #define LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME "learned_route_sync"
> #define DYNAMIC_ROUTES_RUN_STOPWATCH_NAME "dynamic_routes"
> diff --git a/northd/automake.mk b/northd/automake.mk
> index 45ca0337f..8cd4fb3a1 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -44,6 +44,8 @@ northd_ovn_northd_SOURCES = \
> northd/en-lr-stateful.h \
> northd/en-ls-stateful.c \
> northd/en-ls-stateful.h \
> + northd/en-ls-arp.c \
> + northd/en-ls-arp.h \
> northd/en-sampling-app.c \
> northd/en-sampling-app.h \
> northd/en-acl-ids.c \
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 30bf7e35c..21a244a1b 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -23,6 +23,7 @@
> #include "en-lr-nat.h"
> #include "en-lr-stateful.h"
> #include "en-ls-stateful.h"
> +#include "en-ls-arp.h"
> #include "en-multicast.h"
> #include "en-northd.h"
> #include "en-meters.h"
> @@ -62,6 +63,8 @@ lflow_get_input_data(struct engine_node *node,
> engine_get_input_data("lr_stateful", node);
> struct ed_type_ls_stateful *ls_stateful_data =
> engine_get_input_data("ls_stateful", node);
> + struct ed_type_ls_arp *ls_arp_data =
> + engine_get_input_data("ls_arp", node);
> struct multicast_igmp_data *multicat_igmp_data =
> engine_get_input_data("multicast_igmp", node);
> struct ic_learned_svc_monitors_data *ic_learned_svc_monitors_data =
> @@ -86,6 +89,7 @@ lflow_get_input_data(struct engine_node *node,
> lflow_input->ls_port_groups = &pg_data->ls_port_groups;
> lflow_input->lr_stateful_table = &lr_stateful_data->table;
> lflow_input->ls_stateful_table = &ls_stateful_data->table;
> + lflow_input->ls_arp_table = &ls_arp_data->table;
> lflow_input->meter_groups = &sync_meters_data->meter_groups;
> lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map;
> lflow_input->local_svc_monitors_map =
> @@ -229,6 +233,31 @@ lflow_ls_stateful_handler(struct engine_node *node, void
> *data)
> return EN_HANDLED_UPDATED;
> }
>
> +enum engine_input_handler_result
> +lflow_ls_arp_handler(struct engine_node *node, void *data)
> +{
> + struct ed_type_ls_arp *ls_arp_data =
> + engine_get_input_data("ls_arp", node);
> +
> + if (!ls_arp_has_tracked_data(&ls_arp_data->trk_data)) {
> + return EN_UNHANDLED;
> + }
> +
> + const struct engine_context *eng_ctx = engine_get_context();
> + struct lflow_data *lflow_data = data;
> + struct lflow_input lflow_input;
> +
> + lflow_get_input_data(node, &lflow_input);
> + if (!lflow_handle_ls_arp_changes(eng_ctx->ovnsb_idl_txn,
> + &ls_arp_data->trk_data,
> + &lflow_input,
> + lflow_data->lflow_table)) {
> + return EN_UNHANDLED;
> + }
> +
> + return EN_HANDLED_UPDATED;
> +}
> +
> enum engine_input_handler_result
> lflow_multicast_igmp_handler(struct engine_node *node, void *data)
> {
> diff --git a/northd/en-lflow.h b/northd/en-lflow.h
> index 99bcfda15..d2a92e49f 100644
> --- a/northd/en-lflow.h
> +++ b/northd/en-lflow.h
> @@ -25,6 +25,8 @@ lflow_lr_stateful_handler(struct engine_node *, void *data);
> enum engine_input_handler_result
> lflow_ls_stateful_handler(struct engine_node *node, void *data);
> enum engine_input_handler_result
> +lflow_ls_arp_handler(struct engine_node *, void *);
> +enum engine_input_handler_result
> lflow_multicast_igmp_handler(struct engine_node *node, void *data);
> enum engine_input_handler_result
> lflow_group_ecmp_route_change_handler(struct engine_node *node, void *data);
> diff --git a/northd/en-ls-arp.c b/northd/en-ls-arp.c
> new file mode 100644
> index 000000000..69d76ef3e
> --- /dev/null
> +++ b/northd/en-ls-arp.c
> @@ -0,0 +1,355 @@
> +/*
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +/* OVS includes */
> +#include "include/openvswitch/hmap.h"
> +#include "openvswitch/util.h"
> +#include "openvswitch/vlog.h"
> +#include "stopwatch.h"
> +
> +/* OVN includes */
> +#include "en-lr-nat.h"
> +#include "en-ls-arp.h"
> +#include "lib/inc-proc-eng.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/ovn-sb-idl.h"
> +#include "lib/ovn-util.h"
> +#include "lib/stopwatch-names.h"
> +#include "lflow-mgr.h"
> +#include "northd.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_ls_arp);
> +
> +/* Static functions. */
> +struct ls_arp_input {
> + const struct ovn_datapaths *ls_datapaths;
> + const struct lr_nat_table *lr_nats;
> +};
> +
> +static struct ls_arp_input
> +ls_arp_get_input_data(struct engine_node *node)
> +{
> + const struct northd_data *northd_data =
> + engine_get_input_data("northd", node);
> + struct ed_type_lr_nat_data *lr_nat_data =
> + engine_get_input_data("lr_nat", node);
> +
> + return (struct ls_arp_input) {
> + .ls_datapaths = &northd_data->ls_datapaths,
> + .lr_nats = &lr_nat_data->lr_nats,
> + };
> +}
> +
> +static void
> +ls_arp_record_clear(struct ls_arp_record *ls_arp_record)
> +{
> + lflow_ref_destroy(ls_arp_record->lflow_ref);
> + hmapx_destroy(&ls_arp_record->nat_records);
> + free(ls_arp_record);
> +}
> +
> +static void
> +ls_arp_table_clear(struct ls_arp_table *table)
> +{
> + struct ls_arp_record *ls_arp_record;
> + HMAP_FOR_EACH_POP (ls_arp_record, key_node, &table->entries) {
> + ls_arp_record_clear(ls_arp_record);
> + }
> +}
> +
> +static inline bool
> +is_centralized_nat_record(const struct ovn_nat *nat_entry)
> +{
> + return nat_entry->is_valid
> + && nat_entry->l3dgw_port
> + && nat_entry->l3dgw_port->peer
> + && nat_entry->l3dgw_port->peer->od
> + && !nat_entry->is_distributed;
> +}
> +
> +static void
> +nat_record_data_create(struct ls_arp_record *ls_arp_record,
> + const struct ovn_datapath *od,
> + const struct lr_nat_table *lr_nats)
> +{
> + struct ovn_port *op;
> + VECTOR_FOR_EACH (&od->router_ports, op) {
> + const struct ovn_datapath *lr_od = op->peer->od;
> + const struct lr_nat_record *lrnat_rec =
> + lr_nat_table_find_by_uuid(lr_nats, lr_od->key);
> +
> + if (!lrnat_rec) {
> + continue;
> + }
> +
> + for (size_t i = 0; i < lrnat_rec->n_nat_entries; i++) {
> + const struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i];
> +
> + if (is_centralized_nat_record(nat_entry)) {
> + hmapx_add(&ls_arp_record->nat_records,
> + (struct lrnat_rec *) lrnat_rec);
Nit: indentation.
> + }
> + }
> + }
> +}
> +
> +static struct ls_arp_record*
Nit: missing space before '*'.
> +ls_arp_record_lookup_by_od_(const struct ls_arp_table *table,
> + const struct ovn_datapath *od)
> +{
> + struct ls_arp_record *ls_arp_record;
> + HMAP_FOR_EACH_WITH_HASH (ls_arp_record, key_node,
> + uuid_hash(&od->nbs->header_.uuid),
> + &table->entries) {
> + if (uuid_equals(&ls_arp_record->nbs_uuid,
> + &od->nbs->header_.uuid)) {
> + return ls_arp_record;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static struct ls_arp_record *
> +ls_arp_record_create(struct ls_arp_table *table,
> + const struct ovn_datapath *od,
> + const struct lr_nat_table *lr_nats)
> +{
> + struct ls_arp_record *ls_arp_record = xzalloc(sizeof *ls_arp_record);
> +
> + ls_arp_record->ls_index = od->index;
> + ls_arp_record->nbs_uuid = od->nbs->header_.uuid;
> +
> + hmapx_init(&ls_arp_record->nat_records);
> + nat_record_data_create(ls_arp_record, od, lr_nats);
> +
> + ls_arp_record->lflow_ref = lflow_ref_create();
> +
> + hmap_insert(&table->entries, &ls_arp_record->key_node,
> + uuid_hash(&od->nbs->header_.uuid));
> +
> + return ls_arp_record;
> +}
> +
> +/* Public functions. */
> +void*
> +en_ls_arp_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + struct ed_type_ls_arp *data = xzalloc(sizeof *data);
> +
> + hmap_init(&data->table.entries);
> + hmapx_init(&data->trk_data.crupdated);
> + hmapx_init(&data->trk_data.deleted);
> +
> + return data;
> +}
> +
> +void
> +en_ls_arp_clear_tracked_data(void *data_)
> +{
> + struct ed_type_ls_arp *data = data_;
> + hmapx_clear(&data->trk_data.crupdated);
> +
> + struct hmapx_node *n;
> + HMAPX_FOR_EACH_SAFE (n, &data->trk_data.deleted) {
> + ls_arp_record_clear(n->data);
> + hmapx_delete(&data->trk_data.deleted, n);
> + }
> + hmapx_clear(&data->trk_data.deleted);
> +}
> +
> +void
> +en_ls_arp_cleanup(void *data_)
> +{
> + struct ed_type_ls_arp *data = data_;
> +
> + ls_arp_table_clear(&data->table);
> + hmap_destroy(&data->table.entries);
> + hmapx_destroy(&data->trk_data.crupdated);
> +
> + struct hmapx_node *n;
> + HMAPX_FOR_EACH_SAFE (n, &data->trk_data.deleted) {
> + ls_arp_record_clear(n->data);
> + hmapx_delete(&data->trk_data.deleted, n);
> + }
> + hmapx_destroy(&data->trk_data.deleted);
> +}
> +
> +enum engine_node_state
> +en_ls_arp_run(struct engine_node *node, void *data_)
> +{
> + struct ls_arp_input input_data = ls_arp_get_input_data(node);
> + struct ed_type_ls_arp *data = data_;
> +
> + stopwatch_start(LS_ARP_RUN_STOPWATCH_NAME, time_msec());
> +
> + ls_arp_table_clear(&data->table);
> +
> + const struct ovn_datapath *od;
> + HMAP_FOR_EACH (od, key_node, &input_data.ls_datapaths->datapaths) {
> + /* Filtering ARP entries at logical switch works
> + * when there are physical ports on the switch. */
Nit: indendation.
> + if (hmapx_is_empty(&od->ph_ports)) {
> + continue;
> + }
> +
> + ls_arp_record_create(&data->table, od, input_data.lr_nats);
> + }
> +
> + stopwatch_stop(LS_ARP_RUN_STOPWATCH_NAME, time_msec());
> +
> + return EN_UPDATED;
> +}
> +
> +/* Handler functions. */
> +enum engine_input_handler_result
> +ls_arp_northd_handler(struct engine_node *node, void *data_)
> +{
> + struct northd_data *northd_data = engine_get_input_data("northd", node);
> + if (!northd_has_tracked_data(&northd_data->trk_data)) {
> + return EN_UNHANDLED;
> + }
> +
> + if (!northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
> + return EN_HANDLED_UNCHANGED;
> + }
> +
> + struct northd_tracked_data *nd_changes = &northd_data->trk_data;
> + struct ls_arp_input input_data = ls_arp_get_input_data(node);
> + struct ed_type_ls_arp *data = data_;
> + struct hmapx_node *hmapx_node;
> + struct ls_arp_record *ls_arp_record;
> +
> + HMAPX_FOR_EACH (hmapx_node, &nd_changes->trk_switches.crupdated) {
> + const struct ovn_datapath *od = hmapx_node->data;
> +
> + ls_arp_record = ls_arp_record_lookup_by_od_(&data->table, od);
> +
> + if (!ls_arp_record) {
> + /* Filtering ARP entries at logical switch works
> + * when there are physical ports on the switch. */
> + if (hmapx_is_empty(&od->ph_ports)) {
> + continue;
This continue here made me spend quite some time looking at the code and
wondering why it works fine for the case when a switch used to have a
localnet/l2gw port but that got removed in the current iteration.
It turns out we then cause a lr_nat node recompute (because the northd
node didn't have any NAT tracked data) which in turn causes the
ls_arp_lr_nat_handler() below to fail incremental processing.
I think it would be good to have a note about this and also to update
the ovn-northd.at test to cover this scenario.
> + }
> + ls_arp_record = ls_arp_record_create(&data->table,
> + od, input_data.lr_nats);
> + } else {
> + nat_record_data_create(ls_arp_record, od, input_data.lr_nats);
> + }
> +
> + hmapx_add(&data->trk_data.crupdated, ls_arp_record);
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node, &nd_changes->trk_switches.deleted) {
> + const struct ovn_datapath *od = hmapx_node->data;
> +
> + ls_arp_record = ls_arp_record_lookup_by_od_(&data->table, od);
> + if (ls_arp_record) {
> + hmap_remove(&data->table.entries, &ls_arp_record->key_node);
> + hmapx_add(&data->trk_data.deleted, ls_arp_record);
> + }
> + }
> +
> + if (ls_arp_has_tracked_data(&data->trk_data)) {
> + return EN_HANDLED_UPDATED;
> + }
> +
> + return EN_HANDLED_UNCHANGED;
> +}
> +
> +static void
> +nat_odmap_create(struct lr_nat_record *lrnat_rec,
> + struct hmapx *odmap)
> +{
> + for (size_t i = 0; i < lrnat_rec->n_nat_entries; i++) {
> + const struct ovn_nat *nat_entry = &lrnat_rec->nat_entries[i];
> +
> + if (is_centralized_nat_record(nat_entry)) {
> + hmapx_add(odmap, nat_entry->l3dgw_port->peer->od);
> + }
> + }
> +}
> +
> +enum engine_input_handler_result
> +ls_arp_lr_nat_handler(struct engine_node *node, void *data_)
> +{
> + struct ed_type_lr_nat_data *lr_nat_data =
> + engine_get_input_data("lr_nat", node);
> + struct ls_arp_input input_data = ls_arp_get_input_data(node);
> +
> + if (!lr_nat_has_tracked_data(&lr_nat_data->trk_data)) {
> + return EN_UNHANDLED;
> + }
> +
> + struct ed_type_ls_arp *data = data_;
> +
> + struct hmapx_node *hmapx_node;
> + struct ls_arp_record *ls_arp_record;
> + HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.crupdated) {
> + struct lr_nat_record *nat_record_p = hmapx_node->data;
> +
> + struct hmapx ls_links_map = HMAPX_INITIALIZER(&ls_links_map);
> + nat_odmap_create(nat_record_p, &ls_links_map);
> +
> + LS_ARP_TABLE_FOR_EACH (ls_arp_record, &data->table) {
> + struct hmapx_node *nr_node =
> + hmapx_find(&ls_arp_record->nat_records, nat_record_p);
> +
> + if (nr_node) {
> + hmapx_add(&data->trk_data.crupdated, ls_arp_record);
> + hmapx_delete(&ls_arp_record->nat_records, nr_node);
> + }
> + }
> +
> + struct hmapx_node *crupdated_ls_hmapx;
> + HMAPX_FOR_EACH (crupdated_ls_hmapx, &ls_links_map) {
> + struct ovn_datapath *crupdated_ls = crupdated_ls_hmapx->data;
> + ls_arp_record =
> + ls_arp_record_lookup_by_od_(&data->table, crupdated_ls);
> +
> + if (!ls_arp_record) {
> + ls_arp_record = ls_arp_record_create(&data->table,
> + crupdated_ls,
> + input_data.lr_nats);
> + }
> +
> + hmapx_add(&data->trk_data.crupdated, ls_arp_record);
> + hmapx_add(&ls_arp_record->nat_records, nat_record_p);
> + }
> + hmapx_destroy(&ls_links_map);
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node, &lr_nat_data->trk_data.deleted) {
> + struct lr_nat_record *nr_cur = hmapx_node->data;
> +
> + struct ls_arp_record *ar;
> + LS_ARP_TABLE_FOR_EACH (ar, &data->table) {
> + struct hmapx_node *nr_node = hmapx_find(&ar->nat_records,
> nr_cur);
> +
> + if (nr_node) {
> + hmapx_add(&data->trk_data.crupdated, ar);
> + hmapx_delete(&ar->nat_records, nr_node);
> + }
> + }
> + }
> +
> + if (ls_arp_has_tracked_data(&data->trk_data)) {
> + return EN_HANDLED_UPDATED;
> + }
> +
> + return EN_HANDLED_UNCHANGED;
> +}
> diff --git a/northd/en-ls-arp.h b/northd/en-ls-arp.h
> new file mode 100644
> index 000000000..5eaf913bb
> --- /dev/null
> +++ b/northd/en-ls-arp.h
> @@ -0,0 +1,86 @@
> +/*
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef EN_LS_ARP_H
> +#define EN_LS_ARP_H 1
> +
> +/* OVS includes. */
> +#include "lib/hmapx.h"
> +#include "openvswitch/hmap.h"
> +
> +/* OVN includes. */
> +#include "lib/inc-proc-eng.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/ovn-sb-idl.h"
> +#include "lib/ovn-util.h"
> +#include "lib/stopwatch-names.h"
> +
> +struct lflow_ref;
> +
> +struct ls_arp_record {
> + struct hmap_node key_node;
> +
> + /* UUID of the NB Logical switch. */
> + struct uuid nbs_uuid;
> +
> + /* Index of logical switch item in northd. */
> + size_t ls_index;
> +
> + /* 'lflow_ref' is used to reference logical flows generated for
> + * this ls_arp record. */
> + struct lflow_ref *lflow_ref;
> +
> + /* lr_nat_record ptrs that trigger this od to rebuild lflow. */
> + struct hmapx nat_records;
> +};
> +
> +struct ls_arp_table {
> + struct hmap entries;
> +};
> +
> +#define LS_ARP_TABLE_FOR_EACH(LS_ARP_REC, TABLE) \
> + HMAP_FOR_EACH (LS_ARP_REC, key_node, \
> + &(TABLE)->entries)
> +
> +#define LS_ARP_TABLE_FOR_EACH_IN_P(LS_ARP_REC, JOBID, TABLE) \
> + HMAP_FOR_EACH_IN_PARALLEL (LS_ARP_REC, key_node, JOBID, \
> + &(TABLE)->entries)
> +
> +struct ls_arp_tracked_data {
> + struct hmapx crupdated;
> + struct hmapx deleted;
> +};
> +
> +struct ed_type_ls_arp {
> + struct ls_arp_table table;
> + struct ls_arp_tracked_data trk_data;
> +};
> +
> +void *en_ls_arp_init(struct engine_node *, struct engine_arg *);
> +void en_ls_arp_cleanup(void *);
> +void en_ls_arp_clear_tracked_data(void *);
> +enum engine_node_state en_ls_arp_run(struct engine_node *, void *);
> +
> +enum engine_input_handler_result
> +ls_arp_lr_nat_handler(struct engine_node *, void *);
> +enum engine_input_handler_result
> +ls_arp_northd_handler(struct engine_node *, void *);
> +
> +static inline bool
> +ls_arp_has_tracked_data(struct ls_arp_tracked_data *trk_data) {
> + return !hmapx_is_empty(&trk_data->crupdated) ||
> + !hmapx_is_empty(&trk_data->deleted);
> +}
> +
> +#endif /* EN_LS_ARP_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 94199de12..748e09769 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -34,6 +34,7 @@
> #include "en-lr-stateful.h"
> #include "en-lr-nat.h"
> #include "en-ls-stateful.h"
> +#include "en-ls-arp.h"
> #include "en-multicast.h"
> #include "en-northd.h"
> #include "en-lflow.h"
> @@ -174,6 +175,7 @@ static ENGINE_NODE(lb_data, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(lr_nat, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(lr_stateful, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(ls_stateful, CLEAR_TRACKED_DATA);
> +static ENGINE_NODE(ls_arp, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(route_policies);
> static ENGINE_NODE(routes);
> static ENGINE_NODE(bfd);
> @@ -305,6 +307,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> ls_stateful_port_group_handler);
> engine_add_input(&en_ls_stateful, &en_nb_acl, ls_stateful_acl_handler);
>
> + engine_add_input(&en_ls_arp, &en_lr_nat, ls_arp_lr_nat_handler);
> + engine_add_input(&en_ls_arp, &en_northd, ls_arp_northd_handler);
> +
> engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding, NULL);
> engine_add_input(&en_mac_binding_aging, &en_northd, NULL);
> engine_add_input(&en_mac_binding_aging, &en_mac_binding_aging_waker,
> NULL);
> @@ -409,6 +414,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_lflow, &en_port_group, engine_noop_handler);
> engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler);
> engine_add_input(&en_lflow, &en_ls_stateful, lflow_ls_stateful_handler);
> +
> + engine_add_input(&en_lflow, &en_ls_arp, lflow_ls_arp_handler);
> +
Nit: I would remove the newlines before and after this one.
> engine_add_input(&en_lflow, &en_multicast_igmp,
> lflow_multicast_igmp_handler);
> engine_add_input(&en_lflow, &en_sb_acl_id, NULL);
> diff --git a/northd/northd.c b/northd/northd.c
> index e978b66c2..a0db6f226 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -50,6 +50,7 @@
> #include "en-lr-nat.h"
> #include "en-lr-stateful.h"
> #include "en-ls-stateful.h"
> +#include "en-ls-arp.h"
> #include "en-multicast.h"
> #include "en-sampling-app.h"
> #include "en-datapath-logical-switch.h"
> @@ -136,6 +137,7 @@ static bool vxlan_mode;
> #define REGBIT_IP_FRAG "reg0[19]"
> #define REGBIT_ACL_PERSIST_ID "reg0[20]"
> #define REGBIT_ACL_HINT_ALLOW_PERSISTED "reg0[21]"
> +#define REGBIT_EXT_ARP "reg0[22]"
>
> /* Register definitions for switches and routers. */
>
> @@ -547,6 +549,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct
> uuid *key,
> hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key));
> od->lr_group = NULL;
> hmap_init(&od->ports);
> + hmapx_init(&od->ph_ports);
> sset_init(&od->router_ips);
> od->ls_peers = VECTOR_EMPTY_INITIALIZER(struct ovn_datapath *);
> od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
> @@ -584,6 +587,7 @@ ovn_datapath_destroy(struct ovn_datapath *od)
> vector_destroy(&od->l3dgw_ports);
> destroy_mcast_info_for_datapath(od);
> destroy_ports_for_datapath(od);
> + hmapx_destroy(&od->ph_ports);
> sset_destroy(&od->router_ips);
> lflow_ref_destroy(od->datapath_lflows);
> free(od);
> @@ -1218,6 +1222,12 @@ lsp_is_vtep(const struct nbrec_logical_switch_port
> *nbsp)
> return !strcmp(nbsp->type, "vtep");
> }
>
> +static bool
> +lsp_is_l2gw(const struct nbrec_logical_switch_port *nbsp)
> +{
> + return !strcmp(nbsp->type, "l2gateway");
> +}
> +
> static bool
> localnet_can_learn_mac(const struct nbrec_logical_switch_port *nbsp)
> {
> @@ -1604,6 +1614,10 @@ join_logical_ports_lsp(struct hmap *ports,
> od->has_vtep_lports = true;
> }
>
> + if (lsp_is_localnet(nbsp) || lsp_is_l2gw(nbsp)) {
> + hmapx_add(&od->ph_ports, op);
> + }
> +
> parse_lsp_addrs(op);
>
> op->od = od;
> @@ -1997,6 +2011,15 @@ join_logical_ports(const struct
> sbrec_port_binding_table *sbrec_pb_table,
> }
> }
>
> +static bool
> +is_nat_distributed(const struct nbrec_nat *nat,
> + const struct ovn_datapath *od)
> +{
> + return !vector_is_empty(&od->l3dgw_ports)
> + && nat->logical_port && nat->external_mac
> + && !strcmp(nat->type, "dnat_and_snat");
> +}
> +
> /* Returns an array of strings, each consisting of a MAC address followed
> * by one or more IP addresses, and if the port is a distributed gateway
> * port, followed by 'is_chassis_resident("LPORT_NAME")', where the
> @@ -2055,9 +2078,7 @@ get_nat_addresses(const struct ovn_port *op, size_t *n,
> bool routable_only,
>
> /* Determine whether this NAT rule satisfies the conditions for
> * distributed NAT processing. */
> - if (!vector_is_empty(&op->od->l3dgw_ports) &&
> - !strcmp(nat->type, "dnat_and_snat") &&
> - nat->logical_port && nat->external_mac) {
> + if (is_nat_distributed(nat, op->od)) {
> /* Distributed NAT rule. */
> if (eth_addr_from_string(nat->external_mac, &mac)) {
> struct ds address = DS_EMPTY_INITIALIZER;
> @@ -9676,6 +9697,99 @@
> build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
> ds_destroy(&match);
> }
>
> +/*
> + * Create ARP filtering flow for od, assumed logical switch,
> + * for the following condition:
> + * Given lswitch has either localnet or l2gateway ports and
> + * router connection ports that requires chassis residence.
> + * ARP requests coming from localnet/l2gateway ports
> + * allowed for processing on resident chassis only.
> + */
> +static void
> +build_lswitch_arp_chassis_resident(const struct ovn_datapath *od,
> + struct lflow_table *lflows,
> + const struct ls_arp_record *ar)
> +{
> + struct sset inports;
> + struct sset resident_ports;
> + struct sset distributed_nat_ports;
Nit: reverse xmas tree and we can use SSET_INITIALIZER() directly.
> +
> + sset_init(&inports);
> + sset_init(&resident_ports);
> + sset_init(&distributed_nat_ports);
> +
> + struct ds match = DS_EMPTY_INITIALIZER;
> +
> + struct hmapx_node *node;
> + HMAPX_FOR_EACH (node, &od->ph_ports) {
> + struct ovn_port *op = node->data;
> + sset_add(&inports, op->json_key);
> + }
> +
> + struct ovn_port *op;
> + VECTOR_FOR_EACH (&od->router_ports, op) {
> + struct ovn_port *op_r = op->peer;
> +
> + if (lrp_is_l3dgw(op_r)) {
> + sset_add(&resident_ports, op_r->cr_port->json_key);
> + }
> + }
> +
> + struct hmapx_node *hmapx_node;
> + HMAPX_FOR_EACH (hmapx_node, &ar->nat_records) {
> + struct lr_nat_record *nr = hmapx_node->data;
> +
> + for (size_t i = 0; i < nr->n_nat_entries; i++) {
> + struct ovn_nat *ent = &nr->nat_entries[i];
> + if (ent->is_valid && ent->is_distributed) {
> + sset_add(&distributed_nat_ports, ent->nb->logical_port);
> + }
> + }
> + }
> +
> +
> + if (!sset_is_empty(&inports) && !sset_is_empty(&resident_ports)) {
> + const char *item;
> +
> + SSET_FOR_EACH (item, &inports) {
> + ds_clear(&match);
> + ds_put_format(&match,
> + "(arp.op == 1 || arp.op == 2) && inport == %s",
> + item);
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 75,
> + ds_cstr(&match), REGBIT_EXT_ARP " = 1; next;",
> + ar->lflow_ref);
> + }
> +
> + SSET_FOR_EACH (item, &resident_ports) {
> + ds_clear(&match);
> + ds_put_format(&match,
> + REGBIT_EXT_ARP" == 1 && is_chassis_resident(%s)",
> + item);
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 75,
> + ds_cstr(&match), "next;", ar->lflow_ref);
> + }
> +
> + SSET_FOR_EACH (item, &distributed_nat_ports) {
> + ds_clear(&match);
> + ds_put_format(&match,
> + REGBIT_EXT_ARP
> + " == 1 && is_chassis_resident(\"%s\")",
> + item);
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 75,
> + ds_cstr(&match), "next;", ar->lflow_ref);
> + }
> +
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 70,
> + REGBIT_EXT_ARP" == 1", "drop;", ar->lflow_ref);
> + }
> +
> + ds_destroy(&match);
> + sset_destroy(&inports);
> + sset_destroy(&resident_ports);
> + sset_destroy(&distributed_nat_ports);
> +}
> +
> static bool
> is_vlan_transparent(const struct ovn_datapath *od)
> {
> @@ -14117,10 +14231,6 @@ build_neigh_learning_flows_for_lrouter_port(
> op->lrp_networks.ipv4_addrs[i].network_s,
> op->lrp_networks.ipv4_addrs[i].plen,
> op->lrp_networks.ipv4_addrs[i].addr_s);
> - if (lrp_is_l3dgw(op)) {
> - ds_put_format(match, " && is_chassis_resident(%s)",
> - op->cr_port->json_key);
> - }
> const char *actions_s = REGBIT_LOOKUP_NEIGHBOR_RESULT
> " = lookup_arp(inport, arp.spa, arp.sha); "
> REGBIT_LOOKUP_NEIGHBOR_IP_RESULT" = 1;"
> @@ -14137,10 +14247,6 @@ build_neigh_learning_flows_for_lrouter_port(
> op->json_key,
> op->lrp_networks.ipv4_addrs[i].network_s,
> op->lrp_networks.ipv4_addrs[i].plen);
> - if (lrp_is_l3dgw(op)) {
> - ds_put_format(match, " && is_chassis_resident(%s)",
> - op->cr_port->json_key);
> - }
> ds_clear(actions);
> ds_put_format(actions, REGBIT_LOOKUP_NEIGHBOR_RESULT
> " = lookup_arp(inport, arp.spa, arp.sha); %snext;",
> @@ -18552,6 +18658,7 @@ struct lswitch_flow_build_info {
> const struct ls_port_group_table *ls_port_groups;
> const struct lr_stateful_table *lr_stateful_table;
> const struct ls_stateful_table *ls_stateful_table;
> + const struct ls_arp_table *ls_arp_table;
> struct lflow_table *lflows;
> const struct shash *meter_groups;
> const struct hmap *lb_dps_map;
> @@ -18744,6 +18851,7 @@ build_lflows_thread(void *arg)
> struct worker_control *control = (struct worker_control *) arg;
> const struct lr_stateful_record *lr_stateful_rec;
> const struct ls_stateful_record *ls_stateful_rec;
> + const struct ls_arp_record *ls_arp_rec;
> struct lswitch_flow_build_info *lsi;
> struct ovn_lb_datapaths *lb_dps;
> struct ovn_datapath *od;
> @@ -18900,6 +19008,19 @@ build_lflows_thread(void *arg)
> }
> }
>
> + for (bnum = control->id;
> + bnum <= lsi->ls_arp_table->entries.mask;
> + bnum += control->pool->size)
> + {
> + LS_ARP_TABLE_FOR_EACH_IN_P (ls_arp_rec, bnum,
> + lsi->ls_arp_table) {
> + od = ovn_datapaths_find_by_index(
> + lsi->ls_datapaths, ls_arp_rec->ls_index);
> + build_lswitch_arp_chassis_resident(od, lsi->lflows,
> + ls_arp_rec);
> + }
> + }
> +
> lsi->thread_lflow_counter = thread_lflow_counter;
> }
> post_completed_work(control);
> @@ -18948,6 +19069,7 @@ build_lswitch_and_lrouter_flows(
> const struct ls_port_group_table *ls_pgs,
> const struct lr_stateful_table *lr_stateful_table,
> const struct ls_stateful_table *ls_stateful_table,
> + const struct ls_arp_table *ls_arp_table,
> struct lflow_table *lflows,
> const struct shash *meter_groups,
> const struct hmap *lb_dps_map,
> @@ -18984,6 +19106,7 @@ build_lswitch_and_lrouter_flows(
> lsiv[index].ls_port_groups = ls_pgs;
> lsiv[index].lr_stateful_table = lr_stateful_table;
> lsiv[index].ls_stateful_table = ls_stateful_table;
> + lsiv[index].ls_arp_table = ls_arp_table;
> lsiv[index].meter_groups = meter_groups;
> lsiv[index].lb_dps_map = lb_dps_map;
> lsiv[index].local_svc_monitor_map =
> @@ -19020,6 +19143,7 @@ build_lswitch_and_lrouter_flows(
> } else {
> const struct lr_stateful_record *lr_stateful_rec;
> const struct ls_stateful_record *ls_stateful_rec;
> + const struct ls_arp_record *ls_arp_rec;
> struct ovn_lb_datapaths *lb_dps;
> struct ovn_datapath *od;
> struct ovn_port *op;
> @@ -19032,6 +19156,7 @@ build_lswitch_and_lrouter_flows(
> .ls_port_groups = ls_pgs,
> .lr_stateful_table = lr_stateful_table,
> .ls_stateful_table = ls_stateful_table,
> + .ls_arp_table = ls_arp_table,
> .lflows = lflows,
> .meter_groups = meter_groups,
> .lb_dps_map = lb_dps_map,
> @@ -19127,6 +19252,12 @@ build_lswitch_and_lrouter_flows(
> lsi.sbrec_acl_id_table);
> }
>
> + LS_ARP_TABLE_FOR_EACH (ls_arp_rec, ls_arp_table) {
> + od = ovn_datapaths_find_by_index(lsi.ls_datapaths,
> + ls_arp_rec->ls_index);
> + build_lswitch_arp_chassis_resident(od, lsi.lflows, ls_arp_rec);
> + }
> +
> ds_destroy(&lsi.match);
> ds_destroy(&lsi.actions);
> }
> @@ -19210,6 +19341,7 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
> input_data->ls_port_groups,
> input_data->lr_stateful_table,
> input_data->ls_stateful_table,
> + input_data->ls_arp_table,
> lflows,
> input_data->meter_groups,
> input_data->lb_datapaths_map,
> @@ -19712,6 +19844,43 @@ lflow_handle_ls_stateful_changes(struct
> ovsdb_idl_txn *ovnsb_txn,
> return true;
> }
>
> +bool
> +lflow_handle_ls_arp_changes(struct ovsdb_idl_txn *ovnsb_txn,
> + struct ls_arp_tracked_data *trk_data,
> + struct lflow_input *lflow_input,
> + struct lflow_table *lflows)
> +{
> + struct hmapx_node *hmapx_node;
> +
> + HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
> + const struct ls_arp_record *ls_arp_record = hmapx_node->data;
> + const struct ovn_datapath *od =
> + ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
> + ls_arp_record->ls_index);
> + lflow_ref_unlink_lflows(ls_arp_record->lflow_ref);
> +
> + build_lswitch_arp_chassis_resident(od, lflows, ls_arp_record);
> +
> + bool handled = lflow_ref_sync_lflows(
> + ls_arp_record->lflow_ref, lflows, ovnsb_txn,
> + lflow_input->ls_datapaths,
> + lflow_input->lr_datapaths,
> + lflow_input->ovn_internal_version_changed,
> + lflow_input->sbrec_logical_flow_table,
> + lflow_input->sbrec_logical_dp_group_table);
> + if (!handled) {
> + return false;
> + }
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node, &trk_data->deleted) {
> + struct ls_arp_record *ls_arp_record = hmapx_node->data;
> + lflow_ref_unlink_lflows(ls_arp_record->lflow_ref);
> + }
> +
> + return true;
> +}
> +
> static bool
> mirror_needs_update(const struct nbrec_mirror *nb_mirror,
> const struct sbrec_mirror *sb_mirror)
> diff --git a/northd/northd.h b/northd/northd.h
> index 32134d36e..c41c41e00 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -20,6 +20,7 @@
> #include "lib/ovn-util.h"
> #include "lib/ovs-atomic.h"
> #include "lib/sset.h"
> +#include "lib/hmapx.h"
> #include "northd/en-port-group.h"
> #include "northd/ipam.h"
> #include "northd/lb.h"
> @@ -27,6 +28,7 @@
> #include "simap.h"
> #include "ovs-thread.h"
> #include "en-lr-stateful.h"
> +#include "en-ls-arp.h"
> #include "vec.h"
> #include "datapath-sync.h"
>
> @@ -272,6 +274,7 @@ struct lflow_input {
> const struct ls_port_group_table *ls_port_groups;
> const struct lr_stateful_table *lr_stateful_table;
> const struct ls_stateful_table *ls_stateful_table;
> + const struct ls_arp_table *ls_arp_table;
> const struct shash *meter_groups;
> const struct hmap *lb_datapaths_map;
> const struct sset *bfd_ports;
> @@ -470,6 +473,9 @@ struct ovn_datapath {
> * Valid only if it is logical router datapath. NULL otherwise. */
> struct lrouter_group *lr_group;
>
> + /* Set of localnet or l2gw ports. */
> + struct hmapx ph_ports;
Nit: I think a more explicit name is a bit better, e.g., "phys_ports".
> +
> /* Map of ovn_port objects belonging to this datapath.
> * This map doesn't include derived ports. */
> struct hmap ports;
> @@ -957,6 +963,10 @@ bool lflow_handle_ls_stateful_changes(struct
> ovsdb_idl_txn *,
> struct ls_stateful_tracked_data *,
> struct lflow_input *,
> struct lflow_table *lflows);
> +bool lflow_handle_ls_arp_changes(struct ovsdb_idl_txn *,
> + struct ls_arp_tracked_data *,
> + struct lflow_input *,
> + struct lflow_table *lflows);
> bool northd_handle_sb_port_binding_changes(
> const struct sbrec_port_binding_table *, struct hmap *ls_ports,
> struct hmap *lr_ports);
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 005fd87d1..0f6693b2f 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -310,6 +310,15 @@
> </p>
> </li>
>
> + <li>
> + For each logical switch that has connected physical ports
> + (localnet or l2gateway) and is also connected to a distributed
> router,
> + filtering rules are added for ARP requests coming from localnet or
> + l2gateway ports, allowed for processing on gateway chassis.
> + The <code>REGBIT_EXT_ARP</code> register is set for all ARP requests
> + originating from physical ports with priority 75 flow.
> + </li>
> +
> <li>
> For each (enabled) vtep logical port, a priority 70 flow is added
> which
> matches on all packets and applies the action
> @@ -396,6 +405,13 @@
> One priority-0 fallback flow that matches all packets and advances to
> the next table.
> </li>
> +
> + <li>
> + Priority 75: Allows <code>REGBIT_EXT_ARP</code> packets only on
> gateway
> + chassis and chassis with distributed NAT entries.
> + Priority 70: Drops <code>REGBIT_EXT_ARP</code> packets on non-gateway
> + chassis (complements the priority 75 flow).
> + </li>
> </ul>
>
> <h3>Ingress Table 2: Mirror </h3>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 52a3c7883..25677d73e 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1016,6 +1016,7 @@ main(int argc, char *argv[])
> stopwatch_create(LR_NAT_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(LR_STATEFUL_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(LS_STATEFUL_RUN_STOPWATCH_NAME, SW_MS);
> + stopwatch_create(LS_ARP_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME, SW_MS);
> stopwatch_create(DYNAMIC_ROUTES_RUN_STOPWATCH_NAME, SW_MS);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index f7ec6a33a..1533405b2 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -7335,9 +7335,6 @@ AT_CHECK([grep lr_in_admission lrflows | grep cr-DR |
> ovn_strip_lflows], [0], [d
> ])
> # Check the flows in lr_in_lookup_neighbor stage
> AT_CHECK([grep lr_in_lookup_neighbor lrflows | grep cr-DR |
> ovn_strip_lflows], [0], [dnl
> - table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "DR-S1"
> && arp.spa == 172.16.1.0/24 && arp.op == 1 &&
> is_chassis_resident("cr-DR-S1")), action=(reg9[[2]] = lookup_arp(inport,
> arp.spa, arp.sha); next;)
> - table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "DR-S2"
> && arp.spa == 172.16.2.0/24 && arp.op == 1 &&
> is_chassis_resident("cr-DR-S2")), action=(reg9[[2]] = lookup_arp(inport,
> arp.spa, arp.sha); next;)
> - table=??(lr_in_lookup_neighbor), priority=100 , match=(inport == "DR-S3"
> && arp.spa == 172.16.3.0/24 && arp.op == 1 &&
> is_chassis_resident("cr-DR-S3")), action=(reg9[[2]] = lookup_arp(inport,
> arp.spa, arp.sha); next;)
> table=??(lr_in_lookup_neighbor), priority=120 , match=(inport == "DR-S1"
> && (nd_na || nd_ns) && eth.mcast && !is_chassis_resident("cr-DR-S1")),
> action=(reg9[[2]] = 1; next;)
> table=??(lr_in_lookup_neighbor), priority=120 , match=(inport == "DR-S2"
> && (nd_na || nd_ns) && eth.mcast && !is_chassis_resident("cr-DR-S2")),
> action=(reg9[[2]] = 1; next;)
> table=??(lr_in_lookup_neighbor), priority=120 , match=(inport == "DR-S3"
> && (nd_na || nd_ns) && eth.mcast && !is_chassis_resident("cr-DR-S3")),
> action=(reg9[[2]] = 1; next;)
> @@ -14031,7 +14028,7 @@ AT_CHECK([grep -Fe "172.168.0.110" -e "172.168.0.120"
> -e "10.0.0.3" -e "20.0.0.3
> table=??(lr_out_undnat ), priority=100 , match=(ip && ip4.src ==
> 20.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")),
> action=(ct_dnat;)
> ])
>
> -AT_CHECK([grep -Fe "172.168.0.110" -e "172.168.0.120" -e "10.0.0.3" -e
> "20.0.0.3" -e "30:54:00:00:00:03" -e "sw0-port1" publicflows |
> ovn_strip_lflows], [0], [dnl
> +AT_CHECK([grep -Fe "172.168.0.110" -e "172.168.0.120" -e "10.0.0.3" -e
> "20.0.0.3" -e "30:54:00:00:00:03" -e "sw0-port1" publicflows |
> ovn_strip_lflows | grep -v "reg0.*22"], [0], [dnl
> table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst ==
> 30:54:00:00:00:03 && is_chassis_resident("sw0-port1")), action=(outport =
> "public-lr0"; output;)
> table=??(ls_in_l2_lkup ), priority=75 , match=(eth.src ==
> {00:00:00:00:ff:02, 30:54:00:00:00:03} && eth.dst == ff:ff:ff:ff:ff:ff &&
> (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport = "_MC_flood_l2";
> output;)
> table=??(ls_in_l2_lkup ), priority=80 , match=(flags[[1]] == 0 &&
> arp.op == 1 && arp.tpa == 172.168.0.110), action=(clone {outport =
> "public-lr0"; output; }; outport = "_MC_flood_l2"; output;)
> @@ -18760,3 +18757,77 @@ check_row_count Advertised_MAC_Binding 0
> OVN_CLEANUP_NORTHD
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Logical Switch ARP filtering])
> +ovn_start
> +
> +check ovn-nbctl lr-add lr1
> +check ovn-nbctl lrp-add lr1 down_link f0:00:00:00:00:f1 192.168.1.1/24
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 up_link
> +check ovn-nbctl lsp-add ls1 down_vif1
> +check ovn-nbctl lsp-add ls1 down_vif2
> +check ovn-nbctl lsp-add ls1 down_ext
> +
> +check ovn-nbctl set Logical_Switch_Port up_link \
> + type=router \
> + options:router-port=down_link \
> + addresses=router
> +
> +check ovn-nbctl lsp-set-addresses down_vif1 'f0:00:00:00:00:01 192.168.1.101'
> +check ovn-nbctl lsp-set-addresses down_vif2 'f0:00:00:00:00:02 192.168.1.102'
> +check ovn-nbctl lrp-set-gateway-chassis down_link hv1
> +
There's a race here, we need --wait=sb sync (also in a bunch of other places in
this test).
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> +])
> +
> +# Check localnet port addings trigger ls-arp flow
> +check ovn-nbctl lsp-set-type down_ext localnet
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
> is_chassis_resident("cr-down_link")), action=(next;)
> +])
> +
> +# Check nat adding to dgr attached to logical switch trigger ls-arp flow
> +check ovn-nbctl lr-nat-add lr1 dnat_and_snat 192.168.0.4 10.0.0.4
> +check ovn-nbctl lr-nat-add lr1 dnat_and_snat 192.168.0.3 10.0.0.3 down_vif1
> f0:00:00:00:00:03
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
> is_chassis_resident("cr-down_link")), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
> is_chassis_resident("down_vif1")), action=(next;)
> +])
> +
> +check ovn-nbctl lr-nat-del lr1 dnat_and_snat 192.168.0.3
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
> is_chassis_resident("cr-down_link")), action=(next;)
> +])
> +
> +# Check changinf logical port type
> +check ovn-nbctl lsp-set-type down_ext l2gateway
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
> action=(drop;)
> + table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
> is_chassis_resident("cr-down_link")), action=(next;)
> +])
> +
> +check ovn-nbctl lsp-del down_ext
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
> action=(drop;)
> +])
> +
> +AT_CLEANUP
> +])
> \ No newline at end of file
No newline at end of file. :)
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 30560f883..113d64229 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -43885,3 +43885,156 @@ OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], expected)
> OVN_CLEANUP([hv1])
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([GARP delivery: gw and external ports])
> +AT_SKIP_IF([test $HAVE_SCAPY = no])
> +ovn_start
> +
> +# Configure initial environment
> +# LR1: down_link <-> LS1: up_link
> +# set lr_down: gateway port (chassis redirect) bound to hv1
> +# LS1: down_vif1 - vif port bound to hv1
> +# down_vif2 - vif port bound to hv2
> +# down_ext - outher port will be iteratred as localnet, l2gateway
> +#
> +# Test: throw ARP annonce request from vitrual ports (down_vif1, down_vif2)
> +# ensure mac_binding is always updated.
> +# (Fixing the issue: mac_binding is only updated for packets came from
> +# down_link's resident chassis)
> +# throw ARP annonce request from from localnet.
> +# ensure mac_binding is updated only if localnet bound to same hv as
> l3dgw
> +
> +check ovn-nbctl lr-add lr1
> +check ovn-nbctl lrp-add lr1 down_link f0:00:00:00:00:f1 192.168.1.1/24
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 up_link
> +check ovn-nbctl lsp-add ls1 down_vif1
> +check ovn-nbctl lsp-add ls1 down_vif2
> +check ovn-nbctl lsp-add ls1 down_ext
> +
> +check ovn-nbctl set Logical_Switch_Port up_link \
> + type=router \
> + options:router-port=down_link \
> + addresses=router
> +
> +check ovn-nbctl lsp-set-addresses down_vif1 'f0:00:00:00:00:01 192.168.1.101'
> +check ovn-nbctl lsp-set-addresses down_vif2 'f0:00:00:00:00:02 192.168.1.102'
> +
> +check ovn-nbctl lsp-set-type down_ext localnet
> +check ovn-nbctl lsp-set-options down_ext network_name=physnet1
> +check ovn-nbctl lrp-set-gateway-chassis down_link hv1
> +
> +net_add n1
> +
> +# Create hypervisor hv1 connected to 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 vif1 -- \
> + set Interface vif1 external-ids:iface-id=down_vif1 \
> + options:tx_pcap=hv1/vif1-tx.pcap options:rxq_pcap=hv1/vif1-rx.pcap
> +
> +# Create hypervisor hv2 connected to n1, add localnet here
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovs-vsctl add-br br-eth0
> +ovn_attach n1 br-phys 192.168.0.2
> +ovs-vsctl add-port br-int vif2 -- \
> + set Interface vif2 external-ids:iface-id=down_vif2 \
> + options:tx_pcap=hv2/vif2-tx.pcap options:rxq_pcap=hv2/vif2-rx.pcap
> +
> +ovs-vsctl set Open_vSwitch .
> external_ids:ovn-bridge-mappings="physnet1:br-eth0"
> +
> +ovs-vsctl add-port br-eth0 vif_ext -- \
> + set Interface vif_ext options:tx_pcap=hv2/vif_ext-tx.pcap \
> + options:rxq_pcap=hv2/vif_ext-rx.pcap
> +
> +# Pre-populate the hypervisors' ARP tables so that we don't lose any
> +# packets for ARP resolution (native tunneling doesn't queue packets
> +# for ARP resolution).
> +OVN_POPULATE_ARP
> +
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +
> +# Annonce 192.168.1.222 from localnet in hv2
> +# result: drop, hv2 is not gateway chassis for down_link
> +sha=02:00:00:00:00:ee
> +tha=00:00:00:00:00:00
> +spa=192.168.1.222
> +tpa=$spa
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv2 ovs-appctl netdev-dummy/receive vif_ext $garp
> +
> +# Make hv2 gateway chassis
> +# Annonce 192.168.1.223 from localnet in hv2
> +# result: ok, hv2 is gateway chassis for down_link
> +#
> +check ovn-nbctl lrp-set-gateway-chassis down_link hv2
> +
> +wait_row_count Port_Binding 1 logical_port=cr-down_link 'chassis!=[[]]'
I think to be completely sure we don't race here we should add a:
check ovn-nbctl --wait=hv sync
> +
> +sha=02:00:00:00:00:ee
> +tha=00:00:00:00:00:00
> +spa=192.168.1.223
> +tpa=$spa
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv2 ovs-appctl netdev-dummy/receive vif_ext $garp
> +
> +# Annonce 192.168.1.111, 112 from vif1, vif2 in hv1, hv2
> +# result: ok, vif1, vif2 are virtual ports, restrictions are not applied.
> +sha=f0:00:00:00:00:01
> +tha=00:00:00:00:00:00
> +spa=192.168.1.111
> +tpa=0.0.0.0
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv1 ovs-appctl netdev-dummy/receive vif1 $garp
> +
> +sha=f0:00:00:00:00:02
> +tha=00:00:00:00:00:00
> +spa=192.168.1.112
> +tpa=0.0.0.0
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv2 ovs-appctl netdev-dummy/receive vif2 $garp
> +
> +# Set down_ext type to l2gateway
> +# Annonce 192.168.1.113, 114 from vif1, vif2 in hv1, hv2
> +# result: ok, vif1, vif2 are virtual ports, restrictions are not applied.
> +check ovn-nbctl --wait=hv lsp-set-type down_ext l2gateway
> +
> +sha=f0:00:00:00:00:01
> +tha=00:00:00:00:00:00
> +spa=192.168.1.113
> +tpa=0.0.0.0
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv1 ovs-appctl netdev-dummy/receive vif1 $garp
> +
> +sha=f0:00:00:00:00:02
> +tha=00:00:00:00:00:00
> +spa=192.168.1.114
> +tpa=0.0.0.0
> +garp=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${sha}')/ \
> + ARP(hwsrc='${sha}', hwdst='${tha}', psrc='${spa}',
> pdst='${tpa}')")
> +as hv2 ovs-appctl netdev-dummy/receive vif2 $garp
> +
> +wait_row_count MAC_Binding 1 ip="192.168.1.111" mac='"f0:00:00:00:00:01"'
> logical_port='"down_link"'
> +wait_row_count MAC_Binding 1 ip="192.168.1.112" mac='"f0:00:00:00:00:02"'
> logical_port='"down_link"'
> +wait_row_count MAC_Binding 1 ip="192.168.1.113" mac='"f0:00:00:00:00:01"'
> logical_port='"down_link"'
> +wait_row_count MAC_Binding 1 ip="192.168.1.114" mac='"f0:00:00:00:00:02"'
> logical_port='"down_link"'
> +wait_row_count MAC_Binding 1 ip="192.168.1.223" mac='"02:00:00:00:00:ee"'
> logical_port='"down_link"'
> +wait_row_count MAC_Binding 0 ip="192.168.1.222" mac='"02:00:00:00:00:ee"'
> logical_port='"down_link"'
> +
> +check ovn-nbctl --wait=hv lsp-set-type down_ext localnet
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
The changes I'm suggesting are minor, here's the full diff (some more style
issues
are addressed in it):
https://github.com/dceara/ovn/commit/a2307c9
If it looks good to you too I can squash it in and push the patch to main.
Please let me know what you think.
Regards,
Dumitru
---
diff --git a/northd/en-ls-arp.c b/northd/en-ls-arp.c
index 69d76ef3ee..72622fce56 100644
--- a/northd/en-ls-arp.c
+++ b/northd/en-ls-arp.c
@@ -100,13 +100,13 @@ nat_record_data_create(struct ls_arp_record
*ls_arp_record,
if (is_centralized_nat_record(nat_entry)) {
hmapx_add(&ls_arp_record->nat_records,
- (struct lrnat_rec *) lrnat_rec);
+ (struct lrnat_rec *) lrnat_rec);
}
}
}
}
-static struct ls_arp_record*
+static struct ls_arp_record *
ls_arp_record_lookup_by_od_(const struct ls_arp_table *table,
const struct ovn_datapath *od)
{
@@ -202,8 +202,8 @@ en_ls_arp_run(struct engine_node *node, void *data_)
const struct ovn_datapath *od;
HMAP_FOR_EACH (od, key_node, &input_data.ls_datapaths->datapaths) {
/* Filtering ARP entries at logical switch works
- * when there are physical ports on the switch. */
- if (hmapx_is_empty(&od->ph_ports)) {
+ * when there are physical ports on the switch. */
+ if (hmapx_is_empty(&od->phys_ports)) {
continue;
}
@@ -242,7 +242,12 @@ ls_arp_northd_handler(struct engine_node *node, void
*data_)
if (!ls_arp_record) {
/* Filtering ARP entries at logical switch works
* when there are physical ports on the switch. */
- if (hmapx_is_empty(&od->ph_ports)) {
+ if (hmapx_is_empty(&od->phys_ports)) {
+ /* NOTE: If the switch used to have physical ports but those
+ * were removed the lr_nat node has recomputed and triggers
+ * the ls_arp_lr_nat_handler() which cannot incrementally
+ * process changes. This implicitly triggers correct
+ * handling of the removal.*/
continue;
}
ls_arp_record = ls_arp_record_create(&data->table,
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 748e097697..b9ac33dc3a 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -414,9 +414,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_lflow, &en_port_group, engine_noop_handler);
engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler);
engine_add_input(&en_lflow, &en_ls_stateful, lflow_ls_stateful_handler);
-
engine_add_input(&en_lflow, &en_ls_arp, lflow_ls_arp_handler);
-
engine_add_input(&en_lflow, &en_multicast_igmp,
lflow_multicast_igmp_handler);
engine_add_input(&en_lflow, &en_sb_acl_id, NULL);
diff --git a/northd/northd.c b/northd/northd.c
index 6ad120dde6..ec219a0c71 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -549,7 +549,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct
uuid *key,
hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key));
od->lr_group = NULL;
hmap_init(&od->ports);
- hmapx_init(&od->ph_ports);
+ hmapx_init(&od->phys_ports);
sset_init(&od->router_ips);
od->ls_peers = VECTOR_EMPTY_INITIALIZER(struct ovn_datapath *);
od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *);
@@ -586,7 +586,7 @@ ovn_datapath_destroy(struct ovn_datapath *od)
vector_destroy(&od->l3dgw_ports);
destroy_mcast_info_for_datapath(od);
destroy_ports_for_datapath(od);
- hmapx_destroy(&od->ph_ports);
+ hmapx_destroy(&od->phys_ports);
sset_destroy(&od->router_ips);
free(od);
}
@@ -1613,7 +1613,7 @@ join_logical_ports_lsp(struct hmap *ports,
}
if (lsp_is_localnet(nbsp) || lsp_is_l2gw(nbsp)) {
- hmapx_add(&od->ph_ports, op);
+ hmapx_add(&od->phys_ports, op);
}
parse_lsp_addrs(op);
@@ -9708,18 +9708,14 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
struct lflow_table *lflows,
const struct ls_arp_record *ar)
{
- struct sset inports;
- struct sset resident_ports;
- struct sset distributed_nat_ports;
-
- sset_init(&inports);
- sset_init(&resident_ports);
- sset_init(&distributed_nat_ports);
-
+ struct sset distributed_nat_ports =
+ SSET_INITIALIZER(&distributed_nat_ports);
+ struct sset resident_ports = SSET_INITIALIZER(&resident_ports);
+ struct sset inports = SSET_INITIALIZER(&inports);
struct ds match = DS_EMPTY_INITIALIZER;
struct hmapx_node *node;
- HMAPX_FOR_EACH (node, &od->ph_ports) {
+ HMAPX_FOR_EACH (node, &od->phys_ports) {
struct ovn_port *op = node->data;
sset_add(&inports, op->json_key);
}
@@ -9745,35 +9741,33 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
}
}
-
if (!sset_is_empty(&inports) && !sset_is_empty(&resident_ports)) {
- const char *item;
+ const char *port_name;
- SSET_FOR_EACH (item, &inports) {
+ SSET_FOR_EACH (port_name, &inports) {
ds_clear(&match);
- ds_put_format(&match,
- "(arp.op == 1 || arp.op == 2) && inport == %s",
- item);
+ ds_put_format(&match, "(arp.op == 1 || arp.op == 2) "
+ "&& inport == %s",
+ port_name);
ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 75,
ds_cstr(&match), REGBIT_EXT_ARP " = 1; next;",
ar->lflow_ref);
}
- SSET_FOR_EACH (item, &resident_ports) {
+ SSET_FOR_EACH (port_name, &resident_ports) {
ds_clear(&match);
- ds_put_format(&match,
- REGBIT_EXT_ARP" == 1 && is_chassis_resident(%s)",
- item);
+ ds_put_format(&match, REGBIT_EXT_ARP" == 1 "
+ "&& is_chassis_resident(%s)",
+ port_name);
ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 75,
ds_cstr(&match), "next;", ar->lflow_ref);
}
- SSET_FOR_EACH (item, &distributed_nat_ports) {
+ SSET_FOR_EACH (port_name, &distributed_nat_ports) {
ds_clear(&match);
- ds_put_format(&match,
- REGBIT_EXT_ARP
- " == 1 && is_chassis_resident(\"%s\")",
- item);
+ ds_put_format(&match, REGBIT_EXT_ARP " == 1 "
+ "&& is_chassis_resident(\"%s\")",
+ port_name);
ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 75,
ds_cstr(&match), "next;", ar->lflow_ref);
}
@@ -9782,10 +9776,10 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
REGBIT_EXT_ARP" == 1", "drop;", ar->lflow_ref);
}
- ds_destroy(&match);
- sset_destroy(&inports);
- sset_destroy(&resident_ports);
sset_destroy(&distributed_nat_ports);
+ sset_destroy(&resident_ports);
+ sset_destroy(&inports);
+ ds_destroy(&match);
}
static bool
@@ -19802,8 +19796,7 @@ lflow_handle_ls_arp_changes(struct ovsdb_idl_txn
*ovnsb_txn,
bool handled = lflow_ref_sync_lflows(
ls_arp_record->lflow_ref, lflows, ovnsb_txn,
- lflow_input->ls_datapaths,
- lflow_input->lr_datapaths,
+ lflow_input->ls_datapaths, lflow_input->lr_datapaths,
lflow_input->ovn_internal_version_changed,
lflow_input->sbrec_logical_flow_table,
lflow_input->sbrec_logical_dp_group_table);
diff --git a/northd/northd.h b/northd/northd.h
index b5f1fac389..2869ea97e4 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -474,7 +474,7 @@ struct ovn_datapath {
struct lrouter_group *lr_group;
/* Set of localnet or l2gw ports. */
- struct hmapx ph_ports;
+ struct hmapx phys_ports;
/* Map of ovn_port objects belonging to this datapath.
* This map doesn't include derived ports. */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 0018fb0efa..380c9ff802 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -18816,14 +18816,14 @@ check ovn-nbctl set Logical_Switch_Port up_link \
check ovn-nbctl lsp-set-addresses down_vif1 'f0:00:00:00:00:01 192.168.1.101'
check ovn-nbctl lsp-set-addresses down_vif2 'f0:00:00:00:00:02 192.168.1.102'
check ovn-nbctl lrp-set-gateway-chassis down_link hv1
-
+check ovn-nbctl --wait=sb sync
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
])
# Check localnet port addings trigger ls-arp flow
-check ovn-nbctl lsp-set-type down_ext localnet
+check ovn-nbctl --wait=sb lsp-set-type down_ext localnet
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
@@ -18831,9 +18831,10 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
ls_in_apply_port_sec | ovn_strip_lflow
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-down_link")), action=(next;)
])
-# Check nat adding to dgr attached to logical switch trigger ls-arp flow
+# Check nat adding to dgr attached to logical switch trigger ls-arp flow.
check ovn-nbctl lr-nat-add lr1 dnat_and_snat 192.168.0.4 10.0.0.4
check ovn-nbctl lr-nat-add lr1 dnat_and_snat 192.168.0.3 10.0.0.3 down_vif1
f0:00:00:00:00:03
+check ovn-nbctl --wait=sb sync
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
@@ -18842,7 +18843,7 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
ls_in_apply_port_sec | ovn_strip_lflow
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("down_vif1")), action=(next;)
])
-check ovn-nbctl lr-nat-del lr1 dnat_and_snat 192.168.0.3
+check ovn-nbctl --wait=sb lr-nat-del lr1 dnat_and_snat 192.168.0.3
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
@@ -18850,9 +18851,24 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
ls_in_apply_port_sec | ovn_strip_lflow
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-down_link")), action=(next;)
])
-# Check changinf logical port type
-check ovn-nbctl lsp-set-type down_ext l2gateway
+# Check changing logical port type to l2gateway.
+check ovn-nbctl --wait=sb lsp-set-type down_ext l2gateway
+AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
+ table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
action=(drop;)
+ table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-down_link")), action=(next;)
+])
+# Check changing logical port type to vif.
+check ovn-nbctl --wait=sb lsp-set-type down_ext ''
+AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
+])
+
+# Check changing logical port type back to localnet.
+check ovn-nbctl --wait=sb lsp-set-type down_ext localnet
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
@@ -18860,11 +18876,12 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
ls_in_apply_port_sec | ovn_strip_lflow
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-down_link")), action=(next;)
])
-check ovn-nbctl lsp-del down_ext
+# Check changing removing logical port.
+check ovn-nbctl --wait=sb lsp-del down_ext
AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_in_apply_port_sec |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_apply_port_sec), priority=0 , match=(1), action=(next;)
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
])
AT_CLEANUP
-])
\ No newline at end of file
+])
diff --git a/tests/ovn.at b/tests/ovn.at
index 113d64229f..6ea0d1de50 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -43896,13 +43896,13 @@ ovn_start
# set lr_down: gateway port (chassis redirect) bound to hv1
# LS1: down_vif1 - vif port bound to hv1
# down_vif2 - vif port bound to hv2
-# down_ext - outher port will be iteratred as localnet, l2gateway
+# down_ext - outer (port will be iterated as localnet, l2gateway)
#
-# Test: throw ARP annonce request from vitrual ports (down_vif1, down_vif2)
+# Test: send GARP request from virtual ports (down_vif1, down_vif2)
# ensure mac_binding is always updated.
# (Fixing the issue: mac_binding is only updated for packets came from
# down_link's resident chassis)
-# throw ARP annonce request from from localnet.
+# send GARP request from from localnet.
# ensure mac_binding is updated only if localnet bound to same hv as
l3dgw
check ovn-nbctl lr-add lr1
@@ -43978,6 +43978,7 @@ as hv2 ovs-appctl netdev-dummy/receive vif_ext $garp
check ovn-nbctl lrp-set-gateway-chassis down_link hv2
wait_row_count Port_Binding 1 logical_port=cr-down_link 'chassis!=[[]]'
+check ovn-nbctl --wait=hv sync
sha=02:00:00:00:00:ee
tha=00:00:00:00:00:00
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
