On Thu, 25 Apr 2019 19:31:51 +0000, Saeed Mahameed wrote:
> > diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> > index cb368efe3567..6686013b4e9e 100644
> > --- a/net/tls/tls_device.c
> > +++ b/net/tls/tls_device.c
> > @@ -567,7 +567,7 @@ void (struct sock *sk, u32
> > seq, u64 rcd_sn)
> >
> > rx_ctx = tls_offload_ctx_rx(tls_ctx);
> > resync_req = atomic64_read(&rx_ctx->resync_req);
> > - req_seq = ntohl(resync_req >> 32) - ((u32)TLS_HEADER_SIZE - 1);
> > + req_seq = (resync_req >> 32) - ((u32)TLS_HEADER_SIZE - 1);
>
> this is not equivalent to what was before,
> resync_req is expected to be in network order,
> (TLS_HEADER_SIZE -1) is still in cpu indianness.
Naw, I think they are both in host order.
The driver passes network order.
But the stack has it in host order, this is the call site:
#ifdef CONFIG_TLS_DEVICE
handle_device_resync(strp->sk, TCP_SKB_CB(skb)->seq + rxm->offset,
*(u64*)tls_ctx->rx.rec_seq);
#endif
The value passed by the driver used to be byte swapped when read from
the atomic, but I moved the byte swap to when it's stored to the atomic.
We used to have a weird situation where the atomic would have a be32 on
the top 32bits, and lower 32 bits would store the 1, in host order.
IOW the tls_offload_rx_resync_request() is the only thing setting this
value and I moved the byte swap there.
$ git grep '.->resync_req'
include/net/tls.h: atomic64_set(&rx_ctx->resync_req, ((u64)ntohl(seq) <<
32) | 1);
net/tls/tls_device.c: resync_req = atomic64_read(&rx_ctx->resync_req);
net/tls/tls_device.c: atomic64_try_cmpxchg(&rx_ctx->resync_req,
&resync_req, 0))
Did I miss something or screw up tls_offload_rx_resync_request()?
