| Message ID | 14a9b54908070dd8641000a7e223efef90b49efa.1674841636.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] northd: do not create flows for reserved multicast IPv6 groups | 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 | fail | github build: failed |
On 1/27/23 18:50, Lorenzo Bianconi wrote: > Avoid creating logical flows for Link-Local reserved multicast addresses if > advertised in a MLD reports since this interferes with Slaac IPv6 address > resolution implemented in OVN. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2154930 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- Hi Lorenzo, > lib/ovn-util.h | 13 +++++++++++++ > northd/northd.c | 6 ++++-- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index cd19919cb..46f57ad47 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -70,6 +70,19 @@ struct lport_addresses { > struct ipv6_netaddr *ipv6_addrs; > }; > > +static inline bool ipv6_is_all_router(const struct in6_addr *addr) { Nit: according to the coding style we need a newline after the return type, after "static inline bool". > + return ipv6_addr_equals(addr, &in6addr_all_routers); > +}; > + > +static const struct in6_addr in6addr_all_site_routers = {{{ > + 0xff,0x05,0x00,0x00,0x00,0x00,0x00,0x00, > + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02 > +}}}; > + > +static inline bool ipv6_is_all_site_router(const struct in6_addr *addr) { > + return ipv6_addr_equals(addr, &in6addr_all_site_routers); > +}; > + Nit: same here. > bool is_dynamic_lsp_address(const char *address); > bool extract_addresses(const char *address, struct lport_addresses *, > int *ofs); > diff --git a/northd/northd.c b/northd/northd.c > index 0944a7b56..50d74f932 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -9020,9 +9020,11 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, > igmp_group->mcgroup.name); > } else { > /* RFC 4291, section 2.7.1: Skip groups that correspond to all > - * hosts. > + * hosts, all link-local routers and all site routers. > */ > - if (ipv6_is_all_hosts(&igmp_group->address)) { > + if (ipv6_is_all_hosts(&igmp_group->address) || > + ipv6_is_all_router(&igmp_group->address) || > + ipv6_is_all_site_router(&igmp_group->address)) { > return; > } > if (atomic_compare_exchange_strong( The change looks good to me, thanks for fixing this! Feel free to add my ack if you address the two minor comments I had above. Acked-by: Dumitru Ceara <dceara@redhat.com> Do you also plan to send an OVS patch to add ipv6_is_all_site_router() and ipv6_is_all_router() to packets.h where they actually belong? Thanks, Dumitru
On Feb 02, Dumitru Ceara wrote: > On 1/27/23 18:50, Lorenzo Bianconi wrote: > > Avoid creating logical flows for Link-Local reserved multicast addresses if > > advertised in a MLD reports since this interferes with Slaac IPv6 address > > resolution implemented in OVN. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2154930 > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > Hi Lorenzo, > > > lib/ovn-util.h | 13 +++++++++++++ > > northd/northd.c | 6 ++++-- > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index cd19919cb..46f57ad47 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -70,6 +70,19 @@ struct lport_addresses { > > struct ipv6_netaddr *ipv6_addrs; > > }; > > > > +static inline bool ipv6_is_all_router(const struct in6_addr *addr) { > > Nit: according to the coding style we need a newline after the return > type, after "static inline bool". > ack, I will fix it. > > + return ipv6_addr_equals(addr, &in6addr_all_routers); > > +}; > > + > > +static const struct in6_addr in6addr_all_site_routers = {{{ > > + 0xff,0x05,0x00,0x00,0x00,0x00,0x00,0x00, > > + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02 > > +}}}; > > + > > +static inline bool ipv6_is_all_site_router(const struct in6_addr *addr) { > > + return ipv6_addr_equals(addr, &in6addr_all_site_routers); > > +}; > > + > > Nit: same here. ditto. > > > bool is_dynamic_lsp_address(const char *address); > > bool extract_addresses(const char *address, struct lport_addresses *, > > int *ofs); > > diff --git a/northd/northd.c b/northd/northd.c > > index 0944a7b56..50d74f932 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -9020,9 +9020,11 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, > > igmp_group->mcgroup.name); > > } else { > > /* RFC 4291, section 2.7.1: Skip groups that correspond to all > > - * hosts. > > + * hosts, all link-local routers and all site routers. > > */ > > - if (ipv6_is_all_hosts(&igmp_group->address)) { > > + if (ipv6_is_all_hosts(&igmp_group->address) || > > + ipv6_is_all_router(&igmp_group->address) || > > + ipv6_is_all_site_router(&igmp_group->address)) { > > return; > > } > > if (atomic_compare_exchange_strong( > > The change looks good to me, thanks for fixing this! Feel free to add > my ack if you address the two minor comments I had above. > > Acked-by: Dumitru Ceara <dceara@redhat.com> > > Do you also plan to send an OVS patch to add ipv6_is_all_site_router() > and ipv6_is_all_router() to packets.h where they actually belong? ack, I will do. Regards, Lorenzo > > Thanks, > Dumitru >
diff --git a/lib/ovn-util.h b/lib/ovn-util.h index cd19919cb..46f57ad47 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -70,6 +70,19 @@ struct lport_addresses { struct ipv6_netaddr *ipv6_addrs; }; +static inline bool ipv6_is_all_router(const struct in6_addr *addr) { + return ipv6_addr_equals(addr, &in6addr_all_routers); +}; + +static const struct in6_addr in6addr_all_site_routers = {{{ + 0xff,0x05,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02 +}}}; + +static inline bool ipv6_is_all_site_router(const struct in6_addr *addr) { + return ipv6_addr_equals(addr, &in6addr_all_site_routers); +}; + bool is_dynamic_lsp_address(const char *address); bool extract_addresses(const char *address, struct lport_addresses *, int *ofs); diff --git a/northd/northd.c b/northd/northd.c index 0944a7b56..50d74f932 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -9020,9 +9020,11 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, igmp_group->mcgroup.name); } else { /* RFC 4291, section 2.7.1: Skip groups that correspond to all - * hosts. + * hosts, all link-local routers and all site routers. */ - if (ipv6_is_all_hosts(&igmp_group->address)) { + if (ipv6_is_all_hosts(&igmp_group->address) || + ipv6_is_all_router(&igmp_group->address) || + ipv6_is_all_site_router(&igmp_group->address)) { return; } if (atomic_compare_exchange_strong(
Avoid creating logical flows for Link-Local reserved multicast addresses if advertised in a MLD reports since this interferes with Slaac IPv6 address resolution implemented in OVN. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2154930 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- lib/ovn-util.h | 13 +++++++++++++ northd/northd.c | 6 ++++-- 2 files changed, 17 insertions(+), 2 deletions(-)