On Fri, Feb 21, 2020 at 1:03 AM Russell Bryant <rbry...@redhat.com> wrote:
>
> 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.

Hi Russell,
I applied this patch to branch-20.03.

Thanks
Numan

>
>
>
> > ****
> > 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
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to