ICMP redirect (icmp type 5, code 1 - Redirect for the Host) is used to tell sender to use more efficient route for sending packets to a destination. When a router receives a packet and determines that another router on the same network provides a shorter path, it sends an ICMP Redirect message to sender, advising it to send future packets to the optimal router.
The use of this functionality in northd will be implemented in the next commit, in addition, rate-limiting support for such packets will be added. Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- controller/pinctrl.c | 38 ++++++++++++++++++++++++++++++-------- include/ovn/actions.h | 11 ++++++++++- lib/actions.c | 23 +++++++++++++++++++++++ lib/ovn-util.c | 2 +- utilities/ovn-trace.c | 37 ++++++++++++++++++++++++++++++------- 5 files changed, 94 insertions(+), 17 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index e10091251..005c9956c 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -1701,7 +1701,7 @@ static void pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, struct dp_packet *pkt_in, const struct match *md, struct ofpbuf *userdata, - bool set_icmp_code, bool loopback) + bool set_icmp_code, bool loopback, bool redirect) { enum ofp_version version = rconn_get_version(swconn); @@ -1716,6 +1716,14 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, return; } + /* Northd decides whether a packet deserves an ICMP Redirect, but one of + * 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; + } + uint64_t ofpacts_stub[4096 / 8]; struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub); @@ -1784,10 +1792,18 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, void *data = ih + 1; memcpy(data, in_ip, in_ip_len); - ovs_be16 *mtu = ofpacts_get_ovn_field(&ofpacts, OVN_ICMP4_FRAG_MTU); - if (mtu) { - ih->icmp_fields.frag.mtu = *mtu; - ih->icmp_code = 4; + if (redirect) { + put_16aligned_be32(&ih->icmp_fields.gateway, + htonl(md->flow.regs[0])); + ih->icmp_type = ICMP4_REDIRECT; + ih->icmp_code = 1; + } else { + ovs_be16 *mtu = ofpacts_get_ovn_field(&ofpacts, + OVN_ICMP4_FRAG_MTU); + if (mtu) { + ih->icmp_fields.frag.mtu = *mtu; + ih->icmp_code = 4; + } } ih->icmp_csum = 0; @@ -2072,7 +2088,8 @@ pinctrl_handle_reject(struct rconn *swconn, const struct flow *ip_flow, } else if (ip_flow->nw_proto == IPPROTO_SCTP) { pinctrl_handle_sctp_abort(swconn, ip_flow, pkt_in, md, userdata, true); } else { - pinctrl_handle_icmp(swconn, ip_flow, pkt_in, md, userdata, true, true); + pinctrl_handle_icmp(swconn, ip_flow, pkt_in, md, userdata, true, true, + false); } } @@ -3855,13 +3872,18 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg) case ACTION_OPCODE_ICMP: pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, - &userdata, true, false); + &userdata, true, false, false); + break; + + case ACTION_OPCODE_ICMP4_REDIRECT: + pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, + &userdata, false, false, true); break; case ACTION_OPCODE_ICMP4_ERROR: case ACTION_OPCODE_ICMP6_ERROR: pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, - &userdata, false, false); + &userdata, false, false, false); break; case ACTION_OPCODE_TCP_RESET: diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 7def8917e..997b0e05b 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -139,6 +139,7 @@ struct collector_set_ids; OVNACT(CHK_EVPN_ARP, ovnact_chk_evpn_arp) \ OVNACT(NF_LEARN_ORIG_INPORT, ovnact_nf_learn) \ OVNACT(NF_LOOKUP_ORIG_INPORT, ovnact_nf_lookup) \ + OVNACT(ICMP4_REDIRECT, ovnact_nest) \ /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ enum OVS_PACKED_ENUM ovnact_type { @@ -845,7 +846,15 @@ OVNACTS * Arguments follow the action_header, in this format: * - The 32-bit IPv4 address. */ \ - ACTION_OPCODE(PUT_ICMP4_INNER_IP4_SRC) + ACTION_OPCODE(PUT_ICMP4_INNER_IP4_SRC) \ + \ + /* "icmp4_redirect { ...actions... }". + * + * The actions, in OpenFlow 1.3 format, follow the action_header. The + * address of the better first hop is taken from reg0, where the logical + * router pipeline leaves the resolved next hop. + */ \ + ACTION_OPCODE(ICMP4_REDIRECT) enum action_opcode { diff --git a/lib/actions.c b/lib/actions.c index 0e95e2d70..026d9ed5a 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -1828,6 +1828,12 @@ parse_ICMP4(struct action_context *ctx) parse_nested_action(ctx, OVNACT_ICMP4, "ip4", ctx->scope); } +static void +parse_ICMP4_REDIRECT(struct action_context *ctx) +{ + parse_nested_action(ctx, OVNACT_ICMP4_REDIRECT, "ip4", ctx->scope); +} + static void parse_ICMP4_ERROR(struct action_context *ctx) { @@ -1909,6 +1915,12 @@ format_ICMP4(const struct ovnact_nest *nest, struct ds *s) format_nested_action(nest, "icmp4", s); } +static void +format_ICMP4_REDIRECT(const struct ovnact_nest *nest, struct ds *s) +{ + format_nested_action(nest, "icmp4_redirect", s); +} + static void format_ICMP4_ERROR(const struct ovnact_nest *nest, struct ds *s) { @@ -2010,6 +2022,7 @@ is_paused_nested_action(enum action_opcode opcode) case ACTION_OPCODE_PUT_ND_RA_OPTS: case ACTION_OPCODE_ICMP: case ACTION_OPCODE_ICMP4_ERROR: + case ACTION_OPCODE_ICMP4_REDIRECT: case ACTION_OPCODE_ICMP6_ERROR: case ACTION_OPCODE_TCP_RESET: case ACTION_OPCODE_SCTP_ABORT: @@ -2073,6 +2086,14 @@ encode_ICMP4(const struct ovnact_nest *on, encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts); } +static void +encode_ICMP4_REDIRECT(const struct ovnact_nest *on, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + encode_nested_actions(on, ep, ACTION_OPCODE_ICMP4_REDIRECT, ofpacts); +} + static void encode_ICMP4_ERROR(const struct ovnact_nest *on, const struct ovnact_encode_params *ep, @@ -5942,6 +5963,8 @@ parse_action(struct action_context *ctx) parse_CLONE(ctx); } else if (lexer_match_id(ctx->lexer, "arp")) { parse_ARP(ctx); + } else if (lexer_match_id(ctx->lexer, "icmp4_redirect")) { + parse_ICMP4_REDIRECT(ctx); } else if (lexer_match_id(ctx->lexer, "icmp4")) { parse_ICMP4(ctx); } else if (lexer_match_id(ctx->lexer, "icmp4_error")) { diff --git a/lib/ovn-util.c b/lib/ovn-util.c index eb1fa8a06..f8094afc3 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -1007,7 +1007,7 @@ 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 "3980195012 11262" +#define OVN_NORTHD_PIPELINE_CSUM "289408664 11318" #define OVN_INTERNAL_MINOR_VER 16 /* Returns the OVN version. The caller must free the returned value. */ diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 8901354cb..3a8414105 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -1859,7 +1859,8 @@ static void execute_icmp4(const struct ovnact_nest *on, const struct ovntrace_datapath *dp, const struct flow *uflow, uint8_t table_id, bool loopback, - enum ovnact_pipeline pipeline, struct ovs_list *super) + bool redirect, enum ovnact_pipeline pipeline, + struct ovs_list *super) { struct flow icmp4_flow = *uflow; @@ -1867,6 +1868,14 @@ execute_icmp4(const struct ovnact_nest *on, return; /* Avoid recirculation. */ } + /* Same check as ovn-controller: no Redirect to the next hop itself. */ + if (redirect && htonl(uflow->regs[0]) == uflow->nw_src) { + ovntrace_node_append(super, OVNTRACE_NODE_TRANSFORMATION, + "icmp4_redirect: suppressed, next hop "IP_FMT + " is the source", IP_ARGS(uflow->nw_src)); + return; + } + /* Update fields for ICMP. */ if (loopback) { icmp4_flow.dl_dst = uflow->dl_src; @@ -1881,11 +1890,19 @@ execute_icmp4(const struct ovnact_nest *on, } icmp4_flow.nw_proto = IPPROTO_ICMP; icmp4_flow.nw_ttl = 255; - icmp4_flow.tp_src = htons(ICMP4_DST_UNREACH); /* icmp type */ + + icmp4_flow.tp_src = htons(redirect ? ICMP4_REDIRECT + : ICMP4_DST_UNREACH); /* icmp type */ icmp4_flow.tp_dst = htons(1); /* icmp code */ - struct ovntrace_node *node = ovntrace_node_append( - super, OVNTRACE_NODE_TRANSFORMATION, "icmp4"); + /* The Redirect names the next hop that routing left in reg0. It is not + * a field of the flow, so spell it out here: which better first hop the + * sender is told about is the whole point of tracing one. */ + struct ovntrace_node *node = redirect + ? ovntrace_node_append(super, OVNTRACE_NODE_TRANSFORMATION, + "icmp4_redirect: gw = "IP_FMT, + IP_ARGS(htonl(uflow->regs[0]))) + : ovntrace_node_append(super, OVNTRACE_NODE_TRANSFORMATION, "icmp4"); trace_actions(on->nested, on->nested_len, dp, &icmp4_flow, table_id, pipeline, &node->subs); @@ -2116,7 +2133,8 @@ execute_reject(const struct ovnact_nest *on, execute_sctp_abort(on, dp, uflow, table_id, true, pipeline, super); } else { if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) { - execute_icmp4(on, dp, uflow, table_id, true, pipeline, super); + execute_icmp4(on, dp, uflow, table_id, true, false, + pipeline, super); } else { execute_icmp6(on, dp, uflow, table_id, true, pipeline, super); } @@ -3512,12 +3530,17 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, case OVNACT_ICMP4: execute_icmp4(ovnact_get_ICMP4(a), dp, uflow, table_id, false, - pipeline, super); + false, pipeline, super); break; case OVNACT_ICMP4_ERROR: execute_icmp4(ovnact_get_ICMP4_ERROR(a), dp, uflow, table_id, - false, pipeline, super); + false, false, pipeline, super); + break; + + case OVNACT_ICMP4_REDIRECT: + execute_icmp4(ovnact_get_ICMP4_REDIRECT(a), dp, uflow, table_id, + false, true, pipeline, super); break; case OVNACT_ICMP6: -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
