Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-07-03 Thread Johannes Berg
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

2007-07-03 Thread Patrick McHardy
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

2007-07-02 Thread Johannes Berg
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

2007-07-02 Thread Patrick McHardy
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

2007-07-02 Thread Johannes Berg
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

2007-07-02 Thread Patrick McHardy
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

2007-07-02 Thread Johannes Berg
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

2007-06-30 Thread jamal
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

2007-06-29 Thread jamal
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 to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread jamal
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread jamal
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread jamal
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

2007-06-29 Thread jamal
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

2007-06-29 Thread jamal
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread Johannes Berg
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

2007-06-29 Thread Patrick McHardy
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

2007-06-29 Thread jamal
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

2007-06-29 Thread Johannes Berg
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 net/netlink.h
 
 /**
+ * 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 linux/module.h
@@ -13,6 +14,7 @@
 #include linux/string.h
 #include linux/skbuff.h
 #include linux/mutex.h
+#include linux/bitmap.h
 #include net/sock.h
 #include net/genetlink.h
 
@@ -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 

Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-29 Thread Johannes Berg
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 net/netlink.h
 
 /**
+ * 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 linux/module.h
@@ -13,6 +14,7 @@
 #include linux/string.h
 #include linux/skbuff.h
 #include linux/mutex.h
+#include linux/bitmap.h
 #include net/sock.h
 #include net/genetlink.h
 
@@ -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;
+}

Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-29 Thread Johannes Berg
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 net/netlink.h
 
 /**
+ * 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 linux/module.h
@@ -13,6 +14,7 @@
 #include linux/string.h
 #include linux/skbuff.h
 #include linux/mutex.h
+#include linux/bitmap.h
 #include net/sock.h
 #include net/genetlink.h
 
@@ -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 

Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-28 Thread Johannes Berg
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

2007-06-27 Thread jamal

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 net/netlink.h
  
  /**
 + * 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

2007-06-26 Thread Johannes Berg
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

2007-06-26 Thread Johannes Berg
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 net/netlink.h
 
 /**
+ * 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 linux/module.h
@@ -13,6 +14,7 @@
 #include linux/string.h
 #include linux/skbuff.h
 #include linux/mutex.h
+#include linux/bitmap.h
 #include net/sock.h
 #include net/genetlink.h
 
@@ -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) {
+   

Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-26 Thread jamal
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

2007-06-26 Thread Johannes Berg
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

2007-06-25 Thread jamal
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

2007-06-22 Thread Johannes Berg
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


Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-21 Thread jamal
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

2007-06-20 Thread Johannes Berg
[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

2007-06-19 Thread Johannes Berg
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

2007-06-18 Thread jamal
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

2007-06-18 Thread Zhang Rui
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

2007-06-15 Thread jamal
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

2007-06-14 Thread Zhang Rui
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: application/compressed-tar


Re: Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-06-14 Thread jamal
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

2007-06-14 Thread Zhang Rui
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


Fwd: [PATCH] [-mm] ACPI: export ACPI events via netlink

2007-05-27 Thread jamal

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