Re: [ovs-dev] [PATCH ovn] Add IPv6 support for lb health-check
> On Tue, Nov 22, 2022 at 12:54 PM Lorenzo Bianconi
> wrote:
> >
> > Add Similar to IPv4 counterpart, introduce IPv6 load-balancer health
> > check support.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2136094
> > Signed-off-by: Lorenzo Bianconi
>
> Hi Lorenzo,
>
> Thanks for adding this missing feature. Please see a few comments below.
Hi Numan,
thx for the review :)
>
>
> > ---
> > controller/pinctrl.c| 213 -
> > northd/northd.c | 77 ++
> > northd/ovn-northd.8.xml | 17 +++
> > ovn-nb.xml | 8 +-
> > tests/ovn.at| 201 ++-
> > tests/system-ovn.at | 230 +++-
> > 6 files changed, 647 insertions(+), 99 deletions(-)
> >
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index f44775c7e..89c2f207b 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -6676,7 +6676,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > ovs_be32 ip4;
> > if (ip_parse(sb_svc_mon->ip, )) {
> > ip_addr = in6_addr_mapped_ipv4(ip4);
> > -} else {
> > +} else if (!ipv6_parse(sb_svc_mon->ip, _addr)) {
> > continue;
> > }
> >
> > @@ -6689,16 +6689,27 @@ sync_svc_monitors(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> > continue;
> > }
> >
> > -for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> > -if (ip4 == laddrs.ipv4_addrs[j].addr) {
> > -ea = laddrs.ea;
> > -mac_found = true;
> > -break;
> > +if (IN6_IS_ADDR_V4MAPPED(_addr)) {
> > +for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> > +if (ip4 == laddrs.ipv4_addrs[j].addr) {
> > +ea = laddrs.ea;
> > +mac_found = true;
> > +break;
> > +}
> > +}
> > +} else {
> > +for (size_t j = 0; j < laddrs.n_ipv6_addrs; j++) {
> > +if (!memcmp(_addr, _addrs[j].addr,
> > +sizeof(ovs_be32[4]))) {
>
> It's a bit odd to use size of ovs_be32 here since we are memcmping
> 'struct in6_addr'.
>
> I'd suggest using the macro - IN6_ARE_ADDR_EQUAL here.
ack, I will fix it in v2.
>
>
> > +ea = laddrs.ea;
> > +mac_found = true;
> > +break;
> > +}
> > }
> > }
> >
> > -if (!mac_found && !laddrs.n_ipv4_addrs) {
> > -/* IPv4 address(es) are not configured. Use the first mac.
> > */
> > +if (!mac_found && !laddrs.n_ipv4_addrs &&
> > !laddrs.n_ipv6_addrs) {
> > +/* IP address(es) are not configured. Use the first mac. */
> > ea = laddrs.ea;
> > mac_found = true;
> > }
> > @@ -6732,7 +6743,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
> > svc_mon->port_key = port_key;
> > svc_mon->proto_port = sb_svc_mon->port;
> > svc_mon->ip = ip_addr;
> > -svc_mon->is_ip6 = false;
> > +svc_mon->is_ip6 = !IN6_IS_ADDR_V4MAPPED(_addr);
> > svc_mon->state = SVC_MON_S_INIT;
> > svc_mon->status = SVC_MON_ST_UNKNOWN;
> > svc_mon->protocol = protocol;
> > @@ -7500,26 +7511,30 @@ svc_monitor_send_tcp_health_check__(struct rconn
> > *swconn,
> > ovs_be32 tcp_ack,
> > ovs_be16 tcp_src)
> > {
> > -if (svc_mon->is_ip6) {
> > -return;
> > -}
> > -
> > /* Compose a TCP-SYN packet. */
> > uint64_t packet_stub[128 / 8];
> > struct dp_packet packet;
> > +dp_packet_use_stub(, packet_stub, sizeof packet_stub);
> >
> > struct eth_addr eth_src;
> > eth_addr_from_string(svc_mon->sb_svc_mon->src_mac, _src);
> > -ovs_be32 ip4_src;
> > -ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
> > -
> > -dp_packet_use_stub(, packet_stub, sizeof packet_stub);
> > -pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
> > - ip4_src, in6_addr_get_mapped_ipv4(_mon->ip),
> > - IPPROTO_TCP, 63, TCP_HEADER_LEN);
> > +if (svc_mon->is_ip6) {
> > +struct in6_addr ip6_src;
> > +ipv6_parse(svc_mon->sb_svc_mon->src_ip, _src);
> > +pinctrl_compose_ipv6(, eth_src, svc_mon->ea,
> > + _src, _mon->ip, IPPROTO_TCP,
> > + 63, TCP_HEADER_LEN);
> > +} else {
> > +ovs_be32 ip4_src;
> > +ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
> > +pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
> > + ip4_src,
Re: [ovs-dev] [PATCH ovn] Add IPv6 support for lb health-check
On Tue, Nov 22, 2022 at 12:54 PM Lorenzo Bianconi
wrote:
>
> Add Similar to IPv4 counterpart, introduce IPv6 load-balancer health
> check support.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2136094
> Signed-off-by: Lorenzo Bianconi
Hi Lorenzo,
Thanks for adding this missing feature. Please see a few comments below.
> ---
> controller/pinctrl.c| 213 -
> northd/northd.c | 77 ++
> northd/ovn-northd.8.xml | 17 +++
> ovn-nb.xml | 8 +-
> tests/ovn.at| 201 ++-
> tests/system-ovn.at | 230 +++-
> 6 files changed, 647 insertions(+), 99 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index f44775c7e..89c2f207b 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -6676,7 +6676,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
> ovs_be32 ip4;
> if (ip_parse(sb_svc_mon->ip, )) {
> ip_addr = in6_addr_mapped_ipv4(ip4);
> -} else {
> +} else if (!ipv6_parse(sb_svc_mon->ip, _addr)) {
> continue;
> }
>
> @@ -6689,16 +6689,27 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
> continue;
> }
>
> -for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> -if (ip4 == laddrs.ipv4_addrs[j].addr) {
> -ea = laddrs.ea;
> -mac_found = true;
> -break;
> +if (IN6_IS_ADDR_V4MAPPED(_addr)) {
> +for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
> +if (ip4 == laddrs.ipv4_addrs[j].addr) {
> +ea = laddrs.ea;
> +mac_found = true;
> +break;
> +}
> +}
> +} else {
> +for (size_t j = 0; j < laddrs.n_ipv6_addrs; j++) {
> +if (!memcmp(_addr, _addrs[j].addr,
> +sizeof(ovs_be32[4]))) {
It's a bit odd to use size of ovs_be32 here since we are memcmping
'struct in6_addr'.
I'd suggest using the macro - IN6_ARE_ADDR_EQUAL here.
> +ea = laddrs.ea;
> +mac_found = true;
> +break;
> +}
> }
> }
>
> -if (!mac_found && !laddrs.n_ipv4_addrs) {
> -/* IPv4 address(es) are not configured. Use the first mac. */
> +if (!mac_found && !laddrs.n_ipv4_addrs && !laddrs.n_ipv6_addrs) {
> +/* IP address(es) are not configured. Use the first mac. */
> ea = laddrs.ea;
> mac_found = true;
> }
> @@ -6732,7 +6743,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
> svc_mon->port_key = port_key;
> svc_mon->proto_port = sb_svc_mon->port;
> svc_mon->ip = ip_addr;
> -svc_mon->is_ip6 = false;
> +svc_mon->is_ip6 = !IN6_IS_ADDR_V4MAPPED(_addr);
> svc_mon->state = SVC_MON_S_INIT;
> svc_mon->status = SVC_MON_ST_UNKNOWN;
> svc_mon->protocol = protocol;
> @@ -7500,26 +7511,30 @@ svc_monitor_send_tcp_health_check__(struct rconn
> *swconn,
> ovs_be32 tcp_ack,
> ovs_be16 tcp_src)
> {
> -if (svc_mon->is_ip6) {
> -return;
> -}
> -
> /* Compose a TCP-SYN packet. */
> uint64_t packet_stub[128 / 8];
> struct dp_packet packet;
> +dp_packet_use_stub(, packet_stub, sizeof packet_stub);
>
> struct eth_addr eth_src;
> eth_addr_from_string(svc_mon->sb_svc_mon->src_mac, _src);
> -ovs_be32 ip4_src;
> -ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
> -
> -dp_packet_use_stub(, packet_stub, sizeof packet_stub);
> -pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
> - ip4_src, in6_addr_get_mapped_ipv4(_mon->ip),
> - IPPROTO_TCP, 63, TCP_HEADER_LEN);
> +if (svc_mon->is_ip6) {
> +struct in6_addr ip6_src;
> +ipv6_parse(svc_mon->sb_svc_mon->src_ip, _src);
> +pinctrl_compose_ipv6(, eth_src, svc_mon->ea,
> + _src, _mon->ip, IPPROTO_TCP,
> + 63, TCP_HEADER_LEN);
> +} else {
> +ovs_be32 ip4_src;
> +ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
> +pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
> + ip4_src, in6_addr_get_mapped_ipv4(_mon->ip),
> + IPPROTO_TCP, 63, TCP_HEADER_LEN);
> +}
>
> struct tcp_header *th = dp_packet_l4();
> dp_packet_set_l4(, th);
> +th->tcp_csum = 0;
> th->tcp_dst = htons(svc_mon->proto_port);
> th->tcp_src = tcp_src;
>
> @@ -7530,7 +7545,11 @@
[ovs-dev] [PATCH ovn] Add IPv6 support for lb health-check
Add Similar to IPv4 counterpart, introduce IPv6 load-balancer health
check support.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2136094
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c| 213 -
northd/northd.c | 77 ++
northd/ovn-northd.8.xml | 17 +++
ovn-nb.xml | 8 +-
tests/ovn.at| 201 ++-
tests/system-ovn.at | 230 +++-
6 files changed, 647 insertions(+), 99 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index f44775c7e..89c2f207b 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6676,7 +6676,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
ovs_be32 ip4;
if (ip_parse(sb_svc_mon->ip, )) {
ip_addr = in6_addr_mapped_ipv4(ip4);
-} else {
+} else if (!ipv6_parse(sb_svc_mon->ip, _addr)) {
continue;
}
@@ -6689,16 +6689,27 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
continue;
}
-for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
-if (ip4 == laddrs.ipv4_addrs[j].addr) {
-ea = laddrs.ea;
-mac_found = true;
-break;
+if (IN6_IS_ADDR_V4MAPPED(_addr)) {
+for (size_t j = 0; j < laddrs.n_ipv4_addrs; j++) {
+if (ip4 == laddrs.ipv4_addrs[j].addr) {
+ea = laddrs.ea;
+mac_found = true;
+break;
+}
+}
+} else {
+for (size_t j = 0; j < laddrs.n_ipv6_addrs; j++) {
+if (!memcmp(_addr, _addrs[j].addr,
+sizeof(ovs_be32[4]))) {
+ea = laddrs.ea;
+mac_found = true;
+break;
+}
}
}
-if (!mac_found && !laddrs.n_ipv4_addrs) {
-/* IPv4 address(es) are not configured. Use the first mac. */
+if (!mac_found && !laddrs.n_ipv4_addrs && !laddrs.n_ipv6_addrs) {
+/* IP address(es) are not configured. Use the first mac. */
ea = laddrs.ea;
mac_found = true;
}
@@ -6732,7 +6743,7 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
svc_mon->port_key = port_key;
svc_mon->proto_port = sb_svc_mon->port;
svc_mon->ip = ip_addr;
-svc_mon->is_ip6 = false;
+svc_mon->is_ip6 = !IN6_IS_ADDR_V4MAPPED(_addr);
svc_mon->state = SVC_MON_S_INIT;
svc_mon->status = SVC_MON_ST_UNKNOWN;
svc_mon->protocol = protocol;
@@ -7500,26 +7511,30 @@ svc_monitor_send_tcp_health_check__(struct rconn
*swconn,
ovs_be32 tcp_ack,
ovs_be16 tcp_src)
{
-if (svc_mon->is_ip6) {
-return;
-}
-
/* Compose a TCP-SYN packet. */
uint64_t packet_stub[128 / 8];
struct dp_packet packet;
+dp_packet_use_stub(, packet_stub, sizeof packet_stub);
struct eth_addr eth_src;
eth_addr_from_string(svc_mon->sb_svc_mon->src_mac, _src);
-ovs_be32 ip4_src;
-ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
-
-dp_packet_use_stub(, packet_stub, sizeof packet_stub);
-pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
- ip4_src, in6_addr_get_mapped_ipv4(_mon->ip),
- IPPROTO_TCP, 63, TCP_HEADER_LEN);
+if (svc_mon->is_ip6) {
+struct in6_addr ip6_src;
+ipv6_parse(svc_mon->sb_svc_mon->src_ip, _src);
+pinctrl_compose_ipv6(, eth_src, svc_mon->ea,
+ _src, _mon->ip, IPPROTO_TCP,
+ 63, TCP_HEADER_LEN);
+} else {
+ovs_be32 ip4_src;
+ip_parse(svc_mon->sb_svc_mon->src_ip, _src);
+pinctrl_compose_ipv4(, eth_src, svc_mon->ea,
+ ip4_src, in6_addr_get_mapped_ipv4(_mon->ip),
+ IPPROTO_TCP, 63, TCP_HEADER_LEN);
+}
struct tcp_header *th = dp_packet_l4();
dp_packet_set_l4(, th);
+th->tcp_csum = 0;
th->tcp_dst = htons(svc_mon->proto_port);
th->tcp_src = tcp_src;
@@ -7530,7 +7545,11 @@ svc_monitor_send_tcp_health_check__(struct rconn *swconn,
th->tcp_winsz = htons(65160);
uint32_t csum;
-csum = packet_csum_pseudoheader(dp_packet_l3());
+if (svc_mon->is_ip6) {
+csum = packet_csum_pseudoheader6(dp_packet_l3());
+} else {
+csum = packet_csum_pseudoheader(dp_packet_l3());
+}
csum = csum_continue(csum, th, dp_packet_size() -
((const unsigned char *)th -
(const unsigned char
