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.
i40iw uses a variant of the same code, so I do that same thing there
for ipv4. Unlike nes, i40e also supports ipv6, which needs to call
secure_tcpv6_seq instead.
Signed-off-by: Arnd Bergmann <[email protected]>
---
v2: use secure_tcpv6_seq for IPv6 support as suggested by Shiraz Saleem.
---
drivers/infiniband/hw/i40iw/i40iw_cm.c | 26 +++++++++++++++++++++-----
drivers/infiniband/hw/nes/nes_cm.c | 8 +++++---
net/core/secure_seq.c | 1 +
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 7b2655128b9f..e2590784b857 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -57,6 +57,7 @@
#include <net/addrconf.h>
#include <net/ip6_route.h>
#include <net/ip_fib.h>
+#include <net/secure_seq.h>
#include <net/tcp.h>
#include <asm/checksum.h>
@@ -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,10 +2214,26 @@ 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.mss = (cm_node->ipv4) ? (iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV4) :
- (iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV6);
+ if (cm_node->ipv4) {
+ 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));
+ cm_node->tcp_cntxt.mss = iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV4;
+ } else {
+ __be32 loc[4] = {
+ htonl(cm_node->loc_addr[0]), htonl(cm_node->loc_addr[1]),
+ htonl(cm_node->loc_addr[2]), htonl(cm_node->loc_addr[3])
+ };
+ __be32 rem[4] = {
+ htonl(cm_node->rem_addr[0]), htonl(cm_node->rem_addr[1]),
+ htonl(cm_node->rem_addr[2]), htonl(cm_node->rem_addr[3])
+ };
+ cm_node->tcp_cntxt.loc_seq_num = secure_tcpv6_seq(loc, rem,
+ htons(cm_node->loc_port),
+ htons(cm_node->rem_port));
+ cm_node->tcp_cntxt.mss = iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV6;
+ }
cm_node->iwdev = iwdev;
cm_node->dev = &iwdev->sc_dev;
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index 6cdfbf8c5674..2b67ace5b614 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -58,6 +58,7 @@
#include <net/neighbour.h>
#include <net/route.h>
#include <net/ip_fib.h>
+#include <net/secure_seq.h>
#include <net/tcp.h>
#include <linux/fcntl.h>
@@ -1445,7 +1446,6 @@ static struct nes_cm_node *make_cm_node(struct nes_cm_core *cm_core,
struct nes_cm_listener *listener)
{
struct nes_cm_node *cm_node;
- struct timespec ts;
int oldarpindex = 0;
int arpindex = 0;
struct nes_device *nesdev;
@@ -1496,8 +1496,10 @@ static struct nes_cm_node *make_cm_node(struct nes_cm_core *cm_core,
cm_node->tcp_cntxt.rcv_wscale = NES_CM_DEFAULT_RCV_WND_SCALE;
cm_node->tcp_cntxt.rcv_wnd = NES_CM_DEFAULT_RCV_WND_SCALED >>
NES_CM_DEFAULT_RCV_WND_SCALE;
- ts = current_kernel_time();
- cm_node->tcp_cntxt.loc_seq_num = htonl(ts.tv_nsec);
+ cm_node->tcp_cntxt.loc_seq_num = secure_tcp_seq(htonl(cm_node->loc_addr),
+ htonl(cm_node->rem_addr),
+ htons(cm_node->loc_port),
+ htons(cm_node->rem_port));
cm_node->tcp_cntxt.mss = nesvnic->max_frame_size - sizeof(struct iphdr) -
sizeof(struct tcphdr) - ETH_HLEN - VLAN_HLEN;
cm_node->tcp_cntxt.rcv_nxt = 0;
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 7232274de334..af6ad467ed61 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -140,6 +140,7 @@ u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
&net_secret);
return seq_scale(hash);
}
+EXPORT_SYMBOL_GPL(secure_tcp_seq);
u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
{
--
2.9.0
On Wed, Jun 27, 2018 at 07:26:05AM -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.
>
> i40iw uses a variant of the same code, so I do that same thing there
> for ipv4. Unlike nes, i40e also supports ipv6, which needs to call
> secure_tcpv6_seq instead.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> v2: use secure_tcpv6_seq for IPv6 support as suggested by Shiraz Saleem.
> ---
Looks good.
Acked-by: Shiraz Saleem <[email protected]>
Hi Arnd,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc2 next-20180629]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Arnd-Bergmann/infiniband-i40iw-nes-don-t-use-wall-time-for-TCP-sequence-numbers/20180627-221142
config: x86_64-randconfig-s2-06302231 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/infiniband/hw/i40iw/i40iw_cm.o: In function `i40iw_make_cm_node':
>> drivers/infiniband/hw/i40iw/i40iw_cm.c:2232: undefined reference to `secure_tcpv6_seq'
vim +2232 drivers/infiniband/hw/i40iw/i40iw_cm.c
2153
2154 /**
2155 * i40iw_make_cm_node - create a new instance of a cm node
2156 * @cm_core: cm's core
2157 * @iwdev: iwarp device structure
2158 * @cm_info: quad info for connection
2159 * @listener: passive connection's listener
2160 */
2161 static struct i40iw_cm_node *i40iw_make_cm_node(
2162 struct i40iw_cm_core *cm_core,
2163 struct i40iw_device *iwdev,
2164 struct i40iw_cm_info *cm_info,
2165 struct i40iw_cm_listener *listener)
2166 {
2167 struct i40iw_cm_node *cm_node;
2168 int oldarpindex;
2169 int arpindex;
2170 struct net_device *netdev = iwdev->netdev;
2171
2172 /* create an hte and cm_node for this instance */
2173 cm_node = kzalloc(sizeof(*cm_node), GFP_ATOMIC);
2174 if (!cm_node)
2175 return NULL;
2176
2177 /* set our node specific transport info */
2178 cm_node->ipv4 = cm_info->ipv4;
2179 cm_node->vlan_id = cm_info->vlan_id;
2180 if ((cm_node->vlan_id == I40IW_NO_VLAN) && iwdev->dcb)
2181 cm_node->vlan_id = 0;
2182 cm_node->tos = cm_info->tos;
2183 cm_node->user_pri = cm_info->user_pri;
2184 if (listener) {
2185 if (listener->tos != cm_info->tos)
2186 i40iw_debug(&iwdev->sc_dev, I40IW_DEBUG_DCB,
2187 "application TOS[%d] and remote client TOS[%d] mismatch\n",
2188 listener->tos, cm_info->tos);
2189 cm_node->tos = max(listener->tos, cm_info->tos);
2190 cm_node->user_pri = rt_tos2priority(cm_node->tos);
2191 i40iw_debug(&iwdev->sc_dev, I40IW_DEBUG_DCB, "listener: TOS:[%d] UP:[%d]\n",
2192 cm_node->tos, cm_node->user_pri);
2193 }
2194 memcpy(cm_node->loc_addr, cm_info->loc_addr, sizeof(cm_node->loc_addr));
2195 memcpy(cm_node->rem_addr, cm_info->rem_addr, sizeof(cm_node->rem_addr));
2196 cm_node->loc_port = cm_info->loc_port;
2197 cm_node->rem_port = cm_info->rem_port;
2198
2199 cm_node->mpa_frame_rev = iwdev->mpa_version;
2200 cm_node->send_rdma0_op = SEND_RDMA_READ_ZERO;
2201 cm_node->ird_size = I40IW_MAX_IRD_SIZE;
2202 cm_node->ord_size = I40IW_MAX_ORD_SIZE;
2203
2204 cm_node->listener = listener;
2205 cm_node->cm_id = cm_info->cm_id;
2206 ether_addr_copy(cm_node->loc_mac, netdev->dev_addr);
2207 spin_lock_init(&cm_node->retrans_list_lock);
2208 cm_node->ack_rcvd = false;
2209
2210 atomic_set(&cm_node->ref_count, 1);
2211 /* associate our parent CM core */
2212 cm_node->cm_core = cm_core;
2213 cm_node->tcp_cntxt.loc_id = I40IW_CM_DEF_LOCAL_ID;
2214 cm_node->tcp_cntxt.rcv_wscale = I40IW_CM_DEFAULT_RCV_WND_SCALE;
2215 cm_node->tcp_cntxt.rcv_wnd =
2216 I40IW_CM_DEFAULT_RCV_WND_SCALED >> I40IW_CM_DEFAULT_RCV_WND_SCALE;
2217 if (cm_node->ipv4) {
2218 cm_node->tcp_cntxt.loc_seq_num = secure_tcp_seq(htonl(cm_node->loc_addr[0]),
2219 htonl(cm_node->rem_addr[0]),
2220 htons(cm_node->loc_port),
2221 htons(cm_node->rem_port));
2222 cm_node->tcp_cntxt.mss = iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV4;
2223 } else {
2224 __be32 loc[4] = {
2225 htonl(cm_node->loc_addr[0]), htonl(cm_node->loc_addr[1]),
2226 htonl(cm_node->loc_addr[2]), htonl(cm_node->loc_addr[3])
2227 };
2228 __be32 rem[4] = {
2229 htonl(cm_node->rem_addr[0]), htonl(cm_node->rem_addr[1]),
2230 htonl(cm_node->rem_addr[2]), htonl(cm_node->rem_addr[3])
2231 };
> 2232 cm_node->tcp_cntxt.loc_seq_num = secure_tcpv6_seq(loc, rem,
2233 htons(cm_node->loc_port),
2234 htons(cm_node->rem_port));
2235 cm_node->tcp_cntxt.mss = iwdev->vsi.mtu - I40IW_MTU_TO_MSS_IPV6;
2236 }
2237
2238 cm_node->iwdev = iwdev;
2239 cm_node->dev = &iwdev->sc_dev;
2240
2241 if ((cm_node->ipv4 &&
2242 i40iw_ipv4_is_loopback(cm_node->loc_addr[0], cm_node->rem_addr[0])) ||
2243 (!cm_node->ipv4 && i40iw_ipv6_is_loopback(cm_node->loc_addr,
2244 cm_node->rem_addr))) {
2245 arpindex = i40iw_arp_table(iwdev,
2246 cm_node->rem_addr,
2247 false,
2248 NULL,
2249 I40IW_ARP_RESOLVE);
2250 } else {
2251 oldarpindex = i40iw_arp_table(iwdev,
2252 cm_node->rem_addr,
2253 false,
2254 NULL,
2255 I40IW_ARP_RESOLVE);
2256 if (cm_node->ipv4)
2257 arpindex = i40iw_addr_resolve_neigh(iwdev,
2258 cm_info->loc_addr[0],
2259 cm_info->rem_addr[0],
2260 oldarpindex);
2261 else if (IS_ENABLED(CONFIG_IPV6))
2262 arpindex = i40iw_addr_resolve_neigh_ipv6(iwdev,
2263 cm_info->loc_addr,
2264 cm_info->rem_addr,
2265 oldarpindex);
2266 else
2267 arpindex = -EINVAL;
2268 }
2269 if (arpindex < 0) {
2270 i40iw_pr_err("cm_node arpindex\n");
2271 kfree(cm_node);
2272 return NULL;
2273 }
2274 ether_addr_copy(cm_node->rem_mac, iwdev->arp_table[arpindex].mac_addr);
2275 i40iw_add_hte_node(cm_core, cm_node);
2276 cm_core->stats_nodes_created++;
2277 return cm_node;
2278 }
2279
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, Jun 30, 2018 at 9:00 PM, kbuild test robot <[email protected]> wrote:
> Hi Arnd,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.18-rc2 next-20180629]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Arnd-Bergmann/infiniband-i40iw-nes-don-t-use-wall-time-for-TCP-sequence-numbers/20180627-221142
> config: x86_64-randconfig-s2-06302231 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> drivers/infiniband/hw/i40iw/i40iw_cm.o: In function `i40iw_make_cm_node':
>>> drivers/infiniband/hw/i40iw/i40iw_cm.c:2232: undefined reference to `secure_tcpv6_seq'
>
Fixed this now by added a
depends on IPV6 || !IPV6
dependency in Kconfig. This ensures that with IPV6=m, i40iw cannot be built-in.
Will send v3 after a little more build testing.
Arnd