2021-07-28 09:11:07

by Yang Yingliang

[permalink] [raw]
Subject: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()

If nfp_tunnel_add_ipv6_off() fails, it should return error code
in nfp_fl_ct_add_offload().

Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/net/ethernet/netronome/nfp/flower/conntrack.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
index 1ac3b65df600..bfd7d1c35076 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
@@ -710,8 +710,10 @@ static int nfp_fl_ct_add_offload(struct nfp_fl_nft_tc_merge *m_entry)
dst = &gre_match->ipv6.dst;

entry = nfp_tunnel_add_ipv6_off(priv->app, dst);
- if (!entry)
+ if (!entry) {
+ err = -ENOMEM;
goto ct_offload_err;
+ }

flow_pay->nfp_tun_ipv6 = entry;
} else {
@@ -760,8 +762,10 @@ static int nfp_fl_ct_add_offload(struct nfp_fl_nft_tc_merge *m_entry)
dst = &udp_match->ipv6.dst;

entry = nfp_tunnel_add_ipv6_off(priv->app, dst);
- if (!entry)
+ if (!entry) {
+ err = -ENOMEM;
goto ct_offload_err;
+ }

flow_pay->nfp_tun_ipv6 = entry;
} else {
--
2.25.1



2021-07-28 09:41:22

by Louis Peens

[permalink] [raw]
Subject: Re: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()



On 2021/07/28 11:16, Yang Yingliang wrote:
> If nfp_tunnel_add_ipv6_off() fails, it should return error code
> in nfp_fl_ct_add_offload().
>
> Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>
Ah, thanks Yang, I was just preparing a patch for this myself. This was first reported by
Dan Carpenter <[email protected]> on 26 Jul 2021 (added to CC).

'Hello Louis Peens,

The patch 5a2b93041646: "nfp: flower-ct: compile match sections of
flow_payload" from Jul 22, 2021, leads to the following static
checker warning:
.....'

I'm not sure what the usual procedure would be for this, I would think adding
another "Reported-by" line would be sufficient?

Anyway, for the patch itself the change looks good to me, thanks:
Signed-off-by: Louis Peens <[email protected]>
> ---
> drivers/net/ethernet/netronome/nfp/flower/conntrack.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
> index 1ac3b65df600..bfd7d1c35076 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/conntrack.c
> @@ -710,8 +710,10 @@ static int nfp_fl_ct_add_offload(struct nfp_fl_nft_tc_merge *m_entry)
> dst = &gre_match->ipv6.dst;
>
> entry = nfp_tunnel_add_ipv6_off(priv->app, dst);
> - if (!entry)
> + if (!entry) {
> + err = -ENOMEM;
> goto ct_offload_err;
> + }
>
> flow_pay->nfp_tun_ipv6 = entry;
> } else {
> @@ -760,8 +762,10 @@ static int nfp_fl_ct_add_offload(struct nfp_fl_nft_tc_merge *m_entry)
> dst = &udp_match->ipv6.dst;
>
> entry = nfp_tunnel_add_ipv6_off(priv->app, dst);
> - if (!entry)
> + if (!entry) {
> + err = -ENOMEM;
> goto ct_offload_err;
> + }
>
> flow_pay->nfp_tun_ipv6 = entry;
> } else {
>

2021-07-28 09:58:15

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()

On Wed, Jul 28, 2021 at 11:36:43AM +0200, Louis Peens wrote:
>
>
> On 2021/07/28 11:16, Yang Yingliang wrote:
> > If nfp_tunnel_add_ipv6_off() fails, it should return error code
> > in nfp_fl_ct_add_offload().
> >
> > Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: Yang Yingliang <[email protected]>
> Ah, thanks Yang, I was just preparing a patch for this myself. This was first reported by
> Dan Carpenter <[email protected]> on 26 Jul 2021 (added to CC).
>
> 'Hello Louis Peens,
>
> The patch 5a2b93041646: "nfp: flower-ct: compile match sections of
> flow_payload" from Jul 22, 2021, leads to the following static
> checker warning:
> .....'
>
> I'm not sure what the usual procedure would be for this, I would think adding
> another "Reported-by" line would be sufficient?'

Just leave it, it's fine.

>
> Anyway, for the patch itself the change looks good to me, thanks:
> Signed-off-by: Louis Peens <[email protected]>

Normally it would be Acked-by. Signed-off-by means you handled the
patch and it's like signing a legal document that you didn't violate
SCO copyrights etc.

regards,
dan carpenter


2021-07-28 10:09:56

by Louis Peens

[permalink] [raw]
Subject: Re: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()



On 2021/07/28 11:56, Dan Carpenter wrote:
> On Wed, Jul 28, 2021 at 11:36:43AM +0200, Louis Peens wrote:
>>
>>
>> On 2021/07/28 11:16, Yang Yingliang wrote:
>>> If nfp_tunnel_add_ipv6_off() fails, it should return error code
>>> in nfp_fl_ct_add_offload().
>>>
>>> Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
>>> Reported-by: Hulk Robot <[email protected]>
>>> Signed-off-by: Yang Yingliang <[email protected]>
>> Ah, thanks Yang, I was just preparing a patch for this myself. This was first reported by
>> Dan Carpenter <[email protected]> on 26 Jul 2021 (added to CC).
>>
>> 'Hello Louis Peens,
>>
>> The patch 5a2b93041646: "nfp: flower-ct: compile match sections of
>> flow_payload" from Jul 22, 2021, leads to the following static
>> checker warning:
>> .....'
>>
>> I'm not sure what the usual procedure would be for this, I would think adding
>> another "Reported-by" line would be sufficient?'
>
> Just leave it, it's fine.
>
>>
>> Anyway, for the patch itself the change looks good to me, thanks:
>> Signed-off-by: Louis Peens <[email protected]>
>
> Normally it would be Acked-by. Signed-off-by means you handled the
> patch and it's like signing a legal document that you didn't violate
> SCO copyrights etc.
ack :) Thanks for the clarification, the distinction does confuse me,
thinking about it this way would definitely help.

Regards
Louis Peens
>
> regards,
> dan carpenter
>

2021-07-28 11:50:50

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()

On Wed, Jul 28, 2021 at 05:16:31PM +0800, Yang Yingliang wrote:
> If nfp_tunnel_add_ipv6_off() fails, it should return error code
> in nfp_fl_ct_add_offload().
>
> Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2021-07-28 12:01:22

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Wed, 28 Jul 2021 17:16:31 +0800 you wrote:
> If nfp_tunnel_add_ipv6_off() fails, it should return error code
> in nfp_fl_ct_add_offload().
>
> Fixes: 5a2b93041646 ("nfp: flower-ct: compile match sections of flow_payload")
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: Yang Yingliang <[email protected]>
>
> [...]

Here is the summary with links:
- [net-next] nfp: flower-ct: fix error return code in nfp_fl_ct_add_offload()
https://git.kernel.org/netdev/net-next/c/d80f6d6665a6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html