On 08/12/2016 13:47, amine.ahd wrote:
> In the current implementation, a fd can be sent in the reply to an existing 
> request which can be inefficient in certain cases.
> Example: when you want to create a pipe between two process with the help of 
> a 3rd process acting as a manager: you need 3 pipe with the current 
> implementation and only one pipe with this patch.
> 

explain how you achieve this

> Basically the patch adds two new functions (ubus_invoke_fd and 
> ubus_invoke_async_fd) that are very similar to the original functions (open 
> to any other alternatives here) and a new msg type (UBUS_MSG_INVOKE_FD) to 
> copy the received fd to req_fd when processing the invocation.

missing SoB here

> ---
>  libubus-internal.h |  4 +++-
>  libubus-obj.c      | 19 ++++++++++++++-----
>  libubus-req.c      | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libubus.c          |  3 ++-
>  libubus.h          |  9 +++++++++
>  ubusd.c            |  1 -
>  ubusd_proto.c      | 13 +++++++++++--
>  ubusmsg.h          |  2 ++
>  8 files changed, 90 insertions(+), 10 deletions(-)
> 
> diff --git a/libubus-internal.h b/libubus-internal.h
> index f62edc3..9513fb7 100644
> --- a/libubus-internal.h
> +++ b/libubus-internal.h
> @@ -24,7 +24,9 @@ int ubus_send_msg(struct ubus_context *ctx, uint32_t seq,
>  void ubus_process_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf, 
> int fd);
>  int __hidden ubus_start_request(struct ubus_context *ctx, struct 
> ubus_request *req,
>                               struct blob_attr *msg, int cmd, uint32_t peer);
> -void ubus_process_obj_msg(struct ubus_context *ctx, struct ubus_msghdr_buf 
> *buf);
> +int __hidden ubus_start_request_fd(struct ubus_context *ctx, struct 
> ubus_request *req,
> +                             struct blob_attr *msg, int cmd, uint32_t peer, 
> int fd);
> +void ubus_process_obj_msg(struct ubus_context *ctx, struct ubus_msghdr_buf 
> *buf, int fd);
>  void ubus_process_req_msg(struct ubus_context *ctx, struct ubus_msghdr_buf 
> *buf, int fd);
>  void __hidden ubus_poll_data(struct ubus_context *ctx, int timeout);
>  
> diff --git a/libubus-obj.c b/libubus-obj.c
> index 990d04b..94af630 100644
> --- a/libubus-obj.c
> +++ b/libubus-obj.c
> @@ -44,11 +44,13 @@ ubus_process_notify(struct ubus_context *ctx, struct 
> ubus_msghdr *hdr,
>  }
>  static void
>  ubus_process_invoke(struct ubus_context *ctx, struct ubus_msghdr *hdr,
> -                 struct ubus_object *obj, struct blob_attr **attrbuf)
> +                 struct ubus_object *obj, struct blob_attr **attrbuf, int fd)
>  {
>       struct ubus_request_data req = {
>               .fd = -1,
> +             .req_fd = fd,
>       };
> +
>       int method;
>       int ret;
>       bool no_reply = false;
> @@ -94,11 +96,13 @@ found:
>  send:
>       ubus_complete_deferred_request(ctx, &req, ret);
>  }
> -
> -void __hidden ubus_process_obj_msg(struct ubus_context *ctx, struct 
> ubus_msghdr_buf *buf)
> + 
> +void __hidden ubus_process_obj_msg(struct ubus_context *ctx, struct 
> ubus_msghdr_buf *buf, int fd)
>  {
>       void (*cb)(struct ubus_context *, struct ubus_msghdr *,
>                  struct ubus_object *, struct blob_attr **);
> +     void (*cb_fd)(struct ubus_context *, struct ubus_msghdr *,
> +                struct ubus_object *, struct blob_attr **, int fd);
>       struct ubus_msghdr *hdr = &buf->hdr;
>       struct blob_attr **attrbuf;
>       struct ubus_object *obj;
> @@ -114,7 +118,9 @@ void __hidden ubus_process_obj_msg(struct ubus_context 
> *ctx, struct ubus_msghdr_
>  
>       switch (hdr->type) {
>       case UBUS_MSG_INVOKE:
> -             cb = ubus_process_invoke;
> +     case UBUS_MSG_INVOKE_FD:
> +             cb_fd = ubus_process_invoke;
> +             cb = NULL;
>               break;
>       case UBUS_MSG_UNSUBSCRIBE:
>               cb = ubus_process_unsubscribe;
> @@ -131,7 +137,10 @@ void __hidden ubus_process_obj_msg(struct ubus_context 
> *ctx, struct ubus_msghdr_
>               buf->data = NULL;
>       }
>  
> -     cb(ctx, hdr, obj, attrbuf);
> +     if (hdr->type == UBUS_MSG_INVOKE || hdr->type == UBUS_MSG_INVOKE_FD)
> +             cb_fd(ctx, hdr, obj, attrbuf, fd);
> +     else
> +             cb(ctx, hdr, obj, attrbuf);
>  
>       if (prev_data) {
>               if (buf->data)
> diff --git a/libubus-req.c b/libubus-req.c
> index 416adab..9913f1e 100644
> --- a/libubus-req.c
> +++ b/libubus-req.c
> @@ -62,9 +62,27 @@ int __hidden ubus_start_request(struct ubus_context *ctx, 
> struct ubus_request *r
>       req->ctx = ctx;
>       req->peer = peer;
>       req->seq = ++ctx->request_seq;
> +
>       return ubus_send_msg(ctx, req->seq, msg, cmd, peer, -1);
>  }
>  
> +int __hidden ubus_start_request_fd(struct ubus_context *ctx, struct 
> ubus_request *req,
> +                             struct blob_attr *msg, int cmd, uint32_t peer, 
> int fd)
> +{
> +     memset(req, 0, sizeof(*req));
> +
> +     if (msg && blob_pad_len(msg) > UBUS_MAX_MSGLEN)
> +             return -1;
> +
> +     INIT_LIST_HEAD(&req->list);
> +     INIT_LIST_HEAD(&req->pending);
> +     req->ctx = ctx;
> +     req->peer = peer;
> +     req->seq = ++ctx->request_seq;
> +
> +     return ubus_send_msg(ctx, req->seq, msg, cmd, peer, fd);
> +}
> +
>  void ubus_abort_request(struct ubus_context *ctx, struct ubus_request *req)
>  {
>       if (list_empty(&req->list))
> @@ -224,6 +242,21 @@ int ubus_invoke_async(struct ubus_context *ctx, uint32_t 
> obj, const char *method
>       return 0;
>  }
>  
> +
> +int ubus_invoke_async_fd(struct ubus_context *ctx, uint32_t obj, const char 
> *method,
> +                       struct blob_attr *msg, struct ubus_request *req, int 
> fd)
> +{
> +     blob_buf_init(&b, 0);
> +     blob_put_int32(&b, UBUS_ATTR_OBJID, obj);
> +     blob_put_string(&b, UBUS_ATTR_METHOD, method);
> +     if (msg)
> +             blob_put(&b, UBUS_ATTR_DATA, blob_data(msg), blob_len(msg));
> +
> +     if (ubus_start_request_fd(ctx, req, b.head, UBUS_MSG_INVOKE_FD, obj, 
> fd) < 0)
> +             return UBUS_STATUS_INVALID_ARGUMENT;
> +     return 0;
> +}
> +
>  int ubus_invoke(struct ubus_context *ctx, uint32_t obj, const char *method,
>                  struct blob_attr *msg, ubus_data_handler_t cb, void *priv,
>               int timeout)
> @@ -240,6 +273,22 @@ int ubus_invoke(struct ubus_context *ctx, uint32_t obj, 
> const char *method,
>       return ubus_complete_request(ctx, &req, timeout);
>  }
>  
> +int ubus_invoke_fd(struct ubus_context *ctx, uint32_t obj, const char 
> *method,
> +                struct blob_attr *msg, ubus_data_handler_t cb, void *priv,
> +             int timeout, int fd)
> +{
> +     struct ubus_request req;
> +     int rc;
> +
> +     rc = ubus_invoke_async_fd(ctx, obj, method, msg, &req, fd);
> +     if (rc)
> +             return rc;
> +
> +     req.data_cb = cb;
> +     req.priv = priv;
> +     return ubus_complete_request(ctx, &req, timeout);
> +}
> +
>  static void
>  ubus_notify_complete_cb(struct ubus_request *req, int ret)
>  {
> diff --git a/libubus.c b/libubus.c
> index 8163ff7..d9602e7 100644
> --- a/libubus.c
> +++ b/libubus.c
> @@ -96,6 +96,7 @@ ubus_process_msg(struct ubus_context *ctx, struct 
> ubus_msghdr_buf *buf, int fd)
>               break;
>  
>       case UBUS_MSG_INVOKE:
> +     case UBUS_MSG_INVOKE_FD:
>       case UBUS_MSG_UNSUBSCRIBE:
>       case UBUS_MSG_NOTIFY:
>               if (ctx->stack_depth) {
> @@ -103,7 +104,7 @@ ubus_process_msg(struct ubus_context *ctx, struct 
> ubus_msghdr_buf *buf, int fd)
>                       break;
>               }
>  
> -             ubus_process_obj_msg(ctx, buf);
> +             ubus_process_obj_msg(ctx, buf, fd);
>               break;
>       case UBUS_MSG_MONITOR:
>               if (ctx->monitor_cb)
> diff --git a/libubus.h b/libubus.h
> index 07239d6..0a05416 100644
> --- a/libubus.h
> +++ b/libubus.h
> @@ -188,6 +188,7 @@ struct ubus_request_data {
>       /* internal use */
>       bool deferred;
>       int fd;
> +     int req_fd; /* fd received from the initial request */
>  };
>  
>  struct ubus_request {
> @@ -336,6 +337,14 @@ int ubus_invoke(struct ubus_context *ctx, uint32_t obj, 
> const char *method,
>  int ubus_invoke_async(struct ubus_context *ctx, uint32_t obj, const char 
> *method,
>                     struct blob_attr *msg, struct ubus_request *req);
>  
> +int ubus_invoke_fd(struct ubus_context *ctx, uint32_t obj, const char 
> *method,
> +             struct blob_attr *msg, ubus_data_handler_t cb, void *priv,
> +             int timeout, int fd);
> +
> +/* asynchronous version of ubus_invoke() */
> +int ubus_invoke_async_fd(struct ubus_context *ctx, uint32_t obj, const char 
> *method,
> +                   struct blob_attr *msg, struct ubus_request *req, int fd);
> +
>  /* send a reply to an incoming object method call */
>  int ubus_send_reply(struct ubus_context *ctx, struct ubus_request_data *req,
>                   struct blob_attr *msg);
> diff --git a/ubusd.c b/ubusd.c
> index 7279a70..cf38c31 100644
> --- a/ubusd.c
> +++ b/ubusd.c
> @@ -345,7 +345,6 @@ static bool get_next_connection(int fd)
>               uloop_fd_add(&cl->sock, ULOOP_READ | ULOOP_EDGE_TRIGGER);
>       else
>               close(client_fd);
> -
>       return true;
>  }
>  
> diff --git a/ubusd_proto.c b/ubusd_proto.c
> index b591384..596a981 100644
> --- a/ubusd_proto.c
> +++ b/ubusd_proto.c
> @@ -80,11 +80,16 @@ void
>  ubus_proto_send_msg_from_blob(struct ubus_client *cl, struct ubus_msg_buf 
> *ub,
>                       uint8_t type)
>  {
> +     /* keep the fd to be passed if it is UBUS_MSG_INVOKE_FD */
> +     int fd = ub->fd;
>       ub = ubus_reply_from_blob(ub, true);
>       if (!ub)
>               return;
>  
>       ub->hdr.type = type;
> +     if(type == UBUS_MSG_INVOKE_FD)
> +             ub->fd = fd;
> +
>       ubus_msg_send(cl, ub, true);
>  }
>  
> @@ -239,7 +244,10 @@ ubusd_forward_invoke(struct ubus_client *cl, struct 
> ubus_object *obj,
>       if (data)
>               blob_put(&b, UBUS_ATTR_DATA, blob_data(data), blob_len(data));
>  
> -     ubus_proto_send_msg_from_blob(obj->client, ub, UBUS_MSG_INVOKE);
> +     if (ub->hdr.type == UBUS_MSG_INVOKE_FD)
> +             ubus_proto_send_msg_from_blob(obj->client, ub, 
> UBUS_MSG_INVOKE_FD);
> +     else
> +             ubus_proto_send_msg_from_blob(obj->client, ub, UBUS_MSG_INVOKE);
>  }
>  
>  static int ubusd_handle_invoke(struct ubus_client *cl, struct ubus_msg_buf 
> *ub, struct blob_attr **attr)
> @@ -428,6 +436,7 @@ static const ubus_cmd_cb handlers[__UBUS_MSG_LAST] = {
>       [UBUS_MSG_REMOVE_OBJECT] = ubusd_handle_remove_object,
>       [UBUS_MSG_LOOKUP] = ubusd_handle_lookup,
>       [UBUS_MSG_INVOKE] = ubusd_handle_invoke,
> +     [UBUS_MSG_INVOKE_FD] = ubusd_handle_invoke,
>       [UBUS_MSG_STATUS] = ubusd_handle_response,
>       [UBUS_MSG_DATA] = ubusd_handle_response,
>       [UBUS_MSG_SUBSCRIBE] = ubusd_handle_add_watch,
> @@ -446,7 +455,7 @@ void ubusd_proto_receive_message(struct ubus_client *cl, 
> struct ubus_msg_buf *ub
>       if (ub->hdr.type < __UBUS_MSG_LAST)
>               cb = handlers[ub->hdr.type];
>  
> -     if (ub->hdr.type != UBUS_MSG_STATUS)
> +     if (ub->hdr.type != UBUS_MSG_STATUS && ub->hdr.type != 
> UBUS_MSG_INVOKE_FD)
>               ubus_msg_close_fd(ub);
>  
>       if (cb)
> diff --git a/ubusmsg.h b/ubusmsg.h
> index 398b126..aa3955c 100644
> --- a/ubusmsg.h
> +++ b/ubusmsg.h
> @@ -51,6 +51,8 @@ enum ubus_msg_type {
>  
>       /* invoke a method on a single object */
>       UBUS_MSG_INVOKE,
> +     /* invoke a method and send a fd along */
> +     UBUS_MSG_INVOKE_FD,
>  
>       UBUS_MSG_ADD_OBJECT,
>       UBUS_MSG_REMOVE_OBJECT,
> 

_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Reply via email to