Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/23025/?format=api
{ "id": 23025, "url": "https://patches.dpdk.org/api/patches/23025/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/ed50c49b886a7f891a8ec13b0baea74f80c79439.1490966849.git.pascal.mazon@6wind.com/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<ed50c49b886a7f891a8ec13b0baea74f80c79439.1490966849.git.pascal.mazon@6wind.com>", "list_archive_url": "https://inbox.dpdk.org/dev/ed50c49b886a7f891a8ec13b0baea74f80c79439.1490966849.git.pascal.mazon@6wind.com", "date": "2017-03-31T13:54:09", "name": "[dpdk-dev,v3,1/3] net/tap: update netlink error code management", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "38f8cc3b39a63869b8fa492be39c34b0c78263b5", "submitter": { "id": 657, "url": "https://patches.dpdk.org/api/people/657/?format=api", "name": "Pascal Mazon", "email": "pascal.mazon@6wind.com" }, "delegate": { "id": 319, "url": "https://patches.dpdk.org/api/users/319/?format=api", "username": "fyigit", "first_name": "Ferruh", "last_name": "Yigit", "email": "ferruh.yigit@amd.com" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/ed50c49b886a7f891a8ec13b0baea74f80c79439.1490966849.git.pascal.mazon@6wind.com/mbox/", "series": [], "comments": "https://patches.dpdk.org/api/patches/23025/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/23025/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 4DB872BF2;\n\tFri, 31 Mar 2017 15:55:59 +0200 (CEST)", "from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com\n\t[62.23.145.76]) by dpdk.org (Postfix) with ESMTP id EDD342BA4\n\tfor <dev@dpdk.org>; Fri, 31 Mar 2017 15:55:51 +0200 (CEST)", "from 6wind.com (unknown [10.16.0.184])\n\tby proxy.6wind.com (Postfix) with SMTP id CDC822780B;\n\tFri, 31 Mar 2017 15:55:45 +0200 (CEST)", "by 6wind.com (sSMTP sendmail emulation);\n\tFri, 31 Mar 2017 15:54:38 +0200" ], "From": "Pascal Mazon <pascal.mazon@6wind.com>", "To": "keith.wiles@intel.com", "Cc": "dev@dpdk.org,\n\tPascal Mazon <pascal.mazon@6wind.com>", "Date": "Fri, 31 Mar 2017 15:54:09 +0200", "Message-Id": "<ed50c49b886a7f891a8ec13b0baea74f80c79439.1490966849.git.pascal.mazon@6wind.com>", "X-Mailer": "git-send-email 2.12.0.306.g4a9b9b3", "In-Reply-To": "<cover.1490966849.git.pascal.mazon@6wind.com>", "References": "<3f667d40be2f2d4db572bfa4200561e5818514b6.1490965230.git.pascal.mazon@6wind.com>\n\t<cover.1490966849.git.pascal.mazon@6wind.com>", "Subject": "[dpdk-dev] [PATCH v3 1/3] net/tap: update netlink error code\n\tmanagement", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Some errors received from the kernel are acceptable, such as a -ENOENT\nfor a rule deletion (the rule was already no longer existing in the\nkernel). Make sure we consider return codes properly. For that,\nnl_recv() has been simplified.\n\nqdisc_exists() function is no longer needed as we can check whether the\nkernel returned -EEXIST when requiring the qdisc creation. It's simpler\nand faster.\n\nAdd a few messages for clarity when a netlink error occurs.\n\nSigned-off-by: Pascal Mazon <pascal.mazon@6wind.com>\n---\n drivers/net/tap/tap_flow.c | 22 ++++++++-\n drivers/net/tap/tap_netlink.c | 73 +++++++++++++---------------\n drivers/net/tap/tap_tcmsgs.c | 107 ++++++++++--------------------------------\n drivers/net/tap/tap_tcmsgs.h | 2 -\n 4 files changed, 80 insertions(+), 124 deletions(-)", "diff": "diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c\nindex 7f1693d40468..514e3fae5c38 100644\n--- a/drivers/net/tap/tap_flow.c\n+++ b/drivers/net/tap/tap_flow.c\n@@ -31,6 +31,8 @@\n * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n */\n \n+#include <errno.h>\n+#include <string.h>\n #include <sys/queue.h>\n \n #include <rte_byteorder.h>\n@@ -1165,6 +1167,9 @@ tap_flow_create(struct rte_eth_dev *dev,\n \t}\n \terr = nl_recv_ack(pmd->nlsk_fd);\n \tif (err < 0) {\n+\t\tRTE_LOG(ERR, PMD,\n+\t\t\t\"Kernel refused TC filter rule creation (%d): %s\\n\",\n+\t\t\terrno, strerror(errno));\n \t\trte_flow_error_set(error, EEXIST, RTE_FLOW_ERROR_TYPE_HANDLE,\n \t\t\t\t NULL, \"overlapping rules\");\n \t\tgoto fail;\n@@ -1206,6 +1211,9 @@ tap_flow_create(struct rte_eth_dev *dev,\n \t\t}\n \t\terr = nl_recv_ack(pmd->nlsk_fd);\n \t\tif (err < 0) {\n+\t\t\tRTE_LOG(ERR, PMD,\n+\t\t\t\t\"Kernel refused TC filter rule creation (%d): %s\\n\",\n+\t\t\t\terrno, strerror(errno));\n \t\t\trte_flow_error_set(\n \t\t\t\terror, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,\n \t\t\t\tNULL, \"overlapping rules\");\n@@ -1253,7 +1261,13 @@ tap_flow_destroy_pmd(struct pmd_internals *pmd,\n \t\tgoto end;\n \t}\n \tret = nl_recv_ack(pmd->nlsk_fd);\n+\t/* If errno is ENOENT, the rule is already no longer in the kernel. */\n+\tif (ret < 0 && errno == ENOENT)\n+\t\tret = 0;\n \tif (ret < 0) {\n+\t\tRTE_LOG(ERR, PMD,\n+\t\t\t\"Kernel refused TC filter rule deletion (%d): %s\\n\",\n+\t\t\terrno, strerror(errno));\n \t\trte_flow_error_set(\n \t\t\terror, ENOTSUP, RTE_FLOW_ERROR_TYPE_HANDLE, NULL,\n \t\t\t\"couldn't receive kernel ack to our request\");\n@@ -1271,7 +1285,12 @@ tap_flow_destroy_pmd(struct pmd_internals *pmd,\n \t\t\tgoto end;\n \t\t}\n \t\tret = nl_recv_ack(pmd->nlsk_fd);\n+\t\tif (ret < 0 && errno == ENOENT)\n+\t\t\tret = 0;\n \t\tif (ret < 0) {\n+\t\t\tRTE_LOG(ERR, PMD,\n+\t\t\t\t\"Kernel refused TC filter rule deletion (%d): %s\\n\",\n+\t\t\t\terrno, strerror(errno));\n \t\t\trte_flow_error_set(\n \t\t\t\terror, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,\n \t\t\t\tNULL, \"Failure trying to receive nl ack\");\n@@ -1386,7 +1405,8 @@ int tap_flow_implicit_create(struct pmd_internals *pmd,\n \terr = nl_recv_ack(pmd->nlsk_fd);\n \tif (err < 0) {\n \t\tRTE_LOG(ERR, PMD,\n-\t\t\t\"Kernel refused TC filter rule creation\");\n+\t\t\t\"Kernel refused TC filter rule creation (%d): %s\\n\",\n+\t\t\terrno, strerror(errno));\n \t\tgoto fail;\n \t}\n \tLIST_INSERT_HEAD(&pmd->implicit_flows, remote_flow, next);\ndiff --git a/drivers/net/tap/tap_netlink.c b/drivers/net/tap/tap_netlink.c\nindex 6de896ab17b6..ee92e2e7ed13 100644\n--- a/drivers/net/tap/tap_netlink.c\n+++ b/drivers/net/tap/tap_netlink.c\n@@ -159,7 +159,7 @@ nl_send(int nlsk_fd, struct nlmsghdr *nh)\n * The netlink socket file descriptor used for communication.\n *\n * @return\n- * 0 on success, -1 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n int\n nl_recv_ack(int nlsk_fd)\n@@ -179,14 +179,13 @@ nl_recv_ack(int nlsk_fd)\n * Custom arguments for the callback.\n *\n * @return\n- * 0 on success, -1 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n int\n nl_recv(int nlsk_fd, int (*cb)(struct nlmsghdr *, void *arg), void *arg)\n {\n \t/* man 7 netlink EXAMPLE */\n \tstruct sockaddr_nl sa;\n-\tstruct nlmsghdr *nh;\n \tchar buf[BUF_SIZE];\n \tstruct iovec iov = {\n \t\t.iov_base = buf,\n@@ -196,49 +195,43 @@ nl_recv(int nlsk_fd, int (*cb)(struct nlmsghdr *, void *arg), void *arg)\n \t\t.msg_name = &sa,\n \t\t.msg_namelen = sizeof(sa),\n \t\t.msg_iov = &iov,\n+\t\t/* One message at a time */\n \t\t.msg_iovlen = 1,\n \t};\n-\tint recv_bytes = 0, done = 0, multipart = 0, error = 0;\n+\tint multipart = 0;\n+\tint ret = 0;\n \n-read:\n-\trecv_bytes = recvmsg(nlsk_fd, &msg, 0);\n-\tif (recv_bytes < 0)\n-\t\treturn -1;\n-\tfor (nh = (struct nlmsghdr *)buf;\n-\t NLMSG_OK(nh, (unsigned int)recv_bytes);\n-\t nh = NLMSG_NEXT(nh, recv_bytes)) {\n-\t\t/*\n-\t\t * Multi-part messages and their following DONE message have the\n-\t\t * NLM_F_MULTI flag set. Make note, in order to read the DONE\n-\t\t * message afterwards.\n-\t\t */\n-\t\tif (nh->nlmsg_flags & NLM_F_MULTI)\n-\t\t\tmultipart = 1;\n-\t\tif (nh->nlmsg_type == NLMSG_ERROR) {\n-\t\t\tstruct nlmsgerr *err_data = NLMSG_DATA(nh);\n+\tdo {\n+\t\tstruct nlmsghdr *nh;\n+\t\tint recv_bytes = 0;\n+\n+\t\trecv_bytes = recvmsg(nlsk_fd, &msg, 0);\n+\t\tif (recv_bytes < 0)\n+\t\t\treturn -1;\n+\t\tfor (nh = (struct nlmsghdr *)buf;\n+\t\t NLMSG_OK(nh, (unsigned int)recv_bytes);\n+\t\t nh = NLMSG_NEXT(nh, recv_bytes)) {\n+\t\t\tif (nh->nlmsg_type == NLMSG_ERROR) {\n+\t\t\t\tstruct nlmsgerr *err_data = NLMSG_DATA(nh);\n \n-\t\t\tif (err_data->error == 0)\n-\t\t\t\tRTE_LOG(DEBUG, PMD, \"%s() ack message recvd\\n\",\n-\t\t\t\t\t__func__);\n-\t\t\telse {\n-\t\t\t\tRTE_LOG(DEBUG, PMD,\n-\t\t\t\t\t\"%s() error message recvd\\n\", __func__);\n-\t\t\t\terror = 1;\n+\t\t\t\tif (err_data->error < 0) {\n+\t\t\t\t\terrno = -err_data->error;\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t\t/* Ack message. */\n+\t\t\t\treturn 0;\n \t\t\t}\n+\t\t\t/* Multi-part msgs and their trailing DONE message. */\n+\t\t\tif (nh->nlmsg_flags & NLM_F_MULTI) {\n+\t\t\t\tif (nh->nlmsg_type == NLMSG_DONE)\n+\t\t\t\t\treturn 0;\n+\t\t\t\tmultipart = 1;\n+\t\t\t}\n+\t\t\tif (cb)\n+\t\t\t\tret = cb(nh, arg);\n \t\t}\n-\t\t/* The end of multipart message. */\n-\t\tif (nh->nlmsg_type == NLMSG_DONE)\n-\t\t\t/* No need to call the callback for a DONE message. */\n-\t\t\tdone = 1;\n-\t\telse if (cb)\n-\t\t\tif (cb(nh, arg) < 0)\n-\t\t\t\terror = 1;\n-\t}\n-\tif (multipart && !done)\n-\t\tgoto read;\n-\tif (error)\n-\t\treturn -1;\n-\treturn 0;\n+\t} while (multipart);\n+\treturn ret;\n }\n \n /**\ndiff --git a/drivers/net/tap/tap_tcmsgs.c b/drivers/net/tap/tap_tcmsgs.c\nindex af1c9aec0d22..d74ac805b184 100644\n--- a/drivers/net/tap/tap_tcmsgs.c\n+++ b/drivers/net/tap/tap_tcmsgs.c\n@@ -94,7 +94,7 @@ tc_init_msg(struct nlmsg *msg, uint16_t ifindex, uint16_t type, uint16_t flags)\n * Additional info to identify the QDISC (handle and parent).\n *\n * @return\n- * 0 on success, -1 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n static int\n qdisc_del(int nlsk_fd, uint16_t ifindex, struct qdisc *qinfo)\n@@ -117,12 +117,16 @@ qdisc_del(int nlsk_fd, uint16_t ifindex, struct qdisc *qinfo)\n \t\tfd = nlsk_fd;\n \t}\n \tif (nl_send(fd, &msg.nh) < 0)\n-\t\treturn -1;\n+\t\tgoto error;\n \tif (nl_recv_ack(fd) < 0)\n-\t\treturn -1;\n+\t\tgoto error;\n \tif (!nlsk_fd)\n \t\treturn nl_final(fd);\n \treturn 0;\n+error:\n+\tif (!nlsk_fd)\n+\t\tnl_final(fd);\n+\treturn -1;\n }\n \n /**\n@@ -134,7 +138,7 @@ qdisc_del(int nlsk_fd, uint16_t ifindex, struct qdisc *qinfo)\n * The netdevice ifindex where to add the multiqueue QDISC.\n *\n * @return\n- * -1 if the qdisc cannot be added, and 0 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n int\n qdisc_add_multiq(int nlsk_fd, uint16_t ifindex)\n@@ -164,7 +168,7 @@ qdisc_add_multiq(int nlsk_fd, uint16_t ifindex)\n * The netdevice ifindex where the QDISC will be added.\n *\n * @return\n- * -1 if the qdisc cannot be added, and 0 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n int\n qdisc_add_ingress(int nlsk_fd, uint16_t ifindex)\n@@ -184,34 +188,6 @@ qdisc_add_ingress(int nlsk_fd, uint16_t ifindex)\n }\n \n /**\n- * Callback function to check for QDISC existence.\n- * If the QDISC is found to exist, increment \"exists\" in the custom arg.\n- *\n- * @param[in] nh\n- * The netlink message to parse, received from the kernel.\n- * @param[in, out] arg\n- * Custom arguments for the callback.\n- *\n- * @return\n- * 0.\n- */\n-static int\n-qdisc_exist_cb(struct nlmsghdr *nh, void *arg)\n-{\n-\tstruct list_args *args = (struct list_args *)arg;\n-\tstruct qdisc_custom_arg *custom = args->custom_arg;\n-\tstruct tcmsg *t = NLMSG_DATA(nh);\n-\n-\t/* filter by request iface */\n-\tif (args->ifindex != (unsigned int)t->tcm_ifindex)\n-\t\treturn 0;\n-\tif (t->tcm_handle != custom->handle || t->tcm_parent != custom->parent)\n-\t\treturn 0;\n-\tcustom->exists++;\n-\treturn 0;\n-}\n-\n-/**\n * Callback function to delete a QDISC.\n *\n * @param[in] nh\n@@ -220,7 +196,7 @@ qdisc_exist_cb(struct nlmsghdr *nh, void *arg)\n * Custom arguments for the callback.\n *\n * @return\n- * 0.\n+ * 0 on success, -1 otherwise with errno set.\n */\n static int\n qdisc_del_cb(struct nlmsghdr *nh, void *arg)\n@@ -256,10 +232,7 @@ qdisc_del_cb(struct nlmsghdr *nh, void *arg)\n * The arguments to provide the callback function with.\n *\n * @return\n- * -1 if either sending the netlink message failed, or if receiving the answer\n- * failed, or finally if the callback returned a negative value for that\n- * answer.\n- * 0 is returned otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n static int\n qdisc_iterate(int nlsk_fd, uint16_t ifindex,\n@@ -281,36 +254,6 @@ qdisc_iterate(int nlsk_fd, uint16_t ifindex,\n }\n \n /**\n- * Check whether a given QDISC already exists for the netdevice.\n- *\n- * @param[in] nlsk_fd\n- * The netlink socket file descriptor used for communication.\n- * @param[in] ifindex\n- * The netdevice ifindex to check QDISC existence for.\n- * @param[in] callback\n- * The function to call for each QDISC.\n- * @param[in, out] arg\n- * The arguments to provide the callback function with.\n- *\n- * @return\n- * 1 if the qdisc exists, 0 otherwise.\n- */\n-int\n-qdisc_exists(int nlsk_fd, uint16_t ifindex, uint32_t handle, uint32_t parent)\n-{\n-\tstruct qdisc_custom_arg arg = {\n-\t\t.handle = handle,\n-\t\t.parent = parent,\n-\t\t.exists = 0,\n-\t};\n-\n-\tqdisc_iterate(nlsk_fd, ifindex, qdisc_exist_cb, &arg);\n-\tif (arg.exists)\n-\t\treturn 1;\n-\treturn 0;\n-}\n-\n-/**\n * Delete all QDISCs for a given netdevice.\n *\n * @param[in] nlsk_fd\n@@ -319,7 +262,7 @@ qdisc_exists(int nlsk_fd, uint16_t ifindex, uint32_t handle, uint32_t parent)\n * The netdevice ifindex where to find QDISCs.\n *\n * @return\n- * -1 if the lookup failed, 0 otherwise.\n+ * 0 on success, -1 otherwise with errno set.\n */\n int\n qdisc_flush(int nlsk_fd, uint16_t ifindex)\n@@ -342,12 +285,13 @@ qdisc_flush(int nlsk_fd, uint16_t ifindex)\n int\n qdisc_create_multiq(int nlsk_fd, uint16_t ifindex)\n {\n-\tif (!qdisc_exists(nlsk_fd, ifindex,\n-\t\t\t TC_H_MAKE(MULTIQ_MAJOR_HANDLE, 0), TC_H_ROOT)) {\n-\t\tif (qdisc_add_multiq(nlsk_fd, ifindex) < 0) {\n-\t\t\tRTE_LOG(ERR, PMD, \"Could not add multiq qdisc\\n\");\n-\t\t\treturn -1;\n-\t\t}\n+\tint err = 0;\n+\n+\terr = qdisc_add_multiq(nlsk_fd, ifindex);\n+\tif (err < 0 && errno != -EEXIST) {\n+\t\tRTE_LOG(ERR, PMD, \"Could not add multiq qdisc (%d): %s\\n\",\n+\t\t\terrno, strerror(errno));\n+\t\treturn -1;\n \t}\n \treturn 0;\n }\n@@ -367,12 +311,13 @@ qdisc_create_multiq(int nlsk_fd, uint16_t ifindex)\n int\n qdisc_create_ingress(int nlsk_fd, uint16_t ifindex)\n {\n-\tif (!qdisc_exists(nlsk_fd, ifindex,\n-\t\t\t TC_H_MAKE(TC_H_INGRESS, 0), TC_H_INGRESS)) {\n-\t\tif (qdisc_add_ingress(nlsk_fd, ifindex) < 0) {\n-\t\t\tRTE_LOG(ERR, PMD, \"Could not add ingress qdisc\\n\");\n-\t\t\treturn -1;\n-\t\t}\n+\tint err = 0;\n+\n+\terr = qdisc_add_ingress(nlsk_fd, ifindex);\n+\tif (err < 0 && errno != -EEXIST) {\n+\t\tRTE_LOG(ERR, PMD, \"Could not add ingress qdisc (%d): %s\\n\",\n+\t\t\terrno, strerror(errno));\n+\t\treturn -1;\n \t}\n \treturn 0;\n }\ndiff --git a/drivers/net/tap/tap_tcmsgs.h b/drivers/net/tap/tap_tcmsgs.h\nindex a571a56d6964..789595771d63 100644\n--- a/drivers/net/tap/tap_tcmsgs.h\n+++ b/drivers/net/tap/tap_tcmsgs.h\n@@ -50,8 +50,6 @@\n \n void tc_init_msg(struct nlmsg *msg, uint16_t ifindex, uint16_t type,\n \t\t uint16_t flags);\n-int qdisc_exists(int nlsk_fd, uint16_t ifindex, uint32_t handle,\n-\t\t uint32_t parent);\n int qdisc_list(int nlsk_fd, uint16_t ifindex);\n int qdisc_flush(int nlsk_fd, uint16_t ifindex);\n int qdisc_create_ingress(int nlsk_fd, uint16_t ifindex);\n", "prefixes": [ "dpdk-dev", "v3", "1/3" ] }