On 2/20/25 3:41 PM, Lucas Vargas Dias wrote:
> Hi Dumitru,
>
>
>
>
> Em qui., 20 de fev. de 2025 às 11:19, Dumitru Ceara <[email protected]>
> escreveu:
>
>> On 2/7/25 2:43 PM, Lucas Vargas Dias via dev wrote:
>>> Fix the prefix filter function as the return condition when IPv6
>>> prefixes have same length. If denylist prefix and prefix verified
>>> have the same length, it must be have compared only.
>>> Without this fix if denylist filter has
>>> 2003:db08::/64 and it exists a route to 2003:db88, this route
>>> is blocked because the calc for IPv6 doesn't check if they have
>>> the same prefix lenght.
>>> AND operator between prefix and denylist prefix will be
>>> 2003:db08::/68 in this example (2003:db88::/64 & 2003:db08::64)
>>>
>>> Signed-off-by: Lucas Vargas Dias <[email protected]>
>>> ---
>>
>> Hi Lucas,
>>
>> Thanks for the patch!
>>
>>> ic/ovn-ic.c | 12 +++++++++++-
>>> tests/ovn-ic.at | 14 ++++++++++----
>>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>>> index 8320cbea5..7339811a4 100644
>>> --- a/ic/ovn-ic.c
>>> +++ b/ic/ovn-ic.c
>>> @@ -1073,12 +1073,22 @@ prefix_is_deny_listed(const struct smap
>> *nb_options,
>>> }
>>> } else {
>>> struct in6_addr mask = ipv6_create_mask(plen);
>>> + struct in6_addr m_bl_prefix = ipv6_addr_bitand(&bl_prefix,
>> &mask);
>>> +
>>> + if (plen == bl_plen) {
>>> + struct in6_addr prefix_v6 = ipv6_addr_bitand(prefix,
>> &mask);
>>> + if (!ipv6_addr_equals(&prefix_v6, &m_bl_prefix)) {
>>> + continue;
>>> + }
>>> + matched = true;
>>> + break;
>>> + }
>>
>> Why isn't this a problem for IPv4 too?
>>
>> Because for ipv4 it will be compared in if IN6_IS_ADDR_V4MAPPED and
> it compares (prefix and mask) with (bl_prefix and mask). It considers the
> mask
> before the comparison.
>
>
OK, thanks for the reply. But then why don't we do the same thing for IPv6?
The following passes your test:
struct in6_addr bl_mask = ipv6_create_mask(bl_plen);
struct in6_addr m_prefix = ipv6_addr_bitand(prefix, &bl_mask);
struct in6_addr m_bl_prefix = ipv6_addr_bitand(&bl_prefix, &bl_mask);
if (!ipv6_addr_equals(&m_prefix, &m_bl_prefix)) {
continue;
}
Would this be correct? Or am I missing something?
Thanks,
Dumitru
>>> /* First calculate the difference between bl_prefix and
>> prefix, so
>>> * use the bl mask to ensure prefixes are correctly
>> validated.
>>> * e.g.: 2005:1734:5678::/50 is a subnet of 2005:1234::/21
>> */
>>> struct in6_addr m_prefixes = ipv6_addr_bitand(prefix,
>> &bl_prefix);
>>> struct in6_addr m_prefix = ipv6_addr_bitand(&m_prefixes,
>> &mask);
>>> - struct in6_addr m_bl_prefix = ipv6_addr_bitand(&bl_prefix,
>> &mask);
>>> +
>>> if (!ipv6_addr_equals(&m_prefix, &m_bl_prefix)) {
>>> continue;
>>> }
>>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>>> index 9fc386131..0ce08260c 100644
>>> --- a/tests/ovn-ic.at
>>> +++ b/tests/ovn-ic.at
>>> @@ -1352,7 +1352,7 @@ for i in 1 2; do
>>> check ovn-nbctl set nb_global . options:ic-route-adv=true
>>> # Enable denylist single filter for IPv6
>>> check ovn-nbctl set nb_global . options:ic-route-denylist=" \
>>> - 2003:db8:1::/64,2004:aaaa::/32,2005:1234::/21"
>>> + 2003:db08:1::/64,2004:aaaa::/32,2005:1234::/21"
>>>
>>> check ovn-ic-nbctl --wait=sb sync
>>> # Create LRP and connect to TS
>>> @@ -1369,7 +1369,10 @@ for i in 1 2; do
>>>
>>> # Create denylisted LRPs and connect to TS
>>> check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
>>> - 11:11:11:11:11:1$i 2003:db8:1::$i/64
>>> + 11:11:11:11:11:1$i 2003:db88:1::$i/64
>>> +
>>> + check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext1$i \
>>> + 11:11:11:11:12:1$i 2003:db08:1::$i/64
>>>
>>> check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
>>> 22:22:22:22:22:2$i 2004:aaaa:bbb::$i/48
>>> @@ -1388,6 +1391,7 @@ check ovn-ic-nbctl --wait=sb sync
>>> AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
>>> awk '/learned/{print $1, $2}' ], [0], [dnl
>>> 2002:db8:1::/64 2001:db8:1::2
>>> +2003:db88:1::/64 2001:db8:1::2
>>> 2005:1834:5678::/50 2001:db8:1::2
>>> ])
>>>
>>> @@ -1403,7 +1407,8 @@ check ovn-ic-nbctl --wait=sb sync
>>> AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
>>> awk '/learned/{print $1, $2}' | sort ], [0], [dnl
>>> 2002:db8:1::/64 2001:db8:1::2
>>> -2003:db8:1::/64 2001:db8:1::2
>>> +2003:db08:1::/64 2001:db8:1::2
>>> +2003:db88:1::/64 2001:db8:1::2
>>> 2004:aaaa:bbb::/48 2001:db8:1::2
>>> 2005:1734:5678::/50 2001:db8:1::2
>>> 2005:1834:5678::/50 2001:db8:1::2
>>> @@ -1413,7 +1418,7 @@ for i in 1 2; do
>>> ovn_as az$i
>>>
>>> check ovn-nbctl set nb_global . \
>>> - options:ic-route-denylist="2003:db8:1::/64,2004:db8:1::/64"
>>> + options:ic-route-denylist="2003:db88:1::/64,2004:db8:1::/64"
>>>
>>> # Create an 'extra' denylisted LRP and connect to TS
>>> check ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext5$i \
>>> @@ -1424,6 +1429,7 @@ check ovn-ic-nbctl --wait=sb sync
>>> AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 |
>>> awk '/learned/{print $1, $2}' | sort ], [0], [dnl
>>> 2002:db8:1::/64 2001:db8:1::2
>>> +2003:db08:1::/64 2001:db8:1::2
>>> 2004:aaaa:bbb::/48 2001:db8:1::2
>>> 2005:1734:5678::/50 2001:db8:1::2
>>> 2005:1834:5678::/50 2001:db8:1::2
>>
>> Regards,
>> Dumitru
>>
>>
>> Regards,
> Lucas
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
