| Message ID | 20260115215114.78527-1-arukomoinikova@k2.cloud |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,v5] northd: Improvements of ICMP TTL exceeded behavior. | 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 |
Thanks for the rebase, Alexandra! Acked-by: Mark Michelson <mmichels@redhat.com> On Thu, Jan 15, 2026 at 4:51 PM Alexandra Rukomoinikova <arukomoinikova@k2.cloud> wrote: > > When a logical router port has multiple IP addresses from different networks, > northd generates multiple TTL exceeded flows. Previously, these flows had > identical match conditions but different actions (using different ICMP reply > source IPs), leading to non-deterministic behavior where replies could use > an incorrect source IP not belonging to the original packet's destination network. > > The fix adds source IP network matching to flow, ensuring that ICMP TTL exceeded > replies always originate from an IP in the same network as the source of the original packet. > > Additionally, the default TTL exceeded flow behavior has been unified for IPv4 > and IPv6: previously, packets that didn't match any configured subnet were > dropped; now we trigger a reply using the first IP address configured on the > router port. > > Fixes: c0321040c703 ("OVN: add ICMPv6 time exceeded support to OVN logical router") > Fixes: 7f19374c5933 ("OVN: add ICMP time exceeded support to OVN logical router") > Reported-at: https://issues.redhat.com/browse/FDP-2870 > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > --- > v4 --> v5: rebased to use new lflow addition API: changed ovn_lflow_add_with_hint__ to ovn_lflow_add > --- > northd/northd.c | 226 ++++++++++++++++++++++++++++++-------------- > tests/ovn-northd.at | 38 +++++--- > tests/ovn.at | 32 ++++--- > 3 files changed, 201 insertions(+), 95 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index afecbabc1..b13e24221 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -15866,7 +15866,7 @@ build_misc_local_traffic_drop_flows_for_lrouter( > lflow_ref); > > /* TTL discard */ > - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30, > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 29, > "ip.ttl == {0, 1}", debug_drop_action(), > lflow_ref); > > @@ -16059,6 +16059,154 @@ build_dhcp_relay_flows_for_lrouter_port(struct ovn_port *op, > free(server_ip_str); > } > > +static void > +build_lrouter_ipv4_default_ttl_expired_flows( > + struct ovn_port *op, struct lflow_table *lflows, > + struct ds *match, struct ds *actions, > + const struct shash *meter_groups, > + struct lflow_ref *lflow_ref) > +{ > + if (!op->lrp_networks.n_ipv4_addrs) { > + return; > + } > + > + struct ds ip_ds = DS_EMPTY_INITIALIZER; > + for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > + ds_clear(match); > + ds_clear(actions); > + ds_clear(&ip_ds); > + if (lrp_is_l3dgw(op)) { > + ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src"); > + } else { > + ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s", > + op->lrp_networks.ipv4_addrs[i].addr_s); > + } > + ds_put_format(match, > + "inport == %s && ip4 && " > + "ip4.src == %s/%d && " > + "ip.ttl == {0, 1} && !ip.later_frag", > + op->json_key, > + op->lrp_networks.ipv4_addrs[i].network_s, > + op->lrp_networks.ipv4_addrs[i].plen); > + ds_put_format(actions, > + "icmp4 {" > + "eth.dst <-> eth.src; " > + "icmp4.type = 11; /* Time exceeded */ " > + "icmp4.code = 0; /* TTL exceeded in transit */ " > + "%s ; ip.ttl = 254; " > + "outport = %s; flags.loopback = 1; output; };", > + ds_cstr(&ip_ds), op->json_key); > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, > + ds_cstr(match), ds_cstr(actions), lflow_ref, > + WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, > + op->od->nbr->copp, > + meter_groups)), > + WITH_HINT(&op->nbrp->header_)); > + > + } > + ds_destroy(&ip_ds); > + ds_clear(match); > + ds_clear(actions); > + > + /* Default flow for IPv4 packets with expired TTL (0 or 1). > + * Generate ICMPv4 Time Exceeded reply using the first IPv4 address > + * of the logical router port as source address. */ > + ds_put_format(match, > + "inport == %s && ip4 && " > + "ip.ttl == {0, 1} && !ip.later_frag", > + op->json_key); > + ds_put_format(actions, > + "icmp4 {" > + "eth.dst <-> eth.src; " > + "icmp4.type = 11; /* Time exceeded */ " > + "icmp4.code = 0; /* TTL exceeded in transit */ " > + "ip4.dst = ip4.src; ip4.src = %s; ip.ttl = 254; " > + "outport = %s; flags.loopback = 1; output; };", > + op->lrp_networks.ipv4_addrs[0].addr_s, > + op->json_key); > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 30, > + ds_cstr(match), ds_cstr(actions), lflow_ref, > + WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, > + op->od->nbr->copp, > + meter_groups)), > + WITH_HINT(&op->nbrp->header_)); > +} > + > +static void > +build_lrouter_ipv6_default_ttl_expired_flows( > + struct ovn_port *op, struct lflow_table *lflows, > + struct ds *match, struct ds *actions, > + const struct shash *meter_groups, > + struct lflow_ref *lflow_ref) > +{ > + /* Early return if no IPv6 addresses are configured. > + * Note: op->lrp_networks.ipv6_addrs will always have LLA and that > + * will be last in the list. */ > + if (op->lrp_networks.n_ipv6_addrs < 1) { > + return; > + } > + > + struct ds ip_ds = DS_EMPTY_INITIALIZER; > + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs - 1; i++) { > + ds_clear(match); > + ds_clear(actions); > + ds_clear(&ip_ds); > + if (lrp_is_l3dgw(op)) { > + ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src"); > + } else { > + ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s", > + op->lrp_networks.ipv6_addrs[i].addr_s); > + } > + ds_put_format(match, > + "inport == %s && ip6 && " > + "ip6.src == %s/%d && " > + "ip.ttl == {0, 1} && !ip.later_frag", > + op->json_key, > + op->lrp_networks.ipv6_addrs[i].network_s, > + op->lrp_networks.ipv6_addrs[i].plen); > + ds_put_format(actions, > + "icmp6 {" > + "eth.dst <-> eth.src; " > + "%s ; ip.ttl = 254; " > + "icmp6.type = 3; /* Time exceeded */ " > + "icmp6.code = 0; /* TTL exceeded in transit */ " > + "outport = %s; flags.loopback = 1; output; };", > + ds_cstr(&ip_ds), op->json_key); > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, > + ds_cstr(match), ds_cstr(actions), lflow_ref, > + WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, > + op->od->nbr->copp, > + meter_groups)), > + WITH_HINT(&op->nbrp->header_)); > + } > + ds_destroy(&ip_ds); > + ds_clear(match); > + ds_clear(actions); > + > + /* Default flow for IPv6 packets with expired TTL (0 or 1). > + * Generate ICMPv6 Time Exceeded reply using the first IPv6 address > + * of the logical router port as source address. */ > + ds_put_format(match, > + "inport == %s && ip6 && " > + "ip.ttl == {0, 1} && !ip.later_frag", > + op->json_key); > + ds_put_format(actions, > + "icmp6 {" > + "eth.dst <-> eth.src; " > + "ip6.dst = ip6.src; ip6.src = %s; " > + "ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ " > + "icmp6.code = 0; /* TTL exceeded in transit */ " > + "outport = %s; flags.loopback = 1; output; };", > + op->lrp_networks.ipv6_addrs[0].addr_s, > + op->json_key); > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 30, > + ds_cstr(match), ds_cstr(actions), lflow_ref, > + WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, > + op->od->nbr->copp, > + meter_groups)), > + WITH_HINT(&op->nbrp->header_)); > +} > + > static void > build_ipv6_input_flows_for_lrouter_port( > struct ovn_port *op, struct lflow_table *lflows, > @@ -16185,45 +16333,9 @@ build_ipv6_input_flows_for_lrouter_port( > } > > /* ICMPv6 time exceeded */ > - struct ds ip_ds = DS_EMPTY_INITIALIZER; > - for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { > - /* skip link-local address */ > - if (in6_is_lla(&op->lrp_networks.ipv6_addrs[i].network)) { > - continue; > - } > - > - ds_clear(match); > - ds_clear(actions); > - ds_clear(&ip_ds); > - if (lrp_is_l3dgw(op)) { > - ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src"); > - } else { > - ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s", > - op->lrp_networks.ipv6_addrs[i].addr_s); > - } > - ds_put_format(match, > - "inport == %s && ip6 && " > - "ip6.src == %s/%d && " > - "ip.ttl == {0, 1} && !ip.later_frag", > - op->json_key, > - op->lrp_networks.ipv6_addrs[i].network_s, > - op->lrp_networks.ipv6_addrs[i].plen); > - ds_put_format(actions, > - "icmp6 {" > - "eth.dst <-> eth.src; " > - "%s ; ip.ttl = 254; " > - "icmp6.type = 3; /* Time exceeded */ " > - "icmp6.code = 0; /* TTL exceeded in transit */ " > - "outport = %s; flags.loopback = 1; output; };", > - ds_cstr(&ip_ds), op->json_key); > - ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, > - ds_cstr(match), ds_cstr(actions), lflow_ref, > - WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, > - op->od->nbr->copp, > - meter_groups)), > - WITH_HINT(&op->nbrp->header_)); > - } > - ds_destroy(&ip_ds); > + build_lrouter_ipv6_default_ttl_expired_flows(op, lflows, > + match, actions, > + meter_groups, lflow_ref); > } > > static void > @@ -16331,37 +16443,9 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, > build_lrouter_bfd_flows(lflows, op, meter_groups, bfd_ports, lflow_ref); > > /* ICMP time exceeded */ > - struct ds ip_ds = DS_EMPTY_INITIALIZER; > - for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > - ds_clear(match); > - ds_clear(actions); > - ds_clear(&ip_ds); > - if (lrp_is_l3dgw(op)) { > - ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src"); > - } else { > - ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s", > - op->lrp_networks.ipv4_addrs[i].addr_s); > - } > - ds_put_format(match, > - "inport == %s && ip4 && " > - "ip.ttl == {0, 1} && !ip.later_frag", op->json_key); > - ds_put_format(actions, > - "icmp4 {" > - "eth.dst <-> eth.src; " > - "icmp4.type = 11; /* Time exceeded */ " > - "icmp4.code = 0; /* TTL exceeded in transit */ " > - "%s ; ip.ttl = 254; " > - "outport = %s; flags.loopback = 1; output; };", > - ds_cstr(&ip_ds), op->json_key); > - ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, > - ds_cstr(match), ds_cstr(actions), lflow_ref, > - WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, > - op->od->nbr->copp, > - meter_groups)), > - WITH_HINT(&op->nbrp->header_)); > - > - } > - ds_destroy(&ip_ds); > + build_lrouter_ipv4_default_ttl_expired_flows(op, lflows, > + match, actions, > + meter_groups, lflow_ref); > > /* ARP reply. These flows reply to ARP requests for the router's own > * IP address. */ > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index e3df8c9fc..53fef9c61 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -8185,10 +8185,14 @@ dnl Flows to skip TTL == {0, 1} check for IGMP and MLD packets. > AT_CHECK([grep -e 'lr_in_ip_input ' lrflows | grep -e 'igmp' -e 'mld' -e 'ip.ttl == {0, 1}' | ovn_strip_lflows], [0], [dnl > table=??(lr_in_ip_input ), priority=120 , match=((mldv1 || mldv2) && ip.ttl == 1), action=(next;) > table=??(lr_in_ip_input ), priority=120 , match=(igmp && ip.ttl == 1), 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 == "lrp1" && 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; ip4.src = 10.10.10.1 ; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp1" && 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; ip4.src = 10.10.10.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 1010::1; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp2" && 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; ip4.src = 20.20.20.1; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp2" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 2020::1; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp2"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp1" && ip4 && ip4.src == 10.10.10.0/24 && 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; ip4.src = 10.10.10.1 ; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) > table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp1" && ip6 && ip6.src == 1010::/64 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 1010::1 ; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp1"; flags.loopback = 1; output; };) > - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && 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; ip4.src = 20.20.20.1 ; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && ip4 && ip4.src == 20.20.20.0/24 && 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; ip4.src = 20.20.20.1 ; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) > table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && ip6 && ip6.src == 2020::/64 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 2020::1 ; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp2"; flags.loopback = 1; output; };) > table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) > ]) > @@ -13925,10 +13929,16 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl > 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=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; };) > - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=30 , 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; ip4.src = 172.168.0.10; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-public" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff02; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-public"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff01; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff03; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip4.src == 172.168.0.0/24 && 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; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && ip4 && ip4.src == 10.0.0.0/24 && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && ip4 && ip4.src == 20.0.0.0/24 && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) > table=??(lr_in_ip_input ), priority=50 , match=(eth.bcast), action=(drop;) > table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.0.0.1}), action=(drop;) > @@ -14102,10 +14112,16 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl > 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=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; };) > - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) > + table=??(lr_in_ip_input ), priority=30 , 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; ip4.src = 172.168.0.10; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-public" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff02; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-public"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff01; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff03; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw1"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip4.src == 172.168.0.0/24 && 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; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && ip4 && ip4.src == 10.0.0.0/24 && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) > + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && ip4 && ip4.src == 20.0.0.0/24 && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) > table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) > table=??(lr_in_ip_input ), priority=50 , match=(eth.bcast), action=(drop;) > table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.0.0.1}), action=(drop;) > diff --git a/tests/ovn.at b/tests/ovn.at > index 445a74ce5..c74a1aad1 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -20503,7 +20503,7 @@ AT_SETUP([TTL exceeded]) > AT_KEYWORDS([ttl-exceeded]) > ovn_start > > -# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IPV4_ROUTER IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM SHOULD_REPLY > +# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IP_SRC_REPLY IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM SHOULD_REPLY > # > # Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv4 packet with > # ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM as specified and TTL set to 1. > @@ -20513,10 +20513,10 @@ ovn_start > # INPORT is a lport number, e.g. 11 for vif11. > # HV is a hypervisor number > # ETH_SRC and ETH_DST are each 12 hex digits. > -# IPV4_SRC, IPV4_DST and IPV4_ROUTER are each 8 hex digits. > +# IPV4_SRC, IPV4_DST and IP_SRC_REPLY are each 8 hex digits. > # IP_CHKSUM, EXP_IP_CHSUM and EXP_ICMP_CHKSUM are each 4 hex digits > test_ip_packet() { > - local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_router=$7 ip_chksum=$8 > + local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_src_reply=$7 ip_chksum=$8 > local exp_ip_chksum=$9 exp_icmp_chksum=${10} > shift 10 > local should_reply=$1 > @@ -20529,28 +20529,28 @@ test_ip_packet() { > local icmp_data=00000000 > local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} > if test $should_reply == yes; then > - local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} > + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_src_reply}${ipv4_src}${reply_icmp_payload} > echo $reply$orig_pkt_in_reply >> vif$inport.expected > fi > > as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet > } > > -# test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_ROUTER EXP_ICMP_CHKSUM SHOULD_REPLY > +# test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_SRC_REPLY EXP_ICMP_CHKSUM SHOULD_REPLY > # > # Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv6 > # packet with ETH_SRC, ETH_DST, IPV6_SRC and IPV6_DST as specified. > # IPV6_ROUTER and EXP_ICMP_CHKSUM are the source IP and checksum of the icmpv6 ttl exceeded > # packet sent by OVN logical router > test_ip6_packet() { > - local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_dst=$6 ipv6_router=$7 exp_icmp_chksum=$8 should_reply=$9 > + local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_dst=$6 ipv6_src_reply=$7 exp_icmp_chksum=$8 should_reply=$9 > shift 8 > > local ip6_hdr=6000000000151101${ipv6_src}${ipv6_dst} > local packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > > if test $should_reply == yes; then > - local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > + local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_src_reply}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a > echo $reply >> vif$inport.expected > fi > > @@ -20581,10 +20581,8 @@ done > > check ovn-nbctl lr-add lr0 > for i in 1 2; do > - check ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 192.168.$i.254/24 2001:db8:$i::1/64 > - check ovn-nbctl -- lsp-add sw$i lrp$i-attachment \ > - -- set Logical_Switch_Port lrp$i-attachment type=router \ > - options:router-port=lrp$i addresses='"00:00:00:00:ff:0'$i' 192.168.'$i'.254 2001:db8:'$i'::1"' > + check ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 172.31.$i.254/24 192.168.$i.254/24 2001:db8:$i::1/64 2002:db8:$i::1/64 > + check ovn-nbctl lsp-add-router-port sw$i lrp$i-attachment lrp$i > done > > OVN_POPULATE_ARP > @@ -20592,20 +20590,28 @@ OVN_POPULATE_ARP > wait_for_ports_up > check ovn-nbctl --wait=hv sync > > +# Test packet with source IP from router's networks > test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 f87c ea96 yes > test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c22 yes > > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 172 31 1 5) $(ip_to_hex 192 168 2 1) $(ip_to_hex 172 31 1 254) 0000 218b ff1b yes > +test_ip6_packet 1 1 000000000001 00000000ff01 20020db8000100000000000000000011 20010db8000200000000000000000011 20020db8000100000000000000000001 1c1f yes > + > +# Test packet with source IP from external network - expect reply from first router's IP > +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 1 1 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 172 31 1 254) 0000 ccad aa3e yes > +test_ip6_packet 1 1 000000000001 00000000ff01 20030db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c1e yes > + > # Should not send ICMP for multicast > test_ip_packet 1 1 000000000001 01005e7f0001 $(ip_to_hex 192 168 1 1) $(ip_to_hex 239 255 0 1) $(ip_to_hex 192 168 1 254) 0000 000000000 no > test_ip6_packet 1 1 000000000001 333300000001 20010db8000100000000000000000011 ff020000000000000000000000000001 20010db8000100000000000000000001 0000 no > > OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) > > -# Confirm from debug log that we only see 2 packet-ins (no packet-ins for > +# Confirm from debug log that we only see 6 packet-ins (no packet-ins for > # multicasts). This is necessary because not seeing ICMP messages doesn't > # necessarily mean the packet-in didn't happen. It is possible that packet-in > # is processed but the ICMP message got dropped. > -AT_CHECK([grep -c packet-in hv1/ovn-controller.log], [0], [2 > +AT_CHECK([grep -c packet-in hv1/ovn-controller.log], [0], [6 > ]) > > OVN_CLEANUP([hv1], [hv2]) > -- > 2.48.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 1/16/26 7:44 PM, Mark Michelson via dev wrote: > Thanks for the rebase, Alexandra! > > Acked-by: Mark Michelson <mmichels@redhat.com> > > On Thu, Jan 15, 2026 at 4:51 PM Alexandra Rukomoinikova > <arukomoinikova@k2.cloud> wrote: >> >> When a logical router port has multiple IP addresses from different networks, >> northd generates multiple TTL exceeded flows. Previously, these flows had >> identical match conditions but different actions (using different ICMP reply >> source IPs), leading to non-deterministic behavior where replies could use >> an incorrect source IP not belonging to the original packet's destination network. >> >> The fix adds source IP network matching to flow, ensuring that ICMP TTL exceeded >> replies always originate from an IP in the same network as the source of the original packet. >> >> Additionally, the default TTL exceeded flow behavior has been unified for IPv4 >> and IPv6: previously, packets that didn't match any configured subnet were >> dropped; now we trigger a reply using the first IP address configured on the >> router port. >> >> Fixes: c0321040c703 ("OVN: add ICMPv6 time exceeded support to OVN logical router") >> Fixes: 7f19374c5933 ("OVN: add ICMP time exceeded support to OVN logical router") >> Reported-at: https://issues.redhat.com/browse/FDP-2870 >> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >> --- >> v4 --> v5: rebased to use new lflow addition API: changed ovn_lflow_add_with_hint__ to ovn_lflow_add >> --- Hi Alexandra, Mark, Thanks for the fix and review! Applied to main and backported to all stable branches down to 24.03. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index afecbabc1..b13e24221 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -15866,7 +15866,7 @@ build_misc_local_traffic_drop_flows_for_lrouter( lflow_ref); /* TTL discard */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30, + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 29, "ip.ttl == {0, 1}", debug_drop_action(), lflow_ref); @@ -16059,6 +16059,154 @@ build_dhcp_relay_flows_for_lrouter_port(struct ovn_port *op, free(server_ip_str); } +static void +build_lrouter_ipv4_default_ttl_expired_flows( + struct ovn_port *op, struct lflow_table *lflows, + struct ds *match, struct ds *actions, + const struct shash *meter_groups, + struct lflow_ref *lflow_ref) +{ + if (!op->lrp_networks.n_ipv4_addrs) { + return; + } + + struct ds ip_ds = DS_EMPTY_INITIALIZER; + for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + ds_clear(match); + ds_clear(actions); + ds_clear(&ip_ds); + if (lrp_is_l3dgw(op)) { + ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src"); + } else { + ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s", + op->lrp_networks.ipv4_addrs[i].addr_s); + } + ds_put_format(match, + "inport == %s && ip4 && " + "ip4.src == %s/%d && " + "ip.ttl == {0, 1} && !ip.later_frag", + op->json_key, + op->lrp_networks.ipv4_addrs[i].network_s, + op->lrp_networks.ipv4_addrs[i].plen); + ds_put_format(actions, + "icmp4 {" + "eth.dst <-> eth.src; " + "icmp4.type = 11; /* Time exceeded */ " + "icmp4.code = 0; /* TTL exceeded in transit */ " + "%s ; ip.ttl = 254; " + "outport = %s; flags.loopback = 1; output; };", + ds_cstr(&ip_ds), op->json_key); + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, + ds_cstr(match), ds_cstr(actions), lflow_ref, + WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, + op->od->nbr->copp, + meter_groups)), + WITH_HINT(&op->nbrp->header_)); + + } + ds_destroy(&ip_ds); + ds_clear(match); + ds_clear(actions); + + /* Default flow for IPv4 packets with expired TTL (0 or 1). + * Generate ICMPv4 Time Exceeded reply using the first IPv4 address + * of the logical router port as source address. */ + ds_put_format(match, + "inport == %s && ip4 && " + "ip.ttl == {0, 1} && !ip.later_frag", + op->json_key); + ds_put_format(actions, + "icmp4 {" + "eth.dst <-> eth.src; " + "icmp4.type = 11; /* Time exceeded */ " + "icmp4.code = 0; /* TTL exceeded in transit */ " + "ip4.dst = ip4.src; ip4.src = %s; ip.ttl = 254; " + "outport = %s; flags.loopback = 1; output; };", + op->lrp_networks.ipv4_addrs[0].addr_s, + op->json_key); + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 30, + ds_cstr(match), ds_cstr(actions), lflow_ref, + WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, + op->od->nbr->copp, + meter_groups)), + WITH_HINT(&op->nbrp->header_)); +} + +static void +build_lrouter_ipv6_default_ttl_expired_flows( + struct ovn_port *op, struct lflow_table *lflows, + struct ds *match, struct ds *actions, + const struct shash *meter_groups, + struct lflow_ref *lflow_ref) +{ + /* Early return if no IPv6 addresses are configured. + * Note: op->lrp_networks.ipv6_addrs will always have LLA and that + * will be last in the list. */ + if (op->lrp_networks.n_ipv6_addrs < 1) { + return; + } + + struct ds ip_ds = DS_EMPTY_INITIALIZER; + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs - 1; i++) { + ds_clear(match); + ds_clear(actions); + ds_clear(&ip_ds); + if (lrp_is_l3dgw(op)) { + ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src"); + } else { + ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s", + op->lrp_networks.ipv6_addrs[i].addr_s); + } + ds_put_format(match, + "inport == %s && ip6 && " + "ip6.src == %s/%d && " + "ip.ttl == {0, 1} && !ip.later_frag", + op->json_key, + op->lrp_networks.ipv6_addrs[i].network_s, + op->lrp_networks.ipv6_addrs[i].plen); + ds_put_format(actions, + "icmp6 {" + "eth.dst <-> eth.src; " + "%s ; ip.ttl = 254; " + "icmp6.type = 3; /* Time exceeded */ " + "icmp6.code = 0; /* TTL exceeded in transit */ " + "outport = %s; flags.loopback = 1; output; };", + ds_cstr(&ip_ds), op->json_key); + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, + ds_cstr(match), ds_cstr(actions), lflow_ref, + WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, + op->od->nbr->copp, + meter_groups)), + WITH_HINT(&op->nbrp->header_)); + } + ds_destroy(&ip_ds); + ds_clear(match); + ds_clear(actions); + + /* Default flow for IPv6 packets with expired TTL (0 or 1). + * Generate ICMPv6 Time Exceeded reply using the first IPv6 address + * of the logical router port as source address. */ + ds_put_format(match, + "inport == %s && ip6 && " + "ip.ttl == {0, 1} && !ip.later_frag", + op->json_key); + ds_put_format(actions, + "icmp6 {" + "eth.dst <-> eth.src; " + "ip6.dst = ip6.src; ip6.src = %s; " + "ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ " + "icmp6.code = 0; /* TTL exceeded in transit */ " + "outport = %s; flags.loopback = 1; output; };", + op->lrp_networks.ipv6_addrs[0].addr_s, + op->json_key); + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 30, + ds_cstr(match), ds_cstr(actions), lflow_ref, + WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, + op->od->nbr->copp, + meter_groups)), + WITH_HINT(&op->nbrp->header_)); +} + static void build_ipv6_input_flows_for_lrouter_port( struct ovn_port *op, struct lflow_table *lflows, @@ -16185,45 +16333,9 @@ build_ipv6_input_flows_for_lrouter_port( } /* ICMPv6 time exceeded */ - struct ds ip_ds = DS_EMPTY_INITIALIZER; - for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - /* skip link-local address */ - if (in6_is_lla(&op->lrp_networks.ipv6_addrs[i].network)) { - continue; - } - - ds_clear(match); - ds_clear(actions); - ds_clear(&ip_ds); - if (lrp_is_l3dgw(op)) { - ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src"); - } else { - ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s", - op->lrp_networks.ipv6_addrs[i].addr_s); - } - ds_put_format(match, - "inport == %s && ip6 && " - "ip6.src == %s/%d && " - "ip.ttl == {0, 1} && !ip.later_frag", - op->json_key, - op->lrp_networks.ipv6_addrs[i].network_s, - op->lrp_networks.ipv6_addrs[i].plen); - ds_put_format(actions, - "icmp6 {" - "eth.dst <-> eth.src; " - "%s ; ip.ttl = 254; " - "icmp6.type = 3; /* Time exceeded */ " - "icmp6.code = 0; /* TTL exceeded in transit */ " - "outport = %s; flags.loopback = 1; output; };", - ds_cstr(&ip_ds), op->json_key); - ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, - ds_cstr(match), ds_cstr(actions), lflow_ref, - WITH_CTRL_METER(copp_meter_get(COPP_ICMP6_ERR, - op->od->nbr->copp, - meter_groups)), - WITH_HINT(&op->nbrp->header_)); - } - ds_destroy(&ip_ds); + build_lrouter_ipv6_default_ttl_expired_flows(op, lflows, + match, actions, + meter_groups, lflow_ref); } static void @@ -16331,37 +16443,9 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, build_lrouter_bfd_flows(lflows, op, meter_groups, bfd_ports, lflow_ref); /* ICMP time exceeded */ - struct ds ip_ds = DS_EMPTY_INITIALIZER; - for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - ds_clear(match); - ds_clear(actions); - ds_clear(&ip_ds); - if (lrp_is_l3dgw(op)) { - ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src"); - } else { - ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s", - op->lrp_networks.ipv4_addrs[i].addr_s); - } - ds_put_format(match, - "inport == %s && ip4 && " - "ip.ttl == {0, 1} && !ip.later_frag", op->json_key); - ds_put_format(actions, - "icmp4 {" - "eth.dst <-> eth.src; " - "icmp4.type = 11; /* Time exceeded */ " - "icmp4.code = 0; /* TTL exceeded in transit */ " - "%s ; ip.ttl = 254; " - "outport = %s; flags.loopback = 1; output; };", - ds_cstr(&ip_ds), op->json_key); - ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 31, - ds_cstr(match), ds_cstr(actions), lflow_ref, - WITH_CTRL_METER(copp_meter_get(COPP_ICMP4_ERR, - op->od->nbr->copp, - meter_groups)), - WITH_HINT(&op->nbrp->header_)); - - } - ds_destroy(&ip_ds); + build_lrouter_ipv4_default_ttl_expired_flows(op, lflows, + match, actions, + meter_groups, lflow_ref); /* ARP reply. These flows reply to ARP requests for the router's own * IP address. */ diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index e3df8c9fc..53fef9c61 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -8185,10 +8185,14 @@ dnl Flows to skip TTL == {0, 1} check for IGMP and MLD packets. AT_CHECK([grep -e 'lr_in_ip_input ' lrflows | grep -e 'igmp' -e 'mld' -e 'ip.ttl == {0, 1}' | ovn_strip_lflows], [0], [dnl table=??(lr_in_ip_input ), priority=120 , match=((mldv1 || mldv2) && ip.ttl == 1), action=(next;) table=??(lr_in_ip_input ), priority=120 , match=(igmp && ip.ttl == 1), 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 == "lrp1" && 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; ip4.src = 10.10.10.1 ; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp1" && 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; ip4.src = 10.10.10.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 1010::1; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp2" && 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; ip4.src = 20.20.20.1; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lrp2" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 2020::1; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp2"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp1" && ip4 && ip4.src == 10.10.10.0/24 && 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; ip4.src = 10.10.10.1 ; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; };) table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp1" && ip6 && ip6.src == 1010::/64 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 1010::1 ; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp1"; flags.loopback = 1; output; };) - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && 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; ip4.src = 20.20.20.1 ; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && ip4 && ip4.src == 20.20.20.0/24 && 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; ip4.src = 20.20.20.1 ; ip.ttl = 254; outport = "lrp2"; flags.loopback = 1; output; };) table=??(lr_in_ip_input ), priority=31 , match=(inport == "lrp2" && ip6 && ip6.src == 2020::/64 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = 2020::1 ; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lrp2"; flags.loopback = 1; output; };) table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) ]) @@ -13925,10 +13929,16 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl 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=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; };) - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) + table=??(lr_in_ip_input ), priority=30 , 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; ip4.src = 172.168.0.10; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-public" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff02; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-public"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff01; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff03; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip4.src == 172.168.0.0/24 && 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; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && ip4 && ip4.src == 10.0.0.0/24 && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && ip4 && ip4.src == 20.0.0.0/24 && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) table=??(lr_in_ip_input ), priority=50 , match=(eth.bcast), action=(drop;) table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.0.0.1}), action=(drop;) @@ -14102,10 +14112,16 @@ AT_CHECK([grep "lr_in_ip_input" lr0flows | ovn_strip_lflows], [0], [dnl 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=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; };) - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) - table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=29 , match=(ip.ttl == {0, 1}), action=(drop;) + table=??(lr_in_ip_input ), priority=30 , 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; ip4.src = 172.168.0.10; ip.ttl = 254; outport = "lr0-public"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-public" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff02; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-public"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && 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; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw0" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff01; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && 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; ip4.src = 20.0.0.1; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=30 , match=(inport == "lr0-sw1" && ip6 && ip.ttl == {0, 1} && !ip.later_frag), action=(icmp6 {eth.dst <-> eth.src; ip6.dst = ip6.src; ip6.src = fe80::200:ff:fe00:ff03; ip.ttl = 254; icmp6.type = 3; /* Time exceeded */ icmp6.code = 0; /* TTL exceeded in transit */ outport = "lr0-sw1"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-public" && ip4 && ip4.src == 172.168.0.0/24 && 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; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw0" && ip4 && ip4.src == 10.0.0.0/24 && 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; ip4.src = 10.0.0.1 ; ip.ttl = 254; outport = "lr0-sw0"; flags.loopback = 1; output; };) + table=??(lr_in_ip_input ), priority=31 , match=(inport == "lr0-sw1" && ip4 && ip4.src == 20.0.0.0/24 && 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; ip4.src = 20.0.0.1 ; ip.ttl = 254; outport = "lr0-sw1"; flags.loopback = 1; output; };) table=??(lr_in_ip_input ), priority=32 , match=(ip.ttl == {0, 1} && !ip.later_frag && (ip4.mcast || ip6.mcast)), action=(drop;) table=??(lr_in_ip_input ), priority=50 , match=(eth.bcast), action=(drop;) table=??(lr_in_ip_input ), priority=60 , match=(ip4.dst == {10.0.0.1}), action=(drop;) diff --git a/tests/ovn.at b/tests/ovn.at index 445a74ce5..c74a1aad1 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -20503,7 +20503,7 @@ AT_SETUP([TTL exceeded]) AT_KEYWORDS([ttl-exceeded]) ovn_start -# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IPV4_ROUTER IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM SHOULD_REPLY +# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IP_SRC_REPLY IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM SHOULD_REPLY # # Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv4 packet with # ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM as specified and TTL set to 1. @@ -20513,10 +20513,10 @@ ovn_start # INPORT is a lport number, e.g. 11 for vif11. # HV is a hypervisor number # ETH_SRC and ETH_DST are each 12 hex digits. -# IPV4_SRC, IPV4_DST and IPV4_ROUTER are each 8 hex digits. +# IPV4_SRC, IPV4_DST and IP_SRC_REPLY are each 8 hex digits. # IP_CHKSUM, EXP_IP_CHSUM and EXP_ICMP_CHKSUM are each 4 hex digits test_ip_packet() { - local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_router=$7 ip_chksum=$8 + local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_src_reply=$7 ip_chksum=$8 local exp_ip_chksum=$9 exp_icmp_chksum=${10} shift 10 local should_reply=$1 @@ -20529,28 +20529,28 @@ test_ip_packet() { local icmp_data=00000000 local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data} if test $should_reply == yes; then - local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_router}${ipv4_src}${reply_icmp_payload} + local reply=${eth_src}${eth_dst}08004500003000004000${reply_icmp_ttl}01${exp_ip_chksum}${ip_src_reply}${ipv4_src}${reply_icmp_payload} echo $reply$orig_pkt_in_reply >> vif$inport.expected fi as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet } -# test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_ROUTER EXP_ICMP_CHKSUM SHOULD_REPLY +# test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_SRC_REPLY EXP_ICMP_CHKSUM SHOULD_REPLY # # Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv6 # packet with ETH_SRC, ETH_DST, IPV6_SRC and IPV6_DST as specified. # IPV6_ROUTER and EXP_ICMP_CHKSUM are the source IP and checksum of the icmpv6 ttl exceeded # packet sent by OVN logical router test_ip6_packet() { - local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_dst=$6 ipv6_router=$7 exp_icmp_chksum=$8 should_reply=$9 + local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_dst=$6 ipv6_src_reply=$7 exp_icmp_chksum=$8 should_reply=$9 shift 8 local ip6_hdr=6000000000151101${ipv6_src}${ipv6_dst} local packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a if test $should_reply == yes; then - local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a + local reply=${eth_src}${eth_dst}86dd6000000000453afe${ipv6_src_reply}${ipv6_src}0300${exp_icmp_chksum}00000000${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a echo $reply >> vif$inport.expected fi @@ -20581,10 +20581,8 @@ done check ovn-nbctl lr-add lr0 for i in 1 2; do - check ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 192.168.$i.254/24 2001:db8:$i::1/64 - check ovn-nbctl -- lsp-add sw$i lrp$i-attachment \ - -- set Logical_Switch_Port lrp$i-attachment type=router \ - options:router-port=lrp$i addresses='"00:00:00:00:ff:0'$i' 192.168.'$i'.254 2001:db8:'$i'::1"' + check ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 172.31.$i.254/24 192.168.$i.254/24 2001:db8:$i::1/64 2002:db8:$i::1/64 + check ovn-nbctl lsp-add-router-port sw$i lrp$i-attachment lrp$i done OVN_POPULATE_ARP @@ -20592,20 +20590,28 @@ OVN_POPULATE_ARP wait_for_ports_up check ovn-nbctl --wait=hv sync +# Test packet with source IP from router's networks test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 0000 f87c ea96 yes test_ip6_packet 1 1 000000000001 00000000ff01 20010db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c22 yes +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 172 31 1 5) $(ip_to_hex 192 168 2 1) $(ip_to_hex 172 31 1 254) 0000 218b ff1b yes +test_ip6_packet 1 1 000000000001 00000000ff01 20020db8000100000000000000000011 20010db8000200000000000000000011 20020db8000100000000000000000001 1c1f yes + +# Test packet with source IP from external network - expect reply from first router's IP +test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 1 1 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 172 31 1 254) 0000 ccad aa3e yes +test_ip6_packet 1 1 000000000001 00000000ff01 20030db8000100000000000000000011 20010db8000200000000000000000011 20010db8000100000000000000000001 1c1e yes + # Should not send ICMP for multicast test_ip_packet 1 1 000000000001 01005e7f0001 $(ip_to_hex 192 168 1 1) $(ip_to_hex 239 255 0 1) $(ip_to_hex 192 168 1 254) 0000 000000000 no test_ip6_packet 1 1 000000000001 333300000001 20010db8000100000000000000000011 ff020000000000000000000000000001 20010db8000100000000000000000001 0000 no OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) -# Confirm from debug log that we only see 2 packet-ins (no packet-ins for +# Confirm from debug log that we only see 6 packet-ins (no packet-ins for # multicasts). This is necessary because not seeing ICMP messages doesn't # necessarily mean the packet-in didn't happen. It is possible that packet-in # is processed but the ICMP message got dropped. -AT_CHECK([grep -c packet-in hv1/ovn-controller.log], [0], [2 +AT_CHECK([grep -c packet-in hv1/ovn-controller.log], [0], [6 ]) OVN_CLEANUP([hv1], [hv2])
When a logical router port has multiple IP addresses from different networks, northd generates multiple TTL exceeded flows. Previously, these flows had identical match conditions but different actions (using different ICMP reply source IPs), leading to non-deterministic behavior where replies could use an incorrect source IP not belonging to the original packet's destination network. The fix adds source IP network matching to flow, ensuring that ICMP TTL exceeded replies always originate from an IP in the same network as the source of the original packet. Additionally, the default TTL exceeded flow behavior has been unified for IPv4 and IPv6: previously, packets that didn't match any configured subnet were dropped; now we trigger a reply using the first IP address configured on the router port. Fixes: c0321040c703 ("OVN: add ICMPv6 time exceeded support to OVN logical router") Fixes: 7f19374c5933 ("OVN: add ICMP time exceeded support to OVN logical router") Reported-at: https://issues.redhat.com/browse/FDP-2870 Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> --- v4 --> v5: rebased to use new lflow addition API: changed ovn_lflow_add_with_hint__ to ovn_lflow_add --- northd/northd.c | 226 ++++++++++++++++++++++++++++++-------------- tests/ovn-northd.at | 38 +++++--- tests/ovn.at | 32 ++++--- 3 files changed, 201 insertions(+), 95 deletions(-)