On 6/20/23 10:08, Frode Nordahl wrote:
> Allow building the upstream Open vSwitch deb package with AF_XDP
> datapath enabled by passing in the string 'afxdp' in the
> DEB_BUILD_OPTIONS environment variable.

Hi, Frode.  Thanks for the patch!

Can we also update the debian-deb make target to pass this argument
whenever we HAVE_AF_XDP ?  And maybe add a CI job for it?
Documentation/intro/install/debian.rst may also need some updates.

Some more comments below.

> 
> Signed-off-by: Frode Nordahl <[email protected]>
> ---
>  debian/automake.mk | 30 ++++++++++++++++++++++--------
>  debian/control.in  |  2 ++
>  debian/rules       |  8 +++++++-
>  3 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/debian/automake.mk b/debian/automake.mk
> index 7b2afafae..3195964c9 100644
> --- a/debian/automake.mk
> +++ b/debian/automake.mk
> @@ -95,17 +95,29 @@ CLEANFILES += debian/copyright
>  
>  
>  if DPDK_NETDEV
> -update_deb_control = \
> -     $(AM_V_GEN) sed -e 's/^\# DPDK_NETDEV //' \
> -             < $(srcdir)/debian/control.in > debian/control
> +update_deb_control_dpdk = \
> +     $(AM_V_GEN) sed -i 's/^\# DPDK_NETDEV //' \
> +             $(srcdir)/debian/control
>  else
> -update_deb_control = \
> -     $(AM_V_GEN) grep -v '^\# DPDK_NETDEV' \
> -             < $(srcdir)/debian/control.in > debian/control
> +update_deb_control_dpdk = \
> +     $(AM_V_GEN) sed -i '/^\# DPDK_NETDEV /d' \
> +             $(srcdir)/debian/control
> +endif
> +
> +if HAVE_AF_XDP
> +update_deb_control_afxdp = \
> +     $(AM_V_GEN) sed -i 's/^\# HAVE_AF_XDP //' \
> +             $(srcdir)/debian/control
> +else
> +update_deb_control_afxdp = \
> +     $(AM_V_GEN) sed -i '/^\# HAVE_AF_XDP /d' \
> +             $(srcdir)/debian/control
>  endif
>  
>  debian/control: $(srcdir)/debian/control.in Makefile
> -     $(update_deb_control)
> +     cp $(srcdir)/debian/control.in $(srcdir)/debian/control
> +     $(update_deb_control_afxdp)
> +     $(update_deb_control_dpdk)
>  
>  CLEANFILES += debian/control
>  
> @@ -120,8 +132,10 @@ debian-deb: debian
>               exit 1;                                                         
> \
>       fi
>       $(MAKE) distclean
> +     cp $(srcdir)/debian/control.in $(srcdir)/debian/control
>       $(update_deb_copyright)
> -     $(update_deb_control)
> +     $(update_deb_control_afxdp)
> +     $(update_deb_control_dpdk)
>       $(AM_V_GEN) fakeroot debian/rules clean
>  if DPDK_NETDEV
>       $(AM_V_GEN) DEB_BUILD_OPTIONS="nocheck parallel=`nproc`" \
> diff --git a/debian/control.in b/debian/control.in
> index 19f590d06..d2711deaa 100644
> --- a/debian/control.in
> +++ b/debian/control.in
> @@ -19,6 +19,7 @@ Build-Depends:
>   dh-sequence-sphinxdoc,
>   graphviz,
>   iproute2,
> +# HAVE_AF_XDP  libbpf-dev,
>   libcap-ng-dev,
>   libdbus-1-dev [amd64 i386 ppc64el arm64],
>  # DPDK_NETDEV  libdpdk-dev (>= 22.11) [amd64 i386 ppc64el arm64],
> @@ -27,6 +28,7 @@ Build-Depends:
>   libssl-dev,
>   libtool,
>   libunbound-dev,
> +# HAVE_AF_XDP  libxdp-dev (>= 1.2.9~) [!alpha !arc !hppa !ia64 !m68k !sh4],
>   openssl,
>   pkg-config,
>   procps,

These are build dependencies, but the package will have also a runtime
dependency on libxdp (and libbpf ?).  Should these be added to 'Depends'
section or will they be added automagically somehow as rpm does?

Also, we do not update the dpdk-enabled package here.  It should be
possible to build a package with both dpdk and afxdp at the same time
if we change the 'sed' calls above to not match on the start of a line.
This way we can have '# DPDK_NETDEV # HAVE_AF_XDP  libbpf-dev,' that
will be uncommented only if both are enabled.  What do you think?

> diff --git a/debian/rules b/debian/rules
> index 28c249d07..8ed19db70 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -19,13 +19,19 @@ endif
>  PYTHON3S:=$(shell py3versions -vr)
>  DEB_HOST_ARCH?=$(shell dpkg-architecture -qDEB_HOST_ARCH)
>  
> +ifneq (,$(filter afxdp, $(DEB_BUILD_OPTIONS)))
> +AFXDP:=--enable-afxdp
> +else
> +AFXDP:=--disable-afxdp
> +endif
> +
>  override_dh_auto_configure:
>       test -d _debian || mkdir _debian
>       cd _debian && ( \
>               test -e Makefile || \
>               ../configure --prefix=/usr --localstatedir=/var \
>                                       --enable-ssl \
> -                                     --disable-afxdp \
> +                                     $(AFXDP) \
>                                       --sysconfdir=/etc \
>                                       $(DATAPATH_CONFIGURE_OPTS) \
>                                       $(EXTRA_CONFIGURE_OPTS) \

Not sure if that can improve anything, but the --enable-afxdp=no|yes|auto
is an acceptable syntax too.  In case that can save some conditionals.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to