Rate limiting for ICMP Redirects is added in ovn-controller. The logic follows the Linux kernel: there, an interface answers the first 9 packets with an ICMP Redirect, doubling a backoff timer between sends. After the 9th packet it stops answering altogether and waits for about 20 seconds of complete silence from the sender before sending Redirects again. OVN uses a limit of 5 Redirects instead, with a backoff interval that starts at 1 second. A new option, ovn-icmp-redirect-quiet-timeout-sec, allows the quiet interval to be adjusted.
Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- controller/ovn-controller.8.xml | 11 ++ controller/pinctrl.c | 172 +++++++++++++++++++++++++++++++- ovn-nb.xml | 11 ++ tests/ovn.at | 109 ++++++++++++++++++++ 4 files changed, 301 insertions(+), 2 deletions(-) diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index 3c33654ff..39f4338fa 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -328,6 +328,17 @@ vlan type logical switch. </dd> + <dt><code>external_ids:ovn-icmp-redirect-quiet-timeout-sec</code></dt> + <dd> + How long, in seconds, a sender has to stop asking for ICMPv4 + Redirects before <code>ovn-controller</code> answers it again. + A sender gets at most five Redirects, spaced by an interval that + starts at one second and doubles every time, and no more until it + has been quiet for this long. The default is <code>20</code> sec. + Setting it to <code>0</code> turns the per-sender limiting off, + leaving only the meter. + </dd> + <dt><code>external_ids:ovn-is-interconn</code></dt> <dd> The boolean flag indicates if the chassis is used as an diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 005c9956c..b4e10b4aa 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -196,6 +196,9 @@ static struct pinctrl pinctrl; static bool pinctrl_is_sb_commited(int64_t commit_cfg, int64_t cur_cfg); static void init_buffered_packets_map(void); static void destroy_buffered_packets_map(void); +static void init_icmp_redirect_map(void); +static void destroy_icmp_redirect_map(void); +static void icmp_redirect_map_gc(long long int now); static void run_buffered_binding(const struct sbrec_mac_binding_table *mac_binding_table, const struct hmap *local_datapaths, @@ -395,6 +398,8 @@ COVERAGE_DEFINE(pinctrl_ring_full_put_mac_binding); COVERAGE_DEFINE(pinctrl_drop_put_fdb); COVERAGE_DEFINE(pinctrl_ring_full_put_fdb); COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map); +COVERAGE_DEFINE(pinctrl_icmp_redirect_suppressed); +COVERAGE_DEFINE(pinctrl_icmp_redirect_map_full); COVERAGE_DEFINE(pinctrl_drop_controller_event); COVERAGE_DEFINE(pinctrl_drop_put_vport_binding); COVERAGE_DEFINE(pinctrl_notify_main_thread); @@ -572,6 +577,7 @@ pinctrl_init(void) init_ipv6_ras(); init_ipv6_prefixd(); init_buffered_packets_map(); + init_icmp_redirect_map(); init_activated_ports(); init_event_table(); ip_mcast_snoop_init(); @@ -1696,6 +1702,157 @@ pinctrl_handle_arp(struct rconn *swconn, const struct flow *ip_flow, dp_packet_uninit(&packet); } +#define ICMP_REDIRECT_MIN_INTERVAL_MS 1000 +#define ICMP_REDIRECT_BURST 5 +#define ICMP_REDIRECT_QUIET_TIMEOUT_DEF_MS (20 * 1000) +#define ICMP_REDIRECT_MAP_MAX_SIZE 8192 + +/* Written by the main thread, read by the pinctrl_handler thread. */ +static atomic_llong icmp_redirect_quiet_timeout = + ICMP_REDIRECT_QUIET_TIMEOUT_DEF_MS; + +struct icmp_redirect_entry { + struct hmap_node hmap_node; + ovs_be64 dp_key; + ovs_be32 src_ip; + ovs_be32 gateway; + long long int last_seen; /* Time of the last packet, in ms. */ + long long int last_sent; /* Time of the last Redirect, in ms. */ + long long int interval; /* Minimum time until the next one, in ms. */ + unsigned int sent; /* Redirects sent in the current burst. */ +}; + +static struct hmap icmp_redirect_map; + +static void +init_icmp_redirect_map(void) +{ + hmap_init(&icmp_redirect_map); +} + +static void +destroy_icmp_redirect_map(void) +{ + struct icmp_redirect_entry *e; + HMAP_FOR_EACH_POP (e, hmap_node, &icmp_redirect_map) { + free(e); + } + hmap_destroy(&icmp_redirect_map); +} + +static uint32_t +icmp_redirect_hash(ovs_be64 dp_key, ovs_be32 src_ip, ovs_be32 gateway) +{ + return hash_2words((OVS_FORCE uint32_t) src_ip, + hash_uint64_basis((OVS_FORCE uint64_t) dp_key, + (OVS_FORCE uint32_t) gateway)); +} + +static struct icmp_redirect_entry * +icmp_redirect_lookup(ovs_be64 dp_key, ovs_be32 src_ip, ovs_be32 gateway, + uint32_t hash) +{ + struct icmp_redirect_entry *e; + HMAP_FOR_EACH_WITH_HASH (e, hmap_node, hash, &icmp_redirect_map) { + if (e->dp_key == dp_key && e->src_ip == src_ip + && e->gateway == gateway) { + return e; + } + } + return NULL; +} + +static bool +icmp_redirect_may_send(ovs_be64 dp_key, ovs_be32 src_ip, ovs_be32 gateway, + long long int now) +{ + uint32_t hash = icmp_redirect_hash(dp_key, src_ip, gateway); + struct icmp_redirect_entry *e = icmp_redirect_lookup(dp_key, src_ip, + gateway, hash); + if (!e) { + if (hmap_count(&icmp_redirect_map) >= ICMP_REDIRECT_MAP_MAX_SIZE) { + /* Too many senders to keep track of. + * Send withous rate limiting. */ + COVERAGE_INC(pinctrl_icmp_redirect_map_full); + return true; + } + e = xmalloc(sizeof *e); + e->dp_key = dp_key; + e->src_ip = src_ip; + e->gateway = gateway; + e->last_seen = now; + e->last_sent = now; + e->interval = ICMP_REDIRECT_MIN_INTERVAL_MS; + e->sent = 1; + hmap_insert(&icmp_redirect_map, &e->hmap_node, hash); + return true; + } + + long long int quiet_timeout; + atomic_read_relaxed(&icmp_redirect_quiet_timeout, &quiet_timeout); + if (now - e->last_seen >= quiet_timeout) { + /* The sender has been quiet: start a new burst. */ + e->last_seen = now; + e->last_sent = now; + e->interval = ICMP_REDIRECT_MIN_INTERVAL_MS; + e->sent = 1; + return true; + } + e->last_seen = now; + + if (e->sent >= ICMP_REDIRECT_BURST + || now - e->last_sent < e->interval) { + COVERAGE_INC(pinctrl_icmp_redirect_suppressed); + return false; + } + + e->interval *= 2; + e->last_sent = now; + e->sent++; + return true; +} + +static void +icmp_redirect_config_run(const struct ovsrec_open_vswitch_table *ovs_table, + const struct sbrec_chassis *chassis) +{ + const struct ovsrec_open_vswitch *cfg = + ovsrec_open_vswitch_table_first(ovs_table); + + if (!cfg || !chassis) { + return; + } + + long long int quiet_timeout = + (long long int) get_chassis_external_id_value_uint( + &cfg->external_ids, chassis->name, + "ovn-icmp-redirect-quiet-timeout-sec", + ICMP_REDIRECT_QUIET_TIMEOUT_DEF_MS / 1000) * 1000; + atomic_store_relaxed(&icmp_redirect_quiet_timeout, quiet_timeout); +} + +static void +icmp_redirect_map_gc(long long int now) +{ + static long long int next_gc = LLONG_MIN; + + if (now < next_gc) { + return; + } + + long long int quiet_timeout; + atomic_read_relaxed(&icmp_redirect_quiet_timeout, &quiet_timeout); + next_gc = now + quiet_timeout; + + struct icmp_redirect_entry *e; + HMAP_FOR_EACH_SAFE (e, hmap_node, &icmp_redirect_map) { + if (now - e->last_seen >= quiet_timeout) { + hmap_remove(&icmp_redirect_map, &e->hmap_node); + free(e); + } + } +} + /* Called with in the pinctrl_handler thread context. */ static void pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, @@ -1720,8 +1877,15 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, * the conditions - that the next hop is not the source of the packet - * compares two run-time values, which the logical flow match language * cannot express. So check it here instead. */ - if (redirect && htonl(md->flow.regs[0]) == ip_flow->nw_src) { - return; + if (redirect) { + ovs_be32 gateway = htonl(md->flow.regs[0]); + if (gateway == ip_flow->nw_src) { + return; + } + if (!icmp_redirect_may_send(md->flow.metadata, ip_flow->nw_src, + gateway, time_msec())) { + return; + } } uint64_t ofpacts_stub[4096 / 8]; @@ -4083,6 +4247,8 @@ pinctrl_handler(void *arg_) lock_failed = true; } + icmp_redirect_map_gc(time_msec()); + rconn_run(swconn); new_seq = seq_read(pinctrl_handler_seq); if (rconn_is_connected(swconn)) { @@ -4242,6 +4408,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, run_put_vport_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key, sbrec_port_binding_by_key, chassis, cur_cfg); send_garp_rarp_prepare(ecmp_nh_table, chassis, ovs_table); + icmp_redirect_config_run(ovs_table, chassis); prepare_ipv6_ras(local_active_ports_ras, sbrec_port_binding_by_name, chassis); prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name, @@ -4810,6 +4977,7 @@ pinctrl_destroy(void) destroy_ipv6_ras(); destroy_ipv6_prefixd(); destroy_buffered_packets_map(); + destroy_icmp_redirect_map(); destroy_activated_ports(); event_table_destroy(); destroy_mac_bindings(); diff --git a/ovn-nb.xml b/ovn-nb.xml index 14fcf49c5..4e3290eb8 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -5044,6 +5044,17 @@ or router's network: a Redirect always points to another router. It is <code>false</code> by default. </p> + <p> + Redirects are sent by <code>ovn-controller</code> and are rate + limited per sender and next hop: a sender gets at most five + redirects, spaced by an interval that starts at one second and + doubles every time, and no more until it stops sending packets + that need one for the time set + by <code>external_ids:ovn-icmp-redirect-quiet-timeout-sec</code> + in the local <code>Open_vSwitch</code> table, twenty seconds by + default. A sender that ignores Redirects therefore does not flood + the controller. + </p> </column> </group> diff --git a/tests/ovn.at b/tests/ovn.at index 7e6b27bbe..9dda0e28a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -47443,3 +47443,112 @@ AT_CHECK([ovn-trace lr1 'inport == "lrp0" && eth.src == 00:00:00:00:00:80 && eth OVN_CLEANUP_NORTHD AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ICMPv4 redirect - rate limiting]) +AT_KEYWORDS([icmp redirect]) +CHECK_SCAPY +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" +check ovn-nbctl lsp-add sw1 sw1-lport2 +check ovn-nbctl lsp-set-addresses sw1-lport2 "00:00:00:00:00:80 10.0.0.80" + +# A better first hop for 172.16.0.0/24 sits on the same segment as the VM. +check ovn-nbctl lr-route-add lr1 172.16.0.0/24 10.0.0.80 + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl -- add-port br-int vif1 -- \ + set interface vif1 external-ids:iface-id=sw1-lport1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap +ovs-vsctl -- add-port br-int vif2 -- \ + set interface vif2 external-ids:iface-id=sw1-lport2 \ + options:tx_pcap=hv1/vif2-tx.pcap \ + options:rxq_pcap=hv1/vif2-rx.pcap + +wait_for_ports_up +check ovn-nbctl --wait=hv sync +OVN_POPULATE_ARP + +packet=$(fmt_pkt "Ether(dst='00:00:00:00:00:01', src='00:00:00:00:00:10')/ \ + IP(src='10.0.0.10', dst='172.16.0.5', ttl=64)/ \ + ICMP(type=8, id=0x1234, seq=1)") + +# The forwarded packet, as sw1-lport2 sees it: routed, so from lrp0's MAC. +forwarded=$(fmt_pkt "Ether(dst='00:00:00:00:00:80', src='00:00:00:00:00:01')/ \ + IP(src='10.0.0.10', dst='172.16.0.5', ttl=63)/ \ + ICMP(type=8, id=0x1234, seq=1)") + +# ICMP Redirect for host, from lrp0 to the sender, naming 10.0.0.80. +# Match on the IP addresses and the ICMP type/code that follow them. +redirect_pattern="0a0000010a00000a0501" + +count_redirects() { + $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap | \ + grep -c "$redirect_pattern" +} + +# A burst of packets from the same sender earns a single Redirect... +for i in 1 2 3; do + check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet +done + +# ...while every packet is still forwarded. +for i in 1 2 3; do echo $forwarded; done > vif2.expected +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [vif2.expected]) + +OVS_WAIT_UNTIL([test "$(count_redirects)" = 1]) +sleep 1 +AT_CHECK([count_redirects], [0], [1 +]) + +# Once the backoff interval has passed, the next packet earns another one. +# The interval doubles every time: 1s, 2s, 4s and 8s. +n=2 +for interval in 2 3 5 9; do + sleep $interval + check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet + OVS_WAIT_UNTIL([test "$(count_redirects)" = $n]) + n=$((n + 1)) +done + +# The burst of 5 is spent: no more Redirects, however long we wait. +for i in 1 2 3; do + check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet + sleep 1 +done +AT_CHECK([count_redirects], [0], [5 +]) + +# A sender that goes quiet starts over. Shorten the quiet timeout from its +# 20s default so that the test does not have to wait that long. +check as hv1 ovs-vsctl set open . \ + external_ids:ovn-icmp-redirect-quiet-timeout-sec=2 +sleep 3 +check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet +OVS_WAIT_UNTIL([test "$(count_redirects)" = 6]) + +# A zero timeout turns the per-sender limiting off: every packet that +# deserves a Redirect gets one. The packet is resent until ovn-controller +# has picked the new value up. +check as hv1 ovs-vsctl set open . \ + external_ids:ovn-icmp-redirect-quiet-timeout-sec=0 +OVS_WAIT_UNTIL([as hv1 ovs-appctl netdev-dummy/receive vif1 $packet + test "$(count_redirects)" = 7]) +check as hv1 ovs-appctl netdev-dummy/receive vif1 $packet +OVS_WAIT_UNTIL([test "$(count_redirects)" = 8]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
