netdev - RE: [PATCH net-next v3 2/3] net/macsec: Add MACsec skb extension Rx Data path 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR12MB5449F670E890436B0C454D2ABFB39@PH0PR12MB5449.namprd12.prod.outlook.com>
Date:   Tue, 21 Jun 2022 12:39:23 +0000
From:   Lior Nahmanson <liorna@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
CC:     "edumazet@...gle.com" <edumazet@...gle.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Raed Salem <raeds@...dia.com>, Jiri Pirko <jiri@...dia.com>,
        Ben Ben Ishay <benishay@...dia.com>
Subject: RE: [PATCH net-next v3 2/3] net/macsec: Add MACsec skb extension Rx
 Data path support

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, June 14, 2022 7:15 PM
> To: Paolo Abeni <pabeni@...hat.com>
> Cc: Lior Nahmanson <liorna@...dia.com>; edumazet@...gle.com;
> davem@...emloft.net; netdev@...r.kernel.org; Raed Salem
> <raeds@...dia.com>; Jiri Pirko <jiri@...dia.com>; Ben Ben Ishay
> <benishay@...dia.com>
> Subject: Re: [PATCH net-next v3 2/3] net/macsec: Add MACsec skb
> extension Rx Data path support
> 
> External email: Use caution opening links or attachments
> 
> 
> On Tue, 14 Jun 2022 15:55:13 +0200 Paolo Abeni wrote:
> > The main reason I suggested to look for the a possible alternative to
> > the skb extension is that the GRO stage is becoming bigger (and
> > slower) with any of such addition.
> >
> > The 'slow_gro' protects the common use-case from any additional
> > conditionals and intructions, I still have some concerns due to the
> > increased code size.
> >
> > This is not a reject, I'm mostly looking for a 2nd opinion.
> 
> Shooting from the hip a little bit, but macsec being a tightly bound L2 upper
> maybe metadata dst is a workable solution for carrying the sci and offload
> status between upper and lower? The range of values should be well known
> and limited.

Under the assumption that by skb_metadata you meant metadata_dst,
I think there are few reasons why i think is better to use skb extensions:

1. Unlike skb extension, the metadata_dst deallaction is handled directly by the allocator.
Since the sci and offloaded fields are shared between the MACsec driver and the offload driver
(in our case mlx5 driver), for Rx, the metadata_dst allocation is done in the mlx5 driver,
while the dealloction should be done in the MACsec driver.
This is undesired behavior.

2. medadata_dst is attached to the skb using skb_dst_set(), which set the slow_gro bit.
So there is no gain regarding slow_gro flow.

3. metadata_dst allocation require much more memory than needed for MACsec use case
(mainly because struct dst_entry which seems redundant for this case).