|
[CCed lvs-devel, just so this is archived somewhere
CCed Wensong and Julian, the other LVS maintainers]
On Sun, Oct 28, 2007 at 10:37:08PM +0200, Rumen Bogdanovski wrote:
> Hi all
> I have been looking through the IPVS code, because I wanted to implement
> more persistant connection-destination binding. I mean to try to bind
> the unbound connection every time its status is received. This is in
> case the destination is created later on.
> So I found something just beneath the place I have changed to bind the
> received connections to the services. Line ~330 of ip_vs_sync.c reads:
>
> } else if (!cp->dest) {
> /* it is an entry created by the synchronization */
> cp->state = ntohs(s->state);
> cp->flags = flags | IP_VS_CONN_F_HASHED;
> } /* Note that we don't touch its state and flags
> if it is a normal entry. */
>
> So with my patch the bound connections will not be considered as created
> by the synchronization. But this does not seem to be a problem.
> I wonder if this is removed and set state and flags for all received
> connections. As far as I understand this will not be a problem because
> the connections will be updated only if the current director is working
> as a backup, It will not handle any connection so it will not send any
> to the other directors. Is there a scenario to have two master directors
> for the same service? I think no, am I right? If I am, there should not
> be any difference where the connection is created, right?
> Is this code remnant from the past? What is the reason for it?
>
> How ever if there is a problem removing this test, I can introduce a
> flag in "struct ip_vs_conn" and make it true for the received
> connection and false for the local. In this case the code above should
> read something like:
>
> } else if (cp->foreign) {
> /* it is an entry created by the synchronization */
> cp->state = ntohs(s->state);
> cp->flags = flags | IP_VS_CONN_F_HASHED;
> } /* Note that we don't touch its state and flags
> if it is a normal entry. */
>
> And the old behavior will be restored.
>
> Any comments on that?
I think that the check is required.
cp->state = ntohs(s->state);
This portion synchronises the state synchronised when the master
sends a synchronisation message, which is generally what you want when
dealing with an entry created by synchronisation - basically
the master should know more about these connections than the slave.
cp->flags = flags | IP_VS_CONN_F_HASHED;
I'm not sure about the purpose of this portion at at a glance
it seems spurious. If ip_vs_conn_in_get() or ip_vs_ct_in_get()
found cp then surely IP_VS_CONN_F_HASHED must be set, as
for either of these functions to find cp it must be present in
the hash, and the only other places where IP_VS_CONN_F_HASHED is
set is when a connection is added or removed from the hash.
--
Horms
H: http://www.vergenet.net/~horms/
W: http://www.valinux.co.jp/en/
|
