Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 10 Feb 2019 13:13:53 +0100
From:   Lukas Wunner <lukas@wunner.de>
To:     Mika Westerberg <mika.westerberg@linux.intel.com>
Cc:     linux-kernel@vger.kernel.org,
        Michael Jamet <michael.jamet@intel.com>,
        Yehezkel Bernat <YehezkelShB@gmail.com>,
        Andreas Noever <andreas.noever@gmail.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v2 12/28] thunderbolt: Add functions for allocating and
 releasing hop IDs
Message-ID: <20190210121353.fw7gj4pm7ce3flvf@wunner.de>
References: <20190206131738.43696-1-mika.westerberg@linux.intel.com>
 <20190206131738.43696-13-mika.westerberg@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190206131738.43696-13-mika.westerberg@linux.intel.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 06, 2019 at 04:17:22PM +0300, Mika Westerberg wrote:
> Each port has a separate path configuration space that is used for
> finding the next hop (switch) in the path. Hop ID is an index to this
> configuration space and hop IDs 0 - 7 are reserved.
> 
> In order to get next available hop ID for each direction we provide two
> pairs of helper functions that can be used to allocate and release hop
> IDs for a given port.
[...]
> +static int tb_port_alloc_hopid(struct tb_port *port, bool in, int min_hopid,
> +			       int max_hopid)
> +{
> +	int port_max_hopid;
> +	struct ida *ida;
> +
> +	if (in) {
> +		port_max_hopid = port->config.max_in_hop_id;
> +		ida = &port->in_hopids;
> +	} else {
> +		port_max_hopid = port->config.max_out_hop_id;
> +		ida = &port->out_hopids;
> +	}
> +
> +	/* Hop IDs 0-7 are reserved */
> +	if (min_hopid < 8)
> +		min_hopid = 8;
> +
> +	if (max_hopid < 0 || max_hopid > port_max_hopid)
> +		max_hopid = port_max_hopid;
> +
> +	return ida_simple_get(ida, min_hopid, max_hopid + 1, GFP_KERNEL);
> +}

If there are two Macs at the ends of the daisy-chain with Thunderbolt
devices in-between, the other Mac may already have established tunnels
to some of the devices and therefore has occupied hop entries in the
devices' path config space.  How do you ensure that you don't allocate
the same entries and overwrite the other Mac's hop entries, thereby
breaking its tunnels?  Because you're only allocating the hop entries
locally here.  Maybe there's some check in a later patch whether a
hop entry is already occupied, I'm not even halfway through this patch
bomb.

Thanks,

Lukas