Skip to content

Commit

Permalink
northd: Don't generate IPv6 prefix delegation flows if not configured.
Browse files Browse the repository at this point in the history
If the feature is not enabled, there is no need to create extra
logical flows per network for each router port.  These flows match on
exact IPv6 addresses and UDP ports contributing to increased number of
datapath flows generated in OVS on the nodes.  This turns into exact
matches in most cases potentially causing datapath flow explosion for
the traffic entering OVN network from multiple sources.

Flows removed from unrelated tests as a result.

Fixes: 5c1d2d2 ("northd: Add logical flows for dhcpv6 pfd parsing")
Reported-at: https://issues.redhat.com/browse/FDP-992
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
igsilya authored and dceara committed Nov 21, 2024
1 parent 03ef56f commit 3e177f7
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
9 changes: 6 additions & 3 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -2330,6 +2330,9 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
op->lrp_networks = lrp_networks;
op->od = od;

op->prefix_delegation = smap_get_bool(&op->nbrp->options,
"prefix_delegation", false);

for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) {
sset_add(&op->od->router_ips,
op->lrp_networks.ipv4_addrs[j].addr_s);
Expand Down Expand Up @@ -7247,8 +7250,8 @@ ovn_update_ipv6_opt_for_op(struct ovn_port *op)
smap_clone(&options, &op->sb->options);

/* enable IPv6 prefix delegation */
bool prefix_delegation = smap_get_bool(&op->nbrp->options,
"prefix_delegation", false);
bool prefix_delegation = op->prefix_delegation;

if (!lrport_is_enabled(op->nbrp)) {
prefix_delegation = false;
}
Expand Down Expand Up @@ -15081,7 +15084,7 @@ build_dhcpv6_reply_flows_for_lrouter_port(
struct lflow_ref *lflow_ref)
{
ovs_assert(op->nbrp);
if (is_cr_port(op)) {
if (!op->prefix_delegation || is_cr_port(op)) {
return;
}
for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
Expand Down
1 change: 1 addition & 0 deletions northd/northd.h
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ struct ovn_port {
const struct nbrec_logical_router_port *nbrp; /* May be NULL. */

struct lport_addresses lrp_networks;
bool prefix_delegation; /* True if IPv6 prefix delegation enabled. */

/* Logical port multicast data. */
struct mcast_port_info mcast_info;
Expand Down
6 changes: 0 additions & 6 deletions tests/ovn-northd.at
Original file line number Diff line number Diff line change
Expand Up @@ -13391,9 +13391,6 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == {172.168.0.10, 172.168.0.255} && reg9[[0]] == 0), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == {20.0.0.1, 20.0.0.255} && reg9[[0]] == 0), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src_mcast ||ip4.src == 255.255.255.255 || ip4.src == 127.0.0.0/8 || ip4.dst == 127.0.0.0/8 || ip4.src == 0.0.0.0/8 || ip4.dst == 0.0.0.0/8), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff01 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff02 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff03 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=120 , match=(inport == "lr0-public" && ip4.src == 172.168.0.100), action=(next;)
table=??(lr_in_ip_input ), priority=30 , match=(ip.ttl == {0, 1}), action=(drop;)
table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp4 {eth.dst <-> eth.src; icmp4.type = 11; /* Time exceeded */ icmp4.code = 0; /* TTL exceeded in transit */ ip4.dst <-> ip4.src ; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };)
Expand Down Expand Up @@ -13570,9 +13567,6 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == {172.168.0.10, 172.168.0.255} && reg9[[0]] == 0), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src == {20.0.0.1, 20.0.0.255} && reg9[[0]] == 0), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip4.src_mcast ||ip4.src == 255.255.255.255 || ip4.src == 127.0.0.0/8 || ip4.dst == 127.0.0.0/8 || ip4.src == 0.0.0.0/8 || ip4.dst == 0.0.0.0/8), action=(drop;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff01 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff02 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=100 , match=(ip6.dst == fe80::200:ff:fe00:ff03 && udp.src == 547 && udp.dst == 546), action=(reg0 = 0; handle_dhcpv6_reply;)
table=??(lr_in_ip_input ), priority=120 , match=(inport == "lr0-public" && ip4.src == 172.168.0.100), action=(next;)
table=??(lr_in_ip_input ), priority=30 , match=(ip.ttl == {0, 1}), action=(drop;)
table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp4 {eth.dst <-> eth.src; icmp4.type = 11; /* Time exceeded */ icmp4.code = 0; /* TTL exceeded in transit */ ip4.dst <-> ip4.src ; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };)
Expand Down

0 comments on commit 3e177f7

Please sign in to comment.