Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-15 Thread Julian Anastasov

Hello,

On Wed, 14 Nov 2007, Simon Horman wrote:

  So, as long as these entries are not
  accessible from sysctl it is safe to run without strategy handler but if
  values can be changed then we will need strategy handler to
  properly call update_defense_level() as done in proc_do_defense_mode()
  as proc_handler. There could be side effects if new mode is not applied.
 
 I'm not sure what you are getting at there. I did write a stratergy
 for update_defense_level(), but I didn't post it, as I thought that
 it would not be needed if CTL_UNNUMBERED is used.

Thanks everyone! I now see that by using CTL_UNNUMBERED
for ctl_name stops any writes to 'data', so there is no need for
'strategy' handler.

Regards

--
Julian Anastasov [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Simon Horman
On Tue, Nov 13, 2007 at 02:45:00AM -0800, David Miller wrote:
 From: Christian Borntraeger [EMAIL PROTECTED]
 Date: Tue, 13 Nov 2007 11:29:58 +0100
 
  Running the latest git code I get the following messages during boot:
  sysctl table check failed: /net/ipv4/vs/drop_entry .3.5.21.4 Missing 
  strategy
  [...] 
  sysctl table check failed: /net/ipv4/vs/drop_packet .3.5.21.5 Missing 
  strategy
  [...]
  sysctl table check failed: /net/ipv4/vs/secure_tcp .3.5.21.6 Missing 
  strategy
  [...]
  sysctl table check failed: /net/ipv4/vs/sync_threshold .3.5.21.24 Missing 
  strategy
  
  I removed the binary sysctl handler for those messages and also removed
  the definitions in ip_vs.h. The alternative would be to implement a 
  proper strategy handler, but syscall sysctl is deprecated.
  
  There are other sysctl definitions that are commented out or work with 
  the default sysctl_data strategy. I did not touch these. 
  
  Eric, IPVS team, are you ok with that change?
  
  CC: Eric W. Biederman [EMAIL PROTECTED]
  CC: Wensong Zhang [EMAIL PROTECTED]
  CC: Simon Horman [EMAIL PROTECTED]
  CC: Julian Anastasov [EMAIL PROTECTED]
  Signed-off-by: Christian Borntraeger [EMAIL PROTECTED]
 
 Simon planned to make a similar change to dice all of this
 stuff up.  He is travelling currently, and I think it's
 reasonable to give him some time to get to it.

Hi Christian, Hi Dave,

I have indeed been looking into this of late. Assuming that you use of
CTL_UNNUMBERED is correct, this patch looks fine to me.  Acked.

I was planning to do the same and also switch over all the other entries
over to use CTL_UNNUMBERED, as its hard to imagine that anyone is using
the sys_sysctl interface to IPVS.

As for the commented out entries. They are supposed to be exposed by
some other means - I believe the thinking was to comply with the don't
expose stuff in proc any more idea. Where is the best place to expose
this kind of stuff?

Lastly, as Dave mentions, I'm travelling this week, so please
excuse any slowness.

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Eric W. Biederman
Simon Horman [EMAIL PROTECTED] writes:

 Hi Christian, Hi Dave,

 I have indeed been looking into this of late. Assuming that you use of
 CTL_UNNUMBERED is correct, this patch looks fine to me.  Acked.

 I was planning to do the same and also switch over all the other entries
 over to use CTL_UNNUMBERED, as its hard to imagine that anyone is using
 the sys_sysctl interface to IPVS.

 As for the commented out entries. They are supposed to be exposed by
 some other means - I believe the thinking was to comply with the don't
 expose stuff in proc any more idea. Where is the best place to expose
 this kind of stuff?

 Lastly, as Dave mentions, I'm travelling this week, so please
 excuse any slowness.


Looking at this patch it looks sane enough. Either removing ctl_name
or explicitly using CTL_UNNUMBERED is fine.  It may be wise to leave
the binary entries in ip_vs.h and sysctl_check.c as documentation,
but even there it doesn't much matter since we don't plan on adding more.

Eric
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Julian Anastasov

Hello,

On Tue, 13 Nov 2007, Simon Horman wrote:

   Running the latest git code I get the following messages during boot:
   sysctl table check failed: /net/ipv4/vs/drop_entry .3.5.21.4 Missing 
   strategy
   [...]   
   sysctl table check failed: /net/ipv4/vs/drop_packet .3.5.21.5 Missing 
   strategy
   [...]
   sysctl table check failed: /net/ipv4/vs/secure_tcp .3.5.21.6 Missing 
   strategy
   [...]
   sysctl table check failed: /net/ipv4/vs/sync_threshold .3.5.21.24 Missing 
   strategy
   
   I removed the binary sysctl handler for those messages and also removed
   the definitions in ip_vs.h. The alternative would be to implement a 
   proper strategy handler, but syscall sysctl is deprecated.
   
   There are other sysctl definitions that are commented out or work with 
   the default sysctl_data strategy. I did not touch these. 
   
 Hi Christian, Hi Dave,
 
 I have indeed been looking into this of late. Assuming that you use of
 CTL_UNNUMBERED is correct, this patch looks fine to me.  Acked.
 
 I was planning to do the same and also switch over all the other entries
 over to use CTL_UNNUMBERED, as its hard to imagine that anyone is using
 the sys_sysctl interface to IPVS.
 
 As for the commented out entries. They are supposed to be exposed by
 some other means - I believe the thinking was to comply with the don't
 expose stuff in proc any more idea. Where is the best place to expose
 this kind of stuff?

I assume /proc/sys is still valid place, only sysctl interface
is scheduled for removal. So, as long as these entries are not
accessible from sysctl it is safe to run without strategy handler but if
values can be changed then we will need strategy handler to
properly call update_defense_level() as done in proc_do_defense_mode()
as proc_handler. There could be side effects if new mode is not applied.

Regards

--
Julian Anastasov [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Eric W. Biederman
Julian Anastasov [EMAIL PROTECTED] writes:

   I assume /proc/sys is still valid place, only sysctl interface
 is scheduled for removal. 

Yes. The ascii versions of the sysctls that show up in /proc/sys are
definitely still valid.

 So, as long as these entries are not
 accessible from sysctl it is safe to run without strategy handler but if
 values can be changed then we will need strategy handler to
 properly call update_defense_level() as done in proc_do_defense_mode()
 as proc_handler. There could be side effects if new mode is not applied.

Yes.  The current mode of 0644 allows them to be both read and updated
with sys_sysctl.  By removing the ctl_name entry those entries become
inaccessible from the /proc/sys interface.  Which is some easier then
writing a strategy routine.

Eric
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Simon Horman
On Thu, Nov 15, 2007 at 02:38:32AM +0200, Julian Anastasov wrote:

Hi Julian,

 On Tue, 13 Nov 2007, Simon Horman wrote:

[snip]

  As for the commented out entries. They are supposed to be exposed by
  some other means - I believe the thinking was to comply with the don't
  expose stuff in proc any more idea. Where is the best place to expose
  this kind of stuff?
 
   I assume /proc/sys is still valid place, only sysctl interface
 is scheduled for removal.

I'm happy to add them there, so long as that is a good place.

 So, as long as these entries are not
 accessible from sysctl it is safe to run without strategy handler but if
 values can be changed then we will need strategy handler to
 properly call update_defense_level() as done in proc_do_defense_mode()
 as proc_handler. There could be side effects if new mode is not applied.

I'm not sure what you are getting at there. I did write a stratergy
for update_defense_level(), but I didn't post it, as I thought that
it would not be needed if CTL_UNNUMBERED is used.

-- 
Horms, California Edition

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Eric W. Biederman
Simon Horman [EMAIL PROTECTED] writes:

 On Thu, Nov 15, 2007 at 02:38:32AM +0200, Julian Anastasov wrote:

 Hi Julian,

 On Tue, 13 Nov 2007, Simon Horman wrote:

 [snip]

  As for the commented out entries. They are supposed to be exposed by
  some other means - I believe the thinking was to comply with the don't
  expose stuff in proc any more idea. Where is the best place to expose
  this kind of stuff?
 
  I assume /proc/sys is still valid place, only sysctl interface
 is scheduled for removal.

 I'm happy to add them there, so long as that is a good place.

For simple integer values /proc/sys (ala the ascii sysctl interface)
seems as good as any to me.

The binary interface is problematic because it doesn't get used and
so we don't show proper discipline with binary integers leading to
silent ABI changes, and the actual implementation of the handler
routines get out of sync with the proc side giving us different
meanings.

 So, as long as these entries are not
 accessible from sysctl it is safe to run without strategy handler but if
 values can be changed then we will need strategy handler to
 properly call update_defense_level() as done in proc_do_defense_mode()
 as proc_handler. There could be side effects if new mode is not applied.

 I'm not sure what you are getting at there. I did write a stratergy
 for update_defense_level(), but I didn't post it, as I thought that
 it would not be needed if CTL_UNNUMBERED is used.

Strategy routines are never called if CTL_UNNUMBERED is used.  So you
should be safe just killing the ctl_name field or setting it
explicitly to CTL_UNNUMBERED.

Eric

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-14 Thread Simon Horman
On Wed, Nov 14, 2007 at 06:25:00PM -0700, Eric W. Biederman wrote:
 Simon Horman [EMAIL PROTECTED] writes:
 
  On Thu, Nov 15, 2007 at 02:38:32AM +0200, Julian Anastasov wrote:
 
  Hi Julian,
 
  On Tue, 13 Nov 2007, Simon Horman wrote:
 
  [snip]
 
   As for the commented out entries. They are supposed to be exposed by
   some other means - I believe the thinking was to comply with the don't
   expose stuff in proc any more idea. Where is the best place to expose
   this kind of stuff?
  
 I assume /proc/sys is still valid place, only sysctl interface
  is scheduled for removal.
 
  I'm happy to add them there, so long as that is a good place.
 
 For simple integer values /proc/sys (ala the ascii sysctl interface)
 seems as good as any to me.

Understood.

 The binary interface is problematic because it doesn't get used and
 so we don't show proper discipline with binary integers leading to
 silent ABI changes, and the actual implementation of the handler
 routines get out of sync with the proc side giving us different
 meanings.
 
  So, as long as these entries are not
  accessible from sysctl it is safe to run without strategy handler but if
  values can be changed then we will need strategy handler to
  properly call update_defense_level() as done in proc_do_defense_mode()
  as proc_handler. There could be side effects if new mode is not applied.
 
  I'm not sure what you are getting at there. I did write a stratergy
  for update_defense_level(), but I didn't post it, as I thought that
  it would not be needed if CTL_UNNUMBERED is used.
 
 Strategy routines are never called if CTL_UNNUMBERED is used.  So you
 should be safe just killing the ctl_name field or setting it
 explicitly to CTL_UNNUMBERED.

Thanks Eric, thats more or less what I thought.

-- 
Horms, California Edition

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-13 Thread Christian Borntraeger
Running the latest git code I get the following messages during boot:
sysctl table check failed: /net/ipv4/vs/drop_entry .3.5.21.4 Missing strategy
[...] 
sysctl table check failed: /net/ipv4/vs/drop_packet .3.5.21.5 Missing strategy
[...]
sysctl table check failed: /net/ipv4/vs/secure_tcp .3.5.21.6 Missing strategy
[...]
sysctl table check failed: /net/ipv4/vs/sync_threshold .3.5.21.24 Missing 
strategy

I removed the binary sysctl handler for those messages and also removed
the definitions in ip_vs.h. The alternative would be to implement a 
proper strategy handler, but syscall sysctl is deprecated.

There are other sysctl definitions that are commented out or work with 
the default sysctl_data strategy. I did not touch these. 

Eric, IPVS team, are you ok with that change?

CC: Eric W. Biederman [EMAIL PROTECTED]
CC: Wensong Zhang [EMAIL PROTECTED]
CC: Simon Horman [EMAIL PROTECTED]
CC: Julian Anastasov [EMAIL PROTECTED]
Signed-off-by: Christian Borntraeger [EMAIL PROTECTED]

---
 include/net/ip_vs.h   |4 
 kernel/sysctl_check.c |4 
 net/ipv4/ipvs/ip_vs_ctl.c |4 
 3 files changed, 12 deletions(-)

Index: linux-2.6/include/net/ip_vs.h
===
--- linux-2.6.orig/include/net/ip_vs.h
+++ linux-2.6/include/net/ip_vs.h
@@ -336,9 +336,6 @@ enum {
NET_IPV4_VS_DEBUG_LEVEL=1,
NET_IPV4_VS_AMEMTHRESH=2,
NET_IPV4_VS_AMDROPRATE=3,
-   NET_IPV4_VS_DROP_ENTRY=4,
-   NET_IPV4_VS_DROP_PACKET=5,
-   NET_IPV4_VS_SECURE_TCP=6,
NET_IPV4_VS_TO_ES=7,
NET_IPV4_VS_TO_SS=8,
NET_IPV4_VS_TO_SR=9,
@@ -355,7 +352,6 @@ enum {
NET_IPV4_VS_LBLCR_EXPIRE=20,
NET_IPV4_VS_CACHE_BYPASS=22,
NET_IPV4_VS_EXPIRE_NODEST_CONN=23,
-   NET_IPV4_VS_SYNC_THRESHOLD=24,
NET_IPV4_VS_NAT_ICMP_SEND=25,
NET_IPV4_VS_EXPIRE_QUIESCENT_TEMPLATE=26,
NET_IPV4_VS_LAST
Index: linux-2.6/net/ipv4/ipvs/ip_vs_ctl.c
===
--- linux-2.6.orig/net/ipv4/ipvs/ip_vs_ctl.c
+++ linux-2.6/net/ipv4/ipvs/ip_vs_ctl.c
@@ -1451,7 +1451,6 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_dointvec,
},
{
-   .ctl_name   = NET_IPV4_VS_DROP_ENTRY,
.procname   = drop_entry,
.data   = sysctl_ip_vs_drop_entry,
.maxlen = sizeof(int),
@@ -1459,7 +1458,6 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_do_defense_mode,
},
{
-   .ctl_name   = NET_IPV4_VS_DROP_PACKET,
.procname   = drop_packet,
.data   = sysctl_ip_vs_drop_packet,
.maxlen = sizeof(int),
@@ -1467,7 +1465,6 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_do_defense_mode,
},
{
-   .ctl_name   = NET_IPV4_VS_SECURE_TCP,
.procname   = secure_tcp,
.data   = sysctl_ip_vs_secure_tcp,
.maxlen = sizeof(int),
@@ -1597,7 +1594,6 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_dointvec,
},
{
-   .ctl_name   = NET_IPV4_VS_SYNC_THRESHOLD,
.procname   = sync_threshold,
.data   = sysctl_ip_vs_sync_threshold,
.maxlen = sizeof(sysctl_ip_vs_sync_threshold),
Index: linux-2.6/kernel/sysctl_check.c
===
--- linux-2.6.orig/kernel/sysctl_check.c
+++ linux-2.6/kernel/sysctl_check.c
@@ -242,9 +242,6 @@ static struct trans_ctl_table trans_net_
{ NET_IPV4_VS_AMEMTHRESH,   amemthresh },
{ NET_IPV4_VS_DEBUG_LEVEL,  debug_level },
{ NET_IPV4_VS_AMDROPRATE,   am_droprate },
-   { NET_IPV4_VS_DROP_ENTRY,   drop_entry },
-   { NET_IPV4_VS_DROP_PACKET,  drop_packet },
-   { NET_IPV4_VS_SECURE_TCP,   secure_tcp },
{ NET_IPV4_VS_TO_ES,timeout_established },
{ NET_IPV4_VS_TO_SS,timeout_synsent },
{ NET_IPV4_VS_TO_SR,timeout_synrecv },
@@ -260,7 +257,6 @@ static struct trans_ctl_table trans_net_
{ NET_IPV4_VS_CACHE_BYPASS, cache_bypass },
{ NET_IPV4_VS_EXPIRE_NODEST_CONN,   expire_nodest_conn },
{ NET_IPV4_VS_EXPIRE_QUIESCENT_TEMPLATE,
expire_quiescent_template },
-   { NET_IPV4_VS_SYNC_THRESHOLD,   sync_threshold },
{ NET_IPV4_VS_NAT_ICMP_SEND,nat_icmp_send },
{ NET_IPV4_VS_LBLC_EXPIRE,  lblc_expiration },
{ NET_IPV4_VS_LBLCR_EXPIRE, lblcr_expiration },
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  

Re: [PATCH] IPVS: Fix sysctl warnings about missing strategy

2007-11-13 Thread David Miller
From: Christian Borntraeger [EMAIL PROTECTED]
Date: Tue, 13 Nov 2007 11:29:58 +0100

 Running the latest git code I get the following messages during boot:
 sysctl table check failed: /net/ipv4/vs/drop_entry .3.5.21.4 Missing strategy
 [...]   
 sysctl table check failed: /net/ipv4/vs/drop_packet .3.5.21.5 Missing strategy
 [...]
 sysctl table check failed: /net/ipv4/vs/secure_tcp .3.5.21.6 Missing strategy
 [...]
 sysctl table check failed: /net/ipv4/vs/sync_threshold .3.5.21.24 Missing 
 strategy
 
 I removed the binary sysctl handler for those messages and also removed
 the definitions in ip_vs.h. The alternative would be to implement a 
 proper strategy handler, but syscall sysctl is deprecated.
 
 There are other sysctl definitions that are commented out or work with 
 the default sysctl_data strategy. I did not touch these. 
 
 Eric, IPVS team, are you ok with that change?
 
 CC: Eric W. Biederman [EMAIL PROTECTED]
 CC: Wensong Zhang [EMAIL PROTECTED]
 CC: Simon Horman [EMAIL PROTECTED]
 CC: Julian Anastasov [EMAIL PROTECTED]
 Signed-off-by: Christian Borntraeger [EMAIL PROTECTED]

Simon planned to make a similar change to dice all of this
stuff up.  He is travelling currently, and I think it's
reasonable to give him some time to get to it.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html