* [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()`
@ 2025-06-17 9:18 Thomas Fourier
2025-06-17 14:35 ` Michal Swiatkowski
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Thomas Fourier @ 2025-06-17 9:18 UTC (permalink / raw)
Cc: Thomas Fourier, Shannon Nelson, Brett Creeley, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Vladimir Oltean,
Caleb Sander Mateos, Taehee Yoo, netdev, linux-kernel, bpf
The `ionic_tx_map_frag()` wrapper function is used which returns 0 or a
valid DMA address. Testing that pointer with `dma_mapping_error()`could
be eroneous since the error value exptected by `dma_mapping_error()` is
not 0 but `DMA_MAPPING_ERROR` which is often ~0.
Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 2ac59564ded1..beefdc43013e 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -357,7 +357,7 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
} else {
dma_addr = ionic_tx_map_frag(q, frag, 0,
skb_frag_size(frag));
- if (dma_mapping_error(q->dev, dma_addr)) {
+ if (!dma_addr) {
ionic_tx_desc_unmap_bufs(q, desc_info);
return -EIO;
}
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` 2025-06-17 9:18 [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Thomas Fourier @ 2025-06-17 14:35 ` Michal Swiatkowski 2025-06-17 15:01 ` Alexander Lobakin 2025-06-17 16:56 ` Brett Creeley 2 siblings, 0 replies; 7+ messages in thread From: Michal Swiatkowski @ 2025-06-17 14:35 UTC (permalink / raw) To: Thomas Fourier Cc: Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Vladimir Oltean, Caleb Sander Mateos, Taehee Yoo, netdev, linux-kernel, bpf On Tue, Jun 17, 2025 at 11:18:36AM +0200, Thomas Fourier wrote: > The `ionic_tx_map_frag()` wrapper function is used which returns 0 or a > valid DMA address. Testing that pointer with `dma_mapping_error()`could > be eroneous since the error value exptected by `dma_mapping_error()` is > not 0 but `DMA_MAPPING_ERROR` which is often ~0. > > Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> > --- > drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > index 2ac59564ded1..beefdc43013e 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > @@ -357,7 +357,7 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, > } else { > dma_addr = ionic_tx_map_frag(q, frag, 0, > skb_frag_size(frag)); > - if (dma_mapping_error(q->dev, dma_addr)) { > + if (!dma_addr) { > ionic_tx_desc_unmap_bufs(q, desc_info); > return -EIO; Right, in ionic_tx_map_skb() it is used correctly (like here now). Thanks for fixing. Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > } > -- > 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` 2025-06-17 9:18 [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Thomas Fourier 2025-06-17 14:35 ` Michal Swiatkowski @ 2025-06-17 15:01 ` Alexander Lobakin 2025-06-17 16:56 ` Brett Creeley 2 siblings, 0 replies; 7+ messages in thread From: Alexander Lobakin @ 2025-06-17 15:01 UTC (permalink / raw) To: Thomas Fourier Cc: Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Vladimir Oltean, Caleb Sander Mateos, Taehee Yoo, netdev, linux-kernel, bpf From: Thomas Fourier <fourier.thomas@gmail.com> Date: Tue, 17 Jun 2025 11:18:36 +0200 > The `ionic_tx_map_frag()` wrapper function is used which returns 0 or a > valid DMA address. Testing that pointer with `dma_mapping_error()`could > be eroneous since the error value exptected by `dma_mapping_error()` is > not 0 but `DMA_MAPPING_ERROR` which is often ~0. BTW I remember clearly that Chris wrote that dma_addr == 0 is a perfectly valid DMA address, that's why ~0 was picked for error checking. So maybe it's better to fix the driver. I realize that in the real world, it's almost impossible to get dma_addr == 0 from dma_map_*(), but still. > > Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> Thanks, Olek ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` 2025-06-17 9:18 [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Thomas Fourier 2025-06-17 14:35 ` Michal Swiatkowski 2025-06-17 15:01 ` Alexander Lobakin @ 2025-06-17 16:56 ` Brett Creeley 2025-06-17 18:57 ` [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests Thomas Fourier 2025-06-18 17:05 ` [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Alexander Lobakin 2 siblings, 2 replies; 7+ messages in thread From: Brett Creeley @ 2025-06-17 16:56 UTC (permalink / raw) To: Thomas Fourier Cc: Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Vladimir Oltean, Caleb Sander Mateos, Taehee Yoo, netdev, linux-kernel, bpf On 6/17/2025 2:18 AM, Thomas Fourier wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > The `ionic_tx_map_frag()` wrapper function is used which returns 0 or a > valid DMA address. Testing that pointer with `dma_mapping_error()`could > be eroneous since the error value exptected by `dma_mapping_error()` is > not 0 but `DMA_MAPPING_ERROR` which is often ~0. > > Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> > --- > drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > index 2ac59564ded1..beefdc43013e 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > @@ -357,7 +357,7 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, > } else { > dma_addr = ionic_tx_map_frag(q, frag, 0, > skb_frag_size(frag)); > - if (dma_mapping_error(q->dev, dma_addr)) { > + if (!dma_addr) { Thanks for the fix. After looking at this and Olek's comment, I think it makes the most sense to return DMA_MAPPING_ERROR from ionic_tx_map_frag() and ionic_tx_map_single() instead of 0 on failures. Then any callers would do the following check: if (unlikely(dma_addr == DMA_MAPPING_ERROR)) /* failure path */ Another option is always returning dma_addr regardless of success/failure from the ionic_tx_map* functions, but then I'd be inclined to use dma_mapping_error() again in the caller. This approach seems wrong to me, especially when CONFIG_DMA_API_DEBUG is enabled. Thanks, Brett > ionic_tx_desc_unmap_bufs(q, desc_info); > return -EIO; > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests 2025-06-17 16:56 ` Brett Creeley @ 2025-06-17 18:57 ` Thomas Fourier 2025-06-18 23:15 ` Brett Creeley 2025-06-18 17:05 ` [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Alexander Lobakin 1 sibling, 1 reply; 7+ messages in thread From: Thomas Fourier @ 2025-06-17 18:57 UTC (permalink / raw) Cc: Thomas Fourier, Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Arthur Kiyanovski, Taehee Yoo, Caleb Sander Mateos, netdev, linux-kernel Removing wrappers around `dma_map_XXX()` to prevent collision between 0 as a valid address and 0 as an error code. Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> --- Another solution is to remove the wrappers altogether so that it doesn't call multiple times the `dma_mapping_error()`. This also makes the code more similar to other calls of the DMA mapping API (even if wrappers around the DMA API are quite common, checking the return code of mapping in the wrapper does not seem to be common). Thomas .../net/ethernet/pensando/ionic/ionic_txrx.c | 70 ++++++------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c index 2ac59564ded1..1905790d0c4d 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c @@ -12,12 +12,7 @@ #include "ionic_lif.h" #include "ionic_txrx.h" -static dma_addr_t ionic_tx_map_single(struct ionic_queue *q, - void *data, size_t len); -static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q, - const skb_frag_t *frag, - size_t offset, size_t len); static void ionic_tx_desc_unmap_bufs(struct ionic_queue *q, struct ionic_tx_desc_info *desc_info); @@ -320,9 +315,9 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, dma_sync_single_for_device(q->dev, dma_addr, len, DMA_TO_DEVICE); } else /* XDP_REDIRECT */ { - dma_addr = ionic_tx_map_single(q, frame->data, len); - if (!dma_addr) - return -EIO; + dma_addr = dma_map_single(q->dev, frame->data, len, DMA_TO_DEVICE); + if (dma_mapping_error(q->dev, dma_addr)) + goto dma_err; } buf_info->dma_addr = dma_addr; @@ -355,11 +350,12 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, skb_frag_size(frag), DMA_TO_DEVICE); } else { - dma_addr = ionic_tx_map_frag(q, frag, 0, - skb_frag_size(frag)); + dma_addr = skb_frag_dma_map(q->dev, frag, 0, + skb_frag_size(frag), + DMA_TO_DEVICE); if (dma_mapping_error(q->dev, dma_addr)) { ionic_tx_desc_unmap_bufs(q, desc_info); - return -EIO; + goto dma_err; } } bi->dma_addr = dma_addr; @@ -388,6 +384,12 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, ionic_txq_post(q, ring_doorbell); return 0; + +dma_err: + net_warn_ratelimited("%s: DMA map failed on %s!\n", + dev_name(q->dev), q->name); + q_to_tx_stats(q)->dma_map_err++; + return -EIO; } int ionic_xdp_xmit(struct net_device *netdev, int n, @@ -1072,38 +1074,6 @@ int ionic_txrx_napi(struct napi_struct *napi, int budget) return rx_work_done; } -static dma_addr_t ionic_tx_map_single(struct ionic_queue *q, - void *data, size_t len) -{ - struct device *dev = q->dev; - dma_addr_t dma_addr; - - dma_addr = dma_map_single(dev, data, len, DMA_TO_DEVICE); - if (unlikely(dma_mapping_error(dev, dma_addr))) { - net_warn_ratelimited("%s: DMA single map failed on %s!\n", - dev_name(dev), q->name); - q_to_tx_stats(q)->dma_map_err++; - return 0; - } - return dma_addr; -} - -static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q, - const skb_frag_t *frag, - size_t offset, size_t len) -{ - struct device *dev = q->dev; - dma_addr_t dma_addr; - - dma_addr = skb_frag_dma_map(dev, frag, offset, len, DMA_TO_DEVICE); - if (unlikely(dma_mapping_error(dev, dma_addr))) { - net_warn_ratelimited("%s: DMA frag map failed on %s!\n", - dev_name(dev), q->name); - q_to_tx_stats(q)->dma_map_err++; - return 0; - } - return dma_addr; -} static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, struct ionic_tx_desc_info *desc_info) @@ -1115,9 +1085,9 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, skb_frag_t *frag; int frag_idx; - dma_addr = ionic_tx_map_single(q, skb->data, skb_headlen(skb)); - if (!dma_addr) - return -EIO; + dma_addr = dma_map_single(q->dev, skb->data, skb_headlen(skb), DMA_TO_DEVICE); + if (dma_mapping_error(q->dev, dma_addr)) + goto dma_early_fail; buf_info->dma_addr = dma_addr; buf_info->len = skb_headlen(skb); buf_info++; @@ -1125,8 +1095,8 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, frag = skb_shinfo(skb)->frags; nfrags = skb_shinfo(skb)->nr_frags; for (frag_idx = 0; frag_idx < nfrags; frag_idx++, frag++) { - dma_addr = ionic_tx_map_frag(q, frag, 0, skb_frag_size(frag)); - if (!dma_addr) + dma_addr = skb_frag_dma_map(q->dev, frag, 0, skb_frag_size(frag), DMA_TO_DEVICE); + if (dma_mapping_error(q->dev, dma_addr)) goto dma_fail; buf_info->dma_addr = dma_addr; buf_info->len = skb_frag_size(frag); @@ -1147,6 +1117,10 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, } dma_unmap_single(dev, desc_info->bufs[0].dma_addr, desc_info->bufs[0].len, DMA_TO_DEVICE); +dma_early_fail: + net_warn_ratelimited("%s: DMA map failed on %s!\n", + dev_name(dev), q->name); + q_to_tx_stats(q)->dma_map_err++; return -EIO; } -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests 2025-06-17 18:57 ` [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests Thomas Fourier @ 2025-06-18 23:15 ` Brett Creeley 0 siblings, 0 replies; 7+ messages in thread From: Brett Creeley @ 2025-06-18 23:15 UTC (permalink / raw) To: Thomas Fourier Cc: Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Arthur Kiyanovski, Taehee Yoo, Caleb Sander Mateos, netdev, linux-kernel On 6/17/2025 11:57 AM, Thomas Fourier wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Removing wrappers around `dma_map_XXX()` to prevent collision between > 0 as a valid address and 0 as an error code. > > Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") > Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> > --- > Another solution is to remove the wrappers altogether so that it doesn't > call multiple times the `dma_mapping_error()`. This also makes the code > more similar to other calls of the DMA mapping API (even if wrappers > around the DMA API are quite common, checking the return code of mapping > in the wrapper does not seem to be common). This is one solution, but this requires adding extra goto labels and pulling the print(s) outside of the helper functions. It also increases the size of the diff from a fixes patch that could be a few lines. I would prefer returning DMA_MAPPING_ERROR from the ionic_tx_map_single() and ionic_tx_map_frag() on failure and checking for that in the callers. Thanks, Brett > > Thomas > > .../net/ethernet/pensando/ionic/ionic_txrx.c | 70 ++++++------------- > 1 file changed, 22 insertions(+), 48 deletions(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > index 2ac59564ded1..1905790d0c4d 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > @@ -12,12 +12,7 @@ > #include "ionic_lif.h" > #include "ionic_txrx.h" > > -static dma_addr_t ionic_tx_map_single(struct ionic_queue *q, > - void *data, size_t len); > > -static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q, > - const skb_frag_t *frag, > - size_t offset, size_t len); > > static void ionic_tx_desc_unmap_bufs(struct ionic_queue *q, > struct ionic_tx_desc_info *desc_info); > @@ -320,9 +315,9 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, > dma_sync_single_for_device(q->dev, dma_addr, > len, DMA_TO_DEVICE); > } else /* XDP_REDIRECT */ { > - dma_addr = ionic_tx_map_single(q, frame->data, len); > - if (!dma_addr) > - return -EIO; > + dma_addr = dma_map_single(q->dev, frame->data, len, DMA_TO_DEVICE); > + if (dma_mapping_error(q->dev, dma_addr)) > + goto dma_err; > } > > buf_info->dma_addr = dma_addr; > @@ -355,11 +350,12 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, > skb_frag_size(frag), > DMA_TO_DEVICE); > } else { > - dma_addr = ionic_tx_map_frag(q, frag, 0, > - skb_frag_size(frag)); > + dma_addr = skb_frag_dma_map(q->dev, frag, 0, > + skb_frag_size(frag), > + DMA_TO_DEVICE); > if (dma_mapping_error(q->dev, dma_addr)) { > ionic_tx_desc_unmap_bufs(q, desc_info); > - return -EIO; > + goto dma_err; > } > } > bi->dma_addr = dma_addr; > @@ -388,6 +384,12 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame, > ionic_txq_post(q, ring_doorbell); > > return 0; > + > +dma_err: > + net_warn_ratelimited("%s: DMA map failed on %s!\n", > + dev_name(q->dev), q->name); > + q_to_tx_stats(q)->dma_map_err++; > + return -EIO; > } > > int ionic_xdp_xmit(struct net_device *netdev, int n, > @@ -1072,38 +1074,6 @@ int ionic_txrx_napi(struct napi_struct *napi, int budget) > return rx_work_done; > } > > -static dma_addr_t ionic_tx_map_single(struct ionic_queue *q, > - void *data, size_t len) > -{ > - struct device *dev = q->dev; > - dma_addr_t dma_addr; > - > - dma_addr = dma_map_single(dev, data, len, DMA_TO_DEVICE); > - if (unlikely(dma_mapping_error(dev, dma_addr))) { > - net_warn_ratelimited("%s: DMA single map failed on %s!\n", > - dev_name(dev), q->name); > - q_to_tx_stats(q)->dma_map_err++; > - return 0; > - } > - return dma_addr; > -} > - > -static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q, > - const skb_frag_t *frag, > - size_t offset, size_t len) > -{ > - struct device *dev = q->dev; > - dma_addr_t dma_addr; > - > - dma_addr = skb_frag_dma_map(dev, frag, offset, len, DMA_TO_DEVICE); > - if (unlikely(dma_mapping_error(dev, dma_addr))) { > - net_warn_ratelimited("%s: DMA frag map failed on %s!\n", > - dev_name(dev), q->name); > - q_to_tx_stats(q)->dma_map_err++; > - return 0; > - } > - return dma_addr; > -} > > static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, > struct ionic_tx_desc_info *desc_info) > @@ -1115,9 +1085,9 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, > skb_frag_t *frag; > int frag_idx; > > - dma_addr = ionic_tx_map_single(q, skb->data, skb_headlen(skb)); > - if (!dma_addr) > - return -EIO; > + dma_addr = dma_map_single(q->dev, skb->data, skb_headlen(skb), DMA_TO_DEVICE); > + if (dma_mapping_error(q->dev, dma_addr)) > + goto dma_early_fail; > buf_info->dma_addr = dma_addr; > buf_info->len = skb_headlen(skb); > buf_info++; > @@ -1125,8 +1095,8 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, > frag = skb_shinfo(skb)->frags; > nfrags = skb_shinfo(skb)->nr_frags; > for (frag_idx = 0; frag_idx < nfrags; frag_idx++, frag++) { > - dma_addr = ionic_tx_map_frag(q, frag, 0, skb_frag_size(frag)); > - if (!dma_addr) > + dma_addr = skb_frag_dma_map(q->dev, frag, 0, skb_frag_size(frag), DMA_TO_DEVICE); > + if (dma_mapping_error(q->dev, dma_addr)) > goto dma_fail; > buf_info->dma_addr = dma_addr; > buf_info->len = skb_frag_size(frag); > @@ -1147,6 +1117,10 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb, > } > dma_unmap_single(dev, desc_info->bufs[0].dma_addr, > desc_info->bufs[0].len, DMA_TO_DEVICE); > +dma_early_fail: > + net_warn_ratelimited("%s: DMA map failed on %s!\n", > + dev_name(dev), q->name); > + q_to_tx_stats(q)->dma_map_err++; > return -EIO; > } > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` 2025-06-17 16:56 ` Brett Creeley 2025-06-17 18:57 ` [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests Thomas Fourier @ 2025-06-18 17:05 ` Alexander Lobakin 1 sibling, 0 replies; 7+ messages in thread From: Alexander Lobakin @ 2025-06-18 17:05 UTC (permalink / raw) To: Brett Creeley, Thomas Fourier Cc: Shannon Nelson, Brett Creeley, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev, Vladimir Oltean, Caleb Sander Mateos, Taehee Yoo, netdev, linux-kernel, bpf From: Brett Creeley <bcreeley@amd.com> Date: Tue, 17 Jun 2025 09:56:55 -0700 > On 6/17/2025 2:18 AM, Thomas Fourier wrote: >> Caution: This message originated from an External Source. Use proper >> caution when opening attachments, clicking links, or responding. >> >> >> The `ionic_tx_map_frag()` wrapper function is used which returns 0 or a >> valid DMA address. Testing that pointer with `dma_mapping_error()`could >> be eroneous since the error value exptected by `dma_mapping_error()` is >> not 0 but `DMA_MAPPING_ERROR` which is often ~0. >> >> Fixes: ac8813c0ab7d ("ionic: convert Rx queue buffers to use page_pool") >> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com> >> --- >> drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/ >> drivers/net/ethernet/pensando/ionic/ionic_txrx.c >> index 2ac59564ded1..beefdc43013e 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c >> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c >> @@ -357,7 +357,7 @@ static int ionic_xdp_post_frame(struct ionic_queue >> *q, struct xdp_frame *frame, >> } else { >> dma_addr = ionic_tx_map_frag(q, frag, 0, >> >> skb_frag_size(frag)); >> - if (dma_mapping_error(q->dev, >> dma_addr)) { >> + if (!dma_addr) { > > Thanks for the fix. > > After looking at this and Olek's comment, I think it makes the most > sense to return DMA_MAPPING_ERROR from ionic_tx_map_frag() and > ionic_tx_map_single() instead of 0 on failures. > > Then any callers would do the following check: > > if (unlikely(dma_addr == DMA_MAPPING_ERROR)) > /* failure path */ > > Another option is always returning dma_addr regardless of success/ > failure from the ionic_tx_map* functions, but then I'd be inclined to > use dma_mapping_error() again in the caller. This approach seems wrong > to me, especially when CONFIG_DMA_API_DEBUG is enabled. Yup, I'd agree that dma_mapping_error() should be used only for checking return values of the generic DMA API functions and only once per one mapping. Otherwise, it's fine to pass DMA_MAPPING_ERROR and test against it directly (after dma_mapping_error() was called already). > > Thanks, > > Brett Thanks, Olek ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-18 23:15 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-06-17 9:18 [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Thomas Fourier 2025-06-17 14:35 ` Michal Swiatkowski 2025-06-17 15:01 ` Alexander Lobakin 2025-06-17 16:56 ` Brett Creeley 2025-06-17 18:57 ` [PATCH net,v2] ethernet: ionic: Fix DMA mapping tests Thomas Fourier 2025-06-18 23:15 ` Brett Creeley 2025-06-18 17:05 ` [PATCH net] ethernet: ionic: Fix DMA mapping test in `ionic_xdp_post_frame()` Alexander Lobakin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®