Re: [ovs-dev] [PATCH] ovsdb-tool: add election timer argument to create-cluster command

[Dan Williams]

On Tue, 2021-04-13 at 20:50 -0500, Dan Williams wrote:
> After creating the new cluster database write a raft entry that
> sets the desired election timer. This allows CMSes to set the
> election timer at cluster start and avoid an error-prone
> election timer modification process after the cluster is up.
> 
> Reported-at: https://bugzilla.redhat.com/1831778

Anyone have thoughts on this or a better approach?

Thanks,
Dan

> 
> Signed-off-by: Dan Williams 
> ---
>  ovsdb/ovsdb-tool.c | 30 +-
>  ovsdb/raft.c   |  2 +-
>  ovsdb/raft.h   |  8 
>  3 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
> index b8560f850cd08..8d029cef56df6 100644
> --- a/ovsdb/ovsdb-tool.c
> +++ b/ovsdb/ovsdb-tool.c
> @@ -276,10 +276,22 @@ do_create_cluster(struct ovs_cmdl_context *ctx)
>  const char *db_file_name = ctx->argv[1];
>  const char *src_file_name = ctx->argv[2];
>  const char *local = ctx->argv[3];
> +    uint64_t election_timer = 0;
>  
>  struct ovsdb_schema *schema;
>  struct json *data;
>  
> +    if (ctx->argc >= 5) {
> +    election_timer = atoll(ctx->argv[4]);
> +    /* Election timer smaller than 100ms or bigger than 10min
> doesn't make
> + * sense. */
> +    if (election_timer < 100 || election_timer > 60) {
> +    ovs_fatal(0, "election timer must be between 100 and
> 60, "
> +  "in msec.");
> +    return;
> +    }
> +    }
> +
>  struct ovsdb_error *error =
> ovsdb_schema_from_file(src_file_name, );
>  if (!error) {
>  /* It's just a schema file. */
> @@ -306,6 +318,21 @@ do_create_cluster(struct ovs_cmdl_context *ctx)
>    local, snapshot));
>  ovsdb_schema_destroy(schema);
>  json_destroy(snapshot);
> +
> +    if (election_timer > 0) {
> +    struct ovsdb_log *log = NULL;
> +    struct raft *raft = NULL;
> +
> +    check_ovsdb_error(ovsdb_log_open(db_file_name, RAFT_MAGIC,
> + OVSDB_LOG_READ_WRITE, -1,
> ));
> +    check_ovsdb_error(raft_open(log, ));
> +    /* Use term=1 since raft_open() triggers leader election and
> sets
> + * the term to 1. */
> +    check_ovsdb_error(raft_write_entry(raft, 1,
> json_nullable_clone(NULL),
> +   NULL,
> json_nullable_clone(NULL),
> +   election_timer));
> +    raft_close(raft);
> +    }
>  }
>  
>  static void
> @@ -1675,7 +1702,8 @@ do_list_commands(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>  
>  static const struct ovs_cmdl_command all_commands[] = {
>  { "create", "[db [schema]]", 0, 2, do_create, OVS_RW },
> -    { "create-cluster", "db contents local", 3, 3,
> do_create_cluster, OVS_RW },
> +    { "create-cluster", "db contents local [election timer ms]", 3,
> 4,
> +  do_create_cluster, OVS_RW },
>  { "join-cluster", "db name local remote...", 4, INT_MAX,
> do_join_cluster,
>    OVS_RW },
>  { "compact", "[db [dst]]", 0, 2, do_compact, OVS_RW },
> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> index d2ff643b27379..4e2a508033ce0 100644
> --- a/ovsdb/raft.c
> +++ b/ovsdb/raft.c
> @@ -697,7 +697,7 @@ raft_add_entry(struct raft *raft,
>  
>  /* Writes a RAFT_REC_ENTRY record for 'term', 'data', 'eid',
> 'servers',
>   * 'election_timer' to * 'raft''s log and returns an error
> indication. */
> -static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
> +struct ovsdb_error * OVS_WARN_UNUSED_RESULT
>  raft_write_entry(struct raft *raft, uint64_t term, struct json
> *data,
>   const struct uuid *eid, struct json *servers,
>   uint64_t election_timer)
> diff --git a/ovsdb/raft.h b/ovsdb/raft.h
> index 99d5307e54684..92b1c7741ed96 100644
> --- a/ovsdb/raft.h
> +++ b/ovsdb/raft.h
> @@ -170,6 +170,14 @@ uint64_t raft_command_get_commit_index(const
> struct raft_command *);
>  void raft_command_unref(struct raft_command *);
>  void raft_command_wait(const struct raft_command *);
>  
> +struct ovsdb_error *raft_write_entry(struct raft *raft,
> + uint64_t term,
> + struct json *data,
> + const struct uuid *eid,
> + struct json *servers,
> + uint64_t election_timer)
> +    OVS_WARN_UNUSED_RESULT;
> +
>  /* Replacing the local log by a snapshot. */
>  bool raft_grew_lots(const struct raft *);
>  uint64_t raft_get_log_length(const struct raft *);


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

Re: [ovs-dev] raft: print local server ID when opening RAFT database

[Dan Williams]

On Mon, 2021-04-05 at 10:34 -0500, Dan Williams wrote:
> Signed-off-by: Dan Williams 

Friendly ping for review on this one...

Thanks!
Dan

> ---
>  ovsdb/raft.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> index d2ff643b27379..7f3ee733dbcc0 100644
> --- a/ovsdb/raft.c
> +++ b/ovsdb/raft.c
> @@ -1026,6 +1026,8 @@ raft_open(struct ovsdb_log *log, struct raft
> **raftp)
>  raft_reset_ping_timer(raft);
>  raft_reset_election_timer(raft);
>  
> +    VLOG_INFO("local server ID is "SID_FMT, SID_ARGS(>sid));
> +
>  *raftp = raft;
>  hmap_insert(_rafts, >hmap_node, hash_string(raft-
> >name, 0));
>  return NULL;


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

[ovs-dev] ovn master test failures

[Ihar Hrachyshka]

Hi Numan,

both 3187b9fef124e038e474270a2728fe94bdca8eef (ovn-northd: introduce
new allow-stateless ACL verb) and
127bf166ccf4a2509f670c48a00b0340039f20d2 (northd: Support flow
offloading for logical switches with no ACLs.) got merged in upstream
master, and this combination broke the following tests:

774: ovn -- ACL allow-stateless omit conntrack - Port_Group --
ovn-northd-ddlog -- dp-groups=yes FAILED (ovn-northd.at:2752)
775: ovn -- ACL allow-stateless omit conntrack - Port_Group --
ovn-northd-ddlog FAILED (ovn-northd.at:2752)

while the other scenarios are passing:

768: ovn -- ACL allow-stateless omit conntrack - Logical_Switch --
ovn-northd -- dp-groups=yes ok
769: ovn -- ACL allow-stateless omit conntrack - Logical_Switch -- ovn-northd ok
770: ovn -- ACL allow-stateless omit conntrack - Logical_Switch --
ovn-northd-ddlog -- dp-groups=yes ok
771: ovn -- ACL allow-stateless omit conntrack - Logical_Switch --
ovn-northd-ddlog ok
772: ovn -- ACL allow-stateless omit conntrack - Port_Group --
ovn-northd -- dp-groups=yes ok
773: ovn -- ACL allow-stateless omit conntrack - Port_Group -- ovn-northd ok

These scenarios (both ok and FAILED) were added with allow-stateless
patch. If I revert "northd: Support flow offloading for logical
switches with no ACLs.", all tests pass.

Two things to note:
1) only ddlog tests fail;
2) only port_group scenarios fail while logical_switch counterparts don't.

Scenarios fail with the following message in testsuite.log:

+2021-05-13T01:06:37Z|1|ovntrace|WARN|lsp1: unknown logical port
+2021-05-13T01:06:37Z|2|ovntrace|WARN|microflow does not specify
ingress port

This is because SB database Port_Binding table is empty when ovn-trace
is executed. In ddlog northd log, I see the following actions:

2021-05-13T01:06:37.902Z|00113|jsonrpc|DBG|unix:/home/ihrachys/dev/ovn/tests/testsuite.dir/774/ovn-nb/ovn-nb.sock:
received notification, method="update",
params=[["monid","OVN_Northbound"],{"Port_Group":{"9ad4d0eb-e643-43e3-aba9-bf80e5349622":{"new":{"ports":["set",[["uuid","04169cef-5ba2-46ee-803c-559e448f9e0e"],["uuid","edd3283e-a056-49d5-9412-6422f01c66df"]]],"name":"pg","external_ids":["map",[]],"acls":["set",[["uuid","02193db0-0b05-4078-a46f-905e12585a22"],["uuid","0277809c-c018-4a35-9100-6f24102ec204"],["uuid","1580f03b-0fb3-4a4b-866b-70c23f8029c5"],["uuid","2a62a6ad-f7f7-42e8-a757-0e5c5215cd0f"],["uuid","2be534fe-8646-4f17-9cec-9cdff8cd1d21"],["uuid","4a714f8c-b901-4ba6-99cd-6150c715f758"],["uuid","7783fd4d-d28f-4636-a191-99c8bd611761"],["uuid","fbfa736e-03d0-4924-9c14-dd3dbc9bb743"]]]},"old":{"acls":["set",[["uuid","02193db0-0b05-4078-a46f-905e12585a22"],["uuid","0277809c-c018-4a35-9100-6f24102ec204"],["uuid","1580f03b-0fb3-4a4b-866b-70c23f8029c5"],["uuid","2a62a6ad-f7f7-42e8-a757-0e5c5215cd0f"],["uuid","2be534fe-8646-4f17-9cec-9cdff8cd1d21"],["uuid","4a714f8c-b901-4ba6-99cd-6150c715f758"],["uuid","7783fd4d-d28f-4636-a191-99c8
 
bd611761"]]]}}},"ACL":{"fbfa736e-03d0-4924-9c14-dd3dbc9bb743":{"new":{"name":["set",[]],"priority":1,"log":false,"external_ids":["map",[]],"direction":"to-lport","meter":["set",[]],"action":"allow-stateless","match":"tcp","severity":["set",[]]]
2021-05-13T01:06:37.902Z|00114|jsonrpc|DBG|unix:/home/ihrachys/dev/ovn/tests/testsuite.dir/774/ovn-nb/ovn-nb.sock:
received notification, method="update",
params=[["monid","OVN_Northbound"],{"NB_Global":{"ae62228c-aa3d-479a-a251-e612e38e7fdc":{"new":{"name":"","sb_cfg_timestamp":1620867997820,"hv_cfg":1,"nb_cfg":2,"external_ids":["map",[]],"options":["map",[["mac_prefix","06:30:c8"],["max_tunid","16711680"],["northd_internal_version","21.03.90-20.17.0-56.0"],["svc_monitor_mac","c2:cb:ea:d4:18:86"],["use_logical_dp_groups","true"]]],"sb_cfg":1,"ssl":["set",[]],"ipsec":false,"hv_cfg_timestamp":0,"connections":["set",[]],"nb_cfg_timestamp":1620867997730},"old":{"nb_cfg":1]
2021-05-13T01:06:37.934Z|00115|jsonrpc|DBG|unix:/home/ihrachys/dev/ovn/tests/testsuite.dir/774/ovn-sb/ovn-sb.sock:
received reply,
result=[{"uuid":["uuid","1409c320-3e56-2ea6-9f96-ec17e491f2b2"]},{"uuid":["uuid","212b5074-33d6-6f20-d3d4-453b01bb7484"]},{},{}],
id=23
2021-05-13T01:06:37.934Z|00116|jsonrpc|DBG|unix:/home/ihrachys/dev/ovn/tests/testsuite.dir/774/ovn-nb/ovn-nb.sock:
send request, method="transact",
params=["OVN_Northbound",{"where":[["_uuid","==",["uuid","ae62228c-aa3d-479a-a251-e612e38e7fdc"]]],"table":"NB_Global","op":"update","row":{"ipsec":false,"hv_cfg":2,"hv_cfg_timestamp":0,"sb_cfg":1,"options":["map",[["mac_prefix","06:30:c8"],["max_tunid","16711680"],["northd_internal_version","21.03.90-20.17.0-56.0"],["svc_monitor_mac","c2:cb:ea:d4:18:86"],["use_logical_dp_groups","true"]]],"nb_cfg_timestamp":1620867997903}},{"comment":"ovn-northd-ddlog","op":"comment"}],
id=24
2021-05-13T01:06:37.934Z|00117|jsonrpc|DBG|unix:/home/ihrachys/dev/ovn/tests/testsuite.dir/774/ovn-sb/ovn-sb.sock:
send request, method="transact",

Re: [ovs-dev] [PATCH ovn v2 1/1] ovn-controller: Ensure br-int is using secure fail-mode

[Numan Siddique]

On Fri, May 7, 2021 at 5:45 PM Frode Nordahl
 wrote:
>
> fre. 7. mai 2021, 19:50 skrev Flavio Fernandes :
>
> > By default, OVS bridges use standalone fail-mode, which means it is
> > configured with a single row with the NORMAL action as its OpenFlow table.
> > Upon system reboot, an integration bridge with many ports and such a table
> > could create broadcast storms and duplicate packets. That is why
> > ovn-controller creates the integration bridge with secure fail-mode.
> > Under that mode, the OpenFlow table remains empty until the controller
> > populates it, which could happen many seconds after the bridge is
> > operational. Unfortunately, the fail-mode setting was not being
> > done if the bridge was already created by the time ovn-controller
> > starts. This change fixes that and logs a warning should the fail-mode
> > ever needed to be corrected.
> >
> > Reported-at: https://bugzilla.redhat.com/1957025
> > Signed-off-by: Flavio Fernandes 
> > ---
> >  v1->v2: Changes from code review. Thanks, Frode!
> >
> >  controller/ovn-controller.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index 6106a9661..d925522e1 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -413,6 +413,10 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
> >  if (datapath_type && strcmp(br_int->datapath_type,
> > datapath_type)) {
> >  ovsrec_bridge_set_datapath_type(br_int, datapath_type);
> >  }
> > +if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
> > +ovsrec_bridge_set_fail_mode(br_int, "secure");
> > +VLOG_WARN("Integration bridge fail-mode changed to
> > 'secure'.");
> > +}
> >  }
> >  return br_int;
> >  }
> > --
> > 2.25.1
> >
>
> Reviewed-by: Frode Nordahl 
>
> (And now I need to go watch that movie where the term Cool beans was
> coined).

Thanks Flavio and Frode for the patch and reviews.  I applied this
patch to the main
branch and to branch-21.03.  Let me know if we need to backport any further ?

Thanks
Numan

>
>
> >
> >
> > ___
> > 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
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] mac-learning: Remove obsolete comments about tags.

[Ben Pfaff]

On Wed, May 12, 2021 at 10:03:43PM +0200, Ilya Maximets wrote:
> On 4/15/21 4:45 AM, Ben Pfaff wrote:
> > Commit a14502a7c529 ("classifier: Retire partitions.") in 2015 removed
> > OVS support for tags for the second time (!), so these comments about
> > them should be removed too.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> Acked-by: Ilya Maximets 

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

Re: [ovs-dev] [PATCH] ovs-actions: Document normal pipeline.

[Ben Pfaff]

On Wed, May 12, 2021 at 07:09:50PM +0200, Ilya Maximets wrote:
> On 4/15/21 5:34 AM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/ovs-actions.xml | 288 +++-
> >  1 file changed, 286 insertions(+), 2 deletions(-)
> > 
> 
> Hi, Ben.  Thanks for writing this down!
> It looks good to me in general.  Few comments inline.

Thanks!  Your comments make sense.  I fixed them, and did a re-read of
my own to find other ways the writing could be improved, and sent v2 for
a second round of review:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382963.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovs-actions: Document normal pipeline.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
v1->v2: Break bond admissibility step into two steps to make it clearer.
  Fix a typo.  Rephrase some text for clarity.  Thanks to Ilya Maximets
  for the review.

 lib/ovs-actions.xml | 304 +++-
 1 file changed, 302 insertions(+), 2 deletions(-)

diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml
index a2778de4bcd6..7beafa7943bb 100644
--- a/lib/ovs-actions.xml
+++ b/lib/ovs-actions.xml
@@ -509,7 +509,8 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
actions=mod_nw_src:1.2.3.4
 
   Subjects the packet to the device's normal L2/L3 processing.  This
   action is not implemented by all OpenFlow switches, and each switch
-  implements it differently.
+  implements it differently.  The section ``The OVS Normal Pipeline''
+  below documents the OVS implementation.
 
 
 flood
@@ -582,7 +583,6 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
actions=mod_nw_src:1.2.3.4
 OpenFlow allows switches to reject such actions.
   
 
-  
   
 
   Output to the Input Port
@@ -664,6 +664,306 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
actions=mod_nw_src:1.2.3.4
   
 
 
+The OVS Normal Pipeline
+
+
+  This section documents how Open vSwitch implements output to the
+  normal port.  The OpenFlow specification places no
+  requirements on how this port works, so all of this documentation is
+  specific to Open vSwitch.
+
+
+
+  Open vSwitch uses the Open_vSwitch database, detailed in
+  ovs-vswitchd.conf.db(5), to determine the details of the
+  normal pipeline.
+
+
+
+  The normal pipeline executes the following ingress stages for each
+  packet.  Each stage either accepts the packet, in which case the packet
+  goes on to the next stage, or drops the packet, which terminates the
+  pipeline.  The result of the ingress stages is a set of output ports,
+  which is the empty set if some ingress stage drops the packet:
+
+
+
+  
+
+  Input port lookup: Looks up the OpenFlow
+  in_port field's value to the corresponding
+  Port and Interface record in the database.
+
+
+
+  The in_port is normally the OpenFlow port that the
+  packet was received on.  If set_field or another actions
+  changes the in_port, the updated value is honored.
+  Accept the packet if the lookup succeeds, which it normally will.  If
+  the lookupn fails, for example because in_port was
+  changed to an unknown value, drop the packet.
+
+  
+
+  
+Drop malformed packet: If the packet is malformed enough that it
+contains only part of an 802.1Q header, then drop the packet with an
+error.
+  
+
+  
+Drop packets sent to a port reserved for mirroring: If the
+packet was received on a port that is configured as the output port for
+a mirror (that is, it is the output_port in some
+Mirror record), then drop the packet.
+  
+
+  
+
+  VLAN input processing: This stage determines what VLAN the
+  packet is in.  It also verifies that this VLAN is valid for the port;
+  if not, drop the packet.  How the VLAN is determined and which ones
+  are valid vary based on the vlan-mode in the input
+  port's Port record:
+
+
+
+  trunk
+  
+The packet is in the VLAN specified in its 802.1Q header, or in
+VLAN 0 if there is no 802.1Q header.  The trunks
+column in the Port record lists the valid VLANs; if it
+is empty, all VLANs are valid.
+  
+
+  access
+  
+The packet is in the VLAN specified in the tag column
+of its Port record.  The packet must not have an
+802.1Q header with a nonzero VLAN ID; if it does, drop the packet.
+  
+
+  native-tagged
+  native-untagged
+  
+Same as trunk except that the VLAN of a packet without
+an 802.1Q header is not necessarily zero; instead, it is taken from
+the tag column.
+  
+
+  dot1q-tunnel
+  
+The packet is in the VLAN specified in the tag column
+of its Port record, which is a QinQ service VLAN with
+the Ethertype specified by the Port's
+other_config : qinq-ethtype.  If the
+packet has an 802.1Q header, then it specifies the customer VLAN.
+The cvlans column specifies the valid customer VLANs;
+if it is empty, all customer VLANs are valid.
+  
+
+  
+
+  
+Drop reserved multicast addresses: If the packet is addressed to
+a reserved Ethernet multicast address and the Bridge
+record does not have 

[ovs-dev] [PATCH ovn] ovn-nbctl: do not report an error for duplicated ecmp routes with --may-exist

[Lorenzo Bianconi]

Do not report an error adding a duplicated ecmp route if --may-exist
parameter is provided.
Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1959909

Signed-off-by: Lorenzo Bianconi 
---
 tests/ovn-nbctl.at| 2 ++
 utilities/ovn-nbctl.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 6d91aa4c5..8af55161f 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1551,6 +1551,7 @@ IPv4 Routes
 AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.2], [1], [],
   [ovn-nbctl: duplicate nexthop for the same ECMP route
 ])
+AT_CHECK([ovn-nbctl --may-exist --ecmp lr-route-add lr0 10.0.0.0/24 11.0.0.2])
 
 dnl Delete ecmp routes
 AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.0.0/24 11.0.0.1])
@@ -1614,6 +1615,7 @@ AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add 
lr0 2003:0db8:1::/64 200
 AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 2003:0db8:1::/64 
2001:0db8:0:f103::6], [1], [],
   [ovn-nbctl: duplicate nexthop for the same ECMP route
 ])
+AT_CHECK([ovn-nbctl --may-exist --ecmp-symmetric-reply lr-route-add lr0 
2003:0db8:1::/64 2001:0db8:0:f103::6])
 
 AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
 IPv4 Routes
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 48fd0b7ee..84e228f02 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4073,7 +4073,9 @@ nbctl_lr_route_add(struct ctl_context *ctx)
 goto cleanup;
 }
 } else if (route) {
-ctl_error(ctx, "duplicate nexthop for the same ECMP route");
+if (!may_exist) {
+ctl_error(ctx, "duplicate nexthop for the same ECMP route");
+}
 goto cleanup;
 }
 
-- 
2.31.1

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

Re: [ovs-dev] [PATCH] mac-learning: Remove obsolete comments about tags.

[Ilya Maximets]

On 4/15/21 4:45 AM, Ben Pfaff wrote:
> Commit a14502a7c529 ("classifier: Retire partitions.") in 2015 removed
> OVS support for tags for the second time (!), so these comments about
> them should be removed too.
> 
> Signed-off-by: Ben Pfaff 
> ---

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

[ovs-dev] [PATCH] ovs-dpctl-top: fix ovs-dpctl-top via pipe

[Timothy Redaelli]

Currently it's not possible to use ovs-dpctl-top via pipe (eg:
ovs-dpctl dump-flows | ovs-dpctl-top --script --verbose) since Python3
doesn't allow to open a file (stdin in our case) in binary mode without
buffering enabled.

This commit changes the behaviour in order to directly pass stdin to
flows_read instead of re-opening it without buffering.

Signed-off-by: Timothy Redaelli 
---
 utilities/ovs-dpctl-top.in | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/utilities/ovs-dpctl-top.in b/utilities/ovs-dpctl-top.in
index fbe6e4f56..2c1766eff 100755
--- a/utilities/ovs-dpctl-top.in
+++ b/utilities/ovs-dpctl-top.in
@@ -1236,11 +1236,7 @@ def flows_script(args):
 
 if (args.flowFiles is None):
 logging.info("reading flows from stdin")
-ihdl = os.fdopen(sys.stdin.fileno(), 'r', 0)
-try:
-flow_db = flows_read(ihdl, flow_db)
-finally:
-ihdl.close()
+flow_db = flows_read(sys.stdin, flow_db)
 else:
 for flowFile in args.flowFiles:
 logging.info("reading flows from %s", flowFile)
-- 
2.31.1

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

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Improve concurrency by adjust flow-limit

[Ben Pfaff]

On Wed, May 12, 2021 at 10:44:12AM +0800, taoyunupt wrote:
> At 2021-05-12 04:00:30, "Ben Pfaff"  wrote:
> >On Tue, May 11, 2021 at 03:52:00PM +0800, Tao YunXiang wrote:
> >> +/* Use duration as a reference, adjust the number of 
> >> flow_limit,
> >> + * when the duration is small, increase the flow-limit, and 
> >> vice versa */
> >> +if (duration >= 1000) {
> >>  flow_limit /= duration / 1000;
> >> -} else if (duration > 1300) {
> >> -flow_limit = flow_limit * 3 / 4;
> >> -} else if (duration < 1000 &&
> >> -   flow_limit < n_flows * 1000 / duration) {
> >> -flow_limit += 1000;
> >> +} else {
> >> +flow_limit *= 1000 / duration;
> >>  }
> >>  flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));
> >>  atomic_store_relaxed(>flow_limit, flow_limit);
> >
> >The above is very abrupt.  It always tries to adjust the flow limit
> >upward or downward.  I think that this is a bad idea, especially in the
> >upward direction.  If there are only a few flows, which only take a few
> >milliseconds to revalidate, then it will keep increasing the flow limit
> >upward until it overflows the range of unsigned int.  It will happen
> >very quickly, in fact: if duration is 1 ms three times in a row, then we
> >will multiply flow_limit by 1,000,000,000 and overflow 32-bit UINT_MAX;
> >if it happens six times in a row, we will overflow 64-bit.
> 
> >
> Hi,Ben, 
> Thanks for your review. 
> It won't overflow, because of the following line of 
> code(https://github.com/openvswitch/ovs/blob/master/ofproto/ofproto-dpif-upcall.c#L974),which
>  makes it won't over   `ofproto_flow_limit` .
> For example, if currently flow-limit is 200,000(the default 
> ofproto_flow_limit), and duration is 1ms, it will become 200,000,000 ,which 
> is much less than 32-bit UINT_MAX(4,294,967,295).  
> And then,  it wil be back to 200,000,because of the limtitaion mentioned 
> above, I think this is important.

OK.  Is there anything that ensures that ofproto_flow_limit is no more
than UINT_MAX/1000? Otherwise we still have the problem, if
ofproto_flow_limit is set high.

> >Furthermore, it doesn't work well even if we have longer durations.  If
> >revalidation takes 501 ms, then we can adjust the flow_limit upward, but
> >this won't do it.
> 
> 
> emm...though，it won't change when `501ms`， but it will change when `500ms` or 
> times of 2 or 5 .
> If you think it's needed to make  process more linear, we can change its type 
> from int to float. 
> what do you think?

I think that more linear makes more sense.  Always adjusting by a factor
of 2 will probably cause oscillation.

> >On the downward direction, this new code does nothing if the duration is
> >less than 2 seconds.  We want to aim for 1-second revalidation times.
> 
> >
> 
> 
> In this code ,I changed the benchmark value from 2s and 1.3s to 1s. It aims 
> for 1-second revalidation times. 
> Do I misunderstand your point?  please let me know , thanks. The following is 
> the new code. 
> 
> 
>  if (duration >= 1000) {
>  flow_limit /= duration / 1000;
>  } else {
>  flow_limit *= 1000 / duration;
>  }
>  flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));

This will do nothing if the duration is between 501 ms and 1999 ms, and
if it's outside that range then it will always adjust by a factor of 2
or more.  I think that is too wide a range and I think that the minimum
adjustment factor is too high.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 0/2] Enable support for non-contiguous NUMA nodes

[David Christensen]

Systems such as the IBM POWER9 do not allocate contiguous NUMA nodes,
nor does the DPDK framework require that they be contiguous.  This
patchset enables OVS support for systems with non-contiguous NUMA nodes
and adds additional tests using the "--dummy-numa" parameter to verify
the functionality.

David Christensen (2):
  dpdk: support non-contiguous NUMA nodes for IBM POWER systems
  dpdk: add non-contiguous NUMA node support to auto tests

 lib/dpdk.c|  27 ++---
 lib/ovs-numa.c|  12 +---
 lib/ovs-numa.h|   1 +
 tests/dpif-netdev.at  |  64 +++-
 tests/ofproto-dpif.at | 100 ++--
 tests/pmd.at  | 132 ++
 6 files changed, 186 insertions(+), 150 deletions(-)

-- 
2.27.0

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

Re: [ovs-dev] [PATCH] ovs-actions: Document normal pipeline.

[Ilya Maximets]

On 4/15/21 5:34 AM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff 
> ---
>  lib/ovs-actions.xml | 288 +++-
>  1 file changed, 286 insertions(+), 2 deletions(-)
> 

Hi, Ben.  Thanks for writing this down!
It looks good to me in general.  Few comments inline.

Best regards, Ilya Maximets.

> diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml
> index a2778de4bcd6..de934a244de9 100644
> --- a/lib/ovs-actions.xml
> +++ b/lib/ovs-actions.xml
> @@ -509,7 +509,8 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
> actions=mod_nw_src:1.2.3.4
>  
>Subjects the packet to the device's normal L2/L3 processing.  This
>action is not implemented by all OpenFlow switches, and each switch
> -  implements it differently.
> +  implements it differently.  The section ``The OVS Normal Pipeline''
> +  below documents the OVS implementation.
>  
>  
>  flood
> @@ -582,7 +583,6 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
> actions=mod_nw_src:1.2.3.4
>  OpenFlow allows switches to reject such actions.
>
>  
> -  
>
>  
>Output to the Input Port
> @@ -664,6 +664,290 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
> actions=mod_nw_src:1.2.3.4
>
>  
>  
> +The OVS Normal Pipeline
> +
> +
> +  This section documents how Open vSwitch implements output to the
> +  normal port.  The OpenFlow specification places no
> +  requirements on how this port works, so all of this documentation is
> +  specific to Open vSwitch.
> +
> +
> +
> +  Open vSwitch uses the Open_vSwitch database, detailed in
> +  ovs-vswitchd.conf.db(5), to determine the details of the
> +  normal pipeline.
> +
> +
> +
> +  The normal pipeline executes the following ingress stages for each
> +  packet.  The result of the ingress stages is a set of output ports, 
> which
> +  is the empty set if some ingress stage drops the packet:
> +
> +
> +
> +  
> +
> +  Input port lookup: Looks up the OpenFlow
> +  in_port field's value to the corresponding
> +  Port and Interface record in the 
> database.
> +
> +
> +
> +  The in_port is normally the OpenFlow port that the
> +  packet was received on.  If set_field or another 
> actions
> +  changes the in_port, the updated value is honored.  
> This
> +  lookup will ordinarily succeed; if it fails, for example because
> +  in_port was changed to an unknown value, then the 
> normal
> +  pipeline exits.
> +
> +  
> +
> +  
> +Drop malformed packet: If the packet is malformed enough that 
> it
> +contains only part of an 802.1Q header, then the normal pipeline 
> exits
> +error.

Should it be "exits with error"?

> +  
> +
> +  
> +Drop packets sent to a port reserved for mirroring: If the
> +packet was received on a port that is configured as the output port 
> for
> +a mirror (that is, it is the output_port in some
> +Mirror record), then the normal pipeline exits.  Ports
> +used as mirror outputs don't accept any packets.
> +  
> +
> +  
> +
> +  VLAN input processing: This stage determines what VLAN the
> +  packet is in.  It also verifies that this VLAN is valid for the 
> port;
> +  if not, the normal pipeline exits.  How the VLAN is determined and
> +  which ones are valid vary based on the vlan-mode in 
> the
> +  input port's Port record:
> +
> +
> +
> +  trunk
> +  
> +The packet is in the VLAN specified in its 802.1Q header, or in
> +VLAN 0 if there is no 802.1Q header.  The trunks
> +column in the Port record lists the valid VLANs; if 
> it
> +is empty, all VLANs are valid.
> +  
> +
> +  access
> +  
> +The packet is in the VLAN specified in the tag 
> column
> +of its Port record.  The packet must not have an
> +802.1Q header with a nonzero VLAN ID; if it does, the pipeline
> +exits.
> +  
> +
> +  native-tagged
> +  native-untagged
> +  
> +Same as trunk except that the VLAN of a packet 
> without
> +an 802.1Q header is not necessarily zero; instead, it is taken 
> from
> +the tag column.
> +  
> +
> +  dot1q-tunnel
> +  
> +The packet is in the VLAN specified in the tag 
> column
> +of its Port record, which is a QinQ service VLAN 
> with
> +the Ethertype specified by the Port's
> +other_config : qinq-ethtype.  If the
> +packet has an 802.1Q header, then it specifies the customer VLAN.
> +The cvlans column specifies the valid customer 
> VLANs;
> +   

[ovs-dev] [PATCH 1/2] dpdk: support non-contiguous NUMA nodes for IBM POWER systems

[David Christensen]

NUMA nodes are not guaranteed to be continguous on IBM POWER systems,
nor are all NUMA nodes guaranteed to have CPUs attached to them.  For
example:

$ lscpu
Architecture:ppc64le
Byte Order:  Little Endian
CPU(s):  128
On-line CPU(s) list: 0-127
Thread(s) per core:  4
Core(s) per socket:  16
Socket(s):   2
NUMA node(s):6
Model:   2.3 (pvr 004e 1203)
Model name:  POWER9, altivec supported
CPU max MHz: 3800.
CPU min MHz: 2300.
L1d cache:   32K
L1i cache:   32K
L2 cache:512K
L3 cache:10240K
NUMA node0 CPU(s):   0-63
NUMA node8 CPU(s):   64-127
NUMA node252 CPU(s):
NUMA node253 CPU(s):
NUMA node254 CPU(s):
NUMA node255 CPU(s):

Modify NUMA node detection logic to allow non-contiguous NUMA nodes.

Signed-off-by: David Christensen 
---
 lib/dpdk.c | 27 ++-
 lib/ovs-numa.c |  4 +---
 lib/ovs-numa.h |  1 +
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 319540394..dd1816427 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -133,18 +133,27 @@ static char *
 construct_dpdk_socket_mem(void)
 {
 const char *def_value = "1024";
-int numa, numa_nodes = ovs_numa_get_n_numas();
-struct ds dpdk_socket_mem = DS_EMPTY_INITIALIZER;
-
-if (numa_nodes == 0 || numa_nodes == OVS_NUMA_UNSPEC) {
-numa_nodes = 1;
-}
+struct ovs_numa_dump *dump;
+int last_node = 0;
 
+/* Build a list of all numa nodes with at least one core */
+dump = ovs_numa_dump_n_cores_per_numa(1);
+struct ds dpdk_socket_mem = DS_EMPTY_INITIALIZER;
 ds_put_cstr(_socket_mem, def_value);
-for (numa = 1; numa < numa_nodes; ++numa) {
-ds_put_format(_socket_mem, ",%s", def_value);
-}
 
+const struct ovs_numa_info_numa *node;
+FOR_EACH_NUMA_ON_DUMP(node,dump) {
+while (node->numa_id > last_node+1 &&
+   node->numa_id != OVS_NUMA_UNSPEC &&
+   node->numa_id <= MAX_NUMA_NODES){
+ds_put_format(_socket_mem, ",%s", "0");
+++last_node;
+}
+if (node->numa_id != 0) {
+ds_put_format(_socket_mem, ",%s", def_value);
+}
+}
+ovs_numa_dump_destroy(dump);
 return ds_cstr(_socket_mem);
 }
 
diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index 6d0a68522..6f85d7023 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -56,8 +56,6 @@ VLOG_DEFINE_THIS_MODULE(ovs_numa);
  * TODO: Fix ovs-numa when cpu hotplug is used.
  */
 
-#define MAX_NUMA_NODES 128
-
 /* numa node. */
 struct numa_node {
 struct hmap_node hmap_node; /* In the 'all_numa_nodes'. */
@@ -229,7 +227,7 @@ discover_numa_and_core(void)
 }
 
 free(path);
-if (!dir || !numa_supported) {
+if (!numa_supported) {
 break;
 }
 }
diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h
index 8f2ea3430..7d4b5af4c 100644
--- a/lib/ovs-numa.h
+++ b/lib/ovs-numa.h
@@ -25,6 +25,7 @@
 
 #define OVS_CORE_UNSPEC INT_MAX
 #define OVS_NUMA_UNSPEC INT_MAX
+#define MAX_NUMA_NODES 128
 
 /* Dump of a list of 'struct ovs_numa_info'. */
 struct ovs_numa_dump {
-- 
2.27.0

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

[ovs-dev] [PATCH 2/2] dpdk: add non-contiguous NUMA node support to auto tests

[David Christensen]

Remove explicit "--dummy-numa" check for contiguous NUMA nodes, extend
existing auto tests with an additional "--dummy-numa" configuration
that uses non-contiguous NUMA nodes.

Signed-off-by: David Christensen 
---
 lib/ovs-numa.c|   8 +--
 tests/dpif-netdev.at  |  64 +++-
 tests/ofproto-dpif.at | 100 ++--
 tests/pmd.at  | 132 ++
 4 files changed, 166 insertions(+), 138 deletions(-)

diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index 6f85d7023..ba12ccec5 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -128,8 +128,8 @@ insert_new_cpu_core(struct numa_node *n, unsigned core_id)
  * - "0,0,0,0": four cores on numa socket 0.
  * - "0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1": 16 cores on two numa sockets.
  * - "0,0,0,0,1,1,1,1": 8 cores on two numa sockets.
- *
- * The different numa ids must be consecutives or the function will abort. */
+ * - "0,0,0,0,8,8,8,8": 8 cores on two numa sockets, non-contiguous.
+ */
 static void
 discover_numa_and_core_dummy(void)
 {
@@ -166,10 +166,6 @@ discover_numa_and_core_dummy(void)
 }
 
 free(conf);
-
-if (max_numa_id + 1 != hmap_count(_numa_nodes)) {
-ovs_fatal(0, "dummy numa contains non consecutive numa ids");
-}
 }
 
 /* Discovers all numa nodes and the corresponding cpu cores.
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 57cae383f..d519d7a9f 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -85,7 +85,7 @@ AT_CLEANUP
 
 
 m4_define([DPIF_NETDEV_DUMMY_IFACE],
-  [AT_SETUP([dpif-netdev - $1 interface])
+  [AT_SETUP([dpif-netdev - $1 interface $2])
# Create br0 with interfaces p1 and p7
#and br1 with interfaces p2 and p8
# with p1 and p2 connected via unix domain socket
@@ -98,7 +98,7 @@ m4_define([DPIF_NETDEV_DUMMY_IFACE],
  fail-mode=secure -- \
   add-port br1 p2 -- set interface p2 type=$1 
options:stream=unix:$OVS_RUNDIR/p0.sock ofport_request=2 -- \
   add-port br1 p8 -- set interface p8 ofport_request=8 type=$1 --], [], [],
-  [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
+  [m4_if([$2], [(consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,1,1,1,1"], [$2], [(non-consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,8,8,8,8"], [])])
AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
 
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
@@ -120,17 +120,18 @@ 
recirc_id(0),in_port(8),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:
OVS_VSWITCHD_STOP
AT_CLEANUP])
 
-DPIF_NETDEV_DUMMY_IFACE([dummy])
-DPIF_NETDEV_DUMMY_IFACE([dummy-pmd])
+DPIF_NETDEV_DUMMY_IFACE([dummy], [])
+DPIF_NETDEV_DUMMY_IFACE([dummy-pmd], [(consecutive NUMA nodes)])
+DPIF_NETDEV_DUMMY_IFACE([dummy-pmd], [(non-consecutive NUMA nodes)])
 
 m4_define([DPIF_NETDEV_MISS_FLOW_INSTALL],
-  [AT_SETUP([dpif-netdev - miss upcall key matches flow_install - $1])
+  [AT_SETUP([dpif-netdev - miss upcall key matches flow_install - $1 $2])
OVS_VSWITCHD_START(
  [add-port br0 p1 \
   -- set interface p1 type=$1 options:pstream=punix:$OVS_RUNDIR/p0.sock \
   -- set bridge br0 datapath-type=dummy \
 other-config:datapath-id=1234 fail-mode=secure], [], 
[],
-  [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
+  [m4_if([$2], [(consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,1,1,1,1"], [$2], [(non-consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,8,8,8,8"], [])])
AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
 
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
@@ -162,17 +163,18 @@ 
skb_priority(0),skb_mark(0),ct_state(-new-est-rel-rpl-inv-trk-snat-dnat),ct_zone
OVS_VSWITCHD_STOP
AT_CLEANUP])
 
-DPIF_NETDEV_MISS_FLOW_INSTALL([dummy])
-DPIF_NETDEV_MISS_FLOW_INSTALL([dummy-pmd])
+DPIF_NETDEV_MISS_FLOW_INSTALL([dummy], [])
+DPIF_NETDEV_MISS_FLOW_INSTALL([dummy-pmd], [(consecutive NUMA nodes)])
+DPIF_NETDEV_MISS_FLOW_INSTALL([dummy-pmd], [(non-consecutive NUMA nodes)])
 
 m4_define([DPIF_NETDEV_FLOW_PUT_MODIFY],
-  [AT_SETUP([dpif-netdev - datapath flow modification - $1])
+  [AT_SETUP([dpif-netdev - datapath flow modification - $1 $2])
OVS_VSWITCHD_START(
  [add-port br0 p1 -- set interface p1 type=$1 ofport_request=1 
options:pstream=punix:$OVS_RUNDIR/p1.sock -- \
   add-port br0 p2 -- set interface p2 type=$1 ofport_request=2 
options:pstream=punix:$OVS_RUNDIR/p2.sock -- \
   set bridge br0 datapath-type=dummy \
  other-config:datapath-id=1234 fail-mode=secure], [], [],
-  [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
+  [m4_if([$2], [(consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,1,1,1,1"], [$2], [(non-consecutive NUMA nodes)], 
[--dummy-numa="0,0,0,0,8,8,8,8"], [])])
AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
# Add a flow that directs some packets received on p1 to p2 and the
@@ -215,18 

Re: [ovs-dev] [PATCH] rhel: use /run instead of /var/run

[0-day Robot]

Bleep bloop.  Greetings Timothy Redaelli, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 81 characters long (recommended limit is 79)
#103 FILE: rhel/usr_lib_systemd_system_ovsdb-server.service:21:
ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /run/openvswitch 
/var/log/openvswitch

Lines checked: 109, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: use /run instead of /var/run

[Timothy Redaelli]

Systemd unit file generates warnings about PID file path since /var/run
is a legacy path so just use /run instead of /var/run.

/var/run is a symlink of /run starting from RHEL7 (and any other distribution
that uses systemd).

Reported-at: https://bugzilla.redhat.com/1952081
Signed-off-by: Timothy Redaelli 
---
 rhel/etc_logrotate.d_openvswitch| 4 ++--
 rhel/usr_lib_systemd_system_openvswitch-ipsec.service   | 2 +-
 ...sr_lib_systemd_system_ovs-delete-transient-ports.service | 2 +-
 rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 6 +++---
 rhel/usr_lib_systemd_system_ovsdb-server.service| 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index f4302ffbc..c0f476744 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -13,8 +13,8 @@
 missingok
 postrotate
 # Tell Open vSwitch daemons to reopen their log files
-if [ -d /var/run/openvswitch ]; then
-for ctl in /var/run/openvswitch/*.ctl; do
+if [ -d /run/openvswitch ]; then
+for ctl in /run/openvswitch/*.ctl; do
 ovs-appctl -t "$ctl" vlog/reopen 2>/dev/null || :
 done
 fi
diff --git a/rhel/usr_lib_systemd_system_openvswitch-ipsec.service 
b/rhel/usr_lib_systemd_system_openvswitch-ipsec.service
index d8f47af68..92dad44f9 100644
--- a/rhel/usr_lib_systemd_system_openvswitch-ipsec.service
+++ b/rhel/usr_lib_systemd_system_openvswitch-ipsec.service
@@ -5,7 +5,7 @@ After=openvswitch.service
 
 [Service]
 Type=forking
-PIDFile=/var/run/openvswitch/ovs-monitor-ipsec.pid
+PIDFile=/run/openvswitch/ovs-monitor-ipsec.pid
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
 --ike-daemon=libreswan start-ovs-ipsec
 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop-ovs-ipsec
diff --git a/rhel/usr_lib_systemd_system_ovs-delete-transient-ports.service 
b/rhel/usr_lib_systemd_system_ovs-delete-transient-ports.service
index 4cd4d7f57..d4d7b204b 100644
--- a/rhel/usr_lib_systemd_system_ovs-delete-transient-ports.service
+++ b/rhel/usr_lib_systemd_system_ovs-delete-transient-ports.service
@@ -2,7 +2,7 @@
 Description=Open vSwitch Delete Transient Ports
 After=ovsdb-server.service
 Before=ovs-vswitchd.service
-AssertPathExists=/var/run/openvswitch/db.sock
+AssertPathExists=/run/openvswitch/db.sock
 
 [Service]
 Type=oneshot
diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in 
b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
index ff43dae96..6d021618b 100644
--- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
+++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
@@ -4,14 +4,14 @@ After=ovsdb-server.service network-pre.target 
systemd-udev-settle.service
 Before=network.target network.service
 Requires=ovsdb-server.service
 ReloadPropagatedFrom=ovsdb-server.service
-AssertPathIsReadWrite=/var/run/openvswitch/db.sock
+AssertPathIsReadWrite=/run/openvswitch/db.sock
 PartOf=openvswitch.service
 
 [Service]
 Type=forking
-PIDFile=/var/run/openvswitch/ovs-vswitchd.pid
+PIDFile=/run/openvswitch/ovs-vswitchd.pid
 Restart=on-failure
-Environment=XDG_RUNTIME_DIR=/var/run/openvswitch
+Environment=XDG_RUNTIME_DIR=/run/openvswitch
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
 EnvironmentFile=-/run/openvswitch.useropts
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index ed6419f31..558632320 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -7,7 +7,7 @@ PartOf=openvswitch.service
 
 [Service]
 Type=forking
-PIDFile=/var/run/openvswitch/ovsdb-server.pid
+PIDFile=/run/openvswitch/ovsdb-server.pid
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
@@ -18,7 +18,7 @@ EnvironmentFile=-/run/openvswitch.useropts
 # OVS_USER_ID from default.conf or sysconfig.
 ExecStartPre=/usr/bin/rm -f /run/openvswitch.useropts
 
-ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
/var/log/openvswitch
+ExecStartPre=-/usr/bin/chown ${OVS_USER_ID} /run/openvswitch 
/var/log/openvswitch
 ExecStartPre=/bin/sh -c '/usr/bin/echo "OVS_USER_ID=${OVS_USER_ID}" > 
/run/openvswitch.useropts'
 ExecStartPre=/bin/sh -c 'if [ "$${OVS_USER_ID/:*/}" != "root" ]; then 
/usr/bin/echo "OVS_USER_OPT=--ovs-user=${OVS_USER_ID}" >> 
/run/openvswitch.useropts; fi'
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
-- 
2.31.1

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

Re: [ovs-dev] [PATCH] ovsdb-server: Add limitation for ovsdb-server remotes

[Ilya Maximets]

On 1/11/21 2:27 PM, wangyunjian wrote:
>> -Original Message-
>> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
>> Sent: Monday, January 11, 2021 8:40 PM
>> To: wangyunjian ; d...@openvswitch.org
>> Cc: i.maxim...@ovn.org; Lilijun (Jerry) ; xudingke
>> ; chenchanghu 
>> Subject: Re: [ovs-dev] [PATCH] ovsdb-server: Add limitation for ovsdb-server
>> remotes
>>
>> On 12/22/20 12:31 PM, wangyunjian wrote:
>>> From: Yunjian Wang 
>>>
>>> Currently there is no limit to add ovsdb-server remotes, which will
>>> cause all FDs maybe be consumed when we always call
>>> ovsdb_server_add_remote() function. And as a result, other connections
>>> cannot be created. To fix this issue, we can add limitation for
>>> ovsdb-server remotes. It's limited to 64, witch is just an empirical
>>> value.
>>
>> Hi.  Why do you need so many remotes?
>> And why not removing ones that you don't need?
> 
> This issue is caused by pressure tests. The remotes are
> continuously created. The ovsdb-server service is abnormal
> and cannot be recovered.

Thinking more about this, it makes some sense to limit number of
remotes, but this patch only limits it in one place.  Commonly,
some database table is used as a source of remotes for an ovsdb
process and this patch doesn't limit number of connections that
could be added to these tables.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] northd: Combine router arp flows.

[Numan Siddique]

On Wed, May 12, 2021 at 8:14 AM Numan Siddique  wrote:
>
> On Fri, May 7, 2021 at 12:23 PM Ilya Maximets  wrote:
> >
> > This flow already matches on 'arp.tpa == ip_address', so instead of
> > setting 'arp.spa' to this 'ip_address' we can just swap values of
> > 'arp.tpa' and 'arp.spa'.
> >
> > This elimintes the variable from the 'action', allowing us to just
> > use the list of all IPs in the match.
> >
> > Current OVN generates one ARP flow for each load balancer IP for
> > each router port.  So, if we have 100 ports and 1000 IP addresses,
> > northd will generate 10^5 ARP flows.  With this change it will
> > generate only 100 of them.
> >
> > In case of ovn-k8s scale test, this change alone reduced the database
> > size from 500 to 200 MB.
> >
> > Also added more test cses for loadbalancer flows.
> >
> > Signed-off-by: Ilya Maximets 
>
> Thanks Ilya for the improvements.
>
> I applied this patch to the main branch with the below changes to
> ovn-northd.8.xml
>
> 
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index b63b69b5d0..bbc28d7ab3 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
>  arp.op = 2; /* ARP reply. */
>  arp.tha = arp.sha;
>  arp.sha = xreg0[0..47];
> -arp.tpa = arp.spa;
> -arp.spa = A;
> +arp.tpa <-> arp.spa;

Oops. Sorry Ilya for goofing up here.
It should have been
arp.tpa - arp.spa;

I fixed this in a separate commit.

Thanks
Numan

>  outport = inport;
>  flags.loopback = 1;
>  output;
> -
>
> Thanks
> Numan
>
> > ---
> >
> > Similar change could be applied to ipv6 ND flows, but it's harder to
> > implement (at least in ddlog), so it's not implemented for now.
> >
> > Alternatively, it's possible to create an address set with all these
> > addtesses and use it in the flow, but I'm not sure if it's needed.
> >
> >  northd/ovn-northd.c  | 20 +++
> >  northd/ovn_northd.dl | 23 +++--
> >  tests/ovn-northd.at  | 81 +++-
> >  tests/ovn.at |  2 +-
> >  4 files changed, 91 insertions(+), 35 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 1d4c5b67b..de8052f21 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -9016,14 +9016,12 @@ build_lrouter_arp_flow(struct ovn_datapath *od, 
> > struct ovn_port *op,
> >"arp.op = 2; /* ARP reply */ "
> >"arp.tha = arp.sha; "
> >"arp.sha = %s; "
> > -  "arp.tpa = arp.spa; "
> > -  "arp.spa = %s; "
> > +  "arp.tpa <-> arp.spa; "
> >"outport = inport; "
> >"flags.loopback = 1; "
> >"output;",
> >eth_addr,
> > -  eth_addr,
> > -  ip_address);
> > +  eth_addr);
> >  }
> >
> >  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
> > @@ -10978,16 +10976,24 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> >  get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
> >
> >  const char *ip_address;
> > -SSET_FOR_EACH (ip_address, _ips_v4) {
> > +if (sset_count(_ips_v4)) {
> >  ds_clear(match);
> >  if (op == op->od->l3dgw_port) {
> >  ds_put_format(match, "is_chassis_resident(%s)",
> >op->od->l3redirect_port->json_key);
> >  }
> >
> > -build_lrouter_arp_flow(op->od, op,
> > -   ip_address, REG_INPORT_ETH_ADDR,
> > +struct ds load_balancer_ips_v4 = DS_EMPTY_INITIALIZER;
> > +
> > +/* For IPv4 we can just create one rule with all required IPs. 
> > */
> > +ds_put_cstr(_balancer_ips_v4, "{ ");
> > +ds_put_and_free_cstr(_balancer_ips_v4,
> > + sset_join(_ips_v4, ", ", " }"));
> > +
> > +build_lrouter_arp_flow(op->od, op, 
> > ds_cstr(_balancer_ips_v4),
> > +   REG_INPORT_ETH_ADDR,
> > match, false, 90, NULL, lflows);
> > +ds_destroy(_balancer_ips_v4);
> >  }
> >
> >  SSET_FOR_EACH (ip_address, _ips_v6) {
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index ffd09c35f..a63da8e66 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -4874,7 +4874,7 @@ relation LogicalRouterArpNdFlow(
> >  extra_match: Option,
> >  drop: bool,
> >  priority: integer)
> > -LogicalRouterArpFlow(router, lrp, ipv4, mac, extra_match, drop, priority,
> > +LogicalRouterArpFlow(router, lrp, "${ipv4}", mac, extra_match, drop, 
> > priority,
> >   stage_hint(nat.nat._uuid)) :-
> >  LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = 

Re: [ovs-dev] [PATCH ovn] northd: Combine router arp flows.

[Numan Siddique]

On Fri, May 7, 2021 at 12:23 PM Ilya Maximets  wrote:
>
> This flow already matches on 'arp.tpa == ip_address', so instead of
> setting 'arp.spa' to this 'ip_address' we can just swap values of
> 'arp.tpa' and 'arp.spa'.
>
> This elimintes the variable from the 'action', allowing us to just
> use the list of all IPs in the match.
>
> Current OVN generates one ARP flow for each load balancer IP for
> each router port.  So, if we have 100 ports and 1000 IP addresses,
> northd will generate 10^5 ARP flows.  With this change it will
> generate only 100 of them.
>
> In case of ovn-k8s scale test, this change alone reduced the database
> size from 500 to 200 MB.
>
> Also added more test cses for loadbalancer flows.
>
> Signed-off-by: Ilya Maximets 

Thanks Ilya for the improvements.

I applied this patch to the main branch with the below changes to
ovn-northd.8.xml


diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b63b69b5d0..bbc28d7ab3 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
-arp.tpa = arp.spa;
-arp.spa = A;
+arp.tpa <-> arp.spa;
 outport = inport;
 flags.loopback = 1;
 output;
-

Thanks
Numan

> ---
>
> Similar change could be applied to ipv6 ND flows, but it's harder to
> implement (at least in ddlog), so it's not implemented for now.
>
> Alternatively, it's possible to create an address set with all these
> addtesses and use it in the flow, but I'm not sure if it's needed.
>
>  northd/ovn-northd.c  | 20 +++
>  northd/ovn_northd.dl | 23 +++--
>  tests/ovn-northd.at  | 81 +++-
>  tests/ovn.at |  2 +-
>  4 files changed, 91 insertions(+), 35 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 1d4c5b67b..de8052f21 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9016,14 +9016,12 @@ build_lrouter_arp_flow(struct ovn_datapath *od, 
> struct ovn_port *op,
>"arp.op = 2; /* ARP reply */ "
>"arp.tha = arp.sha; "
>"arp.sha = %s; "
> -  "arp.tpa = arp.spa; "
> -  "arp.spa = %s; "
> +  "arp.tpa <-> arp.spa; "
>"outport = inport; "
>"flags.loopback = 1; "
>"output;",
>eth_addr,
> -  eth_addr,
> -  ip_address);
> +  eth_addr);
>  }
>
>  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
> @@ -10978,16 +10976,24 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
>  get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
>
>  const char *ip_address;
> -SSET_FOR_EACH (ip_address, _ips_v4) {
> +if (sset_count(_ips_v4)) {
>  ds_clear(match);
>  if (op == op->od->l3dgw_port) {
>  ds_put_format(match, "is_chassis_resident(%s)",
>op->od->l3redirect_port->json_key);
>  }
>
> -build_lrouter_arp_flow(op->od, op,
> -   ip_address, REG_INPORT_ETH_ADDR,
> +struct ds load_balancer_ips_v4 = DS_EMPTY_INITIALIZER;
> +
> +/* For IPv4 we can just create one rule with all required IPs. */
> +ds_put_cstr(_balancer_ips_v4, "{ ");
> +ds_put_and_free_cstr(_balancer_ips_v4,
> + sset_join(_ips_v4, ", ", " }"));
> +
> +build_lrouter_arp_flow(op->od, op, 
> ds_cstr(_balancer_ips_v4),
> +   REG_INPORT_ETH_ADDR,
> match, false, 90, NULL, lflows);
> +ds_destroy(_balancer_ips_v4);
>  }
>
>  SSET_FOR_EACH (ip_address, _ips_v6) {
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index ffd09c35f..a63da8e66 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -4874,7 +4874,7 @@ relation LogicalRouterArpNdFlow(
>  extra_match: Option,
>  drop: bool,
>  priority: integer)
> -LogicalRouterArpFlow(router, lrp, ipv4, mac, extra_match, drop, priority,
> +LogicalRouterArpFlow(router, lrp, "${ipv4}", mac, extra_match, drop, 
> priority,
>   stage_hint(nat.nat._uuid)) :-
>  LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = IPv4{ipv4}}, lrp,
> mac, extra_match, drop, priority).
> @@ -4886,7 +4886,7 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, 
> mac, extra_match, drop, pr
>  relation LogicalRouterArpFlow(
>  lr: Ref,
>  lrp: Option,
> -ip: in_addr,
> +ip: string,
>  mac: string,
>  extra_match: Option,
>  drop: bool,
> @@ -4919,8 +4919,7 @@ Flow(.logical_datapath = lr.lr._uuid,
>  

Re: [ovs-dev] [PATCH] dpif-netdev: Remove meter rate from the bucket size calculation.

[Tonghao Zhang]

On Wed, Apr 21, 2021 at 9:48 PM Ilya Maximets  wrote:
>
> Implementation of meters supposed to be a classic token bucket with 2
> typical parameters: rate and burst size.
>
> Burst size in this schema is the maximum number of bytes/packets that
> could pass without being rate limited.
>
> Recent changes to userspace datapath made meter implementation to be
> in line with the kernel one, and this uncovered several issues.
>
> The main problem is that maximum bucket size for unknown reason
> accounts not only burst size, but also the numerical value of rate.
> This creates a lot of confusion around behavior of meters.
>
> For example, if rate is configured as 1000 pps and burst size set to 1,
> this should mean that meter will tolerate bursts of 1 packet at most,
> i.e. not a single packet above the rate should pass the meter.
> However, current implementation calculates maximum bucket size as
> (rate + burst size), so the effective bucket size will be 1001.  This
> means that first 1000 packets will not be rate limited and average
> rate might be twice as high as the configured rate.  This also makes
> it practically impossible to configure meter that will have burst size
> lower than the rate, which might be a desirable configuration if the
> rate is high.
>
> Inability to configure low values of a burst size and overall inability
> for a user to predict what will be a maximum and average rate from the
> configured parameters of a meter without looking at the OVS and kernel
> code might be also classified as a security issue, because drop meters
> are frequently used as a way of protection from DoS attacks.
>
> This change removes rate from the calculation of a bucket size, making
> it in line with the classic token bucket algorithm and essentially
> making the rate and burst tolerance being predictable from a users'
> perspective.
>
> Same change will be proposed for the kernel implementation.
> Unit tests changed back to their correct version and enhanced.
>
> Signed-off-by: Ilya Maximets 
Reviewed-by: Tonghao Zhang 

Thanks!
> ---
>  lib/dpif-netdev.c |  5 ++--
>  tests/dpif-netdev.at  | 53 +++
>  tests/ofproto-dpif.at |  2 +-
>  3 files changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 251788b04..650e67ab3 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -6229,7 +6229,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
> dp_packet_batch *packets_,
>  uint64_t max_bucket_size;
>
>  band = >bands[m];
> -max_bucket_size = (band->rate + band->burst_size) * 1000ULL;
> +max_bucket_size = band->burst_size * 1000ULL;
>  /* Update band's bucket. */
>  band->bucket += (uint64_t) delta_t * band->rate;
>  if (band->bucket > max_bucket_size) {
> @@ -6357,8 +6357,7 @@ dpif_netdev_meter_set(struct dpif *dpif, 
> ofproto_meter_id meter_id,
>  meter->bands[i].rate = config->bands[i].rate;
>  meter->bands[i].burst_size = config->bands[i].burst_size;
>  /* Start with a full bucket. */
> -meter->bands[i].bucket =
> -(meter->bands[i].burst_size + meter->bands[i].rate) * 1000ULL;
> +meter->bands[i].bucket = meter->bands[i].burst_size * 1000ULL;
>
>  /* Figure out max delta_t that is enough to fill any bucket. */
>  band_max_delta_t
> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> index 57cae383f..16402ebae 100644
> --- a/tests/dpif-netdev.at
> +++ b/tests/dpif-netdev.at
> @@ -314,21 +314,21 @@ done
>  sleep 1  # wait for forwarders process packets
>
>  # Meter 1 is measuring packets, allowing one packet per second with
> -# bursts of one packet, so 3 out of 5 packets should hit the drop
> -# band.
> -# Meter 2 is measuring kbps, with burst size 2 (== 3000 bits). 6 packets
> -# (360 bytes == 2880 bits) pass, but the last packet should hit the drop 
> band.
> +# bursts of one packet, so 4 out of 5 packets should hit the drop band.
> +# Meter 2 is measuring kbps, with burst size 2 (== 2000 bits). 4 packets
> +# (240 bytes == 1920 bits) pass, but the last three packets should hit the
> +# drop band.  There should be 80 bits remaining for the next packets.
>  AT_CHECK([ovs-ofctl -O OpenFlow13 meter-stats br0 | strip_timers], [0], [dnl
>  OFPST_METER reply (OF1.3) (xid=0x2):
>  meter:1 flow_count:1 packet_in_count:5 byte_in_count:300 duration:0.0s bands:
> -0: packet_count:3 byte_count:180
> +0: packet_count:4 byte_count:240
>
>  meter:2 flow_count:1 packet_in_count:7 byte_in_count:420 duration:0.0s bands:
> -0: packet_count:1 byte_count:60
> +0: packet_count:3 byte_count:180
>  ])
>
> -# Advance time by 1/2 second
> -ovs-appctl time/warp 500
> +# Advance time by 870 ms
> +ovs-appctl time/warp 870
>
>  for i in `seq 1 5`; do
>AT_CHECK(
> @@ -345,16 +345,41 @@ sleep 1  # wait for forwarders process packets
>  # Meter 1 is measuring packets, allowing one packet per second with

Re: [ovs-dev] [PATCH dpdk-latest 2/6] travis: Check compilation with DPDK experimental API.

[Stokes, Ian]

> On 06/05/2021 16:25, David Marchand wrote:
> > Add Travis jobs to check compilation with DPDK experimental API enabled.
> > This will help us catch issues for the day we need one of them.
> >
> > Note: this should not be merged to master, intended for dpdk-latest
> > branch only.
> >
> > Signed-off-by: David Marchand 
> > Signed-off-by: Ian Stokes 
> > ---
> >  .ci/linux-build.sh   |  8 +++-
> >  .github/workflows/build-and-test.yml | 10 +++---
> >  .travis.yml  |  3 +++
> >  3 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> > index 977449350b..c8621201ab 100755
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -7,6 +7,9 @@ CFLAGS_FOR_OVS="-g -O2"
> >  SPARSE_FLAGS=""
> >  EXTRA_OPTS="--enable-Werror"
> >
> > +[ -z "$DPDK_EXPERIMENTAL" ] || DPDK=1
> > +[ -z "$DPDK_SHARED" ] || DPDK=1
> > +
> >  function install_kernel()
> >  {
> >  if [[ "$1" =~ ^5.* ]]; then
> > @@ -199,7 +202,7 @@ if [ "$KERNEL" ]; then
> >  install_kernel $KERNEL
> >  fi
> >
> > -if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> > +if [ "$DPDK" ]; then
> >  if [ -z "$DPDK_VER" ]; then
> >  DPDK_VER="20.11"
> >  fi
> > @@ -208,6 +211,9 @@ if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> >  # Disregard cast alignment errors until DPDK is fixed
> >  CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align"
> >  fi
> > +if [ -n "$DPDK_EXPERIMENTAL" ]; then
> > +CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -
> DALLOW_EXPERIMENTAL_API"
> > +fi
> >  fi
> >
> >  if [ "$CC" = "clang" ]; then
> > diff --git a/.github/workflows/build-and-test.yml
> b/.github/workflows/build-and-test.yml
> > index ce98a9f98f..649b1405d2 100644
> > --- a/.github/workflows/build-and-test.yml
> > +++ b/.github/workflows/build-and-test.yml
> > @@ -17,6 +17,7 @@ jobs:
> >DEB_PACKAGE: ${{ matrix.deb_package }}
> >DPDK:${{ matrix.dpdk }}
> >DPDK_SHARED: ${{ matrix.dpdk_shared }}
> > +  DPDK_EXPERIMENTAL: ${{ matrix.dpdk_experimental }}
> >KERNEL:  ${{ matrix.kernel }}
> >KERNEL_LIST: ${{ matrix.kernel_list }}
> >LIBS:${{ matrix.libs }}
> > @@ -102,6 +103,9 @@ jobs:
> >  dpdk_shared:  dpdk-shared
> >  opts: --enable-shared
> >
> > +  - compiler: gcc
> > +dpdk_shared:  dpdk-experimental
> > +
> >- compiler: gcc
> >  m32:  m32
> >  opts: --disable-ssl
> > @@ -124,7 +128,7 @@ jobs:
> >  python-version: '3.x'
> >
> >  - name: create ci signature file for the dpdk cache key
> > -  if:   matrix.dpdk != '' || matrix.dpdk_shared != ''
> > +  if:   matrix.dpdk != '' || matrix.dpdk_shared != '' ||
> matrix.dpdk_experimental != ''
> ># This will collect most of DPDK related lines, so hash will be 
> > different
> ># if something changed in a way we're building DPDK including
> DPDK_VER.
> ># This also allows us to use cache from any branch as long as version
> > @@ -134,10 +138,10 @@ jobs:
> >  cat dpdk-ci-signature
> >
> >  - name: cache
> > -  if:   matrix.dpdk != '' || matrix.dpdk_shared != ''
> > +  if:   matrix.dpdk != '' || matrix.dpdk_shared != '' ||
> matrix.dpdk_experimental != ''
> >uses: actions/cache@v2
> >env:
> > -matrix_key: ${{ matrix.dpdk }}${{ matrix.dpdk_shared }}
> > +matrix_key: ${{ matrix.dpdk }}${{ matrix.dpdk_shared }}${{
> matrix.dpdk_experimental}}
> 
> minor, but fyi checkpatch is complaining about these long lines:
> 
> WARNING: Line is 91 characters long (recommended limit is 79)
> #64 FILE: .github/workflows/build-and-test.yml:131:
>   if:   matrix.dpdk != '' || matrix.dpdk_shared != '' ||
> matrix.dpdk_experimental != ''
> 
> WARNING: Line is 91 characters long (recommended limit is 79)
> #71 FILE: .github/workflows/build-and-test.yml:141:
>   if:   matrix.dpdk != '' || matrix.dpdk_shared != '' ||
> matrix.dpdk_experimental != ''
> 
> WARNING: Line is 93 characters long (recommended limit is 79)
> #75 FILE: .github/workflows/build-and-test.yml:144:
> matrix_key: ${{ matrix.dpdk }}${{ matrix.dpdk_shared }}${{
> matrix.dpdk_experimental}}
> 

I spotted this as well, however I'm torn, for readability is it easier to keep 
the same line? I wouldn't be an expert on yml, is there an easy and nice 
looking way to split these?

IMO, as this is just for dpdk-latest its acceptable as is above.

BR
Ian
> 
> >  ci_key: ${{ hashFiles('dpdk-ci-signature') }}
> >with:
> >  path: dpdk-dir
> > diff --git a/.travis.yml b/.travis.yml
> > index 7e87360256..7f4d5d99a3 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -39,6 +39,9 @@ matrix:
> >  - arch: arm64
> >compiler: gcc
> >env: TESTSUITE=1 DPDK=1
> > +- arch: arm64
> > +  compiler: gcc
> > +  env: 

Re: [ovs-dev] [PATCH dpdk-latest 1/6] travis: Switch to dpdk main branch.

[Stokes, Ian]

> On 06/05/2021 16:25, David Marchand wrote:
> > Make this branch point to current main master branch so that we can
> > track API breakage.
> >
> 
> s/master//

I can strip out master above and keep it as main on commit.
 
> 
> > Note: this should not be merged to master, intended for dpdk-latest
> > branch only.
> >
> 
> It's probably better to give full project/branch names when branches
> from dpdk and ovs are referred to in the commit log. e.g. "dpdk main
> branch". It would get confusing for someone not familiar.

Agreed, can make this change on commit.

> 
> > Signed-off-by: David Marchand 
> > Acked-by: Ilya Maximets 
> > Signed-off-by: Ian Stokes 
> >
> > The default branch name in DPDK is changed from master to main.
> > This patch reflects the same on travis builds for dpdk-latest branch.
> >
> > Tested-at: https://travis-ci.org/github/Sunil-Pai-G/ovs-
> copy/builds/723223426
> 
> "We couldn't display the repository Sunil-Pai-G/ovs-copy".

Yes looks like this isn't available anymore, also this points to travis-ci.org, 
this is set to be removed completely in a few weeks and changed to 
travis-ci.com.

As such it probably doesn't make sense to have the tested by tag above as is. 
As this will never be in an release we could just remove it, otherwise wait for 
a new tag from travis-ci.com.

As the change is small I thinks its ok to remove? Thoughts?

> 
> > Signed-off-by: Sunil Pai G 
> > Signed-off-by: Ian Stokes 
> 
> It's a very small patch to be squashed patches with multiple signoffs?
> Ignore comment if commit log makes sense, but checkpatch is complaining
> too.
> 
> WARNING: Unexpected sign-offs from developers who are not authors or
> co-authors or committers: Ian Stokes , Sunil Pai G
> , Ian Stokes 
> Lines checked: 36, Warnings: 1, Errors: 0

I think it's minor, can add a co-author and sign off for Sunil here to avoid 
the error? As David has done the rebase maybe that’s the correct approach?


BR 
Ian
> 
> > ---
> >  .travis.yml | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/.travis.yml b/.travis.yml
> > index 51d0511080..7e87360256 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -30,7 +30,9 @@ addons:
> >
> >  before_install: ./.ci/${TRAVIS_OS_NAME}-prepare.sh
> >
> > -before_script: export PATH=$PATH:$HOME/bin
> > +before_script:
> > +  - export PATH=$PATH:$HOME/bin
> > +  - export DPDK_VER=refs/heads/main
> >
> >  matrix:
> >include:
> >

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

Re: [ovs-dev] [PATCH branch-2.13 v2] dpdk: Use DPDK 19.11.8 release

[Stokes, Ian]

> > -Original Message-
> > From: Kevin Traynor 
> > Sent: Monday, April 26, 2021 8:36 PM
> > To: Govindharajan, Hariprasad ;
> > d...@openvswitch.org
> > Cc: Christian Ehrhardt ; Pai G, Sunil
> > 
> > Subject: Re: [ovs-dev] [PATCH branch-2.13 v2] dpdk: Use DPDK 19.11.8
> > release
> >
> > +cc Christian/Sunil
> >
> > On 23/04/2021 13:16, Hariprasad Govindharajan wrote:
> > > Modify ci linux build script to use the latest DPDK stable release.
> > > Modify Documentation to use the latest DPDK stable release 19.11.8
> > > Update NEWS file to reflect the latest DPDK stable release.
> > >
> > > Building DPDK releases 19.11.6 and 19.11.7,with meson was found to
> > > break the OvS build.
> > > DPDK 19.11.8 was released to fix the above said issue.
> > > Link to the discussion about the issue can be found below:
> > > http://mails.dpdk.org/archives/stable/2021-April/029786.html
> > >
> > > Signed-off-by: Hariprasad Govindharajan
> > > 
> > >
> > > ---
> > > v2:
> > > updated the commit message based on review comments
> > > ---
> > >  .ci/linux-build.sh   | 2 +-
> > >  Documentation/faq/releases.rst   | 2 +-
> > >  Documentation/intro/install/dpdk.rst | 8 
> > >  Documentation/topics/dpdk/vhost-user.rst | 6 +++---
> > >  NEWS | 2 +-
> > >  5 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > c1cf85d..b22ad95 100755
> > > --- a/.ci/linux-build.sh
> > > +++ b/.ci/linux-build.sh
> > > @@ -182,7 +182,7 @@ fi
> > >
> > >  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> > >  if [ -z "$DPDK_VER" ]; then
> > > -DPDK_VER="19.11.6"
> > > +DPDK_VER="19.11.8"
> > >  fi
> > >  install_dpdk $DPDK_VER
> > >  # Enable pdump support in OVS.
> > > diff --git a/Documentation/faq/releases.rst
> > > b/Documentation/faq/releases.rst index 4a0dcac..70a0e9b 100644
> > > --- a/Documentation/faq/releases.rst
> > > +++ b/Documentation/faq/releases.rst
> > > @@ -192,7 +192,7 @@ Q: What DPDK version does each Open vSwitch
> > release work with?
> > >  2.10.x   17.11.10
> > >  2.11.x   18.11.11
> > >  2.12.x   18.11.11
> > > -2.13.x   19.11.6
> > > +2.13.x   19.11.8
> > >   
> > >
> > >  Q: Are all the DPDK releases that OVS versions work with maintained?
> > > diff --git a/Documentation/intro/install/dpdk.rst
> > > b/Documentation/intro/install/dpdk.rst
> > > index 050e554..b603b6f 100644
> > > --- a/Documentation/intro/install/dpdk.rst
> > > +++ b/Documentation/intro/install/dpdk.rst
> > > @@ -42,7 +42,7 @@ Build requirements
> > >  In addition to the requirements described in :doc:`general`, building
> > > Open  vSwitch with DPDK will require the following:
> > >
> > > -- DPDK 19.11.6
> > > +- DPDK 19.11.8
> > >
> > >  - A `DPDK supported NIC`_
> > >
> > > @@ -71,9 +71,9 @@ Install DPDK
> > >  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
> > >
> > > $ cd /usr/src/
> > > -   $ wget https://fast.dpdk.org/rel/dpdk-19.11.6.tar.xz
> > > -   $ tar xf dpdk-19.11.6.tar.xz
> > > -   $ export DPDK_DIR=/usr/src/dpdk-stable-19.11.6
> > > +   $ wget https://fast.dpdk.org/rel/dpdk-19.11.8.tar.xz
> > > +   $ tar xf dpdk-19.11.8.tar.xz
> > > +   $ export DPDK_DIR=/usr/src/dpdk-stable-19.11.8
> > > $ cd $DPDK_DIR
> > >
> > >  #. (Optional) Configure DPDK as a shared library diff --git
> > > a/Documentation/topics/dpdk/vhost-user.rst
> > > b/Documentation/topics/dpdk/vhost-user.rst
> > > index d020115..b10daa5 100644
> > > --- a/Documentation/topics/dpdk/vhost-user.rst
> > > +++ b/Documentation/topics/dpdk/vhost-user.rst
> > > @@ -392,9 +392,9 @@ To begin, instantiate a guest as described in
> > > :ref:`dpdk-vhost-user` or  DPDK sources to VM and build DPDK::
> > >
> > >  $ cd /root/dpdk/
> > > -$ wget https://fast.dpdk.org/rel/dpdk-19.11.6.tar.xz
> > > -$ tar xf dpdk-19.11.6.tar.xz
> > > -$ export DPDK_DIR=/root/dpdk/dpdk-stable-19.11.6
> > > +$ wget https://fast.dpdk.org/rel/dpdk-19.11.8.tar.xz
> > > +$ tar xf dpdk-19.11.8.tar.xz
> > > +$ export DPDK_DIR=/root/dpdk/dpdk-stable-19.11.8
> > >  $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> > >  $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> > >  $ cd $DPDK_DIR
> > > diff --git a/NEWS b/NEWS
> > > index 32ec919..733c785 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -1,7 +1,7 @@
> > >  v2.13.4 - xx xxx 
> > >  -
> > > - DPDK:
> > > - * OVS validated with DPDK 19.11.6. It is recommended to use this
> > version
> > > + * OVS validated with DPDK 19.11.8. It is recommended to use this
> > > + version
> > > until further releases.
> > > - ovs-ctl:
> > >   * New option '--no-record-hostname' to disable hostname
> > > configuration
> > >
> 
> LGTM
> 
> Acked-by: Sunil Pai G 


Thanks All, having spoken to 

Re: [ovs-dev] [PATCH branch-2.14 v2] dpdk: Use DPDK 19.11.8 release

[Stokes, Ian]

> > -Original Message-
> > From: Kevin Traynor 
> > Sent: Monday, April 26, 2021 8:36 PM
> > To: Govindharajan, Hariprasad ;
> > d...@openvswitch.org
> > Cc: Christian Ehrhardt ; Pai G, Sunil
> > 
> > Subject: Re: [ovs-dev] [PATCH branch-2.14 v2] dpdk: Use DPDK 19.11.8
> > release
> >
> > +cc Christian/Sunil
> >
> > On 23/04/2021 13:11, Hariprasad Govindharajan wrote:
> > > Modify ci linux build script to use the latest DPDK stable release.
> > > Modify Documentation to use the latest DPDK stable release 19.11.8
> > > Update NEWS file to reflect the latest DPDK stable release.
> > >
> > > Building DPDK releases 19.11.6 and 19.11.7,with meson was found to
> > > break the OvS build.
> > > DPDK 19.11.8 was released to fix the above said issue.
> > > Link to the discussion about the issue can be found below:
> > > http://mails.dpdk.org/archives/stable/2021-April/029786.html
> > >
> > > Signed-off-by: Hariprasad Govindharajan
> > > 
> > > ---
> > >  .ci/linux-build.sh   | 2 +-
> > >  Documentation/faq/releases.rst   | 4 ++--
> > >  Documentation/intro/install/dpdk.rst | 8 
> > >  Documentation/topics/dpdk/vhost-user.rst | 6 +++---
> > >  NEWS | 2 +-
> > >  5 files changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > 1896849..7e16ebb 100755
> > > --- a/.ci/linux-build.sh
> > > +++ b/.ci/linux-build.sh
> > > @@ -187,7 +187,7 @@ fi
> > >
> > >  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> > >  if [ -z "$DPDK_VER" ]; then
> > > -DPDK_VER="19.11.6"
> > > +DPDK_VER="19.11.8"
> > >  fi
> > >  install_dpdk $DPDK_VER
> > >  if [ "$CC" = "clang" ]; then
> > > diff --git a/Documentation/faq/releases.rst
> > > b/Documentation/faq/releases.rst index cafbebc..9e83826 100644
> > > --- a/Documentation/faq/releases.rst
> > > +++ b/Documentation/faq/releases.rst
> > > @@ -195,8 +195,8 @@ Q: What DPDK version does each Open vSwitch
> > release work with?
> > >  2.10.x   17.11.10
> > >  2.11.x   18.11.11
> > >  2.12.x   18.11.11
> > > -2.13.x   19.11.6
> > > -2.14.x   19.11.6
> > > +2.13.x   19.11.8
> > > +2.14.x   19.11.8
> > >   
> > >
> > >  Q: Are all the DPDK releases that OVS versions work with maintained?
> > > diff --git a/Documentation/intro/install/dpdk.rst
> > > b/Documentation/intro/install/dpdk.rst
> > > index c0754bc..2835c83 100644
> > > --- a/Documentation/intro/install/dpdk.rst
> > > +++ b/Documentation/intro/install/dpdk.rst
> > > @@ -42,7 +42,7 @@ Build requirements
> > >  In addition to the requirements described in :doc:`general`, building
> > > Open  vSwitch with DPDK will require the following:
> > >
> > > -- DPDK 19.11.6
> > > +- DPDK 19.11.8
> > >
> > >  - A `DPDK supported NIC`_
> > >
> > > @@ -71,9 +71,9 @@ Install DPDK
> > >  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
> > >
> > > $ cd /usr/src/
> > > -   $ wget https://fast.dpdk.org/rel/dpdk-19.11.6.tar.xz
> > > -   $ tar xf dpdk-19.11.6.tar.xz
> > > -   $ export DPDK_DIR=/usr/src/dpdk-stable-19.11.6
> > > +   $ wget https://fast.dpdk.org/rel/dpdk-19.11.8.tar.xz
> > > +   $ tar xf dpdk-19.11.8.tar.xz
> > > +   $ export DPDK_DIR=/usr/src/dpdk-stable-19.11.8
> > > $ cd $DPDK_DIR
> > >
> > >  #. (Optional) Configure DPDK as a shared library diff --git
> > > a/Documentation/topics/dpdk/vhost-user.rst
> > > b/Documentation/topics/dpdk/vhost-user.rst
> > > index 4ccff07..f610d43 100644
> > > --- a/Documentation/topics/dpdk/vhost-user.rst
> > > +++ b/Documentation/topics/dpdk/vhost-user.rst
> > > @@ -392,9 +392,9 @@ To begin, instantiate a guest as described in
> > > :ref:`dpdk-vhost-user` or  DPDK sources to VM and build DPDK::
> > >
> > >  $ cd /root/dpdk/
> > > -$ wget https://fast.dpdk.org/rel/dpdk-19.11.6.tar.xz
> > > -$ tar xf dpdk-19.11.6.tar.xz
> > > -$ export DPDK_DIR=/root/dpdk/dpdk-stable-19.11.6
> > > +$ wget https://fast.dpdk.org/rel/dpdk-19.11.8.tar.xz
> > > +$ tar xf dpdk-19.11.8.tar.xz
> > > +$ export DPDK_DIR=/root/dpdk/dpdk-stable-19.11.8
> > >  $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> > >  $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> > >  $ cd $DPDK_DIR
> > > diff --git a/NEWS b/NEWS
> > > index 4e9aca6..440ba09 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -1,7 +1,7 @@
> > >  v2.14.3 - xx xxx 
> > >  -
> > > - DPDK:
> > > - * OVS validated with DPDK 19.11.6. It is recommended to use this
> > version
> > > + * OVS validated with DPDK 19.11.8. It is recommended to use this
> > > + version
> > > until further releases.
> > > - ovs-ctl:
> > >   * New option '--no-record-hostname' to disable hostname
> > > configuration
> > >
> 
> 
> LGTM
> 
> Acked-by: Sunil Pai G 

Thanks All, having spoken to Christian 

Re: [ovs-dev] [PATCH v3] dpif-netdev: Expand the meter capacity.

[0-day Robot]

Bleep bloop.  Greetings Tonghao Zhang, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Comment with 'xxx' marker
#107 FILE: lib/dpif-netdev.c:1745:
 * Use them directly instead of hash_xxx function

Lines checked: 333, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpdk: Use DPDK 20.11.1 release

[Stokes, Ian]

> On 21/04/2021 11:50, Hariprasad Govindharajan wrote:
> > Modify ci linux build script to use the latest DPDK stable release.
> > Modify Documentation to use the latest DPDK stable release 20.11.1
> > Update NEWS file to reflect the latest DPDK stable releases.
> > FAQ is updated to reflect the latest DPDK for each branch.
> >
> 
> Reviewed, Ran PVP, passed checkpatch, passed github actions [1].
> 
> One minor comment below, otherwise
> 
> Acked-by: Kevin Traynor 
> 
> [1] https://github.com/kevintraynor/ovs/actions/runs/774292423
> 
> > Signed-off-by: Hariprasad Govindharajan
> 
> > ---
> >  .ci/linux-build.sh   | 2 +-
> >  Documentation/faq/releases.rst   | 6 +++---
> >  Documentation/intro/install/dpdk.rst | 8 
> >  NEWS | 3 +++
> >  4 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> > index 9774493..0210d6a 100755
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -201,7 +201,7 @@ fi
> >
> >  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> >  if [ -z "$DPDK_VER" ]; then
> > -DPDK_VER="20.11"
> > +DPDK_VER="20.11.1"
> >  fi
> >  install_dpdk $DPDK_VER
> >  if [ "$CC" = "clang" ]; then
> > diff --git a/Documentation/faq/releases.rst
> b/Documentation/faq/releases.rst
> > index 6a5e414..3bc34c8 100644
> > --- a/Documentation/faq/releases.rst
> > +++ b/Documentation/faq/releases.rst
> > @@ -204,9 +204,9 @@ Q: What DPDK version does each Open vSwitch
> release work with?
> >  2.10.x   17.11.10
> >  2.11.x   18.11.9
> >  2.12.x   18.11.9
> > -2.13.x   19.11.2
> > -2.14.x   19.11.2
> > -2.15.x   20.11.0
> 
> > +2.13.x   19.11.8
> > +2.14.x   19.11.8
> 
> These ^ aren't merged yet, so shouldn't _really_ be updated as part of
> this patch. OTOH, having to send additional patches to branch-2.15 and
> master for the 19.11.8 updates on 2.13 and 2.14 is hardly worth it.
> 
> If the 19.11.8 patches are merged first, I would just keep the update in
> this patch, if they are not merged first, then perhaps they should be
> removed here.
> 
> > +2.15.x   20.11.1
> >   
> >
> >  Q: Are all the DPDK releases that OVS versions work with maintained?
> > diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> > index 3a24e54..612f2fd 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -42,7 +42,7 @@ Build requirements
> >  In addition to the requirements described in :doc:`general`, building Open
> >  vSwitch with DPDK will require the following:
> >
> > -- DPDK 20.11
> > +- DPDK 20.11.1
> >
> >  - A `DPDK supported NIC`_
> >
> > @@ -73,9 +73,9 @@ Install DPDK
> >  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
> >
> > $ cd /usr/src/
> > -   $ wget https://fast.dpdk.org/rel/dpdk-20.11.tar.xz
> > -   $ tar xf dpdk-20.11.tar.xz
> > -   $ export DPDK_DIR=/usr/src/dpdk-20.11
> > +   $ wget https://fast.dpdk.org/rel/dpdk-20.11.1.tar.xz
> > +   $ tar xf dpdk-20.11.1.tar.xz
> > +   $ export DPDK_DIR=/usr/src/dpdk-stable-20.11.1
> > $ cd $DPDK_DIR
> >
> >  #. Configure and install DPDK using Meson
> > diff --git a/NEWS b/NEWS
> > index 95cf922..402ce59 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,6 +9,9 @@ Post-v2.15.0
> >   * New option '--no-record-hostname' to disable hostname configuration
> > in ovsdb on startup.
> >   * New command 'record-hostname-if-not-set' to update hostname in
> ovsdb.
> > +   - DPDK:
> > + * OVS validated with DPDK 20.11.1. It is recommended to use this
> version
> > +   until further releases.
> >
> >
> >  v2.15.0 - 15 Feb 2021
> >

Thanks All applied to master.

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

Re: [ovs-dev] [PATCH branch-2.15] dpdk: Use DPDK 20.11.1 release

[Stokes, Ian]

> On 21/04/2021 11:49, Hariprasad Govindharajan wrote:
> > Modify ci linux build script to use the latest DPDK stable release.
> > Modify Documentation to use the latest DPDK stable release 20.11.1
> > Update NEWS file to reflect the latest DPDK stable releases.
> > FAQ is updated to reflect the latest DPDK for each branch.
> >
> 
> Reviewed, Ran PVP, passed checkpatch,
> 
> Similar comment as master branch patch wrt 19.11.8 update.
> 
> Otherwise
> Acked-by: Kevin Traynor 
> 
> > Signed-off-by: Hariprasad Govindharajan
> 
> > ---
> >  .ci/linux-build.sh   | 2 +-
> >  Documentation/faq/releases.rst   | 6 +++---
> >  Documentation/intro/install/dpdk.rst | 8 
> >  NEWS | 3 +++
> >  4 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> > index dd29a41..1acf501 100755
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -201,7 +201,7 @@ fi
> >
> >  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
> >  if [ -z "$DPDK_VER" ]; then
> > -DPDK_VER="20.11"
> > +DPDK_VER="20.11.1"
> >  fi
> >  install_dpdk $DPDK_VER
> >  if [ "$CC" = "clang" ]; then
> > diff --git a/Documentation/faq/releases.rst
> b/Documentation/faq/releases.rst
> > index 6a5e414..3bc34c8 100644
> > --- a/Documentation/faq/releases.rst
> > +++ b/Documentation/faq/releases.rst
> > @@ -204,9 +204,9 @@ Q: What DPDK version does each Open vSwitch
> release work with?
> >  2.10.x   17.11.10
> >  2.11.x   18.11.9
> >  2.12.x   18.11.9
> > -2.13.x   19.11.2
> > -2.14.x   19.11.2
> > -2.15.x   20.11.0
> > +2.13.x   19.11.8
> > +2.14.x   19.11.8
> > +2.15.x   20.11.1
> >   
> >
> >  Q: Are all the DPDK releases that OVS versions work with maintained?
> > diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> > index 3a24e54..612f2fd 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -42,7 +42,7 @@ Build requirements
> >  In addition to the requirements described in :doc:`general`, building Open
> >  vSwitch with DPDK will require the following:
> >
> > -- DPDK 20.11
> > +- DPDK 20.11.1
> >
> >  - A `DPDK supported NIC`_
> >
> > @@ -73,9 +73,9 @@ Install DPDK
> >  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
> >
> > $ cd /usr/src/
> > -   $ wget https://fast.dpdk.org/rel/dpdk-20.11.tar.xz
> > -   $ tar xf dpdk-20.11.tar.xz
> > -   $ export DPDK_DIR=/usr/src/dpdk-20.11
> > +   $ wget https://fast.dpdk.org/rel/dpdk-20.11.1.tar.xz
> > +   $ tar xf dpdk-20.11.1.tar.xz
> > +   $ export DPDK_DIR=/usr/src/dpdk-stable-20.11.1
> > $ cd $DPDK_DIR
> >
> >  #. Configure and install DPDK using Meson
> > diff --git a/NEWS b/NEWS
> > index 036d403..d6a9395 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -4,6 +4,9 @@ v2.15.1 - xx xxx 
> >   * New option '--no-record-hostname' to disable hostname configuration
> > in ovsdb on startup.
> >   * New command 'record-hostname-if-not-set' to update hostname in
> ovsdb.
> > +   - DPDK:
> > + * OVS validated with DPDK 20.11.1. It is recommended to use this
> version
> > +   until further releases.
> >
> >
> >  v2.15.0 - 15 Feb 2021
> >

Thanks all, applied.

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

Re: [ovs-dev] [PATCH v2 1/2] dpif-netdev: Expand the meter capacity using cmap

[Tonghao Zhang]

On Tue, Apr 27, 2021 at 2:38 AM Ilya Maximets  wrote:
>
> On 3/15/20 7:43 AM, xiangxia.m@gmail.com wrote:
> > From: Tonghao Zhang 
> >
> > For now, ovs-vswitchd use the array of the dp_meter struct
> > to store meter's data, and at most, there are only 65536
> > (defined by MAX_METERS) meters that can be used. But in some
> > case, for example, in the edge gateway, we should use 200,000+,
> > at least, meters for IP address bandwidth limitation.
> > Every one IP address will use two meters for its rx and tx
> > path[1]. In other way, ovs-vswitchd should support meter-offload
> > (rte_mtr_xxx api introduced by dpdk.), but there are more than
> > 65536 meters in the hardware, such as Mellanox ConnectX-6.
> >
> > This patch use cmap to manage the meter, instead of the array.
>
> Hi.  Here is a around of review as discussed in review of v3 of this series.
>
> It's a bit confusing that there was two versions of v2 of this patch set.
> Some comments might be same as I already had for another v2...
>
> >
> > * Insertion performance, ovs-ofctl add-meter 1000+ meters,
> >   the cmap takes abount 4000ms, as same as previous implementation.
> > * Lookup performance in datapath, we add 1000+ meter which rate is
> >   10G (the NIC cards are 10Gb, so netdev-datapath will not drop the
> >   packets.), and a flow which only forwarding the packets from p0
> >   to p1, with meter action[2]. On other server, the pktgen-dpdk
> >   will generate 64B packets to p0.
> >   The forwarding performance is 4,814,400 pps. Without this path,
> >   4,935,584 pps. There are about 1% performance loss. For addressing
> >   this issue, next patch add a meter cache.
>
> To get back some of the performance you may consider switching form
> mutex to spin lock for meters themselves.  (Note that fallback for
> systems that has no spin locks will be needed in this case, so you
> might want to keep meter_lock/unclock functions.)
>
> Also, this test case looks very synthetic and in a more real-world
> setup perfromance diference should be lower, so, likely, doesn't
> worth efforts to create caches and stuff.
>
> >
> > [1].
> > $ in_port=p0,ip,ip_dst=1.1.1.x action=meter:n,output:p1
> > $ in_port=p1,ip,ip_src=1.1.1.x action=meter:m,output:p0
> >
> > [2].
> > $ in_port=p0 action=meter:100,output:p1
> >
> > Cc: Ben Pfaff 
> > Cc: Jarno Rajahalme 
> > Cc: Ilya Maximets 
> > Cc: Andy Zhou 
> > Signed-off-by: Tonghao Zhang 
> > --->  lib/dpif-netdev.c | 199 
> > +++---
> >  1 file changed, 130 insertions(+), 69 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index d393aab..5474d52 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -98,9 +98,11 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
> >
> >  /* Configuration parameters. */
> >  enum { MAX_FLOWS = 65536 }; /* Maximum number of flows in flow table. 
> > */
> > -enum { MAX_METERS = 65536 };/* Maximum number of meters. */
> > -enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
> > -enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
> > +
> > +/* Maximum number of meters in the table. */
> > +#define METER_ENTRY_MAX (1 << 19)
> > +/* Maximum number of bands / meter. */
> > +#define METER_BAND_MAX (8)
>
> It's unclear why enums converted to macros and why we need to change names.
> Please, keep them as is.
Hi Ilya
v3 is posted, please review.
http://patchwork.ozlabs.org/project/openvswitch/patch/20210512091736.48172-1-xiangxia.m@gmail.com/
> >
> >  COVERAGE_DEFINE(datapath_drop_meter);
> >  COVERAGE_DEFINE(datapath_drop_upcall_error);
> > @@ -280,6 +282,9 @@ struct dp_meter_band {
> >  };
> >
> >  struct dp_meter {
> > +struct cmap_node node;
> > +struct ovs_mutex lock;
> > +uint32_t id;
> >  uint16_t flags;
> >  uint16_t n_bands;
> >  uint32_t max_delta_t;
> > @@ -289,6 +294,12 @@ struct dp_meter {
> >  struct dp_meter_band bands[];
> >  };
> >
> > +struct dp_netdev_meter {
> > +struct cmap table OVS_GUARDED;
> > +struct ovs_mutex lock;  /* Used for meter table. */
> > +uint32_t hash_basis;
>
> Not sure if we need a hash basis, especially because you're choosing
> it randomly.  Basis for hash functions used to choose a beeter hashing
> schema if you know that some particular base value will provide better
> hash distribution for particular type of data, or if you need ot hash
> two equal values and you want them to be hashed differently.  Both cases
> are not applicable here, so, we can simply use 0 always as a hash basis.
> Also hashing might be a bit faster if basis is a compile-time constant.
>
> > +};
> > +
> >  struct pmd_auto_lb {
> >  bool auto_lb_requested; /* Auto load balancing requested by user. 
> > */
> >  bool is_enabled;/* Current status of Auto load balancing. 
> > */
> > @@ -329,8 +340,7 @@ struct dp_netdev {
> >  atomic_uint32_t tx_flush_interval;
> >
> >  /* 

[ovs-dev] [PATCH v3] dpif-netdev: Expand the meter capacity.

[xiangxia . m . yue]

From: Tonghao Zhang 

For now, ovs-vswitchd use the array of the dp_meter struct
to store meter's data, and at most, there are only 65536
(defined by MAX_METERS) meters that can be used. But in some
case, for example, in the edge gateway, we should use 200,000+,
at least, meters for IP address bandwidth limitation.
Every one IP address will use two meters for its rx and tx
path[1]. In other way, ovs-vswitchd should support meter-offload
(rte_mtr_xxx api introduced by dpdk.), but there are more than
65536 meters in the hardware, such as Mellanox ConnectX-6.

This patch use cmap to manage the meter, instead of the array.

* Insertion performance, ovs-ofctl add-meter 1000+ meters,
  the cmap takes abount 4000ms, as same as previous implementation.
* Lookup performance in datapath, we add 1000+ meters which rate limit
  are 10Gbps (the NIC cards are 10Gbps, so netdev-datapath will not
  drop the packets.), and a flow which only forward packets from p0
  to p1, with meter action[2]. On other machine, pktgen-dpdk will
  generate 64B packets to p0.

  The forwarding performance always is 1324 Kpps on my server
  which CPU is Intel E5-2650, 2.00GHz.

[1].
$ in_port=p0,ip,ip_dst=1.1.1.x action=meter:n,output:p1
$ in_port=p1,ip,ip_src=1.1.1.x action=meter:m,output:p0

[2].
$ in_port=p0 action=meter:100,output:p1

Signed-off-by: Tonghao Zhang 
---
v3:
* update the commit message
* remove dp_netdev_meter struct
* remove create_dp_netdev function
* don't use the hash_basis
* use the meter_id as a hash instead of hash_xxx function. see *dp_meter_hash 
for details
* fix coding style
* v2: 
http://patchwork.ozlabs.org/project/openvswitch/patch/1584254601-7321-1-git-send-email-xiangxia.m@gmail.com/
---
 lib/dpif-netdev.c | 158 --
 1 file changed, 97 insertions(+), 61 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 251788b04965..f800c8c65e13 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -99,9 +99,8 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 #define DEFAULT_TX_FLUSH_INTERVAL 0
 
 /* Configuration parameters. */
-enum { MAX_METERS = 65536 };/* Maximum number of meters. */
+enum { MAX_METERS = 1 << 18 };/* Maximum number of meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
-enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
 
 COVERAGE_DEFINE(datapath_drop_meter);
 COVERAGE_DEFINE(datapath_drop_upcall_error);
@@ -287,6 +286,9 @@ struct dp_meter_band {
 };
 
 struct dp_meter {
+struct cmap_node node;
+struct ovs_mutex lock;
+uint32_t id;
 uint16_t flags;
 uint16_t n_bands;
 uint32_t max_delta_t;
@@ -339,8 +341,8 @@ struct dp_netdev {
 atomic_uint32_t tx_flush_interval;
 
 /* Meters. */
-struct ovs_mutex meter_locks[N_METER_LOCKS];
-struct dp_meter *meters[MAX_METERS]; /* Meter bands. */
+struct ovs_mutex meters_lock;
+struct cmap meters OVS_GUARDED;
 
 /* Probability of EMC insertions is a factor of 'emc_insert_min'.*/
 OVS_ALIGNED_VAR(CACHE_LINE_SIZE) atomic_uint32_t emc_insert_min;
@@ -392,19 +394,6 @@ struct dp_netdev {
 struct cmap tx_bonds; /* Contains 'struct tx_bond'. */
 };
 
-static void meter_lock(const struct dp_netdev *dp, uint32_t meter_id)
-OVS_ACQUIRES(dp->meter_locks[meter_id % N_METER_LOCKS])
-{
-ovs_mutex_lock(>meter_locks[meter_id % N_METER_LOCKS]);
-}
-
-static void meter_unlock(const struct dp_netdev *dp, uint32_t meter_id)
-OVS_RELEASES(dp->meter_locks[meter_id % N_METER_LOCKS])
-{
-ovs_mutex_unlock(>meter_locks[meter_id % N_METER_LOCKS]);
-}
-
-
 static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev *dp,
 odp_port_t)
 OVS_REQUIRES(dp->port_mutex);
@@ -1747,6 +1736,68 @@ choose_port(struct dp_netdev *dp, const char *name)
 return ODPP_NONE;
 }
 
+static uint32_t
+dp_meter_hash(uint32_t meter_id)
+{
+/* In the ofproto-dpif layer, we use the id-pool
+ * to alloc meter id orderly (e.g. 1, 2, ... N.),
+ * which provide a better hash distribution.
+ * Use them directly instead of hash_xxx function
+ * for achieving high-performance.
+ */
+return meter_id;
+}
+
+static void
+dp_netdev_meter_destroy(struct dp_netdev *dp)
+{
+struct dp_meter *m;
+
+ovs_mutex_lock(>meters_lock);
+CMAP_FOR_EACH (m, node, >meters) {
+cmap_remove(>meters, >node, dp_meter_hash(m->id));
+ovsrcu_postpone(free, m);
+}
+
+cmap_destroy(>meters);
+ovs_mutex_unlock(>meters_lock);
+
+ovs_mutex_destroy(>meters_lock);
+}
+
+static struct dp_meter*
+dp_meter_lookup(struct cmap *meters, uint32_t meter_id)
+{
+uint32_t hash = dp_meter_hash(meter_id);
+struct dp_meter *m;
+
+CMAP_FOR_EACH_WITH_HASH (m, node, hash, meters) {
+if (m->id == meter_id) {
+return m;
+}
+}
+
+return NULL;
+}
+
+static void