Bug 144000
| Summary: | [tcp] setting TCP_MAXSEG by setsockopt() does not seem to have any effect | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Andrey Zonov <andrey.zonov> | ||||||||
| Component: | kern | Assignee: | Gleb Smirnoff <glebius> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | CC: | hiren, me | ||||||||
| Priority: | Normal | ||||||||||
| Version: | 7.2-STABLE | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Andrey Zonov
2010-02-16 12:20:02 UTC
Responsible Changed From-To: freebsd-bugs->freebsd-net Over to maintainer(s). I have found patch at [1] and adapted for 8.0-p1 jhb, why you not added this patch in HEAD? [1] http://people.freebsd.org/~jhb/patches/tcp_maxseg.patch -- Andrey Zonov On Thursday 25 February 2010 3:49:00 am Andrey Zonov wrote:
> I have found patch at [1] and adapted for 8.0-p1
>
> jhb, why you not added this patch in HEAD?
>
>
> [1] http://people.freebsd.org/~jhb/patches/tcp_maxseg.patch
Actually, can you try this simpler patch instead:
Index: tcp_input.c
===================================================================
--- tcp_input.c (revision 205624)
+++ tcp_input.c (working copy)
@@ -3100,12 +3100,10 @@
#ifdef INET6
if (isipv6) {
maxmtu = tcp_maxmtu6(&inp->inp_inc, mtuflags);
- tp->t_maxopd = tp->t_maxseg = V_tcp_v6mssdflt;
} else
#endif
{
maxmtu = tcp_maxmtu(&inp->inp_inc, mtuflags);
- tp->t_maxopd = tp->t_maxseg = V_tcp_mssdflt;
}
/*
--
John Baldwin
For 7/8-stable not helped me. John Baldwin ?????: > On Thursday 25 February 2010 3:49:00 am Andrey Zonov wrote: > >> I have found patch at [1] and adapted for 8.0-p1 >> >> jhb, why you not added this patch in HEAD? >> >> >> [1] http://people.freebsd.org/~jhb/patches/tcp_maxseg.patch >> > > Actually, can you try this simpler patch instead: > > Index: tcp_input.c > =================================================================== > --- tcp_input.c (revision 205624) > +++ tcp_input.c (working copy) > @@ -3100,12 +3100,10 @@ > #ifdef INET6 > if (isipv6) { > maxmtu = tcp_maxmtu6(&inp->inp_inc, mtuflags); > - tp->t_maxopd = tp->t_maxseg = V_tcp_v6mssdflt; > } else > #endif > { > maxmtu = tcp_maxmtu(&inp->inp_inc, mtuflags); > - tp->t_maxopd = tp->t_maxseg = V_tcp_mssdflt; > } > > /* > > -- Andrey Zonov Responsible Changed From-To: freebsd-net->andre Take over. Andrey please test the attached patch. It allows the MSS to be set either before or after a connection has been established. It does not allow the mss to be set on listen sockets and their values are not inherited on inbound connections. This is my understanding form the Linux man page. -- Andre State Changed From-To: open->analyzed Hi, This patch works as you say, only for outbound connections and it's bad... I don't know which man you read, but in ubuntu-10.04 I can set MSS for both of types connections inbound and outbound. This patch also may not set MSS more than net.inet.tcp.mssdflt and less than net.inet.tcp.minmss. 2010/8/15 Andre Oppermann <andre@freebsd.org>: > Andrey > > please test the attached patch. =A0It allows the MSS to be set > either before or after a connection has been established. It > does not allow the mss to be set on listen sockets and their > values are not inherited on inbound connections. =A0This is my > understanding form the Linux man page. > > -- > Andre > --=20 Andrey Zonov Hi Andre, Is there any chance to get a real fix for this problem or you're not interested in this? -- Andrey Zonov Hello, some thoughts on this PR follows. First, we've got two fields in the tcpcb representing almost same: the t_maxseg and t_maxopd. It seems to me that a constant equation is (almost) always true: tp->t_maxseg = t_maxopd - delta , where delta is constant for given tp. Looking into the past, I've found that t_maxopd was introduced with T/TCP. Before that the TCP stack worked fine without t_maxopd, although there already were TCP options in the stack, thus t_maxseg wasn't equal to (mtu - min_proto_header). Later, when T/TCP was removed from the stack, t_maxopd left here, since was used widely. Second, looking at patches from Andre for kern/144000 and the one by Peter to YBSD, I like the latter. But it requires growing of struct tcpcb. However, if we transform all the code that uses t_maxopd to use t_maxseg, then we've got a spare field for t_usermss. Your opinion? -- Totus tuus, Glebius. (In reply to Gleb Smirnoff from comment #10) Stumbled upon this while looking for something else but I also think YBSD patch from Peter is good and if we can, we should remove t_maxopd which will make room for t_usermss. Gleb: assigning this to you for now. If you don't want it, feel free to punt it back to freebsd-net. Now that t_maxopd is gone with r293284, we should update Peter's patch from yahoo and fix this. I guess that after r293284 the bug can be closed. The problem before r293284 was that TCP_MAXSEG socketoption was working with t_maxseg, while rest of the stack used t_maxopd. I'm pretty sure that now TCP_MAXSEG works right as it is documented in tcp(4). I think this is still now fixed. I tried a quick C program with setsockopt(listenfd, IPPROTO_TCP, TCP_MAXSEG, 1100, 4); where call succeeded but mss is still the default (1460). The "listenfd" says that you are expecting inheritance of TCP_MAXSEG. AFAIU, the PR is not about that. AFAIU, the PR says that an established connection would ignore TCP_MAXSEG. And I'm pretty sure it is now fixed in head. Inheritance of the MAXSEG is a separate issue to be discussed. Linux supports changing MSS with setsockopt before and after establishing the connections. FreeBSD only support that after connection is established. (I made sure that part works.) The patch from Peter would let us set in listen state also. Gleb suggested that we may be able to do this without adding extra variable to tcpcb. (Noting here for posterity so whoever wants to fix can pick it up) We may probably want to make that very clear in our tcp(4) manpage that currently you can only change MSS after connection is established. Hiren, let's make update on this bug. First, I think that original bug is fixed. Second, inheritance of t_maxseg from a listening socket is a separate issue, that should be looked at from scratch. That's why I want to close this bug. Third, now we have no constraints on modifying struct tcpcb. Do you mind if I close it? Sure, go ahead. |
