On Wed, Dec 18, 2024 at 04:57:06PM +0100, Felix Huettner via dev wrote:
> On Wed, Dec 18, 2024 at 11:24:59AM +0100, Felix Huettner via dev wrote:
> > learned routes must be bound to a lrp on which the routes where learned.
> > In case the lrp is deleted for whatever reason no ovn-controller would
> > clean these routes up, therefor we do this in northd.
> > 
> 
> Recheck-request: github-robot

I am not sure what has happened here. The previous run did not actually
recheck Build_and_Test, but reused the previously failed one.
lets see if another try helps.

Recheck-request: github-robot

> 
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> >  northd/en-learned-route-sync.c |  5 +++++
> >  northd/northd.c                | 35 +---------------------------------
> >  northd/northd.h                | 34 +++++++++++++++++++++++++++++++++
> >  tests/ovn-northd.at            | 15 +++++++++++++++
> >  4 files changed, 55 insertions(+), 34 deletions(-)
> > 
> > diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
> > index 962ccd10e..7ec9f48db 100644
> > --- a/northd/en-learned-route-sync.c
> > +++ b/northd/en-learned-route-sync.c
> > @@ -205,6 +205,11 @@ routes_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
> >  
> >      const struct sbrec_learned_route *sb_route;
> >      SBREC_LEARNED_ROUTE_TABLE_FOR_EACH (sb_route, 
> > sbrec_learned_route_table) {
> > +        if (!ovn_port_find(lr_ports,
> > +                           sb_route->logical_port->logical_port)) {
> > +            sbrec_learned_route_delete(sb_route);
> > +            continue;
> > +        }
> >          parse_route_from_sbrec_route(parsed_routes_out, lr_ports,
> >                                       &lr_datapaths->datapaths,
> >                                       sb_route);
> > diff --git a/northd/northd.c b/northd/northd.c
> > index b003d4791..75519e734 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1289,34 +1289,6 @@ ovn_port_destroy(struct hmap *ports, struct ovn_port 
> > *port)
> >      }
> >  }
> >  
> > -/* Returns the ovn_port that matches 'name'.  If 'prefer_bound' is true and
> > - * multiple ports share the same name, gives precendence to ports bound to
> > - * an ovn_datapath.
> > - */
> > -static struct ovn_port *
> > -ovn_port_find__(const struct hmap *ports, const char *name,
> > -                bool prefer_bound)
> > -{
> > -    struct ovn_port *matched_op = NULL;
> > -    struct ovn_port *op;
> > -
> > -    HMAP_FOR_EACH_WITH_HASH (op, key_node, hash_string(name, 0), ports) {
> > -        if (!strcmp(op->key, name)) {
> > -            matched_op = op;
> > -            if (!prefer_bound || op->od) {
> > -                return op;
> > -            }
> > -        }
> > -    }
> > -    return matched_op;
> > -}
> > -
> > -static struct ovn_port *
> > -ovn_port_find(const struct hmap *ports, const char *name)
> > -{
> > -    return ovn_port_find__(ports, name, false);
> > -}
> > -
> >  static bool
> >  lsp_is_clone_to_unknown(const struct nbrec_logical_switch_port *nbsp)
> >  {
> > @@ -1331,12 +1303,6 @@ lsp_is_clone_to_unknown(const struct 
> > nbrec_logical_switch_port *nbsp)
> >      return false;
> >  }
> >  
> > -static struct ovn_port *
> > -ovn_port_find_bound(const struct hmap *ports, const char *name)
> > -{
> > -    return ovn_port_find__(ports, name, true);
> > -}
> > -
> >  /* Returns true if the logical switch port 'enabled' column is empty or
> >   * set to true.  Otherwise, returns false. */
> >  static bool
> > @@ -3410,6 +3376,7 @@ cleanup_mac_bindings(
> >      }
> >  }
> >  
> > +
> >  static void
> >  cleanup_sb_ha_chassis_groups(
> >      const struct sbrec_ha_chassis_group_table 
> > *sbrec_ha_chassis_group_table,
> > diff --git a/northd/northd.h b/northd/northd.h
> > index 385a46ade..9b80f422d 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -904,4 +904,38 @@ is_vxlan_mode(const struct smap *nb_options,
> >  
> >  uint32_t get_ovn_max_dp_key_local(bool _vxlan_mode);
> >  
> > +/* Returns the ovn_port that matches 'name'.  If 'prefer_bound' is true and
> > + * multiple ports share the same name, gives precendence to ports bound to
> > + * an ovn_datapath.
> > + */
> > +static struct ovn_port *
> > +ovn_port_find__(const struct hmap *ports, const char *name,
> > +                bool prefer_bound)
> > +{
> > +    struct ovn_port *matched_op = NULL;
> > +    struct ovn_port *op;
> > +
> > +    HMAP_FOR_EACH_WITH_HASH (op, key_node, hash_string(name, 0), ports) {
> > +        if (!strcmp(op->key, name)) {
> > +            matched_op = op;
> > +            if (!prefer_bound || op->od) {
> > +                return op;
> > +            }
> > +        }
> > +    }
> > +    return matched_op;
> > +}
> > +
> > +static inline struct ovn_port *
> > +ovn_port_find(const struct hmap *ports, const char *name)
> > +{
> > +    return ovn_port_find__(ports, name, false);
> > +}
> > +
> > +static inline struct ovn_port *
> > +ovn_port_find_bound(const struct hmap *ports, const char *name)
> > +{
> > +    return ovn_port_find__(ports, name, true);
> > +}
> > +
> >  #endif /* NORTHD_H */
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index b1c88fd8e..1dd2613c3 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14589,6 +14589,21 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> >    table=??(lr_in_ip_routing   ), priority=518  , match=(inport == 
> > "lr0-sw1" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff02; eth.src = 
> > 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1; reg9[[9]] = 0; 
> > next;)
> >  ])
> >  
> > +# deleting lr0-sw1 will remove the flows and also the learned route
> > +check ovn-nbctl --wait=sb lrp-del lr0-sw1
> > +check_row_count Advertised_Route 2
> > +check_row_count Learned_Route 1
> > +check_row_count Learned_Route 1 logical_port=$sw0
> > +ovn-sbctl dump-flows lr0 > lr0flows
> > +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], 
> > [dnl
> > +  table=??(lr_in_ip_routing   ), priority=0    , match=(1), action=(drop;)
> > +  table=??(lr_in_ip_routing   ), priority=10550, match=(nd_rs || nd_ra), 
> > action=(drop;)
> > +  table=??(lr_in_ip_routing   ), priority=194  , match=(reg7 == 0 && 
> > ip4.dst == 172.16.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 10.0.0.11; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = 
> > "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=196  , match=(reg7 == 0 && 
> > ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 10.0.0.10; reg5 = 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = 
> > "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=198  , match=(ip4.dst == 
> > 10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 = 
> > 10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback 
> > = 1; reg9[[9]] = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=518  , match=(inport == 
> > "lr0-sw0" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:ff:fe00:ff01; eth.src = 
> > 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0; 
> > next;)
> > +])
> > +
> >  AT_CLEANUP
> >  ])
> >  
> > -- 
> > 2.47.1
> > 
> > 
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to