On Tue, Feb 04, 2025 at 12:09:22AM +0100, Lorenzo Bianconi wrote:
> On Jan 21, 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.
> >
> > Signed-off-by: Felix Huettner <[email protected]>
>
> Acked-by: Lorenzo Bianconi <[email protected]>
Hi Lorenzo,
sorry i missed the mail before sending out v6. I'll add it once i send a v7.
Thanks a lot,
Felix
>
> > ---
> > v2->v3:
> > * A lot of minor review comments.
> >
> > northd/en-learned-route-sync.c | 8 +++++++-
> > northd/northd.c | 34 ----------------------------------
> > northd/northd.h | 34 ++++++++++++++++++++++++++++++++++
> > tests/ovn-northd.at | 16 ++++++++++++++++
> > 4 files changed, 57 insertions(+), 35 deletions(-)
> >
> > diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
> > index 048b807ed..a144b4487 100644
> > --- a/northd/en-learned-route-sync.c
> > +++ b/northd/en-learned-route-sync.c
> > @@ -197,7 +197,13 @@ routes_table_sync(
> > const struct parsed_route *route;
> >
> > const struct sbrec_learned_route *sb_route;
> > - SBREC_LEARNED_ROUTE_TABLE_FOR_EACH (sb_route,
> > sbrec_learned_route_table) {
> > + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE (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 0eff92125..baffa7c90 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1297,34 +1297,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)
> > {
> > @@ -1339,12 +1311,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
> > diff --git a/northd/northd.h b/northd/northd.h
> > index bef7d3778..6d0e7998a 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -913,4 +913,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 885e6996a..0f146ee7f 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14661,6 +14661,22 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
> > ovn_strip_lflows], [0], [dnl
> > table=??(lr_in_ip_routing ), priority=518 , match=(ip6.dst ==
> > 2001:db8:ffff::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = ip6.dst;
> > xxreg1 = 2001:db8:ffff::1; 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 2
> > +check_row_count Learned_Route 2 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=514 , match=(reg7 == 0 &&
> > ip6.dst == 2001:db8:3::/64), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
> > 10.0.0.20; 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
