Re: [ovs-dev] [PATCH 05/10] actions: Add new OVN action "clone".

2017-01-20 Thread Ben Pfaff
On Fri, Jan 20, 2017 at 11:17:53AM -0800, Mickey Spiegel wrote:
> On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:
> 
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Mickey Spiegel 
> 
> One comment below, found a copy/paste error in ovn-sb.xml.

Thanks, I fixed it in my copy here.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH 05/10] actions: Add new OVN action "clone".

2017-01-20 Thread Mickey Spiegel
On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:

> Signed-off-by: Ben Pfaff 
>

Acked-by: Mickey Spiegel 

One comment below, found a copy/paste error in ovn-sb.xml.

---
>  include/ovn/actions.h |  5 ++--
>  ovn/lib/actions.c | 61 ++
> ++---
>  ovn/ovn-sb.xml| 10 
>  ovn/utilities/ovn-trace.c | 21 +++-
>  tests/ovn.at  |  5 
>  5 files changed, 85 insertions(+), 17 deletions(-)
>




>
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index 5704f41..bca82e0 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1137,6 +1137,16 @@
>
>  
>
> +
> +clone { action; ...
> };
> +
> +  Makes a copy of the packet being processed and executes each
> +  action on the copy.  Actions following the
> +  arp action, if any, apply to the original, unmodified
>

s/arp/clone

Mickey

+  packet.  This can be used as a way to ``save and restore'' the
> packet
> +  around a set of actions that may modify it and should not
> persist.
> +
> +
>  arp { action; ... };
>  
>
>


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH 05/10] actions: Add new OVN action "clone".

2017-01-20 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h |  5 ++--
 ovn/lib/actions.c | 61 ---
 ovn/ovn-sb.xml| 10 
 ovn/utilities/ovn-trace.c | 21 +++-
 tests/ovn.at  |  5 
 5 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0bf6145..a1b6b90 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,6 +60,7 @@ struct simap;
 OVNACT(CT_DNAT,   ovnact_ct_nat)\
 OVNACT(CT_SNAT,   ovnact_ct_nat)\
 OVNACT(CT_LB, ovnact_ct_lb) \
+OVNACT(CLONE, ovnact_nest)  \
 OVNACT(ARP,   ovnact_nest)  \
 OVNACT(ND_NA, ovnact_nest)  \
 OVNACT(GET_ARP,   ovnact_get_mac_bind)  \
@@ -186,7 +187,7 @@ struct ovnact_ct_lb {
 uint8_t ltable; /* Logical table ID of next table. */
 };
 
-/* OVNACT_ARP, OVNACT_ND_NA. */
+/* OVNACT_ARP, OVNACT_ND_NA, OVNACT_CLONE. */
 struct ovnact_nest {
 struct ovnact ovnact;
 struct ovnact *nested;
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 7c5a292..213e22e 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1001,8 +1001,8 @@ ovnact_ct_lb_free(struct ovnact_ct_lb *ct_lb)
 free(ct_lb->dsts);
 }
 
-/* Implements the "arp" and "nd_na" actions, which execute nested actions on a
- * packet derived from the one being processed. */
+/* Implements the "arp", "nd_na", and "clone" actions, which execute nested
+ * actions on a packet derived from the one being processed. */
 static void
 parse_nested_action(struct action_context *ctx, enum ovnact_type type,
 const char *prereq)
@@ -1018,13 +1018,21 @@ parse_nested_action(struct action_context *ctx, enum 
ovnact_type type,
 .pp = ctx->pp,
 .lexer = ctx->lexer,
 .ovnacts = ,
-.prereqs = NULL
+.prereqs = NULL,
 };
 parse_actions(_ctx, LEX_T_RCURLY);
 
-/* XXX Not really sure what we should do with prerequisites for nested
- * actions. */
-expr_destroy(inner_ctx.prereqs);
+if (prereq) {
+/* XXX Not really sure what we should do with prerequisites for "arp"
+ * and "nd_na" actions. */
+expr_destroy(inner_ctx.prereqs);
+add_prerequisite(ctx, prereq);
+} else {
+/* For "clone", the inner prerequisites should just add to the outer
+ * ones. */
+ctx->prereqs = expr_combine(EXPR_T_AND,
+inner_ctx.prereqs, ctx->prereqs);
+}
 
 if (inner_ctx.lexer->error) {
 ovnacts_free(nested.data, nested.size);
@@ -1032,8 +1040,6 @@ parse_nested_action(struct action_context *ctx, enum 
ovnact_type type,
 return;
 }
 
-add_prerequisite(ctx, prereq);
-
 struct ovnact_nest *on = ovnact_put(ctx->ovnacts, type,
 OVNACT_ALIGN(sizeof *on));
 on->nested_len = nested.size;
@@ -1053,6 +1059,12 @@ parse_ND_NA(struct action_context *ctx)
 }
 
 static void
+parse_CLONE(struct action_context *ctx)
+{
+parse_nested_action(ctx, OVNACT_CLONE, NULL);
+}
+
+static void
 format_nested_action(const struct ovnact_nest *on, const char *name,
  struct ds *s)
 {
@@ -1074,10 +1086,16 @@ format_ND_NA(const struct ovnact_nest *nest, struct ds 
*s)
 }
 
 static void
-encode_nested_actions(const struct ovnact_nest *on,
-  const struct ovnact_encode_params *ep,
-  enum action_opcode opcode,
-  struct ofpbuf *ofpacts)
+format_CLONE(const struct ovnact_nest *nest, struct ds *s)
+{
+format_nested_action(nest, "clone", s);
+}
+
+static void
+encode_nested_neighbor_actions(const struct ovnact_nest *on,
+   const struct ovnact_encode_params *ep,
+   enum action_opcode opcode,
+   struct ofpbuf *ofpacts)
 {
 /* Convert nested actions into ofpacts. */
 uint64_t inner_ofpacts_stub[1024 / 8];
@@ -1102,7 +1120,7 @@ encode_ARP(const struct ovnact_nest *on,
const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
 {
-encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
+encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
 }
 
 static void
@@ -1110,9 +1128,22 @@ encode_ND_NA(const struct ovnact_nest *on,
  const struct ovnact_encode_params *ep,
  struct ofpbuf *ofpacts)
 {
-encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts);
+encode_nested_neighbor_actions(on, ep,