On 5/8/25 9:23 PM, Alexandra Rukomoinikova wrote:
> Added checksum which is calculated for pipeline stages.
> Also, the tool calculate-pipeline-cksum is introduced. This tool
> calculates the cksum of a pipeline stages.
> 
> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
> ---
> v4 --> v5: addressed Ilya comments.
>          rebased
> ---
>  Makefile.am                        | 10 ++++++++++
>  build-aux/automake.mk              |  2 ++
>  build-aux/calculate-pipeline-cksum | 10 ++++++++++
>  build-aux/cksum-pipeline-check     | 19 +++++++++++++++++++
>  lib/ovn-util.c                     |  6 +++++-
>  5 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100755 build-aux/calculate-pipeline-cksum
>  create mode 100755 build-aux/cksum-pipeline-check

Thanks, Alexandra, for v5!  I left a few nits below that maybe maintainers
can address while applying.  The patch seems to work fine otherwise.

Acked-by: Ilya Maximets <[email protected]>

> 
> diff --git a/Makefile.am b/Makefile.am
> index 6b0f1913a..c4af17ce6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -379,6 +379,16 @@ check-ifconfig:
>       fi
>  .PHONY: check-ifconfig
>  
> +northd_h = northd/northd.h
> +action_h = include/ovn/actions.h
> +checksum_file = lib/ovn-util.c
> +
> +# Check if northd stages and actions cksum valid.

*is valid.

> +ALL_LOCAL += check-northd
> +.PHONY: check-northd

Other targets are usually marked as PHONY after the actual target definition.
So, it might make sense to move this two lines below.

> +check-northd: $(northd_h) $(action_h) $(checksum_file)
> +     @$(srcdir)/build-aux/cksum-pipeline-check $^
> +
>  if HAVE_GROFF
>  ALL_LOCAL += manpage-check
>  manpage-check: $(man_MANS) $(dist_man_MANS) $(noinst_man_MANS)
> diff --git a/build-aux/automake.mk b/build-aux/automake.mk
> index 255f573d7..046d7d84f 100644
> --- a/build-aux/automake.mk
> +++ b/build-aux/automake.mk
> @@ -1,6 +1,8 @@
>  EXTRA_DIST += \
> +     build-aux/calculate-pipeline-cksum \
>       build-aux/calculate-schema-cksum \
>       build-aux/cccl \
> +     build-aux/cksum-pipeline-check \
>       build-aux/cksum-schema-check \
>       build-aux/dist-docs \
>       build-aux/dpdkstrip.py \
> diff --git a/build-aux/calculate-pipeline-cksum 
> b/build-aux/calculate-pipeline-cksum
> new file mode 100755
> index 000000000..287a6a23f
> --- /dev/null
> +++ b/build-aux/calculate-pipeline-cksum
> @@ -0,0 +1,10 @@
> +#!/bin/sh
> +
> +northd_h=$1
> +action_h=$2
> +
> +stages=$(grep '^\s*PIPELINE_STAGE(' $northd_h)
> +
> +actions=$(grep 'OVNACT(' $action_h | grep -v 'OVNACT(ENUM,')
> +
> +echo "$stages" "$actions" | cksum
> diff --git a/build-aux/cksum-pipeline-check b/build-aux/cksum-pipeline-check
> new file mode 100755
> index 000000000..f6afb2031
> --- /dev/null
> +++ b/build-aux/cksum-pipeline-check
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +
> +northd_h=$1
> +action_h=$2
> +ovn_util_c=$3
> +
> +cksumcheckpath=$(dirname "$0")
> +sum=$("$cksumcheckpath/calculate-pipeline-cksum" "$northd_h" "$action_h")
> +checksum=$(sed -n \
> +    's/^#define OVN_NORTHD_PIPELINE_CSUM  *"\([^"]*\)".*/\1/p' "$ovn_util_c")
> +
> +if [ "$sum" != "$checksum" ]; then
> +    echo >&2 "The checksum \"$sum\" calculated from the logical actions" \
> +             "and pipeline stages does not match the" \
> +             "OVN_NORTHD_PIPELINE_CSUM. You should update the checksum" \
> +             "with the value listed here and consider updating the" \
> +             "OVN internal version number in \"$ovn_util_c\" as well."
> +    exit 1
> +fi
> \ No newline at end of file

This file is still missing a line break at the end.

> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index af17082a9..45a230b03 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -897,7 +897,11 @@ ip_address_and_port_from_lb_key(const char *key, char 
> **ip_address,
>   * modified or a stage is added to a logical pipeline.
>   *
>   * This value is also used to handle some backward compatibility during
> - * upgrading. It should never decrease or rewind. */
> + * upgrading. It should never decrease or rewind.
> + *
> + * NOTE: If OVN_NORTHD_PIPELINE_CSUM is updated make sure to double check
> + * whether an update of OVN_INTERNAL_MINOR_VER is required. */
> +#define OVN_NORTHD_PIPELINE_CSUM "1166037917 10535"
>  #define OVN_INTERNAL_MINOR_VER 8
>  
>  /* Returns the OVN version. The caller must free the returned value. */

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to