| Message ID | 20240205010246.25409-1-roberto.acosta@luizalabs.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v4] ovn-ic: Fix global blacklist filter for IPv6 addresses. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Feb 5, 2024 at 2:02 AM Roberto Bartzen Acosta via dev < ovs-dev@openvswitch.org> 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 <roberto.acosta@luizalabs.com> > --- > ic/ovn-ic.c | 15 +++++--- > tests/ovn-ic.at | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 109 insertions(+), 6 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index 6f8f5734d..bc9aea057 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -1064,12 +1064,15 @@ 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; > - } > + struct in6_addr mask = ipv6_create_mask(plen); > + /* 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; > } > } > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
Hi Roberto, Thanks for the fix! I have a few minor comments though. On 2/5/24 10:01, Ales Musil wrote: > On Mon, Feb 5, 2024 at 2:02 AM Roberto Bartzen Acosta via dev < > ovs-dev@openvswitch.org> 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. Please use shorter lines in the commit message. >> >> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804 >> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.") >> Signed-off-by: Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> Should we update the .mailmap file too? The AUTHORS file has you listed with your gmail address. Anyway, that can be a separate patch or I can just add a commit when applying this fix. >> --->> ic/ovn-ic.c | 15 +++++--- >> tests/ovn-ic.at | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 109 insertions(+), 6 deletions(-) >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index 6f8f5734d..bc9aea057 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -1064,12 +1064,15 @@ 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; >> - } >> + struct in6_addr mask = ipv6_create_mask(plen); >> + /* 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; >> } >> } >> 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 Please prefix all ovn-nbctl commands with "check ". That will fail if the command errors out. It turns out your test adds some duplicate ports, in the logs: ovn-nbctl: lrp-lr1-p-ext21: a port with this name already exists ovn-nbctl: lrp-lr2-p-ext22: a port with this name already exists >> + >> + # 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 >> + Nit: no need for empty line. >> +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 >> + Nit: no need for an empty line here. >> +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 >> + Nit: no need for an empty line here. >> +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 >> Regards, Dumitru
Hi Dumitru, thanks for the feedback! I will change the test file and send a new patch as you suggested, thanks. Em sex., 9 de fev. de 2024 às 06:47, Dumitru Ceara <dceara@redhat.com> escreveu: > Hi Roberto, > > Thanks for the fix! I have a few minor comments though. > > > On 2/5/24 10:01, Ales Musil wrote: > > On Mon, Feb 5, 2024 at 2:02 AM Roberto Bartzen Acosta via dev < > > ovs-dev@openvswitch.org> 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. > > Please use shorter lines in the commit message. > > >> > >> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804 > >> Fixes: 57b347c55168 ("ovn-ic: Route advertisement.") > >> Signed-off-by: Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> > > Should we update the .mailmap file too? The AUTHORS file has you listed > with your gmail address. Anyway, that can be a separate patch or I can > just add a commit when applying this fix. > Sure, go ahead with updating the .mailmap if it's not a problem for you. Thanks > > >> --->> ic/ovn-ic.c | 15 +++++--- > >> tests/ovn-ic.at | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 109 insertions(+), 6 deletions(-) > >> > >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > >> index 6f8f5734d..bc9aea057 100644 > >> --- a/ic/ovn-ic.c > >> +++ b/ic/ovn-ic.c > >> @@ -1064,12 +1064,15 @@ 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; > >> - } > >> + struct in6_addr mask = ipv6_create_mask(plen); > >> + /* 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; > >> } > >> } > >> 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 > > Please prefix all ovn-nbctl commands with "check ". That will fail if > the command errors out. It turns out your test adds some duplicate > ports, in the logs: > > ovn-nbctl: lrp-lr1-p-ext21: a port with this name already exists > ovn-nbctl: lrp-lr2-p-ext22: a port with this name already exists > > >> + > >> + # 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 > >> + > > Nit: no need for empty line. > > >> +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 > >> + > > Nit: no need for an empty line here. > > >> +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 > >> + > > Nit: no need for an empty line here. > > >> +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 > >> > > Regards, > Dumitru > >
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c index 6f8f5734d..bc9aea057 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -1064,12 +1064,15 @@ 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; - } + struct in6_addr mask = ipv6_create_mask(plen); + /* 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; } } 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 +])
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 <roberto.acosta@luizalabs.com> --- ic/ovn-ic.c | 15 +++++--- tests/ovn-ic.at | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-)