Add headers in lapi
cover Add headers in lapi
Hi,
I posted this originally in "netstress: Add imports for TCP_FASTOPEN definition" [1], got reply from Alexey [2].
Do we want to include system headers which originaly define definitions in lapi headers? (e.g. include <linux/falloc.h> in include/lapi/fallocate.h as it defines FALLOC_FL_KEEP_SIZE from linux/falloc.h)?
Some of them do it, some not.
If yes, do we want to check presence of them with autotools?
What is the purpose of lapi anyway? I suppose just to backport missing features added in recent glibc to older versions or missing in other libc (musl, uclibc, binder).
Kind regards,
Petr
[1] http://lists.linux.it/pipermail/ltp/2017-October/006275.html
[2] http://lists.linux.it/pipermail/ltp/2017-November/006345.html
Petr Vorel (4):
lapi: Add lapi/tcp.h with definitions moved from netstress.c lapi/socket.h: Move definitions from netstress.c
lapi: Add lapi/in.h with definition moved from netstress.c
lapi: Add lapi/dccp.h with definition moved from netstress.c
include/lapi/dccp.h | 25 +++++++++++++++++++++++++ include/lapi/in.h | 25 +++++++++++++++++++++++++ include/lapi/socket.h | 18 +++++++++++++++++- include/lapi/tcp.h | 25 +++++++++++++++++++++++++ testcases/network/netstress/netstress.c | 33 ++++----------------------------- 5 files changed, 96 insertions(+), 30 deletions(-) create mode 100644 include/lapi/dccp.h create mode 100644 include/lapi/in.h create mode 100644 include/lapi/tcp.h
Signature · 1 line
2.15.0
RFC PATCH 1/4 lapi: Add lapi/tcp.h with definitions moved from netstress.c
--- include/lapi/tcp.h | 25 +++++++++++++++++++++++++ testcases/network/netstress/netstress.c | 6 +----- 2 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 include/lapi/tcp.h
include/lapi/tcp.h new file +25 -0
@@ -0,0 +1,25 @@ +/* +* Copyright (c) 2017 Petr Vorel <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef TCP_H__ +#define TCP_H__ + +#ifndef TCP_FASTOPEN +#define TCP_FASTOPEN 23 +#endif + +#endif /* TCP_H__ */
testcases/network/netstress/netstress.c +1 -5
@@ -33,6 +33,7 @@#include <errno.h> #include "lapi/posix_clocks.h" +#include "lapi/tcp.h" #include "tst_safe_pthread.h" #include "tst_test.h"
@@ -51,11 +52,6 @@ static const int max_msg_len = (1 << 16) - 1;#define DCCP_SOCKOPT_SERVICE 2#endif -/* TCP server requiers */ -#ifndef TCP_FASTOPEN -#define TCP_FASTOPEN 23 -#endif -#ifndef SO_BUSY_POLL #define SO_BUSY_POLL 46#endif
Signature · 1 line
2.15.0
RFC PATCH 2/4 lapi/socket.h: Move definitions from netstress.c
In reply to Petr Vorel+ remove sys/socket.h from netstress.c
--- include/lapi/socket.h | 18 +++++++++++++++++- testcases/network/netstress/netstress.c | 18 ++---------------- 2 files changed, 19 insertions(+), 17 deletions(-)
include/lapi/socket.h +17 -1
@@ -19,8 +19,24 @@#ifndef __LAPI_SOCKET_H__#define __LAPI_SOCKET_H__ +#ifndef MSG_FASTOPEN +#define MSG_FASTOPEN 0x20000000/* Send data in TCP SYN */ +#endif + +#ifndef SO_BUSY_POLL +#define SO_BUSY_POLL 46 +#endif +#ifndef SOCK_CLOEXEC -# define SOCK_CLOEXEC 02000000 +#define SOCK_CLOEXEC 02000000 +#endif + +#ifndef SOCK_DCCP +#define SOCK_DCCP 6 +#endif + +#ifndef SOL_DCCP +#define SOL_DCCP 269#endif #endif /* __LAPI_SOCKET_H__ */
testcases/network/netstress/netstress.c +2 -16
@@ -22,7 +22,6 @@#include <stdlib.h> #include <limits.h> #include <sys/types.h> -#include <sys/socket.h> #include <netdb.h> #include <netinet/in.h> #include <arpa/inet.h>
@@ -33,34 +32,21 @@#include <errno.h> #include "lapi/posix_clocks.h" +#include "lapi/socket.h" #include "lapi/tcp.h" #include "tst_safe_pthread.h" #include "tst_test.h" static const int max_msg_len = (1 <<16 ) -1 ; -#ifndef SOCK_DCCP -#define SOCK_DCCP 6 -#endif #ifndef IPPROTO_DCCP #define IPPROTO_DCCP 33#endif -#ifndef SOL_DCCP -#define SOL_DCCP 269 -#endif +#ifndef DCCP_SOCKOPT_SERVICE #define DCCP_SOCKOPT_SERVICE 2#endif -#ifndef SO_BUSY_POLL -#define SO_BUSY_POLL 46 -#endif - -/* TCP client requiers */ -#ifndef MSG_FASTOPEN -#define MSG_FASTOPEN 0x20000000/* Send data in TCP SYN */ -#endif -enum {SERVER_HOST =0 ,CLIENT_HOST ,
Signature · 1 line
2.15.0
RFC PATCH 2/4 lapi/socket.h: Move definitions from netstress.c
Hi!
include/lapi/socket.h line 32
Quoted 4 lines
#ifndef
SOCK_CLOEXECinclude/lapi/socket.h +2
-1-# define SOCK_CLOEXEC 02000000 +#define SOCK_CLOEXEC 02000000 +#endif
FYI the prefered way how to indent innner macros are spaces after the hash #.
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 2/4 lapi/socket.h: Move definitions from netstress.c
In reply to Petr VorelQuoted 1 line · Petr Vorel
+ remove
sys/socket.hfromnetstress.c
What about including it in lapi/socket.h?
Quoted 20 lines
Signed-off-by Petr Vorel <[email protected]>--- include/lapi/socket.h | 18 +++++++++++++++++- testcases/network/netstress/netstress.c | 18 ++---------------- 2 files changed, 19 insertions(+), 17 deletions(-)include/lapi/socket.h +6
-0@@ -19,8 +19,24 @@#ifndef __LAPI_SOCKET_H__#define __LAPI_SOCKET_H__ +#ifndef MSG_FASTOPEN +#define MSG_FASTOPEN 0x20000000/* Send data in TCP SYN */ +#endif + +#ifndef SO_BUSY_POLL +#define SO_BUSY_POLL 46
include/lapi/socket.h line 36
Quoted 10 lines
include/lapi/socket.h +8
-1+#endif +#ifndef SOCK_CLOEXEC -# define SOCK_CLOEXEC 02000000 +#define SOCK_CLOEXEC 02000000 +#endif + +#ifndef SOCK_DCCP +#define SOCK_DCCP 6 +#endif
The two above should probably be in ascending order...
SOCK_CLOEXEC along with SOCK_NONBLOCK already defined in lapi/fcntl.h. I guess we should move them to lapi/socket.h instead.
Thanks,
Alexey
RFC PATCH 2/4 lapi/socket.h: Move definitions from netstress.c
Quoted 29 lines · Petr Vorel
Quoted 1 line
+ remove
sys/socket.hfromnetstress.cWhat about including it in
lapi/socket.h?Quoted 24 lines
Signed-off-by Petr Vorel <[email protected]>--- include/lapi/socket.h | 18 +++++++++++++++++- testcases/network/netstress/netstress.c | 18 ++---------------- 2 files changed, 19 insertions(+), 17 deletions(-)include/lapi/socket.h +8
-1@@ -19,8 +19,24 @@#ifndef __LAPI_SOCKET_H__#define __LAPI_SOCKET_H__ +#ifndef MSG_FASTOPEN +#define MSG_FASTOPEN 0x20000000/* Send data in TCP SYN */ +#endif + +#ifndef SO_BUSY_POLL +#define SO_BUSY_POLL 46 +#endif +#ifndef SOCK_CLOEXEC -# define SOCK_CLOEXEC 02000000
Quoted 8 lines
include/lapi/socket.h line 36
Quoted 6 lines
include/lapi/socket.h +6
-0+#define SOCK_CLOEXEC 02000000 +#endif + +#ifndef SOCK_DCCP +#define SOCK_DCCP 6 +#endif The two above should probably be in ascending order...
Thanks, I'll fix it.
Quoted 2 lines
SOCK_CLOEXECalong withSOCK_NONBLOCKalready defined inlapi/fcntl.h. I guess we should move them tolapi/socket.hinstead.
Good point. include/lapi/fcntl.h defines:
#ifndef O_CLOEXEC
# define O_CLOEXEC 02000000 #endif
#ifndef SOCK_CLOEXEC
# define SOCK_CLOEXEC O_CLOEXEC #endif
This actually makes sense as these both have the same value on the same architecture, but in glibc headers are these defined separately (in /usr/include/bits/socket_type.h and /usr/include/asm-generic/fcntl.h).
Both musl and uclibc-ng use the same approach, so I propose to split these definitions in according to (g)libc headers.
Kind regards,
Petr
RFC PATCH 3/4 lapi: Add lapi/in.h with definition moved from netstress.c
In reply to Petr Vorel+ remove now unused include netinet/in.h
--- include/lapi/in.h | 25 +++++++++++++++++++++++++ testcases/network/netstress/netstress.c | 6 +----- 2 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 include/lapi/in.h
include/lapi/in.h new file +25 -0
@@ -0,0 +1,25 @@ +/* +* Copyright (c) 2017 Petr Vorel <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef IN_H__ +#define IN_H__ + +#ifndef IPPROTO_DCCP +#define IPPROTO_DCCP 33 +#endif + +#endif /* IN_H__ */
testcases/network/netstress/netstress.c +1 -5
@@ -23,7 +23,6 @@#include <limits.h> #include <sys/types.h> #include <netdb.h> -#include <netinet/in.h> #include <arpa/inet.h> #include <poll.h> #include <time.h>
@@ -31,6 +30,7 @@#include <unistd.h> #include <errno.h> +#include "lapi/in.h" #include "lapi/posix_clocks.h" #include "lapi/socket.h" #include "lapi/tcp.h"
@@ -39,10 +39,6 @@static const int max_msg_len = (1 <<16 ) -1 ; -#ifndef IPPROTO_DCCP -#define IPPROTO_DCCP 33 -#endif -#ifndef DCCP_SOCKOPT_SERVICE #define DCCP_SOCKOPT_SERVICE 2#endif
Signature · 1 line
2.15.0
RFC PATCH 3/4 lapi: Add lapi/in.h with definition moved from netstress.c
Quoted 18 lines
include/lapi/in.h new file +15
-0@@ -0,0 +1,25 @@ +/* +* Copyright (c) 2017 Petr Vorel <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>.
include/lapi/in.h line 25
Quoted 10 lines
include/lapi/in.h new file +10
-0+*/ + +#ifndef IN_H__ +#define IN_H__ + +#ifndef IPPROTO_DCCP +#define IPPROTO_DCCP 33 +#endif + +#endif /* IN_H__ */
Maybe we should rename this file to netinet_in.h or create netinet directory in lapi. Including just lapi/in.h is a bit confusing.
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 3/4 lapi: Add lapi/in.h with definition moved from netstress.c
Quoted 2 lines
Maybe we should rename this file to
netinet_in.hor create netinet directory in lapi. Including justlapi/in.his a bit confusing.
Is it because in.h is also in bits/ directory (not only in linux and some glibc directory)? /usr/include/bits/in.h/usr/include/netinet/in.h/usr/include/linux/in.h
As the same we have with lapi/tcp.h (in both kernel and glibc)... /usr/include/netinet/tcp.h/usr/include/linux/tcp.h
And I'm for directory.
Kind regards,
Petr
RFC PATCH 3/4 lapi: Add lapi/in.h with definition moved from netstress.c
Hi,
Quoted 7 lines
Quoted 2 lines
Maybe we should rename this file to
netinet_in.hor create netinet directory in lapi. Including justlapi/in.his a bit confusing.Is it because
in.his also in bits/ directory (not only in linux and some glibc directory)?/usr/include/bits/in.h/usr/include/netinet/in.h/usr/include/linux/in.h
Quoted 5 lines
As the same we have with
lapi/tcp.h(in both kernel and glibc).../usr/include/netinet/tcp.h/usr/include/linux/tcp.hAnd I'm for directory.
Actually I'd be for just plain "in.h" as other fileas in lapi/ directory don't prefix anything, aren't in any special directory.
But if you really don't like that, I'll put it into netinet_in.h.
Kind regards,
Petr
RFC PATCH 4/4 lapi: Add lapi/dccp.h with definition moved from netstress.c
In reply to Petr Vorel--- include/lapi/dccp.h | 25 +++++++++++++++++++++++++ testcases/network/netstress/netstress.c | 5 +---- 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 include/lapi/dccp.h
include/lapi/dccp.h new file +25 -0
@@ -0,0 +1,25 @@ +/* +* Copyright (c) 2017 Petr Vorel <[email protected]> +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef DCCP_H__ +#define DCCP_H__ + +#ifndef DCCP_SOCKOPT_SERVICE +#define DCCP_SOCKOPT_SERVICE 2 +#endif + +#endif /* DCCP_H__ */
testcases/network/netstress/netstress.c +1 -4
@@ -30,6 +30,7 @@#include <unistd.h> #include <errno.h> +#include "lapi/dccp.h" #include "lapi/in.h" #include "lapi/posix_clocks.h" #include "lapi/socket.h"
@@ -39,10 +40,6 @@static const int max_msg_len = (1 <<16 ) -1 ; -#ifndef DCCP_SOCKOPT_SERVICE -#define DCCP_SOCKOPT_SERVICE 2 -#endif -enum {SERVER_HOST =0 ,CLIENT_HOST ,
Signature · 1 line
2.15.0
cover Add headers in lapi
In reply to Petr VorelHi!
Quoted 6 lines
I posted this originally in "netstress: Add imports for
TCP_FASTOPENdefinition" [1], got reply from Alexey [2].Do we want to include system headers which originaly define definitions in lapi headers? (e.g. include <
linux/falloc.h> ininclude/lapi/fallocate.has it definesFALLOC_FL_KEEP_SIZEfromlinux/falloc.h)?
Some of them do it, some not.
I think that it's a bit cleaner to include the system header before we define the fallback definitions, since that way we are sure to pick the system ones even if test writer forgets to include the system header in the test.
But be wary of combining the linux and glibc headers, sometimes that causes collisions and all kinds of problems.
In the case of fallocate the bits/fcntl-linux.h which is included by bits/fcntl.h which is included from fcntl.h defines these constants (on older glibcs, newer include linux/fallocate.h in bits/fcntl-linux.h), so including both fcntl.h and linux/falloc.h will cause macro redefinitions in certain cases...
Quoted 1 line
If yes, do we want to check presence of them with autotools?
Most of the time we do not have to, since these headers are present everywhere. I do not think that we should be adding autoconf checks just in case. So until there is a breakage we should assume that check is not needed.
Quoted 3 lines
What is the purpose of lapi anyway? I suppose just to backport missing features added in recent glibc to older versions or missing in other libc (musl, uclibc, binder).
The purpose is to have fallback definitions in a single place, previously these were repeated over and over in each testcase, and expecially with syscall wrappers that sometimes differ sublty between architectures this was quite complicated to maintain.
Signature · 2 lines
Cyril Hrubis [email protected]
cover Add headers in lapi
Hi Cyril,
thanks for info!
Quoted 4 lines
I think that it's a bit cleaner to include the system header before we define the fallback definitions, since that way we are sure to pick the system ones even if test writer forgets to include the system header in the test.
OK. And so it's proposed to include system headers even when using lapi headers. This looks to me as a duplicity, but probably good practise.
Quoted 8 lines
But be wary of combining the linux and glibc headers, sometimes that causes collisions and all kinds of problems.
In the case of fallocate the
bits/fcntl-linux.hwhich is included bybits/fcntl.hwhich is included fromfcntl.hdefines these constants (on older glibcs, newer includelinux/fallocate.hinbits/fcntl-linux.h), so including bothfcntl.handlinux/falloc.hwill cause macro redefinitions in certain cases...
OK. IMHO most of the time glibc headers are enough (it's a question what will be newer in old distro: glibc header or kernel one).
Quoted 6 lines
Quoted 1 line
If yes, do we want to check presence of them with autotools?
Most of the time we do not have to, since these headers are present everywhere. I do not think that we should be adding autoconf checks just in case. So until there is a breakage we should assume that check is not needed.
Right!
Quoted 8 lines
Quoted 3 lines
What is the purpose of lapi anyway? I suppose just to backport missing features added in recent glibc to older versions or missing in other libc (musl, uclibc, binder).
The purpose is to have fallback definitions in a single place, previously these were repeated over and over in each testcase, and expecially with syscall wrappers that sometimes differ sublty between architectures this was quite complicated to maintain.
Sure, I understand DRY principle. I just wasn't sure of the case in testcases/network/netstress/netstress.c where no system header was used (none of linux/tcp.h and netinet/tcp.h) and macro was defined, but missing header was probably left accidentally. Checking that TCP_FASTOPEN was added in 2.18, so it's not in older distros and in current RHEL (so it's valid to be in lapi header).
Kind regards,
Petr