From patchwork Fri Sep 17 12:31:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elad Nachman X-Patchwork-Id: 99139 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 52BA3A0C46; Fri, 17 Sep 2021 14:31:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 08E3A406B4; Fri, 17 Sep 2021 14:31:37 +0200 (CEST) Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by mails.dpdk.org (Postfix) with ESMTP id AE37240689 for ; Fri, 17 Sep 2021 14:31:35 +0200 (CEST) Received: by mail-ed1-f42.google.com with SMTP id v22so25139695edd.11 for ; Fri, 17 Sep 2021 05:31:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id; bh=GtFv1FedLUIlpYxMACHbkeZi/xpTR3cBcGsTgId5Mqo=; b=jn80nCMme7vJwYjjaFSfsiVbyJpZ0tAl9Na1jdQy4128vG0sJwdN3ctaaMsH2clC9S k2DjHIBfm3kt/2wqNywlduVOaY7sObefdp9jIaSsqnTCirBbfXb4ezi48Dabl/2K3N9z 4EKC10vsvKwW1txF43uSbkdntHK+13ibQ52ah3cuRuQjlX4cj3E97LCyoFoBObn5VoEv tWxAsQyw2KXNDRRW8UBVeSmp7Bo6tNWyGc0Ryrlvt36w5tdhlWrfCvD6yI3KbENWoVIP ZGgidzIYqcmVkyWenbDqpDaCZ8WD2wpDOPFOklhmWsB+qqpj7hH/nAYe1j3HHLyaSvuD bZ3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=GtFv1FedLUIlpYxMACHbkeZi/xpTR3cBcGsTgId5Mqo=; b=vX+3iH2Khu0vT3E6YVwXB+yVI8hFYrqOOP2pOUYo3PHe8AmBcPB5ZHbLCDJUtEN3Y0 UBoI5LC/mnOiTMrYHkZiYFSExPxC0YQWJUkyOpLDH9+5Js8xNx9DFmfKRkI4Nu/knA8m uQvOkFjkQziSguexLnP43QwUJj6YLhz6I0zKR9ypuLE4dvZl6csX55+lL35yjIZ9yu+Y 4nw8o9HilM9hO7PQz9Rvtgq7U71qTEZ3GvbG6Bqk+uSHvZ7NwjsjGtxl3PLIzDOxeUT+ 3ebFJyuZ8WpoPdWyI1ZNbiT24FXckfzGN79lg8H4cBNmDg0c1DTkfSWfiD8KViAcJHQm 1YTg== X-Gm-Message-State: AOAM5311tre1rEDPZpG6QiuJ4crHAdTZPQWsmu1lTvFrEzb4pOn5VDGt Nu2vQuS2YOyEXVe2St4sKD8= X-Google-Smtp-Source: ABdhPJzm+LM6pueq4YwwoNq/pzEviYdoqCsS5obt0QTVfrlknAOcqmSRFJModDKIC3QaG7NfN6+4eg== X-Received: by 2002:a17:906:a18f:: with SMTP id s15mr12417485ejy.269.1631881894780; Fri, 17 Sep 2021 05:31:34 -0700 (PDT) Received: from localhost (89-139-18-237.bb.netvision.net.il. [89.139.18.237]) by smtp.gmail.com with ESMTPSA id b3sm2241250ejb.7.2021.09.17.05.31.33 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Sep 2021 05:31:34 -0700 (PDT) From: Elad Nachman To: ferruh.yigit@intel.com Cc: dev@dpdk.org, erclists@gmail.com, iryzhov@nfware.com, Elad Nachman Date: Fri, 17 Sep 2021 15:31:31 +0300 Message-Id: <20210917123131.6329-1-eladv6@gmail.com> X-Mailer: git-send-email 2.17.1 Subject: [dpdk-dev] [PATCH] kni: Fix request overwritten X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Fix lack of multiple KNI requests handling support by introducing a rotating ring mechanism for the sync buffer. Prevent kni_net_change_rx_flags() from calling kni_net_process_request() with a malformed request. Bugzilla ID: 809 Signed-off-by: Elad Nachman --- kernel/linux/kni/kni_dev.h | 2 ++ kernel/linux/kni/kni_misc.c | 2 ++ kernel/linux/kni/kni_net.c | 25 +++++++++++++++++-------- lib/kni/rte_kni.c | 14 +++++++------- lib/kni/rte_kni_common.h | 1 + 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index c15da311ba..b9e8b3d10d 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -74,6 +74,8 @@ struct kni_dev { void *sync_kva; void *sync_va; + unsigned int sync_ring_size; + atomic_t sync_ring_idx; void *mbuf_kva; void *mbuf_va; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 2b464c4381..f3cee97818 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -345,6 +345,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni->net_dev = net_dev; kni->core_id = dev_info.core_id; + kni->sync_ring_size = dev_info.sync_ring_size; + kni->sync_ring_idx.counter = 0; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); /* Translate user space info into kernel space info */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 611719b5ee..dc066cdd98 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -110,6 +110,8 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) void *resp_va; uint32_t num; int ret_val; + unsigned int idx; + char *k_reqptr, *v_reqptr; ASSERT_RTNL(); @@ -122,10 +124,17 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) } mutex_lock(&kni->sync_lock); - + idx = atomic_read(&kni->sync_ring_idx); + atomic_cmpxchg(&kni->sync_ring_idx, idx, + (idx + 1) % kni->sync_ring_size); /* Construct data */ - memcpy(kni->sync_kva, req, sizeof(struct rte_kni_request)); - num = kni_fifo_put(kni->req_q, &kni->sync_va, 1); + k_reqptr = (char *)((uintptr_t)kni->sync_kva + + sizeof(struct rte_kni_request) * idx); + v_reqptr = (char *)((uintptr_t)kni->sync_va + + sizeof(struct rte_kni_request) * idx); + + memcpy(k_reqptr, req, sizeof(struct rte_kni_request)); + num = kni_fifo_put(kni->req_q, (void **)&v_reqptr, 1); if (num < 1) { pr_err("Cannot send to req_q\n"); ret = -EBUSY; @@ -147,14 +156,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) goto fail; } num = kni_fifo_get(kni->resp_q, (void **)&resp_va, 1); - if (num != 1 || resp_va != kni->sync_va) { + if (num != 1) { /* This should never happen */ pr_err("No data in resp_q\n"); ret = -ENODATA; goto fail; } - memcpy(req, kni->sync_kva, sizeof(struct rte_kni_request)); + memcpy(req, k_reqptr, sizeof(struct rte_kni_request)); async: ret = 0; @@ -681,13 +690,12 @@ kni_net_change_mtu(struct net_device *dev, int new_mtu) static void kni_net_change_rx_flags(struct net_device *netdev, int flags) { - struct rte_kni_request req; + struct rte_kni_request req = { 0 }; memset(&req, 0, sizeof(req)); if (flags & IFF_ALLMULTI) { req.req_id = RTE_KNI_REQ_CHANGE_ALLMULTI; - if (netdev->flags & IFF_ALLMULTI) req.allmulti = 1; else @@ -703,7 +711,8 @@ kni_net_change_rx_flags(struct net_device *netdev, int flags) req.promiscusity = 0; } - kni_net_process_request(netdev, &req); + if (req.req_id) + kni_net_process_request(netdev, &req); } /* diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c index d3e236005e..e921d41ce8 100644 --- a/lib/kni/rte_kni.c +++ b/lib/kni/rte_kni.c @@ -31,6 +31,9 @@ #define KNI_FIFO_COUNT_MAX 1024 #define KNI_FIFO_SIZE (KNI_FIFO_COUNT_MAX * sizeof(void *) + \ sizeof(struct rte_kni_fifo)) +#define KNI_REQS_SIZE (KNI_FIFO_COUNT_MAX * \ + sizeof(struct rte_kni_request) + \ + sizeof(struct rte_kni_fifo)) #define KNI_REQUEST_MBUF_NUM_MAX 32 @@ -175,8 +178,8 @@ kni_reserve_mz(struct rte_kni *kni) KNI_MEM_CHECK(kni->m_resp_q == NULL, resp_q_fail); snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_SYNC_ADDR_MZ_NAME_FMT, kni->name); - kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, - RTE_MEMZONE_IOVA_CONTIG); + kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_REQS_SIZE, + SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); KNI_MEM_CHECK(kni->m_sync_addr == NULL, sync_addr_fail); return 0; @@ -307,6 +310,7 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool, kni->sync_addr = kni->m_sync_addr->addr; dev_info.sync_va = kni->m_sync_addr->addr; dev_info.sync_phys = kni->m_sync_addr->iova; + dev_info.sync_ring_size = KNI_FIFO_COUNT_MAX; kni->pktmbuf_pool = pktmbuf_pool; kni->group_id = conf->group_id; @@ -544,11 +548,7 @@ rte_kni_handle_request(struct rte_kni *kni) if (ret != 1) return 0; /* It is OK of can not getting the request mbuf */ - if (req != kni->sync_addr) { - RTE_LOG(ERR, KNI, "Wrong req pointer %p\n", req); - return -1; - } - + printf("%s: req %p id %u\n", __func__, req, req->req_id); /* Analyze the request and call the relevant actions for it */ switch (req->req_id) { case RTE_KNI_REQ_CHANGE_MTU: /* Change MTU */ diff --git a/lib/kni/rte_kni_common.h b/lib/kni/rte_kni_common.h index b547ea5501..a78b9bafa0 100644 --- a/lib/kni/rte_kni_common.h +++ b/lib/kni/rte_kni_common.h @@ -110,6 +110,7 @@ struct rte_kni_device_info { phys_addr_t resp_phys; phys_addr_t sync_phys; void * sync_va; + unsigned int sync_ring_size; /* mbuf mempool */ void * mbuf_va;