netdev - Re: [net-next V2 09/15] net/mlx5e: CT: Use the same counter for both directions

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9b24cd270def8ea5432fc117e4fd1ed9c756a58d.camel@kernel.org>
Date:   Tue, 01 Dec 2020 13:41:21 -0800
From:   Saeed Mahameed <saeed@...nel.org>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Ariel Levkovich <lariel@...dia.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Oz Shlomo <ozsh@...lanox.com>, Roi Dayan <roid@...dia.com>,
        Marcelo Ricardo Leitner <mleitner@...hat.com>
Subject: Re: [net-next V2 09/15] net/mlx5e: CT: Use the same counter for
 both directions

On Fri, 2020-11-27 at 11:01 -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 23, 2020 at 03:48:18PM -0700, saeed@...nel.org wrote:
> > From: Oz Shlomo <ozsh@...lanox.com>
> 
> Sorry for reviving this one, but seemed better for the context.
> 
> > A connection is represented by two 5-tuple entries, one for each
> > direction.
> > Currently, each direction allocates its own hw counter, which is
> > inefficient as ct aging is managed per connection.
> > 
> > Share the counter that was allocated for the original direction
> > with the
> > reverse direction.
> 
> Yes, aging is done per connection, but the stats are not. With this
> patch, with netperf TCP_RR test, I get this: (mangled for
> readability)
> 
> # grep 172.0.0.4 /proc/net/nf_conntrack
> ipv4     2 tcp      6
>   src=172.0.0.3 dst=172.0.0.4 sport=34018 dport=33396 packets=3941992
> bytes=264113427
>   src=172.0.0.4 dst=172.0.0.3 sport=33396 dport=34018 packets=4
> bytes=218 [HW_OFFLOAD]
>   mark=0 secctx=system_u:object_r:unlabeled_t:s0 zone=0 use=3
> 
> while without it (594e31bceb + act_ct patch to enable it posted
> yesterday + revert), I get:
> 
> # grep 172.0.0.4 /proc/net/nf_conntrack
> ipv4     2 tcp      6
>   src=172.0.0.3 dst=172.0.0.4 sport=41856 dport=32776 packets=1876763
> bytes=125743084
>   src=172.0.0.4 dst=172.0.0.3 sport=32776 dport=41856 packets=1876761
> bytes=125742951 [HW_OFFLOAD]
>   mark=0 secctx=system_u:object_r:unlabeled_t:s0 zone=0 use=3
> 
> The same is visible on 'ovs-appctl dpctl/dump-conntrack -s' then.
> Summing both directions in one like this is at least very misleading.
> Seems this change was motivated only by hw resources constrains. That
> said, I'm wondering, can this change be reverted somehow?
> 
>   Marcelo

Hi Marcelo, thanks for the report, 
Sorry i am not familiar with this /procfs
Oz, Ariel, Roi, what is your take on this, it seems that we changed the
behavior of stats incorrectly.

Thanks,
Saeed.