+Felix (explicitly) I'm not sure why mailman overwrites the "From" smtp header in your case, i.e.:
From: Felix Huettner via dev <[email protected]> +Frode and Martin (CC) On 1/14/25 12:32 PM, Dumitru Ceara wrote: > Hi Felix, > > Thanks for the patch! > > On 12/18/24 11:24 AM, Felix Huettner via dev wrote: >> learned routes must be bound to a lrp on which the routes where learned. > > Nit: 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]> >> --- >> 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); > > This is unsafe. Please use SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE() above. > >> + 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( >> } >> } >> >> + > > Unrelated newline addition. > >> 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 >> ]) >> > > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
