netdev - Re: [PATCH v9 net-next 13/14] pds_core: publish events to the clients

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: <20230413081410.2cbaf2a2@kernel.org>
Date:   Thu, 13 Apr 2023 08:14:10 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Shannon Nelson <shannon.nelson@....com>, brett.creeley@....com,
        davem@...emloft.net, netdev@...r.kernel.org, drivers@...sando.io,
        jiri@...nulli.us
Subject: Re: [PATCH v9 net-next 13/14] pds_core: publish events to the
 clients

On Thu, 13 Apr 2023 11:55:01 +0300 Leon Romanovsky wrote:
> > > > diff --git a/drivers/net/ethernet/amd/pds_core/adminq.c b/drivers/net/ethernet/amd/pds_core/adminq.c
> > > > index 25c7dd0d37e5..bb18ac1aabab 100644
> > > > --- a/drivers/net/ethernet/amd/pds_core/adminq.c
> > > > +++ b/drivers/net/ethernet/amd/pds_core/adminq.c
> > > > @@ -27,11 +27,13 @@ static int pdsc_process_notifyq(struct pdsc_qcq *qcq)
> > > >                case PDS_EVENT_LINK_CHANGE:
> > > >                        dev_info(pdsc->dev, "NotifyQ LINK_CHANGE ecode %d eid %lld\n",
> > > >                                 ecode, eid);
> > > > +                     pdsc_notify(PDS_EVENT_LINK_CHANGE, comp);  
> > > 
> > > Aren't you "resending" standard netdev event?
> > > It will be better to send only custom, specific to pds_core events,
> > > while leaving general ones to netdev.  
> > 
> > We have no netdev in pds_core, so we have to publish this to clients that
> > might have a netdev or some other need to know.  
> 
> I don't know netdev well enough if it is ok or not and maybe netdev will
> sent this LINK_CHANGE by itself anyway.
> 
> Jakub???

I actually prefer for the driver to distribute the event via its own
means than some random borderline proprietary stuff outside of netdev
using netdev events.

> > > We can argue if clients should get this event. Once reset is detected,
> > > the pds_core should close devices by deleting aux drivers.  
> > 
> > We can get a reset signal from the device when it has done a crash recovery
> > or when it is preparing to do an update, and this allows clients to quiesce
> > their operations when reset.state==0 and restart when they see
> > reset.state==1  
> 
> I don't think that it is safe behaviour from user POV. If FW resets
> itself under the hood, how can client be sure that nothing changes
> in its operation? Once FW reset occurs, it is much safer for the clients
> to reconfigure everything.

What's the argument exactly? We do have async resets including in mlx5,
grep for enable_remote_dev_reset