Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-26 Thread Gleb Smirnoff
On Thu, Oct 25, 2012 at 10:29:51PM +0200, Andre Oppermann wrote:
A On 25.10.2012 18:25, Andrey V. Elsukov wrote:
A  On 25.10.2012 19:54, Andre Oppermann wrote:
A  I still don't agree with naming the sysctl net.pfil.forward.  This
A  type of forwarding is a property of IPv4 and IPv6 and thus should
A  be put there.  Pfil hooking can be on layer 2, 2-bridging, 3 and
A  who knows where else in the future.  Forwarding works only for IPv46.
A 
A  You haven't even replied to my comment on net@.  Please change the
A  sysctl location and name to its appropriate place.
A 
A  Hi Andre,
A 
A  There were two replies related to this subject, you did not replied to
A  them and i thought that you became agree.
A 
A I replied to your reply to mine.  Other than that I didn't find
A anything else from you.
A 
A  So, if not, what you think about the name net.pfil.ipforward?
A 
A net.inet.ip.pfil_forward
A net.inet6.ip6.pfil_forward
A 
A or something like that.
A 
A If you can show with your performance profiling that the sysctl
A isn't even necessary, you could leave it completely away and have
A pfil_forward enabled permanently.  That would be even better for
A everybody.

I'd prefer to have the sysctl. Benchmarking will definitely show
no regression, because in default case packets are tagless. But if
packets would carry 1 or 2 tags each, which don't actually belong
to PACKET_TAG_IPFORWARD, then processing would be pessimized.

-- 
Totus tuus, Glebius.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-26 Thread Andre Oppermann

On 26.10.2012 13:26, Gleb Smirnoff wrote:

On Thu, Oct 25, 2012 at 10:29:51PM +0200, Andre Oppermann wrote:
A On 25.10.2012 18:25, Andrey V. Elsukov wrote:
A  On 25.10.2012 19:54, Andre Oppermann wrote:
A  I still don't agree with naming the sysctl net.pfil.forward.  This
A  type of forwarding is a property of IPv4 and IPv6 and thus should
A  be put there.  Pfil hooking can be on layer 2, 2-bridging, 3 and
A  who knows where else in the future.  Forwarding works only for IPv46.
A 
A  You haven't even replied to my comment on net@.  Please change the
A  sysctl location and name to its appropriate place.
A 
A  Hi Andre,
A 
A  There were two replies related to this subject, you did not replied to
A  them and i thought that you became agree.
A
A I replied to your reply to mine.  Other than that I didn't find
A anything else from you.
A
A  So, if not, what you think about the name net.pfil.ipforward?
A
A net.inet.ip.pfil_forward
A net.inet6.ip6.pfil_forward
A
A or something like that.
A
A If you can show with your performance profiling that the sysctl
A isn't even necessary, you could leave it completely away and have
A pfil_forward enabled permanently.  That would be even better for
A everybody.

I'd prefer to have the sysctl. Benchmarking will definitely show
no regression, because in default case packets are tagless. But if
packets would carry 1 or 2 tags each, which don't actually belong
to PACKET_TAG_IPFORWARD, then processing would be pessimized.


With M_FASTFWD_OURS I used an overlay of the protocol specific M_PROTO[1-5]
mbuf flags.  The same can be done with M_IPFORWARD.  The ipfw code then
will not only add the m_tag but also set M_IPFORWARD flag.  That way no
sysctl is required and the feature is always available.  The overlay
definition is in ip_var.h.

--
Andre

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-26 Thread Andrey V. Elsukov
On 26.10.2012 15:43, Andre Oppermann wrote:
 A If you can show with your performance profiling that the sysctl
 A isn't even necessary, you could leave it completely away and have
 A pfil_forward enabled permanently.  That would be even better for
 A everybody.

 I'd prefer to have the sysctl. Benchmarking will definitely show
 no regression, because in default case packets are tagless. But if
 packets would carry 1 or 2 tags each, which don't actually belong
 to PACKET_TAG_IPFORWARD, then processing would be pessimized.
 
 With M_FASTFWD_OURS I used an overlay of the protocol specific M_PROTO[1-5]
 mbuf flags.  The same can be done with M_IPFORWARD.  The ipfw code then
 will not only add the m_tag but also set M_IPFORWARD flag.  That way no
 sysctl is required and the feature is always available.  The overlay
 definition is in ip_var.h.

It seems we have only one bit in the m_flags that can be used, so, maybe
we left it to some things that can appear in the future?

-- 
WBR, Andrey V. Elsukov
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-26 Thread Andre Oppermann

On 26.10.2012 14:29, Andrey V. Elsukov wrote:

On 26.10.2012 15:43, Andre Oppermann wrote:

A If you can show with your performance profiling that the sysctl
A isn't even necessary, you could leave it completely away and have
A pfil_forward enabled permanently.  That would be even better for
A everybody.

I'd prefer to have the sysctl. Benchmarking will definitely show
no regression, because in default case packets are tagless. But if
packets would carry 1 or 2 tags each, which don't actually belong
to PACKET_TAG_IPFORWARD, then processing would be pessimized.


With M_FASTFWD_OURS I used an overlay of the protocol specific M_PROTO[1-5]
mbuf flags.  The same can be done with M_IPFORWARD.  The ipfw code then
will not only add the m_tag but also set M_IPFORWARD flag.  That way no
sysctl is required and the feature is always available.  The overlay
definition is in ip_var.h.


It seems we have only one bit in the m_flags that can be used, so, maybe
we left it to some things that can appear in the future?


That's what the M_PROTO flags are for:

#define M_IPFW_FORWARD  M_PROTO2/* ip forwarding */

of course you have to do the same for ip6.

The M_PROTO[1-5] flags are only valid within a protocol layer.  For
example they get cleared in ip_output() before the packet is handed
to layer 2.

--
Andre

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-26 Thread Andre Oppermann

On 26.10.2012 15:24, Andre Oppermann wrote:

On 26.10.2012 14:29, Andrey V. Elsukov wrote:

On 26.10.2012 15:43, Andre Oppermann wrote:

A If you can show with your performance profiling that the sysctl
A isn't even necessary, you could leave it completely away and have
A pfil_forward enabled permanently.  That would be even better for
A everybody.

I'd prefer to have the sysctl. Benchmarking will definitely show
no regression, because in default case packets are tagless. But if
packets would carry 1 or 2 tags each, which don't actually belong
to PACKET_TAG_IPFORWARD, then processing would be pessimized.


With M_FASTFWD_OURS I used an overlay of the protocol specific M_PROTO[1-5]
mbuf flags.  The same can be done with M_IPFORWARD.  The ipfw code then
will not only add the m_tag but also set M_IPFORWARD flag.  That way no
sysctl is required and the feature is always available.  The overlay
definition is in ip_var.h.


It seems we have only one bit in the m_flags that can be used, so, maybe
we left it to some things that can appear in the future?


That's what the M_PROTO flags are for:

#defineM_IPFW_FORWARDM_PROTO2/* ip forwarding */


Actually looking at it technically this isn't forwarding but specifying
a different nexthop.  Hence the #define and description should be more
like

#define M_IP_NEXTHOPM_PROTO2/* explicit ip nexthop */

Of course the userspace ipfw feature naming and usage doesn't change.
But within the kernel it's really nexthop manipulation within the
forwarding path.

--
Andre


of course you have to do the same for ip6.

The M_PROTO[1-5] flags are only valid within a protocol layer.  For
example they get cleared in ip_output() before the packet is handed
to layer 2.



___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-25 Thread John Baldwin
On Thursday, October 25, 2012 5:39:15 am Andrey V. Elsukov wrote:
 Author: ae
 Date: Thu Oct 25 09:39:14 2012
 New Revision: 242079
 URL: http://svn.freebsd.org/changeset/base/242079
 
 Log:
   Remove the IPFIREWALL_FORWARD kernel option and make possible to turn
   on the related functionality in the runtime via the sysctl variable
   net.pfil.forward. It is turned off by default.

Certainly for MFC's I think it makes sense to retain the option, but
make the option simply change the default from off to on.  That avoids
breaking existing kernel configurations.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-25 Thread Andrey V. Elsukov
On 25.10.2012 17:28, John Baldwin wrote:
   Remove the IPFIREWALL_FORWARD kernel option and make possible to turn
   on the related functionality in the runtime via the sysctl variable
   net.pfil.forward. It is turned off by default.
 
 Certainly for MFC's I think it makes sense to retain the option, but
 make the option simply change the default from off to on.  That avoids
 breaking existing kernel configurations.

Yes, this is exactly what i plan to do.

-- 
WBR, Andrey V. Elsukov



signature.asc
Description: OpenPGP digital signature


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-25 Thread Andre Oppermann

On 25.10.2012 11:39, Andrey V. Elsukov wrote:

Author: ae
Date: Thu Oct 25 09:39:14 2012
New Revision: 242079
URL: http://svn.freebsd.org/changeset/base/242079

Log:
   Remove the IPFIREWALL_FORWARD kernel option and make possible to turn
   on the related functionality in the runtime via the sysctl variable
   net.pfil.forward. It is turned off by default.

   Sponsored by:Yandex LLC
   Discussed with:  net@
   MFC after:   2 weeks


I still don't agree with naming the sysctl net.pfil.forward.  This
type of forwarding is a property of IPv4 and IPv6 and thus should
be put there.  Pfil hooking can be on layer 2, 2-bridging, 3 and
who knows where else in the future.  Forwarding works only for IPv46.

You haven't even replied to my comment on net@.  Please change the
sysctl location and name to its appropriate place.

Also an MFC's after 2 weeks must ensure that compiling with IPFIREWALL_
FORWARD enabled the sysctl at the same time to keep kernel configs
within 9-stable working.

--
Andre

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-25 Thread Andrey V. Elsukov
On 25.10.2012 19:54, Andre Oppermann wrote:
 I still don't agree with naming the sysctl net.pfil.forward.  This
 type of forwarding is a property of IPv4 and IPv6 and thus should
 be put there.  Pfil hooking can be on layer 2, 2-bridging, 3 and
 who knows where else in the future.  Forwarding works only for IPv46.
 
 You haven't even replied to my comment on net@.  Please change the
 sysctl location and name to its appropriate place.

Hi Andre,

There were two replies related to this subject, you did not replied to
them and i thought that you became agree.
So, if not, what you think about the name net.pfil.ipforward?

 Also an MFC's after 2 weeks must ensure that compiling with IPFIREWALL_
 FORWARD enabled the sysctl at the same time to keep kernel configs
 within 9-stable working.

Yes, it will work like that.

-- 
WBR, Andrey V. Elsukov
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r242079 - in head: sbin/ipfw share/man/man4 sys/conf sys/net sys/netinet sys/netinet6 sys/netpfil/ipfw

2012-10-25 Thread Andre Oppermann

On 25.10.2012 18:25, Andrey V. Elsukov wrote:

On 25.10.2012 19:54, Andre Oppermann wrote:

I still don't agree with naming the sysctl net.pfil.forward.  This
type of forwarding is a property of IPv4 and IPv6 and thus should
be put there.  Pfil hooking can be on layer 2, 2-bridging, 3 and
who knows where else in the future.  Forwarding works only for IPv46.

You haven't even replied to my comment on net@.  Please change the
sysctl location and name to its appropriate place.


Hi Andre,

There were two replies related to this subject, you did not replied to
them and i thought that you became agree.


I replied to your reply to mine.  Other than that I didn't find
anything else from you.


So, if not, what you think about the name net.pfil.ipforward?


net.inet.ip.pfil_forward
net.inet6.ip6.pfil_forward

or something like that.

If you can show with your performance profiling that the sysctl
isn't even necessary, you could leave it completely away and have
pfil_forward enabled permanently.  That would be even better for
everybody.


Also an MFC's after 2 weeks must ensure that compiling with IPFIREWALL_
FORWARD enabled the sysctl at the same time to keep kernel configs
within 9-stable working.


Yes, it will work like that.


Excellent.  Thank you.

--
Andre

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org