* [PATCH net-next] net: ethernet: mtk_wed: do not assume offload callbacks are always set
@ 2023-09-12 8:26 Lorenzo Bianconi
2023-09-12 18:06 ` Simon Horman
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2023-09-12 8:26 UTC (permalink / raw)
To: netdev
Cc: lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee, davem,
edumazet, kuba, pabeni
Check if wlan.offload_enable and wlan.offload_disable callbacks are set
in mtk_wed_flow_add/mtk_wed_flow_remove since mt7996 will not rely
on them.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/ethernet/mediatek/mtk_wed.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 94376aa2b34c..d8cd59f44401 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -1718,6 +1718,9 @@ int mtk_wed_flow_add(int index)
if (!hw || !hw->wed_dev)
return -ENODEV;
+ if (!hw->wed_dev->wlan.offload_enable)
+ return 0;
+
if (hw->num_flows) {
hw->num_flows++;
return 0;
@@ -1747,6 +1750,9 @@ void mtk_wed_flow_remove(int index)
if (!hw)
return;
+ if (!hw->wed_dev->wlan.offload_disable)
+ return;
+
if (--hw->num_flows)
return;
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: ethernet: mtk_wed: do not assume offload callbacks are always set
2023-09-12 8:26 [PATCH net-next] net: ethernet: mtk_wed: do not assume offload callbacks are always set Lorenzo Bianconi
@ 2023-09-12 18:06 ` Simon Horman
2023-09-12 20:31 ` Lorenzo Bianconi
0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2023-09-12 18:06 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: netdev, lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee,
davem, edumazet, kuba, pabeni
On Tue, Sep 12, 2023 at 10:26:07AM +0200, Lorenzo Bianconi wrote:
> Check if wlan.offload_enable and wlan.offload_disable callbacks are set
> in mtk_wed_flow_add/mtk_wed_flow_remove since mt7996 will not rely
> on them.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Hi Lorenzo,
It's not not a big deal from my perspective, but
I do wonder if these mediatek patches could have been a series.
> ---
> drivers/net/ethernet/mediatek/mtk_wed.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index 94376aa2b34c..d8cd59f44401 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -1718,6 +1718,9 @@ int mtk_wed_flow_add(int index)
> if (!hw || !hw->wed_dev)
> return -ENODEV;
>
> + if (!hw->wed_dev->wlan.offload_enable)
> + return 0;
A little further down in this function it is assumed that hw->wed_dev may
be NULL, a check made under a lock no less. But it is dereferenced
unconditionally here without a lock. This doesn't seem right one way or
another.
As flagged by Smatch.
> +
> if (hw->num_flows) {
> hw->num_flows++;
> return 0;
> @@ -1747,6 +1750,9 @@ void mtk_wed_flow_remove(int index)
> if (!hw)
> return;
>
> + if (!hw->wed_dev->wlan.offload_disable)
> + return;
> +
> if (--hw->num_flows)
> return;
>
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: ethernet: mtk_wed: do not assume offload callbacks are always set
2023-09-12 18:06 ` Simon Horman
@ 2023-09-12 20:31 ` Lorenzo Bianconi
0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2023-09-12 20:31 UTC (permalink / raw)
To: Simon Horman
Cc: netdev, lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee,
davem, edumazet, kuba, pabeni
[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]
> On Tue, Sep 12, 2023 at 10:26:07AM +0200, Lorenzo Bianconi wrote:
> > Check if wlan.offload_enable and wlan.offload_disable callbacks are set
> > in mtk_wed_flow_add/mtk_wed_flow_remove since mt7996 will not rely
> > on them.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>
> Hi Lorenzo,
>
> It's not not a big deal from my perspective, but
> I do wonder if these mediatek patches could have been a series.
>
> > ---
> > drivers/net/ethernet/mediatek/mtk_wed.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index 94376aa2b34c..d8cd59f44401 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -1718,6 +1718,9 @@ int mtk_wed_flow_add(int index)
> > if (!hw || !hw->wed_dev)
> > return -ENODEV;
> >
> > + if (!hw->wed_dev->wlan.offload_enable)
> > + return 0;
>
> A little further down in this function it is assumed that hw->wed_dev may
> be NULL, a check made under a lock no less. But it is dereferenced
> unconditionally here without a lock. This doesn't seem right one way or
> another.
>
> As flagged by Smatch.
Hi Simon,
you are right. I will fix it in v2.
Regards,
Lorenzo
>
> > +
> > if (hw->num_flows) {
> > hw->num_flows++;
> > return 0;
> > @@ -1747,6 +1750,9 @@ void mtk_wed_flow_remove(int index)
> > if (!hw)
> > return;
> >
> > + if (!hw->wed_dev->wlan.offload_disable)
> > + return;
> > +
> > if (--hw->num_flows)
> > return;
> >
> > --
> > 2.41.0
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-12 20:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 8:26 [PATCH net-next] net: ethernet: mtk_wed: do not assume offload callbacks are always set Lorenzo Bianconi
2023-09-12 18:06 ` Simon Horman
2023-09-12 20:31 ` Lorenzo Bianconi
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).