tcp: constify congestion_ops
Subsystems (4)
cover tcp: constify congestion_ops
This is a proposed method of making TCP congestion_ops structure const.
I wonder if restricting congestion control choices is still necessary? It seems like being overly paranoid, and better enforced by having a more limited kernel config, seccomp or other mechanism.
Stephen Hemminger (1):
constify tcp congestion
include/net/tcp.h | 10 ++- net/ipv4/tcp.c | 2 - net/ipv4/tcp_bbr.c | 2 +- net/ipv4/tcp_bic.c | 2 +- net/ipv4/tcp_cdg.c | 2 +- net/ipv4/tcp_cong.c | 162 +++++++++++++++++++++++++++++------------------ net/ipv4/tcp_cubic.c | 2 +- net/ipv4/tcp_dctcp.c | 6 +- net/ipv4/tcp_highspeed.c | 2 +- net/ipv4/tcp_htcp.c | 2 +- net/ipv4/tcp_hybla.c | 2 +- net/ipv4/tcp_illinois.c | 2 +- net/ipv4/tcp_lp.c | 2 +- net/ipv4/tcp_nv.c | 2 +- net/ipv4/tcp_scalable.c | 2 +- net/ipv4/tcp_vegas.c | 2 +- net/ipv4/tcp_veno.c | 2 +- net/ipv4/tcp_westwood.c | 2 +- net/ipv4/tcp_yeah.c | 2 +- 19 files changed, 124 insertions(+), 86 deletions(-)
Signature · 1 line
2.11.0
RFC 1/1 constify tcp congestion
Split the TCP congestion ops structure into const and mutable portions. Put the list pointers, key and a copy of the flags in new tcp_congestion_entry structure.
Signed-off-by Stephen Hemminger <[email protected]>
--- include/net/tcp.h | 10 ++- net/ipv4/tcp.c | 2 - net/ipv4/tcp_bbr.c | 2 +- net/ipv4/tcp_bic.c | 2 +- net/ipv4/tcp_cdg.c | 2 +- net/ipv4/tcp_cong.c | 162 +++++++++++++++++++++++++++++------------------ net/ipv4/tcp_cubic.c | 2 +- net/ipv4/tcp_dctcp.c | 6 +- net/ipv4/tcp_highspeed.c | 2 +- net/ipv4/tcp_htcp.c | 2 +- net/ipv4/tcp_hybla.c | 2 +- net/ipv4/tcp_illinois.c | 2 +- net/ipv4/tcp_lp.c | 2 +- net/ipv4/tcp_nv.c | 2 +- net/ipv4/tcp_scalable.c | 2 +- net/ipv4/tcp_vegas.c | 2 +- net/ipv4/tcp_veno.c | 2 +- net/ipv4/tcp_westwood.c | 2 +- net/ipv4/tcp_yeah.c | 2 +- 19 files changed, 124 insertions(+), 86 deletions(-)
include/net/tcp.h +4 -6
@@ -931,8 +931,6 @@ struct rate_sample { };struct tcp_congestion_ops { -struct list_head list; -u32 key;u32 flags;/* initialize private data (optional) */
@@ -970,8 +968,8 @@ struct tcp_congestion_ops {struct module *owner; }; -int tcp_register_congestion_control (struct tcp_congestion_ops *type); -void tcp_unregister_congestion_control (struct tcp_congestion_ops *type); +int tcp_register_congestion_control (const struct tcp_congestion_ops *type); +void tcp_unregister_congestion_control (const struct tcp_congestion_ops *type);void tcp_assign_congestion_control (struct sock *sk);void tcp_init_congestion_control (struct sock *sk);
@@ -990,9 +988,9 @@ void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w, u32 acked);u32 tcp_reno_ssthresh (struct sock *sk);u32 tcp_reno_undo_cwnd (struct sock *sk);void tcp_reno_cong_avoid (struct sock *sk,u32 ack,u32 acked); -extern struct tcp_congestion_ops tcp_reno; +extern const struct tcp_congestion_ops tcp_reno; -struct tcp_congestion_ops *tcp_ca_find_key (u32 key); +const struct tcp_congestion_ops *tcp_ca_find_key (u32 key);u32 tcp_ca_get_key_by_name (const char *name,bool *ecn_ca);#ifdef CONFIG_INET char *tcp_ca_get_name_by_key (u32 key,char *buffer);
net/ipv4/tcp.c +0 -2
@@ -3344,8 +3344,6 @@ int tcp_abort(struct sock *sk, int err) }EXPORT_SYMBOL_GPL (tcp_abort); -extern struct tcp_congestion_ops tcp_reno; -static __initdata unsigned long thash_entries;static int __init set_thash_entries (char *str) {
net/ipv4/tcp_bbr.c +1 -1
@@ -917,7 +917,7 @@ static void bbr_set_state(struct sock *sk, u8 new_state) } } -static struct tcp_congestion_ops tcp_bbr_cong_ops__read_mostly = { +static const struct tcp_congestion_ops tcp_bbr_cong_ops = { .flags =TCP_CONG_NON_RESTRICTED , .name ="bbr" , .owner =THIS_MODULE ,
net/ipv4/tcp_bic.c +1 -1
@@ -209,7 +209,7 @@ static void bictcp_acked(struct sock *sk, const struct ack_sample *sample) } } -static struct tcp_congestion_ops bictcp__read_mostly = { +static const struct tcp_congestion_ops bictcp = { .init = bictcp_init, .ssthresh = bictcp_recalc_ssthresh, .cong_avoid = bictcp_cong_avoid,
net/ipv4/tcp_cdg.c +1 -1
@@ -399,7 +399,7 @@ static void tcp_cdg_release(struct sock *sk)kfree (ca->gradients); } -struct tcp_congestion_ops tcp_cdg__read_mostly = { +static const struct tcp_congestion_ops tcp_cdg = { .cong_avoid = tcp_cdg_cong_avoid, .cwnd_event = tcp_cdg_cwnd_event, .pkts_acked = tcp_cdg_acked,
net/ipv4/tcp_cong.c +102 -60
345 lines not on this page · raw message
net/ipv4/tcp_cubic.c +1 -1
@@ -465,7 +465,7 @@ static void bictcp_acked(struct sock *sk, const struct ack_sample *sample)hystart_update (sk, delay); } -static struct tcp_congestion_ops cubictcp__read_mostly = { +static const struct tcp_congestion_ops cubictcp = { .init = bictcp_init, .ssthresh = bictcp_recalc_ssthresh, .cong_avoid = bictcp_cong_avoid,
net/ipv4/tcp_dctcp.c +3 -3
@@ -72,7 +72,7 @@ module_param(dctcp_clamp_alpha_on_loss, uint, 0644);MODULE_PARM_DESC (dctcp_clamp_alpha_on_loss,"parameter for clamping alpha on loss" ); -static struct tcp_congestion_ops dctcp_reno; +static const struct tcp_congestion_ops dctcp_reno;static void dctcp_reset (const struct tcp_sock *tp,struct dctcp *ca) {
@@ -318,7 +318,7 @@ static u32 dctcp_cwnd_undo(struct sock *sk)return max (tcp_sk (sk)->snd_cwnd, ca->loss_cwnd); } -static struct tcp_congestion_ops dctcp__read_mostly = { +static const struct tcp_congestion_ops dctcp = { .init = dctcp_init, .in_ack_event = dctcp_update_alpha, .cwnd_event = dctcp_cwnd_event,
@@ -332,7 +332,7 @@ static struct tcp_congestion_ops dctcp __read_mostly = { .name = "dctcp", }; -static struct tcp_congestion_ops dctcp_reno__read_mostly = { +static const struct tcp_congestion_ops dctcp_reno = { .ssthresh = tcp_reno_ssthresh, .cong_avoid = tcp_reno_cong_avoid, .undo_cwnd = tcp_reno_undo_cwnd,
net/ipv4/tcp_highspeed.c +1 -1
@@ -165,7 +165,7 @@ static u32 hstcp_cwnd_undo(struct sock *sk)return max (tcp_sk (sk)->snd_cwnd, ca->loss_cwnd); } -static struct tcp_congestion_ops tcp_highspeed__read_mostly = { +static const struct tcp_congestion_ops tcp_highspeed = { .init = hstcp_init, .ssthresh = hstcp_ssthresh, .undo_cwnd = hstcp_cwnd_undo,
net/ipv4/tcp_htcp.c +1 -1
@@ -286,7 +286,7 @@ static void htcp_state(struct sock *sk, u8 new_state) } } -static struct tcp_congestion_ops htcp__read_mostly = { +static const struct tcp_congestion_ops htcp = { .init = htcp_init, .ssthresh = htcp_recalc_ssthresh, .cong_avoid = htcp_cong_avoid,
net/ipv4/tcp_hybla.c +1 -1
@@ -163,7 +163,7 @@ static void hybla_cong_avoid(struct sock *sk, u32 ack, u32 acked) tp->snd_cwnd =min_t (u32, tp->snd_cwnd, tp->snd_cwnd_clamp); } -static struct tcp_congestion_ops tcp_hybla__read_mostly = { +static const struct tcp_congestion_ops tcp_hybla = { .init = hybla_init, .ssthresh = tcp_reno_ssthresh, .undo_cwnd = tcp_reno_undo_cwnd,
net/ipv4/tcp_illinois.c +1 -1
@@ -333,7 +333,7 @@ static size_t tcp_illinois_info(struct sock *sk, u32 ext, int *attr,return 0 ; } -static struct tcp_congestion_ops tcp_illinois__read_mostly = { +static const struct tcp_congestion_ops tcp_illinois = { .init = tcp_illinois_init, .ssthresh = tcp_illinois_ssthresh, .undo_cwnd = tcp_illinois_cwnd_undo,
net/ipv4/tcp_lp.c +1 -1
@@ -316,7 +316,7 @@ static void tcp_lp_pkts_acked(struct sock *sk, const struct ack_sample *sample) lp->last_drop = now; } -static struct tcp_congestion_ops tcp_lp__read_mostly = { +static const struct tcp_congestion_ops tcp_lp = { .init = tcp_lp_init, .ssthresh = tcp_reno_ssthresh, .undo_cwnd = tcp_reno_undo_cwnd,
net/ipv4/tcp_nv.c +1 -1
@@ -441,7 +441,7 @@ static size_t tcpnv_get_info(struct sock *sk, u32 ext, int *attr,return 0 ; } -static struct tcp_congestion_ops tcpnv__read_mostly = { +static const struct tcp_congestion_ops tcpnv = { .init = tcpnv_init, .ssthresh = tcpnv_recalc_ssthresh, .cong_avoid = tcpnv_cong_avoid,
net/ipv4/tcp_scalable.c +1 -1
@@ -50,7 +50,7 @@ static u32 tcp_scalable_cwnd_undo(struct sock *sk)return max (tcp_sk (sk)->snd_cwnd, ca->loss_cwnd); } -static struct tcp_congestion_ops tcp_scalable__read_mostly = { +static const struct tcp_congestion_ops tcp_scalable = { .ssthresh = tcp_scalable_ssthresh, .undo_cwnd = tcp_scalable_cwnd_undo, .cong_avoid = tcp_scalable_cong_avoid,
net/ipv4/tcp_vegas.c +1 -1
@@ -304,7 +304,7 @@ size_t tcp_vegas_get_info(struct sock *sk, u32 ext, int *attr, }EXPORT_SYMBOL_GPL (tcp_vegas_get_info); -static struct tcp_congestion_ops tcp_vegas__read_mostly = { +static const struct tcp_congestion_ops tcp_vegas = { .init = tcp_vegas_init, .ssthresh = tcp_reno_ssthresh, .undo_cwnd = tcp_reno_undo_cwnd,
net/ipv4/tcp_veno.c +1 -1
@@ -210,7 +210,7 @@ static u32 tcp_veno_cwnd_undo(struct sock *sk)return max (tcp_sk (sk)->snd_cwnd, veno->loss_cwnd); } -static struct tcp_congestion_ops tcp_veno__read_mostly = { +static const struct tcp_congestion_ops tcp_veno = { .init = tcp_veno_init, .ssthresh = tcp_veno_ssthresh, .undo_cwnd = tcp_veno_cwnd_undo,
net/ipv4/tcp_westwood.c +1 -1
@@ -251,7 +251,7 @@ static size_t tcp_westwood_info(struct sock *sk, u32 ext, int *attr,return 0 ; } -static struct tcp_congestion_ops tcp_westwood__read_mostly = { +static const struct tcp_congestion_ops tcp_westwood = { .init = tcp_westwood_init, .ssthresh = tcp_reno_ssthresh, .cong_avoid = tcp_reno_cong_avoid,
net/ipv4/tcp_yeah.c +1 -1
@@ -232,7 +232,7 @@ static u32 tcp_yeah_cwnd_undo(struct sock *sk)return max (tcp_sk (sk)->snd_cwnd, yeah->loss_cwnd); } -static struct tcp_congestion_ops tcp_yeah__read_mostly = { +static const struct tcp_congestion_ops tcp_yeah = { .init = tcp_yeah_init, .ssthresh = tcp_yeah_ssthresh, .undo_cwnd = tcp_yeah_cwnd_undo,
Signature · 1 line
2.11.0
Re: [RFC 1/1] constify tcp congestion
Quoted 3 lines · Stephen Hemminger
Split the TCP congestion ops structure into const and mutable portions. Put the list pointers, key and a copy of the flags in new
tcp_congestion_entrystructure.
...
Quoted 6 lines
net/ipv4/tcp_cong.c +3
-1-void tcp_unregister_congestion_control (struct tcp_congestion_ops *ca) +void tcp_unregister_congestion_control (const struct tcp_congestion_ops *ca) { +struct tcp_congestion_entry *e; +
net/ipv4/tcp_cong.c EXPORT_SYMBOL_GPL(tcp_register_congestion_control); · line 130
Quoted 10 lines
net/ipv4/tcp_cong.c +7
-1spin_lock (&tcp_cong_list_lock); -list_del_rcu (&ca->list); +list_for_each_entry_rcu (e, &tcp_cong_list, list) { +if (e->ops == ca) { +list_del_rcu (&e->list); +kfree_rcu (e, rcu); +break ; + } + }spin_unlock (&tcp_cong_list_lock);
Since you switched to kfree_rcu(), you could remove the synchronize_rcu() from tcp_unregister_congestion_control()
Otherwise, no need for kfree_rcu() and the rcu head added in struct tcp_congestion_entry :
- You could do a kfree() after the synchronize_rcu().
Also, I am not convinced this would be a good move, since you added an extra structure which is not const anyway.
Re: [RFC 1/1] constify tcp congestion
Quoted 2 lines · Eric Dumazet
Also, I am not convinced this would be a good move, since you added an extra structure which is not const anyway.
Actually, this extra structure might be needed if we want to move the tcp congestion sysctls to net namespace.
Re: [RFC 1/1] constify tcp congestion
Quoted 10 lines · On Wed, 02 Aug 2017 01:13:09 -0700 Eric Dumazet <[email protected]> wrote:
Quoted 2 lines · Eric Dumazet
Also, I am not convinced this would be a good move, since you added an extra structure which is not const anyway.
Actually, this extra structure might be needed if we want to move the tcp congestion sysctls to net namespace.
I was thinking about this. It should be possible to have different list of congestion controls for each net namespace. The list would be inherited, but each namespace could change:
* default CC
* available CC
* non-restricted CC
Quoted 3 lines · Stephen Hemminger
I wonder if restricting congestion control choices is still necessary? It seems like being overly paranoid, and better enforced by having a more limited kernel config, seccomp or other mechanism.
Agree, I think it can be removed.