Skip to content

Commit e1e74d2

Browse files
Sashhkaaalmusil
authored andcommitted
controller: Derive RAMP tunnel flag from the SB Chassis record.
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] 3391e61 Fixes: 3391e61 ("controller: Skip frag-needed handling for VTEP ICMP packets.") Signed-off-by: Alexandra Rukomoinikova <ARukomoinikova@k2.cloud> Signed-off-by: Ales Musil <amusil@redhat.com>
1 parent 38b0421 commit e1e74d2

4 files changed

Lines changed: 8 additions & 12 deletions

File tree

controller/encaps.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
4343
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
4444
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_type);
4545
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options);
46-
ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_other_config);
4746
}
4847

4948
/* Enough context to create a new tunnel, using tunnel_add(). */
@@ -208,7 +207,6 @@ tunnel_add(struct tunnel_ctx *tc,
208207
const struct ovsrec_open_vswitch_table *ovs_table)
209208
{
210209
struct smap options = SMAP_INITIALIZER(&options);
211-
struct smap other_config = SMAP_INITIALIZER(&other_config);
212210
smap_add(&options, "remote_ip", encap->ip);
213211
smap_add(&options, "local_ip", local_ip);
214212
smap_add(&options, "key", "flow");
@@ -286,11 +284,6 @@ tunnel_add(struct tunnel_ctx *tc,
286284
}
287285
}
288286

289-
if (is_ramp_tunnel(&chassis_rec->other_config)) {
290-
/* Propagate ramp switch flag from chassis to interface. */
291-
smap_add(&other_config, "is-vtep", "true");
292-
}
293-
294287
/* If there's an existing tunnel record that does not need any change,
295288
* keep it. Otherwise, create a new record (if there was an existing
296289
* record, the new record will supplant it and encaps_run() will delete
@@ -338,7 +331,6 @@ tunnel_add(struct tunnel_ctx *tc,
338331
ovsrec_interface_set_name(iface, port_name);
339332
ovsrec_interface_set_type(iface, encap->type);
340333
ovsrec_interface_set_options(iface, &options);
341-
ovsrec_interface_set_other_config(iface, &other_config);
342334

343335
struct ovsrec_port *port = ovsrec_port_insert(tc->ovs_txn);
344336
ovsrec_port_set_name(port, port_name);
@@ -354,7 +346,6 @@ tunnel_add(struct tunnel_ctx *tc,
354346
free(tunnel_entry_id);
355347
free(tunnel_entry_id_old);
356348
smap_destroy(&options);
357-
smap_destroy(&other_config);
358349
}
359350

360351
static bool

controller/local_data.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "lport.h"
3131
#include "lib/ovn-util.h"
3232
#include "lib/ovn-sb-idl.h"
33+
#include "lib/chassis-index.h"
3334
#include "local_data.h"
3435
#include "lport.h"
3536

@@ -457,6 +458,7 @@ tracked_datapaths_destroy(struct hmap *tracked_datapaths)
457458
void
458459
local_nonvif_data_run(const struct ovsrec_bridge *br_int,
459460
const struct sbrec_chassis *chassis_rec,
461+
struct ovsdb_idl_index *sbrec_chassis_by_name,
460462
struct simap *patch_ofports,
461463
struct hmap *chassis_tunnels,
462464
struct flow_based_tunnel *flow_tunnels)
@@ -525,15 +527,17 @@ local_nonvif_data_run(const struct ovsrec_bridge *br_int,
525527
if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, &ip, NULL)) {
526528
continue;
527529
}
530+
const struct sbrec_chassis *chassis =
531+
chassis_lookup_by_name(sbrec_chassis_by_name, hash_id);
528532
struct chassis_tunnel *tun = xmalloc(sizeof *tun);
529533
hmap_insert(chassis_tunnels, &tun->hmap_node,
530534
hash_string(hash_id, 0));
531535
tun->chassis_id = xstrdup(tunnel_id);
532536
tun->ofport = u16_to_ofp(ofport);
533537
tun->type = tunnel_type;
534538
tun->is_ipv6 = ip ? addr_is_ipv6(ip) : false;
535-
tun->is_ramp_tunnel = is_ramp_tunnel(&iface_rec->other_config);
536-
539+
tun->is_ramp_tunnel =
540+
chassis ? is_ramp_tunnel(&chassis->other_config) : false;
537541
free(hash_id);
538542
free(ip);
539543
break;

controller/local_data.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ struct flow_based_tunnel {
161161

162162
void local_nonvif_data_run(const struct ovsrec_bridge *br_int,
163163
const struct sbrec_chassis *chassis,
164+
struct ovsdb_idl_index *sbrec_chassis_by_name,
164165
struct simap *patch_ofports,
165166
struct hmap *chassis_tunnels,
166167
struct flow_based_tunnel *flow_tunnels);

controller/ovn-controller.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3730,7 +3730,7 @@ en_non_vif_data_run(struct engine_node *node, void *data)
37303730
ed_non_vif_data->use_flow_based_tunnels =
37313731
is_flow_based_tunnels_enabled(ovs_table, chassis);
37323732

3733-
local_nonvif_data_run(br_int, chassis,
3733+
local_nonvif_data_run(br_int, chassis, sbrec_chassis_by_name,
37343734
&ed_non_vif_data->patch_ofports,
37353735
&ed_non_vif_data->chassis_tunnels,
37363736
ed_non_vif_data->flow_tunnels);

0 commit comments

Comments
 (0)