[ovs-dev] [PATCH] [PATCH, v7] ovn: Add 'na' action and lflow for ND
Ben Pfaff
blp at ovn.org
Sat Jul 2 18:43:41 UTC 2016
On Mon, Jun 27, 2016 at 02:54:52PM +0800, Zong Kai LI wrote:
> This patch tries to support ND versus ARP for OVN.
>
> It adds a new OVN action 'na' in ovn-controller side, and modify lflows
> for 'na' action and relevant packets in ovn-northd.
>
> First, for ovn-northd, it will generate lflows per each lport with its
> IPv6 addresses and mac addresss, with 'na' action, such as:
> match=(icmp6 && icmp6.type == 135 &&
> (nd.target == fd81:ce49:a948:0:f816:3eff:fe46:8a42 ||
> nd.target == fd81:ce49:b123:0:f816:3eff:fe46:8a42)),
> action=(na { eth.src = fa:16:3e:46:8a:42; nd.tll = fa:16:3e:46:8a:42;
> outport = inport;
> inport = ""; /* Allow sending out inport. */ output; };)
>
> and new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
> from previous ls_in_arp_rsp.
>
> Later, for ovn-controller, when it received a ND packet, it frames a
> template NA packet for reply. The NA packet will be initialized based on
> ND packet, such as NA packet will use:
> - ND packet eth.src as eth.dst,
> - ND packet eth.dst as eth.src,
> - ND packet ip6.src as ip6.dst,
> - ND packet nd.target as ip6.src,
> - ND packet eth.dst as nd.tll.
>
> Finally, nested actions in 'na' action will update necessary fileds
> for NA packet, such as:
> - eth.src, nd.tll
> - inport, outport
>
> Since patch port for IPv6 router interface is not ready yet, this
> patch will only try to deal with ND from VM. This patch will set
> RSO flags to 011 for NA packets.
>
> This patch also modified current ACL lflows for ND, not to do conntrack
> on ND and NA packets in following tables:
> - S_SWITCH_IN_PRE_ACL
> - S_SWITCH_OUT_PRE_ACL
> - S_SWITCH_IN_ACL
> - S_SWITCH_OUT_ACL
>
> (Rebase on upstream)
>
> Signed-off-by: Zong Kai LI <zealokii at gmail.com>
Thanks for the patch!
I made some minor simplifications and improvements. Most notably, one
of the tests didn't pass because the wrong opcode was being used (2
instead of 3); I fixed that. I'm appending an incremental diff below.
I also added a preparatory patch just before it. I'll send the
resulting two-patch series separately.
I applied both patches to master.
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index f7edfd3..f960ea9 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -25,7 +25,6 @@
#include "lport.h"
#include "nx-match.h"
#include "ovn-controller.h"
-#include "lib/byte-order.h"
#include "lib/packets.h"
#include "lib/sset.h"
#include "openvswitch/ofp-actions.h"
@@ -943,12 +942,11 @@ pinctrl_handle_na(const struct flow *ip_flow,
const struct match *md,
struct ofpbuf *userdata)
{
- /* This action only works for IPv6 packets, and the switch should only send
- * us IPv6 packets this way, but check here just to be sure. */
- if (ip_flow->dl_type != htons(ETH_TYPE_IPV6)) {
+ /* This action only works for IPv6 ND packets, and the switch should only
+ * send us ND packets this way, but check here just to be sure. */
+ if (!is_nd(ip_flow, NULL)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
- VLOG_WARN_RL(&rl, "NA action on non-IPv6 packet (Ethertype %"PRIx16")",
- ntohs(ip_flow->dl_type));
+ VLOG_WARN_RL(&rl, "NA action on non-ND packet");
return;
}
@@ -958,17 +956,11 @@ pinctrl_handle_na(const struct flow *ip_flow,
uint64_t packet_stub[128 / 8];
struct dp_packet packet;
dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
+
ovs_be32 ipv6_src[4], ipv6_dst[4];
- for (int i = 0; i < 4; i++) {
- ipv6_dst[i] = BYTES_TO_BE32(ip_flow->ipv6_src.s6_addr[4*i],
- ip_flow->ipv6_src.s6_addr[4*i+1],
- ip_flow->ipv6_src.s6_addr[4*i+2],
- ip_flow->ipv6_src.s6_addr[4*i+3]);
- ipv6_src[i] = BYTES_TO_BE32(ip_flow->nd_target.s6_addr[4*i],
- ip_flow->nd_target.s6_addr[4*i+1],
- ip_flow->nd_target.s6_addr[4*i+2],
- ip_flow->nd_target.s6_addr[4*i+3]);
- }
+ memcpy(ipv6_dst, &ip_flow->ipv6_src, sizeof ipv6_src);
+ memcpy(ipv6_src, &ip_flow->nd_target, sizeof ipv6_dst);
+
/* Frame the NA packet with RSO=011. */
compose_na(&packet,
ip_flow->dl_dst, ip_flow->dl_src,
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 5ec73be..550bb87 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -246,8 +246,11 @@ put_controller_op(struct ofpbuf *ofpacts, enum action_opcode opcode)
finish_controller_op(ofpacts, ofs);
}
+/* Implements the "arp" and "na" actions, which execute nested actions on a
+ * packet derived from the one being processed. */
static void
-parse_arp_action(struct action_context *ctx)
+parse_nested_action(struct action_context *ctx, enum action_opcode opcode,
+ const char *prereq)
{
if (!lexer_match(ctx->lexer, LEX_T_LCURLY)) {
action_syntax_error(ctx, "expecting `{'");
@@ -272,13 +275,11 @@ parse_arp_action(struct action_context *ctx)
ctx->ofpacts = outer_ofpacts;
- /* Add a "controller" action with the actions nested inside "arp {...}",
+ /* Add a "controller" action with the actions nested inside "{...}",
* converted to OpenFlow, as its userdata. ovn-controller will convert the
- * packet to an ARP and then send the packet and actions back to the switch
- * inside an OFPT_PACKET_OUT message. */
- /* controller. */
- size_t oc_offset = start_controller_op(ctx->ofpacts, ACTION_OPCODE_ARP,
- false);
+ * packet to ARP or NA and then send the packet and actions back to the
+ * switch inside an OFPT_PACKET_OUT message. */
+ size_t oc_offset = start_controller_op(ctx->ofpacts, opcode, false);
ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
ctx->ofpacts, OFP13_VERSION);
finish_controller_op(ctx->ofpacts, oc_offset);
@@ -286,7 +287,7 @@ parse_arp_action(struct action_context *ctx)
/* Restore prerequisites. */
expr_destroy(ctx->prereqs);
ctx->prereqs = outer_prereqs;
- add_prerequisite(ctx, "ip4");
+ add_prerequisite(ctx, prereq);
/* Free memory. */
ofpbuf_uninit(&inner_ofpacts);
@@ -876,51 +877,6 @@ parse_ct_nat(struct action_context *ctx, bool snat)
ofpbuf_push_uninit(ctx->ofpacts, ct_offset);
}
-static void
-parse_na_action(struct action_context *ctx)
-{
- if (!lexer_match(ctx->lexer, LEX_T_LCURLY)) {
- action_syntax_error(ctx, "expecting `{'");
- return;
- }
-
- struct ofpbuf *outer_ofpacts = ctx->ofpacts;
- uint64_t inner_ofpacts_stub[1024 / 8];
- struct ofpbuf inner_ofpacts = OFPBUF_STUB_INITIALIZER(inner_ofpacts_stub);
- ctx->ofpacts = &inner_ofpacts;
-
- /* Save prerequisites. (XXX What is the right treatment for prereqs?) */
- struct expr *outer_prereqs = ctx->prereqs;
- ctx->prereqs = NULL;
-
- /* Parse inner actions. */
- while (!lexer_match(ctx->lexer, LEX_T_RCURLY)) {
- if (!parse_action(ctx)) {
- break;
- }
- }
-
- ctx->ofpacts = outer_ofpacts;
-
- /* Add a "controller" action with the actions nested inside "na {...}",
- * converted to OpenFlow, as its userdata. ovn-controller will convert the
- * packet to an NA and then send the packet and actions back to the switch
- * inside an OFPT_PACKET_OUT message. */
- size_t oc_offset = start_controller_op(ctx->ofpacts, ACTION_OPCODE_NA,
- true);
- ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
- ctx->ofpacts, OFP13_VERSION);
- finish_controller_op(ctx->ofpacts, oc_offset);
-
- /* Restore prerequisites. */
- expr_destroy(ctx->prereqs);
- ctx->prereqs = outer_prereqs;
- add_prerequisite(ctx, "nd");
-
- /* Free memory. */
- ofpbuf_uninit(&inner_ofpacts);
-}
-
static bool
parse_action(struct action_context *ctx)
{
@@ -953,13 +909,13 @@ parse_action(struct action_context *ctx)
} else if (lexer_match_id(ctx->lexer, "ct_snat")) {
parse_ct_nat(ctx, true);
} else if (lexer_match_id(ctx->lexer, "arp")) {
- parse_arp_action(ctx);
+ parse_nested_action(ctx, ACTION_OPCODE_ARP, "ip4");
+ } else if (lexer_match_id(ctx->lexer, "na")) {
+ parse_nested_action(ctx, ACTION_OPCODE_NA, "nd");
} else if (lexer_match_id(ctx->lexer, "get_arp")) {
parse_get_arp_action(ctx);
} else if (lexer_match_id(ctx->lexer, "put_arp")) {
parse_put_arp_action(ctx);
- } else if (lexer_match_id(ctx->lexer, "na")) {
- parse_na_action(ctx);
} else {
action_syntax_error(ctx, "expecting action");
}
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index dcf98e5..ebc5cf0 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1048,10 +1048,11 @@
<dd>
<p>
- Temporarily replaces the IPv6 packet being processed by an NA
- packet and executes each nested <var>action</var> on the NA
- packet. Actions following the <var>na</var> action, if any, apply
- to the original, unmodified packet.
+ Temporarily replaces the IPv6 packet being processed by an IPv6
+ neighbor advertisement (NA) packet and executes each nested
+ <var>action</var> on the NA packet. Actions following the
+ <var>na</var> action, if any, apply to the original, unmodified
+ packet.
</p>
<p>
diff --git a/tests/ovn.at b/tests/ovn.at
index fb1d6f1..778cac7 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -574,7 +574,7 @@ reg1[0] = put_dhcp_opts(offerip="xyzzy"); => DHCP option offerip requires numeri
reg1[0] = put_dhcp_opts(offerip=1.2.3.4, domain=1.2.3.4); => DHCP option domain requires string value.
# na
-na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport = inport; inport = ""; /* Allow sending out inport. */ output; }; => actions=controller(userdata=00.00.00.02.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.0c.04.00.01.0e.04.00.19.00.10.00.01.0c.04.00.00.00.00.00.00.00.00.00.19.00.10.00.00.00.02.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00), prereqs=nd
+na { eth.src = 12:34:56:78:9a:bc; nd.tll = 12:34:56:78:9a:bc; outport = inport; inport = ""; /* Allow sending out inport. */ output; }; => actions=controller(userdata=00.00.00.03.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.10.80.00.42.06.12.34.56.78.9a.bc.00.00.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.0c.04.00.01.0e.04.00.19.00.10.00.01.0c.04.00.00.00.00.00.00.00.00.00.19.00.10.00.00.00.02.00.00.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00), prereqs=nd
# Contradictionary prerequisites (allowed but not useful):
ip4.src = ip6.src[0..31]; => actions=move:NXM_NX_IPV6_SRC[0..31]->NXM_OF_IP_SRC[], prereqs=eth.type == 0x800 && eth.type == 0x86dd
More information about the dev
mailing list