On Thu, Aug 21, 2008 at 11:29 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> On Thu, Aug 21, 2008 at 11:59:44AM +0200, Julius Volz wrote:
>> There shouldn't be any fundamental restrictions, it's just a piece of
>> the puzzle that I could easily leave out of the picture for now.
>>
>> I haven't studied the sync daemon closely yet, but one thing I was
>> briefly wondering about was whether we should just blow up the
>> addresses in struct ip_vs_sync_conn to be of type union nf_inet_addr
>> (probably not acceptable, wasting too much bandwidth for v4 entries)
>> or how to send differently sized entries based on the IP version in a
>> clean way. But it sounds like you'd want to redesign a lot of that
>> anyways? I'm glad to help with anything, I just don't know this code
>> as well as you or Sven, but I'll study it more. Maybe you can share
>> some ideas on the extensibility you want to see?
>
> What I was thinking is that any change
Hm, I think you forgot to finish this sentence ;)
> Here are a few thoughts I have had in my breif overview of the code so far
> - mainly just simple style things. If any of my comments are obviously
> stupid, please just say so as I haven't got to the end of the series yet
> and I'm sure some of my questions are answered in the code.
Thanks for looking at this, no problem. I know it's a bit much to
digest, but I think there is no smaller part that I could post that is
useful in itself already.
> [PATCH RFC 01/24] IPVS: Add genetlink interface definitions to ip_vs.h
> [PATCH RFC 02/24] IPVS: Add genetlink interface implementation
>
> * Already in lvs-2.6. Are there any changes?
No, those are just exactly what I sent you before. I included them
because I was basing it on net-2.6.
Btw., David just announced that he opened net-next-2.6, so perhaps he
could pull the changes from your tree into that now?
> [PATCH RFC 02/24] IPVS: Add genetlink interface implementation
>
> * What is IP_VS = IPV6 ?
(IPV6 = y || IP_VS = IPV6) means that this option will only be visible
if either CONFIG_IPV6 is set to Y or if both CONFIG_IPV6 and
CONFIG_IP_VS are modules (if both are off, the whole submenu will be
hidden). So CONFIG_IPV6 has to be set at least as high as CONFIG_IP_VS
for this option to be visible.
> [PATCH RFC 04/24] IPVS: Change IPVS data structures to support IPv6 addresses
>
> * Indentation of af in struct ip_vs_conn seems inconsistent with other
> elements.
Yeah, it's broken like this in the original and I kept it like that so
that things would still line up (because I didn't want to add noise by
touching the neighboring lines just for whitespace fixes). I could
just send a small whitespace fix to you before.
> [PATCH RFC 05/24] IPVS: Add general v4/v6 helper functions / data structures
>
> * Use of p ? a : b construct in ip_vs_addr_equal() seems a bit aquard.
> How about
>
> static inline int ip_vs_addr_equal(int af, const union nf_inet_addr *a,
> const union nf_inet_addr *b)
> {
> #ifdef CONFIG_IP_VS_IPV6
> if (af == AF_INET)
> return ipv6_addr_equal(&a->in6, &b->in6);
> #endif
> return a->ip == b->ip;
> }
Right, that is nicer! With AF_INET6 in that if-condition, of course.
Julius
--
Google Switzerland GmbH
--
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
