From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F3A9CA00C3; Tue, 18 Jan 2022 12:21:31 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BEC4B42707; Tue, 18 Jan 2022 12:21:31 +0100 (CET) Received: from mail-io1-f49.google.com (mail-io1-f49.google.com [209.85.166.49]) by mails.dpdk.org (Postfix) with ESMTP id 426834068E for ; Tue, 18 Jan 2022 12:21:30 +0100 (CET) Received: by mail-io1-f49.google.com with SMTP id n137so19003402iod.4 for ; Tue, 18 Jan 2022 03:21:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gWQrVoICX3tKFcSezbBzARDG41wtMtRRL8LaeU9deYA=; b=j/THqz36FTJdxYADVA3DTym1cXSJKdR3CoOvWaSU0+5ib6r2Vwkiopjt3ghOD0pmZY FDKB0ZiE1o1gqYSYN+vnRDUZBDM1QO7VTTR6ALkDnESP3bvotqubL/ua2G9wnPebH67t iZOqKBztjEapEST2OIj9T+r/2Em3bIi3z0DpbXRONWGrFqPBSE1keMyKSO48Jn/53pCX tgh0z0V1kfliqLFSfsVnxFs6bkc9NhVFhSeNONvCZxiaEuLWDIkLAOkdkbnaIbyPQKrN SL6h9pAXj5yh9xTG81WzUQwQrCblVXHm+2f7DYFXQhB82WbsW3ChkXJNbw1uShzY+TnK Sj+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gWQrVoICX3tKFcSezbBzARDG41wtMtRRL8LaeU9deYA=; b=kJkDtlnQa2qkU2KgqLI+u5IwWSjT/QUdqKvE9Dzpc7dNhPCi0l77xiacmYCGBF7+yc 5iC8BozyTP3DUiTKdthcLPvtDWpqWlNtPUrBAOp7uNCIHr4/ppAtEwN8bv2eDuWORk4E +kXjcYgZEGvdiFJaqpZPcIhivqacyS3jL1+nIRYEPj/m/0HvMNxNFRj4/BKS/QY8GBTP jCK9ercSIUzYZhUBUI5iLzNZzw/iM4sXEWDzKOCDbhhXfC5yUyZCXIpwZ3JeWCmgtOZ+ 5me4rst4V+UYjFDXVUX/E4tV3gsKJipJuqNXr45AkgH09dJhd8IVCmBjN+Y3wBMgJEi6 /ePg== X-Gm-Message-State: AOAM532i1swMXlmF7nOefg1tGJ2XCp9xrnsBAx7zWpmIBIIl8askIe0j Gq6zHvUFM8cxx8lFQ2042S72iupOf3h9UASi/7w= X-Google-Smtp-Source: ABdhPJyryFOpo23BVkbtPJyWTA/R07NcAmv0mX3UoaGVYZNzSIuDsnGVQwViimlT1LD+yI2XeRTiztFNbsFc7ed8zQo= X-Received: by 2002:a5e:c319:: with SMTP id a25mr2005015iok.94.1642504889628; Tue, 18 Jan 2022 03:21:29 -0800 (PST) MIME-Version: 1.0 References: <20211126041515.96259-1-kumaraparamesh92@gmail.com> <3166646.G96rZvMJ2N@thomas> <6aed79d2-be6c-7f30-a562-7acd69ebae57@intel.com> In-Reply-To: <6aed79d2-be6c-7f30-a562-7acd69ebae57@intel.com> From: kumaraparameshwaran rathinavel Date: Tue, 18 Jan 2022 16:51:18 +0530 Message-ID: Subject: Re: [PATCH] net/tap: Bug fix to populate fds in secondary process To: Ferruh Yigit Cc: Thomas Monjalon , "Wiles, Keith" , dev@dpdk.org, Kumara Parameshwaran , Andrew Rybchenko , David Marchand Content-Type: multipart/alternative; boundary="0000000000007cfe4f05d5d9785e" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000007cfe4f05d5d9785e Content-Type: text/plain; charset="UTF-8" Just wanted to bring it to your attention, In Mellanox driver there is a requirement to exchange fds between primary and secondary and similar usage is seen, the primary sends the port_id and the secondary refers to the rte_eth_devices in the driver, The functions are - mlx5_mp_secondary_handle in secondary - mlx5_mp_req_start_rxtx in primary which is invoked from mlx5_dev_start. In my implementation I have used the name and invoked get_port_by_name, I can also pass the port_id from the primary to make it uniform. So with similar usage in Mellanox is there a problem there as well on referring to the rte_eth_devices from the PMD ? On Tue, Jan 18, 2022 at 3:17 PM Ferruh Yigit wrote: > On 1/17/2022 6:33 PM, Thomas Monjalon wrote: > > 17/01/2022 19:28, Ferruh Yigit: > >>> + ret = rte_eth_dev_get_port_by_name(request_param->port_name, > &port_id); > >>> + if (ret) { > >>> + TAP_LOG(ERR, "Failed to get port id for %s", > >>> + request_param->port_name); > >>> + return -1; > >>> + } > >>> + dev = &rte_eth_devices[port_id]; > >> > >> Since this is not really related with your patch, I want to have a > separate thread for it. > >> > >> It is not good to access the 'rte_eth_devices' global variable directly > from a driver, that > >> is error prone. > >> > >> Btw, what 'peer' supposed to contain? > >> > >> It can be solved by adding an internal API, only for drivers to get > eth_dev from the name, > >> like: 'rte_eth_dev_get_by_name()'. > >> This way a few other usage can be converted to this API. > >> > >> @Thomas and @Andrew what do you think about the new API proposal? > > > > It looks similar to rte_eth_dev_get_port_by_name() which returns a > port_id. > > Exactly, but get eth_dev directly for drivers. For drivers no need to work > with port_id > handler, they can use eth_dev directly. > > Another solution can be an getter function for drivers, which gets port_id > and returns > the eth_dev. > > > It is a bit strange for an ethdev driver to not have access to its own > ethdev struct. > > Isn't there something broken in the logic? > > > > This is callback function between primary and secondary applications sync. > So port name > will be same for both, but eth_dev will be different and port_id may be > different. > Driver finds its own eth_dev from the shared port name. > > --0000000000007cfe4f05d5d9785e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Just wanted to bring it to your attention,
=
In Mellanox driver there is a requirement to exchange fds be= tween primary and secondary and similar usage is seen, the primary sends th= e port_id and the secondary refers to the rte_eth_devices in the driver,
The functions are
=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 - mlx5_mp_secondary_handle in secondary <= br>
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = - mlx5_mp_req_start_rxtx in primary which is invoked from mlx5_dev_start. <= br>

In my implementation I have used the name and = invoked get_port_by_name, I can also pass the port_id from the primary to m= ake it uniform. So with similar usage in Mellanox is there a problem there = as well on referring to the rte_eth_devices from the PMD ?
<= br>

On Tue, Jan 18, 2022 at 3:17 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
On 1/17/2022 6:33 PM, Thomas= Monjalon wrote:
> 17/01/2022 19:28, Ferruh Yigit:
>>> +=C2=A0 =C2=A0ret =3D rte_eth_dev_get_port_by_name(request_par= am->port_name, &port_id);
>>> +=C2=A0 =C2=A0if (ret) {
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0TAP_LOG(ERR, "F= ailed to get port id for %s",
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0request_param->port_name);
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -1;
>>> +=C2=A0 =C2=A0}
>>> +=C2=A0 =C2=A0dev =3D &rte_eth_devices[port_id];
>>
>> Since this is not really related with your patch, I want to have a= separate thread for it.
>>
>> It is not good to access the 'rte_eth_devices' global vari= able directly from a driver, that
>> is error prone.
>>
>> Btw, what 'peer' supposed to contain?
>>
>> It can be solved by adding an internal API, only for drivers to ge= t eth_dev from the name,
>> like: 'rte_eth_dev_get_by_name()'.
>> This way a few other usage can be converted to this API.
>>
>> @Thomas and @Andrew what do you think about the new API proposal?<= br> >
> It looks similar to rte_eth_dev_get_port_by_name() which returns a por= t_id.

Exactly, but get eth_dev directly for drivers. For drivers no need to work = with port_id
handler, they can use eth_dev directly.

Another solution can be an getter function for drivers, which gets port_id = and returns
the eth_dev.

> It is a bit strange for an ethdev driver to not have access to its own= ethdev struct.
> Isn't there something broken in the logic?
>

This is callback function between primary and secondary applications sync. = So port name
will be same for both, but eth_dev will be different and port_id may be dif= ferent.
Driver finds its own eth_dev from the shared port name.

--0000000000007cfe4f05d5d9785e--