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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 11 Jul 2020 00:47:01 +0900 From: Kuniyuki Iwashima <kuniyu@...zon.co.jp> To: <edumazet@...gle.com> CC: <benh@...zon.com>, <davem@...emloft.net>, <ja@....bg>, <kuba@...nel.org>, <kuni1840@...il.com>, <kuniyu@...zon.co.jp>, <kuznet@....inr.ac.ru>, <netdev@...r.kernel.org>, <osa-contribution-log@...zon.com>, <yoshfuji@...ux-ipv6.org> Subject: Re: [PATCH v3 net-next] inet: Remove an unnecessary argument of syn_ack_recalc(). From: Eric Dumazet <edumazet@...gle.com> Date: Fri, 10 Jul 2020 08:20:03 -0700 > On Fri, Jul 10, 2020 at 7:11 AM Kuniyuki Iwashima <kuniyu@...zon.co.jp> wrote: > > > > Commit 0c3d79bce48034018e840468ac5a642894a521a3 ("tcp: reduce SYN-ACK > > retrans for TCP_DEFER_ACCEPT") introduces syn_ack_recalc() which decides > > if a minisock is held and a SYN+ACK is retransmitted or not. > > > > If rskq_defer_accept is not zero in syn_ack_recalc(), max_retries always > > has the same value because max_retries is overwritten by rskq_defer_accept > > in reqsk_timer_handler(). > > > > This commit adds three changes: > > - remove redundant non-zero check for rskq_defer_accept in > > reqsk_timer_handler(). > > - remove max_retries from the arguments of syn_ack_recalc() and use > > rskq_defer_accept instead. > > - rename thresh to max_syn_ack_retries for readability. > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.co.jp> > > Reviewed-by: Benjamin Herrenschmidt <benh@...zon.com> > > CC: Julian Anastasov <ja@....bg> > > --- > > net/ipv4/inet_connection_sock.c | 33 +++++++++++++++------------------ > > 1 file changed, 15 insertions(+), 18 deletions(-) > > > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c > > index afaf582a5aa9..21bc80a3c7cf 100644 > > --- a/net/ipv4/inet_connection_sock.c > > +++ b/net/ipv4/inet_connection_sock.c > > @@ -648,20 +648,23 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk, > > EXPORT_SYMBOL_GPL(inet_csk_route_child_sock); > > > > /* Decide when to expire the request and when to resend SYN-ACK */ > > -static inline void syn_ack_recalc(struct request_sock *req, const int thresh, > > - const int max_retries, > > +static inline void syn_ack_recalc(struct request_sock *req, > > While we are at it, please remove the inline keyword. I will remove 'inline' in next spin. > > + const int max_syn_ack_retries, > > const u8 rskq_defer_accept, > > int *expire, int *resend) > > { > > if (!rskq_defer_accept) { > > - *expire = req->num_timeout >= thresh; > > + *expire = req->num_timeout >= max_syn_ack_retries; > > *resend = 1; > > return; > > } > > - *expire = req->num_timeout >= thresh && > > - (!inet_rsk(req)->acked || req->num_timeout >= max_retries); > > - /* > > - * Do not resend while waiting for data after ACK, > > + /* If a bare ACK has already been dropped, the client is alive, so > > + * do not free the request_sock to drop a bare ACK at most > > + * rskq_defer_accept times and wait for data. > > + */ > > I honestly do not believe this comment is needed. > The bare ack has not been 'dropped' since it had the effect of > validating the 3WHS. > I find it confusing, and not describing the order of the conditions > expressed in the C code. Exactly, thank you for correcting my misunderstanding. I will remove the comment. Best Regards, Kuniyuki
Powered by blists - more mailing lists