From: William Allen Simpson on 19 Jan 2010 10:50 Simon Arlott wrote: > On Tue, January 19, 2010 09:17, William Allen Simpson wrote: >> 2) There certainly *can* be data on SYN. That code is already in >> 2.6.33.... > > I could change the comment too, but the same logic applies when > there is data and no MSS option - the packet can't be increased > in size if it would then exceed 576 bytes and/or the destination > MTU. > Please change the comment. If there is no MSS option, it should *not* be added, under *ANY* circumstances. That violates the end-to-end arguments (some call them principles). MSS isn't about the _destination_ MTU, it's about the *source*. If you cannot guarantee you know the source MTU, there's no basis for deciding the MSS. While I understand that sometimes it's useful to reduce (never, NEVER, *NEVER* increase) the MSS as a packet goes into a tunnel (because there are problems in some NAT'd networks with determining Path MTU via ICMP), I'm not aware of any circumstance where the MSS would need to be reduced below 536. I'm having some difficulty figuring out how this code originated -- with a nice log entry explaining the exact manufacturer's device and network topology that the contributor had in mind? > If it's possible to know that the packet can have an additional > option added without exceeding MTU then this could be changed. > The data part would need to be moved to make space at the end of > the header. > No options should be added to TCP in a router -- ever! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Simon Arlott on 20 Jan 2010 08:00 On Tue, January 19, 2010 15:44, William Allen Simpson wrote: > Simon Arlott wrote: >> On Tue, January 19, 2010 09:17, William Allen Simpson wrote: >> I could change the comment too, but the same logic applies when >> there is data and no MSS option - the packet can't be increased >> in size if it would then exceed 576 bytes and/or the destination >> MTU. >> > Please change the comment. I've made a new version of the patch which I'll be able to test tonight. > If there is no MSS option, it should *not* be added, under *ANY* > circumstances. That violates the end-to-end arguments (some call > them principles). Agreed. The added MSS is likely to be larger than 536 too... I've removed this code. > MSS isn't about the _destination_ MTU, it's about the *source*. > If you cannot guarantee you know the source MTU, there's no basis > for deciding the MSS. I was referring to the SYN packet itself. It wouldn't always be possible to add an option without exceeding the MTU of that packet's destination if it had data. On 19/01/10 12:53, Patrick McHardy wrote: > Simon Arlott wrote: >> If this is 20 (IPv4 Header) + 20 (TCP Header) = 40 bytes, then >> there is no data and the header offset is wrong so it hasn't been >> checked. > > That's odd. If the packet is really only 40 bytes large, then there > are no TCP options, so your patch shouldn't have any effect. Except to remove the printk which fills up my serial console (because the header offset is wrong). -- Simon Arlott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jan Engelhardt on 20 Jan 2010 16:40 On Wednesday 2010-01-20 22:21, Simon Arlott wrote: >The TCPMSS target is dropping SYN packets where: > 1) There is data, or > 2) The data offset makes the TCP header larger than > the packet. > >Both of these result in an error level printk. > >This change fixes the drop of SYN packets with data >(because the MSS option can safely be modified) and >passes packets with no MSS option instead of adding >one (which is not valid). Can you explain why the automatic addition of a MSS option is removed? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jan Engelhardt on 20 Jan 2010 16:50 On Wednesday 2010-01-20 22:39, Jan Engelhardt wrote: >On Wednesday 2010-01-20 22:21, Simon Arlott wrote: > >>The TCPMSS target is dropping SYN packets where: >> 1) There is data, or >> 2) The data offset makes the TCP header larger than >> the packet. >> >>Both of these result in an error level printk. >> >>This change fixes the drop of SYN packets with data >>(because the MSS option can safely be modified) and >>passes packets with no MSS option instead of adding >>one (which is not valid). > >Can you explain why the automatic addition of a MSS option is removed? That is, of course, for the git log. If I followed the thread right, it was that adding the option could exceed the MTU. Well, can't we check for the outgoing MTU? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Simon Arlott on 20 Jan 2010 17:00 On 20/01/10 21:41, Jan Engelhardt wrote: > On Wednesday 2010-01-20 22:39, Jan Engelhardt wrote: > >>On Wednesday 2010-01-20 22:21, Simon Arlott wrote: >> >>>The TCPMSS target is dropping SYN packets where: >>> 1) There is data, or >>> 2) The data offset makes the TCP header larger than >>> the packet. >>> >>>Both of these result in an error level printk. >>> >>>This change fixes the drop of SYN packets with data >>>(because the MSS option can safely be modified) and >>>passes packets with no MSS option instead of adding >>>one (which is not valid). >> >>Can you explain why the automatic addition of a MSS option is removed? > > That is, of course, for the git log. If I followed the thread right, it > was that adding the option could exceed the MTU. Well, can't we check > for the outgoing MTU? The MSS option is for the MRU of whoever sent the SYN packet. There's no way of knowing this information so it's not possible to avoid using an MSS that is too large. With no option, "any" segment size could be used, which implies 536 to match the MRU of 576. The other reason for not being able to add it is that it may increase the packet size beyond an MRU/MTU limit if there is data. There's no guarantee we'll see an ICMP error message if this occurs, because the limit doesn't have to be local and the return path does not need to be the same. The original host won't know that the packet is going to be increased in size. -- Simon Arlott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
First
|
Prev
|
Next
|
Last
Pages: 1 2 3 4 Prev: loan at 2% interest rate Next: cmd64x: fix PIO and MWDMA timings calculations |