[ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.
Han Zhou
zhouhan at gmail.com
Tue Nov 12 17:02:00 UTC 2019
On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Function get_router_load_balancer_ips() was incorrectly returning a
> single address_family even though the IP set could contain a mix of
> IPv4 and IPv6 addresses.
>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
> northd/ovn-northd.c | 126
+++++++++++++++++++++++++++++----------------------
> 1 file changed, 72 insertions(+), 54 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 2f0f501..32f3200 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2184,7 +2184,7 @@ ip_address_and_port_from_lb_key(const char *key,
char **ip_address,
>
> static void
> get_router_load_balancer_ips(const struct ovn_datapath *od,
> - struct sset *all_ips, int *addr_family)
> + struct sset *all_ips_v4, struct sset
*all_ips_v6)
> {
> if (!od->nbr) {
> return;
> @@ -2199,13 +2199,21 @@ get_router_load_balancer_ips(const struct
ovn_datapath *od,
> /* node->key contains IP:port or just IP. */
> char *ip_address = NULL;
> uint16_t port;
> + int addr_family;
>
> ip_address_and_port_from_lb_key(node->key, &ip_address,
&port,
> - addr_family);
> + &addr_family);
> if (!ip_address) {
> continue;
> }
>
> + struct sset *all_ips;
> + if (addr_family == AF_INET) {
> + all_ips = all_ips_v4;
> + } else {
> + all_ips = all_ips_v6;
> + }
> +
> if (!sset_contains(all_ips, ip_address)) {
> sset_add(all_ips, ip_address);
> }
> @@ -2299,17 +2307,22 @@ get_nat_addresses(const struct ovn_port *op,
size_t *n)
> }
> }
>
> - /* A set to hold all load-balancer vips. */
> - struct sset all_ips = SSET_INITIALIZER(&all_ips);
> - int addr_family;
> - get_router_load_balancer_ips(op->od, &all_ips, &addr_family);
> + /* Two sets to hold all load-balancer vips. */
> + struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4);
> + struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6);
> + get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6);
>
> const char *ip_address;
> - SSET_FOR_EACH (ip_address, &all_ips) {
> + SSET_FOR_EACH (ip_address, &all_ips_v4) {
> ds_put_format(&c_addresses, " %s", ip_address);
> central_ip_address = true;
> }
> - sset_destroy(&all_ips);
> + SSET_FOR_EACH (ip_address, &all_ips_v6) {
> + ds_put_format(&c_addresses, " %s", ip_address);
> + central_ip_address = true;
> + }
> + sset_destroy(&all_ips_v4);
> + sset_destroy(&all_ips_v6);
>
> if (central_ip_address) {
> /* Gratuitous ARP for centralized NAT rules on distributed
gateway
> @@ -6911,61 +6924,66 @@ build_lrouter_flows(struct hmap *datapaths,
struct hmap *ports,
> }
>
> /* A set to hold all load-balancer vips that need ARP responses.
*/
> - struct sset all_ips = SSET_INITIALIZER(&all_ips);
> - int addr_family;
> - get_router_load_balancer_ips(op->od, &all_ips, &addr_family);
> + struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4);
> + struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6);
> + get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6);
>
> const char *ip_address;
> - SSET_FOR_EACH(ip_address, &all_ips) {
> + SSET_FOR_EACH (ip_address, &all_ips_v4) {
> ds_clear(&match);
> - if (addr_family == AF_INET) {
> - ds_put_format(&match,
> - "inport == %s && arp.tpa == %s && arp.op
== 1",
> - op->json_key, ip_address);
> - } else {
> - ds_put_format(&match,
> - "inport == %s && nd_ns && nd.target == %s",
> - op->json_key, ip_address);
> - }
> + ds_put_format(&match,
> + "inport == %s && arp.tpa == %s && arp.op == 1",
> + op->json_key, ip_address);
>
> ds_clear(&actions);
> - if (addr_family == AF_INET) {
> - ds_put_format(&actions,
> - "eth.dst = eth.src; "
> - "eth.src = %s; "
> - "arp.op = 2; /* ARP reply */ "
> - "arp.tha = arp.sha; "
> - "arp.sha = %s; "
> - "arp.tpa = arp.spa; "
> - "arp.spa = %s; "
> - "outport = %s; "
> - "flags.loopback = 1; "
> - "output;",
> - op->lrp_networks.ea_s,
> - op->lrp_networks.ea_s,
> - ip_address,
> - op->json_key);
> - } else {
> - ds_put_format(&actions,
> - "nd_na { "
> - "eth.src = %s; "
> - "ip6.src = %s; "
> - "nd.target = %s; "
> - "nd.tll = %s; "
> - "outport = inport; "
> - "flags.loopback = 1; "
> - "output; "
> - "};",
> - op->lrp_networks.ea_s,
> - ip_address,
> - ip_address,
> - op->lrp_networks.ea_s);
> - }
> + ds_put_format(&actions,
> + "eth.dst = eth.src; "
> + "eth.src = %s; "
> + "arp.op = 2; /* ARP reply */ "
> + "arp.tha = arp.sha; "
> + "arp.sha = %s; "
> + "arp.tpa = arp.spa; "
> + "arp.spa = %s; "
> + "outport = %s; "
> + "flags.loopback = 1; "
> + "output;",
> + op->lrp_networks.ea_s,
> + op->lrp_networks.ea_s,
> + ip_address,
> + op->json_key);
> +
> ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> ds_cstr(&match), ds_cstr(&actions));
> }
>
> - sset_destroy(&all_ips);
> + SSET_FOR_EACH (ip_address, &all_ips_v6) {
> + ds_clear(&match);
> + ds_put_format(&match,
> + "inport == %s && nd_ns && nd.target == %s",
> + op->json_key, ip_address);
> +
> + ds_clear(&actions);
> + ds_put_format(&actions,
> + "nd_na { "
> + "eth.src = %s; "
> + "ip6.src = %s; "
> + "nd.target = %s; "
> + "nd.tll = %s; "
> + "outport = inport; "
> + "flags.loopback = 1; "
> + "output; "
> + "};",
> + op->lrp_networks.ea_s,
> + ip_address,
> + ip_address,
> + op->lrp_networks.ea_s);
> +
> + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> + ds_cstr(&match), ds_cstr(&actions));
> + }
> +
> + sset_destroy(&all_ips_v4);
> + sset_destroy(&all_ips_v6);
>
> /* A gateway router can have 2 SNAT IP addresses to force DNATed
and
> * LBed traffic respectively to be SNATed. In addition, there
can be
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks Dumitru. I applied this to master.
More information about the dev
mailing list