librdmacm "synchronisation" against libibverbs

Hi,

Please find a few patches to bring librdmacm up to date regarding autotools support and some others fixes ...

I've used patches from libibverbs to update autoconf/automake
support: I reused commit messages and author of libibverbs commit, I hope it's ok for you to get credit about those patches to librdmacm.

I've added .gitignore files to the sources and cleanup file modes.

At last, I've added support for close on exec, just like I did for libibverbs. It might be good to add support for close on exec since I've managed to get the change applied in the kernel (Thanks Roland :).

Bart Van Assche (2):
Add "foreign" option to AM_INIT_AUTOMAKE
Makefile.am: Fix an automake warning

Jeff Squyres (1):
autogen.sh: Use autoreconf in autogen.sh

Yann Droneaud (5):
configure: Apply updates proposed by autoupdate
configure: Use automake's option "subdir-objects" Add .gitignore rules
Remove executable mode bit on plain text files Open files with "close on exec" flag

 .gitignore                | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 Makefile.am               |  2 +-
 autogen.sh                |  7 +-----
 configure.ac              | 40 +++++++++++++++++++++++------
 examples/.gitignore       | 22 ++++++++++++++++
 include/rdma/rdma_cma.h   |  0
 man/rdma_accept.3         |  0
 man/rdma_connect.3        |  0
 man/rdma_create_qp.3      |  0
 man/rdma_getaddrinfo.3    |  0
 man/rdma_join_multicast.3 |  0
 man/rdma_notify.3         |  0
 src/.gitignore            |  9 +++++++
 src/acm.c                 |  4 +--
 src/addrinfo.c            |  0
 src/cma.c                 |  2 +-
 16 files changed, 132 insertions(+), 18 deletions(-)
 create mode 100644 .gitignore
 create mode 100644 examples/.gitignore
 mode change 100755 => 100644 include/rdma/rdma_cma.h
 mode change 100755 => 100644 man/rdma_accept.3
 mode change 100755 => 100644 man/rdma_connect.3
 mode change 100755 => 100644 man/rdma_create_qp.3
 mode change 100755 => 100644 man/rdma_getaddrinfo.3
 mode change 100755 => 100644 man/rdma_join_multicast.3
 mode change 100755 => 100644 man/rdma_notify.3
 create mode 100644 src/.gitignore
 mode change 100755 => 100644 src/acm.c
 mode change 100755 => 100644 src/addrinfo.c
 mode change 100755 => 100644 src/cma.c

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Bart Van Assche <[email protected]>

Switch to the modern form of the AM_INIT_AUTOMAKE macro and tell automake that the librdmacm package does not follow the GNU standards. This change makes it possible to use 'autoreconf' for the librdmacm package.

---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 35d79dd..20c2f94 100644
@@ -6,7 +6,7 @@ AC_CONFIG_SRCDIR([src/cma.c])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR(config)
 AC_CONFIG_HEADERS(config.h)
-AM_INIT_AUTOMAKE(librdmacm, 1.0.17)
+AM_INIT_AUTOMAKE([foreign])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
 AM_PROG_LIBTOOL

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Bart Van Assche <[email protected]>

Fix the following automake warning message:

Makefile.am:1: INCLUDES' is the old name for AM_CPPFLAGS' (or `*_CPPFLAGS')

A quote from the automake manual:

INCLUDES
This does the same job as AM_CPPFLAGS (or any per-target _CPPFLAGS variable if it is used). It is an older name for the same functionality. This variable is deprecated; we suggest using AM_CPPFLAGS and per-target _CPPFLAGS instead.

---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 3198165..e1be9f3 100644
@@ -1,4 +1,4 @@
-INCLUDES = -I$(srcdir)/include
+AM_CPPFLAGS = -I$(srcdir)/include
 
 rslibdir = $(libdir)/rsocket
 lib_LTLIBRARIES = src/librdmacm.la

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Jeff Squyres <[email protected]>

The old sequence of Autotools commands listed in autogen.sh is no longer correct. Instead, just use the single "autoreconf" command, which will invoke all the Right Autotools commands in the correct order.

---
 autogen.sh | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

index f433312..6c9233e 100755
@@ -1,9 +1,4 @@
 #! /bin/sh
 
 set -x
-test -d ./config || mkdir ./config
-aclocal -I config
-libtoolize --force --copy
-autoheader
-automake --foreign --add-missing --copy
-autoconf
+autoreconf -ifv -I config

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks - I pulled in these patches, but see below:

Hi,

Le 17.07.2013 06:22, Hefty, Sean a écrit :

Thanks.

Sorry for the inconvenience.

I've checked libibverbs: it has a .gitignore in ./config so that git kept the empty directory.

It's a different solution for the same problem, I'm not able to say if it's a better one.

Regards.

Yann Droneaud
OPTEYA

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Sean,

If you do this house cleanup, could you also address the below build warnings. I can see them when I build rpm from the 1.0.17 tar ball, but not when doing plain make on the latest git, probably b/c the build through the spec uses some more build/warnings flags.

Or.

configure: creating ./config.status
config.status: creating Makefile
config.status: creating librdmacm.spec
config.status: creating config.h
config.status: executing depfiles commands
config.status: executing libtool commands
+ make -j2
make all-am
make[1]: Entering directory `/usr/src/redhat/BUILD/librdmacm-1.0.17'
CC src_librdmacm_la-cma.lo
CC src_librdmacm_la-addrinfo.lo
src/addrinfo.c: In function 'ucma_convert_to_rai':
src/addrinfo.c:193: warning: dereferencing type-punned pointer will break strict-aliasing rules src/addrinfo.c:210: warning: dereferencing type-punned pointer will break strict-aliasing rules
CC src_librdmacm_la-acm.lo
CC src_librdmacm_la-rsocket.lo
src/rsocket.c: In function 'rs_modify_svcs':
src/rsocket.c:403: warning: ignoring return value of 'write', declared with attribute warn_unused_result src/rsocket.c:404: warning: ignoring return value of 'read', declared with attribute warn_unused_result src/rsocket.c: In function 'rs_configure':
src/rsocket.c:460: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:465: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:473: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:478: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:483: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:491: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c:498: warning: ignoring return value of 'fscanf', declared with attribute warn_unused_result src/rsocket.c: In function 'rs_svc_process_sock':
src/rsocket.c:3623: warning: ignoring return value of 'read', declared with attribute warn_unused_result src/rsocket.c:3632: warning: ignoring return value of 'write', declared with attribute warn_unused_result src/rsocket.c: In function 'rs_svc_run':
src/rsocket.c:3805: warning: ignoring return value of 'write', declared with attribute warn_unused_result src/rsocket.c: In function 'ds_get_dest':
src/rsocket.c:1451: warning: 'qp' may be used uninitialized in this function src/rsocket.c: In function 'rs_send_iomaps':
src/rsocket.c:2305: warning: 'ret' may be used uninitialized in this function
CC src_librdmacm_la-indexer.lo
CC src_librspreload_la-preload.lo
src/preload.c: In function 'dup2':
src/preload.c:1020: warning: value computed is not used

  CC     src_librspreload_la-indexer.lo
  CC     cmatose.o
  CC     common.o
  CC     rping.o
  CC     udaddy.o
  CC     mckey.o
  CC     rdma_client.o
  CC     rdma_server.o
  CC     rdma_xclient.o
  CC     rdma_xserver.o
  CC     rstream.o
  CC     rcopy.o
  CC     riostream.o
  CC     udpong.o
  CCLD   src/librdmacm.la
  CCLD   src/librspreload.la
  CCLD   examples/ucmatose
  CCLD   examples/rping
  CCLD   examples/udaddy
  CCLD   examples/mckey
  CCLD   examples/rdma_client
  CCLD   examples/rdma_server
  CCLD   examples/rdma_xclient
  CCLD   examples/rdma_xserver
  CCLD   examples/rstream
  CCLD   examples/rcopy
  CCLD   examples/riostream
  CCLD   examples/udpong

make[1]: Leaving directory `/usr/src/redhat/BUILD/librdmacm-1.0.17'
+ exit 0
Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.10929

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Sean,

Did you had the chance to look on cleaning these warnings too? I don't think its the one Bart was fixing, correct?

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I have not. I saved your email, but have not had time to try fixing the warnings.

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

'autoupdate' is a tool to help developer to update configure.ac.

This patch applies a few fixes as suggested by autoupdate.

---
 configure.ac | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

index 20c2f94..a9e95c0 100644
@@ -1,18 +1,18 @@
 dnl Process this file with autoconf to produce a configure script.
 
-AC_PREREQ(2.57)
-AC_INIT(librdmacm, 1.0.17, [email protected])
+AC_PREREQ([2.63])
+AC_INIT([librdmacm],[1.0.17],[[email protected]])
 AC_CONFIG_SRCDIR([src/cma.c])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR(config)
-AC_CONFIG_HEADERS(config.h)
+AC_CONFIG_HEADERS([config.h])
 AM_INIT_AUTOMAKE([foreign])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
-AM_PROG_LIBTOOL
+LT_INIT
 
 AC_ARG_WITH([valgrind],
-    AC_HELP_STRING([--with-valgrind],
+    AS_HELP_STRING([--with-valgrind],
 		   [Enable valgrind annotations - default NO]))
 
 if test "$with_valgrind" != "" && test "$with_valgrind" != "no"; then

@@ -50,8 +50,8 @@ AC_CHECK_MEMBER(struct ibv_path_record.service_id, [],
 
 dnl Check for gcc atomic intrinsics
 AC_MSG_CHECKING(compiler support for atomics)
-AC_TRY_LINK([int i = 0;],
-    [ return __sync_add_and_fetch(&i, 1) != __sync_sub_and_fetch(&i, 1); ],
+AC_LINK_IFELSE([AC_LANG_PROGRAM([[int i = 0;]],
+				[[ return __sync_add_and_fetch(&i, 1) != __sync_sub_and_fetch(&i, 1); ]])],
     [ AC_MSG_RESULT(yes) ],
     [
         AC_MSG_RESULT(no)

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Following advice in "Autotool Mythbuster" [1], option subdir-objects can be used to have Makefiles create object files in the same directory than theirs source files.

It reduces clobbering in the build directory.

[1] "Autotool Mythbuster", by Diego Elio "Flameeyes" Petten`o
http://www.flameeyes.eu/autotools-mythbuster/automake/nonrecursive.html

---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index a9e95c0..807470f 100644
@@ -6,7 +6,7 @@ AC_CONFIG_SRCDIR([src/cma.c])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR(config)
 AC_CONFIG_HEADERS([config.h])
-AM_INIT_AUTOMAKE([foreign])
+AM_INIT_AUTOMAKE([foreign subdir-objects])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
 LT_INIT

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Add the list of files/patterns to be exclueded from git status output. Additionally it will prevent such files/patterns to be added and committed.

---
 .gitignore          | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 examples/.gitignore | 22 ++++++++++++++++++
 src/.gitignore      |  9 ++++++++
 3 files changed, 95 insertions(+)
 create mode 100644 .gitignore
 create mode 100644 examples/.gitignore
 create mode 100644 src/.gitignore

new file mode 100644
index 0000000..cea1f71
@@ -0,0 +1,64 @@
+# - see gitignore(5)
+#
+# http://www.kernel.org/pub/software/scm/git/docs/gitignore.html
+# http://git-scm.com/docs/gitignore.html
+#
+# - ignore files in this current directory with a leading /
+# so that files with the same name is a subdirectory
+# would not be ignored by mistake.
+#
+# per directory .gitignore would be used to ignore specific
+# files/patterns
+#
+# - more examples can be found at
+#
+# https://github.com/github/gitignore
+# http://git-scm.com/book/en/Git-Basics-Recording-Changes-to-the-Repository#Ignoring-Files
+#
+
+# global ignore rules:
+#
+# backup files
+#
+*~
+
+# local (eg. current directory) rules:
+#
+# /<pattern>
+
+#
+# files created by autoreconf (autoconf, automake, libtool)
+#
+
+/INSTALL
+/config.guess
+/config.sub
+/missing
+/depcomp
+/install-sh
+/configure
+/aclocal.m4
+/autom4te.cache/
+/config.h.in
+/Makefile.in
+/compile
+/config
+
+#
+# files created by autoscan
+#
+
+/autoscan.log
+/configure.scan
+
+#
+# files created by ./configure
+#
+
+/config.h
+/config.status
+/config.log
+/Makefile
+/stamp-h1
+/libtool
+/librdmacm.spec

new file mode 100644
index 0000000..ed17494
@@ -0,0 +1,22 @@
+# files produced by ./configure
+/.deps
+
+# files produced by make (libtool)
+/*.o
+/*.lo
+/*.la
+/.libs
+/.dirstamp
+
+/mckey
+/rcopy
+/rdma_client
+/rdma_server
+/rdma_xclient
+/rdma_xserver
+/riostream
+/rping
+/rstream
+/ucmatose
+/udaddy
+/udpong

new file mode 100644
index 0000000..139417a
@@ -0,0 +1,9 @@
+# files produced by ./configure
+/.deps
+
+# files produced by make (libtool)
+/*.o
+/*.lo
+/*.la
+/.libs
+/.dirstamp

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Source code and man page should not be executable.

---
 include/rdma/rdma_cma.h   | 0
 man/rdma_accept.3         | 0
 man/rdma_connect.3        | 0
 man/rdma_create_qp.3      | 0
 man/rdma_getaddrinfo.3    | 0
 man/rdma_join_multicast.3 | 0
 man/rdma_notify.3         | 0
 src/acm.c                 | 0
 src/addrinfo.c            | 0
 src/cma.c                 | 0
 10 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 include/rdma/rdma_cma.h
 mode change 100755 => 100644 man/rdma_accept.3
 mode change 100755 => 100644 man/rdma_connect.3
 mode change 100755 => 100644 man/rdma_create_qp.3
 mode change 100755 => 100644 man/rdma_getaddrinfo.3
 mode change 100755 => 100644 man/rdma_join_multicast.3
 mode change 100755 => 100644 man/rdma_notify.3
 mode change 100755 => 100644 src/acm.c
 mode change 100755 => 100644 src/addrinfo.c
 mode change 100755 => 100644 src/cma.c

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

old mode 100755
new mode 100644

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

File opened by librdmacm are not supposed to be inherited across exec*(), most of the files are of no use for another program, and others cannot be used without the associated memory mapping.

This patch changes fopen() open() and socket() to always set close on exec flag.

This patch also add checks to configure to guess if fopen() supports "e" flag. If O_CLOEXEC and SOCK_CLOEXEC are supported, fopen() should support "e". If not supported, its discarded according to POSIX. Many operating systems have support for fopen("e").

You might find more information about close on exec in the following articles:

- "Excuse me son, but your code is leaking !!!" by Dan Walsh
http://danwalsh.livejournal.com/53603.html

- "Secure File Descriptor Handling" by Ulrich Drepper
http://udrepper.livejournal.com/20407.html

Note: this patch won't set close on exec flag on file descriptors created by the kernel for completion channel and such. This is addressed by another kernel patch.

---
 configure.ac | 24 ++++++++++++++++++++++++
 src/acm.c    |  4 ++--
 src/cma.c    |  2 +-
 3 files changed, 27 insertions(+), 3 deletions(-)

index 807470f..31b1a0e 100644
@@ -78,6 +78,30 @@ AC_CHECK_HEADER(infiniband/acm.h,
 		AC_DEFINE(DEFINE_ACM_MSG, 1, [adding ACM message definition]),
 			[#include <infiniband/acm.h>]), [])
 
+dnl Checks close on exec support
+AC_CHECK_HEADERS([fcntl.h sys/socket.h])
+
+AC_CHECK_DECLS([O_CLOEXEC],,[AC_DEFINE([O_CLOEXEC],[0], [Defined to 0 if not provided])],
+[[
+#ifdef HAVE_FCNTL_H
+# include <fcntl.h>
+#endif
+]])
+AC_CHECK_DECLS([SOCK_CLOEXEC],,[AC_DEFINE([SOCK_CLOEXEC],[0],[Defined to 0 if not provided])],
+[[
+#ifdef HAVE_SYS_SOCKET_H
+# include <sys/socket.h>
+#endif
+]])
+
+AC_CACHE_CHECK(for close on exec modifier for fopen(), ac_cv_feature_stream_cloexec_flag,
+   [if test $ac_cv_have_decl_O_CLOEXEC = yes ; then
+        if test $ac_cv_have_decl_SOCK_CLOEXEC = yes ; then
+            ac_cv_feature_stream_cloexec_flag="e"
+        fi
+    fi])
+AC_DEFINE_UNQUOTED([STREAM_CLOEXEC], "$ac_cv_feature_stream_cloexec_flag", [fopen() modifier for setting close on exec flag])
+
 AC_CACHE_CHECK(whether ld accepts --version-script, ac_cv_version_script,
     if test -n "`$LD --help < /dev/null 2>/dev/null | grep version-script`"; then
         ac_cv_version_script=yes

index c9ca5b5..6e8e173 100644
@@ -80,7 +80,7 @@ static int ucma_set_server_port(void)
 {
 	FILE *f;
 
-	if ((f = fopen("/var/run/ibacm.port", "r"))) {
+	if ((f = fopen("/var/run/ibacm.port", "r" STREAM_CLOEXEC))) {
 		fscanf(f, "%hu", (unsigned short *) &server_port);
 		fclose(f);
 	}

@@ -100,7 +100,7 @@ void ucma_ib_init(void)
 	if (!ucma_set_server_port())
 		goto out;
 
-	sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	sock = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 	if (sock < 0)
 		goto out;
 

index 2fb9913..baebecd 100644
@@ -328,7 +328,7 @@ struct rdma_event_channel *rdma_create_event_channel(void)
 	if (!channel)
 		return NULL;
 
-	channel->fd = open("/dev/infiniband/rdma_cm", O_RDWR);
+	channel->fd = open("/dev/infiniband/rdma_cm", O_RDWR | O_CLOEXEC);
 	if (channel->fd < 0) {
 		fprintf(stderr, PFX "Fatal: unable to open /dev/infiniband/rdma_cm\n");
 		goto err;

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html