* [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing
@ 2016-07-04 7:51 Wang Xiao W
2016-07-05 9:43 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: Wang Xiao W @ 2016-07-04 7:51 UTC (permalink / raw)
To: jing.d.chen; +Cc: dev, Wang Xiao W
We find that when traffic is light, a few amount of packets will be
wrongly parsed (e.g. packet type), however this issue will not happen
when traffic is heavy.
The root cause is some fields in fm10k_rx_desc are read at wrong timing.
When the input speed is slower than software's capability, fm10k scalar
Rx function accesses descriptors closely to HW, so there's potential
sequence: some fields like pkt_info in fm10k_rx_desc are read before HW
writeback but some fields like DD bit are read after HW writeback, this
will lead to the later packet parsing function using incorrect value.
This patch fixes this issue by reading and parsing Rx descriptor after
DD bit is set.
Fixes: 4b61d3bfa941 ("fm10k: add receive and tranmit")
Fixes: c82dd0a7bfa5 ("fm10k: add scatter receive")
Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
---
drivers/net/fm10k/fm10k_rxtx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index dd92a91..5b2d04b 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -114,10 +114,10 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nb_pkts = RTE_MIN(nb_pkts, q->alloc_thresh);
for (count = 0; count < nb_pkts; ++count) {
+ if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD))
+ break;
mbuf = q->sw_ring[next_dd];
desc = q->hw_ring[next_dd];
- if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
- break;
#ifdef RTE_LIBRTE_FM10K_DEBUG_RX
dump_rxd(&desc);
#endif
@@ -228,10 +228,10 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
nb_seg = RTE_MIN(nb_pkts, q->alloc_thresh);
for (count = 0; count < nb_seg; count++) {
+ if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD))
+ break;
mbuf = q->sw_ring[next_dd];
desc = q->hw_ring[next_dd];
- if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
- break;
#ifdef RTE_LIBRTE_FM10K_DEBUG_RX
dump_rxd(&desc);
#endif
--
1.9.3
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing
2016-07-04 7:51 [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing Wang Xiao W
@ 2016-07-05 9:43 ` Bruce Richardson
2016-07-05 9:47 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: Bruce Richardson @ 2016-07-05 9:43 UTC (permalink / raw)
To: Wang Xiao W; +Cc: jing.d.chen, dev
On Mon, Jul 04, 2016 at 03:51:08PM +0800, Wang Xiao W wrote:
> We find that when traffic is light, a few amount of packets will be
> wrongly parsed (e.g. packet type), however this issue will not happen
> when traffic is heavy.
>
> The root cause is some fields in fm10k_rx_desc are read at wrong timing.
> When the input speed is slower than software's capability, fm10k scalar
> Rx function accesses descriptors closely to HW, so there's potential
> sequence: some fields like pkt_info in fm10k_rx_desc are read before HW
> writeback but some fields like DD bit are read after HW writeback, this
> will lead to the later packet parsing function using incorrect value.
>
> This patch fixes this issue by reading and parsing Rx descriptor after
> DD bit is set.
>
> Fixes: 4b61d3bfa941 ("fm10k: add receive and tranmit")
> Fixes: c82dd0a7bfa5 ("fm10k: add scatter receive")
>
> Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
Fix looks correct to me. The key part I think is that the read of the
descriptor at line "desc = q->hw_ring[next_dd];" is not atomic as it's
a read of 16B, allowing part of the descriptor to be read either side of
a HW write-back.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> drivers/net/fm10k/fm10k_rxtx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
> index dd92a91..5b2d04b 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -114,10 +114,10 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>
> nb_pkts = RTE_MIN(nb_pkts, q->alloc_thresh);
> for (count = 0; count < nb_pkts; ++count) {
> + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD))
> + break;
> mbuf = q->sw_ring[next_dd];
> desc = q->hw_ring[next_dd];
> - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
> - break;
> #ifdef RTE_LIBRTE_FM10K_DEBUG_RX
> dump_rxd(&desc);
> #endif
> @@ -228,10 +228,10 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>
> nb_seg = RTE_MIN(nb_pkts, q->alloc_thresh);
> for (count = 0; count < nb_seg; count++) {
> + if (!(q->hw_ring[next_dd].d.staterr & FM10K_RXD_STATUS_DD))
> + break;
> mbuf = q->sw_ring[next_dd];
> desc = q->hw_ring[next_dd];
> - if (!(desc.d.staterr & FM10K_RXD_STATUS_DD))
> - break;
> #ifdef RTE_LIBRTE_FM10K_DEBUG_RX
> dump_rxd(&desc);
> #endif
> --
> 1.9.3
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing
2016-07-05 9:43 ` Bruce Richardson
@ 2016-07-05 9:47 ` Bruce Richardson
0 siblings, 0 replies; 3+ messages in thread
From: Bruce Richardson @ 2016-07-05 9:47 UTC (permalink / raw)
To: Wang Xiao W; +Cc: jing.d.chen, dev
On Tue, Jul 05, 2016 at 10:43:42AM +0100, Bruce Richardson wrote:
> On Mon, Jul 04, 2016 at 03:51:08PM +0800, Wang Xiao W wrote:
> > We find that when traffic is light, a few amount of packets will be
> > wrongly parsed (e.g. packet type), however this issue will not happen
> > when traffic is heavy.
> >
> > The root cause is some fields in fm10k_rx_desc are read at wrong timing.
> > When the input speed is slower than software's capability, fm10k scalar
> > Rx function accesses descriptors closely to HW, so there's potential
> > sequence: some fields like pkt_info in fm10k_rx_desc are read before HW
> > writeback but some fields like DD bit are read after HW writeback, this
> > will lead to the later packet parsing function using incorrect value.
> >
> > This patch fixes this issue by reading and parsing Rx descriptor after
> > DD bit is set.
> >
> > Fixes: 4b61d3bfa941 ("fm10k: add receive and tranmit")
> > Fixes: c82dd0a7bfa5 ("fm10k: add scatter receive")
> >
> > Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
>
> Fix looks correct to me. The key part I think is that the read of the
> descriptor at line "desc = q->hw_ring[next_dd];" is not atomic as it's
> a read of 16B, allowing part of the descriptor to be read either side of
> a HW write-back.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
Applied to dpdk-next-net/rel_16_07
/Bruce
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-05 9:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 7:51 [dpdk-dev] [PATCH] net/fm10k: fix Rx descriptor read timing Wang Xiao W
2016-07-05 9:43 ` Bruce Richardson
2016-07-05 9:47 ` Bruce Richardson
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).