Re: [ovs-dev] clone action is broken in the master

[Andy Zhou]

On Thu, Mar 9, 2017 at 7:31 PM, Numan Siddique  wrote:
>
>
> On Fri, Mar 10, 2017 at 4:35 AM, Andy Zhou  wrote:
>>
>> On Thu, Mar 9, 2017 at 2:34 AM, Numan Siddique 
>> wrote:
>> > Hi Andy and All,
>> >
>> > The clone action is broken after the commit [1] - xlate: Translate
>> > openflow
>> > clone into odp sample action.
>> >
>> > Because of which, the openstack CI job for OVN is broken.
>> > It works once I revert this commit.
>>
>> Thanks for reporting. This change suppose to be used with the
>> corresponding kernel
>> datapath changes that is currently under review on the netdev mailing
>> list.
>>
>> Given that user space changes and kernel datapath change may not always
>> line up,
>> this situation is not ideal.  To solve the issue reported, I have
>> posted a patch series,
>> that should solve the problem reported.
>>
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329625.html
>>
>> Please let me know if this is not the case after they are pushed. Thanks.
>
>
> Thanks. I tested the patches and it works.
>
> Numan
>
Thanks for testing it.  Just pushed them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] ofproto-dpif-xlate: Avoid using sample action when nesting level is low

[Andy Zhou]

On Fri, Mar 10, 2017 at 5:10 PM, Joe Stringer  wrote:
> On 10 March 2017 at 16:51, Joe Stringer  wrote:
>> On 9 March 2017 at 14:50, Andy Zhou  wrote:
>>> When datapath sample action only allow a small number of nested actions
>>> (i.e. less than 3), do not translate the OpenFlow's 'clone' action
>>> into datapath 'sample' action, since such translation would cause
>>> datapath to reject the flow, with 'EOVERFLOW', when OVS is used to
>>> implement the OVN pipeline, or more generally, when deeper nested
>>> clone are expected.
>>>
>>> Reported-by: Numan Siddique 
>>> Reported-at: 
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329586.html
>>> Signed-off-by: Andy Zhou 
>>> ---
>>>  ofproto/ofproto-dpif-xlate.c | 13 ++---
>>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>> index 1998521..090e8d6 100644
>>> --- a/ofproto/ofproto-dpif-xlate.c
>>> +++ b/ofproto/ofproto-dpif-xlate.c
>>> @@ -4794,13 +4794,20 @@ xlate_clone(struct xlate_ctx *ctx, const struct 
>>> ofpact_nest *oc)
>>>  /* 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;
>>>  if (ctx->xbridge->support.clone) {
>>> +struct flow old_base_flow = ctx->base_flow;
>>>  compose_clone_action(ctx, oc);
>>> -} else {
>>> +ctx->base_flow = old_base_flow;
>>> +} else if (ctx->xbridge->support.sample_nesting > 3) {
>>> +/* Avoid generate sample action if datapath
>>> + * only allow small number of nesting. Deeper nesting
>>> + * can cause the datapath to reject the generated flow.  */
>>> +struct flow old_base_flow = ctx->base_flow;
>>>  compose_clone_action_using_sample(ctx, oc);
>>> +ctx->base_flow = old_base_flow;
>>> +} else {
>>> +do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
>>>  }
>>> -ctx->base_flow = old_base_flow;
>>
>> I think that we should always store the old base flow before and
>> restore afterwards, regardless of how the clone gets actually
>> implemented.
>
> Never mind, the do_xlate_actions() wasn't wrapped in the baseflow
> save/restore before commit 456024cb1c5e ("xlate: Translate openflow
> clone into odp sample action.").
>
> Basically when we serialize clone using the `A,clone(x,y,z),B -->
> A,x,y,z,z',y',x',B` method of putting actions then putting the
> inverse, the changes must only be composed down to the datapath
> actions list after x,y,z, then so long as z',y',x' changes are applied
> back to the flow, then they will be correct the next time that the
> actions are composed down to the datapath actions list. Thanks for the
> explanation.
>
> LGTM,
>
> Acked-by: Joe Stringer 

Thanks for the review.  Push all patches in this series.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] ofproto: Probe for sample nesting level.

[Andy Zhou]

On Fri, Mar 10, 2017 at 4:48 PM, Joe Stringer  wrote:
> On 9 March 2017 at 14:50, Andy Zhou  wrote:
>> Add logics to detect the max level of nesting allowed by the
>> sample action implemented in the datapath.
>>
>> Future patch allows xlate code to generate different odp actions
>> based on this information.
>>
>> Signed-off-by: Andy Zhou 
>
> Acked-by: Joe Stringer 
>
> Couple of minor comments below..
>
> 
>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 5289693..4b88d4b 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -983,6 +983,65 @@ check_max_mpls_depth(struct dpif_backer *backer)
>>  return n;
>>  }
>>
>> +static void
>> +add_sample_actions(struct ofpbuf *actions, int nesting)
>> +{
>> +if (nesting == 0) {
>> +nl_msg_put_odp_port(actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
>> +return;
>> +}
>> +
>> +size_t start, actions_start;
>> +
>> +start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SAMPLE);
>> +actions_start = nl_msg_start_nested(actions, OVS_SAMPLE_ATTR_ACTIONS);
>> +add_sample_actions(actions, nesting - 1);
>> +nl_msg_end_nested(actions, actions_start);
>> +nl_msg_put_u32(actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
>> +nl_msg_end_nested(actions, start);
>> +}
>> +
>> +/* Tests the nested sample actions levels supported by 'backer''s datapath.
>> + *
>> + * Returns the number of nested sample actions accepted by the datapath
>> + * Otherwise returns the number of MPLS push actions supported by
>> + * the datapath. */
>
> Huh? What does MPLS have to do with sampling?
Cut-and-paste error.  Will update the comment.
>
>> +static size_t
>> +check_max_sample_nesting(struct dpif_backer *backer)
>> +{
>> +struct odputil_keybuf keybuf;
>> +struct ofpbuf key;
>> +struct flow flow;
>> +int n;
>> +
>> +struct odp_flow_key_parms odp_parms = {
>> +.flow = ,
>> +};
>> +
>> +memset(, 0, sizeof flow);
>> +ofpbuf_use_stack(, , sizeof keybuf);
>> +odp_flow_key_from_flow(_parms, );
>> +
>> +/* OVS datapath has always supported at least 3 nested leves.  */
>
> *levels
Will fix. Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] ofproto-dpif-xlate: Avoid using sample action when nesting level is low

[Joe Stringer]

On 10 March 2017 at 16:51, Joe Stringer  wrote:
> On 9 March 2017 at 14:50, Andy Zhou  wrote:
>> When datapath sample action only allow a small number of nested actions
>> (i.e. less than 3), do not translate the OpenFlow's 'clone' action
>> into datapath 'sample' action, since such translation would cause
>> datapath to reject the flow, with 'EOVERFLOW', when OVS is used to
>> implement the OVN pipeline, or more generally, when deeper nested
>> clone are expected.
>>
>> Reported-by: Numan Siddique 
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329586.html
>> Signed-off-by: Andy Zhou 
>> ---
>>  ofproto/ofproto-dpif-xlate.c | 13 ++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 1998521..090e8d6 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -4794,13 +4794,20 @@ xlate_clone(struct xlate_ctx *ctx, const struct 
>> ofpact_nest *oc)
>>  /* 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;
>>  if (ctx->xbridge->support.clone) {
>> +struct flow old_base_flow = ctx->base_flow;
>>  compose_clone_action(ctx, oc);
>> -} else {
>> +ctx->base_flow = old_base_flow;
>> +} else if (ctx->xbridge->support.sample_nesting > 3) {
>> +/* Avoid generate sample action if datapath
>> + * only allow small number of nesting. Deeper nesting
>> + * can cause the datapath to reject the generated flow.  */
>> +struct flow old_base_flow = ctx->base_flow;
>>  compose_clone_action_using_sample(ctx, oc);
>> +ctx->base_flow = old_base_flow;
>> +} else {
>> +do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
>>  }
>> -ctx->base_flow = old_base_flow;
>
> I think that we should always store the old base flow before and
> restore afterwards, regardless of how the clone gets actually
> implemented.

Never mind, the do_xlate_actions() wasn't wrapped in the baseflow
save/restore before commit 456024cb1c5e ("xlate: Translate openflow
clone into odp sample action.").

Basically when we serialize clone using the `A,clone(x,y,z),B -->
A,x,y,z,z',y',x',B` method of putting actions then putting the
inverse, the changes must only be composed down to the datapath
actions list after x,y,z, then so long as z',y',x' changes are applied
back to the flow, then they will be correct the next time that the
actions are composed down to the datapath actions list. Thanks for the
explanation.

LGTM,

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

Re: [ovs-dev] [PATCH 3/3] ofproto-dpif-xlate: Avoid using sample action when nesting level is low

[Joe Stringer]

On 9 March 2017 at 14:50, Andy Zhou  wrote:
> When datapath sample action only allow a small number of nested actions
> (i.e. less than 3), do not translate the OpenFlow's 'clone' action
> into datapath 'sample' action, since such translation would cause
> datapath to reject the flow, with 'EOVERFLOW', when OVS is used to
> implement the OVN pipeline, or more generally, when deeper nested
> clone are expected.
>
> Reported-by: Numan Siddique 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329586.html
> Signed-off-by: Andy Zhou 
> ---
>  ofproto/ofproto-dpif-xlate.c | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 1998521..090e8d6 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4794,13 +4794,20 @@ xlate_clone(struct xlate_ctx *ctx, const struct 
> ofpact_nest *oc)
>  /* 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;
>  if (ctx->xbridge->support.clone) {
> +struct flow old_base_flow = ctx->base_flow;
>  compose_clone_action(ctx, oc);
> -} else {
> +ctx->base_flow = old_base_flow;
> +} else if (ctx->xbridge->support.sample_nesting > 3) {
> +/* Avoid generate sample action if datapath
> + * only allow small number of nesting. Deeper nesting
> + * can cause the datapath to reject the generated flow.  */
> +struct flow old_base_flow = ctx->base_flow;
>  compose_clone_action_using_sample(ctx, oc);
> +ctx->base_flow = old_base_flow;
> +} else {
> +do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx);
>  }
> -ctx->base_flow = old_base_flow;

I think that we should always store the old base flow before and
restore afterwards, regardless of how the clone gets actually
implemented.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] ofproto: Probe for sample nesting level.

[Joe Stringer]

On 9 March 2017 at 14:50, Andy Zhou  wrote:
> Add logics to detect the max level of nesting allowed by the
> sample action implemented in the datapath.
>
> Future patch allows xlate code to generate different odp actions
> based on this information.
>
> Signed-off-by: Andy Zhou 

Acked-by: Joe Stringer 

Couple of minor comments below..



> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 5289693..4b88d4b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -983,6 +983,65 @@ check_max_mpls_depth(struct dpif_backer *backer)
>  return n;
>  }
>
> +static void
> +add_sample_actions(struct ofpbuf *actions, int nesting)
> +{
> +if (nesting == 0) {
> +nl_msg_put_odp_port(actions, OVS_ACTION_ATTR_OUTPUT, u32_to_odp(1));
> +return;
> +}
> +
> +size_t start, actions_start;
> +
> +start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SAMPLE);
> +actions_start = nl_msg_start_nested(actions, OVS_SAMPLE_ATTR_ACTIONS);
> +add_sample_actions(actions, nesting - 1);
> +nl_msg_end_nested(actions, actions_start);
> +nl_msg_put_u32(actions, OVS_SAMPLE_ATTR_PROBABILITY, UINT32_MAX);
> +nl_msg_end_nested(actions, start);
> +}
> +
> +/* Tests the nested sample actions levels supported by 'backer''s datapath.
> + *
> + * Returns the number of nested sample actions accepted by the datapath
> + * Otherwise returns the number of MPLS push actions supported by
> + * the datapath. */

Huh? What does MPLS have to do with sampling?

> +static size_t
> +check_max_sample_nesting(struct dpif_backer *backer)
> +{
> +struct odputil_keybuf keybuf;
> +struct ofpbuf key;
> +struct flow flow;
> +int n;
> +
> +struct odp_flow_key_parms odp_parms = {
> +.flow = ,
> +};
> +
> +memset(, 0, sizeof flow);
> +ofpbuf_use_stack(, , sizeof keybuf);
> +odp_flow_key_from_flow(_parms, );
> +
> +/* OVS datapath has always supported at least 3 nested leves.  */

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

Re: [ovs-dev] [PATCH 1/3] dpif: Refactor dpif_probe_feature()

[Joe Stringer]

On 9 March 2017 at 14:50, Andy Zhou  wrote:
> Allow actions to be part of the probe. No functional changes.
> Future patch will make use this new API.
>
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [PATCH v2 2/2] ofproto: Add ref counting for variable length mf_fields.

[Joe Stringer]

On 9 March 2017 at 10:25, Yi-Hung Wei  wrote:
> Currently, a controller may potentially trigger a segmentation fault if it
> accidentally removes a TLV mapping that is still used by an active flow.
> To resolve this issue, in this patch, we maintain reference counting for each
> dynamically allocated variable length mf_fields, so that vswitchd can use this
> information to properly remove a TLV mapping, and to return an error if the
> controller tries to remove a TLV mapping that is still used by any active 
> flow.
>
> To keep track of the usage of tun_metadata for each flow, two 'uint64_t'
> bitmaps are introduce for the flow match and flow action respectively. We use
> 'uint64_t' as a bitmap since the 64 geneve TLV tunnel metadata are the only
> available variable length mf_fields for now. We shall adopt general bitmap 
> when
> more variable length mf_fields are introduced. The bitmaps are configured
> during the flow decoding process, and vswitchd use these bitmaps to increase 
> or
> decrease the ref counting when the flow is created or deleted.
>
> VMWare-BZ: #1768370
> Fixes: 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs.")
> Suggested-by: Jarno Rajahalme 
> Suggested-by: Joe Stringer 
> Signed-off-by: Yi-Hung Wei 
> ---

Couple of minor comments to add from previous discussion,



> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index 40704e628aaa..e844008f6294 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -27,6 +27,8 @@
>  #include "openvswitch/dynamic-string.h"
>  #include "nx-match.h"
>  #include "openvswitch/ofp-util.h"
> +#include "ofproto/ofproto-provider.h"

We can drop this include now.



> @@ -2697,53 +2710,98 @@ mf_get_vl_mff(const struct mf_field *mff,
>  return NULL;
>  }
>
> -/* Updates the tun_metadata mf_field in 'vl_mff_map' according to 'ttm'.
> - * This function is supposed to be invoked after tun_metadata_table_mod(). */
> -enum ofperr
> -mf_vl_mff_map_mod_from_tun_metadata(struct vl_mff_map *vl_mff_map,
> -const struct ofputil_tlv_table_mod *ttm)
> +static enum ofperr
> +mf_vl_mff_map_del(struct vl_mff_map *vl_mff_map,
> +  const struct ofputil_tlv_table_mod *ttm, bool force)
>  OVS_REQUIRES(vl_mff_map->mutex)
>  {
>  struct ofputil_tlv_map *tlv_map;
> +struct vl_mf_field *vmf;
> +unsigned int idx;
>
> -if (ttm->command == NXTTMC_CLEAR) {
> -mf_vl_mff_map_clear(vl_mff_map);
> -return 0;
> +if (!force) {
> +LIST_FOR_EACH (tlv_map, list_node, >mappings) {
> +idx = MFF_TUN_METADATA0 + tlv_map->index;

Idly wondering whether it improves code readability to have some
simple functions like

mff_to_idx(int)
idx_to_mff(int)

which handle the +/- of MFF_TUN_METADATA0 in so many places around the code.

If you think it's worthwhile it could be a separate followup patch.

> @@ -6332,6 +6365,11 @@ ofpacts_pull_openflow_actions__(struct ofpbuf 
> *openflow,
>   * you should call ofpacts_pull_openflow_instructions() instead of this
>   * function.
>   *
> + * 'vl_mff_map' and 'ofpacts_tlv_bitmap' are optional. If 'vl_mff_map' is not
> + * NULL, it is used to drive the variable length mf_fields, and

Where does it 'drive' the variable to? :-)

(This comment might benefit from the feedback I had on similar
comments in the previous patch)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

[Daniele Di Proietto]

Commit daf4d3c18da4("odp: Support conntrack orig tuple key.") introduced
new fields in struct 'pkt_metadata'.  pkt_metadata_init() is called for
every packet in the userspace datapath.  When testing a simple single
flow case with DPDK, we observe a lower throughput after the above
commit (it was 14.88 Mpps before, it is 13 Mpps after).

This patch skips initializing ct_orig_tuple in pkt_metadata_init().
It should be enough to initialize ct_state, because nobody should look
at ct_orig_tuple unless ct_state is != 0.

CC: Jarno Rajahalme 
Signed-off-by: Daniele Di Proietto 
---
I'm sending this as an RFC because I didn't check very carefully if we can
really avoid initializing ct_orig_tuple.

Maybe there are better solutions to this problem.
---
 lib/packets.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/packets.h b/lib/packets.h
index a5a483bc8..6f1791c7a 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t port)
 /* It can be expensive to zero out all of the tunnel metadata. However,
  * we can just zero out ip_dst and the rest of the data will never be
  * looked at. */
-memset(md, 0, offsetof(struct pkt_metadata, in_port));
+memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
 md->tunnel.ip_dst = 0;
 md->tunnel.ipv6_dst = in6addr_any;
 
-- 
2.11.0

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

Re: [ovs-dev] [PATCH v2 branch-2.6] docs: Use DPDK 16.07.2 stable release

[Daniele Di Proietto]

2017-03-10 3:47 GMT-08:00 Ian Stokes :
> DPDK now provides a stable release branch. Modify dpdk docs and travis
> linux build script to use the DPDK 16.07.2 stable branch to benefit from
> most recent bug fixes.
>
> Signed-off-by: Ian Stokes 

Thanks, applied to branch-2.6

> ---
> v1 -> v2
> * Set correct path to DPDK stable branch for EXTRA_OPTS in travis linux
>   build.
> ---
>  .travis/linux-build.sh   |   14 +++---
>  FAQ.md   |4 ++--
>  INSTALL.DPDK-ADVANCED.md |6 +++---
>  INSTALL.DPDK.md  |   22 ++
>  4 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 3bcec93..f15f706 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -52,13 +52,13 @@ function install_kernel()
>  function install_dpdk()
>  {
>  if [ -n "$DPDK_GIT" ]; then
> -git clone $DPDK_GIT dpdk-$1
> -cd dpdk-$1
> -git checkout v$1
> +git clone $DPDK_GIT dpdk-stable-$1
> +cd dpdk-stable-$1
> +git checkout tags/v$1
>  else
> -wget http://www.dpdk.org/browse/dpdk/snapshot/dpdk-$1.tar.gz
> +wget http://fast.dpdk.org/rel/dpdk-$1.tar.gz
>  tar xzvf dpdk-$1.tar.gz > /dev/null
> -cd dpdk-$1
> +cd dpdk-stable-$1
>  fi
>  find ./ -type f | xargs sed -i 
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
>  echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
> @@ -80,14 +80,14 @@ fi
>
>  if [ "$DPDK" ]; then
>  if [ -z "$DPDK_VER" ]; then
> -DPDK_VER="16.07"
> +DPDK_VER="16.07.2"
>  fi
>  install_dpdk $DPDK_VER
>  if [ "$CC" = "clang" ]; then
>  # Disregard cast alignment errors until DPDK is fixed
>  CFLAGS="$CFLAGS -Wno-cast-align"
>  fi
> -EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-$DPDK_VER/build"
> +EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-stable-$DPDK_VER/build"
>  elif [ "$CC" != "clang" ]; then
>  # DPDK headers currently trigger sparse errors
>  SPARSE_FLAGS="$SPARSE_FLAGS -Wsparse-error"
> diff --git a/FAQ.md b/FAQ.md
> index cf30f9b..75a393b 100644
> --- a/FAQ.md
> +++ b/FAQ.md
> @@ -256,12 +256,12 @@ A: The following table lists the DPDK version against 
> which the
> given versions of Open vSwitch will successfully build.
>
>  | Open vSwitch | DPDK
> -|::|:-:
> +|::|:---:
>  |2.2.x | 1.6
>  |2.3.x | 1.6
>  |2.4.x | 2.0
>  |2.5.x | 2.2
> -|2.6.x | 16.07
> +|2.6.x | 16.07.2
>
>  ### Q: I get an error like this when I configure Open vSwitch:
>
> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
> index e3603a1..ae21aca 100755
> --- a/INSTALL.DPDK-ADVANCED.md
> +++ b/INSTALL.DPDK-ADVANCED.md
> @@ -46,7 +46,7 @@ for DPDK and OVS.
>  For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
>
>  ```
> -export DPDK_DIR=/usr/src/dpdk-16.07
> +export DPDK_DIR=/usr/src/dpdk-stable-16.07.2
>  export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>  make install T=$DPDK_TARGET DESTDIR=install
>  ```
> @@ -342,7 +342,7 @@ For users wanting to do packet forwarding using kernel 
> stack below are the steps
> cd /usr/src/cmdline_generator
> wget 
> https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/cmdline_generator.c
> wget 
> https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/Makefile
> -   export RTE_SDK=/usr/src/dpdk-16.07
> +   export RTE_SDK=/usr/src/dpdk-stable-16.07.2
> export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
> make
> ./build/cmdline_generator -m -p dpdkr0 XXX
> @@ -366,7 +366,7 @@ For users wanting to do packet forwarding using kernel 
> stack below are the steps
> mount -t hugetlbfs nodev /dev/hugepages (if not already mounted)
>
> # Build the DPDK ring application in the VM
> -   export RTE_SDK=/root/dpdk-16.07
> +   export RTE_SDK=/root/dpdk-stable-16.07.2
> export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
> make
>
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
> index 30e9258..9ab29f3 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -21,7 +21,7 @@ The DPDK support of Open vSwitch is considered 
> 'experimental'.
>
>  ### Prerequisites
>
> -* Required: DPDK 16.07
> +* Required: DPDK 16.07.2
>  * Hardware: [DPDK Supported NICs] when physical ports in use
>
>  ##  2. Building and Installation
> @@ -42,10 +42,9 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
>
>   ```
>   cd /usr/src/
> - wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
> - unzip dpdk-16.07.zip
> -
> - export DPDK_DIR=/usr/src/dpdk-16.07
> + 

Re: [ovs-dev] [PATCH v2] docs: Use DPDK 16.11.1 stable release.

[Daniele Di Proietto]

2017-03-10 3:47 GMT-08:00 Ian Stokes :
> DPDK now provides a stable release branch. Modify dpdk docs and travis linux
> build script to use the DPDK 16.11.1 stable branch to benefit from most
> recent bug fixes.
>
> Signed-off-by: Ian Stokes 

Thanks, applied to master and branch-2.7

> ---
> v1 -> v2
> * Set correct path to DPDK stable branch for EXTRA_OPTS in travis linux
>   build.
> ---
>  .travis/linux-build.sh   |   12 ++--
>  Documentation/faq/releases.rst   |   10 +-
>  Documentation/intro/install/dpdk.rst |6 +++---
>  Documentation/topics/dpdk/vhost-user.rst |8 
>  4 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 4175d72..8750d68 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -52,13 +52,13 @@ function install_kernel()
>  function install_dpdk()
>  {
>  if [ -n "$DPDK_GIT" ]; then
> -git clone $DPDK_GIT dpdk-$1
> -cd dpdk-$1
> -git checkout v$1
> +git clone $DPDK_GIT dpdk-stable-$1
> +cd dpdk-stable-$1
> +git checkout tags/v$1
>  else
>  wget http://fast.dpdk.org/rel/dpdk-$1.tar.gz
>  tar xzvf dpdk-$1.tar.gz > /dev/null
> -cd dpdk-$1
> +cd dpdk-stable-$1
>  fi
>  find ./ -type f | xargs sed -i 
> 's/max-inline-insns-single=100/max-inline-insns-single=400/'
>  echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
> @@ -80,14 +80,14 @@ fi
>
>  if [ "$DPDK" ]; then
>  if [ -z "$DPDK_VER" ]; then
> -DPDK_VER="16.11"
> +DPDK_VER="16.11.1"
>  fi
>  install_dpdk $DPDK_VER
>  if [ "$CC" = "clang" ]; then
>  # Disregard cast alignment errors until DPDK is fixed
>  CFLAGS="$CFLAGS -Wno-cast-align"
>  fi
> -EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-$DPDK_VER/build"
> +EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-stable-$DPDK_VER/build"
>  elif [ "$CC" != "clang" ]; then
>  # DPDK headers currently trigger sparse errors
>  SPARSE_FLAGS="$SPARSE_FLAGS -Wsparse-error"
> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
> index 118c88d..98f5636 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -152,16 +152,16 @@ Q: What DPDK version does each Open vSwitch release 
> work with?
>  A: The following table lists the DPDK version against which the given
>  versions of Open vSwitch will successfully build.
>
> - =
> + ===
>  Open vSwitch DPDK
> - =
> + ===
>  2.2.x1.6
>  2.3.x1.6
>  2.4.x2.0
>  2.5.x2.2
> -2.6.x16.07
> -2.7.x16.11
> - =
> +2.6.x16.07.2
> +2.7.x16.11.1
> + ===
>
>  Q: I get an error like this when I configure Open vSwitch::
>
> diff --git a/Documentation/intro/install/dpdk.rst 
> b/Documentation/intro/install/dpdk.rst
> index 3018590..b947bd5 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -64,9 +64,9 @@ Install DPDK
>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>
> $ cd /usr/src/
> -   $ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
> -   $ tar xf dpdk-16.11.tar.xz
> -   $ export DPDK_DIR=/usr/src/dpdk-16.11
> +   $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> +   $ tar xf dpdk-16.11.1.tar.xz
> +   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
> $ cd $DPDK_DIR
>
>  #. (Optional) Configure DPDK as a shared library
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index 5448bd2..ba22684 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in 
> :ref:`dpdk-vhost-user` or
>  DPDK sources to VM and build DPDK::
>
>  $ cd /root/dpdk/
> -$ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
> -$ tar xf dpdk-16.11.tar.xz
> -$ export DPDK_DIR=/root/dpdk/dpdk-16.11
> +$ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> +$ tar xf dpdk-16.11.1.tar.xz
> +$ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
>  $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>  $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>  $ cd $DPDK_DIR
> @@ -364,7 +364,7 @@ Sample XML
>  
>  
>
> -  
> +  
>
>
>  
> --
> 1.7.0.7
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org

[ovs-dev] [PATCH v3 3/3] ofp-actions: Add limit to learn action.

[Daniele Di Proietto]

This commit adds a new feature to the learn actions: the possibility to
limit the number of learned flows.

To be compatible with users of the old learn action, a new structure is
introduced as well as a new OpenFlow raw action number.

There's a small corner case when we have to delete the ukey.  This
happens when:
* The learned rule has expired (or has been deleted).
* The ukey that learned the rule is still in the datapath.
* No packets hit the datapath flow recently.
In this case we cannot relearn the rule (because there are no new
packets), and the actions might depend on the learn execution, so the
only option is to delete the ukey.  I don't think this has big
performance implications since it's done only for ukey with no traffic.

We could also slowpath it, but that will cause an action upcall and the
correct datapath actions will be installed later by a revalidator.  If
we delete the ukey, the next upcall will be a miss upcall and that will
immediatedly install the correct datapath flow.

Signed-off-by: Daniele Di Proietto 
---
 include/openvswitch/ofp-actions.h  |  12 +++
 lib/learn.c|  24 +
 lib/ofp-actions.c  |  88 -
 ofproto/ofproto-dpif-upcall.c  |   4 +
 ofproto/ofproto-dpif-xlate-cache.c |   3 +-
 ofproto/ofproto-dpif-xlate-cache.h |   1 +
 ofproto/ofproto-dpif-xlate.c   |  26 -
 ofproto/ofproto-dpif-xlate.h   |   3 +
 ofproto/ofproto-provider.h |   3 +-
 ofproto/ofproto.c  |  46 +++--
 tests/learn.at | 191 +
 tests/ofp-actions.at   |  14 +++
 utilities/ovs-ofctl.8.in   |  16 
 13 files changed, 419 insertions(+), 12 deletions(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index d4be46f0c..6f554fe0c 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -660,6 +660,10 @@ struct ofpact_resubmit {
  * If NX_LEARN_F_SEND_FLOW_REM is set, then the learned flows will have their
  * OFPFF_SEND_FLOW_REM flag set.
  *
+ * If NX_LEARN_F_WRITE_RESULT is set, then the actions will write whether the
+ * learn operation succeded on a bit.  If the learn is successful the bit will
+ * be set, otherwise (e.g. if the limit is hit) the bit will be unset.
+ *
  * If NX_LEARN_F_DELETE_LEARNED is set, then removing this action will delete
  * all the flows from the learn action's 'table_id' that have the learn
  * action's 'cookie'.  Important points:
@@ -685,6 +689,7 @@ struct ofpact_resubmit {
 enum nx_learn_flags {
 NX_LEARN_F_SEND_FLOW_REM = 1 << 0,
 NX_LEARN_F_DELETE_LEARNED = 1 << 1,
+NX_LEARN_F_WRITE_RESULT = 1 << 2,
 };
 
 #define NX_LEARN_N_BITS_MASK0x3ff
@@ -748,6 +753,13 @@ struct ofpact_learn {
 ovs_be64 cookie;   /* Cookie for new flow. */
 uint16_t fin_idle_timeout; /* Idle timeout after FIN, if nonzero. */
 uint16_t fin_hard_timeout; /* Hard timeout after FIN, if nonzero. */
+/* If the number of flows on 'table_id' with 'cookie' exceeds this,
+ * the action will not add a new flow. */
+uint32_t limit;
+/* Used only if 'flags' has NX_LEARN_F_WRITE_RESULT.  If the execution
+ * failed to install a new flow because 'limit' is exceeded,
+ * result_dst will be set to 0, otherwise to 1. */
+struct mf_subfield result_dst;
 );
 
 struct ofpact_learn_spec specs[];
diff --git a/lib/learn.c b/lib/learn.c
index 199084905..d747d255e 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -407,6 +407,22 @@ learn_parse__(char *orig, char *arg, struct ofpbuf 
*ofpacts)
 learn->flags |= NX_LEARN_F_SEND_FLOW_REM;
 } else if (!strcmp(name, "delete_learned")) {
 learn->flags |= NX_LEARN_F_DELETE_LEARNED;
+} else if (!strcmp(name, "limit")) {
+learn->limit = atoi(value);
+} else if (!strcmp(name, "result_dst")) {
+char *error;
+learn->flags |= NX_LEARN_F_WRITE_RESULT;
+error = mf_parse_subfield(>result_dst, value);
+if (error) {
+return error;
+}
+if (!learn->result_dst.field->writable) {
+return xasprintf("%s is read-only", value);
+}
+if (learn->result_dst.n_bits != 1) {
+return xasprintf("result_dst in 'learn' action must be a "
+ "single bit");
+}
 } else {
 struct ofpact_learn_spec *spec;
 char *error;
@@ -488,6 +504,14 @@ learn_format(const struct ofpact_learn *learn, struct ds 
*s)
 ds_put_format(s, ",%scookie=%s%#"PRIx64,
   colors.param, colors.end, ntohll(learn->cookie));
 }
+if (learn->limit != 0) {
+ds_put_format(s, ",%slimit=%s%"PRIu32,
+  colors.param, colors.end, learn->limit);
+}
+ 

[ovs-dev] [PATCH v3 2/3] ofp-actions: Factor out decode_LEARN_{common, spec}().

[Daniele Di Proietto]

No functional change, they will be used by next commit.

Signed-off-by: Daniele Di Proietto 
---
 lib/ofp-actions.c | 58 ++-
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 2c8ab1788..603435857 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -4352,23 +4352,14 @@ learn_min_len(uint16_t header)
 return min_len;
 }
 
-/* Converts 'nal' into a "struct ofpact_learn" and appends that struct to
- * 'ofpacts'.  Returns 0 if successful, otherwise an OFPERR_*. */
 static enum ofperr
-decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
-   enum ofp_version ofp_version OVS_UNUSED,
-   const struct vl_mff_map *vl_mff_map,
-   struct ofpbuf *ofpacts)
+decode_LEARN_common(const struct nx_action_learn *nal,
+struct ofpact_learn *learn)
 {
-struct ofpact_learn *learn;
-const void *p, *end;
-
 if (nal->pad) {
 return OFPERR_OFPBAC_BAD_ARGUMENT;
 }
 
-learn = ofpact_put_LEARN(ofpacts);
-
 learn->idle_timeout = ntohs(nal->idle_timeout);
 learn->hard_timeout = ntohs(nal->hard_timeout);
 learn->priority = ntohs(nal->priority);
@@ -4376,19 +4367,23 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn 
*nal,
 learn->table_id = nal->table_id;
 learn->fin_idle_timeout = ntohs(nal->fin_idle_timeout);
 learn->fin_hard_timeout = ntohs(nal->fin_hard_timeout);
-
 learn->flags = ntohs(nal->flags);
-if (learn->flags & ~(NX_LEARN_F_SEND_FLOW_REM |
- NX_LEARN_F_DELETE_LEARNED)) {
-return OFPERR_OFPBAC_BAD_ARGUMENT;
-}
 
 if (learn->table_id == 0xff) {
 return OFPERR_OFPBAC_BAD_ARGUMENT;
 }
 
-end = (char *) nal + ntohs(nal->len);
-for (p = nal + 1; p != end; ) {
+return 0;
+}
+
+static enum ofperr
+decode_LEARN_specs(const void *p, const void *end,
+   const struct vl_mff_map *vl_mff_map,
+   struct ofpbuf *ofpacts)
+{
+struct ofpact_learn *learn = ofpacts->header;
+
+while (p != end) {
 struct ofpact_learn_spec *spec;
 uint16_t header = ntohs(get_be16());
 
@@ -4461,6 +4456,33 @@ decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
 return 0;
 }
 
+/* Converts 'nal' into a "struct ofpact_learn" and appends that struct to
+ * 'ofpacts'.  Returns 0 if successful, otherwise an OFPERR_*. */
+static enum ofperr
+decode_NXAST_RAW_LEARN(const struct nx_action_learn *nal,
+   enum ofp_version ofp_version OVS_UNUSED,
+   const struct vl_mff_map *vl_mff_map,
+   struct ofpbuf *ofpacts)
+{
+struct ofpact_learn *learn;
+enum ofperr error;
+
+learn = ofpact_put_LEARN(ofpacts);
+
+error = decode_LEARN_common(nal, learn);
+if (error) {
+return error;
+}
+
+if (learn->flags & ~(NX_LEARN_F_SEND_FLOW_REM |
+ NX_LEARN_F_DELETE_LEARNED)) {
+return OFPERR_OFPBAC_BAD_ARGUMENT;
+}
+
+return decode_LEARN_specs(nal + 1, (char *) nal + ntohs(nal->len),
+  vl_mff_map, ofpacts);
+}
+
 static void
 put_be16(struct ofpbuf *b, ovs_be16 x)
 {
-- 
2.11.0

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

[ovs-dev] [PATCH v3 1/3] ofproto-dpif-xlate: Create XC_LEARN entry after learning.

[Daniele Di Proietto]

This will be useful in a separate commit, because learning can fail.

Signed-off-by: Daniele Di Proietto 
Acked-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-xlate.c | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8ce6a5939..8c4b714b2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4579,11 +4579,7 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct 
ofpact_learn *learn)
 enum ofperr error;
 
 if (ctx->xin->xcache) {
-struct xc_entry *entry;
-
-entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
-entry->learn.ofm = xmalloc(sizeof *entry->learn.ofm);
-ofm = entry->learn.ofm;
+ofm = xmalloc(sizeof *ofm);
 } else {
 ofm = __;
 }
@@ -4617,8 +4613,22 @@ xlate_learn_action(struct xlate_ctx *ctx, const struct 
ofpact_learn *learn)
  , ofm);
 ofpbuf_uninit();
 
-if (!error && ctx->xin->allow_side_effects) {
-error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL);
+if (!error) {
+if (ctx->xin->allow_side_effects) {
+error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL);
+}
+
+if (ctx->xin->xcache) {
+struct xc_entry *entry;
+
+entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
+entry->learn.ofm = ofm;
+ofm = NULL;
+}
+}
+
+if (ctx->xin->xcache) {
+free(ofm);
 }
 
 if (error) {
-- 
2.11.0

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

[ovs-dev] [PATCH v3 0/3] Learn action limit

[Daniele Di Proietto]

This series implements the possibility to have a limit on the number of
flows learned by a learn action.  After the learn action execution the
pipeline can read the result (to know if the limit was exceeded).

v2->v3:
* When the learned rule expired and there are no packets, instead of
  slowpathing the ukey, delete it.  This should make the next upcall
  a miss upcall and ovs should immediately install a flow. Suggested
  by Joe (thanks!).
* NXAST_RAW_LEARN2 is now subtype 45 instead of 44.

v1->v2:
* 'limit' counts both learn flows and flows installed by controller,
  suggested by Ben.
* Don't keep a reference to the counter in ukeys
* Squash tests, openflow interface changes and ofproto implementation
  into a single commit
* The new cookie-counters module is not used anymore, therefore it's removed
* Fix memory leak in ofproto_flow_mod_learn(): we have to call
  ofproto_flow_mod_uninit() if we don't call ofproto_flow_mod_learn_start().
* Simplify ofp-actions changes according to Ben comments(thanks!)


Daniele Di Proietto (3):
  ofproto-dpif-xlate: Create XC_LEARN entry after learning.
  ofp-actions: Factor out decode_LEARN_{common,spec}().
  ofp-actions: Add limit to learn action.

 include/openvswitch/ofp-actions.h  |  12 +++
 lib/learn.c|  24 +
 lib/ofp-actions.c  | 146 
 ofproto/ofproto-dpif-upcall.c  |   4 +
 ofproto/ofproto-dpif-xlate-cache.c |   3 +-
 ofproto/ofproto-dpif-xlate-cache.h |   1 +
 ofproto/ofproto-dpif-xlate.c   |  46 +++--
 ofproto/ofproto-dpif-xlate.h   |   3 +
 ofproto/ofproto-provider.h |   3 +-
 ofproto/ofproto.c  |  46 +++--
 tests/learn.at | 191 +
 tests/ofp-actions.at   |  14 +++
 utilities/ovs-ofctl.8.in   |  16 
 13 files changed, 474 insertions(+), 35 deletions(-)

-- 
2.11.0

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

Re: [ovs-dev] [PATCH] ovn-northd: Skip icmp4 packets destined for router ports from conntrack

[Russell Bryant]

On Fri, Mar 10, 2017 at 2:35 PM, Russell Bryant  wrote:
> On Thu, Mar 9, 2017 at 11:52 PM, Numan Siddique  wrote:
>> Thanks for the review. Please see inline.
>>
>>
>> On Fri, Mar 10, 2017 at 1:44 AM, Russell Bryant  wrote:
>>>
>>> On Mon, Feb 27, 2017 at 12:59 AM,   wrote:
>>> > From: Numan Siddique 
>>> >
>>> > Presently, the icmp4 requests to the router gateway ip are sent to the
>>> > connectiont tracker, but the icmp4 reply packets responded by
>>> > 'lr_in_ip_input' stage are not sent to the connection tracker.
>>> > Also no zone ids are assigned for the router ports. Because of which
>>> > the icmp4 request packets in the connection tracker will be in the
>>> > UNREPLIED state. If the CMS has added ACLs to drop packets which
>>> > are not in ESTABLISHED state, the icmp4 replies will be dropped.
>>> >
>>> > To fix this issue, this patch adds a priority-110 flow in
>>> > 'ls_in_pre_acl'
>>> > stage which doesn't set reg0[0] = 1 for icmp4 request packets destined
>>> > to the router gateway ips.
>>> >
>>> > Alternate solution would be to assign zone ids for the router ports
>>> > and send the packets from the router ports to the connection tracker.
>>> >
>>> > The approach used in this patch seems to be simpler.
>>> >
>>> > Signed-off-by: Numan Siddique 
>>> > ---
>>> >  ovn/northd/ovn-northd.8.xml | 29 +++---
>>> >  ovn/northd/ovn-northd.c | 92
>>> > +++--
>>> >  2 files changed, 79 insertions(+), 42 deletions(-)
>>>
>>>
>>> Can you clarify where the packet gets dropped?  It seems like we have
>>> flows trying to handle this already.  We skip conntrack for the router
>>> interface ports.  Roughly, I would expect:
>>>
>>
>> The packets are getting dropped because of the flow
>>
>> table=52, n_packets=40, n_bytes=3920,
>> priority=2001,ct_state=-est+trk,ip,reg15=0x3,metadata=0x3 ac
>> tions=drop
>>
>> This flow corresponds to logical flow -
>> table=4 (ls_out_acl ), priority=2001 , match=((!ct.est || (ct.est &&
>> ct_label.blocked == 1)) && (outport ==
>> "ce575934-f308-45b8-b9cd-457646da213d" && ip)), action=(drop;)
>>
>>
>> This logical flow is added by neutron OVN plugin when the port is configured
>> with security groups. When I clear the security groups for the port or add a
>> specific security group rule to allow icmp, it works fine.
>
> The above flow could be hit at two different points (#2 and #5 below).
> In my local testing, it looks like it's happening in #5 so at least we
> aren't hitting conntrack related flows in a part of the pipeline where
> we don't expect it.
>
>>
>>>
>>> 1) inport == lport1, logical switch ingress pipeline.  packet sent
>>> through conntrack.
>>>
>>> 2) outport == router interface port, logical switch egress pipeline.
>>> packet *skips* conntrack since it's a router interface.
>>>
>>> 3) router datapath, icmp respose generated, sent back to logical switch
>>> ...
>>>
>>> 4) inport == router interface, logical switch ingress pipeline, packet
>>> *skips* conntrack since it's a router interface
>>>
>>> 5) outport == lport1, logical switch egress pipeline, packet sent
>>> through conntrack, which should find an existing conntrack entry
>>> established in step 1.  packet delivered to lport1.
>>>
>>
>> The connection tracking entry has this
>>
>> $ sudo conntrack -L | grep 10.0.0.1
>> conntrack v1.4.3 (conntrack-tools): 54 flow entries have been shown.
>> icmp 1 29 src=10.0.0.6 dst=10.0.0.1 type=8 code=0 id=7486 [UNREPLIED]
>> src=10.0.0.1 dst=10.0.0.6 type=0 code=0 id=7486 mark=0
>> secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
>>
>> You think this should be addressed by neutron OVN plugin ?
>
> I don't think it's a Neutron issue.
>
> I see the conntrack entry remaining in the UNREPLIED state, even in
> the working case where there's not an ACL dropping the reply.
>
> icmp 1 29 src=10.0.0.10 dst=10.0.0.1 type=8 code=0 id=25857
> [UNREPLIED] src=10.0.0.1 dst=10.0.0.10 type=0 code=0 id=25857 mark=0
> zone=8 use=1
>
> If I ping a different address (something past the logical router), I
> see a proper conntrack entry that's not in the UNREPLIED state.
>
> I wonder if there's something about how we are generating the ICMP
> response from the logical router that's making conntrack not properly
> associate it with the request?

I checked into this and there's no meaningful difference in how we
form the ICMP reply.

I'm guessing this has to do with the request and reply both going
through conntrack as a part of processing the same packet in the OVS
data path.  That's not behaving how we would expect.  I'll keep
looking next week to try to identify the root cause here, but I would
appreciate any insight others may have about the behavior we should
expect in this scenario.

-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org

Re: [ovs-dev] [PATCH 2/2] ofproto: Add ref counting for variable length mf_fields.

[Joe Stringer]

On 10 March 2017 at 11:34, Joe Stringer  wrote:
> On 9 March 2017 at 10:22, Yi-Hung Wei  wrote:
>> On Wed, Mar 8, 2017 at 11:40 AM, Joe Stringer  wrote:
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index e844008f6294..bef5aad768a3 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -2836,9 +2836,13 @@ mf_vl_mff_ref_cnt_mod(const struct vl_mff_map
> *map, uint64_t tlv_bitmap,
> vmf = mf_get_vl_mff__(i + MFF_TUN_METADATA0, map);
> if (vmf) {
> if (ref) {
> -ovs_refcount_ref(>ref_cnt);
> +if (ovs_refcount_ref(>ref_cnt) == 0) {
> +VLOG_WARN("Taking reference on freed VMF %d", i);
> +}

Turns out that ovs_refcount_ref() already asserts this ;-)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-northd: Skip icmp4 packets destined for router ports from conntrack

[Russell Bryant]

On Thu, Mar 9, 2017 at 11:52 PM, Numan Siddique  wrote:
> Thanks for the review. Please see inline.
>
>
> On Fri, Mar 10, 2017 at 1:44 AM, Russell Bryant  wrote:
>>
>> On Mon, Feb 27, 2017 at 12:59 AM,   wrote:
>> > From: Numan Siddique 
>> >
>> > Presently, the icmp4 requests to the router gateway ip are sent to the
>> > connectiont tracker, but the icmp4 reply packets responded by
>> > 'lr_in_ip_input' stage are not sent to the connection tracker.
>> > Also no zone ids are assigned for the router ports. Because of which
>> > the icmp4 request packets in the connection tracker will be in the
>> > UNREPLIED state. If the CMS has added ACLs to drop packets which
>> > are not in ESTABLISHED state, the icmp4 replies will be dropped.
>> >
>> > To fix this issue, this patch adds a priority-110 flow in
>> > 'ls_in_pre_acl'
>> > stage which doesn't set reg0[0] = 1 for icmp4 request packets destined
>> > to the router gateway ips.
>> >
>> > Alternate solution would be to assign zone ids for the router ports
>> > and send the packets from the router ports to the connection tracker.
>> >
>> > The approach used in this patch seems to be simpler.
>> >
>> > Signed-off-by: Numan Siddique 
>> > ---
>> >  ovn/northd/ovn-northd.8.xml | 29 +++---
>> >  ovn/northd/ovn-northd.c | 92
>> > +++--
>> >  2 files changed, 79 insertions(+), 42 deletions(-)
>>
>>
>> Can you clarify where the packet gets dropped?  It seems like we have
>> flows trying to handle this already.  We skip conntrack for the router
>> interface ports.  Roughly, I would expect:
>>
>
> The packets are getting dropped because of the flow
>
> table=52, n_packets=40, n_bytes=3920,
> priority=2001,ct_state=-est+trk,ip,reg15=0x3,metadata=0x3 ac
> tions=drop
>
> This flow corresponds to logical flow -
> table=4 (ls_out_acl ), priority=2001 , match=((!ct.est || (ct.est &&
> ct_label.blocked == 1)) && (outport ==
> "ce575934-f308-45b8-b9cd-457646da213d" && ip)), action=(drop;)
>
>
> This logical flow is added by neutron OVN plugin when the port is configured
> with security groups. When I clear the security groups for the port or add a
> specific security group rule to allow icmp, it works fine.

The above flow could be hit at two different points (#2 and #5 below).
In my local testing, it looks like it's happening in #5 so at least we
aren't hitting conntrack related flows in a part of the pipeline where
we don't expect it.

>
>>
>> 1) inport == lport1, logical switch ingress pipeline.  packet sent
>> through conntrack.
>>
>> 2) outport == router interface port, logical switch egress pipeline.
>> packet *skips* conntrack since it's a router interface.
>>
>> 3) router datapath, icmp respose generated, sent back to logical switch
>> ...
>>
>> 4) inport == router interface, logical switch ingress pipeline, packet
>> *skips* conntrack since it's a router interface
>>
>> 5) outport == lport1, logical switch egress pipeline, packet sent
>> through conntrack, which should find an existing conntrack entry
>> established in step 1.  packet delivered to lport1.
>>
>
> The connection tracking entry has this
>
> $ sudo conntrack -L | grep 10.0.0.1
> conntrack v1.4.3 (conntrack-tools): 54 flow entries have been shown.
> icmp 1 29 src=10.0.0.6 dst=10.0.0.1 type=8 code=0 id=7486 [UNREPLIED]
> src=10.0.0.1 dst=10.0.0.6 type=0 code=0 id=7486 mark=0
> secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
>
> You think this should be addressed by neutron OVN plugin ?

I don't think it's a Neutron issue.

I see the conntrack entry remaining in the UNREPLIED state, even in
the working case where there's not an ACL dropping the reply.

icmp 1 29 src=10.0.0.10 dst=10.0.0.1 type=8 code=0 id=25857
[UNREPLIED] src=10.0.0.1 dst=10.0.0.10 type=0 code=0 id=25857 mark=0
zone=8 use=1

If I ping a different address (something past the logical router), I
see a proper conntrack entry that's not in the UNREPLIED state.

I wonder if there's something about how we are generating the ICMP
response from the logical router that's making conntrack not properly
associate it with the request?

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

Re: [ovs-dev] [PATCH v2 1/2] nx-match: Use vl_mff_map to parse match field.

[Joe Stringer]

On 9 March 2017 at 10:24, Yi-Hung Wei  wrote:
> vl_mff_map is introduced in commit 04f48a68c428 ("ofp-actions: Fix variable
> length meta-flow OXMs") to account variable length mf_field, and it is used
> to decode variable length mf_field in ofp_action. In this patch, vl_mff_map
> is further used to decode the variable length match field as well.
>
> Signed-off-by: Yi-Hung Wei 

Thanks for v2, I had a bit of trouble applying to master but I've
reviewed below anyway. I think that Jarno's changes yesterday
conflict, so you'll need to rebase before submitting the next version.

> @@ -594,16 +598,21 @@ nx_pull_match__(struct ofpbuf *b, unsigned int 
> match_len, bool strict,
>   * are valid pointers, then stores the cookie and mask in them if 'b' 
> contains
>   * a "NXM_NX_COOKIE*" match.  Otherwise, stores 0 in both.
>   *
> + * 'vl_mff_map" is an optional parameter that is used to derive variable 
> length
> + * mf_fields in flow match. If it is not provided, the default mf_fields with
> + * maximum length will be used.
> + *

Would the below be more accurate and informative? Or is it too specific?

'vl_mff_map' is an optional parameter that is used to validate the
length of variable
length mf_fields in 'match'. If it is not provided, the default
mf_fields with maximum
length will be used.

I guess there's a couple of places where this comment exists that
would benefit from this.

>   * Fails with an error upon encountering an unknown NXM header.
>   *
>   * Returns 0 if successful, otherwise an OpenFlow error code. */
>  enum ofperr
>  nx_pull_match(struct ofpbuf *b, unsigned int match_len, struct match *match,
>ovs_be64 *cookie, ovs_be64 *cookie_mask,
> -  const struct tun_table *tun_table)
> +  const struct tun_table *tun_table,
> +  const struct vl_mff_map *vl_mff_map)
>  {
>  return nx_pull_match__(b, match_len, true, match, cookie, cookie_mask,
> -   tun_table);
> +   tun_table, vl_mff_map);
>  }
>
>  /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers,
> @@ -612,15 +621,17 @@ enum ofperr
>  nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len,
>  struct match *match,
>  ovs_be64 *cookie, ovs_be64 *cookie_mask,
> -const struct tun_table *tun_table)
> +const struct tun_table *tun_table,
> +const struct vl_mff_map *vl_mff_map)
>  {
>  return nx_pull_match__(b, match_len, false, match, cookie, cookie_mask,
> -   tun_table);
> +   tun_table, vl_mff_map);
>  }

Now that I look at the comment above these *_loose() functions, I
wonder - should the vl_mff_map ever be specified for these versions?

If I follow correctly, these loose functions are useful for decoupling
controller and switch support for fields. For example, if the switch
sends up something in a slightly different format than what the
controller expects, the controller is able to basically skip over the
unknown bits but still process the rest of the message. One case might
be a packet_in message from the switch to the controller. The switch
would provide packet and its view of the parsed packet to the
controller, but the controller doesn't have to work with the same
knowledge of all packet fields as what the switch understands. Should
OXM lengths be allowed to be flexible when decoding in a controller?
Well, I guess that the controller should be responsible for ensuring
that it configures the TLV map correctly ahead of time.. so if the
controller already has this knowledge, then it's not unreasonable for
it to keep its own view of the TLV maps in sync with the switch. On
the flip side, we enforce that the controller must maintain the same
vll mff map as what the switch has.

I discussed this briefly with Jarno (thanks!), and figured it would be
easy enough to try this out..

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 1a0f0de594e7..f1e020a766c6 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -624,6 +624,8 @@ nx_pull_match_loose(struct ofpbuf *b, unsigned int
match_len,
const struct tun_table *tun_table,
const struct vl_mff_map *vl_mff_map)
{
+ovs_assert(!tun_table);
+ovs_assert(!vl_mff_map);
return nx_pull_match__(b, match_len, false, match, cookie, cookie_mask,
   tun_table, vl_mff_map);
}

When I ran the OVS testsuite with this applied, all the tests ran
successfully so it seems to me that it would be safe not to add the
extra vl_mff_map argument to these *_loose() functions. It would be
good if you could later on submit a separate patch to remove tun_table
argument from these as well.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto: Add some examples for 'ofproto/trace' in ovs-vswitchd man page

[Ben Pfaff]

On Fri, Mar 10, 2017 at 07:17:28PM +0100, Timothy Redaelli wrote:
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1339097
> Signed-off-by: Timothy Redaelli 

Thanks for the examples!

These first two look a little odd.  It wouldn't normally make sense to
include a mask on any field in an ofproto/trace request, like the masks
shown on the dl_dst fields below.  What's the goal there?
> +\fBTrace an unicast ICMP echo request on ingress port 1\fR
> +.RS 4
> +.nf
> +ofproto/trace br in_port=1,icmp,icmp_type=8,\\
> +dl_dst=00:00:00:00:00:00/01:00:00:00:00:00
> +.RE
> +.fi
> +.PP
> +\fBTrace an unicast ICMP echo reply on ingress port 1\fR
> +.RS 4
> +.nf
> +ofproto/trace br in_port=1,icmp,icmp_type=0,\\
> +dl_dst=00:00:00:00:00:00/01:00:00:00:00:00


Thanks,

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

Re: [ovs-dev] [PATCH] ovn-controller: Don't bind non-existent interfaces.

[Guru Shetty]

On 8 March 2017 at 09:42, Russell Bryant  wrote:

> On Wed, Mar 8, 2017 at 2:18 AM, Gurucharan Shetty  wrote:
> > There are multiple reasons why a interface can exist
> > in the Open vSwitch database but not exist in the system.
> > For e.g, a restart of a host after a system crash. Ideally,
> > whoever added the interface in the Open vSwitch database
> > should remove those interfaces. But that usually does not
> > happen in practise. Based on experience, I have observerd
>
> practice and observed
>
Ugh. Though I fixed this locally, I forgot to regenerate the patch to
apply. So the commit message still has the typo. Sorry about that.

>
> > that on any long lasting OVS installation there are always
> > a couple of stale interfaces.
> >
> > When a stale interface remains in the Open vSwitch database
> > and the container/VM initially backing that stale interface
> > is moved to a different machine, the two ovn-controllers
> > start over-writing the OVN-SB's port_binding table in a loop.
> >
> > This situation can be avoided, if ovn-controller only binds
> > the interfaces that actually have a valid 'ofport'.
> >
> > Signed-off-by: Gurucharan Shetty 
>
> This sounds reasonable to me.
>
> Acked-by: Russell Bryant 
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Don't bind non-existent interfaces.

[Guru Shetty]

On 8 March 2017 at 09:42, Russell Bryant  wrote:

> On Wed, Mar 8, 2017 at 2:18 AM, Gurucharan Shetty  wrote:
> > There are multiple reasons why a interface can exist
> > in the Open vSwitch database but not exist in the system.
> > For e.g, a restart of a host after a system crash. Ideally,
> > whoever added the interface in the Open vSwitch database
> > should remove those interfaces. But that usually does not
> > happen in practise. Based on experience, I have observerd
>
> practice and observed
>
> > that on any long lasting OVS installation there are always
> > a couple of stale interfaces.
> >
> > When a stale interface remains in the Open vSwitch database
> > and the container/VM initially backing that stale interface
> > is moved to a different machine, the two ovn-controllers
> > start over-writing the OVN-SB's port_binding table in a loop.
> >
> > This situation can be avoided, if ovn-controller only binds
> > the interfaces that actually have a valid 'ofport'.
> >
> > Signed-off-by: Gurucharan Shetty 
>
> This sounds reasonable to me.
>
> Acked-by: Russell Bryant 
>
Thanks. I applied this to master and 2.7
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofproto: Add some examples for 'ofproto/trace' in ovs-vswitchd man page

[Timothy Redaelli]

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1339097
Signed-off-by: Timothy Redaelli 
---
 ofproto/ofproto-unixctl.man | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man
index ce4f7a2..d61fc3e 100644
--- a/ofproto/ofproto-unixctl.man
+++ b/ofproto/ofproto-unixctl.man
@@ -135,3 +135,37 @@ necessary because the command does not ordinarily imply a 
particular
 OpenFlow version.  One exception is that, when \fIactions\fR includes
 an action that only OpenFlow 1.1 and later supports (such as
 \fBpush_vlan\fR), \fB\-consistent\fR is automatically enabled.
+.
+.IP "Usage examples:"
+.RS 4
+.PP
+\fBTrace an unicast ICMP echo request on ingress port 1\fR
+.RS 4
+.nf
+ofproto/trace br in_port=1,icmp,icmp_type=8,\\
+dl_dst=00:00:00:00:00:00/01:00:00:00:00:00
+.RE
+.fi
+.PP
+\fBTrace an unicast ICMP echo reply on ingress port 1\fR
+.RS 4
+.nf
+ofproto/trace br in_port=1,icmp,icmp_type=0,\\
+dl_dst=00:00:00:00:00:00/01:00:00:00:00:00
+.fi
+.RE
+.PP
+\fBTrace an ARP request on ingress port 1\fR
+.RS 4
+.nf
+ofproto/trace br in_port=1,arp,arp_op=1
+.fi
+.RE
+.PP
+\fBTrace an ARP reply on ingress port 1\fR
+.RS 4
+.nf
+ofproto/trace br in_port=1,arp,arp_op=2
+.fi
+.RE
+.RE
-- 
2.9.3

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

[ovs-dev] [PATCH 2/2] ovn-util: Allow /32 IP addresses for router ports.

[Gurucharan Shetty]

On Google cloud, a VM gets a /32 IP address. When OVN
is deployed on such VMs, the OVN gateway router's IP
address becomes a /32 IP address. This commit allows
such a configuration.

Signed-off-by: Gurucharan Shetty 
---
 ovn/lib/ovn-util.c |   7 +---
 tests/ovn.at   | 100 +
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index 99e4a0e..475fa97 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -171,7 +171,7 @@ extract_lrp_networks(const struct nbrec_logical_router_port 
*lrp,
 
 error = ip_parse_cidr(lrp->networks[i], , );
 if (!error) {
-if (!ip4 || plen == 32) {
+if (!ip4) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
 VLOG_WARN_RL(, "bad 'networks' %s", lrp->networks[i]);
 continue;
@@ -184,11 +184,6 @@ extract_lrp_networks(const struct 
nbrec_logical_router_port *lrp,
 
 error = ipv6_parse_cidr(lrp->networks[i], , );
 if (!error) {
-if (plen == 128) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(, "bad 'networks' %s", lrp->networks[i]);
-continue;
-}
 add_ipv6_netaddr(laddrs, ip6, plen);
 } else {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
diff --git a/tests/ovn.at b/tests/ovn.at
index bbbec90..6cf8d0c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6660,3 +6660,103 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], 
[hv2-vif1.expected])
 OVN_CLEANUP([hv1],[hv2],[hv3])
 
 AT_CLEANUP
+
+AT_SETUP([ovn -- /32 router IP address])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Logical network:
+# 2 LS 'foo' and 'alice' connected via router R1.
+# R1 connects to 'alice' with a /32 IP address. We use static routes and
+# nexthop to push traffic to a logical port in switch 'alice'
+
+ovn-nbctl lr-add R1
+
+ovn-nbctl ls-add foo
+ovn-nbctl ls-add alice
+
+# Connect foo to R1
+ovn-nbctl lrp-add R1 foo 00:00:00:01:02:03 192.168.1.1/24
+ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo type=router \
+  options:router-port=foo addresses=\"00:00:00:01:02:03\"
+
+# Connect alice to R1.
+ovn-nbctl lrp-add R1 alice 00:00:00:01:02:04 172.16.1.1/32
+ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \
+  type=router options:router-port=alice addresses=\"00:00:00:01:02:04\"
+
+# Create logical port foo1 in foo
+ovn-nbctl lsp-add foo foo1 \
+-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2"
+
+# Create logical port alice1 in alice
+ovn-nbctl lsp-add alice alice1 \
+-- lsp-set-addresses alice1 "f0:00:00:01:02:04 10.0.0.2"
+
+#install default route in R1 to use alice1's IP address as nexthop
+ovn-nbctl lr-route-add R1 0.0.0.0/0 10.0.0.2 alice
+
+# Create two hypervisor and create OVS ports corresponding to logical ports.
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+set interface hv1-vif1 external-ids:iface-id=foo1 \
+options:tx_pcap=hv1/vif1-tx.pcap \
+options:rxq_pcap=hv1/vif1-rx.pcap \
+ofport-request=1
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+ovs-vsctl -- add-port br-int hv2-vif1 -- \
+set interface hv2-vif1 external-ids:iface-id=alice1 \
+options:tx_pcap=hv2/vif1-tx.pcap \
+options:rxq_pcap=hv2/vif1-rx.pcap \
+ofport-request=1
+
+
+# Pre-populate the hypervisors' ARP tables so that we don't lose any
+# packets for ARP resolution (native tunneling doesn't queue packets
+# for ARP resolution).
+ovn_populate_arp
+
+# Allow some time for ovn-northd and ovn-controller to catch up.
+# XXX This should be more systematic.
+sleep 1
+
+ip_to_hex() {
+printf "%02x%02x%02x%02x" "$@"
+}
+
+# Send ip packets between foo1 and alice1
+src_mac="f0010203"
+dst_mac="00010203"
+src_ip=`ip_to_hex 192 168 1 2`
+dst_ip=`ip_to_hex 10 0 0 2`
+packet=${dst_mac}${src_mac}0800451c4011${src_ip}${dst_ip}00350008
+
+# Send the first packet to trigger a ARP response and population of
+# mac_bindings table.
+as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
+OVS_WAIT_UNTIL([test `ovn-sbctl find mac_binding ip="10.0.0.2" | wc -l` -gt 0])
+
+# Send the second packet to reach the destination.
+as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
+
+# Packet to Expect at 'alice1'
+src_mac="00010204"
+dst_mac="f0010204"
+src_ip=`ip_to_hex 192 168 1 2`
+dst_ip=`ip_to_hex 10 0 0 2`
+echo 
"${dst_mac}${src_mac}0800451c3f110100${src_ip}${dst_ip}00350008"
 > expected
+
+OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
+
+OVN_CLEANUP([hv1],[hv2])
+
+AT_CLEANUP
-- 
1.9.1

___
dev mailing list
d...@openvswitch.org

[ovs-dev] [PATCH 1/2] ovn-northd: Allow static routes with nexthop in different subnet.

[Gurucharan Shetty]

There are cases where the default gateway of a interface is in
a different subnet than its IP address. Linux allows such
configuration. For e.g, one could set the IP address of
a Linux interface as 172.16.1.2/32 and then give it a default
gateway of 172.16.1.1.  This can be done for e.g. by running the
following commands.

ifconfig eth0 172.16.1.2 netmask 255.255.255.255 broadcast 172.16.1.2
route add 172.16.1.1 dev eth0
route add default gw 172.16.1.1

The above configuration is what google cloud uses for its VMs.

In OVN static routes, we currently have the ability to specify the
router port via which the packet needs to be pushed out to reach a
next hop.  But when support for IPv6 was added, we only allowed
nexthops to be in the same subnet as one of the router's IP addresses.

This commit relaxes that restriction. When a outport is specified in
static routes and when a nexthop is in a different subnet than any
of the router IP addresses, we will assume that it is reachable from
the first IP address of the router.  Since this is a corner case,
we just go with the first IP address.  If it turns out that there
are more cases, we can let users choose the IP address via which
the destination is reachable.

Signed-off-by: Gurucharan Shetty 
---
Patch2 of the series includes a unit test that also covers this
case.
---
 ovn/northd/ovn-northd.c | 17 +
 ovn/ovn-nb.xml  |  5 -
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index cc9b934..59ebc05 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3697,6 +3697,23 @@ build_static_route_flow(struct hmap *lflows, struct 
ovn_datapath *od,
 goto free_prefix_s;
 }
 lrp_addr_s = find_lrp_member_ip(out_port, route->nexthop);
+if (!lrp_addr_s) {
+/* There are no IP networks configured on the router's port via
+ * which 'route->nexthop' is theoretically reachable.  But since
+ * 'out_port' has been specified, we honor it by trying to reach
+ * 'route->nexthop' via the first IP address of 'out_port'.
+ * (There are cases, e.g in GCE, where each VM gets a /32 IP
+ * address and the default gateway is still reachable from it.) */
+if (is_ipv4) {
+if (out_port->lrp_networks.n_ipv4_addrs) {
+lrp_addr_s = out_port->lrp_networks.ipv4_addrs[0].addr_s;
+}
+} else {
+if (out_port->lrp_networks.n_ipv6_addrs) {
+lrp_addr_s = out_port->lrp_networks.ipv6_addrs[0].addr_s;
+}
+}
+}
 } else {
 /* output_port is not specified, find the
  * router port matching the next hop. */
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 88a6082..c1f4e1f 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1264,7 +1264,10 @@
 The name of the  via which the packet
 needs to be sent out.  This is optional and when not specified,
 OVN will automatically figure this out based on the
-.
+.  When this is specified and there are
+multiple IP addresses on the router port and none of them are in the
+same subnet of , OVN chooses the first IP
+address as the one via which the  is reachable.
   
 
   
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] tests: Fix mcast test on slow systems

[Ben Pfaff]

On Wed, Mar 08, 2017 at 02:31:56PM +, Alin Serdean wrote:
> On slow systems(or which start processes slow) the test:
> `testing mcast - delete the port mdb when port destroyed`
> is influenced by the running time.
> i.e.: 
> http://64.119.130.115/ovs/911b7e9b08b9f4f890eeecd228d5124f4ce94d4e/testsuite.dir/2326/testsuite.log.gz
> 
> This patches adds a time stop on vswitchd.
> 
> Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] ovn-northd: Skip icmp4 packets destined for router ports from conntrack

[Numan Siddique]

On Fri, Mar 10, 2017 at 10:22 AM, Numan Siddique 
wrote:

> Thanks for the review. Please see inline.
>
>
> On Fri, Mar 10, 2017 at 1:44 AM, Russell Bryant  wrote:
>
>> On Mon, Feb 27, 2017 at 12:59 AM,   wrote:
>> > From: Numan Siddique 
>> >
>> > Presently, the icmp4 requests to the router gateway ip are sent to the
>> > connectiont tracker, but the icmp4 reply packets responded by
>> > 'lr_in_ip_input' stage are not sent to the connection tracker.
>> > Also no zone ids are assigned for the router ports. Because of which
>> > the icmp4 request packets in the connection tracker will be in the
>> > UNREPLIED state.
>
>
​​Also, this commit message is not accurate.
​

> If the CMS has added ACLs to drop packets which
>> > are not in ESTABLISHED state, the icmp4 replies will be dropped.
>> >
>> > To fix this issue, this patch adds a priority-110 flow in
>> 'ls_in_pre_acl'
>> > stage which doesn't set reg0[0] = 1 for icmp4 request packets destined
>> > to the router gateway ips.
>
> >
>> > Alternate solution would be to assign zone ids for the router ports
>> > and send the packets from the router ports to the connection tracker.
>> >
>> > The approach used in this patch seems to be simpler.
>> >
>> > Signed-off-by: Numan Siddique 
>> > ---
>> >  ovn/northd/ovn-northd.8.xml | 29 +++---
>> >  ovn/northd/ovn-northd.c | 92 +++---
>> ---
>> >  2 files changed, 79 insertions(+), 42 deletions(-)
>>
>>
>> Can you clarify where the packet gets dropped?  It seems like we have
>> flows trying to handle this already.  We skip conntrack for the router
>> interface ports.  Roughly, I would expect:
>>
>>
> ​
> ​The packets are getting dropped because of the flow
>
> ​
> table=52, n_packets=40, n_bytes=3920,  
> priority=2001,ct_state=-est+trk,ip,reg15=0x3,metadata=0x3
> ac
> tions=drop
>
> This flow corresponds to logical flow -
> table=4 (ls_out_acl ), priority=2001 , match=((!ct.est || (ct.est
> && ct_label.blocked == 1)) && (outport == 
> "ce575934-f308-45b8-b9cd-457646da213d"
> && ip)), action=(drop;)
> ​
>
>
> ​This logical flow is added by neutron OVN plugin when the port is
> configured with security groups. When I clear the security groups for the
> port or add a specific security group rule to allow icmp, it works fine.
> ​
>
>
>> 1) inport == lport1, logical switch ingress pipeline.  packet sent
>> through conntrack.
>>
>> 2) outport == router interface port, logical switch egress pipeline.
>> packet *skips* conntrack since it's a router interface.
>>
>> 3) router datapath, icmp respose generated, sent back to logical switch
>> ...
>>
>> 4) inport == router interface, logical switch ingress pipeline, packet
>> *skips* conntrack since it's a router interface
>>
>> 5) outport == lport1, logical switch egress pipeline, packet sent
>> through conntrack, which should find an existing conntrack entry
>> established in step 1.  packet delivered to lport1.
>>
>>
> The connection tracking entry ​has this
>
> $ sudo conntrack -L | grep 10.0.0.1
> conntrack v1.4.3 (conntrack-tools): 54 flow entries have been shown.
> icmp 1 29 src=10.0.0.6 dst=10.0.0.1 type=8 code=0 id=7486 [UNREPLIED]
> src=10.0.0.1 dst=10.0.0.6 type=0 code=0 id=7486 mark=0
> secctx=system_u:object_r:unlabeled_t:s0 zone=1 use=1
>
> You think this should be addressed by neutron OVN plugin ?
>
>
>
> Where in the above process is it coming apart?  If it's broken, it
>> sounds like a more general problem than ICMP.  It would be any type of
>> traffic to the router IP where we expect a response.
>>
>> --
>> Russell Bryant
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] docs: Use DPDK 16.11.1 stable release.

[Stokes, Ian]

> 2017-03-09 13:15 GMT-08:00 Ian Stokes :
> > DPDK now provides a stable release branch. Modify dpdk docs and travis
> > linux build script to use the DPDK 16.11.1 stable branch to benefit
> > from most recent bug fixes.
> >
> > Signed-off-by: Ian Stokes 
> 
> Thanks for the patch, it looks good to me.
> 
> This is for master and branch-2.7, right?
> 
Correct, this patch is meant for master and 2.7.

I've created a separate patch for 2.6 branch as there was considerable changes 
to documentation layout between 2.6 and 2.7.

> Just one comment, this appears to break the travis build:
> 
> https://travis-ci.org/ddiproietto/ovs/jobs/209586728
> 
> I guess we need to update the --with-dpdk argument in .travis/linux-
> build.sh

Thanks Daniele, a silly mistake on my part, I've sent a v2 with the fix

https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329654.html

I sent a v2 of the 2.6 patch also

https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329655.html

Ian
> 
> > ---
> >  .travis/linux-build.sh   |   10 +-
> >  Documentation/faq/releases.rst   |   10 +-
> >  Documentation/intro/install/dpdk.rst |6 +++---
> >  Documentation/topics/dpdk/vhost-user.rst |8 
> >  4 files changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index
> > 4175d72..06c8422 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -52,13 +52,13 @@ function install_kernel()  function install_dpdk()
> > {
> >  if [ -n "$DPDK_GIT" ]; then
> > -git clone $DPDK_GIT dpdk-$1
> > -cd dpdk-$1
> > -git checkout v$1
> > +git clone $DPDK_GIT dpdk-stable-$1
> > +cd dpdk-stable-$1
> > +git checkout tags/v$1
> >  else
> >  wget http://fast.dpdk.org/rel/dpdk-$1.tar.gz
> >  tar xzvf dpdk-$1.tar.gz > /dev/null
> > -cd dpdk-$1
> > +cd dpdk-stable-$1
> >  fi
> >  find ./ -type f | xargs sed -i 's/max-inline-insns-single=100/max-
> inline-insns-single=400/'
> >  echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp @@ -80,7
> > +80,7 @@ fi
> >
> >  if [ "$DPDK" ]; then
> >  if [ -z "$DPDK_VER" ]; then
> > -DPDK_VER="16.11"
> > +DPDK_VER="16.11.1"
> >  fi
> >  install_dpdk $DPDK_VER
> >  if [ "$CC" = "clang" ]; then
> > diff --git a/Documentation/faq/releases.rst
> > b/Documentation/faq/releases.rst index 118c88d..98f5636 100644
> > --- a/Documentation/faq/releases.rst
> > +++ b/Documentation/faq/releases.rst
> > @@ -152,16 +152,16 @@ Q: What DPDK version does each Open vSwitch
> release work with?
> >  A: The following table lists the DPDK version against which the
> given
> >  versions of Open vSwitch will successfully build.
> >
> > - =
> > + ===
> >  Open vSwitch DPDK
> > - =
> > + ===
> >  2.2.x1.6
> >  2.3.x1.6
> >  2.4.x2.0
> >  2.5.x2.2
> > -2.6.x16.07
> > -2.7.x16.11
> > - =
> > +2.6.x16.07.2
> > +2.7.x16.11.1
> > + ===
> >
> >  Q: I get an error like this when I configure Open vSwitch::
> >
> > diff --git a/Documentation/intro/install/dpdk.rst
> > b/Documentation/intro/install/dpdk.rst
> > index 3018590..b947bd5 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -64,9 +64,9 @@ Install DPDK
> >  #. Download the `DPDK sources`_, extract the file and set
> ``DPDK_DIR``::
> >
> > $ cd /usr/src/
> > -   $ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
> > -   $ tar xf dpdk-16.11.tar.xz
> > -   $ export DPDK_DIR=/usr/src/dpdk-16.11
> > +   $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> > +   $ tar xf dpdk-16.11.1.tar.xz
> > +   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
> > $ cd $DPDK_DIR
> >
> >  #. (Optional) Configure DPDK as a shared library diff --git
> > a/Documentation/topics/dpdk/vhost-user.rst
> > b/Documentation/topics/dpdk/vhost-user.rst
> > index 5448bd2..ba22684 100644
> > --- a/Documentation/topics/dpdk/vhost-user.rst
> > +++ b/Documentation/topics/dpdk/vhost-user.rst
> > @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in
> > :ref:`dpdk-vhost-user` or  DPDK sources to VM and build DPDK::
> >
> >  $ cd /root/dpdk/
> > -$ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
> > -$ tar xf dpdk-16.11.tar.xz
> > -$ export DPDK_DIR=/root/dpdk/dpdk-16.11
> > +$ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> > +$ tar xf dpdk-16.11.1.tar.xz
> > +$ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
> >  $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> >  $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> >  $ cd $DPDK_DIR
> > @@ -364,7 +364,7 

[ovs-dev] [PATCH v2] docs: Use DPDK 16.11.1 stable release.

[Ian Stokes]

DPDK now provides a stable release branch. Modify dpdk docs and travis linux
build script to use the DPDK 16.11.1 stable branch to benefit from most
recent bug fixes.

Signed-off-by: Ian Stokes 
---
v1 -> v2
* Set correct path to DPDK stable branch for EXTRA_OPTS in travis linux
  build.
---
 .travis/linux-build.sh   |   12 ++--
 Documentation/faq/releases.rst   |   10 +-
 Documentation/intro/install/dpdk.rst |6 +++---
 Documentation/topics/dpdk/vhost-user.rst |8 
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 4175d72..8750d68 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -52,13 +52,13 @@ function install_kernel()
 function install_dpdk()
 {
 if [ -n "$DPDK_GIT" ]; then
-git clone $DPDK_GIT dpdk-$1
-cd dpdk-$1
-git checkout v$1
+git clone $DPDK_GIT dpdk-stable-$1
+cd dpdk-stable-$1
+git checkout tags/v$1
 else
 wget http://fast.dpdk.org/rel/dpdk-$1.tar.gz
 tar xzvf dpdk-$1.tar.gz > /dev/null
-cd dpdk-$1
+cd dpdk-stable-$1
 fi
 find ./ -type f | xargs sed -i 
's/max-inline-insns-single=100/max-inline-insns-single=400/'
 echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
@@ -80,14 +80,14 @@ fi
 
 if [ "$DPDK" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="16.11"
+DPDK_VER="16.11.1"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
 # Disregard cast alignment errors until DPDK is fixed
 CFLAGS="$CFLAGS -Wno-cast-align"
 fi
-EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-$DPDK_VER/build"
+EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-stable-$DPDK_VER/build"
 elif [ "$CC" != "clang" ]; then
 # DPDK headers currently trigger sparse errors
 SPARSE_FLAGS="$SPARSE_FLAGS -Wsparse-error"
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 118c88d..98f5636 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -152,16 +152,16 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 A: The following table lists the DPDK version against which the given
 versions of Open vSwitch will successfully build.
 
- =
+ ===
 Open vSwitch DPDK
- =
+ ===
 2.2.x1.6
 2.3.x1.6
 2.4.x2.0
 2.5.x2.2
-2.6.x16.07
-2.7.x16.11
- =
+2.6.x16.07.2
+2.7.x16.11.1
+ ===
 
 Q: I get an error like this when I configure Open vSwitch::
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 3018590..b947bd5 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -64,9 +64,9 @@ Install DPDK
 #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
 
$ cd /usr/src/
-   $ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
-   $ tar xf dpdk-16.11.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-16.11
+   $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
+   $ tar xf dpdk-16.11.1.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
$ cd $DPDK_DIR
 
 #. (Optional) Configure DPDK as a shared library
diff --git a/Documentation/topics/dpdk/vhost-user.rst 
b/Documentation/topics/dpdk/vhost-user.rst
index 5448bd2..ba22684 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -278,9 +278,9 @@ To begin, instantiate a guest as described in 
:ref:`dpdk-vhost-user` or
 DPDK sources to VM and build DPDK::
 
 $ cd /root/dpdk/
-$ wget http://fast.dpdk.org/rel/dpdk-16.11.tar.xz
-$ tar xf dpdk-16.11.tar.xz
-$ export DPDK_DIR=/root/dpdk/dpdk-16.11
+$ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
+$ tar xf dpdk-16.11.1.tar.xz
+$ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
 $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
 $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
 $ cd $DPDK_DIR
@@ -364,7 +364,7 @@ Sample XML
 
 
   
-  
+  
   
   
 
-- 
1.7.0.7

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

[ovs-dev] [PATCH v2 branch-2.6] docs: Use DPDK 16.07.2 stable release

[Ian Stokes]

DPDK now provides a stable release branch. Modify dpdk docs and travis
linux build script to use the DPDK 16.07.2 stable branch to benefit from
most recent bug fixes.

Signed-off-by: Ian Stokes 
---
v1 -> v2
* Set correct path to DPDK stable branch for EXTRA_OPTS in travis linux
  build.
---
 .travis/linux-build.sh   |   14 +++---
 FAQ.md   |4 ++--
 INSTALL.DPDK-ADVANCED.md |6 +++---
 INSTALL.DPDK.md  |   22 ++
 4 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 3bcec93..f15f706 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -52,13 +52,13 @@ function install_kernel()
 function install_dpdk()
 {
 if [ -n "$DPDK_GIT" ]; then
-git clone $DPDK_GIT dpdk-$1
-cd dpdk-$1
-git checkout v$1
+git clone $DPDK_GIT dpdk-stable-$1
+cd dpdk-stable-$1
+git checkout tags/v$1
 else
-wget http://www.dpdk.org/browse/dpdk/snapshot/dpdk-$1.tar.gz
+wget http://fast.dpdk.org/rel/dpdk-$1.tar.gz
 tar xzvf dpdk-$1.tar.gz > /dev/null
-cd dpdk-$1
+cd dpdk-stable-$1
 fi
 find ./ -type f | xargs sed -i 
's/max-inline-insns-single=100/max-inline-insns-single=400/'
 echo 'CONFIG_RTE_BUILD_FPIC=y' >>config/common_linuxapp
@@ -80,14 +80,14 @@ fi
 
 if [ "$DPDK" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="16.07"
+DPDK_VER="16.07.2"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
 # Disregard cast alignment errors until DPDK is fixed
 CFLAGS="$CFLAGS -Wno-cast-align"
 fi
-EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-$DPDK_VER/build"
+EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=./dpdk-stable-$DPDK_VER/build"
 elif [ "$CC" != "clang" ]; then
 # DPDK headers currently trigger sparse errors
 SPARSE_FLAGS="$SPARSE_FLAGS -Wsparse-error"
diff --git a/FAQ.md b/FAQ.md
index cf30f9b..75a393b 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -256,12 +256,12 @@ A: The following table lists the DPDK version against 
which the
given versions of Open vSwitch will successfully build.
 
 | Open vSwitch | DPDK
-|::|:-:
+|::|:---:
 |2.2.x | 1.6
 |2.3.x | 1.6
 |2.4.x | 2.0
 |2.5.x | 2.2
-|2.6.x | 16.07
+|2.6.x | 16.07.2
 
 ### Q: I get an error like this when I configure Open vSwitch:
 
diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
index e3603a1..ae21aca 100755
--- a/INSTALL.DPDK-ADVANCED.md
+++ b/INSTALL.DPDK-ADVANCED.md
@@ -46,7 +46,7 @@ for DPDK and OVS.
 For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
 
 ```
-export DPDK_DIR=/usr/src/dpdk-16.07
+export DPDK_DIR=/usr/src/dpdk-stable-16.07.2
 export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
 make install T=$DPDK_TARGET DESTDIR=install
 ```
@@ -342,7 +342,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
cd /usr/src/cmdline_generator
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/cmdline_generator.c
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/Makefile
-   export RTE_SDK=/usr/src/dpdk-16.07
+   export RTE_SDK=/usr/src/dpdk-stable-16.07.2
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
./build/cmdline_generator -m -p dpdkr0 XXX
@@ -366,7 +366,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
mount -t hugetlbfs nodev /dev/hugepages (if not already mounted)
 
# Build the DPDK ring application in the VM
-   export RTE_SDK=/root/dpdk-16.07
+   export RTE_SDK=/root/dpdk-stable-16.07.2
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
 
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 30e9258..9ab29f3 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -21,7 +21,7 @@ The DPDK support of Open vSwitch is considered 'experimental'.
 
 ### Prerequisites
 
-* Required: DPDK 16.07
+* Required: DPDK 16.07.2
 * Hardware: [DPDK Supported NICs] when physical ports in use
 
 ##  2. Building and Installation
@@ -42,10 +42,9 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
 
  ```
  cd /usr/src/
- wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
- unzip dpdk-16.07.zip
-
- export DPDK_DIR=/usr/src/dpdk-16.07
+ wget http://fast.dpdk.org/rel/dpdk-16.07.2.tar.xz
+ tar xf dpdk-16.07.2.tar.xz
+ export DPDK_DIR=/usr/src/dpdk-stable-16.07.2
  cd $DPDK_DIR
  ```
 
@@ -372,9 +371,9 @@ can be found in [Vhost Walkthrough].
 
   ```
   cd /root/dpdk/
-  wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
-  unzip dpdk-16.07.zip
-  

Re: [ovs-dev] Documentation: Report errors for use of features not in Sphinx 1.1.3.

[Ilya Maximets]

I've sent the patch for removing highlighting at all here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329651.html

Only 'windows.rst' uses this functionality. So, I think, it's better
to just remove it and forbid the highlighting to avoid any issues
with external dependencies.

Best regards, Ilya Maximets.

On 10.03.2017 10:47, Ilya Maximets wrote:
> On 10.03.2017 02:27, Ben Pfaff wrote:
>> On Thu, Mar 09, 2017 at 06:15:13PM +0300, Ilya Maximets wrote:
>>> On 07.03.2017 21:54, Ben Pfaff wrote:
 Signed-off-by: Ben Pfaff 
 Acked-by: Stephen Finucane 
 ---
  Documentation/automake.mk  | 15 ++-
  Documentation/sphinx-version-blacklist |  2 ++
  2 files changed, 16 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/sphinx-version-blacklist

 diff --git a/Documentation/automake.mk b/Documentation/automake.mk
 index a74807fde532..f7f1fe61d1b7 100644
 --- a/Documentation/automake.mk
 +++ b/Documentation/automake.mk
 @@ -86,7 +86,8 @@ EXTRA_DIST += \
Documentation/internals/contributing/documentation-style.rst \
Documentation/internals/contributing/libopenvswitch-abi.rst \
Documentation/internals/contributing/submitting-patches.rst \
 -  Documentation/requirements.txt
 +  Documentation/requirements.txt \
 +  Documentation/sphinx-version-blacklist
  
  # You can set these variables from the command line.
  SPHINXOPTS =
 @@ -120,3 +121,15 @@ endif
  .PHONY: htmldocs
  .PHONY: check-docs
  .PHONY: clean-docs
 +
 +ALL_LOCAL += sphinx-version-check
 +sphinx-version-check: $(EXTRA_DIST)
 +  @if grep -n -f $(srcdir)/Documentation/sphinx-version-blacklist $?; \
 +  then \
 +echo "See above for list of uses of features that Sphinx 1.1.3"; \
 +echo "does not support.  Please avoid using these features.."; \
 +exit 1; \
 +  else \
 +  : > $@; \
 +  fi
 +CLEANFILES += sphinx-version-check
 diff --git a/Documentation/sphinx-version-blacklist 
 b/Documentation/sphinx-version-blacklist
 new file mode 100644
 index ..a67339bf2758
 --- /dev/null
 +++ b/Documentation/sphinx-version-blacklist
 @@ -0,0 +1,2 @@
 +code-block:: *ps1con
 +code-block:: *doscon
>>>
>>> I don't feel this patch is fully correct, because it's not the features of
>>> sphinx. And its version not really connected with version of 'pygments' 
>>> library.
>>
>> OK, can you explain the real problem then?  We're making changes to the
>> documentation on the basis that old versions of Sphinx does not support
>> features.
> 
> The real problem is the version of 'pygments' library. Sphinx uses this 
> library
> to highlight code blocks.
> So, RHEL7.3 contains package 'python-pygments-2.0.2', but lexers 'ps1con' and
> 'doscon' was introduced only in 'pygments-2.1'. That is why build fails.
> 
> '''
> class pygments.lexers.shell.MSDOSSessionLexer
> Short names:  doscon
> Filenames:None
> MIME types:   None
> 
> Lexer for simplistic MSDOS sessions.
> 
> New in version 2.1.
> 
> class pygments.lexers.shell.PowerShellSessionLexer
> Short names:  ps1con
> Filenames:None
> MIME types:   None
> 
> Lexer for simplistic Windows PowerShell sessions.
> 
> New in version 2.1.
> '''
> 
> On page [1] of 'pygments' project you can check the minimal version required
> for every lexer.
> 
> Maybe we need to add minimal version of 'pygments' to requirements.txt .
> In this case we will be able to create a whitelist of all supported lexers.
> 
> Another option:
> Do we need the code highlighting at all?
> We can just replace all the '.. code-block:: ' with simple '::' 
> [2].
> In this case, we will not have any external dependencies other than sphinx.
> 
> P.S. My previous patch [3] is just about ability to build documentation
>  with sphinx 1.1 because there is no any reason to block it.
> 
> [1] http://pygments.org/docs/lexers/
> [2] http://www.sphinx-doc.org/en/stable/rest.html#source-code
> [3] https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329590.html
> 
> Best regards, Ilya Maximets.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Documentation: Remove external dependence on pygments.

[Ilya Maximets]

Current documentation uses syntax highlighting in 'sphinx'
via 'pygments' library. This leads to build failures on the
systems with old version of this library.

In fact that only 'windows.rst' uses highlighting it's a
very simple change. This helps us to avoid build issues
on different systems and allows to remove painful external
dependency.

Signed-off-by: Ilya Maximets 
---
 Documentation/conf.py  |  3 -
 .../internals/contributing/documentation-style.rst | 10 ++-
 Documentation/intro/install/windows.rst| 88 +++---
 Documentation/topics/language-bindings.rst |  2 +-
 4 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 5909669..6a924b3 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -107,9 +107,6 @@ exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']
 #
 # show_authors = False
 
-# The name of the Pygments (syntax highlighting) style to use.
-# pygments_style = 'friendly'
-
 # A list of ignored prefixes for module index sorting.
 # modindex_common_prefix = []
 
diff --git a/Documentation/internals/contributing/documentation-style.rst 
b/Documentation/internals/contributing/documentation-style.rst
index ea41a07..99eec69 100644
--- a/Documentation/internals/contributing/documentation-style.rst
+++ b/Documentation/internals/contributing/documentation-style.rst
@@ -115,9 +115,11 @@ Titles
 Code
 
 
-- Use ``::``, the ``code`` role or the ``code-block:: `` role to prefix
-  code. The ``code-block:: `` format is preferred as this provides
-  syntax highlighting for non-Python languages, such as Bash or PowerShell.
+- Use ``::`` to prefix code.
+
+- Don't use syntax highlighting such as ``.. highlight:: `` or
+  ``code-block:: `` because it depends on external ``pygments``
+  library.
 
 - Prefix commands with ``$``.
 
@@ -259,7 +261,7 @@ Figures and Other Media
 - All images should be in PNG format and compressed where possible. For PNG
   files, use OptiPNG and AdvanceCOMP's ``advpng``:
 
-  .. code-block:: shell
+  ::
 
  $ optipng -o7 -zm1-9 -i0 -strip all 
  $ advpng -z4 
diff --git a/Documentation/intro/install/windows.rst 
b/Documentation/intro/install/windows.rst
index caa9f40..2be4eb5 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ -63,7 +63,7 @@ The following explains the steps in some detail.
   We require that you have Python six and pypiwin32 libraries installed.
   The libraries can be installed via pip command:
 
-   .. code-block:: console
+   ::
 
   $ pip install six
   $ pip install pypiwin32
@@ -140,7 +140,7 @@ you pulled the sources directly from an Open vSwitch Git 
tree or got a
 Git tree snapshot, then run boot.sh in the top source directory to build
 the "configure" script:
 
-.. code-block:: console
+::
 
$ ./boot.sh
 
@@ -153,7 +153,7 @@ Configure the package by running the configure script.  You 
should provide some
 configure options to choose the right compiler, linker, libraries, Open vSwitch
 component installation directories, etc. For example:
 
-.. code-block:: console
+::
 
$ ./configure CC=./build-aux/cccl LD="$(which link)" \
LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
@@ -169,7 +169,7 @@ component installation directories, etc. For example:
 
 To configure with SSL support, add the requisite additional options:
 
-.. code-block:: console
+::
 
$ ./configure CC=./build-aux/cccl LD="`which link`"  \
LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
@@ -181,7 +181,7 @@ To configure with SSL support, add the requisite additional 
options:
 
 Finally, to the kernel module also:
 
-.. code-block:: console
+::
 
$ ./configure CC=./build-aux/cccl LD="`which link`" \
LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
@@ -211,7 +211,7 @@ building on Linux, FreeBSD, or NetBSD.
 
 #. Run make for the ported executables in the top source directory, e.g.:
 
-   .. code-block:: console
+   ::
 
   $ make
 
@@ -225,25 +225,25 @@ building on Linux, FreeBSD, or NetBSD.
   all MinGW sessions and then run the below command from MSVC developers
   command prompt.:
 
-  .. code-block:: doscon
+  ::
 
  > mingw-get upgrade msys-core-bin=1.0.17-1
 
 #. To run all the unit tests in Open vSwitch, one at a time:
 
-   .. code-block:: console
+   ::
 
   $ make check
 
To run all the unit tests in Open vSwitch, up to 8 in parallel:
 
-   .. code-block:: console
+   ::
 
   $ make check TESTSUITEFLAGS="-j8"
 
 #. To install all the compiled executables on the local machine, run:
 
-   .. code-block:: console
+   ::
 
   $ make install
 
@@ -276,7 +276,7 @@ Now run ``./uninstall.cmd`` to remove the old extension. 
Once complete, run
 turn on ``TESTSIGNING`` boot option or 'Disable Driver Signature
 Enforcement' during boot.  The following 

Re: [ovs-dev] [PATCH ovs V3 00/25] Introducing HW offload support for openvswitch

[Simon Horman]

On Wed, Feb 08, 2017 at 05:29:13PM +0200, Roi Dayan wrote:
> This patch series introduces rule offload functionality to dpif-netlink 
> via netdev ports new flow offloading API. The user can specify whether to
> enable rule offloading or not via OVS configuration. Netdev providers
> are able to implement netdev flow offload API in order to offload rules.
> 
> This patch series also implements one offload scheme for netdev-linux,
> using TC flower classifier, which was chosen because its sort of natural
> to state OVS DP rules for this classifier. However, the code can be
> extended to support other classifiers such as U32, eBPF, etc which support
> offload as well.
> 
> The use-case we are currently addressing is the newly sriov switchdev mode
> in the Linux kernel which was introduced in version 4.8 [1][2].
> This series was tested against sriov vfs vports representors of the
> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.

Hi Roi,

its been a little while since v3 was posted.
I'd like to encourage you to post v4 and with a view to moving
towards getting this feature accepted.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev