[dpdk-dev,v10,11/20] unci: add netlink exec
Checks
Commit Message
Add netlink exec function, which sends a message to userspace and waits
and receives the response from userspace.
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
.../linuxapp/eal/include/exec-env/unci.h | 6 +
lib/librte_eal/linuxapp/unci/unci_dev.h | 4 +
lib/librte_eal/linuxapp/unci/unci_net.c | 5 +
lib/librte_eal/linuxapp/unci/unci_nl.c | 180 +++++++++++++++++++++
4 files changed, 195 insertions(+)
Comments
On Tue, 4 Jul 2017 17:13:28 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> +int unci_nl_exec(u32 cmd, struct net_device *dev, void *in_data,
> + size_t in_data_len, void *out_data, size_t out_data_len)
> +{
> + struct unci_dev *unci = netdev_priv(dev);
> + int err = -EINVAL;
> + int ret;
> +
> + if (out_data_len > UNCI_NL_MSG_LEN) {
> + pr_err("Message is too big to receive:%zu\n", out_data_len);
> + return err;
> + }
> +
> + mutex_lock(&sync_lock);
> + ret = unci_response_buffer_register(cmd, out_data, out_data_len,
> + &unci->msg_received, &err);
> + if (ret) {
> + mutex_unlock(&sync_lock);
> + return -EINVAL;
> + }
> +
> + ret = unci_nl_send(cmd, unci->port_id, unci->pid, in_data, in_data_len);
> + if (ret) {
> + unci_response_buffer_unregister(response_buffer.magic);
> + mutex_unlock(&sync_lock);
> + return ret;
> + }
> +
> + ret = wait_for_completion_interruptible_timeout(&unci->msg_received,
> + msecs_to_jiffies(UNCI_CMD_TIMEOUT));
Blocking for completion with mutex held?
Sleeping with mutex held is not allowed in Linux.
You will see this if you run with lockdep and all the other kernel debug
config options.
On Tue, 4 Jul 2017 17:13:28 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> +
> + if (nl_msg->err == 0 && recv_len != expected_len)
> + pr_info("Expected and received len not match "
> + "%zu - %zu\n", recv_len, expected_len);
> + }
> +
Checkpatch complains about string split, just put the whole string on one line.
pr_info("Expected and received len not match %zu - %zu\n",
recv_len, expected_len)
WARNING: quoted string split across lines
#99: FILE: lib/librte_eal/linuxapp/unci/unci_nl.c:99:
+ pr_info("Expected and received len not match "
+ "%zu - %zu\n", recv_len, expected_len);
On 7/5/2017 8:07 PM, Stephen Hemminger wrote:
> On Tue, 4 Jul 2017 17:13:28 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
>> +int unci_nl_exec(u32 cmd, struct net_device *dev, void *in_data,
>> + size_t in_data_len, void *out_data, size_t out_data_len)
>> +{
>> + struct unci_dev *unci = netdev_priv(dev);
>> + int err = -EINVAL;
>> + int ret;
>> +
>> + if (out_data_len > UNCI_NL_MSG_LEN) {
>> + pr_err("Message is too big to receive:%zu\n", out_data_len);
>> + return err;
>> + }
>> +
>> + mutex_lock(&sync_lock);
>> + ret = unci_response_buffer_register(cmd, out_data, out_data_len,
>> + &unci->msg_received, &err);
>> + if (ret) {
>> + mutex_unlock(&sync_lock);
>> + return -EINVAL;
>> + }
>> +
>> + ret = unci_nl_send(cmd, unci->port_id, unci->pid, in_data, in_data_len);
>> + if (ret) {
>> + unci_response_buffer_unregister(response_buffer.magic);
>> + mutex_unlock(&sync_lock);
>> + return ret;
>> + }
>> +
>> + ret = wait_for_completion_interruptible_timeout(&unci->msg_received,
>> + msecs_to_jiffies(UNCI_CMD_TIMEOUT));
>
> Blocking for completion with mutex held?
> Sleeping with mutex held is not allowed in Linux.
>
> You will see this if you run with lockdep and all the other kernel debug
> config options.
Thank you for the review,
I will send a new version addressing all comments.
Thanks,
ferruh
On Thu, 6 Jul 2017 11:45:48 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> > Blocking for completion with mutex held?
> > Sleeping with mutex held is not allowed in Linux.
> >
> > You will see this if you run with lockdep and all the other kernel debug
> > config options.
>
> Thank you for the review,
>
> I will send a new version addressing all comments.
>
> Thanks,
> ferruh
>
I was wrong, it is okay to sleep with mutex held.
You might find it easier to use a spin_lock and waitqueue directly
via wait_event_interruptible_locked
@@ -74,6 +74,12 @@ struct unci_nl_msg {
int err;
};
+enum unci_ethtool_msg_flag {
+ UNCI_MSG_FLAG_NONE,
+ UNCI_MSG_FLAG_REQUEST,
+ UNCI_MSG_FLAG_RESPONSE,
+};
+
/* can go into include/uapi/linux/if_link.h */
enum {
IFLA_UNCI_UNSPEC,
@@ -37,9 +37,13 @@
struct unci_dev {
__u32 port_id;
__u32 pid;
+ struct completion msg_received;
+ u32 nb_timedout_msg;
};
int unci_nl_init(void);
void unci_nl_release(void);
+int unci_nl_exec(u32 cmd, struct net_device *dev, void *in_data,
+ size_t in_len, void *out_data, size_t out_len);
#endif /* _UNCI_DEV_H_ */
@@ -31,8 +31,13 @@ static const struct net_device_ops unci_net_netdev_ops = { 0 };
static void unci_net_setup(struct net_device *dev)
{
+ struct unci_dev *unci;
+
ether_setup(dev);
dev->netdev_ops = &unci_net_netdev_ops;
+
+ unci = netdev_priv(dev);
+ init_completion(&unci->msg_received);
}
static int unci_net_newlink(struct net *net, struct net_device *dev,
@@ -26,6 +26,180 @@
#include "unci_dev.h"
+#define UNCI_GENL_MSG_LEN 1536
+
+#define UNCI_CMD_TIMEOUT 500 /* ms */
+
+static struct response_buffer {
+ int magic; /* for sanity check */
+ void *buffer;
+ size_t length;
+ struct completion *msg_received;
+ int *err;
+ u32 in_use;
+} response_buffer;
+
+static DEFINE_MUTEX(sync_lock);
+
+static int unci_response_buffer_register(int magic, void *buffer, size_t length,
+ struct completion *msg_received, int *err)
+{
+ if (!response_buffer.in_use) {
+ response_buffer.magic = magic;
+ response_buffer.buffer = buffer;
+ response_buffer.length = length;
+ response_buffer.msg_received = msg_received;
+ response_buffer.err = err;
+ response_buffer.in_use = 1;
+ return 0;
+ }
+
+ return 1;
+}
+
+static void unci_response_buffer_unregister(int magic)
+{
+ if (response_buffer.in_use) {
+ if (magic == response_buffer.magic) {
+ response_buffer.magic = -1;
+ response_buffer.buffer = NULL;
+ response_buffer.length = 0;
+ response_buffer.msg_received = NULL;
+ response_buffer.err = NULL;
+ response_buffer.in_use = 0;
+ } else {
+ pr_err("Unregister magic mismatch\n");
+ }
+ }
+}
+
+static void nl_recv_user_request(struct unci_nl_msg *nl_msg)
+{
+ /* Userspace requests not supported yet */
+ pr_debug("Request from userspace received\n");
+}
+
+static void nl_recv_user_response(struct unci_nl_msg *nl_msg)
+{
+ struct completion *msg_received;
+ size_t recv_len;
+ size_t expected_len;
+
+ if (response_buffer.in_use) {
+ if (response_buffer.buffer != NULL) {
+ recv_len = nl_msg->output_buffer_len;
+ expected_len = response_buffer.length;
+
+ memcpy(response_buffer.buffer,
+ nl_msg->output_buffer,
+ response_buffer.length);
+
+ if (nl_msg->err == 0 && recv_len != expected_len)
+ pr_info("Expected and received len not match "
+ "%zu - %zu\n", recv_len, expected_len);
+ }
+
+ *response_buffer.err = nl_msg->err;
+ msg_received = response_buffer.msg_received;
+ unci_response_buffer_unregister(response_buffer.magic);
+ complete(msg_received);
+ }
+}
+
+static struct genl_family unci_genl_family;
+
+static int unci_nl_send(u32 cmd_id, u32 port_id, u32 pid, void *in_data,
+ size_t in_data_len)
+{
+ struct unci_nl_msg nl_msg;
+ struct sk_buff *skb;
+ void *payload;
+ u32 size = 0;
+
+ if (pid == 0)
+ return -1;
+
+ memset(&nl_msg, 0, sizeof(struct unci_nl_msg));
+ nl_msg.cmd_id = cmd_id;
+ nl_msg.port_id = port_id;
+
+ if (in_data) {
+ if (in_data_len == 0 || in_data_len > UNCI_NL_MSG_LEN)
+ return -EINVAL;
+ nl_msg.input_buffer_len = in_data_len;
+ memcpy(nl_msg.input_buffer, in_data, in_data_len);
+ }
+
+ skb = genlmsg_new(UNCI_GENL_MSG_LEN, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ size = sizeof(struct unci_nl_msg) + GENL_HDRLEN +
+ unci_genl_family.hdrsize;
+
+ payload = genlmsg_put(skb, 0, 0, &unci_genl_family, 0, UNCI_CMD_MSG);
+ if (payload == NULL) {
+ nlmsg_free(skb);
+ return -EMSGSIZE;
+ }
+
+ nla_put(skb, UNCI_ATTR_MSG, sizeof(struct unci_nl_msg), &nl_msg);
+
+ genlmsg_end(skb, payload);
+
+ genlmsg_unicast(&init_net, skb, pid);
+ pr_debug("Sent cmd:%u port:%u pid:%u\n", cmd_id, port_id, pid);
+
+ return 0;
+}
+
+int unci_nl_exec(u32 cmd, struct net_device *dev, void *in_data,
+ size_t in_data_len, void *out_data, size_t out_data_len)
+{
+ struct unci_dev *unci = netdev_priv(dev);
+ int err = -EINVAL;
+ int ret;
+
+ if (out_data_len > UNCI_NL_MSG_LEN) {
+ pr_err("Message is too big to receive:%zu\n", out_data_len);
+ return err;
+ }
+
+ mutex_lock(&sync_lock);
+ ret = unci_response_buffer_register(cmd, out_data, out_data_len,
+ &unci->msg_received, &err);
+ if (ret) {
+ mutex_unlock(&sync_lock);
+ return -EINVAL;
+ }
+
+ ret = unci_nl_send(cmd, unci->port_id, unci->pid, in_data, in_data_len);
+ if (ret) {
+ unci_response_buffer_unregister(response_buffer.magic);
+ mutex_unlock(&sync_lock);
+ return ret;
+ }
+
+ ret = wait_for_completion_interruptible_timeout(&unci->msg_received,
+ msecs_to_jiffies(UNCI_CMD_TIMEOUT));
+ if (ret == 0 || err < 0) {
+ unci_response_buffer_unregister(response_buffer.magic);
+ mutex_unlock(&sync_lock);
+ if (ret == 0) { /* timeout */
+ unci->nb_timedout_msg++;
+ pr_info("Command timed-out for port:%u cmd:%u (%u)\n",
+ unci->port_id, cmd, unci->nb_timedout_msg);
+ return -EINVAL;
+ }
+ pr_debug("Command return error for port:%d cmd:%d err:%d\n",
+ unci->port_id, cmd, err);
+ return err;
+ }
+ mutex_unlock(&sync_lock);
+
+ return 0;
+}
+
static int unci_genl_process(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr **attrs = info->attrs;
@@ -37,6 +211,12 @@ static int unci_genl_process(struct sk_buff *skb, struct genl_info *info)
nla_memcpy(&nl_msg, attrs[UNCI_ATTR_MSG], sizeof(struct unci_nl_msg));
pr_debug("cmd: %u\n", nl_msg.cmd_id);
+ if (nl_msg.flag & UNCI_MSG_FLAG_REQUEST) {
+ nl_recv_user_request(&nl_msg);
+ return 0;
+ }
+
+ nl_recv_user_response(&nl_msg);
return 0;
}