Add per logical router port option for ICMPv4 redirect support, The option is disabled by default.
RFC 1812 describes the operation of this functionality for the following scenario: When a router forwards a packet back out the port it arrived on, and the next hop is on the sender's own network, the sender could have reached that next hop directly. RFC 1812 (5.2.7.2) requires the router to point this out with an ICMPv4 Redirect (RFC 792, type 5, code 1), naming the better first hop. OVN routers do not send Redirects, so such traffic keeps taking the extra hop through the router. In this commit we assume that ip destination of packet is not in one of directly connected router's networks, so the Redirect always points to another router and never to the destination host. Of the conditions RFC 1812 5.2.7.2 and RFC 1122 3.2.2 put on sending a Redirect, this implements: - the packet leaves through the port it arrived on; - the sender and the next hop are on the same network of that port; - the next hop is not the sender itself; - the packet is not a later fragment; - the packet is not itself an ICMP Redirect. Not implemented: packets with a source route option. Routing overwrites eth.src with the egress router port mac, so the original eth.src has to be saved in REG_ORIG_ETH_SRC beforehand. Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- Documentation/ref/ovn-logical-flows.7.rst | 92 +++++++++++---- NEWS | 5 + lib/ovn-util.c | 4 +- northd/northd.c | 129 +++++++++++++++++++--- northd/northd.h | 19 ++-- ovn-nb.xml | 17 +++ tests/ovn-northd.at | 68 ++++++++++++ tests/ovn.at | 36 ++++++ tests/system-ovn.at | 106 ++++++++++++++++++ 9 files changed, 432 insertions(+), 44 deletions(-) diff --git a/Documentation/ref/ovn-logical-flows.7.rst b/Documentation/ref/ovn-logical-flows.7.rst index 44fd560bf..f55567852 100644 --- a/Documentation/ref/ovn-logical-flows.7.rst +++ b/Documentation/ref/ovn-logical-flows.7.rst @@ -3097,6 +3097,12 @@ with priority 100 and action setting unique-generated per-datapath 32-bit value If packet didn't match any configured inport (*<main>* route table), register 7 value is set to 0. +If Logical Router Port *P* has ``options:send_icmp4_redirects`` set to +``true``, the original Ethernet source address of the packet is also saved +in ``xreg1[0..47]``, because routing overwrites ``eth.src`` with the +address of the egress router port. The saved address is used by +:ref:`ICMP Redirect <lr-in-20>` as the destination of the ICMPv4 Redirect. + This table contains the following logical flows: - Priority-100 flow with match ``inport == "LRP_NAME"`` value and action, which @@ -3115,7 +3121,7 @@ setting ``reg0`` (or ``xxreg0`` for IPv6) to the next-hop IP address (leaving ``ip4.dst`` or ``ip6.dst``, the packet's final destination, unchanged) and advances to the next table for ARP resolution. It also sets ``reg1`` (or ``xxreg1``) to the IP address owned by the selected router port (ingress table -:ref:`ARP Request <lr-in-27>` will generate an ARP request, if needed, with +:ref:`ARP Request <lr-in-28>` will generate an ARP request, if needed, with ``reg0`` as the target protocol address and ``reg1`` as the source protocol address). @@ -3251,7 +3257,7 @@ setting ``reg0`` (or ``xxreg0`` for IPv6) to the next-hop IP address (leaving ``ip4.dst`` or ``ip6.dst``, the packet's final destination, unchanged) and advances to the next table for ARP resolution. It also sets ``reg1`` (or ``xxreg1``) to the IP address owned by the selected router port (ingress table -:ref:`ARP Request <lr-in-27>` will generate an ARP request, if needed, with +:ref:`ARP Request <lr-in-28>` will generate an ARP request, if needed, with ``reg0`` as the target protocol address and ``reg1`` as the source protocol address). @@ -3348,7 +3354,55 @@ nexthops. .. _lr-in-20: -Ingress Table 20: DHCP Relay Response Check +Ingress Table 20: ICMP Redirect ++~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This table sends ICMPv4 Redirect messages (RFC 792, type 5, code 1) as +described in RFC 1812 section 5.2.7.2. When a packet is forwarded back out +of the logical router port it arrived on and the next hop is on the same +network as the sender, the sender is told to use that next hop directly. The +original packet is still forwarded. Flows are added only for logical router +ports with ``options:send_icmp4_redirects`` set to ``true``. + +- For each logical router port *P* with + ``options:send_icmp4_redirects=true``, a priority-110 flow with match + ``inport == P && icmp4.type == {3, 5, 11,}`` and action ``next;``, + so that no Redirect is sent in reply to icmp4 errors (RFC 1122 3.2.2). + +- For each IPv4 network *N* of such a port *P*, a priority-110 flow with match + ``ip4 && ip4.dst == N`` and action ``next;``. No Redirect is sent when the + destination itself is on the network of the port, so a Redirect always + points to another router and never to the destination host. + +- For each IPv4 network *N* of such a port *P*, whose Ethernet address is *E* + and IPv4 address on *N* is *A*, a priority-100 flow with match ``inport == P + && outport == P && ip4 && ip4.src == N && reg0 == N && !ip.later_frag`` and + the following actions:: + + icmp4_redirect { + eth.dst = xreg1[0..47]; + eth.src = E; + ip4.dst = ip4.src; + ip4.src = A; + ip.ttl = 254; + outport = P; + flags.loopback = 1; + output; + }; + next; + + ``reg0`` holds the next hop selected by routing, and ``xreg1[0..47]`` holds + the original Ethernet source address saved in :ref:`IP Routing Pre + <lr-in-15>`. The next hop is sent in the gateway field of the Redirect. + ``ovn-controller`` does not send the Redirect if the next hop is the source + of the packet itself. These packets are rate-limited by the ``icmp4-error`` + control plane protection meter. + +- A priority-0 flow that matches all packets to advance to the next table. + +.. _lr-in-21: + +Ingress Table 21: DHCP Relay Response Check ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This stage process the DHCP response packets coming from the DHCP server. @@ -3366,9 +3420,9 @@ This stage process the DHCP response packets coming from the DHCP server. - A priority-0 flow that matches all packets to advance to the next table. -.. _lr-in-21: +.. _lr-in-22: -Ingress Table 21: DHCP Relay Response +Ingress Table 22: DHCP Relay Response ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This stage process the DHCP response packets on which ``dhcp_relay_resp_chk`` @@ -3393,9 +3447,9 @@ action is applied in the previous stage. - A priority-0 flow that matches all packets to advance to the next table. -.. _lr-in-22: +.. _lr-in-23: -Ingress Table 22: ARP/ND Resolution +Ingress Table 23: ARP/ND Resolution ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Any packet that reaches this table is an IP packet whose next-hop IPv4 address @@ -3509,9 +3563,9 @@ contains the final destination.) This table resolves the IP address in ``reg0`` !is_chassis_resident("cr-ROUTER_PORT")`` has actions ``eth.dst = E; next;``, where *E* is the ethernet address of the logical router port. -.. _lr-in-23: +.. _lr-in-24: -Ingress Table 23: Check packet length +Ingress Table 24: Check packet length ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For distributed logical routers or gateway routers with gateway port configured @@ -3534,9 +3588,9 @@ flow is added, with priority-55, to bypass the ``check_pkt_larger`` flow. This table adds one priority-0 fallback flow that matches all packets and advances to the next table. -.. _lr-in-24: +.. _lr-in-25: -Ingress Table 24: Handle larger packets +Ingress Table 25: Handle larger packets ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For distributed logical routers or gateway routers with gateway port configured @@ -3583,9 +3637,9 @@ respectively:: This table adds one priority-0 fallback flow that matches all packets and advances to the next table. -.. _lr-in-25: +.. _lr-in-26: -Ingress Table 25: Gateway Redirect +Ingress Table 26: Gateway Redirect ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For distributed logical routers where one or more of the logical router ports @@ -3633,9 +3687,9 @@ following flows: - A priority-0 logical flow with match ``1`` has actions ``next;``. -.. _lr-in-26: +.. _lr-in-27: -Ingress Table 26: Network ID +Ingress Table 27: Network ID ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This table contains flows that set ``flags.network_id`` for IP packets: @@ -3661,9 +3715,9 @@ This table contains flows that set ``flags.network_id`` for IP packets: - Catch-all: A priority-0 flow with match ``1`` has actions ``next;``. -.. _lr-in-27: +.. _lr-in-28: -Ingress Table 27: ARP Request +Ingress Table 28: ARP Request ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In the common case where the Ethernet destination has been resolved, this table @@ -3696,9 +3750,9 @@ or IPv6 Neighbor Solicitation request. It holds the following flows: - Known MAC address. A priority-0 flow with match ``1`` has actions ``next;``. -.. _lr-in-28: +.. _lr-in-29: -Ingress Table 28: ECMP symmetric reply processing for egress +Ingress Table 29: ECMP symmetric reply processing for egress ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This table contains logical flows that commit IP traffic forwarded by ECMP diff --git a/NEWS b/NEWS index f1c56dd14..f75309b9d 100644 --- a/NEWS +++ b/NEWS @@ -129,6 +129,11 @@ OVN v26.09.0 - xxx xx xxxx represent a logical router port (e.g. ovn-ic transit switch LSPs). Such ports are treated like type=router, including omission from _MC_flood_l2. + - Logical_Router_Port: Added a new "options:send_icmp4_redirects" key. + If set to true, the router sends an ICMPv4 Redirect (RFC 1812, 5.2.7.2) + to the source of a packet that is forwarded back out of the port it + arrived on, when the source and the next hop are on the same network + of that port. Disabled by default. OVN v26.03.0 - xxx xx xxxx -------------------------- diff --git a/lib/ovn-util.c b/lib/ovn-util.c index f8094afc3..3d3bebf9f 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -1007,8 +1007,8 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address, * * NOTE: If OVN_NORTHD_PIPELINE_CSUM is updated make sure to double check * whether an update of OVN_INTERNAL_MINOR_VER is required. */ -#define OVN_NORTHD_PIPELINE_CSUM "289408664 11318" -#define OVN_INTERNAL_MINOR_VER 16 +#define OVN_NORTHD_PIPELINE_CSUM "4027926261 11398" +#define OVN_INTERNAL_MINOR_VER 17 /* Returns the OVN version. The caller must free the returned value. */ char * diff --git a/northd/northd.c b/northd/northd.c index 4eb2ea44b..3fdcf746d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -234,6 +234,13 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2)); #define REG_POLICY_CHAIN_ID "reg9[16..31]" #define REG_ROUTE_TABLE_ID "reg7" +/* Register that holds the Ethernet source address of the packet as + * received. Must be saved, since routing will overwrite eth.src + * with the egress router port's address. Read back when building + * the ICMP redirect packet. + * Only set for ports with options:send_icmp4_redirects=true. */ +#define REG_ORIG_ETH_SRC "xreg1[0..47]" + /* Registers used for pasing observability information for switches: * domain and point ID. */ #define REG_OBS_POINT_ID_NEW "reg3" @@ -330,18 +337,18 @@ static const char *reg_ct_state[] = { * | R0 | REGBIT_ND_RA_OPTS_RESULT | | | | | * | | (= IN_ND_RA_OPTIONS) | X | | | | * | | NEXT_HOP_IPV4 | R | | | | - * | | (>= IN_IP_ROUTING) | E | INPORT_ETH_ADDR | X | | - * +-----+---------------------------+ G | (< IP_INPUT) | X | | - * | R1 | REG_CT_TP_DST (0..15) | 0 | | R | | - * | | REG_CT_PROTO (16..23) | | | E | NEXT_HOP_IPV6 (>= IN_IP_ROUTING) | - * | | (>= IN_CT_EXTRACT && | | | G | | - * | | <= IN_LB_AFF_LEARN) | | | | | - * +-----+---------------------------+---+-----------------+---+------------------------------------+ + * | | (>= IN_IP_ROUTING) | E | INPORT_ETH_ADDR | | | + * +-----+---------------------------+ G | (< IP_INPUT) | | | + * | R1 | REG_CT_TP_DST (0..15) | 0 | | X | | + * | | REG_CT_PROTO (16..23) | | | X | NEXT_HOP_IPV6 (>= IN_IP_ROUTING) | + * | | (>= IN_CT_EXTRACT && | | | R | | + * | | <= IN_LB_AFF_LEARN) | | | E | | + * +-----+---------------------------+---+-----------------+ G +------------------------------------+ * | R2 | REG_DHCP_RELAY_DIP_IPV4 | | | 0 | | - * | | REG_LB_PORT | X | | 0 | | - * | | (>= IN_LB_AFF_CHECK | R | | | | - * | | <= IN_LB_AFF_LEARN) | E | | | | - * +-----+---------------------------+ G | UNUSED | | | + * | | REG_LB_PORT | X | | | | + * | | (>= IN_LB_AFF_CHECK | R | REG_ORIG_ETH_SRC| | | + * | | <= IN_LB_AFF_LEARN) | E | (>= IP_ROUTING_ | | | + * +-----+---------------------------+ G |PRE <= ICMP_RED) | | | * | R3 | UNUSED | 1 | | | | * | | | | | | | * +-----+---------------------------+---+-----------------+---+------------------------------------+ @@ -12503,13 +12510,18 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, const char *route_table_name = smap_get(&lrp->options, "route_table"); uint32_t rtb_id = get_route_table_id(route_tables, route_table_name); - if (!rtb_id) { + bool icmp_redirect = + smap_get_bool(&lrp->options, "send_icmp4_redirects", false); + if (!rtb_id && !icmp_redirect) { return; } ds_put_format(&match, "inport == \"%s\"", lrp->name); - ds_put_format(&actions, "%s = %d; next;", - REG_ROUTE_TABLE_ID, rtb_id); + if (icmp_redirect) { + /* Routing overwrites eth.src, save it for the ICMP redirect. */ + ds_put_format(&actions, "%s = eth.src; ", REG_ORIG_ETH_SRC); + } + ds_put_format(&actions, "%s = %d; next;", REG_ROUTE_TABLE_ID, rtb_id); ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_PRE, 100, ds_cstr(&match), ds_cstr(&actions), lflow_ref); @@ -12704,6 +12716,90 @@ parsed_route_lookup_by_source(enum route_source source, return NULL; } +/* Flow for building ICMP redirect packet (ICMP error type 5, + * code 1 - Redirect for Destination Host). + * + * RFC 1812 5.2.7.2 allows the Redirect only when: + * 1) the ingress and egress interfaces are the same. + * 2) the next hop sits on an ingress port network. + * 3) the next hop does not match source ip - this is + * checked by ovn-controller. + * 4) the packet is not icmp redirect itself. + * + * RFC 1812 lists additional conditions for sending a Redirect (e.g. the + * datagram is not source-routed (LSRR/SSRR options)), but OVN currently + * has no way to check those conditions, so they are not enforced here. + * + * We also rely on the destination not being on the ingress port network + * itself, so the Redirect always points to another router and never to the + * destination host. + */ +static void +build_icmp_redirect_flows_for_lrouter_port( + struct lflow_table *lflows, const struct ovn_port *op, + const struct shash *meter_groups, struct lflow_ref *lflow_ref, + struct ds *match, struct ds *actions) +{ + if (!smap_get_bool(&op->nbrp->options, "send_icmp4_redirects", false)) { + return; + } + + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i]; + + ds_clear(match); + ds_clear(actions); + ds_put_format(match, "ip4 && ip4.dst == %s/%u", + na->network_s, na->plen); + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ICMP_REDIRECT, 110, + ds_cstr(match), "next;", lflow_ref); + ds_clear(match); + ds_clear(actions); + + ds_put_format(match, + "inport == %s && outport == %s && ip4 && " + "ip4.src == %s/%u && " + REG_NEXT_HOP_IPV4" == %s/%u && " + "!ip.later_frag", + op->json_key, op->json_key, + na->network_s, na->plen, na->network_s, na->plen); + + ds_put_format(actions, + "icmp4_redirect {" + "eth.dst = "REG_ORIG_ETH_SRC"; eth.src = %s; " + "ip4.dst = ip4.src; ip4.src = %s; ip.ttl = 254; " + "outport = %s; flags.loopback = 1; output; }; next;", + op->lrp_networks.ea_s, na->addr_s, op->json_key); + + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ICMP_REDIRECT, 100, + 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_icmp_redirect_flows_for_lrouter(struct ovn_datapath *od, + struct lflow_table *lflows) +{ + ovn_lflow_add(lflows, od, S_ROUTER_IN_ICMP_REDIRECT, 0, "1", "next;", + od->datapath_lflows); + struct ovn_port *op; + HMAP_FOR_EACH (op, dp_node, &od->ports) { + if (op->lrp_networks.n_ipv4_addrs + && smap_get_bool(&op->nbrp->options, + "send_icmp4_redirects", false)) { + /* No Redirect in reply to a Redirect (RFC 1122 3.2.2). */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_ICMP_REDIRECT, 110, + "icmp4.type == {3, 5, 11}", "next;", + od->datapath_lflows); + break; + } + } +} + /* This hash needs to be equal to the one used in * build_route_flows_for_lrouter to iterate over all routes of a datapath. * This is distinct from route_hash which is stored in parsed_route->hash. */ @@ -15641,6 +15737,7 @@ build_route_flows_for_lrouter( { ovs_assert(od->nbr); build_default_route_flows_for_lrouter(od, lflows, route_tables); + build_icmp_redirect_flows_for_lrouter(od, lflows); const struct group_ecmp_datapath *datapath_node = group_ecmp_datapath_lookup(route_data, od); @@ -17800,6 +17897,10 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, match, actions, meter_groups, lflow_ref); + /* ICMP redirect */ + build_icmp_redirect_flows_for_lrouter_port(lflows, op, meter_groups, + lflow_ref, match, actions); + /* ARP reply. These flows reply to ARP requests for the router's own * IP address. */ for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { diff --git a/northd/northd.h b/northd/northd.h index 9a74a4abc..549907245 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -611,17 +611,18 @@ ls_has_localnet_port(const struct ovn_datapath *od) PIPELINE_STAGE(ROUTER, IN, IP_ROUTING_ECMP, 17, "lr_in_ip_routing_ecmp") \ PIPELINE_STAGE(ROUTER, IN, POLICY, 18, "lr_in_policy") \ PIPELINE_STAGE(ROUTER, IN, POLICY_ECMP, 19, "lr_in_policy_ecmp") \ - PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP_CHK, 20, \ + PIPELINE_STAGE(ROUTER, IN, ICMP_REDIRECT, 20, "lr_in_icmp_redirect") \ + PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP_CHK, 21, \ "lr_in_dhcp_relay_resp_chk") \ - PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP, 21, \ + PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP, 22, \ "lr_in_dhcp_relay_resp") \ - PIPELINE_STAGE(ROUTER, IN, ARP_RESOLVE, 22, "lr_in_arp_resolve") \ - PIPELINE_STAGE(ROUTER, IN, CHK_PKT_LEN, 23, "lr_in_chk_pkt_len") \ - PIPELINE_STAGE(ROUTER, IN, LARGER_PKTS, 24, "lr_in_larger_pkts") \ - PIPELINE_STAGE(ROUTER, IN, GW_REDIRECT, 25, "lr_in_gw_redirect") \ - PIPELINE_STAGE(ROUTER, IN, NETWORK_ID, 26, "lr_in_network_id") \ - PIPELINE_STAGE(ROUTER, IN, ARP_REQUEST, 27, "lr_in_arp_request") \ - PIPELINE_STAGE(ROUTER, IN, ECMP_STATEFUL_EGR, 28, \ + PIPELINE_STAGE(ROUTER, IN, ARP_RESOLVE, 23, "lr_in_arp_resolve") \ + PIPELINE_STAGE(ROUTER, IN, CHK_PKT_LEN, 24, "lr_in_chk_pkt_len") \ + PIPELINE_STAGE(ROUTER, IN, LARGER_PKTS, 25, "lr_in_larger_pkts") \ + PIPELINE_STAGE(ROUTER, IN, GW_REDIRECT, 26, "lr_in_gw_redirect") \ + PIPELINE_STAGE(ROUTER, IN, NETWORK_ID, 27, "lr_in_network_id") \ + PIPELINE_STAGE(ROUTER, IN, ARP_REQUEST, 28, "lr_in_arp_request") \ + PIPELINE_STAGE(ROUTER, IN, ECMP_STATEFUL_EGR, 29, \ "lr_in_ecmp_stateful_egr") /* Logical router egress stages. */ diff --git a/ovn-nb.xml b/ovn-nb.xml index 078bd6068..14fcf49c5 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -5028,6 +5028,23 @@ or router version of this option. </p> </column> + + <column name="options" key="send_icmp4_redirects" + type='{"type": "boolean"}'> + <p> + If set to <code>true</code>, the router sends an ICMPv4 Redirect to + the source of a packet that arrived on this port and is forwarded + back out of it, provided that the source and the next hop are on + the same network of the port. The Redirect names the next hop as + the better first hop; the packet itself is still forwarded. + No Redirect is sent when : + - The next hop is the source of the packet. + - When the packet is itself an ICMP Error. + - When the destination itself belongs to one of directly connected + router's network: a Redirect always points to another router. + It is <code>false</code> by default. + </p> + </column> </group> <group title="Attachment"> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 6572b1318..cdff4d06c 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -24352,3 +24352,71 @@ OVN_CLEANUP_NORTHD AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([ICMPv4 redirect]) +ovn_start + +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lrp0 00:00:00:00:00:01 192.168.1.1/24 +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:02 10.0.0.1/24 10.0.1.1/24 +check ovn-nbctl --wait=sb lrp-add lr1 lrp2 00:00:00:00:00:03 2001:db8::1/64 + +# Off by default. +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_icmp_redirect" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_icmp_redirect), priority=0 , match=(1), action=(next;) +]) +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_ip_routing_pre" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_ip_routing_pre), priority=0 , match=(1), action=(reg7 = 0; next;) +]) + +# Enabled per router port. +check ovn-nbctl --wait=sb set logical_router_port lrp1 options:send_icmp4_redirects=true +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_icmp_redirect" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_icmp_redirect), priority=0 , match=(1), action=(next;) + table=??(lr_in_icmp_redirect), priority=100 , match=(inport == "lrp1" && outport == "lrp1" && ip4 && ip4.src == 10.0.0.0/24 && reg0 == 10.0.0.0/24 && !ip.later_frag), action=(icmp4_redirect {eth.dst = xreg1[[0..47]]; eth.src = 00:00:00:00:00:02; ip4.dst = ip4.src; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; }; next;) + table=??(lr_in_icmp_redirect), priority=100 , match=(inport == "lrp1" && outport == "lrp1" && ip4 && ip4.src == 10.0.1.0/24 && reg0 == 10.0.1.0/24 && !ip.later_frag), action=(icmp4_redirect {eth.dst = xreg1[[0..47]]; eth.src = 00:00:00:00:00:02; ip4.dst = ip4.src; ip4.src = 10.0.1.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; }; next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(icmp4.type == {3, 5, 11}), action=(next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(ip4 && ip4.dst == 10.0.0.0/24), action=(next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(ip4 && ip4.dst == 10.0.1.0/24), action=(next;) +]) + +# The original eth.src is saved only on ports with the option enabled. +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_ip_routing_pre" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_ip_routing_pre), priority=0 , match=(1), action=(reg7 = 0; next;) + table=??(lr_in_ip_routing_pre), priority=100 , match=(inport == "lrp1"), action=(xreg1[[0..47]] = eth.src; reg7 = 0; next;) +]) + +# Together with a route table. +check ovn-nbctl --wait=sb set logical_router_port lrp1 options:route_table=rtb1 +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_ip_routing_pre" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_ip_routing_pre), priority=0 , match=(1), action=(reg7 = 0; next;) + table=??(lr_in_ip_routing_pre), priority=100 , match=(inport == "lrp1"), action=(xreg1[[0..47]] = eth.src; reg7 = 1; next;) +]) +check ovn-nbctl --wait=sb remove logical_router_port lrp1 options route_table + +check ovn-nbctl --wait=sb set logical_router_port lrp0 options:send_icmp4_redirects=true +check ovn-nbctl --wait=sb set logical_router_port lrp2 options:send_icmp4_redirects=true +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_icmp_redirect" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_icmp_redirect), priority=0 , match=(1), action=(next;) + table=??(lr_in_icmp_redirect), priority=100 , match=(inport == "lrp0" && outport == "lrp0" && ip4 && ip4.src == 192.168.1.0/24 && reg0 == 192.168.1.0/24 && !ip.later_frag), action=(icmp4_redirect {eth.dst = xreg1[[0..47]]; eth.src = 00:00:00:00:00:01; ip4.dst = ip4.src; ip4.src = 192.168.1.1; ip.ttl = 254; outport = "lrp0"; flags.loopback = 1; output; }; next;) + table=??(lr_in_icmp_redirect), priority=100 , match=(inport == "lrp1" && outport == "lrp1" && ip4 && ip4.src == 10.0.0.0/24 && reg0 == 10.0.0.0/24 && !ip.later_frag), action=(icmp4_redirect {eth.dst = xreg1[[0..47]]; eth.src = 00:00:00:00:00:02; ip4.dst = ip4.src; ip4.src = 10.0.0.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; }; next;) + table=??(lr_in_icmp_redirect), priority=100 , match=(inport == "lrp1" && outport == "lrp1" && ip4 && ip4.src == 10.0.1.0/24 && reg0 == 10.0.1.0/24 && !ip.later_frag), action=(icmp4_redirect {eth.dst = xreg1[[0..47]]; eth.src = 00:00:00:00:00:02; ip4.dst = ip4.src; ip4.src = 10.0.1.1; ip.ttl = 254; outport = "lrp1"; flags.loopback = 1; output; }; next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(icmp4.type == {3, 5, 11}), action=(next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(ip4 && ip4.dst == 10.0.0.0/24), action=(next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(ip4 && ip4.dst == 10.0.1.0/24), action=(next;) + table=??(lr_in_icmp_redirect), priority=110 , match=(ip4 && ip4.dst == 192.168.1.0/24), action=(next;) +]) + +check ovn-nbctl --wait=sb set logical_router_port lrp0 options:send_icmp4_redirects=false +check ovn-nbctl --wait=sb remove logical_router_port lrp1 options send_icmp4_redirects +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_icmp_redirect" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_icmp_redirect), priority=0 , match=(1), action=(next;) +]) +AT_CHECK([ovn-sbctl lflow-list | grep "lr_in_ip_routing_pre" | ovn_strip_lflows], [0], [dnl + table=??(lr_in_ip_routing_pre), priority=0 , match=(1), action=(reg7 = 0; next;) + table=??(lr_in_ip_routing_pre), priority=100 , match=(inport == "lrp2"), action=(xreg1[[0..47]] = eth.src; reg7 = 0; next;) +]) + +OVN_CLEANUP_NORTHD +AT_CLEANUP +]) diff --git a/tests/ovn.at b/tests/ovn.at index 13e95f9db..7e6b27bbe 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -47407,3 +47407,39 @@ AT_CHECK([grep "skipping output to input port" \ OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([ICMPv4 redirect - ovn-trace]) +ovn_start + +check ovn-nbctl lr-add lr1 +check ovn-nbctl lrp-add lr1 lrp0 00:00:00:00:00:01 10.0.0.1/24 +check ovn-nbctl lrp-set-options lrp0 send_icmp4_redirects=true +check ovn-nbctl ls-add sw1 +check ovn-nbctl lsp-add-router-port sw1 sw-lr1 lrp0 +check ovn-nbctl lsp-add sw1 sw1-lport1 +check ovn-nbctl lsp-set-addresses sw1-lport1 "00:00:00:00:00:10 10.0.0.10" + +# A better first hop for 172.16.0.0/24 sits on the same segment as the VM. +check ovn-nbctl --wait=sb lr-route-add lr1 172.16.0.0/24 10.0.0.80 + +AT_CHECK([ovn-trace lr1 'inport == "lrp0" && eth.src == 00:00:00:00:00:10 && eth.dst == 00:00:00:00:00:01 && ip4.src == 10.0.0.10 && ip4.dst == 172.16.0.5 && ip.ttl == 64 && icmp4.type == 8' | + grep -c "icmp4_redirect: gw = 10.0.0.80"], [0], [dnl +1 +]) + +# No Redirect in reply to a Redirect. +AT_CHECK([ovn-trace lr1 'inport == "lrp0" && eth.src == 00:00:00:00:00:10 && eth.dst == 00:00:00:00:00:01 && ip4.src == 10.0.0.10 && ip4.dst == 172.16.0.5 && ip.ttl == 64 && icmp4.type == 5' | \ + grep -c "icmp4_redirect"], [1], [dnl +0 +]) + +# No Redirect to the next hop itself. +AT_CHECK([ovn-trace lr1 'inport == "lrp0" && eth.src == 00:00:00:00:00:80 && eth.dst == 00:00:00:00:00:01 && ip4.src == 10.0.0.80 && ip4.dst == 172.16.0.5 && ip.ttl == 64 && icmp4.type == 8' | \ + grep -c "icmp4_redirect: suppressed, next hop 10.0.0.80 is the source"], [0], [dnl +1 +]) + +OVN_CLEANUP_NORTHD +AT_CLEANUP +]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 5b6ba3731..f18f0b06b 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -23978,3 +23978,109 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ICMPv4 redirect]) +AT_KEYWORDS([icmp redirect]) + +ovn_start + +OVS_TRAFFIC_VSWITCHD_START() +ADD_BR([br-int]) + +# Set external-ids in br-int needed for ovn-controller +ovs-vsctl \ + -- set Open_vSwitch . external-ids:system-id=hv1 \ + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true + +# Start ovn-controller +start_daemon ovn-controller + +# One switch, one router. sw0-p1 is the host with the bad routing table: +# it sends everything to lr0. sw0-p2 is the better first hop: it also owns +# 10.10.10.1, and lr0 has a static route for 10.10.10.0/24 pointing at it. +# So a packet from sw0-p1 to 10.10.10.1 goes lr0 -> sw0-p2, back out of the +# port it came in on, and lr0 must tell sw0-p1 to use sw0-p2 directly. +check ovn-nbctl ls-add sw0 + +check ovn-nbctl lsp-add sw0 sw0-p1 +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:04 172.31.0.4" + +check ovn-nbctl lsp-add sw0 sw0-p2 +check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:50 172.31.0.80" + +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 172.31.0.1/16 +check ovn-nbctl lrp-set-options lr0-sw0 send_icmp4_redirects=true +check ovn-nbctl lsp-add-router-port sw0 sw0-lr0 lr0-sw0 +check ovn-nbctl lr-route-add lr0 10.10.10.0/24 172.31.0.80 + +ADD_NAMESPACES(sw0-p1) +ADD_VETH(sw0-p1, sw0-p1, br-int, "172.31.0.4/16", "50:54:00:00:00:04", \ + "172.31.0.1") +NS_CHECK_EXEC([sw0-p1], [sysctl -q -w net.ipv4.conf.all.accept_redirects=1]) +NS_CHECK_EXEC([sw0-p1], [sysctl -q -w net.ipv4.conf.sw0-p1.accept_redirects=1]) + +ADD_NAMESPACES(sw0-p2) +ADD_VETH(sw0-p2, sw0-p2, br-int, "172.31.0.80/16", "50:54:00:00:00:50") +NS_CHECK_EXEC([sw0-p2], [ip addr add 10.10.10.1/24 dev sw0-p2]) + +OVN_POPULATE_ARP +check ovn-nbctl --wait=hv sync + +NS_CHECK_EXEC([sw0-p1], [ping -q -c 1 -w 2 172.31.0.80 | FORMAT_PING], \ +[0], [dnl +1 packets transmitted, 1 received, 0% packet loss, time 0ms +]) + +NETNS_START_TCPDUMP([sw0-p1], [-nn -i sw0-p1 icmp and src 172.31.0.1], [sw0-p1-redirect]) +NETNS_START_TCPDUMP([sw0-p2], [-nn -e -i sw0-p2 icmp and dst 10.10.10.1], [sw0-p2-echo]) + +NS_CHECK_EXEC([sw0-p1], [ping -q -c 3 -i 0.3 -w 2 10.10.10.1 | FORMAT_PING], \ +[0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +OVS_WAIT_UNTIL([ + grep -q "172.31.0.1 > 172.31.0.4: ICMP redirect 10.10.10.1 to host 172.31.0.80" sw0-p1-redirect.tcpdump +]) + +# The first echo request went through lr0, i.e. left it with lr0's MAC. +OVS_WAIT_UNTIL([ + grep "00:00:00:00:ff:01 > 50:54:00:00:00:50" sw0-p2-echo.tcpdump | grep -q "172.31.0.4 > 10.10.10.1" +]) + +# sw0-p1 applied the Redirect: later ones come straight from it. +OVS_WAIT_UNTIL([ + grep "50:54:00:00:00:04 > 50:54:00:00:00:50" sw0-p2-echo.tcpdump | grep -q "172.31.0.4 > 10.10.10.1" +]) +AT_CHECK([ip netns exec sw0-p1 ip route get 10.10.10.1 | grep -q "via 172.31.0.80"]) + +# Turning the option off stops the Redirects but not the traffic. +check ovn-nbctl --wait=hv set logical_router_port lr0-sw0 options:send_icmp4_redirects=false +# Drop the learned exception so that sw0-p1 goes through lr0 again. +NS_CHECK_EXEC([sw0-p1], [ip route flush cache]) +kill $(cat sw0-p1-redirect.pid) +rm -f sw0-p1-redirect.tcpdump +NETNS_START_TCPDUMP([sw0-p1], [-nn -i sw0-p1 icmp and src 172.31.0.1], [sw0-p1-redirect]) + +NS_CHECK_EXEC([sw0-p1], [ping -q -c 3 -i 0.3 -w 2 10.10.10.1 | FORMAT_PING], \ +[0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) +ovn-sbctl lflow-list | grep icmp_redirect +AT_CHECK([grep -c "ICMP redirect" sw0-p1-redirect.tcpdump], [1], [dnl +0 +]) + +OVN_CLEANUP_CONTROLLER([hv1]) +OVN_CLEANUP_NORTHD + +as +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d +/connection dropped.*/d"]) +AT_CLEANUP +]) -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
