> On Thu, 2025-02-13 at 14:45 +0100, Dumitru Ceara wrote:
> > On 2/13/25 2:33 PM, Martin Kalcok wrote:
> > > From: Frode Nordahl <[email protected]>
> > >
> > > While IPv6 and NAT does not ring particularly well together in my
> > > mind, it is a supported feature.
> > >
> > > We need this function to allow them in a subsequent patch adding
> > > host route exchange for NAT addresses.
> > >
> > > Signed-off-by: Frode Nordahl <[email protected]>
> > > Signed-off-by: Martin Kalcok <[email protected]>
> > > ---
> >
> > Hi Martin, Frode,
> >
> > Is this patch still needed?  I don't think we use get_nat_addresses()
> > anymore, do we?
> >

[ edited Martin's likely oversight of doing a top-post to its intended
position ]

On Thu, Feb 13, 2025 at 3:10 PM <[email protected]> wrote:
>
> I guess technically it's not needed. I carried it over because it's a
> small and potentially useful change. However I also see a point in not
> bloating the code with unused feature, so feel free to drop it.

This patch was indeed part of the original proposal, and looking
through patch 2-4 it does indeed not appear to be used anymore.

Mailing lists are good archives for unused ideas, so I see no need to
keep it. Not having to excuse the use of the words "IPv6" and "NAT" in
the same commit message is also a bonus! :)

Dumitru: Do tell if you want a respin to get rid of it!

--
Frode Nordahl

> > Regards,
> > Dumitru
> >
> > >  northd/northd.c | 22 +++++++++++++++++-----
> > >  1 file changed, 17 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index 1097bb159..9fa627db7 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -1200,7 +1200,8 @@ destroy_routable_addresses(struct
> > > ovn_port_routable_addresses *ra)
> > >
> > >  static char **get_nat_addresses(const struct ovn_port *op, size_t
> > > *n,
> > >                                  bool routable_only, bool
> > > include_lb_ips,
> > > -                                const struct lr_stateful_record
> > > *);
> > > +                                const struct lr_stateful_record *,
> > > +                                bool allow_ipv6);
> > >
> > >  struct ovn_port_routable_addresses
> > >  get_op_addresses(const struct ovn_port *op,
> > > @@ -1209,7 +1210,7 @@ get_op_addresses(const struct ovn_port *op,
> > >  {
> > >      size_t n;
> > >      char **nats = get_nat_addresses(op, &n, routable_only, true,
> > > -                                    lr_stateful_rec);
> > > +                                    lr_stateful_rec, false);
> > >
> > >      if (!nats) {
> > >          return (struct ovn_port_routable_addresses) {
> > > @@ -2627,7 +2628,8 @@ join_logical_ports(const struct
> > > sbrec_port_binding_table *sbrec_pb_table,
> > >  static char **
> > >  get_nat_addresses(const struct ovn_port *op, size_t *n, bool
> > > routable_only,
> > >                    bool include_lb_ips,
> > > -                  const struct lr_stateful_record
> > > *lr_stateful_rec)
> > > +                  const struct lr_stateful_record
> > > *lr_stateful_rec,
> > > +                  bool allow_ipv6)
> > >  {
> > >      size_t n_nats = 0;
> > >      struct eth_addr mac;
> > > @@ -2650,6 +2652,7 @@ get_nat_addresses(const struct ovn_port *op,
> > > size_t *n, bool routable_only,
> > >      for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
> > >          const struct nbrec_nat *nat = op->od->nbr->nat[i];
> > >          ovs_be32 ip, mask;
> > > +        struct in6_addr ip6, mask6;
> > >
> > >          if (routable_only &&
> > >              (!strcmp(nat->type, "snat") ||
> > > @@ -2660,7 +2663,15 @@ get_nat_addresses(const struct ovn_port *op,
> > > size_t *n, bool routable_only,
> > >          char *error = ip_parse_masked(nat->external_ip, &ip,
> > > &mask);
> > >          if (error || mask != OVS_BE32_MAX) {
> > >              free(error);
> > > -            continue;
> > > +            if (allow_ipv6) {
> > > +                error = ipv6_parse_masked(nat->external_ip, &ip6,
> > > &mask6);
> > > +                if (error || ipv6_count_cidr_bits(&mask6) != 128)
> > > {
> > > +                    free(error);
> > > +                    continue;
> > > +                }
> > > +            } else {
> > > +                continue;
> > > +            }
> > >          }
> > >
> > >          /* Not including external IP of NAT rules whose
> > > gateway_port is
> > > @@ -4049,7 +4060,8 @@ sync_pb_for_lsp(struct ovn_port *op,
> > >                          lr_stateful_table, op->peer->od->index);
> > >                  }
> > >                  nats = get_nat_addresses(op->peer, &n_nats, false,
> > > -                                         include_lb_vips,
> > > lr_stateful_rec);
> > > +                                         include_lb_vips,
> > > lr_stateful_rec,
> > > +                                         false);
> > >              }
> > >          } else if (nat_addresses && (chassis || l3dgw_ports)) {
> > >              struct lport_addresses laddrs;
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to