Skip to content

Commit

Permalink
ovn: Add a new OVN action 'icmp4_error'
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Acked-by: Mark Michelson <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
@numansiddique @blp
numansiddique authored and blp committed Apr 22, 2019
1 parent 086470c commit 5e35f78
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 7 deletions.
7 changes: 7 additions & 0 deletions include/ovn/actions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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. */
Expand Down
47 changes: 45 additions & 2 deletions ovn/controller/pinctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,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. */
Expand All @@ -555,6 +556,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);
Expand All @@ -570,6 +581,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;
Expand Down Expand Up @@ -1673,7 +1711,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:
Expand Down
22 changes: 22 additions & 0 deletions ovn/lib/actions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")) {
Expand Down
20 changes: 15 additions & 5 deletions ovn/ovn-sb.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1853,19 +1853,22 @@

<dl>
<dt><code>icmp4 { <var>action</var>; </code>...<code> };</code></dt>
<dt>
<code>icmp4_error { <var>action</var>; </code>...<code> };</code>
</dt>
<dd>
<p>
Temporarily replaces the IPv4 packet being processed by an ICMPv4
packet and executes each nested <var>action</var> on the ICMPv4
packet. Actions following the <var>icmp4</var> action, if any,
packet. Actions following these actions, if any,
apply to the original, unmodified packet.
</p>

<p>
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:
</p>

<ul>
Expand All @@ -1876,6 +1879,13 @@
<li><code>icmp4.code = 1</code> (host unreachable)</li>
</ul>

<p>
<code>icmp4_error</code> 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.
</p>
<p><b>Prerequisite:</b> <code>ip4</code></p>
</dd>

Expand Down
5 changes: 5 additions & 0 deletions ovn/utilities/ovn-trace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions tests/ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 5e35f78

Please sign in to comment.