Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.
On Tue, Nov 12, 2019 at 6:02 PM Han Zhou wrote:
>
>
>
> On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara 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
> > ---
> > 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, _address, ,
> > -addr_family);
> > +_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(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +/* Two sets to hold all load-balancer vips. */
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> > const char *ip_address;
> > -SSET_FOR_EACH (ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> > ds_put_format(_addresses, " %s", ip_address);
> > central_ip_address = true;
> > }
> > -sset_destroy(_ips);
> > +SSET_FOR_EACH (ip_address, _ips_v6) {
> > +ds_put_format(_addresses, " %s", ip_address);
> > +central_ip_address = true;
> > +}
> > +sset_destroy(_ips_v4);
> > +sset_destroy(_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(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> > const char *ip_address;
> > -SSET_FOR_EACH(ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> > ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > - "inport == %s && arp.tpa == %s && arp.op ==
> > 1",
> > - op->json_key, ip_address);
> > -} else {
> > -ds_put_format(,
> > - "inport == %s && nd_ns && nd.target == %s",
> > - op->json_key, ip_address);
> > -}
> > +ds_put_format(,
> > + "inport == %s && arp.tpa == %s && arp.op == 1",
> > + op->json_key, ip_address);
> >
> > ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -"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;",
> > -
Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.
On Tue, Nov 12, 2019 at 9:02 AM Han Zhou wrote:
>
>
>
> On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara 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
> > ---
> > 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, _address,
,
> > -addr_family);
> > +_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(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +/* Two sets to hold all load-balancer vips. */
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> > const char *ip_address;
> > -SSET_FOR_EACH (ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> > ds_put_format(_addresses, " %s", ip_address);
> > central_ip_address = true;
> > }
> > -sset_destroy(_ips);
> > +SSET_FOR_EACH (ip_address, _ips_v6) {
> > +ds_put_format(_addresses, " %s", ip_address);
> > +central_ip_address = true;
> > +}
> > +sset_destroy(_ips_v4);
> > +sset_destroy(_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(_ips);
> > -int addr_family;
> > -get_router_load_balancer_ips(op->od, _ips, _family);
> > +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> > +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> > +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> > const char *ip_address;
> > -SSET_FOR_EACH(ip_address, _ips) {
> > +SSET_FOR_EACH (ip_address, _ips_v4) {
> > ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > - "inport == %s && arp.tpa == %s && arp.op
== 1",
> > - op->json_key, ip_address);
> > -} else {
> > -ds_put_format(,
> > - "inport == %s && nd_ns && nd.target ==
%s",
> > - op->json_key, ip_address);
> > -}
> > +ds_put_format(,
> > + "inport == %s && arp.tpa == %s && arp.op ==
1",
> > + op->json_key, ip_address);
> >
> > ds_clear();
> > -if (addr_family == AF_INET) {
> > -ds_put_format(,
> > -"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,
> > -
Re: [ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.
On Tue, Nov 12, 2019 at 2:28 AM Dumitru Ceara 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
> ---
> 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, _address,
,
> -addr_family);
> +_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(_ips);
> -int addr_family;
> -get_router_load_balancer_ips(op->od, _ips, _family);
> +/* Two sets to hold all load-balancer vips. */
> +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
>
> const char *ip_address;
> -SSET_FOR_EACH (ip_address, _ips) {
> +SSET_FOR_EACH (ip_address, _ips_v4) {
> ds_put_format(_addresses, " %s", ip_address);
> central_ip_address = true;
> }
> -sset_destroy(_ips);
> +SSET_FOR_EACH (ip_address, _ips_v6) {
> +ds_put_format(_addresses, " %s", ip_address);
> +central_ip_address = true;
> +}
> +sset_destroy(_ips_v4);
> +sset_destroy(_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(_ips);
> -int addr_family;
> -get_router_load_balancer_ips(op->od, _ips, _family);
> +struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
> +struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
> +get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
>
> const char *ip_address;
> -SSET_FOR_EACH(ip_address, _ips) {
> +SSET_FOR_EACH (ip_address, _ips_v4) {
> ds_clear();
> -if (addr_family == AF_INET) {
> -ds_put_format(,
> - "inport == %s && arp.tpa == %s && arp.op
== 1",
> - op->json_key, ip_address);
> -} else {
> -ds_put_format(,
> - "inport == %s && nd_ns && nd.target == %s",
> - op->json_key, ip_address);
> -}
> +ds_put_format(,
> + "inport == %s && arp.tpa == %s && arp.op == 1",
> + op->json_key, ip_address);
>
> ds_clear();
> -if (addr_family == AF_INET) {
> -ds_put_format(,
> -"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(,
> -"nd_na { "
> -"eth.src = %s; "
> -"ip6.src = %s; "
> -"nd.target =
[ovs-dev] [PATCH v7 ovn 1/2] ovn-northd: Fix get_router_load_balancer_ips() for mixed address families.
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
---
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, _address, ,
-addr_family);
+_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(_ips);
-int addr_family;
-get_router_load_balancer_ips(op->od, _ips, _family);
+/* Two sets to hold all load-balancer vips. */
+struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
+struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
+get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
const char *ip_address;
-SSET_FOR_EACH (ip_address, _ips) {
+SSET_FOR_EACH (ip_address, _ips_v4) {
ds_put_format(_addresses, " %s", ip_address);
central_ip_address = true;
}
-sset_destroy(_ips);
+SSET_FOR_EACH (ip_address, _ips_v6) {
+ds_put_format(_addresses, " %s", ip_address);
+central_ip_address = true;
+}
+sset_destroy(_ips_v4);
+sset_destroy(_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(_ips);
-int addr_family;
-get_router_load_balancer_ips(op->od, _ips, _family);
+struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
+struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
+get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
const char *ip_address;
-SSET_FOR_EACH(ip_address, _ips) {
+SSET_FOR_EACH (ip_address, _ips_v4) {
ds_clear();
-if (addr_family == AF_INET) {
-ds_put_format(,
- "inport == %s && arp.tpa == %s && arp.op == 1",
- op->json_key, ip_address);
-} else {
-ds_put_format(,
- "inport == %s && nd_ns && nd.target == %s",
- op->json_key, ip_address);
-}
+ds_put_format(,
+ "inport == %s && arp.tpa == %s && arp.op == 1",
+ op->json_key, ip_address);
ds_clear();
-if (addr_family == AF_INET) {
-ds_put_format(,
-"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(,
-"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,
-
