Re: [ovs-dev] [PATCH ovn v7 3/5] northd: Save all router addresses in Port_Bindings

2021-05-05 Thread 0-day Robot
Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#252 FILE: northd/ovn-northd.c:3207:
ds_put_format(&router_networks, "%s", 
op->peer->lrp_networks.ea_s);

WARNING: Line is 83 characters long (recommended limit is 79)
#264 FILE: northd/ovn-northd.c:3214:
ds_put_format(&router_networks, " 
is_chassis_resident(%s)",

WARNING: Line is 83 characters long (recommended limit is 79)
#270 FILE: northd/ovn-northd.c:3220:
ds_cstr(&router_networks), 
peer_is_gateway,

Lines checked: 759, Warnings: 3, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn v7 3/5] northd: Save all router addresses in Port_Bindings

2021-05-05 Thread Mark Michelson
northd currently stores logical switch port's configured "nat-addresses"
in the southbound Port_Binding "nat_addresses" column. This allows for
explicit configuration of which NAT addresses should be broadcast in
gARP messages by OVN.

This adds a similar column, "router_addresses" to the Port_Binding. The
difference is that this column contains all NAT, load balancer, and
router interface addresses, no matter the northbound configuration.

This column will be used in an upcoming commit.

Signed-off-by: Mark Michelson 
---
 northd/ovn-northd.c  | 217 +
 northd/ovn_northd.dl |  45 ++--
 ovn-sb.ovsschema |   5 +-
 ovn-sb.xml   |  20 
 tests/ovn-northd.at  | 253 +++
 5 files changed, 461 insertions(+), 79 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d8ee65a5f..38de13090 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2486,7 +2486,7 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
 {
 size_t n_nats = 0;
 struct eth_addr mac;
-if (!op->nbrp || !op->od || !op->od->nbr
+if (!op || !op->nbrp || !op->od || !op->od->nbr
 || (!op->od->nbr->n_nat && !op->od->nbr->n_load_balancer)
 || !eth_addr_from_string(op->nbrp->mac, &mac)) {
 *n = n_nats;
@@ -2878,6 +2878,123 @@ ovn_update_ipv6_prefix(struct hmap *ports)
 }
 }
 
+static void
+ovn_port_set_garp_addresses(const struct ovn_port *op, const char **nats,
+size_t n_nats, const char *router_networks,
+bool peer_is_gateway, bool peer_has_dist_gw_port)
+{
+const char *nat_addresses = smap_get(&op->nbsp->options,
+ "nat-addresses");
+size_t n_sources = 0;
+const char **source = NULL;
+if (nat_addresses && (peer_is_gateway || peer_has_dist_gw_port)) {
+if (!strcmp(nat_addresses, "router")) {
+/* We size this to n_nats + 1 since it can at most include
+ * all NAT addresses plus the router's networks.
+ */
+source = nats;
+n_sources = n_nats;
+/* Only accept manual specification of ethernet address
+ * followed by IPv4 addresses on type "l3gateway" ports. */
+} else if (peer_is_gateway) {
+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);
+/* Allocate enough space for the nat_addresses plus
+ * the router networks
+ */
+source = &nat_addresses;
+n_sources = 1;
+}
+}
+}
+
+size_t n_garps = 0;
+/* The "garps" array contains the addresses for which
+ * ovn-controller should send GARPs. This is controlled
+ * by the "nat-addresses" option on the logical switch.
+ *
+ * Size the garps array to one more than the number of NAT
+ * sources so that we can fit the NATs and the router
+ * networks.
+ */
+char **garps = xcalloc(n_sources + 1, sizeof *garps);
+for (n_garps = 0; n_garps < n_sources; n_garps++) {
+garps[n_garps] = xstrdup(source[n_garps]);
+}
+
+/* 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 (peer_has_dist_gw_port &&
+(smap_get_bool(&op->peer->nbrp->options,
+  "reside-on-redirect-chassis", false) ||
+op->peer == op->peer->od->l3dgw_port)) {
+add_router_port_garp = true;
+} else if (peer_is_gateway && op->od->n_localnet_ports) {
+add_router_port_garp = true;
+}
+
+if (add_router_port_garp) {
+/* "garps" was allocated with enough space to hold
+ * this address without reallocating.
+ */
+garps[n_garps] = xstrdup(router_networks);
+n_garps++;
+}
+
+sbrec_port_binding_set_nat_addresses(op->sb,
+ (const char **) garps,
+ n_garps);
+for (size_t i = 0; i < n_garps; i++) {
+free(garps[i]);
+}
+