From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH v3 net-next 1/6] tcp: Add clean acked data hook
Date: Tue, 19 Dec 2017 11:28:51 -0800
Message-ID: <1513711731.64874.6.camel@gmail.com>
References: <20171218111033.13256-1-ilyal@mellanox.com>
         <20171218111033.13256-2-ilyal@mellanox.com>
         <1513710804.64874.3.camel@gmail.com>
         <AM4PR0501MB2723A3177E58E20777689A56D40F0@AM4PR0501MB2723.eurprd05.prod.outlook.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Cc: "davejwatson@fb.com" <davejwatson@fb.com>,
        "tom@herbertland.com" <tom@herbertland.com>,
        "hannes@stressinduktion.org" <hannes@stressinduktion.org>,
        Boris Pismenny <borisp@mellanox.com>,
        Aviad Yehezkel <aviadye@mellanox.com>,
        Liran Liss <liranl@mellanox.com>
To: Ilya Lesokhin <ilyal@mellanox.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wm0-f44.google.com ([74.125.82.44]:42982 "EHLO
        mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752776AbdLST2z (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 19 Dec 2017 14:28:55 -0500
Received: by mail-wm0-f44.google.com with SMTP id b199so5867341wme.1
        for <netdev@vger.kernel.org>; Tue, 19 Dec 2017 11:28:55 -0800 (PST)
In-Reply-To: <AM4PR0501MB2723A3177E58E20777689A56D40F0@AM4PR0501MB2723.eurprd05.prod.outlook.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Tue, 2017-12-19 at 19:21 +0000, Ilya Lesokhin wrote:
> > 1) tcp_ack() is already very expensive.
> > 
> 
> I'm not sure how what we should do with that comment. We need
> Some trigger to free TLS records. tcp_ack seemed like a reasonable
> Trigger.

TLS records should be attached to skbs ?

It seems more reasonable to free TLS when skb are freed, and not in
general tcp_ack() path.

> 
> > 2) Since you do not pass any state here, this looks very suspicious to
> > me.
> > 
> 
> The state we need is the acknowledged sequence and it located in the socket.
> https://github.com/Mellanox/tls-offload/blob/tls_device_v3/net/tls/tls_device.c#L157

So it looks like TCP stack is bleeding all over the places ?

So in the future, a change in TCP stack will have to make sure we do
not break net/tls/... compilation.

Not pretty.