Re: [ovs-dev] [PATCH v2] dpif-netdev: Change definitions of 'idle' & 'processing' cycles

[Daniele Di Proietto]

2017-01-20 5:59 GMT-08:00 Jan Scheurich :
>
>
> On 2017-01-18 17:32, Kevin Traynor wrote:
>>
>> On 01/18/2017 01:34 AM, Daniele Di Proietto wrote:
>>>
>>> 2017-01-17 11:43 GMT-08:00 Kevin Traynor :

 On 01/17/2017 05:43 PM, Ciara Loftus wrote:
>
> Instead of counting all polling cycles as processing cycles, only count
> the cycles where packets were received from the polling.

 This makes these stats much clearer. One minor comment below, other than
 that

 Acked-by: Kevin Traynor 

> Signed-off-by: Georg Schmuecking 
> Signed-off-by: Ciara Loftus 
> Co-authored-by: Ciara Loftus 
>>>
>>> Minor: the co-authored-by tag should be different from the main author.
>>>
>>> This makes it easier to understand how busy a pmd thread is, a valid
>>> question
>>> that a sysadmin might have.
>>>
>>> The counters were originally introduced to help developers understand how
>>> cycles
>>> are spent between drivers(netdev rx) and datapath processing(dpif).
>>> Do you think
>>> it's ok to lose this type of information?  Perhaps it is, since a
>>> developer can also
>>> use a profiler, I'm not sure.
>>>
>>> Maybe we could 'last_cycles' as it is and introduce a separate counter to
>>> get
>>> the idle/busy ratio.  I'm not 100% sure this is the best way.
>>>
>>> What do you guys think?
>>>
>> I've only ever used the current stats for trying to estimate if polling
>> was getting packets or not, so the addition of an idle stat helps that.
>> I like your suggestion of having all three stats, so then it would be
>> something like:
>>
>> polling unsuccessful (idle)
>> polling successful (got pkts)
>> processing pkts
>>
>> That would keep the info for a developer and it could help initial debug
>> if pkt rates drop on a pmd.
>>
>> Kevin.
>
>
> From an operational perspective, the most important data is clearly the
> fraction of busy cycles. Any additional breakdown of busy cycles is
> debatable. We have always been wondering why Rx cost was accounted for
> separately in the current code, while Tx cost was included in the
> processing. That didn't make much sense to us.
>
> A developer should be able to split the busy cycles between Rx polling,
> processing (parsing, EMC lookup, dplcs lookup, upcall(!), actions) and Tx to
> port by analysing "perf top" output, as we have done in the analysis for our
> performance patches, or using a fancier profiler.

Thanks Kevin and Jan, based on the above discussion I think we can remove
the distinction between successfully polling and processing, meaning that the
patch is good.

Since we want to expose this the user rather than the developer, I think that
the documentation in vswitchd/ovs-vswitchd.8.in should explain the meaning
of idle cycles and processing cycles.

>
> One additional metric that would be interesting to see in pmd_stats_show,
> however, is the average number of packets per batch polled from a port (or
> recirculated).

Good point.  Maybe we can address this in a separate patch?

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

[ovs-dev] [userspace clone v3 4/4] xlate: Generate of datapath clone action when supported

[Andy Zhou]

Add logic to detect whether datapath support clone.
Enhance the xlate logic to make use of it.
Added logic to turn on/off clone support for testing.

Signed-off-by: Andy Zhou 
Acked-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c | 38 --
 ofproto/ofproto-dpif-xlate.h |  2 ++
 ofproto/ofproto-dpif.c   | 23 +++
 tests/ofproto-dpif.at| 11 +++
 4 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9a15ec3..0513394 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4520,9 +4520,23 @@ xlate_sample_action(struct xlate_ctx *ctx,
   tunnel_out_port, false);
 }
 
+/* Only called if the datapath supports 'OVS_ACTION_ATTR_CLONE'.
+ *
+ * Translates 'oc' within OVS_ACTION_ATTR_CLONE. */
 static void
 compose_clone_action(struct xlate_ctx *ctx, const struct ofpact_nest *oc)
 {
+size_t clone_offset = nl_msg_start_nested(ctx->odp_actions,
+  OVS_ACTION_ATTR_CLONE);
+
+do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
+
+nl_msg_end_non_empty_nested(ctx->odp_actions, clone_offset);
+}
+
+static void
+xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc)
+{
 bool old_was_mpls = ctx->was_mpls;
 bool old_conntracked = ctx->conntracked;
 struct flow old_flow = ctx->xin->flow;
@@ -4537,7 +4551,16 @@ compose_clone_action(struct xlate_ctx *ctx, const struct 
ofpact_nest *oc)
 ofpbuf_use_stub(>action_set, actset_stub, sizeof actset_stub);
 ofpbuf_put(>action_set, old_action_set.data, old_action_set.size);
 
-do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
+if (ctx->xbridge->support.clone) {
+/* Datapath clone action will make sure the pre clone packets
+ * are used for actions after clone. Save and restore
+ * ctx->base_flow to reflect this for the openflow pipeline. */
+struct flow old_base_flow = ctx->base_flow;
+compose_clone_action(ctx, oc);
+ctx->base_flow = old_base_flow;
+} else {
+do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
+}
 
 ofpbuf_uninit(>action_set);
 ctx->action_set = old_action_set;
@@ -5383,7 +5406,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t 
ofpacts_len,
 break;
 
 case OFPACT_CLONE:
-compose_clone_action(ctx, ofpact_get_CLONE(a));
+xlate_clone(ctx, ofpact_get_CLONE(a));
 break;
 
 case OFPACT_CT:
@@ -6169,3 +6192,14 @@ xlate_mac_learning_update(const struct ofproto_dpif 
*ofproto,
 
 update_learning_table__(xbridge, xbundle, dl_src, vlan, is_grat_arp);
 }
+
+void
+xlate_disable_dp_clone(const struct ofproto_dpif *ofproto)
+{
+struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, );
+struct xbridge *xbridge = xbridge_lookup(xcfg, ofproto);
+
+if (xbridge) {
+xbridge->support.clone = false;
+}
+}
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 5d00d6d..3986f26 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -206,6 +206,8 @@ void xlate_mac_learning_update(const struct ofproto_dpif 
*ofproto,
ofp_port_t in_port, struct eth_addr dl_src,
int vlan, bool is_grat_arp);
 
+void xlate_disable_dp_clone(const struct ofproto_dpif *);
+
 void xlate_txn_start(void);
 void xlate_txn_commit(void);
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index df291f3..e1112eb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5025,6 +5025,26 @@ disable_datapath_truncate(struct unixctl_conn *conn 
OVS_UNUSED,
 }
 
 static void
+disable_datapath_clone(struct unixctl_conn *conn OVS_UNUSED,
+   int argc, const char *argv[],
+   void *aux OVS_UNUSED)
+{
+struct ds ds = DS_EMPTY_INITIALIZER;
+const char *br = argv[argc -1];
+struct ofproto_dpif *ofproto;
+
+ofproto = ofproto_dpif_lookup(br);
+if (!ofproto) {
+unixctl_command_reply_error(conn, "no such bridge");
+return;
+}
+xlate_disable_dp_clone(ofproto);
+udpif_flush(ofproto->backer->udpif);
+ds_put_format(, "Datapath clone action disabled for bridge %s", br);
+unixctl_command_reply(conn, ds_cstr());
+}
+
+static void
 ofproto_unixctl_init(void)
 {
 static bool registered;
@@ -5053,6 +5073,9 @@ ofproto_unixctl_init(void)
 
 unixctl_command_register("dpif/disable-truncate", "", 0, 0,
  disable_datapath_truncate, NULL);
+
+unixctl_command_register("dpif/disable-dp-clone", "bridge", 1, 1,
+ disable_datapath_clone, NULL);
 }
 
 static odp_port_t
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at

[ovs-dev] [userspace clone v3 3/4] dpif-netdev: Add clone action

[Andy Zhou]

Add support for userspace datapath clone action.  The clone action
provides an action envelope to enclose an action list.
For example, with actions A, B, C and D,  and an action list:
  A, clone(B, C), D

The clone action will ensure that:

- D will see the same packet, and any meta states, such as flow, as
  action B.

- D will be executed regardless whether B, or C drops a packet. They
  can only drop a clone.

- When B drops a packet, clone will skip all remaining actions
  within the clone envelope. This feature is useful when we add
  meter action later:  The meter action can be implemented as a
  simple action without its own envolop (unlike the sample action).
  When necessary, the flow translation layer can enclose a meter action
  in clone.

The clone action is very similar with the OpenFlow clone action.
This is by design to simplify vswitchd flow translation logic.

Without datapath clone, vswitchd simulate the effect by inserting
datapath actions to "undo" clone actions. The above flow will be
translated into   A, B, C, -C, -B, D.

However, there are two issues:
- The resulting datapath action list may be longer without using
  clone.

- Some actions, such as NAT may not be possible to reverse.

This patch implements clone() simply with packet copy. The performance
can be improved with later patches, for example, to delay or avoid
packet copy if possible.  It seems datapath should have enough context
to carry out such optimization without the userspace context.

Signed-off-by: Andy Zhou 
---
 datapath/linux/compat/include/linux/openvswitch.h |  3 +-
 lib/dpif-netdev.c |  1 +
 lib/dpif.c|  1 +
 lib/odp-execute.c | 34 ++
 lib/odp-util.c| 16 +++
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif.c| 55 +++
 ofproto/ofproto-dpif.h|  5 ++-
 8 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8..425d3a4 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007-2014 Nicira, Inc.
+ * Copyright (c) 2007-2017 Nicira, Inc.
  *
  * This file is offered under your choice of two licenses: Apache 2.0 or GNU
  * GPL 2.0 or later.  The permission statements for each of these licenses is
@@ -818,6 +818,7 @@ enum ovs_action_attr {
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*.  */
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9c21af3..42631bc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4731,6 +4731,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_ACTION_ATTR_HASH:
 case OVS_ACTION_ATTR_UNSPEC:
 case OVS_ACTION_ATTR_TRUNC:
+case OVS_ACTION_ATTR_CLONE:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
 }
diff --git a/lib/dpif.c b/lib/dpif.c
index 50c3382..7c953b5 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1182,6 +1182,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_ACTION_ATTR_SET_MASKED:
 case OVS_ACTION_ATTR_SAMPLE:
 case OVS_ACTION_ATTR_TRUNC:
+case OVS_ACTION_ATTR_CLONE:
 case OVS_ACTION_ATTR_UNSPEC:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 73e1016..763735a 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -528,6 +528,26 @@ odp_execute_sample(void *dp, struct dp_packet *packet, 
bool steal,
 nl_attr_get_size(subactions), dp_execute_action);
 }
 
+static void
+odp_execute_clone(void *dp, struct dp_packet *packet, bool steal,
+   const struct nlattr *actions,
+   odp_execute_cb dp_execute_action)
+{
+struct dp_packet_batch pb;
+
+if (!steal) {
+/* The 'actions' may modify the packet, but the modification
+ * should not propagate beyond this clone action. Make a copy
+ * the packet in case we don't own the packet, so that the
+ * 'actions' are only applid to the clone.  'odp_execute_actions'
+ * will free the clone.  */
+packet = dp_packet_clone(packet);
+}
+packet_batch_init_packet(, packet);
+odp_execute_actions(dp, , true, nl_attr_get(actions),
+nl_attr_get_size(actions), dp_execute_action);
+}
+
 static bool
 

Re: [ovs-dev] [PATCH v2 6/7] actions: Make "next" action able to jump from egress to ingress pipeline.

[Mickey Spiegel]

On Fri, Jan 20, 2017 at 2:48 PM, Ben Pfaff  wrote:

> This feature is useful for centralized gateways.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Mickey Spiegel 

I think there is some missing functionality in ovn-trace.c.
It looks to me like ovn-trace.c assumes that "next" actions
always go to the next table, i.e. it ignores "(3)" or
"(pipeline=ingress, table=3)".  For my particular usage,
egress loopback will only happen after NAT, so the trace
will never reach the "next(pipeline=ingress, table=0)"
action.

Mickey


> ---
>  include/ovn/actions.h | 63 --
>  ovn/controller/lflow.c|  7 +++--
>  ovn/lib/actions.c | 70 ++
> ++---
>  ovn/ovn-sb.xml| 12 ++--
>  ovn/utilities/ovn-trace.c |  3 ++
>  tests/ovn.at  | 22 ++-
>  tests/test-ovn.c  |  6 ++--
>  7 files changed, 138 insertions(+), 45 deletions(-)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 4/7] actions: Omit table number when possible for formatting "next" action.

[Mickey Spiegel]

On Fri, Jan 20, 2017 at 2:48 PM, Ben Pfaff  wrote:

> Until now, formatting the "next" action has always required including
> the table number, because the action struct didn't include enough context
> so that the formatter could decide whether the table number was the next
> table or some other table.  This is more or less OK, but an upcoming commit
> will add a "pipeline" field to the "next" action, which means that the same
> policy there would require that the pipeline always be printed.  That's a
> little obnoxious because 99+% of the time, the pipeline to be printed is
> the same pipeline that the flow is in and printing it would be distracting.
> So it's better to store some context to help with formatting.  This commit
> begins adopting that policy for the existing table number field.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Mickey Spiegel 

One comment inline.


> ---
>  include/ovn/actions.h |  8 
>  ovn/lib/actions.c | 43 +--
>  tests/ovn.at  |  8 
>  3 files changed, 33 insertions(+), 26 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 92c..38764fe 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -143,7 +143,15 @@ struct ovnact_null {
>  /* OVNACT_NEXT. */
>  struct ovnact_next {
>  struct ovnact ovnact;
> +
> +/* Arguments. */
>  uint8_t ltable; /* Logical table ID of next table. */
> +
> +/* Information about the flow that the action is in.  This does not
> affect
> + * behavior, since the implementation of "next" doesn't depend on the
> + * source table or pipeline.  It does affect how ovnacts_format()
> prints
> + * the action. */
> +uint8_t src_ltable;/* Logical table ID of source table. */
>  };
>
>  /* OVNACT_LOAD. */
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 2162dad..bcc690f 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -259,36 +259,35 @@ parse_NEXT(struct action_context *ctx)
>  {
>  if (!ctx->pp->n_tables) {
>  lexer_error(ctx->lexer, "\"next\" action not allowed here.");
> -} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> -int ltable;
> -
> -if (!lexer_force_int(ctx->lexer, ) ||
> -!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) {
> -return;
> -}
> +return;
> +}
>
> -if (ltable >= ctx->pp->n_tables) {
> -lexer_error(ctx->lexer,
> -"\"next\" argument must be in range 0 to %d.",
> - ctx->pp->n_tables - 1);
> -return;
> -}
> +int table = ctx->pp->cur_ltable + 1;
> +if (lexer_match(ctx->lexer, LEX_T_LPAREN)
> +&& (!lexer_force_int(ctx->lexer, ) ||
> +!lexer_force_match(ctx->lexer, LEX_T_RPAREN))) {
> +return;
> +}
>
> -ovnact_put_NEXT(ctx->ovnacts)->ltable = ltable;
> -} else {
> -if (ctx->pp->cur_ltable < ctx->pp->n_tables) {
> -ovnact_put_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable
> + 1;
> -} else {
> -lexer_error(ctx->lexer,
> -"\"next\" action not allowed in last table.");
> -}
> +if (table >= ctx->pp->n_tables) {
> +lexer_error(ctx->lexer,
> +"\"next\" action cannot advance beyond table %d.",
> +ctx->pp->n_tables - 1);
>

Should there be a "return;" here?

Mickey


>  }
> +
> +struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
> +next->ltable = table;
> +next->src_ltable = ctx->pp->cur_ltable;
>  }
>
>  static void
>  format_NEXT(const struct ovnact_next *next, struct ds *s)
>  {
> -ds_put_format(s, "next(%d);", next->ltable);
> +if (next->ltable != next->src_ltable + 1) {
> +ds_put_format(s, "next(%d);", next->ltable);
> +} else {
> +ds_put_cstr(s, "next;");
> +}
>  }
>
>  static void
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 67d73c5..f71a4af 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -643,9 +643,9 @@ output;
>
>  # next
>  next;
> -formats as next(11);
>  encodes as resubmit(,27)
>  next(11);
> +formats as next;
>  encodes as resubmit(,27)
>  next(0);
>  encodes as resubmit(,16)
> @@ -657,7 +657,7 @@ next();
>  next(10;
>  Syntax error at `;' expecting `)'.
>  next(16);
> -"next" argument must be in range 0 to 15.
> +"next" action cannot advance beyond table 15.
>
>  # Loading a constant value.
>  tcp.dst=80;
> @@ -678,7 +678,7 @@ ip.ttl=4;
>  encodes as set_field:4->nw_ttl
>  has prereqs eth.type == 0x800 || eth.type == 0x86dd
>  outport="eth0"; next; outport="LOCAL"; next;
> -formats as outport = "eth0"; next(11); outport = "LOCAL"; next(11);
> +formats as outport = "eth0"; next; outport = "LOCAL"; next;
>  encodes as 

Re: [ovs-dev] [PATCH 00/10] Add actions for egress loopback

[Mickey Spiegel]

On Fri, Jan 20, 2017 at 3:33 PM, Ben Pfaff  wrote:

> On Fri, Jan 20, 2017 at 03:17:19PM -0800, Mickey Spiegel wrote:
> > On Fri, Jan 20, 2017 at 2:43 PM, Ben Pfaff  wrote:
> >
> > > On Fri, Jan 20, 2017 at 12:29:49PM -0800, Mickey Spiegel wrote:
> > > > On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:
> > > >
> > > > > I believe that, with these patches, egress loopback as proposed by
> > > Mickey's
> > > > > patches can be implemented with:
> > > > > clone { inport = outport; outport = ""; flags.loopback = 0;
> > > > > reg0 = 0; reg1 = 0; ... regN = 0;
> > > > > next(pipeline=ingress, table=0); }
> > > > >
> > > >
> > > > My main concern is maintainability as new flags or registers are
> added.
> > > > Having one line of code buried deep inside ovn/northd/ovn-northd.c
> that
> > > > needs to be updated whenever a flag or register is added worries me.
> > > > Does it make sense to add "clear_regs" and "clear_flags" actions in
> > > > order to address that concern?
> > >
> > > > I would also need to add in_port to symtab in
> ovn/lib/logical-fields.c so
> > > > that I can clear it.
> > >
> > > Can you explain why in_port needs to be cleared?
> > >
> >
> > My thought was that the packet should look like it arrived on the
> > distributed gateway port.
> > One question is whether there can be any bad implications if
> > in_port and logical inport do not match?
> > It is also possible that the packet will return back to the original
> > port after SNAT and DNAT on the distributed gateway port, so
> > if I do not clear in_port then I would have to set flags.loopback.
>
> Once a packet has been translated from physical to logical, the only
> real use of in_port is to discard packets that would loop back on the
> physical input port.  If we want to disable that, one way to do it is to
> set flags.loopback to 1, although that also disables discarding packets
> that would loop back to the logical input port.
>

I was thinking that egress loopback should be similar to traversing a
logical patch port. When traversing a logical patch port, in_port is
cleared.

If you have a problem with adding in_port to symtab, then I can just
set flags.loopback to 1. I would rather have the loopback check, but
at the moment add_route in ovn-northd.c sets flags.loopback to 1 in
all cases, so there is no difference on a logical router. Otherwise, I
will probably throw in a small patch to add in_port to symtab.

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

Re: [ovs-dev] [PATCH 00/10] Add actions for egress loopback

[Ben Pfaff]

On Fri, Jan 20, 2017 at 03:17:19PM -0800, Mickey Spiegel wrote:
> On Fri, Jan 20, 2017 at 2:43 PM, Ben Pfaff  wrote:
> 
> > On Fri, Jan 20, 2017 at 12:29:49PM -0800, Mickey Spiegel wrote:
> > > On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:
> > >
> > > > I believe that, with these patches, egress loopback as proposed by
> > Mickey's
> > > > patches can be implemented with:
> > > > clone { inport = outport; outport = ""; flags.loopback = 0;
> > > > reg0 = 0; reg1 = 0; ... regN = 0;
> > > > next(pipeline=ingress, table=0); }
> > > >
> > >
> > > My main concern is maintainability as new flags or registers are added.
> > > Having one line of code buried deep inside ovn/northd/ovn-northd.c that
> > > needs to be updated whenever a flag or register is added worries me.
> > > Does it make sense to add "clear_regs" and "clear_flags" actions in
> > > order to address that concern?
> >
> > > I would also need to add in_port to symtab in ovn/lib/logical-fields.c so
> > > that I can clear it.
> >
> > Can you explain why in_port needs to be cleared?
> >
> 
> My thought was that the packet should look like it arrived on the
> distributed gateway port.
> One question is whether there can be any bad implications if
> in_port and logical inport do not match?
> It is also possible that the packet will return back to the original
> port after SNAT and DNAT on the distributed gateway port, so
> if I do not clear in_port then I would have to set flags.loopback.

Once a packet has been translated from physical to logical, the only
real use of in_port is to discard packets that would loop back on the
physical input port.  If we want to disable that, one way to do it is to
set flags.loopback to 1, although that also disables discarding packets
that would loop back to the logical input port.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ofp-actions: Fix variable length meta-flow field usage in flow actions.

[Yi-Hung Wei]

Previously, if a flow action that involves a tunnel metadata meta-flow
field is dumped from vswitchd, the replied field length in the OXM header
is filled with the maximum possible field length, instead of the length
configured in the tunnel TLV mapping table. To solve this issue, this patch
introduces the following changes.

In order to maintain the correct length of variable length mf_fields (i.e.
tun_metadata), this patch creates a per-switch based map (struct vl_mff_map)
that hosts the variable length mf_fields. This map is updated when a
controller adds/deletes tlv-mapping entries to/from a switch. Although the
per-swtch based vl_mff_map only hosts tun_metadata for now, it is able to
support new variable length mf_fields in the future.

With this commit, when a switch decodes a flow action with mf_field, the switch
firstly looks up the global mf_fields map to identify the mf_field type. For
the variable length mf_fields, the switch uses the vl_mff_map to get the
configured mf_field entries. By lookig up vl_mff_map, the switch can check
if the added flow action access beyond the configured size of a variable
length mf_field, and the switch reports an ofperr if the controller adds a flow
with unmapped variable length mf_field. Later on, when a controller request
flows from the switch, with the per-switch based mf_fields, the switch will
encode the OXM header with correct length for variable length mf_fields.

To use the vl_mff_map for decoding flow actions, extract-ofp-actions is
updated to pass the vl_mff_map to the required action decoding functions.
Also, a new error code is introduced to identify a flow with an invalid
variable length mf_field. Moreover, a testcase is added to prevent future
regressions.

VMWare-BZ: #1768370
Reported-by: Harold Lim 
Suggested-by: Joe Stringer 
Suggested-by: Jarno Rajahalme 
Signed-off-by: Yi-Hung Wei 
---
 build-aux/extract-ofp-actions |  46 +---
 build-aux/extract-ofp-fields  |   5 +-
 include/openvswitch/meta-flow.h   |  24 
 include/openvswitch/ofp-actions.h |  11 +-
 include/openvswitch/ofp-errors.h  |   4 +
 include/openvswitch/ofp-util.h|   1 +
 lib/meta-flow.c   | 116 +++
 lib/nx-match.c|  89 +++
 lib/nx-match.h|  17 ++-
 lib/ofp-actions.c | 230 +-
 lib/ofp-print.c   |   2 +-
 lib/ofp-util.c|  21 ++--
 lib/tun-metadata.c|   2 +-
 ofproto/ofproto-provider.h|   5 +
 ofproto/ofproto.c |  20 +++-
 ovn/controller/pinctrl.c  |   9 +-
 tests/ofproto.at  | 125 +
 utilities/ovs-ofctl.c |   2 +-
 18 files changed, 579 insertions(+), 150 deletions(-)

diff --git a/build-aux/extract-ofp-actions b/build-aux/extract-ofp-actions
index 3a72349..184447b 100755
--- a/build-aux/extract-ofp-actions
+++ b/build-aux/extract-ofp-actions
@@ -123,7 +123,13 @@ def extract_ofp_actions(fn, definitions):
 fatal("unexpected syntax between actions")
 
 dsts = m.group(1)
-argtype = m.group(2).strip().replace('.', '', 1)
+argtypes = m.group(2).strip().replace('.', '', 1)
+
+if 'VLMFF' in argtypes:
+arg_vl_mff_map = True
+else:
+arg_vl_mff_map = False
+argtype = argtypes.replace('VLMFF', '', 1).rstrip()
 
 get_line()
 m = re.match(r'\s+(([A-Z]+)_RAW([0-9]*)_([A-Z0-9_]+)),?', line)
@@ -210,17 +216,18 @@ def extract_ofp_actions(fn, definitions):
 else:
 max_length = min_length
 
-info = {"enum": enum, # 0
-"deprecation": deprecation,   # 1
-"file_name": file_name,   # 2
-"line_number": line_number,   # 3
-"min_length": min_length, # 4
-"max_length": max_length, # 5
-"arg_ofs": arg_ofs,   # 6
-"arg_len": arg_len,   # 7
-"base_argtype": base_argtype, # 8
-"version": version,   # 9
-"type": type_}# 10
+info = {"enum": enum, # 0
+"deprecation": deprecation,   # 1
+"file_name": file_name,   # 2
+"line_number": line_number,   # 3
+"min_length": min_length, # 4
+"max_length": max_length, # 5
+"arg_ofs": arg_ofs,   # 6
+"arg_len": arg_len,   # 7
+   

[ovs-dev] [PATCH v2 6/7] actions: Make "next" action able to jump from egress to ingress pipeline.

[Ben Pfaff]

This feature is useful for centralized gateways.

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h | 63 --
 ovn/controller/lflow.c|  7 +++--
 ovn/lib/actions.c | 70 ---
 ovn/ovn-sb.xml| 12 ++--
 ovn/utilities/ovn-trace.c |  3 ++
 tests/ovn.at  | 22 ++-
 tests/test-ovn.c  |  6 ++--
 7 files changed, 138 insertions(+), 45 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f392d03..6691116 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -151,13 +151,15 @@ struct ovnact_next {
 struct ovnact ovnact;
 
 /* Arguments. */
-uint8_t ltable; /* Logical table ID of next table. */
+uint8_t ltable;/* Logical table ID of next table. */
+enum ovnact_pipeline pipeline; /* Pipeline of next table. */
 
 /* Information about the flow that the action is in.  This does not affect
  * behavior, since the implementation of "next" doesn't depend on the
  * source table or pipeline.  It does affect how ovnacts_format() prints
  * the action. */
-uint8_t src_ltable;/* Logical table ID of source table. */
+uint8_t src_ltable;/* Logical table ID of source table. */
+enum ovnact_pipeline src_pipeline; /* Pipeline of source table. */
 };
 
 /* OVNACT_LOAD. */
@@ -402,22 +404,26 @@ struct ovnact_parse_params {
 /* hmap of 'struct dhcp_opts_map'  to support 'put_dhcpv6_opts' action */
 const struct hmap *dhcpv6_opts;
 
-/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
- * OpenFlow flow table (ptable).  A number of parameters describe this
- * mapping and data related to flow tables:
+/* Each OVN flow exists in a logical table within a logical pipeline.
+ * These parameters express this context for a set of OVN actions being
+ * parsed:
  *
- * - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
- *to which the logical "next" action should be able to jump.
- *Logical table 0 maps to OpenFlow table 'first_ptable', logical
- *table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
- *then "next" is disallowed entirely.
+ * - 'n_tables' is the number of tables in the logical ingress and
+ *egress pipelines, that is, "next" may specify a table less than
+ *or equal to 'n_tables'.  If 'n_tables' is 0 then "next" is
+ *disallowed entirely.
  *
- * - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
- *   cur_ltable < n_tables) of the logical flow that contains the
- *   actions.  If cur_ltable + 1 < n_tables, then this defines the
- *   default table that "next" will jump to. */
-uint8_t n_tables;   /* Number of flow tables. */
-uint8_t cur_ltable; /* 0 <= cur_ltable < n_tables. */
+ * - 'cur_ltable' is the logical table of the current flow, within
+ *   'pipeline'.  If cur_ltable + 1 < n_tables, then this defines the
+ *   default table that "next" will jump to.
+ *
+ * - 'pipeline' is the logical pipeline.  It is the default pipeline to
+ *   which 'next' will jump.  If 'pipeline' is OVNACT_P_EGRESS, then
+ *   'next' will also be able to jump into the ingress pipeline, but
+ *   the reverse is not true. */
+enum ovnact_pipeline pipeline; /* Logical pipeline. */
+uint8_t n_tables;  /* Number of logical flow tables. */
+uint8_t cur_ltable;/* 0 <= cur_ltable < n_tables. */
 };
 
 bool ovnacts_parse(struct lexer *, const struct ovnact_parse_params *,
@@ -448,20 +454,23 @@ struct ovnact_encode_params {
  * OpenFlow flow table (ptable).  A number of parameters describe this
  * mapping and data related to flow tables:
  *
- * - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
- *to which the logical "next" action should be able to jump.
- *Logical table 0 maps to OpenFlow table 'first_ptable', logical
- *table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
- *then "next" is disallowed entirely.
+ * - 'pipeline' is the logical pipeline in which the actions are
+ *   executing.
  *
- * - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
- *   cur_ltable < n_ptables) of the logical flow that contains the
- *   actions.  If cur_ltable + 1 < n_tables, then this defines the
- *   default table that "next" will jump to.
+ * - 'ingress_ptable' is the OpenFlow table that corresponds to OVN
+ *   ingress table 0.
+ *
+ * - 'egress_ptable' is the OpenFlow table that corresponds to OVN
+ *   egress table 0.
  *
  *  

[ovs-dev] [PATCH v2 5/7] actions: Introduce enum ovnact_pipeline.

[Ben Pfaff]

This isn't used yet by the actions code, but an upcoming commit will
introduce a user.  This commit just adjusts ovn-trace to use this common
type instead of its own local type.

Signed-off-by: Ben Pfaff 
Acked-by: Mickey Spiegel 
---
 include/ovn/actions.h |  6 ++
 ovn/utilities/ovn-trace.c | 42 --
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 38764fe..f392d03 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -140,6 +140,12 @@ struct ovnact_null {
 struct ovnact ovnact;
 };
 
+/* Logical pipeline in which a set of actions is executed. */
+enum ovnact_pipeline {
+OVNACT_P_INGRESS,
+OVNACT_P_EGRESS,
+};
+
 /* OVNACT_NEXT. */
 struct ovnact_next {
 struct ovnact ovnact;
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 982a81a..654b8d0 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -322,11 +322,9 @@ struct ovntrace_mcgroup {
 size_t n_ports;
 };
 
-enum ovntrace_pipeline { P_INGRESS, P_EGRESS };
-
 struct ovntrace_flow {
 struct uuid uuid;
-enum ovntrace_pipeline pipeline;
+enum ovnact_pipeline pipeline;
 int table_id;
 char *stage_name;
 char *source;
@@ -632,8 +630,8 @@ compare_flow(const void *a_, const void *b_)
 const struct ovntrace_flow *b = *bp;
 
 if (a->pipeline != b->pipeline) {
-/* Sort P_INGRESS before P_EGRESS. */
-return a->pipeline == P_EGRESS ? 1 : -1;
+/* Sort OVNACT_P_INGRESS before OVNACT_P_EGRESS. */
+return a->pipeline == OVNACT_P_EGRESS ? 1 : -1;
 } else if (a->table_id != b->table_id) {
 /* Sort in increasing order of table_id. */
 return a->table_id > b->table_id ? 1 : -1;
@@ -706,8 +704,8 @@ read_flows(void)
 struct ovntrace_flow *flow = xzalloc(sizeof *flow);
 flow->uuid = sblf->header_.uuid;
 flow->pipeline = (!strcmp(sblf->pipeline, "ingress")
-  ? P_INGRESS
-  : P_EGRESS);
+  ? OVNACT_P_INGRESS
+  : OVNACT_P_EGRESS);
 flow->table_id = sblf->table_id;
 flow->stage_name = nullable_xstrdup(smap_get(>external_ids,
  "stage-name"));
@@ -835,7 +833,7 @@ ovntrace_lookup_port(const void *dp_, const char *port_name,
 static const struct ovntrace_flow *
 ovntrace_flow_lookup(const struct ovntrace_datapath *dp,
  const struct flow *uflow,
- uint8_t table_id, enum ovntrace_pipeline pipeline)
+ uint8_t table_id, enum ovnact_pipeline pipeline)
 {
 for (size_t i = 0; i < dp->n_flows; i++) {
 const struct ovntrace_flow *flow = dp->flows[i];
@@ -850,7 +848,7 @@ ovntrace_flow_lookup(const struct ovntrace_datapath *dp,
 
 static char *
 ovntrace_stage_name(const struct ovntrace_datapath *dp,
-uint8_t table_id, enum ovntrace_pipeline pipeline)
+uint8_t table_id, enum ovnact_pipeline pipeline)
 {
 for (size_t i = 0; i < dp->n_flows; i++) {
 const struct ovntrace_flow *flow = dp->flows[i];
@@ -1100,17 +1098,17 @@ execute_exchange(const struct ovnact_move *move, struct 
flow *uflow,
 
 static void
 trace__(const struct ovntrace_datapath *dp, struct flow *uflow,
-uint8_t table_id, enum ovntrace_pipeline pipeline,
+uint8_t table_id, enum ovnact_pipeline pipeline,
 struct ovs_list *super);
 
 static void
 trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
   const struct ovntrace_datapath *dp, struct flow *uflow,
-  uint8_t table_id, enum ovntrace_pipeline pipeline,
+  uint8_t table_id, enum ovnact_pipeline pipeline,
   struct ovs_list *super);
 static void
 execute_output(const struct ovntrace_datapath *dp, struct flow *uflow,
-   enum ovntrace_pipeline pipeline, struct ovs_list *super)
+   enum ovnact_pipeline pipeline, struct ovs_list *super)
 {
 uint16_t key = uflow->regs[MFF_LOG_OUTPORT - MFF_REG0];
 if (!key) {
@@ -1131,7 +1129,7 @@ execute_output(const struct ovntrace_datapath *dp, struct 
flow *uflow,
  key);
 }
 
-if (pipeline == P_EGRESS) {
+if (pipeline == OVNACT_P_EGRESS) {
 ovntrace_node_append(super, OVNTRACE_NODE_OUTPUT,
  "/* output to \"%s\", type \"%s\" */",
  out_name, port ? port->type : "");
@@ -1146,7 +1144,7 @@ execute_output(const struct ovntrace_datapath *dp, struct 
flow *uflow,
 struct flow new_uflow = *uflow;
 new_uflow.regs[MFF_LOG_INPORT - MFF_REG0] = peer->tunnel_key;
 new_uflow.regs[MFF_LOG_OUTPORT - MFF_REG0] = 0;
-trace__(peer->dp, _uflow, 0, P_INGRESS, >subs);
+

[ovs-dev] [PATCH v2 3/7] actions: Separate action structures for "next" and "ct_next".

[Ben Pfaff]

These actions aren't very similar but until now they both had the same
action structure.  These structures are going to diverge in an upcoming
commit, so separate them now.

Signed-off-by: Ben Pfaff 
Acked-by: Mickey Spiegel 
---
 include/ovn/actions.h | 10 --
 ovn/lib/actions.c | 11 ---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index a1b6b90..92c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -55,7 +55,7 @@ struct simap;
 OVNACT(MOVE,  ovnact_move)  \
 OVNACT(EXCHANGE,  ovnact_move)  \
 OVNACT(DEC_TTL,   ovnact_null)  \
-OVNACT(CT_NEXT,   ovnact_next)  \
+OVNACT(CT_NEXT,   ovnact_ct_next)   \
 OVNACT(CT_COMMIT, ovnact_ct_commit) \
 OVNACT(CT_DNAT,   ovnact_ct_nat)\
 OVNACT(CT_SNAT,   ovnact_ct_nat)\
@@ -140,7 +140,7 @@ struct ovnact_null {
 struct ovnact ovnact;
 };
 
-/* OVNACT_NEXT, OVNACT_CT_NEXT. */
+/* OVNACT_NEXT. */
 struct ovnact_next {
 struct ovnact ovnact;
 uint8_t ltable; /* Logical table ID of next table. */
@@ -160,6 +160,12 @@ struct ovnact_move {
 struct expr_field rhs;
 };
 
+/* OVNACT_CT_NEXT. */
+struct ovnact_ct_next {
+struct ovnact ovnact;
+uint8_t ltable;/* Logical table ID of next table. */
+};
+
 /* OVNACT_CT_COMMIT. */
 struct ovnact_ct_commit {
 struct ovnact ovnact;
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 213e22e..2162dad 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -530,24 +530,29 @@ parse_CT_NEXT(struct action_context *ctx)
 }
 
 static void
-format_CT_NEXT(const struct ovnact_next *next OVS_UNUSED, struct ds *s)
+format_CT_NEXT(const struct ovnact_ct_next *ct_next OVS_UNUSED, struct ds *s)
 {
 ds_put_cstr(s, "ct_next;");
 }
 
 static void
-encode_CT_NEXT(const struct ovnact_next *next,
+encode_CT_NEXT(const struct ovnact_ct_next *ct_next,
 const struct ovnact_encode_params *ep,
 struct ofpbuf *ofpacts)
 {
 struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
-ct->recirc_table = ep->first_ptable + next->ltable;
+ct->recirc_table = ep->first_ptable + ct_next->ltable;
 ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE)
 : mf_from_id(MFF_LOG_DNAT_ZONE);
 ct->zone_src.ofs = 0;
 ct->zone_src.n_bits = 16;
 ofpact_finish(ofpacts, >ofpact);
 }
+
+static void
+ovnact_ct_next_free(struct ovnact_ct_next *a OVS_UNUSED)
+{
+}
 
 static void
 parse_ct_commit_arg(struct action_context *ctx,
-- 
2.10.2

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

[ovs-dev] [PATCH v2 0/7] Add actions for egress loopback

[Ben Pfaff]

I believe that, with these patches, egress loopback as proposed by Mickey's
patches can be implemented with:
clone { inport = outport; outport = ""; flags = 0;
reg0 = 0; reg1 = 0; ... regN = 0;
next(pipeline=ingress, table=0); }

v1->v2:
  - Patches 1, 2, and 3 applied and dropped.
  - Acks applied to other patches.
  - Typo fixed in patch 5 (now patch 2).
  - Stupid bugs fixed in patch 7 (now patch 4).

Ben Pfaff (7):
  actions: Make "free" functions per-struct, not per-action.
  actions: Add new OVN action "clone".
  actions: Separate action structures for "next" and "ct_next".
  actions: Omit table number when possible for formatting "next" action.
  actions: Introduce enum ovnact_pipeline.
  actions: Make "next" action able to jump from egress to ingress
pipeline.
  actions: Add new "ct_clear" action.

 include/ovn/actions.h |  91 ++--
 ovn/controller/lflow.c|   7 +-
 ovn/lib/actions.c | 260 +++---
 ovn/ovn-sb.xml|  26 -
 ovn/utilities/ovn-trace.c |  65 
 tests/ovn.at  |  37 ++-
 tests/test-ovn.c  |   6 +-
 7 files changed, 324 insertions(+), 168 deletions(-)

-- 
2.10.2

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

Re: [ovs-dev] [PATCH 00/10] Add actions for egress loopback

[Ben Pfaff]

On Fri, Jan 20, 2017 at 12:29:49PM -0800, Mickey Spiegel wrote:
> On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:
> 
> > I believe that, with these patches, egress loopback as proposed by Mickey's
> > patches can be implemented with:
> > clone { inport = outport; outport = ""; flags.loopback = 0;
> > reg0 = 0; reg1 = 0; ... regN = 0;
> > next(pipeline=ingress, table=0); }
> >
> 
> My main concern is maintainability as new flags or registers are added.
> Having one line of code buried deep inside ovn/northd/ovn-northd.c that
> needs to be updated whenever a flag or register is added worries me.
> Does it make sense to add "clear_regs" and "clear_flags" actions in
> order to address that concern?

I don't think that flags are a problem.  We can just write "flags = 0;"
instead of specific flags; I didn't think of that before.

To ensure that all the registers are cleared, we can just use a loop in
ovn-northd:
for (int i = 0; i < MFF_N_LOG_REGS; i++) {
ds_put_format(, "reg%d = 0; ", i);
}

> I would also need to add in_port to symtab in ovn/lib/logical-fields.c so
> that I can clear it.

Can you explain why in_port needs to be cleared?

> For patches 1 through 4, 6, and 8:
> Acked-by: Mickey Spiegel 
> 
> I commented separately on patches 5 and 7.
> 
> I could not apply patches 9 and 10 since I manually fixed patch 7
> and the indexes did not match.

Thanks a lot for the reviews.

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

Re: [ovs-dev] [PATCH 03/10] actions: Make "arp { drop; }; " acceptable.

[Ben Pfaff]

On Fri, Jan 20, 2017 at 09:16:25AM -0800, Ben Pfaff wrote:
> Before this commit, the OVN action parser would accept "arp {};" and then
> the formatter would format it back as "arp { drop; };", but the parser
> didn't accept the latter.  There were basically two choices: make the
> parser accept "arp { drop; };" or make the formatter output "arp {};"
> (or both).  This patch does (only) the former, and adds a test to avoid
> regression.
> 
> Signed-off-by: Ben Pfaff 

Thanks for the acks.  I applied the first three patches to master and
branch-2.7 (since they are bug fixes).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 07/10] actions: Omit table number when possible for formatting "next" action.

[Ben Pfaff]

I did a bad job on this patch.  I've rewritten it and will repost.

On Fri, Jan 20, 2017 at 11:51:56AM -0800, Mickey Spiegel wrote:
> On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:
> 
> > Until now, formatting the "next" action has always required including
> > the table number, because the action struct didn't include enough context
> > so that the formatter could decide whether the table number was the next
> > table or some other table.  This is more or less OK, but an upcoming commit
> > will add a "pipeline" field to the "next" action, which means that the same
> > policy there would require that the pipeline always be printed.  That's a
> > little obnoxious because 99+% of the time, the pipeline to be printed is
> > the same pipeline that the flow is in and printing it would be distracting.
> > So it's better to store some context to help with formatting.  This commit
> > begins adopting that policy for the existing table number field.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  include/ovn/actions.h |  8 
> >  ovn/lib/actions.c | 29 +++--
> >  tests/ovn.at  |  8 
> >  3 files changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> > index 92c..38764fe 100644
> > --- a/include/ovn/actions.h
> > +++ b/include/ovn/actions.h
> > @@ -143,7 +143,15 @@ struct ovnact_null {
> >  /* OVNACT_NEXT. */
> >  struct ovnact_next {
> >  struct ovnact ovnact;
> > +
> > +/* Arguments. */
> >  uint8_t ltable; /* Logical table ID of next table. */
> > +
> > +/* Information about the flow that the action is in.  This does not
> > affect
> > + * behavior, since the implementation of "next" doesn't depend on the
> > + * source table or pipeline.  It does affect how ovnacts_format()
> > prints
> > + * the action. */
> > +uint8_t src_ltable;/* Logical table ID of source table. */
> >  };
> >
> >  /* OVNACT_LOAD. */
> > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> > index 2162dad..5548234 100644
> > --- a/ovn/lib/actions.c
> > +++ b/ovn/lib/actions.c
> > @@ -259,7 +259,11 @@ parse_NEXT(struct action_context *ctx)
> >  {
> >  if (!ctx->pp->n_tables) {
> >  lexer_error(ctx->lexer, "\"next\" action not allowed here.");
> > -} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> > +return;
> > +}
> > +
> > +int table = ctx->pp->cur_ltable + 1;
> > +if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> >  int ltable;
> >
> >  if (!lexer_force_int(ctx->lexer, ) ||
> > @@ -273,22 +277,27 @@ parse_NEXT(struct action_context *ctx)
> >   ctx->pp->n_tables - 1);
> >  return;
> >  }
> >
> 
> You never set "table = ltable;", which belongs here. As a result,
> "next(11);" always encodes as "next;", i.e. the number is ignored.
> 
> 
> > +}
> >
> > -ovnact_put_NEXT(ctx->ovnacts)->ltable = ltable;
> > -} else {
> > -if (ctx->pp->cur_ltable < ctx->pp->n_tables) {
> > -ovnact_put_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable
> > + 1;
> > -} else {
> > -lexer_error(ctx->lexer,
> > -"\"next\" action not allowed in last table.");
> > -}
> > +if (table >= ctx->pp->n_tables) {
> > +lexer_error(ctx->lexer,
> > +"\"next\" action cannot advance beyond table %d.",
> > +ctx->pp->n_tables - 1);
> >  }
> > +
> > +struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
> > +next->ltable = table;
> > +next->src_ltable = ctx->pp->cur_ltable;
> >  }
> >
> >  static void
> >  format_NEXT(const struct ovnact_next *next, struct ds *s)
> >  {
> > -ds_put_format(s, "next(%d);", next->ltable);
> > +if (next->ltable != next->src_ltable + 1) {
> > +ds_put_format(s, "next(%d);", next->ltable);
> > +} else {
> > +ds_put_cstr(s, "next;");
> > +}
> >  }
> >
> >  static void
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 67d73c5..f71a4af 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -643,9 +643,9 @@ output;
> >
> >  # next
> >  next;
> > -formats as next(11);
> >  encodes as resubmit(,27)
> >  next(11);
> > +formats as next;
> >  encodes as resubmit(,27)
> >  next(0);
> >  encodes as resubmit(,16)
> > @@ -657,7 +657,7 @@ next();
> >  next(10;
> >  Syntax error at `;' expecting `)'.
> >  next(16);
> > -"next" argument must be in range 0 to 15.
> > +"next" action cannot advance beyond table 15.
> >
> 
> Using this diff, there are two different responses
> depending on whether the command is "next;" or
> "next(11);".
> The former gives the "cannot advance beyond" message.
> The latter gives the "in range 0 to 15" message.
> Unless the error messages are changed, this change
> should be reverted.
> 
> Mickey
> 
> 
> >  # Loading a constant 

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

[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] [userspace clone v2 1/4] dpif-netdev: Avoid sending probe packets

[Andy Zhou]

On Fri, Jan 20, 2017 at 11:06 AM, Jarno Rajahalme  wrote:

>
> > On Jan 19, 2017, at 11:07 PM, Andy Zhou  wrote:
> >
> > When ofproto probe for datapath features, no packets should actually
> > be sent to the network. This pactch fixes the userspace by dropping
> > probe packets before action execution.
> >
> > Signed-off-by: Andy Zhou 
> > ---
> > lib/dpif-netdev.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 3901129..9c21af3 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -2660,6 +2660,13 @@ dpif_netdev_execute(struct dpif *dpif, struct
> dpif_execute *execute)
> > }
> > }
> >
> > +if (execute->probe) {
> > +/* If this is part of a probe, Drop the packet, since executing
> > + * the action may actually cause spurious packets be sent into
> > + * the network. */
> > +return 0;
> > +}
> > +
>
> On the linux kernel datapath the execution can also return an error code,
> so the probe may be inconclusive if we do not execute the actions. Have you
> verified this is not the case for the userspace datapath?
>

This patch only fixes the user space datapath. Kernel datapah should be
O.K. if we take your following suggestion.

>
> This is why I’d rather have the probes not include actions that can have
> visible side effects.
>

The idea of probe is to get feedback on netlink encoding, not the actual
action execution. In this case, we should drop the output action, (or any
action that have visible side effects
as you have suggested).

>
>   Jarno
>
> > /* If the current thread is non-pmd thread, acquires
> >  * the 'non_pmd_mutex'. */
> > if (pmd->core_id == NON_PMD_CORE_ID) {
> > --
> > 1.9.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 00/10] Add actions for egress loopback

[Mickey Spiegel]

On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:

> I believe that, with these patches, egress loopback as proposed by Mickey's
> patches can be implemented with:
> clone { inport = outport; outport = ""; flags.loopback = 0;
> reg0 = 0; reg1 = 0; ... regN = 0;
> next(pipeline=ingress, table=0); }
>

My main concern is maintainability as new flags or registers are added.
Having one line of code buried deep inside ovn/northd/ovn-northd.c that
needs to be updated whenever a flag or register is added worries me.
Does it make sense to add "clear_regs" and "clear_flags" actions in
order to address that concern?

I would also need to add in_port to symtab in ovn/lib/logical-fields.c so
that I can clear it.


> Ben Pfaff (10):
>   actions: Fix "arp" and "nd_na" followed by another action.
>   lex: Make lexer_force_match() work for LEX_T_END.
>   actions: Make "arp { drop; };" acceptable.
>   actions: Make "free" functions per-struct, not per-action.
>   actions: Add new OVN action "clone".
>   actions: Separate action structures for "next" and "ct_next".
>   actions: Omit table number when possible for formatting "next" action.
>   actions: Introduce enum ovnact_pipeline.
>   actions: Make "next" action able to jump from egress to ingress
> pipeline.
>   actions: Add new "ct_clear" action.
>

For patches 1 through 4, 6, and 8:
Acked-by: Mickey Spiegel 

I commented separately on patches 5 and 7.

I could not apply patches 9 and 10 since I manually fixed patch 7
and the indexes did not match.

Mickey


>  include/ovn/actions.h |  91 ++-
>  include/ovn/lex.h |   4 +-
>  ovn/controller/lflow.c|   7 +-
>  ovn/lib/actions.c | 284 +++---
> 
>  ovn/lib/lex.c |  13 ++-
>  ovn/ovn-sb.xml|  26 -
>  ovn/utilities/ovn-trace.c |  65 +++
>  tests/ovn.at  |  45 +++-
>  tests/test-ovn.c  |   6 +-
>  9 files changed, 352 insertions(+), 189 deletions(-)
>
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Calendario de Capacitación Febrero

[Planifique la Capacitación de su Equipo]

Estimado Cliente:

Le hacemos llegar el programa de capacitación correspondiente al mes de Febrero 
del ao en curso.


- Auditorías para la adquisición de empresas: Due Diligence - 2 de Febrero

- Matemáticas Financieras y Evaluación de Proyectos de Inversión - 3 de Febrero 

- Habilidades de Liderazgo y Trabajo en Equipo con Lego® Serious Play® - 3 de 
Febrero 

- El Éxito en los Negocios y las Finanzas basado en el Modelo Disney® - 10 de 
Febrero 

- Implementación de las NIIF 9 (reemplaza a la NIC 39) - 16 de Febrero 

- Curso Práctico de Análisis Financiero - 16 de marzo

- La Asistente Asertiva, Activa y Efectiva - 23 de Febrero 

- Taller de Grafología - 23 de Febrero 

-  Estrategias de Negociación de Compras - 24 de Febrero 



Para Obtener Información Completa, responda este correo con los siguientes 
datos: Nombre,Empresa,Teléfono,Curso. De igual manera Puede llamarnos sin 
compromiso al 018002120744. 


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

Re: [ovs-dev] [PATCH 07/10] actions: Omit table number when possible for formatting "next" action.

[Mickey Spiegel]

On Fri, Jan 20, 2017 at 9:16 AM, Ben Pfaff  wrote:

> Until now, formatting the "next" action has always required including
> the table number, because the action struct didn't include enough context
> so that the formatter could decide whether the table number was the next
> table or some other table.  This is more or less OK, but an upcoming commit
> will add a "pipeline" field to the "next" action, which means that the same
> policy there would require that the pipeline always be printed.  That's a
> little obnoxious because 99+% of the time, the pipeline to be printed is
> the same pipeline that the flow is in and printing it would be distracting.
> So it's better to store some context to help with formatting.  This commit
> begins adopting that policy for the existing table number field.
>
> Signed-off-by: Ben Pfaff 
> ---
>  include/ovn/actions.h |  8 
>  ovn/lib/actions.c | 29 +++--
>  tests/ovn.at  |  8 
>  3 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 92c..38764fe 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -143,7 +143,15 @@ struct ovnact_null {
>  /* OVNACT_NEXT. */
>  struct ovnact_next {
>  struct ovnact ovnact;
> +
> +/* Arguments. */
>  uint8_t ltable; /* Logical table ID of next table. */
> +
> +/* Information about the flow that the action is in.  This does not
> affect
> + * behavior, since the implementation of "next" doesn't depend on the
> + * source table or pipeline.  It does affect how ovnacts_format()
> prints
> + * the action. */
> +uint8_t src_ltable;/* Logical table ID of source table. */
>  };
>
>  /* OVNACT_LOAD. */
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 2162dad..5548234 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -259,7 +259,11 @@ parse_NEXT(struct action_context *ctx)
>  {
>  if (!ctx->pp->n_tables) {
>  lexer_error(ctx->lexer, "\"next\" action not allowed here.");
> -} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> +return;
> +}
> +
> +int table = ctx->pp->cur_ltable + 1;
> +if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
>  int ltable;
>
>  if (!lexer_force_int(ctx->lexer, ) ||
> @@ -273,22 +277,27 @@ parse_NEXT(struct action_context *ctx)
>   ctx->pp->n_tables - 1);
>  return;
>  }
>

You never set "table = ltable;", which belongs here. As a result,
"next(11);" always encodes as "next;", i.e. the number is ignored.


> +}
>
> -ovnact_put_NEXT(ctx->ovnacts)->ltable = ltable;
> -} else {
> -if (ctx->pp->cur_ltable < ctx->pp->n_tables) {
> -ovnact_put_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable
> + 1;
> -} else {
> -lexer_error(ctx->lexer,
> -"\"next\" action not allowed in last table.");
> -}
> +if (table >= ctx->pp->n_tables) {
> +lexer_error(ctx->lexer,
> +"\"next\" action cannot advance beyond table %d.",
> +ctx->pp->n_tables - 1);
>  }
> +
> +struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
> +next->ltable = table;
> +next->src_ltable = ctx->pp->cur_ltable;
>  }
>
>  static void
>  format_NEXT(const struct ovnact_next *next, struct ds *s)
>  {
> -ds_put_format(s, "next(%d);", next->ltable);
> +if (next->ltable != next->src_ltable + 1) {
> +ds_put_format(s, "next(%d);", next->ltable);
> +} else {
> +ds_put_cstr(s, "next;");
> +}
>  }
>
>  static void
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 67d73c5..f71a4af 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -643,9 +643,9 @@ output;
>
>  # next
>  next;
> -formats as next(11);
>  encodes as resubmit(,27)
>  next(11);
> +formats as next;
>  encodes as resubmit(,27)
>  next(0);
>  encodes as resubmit(,16)
> @@ -657,7 +657,7 @@ next();
>  next(10;
>  Syntax error at `;' expecting `)'.
>  next(16);
> -"next" argument must be in range 0 to 15.
> +"next" action cannot advance beyond table 15.
>

Using this diff, there are two different responses
depending on whether the command is "next;" or
"next(11);".
The former gives the "cannot advance beyond" message.
The latter gives the "in range 0 to 15" message.
Unless the error messages are changed, this change
should be reverted.

Mickey


>  # Loading a constant value.
>  tcp.dst=80;
> @@ -678,7 +678,7 @@ ip.ttl=4;
>  encodes as set_field:4->nw_ttl
>  has prereqs eth.type == 0x800 || eth.type == 0x86dd
>  outport="eth0"; next; outport="LOCAL"; next;
> -formats as outport = "eth0"; next(11); outport = "LOCAL"; next(11);
> +formats as outport = "eth0"; next; outport = "LOCAL"; next;
>  encodes as set_field:0x5->reg15,resubmit(
> 

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

[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

Re: [ovs-dev] [userspace clone v2 4/4] xlate: Generate of datapath clone action when supported

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme 

> On Jan 19, 2017, at 11:07 PM, Andy Zhou  wrote:
> 
> Add logic to detect whether datapath support clone.
> Enhance the xlate logic to make use of it.
> Added logic to turn on/off clone support for testing.
> 
> Signed-off-by: Andy Zhou 
> ---
> ofproto/ofproto-dpif-xlate.c | 38 --
> ofproto/ofproto-dpif-xlate.h |  2 ++
> ofproto/ofproto-dpif.c   | 23 +++
> tests/ofproto-dpif.at| 11 +++
> 4 files changed, 72 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 9a15ec3..0513394 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4520,9 +4520,23 @@ xlate_sample_action(struct xlate_ctx *ctx,
>   tunnel_out_port, false);
> }
> 
> +/* Only called if the datapath supports 'OVS_ACTION_ATTR_CLONE'.
> + *
> + * Translates 'oc' within OVS_ACTION_ATTR_CLONE. */
> static void
> compose_clone_action(struct xlate_ctx *ctx, const struct ofpact_nest *oc)
> {
> +size_t clone_offset = nl_msg_start_nested(ctx->odp_actions,
> +  OVS_ACTION_ATTR_CLONE);
> +
> +do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
> +
> +nl_msg_end_non_empty_nested(ctx->odp_actions, clone_offset);
> +}
> +
> +static void
> +xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc)
> +{
> bool old_was_mpls = ctx->was_mpls;
> bool old_conntracked = ctx->conntracked;
> struct flow old_flow = ctx->xin->flow;
> @@ -4537,7 +4551,16 @@ compose_clone_action(struct xlate_ctx *ctx, const 
> struct ofpact_nest *oc)
> ofpbuf_use_stub(>action_set, actset_stub, sizeof actset_stub);
> ofpbuf_put(>action_set, old_action_set.data, old_action_set.size);
> 
> -do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
> +if (ctx->xbridge->support.clone) {
> +/* Datapath clone action will make sure the pre clone packets
> + * are used for actions after clone. Save and restore
> + * ctx->base_flow to reflect this for the openflow pipeline. */
> +struct flow old_base_flow = ctx->base_flow;
> +compose_clone_action(ctx, oc);
> +ctx->base_flow = old_base_flow;
> +} else {
> +do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
> +}
> 
> ofpbuf_uninit(>action_set);
> ctx->action_set = old_action_set;
> @@ -5383,7 +5406,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t 
> ofpacts_len,
> break;
> 
> case OFPACT_CLONE:
> -compose_clone_action(ctx, ofpact_get_CLONE(a));
> +xlate_clone(ctx, ofpact_get_CLONE(a));
> break;
> 
> case OFPACT_CT:
> @@ -6169,3 +6192,14 @@ xlate_mac_learning_update(const struct ofproto_dpif 
> *ofproto,
> 
> update_learning_table__(xbridge, xbundle, dl_src, vlan, is_grat_arp);
> }
> +
> +void
> +xlate_disable_dp_clone(const struct ofproto_dpif *ofproto)
> +{
> +struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, );
> +struct xbridge *xbridge = xbridge_lookup(xcfg, ofproto);
> +
> +if (xbridge) {
> +xbridge->support.clone = false;
> +}
> +}
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index 5d00d6d..3986f26 100644
> --- a/ofproto/ofproto-dpif-xlate.h
> +++ b/ofproto/ofproto-dpif-xlate.h
> @@ -206,6 +206,8 @@ void xlate_mac_learning_update(const struct ofproto_dpif 
> *ofproto,
>ofp_port_t in_port, struct eth_addr dl_src,
>int vlan, bool is_grat_arp);
> 
> +void xlate_disable_dp_clone(const struct ofproto_dpif *);
> +
> void xlate_txn_start(void);
> void xlate_txn_commit(void);
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 5ef0720..72bfbbc 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5025,6 +5025,26 @@ disable_datapath_truncate(struct unixctl_conn *conn 
> OVS_UNUSED,
> }
> 
> static void
> +disable_datapath_clone(struct unixctl_conn *conn OVS_UNUSED,
> +   int argc, const char *argv[],
> +   void *aux OVS_UNUSED)
> +{
> +struct ds ds = DS_EMPTY_INITIALIZER;
> +const char *br = argv[argc -1];
> +struct ofproto_dpif *ofproto;
> +
> +ofproto = ofproto_dpif_lookup(br);
> +if (!ofproto) {
> +unixctl_command_reply_error(conn, "no such bridge");
> +return;
> +}
> +xlate_disable_dp_clone(ofproto);
> +udpif_flush(ofproto->backer->udpif);
> +ds_put_format(, "Datapath clone action disabled for bridge %s", br);
> +unixctl_command_reply(conn, ds_cstr());
> +}
> +
> +static void
> ofproto_unixctl_init(void)
> {
> static bool registered;
> @@ -5053,6 +5073,9 @@ ofproto_unixctl_init(void)
> 
> unixctl_command_register("dpif/disable-truncate", 

Re: [ovs-dev] [userspace clone v2 3/4] dpif-netdev: Add clone action

[Jarno Rajahalme]

> On Jan 19, 2017, at 11:07 PM, Andy Zhou  wrote:
> 
> Add support for userspace datapath clone action.  The clone action
> provides an action envelope to enclose an action list.
> For example, with actions A, B, C and D,  and an action list:
>  A, clone(B, C), D
> 
> The clone action will ensure that:
> 
> - D will see the same packet, and any meta states, such as flow, as
>  action B.
> 
> - D will be executed regardless whether B, or C drops a packet. They
>  can only drop a clone.
> 
> - When B drops a packet, clone will skip all remaining actions
>  within the clone envelope. This feature is useful when we add
>  meter action later:  The meter action can be implemented as a
>  simple action without its own envolop (unlike the sample action).
>  When necessary, the flow translation layer can enclose a meter action
>  in clone.
> 
> The clone action is very similar with the OpenFlow clone action.
> This is by design to simplify vswitchd flow translation logic.
> 
> Without datapath clone, vswitchd simulate the effect by inserting
> datapath actions to "undo" clone actions. The above flow will be
> translated into   A, B, C, -C, -B, D.
> 
> However, there are two issues:
> - The resulting datapath action list may be longer without using
>  clone.
> 
> - Some actions, such as NAT may not be possible to reverse.
> 
> This patch implements clone() simply with packet copy. The performance
> can be improved with later patches, for example, to delay or avoid
> packet copy if possible.  It seems datapath should have enough context
> to carry out such optimization without the userspace context.
> 
> Signed-off-by: Andy Zhou 
> ---
> datapath/linux/compat/include/linux/openvswitch.h |  3 +-
> lib/dpif-netdev.c |  1 +
> lib/dpif.c|  1 +
> lib/odp-execute.c | 34 ++
> lib/odp-util.c| 16 +++
> ofproto/ofproto-dpif-sflow.c  |  1 +
> ofproto/ofproto-dpif.c| 55 +++
> ofproto/ofproto-dpif.h|  3 ++
> 8 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index 12260d8..425d3a4 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2007-2014 Nicira, Inc.
> + * Copyright (c) 2007-2017 Nicira, Inc.
>  *
>  * This file is offered under your choice of two licenses: Apache 2.0 or GNU
>  * GPL 2.0 or later.  The permission statements for each of these licenses is
> @@ -818,6 +818,7 @@ enum ovs_action_attr {
> #ifndef __KERNEL__
>   OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
>   OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
> + OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*.  */
> #endif
>   __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>  * from userspace. */
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9c21af3..42631bc 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4731,6 +4731,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
> *packets_,
> case OVS_ACTION_ATTR_HASH:
> case OVS_ACTION_ATTR_UNSPEC:
> case OVS_ACTION_ATTR_TRUNC:
> +case OVS_ACTION_ATTR_CLONE:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 50c3382..7c953b5 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1182,6 +1182,7 @@ dpif_execute_helper_cb(void *aux_, struct 
> dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_SET_MASKED:
> case OVS_ACTION_ATTR_SAMPLE:
> case OVS_ACTION_ATTR_TRUNC:
> +case OVS_ACTION_ATTR_CLONE:
> case OVS_ACTION_ATTR_UNSPEC:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 73e1016..b8827ab 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -528,6 +528,26 @@ odp_execute_sample(void *dp, struct dp_packet *packet, 
> bool steal,
> nl_attr_get_size(subactions), dp_execute_action);
> }
> 
> +static void
> +odp_execute_clone(void *dp, struct dp_packet *packet, bool steal,
> +   const struct nlattr *actions,
> +   odp_execute_cb dp_execute_action)
> +{
> +struct dp_packet_batch pb;
> +
> +if (!steal) {
> +/* The 'subactions' may modify the packet, but the modification

“subsections”->”actions”

> + * should not propagate beyond this clone action. Make a copy
> + * the packet in case we don't own the packet, so that the
> + * 'subactions' are only applid to the clone.  ‘odp_execute_actions'

same

> +   

Re: [ovs-dev] [userspace clone v2 1/4] dpif-netdev: Avoid sending probe packets

[Jarno Rajahalme]

> On Jan 19, 2017, at 11:07 PM, Andy Zhou  wrote:
> 
> When ofproto probe for datapath features, no packets should actually
> be sent to the network. This pactch fixes the userspace by dropping
> probe packets before action execution.
> 
> Signed-off-by: Andy Zhou 
> ---
> lib/dpif-netdev.c | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 3901129..9c21af3 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2660,6 +2660,13 @@ dpif_netdev_execute(struct dpif *dpif, struct 
> dpif_execute *execute)
> }
> }
> 
> +if (execute->probe) {
> +/* If this is part of a probe, Drop the packet, since executing
> + * the action may actually cause spurious packets be sent into
> + * the network. */
> +return 0;
> +}
> +

On the linux kernel datapath the execution can also return an error code, so 
the probe may be inconclusive if we do not execute the actions. Have you 
verified this is not the case for the userspace datapath?

This is why I’d rather have the probes not include actions that can have 
visible side effects.

  Jarno

> /* If the current thread is non-pmd thread, acquires
>  * the 'non_pmd_mutex'. */
> if (pmd->core_id == NON_PMD_CORE_ID) {
> -- 
> 1.9.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] ARP reply does not lead to revalidate

[Ben Pfaff]

On Fri, Jan 20, 2017 at 03:38:11PM +, László Sürü wrote:
> Thanks! It looks working for the first tests.

Thanks.  I applied this to master and the branches back to 2.5.x.

> In plus I propose to add some more printouts to the error cases in
> build_tunnel_send() which helped me to understand the root cause.
> What would be the next step for the correction?

The bug fix is applied.  If you want to propose more logging, then
please post a patch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v10 4/8] ovn: add egress_loopback action

[Ben Pfaff]

On Tue, Jan 17, 2017 at 01:45:05AM -0800, Mickey Spiegel wrote:
> This patch adds an action that loops a clone of the packet back to the
> beginning of the ingress pipeline with logical inport equal to the value
> of the current logical outport.  The following actions are executed on
> the clone:
> 
> clears the connection tracking state
> in_port = 0
> inport = outport
> outport = 0
> flags = 0
> reg0 ... reg9 = 0
> nested actions from inside "{ ... }"
> for example "reg9[1] = 1" to indicate that egress loopback has
> occurred
> executes the ingress pipeline as a subroutine

I think that we can get something equivalent by defining finer-grained
primitives, which is usually my own preference.  I posted a series of
patches to enable that:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327820.html

I believe that, with these patches, egress loopback explained above can
be implemented with:

clone { inport = outport; outport = ""; flags.loopback = 0; 
reg0 = 0; reg1 = 0; ... regN = 0;
next(pipeline=ingress, table=0); }

What do you think?

Thanks,

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

[ovs-dev] [PATCH 10/10] actions: Add new "ct_clear" action.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h |  1 +
 ovn/lib/actions.c | 16 
 ovn/ovn-sb.xml|  4 
 ovn/utilities/ovn-trace.c |  1 +
 tests/ovn.at  |  4 
 5 files changed, 26 insertions(+)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 6691116..1d7bd69 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -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(CT_CLEAR,  ovnact_null)  \
 OVNACT(CLONE, ovnact_nest)  \
 OVNACT(ARP,   ovnact_nest)  \
 OVNACT(ND_NA, ovnact_nest)  \
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 5aef9f9..34c7d2e 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1055,6 +1055,20 @@ ovnact_ct_lb_free(struct ovnact_ct_lb *ct_lb)
 free(ct_lb->dsts);
 }
 
+static void
+format_CT_CLEAR(const struct ovnact_null *null OVS_UNUSED, struct ds *s)
+{
+ds_put_cstr(s, "ct_clear;");
+}
+
+static void
+encode_CT_CLEAR(const struct ovnact_null *null OVS_UNUSED,
+const struct ovnact_encode_params *ep OVS_UNUSED,
+struct ofpbuf *ofpacts)
+{
+ofpact_put_CT_CLEAR(ofpacts);
+}
+
 /* Implements the "arp", "nd_na", and "clone" actions, which execute nested
  * actions on a packet derived from the one being processed. */
 static void
@@ -1749,6 +1763,8 @@ parse_action(struct action_context *ctx)
 parse_CT_SNAT(ctx);
 } else if (lexer_match_id(ctx->lexer, "ct_lb")) {
 parse_ct_lb_action(ctx);
+} else if (lexer_match_id(ctx->lexer, "ct_clear")) {
+ovnact_put_CT_CLEAR(ctx->ovnacts);
 } else if (lexer_match_id(ctx->lexer, "clone")) {
 parse_CLONE(ctx);
 } else if (lexer_match_id(ctx->lexer, "arp")) {
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index d457f9e..53198bc 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1143,6 +1143,10 @@
   
 
 
+ct_clear;
+
+  Clears connection tracking state.
+
 
 clone { action; ... };
 
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 35d0645..d4060b8 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1427,6 +1427,7 @@ trace_actions(const struct ovnact *ovnacts, size_t 
ovnacts_len,
 case OVNACT_CT_DNAT:
 case OVNACT_CT_SNAT:
 case OVNACT_CT_LB:
+case OVNACT_CT_CLEAR:
 ovntrace_node_append(super, OVNTRACE_NODE_ERROR,
  "*** ct_* actions not implemented");
 break;
diff --git a/tests/ovn.at b/tests/ovn.at
index ecdb2be..126574c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -887,6 +887,10 @@ ct_snat(foo, bar);
 ct_snat();
 Syntax error at `)' expecting IPv4 address.
 
+# ct_clear
+ct_clear;
+encodes as ct_clear
+
 # clone
 clone { ip4.dst = 255.255.255.255; output; }; next;
 encodes as 
clone(set_field:255.255.255.255->ip_dst,resubmit(,64)),resubmit(,27)
-- 
2.10.2

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

[ovs-dev] [PATCH 09/10] actions: Make "next" action able to jump from egress to ingress pipeline.

[Ben Pfaff]

This feature is useful for centralized gateways.

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h | 63 +++--
 ovn/controller/lflow.c|  7 +++--
 ovn/lib/actions.c | 72 ---
 ovn/ovn-sb.xml| 12 ++--
 ovn/utilities/ovn-trace.c |  3 ++
 tests/ovn.at  | 22 ++-
 tests/test-ovn.c  |  6 ++--
 7 files changed, 134 insertions(+), 51 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f392d03..6691116 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -151,13 +151,15 @@ struct ovnact_next {
 struct ovnact ovnact;
 
 /* Arguments. */
-uint8_t ltable; /* Logical table ID of next table. */
+uint8_t ltable;/* Logical table ID of next table. */
+enum ovnact_pipeline pipeline; /* Pipeline of next table. */
 
 /* Information about the flow that the action is in.  This does not affect
  * behavior, since the implementation of "next" doesn't depend on the
  * source table or pipeline.  It does affect how ovnacts_format() prints
  * the action. */
-uint8_t src_ltable;/* Logical table ID of source table. */
+uint8_t src_ltable;/* Logical table ID of source table. */
+enum ovnact_pipeline src_pipeline; /* Pipeline of source table. */
 };
 
 /* OVNACT_LOAD. */
@@ -402,22 +404,26 @@ struct ovnact_parse_params {
 /* hmap of 'struct dhcp_opts_map'  to support 'put_dhcpv6_opts' action */
 const struct hmap *dhcpv6_opts;
 
-/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
- * OpenFlow flow table (ptable).  A number of parameters describe this
- * mapping and data related to flow tables:
+/* Each OVN flow exists in a logical table within a logical pipeline.
+ * These parameters express this context for a set of OVN actions being
+ * parsed:
  *
- * - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
- *to which the logical "next" action should be able to jump.
- *Logical table 0 maps to OpenFlow table 'first_ptable', logical
- *table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
- *then "next" is disallowed entirely.
+ * - 'n_tables' is the number of tables in the logical ingress and
+ *egress pipelines, that is, "next" may specify a table less than
+ *or equal to 'n_tables'.  If 'n_tables' is 0 then "next" is
+ *disallowed entirely.
  *
- * - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
- *   cur_ltable < n_tables) of the logical flow that contains the
- *   actions.  If cur_ltable + 1 < n_tables, then this defines the
- *   default table that "next" will jump to. */
-uint8_t n_tables;   /* Number of flow tables. */
-uint8_t cur_ltable; /* 0 <= cur_ltable < n_tables. */
+ * - 'cur_ltable' is the logical table of the current flow, within
+ *   'pipeline'.  If cur_ltable + 1 < n_tables, then this defines the
+ *   default table that "next" will jump to.
+ *
+ * - 'pipeline' is the logical pipeline.  It is the default pipeline to
+ *   which 'next' will jump.  If 'pipeline' is OVNACT_P_EGRESS, then
+ *   'next' will also be able to jump into the ingress pipeline, but
+ *   the reverse is not true. */
+enum ovnact_pipeline pipeline; /* Logical pipeline. */
+uint8_t n_tables;  /* Number of logical flow tables. */
+uint8_t cur_ltable;/* 0 <= cur_ltable < n_tables. */
 };
 
 bool ovnacts_parse(struct lexer *, const struct ovnact_parse_params *,
@@ -448,20 +454,23 @@ struct ovnact_encode_params {
  * OpenFlow flow table (ptable).  A number of parameters describe this
  * mapping and data related to flow tables:
  *
- * - 'first_ptable' and 'n_tables' define the range of OpenFlow tables
- *to which the logical "next" action should be able to jump.
- *Logical table 0 maps to OpenFlow table 'first_ptable', logical
- *table 1 to 'first_ptable + 1', and so on.  If 'n_tables' is 0
- *then "next" is disallowed entirely.
+ * - 'pipeline' is the logical pipeline in which the actions are
+ *   executing.
  *
- * - 'cur_ltable' is an offset from 'first_ptable' (e.g. 0 <=
- *   cur_ltable < n_ptables) of the logical flow that contains the
- *   actions.  If cur_ltable + 1 < n_tables, then this defines the
- *   default table that "next" will jump to.
+ * - 'ingress_ptable' is the OpenFlow table that corresponds to OVN
+ *   ingress table 0.
+ *
+ * - 'egress_ptable' is the OpenFlow table that corresponds to OVN
+ *   egress table 0.
  *
  *   

[ovs-dev] [PATCH 08/10] actions: Introduce enum ovnact_pipeline.

[Ben Pfaff]

This isn't used yet by the actions code, but an upcoming commit will
introduce a user.  This commit just adjusts ovn-trace to use this common
type instead of its own local type.

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h |  6 ++
 ovn/utilities/ovn-trace.c | 42 --
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 38764fe..f392d03 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -140,6 +140,12 @@ struct ovnact_null {
 struct ovnact ovnact;
 };
 
+/* Logical pipeline in which a set of actions is executed. */
+enum ovnact_pipeline {
+OVNACT_P_INGRESS,
+OVNACT_P_EGRESS,
+};
+
 /* OVNACT_NEXT. */
 struct ovnact_next {
 struct ovnact ovnact;
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 982a81a..654b8d0 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -322,11 +322,9 @@ struct ovntrace_mcgroup {
 size_t n_ports;
 };
 
-enum ovntrace_pipeline { P_INGRESS, P_EGRESS };
-
 struct ovntrace_flow {
 struct uuid uuid;
-enum ovntrace_pipeline pipeline;
+enum ovnact_pipeline pipeline;
 int table_id;
 char *stage_name;
 char *source;
@@ -632,8 +630,8 @@ compare_flow(const void *a_, const void *b_)
 const struct ovntrace_flow *b = *bp;
 
 if (a->pipeline != b->pipeline) {
-/* Sort P_INGRESS before P_EGRESS. */
-return a->pipeline == P_EGRESS ? 1 : -1;
+/* Sort OVNACT_P_INGRESS before OVNACT_P_EGRESS. */
+return a->pipeline == OVNACT_P_EGRESS ? 1 : -1;
 } else if (a->table_id != b->table_id) {
 /* Sort in increasing order of table_id. */
 return a->table_id > b->table_id ? 1 : -1;
@@ -706,8 +704,8 @@ read_flows(void)
 struct ovntrace_flow *flow = xzalloc(sizeof *flow);
 flow->uuid = sblf->header_.uuid;
 flow->pipeline = (!strcmp(sblf->pipeline, "ingress")
-  ? P_INGRESS
-  : P_EGRESS);
+  ? OVNACT_P_INGRESS
+  : OVNACT_P_EGRESS);
 flow->table_id = sblf->table_id;
 flow->stage_name = nullable_xstrdup(smap_get(>external_ids,
  "stage-name"));
@@ -835,7 +833,7 @@ ovntrace_lookup_port(const void *dp_, const char *port_name,
 static const struct ovntrace_flow *
 ovntrace_flow_lookup(const struct ovntrace_datapath *dp,
  const struct flow *uflow,
- uint8_t table_id, enum ovntrace_pipeline pipeline)
+ uint8_t table_id, enum ovnact_pipeline pipeline)
 {
 for (size_t i = 0; i < dp->n_flows; i++) {
 const struct ovntrace_flow *flow = dp->flows[i];
@@ -850,7 +848,7 @@ ovntrace_flow_lookup(const struct ovntrace_datapath *dp,
 
 static char *
 ovntrace_stage_name(const struct ovntrace_datapath *dp,
-uint8_t table_id, enum ovntrace_pipeline pipeline)
+uint8_t table_id, enum ovnact_pipeline pipeline)
 {
 for (size_t i = 0; i < dp->n_flows; i++) {
 const struct ovntrace_flow *flow = dp->flows[i];
@@ -1100,17 +1098,17 @@ execute_exchange(const struct ovnact_move *move, struct 
flow *uflow,
 
 static void
 trace__(const struct ovntrace_datapath *dp, struct flow *uflow,
-uint8_t table_id, enum ovntrace_pipeline pipeline,
+uint8_t table_id, enum ovnact_pipeline pipeline,
 struct ovs_list *super);
 
 static void
 trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
   const struct ovntrace_datapath *dp, struct flow *uflow,
-  uint8_t table_id, enum ovntrace_pipeline pipeline,
+  uint8_t table_id, enum ovnact_pipeline pipeline,
   struct ovs_list *super);
 static void
 execute_output(const struct ovntrace_datapath *dp, struct flow *uflow,
-   enum ovntrace_pipeline pipeline, struct ovs_list *super)
+   enum ovnact_pipeline pipeline, struct ovs_list *super)
 {
 uint16_t key = uflow->regs[MFF_LOG_OUTPORT - MFF_REG0];
 if (!key) {
@@ -1131,7 +1129,7 @@ execute_output(const struct ovntrace_datapath *dp, struct 
flow *uflow,
  key);
 }
 
-if (pipeline == P_EGRESS) {
+if (pipeline == OVNACT_P_EGRESS) {
 ovntrace_node_append(super, OVNTRACE_NODE_OUTPUT,
  "/* output to \"%s\", type \"%s\" */",
  out_name, port ? port->type : "");
@@ -1146,7 +1144,7 @@ execute_output(const struct ovntrace_datapath *dp, struct 
flow *uflow,
 struct flow new_uflow = *uflow;
 new_uflow.regs[MFF_LOG_INPORT - MFF_REG0] = peer->tunnel_key;
 new_uflow.regs[MFF_LOG_OUTPORT - MFF_REG0] = 0;
-trace__(peer->dp, _uflow, 0, P_INGRESS, >subs);
+trace__(peer->dp, _uflow, 0, OVNACT_P_INGRESS, >subs);

[ovs-dev] [PATCH 07/10] actions: Omit table number when possible for formatting "next" action.

[Ben Pfaff]

Until now, formatting the "next" action has always required including
the table number, because the action struct didn't include enough context
so that the formatter could decide whether the table number was the next
table or some other table.  This is more or less OK, but an upcoming commit
will add a "pipeline" field to the "next" action, which means that the same
policy there would require that the pipeline always be printed.  That's a
little obnoxious because 99+% of the time, the pipeline to be printed is
the same pipeline that the flow is in and printing it would be distracting.
So it's better to store some context to help with formatting.  This commit
begins adopting that policy for the existing table number field.

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h |  8 
 ovn/lib/actions.c | 29 +++--
 tests/ovn.at  |  8 
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 92c..38764fe 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -143,7 +143,15 @@ struct ovnact_null {
 /* OVNACT_NEXT. */
 struct ovnact_next {
 struct ovnact ovnact;
+
+/* Arguments. */
 uint8_t ltable; /* Logical table ID of next table. */
+
+/* Information about the flow that the action is in.  This does not affect
+ * behavior, since the implementation of "next" doesn't depend on the
+ * source table or pipeline.  It does affect how ovnacts_format() prints
+ * the action. */
+uint8_t src_ltable;/* Logical table ID of source table. */
 };
 
 /* OVNACT_LOAD. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 2162dad..5548234 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -259,7 +259,11 @@ parse_NEXT(struct action_context *ctx)
 {
 if (!ctx->pp->n_tables) {
 lexer_error(ctx->lexer, "\"next\" action not allowed here.");
-} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+return;
+}
+
+int table = ctx->pp->cur_ltable + 1;
+if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
 int ltable;
 
 if (!lexer_force_int(ctx->lexer, ) ||
@@ -273,22 +277,27 @@ parse_NEXT(struct action_context *ctx)
  ctx->pp->n_tables - 1);
 return;
 }
+}
 
-ovnact_put_NEXT(ctx->ovnacts)->ltable = ltable;
-} else {
-if (ctx->pp->cur_ltable < ctx->pp->n_tables) {
-ovnact_put_NEXT(ctx->ovnacts)->ltable = ctx->pp->cur_ltable + 1;
-} else {
-lexer_error(ctx->lexer,
-"\"next\" action not allowed in last table.");
-}
+if (table >= ctx->pp->n_tables) {
+lexer_error(ctx->lexer,
+"\"next\" action cannot advance beyond table %d.",
+ctx->pp->n_tables - 1);
 }
+
+struct ovnact_next *next = ovnact_put_NEXT(ctx->ovnacts);
+next->ltable = table;
+next->src_ltable = ctx->pp->cur_ltable;
 }
 
 static void
 format_NEXT(const struct ovnact_next *next, struct ds *s)
 {
-ds_put_format(s, "next(%d);", next->ltable);
+if (next->ltable != next->src_ltable + 1) {
+ds_put_format(s, "next(%d);", next->ltable);
+} else {
+ds_put_cstr(s, "next;");
+}
 }
 
 static void
diff --git a/tests/ovn.at b/tests/ovn.at
index 67d73c5..f71a4af 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -643,9 +643,9 @@ output;
 
 # next
 next;
-formats as next(11);
 encodes as resubmit(,27)
 next(11);
+formats as next;
 encodes as resubmit(,27)
 next(0);
 encodes as resubmit(,16)
@@ -657,7 +657,7 @@ next();
 next(10;
 Syntax error at `;' expecting `)'.
 next(16);
-"next" argument must be in range 0 to 15.
+"next" action cannot advance beyond table 15.
 
 # Loading a constant value.
 tcp.dst=80;
@@ -678,7 +678,7 @@ ip.ttl=4;
 encodes as set_field:4->nw_ttl
 has prereqs eth.type == 0x800 || eth.type == 0x86dd
 outport="eth0"; next; outport="LOCAL"; next;
-formats as outport = "eth0"; next(11); outport = "LOCAL"; next(11);
+formats as outport = "eth0"; next; outport = "LOCAL"; next;
 encodes as 
set_field:0x5->reg15,resubmit(,27),set_field:0xfffe->reg15,resubmit(,27)
 
 inport[1] = 1;
@@ -868,7 +868,7 @@ ct_snat();
 Syntax error at `)' expecting IPv4 address.
 
 # clone
-clone { ip4.dst = 255.255.255.255; output; }; next(11);
+clone { ip4.dst = 255.255.255.255; output; }; next;
 encodes as 
clone(set_field:255.255.255.255->ip_dst,resubmit(,64)),resubmit(,27)
 has prereqs eth.type == 0x800
 
-- 
2.10.2

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

[ovs-dev] [PATCH 06/10] actions: Separate action structures for "next" and "ct_next".

[Ben Pfaff]

These actions aren't very similar but until now they both had the same
action structure.  These structures are going to diverge in an upcoming
commit, so separate them now.

Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h | 10 --
 ovn/lib/actions.c | 11 ---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index a1b6b90..92c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -55,7 +55,7 @@ struct simap;
 OVNACT(MOVE,  ovnact_move)  \
 OVNACT(EXCHANGE,  ovnact_move)  \
 OVNACT(DEC_TTL,   ovnact_null)  \
-OVNACT(CT_NEXT,   ovnact_next)  \
+OVNACT(CT_NEXT,   ovnact_ct_next)   \
 OVNACT(CT_COMMIT, ovnact_ct_commit) \
 OVNACT(CT_DNAT,   ovnact_ct_nat)\
 OVNACT(CT_SNAT,   ovnact_ct_nat)\
@@ -140,7 +140,7 @@ struct ovnact_null {
 struct ovnact ovnact;
 };
 
-/* OVNACT_NEXT, OVNACT_CT_NEXT. */
+/* OVNACT_NEXT. */
 struct ovnact_next {
 struct ovnact ovnact;
 uint8_t ltable; /* Logical table ID of next table. */
@@ -160,6 +160,12 @@ struct ovnact_move {
 struct expr_field rhs;
 };
 
+/* OVNACT_CT_NEXT. */
+struct ovnact_ct_next {
+struct ovnact ovnact;
+uint8_t ltable;/* Logical table ID of next table. */
+};
+
 /* OVNACT_CT_COMMIT. */
 struct ovnact_ct_commit {
 struct ovnact ovnact;
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 213e22e..2162dad 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -530,24 +530,29 @@ parse_CT_NEXT(struct action_context *ctx)
 }
 
 static void
-format_CT_NEXT(const struct ovnact_next *next OVS_UNUSED, struct ds *s)
+format_CT_NEXT(const struct ovnact_ct_next *ct_next OVS_UNUSED, struct ds *s)
 {
 ds_put_cstr(s, "ct_next;");
 }
 
 static void
-encode_CT_NEXT(const struct ovnact_next *next,
+encode_CT_NEXT(const struct ovnact_ct_next *ct_next,
 const struct ovnact_encode_params *ep,
 struct ofpbuf *ofpacts)
 {
 struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
-ct->recirc_table = ep->first_ptable + next->ltable;
+ct->recirc_table = ep->first_ptable + ct_next->ltable;
 ct->zone_src.field = ep->is_switch ? mf_from_id(MFF_LOG_CT_ZONE)
 : mf_from_id(MFF_LOG_DNAT_ZONE);
 ct->zone_src.ofs = 0;
 ct->zone_src.n_bits = 16;
 ofpact_finish(ofpacts, >ofpact);
 }
+
+static void
+ovnact_ct_next_free(struct ovnact_ct_next *a OVS_UNUSED)
+{
+}
 
 static void
 parse_ct_commit_arg(struct action_context *ctx,
-- 
2.10.2

___
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".

[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, 

[ovs-dev] [PATCH 04/10] actions: Make "free" functions per-struct, not per-action.

[Ben Pfaff]

In some cases multiple kinds of OVN action share the same structure.  In
all of these cases, a given kind of structure is freed one particular way
(it would be confusing if this were not the case), so there's no benefit
in having per-action free functions.  Therefore, this commit switches to
a free function per structure type.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 93 ---
 1 file changed, 20 insertions(+), 73 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index f1faab3..7c5a292 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -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.
@@ -45,7 +45,7 @@ VLOG_DEFINE_THIS_MODULE(actions);
 static void encode_##ENUM(const struct STRUCT *,\
   const struct ovnact_encode_params *,  \
   struct ofpbuf *ofpacts);  \
-static void free_##ENUM(struct STRUCT *a);
+static void STRUCT##_free(struct STRUCT *a);
 OVNACTS
 #undef OVNACT
 
@@ -226,6 +226,11 @@ add_prerequisite(struct action_context *ctx, const char 
*prerequisite)
 ovs_assert(!error);
 ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
+
+static void
+ovnact_null_free(struct ovnact_null *a OVS_UNUSED)
+{
+}
 
 static void
 format_OUTPUT(const struct ovnact_null *a OVS_UNUSED, struct ds *s)
@@ -248,11 +253,6 @@ encode_OUTPUT(const struct ovnact_null *a OVS_UNUSED,
 {
 emit_resubmit(ofpacts, ep->output_ptable);
 }
-
-static void
-free_OUTPUT(struct ovnact_null *a OVS_UNUSED)
-{
-}
 
 static void
 parse_NEXT(struct action_context *ctx)
@@ -300,7 +300,7 @@ encode_NEXT(const struct ovnact_next *next,
 }
 
 static void
-free_NEXT(struct ovnact_next *a OVS_UNUSED)
+ovnact_next_free(struct ovnact_next *a OVS_UNUSED)
 {
 }
 
@@ -374,7 +374,7 @@ encode_LOAD(const struct ovnact_load *load,
 }
 
 static void
-free_LOAD(struct ovnact_load *load)
+ovnact_load_free(struct ovnact_load *load)
 {
 expr_constant_destroy(>imm, load_type(load));
 }
@@ -490,12 +490,7 @@ encode_EXCHANGE(const struct ovnact_move *xchg,
 }
 
 static void
-free_MOVE(struct ovnact_move *move OVS_UNUSED)
-{
-}
-
-static void
-free_EXCHANGE(struct ovnact_move *xchg OVS_UNUSED)
+ovnact_move_free(struct ovnact_move *move OVS_UNUSED)
 {
 }
 
@@ -520,11 +515,6 @@ encode_DEC_TTL(const struct ovnact_null *null OVS_UNUSED,
 {
 ofpact_put_DEC_TTL(ofpacts);
 }
-
-static void
-free_DEC_TTL(struct ovnact_null *null OVS_UNUSED)
-{
-}
 
 static void
 parse_CT_NEXT(struct action_context *ctx)
@@ -558,11 +548,6 @@ encode_CT_NEXT(const struct ovnact_next *next,
 ct->zone_src.n_bits = 16;
 ofpact_finish(ofpacts, >ofpact);
 }
-
-static void
-free_CT_NEXT(struct ovnact_next *next OVS_UNUSED)
-{
-}
 
 static void
 parse_ct_commit_arg(struct action_context *ctx,
@@ -680,7 +665,7 @@ encode_CT_COMMIT(const struct ovnact_ct_commit *cc,
 }
 
 static void
-free_CT_COMMIT(struct ovnact_ct_commit *cc OVS_UNUSED)
+ovnact_ct_commit_free(struct ovnact_ct_commit *cc OVS_UNUSED)
 {
 }
 
@@ -819,12 +804,7 @@ encode_CT_SNAT(const struct ovnact_ct_nat *cn,
 }
 
 static void
-free_CT_DNAT(struct ovnact_ct_nat *ct_nat OVS_UNUSED)
-{
-}
-
-static void
-free_CT_SNAT(struct ovnact_ct_nat *ct_nat OVS_UNUSED)
+ovnact_ct_nat_free(struct ovnact_ct_nat *ct_nat OVS_UNUSED)
 {
 }
 
@@ -1016,7 +996,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
 }
 
 static void
-free_CT_LB(struct ovnact_ct_lb *ct_lb)
+ovnact_ct_lb_free(struct ovnact_ct_lb *ct_lb)
 {
 free(ct_lb->dsts);
 }
@@ -1133,24 +1113,13 @@ encode_ND_NA(const struct ovnact_nest *on,
 encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts);
 }
 
+
 static void
-free_nested_actions(struct ovnact_nest *on)
+ovnact_nest_free(struct ovnact_nest *on)
 {
 ovnacts_free(on->nested, on->nested_len);
 free(on->nested);
 }
-
-static void
-free_ARP(struct ovnact_nest *nest)
-{
-free_nested_actions(nest);
-}
-
-static void
-free_ND_NA(struct ovnact_nest *nest)
-{
-free_nested_actions(nest);
-}
 
 static void
 parse_get_mac_bind(struct action_context *ctx, int width,
@@ -1221,12 +1190,7 @@ encode_GET_ND(const struct ovnact_get_mac_bind *get_mac,
 }
 
 static void
-free_GET_ARP(struct ovnact_get_mac_bind *get_mac OVS_UNUSED)
-{
-}
-
-static void
-free_GET_ND(struct ovnact_get_mac_bind *get_mac OVS_UNUSED)
+ovnact_get_mac_bind_free(struct ovnact_get_mac_bind *get_mac OVS_UNUSED)
 {
 }
 
@@ -1300,12 +1264,7 @@ encode_PUT_ND(const struct ovnact_put_mac_bind *put_mac,
 }
 
 static void
-free_PUT_ARP(struct ovnact_put_mac_bind *put_mac OVS_UNUSED)
-{
-}
-
-static void
-free_PUT_ND(struct ovnact_put_mac_bind *put_mac OVS_UNUSED)
+ovnact_put_mac_bind_free(struct ovnact_put_mac_bind *put_mac OVS_UNUSED)
 

[ovs-dev] [PATCH 02/10] lex: Make lexer_force_match() work for LEX_T_END.

[Ben Pfaff]

Without this change, lexer_force_match(lex, LEX_T_END) mostly works, except
that in the failure case it emits an error that says "expecting `$'",
which is a surprising error message.

Arguably, lexer_force_end() could be removed entirely, but I don't see a
real problem with the existing arrangement.

Signed-off-by: Ben Pfaff 
---
 include/ovn/lex.h |  4 ++--
 ovn/lib/lex.c | 13 +
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/ovn/lex.h b/include/ovn/lex.h
index 0876d1b..afa4a23 100644
--- a/include/ovn/lex.h
+++ b/include/ovn/lex.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.
@@ -139,7 +139,7 @@ bool lexer_is_int(const struct lexer *);
 bool lexer_get_int(struct lexer *, int *value);
 bool lexer_force_int(struct lexer *, int *value);
 
-void lexer_force_end(struct lexer *);
+bool lexer_force_end(struct lexer *);
 
 void lexer_error(struct lexer *, const char *message, ...)
 OVS_PRINTF_FORMAT(2, 3);
diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c
index d64ce83..4b504cb 100644
--- a/ovn/lib/lex.c
+++ b/ovn/lib/lex.c
@@ -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.
@@ -856,7 +856,9 @@ lexer_match(struct lexer *lexer, enum lex_type type)
 bool
 lexer_force_match(struct lexer *lexer, enum lex_type t)
 {
-if (lexer_match(lexer, t)) {
+if (t == LEX_T_END) {
+return lexer_force_end(lexer);
+} else if (lexer_match(lexer, t)) {
 return true;
 } else {
 struct lex_token token = { .type = t };
@@ -915,11 +917,14 @@ lexer_force_int(struct lexer *lexer, int *value)
 return ok;
 }
 
-void
+bool
 lexer_force_end(struct lexer *lexer)
 {
-if (lexer->token.type != LEX_T_END) {
+if (lexer->token.type == LEX_T_END) {
+return true;
+} else {
 lexer_syntax_error(lexer, "expecting end of input");
+return false;
 }
 }
 
-- 
2.10.2

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

[ovs-dev] [PATCH 03/10] actions: Make "arp { drop; }; " acceptable.

[Ben Pfaff]

Before this commit, the OVN action parser would accept "arp {};" and then
the formatter would format it back as "arp { drop; };", but the parser
didn't accept the latter.  There were basically two choices: make the
parser accept "arp { drop; };" or make the formatter output "arp {};"
(or both).  This patch does (only) the former, and adds a test to avoid
regression.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 16 ++--
 tests/ovn.at  |  4 
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 5770488..f1faab3 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -179,7 +179,7 @@ struct action_context {
 struct expr *prereqs;   /* Prerequisites to apply to match. */
 };
 
-static bool parse_action(struct action_context *);
+static void parse_actions(struct action_context *, enum lex_type sentinel);
 
 static bool
 action_parse_field(struct action_context *ctx,
@@ -1040,11 +1040,7 @@ parse_nested_action(struct action_context *ctx, enum 
ovnact_type type,
 .ovnacts = ,
 .prereqs = NULL
 };
-while (!lexer_match(ctx->lexer, LEX_T_RCURLY)) {
-if (!parse_action(_ctx)) {
-break;
-}
-}
+parse_actions(_ctx, LEX_T_RCURLY);
 
 /* XXX Not really sure what we should do with prerequisites for nested
  * actions. */
@@ -1743,7 +1739,7 @@ parse_action(struct action_context *ctx)
 }
 
 static void
-parse_actions(struct action_context *ctx)
+parse_actions(struct action_context *ctx, enum lex_type sentinel)
 {
 /* "drop;" by itself is a valid (empty) set of actions, but it can't be
  * combined with other actions because that doesn't make sense. */
@@ -1752,11 +1748,11 @@ parse_actions(struct action_context *ctx)
 && lexer_lookahead(ctx->lexer) == LEX_T_SEMICOLON) {
 lexer_get(ctx->lexer);  /* Skip "drop". */
 lexer_get(ctx->lexer);  /* Skip ";". */
-lexer_force_end(ctx->lexer);
+lexer_force_match(ctx->lexer, sentinel);
 return;
 }
 
-while (ctx->lexer->token.type != LEX_T_END) {
+while (!lexer_match(ctx->lexer, sentinel)) {
 if (!parse_action(ctx)) {
 return;
 }
@@ -1791,7 +1787,7 @@ ovnacts_parse(struct lexer *lexer, const struct 
ovnact_parse_params *pp,
 .prereqs = NULL,
 };
 if (!lexer->error) {
-parse_actions();
+parse_actions(, LEX_T_END);
 }
 
 if (!lexer->error) {
diff --git a/tests/ovn.at b/tests/ovn.at
index bc915c6..a514ebbd 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -871,6 +871,10 @@ ct_snat();
 arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
 encodes as 
controller(userdata=00.00.00.00.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
+arp { };
+formats as arp { drop; };
+encodes as controller(userdata=00.00.00.00.00.00.00.00)
+has prereqs ip4
 
 # get_arp
 get_arp(outport, ip4.dst);
-- 
2.10.2

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

[ovs-dev] [PATCH 00/10] Add actions for egress loopback

[Ben Pfaff]

I believe that, with these patches, egress loopback as proposed by Mickey's
patches can be implemented with:
clone { inport = outport; outport = ""; flags.loopback = 0; 
reg0 = 0; reg1 = 0; ... regN = 0;
next(pipeline=ingress, table=0); }

Ben Pfaff (10):
  actions: Fix "arp" and "nd_na" followed by another action.
  lex: Make lexer_force_match() work for LEX_T_END.
  actions: Make "arp { drop; };" acceptable.
  actions: Make "free" functions per-struct, not per-action.
  actions: Add new OVN action "clone".
  actions: Separate action structures for "next" and "ct_next".
  actions: Omit table number when possible for formatting "next" action.
  actions: Introduce enum ovnact_pipeline.
  actions: Make "next" action able to jump from egress to ingress
pipeline.
  actions: Add new "ct_clear" action.

 include/ovn/actions.h |  91 ++-
 include/ovn/lex.h |   4 +-
 ovn/controller/lflow.c|   7 +-
 ovn/lib/actions.c | 284 +++---
 ovn/lib/lex.c |  13 ++-
 ovn/ovn-sb.xml|  26 -
 ovn/utilities/ovn-trace.c |  65 +++
 tests/ovn.at  |  45 +++-
 tests/test-ovn.c  |   6 +-
 9 files changed, 352 insertions(+), 189 deletions(-)

-- 
2.10.2

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

[ovs-dev] [PATCH 01/10] actions: Fix "arp" and "nd_na" followed by another action.

[Ben Pfaff]

OVN logical actions are supposed to be padded to a multiple of 8 bytes,
but the code for parsing "arp" and "nd_na" actions didn't do this properly.
The result was that it worked OK if one of these actions was the last one
in a sequence of logical actions, but failed badly if they were in the
middle.  This commit fixes the problem, adds assertions to make it harder
for the problem to recur, and adds a test.

Signed-off-by: Ben Pfaff 
---
 ovn/lib/actions.c | 8 +---
 tests/ovn.at  | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 686ecc5..5770488 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -55,10 +55,10 @@ OVNACTS
 void *
 ovnact_put(struct ofpbuf *ovnacts, enum ovnact_type type, size_t len)
 {
-struct ovnact *ovnact;
+ovs_assert(len == OVNACT_ALIGN(len));
 
 ovnacts->header = ofpbuf_put_uninit(ovnacts, len);
-ovnact = ovnacts->header;
+struct ovnact *ovnact = ovnacts->header;
 ovnact_init(ovnact, type, len);
 return ovnact;
 }
@@ -67,6 +67,7 @@ ovnact_put(struct ofpbuf *ovnacts, enum ovnact_type type, 
size_t len)
 void
 ovnact_init(struct ovnact *ovnact, enum ovnact_type type, size_t len)
 {
+ovs_assert(len == OVNACT_ALIGN(len));
 memset(ovnact, 0, len);
 ovnact->type = type;
 ovnact->len = len;
@@ -1057,7 +1058,8 @@ parse_nested_action(struct action_context *ctx, enum 
ovnact_type type,
 
 add_prerequisite(ctx, prereq);
 
-struct ovnact_nest *on = ovnact_put(ctx->ovnacts, type, sizeof *on);
+struct ovnact_nest *on = ovnact_put(ctx->ovnacts, type,
+OVNACT_ALIGN(sizeof *on));
 on->nested_len = nested.size;
 on->nested = ofpbuf_steal_data();
 }
diff --git a/tests/ovn.at b/tests/ovn.at
index 1792956..bc915c6 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -868,8 +868,8 @@ ct_snat();
 Syntax error at `)' expecting IPv4 address.
 
 # arp
-arp { eth.dst = ff:ff:ff:ff:ff:ff; output; };
-encodes as 
controller(userdata=00.00.00.00.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)
+arp { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
+encodes as 
controller(userdata=00.00.00.00.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
 
 # get_arp
-- 
2.10.2

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

Re: [ovs-dev] ARP reply does not lead to revalidate

[László Sürü]

Thanks! It looks working for the first tests.
In plus I propose to add some more printouts to the error cases in 
build_tunnel_send() which helped me to understand the root cause.
What would be the next step for the correction?

Best regards
Laszlo

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Thursday, January 19, 2017 6:35 PM
To: László Sürü 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] ARP reply does not lead to revalidate

On Thu, Jan 19, 2017 at 04:04:10PM +, László Sürü wrote:
> Hi,
> 
> We are using OVS 2.6.1 to connect VMs via GRE tunneling in OpenStack 
> environment.
> The bridge configuration is similar to proposed userspace tunneling at 
> https://github.com/openvswitch/ovs/blob/master/Documentation/howto/use
> rspace-tunneling.rst The 2 bridges are of netdev type as below.
> - br-int contains OF rules to forward packets from VM to GRE tunnel and 
> vice-versa.
> - br-phy is MAC learning switch.
> 
> In the tunnel configuration local and remote IPs are in different IP subnets:
> The use case is about to ping from one VM to another one using MPLS 
> encapsulated in GRE tunnel in between 2 computes via a gateway.
> The expected packet path is
> VM ->  br-int ->br-phy LOCAL ->out
> 
> However, for the first ping this doesn't work in case the tunnel 
> encapsulation cannot lookup the destination IP in the ARP cache.
> ofproto-dpif-xlate.c: build_tunnel_send();
> "neighbor cache miss for 10.85.59.49 on bridge br-phy, sending ARP 
> request"
> As a consequence, the tunnel_push action cannot be added to the flow, 
> instead the dpif flow will be truncated missing the tnl_push action.
> The ARP reply to the previous request arrives normally, but it doesn't lead 
> to revalidation in datapath.
> 
> This results no packet out from the VM for some random number of 
> minutes, when finally dpif revalidation takes place for some other 
> reason
> 
> Have you experienced similar behavior?
> How the tunnel push supposed to work in such configuration?
> I expect the ARP reply to revalidate dpif flows in this case.

Thanks for the report.

Does the following patch fix the problem?

diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c index 
499efff..1f2b599 100644
--- a/lib/tnl-neigh-cache.c
+++ b/lib/tnl-neigh-cache.c
@@ -126,8 +126,8 @@ tnl_neigh_set__(const char name[IFNAMSIZ], const struct 
in6_addr *dst,
 return;
 }
 tnl_neigh_delete(neigh);
-seq_change(tnl_conf_seq);
 }
+seq_change(tnl_conf_seq);
 
 neigh = xmalloc(sizeof *neigh);
 

Thanks,

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

Re: [ovs-dev] [PATCH 0/1] dpif-netdev: Conditional EMC insert

[Loftus, Ciara]

> 
> On 01/12/2017 04:49 PM, Ciara Loftus wrote:
> > This patch is part of the OVS-DPDK performance optimizations presented
> > on the OVS fall conference
> > (http://openvswitch.org/support/ovscon2016/8/1400-gray.pdf)
> >
> > The Exact Match Cache does not perform well in use cases with a high
> > numbers of parallel packet flows. When the flow count exceeds 8k, which
> > is the size of the EMC, 'thrashing' occurs. Subsequent packets incur
> > EMC misses and lead to the insertion of new entries, which are likely
> > already overwritten by the time the next packet of a flow arrives.
> >
> > The extra cost of useless EMC insertions and failed EMC lookups degrades
> > the performance of the netdev-dpdk datapath up to 30% compared to the
> > pure performance of the dpcls classifier with EMC disabled. Profiling
> > has shown that the bulk of the degradation is caused by the EMC
> > insertions.
> >
> > To avoid this degradation we apply 'probabilistic' EMC insertions, whereby
> > an EMC miss only results in an EMC insertion with a random probability
> > of P=1/N (N integer). With this model, the insertion overhead of the EMC
> > can be reduced by a factor N, while the EMC speedup is maintained for a
> > small to medium number of flows. Probabilistic insertion can be
> > implemented with minimal run-time cost and naturally favors long-lived
> > flows.
> >
> > Different values for P from 1/100 to 1/4000 were validated and
> benchmarked
> > when creating this patch. Not much variance between different
> probabilities
> > was observed.
> >
> > We chose P=1/100 because it gives almost the same improvement as lower
> > probabilities while reaching the EMC capacity and thus optimal
> performance
> > quicker than the lower probabilities.
> >
> > For P=1/100, the EMC reached full capacity after 843ms when subjecting
> the
> > datapath with 10 long-lived flows at 14.8 Mpps. This is much quicker
> > compared to P=1/500 and P=1/1000, which took 3792ms and 7370ms
> > respectively.
> 
> The performance results are very impressive - it looks like ~50%
> performance improvement after about 10K flows.
> 
> Did you measure any negative effects when the the emc is not full?

Hi Kevin,

When the EMC is not full the impact is negligible.
For example I tested with 1, 128 & 1024 flows and measured a -0.04% degradation 
and +2% and +1% improvement with the patch.

> 
> What about about only using this type of scheme once the emc is full or
> above a certain threshold?

Given the above results I don't think implementing something like that is 
necessary. The additional logic could be what ends up degrading performance.

Thanks,
Ciara

> 
> Kevin.
> 
> >
> >  lib/dpif-netdev.c | 20 ++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >

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

[ovs-dev] [PATCH v2 1/2] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

[Zoltán Balogh]

From: Sugesh Chandran 

Openvswitch datapath recirculates packets for tunneling, i.e.
the incoming packets are encapsulated at first pass. Further actions are
applied on encapsulated packets on the second pass after recirculating.
The proposed patch compute and append the post tunnel actions at the time of
translation itself instead of recirculating at datapath. These actions are 
solely
depends on tunnel attributes so there is no need of datapath recirculation.
By avoiding the recirculation at datapath, the patch offers upto 30%
performance improvement for VxLAN tunneling in our testing.
The action execution logic is also extended with new CLONE action to define
the packet cloning when the actions are combined. The lenght in the CLONE
action specifies the size of nested action set.

Signed-off-by: Sugesh Chandran 
Signed-off-by: Zoltán Balogh 
Co-authored-by: Zoltán Balogh 
---
 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  20 +-
 lib/dpif.c|   1 +
 lib/odp-execute.c |  57 -
 lib/odp-util.c|  96 +++-
 lib/odp-util.h|   5 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  | 267 +++---
 8 files changed, 288 insertions(+), 160 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8..91849d6 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -818,6 +818,7 @@ enum ovs_action_attr {
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_CLONE,
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3901129..0b85fe4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4547,24 +4547,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
 if (*depth < MAX_RECIRC_DEPTH) {
-struct dp_packet_batch tnl_pkt;
-struct dp_packet_batch *orig_packets_ = packets_;
-int err;
-
-if (!may_steal) {
-dp_packet_batch_clone(_pkt, packets_);
-packets_ = _pkt;
-dp_packet_batch_reset_cutlen(orig_packets_);
-}
-
 dp_packet_batch_apply_cutlen(packets_);
-
-err = push_tnl_action(pmd, a, packets_);
-if (!err) {
-(*depth)++;
-dp_netdev_recirculate(pmd, packets_);
-(*depth)--;
-}
+push_tnl_action(pmd, a, packets_);
 return;
 }
 break;
@@ -4714,6 +4698,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 break;
 }
 
+case OVS_ACTION_ATTR_CLONE:
+break;
 case OVS_ACTION_ATTR_PUSH_VLAN:
 case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_PUSH_MPLS:
diff --git a/lib/dpif.c b/lib/dpif.c
index 50c3382..b698da2 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1183,6 +1183,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_ACTION_ATTR_SAMPLE:
 case OVS_ACTION_ATTR_TRUNC:
 case OVS_ACTION_ATTR_UNSPEC:
+case OVS_ACTION_ATTR_CLONE:
 case __OVS_ACTION_ATTR_MAX:
 OVS_NOT_REACHED();
 }
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 73e1016..77bca94 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -541,6 +541,7 @@ requires_datapath_assistance(const struct nlattr *a)
 case OVS_ACTION_ATTR_USERSPACE:
 case OVS_ACTION_ATTR_RECIRC:
 case OVS_ACTION_ATTR_CT:
+case OVS_ACTION_ATTR_CLONE:
 return true;
 
 case OVS_ACTION_ATTR_SET:
@@ -562,6 +563,29 @@ requires_datapath_assistance(const struct nlattr *a)
 return false;
 }
 
+static inline size_t
+find_combine_action_end(const struct nlattr **actions_, size_t *actions_len,
+const struct nlattr **comb_start,
+bool *comb_last_action)
+{
+const struct nlattr *a;
+const struct nlattr *actions = *actions_;
+unsigned int left;
+bool last_action;
+size_t comb_size = nl_attr_get_u32(actions); /* Size of combined actions */
+*actions_len -= NLA_ALIGN(actions->nla_len);
+actions = nl_attr_next(actions);
+
+*comb_start = actions;
+a = (void *) ((uint8_t *) actions + NLA_ALIGN(comb_size));
+left = (*actions_len) - comb_size;
+last_action = (left <= 

[ovs-dev] [PATCH v2 2/2] Fix test suite failures on tunneling CLONE action.

[Zoltán Balogh]

From: Sugesh Chandran 

Following tunneling specific test cases are failing due to the introduction of
new CLONE action for the tunnel push.

769: tunnel_push_pop_ipv6 - action   FAILED 
(tunnel-push-pop-ipv6.at:92)
767: tunnel_push_pop - actionFAILED 
(tunnel-push-pop.at:97)
1097: ofproto-dpif - sFlow packet sampling - tunnel push FAILED 
(ofproto-dpif.at:5965)
2248: ovn -- 3 HVs, 1 LS, 3 lports/HV FAILED (ovn.at:1281)
2253: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED (ovn.at:2454)

This patch fixes test cases 767, 769 and 1097.

[Zoltán Balogh] After applying this patch from Sugesh, I found further test 
cases do fail.

768: tunnel_push_pop - packet_outFAILED 
(tunnel-push-pop.at:203)
2261: ovn -- 3 HVs, 1 LS, 3 lports/HV FAILED (ovn.at:1295)
2266: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED (ovn.at:2468)
2268: ovn -- 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs  FAILED (ovn.at:2979)
2298: ovn -- 1 LR with distributed router gateway port FAILED (ovn.at:6480)

Signed-off-by: Sugesh Chandran 
Signed-off-by: Zoltán Balogh 
Co-authored-by: Zoltán Balogh 
---
 tests/ofproto-dpif.at | 11 +--
 tests/tunnel-push-pop-ipv6.at | 10 +-
 tests/tunnel-push-pop.at  | 13 ++---
 3 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index a4bf5a3..945a415 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5971,15 +5971,6 @@ HEADER
dgramSeqNo=1
ds=127.0.0.1>2:1000
fsSeqNo=1
-   tunnel4_out_length=0
-   tunnel4_out_protocol=47
-   tunnel4_out_src=1.1.2.88
-   tunnel4_out_dst=1.1.2.92
-   tunnel4_out_src_port=0
-   tunnel4_out_dst_port=0
-   tunnel4_out_tcp_flags=0
-   tunnel4_out_tos=0
-   tunnel_out_vni=456
in_vlan=0
in_priority=0
out_vlan=0
@@ -5989,7 +5980,7 @@ HEADER
dropEvents=0
in_ifindex=2011
in_format=0
-   out_ifindex=1
+   out_ifindex=2
out_format=2
hdr_prot=1
pkt_len=46
diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
index 16dc571..2e27370 100644
--- a/tests/tunnel-push-pop-ipv6.at
+++ b/tests/tunnel-push-pop-ipv6.at
@@ -90,28 +90,28 @@ dnl Check VXLAN tunnel push
 AT_CHECK([ovs-ofctl add-flow int-br action=2])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
 [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: 
tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100))
+  [Datapath actions: 
{tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100)),1}
 ])
 
 dnl Check VXLAN tunnel push set tunnel id by flow and checksum
 AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
 [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: 
tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7c)),out_port(100))
+  [Datapath actions: 
{tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7c)),out_port(100)),1}
 ])
 
 dnl Check GRE tunnel push
 AT_CHECK([ovs-ofctl add-flow int-br action=3])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
 [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: 
tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
+  [Datapath actions: 

[ovs-dev] [PATCH v2 0/2] tunneling : Improving tunneling performance by avoiding the recirculation on datapath.

[Zoltán Balogh]

From: Sugesh Chandran 

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch set removes the recirculation of encapsulated tunnel packets by  
computing the post tunnel actions at the time of translation.   

v2
- Use only single CLONE action with length to mark the tunnel combine action 
set.
- Update the datapath trace display functions to handle CLONE.
- Fixed test cases to work with CLONE action.

Sugesh Chandran (2):
  tunneling: Avoid recirculation on datapath by computing the
recirculate actions at translate time.
  Fix test suite failures on tunneling CLONE action.

 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  20 +-
 lib/dpif.c|   1 +
 lib/odp-execute.c |  57 -
 lib/odp-util.c|  96 +++-
 lib/odp-util.h|   5 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  | 267 +++---
 tests/ofproto-dpif.at |  11 +-
 tests/tunnel-push-pop-ipv6.at |  10 +-
 tests/tunnel-push-pop.at  |  13 +-
 11 files changed, 300 insertions(+), 182 deletions(-)

-- 
2.7.4

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