From: Alan T. DeKok <aland@freeradius.org>
Date: Sun, 30 Jul 2017 14:58:29 +0000 (-0400)
Subject: add more notes on TODO's
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a2d5b264eef6d441e355fffc5d3e2499b5bbbf4;p=thirdparty%2Ffreeradius-server.git

add more notes on TODO's
---

diff --git a/src/modules/rlm_radius/TODO.md b/src/modules/rlm_radius/TODO.md
index fa39a919284..35783daf969 100644
--- a/src/modules/rlm_radius/TODO.md
+++ b/src/modules/rlm_radius/TODO.md
@@ -1,5 +1,11 @@
 # rlm_radius
 
+## RADIUS fixups
+
+It has to add Proxy-State to outbound packets (oops)
+
+And do CHAP-Challenge fixups
+
 ## Multiple connections
 
 rlm_radius_udp.c now has one connection, in `c->active`, which is an
@@ -56,6 +62,12 @@ We need some more configuration options:
 	connect_timeout
 	reconnect_delay
 	idle_timeout
+
+	# as per 3.0
+	response_window
+	response_timeouts
+	zombie_period
+	revive_interval
     }
     
     # return RLM_MODULE_USERLOCK if we're sitting on too many packets
@@ -80,12 +92,6 @@ We need some more configuration options:
 		User-Name = ...
 		User-Password = ...
 	}
-	
-	# as per 3.0
-	response_window
-	response_timeouts
-	zombie_period
-	revive_interval
     }
 
 The `rlm_radius` module should not have an idea as to the status of
@@ -123,3 +129,26 @@ list of allowed packet types.  We then need to require config for
 username / password, for Access-Request, and just username for
 Accounting-Request.
 
+## synchronous proxying
+
+ala v3.  All retransmissions started by the client.
+
+This requires a "signal" handler to be added when the module calls unlang_yield.
+
+The call to the signal handler is already in proto_radius_auth and friends.
+
+We could probably add a signal handler to the module, to handle the
+DONE signal.  This would allow graceful cleanups.  Those are mostly
+already handled via the talloc_free() hierarchy and destructors. But
+it may be nice to distinguish the situations.  And, it lets us test
+the signal handler independent of anything else.
+
+Doing synchronous proxying also mean having the network side return
+DUP PACKET (somehow).  And, send that dup packet signal to the worker.
+Which somehow associates it with a request (probably via a simple
+network thread + packet identifier).  This means that the worker has
+to have yet another tree tracking packets... but it will allow for
+signaling if necessary.
+
+We probaby want the network + worker to be able to send IDs of 0/0,
+which means "no tracking", as that will likely be the common case.
diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c
index 22d90b812f3..30fed9dbfab 100644
--- a/src/modules/rlm_radius/rlm_radius_udp.c
+++ b/src/modules/rlm_radius/rlm_radius_udp.c
@@ -359,6 +359,11 @@ static void conn_writable(fr_event_list_t *el, int fd, UNUSED int flags, void *u
 			return;
 		}
 
+		/*
+		 *	@todo - print out the packet we're proxying,
+		 *	including socket name.
+		 */
+
 		MEM(u->packet = talloc_memdup(u, c->buffer, packet_len));
 		u->packet_len = packet_len;
 
@@ -758,6 +763,25 @@ static int mod_push(void *instance, REQUEST *request, rlm_radius_link_t *link, v
 	 *
 	 *	conn_writable() will set c->pending, and call
 	 *	fd_active() as necessary.
+	 *
+	 *	@todo - if there's an error, and we call
+	 *	mod_finished_request(), it will call
+	 *	unlang_resumable().  This marks it as resumable BEFORE
+	 *	rlm_radius calls unlang_yield.  Oops...
+	 *
+	 *	We need to update the push() API to return
+	 *	-1 error
+	 *	0  should yield
+	 *	1  written immediately
+	 *
+	 *	and add an rlm_rcode_t* pointer, so that we can return
+	 *	it here.
+	 *
+	 *	This also means splitting conn_writable() into two
+	 *	parts.  One, a loop around the queues.  And two, a
+	 *	function that does the actual write.  We can then call
+	 *	the write function from here, and have it return an
+	 *	OK/yield return code.
 	 */
 	if (!c->pending) {
 		conn_writable(t->el, c->fd, 0, c);