Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd.c: Omit unused columns in SB_Global.

2023-06-13 Thread Han Zhou 
On Tue, Jun 13, 2023 at 6:44 AM Mark Michelson  wrote:
>
> On 6/12/23 22:07, Han Zhou wrote:
> >
> >
> > On Mon, Jun 12, 2023 at 11:05 AM Mark Michelson  > > wrote:
> >  >
> >  > Hi Han,
> >  >
> >  > Acked-by: Mark Michelson  > >
> >  >
> >  > I like the changes, but I don't like that patch 1 on its own results
in
> >  > breaking CI.
> >  >
> >  > To whoever merges this, I suggest reversing the order of the patches.
> >  > This way CI is likely to succeed after patch 1 and will definitely
> >  > succeed after patch 2. This is better than definitely failing after
> >  > patch 1 and definitely succeeding after patch 2.
> >  >
> > Thanks Mark. Is your Ack for both patches? If so, I can reorder and
> > adjust the commit message for that when merging.
> > Or I can do the same and send a v2 waiting for Ack for patch2.
> >
> > Thanks,
> > Han
>
> Sorry for the confusion. My ack was for both patches.
>
Thanks Mark. I applied both patches to main with the order and commit
message adjusted accordingly.

Regards,
Han

> >
> >  > On 6/9/23 15:12, Han Zhou wrote:
> >  > > Connections and SSL columns are not used by ovn-northd. Omit them
in
> >  > > IDL.
> >  > >
> >  > > This is not a functional problem, but it may hide incremental
> > processing
> >  > > problems, because when status changes in Connection table, which is
> >  > > referenced by SB_Global, ovn-northd will receive notifications for
> >  > > SB_Global change, which triggers recompute. Some tests may pass
when
> >  > > this happens, while it should have failed if the recompute were not
> >  > > triggered. An example is the test case "testing propagate
> >  > > Port_Binding.up to NB and OVS", which has passed in most cases but
> > fails
> >  > > permanently afte this change (and will be fixed in a separate
patch).
> >  > >
> >  > > Signed-off-by: Han Zhou mailto:hz...@ovn.org>>
> >  > > ---
> >  > >   northd/ovn-northd.c | 4 
> >  > >   1 file changed, 4 insertions(+)
> >  > >
> >  > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> >  > > index 0b8bbfb95cf7..647f60c8583f 100644
> >  > > --- a/northd/ovn-northd.c
> >  > > +++ b/northd/ovn-northd.c
> >  > > @@ -818,6 +818,10 @@ main(int argc, char *argv[])
> >  > >   ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
> >  > >   ovsdb_idl_set_write_changed_only_all(ovnsb_idl_loop.idl,
true);
> >  > >
> >  > > +/* Omit unused columns. */
> >  > > +ovsdb_idl_omit(ovnsb_idl_loop.idl,
> > _sb_global_col_connections);
> >  > > +ovsdb_idl_omit(ovnsb_idl_loop.idl, _sb_global_col_ssl);
> >  > > +
> >  > >   /* Disable alerting for pure write-only columns. */
> >  > >   ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> > _sb_global_col_nb_cfg);
> >  > >   ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> > _address_set_col_name);
> >  >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v14 4/4] userspace: Enable L4 checksum offloading by default.

2023-06-13 Thread Ilya Maximets 
On 6/2/23 20:59, Mike Pattrick wrote:
> The netdev receiving packets is supposed to provide the flags
> indicating if the L4 checksum was verified and it is OK or BAD,
> otherwise the stack will check when appropriate by software.
> 
> If the packet comes with good checksum, then postpone the
> checksum calculation to the egress device if needed.
> 
> When encapsulate a packet with that flag, set the checksum
> of the inner L4 header since that is not yet supported.
> 
> Calculate the L4 checksum when the packet is going to be sent
> over a device that doesn't support the feature.
> 
> Linux tap devices allows enabling L3 and L4 offload, so this
> patch enables the feature. However, Linux socket interface
> remains disabled because the API doesn't allow enabling
> those two features without enabling TSO too.
> 
> Signed-off-by: Flavio Leitner 
> Co-authored-by: Flavio Leitner 
> Signed-off-by: Mike Pattrick 
> ---
>  Since v9:
>   - Extended miniflow_extract changes into avx512 code
>   - Formatting changes
>   - Note that we cannot currently enable checksum offloading in
> CONFIGURE_VETH_OFFLOADS for check-system-userspace as
> netdev-linux.c currently only parses the vnet header if TSO
> is enabled.
>  Since v10:
>   - No change
>  Since v11:
>   - Added AVX512 IPv6 checksum offload support.
>   - Improved error messages and logging.
>  Since v12:
>   - Added missing mutex annotations
>  Since v13:
>   - Added TUNGETFEATURES check in netdev-linux
> ---
>  lib/conntrack.c  |  15 +-
>  lib/dp-packet.c  |  25 +++
>  lib/dp-packet.h  |  78 +-
>  lib/dpif-netdev-extract-avx512.c |  62 +++-
>  lib/flow.c   |  23 +++
>  lib/netdev-dpdk.c| 176 ++---
>  lib/netdev-linux.c   | 253 ++-
>  lib/netdev-native-tnl.c  |  32 +---
>  lib/netdev.c |  46 ++
>  lib/odp-execute-avx512.c |  88 +++
>  lib/packets.c| 175 -
>  lib/packets.h|   3 +
>  12 files changed, 707 insertions(+), 269 deletions(-)
> 



> @@ -4268,6 +4351,14 @@ destroy_device(int vid)
> dev->up.n_rxq * sizeof *dev->vhost_rxq_enabled);
>  netdev_dpdk_txq_map_clear(dev);
>  
> +/* Clear offload capabilities before next new_device. */
> +dev->hw_ol_features &= ~NETDEV_TX_IPV4_CKSUM_OFFLOAD;
> +dev->hw_ol_features &= ~NETDEV_TX_TCP_CKSUM_OFFLOAD;
> +dev->hw_ol_features &= ~NETDEV_TX_UDP_CKSUM_OFFLOAD;
> +dev->hw_ol_features &= ~NETDEV_TX_SCTP_CKSUM_OFFLOAD;
> +dev->hw_ol_features &= ~NETDEV_TX_TSO_OFFLOAD;

Can we just set it to zero?  To avoid potentially missing some flags.

> +netdev_dpdk_update_netdev_flags(dev);
> +
>  netdev_change_seq_changed(>up);
>  ovs_mutex_unlock(>mutex);
>  exists = true;



> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 36620199e..0ad36ded9 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -530,6 +530,11 @@ static struct vlog_rate_limit rl = 
> VLOG_RATE_LIMIT_INIT(5, 20);
>   * changes in the device miimon status, so we can use atomic_count. */
>  static atomic_count miimon_cnt = ATOMIC_COUNT_INIT(0);
>  
> +/* Very old kernels from the 2.6 era don't support vnet headers with the tun
> + * device. We can detect this while constructing a netdev, but need this for
> + * packet rx/tx. */
> +static bool enable_vnet_hdr = true;
> +
>  static int netdev_linux_parse_vnet_hdr(struct dp_packet *b);
>  static void netdev_linux_prepend_vnet_hdr(struct dp_packet *b, int mtu);
>  static int netdev_linux_do_ethtool(const char *name, struct ethtool_cmd *,
> @@ -938,14 +943,6 @@ netdev_linux_common_construct(struct netdev *netdev_)
>  netnsid_unset(>netnsid);
>  ovs_mutex_init(>mutex);
>  
> -if (userspace_tso_enabled()) {
> -netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
> -netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CKSUM;
> -netdev_->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CKSUM;
> -netdev_->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CKSUM;
> -netdev_->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CKSUM;
> -}
> -
>  return 0;
>  }
>  
> @@ -959,6 +956,16 @@ netdev_linux_construct(struct netdev *netdev_)
>  return error;
>  }
>  
> +/* The socket interface doesn't offer the option to enable only
> + * csum offloading without TSO. */
> +if (userspace_tso_enabled()) {
> +netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
> +netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CKSUM;
> +netdev_->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CKSUM;
> +netdev_->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CKSUM;
> +netdev_->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CKSUM;
> +}
> +
>  error = get_flags(>up, >ifi_flags);
>  if (error

Re: [ovs-dev] [PATCH v14 3/4] userspace: Enable IP checksum offloading by default.

2023-06-13 Thread Ilya Maximets 
On 6/2/23 20:59, Mike Pattrick wrote:
> The netdev receiving packets is supposed to provide the flags
> indicating if the IP checksum was verified and it is GOOD or BAD,
> otherwise the stack will check when appropriate by software.
> 
> If the packet comes with good checksum, then postpone the
> checksum calculation to the egress device if needed.
> 
> When encapsulate a packet with that flag, set the checksum
> of the inner IP header since that is not yet supported.
> 
> Calculate the IP checksum when the packet is going to be sent over
> a device that doesn't support the feature.
> 
> Linux devices don't support IP checksum offload alone, so the
> support is not enabled.
> 
> Signed-off-by: Flavio Leitner 
> Co-authored-by: Flavio Leitner 
> Signed-off-by: Mike Pattrick 
> ---
>  Since v9:
>   - Removed duplicative field tx_ip_csum_offload from netdev-dpdk.c
>   - Left rx_csum_offload field as it is not duplicative
>   - Moved system-userspace-offload.at tests to dpif-netdev.at
>   - Various visual changes
>   - Extended miniflow_extract changes into avx512 code
>  Since v10:
>   - avx512 checksum length corrected
>  Since v11:
>   - If hw-offload and userspace-tso is enabled, don't allow dpdk to
>   offload RAW_ENCAP
>  Since v13:
>   - Removed dpdk hw-offload related code
> ---
>  lib/conntrack.c  | 19 
>  lib/dp-packet.c  | 15 ++
>  lib/dp-packet.h  | 62 +++--
>  lib/dpif-netdev-extract-avx512.c |  5 ++
>  lib/dpif-netdev.c|  2 +
>  lib/flow.c   | 15 --
>  lib/ipf.c| 11 +++--
>  lib/netdev-dpdk.c| 71 +++--
>  lib/netdev-dummy.c   | 22 +
>  lib/netdev-native-tnl.c  | 21 ++---
>  lib/netdev.c | 16 +++
>  lib/odp-execute-avx512.c | 20 +---
>  lib/odp-execute.c| 21 +++--
>  lib/packets.c| 34 +++---
>  tests/dpif-netdev.at | 78 
>  15 files changed, 345 insertions(+), 67 deletions(-)
> 



> @@ -1174,6 +1190,12 @@ netdev_dummy_send(struct netdev *netdev, int qid,
>  }
>  
>  ovs_mutex_lock(>mutex);
> +if (dp_packet_hwol_tx_ip_csum(packet) &&
> +!dp_packet_ip_checksum_good(packet)) {
> +dp_packet_ip_set_header_csum(packet);
> +dp_packet_ol_set_ip_csum_good(packet);
> +}
> +

These do not need to be under a mutex.

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

[ovs-dev] [PATCH v12 8/8] treewide: Add `ovs_assert` to check for null pointers

2023-06-13 Thread James Raphael Tiovalen 
This patch adds an assortment of `ovs_assert` statements to check for
null pointers. We use assertions since it should be impossible for any
of these pointers to be NULL.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 lib/dp-packet.c| 1 +
 lib/dpctl.c| 4 
 lib/odp-execute.c  | 4 
 lib/rtnetlink.c| 4 ++--
 lib/shash.c| 2 +-
 lib/sset.c | 2 ++
 ovsdb/jsonrpc-server.c | 4 
 ovsdb/monitor.c| 3 +++
 ovsdb/ovsdb-server.c   | 2 ++
 ovsdb/ovsdb.c  | 8 +++-
 ovsdb/query.c  | 2 ++
 ovsdb/transaction.c| 2 ++
 utilities/ovs-vsctl.c  | 3 +++
 vtep/vtep-ctl.c| 5 -
 14 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ca29b1f90..059db48ba 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -171,6 +171,7 @@ dp_packet_new_with_headroom(size_t size, size_t headroom)
 struct dp_packet *
 dp_packet_clone(const struct dp_packet *buffer)
 {
+ovs_assert(buffer);
 return dp_packet_clone_with_headroom(buffer, 0);
 }
 
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 15950bd50..51cd6c88c 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -336,6 +336,8 @@ dpctl_add_if(int argc OVS_UNUSED, const char *argv[],
 value = "";
 }
 
+ovs_assert(key);
+
 if (!strcmp(key, "type")) {
 type = value;
 } else if (!strcmp(key, "port_no")) {
@@ -454,6 +456,8 @@ dpctl_set_if(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 value = "";
 }
 
+ovs_assert(key);
+
 if (!strcmp(key, "type")) {
 if (strcmp(value, type)) {
 dpctl_error(dpctl_p, 0,
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5cf6fbec0..44b2cd360 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -147,6 +147,8 @@ odp_set_ipv4(struct dp_packet *packet, const struct 
ovs_key_ipv4 *key,
 uint8_t new_tos;
 uint8_t new_ttl;
 
+ovs_assert(nh);
+
 if (mask->ipv4_src) {
 ip_src_nh = get_16aligned_be32(>ip_src);
 new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src);
@@ -276,6 +278,8 @@ set_arp(struct dp_packet *packet, const struct ovs_key_arp 
*key,
 {
 struct arp_eth_header *arp = dp_packet_l3(packet);
 
+ovs_assert(arp);
+
 if (!mask) {
 arp->ar_op = key->arp_op;
 arp->ar_sha = key->arp_sha;
diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
index f67352603..37078d00e 100644
--- a/lib/rtnetlink.c
+++ b/lib/rtnetlink.c
@@ -112,7 +112,7 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change 
*change)
 if (parsed) {
 const struct ifinfomsg *ifinfo;
 
-ifinfo = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifinfo);
+ifinfo = ofpbuf_at_assert(buf, NLMSG_HDRLEN, sizeof *ifinfo);
 
 /* Wireless events can be spammy and cause a
  * lot of unnecessary churn and CPU load in
@@ -175,7 +175,7 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change 
*change)
 if (parsed) {
 const struct ifaddrmsg *ifaddr;
 
-ifaddr = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifaddr);
+ifaddr = ofpbuf_at_assert(buf, NLMSG_HDRLEN, sizeof *ifaddr);
 
 change->nlmsg_type = nlmsg->nlmsg_type;
 change->if_index   = ifaddr->ifa_index;
diff --git a/lib/shash.c b/lib/shash.c
index b72b5bf27..c712b3769 100644
--- a/lib/shash.c
+++ b/lib/shash.c
@@ -270,7 +270,7 @@ void *
 shash_find_and_delete_assert(struct shash *sh, const char *name)
 {
 void *data = shash_find_and_delete(sh, name);
-ovs_assert(data != NULL);
+ovs_assert(data);
 return data;
 }
 
diff --git a/lib/sset.c b/lib/sset.c
index aa1790020..afdc4392c 100644
--- a/lib/sset.c
+++ b/lib/sset.c
@@ -20,6 +20,7 @@
 
 #include "openvswitch/dynamic-string.h"
 #include "hash.h"
+#include "util.h"
 
 static uint32_t
 hash_name__(const char *name, size_t length)
@@ -261,6 +262,7 @@ char *
 sset_pop(struct sset *set)
 {
 const char *name = SSET_FIRST(set);
+ovs_assert(name);
 char *copy = xstrdup(name);
 sset_delete(set, SSET_NODE_FROM_NAME(name));
 return copy;
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 5361b3c76..a3ca48a7b 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1131,6 +1131,8 @@ static void
 ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
  struct jsonrpc_msg *request)
 {
+ovs_assert(db);
+
 /* Check for duplicate ID. */
 size_t hash = json_hash(request->id, 0);
 struct ovsdb_jsonrpc_trigger *t
@@ -1391,6 +1393,8 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session 
*s, struct ovsdb *db,
  enum ovsdb_monitor_version version,
  const struct json *request_id)
 {
+

[ovs-dev] [PATCH v12 7/8] lib, ovsdb: Add various null pointer checks

2023-06-13 Thread James Raphael Tiovalen 
This commit adds various null pointer checks to some files in the `lib`
and the `ovsdb` directories to fix several Coverity defects. These
changes are grouped together as they perform similar checks, returning
early or skipping some action if a null pointer is encountered.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 lib/dp-packet.c| 8 
 lib/shash.c| 7 ++-
 ovsdb/jsonrpc-server.c | 2 +-
 ovsdb/monitor.c| 7 ++-
 ovsdb/ovsdb-client.c   | 7 ++-
 ovsdb/row.c| 4 +++-
 6 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 445cb4761..ca29b1f90 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -353,7 +353,7 @@ void *
 dp_packet_put_zeros(struct dp_packet *b, size_t size)
 {
 void *dst = dp_packet_put_uninit(b, size);
-memset(dst, 0, size);
+nullable_memset(dst, 0, size);
 return dst;
 }
 
@@ -364,7 +364,7 @@ void *
 dp_packet_put(struct dp_packet *b, const void *p, size_t size)
 {
 void *dst = dp_packet_put_uninit(b, size);
-memcpy(dst, p, size);
+nullable_memcpy(dst, p, size);
 return dst;
 }
 
@@ -436,7 +436,7 @@ void *
 dp_packet_push_zeros(struct dp_packet *b, size_t size)
 {
 void *dst = dp_packet_push_uninit(b, size);
-memset(dst, 0, size);
+nullable_memset(dst, 0, size);
 return dst;
 }
 
@@ -447,7 +447,7 @@ void *
 dp_packet_push(struct dp_packet *b, const void *p, size_t size)
 {
 void *dst = dp_packet_push_uninit(b, size);
-memcpy(dst, p, size);
+nullable_memcpy(dst, p, size);
 return dst;
 }
 
diff --git a/lib/shash.c b/lib/shash.c
index 2bfc8eb50..b72b5bf27 100644
--- a/lib/shash.c
+++ b/lib/shash.c
@@ -205,8 +205,13 @@ shash_delete(struct shash *sh, struct shash_node *node)
 char *
 shash_steal(struct shash *sh, struct shash_node *node)
 {
-char *name = node->name;
+char *name;
 
+if (!node) {
+return NULL;
+}
+
+name = node->name;
 hmap_remove(>map, >node);
 free(node);
 return name;
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 17868f5b7..5361b3c76 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1038,7 +1038,7 @@ ovsdb_jsonrpc_session_got_request(struct 
ovsdb_jsonrpc_session *s,
  request->id);
 } else if (!strcmp(request->method, "get_schema")) {
 struct ovsdb *db = ovsdb_jsonrpc_lookup_db(s, request, );
-if (!reply) {
+if (db && !reply) {
 reply = jsonrpc_create_reply(ovsdb_schema_to_json(db->schema),
  request->id);
 }
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index c32af7b02..b560b0745 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -483,7 +483,9 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
 struct ovsdb_monitor_column *c;
 
 mt = shash_find_data(>tables, table->schema->name);
-
+if (!mt) {
+return NULL;
+}
 /* Check for column duplication. Return duplicated column name. */
 if (mt->columns_index_map[column->index] != -1) {
 return column->name;
@@ -810,6 +812,9 @@ ovsdb_monitor_table_condition_update(
 
 struct ovsdb_monitor_table_condition *mtc =
 shash_find_data(>tables, table->schema->name);
+if (!mtc) {
+return NULL;
+}
 struct ovsdb_error *error;
 struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER();
 
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 46484630d..2b12907b6 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -1873,6 +1873,10 @@ do_dump(struct jsonrpc *rpc, const char *database,
 n_tables = shash_count(>tables);
 }
 
+if (!tables) {
+goto end;
+}
+
 /* Construct transaction to retrieve entire database. */
 transaction = json_array_create_1(json_string_create(database));
 for (i = 0; i < n_tables; i++) {
@@ -1932,8 +1936,9 @@ do_dump(struct jsonrpc *rpc, const char *database,
 }
 
 jsonrpc_msg_destroy(reply);
-shash_destroy(_columns);
 free(tables);
+end:
+shash_destroy(_columns);
 ovsdb_schema_destroy(schema);
 }
 
diff --git a/ovsdb/row.c b/ovsdb/row.c
index d7bfbdd36..9f87c832a 100644
--- a/ovsdb/row.c
+++ b/ovsdb/row.c
@@ -399,7 +399,9 @@ ovsdb_row_set_add_row(struct ovsdb_row_set *set, const 
struct ovsdb_row *row)
 set->rows = x2nrealloc(set->rows, >allocated_rows,
sizeof *set->rows);
 }
-set->rows[set->n_rows++] = row;
+if (set->rows) {
+set->rows[set->n_rows++] = row;
+}
 }
 
 struct json *
-- 
2.40.1

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

[ovs-dev] [PATCH v12 5/8] file, monitor: Add null pointer assertions for old and new ovsdb_rows

2023-06-13 Thread James Raphael Tiovalen 
This commit adds non-null pointer assertions in some code that performs
some decisions based on old and new input ovsdb_rows.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 ovsdb/file.c| 2 ++
 ovsdb/monitor.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 2d887e53e..b1d386e76 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -522,7 +522,9 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn,
 }
 
 if (row) {
+ovs_assert(new || old);
 struct ovsdb_table *table = new ? new->table : old->table;
+ovs_assert(table);
 char uuid[UUID_LEN + 1];
 
 if (table != ftxn->table) {
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 4afaa89f4..c32af7b02 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -1372,8 +1372,10 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old,
  const struct ovsdb_monitor_table *mt,
  struct ovsdb_monitor_change_set_for_table *mcst)
 {
+ovs_assert(new || old);
 const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old);
-struct ovsdb_monitor_row *change;
+ovs_assert(uuid);
+struct ovsdb_monitor_row *change = NULL;
 
 change = ovsdb_monitor_changes_row_find(mcst, uuid);
 if (!change) {
-- 
2.40.1

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

[ovs-dev] [PATCH v12 6/8] ovs-vsctl: Fix crash when routing is enabled

2023-06-13 Thread James Raphael Tiovalen 
In the case where routing is enabled, the bridge member of the
`vsctl_port` structs is not populated. This can cause a crash if we
attempt to access it. This patch fixes the crash by checking if the
bridge member is valid before attempting to access it. In the
`check_conflicts` function, we print both the port name and the bridge
name if routing is disabled and we only print the port name if routing
is enabled.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 utilities/ovs-vsctl.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index b3c75f8ba..f55c2965a 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -888,14 +888,23 @@ check_conflicts(struct vsctl_context *vsctl_ctx, const 
char *name,
 
 port = shash_find_data(_ctx->ports, name);
 if (port) {
-ctl_fatal("%s because a port named %s already exists on "
-"bridge %s", msg, name, port->bridge->name);
+if (port->bridge) {
+ctl_fatal("%s because a port named %s already exists on "
+  "bridge %s", msg, name, port->bridge->name);
+} else {
+ctl_fatal("%s because a port named %s already exists", msg, name);
+}
 }
 
 iface = shash_find_data(_ctx->ifaces, name);
 if (iface) {
-ctl_fatal("%s because an interface named %s already exists "
-"on bridge %s", msg, name, iface->port->bridge->name);
+if (iface->port->bridge) {
+ctl_fatal("%s because an interface named %s already exists "
+  "on bridge %s", msg, name, iface->port->bridge->name);
+} else {
+ctl_fatal("%s because an interface named %s already exists", msg,
+  name);
+}
 }
 
 free(msg);
@@ -935,7 +944,7 @@ find_port(struct vsctl_context *vsctl_ctx, const char 
*name, bool must_exist)
 ovs_assert(vsctl_ctx->cache_valid);
 
 port = shash_find_data(_ctx->ports, name);
-if (port && !strcmp(name, port->bridge->name)) {
+if (port && port->bridge && !strcmp(name, port->bridge->name)) {
 port = NULL;
 }
 if (must_exist && !port) {
@@ -953,7 +962,8 @@ find_iface(struct vsctl_context *vsctl_ctx, const char 
*name, bool must_exist)
 ovs_assert(vsctl_ctx->cache_valid);
 
 iface = shash_find_data(_ctx->ifaces, name);
-if (iface && !strcmp(name, iface->port->bridge->name)) {
+if (iface && iface->port->bridge &&
+!strcmp(name, iface->port->bridge->name)) {
 iface = NULL;
 }
 if (must_exist && !iface) {
-- 
2.40.1

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

[ovs-dev] [PATCH v12 2/8] lib, ovs-vsctl: Add zero-initializations

2023-06-13 Thread James Raphael Tiovalen 
This commit adds zero-initializations by changing `SFL_ALLOC` from
`malloc` to `xzalloc`, adding a `memset` call to `sflAlloc`,
initializing a `pollfd` struct variable with zeroes, and changing some
calls to `xmalloc` to `xzalloc`. This is to prevent potential data leaks
or undefined behavior from potentially uninitialized variables.

Some variables would always be initialized by either the code flow or
the compiler. Thus, some of the associated Coverity reports might be
false positives. That said, it is still considered best practice to
zero-initialize variables upfront just in case to ensure the overall
resilience and security of OVS, as long as they do not impact
performance-critical code. As a bonus, it would also make static
analyzer tools, such as Coverity, happy.

Signed-off-by: James Raphael Tiovalen 
---
 lib/latch-unix.c  |  2 +-
 lib/sflow_agent.c | 12 ++--
 lib/sflow_api.h   |  2 +-
 utilities/ovs-vsctl.c |  5 ++---
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/lib/latch-unix.c b/lib/latch-unix.c
index f4d10c39a..c62bb024b 100644
--- a/lib/latch-unix.c
+++ b/lib/latch-unix.c
@@ -71,7 +71,7 @@ latch_set(struct latch *latch)
 bool
 latch_is_set(const struct latch *latch)
 {
-struct pollfd pfd;
+struct pollfd pfd = {0};
 int retval;
 
 pfd.fd = latch->fds[0];
diff --git a/lib/sflow_agent.c b/lib/sflow_agent.c
index c95f654a5..b15758ae1 100644
--- a/lib/sflow_agent.c
+++ b/lib/sflow_agent.c
@@ -510,8 +510,16 @@ void sfl_agent_sysError(SFLAgent *agent, char *modName, 
char *msg)
 
 static void * sflAlloc(SFLAgent *agent, size_t bytes)
 {
-if(agent->allocFn) return (*agent->allocFn)(agent->magic, agent, bytes);
-else return SFL_ALLOC(bytes);
+void *alloc;
+if (agent->allocFn) {
+alloc = (*agent->allocFn)(agent->magic, agent, bytes);
+ovs_assert(alloc);
+memset(alloc, 0, bytes);
+} else {
+alloc = SFL_ALLOC(bytes);
+ovs_assert(alloc);
+}
+return alloc;
 }
 
 static void sflFree(SFLAgent *agent, void *obj)
diff --git a/lib/sflow_api.h b/lib/sflow_api.h
index a0530b37a..eb23e2acd 100644
--- a/lib/sflow_api.h
+++ b/lib/sflow_api.h
@@ -337,7 +337,7 @@ void sfl_agent_sysError(SFLAgent *agent, char *modName, 
char *msg);
 
 u_int32_t sfl_receiver_samplePacketsSent(SFLReceiver *receiver);
 
-#define SFL_ALLOC malloc
+#define SFL_ALLOC xzalloc
 #define SFL_FREE free
 
 #endif /* SFLOW_API_H */
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 2f5ac1a26..b3c75f8ba 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -575,7 +575,7 @@ add_bridge_to_cache(struct vsctl_context *vsctl_ctx,
 struct ovsrec_bridge *br_cfg, const char *name,
 struct vsctl_bridge *parent, int vlan)
 {
-struct vsctl_bridge *br = xmalloc(sizeof *br);
+struct vsctl_bridge *br = xzalloc(sizeof *br);
 br->br_cfg = br_cfg;
 br->name = xstrdup(name);
 ovs_list_init(>ports);
@@ -659,7 +659,7 @@ static struct vsctl_port *
 add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge *parent,
   struct ovsrec_port *port_cfg)
 {
-struct vsctl_port *port;
+struct vsctl_port *port = xzalloc(sizeof *port);
 
 if (port_cfg->tag
 && *port_cfg->tag >= 0 && *port_cfg->tag <= 4095) {
@@ -671,7 +671,6 @@ add_port_to_cache(struct vsctl_context *vsctl_ctx, struct 
vsctl_bridge *parent,
 }
 }
 
-port = xmalloc(sizeof *port);
 ovs_list_push_back(>ports, >ports_node);
 ovs_list_init(>ifaces);
 port->port_cfg = port_cfg;
-- 
2.40.1

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

[ovs-dev] [PATCH v12 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James Raphael Tiovalen 
This commit adds a few null pointer assertions and checks to some return
values of `ovsdb_table_schema_get_column`. If a null pointer is
encountered in these blocks, either the assertion will fail or the
control flow will now be redirected to alternative paths which will
output the appropriate error messages.

A few ovsdb-rbac and ovsdb-server tests are also updated to verify the
expected warning logs by adding said logs to the ALLOWLIST of the
OVSDB_SERVER_SHUTDOWN statements.

Signed-off-by: James Raphael Tiovalen 
---
 ovsdb/condition.c | 5 -
 ovsdb/ovsdb-client.c  | 7 +--
 ovsdb/ovsdb-util.c| 6 ++
 tests/ovsdb-rbac.at   | 4 +++-
 tests/ovsdb-server.at | 8 ++--
 5 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/ovsdb/condition.c b/ovsdb/condition.c
index 09c89b2a0..5a3eb4e8a 100644
--- a/ovsdb/condition.c
+++ b/ovsdb/condition.c
@@ -47,7 +47,10 @@ ovsdb_clause_from_json(const struct ovsdb_table_schema *ts,
 
 /* Column and arg fields are not being used with boolean functions.
  * Use dummy values */
-clause->column = ovsdb_table_schema_get_column(ts, "_uuid");
+const struct ovsdb_column *uuid_column =
+  ovsdb_table_schema_get_column(ts, "_uuid");
+ovs_assert(uuid_column);
+clause->column = uuid_column;
 clause->index = clause->column->index;
 ovsdb_datum_init_default(>arg, >column->type);
 return NULL;
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index bae2c5f04..46484630d 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -1232,8 +1232,11 @@ parse_monitor_columns(char *arg, const char *server, 
const char *database,
 }
 free(nodes);
 
-add_column(server, ovsdb_table_schema_get_column(table, "_version"),
-   columns, columns_json);
+const struct ovsdb_column *version_column =
+ovsdb_table_schema_get_column(table, "_version");
+
+ovs_assert(version_column);
+add_column(server, version_column, columns, columns_json);
 }
 
 if (!initial || !insert || !delete || !modify) {
diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c
index 303191dc8..0b9e1df54 100644
--- a/ovsdb/ovsdb-util.c
+++ b/ovsdb/ovsdb-util.c
@@ -291,9 +291,15 @@ ovsdb_util_write_string_string_column(struct ovsdb_row 
*row,
 size_t i;
 
 column = ovsdb_table_schema_get_column(row->table->schema, column_name);
+if (!column) {
+VLOG_WARN("No %s column present in the %s table",
+ column_name, row->table->schema->name);
+goto unwind;
+}
 datum = ovsdb_util_get_datum(row, column_name, OVSDB_TYPE_STRING,
 OVSDB_TYPE_STRING, UINT_MAX);
 if (!datum) {
+unwind:
 for (i = 0; i < n; i++) {
 free(keys[i]);
 free(values[i]);
diff --git a/tests/ovsdb-rbac.at b/tests/ovsdb-rbac.at
index 7de3711fb..3172e4bf5 100644
--- a/tests/ovsdb-rbac.at
+++ b/tests/ovsdb-rbac.at
@@ -371,5 +371,7 @@ cat stdout >> output
 AT_CHECK([uuidfilt stdout], [0], [[[{"count":1}]]
 ], [ignore])
 
-OVSDB_SERVER_SHUTDOWN
+OVSDB_SERVER_SHUTDOWN(["
+  /No status column present in the Connection table/d
+"])
 AT_CLEANUP
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index b53ab8f52..8ccec80bc 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -428,7 +428,9 @@ AT_CHECK(
   
[[[{"rows":[{"managers":"punix:socket1"}]},{"rows":[{"is_connected":false,"target":"punix:socket2"}]}]
 ]], 
   [ignore])
-OVSDB_SERVER_SHUTDOWN
+OVSDB_SERVER_SHUTDOWN(["
+  /No status column present in the Manager table/d
+"])
 AT_CLEANUP
 
 AT_SETUP([ovsdb-server/add-remote and remove-remote])
@@ -2110,7 +2112,9 @@ AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT \
 cat stdout >> output
 AT_CHECK([uuidfilt output], [0], [[[{"details":"insert operation not allowed 
when database server is in read only mode","error":"not allowed"}]]
 ], [ignore])
-OVSDB_SERVER_SHUTDOWN
+OVSDB_SERVER_SHUTDOWN(["
+  /No status column present in the Manager table/d
+"])
 AT_CLEANUP
 
 AT_SETUP([ovsdb-server replication with schema mismatch])
-- 
2.40.1

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

[ovs-dev] [PATCH v12 3/8] shash, simap, smap: Add assertions to `*_count` functions

2023-06-13 Thread James Raphael Tiovalen 
This commit adds assertions in the functions `shash_count`,
`simap_count`, and `smap_count` to ensure that the corresponding input
struct pointer is not NULL.

This ensures that if the return values of `shash_sort`, `simap_sort`,
or `smap_sort` are NULL, then the following for loops would not attempt
to access the pointer, which might result in segmentation faults or
undefined behavior.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 lib/shash.c | 2 ++
 lib/simap.c | 2 ++
 lib/smap.c  | 1 +
 3 files changed, 5 insertions(+)

diff --git a/lib/shash.c b/lib/shash.c
index a7b2c6458..2bfc8eb50 100644
--- a/lib/shash.c
+++ b/lib/shash.c
@@ -17,6 +17,7 @@
 #include 
 #include "openvswitch/shash.h"
 #include "hash.h"
+#include "util.h"
 
 static struct shash_node *shash_find__(const struct shash *,
const char *name, size_t name_len,
@@ -100,6 +101,7 @@ shash_is_empty(const struct shash *shash)
 size_t
 shash_count(const struct shash *shash)
 {
+ovs_assert(shash);
 return hmap_count(>map);
 }
 
diff --git a/lib/simap.c b/lib/simap.c
index 0ee08d74d..1c01d4ebe 100644
--- a/lib/simap.c
+++ b/lib/simap.c
@@ -17,6 +17,7 @@
 #include 
 #include "simap.h"
 #include "hash.h"
+#include "util.h"
 
 static size_t hash_name(const char *, size_t length);
 static struct simap_node *simap_find__(const struct simap *,
@@ -84,6 +85,7 @@ simap_is_empty(const struct simap *simap)
 size_t
 simap_count(const struct simap *simap)
 {
+ovs_assert(simap);
 return hmap_count(>map);
 }
 
diff --git a/lib/smap.c b/lib/smap.c
index 47fb34502..122adca27 100644
--- a/lib/smap.c
+++ b/lib/smap.c
@@ -300,6 +300,7 @@ smap_is_empty(const struct smap *smap)
 size_t
 smap_count(const struct smap *smap)
 {
+ovs_assert(smap);
 return hmap_count(>map);
 }
 
-- 
2.40.1

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

[ovs-dev] [PATCH v12 1/8] lib: Add non-null assertions to some return values of `dp_packet_data`

2023-06-13 Thread James Raphael Tiovalen 
This commit adds some `ovs_assert()` checks to some return values of
`dp_packet_data()` to ensure that they are not NULL and to prevent
null-pointer dereferences, which might lead to unwanted crashes. We use
assertions since it should be impossible for these calls to
`dp_packet_data()` to return NULL.

Signed-off-by: James Raphael Tiovalen 
Reviewed-by: Simon Horman 
---
 lib/dp-packet.c | 15 ++-
 lib/netdev-native-tnl.c | 17 +++--
 lib/pcap-file.c |  4 +++-
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ae8ab5800..445cb4761 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -183,9 +183,12 @@ dp_packet_clone_with_headroom(const struct dp_packet 
*buffer, size_t headroom)
 struct dp_packet *new_buffer;
 uint32_t mark;
 
-new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
- dp_packet_size(buffer),
- headroom);
+const void *data_dp = dp_packet_data(buffer);
+ovs_assert(data_dp);
+
+new_buffer = dp_packet_clone_data_with_headroom(data_dp,
+dp_packet_size(buffer),
+headroom);
 /* Copy the following fields into the returned buffer: l2_pad_size,
  * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
 memcpy(_buffer->l2_pad_size, >l2_pad_size,
@@ -322,8 +325,10 @@ dp_packet_shift(struct dp_packet *b, int delta)
: true);
 
 if (delta != 0) {
-char *dst = (char *) dp_packet_data(b) + delta;
-memmove(dst, dp_packet_data(b), dp_packet_size(b));
+const void *data_dp = dp_packet_data(b);
+ovs_assert(data_dp);
+char *dst = (char *) data_dp + delta;
+memmove(dst, data_dp, dp_packet_size(b));
 dp_packet_set_data(b, dst);
 }
 }
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index c2c6ca559..7781b162d 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -43,6 +43,7 @@
 #include "seq.h"
 #include "unaligned.h"
 #include "unixctl.h"
+#include "util.h"
 #include "openvswitch/vlog.h"
 
 VLOG_DEFINE_THIS_MODULE(native_tnl);
@@ -222,12 +223,13 @@ netdev_tnl_calc_udp_csum(struct udp_header *udp, struct 
dp_packet *packet,
 {
 uint32_t csum;
 
-if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
-csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr(
- dp_packet_data(packet)));
+void *data_dp = dp_packet_data(packet);
+ovs_assert(data_dp);
+
+if (netdev_tnl_is_header_ipv6(data_dp)) {
+csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr(data_dp));
 } else {
-csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr(
-dp_packet_data(packet)));
+csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr(data_dp));
 }
 
 csum = csum_continue(csum, udp, ip_tot_size);
@@ -428,7 +430,10 @@ netdev_gre_pop_header(struct dp_packet *packet)
 struct flow_tnl *tnl = >tunnel;
 int hlen = sizeof(struct eth_header) + 4;
 
-hlen += netdev_tnl_is_header_ipv6(dp_packet_data(packet)) ?
+const void *data_dp = dp_packet_data(packet);
+ovs_assert(data_dp);
+
+hlen += netdev_tnl_is_header_ipv6(data_dp) ?
 IPV6_HEADER_LEN : IP_HEADER_LEN;
 
 pkt_metadata_init_tnl(md);
diff --git a/lib/pcap-file.c b/lib/pcap-file.c
index 3ed7ea488..9f4e2e1e2 100644
--- a/lib/pcap-file.c
+++ b/lib/pcap-file.c
@@ -284,6 +284,8 @@ ovs_pcap_write(struct pcap_file *p_file, struct dp_packet 
*buf)
 struct timeval tv;
 
 ovs_assert(dp_packet_is_eth(buf));
+const void *data_dp = dp_packet_data(buf);
+ovs_assert(data_dp);
 
 xgettimeofday();
 prh.ts_sec = tv.tv_sec;
@@ -291,7 +293,7 @@ ovs_pcap_write(struct pcap_file *p_file, struct dp_packet 
*buf)
 prh.incl_len = dp_packet_size(buf);
 prh.orig_len = dp_packet_size(buf);
 ignore(fwrite(, sizeof prh, 1, p_file->file));
-ignore(fwrite(dp_packet_data(buf), dp_packet_size(buf), 1, p_file->file));
+ignore(fwrite(data_dp, dp_packet_size(buf), 1, p_file->file));
 fflush(p_file->file);
 }
 
-- 
2.40.1

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

[ovs-dev] [PATCH v12 0/8] treewide: Fix multiple Coverity defects

2023-06-13 Thread James Raphael Tiovalen 
This cleanup patchset addresses several high and medium-impact Coverity
defects.

Unit tests have been successfully executed via `make check` and they
successfully passed.

The list of revisions so far:

v2:
Fix some apply-robot checkpatch errors and warnings.

v3:
Fix some apply-robot checkpatch errors and warnings.

v4:
- Fix some apply-robot checkpatch errors and warnings.
- Fix github-robot build failure: linux clang test asan.

v5:
Improve commit message to better describe the intent of this patch.

v6:
- Convert some explicit null pointer checks to assertions since they are
checking for impossible conditions.
- Use `nullable_memset()` and `nullable_memcpy()` instead of manually
performing null checks for `memset()` and `memcpy()`.

v7:
- Split the single commit into a patch series which consists of 8
patches to make it easier to review and help keep things organized.
- Incorporated feedback from Aaron Conole.

v8:
Incorporated feedback from Simon Horman and Ilya Maximets.

v9:
Fix warning for patch 2/8 due to using `sizeof` on a void pointer.

v10:
Resolve merge conflict for patch 8/8.

v11:
Fix order of calling `ovs_assert` before `memset` in patch 2/8.

v12:
Fix failure of some tests in patch 4/8 by checking for warning logs in
OVSDB_SERVER_SHUTDOWN.

James Raphael Tiovalen (8):
  lib: Add non-null assertions to some return values of `dp_packet_data`
  lib, ovs-vsctl: Add zero-initializations
  shash, simap, smap: Add assertions to `*_count` functions
  ovsdb: Assert and check return values of
`ovsdb_table_schema_get_column`
  file, monitor: Add null pointer assertions for old and new ovsdb_rows
  ovs-vsctl: Fix crash when routing is enabled
  lib, ovsdb: Add various null pointer checks
  treewide: Add `ovs_assert` to check for null pointers

 lib/dp-packet.c | 24 +++-
 lib/dpctl.c |  4 
 lib/latch-unix.c|  2 +-
 lib/netdev-native-tnl.c | 17 +++--
 lib/odp-execute.c   |  4 
 lib/pcap-file.c |  4 +++-
 lib/rtnetlink.c |  4 ++--
 lib/sflow_agent.c   | 12 ++--
 lib/sflow_api.h |  2 +-
 lib/shash.c | 11 +--
 lib/simap.c |  2 ++
 lib/smap.c  |  1 +
 lib/sset.c  |  2 ++
 ovsdb/condition.c   |  5 -
 ovsdb/file.c|  2 ++
 ovsdb/jsonrpc-server.c  |  6 +-
 ovsdb/monitor.c | 14 --
 ovsdb/ovsdb-client.c| 14 +++---
 ovsdb/ovsdb-server.c|  2 ++
 ovsdb/ovsdb-util.c  |  6 ++
 ovsdb/ovsdb.c   |  8 +++-
 ovsdb/query.c   |  2 ++
 ovsdb/row.c |  4 +++-
 ovsdb/transaction.c |  2 ++
 tests/ovsdb-rbac.at |  4 +++-
 tests/ovsdb-server.at   |  8 ++--
 utilities/ovs-vsctl.c   | 30 +-
 vtep/vtep-ctl.c |  5 -
 28 files changed, 155 insertions(+), 46 deletions(-)

--
2.40.1

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

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James R T 
On Wed, Jun 14, 2023 at 1:38 AM Simon Horman  wrote:
>
> > That said, I do have one question before I submit the next version of
> > this patch. Should I change the log severity from an error to a
> > warning instead since it seems that there are test cases that will
> > print the log message? Or should I keep it as an error?
>
> I guess the distinction between an error and a warning
> is somewhat subjective.
>
> If the log is occurring during tests does it indicate that it's something
> that can occur during normal usage. And doesn't adversely affect the
> overall running of the system?. Then perhaps it is more of a warning than
> an error. But is just my opinion.

Understood Simon. Will change it to a warning for now since it should
not adversely impact the system. If there are any further valid
concerns in the future, we can always change it back to an error.

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

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread Simon Horman 
On Wed, Jun 14, 2023 at 12:09:42AM +0800, James R T wrote:
> Hi Simon,
> 
> On Mon, Jun 12, 2023 at 10:56 PM Simon Horman  
> wrote:
> >
> > On Mon, Jun 05, 2023 at 01:30:03AM +0800, James Raphael Tiovalen wrote:
> > > This commit adds a few null pointer assertions and checks to some return
> > > values of `ovsdb_table_schema_get_column`. If a null pointer is
> > > encountered in these blocks, either the assertion will fail or the
> > > control flow will now be redirected to alternative paths which will
> > > output the appropriate error messages.
> > >
> > > Signed-off-by: James Raphael Tiovalen 
> > > Reviewed-by: Simon Horman 
> >
> > Hi James,
> >
> > unfortunately the CI is showing a number of failures with this patch
> > applied. I haven't dug into them, but the first is the "ovsdb-server rbac"
> > test for the "linux gcc test" run.
> >
> > Link: 
> > https://github.com/ovsrobot/ovs/actions/runs/5170413012/jobs/9313335549#step:11:5574
> 
> I have identified the cause of the test failures. The commit
> d56366bfa05b90e7b610716ebf9164bfd06e25f1 added a check for
> ovsdb-server logs for any warnings or errors. Since this patch adds an
> additional error message in the
> `ovsdb_util_write_string_string_column` function, which is unexpected
> from the perspective of some of those tests, they fail.
> 
> To fix this, we can simply add the error messages as expected to the
> ALLOWLIST of the OVSDB_SERVER_SHUTDOWN statements within the tests.

Thanks, that sounds sensible to me.

> That said, I do have one question before I submit the next version of
> this patch. Should I change the log severity from an error to a
> warning instead since it seems that there are test cases that will
> print the log message? Or should I keep it as an error?

I guess the distinction between an error and a warning
is somewhat subjective.

If the log is occurring during tests does it indicate that it's something
that can occur during normal usage. And doesn't adversely affect the
overall running of the system?. Then perhaps it is more of a warning than
an error. But is just my opinion.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v11 4/8] ovsdb: Assert and check return values of `ovsdb_table_schema_get_column`

2023-06-13 Thread James R T 
Hi Simon,

On Mon, Jun 12, 2023 at 10:56 PM Simon Horman  wrote:
>
> On Mon, Jun 05, 2023 at 01:30:03AM +0800, James Raphael Tiovalen wrote:
> > This commit adds a few null pointer assertions and checks to some return
> > values of `ovsdb_table_schema_get_column`. If a null pointer is
> > encountered in these blocks, either the assertion will fail or the
> > control flow will now be redirected to alternative paths which will
> > output the appropriate error messages.
> >
> > Signed-off-by: James Raphael Tiovalen 
> > Reviewed-by: Simon Horman 
>
> Hi James,
>
> unfortunately the CI is showing a number of failures with this patch
> applied. I haven't dug into them, but the first is the "ovsdb-server rbac"
> test for the "linux gcc test" run.
>
> Link: 
> https://github.com/ovsrobot/ovs/actions/runs/5170413012/jobs/9313335549#step:11:5574

I have identified the cause of the test failures. The commit
d56366bfa05b90e7b610716ebf9164bfd06e25f1 added a check for
ovsdb-server logs for any warnings or errors. Since this patch adds an
additional error message in the
`ovsdb_util_write_string_string_column` function, which is unexpected
from the perspective of some of those tests, they fail.

To fix this, we can simply add the error messages as expected to the
ALLOWLIST of the OVSDB_SERVER_SHUTDOWN statements within the tests.
That said, I do have one question before I submit the next version of
this patch. Should I change the log severity from an error to a
warning instead since it seems that there are test cases that will
print the log message? Or should I keep it as an error?

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

[ovs-dev] [PATCH ovn] tests: Add missing FOR_EACH_NORTHD

2023-06-13 Thread Ales Musil 
Add missing OVN_FOR_EACH_NORTHD around
FDB aging test.

Signed-off-by: Ales Musil 
---
 tests/ovn.at | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ovn.at b/tests/ovn.at
index e49148411..7f83ba8ab 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -36181,6 +36181,7 @@ OVS_WAIT_UNTIL([test $(as hv-1 ovs-vsctl list queue | 
grep -c 'burst="80
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
 AT_SETUP([FDB aging])
 ovn_start
 
-- 
2.40.1

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

Re: [ovs-dev] [PATCH ovn 1/2] ovn-northd.c: Omit unused columns in SB_Global.

2023-06-13 Thread Mark Michelson 

On 6/12/23 22:07, Han Zhou wrote:



On Mon, Jun 12, 2023 at 11:05 AM Mark Michelson > wrote:

 >
 > Hi Han,
 >
 > Acked-by: Mark Michelson >

 >
 > I like the changes, but I don't like that patch 1 on its own results in
 > breaking CI.
 >
 > To whoever merges this, I suggest reversing the order of the patches.
 > This way CI is likely to succeed after patch 1 and will definitely
 > succeed after patch 2. This is better than definitely failing after
 > patch 1 and definitely succeeding after patch 2.
 >
Thanks Mark. Is your Ack for both patches? If so, I can reorder and 
adjust the commit message for that when merging.

Or I can do the same and send a v2 waiting for Ack for patch2.

Thanks,
Han


Sorry for the confusion. My ack was for both patches.



 > On 6/9/23 15:12, Han Zhou wrote:
 > > Connections and SSL columns are not used by ovn-northd. Omit them in
 > > IDL.
 > >
 > > This is not a functional problem, but it may hide incremental 
processing

 > > problems, because when status changes in Connection table, which is
 > > referenced by SB_Global, ovn-northd will receive notifications for
 > > SB_Global change, which triggers recompute. Some tests may pass when
 > > this happens, while it should have failed if the recompute were not
 > > triggered. An example is the test case "testing propagate
 > > Port_Binding.up to NB and OVS", which has passed in most cases but 
fails

 > > permanently afte this change (and will be fixed in a separate patch).
 > >
 > > Signed-off-by: Han Zhou mailto:hz...@ovn.org>>
 > > ---
 > >   northd/ovn-northd.c | 4 
 > >   1 file changed, 4 insertions(+)
 > >
 > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
 > > index 0b8bbfb95cf7..647f60c8583f 100644
 > > --- a/northd/ovn-northd.c
 > > +++ b/northd/ovn-northd.c
 > > @@ -818,6 +818,10 @@ main(int argc, char *argv[])
 > >       ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
 > >       ovsdb_idl_set_write_changed_only_all(ovnsb_idl_loop.idl, true);
 > >
 > > +    /* Omit unused columns. */
 > > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, 
_sb_global_col_connections);

 > > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, _sb_global_col_ssl);
 > > +
 > >       /* Disable alerting for pure write-only columns. */
 > >       ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, 
_sb_global_col_nb_cfg);
 > >       ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, 
_address_set_col_name);

 >


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

Re: [ovs-dev] [PATCH 1/2] utilities: add "--detach" option to ovs-ctl

2023-06-13 Thread Vladislav Odintsov 
Hi Ilya,

thanks for the attention on this patchset.

> On 13 Jun 2023, at 14:58, Ilya Maximets  wrote:
> 
> On 6/7/23 08:33, Vladislav Odintsov wrote:
>> Default is yes (current behavior).  This means that after start process
>> will be run in background.  When "no" is given, process is run in
>> foreground with `exec` call, which replaces current process (ovs-ctl) with
>> wanted ovs process (ovsdb-server or ovs-vswitchd).
>> 
>> This option is useful when running ovs-ctl inside docker container to make
>> ovs* process to be a pid 1.
>> 
>> Note, that with `--detach=no` database settings initialization is not done.
>> db-version, system-ids are not set and transient ports are not deleted.
> 
> Hi, Vladislav.
> 
> It seems like this option makes too many compromises and not really possible
> to use with many of the other options.  The main use case for ovs-ctl from
> the beginning was to automate managing of two processes (ovsdb-server and
> ovs-vswitchd) at the same time.  Later the ability to manage only one of them
> was added.  With this new option it will also be not possible to run both.

If we’re talking about docker container, where it’s not common practive to run 
more than one process inside of container, so I think it should not be a 
surprise that two processes can’t be run with using this option.
Maybe it should have been documented in more details instead? To remove 
improper understanding of this option?

> 
> Why not just starting ovsdb/vswitchd processes explicitly in the container?
> 

I do really like how ovs-ctl script parametrises OVS daemons to run in 
systemd.service unit files and prepares/maintains db files (including schema 
update).
Recently I was looking for official solutions how to run OVS daemons inside 
docker and haven’t found anything.
I guess it’s a problem for the OVS project since it’s definitely used by many 
people to run inside containers and how to create and spawn them is outside of 
OVS project and everybody decides each time in its own way...
Using ovs-ctl script helps maintain same argument set for both: systemd and 
in-container process.

Moreover, as a user, I’d be very happy if I can just get official Dockerfile 
for popular platforms to run OVS daemons. Just build and run.

What do you think?

Maybe I’m wrong :)
How do you maintain OVS daemons inside containers? Just write CMD ["sh", "-c", 
"ovsdb-server $OVSDB_SERVER_OPTIONS"]?


> Best regards, Ilya Maximets.
> 
>> 
>> Signed-off-by: Vladislav Odintsov 
>> ---
>> utilities/ovs-ctl.in | 5 +
>> utilities/ovs-lib.in | 8 +++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
>> index 0b2820c36..72c8881e3 100644
>> --- a/utilities/ovs-ctl.in
>> +++ b/utilities/ovs-ctl.in
>> @@ -338,6 +338,7 @@ set_defaults () {
>> DUMP_HUGEPAGES=no
>> MLOCKALL=yes
>> SELF_CONFINEMENT=yes
>> +DETACH=yes
>> MONITOR=yes
>> OVS_USER=
>> OVSDB_SERVER=yes
>> @@ -442,6 +443,10 @@ Less important options for "start", "restart" and 
>> "force-reload-kmod":
>>   --no-full-hostname set short hostname instead of full hostname
>>   --no-record-hostname   do not attempt to determine/record system
>>  hostname as part of start command
>> +  --detach=yes|noRun process in background (default: 
>> $DETACH).
>> + If "no", replace current process with 
>> "exec".
>> + Note, that with "no" database settings 
>> initialization is not done.
>> + db-version, system-ids are not set and 
>> transient ports are not deleted.
>> 
>> Debugging options for "start", "restart" and "force-reload-kmod":
>>   --ovsdb-server-wrapper=WRAPPER
>> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
>> index 7812a94ee..9db700e6a 100644
>> --- a/utilities/ovs-lib.in
>> +++ b/utilities/ovs-lib.in
>> @@ -183,7 +183,13 @@ start_daemon () {
>> # pidfile and monitoring
>> install_dir "$rundir"
>> set "$@" --pidfile="$rundir/$daemon.pid"
>> -set "$@" --detach
>> +
>> +if test X"$DETACH" != Xno; then
>> +set "$@" --detach
>> +else
>> +set exec "$@"
>> +fi
>> +
>> test X"$MONITOR" = Xno || set "$@" --monitor
>> 
>> # wrapper
> 


Regards,
Vladislav Odintsov

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

[ovs-dev] [PATCH v3] Add editorconfig file.

2023-06-13 Thread Robin Jarry 
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.

Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst and add exceptions
declared in build-aux/initial-tab-allowed-files. Only enforce rules for
*.c and *.h files. Other files should use the default indenting rules
from text editors.

In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.

Notes:

* All matching rules are considered. The last matching rule's properties
  will override the previous ones.
* The max_line_length property is only supported by a limited number of
  EditorConfig plugins. It will be ignored if unsupported.

Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link: 
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry 
Acked-by: Mike Pattrick 
Acked-by: Eelco Chaudron 
Cc: Ilya Maximets 
---

Notes:
v3: added exceptions for some *.{c,h} files that are using tabs

 .editorconfig | 43 +++
 Makefile.am   |  1 +
 2 files changed, 44 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..6d1842ca0f35
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,43 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
+max_line_length = 79
+
+[include/linux/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/sparse/rte_*.h]
+indent_style = tab
+tab_width = 8
+
+[include/windows/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/getopt_long.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/sflow*.{c,h}]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/strsep.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
diff --git a/Makefile.am b/Makefile.am
index df9c33dfe631..db341504d37f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
.cirrus.yml \
+   .editorconfig \
.github/workflows/build-and-test.yml \
appveyor.yml \
boot.sh \
-- 
2.40.1

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

Re: [ovs-dev] [PATCH 2/2] utilities: add --in-db-ssl option to ovs-ctl

2023-06-13 Thread Ilya Maximets 
On 6/7/23 08:33, Vladislav Odintsov wrote:
> It is possible to parametrize ovs-ctl script to start ovsdb-server with
> DB_SCHEME other than Open_vSwitch.  This scheme may not have currently
> required table "SSL" with "key", "cert" and "cacert" columns.

The db-schema option is primarily exists to override the path
to the Open_vSwitch schema file, in case of an unconventional
installation.  It's not for using a completely different schema.

Many other parts of the script rely on using ovs-vsctl against
a running database.  Also, the OVS-ctl name means that this
script controls OVS, it's not intended for and shouldn't be used
for anything else.

Best regards, Ilya Maximets.

> 
> This patch adds configuration knob "--in-db-ssl", which has default
> behavior as it is now: run ovsdb server with ssl options.
> 
> User must pass "--in-db-ssl=no" to run ovsdb-server without these
> arguments.
> 
> Signed-off-by: Vladislav Odintsov 
> ---
>  utilities/ovs-ctl.in | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index 72c8881e3..44a6496ef 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -150,9 +150,11 @@ do_start_ovsdb () {
>  fi
>  set "$@" -vconsole:emer -vsyslog:err -vfile:info
>  set "$@" --remote=punix:"$DB_SOCK"
> -set "$@" --private-key=db:Open_vSwitch,SSL,private_key
> -set "$@" --certificate=db:Open_vSwitch,SSL,certificate
> -set "$@" --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert
> +if test X"$IN_DB_SSL" = Xyes; then
> +set "$@" --private-key=db:Open_vSwitch,SSL,private_key
> +set "$@" --certificate=db:Open_vSwitch,SSL,certificate
> +set "$@" --bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert
> +fi
>  [ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
>  [ "$OVSDB_SERVER_OPTIONS" != "" ] && set "$@" $OVSDB_SERVER_OPTIONS
>  
> @@ -356,6 +358,7 @@ set_defaults () {
>  DB_SOCK=$rundir/db.sock
>  DB_SCHEMA=$datadir/vswitch.ovsschema
>  EXTRA_DBS=
> +IN_DB_SSL=yes
>  
>  PROTOCOL=gre
>  DPORT=
> @@ -457,6 +460,8 @@ File location options:
>--db-file=FILE database file name (default: $DB_FILE)
>--db-sock=SOCKET   JSON-RPC socket name (default: $DB_SOCK)
>--db-schema=FILE   database schema file name (default: $DB_SCHEMA)
> +  --in-db-ssl=yes|no use ssl key, cert and cacert file paths from 
> Open_vSwitch
> + database (default: $IN_DB_SSL)
>  
>  Options for "enable-protocol":
>--protocol=PROTOCOL  protocol to enable with iptables (default: gre)

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

Re: [ovs-dev] [PATCH 1/2] utilities: add "--detach" option to ovs-ctl

2023-06-13 Thread Ilya Maximets 
On 6/7/23 08:33, Vladislav Odintsov wrote:
> Default is yes (current behavior).  This means that after start process
> will be run in background.  When "no" is given, process is run in
> foreground with `exec` call, which replaces current process (ovs-ctl) with
> wanted ovs process (ovsdb-server or ovs-vswitchd).
> 
> This option is useful when running ovs-ctl inside docker container to make
> ovs* process to be a pid 1.
> 
> Note, that with `--detach=no` database settings initialization is not done.
> db-version, system-ids are not set and transient ports are not deleted.

Hi, Vladislav.

It seems like this option makes too many compromises and not really possible
to use with many of the other options.  The main use case for ovs-ctl from
the beginning was to automate managing of two processes (ovsdb-server and
ovs-vswitchd) at the same time.  Later the ability to manage only one of them
was added.  With this new option it will also be not possible to run both.

Why not just starting ovsdb/vswitchd processes explicitly in the container?

Best regards, Ilya Maximets.

> 
> Signed-off-by: Vladislav Odintsov 
> ---
>  utilities/ovs-ctl.in | 5 +
>  utilities/ovs-lib.in | 8 +++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index 0b2820c36..72c8881e3 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -338,6 +338,7 @@ set_defaults () {
>  DUMP_HUGEPAGES=no
>  MLOCKALL=yes
>  SELF_CONFINEMENT=yes
> +DETACH=yes
>  MONITOR=yes
>  OVS_USER=
>  OVSDB_SERVER=yes
> @@ -442,6 +443,10 @@ Less important options for "start", "restart" and 
> "force-reload-kmod":
>--no-full-hostname set short hostname instead of full hostname
>--no-record-hostname   do not attempt to determine/record system
>   hostname as part of start command
> +  --detach=yes|noRun process in background (default: 
> $DETACH).
> + If "no", replace current process with 
> "exec".
> + Note, that with "no" database settings 
> initialization is not done.
> + db-version, system-ids are not set and 
> transient ports are not deleted.
>  
>  Debugging options for "start", "restart" and "force-reload-kmod":
>--ovsdb-server-wrapper=WRAPPER
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 7812a94ee..9db700e6a 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -183,7 +183,13 @@ start_daemon () {
>  # pidfile and monitoring
>  install_dir "$rundir"
>  set "$@" --pidfile="$rundir/$daemon.pid"
> -set "$@" --detach
> +
> +if test X"$DETACH" != Xno; then
> +set "$@" --detach
> +else
> +set exec "$@"
> +fi
> +
>  test X"$MONITOR" = Xno || set "$@" --monitor
>  
>  # wrapper

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

Re: [ovs-dev] [PATCH v2] ovs-save: add bindir to PATH

2023-06-13 Thread Adrian Moreno 



On 6/12/23 16:59, Simon Horman wrote:

On Fri, Jun 17, 2022 at 05:03:46PM +0200, Adrian Moreno wrote:

If openvswitch is not installed in the default system's path ovs-save
script will fail to find the tools it requires.

Fix this by adding $bindir to the PATH.
Refactor common path calculation into ovs-lib.

Signed-off-by: Adrian Moreno 


Hi Adrian,

this patch seems to have gone stale.
Please consider reposting it if it is still relevant.



Thanks Simon for the archeology!

You are right. I completely forgot about it. I'll repost.

Cheers.
--
Adrián Moreno

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