Emerson,

Am 03.01.19 um 21:58 schrieb Emerson Gomes:
> However, the underflow scenario only seem to be possible if the peers are
> sending relative values, rather than absolute ones.

I don't believe so. My hypothetical timeline was created with absolute
values in mind.

> Apparently both cases (absolut and offset values) exist.
> I am looking at src/peers.c to understand how the peer protocol works and
> maybe create the patch you proposed (do not decrement counter if already 0).

I attached a patch which I believe fixes the issue (checking for 0 when
decrementing, not touching the peers).

> However it seems that a real fix would require some big changes on the
> protocol itself.

Yes I agree.

> One potencial implementation I could imagine, would be to, rather than
> broadcasting absolute values or offsets, each neighbor peer could report
> the amount of connection it has locally only, and it would be up to the
> local node to resolve the actual value by adding up the different values
> received from all neighbors.

Yes, that probably would be the most reliable implementation. It takes
up more memory and processing power, though.

> Not even sure if my understading is correct, but it's task currently out of
> my reach.
> Should I do a bug report somewhere? :)
> 

I suspect that the developers will notice this thread. A proper issue
tracker is a wish of mine as well
(https://www.mail-archive.com/haproxy@formilux.org/msg32239.html).

Best regards
Tim Düsterhus
From 10a271fb167c23d6db628f225bdc8be15d1a400a Mon Sep 17 00:00:00 2001
From: Tim Duesterhus <t...@bastelstu.be>
Date: Fri, 4 Jan 2019 00:11:59 +0100
Subject: [PATCH] BUG/MINOR: stick_table: Prevent conn_cur from underflowing

When using the peers feature a race condition could prevent
a connection from being properly counted. When this connection
exits it is being "uncounted" nonetheless, leading to a possible
underflow (-1) of the conn_curr stick table entry.
---
 include/proto/session.h | 3 ++-
 include/proto/stream.h  | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/proto/session.h b/include/proto/session.h
index d54e9454..0b7d08d9 100644
--- a/include/proto/session.h
+++ b/include/proto/session.h
@@ -62,7 +62,8 @@ static inline void session_store_counters(struct session *sess)
 		if (ptr) {
 			HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock);
 
-			stktable_data_cast(ptr, conn_cur)--;
+			if (stktable_data_cast(ptr, conn_cur) > 0)
+				stktable_data_cast(ptr, conn_cur)--;
 
 			HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
 
diff --git a/include/proto/stream.h b/include/proto/stream.h
index f4482097..a8c29921 100644
--- a/include/proto/stream.h
+++ b/include/proto/stream.h
@@ -103,7 +103,8 @@ static inline void stream_store_counters(struct stream *s)
 		if (ptr) {
 			HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock);
 
-			stktable_data_cast(ptr, conn_cur)--;
+			if (stktable_data_cast(ptr, conn_cur) > 0)
+				stktable_data_cast(ptr, conn_cur)--;
 
 			HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
 
@@ -141,7 +142,8 @@ static inline void stream_stop_content_counters(struct stream *s)
 		if (ptr) {
 			HA_RWLOCK_WRLOCK(STK_SESS_LOCK, &ts->lock);
 
-			stktable_data_cast(ptr, conn_cur)--;
+			if (stktable_data_cast(ptr, conn_cur) > 0)
+				stktable_data_cast(ptr, conn_cur)--;
 
 			HA_RWLOCK_WRUNLOCK(STK_SESS_LOCK, &ts->lock);
 
-- 
2.20.1

Reply via email to