[rds-devel] [PATCH v3 net-next 0/3] rds: IPv6 support

[Sowmini Varadhan]

-  Looks like rds_connect() is checking things in the right order (thanks)
   However, rds_cancel_sent_to is still looking at the len to figure
   out the family.. as we move to ipv6,  it would be better if we allow
   the caller to specify struct sockaddr_storage, or even a union of
   sockaddr_in/sockaddr_in6, rather than require them to hint at which 
   one of ipv4/ipv6 through the optlen.

   Please see __sys_connect and move_addr_to_kernel if the user-kernel
   copy is the reason you are not doing this. Similar to inet_dgram_connect
   you can then check the sa_family and use that to figure out the
   "Assume IPv4" etc stuff.

   This would also make the CANCEL_SEND_TO API consistent with the bind/
   connect etc semantics.
   
-  net/rds/rds.h: thanks for moving RDS_CM_PORT to the rdma specific file.

   I am guessing (?) that you want to update the comment to talk about
   the non-existent "RDS over UDP" based on the title of the IANA registration?
   I would just like to re-iterate that this is actually inaccurate
   (and confusing to someone looking at this for the first time, since
   there is no RDS-over-UDP today). If it were up to me, I would update
   the comment to say

/* The following ports, 16385, 18634, 18635, are registered with IANA as
 * the ports to be used for "RDS over TCP and UDP".
 * The current linux implementation supports RDS over TCP and IB, and uses
 * the ports as follows: 18634 is the historical value used for the
 * RDMA_CM listener port.  RDS/TCP uses port 16385.  After
 * IPv6 work, RDMA_CM also uses 16385 as the listener port.  18634 is kept
 * to ensure compatibility with older RDS modules.  Those ports are defined
 * in each transport's header file.

IMHO that makes the comment look a little less odd (I've already explained
to you why RDS-over-UDP does not make much practical sense for the RDS
use-cases we anticipate). YMMV.

Thanks,

--Sowmini