Re: [ovs-dev] [PATCH ovn] ovn-trace: Use the original ovnact for execute_load
Thanks Ales and Dumitru, I merged this to main and all branches back to 21.12. On 1/17/23 11:10, Dumitru Ceara wrote: On 12/15/22 10:13, Ales Musil wrote: Compiling with GCC and -fsanitize=undefined throws a warning [0]. To avoid that, use the original ovnact pointer with the inner size rather than getting the inner "struct ovnact" from the "struct ovnact_load". At the same time simplify the computation of size for ovnacts_format call. [0] In function ‘execute_load’, inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13: utilities/ovn-trace.c:1501:5: error: ‘ovnacts_encode’ reading 4 bytes from a region of size 1 [-Werror=stringop-overread] 1501 | ovnacts_encode(>ovnact, sizeof *load, , ); | ^~ utilities/ovn-trace.c:1501:5: note: referencing argument 1 of type ‘const struct ovnact[0]’ In file included from utilities/ovn-trace.c:35: ./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’: ./include/ovn/actions.h:869:6: note: in a call to function ‘ovnacts_encode’ 869 | void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, | ^~ In function ‘execute_load’, inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13: utilities/ovn-trace.c:1509:13: error: ‘ovnacts_format’ reading 4 bytes from a region of size 1 [-Werror=stringop-overread] 1509 | ovnacts_format(>ovnact, OVNACT_LOAD_SIZE, ); | ^~~ utilities/ovn-trace.c:1509:13: note: referencing argument 1 of type ‘const struct ovnact[0]’ ./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’: ./include/ovn/actions.h:792:6: note: in a call to function ‘ovnacts_format’ 792 | void ovnacts_format(const struct ovnact[], size_t ovnacts_len, struct ds *); | ^~ Signed-off-by: Ales Musil --- Looks good to me, thanks! Acked-by: Dumitru Ceara ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-trace: Use the original ovnact for execute_load
On 12/15/22 10:13, Ales Musil wrote: > Compiling with GCC and -fsanitize=undefined throws > a warning [0]. To avoid that, use the original > ovnact pointer with the inner size rather than getting > the inner "struct ovnact" from the "struct ovnact_load". > > At the same time simplify the computation of size for > ovnacts_format call. > > [0] > In function ‘execute_load’, > inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13: > utilities/ovn-trace.c:1501:5: error: ‘ovnacts_encode’ reading 4 bytes from a > region of size 1 [-Werror=stringop-overread] > 1501 | ovnacts_encode(>ovnact, sizeof *load, , ); > | ^~ > utilities/ovn-trace.c:1501:5: note: referencing argument 1 of type ‘const > struct ovnact[0]’ > In file included from utilities/ovn-trace.c:35: > ./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’: > ./include/ovn/actions.h:869:6: note: in a call to function ‘ovnacts_encode’ > 869 | void ovnacts_encode(const struct ovnact[], size_t ovnacts_len, > | ^~ > In function ‘execute_load’, > inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13: > utilities/ovn-trace.c:1509:13: error: ‘ovnacts_format’ reading 4 bytes from a > region of size 1 [-Werror=stringop-overread] > 1509 | ovnacts_format(>ovnact, OVNACT_LOAD_SIZE, ); > | ^~~ > utilities/ovn-trace.c:1509:13: note: referencing argument 1 of type ‘const > struct ovnact[0]’ > ./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’: > ./include/ovn/actions.h:792:6: note: in a call to function ‘ovnacts_format’ > 792 | void ovnacts_format(const struct ovnact[], size_t ovnacts_len, struct > ds *); > | ^~ > > Signed-off-by: Ales Musil > --- Looks good to me, thanks! Acked-by: Dumitru Ceara ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-trace: Use the original ovnact for execute_load
Compiling with GCC and -fsanitize=undefined throws
a warning [0]. To avoid that, use the original
ovnact pointer with the inner size rather than getting
the inner "struct ovnact" from the "struct ovnact_load".
At the same time simplify the computation of size for
ovnacts_format call.
[0]
In function ‘execute_load’,
inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13:
utilities/ovn-trace.c:1501:5: error: ‘ovnacts_encode’ reading 4 bytes from a
region of size 1 [-Werror=stringop-overread]
1501 | ovnacts_encode(>ovnact, sizeof *load, , );
| ^~
utilities/ovn-trace.c:1501:5: note: referencing argument 1 of type ‘const
struct ovnact[0]’
In file included from utilities/ovn-trace.c:35:
./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’:
./include/ovn/actions.h:869:6: note: in a call to function ‘ovnacts_encode’
869 | void ovnacts_encode(const struct ovnact[], size_t ovnacts_len,
| ^~
In function ‘execute_load’,
inlined from ‘trace_actions.part.0.isra’ at utilities/ovn-trace.c:3075:13:
utilities/ovn-trace.c:1509:13: error: ‘ovnacts_format’ reading 4 bytes from a
region of size 1 [-Werror=stringop-overread]
1509 | ovnacts_format(>ovnact, OVNACT_LOAD_SIZE, );
| ^~~
utilities/ovn-trace.c:1509:13: note: referencing argument 1 of type ‘const
struct ovnact[0]’
./include/ovn/actions.h: In function ‘trace_actions.part.0.isra’:
./include/ovn/actions.h:792:6: note: in a call to function ‘ovnacts_format’
792 | void ovnacts_format(const struct ovnact[], size_t ovnacts_len, struct
ds *);
| ^~
Signed-off-by: Ales Musil
---
utilities/ovn-trace.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index 07ebac5e5..e5766ed67 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -1486,9 +1486,8 @@ ovntrace_node_prune_hard(struct ovs_list *nodes)
}
static void
-execute_load(const struct ovnact_load *load,
- const struct ovntrace_datapath *dp, struct flow *uflow,
- struct ovs_list *super OVS_UNUSED)
+execute_load(const struct ovnact *ovnact, const struct ovntrace_datapath *dp,
+ struct flow *uflow, struct ovs_list *super OVS_UNUSED)
{
const struct ovnact_encode_params ep = {
.lookup_port = ovntrace_lookup_port,
@@ -1498,7 +1497,7 @@ execute_load(const struct ovnact_load *load,
uint64_t stub[512 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
-ovnacts_encode(>ovnact, sizeof *load, , );
+ovnacts_encode(ovnact, OVNACT_ALIGN(ovnact->len), , );
struct ofpact *a;
OFPACT_FOR_EACH (a, ofpacts.data, ofpacts.size) {
@@ -1506,12 +1505,11 @@ execute_load(const struct ovnact_load *load,
if (!mf_is_register(sf->field->id)) {
struct ds s = DS_EMPTY_INITIALIZER;
-ovnacts_format(>ovnact, OVNACT_LOAD_SIZE, );
-ds_chomp(, ';');
-char *friendly = ovntrace_make_names_friendly(ds_cstr());
-ovntrace_node_append(super, OVNTRACE_NODE_MODIFY, "%s", friendly);
-free(friendly);
+ovnacts_format(ovnact, OVNACT_ALIGN(ovnact->len), );
+ds_chomp(, ';');
+ovntrace_node_append(super, OVNTRACE_NODE_MODIFY, "%s",
+ ds_cstr());
ds_destroy();
}
@@ -3057,7 +3055,7 @@ trace_actions(const struct ovnact *ovnacts, size_t
ovnacts_len,
const struct ovnact *a;
OVNACT_FOR_EACH (a, ovnacts, ovnacts_len) {
ds_clear();
-ovnacts_format(a, sizeof *a * (ovnact_next(a) - a), );
+ovnacts_format(a, OVNACT_ALIGN(a->len), );
char *friendly = ovntrace_make_names_friendly(ds_cstr());
ovntrace_node_append(super, OVNTRACE_NODE_ACTION, "%s", friendly);
free(friendly);
@@ -3072,7 +3070,7 @@ trace_actions(const struct ovnact *ovnacts, size_t
ovnacts_len,
break;
case OVNACT_LOAD:
-execute_load(ovnact_get_LOAD(a), dp, uflow, super);
+execute_load(a, dp, uflow, super);
break;
case OVNACT_MOVE:
--
2.38.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
