On Tue, Aug 27, 2024 at 3:49 PM Xavier Simonart <[email protected]> wrote:
> Do not allocate ct_zones for virtual ports. > > Signed-off-by: Xavier Simonart <[email protected]> > --- > Hi Xavier, I have one comment down below. controller/ct-zone.c | 11 +++++++++-- > controller/ct-zone.h | 3 ++- > controller/ovn-controller.c | 5 ++++- > tests/ovn.at | 26 +++++++++++++++++++++++++- > 4 files changed, 40 insertions(+), 5 deletions(-) > > diff --git a/controller/ct-zone.c b/controller/ct-zone.c > index 77eb16ac9..2385b700d 100644 > --- a/controller/ct-zone.c > +++ b/controller/ct-zone.c > @@ -19,6 +19,7 @@ > #include "binding.h" > #include "chassis.h" > #include "ct-zone.h" > +#include "lport.h" > #include "local_data.h" > #include "openvswitch/vlog.h" > > @@ -169,7 +170,8 @@ out: > void > ct_zones_update(const struct sset *local_lports, > const struct ovsrec_open_vswitch_table *ovs_table, > - const struct hmap *local_datapaths, struct ct_zone_ctx > *ctx) > + const struct hmap *local_datapaths, struct ct_zone_ctx > *ctx, > + struct ovsdb_idl_index *sbrec_port_binding_by_name) > { > int min_ct_zone, max_ct_zone; > const char *user; > @@ -179,8 +181,13 @@ ct_zones_update(const struct sset *local_lports, > struct simap unreq_snat_zones = SIMAP_INITIALIZER(&unreq_snat_zones); > > const char *local_lport; > + const struct sbrec_port_binding *pb; > SSET_FOR_EACH (local_lport, local_lports) { > - sset_add(&all_users, local_lport); > + pb = lport_lookup_by_name(sbrec_port_binding_by_name, > + local_lport); > + if (!pb || strcmp(pb->type, "virtual")) { > + sset_add(&all_users, local_lport); > + } > I don't like the fact that we would have to do a lookup for each local port. Could we utilize "struct local_binding_data" as we do for "ct_zones_limits_sync()"? } > > /* Local patched datapath (gateway routers) need zones assigned. */ > diff --git a/controller/ct-zone.h b/controller/ct-zone.h > index 6df03975c..fcb100619 100644 > --- a/controller/ct-zone.h > +++ b/controller/ct-zone.h > @@ -73,7 +73,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, > void ct_zones_update(const struct sset *local_lports, > const struct ovsrec_open_vswitch_table *ovs_table, > const struct hmap *local_datapaths, > - struct ct_zone_ctx *ctx); > + struct ct_zone_ctx *ctx, > + struct ovsdb_idl_index *sbrec_port_binding_by_name); > void ct_zones_commit(const struct ovsrec_bridge *br_int, > const struct ovsrec_datapath *ovs_dp, > struct ovsdb_idl_txn *ovs_idl_txn, > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index a10f1af8a..911c7f162 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1174,6 +1174,7 @@ struct ed_type_runtime_data { > struct shash local_active_ports_ras; > > struct sset *postponed_ports; > + struct ovsdb_idl_index *sbrec_port_binding_by_name; > }; > > /* struct ed_type_runtime_data has the below members for tracking the > @@ -1438,6 +1439,7 @@ en_runtime_data_run(struct engine_node *node, void > *data) > > binding_run(&b_ctx_in, &b_ctx_out); > rt_data->localnet_learn_fdb = b_ctx_out.localnet_learn_fdb; > + rt_data->sbrec_port_binding_by_name = > b_ctx_in.sbrec_port_binding_by_name; > > engine_set_node_state(node, EN_UPDATED); > } > @@ -2230,7 +2232,8 @@ en_ct_zones_run(struct engine_node *node, void *data) > > ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); > ct_zones_update(&rt_data->local_lports, ovs_table, > - &rt_data->local_datapaths, &ct_zones_data->ctx); > + &rt_data->local_datapaths, &ct_zones_data->ctx, > + rt_data->sbrec_port_binding_by_name); > ct_zones_limits_sync(&ct_zones_data->ctx, &rt_data->local_datapaths, > &rt_data->lbinding_data.lports); > > diff --git a/tests/ovn.at b/tests/ovn.at > index 4d9beea1b..ff7281e59 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -38951,8 +38951,29 @@ check ovn-nbctl lrp-add r1 r1-ls2 > f0:00:00:00:00:02 192.168.3.1/24 \ > -- lsp-add ls2 ls2-r1 \ > -- set Logical_Switch_Port ls2-r1 type=router > options:router-port=r1-ls2 addresses=router > > +check ovn-nbctl lsp-add ls1 ls1-vir \ > + -- lsp-set-addresses ls1-vir "50:54:00:00:00:10 > 192.168.2.2" \ > + -- lsp-set-type ls1-vir virtual \ > + -- set logical_switch_port ls1-vir > options:virtual-ip=192.168.2.2 \ > + -- set logical_switch_port ls1-vir > options:virtual-parents=lsp1 > + > check ovn-nbctl --wait=hv sync > > +send_garp() { > + hv=$1 > + dev=$2 > + eth_src=$3 > + ip=$4 > + > + packet=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${eth_src}')/ \ > + ARP(op=2, hwsrc='${eth_src}', hwdst='${eth_src}', \ > + psrc='${ip}', pdst='${ip}')") > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > +} > + > +# Sending garp to claim virtual port. > +send_garp hv1 vif1 "50:54:00:00:00:03" 192.168.2.2 > + > AS_BOX([With patch ports]) > zone_list_hv1=$(as hv1 ovn-appctl -t ovn-controller ct-zone-list | sort) > AT_CHECK([echo "$zone_list_hv1" | sed "s/ [[0-9]]*/ ??/" | sort], [0], > [dnl > @@ -39062,7 +39083,10 @@ zone_list_hv2_after_recompute=$(as hv2 ovn-appctl > -t ovn-controller ct-zone-list > AT_CHECK([test "X$zone_list_hv1" = "X$zone_list_hv1_after_recompute"]) > AT_CHECK([test "X$zone_list_hv2" = "X$zone_list_hv2_after_recompute"]) > > -OVN_CLEANUP([hv1],[hv2]) > + > +OVN_CLEANUP([hv1 > +/parse MAC binding/d > +],[hv2]) > > AT_CLEANUP > ]) > -- > 2.31.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
