Date:   Mon, 19 Sep 2022 14:31:37 +0530
From:   sundeep subbaraya <sundeep.lkml@...il.com>
To:     Antoine Tenart <atenart@...nel.org>
Cc:     Saeed Mahameed <saeedm@...dia.com>,
        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

On Thu, Sep 15, 2022 at 1:32 PM Antoine Tenart <atenart@...nel.org> wrote:
>
> 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.
>
I could not think of advantages we have with two phase init for
software fallback.
As of now we will send the new driver to do all the init in the
prepare phase and
commit phase will return 0 always.

Thanks,
Sundeep


> 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

Powered by blists - more mailing lists