DCCP protocol discussions
 help / color / mirror / Atom feed
From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
To: dccp@vger.kernel.org
Subject: [Patch 9/12]: Perform history operations only after packet has been sent
Date: Thu, 07 Dec 2006 21:13:21 +0000	[thread overview]
Message-ID: <200612072113.21700@strip-the-willow> (raw)

[CCID3]: Perform history operations only after packet has been sent

 This migrates all packet history operations into the routine
 ccid3_hc_tx_packet_sent, thereby removing synchronization problems
 that occur when, as before, the operations are spread over multiple
 routines.
 The following minor simplifications are also applied:
  * several simplifications now follow from this change - several tests 
    are now no longer required
  * removal of one unnecessary variable (dp)

Justification:
--------------
 Currently packet history operations span two different routines, 
 one of which is likely to pass through several iterations of sleeping
 and awakening. 
 The first routine, ccid3_hc_tx_send_packet, allocates an entry and
 sets a few fields. The remaining fields are filled in when the second
 routine (which is not within a sleeping context), ccid3_hc_tx_packet_sent,
 is called. This has several strong drawbacks:
  * it is not necessary to split history operations - all fields can be
    filled in by the second routine
  * the first routine is called multiple times, until a packet can be sent,
    and sleeps meanwhile - this causes a lot of difficulties with regard to
    keeping the list consistent
  * since both routines do not have a producer-consumer like synchronization,
    it is very difficult to maintain data across calls to these routines
  * the fact that the routines are called in different contexts (sleeping, not
    sleeping) adds further problems		  

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/ccids/ccid3.c |   38 +++++++++-----------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -296,7 +296,6 @@ static int ccid3_hc_tx_send_packet(struc
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 	struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);
-	struct dccp_tx_hist_entry *new_packet;
 	struct timeval now;
 	suseconds_t delay;
 
@@ -310,21 +309,6 @@ static int ccid3_hc_tx_send_packet(struc
 	if (unlikely(skb->len = 0))
 		return -EBADMSG;
 
-	/* See if last packet allocated was not sent */
-	new_packet = dccp_tx_hist_head(&hctx->ccid3hctx_hist);
-	if (new_packet = NULL || new_packet->dccphtx_sent) {
-		new_packet = dccp_tx_hist_entry_new(ccid3_tx_hist,
-						    SLAB_ATOMIC);
-
-		if (unlikely(new_packet = NULL)) {
-			DCCP_WARN("%s, sk=%p, not enough mem to add to history,"
-				  "send refused\n", dccp_role(sk), sk);
-			return -ENOBUFS;
-		}
-
-		dccp_tx_hist_add_entry(&hctx->ccid3hctx_hist, new_packet);
-	}
-
 	dccp_timestamp(sk, &now);
 
 	switch (hctx->ccid3hctx_state) {
@@ -380,31 +364,27 @@ static int ccid3_hc_tx_send_packet(struc
 
 static void ccid3_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len)
 {
-	const struct dccp_sock *dp = dccp_sk(sk);
 	struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);
 	struct timeval now;
 	struct dccp_tx_hist_entry *packet;
 
 	BUG_ON(hctx = NULL);
 
-	dccp_timestamp(sk, &now);
-
 	ccid3_hc_tx_update_s(hctx, len);
 
-	packet = dccp_tx_hist_head(&hctx->ccid3hctx_hist);
+	packet = dccp_tx_hist_entry_new(ccid3_tx_hist, SLAB_ATOMIC);
 	if (unlikely(packet = NULL)) {
-		DCCP_WARN("packet doesn't exist in history!\n");
-		return;
-	}
-	if (unlikely(packet->dccphtx_sent)) {
-		DCCP_WARN("no unsent packet in history!\n");
+		DCCP_CRIT("packet history - out of memory!");
 		return;
 	}
+	dccp_tx_hist_add_entry(&hctx->ccid3hctx_hist, packet);
+
+	dccp_timestamp(sk, &now);
 	packet->dccphtx_tstamp = now;
-	packet->dccphtx_seqno  = dp->dccps_gss;
-	hctx->ccid3hctx_idle = 0;
-	packet->dccphtx_rtt  = hctx->ccid3hctx_rtt;
-	packet->dccphtx_sent = 1;
+	packet->dccphtx_seqno  = dccp_sk(sk)->dccps_gss;
+	packet->dccphtx_rtt    = hctx->ccid3hctx_rtt;
+	packet->dccphtx_sent   = 1;
+	hctx->ccid3hctx_idle   = 0;
 }
 
 static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)

             reply	other threads:[~2006-12-07 21:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-07 21:13 Gerrit Renker [this message]
2006-12-07 22:41 ` [Patch 9/12]: Perform history operations only after packet has been sent Ian McDonald

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200612072113.21700@strip-the-willow \
    --to=gerrit@erg.abdn.ac.uk \
    --cc=dccp@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox