diff mbox

gtp_duplicate(): comment, add todo.

Message ID 1444654956-29744-1-git-send-email-nhofmeyr@sysmocom.de
State Superseded
Headers show

Commit Message

Neels Hofmeyr Oct. 12, 2015, 1:02 p.m. UTC 
---
 gtp/gtp.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

Comments

Neels Hofmeyr Oct. 12, 2015, 1:12 p.m. UTC | #1 
On Mon, Oct 12, 2015 at 03:02:36PM +0200, Neels Hofmeyr wrote:
> --- a/gtp/gtp.c
> +++ b/gtp/gtp.c
> @@ -614,6 +614,15 @@ int gtp_notification(struct gsn_t *gsn, int version,
>  	return 0;
>  }
>  
> +/* Look for a message in the response queue for the given peer and sequence
> + * number seq. If found, (re-)send the response message and return 0. Otherwise
> + * return nonzero. This allows catching duplicate messages.
> + * A response message stays in the response queue until it times out.
> + * So, for any number of identical requests within that timeout period, the
> + * same packet can be resent.
> + * TODO this only checks the peer and sequence number, and does not verify that
> + * the GTP request is identical to the previous one for this peer and sequence
> + * nr: a mere peer or seq glitch can cause leaks of previous responses. */

Actually, the repeated transfer will go to the same qmsg_t.peer that the
response was originially addressed to. So fishing for other clients'
responses is not likely... I can drop the todo...

Any opinions on this?

~Neels
diff mbox

Patch

diff --git a/gtp/gtp.c b/gtp/gtp.c
index e6a610d..de6a7b4 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -614,6 +614,15 @@  int gtp_notification(struct gsn_t *gsn, int version,
 	return 0;
 }
 
+/* Look for a message in the response queue for the given peer and sequence
+ * number seq. If found, (re-)send the response message and return 0. Otherwise
+ * return nonzero. This allows catching duplicate messages.
+ * A response message stays in the response queue until it times out.
+ * So, for any number of identical requests within that timeout period, the
+ * same packet can be resent.
+ * TODO this only checks the peer and sequence number, and does not verify that
+ * the GTP request is identical to the previous one for this peer and sequence
+ * nr: a mere peer or seq glitch can cause leaks of previous responses. */
 int gtp_duplicate(struct gsn_t *gsn, int version,
 		  struct sockaddr_in *peer, uint16_t seq)
 {
@@ -873,7 +882,7 @@  int gtp_echo_ind(struct gsn_t *gsn, int version, struct sockaddr_in *peer,
 		 int fd, void *pack, unsigned len)
 {
 
-	/* Check if it was a duplicate request */
+	/* Repeat previous response in case of duplicate request */
 	if (!gtp_duplicate(gsn, 0, peer, get_seq(pack)))
 		return 0;
 
@@ -1258,7 +1267,7 @@  int gtp_create_pdp_ind(struct gsn_t *gsn, int version,
 	uint8_t linked_nsapi = 0;
 	struct pdp_t *linked_pdp = NULL;
 
-	/* TODO describe what this is all about: */
+	/* Repeat previous response in case of duplicate request */
 	if (!gtp_duplicate(gsn, version, peer, seq))
 		return 0;
 
@@ -1955,10 +1964,9 @@  int gtp_update_pdp_ind(struct gsn_t *gsn, int version,
 	uint64_t imsi;
 	uint8_t nsapi;
 
-	/* Is this a duplicate ? */
-	if (!gtp_duplicate(gsn, version, peer, seq)) {
-		return 0;	/* We already sent of response once */
-	}
+	/* Repeat previous response in case of duplicate request */
+	if (!gtp_duplicate(gsn, version, peer, seq))
+		return 0;
 
 	/* Decode information elements */
 	if (gtpie_decaps(ie, version, pack + hlen, len - hlen)) {
@@ -2432,10 +2440,9 @@  int gtp_delete_pdp_ind(struct gsn_t *gsn, int version,
 	int n;
 	int count = 0;
 
-	/* Is this a duplicate ? */
-	if (!gtp_duplicate(gsn, version, peer, seq)) {
-		return 0;	/* We already sent off response once */
-	}
+	/* Repeat previous response in case of duplicate request */
+	if (!gtp_duplicate(gsn, version, peer, seq))
+		return 0;
 
 	/* Find the linked context in question */
 	if (pdp_getgtp1(&linked_pdp, get_tei(pack))) {