| Message ID | 1336574243-12063-1-git-send-email-alban.crequy@collabora.co.uk |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote: > In a connector callback, it was not possible to reply to a message only to a > sender. This patch implements cn_netlink_reply(). It uses the connector socket > to send an unicast netlink message back to the sender. [...] We try to avoid adding functions with no users. You'll need to submit the code that's intended to use this as well. Ben.
On Thu, May 10, 2012 at 01:20:48AM +0100, Ben Hutchings (bhutchings@solarflare.com) wrote: > On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote: > > In a connector callback, it was not possible to reply to a message only to a > > sender. This patch implements cn_netlink_reply(). It uses the connector socket > > to send an unicast netlink message back to the sender. > [...] > > We try to avoid adding functions with no users. You'll need to submit > the code that's intended to use this as well. I have no objection against this patch, but as correctly stated it is useless without users. Alban, what is the code you want this functionality to be used in? Do you plan to submit it? Can you submit this change in the patch with your code?
On Thu, 10 May 2012 04:45:53 +0400, Evgeniy Polyakov <zbr@ioremap.net> wrote : > On Thu, May 10, 2012 at 01:20:48AM +0100, Ben Hutchings > (bhutchings@solarflare.com) wrote: > > On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote: > > > In a connector callback, it was not possible to reply to a > > > message only to a sender. This patch implements > > > cn_netlink_reply(). It uses the connector socket to send an > > > unicast netlink message back to the sender. > > [...] > > > > We try to avoid adding functions with no users. You'll need to > > submit the code that's intended to use this as well. > > I have no objection against this patch, but as correctly stated it is > useless without users. Alban, what is the code you want this > functionality to be used in? Do you plan to submit it? Can you submit > this change in the patch with your code? The code to use the feature is not yet ready for submission and we will add this patch to the front of that submission in due course. We are just being good community members and making each patch available early. Thanks for your feedback on this patch. Please let me know if I can add any reviewed-by. Alban -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 10, 2012 at 11:39:08AM +0100, Alban Crequy (alban.crequy@collabora.co.uk) wrote: > The code to use the feature is not yet ready for submission and we will > add this patch to the front of that submission in due course. > > We are just being good community members and making each patch > available early. Thanks for your feedback on this patch. Please let > me know if I can add any reviewed-by. Yes, you can add review tag with your submission for connector, your changes look good
On Thu, 2012-05-10 at 11:39 +0100, Alban Crequy wrote: > On Thu, 10 May 2012 04:45:53 +0400, > Evgeniy Polyakov <zbr@ioremap.net> wrote : > > > On Thu, May 10, 2012 at 01:20:48AM +0100, Ben Hutchings > > (bhutchings@solarflare.com) wrote: > > > On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote: > > > > In a connector callback, it was not possible to reply to a > > > > message only to a sender. This patch implements > > > > cn_netlink_reply(). It uses the connector socket to send an > > > > unicast netlink message back to the sender. > > > [...] > > > > > > We try to avoid adding functions with no users. You'll need to > > > submit the code that's intended to use this as well. > > > > I have no objection against this patch, but as correctly stated it is > > useless without users. Alban, what is the code you want this > > functionality to be used in? Do you plan to submit it? Can you submit > > this change in the patch with your code? > > The code to use the feature is not yet ready for submission and we will > add this patch to the front of that submission in due course. > > We are just being good community members and making each patch > available early. When you send a message with a subject beginning [PATCH] to a kernel mailing list, it's generally assumed to be a request to the relevant maintainer to apply that patch. But kernel API functions are added only to support features that are exposed exernally, and tend to be removed when there are no in-tree users. You could of course send such patches as RFCs ([RFC][PATCH] in the subject), so it's clear that you don't expect them to be applied yet. Ben. > Thanks for your feedback on this patch. Please let > me know if I can add any reviewed-by.
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c index dde6a0f..1cb488d 100644 --- a/drivers/connector/connector.c +++ b/drivers/connector/connector.c @@ -118,6 +118,38 @@ nlmsg_failure: EXPORT_SYMBOL_GPL(cn_netlink_send); /* + * Send an unicast reply from a connector callback + * + */ +int cn_netlink_reply(struct cn_msg *msg, u32 pid, gfp_t gfp_mask) +{ + unsigned int size; + struct sk_buff *skb; + struct nlmsghdr *nlh; + struct cn_msg *data; + struct cn_dev *dev = &cdev; + + size = NLMSG_SPACE(sizeof(*msg) + msg->len); + + skb = alloc_skb(size, gfp_mask); + if (!skb) + return -ENOMEM; + + nlh = NLMSG_PUT(skb, 0, msg->seq, NLMSG_DONE, size - sizeof(*nlh)); + + data = NLMSG_DATA(nlh); + + memcpy(data, msg, sizeof(*data) + msg->len); + + return netlink_unicast(dev->nls, skb, pid, 1); + +nlmsg_failure: + kfree_skb(skb); + return -EINVAL; +} +EXPORT_SYMBOL_GPL(cn_netlink_reply); + +/* * Callback helper - queues work and setup destructor for given data. */ static int cn_call_callback(struct sk_buff *skb) diff --git a/include/linux/connector.h b/include/linux/connector.h index 7638407..c27be60 100644 --- a/include/linux/connector.h +++ b/include/linux/connector.h @@ -125,6 +125,7 @@ int cn_add_callback(struct cb_id *id, const char *name, void (*callback)(struct cn_msg *, struct netlink_skb_parms *)); void cn_del_callback(struct cb_id *); int cn_netlink_send(struct cn_msg *, u32, gfp_t); +int cn_netlink_reply(struct cn_msg *, u32, gfp_t); int cn_queue_add_callback(struct cn_queue_dev *dev, const char *name, struct cb_id *id,
In a connector callback, it was not possible to reply to a message only to a sender. This patch implements cn_netlink_reply(). It uses the connector socket to send an unicast netlink message back to the sender. The following pseudo-code can be used from a connector callback: struct cn_msg *cn_reply; cn_reply = kzalloc(sizeof(struct cn_msg) + sizeof(struct ..._nl_cfg_reply), GFP_KERNEL); cn_reply->id = msg->id; cn_reply->seq = msg->seq; cn_reply->ack = msg->ack + 1; cn_reply->len = sizeof(struct ..._nl_cfg_reply); cn_reply->flags = 0; rr = cn_netlink_reply(cn_reply, nsp->pid, GFP_KERNEL); Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk> Reviewed-by: Vincent Sanders <vincent.sanders@collabora.co.uk> Acked-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> CC: Rodrigo Moya <rodrigo.moya@collabora.co.uk> CC: Evgeniy Polyakov <zbr@ioremap.net> --- drivers/connector/connector.c | 32 ++++++++++++++++++++++++++++++++ include/linux/connector.h | 1 + 2 files changed, 33 insertions(+), 0 deletions(-)