Re: [PATCH] infiniband: i40iw, nes: don't use wall time for TCP sequence numbers
From: Shiraz Saleem <hidden>
Date: 2018-06-23 00:01:49
Also in:
linux-rdma, lkml
On Mon, Jun 18, 2018 at 08:44:17AM -0600, Arnd Bergmann wrote:
The nes infiniband driver uses current_kernel_time() to get a nanosecond granunarity timestamp to initialize its tcp sequence counters. This is one of only a few remaining users of that deprecated function, so we should try to get rid of it. Aside from using a deprecated API, there are several problems I see here: - Using a CLOCK_REALTIME based time source makes it predictable in case the time base is synchronized. - Using a coarse timestamp means it only gets updated once per jiffie, making it even more predictable in order to avoid having to access the hardware clock source - The upper 2 bits are always zero because the nanoseconds are at most 999999999. For the Linux TCP implementation, we use secure_tcp_seq(), which appears to be appropriate here as well, and solves all the above problems. I'm doing the same change in both versions of the nes driver, with i40iw being a later copy of the same code. Signed-off-by: Arnd Bergmann <arnd@arndb.de> ---
Thanks Arnd for the patch! [...]
quoted hunk ↗ jump to hunk
@@ -2164,7 +2165,6 @@ static struct i40iw_cm_node *i40iw_make_cm_node( struct i40iw_cm_listener *listener) { struct i40iw_cm_node *cm_node; - struct timespec ts; int oldarpindex; int arpindex; struct net_device *netdev = iwdev->netdev;@@ -2214,8 +2214,10 @@ static struct i40iw_cm_node *i40iw_make_cm_node( cm_node->tcp_cntxt.rcv_wscale = I40IW_CM_DEFAULT_RCV_WND_SCALE; cm_node->tcp_cntxt.rcv_wnd = I40IW_CM_DEFAULT_RCV_WND_SCALED >> I40IW_CM_DEFAULT_RCV_WND_SCALE; - ts = current_kernel_time(); - cm_node->tcp_cntxt.loc_seq_num = ts.tv_nsec; + cm_node->tcp_cntxt.loc_seq_num = secure_tcp_seq(htonl(cm_node->loc_addr[0]), + htonl(cm_node->rem_addr[0]), + htons(cm_node->loc_port), + htons(cm_node->rem_port));
Should we not be using secure_tcpv6_seq() when we are ipv6? Shiraz
cm_node->tcp_cntxt.mss = (cm_node->ipv4) ? (iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV4) : (iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV6);