Re: [ovs-dev] [patch v8 9/9] ipf: Add fragmentation status reporting.

[Darrell Ball]

Thanks Justin

By the way, I just noticed that you reviewed most patches on Aug 1.
Since the company e-mail is filtering some OVS list e-mails and I have
relied on
it just for notifications and was otherwise busy, I missed those. I am going
through those carefully now.

One outstanding comment inline that I wanted to double check.


On Fri, Aug 10, 2018 at 8:00 PM, Justin Pettit  wrote:

>
> > On Jul 16, 2018, at 4:39 PM, Darrell Ball  wrote:
> >
> > void
> > ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
> > {
> > diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
> > index f886ab9..2ff7e26 100644
> > --- a/lib/ct-dpif.h
> > +++ b/lib/ct-dpif.h
> > @@ -204,6 +204,15 @@ int ct_dpif_get_nconns(struct dpif *dpif, uint32_t
> *nconns);
> > int ct_dpif_ipf_set_enabled(struct dpif *, bool v6, bool enable);
> > int ct_dpif_ipf_set_min_frag(struct dpif *, bool, uint32_t);
> > int ct_dpif_ipf_set_max_nfrags(struct dpif *, uint32_t);
> > +int ct_dpif_ipf_get_status(struct dpif *dpif, bool *, unsigned int *,
> > +   unsigned int *, unsigned int *, unsigned int
> *,
> > +   unsigned int *, unsigned int *, unsigned int
> *,
> > +   unsigned int *, bool *, unsigned int *,
> > +   unsigned int *, unsigned int *, unsigned int
> *,
> > +   unsigned int *, unsigned int *);
> > +int ct_dpif_ipf_dump_start(struct dpif *dpif, struct ipf_dump_ctx **);
>
> On my system, I needed to declare "ipf_dump_ctx" for the build to finish.
>


I re-ran Travis

Bssically the same code as V8

Here is the link:

Travis:
https://travis-ci.org/darball/ovs_master

The corresponding public git repo branch:
https://github.com/darball/ovs_master/commits/conntrack

Possibility, the build issue you saw is related to your proposed change in
removing #include "ipf.h"
from patch 7.

diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 6eb55b4b0197..f8a31921e16a 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -17,7 +17,6 @@
 #ifndef CT_DPIF_H
 #define CT_DPIF_H

-#include "ipf.h"
 #include "openvswitch/types.h"
 #include "packets.h"

ipf.h has the declaration for ipf_dump_ctx.





>
> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > index 5ec36bd..b3e7ce7 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > ..
> > +static int
> > +dpctl_ct_ipf_get_status(int argc, const char *argv[],
> > +struct dpctl_params *dpctl_p)
> > +{
> > +struct dpif *dpif;
> > +int error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif);
> > +if (!error) {
> > +bool ipf_v4_enabled;
> > +unsigned int min_v4_frag_size;
> > +unsigned int nfrag_max;
> > +unsigned int nfrag;
> > +unsigned int n4frag_accepted;
> > +unsigned int n4frag_completed_sent;
> > +unsigned int n4frag_expired_sent;
> > +unsigned int n4frag_too_small;
> > +unsigned int n4frag_overlap;
> > +unsigned int min_v6_frag_size;
> > +bool ipf_v6_enabled;
> > +unsigned int n6frag_accepted;
> > +unsigned int n6frag_completed_sent;
> > +unsigned int n6frag_expired_sent;
> > +unsigned int n6frag_too_small;
> > +unsigned int n6frag_overlap;
> > +error = ct_dpif_ipf_get_status(dpif, &ipf_v4_enabled,
> > +&min_v4_frag_size, &nfrag_max, &nfrag, &n4frag_accepted,
> > +&n4frag_completed_sent, &n4frag_expired_sent,
> &n4frag_too_small,
> > +&n4frag_overlap, &ipf_v6_enabled, &min_v6_frag_size,
> > +&n6frag_accepted, &n6frag_completed_sent,
> &n6frag_expired_sent,
> > +&n6frag_too_small, &n6frag_overlap);
> > +
>
> If we just use 'ipf_status', we can delete a lot of these variable
> declarations.
>
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 76bc1d9..db551ea 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -6553,6 +6553,60 @@ dpif_netdev_ipf_set_max_nfrags(struct dpif *dpif
> OVS_UNUSED,
> > return ipf_set_max_nfrags(max_frags);
> > }
> >
> > +static int
> > +dpif_netdev_ipf_get_status(struct dpif *dpif OVS_UNUSED,
> > +bool *ipf_v4_enabled, unsigned int *min_v4_frag_size,
> > +unsigned int *nfrag_max, unsigned int *nfrag,
> > +unsigned int *n4frag_accepted, unsigned int *n4frag_completed_sent,
> > +unsigned int *n4frag_expired_sent, unsigned int *n4frag_too_small,
> > +unsigned int *n4frag_overlap, bool *ipf_v6_enabled,
> > +unsigned int *min_v6_frag_size, unsigned int *n6frag_accepted,
> > +unsigned int *n6frag_completed_sent, unsigned int
> *n6frag_expired_sent,
> > +unsigned int *n6frag_too_small, unsigned int *n6frag_overlap)
> > +{
> > +struct ipf_status ipf_status;
> > +ipf_get_status(&ipf_status);
> > +*ipf_v4_enabled = ipf_status.ifp_v4_enabled;
> > +*min_v4_frag_size = ipf_status.min_v4_frag_size;
> > +*nfrag_max = ipf_status.nfrag_max;
> > +*nfrag = ipf_status.nfrag;
> > +*n4frag_acce

Re: [ovs-dev] [PATCH] ovsdb-idl: Adjust indexes during transactions.

[Han Zhou]

On Wed, Aug 15, 2018 at 8:53 PM, Han Zhou  wrote:
>
>
>
> On Tue, Aug 14, 2018 at 12:39 PM, Han Zhou  wrote:
> >
> >
> >
> > On Tue, Aug 14, 2018 at 11:31 AM, Ben Pfaff  wrote:
> > >
> > > When transactions modified tables with indexes, the indexes were not
> > > properly updated to reflect the changes.  For deleted rows, in
particular,
> > > this could cause use-after-free errors.
> > >
> > > This commit fixes the problem and adds a very simple test case
provided by
> > > Han Zhou that, without the fix, causes a crash.
> > >
> > > Reported-by: Han Zhou 
> > > Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047185.html
> > > Signed-off-by: Ben Pfaff 
> > > ---
> > >  lib/ovsdb-idl.c| 25 +++--
> > >  tests/test-ovsdb.c |  1 +
> > >  2 files changed, 24 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > > index 8fdd18f4688e..a4d66113b18d 100644
> > > --- a/lib/ovsdb-idl.c
> > > +++ b/lib/ovsdb-idl.c
> > > @@ -3508,9 +3508,18 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn
*txn)
> > >  txn->db->txn = NULL;
> > >
> > >  HMAP_FOR_EACH_SAFE (row, next, txn_node, &txn->txn_rows) {
> > > +enum { INSERTED, MODIFIED, DELETED } op
> > > += (!row->new_datum ? DELETED
> > > +   : !row->old_datum ? INSERTED
> > > +   : MODIFIED);
> > > +
> > > +if (op != DELETED) {
> > > +ovsdb_idl_remove_from_indexes(row);
> > > +}
> > > +
> > >  ovsdb_idl_destroy_all_map_op_lists(row);
> > >  ovsdb_idl_destroy_all_set_op_lists(row);
> > > -if (row->old_datum) {
> > > +if (op != INSERTED) {
> > >  if (row->written) {
> > >  ovsdb_idl_row_unparse(row);
> > >  ovsdb_idl_row_clear_arcs(row, false);
> > > @@ -3529,7 +3538,9 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn
*txn)
> > >
> > >  hmap_remove(&txn->txn_rows, &row->txn_node);
> > >  hmap_node_nullify(&row->txn_node);
> > > -if (!row->old_datum) {
> > > +if (op != INSERTED) {
> > > +ovsdb_idl_add_to_indexes(row);
> > > +} else {
> > >  hmap_remove(&row->table->rows, &row->hmap_node);
> > >  free(row);
> > >  }
> > > @@ -4209,6 +4220,10 @@ ovsdb_idl_txn_write__(const struct
ovsdb_idl_row *row_,
> > >  goto discard_datum;
> > >  }
> > >
> > > +bool index_row = is_index_row(row);
> > > +if (!index_row) {
> > > +ovsdb_idl_remove_from_indexes(row);
> > > +}
> > >  if (hmap_node_is_null(&row->txn_node)) {
> > >  hmap_insert(&row->table->db->txn->txn_rows, &row->txn_node,
> > >  uuid_hash(&row->uuid));
> > > @@ -4231,6 +4246,9 @@ ovsdb_idl_txn_write__(const struct
ovsdb_idl_row *row_,
> > >  }
> > >  (column->unparse)(row);
> > >  (column->parse)(row, &row->new_datum[column_idx]);
> > > +if (!index_row) {
> > > +ovsdb_idl_add_to_indexes(row);
> > > +}
> > >  return;
> > >
> > >  discard_datum:
> > > @@ -4358,6 +4376,8 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row
*row_)
> > >  }
> > >
> > >  ovs_assert(row->new_datum != NULL);
> > > +ovs_assert(!is_index_row(row_));
> > > +ovsdb_idl_remove_from_indexes(row_);
> > >  if (!row->old_datum) {
> > >  ovsdb_idl_row_unparse(row);
> > >  ovsdb_idl_row_clear_new(row);
> > > @@ -4405,6 +4425,7 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn,
> > >  row->new_datum = xmalloc(class->n_columns * sizeof
*row->new_datum);
> > >  hmap_insert(&row->table->rows, &row->hmap_node,
uuid_hash(&row->uuid));
> > >  hmap_insert(&txn->txn_rows, &row->txn_node,
uuid_hash(&row->uuid));
> > > +ovsdb_idl_add_to_indexes(row);
> > >  return row;
> > >  }
> > >
> > > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> > > index 793220400c3a..e981b588849d 100644
> > > --- a/tests/test-ovsdb.c
> > > +++ b/tests/test-ovsdb.c
> > > @@ -2882,6 +2882,7 @@ test_idl_compound_index_single_column(struct
ovsdb_idl *idl,
> > >  ovs_assert(myRow->i == 4);
> > >  txn = ovsdb_idl_txn_create(idl);
> > >  idltest_simple_delete(myRow);
> > > +myRow = idltest_simple_index_find(i_index, toDelete);
> > >  toInsert = idltest_simple_insert(txn);
> > >  idltest_simple_set_i(toInsert, 54);
> > >  idltest_simple_set_s(toInsert, "Lista054");
> > > --
> > > 2.16.1
> > >
> >
> > Thanks Ben for the quick fix!! It may be better if we add a little more
tests to ensure change is reflected in indexes before transaction commit.
> >
> > Acked-by: Han Zhou 
>
> Hi Ben, I added below tests on top of your patch and it works as
expected. So consider it as my Tested-by, too.
>
> 8><--><8-
> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> index 459b4ee..4004cab 100644
> --- a/tests/test-ovsdb.c
> +++ b/tests/test-ovsdb.c
> @@ -288

Re: [ovs-dev] [PATCH] ovsdb-idl: Adjust indexes during transactions.

[Han Zhou]

On Tue, Aug 14, 2018 at 12:39 PM, Han Zhou  wrote:
>
>
>
> On Tue, Aug 14, 2018 at 11:31 AM, Ben Pfaff  wrote:
> >
> > When transactions modified tables with indexes, the indexes were not
> > properly updated to reflect the changes.  For deleted rows, in
particular,
> > this could cause use-after-free errors.
> >
> > This commit fixes the problem and adds a very simple test case provided
by
> > Han Zhou that, without the fix, causes a crash.
> >
> > Reported-by: Han Zhou 
> > Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047185.html
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/ovsdb-idl.c| 25 +++--
> >  tests/test-ovsdb.c |  1 +
> >  2 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > index 8fdd18f4688e..a4d66113b18d 100644
> > --- a/lib/ovsdb-idl.c
> > +++ b/lib/ovsdb-idl.c
> > @@ -3508,9 +3508,18 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn
*txn)
> >  txn->db->txn = NULL;
> >
> >  HMAP_FOR_EACH_SAFE (row, next, txn_node, &txn->txn_rows) {
> > +enum { INSERTED, MODIFIED, DELETED } op
> > += (!row->new_datum ? DELETED
> > +   : !row->old_datum ? INSERTED
> > +   : MODIFIED);
> > +
> > +if (op != DELETED) {
> > +ovsdb_idl_remove_from_indexes(row);
> > +}
> > +
> >  ovsdb_idl_destroy_all_map_op_lists(row);
> >  ovsdb_idl_destroy_all_set_op_lists(row);
> > -if (row->old_datum) {
> > +if (op != INSERTED) {
> >  if (row->written) {
> >  ovsdb_idl_row_unparse(row);
> >  ovsdb_idl_row_clear_arcs(row, false);
> > @@ -3529,7 +3538,9 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn
*txn)
> >
> >  hmap_remove(&txn->txn_rows, &row->txn_node);
> >  hmap_node_nullify(&row->txn_node);
> > -if (!row->old_datum) {
> > +if (op != INSERTED) {
> > +ovsdb_idl_add_to_indexes(row);
> > +} else {
> >  hmap_remove(&row->table->rows, &row->hmap_node);
> >  free(row);
> >  }
> > @@ -4209,6 +4220,10 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row
*row_,
> >  goto discard_datum;
> >  }
> >
> > +bool index_row = is_index_row(row);
> > +if (!index_row) {
> > +ovsdb_idl_remove_from_indexes(row);
> > +}
> >  if (hmap_node_is_null(&row->txn_node)) {
> >  hmap_insert(&row->table->db->txn->txn_rows, &row->txn_node,
> >  uuid_hash(&row->uuid));
> > @@ -4231,6 +4246,9 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row
*row_,
> >  }
> >  (column->unparse)(row);
> >  (column->parse)(row, &row->new_datum[column_idx]);
> > +if (!index_row) {
> > +ovsdb_idl_add_to_indexes(row);
> > +}
> >  return;
> >
> >  discard_datum:
> > @@ -4358,6 +4376,8 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row
*row_)
> >  }
> >
> >  ovs_assert(row->new_datum != NULL);
> > +ovs_assert(!is_index_row(row_));
> > +ovsdb_idl_remove_from_indexes(row_);
> >  if (!row->old_datum) {
> >  ovsdb_idl_row_unparse(row);
> >  ovsdb_idl_row_clear_new(row);
> > @@ -4405,6 +4425,7 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn,
> >  row->new_datum = xmalloc(class->n_columns * sizeof
*row->new_datum);
> >  hmap_insert(&row->table->rows, &row->hmap_node,
uuid_hash(&row->uuid));
> >  hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid));
> > +ovsdb_idl_add_to_indexes(row);
> >  return row;
> >  }
> >
> > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> > index 793220400c3a..e981b588849d 100644
> > --- a/tests/test-ovsdb.c
> > +++ b/tests/test-ovsdb.c
> > @@ -2882,6 +2882,7 @@ test_idl_compound_index_single_column(struct
ovsdb_idl *idl,
> >  ovs_assert(myRow->i == 4);
> >  txn = ovsdb_idl_txn_create(idl);
> >  idltest_simple_delete(myRow);
> > +myRow = idltest_simple_index_find(i_index, toDelete);
> >  toInsert = idltest_simple_insert(txn);
> >  idltest_simple_set_i(toInsert, 54);
> >  idltest_simple_set_s(toInsert, "Lista054");
> > --
> > 2.16.1
> >
>
> Thanks Ben for the quick fix!! It may be better if we add a little more
tests to ensure change is reflected in indexes before transaction commit.
>
> Acked-by: Han Zhou 

Hi Ben, I added below tests on top of your patch and it works as expected.
So consider it as my Tested-by, too.

8><--><8-
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 459b4ee..4004cab 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2881,9 +2881,16 @@ test_idl_compound_index_single_column(struct
ovsdb_idl *idl,
 txn = ovsdb_idl_txn_create(idl);
 idltest_simple_delete(myRow);
 myRow = idltest_simple_index_find(i_index, toDelete);
-toInsert = idltest_simple_insert(txn);
-idltest_simple_set_i(toInsert, 54);
-   

Re: [ovs-dev] [PATCH] ofp-ed-props: Fix hang for crafted OpenFlow encap/decap properties.

[Darrell Ball]

On Wed, Aug 15, 2018 at 3:03 PM, Ben Pfaff  wrote:

> decode_ed_prop() accepted encap/decap properties with a reported length of
> 0, without consuming any data from the property list, which yielded an
> infinite loop.
>
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9918
> Signed-off-by: Ben Pfaff 
> ---
>  lib/ofp-ed-props.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ofp-ed-props.c b/lib/ofp-ed-props.c
> index 901da2f0dd1b..28382e01235c 100644
> --- a/lib/ofp-ed-props.c
> +++ b/lib/ofp-ed-props.c
> @@ -35,7 +35,7 @@ decode_ed_prop(const struct ofp_ed_prop_header
> **ofp_prop,

 size_t len = (*ofp_prop)->len;
>  size_t pad_len = ROUND_UP(len, 8);
>
> -if (pad_len > *remaining) {
> +if (len < sizeof **ofp_prop || pad_len > *remaining) {
>

Is *remaining > pad_len valid ?
If it is, which is not intuitive, maybe a comment will help ?



>  return OFPERR_OFPBAC_BAD_LEN;
>  }
>
> --
> 2.16.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-ed-props: Fix hang for crafted OpenFlow encap/decap properties.

[Ben Pfaff]

decode_ed_prop() accepted encap/decap properties with a reported length of
0, without consuming any data from the property list, which yielded an
infinite loop.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9918
Signed-off-by: Ben Pfaff 
---
 lib/ofp-ed-props.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ofp-ed-props.c b/lib/ofp-ed-props.c
index 901da2f0dd1b..28382e01235c 100644
--- a/lib/ofp-ed-props.c
+++ b/lib/ofp-ed-props.c
@@ -35,7 +35,7 @@ decode_ed_prop(const struct ofp_ed_prop_header **ofp_prop,
 size_t len = (*ofp_prop)->len;
 size_t pad_len = ROUND_UP(len, 8);
 
-if (pad_len > *remaining) {
+if (len < sizeof **ofp_prop || pad_len > *remaining) {
 return OFPERR_OFPBAC_BAD_LEN;
 }
 
-- 
2.16.1

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

[ovs-dev] [PATCH] ofp-actions: Avoid assertion failure for clone(ct(...bad actions...)).

[Ben Pfaff]

decode_NXAST_RAW_CT() temporarily pulls data off the beginning of its
ofpacts output ofpbuf and, on its error path, fails to push it back on.
At a higher layer, decode_NXAST_RAW_CLONE() asserts, via
ofpact_finish_CLONE(), that the ofpact_clone that it put is still in the
place where it put it, which causes an assertion failure.

The root cause here is the failure to re-push the clone header.  One could
fix that, but it would be pretty easy for that to go wrong again on some
other obscure error path.  Instead, this commit just makes the problem go
away by always saving and restoring 'ofpact->data' if a decode fails.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9862
Signed-off-by: Ben Pfaff 
---
 lib/ofp-actions.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 93e212f07196..6adb55d23b02 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -7500,6 +7500,7 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
 uint64_t *ofpacts_tlv_bitmap)
 {
 const struct ofp_action_header *actions;
+void *orig_data = ofpacts->data;
 size_t orig_size = ofpacts->size;
 enum ofperr error;
 
@@ -7519,14 +7520,17 @@ ofpacts_pull_openflow_actions__(struct ofpbuf *openflow,
 
 error = ofpacts_decode(actions, actions_len, version, vl_mff_map,
ofpacts_tlv_bitmap, ofpacts);
-if (error) {
-ofpacts->size = orig_size;
-return error;
+if (!error) {
+error = ofpacts_verify(ofpacts->data, ofpacts->size, allowed_ovsinsts,
+   outer_action);
 }
-
-error = ofpacts_verify(ofpacts->data, ofpacts->size, allowed_ovsinsts,
-   outer_action);
 if (error) {
+/* Back out changes to 'ofpacts'.  (Normally, decoding would only
+ * append to 'ofpacts', so that one would expect only to need to
+ * restore 'ofpacts->size', but some action parsing temporarily pulls
+ * off data from the start of 'ofpacts' and may not properly re-push it
+ * on error paths.) */
+ofpacts->data = orig_data;
 ofpacts->size = orig_size;
 }
 return error;
-- 
2.16.1

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

Re: [ovs-dev] [PATCH 10/11] dpctl: Implement dpctl commands for conntrack per zone limit

[Darrell Ball]

sorry, totally missed this e-mail

On Thu, Aug 2, 2018 at 2:35 PM, Yi-Hung Wei  wrote:

> On Wed, Aug 1, 2018 at 10:37 PM, Darrell Ball  wrote:
> > On Wed, Aug 1, 2018 at 3:46 PM, Yi-Hung Wei 
> wrote:
> >
> > Do these commands need a ‘zone’ keyword
> >
> > eg) ct-set-zone-limits
> >
> >>
> >> $ ovs-appctl dpctl/ct-set-limits default=10 zone=0,limit=5
> zone=1,limit=3
>
> I think I am fine either way. I can change ct-set-zone-limits to
> ct-set-limits in later version.
>
> >
> > I wonder if it makes sense to write
> >
> > ‘zone=default,limit=10’
> >
> > so that ‘default’ is treated like any zone?
> That makes sense, and I think it does simplify the syntax.  Will make
> this change in v3.
>


I can add a quick incremental later as well.



>
> >>
> >> $ ovs-appctl dpct/ct-del-limits zone=0
> >
> >
> >
> > I wonder if we set zone limit to zero (unlimited), it could be like
> deleting
> > a zone limit.
> >
> > $ ovs-appctl dpctl/ct-set-limits zone=1,limit=0
>
> The default limit may change from time to time, it is not necessarily
> to be zero (unlimited).
>


I understand; more generally:

If these special values were strings at the dpctl level then it would be
more obvious
what I meant.

i.e.
ovs-appctl dpctl/ct-zone-set-limits zone=1,limit=unlimited
ovs-appctl dpctl/ct-zone-set-limits zone=1,limit=default  (i.e. "delete")
ovs-appctl dpctl/ct-zone-set-limits zone=1,limit=4

The internal values could be
unlimited: 0
default: -1
or whatever.



>
> On the other hand, we may set limit to 'default', such as having
> commands like $ ovs-appctl dpctl/ct-set-limits zone=1,limit=default
> zone=2,limit=default zone=3,limit=default
>


But I understand your time constraint, so don't worry about it for your
patchset.



>
> But it  seems to be easier to have syntax like $ ovs-appctl
> dpctl/ct-del-lmits zone=1,2,3
>
> Therefore, I prefer to keep the ct-del-limits command as is.
>







>
> Thanks,
>
> -Yi-Hung
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v8] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

On 8/15/18, 1:52 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Fri, Aug 10, 2018 at 08:56:17AM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers and avoids trying to open potentially bogus
> datapaths provided by the user. Also, the error handling is improved by
> reducing bogus errors and having more specific real errors.
> 
> Signed-off-by: Darrell Ball 

Thanks.  I folded in the following simplifications and applied this to
master.



diff --git a/lib/dpctl.c b/lib/dpctl.c
index 594497473c49..c228dc10c2e3 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -195,13 +195,10 @@ dp_exists(const char *queried_dp)
 struct sset dpif_names = SSET_INITIALIZER(&dpif_names),
 dpif_types = SSET_INITIALIZER(&dpif_types);
 dp_enumerate_types(&dpif_types);
-bool found = false;
 
-if (sset_contains(&dpif_types, queried_type) &&
-!dp_enumerate_names(queried_type, &dpif_names) &&
-sset_contains(&dpif_names, queried_name)) {
-found = true;
-}
+bool found = (sset_contains(&dpif_types, queried_type) &&
+  !dp_enumerate_names(queried_type, &dpif_names) &&
+  sset_contains(&dpif_names, queried_name));
 
 sset_destroy(&dpif_names);
 sset_destroy(&dpif_types);
@@ -211,10 +208,7 @@ dp_exists(const char *queried_dp)
 static bool
 dp_arg_exists(int argc, const char *argv[])
 {
-if (argc > 1 && dp_exists(argv[1])) {
-return true;
-}
-return false;
+return argc > 1 && dp_exists(argv[1]);
 }


cool; thanks Ben
 
 /* Open a dpif with an optional name argument.
___
dev mailing list
d...@openvswitch.org

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Cdball%40vmware.com%7C9cb84e244708465e9ad608d602f10d3f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636699631682655681&sdata=c%2F1x1O6rnqbNIU3HjIAoeyBtlyeegw4GfRVkYonJ7Vs%3D&reserved=0


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

Re: [ovs-dev] [patch v8] dpctl: Make opt_dpif_open() more general.

[Ben Pfaff]

On Fri, Aug 10, 2018 at 08:56:17AM -0700, Darrell Ball wrote:
> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers and avoids trying to open potentially bogus
> datapaths provided by the user. Also, the error handling is improved by
> reducing bogus errors and having more specific real errors.
> 
> Signed-off-by: Darrell Ball 

Thanks.  I folded in the following simplifications and applied this to
master.



diff --git a/lib/dpctl.c b/lib/dpctl.c
index 594497473c49..c228dc10c2e3 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -195,13 +195,10 @@ dp_exists(const char *queried_dp)
 struct sset dpif_names = SSET_INITIALIZER(&dpif_names),
 dpif_types = SSET_INITIALIZER(&dpif_types);
 dp_enumerate_types(&dpif_types);
-bool found = false;
 
-if (sset_contains(&dpif_types, queried_type) &&
-!dp_enumerate_names(queried_type, &dpif_names) &&
-sset_contains(&dpif_names, queried_name)) {
-found = true;
-}
+bool found = (sset_contains(&dpif_types, queried_type) &&
+  !dp_enumerate_names(queried_type, &dpif_names) &&
+  sset_contains(&dpif_names, queried_name));
 
 sset_destroy(&dpif_names);
 sset_destroy(&dpif_types);
@@ -211,10 +208,7 @@ dp_exists(const char *queried_dp)
 static bool
 dp_arg_exists(int argc, const char *argv[])
 {
-if (argc > 1 && dp_exists(argv[1])) {
-return true;
-}
-return false;
+return argc > 1 && dp_exists(argv[1]);
 }
 
 /* Open a dpif with an optional name argument.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] netdev: Retry getting interfaces on inconsistent dumps from kernel

[Ben Pfaff]

On Mon, Aug 13, 2018 at 05:39:07PM -0700, Ben Pfaff wrote:
> On Mon, Aug 13, 2018 at 02:07:45PM +0200, Daniel Alvarez wrote:
> > This patch in glibc [0] is fixing a bug where we may be getting
> > inconsistent dumps from the kernel when listing interfaces due to
> > a race condition.
> > 
> > This could happen if we try to retrieve them while interfaces are
> > being added/removed from the system at the same time.
> > For systems running against old glibc versions, this patch is retrying
> > the operation up to 3 times and then proceeding by logging a
> > warning.
> > 
> > Note that 3 times should be enough to not delay the operation much
> > and since it's unlikely that we hit the race condition 3 times in
> > a row. Still, if this happened, this patch is not changing the
> > current behavior.
> > 
> > [0] 
> > https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c1f86a33ca32e26a9d6e29fc961e5ecb5e2e5eb4
> > 
> > Signed-off-by: Daniel Alvarez 
> > Co-authored-by: Jiri Benc 
> 
> Thanks for the patch.
> 
> As a co-author, Jiri also needs to sign off.
> 
> Acked-by: Ben Pfaff 

I see that Jiri signed off earlier and it just didn't get propagated.

Thanks, I applied this to master and branch-2.10.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ip_tunnel: Fix bugs that could crash kernel

[Ben Pfaff]

On Fri, Jul 20, 2018 at 04:13:14PM -0700, William Tu wrote:
> On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun  wrote:
> > Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> > one more time than necessary, which causes kernel crash.
> >
> > On kernel 4.4.0-116-generic, the crash can be reproduced by running
> > the simple test provided below through check-kernel.
> >
> >   make & make modules_install
> >   rmmod ip_gre gre ip_tunnel
> >   modprobe openvswitch
> >   make check-kernel TESTSUITEFLAGS=x
> >   dmesg
> >
> > Simple test:
> >
> > AT_SETUP([datapath - crash test])
> > OVS_CHECK_GRE()
> > ip link del gre0
> > OVS_TRAFFIC_VSWITCHD_START()
> > AT_CHECK([ovs-vsctl -- set bridge br0])
> > ADD_BR([br-underlay], [set bridge br-underlay])
> > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > ADD_NAMESPACES(at_ns0)
> > ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > AT_CHECK([ip link set dev br-underlay up])
> > ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> > tcpdump -U -i br-underlay -w underlay.pcap &
> > sleep 1
> > OVS_TRAFFIC_VSWITCHD_STOP
> > AT_CLEANUP
> >
> > Signed-off-by: Yifeng Sun 
> > ---
> 
> LGTM, Thanks for the fix.
> Acked-by: William Tu 

Thanks Yifeng (and William).

I applied this to master and branch-2.10.  If it should be backported
farther, please let me know.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ben Pfaff]

On Wed, Aug 15, 2018 at 11:17:21AM -0700, Ben Pfaff wrote:
> On Wed, Aug 15, 2018 at 01:49:45PM +0100, Ian Stokes wrote:
> > On 8/14/2018 10:19 PM, Ben Pfaff wrote:
> > >On Tue, Aug 14, 2018 at 04:42:06PM +, Stokes, Ian wrote:
> > >>Recently at the OVS DPDK community meeting the case for 2 new branches 
> > >>was raised.
> > >>
> > >>https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350898.html
> > >>
> > >>These branches would be:
> > >>
> > >>(i) OVS DPDK Latest: This branch would essentially be OVS master using 
> > >>the latest DPDK release (Including non LTS releases).
> > >>
> > >>The purpose of this branch would be to allow OVS DPDK developers to 
> > >>assess the latest DPDK releases with OVS and provide feedback to the DPDK 
> > >>community if changes are required. Currently OVS transitions between 
> > >>supported DPDK releases using DPDK LTS releases only. DPDK LTS releases 
> > >>happen annually. The next DPDK LTS release would be 18.11. However the 
> > >>other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs 
> > >>that impact OVS DPDK (Such as the HWOL). This feedback would be in place 
> > >>for the next LTS release before OVS transitions to the next x.11 LTS.
> > >>
> > >>(ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but 
> > >>would encompass the HWOL development work that is ongoing.
> > >>
> > >>The feeling as regards the need for a OVS DPDK HWOL branch is that it 
> > >>requires new features only available in the latest DPDK releases and that 
> > >>there will be a lot of code rework required as its validated with various 
> > >>HW devices over time before an acceptable solution will be in place.
> > >>
> > >>There was a question as regards the logistics of where the branches 
> > >>should reside. It was suggested that they could be part of the OVS Repo 
> > >>to centralize the development work but that is obviously something that 
> > >>would have to be raised with yourself and the other project maintainers.
> > >>
> > >>An alternative would be that it would be hosted on a developers GitHub 
> > >>repo similar to how the dpdk_merge branches currently work.
> > >>
> > >>Neither of the branches would be subject to releases as the end goal of 
> > >>the development work carried out on them would make its way into OVS 
> > >>Master eventually.
> > >
> > >This seems reasonable, as long as it doesn't overburden developers with
> > >branches.
> > 
> > I agree, and I think the messaging as regards its purpose of usage should be
> > clear, it's intended as a tool to aid development of features for intended
> > for OVS master in the future such as DPDK releases or . Developers could
> > ignore these branches if the features they develop are already available in
> > the existing supported DPDK.
> > 
> > >
> > >How do you prefer to host it?  Any of the following would be fine with
> > >me, in descending order of preference:
> > >
> > >* Host in openvswitch github repo forked from main openvswitch/ovs repo.
> > 
> > I think the option above would be good, better than being provisioned on a
> > developers own github repo. At least when documenting its purpose it would
> > look a little more official rather than pointing to a personal github repo.
> > 
> > In terms of commit access I take it the default ovs committers would have
> > access here as well (I take it they would be the people to create the repo).
> > In terms of merging would this follow the pull_request process to yourself?
> 
> I'd think it would make sense to give you the ability to push directly
> to this new repo.  It's only merging into release branches that the OVS
> contribution policy is supposed to gate.

I talked to Justin about this.  He pointed out that if we create a repo
with "dpdk" in the name, it's likely to confuse people who are casually
looking for OVS with DPDK support.  It still makes sense to do this in
some kind of "official" place, though, so now I'm willing to support the
idea of just putting the branches into the main OVS repo.  Let me check
with the committers list, first.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix for flow limit issue in revalidator

[Ben Pfaff]

On Fri, Aug 03, 2018 at 06:04:23AM +0530, Vishal Deep Ajmera wrote:
> When the revalidator thread takes a long time to dump data path
> flows (e.g. due to busy CPU), it reduces the maximum limit for
> new flows that can be added. This results in more upcalls for
> packets which do not find data path flows and temporarily reduces
> overall throughput. When the situation improves and the revalidator
> gets enough CPU cycles, it should increase the flow limit allowing
> more flows to get inserted.
> 
> Currently the flow limit does not increase if the existing number of
> flows is less than 2000 and does not allow any new flows due to
> incorrect condition check. This results in a permanent drop in
> performance in OVS with no automatic recovery.
> 
> This patch fixes the conditional check for increasing flow limit.
> 
> Signed-off-by: Vishal Deep Ajmera 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/3] tests: Environment variable for default timeout.

[Ben Pfaff]

On Tue, Aug 14, 2018 at 10:53:14AM +0300, Ilya Maximets wrote:
> This was inspired by my previous patch set about syslog [1] and
> the new environment variable for syslog method [2].
> 
> Patch set introduces new env. variable 'OVS_CTL_TIMEOUT', which
> is used as a default timeout for control utilities. Using it we
> can avoid shell tricks with aliases. It will work in any shell
> and even in a subshell.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350813.html
> [2] https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350872.html

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

[ovs-dev] [PATCH v2 4/4] system-traffic: Add 5 new tunnel tests that don't depend on native linux modules

[Yifeng Sun]

Introduce 5 new tests that don't require native gre or erspan tunnels but sends
simulated raw packets.

These tests are supposed to only run for kernel version from 3.10.x to 4.15.x
where compatible gre is being used by OVS kernel module.

Signed-off-by: Yifeng Sun 
---
v1->v2: Add support for kernel 3.10
Improve tcpdump speed, thanks Darrell!

 tests/system-traffic.at | 264 
 1 file changed, 264 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 7d236b8..da5d29d 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -575,6 +575,270 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 
10.1.1.100 | FORMAT_PI
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over gre tunnel by simulated packets])
+OVS_CHECK_KERNEL(3, 10, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace.
+ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
+
+dnl Certain Linux distributions, like CentOS, have default iptable rules
+dnl to reject input traffic from br-underlay. Here we add a rule to walk
+dnl around it.
+iptables -I INPUT 1 -i br-underlay -j ACCEPT
+on_exit 'iptables -D INPUT 1'
+
+ip netns exec at_ns0 tcpdump -n -i p0 dst host 172.31.1.1 -l > p0.pcap &
+sleep 1
+
+dnl First, check the underlay.
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl We don't actually add gretap port as below, instead, we will
+dnl emulate one that sends packets. Suppose its mac address is 
f2:ff:00:00:00:04.
+dnl ADD_NATIVE_TUNNEL([gretap], [ns_gre0], [at_ns0], [172.31.1.100], 
[10.1.1.1/24])
+
+dnl Now, check the overlay by sending out raw arp and icmp packets.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000308004542ec2c4000402ff3bcac1f0101ac1f01646558f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=NORMAL"
+
+OVS_WAIT_UNTIL([cat p0.pcap | egrep "IP 172.31.1.100 > 172.31.1.1: GREv0, 
length 46: ARP, Reply 10.1.1.100 is-at f2:ff:00:00:00:01.* length 28" 2>&1 
1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff00030800457aec8e4000402ff322ac1f0101ac1f01646558f2ff0001f2ff000408004554548f40004001cfb30a0101010a0101640800e6e829270003e1a3435bff1a0500101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
 actions=NORMAL"
+
+OVS_WAIT_UNTIL([cat p0.pcap | egrep "IP 172.31.1.100 > 172.31.1.1: GREv0, 
length 102: IP 10.1.1.100 > 10.1.1.1: ICMP echo reply,.* length 64$" 2>&1 
1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([datapath - ping over erspan v1 tunnel by simulated packets])
+OVS_CHECK_KERNEL(3, 10, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and emulate a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [10.1.1.100/24], 
[options:key=1 options:erspan_ver=1 options:erspan_idx=7])
+
+dnl Certain Linux distributions, like CentOS, have default iptable rules
+dnl to reject input traffic from br-underlay. Here we add a rule to walk
+dnl around it.
+iptables -I INPUT 1 -i br-underlay -j ACCEPT
+on_exit 'iptables -D INPUT 1'
+
+ip netns exec at_ns0 tcpdump -n -x -i p0 dst host 172.31.1.1 -l > p0.pcap &
+sleep 1
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now send out an arp request from 10.1.1.1 for 10.1.1.100 in erspan.
+ovs-ofctl -O OpenFlow13 packe

[ovs-dev] [PATCH v2 3/4] system-traffic: Skip 5 tunnel tests on certain kernel versions

[Yifeng Sun]

Skip gre, erspan and ip6erspan related tests on kernel version from 3.10.x
to 4.15.x because compatable gre is used and these tests will always fail.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..7d236b8 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -300,6 +300,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over gre tunnel])
+OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -340,6 +341,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v1 tunnel])
+OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -375,6 +377,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v2 tunnel])
+OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -410,6 +413,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v1 tunnel])
+OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -448,6 +452,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v2 tunnel])
+OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
-- 
2.7.4

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

[ovs-dev] [PATCH v2 2/4] test: Add two m4 functions to skip tests for certain kernel versions

[Yifeng Sun]

Some tests depend on native Linux gre modules to setup testing environments.
However, some kernel versions require OVS to use compatable gre modules. In
this case, these tests always fail.

This patch helps to skip a test if it fails due to this reason. The new m4
functions will be used by later patches.

Signed-off-by: Yifeng Sun 
---
v1->v2: Let tests skip in check-system-userspace when they always fail.

 tests/system-kmod-macros.at  | 22 ++
 tests/system-userspace-macros.at | 16 
 2 files changed, 38 insertions(+)

diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index 61911ac..770703b 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -132,3 +132,25 @@ m4_define([CHECK_CT_DPIF_GET_NCONNS],
 [
 AT_SKIP_IF([:])
 ])
+
+# OVS_CHECK_KERNEL([minversion], [minsublevel], [maxversion], [maxsublevel])
+#
+# Check if kernel version falls between minversion.minsublevel and
+# maxversion.maxsublevel, skip this test if it is not.
+m4_define([OVS_CHECK_KERNEL],
+[
+version=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 1}')
+sublevel=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 2}')
+AT_SKIP_IF([test $version -lt $1 || ( test $version -eq $1 && test 
$sublevel -lt $2 ) || test $version -gt $3 || ( test $version -eq $3 && test 
$sublevel -gt $4 )])
+])
+
+# OVS_CHECK_KERNEL_EXCL([minversion], [minsublevel], [maxversion], 
[maxsublevel])
+#
+# Check that kernel version doesn't fall between minversion.minsublevel and
+# maxversion.maxsublevel, skip this test if it is.
+m4_define([OVS_CHECK_KERNEL_EXCL],
+[
+version=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 1}')
+sublevel=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 2}')
+AT_SKIP_IF([ ! ( test $version -lt $1 || ( test $version -eq $1 && test 
$sublevel -lt $2 ) || test $version -gt $3 || ( test $version -eq $3 && test 
$sublevel -gt $4 ) ) ])
+])
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index fb7b09c..968a95e 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -127,3 +127,19 @@ m4_define([CHECK_CT_DPIF_SET_GET_MAXCONNS])
 # Perform requirements checks for running ovs-dpctl ct-get-nconns. The
 # userspace datapath does support this feature.
 m4_define([CHECK_CT_DPIF_GET_NCONNS])
+
+# OVS_CHECK_KERNEL([minversion], [maxversion], [minsublevel], [maxsublevel])
+#
+# The userspace skips all tests that check kernel version.
+m4_define([OVS_CHECK_KERNEL],
+[
+AT_SKIP_IF([:])
+])
+
+# OVS_CHECK_KERNEL_EXCL([minversion], [maxversion], [minsublevel], 
[maxsublevel])
+#
+# The userspace skips all tests that check kernel version.
+m4_define([OVS_CHECK_KERNEL_EXCL],
+[
+AT_SKIP_IF([:])
+])
-- 
2.7.4

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

[ovs-dev] [PATCH v2 1/4] ip6_gre: Fix a bug that clears address bits

[Yifeng Sun]

In compatable gre module, skb->cb is solely used as ovs_gso_cb.
However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps
with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit
in the address of ovs_gso_cb.tun_dst and causes kernel to crash.

Signed-off-by: Yifeng Sun 
---
v1->v2: Improved commit message and fixed __gre6_xmit, thanks Greg!

 datapath/linux/compat/ip6_gre.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
index 54a76ab..3904455 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -876,9 +876,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
struct tnl_ptk_info tpi;
__be16 protocol;
 
-   if (dev->type == ARPHRD_ETHER)
-   IPCB(skb)->flags = 0;
-
if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;
else
@@ -1146,7 +1143,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff 
*skb,
goto tx_err;
 
t->parms.o_flags &= ~TUNNEL_KEY;
-   IPCB(skb)->flags = 0;
 
tun_info = ovs_skb_tunnel_info(skb);
if (unlikely(!tun_info ||
-- 
2.7.4

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

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ben Pfaff]

On Wed, Aug 15, 2018 at 01:49:45PM +0100, Ian Stokes wrote:
> On 8/14/2018 10:19 PM, Ben Pfaff wrote:
> >On Tue, Aug 14, 2018 at 04:42:06PM +, Stokes, Ian wrote:
> >>Recently at the OVS DPDK community meeting the case for 2 new branches was 
> >>raised.
> >>
> >>https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350898.html
> >>
> >>These branches would be:
> >>
> >>(i) OVS DPDK Latest: This branch would essentially be OVS master using the 
> >>latest DPDK release (Including non LTS releases).
> >>
> >>The purpose of this branch would be to allow OVS DPDK developers to assess 
> >>the latest DPDK releases with OVS and provide feedback to the DPDK 
> >>community if changes are required. Currently OVS transitions between 
> >>supported DPDK releases using DPDK LTS releases only. DPDK LTS releases 
> >>happen annually. The next DPDK LTS release would be 18.11. However the 
> >>other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs 
> >>that impact OVS DPDK (Such as the HWOL). This feedback would be in place 
> >>for the next LTS release before OVS transitions to the next x.11 LTS.
> >>
> >>(ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but 
> >>would encompass the HWOL development work that is ongoing.
> >>
> >>The feeling as regards the need for a OVS DPDK HWOL branch is that it 
> >>requires new features only available in the latest DPDK releases and that 
> >>there will be a lot of code rework required as its validated with various 
> >>HW devices over time before an acceptable solution will be in place.
> >>
> >>There was a question as regards the logistics of where the branches should 
> >>reside. It was suggested that they could be part of the OVS Repo to 
> >>centralize the development work but that is obviously something that would 
> >>have to be raised with yourself and the other project maintainers.
> >>
> >>An alternative would be that it would be hosted on a developers GitHub repo 
> >>similar to how the dpdk_merge branches currently work.
> >>
> >>Neither of the branches would be subject to releases as the end goal of the 
> >>development work carried out on them would make its way into OVS Master 
> >>eventually.
> >
> >This seems reasonable, as long as it doesn't overburden developers with
> >branches.
> 
> I agree, and I think the messaging as regards its purpose of usage should be
> clear, it's intended as a tool to aid development of features for intended
> for OVS master in the future such as DPDK releases or . Developers could
> ignore these branches if the features they develop are already available in
> the existing supported DPDK.
> 
> >
> >How do you prefer to host it?  Any of the following would be fine with
> >me, in descending order of preference:
> >
> >* Host in openvswitch github repo forked from main openvswitch/ovs repo.
> 
> I think the option above would be good, better than being provisioned on a
> developers own github repo. At least when documenting its purpose it would
> look a little more official rather than pointing to a personal github repo.
> 
> In terms of commit access I take it the default ovs committers would have
> access here as well (I take it they would be the people to create the repo).
> In terms of merging would this follow the pull_request process to yourself?

I'd think it would make sense to give you the ability to push directly
to this new repo.  It's only merging into release branches that the OVS
contribution policy is supposed to gate.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] www.apprendre.ovh

[henry]

Épanouissez-vous en aidant les autres !



Découvrez notre formation en .M.A.G.N.E.T.I.S.M.E.

























Apprenez,  par simple imposition des mains à soulager en ré-harmonisant les 
énergies.  Découvrez, comprenez et contrôlez ces incroyables pouvoirs qui sont 
cachés en vous.













 Ce cursus estOUVERT A TOUSLe Magnétisme est une thérapie "alternative" 
efficace, sans danger, facile à  apprendre et à  pratiquer, afin de pouvoir 
aider les autres à  retrouver Santé et bien-être tant physique que 
psychologique.













Pour élaborer le contenu de ce stage nous nous sommes inspirés, à  la fois des 
méthodes utilisées par les guérisseurs traditionnels tels H. Durville, Kaly, 
Charly Samson, tout en y 

























Répondez à ce mail avec RETRAIT dans l'objet, afin de ne plus en recevoir 




























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

Re: [ovs-dev] [PATCH 2/2] test-unixctl.py: Don't suppress exceptions.

[Ben Pfaff]

On Fri, Jul 13, 2018 at 02:38:36PM +0530, Numan Siddique wrote:
> On Sat, Jun 16, 2018 at 3:42 AM Ben Pfaff  wrote:
> 
> > A user reported a failure of test 2364 "vlog - RFC5424 facility - Python2"
> > with an exit code that says that the test-unixctl process died from an
> > uncaught exception.  Unfortunately the exception didn't show up in the log.
> > This commit should make the exception show up (it deletes some boilerplate
> > we use in our Python-based daemons to make them restart themselves on
> > failure, which isn't needed or appropriate for a test script).
> >
> > Reported-by: Sanket Sudake 
> > Reported-at:
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046840.html
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Numan Siddique 

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

Re: [ovs-dev] [PATCH] configure: Enable GCC relevant new 8.x warning options.

[Ben Pfaff]

On Fri, Jul 13, 2018 at 11:17:16AM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > These don't trigger any new actual warnings in my own build.
> >
> > GCC 8.x adds other new warning options that are enabled by -Wall or
> > -Wextra.  This commit doesn't explicitly enable those because OVS already
> > enables -Wall and -Wextra.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> LGTM.  Sorry it took so long to review.
> 
> Acked-by: Aaron Conole 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 5/5] netdev: Clean up class initialization.

[Ben Pfaff]

On Wed, Aug 01, 2018 at 03:23:38PM +0100, Ian Stokes wrote:
> On 7/12/2018 10:55 PM, Ben Pfaff wrote:
> >The macros are hard to read.  This makes it a little more readable.
> >
> 
> Thanks for this Ben, one minor comment below.
> 
> >Signed-off-by: Ben Pfaff 
> >---
> >  configure.ac  |   1 +
> >  lib/netdev-dpdk.c | 235 --
> >  lib/netdev-dummy.c| 134 
> >  lib/netdev-linux.c| 340 
> > +++---
> >  lib/netdev-linux.h|  18 +--
> >  lib/netdev-provider.h |   2 -
> >  lib/netdev-vport.c| 223 +++--
> >  7 files changed, 369 insertions(+), 584 deletions(-)
> >
> >diff --git a/configure.ac b/configure.ac
> >index c89c607c7124..66281c4d6811 100644
> >--- a/configure.ac
> >+++ b/configure.ac
> >@@ -172,6 +172,7 @@ OVS_ENABLE_OPTION([-Wduplicated-cond])
> >  OVS_ENABLE_OPTION([-Qunused-arguments])
> >  OVS_ENABLE_OPTION([-Wshadow])
> >  OVS_ENABLE_OPTION([-Wno-null-pointer-arithmetic])
> >+OVS_ENABLE_OPTION([-Warray-bounds-pointer-arithmetic])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], 
> > [HAVE_WNO_UNUSED_PARAMETER])
> >  OVS_ENABLE_WERROR
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 9bf21856075b..4de4cf116e92 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -4695,161 +4695,86 @@ netdev_dpdk_flow_del(struct netdev *netdev, const 
> >ovs_u128 *ufid,
> >  ufid, rte_flow);
> >  }
> >-#define DPDK_FLOW_OFFLOAD_API \
> >-NULL,   /* flow_flush */  \
> >-NULL,   /* flow_dump_create */\
> >-NULL,   /* flow_dump_destroy */   \
> >-NULL,   /* flow_dump_next */  \
> >-netdev_dpdk_flow_put, \
> >-NULL,   /* flow_get */\
> >-netdev_dpdk_flow_del, \
> >-NULL/* init_flow_api */
> 
> Not sure if DPDK_FLOW_OFFLOAD_API should be completely removed, as I
> understand it the remaining offload functionality is currently being worked
> on with a view to enable full HW offload so they will be re-introduced in
> the future.
> 
> The macro could be moved from here to netdev-dpdk.h and then added to the
> NETDEV_DPDK_CLASS_BASE macro you introduce below (this would be similar to
> what is implemented for netdev-linux, and a more uniform approach across the
> netdevs).
> 
> Sugesh, you've been involved in the HW full offload work, do you have an
> opinion on this?

OK.  I folded in the following and reposted v2 as:
https://patchwork.ozlabs.org/patch/957990/

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 917b3b82c88e..628d75a75024 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4715,8 +4715,7 @@ netdev_dpdk_flow_del(struct netdev *netdev, const 
ovs_u128 *ufid,
 .rxq_construct = netdev_dpdk_rxq_construct, \
 .rxq_destruct = netdev_dpdk_rxq_destruct,   \
 .rxq_dealloc = netdev_dpdk_rxq_dealloc, \
-.flow_put = netdev_dpdk_flow_put,   \
-.flow_del = netdev_dpdk_flow_del
+DPDK_FLOW_OFFLOAD_API
 
 #define NETDEV_DPDK_CLASS_BASE  \
 NETDEV_DPDK_CLASS_COMMON,   \
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index b7d02a77dc72..cc0501d6879a 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -25,6 +25,10 @@ struct dp_packet;
 
 #ifdef DPDK_NETDEV
 
+#define DPDK_FLOW_OFFLOAD_API   \
+.flow_put = netdev_dpdk_flow_put,   \
+.flow_del = netdev_dpdk_flow_del
+
 void netdev_dpdk_register(void);
 void free_dpdk_buf(struct dp_packet *);
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] netdev: Clean up class initialization.

[Ben Pfaff]

The macros are hard to read.  This makes it a little more readable.

Signed-off-by: Ben Pfaff 
---
v1->v2: Refactor DPDK_FLOW_OFFLOAD_API as Ian requested.

 configure.ac  |   1 +
 lib/netdev-dpdk.c | 234 --
 lib/netdev-dpdk.h |   4 +
 lib/netdev-dummy.c| 134 
 lib/netdev-linux.c| 340 +++---
 lib/netdev-linux.h|  18 +--
 lib/netdev-provider.h |   2 -
 lib/netdev-vport.c| 223 +++--
 8 files changed, 372 insertions(+), 584 deletions(-)

diff --git a/configure.ac b/configure.ac
index 09e5888d4215..6059ff4fa9e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -173,6 +173,7 @@ OVS_ENABLE_OPTION([-Wduplicated-cond])
 OVS_ENABLE_OPTION([-Qunused-arguments])
 OVS_ENABLE_OPTION([-Wshadow])
 OVS_ENABLE_OPTION([-Wno-null-pointer-arithmetic])
+OVS_ENABLE_OPTION([-Warray-bounds-pointer-arithmetic])
 OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED])
 OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], [HAVE_WNO_UNUSED_PARAMETER])
 OVS_ENABLE_WERROR
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ac02a09acb9e..628d75a75024 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4693,161 +4693,85 @@ netdev_dpdk_flow_del(struct netdev *netdev, const 
ovs_u128 *ufid,
 ufid, rte_flow);
 }
 
-#define DPDK_FLOW_OFFLOAD_API \
-NULL,   /* flow_flush */  \
-NULL,   /* flow_dump_create */\
-NULL,   /* flow_dump_destroy */   \
-NULL,   /* flow_dump_next */  \
-netdev_dpdk_flow_put, \
-NULL,   /* flow_get */\
-netdev_dpdk_flow_del, \
-NULL/* init_flow_api */
-
-
-#define NETDEV_DPDK_CLASS(NAME, INIT, CONSTRUCT, DESTRUCT,\
-  SET_CONFIG, SET_TX_MULTIQ, SEND,\
-  GET_CARRIER, GET_STATS,\
-  GET_CUSTOM_STATS,
  \
-  GET_FEATURES, GET_STATUS,   \
-  RECONFIGURE, RXQ_RECV)  \
-{ \
-NAME, \
-true,   /* is_pmd */  \
-INIT,   /* init */\
-NULL,   /* netdev_dpdk_run */ \
-NULL,   /* netdev_dpdk_wait */\
-  \
-netdev_dpdk_alloc,\
-CONSTRUCT,\
-DESTRUCT, \
-netdev_dpdk_dealloc,  \
-netdev_dpdk_get_config,   \
-SET_CONFIG,   \
-NULL,   /* get_tunnel_config */   \
-NULL,   /* build header */\
-NULL,   /* push header */ \
-NULL,   /* pop header */  \
-netdev_dpdk_get_numa_id,/* get_numa_id */ \
-SET_TX_MULTIQ,\
-  \
-SEND,   /* send */\
-NULL,   /* send_wait */   \
-  \
-netdev_dpdk_set_etheraddr,\
-netdev_dpdk_get_etheraddr,\
-netdev_dpdk_get_mtu,  \
-netdev_dpdk_set_mtu,  \
-netdev_dpdk_get_ifindex,  \
-GET_CARRIER,  \
-netdev_dpdk_get_carrier_resets,   \
-netdev_dpdk_set_miimon,   \
-GET_STATS,\
-GET_CUSTOM_STATS,  
  \
-GET_FEATURES, \
-NULL,   /* set_advertisements */  \
-NULL,   /* get_pt_mode */ \
-  \
-netdev_dpdk_set_policing, \
-netdev_dpdk_get_qos_types,\
-NULL, 

Re: [ovs-dev] [PATCH v4 0/2] Partial cluster support in Python IDL client

[Ben Pfaff]

On Wed, Aug 15, 2018 at 02:39:42PM +0530, Numan Siddique wrote:
> On Wed, Aug 15, 2018 at 12:20 AM Ben Pfaff  wrote:
> 
> > On Tue, Aug 07, 2018 at 05:07:19PM +0530, nusid...@redhat.com wrote:
> > > From: Numan Siddique 
> > >
> > > Python IDL library is lacking the functionality to connect to the
> > > clustered db servers by providing multiple remotes (like -
> > > "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> > > connection string.
> > >
> > > This patch adds this functionality to the python idl library.
> > > It still lacks the feature to connect to the master of the cluster.
> > > To add this
> > >   - python idl client should monitor and read the '_Server' schema
> > >   - connect to the master of the cluster.
> > >
> > > I will submit the patch once that is ready. But for now I think this
> > > is good enough for the clients to connect to the cluster dbs.
> >
> > I applied this series to master.  Do you want it backported?
> >
> 
> Thanks for the review and applying the patches. Yes. It would be great if
> it is back ported to
> branch 2.10.

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

[ovs-dev] Response

[Mr. Justin H.Murphy]

Hi!

My name is Justin Murphy; I am a lawyer from the UK.
I contact you because of my dead client.
My client died without a surviving family member.

I want to nominate you as her heir because you have the same surname as my
client, he left a large amount of money.

This is legal and safe, come to me for better explanation and more details.

Sincerely,
Justin H.Murphy
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ben Pfaff]

On Wed, Aug 15, 2018 at 11:52:16AM -0400, Aaron Conole wrote:
> Probably best to follow the existing branch convention:
> 
>   "branch-XXX"
> 
> So, "branch-dpdk-next" and "branch-dpdk-hwol"

For what it's worth, the current branches' names start with "branch-"
only because, e.g. "2.10" seems like a confusingly short and numerical
name for a branch.

It might still be a reasonable convention for emails.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ian Stokes]

On 8/15/2018 4:52 PM, Aaron Conole wrote:

Ian Stokes  writes:


On 8/15/2018 10:51 AM, Ophir Munk wrote:

Hi,
Please find comments inline.


-Original Message-
From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
boun...@openvswitch.org] On Behalf Of Stokes, Ian
Sent: Tuesday, August 14, 2018 7:42 PM
To: Ben Pfaff (b...@ovn.org) 
Cc: d...@openvswitch.org
Subject: [ovs-dev] OVS DPDK Latest & HWOL Branches

Hi Ben,

Recently at the OVS DPDK community meeting the case for 2 new branches
was raised.

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
l.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
August%2F350898.html&data=02%7C01%7Cophirmu%40mellanox.com
%7C61e0d527081c4e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149
256f461b%7C0%7C1%7C636698617407744747&sdata=0tKB9%2BdVH%
2BIfWaCHzaD6oy1mldVs9FUeZVbR2T7Ym1E%3D&reserved=0

These branches would be:

(i) OVS DPDK Latest: This branch would essentially be OVS master using the
latest DPDK release (Including non LTS releases).

The purpose of this branch would be to allow OVS DPDK developers to assess
the latest DPDK releases with OVS and provide feedback to the DPDK
community if changes are required. Currently OVS transitions between
supported DPDK releases using DPDK LTS releases only. DPDK LTS releases
happen annually. The next DPDK LTS release would be 18.11. However the
other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs that
impact OVS DPDK (Such as the HWOL). This feedback would be in place for
the next LTS release before OVS transitions to the next x.11 LTS.

(ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but
would encompass the HWOL development work that is ongoing.

The feeling as regards the need for a OVS DPDK HWOL branch is that it
requires new features only available in the latest DPDK releases and that
there will be a lot of code rework required as its validated with various HW
devices over time before an acceptable solution will be in place.

There was a question as regards the logistics of where the branches should
reside. It was suggested that they could be part of the OVS Repo to
centralize the development work but that is obviously something that would
have to be raised with yourself and the other project maintainers.



If using OVS Repo for the 2 branches will also enable having
frequent builds and
running automated CI tests - that would greatly help to make sure the branches
are backward compatible with master OVS.


I think this should be simple enough to enable.




An alternative would be that it would be hosted on a developers GitHub repo
similar to how the dpdk_merge branches currently work.



Where ever the branches are - I suggest using the patchwork mailing list
for sending patches to these branches.
We will need to decide on a header prefix to designate a specific patch to
its relevant branch (e.g. "HWOL PATCH v1" or "DPDKLATEST PATCH v1").


Sure, I'm not familiar with the inner workings of patchwork but I
believe it should work once a patch is sent to d...@openvswitch.org.

As you said we just need a to agree the header prefix to distinguish
that it's not intended for the main OVS repo. (I was thinking
"OVS_DPDK_LATEST" and "OVS_DPDK_HWOL"), using just DPDK could confuse
people into think it's a patch for DPDK itself rather than ovs.


Probably best to follow the existing branch convention:

   "branch-XXX"

So, "branch-dpdk-next" and "branch-dpdk-hwol"

Then patches get posted as:
[PATCH branch-dpdk-next]
[PATCH branch-hwol]

I don't think it should be confusing to omit the ovs - after all they're
OVS branches / trees / whatever you want to consider them.  I don't
think anyone would mistake them that way (but maybe they would if there
was an xpost?  the file layout is radically different so the patch won't
even apply accidentally.. I dunno).


Probably overthinking it on my side, I think the branch convention you 
have above look good tbh.





Ian



Neither of the branches would be subject to releases as the end goal of the
development work carried out on them would make its way into OVS Master
eventually.

Curious as to what your thoughts on this would be?


I think it makes sense - it can help give early feedback.  We can
probably be a bit more 'lax' in that tree as well, since some of the
dpdk compatibility work might need to be thrown away (ie: change in
XX.05, then XX.08, then XX.11-LTS which would merge mainline might have
3 different sets of changes, but hopefully not).

I also like the idea that we could do a nightly integration with dpdk
mainline without needing to affect the ovs master branch.  That means we
could detect breakages in the ovs<->dpdk interface much earlier.


Yes, I think this will be a major benefit to my mind. At this point it's 
hard to judge how easy a transition to the latest DPDK LTS will be. WIth 
this it would start to give some insight at least.




All that said - I really hope that people don't start thinking of
whateve

Re: [ovs-dev] [PATCH] conntrack.c: Add missing return value check to prevent nptr dereference.

[Aaron Conole]

Jiecheng Wu  writes:

> Function ovs_ct_limit_cmd_get() defined in net/openvswitch/conntrack.c may 
> cause a null pointer dereference as it calls nla_nest_start which may return 
> NULL. The returned value is used in function nla_nest_end() later where the 
> pointer is dereferenced.
> ---
>  net/openvswitch/conntrack.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 284aca2..dad0456 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -2132,6 +2132,10 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb, 
> struct genl_info *info)
>   return PTR_ERR(reply);
>  
>   nla_reply = nla_nest_start(reply, OVS_CT_LIMIT_ATTR_ZONE_LIMIT);
> + if (!nla_reply) {
> + err = -ENOMEM;
> + goto exit_err;
> + }
>  
>   if (a[OVS_CT_LIMIT_ATTR_ZONE_LIMIT]) {
>   err = ovs_ct_limit_get_zone_limit(

This patch is appropriate to the net...@vger.kernel.org mailing list.  A
version was submitted already by Stephen Hemminger (Cc'd).

See:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349816.html

Looks like these were not accepted per David's response at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349929.html

Stephen, are you going to resubmit your patches to netdev?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Aaron Conole]

Ian Stokes  writes:

> On 8/15/2018 10:51 AM, Ophir Munk wrote:
>> Hi,
>> Please find comments inline.
>>
>>> -Original Message-
>>> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>>> boun...@openvswitch.org] On Behalf Of Stokes, Ian
>>> Sent: Tuesday, August 14, 2018 7:42 PM
>>> To: Ben Pfaff (b...@ovn.org) 
>>> Cc: d...@openvswitch.org
>>> Subject: [ovs-dev] OVS DPDK Latest & HWOL Branches
>>>
>>> Hi Ben,
>>>
>>> Recently at the OVS DPDK community meeting the case for 2 new branches
>>> was raised.
>>>
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
>>> l.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
>>> August%2F350898.html&data=02%7C01%7Cophirmu%40mellanox.com
>>> %7C61e0d527081c4e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149
>>> 256f461b%7C0%7C1%7C636698617407744747&sdata=0tKB9%2BdVH%
>>> 2BIfWaCHzaD6oy1mldVs9FUeZVbR2T7Ym1E%3D&reserved=0
>>>
>>> These branches would be:
>>>
>>> (i) OVS DPDK Latest: This branch would essentially be OVS master using the
>>> latest DPDK release (Including non LTS releases).
>>>
>>> The purpose of this branch would be to allow OVS DPDK developers to assess
>>> the latest DPDK releases with OVS and provide feedback to the DPDK
>>> community if changes are required. Currently OVS transitions between
>>> supported DPDK releases using DPDK LTS releases only. DPDK LTS releases
>>> happen annually. The next DPDK LTS release would be 18.11. However the
>>> other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs 
>>> that
>>> impact OVS DPDK (Such as the HWOL). This feedback would be in place for
>>> the next LTS release before OVS transitions to the next x.11 LTS.
>>>
>>> (ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but
>>> would encompass the HWOL development work that is ongoing.
>>>
>>> The feeling as regards the need for a OVS DPDK HWOL branch is that it
>>> requires new features only available in the latest DPDK releases and that
>>> there will be a lot of code rework required as its validated with various HW
>>> devices over time before an acceptable solution will be in place.
>>>
>>> There was a question as regards the logistics of where the branches should
>>> reside. It was suggested that they could be part of the OVS Repo to
>>> centralize the development work but that is obviously something that would
>>> have to be raised with yourself and the other project maintainers.
>>>
>>
>> If using OVS Repo for the 2 branches will also enable having
>> frequent builds and
>> running automated CI tests - that would greatly help to make sure the 
>> branches
>> are backward compatible with master OVS.
>
> I think this should be simple enough to enable.
>
>>
>>> An alternative would be that it would be hosted on a developers GitHub repo
>>> similar to how the dpdk_merge branches currently work.
>>>
>>
>> Where ever the branches are - I suggest using the patchwork mailing list
>> for sending patches to these branches.
>> We will need to decide on a header prefix to designate a specific patch to
>> its relevant branch (e.g. "HWOL PATCH v1" or "DPDKLATEST PATCH v1").
>
> Sure, I'm not familiar with the inner workings of patchwork but I
> believe it should work once a patch is sent to d...@openvswitch.org.
>
> As you said we just need a to agree the header prefix to distinguish
> that it's not intended for the main OVS repo. (I was thinking
> "OVS_DPDK_LATEST" and "OVS_DPDK_HWOL"), using just DPDK could confuse
> people into think it's a patch for DPDK itself rather than ovs.

Probably best to follow the existing branch convention:

  "branch-XXX"

So, "branch-dpdk-next" and "branch-dpdk-hwol"

Then patches get posted as:
[PATCH branch-dpdk-next]
[PATCH branch-hwol]

I don't think it should be confusing to omit the ovs - after all they're
OVS branches / trees / whatever you want to consider them.  I don't
think anyone would mistake them that way (but maybe they would if there
was an xpost?  the file layout is radically different so the patch won't
even apply accidentally.. I dunno).

> Ian
>>
>>> Neither of the branches would be subject to releases as the end goal of the
>>> development work carried out on them would make its way into OVS Master
>>> eventually.
>>>
>>> Curious as to what your thoughts on this would be?

I think it makes sense - it can help give early feedback.  We can
probably be a bit more 'lax' in that tree as well, since some of the
dpdk compatibility work might need to be thrown away (ie: change in
XX.05, then XX.08, then XX.11-LTS which would merge mainline might have
3 different sets of changes, but hopefully not).

I also like the idea that we could do a nightly integration with dpdk
mainline without needing to affect the ovs master branch.  That means we
could detect breakages in the ovs<->dpdk interface much earlier.

All that said - I really hope that people don't start thinking of
whatever you want to call those branches as anything other than
experimentation / early feedba

[ovs-dev] Hej, har brug for et lån

[har brug for et lån]

  Hej, har brug for et lån. Uanset din økonomiske situation. Jeg er en 
person, der giver uanset mængden, enhver type af lån til en rente på 2 
% til 3 % over en periode på højst 15 år til alle mennesker i stand til 
at respektere hans kompromis, så du kan fortælle mig, din månedlig 
betalinger. Der eksisterer ingen kontakt mig for mere information.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ian Stokes]

On 8/15/2018 10:51 AM, Ophir Munk wrote:

Hi,
Please find comments inline.


-Original Message-
From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
boun...@openvswitch.org] On Behalf Of Stokes, Ian
Sent: Tuesday, August 14, 2018 7:42 PM
To: Ben Pfaff (b...@ovn.org) 
Cc: d...@openvswitch.org
Subject: [ovs-dev] OVS DPDK Latest & HWOL Branches

Hi Ben,

Recently at the OVS DPDK community meeting the case for 2 new branches
was raised.

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
l.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
August%2F350898.html&data=02%7C01%7Cophirmu%40mellanox.com
%7C61e0d527081c4e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149
256f461b%7C0%7C1%7C636698617407744747&sdata=0tKB9%2BdVH%
2BIfWaCHzaD6oy1mldVs9FUeZVbR2T7Ym1E%3D&reserved=0

These branches would be:

(i) OVS DPDK Latest: This branch would essentially be OVS master using the
latest DPDK release (Including non LTS releases).

The purpose of this branch would be to allow OVS DPDK developers to assess
the latest DPDK releases with OVS and provide feedback to the DPDK
community if changes are required. Currently OVS transitions between
supported DPDK releases using DPDK LTS releases only. DPDK LTS releases
happen annually. The next DPDK LTS release would be 18.11. However the
other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs that
impact OVS DPDK (Such as the HWOL). This feedback would be in place for
the next LTS release before OVS transitions to the next x.11 LTS.

(ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but
would encompass the HWOL development work that is ongoing.

The feeling as regards the need for a OVS DPDK HWOL branch is that it
requires new features only available in the latest DPDK releases and that
there will be a lot of code rework required as its validated with various HW
devices over time before an acceptable solution will be in place.

There was a question as regards the logistics of where the branches should
reside. It was suggested that they could be part of the OVS Repo to
centralize the development work but that is obviously something that would
have to be raised with yourself and the other project maintainers.



If using OVS Repo for the 2 branches will also enable having frequent builds and
running automated CI tests - that would greatly help to make sure the branches
are backward compatible with master OVS.


I think this should be simple enough to enable.




An alternative would be that it would be hosted on a developers GitHub repo
similar to how the dpdk_merge branches currently work.



Where ever the branches are - I suggest using the patchwork mailing list
for sending patches to these branches.
We will need to decide on a header prefix to designate a specific patch to
its relevant branch (e.g. "HWOL PATCH v1" or "DPDKLATEST PATCH v1").


Sure, I'm not familiar with the inner workings of patchwork but I 
believe it should work once a patch is sent to d...@openvswitch.org.


As you said we just need a to agree the header prefix to distinguish 
that it's not intended for the main OVS repo. (I was thinking 
"OVS_DPDK_LATEST" and "OVS_DPDK_HWOL"), using just DPDK could confuse 
people into think it's a patch for DPDK itself rather than ovs.


Ian



Neither of the branches would be subject to releases as the end goal of the
development work carried out on them would make its way into OVS Master
eventually.

Curious as to what your thoughts on this would be?

Thanks
Ian

___
dev mailing list
d...@openvswitch.org
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
dev&data=02%7C01%7Cophirmu%40mellanox.com%7C61e0d527081c4
e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1
%7C636698617407744747&sdata=yp3ptjL0FWnQwtTsNwQq1C5HRP6z
NXotnNijJTVdbT4%3D&reserved=0


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

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ian Stokes]

On 8/14/2018 10:19 PM, Ben Pfaff wrote:

On Tue, Aug 14, 2018 at 04:42:06PM +, Stokes, Ian wrote:

Recently at the OVS DPDK community meeting the case for 2 new branches was 
raised.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350898.html

These branches would be:

(i) OVS DPDK Latest: This branch would essentially be OVS master using the 
latest DPDK release (Including non LTS releases).

The purpose of this branch would be to allow OVS DPDK developers to assess the 
latest DPDK releases with OVS and provide feedback to the DPDK community if 
changes are required. Currently OVS transitions between supported DPDK releases 
using DPDK LTS releases only. DPDK LTS releases happen annually. The next DPDK 
LTS release would be 18.11. However the other non-lts DPDK releases (x.02, 
x.05, x.08) can introduce/change APIs that impact OVS DPDK (Such as the HWOL). 
This feedback would be in place for the next LTS release before OVS transitions 
to the next x.11 LTS.

(ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but would 
encompass the HWOL development work that is ongoing.

The feeling as regards the need for a OVS DPDK HWOL branch is that it requires 
new features only available in the latest DPDK releases and that there will be 
a lot of code rework required as its validated with various HW devices over 
time before an acceptable solution will be in place.

There was a question as regards the logistics of where the branches should 
reside. It was suggested that they could be part of the OVS Repo to centralize 
the development work but that is obviously something that would have to be 
raised with yourself and the other project maintainers.

An alternative would be that it would be hosted on a developers GitHub repo 
similar to how the dpdk_merge branches currently work.

Neither of the branches would be subject to releases as the end goal of the 
development work carried out on them would make its way into OVS Master 
eventually.


This seems reasonable, as long as it doesn't overburden developers with
branches.


I agree, and I think the messaging as regards its purpose of usage 
should be clear, it's intended as a tool to aid development of features 
for intended for OVS master in the future such as DPDK releases or . 
Developers could ignore these branches if the features they develop are 
already available in the existing supported DPDK.




How do you prefer to host it?  Any of the following would be fine with
me, in descending order of preference:

* Host in openvswitch github repo forked from main openvswitch/ovs repo.


I think the option above would be good, better than being provisioned on 
a developers own github repo. At least when documenting its purpose it 
would look a little more official rather than pointing to a personal 
github repo.


In terms of commit access I take it the default ovs committers would 
have access here as well (I take it they would be the people to create 
the repo). In terms of merging would this follow the pull_request 
process to yourself?


I think with this model we could hook it into travis, read the docs and 
appveyor quite easily too?


Thanks
Ian


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

Re: [ovs-dev] OVS DPDK Latest & HWOL Branches

[Ophir Munk]

Hi,
Please find comments inline.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Stokes, Ian
> Sent: Tuesday, August 14, 2018 7:42 PM
> To: Ben Pfaff (b...@ovn.org) 
> Cc: d...@openvswitch.org
> Subject: [ovs-dev] OVS DPDK Latest & HWOL Branches
> 
> Hi Ben,
> 
> Recently at the OVS DPDK community meeting the case for 2 new branches
> was raised.
> 
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
> l.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
> August%2F350898.html&data=02%7C01%7Cophirmu%40mellanox.com
> %7C61e0d527081c4e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149
> 256f461b%7C0%7C1%7C636698617407744747&sdata=0tKB9%2BdVH%
> 2BIfWaCHzaD6oy1mldVs9FUeZVbR2T7Ym1E%3D&reserved=0
> 
> These branches would be:
> 
> (i) OVS DPDK Latest: This branch would essentially be OVS master using the
> latest DPDK release (Including non LTS releases).
> 
> The purpose of this branch would be to allow OVS DPDK developers to assess
> the latest DPDK releases with OVS and provide feedback to the DPDK
> community if changes are required. Currently OVS transitions between
> supported DPDK releases using DPDK LTS releases only. DPDK LTS releases
> happen annually. The next DPDK LTS release would be 18.11. However the
> other non-lts DPDK releases (x.02, x.05, x.08) can introduce/change APIs that
> impact OVS DPDK (Such as the HWOL). This feedback would be in place for
> the next LTS release before OVS transitions to the next x.11 LTS.
> 
> (ii) OVS DPDK HWOL: This branch would be forked from OVS DPDK Latest but
> would encompass the HWOL development work that is ongoing.
> 
> The feeling as regards the need for a OVS DPDK HWOL branch is that it
> requires new features only available in the latest DPDK releases and that
> there will be a lot of code rework required as its validated with various HW
> devices over time before an acceptable solution will be in place.
> 
> There was a question as regards the logistics of where the branches should
> reside. It was suggested that they could be part of the OVS Repo to
> centralize the development work but that is obviously something that would
> have to be raised with yourself and the other project maintainers.
> 

If using OVS Repo for the 2 branches will also enable having frequent builds and
running automated CI tests - that would greatly help to make sure the branches
are backward compatible with master OVS.

> An alternative would be that it would be hosted on a developers GitHub repo
> similar to how the dpdk_merge branches currently work.
> 

Where ever the branches are - I suggest using the patchwork mailing list 
for sending patches to these branches.
We will need to decide on a header prefix to designate a specific patch to
its relevant branch (e.g. "HWOL PATCH v1" or "DPDKLATEST PATCH v1").

> Neither of the branches would be subject to releases as the end goal of the
> development work carried out on them would make its way into OVS Master
> eventually.
> 
> Curious as to what your thoughts on this would be?
> 
> Thanks
> Ian
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
> l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Cophirmu%40mellanox.com%7C61e0d527081c4
> e1fbfbf08d60204e62b%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1
> %7C636698617407744747&sdata=yp3ptjL0FWnQwtTsNwQq1C5HRP6z
> NXotnNijJTVdbT4%3D&reserved=0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] (no subject)

[lindacarter]

good news 

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

Re: [ovs-dev] [PATCH v4 0/2] Partial cluster support in Python IDL client

[Numan Siddique]

On Wed, Aug 15, 2018 at 12:20 AM Ben Pfaff  wrote:

> On Tue, Aug 07, 2018 at 05:07:19PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > Python IDL library is lacking the functionality to connect to the
> > clustered db servers by providing multiple remotes (like -
> > "tcp:10.0.0.1:6641, tcp:10.0.0.2:6641, tcp:10.0.0.3:6641") in the
> > connection string.
> >
> > This patch adds this functionality to the python idl library.
> > It still lacks the feature to connect to the master of the cluster.
> > To add this
> >   - python idl client should monitor and read the '_Server' schema
> >   - connect to the master of the cluster.
> >
> > I will submit the patch once that is ready. But for now I think this
> > is good enough for the clients to connect to the cluster dbs.
>
> I applied this series to master.  Do you want it backported?
>

Thanks for the review and applying the patches. Yes. It would be great if
it is back ported to
branch 2.10.

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

Re: [ovs-dev] [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order

[Pravin Shelar]

Hi Stefano

On Tue, Aug 7, 2018 at 6:31 AM, Stefano Brivio  wrote:
> Hi Pravin,
>
> On Tue, 31 Jul 2018 16:12:03 -0700
> Pravin Shelar  wrote:
>
>> Rather than reducing number of thread down to 1, we could find better
>> number of FDs per port.
>> How about this simple solution:
>> 1. Allocate (N * P) FDs as long as it is under FD limit.
>> 2. If FD limit (-EMFILE) is hit reduce N value by half and repeat step 1.
>
> I still see a few quantitative issues with this approach, other than
> Ben's observation about design (which, by the way, looks entirely
> reasonable to me).
>
> We're talking about a disproportionate amount of sockets in any case.
> We can have up to 2^16 vports here, with 5k vports being rather common,
> and for any reasonable value of N that manages somehow to perturbate the
> distribution of upcalls per thread, we are talking about something well
> in excess of 100k sockets. I think this doesn't really scale.
>
My argument is not about proposed fairness algorithm. It is about cost
of the fairness and I do not see it is addressed in any of the follow
ups. You seems to be worried about memory cost and fairness aspects, I
am worried about CPU cost of the solution.
I think proposed solution is solving the fairness issue but it is also
creating bottleneck in upcall processing. OVS is known to have slower
upcall processing. This patch is adding even more cost to the upcall
handling. The latency of first packet handling is also going up with
this approach.

I revisited the original patch, here is what I see in term of added
cost to existing upcall processing:
1. one "kzalloc(sizeof(*upcall), GFP_ATOMIC);" This involve allocate
and initialize memory
2. copy flow key which is more than 1 KB (upcall->key = *key)
3. Acquire spin_lock_bh dp->upcalls.lock, which would disable bottom
half processing on CPU while waiting for the global lock.
4. Iterate list of queued upcalls, one of objective it is to avoid out
of order packet. But I do not see point of ordering packets from
different streams.
5. signal upcall thread after delay ovs_dp_upcall_delay(). This adds
further to the latency.
6. upcall is then handed over to different thread (context switch),
likely on different CPU.
8. the upcall object is freed on remote CPU.
9. single lock essentially means OVS kernel datapath upcall processing
is single threaded no matter number of cores in system.

I would be interested in how are we going to address these issues.

In example you were talking about netlink fd issue on server with 48
core, how does this solution works when there are 5K ports each
triggering upcall ? Can you benchmark your patch? Do you have
performance numbers for TCP_CRR with and without this patch ? Also
publish latency numbers for this patch. Please turn off megaflow to
exercise upcall handling.

I understand fairness has cost, but we need to find right balance
between performance and fairness. Current fairness scheme is a
lockless algorithm without much computational overhead, did you try to
improve current algorithm so that it uses less number of ports.


> With the current value for N (3/4 * number of threads) this can even get
> close to /proc/sys/fs/file-max on some systems, and there raising the
> number of allowed file descriptors for ovs-vswitchd isn't a solution
> anymore.
>
> I would instead try to address the concerns that you had about the
> original patch adding fairness in the kernel, rather than trying to
> make the issue appear less severe in ovs-vswitchd.
>
> --
> Stefano
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev