Hello,

this is the reworked patch - rebased on the current development-HEAD
and formatted according to CONTRIBUTIONS.


>From 6721c1cf02a67845441354417006e928c84fb370 Mon Sep 17 00:00:00 2001
From: Christian Lachner <glad...@gmail.com>
Date: Mon, 10 Feb 2020 10:29:13 +0100
Subject: [PATCH] MINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs

As haproxy wont build on AIX 7.2 using the old "aix52" TARGET a new
TARGET was introduced which adds two special CFLAGS to prevent the
loading of AIX's xmem.h and var.h. This is done by defining the
corresponding include-guards _H_XMEM and _H_VAR. Without excluding
those header-files the build fails because of redefinition errors:

1)
  CC      src/mux_fcgi.o
In file included from /usr/include/sys/uio.h:90,
                 from
/opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/include-fixed/sys/socket.h:104,
                 from include/common/compat.h:32,
                 from include/common/cfgparse.h:25,
                 from src/mux_fcgi.c:13:
src/mux_fcgi.c:204:13: error: expected ':', ',', ';', '}' or
'__attribute__' before '.' token
  struct ist rem_addr;
             ^~~~~~~~

2)
  CC      src/cfgparse-listen.o
In file included from include/types/arg.h:31,
                 from include/types/acl.h:29,
                 from include/types/proxy.h:41,
                 from include/proto/log.h:34,
                 from include/common/cfgparse.h:30,
                 from src/mux_h2.c:13:
include/types/vars.h:30:8: error: redefinition of 'struct var'
 struct var {
        ^~~

Futhermore, to enable multithreading via USE_THREAD, the atomic
library was added to the LDFLAGS. Finally, two new CPUs were added
to simplify the usage of power8 and power9 optimizations.

This TARGET was only tested on GCC 8.3 and may or may not work on
IBM's native C-compiler (XLC).

Should be backported to 2.1.
---
 INSTALL  |  1 +
 Makefile | 20 +++++++++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/INSTALL b/INSTALL
index 40e3a04..83c6b05 100644
--- a/INSTALL
+++ b/INSTALL
@@ -379,6 +379,7 @@ and assign it to the TARGET variable :
   - openbsd             for OpenBSD 5.7 and above
   - aix51               for AIX 5.1
   - aix52               for AIX 5.2
+  - aix72-gcc           for AIX 7.2 (using gcc)
   - cygwin              for Cygwin
   - haiku               for Haiku
   - generic             for any other OS or version.
diff --git a/Makefile b/Makefile
index bb494fa..042a616 100644
--- a/Makefile
+++ b/Makefile
@@ -144,13 +144,13 @@ DOCDIR = $(PREFIX)/doc/haproxy
 # Use TARGET=<target_name> to optimize for a specifc target OS among the
 # following list (use the default "generic" if uncertain) :
 #    linux-glibc, linux-glibc-legacy, solaris, freebsd, openbsd, netbsd,
-#    cygwin, haiku, aix51, aix52, osx, generic, custom
+#    cygwin, haiku, aix51, aix52, aix72-gcc, osx, generic, custom
 TARGET =

 #### TARGET CPU
 # Use CPU=<cpu_name> to optimize for a particular CPU, among the following
 # list :
-#    generic, native, i586, i686, ultrasparc, custom
+#    generic, native, i586, i686, ultrasparc, power8, power9, custom
 CPU = generic

 #### Architecture, used when not building for native architecture
@@ -257,6 +257,8 @@ CPU_CFLAGS.native     = -O2 -march=native
 CPU_CFLAGS.i586       = -O2 -march=i586
 CPU_CFLAGS.i686       = -O2 -march=i686
 CPU_CFLAGS.ultrasparc = -O6 -mcpu=v9 -mtune=ultrasparc
+CPU_CFLAGS.power8     = -O2 -mcpu=power8 -mtune=power8
+CPU_CFLAGS.power9     = -O2 -mcpu=power9 -mtune=power9
 CPU_CFLAGS            = $(CPU_CFLAGS.$(CPU))

 #### ARCH dependant flags, may be overridden by CPU flags
@@ -381,7 +383,7 @@ ifeq ($(TARGET),aix51)
   DEBUG_CFLAGS    =
 endif

-# AIX 5.2 and above
+# AIX 5.2
 ifeq ($(TARGET),aix52)
   set_target_defaults = $(call default_opts, \
     USE_POLL USE_LIBCRYPT USE_OBSOLETE_LINKER)
@@ -389,6 +391,14 @@ ifeq ($(TARGET),aix52)
   DEBUG_CFLAGS    =
 endif

+# AIX 7.2 and above
+ifeq ($(TARGET),aix72-gcc)
+  set_target_defaults = $(call default_opts, \
+    USE_POLL USE_THREAD USE_LIBCRYPT USE_OBSOLETE_LINKER USE_GETADDRINFO)
+  TARGET_CFLAGS   = -D_H_XMEM -D_H_VAR
+  TARGET_LDFLAGS  = -latomic
+endif
+
 # Cygwin
 ifeq ($(TARGET),cygwin)
   set_target_defaults = $(call default_opts, \
@@ -754,7 +764,7 @@ all:
     @echo "Please choose the target among the following supported list :"
     @echo
     @echo "   linux-glibc, linux-glibc-legacy, solaris, freebsd,
openbsd, netbsd,"
-    @echo "   cygwin, haiku, aix51, aix52, osx, generic, custom"
+    @echo "   cygwin, haiku, aix51, aix52, aix72-gcc, osx, generic, custom"
     @echo
     @echo "Use \"generic\" if you don't want any optimization,
\"custom\" if you"
     @echo "want to precisely tweak every option, or choose the target which"
@@ -832,7 +842,7 @@ help:
        else \
          echo "TARGET not set, you may pass 'TARGET=xxx' to set one among :";\
          echo "  linux-glibc, linux-glibc-legacy, solaris, freebsd,
netbsd, osx,"; \
-         echo "  openbsd, aix51, aix52, cygwin, haiku, generic, custom"; \
+         echo "  openbsd, aix51, aix52, aix72-gcc, cygwin, haiku,
generic, custom"; \
        fi
     $(Q)echo;echo "Enabled features for TARGET '$(TARGET)' (disable
with 'USE_xxx=') :"
     $(Q)set -- $(foreach opt,$(patsubst USE_%,%,$(use_opts)),$(if
$(USE_$(opt)),$(opt),)); echo "  $$*" | (fmt || cat) 2>/dev/null
-- 
2.25.0


Best regards,
Christian

On Mon, Feb 10, 2020 at 12:41 PM Chris <glad...@gmail.com> wrote:
>
> Hi Willy,
>
> thanks a lot for your input and sorry for the delay. Work is pretty
> rough right now...
>
> > Thank you for doing this work. I'm having a few questions below.
> Sure, I am glad I can help :).
>
> > > +# AIX 7.2 and above
> > > +ifeq ($(TARGET),aix72-gcc)
> > > +  set_target_defaults = $(call default_opts, \
> > > +    USE_POLL USE_THREAD USE_LIBCRYPT USE_OBSOLETE_LINKER
> > > USE_GETADDRINFO USE_TFO)
> >
> > Are you really really sure about USE_TFO ? I suspect you might have
> > accidently borrowed it from another line. It stands for TCP FastOpen
> > and is really not broadly deployed, I was only aware of Linux and
> > FreeBSD, but there are likely a few other ones. And the fact that it
> > looks OK is possibly just that it's causing a different TCP socket
> > option to be set on the connection, so if you're not certain we'd
> > rather avoid it.
> I think you are right. I kept USE_TFO in there because it neither
> introduced any build-issues nor did it cause any runtime issues.
> However, it probably does nothing and should be removed as long as we
> are not certain it is actually beneficial. I will have a look at the
> corresponding AIX internals later at some point.
>
> > > src/hlua.c: In function 'hlua_panic_ljmp':
> > > src/hlua.c:128:1: warning: no return statement in function returning
> > > non-void [-Wreturn-type]
> > >  static int hlua_panic_ljmp(lua_State *L) { longjmp(safe_ljmp_env, 1); }
> > >  ^~~~~~
> >
> > Don't worry about this one, I'll handle it. I suspect that on linux
> > platforms the longjmp() function prototype is decorated with
> > __attribute__((noreturn)) which makes the compiler happy, but that's
> > likely not the case on any system not relying on a gcc-compatible
> > compiler by default.
> Thanks for handling this one. If there is something I shoult test
> please let me know.
>
> > Just two small extra requests :
> >   - please rebase it on top of the development branch. If you really
> >     need it in 2.1, just indicate it. Given that it's well isolated,
> >     I'm fine with having it backported.
> >
> >   - please have a look at CONTRIBUTING to get guidance to write a
> >     subject and a commit message. It doesn't need to be very long
> >     but at least indicating what options you chosed to enable/disable
> >     and on what system you tested it will be enough if we need to
> >     reconsider parts of it later.
> I already made a new patch based on the current development-HEAD and
> tried to follow all the guidelines from CONTRIBUTING. I would love to
> have it backported to 2.1 as the patch is pretty much identical and
> should not cause any regressions. The patch will follow shortly!
>
> > Do you have a permanent and durable access to this machine, with the
> > ability to occasionally re-run a build test in case we ask you (likely
> > no more than 2-3 times a year in the worst case) ? I'm asking because
> > I'm still keeping a very old IBM server running 5.2 on a Power3 just
> > for the sake of revalidating new releases once in a while. Given that
> > I could not upgrade it to latest OpenSSL, it cannot even be used to
> > provide complete binaries to those needing them, so it's getting very
> > obsolete and knowing that there's a better solution somewhere would
> > allow me to get rid of it.
> Well, I am not the primary administrator of this machine. However, I
> have permanent access and no problem doing the occasional build on it.
> I will have a chat with the server admin in regards to the longtime
> prospects of this server and report back to you.
>
> > thanks!
> > Willy
> You are welcome - I am happy I can contribute!
>
> thanks,
> Christian
>
> On Thu, Feb 6, 2020 at 3:36 PM Willy Tarreau <w...@1wt.eu> wrote:
> >
> > Hello Christian,
> >
> > On Mon, Feb 03, 2020 at 12:09:46PM +0100, Chris wrote:
> > > Hello everybody,
> > >
> > > I spent some time making haproxy compile and run successfully on AIX
> > > 7.2 using GCC 8.3 and wanted to contribute my patch in the hope that
> > > it could be merged. The patch is based on the current haproxy 2.1 head
> > > revision. I can make one for the development branch too - but it
> > > should be basically identical.
> >
> > Thank you for doing this work. I'm having a few questions below.
> >
> > > +# AIX 7.2 and above
> > > +ifeq ($(TARGET),aix72-gcc)
> > > +  set_target_defaults = $(call default_opts, \
> > > +    USE_POLL USE_THREAD USE_LIBCRYPT USE_OBSOLETE_LINKER
> > > USE_GETADDRINFO USE_TFO)
> >
> > Are you really really sure about USE_TFO ? I suspect you might have
> > accidently borrowed it from another line. It stands for TCP FastOpen
> > and is really not broadly deployed, I was only aware of Linux and
> > FreeBSD, but there are likely a few other ones. And the fact that it
> > looks OK is possibly just that it's causing a different TCP socket
> > option to be set on the connection, so if you're not certain we'd
> > rather avoid it.
> >
> > > The patch implements a new TARGET called aix72-gcc and also adds 2
> > > CPUs (power8 and power9). Here is my proof-of-work:
> > (...)
> > > src/hlua.c: In function 'hlua_panic_ljmp':
> > > src/hlua.c:128:1: warning: no return statement in function returning
> > > non-void [-Wreturn-type]
> > >  static int hlua_panic_ljmp(lua_State *L) { longjmp(safe_ljmp_env, 1); }
> > >  ^~~~~~
> >
> > Don't worry about this one, I'll handle it. I suspect that on linux
> > platforms the longjmp() function prototype is decorated with
> > __attribute__((noreturn)) which makes the compiler happy, but that's
> > likely not the case on any system not relying on a gcc-compatible
> > compiler by default.
> >
> > > -bash-4.4$ ./haproxy -vv
> > > HA-Proxy version 2.1.2 2019/12/21 - https://haproxy.org/
> > > Status: stable branch - will stop receiving fixes around Q1 2021.
> > (...)
> >
> > Looks good!
> >
> > > If you have any questions feel free to ask and please keep me on CC
> > > for this topic!
> >
> > Just two small extra requests :
> >   - please rebase it on top of the development branch. If you really
> >     need it in 2.1, just indicate it. Given that it's well isolated,
> >     I'm fine with having it backported.
> >
> >   - please have a look at CONTRIBUTING to get guidance to write a
> >     subject and a commit message. It doesn't need to be very long
> >     but at least indicating what options you chosed to enable/disable
> >     and on what system you tested it will be enough if we need to
> >     reconsider parts of it later.
> >
> > thanks!
> > Willy

Reply via email to