| Message ID | 20230524083350.54197-1-cambda@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1] tcp: Return user_mss for TCP_MAXSEG in CLOSE/LISTEN state if user_mss set | expand |
> On May 24, 2023, at 16:33, Cambda Zhu <cambda@linux.alibaba.com> wrote: > > This patch replaces the tp->mss_cache check in getting TCP_MAXSEG > with tp->rx_opt.user_mss check for CLOSE/LISTEN sock. Since > tp->mss_cache is initialized with TCP_MSS_DEFAULT, checking if > it's zero is probably a bug. > > With this change, getting TCP_MAXSEG before connecting will return > default MSS normally, and return user_mss if user_mss is set. > > Fixes: 0c409e85f0ac ("Import 2.3.41pre2") > Reported-by: Jack Yang <mingliang@linux.alibaba.com> > Suggested-by: Eric Dumazet <edumazet@google.com> > Link: https://lore.kernel.org/netdev/CANn89i+3kL9pYtkxkwxwNMzvC_w3LNUum_2=3u+UyLBmGmifHA@mail.gmail.com/#t > Signed-off-by: Cambda Zhu <cambda@linux.alibaba.com> > Link: https://lore.kernel.org/netdev/14D45862-36EA-4076-974C-EA67513C92F6@linux.alibaba.com/ > --- > v1: > - Return default MSS if user_mss not set for backwards compatibility. > - Send patch to net instead of net-next, with Fixes tag. > - Add Eric's tags. > --- > net/ipv4/tcp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 4d6392c16b7a..3e01a58724b8 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -4081,7 +4081,8 @@ int do_tcp_getsockopt(struct sock *sk, int level, > switch (optname) { > case TCP_MAXSEG: > val = tp->mss_cache; > - if (!val && ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > + if (tp->rx_opt.user_mss && > + ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > val = tp->rx_opt.user_mss; > if (tp->repair) > val = tp->rx_opt.mss_clamp; > -- > 2.16.6 I see netdev/verify_fixes check failed for the commit 0c409e85f0ac ("Import 2.3.41pre2"). The commit is from: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git Should I remove the Fixes tag? Thanks! Cambda
On Wed, May 24, 2023 at 5:11 PM Cambda Zhu <cambda@linux.alibaba.com> wrote: > > > > On May 24, 2023, at 16:33, Cambda Zhu <cambda@linux.alibaba.com> wrote: > > > > This patch replaces the tp->mss_cache check in getting TCP_MAXSEG > > with tp->rx_opt.user_mss check for CLOSE/LISTEN sock. Since > > tp->mss_cache is initialized with TCP_MSS_DEFAULT, checking if > > it's zero is probably a bug. > > > > With this change, getting TCP_MAXSEG before connecting will return > > default MSS normally, and return user_mss if user_mss is set. > > > > Fixes: 0c409e85f0ac ("Import 2.3.41pre2") > > Reported-by: Jack Yang <mingliang@linux.alibaba.com> > > Suggested-by: Eric Dumazet <edumazet@google.com> > > Link: https://lore.kernel.org/netdev/CANn89i+3kL9pYtkxkwxwNMzvC_w3LNUum_2=3u+UyLBmGmifHA@mail.gmail.com/#t > > Signed-off-by: Cambda Zhu <cambda@linux.alibaba.com> > > Link: https://lore.kernel.org/netdev/14D45862-36EA-4076-974C-EA67513C92F6@linux.alibaba.com/ > > --- > > v1: > > - Return default MSS if user_mss not set for backwards compatibility. > > - Send patch to net instead of net-next, with Fixes tag. > > - Add Eric's tags. > > --- > > net/ipv4/tcp.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > > index 4d6392c16b7a..3e01a58724b8 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@ -4081,7 +4081,8 @@ int do_tcp_getsockopt(struct sock *sk, int level, > > switch (optname) { > > case TCP_MAXSEG: > > val = tp->mss_cache; > > - if (!val && ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > > + if (tp->rx_opt.user_mss && > > + ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > > val = tp->rx_opt.user_mss; > > if (tp->repair) > > val = tp->rx_opt.mss_clamp; > > -- > > 2.16.6 > > I see netdev/verify_fixes check failed for the commit 0c409e85f0ac ("Import 2.3.41pre2"). > The commit is from: > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git > > Should I remove the Fixes tag? Please using the correct Fixes tag as below because we cannot trace it (0c409e85f0ac) in the current tree: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Probably you have to send the v3 version. Let's wait to hear more opinions about this patch. Otherwise it looks good to me. Please feel free to add: Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> Thanks, Jason > > Thanks! > > Cambda
On Wed, May 24, 2023 at 05:10:54PM +0800, Cambda Zhu wrote: > > > On May 24, 2023, at 16:33, Cambda Zhu <cambda@linux.alibaba.com> wrote: > > > > This patch replaces the tp->mss_cache check in getting TCP_MAXSEG > > with tp->rx_opt.user_mss check for CLOSE/LISTEN sock. Since > > tp->mss_cache is initialized with TCP_MSS_DEFAULT, checking if > > it's zero is probably a bug. > > > > With this change, getting TCP_MAXSEG before connecting will return > > default MSS normally, and return user_mss if user_mss is set. > > > > Fixes: 0c409e85f0ac ("Import 2.3.41pre2") > > Reported-by: Jack Yang <mingliang@linux.alibaba.com> > > Suggested-by: Eric Dumazet <edumazet@google.com> > > Link: https://lore.kernel.org/netdev/CANn89i+3kL9pYtkxkwxwNMzvC_w3LNUum_2=3u+UyLBmGmifHA@mail.gmail.com/#t > > Signed-off-by: Cambda Zhu <cambda@linux.alibaba.com> > > Link: https://lore.kernel.org/netdev/14D45862-36EA-4076-974C-EA67513C92F6@linux.alibaba.com/ > > --- > > v1: > > - Return default MSS if user_mss not set for backwards compatibility. > > - Send patch to net instead of net-next, with Fixes tag. > > - Add Eric's tags. > > --- > > net/ipv4/tcp.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > > index 4d6392c16b7a..3e01a58724b8 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@ -4081,7 +4081,8 @@ int do_tcp_getsockopt(struct sock *sk, int level, > > switch (optname) { > > case TCP_MAXSEG: > > val = tp->mss_cache; > > - if (!val && ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > > + if (tp->rx_opt.user_mss && > > + ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) > > val = tp->rx_opt.user_mss; > > if (tp->repair) > > val = tp->rx_opt.mss_clamp; > > -- > > 2.16.6 > > I see netdev/verify_fixes check failed for the commit 0c409e85f0ac ("Import 2.3.41pre2"). > The commit is from: > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git > > Should I remove the Fixes tag? Hi Cambda Zhu, AFAIK, that is a tree of historical versions that predate git history, which began with version v2.6.12-rc2. If this is a fix for a but ghat has been present since before v2.6.12-rc2 then I think standard practice is to use the initial commit of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git That is: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") N.B.: I did not check to see if the bug you are fixing has been present since then. But in any case, the fixes tag in your patch is not correct.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 4d6392c16b7a..3e01a58724b8 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -4081,7 +4081,8 @@ int do_tcp_getsockopt(struct sock *sk, int level, switch (optname) { case TCP_MAXSEG: val = tp->mss_cache; - if (!val && ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) + if (tp->rx_opt.user_mss && + ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) val = tp->rx_opt.user_mss; if (tp->repair) val = tp->rx_opt.mss_clamp;
This patch replaces the tp->mss_cache check in getting TCP_MAXSEG with tp->rx_opt.user_mss check for CLOSE/LISTEN sock. Since tp->mss_cache is initialized with TCP_MSS_DEFAULT, checking if it's zero is probably a bug. With this change, getting TCP_MAXSEG before connecting will return default MSS normally, and return user_mss if user_mss is set. Fixes: 0c409e85f0ac ("Import 2.3.41pre2") Reported-by: Jack Yang <mingliang@linux.alibaba.com> Suggested-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/netdev/CANn89i+3kL9pYtkxkwxwNMzvC_w3LNUum_2=3u+UyLBmGmifHA@mail.gmail.com/#t Signed-off-by: Cambda Zhu <cambda@linux.alibaba.com> Link: https://lore.kernel.org/netdev/14D45862-36EA-4076-974C-EA67513C92F6@linux.alibaba.com/ --- v1: - Return default MSS if user_mss not set for backwards compatibility. - Send patch to net instead of net-next, with Fixes tag. - Add Eric's tags. --- net/ipv4/tcp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)