Thanks for your review Ales. On Fri, 2 Feb 2024 at 07:34 Ales Musil <[email protected]> wrote:
> > > On Tue, Jan 30, 2024 at 3:10 PM Roberto Bartzen Acosta via dev < > [email protected]> wrote: > >> This commit fixes the prefix filter function as the return condition for >> IPv6 addresses is disabling the advertisement of all learned prefixes >> regardless of the match with the blacklist or not. >> >> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804 >> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.") >> Signed-off-by: Roberto Bartzen Acosta <[email protected]> >> --- >> > > Hi Roberto, > > thank you for the patch. > > >> ic/ovn-ic.c | 22 ++++++++--- >> tests/ovn-ic.at | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 116 insertions(+), 6 deletions(-) >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index 6f8f5734d..1c9c9ae2c 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -1024,6 +1024,20 @@ prefix_is_link_local(struct in6_addr *prefix, >> unsigned int plen) >> ((prefix->s6_addr[1] & 0xc0) == 0x80)); >> } >> >> +static bool >> +compare_ipv6_prefixes(const struct in6_addr *s_prefix, >> + const struct in6_addr *d_prefix2, int plen) >> +{ >> + struct in6_addr mask = ipv6_create_mask(plen); >> + for (int i = 0; i <= (plen / 8); i++) { >> + if ((s_prefix->s6_addr[i] & mask.s6_addr[i]) ^ >> + (d_prefix2->s6_addr[i] & mask.s6_addr[i])) { >> + return false; >> + } >> + } >> + return true; >> +} >> + >> > static bool >> prefix_is_black_listed(const struct smap *nb_options, >> struct in6_addr *prefix, >> @@ -1064,12 +1078,8 @@ prefix_is_black_listed(const struct smap >> *nb_options, >> continue; >> } >> } else { >> - struct in6_addr mask = ipv6_create_mask(bl_plen); >> - for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) { >> - if ((prefix->s6_addr[i] & mask.s6_addr[i]) >> - != (bl_prefix.s6_addr[i] & mask.s6_addr[i])) { >> - continue; >> - } >> + if (!compare_ipv6_prefixes(prefix, &bl_prefix, bl_plen)) { >> + continue; >> } >> > > > There is no need for hand implementation, OvS provides plenty of helpers. > This can be replaced with something like the snippet below: > > struct in6_addr mask = ipv6_create_mask(bl_plen); > struct in6_addr m_prefix = ipv6_addr_bitand(prefix, &mask); > struct in6_addr m_bl_prefix = ipv6_addr_bitand(&bl_prefix, &mask); > > if (!ipv6_addr_equals(&m_prefix, &m_bl_prefix)) { > continue; > } > Sure, this makes sense. I will change it and send a new patch version. Thanks > >> } >> matched = true; >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> index d4c436f84..1f9df71e9 100644 >> --- a/tests/ovn-ic.at >> +++ b/tests/ovn-ic.at >> @@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2]) >> >> AT_CLEANUP >> ]) >> + >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter]) >> +AT_KEYWORDS([IPv6-route-sync-blacklist]) >> + >> +ovn_init_ic_db >> +ovn-ic-nbctl ts-add ts1 >> + >> +for i in 1 2; do >> + ovn_start az$i >> + ovn_as az$i >> + >> + # Enable route learning at AZ level >> + ovn-nbctl set nb_global . options:ic-route-learn=true >> + # Enable route advertising at AZ level >> + ovn-nbctl set nb_global . options:ic-route-adv=true >> + # Enable blacklist single filter for IPv6 >> + ovn-nbctl set nb_global . >> options:ic-route-blacklist="2003:db8:1::/64,\ >> + 2004:aaaa::/32,2005:1234::/21" >> + >> + OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1]) >> + >> + # Create LRP and connect to TS >> + ovn-nbctl lr-add lr$i >> + ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i >> 2001:db8:1::$i/64 >> + ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \ >> + -- lsp-set-addresses lsp-ts1-lr$i router \ >> + -- lsp-set-type lsp-ts1-lr$i router \ >> + -- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1 >> + >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i >> 2002:db8:1::$i/64 >> + >> + # Create blacklisted LRPs and connect to TS >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \ >> + 11:11:11:11:11:1$i 2003:db8:1::$i/64 >> + >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \ >> + 22:22:22:22:22:2$i 2004:aaaa:bbb::$i/48 >> + >> + # filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:) >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \ >> + 33:33:33:33:33:3$i 2005:1734:5678::$i/50 >> + >> + # additional not filtered prefix -> different subnet bits >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \ >> + 33:33:33:33:33:3$i 2005:1834:5678::$i/50 >> + >> +done >> + >> +for i in 1 2; do >> + OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep >> learned]) >> +done >> + >> +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 >> +2005:1834:5678::/50 2001:db8:1::2 >> +]) >> + >> +for i in 1 2; do >> + ovn_as az$i >> + >> + # Drop blacklist >> + ovn-nbctl remove nb_global . options ic-route-blacklist >> + >> +done >> + >> +OVS_WAIT_FOR_OUTPUT([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 >> +2004:aaaa:bbb::/48 2001:db8:1::2 >> +2005:1734:5678::/50 2001:db8:1::2 >> +2005:1834:5678::/50 2001:db8:1::2 >> +]) >> + >> +for i in 1 2; do >> + ovn_as az$i >> + >> + ovn-nbctl set nb_global . \ >> + options:ic-route-blacklist="2003:db8:1::/64,2004:db8:1::/64" >> + >> + # Create an 'extra' blacklisted LRP and connect to TS >> + ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \ >> + 44:44:44:44:44:4$i 2004:db8:1::$i/64 >> + >> +done >> + >> +OVS_WAIT_FOR_OUTPUT([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 >> +2004:aaaa:bbb::/48 2001:db8:1::2 >> +2005:1734:5678::/50 2001:db8:1::2 >> +2005:1834:5678::/50 2001:db8:1::2 >> +]) >> + >> +OVN_CLEANUP_IC([az1], [az2]) >> + >> +AT_CLEANUP >> +]) >> -- >> 2.25.1 >> >> >> -- >> >> >> >> >> _'Esta mensagem é direcionada apenas para os endereços constantes no >> cabeçalho inicial. Se você não está listado nos endereços constantes no >> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa >> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas >> estão >> imediatamente anuladas e proibidas'._ >> >> >> * **'Apesar do Magazine Luiza tomar >> todas as precauções razoáveis para assegurar que nenhum vírus esteja >> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade >> por >> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Thank you, > Ales. > > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] > <https://red.ht/sig> > -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
