Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
I can never get these web-browser email clients to work well. Sorry for sending fscking html earlier. cheers, jamal -- Forwarded message -- From: jamal <[EMAIL PROTECTED]> Date: May 27, 2007 9:29 AM Subject: Re: [PATCH] [-mm] ACPI: export ACPI events via netlink To: Zhang Rui <[EMAIL PROTECTED]> Cc: [EMAIL PROTECTED], netdev@vger.kernel.org On 5/27/07, Zhang Rui <[EMAIL PROTECTED]> wrote: I need to write a user application to test my patch. Netlink messages can be sent/received using the standard socket API. sure. But how to receive Genetlink messages from specified genetlink family? There is no socket ACPI with such a parameter, right? Each module has a unique identifier that it receives dynamically on insertion at the kernel. Do I have to receive all the genetlink messages first? No, just the ones for your dynamic id. Try what i described first for kernel side on the earlier email. I will repeat it here for clarity. Then look at genl code and if you have questions i can help. Note: You need to discover your dynamic id (the iproute2/genl code has a stub example code) As i told you in the earlier email, in your development: - start first by just writting your kernel side. - Then use the genl utility - which is part of iproute2 to see if the kernel side is "discoverable". E.g if i wanted to "discover" currently loaded modules on my laptop, i would do this: --- [EMAIL PROTECTED]:~$ genl ctrl ls Name: nlctrl ID: 0x10 Version: 0x2 header size: 0 max attribs: 6 commands supported: #1: ID-0x3 flags-0xe Name: nl80211 ID: 0x11 Version: 0x1 header size: 0 max attribs: 22 commands supported: #1: ID-0x1 flags-0xa #2: ID-0x6 flags-0xa #3: ID-0x8 flags-0xa #4: ID-0x3 flags-0xb #5: ID-0x4 flags-0xb #6: ID-0x5 flags-0xb #7: ID-0xa flags-0xb #8: ID-0xb flags-0xa #9: ID-0xf flags-0xb #10: ID-0x10 flags-0xa #11: ID-0x12 flags-0xb #12: ID-0x13 flags-0xa #13: ID-0x15 flags-0xa #14: ID-0x19 flags-0xb #15: ID-0x17 flags-0xb #16: ID-0x18 flags-0xb #17: ID-0x1a flags-0xb #18: ID-0x1b flags-0xa #19: ID-0xd flags-0xb Name: TASKSTATS ID: 0x12 Version: 0x1 header size: 0 max attribs: 4 commands supported: #1: ID-0x1 flags-0xa --- As you can see, i can see from user space the name of the kernel end point, its numeric id, what version it is running (so i can make sure user space is compatible), what extra header it may have, what the maximum number of attributes it can take. The last thing that gets listed is the commands, and flags for those commands. Lets load tipc kernel module and repeat... --- [EMAIL PROTECTED]:~$ sudo modprobe tipc Name: nlctrl ID: 0x10 Version: 0x2 header size: 0 max attribs: 6 commands supported: #1: ID-0x3 flags-0xe [same as before] Name: TIPC ID: 0x13 Version: 0x1 header size: 8 max attribs: 0 commands supported: #1: ID-0x1 flags-0x2 === It would be great if there are any examples on user space communication. Bug Thomas - he has written some simple example. I also have some but i changed laptops and i have to go and dig it up for you. I do have plans for making this easier for people - but havent had time. If there is persistence - or someone out there wants to be a hero email me privately and i will explain it. Or should I use libnl library instead? Why am i answering all these questions if you are fine with using libnl? Last time you said you couldnt use a library, no? cheers, jamal Thanks, Rui. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-22-06 at 12:09 +0200, Johannes Berg wrote: > Why do you think that would be hard? It'd basically just mean replacing > the netlink_capable(sock, NL_NONROOT_RECV) calls with a call that > actually tests depending on the group(s) it wants. I think it could be done. You will need to have root maybe initially set such permissions etc - but it may be overkill. > Yeah, sounds reasonable, you could ask the controller for which groups > are attached to a family and then get the IDs for those groups by name. Yes, we would need a newer api to do it right. But it could be done if you register for multi groups. cheers, jamal PS:- I just noticed Thomas wasnt CCed. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-06-25 at 13:08 -0400, jamal wrote: > > Why do you think that would be hard? It'd basically just mean replacing > > the netlink_capable(sock, NL_NONROOT_RECV) calls with a call that > > actually tests depending on the group(s) it wants. > > I think it could be done. You will need to have root maybe initially set > such permissions etc - but it may be overkill. I think we pretty much know in the kernel whether we want to require CAP_NET_ADMIN or not, let's punt the rest to userspace. > > Yeah, sounds reasonable, you could ask the controller for which groups > > are attached to a family and then get the IDs for those groups by name. > > Yes, we would need a newer api to do it right. But it could be done if > you register for multi groups. I've just replied somewhere else in this thread with a patch, I haven't actually tested that patch yet though. Once the generic netlink multicast is figured out we can start attacking the permissions issue. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Tue, 2007-06-19 at 11:32 +0800, Zhang Rui wrote:
> > Ok, by inspection (sorry, still dont have much time) - your kernel code
> > is sending to group 1; i.e
> >
> > genlmsg_multicast(skb, 0, 1, GFP_ATOMIC);
> >
> > you need to change that to send to your assigned id, i.e:
> > genlmsg_multicast(skb, 0, acpi_event_genl_family.id, GFP_ATOMIC);
> >
> Oh, that's the problem.
> Great, now it works happily. :).
> Jamal, thanks for your help!
I wonder if we should hold off on this API until we've worked out the
multicast issue.
Right now we have (mostly by convention afaict) in generic netlink that
everybody has the same group ID as the family ID but that breaks down as
soon as somebody needs more groups than that, which nl80211 will most
likely need.
Hence, the proposal Jamal had was to have a dynamic multicast number
allocator and (if I understood correctly) look up multicast numbers by
family ID/name. This is fairly extensive API/ABI change, but luckily
there are no generic netlink multicast users yet except for the
controller which luckily has the fixed ID "1".
Therefore, if we hold off on this patch until we've written the code for
dynamic multicast groups, we can hardcode the group for controller and
have all others dynamically assigned; if we merge the ACPI events now
we'll have to hardcode the ACPI family ID (and thus multicast group) to
a small number to avoid problems with dynamic multicast groups where the
numbers will be != family ID.
My proposition for the actual dynamic registration interface would be to
add a .groups array to pointers to struct genl_family with that just
being
struct genl_multicast_group {
char *name;
u32 id;
}
(as usual, NULL signifies array termination)
and the controller is responsible for assigning the ID and exporting it
to userspace. "name" is a per-family field, something like this patch:
---
include/linux/genetlink.h |3 +
include/net/genetlink.h | 15 ++
net/netlink/genetlink.c | 111 ++
3 files changed, 129 insertions(+)
--- wireless-dev.orig/include/net/genetlink.h 2007-06-25 23:56:59.085732308
+0200
+++ wireless-dev/include/net/genetlink.h2007-06-26 00:01:43.935732308
+0200
@@ -5,12 +5,26 @@
#include
/**
+ * struct genl_multicast_group - generic netlink multicast group
+ * @name: name of the multicast group, names are per-family
+ * @id: multicast group ID, assigned by the core, to use with
+ * genlmsg_multicast().
+ */
+struct genl_multicast_group
+{
+ charname[GENL_NAMSIZ];
+ u32 id;
+};
+
+/**
* struct genl_family - generic netlink family
* @id: protocol family idenfitier
* @hdrsize: length of user specific header in bytes
* @name: name of family
* @version: protocol version
* @maxattr: maximum number of attributes supported
+ * @multicast_groups: multicast groups to be registered
+ * for this family (%NULL-terminated array)
* @attrbuf: buffer to store parsed attributes
* @ops_list: list of all assigned operations
* @family_list: family list
@@ -22,6 +36,7 @@ struct genl_family
charname[GENL_NAMSIZ];
unsigned intversion;
unsigned intmaxattr;
+ struct genl_multicast_group **multicast_groups;
struct nlattr **attrbuf;/* private */
struct list_headops_list; /* private */
struct list_headfamily_list;/* private */
--- wireless-dev.orig/net/netlink/genetlink.c 2007-06-25 23:56:02.805732308
+0200
+++ wireless-dev/net/netlink/genetlink.c2007-06-26 00:39:26.985732308
+0200
@@ -3,6 +3,7 @@
*
* Authors:Jamal Hadi Salim
* Thomas Graf <[EMAIL PROTECTED]>
+ * Johannes Berg <[EMAIL PROTECTED]>
*/
#include
@@ -13,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,6 +44,15 @@ static void genl_unlock(void)
#define GENL_FAM_TAB_MASK (GENL_FAM_TAB_SIZE - 1)
static struct list_head family_ht[GENL_FAM_TAB_SIZE];
+/*
+ * To avoid an allocation at boot of just one unsigned long,
+ * declare it global instead.
+ * Bit 0 (special?) and bit 1 are marked as already used
+ * since group 1 is the controller group.
+ */
+static unsigned long mcast_group_start = 0x3;
+static unsigned long *multicast_groups = &mcast_group_start;
+static unsigned long multicast_group_bits = BITS_PER_LONG;
static int genl_ctrl_event(int event, void *data);
@@ -116,6 +127,76 @@ static inline u16 genl_generate_id(void)
return id_gen_idx;
}
+static int genl_register_mcast_group(struct genl_multicast_group *grp)
+{
+ int id = find_first_zero_bit(multicast_groups, multicast_group_bits);
+
+ if (id >= multicast_group_bits) {
+ if (multicast_groups == &mcast_group_start) {
+ multicast_groups =
+ kzallo
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Tue, 2007-26-06 at 00:40 +0200, Johannes Berg wrote: > I wonder if we should hold off on this API until we've worked out the > multicast issue. I think we can fix all the code in one shot later. I just glanced at your patch but i have to run out, i will stare at it later - seems to be in the right direction. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Tue, 2007-06-26 at 09:33 -0400, jamal wrote: > On Tue, 2007-26-06 at 00:40 +0200, Johannes Berg wrote: > > > I wonder if we should hold off on this API until we've worked out the > > multicast issue. > > I think we can fix all the code in one shot later. Yes, we could fix the code in the kernel, but since the family ID is dynamically assigned and I'm trying to decouple the multicast group ID from the family ID that would break userspace relying on family==multicast group unless we somehow reserved the family ID number ACPI got to make sure that ACPI gets the same multicast group ID. Combined with the fact that ACPI might be modular and get into generic netlink late in the game this seems non-trivial; also I think it's not necessary since holding off on this ACPI genetlink multicast user (which is the first besides the controller!) until we've worked out the patch shouldn't hurt much. > I just glanced at > your patch but i have to run out, i will stare at it later - seems to be > in the right direction. Thanks. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Tue, 2007-26-06 at 00:40 +0200, Johannes Berg wrote:
> I wonder if we should hold off on this API until we've worked out the
> multicast issue.
Even if the ACPI thing goes in first, you will have to change a few
others that are existing in-kernel that need to be changed sooner or
later. So either way is fine.
> My proposition for the actual dynamic registration interface would be to
> add a .groups array to pointers to struct genl_family with that just
> being
>
> struct genl_multicast_group {
> char *name;
> u32 id;
> }
heres some feedback:
- I think you should use the same approach as we use for ops.
a) i.e other than the reserved group for controller (which you seem to
be taking care of), every other genetlink user has to explicitly
register when they need a mcast group.
b) While the names may vary on a per-family basis, the Grpids should be
unique (e.g when you run out of group ids - it would take a lot more
allocations than there are families; 32 bit vs 16 bit for that to
happen)
c) Use a global hash table to store all the genl_multicast_groups;
I think this (handwaving) should be searchable by i) name ii)ID and iii)
family.
> --- wireless-dev.orig/include/net/genetlink.h 2007-06-25 23:56:59.085732308
> +0200
> +++ wireless-dev/include/net/genetlink.h 2007-06-26 00:01:43.935732308
> +0200
> @@ -5,12 +5,26 @@
> #include
>
> /**
> + * struct genl_multicast_group - generic netlink multicast group
> + * @name: name of the multicast group, names are per-family
> + * @id: multicast group ID, assigned by the core, to use with
> + * genlmsg_multicast().
> + */
> +struct genl_multicast_group
> +{
> + charname[GENL_NAMSIZ];
> + u32 id;
> +};
Add the list constructs on the struct - look at the way commands are
done.
We do use hash tables for global storage of families for example.
> +/**
> * struct genl_family - generic netlink family
> * @id: protocol family idenfitier
> * @hdrsize: length of user specific header in bytes
> * @name: name of family
> * @version: protocol version
> * @maxattr: maximum number of attributes supported
> + * @multicast_groups: multicast groups to be registered
> + * for this family (%NULL-terminated array)
> * @attrbuf: buffer to store parsed attributes
> * @ops_list: list of all assigned operations
> * @family_list: family list
> @@ -22,6 +36,7 @@ struct genl_family
> charname[GENL_NAMSIZ];
> unsigned intversion;
> unsigned intmaxattr;
> + struct genl_multicast_group **multicast_groups;
If you do the lists (struct list_head) you wont need this.
> +static unsigned long mcast_group_start = 0x3;
> +static unsigned long *multicast_groups = &mcast_group_start;
> +static unsigned long multicast_group_bits = BITS_PER_LONG;
I think if you used a hash table you wont need to keep track of the
above; maybe not - You may still need the above to keep track of which
IDs are in use and where holes in multicast group ID space exist
(assuming some are going to be unregistered over time etc)
> +
> +static int genl_register_mcast_groups(struct genl_family *family)
> +{
we use a simple scheme in the case of families; once all
IDs are consumed we dont alloc more - in the case of mcast grps this is
not necessary IMO i.e it doesnt matter if there is collision when you
run out. You return errors at the moment.
> --- wireless-dev.orig/include/linux/genetlink.h 2007-06-25
> 23:56:19.895732308 +0200
> +++ wireless-dev/include/linux/genetlink.h2007-06-26 00:33:36.785732308
> +0200
> @@ -52,6 +52,9 @@ enum {
> CTRL_ATTR_HDRSIZE,
> CTRL_ATTR_MAXATTR,
> CTRL_ATTR_OPS,
> + CTRL_ATTR_MCAST_GROUPS,
> + CTRL_ATTR_MCAST_GRP_NAME,
> + CTRL_ATTR_MCAST_GRP_ID,
Dont think those last two belong in the same namespace. i.e they should
be a separate enum; even more the name/id pair probably belong in one
TLV under the struct genl_multicast_group that is exported to user
space.
Overall: I think you are on the right path. Good stuff.
I see user space doing a discovery of which groups a family belongs to
or even doing a bind-by-name in which the underlying user-space code
does a discovery to find the id, then does a bind to that id.
cheers,
jamal
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Wed, 2007-06-27 at 19:24 -0400, jamal wrote:
> On Tue, 2007-26-06 at 00:40 +0200, Johannes Berg wrote:
>
> > I wonder if we should hold off on this API until we've worked out the
> > multicast issue.
>
> Even if the ACPI thing goes in first, you will have to change a few
> others that are existing in-kernel that need to be changed sooner or
> later. So either way is fine.
No, the controller is the only other generic netlink multicast user
according to what I've found. :)
> heres some feedback:
> - I think you should use the same approach as we use for ops.
> a) i.e other than the reserved group for controller (which you seem to
> be taking care of), every other genetlink user has to explicitly
> register when they need a mcast group.
Alright. I think that most likely this will happen right at setup, but I
can change it.
> b) While the names may vary on a per-family basis, the Grpids should be
> unique (e.g when you run out of group ids - it would take a lot more
> allocations than there are families; 32 bit vs 16 bit for that to
> happen)
actually didn't you just say that groups don't run out at 32-bit because
the groups bitmap can be extended? You wouldn't be able to sign up for
the groups >31 at bind() time but with ioctls you can bind higher group
numbers after the fact. And the dynamic groups mean you need to bind
later anyway since you don't know the ID when you create the socket.
> c) Use a global hash table to store all the genl_multicast_groups;
> I think this (handwaving) should be searchable by i) name ii)ID and iii)
> family.
Yeah, makes sense, I never liked the bitmap stuff I did there.
> Add the list constructs on the struct - look at the way commands are
> done.
> We do use hash tables for global storage of families for example.
Right, with dynamic registration that's basically a given.
> > +static unsigned long mcast_group_start = 0x3;
> > +static unsigned long *multicast_groups = &mcast_group_start;
> > +static unsigned long multicast_group_bits = BITS_PER_LONG;
>
> I think if you used a hash table you wont need to keep track of the
> above; maybe not - You may still need the above to keep track of which
> IDs are in use and where holes in multicast group ID space exist
> (assuming some are going to be unregistered over time etc)
Ah, holes are a good point. I'll think about it.
> we use a simple scheme in the case of families; once all
> IDs are consumed we dont alloc more - in the case of mcast grps this is
> not necessary IMO i.e it doesnt matter if there is collision when you
> run out. You return errors at the moment.
Are you saying I should double-allocate groups? I really don't want that
since I plan to add permission checks on top of this.
> > --- wireless-dev.orig/include/linux/genetlink.h 2007-06-25
> > 23:56:19.895732308 +0200
> > +++ wireless-dev/include/linux/genetlink.h 2007-06-26 00:33:36.785732308
> > +0200
> > @@ -52,6 +52,9 @@ enum {
> > CTRL_ATTR_HDRSIZE,
> > CTRL_ATTR_MAXATTR,
> > CTRL_ATTR_OPS,
> > + CTRL_ATTR_MCAST_GROUPS,
> > + CTRL_ATTR_MCAST_GRP_NAME,
> > + CTRL_ATTR_MCAST_GRP_ID,
>
> Dont think those last two belong in the same namespace. i.e they should
> be a separate enum; even more the name/id pair probably belong in one
> TLV under the struct genl_multicast_group that is exported to user
> space.
Hm? Not sure I understand this. These are attributes for the generic
netlink controller messages, where else should I put them? Why give them
numbers from a different namespace because they're used inside nested
attributes?
> Overall: I think you are on the right path. Good stuff.
> I see user space doing a discovery of which groups a family belongs to
> or even doing a bind-by-name in which the underlying user-space code
> does a discovery to find the id, then does a bind to that id.
Yeah, that's what I was thinking of, although the bind-by-name needs
(family-id, group-name) and nost just group-name
johannes
signature.asc
Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Thu, 2007-28-06 at 11:45 +0200, Johannes Berg wrote:
> No, the controller is the only other generic netlink multicast user
> according to what I've found. :)
Scanning the code - true ;-> Iam a little suprised that the task
accounting folks didnt use it to periodically dump stats.
> actually didn't you just say that groups don't run out at 32-bit because
> the groups bitmap can be extended? You wouldn't be able to sign up for
> the groups >31 at bind() time but with ioctls you can bind higher group
> numbers after the fact. And the dynamic groups mean you need to bind
> later anyway since you don't know the ID when you create the socket.
Sorry i meant there are 2^16 families (-1 considering controller)
There are 2^32 groups (-1 considering controller) for the same number of
families. i.e i see those items as being global within genetlink.
> > c) Use a global hash table to store all the genl_multicast_groups;
> > I think this (handwaving) should be searchable by i) name ii)ID and iii)
> > family.
>
> Yeah, makes sense, I never liked the bitmap stuff I did there.
>
It may be needed and better than how we keep track of the families - I
was just making a handwaving suggestion; when you code/test it will
become obvious.
> > we use a simple scheme in the case of families; once all
> > IDs are consumed we dont alloc more - in the case of mcast grps this is
> > not necessary IMO i.e it doesnt matter if there is collision when you
> > run out. You return errors at the moment.
>
> Are you saying I should double-allocate groups?
I am not sure "double-allocate" would be the right description.
It sort of like shared-irq or a netdevice running in promisc/bcast mode.
More below.
> I really don't want that
> since I plan to add permission checks on top of this.
It is just a boundary condition policy. When there are no more groups
left (Note: it will take a lot of effort to hit that), then what do you
do?
Your current approach seems to say "return an error". My suggestion is
to just reuse the first or last allocated one or reserve some groups for
sharing and always make sure someone is guaranteed they will get a group
when they ask for it. Infact you can even let the family tell you on
registration whether it is willing to share (and fail allocation if
nothing shareable is available).
Each family when using a shared group will always have a unique name to
itself, so discovery from user space still works - but the id will be
unique across multiple groups. User space code will be like promisc
mode in networking, it will have to filter out the noise. If i ask the
controller to tell me about a family - it will show all group name/id
and a tag whether it is shared or not (that way my user space code knows
it needs to ignore noise it sees).
I am begining to wonder if Evgeniy's connector actually does this - i
dont remember. I am almost certain TIPC also does something similar.
In any case, this is a corner case - the suggestion is how to deal with
it. You can skin it many ways. Toss a coin - pick one.
> > > --- wireless-dev.orig/include/linux/genetlink.h 2007-06-25
> > > 23:56:19.895732308 +0200
> > > +++ wireless-dev/include/linux/genetlink.h2007-06-26
> > > 00:33:36.785732308 +0200
> > > @@ -52,6 +52,9 @@ enum {
> > > CTRL_ATTR_HDRSIZE,
> > > CTRL_ATTR_MAXATTR,
> > > CTRL_ATTR_OPS,
> > > + CTRL_ATTR_MCAST_GROUPS,
> > > + CTRL_ATTR_MCAST_GRP_NAME,
> > > + CTRL_ATTR_MCAST_GRP_ID,
> >
> > Dont think those last two belong in the same namespace. i.e they should
> > be a separate enum; even more the name/id pair probably belong in one
> > TLV under the struct genl_multicast_group that is exported to user
> > space.
>
> Hm? Not sure I understand this. These are attributes for the generic
> netlink controller messages,
Group name/id are nested into CTRL_ATTR_MCAST_GROUPS. Once you nest, you
are starting to a new namespace i.e as good practice you start counting
from 0.
> where else should I put them?
A new enum.
> Why give them
> numbers from a different namespace because they're used inside nested
> attributes?
Sorry - i should have read up to here ;-> Yes, it is because of the
nesting. Again consider the suggestion of sending just one TLV.
> > Overall: I think you are on the right path. Good stuff.
> > I see user space doing a discovery of which groups a family belongs to
> > or even doing a bind-by-name in which the underlying user-space code
> > does a discovery to find the id, then does a bind to that id.
>
> Yeah, that's what I was thinking of, although the bind-by-name needs
> (family-id, group-name) and nost just group-name
Well, you can hide that from the user in the form of a library etc. They
dont have to know; what they know is group named "link-events" is where
they can join to listen to link events (and your abstraction generic
code does all the dirty work).
cheers,
jamal
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote:
> On Wed, 2007-06-27 at 19:24 -0400, jamal wrote:
>
>>c) Use a global hash table to store all the genl_multicast_groups;
>>I think this (handwaving) should be searchable by i) name ii)ID and iii)
>>family.
>
>
> Yeah, makes sense, I never liked the bitmap stuff I did there.
Do multicast groups have to have a seperate name? Or would it suffice
to have them associated with the genl family and be able to find out
the starting group number? In that case something like
struct genl_mc_groups {
struct genl_family *family or char *family_name or similar;
unsigned int group_off;
unsigned int group_num;
unsigned long groups[];
};
seems to make more sense since you only need a single struct
per family.
>>>+static unsigned long mcast_group_start = 0x3;
>>>+static unsigned long *multicast_groups = &mcast_group_start;
>>>+static unsigned long multicast_group_bits = BITS_PER_LONG;
That looks pretty similar.
>>I think if you used a hash table you wont need to keep track of the
>>above; maybe not - You may still need the above to keep track of which
>>IDs are in use and where holes in multicast group ID space exist
>>(assuming some are going to be unregistered over time etc)
Why would you care about holes? If you really want to use sparse
bitmaps that would complicate the code a lot.
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 07:17 -0400, jamal wrote: > > No, the controller is the only other generic netlink multicast user > > according to what I've found. :) > > Scanning the code - true ;-> Iam a little suprised that the task > accounting folks didnt use it to periodically dump stats. :) Because of this I'd really prefer if we could hold off on adding groups, but I can handle both cases fine by just reserving a family and group ID for the current users. > Sorry i meant there are 2^16 families (-1 considering controller) > There are 2^32 groups (-1 considering controller) for the same number of > families. i.e i see those items as being global within genetlink. Yeah, so we shouldn't really be worried about running out of either. > It is just a boundary condition policy. When there are no more groups > left (Note: it will take a lot of effort to hit that), then what do you > do? > Your current approach seems to say "return an error". [suggestion snipped] I think I'd prefer if we just returned an error. See, we aren't going to run out of groups in the foreseeable future, and if we ever have users that generate *a lot* of groups we can still add the sharing code etc. For now it seems just bloat and a code path we won't ever touch so prone to errors in it. > > Why give them > > numbers from a different namespace because they're used inside nested > > attributes? > > Sorry - i should have read up to here ;-> Yes, it is because of the > nesting. Again consider the suggestion of sending just one TLV. Ok :) Though I'm not sure I understood the suggestion of sending just one TLV, what should I send? Or are you referring to dynamic group registration where the whole nesting is going away anyway since you get one message per new group... > Well, you can hide that from the user in the form of a library etc. They > dont have to know; what they know is group named "link-events" is where > they can join to listen to link events (and your abstraction generic > code does all the dirty work). Right. We'll see how it turns out in practice when we start using it :) johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 13:28 +0200, Johannes Berg wrote: > On Fri, 2007-06-29 at 07:17 -0400, jamal wrote: > > Because of this I'd really prefer if we could hold off on adding groups, > but I can handle both cases fine by just reserving a family and group ID > for the current users. > sure - if you rush you can make it into Daves 2.6.23; then both can conform at the same time. > I think I'd prefer if we just returned an error. See, we aren't going to > run out of groups in the foreseeable future, and if we ever have users > that generate *a lot* of groups we can still add the sharing code etc. > For now it seems just bloat and a code path we won't ever touch so prone > to errors in it. > Ok, you are doing the coding and i dont have strong opinions on either; so go for it. > Ok :) Though I'm not sure I understood the suggestion of sending just > one TLV, what should I send? I meant when user space asks the controller "please tell me details about family X" you will always pass an id and name. Never just one. So why not just send that in a struct which has a id/name (as you already have defined in your current patch). cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 07:48 -0400, jamal wrote: > sure - if you rush you can make it into Daves 2.6.23; then both can > conform at the same time. Yeah, I'll have to see whether I can make that. If not, no big deal either. > > Ok :) Though I'm not sure I understood the suggestion of sending just > > one TLV, what should I send? > > I meant when user space asks the controller "please tell me details > about family X" you will always pass an id and name. Never just one. > So why not just send that in a struct which has a id/name (as you > already have defined in your current patch). I'll have to look at the struct stuff again, but yeah, something like that can be done. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 13:51 +0200, Patrick McHardy wrote:
> Do multicast groups have to have a seperate name?
As i see it: the name would be unique per family
Its like DNS IP to name mapping essentially (in the simple case of IP
being globaly reachable). You do a discovery of the ID by knowing the
name.
> Or would it suffice
> to have them associated with the genl family and be able to find out
> the starting group number?
The id space is global.
> In that case something like
>
> struct genl_mc_groups {
> struct genl_family *family or char *family_name or similar;
> unsigned int group_off;
> unsigned int group_num;
> unsigned long groups[];
> };
>
> seems to make more sense since you only need a single struct
> per family.
I think something that ties to the family would be needed.
> >>>+static unsigned long mcast_group_start = 0x3;
> >>>+static unsigned long *multicast_groups = &mcast_group_start;
> >>>+static unsigned long multicast_group_bits = BITS_PER_LONG;
>
>
> That looks pretty similar.
I know-;> when i first saw it i asked myself "Hrm, where have i seen
that before?" ;->
> Why would you care about holes? If you really want to use sparse
> bitmaps that would complicate the code a lot.
similar to ifindices. You want to reuse/recycle them.
cheers,
jamal
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote:
> On Fri, 2007-06-29 at 13:51 +0200, Patrick McHardy wrote:
>
>
>>Do multicast groups have to have a seperate name? Or would it suffice
>>to have them associated with the genl family and be able to find out
>>the starting group number? In that case something like
>>
>>struct genl_mc_groups {
>> struct genl_family *family or char *family_name or similar;
>> unsigned int group_off;
>> unsigned int group_num;
>> unsigned long groups[];
>>};
>>
>>seems to make more sense since you only need a single struct
>>per family.
>
>
> Hm. For me that'd work too but Jamal wanted dynamically allocated groups
> if I understood him correctly. I'm not too concerned with that case, I'd
> think most people know the groups up-front. On the other hand, I can see
> something like a group per netdev or whatever other instance too.
Maybe use a mix. Use the bitmap, but allow families to register
multiple of them. In the common case it would only be a single
one, but it would be possible to register groups dynamically.
>>Why would you care about holes? If you really want to use sparse
>>bitmaps that would complicate the code a lot.
>
>
> No, not sparse bitmaps, but the bitmap could have a hole when a family
> goes away, and we could reuse that group number later. If we have it in
> a bitmap we know without checking all group IDs.
Right.
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
jamal wrote:
> On Fri, 2007-29-06 at 13:51 +0200, Patrick McHardy wrote:
>
>
>>Do multicast groups have to have a seperate name?
>
>
> As i see it: the name would be unique per family
> Its like DNS IP to name mapping essentially (in the simple case of IP
> being globaly reachable). You do a discovery of the ID by knowing the
> name.
Mhh .. I agree that it would be necessary to have a group identifier
for dynamically allocated groups like one group per device or something.
I'm wondering if anyone really needs this though. Having messages
grouped logically by type makes more sense to me.
>>Or would it suffice
>>to have them associated with the genl family and be able to find out
>>the starting group number?
>
>
> The id space is global.
>
>
>>In that case something like
>>
>>struct genl_mc_groups {
>> struct genl_family *family or char *family_name or similar;
>> unsigned int group_off;
>> unsigned int group_num;
>> unsigned long groups[];
>>};
>>
>>seems to make more sense since you only need a single struct
>>per family.
>
>
> I think something that ties to the family would be needed.
My example includes a pointer to the family or the name, but anything
else that works is fine also.
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 13:51 +0200, Patrick McHardy wrote:
> Do multicast groups have to have a seperate name? Or would it suffice
> to have them associated with the genl family and be able to find out
> the starting group number? In that case something like
>
> struct genl_mc_groups {
> struct genl_family *family or char *family_name or similar;
> unsigned int group_off;
> unsigned int group_num;
> unsigned long groups[];
> };
>
> seems to make more sense since you only need a single struct
> per family.
Hm. For me that'd work too but Jamal wanted dynamically allocated groups
if I understood him correctly. I'm not too concerned with that case, I'd
think most people know the groups up-front. On the other hand, I can see
something like a group per netdev or whatever other instance too.
> Why would you care about holes? If you really want to use sparse
> bitmaps that would complicate the code a lot.
No, not sparse bitmaps, but the bitmap could have a hole when a family
goes away, and we could reuse that group number later. If we have it in
a bitmap we know without checking all group IDs.
johannes
signature.asc
Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote:
> On Fri, 2007-06-29 at 13:51 +0200, Patrick McHardy wrote:
>
>
>>Do multicast groups have to have a seperate name? Or would it suffice
>>to have them associated with the genl family and be able to find out
>>the starting group number? In that case something like
>>
>>struct genl_mc_groups {
>> struct genl_family *family or char *family_name or similar;
>> unsigned int group_off;
>> unsigned int group_num;
>> unsigned long groups[];
>>};
>>
>>seems to make more sense since you only need a single struct
>>per family.
>
>
> Hmm, another thought: since we have 32 bits for group numbers and 16
> bits for families we could just reserve 16 bits for groups within each
> family. Or do we get trouble with that because in some place there's a
> group bitmap or such?
Yes, af_netlink has a bitmap per socket that is subscribed to any group.
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 14:48 +0200, Patrick McHardy wrote: > > Hmm, another thought: since we have 32 bits for group numbers and 16 > > bits for families we could just reserve 16 bits for groups within each > > family. Or do we get trouble with that because in some place there's a > > group bitmap or such? > > > Yes, af_netlink has a bitmap per socket that is subscribed to any group. Not a good idea then. I'll see what I can do. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 13:51 +0200, Patrick McHardy wrote:
> Do multicast groups have to have a seperate name? Or would it suffice
> to have them associated with the genl family and be able to find out
> the starting group number? In that case something like
>
> struct genl_mc_groups {
> struct genl_family *family or char *family_name or similar;
> unsigned int group_off;
> unsigned int group_num;
> unsigned long groups[];
> };
>
> seems to make more sense since you only need a single struct
> per family.
Hmm, another thought: since we have 32 bits for group numbers and 16
bits for families we could just reserve 16 bits for groups within each
family. Or do we get trouble with that because in some place there's a
group bitmap or such?
johannes
signature.asc
Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Wed, 2007-06-27 at 19:24 -0400, jamal wrote: > a) i.e other than the reserved group for controller (which you seem to > be taking care of), every other genetlink user has to explicitly > register when they need a mcast group. Hm. I'm starting to dislike the dynamic registration the more I think about it. Now when a group is unregistered I'd have to unbind everybody who's currently using it... At least when I want to enforce root/non-root binds which is a further goal. > c) Use a global hash table to store all the genl_multicast_groups; > I think this (handwaving) should be searchable by i) name ii)ID and iii) > family. Also, it doesn't look like I'll ever be searching for a group unless we want to allow userspace to explicitly ask for a group ID, but I'd think it should just get the list of all IDs when asking for family information and then cache that. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 09:02 -0400, jamal wrote: > Maybe a mix (of a few static and mostly dynamic) as Patrick says - but > that would mean more coding for you ;-> Actually i like the idea of at > least your ID being your static mcast group and the rest are in the > dynamic pool (Hey, thanks Patrick;->). This means the first 2^16 are > static/reserved and if you want more groups, you register for them. Hah, that means I get to rewrite af_netlink.c to not have a bitmap since that'd mean that with just a single dynamically registered multicast group it gets ID 2^16 which means an 8 kilobyte bitmap... johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
jamal wrote: > On Fri, 2007-29-06 at 14:48 +0200, Patrick McHardy wrote: > >>Johannes Berg wrote: > > >>>Hmm, another thought: since we have 32 bits for group numbers and 16 >>>bits for families we could just reserve 16 bits for groups within each >>>family. Or do we get trouble with that because in some place there's a >>>group bitmap or such? >> >> >>Yes, af_netlink has a bitmap per socket that is subscribed to any group. > > > > I think this is the challenge. The groups belong to a global namespace. > i.e when you do a socket bind to group - it is unique regardless of the > family. > Our philosophy in genetlink is to have dynamic resources allocated and > released - remember the real reason we even have this is because we were > running out of numbers ;-> That was more of a rumour :) We have 2^32-1 groups and I think 256 families, of which about 20 are used. > So while the static allocation of 16 bits per group will work (famous > last words "noone will ever need more than 640K of RAM";->) it will be > cleaner imo to allow dynamic allocation/release. > Maybe a mix (of a few static and mostly dynamic) as Patrick says - but > that would mean more coding for you ;-> Actually i like the idea of at > least your ID being your static mcast group and the rest are in the > dynamic pool (Hey, thanks Patrick;->). This means the first 2^16 are > static/reserved and if you want more groups, you register for them. I wouldn't reserve any, just hand them out as needed. Otherwise we'll run into problems with the af_netlink bitmaps. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 15:15 +0200, Johannes Berg wrote: > (1) register group X with non-root > (2) non-root app A binds group X > (3) kernel unregisters group X Kernel sends event (controller) "Group X is gone" or "family Y which used to own group X is gone" > (4) something else in kernel reregisters group ID X but root-only > -> non-root app A is bound to root-only group X Kernel (controller genetlink) sends event for that too. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 14:57 +0200, Johannes Berg wrote: > On Wed, 2007-06-27 at 19:24 -0400, jamal wrote: > Hm. I'm starting to dislike the dynamic registration the more I think > about it. Now when a group is unregistered I'd have to unbind everybody > who's currently using it... I understood you earlier to say only one family (in kernel) can own a group else it is an error, no? So there is "somebody" but not "everybody" BTW, refer to my earlier email in response to Patrick. I think preserving a group per family is also an interesting idea. i.e the first 2^16 are reserved/static and the others are in all for grabs area. > At least when I want to enforce > root/non-root binds which is a further goal. Permissions would be group attributes - maybe something in the group struct to say who is allowed to bind? > > c) Use a global hash table to store all the genl_multicast_groups; > > I think this (handwaving) should be searchable by i) name ii)ID and iii) > > family. > > Also, it doesn't look like I'll ever be searching for a group unless we > want to allow userspace to explicitly ask for a group ID, but I'd think > it should just get the list of all IDs when asking for family > information and then cache that. I think its useful to have the filters in the kernel when possible/sensible (one of my beefs against netlink in general is it lacks that). cheers, jamal > 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 14:48 +0200, Patrick McHardy wrote: > Johannes Berg wrote: > > Hmm, another thought: since we have 32 bits for group numbers and 16 > > bits for families we could just reserve 16 bits for groups within each > > family. Or do we get trouble with that because in some place there's a > > group bitmap or such? > > > Yes, af_netlink has a bitmap per socket that is subscribed to any group. I think this is the challenge. The groups belong to a global namespace. i.e when you do a socket bind to group - it is unique regardless of the family. Our philosophy in genetlink is to have dynamic resources allocated and released - remember the real reason we even have this is because we were running out of numbers ;-> So while the static allocation of 16 bits per group will work (famous last words "noone will ever need more than 640K of RAM";->) it will be cleaner imo to allow dynamic allocation/release. Maybe a mix (of a few static and mostly dynamic) as Patrick says - but that would mean more coding for you ;-> Actually i like the idea of at least your ID being your static mcast group and the rest are in the dynamic pool (Hey, thanks Patrick;->). This means the first 2^16 are static/reserved and if you want more groups, you register for them. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Wed, 2007-06-27 at 19:24 -0400, jamal wrote: > > >>a) i.e other than the reserved group for controller (which you seem to >>be taking care of), every other genetlink user has to explicitly >>register when they need a mcast group. > > > Hm. I'm starting to dislike the dynamic registration the more I think > about it. Now when a group is unregistered I'd have to unbind everybody > who's currently using it... At least when I want to enforce > root/non-root binds which is a further goal. How about using module references to prevent unloading while sockets are bound? We already do the same thing for netlink families. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 15:11 +0200, Patrick McHardy wrote: > > Hm. I'm starting to dislike the dynamic registration the more I think > > about it. Now when a group is unregistered I'd have to unbind everybody > > who's currently using it... At least when I want to enforce > > root/non-root binds which is a further goal. > > > How about using module references to prevent unloading while sockets > are bound? We already do the same thing for netlink families. Ah, no, you're both thinking something different. I'm thinking if we allow dynamic registration the only sensible thing is to have dynamic unregistration as well, and then we can have this sequence (1) register group X with non-root (2) non-root app A binds group X (3) kernel unregisters group X (4) something else in kernel reregisters group ID X but root-only -> non-root app A is bound to root-only group X johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Fri, 2007-06-29 at 15:11 +0200, Patrick McHardy wrote: > > >>>Hm. I'm starting to dislike the dynamic registration the more I think >>>about it. Now when a group is unregistered I'd have to unbind everybody >>>who's currently using it... At least when I want to enforce >>>root/non-root binds which is a further goal. >> >> >>How about using module references to prevent unloading while sockets >>are bound? We already do the same thing for netlink families. > > > Ah, no, you're both thinking something different. I'm thinking if we > allow dynamic registration the only sensible thing is to have dynamic > unregistration as well, and then we can have this sequence > > (1) register group X with non-root > (2) non-root app A binds group X > (3) kernel unregisters group X > (4) something else in kernel reregisters group ID X but root-only > -> non-root app A is bound to root-only group X I'm not sure that "the only sensible thing to do" is right, we do allow dynamic registration of netlink families and do the module reference thing anyway (admittedly, I never liked that and the autoloading part very much). I guess it depends on how this will be used in the end, if you really do have a group per device or something like that you probably need to be able to unregister at any time. But as I said previously I believe its more in the spirit of netlink to group things logically by message type, in which case some core part would own the family and not a single device. If you do want the dynamic unregistation *and* the non-root mc listening then I guess you don't have a choice but to unbind sockets at unregistration. That shouln't be a real problem, without having though much about it, I believe just clearing the mc group from the bitmap and calling netlink_update_subscriptions should be fine. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
jamal wrote: > On Fri, 2007-29-06 at 15:12 +0200, Patrick McHardy wrote: > >>jamal wrote: >> >>>On Fri, 2007-29-06 at 14:48 +0200, Patrick McHardy wrote: > > >>>Our philosophy in genetlink is to have dynamic resources allocated and >>>released - remember the real reason we even have this is because we were >>>running out of numbers ;-> >> >> >>That was more of a rumour :) We have 2^32-1 groups and I think 256 >>families, of which about 20 are used. > > > Well, if thats not the case - we need to fix it ;-> I meant the "running out of numbers" part, we have plenty left. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 15:23 +0200, Patrick McHardy wrote: > I'm not sure that "the only sensible thing to do" is right, we > do allow dynamic registration of netlink families and do the > module reference thing anyway (admittedly, I never liked that > and the autoloading part very much). I guess it depends on how > this will be used in the end, if you really do have a group per > device or something like that you probably need to be able to > unregister at any time. But as I said previously I believe its > more in the spirit of netlink to group things logically by > message type, in which case some core part would own the > family and not a single device. > > If you do want the dynamic unregistation *and* the non-root mc > listening then I guess you don't have a choice but to unbind > sockets at unregistration. That shouln't be a real problem, > without having though much about it, I believe just clearing > the mc group from the bitmap and calling netlink_update_subscriptions > should be fine. Yeah, true, but the patch gets larger with every little thing here :) How about for now I only allow dynamic registration (no unregistration) and just send out when new groups are registered, and also give userspace a list of registered mc groups when they ask for a family description? That should make this patch not too big and still leaves room for dynamic unregistration later. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Fri, 2007-06-29 at 15:23 +0200, Patrick McHardy wrote: > >>If you do want the dynamic unregistation *and* the non-root mc >>listening then I guess you don't have a choice but to unbind >>sockets at unregistration. That shouln't be a real problem, >>without having though much about it, I believe just clearing >>the mc group from the bitmap and calling netlink_update_subscriptions >>should be fine. > > > Yeah, true, but the patch gets larger with every little thing here :) Just do a seperate patch to add mc unregistation to af_netlink. > How about for now I only allow dynamic registration (no unregistration) > and just send out when new groups are registered, and also give > userspace a list of registered mc groups when they ask for a family > description? That should make this patch not too big and still leaves > room for dynamic unregistration later. How does it deal with unregistration currently? If it leaves sockets subscribed that seems like a bug already, at least if the id was dynamically generated. Of course it would require the id to be reused to actually matter. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 15:44 +0200, Patrick McHardy wrote: > > How about for now I only allow dynamic registration (no unregistration) > > and just send out when new groups are registered, and also give > > userspace a list of registered mc groups when they ask for a family > > description? That should make this patch not too big and still leaves > > room for dynamic unregistration later. > > > How does it deal with unregistration currently? If it leaves sockets > subscribed that seems like a bug already, at least if the id was > dynamically generated. Of course it would require the id to be > reused to actually matter. Hmm. I don't see it kicking out the socket subscriptions in genl_unregister_family so I guess that bug is present. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Fri, 2007-06-29 at 15:44 +0200, Patrick McHardy wrote: > > >>>How about for now I only allow dynamic registration (no unregistration) >>>and just send out when new groups are registered, and also give >>>userspace a list of registered mc groups when they ask for a family >>>description? That should make this patch not too big and still leaves >>>room for dynamic unregistration later. >> >> >>How does it deal with unregistration currently? If it leaves sockets >>subscribed that seems like a bug already, at least if the id was >>dynamically generated. Of course it would require the id to be >>reused to actually matter. > > > Hmm. I don't see it kicking out the socket subscriptions in > genl_unregister_family so I guess that bug is present. If you're worried about patch size, you could sell that part as a bugfix :) - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 15:12 +0200, Patrick McHardy wrote: > jamal wrote: > > On Fri, 2007-29-06 at 14:48 +0200, Patrick McHardy wrote: > > Our philosophy in genetlink is to have dynamic resources allocated and > > released - remember the real reason we even have this is because we were > > running out of numbers ;-> > > > That was more of a rumour :) We have 2^32-1 groups and I think 256 > families, of which about 20 are used. Well, if thats not the case - we need to fix it ;-> Anyways, I have to run off. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 15:53 +0200, Patrick McHardy wrote:
> If you're worried about patch size, you could sell that part as a
> bugfix :)
Heh. Actually, right now I'm more worried about how much work I have to
do short-term.
This patch keeps the bitmap but does dynamic group allocation. Just to
see where I'm right now, I have a few things I'd like to change like not
sending out the full family info if it registers a new group.
---
include/linux/genetlink.h | 13 ++
include/net/genetlink.h | 18 +
net/netlink/genetlink.c | 89 ++
3 files changed, 120 insertions(+)
--- wireless-dev.orig/include/net/genetlink.h 2007-06-26 16:58:42.154806179
+0200
+++ wireless-dev/include/net/genetlink.h2007-06-29 15:40:35.547910932
+0200
@@ -5,6 +5,20 @@
#include
/**
+ * struct genl_multicast_group - generic netlink multicast group
+ * @name: name of the multicast group, names are per-family
+ * @id: multicast group ID, assigned by the core, to use with
+ * genlmsg_multicast().
+ * @list: list entry for linking
+ */
+struct genl_multicast_group
+{
+ struct list_headlist; /* private */
+ charname[GENL_NAMSIZ];
+ u32 id;
+};
+
+/**
* struct genl_family - generic netlink family
* @id: protocol family idenfitier
* @hdrsize: length of user specific header in bytes
@@ -14,6 +28,7 @@
* @attrbuf: buffer to store parsed attributes
* @ops_list: list of all assigned operations
* @family_list: family list
+ * @mcast_groups: multicast groups list
*/
struct genl_family
{
@@ -25,6 +40,7 @@ struct genl_family
struct nlattr **attrbuf;/* private */
struct list_headops_list; /* private */
struct list_headfamily_list;/* private */
+ struct list_headmcast_groups; /* private */
};
/**
@@ -73,6 +89,8 @@ extern int genl_register_family(struct g
extern int genl_unregister_family(struct genl_family *family);
extern int genl_register_ops(struct genl_family *, struct genl_ops *ops);
extern int genl_unregister_ops(struct genl_family *, struct genl_ops *ops);
+extern int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp);
extern struct sock *genl_sock;
--- wireless-dev.orig/net/netlink/genetlink.c 2007-06-26 16:58:42.244806179
+0200
+++ wireless-dev/net/netlink/genetlink.c2007-06-29 16:01:38.487910932
+0200
@@ -3,6 +3,7 @@
*
* Authors:Jamal Hadi Salim
* Thomas Graf <[EMAIL PROTECTED]>
+ * Johannes Berg <[EMAIL PROTECTED]>
*/
#include
@@ -13,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,6 +44,16 @@ static void genl_unlock(void)
#define GENL_FAM_TAB_MASK (GENL_FAM_TAB_SIZE - 1)
static struct list_head family_ht[GENL_FAM_TAB_SIZE];
+/*
+ * Bitmap of multicast groups that are currently in use.
+ *
+ * To avoid an allocation at boot of just one unsigned long,
+ * declare it global instead.
+ * Bit 1 is marked as already used since group 1 is the controller group.
+ */
+static unsigned long mc_group_start = 0x2;
+static unsigned long *mc_groups = &mc_group_start;
+static unsigned long mc_groups_bits = BITS_PER_LONG;
static int genl_ctrl_event(int event, void *data);
@@ -116,6 +128,55 @@ static inline u16 genl_generate_id(void)
return id_gen_idx;
}
+int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp)
+{
+ int id = find_first_zero_bit(mc_groups, mc_groups_bits);
+
+ if (id >= mc_groups_bits) {
+ if (mc_groups == &mc_group_start) {
+ mc_groups =
+ kzalloc(mc_groups_bits/BITS_PER_LONG + 1,
+ GFP_KERNEL);
+ if (!mc_groups)
+ return -ENOMEM;
+ *mc_groups = mc_group_start;
+ } else {
+ mc_groups =
+ krealloc(mc_groups,
+mc_groups_bits/BITS_PER_LONG + 1,
+GFP_KERNEL);
+ if (!mc_groups)
+ return -ENOMEM;
+ mc_groups[mc_groups_bits/BITS_PER_LONG] = 0;
+ }
+ mc_groups_bits += BITS_PER_LONG;
+ }
+
+ grp->id = id;
+ set_bit(id, mc_groups);
+ list_add_tail(&grp->list, &family->mcast_groups);
+
+ genl_ctrl_event(CTRL_CMD_NEWMCAST_GRP, family);
+
+ return 0;
+}
+EXPORT_SYMBOL(genl_register_mc_group);
+
+static void genl_unregister_mcast_group(struct genl_multicast_group *grp)
+{
+ clear_bit(grp->id, mc_groups);
+ list_del(&grp->list)
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 16:05 +0200, Johannes Berg wrote:
> + mc_groups =
> + kzalloc(mc_groups_bits/BITS_PER_LONG + 1,
> + GFP_KERNEL);
> + if (!mc_groups)
> + return -ENOMEM;
> + *mc_groups = mc_group_start;
Uh huh, this reallocation is buggy. Fixed version below.
---
include/linux/genetlink.h | 13 ++
include/net/genetlink.h | 18 +
net/netlink/genetlink.c | 90 ++
3 files changed, 121 insertions(+)
--- wireless-dev.orig/include/net/genetlink.h 2007-06-26 16:58:42.154806179
+0200
+++ wireless-dev/include/net/genetlink.h2007-06-29 15:40:35.547910932
+0200
@@ -5,6 +5,20 @@
#include
/**
+ * struct genl_multicast_group - generic netlink multicast group
+ * @name: name of the multicast group, names are per-family
+ * @id: multicast group ID, assigned by the core, to use with
+ * genlmsg_multicast().
+ * @list: list entry for linking
+ */
+struct genl_multicast_group
+{
+ struct list_headlist; /* private */
+ charname[GENL_NAMSIZ];
+ u32 id;
+};
+
+/**
* struct genl_family - generic netlink family
* @id: protocol family idenfitier
* @hdrsize: length of user specific header in bytes
@@ -14,6 +28,7 @@
* @attrbuf: buffer to store parsed attributes
* @ops_list: list of all assigned operations
* @family_list: family list
+ * @mcast_groups: multicast groups list
*/
struct genl_family
{
@@ -25,6 +40,7 @@ struct genl_family
struct nlattr **attrbuf;/* private */
struct list_headops_list; /* private */
struct list_headfamily_list;/* private */
+ struct list_headmcast_groups; /* private */
};
/**
@@ -73,6 +89,8 @@ extern int genl_register_family(struct g
extern int genl_unregister_family(struct genl_family *family);
extern int genl_register_ops(struct genl_family *, struct genl_ops *ops);
extern int genl_unregister_ops(struct genl_family *, struct genl_ops *ops);
+extern int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp);
extern struct sock *genl_sock;
--- wireless-dev.orig/net/netlink/genetlink.c 2007-06-26 16:58:42.244806179
+0200
+++ wireless-dev/net/netlink/genetlink.c2007-06-29 16:14:06.307910932
+0200
@@ -3,6 +3,7 @@
*
* Authors:Jamal Hadi Salim
* Thomas Graf <[EMAIL PROTECTED]>
+ * Johannes Berg <[EMAIL PROTECTED]>
*/
#include
@@ -13,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,6 +44,16 @@ static void genl_unlock(void)
#define GENL_FAM_TAB_MASK (GENL_FAM_TAB_SIZE - 1)
static struct list_head family_ht[GENL_FAM_TAB_SIZE];
+/*
+ * Bitmap of multicast groups that are currently in use.
+ *
+ * To avoid an allocation at boot of just one unsigned long,
+ * declare it global instead.
+ * Bit 1 is marked as already used since group 1 is the controller group.
+ */
+static unsigned long mc_group_start = 0x2;
+static unsigned long *mc_groups = &mc_group_start;
+static unsigned long mc_groups_bits = BITS_PER_LONG;
static int genl_ctrl_event(int event, void *data);
@@ -116,6 +128,56 @@ static inline u16 genl_generate_id(void)
return id_gen_idx;
}
+int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp)
+{
+ int id = find_first_zero_bit(mc_groups, mc_groups_bits);
+ unsigned long *new_groups;
+
+ if (id >= mc_groups_bits) {
+ if (mc_groups == &mc_group_start) {
+ new_groups = kzalloc(mc_groups_bits/BITS_PER_LONG + 1,
+GFP_KERNEL);
+ if (!mc_groups)
+ return -ENOMEM;
+ mc_groups = new_groups;
+ *mc_groups = mc_group_start;
+ } else {
+ new_groups = krealloc(mc_groups,
+ mc_groups_bits/BITS_PER_LONG + 1,
+ GFP_KERNEL);
+ if (!new_groups)
+ return -ENOMEM;
+ mc_groups = new_groups;
+ mc_groups[mc_groups_bits/BITS_PER_LONG] = 0;
+ }
+ mc_groups_bits += BITS_PER_LONG;
+ }
+
+ grp->id = id;
+ set_bit(id, mc_groups);
+ list_add_tail(&grp->list, &family->mcast_groups);
+
+ genl_ctrl_event(CTRL_CMD_NEWMCAST_GRP, family);
+
+ return 0;
+}
+EXPORT_SYMBOL(genl_register_mc_group);
+
+static void genl_unregister_mcast_group(struct genl_multicast_group *grp)
+
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-06-29 at 16:19 +0200, Johannes Berg wrote:
> Uh huh, this reallocation is buggy. Fixed version below.
More breakage, sorry about the patch-spam.
Btw, I notice that the bug we talked about isn't present in practice
since we have no multicast users except for the always-present
controller :)
New patch below. I'll get this tested and maybe a bit nicer and I'll try
to fix the bug too, but not before the weekend.
---
include/linux/genetlink.h | 13 ++
include/net/genetlink.h | 18
net/netlink/genetlink.c | 93 ++
3 files changed, 124 insertions(+)
--- wireless-dev.orig/include/net/genetlink.h 2007-06-26 16:58:42.154806179
+0200
+++ wireless-dev/include/net/genetlink.h2007-06-29 15:40:35.547910932
+0200
@@ -5,6 +5,20 @@
#include
/**
+ * struct genl_multicast_group - generic netlink multicast group
+ * @name: name of the multicast group, names are per-family
+ * @id: multicast group ID, assigned by the core, to use with
+ * genlmsg_multicast().
+ * @list: list entry for linking
+ */
+struct genl_multicast_group
+{
+ struct list_headlist; /* private */
+ charname[GENL_NAMSIZ];
+ u32 id;
+};
+
+/**
* struct genl_family - generic netlink family
* @id: protocol family idenfitier
* @hdrsize: length of user specific header in bytes
@@ -14,6 +28,7 @@
* @attrbuf: buffer to store parsed attributes
* @ops_list: list of all assigned operations
* @family_list: family list
+ * @mcast_groups: multicast groups list
*/
struct genl_family
{
@@ -25,6 +40,7 @@ struct genl_family
struct nlattr **attrbuf;/* private */
struct list_headops_list; /* private */
struct list_headfamily_list;/* private */
+ struct list_headmcast_groups; /* private */
};
/**
@@ -73,6 +89,8 @@ extern int genl_register_family(struct g
extern int genl_unregister_family(struct genl_family *family);
extern int genl_register_ops(struct genl_family *, struct genl_ops *ops);
extern int genl_unregister_ops(struct genl_family *, struct genl_ops *ops);
+extern int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp);
extern struct sock *genl_sock;
--- wireless-dev.orig/net/netlink/genetlink.c 2007-06-26 16:58:42.244806179
+0200
+++ wireless-dev/net/netlink/genetlink.c2007-06-29 16:48:35.007910932
+0200
@@ -3,6 +3,7 @@
*
* Authors:Jamal Hadi Salim
* Thomas Graf <[EMAIL PROTECTED]>
+ * Johannes Berg <[EMAIL PROTECTED]>
*/
#include
@@ -13,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,6 +44,16 @@ static void genl_unlock(void)
#define GENL_FAM_TAB_MASK (GENL_FAM_TAB_SIZE - 1)
static struct list_head family_ht[GENL_FAM_TAB_SIZE];
+/*
+ * Bitmap of multicast groups that are currently in use.
+ *
+ * To avoid an allocation at boot of just one unsigned long,
+ * declare it global instead.
+ * Bit 1 is marked as already used since group 1 is the controller group.
+ */
+static unsigned long mc_group_start = 0x2;
+static unsigned long *mc_groups = &mc_group_start;
+static unsigned long mc_groups_longs = 1;
static int genl_ctrl_event(int event, void *data);
@@ -116,6 +128,59 @@ static inline u16 genl_generate_id(void)
return id_gen_idx;
}
+int genl_register_mc_group(struct genl_family *family,
+ struct genl_multicast_group *grp)
+{
+ int id = find_first_zero_bit(mc_groups,
+mc_groups_longs * BITS_PER_LONG);
+ unsigned long *new_groups;
+ size_t nlen = (mc_groups_longs + 1) * sizeof(unsigned long);
+
+ if (id >= mc_groups_longs * BITS_PER_LONG) {
+ if (mc_groups == &mc_group_start) {
+ new_groups = kzalloc(nlen, GFP_KERNEL);
+ if (!mc_groups)
+ return -ENOMEM;
+ mc_groups = new_groups;
+ *mc_groups = mc_group_start;
+ } else {
+ new_groups = krealloc(mc_groups, nlen, GFP_KERNEL);
+ if (!new_groups)
+ return -ENOMEM;
+ mc_groups = new_groups;
+ mc_groups[mc_groups_longs] = 0;
+ }
+ mc_groups_longs++;
+ }
+
+ grp->id = id;
+ set_bit(id, mc_groups);
+ list_add_tail(&grp->list, &family->mcast_groups);
+
+ genl_ctrl_event(CTRL_CMD_NEWMCAST_GRP, family);
+
+ return 0;
+}
+EXPORT_SYMBOL(genl_register_mc_group);
+
+static void genl_unregister_mc_group(struct genl_multicast_group *grp)
+{
+ /*
+* TODO: fix multicast group re-use by clearing the bit
+
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-29-06 at 16:56 +0200, Johannes Berg wrote:
> +static void genl_unregister_mc_group(struct genl_multicast_group *grp)
> +{
> + /*
> + * TODO: fix multicast group re-use by clearing the bit
> + * for this group in all genetlink sockets.
> + */
> + clear_bit(grp->id, mc_groups);
> + list_del(&grp->list);
I think you need a
genl_ctrl_event(CTRL_CMD_DELMCAST_GRP, family);
here? You may need to save the mcast details before you delete.
> + NLA_PUT_U32(skb, CTRL_ATTR_MCAST_GRP_ID, grp->id);
> + NLA_PUT_STRING(skb, CTRL_ATTR_MCAST_GRP_NAME,
> +grp->name);
> +
Consider my earlier suggestion to use CTRL_ATTR_MCAST_GRP which has both
id and name in one struct.
Other than that - looking good.
cheers,
jamal
-
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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Sat, 2007-06-30 at 11:32 -0400, jamal wrote:
> > +static void genl_unregister_mc_group(struct genl_multicast_group *grp)
> > +{
> > + /*
> > +* TODO: fix multicast group re-use by clearing the bit
> > +* for this group in all genetlink sockets.
> > +*/
> > + clear_bit(grp->id, mc_groups);
> > + list_del(&grp->list);
>
> I think you need a
> genl_ctrl_event(CTRL_CMD_DELMCAST_GRP, family);
> here? You may need to save the mcast details before you delete.
I will, but not currently because right now mcast groups are only
deleted when the family is dropped.
> > + NLA_PUT_U32(skb, CTRL_ATTR_MCAST_GRP_ID, grp->id);
> > + NLA_PUT_STRING(skb, CTRL_ATTR_MCAST_GRP_NAME,
> > + grp->name);
> > +
>
> Consider my earlier suggestion to use CTRL_ATTR_MCAST_GRP which has both
> id and name in one struct.
Yeah I thought about that but then saw Patrick's patches to convert
other things away from structs so I wasn't sure what the idea here is.
Patrick, care to comment?
> Other than that - looking good.
:)
johannes
signature.asc
Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Sat, 2007-06-30 at 11:32 -0400, jamal wrote: > > >>>+NLA_PUT_U32(skb, CTRL_ATTR_MCAST_GRP_ID, grp->id); >>>+NLA_PUT_STRING(skb, CTRL_ATTR_MCAST_GRP_NAME, >>>+ grp->name); >>>+ >> >>Consider my earlier suggestion to use CTRL_ATTR_MCAST_GRP which has both >>id and name in one struct. > > > Yeah I thought about that but then saw Patrick's patches to convert > other things away from structs so I wasn't sure what the idea here is. > Patrick, care to comment? For information that belongs together logically a struct is fine. The main reason to use nested attributes is when you only have a single attribute to store your data in (for example TCA_OPTIONS for qdiscs). In that case a nested attribute should be used to allow to extend it in the future. Below that nested attribute you could put a struct of course. In this case I think using a string attribute instead of a fixed sized structure also makes sense for a different reason. Its unlikely that groups will really use the maximum name length allowed, so it should save some bandwidth. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-07-02 at 14:56 +0200, Patrick McHardy wrote: > For information that belongs together logically a struct is fine. Ok. > The main reason to use nested attributes is when you only have a > single attribute to store your data in (for example TCA_OPTIONS > for qdiscs). In that case a nested attribute should be used to > allow to extend it in the future. Below that nested attribute > you could put a struct of course. Right, but that's not applicable to this unless I'm misunderstanding you. > In this case I think using a string attribute instead of a fixed > sized structure also makes sense for a different reason. Its > unlikely that groups will really use the maximum name length > allowed, so it should save some bandwidth. I suppose if I put (ID,name) into the struct it needn't be fixed-size length, but I dislike that as well. Do I understand you correctly in that you prefer the way I did it now? johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Mon, 2007-07-02 at 14:56 +0200, Patrick McHardy wrote: > >>The main reason to use nested attributes is when you only have a >>single attribute to store your data in (for example TCA_OPTIONS >>for qdiscs). In that case a nested attribute should be used to >>allow to extend it in the future. Below that nested attribute >>you could put a struct of course. > > > Right, but that's not applicable to this unless I'm misunderstanding > you. Not really, it already uses a nested top-level attribute. >>In this case I think using a string attribute instead of a fixed >>sized structure also makes sense for a different reason. Its >>unlikely that groups will really use the maximum name length >>allowed, so it should save some bandwidth. > > > I suppose if I put (ID,name) into the struct it needn't be fixed-size > length, but I dislike that as well. Me too. > Do I understand you correctly in that you prefer the way I did it now? Yes. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-07-02 at 16:38 +0200, Patrick McHardy wrote: > > Do I understand you correctly in that you prefer the way I did it now? > > > Yes. Alright, then I'll rework the event generation to not include the whole family info and repost with that tomorrow or so. If I find time I might actually fix the unregistration bug too, but I have a feeling digging in the socket code might take more time than I have right now. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-07-02 at 16:48 +0200, Johannes Berg wrote: > If I find time I might > actually fix the unregistration bug too, but I have a feeling digging in > the socket code might take more time than I have right now. Hmm. I started digging into the af_netlink.c code and realised that the whole thing I've been doing cannot possibly work completely since the genl socket is created with GENL_MAX_ID as the "groups" parameter to netlink_kernel_create() and that limits the groups, and the af_netlink code really wants to know the number of groups up-front. So some deeper surgery is required to lift the limit of 1023 multicast group now. Not that I like the current genetlink code, we allocate 256 bytes for the in-kernel socket just for the listeners bitmap, and just as many for each socket's groups bitmaps while it's unlikely a regular system right now will ever reach that limit. I'll be posting some patches as replies. johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Johannes Berg wrote: > On Mon, 2007-07-02 at 16:48 +0200, Johannes Berg wrote: > > >>If I find time I might >>actually fix the unregistration bug too, but I have a feeling digging in >>the socket code might take more time than I have right now. > > > Hmm. I started digging into the af_netlink.c code and realised that the > whole thing I've been doing cannot possibly work completely since the > genl socket is created with GENL_MAX_ID as the "groups" parameter to > netlink_kernel_create() and that limits the groups, and the af_netlink > code really wants to know the number of groups up-front. Good point, I missed that. > So some deeper surgery is required to lift the limit of 1023 multicast > group now. Not that I like the current genetlink code, we allocate 256 > bytes for the in-kernel socket just for the listeners bitmap, and just > as many for each socket's groups bitmaps while it's unlikely a regular > system right now will ever reach that limit. The kernel socket doesn't have a bitmap allocated, only user sockets have, and only if they actually bind to a multicast group. In case of genetlink that still almost as bad of course. - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
Hi, Jamal, Now the genl utility can find the acpi event genetlink family. And a simple user space demo is finished for handling acpi event. I really appreciate your help. :) I think the patch which exposes ACPI events via netlink is ok. But I still have some problems on how to listen to specified genetlink family in user space? I can get the dynamic id for "acpi_event" genl family. But I don't know how to use this to receive messages from specified genl family. It seems that "#genl ctrl monitor" has something to do with this, IMO, rtnl_open_byproto(&rth, nl_mgrp(GENL_ID_CTRL), NETLINK_GENERIC) is used to receive messages from the nlctrl(controller) only, but unfortunately it never works for me. :( Any suggestions? What interfaces should I use? Or where can I find some example code? Attachment is the simple user space demo I made. It receives all the broadcasted genetlink messages and only parses the ones sent by "acpi_event" genl family. Thanks, Rui On Sun, 2007-05-27 at 09:34 -0400, jamal wrote: > On 5/27/07, Zhang Rui <[EMAIL PROTECTED]> wrote: > > > > I need to write a user application to test my patch. > > Netlink messages can be sent/received using the standard socket API. > > sure. > > > But how to receive Genetlink messages from specified genetlink family? > > There is no socket ACPI with such a parameter, right? > > Each module has a unique identifier that it receives dynamically on > insertion at the kernel. > > > Do I have to receive all the genetlink messages first? > > No, just the ones for your dynamic id. Try what i described first for > kernel side on the earlier email. I will repeat it here for clarity. > Then look at genl code and if you have questions i can > help. > Note: You need to discover your dynamic id (the iproute2/genl code has a stub > example code) > As i told you in the earlier email, in your development: > - start first by just writting your kernel side. > - Then use the genl utility - which is part of iproute2 to see if the > kernel side is "discoverable". > > E.g if i wanted to "discover" currently loaded modules on my laptop, i > would do this: > > --- > [EMAIL PROTECTED]:~$ genl ctrl ls > > Name: nlctrl > ID: 0x10 Version: 0x2 header size: 0 max attribs: 6 > commands supported: > #1: ID-0x3 flags-0xe > > > Name: nl80211 > ID: 0x11 Version: 0x1 header size: 0 max attribs: 22 > commands supported: > #1: ID-0x1 flags-0xa > #2: ID-0x6 flags-0xa > #3: ID-0x8 flags-0xa > #4: ID-0x3 flags-0xb > #5: ID-0x4 flags-0xb > #6: ID-0x5 flags-0xb > #7: ID-0xa flags-0xb > #8: ID-0xb flags-0xa > #9: ID-0xf flags-0xb > #10: ID-0x10 flags-0xa > #11: ID-0x12 flags-0xb > #12: ID-0x13 flags-0xa > #13: ID-0x15 flags-0xa > #14: ID-0x19 flags-0xb > #15: ID-0x17 flags-0xb > #16: ID-0x18 flags-0xb > #17: ID-0x1a flags-0xb > #18: ID-0x1b flags-0xa > #19: ID-0xd flags-0xb > > > Name: TASKSTATS > ID: 0x12 Version: 0x1 header size: 0 max attribs: 4 > commands supported: > #1: ID-0x1 flags-0xa > --- > > As you can see, i can see from user space the name of the kernel end > point, its numeric id, what version it is running (so i can make sure > user space is compatible), what extra header it may have, what the > maximum number of attributes it can take. The last thing that gets > listed is the commands, and flags for those commands. > > Lets load tipc kernel module and repeat... > > --- > > [EMAIL PROTECTED]:~$ sudo modprobe tipc > Name: nlctrl > ID: 0x10 Version: 0x2 header size: 0 max attribs: 6 > commands supported: > #1: ID-0x3 flags-0xe > > > [same as before] > > > Name: TIPC > ID: 0x13 Version: 0x1 header size: 8 max attribs: 0 > commands supported: > #1: ID-0x1 flags-0x2 > > === > > > It would be great if there are any examples on user space communication. > > > > Bug Thomas - he has written some simple example. I also have some but i > changed laptops and i have to go and dig it up for you. > I do have plans for making this easier for people - but havent had time. > If there is persistence - or someone out there wants to be a hero email > me privately and i will explain it. > > > Or should I use libnl library instead? > > Why am i answering all these questions if you are fine with using libnl? > Last time you said you couldnt use a library, no? > > cheers, > jamal > > > > Thanks, > > Rui. > > acpi_genl.tgz Description: applicatio
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Thu, 2007-14-06 at 16:59 +0800, Zhang Rui wrote: > Hi, Jamal, > > Now the genl utility can find the acpi event genetlink family. > And a simple user space demo is finished for handling acpi event. > I really appreciate your help. :) np. > I think the patch which exposes ACPI events via netlink is ok. > But I still have some problems on > how to listen to specified genetlink family in user space? > > I can get the dynamic id for "acpi_event" genl family. > But I don't know how to use this to receive messages from > specified genl family. > It seems that "#genl ctrl monitor" has something to do with this, > IMO, rtnl_open_byproto(&rth, nl_mgrp(GENL_ID_CTRL), NETLINK_GENERIC) is > used to receive messages from the nlctrl(controller) only, but > unfortunately it never works for me. :( > I dont have much time to look at your code given travel, but did you try to use your group id instead of the controller's? i.e: rtnl_open_byproto(&rth, nl_mgrp(mydiscoveredacpiid), NETLINK_GENERIC) If this doesnt work, ping me and i will take a look - just expect some latency in response. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Thu, 2007-06-14 at 07:28 -0400, jamal wrote: > On Thu, 2007-14-06 at 16:59 +0800, Zhang Rui wrote: > > Hi, Jamal, > > > > Now the genl utility can find the acpi event genetlink family. > > And a simple user space demo is finished for handling acpi event. > > I really appreciate your help. :) > > np. > > > I think the patch which exposes ACPI events via netlink is ok. > > But I still have some problems on > > how to listen to specified genetlink family in user space? > > > > I can get the dynamic id for "acpi_event" genl family. > > But I don't know how to use this to receive messages from > > specified genl family. > > It seems that "#genl ctrl monitor" has something to do with this, > > IMO, rtnl_open_byproto(&rth, nl_mgrp(GENL_ID_CTRL), NETLINK_GENERIC) is > > used to receive messages from the nlctrl(controller) only, but > > unfortunately it never works for me. :( > > > > I dont have much time to look at your code given travel, but did you > try to use your group id instead of the controller's? > i.e: > rtnl_open_byproto(&rth, nl_mgrp(mydiscoveredacpiid), NETLINK_GENERIC) > Yes. It doesn't work if I use my group id here. In fact, I'm using rtnl_open_byproto(&rth, 1, NETLINK_GENERIC) now. That's why I said that this demo receives all the broadcasted genetlink messages. > If this doesnt work, ping me and i will take a look - just expect some > latency in response. > That's great. Thanks. Best regards, Rui - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-15-06 at 09:01 +0800, Zhang Rui wrote: > > rtnl_open_byproto(&rth, nl_mgrp(mydiscoveredacpiid), NETLINK_GENERIC) > > > Yes. It doesn't work if I use my group id here. > In fact, I'm using rtnl_open_byproto(&rth, 1, NETLINK_GENERIC) now. > That's why I said that this demo receives all the broadcasted genetlink > messages. Ok, tell me how to generate these ACPI events and i will patch my laptop and test it. What kernel compile options do i need? 2.6.24-rc4 will do? cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Fri, 2007-15-06 at 09:01 +0800, Zhang Rui wrote: > > I dont have much time to look at your code given travel, but did you > > try to use your group id instead of the controller's? > > i.e: > > rtnl_open_byproto(&rth, nl_mgrp(mydiscoveredacpiid), NETLINK_GENERIC) > > > Yes. It doesn't work if I use my group id here. > In fact, I'm using rtnl_open_byproto(&rth, 1, NETLINK_GENERIC) now. > That's why I said that this demo receives all the broadcasted genetlink > messages. Ok, by inspection (sorry, still dont have much time) - your kernel code is sending to group 1; i.e genlmsg_multicast(skb, 0, 1, GFP_ATOMIC); you need to change that to send to your assigned id, i.e: genlmsg_multicast(skb, 0, acpi_event_genl_family.id, GFP_ATOMIC); then the user space code will work. I should be able to look at it if it doesnt work by end of week. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-06-18 at 11:01 -0400, jamal wrote: > On Fri, 2007-15-06 at 09:01 +0800, Zhang Rui wrote: > > > > I dont have much time to look at your code given travel, but did you > > > try to use your group id instead of the controller's? > > > i.e: > > > rtnl_open_byproto(&rth, nl_mgrp(mydiscoveredacpiid), NETLINK_GENERIC) > > > > > Yes. It doesn't work if I use my group id here. > > In fact, I'm using rtnl_open_byproto(&rth, 1, NETLINK_GENERIC) now. > > That's why I said that this demo receives all the broadcasted genetlink > > messages. > > Ok, by inspection (sorry, still dont have much time) - your kernel code > is sending to group 1; i.e > > genlmsg_multicast(skb, 0, 1, GFP_ATOMIC); > > you need to change that to send to your assigned id, i.e: > genlmsg_multicast(skb, 0, acpi_event_genl_family.id, GFP_ATOMIC); > Oh, that's the problem. Great, now it works happily. :). Jamal, thanks for your help! Best regards, Rui - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Mon, 2007-06-18 at 11:01 -0400, jamal wrote: > Ok, by inspection (sorry, still dont have much time) - your kernel code > is sending to group 1; i.e > > genlmsg_multicast(skb, 0, 1, GFP_ATOMIC); > > you need to change that to send to your assigned id, i.e: > genlmsg_multicast(skb, 0, acpi_event_genl_family.id, GFP_ATOMIC); > > then the user space code will work. I should be able to look at it if it > doesnt work by end of week. Ah, that coincides with something I was wondering about. Isn't it possible to have multiple multicast groups with generic netlink? If so, we might have to use real netlink for wireless... johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Tue, 2007-19-06 at 13:30 +0200, Johannes Berg wrote: > Ah, that coincides with something I was wondering about. Isn't it > possible to have multiple multicast groups with generic netlink? If so, > we might have to use real netlink for wireless... There is one default mcast group per entity. Most users only need that one. If you need more, we go one of two ways: a) Register for multiple IDs with the code as is. b) we could add a "reserve multicast group" interface in the kernel. Thoughts? cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
[took off acpi list] On Tue, 2007-06-19 at 12:20 -0400, jamal wrote: > There is one default mcast group per entity. Most users only need that > one. Ok. That's definitely a bug in nl80211 as we have it in development right now. Btw, what happens if the group id gets larger than 31? > If you need more, we go one of two ways: > a) Register for multiple IDs with the code as is. > b) we could add a "reserve multicast group" interface in the kernel. > > Thoughts? I'd really like to be able to reserve multicast groups with special semantics too, especially I might want to permit/deny non-CAP_NET_ADMIN users from binding specific multicast groups. That isn't actually possible with netlink nor genetlink right now afaict. If we register multiple IDs then we'll end up filling up the generic netlink family space really soon. I was under the impression that generic netlink was basically open-ended because the family is a large enough number, but with this arbitrary limit on multicast groups that's really not true and we might run out of multicast groups fairly soon since most users of generic netlink will want at least one... johannes signature.asc Description: This is a digitally signed message part
Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Wed, 2007-20-06 at 13:25 +0200, Johannes Berg wrote: > Ok. That's definitely a bug in nl80211 as we have it in development > right now. Sorry, have never looked at that code. > Btw, what happens if the group id gets larger than 31? You can use setsockopt to set the multicast groups. What you cant do with that is subscribe to many groups in one shot. The call in iproute2 hasnt reflected this reality yet. > I'd really like to be able to reserve multicast groups with special > semantics too, especially I might want to permit/deny non-CAP_NET_ADMIN > users from binding specific multicast groups. That isn't actually > possible with netlink nor genetlink right now afaict. This would be hard - but doable via SELinux interface. I think you should be able to extend your tool to make calls to that interface. > If we register multiple IDs then we'll end up filling up the generic > netlink family space really soon. Theres a huge number of these groups; and not just that, but considering that some genetlink users may not be interested in such multicast groups, it is quiet usable to have many groups as long as we avoid conflict. > I was under the impression that > generic netlink was basically open-ended because the family is a large > enough number, but with this arbitrary limit on multicast groups that's > really not true and we might run out of multicast groups fairly soon > since most users of generic netlink will want at least one... > The multicast issue wasnt well-attacked. We have a group magically assigned to a user based on their allocated id. It should be feasible to add an API to the kernel for registering for many groups and allow user space to discover these groups before registering. Maybe thats the path to proceed to. cheers, jamal - 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: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink
On Thu, 2007-06-21 at 11:47 -0400, jamal wrote: > On Wed, 2007-20-06 at 13:25 +0200, Johannes Berg wrote: > > > Ok. That's definitely a bug in nl80211 as we have it in development > > right now. > > Sorry, have never looked at that code. No worries, I was just stating that. > You can use setsockopt to set the multicast groups. What you cant do > with that is subscribe to many groups in one shot. > The call in iproute2 hasnt reflected this reality yet. Ah, ok, I see now. I was under the impression that groups was always just a u32. > > I'd really like to be able to reserve multicast groups with special > > semantics too, especially I might want to permit/deny non-CAP_NET_ADMIN > > users from binding specific multicast groups. That isn't actually > > possible with netlink nor genetlink right now afaict. > > This would be hard - but doable via SELinux interface. I think you > should be able to extend your tool to make calls to that interface. Why do you think that would be hard? It'd basically just mean replacing the netlink_capable(sock, NL_NONROOT_RECV) calls with a call that actually tests depending on the group(s) it wants. > > If we register multiple IDs then we'll end up filling up the generic > > netlink family space really soon. > > Theres a huge number of these groups; and not just that, but considering > that some genetlink users may not be interested in such multicast > groups, it is quiet usable to have many groups as long as we avoid > conflict. Yeah, never mind, I thought that the number of groups was limited to 32. > The multicast issue wasnt well-attacked. We have a group magically > assigned to a user based on their allocated id. It should be feasible > to add an API to the kernel for registering for many groups and allow > user space to discover these groups before registering. Maybe thats > the path to proceed to. Yeah, sounds reasonable, you could ask the controller for which groups are attached to a family and then get the IDs for those groups by name. johannes signature.asc Description: This is a digitally signed message part
