* [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path
@ 2020-06-24 1:11 longli
2020-06-24 1:11 ` [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure longli
2020-07-02 0:06 ` [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path Stephen Hemminger
0 siblings, 2 replies; 5+ messages in thread
From: longli @ 2020-06-24 1:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Long Li
From: Long Li <longli@microsoft.com>
When rte_pktmbuf_attach_extbuf() is used, the driver should not decrease the
reference count in its callback function hn_rx_buf_free_cb, because the
reference count is already decreased by rte_pktmbuf. Doing it twice may result
in underflow and driver may never send an ack packet over vmbus to host.
Also declares rxbuf_outstanding as atomic, because this value is shared
among all receive queues.
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/net/netvsc/hn_nvs.c | 1 -
drivers/net/netvsc/hn_rxtx.c | 30 +++++++++++-------------------
drivers/net/netvsc/hn_var.h | 2 +-
3 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/net/netvsc/hn_nvs.c b/drivers/net/netvsc/hn_nvs.c
index 0fa74cd46..466b1948e 100644
--- a/drivers/net/netvsc/hn_nvs.c
+++ b/drivers/net/netvsc/hn_nvs.c
@@ -99,7 +99,6 @@ __hn_nvs_execute(struct hn_data *hv,
/* Silently drop received packets while waiting for response */
if (hdr->type == NVS_TYPE_RNDIS) {
hn_nvs_ack_rxbuf(chan, xactid);
- --hv->rxbuf_outstanding;
goto retry;
}
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index a4f6d1b6a..aece38e2d 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -519,24 +519,13 @@ hn_rndis_rxinfo(const void *info_data, unsigned int info_dlen,
return 0;
}
-/*
- * Ack the consumed RXBUF associated w/ this channel packet,
- * so that this RXBUF can be recycled by the hypervisor.
- */
-static void hn_rx_buf_release(struct hn_rx_bufinfo *rxb)
+static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque)
{
- struct rte_mbuf_ext_shared_info *shinfo = &rxb->shinfo;
+ struct hn_rx_bufinfo *rxb = opaque;
struct hn_data *hv = rxb->hv;
- if (rte_mbuf_ext_refcnt_update(shinfo, -1) == 0) {
- hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid);
- --hv->rxbuf_outstanding;
- }
-}
-
-static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque)
-{
- hn_rx_buf_release(opaque);
+ rte_atomic32_dec(&hv->rxbuf_outstanding);
+ hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid);
}
static struct hn_rx_bufinfo *hn_rx_buf_init(const struct hn_rx_queue *rxq,
@@ -576,7 +565,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
* some space available in receive area for later packets.
*/
if (dlen >= HN_RXCOPY_THRESHOLD &&
- hv->rxbuf_outstanding < hv->rxbuf_section_cnt / 2) {
+ (uint32_t)rte_atomic32_read(&hv->rxbuf_outstanding) <
+ hv->rxbuf_section_cnt / 2) {
struct rte_mbuf_ext_shared_info *shinfo;
const void *rxbuf;
rte_iova_t iova;
@@ -590,8 +580,9 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
iova = rte_mem_virt2iova(rxbuf) + RTE_PTR_DIFF(data, rxbuf);
shinfo = &rxb->shinfo;
- if (rte_mbuf_ext_refcnt_update(shinfo, 1) == 1)
- ++hv->rxbuf_outstanding;
+ /* shinfo is already set to 1 by the caller */
+ if (rte_mbuf_ext_refcnt_update(shinfo, 1) == 2)
+ rte_atomic32_inc(&hv->rxbuf_outstanding);
rte_pktmbuf_attach_extbuf(m, data, iova,
dlen + headroom, shinfo);
@@ -832,7 +823,8 @@ hn_nvs_handle_rxbuf(struct rte_eth_dev *dev,
}
/* Send ACK now if external mbuf not used */
- hn_rx_buf_release(rxb);
+ if (rte_mbuf_ext_refcnt_update(&rxb->shinfo, -1) == 0)
+ hn_nvs_ack_rxbuf(rxb->chan, rxb->xactid);
}
/*
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 881832d85..7cb7713e9 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -113,7 +113,7 @@ struct hn_data {
struct rte_mem_resource *rxbuf_res; /* UIO resource for Rx */
struct hn_rx_bufinfo *rxbuf_info;
uint32_t rxbuf_section_cnt; /* # of Rx sections */
- volatile uint32_t rxbuf_outstanding;
+ rte_atomic32_t rxbuf_outstanding;
uint16_t max_queues; /* Max available queues */
uint16_t num_queues;
uint64_t rss_offloads;
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure
2020-06-24 1:11 [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path longli
@ 2020-06-24 1:11 ` longli
2020-07-02 0:05 ` Stephen Hemminger
2020-07-02 0:06 ` [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path Stephen Hemminger
1 sibling, 1 reply; 5+ messages in thread
From: longli @ 2020-06-24 1:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Long Li
From: Long Li <longli@microsoft.com>
When external buffer is used, driver should detach it if it doesn't make it
successfully to the queue.
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/net/netvsc/hn_rxtx.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index aece38e2d..52f5c0191 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -550,6 +550,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
{
struct hn_data *hv = rxq->hv;
struct rte_mbuf *m;
+ bool use_extbuf = false;
m = rte_pktmbuf_alloc(rxq->mb_pool);
if (unlikely(!m)) {
@@ -587,6 +588,7 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
rte_pktmbuf_attach_extbuf(m, data, iova,
dlen + headroom, shinfo);
m->data_off = headroom;
+ use_extbuf = true;
} else {
/* Mbuf's in pool must be large enough to hold small packets */
if (unlikely(rte_pktmbuf_tailroom(m) < dlen)) {
@@ -614,6 +616,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
if (!hv->vlan_strip && rte_vlan_insert(&m)) {
PMD_DRV_LOG(DEBUG, "vlan insert failed");
++rxq->stats.errors;
+ if (use_extbuf)
+ rte_pktmbuf_detach_extbuf(m);
rte_pktmbuf_free(m);
return;
}
@@ -648,6 +652,8 @@ static void hn_rxpkt(struct hn_rx_queue *rxq, struct hn_rx_bufinfo *rxb,
if (unlikely(rte_ring_sp_enqueue(rxq->rx_ring, m) != 0)) {
++rxq->stats.ring_full;
PMD_RX_LOG(DEBUG, "rx ring full");
+ if (use_extbuf)
+ rte_pktmbuf_detach_extbuf(m);
rte_pktmbuf_free(m);
}
}
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure
2020-06-24 1:11 ` [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure longli
@ 2020-07-02 0:05 ` Stephen Hemminger
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2020-07-02 0:05 UTC (permalink / raw)
To: longli; +Cc: Stephen Hemminger, dev, Long Li
On Tue, 23 Jun 2020 18:11:46 -0700
longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> When external buffer is used, driver should detach it if it doesn't make it
> successfully to the queue.
>
> Signed-off-by: Long Li <longli@microsoft.com>
Good catch
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path
2020-06-24 1:11 [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path longli
2020-06-24 1:11 ` [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure longli
@ 2020-07-02 0:06 ` Stephen Hemminger
2020-07-09 15:28 ` Ferruh Yigit
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2020-07-02 0:06 UTC (permalink / raw)
To: longli; +Cc: Stephen Hemminger, dev, Long Li
On Tue, 23 Jun 2020 18:11:45 -0700
longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> When rte_pktmbuf_attach_extbuf() is used, the driver should not decrease the
> reference count in its callback function hn_rx_buf_free_cb, because the
> reference count is already decreased by rte_pktmbuf. Doing it twice may result
> in underflow and driver may never send an ack packet over vmbus to host.
>
> Also declares rxbuf_outstanding as atomic, because this value is shared
> among all receive queues.
>
> Signed-off-by: Long Li <longli@microsoft.com>
Good catch. DPDK ought to have a real refcnt type like the kernel.
One that traps on underflow.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path
2020-07-02 0:06 ` [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path Stephen Hemminger
@ 2020-07-09 15:28 ` Ferruh Yigit
0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2020-07-09 15:28 UTC (permalink / raw)
To: Stephen Hemminger, longli; +Cc: Stephen Hemminger, dev, Long Li
On 7/2/2020 1:06 AM, Stephen Hemminger wrote:
> On Tue, 23 Jun 2020 18:11:45 -0700
> longli@linuxonhyperv.com wrote:
>
>> From: Long Li <longli@microsoft.com>
>>
>> When rte_pktmbuf_attach_extbuf() is used, the driver should not decrease the
>> reference count in its callback function hn_rx_buf_free_cb, because the
>> reference count is already decreased by rte_pktmbuf. Doing it twice may result
>> in underflow and driver may never send an ack packet over vmbus to host.
>>
>> Also declares rxbuf_outstanding as atomic, because this value is shared
>> among all receive queues.
>>
>> Signed-off-by: Long Li <longli@microsoft.com>
>
> Good catch. DPDK ought to have a real refcnt type like the kernel.
> One that traps on underflow.
>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-09 15:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-24 1:11 [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path longli
2020-06-24 1:11 ` [dpdk-dev] [PATCH 2/2] net/netvsc: detach external buffer on failure longli
2020-07-02 0:05 ` Stephen Hemminger
2020-07-02 0:06 ` [dpdk-dev] [PATCH 1/2] net/netvsc: fix underflow error when external mbuf are used in the receive path Stephen Hemminger
2020-07-09 15:28 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).