* [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
@ 2018-11-05 13:53 Suraj Singh
2018-11-05 18:20 ` David Miller
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Suraj Singh @ 2018-11-05 13:53 UTC (permalink / raw)
To: edumazet; +Cc: davem, kuznet, yoshfuji, netdev, linux-kernel, Suraj Singh
Fixed warnings and checks for TCP Westwood
Signed-off-by: Suraj Singh <suraj1998@gmail.com>
---
net/ipv4/tcp_westwood.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index bec9caf..0b4c67a9 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -43,11 +43,10 @@ struct westwood {
};
/* TCP Westwood functions and constants */
-#define TCP_WESTWOOD_RTT_MIN (HZ/20) /* 50ms */
-#define TCP_WESTWOOD_INIT_RTT (20*HZ) /* maybe too conservative?! */
+#define TCP_WESTWOOD_RTT_MIN (HZ / 20) /* 50ms */
+#define TCP_WESTWOOD_INIT_RTT (20 * HZ) /* maybe too conservative?! */
-/*
- * @tcp_westwood_create
+/* @tcp_westwood_create
* This function initializes fields used in TCP Westwood+,
* it is called after the initial SYN, so the sequence numbers
* are correct but new passive connections we have no
@@ -67,14 +66,14 @@ static void tcp_westwood_init(struct sock *sk)
w->accounted = 0;
w->cumul_ack = 0;
w->reset_rtt_min = 1;
- w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
+ w->rtt_min = TCP_WESTWOOD_INIT_RTT;
+ w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_jiffies32;
w->snd_una = tcp_sk(sk)->snd_una;
w->first_ack = 1;
}
-/*
- * @westwood_do_filter
+/* @westwood_do_filter
* Low-pass filter. Implemented using constant coefficients.
*/
static inline u32 westwood_do_filter(u32 a, u32 b)
@@ -94,8 +93,7 @@ static void westwood_filter(struct westwood *w, u32 delta)
}
}
-/*
- * @westwood_pkts_acked
+/* @westwood_pkts_acked
* Called after processing group of packets.
* but all westwood needs is the last sample of srtt.
*/
@@ -108,8 +106,7 @@ static void tcp_westwood_pkts_acked(struct sock *sk,
w->rtt = usecs_to_jiffies(sample->rtt_us);
}
-/*
- * @westwood_update_window
+/* @westwood_update_window
* It updates RTT evaluation window if it is the right moment to do
* it. If so it calls filter for evaluating bandwidth.
*/
@@ -127,8 +124,7 @@ static void westwood_update_window(struct sock *sk)
w->first_ack = 0;
}
- /*
- * See if a RTT-window has passed.
+ /* See if a RTT-window has passed.
* Be careful since if RTT is less than
* 50ms we don't filter but we continue 'building the sample'.
* This minimum limit was chosen since an estimation on small
@@ -149,12 +145,12 @@ static inline void update_rtt_min(struct westwood *w)
if (w->reset_rtt_min) {
w->rtt_min = w->rtt;
w->reset_rtt_min = 0;
- } else
+ } else {
w->rtt_min = min(w->rtt, w->rtt_min);
+ }
}
-/*
- * @westwood_fast_bw
+/* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
* header prediction is successful. In such case in fact update is
* straight forward and doesn't need any particular care.
@@ -171,8 +167,7 @@ static inline void westwood_fast_bw(struct sock *sk)
update_rtt_min(w);
}
-/*
- * @westwood_acked_count
+/* @westwood_acked_count
* This function evaluates cumul_ack for evaluating bk in case of
* delayed or partial acks.
*/
@@ -207,8 +202,7 @@ static inline u32 westwood_acked_count(struct sock *sk)
return w->cumul_ack;
}
-/*
- * TCP Westwood
+/* TCP Westwood
* Here limit is evaluated as Bw estimation*RTTmin (for obtaining it
* in packets we use mss_cache). Rttmin is guaranteed to be >= 2
* so avoids ever returning 0.
@@ -243,7 +237,8 @@ static void tcp_westwood_event(struct sock *sk, enum tcp_ca_event event)
switch (event) {
case CA_EVENT_COMPLETE_CWR:
- tp->snd_cwnd = tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
+ tp->snd_cwnd = tcp_westwood_bw_rttmin(sk);
+ tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
break;
case CA_EVENT_LOSS:
tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
2018-11-05 13:53 [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks Suraj Singh
@ 2018-11-05 18:20 ` David Miller
2018-11-06 19:15 ` David Miller
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2018-11-05 18:20 UTC (permalink / raw)
To: suraj1998; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel
From: Suraj Singh <suraj1998@gmail.com>
Date: Mon, 5 Nov 2018 19:23:05 +0530
> Fixed warnings and checks for TCP Westwood
>
> Signed-off-by: Suraj Singh <suraj1998@gmail.com>
Why 'staging' in the subject line?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
2018-11-05 13:53 [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks Suraj Singh
2018-11-05 18:20 ` David Miller
@ 2018-11-06 19:15 ` David Miller
2018-11-08 9:16 ` Suraj Singh
2018-11-08 9:46 ` [PATCH] staging: " Suraj Singh
3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2018-11-06 19:15 UTC (permalink / raw)
To: suraj1998; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel
From: Suraj Singh <suraj1998@gmail.com>
Date: Mon, 5 Nov 2018 19:23:05 +0530
> Fixed warnings and checks for TCP Westwood
>
> Signed-off-by: Suraj Singh <suraj1998@gmail.com>
I asked you yesterday why "staging: " appears in your subject line
and you have failed to respond and explain.
There are also functional issues with your patch:
> - tp->snd_cwnd = tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
> + tp->snd_cwnd = tcp_westwood_bw_rttmin(sk);
> + tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
This is bogus, now tcp_westwood_bw_rttmin(sk) will potentially be called
two times instead of once.
The existing code is fine, please do not modify it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: net: ipv4: tcp_westwood: fixed warnings and checks
2018-11-05 13:53 [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks Suraj Singh
2018-11-05 18:20 ` David Miller
2018-11-06 19:15 ` David Miller
@ 2018-11-08 9:16 ` Suraj Singh
2018-11-08 9:46 ` [PATCH] staging: " Suraj Singh
3 siblings, 0 replies; 7+ messages in thread
From: Suraj Singh @ 2018-11-08 9:16 UTC (permalink / raw)
To: davem; +Cc: kuznet, yoshfuji, netdev, linux-kernel, suraj1998, Suraj Singh
From: Suraj Singh <suraj998@gmail.com>
Regrding why I used "staging: " in the commmit message, I was following Greg Kroah-Hartman's video on YouTube on how to submit your first patch, and in his sample commit, he'd started his commit message with "staging: ", and so I thought it was convention to do so. I'll remove that immediately. I made this same mistake in another patch that I just sent for TCP Vegas, I'll make the change there as well.
I didn't consider the complexities of calling the same function twice. I was looking more towards satisying the scriptpatch.pl's requirements.
- tp->snd_cwnd = tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
+ tp->snd_cwnd = tcp_westwood_bw_rttmin(sk);
+ tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
I've made the same mistake here. I'll make these changes and resubmit. Is there anything else that's wrong?
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
2018-11-05 13:53 [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks Suraj Singh
` (2 preceding siblings ...)
2018-11-08 9:16 ` Suraj Singh
@ 2018-11-08 9:46 ` Suraj Singh
2018-11-08 10:22 ` Aw: " Lino Sanfilippo
2018-11-09 12:18 ` net: ipv4: tcp_westwood Suraj Singh
3 siblings, 2 replies; 7+ messages in thread
From: Suraj Singh @ 2018-11-08 9:46 UTC (permalink / raw)
To: davem; +Cc: kuznet, yoshfuji, netdev, linux-kernel, suraj1998
Fixed warnings and checks for TCP Westwood
Signed-off-by: Suraj Singh <suraj1998@gmail.com>
---
net/ipv4/tcp_westwood.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index bec9caf..8879152 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -43,11 +43,10 @@ struct westwood {
};
/* TCP Westwood functions and constants */
-#define TCP_WESTWOOD_RTT_MIN (HZ/20) /* 50ms */
-#define TCP_WESTWOOD_INIT_RTT (20*HZ) /* maybe too conservative?! */
+#define TCP_WESTWOOD_RTT_MIN (HZ / 20) /* 50ms */
+#define TCP_WESTWOOD_INIT_RTT (20 * HZ) /* maybe too conservative?! */
-/*
- * @tcp_westwood_create
+/* @tcp_westwood_create
* This function initializes fields used in TCP Westwood+,
* it is called after the initial SYN, so the sequence numbers
* are correct but new passive connections we have no
@@ -73,8 +72,7 @@ static void tcp_westwood_init(struct sock *sk)
w->first_ack = 1;
}
-/*
- * @westwood_do_filter
+/* @westwood_do_filter
* Low-pass filter. Implemented using constant coefficients.
*/
static inline u32 westwood_do_filter(u32 a, u32 b)
@@ -94,8 +92,7 @@ static void westwood_filter(struct westwood *w, u32 delta)
}
}
-/*
- * @westwood_pkts_acked
+/* @westwood_pkts_acked
* Called after processing group of packets.
* but all westwood needs is the last sample of srtt.
*/
@@ -108,8 +105,7 @@ static void tcp_westwood_pkts_acked(struct sock *sk,
w->rtt = usecs_to_jiffies(sample->rtt_us);
}
-/*
- * @westwood_update_window
+/* @westwood_update_window
* It updates RTT evaluation window if it is the right moment to do
* it. If so it calls filter for evaluating bandwidth.
*/
@@ -127,8 +123,7 @@ static void westwood_update_window(struct sock *sk)
w->first_ack = 0;
}
- /*
- * See if a RTT-window has passed.
+ /* See if a RTT-window has passed.
* Be careful since if RTT is less than
* 50ms we don't filter but we continue 'building the sample'.
* This minimum limit was chosen since an estimation on small
@@ -149,12 +144,12 @@ static inline void update_rtt_min(struct westwood *w)
if (w->reset_rtt_min) {
w->rtt_min = w->rtt;
w->reset_rtt_min = 0;
- } else
+ } else {
w->rtt_min = min(w->rtt, w->rtt_min);
+ }
}
-/*
- * @westwood_fast_bw
+/* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
* header prediction is successful. In such case in fact update is
* straight forward and doesn't need any particular care.
@@ -171,8 +166,7 @@ static inline void westwood_fast_bw(struct sock *sk)
update_rtt_min(w);
}
-/*
- * @westwood_acked_count
+/* @westwood_acked_count
* This function evaluates cumul_ack for evaluating bk in case of
* delayed or partial acks.
*/
@@ -207,8 +201,7 @@ static inline u32 westwood_acked_count(struct sock *sk)
return w->cumul_ack;
}
-/*
- * TCP Westwood
+/* TCP Westwood
* Here limit is evaluated as Bw estimation*RTTmin (for obtaining it
* in packets we use mss_cache). Rttmin is guaranteed to be >= 2
* so avoids ever returning 0.
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Aw: [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
2018-11-08 9:46 ` [PATCH] staging: " Suraj Singh
@ 2018-11-08 10:22 ` Lino Sanfilippo
2018-11-09 12:18 ` net: ipv4: tcp_westwood Suraj Singh
1 sibling, 0 replies; 7+ messages in thread
From: Lino Sanfilippo @ 2018-11-08 10:22 UTC (permalink / raw)
To: Suraj Singh; +Cc: davem, kuznet, yoshfuji, netdev, linux-kernel, suraj1998
Hi,
> Gesendet: Donnerstag, 08. November 2018 um 10:46 Uhr
> Von: "Suraj Singh" <suraj1998@gmail.com>
> An: davem@davemloft.net
> Cc: kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, suraj1998@gmail.com
> Betreff: [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
>
> Fixed warnings and checks for TCP Westwood
>
> Signed-off-by: Suraj Singh <suraj1998@gmail.com>
> ---
You use the prefix "staging" again in you subject line, which still is wrong. "staging" means that you
fix something from the staging area (see drivers/staging/ in the kernel sources. BTW: the staging area is a much
better starting point for first patches than the core parts of the network subsystem).
Also if you send a subsequent version of a patch you have to write the version number (e.g. v2) in the
subject line. Have a look at other patch submissions on this mailing list for examples.
Regards,
Lino
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: net: ipv4: tcp_westwood
2018-11-08 9:46 ` [PATCH] staging: " Suraj Singh
2018-11-08 10:22 ` Aw: " Lino Sanfilippo
@ 2018-11-09 12:18 ` Suraj Singh
1 sibling, 0 replies; 7+ messages in thread
From: Suraj Singh @ 2018-11-09 12:18 UTC (permalink / raw)
To: davem; +Cc: kuznet, yoshfuji, netdev, linux-kernel, suraj1998, LinoSanfilippo
Hi Lino,
Thank you for clarifying the doubts I had and for the suggestion on where to
start contributing. Previously, and in this patch I hadn't configured mutt
and was manually using git send-mail to respond to each thread using the
corresponding mail-id. That's why I wasn't sure why even in this patch,
"staging: " had appeared even after editing the commit message. Now that I've
got the email client setup, I'll fix that "staging: " issue and send a [v3]
patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-09 12:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05 13:53 [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks Suraj Singh
2018-11-05 18:20 ` David Miller
2018-11-06 19:15 ` David Miller
2018-11-08 9:16 ` Suraj Singh
2018-11-08 9:46 ` [PATCH] staging: " Suraj Singh
2018-11-08 10:22 ` Aw: " Lino Sanfilippo
2018-11-09 12:18 ` net: ipv4: tcp_westwood Suraj Singh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.