Re: [Openvpn-devel] [PATCH v4] dco-freebsd: use m->instances[] instead of m->hash

2023-03-23 Thread Arne Schwabe 

Am 23.03.23 um 09:03 schrieb Gert Doering:

From: Antonio Quartulli 

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

v4: use "peerid" everywhere as that's what FreeBSD does, change message text

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
  src/openvpn/dco_freebsd.c | 22 ++
  1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..a334d5d2 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,17 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
  static void
  dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
  {
-struct hash_element *he;
-struct hash_iterator hi;
  
-hash_iterator_init(m->hash, );

-
-while ((he = hash_iterator_next()))
+if (peerid >= m->max_clients || !m->instances[peerid])
  {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_WARN, "dco_update_peer_stat: invalid peer ID %d returned by 
kernel", peerid);
  return;
  }
  
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);

+struct multi_instance *mi = m->instances[peerid];
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
  }
  
  int


Acked-By: Arne Schwabe 


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4] dco-freebsd: use m->instances[] instead of m->hash

2023-03-23 Thread Antonio Quartulli 

Hi,

On 23/03/2023 09:03, Gert Doering wrote:

From: Antonio Quartulli 

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

v4: use "peerid" everywhere as that's what FreeBSD does, change message text

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
  src/openvpn/dco_freebsd.c | 22 ++
  1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..a334d5d2 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,17 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
  static void
  dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
  {
-struct hash_element *he;
-struct hash_iterator hi;
  
-hash_iterator_init(m->hash, );

-
-while ((he = hash_iterator_next()))
+if (peerid >= m->max_clients || !m->instances[peerid])
  {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_WARN, "dco_update_peer_stat: invalid peer ID %d returned by 
kernel", peerid);


I'd prefer to use __func__ here, but it is noted that dco_freebsd.c 
seldomly uses this approach.

For this reason I think hardcoding the function name is fine for now.

Later on we can do a full cleanup.


  return;
  }
  
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);

+struct multi_instance *mi = m->instances[peerid];
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
  }
  
  int


This patch is basically my v3 with the peerid fixed and the message changed.

I can't ACK since this patch bears my own signature, but I am fine with 
the applied modification.


Cheers,


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v4] dco-freebsd: use m->instances[] instead of m->hash

2023-03-23 Thread Gert Doering 
From: Antonio Quartulli 

When retrieving the multi_instance of a specific peer,
there is no need to peform a linear search across the
whole m->hash list. We can directly access the needed
object via m->instances[peer-id] in constant time (and
just one line of code).

Adapt the dco-freebsd code to do so.

v4: use "peerid" everywhere as that's what FreeBSD does, change message text

Cc: Kristof Provost 
Change-Id: I8d8af6f872146604a9710edf443db65df48ac3cb
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/dco_freebsd.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 225b3cf8..a334d5d2 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -674,27 +674,17 @@ dco_event_set(dco_context_t *dco, struct event_set *es, 
void *arg)
 static void
 dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t 
*nvl)
 {
-struct hash_element *he;
-struct hash_iterator hi;
 
-hash_iterator_init(m->hash, );
-
-while ((he = hash_iterator_next()))
+if (peerid >= m->max_clients || !m->instances[peerid])
 {
-struct multi_instance *mi = (struct multi_instance *) he->value;
-
-if (mi->context.c2.tls_multi->peer_id != peerid)
-{
-continue;
-}
-
-mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
-mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
-
+msg(M_WARN, "dco_update_peer_stat: invalid peer ID %d returned by 
kernel", peerid);
 return;
 }
 
-msg(M_INFO, "Peer %d returned by kernel, but not found locally", peerid);
+struct multi_instance *mi = m->instances[peerid];
+
+mi->context.c2.dco_read_bytes = nvlist_get_number(nvl, "in");
+mi->context.c2.dco_write_bytes = nvlist_get_number(nvl, "out");
 }
 
 int
-- 
2.39.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel