Re: [PATCH] ACPI: export acpi events via generic netlink

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]

On Tue, 2007-06-19 at 11:40 +0800, Zhang Rui wrote:

> +/* attributes of acpi_genl_family */
> +enum {
> +	ACPI_GENL_ATTR_UNSPEC,
> +	ACPI_GENL_ATTR_EVENT,	/* ACPI event info needed by user space */
> +	__ACPI_GENL_ATTR_MAX,
> +};
> +#define ACPI_GENL_ATTR_MAX (__ACPI_GENL_ATTR_MAX - 1)
> +
> +/* commands supported by the acpi_genl_family */
> +enum {
> +	ACPI_GENL_CMD_UNSPEC,
> +	ACPI_GENL_CMD_EVENT,	/* kernel->user notifications for ACPI events */
> +	__ACPI_GENL_CMD_MAX,
> +};
> +#define ACPI_GENL_CMD_MAX (__ACPI_GENL_CMD_MAX - 1)

Shouldn't you put these into some header file instead of copying them
into your userspace tool?

> +#define ACPI_GENL_NAME		"acpi_event"
> +#define ACPI_GENL_VERSION	0x01

and maybe those too?

> +/* .doit: standard command callback */
> +static int acpi_genl_cmd_event(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct acpi_genl_event *event = info->userhdr;
> +
> +	if (!event)
> +		ACPI_DEBUG_PRINT((ACPI_DB_WARN, "ACPI event: NULL\n"));
> +
> +	return 0;
> +}
> +
> +static struct genl_ops acpi_event_genl_ops = {
> +	.cmd = ACPI_GENL_CMD_EVENT,
> +	.doit = acpi_genl_cmd_event,
> +};

This I don't understand, why would userspace ever send events to the
kernel? I think you should just leave this out completely.

> +	result =
> +	    genlmsg_multicast(skb, 0, acpi_event_genl_family.id, GFP_ATOMIC);

If my patches are merged you should now register a multicast group.

Here's how I do it in nl80211 now:

| static struct genl_multicast_group nl80211_config_mcgrp = {
| 	.name = "config",
| };

during init:

| err = genl_register_mc_group(&nl80211_fam, &nl80211_config_mcgrp);
| if (err)
| 	goto err_out;


for sending the message:

| genlmsg_multicast(msg, 0, nl80211_config_mcgrp.id, GFP_KERNEL);

(I suppose you want GFP_ATOMIC for a reason)

In userspace you query the new multicast group attribute(s) [but let's
see what Jamal wants me to change there first] and bind to that group
using
| int group = ...;
| setsockopt(s, 270 /*SOL_NETLINK*/, NETLINK_ADD_MEMBERSHIP, &group, sizeof(group));.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]