Skip to content

Commit

Permalink
northd: Sync SB Port bindings NAT column in a separate engine node.
Browse files Browse the repository at this point in the history 
A new engine node 'sync_to_sb_pb' is added within 'sync_to_sb'
node to sync NAT column of Port bindings table.  This separation
is required in order to add load balancer group I-P handling
in 'northd' engine node (which is handled in the next commit).

'sync_to_sb_pb' engine node can be later expanded to sync other
Port binding columns if required.

Reviewed-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
@numansiddique
numansiddique committed Sep 14, 2023
1 parent cc27dcc commit b16121f
Show file tree
Hide file tree
Showing 6 changed files with 239 additions and 120 deletions.
52 changes: 52 additions & 0 deletions northd/en-sync-sb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,58 @@ sync_to_sb_lb_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
return true;
}

/* sync_to_sb_pb engine node functions.
* This engine node syncs the SB Port Bindings (partly).
* en_northd engine create the SB Port binding rows and
* updates most of the columns.
* This engine node updates the port binding columns which
* needs to be updated after northd engine is run.
*/

void *
en_sync_to_sb_pb_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
{
return NULL;
}

void
en_sync_to_sb_pb_run(struct engine_node *node, void *data OVS_UNUSED)
{
const struct engine_context *eng_ctx = engine_get_context();
struct northd_data *northd_data = engine_get_input_data("northd", node);

sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports);
engine_set_node_state(node, EN_UPDATED);
}

void
en_sync_to_sb_pb_cleanup(void *data OVS_UNUSED)
{

}

bool
sync_to_sb_pb_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
{
const struct engine_context *eng_ctx = engine_get_context();
if (!eng_ctx->ovnsb_idl_txn) {
return false;
}

struct northd_data *nd = engine_get_input_data("northd", node);
if (!nd->change_tracked) {
return false;
}

if (!sync_pbs_for_northd_ls_changes(&nd->tracked_ls_changes)) {
return false;
}

engine_set_node_state(node, EN_UPDATED);
return true;
}

/* static functions. */
static void
sync_addr_set(struct ovsdb_idl_txn *ovnsb_txn, const char *name,
Expand Down
5 changes: 5 additions & 0 deletions northd/en-sync-sb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,9 @@ void en_sync_to_sb_lb_run(struct engine_node *, void *data);
void en_sync_to_sb_lb_cleanup(void *data);
bool sync_to_sb_lb_northd_handler(struct engine_node *, void *data OVS_UNUSED);

void *en_sync_to_sb_pb_init(struct engine_node *, struct engine_arg *);
void en_sync_to_sb_pb_run(struct engine_node *, void *data);
void en_sync_to_sb_pb_cleanup(void *data);
bool sync_to_sb_pb_northd_handler(struct engine_node *, void *data OVS_UNUSED);

#endif /* end of EN_SYNC_SB_H */
8 changes: 7 additions & 1 deletion northd/inc-proc-northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_group, "port_group");
static ENGINE_NODE(fdb_aging, "fdb_aging");
static ENGINE_NODE(fdb_aging_waker, "fdb_aging_waker");
static ENGINE_NODE(sync_to_sb_lb, "sync_to_sb_lb");
static ENGINE_NODE(sync_to_sb_pb, "sync_to_sb_pb");
static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");

void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
Expand Down Expand Up @@ -228,15 +229,20 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
sync_to_sb_lb_northd_handler);
engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer, NULL);

engine_add_input(&en_sync_to_sb_pb, &en_northd,
sync_to_sb_pb_northd_handler);

/* en_sync_to_sb engine node syncs the SB database tables from
* the NB database tables.
* Right now this engine syncs the SB Address_Set table, Port_Group table
* SB Meter/Meter_Band tables and SB Load_Balancer table.
* SB Meter/Meter_Band tables and SB Load_Balancer table and
* (partly) SB Port_Binding table.
*/
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_addr_set, NULL);
engine_add_input(&en_sync_to_sb, &en_port_group, NULL);
engine_add_input(&en_sync_to_sb, &en_sync_meters, NULL);
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_lb, NULL);
engine_add_input(&en_sync_to_sb, &en_sync_to_sb_pb, NULL);

engine_add_input(&en_sync_from_sb, &en_northd,
sync_from_sb_northd_handler);
Expand Down
274 changes: 162 additions & 112 deletions northd/northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3527,8 +3527,6 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
ds_destroy(&s);

sbrec_port_binding_set_external_ids(op->sb, &op->nbrp->external_ids);

sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
} else {
if (!lsp_is_router(op->nbsp)) {
uint32_t queue_id = smap_get_int(
Expand Down Expand Up @@ -3672,116 +3670,6 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
} else {
sbrec_port_binding_set_options(op->sb, NULL);
}
const char *nat_addresses = smap_get(&op->nbsp->options,
"nat-addresses");
size_t n_nats = 0;
char **nats = NULL;
bool l3dgw_ports = op->peer && op->peer->od &&
op->peer->od->n_l3dgw_ports;
if (nat_addresses && !strcmp(nat_addresses, "router")) {
if (op->peer && op->peer->od
&& (chassis || op->peer->od->n_l3dgw_ports)) {
bool exclude_lb_vips = smap_get_bool(&op->nbsp->options,
"exclude-lb-vips-from-garp", false);
nats = get_nat_addresses(op->peer, &n_nats, false,
!exclude_lb_vips);
}
} else if (nat_addresses && (chassis || l3dgw_ports)) {
struct lport_addresses laddrs;
if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
} else {
destroy_lport_addresses(&laddrs);
n_nats = 1;
nats = xcalloc(1, sizeof *nats);
struct ds nat_addr = DS_EMPTY_INITIALIZER;
ds_put_format(&nat_addr, "%s", nat_addresses);
if (l3dgw_ports) {
const struct ovn_port *l3dgw_port = (
is_l3dgw_port(op->peer)
? op->peer
: op->peer->od->l3dgw_ports[0]);
ds_put_format(&nat_addr, " is_chassis_resident(%s)",
l3dgw_port->cr_port->json_key);
}
nats[0] = xstrdup(ds_cstr(&nat_addr));
ds_destroy(&nat_addr);
}
}

/* Add the router mac and IPv4 addresses to
* Port_Binding.nat_addresses so that GARP is sent for these
* IPs by the ovn-controller on which the distributed gateway
* router port resides if:
*
* - op->peer has 'reside-on-redirect-chassis' set and the
* the logical router datapath has distributed router port.
*
* - op->peer is distributed gateway router port.
*
* - op->peer's router is a gateway router and op has a localnet
* port.
*
* Note: Port_Binding.nat_addresses column is also used for
* sending the GARPs for the router port IPs.
* */
bool add_router_port_garp = false;
if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
if (is_l3dgw_port(op->peer)) {
add_router_port_garp = true;
} else if (smap_get_bool(&op->peer->nbrp->options,
"reside-on-redirect-chassis", false)) {
if (op->peer->od->n_l3dgw_ports == 1) {
add_router_port_garp = true;
} else {
static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
"set on logical router port %s, which "
"is on logical router %s, which has %"
PRIuSIZE" distributed gateway ports. This"
"option can only be used when there is "
"a single distributed gateway port.",
op->peer->key, op->peer->od->nbr->name,
op->peer->od->n_l3dgw_ports);
}
}
} else if (chassis && op->od->n_localnet_ports) {
add_router_port_garp = true;
}

if (add_router_port_garp) {
struct ds garp_info = DS_EMPTY_INITIALIZER;
ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);

for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
i++) {
ds_put_format(&garp_info, " %s",
op->peer->lrp_networks.ipv4_addrs[i].addr_s);
}

if (op->peer->od->n_l3dgw_ports) {
const struct ovn_port *l3dgw_port = (
is_l3dgw_port(op->peer)
? op->peer
: op->peer->od->l3dgw_ports[0]);
ds_put_format(&garp_info, " is_chassis_resident(%s)",
l3dgw_port->cr_port->json_key);
}

n_nats++;
nats = xrealloc(nats, (n_nats * sizeof *nats));
nats[n_nats - 1] = ds_steal_cstr(&garp_info);
ds_destroy(&garp_info);
}
sbrec_port_binding_set_nat_addresses(op->sb,
(const char **) nats, n_nats);
for (size_t i = 0; i < n_nats; i++) {
free(nats[i]);
}
free(nats);
}

sbrec_port_binding_set_parent_port(op->sb, op->nbsp->parent_name);
Expand Down Expand Up @@ -4735,6 +4623,168 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
}
}

/* Syncs the SB port binding for the ovn_port 'op'. Caller should make sure
* that the OVN SB IDL txn is not NULL. Presently it only syncs the nat
* column of port binding corresponding to the 'op->nbsp' */
static void
sync_pb_for_op(struct ovn_port *op)
{
if (lsp_is_router(op->nbsp)) {
const char *chassis = NULL;
if (op->peer && op->peer->od && op->peer->od->nbr) {
chassis = smap_get(&op->peer->od->nbr->options, "chassis");
}

const char *nat_addresses = smap_get(&op->nbsp->options,
"nat-addresses");
size_t n_nats = 0;
char **nats = NULL;
bool l3dgw_ports = op->peer && op->peer->od &&
op->peer->od->n_l3dgw_ports;
if (nat_addresses && !strcmp(nat_addresses, "router")) {
if (op->peer && op->peer->od
&& (chassis || op->peer->od->n_l3dgw_ports)) {
bool exclude_lb_vips = smap_get_bool(&op->nbsp->options,
"exclude-lb-vips-from-garp", false);
nats = get_nat_addresses(op->peer, &n_nats, false,
!exclude_lb_vips);
}
} else if (nat_addresses && (chassis || l3dgw_ports)) {
struct lport_addresses laddrs;
if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
} else {
destroy_lport_addresses(&laddrs);
n_nats = 1;
nats = xcalloc(1, sizeof *nats);
struct ds nat_addr = DS_EMPTY_INITIALIZER;
ds_put_format(&nat_addr, "%s", nat_addresses);
if (l3dgw_ports) {
const struct ovn_port *l3dgw_port = (
is_l3dgw_port(op->peer)
? op->peer
: op->peer->od->l3dgw_ports[0]);
ds_put_format(&nat_addr, " is_chassis_resident(%s)",
l3dgw_port->cr_port->json_key);
}
nats[0] = xstrdup(ds_cstr(&nat_addr));
ds_destroy(&nat_addr);
}
}

/* Add the router mac and IPv4 addresses to
* Port_Binding.nat_addresses so that GARP is sent for these
* IPs by the ovn-controller on which the distributed gateway
* router port resides if:
*
* - op->peer has 'reside-on-redirect-chassis' set and the
* the logical router datapath has distributed router port.
*
* - op->peer is distributed gateway router port.
*
* - op->peer's router is a gateway router and op has a localnet
* port.
*
* Note: Port_Binding.nat_addresses column is also used for
* sending the GARPs for the router port IPs.
* */
bool add_router_port_garp = false;
if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
if (is_l3dgw_port(op->peer)) {
add_router_port_garp = true;
} else if (smap_get_bool(&op->peer->nbrp->options,
"reside-on-redirect-chassis", false)) {
if (op->peer->od->n_l3dgw_ports == 1) {
add_router_port_garp = true;
} else {
static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
"set on logical router port %s, which "
"is on logical router %s, which has %"
PRIuSIZE" distributed gateway ports. This"
"option can only be used when there is "
"a single distributed gateway port.",
op->peer->key, op->peer->od->nbr->name,
op->peer->od->n_l3dgw_ports);
}
}
} else if (chassis && op->od->n_localnet_ports) {
add_router_port_garp = true;
}

if (add_router_port_garp) {
struct ds garp_info = DS_EMPTY_INITIALIZER;
ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);

for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
i++) {
ds_put_format(&garp_info, " %s",
op->peer->lrp_networks.ipv4_addrs[i].addr_s);
}

if (op->peer->od->n_l3dgw_ports) {
const struct ovn_port *l3dgw_port = (
is_l3dgw_port(op->peer)
? op->peer
: op->peer->od->l3dgw_ports[0]);
ds_put_format(&garp_info, " is_chassis_resident(%s)",
l3dgw_port->cr_port->json_key);
}

n_nats++;
nats = xrealloc(nats, (n_nats * sizeof *nats));
nats[n_nats - 1] = ds_steal_cstr(&garp_info);
ds_destroy(&garp_info);
}
sbrec_port_binding_set_nat_addresses(op->sb,
(const char **) nats, n_nats);
for (size_t i = 0; i < n_nats; i++) {
free(nats[i]);
}
free(nats);
} else {
sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
}
}

/* Sync the SB Port bindings which needs to be updated.
* Presently it syncs the nat column of port bindings corresponding to
* the logical switch ports. */
void
sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports)
{
ovs_assert(ovnsb_idl_txn);

struct ovn_port *op;
HMAP_FOR_EACH (op, key_node, ls_ports) {
sync_pb_for_op(op);
}
}

/* Sync the SB Port bindings for the added and updated logical switch ports
* of the tracked logical switches (from the northd engine node). */
bool
sync_pbs_for_northd_ls_changes(struct tracked_ls_changes *ls_changes)
{
struct ls_change *ls_change;
LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated) {
struct ovn_port *op;

LIST_FOR_EACH (op, list, &ls_change->added_ports) {
sync_pb_for_op(op);
}

LIST_FOR_EACH (op, list, &ls_change->updated_ports) {
sync_pb_for_op(op);
}
}

return true;
}

static bool
ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key)
{
Expand Down

0 comments on commit b16121f

Please sign in to comment.