Re: [PATCH 00/26] IPVS: Add first IPv6 support to IPVS.

[Patrick McHardy <kaber@trash.net>]

Julius Volz wrote:
> On Tue, Jun 17, 2008 at 10:08 PM, Patrick McHardy <kaber@trash.net> wrote:
>   
>> IPVS_LIST_ELEM                  - NLA_NESTED
>>
>> for list elements of every kind. Since you can only put one
>> kind of element in the lists anyway (I think), different
>> types don't allow any increased flexibility and the LIST
>> naming is more clear in my opinion.
>>     
>
> However, since these container attributes (for daemons, services and
> dests) also appear as single elements outside of lists, it might be
> better to reuse the same names inside the list?
>   

I didn't realize that. Yes, agreed.

>>> IPVS_SVC_ATTR_FLAGS             - NLA_U32
>>>       
>> As I mentioned above, you usually want a MASK in combination
>> with flags to allow to unset them. This is best done using
>> a structure.
>>     
>
> Hm, I'm not sure if I understand exactly what this struct is supposed
> to look like. Could you give an example?
>   
struct {
    u32 flags;
    u32 mask;
} flags;

and then:

obj->flags = (obj->flags & ~flags->mask) |
                    (flags->flags | flags->mask);

>>> IPVS_SVC_ATTR_TIMEOUT           - NLA_U32
>>> IPVS_SVC_ATTR_NETMASK           - NLA_U32
>>>       
>> Shouldn't this also be able to carry IPv6 masks?
>>     
>
> We only need the prefix length for IPv6, for which we reused the
> netmask field. This (only slightly) changes the semantics of the field
> between address families. Acceptable or better have a separate field
> for the prefix length?
>   
I guess thats fine.

>>> IPVS_SVC_ATTR_NUM_DESTS         - NLA_U32
>>>       
>> Is this number related to the IPVS_ENTRY_ATTR_DESTS list?
>> If so, it shouldn't be contained as seperate attribute,
>> that just allows for potential inconsistency.
>>     
>
> Yes, but this count is only returned from commands that do not at the
> same time return the list of destinations, so there is no
> inconsistency within a message. However, I'm pretty sure the count was
> only used in the old interface to allocate enough memory for the
> destination list, so it can probably be deleted anyways.
>   

You're probably right, this looks similar to iptables.