netdev - Re: [PATCH net-next V2 07/17] net/mlx5: Add MACsec offload Tx command 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]

Date:   Thu, 15 Sep 2022 10:02:12 +0200
From:   Antoine Tenart <atenart@...nel.org>
To:     Saeed Mahameed <saeedm@...dia.com>,
        sundeep subbaraya <sundeep.lkml@...il.com>
Cc:     Saeed Mahameed <saeed@...nel.org>, liorna@...dia.com,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
        Tariq Toukan <tariqt@...dia.com>,
        Raed Salem <raeds@...dia.com>,
        Subbaraya Sundeep <sbhatta@...vell.com>, naveenm@...vell.com,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Geetha sowjanya <gakula@...vell.com>, andrew@...n.ch
Subject: Re: [PATCH net-next V2 07/17] net/mlx5: Add MACsec offload Tx command support

Quoting sundeep subbaraya (2022-09-15 07:20:05)
> On Thu, Sep 15, 2022 at 10:44 AM sundeep subbaraya
> <sundeep.lkml@...il.com> wrote:
> > On Thu, Sep 15, 2022 at 2:08 AM Saeed Mahameed <saeedm@...dia.com> wrote:
> > > On 14 Sep 20:09, sundeep subbaraya wrote:
> > > >Hi Saeed and Lior,
> > > >
> > > >Your mdo_ops can fail in the commit phase and do not validate input
> > > >params in the prepare phase.
> > > >Is that okay? I am developing MACSEC offload driver for Marvell CN10K
> > >
> > > It's ok since i think there is no reason to have the two steps system ! it
> > > doesn't make any sense to me ! prepare and commit are invoked consecutively
> > > one after the other for all mdo_ops and in every offload flow, with no extra
> > > step in between! so it's totally redundant.
> > >
> > > when i reviewed the series initially i was hesitant to check params
> > > on prepare step but i didn't see any reason since commit can still fail in
> > > the firmware anyways and there is nothing we can do about it !
> >
> > Yes, same with us where messages sent to the AF driver can fail in the
> > commit phase.
> >
> > > so we've decide to keep all the flows in one context for better readability
> > > and since the prepare/commit phases are confusing.

> > > >and I had to write some clever code
> > > >to honour that :). Please someone help me understand why two phase
> > > >init was needed for offloading.
> > >
> > > I don't know, let's ask the original author, Antoine ?

This two steps configuration wasn't part of the initial RFC and there
was a suggestion to go this way as it could allow the hardware to reject
some configurations and have an easier s/w fallback (w/ phase 1 error
being ignored but not phase 2). This mapped ~quite well to the first
device supporting this so I tried it. But looking back, this wasn't
discussed anymore nor improved and stayed this way. As you can see the
offloading doesn't fallback to s/w currently and I'd say if we want that
we should discuss it first; not sure if that is wanted after all.

If in the end all drivers ignore the first phase, or can't do much, it's
probably an indication the pattern doesn't fit well. We can still change
this, especially considering there are not that many drivers
implementing MACsec h/w offload for now. Now is a good time to discuss
this, thanks for raising that point.

[Adding Andrew who IIRC looked at the initial RFC; in case he wants to
add something].

Thanks,
Antoine