The rds network protocol was created mainly for Oracle and is used heavily by Oracle in the latest generation RAC products. The protocol is basically UDP with a reliability guarantee, and RDS stands for Reliable Datagram Service. These utilities allow a user to test the RDS stack in the kernel to make sure it is properly configured and operational.
Just some comments: - License seems to be GPLv2 or BSD -- not GPLv2+ or BSD - You can use the name macro in Source0 - Comment on the patches in the spec file, if possible - FSF address is wrong in rds-sample.c - Please harmonize the use of RPM_BUILD_ROOT and buildroot macro - If you don't go for EPEL 5, you can delete the buildroot definition, the clean section and the rm in the install section - defattr is no longer necessary - Remove README from the documentation as it holds no valuable information - The optflags are not used when compiling - You can patch the permissions for the executables in rds-tools-make.patch instead of changing them in the install section - What is that rds.conf file about?
I forgot: There are two consecutive spaces in your description. This seems to be the case for some of your other packages too. By the way, review requests should follow this guideline: http://fedoraproject.org/wiki/New_package_process_for_existing_contributors That is, giving the URL to spec file and SRPM in the text. You can also add the rpmlint output and a link to a Koji scratch build, what many reviewers appreciate.
$ rpmlint rds-tools.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint rds-tools-2.0.6-1.fc15.x86_64.rpm rds-tools.x86_64: W: spelling-error %description -l en_US iWARP -> i Warp, warp, antiwar rds-tools.x86_64: W: non-conffile-in-etc /etc/modprobe.d/rds.conf rds-tools.x86_64: E: incorrect-fsf-address /usr/share/doc/rds-tools-2.0.6/examples/rds-sample.c 1 packages and 0 specfiles checked; 1 errors, 2 warnings. The tools run fine here.
By the way, running rds-info causes the following kernel warning: [38559.194671] ------------[ cut here ]------------ [38559.194752] WARNING: at kernel/softirq.c:159 _local_bh_enable_ip+0x44/0x8e() [38559.194824] Hardware name: MacPro3,1 [38559.194893] Modules linked in: rds_rdma rds_tcp rds bluetooth rfkill appletalk ipx p8022 psnap llc p8023 rose ax25 lockd nf_conntrack_tftp nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ftp xt_state ib_srp scsi_transport_srp scsi_tgt ib_ipoib rdma_ucm ipt_MASQUERADE ib_ucm coretemp ib_uverbs iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack ib_umad ip6_tables nf_defrag_ipv4 rdma_cm ib_cm iw_cm ib_addr ib_sa mlx4_ib ib_mad ib_core mlx4_en snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm joydev fglrx(P) snd_timer snd i5400_edac ioatdma edac_core soundcore iTCO_wdt iTCO_vendor_support shpchp igb e1000e mlx4_core applesmc i2c_i801 input_polldev i5k_amb snd_page_alloc dca microcode virtio_net kvm_intel kvm binfmt_misc sunrpc raid10 firewire_ohci firewire_core crc_itu_t radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded: scsi_wait_scan] [38559.195007] Pid: 24813, comm: rds-info Tainted: P I 3.1.1-1.fc16.x86_64 #1 [38559.195007] Call Trace: [38559.195007] [<ffffffff81057a1e>] warn_slowpath_common+0x83/0x9b [38559.195007] [<ffffffff81057a50>] warn_slowpath_null+0x1a/0x1c [38559.195007] [<ffffffff8105d42a>] _local_bh_enable_ip+0x44/0x8e [38559.195007] [<ffffffff8105d482>] local_bh_enable_ip+0xe/0x10 [38559.195007] [<ffffffff814b72ed>] _raw_read_unlock_bh+0x15/0x17 [38559.195007] [<ffffffff813cb0bc>] sock_i_ino+0x38/0x40 [38559.195007] [<ffffffffa073a786>] rds_sock_info+0xa7/0x10e [rds] [38559.195007] [<ffffffffa073c35d>] rds_info_getsockopt+0x11a/0x1b9 [rds] [38559.195007] [<ffffffffa073a886>] rds_getsockopt+0x70/0xcc [rds] [38559.195007] [<ffffffff813c955b>] sys_getsockopt+0x7a/0x98 [38559.195007] [<ffffffff814bd8c2>] system_call_fastpath+0x16/0x1b [38559.195007] ---[ end trace 40181a0855375f5a ]--- I reported it here: https://bugzilla.redhat.com/show_bug.cgi?id=718790 but that bug got closed.
(In reply to comment #1) > Just some comments: > > - License seems to be GPLv2 or BSD -- not GPLv2+ or BSD > - You can use the name macro in Source0 > - Comment on the patches in the spec file, if possible All done. > - FSF address is wrong in rds-sample.c While checking on this, I noted that the main source files redirect a person to a copy of the GPLv2 in the file COPYING, yet that file isn't present. > - Please harmonize the use of RPM_BUILD_ROOT and buildroot macro Done. > - If you don't go for EPEL 5, you can delete the buildroot definition, the > clean section and the rm in the install section The package will be shared with RHEL, including RHEL5, so these should stay for now. > - defattr is no longer necessary See above. > - Remove README from the documentation as it holds no valuable information Done. > - The optflags are not used when compiling This is fixed, but it's kind of ugly. The upstream build files simply do not honor passed in CFLAGS via the environment, only if you explicitly set CFLAGS to the make comment are they honored (and this is in contradiction with the fact that configure says you can do CFLAGS=blah and it will put it in the makefile, it doesn't actually do so however). And the makefile doesn't append its own stuff to the passed in CFLAGS, it's either the built in CFLAGS or the passed in, no merging. So, I had to include the one item that the makefile needs in addition to the rpm opt stuff in order for it to compile. > - You can patch the permissions for the executables in rds-tools-make.patch > instead of changing them in the install section Done. > - What is that rds.conf file about? The rds protocol is autoloaded if anyone attempts to open an rds socket. However, there is no method in place to determine if either the tcp transport, rdma transport, or both should be loaded in order to satisfy the needs of the rds protocol. So, whenever we load the rds protocol, also load both transport modules. That way, whatever one is appropriate can be used to service the request versus failing to open an rds socket even though the rds protocol was successfully loaded.
(In reply to comment #5) > (In reply to comment #1) > > - FSF address is wrong in rds-sample.c > > While checking on this, I noted that the main source files redirect a person to > a copy of the GPLv2 in the file COPYING, yet that file isn't present. I meant to continue on to say that this is a legitimate issue that upstream needs to address. I'll contact them about both adding a COPYING file and making sure that the address in it is correct and also about updating the rds-sample.c file to have the proper address.
(In reply to comment #4) > By the way, running rds-info causes the following kernel warning: > > > [38559.194671] ------------[ cut here ]------------ > [38559.194752] WARNING: at kernel/softirq.c:159 _local_bh_enable_ip+0x44/0x8e() Yes, the upstream rds kernel support is somewhat lacking. Last I knew, Oracle carried over 50 patches in their kernel that they haven't bothered to send upstream, and presumably the fix for such an obvious DOA type issue as this one is among them.
Upstream has been notified of the license issues. I'll ping in this review bug again when a package with a new tarball that fixes the issue is available.
Until the new tarball is here, I marked this package as "NotReady", remove it once it will be good again to be reviewed.
Is this review still alive?
Considering that upstream rds kernel support is totally broken, I don't really care much about this package. I generally recommend that rds support in the kernel be entirely disabled at this point. If that changes, I can issue a new review request against an updated version of this package.