- Linuxarm - mailweb.openeuler.org

Re: [PATCH net v8 0/3] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 14 May '21

14 May '21

On 2021/5/14 10:26, Yunsheng Lin wrote: > 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. > Patch 3: Fix the significantly higher CPU consumption problem when > multiple threads are competing on a saturated outgoing > device. > > V8: Change function name in patch 3 as suggested by Jakub, adjust > commit log in patch 2, and add Acked-by from Jakub. Please ignore this patchset, there is some typo in patch 3. > V7: Fix netif_tx_wake_queue() data race noted by Jakub. > V6: Some performance optimization in patch 1 suggested by Jakub > and drop NET_XMIT_DROP checking in patch 3. > V5: add patch 3 to fix the problem reported by Michal Kubecek. > V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. > > [1]. https://lkml.org/lkml/2019/10/9/42 > > Yunsheng Lin (3): > net: sched: fix packet stuck problem for lockless qdisc > net: sched: fix tx action rescheduling issue during deactivation > net: sched: fix tx action reschedule issue with stopped queue > > include/net/pkt_sched.h | 7 +------ > include/net/sch_generic.h | 35 ++++++++++++++++++++++++++++++++- > net/core/dev.c | 29 ++++++++++++++++++++++----- > net/sched/sch_generic.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 107 insertions(+), 14 deletions(-) >

1 0

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

14 May '21

On Thu, 13 May 2021 17:52:56 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi(a)huawei.com> wrote: > Hi Alex, > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: 13 May 2021 18:04 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi(a)huawei.com> > > Cc: Jason Gunthorpe <jgg(a)nvidia.com>; liulongfang > > <liulongfang(a)huawei.com>; cohuck(a)redhat.com; > > linux-kernel(a)vger.kernel.org; linuxarm(a)openeuler.org > > Subject: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to > > vfio > > > > On Thu, 13 May 2021 15:49:25 +0000 > > Shameerali Kolothum Thodi <shameerali.kolothum.thodi(a)huawei.com> > > wrote: > > > > > > -----Original Message----- > > > > From: Jason Gunthorpe [mailto:jgg@nvidia.com] > > > > Sent: 13 May 2021 14:44 > > > > To: liulongfang <liulongfang(a)huawei.com> > > > > Cc: Alex Williamson <alex.williamson(a)redhat.com>; cohuck(a)redhat.com; > > > > linux-kernel(a)vger.kernel.org; linuxarm(a)openeuler.org > > > > Subject: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to > > > > vfio > > > > > > > > On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote: > > > > > On 2021/5/12 20:10, Jason Gunthorpe wrote: > > > > > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > > > > > > > > > > > >> Therefore, this method of limiting the length of the BAR > > > > > >> configuration space can prevent unsafe operations of the memory. > > > > > > > > > > > > The issue is DMA controlled by the guest accessing the secure BAR > > > > > > area, not the guest CPU. > > > > > > > > > > > > Jason > > > > > > . > > > > > > > > > > > This secure BAR area is not presented to the Guest, > > > > > which makes it impossible for the Guest to obtain the secure BAR area > > > > > when establishing the DMA mapping of the configuration space. > > > > > If the DMA controller accesses the secure BAR area, the access will > > > > > be blocked by the SMMU. > > > > > > > > There are scenarios where this is not true. > > > > > > > > At a minimum the mdev driver should refuse to work in those cases. > > > > > > > > > > Hi, > > > > > > I think the idea here is not a generic solution, but a quirk for this specific dev. > > > > > > Something like, > > > > > > --- a/drivers/vfio/pci/vfio_pci.c > > > +++ b/drivers/vfio/pci/vfio_pci.c > > > @@ -866,7 +866,12 @@ static long vfio_pci_ioctl(struct vfio_device > > *core_vdev, > > > break; > > > case VFIO_PCI_BAR0_REGION_INDEX ... > > VFIO_PCI_BAR5_REGION_INDEX: > > > info.offset = > > VFIO_PCI_INDEX_TO_OFFSET(info.index); > > > - info.size = pci_resource_len(pdev, info.index); > > > + > > > + if (check_hisi_acc_quirk(pdev, info)) > > > + info.size = new_size;// BAR is limited > > without migration region. > > > + else > > > + info.size = pci_resource_len(pdev, > > info.index); > > > + > > > if (!info.size) { > > > info.flags = 0; > > > break; > > > > > > Is this an acceptable/workable solution here? > > > > As Jason says, this only restricts CPU access to the BAR, the issue is > > DMA access. As the hardware vendor you may be able to guarantee that > > a DMA transaction generated by the device targeting the remainder of > > the BAR will always go upstream, but can you guarantee the routing > > between the device and the SMMU? For instance if this device can be > > implemented as a plugin card, then it can be installed into a > > downstream port that may not support ACS. That downstream port may > > implement request redirection allowing the transaction to reflect back > > to the device without IOMMU translation. At that point the userspace > > driver can target the kernel driver half of the BAR and potentially > > expose a security risk. Thanks, > > The ACC devices on this platform are not pluggable devices. They are exposed > as integrated endpoint devices. So I am not sure the above concern is valid in this > case. > > I had a look at the userspace driver approach you suggested. But unfortunately > the migration state change for the vf has to check some of the pf registers for > confirming the state. So even if we move the implementation to Qemu, we > still may have to use the migration uAPI to access the pf device registers. > > Since the devices we are concerned here are all integrated endpoints and if the > above quirk is an acceptable one, then we can use the uAPI as done in this > series without overly complicating things here. If you expect this device to appear only as an integrated endpoint, then I think Jason's suggestion above is correct. Your driver that supports migration can refuse to load for devices there the topology is other than expected and you're effectively guaranteeing DMA isolation of the user and in-kernel drivers by hardware DMA semantics and topology. Requiring access to the PF to support the migration protocol also suggests that an in-kernel driver to support migration is our best option. Thanks, Alex

3 2

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

14 May '21

On Thu, 13 May 2021 15:49:25 +0000 Shameerali Kolothum Thodi <shameerali.kolothum.thodi(a)huawei.com> wrote: > > -----Original Message----- > > From: Jason Gunthorpe [mailto:jgg@nvidia.com] > > Sent: 13 May 2021 14:44 > > To: liulongfang <liulongfang(a)huawei.com> > > Cc: Alex Williamson <alex.williamson(a)redhat.com>; cohuck(a)redhat.com; > > linux-kernel(a)vger.kernel.org; linuxarm(a)openeuler.org > > Subject: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to > > vfio > > > > On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote: > > > On 2021/5/12 20:10, Jason Gunthorpe wrote: > > > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > > > > > > > >> Therefore, this method of limiting the length of the BAR > > > >> configuration space can prevent unsafe operations of the memory. > > > > > > > > The issue is DMA controlled by the guest accessing the secure BAR > > > > area, not the guest CPU. > > > > > > > > Jason > > > > . > > > > > > > This secure BAR area is not presented to the Guest, > > > which makes it impossible for the Guest to obtain the secure BAR area > > > when establishing the DMA mapping of the configuration space. > > > If the DMA controller accesses the secure BAR area, the access will > > > be blocked by the SMMU. > > > > There are scenarios where this is not true. > > > > At a minimum the mdev driver should refuse to work in those cases. > > > > Hi, > > I think the idea here is not a generic solution, but a quirk for this specific dev. > > Something like, > > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -866,7 +866,12 @@ static long vfio_pci_ioctl(struct vfio_device *core_vdev, > break; > case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX: > info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index); > - info.size = pci_resource_len(pdev, info.index); > + > + if (check_hisi_acc_quirk(pdev, info)) > + info.size = new_size;// BAR is limited without migration region. > + else > + info.size = pci_resource_len(pdev, info.index); > + > if (!info.size) { > info.flags = 0; > break; > > Is this an acceptable/workable solution here? As Jason says, this only restricts CPU access to the BAR, the issue is DMA access. As the hardware vendor you may be able to guarantee that a DMA transaction generated by the device targeting the remainder of the BAR will always go upstream, but can you guarantee the routing between the device and the SMMU? For instance if this device can be implemented as a plugin card, then it can be installed into a downstream port that may not support ACS. That downstream port may implement request redirection allowing the transaction to reflect back to the device without IOMMU translation. At that point the userspace driver can target the kernel driver half of the BAR and potentially expose a security risk. Thanks, Alex

1 0

[PATCH net v7 0/3] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 13 May '21

13 May '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. Patch 3: Fix the significantly higher CPU consumption problem when multiple threads are competing on a saturated outgoing device. V7: Fix netif_tx_wake_queue() data race noted by Jakub. V6: Some performance optimization in patch 1 suggested by Jakub and drop NET_XMIT_DROP checking in patch 3. V5: add patch 3 to fix the problem reported by Michal Kubecek. V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. [1]. https://lkml.org/lkml/2019/10/9/42 Yunsheng Lin (3): net: sched: fix packet stuck problem for lockless qdisc net: sched: fix endless tx action reschedule during deactivation net: sched: fix tx action reschedule issue with stopped queue include/net/pkt_sched.h | 7 +------ include/net/sch_generic.h | 35 ++++++++++++++++++++++++++++++++- net/core/dev.c | 29 ++++++++++++++++++++++----- net/sched/sch_generic.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 107 insertions(+), 14 deletions(-) -- 2.7.4

2 5

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

13 May '21

On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote: > On 2021/5/12 20:10, Jason Gunthorpe wrote: > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > > > >> Therefore, this method of limiting the length of the BAR > >> configuration space can prevent unsafe operations of the memory. > > > > The issue is DMA controlled by the guest accessing the secure BAR > > area, not the guest CPU. > > > > Jason > > . > > > This secure BAR area is not presented to the Guest, > which makes it impossible for the Guest to obtain the secure BAR area > when establishing the DMA mapping of the configuration space. > If the DMA controller accesses the secure BAR area, the access will > be blocked by the SMMU. There are scenarios where this is not true. At a minimum the mdev driver should refuse to work in those cases. Jason

1 0

[PATCH net v6 0/3] fix packet stuck problem for lockless qdisc
by Yunsheng Lin 13 May '21

13 May '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. Patch 3: Fix the significantly higher CPU consumption problem when multiple threads are competing on a saturated outgoing device. V6: Some performance optimization in patch 1 suggested by Jakub and drop NET_XMIT_DROP checking in patch 3. V5: add patch 3 to fix the problem reported by Michal Kubecek. V4: Change STATE_NEED_RESCHEDULE to STATE_MISSED and add patch 2. [1]. https://lkml.org/lkml/2019/10/9/42 Yunsheng Lin (3): net: sched: fix packet stuck problem for lockless qdisc net: sched: fix endless tx action reschedule during deactivation net: sched: fix tx action reschedule issue with stopped queue include/net/pkt_sched.h | 7 +------ include/net/sch_generic.h | 33 ++++++++++++++++++++++++++++++++- net/core/dev.c | 29 ++++++++++++++++++++++++----- net/sched/sch_generic.c | 31 +++++++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 14 deletions(-) -- 2.7.4

2 9

Re: [Intel-wired-lan] [PATCH V3 net] ice: Re-organizes reqstd/avail {R, T}XQ check/code for efficiency
by Brelinski, TonyX 12 May '21

12 May '21

> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces(a)osuosl.org> On Behalf Of > Salil Mehta > Sent: Thursday, April 22, 2021 5:00 PM > To: davem(a)davemloft.net; kuba(a)kernel.org > Cc: salil.mehta(a)huawei.com; netdev(a)vger.kernel.org; > linuxarm(a)huawei.com; linuxarm(a)openeuler.org; linux- > kernel(a)vger.kernel.org; intel-wired-lan(a)lists.osuosl.org > Subject: [Intel-wired-lan] [PATCH V3 net] ice: Re-organizes reqstd/avail {R, > T}XQ check/code for efficiency > > 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. > > Re-organize this code for improving the flow, readability and efficiency. > This scope of improvement was found during the review of the ICE driver > code. > > Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") > Cc: intel-wired-lan(a)lists.osuosl.org > Tested-by: Tony Brelinski <tonyx.brelinski(a)intel.com> > Signed-off-by: Salil Mehta <salil.mehta(a)huawei.com> > --- > Change: > V2->V3 > (*) Addressed some comments from Paul Menzel > Link: https://lkml.org/lkml/2021/4/21/136 > 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(-) Tested-by: Tony Brelinski <tonyx.brelinski(a)intel.com> (A Contingent Worker at Intel)

1 0

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

12 May '21

On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote: > Therefore, this method of limiting the length of the BAR > configuration space can prevent unsafe operations of the memory. The issue is DMA controlled by the guest accessing the secure BAR area, not the guest CPU. Jason

1 0

[PATCH 00/17] tty: Fix some coding style issues
by Xiaofei Tan 12 May '21

12 May '21

Fix some issues reported by checkpatch.pl. All of them are coding style issues, no function changes. Xiaofei Tan (17): tty: tty_baudrate: Remove unnecessary tab and spaces in comment sentence tty: tty_baudrate: Fix coding style issues of block comments tty: tty_buffer: Add a blank line after declarations tty: tty_buffer: Remove the repeated word 'the' tty: tty_buffer: Fix coding style issues of block comments tty: tty_io: Remove spaces before tabs tty: tty_io: Add a blank line after declarations tty: tty_io: Fix spaces required around that ':' tty: tty_io: Fix trailing whitespace issues tty: tty_io: Fix coding style issues of block comments tty: tty_io: Remove the repeated word 'can' tty: tty_io: Fix an issue of code indent for conditional statements tty: tty_io: Delete a blank line before EXPORT_SYMBOL(foo) tty: tty_io: Remove return in void function tty: tty_port: Delete a blank line before EXPORT_SYMBOL(foo) tty: tty_port: Add a blank line after declarations tty: tty_port: Fix coding style issues of block comments drivers/tty/tty_baudrate.c | 13 ++++++---- drivers/tty/tty_buffer.c | 20 +++++++++++---- drivers/tty/tty_io.c | 61 ++++++++++++++++++++++++++-------------------- drivers/tty/tty_port.c | 16 +++++++----- 4 files changed, 67 insertions(+), 43 deletions(-) -- 2.8.1

1 6

[PATCH] RDMA/ucma: Cleanup to reduce duplicate code
by Xiaofei Tan 12 May '21

12 May '21

The lable "err1" does the same thing as the branch of copy_to_user() failed in the function ucma_create_id(). Just jump to the label directly to reduce duplicate code. Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com> --- drivers/infiniband/core/ucma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 15d57ba..1f198c1 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -468,8 +468,8 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf, resp.id = ctx->id; if (copy_to_user(u64_to_user_ptr(cmd.response), &resp, sizeof(resp))) { - ucma_destroy_private_ctx(ctx); - return -EFAULT; + ret = -EFAULT; + goto err1; } mutex_lock(&file->mut); -- 2.8.1

3 2