LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [*v4 PATCH 0/3] IPVS: Backup Adding Ipv6 and Persistence support

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [*v4 PATCH 0/3] IPVS: Backup Adding Ipv6 and Persistence support
Cc: Simon Horms <horms@xxxxxxxxxxxx>, "LVS-Devel" <lvs-devel@xxxxxxxxxxxxxxx>, Daniel Lezcano <daniel.lezcano@xxxxxxx>
From: Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx>
Date: Tue, 16 Nov 2010 15:51:52 +0100
On Monday 15 November 2010 23:29:11 Julian Anastasov wrote:
> 
>       Hello,
> 
> On Mon, 15 Nov 2010, Hans Schillstrom wrote:
> 
> > This patch series adds/(updates) the following functionality
> > in the synchronization between master and backup daemons.
> >
> > - IPv6
> > - Persistence Engine
> > - Firewall marks transferred
> > - Time-outs transferred.
> > - Flag field increased to 32 bits.
> 
>       Great work! I don't see other fatal problems. May be you
> can finally fix some grammatical and space problems and we can
> wait next weeks for comments about the new message format from
> other IPVS users. We also talked about checking if cp->pe
> matches cp->dest->svc->pe in ip_vs_find_dest(). Or may be it
> is already handled by the recent change that compares p->pe
> with cp->pe before calling ct_match in ip_vs_ct_in_get?

I do think it's handled by patch
 "Only match pe_data created by the same pe"

> 
> v4 PATCH 1/3
>       - still the case for pe_data_len=0 and pe_name_len!=0 is
>       not handled properly as error because we now ignore
>       pe_name silently

Do you mean that IP_VS_DBG should be replaced ?

        if (pe_data_len) {
                if (pe_name_len) {
...
                } else {
                        IP_VS_DBG(3, "BACKUP, Invalid PE parameters\n");
                        return 1;
                }
to this ?
        ...
                        IP_VS_ERR_RL("BACKUP, Invalid PE parameters\n");
    ...
> 
> v4 PATCH 2/3
>       ip_vs_sync_conn:
>               - I now see that we can calculate the padding
>               from previous message, so that we can skip
>               sending padding after the last connection:

Done.

> 
>               pad = (4 - (int) curr_sb->head) & 3;
> 
>               then we should clear the data before message:
> 
>               p = curr_sb->head;
>               curr_sb->head += pad;
>               while (pad--)
>                       *(p++) = 0;
>               s = (union ip_vs_sync_conn *) p;
> 
>               - Some checks are not needed, may be
>               cp->pe_data_len is enough:
> 
>               -if (cp->pe_data_len && cp->dest->svc && cp->pe && cp->pe->name)
>               +if (cp->pe_data_len)

OK, I'll do that
i.e. we should accept the result of: "pe or name is null" as a BUG...

> 
> v4 PATCH 3/3
>       OK
> 
> Regards
> 
> --
> Julian Anastasov <ja@xxxxxx>
> 

-- 
Regards
Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>