linux-kernel - Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent number

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: <20180530151547.GC30754@ziepe.ca>
Date:   Wed, 30 May 2018 09:15:47 -0600
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Håkon Bugge <haakon.bugge@...cle.com>
Cc:     Hans Westgaard Ry <hans.westgaard.ry@...cle.com>,
        Doug Ledford <dledford@...hat.com>,
        Jack Morgenstein <jackm@....mellanox.co.il>,
        Daniel Jurgens <danielj@...lanox.com>,
        Parav Pandit <parav@...lanox.com>,
        Pravin Shedge <pravin.shedge4linux@...il.com>,
        OFED mailing list <linux-rdma@...r.kernel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent
 number

On Wed, May 30, 2018 at 09:32:02AM +0200, Håkon Bugge wrote:
> 
> 
> > On 29 May 2018, at 18:40, Jason Gunthorpe <jgg@...pe.ca> wrote:
> > 
> > On Tue, May 29, 2018 at 06:16:14PM +0200, Håkon Bugge wrote:
> >> 
> >>> On 29 May 2018, at 17:49, Jason Gunthorpe <jgg@...pe.ca> wrote:
> >>> 
> >>> On Tue, May 29, 2018 at 09:38:08AM +0200, Hans Westgaard Ry wrote:
> >>>> The agent TID is a 64 bit value split in two dwords.  The least
> >>>> significant dword is the TID running counter. The most significant
> >>>> dword is the agent number. In the CX-3 shared port model, the mlx4
> >>>> driver uses the most significant byte of the agent number to store the
> >>>> slave number, making agent numbers greater and equal to 2^24 (3 bytes)
> >>>> unusable.
> >>> 
> >>> There is no reason for this to be an ida, just do something like
> >>> 
> >>> mad_agent_priv->agent.hi_tid = atomic_inc_return(&ib_mad_client_id) & mad_agent_priv->ib_dev->tid_mask;
> >>> 
> >>> And have the driver set tid_mask to 3 bytes of 0xFF
> >> 
> >> The issue is that some of the mad agents are long-lived, so you will
> >> wrap and use the same TID twice.
> > 
> > We already have that problem, and using ida is problematic because we
> > need to maximize the time between TID re-use, which ida isn't doing.
> 
> Initially, I thought that too, but consulted the spec which states:
> 
> C13-18.1.1: When initiating a new operation, MADHeader:TransactionID
> shall be set to such a value that within that MAD the combination of TID,
> SGID, and MgmtClass is different from that of any other currently executing
> operation. []
> 
> "currently executing operation" that is.

'currently executing operation' encompasses a wide range of behaviors
and it is not just 'completing the MAD at the local node'

You have to exceed all the timeouts before a single node can be
certain that the condition is satisfied.

This is why we just simply maximize the time between allocations, and
encourage the user providing the rest of the tid to do the same.

> > Preventing re-use seems like a seperate issue from limiting the range
> > to be compatible with mlx4.
> 
> Yes. But a v2 of Hans' patch using idr_alloc_cyclic() solves both issues.

That might work, but still have to get rid of the sysctl

Jason