Re: [ovs-dev] [PATCH ovn] northd: Fix population of ipv6_ra_prefixes from IPv6 PD.

2024-03-28 Thread Dumitru Ceara 
On 3/28/24 15:19, Frode Nordahl wrote:
> On Thu, Mar 28, 2024 at 3:03 PM Dumitru Ceara  wrote:
>>
>> On 3/18/24 22:21, Mark Michelson wrote:
>>> Thanks Frode, looks good to me.
>>>
>>> Acked-by: Mark Michelson 
>>>
>>> On 3/18/24 07:43, Frode Nordahl wrote:
 The current code puts the contents of the ``ipb6_ra_pd_list``
 option verbatim into the ``ipv6_ra_prefixes`` option.

 This does not work, because the ``ipv6_ra_pd_list`` is not an IPv6
 prefix, but a string composed of aid:prefix/length, and as a
 consequence the controller would log a message like this:

  pinctrl|WARN|Invalid IPv6 prefixes: 18578:fde8:7f0f:11fe:8::/62

 Northd already parses the ``ipv6_ra_pd_list`` string and
 populates the ``ipv6_prefix`` list of strings.

 Make use of the ``ipv6_prefix`` list of strings to populate the
 ``ipv6_ra_prefixes`` option.

 Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
 Signed-off-by: Frode Nordahl 
 ---
   northd/northd.c | 7 ---
   1 file changed, 4 insertions(+), 3 deletions(-)

 diff --git a/northd/northd.c b/northd/northd.c
 index 1839b7d8b..7fdd722b5 100644
 --- a/northd/northd.c
 +++ b/northd/northd.c
 @@ -11378,10 +11378,11 @@ copy_ra_to_sb(struct ovn_port *op, const
 char *address_mode)
   ds_put_format(&s, "%s/%u ", addrs->network_s, addrs->plen);
   }
   -const char *ra_pd_list = smap_get(&op->sb->options,
 "ipv6_ra_pd_list");
 -if (ra_pd_list) {
 -ds_put_cstr(&s, ra_pd_list);
 +for (int i = 0; i < op->nbrp->n_ipv6_prefix; i++) {
>>
>> I changed this to 'size_t i' and then pushed it to main and backported
>> down to 23.06.
> 
> Thanks alot for reviews, merges and backports, Dumitru and Mark. I'll
> make sure to use `size_t` as array iterators moving forward.
> 

No worries.  Looking at the code base we have more than 130 other places
that use "int i" as array iterator.  It's not that terrible, I was nit
picking a bit. :)


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] northd: Fix population of ipv6_ra_prefixes from IPv6 PD.

2024-03-28 Thread Frode Nordahl 
On Thu, Mar 28, 2024 at 3:03 PM Dumitru Ceara  wrote:
>
> On 3/18/24 22:21, Mark Michelson wrote:
> > Thanks Frode, looks good to me.
> >
> > Acked-by: Mark Michelson 
> >
> > On 3/18/24 07:43, Frode Nordahl wrote:
> >> The current code puts the contents of the ``ipb6_ra_pd_list``
> >> option verbatim into the ``ipv6_ra_prefixes`` option.
> >>
> >> This does not work, because the ``ipv6_ra_pd_list`` is not an IPv6
> >> prefix, but a string composed of aid:prefix/length, and as a
> >> consequence the controller would log a message like this:
> >>
> >>  pinctrl|WARN|Invalid IPv6 prefixes: 18578:fde8:7f0f:11fe:8::/62
> >>
> >> Northd already parses the ``ipv6_ra_pd_list`` string and
> >> populates the ``ipv6_prefix`` list of strings.
> >>
> >> Make use of the ``ipv6_prefix`` list of strings to populate the
> >> ``ipv6_ra_prefixes`` option.
> >>
> >> Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
> >> Signed-off-by: Frode Nordahl 
> >> ---
> >>   northd/northd.c | 7 ---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/northd/northd.c b/northd/northd.c
> >> index 1839b7d8b..7fdd722b5 100644
> >> --- a/northd/northd.c
> >> +++ b/northd/northd.c
> >> @@ -11378,10 +11378,11 @@ copy_ra_to_sb(struct ovn_port *op, const
> >> char *address_mode)
> >>   ds_put_format(&s, "%s/%u ", addrs->network_s, addrs->plen);
> >>   }
> >>   -const char *ra_pd_list = smap_get(&op->sb->options,
> >> "ipv6_ra_pd_list");
> >> -if (ra_pd_list) {
> >> -ds_put_cstr(&s, ra_pd_list);
> >> +for (int i = 0; i < op->nbrp->n_ipv6_prefix; i++) {
>
> I changed this to 'size_t i' and then pushed it to main and backported
> down to 23.06.

Thanks alot for reviews, merges and backports, Dumitru and Mark. I'll
make sure to use `size_t` as array iterators moving forward.

-- 
Frode Nordahl

> Thanks, Frode and Mark!
>
> Regards,
> Dumitru
>
> >> +ds_put_cstr(&s, op->nbrp->ipv6_prefix[i]);
> >> +ds_put_char(&s, ' ');
> >>   }
> >> +
> >>   /* Remove trailing space */
> >>   ds_chomp(&s, ' ');
> >>   smap_add(&options, "ipv6_ra_prefixes", ds_cstr(&s));
> >
> > ___
> > 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] northd: Fix population of ipv6_ra_prefixes from IPv6 PD.

2024-03-28 Thread Dumitru Ceara 
On 3/18/24 22:21, Mark Michelson wrote:
> Thanks Frode, looks good to me.
> 
> Acked-by: Mark Michelson 
> 
> On 3/18/24 07:43, Frode Nordahl wrote:
>> The current code puts the contents of the ``ipb6_ra_pd_list``
>> option verbatim into the ``ipv6_ra_prefixes`` option.
>>
>> This does not work, because the ``ipv6_ra_pd_list`` is not an IPv6
>> prefix, but a string composed of aid:prefix/length, and as a
>> consequence the controller would log a message like this:
>>
>>  pinctrl|WARN|Invalid IPv6 prefixes: 18578:fde8:7f0f:11fe:8::/62
>>
>> Northd already parses the ``ipv6_ra_pd_list`` string and
>> populates the ``ipv6_prefix`` list of strings.
>>
>> Make use of the ``ipv6_prefix`` list of strings to populate the
>> ``ipv6_ra_prefixes`` option.
>>
>> Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
>> Signed-off-by: Frode Nordahl 
>> ---
>>   northd/northd.c | 7 ---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 1839b7d8b..7fdd722b5 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -11378,10 +11378,11 @@ copy_ra_to_sb(struct ovn_port *op, const
>> char *address_mode)
>>   ds_put_format(&s, "%s/%u ", addrs->network_s, addrs->plen);
>>   }
>>   -    const char *ra_pd_list = smap_get(&op->sb->options,
>> "ipv6_ra_pd_list");
>> -    if (ra_pd_list) {
>> -    ds_put_cstr(&s, ra_pd_list);
>> +    for (int i = 0; i < op->nbrp->n_ipv6_prefix; i++) {

I changed this to 'size_t i' and then pushed it to main and backported
down to 23.06.

Thanks, Frode and Mark!

Regards,
Dumitru

>> +    ds_put_cstr(&s, op->nbrp->ipv6_prefix[i]);
>> +    ds_put_char(&s, ' ');
>>   }
>> +
>>   /* Remove trailing space */
>>   ds_chomp(&s, ' ');
>>   smap_add(&options, "ipv6_ra_prefixes", ds_cstr(&s));
> 
> ___
> 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] northd: Fix population of ipv6_ra_prefixes from IPv6 PD.

2024-03-18 Thread Mark Michelson 

Thanks Frode, looks good to me.

Acked-by: Mark Michelson 

On 3/18/24 07:43, Frode Nordahl wrote:

The current code puts the contents of the ``ipb6_ra_pd_list``
option verbatim into the ``ipv6_ra_prefixes`` option.

This does not work, because the ``ipv6_ra_pd_list`` is not an IPv6
prefix, but a string composed of aid:prefix/length, and as a
consequence the controller would log a message like this:

 pinctrl|WARN|Invalid IPv6 prefixes: 18578:fde8:7f0f:11fe:8::/62

Northd already parses the ``ipv6_ra_pd_list`` string and
populates the ``ipv6_prefix`` list of strings.

Make use of the ``ipv6_prefix`` list of strings to populate the
``ipv6_ra_prefixes`` option.

Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
Signed-off-by: Frode Nordahl 
---
  northd/northd.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 1839b7d8b..7fdd722b5 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11378,10 +11378,11 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
  ds_put_format(&s, "%s/%u ", addrs->network_s, addrs->plen);
  }
  
-const char *ra_pd_list = smap_get(&op->sb->options, "ipv6_ra_pd_list");

-if (ra_pd_list) {
-ds_put_cstr(&s, ra_pd_list);
+for (int i = 0; i < op->nbrp->n_ipv6_prefix; i++) {
+ds_put_cstr(&s, op->nbrp->ipv6_prefix[i]);
+ds_put_char(&s, ' ');
  }
+
  /* Remove trailing space */
  ds_chomp(&s, ' ');
  smap_add(&options, "ipv6_ra_prefixes", ds_cstr(&s));


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] northd: Fix population of ipv6_ra_prefixes from IPv6 PD.

2024-03-18 Thread Frode Nordahl 
The current code puts the contents of the ``ipb6_ra_pd_list``
option verbatim into the ``ipv6_ra_prefixes`` option.

This does not work, because the ``ipv6_ra_pd_list`` is not an IPv6
prefix, but a string composed of aid:prefix/length, and as a
consequence the controller would log a message like this:

pinctrl|WARN|Invalid IPv6 prefixes: 18578:fde8:7f0f:11fe:8::/62

Northd already parses the ``ipv6_ra_pd_list`` string and
populates the ``ipv6_prefix`` list of strings.

Make use of the ``ipv6_prefix`` list of strings to populate the
``ipv6_ra_prefixes`` option.

Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
Signed-off-by: Frode Nordahl 
---
 northd/northd.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 1839b7d8b..7fdd722b5 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11378,10 +11378,11 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
 ds_put_format(&s, "%s/%u ", addrs->network_s, addrs->plen);
 }
 
-const char *ra_pd_list = smap_get(&op->sb->options, "ipv6_ra_pd_list");
-if (ra_pd_list) {
-ds_put_cstr(&s, ra_pd_list);
+for (int i = 0; i < op->nbrp->n_ipv6_prefix; i++) {
+ds_put_cstr(&s, op->nbrp->ipv6_prefix[i]);
+ds_put_char(&s, ' ');
 }
+
 /* Remove trailing space */
 ds_chomp(&s, ' ');
 smap_add(&options, "ipv6_ra_prefixes", ds_cstr(&s));
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev