On Fri, Aug 22, 2008 at 12:01:19AM +0200, Julius Volz wrote: > 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? Yes, I hope so. > > [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. Thanks, that was confusing me. > > [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. Ok > > [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. Yeah, of course, I meant AF_INET6. -- 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