Re: [RFC] genetlink custom attribute type
On Wed, 2006-09-20 at 08:24 +0200, Thomas Graf wrote: I think its best to use your patch for now and see where this leads to. Alright, should I repost with a proper [PATCH] subject or is it good to take as-is? :) johannes - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] genetlink custom attribute type
From: Johannes Berg [EMAIL PROTECTED] Date: Mon, 25 Sep 2006 16:58:28 +0200 On Wed, 2006-09-20 at 08:24 +0200, Thomas Graf wrote: I think its best to use your patch for now and see where this leads to. Alright, should I repost with a proper [PATCH] subject or is it good to take as-is? :) Please resend it as I've deleted the copy that was in my inbox. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] genetlink custom attribute type
* Johannes Berg [EMAIL PROTECTED] 2006-09-14 11:21 On Thu, 2006-09-14 at 10:14 +0200, Thomas Graf wrote: Looks good, we have to watch the size of struct nla_policy though. This bumps the size from 4 bytes to 16 bytes on 64bit architectures which might become a problem since we always use ATTR_MAX sized arrays. Yes, I'm aware of that, but I couldn't think of a way to handle it well. I thought about using a second array containing just the check functions, and then (ab)using `len' to index into it but that didn't seem clean enough. I agree, I talked about this with various people and some ideas came up. Always use function pointers to define the validation policy, i.e. there would be nla_validate_u32() etc. The problem with this approach is that for every string attribute with different length a separate validation function is required which simply adds to code what it saved from text. It also makes exporting the policy to userspace impossible. Following up on your idea, we could save a bit by using ERR_PTR() to store both the callback pointer and type/len tuple in the very same pointer but that gets very ugly. I think its best to use your patch for now and see where this leads to. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] genetlink custom attribute type
This patch adds an NLA_CUSTOM_CHECK type for netlink attributes
in order to be able to centrally define new attribute structures
instead of having to check these special types in each function
that uses such an attribute.
nl80211 will benefit from this because it needs an information
element attribute with a fixed type|length|value structure defined
in IEEE 802.11 which ought to be checked when passing an IE from
userspace.
Signed-off-by: Johannes Berg [EMAIL PROTECTED]
--- wireless-dev.orig/include/net/netlink.h 2006-09-13 22:06:09.979647141
+0200
+++ wireless-dev/include/net/netlink.h 2006-09-13 22:06:11.329647141 +0200
@@ -159,6 +159,7 @@ enum {
NLA_MSECS,
NLA_NESTED,
NLA_NUL_STRING,
+ NLA_CUSTOM_CHECK,
__NLA_TYPE_MAX,
};
@@ -176,6 +177,8 @@ enum {
*NLA_STRING Maximum length of string
*NLA_NUL_STRING Maximum length of string (excluding NUL)
*NLA_FLAG Unused
+ *NLA_CUSTOM_CHECK Unused, check function must be assigned,
+ * it must return 0 if the attribute is valid
*All otherExact length of attribute payload
*
* Example:
@@ -183,11 +186,13 @@ enum {
* [ATTR_FOO] = { .type = NLA_U16 },
* [ATTR_BAR] = { .type = NLA_STRING, len = BARSIZ },
* [ATTR_BAZ] = { .len = sizeof(struct mystruct) },
+ * [ATTR_CST] = { .type = NLA_CUSTOM_CHECK, .check = my_check },
* };
*/
struct nla_policy {
u16 type;
u16 len;
+ int (*check)(struct nlattr *nla);
};
extern voidnetlink_run_queue(struct sock *sk, unsigned int *qlen,
--- wireless-dev.orig/net/netlink/attr.c2006-09-13 22:06:05.889647141
+0200
+++ wireless-dev/net/netlink/attr.c 2006-09-13 22:06:11.329647141 +0200
@@ -67,6 +67,9 @@ static int validate_nla(struct nlattr *n
}
break;
+ case NLA_CUSTOM_CHECK:
+ BUG_ON(!pt-check);
+ return pt-check(nla);
default:
if (pt-len)
minlen = pt-len;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] genetlink custom attribute type
* Johannes Berg [EMAIL PROTECTED] 2006-09-14 09:44 This patch adds an NLA_CUSTOM_CHECK type for netlink attributes in order to be able to centrally define new attribute structures instead of having to check these special types in each function that uses such an attribute. nl80211 will benefit from this because it needs an information element attribute with a fixed type|length|value structure defined in IEEE 802.11 which ought to be checked when passing an IE from userspace. Looks good, we have to watch the size of struct nla_policy though. This bumps the size from 4 bytes to 16 bytes on 64bit architectures which might become a problem since we always use ATTR_MAX sized arrays. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] genetlink custom attribute type
On Thu, 2006-09-14 at 10:14 +0200, Thomas Graf wrote: Looks good, we have to watch the size of struct nla_policy though. This bumps the size from 4 bytes to 16 bytes on 64bit architectures which might become a problem since we always use ATTR_MAX sized arrays. Yes, I'm aware of that, but I couldn't think of a way to handle it well. I thought about using a second array containing just the check functions, and then (ab)using `len' to index into it but that didn't seem clean enough. Can you think of something better? johannes - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
