lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 9 Sep 2020 23:17:21 -0400 From: Neal Cardwell <ncardwell@...gle.com> To: Martin KaFai Lau <kafai@...com> Cc: David Miller <davem@...emloft.net>, Netdev <netdev@...r.kernel.org>, Yuchung Cheng <ycheng@...gle.com>, Kevin Yang <yyd@...gle.com>, Eric Dumazet <edumazet@...gle.com>, Lawrence Brakmo <brakmo@...com> Subject: Re: [PATCH net-next 3/4] tcp: simplify tcp_set_congestion_control(): always reinitialize On Wed, Sep 9, 2020 at 8:29 PM Martin KaFai Lau <kafai@...com> wrote: > > On Wed, Sep 09, 2020 at 02:15:55PM -0400, Neal Cardwell wrote: > > Now that the previous patches ensure that all call sites for > > tcp_set_congestion_control() want to initialize congestion control, we > > can simplify tcp_set_congestion_control() by removing the reinit > > argument and the code to support it. > > > > Signed-off-by: Neal Cardwell <ncardwell@...gle.com> > > Acked-by: Yuchung Cheng <ycheng@...gle.com> > > Acked-by: Kevin Yang <yyd@...gle.com> > > Signed-off-by: Eric Dumazet <edumazet@...gle.com> > > Cc: Lawrence Brakmo <brakmo@...com> > > --- > > include/net/tcp.h | 2 +- > > net/core/filter.c | 3 +-- > > net/ipv4/tcp.c | 2 +- > > net/ipv4/tcp_cong.c | 11 ++--------- > > 4 files changed, 5 insertions(+), 13 deletions(-) > > > > diff --git a/include/net/tcp.h b/include/net/tcp.h > > index e85d564446c6..f857146c17a5 100644 > > --- a/include/net/tcp.h > > +++ b/include/net/tcp.h > > @@ -1104,7 +1104,7 @@ void tcp_get_available_congestion_control(char *buf, size_t len); > > void tcp_get_allowed_congestion_control(char *buf, size_t len); > > int tcp_set_allowed_congestion_control(char *allowed); > > int tcp_set_congestion_control(struct sock *sk, const char *name, bool load, > > - bool reinit, bool cap_net_admin); > > + bool cap_net_admin); > > u32 tcp_slow_start(struct tcp_sock *tp, u32 acked); > > void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w, u32 acked); > > > > diff --git a/net/core/filter.c b/net/core/filter.c > > index b26c04924fa3..0bd0a97ee951 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -4451,8 +4451,7 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname, > > strncpy(name, optval, min_t(long, optlen, > > TCP_CA_NAME_MAX-1)); > > name[TCP_CA_NAME_MAX-1] = 0; > > - ret = tcp_set_congestion_control(sk, name, false, > > - true, true); > > + ret = tcp_set_congestion_control(sk, name, false, true); > > } else { > > struct inet_connection_sock *icsk = inet_csk(sk); > > struct tcp_sock *tp = tcp_sk(sk); > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > > index 7360d3db2b61..e58ab9db73ff 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@ -3050,7 +3050,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level, int optname, > > name[val] = 0; > > > > lock_sock(sk); > > - err = tcp_set_congestion_control(sk, name, true, true, > > + err = tcp_set_congestion_control(sk, name, true, > > ns_capable(sock_net(sk)->user_ns, > > CAP_NET_ADMIN)); > > release_sock(sk); > > diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c > > index d18d7a1ce4ce..a9b0fb52a1ec 100644 > > --- a/net/ipv4/tcp_cong.c > > +++ b/net/ipv4/tcp_cong.c > > @@ -341,7 +341,7 @@ int tcp_set_allowed_congestion_control(char *val) > > * already initialized. > > */ > > int tcp_set_congestion_control(struct sock *sk, const char *name, bool load, > > - bool reinit, bool cap_net_admin) > > + bool cap_net_admin) > > { > > struct inet_connection_sock *icsk = inet_csk(sk); > > const struct tcp_congestion_ops *ca; > > @@ -365,15 +365,8 @@ int tcp_set_congestion_control(struct sock *sk, const char *name, bool load, > > if (!ca) { > > err = -ENOENT; > > } else if (!load) { > nit. > > I think this "else if (!load)" case can be completely removed and simply > allow it to fall through to the last > "else { tcp_reinit_congestion_control(sk, ca); }" . Thanks, Martin. That is a very nice observation, and I think you are right that we can make the additional refactor/simplification/clean-up that you mention, without changing behavior. For clarity I think that would be nice to have as a separate follow-on commit. Are you interested in posting a follow-on patch for that, since it's your nice idea? thanks, neal
Powered by blists - more mailing lists