[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <954b0239-5d03-997c-d242-cbbd8c0dc8e4@oracle.com>
Date: Tue, 17 Jul 2018 13:32:09 +0800
From: Ka-Cheong Poon <ka-cheong.poon@...cle.com>
To: Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc: netdev@...r.kernel.org, santosh.shilimkar@...cle.com,
davem@...emloft.net, rds-devel@....oracle.com
Subject: Re: [PATCH v3 net-next 0/3] rds: IPv6 support
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@...cle.com
Powered by blists - more mailing lists