On 7/28/25 8:57 AM, Ales Musil wrote:
> There have been multiple definitions of the put_load() function
> through out the code base. There have also been variation of
> put_load that allows to set BE array instead of LE constant.
> Move both variations to ovn-util and djust the code as needed.
>
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---
Hi Ales,
Thanks for the cleanup!
> controller/lflow.c | 51 +++++++------------------------------------
> controller/physical.c | 12 ----------
> controller/pinctrl.c | 16 --------------
> lib/actions.c | 12 ----------
> lib/ovn-util.c | 19 ++++++++++++++++
> lib/ovn-util.h | 6 +++++
> 6 files changed, 33 insertions(+), 83 deletions(-)
>
[...]
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index d7e236059..c8d10f493 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -1484,3 +1484,22 @@ ovn_mirror_port_name(const char *datapath_name,
> {
> return xasprintf("mp-%s-%s", datapath_name, port_name);
> }
> +
> +void
> +put_load_be(const void *value, size_t len, enum mf_field_id dst,
> + size_t ofs, size_t n_bits, struct ofpbuf *ofpacts)
> +{
> + struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
> + mf_from_id(dst), NULL,
> + NULL);
> + bitwise_copy(value, len, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
> + bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
> +}
> +
I find the name "put_load_be()" confusing. That's because the function
copies the value in host endianess which may or may not be BE.
Would it make sense to call this function put_load_bytes()?
> +void
> +put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> + struct ofpbuf *ofpacts)
> +{
> + ovs_be64 n_value = htonll(value);
> + put_load_be(&n_value, 8, dst, ofs, n_bits, ofpacts);
> +}
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index c05bfc53b..eddb07f4d 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -16,6 +16,7 @@
> #ifndef OVN_UTIL_H
> #define OVN_UTIL_H 1
>
> +#include "openvswitch/meta-flow.h"
> #include "ovsdb-idl.h"
> #include "lib/packets.h"
> #include "lib/sset.h"
> @@ -504,6 +505,11 @@ const struct sbrec_port_binding *lport_lookup_by_name(
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> const char *name);
>
> +void put_load_be(const void *value, size_t len, enum mf_field_id dst,
> + size_t ofs, size_t n_bits, struct ofpbuf *ofpacts);
> +void put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> + struct ofpbuf *ofpacts);
> +
Nit: size_t for 'ofs' and 'n_bits'
> /* __NARG__ provides a way to count the number of arguments passed to a
> * variadic macro. As defined below, it's capable of counting up to
> * 16 arguments. This should be more than enough for our purposes. However
With these minor comments addressed:
Acked-by: Dumitru Ceara <dce...@redhat.com>
Thanks,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
