Re: [lvs-devel] Two patches for handling the synced connections

[Rumen Bogdanovski <rumen@xxxxxxxxxxxx>]

Hi Simon,
I did what you asked for, the patches are prepared against the current
net-2.6 tree

1. ipvs-showsync.patch

  1.1. checkpatch.pl
       WARNING: line over 80 characters
       #29: FILE: net/ipv4/ipvs/ip_vs_conn.c:800:
       +   "Pro FromIP   FPrt ToIP     TPrt DestIP   DPrt State
Origin Expires\n");

       WARNING: line over 80 characters
       #73: FILE: net/ipv4/ipvs/ip_vs_conn.c:996:
       +       proc_net_fops_create(&init_net, "ip_vs_conn_sync", 0,
&ip_vs_conn_sync_fops);

       total: 0 errors, 2 warnings, 78 lines checked

       If you find these fatal, let me know, but I do not think it is.

  1.2. This patch had two failures against the current net-2.6 tree 
       the problems were in proc_net_fops_create() and proc_net_remove()
       which actually needed an extra parameter compared to 2.6.23.1. 
       I have added "&init_net" and it compiles. I hope I did it the
       right way, didn't I?

2. ipvs-sync-setstate.patch
  
  2.1. checkpatch.pl
       Your patch has no obvious style problems and is ready for
submission.

  2.2. No problems found with this one for the current net-2.6 tree.
  
  2.3. This is an improved version of the previous patch I have sent.
       The previous one was "a proof of concept". Now the connection
       state is set (if needed) on each sync not only on creation, 
       so the backup will always (well almost) know the real state 
       of the synced connections.

Pay attention to 1.2. please, since the patches are tested on 2.6.23.1. 
Did I make it right? However the net-2.6 tree compiles without any
complaints with these patches applied.


regards, 
Rumen



On Tue, 2007-11-13 at 11:16 +0900, Simon Horman wrote:
> On Sun, Nov 11, 2007 at 11:47:02PM +0200, Rumen Bogdanovski wrote:
> > Hi, 
> > I have been thinking about the best way to handle the 
> > synchronization created connections and I decided (since 
> > there was no comment on that) that the best way to do so 
> > is as they are locally created. The only difference is that
> > synced connections will have IP_VS_CONN_F_SYNC set. The 
> > reasons are:
> > 
> > 1. If the sync connections are treated as inactive, on 
> > fail-over the task schedulers will use them with lower 
> > weight making the scheduling not quite adequate.
> > 
> > 2. Introducing new counter for synced connection means 
> > that schedulers should be altered to take these connections
> > in to account. And the existing tools should be changed too 
> > in order to show the real connection status. 
> > 
> > 3. No backward compatibility issues, no new data to be sent 
> > to the user space. All active connections will be listed as 
> > active and all inactive will be listed as inactive regardless
> > of the connection origin.
> > 
> > 4. The previous patch I have sent several days ago makes it 
> > possible to distinguish the locally created from sync-created
> > connections via /proc/net/ip_vs_conn_sync.
> > 
> > 5. It is less intrusive and much easier to implement.
> > 
> > 6. Active connections are active no matter where they were created.
> > On fail-over they will load the new director the same way the local 
> > connections do. And their life cycle will be just the same as if they
> > were locally created. So why should they be treated differently? 
> > Any reason for that? I could not think of any.  
> > 
> > So I am sending again the patch that
> > implements /proc/net/ip_vs_conn_sync. And a new one that creates the
> > synced connections according to their 
> > real state allowing the backup director, in case of fail-over, to handle
> > them in more appropriate way, avoiding scheduling and compatibility
> > issues. Well In case somebody wants to know which connections are
> > created locally which are sync-created he is always welcome to look in 
> > /proc/net/ip_vs_conn_sync.
> > 
> > Any comments please!
> > Simon, if you are happy with this let me know and I will provide a brief
> > description and sign-off.
> 
> Hi Rumen,
> 
> sorry to be slow.
> 
> I think that your approach is sound. The patches seem to work in
> such a way that they won't break existing tools. But tools could
> be enhanced to take advantage of these new features. Likewise
> for the schedulers.
> 
> The patches look fine. Though I'd like to request two things.
> 
> 1) could you run thr patches through ./scripts/checkpatch.pl (in the
>    kernel tree) and fix the things that it complains about.
> 
> 2) If at all possible, could you make the patches against the
>    net-2.6 tree that you can get using git from git.kernel.org.
>    See http://git.kernel.org/ or just run
> 
>    git clone git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6

ipvs-showsync.patch
Description: Text Data

ipvs-sync-setstate.patch
Description: Text Data

LVS Development mailing list - lvs-devel@xxxxxxxxxxxxxxxxxxxxxx
Send requests to lvs-devel-request@xxxxxxxxxxxxxxxxxxxxxx
or go to http://lists.graemef.net/mailman/listinfo/lvs-devel