* [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32'
@ 2024-11-13 13:41 Hardevsinh Palaniya
2024-11-13 14:06 ` Vadim Fedorenko
2024-11-13 20:00 ` Shahab Vahedi
0 siblings, 2 replies; 5+ messages in thread
From: Hardevsinh Palaniya @ 2024-11-13 13:41 UTC (permalink / raw)
To: ast, vadim.fedorenko, list+bpf
Cc: tarang.raval, Hardevsinh Palaniya, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Vineet Gupta, bpf, linux-snps-arc,
linux-kernel
The original code checks 'if (ARC_CC_AL)', which is always true since
ARC_CC_AL is a constant. This makes the check redundant and likely
obscures the intention of verifying whether the jump is conditional.
Updates the code to check cond == ARC_CC_AL instead, reflecting the intent
to differentiate conditional from unconditional jumps.
Suggested-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
---
Changelog in V2:
- Changed subject line
- Updated condition check to 'if (cond == ARC_CC_AL)' instead of removing it
Link for v1: https://lore.kernel.org/bpf/e6d27adb-151c-46c1-9668-1cd2b492321b@linux.dev/T/#t
---
arch/arc/net/bpf_jit_arcv2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arc/net/bpf_jit_arcv2.c b/arch/arc/net/bpf_jit_arcv2.c
index 4458e409ca0a..6d989b6d88c6 100644
--- a/arch/arc/net/bpf_jit_arcv2.c
+++ b/arch/arc/net/bpf_jit_arcv2.c
@@ -2916,7 +2916,7 @@ bool check_jmp_32(u32 curr_off, u32 targ_off, u8 cond)
addendum = (cond == ARC_CC_AL) ? 0 : INSN_len_normal;
disp = get_displacement(curr_off + addendum, targ_off);
- if (ARC_CC_AL)
+ if (cond == ARC_CC_AL)
return is_valid_far_disp(disp);
else
return is_valid_near_disp(disp);
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32'
2024-11-13 13:41 [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32' Hardevsinh Palaniya
@ 2024-11-13 14:06 ` Vadim Fedorenko
2024-11-13 20:00 ` Shahab Vahedi
1 sibling, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-11-13 14:06 UTC (permalink / raw)
To: Hardevsinh Palaniya, ast, list+bpf
Cc: tarang.raval, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Vineet Gupta,
bpf, linux-snps-arc, linux-kernel
On 13/11/2024 13:41, Hardevsinh Palaniya wrote:
> The original code checks 'if (ARC_CC_AL)', which is always true since
> ARC_CC_AL is a constant. This makes the check redundant and likely
> obscures the intention of verifying whether the jump is conditional.
>
> Updates the code to check cond == ARC_CC_AL instead, reflecting the intent
> to differentiate conditional from unconditional jumps.
>
> Suggested-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
> ---
>
> Changelog in V2:
>
> - Changed subject line
> - Updated condition check to 'if (cond == ARC_CC_AL)' instead of removing it
>
> Link for v1: https://lore.kernel.org/bpf/e6d27adb-151c-46c1-9668-1cd2b492321b@linux.dev/T/#t
> ---
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32'
2024-11-13 13:41 [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32' Hardevsinh Palaniya
2024-11-13 14:06 ` Vadim Fedorenko
@ 2024-11-13 20:00 ` Shahab Vahedi
2024-11-15 15:55 ` Shahab Vahedi
1 sibling, 1 reply; 5+ messages in thread
From: Shahab Vahedi @ 2024-11-13 20:00 UTC (permalink / raw)
To: Hardevsinh Palaniya
Cc: ast, vadim.fedorenko, tarang.raval, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Vineet Gupta, bpf, linux-snps-arc,
linux-kernel
> The original code checks 'if (ARC_CC_AL)', which is always true since
> ARC_CC_AL is a constant. This makes the check redundant and likely
> obscures the intention of verifying whether the jump is conditional.
>
> Updates the code to check cond == ARC_CC_AL instead, reflecting the intent
> to differentiate conditional from unconditional jumps.
>
> Suggested-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
> ---
>
> Changelog in V2:
>
> - Changed subject line
> - Updated condition check to 'if (cond == ARC_CC_AL)' instead of removing it
>
> Link for v1: https://lore.kernel.org/bpf/e6d27adb-151c-46c1-9668-1cd2b492321b@linux.dev/T/#t
> ---
> arch/arc/net/bpf_jit_arcv2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arc/net/bpf_jit_arcv2.c b/arch/arc/net/bpf_jit_arcv2.c
> index 4458e409ca0a..6d989b6d88c6 100644
> --- a/arch/arc/net/bpf_jit_arcv2.c
> +++ b/arch/arc/net/bpf_jit_arcv2.c
> @@ -2916,7 +2916,7 @@ bool check_jmp_32(u32 curr_off, u32 targ_off, u8 cond)
> addendum = (cond == ARC_CC_AL) ? 0 : INSN_len_normal;
> disp = get_displacement(curr_off + addendum, targ_off);
>
> - if (ARC_CC_AL)
> + if (cond == ARC_CC_AL)
> return is_valid_far_disp(disp);
> else
> return is_valid_near_disp(disp);
> --
> 2.43.0
Thank you for your contribution!
Acked-by: Shahab Vahedi <list+bpf@vahedi.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32'
2024-11-13 20:00 ` Shahab Vahedi
@ 2024-11-15 15:55 ` Shahab Vahedi
2024-11-16 0:32 ` Vineet Gupta
0 siblings, 1 reply; 5+ messages in thread
From: Shahab Vahedi @ 2024-11-15 15:55 UTC (permalink / raw)
To: Vineet Gupta
Cc: vadim.fedorenko, tarang.raval, Vineet Gupta, bpf, linux-snps-arc,
linux-kernel, Hardevsinh Palaniya, Shahab Vahedi
Hi Vineet,
Could you pick up this patch [1] in your "next"?
Thanks,
Shahab
[1]
https://lore.kernel.org/bpf/920e71ab-2375-4722-bcf3-d6aaf8e68b3a@vahedi.org/T/#t
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32'
2024-11-15 15:55 ` Shahab Vahedi
@ 2024-11-16 0:32 ` Vineet Gupta
0 siblings, 0 replies; 5+ messages in thread
From: Vineet Gupta @ 2024-11-16 0:32 UTC (permalink / raw)
To: Shahab Vahedi
Cc: vadim.fedorenko, tarang.raval, Vineet Gupta, bpf, linux-snps-arc,
linux-kernel, Hardevsinh Palaniya
On 11/15/24 07:55, Shahab Vahedi wrote:
> Hi Vineet,
>
> Could you pick up this patch [1] in your "next"?
>
> Thanks,
> Shahab
>
> [1]
> https://lore.kernel.org/bpf/920e71ab-2375-4722-bcf3-d6aaf8e68b3a@vahedi.org/T/#t
Done. Given We are in the last week this all will land in 6.13 cycle.
-Vineet
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-16 0:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-13 13:41 [PATCH v2] ARC: bpf: Correct conditional check in 'check_jmp_32' Hardevsinh Palaniya
2024-11-13 14:06 ` Vadim Fedorenko
2024-11-13 20:00 ` Shahab Vahedi
2024-11-15 15:55 ` Shahab Vahedi
2024-11-16 0:32 ` Vineet Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox