On 2022/10/25 2:45, Long Li wrote:
>>> +int mana_cfg_vport(struct mana_port_context *apc, u32
>> protection_dom_id,
>>> + u32 doorbell_pg_id)
>>> {
>>> struct mana_config_vport_resp resp = {};
>>> struct mana_config_vport_req req = {};
>>> int err;
>>>
>>> + /* This function is used to program the Ethernet port in the hardware
>>> + * table. It can be called from the Ethernet driver or the RDMA driver.
>>> + *
>>> + * For Ethernet usage, the hardware supports only one active user on
>> a
>>> + * physical port. The driver checks on the port usage before
>> programming
>>> + * the hardware when creating the RAW QP (RDMA driver) or
>> exposing the
>>> + * device to kernel NET layer (Ethernet driver).
>>> + *
>>> + * Because the RDMA driver doesn't know in advance which QP type
>> the
>>> + * user will create, it exposes the device with all its ports. The user
>>> + * may not be able to create RAW QP on a port if this port is already
>>> + * in used by the Ethernet driver from the kernel.
>>> + *
>>> + * This physical port limitation only applies to the RAW QP. For RC QP,
>>> + * the hardware doesn't have this limitation. The user can create RC
>>> + * QPs on a physical port up to the hardware limits independent of
>> the
>>> + * Ethernet usage on the same port.
>>> + */
>>> + mutex_lock(&apc->vport_mutex);
>>> + if (apc->vport_use_count > 0) {
>>> + mutex_unlock(&apc->vport_mutex);
>>> + return -EBUSY;
>>> + }
>>> + apc->vport_use_count++;
>>> + mutex_unlock(&apc->vport_mutex);
>>> +
>>> mana_gd_init_req_hdr(&req.hdr, MANA_CONFIG_VPORT_TX,
>>> sizeof(req), sizeof(resp));
>>> req.vport = apc->port_handle;
>>> @@ -679,9 +714,16 @@ static int mana_cfg_vport(struct
>>> mana_port_context *apc, u32 protection_dom_id,
>>>
>>> apc->tx_shortform_allowed = resp.short_form_allowed;
>>> apc->tx_vp_offset = resp.tx_vport_offset;
>>> +
>>> + netdev_info(apc->ndev, "Configured vPort %llu PD %u DB %u\n",
>>> + apc->port_handle, protection_dom_id, doorbell_pg_id);
>>> out:
>>> + if (err)
>>> + mana_uncfg_vport(apc);
>>
>> There seems to be a similar race between error handling here and the "apc-
>>> vport_use_count > 0" checking above as pointed out in v7.
>
> Thanks for looking into this.
>
> This is different to the locking bug in mana_ib_cfg_vport(). The vport sharing
> between Ethernet and RDMA is exclusive, not shared. If another driver tries
> to take the vport while it is being configured, it will fail immediately. It is by
Suppose the following steps:
1. Ethernet driver take the lock first and do a "apc->vport_use_count++", and
release the lock;
2. RDMA driver take the lock, do "apc->vport_use_count > 0" checking and return
-EBUSY;
3. mana_send_request() or mana_verify_resp_hdr() return error to Ethernet driver.
It seems that vport is left unused when above happens, if that is what you wanted?
> design to prevent possible deadlock.
I am not sure I understand the deadlock here.
>
