drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Change error values of `ionic_tx_map_single()` and `ionic_tx_map_frag()`
from 0 to `DMA_MAPPING_ERROR` to prevent collision with 0 as a valid
address.
This also fixes the use of `dma_mapping_error()` to test against 0 in
`ionic_xdp_post_frame()`
Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Fixes: 56e41ee12d2d ("ionic: better dma-map error handling")
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 | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 2ac59564ded1..d10b58ebf603 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -321,7 +321,7 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
len, DMA_TO_DEVICE);
} else /* XDP_REDIRECT */ {
dma_addr = ionic_tx_map_single(q, frame->data, len);
- if (!dma_addr)
+ if (dma_addr == DMA_MAPPING_ERROR)
return -EIO;
}
@@ -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 == DMA_MAPPING_ERROR) {
ionic_tx_desc_unmap_bufs(q, desc_info);
return -EIO;
}
@@ -1083,7 +1083,7 @@ static dma_addr_t ionic_tx_map_single(struct ionic_queue *q,
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_MAPPING_ERROR;
}
return dma_addr;
}
@@ -1100,7 +1100,7 @@ static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q,
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_MAPPING_ERROR;
}
return dma_addr;
}
@@ -1116,7 +1116,7 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb,
int frag_idx;
dma_addr = ionic_tx_map_single(q, skb->data, skb_headlen(skb));
- if (!dma_addr)
+ if (dma_addr == DMA_MAPPING_ERROR)
return -EIO;
buf_info->dma_addr = dma_addr;
buf_info->len = skb_headlen(skb);
@@ -1126,7 +1126,7 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb,
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)
+ if (dma_addr == DMA_MAPPING_ERROR)
goto dma_fail;
buf_info->dma_addr = dma_addr;
buf_info->len = skb_frag_size(frag);
--
2.43.0
On 6/19/2025 2:45 AM, Thomas Fourier wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Change error values of `ionic_tx_map_single()` and `ionic_tx_map_frag()`
> from 0 to `DMA_MAPPING_ERROR` to prevent collision with 0 as a valid
> address.
>
> This also fixes the use of `dma_mapping_error()` to test against 0 in
> `ionic_xdp_post_frame()`
>
> Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
I'm not sure the Fixes commit above should be in the list. Functionally
it's correct, except there being multiple calls to dma_mapping_error()
on the same dma_addr.
Other than the minor nit above the commit looks good. Thanks again for
fixing this.
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
> Fixes: 56e41ee12d2d ("ionic: better dma-map error handling")
> 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 | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index 2ac59564ded1..d10b58ebf603 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -321,7 +321,7 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
> len, DMA_TO_DEVICE);
> } else /* XDP_REDIRECT */ {
> dma_addr = ionic_tx_map_single(q, frame->data, len);
> - if (!dma_addr)
> + if (dma_addr == DMA_MAPPING_ERROR)
> return -EIO;
> }
>
> @@ -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 == DMA_MAPPING_ERROR) {
> ionic_tx_desc_unmap_bufs(q, desc_info);
> return -EIO;
> }
> @@ -1083,7 +1083,7 @@ static dma_addr_t ionic_tx_map_single(struct ionic_queue *q,
> 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_MAPPING_ERROR;
> }
> return dma_addr;
> }
> @@ -1100,7 +1100,7 @@ static dma_addr_t ionic_tx_map_frag(struct ionic_queue *q,
> 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_MAPPING_ERROR;
> }
> return dma_addr;
> }
> @@ -1116,7 +1116,7 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb,
> int frag_idx;
>
> dma_addr = ionic_tx_map_single(q, skb->data, skb_headlen(skb));
> - if (!dma_addr)
> + if (dma_addr == DMA_MAPPING_ERROR)
> return -EIO;
> buf_info->dma_addr = dma_addr;
> buf_info->len = skb_headlen(skb);
> @@ -1126,7 +1126,7 @@ static int ionic_tx_map_skb(struct ionic_queue *q, struct sk_buff *skb,
> 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)
> + if (dma_addr == DMA_MAPPING_ERROR)
> goto dma_fail;
> buf_info->dma_addr = dma_addr;
> buf_info->len = skb_frag_size(frag);
> --
> 2.43.0
>
On Thu, Jun 19, 2025 at 03:28:06PM -0700, Brett Creeley wrote:
>
>
> On 6/19/2025 2:45 AM, Thomas Fourier wrote:
> > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> >
> >
> > Change error values of `ionic_tx_map_single()` and `ionic_tx_map_frag()`
> > from 0 to `DMA_MAPPING_ERROR` to prevent collision with 0 as a valid
> > address.
> >
> > This also fixes the use of `dma_mapping_error()` to test against 0 in
> > `ionic_xdp_post_frame()`
> >
> > Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
>
> I'm not sure the Fixes commit above should be in the list. Functionally it's
> correct, except there being multiple calls to dma_mapping_error() on the
> same dma_addr.
>
> Other than the minor nit above the commit looks good. Thanks again for
> fixing this.
>
> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
Hi Brett and Thomas,
Maybe I misunderstand things, if so I apologise.
If this patch fixes a bug - e.g. the may observe a system crash -
then it should be targeted at net and have a Fixes tag. Where the
Fixes tag generally cites the first commit in which the user may
experience the bug.
If, on the other hand, this does not fix a bug then the patch
should be targeted at net-next and should not have a Fixes tag.
In that case, commits may be cited using following form in
the commit message (before the Signed-off-by and other tags).
And, unlike tags, it may be line wrapped.
commit 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
E.g.: This was introduce by commit 0f3154e6bcb3 ("ionic: Add Tx and Rx
handling").
I hope this helps. If not, sorry for the noise.
...
On 6/20/2025 3:51 AM, Simon Horman wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Thu, Jun 19, 2025 at 03:28:06PM -0700, Brett Creeley wrote:
>>
>>
>> On 6/19/2025 2:45 AM, Thomas Fourier wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>
>>>
>>> Change error values of `ionic_tx_map_single()` and `ionic_tx_map_frag()`
>>> from 0 to `DMA_MAPPING_ERROR` to prevent collision with 0 as a valid
>>> address.
>>>
>>> This also fixes the use of `dma_mapping_error()` to test against 0 in
>>> `ionic_xdp_post_frame()`
>>>
>>> Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
>>
>> I'm not sure the Fixes commit above should be in the list. Functionally it's
>> correct, except there being multiple calls to dma_mapping_error() on the
>> same dma_addr.
>>
>> Other than the minor nit above the commit looks good. Thanks again for
>> fixing this.
>>
>> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
>
> Hi Brett and Thomas,
>
> Maybe I misunderstand things, if so I apologise.
>
> If this patch fixes a bug - e.g. the may observe a system crash -
> then it should be targeted at net and have a Fixes tag. Where the
> Fixes tag generally cites the first commit in which the user may
> experience the bug.
>
> If, on the other hand, this does not fix a bug then the patch
> should be targeted at net-next and should not have a Fixes tag.
>
> In that case, commits may be cited using following form in
> the commit message (before the Signed-off-by and other tags).
> And, unlike tags, it may be line wrapped.
>
> commit 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
>
> E.g.: This was introduce by commit 0f3154e6bcb3 ("ionic: Add Tx and Rx
> handling").
>
> I hope this helps. If not, sorry for the noise.
Simon,
I suspect you are right and this probably shouldn't be categorized as a
bug fix since the change only addresses a corner case that would happen
if the DMA mapping API(s) return 0 as a valid adddress, which wouldn't
cause a crash with/without this patch.
Thanks for the feedback.
Brett
>
> ...
On Mon, 23 Jun 2025 08:44:39 -0700 Brett Creeley wrote: > I suspect you are right and this probably shouldn't be categorized as a > bug fix since the change only addresses a corner case that would happen > if the DMA mapping API(s) return 0 as a valid adddress, which wouldn't > cause a crash with/without this patch. It's fine either way, so let me apply it and we can move on..
© 2016 - 2025 Red Hat, Inc.