Skip to content

Commit

Permalink
northd: Allow backwards compatibility for Logical_Switch_Port.up.
Browse files Browse the repository at this point in the history
In general, ovn-northd expects ovn-controller to set Port_Binding.up
before it declares the logical switch port as being up.

Even though the recommended upgrade procedure for OVN states that
ovn-controllers should be upgraded before ovn-northd, there are cases
when CMSs don't follow this guideline.

This would cause all existing and bound Logical_Switch_Ports to be
declared "down" until ovn-controllers are upgraded.

To avoid this situation, ovn-controllers now explicitly set
Chassis.other_config:port-up-notif in their own chassis record.  Based
on this value, ovn-northd can determine if it needs to use the old type
of logic or the new one (Port_Binding.up) when setting LSP.up.

Note:
In case of downgrading ovn-controller before ovn-northd, if
ovn-controller is forcefully stopped it will not clear its chassis
record from the SB. Older versions will not have the capability to
clear the other_config:port-up-notif value so LSPs will be declared
"down" until ovn-northd is downgraded as well.  As this
upgrade/downgrade procedure is not the recommended one, we don't try
to deal with this scenario.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
dceara authored and numansiddique committed Feb 4, 2021
1 parent 8b45fc9 commit a99af03
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 1 deletion.
7 changes: 7 additions & 0 deletions controller/chassis.c
Expand Up @@ -28,6 +28,7 @@
#include "lib/ovn-sb-idl.h"
#include "ovn-controller.h"
#include "lib/util.h"
#include "ovn/features.h"

VLOG_DEFINE_THIS_MODULE(chassis);

Expand Down Expand Up @@ -293,6 +294,7 @@ chassis_build_other_config(struct smap *config, const char *bridge_mappings,
smap_replace(config, "iface-types", iface_types);
smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
}

/*
Expand Down Expand Up @@ -363,6 +365,11 @@ chassis_other_config_changed(const char *bridge_mappings,
return true;
}

if (!smap_get_bool(&chassis_rec->other_config, OVN_FEATURE_PORT_UP_NOTIF,
false)) {
return true;
}

return false;
}

Expand Down
1 change: 1 addition & 0 deletions include/ovn/automake.mk
Expand Up @@ -2,5 +2,6 @@ ovnincludedir = $(includedir)/ovn
ovninclude_HEADERS = \
include/ovn/actions.h \
include/ovn/expr.h \
include/ovn/features.h \
include/ovn/lex.h \
include/ovn/logical-fields.h
22 changes: 22 additions & 0 deletions include/ovn/features.h
@@ -0,0 +1,22 @@
/* Copyright (c) 2021, Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef OVN_FEATURES_H
#define OVN_FEATURES_H 1

/* ovn-controller supported feature names. */
#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"

#endif
13 changes: 12 additions & 1 deletion northd/ovn-northd.c
Expand Up @@ -40,6 +40,7 @@
#include "lib/lb.h"
#include "memory.h"
#include "ovn/actions.h"
#include "ovn/features.h"
#include "ovn/logical-fields.h"
#include "packets.h"
#include "openvswitch/poll-loop.h"
Expand Down Expand Up @@ -12838,7 +12839,17 @@ handle_port_binding_changes(struct northd_context *ctx, struct hmap *ports,
continue;
}

bool up = ((sb->up && (*sb->up)) || lsp_is_router(op->nbsp));
bool up = false;

if (lsp_is_router(op->nbsp)) {
up = true;
} else if (sb->chassis) {
up = smap_get_bool(&sb->chassis->other_config,
OVN_FEATURE_PORT_UP_NOTIF, false)
? sb->n_up && sb->up[0]
: true;
}

if (!op->nbsp->up || *op->nbsp->up != up) {
nbrec_logical_switch_port_set_up(op->nbsp, &up, 1);
}
Expand Down
5 changes: 5 additions & 0 deletions ovn-sb.xml
Expand Up @@ -322,6 +322,11 @@
table. See <code>ovn-controller</code>(8) for more information.
</column>

<column name="other_config" key="port-up-notif">
<code>ovn-controller</code> populates this key with <code>true</code>
when it supports <code>Port_Binding.up</code>.
</column>

<group title="Common Columns">
The overall purpose of these columns is described under <code>Common
Columns</code> at the beginning of this document.
Expand Down
17 changes: 17 additions & 0 deletions tests/ovn-controller.at
Expand Up @@ -414,3 +414,20 @@ OVS_WAIT_UNTIL([ovs-vsctl get Bridge br-int external_ids:ovn-nb-cfg], [0], [1])

OVN_CLEANUP([hv1])
AT_CLEANUP

AT_SETUP([ovn -- features])
AT_KEYWORDS([features])
ovn_start

net_add n1
sim_add hv1
ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.1

# Wait for ovn-controller to register in the SB.
OVS_WAIT_UNTIL([
test "$(ovn-sbctl get chassis hv1 other_config:port-up-notif)" = '"true"'
])

OVN_CLEANUP([hv1])
AT_CLEANUP
22 changes: 22 additions & 0 deletions tests/ovn-northd.at
Expand Up @@ -2421,3 +2421,25 @@ check ovn-nbctl --wait=sb sync
AT_CHECK([grep -qE 'duplicate logical.*port p1' northd/ovn-northd.log], [0])

AT_CLEANUP

AT_SETUP([ovn -- Port_Binding.up backwards compatibility])
ovn_start

ovn-nbctl ls-add ls1
ovn-nbctl --wait=sb lsp-add ls1 lsp1

# Simulate the fact that lsp1 had been previously bound on hv1 by an
# ovn-controller running an older version.
ovn-sbctl \
--id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="geneve" \
-- --id=@c create chassis name=hv1 encaps=@e \
-- set Port_Binding lsp1 chassis=@c

wait_for_ports_up lsp1

# Simulate the fact that hv1 is aware of Port_Binding.up, ovn-northd
# should transition the port state to down.
check ovn-sbctl set chassis hv1 other_config:port-up-notif=true
wait_row_count nb:Logical_Switch_Port 1 up=false name=lsp1

AT_CLEANUP

0 comments on commit a99af03

Please sign in to comment.