Re: [ovs-dev] [PATCH] ovsdb: Count weak reference objects.

2022-12-04 Thread Han Zhou
On Fri, Nov 25, 2022 at 4:36 AM Ilya Maximets  wrote:
>
> OVSDB creates a separate object for each weak reference in order to
> track them and there could be a significant amount of these objects
> in the database.
>
> We also had problems with number of these objects growing out of
> bounds recently.  So, adding them to a memory report seems to be
> a good thing.
>
> Counting them globally to cover all the copied instances in transactions
> and the transaction history (even though there should be none).
> It's also hard to count them per-database, because weak references
> are stored on destination rows and can be destroyed either while
> destroying the destination row or while removing the reference from
> the source row.  Also, not all the involved functions have direct
> access to the database object.  So, there is no single clear place
> where counters should be updated.
>
> Signed-off-by: Ilya Maximets 
> ---
>  ovsdb/ovsdb.c   | 4 
>  ovsdb/ovsdb.h   | 4 
>  ovsdb/row.c | 5 -
>  ovsdb/transaction.c | 2 ++
>  4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
> index 1c011fab0..11786f376 100644
> --- a/ovsdb/ovsdb.c
> +++ b/ovsdb/ovsdb.c
> @@ -43,6 +43,8 @@
>  #include "openvswitch/vlog.h"
>  VLOG_DEFINE_THIS_MODULE(ovsdb);
>
> +size_t n_weak_refs = 0;
> +
>  struct ovsdb_schema *
>  ovsdb_schema_create(const char *name, const char *version, const char
*cksum)
>  {
> @@ -546,6 +548,8 @@ ovsdb_get_memory_usage(const struct ovsdb *db, struct
simap *usage)
>  if (db->storage) {
>  ovsdb_storage_get_memory_usage(db->storage, usage);
>  }
> +
> +simap_put(usage, "n-weak-refs", n_weak_refs);
>  }
>
>  struct ovsdb_table *
> diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
> index d05e7c64a..13d8bf407 100644
> --- a/ovsdb/ovsdb.h
> +++ b/ovsdb/ovsdb.h
> @@ -125,6 +125,10 @@ struct ovsdb {
>  struct ovsdb_compaction_state *snap_state;
>  };
>
> +/* Total number of 'weak reference' objects in all databases
> + * and transactions. */
> +extern size_t n_weak_refs;
> +
>  struct ovsdb *ovsdb_create(struct ovsdb_schema *, struct ovsdb_storage
*);
>  void ovsdb_destroy(struct ovsdb *);
>
> diff --git a/ovsdb/row.c b/ovsdb/row.c
> index 3f0bb8acf..d7bfbdd36 100644
> --- a/ovsdb/row.c
> +++ b/ovsdb/row.c
> @@ -21,8 +21,9 @@
>
>  #include "openvswitch/dynamic-string.h"
>  #include "openvswitch/json.h"
> -#include "ovsdb-error.h"
>  #include "openvswitch/shash.h"
> +#include "ovsdb-error.h"
> +#include "ovsdb.h"
>  #include "sort.h"
>  #include "table.h"
>  #include "util.h"
> @@ -78,6 +79,7 @@ ovsdb_weak_ref_clone(struct ovsdb_weak_ref *src)
>  ovsdb_type_clone(>type, >type);
>  weak->column_idx = src->column_idx;
>  weak->by_key = src->by_key;
> +n_weak_refs++;
>  return weak;
>  }
>
> @@ -130,6 +132,7 @@ ovsdb_weak_ref_destroy(struct ovsdb_weak_ref *weak)
>  }
>  ovsdb_type_destroy(>type);
>  free(weak);
> +n_weak_refs--;
>  }
>
>  struct ovsdb_row *
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 5d7c70a51..03541af85 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -613,6 +613,8 @@ add_weak_ref(const struct ovsdb_row *src, const
struct ovsdb_row *dst_,
>  weak->column_idx = column->index;
>  hmap_node_nullify(>dst_node);
>  ovs_list_push_back(ref_list, >src_node);
> +
> +n_weak_refs++;
>  }
>
>  static void
> --
> 2.38.1
>

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


[ovs-dev] [PATCH] [PATCH v7 net-next] net: openvswitch: Add support to count upcall packets

2022-12-04 Thread wangchuanlei
Add support to count upall packets, when kmod of openvswitch
upcall to userspace , here count the number of packets for
upcall succeed and failed, which is a better way to see how
many packets upcalled to userspace(ovs-vswitchd) on every
interfaces.

Here modify format of code used by comments of v6.

Changes since v4 & v5 & v6:
- optimize the function used by comments

Changes since v3:
- use nested NLA_NESTED attribute in netlink message

Changes since v2:
- add count of upcall failed packets

Changes since v1:
- add count of upcall succeed packets

Signed-off-by: wangchuanlei 
---
 include/uapi/linux/openvswitch.h | 14 ++
 net/openvswitch/datapath.c   | 47 
 net/openvswitch/vport.c  | 40 +++
 net/openvswitch/vport.h  | 16 +++
 4 files changed, 117 insertions(+)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 94066f87e9ee..8422ebf6885b 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -277,11 +277,25 @@ enum ovs_vport_attr {
OVS_VPORT_ATTR_PAD,
OVS_VPORT_ATTR_IFINDEX,
OVS_VPORT_ATTR_NETNSID,
+   OVS_VPORT_ATTR_UPCALL_STATS,
__OVS_VPORT_ATTR_MAX
 };
 
 #define OVS_VPORT_ATTR_MAX (__OVS_VPORT_ATTR_MAX - 1)
 
+/**
+ * enum ovs_vport_upcall_attr - attributes for %OVS_VPORT_UPCALL* commands
+ * @OVS_VPORT_UPCALL_SUCCESS: 64-bit upcall success packets.
+ * @OVS_VPORT_UPCALL_FAIL: 64-bit upcall fail packets.
+ */
+enum ovs_vport_upcall_attr {
+   OVS_VPORT_UPCALL_SUCCESS,
+   OVS_VPORT_UPCALL_FAIL,
+   __OVS_VPORT_UPCALL_MAX
+};
+
+#define OVS_VPORT_UPCALL_MAX (__OVS_VPORT_UPCALL_MAX - 1)
+
 enum {
OVS_VXLAN_EXT_UNSPEC,
OVS_VXLAN_EXT_GBP,  /* Flag or __u32 */
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c8a9075ddd0a..d8ca73dac3b0 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -209,6 +209,26 @@ static struct vport *new_vport(const struct vport_parms 
*parms)
return vport;
 }
 
+static void ovs_vport_update_upcall_stats(struct sk_buff *skb,
+ const struct dp_upcall_info 
*upcall_info,
+ bool upcall_result)
+{
+   struct vport *p = OVS_CB(skb)->input_vport;
+   struct vport_upcall_stats_percpu *stats;
+
+   if (upcall_info->cmd != OVS_PACKET_CMD_MISS &&
+   upcall_info->cmd != OVS_PACKET_CMD_ACTION)
+   return;
+
+   stats = this_cpu_ptr(p->upcall_stats);
+   u64_stats_update_begin(>syncp);
+   if (upcall_result)
+   u64_stats_inc(>n_success);
+   else
+   u64_stats_inc(>n_fail);
+   u64_stats_update_end(>syncp);
+}
+
 void ovs_dp_detach_port(struct vport *p)
 {
ASSERT_OVSL();
@@ -216,6 +236,9 @@ void ovs_dp_detach_port(struct vport *p)
/* First drop references to device. */
hlist_del_rcu(>dp_hash_node);
 
+   /* Free percpu memory */
+   free_percpu(p->upcall_stats);
+
/* Then destroy it. */
ovs_vport_del(p);
 }
@@ -305,6 +328,8 @@ int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
err = queue_userspace_packet(dp, skb, key, upcall_info, cutlen);
else
err = queue_gso_packets(dp, skb, key, upcall_info, cutlen);
+
+   ovs_vport_update_upcall_stats(skb, upcall_info, !err);
if (err)
goto err;
 
@@ -1825,6 +1850,12 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
goto err_destroy_portids;
}
 
+   vport->upcall_stats = netdev_alloc_pcpu_stats(struct 
vport_upcall_stats_percpu);
+   if (!vport->upcall_stats) {
+   err = -ENOMEM;
+   goto err_destroy_portids;
+   }
+
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
   info->snd_seq, 0, OVS_DP_CMD_NEW);
BUG_ON(err < 0);
@@ -2068,6 +2099,7 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, 
struct sk_buff *skb,
 {
struct ovs_header *ovs_header;
struct ovs_vport_stats vport_stats;
+   struct nlattr *nla;
int err;
 
ovs_header = genlmsg_put(skb, portid, seq, _vport_genl_family,
@@ -2097,6 +2129,14 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, 
struct sk_buff *skb,
  OVS_VPORT_ATTR_PAD))
goto nla_put_failure;
 
+   nla = nla_nest_start_noflag(skb, OVS_VPORT_ATTR_UPCALL_STATS);
+   if (!nla)
+   goto nla_put_failure;
+
+   if (ovs_vport_get_upcall_stats(vport, skb))
+   goto nla_put_failure;
+   nla_nest_end(skb, nla);
+
if (ovs_vport_get_upcall_portids(vport, skb))
goto nla_put_failure;
 
@@ -2278,6 +2318,12 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
goto 

Re: [ovs-dev] [PATCH] [PATCH v6 net-next] net: openvswitch: Add support to count upcall packets

2022-12-04 Thread wangchuanlei
Hi, Eelco,
Thank you for review again !  I will give a new version of patch based on 
your comments 
today!

Best regards!
wangchuanlei

-


On 30 Nov 2022, at 10:15, wangchuanlei wrote:

> Add support to count upall packets, when kmod of openvswitch upcall to 
> userspace , here count the number of packets for upcall succeed and 
> failed, which is a better way to see how many packets upcalled to 
> userspace(ovs-vswitchd) on every interfaces.
>
> Here modify format of code used by comments of v6.
>
> Changes since v4 & v5:
> - optimize the function used by comments
>
> Changes since v3:
> - use nested NLA_NESTED attribute in netlink message
>
> Changes since v2:
> - add count of upcall failed packets
>
> Changes since v1:
> - add count of upcall succeed packets
>
> Signed-off-by: wangchuanlei 
> ---
>  include/uapi/linux/openvswitch.h | 14 +
>  net/openvswitch/datapath.c   | 50 
>  net/openvswitch/vport.c  | 44 
>  net/openvswitch/vport.h  | 24 +++
>  4 files changed, 132 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 94066f87e9ee..8422ebf6885b 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -277,11 +277,25 @@ enum ovs_vport_attr {
>   OVS_VPORT_ATTR_PAD,
>   OVS_VPORT_ATTR_IFINDEX,
>   OVS_VPORT_ATTR_NETNSID,
> + OVS_VPORT_ATTR_UPCALL_STATS,
>   __OVS_VPORT_ATTR_MAX
>  };
>
>  #define OVS_VPORT_ATTR_MAX (__OVS_VPORT_ATTR_MAX - 1)
>
> +/**
> + * enum ovs_vport_upcall_attr - attributes for %OVS_VPORT_UPCALL* 
> +commands
> + * @OVS_VPORT_UPCALL_SUCCESS: 64-bit upcall success packets.
> + * @OVS_VPORT_UPCALL_FAIL: 64-bit upcall fail packets.
> + */
> +enum ovs_vport_upcall_attr {
> + OVS_VPORT_UPCALL_SUCCESS,
> + OVS_VPORT_UPCALL_FAIL,
> + __OVS_VPORT_UPCALL_MAX
> +};
> +
> +#define OVS_VPORT_UPCALL_MAX (__OVS_VPORT_UPCALL_MAX - 1)
> +
>  enum {
>   OVS_VXLAN_EXT_UNSPEC,
>   OVS_VXLAN_EXT_GBP,  /* Flag or __u32 */
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c 
> index c8a9075ddd0a..f9279aee2adb 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -209,6 +209,26 @@ static struct vport *new_vport(const struct vport_parms 
> *parms)
>   return vport;
>  }
>
> +static void ovs_vport_upcalls(struct sk_buff *skb,

This function name does not really represent what this function does.
It???s only taking care of statistics, so it should probably be called 
something like:

  ovs_vport_update_upcall_stats() or ovs_vport_inc_upcall_stats()

> +   const struct dp_upcall_info *upcall_info,
> +   bool upcall_result)
> +{
> + struct vport *p = OVS_CB(skb)->input_vport;
> + struct vport_upcall_stats_percpu *vport_stats;

If you just call vport_stats, stats, the reverse Christmas tree order is 
achieved.

> +
> + if (upcall_info->cmd != OVS_PACKET_CMD_MISS &&
> + upcall_info->cmd != OVS_PACKET_CMD_ACTION)
> + return;
> +
> + vport_stats = this_cpu_ptr(p->upcall_stats);
> + u64_stats_update_begin(_stats->syncp);
> + if (upcall_result)
> + u64_stats_inc(_stats->n_success);
> + else
> + u64_stats_inc(_stats->n_fail);
> + u64_stats_update_end(_stats->syncp);
> +}
> +
>  void ovs_dp_detach_port(struct vport *p)  {
>   ASSERT_OVSL();
> @@ -216,6 +236,9 @@ void ovs_dp_detach_port(struct vport *p)
>   /* First drop references to device. */
>   hlist_del_rcu(>dp_hash_node);
>
> + /* Free percpu memory */
> + free_percpu(p->upcall_stats);
> +
>   /* Then destroy it. */
>   ovs_vport_del(p);
>  }
> @@ -305,6 +328,8 @@ int ovs_dp_upcall(struct datapath *dp, struct sk_buff 
> *skb,
>   err = queue_userspace_packet(dp, skb, key, upcall_info, cutlen);
>   else
>   err = queue_gso_packets(dp, skb, key, upcall_info, cutlen);
> +
> + ovs_vport_upcalls(skb, upcall_info, !err);
>   if (err)
>   goto err;
>
> @@ -1825,6 +1850,12 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
> genl_info *info)
>   goto err_destroy_portids;
>   }
>
> + vport->upcall_stats = netdev_alloc_pcpu_stats(struct 
> vport_upcall_stats_percpu);
> + if (!vport->upcall_stats) {
> + err = -ENOMEM;
> + goto err_destroy_portids;
> + }
> +
>   err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
>  info->snd_seq, 0, OVS_DP_CMD_NEW);
>   BUG_ON(err < 0);
> @@ -2068,6 +2099,8 @@ static int ovs_vport_cmd_fill_info(struct vport 
> *vport, struct sk_buff *skb,  {
>   struct ovs_header *ovs_header;
>   struct ovs_vport_stats vport_stats;
> + struct ovs_vport_upcall_stats stat;
> + 

Re: [ovs-dev] [PATCH] [PATCH v6 net-next] net: openvswitch: Add support to count upcall packets

2022-12-04 Thread wangchuanlei
Hi, Simon,
I see that! Thank you for review! 

Best regards!
wangchuanlei


Hi,

On Thu, Dec 01, 2022 at 03:46:01AM -0500, wangchuanlei wrote:
> Hi, Jakub,
> 
>   Thank you for review, the comments below is a little confusing, can 
> you give a explanation?
> 
> Best regards!
> wangchuanlei
> 
> 
> On Wed, 30 Nov 2022 04:15:59 -0500 wangchuanlei wrote:
> > +/**
> > + * ovs_vport_get_upcall_stats - retrieve upcall stats
> > + *
> > + * @vport: vport from which to retrieve the stats
> > + * @ovs_vport_upcall_stats: location to store stats
> 
> s/ovs_vport_upcall_//

I believe Jakub is asking for "ovs_vport_upcall_" to be removed.
Or, in other words, to refer to the parameter as "stats", (which matches the 
name in the function signature below).

> 
> > + *
> > + * Retrieves upcall stats for the given device.
> > + *
> > + * Must be called with ovs_mutex or rcu_read_lock.
> > + */
> > +void ovs_vport_get_upcall_stats(struct vport *vport, struct 
> > +ovs_vport_upcall_stats *stats)
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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


[ovs-dev] [PATCH v4 ovn] binding: add the capability to apply QoS for lsp

2022-12-04 Thread Lorenzo Bianconi
Introduce the capability to apply QoS rules for logical switch ports
claimed by ovn-controller. Rely on shash instead of sset for
egress_ifaces.

Acked-by: Mark Michelson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2129742
Signed-off-by: Lorenzo Bianconi 
---
Changes since v3:
- fix typo in new system-ovn test
Changes since v2:
- fix qos configuration restarting ovn-controller
Changes since v1:
- improve ovs interface lookup
- improve system-tests
---
 controller/binding.c| 155 ++--
 controller/binding.h|   5 +-
 controller/ovn-controller.c |  15 ++--
 tests/system-ovn.at |  48 +++
 4 files changed, 156 insertions(+), 67 deletions(-)

diff --git a/controller/binding.c b/controller/binding.c
index 5df62baef..53520263c 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -115,6 +115,7 @@ struct qos_queue {
 uint32_t min_rate;
 uint32_t max_rate;
 uint32_t burst;
+char *port_name;
 };
 
 void
@@ -147,25 +148,50 @@ static void update_lport_tracking(const struct 
sbrec_port_binding *pb,
   struct hmap *tracked_dp_bindings,
   bool claimed);
 
+static bool is_lport_vif(const struct sbrec_port_binding *pb);
+
+static struct qos_queue *
+get_qos_map_entry(struct hmap *queue_map, const char *name)
+{
+struct qos_queue *qos_node;
+HMAP_FOR_EACH (qos_node, node, queue_map) {
+if (!strcmp(qos_node->port_name, name)) {
+return qos_node;
+}
+}
+
+return NULL;
+}
+
 static void
-get_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
+update_qos_params(const struct sbrec_port_binding *pb, struct hmap *queue_map)
 {
 uint32_t min_rate = smap_get_int(>options, "qos_min_rate", 0);
 uint32_t max_rate = smap_get_int(>options, "qos_max_rate", 0);
 uint32_t burst = smap_get_int(>options, "qos_burst", 0);
 uint32_t queue_id = smap_get_int(>options, "qdisc_queue_id", 0);
 
+struct qos_queue *node = get_qos_map_entry(queue_map, pb->logical_port);
+
 if ((!min_rate && !max_rate && !burst) || !queue_id) {
 /* Qos is not configured for this port. */
+if (node) {
+ hmap_remove(queue_map, >node);
+ free(node->port_name);
+ free(node);
+}
 return;
 }
 
-struct qos_queue *node = xzalloc(sizeof *node);
-hmap_insert(queue_map, >node, hash_int(queue_id, 0));
+if (!node) {
+node = xzalloc(sizeof *node);
+hmap_insert(queue_map, >node, hash_int(queue_id, 0));
+node->port_name = xstrdup(pb->logical_port);
+}
+node->queue_id = queue_id;
 node->min_rate = min_rate;
 node->max_rate = max_rate;
 node->burst = burst;
-node->queue_id = queue_id;
 }
 
 static const struct ovsrec_qos *
@@ -191,7 +217,7 @@ static bool
 set_noop_qos(struct ovsdb_idl_txn *ovs_idl_txn,
  const struct ovsrec_port_table *port_table,
  const struct ovsrec_qos_table *qos_table,
- struct sset *egress_ifaces)
+ struct shash *egress_ifaces)
 {
 if (!ovs_idl_txn) {
 return false;
@@ -206,11 +232,11 @@ set_noop_qos(struct ovsdb_idl_txn *ovs_idl_txn,
 size_t count = 0;
 
 OVSREC_PORT_TABLE_FOR_EACH (port, port_table) {
-if (sset_contains(egress_ifaces, port->name)) {
+if (shash_find(egress_ifaces, port->name)) {
 ovsrec_port_set_qos(port, noop_qos);
 count++;
 }
-if (sset_count(egress_ifaces) == count) {
+if (shash_count(egress_ifaces) == count) {
 break;
 }
 }
@@ -236,7 +262,8 @@ set_qos_type(struct netdev *netdev, const char *type)
 }
 
 static void
-setup_qos(const char *egress_iface, struct hmap *queue_map)
+setup_qos(const char *egress_iface,  const char *logical_port,
+  struct hmap *queue_map)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
 struct netdev *netdev_phy;
@@ -281,7 +308,7 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
  *   a configuration setting.
  *
  * - Otherwise leave the qdisc alone. */
-if (hmap_is_empty(queue_map)) {
+if (!get_qos_map_entry(queue_map, logical_port)) {
 if (!strcmp(qdisc_type, OVN_QOS_TYPE)) {
 set_qos_type(netdev_phy, "");
 }
@@ -338,6 +365,10 @@ setup_qos(const char *egress_iface, struct hmap *queue_map)
 continue;
 }
 
+if (strcmp(sb_info->port_name, logical_port)) {
+continue;
+}
+
 smap_clear(_details);
 smap_add_format(_details, "min-rate", "%d", sb_info->min_rate);
 smap_add_format(_details, "max-rate", "%d", sb_info->max_rate);
@@ -354,11 +385,12 @@ setup_qos(const char *egress_iface, struct hmap 
*queue_map)
 netdev_close(netdev_phy);
 }
 
-static void
+void
 destroy_qos_map(struct hmap *qos_map)
 {
 struct 

[ovs-dev] [PATCH] dpif-netdev: calculate per numa variance

2022-12-04 Thread Cheng Li
Currently, pmd_rebalance_dry_run() calculate overall variance of
all pmds regardless of their numa location. The overall result may
hide un-balance in an individual numa.

Considering the following case. Numa 0 is free because VMs on numa0
are not sending pkts, while numa 1 is busy. Within numa 1, pmds
workloads are not balanced. Obviously, moving 500 kpps workloads from
pmd 126 to pmd 61 will make numa1 much more balance. For numa1
the variance improment will be almost 100%, because after rebalance
each pmd in numa1 holds same workload(variance ~= 0). But the overall
variance improvement is only about 20%, which may not tigger auto_lb.

```
numa_id   core_id  kpps
  030 0
  031 0
  094 0
  095 0
  1   126  1500
  1   127  1000
  163  1000
  162   500
```

As auto_lb doesn't work if any coss_numa rxq exists, it means that
auto_lb only balance rxq assignment within each numa. So it makes
more sense to calculate variance improvemnet per numa node.

Signed-off-by: Cheng Li 
---
 lib/dpif-netdev.c | 90 +--
 1 file changed, 47 insertions(+), 43 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 2c08a71..6a53f13 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6076,39 +6076,33 @@ rxq_scheduling(struct dp_netdev *dp)
 static uint64_t variance(uint64_t a[], int n);
 
 static uint64_t
-sched_numa_list_variance(struct sched_numa_list *numa_list)
+sched_numa_variance(struct sched_numa *numa)
 {
-struct sched_numa *numa;
 uint64_t *percent_busy = NULL;
-unsigned total_pmds = 0;
 int n_proc = 0;
 uint64_t var;
 
-HMAP_FOR_EACH (numa, node, _list->numas) {
-total_pmds += numa->n_pmds;
-percent_busy = xrealloc(percent_busy,
-total_pmds * sizeof *percent_busy);
+percent_busy = xmalloc(numa->n_pmds * sizeof *percent_busy);
 
-for (unsigned i = 0; i < numa->n_pmds; i++) {
-struct sched_pmd *sched_pmd;
-uint64_t total_cycles = 0;
+for (unsigned i = 0; i < numa->n_pmds; i++) {
+struct sched_pmd *sched_pmd;
+uint64_t total_cycles = 0;
 
-sched_pmd = >pmds[i];
-/* Exclude isolated PMDs from variance calculations. */
-if (sched_pmd->isolated == true) {
-continue;
-}
-/* Get the total pmd cycles for an interval. */
-atomic_read_relaxed(_pmd->pmd->intrvl_cycles, _cycles);
+sched_pmd = >pmds[i];
+/* Exclude isolated PMDs from variance calculations. */
+if (sched_pmd->isolated == true) {
+continue;
+}
+/* Get the total pmd cycles for an interval. */
+atomic_read_relaxed(_pmd->pmd->intrvl_cycles, _cycles);
 
-if (total_cycles) {
-/* Estimate the cycles to cover all intervals. */
-total_cycles *= PMD_INTERVAL_MAX;
-percent_busy[n_proc++] = (sched_pmd->pmd_proc_cycles * 100)
- / total_cycles;
-} else {
-percent_busy[n_proc++] = 0;
-}
+if (total_cycles) {
+/* Estimate the cycles to cover all intervals. */
+total_cycles *= PMD_INTERVAL_MAX;
+percent_busy[n_proc++] = (sched_pmd->pmd_proc_cycles * 100)
+/ total_cycles;
+} else {
+percent_busy[n_proc++] = 0;
 }
 }
 var = variance(percent_busy, n_proc);
@@ -6182,6 +6176,7 @@ pmd_rebalance_dry_run(struct dp_netdev *dp)
 struct sched_numa_list numa_list_est;
 bool thresh_met = false;
 uint64_t current_var, estimate_var;
+struct sched_numa *numa_cur, *numa_est;
 uint64_t improvement = 0;
 
 VLOG_DBG("PMD auto load balance performing dry run.");
@@ -6200,24 +6195,33 @@ pmd_rebalance_dry_run(struct dp_netdev *dp)
 sched_numa_list_count(_list_est) == 1) {
 
 /* Calculate variances. */
-current_var = sched_numa_list_variance(_list_cur);
-estimate_var = sched_numa_list_variance(_list_est);
-
-if (estimate_var < current_var) {
- improvement = ((current_var - estimate_var) * 100) / current_var;
-}
-VLOG_DBG("Current variance %"PRIu64" Estimated variance %"PRIu64".",
- current_var, estimate_var);
-VLOG_DBG("Variance improvement %"PRIu64"%%.", improvement);
-
-if (improvement >= dp->pmd_alb.rebalance_improve_thresh) {
-thresh_met = true;
-VLOG_DBG("PMD load variance improvement threshold %u%% "
- "is met.", dp->pmd_alb.rebalance_improve_thresh);
-} else {
-VLOG_DBG("PMD load variance improvement threshold "
- "%u%% is not met.",
-  

Re: [ovs-dev] [PATCH v2] rhel: move conf.db to /var/lib/openvswitch, using symlinks

2022-12-04 Thread Roi Dayan via dev



On 30/11/2022 17:55, Ilya Maximets wrote:
> On 11/14/22 20:41, Timothy Redaelli wrote:
>> conf.db is by default at /etc/openvswitch, but it should be at
>> /var/lib/openvswitch like on Debian or like ovnnb_db.db and ovnsb_db.db.
>>
>> If conf.db already exists in /etc/openvswitch then it's moved to
>> /var/lib/openvswitch.
>> Symlinks are created for conf.db and .conf.db.~lock~ into /etc/openvswitch
>> for backward compatibility.
>>
>> Reported-at: 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2F1830857data=05%7C01%7Croid%40nvidia.com%7Cd69116141ff645fc2c7308dad2eb4612%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638054205222362304%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=%2BIcIVZBKrfhIpq%2B6r6I3QvjdZ9KvjLsrRSlvi9kFHzc%3Dreserved=0
>> Reported-by: Yedidyah Bar David 
>> Signed-off-by: Timothy Redaelli 
>> ---
>> v1 -> v2:
>> - Use hugetlbfs group instead of openvswitch when the package is built
>>   with dpdk (as reported by Flavio)
>> ---
>>  rhel/openvswitch-fedora.spec.in | 27 +++
>>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> If that works for Fedora, then LGTM.  Applied.
> 
> Thanks!
> Best regards, Ilya Maximets.
> ___
> dev mailing list
> d...@openvswitch.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=05%7C01%7Croid%40nvidia.com%7Cd69116141ff645fc2c7308dad2eb4612%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638054205222362304%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=fZZh4iYeUu%2BL2%2F%2FWTIgPNzpvfhpe%2F9MANkVPLmv57aY%3Dreserved=0


hi,

This commit expose some kind of issue and cause openvswitch not
to start on clean systems.

If old conf.db file didn't exists it creates an empty conf.db with
the touch command.
Empty conf.db cause ovsdb-server not to start.

#  /usr/share/openvswitch/scripts/ovs-ctl start
ovsdb-tool: ovsdb error: /etc/openvswitch/conf.db: cannot identify file type
Starting ovsdb-server ovsdb-server: ovsdb error: /etc/openvswitch/conf.db: 
cannot identify file type
   [FAILED]

If I remove the conf.db file (can leave the symbolic link in /etc)
then ovs starts fine.
# rm /var/lib/openvswitch/conf.db
#  /usr/share/openvswitch/scripts/ovs-ctl start
/etc/openvswitch/conf.db does not exist ... (warning).
Creating empty database /etc/openvswitch/conf.db   [  OK  ]
Starting ovsdb-server  [  OK  ]
system ID not configured, please use --system-id ... failed!
Configuring Open vSwitch system IDs[  OK  ]
Starting ovs-vswitchd  [  OK  ]
Enabling remote OVSDB managers [  OK  ]


I'm not sure where it's better to fix this. either the spec here
not to create an empty file or in ovsdb/log.c to an accept empty conf.db,
or maybe even upgrade_db() in ovs-lib bash file to call create_db
even if conf.db exists but it's empty.

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