在 2024/6/15 14:40, Eric Dumazet 写道:
On Sat, Jun 15, 2024 at 12:24 AM Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote:
你好 Eric和Kuniyuk,
From: luoxuanqiang <luoxuanqiang@xxxxxxxxxx>
Date: Fri, 14 Jun 2024 20:42:07 +0800
在 2024/6/14 18:54, Florian Westphal 写道:
luoxuanqiang <luoxuanqiang@xxxxxxxxxx> wrote:
include/net/inet_connection_sock.h | 2 +-
net/dccp/ipv4.c | 2 +-
net/dccp/ipv6.c | 2 +-
net/ipv4/inet_connection_sock.c | 15 +++++++++++----
net/ipv4/tcp_input.c | 11 ++++++++++-
5 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 7d6b1254c92d..8773d161d184 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -264,7 +264,7 @@ struct sock *inet_csk_reqsk_queue_add(struct sock *sk,
struct request_sock *req,
struct sock *child);
void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
- unsigned long timeout);
+ unsigned long timeout, bool *found_dup_sk);
Nit:
I think it would be preferrable to change retval to bool rather than
bool *found_dup_sk extra arg, so one can do
+1
bool inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
unsigned long timeout)
{
if (!reqsk_queue_hash_req(req, timeout))
return false;
i.e. let retval indicate wheter reqsk was inserted or not.
Patch looks good to me otherwise.
Thank you for your confirmation!
Regarding your suggestion, I had considered it before,
but besides tcp_conn_request() calling inet_csk_reqsk_queue_hash_add(),
dccp_v4(v6)_conn_request() also calls it. However, there is no
consideration for a failed insertion within that function, so it's
reasonable to let the caller decide whether to check for duplicate
reqsk.
I guess you followed 01770a1661657 where found_dup_sk was introduced,
but note that the commit is specific to TCP SYN Cookie and TCP Fast Open
and DCCP is not related.
Then, own_req is common to TCP and DCCP, so found_dup_sk was added as an
additional argument.
However, another similar commit 5e0724d027f05 actually added own_req check
in DCCP path.
I personally would'nt care if DCCP was not changed to handle such a
failure because DCCP will be removed next year, but I still prefer
Florian's suggestion.
Other things to consider :
- I presume this patch targets net tree, and luoxuanqiang needs the
fix to reach stable trees.
- This means a Fixes: tag is needed
- This also means that we should favor a patch with no or trivial
conflicts for stable backports.
Should the patch target the net-next tree, then the requirements can
be different.
Hi Kuniyuk and Florian,
I've created version 3 based on your suggestions, but I've kept the use
of 'found_dup_sk' since we need to pass NULL in DCCP to maintain its
logic unchanged. Could you please review this update and let me know if
it's okay? Thank you!
BRs!
