On Wed, Feb 19, 2020 at 9:27 PM Russell Bryant <[email protected]> wrote:
>
> We recently hit a bug in ovn-kubernetes, where I accidentally added
> /64 at the end of ipv6_prefix, to match the format we used for the
> subnet option for IPv4.  This was not allowed.
>
> This patch update ovn-northd to take the ipv6_prefix either with or
> without a trailing "/64".  It still enforces a /64 CIDR prefix length.
>
> A test case was updated to ensure that a prefix with "/64" is now
> accepted.
>
> Signed-off-by: Russell Bryant <[email protected]>

With the below check patch warnings fixed

Acked-by: Numan Siddique <[email protected]>

****
WARNING: Line is 82 characters long (recommended limit is 79)
#40 FILE: northd/ovn-northd.c:676:
                VLOG_WARN_RL(&rl, "bad 'ipv6_prefix' %s: %s",
ipv6_prefix, error);

WARNING: Line is 88 characters long (recommended limit is 79)
#48 FILE: northd/ovn-northd.c:684:
                    VLOG_WARN_RL(&rl, "bad 'ipv6_prefix' %s: must be
/64", ipv6_prefix);

****

Thanks
Numan

> ---
>  northd/ovn-northd.c | 31 +++++++++++++++++++++++++++++--
>  tests/ovn.at        |  4 +++-
>  2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 2580b4ec9..59d085aa9 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -664,8 +664,35 @@ init_ipam_info_for_datapath(struct ovn_datapath *od)
>      const char *ipv6_prefix = smap_get(&od->nbs->other_config, 
> "ipv6_prefix");
>
>      if (ipv6_prefix) {
> -        od->ipam_info.ipv6_prefix_set = ipv6_parse(
> -            ipv6_prefix, &od->ipam_info.ipv6_prefix);
> +        if (strstr(ipv6_prefix, "/")) {
> +            /* If a prefix length was specified, it must be 64. */
> +            struct in6_addr mask;
> +            char *error
> +                = ipv6_parse_masked(ipv6_prefix,
> +                                    &od->ipam_info.ipv6_prefix, &mask);
> +            if (error) {
> +                static struct vlog_rate_limit rl
> +                    = VLOG_RATE_LIMIT_INIT(5, 1);
> +                VLOG_WARN_RL(&rl, "bad 'ipv6_prefix' %s: %s", ipv6_prefix, 
> error);
> +                free(error);
> +            } else {
> +                if (ipv6_count_cidr_bits(&mask) == 64) {
> +                    od->ipam_info.ipv6_prefix_set = true;
> +                } else {
> +                    static struct vlog_rate_limit rl
> +                        = VLOG_RATE_LIMIT_INIT(5, 1);
> +                    VLOG_WARN_RL(&rl, "bad 'ipv6_prefix' %s: must be /64", 
> ipv6_prefix);
> +                }
> +            }
> +        } else {
> +            od->ipam_info.ipv6_prefix_set = ipv6_parse(
> +                ipv6_prefix, &od->ipam_info.ipv6_prefix);
> +            if (!od->ipam_info.ipv6_prefix_set) {
> +                static struct vlog_rate_limit rl
> +                    = VLOG_RATE_LIMIT_INIT(5, 1);
> +                VLOG_WARN_RL(&rl, "bad 'ipv6_prefix' %s", ipv6_prefix);
> +            }
> +        }
>      }
>
>      if (!subnet_str) {
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 254645a3a..cbaa6d4a2 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -12289,8 +12289,10 @@ ovn-nbctl set Logical_Switch ls1 \
>      other_config:subnet=10.1.0.0/24 other_config:ipv6_prefix="2001:db8:1::"
>  ovn-nbctl set Logical_Switch ls2 \
>      other_config:subnet=10.2.0.0/24 other_config:ipv6_prefix="2001:db8:2::"
> +
> +# A prefix length may be specified, but only if it is /64.
>  ovn-nbctl set Logical_Switch ls3 \
> -    other_config:subnet=10.3.0.0/24 other_config:ipv6_prefix="2001:db8:3::"
> +    other_config:subnet=10.3.0.0/24 
> other_config:ipv6_prefix="2001:db8:3::/64"
>
>  ovn-nbctl lsp-add ls1 lp1
>  ovn-nbctl lsp-add ls2 lp2
> --
> 2.24.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to