Netdev List
 help / color / mirror / Atom feed
* [PATCH 5/9] [TCP]: Dropped unnecessary skb/sacked accessing in reneging
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980772309-git-send-email-ilpo.jarvinen@helsinki.fi>

SACK reneging can be precalculated to a FLAG in clean_rtx_queue
which has the right skb looked up. This will help a bit in
future because skb->sacked access will be changed eventually,
changing it already won't hurt any.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 366f63a..7bac1fa 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -105,6 +105,7 @@ int sysctl_tcp_abc __read_mostly;
 #define FLAG_SND_UNA_ADVANCED	0x400 /* Snd_una was changed (!= FLAG_DATA_ACKED) */
 #define FLAG_DSACKING_ACK	0x800 /* SACK blocks contained D-SACK info */
 #define FLAG_NONHEAD_RETRANS_ACKED	0x1000 /* Non-head rexmitted data was ACKed */
+#define FLAG_SACK_RENEGING	0x2000 /* snd_una advanced to a sacked seq */
 
 #define FLAG_ACKED		(FLAG_DATA_ACKED|FLAG_SYN_ACKED)
 #define FLAG_NOT_DUP		(FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
@@ -1918,18 +1919,15 @@ void tcp_enter_loss(struct sock *sk, int how)
 	tp->frto_counter = 0;
 }
 
-static int tcp_check_sack_reneging(struct sock *sk)
+/* If ACK arrived pointing to a remembered SACK, it means that our
+ * remembered SACKs do not reflect real state of receiver i.e.
+ * receiver _host_ is heavily congested (or buggy).
+ *
+ * Do processing similar to RTO timeout.
+ */
+static int tcp_check_sack_reneging(struct sock *sk, int flag)
 {
-	struct sk_buff *skb;
-
-	/* If ACK arrived pointing to a remembered SACK,
-	 * it means that our remembered SACKs do not reflect
-	 * real state of receiver i.e.
-	 * receiver _host_ is heavily congested (or buggy).
-	 * Do processing similar to RTO timeout.
-	 */
-	if ((skb = tcp_write_queue_head(sk)) != NULL &&
-	    (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)) {
+	if (flag & FLAG_SACK_RENEGING) {
 		struct inet_connection_sock *icsk = inet_csk(sk);
 		NET_INC_STATS_BH(LINUX_MIB_TCPSACKRENEGING);
 
@@ -2515,7 +2513,7 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
 		tp->prior_ssthresh = 0;
 
 	/* B. In all the states check for reneging SACKs. */
-	if (tp->sacked_out && tcp_check_sack_reneging(sk))
+	if (tcp_check_sack_reneging(sk, flag))
 		return;
 
 	/* C. Process data loss notification, provided it is valid. */
@@ -2852,6 +2850,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets)
 		tcp_clear_all_retrans_hints(tp);
 	}
 
+	if (skb && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED))
+		flag |= FLAG_SACK_RENEGING;
+
 	if (flag & FLAG_ACKED) {
 		const struct tcp_congestion_ops *ca_ops
 			= inet_csk(sk)->icsk_ca_ops;
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH 6/9] [TCP]: Remove TCPCB_URG & TCPCB_AT_TAIL as unnecessary
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980773732-git-send-email-ilpo.jarvinen@helsinki.fi>

The snd_up check should be enough. I suspect this has been
there to provide a minor optimization in clean_rtx_queue which
used to have a small if (!->sacked) block which could skip
snd_up check among the other work.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 include/net/tcp.h     |    4 ----
 net/ipv4/tcp.c        |    1 -
 net/ipv4/tcp_input.c  |    3 +--
 net/ipv4/tcp_output.c |    7 +++----
 4 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6a732d4..48081ad 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -578,10 +578,6 @@ struct tcp_skb_cb {
 #define TCPCB_EVER_RETRANS	0x80	/* Ever retransmitted frame	*/
 #define TCPCB_RETRANS		(TCPCB_SACKED_RETRANS|TCPCB_EVER_RETRANS)
 
-#define TCPCB_URG		0x20	/* Urgent pointer advanced here	*/
-
-#define TCPCB_AT_TAIL		(TCPCB_URG)
-
 	__u16		urg_ptr;	/* Valid w/URG flags is set.	*/
 	__u32		ack_seq;	/* Sequence number ACK'd	*/
 };
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2cbfa6d..34085e3 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -497,7 +497,6 @@ static inline void tcp_mark_urg(struct tcp_sock *tp, int flags,
 	if (flags & MSG_OOB) {
 		tp->urg_mode = 1;
 		tp->snd_up = tp->write_seq;
-		TCP_SKB_CB(skb)->sacked |= TCPCB_URG;
 	}
 }
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7bac1fa..1e7fd81 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2821,8 +2821,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets)
 		if (sacked & TCPCB_LOST)
 			tp->lost_out -= acked_pcount;
 
-		if (unlikely((sacked & TCPCB_URG) && tp->urg_mode &&
-			     !before(end_seq, tp->snd_up)))
+		if (unlikely(tp->urg_mode && !before(end_seq, tp->snd_up)))
 			tp->urg_mode = 0;
 
 		tp->packets_out -= acked_pcount;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 202d60c..182ae21 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -711,7 +711,6 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
 	TCP_SKB_CB(skb)->flags = flags & ~(TCPCB_FLAG_FIN|TCPCB_FLAG_PSH);
 	TCP_SKB_CB(buff)->flags = flags;
 	TCP_SKB_CB(buff)->sacked = TCP_SKB_CB(skb)->sacked;
-	TCP_SKB_CB(skb)->sacked &= ~TCPCB_AT_TAIL;
 
 	if (!skb_shinfo(skb)->nr_frags && skb->ip_summed != CHECKSUM_PARTIAL) {
 		/* Copy and checksum data tail into the new buffer. */
@@ -1721,7 +1720,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
 		/* All done, get rid of second SKB and account for it so
 		 * packet counting does not break.
 		 */
-		TCP_SKB_CB(skb)->sacked |= TCP_SKB_CB(next_skb)->sacked&(TCPCB_EVER_RETRANS|TCPCB_AT_TAIL);
+		TCP_SKB_CB(skb)->sacked |= TCP_SKB_CB(next_skb)->sacked & TCPCB_EVER_RETRANS;
 		if (TCP_SKB_CB(next_skb)->sacked&TCPCB_SACKED_RETRANS)
 			tp->retrans_out -= tcp_skb_pcount(next_skb);
 		if (TCP_SKB_CB(next_skb)->sacked&TCPCB_LOST)
@@ -2470,7 +2469,7 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent)
 	skb_reserve(skb, MAX_TCP_HEADER);
 	skb->csum = 0;
 	TCP_SKB_CB(skb)->flags = TCPCB_FLAG_ACK;
-	TCP_SKB_CB(skb)->sacked = urgent;
+	TCP_SKB_CB(skb)->sacked = 0;
 	skb_shinfo(skb)->gso_segs = 1;
 	skb_shinfo(skb)->gso_size = 0;
 	skb_shinfo(skb)->gso_type = 0;
@@ -2522,7 +2521,7 @@ int tcp_write_wakeup(struct sock *sk)
 		} else {
 			if (tp->urg_mode &&
 			    between(tp->snd_up, tp->snd_una+1, tp->snd_una+0xFFFF))
-				tcp_xmit_probe_skb(sk, TCPCB_URG);
+				tcp_xmit_probe_skb(sk, 1);
 			return tcp_xmit_probe_skb(sk, 0);
 		}
 	}
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH 4/9] [TCP]: Introduce tcp_wnd_end() to reduce line lengths
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980771495-git-send-email-ilpo.jarvinen@helsinki.fi>

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 include/net/tcp.h     |    6 ++++++
 net/ipv4/tcp_input.c  |    3 +--
 net/ipv4/tcp_output.c |   20 ++++++++++----------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 76286e8..6a732d4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -785,6 +785,12 @@ static __inline__ __u32 tcp_max_burst(const struct tcp_sock *tp)
 	return 3;
 }
 
+/* Returns end sequence number of the receiver's advertised window */
+static inline u32 tcp_wnd_end(const struct tcp_sock *tp)
+{
+	return tp->snd_una + tp->snd_wnd;
+}
+
 /* RFC2861 Check whether we are limited by application or congestion window
  * This is the inverse of cwnd check in tcp_tso_should_defer
  */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 41f4b86..366f63a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2921,8 +2921,7 @@ static void tcp_ack_probe(struct sock *sk)
 
 	/* Was it a usable window open? */
 
-	if (!after(TCP_SKB_CB(tcp_send_head(sk))->end_seq,
-		   tp->snd_una + tp->snd_wnd)) {
+	if (!after(TCP_SKB_CB(tcp_send_head(sk))->end_seq, tcp_wnd_end(tp))) {
 		icsk->icsk_backoff = 0;
 		inet_csk_clear_xmit_timer(sk, ICSK_TIME_PROBE0);
 		/* Socket must be waked up by subsequent tcp_data_snd_check().
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 025dddf..202d60c 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -89,10 +89,10 @@ static inline __u32 tcp_acceptable_seq(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	if (!before(tp->snd_una+tp->snd_wnd, tp->snd_nxt))
+	if (!before(tcp_wnd_end(tp), tp->snd_nxt))
 		return tp->snd_nxt;
 	else
-		return tp->snd_una+tp->snd_wnd;
+		return tcp_wnd_end(tp);
 }
 
 /* Calculate mss to advertise in SYN segment.
@@ -1023,7 +1023,7 @@ static unsigned int tcp_mss_split_point(struct sock *sk, struct sk_buff *skb,
 	struct tcp_sock *tp = tcp_sk(sk);
 	u32 needed, window, cwnd_len;
 
-	window = (tp->snd_una + tp->snd_wnd - TCP_SKB_CB(skb)->seq);
+	window = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
 	cwnd_len = mss_now * cwnd;
 
 	if (likely(cwnd_len <= window && skb != tcp_write_queue_tail(sk)))
@@ -1129,7 +1129,7 @@ static inline int tcp_snd_wnd_test(struct tcp_sock *tp, struct sk_buff *skb, uns
 	if (skb->len > cur_mss)
 		end_seq = TCP_SKB_CB(skb)->seq + cur_mss;
 
-	return !after(end_seq, tp->snd_una + tp->snd_wnd);
+	return !after(end_seq, tcp_wnd_end(tp));
 }
 
 /* This checks if the data bearing packet SKB (usually tcp_send_head(sk))
@@ -1246,7 +1246,7 @@ static int tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb)
 	BUG_ON(tcp_skb_pcount(skb) <= 1 ||
 	       (tp->snd_cwnd <= in_flight));
 
-	send_win = (tp->snd_una + tp->snd_wnd) - TCP_SKB_CB(skb)->seq;
+	send_win = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
 
 	/* From in_flight test above, we know that cwnd > in_flight.  */
 	cong_win = (tp->snd_cwnd - in_flight) * tp->mss_cache;
@@ -1327,7 +1327,7 @@ static int tcp_mtu_probe(struct sock *sk)
 
 	if (tp->snd_wnd < size_needed)
 		return -1;
-	if (after(tp->snd_nxt + size_needed, tp->snd_una + tp->snd_wnd))
+	if (after(tp->snd_nxt + size_needed, tcp_wnd_end(tp)))
 		return 0;
 
 	/* Do we need to wait to drain cwnd? With none in flight, don't stall */
@@ -1682,7 +1682,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
 			return;
 
 		/* Next skb is out of window. */
-		if (after(TCP_SKB_CB(next_skb)->end_seq, tp->snd_una+tp->snd_wnd))
+		if (after(TCP_SKB_CB(next_skb)->end_seq, tcp_wnd_end(tp)))
 			return;
 
 		/* Punt if not enough space exists in the first SKB for
@@ -1826,7 +1826,7 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
 	 * case, when window is shrunk to zero. In this case
 	 * our retransmit serves as a zero window probe.
 	 */
-	if (!before(TCP_SKB_CB(skb)->seq, tp->snd_una+tp->snd_wnd)
+	if (!before(TCP_SKB_CB(skb)->seq, tcp_wnd_end(tp))
 	    && TCP_SKB_CB(skb)->seq != tp->snd_una)
 		return -EAGAIN;
 
@@ -2492,10 +2492,10 @@ int tcp_write_wakeup(struct sock *sk)
 		struct sk_buff *skb;
 
 		if ((skb = tcp_send_head(sk)) != NULL &&
-		    before(TCP_SKB_CB(skb)->seq, tp->snd_una+tp->snd_wnd)) {
+		    before(TCP_SKB_CB(skb)->seq, tcp_wnd_end(tp))) {
 			int err;
 			unsigned int mss = tcp_current_mss(sk, 0);
-			unsigned int seg_size = tp->snd_una+tp->snd_wnd-TCP_SKB_CB(skb)->seq;
+			unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
 
 			if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
 				tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH 7/9] [TCP]: reduce tcp_output's indentation levels a bit
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980772445-git-send-email-ilpo.jarvinen@helsinki.fi>

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |  239 +++++++++++++++++++++++++------------------------
 1 files changed, 121 insertions(+), 118 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 182ae21..199253c 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1668,75 +1668,77 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct sk_buff *next_skb = tcp_write_queue_next(sk, skb);
+	int skb_size, next_skb_size;
+	u16 flags;
 
 	/* The first test we must make is that neither of these two
 	 * SKB's are still referenced by someone else.
 	 */
-	if (!skb_cloned(skb) && !skb_cloned(next_skb)) {
-		int skb_size = skb->len, next_skb_size = next_skb->len;
-		u16 flags = TCP_SKB_CB(skb)->flags;
+	if (skb_cloned(skb) || skb_cloned(next_skb))
+		return;
 
-		/* Also punt if next skb has been SACK'd. */
-		if (TCP_SKB_CB(next_skb)->sacked & TCPCB_SACKED_ACKED)
-			return;
+	skb_size = skb->len;
+	next_skb_size = next_skb->len;
+	flags = TCP_SKB_CB(skb)->flags;
 
-		/* Next skb is out of window. */
-		if (after(TCP_SKB_CB(next_skb)->end_seq, tcp_wnd_end(tp)))
-			return;
+	/* Also punt if next skb has been SACK'd. */
+	if (TCP_SKB_CB(next_skb)->sacked & TCPCB_SACKED_ACKED)
+		return;
 
-		/* Punt if not enough space exists in the first SKB for
-		 * the data in the second, or the total combined payload
-		 * would exceed the MSS.
-		 */
-		if ((next_skb_size > skb_tailroom(skb)) ||
-		    ((skb_size + next_skb_size) > mss_now))
-			return;
+	/* Next skb is out of window. */
+	if (after(TCP_SKB_CB(next_skb)->end_seq, tcp_wnd_end(tp)))
+		return;
 
-		BUG_ON(tcp_skb_pcount(skb) != 1 ||
-		       tcp_skb_pcount(next_skb) != 1);
+	/* Punt if not enough space exists in the first SKB for
+	 * the data in the second, or the total combined payload
+	 * would exceed the MSS.
+	 */
+	if ((next_skb_size > skb_tailroom(skb)) ||
+	    ((skb_size + next_skb_size) > mss_now))
+		return;
 
-		tcp_highest_sack_combine(sk, next_skb, skb);
+	BUG_ON(tcp_skb_pcount(skb) != 1 || tcp_skb_pcount(next_skb) != 1);
 
-		/* Ok.	We will be able to collapse the packet. */
-		tcp_unlink_write_queue(next_skb, sk);
+	tcp_highest_sack_combine(sk, next_skb, skb);
 
-		skb_copy_from_linear_data(next_skb,
-					  skb_put(skb, next_skb_size),
-					  next_skb_size);
+	/* Ok.	We will be able to collapse the packet. */
+	tcp_unlink_write_queue(next_skb, sk);
 
-		if (next_skb->ip_summed == CHECKSUM_PARTIAL)
-			skb->ip_summed = CHECKSUM_PARTIAL;
+	skb_copy_from_linear_data(next_skb, skb_put(skb, next_skb_size),
+				  next_skb_size);
 
-		if (skb->ip_summed != CHECKSUM_PARTIAL)
-			skb->csum = csum_block_add(skb->csum, next_skb->csum, skb_size);
+	if (next_skb->ip_summed == CHECKSUM_PARTIAL)
+		skb->ip_summed = CHECKSUM_PARTIAL;
 
-		/* Update sequence range on original skb. */
-		TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(next_skb)->end_seq;
+	if (skb->ip_summed != CHECKSUM_PARTIAL)
+		skb->csum = csum_block_add(skb->csum, next_skb->csum, skb_size);
 
-		/* Merge over control information. */
-		flags |= TCP_SKB_CB(next_skb)->flags; /* This moves PSH/FIN etc. over */
-		TCP_SKB_CB(skb)->flags = flags;
+	/* Update sequence range on original skb. */
+	TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(next_skb)->end_seq;
 
-		/* All done, get rid of second SKB and account for it so
-		 * packet counting does not break.
-		 */
-		TCP_SKB_CB(skb)->sacked |= TCP_SKB_CB(next_skb)->sacked & TCPCB_EVER_RETRANS;
-		if (TCP_SKB_CB(next_skb)->sacked&TCPCB_SACKED_RETRANS)
-			tp->retrans_out -= tcp_skb_pcount(next_skb);
-		if (TCP_SKB_CB(next_skb)->sacked&TCPCB_LOST)
-			tp->lost_out -= tcp_skb_pcount(next_skb);
-		/* Reno case is special. Sigh... */
-		if (tcp_is_reno(tp) && tp->sacked_out)
-			tcp_dec_pcount_approx(&tp->sacked_out, next_skb);
-
-		tcp_adjust_fackets_out(sk, next_skb, tcp_skb_pcount(next_skb));
-		tp->packets_out -= tcp_skb_pcount(next_skb);
-
-		/* changed transmit queue under us so clear hints */
-		tcp_clear_retrans_hints_partial(tp);
-
-		sk_wmem_free_skb(sk, next_skb);
-	}
+	/* Merge over control information. */
+	flags |= TCP_SKB_CB(next_skb)->flags; /* This moves PSH/FIN etc. over */
+	TCP_SKB_CB(skb)->flags = flags;
+
+	/* All done, get rid of second SKB and account for it so
+	 * packet counting does not break.
+	 */
+	TCP_SKB_CB(skb)->sacked |= TCP_SKB_CB(next_skb)->sacked & TCPCB_EVER_RETRANS;
+	if (TCP_SKB_CB(next_skb)->sacked & TCPCB_SACKED_RETRANS)
+		tp->retrans_out -= tcp_skb_pcount(next_skb);
+	if (TCP_SKB_CB(next_skb)->sacked & TCPCB_LOST)
+		tp->lost_out -= tcp_skb_pcount(next_skb);
+	/* Reno case is special. Sigh... */
+	if (tcp_is_reno(tp) && tp->sacked_out)
+		tcp_dec_pcount_approx(&tp->sacked_out, next_skb);
+
+	tcp_adjust_fackets_out(sk, next_skb, tcp_skb_pcount(next_skb));
+	tp->packets_out -= tcp_skb_pcount(next_skb);
+
+	/* changed transmit queue under us so clear hints */
+	tcp_clear_retrans_hints_partial(tp);
+
+	sk_wmem_free_skb(sk, next_skb);
 }
 
 /* Do a simple retransmit without using the backoff mechanisms in
@@ -2411,37 +2413,38 @@ void tcp_send_delayed_ack(struct sock *sk)
 /* This routine sends an ack and also updates the window. */
 void tcp_send_ack(struct sock *sk)
 {
-	/* If we have been reset, we may not send again. */
-	if (sk->sk_state != TCP_CLOSE) {
-		struct sk_buff *buff;
+	struct sk_buff *buff;
 
-		/* We are not putting this on the write queue, so
-		 * tcp_transmit_skb() will set the ownership to this
-		 * sock.
-		 */
-		buff = alloc_skb(MAX_TCP_HEADER, GFP_ATOMIC);
-		if (buff == NULL) {
-			inet_csk_schedule_ack(sk);
-			inet_csk(sk)->icsk_ack.ato = TCP_ATO_MIN;
-			inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
-						  TCP_DELACK_MAX, TCP_RTO_MAX);
-			return;
-		}
+	/* If we have been reset, we may not send again. */
+	if (sk->sk_state == TCP_CLOSE)
+		return;
 
-		/* Reserve space for headers and prepare control bits. */
-		skb_reserve(buff, MAX_TCP_HEADER);
-		buff->csum = 0;
-		TCP_SKB_CB(buff)->flags = TCPCB_FLAG_ACK;
-		TCP_SKB_CB(buff)->sacked = 0;
-		skb_shinfo(buff)->gso_segs = 1;
-		skb_shinfo(buff)->gso_size = 0;
-		skb_shinfo(buff)->gso_type = 0;
-
-		/* Send it off, this clears delayed acks for us. */
-		TCP_SKB_CB(buff)->seq = TCP_SKB_CB(buff)->end_seq = tcp_acceptable_seq(sk);
-		TCP_SKB_CB(buff)->when = tcp_time_stamp;
-		tcp_transmit_skb(sk, buff, 0, GFP_ATOMIC);
+	/* We are not putting this on the write queue, so
+	 * tcp_transmit_skb() will set the ownership to this
+	 * sock.
+	 */
+	buff = alloc_skb(MAX_TCP_HEADER, GFP_ATOMIC);
+	if (buff == NULL) {
+		inet_csk_schedule_ack(sk);
+		inet_csk(sk)->icsk_ack.ato = TCP_ATO_MIN;
+		inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
+					  TCP_DELACK_MAX, TCP_RTO_MAX);
+		return;
 	}
+
+	/* Reserve space for headers and prepare control bits. */
+	skb_reserve(buff, MAX_TCP_HEADER);
+	buff->csum = 0;
+	TCP_SKB_CB(buff)->flags = TCPCB_FLAG_ACK;
+	TCP_SKB_CB(buff)->sacked = 0;
+	skb_shinfo(buff)->gso_segs = 1;
+	skb_shinfo(buff)->gso_size = 0;
+	skb_shinfo(buff)->gso_type = 0;
+
+	/* Send it off, this clears delayed acks for us. */
+	TCP_SKB_CB(buff)->seq = TCP_SKB_CB(buff)->end_seq = tcp_acceptable_seq(sk);
+	TCP_SKB_CB(buff)->when = tcp_time_stamp;
+	tcp_transmit_skb(sk, buff, 0, GFP_ATOMIC);
 }
 
 /* This routine sends a packet with an out of date sequence
@@ -2486,46 +2489,46 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent)
 
 int tcp_write_wakeup(struct sock *sk)
 {
-	if (sk->sk_state != TCP_CLOSE) {
-		struct tcp_sock *tp = tcp_sk(sk);
-		struct sk_buff *skb;
-
-		if ((skb = tcp_send_head(sk)) != NULL &&
-		    before(TCP_SKB_CB(skb)->seq, tcp_wnd_end(tp))) {
-			int err;
-			unsigned int mss = tcp_current_mss(sk, 0);
-			unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
-
-			if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
-				tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
-
-			/* We are probing the opening of a window
-			 * but the window size is != 0
-			 * must have been a result SWS avoidance ( sender )
-			 */
-			if (seg_size < TCP_SKB_CB(skb)->end_seq - TCP_SKB_CB(skb)->seq ||
-			    skb->len > mss) {
-				seg_size = min(seg_size, mss);
-				TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;
-				if (tcp_fragment(sk, skb, seg_size, mss))
-					return -1;
-			} else if (!tcp_skb_pcount(skb))
-				tcp_set_skb_tso_segs(sk, skb, mss);
+	struct tcp_sock *tp = tcp_sk(sk);
+	struct sk_buff *skb;
 
+	if (sk->sk_state == TCP_CLOSE)
+		return -1;
+
+	if ((skb = tcp_send_head(sk)) != NULL &&
+	    before(TCP_SKB_CB(skb)->seq, tcp_wnd_end(tp))) {
+		int err;
+		unsigned int mss = tcp_current_mss(sk, 0);
+		unsigned int seg_size = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
+
+		if (before(tp->pushed_seq, TCP_SKB_CB(skb)->end_seq))
+			tp->pushed_seq = TCP_SKB_CB(skb)->end_seq;
+
+		/* We are probing the opening of a window
+		 * but the window size is != 0
+		 * must have been a result SWS avoidance ( sender )
+		 */
+		if (seg_size < TCP_SKB_CB(skb)->end_seq - TCP_SKB_CB(skb)->seq ||
+		    skb->len > mss) {
+			seg_size = min(seg_size, mss);
 			TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;
-			TCP_SKB_CB(skb)->when = tcp_time_stamp;
-			err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
-			if (!err)
-				tcp_event_new_data_sent(sk, skb);
-			return err;
-		} else {
-			if (tp->urg_mode &&
-			    between(tp->snd_up, tp->snd_una+1, tp->snd_una+0xFFFF))
-				tcp_xmit_probe_skb(sk, 1);
-			return tcp_xmit_probe_skb(sk, 0);
-		}
+			if (tcp_fragment(sk, skb, seg_size, mss))
+				return -1;
+		} else if (!tcp_skb_pcount(skb))
+			tcp_set_skb_tso_segs(sk, skb, mss);
+
+		TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;
+		TCP_SKB_CB(skb)->when = tcp_time_stamp;
+		err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
+		if (!err)
+			tcp_event_new_data_sent(sk, skb);
+		return err;
+	} else {
+		if (tp->urg_mode &&
+		    between(tp->snd_up, tp->snd_una + 1, tp->snd_una + 0xFFFF))
+			tcp_xmit_probe_skb(sk, 1);
+		return tcp_xmit_probe_skb(sk, 0);
 	}
-	return -1;
 }
 
 /* A window probe timeout has occurred.  If window is not closed send
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH 2/9] [TCP]: Rename update_send_head & include related increment to it
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1199098077874-git-send-email-ilpo.jarvinen@helsinki.fi>

There's very little need to have the packets_out incrementing in
a separate function. Also name the combined function
appropriately.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |   32 ++++++++++++--------------------
 1 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7a4834a..1ca638b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -61,29 +61,22 @@ int sysctl_tcp_base_mss __read_mostly = 512;
 /* By default, RFC2861 behavior.  */
 int sysctl_tcp_slow_start_after_idle __read_mostly = 1;
 
-static inline void tcp_packets_out_inc(struct sock *sk,
-				       const struct sk_buff *skb)
-{
-	struct tcp_sock *tp = tcp_sk(sk);
-	int orig = tp->packets_out;
-
-	tp->packets_out += tcp_skb_pcount(skb);
-	if (!orig)
-		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
-					  inet_csk(sk)->icsk_rto, TCP_RTO_MAX);
-}
-
-static void update_send_head(struct sock *sk, struct sk_buff *skb)
+static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
+	unsigned int prior_packets = tp->packets_out;
 
 	tcp_advance_send_head(sk, skb);
 	tp->snd_nxt = TCP_SKB_CB(skb)->end_seq;
-	tcp_packets_out_inc(sk, skb);
 
 	/* Don't override Nagle indefinately with F-RTO */
 	if (tp->frto_counter == 2)
 		tp->frto_counter = 3;
+
+	tp->packets_out += tcp_skb_pcount(skb);
+	if (!prior_packets)
+		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
+					  inet_csk(sk)->icsk_rto, TCP_RTO_MAX);
 }
 
 /* SND.NXT, if window was not shrunk.
@@ -1410,7 +1403,7 @@ static int tcp_mtu_probe(struct sock *sk)
 		/* Decrement cwnd here because we are sending
 		* effectively two packets. */
 		tp->snd_cwnd--;
-		update_send_head(sk, nskb);
+		tcp_event_new_data_sent(sk, nskb);
 
 		icsk->icsk_mtup.probe_size = tcp_mss_to_mtu(sk, nskb->len);
 		tp->mtu_probe.probe_seq_start = TCP_SKB_CB(nskb)->seq;
@@ -1494,7 +1487,7 @@ static int tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle)
 		/* Advance the send_head.  This one is sent out.
 		 * This call will increment packets_out.
 		 */
-		update_send_head(sk, skb);
+		tcp_event_new_data_sent(sk, skb);
 
 		tcp_minshall_update(tp, mss_now, skb);
 		sent_pkts++;
@@ -1553,7 +1546,7 @@ void tcp_push_one(struct sock *sk, unsigned int mss_now)
 		TCP_SKB_CB(skb)->when = tcp_time_stamp;
 
 		if (likely(!tcp_transmit_skb(sk, skb, 1, sk->sk_allocation))) {
-			update_send_head(sk, skb);
+			tcp_event_new_data_sent(sk, skb);
 			tcp_cwnd_validate(sk);
 			return;
 		}
@@ -2528,9 +2521,8 @@ int tcp_write_wakeup(struct sock *sk)
 			TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;
 			TCP_SKB_CB(skb)->when = tcp_time_stamp;
 			err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
-			if (!err) {
-				update_send_head(sk, skb);
-			}
+			if (!err)
+				tcp_event_new_data_sent(sk, skb);
 			return err;
 		} else {
 			if (tp->urg_mode &&
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH 1/9] [TCP]: Make invariant check complain about invalid sacked_out
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980772344-git-send-email-ilpo.jarvinen@helsinki.fi>

Earlier resolution for NewReno's sacked_out should now keep
it small enough for this to become invariant-like check.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_input.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 722c9cb..41f4b86 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2504,11 +2504,8 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
 				    (tcp_fackets_out(tp) > tp->reordering));
 	int fast_rexmit = 0;
 
-	/* Some technical things:
-	 * 1. Reno does not count dupacks (sacked_out) automatically. */
-	if (!tp->packets_out)
+	if (WARN_ON(!tp->packets_out && tp->sacked_out))
 		tp->sacked_out = 0;
-
 	if (WARN_ON(!tp->sacked_out && tp->fackets_out))
 		tp->fackets_out = 0;
 
-- 
1.5.0.6


^ permalink raw reply related

* [PATCH net-2.6.25 0/9]: TCP cleanups & minor changes.
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Hi Dave,

The first one is restored after getting removed in the
straight-forward revert we did.

Please check that the TCPCB_URG removal is indeed valid. I couldn't
find any use for it but there might be some non-obviously named
things I've missed.

I did a larger cleanup with indent for tcp_input and tcp_output
because I started to find so many unnecessarily line split &
missing spaces here and there. There's still a lot to do because
not every case has a trivial solution but at least something got
cleaner :-).

These should apply cleanly to the rebased net-2.6.25. I did some
trivial test with them before the rebase.                        
                                                    
   
-- 
 i.



^ permalink raw reply

* [PATCH 3/9] [TCP]: Remove unnecessary local variables
From: Ilpo Järvinen @ 2007-12-31 10:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <11990980773391-git-send-email-ilpo.jarvinen@helsinki.fi>

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 1ca638b..025dddf 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -995,9 +995,8 @@ unsigned int tcp_current_mss(struct sock *sk, int large_allowed)
 static void tcp_cwnd_validate(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
-	__u32 packets_out = tp->packets_out;
 
-	if (packets_out >= tp->snd_cwnd) {
+	if (tp->packets_out >= tp->snd_cwnd) {
 		/* Network is feed fully. */
 		tp->snd_cwnd_used = 0;
 		tp->snd_cwnd_stamp = tcp_time_stamp;
@@ -1042,17 +1041,13 @@ static unsigned int tcp_mss_split_point(struct sock *sk, struct sk_buff *skb,
  */
 static inline unsigned int tcp_cwnd_test(struct tcp_sock *tp, struct sk_buff *skb)
 {
-	u32 in_flight, cwnd;
-
 	/* Don't be strict about the congestion window for the final FIN.  */
 	if ((TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) &&
 	    tcp_skb_pcount(skb) == 1)
 		return 1;
 
-	in_flight = tcp_packets_in_flight(tp);
-	cwnd = tp->snd_cwnd;
-	if (in_flight < cwnd)
-		return (cwnd - in_flight);
+	if (tcp_packets_in_flight(tp) < tp->snd_cwnd)
+		return tp->snd_cwnd - tcp_packets_in_flight(tp);
 
 	return 0;
 }
-- 
1.5.0.6


^ permalink raw reply related

* Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
From: Herbert Xu @ 2007-12-31  9:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, haoki, netdev, tyasui, mhiramat, satoshi.oshima.fk,
	billfink, andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf
In-Reply-To: <4778AE48.1040701@cosmosbay.com>

On Mon, Dec 31, 2007 at 09:54:32AM +0100, Eric Dumazet wrote:
>
> Maybe I read the patch incorrectly, or we could add some new sysctl so that
> we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
> limit (say 2 pages), so that number of atomic_inc/dec on 
> udp_memory_allocated (or tcp_memory_allocated) is reduced.

I don't have time to go through these patches right now but
that was the whole point of using forward_alloc, i.e., avoid
touching the global for every packet.

So if it's still doing that then we're doing something wrong.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH net-2.6.25 4/7][ATM]: [br2864] routed support
From: Christoph Hellwig @ 2007-12-31  9:09 UTC (permalink / raw)
  To: chas williams - CONTRACTOR; +Cc: netdev, davem
In-Reply-To: <200712300107.lBU17O8i003566@cmf.nrl.navy.mil>

> +#define BR2684_ETHERTYPE_LEN	2
> +#define BR2684_PAD_LEN		2
> +
> +#define LLC		0xaa, 0xaa, 0x03
> +#define SNAP_BRIDGED	0x00, 0x80, 0xc2
> +#define SNAP_ROUTED	0x00, 0x00, 0x00
> +#define PID_ETHERNET	0x00, 0x07
> +#define ETHERTYPE_IPV4	0x08, 0x00
> +#define ETHERTYPE_IPV6	0x86, 0xdd
> +#define PAD_BRIDGED	0x00, 0x00
> +
> +static unsigned char ethertype_ipv4[] =
> +	{ ETHERTYPE_IPV4 };
> +static unsigned char ethertype_ipv6[] =
> +	{ ETHERTYPE_IPV6 };
>  static unsigned char llc_oui_pid_pad[] =
> -    { 0xAA, 0xAA, 0x03, 0x00, 0x80, 0xC2, 0x00, 0x07, 0x00, 0x00 };
> -#define PADLEN	(2)
> +	{ LLC, SNAP_BRIDGED, PID_ETHERNET, PAD_BRIDGED };
> +static unsigned char llc_oui_ipv4[] =
> +	{ LLC, SNAP_ROUTED, ETHERTYPE_IPV4 };
> +static unsigned char llc_oui_ipv6[] =
> +	{ LLC, SNAP_ROUTED, ETHERTYPE_IPV6 };

I think this should be in a global header.  And IIRC we already have
at least some of these somewhere as part of the general llc support.


^ permalink raw reply

* Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
From: Eric Dumazet @ 2007-12-31  8:54 UTC (permalink / raw)
  To: David Miller
  Cc: haoki, herbert, netdev, tyasui, mhiramat, satoshi.oshima.fk,
	billfink, andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf
In-Reply-To: <20071231.001925.151533664.davem@davemloft.net>

David Miller a écrit :
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
> 
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>>  		   atomic_read(&tcp_memory_allocated));
>> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> +		   atomic_read(&udp_memory_allocated));
>>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
> 
> More careless patch creation.  :-/
> 
> This breaks the build because udp_memory_allocated is not added until
> patch 2.
> 
> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.

I am a litle bit concerned about performance of IVR servers
using SIP protocol.

On those servers, each active channel typically emits/receives 50 UDP/RTP 
frames per second. With G729 codec, each packet contains 10 bytes of payload, 
and about 40 bytes of IP/UDP/RTP encapsulation. (So these messages are very
small)

As I am currently enjoying hollidays at home, I am not able to test on my 
server farm the performance impact of this new UDP receive accounting.

If I understand well the patch, each time a packet is received (on a socket
with no previous message available in its receive queue), we are going to 
atomic_inc(&some_global_var). Then the user thread that will transfert this
message to user land will atomic_dec(&some_global_var). (Granted server is
in normal condition, ie each UDP socket holds at most one message in its
receive or transmit queue)

I have some machines with 400 active SIP channels, so that new hot cache line
will probably slow down our SMP servers, because of cache line ping pong.

I will probably setup a test next week and let you know the results.

Maybe I read the patch incorrectly, or we could add some new sysctl so that
we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
limit (say 2 pages), so that number of atomic_inc/dec on udp_memory_allocated 
(or tcp_memory_allocated) is reduced.

Thank you

^ permalink raw reply

* Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
From: David Miller @ 2007-12-31  8:19 UTC (permalink / raw)
  To: haoki
  Cc: herbert, netdev, tyasui, mhiramat, satoshi.oshima.fk, billfink,
	andi, johnpol, shemminger, yoshfuji, yumiko.sugita.yf
In-Reply-To: <47775E7A.60708@redhat.com>

From: Hideo AOKI <haoki@redhat.com>
Date: Sun, 30 Dec 2007 04:01:46 -0500

> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>  		   atomic_read(&tcp_memory_allocated));
> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
> +		   atomic_read(&udp_memory_allocated));
>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",

More careless patch creation.  :-/

This breaks the build because udp_memory_allocated is not added until
patch 2.

Once again I'll combine all three patches into one but I am extremely
angry about how careless and broken these two patch submissions were.

^ permalink raw reply

* Re: [PATCH 2/4] [CORE]: adding memory accounting points
From: David Miller @ 2007-12-31  7:58 UTC (permalink / raw)
  To: haoki
  Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
	mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
	yoshfuji, yumiko.sugita.yf
In-Reply-To: <47775C20.5010004@redhat.com>

From: Hideo AOKI <haoki@redhat.com>
Date: Sun, 30 Dec 2007 03:51:44 -0500

> To consolidate memory accounting functions, this patch adds memory
> accounting calls to network core functions. Moreover, present
> memory accounting call is renamed to new accounting call.
> 
> Cc: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
> Cc: Masami Hiramatsu <mhiramat@redhat.com>
> signed-off-by: Takahiro Yasui <tyasui@redhat.com>
> signed-off-by: Hideo Aoki <haoki@redhat.com>

This patch would not apply, because is contained changes
present in the first patch, specifically:

> diff -pruN net-2.6.25-t12t19m-p1/include/net/sock.h net-2.6.25-t12t19m-p2/include/net/sock.h
> --- net-2.6.25-t12t19m-p1/include/net/sock.h	2007-12-29 20:16:31.000000000 -0500
> +++ net-2.6.25-t12t19m-p2/include/net/sock.h	2007-12-29 20:28:15.000000000 -0500
> @@ -1116,7 +1116,7 @@ static inline int skb_copy_to_page(struc
>  	skb->data_len	     += copy;
>  	skb->truesize	     += copy;
>  	sk->sk_wmem_queued   += copy;
> -	sk->sk_forward_alloc -= copy;
> +	sk_mem_charge(sk, copy);
>  	return 0;
>  }
> 
> @@ -1142,6 +1142,7 @@ static inline void skb_set_owner_r(struc
>  	skb->sk = sk;
>  	skb->destructor = sock_rfree;
>  	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
> +	sk_mem_charge(sk, skb->truesize);
>  }
> 
>  extern void sk_reset_timer(struct sock *sk, struct timer_list* timer,

And now I see exactly what you did, and it is quite careless.

You wrote one big patch then tried to split it up by hand.  This
proves to me that you did not test the patches individually.  Even
worse, you did not even try to apply each patch nor compile the tree
each step along the way as a basic sanity check.

This wastes a lot of my time, as well as the time of other developers
who might want to try out and test your changes.

I will fix it up this time, but please do not ever do this again.

^ permalink raw reply

* Re: [PATCH 0/4] New interface for memory accounting (take 1)
From: David Miller @ 2007-12-31  7:34 UTC (permalink / raw)
  To: haoki
  Cc: herbert, vladislav.yasevich, netdev, lksctp-developers, tyasui,
	mhiramat, satoshi.oshima.fk, billfink, andi, johnpol, shemminger,
	yoshfuji, yumiko.sugita.yf
In-Reply-To: <47775B25.7020401@redhat.com>

From: Hideo AOKI <haoki@redhat.com>
Date: Sun, 30 Dec 2007 03:47:33 -0500

> Hello,
> 
> This patch set introduces new memory accounting interface.
> Current interface is written for stream protocols only.
> To enable memory accounting in other protocols (e.g. UDP),
> I enhanced the interface and updated TCP and SCTP memory
> accounting.
> 
> The patch set consists of the following 4 patches.
> 
> [1/4] introducing new memory accounting interface
> [2/4] adding memory accounting points to consolidate functions
> [3/4] updating TCP to use new interface
> [4/4] updating SCTP to use new interface
> 
> The patch set was tested on net-2.6.25 tree.

I like this work very much and will add this to net-2.6.25
But I will have to combine it all into one patch.

You cannot have one patch which breaks the build in any way.  All of
the kernel must build properly after each patch in your patchset is
applied.

Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP
stop building.

We enforce this rule, otherwise when users try to use "git bisect" to
find out where regressions are added, they will get stuck in places
like this where the tree will not build due to such careless
changesets.

Thank you.

^ permalink raw reply

* Re: [PATCH] remove useless code from fib6_del_route
From: David Miller @ 2007-12-31  7:27 UTC (permalink / raw)
  To: guijianfeng; +Cc: netdev
In-Reply-To: <4774A1DC.9060308@cn.fujitsu.com>

From: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Date: Fri, 28 Dec 2007 15:12:28 +0800

> There are useless codes in fib6_del_route(). The following patch
> has been tested, every thing looks fine, as usual.
> 
> Signed-off-by: Gui Jianfeng<guijianfeng@cn.fujitsu.com>

Patch applied to net-2.6.25, thanks.

^ permalink raw reply

* Re: [PATCH net-2.6.25] [NEIGH] Remove unused method from include/net/neighbour.h
From: David Miller @ 2007-12-31  7:26 UTC (permalink / raw)
  To: ramirose; +Cc: netdev
In-Reply-To: <eb3ff54b0712280756m45c92c0bhc1298f6b428ac3ed@mail.gmail.com>

From: "Rami Rosen" <ramirose@gmail.com>
Date: Fri, 28 Dec 2007 17:56:46 +0200

> It seems to me that neigh_is_valid() inline method from
> include/net/neighbour.h can be removed since it is not used.
> 
> Though it can be considered as an API for future use, and
> indeed we have neigh_is_connected() method in this same header (which is BTW
> used only in one place, drivers/net/cxgb3/l2t.c), we can easily notice
> that we don't have
> any accessor method to check whether the neighbour is in NUD_IN_TIMER
> (not to mention that we don't have methods to check the single neighbour states,
> like NUD_REACHABLE or NUD_DELAY or the other ones). So for consistency
> I suggest to
> consider removing the neigh_is_valid() method.
>
> Signed-off-by: Rami Rosen <ramirose@gmail.com>

This is a helper function which was used by drivers implementing
support for the old fast routing code which we removed years ago.

So yes we should remove this.

Patch applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25] [IPv4] Remove unused define in include/net/arp.h (HAVE_ARP_CREATE)
From: David Miller @ 2007-12-31  7:23 UTC (permalink / raw)
  To: ramirose; +Cc: netdev
In-Reply-To: <eb3ff54b0712280720r723856afi55f591e214b7ed14@mail.gmail.com>

From: "Rami Rosen" <ramirose@gmail.com>
Date: Fri, 28 Dec 2007 17:20:34 +0200

> Signed-off-by: Rami Rosen <ramirose@gmail.com>

This was added long ago by some bonding driver infrastructure changes
so that code could check whether the arp_create() helper function
existed in the kernel.

But it is totally unused now and yes it should be removed.

Patch applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 7/7][ATM]: [he] fixing compilation when you define USE_RBPS_POOL/USE_RBPL_POOL
From: David Miller @ 2007-12-31  7:20 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300108.lBU18UUa003635@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:08:30 -0500

>     [ATM]: [he] fixing compilation when you define USE_RBPS_POOL/USE_RBPL_POOL
>     
>     Signed-off-by: Jorge Boncompte <jorge@dti2.net>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 6/7][ATM]: [ambassador] kmalloc + memset conversion to kzalloc
From: David Miller @ 2007-12-31  7:19 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300108.lBU18CjE003595@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:08:12 -0500

>     [ATM]: [ambassador] kmalloc + memset conversion to kzalloc
>     
>     Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 5/7][ATM]: [br2864] whitespace cleanup
From: David Miller @ 2007-12-31  7:18 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300107.lBU17nCH003581@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:07:49 -0500

>     [ATM]: [br2864] whitespace cleanup
>     
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 4/7][ATM]: [br2864] routed support
From: David Miller @ 2007-12-31  7:18 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300107.lBU17O8i003566@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:07:24 -0500

>     [ATM]: [br2864] routed support
>     
>     From: Eric Kinzie <ekinzie@cmf.nrl.navy.mil>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 3/7][ATM]: [he] This patch removes the ancient version string.
From: David Miller @ 2007-12-31  7:16 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300106.lBU16umE003551@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:06:56 -0500

>     [ATM]: [he] This patch removes the ancient version string.
>     
>     Signed-off-by: Adrian Bunk <bunk@stusta.de>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 2/7][ATM]: Convert struct class_device to struct device
From: David Miller @ 2007-12-31  7:16 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300106.lBU16O1P003529@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:06:24 -0500

>     [ATM]: Convert struct class_device to struct device
>     
>     Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
>     Cc: Tony Jones <tonyj@suse.de>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6.25 1/7][ATM]: atm is no longer experimental
From: David Miller @ 2007-12-31  7:15 UTC (permalink / raw)
  To: chas; +Cc: netdev
In-Reply-To: <200712300106.lBU16311003517@cmf.nrl.navy.mil>

From: "chas williams - CONTRACTOR" <chas@cmf.nrl.navy.mil>
Date: Sat, 29 Dec 2007 20:06:03 -0500

>     [ATM]: atm is no longer experimental
>     
>     From: Robert P. J. Day <rpjday@mindspring.com>
>     Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>

Applied.

^ permalink raw reply

* (no subject)
From: Ramesh R @ 2007-12-31  7:03 UTC (permalink / raw)
  To: netdev

subscribe linux-wireless

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox