> Allow the l3gateway and patch port to be peers for connected LRPs, > previously it was only allowed to connect two ports of the same > type. This allows the CMS to connect any router with GW router > which is required for the concept of transit routers. > > Acked-by: Numan Siddique <num...@ovn.org> > Signed-off-by: Ales Musil <amu...@redhat.com>
Hi Ales, The patch is fine to me, just a nit inline (I guess they can be fixed applying the patch) Acked-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > --- > v3: Rebase on top of latest main. > Fix the wrong comparison in I-P handler. > Add ack from Numan. > v3: Rebase on top of latest main. > Add function to compare the types. > --- > controller/physical.c | 49 ++++++++++++++++++---- > tests/ovn-controller.at | 92 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 133 insertions(+), 8 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index 7fbdb424e..592af5303 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1066,9 +1066,28 @@ load_logical_ingress_metadata(const struct > sbrec_port_binding *binding, > } > } > > +static bool > +pbs_are_peers(const enum en_lport_type type_a, > + const enum en_lport_type type_b) > +{ > + /* Allow PBs of the same type to be peers. */ > + if (type_a == type_b) { > + return true; > + } > + > + /* Allow L3GW port to be peered with patch port. */ > + if ((type_a == LP_L3GATEWAY && type_b == LP_PATCH) || > + (type_a == LP_PATCH && type_b == LP_L3GATEWAY)) { > + return true; > + } > + > + return false; > +} > + > static const struct sbrec_port_binding * > get_binding_peer(struct ovsdb_idl_index *sbrec_port_binding_by_name, > - const struct sbrec_port_binding *binding) > + const struct sbrec_port_binding *binding, > + const enum en_lport_type type) > { > const char *peer_name = smap_get(&binding->options, "peer"); > if (!peer_name) { > @@ -1077,9 +1096,15 @@ get_binding_peer(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > > const struct sbrec_port_binding *peer = lport_lookup_by_name( > sbrec_port_binding_by_name, peer_name); > - if (!peer || strcmp(peer->type, binding->type)) { > + if (!peer) { > return NULL; > } > + > + enum en_lport_type peer_type = get_lport_type(peer); > + if (!pbs_are_peers(type, peer_type)) { > + return NULL; > + } > + > const char *peer_peer_name = smap_get(&peer->options, "peer"); > if (!peer_peer_name || strcmp(peer_peer_name, binding->logical_port)) { > return NULL; > @@ -1088,6 +1113,15 @@ get_binding_peer(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > return peer; > } > > +static bool > +physical_should_eval_peer_port(const struct sbrec_port_binding *binding, > + const struct sbrec_chassis *chassis, > + const enum en_lport_type type) > +{ > + return type == LP_PATCH || > + (type == LP_L3GATEWAY && binding->chassis == chassis); > +} > + > enum access_type { > PORT_LOCAL = 0, > PORT_LOCALNET, > @@ -1507,11 +1541,9 @@ consider_port_binding(const struct physical_ctx *ctx, > } > > struct match match; > - if (type == LP_PATCH || > - (type == LP_L3GATEWAY && binding->chassis == ctx->chassis)) { > - > + if (physical_should_eval_peer_port(binding, ctx->chassis, type)) { > const struct sbrec_port_binding *peer = get_binding_peer( > - ctx->sbrec_port_binding_by_name, binding); > + ctx->sbrec_port_binding_by_name, binding, type); > if (!peer) { > return; > } > @@ -2379,8 +2411,9 @@ physical_handle_flows_for_lport(const struct > sbrec_port_binding *pb, > physical_eval_port_binding(p_ctx, pb, type, flow_table); > } > > - const struct sbrec_port_binding *peer = type == LP_PATCH > - ? get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb) > + const struct sbrec_port_binding *peer = > + physical_should_eval_peer_port(pb, p_ctx->chassis, type) > + ? get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb, type) > : NULL; > if (peer) { > ofctrl_remove_flows(flow_table, &peer->header_.uuid); > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index 73bd50358..b1c57fc21 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -3554,3 +3554,95 @@ OVS_WAIT_FOR_OUTPUT([ovs-vsctl show | tail -n +2], [], > [dnl > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > AT_CLEANUP > + > +AT_SETUP([ovn-controller - LR peer ports combination]) > +AT_KEYWORDS([ovn]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.20 > + > +check ovn-nbctl ls-add ls0 > +check ovn-nbctl ls-add ls1 > + > +check ovn-nbctl lsp-add ls0 vif0 > +check ovn-nbctl lsp-add ls0 vif1 > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl lr-add lr1 > + > +check ovn-nbctl lrp-add lr0 lr0-ls0 00:00:00:00:20:00 192.168.20.1/24 > +check ovn-nbctl lsp-add ls0 ls0-lr0 \ > + -- lsp-set-type ls0-lr0 router \ > + -- lsp-set-addresses ls0-lr0 router \ > + -- lsp-set-options ls0-lr0 router-port=lr0-ls0 > + > +check ovn-nbctl lrp-add lr0 lr1-ls1 00:00:00:00:30:00 192.168.30.1/24 > +check ovn-nbctl lsp-add ls0 ls1-lr1 \ > + -- lsp-set-type ls1-lr1 router \ > + -- lsp-set-addresses ls1-lr1 router \ > + -- lsp-set-options ls1-lr1 router-port=lr1-ls1 > + > +check ovs-vsctl -- add-port br-int vif0 \ > + -- set Interface vif0 external-ids:iface-id=vif0 > + > +check ovs-vsctl -- add-port br-int vif1 \ > + -- set Interface vif1 external-ids:iface-id=vif1 > + > +check ovn-nbctl lrp-add lr0 lr0-lr1 00:00:00:00:30:01 192.168.30.1/31 > peer=lr1-lr0 > +check ovn-nbctl lrp-add lr1 lr1-lr0 00:00:00:00:30:02 192.168.30.2/31 > peer=lr0-lr1 > + > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +pb_cookie() { > + name=$1 > + fetch_column port_binding _uuid logical_port=$name |\ > + cut -d '-' -f 1 | tr -d '\n' | sed 's/^0\{0,8\}//' > +} > + > +lr0_peer_cookie="0x$(pb_cookie lr0-lr1)" > +lr1_peer_cookie="0x$(pb_cookie lr1-lr0)" > + > +# Patch to patch > +check_row_count Port_Binding 1 logical_port="lr0-lr1" type=patch > +check_row_count Port_Binding 1 logical_port="lr1-lr0" type=patch > +ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY > log_to_phy_flows > +AT_CHECK([grep -c "cookie=$lr0_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > +AT_CHECK([grep -c "cookie=$lr1_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > + > +# L3 gateway to patch I guess you mean connect L3GW to patch, right? I guess you could improve the comment :) > +check ovn-nbctl --wait=hv set Logical_Router lr0 options:chassis=hv1 > + > +check_row_count Port_Binding 1 logical_port="lr0-lr1" type=l3gateway > +check_row_count Port_Binding 1 logical_port="lr1-lr0" type=patch > +ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY > log_to_phy_flows > +AT_CHECK([grep -c "cookie=$lr0_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > +AT_CHECK([grep -c "cookie=$lr1_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > + > +# Patch to L3 gateway ditto > +check ovn-nbctl remove Logical_Router lr0 options chassis > +check ovn-nbctl --wait=hv set Logical_Router lr1 options:chassis=hv1 > + > +check_row_count Port_Binding 1 logical_port="lr0-lr1" type=patch > +check_row_count Port_Binding 1 logical_port="lr1-lr0" type=l3gateway > +ovs-ofctl dump-flows br-int table=OFTABLE_LOG_TO_PHY > log_to_phy_flows > +AT_CHECK([grep -c "cookie=$lr0_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > +AT_CHECK([grep -c "cookie=$lr1_peer_cookie," log_to_phy_flows], [0], [dnl > +1 > +]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > -- > 2.48.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
_______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev