Hi Alexandra, thanks a bunch for the patch. Overall, I think this is a great improvement. We should definitely be matching on the source address for IPv4 packets, and the way you have rearranged the code is much better than how it used to be.
I have one concern, which I bring up below. On Mon, Jan 12, 2026 at 11:24 AM Alexandra Rukomoinikova <[email protected]> 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 <[email protected]> > --- > northd/northd.c | 223 +++++++++++++++++++++++++++++--------------- > tests/ovn-northd.at | 35 ++++--- > tests/ovn.at | 32 ++++--- > 3 files changed, 193 insertions(+), 97 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 9c3fa8735..843c28216 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -16030,11 +16030,6 @@ build_misc_local_traffic_drop_flows_for_lrouter( > "(ip4.mcast || ip6.mcast)", debug_drop_action(), > lflow_ref); > > - /* TTL discard */ > - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30, > - "ip.ttl == {0, 1}", debug_drop_action(), > - lflow_ref); > - > /* Pass other traffic not already handled to the next table for > * routing. */ > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 0, "1", "next;", > @@ -16225,6 +16220,150 @@ 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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > + 31, ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_ICMP4_ERR, op->od->nbr->copp, > + meter_groups), > + &op->nbrp->header_, lflow_ref); > + > + } > + 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}", > + op->json_key); The priority-31 flow that we add a few lines above contains "!ip.later_frag" as part of its match. My understanding is that this is to ensure that for fragmented IP traffic, we only send the ICMP reply to the first fragment. Later fragments do not get the ICMP response. Before this patch, the later fragments would be dropped by the priority-30 flow that was installed by build_misc_local_traffic_drop_flows_for_lrouter(). With the match used here, later fragments will now get an ICMP reply from the first configured IP address on the router. This could be a different address than was used to reply to the first fragment. This seems incorrect to me. If the proper thing to do is *not* to respond to later fragments and drop them, then you should do the following: * Change this match to also include "!ip.later_frag". * Also add another priority 30 flow that matches on "ip.later_frag" and drops the packet if ttl is 0 or 1. If the proper thing to do is to respond to all fragments with the same ICMP reply, then : * Remove the "!ip.later_frag" from the priority-31 flow. * Leave the rest of your submission as-is. > + 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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > + 30, ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_ICMP4_ERR, > + op->od->nbr->copp, > + meter_groups), > + &op->nbrp->header_, lflow_ref); > +} > + > +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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > + 31, ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_ICMP6_ERR, op->od->nbr->copp, > + meter_groups), > + &op->nbrp->header_, lflow_ref); > + } > + 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}", > + 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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > + 30, ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_ICMP6_ERR, > + op->od->nbr->copp, > + meter_groups), > + &op->nbrp->header_, lflow_ref); > +} > + > static void > build_ipv6_input_flows_for_lrouter_port( > struct ovn_port *op, struct lflow_table *lflows, > @@ -16359,44 +16498,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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > - 31, ds_cstr(match), ds_cstr(actions), NULL, > - copp_meter_get(COPP_ICMP6_ERR, op->od->nbr->copp, > - meter_groups), > - &op->nbrp->header_, lflow_ref); > - } > - ds_destroy(&ip_ds); > + build_lrouter_ipv6_default_ttl_expired_flows(op, lflows, > + match, actions, > + meter_groups, lflow_ref); > } > > static void > @@ -16505,36 +16609,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_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, > - 31, ds_cstr(match), ds_cstr(actions), NULL, > - copp_meter_get(COPP_ICMP4_ERR, op->od->nbr->copp, > - meter_groups), > - &op->nbrp->header_, lflow_ref); > - > - } > - 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..2b2e1d4ad 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -8185,10 +8185,13 @@ 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=30 , match=(inport == "lrp1" && > ip4 && ip.ttl == {0, 1}), 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}), 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}), 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}), 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 +13928,15 @@ 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=30 , match=(inport == > "lr0-public" && ip4 && ip.ttl == {0, 1}), 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}), 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}), 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}), 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}), 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}), 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 +14110,15 @@ 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=30 , match=(inport == > "lr0-public" && ip4 && ip.ttl == {0, 1}), 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}), 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}), 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}), 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}), 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}), 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 58127f0d3..a06ee04b0 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -20498,7 +20498,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. > @@ -20508,10 +20508,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 > @@ -20524,28 +20524,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 > > @@ -20576,10 +20576,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 > @@ -20587,20 +20585,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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
