Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.

[Stokes, Ian]

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Tuesday, February 13, 2018 10:35 AM
> To: Stokes, Ian ; d...@openvswitch.org
> Cc: antonio.fische...@gmail.com; Ilya Maximets ;
> Jan Scheurich 
> Subject: Re: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
> 
> On 02/13/2018 09:07 AM, Stokes, Ian wrote:
> > Hi all,
> >
> > Just pinging for feedback for this patch. If we are to include this for
> branch 2.9 as a bug fix I think we need feedback quite soon.
> >
> > Have people had time to validate and review?
> >
> 
> Hi Ian,
> 
> As per the other thread I have reviewed and tested multiple ports with MTU
> reconfigs - creating new mempools, freeing unused mempools, ports sharing
> mempools, ports having separate mempools, numa based reconfig from vhost
> etc. All working fine, so LGTM.

Thanks for reviewing and validating.

> 
> Are you going to send a non-rfc version of this patch?

I've submitted a non rfc patch now, if you can respond with tested-by tags 
there that would be great.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344398.html

Thanks
Ian

> 
> thanks,
> Kevin.
> 
> > Thanks
> > Ian
> >
> >> -Original Message-
> >> From: Stokes, Ian
> >> Sent: Thursday, February 8, 2018 11:39 AM
> >> To: d...@openvswitch.org
> >> Cc: Stokes, Ian ; antonio.fische...@gmail.com;
> >> Ilya Maximets ; Kevin Traynor
> >> ; Jan Scheurich 
> >> Subject: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
> >>
> >> This commit manually reverts the current per port mempool model to
> >> the previous shared mempool model for DPDK ports.
> >>
> >> OVS previously used a shared mempool model for ports with the same
> >> MTU configuration. This was replaced by a per port mempool model to
> >> address issues flagged by users such as:
> >>
> >> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-
> >> September/042560.html
> >>
> >> However the per port model has a number of issues including:
> >>
> >> 1. Requires an increase in memory resource requirements to support
> >> the same number of ports as the shared port model.
> >> 2. Incorrect algorithm for mbuf provisioning for each mempool.
> >>
> >> These are considered blocking factors for current deployments of OVS
> >> when upgrading to OVS 2.9 as a memory redimensioning will be
> >> required. This may not be possible for users.
> >>
> >> For clarity, the commits whose changes are removed include the
> >> following:
> >>
> >> netdev-dpdk: Create separate memory pool for each port: d555d9b
> >> netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> >> mempool names to reflect socket id: f06546a
> >> netdev-dpdk: skip init for existing mempools: 837c176
> >> netdev-dpdk: manage failure in mempool name creation: 65056fd
> >> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> >> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> >> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> >> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> >> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> >> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> >> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> >>
> >> Due to the number of commits and period of time they were introduced
> >> over, a simple revert was not possible. All code from the commits
> >> above is removed and the shared mempool code reintroduced as it was
> >> before its replacement.
> >>
> >> Code introduced by commit
> >>
> >> netdev-dpdk: Add debug appctl to get mempool information: be48173
> >>
> >> has been modified to work with the shared mempool model.
> >>
> >> Cc: antonio.fische...@gmail.com
> >> Cc: Ilya Maximets 
> >> Cc: Kevin Traynor 
> >> Cc: Jan Scheurich 
> >> Signed-off-by: Ian Stokes 
> >> ---
> >>  lib/netdev-dpdk.c | 246
> >> ++---
> >> -
> >>  1 file changed, 138 insertions(+), 108 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> 94fb163..6f3378b
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -91,13 +91,24 @@ static struct vlog_rate_limit rl =
> >> VLOG_RATE_LIMIT_INIT(5, 20);
> >>  #define NETDEV_DPDK_MBUF_ALIGN  1024
> >>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
> >>
> >> -/* Min number of packets in the mempool.  OVS tries to allocate a
> >> mempool with
> >> - * roughly estimated number of mbufs: if this fails (because the
> >> system doesn't
> >> - * have enough hugepages) we keep halving the number until the
> >> allocation
> >> - * succeeds or we reach MIN_NB_MBUF */
> >> +/* Max and min number of packets in the mempool.  OVS tries to
> >> +allocate a
> >> + * mempool with MAX_NB_MBUF: if 

Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.

[Venkatesan Pradeep]

Hi,

I've also reviewed the code and done tests to exercise the mempool code. The 
behavior seems to be same as with previous versions that had the shared mempool 
code. 

Thanks,

Pradeep

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Kevin Traynor
> Sent: Tuesday, February 13, 2018 4:05 PM
> To: Stokes, Ian ; d...@openvswitch.org
> Cc: Ilya Maximets ; antonio.fische...@gmail.com
> Subject: Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared
> mempools.
> 
> On 02/13/2018 09:07 AM, Stokes, Ian wrote:
> > Hi all,
> >
> > Just pinging for feedback for this patch. If we are to include this for 
> > branch
> 2.9 as a bug fix I think we need feedback quite soon.
> >
> > Have people had time to validate and review?
> >
> 
> Hi Ian,
> 
> As per the other thread I have reviewed and tested multiple ports with MTU
> reconfigs - creating new mempools, freeing unused mempools, ports sharing
> mempools, ports having separate mempools, numa based reconfig from vhost
> etc. All working fine, so LGTM.
> 
> Are you going to send a non-rfc version of this patch?
> 
> thanks,
> Kevin.
> 
> > Thanks
> > Ian
> >
> >> -Original Message-
> >> From: Stokes, Ian
> >> Sent: Thursday, February 8, 2018 11:39 AM
> >> To: d...@openvswitch.org
> >> Cc: Stokes, Ian ; antonio.fische...@gmail.com;
> >> Ilya Maximets ; Kevin Traynor
> >> ; Jan Scheurich 
> >> Subject: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
> >>
> >> This commit manually reverts the current per port mempool model to
> >> the previous shared mempool model for DPDK ports.
> >>
> >> OVS previously used a shared mempool model for ports with the same
> >> MTU configuration. This was replaced by a per port mempool model to
> >> address issues flagged by users such as:
> >>
> >> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-
> >> September/042560.html
> >>
> >> However the per port model has a number of issues including:
> >>
> >> 1. Requires an increase in memory resource requirements to support
> >> the same number of ports as the shared port model.
> >> 2. Incorrect algorithm for mbuf provisioning for each mempool.
> >>
> >> These are considered blocking factors for current deployments of OVS
> >> when upgrading to OVS 2.9 as a memory redimensioning will be
> >> required. This may not be possible for users.
> >>
> >> For clarity, the commits whose changes are removed include the
> >> following:
> >>
> >> netdev-dpdk: Create separate memory pool for each port: d555d9b
> >> netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> >> mempool names to reflect socket id: f06546a
> >> netdev-dpdk: skip init for existing mempools: 837c176
> >> netdev-dpdk: manage failure in mempool name creation: 65056fd
> >> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> >> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> >> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> >> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> >> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> >> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> >> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> >>
> >> Due to the number of commits and period of time they were introduced
> >> over, a simple revert was not possible. All code from the commits
> >> above is removed and the shared mempool code reintroduced as it was
> >> before its replacement.
> >>
> >> Code introduced by commit
> >>
> >> netdev-dpdk: Add debug appctl to get mempool information: be48173
> >>
> >> has been modified to work with the shared mempool model.
> >>
> >> Cc: antonio.fische...@gmail.com
> >> Cc: Ilya Maximets 
> >> Cc: Kevin Traynor 
> >> Cc: Jan Scheurich 
> >> Signed-off-by: Ian Stokes 
> >> ---
> >>  lib/netdev-dpdk.c | 246
> >> ++---
> >> -
> >>  1 file changed, 138 insertions(+), 108 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> 94fb163..6f3378b
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -91,13 +91,24 @@ static struct vlog_rate_limit rl =
> >> VLOG_RATE_LIMIT_INIT(5, 20);
> >>  #define NETDEV_DPDK_MBUF_ALIGN  1024
> >>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
> >>
> >> -/* Min number of packets in the mempool.  OVS tries to allocate a
> >> mempool with
> >> - * roughly estimated number of mbufs: if this fails (because the
> >> system doesn't
> >> - * have enough hugepages) we keep halving the number until the
> >> allocation
> >> - * succeeds or we reach MIN_NB_MBUF */
> >> +/* Max and min number of packets in the mempool.  OVS tries to
> >> +allocate a
> >> + * mempool with MAX_NB_MBUF: if this fails 

[ovs-dev] [RFC] ovn-northd: Store computed logical flow hash in SBDB.

[Jakub Sitnicki]

Each time ovn-northd processes the NBDB contents it has to compute the
hash of every logical flow stored in the SBDB in order to identify ones
that need adding to or deleting from SBDB (build_lflows()).

Avoid it by storing the logical flow hash together with its fields the
moment we first add it to SBDB. This saves us the hash computations
afterwards, every time ovn-northd processes the logical flows to find
candidates for removal/addition.

In a simple micro-benchmark that adds 15 logical switches with 100
logical ports each, perf tool shows a drop of 7% in CPU cycles
('cycles:ppp' event) spent in ovn_lflow_find() where we compute the hash
of each logical flow found in SBDB:

Children  Self  Command Shared Object   Symbol
before:   10.61% 0.17%  ovn-northd  ovn-northd  [.] ovn_lflow_find
after: 2.82% 0.19%  ovn-northd  ovn-northd  [.] ovn_lflow_find

Signed-off-by: Jakub Sitnicki 
---
 ovn/northd/ovn-northd.c | 9 +
 ovn/ovn-sb.ovsschema| 7 +--
 ovn/ovn-sb.xml  | 5 +
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4d95a3d9d..dcb9939d9 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2331,7 +2331,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
ovn_datapath *od,
 static struct ovn_lflow *
 ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
enum ovn_stage stage, uint16_t priority,
-   const char *match, const char *actions)
+   const char *match, const char *actions, uint32_t hash)
 {
 struct ovn_lflow target;
 ovn_lflow_init(, od, stage, priority,
@@ -2339,8 +2339,7 @@ ovn_lflow_find(struct hmap *lflows, struct ovn_datapath 
*od,
NULL, NULL);
 
 struct ovn_lflow *lflow;
-HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, ovn_lflow_hash(),
- lflows) {
+HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) {
 if (ovn_lflow_equal(lflow, )) {
 return lflow;
 }
@@ -6014,7 +6013,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
 = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT;
 struct ovn_lflow *lflow = ovn_lflow_find(
 , od, ovn_stage_build(dp_type, pipeline, sbflow->table_id),
-sbflow->priority, sbflow->match, sbflow->actions);
+sbflow->priority, sbflow->match, sbflow->actions, sbflow->hash);
 if (lflow) {
 ovn_lflow_destroy(, lflow);
 } else {
@@ -6034,6 +6033,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
 sbrec_logical_flow_set_priority(sbflow, lflow->priority);
 sbrec_logical_flow_set_match(sbflow, lflow->match);
 sbrec_logical_flow_set_actions(sbflow, lflow->actions);
+sbrec_logical_flow_set_hash(sbflow, lflow->hmap_node.hash);
 
 /* Trim the source locator lflow->where, which looks something like
  * "ovn/northd/ovn-northd.c:1234", down to just the part following the
@@ -6782,6 +6782,7 @@ main(int argc, char *argv[])
 add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_priority);
 add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_match);
 add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_actions);
+add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_hash);
 
 ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_multicast_group);
 add_column_noalert(ovnsb_idl_loop.idl,
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 2abcc6b70..b6a145431 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Southbound",
-"version": "1.15.0",
-"cksum": "70426956 13327",
+"version": "1.16.0",
+"cksum": "340822255 13518",
 "tables": {
 "SB_Global": {
 "columns": {
@@ -70,6 +70,9 @@
   "maxInteger": 65535}}},
 "match": {"type": "string"},
 "actions": {"type": "string"},
+"hash": {"type": {"key": {"type": "integer",
+  "minInteger": 0,
+  "maxInteger": 4294967295}}},
 "external_ids": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index f000b166c..073a4c11d 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1767,6 +1767,11 @@ tcp.flags = RST;
   
 
 
+
+  Cached value of a hash computed over selected fields of this logical flow
+  by ovn-northd.
+
+
 
   Human-readable name for this flow's stage in the pipeline.
 
-- 
2.14.3

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

Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.

[Stokes, Ian]

Hi all,

Just pinging for feedback for this patch. If we are to include this for branch 
2.9 as a bug fix I think we need feedback quite soon.

Have people had time to validate and review?

Thanks
Ian

> -Original Message-
> From: Stokes, Ian
> Sent: Thursday, February 8, 2018 11:39 AM
> To: d...@openvswitch.org
> Cc: Stokes, Ian ; antonio.fische...@gmail.com; Ilya
> Maximets ; Kevin Traynor ;
> Jan Scheurich 
> Subject: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
> 
> This commit manually reverts the current per port mempool model to the
> previous shared mempool model for DPDK ports.
> 
> OVS previously used a shared mempool model for ports with the same MTU
> configuration. This was replaced by a per port mempool model to address
> issues flagged by users such as:
> 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-
> September/042560.html
> 
> However the per port model has a number of issues including:
> 
> 1. Requires an increase in memory resource requirements to support the
> same number of ports as the shared port model.
> 2. Incorrect algorithm for mbuf provisioning for each mempool.
> 
> These are considered blocking factors for current deployments of OVS when
> upgrading to OVS 2.9 as a memory redimensioning will be required. This may
> not be possible for users.
> 
> For clarity, the commits whose changes are removed include the
> following:
> 
> netdev-dpdk: Create separate memory pool for each port: d555d9b
> netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> mempool names to reflect socket id: f06546a
> netdev-dpdk: skip init for existing mempools: 837c176
> netdev-dpdk: manage failure in mempool name creation: 65056fd
> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> 
> Due to the number of commits and period of time they were introduced over,
> a simple revert was not possible. All code from the commits above is
> removed and the shared mempool code reintroduced as it was before its
> replacement.
> 
> Code introduced by commit
> 
> netdev-dpdk: Add debug appctl to get mempool information: be48173
> 
> has been modified to work with the shared mempool model.
> 
> Cc: antonio.fische...@gmail.com
> Cc: Ilya Maximets 
> Cc: Kevin Traynor 
> Cc: Jan Scheurich 
> Signed-off-by: Ian Stokes 
> ---
>  lib/netdev-dpdk.c | 246 ++---
> -
>  1 file changed, 138 insertions(+), 108 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 94fb163..6f3378b
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -91,13 +91,24 @@ static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 20);
>  #define NETDEV_DPDK_MBUF_ALIGN  1024
>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
> 
> -/* Min number of packets in the mempool.  OVS tries to allocate a mempool
> with
> - * roughly estimated number of mbufs: if this fails (because the system
> doesn't
> - * have enough hugepages) we keep halving the number until the allocation
> - * succeeds or we reach MIN_NB_MBUF */
> +/* Max and min number of packets in the mempool.  OVS tries to allocate
> +a
> + * mempool with MAX_NB_MBUF: if this fails (because the system doesn't
> +have
> + * enough hugepages) we keep halving the number until the allocation
> +succeeds
> + * or we reach MIN_NB_MBUF */
> +
> +#define MAX_NB_MBUF  (4096 * 64)
>  #define MIN_NB_MBUF  (4096 * 4)
>  #define MP_CACHE_SZ  RTE_MEMPOOL_CACHE_MAX_SIZE
> 
> +/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
> +BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF /
> MIN_NB_MBUF)
> +  == 0);
> +
> +/* The smallest possible NB_MBUF that we're going to try should be a
> +multiple
> + * of MP_CACHE_SZ. This is advised by DPDK documentation. */
> +BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF /
> MIN_NB_MBUF))
> +  % MP_CACHE_SZ == 0);
> +
>  /*
>   * DPDK XSTATS Counter names definition
>   */
> @@ -295,6 +306,19 @@ static struct ovs_list dpdk_list
> OVS_GUARDED_BY(dpdk_mutex)  static struct ovs_mutex dpdk_mp_mutex
> OVS_ACQ_AFTER(dpdk_mutex)
>  = OVS_MUTEX_INITIALIZER;
> 
> +static struct ovs_list dpdk_mp_list OVS_GUARDED_BY(dpdk_mp_mutex)
> += OVS_LIST_INITIALIZER(_mp_list);
> +
> +
> +struct dpdk_mp {
> + struct rte_mempool *mp;
> + int mtu;
> + int socket_id;
> + int refcount;
> + struct ovs_list list_node 

Re: [ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared mempools.

[Stokes, Ian]

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Tuesday, February 13, 2018 11:36 AM
> To: Stokes, Ian ; d...@openvswitch.org
> Cc: Antonio Fischetti ; Ilya Maximets
> ; Jan Scheurich 
> Subject: Re: [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared
> mempools.
> 
> On 02/13/2018 10:59 AM, Ian Stokes wrote:
> > This commit manually reverts the current per port mempool model to the
> > previous shared mempool model for DPDK ports.
> >
> > OVS previously used a shared mempool model for ports with the same MTU
> > configuration. This was replaced by a per port mempool model to
> > address issues flagged by users such as:
> >
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/0425
> > 60.html
> >
> > However the per port model has a number of issues including:
> >
> > 1. Requires an increase in memory resource requirements to support the
> > same number of ports as the shared port model.
> > 2. Incorrect algorithm for mbuf provisioning for each mempool.
> >
> > These are considered blocking factors for current deployments of OVS
> > when upgrading to OVS 2.9 as a  user may have to redimension memory
> > for the same deployment configuration. This may not be possible for
> users.
> >
> > For clarity, the commits whose changes are removed include the
> > following:
> >
> > netdev-dpdk: Create separate memory pool for each port: d555d9b
> > netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> > mempool names to reflect socket id: f06546a
> > netdev-dpdk: skip init for existing mempools: 837c176
> > netdev-dpdk: manage failure in mempool name creation: 65056fd
> > netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> > netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> > netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> > netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> > netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> > netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> > netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> >
> > Due to the number of commits and period of time they were introduced
> > over, a simple revert was not possible. All code from the commits
> > above is removed and the shared mempool code reintroduced as it was
> > before its replacement.
> >
> > Code introduced by commit
> >
> > netdev-dpdk: Add debug appctl to get mempool information: be48173
> >
> > has been modified to work with the shared mempool model.
> >
> 
> There is a couple of small changes for coding stds to this version from
> the rfc, but one is not correct (see below). Apart from that...

Thanks for flagging this.

> 
> Acked-by: Kevin Traynor 
> Tested-by: Kevin Traynor 
> 

...

> >  ovs_mutex_lock(_mp_mutex);
> > -VLOG_DBG("Releasing \"%s\" mempool", mp->name);
> > -rte_mempool_free(mp);
> > +ovs_assert(dmp->refcount);
> > +
> > +if (! --dmp->refcount) {
> 
> The space should not be added between ! and --
> 
> http://docs.openvswitch.org/en/latest/internals/contributing/coding-
> style/#expressions

Good catch, I can fix this before committing if there are no other comments.

Thanks
Ian
> 
> > +ovs_list_remove(>list_node);
> > +rte_mempool_free(dmp->mp);
> > +rte_free(dmp);
> > + }
> >  ovs_mutex_unlock(_mp_mutex);  }
> >
> > -/* Tries to allocate a new mempool - or re-use an existing one where
> > - * appropriate - on requested_socket_id with a size determined by
> > - * requested_mtu and requested Rx/Tx queues.
> > - * On success - or when re-using an existing mempool - the new
> > configuration
> > - * will be applied.
> > +/* Tries to allocate new mempool on requested_socket_id with
> > + * mbuf size corresponding to requested_mtu.
> > + * On success new configuration will be applied.
> >   * On error, device will be left unchanged. */  static int
> > netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> >  OVS_REQUIRES(dev->mutex)
> >  {
> >  uint32_t buf_size = dpdk_buf_size(dev->requested_mtu);
> > -struct rte_mempool *mp;
> > -int ret = 0;
> > +struct dpdk_mp *mp;
> >
> > -mp = dpdk_mp_create(dev, FRAME_LEN_TO_MTU(buf_size));
> > +mp = dpdk_mp_get(dev->requested_socket_id,
> > + FRAME_LEN_TO_MTU(buf_size));
> >  if (!mp) {
> >  VLOG_ERR("Failed to create memory pool for netdev "
> >   "%s, with MTU %d on socket %d: %s\n",
> >   dev->up.name, dev->requested_mtu, dev-
> >requested_socket_id,
> > - rte_strerror(rte_errno));
> > -ret = rte_errno;
> > +rte_strerror(rte_errno));
> > +return rte_errno;
> >  } else {
> > -/* If a new MTU was requested and its rounded value equals the
> one
> > - * that is currently used, then the existing mempool is
> returned. */
> > -if 

Re: [ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared mempools.

[Kevin Traynor]

On 02/13/2018 10:59 AM, Ian Stokes wrote:
> This commit manually reverts the current per port mempool model to the
> previous shared mempool model for DPDK ports.
> 
> OVS previously used a shared mempool model for ports with the same MTU
> configuration. This was replaced by a per port mempool model to address
> issues flagged by users such as:
> 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/042560.html
> 
> However the per port model has a number of issues including:
> 
> 1. Requires an increase in memory resource requirements to support the same
> number of ports as the shared port model.
> 2. Incorrect algorithm for mbuf provisioning for each mempool.
> 
> These are considered blocking factors for current deployments of OVS when
> upgrading to OVS 2.9 as a  user may have to redimension memory for the same
> deployment configuration. This may not be possible for users.
> 
> For clarity, the commits whose changes are removed include the
> following:
> 
> netdev-dpdk: Create separate memory pool for each port: d555d9b
> netdev-dpdk: fix management of pre-existing mempools: b6b26021d
> Fix mempool names to reflect socket id: f06546a
> netdev-dpdk: skip init for existing mempools: 837c176
> netdev-dpdk: manage failure in mempool name creation: 65056fd
> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> 
> Due to the number of commits and period of time they were introduced
> over, a simple revert was not possible. All code from the commits above
> is removed and the shared mempool code reintroduced as it was before its
> replacement.
> 
> Code introduced by commit
> 
> netdev-dpdk: Add debug appctl to get mempool information: be48173
> 
> has been modified to work with the shared mempool model.
> 

There is a couple of small changes for coding stds to this version from
the rfc, but one is not correct (see below). Apart from that...

Acked-by: Kevin Traynor 
Tested-by: Kevin Traynor 

> Cc: Antonio Fischetti 
> Cc: Ilya Maximets 
> Cc: Kevin Traynor 
> Cc: Jan Scheurich 
> Signed-off-by: Ian Stokes 
> ---
>  lib/netdev-dpdk.c | 246 
> ++
>  1 file changed, 138 insertions(+), 108 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 94fb163..6f3378b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -91,13 +91,24 @@ static struct vlog_rate_limit rl = 
> VLOG_RATE_LIMIT_INIT(5, 20);
>  #define NETDEV_DPDK_MBUF_ALIGN  1024
>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
>  
> -/* Min number of packets in the mempool.  OVS tries to allocate a mempool 
> with
> - * roughly estimated number of mbufs: if this fails (because the system 
> doesn't
> - * have enough hugepages) we keep halving the number until the allocation
> - * succeeds or we reach MIN_NB_MBUF */
> +/* Max and min number of packets in the mempool.  OVS tries to allocate a
> + * mempool with MAX_NB_MBUF: if this fails (because the system doesn't have
> + * enough hugepages) we keep halving the number until the allocation succeeds
> + * or we reach MIN_NB_MBUF */
> +
> +#define MAX_NB_MBUF  (4096 * 64)
>  #define MIN_NB_MBUF  (4096 * 4)
>  #define MP_CACHE_SZ  RTE_MEMPOOL_CACHE_MAX_SIZE
>  
> +/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
> +BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF)
> +  == 0);
> +
> +/* The smallest possible NB_MBUF that we're going to try should be a multiple
> + * of MP_CACHE_SZ. This is advised by DPDK documentation. */
> +BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF))
> +  % MP_CACHE_SZ == 0);
> +
>  /*
>   * DPDK XSTATS Counter names definition
>   */
> @@ -295,6 +306,19 @@ static struct ovs_list dpdk_list 
> OVS_GUARDED_BY(dpdk_mutex)
>  static struct ovs_mutex dpdk_mp_mutex OVS_ACQ_AFTER(dpdk_mutex)
>  = OVS_MUTEX_INITIALIZER;
>  
> +static struct ovs_list dpdk_mp_list OVS_GUARDED_BY(dpdk_mp_mutex)
> += OVS_LIST_INITIALIZER(_mp_list);
> +
> +
> +struct dpdk_mp {
> + struct rte_mempool *mp;
> + int mtu;
> + int socket_id;
> + int refcount;
> + struct ovs_list list_node OVS_GUARDED_BY(dpdk_mp_mutex);
> + };
> +
> +
>  /* There should be one 'struct dpdk_tx_queue' created for
>   * each cpu core. */
>  struct dpdk_tx_queue {
> @@ -371,7 +395,7 @@ struct netdev_dpdk {
>  
>  PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline1,
>  struct 

Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.

[Stokes, Ian]

> -Original Message-
> From: Venkatesan Pradeep [mailto:venkatesan.prad...@ericsson.com]
> Sent: Tuesday, February 13, 2018 11:01 AM
> To: Kevin Traynor ; Stokes, Ian
> ; d...@openvswitch.org
> Cc: Ilya Maximets ; antonio.fische...@gmail.com
> Subject: RE: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared
> mempools.
> 
> Hi,
> 
> I've also reviewed the code and done tests to exercise the mempool code.
> The behavior seems to be same as with previous versions that had the
> shared mempool code.


Thanks for validating Pradeep, I've submitted an non RFC patch now for branch 
2.9.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344398.html

If you want to respond there and add your review/tested by tags that would be 
great.

Thanks
Ian
> 
> Thanks,
> 
> Pradeep
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Kevin Traynor
> > Sent: Tuesday, February 13, 2018 4:05 PM
> > To: Stokes, Ian ; d...@openvswitch.org
> > Cc: Ilya Maximets ;
> > antonio.fische...@gmail.com
> > Subject: Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared
> > mempools.
> >
> > On 02/13/2018 09:07 AM, Stokes, Ian wrote:
> > > Hi all,
> > >
> > > Just pinging for feedback for this patch. If we are to include this
> > > for branch
> > 2.9 as a bug fix I think we need feedback quite soon.
> > >
> > > Have people had time to validate and review?
> > >
> >
> > Hi Ian,
> >
> > As per the other thread I have reviewed and tested multiple ports with
> > MTU reconfigs - creating new mempools, freeing unused mempools, ports
> > sharing mempools, ports having separate mempools, numa based reconfig
> > from vhost etc. All working fine, so LGTM.
> >
> > Are you going to send a non-rfc version of this patch?
> >
> > thanks,
> > Kevin.
> >
> > > Thanks
> > > Ian
> > >
> > >> -Original Message-
> > >> From: Stokes, Ian
> > >> Sent: Thursday, February 8, 2018 11:39 AM
> > >> To: d...@openvswitch.org
> > >> Cc: Stokes, Ian ;
> > >> antonio.fische...@gmail.com; Ilya Maximets
> > >> ; Kevin Traynor ; Jan
> > >> Scheurich 
> > >> Subject: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
> > >>
> > >> This commit manually reverts the current per port mempool model to
> > >> the previous shared mempool model for DPDK ports.
> > >>
> > >> OVS previously used a shared mempool model for ports with the same
> > >> MTU configuration. This was replaced by a per port mempool model to
> > >> address issues flagged by users such as:
> > >>
> > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-
> > >> September/042560.html
> > >>
> > >> However the per port model has a number of issues including:
> > >>
> > >> 1. Requires an increase in memory resource requirements to support
> > >> the same number of ports as the shared port model.
> > >> 2. Incorrect algorithm for mbuf provisioning for each mempool.
> > >>
> > >> These are considered blocking factors for current deployments of
> > >> OVS when upgrading to OVS 2.9 as a memory redimensioning will be
> > >> required. This may not be possible for users.
> > >>
> > >> For clarity, the commits whose changes are removed include the
> > >> following:
> > >>
> > >> netdev-dpdk: Create separate memory pool for each port: d555d9b
> > >> netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> > >> mempool names to reflect socket id: f06546a
> > >> netdev-dpdk: skip init for existing mempools: 837c176
> > >> netdev-dpdk: manage failure in mempool name creation: 65056fd
> > >> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> > >> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> > >> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> > >> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> > >> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> > >> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> > >> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> > >>
> > >> Due to the number of commits and period of time they were
> > >> introduced over, a simple revert was not possible. All code from
> > >> the commits above is removed and the shared mempool code
> > >> reintroduced as it was before its replacement.
> > >>
> > >> Code introduced by commit
> > >>
> > >> netdev-dpdk: Add debug appctl to get mempool information: be48173
> > >>
> > >> has been modified to work with the shared mempool model.
> > >>
> > >> Cc: antonio.fische...@gmail.com
> > >> Cc: Ilya Maximets 
> > >> Cc: Kevin Traynor 
> > >> Cc: Jan Scheurich 
> > >> Signed-off-by: Ian Stokes 
> > >> ---
> > >>  lib/netdev-dpdk.c | 246
> > >> 

[ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared mempools.

[Ian Stokes]

This commit manually reverts the current per port mempool model to the
previous shared mempool model for DPDK ports.

OVS previously used a shared mempool model for ports with the same MTU
configuration. This was replaced by a per port mempool model to address
issues flagged by users such as:

https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/042560.html

However the per port model has a number of issues including:

1. Requires an increase in memory resource requirements to support the same
number of ports as the shared port model.
2. Incorrect algorithm for mbuf provisioning for each mempool.

These are considered blocking factors for current deployments of OVS when
upgrading to OVS 2.9 as a  user may have to redimension memory for the same
deployment configuration. This may not be possible for users.

For clarity, the commits whose changes are removed include the
following:

netdev-dpdk: Create separate memory pool for each port: d555d9b
netdev-dpdk: fix management of pre-existing mempools: b6b26021d
Fix mempool names to reflect socket id: f06546a
netdev-dpdk: skip init for existing mempools: 837c176
netdev-dpdk: manage failure in mempool name creation: 65056fd
netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
netdev-dpdk: Fix mempool creation with large MTU: af5b0da

Due to the number of commits and period of time they were introduced
over, a simple revert was not possible. All code from the commits above
is removed and the shared mempool code reintroduced as it was before its
replacement.

Code introduced by commit

netdev-dpdk: Add debug appctl to get mempool information: be48173

has been modified to work with the shared mempool model.

Cc: Antonio Fischetti 
Cc: Ilya Maximets 
Cc: Kevin Traynor 
Cc: Jan Scheurich 
Signed-off-by: Ian Stokes 
---
 lib/netdev-dpdk.c | 246 ++
 1 file changed, 138 insertions(+), 108 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 94fb163..6f3378b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -91,13 +91,24 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 
20);
 #define NETDEV_DPDK_MBUF_ALIGN  1024
 #define NETDEV_DPDK_MAX_PKT_LEN 9728
 
-/* Min number of packets in the mempool.  OVS tries to allocate a mempool with
- * roughly estimated number of mbufs: if this fails (because the system doesn't
- * have enough hugepages) we keep halving the number until the allocation
- * succeeds or we reach MIN_NB_MBUF */
+/* Max and min number of packets in the mempool.  OVS tries to allocate a
+ * mempool with MAX_NB_MBUF: if this fails (because the system doesn't have
+ * enough hugepages) we keep halving the number until the allocation succeeds
+ * or we reach MIN_NB_MBUF */
+
+#define MAX_NB_MBUF  (4096 * 64)
 #define MIN_NB_MBUF  (4096 * 4)
 #define MP_CACHE_SZ  RTE_MEMPOOL_CACHE_MAX_SIZE
 
+/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
+BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF)
+  == 0);
+
+/* The smallest possible NB_MBUF that we're going to try should be a multiple
+ * of MP_CACHE_SZ. This is advised by DPDK documentation. */
+BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF / MIN_NB_MBUF))
+  % MP_CACHE_SZ == 0);
+
 /*
  * DPDK XSTATS Counter names definition
  */
@@ -295,6 +306,19 @@ static struct ovs_list dpdk_list OVS_GUARDED_BY(dpdk_mutex)
 static struct ovs_mutex dpdk_mp_mutex OVS_ACQ_AFTER(dpdk_mutex)
 = OVS_MUTEX_INITIALIZER;
 
+static struct ovs_list dpdk_mp_list OVS_GUARDED_BY(dpdk_mp_mutex)
+= OVS_LIST_INITIALIZER(_mp_list);
+
+
+struct dpdk_mp {
+ struct rte_mempool *mp;
+ int mtu;
+ int socket_id;
+ int refcount;
+ struct ovs_list list_node OVS_GUARDED_BY(dpdk_mp_mutex);
+ };
+
+
 /* There should be one 'struct dpdk_tx_queue' created for
  * each cpu core. */
 struct dpdk_tx_queue {
@@ -371,7 +395,7 @@ struct netdev_dpdk {
 
 PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline1,
 struct ovs_mutex mutex OVS_ACQ_AFTER(dpdk_mutex);
-struct rte_mempool *mp;
+struct dpdk_mp *dpdk_mp;
 
 /* virtio identifier for vhost devices */
 ovsrcu_index vid;
@@ -510,133 +534,132 @@ ovs_rte_pktmbuf_init(struct rte_mempool *mp OVS_UNUSED,
 dp_packet_init_dpdk((struct dp_packet *) pkt, pkt->buf_len);
 }
 
-/* Returns a valid pointer when either of the following is true:
- *  - a new mempool was just created;
- *  - a matching mempool already exists. */
-static struct 

Re: [ovs-dev] [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.

[Kevin Traynor]

On 02/13/2018 09:07 AM, Stokes, Ian wrote:
> Hi all,
> 
> Just pinging for feedback for this patch. If we are to include this for 
> branch 2.9 as a bug fix I think we need feedback quite soon.
> 
> Have people had time to validate and review?
> 

Hi Ian,

As per the other thread I have reviewed and tested multiple ports with
MTU reconfigs - creating new mempools, freeing unused mempools, ports
sharing mempools, ports having separate mempools, numa based reconfig
from vhost etc. All working fine, so LGTM.

Are you going to send a non-rfc version of this patch?

thanks,
Kevin.

> Thanks
> Ian
> 
>> -Original Message-
>> From: Stokes, Ian
>> Sent: Thursday, February 8, 2018 11:39 AM
>> To: d...@openvswitch.org
>> Cc: Stokes, Ian ; antonio.fische...@gmail.com; Ilya
>> Maximets ; Kevin Traynor ;
>> Jan Scheurich 
>> Subject: [RFC Patch v1] netdev-dpdk: Reintroduce shared mempools.
>>
>> This commit manually reverts the current per port mempool model to the
>> previous shared mempool model for DPDK ports.
>>
>> OVS previously used a shared mempool model for ports with the same MTU
>> configuration. This was replaced by a per port mempool model to address
>> issues flagged by users such as:
>>
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2016-
>> September/042560.html
>>
>> However the per port model has a number of issues including:
>>
>> 1. Requires an increase in memory resource requirements to support the
>> same number of ports as the shared port model.
>> 2. Incorrect algorithm for mbuf provisioning for each mempool.
>>
>> These are considered blocking factors for current deployments of OVS when
>> upgrading to OVS 2.9 as a memory redimensioning will be required. This may
>> not be possible for users.
>>
>> For clarity, the commits whose changes are removed include the
>> following:
>>
>> netdev-dpdk: Create separate memory pool for each port: d555d9b
>> netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
>> mempool names to reflect socket id: f06546a
>> netdev-dpdk: skip init for existing mempools: 837c176
>> netdev-dpdk: manage failure in mempool name creation: 65056fd
>> netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
>> netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
>> netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
>> netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
>> netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
>> netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
>> netdev-dpdk: Fix mempool creation with large MTU: af5b0da
>>
>> Due to the number of commits and period of time they were introduced over,
>> a simple revert was not possible. All code from the commits above is
>> removed and the shared mempool code reintroduced as it was before its
>> replacement.
>>
>> Code introduced by commit
>>
>> netdev-dpdk: Add debug appctl to get mempool information: be48173
>>
>> has been modified to work with the shared mempool model.
>>
>> Cc: antonio.fische...@gmail.com
>> Cc: Ilya Maximets 
>> Cc: Kevin Traynor 
>> Cc: Jan Scheurich 
>> Signed-off-by: Ian Stokes 
>> ---
>>  lib/netdev-dpdk.c | 246 ++---
>> -
>>  1 file changed, 138 insertions(+), 108 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 94fb163..6f3378b
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -91,13 +91,24 @@ static struct vlog_rate_limit rl =
>> VLOG_RATE_LIMIT_INIT(5, 20);
>>  #define NETDEV_DPDK_MBUF_ALIGN  1024
>>  #define NETDEV_DPDK_MAX_PKT_LEN 9728
>>
>> -/* Min number of packets in the mempool.  OVS tries to allocate a mempool
>> with
>> - * roughly estimated number of mbufs: if this fails (because the system
>> doesn't
>> - * have enough hugepages) we keep halving the number until the allocation
>> - * succeeds or we reach MIN_NB_MBUF */
>> +/* Max and min number of packets in the mempool.  OVS tries to allocate
>> +a
>> + * mempool with MAX_NB_MBUF: if this fails (because the system doesn't
>> +have
>> + * enough hugepages) we keep halving the number until the allocation
>> +succeeds
>> + * or we reach MIN_NB_MBUF */
>> +
>> +#define MAX_NB_MBUF  (4096 * 64)
>>  #define MIN_NB_MBUF  (4096 * 4)
>>  #define MP_CACHE_SZ  RTE_MEMPOOL_CACHE_MAX_SIZE
>>
>> +/* MAX_NB_MBUF can be divided by 2 many times, until MIN_NB_MBUF */
>> +BUILD_ASSERT_DECL(MAX_NB_MBUF % ROUND_DOWN_POW2(MAX_NB_MBUF /
>> MIN_NB_MBUF)
>> +  == 0);
>> +
>> +/* The smallest possible NB_MBUF that we're going to try should be a
>> +multiple
>> + * of MP_CACHE_SZ. This is advised by DPDK documentation. */
>> +BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF /
>> MIN_NB_MBUF))
>> +  % MP_CACHE_SZ == 0);
>> +
>>  /*
>>   * DPDK XSTATS Counter names 

Re: [ovs-dev] DPDK: How to send packets from one OVS bridge to another OVS bridge while running in the same user space ?

[Jan Scheurich]

Hi Kapil,

This is what patch ports are for. They are only traversed for the first packet 
of a flow in the slow path. The resulting datapath flow entry collapses the 
processing in both bridges into a single megaflow. So there is no performance 
overhead compared to having a single bridge.

Regards, Jan

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Kapil A
> Sent: Tuesday, 13 February, 2018 16:44
> To: disc...@openvswitch.org; d...@openvswitch.org
> Subject: [ovs-dev] DPDK: How to send packets from one OVS bridge to another 
> OVS bridge while running in the same user space ?
> 
> Hello,
> 
> In my setup, i have two DPDK ports, where dpdk0 is part of br0 and and
> dpdk1 is part of br1. How can i send packets from br0 to br1 in an
> efficient way within userspace with good performance?
> I came across, veth pair as one option, but i couldn't find if it will
> provide good performance in userspace while running in dpdk mode ?
> 
> My query might sound odd, but i have a use case where i need to send the
> packets across bridges within the same userspace, so appreciate some help
> on this.
> 
> Regards
> Kapil
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] ovn-northd: Store computed logical flow hash in SBDB.

[Mark Michelson]

Nice!

Acked-by: Mark Michelson 

On 02/13/2018 07:57 AM, Jakub Sitnicki wrote:

Each time ovn-northd processes the NBDB contents it has to compute the
hash of every logical flow stored in the SBDB in order to identify ones
that need adding to or deleting from SBDB (build_lflows()).

Avoid it by storing the logical flow hash together with its fields the
moment we first add it to SBDB. This saves us the hash computations
afterwards, every time ovn-northd processes the logical flows to find
candidates for removal/addition.

In a simple micro-benchmark that adds 15 logical switches with 100
logical ports each, perf tool shows a drop of 7% in CPU cycles
('cycles:ppp' event) spent in ovn_lflow_find() where we compute the hash
of each logical flow found in SBDB:

 Children  Self  Command Shared Object   Symbol
before:   10.61% 0.17%  ovn-northd  ovn-northd  [.] ovn_lflow_find
after: 2.82% 0.19%  ovn-northd  ovn-northd  [.] ovn_lflow_find

Signed-off-by: Jakub Sitnicki 
---
  ovn/northd/ovn-northd.c | 9 +
  ovn/ovn-sb.ovsschema| 7 +--
  ovn/ovn-sb.xml  | 5 +
  3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4d95a3d9d..dcb9939d9 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2331,7 +2331,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
ovn_datapath *od,
  static struct ovn_lflow *
  ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
 enum ovn_stage stage, uint16_t priority,
-   const char *match, const char *actions)
+   const char *match, const char *actions, uint32_t hash)
  {
  struct ovn_lflow target;
  ovn_lflow_init(, od, stage, priority,
@@ -2339,8 +2339,7 @@ ovn_lflow_find(struct hmap *lflows, struct ovn_datapath 
*od,
 NULL, NULL);
  
  struct ovn_lflow *lflow;

-HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, ovn_lflow_hash(),
- lflows) {
+HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, hash, lflows) {
  if (ovn_lflow_equal(lflow, )) {
  return lflow;
  }
@@ -6014,7 +6013,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
  = !strcmp(sbflow->pipeline, "ingress") ? P_IN : P_OUT;
  struct ovn_lflow *lflow = ovn_lflow_find(
  , od, ovn_stage_build(dp_type, pipeline, sbflow->table_id),
-sbflow->priority, sbflow->match, sbflow->actions);
+sbflow->priority, sbflow->match, sbflow->actions, sbflow->hash);
  if (lflow) {
  ovn_lflow_destroy(, lflow);
  } else {
@@ -6034,6 +6033,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
  sbrec_logical_flow_set_priority(sbflow, lflow->priority);
  sbrec_logical_flow_set_match(sbflow, lflow->match);
  sbrec_logical_flow_set_actions(sbflow, lflow->actions);
+sbrec_logical_flow_set_hash(sbflow, lflow->hmap_node.hash);
  
  /* Trim the source locator lflow->where, which looks something like

   * "ovn/northd/ovn-northd.c:1234", down to just the part following the
@@ -6782,6 +6782,7 @@ main(int argc, char *argv[])
  add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_priority);
  add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_match);
  add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_actions);
+add_column_noalert(ovnsb_idl_loop.idl, _logical_flow_col_hash);
  
  ovsdb_idl_add_table(ovnsb_idl_loop.idl, _table_multicast_group);

  add_column_noalert(ovnsb_idl_loop.idl,
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 2abcc6b70..b6a145431 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
  {
  "name": "OVN_Southbound",
-"version": "1.15.0",
-"cksum": "70426956 13327",
+"version": "1.16.0",
+"cksum": "340822255 13518",
  "tables": {
  "SB_Global": {
  "columns": {
@@ -70,6 +70,9 @@
"maxInteger": 65535}}},
  "match": {"type": "string"},
  "actions": {"type": "string"},
+"hash": {"type": {"key": {"type": "integer",
+  "minInteger": 0,
+  "maxInteger": 4294967295}}},
  "external_ids": {
  "type": {"key": "string", "value": "string",
   "min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index f000b166c..073a4c11d 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1767,6 +1767,11 @@ tcp.flags = RST;

  
  
+

+  Cached value of a hash computed over selected fields of this logical flow
+  by ovn-northd.
+
+
  
Human-readable name for this 

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.7

[Stokes, Ian]

Hi Ben,

The following changes since commit eee9eec34c6131988af1ea3816fadd13af026c9d:

  netdev-dpdk: Fix requested MTU size validation. (2018-01-26 20:43:18 +)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_7

for you to fetch changes up to bfe96bb08e748f140ea6134d4af3b1f1e1cddda7:

  dpdk: Use DPDK 16.11.4 stable release. (2018-02-06 12:10:40 +)


Ian Stokes (1):
  dpdk: Use DPDK 16.11.4 stable release.

 .travis/linux-build.sh   | 2 +-
 Documentation/faq/releases.rst   | 2 +-
 Documentation/intro/install/dpdk.rst | 6 +++---
 Documentation/topics/dpdk/vhost-user.rst | 8 
 4 files changed, 9 insertions(+), 9 deletions(-)

Thanks
Ian

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

Re: [ovs-dev] [PATCH v8 0/3] dpif-netdev: Detailed PMD performance metrics and supervision

[Jan Scheurich]

Gentle reminder to review this series which unfortunately missed the 2.9 
deadline.

I checked and the patches still apply on today's master.
So far I have received one comment from Billy
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343808.html

Thanks, Jan

> -Original Message-
> From: Jan Scheurich
> Sent: Friday, 26 January, 2018 13:20
> To: d...@openvswitch.org
> Cc: ktray...@redhat.com; ian.sto...@intel.com; i.maxim...@samsung.com; 
> billy.o.mah...@intel.com; Jan Scheurich
> 
> Subject: [PATCH v8 0/3] dpif-netdev: Detailed PMD performance metrics and 
> supervision
> 
> The run-time performance of PMDs is often difficult to understand and
> trouble-shoot. The existing PMD statistics counters only provide a coarse
> grained average picture. At packet rates of several Mpps sporadic drops of
> packet bursts happen at sub-millisecond time scales and are impossible to
> capture and analyze with existing tools.
> 
> This patch collects a large number of important PMD performance metrics
> per PMD iteration, maintaining histograms and circular histories for
> iteration metrics and millisecond averages. To capture sporadic drop
> events, the patch set can be configured to monitor iterations for suspicious
> metrics and to log the neighborhood of such iterations for off-line analysis.
> 
> The extra cost for the performance metric collection and the supervision has
> been measured to be in the order of 1% compared to the base commit in a PVP
> setup with L3 pipeline over VXLAN tunnels. For that reason the metrics
> collection is disabled by default and can be enabled at run-time through
> configuration.
> 
> v7 -> v8:
> * Rebased on to master (commit 4e99b70df)
> * Implemented comments from Ilya Maximets and Billy O'Mahony.
> * Replaced netdev_rxq_length() introduced in v7 by optional out
>   parameter for the remaining rx queue len in netdev_rxq_recv().
> * Fixed thread synchronization issues in clearing PMD stats:
>   - Use mutex to control whether to clear from main thread directly
> or in PMD at start of next iteration.
>   - Use mutex to prevent concurrent clearing and printing of metrics.
> * Added tx packet and batch stats to pmd-perf-show output.
> * Delay warning for suspicious iteration to the iteration in which
>   we also log the neighborhood to not pollute the logged iteration
>   stats with logging costs.
> * Corrected the exact number of iterations logged before and after a
>   supicious iteration.
> * Introduced options -e and -ne in pmd-perf-log-set to control whether
>   to *extend* the range of logged iterations when additional supicious
>   iterations are detected before the scheduled end of logging interval
>   is reached.
> * Exclude logging cycles from the iteration stats to avoid confusing
>   ghost peaks.
> * Performance impact compared to master less than 1% even with
>   supervision enabled.
> 
> v5 -> v7:
> * Rebased on to dpdk_merge (commit e68)
>   - New base contains earlier refactoring parts of series.
> * Implemented comments from Ilya Maximets and Billy O'Mahony.
> * Replaced piggybacking qlen on dp_packet_batch with a new netdev API
>   netdev_rxq_length().
> * Thread-safe clearing of pmd counters in pmd_perf_start_iteration().
> * Fixed bug in reporting datapath stats.
> * Work-around a bug in DPDK rte_vhost_rx_queue_count() which sometimes
>   returns bogus in the upper 16 bits of the uint32_t return value.
> 
> v4 -> v5:
> * Rebased to master (commit e9de6c0)
> * Implemented comments from Aaron Conole and Darrel Ball
> 
> v3 -> v4:
> * Rebased to master (commit 4d0a31b)
>   - Reverting changes to struct dp_netdev_pmd_thread.
> * Make metrics collection configurable.
> * Several bugfixes.
> 
> v2 -> v3:
> * Rebased to OVS master (commit 3728b3b).
> * Non-trivial adaptation to struct dp_netdev_pmd_thread.
>   - refactored in commit a807c157 (Bhanu).
> * No other changes compared to v2.
> 
> v1 -> v2:
> * Rebased to OVS master (commit 7468ec788).
> * No other changes compared to v1.
> 
> 
> Jan Scheurich (3):
>   netdev: Add optional qfill output parameter to rxq_recv()
>   dpif-netdev: Detailed performance stats for PMDs
>   dpif-netdev: Detection and logging of suspicious PMD iterations
> 
>  NEWS|   5 +
>  lib/automake.mk |   1 +
>  lib/dpif-netdev-perf.c  | 551 
> +++-
>  lib/dpif-netdev-perf.h  | 300 +++-
>  lib/dpif-netdev-unixctl.man | 216 +
>  lib/dpif-netdev.c   | 187 ++-
>  lib/netdev-bsd.c|   8 +-
>  lib/netdev-dpdk.c   |  25 +-
>  lib/netdev-dummy.c  |   8 +-
>  lib/netdev-linux.c  |   7 +-
>  lib/netdev-provider.h   |   7 +-
>  lib/netdev.c|   5 +-
>  lib/netdev.h|   3 +-
>  manpages.mk |   2 +
>  vswitchd/ovs-vswitchd.8.in  |  27 +--
>  vswitchd/vswitch.xml|  12 +
>  16 

[ovs-dev] [PATCH 2/3] OVN: add icmp4{} action support

[Lorenzo Bianconi]

icmp4 action is used to replace the IPv4 packet been processed with
an ICMPv4 packet initialized based on incoming IPv4 one.
Ethernet and IPv4 fields not listed are not changed:
- ip.proto = 1 (ICMPv4)
- ip.frag = 0 (not a fragment)
- icmp4.type = 3 (destination unreachable)
- icmp4.code = 1 (host unreachable)
Prerequisite: ip4

Signed-off-by: Lorenzo Bianconi 
---
 include/ovn/actions.h |  7 +++
 ovn/controller/pinctrl.c  | 51 +++
 ovn/lib/actions.c | 22 
 ovn/ovn-sb.xml|  4 
 ovn/utilities/ovn-trace.c | 28 ++
 5 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 9554a395d..16bea3f41 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -64,6 +64,7 @@ struct ovn_extend_table;
 OVNACT(CT_CLEAR,  ovnact_null)\
 OVNACT(CLONE, ovnact_nest)\
 OVNACT(ARP,   ovnact_nest)\
+OVNACT(ICMP,  ovnact_nest)\
 OVNACT(ND_NA, ovnact_nest)\
 OVNACT(GET_ARP,   ovnact_get_mac_bind)\
 OVNACT(PUT_ARP,   ovnact_put_mac_bind)\
@@ -429,6 +430,12 @@ enum action_opcode {
  * The actions, in OpenFlow 1.3 format, follow the action_header.
  */
 ACTION_OPCODE_ND_NS,
+
+/* "icmp4 { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ACTION_OPCODE_ICMP,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 14c95ff54..6fe0f64dd 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -217,6 +217,53 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
 dp_packet_uninit();
 }
 
+static void
+pinctrl_handle_icmp(const struct flow *ip_flow, const struct match *md,
+struct ofpbuf *userdata)
+{
+/* This action only works for IP packets, and the switch should only send
+ * us IP packets this way, but check here just to be sure. */
+if (ip_flow->dl_type != htons(ETH_TYPE_IP)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "ICMP action on non-IP packet (Ethertype %"PRIx16")",
+ ntohs(ip_flow->dl_type));
+return;
+}
+
+uint64_t packet_stub[128 / 8];
+struct dp_packet packet;
+
+dp_packet_use_stub(, packet_stub, sizeof packet_stub);
+dp_packet_clear();
+packet.packet_type = htonl(PT_ETH);
+
+struct eth_header *eh = dp_packet_put_zeros(, sizeof *eh);
+eh->eth_dst = ip_flow->dl_dst;
+eh->eth_src = ip_flow->dl_src;
+eh->eth_type = htons(ETH_TYPE_IP);
+
+struct ip_header *nh = dp_packet_put_zeros(, sizeof *nh);
+dp_packet_set_l3(, nh);
+nh->ip_ihl_ver = IP_IHL_VER(5, 4);
+nh->ip_tot_len = htons(sizeof(struct ip_header) +
+   sizeof(struct icmp_header));
+nh->ip_proto = IPPROTO_ICMP;
+nh->ip_frag_off = htons(IP_DF);
+packet_set_ipv4(, ip_flow->nw_src, ip_flow->nw_dst, 0, 255);
+
+struct icmp_header *ih = dp_packet_put_zeros(, sizeof *ih);
+dp_packet_set_l4(, ih);
+packet_set_icmp(, ICMP4_DST_UNREACH, 1);
+
+if (ip_flow->vlans[0].tci & htons(VLAN_CFI)) {
+eth_push_vlan(, htons(ETH_TYPE_VLAN_8021Q),
+  ip_flow->vlans[0].tci);
+}
+
+set_actions_and_enqueue_msg(, md, userdata);
+dp_packet_uninit();
+}
+
 static void
 pinctrl_handle_put_dhcp_opts(
 struct dp_packet *pkt_in, struct ofputil_packet_in *pin,
@@ -1020,6 +1067,10 @@ process_packet_in(const struct ofp_header *msg, struct 
controller_ctx *ctx)
 pinctrl_handle_nd_ns(, _metadata, );
 break;
 
+case ACTION_OPCODE_ICMP:
+pinctrl_handle_icmp(, _metadata, );
+break;
+
 default:
 VLOG_WARN_RL(, "unrecognized packet-in opcode %"PRIu32,
  ntohl(ah->opcode));
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index fde3bff00..6e1bf541e 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1141,6 +1141,12 @@ parse_ARP(struct action_context *ctx)
 parse_nested_action(ctx, OVNACT_ARP, "ip4");
 }
 
+static void
+parse_ICMP(struct action_context *ctx)
+{
+parse_nested_action(ctx, OVNACT_ICMP, "ip4");
+}
+
 static void
 parse_ND_NA(struct action_context *ctx)
 {
@@ -1174,6 +1180,12 @@ format_ARP(const struct ovnact_nest *nest, struct ds *s)
 format_nested_action(nest, "arp", s);
 }
 
+static void
+format_ICMP(const struct ovnact_nest *nest, struct ds *s)
+{
+format_nested_action(nest, "icmp4", s);
+}
+
 static void
 format_ND_NA(const struct ovnact_nest *nest, struct ds *s)
 {
@@ -1224,6 +1236,14 @@ encode_ARP(const struct ovnact_nest *on,
 encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
 }
 
+static void

[ovs-dev] [PATCH 3/3] OVN: add acl reject support using icmp4 action

[Lorenzo Bianconi]

Whenever the acl reject rule is hit send back an ICMPv4 destination
unreachable packet and do not handle reject rule as drop one.
Treat TCP connections as DROP for the moment since tcp_reset{} action
has not been implemented yet.

Signed-off-by: Lorenzo Bianconi 
---
 ovn/northd/ovn-northd.c | 123 +---
 1 file changed, 86 insertions(+), 37 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4d95a3d9d..90fcf9f9e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2895,13 +2895,18 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
*lflows)
 
 /* Ingress and Egress Pre-ACL Table (Priority 110).
  *
- * Not to do conntrack on ND packets. */
+ * Not to do conntrack on ND and ICMP destination
+ * unreachable packets. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "nd", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "(nd_rs || nd_ra)",
   "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+  "ip4 && icmp4.type == 3", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "nd", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
   "(nd_rs || nd_ra)", "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+  "ip4 && icmp4.type == 3", "next;");
 
 /* Ingress and Egress Pre-ACL Table (Priority 100).
  *
@@ -3082,6 +3087,46 @@ build_acl_log(struct ds *actions, const struct nbrec_acl 
*acl)
 ds_put_cstr(actions, "); ");
 }
 
+static void
+build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
+   enum ovn_stage stage, struct nbrec_acl *acl,
+   struct ds *extra_match, struct ds *extra_actions)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+bool ingress = (stage == S_SWITCH_IN_ACL);
+
+/* XXX: Treat TCP connections as "drop;" for now */
+build_acl_log(, acl);
+if (extra_match->length > 0) {
+ds_put_format(, "(%s) && ", extra_match->string);
+}
+ds_put_format(, "ip && tcp && (%s)", acl->match);
+ds_put_cstr(, "/* drop */");
+ovn_lflow_add(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET,
+  ds_cstr(), ds_cstr());
+
+/* IPv4 traffic */
+ds_clear();
+ds_clear();
+build_acl_log(, acl);
+if (extra_match->length > 0) {
+ds_put_format(, "(%s) && ", extra_match->string);
+}
+ds_put_format(, "ip4 && (%s)", acl->match);
+if (extra_actions->length > 0) {
+ds_put_format(, "%s ", extra_actions->string);
+}
+ds_put_format(, "reg0 = 0; "
+  "eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
+  "icmp4 { outport <-> inport; %s };",
+  ingress ? "output;" : "next(pipeline=ingress,table=0);");
+ovn_lflow_add(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET,
+  ds_cstr(), ds_cstr());
+ds_destroy();
+ds_destroy();
+}
+
 static void
 build_acls(struct ovn_datapath *od, struct hmap *lflows)
 {
@@ -3261,29 +3306,26 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
 struct ds match = DS_EMPTY_INITIALIZER;
 struct ds actions = DS_EMPTY_INITIALIZER;
 
-/* XXX Need to support "reject", treat it as "drop;" for now. */
-if (!strcmp(acl->action, "reject")) {
-VLOG_INFO("reject is not a supported action");
-}
-
 /* The implementation of "drop" differs if stateful ACLs are in
  * use for this datapath.  In that case, the actions differ
  * depending on whether the connection was previously committed
  * to the connection tracker with ct_commit. */
 if (has_stateful) {
 /* If the packet is not part of an established connection, then
- * we can simply drop it. */
-ds_put_format(,
-  "(!ct.est || (ct.est && ct_label.blocked == 1)) "
-  "&& (%s)",
-  acl->match);
-build_acl_log(, acl);
-ds_put_cstr(, "/* drop */");
-ovn_lflow_add_with_hint(lflows, od, stage,
-acl->priority + OVN_ACL_PRI_OFFSET,
-ds_cstr(), ds_cstr(),
-stage_hint);
-
+ * we can simply reject/drop it. */
+ds_put_cstr(,
+"(!ct.est || (ct.est && ct_label.blocked == 1))");
+if (!strcmp(acl->action, "reject")) {
+build_reject_acl_rules(od, lflows, stage, acl, ,
+

[ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Stokes, Ian]

Hi Ben,

The following changes since commit 6feddcd5417d8b57a342f4378776f3b3751ea341:

  poc: Introduce Proof of Concepts (Package building) (2018-02-12 10:47:03 
-0800)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge

for you to fetch changes up to f50c1eb4a6879a6a5a2c45c90e18df4ba08a9d34:

  docs: Update supported DPDK versions. (2018-02-13 09:15:24 +)


Ian Stokes (1):
  docs: Update supported DPDK versions.

 Documentation/faq/releases.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.8

[Stokes, Ian]

Hi Ben,

The following changes since commit 995933f224d3f682541ef060a949986620df5f05:

  ovn: Allow DNS lookups over IPv6 (2018-02-09 09:59:50 -0800)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_8

for you to fetch changes up to 4956bbfea5135644fcb6773ad2007524ebfcdc9a:

  docs: Update supported DPDK versions. (2018-02-13 09:19:55 +)


Ian Stokes (1):
  docs: Update supported DPDK versions.

 Documentation/faq/releases.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks
Ian

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

[ovs-dev] [PATCH v3] tests: Add system-dpdk-testsuite

[Marcin Rybka]

New OVS-DPDK testsuite, which can be launched via `make check-dpdk`,
tests OVS using a DPDK datapath. The testsuite contains already
initial tests:
 1. EAL init
 2. Add standard DPDK PHY port
 3. Add vhost-user-client port

Signed-off-by: Marcin Rybka 

---

Changed dpdk-socket-mem to the default value.
Documentation part contains information about root privileges.

---
 Documentation/topics/testing.rst | 19 
 tests/automake.mk| 17 +++
 tests/system-dpdk-macros.at  | 54 +
 tests/system-dpdk-testsuite.at   | 25 
 tests/system-dpdk.at | 65 
 5 files changed, 180 insertions(+)
 create mode 100644 tests/system-dpdk-macros.at
 create mode 100644 tests/system-dpdk-testsuite.at
 create mode 100644 tests/system-dpdk.at

diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst
index 2538571..c5d5b01 100644
--- a/Documentation/topics/testing.rst
+++ b/Documentation/topics/testing.rst
@@ -297,6 +297,25 @@ To invoke the datapath testsuite with the userspace 
datapath, run::
 
 The results of the testsuite are in ``tests/system-userspace-testsuite.dir``.
 
+DPDK datapath
+'
+
+To test :doc:`/intro/install/dpdk` (i.e., the build was configured with
+``--with-dpdk``, the DPDK is installed), run the testsuite and generate
+a report by using the ``check-dpdk`` target::
+
+$ make check-dpdk
+
+To see a list of all the available tests, run::
+
+$ make check-dpdk TESTSUITEFLAGS=--list
+
+These tests require a `DPDK supported NIC`_ and proper DPDK variables
+(``DPDK_DIR`` and ``DPDK_BUILD``). Moreover you need to have root privileges,
+load the required modules and bind the NIC to the DPDK-compatible driver.
+
+.. _DPDK supported NIC: http://dpdk.org/doc/nics
+
 Kernel datapath
 '''
 
diff --git a/tests/automake.mk b/tests/automake.mk
index 18698eb..0d19caa 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -5,10 +5,12 @@ EXTRA_DIST += \
$(SYSTEM_KMOD_TESTSUITE_AT) \
$(SYSTEM_USERSPACE_TESTSUITE_AT) \
$(SYSTEM_OFFLOADS_TESTSUITE_AT) \
+   $(SYSTEM_DPDK_TESTSUITE_AT) \
$(TESTSUITE) \
$(SYSTEM_KMOD_TESTSUITE) \
$(SYSTEM_USERSPACE_TESTSUITE) \
$(SYSTEM_OFFLOADS_TESTSUITE) \
+   $(SYSTEM_DPDK_TESTSUITE) \
tests/atlocal.in \
$(srcdir)/package.m4 \
$(srcdir)/tests/testsuite \
@@ -127,6 +129,12 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \
tests/system-offloads-traffic.at \
tests/system-offloads-testsuite.at
 
+SYSTEM_DPDK_TESTSUITE_AT = \
+   tests/system-common-macros.at \
+   tests/system-dpdk-macros.at \
+   tests/system-dpdk-testsuite.at \
+   tests/system-dpdk.at
+
 check_SCRIPTS += tests/atlocal
 
 TESTSUITE = $(srcdir)/tests/testsuite
@@ -134,6 +142,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
 SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
 SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite
 SYSTEM_OFFLOADS_TESTSUITE = $(srcdir)/tests/system-offloads-testsuite
+SYSTEM_DPDK_TESTSUITE = $(srcdir)/tests/system-dpdk-testsuite
 DISTCLEANFILES += tests/atconfig tests/atlocal
 
 AUTOTEST_PATH = 
utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL):ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller
@@ -257,6 +266,10 @@ check-offloads: all
set $(SHELL) '$(SYSTEM_OFFLOADS_TESTSUITE)' -C tests  
AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1; \
"$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck)
 
+check-dpdk: all
+   set $(SHELL) '$(SYSTEM_DPDK_TESTSUITE)' -C tests  
AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1; \
+   "$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck)
+
 clean-local:
test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
 
@@ -285,6 +298,10 @@ $(SYSTEM_OFFLOADS_TESTSUITE): package.m4 
$(SYSTEM_TESTSUITE_AT) $(SYSTEM_OFFLOAD
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
$(AM_V_at)mv $@.tmp $@
 
+$(SYSTEM_DPDK_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) 
$(SYSTEM_DPDK_TESTSUITE_AT) $(COMMON_MACROS_AT)
+   $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
+   $(AM_V_at)mv $@.tmp $@
+
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
 $(srcdir)/package.m4: $(top_srcdir)/configure.ac
$(AM_V_GEN):;{ \
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
new file mode 100644
index 000..58603aa
--- /dev/null
+++ b/tests/system-dpdk-macros.at
@@ -0,0 +1,54 @@
+# OVS_DPDK_PRE_CHECK()
+#
+# Check prerequisites for DPDK tests. Following settings are checked:
+#  - Hugepages
+#  - UIO driver
+#
+m4_define([OVS_DPDK_PRE_CHECK],
+  [dnl Check Hugepages
+   AT_CHECK([cat /proc/meminfo], [], [stdout])
+   AT_CHECK([grep HugePages_ stdout], [], [stdout])
+   AT_CHECK([mount], [], 

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.6

[Stokes, Ian]

Hi Ben,

The following changes since commit d893a3a47501e236bf514efabdd1d29575aa5766:

  gre: strip gre-tso offload flags (2018-01-26 09:43:50 -0800)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_6

for you to fetch changes up to 2172c6b030bfd7acebfcf31648a3162e7732b1ba:

  netdev-dpdk: Fix requested MTU size validation. (2018-02-06 11:45:35 +)


Ian Stokes (1):
  netdev-dpdk: Fix requested MTU size validation.

zhangliping (1):
  netdev-dpdk: fix ingress_policer leak on error path

 INSTALL.DPDK.md   | 12 
 lib/netdev-dpdk.c | 14 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

Thanks
Ian

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

[ovs-dev] 2018 lottery

[fnfgeewgwefg bgwergegrw]

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

Re: [ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared mempools.

[Stokes, Ian]

> >
> > On 02/13/2018 10:59 AM, Ian Stokes wrote:
> > > This commit manually reverts the current per port mempool model to
> > > the previous shared mempool model for DPDK ports.
> > >
> > > OVS previously used a shared mempool model for ports with the same
> > > MTU configuration. This was replaced by a per port mempool model to
> > > address issues flagged by users such as:
> > >
> > > https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/04
> > > 25
> > > 60.html
> > >
> > > However the per port model has a number of issues including:
> > >
> > > 1. Requires an increase in memory resource requirements to support
> > > the same number of ports as the shared port model.
> > > 2. Incorrect algorithm for mbuf provisioning for each mempool.
> > >
> > > These are considered blocking factors for current deployments of OVS
> > > when upgrading to OVS 2.9 as a  user may have to redimension memory
> > > for the same deployment configuration. This may not be possible for
> > users.
> > >
> > > For clarity, the commits whose changes are removed include the
> > > following:
> > >
> > > netdev-dpdk: Create separate memory pool for each port: d555d9b
> > > netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> > > mempool names to reflect socket id: f06546a
> > > netdev-dpdk: skip init for existing mempools: 837c176
> > > netdev-dpdk: manage failure in mempool name creation: 65056fd
> > > netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> > > netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> > > netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> > > netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> > > netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> > > netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> > > netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> > >
> > > Due to the number of commits and period of time they were introduced
> > > over, a simple revert was not possible. All code from the commits
> > > above is removed and the shared mempool code reintroduced as it was
> > > before its replacement.
> > >
> > > Code introduced by commit
> > >
> > > netdev-dpdk: Add debug appctl to get mempool information: be48173
> > >
> > > has been modified to work with the shared mempool model.
> > >
> >
> > There is a couple of small changes for coding stds to this version
> > from the rfc, but one is not correct (see below). Apart from that...
> 
> Thanks for flagging this.

I've pushed this to dpdk_merge_2_9 and its part of the branch 2.9 pull request.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344413.html

Note this revert only applies to branch 2.9, I have not applied it to master.

Thanks
Ian

> 
> >
> > Acked-by: Kevin Traynor 
> > Tested-by: Kevin Traynor 
> >
> 
> ...
> 
> > >  ovs_mutex_lock(_mp_mutex);
> > > -VLOG_DBG("Releasing \"%s\" mempool", mp->name);
> > > -rte_mempool_free(mp);
> > > +ovs_assert(dmp->refcount);
> > > +
> > > +if (! --dmp->refcount) {
> >
> > The space should not be added between ! and --
> >
> > http://docs.openvswitch.org/en/latest/internals/contributing/coding-
> > style/#expressions
> 
> Good catch, I can fix this before committing if there are no other
> comments.
> 
> Thanks
> Ian
> >
> > > +ovs_list_remove(>list_node);
> > > +rte_mempool_free(dmp->mp);
> > > +rte_free(dmp);
> > > + }
> > >  ovs_mutex_unlock(_mp_mutex);  }
> > >
> > > -/* Tries to allocate a new mempool - or re-use an existing one
> > > where
> > > - * appropriate - on requested_socket_id with a size determined by
> > > - * requested_mtu and requested Rx/Tx queues.
> > > - * On success - or when re-using an existing mempool - the new
> > > configuration
> > > - * will be applied.
> > > +/* Tries to allocate new mempool on requested_socket_id with
> > > + * mbuf size corresponding to requested_mtu.
> > > + * On success new configuration will be applied.
> > >   * On error, device will be left unchanged. */  static int
> > > netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> > >  OVS_REQUIRES(dev->mutex)
> > >  {
> > >  uint32_t buf_size = dpdk_buf_size(dev->requested_mtu);
> > > -struct rte_mempool *mp;
> > > -int ret = 0;
> > > +struct dpdk_mp *mp;
> > >
> > > -mp = dpdk_mp_create(dev, FRAME_LEN_TO_MTU(buf_size));
> > > +mp = dpdk_mp_get(dev->requested_socket_id,
> > > + FRAME_LEN_TO_MTU(buf_size));
> > >  if (!mp) {
> > >  VLOG_ERR("Failed to create memory pool for netdev "
> > >   "%s, with MTU %d on socket %d: %s\n",
> > >   dev->up.name, dev->requested_mtu, dev-
> > >requested_socket_id,
> > > - rte_strerror(rte_errno));
> > > -ret = rte_errno;
> > > +rte_strerror(rte_errno));
> > > +return rte_errno;
> > >  } else {
> > > -/* If a new MTU was requested and its rounded value equals
> the
> > one
> > > - 

Re: [ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared mempools.

[Jan Scheurich]

Thanks, Ian!
This will give us time to come up with a proper solution for 2.10. Let's work 
on that now.
/Jan

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Stokes, Ian
> Sent: Tuesday, 13 February, 2018 16:52
> To: Stokes, Ian ; Kevin Traynor ; 
> d...@openvswitch.org
> Cc: Ilya Maximets ; Antonio Fischetti 
> 
> Subject: Re: [ovs-dev] [PATCH v1 branch-2.9] netdev-dpdk: Reintroduce shared 
> mempools.
> 
> > >
> > > On 02/13/2018 10:59 AM, Ian Stokes wrote:
> > > > This commit manually reverts the current per port mempool model to
> > > > the previous shared mempool model for DPDK ports.
> > > >
> > > > OVS previously used a shared mempool model for ports with the same
> > > > MTU configuration. This was replaced by a per port mempool model to
> > > > address issues flagged by users such as:
> > > >
> > > > https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/04
> > > > 25
> > > > 60.html
> > > >
> > > > However the per port model has a number of issues including:
> > > >
> > > > 1. Requires an increase in memory resource requirements to support
> > > > the same number of ports as the shared port model.
> > > > 2. Incorrect algorithm for mbuf provisioning for each mempool.
> > > >
> > > > These are considered blocking factors for current deployments of OVS
> > > > when upgrading to OVS 2.9 as a  user may have to redimension memory
> > > > for the same deployment configuration. This may not be possible for
> > > users.
> > > >
> > > > For clarity, the commits whose changes are removed include the
> > > > following:
> > > >
> > > > netdev-dpdk: Create separate memory pool for each port: d555d9b
> > > > netdev-dpdk: fix management of pre-existing mempools: b6b26021d Fix
> > > > mempool names to reflect socket id: f06546a
> > > > netdev-dpdk: skip init for existing mempools: 837c176
> > > > netdev-dpdk: manage failure in mempool name creation: 65056fd
> > > > netdev-dpdk: Reword mp_size as n_mbufs: ad9b5b9
> > > > netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free: a08a115
> > > > netdev-dpdk: Fix mp_name leak on snprintf failure: ec6edc8
> > > > netdev-dpdk: Fix dpdk_mp leak in case of EEXIST: 173ef76
> > > > netdev-dpdk: Factor out struct dpdk_mp: 24e78f9
> > > > netdev-dpdk: Remove unused MAX_NB_MBUF: bc57ed9
> > > > netdev-dpdk: Fix mempool creation with large MTU: af5b0da
> > > >
> > > > Due to the number of commits and period of time they were introduced
> > > > over, a simple revert was not possible. All code from the commits
> > > > above is removed and the shared mempool code reintroduced as it was
> > > > before its replacement.
> > > >
> > > > Code introduced by commit
> > > >
> > > > netdev-dpdk: Add debug appctl to get mempool information: be48173
> > > >
> > > > has been modified to work with the shared mempool model.
> > > >
> > >
> > > There is a couple of small changes for coding stds to this version
> > > from the rfc, but one is not correct (see below). Apart from that...
> >
> > Thanks for flagging this.
> 
> I've pushed this to dpdk_merge_2_9 and its part of the branch 2.9 pull 
> request.
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344413.html
> 
> Note this revert only applies to branch 2.9, I have not applied it to master.
> 
> Thanks
> Ian
> 
> >
> > >
> > > Acked-by: Kevin Traynor 
> > > Tested-by: Kevin Traynor 
> > >
> >
> > ...
> >
> > > >  ovs_mutex_lock(_mp_mutex);
> > > > -VLOG_DBG("Releasing \"%s\" mempool", mp->name);
> > > > -rte_mempool_free(mp);
> > > > +ovs_assert(dmp->refcount);
> > > > +
> > > > +if (! --dmp->refcount) {
> > >
> > > The space should not be added between ! and --
> > >
> > > http://docs.openvswitch.org/en/latest/internals/contributing/coding-
> > > style/#expressions
> >
> > Good catch, I can fix this before committing if there are no other
> > comments.
> >
> > Thanks
> > Ian
> > >
> > > > +ovs_list_remove(>list_node);
> > > > +rte_mempool_free(dmp->mp);
> > > > +rte_free(dmp);
> > > > + }
> > > >  ovs_mutex_unlock(_mp_mutex);  }
> > > >
> > > > -/* Tries to allocate a new mempool - or re-use an existing one
> > > > where
> > > > - * appropriate - on requested_socket_id with a size determined by
> > > > - * requested_mtu and requested Rx/Tx queues.
> > > > - * On success - or when re-using an existing mempool - the new
> > > > configuration
> > > > - * will be applied.
> > > > +/* Tries to allocate new mempool on requested_socket_id with
> > > > + * mbuf size corresponding to requested_mtu.
> > > > + * On success new configuration will be applied.
> > > >   * On error, device will be left unchanged. */  static int
> > > > netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
> > > >  OVS_REQUIRES(dev->mutex)
> > > >  {
> > > >  uint32_t buf_size = 

[ovs-dev] DPDK: How to send packets from one OVS bridge to another OVS bridge while running in the same user space ?

[Kapil A]

Hello,

In my setup, i have two DPDK ports, where dpdk0 is part of br0 and and
dpdk1 is part of br1. How can i send packets from br0 to br1 in an
efficient way within userspace with good performance?
I came across, veth pair as one option, but i couldn't find if it will
provide good performance in userspace while running in dpdk mode ?

My query might sound odd, but i have a use case where i need to send the
packets across bridges within the same userspace, so appreciate some help
on this.

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

Re: [ovs-dev] [PATCH 1/8] doc: Add an overview of the 'dpdk' port

[Aaron Conole]

Hi Stephen,

Stephen Finucane  writes:

> These ports are used to allow ingress/egress from the host and are
> therefore _reasonably_ important. However, there is no clear overview of
> what these ports actually are or why things are done the way they are.
> Start closing this gap by providing a standalone example of using these
> ports along with a little more detailed overview of the binding process.
>
> There is additional cleanup to be done for the DPDK howto, but that will
> be done separately.
>
> Signed-off-by: Stephen Finucane 
> Cc: Ciara Loftus 
> Cc: Kevin Traynor 
> ---
>  Documentation/topics/dpdk/index.rst |   1 +
>  Documentation/topics/dpdk/phy.rst   | 111 
> 
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/topics/dpdk/phy.rst
>
> diff --git a/Documentation/topics/dpdk/index.rst 
> b/Documentation/topics/dpdk/index.rst
> index da148b323..5f836a6e9 100644
> --- a/Documentation/topics/dpdk/index.rst
> +++ b/Documentation/topics/dpdk/index.rst
> @@ -28,5 +28,6 @@ The DPDK Datapath
>  .. toctree::
> :maxdepth: 2
>  
> +   phy
> vhost-user
> ring
> diff --git a/Documentation/topics/dpdk/phy.rst 
> b/Documentation/topics/dpdk/phy.rst
> new file mode 100644
> index 0..1c18e4e3d
> --- /dev/null
> +++ b/Documentation/topics/dpdk/phy.rst
> @@ -0,0 +1,111 @@
> +..
> +  Copyright 2018, Red Hat, Inc.
> +
> +  Licensed under the Apache License, Version 2.0 (the "License"); you may
> +  not use this file except in compliance with the License. You may obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See 
> the
> +  License for the specific language governing permissions and limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +DPDK Physical Ports
> +===
> +
> +The DPDK datapath provides a way to attach DPDK-backed physical
> interfaces to

I think it may make more sense:

  The netdev datapath allows attaching DPDK-backed physical interfaces...

> +allow high-performance ingress/egress from the host.
> +
> +.. versionchanged:: 2.7.0
> +
> +   Before Open vSwitch 2.7.0, it was necessary to prefix port names with a
> +   ``dpdk`` prefix. Starting with 2.7.0, this is no longer necessary.
> +
> +Quick Example
> +-
> +
> +This example demonstrates how to bind two ``dpdk`` ports, bound to physical
> +interfaces identified by hardware IDs ``:01:00.0`` and ``:01:00.1``, 
> to
> +an existing bridge called ``br0``::
> +
> +$ ovs-vsctl add-port br0 dpdk-p0 \
> +   -- set Interface dpdk-p0 type=dpdk options:dpdk-devargs=:01:00.0
> +$ ovs-vsctl add-port br0 dpdk-p1 \
> +   -- set Interface dpdk-p1 type=dpdk options:dpdk-devargs=:01:00.1
> +
> +For the above example to work, the two physical interfaces must be bound to
> +the DPDK poll-mode drivers in userspace rather than the traditional kernel
> +drivers. See the `binding NIC drivers ` section for 
> details.
> +
> +.. _dpdk-binding-nics:
> +
> +Binding NIC Drivers
> +---
> +
> +DPDK operates entirely in userspace and, as a result, requires use of its own
> +poll-mode drivers in user space for physical interfaces and a 
> passthrough-style
> +driver for the devices in kernel space.
> +
> +There are two different tools for binding drivers: :command:`driverctl` which
> +is a generic tool for persistently configuring alternative device drivers, 
> and
> +:command:`dpdk-devbind` which is a DPDK-specific tool and whose changes do 
> not
> +persist across reboots. In addition, there are two options available for this
> +kernel space driver - VFIO (Virtual Function I/O) and UIO (Userspace I/O) -
> +along with a number of drivers for each option. We will demonstrate examples 
> of
> +both tools and will use the ``vfio-pci`` driver, which is the more secure,
> +robust driver of those available. More information can be found in the `DPDK
> +documentation `__.

Just commentary - really happy to see driverctl getting some love.

> +
> +To list devices using :command:`driverctl`, run::
> +
> +$ driverctl -v list-devices | grep -i net
> +:07:00.0 igb (I350 Gigabit Network Connection (Ethernet Server 
> Adapter I350-T2))
> +:07:00.1 igb (I350 Gigabit Network Connection (Ethernet Server 

[ovs-dev] [PATCH 0/3] add acl reject rule support introducing icmp4 action

[Lorenzo Bianconi]

Changes since RFC:
- update ovn-sb.xml
- squash patch1 and patch2
- added Jakub comments
- added build_reject_acl_rules routine to add reject entries in logical
  flow hashmap
- treat TCP connections as DROP for the moment since tcp_reset{}
  action has not been implemented yet
- renamed encode_nested_neighbor_actions in encode_nested_actions

Lorenzo Bianconi (3):
  OVN: rename encode_nested_neighbor_actions in encode_nested_actions
  OVN: add icmp4{} action support
  OVN: add acl reject support using icmp4 action

 include/ovn/actions.h |   7 +++
 ovn/controller/pinctrl.c  |  51 +++
 ovn/lib/actions.c |  36 +++---
 ovn/northd/ovn-northd.c   | 123 --
 ovn/ovn-sb.xml|   4 --
 ovn/utilities/ovn-trace.c |  28 +++
 6 files changed, 201 insertions(+), 48 deletions(-)

-- 
2.14.3

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

[ovs-dev] [PATCH 1/3] OVN: rename encode_nested_neighbor_actions in encode_nested_actions

[Lorenzo Bianconi]

Rename encode_nested_neighbor_actions routine in encode_nested_actions
in order to have a more general name for nested actions encoder.

Signed-off-by: Lorenzo Bianconi 
---
 ovn/lib/actions.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index a6977d8ee..fde3bff00 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1193,10 +1193,10 @@ format_CLONE(const struct ovnact_nest *nest, struct ds 
*s)
 }
 
 static void
-encode_nested_neighbor_actions(const struct ovnact_nest *on,
-   const struct ovnact_encode_params *ep,
-   enum action_opcode opcode,
-   struct ofpbuf *ofpacts)
+encode_nested_actions(const struct ovnact_nest *on,
+  const struct ovnact_encode_params *ep,
+  enum action_opcode opcode,
+  struct ofpbuf *ofpacts)
 {
 /* Convert nested actions into ofpacts. */
 uint64_t inner_ofpacts_stub[1024 / 8];
@@ -1221,7 +1221,7 @@ encode_ARP(const struct ovnact_nest *on,
const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
 {
-encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
+encode_nested_actions(on, ep, ACTION_OPCODE_ARP, ofpacts);
 }
 
 static void
@@ -1229,7 +1229,7 @@ encode_ND_NA(const struct ovnact_nest *on,
  const struct ovnact_encode_params *ep,
  struct ofpbuf *ofpacts)
 {
-encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts);
+encode_nested_actions(on, ep, ACTION_OPCODE_ND_NA, ofpacts);
 }
 
 static void
@@ -1237,7 +1237,7 @@ encode_ND_NS(const struct ovnact_nest *on,
  const struct ovnact_encode_params *ep,
  struct ofpbuf *ofpacts)
 {
-encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts);
+encode_nested_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts);
 }
 
 static void
-- 
2.14.3

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

Re: [ovs-dev] [PATCH 1/2] ofp-util: Remove prototypes for unimplemented functions.

[Yifeng Sun]

Thanks for the clean up. Looks good to me.

Reviewed-by: Yifeng Sun 

On Fri, Feb 9, 2018 at 2:57 PM, Ben Pfaff  wrote:

> Signed-off-by: Ben Pfaff 
> ---
>  include/openvswitch/ofp-util.h | 12 
>  1 file changed, 12 deletions(-)
>
> diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-
> util.h
> index 5dd1b34c216b..876ca6ed280a 100644
> --- a/include/openvswitch/ofp-util.h
> +++ b/include/openvswitch/ofp-util.h
> @@ -39,7 +39,6 @@ extern "C" {
>  #endif
>
>  struct ofpbuf;
> -union ofp_action;
>  struct ofpact_set_field;
>  struct vl_mff_map;
>
> @@ -1190,17 +1189,6 @@ struct ofpbuf *make_echo_reply(const struct
> ofp_header *rq);
>
>  struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
>
> -/* Actions. */
> -
> -bool action_outputs_to_port(const union ofp_action *, ovs_be16 port);
> -
> -enum ofperr ofputil_pull_actions(struct ofpbuf *, unsigned int
> actions_len,
> - union ofp_action **, size_t *);
> -
> -bool ofputil_actions_equal(const union ofp_action *a, size_t n_a,
> -   const union ofp_action *b, size_t n_b);
> -union ofp_action *ofputil_actions_clone(const union ofp_action *, size_t
> n);
> -
>  /* Handy utility for parsing flows and actions. */
>  bool ofputil_parse_key_value(char **stringp, char **keyp, char **valuep);
>
> --
> 2.15.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Ben Pfaff]

Thanks for all the pull requests.  I merged all of these.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/4] openflow: Update list of features.

[Ben Pfaff]

On Tue, Feb 13, 2018 at 11:12:45AM -0800, William Tu wrote:
> On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff  wrote:
> > Bundles are implemented for both OF1.3 and OF1.4+, so no need to keep it
> > in the list.  Packet type aware pipeline is now implemented too.
> >
> > Signed-off-by: Ben Pfaff 
> 
> 
> Looks good to me.
> Acked-by: William Tu 

Thank you for the reviews.  I applied these to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/4] Implement OF1.3 extension for OF1.4 role status feature.

[William Tu]

On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff  wrote:
> ONF extension pack 1 for OpenFlow 1.3 defines how to implement the OpenFlow
> 1.4 "role status" message in OpenFlow 1.3.  This commit implements that
> feature.
>
> ONF-JIRA: EXT-191
> Signed-off-by: Ben Pfaff 
> ---

Looks good to me. I tested it without seeing any failure.

Acked-by: William Tu 

>  Documentation/topics/openflow.rst |  8 -
>  NEWS  |  2 ++
>  include/openvswitch/ofp-msgs.h|  5 ++-
>  lib/ofp-util.c| 33 ++--
>  tests/ofp-print.at| 30 ++
>  tests/ofproto.at  | 66 
> +++
>  6 files changed, 118 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/topics/openflow.rst 
> b/Documentation/topics/openflow.rst
> index d82d544d9246..381f94d9c22c 100644
> --- a/Documentation/topics/openflow.rst
> +++ b/Documentation/topics/openflow.rst
> @@ -159,14 +159,6 @@ in OVS.
>(EXT-187)
>(optional for OF1.4+)
>
> -* Role Status
> -
> -  Already implemented as a 1.4 feature.
> -
> -  (EXT-191)
> -
> -  (required for OF1.4+)
> -
>  * Flow entry eviction
>
>OVS has flow eviction functionality.  ``table_mod OFPTC_EVICTION``,
> diff --git a/NEWS b/NEWS
> index 6973f6b45ebf..4f1b3258d31b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.9.0
> default it always accepts names and in interactive use it displays 
> them;
> use --names or --no-names to override.  See ovs-ofctl(8) for details.
> - ovs-vsctl: New commands "add-bond-iface" and "del-bond-iface".
> +   - OpenFlow:
> + * OFPT_ROLE_STATUS is now available in OpenFlow 1.3.
>
>
>  v2.9.0 - xx xxx 
> diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
> index 5f3815c140ef..3f92f2a67751 100644
> --- a/include/openvswitch/ofp-msgs.h
> +++ b/include/openvswitch/ofp-msgs.h
> @@ -262,6 +262,8 @@ enum ofpraw {
>  /* OFPT 1.3+ (29): struct ofp13_meter_mod, uint8_t[8][]. */
>  OFPRAW_OFPT13_METER_MOD,
>
> +/* ONFT 1.3 (1911): struct ofp14_role_status, uint8_t[8][]. */
> +OFPRAW_ONFT13_ROLE_STATUS,
>  /* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */
>  OFPRAW_OFPT14_ROLE_STATUS,
>
> @@ -615,7 +617,8 @@ enum ofptype {
>  OFPTYPE_METER_MOD,/* OFPRAW_OFPT13_METER_MOD. */
>
>  /* Controller role change event messages. */
> -OFPTYPE_ROLE_STATUS,  /* OFPRAW_OFPT14_ROLE_STATUS. */
> +OFPTYPE_ROLE_STATUS,  /* OFPRAW_ONFT13_ROLE_STATUS.
> +   * OFPRAW_OFPT14_ROLE_STATUS. */
>
>  /* Request forwarding by the switch. */
>  OFPTYPE_REQUESTFORWARD,   /* OFPRAW_OFPT14_REQUESTFORWARD. */
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 59fbf5f58425..c3b1222a69fb 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -6201,24 +6201,21 @@ struct ofpbuf *
>  ofputil_encode_role_status(const struct ofputil_role_status *status,
> enum ofputil_protocol protocol)
>  {
> -enum ofp_version version;
> -
> -version = ofputil_protocol_to_ofp_version(protocol);
> -if (version >= OFP14_VERSION) {
> -struct ofp14_role_status *rstatus;
> -struct ofpbuf *buf;
> -
> -buf = ofpraw_alloc_xid(OFPRAW_OFPT14_ROLE_STATUS, version, htonl(0),
> -   0);
> -rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus);
> -rstatus->role = htonl(status->role);
> -rstatus->reason = status->reason;
> -rstatus->generation_id = htonll(status->generation_id);
> -
> -return buf;
> -} else {
> +enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
> +if (version < OFP13_VERSION) {
>  return NULL;
>  }
> +
> +enum ofpraw raw = (version >= OFP14_VERSION
> +   ? OFPRAW_OFPT14_ROLE_STATUS
> +   : OFPRAW_ONFT13_ROLE_STATUS);
> +struct ofpbuf *buf = ofpraw_alloc_xid(raw, version, htonl(0), 0);
> +struct ofp14_role_status *rstatus = ofpbuf_put_zeros(buf, sizeof 
> *rstatus);
> +rstatus->role = htonl(status->role);
> +rstatus->reason = status->reason;
> +rstatus->generation_id = htonll(status->generation_id);
> +
> +return buf;
>  }
>
>  enum ofperr
> @@ -6226,7 +6223,9 @@ ofputil_decode_role_status(const struct ofp_header *oh,
> struct ofputil_role_status *rs)
>  {
>  struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
> -ovs_assert(ofpraw_pull_assert() == OFPRAW_OFPT14_ROLE_STATUS);
> +enum ofpraw raw = ofpraw_pull_assert();
> +ovs_assert(raw == OFPRAW_OFPT14_ROLE_STATUS ||
> +   raw == OFPRAW_ONFT13_ROLE_STATUS);
>
>  const struct ofp14_role_status *r = b.msg;
>  if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) &&
> diff --git a/tests/ofp-print.at 

Re: [ovs-dev] [PATCH 3/4] ofp-errors: Add remaining OF1.4 and OF1.5 errors.

[William Tu]

On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff  wrote:
> Also implements the backports of relevant errors to OF1.3 as specified in
> ONF extension pack 1 for OF1.3.
>
> ONF-JIRA: EXT-237
> ONF-JIRA: EXT-230
> ONF-JIRA: EXT-264
> Signed-off-by: Ben Pfaff 
> ---

Looks good to me.
Acked-by: William Tu 

>  Documentation/topics/openflow.rst | 20 -
>  include/openvswitch/ofp-errors.h  | 88 
> ---
>  2 files changed, 73 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/topics/openflow.rst 
> b/Documentation/topics/openflow.rst
> index 381f94d9c22c..ea5a4450687d 100644
> --- a/Documentation/topics/openflow.rst
> +++ b/Documentation/topics/openflow.rst
> @@ -205,20 +205,6 @@ in OVS.
>
>(optional for OF1.4+)
>
> -* Bad flow entry priority error
> -
> -  Probably not so useful to the software switch.
> -
> -  (EXT-236)
> -
> -  (optional for OF1.4+)
> -
> -* Set async config error
> -
> -  (EXT-237)
> -
> -  (optional for OF1.4+)
> -
>  * PBB UCA header field
>
>See comment on Provider Backbone Bridge in section about OpenFlow 1.3.
> @@ -227,12 +213,6 @@ in OVS.
>
>(optional for OF1.4+)
>
> -* Multipart timeout error
> -
> -  (EXT-264)
> -
> -  (required for OF1.4+)
> -
>  OpenFlow 1.4 only
>  -
>
> diff --git a/include/openvswitch/ofp-errors.h 
> b/include/openvswitch/ofp-errors.h
> index 283b9af40248..6542f10b46fd 100644
> --- a/include/openvswitch/ofp-errors.h
> +++ b/include/openvswitch/ofp-errors.h
> @@ -169,10 +169,10 @@ enum ofperr {
>  /* OF1.0-1.1(1,5), OF1.2+(1,10).  Denied because controller is slave. */
>  OFPERR_OFPBRC_IS_SLAVE,
>
> -/* NX1.0-1.1(1,514), OF1.2+(1,11).  Invalid port.  [ A non-standard error
> - * (1,514), formerly OFPERR_NXBRC_BAD_IN_PORT is used for OpenFlow 1.0 
> and
> - * 1.1 as there seems to be no appropriate error code defined the
> - * specifications. ] */
> +/* NX1.0-1.1(1,514), OF1.2+(1,11).  Invalid or missing port.  [ A
> + * non-standard error (1,514), formerly OFPERR_NXBRC_BAD_IN_PORT is used
> + * for OpenFlow 1.0 and 1.1 as there seems to be no appropriate error 
> code
> + * defined the specifications. ] */
>  OFPERR_OFPBRC_BAD_PORT,
>
>  /* OF1.2+(1,12).  Invalid packet in packet-out. */
> @@ -181,9 +181,22 @@ enum ofperr {
>  /* OF1.3+(1,13).  Multipart request overflowed the assigned buffer. */
>  OFPERR_OFPBRC_MULTIPART_BUFFER_OVERFLOW,
>
> +/* ONF1.3(2640), OF1.4+(1,14).  Timeout during multipart request. */
> +OFPERR_OFPBRC_MULTIPART_REQUEST_TIMEOUT,
> +
> +/* ONF1.3(2641), OF1.4+(1,15).  Timeout during multipart reply. */
> +OFPERR_OFPBRC_MULTIPART_REPLY_TIMEOUT,
> +
> +/* OF1.5+(1,16).  Switch received a OFPMP_BUNDLE_FEATURES request and
> + * failed to update the scheduling tolerance. */
> +OFPERR_OFPBRC_MULTIPART_BAD_SCHED,
> +
>  /* OF1.5+(1,17).  Match fields must include only pipeline fields. */
>  OFPERR_OFPBRC_PIPELINE_FIELDS_ONLY,
>
> +/* OF1.5+(1,18).  Unspecified error. */
> +OFPERR_OFPBRC_UNKNOWN,
> +
>  /* NX1.0-1.1(1,256), NX1.2+(2).  Invalid NXM flow match. */
>  OFPERR_NXBRC_NXM_INVALID,
>
> @@ -212,7 +225,7 @@ enum ofperr {
>  /* ## OFPET_BAD_ACTION ## */
>  /* ##  ## */
>
> -/* OF1.0+(2,0).  Unknown action type. */
> +/* OF1.0+(2,0).  Unknown or unsupported action type. */
>  OFPERR_OFPBAC_BAD_TYPE,
>
>  /* OF1.0+(2,1).  Length problem in actions. */
> @@ -239,7 +252,7 @@ enum ofperr {
>  /* OF1.0+(2,8).  Problem validating output queue. */
>  OFPERR_OFPBAC_BAD_QUEUE,
>
> -/* OF1.1+(2,9).  Invalid group id in forward action. */
> +/* OF1.1+(2,9).  Invalid group id in output action. */
>  OFPERR_OFPBAC_BAD_OUT_GROUP,
>
>  /* NX1.0(1,522), OF1.1+(2,10).  Action can't apply for this match or a
> @@ -269,6 +282,9 @@ enum ofperr {
>   * bit set to 1. */
>  OFPERR_OFPBAC_BAD_SET_MASK,
>
> +/* OF1.5+(2,17).  Invalid meter id in meter action. */
> +OFPERR_OFPBAC_BAD_METER,
> +
>  /* NX1.0-1.1(2,256), NX1.2+(11).  Must-be-zero action argument had 
> nonzero
>   * value. */
>  OFPERR_NXBAC_MUST_BE_ZERO,
> @@ -360,8 +376,8 @@ enum ofperr {
>   * field. */
>  OFPERR_OFPBMC_BAD_VALUE,
>
> -/* NX1.0-1.1(1,259), OF1.2+(4,8).  Unsupported mask specified in the 
> match,
> - * field is not dl-address or nw-address. */
> +/* NX1.0-1.1(1,259), OF1.2+(4,8).  Unsupported mask specified in the
> + * match. */
>  OFPERR_OFPBMC_BAD_MASK,
>
>  /* NX1.0-1.1(1,260), OF1.2+(4,9).  A prerequisite was not met. */
> @@ -413,6 +429,15 @@ enum ofperr {
>   * flags. */
>  OFPERR_OFPFMFC_BAD_FLAGS,
>
> +/* OF1.4+(5,8).  Problem in table synchronization. */
> +OFPERR_OFPFMFC_CANT_SYNC,
> +
> +/* ONF1.3(2360), OF1.4+(5,9).  Unsupported priority value. */
> +OFPERR_OFPFMFC_BAD_PRIORITY,
> +
> +/* OF1.4+(5,10). 

Re: [ovs-dev] [PATCH 2/4] extract-ofp-errors: Minor improvements.

[William Tu]

On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff  wrote:
> This removes the requirement of exactly two spaces before the error
> description (now one or more is fine).  It also makes an error message
> clearer.
>
> Signed-off-by: Ben Pfaff 
> ---

Looks good to me.
Acked-by: William Tu 

>  build-aux/extract-ofp-errors | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/build-aux/extract-ofp-errors b/build-aux/extract-ofp-errors
> index 71ae0bd5f0ff..6c14f68b883a 100755
> --- a/build-aux/extract-ofp-errors
> +++ b/build-aux/extract-ofp-errors
> @@ -247,7 +247,7 @@ def extract_ofp_errors(fn):
>  expected_errors[m.group(1)] = (fileName, lineNumber)
>  continue
>
> -m = re.match('((?:.(?!\.  ))+.)\.  (.*)$', comment)
> +m = re.match('((?:.(?!\.  ))+.)\.\s+(.*)$', comment)
>  if not m:
>  fatal("unexpected syntax between errors")
>
> @@ -257,7 +257,7 @@ def extract_ofp_errors(fn):
>  m = re.match('\s+(?:OFPERR_([A-Z0-9_]+))(\s*=\s*OFPERR_OFS)?,',
>   line)
>  if not m:
> -fatal("syntax error expecting enum value")
> +fatal("syntax error expecting OFPERR_ enum value")
>
>  enum = m.group(1)
>  if enum in names:
> --
> 2.15.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofp-flow: Fix return value for ofputil_decode_flow_stats_reply().

[Ben Pfaff]

This function returned errno values for some errors and OFPERR_* values
for others.  The callers all expected OFPERR_* values.  This fixes the
problem.

Signed-off-by: Ben Pfaff 
---
 lib/ofp-flow.c | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
index 8876b7be5eb1..c29f5b7cb05f 100644
--- a/lib/ofp-flow.c
+++ b/lib/ofp-flow.c
@@ -752,7 +752,7 @@ parse_ofp_flow_stats_request_str(struct 
ofputil_flow_stats_request *fsr,
  * of it.  'fs->ofpacts' will point into the 'ofpacts' buffer.
  *
  * Returns 0 if successful, EOF if no replies were left in this 'msg',
- * otherwise a positive errno value. */
+ * otherwise an OFPERR_* value. */
 int
 ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
 struct ofpbuf *msg,
@@ -783,20 +783,21 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats 
*fs,
 if (!ofs) {
 VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
  "bytes at end", msg->size);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
 
 length = ntohs(ofs->length);
 if (length < sizeof *ofs) {
 VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
  "length %"PRIuSIZE, length);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
 
-if (ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
- _match_len)) {
+error = ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
+ _match_len);
+if (error) {
 VLOG_WARN_RL(, "OFPST_FLOW reply bad match");
-return EINVAL;
+return error;
 }
 instructions_len = length - sizeof *ofs - padded_match_len;
 
@@ -833,14 +834,14 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats 
*fs,
 if (!ofs) {
 VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
  "bytes at end", msg->size);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
 
 length = ntohs(ofs->length);
 if (length < sizeof *ofs) {
 VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
  "length %"PRIuSIZE, length);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
 instructions_len = length - sizeof *ofs;
 
@@ -866,7 +867,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats 
*fs,
 if (!nfs) {
 VLOG_WARN_RL(, "NXST_FLOW reply has %"PRIu32" leftover "
  "bytes at end", msg->size);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
 
 length = ntohs(nfs->length);
@@ -874,11 +875,12 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats 
*fs,
 if (length < sizeof *nfs + ROUND_UP(match_len, 8)) {
 VLOG_WARN_RL(, "NXST_FLOW reply with match_len=%"PRIuSIZE" "
  "claims invalid length %"PRIuSIZE, match_len, length);
-return EINVAL;
+return OFPERR_OFPBRC_BAD_LEN;
 }
-if (nx_pull_match(msg, match_len, >match, NULL, NULL, false, NULL,
-  NULL)) {
-return EINVAL;
+error = nx_pull_match(msg, match_len, >match, NULL, NULL, false,
+  NULL, NULL);
+if (error) {
+return error;
 }
 instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
 
@@ -907,10 +909,12 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats 
*fs,
 OVS_NOT_REACHED();
 }
 
-if (ofpacts_pull_openflow_instructions(msg, instructions_len, oh->version,
-   NULL, NULL, ofpacts)) {
+error = ofpacts_pull_openflow_instructions(msg, instructions_len,
+   oh->version, NULL, NULL,
+   ofpacts);
+if (error) {
 VLOG_WARN_RL(, "OFPST_FLOW reply bad instructions");
-return EINVAL;
+return error;
 }
 fs->ofpacts = ofpacts->data;
 fs->ofpacts_len = ofpacts->size;
-- 
2.16.1

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

Re: [ovs-dev] [PATCH 4/4] openflow: Update list of features.

[William Tu]

On Thu, Feb 8, 2018 at 3:18 PM, Ben Pfaff  wrote:
> Bundles are implemented for both OF1.3 and OF1.4+, so no need to keep it
> in the list.  Packet type aware pipeline is now implemented too.
>
> Signed-off-by: Ben Pfaff 


Looks good to me.
Acked-by: William Tu 

> ---
>  Documentation/topics/openflow.rst | 24 
>  1 file changed, 24 deletions(-)
>
> diff --git a/Documentation/topics/openflow.rst 
> b/Documentation/topics/openflow.rst
> index ea5a4450687d..2f2cc830964b 100644
> --- a/Documentation/topics/openflow.rst
> +++ b/Documentation/topics/openflow.rst
> @@ -175,22 +175,6 @@ in OVS.
>
>(optional for OF1.4+)
>
> -* Bundle
> -
> -  Transactional modification.  OpenFlow 1.4 requires to support
> -  ``flow_mods`` and ``port_mods`` in a bundle if bundle is supported.
> -  (Not related to OVS's 'ofbundle' stuff.)
> -
> -  Implemented as an OpenFlow 1.4 feature.  Only flow_mods and port_mods are
> -  supported in a bundle.  If the bundle includes port mods, it may not 
> specify
> -  the ``OFPBF_ATOMIC`` flag.  Nevertheless, port mods and flow mods in a 
> bundle
> -  are always applied in order and consecutive flow mods between port mods are
> -  made available to lookups atomically.
> -
> -  (EXT-230)
> -
> -  (optional for OF1.4+)
> -
>  * Table synchronisation
>
>Probably not so useful to the software switch.
> @@ -270,14 +254,6 @@ definitive as OpenFlow 1.5 is not yet published.
>
>(optional for OF1.5+)
>
> -* Packet Type aware pipeline
> -
> -  Prototype for OVS was done during specification.
> -
> -  (EXT-112)
> -
> -  (optional for OF1.5+)
> -
>  * Extensible Flow Entry Statistics
>
>(EXT-334)
> --
> 2.15.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] HII

[Smith]

--
Hello My Dear,
is nice to meet you, my names are Mary Smith, how are you doing,i hope  
all is well with you (mariasmitha...@gmail.com),i look forward to  
reading from you soon,

Yours Friend,
Mary Smith.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/3] add acl reject rule support introducing icmp4 action

[Mark Michelson]

Hi Lorenzo,

I had a look through the patch series and it looks good to me. However, 
I think there should be a test added to the testsuite to ensure that the 
ACL reject action works as expected.


On 02/13/2018 08:43 AM, Lorenzo Bianconi wrote:

Changes since RFC:
- update ovn-sb.xml
- squash patch1 and patch2
- added Jakub comments
- added build_reject_acl_rules routine to add reject entries in logical
   flow hashmap
- treat TCP connections as DROP for the moment since tcp_reset{}
   action has not been implemented yet
- renamed encode_nested_neighbor_actions in encode_nested_actions

Lorenzo Bianconi (3):
   OVN: rename encode_nested_neighbor_actions in encode_nested_actions
   OVN: add icmp4{} action support
   OVN: add acl reject support using icmp4 action

  include/ovn/actions.h |   7 +++
  ovn/controller/pinctrl.c  |  51 +++
  ovn/lib/actions.c |  36 +++---
  ovn/northd/ovn-northd.c   | 123 --
  ovn/ovn-sb.xml|   4 --
  ovn/utilities/ovn-trace.c |  28 +++
  6 files changed, 201 insertions(+), 48 deletions(-)


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

[ovs-dev] [PATCH] rhel: don't drop capabilities when running as root

[Aaron Conole]

Currently, regardless of which user is being set as the running user,
Open vSwitch daemons on RHEL systems drop capabilities.  This means the
very powerful CAP_SYS_ADMIN is dropped, even when the user is 'root'.

For the majority of use cases this behavior works, as the user can
enable or disable various configurations, regardless of which datapath
functions are desired.  However, when using certain DPDK PMDs, the
enablement and configuration calls require CAP_SYS_ADMIN.

Instead of retaining CAP_SYS_ADMIN in all cases, which would practically
nullify the uid/gid and privilege drop, we don't pass the --ovs-user
option to the daemons.  This shunts the capability and privilege
dropping code.

Reported-by: Marcos Felipe Schwarz 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/045955.html
Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
Signed-off-by: Aaron Conole 
---
NOTE: I did test this a little bit on my system, passing packets, etc.
  But more eyes can't be bad.

 rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 7 ---
 rhel/usr_lib_systemd_system_ovsdb-server.service| 6 --
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index c6d9aa1..889740f 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -13,17 +13,18 @@ Restart=on-failure
 Environment=HOME=/var/run/openvswitch
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
+EnvironmentFile=-/run/openvswitch/useropts
 @begin_dpdk@
-ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
+ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:} /dev/hugepages'
 ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
 @end_dpdk@
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovsdb-server --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server \
   --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   restart $OPTIONS
 TimeoutSec=300
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 234d393..e05742d 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -11,13 +11,15 @@ Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
 ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
"${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
"OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
+EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
   --no-ovs-vswitchd --no-monitor --system-id=random \
-  --ovs-user=${OVS_USER_ID} \
+  ${OVSUSER} \
   start $OPTIONS
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd stop
 ExecReload=/usr/share/openvswitch/scripts/ovs-ctl --no-ovs-vswitchd \
-   --ovs-user=${OVS_USER_ID} \
+   ${OVSUSER} \
--no-monitor restart $OPTIONS
 RuntimeDirectory=openvswitch
 RuntimeDirectoryMode=0755
--
2.9.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 00/11] Add upstream Linux 4.14 kernel support

[Gregory Rose]

On 2/7/2018 7:49 AM, Greg Rose wrote:

This patch set includes new features from upstream Linux kernel
4.14 as well as some additional bug fixes specific to the new
feature.  It also updates the travis build kernel list and makes
a few other small changes so OVS can compile on Linux 4.14
kernels.  This patch set is dependent on the first 10 patches
posted here:

https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344253.html

V2 of this patch set addresses some suggested changes and also
fixes up the .travis.yml file to include libelf-dev as a required
package.

Andy Zhou (3):
   datapath: export get_dp() API
   datapath: Add meter netlink definitions
   datapath: Add meter infrastructure

Greg Rose (3):
   Documentation: Update NEWS and faq
   acinclude.m4: Enable Linux 4.14
   travis: Update kernel test list from kernel.org

Gustavo A. R. Silva (2):
   datapath: meter: fix NULL pointer dereference in
 ovs_meter_cmd_reply_start
   datapath: meter: Use 64-bit arithmetic instead of 32-bit

Jiri Benc (1):
   datapath: reliable interface indentification in port dumps

Wei Yongjun (2):
   datapath: Fix return value check in ovs_meter_cmd_features()
   datapath: Using kfree_rcu() to simplify the code

  .travis.yml   |  18 +-
  Documentation/faq/releases.rst|   1 +
  NEWS  |   2 +
  acinclude.m4  |  10 +-
  datapath/Modules.mk   |   6 +-
  datapath/datapath.c   |  92 ++--
  datapath/datapath.h   |  38 +-
  datapath/dp_notify.c  |   3 +-
  datapath/linux/compat/include/linux/openvswitch.h |  53 ++
  datapath/linux/compat/include/net/netlink.h   |   9 +
  datapath/meter.c  | 607 ++
  datapath/meter.h  |  54 ++
  12 files changed, 833 insertions(+), 60 deletions(-)
  create mode 100644 datapath/meter.c
  create mode 100644 datapath/meter.h


Pravin,

ping?

Thanks,

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

Re: [ovs-dev] DPDK: How to send packets from one OVS bridge to another OVS bridge while running in the same user space ?

[Kapil A]

Hi Jan,

Thanks for the clarification. I also came across patch port as the best
option between ovs Bridges. But, is it possible to use patch ports to an
external OpenFlow bridge? Like one bridge is OVS, other is Lagopus bridge
or LINC bridge? For my use case, I want to send packets from OVS Bridge to
another OpenFlow bridge(like a loopback port) as OVS doesn't support the
functionality I am looking for.

On Tue, Feb 13, 2018, 21:22 Jan Scheurich 
wrote:

> Hi Kapil,
>
> This is what patch ports are for. They are only traversed for the first
> packet of a flow in the slow path. The resulting datapath flow entry
> collapses the processing in both bridges into a single megaflow. So there
> is no performance overhead compared to having a single bridge.
>
> Regards, Jan
>
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:
> ovs-dev-boun...@openvswitch.org] On Behalf Of Kapil A
> > Sent: Tuesday, 13 February, 2018 16:44
> > To: disc...@openvswitch.org; d...@openvswitch.org
> > Subject: [ovs-dev] DPDK: How to send packets from one OVS bridge to
> another OVS bridge while running in the same user space ?
> >
> > Hello,
> >
> > In my setup, i have two DPDK ports, where dpdk0 is part of br0 and and
> > dpdk1 is part of br1. How can i send packets from br0 to br1 in an
> > efficient way within userspace with good performance?
> > I came across, veth pair as one option, but i couldn't find if it will
> > provide good performance in userspace while running in dpdk mode ?
> >
> > My query might sound odd, but i have a use case where i need to send the
> > packets across bridges within the same userspace, so appreciate some help
> > on this.
> >
> > Regards
> > Kapil
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


-- 

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

[ovs-dev] Felicidad aplicada a la productividad

[Logre una vida más plena]

 
 
Cómo ser feliz en mi trabajo
Febrero 21 - webinar Interactivo

Que ganarás con este webinar:

Renovar tu pasión y vocación para sentirte orgulloso de tu trabajo, tu empresa, 
tu jefe y colaboradores.
Combatir positivamente situaciones estresantes para no dejarte llevar por la 
desmotivación. 
Orientarte sobre cómo la felicidad te puede ayudar a vivir una vida más plena y 
en bienestar.
  
 
Temario e Inscripciones:

Respondiendo por este medio "Trabajo"+TELÉFONO + NOMBRE o marcando al:

045 + 5515546630  



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

[ovs-dev] Detecte Fraudes Oportunamente

[No mas Fugas de Recursos]

En línea y en Vivo / Para todo su Equipo con una sola Conexión 

Sistemas Eficaces de Control a travéz de Auditorías Administrativas
22 de Febrero - Online en Vivo - Horario de 10:00 a 13:00 y de 15:00 a 18:00
   
 
¡No más fugas de recursos! Detecte y prevenga fraudes de manera oportuna, 
establezca las pruebas de control interno adecuadas y asegúrese del 
cumplimiento de las políticas de su organización. La elevada competividad en el 
mundo empresarial, obliga a toda organización, sin importar su tamaño, a 
mejorar sus procesos, ser más eficientes e idenficar sus fallas; para esto 
resulta imprescindible la gestión e implementación de controles internos. 
Una auditoría interna eficiente, ayudará a la gerencia a revisar, evaluar y 
verificar el uso y control de los recursos humanos, materiales y financieros. 

TEMARIO: 

1. Componentes de la Auditoría Administrativa.

2. Procedimientos de Auditoría Administrativa.

3. Instrumentación de la Auditoría Administrativa.

4. Emisión de Informes de Auditoría Administrativa.

- Y mucho más. 




 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 

Auditoría

Nombre:

Teléfono:

Empresa:

Centro telefónico: 018002129393


Lic. María Canul
Líder de Proyecto


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA..
 

 

 



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

Re: [ovs-dev] [PATCH] ofp-flow: Fix return value for ofputil_decode_flow_stats_reply().

[Ben Pfaff]

Thanks for the review.  I applied this to master.

On Tue, Feb 13, 2018 at 03:14:56PM -0800, Yifeng Sun wrote:
> Looks good to me. Thanks for the correction.
> 
> Reviewed-by: Yifeng Sun 
> 
> On Tue, Feb 13, 2018 at 10:49 AM, Ben Pfaff  wrote:
> 
> > This function returned errno values for some errors and OFPERR_* values
> > for others.  The callers all expected OFPERR_* values.  This fixes the
> > problem.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/ofp-flow.c | 36 
> >  1 file changed, 20 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
> > index 8876b7be5eb1..c29f5b7cb05f 100644
> > --- a/lib/ofp-flow.c
> > +++ b/lib/ofp-flow.c
> > @@ -752,7 +752,7 @@ parse_ofp_flow_stats_request_str(struct
> > ofputil_flow_stats_request *fsr,
> >   * of it.  'fs->ofpacts' will point into the 'ofpacts' buffer.
> >   *
> >   * Returns 0 if successful, EOF if no replies were left in this 'msg',
> > - * otherwise a positive errno value. */
> > + * otherwise an OFPERR_* value. */
> >  int
> >  ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
> >  struct ofpbuf *msg,
> > @@ -783,20 +783,21 @@ ofputil_decode_flow_stats_reply(struct
> > ofputil_flow_stats *fs,
> >  if (!ofs) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
> >   "bytes at end", msg->size);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> >
> >  length = ntohs(ofs->length);
> >  if (length < sizeof *ofs) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
> >   "length %"PRIuSIZE, length);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> >
> > -if (ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
> > - _match_len)) {
> > +error = ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
> > + _match_len);
> > +if (error) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply bad match");
> > -return EINVAL;
> > +return error;
> >  }
> >  instructions_len = length - sizeof *ofs - padded_match_len;
> >
> > @@ -833,14 +834,14 @@ ofputil_decode_flow_stats_reply(struct
> > ofputil_flow_stats *fs,
> >  if (!ofs) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
> >   "bytes at end", msg->size);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> >
> >  length = ntohs(ofs->length);
> >  if (length < sizeof *ofs) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
> >   "length %"PRIuSIZE, length);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> >  instructions_len = length - sizeof *ofs;
> >
> > @@ -866,7 +867,7 @@ ofputil_decode_flow_stats_reply(struct
> > ofputil_flow_stats *fs,
> >  if (!nfs) {
> >  VLOG_WARN_RL(, "NXST_FLOW reply has %"PRIu32" leftover "
> >   "bytes at end", msg->size);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> >
> >  length = ntohs(nfs->length);
> > @@ -874,11 +875,12 @@ ofputil_decode_flow_stats_reply(struct
> > ofputil_flow_stats *fs,
> >  if (length < sizeof *nfs + ROUND_UP(match_len, 8)) {
> >  VLOG_WARN_RL(, "NXST_FLOW reply with match_len=%"PRIuSIZE"
> > "
> >   "claims invalid length %"PRIuSIZE, match_len,
> > length);
> > -return EINVAL;
> > +return OFPERR_OFPBRC_BAD_LEN;
> >  }
> > -if (nx_pull_match(msg, match_len, >match, NULL, NULL, false,
> > NULL,
> > -  NULL)) {
> > -return EINVAL;
> > +error = nx_pull_match(msg, match_len, >match, NULL, NULL,
> > false,
> > +  NULL, NULL);
> > +if (error) {
> > +return error;
> >  }
> >  instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
> >
> > @@ -907,10 +909,12 @@ ofputil_decode_flow_stats_reply(struct
> > ofputil_flow_stats *fs,
> >  OVS_NOT_REACHED();
> >  }
> >
> > -if (ofpacts_pull_openflow_instructions(msg, instructions_len,
> > oh->version,
> > -   NULL, NULL, ofpacts)) {
> > +error = ofpacts_pull_openflow_instructions(msg, instructions_len,
> > +   oh->version, NULL, NULL,
> > +   ofpacts);
> > +if (error) {
> >  VLOG_WARN_RL(, "OFPST_FLOW reply bad instructions");
> > -return EINVAL;
> > + 

Re: [ovs-dev] [PATCH] ofp-flow: Fix return value for ofputil_decode_flow_stats_reply().

[Yifeng Sun]

Looks good to me. Thanks for the correction.

Reviewed-by: Yifeng Sun 

On Tue, Feb 13, 2018 at 10:49 AM, Ben Pfaff  wrote:

> This function returned errno values for some errors and OFPERR_* values
> for others.  The callers all expected OFPERR_* values.  This fixes the
> problem.
>
> Signed-off-by: Ben Pfaff 
> ---
>  lib/ofp-flow.c | 36 
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
> index 8876b7be5eb1..c29f5b7cb05f 100644
> --- a/lib/ofp-flow.c
> +++ b/lib/ofp-flow.c
> @@ -752,7 +752,7 @@ parse_ofp_flow_stats_request_str(struct
> ofputil_flow_stats_request *fsr,
>   * of it.  'fs->ofpacts' will point into the 'ofpacts' buffer.
>   *
>   * Returns 0 if successful, EOF if no replies were left in this 'msg',
> - * otherwise a positive errno value. */
> + * otherwise an OFPERR_* value. */
>  int
>  ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
>  struct ofpbuf *msg,
> @@ -783,20 +783,21 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
>  if (!ofs) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
>   "bytes at end", msg->size);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
>
>  length = ntohs(ofs->length);
>  if (length < sizeof *ofs) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
>   "length %"PRIuSIZE, length);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
>
> -if (ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
> - _match_len)) {
> +error = ofputil_pull_ofp11_match(msg, NULL, NULL, >match,
> + _match_len);
> +if (error) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply bad match");
> -return EINVAL;
> +return error;
>  }
>  instructions_len = length - sizeof *ofs - padded_match_len;
>
> @@ -833,14 +834,14 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
>  if (!ofs) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply has %"PRIu32" leftover "
>   "bytes at end", msg->size);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
>
>  length = ntohs(ofs->length);
>  if (length < sizeof *ofs) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply claims invalid "
>   "length %"PRIuSIZE, length);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
>  instructions_len = length - sizeof *ofs;
>
> @@ -866,7 +867,7 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
>  if (!nfs) {
>  VLOG_WARN_RL(, "NXST_FLOW reply has %"PRIu32" leftover "
>   "bytes at end", msg->size);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
>
>  length = ntohs(nfs->length);
> @@ -874,11 +875,12 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
>  if (length < sizeof *nfs + ROUND_UP(match_len, 8)) {
>  VLOG_WARN_RL(, "NXST_FLOW reply with match_len=%"PRIuSIZE"
> "
>   "claims invalid length %"PRIuSIZE, match_len,
> length);
> -return EINVAL;
> +return OFPERR_OFPBRC_BAD_LEN;
>  }
> -if (nx_pull_match(msg, match_len, >match, NULL, NULL, false,
> NULL,
> -  NULL)) {
> -return EINVAL;
> +error = nx_pull_match(msg, match_len, >match, NULL, NULL,
> false,
> +  NULL, NULL);
> +if (error) {
> +return error;
>  }
>  instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8);
>
> @@ -907,10 +909,12 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
>  OVS_NOT_REACHED();
>  }
>
> -if (ofpacts_pull_openflow_instructions(msg, instructions_len,
> oh->version,
> -   NULL, NULL, ofpacts)) {
> +error = ofpacts_pull_openflow_instructions(msg, instructions_len,
> +   oh->version, NULL, NULL,
> +   ofpacts);
> +if (error) {
>  VLOG_WARN_RL(, "OFPST_FLOW reply bad instructions");
> -return EINVAL;
> +return error;
>  }
>  fs->ofpacts = ofpacts->data;
>  fs->ofpacts_len = ofpacts->size;
> --
> 2.16.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org