Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: rtlwifi-new v0.6 and dkms failures
From: David R. Bergstein @ 2019-04-16 21:19 UTC (permalink / raw)
  To: Larry Finger, Tony Chuang
  Cc: Pkshih, linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <60509a60-d0ea-d17f-8b79-e51c374717ba@gmail.com>

Larry,

As it tuns out, I was able to create a new dkms.conf file which works on
my system. Below is  copy of the content from that file:

PACKAGE_NAME="rtlwifi-new"
PACKAGE_VERSION=0.6
MAKE="make -C $kernel_source_dir
M=$dkms_tree/$PACKAGE_NAME/$PACKAGE_VERSION/build"
CLEAN="make -C $kernel_source_dir clean"
BUILT_MODULE_NAME[0]="rtw88"
BUILT_MODULE_NAME[1]="rtwpci"
DEST_MODULE_LOCATION[0]="/updates"
DEST_MODULE_LOCATION[1]="/updates"
REMAKE_INITRD=no
AUTOINSTALL=yes

This also had the benefit of module signing by the version of dkms in
Ubuntu 19.04, so I am now also back on secure boot.

No action on your part is requested at this time.  Feel free to use the
new dkms.conf content if it is useful to you.

Regards,

David R. Bergstein

On 4/14/19 5:39 PM, David R. Bergstein wrote:
> Larry,
>
> Thanks for your prior assistance with the rtw88 module.  Since manually
> compiling and installing works correctly, I have been working to
> register the software in dkms, but am running into some difficulties. 
> Below are some snippets from a terminal session to illustrate the issue:
>
> udo dkms install rtlwifi-new/0.6
>
> Creating symlink /var/lib/dkms/rtlwifi-new/0.6/source ->
>                  /usr/src/rtlwifi-new-0.6
>
> DKMS: add completed.
>
> Kernel preparation unnecessary for this kernel.  Skipping...
>
> Building module:
> cleaning build area....(bad exit status: 2)
> make -j12 KERNELRELEASE=5.0.0-11-lowlatency -C
> /lib/modules/5.0.0-11-lowlatency/build
> M=/var/lib/dkms/rtlwifi-new/0.6/build....
> ERROR (dkms apport): binary package for rtlwifi-new: 0.6 not found
> Error!  Build of rtl_pci.ko failed for: 5.0.0-11-lowlatency (x86_64)
> Consult the make.log in the build directory
> /var/lib/dkms/rtlwifi-new/0.6/build/ for more information.
>
> Below is the make.log contents referenced above:
>
> DKMS make.log for rtlwifi-new-0.6 for kernel 5.0.0-11-lowlatency (x86_64)
> Sun 14 Apr 2019 05:25:37 PM EDT
> make: Entering directory '/usr/src/linux-headers-5.0.0-11-lowlatency'
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/main.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/mac80211.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/util.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/debug.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/tx.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rx.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/mac.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/phy.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/efuse.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/fw.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/ps.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/sec.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/regd.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822b.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822b_table.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822c.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822c_table.o
>   CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/pci.o
>   LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.o
>   LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.o
>   Building modules, stage 2.
>   MODPOST 2 modules
>   CC      /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.mod.o
>   CC      /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.mod.o
>   LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.ko
>   LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.ko
> make: Leaving directory '/usr/src/linux-headers-5.0.0-11-lowlatency'
>
> As you can see, the make log does not show any errors, which is baffling
> to me.  Any insight you have on resolving this issue would be appreciated.
>
> Sincerely,
>
> David R. Bergstein
>
>

^ permalink raw reply

* [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
From: Arnd Bergmann @ 2019-04-16 20:25 UTC (permalink / raw)
  To: Alexander Viro
  Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann,
	Jason Gunthorpe, Daniel Vetter, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, David Sterba, Darren Hart, Jonathan Cameron,
	Bjorn Andersson, devel, qat-linux, linux-crypto, linux-media,
	dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio,
	linux-rdma, linux-nvdimm, linux-nvme, linux-pci,
	platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi,
	linux-usb, linux-btrfs, ceph-devel, linux-wireless, netdev
In-Reply-To: <20190416202013.4034148-1-arnd@arndb.de>

The .ioctl and .compat_ioctl file operations have the same prototype so
they can both point to the same function, which works great almost all
the time when all the commands are compatible.

One exception is the s390 architecture, where a compat pointer is only
31 bit wide, and converting it into a 64-bit pointer requires calling
compat_ptr(). Most drivers here will ever run in s390, but since we now
have a generic helper for it, it's easy enough to use it consistently.

I double-checked all these drivers to ensure that all ioctl arguments
are used as pointers or are ignored, but are not interpreted as integer
values.

Acked-by: Jason Gunthorpe <jgg@mellanox.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: David Sterba <dsterba@suse.com>
Acked-by: Darren Hart (VMware) <dvhart@infradead.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/android/binder.c                    | 2 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
 drivers/dma-buf/dma-buf.c                   | 4 +---
 drivers/dma-buf/sw_sync.c                   | 2 +-
 drivers/dma-buf/sync_file.c                 | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    | 2 +-
 drivers/hid/hidraw.c                        | 4 +---
 drivers/iio/industrialio-core.c             | 2 +-
 drivers/infiniband/core/uverbs_main.c       | 4 ++--
 drivers/media/rc/lirc_dev.c                 | 4 +---
 drivers/mfd/cros_ec_dev.c                   | 4 +---
 drivers/misc/vmw_vmci/vmci_host.c           | 2 +-
 drivers/nvdimm/bus.c                        | 4 ++--
 drivers/nvme/host/core.c                    | 2 +-
 drivers/pci/switch/switchtec.c              | 2 +-
 drivers/platform/x86/wmi.c                  | 2 +-
 drivers/rpmsg/rpmsg_char.c                  | 4 ++--
 drivers/sbus/char/display7seg.c             | 2 +-
 drivers/sbus/char/envctrl.c                 | 4 +---
 drivers/scsi/3w-xxxx.c                      | 4 +---
 drivers/scsi/cxlflash/main.c                | 2 +-
 drivers/scsi/esas2r/esas2r_main.c           | 2 +-
 drivers/scsi/pmcraid.c                      | 4 +---
 drivers/staging/android/ion/ion.c           | 4 +---
 drivers/staging/vme/devices/vme_user.c      | 2 +-
 drivers/tee/tee_core.c                      | 2 +-
 drivers/usb/class/cdc-wdm.c                 | 2 +-
 drivers/usb/class/usbtmc.c                  | 4 +---
 drivers/virt/fsl_hypervisor.c               | 2 +-
 fs/btrfs/super.c                            | 2 +-
 fs/ceph/dir.c                               | 2 +-
 fs/ceph/file.c                              | 2 +-
 fs/fuse/dev.c                               | 2 +-
 fs/notify/fanotify/fanotify_user.c          | 2 +-
 fs/userfaultfd.c                            | 2 +-
 net/rfkill/core.c                           | 2 +-
 36 files changed, 39 insertions(+), 57 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 4b9c7ca492e6..48109ade7234 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5998,7 +5998,7 @@ const struct file_operations binder_fops = {
 	.owner = THIS_MODULE,
 	.poll = binder_poll,
 	.unlocked_ioctl = binder_ioctl,
-	.compat_ioctl = binder_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.mmap = binder_mmap,
 	.open = binder_open,
 	.flush = binder_flush,
diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index abc7a7f64d64..ef0e482ee04f 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
 static const struct file_operations adf_ctl_ops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = adf_ctl_ioctl,
-	.compat_ioctl = adf_ctl_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 struct adf_ctl_drv_info {
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 7c858020d14b..0cb336fe6324 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
 	.llseek		= dma_buf_llseek,
 	.poll		= dma_buf_poll,
 	.unlocked_ioctl	= dma_buf_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= dma_buf_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 /*
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 32dcf7b4c935..411de6a8a0ad 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -419,5 +419,5 @@ const struct file_operations sw_sync_debugfs_fops = {
 	.open           = sw_sync_debugfs_open,
 	.release        = sw_sync_debugfs_release,
 	.unlocked_ioctl = sw_sync_ioctl,
-	.compat_ioctl	= sw_sync_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 4f6305ca52c8..0949f91eb85f 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -488,5 +488,5 @@ static const struct file_operations sync_file_fops = {
 	.release = sync_file_release,
 	.poll = sync_file_poll,
 	.unlocked_ioctl = sync_file_ioctl,
-	.compat_ioctl = sync_file_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 083bd8114db1..5d6ac7885aa7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -49,7 +49,7 @@ static const char kfd_dev_name[] = "kfd";
 static const struct file_operations kfd_fops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = kfd_ioctl,
-	.compat_ioctl = kfd_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.open = kfd_open,
 	.mmap = kfd_mmap,
 };
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 9fc51eff1079..e7284d38b66d 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -476,9 +476,7 @@ static const struct file_operations hidraw_ops = {
 	.release =      hidraw_release,
 	.unlocked_ioctl = hidraw_ioctl,
 	.fasync =	hidraw_fasync,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = hidraw_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 4700fd5d8c90..eed1bea257b4 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1635,7 +1635,7 @@ static const struct file_operations iio_buffer_fileops = {
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
 	.unlocked_ioctl = iio_ioctl,
-	.compat_ioctl = iio_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 70b7d80431a9..ac4321d7c800 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -1120,7 +1120,7 @@ static const struct file_operations uverbs_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static const struct file_operations uverbs_mmap_fops = {
@@ -1131,7 +1131,7 @@ static const struct file_operations uverbs_mmap_fops = {
 	.release = ib_uverbs_close,
 	.llseek	 = no_llseek,
 	.unlocked_ioctl = ib_uverbs_ioctl,
-	.compat_ioctl = ib_uverbs_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static struct ib_client uverbs_client = {
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index f862f1b7f996..9ccc7e9cbc8e 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -730,9 +730,7 @@ static const struct file_operations lirc_fops = {
 	.owner		= THIS_MODULE,
 	.write		= ir_lirc_transmit_ir,
 	.unlocked_ioctl	= ir_lirc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ir_lirc_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.read		= ir_lirc_read,
 	.poll		= ir_lirc_poll,
 	.open		= ir_lirc_open,
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index d275deaecb12..4a602a40d75c 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -251,9 +251,7 @@ static const struct file_operations fops = {
 	.release = ec_device_release,
 	.read = ec_device_read,
 	.unlocked_ioctl = ec_device_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = ec_device_ioctl,
-#endif
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void cros_ec_class_release(struct device *dev)
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index 997f92543dd4..5bb406dabe85 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -969,7 +969,7 @@ static const struct file_operations vmuser_fops = {
 	.release	= vmci_host_close,
 	.poll		= vmci_host_poll,
 	.unlocked_ioctl	= vmci_host_unlocked_ioctl,
-	.compat_ioctl	= vmci_host_unlocked_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static struct miscdevice vmci_host_miscdev = {
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 7bbff0af29b2..065ebd584482 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -1167,7 +1167,7 @@ static const struct file_operations nvdimm_bus_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nd_ioctl,
-	.compat_ioctl = nd_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
@@ -1175,7 +1175,7 @@ static const struct file_operations nvdimm_fops = {
 	.owner = THIS_MODULE,
 	.open = nd_open,
 	.unlocked_ioctl = nvdimm_ioctl,
-	.compat_ioctl = nvdimm_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2c43e12b70af..560929bee5ce 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2739,7 +2739,7 @@ static const struct file_operations nvme_dev_fops = {
 	.owner		= THIS_MODULE,
 	.open		= nvme_dev_open,
 	.unlocked_ioctl	= nvme_dev_ioctl,
-	.compat_ioctl	= nvme_dev_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static ssize_t nvme_sysfs_reset(struct device *dev,
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index e22766c79fe9..3a54b4b616e2 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1006,7 +1006,7 @@ static const struct file_operations switchtec_fops = {
 	.read = switchtec_dev_read,
 	.poll = switchtec_dev_poll,
 	.unlocked_ioctl = switchtec_dev_ioctl,
-	.compat_ioctl = switchtec_dev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void link_event_work(struct work_struct *work)
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7b26b6ccf1a0..dded9cef42f4 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -889,7 +889,7 @@ static const struct file_operations wmi_fops = {
 	.read		= wmi_char_read,
 	.open		= wmi_char_open,
 	.unlocked_ioctl	= wmi_ioctl,
-	.compat_ioctl	= wmi_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static int wmi_dev_probe(struct device *dev)
diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index eea5ebbb5119..507bfe163883 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -290,7 +290,7 @@ static const struct file_operations rpmsg_eptdev_fops = {
 	.write_iter = rpmsg_eptdev_write_iter,
 	.poll = rpmsg_eptdev_poll,
 	.unlocked_ioctl = rpmsg_eptdev_ioctl,
-	.compat_ioctl = rpmsg_eptdev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static ssize_t name_show(struct device *dev, struct device_attribute *attr,
@@ -451,7 +451,7 @@ static const struct file_operations rpmsg_ctrldev_fops = {
 	.open = rpmsg_ctrldev_open,
 	.release = rpmsg_ctrldev_release,
 	.unlocked_ioctl = rpmsg_ctrldev_ioctl,
-	.compat_ioctl = rpmsg_ctrldev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void rpmsg_ctrldev_release_device(struct device *dev)
diff --git a/drivers/sbus/char/display7seg.c b/drivers/sbus/char/display7seg.c
index a36e4cf1841d..c9f60656f54d 100644
--- a/drivers/sbus/char/display7seg.c
+++ b/drivers/sbus/char/display7seg.c
@@ -155,7 +155,7 @@ static long d7s_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 static const struct file_operations d7s_fops = {
 	.owner =		THIS_MODULE,
 	.unlocked_ioctl =	d7s_ioctl,
-	.compat_ioctl =		d7s_ioctl,
+	.compat_ioctl =		compat_ptr_ioctl,
 	.open =			d7s_open,
 	.release =		d7s_release,
 	.llseek = noop_llseek,
diff --git a/drivers/sbus/char/envctrl.c b/drivers/sbus/char/envctrl.c
index 1a6e7224017c..dd2dfa85fc68 100644
--- a/drivers/sbus/char/envctrl.c
+++ b/drivers/sbus/char/envctrl.c
@@ -714,9 +714,7 @@ static const struct file_operations envctrl_fops = {
 	.owner =		THIS_MODULE,
 	.read =			envctrl_read,
 	.unlocked_ioctl =	envctrl_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl =		envctrl_ioctl,
-#endif
+	.compat_ioctl =		compat_ptr_ioctl,
 	.open =			envctrl_open,
 	.release =		envctrl_release,
 	.llseek =		noop_llseek,
diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
index 2b1e0d503020..fb6444d0409c 100644
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1049,9 +1049,7 @@ static int tw_chrdev_open(struct inode *inode, struct file *file)
 static const struct file_operations tw_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= tw_chrdev_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl   = tw_chrdev_ioctl,
-#endif
+	.compat_ioctl   = compat_ptr_ioctl,
 	.open		= tw_chrdev_open,
 	.release	= NULL,
 	.llseek		= noop_llseek,
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 7096810fd222..e13d5de1d76e 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -3589,7 +3589,7 @@ static const struct file_operations cxlflash_chr_fops = {
 	.owner          = THIS_MODULE,
 	.open           = cxlflash_chr_open,
 	.unlocked_ioctl	= cxlflash_chr_ioctl,
-	.compat_ioctl	= cxlflash_chr_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 /**
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index fdbda5c05aa0..80c5a235d193 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -613,7 +613,7 @@ static int __init esas2r_init(void)
 
 /* Handle ioctl calls to "/proc/scsi/esas2r/ATTOnode" */
 static const struct file_operations esas2r_proc_fops = {
-	.compat_ioctl	= esas2r_proc_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.unlocked_ioctl = esas2r_proc_ioctl,
 };
 
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index e338d7a4f571..c0a1a1218c56 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -3988,9 +3988,7 @@ static const struct file_operations pmcraid_fops = {
 	.open = pmcraid_chr_open,
 	.fasync = pmcraid_chr_fasync,
 	.unlocked_ioctl = pmcraid_chr_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = pmcraid_chr_ioctl,
-#endif
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek = noop_llseek,
 };
 
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 92c2914239e3..1663c163edca 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -567,9 +567,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 static const struct file_operations ion_fops = {
 	.owner          = THIS_MODULE,
 	.unlocked_ioctl = ion_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= ion_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 };
 
 static int debug_shrink_set(void *data, u64 val)
diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
index 6a33aaa1a49f..fd0ea4dbcb91 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -494,7 +494,7 @@ static const struct file_operations vme_user_fops = {
 	.write = vme_user_write,
 	.llseek = vme_user_llseek,
 	.unlocked_ioctl = vme_user_unlocked_ioctl,
-	.compat_ioctl = vme_user_unlocked_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.mmap = vme_user_mmap,
 };
 
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 17c64fccbb10..eb97acf09868 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -684,7 +684,7 @@ static const struct file_operations tee_fops = {
 	.open = tee_open,
 	.release = tee_release,
 	.unlocked_ioctl = tee_ioctl,
-	.compat_ioctl = tee_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static void tee_release_device(struct device *dev)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 9e9caff905d5..d48c032580d0 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -724,7 +724,7 @@ static const struct file_operations wdm_fops = {
 	.release =	wdm_release,
 	.poll =		wdm_poll,
 	.unlocked_ioctl = wdm_ioctl,
-	.compat_ioctl = wdm_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.llseek =	noop_llseek,
 };
 
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 4942122b2346..bbd0308b13f5 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -2220,9 +2220,7 @@ static const struct file_operations fops = {
 	.release	= usbtmc_release,
 	.flush		= usbtmc_flush,
 	.unlocked_ioctl	= usbtmc_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= usbtmc_ioctl,
-#endif
+	.compat_ioctl	= compat_ptr_ioctl,
 	.fasync         = usbtmc_fasync,
 	.poll           = usbtmc_poll,
 	.llseek		= default_llseek,
diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 8ba726e600e9..fbf02bf60f62 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -703,7 +703,7 @@ static const struct file_operations fsl_hv_fops = {
 	.poll = fsl_hv_poll,
 	.read = fsl_hv_read,
 	.unlocked_ioctl = fsl_hv_ioctl,
-	.compat_ioctl = fsl_hv_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 };
 
 static struct miscdevice fsl_hv_misc_dev = {
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 120e4340792a..162ea4b6b417 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2307,7 +2307,7 @@ static const struct super_operations btrfs_super_ops = {
 static const struct file_operations btrfs_ctl_fops = {
 	.open = btrfs_control_open,
 	.unlocked_ioctl	 = btrfs_control_ioctl,
-	.compat_ioctl = btrfs_control_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.owner	 = THIS_MODULE,
 	.llseek = noop_llseek,
 };
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 7c060cb22aa3..a493b957713f 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1785,7 +1785,7 @@ const struct file_operations ceph_dir_fops = {
 	.open = ceph_open,
 	.release = ceph_release,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl = ceph_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
 	.fsync = ceph_fsync,
 	.lock = ceph_lock,
 	.flock = ceph_flock,
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 9f53c3d99304..9b5fe7eee3c1 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -2112,7 +2112,7 @@ const struct file_operations ceph_file_fops = {
 	.splice_read = generic_file_splice_read,
 	.splice_write = iter_file_splice_write,
 	.unlocked_ioctl = ceph_ioctl,
-	.compat_ioctl	= ceph_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.fallocate	= ceph_fallocate,
 	.copy_file_range = ceph_copy_file_range,
 };
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 9971a35cf1ef..dcdb26068b71 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2354,7 +2354,7 @@ const struct file_operations fuse_dev_operations = {
 	.release	= fuse_dev_release,
 	.fasync		= fuse_dev_fasync,
 	.unlocked_ioctl = fuse_dev_ioctl,
-	.compat_ioctl   = fuse_dev_ioctl,
+	.compat_ioctl   = compat_ptr_ioctl,
 };
 EXPORT_SYMBOL_GPL(fuse_dev_operations);
 
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index a90bb19dcfa2..a55aa029a308 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -523,7 +523,7 @@ static const struct file_operations fanotify_fops = {
 	.fasync		= NULL,
 	.release	= fanotify_release,
 	.unlocked_ioctl	= fanotify_ioctl,
-	.compat_ioctl	= fanotify_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
 };
 
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 89800fc7dc9d..f93dcf8c996f 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1901,7 +1901,7 @@ static const struct file_operations userfaultfd_fops = {
 	.poll		= userfaultfd_poll,
 	.read		= userfaultfd_read,
 	.unlocked_ioctl = userfaultfd_ioctl,
-	.compat_ioctl	= userfaultfd_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 	.llseek		= noop_llseek,
 };
 
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index abca57040f37..3b2f6ea44397 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1323,7 +1323,7 @@ static const struct file_operations rfkill_fops = {
 	.release	= rfkill_fop_release,
 #ifdef CONFIG_RFKILL_INPUT
 	.unlocked_ioctl	= rfkill_fop_ioctl,
-	.compat_ioctl	= rfkill_fop_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
 #endif
 	.llseek		= no_llseek,
 };
-- 
2.20.0


^ permalink raw reply related

* [PATCH v3 00/26] compat_ioctl: cleanups
From: Arnd Bergmann @ 2019-04-16 20:19 UTC (permalink / raw)
  To: Alexander Viro
  Cc: linux-fsdevel, y2038, linux-kernel, Arnd Bergmann,
	David S. Miller, Greg Kroah-Hartman, Karsten Keil,
	James E.J. Bottomley, Martin K. Petersen, Marcel Holtmann, netdev,
	devel, linux-integrity, qat-linux, linux-crypto, linux-media,
	dri-devel, linux1394-devel, amd-gfx, linux-input, linux-usb,
	linux-arm-kernel, linux-ide, linux-iio, linux-rdma, linuxppc-dev,
	linux-mtd, linux-ppp, linux-nvme, platform-driver-x86,
	linux-remoteproc, linux-rtc, linux-s390, sparclinux, linux-scsi,
	linux-fbdev, linux-btrfs, ceph-devel, linux-bluetooth,
	linux-wireless, alsa-devel

Hi Al,

It took me way longer than I had hoped to revisit this series, see
https://lore.kernel.org/lkml/20180912150142.157913-1-arnd@arndb.de/
for the previously posted version.

I've come to the point where all conversion handlers and most
COMPATIBLE_IOCTL() entries are gone from this file, but for
now, this series only has the parts that have either been reviewed
previously, or that are simple enough to include.

The main missing piece is the SG_IO/SG_GET_REQUEST_TABLE conversion.
I'll post the patches I made for that later, as they need more
testing and review from the scsi maintainers.

I hope you can still take these for the coming merge window, unless
new problems come up.

      Arnd

Arnd Bergmann (26):
  compat_ioctl: pppoe: fix PPPOEIOCSFWD handling
  compat_ioctl: move simple ppp command handling into driver
  compat_ioctl: avoid unused function warning for do_ioctl
  compat_ioctl: move PPPIOCSCOMPRESS32 to ppp-generic.c
  compat_ioctl: move PPPIOCSPASS32/PPPIOCSACTIVE32 to ppp_generic.c
  compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
  compat_ioctl: move rtc handling into rtc-dev.c
  compat_ioctl: add compat_ptr_ioctl()
  compat_ioctl: move drivers to compat_ptr_ioctl
  compat_ioctl: use correct compat_ptr() translation in drivers
  ceph: fix compat_ioctl for ceph_dir_operations
  compat_ioctl: move more drivers to compat_ptr_ioctl
  compat_ioctl: move tape handling into drivers
  compat_ioctl: move ATYFB_CLK handling to atyfb driver
  compat_ioctl: move isdn/capi ioctl translation into driver
  compat_ioctl: move rfcomm handlers into driver
  compat_ioctl: move hci_sock handlers into driver
  compat_ioctl: remove HCIUART handling
  compat_ioctl: remove HIDIO translation
  compat_ioctl: remove translation for sound ioctls
  compat_ioctl: remove IGNORE_IOCTL()
  compat_ioctl: remove /dev/random commands
  compat_ioctl: remove joystick ioctl translation
  compat_ioctl: remove PCI ioctl translation
  compat_ioctl: remove /dev/raw ioctl translation
  compat_ioctl: remove last RAID handling code

 Documentation/networking/ppp_generic.txt    |   2 +
 arch/um/drivers/hostaudio_kern.c            |   1 +
 drivers/android/binder.c                    |   2 +-
 drivers/char/ppdev.c                        |  12 +-
 drivers/char/random.c                       |   1 +
 drivers/char/tpm/tpm_vtpm_proxy.c           |  12 +-
 drivers/crypto/qat/qat_common/adf_ctl_drv.c |   2 +-
 drivers/dma-buf/dma-buf.c                   |   4 +-
 drivers/dma-buf/sw_sync.c                   |   2 +-
 drivers/dma-buf/sync_file.c                 |   2 +-
 drivers/firewire/core-cdev.c                |  12 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c    |   2 +-
 drivers/hid/hidraw.c                        |   4 +-
 drivers/hid/usbhid/hiddev.c                 |  11 +-
 drivers/hwtracing/stm/core.c                |  12 +-
 drivers/ide/ide-tape.c                      |  31 +-
 drivers/iio/industrialio-core.c             |   2 +-
 drivers/infiniband/core/uverbs_main.c       |   4 +-
 drivers/isdn/capi/capi.c                    |  31 +
 drivers/isdn/i4l/isdn_ppp.c                 |  14 +-
 drivers/media/rc/lirc_dev.c                 |   4 +-
 drivers/mfd/cros_ec_dev.c                   |   4 +-
 drivers/misc/cxl/flash.c                    |   8 +-
 drivers/misc/genwqe/card_dev.c              |  23 +-
 drivers/misc/mei/main.c                     |  22 +-
 drivers/misc/vmw_vmci/vmci_host.c           |   2 +-
 drivers/mtd/ubi/cdev.c                      |  36 +-
 drivers/net/ppp/ppp_generic.c               |  99 +++-
 drivers/net/ppp/pppoe.c                     |   7 +
 drivers/net/ppp/pptp.c                      |   3 +
 drivers/net/tap.c                           |  12 +-
 drivers/nvdimm/bus.c                        |   4 +-
 drivers/nvme/host/core.c                    |   2 +-
 drivers/pci/switch/switchtec.c              |   2 +-
 drivers/platform/x86/wmi.c                  |   2 +-
 drivers/rpmsg/rpmsg_char.c                  |   4 +-
 drivers/rtc/dev.c                           |  13 +-
 drivers/rtc/rtc-vr41xx.c                    |  10 +
 drivers/s390/char/tape_char.c               |  41 +-
 drivers/sbus/char/display7seg.c             |   2 +-
 drivers/sbus/char/envctrl.c                 |   4 +-
 drivers/scsi/3w-xxxx.c                      |   4 +-
 drivers/scsi/cxlflash/main.c                |   2 +-
 drivers/scsi/esas2r/esas2r_main.c           |   2 +-
 drivers/scsi/megaraid/megaraid_mm.c         |  28 +-
 drivers/scsi/osst.c                         |  34 +-
 drivers/scsi/pmcraid.c                      |   4 +-
 drivers/scsi/st.c                           |  35 +-
 drivers/staging/android/ion/ion.c           |   4 +-
 drivers/staging/pi433/pi433_if.c            |  12 +-
 drivers/staging/vme/devices/vme_user.c      |   2 +-
 drivers/tee/tee_core.c                      |   2 +-
 drivers/usb/class/cdc-wdm.c                 |   2 +-
 drivers/usb/class/usbtmc.c                  |   4 +-
 drivers/usb/core/devio.c                    |  16 +-
 drivers/usb/gadget/function/f_fs.c          |  12 +-
 drivers/vfio/vfio.c                         |  39 +-
 drivers/vhost/net.c                         |  12 +-
 drivers/vhost/scsi.c                        |  12 +-
 drivers/vhost/test.c                        |  12 +-
 drivers/vhost/vsock.c                       |  12 +-
 drivers/video/fbdev/aty/atyfb_base.c        |  12 +-
 drivers/virt/fsl_hypervisor.c               |   2 +-
 fs/btrfs/super.c                            |   2 +-
 fs/ceph/dir.c                               |   1 +
 fs/ceph/file.c                              |   2 +-
 fs/compat_ioctl.c                           | 602 +-------------------
 fs/fat/file.c                               |  13 +-
 fs/fuse/dev.c                               |   2 +-
 fs/notify/fanotify/fanotify_user.c          |   2 +-
 fs/userfaultfd.c                            |   2 +-
 include/linux/fs.h                          |   7 +
 include/linux/if_pppox.h                    |   2 +
 include/linux/mtio.h                        |  58 ++
 include/uapi/linux/ppp-ioctl.h              |   2 +
 include/uapi/linux/ppp_defs.h               |  14 +
 net/bluetooth/hci_sock.c                    |  21 +-
 net/bluetooth/rfcomm/sock.c                 |  14 +-
 net/l2tp/l2tp_ppp.c                         |   3 +
 net/rfkill/core.c                           |   2 +-
 sound/core/oss/pcm_oss.c                    |   4 +
 sound/oss/dmasound/dmasound_core.c          |   2 +
 82 files changed, 452 insertions(+), 1034 deletions(-)
 create mode 100644 include/linux/mtio.h

-- 
2.20.0

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-integrity@vger.kernel.org
Cc: qat-linux@intel.com
Cc: linux-crypto@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux1394-devel@lists.sourceforge.net
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-input@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ide@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mtd@lists.infradead.org
Cc: linux-ppp@vger.kernel.org
Cc: linux-nvme@lists.infradead.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-remoteproc@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-btrfs@vger.kernel.org
Cc: ceph-devel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-bluetooth@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: alsa-devel@alsa-project.org
Cc: y2038@lists.linaro.org

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Johannes Berg @ 2019-04-16 19:13 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Toke Høiland-Jørgensen, Arend Van Spriel, Felix Fietkau,
	linux-wireless, Eric Dumazet, netdev
In-Reply-To: <20190416131346.u2uolljlrd5t2jro@gondor.apana.org.au>

On Tue, 2019-04-16 at 21:13 +0800, Herbert Xu wrote:
> On Tue, Apr 16, 2019 at 11:39:56AM +0200, Johannes Berg wrote:
> > 
> > > not to mention
> > > breaking certain applications that rely on congestion feedback?
> > 
> > This I don't understand. The congestion feedback happens through socket
> > buffer space etc. which is still there (as long as nobody sneaks in an
> > skb_orphan() call)
> 
> The congestion control happens at two levels.  You are right that
> the socket buffer acts as one limit.  However, other applications
> may also rely on the TX queue being full as the throttle (by setting
> a sufficiently large socket buffer size).

I'm not sure how they'd even realize this, other than packets getting
dropped? Which of course we do here as well, we didn't invent something
that let's us expand memory arbitrarily ;-)

johannes


^ permalink raw reply

* Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
From: Johannes Berg @ 2019-04-16 19:11 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless
In-Reply-To: <7338263c-9e01-1559-888f-adcb4b7c8ca1@wetzel-home.de>

On Tue, 2019-04-16 at 20:28 +0200, Alexander Wetzel wrote:

> They can enable the mode when a key with IEEE80211_KEY_FLAG_NO_AUTO_TX 
> set

This I agree with.

> and quiet it again as soon as they get a MPDU using the new KeyID.

This isn't true, afaict. You need to be sure that no MPDUs remain using
the old key ID, not just that the new key ID showed up.

> Since switching back to normal doesn't have to be done immediately a 
> asyc call from Tx path or even a worker should do the job just fine.

Sure.

> Btw:
> This also means we'll have to update the merged mac80211 Extended Key ID 
> support: We can only enable it for cards without HW crypto when they do 
> not set AMPDU_AGGREGATION. With the updated userspace these cards will 
> start using Extended Key ID with the already merged patches.

I was going to say this is fine, but no, of course not ... we shouldn't
use different key id in the same A-MPDU.

That said, I'd be very surprised if there are any such drivers, except
in corner cases (like loading some drivers like ath9k or iwlwifi with
swcrypto=1 or so)

> Of those only hwsim and brcmsmac seems to support AMPDU and only 
> brcmsmac relly needs the fix to not lose some packets when rekeying.

I can't believe that brcmsmac has no HW crypto support?

Anyway, a patch - even if it serves mostly as documentation - would be
most welcome.

> I assume we still have to wait till the API is in mainline (probably 
> 5.2) to ask hostapd/wpa_supplicant to merge the patches?

No, mac80211-next is (usually?) good enough.

johannes


^ permalink raw reply

* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
From: Toke Høiland-Jørgensen @ 2019-04-16 19:07 UTC (permalink / raw)
  To: Erik Stromdahl, kvalo, linux-wireless, ath10k; +Cc: yiboz
In-Reply-To: <8772dfe6-3f52-05ca-f55a-b7eae14fa7ac@gmail.com>

Erik Stromdahl <erik.stromdahl@gmail.com> writes:

> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>> 
>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>> queue could result in performance penalties on some SMP systems.
>>>
>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>> in mac80211 will be held by the CPU iterating the current queue.
>>>
>>> This will lock up other CPUs trying to push new messages on the TX
>>> queue.
>>>
>>> Instead of iterating the queue we fetch just one packet at the time,
>>> resulting in minimal starvation of the other CPUs.
>> 
>> Did you test this with Felix' patches reducing the time the lock is held
>> in mac80211?
>> 
>> -Toke
>> 
> Hi Toke,
>
> I am not aware of these patches. Can you please point them out for me?

They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
mac80211-next :)

-Toke

^ permalink raw reply

* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
From: Erik Stromdahl @ 2019-04-16 18:54 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, kvalo, linux-wireless, ath10k; +Cc: yiboz
In-Reply-To: <87bm1qq92j.fsf@toke.dk>



On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> 
>> Iterating the TX queue and thereby dequeuing all available packets in the
>> queue could result in performance penalties on some SMP systems.
>>
>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>> in mac80211 will be held by the CPU iterating the current queue.
>>
>> This will lock up other CPUs trying to push new messages on the TX
>> queue.
>>
>> Instead of iterating the queue we fetch just one packet at the time,
>> resulting in minimal starvation of the other CPUs.
> 
> Did you test this with Felix' patches reducing the time the lock is held
> in mac80211?
> 
> -Toke
> 
Hi Toke,

I am not aware of these patches. Can you please point them out for me?

--
Erik

^ permalink raw reply

* Re: iwlwifi: BUG: unable to handle kernel paging request at ffffc90000e1c808
From: Michal Hocko @ 2019-04-16 18:36 UTC (permalink / raw)
  To: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, linux-wireless
  Cc: LKML
In-Reply-To: <20190412094908.GA5223@dhcp22.suse.cz>

On Fri 12-04-19 11:49:08, Michal Hocko wrote:
> On Thu 11-04-19 23:07:20, Michal Hocko wrote:
> > Hi,
> > I have just encountered the following splat with 5.1-rc4. Is this a
> > known problem? Greping throug older logs show that "Error sending SCAN_CFG_CMD"
> > is not new. In older kernels it was followed by "Start IWL Error Log Dump:"
> > but it oopsed now. I was running 5.0 previously. Let me know if you need
> > more information.
> 
> FTR It happened again with 5.1.0-rc2-00001-ga3ac7917b730

Is there anything more I can provide to help with this issue. I had to
go back to 5.0 because the system is not really usable after the oops.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
From: Alexander Wetzel @ 2019-04-16 18:28 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <91d60fa7ba614c96fe2814375a28802e9165218b.camel@sipsolutions.net>

>> Could we not simply ask (at least) the drivers supporting Extended Key
>> IDs to implement a special for the remote STA invisible A-MPDU "stop" mode?
>> In this new mode each A-MPDU would simply be build out of a single MPDU.
>> We then could keep Block-Ack active and we don't have to tell the remote
>> STA anything about that decision. After all a A-MPDU with only one MPDU
>> is allowed...
>> We then could tell the driver to switch to this mode when installing the
>> new key and out of it again when we have activated the new key for Tx.
>> That should be perfectly fine to run only in the control path and
>> comparable simple to set up, too.
> 
> Sounds doable, but I'm still debating if we even should give them a
> signal - they have all the information in a sense, and do we have a good
> idea of when we can go out of this mode again?

Handling that on the Tx path would be tricky, but I you are right: 
Drivers should be able to handle that as it is on the control path, too.

They can enable the mode when a key with IEEE80211_KEY_FLAG_NO_AUTO_TX 
set and quiet it again as soon as they get a MPDU using the new KeyID.
Since switching back to normal doesn't have to be done immediately a 
asyc call from Tx path or even a worker should do the job just fine.

Btw:
This also means we'll have to update the merged mac80211 Extended Key ID 
support: We can only enable it for cards without HW crypto when they do 
not set AMPDU_AGGREGATION. With the updated userspace these cards will 
start using Extended Key ID with the already merged patches.

Drivers which should be able to use Extended Key ID with the two merged 
patches seem to be:
admtek/adm8211.c
ath/ar5523/ar5523.c
broadcom/b43legacy/main.c
broadcom/brcm80211/brcmsmac/mac80211_if.c
mac80211_hwsim.c
marvell/libertas_tf/main.c
ralink/rt2x00/rt2400pci.c
ralink/rt2x00/rt2500pci.c
realtek/rtl818x/rtl8180/dev.c
realtek/rtl818x/rtl8187/dev.c
zydas/zd1211rw/zd_mac.c

Of those only hwsim and brcmsmac seems to support AMPDU and only 
brcmsmac relly needs the fix to not lose some packets when rekeying.


>> That also sounds like something all drivers supporting A-MPDU should be
>> able to pull off somehow. (But then I've never even looked at more than
>> ath9k and iwlwifi so far for A-MPDU, and at those not close, yet.)
>> Do you see any problems with that solution and/or a better idea?
> 
> It ought to be possible, or if not then the device just can't support
> extended key ID?

Agree. Or drivers can decide to deny A-MPDU setup when the key has been 
installed with IEEE80211_KEY_FLAG_NO_AUTO_TX when they want.

>> Also would you prefer we first sort out the A-MPDU issue prior I adding
>> test cases to the hostapd/wpa_supplicant or the other way round?
> 
> I think adding the code to hostapd/wpa_s is fine - right now we're
> obviously OK since we have no drivers using extended key ID that use
> hardware crypto, and if we have software crypto there's no problem
> either way, even if A-MPDUs are built (which is probably not the case
> for any such drivers anyway.)
> 
> In a sense I'd prefer getting the necessary bits and pieces elsewhere in
> the stack upstream first since that's a prerequisite for anyone
> else being able to easily work on this, and that's something we need for
> drivers to pick it up.

Ok:-)

I assume we still have to wait till the API is in mainline (probably 
5.2) to ask hostapd/wpa_supplicant to merge the patches?

At the moment I'm planning to get the patches merge ready and posted as 
RFC till 5.2 is out. I'm also planing to keep Extended Key ID for Mesh 
networks till later, so we can focus on AP/STA.


Alexander


^ permalink raw reply

* Re: [PATCH wireless-drivers] rtlwifi: rtl8723ae: Fix missing break in switch statement
From: Gustavo A. R. Silva @ 2019-04-16 16:03 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Ping-Ke Shih, David S. Miller, linux-wireless, netdev,
	linux-kernel, Kees Cook
In-Reply-To: <87ftqi54cx.fsf@kamboji.qca.qualcomm.com>



On 4/16/19 10:58 AM, Kalle Valo wrote:

>>> You have marked this for wireless-drivers but the commit is 2.5 years
>>> old, so I think wireless-drivers-next is more approriate.
>>>
>>
>> Yeah. This can actually be applied to both of them.  What I'd like is
>> to have this applied to stable, so I thought wireless-drivers was a
>> more straightforward way.
> 
> Sure, makes sense. But I just want to keep the number of patches going
> to wireless-drivers to the minimum, I really want to avoid conflicts as
> much as possible. That's why I'm keeping the bar high for w-d.
> 

Yep. I've got it.

I just sent v2:

https://lore.kernel.org/patchwork/patch/1062748/

Thanks
--
Gustavo

^ permalink raw reply

* Re: [PATCH wireless-drivers] rtlwifi: rtl8723ae: Fix missing break in switch statement
From: Kalle Valo @ 2019-04-16 15:58 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Ping-Ke Shih, David S. Miller, linux-wireless, netdev,
	linux-kernel, Kees Cook
In-Reply-To: <90c56883-6ef3-9f9e-5231-6c2fc65d747d@embeddedor.com>

"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> On 4/15/19 10:36 PM, Kalle Valo wrote:
>> "Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:
>> 
>>> Add missing break statement in order to prevent the code from falling
>>> through to case 0x1025, and erroneously setting rtlhal->oem_id to
>>> RT_CID_819X_ACER when rtlefuse->eeprom_svid is equal to 0x10EC and
>>> none of the cases in switch (rtlefuse->eeprom_smid) match.
>>>
>>> This bug was found thanks to the ongoing efforts to enable
>>> -Wimplicit-fallthrough.
>>>
>>> Fixes: 238ad2ddf34b ("rtlwifi: rtl8723ae: Clean up the hardware info routine")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> 
>> You have marked this for wireless-drivers but the commit is 2.5 years
>> old, so I think wireless-drivers-next is more approriate.
>> 
>
> Yeah. This can actually be applied to both of them.  What I'd like is
> to have this applied to stable, so I thought wireless-drivers was a
> more straightforward way.

Sure, makes sense. But I just want to keep the number of patches going
to wireless-drivers to the minimum, I really want to avoid conflicts as
much as possible. That's why I'm keeping the bar high for w-d.

> Anyway, I'll update the tag and respin.

Thanks.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH wireless-drivers] rtlwifi: rtl8723ae: Fix missing break in switch statement
From: Gustavo A. R. Silva @ 2019-04-16 15:08 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Ping-Ke Shih, David S. Miller, linux-wireless, netdev,
	linux-kernel, Kees Cook
In-Reply-To: <87h8ay8vu9.fsf@kamboji.qca.qualcomm.com>



On 4/15/19 10:36 PM, Kalle Valo wrote:
> "Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:
> 
>> Add missing break statement in order to prevent the code from falling
>> through to case 0x1025, and erroneously setting rtlhal->oem_id to
>> RT_CID_819X_ACER when rtlefuse->eeprom_svid is equal to 0x10EC and
>> none of the cases in switch (rtlefuse->eeprom_smid) match.
>>
>> This bug was found thanks to the ongoing efforts to enable
>> -Wimplicit-fallthrough.
>>
>> Fixes: 238ad2ddf34b ("rtlwifi: rtl8723ae: Clean up the hardware info routine")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> 
> You have marked this for wireless-drivers but the commit is 2.5 years
> old, so I think wireless-drivers-next is more approriate.
> 

Yeah. This can actually be applied to both of them.  What I'd like is
to have this applied to stable, so I thought wireless-drivers was a
more straightforward way.

Anyway, I'll update the tag and respin.

Thanks
--
Gustavo

^ permalink raw reply

* [PATCH v2 wireless-drivers-next] rtlwifi: rtl8723ae: Fix missing break in switch statement
From: Gustavo A. R. Silva @ 2019-04-16 15:17 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, David S. Miller, Larry Finger
  Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva,
	Kees Cook

Add missing break statement in order to prevent the code from falling
through to case 0x1025, and erroneously setting rtlhal->oem_id to
RT_CID_819X_ACER when rtlefuse->eeprom_svid is equal to 0x10EC and
none of the cases in switch (rtlefuse->eeprom_smid) match.

This bug was found thanks to the ongoing efforts to enable
-Wimplicit-fallthrough.

Fixes: 238ad2ddf34b ("rtlwifi: rtl8723ae: Clean up the hardware info routine")
Cc: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
Changes in v2:
 - Update tag from wireless-drivers to wireless-drivers-next

 drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
index 6bab162e1bb8..655460f61bbc 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
@@ -1675,6 +1675,7 @@ static void _rtl8723e_read_adapter_info(struct ieee80211_hw *hw,
 					rtlhal->oem_id = RT_CID_819X_LENOVO;
 					break;
 				}
+				break;
 			case 0x1025:
 				rtlhal->oem_id = RT_CID_819X_ACER;
 				break;
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Toke Høiland-Jørgensen @ 2019-04-16 13:18 UTC (permalink / raw)
  To: Herbert Xu, Johannes Berg
  Cc: Arend Van Spriel, Felix Fietkau, linux-wireless, Eric Dumazet,
	netdev
In-Reply-To: <20190416131346.u2uolljlrd5t2jro@gondor.apana.org.au>

Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Tue, Apr 16, 2019 at 11:39:56AM +0200, Johannes Berg wrote:
>> 
>> > not to mention
>> > breaking certain applications that rely on congestion feedback?
>> 
>> This I don't understand. The congestion feedback happens through socket
>> buffer space etc. which is still there (as long as nobody sneaks in an
>> skb_orphan() call)
>
> The congestion control happens at two levels. You are right that the
> socket buffer acts as one limit. However, other applications may also
> rely on the TX queue being full as the throttle (by setting a
> sufficiently large socket buffer size).

Do you happen to have an example of an application that does this that
could be used for testing? :)

-Toke

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Herbert Xu @ 2019-04-16 13:13 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Toke Høiland-Jørgensen, Arend Van Spriel, Felix Fietkau,
	linux-wireless, Eric Dumazet, netdev
In-Reply-To: <e4c1a3ca0c0c160b7c271ab83deebcf096f38497.camel@sipsolutions.net>

On Tue, Apr 16, 2019 at 11:39:56AM +0200, Johannes Berg wrote:
> 
> > not to mention
> > breaking certain applications that rely on congestion feedback?
> 
> This I don't understand. The congestion feedback happens through socket
> buffer space etc. which is still there (as long as nobody sneaks in an
> skb_orphan() call)

The congestion control happens at two levels.  You are right that
the socket buffer acts as one limit.  However, other applications
may also rely on the TX queue being full as the throttle (by setting
a sufficiently large socket buffer size).

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] staging: wilc1000: Avoid GFP_KERNEL allocation from atomic context.
From: Greg Kroah-Hartman @ 2019-04-16 11:15 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: Adham Abozaeid, Ajay Singh, linux-wireless
In-Reply-To: <1554641923-11503-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>

On Sun, Apr 07, 2019 at 09:58:43PM +0900, Tetsuo Handa wrote:
> Since wilc_set_multicast_list() is called with dev->addr_list_lock
> spinlock held, we can't use GFP_KERNEL memory allocation.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: e624c58cf8eb5116 ("staging: wilc1000: refactor code to avoid use of wilc_set_multicast_list global")

No need for so many sha1 characters, this is the canonical length to
use:
	e624c58cf8eb
thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Toke Høiland-Jørgensen @ 2019-04-16 10:02 UTC (permalink / raw)
  To: Johannes Berg, Herbert Xu
  Cc: Arend Van Spriel, Felix Fietkau, linux-wireless, Eric Dumazet,
	netdev
In-Reply-To: <e4c1a3ca0c0c160b7c271ab83deebcf096f38497.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Tue, 2019-04-16 at 17:37 +0800, Herbert Xu wrote:
>> On Tue, Apr 16, 2019 at 11:33:50AM +0200, Johannes Berg wrote:
>> > On Tue, 2019-04-16 at 10:33 +0100, Toke Høiland-Jørgensen wrote:
>> > > 
>> > > > It is true because we have an entire buffering layer in mac80211 (in
>> > > > this case at least) and never push back to the stack.
>> > > 
>> > > I'm wondering if we should be?
>> > 
>> > I don't think so? We'd just buffer packets in yet another place.
>> 
>> But you do realise that you're giving up on the rich queueing
>> functionality that Linux provides (net/sched),
>
> Yes, that was a trade-off we always knew about. The model that Linux
> provides is just not suited for wifi.

As explained at great length here:
https://www.usenix.org/conference/atc17/technical-sessions/presentation/hoilan-jorgesen
(you already know that of course, Johannes)

>> not to mention
>> breaking certain applications that rely on congestion feedback?
>
> This I don't understand. The congestion feedback happens through socket
> buffer space etc. which is still there (as long as nobody sneaks in an
> skb_orphan() call)

Sure, for TCP, the TSQ mechanism should keep the upper-level queue low
as long as the SKBs are alive. But is this also the case for UDP?

-Toke

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Johannes Berg @ 2019-04-16  9:39 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Toke Høiland-Jørgensen, Arend Van Spriel, Felix Fietkau,
	linux-wireless, Eric Dumazet, netdev
In-Reply-To: <20190416093707.dtlwcmitzqopaeaw@gondor.apana.org.au>

On Tue, 2019-04-16 at 17:37 +0800, Herbert Xu wrote:
> On Tue, Apr 16, 2019 at 11:33:50AM +0200, Johannes Berg wrote:
> > On Tue, 2019-04-16 at 10:33 +0100, Toke Høiland-Jørgensen wrote:
> > > 
> > > > It is true because we have an entire buffering layer in mac80211 (in
> > > > this case at least) and never push back to the stack.
> > > 
> > > I'm wondering if we should be?
> > 
> > I don't think so? We'd just buffer packets in yet another place.
> 
> But you do realise that you're giving up on the rich queueing
> functionality that Linux provides (net/sched),

Yes, that was a trade-off we always knew about. The model that Linux
provides is just not suited for wifi.

> not to mention
> breaking certain applications that rely on congestion feedback?

This I don't understand. The congestion feedback happens through socket
buffer space etc. which is still there (as long as nobody sneaks in an
skb_orphan() call)

johannes


^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Toke Høiland-Jørgensen @ 2019-04-16  9:38 UTC (permalink / raw)
  To: Johannes Berg, Herbert Xu, Arend Van Spriel
  Cc: Felix Fietkau, linux-wireless, Eric Dumazet
In-Reply-To: <95f86cf69dee05a176625925657cf0df0e97b5c9.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Tue, 2019-04-16 at 10:33 +0100, Toke Høiland-Jørgensen wrote:
>> 
>> > It is true because we have an entire buffering layer in mac80211 (in
>> > this case at least) and never push back to the stack.
>> 
>> I'm wondering if we should be?
>
> I don't think so? We'd just buffer packets in yet another place.

Wouldn't we get pushback all the way to the application socket? I.e.,
an UDP application would get sendto() failures?

-Toke

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Herbert Xu @ 2019-04-16  9:37 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Toke Høiland-Jørgensen, Arend Van Spriel, Felix Fietkau,
	linux-wireless, Eric Dumazet, netdev
In-Reply-To: <95f86cf69dee05a176625925657cf0df0e97b5c9.camel@sipsolutions.net>

On Tue, Apr 16, 2019 at 11:33:50AM +0200, Johannes Berg wrote:
> On Tue, 2019-04-16 at 10:33 +0100, Toke Høiland-Jørgensen wrote:
> > 
> > > It is true because we have an entire buffering layer in mac80211 (in
> > > this case at least) and never push back to the stack.
> > 
> > I'm wondering if we should be?
> 
> I don't think so? We'd just buffer packets in yet another place.

But you do realise that you're giving up on the rich queueing
functionality that Linux provides (net/sched), not to mention
breaking certain applications that rely on congestion feedback?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Johannes Berg @ 2019-04-16  9:33 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Herbert Xu, Arend Van Spriel
  Cc: Felix Fietkau, linux-wireless, Eric Dumazet
In-Reply-To: <87ef62uwfm.fsf@toke.dk>

On Tue, 2019-04-16 at 10:33 +0100, Toke Høiland-Jørgensen wrote:
> 
> > It is true because we have an entire buffering layer in mac80211 (in
> > this case at least) and never push back to the stack.
> 
> I'm wondering if we should be?

I don't think so? We'd just buffer packets in yet another place.

johannes



^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Toke Høiland-Jørgensen @ 2019-04-16  9:33 UTC (permalink / raw)
  To: Johannes Berg, Herbert Xu, Arend Van Spriel
  Cc: Felix Fietkau, linux-wireless, Eric Dumazet
In-Reply-To: <eab39dd804405854997e50102f3d0ff925feb8d2.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Tue, 2019-04-16 at 16:36 +0800, Herbert Xu wrote:
>> On Tue, Apr 16, 2019 at 10:04:24AM +0200, Arend Van Spriel wrote:
>> > 
>> > I was just writing up an email clarifying my question. But let me summarize
>> > this email thread. The patch from Felix adds this flag in mac80211 for
>> > drivers that indicate to support pulling packets from the internal TXQ in
>> > mac80211. I found it is deprecated, but as Felix mentioned it is used in
>> > various parts of the network subsystem, ie. batman-adv, bridge, vlan, tunnel
>> > implementations. So its use seems to be restricted rather than deprecated.
>> > Given your response above I guess my question would be to get details about
>> > what you call "proper design" as I think you are saying with that it is not
>> > needed, right?
>> 
>> Essentially the only time it would be OK to use LLTX in its current
>> form is if you have no TX queue/congestion feedback which is clearly
>> not the case with wireless drivers.
>
> It is true because we have an entire buffering layer in mac80211 (in
> this case at least) and never push back to the stack.

I'm wondering if we should be? For instance, fq_codel returns
NET_XMIT_CN if it drops a packet from the same flow that it enqueued to.
We could conceivably do the same in mac80211, although we'd have to
carry the return value out through quite a few layers. Not sure if this
is worth it?

Eric, do you have any insight into what impact the _CN return has from
fq_codel?

-Toke

^ permalink raw reply

* Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
From: Johannes Berg @ 2019-04-16  9:31 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless
In-Reply-To: <577d4307-27ca-c5f5-8814-bbef515559e3@wetzel-home.de>

On Mon, 2019-04-15 at 22:09 +0200, Alexander Wetzel wrote:
> Honestly I don't like my own patch for the KeyId border signal, too 
> complex and not intuitive at all. But I wanted to have some code for 
> discussion and then got carried away a bit in what may have been not the 
> right direction.

:-)

> Could we not simply ask (at least) the drivers supporting Extended Key 
> IDs to implement a special for the remote STA invisible A-MPDU "stop" mode?
> In this new mode each A-MPDU would simply be build out of a single MPDU. 
> We then could keep Block-Ack active and we don't have to tell the remote 
> STA anything about that decision. After all a A-MPDU with only one MPDU 
> is allowed...
> We then could tell the driver to switch to this mode when installing the 
> new key and out of it again when we have activated the new key for Tx.
> That should be perfectly fine to run only in the control path and 
> comparable simple to set up, too.

Sounds doable, but I'm still debating if we even should give them a
signal - they have all the information in a sense, and do we have a good
idea of when we can go out of this mode again?

(FWIW, I'd probably call it "suspend" and "resume" or so)

> That also sounds like something all drivers supporting A-MPDU should be 
> able to pull off somehow. (But then I've never even looked at more than 
> ath9k and iwlwifi so far for A-MPDU, and at those not close, yet.)
> Do you see any problems with that solution and/or a better idea?

It ought to be possible, or if not then the device just can't support
extended key ID?

> Also would you prefer we first sort out the A-MPDU issue prior I adding 
> test cases to the hostapd/wpa_supplicant or the other way round?

I think adding the code to hostapd/wpa_s is fine - right now we're
obviously OK since we have no drivers using extended key ID that use
hardware crypto, and if we have software crypto there's no problem
either way, even if A-MPDUs are built (which is probably not the case
for any such drivers anyway.)

In a sense I'd prefer getting the necessary bits and pieces elsewhere in
the stack upstream first since that's a prerequisite for anyone
else being able to easily work on this, and that's something we need for
drivers to pick it up.

johannes


^ permalink raw reply

* Re: [RFC 3/3] mt76x02: remove bogus mutex usage
From: Lorenzo Bianconi @ 2019-04-16  9:29 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: linux-wireless, Felix Fietkau
In-Reply-To: <20190416091305.4218-4-sgruszka@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4053 bytes --]

> mac .start(), .stop() callbacks are never called concurrently with other
> mac callbacks. The only concurencly is with mt76 works which we cancel
> on stop() and schedule on start().
> 
> This fixes possible deadlock on cancel_delayed_work_sync(&dev->mac_work)
> as mac_work also take mutex.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  .../net/wireless/mediatek/mt76/mt76x0/pci.c   |  6 -----
>  .../net/wireless/mediatek/mt76/mt76x0/usb.c   | 22 +++++--------------
>  .../wireless/mediatek/mt76/mt76x2/pci_main.c  |  5 -----
>  .../wireless/mediatek/mt76/mt76x2/usb_main.c  | 10 ++-------
>  4 files changed, 7 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
> index 156d3d064ba0..ac979128386a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
> @@ -25,8 +25,6 @@ static int mt76x0e_start(struct ieee80211_hw *hw)
>  {
>  	struct mt76x02_dev *dev = hw->priv;
>  
> -	mutex_lock(&dev->mt76.mutex);
> -
>  	mt76x02_mac_start(dev);
>  	mt76x0_phy_calibrate(dev, true);
>  	ieee80211_queue_delayed_work(dev->mt76.hw, &dev->mac_work,
> @@ -35,8 +33,6 @@ static int mt76x0e_start(struct ieee80211_hw *hw)
>  				     MT_CALIBRATE_INTERVAL);
>  	set_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  
> -	mutex_unlock(&dev->mt76.mutex);
> -
>  	return 0;
>  }
>  
> @@ -62,10 +58,8 @@ static void mt76x0e_stop(struct ieee80211_hw *hw)
>  {
>  	struct mt76x02_dev *dev = hw->priv;
>  
> -	mutex_lock(&dev->mt76.mutex);
>  	clear_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  	mt76x0e_stop_hw(dev);
> -	mutex_unlock(&dev->mt76.mutex);
>  }
>  
>  static void

[..]

>  static const struct ieee80211_ops mt76x0u_ops = {
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> index 16dc8e2451b5..1b5caabebff5 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_main.c
> @@ -22,8 +22,6 @@ mt76x2_start(struct ieee80211_hw *hw)
>  	struct mt76x02_dev *dev = hw->priv;
>  	int ret;
>  
> -	mutex_lock(&dev->mt76.mutex);
> -
>  	ret = mt76x2_mac_start(dev);
>  	if (ret)
>  		goto out;

You can remove goto here and just return ret (same below)

> @@ -40,7 +38,6 @@ mt76x2_start(struct ieee80211_hw *hw)
>  	set_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  
>  out:
> -	mutex_unlock(&dev->mt76.mutex);
>  	return ret;
>  }
>  
> @@ -49,10 +46,8 @@ mt76x2_stop(struct ieee80211_hw *hw)
>  {
>  	struct mt76x02_dev *dev = hw->priv;
>  
> -	mutex_lock(&dev->mt76.mutex);
>  	clear_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  	mt76x2_stop_hardware(dev);
> -	mutex_unlock(&dev->mt76.mutex);
>  }
>  
>  static int
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
> index 0771de210c6a..32726b4906ea 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
> @@ -21,30 +21,24 @@ static int mt76x2u_start(struct ieee80211_hw *hw)
>  	struct mt76x02_dev *dev = hw->priv;
>  	int ret;
>  
> -	mutex_lock(&dev->mt76.mutex);
> -
>  	ret = mt76x2u_mac_start(dev);
>  	if (ret)
> -		goto out;
> +		return ret;
>  
>  	ieee80211_queue_delayed_work(mt76_hw(dev), &dev->mac_work,
>  				     MT_MAC_WORK_INTERVAL);
>  	set_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  
> -out:
> -	mutex_unlock(&dev->mt76.mutex);
> -	return ret;
> +	return 0;
>  }
>  
>  static void mt76x2u_stop(struct ieee80211_hw *hw)
>  {
>  	struct mt76x02_dev *dev = hw->priv;
>  
> -	mutex_lock(&dev->mt76.mutex);
>  	clear_bit(MT76_STATE_RUNNING, &dev->mt76.state);
>  	mt76u_stop_tx(&dev->mt76);
>  	mt76x2u_stop_hw(dev);
> -	mutex_unlock(&dev->mt76.mutex);
>  }
>  
>  static int
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Herbert Xu @ 2019-04-16  9:29 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Johannes Berg, Toke Høiland-Jørgensen, Felix Fietkau,
	linux-wireless, netdev
In-Reply-To: <cd0715bb-b75c-63b0-0ae4-858d14afd500@broadcom.com>

On Tue, Apr 16, 2019 at 11:17:53AM +0200, Arend Van Spriel wrote:
> On 4/16/2019 10:37 AM, Johannes Berg wrote:
> > It is true because we have an entire buffering layer in mac80211 (in
> > this case at least) and never push back to the stack.
> 
> Ok, so the crux is the "never push back to the stack" part? Well, the
> internal TXQ and how that is used is obviously enabling that ;-)

So assuming that these drivers all have a TX queue length of zero
and therefore do not make use of Linux queueing disciplines then
yes techincally LLTX is fine.

However, I must say that it is much better to provide real
congestion feedback to the stack when you can because otherwise
things like UDP may fall apart.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox