On Wed, Apr 9, 2025 at 3:00 PM Han Zhou <hz...@ovn.org> wrote:
>
>
>
> On Thu, Apr 3, 2025 at 7:45 PM <num...@ovn.org> wrote:
> >
> > From: Numan Siddique <num...@ovn.org>
> >
> > Expand the helper lport_get_peer() to get the peer for both
> > 'patch' and 'l3gateway' port binding types and add a new
> > helper lport_get_patch_peer() for 'patch' type.
> >
> > This cleans up the code a bit.
> >
> > Signed-off-by: Numan Siddique <num...@ovn.org>
> > ---
> > controller/binding.c | 2 +-
> > controller/local_data.c | 44 ++++++++++++++---------------------------
> > controller/lport.c | 10 ++++++++++
> > controller/lport.h | 3 +++
> > 4 files changed, 29 insertions(+), 30 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 470aa8c536..b657de9e44 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -2970,7 +2970,7 @@ consider_patch_port_for_local_datapaths(const struct
> > sbrec_port_binding *pb,
> > * */
> > const struct sbrec_port_binding *peer;
> > struct local_datapath *peer_ld = NULL;
> > - peer = lport_get_peer(pb, b_ctx_in->sbrec_port_binding_by_name);
> > + peer = lport_get_patch_peer(pb,
> > b_ctx_in->sbrec_port_binding_by_name);
> > if (peer) {
> > peer_ld =
> > get_local_datapath(b_ctx_out->local_datapaths,
> > diff --git a/controller/local_data.c b/controller/local_data.c
> > index 5383f49cf8..d37ea1488b 100644
> > --- a/controller/local_data.c
> > +++ b/controller/local_data.c
> > @@ -208,12 +208,8 @@ add_local_datapath_peer_port(
> > struct hmap *local_datapaths,
> > struct hmap *tracked_datapaths)
> > {
> > - const struct sbrec_port_binding *peer;
> > - if (!strcmp(pb->type, "l3gateway")) {
> > - peer = lport_get_l3gw_peer(pb, sbrec_port_binding_by_name);
> > - } else {
> > - peer = lport_get_peer(pb, sbrec_port_binding_by_name);
> > - }
> > + const struct sbrec_port_binding *peer =
> > + lport_get_peer(pb, sbrec_port_binding_by_name);
> >
> > if (!peer) {
> > return;
> > @@ -630,29 +626,19 @@ add_local_datapath__(struct ovsdb_idl_index
> > *sbrec_datapath_binding_by_key,
> > const struct sbrec_port_binding *pb;
> > SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
> > sbrec_port_binding_by_datapath) {
> > - if (!strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) {
> > - const char *peer_name = smap_get(&pb->options, "peer");
> > - if (peer_name) {
> > - const struct sbrec_port_binding *peer;
> > -
> > - peer = lport_lookup_by_name(sbrec_port_binding_by_name,
> > - peer_name);
> > -
> > - if (peer && peer->datapath) {
> > - if (need_add_peer_to_local(
> > - sbrec_port_binding_by_name, pb, chassis)) {
> > - struct local_datapath *peer_ld =
> > -
> > add_local_datapath__(sbrec_datapath_binding_by_key,
> > -
> > sbrec_port_binding_by_datapath,
> > - sbrec_port_binding_by_name,
> > - depth + 1, peer->datapath,
> > - chassis, local_datapaths,
> > - tracked_datapaths);
> > - local_datapath_peer_port_add(peer_ld, peer, pb);
> > - local_datapath_peer_port_add(ld, pb, peer);
> > - }
> > - }
> > - }
> > + const struct sbrec_port_binding *peer =
> > + lport_get_peer(pb, sbrec_port_binding_by_name);
> > + if (peer && need_add_peer_to_local(sbrec_port_binding_by_name,
>
> Could you double confirm it is safe to remove the condition "peer->datapath"?
> Otherwise:
>
I can confirm that it is fine. The function get_peer_lport() which is
internally called by
lport_get_peer() checks that peer->datapath is not NULL before returning peer.
Thanks
Numan
> Acked-by: Han Zhou <hz...@ovn.org>
>
> > + pb, chassis)) {
> > + struct local_datapath *peer_ld =
> > + add_local_datapath__(sbrec_datapath_binding_by_key,
> > + sbrec_port_binding_by_datapath,
> > + sbrec_port_binding_by_name,
> > + depth + 1, peer->datapath,
> > + chassis, local_datapaths,
> > + tracked_datapaths);
> > + local_datapath_peer_port_add(peer_ld, peer, pb);
> > + local_datapath_peer_port_add(ld, pb, peer);
> > }
> > }
> > sbrec_port_binding_index_destroy_row(target);
> > diff --git a/controller/lport.c b/controller/lport.c
> > index d8ba7ca164..92de375b5e 100644
> > --- a/controller/lport.c
> > +++ b/controller/lport.c
> > @@ -116,6 +116,16 @@ lport_is_local(struct ovsdb_idl_index
> > *sbrec_port_binding_by_name,
> > const struct sbrec_port_binding *
> > lport_get_peer(const struct sbrec_port_binding *pb,
> > struct ovsdb_idl_index *sbrec_port_binding_by_name)
> > +{
> > + const struct sbrec_port_binding *peer =
> > + lport_get_patch_peer(pb, sbrec_port_binding_by_name);
> > +
> > + return peer ? peer : lport_get_l3gw_peer(pb,
> > sbrec_port_binding_by_name);
> > +}
> > +
> > +const struct sbrec_port_binding *
> > +lport_get_patch_peer(const struct sbrec_port_binding *pb,
> > + struct ovsdb_idl_index *sbrec_port_binding_by_name)
> > {
> > if (strcmp(pb->type, "patch")) {
> > return NULL;
> > diff --git a/controller/lport.h b/controller/lport.h
> > index 8463c96232..8b1809a275 100644
> > --- a/controller/lport.h
> > +++ b/controller/lport.h
> > @@ -75,6 +75,9 @@ bool lport_is_local(struct ovsdb_idl_index
> > *sbrec_port_binding_by_name,
> > const struct sbrec_port_binding *lport_get_peer(
> > const struct sbrec_port_binding *,
> > struct ovsdb_idl_index *sbrec_port_binding_by_name);
> > +const struct sbrec_port_binding *lport_get_patch_peer(
> > + const struct sbrec_port_binding *,
> > + struct ovsdb_idl_index *sbrec_port_binding_by_name);
> > const struct sbrec_port_binding *lport_get_l3gw_peer(
> > const struct sbrec_port_binding *,
> > struct ovsdb_idl_index *sbrec_port_binding_by_name);
> > --
> > 2.49.0
> >
> > _______________________________________________
> > 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
