Julius Volz wrote:
Ok, so this is my draft version of the IPVS Generic Netlink interface definition. I'm posting this to see if anyone notices general problems with it right away.
I'm not familiar with the ipvs interface itself, so I'll stick to netlink related comments.
Arrays of the same attribute type are always put into a nested container so that it is easy to add new attributes which are parallel to the array later on.
That makes sense.
Perhaps integer flag fields should also be split up into NLA_FLAG attributes, haven't done that yet.
I personally don't find NLA_FLAG very useful since for flags use usually want the flag and a mask, otherwise you can't unset it without the convention that userspace always includes it, even for change requests when it doesn't want to change it. And that is unusual for netlink and also needlessly complicated and racy in userspace since you'd have to query the current value before sending a change request.
First is a text listing attribute types and how they occur and nest in all of the commands and their replies. After that are the corresponding source excerpts (no patch material yet). Julius ====================================== | IPVS NETLINK ATTRIBUTE TYPES | | (grouped as enums) | ====================================== IPVS_ENTRY_ATTR_SERVICE - NLA_NESTED IPVS_ENTRY_ATTR_SERVICES - NLA_NESTED IPVS_ENTRY_ATTR_DEST - NLA_NESTED IPVS_ENTRY_ATTR_DESTS - NLA_NESTED IPVS_ENTRY_ATTR_DAEMON - NLA_NESTED IPVS_ENTRY_ATTR_DAEMONS - NLA_NESTED
So these are lists I assume. I don't think we have any examples of lists of nested attributes in the mainline kernel, but in some similar (unsubmitted) code of mine I used (names adjusted): IPVS_SERVICE_LIST - NLA_NESTED IPCS_DEST_LIST - NLA_NESTED IPVS_DAEMON_LIST - NLA_NESTED and 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.
IPVS_SVC_ATTR_AF - NLA_U32 IPVS_SVC_ATTR_PROTOCOL - NLA_U32 IPVS_SVC_ATTR_ADDR - union nf_inet_addr
This should probably use NLA_BINARY, which allows addresses of any kind.
IPVS_SVC_ATTR_PORT - NLA_U16 IPVS_SVC_ATTR_FWMARK - NLA_U32 IPVS_SVC_ATTR_SCHED_NAME - NLA_STRING
NLA_NUL_STRING (at least for validation purposes)?
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.
IPVS_SVC_ATTR_TIMEOUT - NLA_U32 IPVS_SVC_ATTR_NETMASK - NLA_U32
Shouldn't this also be able to carry IPv6 masks?
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.
IPVS_SVC_ATTR_STATS - NLA_NESTED IPVS_DEST_ATTR_AF - NLA_U32
Doesn't the family have to be equal for service and dest? If so, having it specified only once avoids potential inconsistencies.
========================== include/net/ip_vs.h ==========================
Please put this under include/linux, this doesn't belong here as its a public header. -- 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
