From patchwork Fri Apr 12 12:54:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1084720 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44gdFx4Hf6z9s8m for ; Fri, 12 Apr 2019 22:57:09 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id B6033F05; Fri, 12 Apr 2019 12:55:03 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 50779EFF for ; Fri, 12 Apr 2019 12:55:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 317F27ED for ; Fri, 12 Apr 2019 12:55:01 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D375B3007C2A for ; Fri, 12 Apr 2019 12:55:00 +0000 (UTC) Received: from nusiddiq.mac (unknown [10.74.10.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id F188860FCD; Fri, 12 Apr 2019 12:54:58 +0000 (UTC) From: nusiddiq@redhat.com To: dev@openvswitch.org Date: Fri, 12 Apr 2019 18:24:53 +0530 Message-Id: <20190412125453.26876-1-nusiddiq@redhat.com> In-Reply-To: <20190412125300.26536-1-nusiddiq@redhat.com> References: <20190412125300.26536-1-nusiddiq@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Fri, 12 Apr 2019 12:55:00 +0000 (UTC) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 4/6] ovn: Add a new OVN action 'icmp4_error' X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Numan Siddique This action is similar to the existing 'icmp4' OVN action except that that this action is expected to be used to generate an ICMPv4 packet in response to an error in original IP packet. When this action injects the icmpv4 packet, it also copies the original IP datagram following the icmp4 header as per RFC 1122: 3.2.2 Signed-off-by: Numan Siddique Acked-by: Mark Michelson --- include/ovn/actions.h | 7 ++++++ ovn/controller/pinctrl.c | 47 +++++++++++++++++++++++++++++++++++++-- ovn/lib/actions.c | 22 ++++++++++++++++++ ovn/ovn-sb.xml | 20 ++++++++++++----- ovn/utilities/ovn-trace.c | 5 +++++ tests/ovn.at | 15 +++++++++++++ 6 files changed, 109 insertions(+), 7 deletions(-) diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 89e28c50c..af8b0a4a0 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -65,6 +65,7 @@ struct ovn_extend_table; OVNACT(CLONE, ovnact_nest) \ OVNACT(ARP, ovnact_nest) \ OVNACT(ICMP4, ovnact_nest) \ + OVNACT(ICMP4_ERROR, ovnact_nest) \ OVNACT(ICMP6, ovnact_nest) \ OVNACT(TCP_RESET, ovnact_nest) \ OVNACT(ND_NA, ovnact_nest) \ @@ -471,6 +472,12 @@ enum action_opcode { /* MTU value (to put in the icmp4 header field - frag_mtu) follow the * action header. */ ACTION_OPCODE_PUT_ICMP4_FRAG_MTU, + + /* "icmp4_error { ...actions... }". + * + * The actions, in OpenFlow 1.3 format, follow the action_header. + */ + ACTION_OPCODE_ICMP4_ERROR, }; /* Header. */ diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 63202e6d4..2f22a57b2 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -531,7 +531,8 @@ pinctrl_handle_arp(struct rconn *swconn, const struct flow *ip_flow, 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) + const struct match *md, struct ofpbuf *userdata, + bool include_orig_ip_datagram) { /* This action only works for IP packets, and the switch should only send * us IP packets this way, but check here just to be sure. */ @@ -556,6 +557,16 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, eh->eth_src = ip_flow->dl_src; if (get_dl_type(ip_flow) == htons(ETH_TYPE_IP)) { + struct ip_header *in_ip = dp_packet_l3(pkt_in); + uint16_t in_ip_len = ntohs(in_ip->ip_tot_len); + if (in_ip_len < IP_HEADER_LEN) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL(&rl, + "ICMP action on IP packet with invalid length (%u)", + in_ip_len); + return; + } + struct ip_header *nh = dp_packet_put_zeros(&packet, sizeof *nh); eh->eth_type = htons(ETH_TYPE_IP); @@ -571,6 +582,33 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow, struct icmp_header *ih = dp_packet_put_zeros(&packet, sizeof *ih); dp_packet_set_l4(&packet, ih); packet_set_icmp(&packet, ICMP4_DST_UNREACH, 1); + + if (include_orig_ip_datagram) { + /* RFC 1122: 3.2.2 MUST send at least the IP header and 8 bytes + * of header. MAY send more. + * RFC says return as much as we can without exceeding 576 + * bytes. + * So, lets return as much as we can. */ + + /* Calculate available room to include the original IP + data. */ + nh = dp_packet_l3(&packet); + uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len)); + if (in_ip_len > room) { + in_ip_len = room; + } + dp_packet_put(&packet, in_ip, in_ip_len); + + /* dp_packet_put may reallocate the buffer. Get the l3 and l4 + * header pointers again. */ + nh = dp_packet_l3(&packet); + ih = dp_packet_l4(&packet); + uint16_t ip_total_len = ntohs(nh->ip_tot_len) + in_ip_len; + nh->ip_tot_len = htons(ip_total_len); + ih->icmp_csum = 0; + ih->icmp_csum = csum(ih, sizeof *ih + in_ip_len); + nh->ip_csum = 0; + nh->ip_csum = csum(nh, sizeof *nh); + } } else { struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh); struct icmp6_error_header *ih; @@ -1674,7 +1712,12 @@ 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); + &userdata, false); + break; + + case ACTION_OPCODE_ICMP4_ERROR: + pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, + &userdata, true); break; case ACTION_OPCODE_TCP_RESET: diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index e8940b091..f78e6ffcb 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -1153,6 +1153,12 @@ parse_ICMP4(struct action_context *ctx) parse_nested_action(ctx, OVNACT_ICMP4, "ip4"); } +static void +parse_ICMP4_ERROR(struct action_context *ctx) +{ + parse_nested_action(ctx, OVNACT_ICMP4_ERROR, "ip4"); +} + static void parse_ICMP6(struct action_context *ctx) { @@ -1210,6 +1216,12 @@ format_ICMP4(const struct ovnact_nest *nest, struct ds *s) format_nested_action(nest, "icmp4", s); } +static void +format_ICMP4_ERROR(const struct ovnact_nest *nest, struct ds *s) +{ + format_nested_action(nest, "icmp4_error", s); +} + static void format_ICMP6(const struct ovnact_nest *nest, struct ds *s) { @@ -1287,6 +1299,14 @@ encode_ICMP4(const struct ovnact_nest *on, encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts); } +static void +encode_ICMP4_ERROR(const struct ovnact_nest *on, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + encode_nested_actions(on, ep, ACTION_OPCODE_ICMP4_ERROR, ofpacts); +} + static void encode_ICMP6(const struct ovnact_nest *on, const struct ovnact_encode_params *ep, @@ -2412,6 +2432,8 @@ parse_action(struct action_context *ctx) parse_ARP(ctx); } else if (lexer_match_id(ctx->lexer, "icmp4")) { parse_ICMP4(ctx); + } else if (lexer_match_id(ctx->lexer, "icmp4_error")) { + parse_ICMP4_ERROR(ctx); } else if (lexer_match_id(ctx->lexer, "icmp6")) { parse_ICMP6(ctx); } else if (lexer_match_id(ctx->lexer, "tcp_reset")) { diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 4e080abff..0d35fe4fc 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -1820,19 +1820,22 @@
icmp4 { action; ... };
+
+ icmp4_error { action; ... }; +

Temporarily replaces the IPv4 packet being processed by an ICMPv4 packet and executes each nested action on the ICMPv4 - packet. Actions following the icmp4 action, if any, + packet. Actions following these actions, if any, apply to the original, unmodified packet.

- The ICMPv4 packet that this action operates on is initialized based - on the IPv4 packet being processed, as follows. These are default - values that the nested actions will probably want to change. - Ethernet and IPv4 fields not listed here are not changed: + The ICMPv4 packet that these actions operates on is initialized + based on the IPv4 packet being processed, as follows. These are + default values that the nested actions will probably want to + change. Ethernet and IPv4 fields not listed here are not changed:

    @@ -1843,6 +1846,13 @@
  • icmp4.code = 1 (host unreachable)
+

+ icmp4_error action is expected to be used to + generate an ICMPv4 packet in response to an error in original + IP packet. When this action generates the ICMPv4 packet, it + also copies the original IP datagram following the ICMPv4 header + as per RFC 1122: 3.2.2. +

Prerequisite: ip4

diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c index e03179c8f..28e2bb075 100644 --- a/ovn/utilities/ovn-trace.c +++ b/ovn/utilities/ovn-trace.c @@ -2116,6 +2116,11 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, super); break; + case OVNACT_ICMP4_ERROR: + execute_icmp4(ovnact_get_ICMP4_ERROR(a), dp, uflow, table_id, + pipeline, super); + break; + case OVNACT_ICMP6: execute_icmp6(ovnact_get_ICMP6(a), dp, uflow, table_id, pipeline, super); diff --git a/tests/ovn.at b/tests/ovn.at index 02ab94a87..92a6bed26 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1295,6 +1295,21 @@ icmp4 { eth.dst = ff:ff:ff:ff:ff:ff; icmp4.frag_mtu = 1500; output; }; output; encodes as controller(userdata=00.00.00.0a.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.28.00.00.23.20.00.25.00.00.00.00.00.00.00.03.00.0e.00.00.00.0d.00.00.00.00.05.dc.00.00.00.04.00.04.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64) has prereqs ip4 +# icmp4_error +icmp4_error { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output; + encodes as controller(userdata=00.00.00.0e.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64) + has prereqs ip4 + +icmp4_error { }; + formats as icmp4_error { drop; }; + encodes as controller(userdata=00.00.00.0e.00.00.00.00) + has prereqs ip4 + +# icmp4_error with icmp4.frag_mtu +icmp4_error { eth.dst = ff:ff:ff:ff:ff:ff; icmp4.frag_mtu = 1500; output; }; output; + encodes as controller(userdata=00.00.00.0e.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.28.00.00.23.20.00.25.00.00.00.00.00.00.00.03.00.0e.00.00.00.0d.00.00.00.00.05.dc.00.00.00.04.00.04.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64) + has prereqs ip4 + icmp4.frag_mtu = 1500; encodes as controller(userdata=00.00.00.0d.00.00.00.00.05.dc,pause)