- Linuxarm - mailweb.openeuler.org

Re: [PATCH V2 1/7] app/testpmd: fix forward lcores number when DCB test
by Li, Xiaoyun 27 Apr '21

27 Apr '21

> -----Original Message----- > From: Huisong Li <lihuisong(a)huawei.com> > Sent: Tuesday, April 20, 2021 15:32 > To: dev(a)dpdk.org > Cc: Yigit, Ferruh <ferruh.yigit(a)intel.com>; Li, Xiaoyun <xiaoyun.li(a)intel.com>; > linuxarm(a)openeuler.org; lihuisong(a)huawei.com > Subject: [PATCH V2 1/7] app/testpmd: fix forward lcores number when DCB test > > Currently, 'nb_fwd_lcores' value are both adjusted based on 'nb_fwd_streams' in > rss/simple/icmp_echo_fwd_config_setup. > But the operation is missing in dcb_fwd_config_setup, which may lead to a bad > behavior in which multiple polling threads operate on the same queue. This patch is OK. But commit log is redundant and confusing. The above is enough to explain what your patch is doing and can even be more simple. >In this > case, the device sends and receives packets, causing unexpected results. The > procedure is as follows: I don't understand what you're saying here. The commands you're showing is 8 nbcores dealing with 16 queues. So it's one thread dealing with multiple queues which doesn't have issues at all. Please remove the useless and confusing commands. > 1/ run testpmd with "--rxq=8 --txq=8" > 2/ port stop all > 3/ set nbcore 8 > 4/ port config 0 dcb vt off 4 pfc on > 5/ port config all rxq 16 > 6/ port config all txq 16 > 7/ port start all > 8/ set fwd mac > 9/ start > > For the DCB forwarding test, each core is assigned to each traffic class and each > core is assigned a multi-stream. > Therefore, 'nb_fwd_lcores' value needs to be adjusted based on 'total_tc_num' > in all forwarding ports. Please refer to the RSS fwd config fix patch to write your own commit log. Use simple and easy-understanding words to explain yourself. Below is the reference of RSS. commit 017d680a91fcf30da14a6d3a2f96d41f6dda3a0f Author: Pablo de Lara <pablo.de.lara.guarch(a)intel.com> Date: Mon Jun 27 23:35:19 2016 +0100 app/testpmd: limit number of forwarding cores Number of forwarding cores must be equal or less than number of forwarding streams, otherwise two cores would try to use a same queue on a port, which is not allowed. > > Fixes: 900550de04a7 ("app/testpmd: add dcb support") > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") > Cc: stable(a)dpdk.org > > Signed-off-by: Huisong Li <lihuisong(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> > --- > app/test-pmd/config.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index > ccb9bd3..03ee40c 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -2961,6 +2961,21 @@ rss_fwd_config_setup(void) > } > } > > +static uint16_t > +get_fwd_port_total_tc_num(void) > +{ > + struct rte_eth_dcb_info dcb_info; > + uint16_t total_tc_num = 0; > + unsigned int i; > + > + for (i = 0; i < nb_fwd_ports; i++) { > + (void)rte_eth_dev_get_dcb_info(fwd_ports_ids[i], &dcb_info); > + total_tc_num += dcb_info.nb_tcs; > + } > + > + return total_tc_num; > +} > + > /** > * For the DCB forwarding test, each core is assigned on each traffic class. > * > @@ -2980,12 +2995,16 @@ dcb_fwd_config_setup(void) > lcoreid_t lc_id; > uint16_t nb_rx_queue, nb_tx_queue; > uint16_t i, j, k, sm_id = 0; > + uint16_t total_tc_num; > uint8_t tc = 0; > > cur_fwd_config.nb_fwd_lcores = (lcoreid_t) nb_fwd_lcores; > cur_fwd_config.nb_fwd_ports = nb_fwd_ports; > cur_fwd_config.nb_fwd_streams = > (streamid_t) (nb_rxq * cur_fwd_config.nb_fwd_ports); > + total_tc_num = get_fwd_port_total_tc_num(); > + if (cur_fwd_config.nb_fwd_lcores > total_tc_num) > + cur_fwd_config.nb_fwd_lcores = total_tc_num; > > /* reinitialize forwarding streams */ > init_fwd_streams(); > -- > 2.7.4

1 0

Re: [PATCH] sched/fair: don't use waker's cpu if the waker of sync wake-up is interrupt
by Mike Galbraith 27 Apr '21

27 Apr '21

On Tue, 2021-04-27 at 14:37 +1200, Barry Song wrote: > > To fix this issue, this patch checks the waker of sync wake-up is a task > but not an interrupt. In this case, the waker will schedule out and give > CPU to wakee. That rash "the waker will schedule out" assumption, ie this really really is a synchronous wakeup, may be true in your particular case, but the sync hint is so overused as to be fairly meaningless. We've squabbled with it repeatedly over the years because of that. It really should either become more of a communication of intent than it currently is, or just go away. I'd argue for go away, simply because there is no way for the kernel to know that there isn't more work directly behind any particular wakeup. Take a pipe, does shoving some bits through a pipe mean you have no further use of your CPU? IFF you're doing nothing but playing ping- pong, sure it does, but how many real proggies have zero overlap among its threads of execution? The mere notion of threaded apps having no overlap *to be converted to throughput* is dainbramaged, which should be the death knell of the sync wakeup hint. Threaded apps can't do stuff like, oh, networking, which uses the sync hint heavily, without at least to some extent defeating the purpose of threading if we were to take the hint seriously. Heck, just look at the beauty (gak) of wake_wide(). It was born specifically to combat the pure-evil side of the sync wakeup hint. Bah, 'nuff "Danger Will Robinson, that thing will *eat you*!!" ;-) -Mike

2 4

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Alex Williamson 26 Apr '21

26 Apr '21

On Wed, 21 Apr 2021 17:59:02 +0800 liulongfang <liulongfang(a)huawei.com> wrote: > On 2021/4/21 6:04, Alex Williamson wrote: > > On Tue, 20 Apr 2021 09:59:57 -0300 > > Jason Gunthorpe <jgg(a)nvidia.com> wrote: > > > >> On Tue, Apr 20, 2021 at 08:50:12PM +0800, liulongfang wrote: > >>> On 2021/4/19 20:33, Jason Gunthorpe wrote: > >>>> On Mon, Apr 19, 2021 at 08:24:40PM +0800, liulongfang wrote: > >>>> > >>>>>> I'm also confused how this works securely at all, as a general rule a > >>>>>> VFIO PCI driver cannot access the MMIO memory of the function it is > >>>>>> planning to assign to the guest. There is a lot of danger that the > >>>>>> guest could access that MMIO space one way or another. > >>>>> > >>>>> VF's MMIO memory is divided into two parts, one is the guest part, > >>>>> and the other is the live migration part. They do not affect each other, > >>>>> so there is no security problem. > >>>> > >>>> AFAIK there are several scenarios where a guest can access this MMIO > >>>> memory using DMA even if it is not mapped into the guest for CPU > >>>> access. > >>>> > >>> The hardware divides VF's MMIO memory into two parts. The live migration > >>> driver in the host uses the live migration part, and the device driver in > >>> the guest uses the guest part. They obtain the address of VF's MMIO memory > >>> in their respective drivers, although these two parts The memory is > >>> continuous on the hardware device, but due to the needs of the drive function, > >>> they will not perform operations on another part of the memory, and the > >>> device hardware also independently responds to the operation commands of > >>> the two parts. > >> > >> It doesn't matter, the memory is still under the same PCI BDF and VFIO > >> supports scenarios where devices in the same IOMMU group are not > >> isolated from each other. > >> > >> This is why the granual of isolation is a PCI BDF - VFIO directly > >> blocks kernel drivers from attaching to PCI BDFs that are not > >> completely isolated from VFIO BDF. > >> > >> Bypassing this prevention and attaching a kernel driver directly to > >> the same BDF being exposed to the guest breaks that isolation model. > >> > >>> So, I still don't understand what the security risk you are talking about is, > >>> and what do you think the security design should look like? > >>> Can you elaborate on it? > >> > >> Each security domain must have its own PCI BDF. > >> > >> The migration control registers must be on a different VF from the VF > >> being plugged into a guest and the two VFs have to be in different > >> IOMMU groups to ensure they are isolated from each other. > > > > I think that's a solution, I don't know if it's the only solution. > > AIUI, the issue here is that we have a device specific kernel driver > > extending vfio-pci with migration support for this device by using an > > If the two parts of the MMIO region are split into different BAR spaces on > the device, the MMIO region of the business function is still placed in BAR2, > and the MMIO region of the live migration function is moved to BAR4. > Only BAR2 is mapped in the guest. only BAR4 is mapped in the host. > This can solve this security issue. The concern is really the "on the device" part rather than whether the resources are within the same BAR or not. We need to assume that a user driver can generate a DMA targeting any address in the system, including in this case the user driver could generate a DMA targeting this migration BAR. Ideally this would be routed upstream to the IOMMU where it would be blocked for lack of a translation entry. However, because this range resides on the same PCIe requester ID, it's logically more similar to a two-function device where the functions are considered non-isolated and are therefore exposed within the same IOMMU group. We would not allow a kernel driver for one of those functions and a userspace driver for the other. In this case those drivers are strongly related, but we still need to consider to what extent a malicious user driver can interfere with or exploit the kernel side driver. > > MMIO region of the same device. This is susceptible to DMA> manipulation by the user device. Whether that's a security issue or> not depends on how the user can break the device. If the scope is > > limited to breaking their own device, they can do that any number of > > ways and it's not very interesting. If the user can manipulate device > > state in order to trigger an exploit of the host-side kernel driver, > > that's obviously more of a problem. > > > > The other side of this is that if migration support can be implemented > > entirely within the VF using this portion of the device MMIO space, why > > do we need the host kernel to support this rather than implementing it > > in userspace? For example, QEMU could know about this device, > > manipulate the BAR size to expose only the operational portion of MMIO > > to the VM and use the remainder to support migration itself. I'm > > afraid that just like mdev, the vfio migration uAPI is going to be used > > as an excuse to create kernel drivers simply to be able to make use of > > that uAPI. I haven't looked at this driver to know if it has some > > When the accelerator device is designed to support the live migration > function, it is based on the uAPI of the migration region to realize the > live migration function, so the live migration function requires a driver > that connects to this uAPI. > Is this set of interfaces not open to us now? In your model, if both BARs are exposed to userspace and a device specific extension in QEMU claims the migration BAR rather than exposing it to the VM, could that driver mimic the migration region uAPI from userspace? For example, you don't need page pinning to interact with the IOMMU, you don't need resources beyond the scope of the endpoint device itself, and the migration info BAR is safe for userspace to manage? If so, then a kernel-based driver to expose a migration uAPI seems like it's only a risk for the kernel, ie. moving what could be a userspace driver into the kernel for the convenience of re-using a kernel uAPI. Thanks, Alex

2 1

Re: [PATCH V4] ethdev: add queue state when retrieve queue information
by Thomas Monjalon 26 Apr '21

26 Apr '21

16/04/2021 10:46, Lijun Ou: > Currently, upper-layer application could get queue state only > through pointers such as dev->data->tx_queue_state[queue_id], > this is not the recommended way to access it. So this patch > add get queue state when call rte_eth_rx_queue_info_get and > rte_eth_tx_queue_info_get API. > > Note: After add queue_state field, the 'struct rte_eth_rxq_info' size > remains 128B, and the 'struct rte_eth_txq_info' size remains 64B, so > it could be ABI compatible. [...] > --- a/doc/guides/rel_notes/release_21_05.rst > +++ b/doc/guides/rel_notes/release_21_05.rst > @@ -251,6 +251,12 @@ ABI Changes > function was already marked as internal in the API documentation for it, > and was not for use by external applications. > > +* Added new field ``queue_state`` to ``rte_eth_rxq_info`` structure > + to provide indicated rxq queue state. > + > +* Added new field ``queue_state`` to ``rte_eth_txq_info`` structure > + to provide indicated txq queue state. Not sure we should add a note here for additions which do not break ABI compatibility. It may be confusing.

4 6

Re: [PATCH 2/2] Documentation/ABI: Move the topology-related sysfs interface to the right place
by Song Bao Hua (Barry Song) 26 Apr '21

26 Apr '21

> -----Original Message----- > From: tiantao (H) > Sent: Monday, April 26, 2021 2:26 PM > To: Song Bao Hua (Barry Song) <song.bao.hua(a)hisilicon.com> > Cc: linuxarm(a)openeuler.org; tiantao (H) <tiantao6(a)hisilicon.com> > Subject: [PATCH 2/2] Documentation/ABI: Move the topology-related sysfs > interface to the right place > > Move the interface that exists under > /sys/devices/system/cpu/cpuX/topology/ to the more logical > Documentation/ABI/ file that can be properly parsed and > displayed to the user space > > Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> > Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> > --- > Documentation/ABI/stable/sysfs-devices-system-cpu | 149 > ++++++++++++++++++++++ > Documentation/admin-guide/cputopology.rst | 104 --------------- > 2 files changed, 149 insertions(+), 104 deletions(-) > > diff --git a/Documentation/ABI/stable/sysfs-devices-system-cpu > b/Documentation/ABI/stable/sysfs-devices-system-cpu > index 33c133e..7d0b23e 100644 > --- a/Documentation/ABI/stable/sysfs-devices-system-cpu > +++ b/Documentation/ABI/stable/sysfs-devices-system-cpu > @@ -1,3 +1,7 @@ > +Export CPU topology info via sysfs. Items (attributes) are similar > +to /proc/cpuinfo output of some architectures. They reside in > +/sys/devices/system/cpu/cpuX/topology/: > + > What: /sys/devices/system/cpu/dscr_default > Date: 13-May-2014 > KernelVersion: v3.15.0 > @@ -23,3 +27,148 @@ Description: Default value for the Data Stream Control > Register (DSCR) on > here). > If set by a process it will be inherited by child processes. > Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/physical_package_id > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: physical package id of cpuX. Typically corresponds to a > physical > + socket number, but the actual value is architecture and platform > + dependent. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/die_id > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: the CPU die ID of cpuX. Typically it is the hardware platform's > + identifier (rather than the kernel's). The actual value is > + architecture and platform dependent. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/core_id > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: the CPU core ID of cpuX. Typically it is the hardware platform's > + identifier (rather than the kernel's). The actual value is > + architecture and platform dependent. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/book_id > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: the book ID of cpuX. Typically it is the hardware platform's > + identifier (rather than the kernel's). The actual value is > + architecture and platform dependent. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/drawer_id > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: the drawer ID of cpuX. Typically it is the hardware platform's > + identifier (rather than the kernel's). The actual value is > + architecture and platform dependent. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/core_cpus > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: internal kernel map of CPUs within the same core. > + (deprecated name: "thread_siblings") > +Values: hexadecimal bitmask. > + > +What: /sys/devices/system/cpu/cpuX/topology/core_cpus_list > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: human-readable list of CPUs within the same core. > + The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > + so the tail of the string will be trimmed while its size is larger > + than PAGE_SIZE. > + (deprecated name: "thread_siblings_list"). > +Values: hexadecimal bitmask. No. this is a list not a mask. > + > +What: /sys/devices/system/cpu/cpuX/topology/package_cpus > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: internal kernel map of the CPUs sharing the same > physical_package_id. > + (deprecated name: "core_siblings"). > +Values: 64 bit unsigned integer (bit field) Id is unsigned integer. Here it is hexadecimal bitmask. > + > +What: /sys/devices/system/cpu/cpuX/topology/package_cpus_list > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: human-readable list of CPUs sharing the same > physical_package_id. > + The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > + so the tail of the string will be trimmed while its size is larger > + than PAGE_SIZE. > + (deprecated name: "core_siblings_list") > +Values: hexadecimal bitmask. As above. > + > +What: /sys/devices/system/cpu/cpuX/topology/die_cpus > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: internal kernel map of CPUs within the same die. > +Values: 64 bit unsigned integer (bit field) As above. > + > +What: /sys/devices/system/cpu/cpuX/topology/die_cpus_list > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: human-readable list of CPUs within the same die. > + The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > + so the tail of the string will be trimmed while its size is larger > + than PAGE_SIZE. > +Values: hexadecimal bitmask. As above. > + > +What: /sys/devices/system/cpu/cpuX/topology/book_siblings > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: internal kernel map of cpuX's hardware threads within the same > + book_id. > +Values: 64 bit unsigned integer (bit field) As above. > + > +What: /sys/devices/system/cpu/cpuX/topology/book_siblings_list > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: human-readable list of cpuX's hardware threads within the same > + book_id. > + The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > + so the tail of the string will be trimmed while its size is larger > + than PAGE_SIZE. For example here we should mark it "available to s390 only" > +Values: hexadecimal bitmask. > + > +What: /sys/devices/system/cpu/cpuX/topology/drawer_siblings > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: internal kernel map of cpuX's hardware threads within the same > + drawer_id. > +Values: 64 bit unsigned integer (bit field) > + > +What: /sys/devices/system/cpu/cpuX/topology/drawer_siblings_list > +Date: 19-Mar-2021 > +KernelVersion: v5.12 > +Contact: > +Description: human-readable list of cpuX's hardware threads within the same > + drawer_id. > + The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > + so the tail of the string will be trimmed while its size is larger > + than PAGE_SIZE. > +Values: hexadecimal bitmask. > + > +Architecture-neutral, drivers/base/topology.c, exports these attributes. > +However, the book and drawer related sysfs files will only be created if > +CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are selected, respectively. > + > +CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are currently only used on s390, > +where they reflect the cpu and cache hierarchy. These are not ABIs, better to be in original doc. Better to describe drawer ABI, book ABIs are only available for s390 in the ABI description. > diff --git a/Documentation/admin-guide/cputopology.rst > b/Documentation/admin-guide/cputopology.rst > index 4538d78..4672465 100644 > --- a/Documentation/admin-guide/cputopology.rst > +++ b/Documentation/admin-guide/cputopology.rst > @@ -2,110 +2,6 @@ > How CPU topology info is exported via sysfs > =========================================== > > -Export CPU topology info via sysfs. Items (attributes) are similar > -to /proc/cpuinfo output of some architectures. They reside in > -/sys/devices/system/cpu/cpuX/topology/: > - > -physical_package_id: > - > - physical package id of cpuX. Typically corresponds to a physical > - socket number, but the actual value is architecture and platform > - dependent. > - > -die_id: > - > - the CPU die ID of cpuX. Typically it is the hardware platform's > - identifier (rather than the kernel's). The actual value is > - architecture and platform dependent. > - > -core_id: > - > - the CPU core ID of cpuX. Typically it is the hardware platform's > - identifier (rather than the kernel's). The actual value is > - architecture and platform dependent. > - > -book_id: > - > - the book ID of cpuX. Typically it is the hardware platform's > - identifier (rather than the kernel's). The actual value is > - architecture and platform dependent. > - > -drawer_id: > - > - the drawer ID of cpuX. Typically it is the hardware platform's > - identifier (rather than the kernel's). The actual value is > - architecture and platform dependent. > - > -core_cpus: > - > - internal kernel map of CPUs within the same core. > - (deprecated name: "thread_siblings") > - > -core_cpus_list: > - > - human-readable list of CPUs within the same core. > - The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > - so the tail of the string will be trimmed while its size is larger > - than PAGE_SIZE. > - (deprecated name: "thread_siblings_list"); > - > -package_cpus: > - > - internal kernel map of the CPUs sharing the same physical_package_id. > - (deprecated name: "core_siblings") > - > -package_cpus_list: > - > - human-readable list of CPUs sharing the same physical_package_id. > - The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > - so the tail of the string will be trimmed while its size is larger > - than PAGE_SIZE. > - (deprecated name: "core_siblings_list") > - > -die_cpus: > - > - internal kernel map of CPUs within the same die. > - > -die_cpus_list: > - > - human-readable list of CPUs within the same die. > - The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > - so the tail of the string will be trimmed while its size is larger > - than PAGE_SIZE. > - > -book_siblings: > - > - internal kernel map of cpuX's hardware threads within the same > - book_id. > - > -book_siblings_list: > - > - human-readable list of cpuX's hardware threads within the same > - book_id. > - The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > - so the tail of the string will be trimmed while its size is larger > - than PAGE_SIZE. > - > -drawer_siblings: > - > - internal kernel map of cpuX's hardware threads within the same > - drawer_id. > - > -drawer_siblings_list: > - > - human-readable list of cpuX's hardware threads within the same > - drawer_id. > - The format is like 0-3, 8-11, 14,17. The maximum size is PAGE_SIZE, > - so the tail of the string will be trimmed while its size is larger > - than PAGE_SIZE. > - > -Architecture-neutral, drivers/base/topology.c, exports these attributes. > -However, the book and drawer related sysfs files will only be created if > -CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are selected, respectively. > - > -CONFIG_SCHED_BOOK and CONFIG_SCHED_DRAWER are currently only used on s390, > -where they reflect the cpu and cache hierarchy. > - > For an architecture to support this feature, it must define some of > these macros in include/asm-XXX/topology.h:: > > -- > 2.7.4

1 0

[PATCH 0/2] clarify and cleanup CPU and NUMA topology ABIs
by Tian Tao 26 Apr '21

26 Apr '21

patch #1: Move the interface that exists under /sys/devices/system/cpu/cpuX/topology/ to the more logical Documentation/ABI/ file that can be properly parsed and displayed to the user space. patch #2: clarify the overflow issue of sysfs pagebuf, and Move the presence of BUILD_BUG_ON to more sensible place. Tian Tao (2): CPU, NUMA topology ABIs: clarify and cleanup CPU and NUMA topology ABIs cpumask: clarify the overflow issue of sysfs pagebuf Documentation/ABI/stable/sysfs-devices-node | 4 + Documentation/ABI/stable/sysfs-devices-system-cpu | 149 ++++++++++++++++++++++ Documentation/admin-guide/cputopology.rst | 89 ------------- drivers/base/node.c | 3 - include/linux/cpumask.h | 9 +- 5 files changed, 161 insertions(+), 93 deletions(-) -- 2.7.4

1 2

Re: [dpdk-dev] [PATCH V5] ethdev: add queue state when retrieve queue information
by Kinsella, Ray 23 Apr '21

23 Apr '21

On 17/04/2021 04:09, Lijun Ou wrote: > Currently, upper-layer application could get queue state only > through pointers such as dev->data->tx_queue_state[queue_id], > this is not the recommended way to access it. So this patch > add get queue state when call rte_eth_rx_queue_info_get and > rte_eth_tx_queue_info_get API. > > Note: After add queue_state field, the 'struct rte_eth_rxq_info' size > remains 128B, and the 'struct rte_eth_txq_info' size remains 64B, so > it could be ABI compatible. > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> > Acked-by: Konstantin Ananyev <konstantin.ananyev(a)intel.com> > --- > V4->V5: > - Add acked-by > - add a note to the "New features" section to annouce the new feature. > > V3->V4: > - update libabigail.abignore for removing the CI warnings > > V2->V3: > - rewrite the commit log and delete the part Note > - rewrite tht comments for queue state > - move the queue_state definition locations > > V1->V2: > - move queue state defines to public file > --- > doc/guides/rel_notes/release_21_05.rst | 6 ++++++ > lib/librte_ethdev/ethdev_driver.h | 7 ------- > lib/librte_ethdev/rte_ethdev.c | 3 +++ > lib/librte_ethdev/rte_ethdev.h | 9 +++++++++ > 4 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst > index 58272e1..1ab3681 100644 > --- a/doc/guides/rel_notes/release_21_05.rst > +++ b/doc/guides/rel_notes/release_21_05.rst > @@ -81,6 +81,12 @@ New Features > representor=[[c#]pf#]sf# sf[0,2-1023] /* 1023 SFs. */ > representor=[c#]pf# c2pf[0,1] /* 2 PFs on controller 2. */ > > +* **Enhanced function for getting rxq/txq info ABI.** > + * Added new field ``queue_state`` to ``rte_eth_rxq_info`` structure to > + provide indicated rxq queue state. > + * Added new field ``queue_state`` to ``rte_eth_txq_info`` structure to > + provide indicated txq queue state. > + > * **Added support for meter PPS profile.** > > Currently meter algorithms only supports bytes per second(BPS). > diff --git a/lib/librte_ethdev/ethdev_driver.h b/lib/librte_ethdev/ethdev_driver.h > index 113129d..40e474a 100644 > --- a/lib/librte_ethdev/ethdev_driver.h > +++ b/lib/librte_ethdev/ethdev_driver.h > @@ -952,13 +952,6 @@ struct eth_dev_ops { > }; > > /** > - * RX/TX queue states > - */ > -#define RTE_ETH_QUEUE_STATE_STOPPED 0 > -#define RTE_ETH_QUEUE_STATE_STARTED 1 > -#define RTE_ETH_QUEUE_STATE_HAIRPIN 2 > - > -/** > * @internal > * Check if the selected Rx queue is hairpin queue. > * > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index c73d263..d5adf4f 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -5038,6 +5038,8 @@ rte_eth_rx_queue_info_get(uint16_t port_id, uint16_t queue_id, > > memset(qinfo, 0, sizeof(*qinfo)); > dev->dev_ops->rxq_info_get(dev, queue_id, qinfo); > + qinfo->queue_state = dev->data->rx_queue_state[queue_id]; > + > return 0; > } > > @@ -5078,6 +5080,7 @@ rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id, > > memset(qinfo, 0, sizeof(*qinfo)); > dev->dev_ops->txq_info_get(dev, queue_id, qinfo); > + qinfo->queue_state = dev->data->tx_queue_state[queue_id]; > > return 0; > } > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h > index 3b773b6..a0d01d2 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -1588,6 +1588,13 @@ struct rte_eth_dev_info { > }; > > /** > + * RX/TX queue states > + */ > +#define RTE_ETH_QUEUE_STATE_STOPPED 0 > +#define RTE_ETH_QUEUE_STATE_STARTED 1 > +#define RTE_ETH_QUEUE_STATE_HAIRPIN 2 > + > +/** > * Ethernet device RX queue information structure. > * Used to retrieve information about configured queue. > */ > @@ -1595,6 +1602,7 @@ struct rte_eth_rxq_info { > struct rte_mempool *mp; /**< mempool used by that queue. */ > struct rte_eth_rxconf conf; /**< queue config parameters. */ > uint8_t scattered_rx; /**< scattered packets RX supported. */ > + uint8_t queue_state; /**< one of RTE_ETH_QUEUE_STATE_*. */ Are we sure this is a false positive? - it is being added mid-structure on the rx-side at least. Shouldn't this be appended to the end - unless it is being sneaked into padding between fields. > uint16_t nb_desc; /**< configured number of RXDs. */ > uint16_t rx_buf_size; /**< hardware receive buffer size. */ > } __rte_cache_min_aligned; > @@ -1606,6 +1614,7 @@ struct rte_eth_rxq_info { > struct rte_eth_txq_info { > struct rte_eth_txconf conf; /**< queue config parameters. */ > uint16_t nb_desc; /**< configured number of TXDs. */ > + uint8_t queue_state; /**< one of RTE_ETH_QUEUE_STATE_*. */ > } __rte_cache_min_aligned; > > /* Generic Burst mode flag definition, values can be ORed. */ >

2 2

Re: [PATCH V6] app/test-pmd: support cleanup txq mbufs command
by Ferruh Yigit 21 Apr '21

21 Apr '21

On 4/21/2021 9:45 AM, Lijun Ou wrote: > From: Chengwen Feng <fengchengwen(a)huawei.com> > > This patch supports cleanup txq mbufs command: > port cleanup (port_id) txq (queue_id) (free_cnt) > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> Reviewed-by: Ferruh Yigit <ferruh.yigit(a)intel.com> Applied to dpdk-next-net/main, thanks.

1 0

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Jason Gunthorpe 21 Apr '21

21 Apr '21

On Tue, Apr 20, 2021 at 08:50:12PM +0800, liulongfang wrote: > On 2021/4/19 20:33, Jason Gunthorpe wrote: > > On Mon, Apr 19, 2021 at 08:24:40PM +0800, liulongfang wrote: > > > >>> I'm also confused how this works securely at all, as a general rule a > >>> VFIO PCI driver cannot access the MMIO memory of the function it is > >>> planning to assign to the guest. There is a lot of danger that the > >>> guest could access that MMIO space one way or another. > >> > >> VF's MMIO memory is divided into two parts, one is the guest part, > >> and the other is the live migration part. They do not affect each other, > >> so there is no security problem. > > > > AFAIK there are several scenarios where a guest can access this MMIO > > memory using DMA even if it is not mapped into the guest for CPU > > access. > > > The hardware divides VF's MMIO memory into two parts. The live migration > driver in the host uses the live migration part, and the device driver in > the guest uses the guest part. They obtain the address of VF's MMIO memory > in their respective drivers, although these two parts The memory is > continuous on the hardware device, but due to the needs of the drive function, > they will not perform operations on another part of the memory, and the > device hardware also independently responds to the operation commands of > the two parts. It doesn't matter, the memory is still under the same PCI BDF and VFIO supports scenarios where devices in the same IOMMU group are not isolated from each other. This is why the granual of isolation is a PCI BDF - VFIO directly blocks kernel drivers from attaching to PCI BDFs that are not completely isolated from VFIO BDF. Bypassing this prevention and attaching a kernel driver directly to the same BDF being exposed to the guest breaks that isolation model. > So, I still don't understand what the security risk you are talking about is, > and what do you think the security design should look like? > Can you elaborate on it? Each security domain must have its own PCI BDF. The migration control registers must be on a different VF from the VF being plugged into a guest and the two VFs have to be in different IOMMU groups to ensure they are isolated from each other. Jason

3 3

Re: [PATCH net v4 1/2] net: sched: fix packet stuck problem for lockless qdisc
by Michal Kubecek 21 Apr '21

21 Apr '21

On Wed, Apr 21, 2021 at 04:21:54PM +0800, Yunsheng Lin wrote: > > I tried using below shell to simulate your testcase: > > #!/bin/sh > > for((i=0; i<20; i++)) > do > taskset -c 0-31 netperf -t TCP_STREAM -H 192.168.100.2 -l 30 -- -m 1048576 > done > > And I got a quite stable result: 9413~9416 (10^6bits/sec) for 10G netdev. Perhaps try it without the taskset, in my test, there was only one connection. > > > > https://github.com/mkubecek/nperf > > > > It is still raw and a lot of features are missing but it can be already > > used for multithreaded TCP_STREAM and TCP_RR tests. In particular, the > > output above was with > > > > nperf -H 172.17.1.1 -l 30 -i 20 --exact -t TCP_STREAM -M 1 > > I tried your nperf too, unfortunately it does not seem to work on my > system(arm64), which exits very quickly and output the blow result: > > root@(none):/home# nperf -H 192.168.100.2 -l 30 -i 20 --exact -t TCP_STREAM -M 1 > server: 192.168.100.2, port 12543 > iterations: 20, threads: 1, test length: 30 > test: TCP_STREAM, message size: 1048576 > > 1 4.0 B/s, avg 4.0 B/s, mdev 0.0 B/s ( 0.0%) [...] Did you start nperfd on the other side? (It plays a role similar to netserver for netperf.) Few days ago I noticed that there is something wrong with error handling in case of failed connection but didn't get to fixing it yet. I'll try running some tests also on other architectures, including arm64 and s390x (to catch potential endinanity issues). Michal

1 0

Re: [PATCH V5] app/test-pmd: support cleanup txq mbufs command
by Ferruh Yigit 21 Apr '21

21 Apr '21

On 4/21/2021 9:09 AM, Lijun Ou wrote: > From: Chengwen Feng <fengchengwen(a)huawei.com> > > This patch supports cleanup txq mbufs command: > port cleanup (port_id) txq (queue_id) (free_cnt) > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> > --- > V4->V5: > - rewrite patch title > - define the new cmd. > - Fix the comments given by Ferruh.yigit > > V3->V4: > - revert the V3 scheme. > > V2->V3: > - The command implementation is changed so that the queuestate does > not depend on the command execution. > > V1->V2: > - use Tx instead of TX > - add note in doc > --- > app/test-pmd/cmdline.c | 85 +++++++++++++++++++++++++++++ > doc/guides/rel_notes/release_21_05.rst | 2 + > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 +++ > 3 files changed, 96 insertions(+) > Can you please update 'cmd_help_long_parsed' for the help string, this was in v4 and seems dropped during change.

1 0

Re: [Intel-wired-lan] [PATCH V2 net] ice: Re-organizes reqstd/avail {R, T}XQ check/code for efficiency+readability
by Paul Menzel 21 Apr '21

21 Apr '21

Dear Salil, Thank you very much for your patch. In the git commit message summary, could you please use imperative mood [1]? > Re-organize reqstd/avail {R, T}XQ check/code for efficiency+readability It’s a bit long though. Maybe: > Avoid unnecessary assignment with user specified {R,T}XQs Am 14.04.21 um 00:44 schrieb Salil Mehta: > If user has explicitly requested the number of {R,T}XQs, then it is > unnecessary to get the count of already available {R,T}XQs from the > PF avail_{r,t}xqs bitmap. This value will get overridden by user specified > value in any case. > > This patch does minor re-organization of the code for improving the flow > and readabiltiy. This scope of improvement was found during the review of readabil*it*y > the ICE driver code. > > FYI, I could not test this change due to unavailability of the hardware. > It would be helpful if somebody can test this patch and provide Tested-by > Tag. Many thanks! This should go outside the commit message (below the --- for example). > Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") Did you check the behavior before is actually a bug? Or is it just for the detection heuristic for commits to be applied to the stable series? > Cc: intel-wired-lan(a)lists.osuosl.org > Cc: Jeff Kirsher <jeffrey.t.kirsher(a)intel.com> > Signed-off-by: Salil Mehta <salil.mehta(a)huawei.com> > -- > Change V1->V2 > (*) Fixed the comments from Anthony Nguyen(Intel) > Link: https://lkml.org/lkml/2021/4/12/1997 > --- > drivers/net/ethernet/intel/ice/ice_lib.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c > index d13c7fc8fb0a..d77133d6baa7 100644 > --- a/drivers/net/ethernet/intel/ice/ice_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c > @@ -161,12 +161,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > > switch (vsi->type) { > case ICE_VSI_PF: > - vsi->alloc_txq = min3(pf->num_lan_msix, > - ice_get_avail_txq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_txq) { > vsi->alloc_txq = vsi->req_txq; > vsi->num_txq = vsi->req_txq; > + } else { > + vsi->alloc_txq = min3(pf->num_lan_msix, > + ice_get_avail_txq_count(pf), > + (u16)num_online_cpus()); > } I am curious, did you check the compiler actually creates different code, or did it notice the inefficiency by itself and optimized it already? > > pf->num_lan_tx = vsi->alloc_txq; > @@ -175,12 +176,13 @@ static void ice_vsi_set_num_qs(struct ice_vsi *vsi, u16 vf_id) > if (!test_bit(ICE_FLAG_RSS_ENA, pf->flags)) { > vsi->alloc_rxq = 1; > } else { > - vsi->alloc_rxq = min3(pf->num_lan_msix, > - ice_get_avail_rxq_count(pf), > - (u16)num_online_cpus()); > if (vsi->req_rxq) { > vsi->alloc_rxq = vsi->req_rxq; > vsi->num_rxq = vsi->req_rxq; > + } else { > + vsi->alloc_rxq = min3(pf->num_lan_msix, > + ice_get_avail_rxq_count(pf), > + (u16)num_online_cpus()); > } > } > Kind regards, Paul

2 3

Re: [PATCH net v4 1/2] net: sched: fix packet stuck problem for lockless qdisc
by Michal Kubecek 21 Apr '21

21 Apr '21

On Wed, Apr 21, 2021 at 09:52:40AM +0800, Yunsheng Lin wrote: > On 2021/4/21 4:34, Michal Kubecek wrote: > > However, I noticed something disturbing in the results of a simple > > 1-thread TCP_STREAM test (client sends data through a TCP connection to > > server using long writes, we measure the amount of data received by the > > server): > > > > server: 172.17.1.1, port 12543 > > iterations: 20, threads: 1, test length: 30 > > test: TCP_STREAM, message size: 1048576 > > > > 1 927403548.4 B/s, avg 927403548.4 B/s, mdev 0.0 B/s ( 0.0%) > > 2 1176317172.1 B/s, avg 1051860360.2 B/s, mdev 124456811.8 B/s ( 11.8%), confid. +/- 1581348251.3 B/s (150.3%) > > 3 927335837.8 B/s, avg 1010352186.1 B/s, mdev 117354970.3 B/s ( 11.6%), confid. +/- 357073677.2 B/s ( 35.3%) > > 4 1176728045.1 B/s, avg 1051946150.8 B/s, mdev 124576544.7 B/s ( 11.8%), confid. +/- 228863127.8 B/s ( 21.8%) > > 5 1176788216.3 B/s, avg 1076914563.9 B/s, mdev 122102985.3 B/s ( 11.3%), confid. +/- 169478943.5 B/s ( 15.7%) > > 6 1158167055.1 B/s, avg 1090456645.8 B/s, mdev 115504209.5 B/s ( 10.6%), confid. +/- 132805140.8 B/s ( 12.2%) > > 7 1176243474.4 B/s, avg 1102711907.0 B/s, mdev 111069717.1 B/s ( 10.1%), confid. +/- 110956822.2 B/s ( 10.1%) > > 8 1176771142.8 B/s, avg 1111969311.5 B/s, mdev 106744173.5 B/s ( 9.6%), confid. +/- 95417120.0 B/s ( 8.6%) > > 9 1176206364.6 B/s, avg 1119106761.8 B/s, mdev 102644185.2 B/s ( 9.2%), confid. +/- 83685200.5 B/s ( 7.5%) > > 10 1175888409.4 B/s, avg 1124784926.6 B/s, mdev 98855550.5 B/s ( 8.8%), confid. +/- 74537085.1 B/s ( 6.6%) > > 11 1176541407.6 B/s, avg 1129490061.2 B/s, mdev 95422224.8 B/s ( 8.4%), confid. +/- 67230249.7 B/s ( 6.0%) > > 12 934185352.8 B/s, avg 1113214668.9 B/s, mdev 106114984.5 B/s ( 9.5%), confid. +/- 70420712.5 B/s ( 6.3%) > > 13 1176550558.1 B/s, avg 1118086660.3 B/s, mdev 103339448.9 B/s ( 9.2%), confid. +/- 65002902.4 B/s ( 5.8%) > > 14 1176521808.8 B/s, avg 1122260599.5 B/s, mdev 100711151.3 B/s ( 9.0%), confid. +/- 60333655.0 B/s ( 5.4%) > > 15 1176744840.8 B/s, avg 1125892882.3 B/s, mdev 98240838.2 B/s ( 8.7%), confid. +/- 56319052.3 B/s ( 5.0%) > > 16 1176593778.5 B/s, avg 1129061688.3 B/s, mdev 95909740.8 B/s ( 8.5%), confid. +/- 52771633.5 B/s ( 4.7%) > > 17 1176583967.4 B/s, avg 1131857116.5 B/s, mdev 93715582.2 B/s ( 8.3%), confid. +/- 49669258.6 B/s ( 4.4%) > > 18 1176853301.8 B/s, avg 1134356904.5 B/s, mdev 91656530.2 B/s ( 8.1%), confid. +/- 46905244.8 B/s ( 4.1%) > > 19 1176592845.7 B/s, avg 1136579848.8 B/s, mdev 89709043.8 B/s ( 7.9%), confid. +/- 44424855.9 B/s ( 3.9%) > > 20 1176608117.3 B/s, avg 1138581262.2 B/s, mdev 87871692.6 B/s ( 7.7%), confid. +/- 42193098.5 B/s ( 3.7%) > > all avg 1138581262.2 B/s, mdev 87871692.6 B/s ( 7.7%), confid. +/- 42193098.5 B/s ( 3.7%) > > > > Each line shows result of one 30 second long test and average, mean > > deviation and 99% confidence interval half width through the iterations > > so far. While 17 iteration results are essentially the wire speed minus > > TCP overhead, iterations 1, 3 and 12 are more than 20% lower. As results > > of the same test on unpatched 5.12-rc7 are much more consistent (the > > lowest iteration result through the whole test was 1175939718.3 and the > > mean deviation only 276889.1 B/s), it doesn't seeem to be just a random > > fluctuation. > > I think I need to relearn the statistial math to understand the above > "99% confidence interval half width ":) An easy way to understand it is that if the last column shows 42 MB/s, it means that with 99% confidence (probability), the measured average is within 42 MB/s off the actual one. > But the problem do not seems related too much with "99% confidence > interval half width ", but with "mean deviation"? Mean deviation is a symptom here. What worries me is that most results show the same value (corresponding to fully saturated line) with very little variation, in some iterations (1, 3 and 12 here) we can suddenly see much lower value (by ~2.5 GB/s, i.e. 20-25%). And as each iteration runs the connection for 30 seconds, it cannot be just some short glitch. I managed to get tcpdump captures yesterday but they are huge even with "-s 128" (client ~5.6 GB, server ~9.0 GB) so that working with them is rather slow so I did not find anything interesting yet. > I tried using netperf, which seems only show throughput of 9415.06 > (10^6bits/sec) using 10G netdev. which tool did you used to show the > above number? 9415.06 * 10^6 b/s is 1176.9 * 10^6 B/s so it's about the same as the numbers above (the good ones, that is). As this was part of a longer test trying different thread counts from 1 to 128, I was using another utility I started writing recently: https://github.com/mkubecek/nperf It is still raw and a lot of features are missing but it can be already used for multithreaded TCP_STREAM and TCP_RR tests. In particular, the output above was with nperf -H 172.17.1.1 -l 30 -i 20 --exact -t TCP_STREAM -M 1 The results are with 1 thread so that they should be also reproducible with netperf too. But it needs to be repeated enough times, when I wanted to get the packet captures, I did 40 iterations and only two of them showed lower result. Michal

1 0

[RFC PATCH v5 0/4] scheduler: expose the topology of clusters and add cluster scheduler
by Barry Song 21 Apr '21

21 Apr '21

ARM64 server chip Kunpeng 920 has 6 or 8 clusters in each NUMA node, and each cluster has 4 cpus. All clusters share L3 cache data while each cluster has local L3 tag. On the other hand, each cluster will share some internal system bus. This means cache is much more affine inside one cluster than across clusters. +-----------------------------------+ +---------+ | +------+ +------+ +---------------------------+ | | | CPU0 | | cpu1 | | +-----------+ | | | +------+ +------+ | | | | | | +----+ L3 | | | | +------+ +------+ cluster | | tag | | | | | CPU2 | | CPU3 | | | | | | | +------+ +------+ | +-----------+ | | | | | | +-----------------------------------+ | | +-----------------------------------+ | | | +------+ +------+ +--------------------------+ | | | | | | | +-----------+ | | | +------+ +------+ | | | | | | | | L3 | | | | +------+ +------+ +----+ tag | | | | | | | | | | | | | | +------+ +------+ | +-----------+ | | | | | | +-----------------------------------+ | L3 | | data | +-----------------------------------+ | | | +------+ +------+ | +-----------+ | | | | | | | | | | | | | +------+ +------+ +----+ L3 | | | | | | tag | | | | +------+ +------+ | | | | | | | | | | ++ +-----------+ | | | +------+ +------+ |---------------------------+ | +-----------------------------------| | | +-----------------------------------| | | | +------+ +------+ +---------------------------+ | | | | | | | +-----------+ | | | +------+ +------+ | | | | | | +----+ L3 | | | | +------+ +------+ | | tag | | | | | | | | | | | | | | +------+ +------+ | +-----------+ | | | | | | +-----------------------------------+ | | +-----------------------------------+ | | | +------+ +------+ +--------------------------+ | | | | | | | +-----------+ | | | +------+ +------+ | | | | | There is a similar need for clustering in x86. Some x86 cores could share L2 caches that is similar to the cluster in Kupeng 920 (e.g. on Jacobsville there are 6 clusters of 4 Atom cores, each cluster sharing a separate L2, and 24 cores sharing L3). Having a sched_domain for clusters will bring two aspects of improvement: 1. spreading unrelated tasks among clusters, which decreases the contention of resources and improve the throughput. unrelated tasks might be put randomly without cluster sched_domain: +-------------------+ +-----------------+ | +----+ +----+ | | | | |task| |task| | | | | |1 | |2 | | | | | +----+ +----+ | | | | | | | | cluster1 | | cluster2 | +-------------------+ +-----------------+ but with cluster sched_domain, they are likely to spread due to LB: +-------------------+ +-----------------+ | +----+ | | +----+ | | |task| | | |task| | | |1 | | | |2 | | | +----+ | | +----+ | | | | | | cluster1 | | cluster2 | +-------------------+ +-----------------+ 2. gathering related tasks within a cluster, which improves the cache affinity of tasks talking with each other. Without cluster sched_domain, related tasks might be put randomly. In case task1-8 have relationship as below: Task1 wakes up task4 Task2 wakes up task5 Task3 wakes up task6 Task4 wakes up task7 With the tuning of select_idle_cpu() to scan local cluster first, those tasks might get a chance to be gathered like: +---------------------------+ +----------------------+ | +----+ +-----+ | | +----+ +-----+ | | |task| |task | | | |task| |task | | | |1 | | 4 | | | |2 | |5 | | | +----+ +-----+ | | +----+ +-----+ | | | | | | cluster1 | | cluster2 | | | | | | | | | | +-----+ +------+ | | +-----+ +------+ | | |task | | task | | | |task | |task | | | |3 | | 6 | | | |4 | |8 | | | +-----+ +------+ | | +-----+ +------+ | +---------------------------+ +----------------------+ Otherwise, the result might be: +---------------------------+ +----------------------+ | +----+ +-----+ | | +----+ +-----+ | | |task| |task | | | |task| |task | | | |1 | | 2 | | | |5 | |6 | | | +----+ +-----+ | | +----+ +-----+ | | | | | | cluster1 | | cluster2 | | | | | | | | | | +-----+ +------+ | | +-----+ +------+ | | |task | | task | | | |task | |task | | | |3 | | 4 | | | |7 | |8 | | | +-----+ +------+ | | +-----+ +------+ | +---------------------------+ +----------------------+ -v5: * split "add scheduler level for clusters" into two patches to evaluate the impact of spreading and gathering separately; * add a tracepoint of select_idle_cpu for debug purpose; add bcc script in commit log; * add cluster_id = -1 in reset_cpu_topology() * rebased to tip/sched/core -v4: * rebased to tip/sched/core with the latest unified code of select_idle_cpu * added Tim's patch for x86 Jacobsville * also added benchmark data of spreading unrelated tasks * avoided the iteration of sched_domain by moving to static_key(addressing Vincent's comment * used acpi_cpu_id for acpi_find_processor_node(addressing Masa's comment) Barry Song (2): scheduler: add scheduler level for clusters scheduler: scan idle cpu in cluster before scanning the whole llc Jonathan Cameron (1): topology: Represent clusters of CPUs within a die Tim Chen (1): scheduler: Add cluster scheduler level for x86 Documentation/admin-guide/cputopology.rst | 26 +++++++++++-- arch/arm64/Kconfig | 7 ++++ arch/arm64/kernel/topology.c | 2 + arch/x86/Kconfig | 8 ++++ arch/x86/include/asm/smp.h | 7 ++++ arch/x86/include/asm/topology.h | 1 + arch/x86/kernel/cpu/cacheinfo.c | 1 + arch/x86/kernel/cpu/common.c | 3 ++ arch/x86/kernel/smpboot.c | 43 ++++++++++++++++++++- drivers/acpi/pptt.c | 63 +++++++++++++++++++++++++++++++ drivers/base/arch_topology.c | 15 ++++++++ drivers/base/topology.c | 10 +++++ include/linux/acpi.h | 5 +++ include/linux/arch_topology.h | 5 +++ include/linux/sched/cluster.h | 19 ++++++++++ include/linux/sched/topology.h | 7 ++++ include/linux/topology.h | 13 +++++++ include/trace/events/sched.h | 22 +++++++++++ kernel/sched/core.c | 20 ++++++++++ kernel/sched/fair.c | 36 +++++++++++++++++- kernel/sched/sched.h | 1 + kernel/sched/topology.c | 5 +++ 22 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 include/linux/sched/cluster.h -- 1.8.3.1

5 14

Re: [PATCH] lib: bitmap: check the overflow of pagebuf
by Song Bao Hua (Barry Song) 21 Apr '21

21 Apr '21

> -----Original Message----- > From: tiantao (H) > Sent: Tuesday, April 20, 2021 11:49 PM > To: Song Bao Hua (Barry Song) <song.bao.hua(a)hisilicon.com>; Jonathan Cameron > <jonathan.cameron(a)huawei.com> > Cc: linuxarm(a)openeuler.org; tiantao (H) <tiantao6(a)hisilicon.com> > Subject: [PATCH] lib: bitmap: check the overflow of pagebuf > > Move the detection of pagebuf overflow from node_read_cpumap to the > more appropriate bitmap_print_to_pagebuf function. Other interfaces > call bitmap_print_to_pagebuf to print without pagebuf overflow due > to too many cpus. This is far from a good commit log to describe the background. > > Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> > --- > drivers/base/node.c | 3 --- > lib/bitmap.c | 3 +++ > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/node.c b/drivers/base/node.c > index 2c36f61d..e24530c3 100644 > --- a/drivers/base/node.c > +++ b/drivers/base/node.c > @@ -33,9 +33,6 @@ static ssize_t node_read_cpumap(struct device *dev, bool list, > char *buf) > cpumask_var_t mask; > struct node *node_dev = to_node(dev); > > - /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */ > - BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1)); > - > if (!alloc_cpumask_var(&mask, GFP_KERNEL)) > return 0; > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index 74ceb02..170fc48 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -482,6 +482,9 @@ int bitmap_print_to_pagebuf(bool list, char *buf, const > unsigned long *maskp, > { > ptrdiff_t len = PAGE_SIZE - offset_in_page(buf); > > + /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */ > + BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1)); > + I don't think it is the right place. expecting cpumap_print_to_pagebuf. BTW, it is not 2008/04/07 now, so just remove it. > return list ? scnprintf(buf, len, "%*pbl\n", nmaskbits, maskp) : > scnprintf(buf, len, "%*pb\n", nmaskbits, maskp); > } > -- > 2.7.4 I would prefer we send two patches as a series "clarify and cleanup CPU and NUMA topology ABIs" with a cover letter with the below as 1/2. 2/2 would be the patch moving the place of cpu topology ABI doc. From b32c0c00a187d4fe4c49d54d30650b0cacb2c351 Mon Sep 17 00:00:00 2001 From: Tian Tao <tiantao6(a)hisilicon.com> Date: Wed, 21 Apr 2021 14:36:11 +1200 Subject: [PATCH 1/2] CPU, NUMA topology ABIs: clarify the overflow issue of sysfs pagebuf Both numa node and cpu use cpu bitmap like 3,ffffffff to expose hardware topology. When cpu number is large, the page buffer of sysfs will over- flow. This doesn't really happen nowadays as the maximum NR_CPUS is 8196 for X86_64 and 4096 for ARM64 since 8196 * 9 / 32 = 2305 which is still smaller than 4KB page size. So this BUILD_BUG_ON() is pretty much preventing future problems similar with Y2K. On the other hand, it should be more sensible to move the guard to common code which can protect both cpu and numa: /sys/devices/system/cpu/cpu0/topology/die_cpus etc. /sys/devices/system/node/node0/cpumap etc. Topology bitmaps shouldn't be larger than PAGE_SIZE as lstopo and numactl depend on them. But other ABIs exposing cpu lists are not really used by common applications, so this patch also marks those lists could be trimmed. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> --- Documentation/ABI/stable/sysfs-devices-node | 5 ++++- Documentation/admin-guide/cputopology.rst | 15 +++++++++++++++ drivers/base/node.c | 3 --- include/linux/cpumask.h | 6 ++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/ABI/stable/sysfs-devices-node b/Documentation/ABI/stable/sysfs-devices-node index 484fc04bcc25..9832a17b2b15 100644 --- a/Documentation/ABI/stable/sysfs-devices-node +++ b/Documentation/ABI/stable/sysfs-devices-node @@ -47,7 +47,10 @@ What: /sys/devices/system/node/nodeX/cpulist Date: October 2002 Contact: Linux Memory Management list <linux-mm(a)kvack.org> Description: - The CPUs associated to the node. + The CPUs associated to the node. The format is like 0-3, + 8-11, 12-13. The maximum size is PAGE_SIZE, so the tail + of the string will be trimmed while its size is larger + than PAGE_SIZE. What: /sys/devices/system/node/nodeX/meminfo Date: October 2002 diff --git a/Documentation/admin-guide/cputopology.rst b/Documentation/admin-guide/cputopology.rst index b90dafcc8237..8fac776a5ffa 100644 --- a/Documentation/admin-guide/cputopology.rst +++ b/Documentation/admin-guide/cputopology.rst @@ -44,6 +44,9 @@ core_cpus: core_cpus_list: human-readable list of CPUs within the same core. + The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE, + so the tail of the string will be trimmed while its size is larger + than PAGE_SIZE. (deprecated name: "thread_siblings_list"); package_cpus: @@ -54,6 +57,9 @@ package_cpus: package_cpus_list: human-readable list of CPUs sharing the same physical_package_id. + The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE, + so the tail of the string will be trimmed while its size is larger + than PAGE_SIZE. (deprecated name: "core_siblings_list") die_cpus: @@ -63,6 +69,9 @@ die_cpus: die_cpus_list: human-readable list of CPUs within the same die. + The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE, + so the tail of the string will be trimmed while its size is larger + than PAGE_SIZE. book_siblings: @@ -73,6 +82,9 @@ book_siblings_list: human-readable list of cpuX's hardware threads within the same book_id. + The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE, + so the tail of the string will be trimmed while its size is larger + than PAGE_SIZE. drawer_siblings: @@ -83,6 +95,9 @@ drawer_siblings_list: human-readable list of cpuX's hardware threads within the same drawer_id. + The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE, + so the tail of the string will be trimmed while its size is larger + than PAGE_SIZE. Architecture-neutral, drivers/base/topology.c, exports these attributes. However, the book and drawer related sysfs files will only be created if diff --git a/drivers/base/node.c b/drivers/base/node.c index f449dbb2c746..50324d06bcd5 100644 --- a/drivers/base/node.c +++ b/drivers/base/node.c @@ -33,9 +33,6 @@ static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf) cpumask_var_t mask; struct node *node_dev = to_node(dev); - /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */ - BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1)); - if (!alloc_cpumask_var(&mask, GFP_KERNEL)) return 0; diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h index 383684e30f12..81f145e0c742 100644 --- a/include/linux/cpumask.h +++ b/include/linux/cpumask.h @@ -12,6 +12,7 @@ #include <linux/bitmap.h> #include <linux/atomic.h> #include <linux/bug.h> +#include <asm/page.h> /* Don't assign or return these: may not be this big! */ typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t; @@ -924,6 +925,11 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu) static inline ssize_t cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask) { + /* + * 32bits requires 9bytes: "ff,ffffffff", thus, too many CPUs will + * cause the overflow of sysfs pagebuf + */ + BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1)); return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask), nr_cpu_ids); } -- 2.25.1 Thanks Barry

1 0

Re: [RFC PATCH v5 4/4] scheduler: Add cluster scheduler level for x86
by Song Bao Hua (Barry Song) 21 Apr '21

21 Apr '21

> -----Original Message----- > From: Tim Chen [mailto:tim.c.chen@linux.intel.com] > Sent: Wednesday, April 21, 2021 6:32 AM > To: Song Bao Hua (Barry Song) <song.bao.hua(a)hisilicon.com>; > catalin.marinas(a)arm.com; will(a)kernel.org; rjw(a)rjwysocki.net; > vincent.guittot(a)linaro.org; bp(a)alien8.de; tglx(a)linutronix.de; > mingo(a)redhat.com; lenb(a)kernel.org; peterz(a)infradead.org; > dietmar.eggemann(a)arm.com; rostedt(a)goodmis.org; bsegall(a)google.com; > mgorman(a)suse.de > Cc: msys.mizuma(a)gmail.com; valentin.schneider(a)arm.com; > gregkh(a)linuxfoundation.org; Jonathan Cameron <jonathan.cameron(a)huawei.com>; > juri.lelli(a)redhat.com; mark.rutland(a)arm.com; sudeep.holla(a)arm.com; > aubrey.li(a)linux.intel.com; linux-arm-kernel(a)lists.infradead.org; > linux-kernel(a)vger.kernel.org; linux-acpi(a)vger.kernel.org; x86(a)kernel.org; > xuwei (O) <xuwei5(a)huawei.com>; Zengtao (B) <prime.zeng(a)hisilicon.com>; > guodong.xu(a)linaro.org; yangyicong <yangyicong(a)huawei.com>; Liguozhu (Kenneth) > <liguozhu(a)hisilicon.com>; linuxarm(a)openeuler.org; hpa(a)zytor.com > Subject: Re: [RFC PATCH v5 4/4] scheduler: Add cluster scheduler level for x86 > > > > On 3/23/21 4:21 PM, Song Bao Hua (Barry Song) wrote: > > >> > >> On 3/18/21 9:16 PM, Barry Song wrote: > >>> From: Tim Chen <tim.c.chen(a)linux.intel.com> > >>> > >>> There are x86 CPU architectures (e.g. Jacobsville) where L2 cahce > >>> is shared among a cluster of cores instead of being exclusive > >>> to one single core. > >>> > >>> To prevent oversubscription of L2 cache, load should be > >>> balanced between such L2 clusters, especially for tasks with > >>> no shared data. > >>> > >>> Also with cluster scheduling policy where tasks are woken up > >>> in the same L2 cluster, we will benefit from keeping tasks > >>> related to each other and likely sharing data in the same L2 > >>> cluster. > >>> > >>> Add CPU masks of CPUs sharing the L2 cache so we can build such > >>> L2 cluster scheduler domain. > >>> > >>> Signed-off-by: Tim Chen <tim.c.chen(a)linux.intel.com> > >>> Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> > >> > >> > >> Barry, > >> > >> Can you also add this chunk to the patch. > >> Thanks. > > > > Sure, Tim, Thanks. I'll put that into patch 4/4 in v6. > > > > Barry, > > This chunk will also need to be added to return cluster id for x86. > Please add it in your next rev. Yes. Thanks. I'll put this in either RFC v7 or Patch v1. For spreading path, things are much easier, though packing path is quite tricky. But It seems RFC v6 has been quite close to what we want to achieve to pack related tasks by scanning cluster for tasks within same NUMA: https://lore.kernel.org/lkml/20210420001844.9116-1-song.bao.hua@hisilicon.c… If couples have been already in same LLC(numa), scanning clusters will gather them further. If they are running in different NUMA nodes, the original scanning LLC will move them to the same node, after that, scanning cluster might put them closer to each other. it seems it is kind of the two-level packing Dietmar has suggested. So perhaps we won't have RFC v7, I will probably send patch v1 afterwards. > > Thanks. > > Tim > > --- > > diff --git a/arch/x86/include/asm/topology.h > b/arch/x86/include/asm/topology.h > index 800fa48c9fcd..2548d824f103 100644 > --- a/arch/x86/include/asm/topology.h > +++ b/arch/x86/include/asm/topology.h > @@ -109,6 +109,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu); > #define topology_physical_package_id(cpu) (cpu_data(cpu).phys_proc_id) > #define topology_logical_die_id(cpu) (cpu_data(cpu).logical_die_id) > #define topology_die_id(cpu) (cpu_data(cpu).cpu_die_id) > +#define topology_cluster_id(cpu) (per_cpu(cpu_l2c_id, cpu)) > #define topology_core_id(cpu) (cpu_data(cpu).cpu_core_id) > > extern unsigned int __max_die_per_package; Thanks Barry

1 0

[PATCH net v4 0/2] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 21 Apr '21

21 Apr '21

This patchset fixes the packet stuck problem mentioned in [1]. Patch 1: Add STATE_MISSED flag to fix packet stuck problem. Patch 2: Fix a tx_action rescheduling problem after STATE_MISSED flag is added in patch 1. V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. [1]. https://lkml.org/lkml/2019/10/9/42 Yunsheng Lin (2): net: sched: fix packet stuck problem for lockless qdisc net: sched: fix endless tx action reschedule during deactivation include/net/pkt_sched.h | 7 +------ include/net/sch_generic.h | 37 ++++++++++++++++++++++++++++++++++++- net/core/dev.c | 26 ++++++++++++++++++++++---- net/sched/sch_generic.c | 16 +++++++++++++++- 4 files changed, 74 insertions(+), 12 deletions(-) -- 2.7.4

2 6

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Jason Gunthorpe 20 Apr '21

20 Apr '21

On Tue, Apr 20, 2021 at 09:28:46PM +0800, liulongfang wrote: > >> So, I still don't understand what the security risk you are talking about is, > >> and what do you think the security design should look like? > >> Can you elaborate on it? > > > > Each security domain must have its own PCI BDF. > > > The basic unit to perform the live migration function is the VF, and the basic > function of the VF is the business function of the device. If the live migration > function and the business function are completely separated, and they are bound > to their respective VFs, it will result in the ability to execute the business > in the guest A functional VF cannot perform live migration, and a VF with a live > migration function cannot perform business functions in the guest. > > In fact, the scenario that requires live migration is to migrate its business > functions to the VFs of other hosts when the VF's business functions are heavily > loaded. > This usage scenario itself requires that the VF needs to have these two functions > at the same time. Other vendors are managing, it is not an unsolvable problem. I think these patches can't advance in any form without somehow addressing the isolation issue. Jason

1 0

[PATCH net v3] net: sched: fix packet stuck problem for lockless qdisc
by Yunsheng Lin 20 Apr '21

20 Apr '21

Lockless qdisc has below concurrent problem: cpu0 cpu1 . . q->enqueue . . . qdisc_run_begin() . . . dequeue_skb() . . . sch_direct_xmit() . . . . q->enqueue . qdisc_run_begin() . return and do nothing . . qdisc_run_end() . cpu1 enqueue a skb without calling __qdisc_run() because cpu0 has not released the lock yet and spin_trylock() return false for cpu1 in qdisc_run_begin(), and cpu0 do not see the skb enqueued by cpu1 when calling dequeue_skb() because cpu1 may enqueue the skb after cpu0 calling dequeue_skb() and before cpu0 calling qdisc_run_end(). Lockless qdisc has below another concurrent problem when tx_action is involved: cpu0(serving tx_action) cpu1 cpu2 . . . . q->enqueue . . qdisc_run_begin() . . dequeue_skb() . . . q->enqueue . . . . sch_direct_xmit() . . . qdisc_run_begin() . . return and do nothing . . . clear __QDISC_STATE_SCHED . . qdisc_run_begin() . . return and do nothing . . . . . . qdisc_run_end() . This patch fixes the above data race by: 1. Get the flag before doing spin_trylock(). 2. If the first spin_trylock() return false and the flag is not set before the first spin_trylock(), Set the flag and retry another spin_trylock() in case other CPU may not see the new flag after it releases the lock. 3. reschedule if the flags is set after the lock is released at the end of qdisc_run_end(). For tx_action case, the flags is also set when cpu1 is at the end if qdisc_run_end(), so tx_action will be rescheduled again to dequeue the skb enqueued by cpu2. Only clear the flag before retrying a dequeuing when dequeuing returns NULL in order to reduce the overhead of the above double spin_trylock() and __netif_schedule() calling. The performance impact of this patch, tested using pktgen and dummy netdev with pfifo_fast qdisc attached: threads without+this_patch with+this_patch delta 1 2.61Mpps 2.60Mpps -0.3% 2 3.97Mpps 3.82Mpps -3.7% 4 5.62Mpps 5.59Mpps -0.5% 8 2.78Mpps 2.77Mpps -0.3% 16 2.22Mpps 2.22Mpps -0.0% Fixes: 6b3ba9146fe6 ("net: sched: allow qdiscs to handle locking") Signed-off-by: Yunsheng Lin <linyunsheng(a)huawei.com> --- V3: fix a compile error and a few comment typo, remove the __QDISC_STATE_DEACTIVATED checking, and update the performance data. V2: Avoid the overhead of fixing the data race as much as possible. --- include/net/sch_generic.h | 38 +++++++++++++++++++++++++++++++++++++- net/sched/sch_generic.c | 12 ++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index f7a6e14..e3f46eb 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -36,6 +36,7 @@ struct qdisc_rate_table { enum qdisc_state_t { __QDISC_STATE_SCHED, __QDISC_STATE_DEACTIVATED, + __QDISC_STATE_NEED_RESCHEDULE, }; struct qdisc_size_table { @@ -159,8 +160,38 @@ static inline bool qdisc_is_empty(const struct Qdisc *qdisc) static inline bool qdisc_run_begin(struct Qdisc *qdisc) { if (qdisc->flags & TCQ_F_NOLOCK) { + bool dont_retry = test_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state); + + if (spin_trylock(&qdisc->seqlock)) + goto nolock_empty; + + /* If the flag is set before doing the spin_trylock() and + * the above spin_trylock() return false, it means other cpu + * holding the lock will do dequeuing for us, or it wil see + * the flag set after releasing lock and reschedule the + * net_tx_action() to do the dequeuing. + */ + if (dont_retry) + return false; + + /* We could do set_bit() before the first spin_trylock(), + * and avoid doing second spin_trylock() completely, then + * we could have multi cpus doing the set_bit(). Here use + * dont_retry to avoid doing the set_bit() and the second + * spin_trylock(), which has 5% performance improvement than + * doing the set_bit() before the first spin_trylock(). + */ + set_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state); + + /* Retry again in case other CPU may not see the new flag + * after it releases the lock at the end of qdisc_run_end(). + */ if (!spin_trylock(&qdisc->seqlock)) return false; + +nolock_empty: WRITE_ONCE(qdisc->empty, false); } else if (qdisc_is_running(qdisc)) { return false; @@ -176,8 +207,13 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) static inline void qdisc_run_end(struct Qdisc *qdisc) { write_seqcount_end(&qdisc->running); - if (qdisc->flags & TCQ_F_NOLOCK) + if (qdisc->flags & TCQ_F_NOLOCK) { spin_unlock(&qdisc->seqlock); + + if (unlikely(test_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state))) + __netif_schedule(qdisc); + } } static inline bool qdisc_may_bulk(const struct Qdisc *qdisc) diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 44991ea..4953430 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -640,8 +640,10 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc) { struct pfifo_fast_priv *priv = qdisc_priv(qdisc); struct sk_buff *skb = NULL; + bool need_retry = true; int band; +retry: for (band = 0; band < PFIFO_FAST_BANDS && !skb; band++) { struct skb_array *q = band2list(priv, band); @@ -652,6 +654,16 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc) } if (likely(skb)) { qdisc_update_stats_at_dequeue(qdisc, skb); + } else if (need_retry && + test_and_clear_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state)) { + /* do another enqueuing after clearing the flag to + * avoid calling __netif_schedule(). + */ + smp_mb__after_atomic(); + need_retry = false; + + goto retry; } else { WRITE_ONCE(qdisc->empty, true); } -- 2.7.4

2 6

[RFC PATCH v6 4/4] scheduler: Add cluster scheduler level for x86
by Barry Song 20 Apr '21

20 Apr '21

From: Tim Chen <tim.c.chen(a)linux.intel.com> There are x86 CPU architectures (e.g. Jacobsville) where L2 cahce is shared among a cluster of cores instead of being exclusive to one single core. To prevent oversubscription of L2 cache, load should be balanced between such L2 clusters, especially for tasks with no shared data. Also with cluster scheduling policy where tasks are woken up in the same L2 cluster, we will benefit from keeping tasks related to each other and likely sharing data in the same L2 cluster. Add CPU masks of CPUs sharing the L2 cache so we can build such L2 cluster scheduler domain. Signed-off-by: Tim Chen <tim.c.chen(a)linux.intel.com> Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> --- -v6: * added topology_cluster_cpumask() for x86, code provided by Tim. arch/x86/Kconfig | 8 ++++++++ arch/x86/include/asm/smp.h | 7 +++++++ arch/x86/include/asm/topology.h | 2 ++ arch/x86/kernel/cpu/cacheinfo.c | 1 + arch/x86/kernel/cpu/common.c | 3 +++ arch/x86/kernel/smpboot.c | 43 ++++++++++++++++++++++++++++++++++++++++- 6 files changed, 63 insertions(+), 1 deletion(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 2792879..d597de2 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1002,6 +1002,14 @@ config NR_CPUS This is purely to save memory: each supported CPU adds about 8KB to the kernel image. +config SCHED_CLUSTER + bool "Cluster scheduler support" + default n + help + Cluster scheduler support improves the CPU scheduler's decision + making when dealing with machines that have clusters of CPUs + sharing L2 cache. If unsure say N here. + config SCHED_SMT def_bool y if SMP diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h index c0538f8..9cbc4ae 100644 --- a/arch/x86/include/asm/smp.h +++ b/arch/x86/include/asm/smp.h @@ -16,7 +16,9 @@ DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_die_map); /* cpus sharing the last level cache: */ DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_llc_shared_map); +DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_l2c_shared_map); DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id); +DECLARE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id); DECLARE_PER_CPU_READ_MOSTLY(int, cpu_number); static inline struct cpumask *cpu_llc_shared_mask(int cpu) @@ -24,6 +26,11 @@ static inline struct cpumask *cpu_llc_shared_mask(int cpu) return per_cpu(cpu_llc_shared_map, cpu); } +static inline struct cpumask *cpu_l2c_shared_mask(int cpu) +{ + return per_cpu(cpu_l2c_shared_map, cpu); +} + DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid); DECLARE_EARLY_PER_CPU_READ_MOSTLY(u32, x86_cpu_to_acpiid); DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid); diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index 9239399..800fa48 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -103,6 +103,7 @@ static inline void setup_node_to_cpumask_map(void) { } #include <asm-generic/topology.h> extern const struct cpumask *cpu_coregroup_mask(int cpu); +extern const struct cpumask *cpu_clustergroup_mask(int cpu); #define topology_logical_package_id(cpu) (cpu_data(cpu).logical_proc_id) #define topology_physical_package_id(cpu) (cpu_data(cpu).phys_proc_id) @@ -114,6 +115,7 @@ static inline void setup_node_to_cpumask_map(void) { } #ifdef CONFIG_SMP #define topology_die_cpumask(cpu) (per_cpu(cpu_die_map, cpu)) +#define topology_cluster_cpumask(cpu) (cpu_clustergroup_mask(cpu)) #define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu)) #define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu)) diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c index 3ca9be4..0d03a71 100644 --- a/arch/x86/kernel/cpu/cacheinfo.c +++ b/arch/x86/kernel/cpu/cacheinfo.c @@ -846,6 +846,7 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c) l2 = new_l2; #ifdef CONFIG_SMP per_cpu(cpu_llc_id, cpu) = l2_id; + per_cpu(cpu_l2c_id, cpu) = l2_id; #endif } diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index ab640ab..0ba282d 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -78,6 +78,9 @@ /* Last level cache ID of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_llc_id) = BAD_APICID; +/* L2 cache ID of each logical CPU */ +DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; + /* correctly size the local cpu masks */ void __init setup_cpu_local_masks(void) { diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 02813a7..c85ffa8 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -101,6 +101,8 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_llc_shared_map); +DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_l2c_shared_map); + /* Per CPU bogomips and other parameters */ DEFINE_PER_CPU_READ_MOSTLY(struct cpuinfo_x86, cpu_info); EXPORT_PER_CPU_SYMBOL(cpu_info); @@ -501,6 +503,21 @@ static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) return topology_sane(c, o, "llc"); } +static bool match_l2c(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) +{ + int cpu1 = c->cpu_index, cpu2 = o->cpu_index; + + /* Do not match if we do not have a valid APICID for cpu: */ + if (per_cpu(cpu_l2c_id, cpu1) == BAD_APICID) + return false; + + /* Do not match if L2 cache id does not match: */ + if (per_cpu(cpu_l2c_id, cpu1) != per_cpu(cpu_l2c_id, cpu2)) + return false; + + return topology_sane(c, o, "l2c"); +} + /* * Unlike the other levels, we do not enforce keeping a * multicore group inside a NUMA node. If this happens, we will @@ -522,7 +539,7 @@ static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) } -#if defined(CONFIG_SCHED_SMT) || defined(CONFIG_SCHED_MC) +#if defined(CONFIG_SCHED_SMT) || defined(CONFIG_SCHED_CLUSTER) || defined(CONFIG_SCHED_MC) static inline int x86_sched_itmt_flags(void) { return sysctl_sched_itmt_enabled ? SD_ASYM_PACKING : 0; @@ -540,12 +557,21 @@ static int x86_smt_flags(void) return cpu_smt_flags() | x86_sched_itmt_flags(); } #endif +#ifdef CONFIG_SCHED_CLUSTER +static int x86_cluster_flags(void) +{ + return cpu_cluster_flags() | x86_sched_itmt_flags(); +} +#endif #endif static struct sched_domain_topology_level x86_numa_in_package_topology[] = { #ifdef CONFIG_SCHED_SMT { cpu_smt_mask, x86_smt_flags, SD_INIT_NAME(SMT) }, #endif +#ifdef CONFIG_SCHED_CLUSTER + { cpu_clustergroup_mask, x86_cluster_flags, SD_INIT_NAME(CLS) }, +#endif #ifdef CONFIG_SCHED_MC { cpu_coregroup_mask, x86_core_flags, SD_INIT_NAME(MC) }, #endif @@ -556,6 +582,9 @@ static int x86_smt_flags(void) #ifdef CONFIG_SCHED_SMT { cpu_smt_mask, x86_smt_flags, SD_INIT_NAME(SMT) }, #endif +#ifdef CONFIG_SCHED_CLUSTER + { cpu_clustergroup_mask, x86_cluster_flags, SD_INIT_NAME(CLS) }, +#endif #ifdef CONFIG_SCHED_MC { cpu_coregroup_mask, x86_core_flags, SD_INIT_NAME(MC) }, #endif @@ -583,6 +612,7 @@ void set_cpu_sibling_map(int cpu) if (!has_mp) { cpumask_set_cpu(cpu, topology_sibling_cpumask(cpu)); cpumask_set_cpu(cpu, cpu_llc_shared_mask(cpu)); + cpumask_set_cpu(cpu, cpu_l2c_shared_mask(cpu)); cpumask_set_cpu(cpu, topology_core_cpumask(cpu)); cpumask_set_cpu(cpu, topology_die_cpumask(cpu)); c->booted_cores = 1; @@ -598,6 +628,8 @@ void set_cpu_sibling_map(int cpu) if ((i == cpu) || (has_mp && match_llc(c, o))) link_mask(cpu_llc_shared_mask, cpu, i); + if ((i == cpu) || (has_mp && match_l2c(c, o))) + link_mask(cpu_l2c_shared_mask, cpu, i); } /* @@ -649,6 +681,11 @@ const struct cpumask *cpu_coregroup_mask(int cpu) return cpu_llc_shared_mask(cpu); } +const struct cpumask *cpu_clustergroup_mask(int cpu) +{ + return cpu_l2c_shared_mask(cpu); +} + static void impress_friends(void) { int cpu; @@ -1332,6 +1369,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus) zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL); zalloc_cpumask_var(&per_cpu(cpu_die_map, i), GFP_KERNEL); zalloc_cpumask_var(&per_cpu(cpu_llc_shared_map, i), GFP_KERNEL); + zalloc_cpumask_var(&per_cpu(cpu_l2c_shared_map, i), GFP_KERNEL); } /* @@ -1556,7 +1594,10 @@ static void remove_siblinginfo(int cpu) cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling)); for_each_cpu(sibling, cpu_llc_shared_mask(cpu)) cpumask_clear_cpu(cpu, cpu_llc_shared_mask(sibling)); + for_each_cpu(sibling, cpu_l2c_shared_mask(cpu)) + cpumask_clear_cpu(cpu, cpu_l2c_shared_mask(sibling)); cpumask_clear(cpu_llc_shared_mask(cpu)); + cpumask_clear(cpu_l2c_shared_mask(cpu)); cpumask_clear(topology_sibling_cpumask(cpu)); cpumask_clear(topology_core_cpumask(cpu)); cpumask_clear(topology_die_cpumask(cpu)); -- 1.8.3.1

1 0

[PATCH net-next 0/3] net: hns3: misc updates for -next
by Huazhong Tan 20 Apr '21

20 Apr '21

This series includes some misc updates for the HNS3 ethernet driver. Huazhong Tan (3): net: hns3: remove a duplicate pf reset counting net: hns3: cleanup inappropriate spaces in struct hlcgevf_tqp_stats net: hns3: change the value of the SEPARATOR_VALUE macro in hclgevf_main.c drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 -- drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 2 +- drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) -- 2.7.4

2 4

Re: [PATCH V4] app/testpmd: support Tx mbuf free on demand cmd
by Ferruh Yigit 19 Apr '21

19 Apr '21

On 4/19/2021 1:36 PM, Lijun Ou wrote: > From: Chengwen Feng <fengchengwen(a)huawei.com> > > This patch support tx_done_cleanup command: > tx_done_cleanup port (port_id) (queue_id) (free_cnt) > Instead of creating a new root level, 'tx_done_cleanup' command, what do you think to use existing port command, something like: "port cleanup <port_id> txq <queue_id> <free_cnt>" > Users must make sure there are no concurrent access to the same Tx > queue (like rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) > this command executed. > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> > --- > V3->V4: > - revert the V3 scheme. > > V2->V3: > - The command implementation is changed so that the queuestate does > not depend on the command execution. > > V1->V2: > - use Tx instead of TX > - add note in doc > --- > app/test-pmd/cmdline.c | 84 +++++++++++++++++++++++++++++ > doc/guides/rel_notes/release_21_05.rst | 2 + > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 ++++ > 3 files changed, 97 insertions(+) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index 5bf1497..b43964d 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -675,6 +675,9 @@ static void cmd_help_long_parsed(void *parsed_result, > "set port (port_id) ptype_mask (ptype_mask)\n" > " set packet types classification for a specific port\n\n" > > + "tx_done_cleanup (port_id) (queue_id) (free_cnt)\n" > + " Cleanup a Tx queue's mbuf on a port\n\n" > + If 'free_cnt' is '0', driver should free all possible mbufs, can you add this detail to help? > "set port (port_id) queue-region region_id (value) " > "queue_start_index (value) queue_num (value)\n" > " Set a queue region on a port\n\n" > @@ -16915,6 +16918,86 @@ cmdline_parse_inst_t cmd_showport_macs = { > }, > }; > > +/* *** tx_done_cleanup *** */ > +struct cmd_tx_done_cleanup_result { > + cmdline_fixed_string_t clean; > + cmdline_fixed_string_t port; > + uint16_t port_id; > + uint16_t queue_id; > + uint32_t free_cnt; > +}; > + > +static void > +cmd_tx_done_cleanup_parsed(void *parsed_result, > + __rte_unused struct cmdline *cl, > + __rte_unused void *data) > +{ > + struct cmd_tx_done_cleanup_result *res = parsed_result; > + uint16_t port_id = res->port_id; > + uint16_t queue_id = res->queue_id; > + uint32_t free_cnt = res->free_cnt; > + struct rte_eth_txq_info qinfo; > + int ret; > + > + if (rte_eth_tx_queue_info_get(port_id, queue_id, &qinfo)) { > + printf("Failed to get port %u Tx queue %u info!\n", > + port_id, queue_id); > + return; > + } > + > + if (qinfo.queue_state != RTE_ETH_QUEUE_STATE_STARTED) { > + printf("TX queue %u not started!\n", queue_id); > + return; > + } > + > + /* > + * rte_eth_tx_done_cleanup is a dataplane API, user must make sure > + * there are no concurrent access to the same Tx queue (like > + * rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) when this API > + * called. > + */ Should there be a check to see if testpmd forwarding is stared or not? This API shouldn't be allowed when forwarding is started. > + ret = rte_eth_tx_done_cleanup(port_id, queue_id, free_cnt); > + if (ret < 0) { > + printf("Failed to cleanup mbuf for port %u Tx queue %u " > + "error desc: %s(%d)\n", > + port_id, queue_id, strerror(-ret), ret); > + return; > + } > + > + printf("Cleanup port %u Tx queue %u mbuf nums: %u\n", > + port_id, queue_id, ret); > +} > + > +cmdline_parse_token_string_t cmd_tx_done_cleanup_clean = > + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, clean, > + "tx_done_cleanup"); > +cmdline_parse_token_string_t cmd_tx_done_cleanup_port = > + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, port, > + "port"); > +cmdline_parse_token_num_t cmd_tx_done_cleanup_port_id = > + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, port_id, > + UINT16); > +cmdline_parse_token_num_t cmd_tx_done_cleanup_queue_id = > + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, queue_id, > + UINT16); > +cmdline_parse_token_num_t cmd_tx_done_cleanup_free_cnt = > + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, free_cnt, > + UINT32); > + > +cmdline_parse_inst_t cmd_tx_done_cleanup = { > + .f = cmd_tx_done_cleanup_parsed, > + .data = NULL, > + .help_str = "tx_done_cleanup port <port_id> <queue_id> <free_cnt>", > + .tokens = { > + (void *)&cmd_tx_done_cleanup_clean, > + (void *)&cmd_tx_done_cleanup_port, > + (void *)&cmd_tx_done_cleanup_port_id, > + (void *)&cmd_tx_done_cleanup_queue_id, > + (void *)&cmd_tx_done_cleanup_free_cnt, > + NULL, > + }, > +}; > + > /* ******************************************************************************** */ > > /* list of instructions */ > @@ -17040,6 +17123,7 @@ cmdline_parse_ctx_t main_ctx[] = { > (cmdline_parse_inst_t *)&cmd_config_rss_reta, > (cmdline_parse_inst_t *)&cmd_showport_reta, > (cmdline_parse_inst_t *)&cmd_showport_macs, > + (cmdline_parse_inst_t *)&cmd_tx_done_cleanup, > (cmdline_parse_inst_t *)&cmd_config_burst, > (cmdline_parse_inst_t *)&cmd_config_thresh, > (cmdline_parse_inst_t *)&cmd_config_threshold, > diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst > index 4538bb9..554499f 100644 > --- a/doc/guides/rel_notes/release_21_05.rst > +++ b/doc/guides/rel_notes/release_21_05.rst > @@ -217,6 +217,8 @@ New Features > ``show port (port_id) rxq (queue_id) desc used count`` > * Added command to dump internal representation information of single flow. > ``flow dump (port_id) rule (rule_id)`` > + * Added command to cleanup a Tx queue's mbuf on a port. > + ``tx_done_cleanup port <port_id> <queue_id> <free_cnt>`` > > * **Updated ipsec-secgw sample application.** > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > index a736e7d..6472647 100644 > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > @@ -270,6 +270,17 @@ and ready to be processed by the driver on a given RX queue:: > > testpmd> show port (port_id) rxq (queue_id) desc used count > > +cleanup txq mbufs > +~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Request the driver to free mbufs currently cached by the driver for a given port's > +Tx queue:: > + testpmd> tx_done_cleanup port (port_id) (queue_id) (free_cnt) > + > +.. note:: > + This command is dangerous, users must make sure there are no cucurrent access to > + the same Tx queue (link rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on). > + > show config > ~~~~~~~~~~~ > >

1 0

[PATCH] app/testpmd: support Tx mbuf free on demand cmd
by Lijun Ou 19 Apr '21

19 Apr '21

From: Chengwen Feng <fengchengwen(a)huawei.com> This patch support tx_done_cleanup command: tx_done_cleanup port (port_id) (queue_id) (free_cnt) User must make sure there are no concurrent access to the same Tx queue (like rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) when this command executed. Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> Signed-off-by: Lijun Ou <oulijun(a)huawei.com> --- app/test-pmd/cmdline.c | 91 +++++++++++++++++++++++++++++ doc/guides/rel_notes/release_21_05.rst | 2 + doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++ 3 files changed, 100 insertions(+) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 14110eb..832ae70 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -36,6 +36,7 @@ #include <rte_pci.h> #include <rte_ether.h> #include <rte_ethdev.h> +#include <rte_ethdev_driver.h> #include <rte_string_fns.h> #include <rte_devargs.h> #include <rte_flow.h> @@ -675,6 +676,9 @@ static void cmd_help_long_parsed(void *parsed_result, "set port (port_id) ptype_mask (ptype_mask)\n" " set packet types classification for a specific port\n\n" + "tx_done_cleanup (port_id) (queue_id) (free_cnt)\n" + " Cleanup a tx queue's mbuf on a port\n\n" + "set port (port_id) queue-region region_id (value) " "queue_start_index (value) queue_num (value)\n" " Set a queue region on a port\n\n" @@ -16910,6 +16914,92 @@ cmdline_parse_inst_t cmd_showport_macs = { }, }; +/* *** tx_done_cleanup *** */ +struct cmd_tx_done_cleanup_result { + cmdline_fixed_string_t clean; + cmdline_fixed_string_t port; + uint16_t port_id; + uint16_t queue_id; + uint32_t free_cnt; +}; + +static void +cmd_tx_done_cleanup_parsed(void *parsed_result, + __rte_unused struct cmdline *cl, + __rte_unused void *data) +{ + struct cmd_tx_done_cleanup_result *res = parsed_result; + struct rte_eth_dev *dev; + uint16_t port_id = res->port_id; + uint16_t queue_id = res->queue_id; + uint32_t free_cnt = res->free_cnt; + int ret; + + if (!rte_eth_dev_is_valid_port(port_id)) { + printf("Invalid port_id %u\n", port_id); + return; + } + + dev = &rte_eth_devices[port_id]; + if (queue_id >= dev->data->nb_tx_queues) { + printf("Invalid TX queue_id %u\n", queue_id); + return; + } + + if (dev->data->tx_queue_state[queue_id] != + RTE_ETH_QUEUE_STATE_STARTED) { + printf("TX queue_id %u not started!\n", queue_id); + return; + } + + /* + * rte_eth_tx_done_cleanup is a dataplane API, user must make sure + * there are no concurrent access to the same Tx queue (like + * rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) when this API + * called. + */ + ret = rte_eth_tx_done_cleanup(port_id, queue_id, free_cnt); + if (ret < 0) { + printf("Failed to cleanup mbuf for port %u TX queue %u " + "error desc: %s(%d)\n", + port_id, queue_id, strerror(-ret), ret); + return; + } + + printf("Cleanup port %u TX queue %u mbuf nums: %u\n", + port_id, queue_id, ret); +} + +cmdline_parse_token_string_t cmd_tx_done_cleanup_clean = + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, clean, + "tx_done_cleanup"); +cmdline_parse_token_string_t cmd_tx_done_cleanup_port = + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, port, + "port"); +cmdline_parse_token_num_t cmd_tx_done_cleanup_port_id = + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, port_id, + UINT16); +cmdline_parse_token_num_t cmd_tx_done_cleanup_queue_id = + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, queue_id, + UINT16); +cmdline_parse_token_num_t cmd_tx_done_cleanup_free_cnt = + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, free_cnt, + UINT32); + +cmdline_parse_inst_t cmd_tx_done_cleanup = { + .f = cmd_tx_done_cleanup_parsed, + .data = NULL, + .help_str = "tx_done_cleanup port <port_id> <queue_id> <free_cnt>", + .tokens = { + (void *)&cmd_tx_done_cleanup_clean, + (void *)&cmd_tx_done_cleanup_port, + (void *)&cmd_tx_done_cleanup_port_id, + (void *)&cmd_tx_done_cleanup_queue_id, + (void *)&cmd_tx_done_cleanup_free_cnt, + NULL, + }, +}; + /* ******************************************************************************** */ /* list of instructions */ @@ -17035,6 +17125,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_config_rss_reta, (cmdline_parse_inst_t *)&cmd_showport_reta, (cmdline_parse_inst_t *)&cmd_showport_macs, + (cmdline_parse_inst_t *)&cmd_tx_done_cleanup, (cmdline_parse_inst_t *)&cmd_config_burst, (cmdline_parse_inst_t *)&cmd_config_thresh, (cmdline_parse_inst_t *)&cmd_config_threshold, diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst index 23f7f0b..8077573 100644 --- a/doc/guides/rel_notes/release_21_05.rst +++ b/doc/guides/rel_notes/release_21_05.rst @@ -69,6 +69,8 @@ New Features * Added command to display Rx queue used descriptor count. ``show port (port_id) rxq (queue_id) desc used count`` + * Added command to cleanup a Tx queue's mbuf on a port. + ``tx_done_cleanup port <port_id> <queue_id> <free_cnt>`` Removed Items diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst index f59eb8a..39281f5 100644 --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst @@ -272,6 +272,13 @@ and ready to be processed by the driver on a given RX queue:: testpmd> show port (port_id) rxq (queue_id) desc used count +cleanup txq mbufs +~~~~~~~~~~~~~~~~~~~~~~~~ + +Request the driver to free mbufs currently cached by the driver for a given port's +Tx queue:: + testpmd> tx_done_cleanup port (port_id) (queue_id) (free_cnt) + show config ~~~~~~~~~~~ -- 2.7.4

7 22

Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio
by Jason Gunthorpe 19 Apr '21

19 Apr '21

On Mon, Apr 19, 2021 at 08:24:40PM +0800, liulongfang wrote: > > I'm also confused how this works securely at all, as a general rule a > > VFIO PCI driver cannot access the MMIO memory of the function it is > > planning to assign to the guest. There is a lot of danger that the > > guest could access that MMIO space one way or another. > > VF's MMIO memory is divided into two parts, one is the guest part, > and the other is the live migration part. They do not affect each other, > so there is no security problem. AFAIK there are several scenarios where a guest can access this MMIO memory using DMA even if it is not mapped into the guest for CPU access. > If pci_release_mem_regions() is not added here, > The guests using pci_request_mem_regions() will return an error. > Then, the guest will not be able to obtain the MMIO address of the VF. Which is why VFIO has this protection to prevent sharing MMIO regions on the VF assigned to the guest Jason

1 0

Re: [PATCH v7] ethdev: add queue state when retrieve queue information
by Thomas Monjalon 19 Apr '21

19 Apr '21

19/04/2021 10:57, Lijun Ou: > Currently, upper-layer application could get queue state only > through pointers such as dev->data->tx_queue_state[queue_id], > this is not the recommended way to access it. So this patch > add get queue state when call rte_eth_rx_queue_info_get and > rte_eth_tx_queue_info_get API. > > Note: After add queue_state field, the 'struct rte_eth_rxq_info' size > remains 128B, and the 'struct rte_eth_txq_info' size remains 64B, so > it could be ABI compatible. > > Signed-off-by: Chengwen Feng <fengchengwen(a)huawei.com> > Signed-off-by: Lijun Ou <oulijun(a)huawei.com> > Acked-by: Konstantin Ananyev <konstantin.ananyev(a)intel.com> Acked-by: Thomas Monjalon <thomas(a)monjalon.net>

2 1