On Thu, Feb 20, 2020 at 10:46 AM Numan Siddique <num...@ovn.org> wrote:
> On Wed, Feb 19, 2020 at 9:27 PM Russell Bryant <russ...@ovn.org> 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 <russ...@ovn.org>
>
> With the below check patch warnings fixed
>
> Acked-by: Numan Siddique <num...@ovn.org>
>
>
Thanks! I fixed the line length issues and pushed to master.
> ****
> 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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
--
Russell Bryant
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
