Re: [ovs-dev] [PATCH ovn v1] northd: Allow /64 after ipv6_prefix
On Fri, Feb 21, 2020 at 1:03 AM Russell Bryant wrote:
>
> On Thu, Feb 20, 2020 at 10:46 AM Numan Siddique wrote:
>
> > On Wed, Feb 19, 2020 at 9:27 PM Russell Bryant 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
> >
> > With the below check patch warnings fixed
> >
> > Acked-by: Numan Siddique
> >
> >
> 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(, "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(, "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(>nbs->other_config,
> > "ipv6_prefix");
> > >
> > > if (ipv6_prefix) {
> > > -od->ipam_info.ipv6_prefix_set = ipv6_parse(
> > > -ipv6_prefix, >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,
> > > +>ipam_info.ipv6_prefix, );
> > > +if (error) {
> > > +static struct vlog_rate_limit rl
> > > += VLOG_RATE_LIMIT_INIT(5, 1);
> > > +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: %s",
> > ipv6_prefix, error);
> > > +free(error);
> > > +} else {
> > > +if (ipv6_count_cidr_bits() == 64) {
> > > +od->ipam_info.ipv6_prefix_set = true;
> > > +} else {
> > > +static struct vlog_rate_limit rl
> > > += VLOG_RATE_LIMIT_INIT(5, 1);
> > > +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: must be
> > /64", ipv6_prefix);
> > > +}
> > > +}
> > > +} else {
> > > +od->ipam_info.ipv6_prefix_set = ipv6_parse(
> > > +ipv6_prefix, >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(, "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
Re: [ovs-dev] [PATCH ovn v1] northd: Allow /64 after ipv6_prefix
On Thu, Feb 20, 2020 at 10:46 AM Numan Siddique wrote:
> On Wed, Feb 19, 2020 at 9:27 PM Russell Bryant 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
>
> With the below check patch warnings fixed
>
> Acked-by: Numan Siddique
>
>
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(, "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(, "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(>nbs->other_config,
> "ipv6_prefix");
> >
> > if (ipv6_prefix) {
> > -od->ipam_info.ipv6_prefix_set = ipv6_parse(
> > -ipv6_prefix, >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,
> > +>ipam_info.ipv6_prefix, );
> > +if (error) {
> > +static struct vlog_rate_limit rl
> > += VLOG_RATE_LIMIT_INIT(5, 1);
> > +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: %s",
> ipv6_prefix, error);
> > +free(error);
> > +} else {
> > +if (ipv6_count_cidr_bits() == 64) {
> > +od->ipam_info.ipv6_prefix_set = true;
> > +} else {
> > +static struct vlog_rate_limit rl
> > += VLOG_RATE_LIMIT_INIT(5, 1);
> > +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: must be
> /64", ipv6_prefix);
> > +}
> > +}
> > +} else {
> > +od->ipam_info.ipv6_prefix_set = ipv6_parse(
> > +ipv6_prefix, >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(, "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
Re: [ovs-dev] [PATCH ovn v1] northd: Allow /64 after ipv6_prefix
On Wed, Feb 19, 2020 at 9:27 PM Russell Bryant 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
With the below check patch warnings fixed
Acked-by: Numan Siddique
WARNING: Line is 82 characters long (recommended limit is 79)
#40 FILE: northd/ovn-northd.c:676:
VLOG_WARN_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(, "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(>nbs->other_config,
> "ipv6_prefix");
>
> if (ipv6_prefix) {
> -od->ipam_info.ipv6_prefix_set = ipv6_parse(
> -ipv6_prefix, >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,
> +>ipam_info.ipv6_prefix, );
> +if (error) {
> +static struct vlog_rate_limit rl
> += VLOG_RATE_LIMIT_INIT(5, 1);
> +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: %s", ipv6_prefix,
> error);
> +free(error);
> +} else {
> +if (ipv6_count_cidr_bits() == 64) {
> +od->ipam_info.ipv6_prefix_set = true;
> +} else {
> +static struct vlog_rate_limit rl
> += VLOG_RATE_LIMIT_INIT(5, 1);
> +VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: must be /64",
> ipv6_prefix);
> +}
> +}
> +} else {
> +od->ipam_info.ipv6_prefix_set = ipv6_parse(
> +ipv6_prefix, >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(, "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
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v1] northd: Allow /64 after ipv6_prefix
Bleep bloop. Greetings Russell Bryant, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 82 characters long (recommended limit is 79) #41 FILE: northd/ovn-northd.c:676: VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: %s", ipv6_prefix, error); WARNING: Line is 88 characters long (recommended limit is 79) #49 FILE: northd/ovn-northd.c:684: VLOG_WARN_RL(, "bad 'ipv6_prefix' %s: must be /64", ipv6_prefix); Lines checked: 82, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
