DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: "Tan, Jianfeng" <jianfeng.tan@intel.com>, dev@dpdk.org
Cc: thomas@monjalon.net
Subject: Re: [dpdk-dev] [PATCH v3 3/5] bus/vdev: bus scan by multi-process channel
Date: Fri, 20 Apr 2018 16:19:32 +0100	[thread overview]
Message-ID: <9253a7b4-6826-f5da-7d6b-f48ba3e8ee3d@intel.com> (raw)
In-Reply-To: <9553c783-9d56-786e-87ed-6fc96982e251@intel.com>

On 20-Apr-18 3:28 PM, Tan, Jianfeng wrote:
> 
> 
> On 4/20/2018 4:41 PM, Burakov, Anatoly wrote:
>> On 19-Apr-18 5:50 PM, Jianfeng Tan wrote:
>>> To scan the vdevs in primary, we send request to primary process
>>> to obtain the names for vdevs.
>>>
>>> Only the name is shared from the primary. In probe(), the device
>>> driver is supposed to locate (or request more) the detail
>>> information from the primary.
>>>
>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>
>> <...>
>>
>>> +static int
>>> +vdev_action(const struct rte_mp_msg *mp_msg, const void *peer)
>>> +{
>>> +    struct rte_vdev_device *dev;
>>> +    struct rte_mp_msg mp_resp;
>>> +    struct vdev_param *ou = (struct vdev_param *)&mp_resp.param;
>>> +    const struct vdev_param *in = (const struct vdev_param 
>>> *)mp_msg->param;
>>> +    const char *devname;
>>> +    int num;
>>> +
>>> +    strcpy(mp_resp.name, "vdev");
>>> +    mp_resp.len_param = sizeof(*ou);
>>> +    mp_resp.num_fds = 0;
>>> +
>>> +    switch (in->type) {
>>> +    case VDEV_SCAN_REQ:
>>> +        ou->type = VDEV_SCAN_ONE;
>>> +        ou->num = 1;
>>> +        num = 0;
>>> +
>>> +        rte_spinlock_lock(&vdev_device_list_lock);
>>> +        TAILQ_FOREACH(dev, &vdev_device_list, next) {
>>> +            devname = rte_vdev_device_name(dev);
>>> +            if (strlen(devname) == 0)
>>> +                VDEV_LOG(INFO, "vdev with no name is not sent");
>>> +            VDEV_LOG(INFO, "send vdev, %s", devname);
>>> +            strncpy(ou->name, devname, RTE_DEV_NAME_MAX_LEN);
>>
>> Probably better use strlcpy as it always null-terminates.
> 
> Yep.
> 
>>
>>> +            if (rte_mp_sendmsg(&mp_resp) < 0)
>>> +                VDEV_LOG(ERR, "send vdev, %s, failed, %s",
>>> +                     devname, strerror(rte_errno));
>>> +            num++;
>>
>> Some comments on what is going on here (why are we sending messages in 
>> response? why multiple? who will receive these messages?) would be nice.
> 
> Yep, will explain that below.
> 
>> I have a sneaking suspicion that you could've packed the response into 
>> one single message, but i'm not completely sure what is going on here, 
>> so maybe what you have here makes sense...
> 
> What's happening here is that:
> 
> a. Secondary process sends a sync request to ask for vdev in primary.
> b. Primary process receives the request, and send vdevs one by one.
> c. Primary process sends back reply, which indicates how many vdevs are 
> sent.
> 
> The reason we don't pack all vdevs in the reply message is that, the 
> message length is RTE_MP_MAX_PARAM_LEN (256) in length. It's possible 
> that we cannot pack all vdevs in the single reply message.
> 

OK. How does secondary know which vdevs are new and which aren't? Does 
it even matter how many vdevs primary has sent? Correct me if i'm wrong, 
but it seems that you're only using sync request as kind of 
synchronization mechanism, and are not actually expecting any useful 
data in the reply. Which is OK, but in that case just don't bother 
sending any data in the reply in the first place :)

-- 
Thanks,
Anatoly

  reply	other threads:[~2018-04-20 15:19 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-04 15:30 [dpdk-dev] [PATCH 0/4] allow procinfo and pdump on eth vdev Jianfeng Tan
2018-03-04 15:30 ` [dpdk-dev] [PATCH 1/4] eal: bring forward multi-process channel init Jianfeng Tan
2018-03-04 15:30 ` [dpdk-dev] [PATCH 2/4] bus/vdev: bus scan by multi-process channel Jianfeng Tan
2018-03-05  9:36   ` Burakov, Anatoly
2018-03-06  0:50     ` Tan, Jianfeng
2018-03-07 14:00   ` Burakov, Anatoly
2018-03-12  3:22     ` Tan, Jianfeng
2018-03-04 15:30 ` [dpdk-dev] [PATCH 3/4] drivers/net: do not allocate rte_eth_dev_data privately Jianfeng Tan
2018-03-06  6:07   ` Matan Azrad
2018-03-06  8:55     ` Tan, Jianfeng
2018-03-07  6:00       ` Matan Azrad
2018-03-07  6:10         ` Matan Azrad
2018-03-12  3:40           ` Tan, Jianfeng
2018-03-04 15:30 ` [dpdk-dev] [PATCH 4/4] drivers/net: share vdev data to secondary process Jianfeng Tan
2018-04-19 16:50 ` [dpdk-dev] [PATCH v3 0/5] allow procinfo and pdump on eth vdev Jianfeng Tan
2018-04-19 16:50   ` [dpdk-dev] [PATCH v3 1/5] eal: bring forward multi-process channel init Jianfeng Tan
2018-04-20  8:16     ` Burakov, Anatoly
2018-04-20 14:08       ` Tan, Jianfeng
2018-04-19 16:50   ` [dpdk-dev] [PATCH v3 2/5] bus/vdev: add lock on vdev device list Jianfeng Tan
2018-04-20  8:26     ` Burakov, Anatoly
2018-04-20 14:19       ` Tan, Jianfeng
2018-04-20 15:16         ` Burakov, Anatoly
2018-04-20 15:23           ` Tan, Jianfeng
2018-04-19 16:50   ` [dpdk-dev] [PATCH v3 3/5] bus/vdev: bus scan by multi-process channel Jianfeng Tan
2018-04-20  8:41     ` Burakov, Anatoly
2018-04-20 14:28       ` Tan, Jianfeng
2018-04-20 15:19         ` Burakov, Anatoly [this message]
2018-04-20 15:32           ` Tan, Jianfeng
2018-04-20 15:39             ` Burakov, Anatoly
2018-04-19 16:50   ` [dpdk-dev] [PATCH v3 4/5] drivers/net: not use private eth dev data Jianfeng Tan
2018-04-19 16:50   ` [dpdk-dev] [PATCH v3 5/5] drivers/net: share vdev data to secondary process Jianfeng Tan
2018-04-20 16:57 ` [dpdk-dev] [PATCH v4 0/5] allow procinfo and pdump on eth vdev Jianfeng Tan
2018-04-20 16:57   ` [dpdk-dev] [PATCH v4 1/5] eal: bring forward multi-process channel init Jianfeng Tan
2018-04-20 16:57   ` [dpdk-dev] [PATCH v4 2/5] bus/vdev: add lock on vdev device list Jianfeng Tan
2018-04-23  9:47     ` Burakov, Anatoly
2018-04-20 16:57   ` [dpdk-dev] [PATCH v4 3/5] bus/vdev: bus scan by multi-process channel Jianfeng Tan
2018-04-23  9:54     ` Burakov, Anatoly
2018-04-24  5:22       ` Tan, Jianfeng
2018-04-20 16:57   ` [dpdk-dev] [PATCH v4 4/5] drivers/net: not use private eth dev data Jianfeng Tan
2018-04-20 16:57   ` [dpdk-dev] [PATCH v4 5/5] drivers/net: share vdev data to secondary process Jianfeng Tan
2018-04-24  5:51 ` [dpdk-dev] [PATCH v5 0/5] allow procinfo and pdump on eth vdev Jianfeng Tan
2018-04-24  5:51   ` [dpdk-dev] [PATCH v5 1/5] eal: bring forward multi-process channel init Jianfeng Tan
2018-04-24  5:51   ` [dpdk-dev] [PATCH v5 2/5] bus/vdev: add lock on vdev device list Jianfeng Tan
2018-04-24  5:51   ` [dpdk-dev] [PATCH v5 3/5] bus/vdev: bus scan by multi-process channel Jianfeng Tan
2018-04-24 10:09     ` Thomas Monjalon
2018-04-24  5:51   ` [dpdk-dev] [PATCH v5 4/5] drivers/net: not use private eth dev data Jianfeng Tan
2018-04-24  5:51   ` [dpdk-dev] [PATCH v5 5/5] drivers/net: share vdev data to secondary process Jianfeng Tan
2018-04-24 10:32   ` [dpdk-dev] [PATCH v5 0/5] allow procinfo and pdump on eth vdev Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9253a7b4-6826-f5da-7d6b-f48ba3e8ee3d@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=jianfeng.tan@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).