Commit [1] propagated other_config:is-vtep from the SB Chassis to the local tunnel Interface and read it back from there. That doesn't survive an ovn-controller upgrade: tunnel_add() sets other_config only when it creates a new interface, while the path that reuses an existing tunnel compares just the interface type and options. After an upgrade the tunnel ports already exist with unchanged options, so the flag is never added to them and chassis_tunnel->is_ramp_tunnel stays false until the tunnel happens to be recreated for some other reason. As a result, ICMP "fragmentation needed" packets arriving from RAMP tunnels kept being dropped on upgraded chassis. Instead, look the chassis up in the SB database by the name encoded in the tunnel id and read other_config:is-vtep directly from there. This doesn't depend on any locally stored state, so it works for tunnels created by older versions as well.
[1] https://github.com/ovn-org/ovn/commit/3391e61cdcd55ba4d11b30a096ba5aee3e435994 Fixes: 3391e61cdcd5 ("controller: Skip frag-needed handling for VTEP ICMP packets.") Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- controller/encaps.c | 9 --------- controller/local_data.c | 8 +++++++- controller/ovn-controller.c | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/controller/encaps.c b/controller/encaps.c index 048e85c38..61ae55965 100644 --- a/controller/encaps.c +++ b/controller/encaps.c @@ -43,7 +43,6 @@ encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name); ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_type); ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options); - ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_other_config); } /* Enough context to create a new tunnel, using tunnel_add(). */ @@ -208,7 +207,6 @@ tunnel_add(struct tunnel_ctx *tc, const struct ovsrec_open_vswitch_table *ovs_table) { struct smap options = SMAP_INITIALIZER(&options); - struct smap other_config = SMAP_INITIALIZER(&other_config); smap_add(&options, "remote_ip", encap->ip); smap_add(&options, "local_ip", local_ip); smap_add(&options, "key", "flow"); @@ -286,11 +284,6 @@ tunnel_add(struct tunnel_ctx *tc, } } - if (is_ramp_tunnel(&chassis_rec->other_config)) { - /* Propagate ramp switch flag from chassis to interface. */ - smap_add(&other_config, "is-vtep", "true"); - } - /* If there's an existing tunnel record that does not need any change, * keep it. Otherwise, create a new record (if there was an existing * record, the new record will supplant it and encaps_run() will delete @@ -338,7 +331,6 @@ tunnel_add(struct tunnel_ctx *tc, ovsrec_interface_set_name(iface, port_name); ovsrec_interface_set_type(iface, encap->type); ovsrec_interface_set_options(iface, &options); - ovsrec_interface_set_other_config(iface, &other_config); struct ovsrec_port *port = ovsrec_port_insert(tc->ovs_txn); ovsrec_port_set_name(port, port_name); @@ -354,7 +346,6 @@ exit: free(tunnel_entry_id); free(tunnel_entry_id_old); smap_destroy(&options); - smap_destroy(&other_config); } static bool diff --git a/controller/local_data.c b/controller/local_data.c index af6c75b40..02c8563fe 100644 --- a/controller/local_data.c +++ b/controller/local_data.c @@ -30,6 +30,7 @@ #include "lport.h" #include "lib/ovn-util.h" #include "lib/ovn-sb-idl.h" +#include "lib/chassis-index.h" #include "local_data.h" #include "lport.h" @@ -457,6 +458,7 @@ tracked_datapaths_destroy(struct hmap *tracked_datapaths) void local_nonvif_data_run(const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis_rec, + struct ovsdb_idl_index *sbrec_chassis_by_name, struct simap *patch_ofports, struct hmap *chassis_tunnels, struct flow_based_tunnel *flow_tunnels) @@ -525,6 +527,8 @@ local_nonvif_data_run(const struct ovsrec_bridge *br_int, if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip, NULL)) { continue; } + const struct sbrec_chassis *chassis = + chassis_lookup_by_name(sbrec_chassis_by_name, hash_id); struct chassis_tunnel *tun = xmalloc(sizeof *tun); hmap_insert(chassis_tunnels, &tun->hmap_node, hash_string(hash_id, 0)); @@ -532,7 +536,9 @@ local_nonvif_data_run(const struct ovsrec_bridge *br_int, tun->ofport = u16_to_ofp(ofport); tun->type = tunnel_type; tun->is_ipv6 = ip ? addr_is_ipv6(ip) : false; - tun->is_ramp_tunnel = is_ramp_tunnel(&iface_rec->other_config); + tun->is_ramp_tunnel = chassis ? + is_ramp_tunnel(&chassis->other_config, + "is-vtep", false) : false; free(hash_id); free(ip); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6551a4e30..1e9639216 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -3730,7 +3730,7 @@ en_non_vif_data_run(struct engine_node *node, void *data) ed_non_vif_data->use_flow_based_tunnels = is_flow_based_tunnels_enabled(ovs_table, chassis); - local_nonvif_data_run(br_int, chassis, + local_nonvif_data_run(br_int, chassis, sbrec_chassis_by_name, &ed_non_vif_data->patch_ofports, &ed_non_vif_data->chassis_tunnels, ed_non_vif_data->flow_tunnels); -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
