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 <zealo...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev