TCPMSS is not restricted to mangle table

TCPMSS is not restricted to mangle table

[Patrick McHardy]

I just noticed the TCPMSS target is not restricted to the
mangle table. Any opinions about whether we should change
this, perhaps with a warning period?

Re: TCPMSS is not restricted to mangle table

[Herve Eychenne]

On Mon, Dec 05, 2005 at 01:27:09AM +0100, Patrick McHardy wrote:

> I just noticed the TCPMSS target is not restricted to the
> mangle table. Any opinions about whether we should change
> this, perhaps with a warning period?

See the manpage itself... I just copy-pasted the kernel config description
(probably written by Marc Boucher?) when adding TCPMSS to the manpage
some years ago.

So look for TCPMSS, notice the given example:
       iptables -A FORWARD -p tcp --tcp-flags SYN,RST SYN \
                    -j TCPMSS --clamp-mss-to-pmtu

and realize that most uses of TCPMSS (which I fear are not that
rare) probably occur within the filter table.
You can expect a global change to be quite difficult, I guess... :-(

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Re: TCPMSS is not restricted to mangle table

[Patrick McHardy]

Herve Eychenne wrote:
> On Mon, Dec 05, 2005 at 01:27:09AM +0100, Patrick McHardy wrote:
> 
> 
>>I just noticed the TCPMSS target is not restricted to the
>>mangle table. Any opinions about whether we should change
>>this, perhaps with a warning period?
> 
> 
> See the manpage itself... I just copy-pasted the kernel config description
> (probably written by Marc Boucher?) when adding TCPMSS to the manpage
> some years ago.
> 
> So look for TCPMSS, notice the given example:
>        iptables -A FORWARD -p tcp --tcp-flags SYN,RST SYN \
>                     -j TCPMSS --clamp-mss-to-pmtu
> 
> and realize that most uses of TCPMSS (which I fear are not that
> rare) probably occur within the filter table.
> You can expect a global change to be quite difficult, I guess... :-(

Thanks, I didn't know this, I'm going to change this to refer to
the mangle table. This still leaves the option of a warning, but
want I really wanted to know was whether anyone cares. From a
consistency point of view it should be restricted, for the
functionality it doesn't matter.

Re: TCPMSS is not restricted to mangle table

[Aleksandar Milivojevic]

Patrick McHardy wrote:

> Thanks, I didn't know this, I'm going to change this to refer to
> the mangle table. This still leaves the option of a warning, but
> want I really wanted to know was whether anyone cares. From a
> consistency point of view it should be restricted, for the
> functionality it doesn't matter.

 From consumer (of your code) point of view, I do care.  The current 
documentation was clearly encouraging (by example) use of TCPMSS from 
filter table.  My guess is that majority of production systems using 
TCPMSS target are using it from filter table.  If the only reason is 
consistency (nothing is going to be fixed by the change, and nothing is 
going to be broken by leaving it as is), a warning now (in manual page, 
right next to the example) and change on next major kernel release (2.7) 
might be the best approach.  I'd leave things as is for 2.6 series of 
kernels.

Just my 2 cents.

Re: TCPMSS is not restricted to mangle table

[Patrick McHardy]

Aleksandar Milivojevic wrote:
> Patrick McHardy wrote:
> 
>> Thanks, I didn't know this, I'm going to change this to refer to
>> the mangle table. This still leaves the option of a warning, but
>> want I really wanted to know was whether anyone cares. From a
>> consistency point of view it should be restricted, for the
>> functionality it doesn't matter.
> 
> 
>  From consumer (of your code) point of view, I do care.  The current 
> documentation was clearly encouraging (by example) use of TCPMSS from 
> filter table.  My guess is that majority of production systems using 
> TCPMSS target are using it from filter table.  If the only reason is 
> consistency (nothing is going to be fixed by the change, and nothing is 
> going to be broken by leaving it as is), a warning now (in manual page, 
> right next to the example) and change on next major kernel release (2.7) 
> might be the best approach.  I'd leave things as is for 2.6 series of 
> kernels.

I agree. Just to make it clear, I do not intend to break it for no
good reason. I was just surprised that when trying to unload it
after flushing the mangle table there was still one reference left
(from the debian ppp ip-up script). There is one potential reason
to change it, packet classification algorithms like nf-hipac have
an easier job if they can rely on certain conditions, like no
changing of the packet in the filter table. But for now I've only
updated the man-page.

Re: TCPMSS is not restricted to mangle table

[Jan Engelhardt]

>> From consumer (of your code) point of view, I do care.  The current
>> documentation was clearly encouraging (by example) use of TCPMSS from
>> filter table.  My guess is that majority of production systems using
>> TCPMSS target are using it from filter table.  If the only reason is
>> consistency (nothing is going to be fixed by the change, and nothing is
>> going to be broken by leaving it as is), a warning now (in manual page,
>> right next to the example) and change on next major kernel release (2.7)
>> might be the best approach.  I'd leave things as is for 2.6 series of
>> kernels.
>
> I agree. Just to make it clear, I do not intend to break it for no
> good reason. I was just surprised that when trying to unload it
> after flushing the mangle table there was still one reference left
> (from the debian ppp ip-up script). There is one potential reason
> to change it, packet classification algorithms like nf-hipac have
> an easier job if they can rely on certain conditions, like no
> changing of the packet in the filter table. But for now I've only
> updated the man-page.

Since the Linux kernel does not follow the old stable<->devel 
(2.even<->2.uneven) model, every new version is allowed to
break an older one.

On the way to restrict TCPMSS to -t mangle, the -m state
could also be obsoleted in favor of -m conntrack.




Jan Engelhardt
-- 

Re: TCPMSS is not restricted to mangle table

[Patrick McHardy]

Jan Engelhardt wrote:
> Since the Linux kernel does not follow the old stable<->devel 
> (2.even<->2.uneven) model, every new version is allowed to
> break an older one.

Thats complete nonsense.

> On the way to restrict TCPMSS to -t mangle, the -m state
> could also be obsoleted in favor of -m conntrack.

I'm not going to restrict TCPMSS to mangle - but I'm
considering adding a warning. I also don't see a reason
to deprecate the state match, its totally trivial code
and needs a lot less memory than the conntrack match.

Re: TCPMSS is not restricted to mangle table

[Jan Engelhardt]

>> On the way to restrict TCPMSS to -t mangle, the -m state
>> could also be obsoleted in favor of -m conntrack.
>
> I'm not going to restrict TCPMSS to mangle - but I'm
> considering adding a warning. I also don't see a reason
> to deprecate the state match, its totally trivial code
> and needs a lot less memory than the conntrack match.
>
But it's duplicate code, in terms of functionality.

Re: TCPMSS is not restricted to mangle table

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Tue, Jan 03, 2006 at 12:38:54PM +0100, Jan Engelhardt wrote:
> 
> >> On the way to restrict TCPMSS to -t mangle, the -m state
> >> could also be obsoleted in favor of -m conntrack.
> >
> > I'm not going to restrict TCPMSS to mangle - but I'm
> > considering adding a warning. I also don't see a reason
> > to deprecate the state match, its totally trivial code
> > and needs a lot less memory than the conntrack match.
> >
> But it's duplicate code, in terms of functionality.

if you're really so worried about the duplicate code, you could create a
single module (with a MODULE_ALIAS) that registers two matches, one
called "state", the other called "conntrack".

That sounds like the cleanest possible solution without breaking anything.

btw: if you seriously consider implementing my suggestion, please use
the "x_tables" local branch of my git tree - othrewise your patch will
clash with x_tables that is to be submitted any day now.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: TCPMSS is not restricted to mangle table

[Tom Eastep]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Sunday 04 December 2005 16:27, Patrick McHardy wrote:
> I just noticed the TCPMSS target is not restricted to the
> mangle table. Any opinions about whether we should change
> this, perhaps with a warning period?

You are aware that the iptables man page includes an example of the TCPMSS 
target being used in the filter table? 

-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net
PGP Public Key   \ https://lists.shorewall.net/teastep.pgp.key

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: TCPMSS is not restricted to mangle table

[Philip Craig]

On 12/05/2005 10:27 AM, Patrick McHardy wrote:
> I just noticed the TCPMSS target is not restricted to the
> mangle table. Any opinions about whether we should change
> this, perhaps with a warning period?

The historical reason for this is that originally the mangle table
only had PREROUTING and OUTPUT hooks, and so you couldn't clamp
forwarded packets in the mangle table (since it needs the outgoing
interface).

The majority of existing scripts and documentation still reference
the filter table.  So changing this will affect most of its users.

So I think update the official documentation for consistency, but
leave the code how it is.