From: 方统浩50450 <fangtonghao@sangfor.com.cn>
To: Jeff Guo <jia.guo@intel.com>
Cc: thomas <thomas@monjalon.net>,
"ferruh.yigit" <ferruh.yigit@intel.com>,
arybchenko <arybchenko@solarflare.com>,
"cunming.liang" <cunming.liang@intel.com>, dev <dev@dpdk.org>,
stable <stable@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ethdev: fix secondary process change share memory
Date: Fri, 10 Jan 2020 15:53:28 +0800 (GMT+08:00) [thread overview]
Message-ID: <ADIAfQAQCB7e9vNw2AblL4pm.2.1578642808325.Hmail.fangtonghao@sangfor.com.cn> (raw)
In-Reply-To: <b748ade2-6061-55a0-a600-205b14e2ea7b@intel.com>
thanks for your correction
I will rewrite my commit log and send email again
方统浩50450
邮箱:fangtonghao@sangfor.com.cn
签名由 网易邮箱大师 定制
On 01/10/2020 15:30, Jeff Guo wrote:
hi, tonghao
On 1/9/2020 8:27 PM, Fang TongHao wrote:
> Hi all,I am from Sangfor Tech.I found a bug when using DPDK in
> multiprocess scenario.The secondary process enters
> "rte_eth_dev_pci_copy_info" function when initializing.Then it
> sets the value of struct "rte_eth_dev_data.dev_flags" to zero,
> but this struct is shared by primary process and secondary
> process, and the value change is unexpected by primary process.
> This may cause very serious damage.I think
> the secondary process should not enter "rte_eth_dev_pci_copy_info"
> function or changes the value of struct "rte_eth_dev_data.dev_flags"
> in shared memory.
> I fixed this bug by adding an if-statement to forbid the secondary
> process changing the above-mentioned value.
> Thansk, All.
i think the format of commit log should be refined to be more formal
like as below. what do you think?
ethdev: XXXXXXXXX
XXXXXXXX
> Signed-off-by: Fang TongHao <fangtonghao@sangfor.com.cn>
if it is a fix, suggest to add the line as "Fixes: XXXXXXXX ("ethdev:
XXXXXXX") to trace it.
> ---
> lib/librte_ethdev/rte_ethdev_pci.h | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev_pci.h b/lib/librte_ethdev/rte_ethdev_pci.h
> index ccdbb46ec..916de8a14 100644
> --- a/lib/librte_ethdev/rte_ethdev_pci.h
> +++ b/lib/librte_ethdev/rte_ethdev_pci.h
> @@ -59,15 +59,16 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
> }
>
> eth_dev->intr_handle = &pci_dev->intr_handle;
> -
> - eth_dev->data->dev_flags = 0;
> - if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
> - eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
> - if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_RMV)
> - eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_RMV;
> -
> - eth_dev->data->kdrv = pci_dev->kdrv;
> - eth_dev->data->numa_node = pci_dev->device.numa_node;
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> + eth_dev->data->dev_flags = 0;
> + if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
> + eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
> + if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_RMV)
> + eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_RMV;
> +
> + eth_dev->data->kdrv = pci_dev->kdrv;
> + eth_dev->data->numa_node = pci_dev->device.numa_node;
From the change log, you said that "rte_eth_dev_data.dev_flags" should
not be touched by secondary process, but you don't mention about
data->kdrv and data->numa_node, could you also explain them in the log
if they need to process as the same.
> + }
> }
>
> static inline int
next prev parent reply other threads:[~2020-01-10 7:53 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 12:27 [dpdk-dev] [PATCH] ethdev: fix secondary process change share memory Fang TongHao
2020-01-10 7:30 ` Jeff Guo
2020-01-10 7:53 ` 方统浩50450 [this message]
2020-01-13 5:03 ` [dpdk-dev] [PATCH v2] Fixes: ethdev: secondary process change shared memory Fang TongHao
2020-01-14 14:45 ` Ferruh Yigit
2020-01-15 6:49 ` 方统浩50450
2020-01-15 18:35 ` Ferruh Yigit
2020-01-15 20:43 ` Thomas Monjalon
2020-01-16 7:43 ` Andrew Rybchenko
2020-01-16 9:04 ` Ferruh Yigit
2020-01-16 11:35 ` 方统浩50450
2020-01-16 12:18 ` Ferruh Yigit
2020-01-17 2:11 ` 方统浩50450
2020-01-16 9:00 ` Ferruh Yigit
2020-01-17 2:08 ` [dpdk-dev] [PATCH v3] " Fang TongHao
2020-01-17 8:33 ` Andrew Rybchenko
2020-01-17 17:58 ` Ferruh Yigit
-- strict thread matches above, loose matches on Subject: below --
2020-01-09 3:14 [dpdk-dev] [PATCH] ethdev: fix secondary process change share memory Fang TongHao
2020-01-10 15:32 ` Stephen Hemminger
2020-01-13 3:02 ` 方统浩50450
2020-01-09 2:35 Fang TongHao
2020-01-15 10:30 ` Burakov, Anatoly
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=ADIAfQAQCB7e9vNw2AblL4pm.2.1578642808325.Hmail.fangtonghao@sangfor.com.cn \
--to=fangtonghao@sangfor.com.cn \
--cc=arybchenko@solarflare.com \
--cc=cunming.liang@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=jia.guo@intel.com \
--cc=stable@dpdk.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.