netdev - Re: [net-next 14/14] net/mlx5e: Add kTLS TX HW offload support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190705124351.4abfc7f3@cakuba.netronome.com>
Date:   Fri, 5 Jul 2019 12:43:51 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Tariq Toukan <tariqt@...lanox.com>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Boris Pismenny <borisp@...lanox.com>
Subject: Re: [net-next 14/14] net/mlx5e: Add kTLS TX HW offload support

On Fri, 5 Jul 2019 14:31:29 +0000, Tariq Toukan wrote:
> On 7/4/2019 11:12 PM, Jakub Kicinski wrote:
> > On Thu, 4 Jul 2019 18:16:15 +0000, Saeed Mahameed wrote:  
> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> >> index 483d321d2151..6854f132d505 100644
> >> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> >> @@ -50,6 +50,15 @@ static const struct counter_desc sw_stats_desc[] = {
> >>   #ifdef CONFIG_MLX5_EN_TLS
> >>   	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_tls_ooo) },
> >>   	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_tls_resync_bytes) },
> >> +
> >> +	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_ktls_ooo) },  
> > 
> > Why do you call this stat tx_ktls_ooo, and not tx_tls_ooo (extra 'k')?
> > 
> > For nfp I used the stats' names from mlx5 FPGA to make sure we are all
> > consistent.  I've added them to the tls-offload.rst doc and Boris has
> > reviewed it.
> > 
> >   * ``rx_tls_decrypted`` - number of successfully decrypted TLS segments
> >   * ``tx_tls_encrypted`` - number of in-order TLS segments passed to device
> >     for encryption
> >   * ``tx_tls_ooo`` - number of TX packets which were part of a TLS stream
> >     but did not arrive in the expected order
> >   * ``tx_tls_drop_no_sync_data`` - number of TX packets dropped because
> >     they arrived out of order and associated record could not be found
> > 
> > Why can't you use the same names for the stats as you used for your mlx5
> > FPGA?
> >   
> 
> Agree. Fixing.
> 
> What about having stats both for packets and bytes?
> tx_tls_encrypted_packets
> tx_tls_encrypted_bytes

Makes sense, I wasn't sure we want too many counters on the fastpath,
therefore I had no bytes counter. Renaming rx_tls_decrypted and
tx_tls_encrypted sounds like a good idea, though! We only have them in
the nfp, and there wasn't any kernel released with nfp TLS offload,
yet, so we can adjust.

Perhaps we should also make it clear in the doc that those counters
count "packets"/bytes on the wire, while the other counters count the
number of skbs, which may be TSO?

Would you be able to make those adjustments if you agree?