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
