[ovs-dev,1/2] OVN: add icmp6{} action support
diff mbox series

Message ID 29e273ef7f7dba71bb5ec51c2775712929b0767a.1523033721.git.lorenzo.bianconi@redhat.com
State Changes Requested
Headers show
Series
  • add icmp6 action support to ovn acl framework
Related show

Commit Message

Lorenzo Bianconi April 6, 2018, 5:05 p.m. UTC
icmp6 action is used to replace the IPv6 packet been processed with
an ICMPv6 packet initialized based on incoming IPv6 one.
Ethernet and IPv6 fields not listed are not changed:
- ip.proto = 58 (ICMPv6)
- ip.ttl = 255
- icmp6.type = 1 (destination unreachable)
- icmp6.code = 1 (communication administratively prohibited)
Prerequisite: ip6

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 include/ovn/actions.h     |  7 +++++
 ovn/controller/pinctrl.c  | 72 ++++++++++++++++++++++++++++++++++-------------
 ovn/lib/actions.c         | 22 +++++++++++++++
 ovn/ovn-sb.xml            | 26 +++++++++++++++++
 ovn/utilities/ovn-trace.c | 30 ++++++++++++++++++++
 tests/ovn.at              | 10 +++++++
 6 files changed, 147 insertions(+), 20 deletions(-)

Comments

Ben Pfaff April 6, 2018, 5:38 p.m. UTC | #1
On Fri, Apr 06, 2018 at 07:05:32PM +0200, Lorenzo Bianconi wrote:
> icmp6 action is used to replace the IPv6 packet been processed with
> an ICMPv6 packet initialized based on incoming IPv6 one.
> Ethernet and IPv6 fields not listed are not changed:
> - ip.proto = 58 (ICMPv6)
> - ip.ttl = 255
> - icmp6.type = 1 (destination unreachable)
> - icmp6.code = 1 (communication administratively prohibited)
> Prerequisite: ip6
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Thank you for implementing this.

It looks like ACTION_OPCODE_ICMP4 and ACTION_OPCODE_ICMP6 actually have
exactly the same implementation.  Maybe ACTION_OPCODE_ICMP4 could be
renamed ACTION_OPCODE_ICMP and ACTION_OPCODE_ICMP6 just omitted.

Thanks,

Ben.

Patch
diff mbox series

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 086ab19e0..10d5d6556 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(ICMP6,             ovnact_nest)            \
     OVNACT(TCP_RESET,         ovnact_nest)            \
     OVNACT(ND_NA,             ovnact_nest)            \
     OVNACT(GET_ARP,           ovnact_get_mac_bind)    \
@@ -443,6 +444,12 @@  enum action_opcode {
      * The actions, in OpenFlow 1.3 format, follow the action_header.
      */
     ACTION_OPCODE_TCP_RESET,
+
+    /* "icmp6 { ...actions... }".
+     *
+     * The actions, in OpenFlow 1.3 format, follow the action_header.
+     */
+    ACTION_OPCODE_ICMP6,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index c816b2dd6..55f58fe39 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -220,15 +220,16 @@  pinctrl_handle_arp(const struct flow *ip_flow, const struct match *md,
 }
 
 static void
-pinctrl_handle_icmp4(const struct flow *ip_flow, const struct match *md,
-                     struct ofpbuf *userdata)
+pinctrl_handle_icmp(const struct flow *ip_flow, struct dp_packet *pkt_in,
+                    const struct match *md, struct ofpbuf *userdata)
 {
     /* 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. */
-    if (ip_flow->dl_type != htons(ETH_TYPE_IP)) {
+    if (ip_flow->dl_type != htons(ETH_TYPE_IP) &&
+        ip_flow->dl_type != htons(ETH_TYPE_IPV6)) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_WARN_RL(&rl,
-                     "ICMP4 action on non-IP packet (eth_type 0x%"PRIx16")",
+                     "ICMP action on non-IP packet (eth_type 0x%"PRIx16")",
                      ntohs(ip_flow->dl_type));
         return;
     }
@@ -243,21 +244,50 @@  pinctrl_handle_icmp4(const struct flow *ip_flow, const struct match *md,
     struct eth_header *eh = dp_packet_put_zeros(&packet, sizeof *eh);
     eh->eth_dst = ip_flow->dl_dst;
     eh->eth_src = ip_flow->dl_src;
-    eh->eth_type = htons(ETH_TYPE_IP);
-
-    struct ip_header *nh = dp_packet_put_zeros(&packet, sizeof *nh);
-    dp_packet_set_l3(&packet, nh);
-    nh->ip_ihl_ver = IP_IHL_VER(5, 4);
-    nh->ip_tot_len = htons(sizeof(struct ip_header) +
-                           sizeof(struct icmp_header));
-    nh->ip_proto = IPPROTO_ICMP;
-    nh->ip_frag_off = htons(IP_DF);
-    packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst,
-                    ip_flow->nw_tos, 255);
-
-    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 (get_dl_type(ip_flow) == htons(ETH_TYPE_IP)) {
+        struct ip_header *nh = dp_packet_put_zeros(&packet, sizeof *nh);
+
+        eh->eth_type = htons(ETH_TYPE_IP);
+        dp_packet_set_l3(&packet, nh);
+        nh->ip_ihl_ver = IP_IHL_VER(5, 4);
+        nh->ip_tot_len = htons(sizeof(struct ip_header) +
+                               sizeof(struct icmp_header));
+        nh->ip_proto = IPPROTO_ICMP;
+        nh->ip_frag_off = htons(IP_DF);
+        packet_set_ipv4(&packet, ip_flow->nw_src, ip_flow->nw_dst,
+                        ip_flow->nw_tos, 255);
+
+        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);
+    } else {
+        struct ip6_hdr *nh = dp_packet_put_zeros(&packet, sizeof *nh);
+        struct icmp6_error_header *ih;
+        uint32_t icmpv6_csum;
+
+        eh->eth_type = htons(ETH_TYPE_IPV6);
+        dp_packet_set_l3(&packet, nh);
+        nh->ip6_vfc = 0x60;
+        nh->ip6_nxt = IPPROTO_ICMPV6;
+        nh->ip6_plen = htons(sizeof(*nh) + ICMP6_ERROR_HEADER_LEN);
+        packet_set_ipv6(&packet, &ip_flow->ipv6_src, &ip_flow->ipv6_dst,
+                        ip_flow->nw_tos, ip_flow->ipv6_label, 255);
+
+        ih = dp_packet_put_zeros(&packet, sizeof *ih);
+        dp_packet_set_l4(&packet, ih);
+        ih->icmp6_base.icmp6_type = ICMP6_DST_UNREACH;
+        ih->icmp6_base.icmp6_code = 1;
+        ih->icmp6_base.icmp6_cksum = 0;
+
+        uint8_t *data = dp_packet_put_zeros(&packet, sizeof *nh);
+        memcpy(data, dp_packet_l3(pkt_in), sizeof(*nh));
+
+        icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet));
+        ih->icmp6_base.icmp6_cksum = csum_finish(
+            csum_continue(icmpv6_csum, ih,
+                          sizeof(*nh) + ICMP6_ERROR_HEADER_LEN));
+    }
 
     if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) {
         eth_push_vlan(&packet, htons(ETH_TYPE_VLAN_8021Q),
@@ -1155,7 +1185,9 @@  process_packet_in(const struct ofp_header *msg, struct controller_ctx *ctx)
         break;
 
     case ACTION_OPCODE_ICMP4:
-        pinctrl_handle_icmp4(&headers, &pin.flow_metadata, &userdata);
+    case ACTION_OPCODE_ICMP6:
+        pinctrl_handle_icmp(&headers, &packet, &pin.flow_metadata,
+                            &userdata);
         break;
 
     case ACTION_OPCODE_TCP_RESET:
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 1e0bcb7b7..d90eb3d40 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1147,6 +1147,12 @@  parse_ICMP4(struct action_context *ctx)
     parse_nested_action(ctx, OVNACT_ICMP4, "ip4");
 }
 
+static void
+parse_ICMP6(struct action_context *ctx)
+{
+    parse_nested_action(ctx, OVNACT_ICMP6, "ip6");
+}
+
 static void
 parse_TCP_RESET(struct action_context *ctx)
 {
@@ -1192,6 +1198,12 @@  format_ICMP4(const struct ovnact_nest *nest, struct ds *s)
     format_nested_action(nest, "icmp4", s);
 }
 
+static void
+format_ICMP6(const struct ovnact_nest *nest, struct ds *s)
+{
+    format_nested_action(nest, "icmp6", s);
+}
+
 static void
 format_TCP_RESET(const struct ovnact_nest *nest, struct ds *s)
 {
@@ -1256,6 +1268,14 @@  encode_ICMP4(const struct ovnact_nest *on,
     encode_nested_actions(on, ep, ACTION_OPCODE_ICMP4, ofpacts);
 }
 
+static void
+encode_ICMP6(const struct ovnact_nest *on,
+             const struct ovnact_encode_params *ep,
+             struct ofpbuf *ofpacts)
+{
+    encode_nested_actions(on, ep, ACTION_OPCODE_ICMP6, ofpacts);
+}
+
 static void
 encode_TCP_RESET(const struct ovnact_nest *on,
                  const struct ovnact_encode_params *ep,
@@ -2289,6 +2309,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, "icmp6")) {
+        parse_ICMP6(ctx);
     } else if (lexer_match_id(ctx->lexer, "tcp_reset")) {
         parse_TCP_RESET(ctx);
     } else if (lexer_match_id(ctx->lexer, "nd_na")) {
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 23dcfb9cf..52750e192 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1733,6 +1733,32 @@ 
           <p><b>Prerequisite:</b> <code>ip4</code></p>
         </dd>
 
+        <dt><code>icmp6 { <var>action</var>; </code>...<code> };</code></dt>
+        <dd>
+          <p>
+            Temporarily replaces the IPv6 packet being processed by an ICMPv6
+            packet and executes each nested <var>action</var> on the ICMPv6
+            packet. Actions following the <var>icmp6</var> action, if any,
+            apply to the original, unmodified packet.
+          </p>
+
+          <p>
+            The ICMPv6 packet that this action operates on is initialized based
+            on the IPv6 packet being processed, as follows. These are default
+            values that the nested actions will probably want to change.
+            Ethernet and IPv6 fields not listed here are not changed:
+          </p>
+
+          <ul>
+            <li><code>ip.proto = 58</code> (ICMPv6)</li>
+            <li><code>ip.ttl = 255</code></li>
+            <li><code>icmp6.type = 1</code> (destination unreachable)</li>
+            <li><code>icmp6.code = 1</code> (administratively prohibited)</li>
+          </ul>
+
+          <p><b>Prerequisite:</b> <code>ip6</code></p>
+        </dd>
+
         <dt><code>tcp_reset;</code></dt>
         <dd>
           <p>
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 4db6f24f2..ac15d4e9e 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1560,6 +1560,31 @@  execute_icmp4(const struct ovnact_nest *on,
                   table_id, pipeline, &node->subs);
 }
 
+static void
+execute_icmp6(const struct ovnact_nest *on,
+              const struct ovntrace_datapath *dp,
+              const struct flow *uflow, uint8_t table_id,
+              enum ovnact_pipeline pipeline, struct ovs_list *super)
+{
+    struct flow icmp6_flow = *uflow;
+
+    /* Update fields for ICMPv6. */
+    icmp6_flow.dl_dst = uflow->dl_dst;
+    icmp6_flow.dl_src = uflow->dl_src;
+    icmp6_flow.ipv6_dst = uflow->ipv6_dst;
+    icmp6_flow.ipv6_src = uflow->ipv6_src;
+    icmp6_flow.nw_proto = IPPROTO_ICMPV6;
+    icmp6_flow.nw_ttl = 255;
+    icmp6_flow.tp_src = htons(ICMP6_DST_UNREACH); /* icmp type */
+    icmp6_flow.tp_dst = htons(1); /* icmp code */
+
+    struct ovntrace_node *node = ovntrace_node_append(
+        super, OVNTRACE_NODE_TRANSFORMATION, "icmp6");
+
+    trace_actions(on->nested, on->nested_len, dp, &icmp6_flow,
+                  table_id, pipeline, &node->subs);
+}
+
 static void
 execute_tcp_reset(const struct ovnact_nest *on,
                   const struct ovntrace_datapath *dp,
@@ -1950,6 +1975,11 @@  trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
                           super);
             break;
 
+        case OVNACT_ICMP6:
+            execute_icmp6(ovnact_get_ICMP6(a), dp, uflow, table_id, pipeline,
+                          super);
+            break;
+
         case OVNACT_TCP_RESET:
             execute_tcp_reset(ovnact_get_TCP_RESET(a), dp, uflow, table_id,
                               pipeline, super);
diff --git a/tests/ovn.at b/tests/ovn.at
index b872a1b7e..0b23e3bcc 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1135,6 +1135,16 @@  icmp4 { };
     encodes as controller(userdata=00.00.00.0a.00.00.00.00)
     has prereqs ip4
 
+# icmp6
+icmp6 { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
+    encodes as controller(userdata=00.00.00.0c.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 ip6
+
+icmp6 { };
+    formats as icmp6 { drop; };
+    encodes as controller(userdata=00.00.00.0c.00.00.00.00)
+    has prereqs ip6
+
 # tcp_reset
 tcp_reset { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
     encodes as controller(userdata=00.00.00.0b.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)