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

[Ka-Cheong Poon <ka-cheong.poon@oracle.com>]

On 07/17/2018 12:20 AM, Sowmini Varadhan wrote:
> 
> -  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.


The app can use either structures to make the call.  When the
app fills in the structure, it knows what it is filling in,
either sockaddr_in or sockaddr_in6.  So it knows the right size
to use.  The app can also use IPv4 mapped address in a sockaddr_in6
without a problem.


>     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.


Could you please explain the inconsistency?  An app can use IPv4
mapped address in a sockaddr_in6 to operate on an IPv4 connection,
in case you are thinking of this new addition in v3 of the patch.


> -  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.


Will update it to


/* The following ports, 16385, 18634, 18635, are registered with IANA as 

  * the ports to be used for RDS over TCP and UDP.  Currently, only RDS 
over
  * TCP and RDS over IB/RDMA are implemented.  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. 

  */



-- 
K. Poon
ka-cheong.poon@oracle.com