2022-01-31 11:00:08

by Luca Coelho

[permalink] [raw]
Subject: [PATCH for v5.17 0/2] iwlwifi: fixes intended for v5.17 2022-01-28 part 2

From: Luca Coelho <[email protected]>

Hi,

This is the second patchset with fixes for v5.17.

These are two fixes that I missed in the previous patchset.

The changes are:

* Remove deprecated feature that causes FW errors when enabled with new FWs;
* Fix SAR Geo FW command failure with 3160 devices.

As usual, I'm pushing this to a pending branch, for kbuild bot. And
since these are fixes for the rc series, please take them directly to
wireless-drivers.git, as we agreed. I'll assign them to you.

Cheers,
Luca.


Luca Coelho (2):
iwlwifi: remove deprecated broadcast filtering feature
iwlwifi: mvm: don't send SAR GEO command for 3160 devices

drivers/net/wireless/intel/iwlwifi/Kconfig | 13 -
drivers/net/wireless/intel/iwlwifi/fw/acpi.c | 11 +-
.../wireless/intel/iwlwifi/fw/api/commands.h | 5 -
.../wireless/intel/iwlwifi/fw/api/filter.h | 88 -------
drivers/net/wireless/intel/iwlwifi/fw/file.h | 2 -
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 3 +-
.../net/wireless/intel/iwlwifi/mvm/debugfs.c | 203 ---------------
drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 2 +-
.../net/wireless/intel/iwlwifi/mvm/mac80211.c | 240 ------------------
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 13 -
drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 1 -
11 files changed, 9 insertions(+), 572 deletions(-)

--
2.34.1


2022-01-31 11:00:09

by Luca Coelho

[permalink] [raw]
Subject: [PATCH for v5.17 2/2] iwlwifi: mvm: don't send SAR GEO command for 3160 devices

From: Luca Coelho <[email protected]>

SAR GEO offsets are not supported on 3160 devices. The code was
refactored and caused us to start sending the command anyway, which
causes a FW assertion failure. Fix that only considering this feature
supported on FW API with major version is 17 if the device is not
3160.

Additionally, fix the caller of iwl_mvm_sar_geo_init() so that it
checks for the return value, which it was ignoring.

Reported-by: Len Brown <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
Fixes: 78a19d5285d9 ("iwlwifi: mvm: Read the PPAG and SAR tables at INIT stage")
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/fw/acpi.c | 11 ++++++-----
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 3 ++-
drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
index 790c96df58cb..c17ab53fcd8f 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
/*
* Copyright (C) 2017 Intel Deutschland GmbH
- * Copyright (C) 2019-2021 Intel Corporation
+ * Copyright (C) 2019-2022 Intel Corporation
*/
#include <linux/uuid.h>
#include "iwl-drv.h"
@@ -888,10 +888,11 @@ bool iwl_sar_geo_support(struct iwl_fw_runtime *fwrt)
* only one using version 36, so skip this version entirely.
*/
return IWL_UCODE_SERIAL(fwrt->fw->ucode_ver) >= 38 ||
- IWL_UCODE_SERIAL(fwrt->fw->ucode_ver) == 17 ||
- (IWL_UCODE_SERIAL(fwrt->fw->ucode_ver) == 29 &&
- ((fwrt->trans->hw_rev & CSR_HW_REV_TYPE_MSK) ==
- CSR_HW_REV_TYPE_7265D));
+ (IWL_UCODE_SERIAL(fwrt->fw->ucode_ver) == 17 &&
+ fwrt->trans->hw_rev != CSR_HW_REV_TYPE_3160) ||
+ (IWL_UCODE_SERIAL(fwrt->fw->ucode_ver) == 29 &&
+ ((fwrt->trans->hw_rev & CSR_HW_REV_TYPE_MSK) ==
+ CSR_HW_REV_TYPE_7265D));
}
IWL_EXPORT_SYMBOL(iwl_sar_geo_support);

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
index f90d4662c164..8e10ba88afb3 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
/*
- * Copyright (C) 2005-2014, 2018-2021 Intel Corporation
+ * Copyright (C) 2005-2014, 2018-2022 Intel Corporation
* Copyright (C) 2013-2014 Intel Mobile Communications GmbH
* Copyright (C) 2016 Intel Deutschland GmbH
*/
@@ -329,6 +329,7 @@ enum {
#define CSR_HW_REV_TYPE_2x00 (0x0000100)
#define CSR_HW_REV_TYPE_105 (0x0000110)
#define CSR_HW_REV_TYPE_135 (0x0000120)
+#define CSR_HW_REV_TYPE_3160 (0x0000164)
#define CSR_HW_REV_TYPE_7265D (0x0000210)
#define CSR_HW_REV_TYPE_NONE (0x00001F0)
#define CSR_HW_REV_TYPE_QNJ (0x0000360)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 6f4690e56a46..ae589b3b8c46 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -1741,7 +1741,7 @@ int iwl_mvm_up(struct iwl_mvm *mvm)
ret = iwl_mvm_sar_init(mvm);
if (ret == 0)
ret = iwl_mvm_sar_geo_init(mvm);
- else if (ret < 0)
+ if (ret < 0)
goto error;

ret = iwl_mvm_sgom_init(mvm);
--
2.34.1

2022-01-31 11:01:35

by Luca Coelho

[permalink] [raw]
Subject: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

From: Luca Coelho <[email protected]>

This feature has been deprecated and should not be used anymore. With
newer firmwares, namely *-67.ucode and above, trying to use it causes an
assertion failure in the FW, similar to this:

[Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT

In order to prevent this feature from being used, remove it entirely
and get rid of the Kconfig option that
enables it (IWLWIFI_BCAST_FILTERING).

Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
Signed-off-by: Luca Coelho <[email protected]>
Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/net/wireless/intel/iwlwifi/Kconfig | 13 -
.../wireless/intel/iwlwifi/fw/api/commands.h | 5 -
.../wireless/intel/iwlwifi/fw/api/filter.h | 88 -------
drivers/net/wireless/intel/iwlwifi/fw/file.h | 2 -
.../net/wireless/intel/iwlwifi/mvm/debugfs.c | 203 ---------------
.../net/wireless/intel/iwlwifi/mvm/mac80211.c | 240 ------------------
drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 13 -
drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 1 -
8 files changed, 565 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
index c21c0c68849a..85e704283755 100644
--- a/drivers/net/wireless/intel/iwlwifi/Kconfig
+++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
@@ -80,19 +80,6 @@ config IWLWIFI_OPMODE_MODULAR
comment "WARNING: iwlwifi is useless without IWLDVM or IWLMVM"
depends on IWLDVM=n && IWLMVM=n

-config IWLWIFI_BCAST_FILTERING
- bool "Enable broadcast filtering"
- depends on IWLMVM
- help
- Say Y here to enable default bcast filtering configuration.
-
- Enabling broadcast filtering will drop any incoming wireless
- broadcast frames, except some very specific predefined
- patterns (e.g. incoming arp requests).
-
- If unsure, don't enable this option, as some programs might
- expect incoming broadcasts for their normal operations.
-
menu "Debugging Options"

config IWLWIFI_DEBUG
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h b/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
index 0703e41403a6..35b8856e511f 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
@@ -501,11 +501,6 @@ enum iwl_legacy_cmds {
*/
DEBUG_LOG_MSG = 0xf7,

- /**
- * @BCAST_FILTER_CMD: &struct iwl_bcast_filter_cmd
- */
- BCAST_FILTER_CMD = 0xcf,
-
/**
* @MCAST_FILTER_CMD: &struct iwl_mcast_filter_cmd
*/
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h b/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
index dd62a63956b3..e44c70b7c790 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
@@ -36,92 +36,4 @@ struct iwl_mcast_filter_cmd {
u8 addr_list[0];
} __packed; /* MCAST_FILTERING_CMD_API_S_VER_1 */

-#define MAX_BCAST_FILTERS 8
-#define MAX_BCAST_FILTER_ATTRS 2
-
-/**
- * enum iwl_mvm_bcast_filter_attr_offset - written by fw for each Rx packet
- * @BCAST_FILTER_OFFSET_PAYLOAD_START: offset is from payload start.
- * @BCAST_FILTER_OFFSET_IP_END: offset is from ip header end (i.e.
- * start of ip payload).
- */
-enum iwl_mvm_bcast_filter_attr_offset {
- BCAST_FILTER_OFFSET_PAYLOAD_START = 0,
- BCAST_FILTER_OFFSET_IP_END = 1,
-};
-
-/**
- * struct iwl_fw_bcast_filter_attr - broadcast filter attribute
- * @offset_type: &enum iwl_mvm_bcast_filter_attr_offset.
- * @offset: starting offset of this pattern.
- * @reserved1: reserved
- * @val: value to match - big endian (MSB is the first
- * byte to match from offset pos).
- * @mask: mask to match (big endian).
- */
-struct iwl_fw_bcast_filter_attr {
- u8 offset_type;
- u8 offset;
- __le16 reserved1;
- __be32 val;
- __be32 mask;
-} __packed; /* BCAST_FILTER_ATT_S_VER_1 */
-
-/**
- * enum iwl_mvm_bcast_filter_frame_type - filter frame type
- * @BCAST_FILTER_FRAME_TYPE_ALL: consider all frames.
- * @BCAST_FILTER_FRAME_TYPE_IPV4: consider only ipv4 frames
- */
-enum iwl_mvm_bcast_filter_frame_type {
- BCAST_FILTER_FRAME_TYPE_ALL = 0,
- BCAST_FILTER_FRAME_TYPE_IPV4 = 1,
-};
-
-/**
- * struct iwl_fw_bcast_filter - broadcast filter
- * @discard: discard frame (1) or let it pass (0).
- * @frame_type: &enum iwl_mvm_bcast_filter_frame_type.
- * @reserved1: reserved
- * @num_attrs: number of valid attributes in this filter.
- * @attrs: attributes of this filter. a filter is considered matched
- * only when all its attributes are matched (i.e. AND relationship)
- */
-struct iwl_fw_bcast_filter {
- u8 discard;
- u8 frame_type;
- u8 num_attrs;
- u8 reserved1;
- struct iwl_fw_bcast_filter_attr attrs[MAX_BCAST_FILTER_ATTRS];
-} __packed; /* BCAST_FILTER_S_VER_1 */
-
-/**
- * struct iwl_fw_bcast_mac - per-mac broadcast filtering configuration.
- * @default_discard: default action for this mac (discard (1) / pass (0)).
- * @reserved1: reserved
- * @attached_filters: bitmap of relevant filters for this mac.
- */
-struct iwl_fw_bcast_mac {
- u8 default_discard;
- u8 reserved1;
- __le16 attached_filters;
-} __packed; /* BCAST_MAC_CONTEXT_S_VER_1 */
-
-/**
- * struct iwl_bcast_filter_cmd - broadcast filtering configuration
- * @disable: enable (0) / disable (1)
- * @max_bcast_filters: max number of filters (MAX_BCAST_FILTERS)
- * @max_macs: max number of macs (NUM_MAC_INDEX_DRIVER)
- * @reserved1: reserved
- * @filters: broadcast filters
- * @macs: broadcast filtering configuration per-mac
- */
-struct iwl_bcast_filter_cmd {
- u8 disable;
- u8 max_bcast_filters;
- u8 max_macs;
- u8 reserved1;
- struct iwl_fw_bcast_filter filters[MAX_BCAST_FILTERS];
- struct iwl_fw_bcast_mac macs[NUM_MAC_INDEX_DRIVER];
-} __packed; /* BCAST_FILTERING_HCMD_API_S_VER_1 */
-
#endif /* __iwl_fw_api_filter_h__ */
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/file.h b/drivers/net/wireless/intel/iwlwifi/fw/file.h
index e4ebda64cd52..efc6540d31af 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/file.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/file.h
@@ -181,7 +181,6 @@ struct iwl_ucode_capa {
* @IWL_UCODE_TLV_FLAGS_NEW_NSOFFL_LARGE: new NS offload (large version)
* @IWL_UCODE_TLV_FLAGS_UAPSD_SUPPORT: General support for uAPSD
* @IWL_UCODE_TLV_FLAGS_P2P_PS_UAPSD: P2P client supports uAPSD power save
- * @IWL_UCODE_TLV_FLAGS_BCAST_FILTERING: uCode supports broadcast filtering.
* @IWL_UCODE_TLV_FLAGS_EBS_SUPPORT: this uCode image supports EBS.
*/
enum iwl_ucode_tlv_flag {
@@ -196,7 +195,6 @@ enum iwl_ucode_tlv_flag {
IWL_UCODE_TLV_FLAGS_UAPSD_SUPPORT = BIT(24),
IWL_UCODE_TLV_FLAGS_EBS_SUPPORT = BIT(25),
IWL_UCODE_TLV_FLAGS_P2P_PS_UAPSD = BIT(26),
- IWL_UCODE_TLV_FLAGS_BCAST_FILTERING = BIT(29),
};

typedef unsigned int __bitwise iwl_ucode_tlv_api_t;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index fb4920b01dbb..63432c24eb59 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -1369,189 +1369,6 @@ static ssize_t iwl_dbgfs_dbg_time_point_write(struct iwl_mvm *mvm,
return count;
}

-#define ADD_TEXT(...) pos += scnprintf(buf + pos, bufsz - pos, __VA_ARGS__)
-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
-static ssize_t iwl_dbgfs_bcast_filters_read(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct iwl_mvm *mvm = file->private_data;
- struct iwl_bcast_filter_cmd cmd;
- const struct iwl_fw_bcast_filter *filter;
- char *buf;
- int bufsz = 1024;
- int i, j, pos = 0;
- ssize_t ret;
-
- buf = kzalloc(bufsz, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- mutex_lock(&mvm->mutex);
- if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd)) {
- ADD_TEXT("None\n");
- mutex_unlock(&mvm->mutex);
- goto out;
- }
- mutex_unlock(&mvm->mutex);
-
- for (i = 0; cmd.filters[i].attrs[0].mask; i++) {
- filter = &cmd.filters[i];
-
- ADD_TEXT("Filter [%d]:\n", i);
- ADD_TEXT("\tDiscard=%d\n", filter->discard);
- ADD_TEXT("\tFrame Type: %s\n",
- filter->frame_type ? "IPv4" : "Generic");
-
- for (j = 0; j < ARRAY_SIZE(filter->attrs); j++) {
- const struct iwl_fw_bcast_filter_attr *attr;
-
- attr = &filter->attrs[j];
- if (!attr->mask)
- break;
-
- ADD_TEXT("\tAttr [%d]: offset=%d (from %s), mask=0x%x, value=0x%x reserved=0x%x\n",
- j, attr->offset,
- attr->offset_type ? "IP End" :
- "Payload Start",
- be32_to_cpu(attr->mask),
- be32_to_cpu(attr->val),
- le16_to_cpu(attr->reserved1));
- }
- }
-out:
- ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos);
- kfree(buf);
- return ret;
-}
-
-static ssize_t iwl_dbgfs_bcast_filters_write(struct iwl_mvm *mvm, char *buf,
- size_t count, loff_t *ppos)
-{
- int pos, next_pos;
- struct iwl_fw_bcast_filter filter = {};
- struct iwl_bcast_filter_cmd cmd;
- u32 filter_id, attr_id, mask, value;
- int err = 0;
-
- if (sscanf(buf, "%d %hhi %hhi %n", &filter_id, &filter.discard,
- &filter.frame_type, &pos) != 3)
- return -EINVAL;
-
- if (filter_id >= ARRAY_SIZE(mvm->dbgfs_bcast_filtering.cmd.filters) ||
- filter.frame_type > BCAST_FILTER_FRAME_TYPE_IPV4)
- return -EINVAL;
-
- for (attr_id = 0; attr_id < ARRAY_SIZE(filter.attrs);
- attr_id++) {
- struct iwl_fw_bcast_filter_attr *attr =
- &filter.attrs[attr_id];
-
- if (pos >= count)
- break;
-
- if (sscanf(&buf[pos], "%hhi %hhi %i %i %n",
- &attr->offset, &attr->offset_type,
- &mask, &value, &next_pos) != 4)
- return -EINVAL;
-
- attr->mask = cpu_to_be32(mask);
- attr->val = cpu_to_be32(value);
- if (mask)
- filter.num_attrs++;
-
- pos += next_pos;
- }
-
- mutex_lock(&mvm->mutex);
- memcpy(&mvm->dbgfs_bcast_filtering.cmd.filters[filter_id],
- &filter, sizeof(filter));
-
- /* send updated bcast filtering configuration */
- if (iwl_mvm_firmware_running(mvm) &&
- mvm->dbgfs_bcast_filtering.override &&
- iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
- err = iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
- sizeof(cmd), &cmd);
- mutex_unlock(&mvm->mutex);
-
- return err ?: count;
-}
-
-static ssize_t iwl_dbgfs_bcast_filters_macs_read(struct file *file,
- char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct iwl_mvm *mvm = file->private_data;
- struct iwl_bcast_filter_cmd cmd;
- char *buf;
- int bufsz = 1024;
- int i, pos = 0;
- ssize_t ret;
-
- buf = kzalloc(bufsz, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- mutex_lock(&mvm->mutex);
- if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd)) {
- ADD_TEXT("None\n");
- mutex_unlock(&mvm->mutex);
- goto out;
- }
- mutex_unlock(&mvm->mutex);
-
- for (i = 0; i < ARRAY_SIZE(cmd.macs); i++) {
- const struct iwl_fw_bcast_mac *mac = &cmd.macs[i];
-
- ADD_TEXT("Mac [%d]: discard=%d attached_filters=0x%x\n",
- i, mac->default_discard, mac->attached_filters);
- }
-out:
- ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos);
- kfree(buf);
- return ret;
-}
-
-static ssize_t iwl_dbgfs_bcast_filters_macs_write(struct iwl_mvm *mvm,
- char *buf, size_t count,
- loff_t *ppos)
-{
- struct iwl_bcast_filter_cmd cmd;
- struct iwl_fw_bcast_mac mac = {};
- u32 mac_id, attached_filters;
- int err = 0;
-
- if (!mvm->bcast_filters)
- return -ENOENT;
-
- if (sscanf(buf, "%d %hhi %i", &mac_id, &mac.default_discard,
- &attached_filters) != 3)
- return -EINVAL;
-
- if (mac_id >= ARRAY_SIZE(cmd.macs) ||
- mac.default_discard > 1 ||
- attached_filters >= BIT(ARRAY_SIZE(cmd.filters)))
- return -EINVAL;
-
- mac.attached_filters = cpu_to_le16(attached_filters);
-
- mutex_lock(&mvm->mutex);
- memcpy(&mvm->dbgfs_bcast_filtering.cmd.macs[mac_id],
- &mac, sizeof(mac));
-
- /* send updated bcast filtering configuration */
- if (iwl_mvm_firmware_running(mvm) &&
- mvm->dbgfs_bcast_filtering.override &&
- iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
- err = iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
- sizeof(cmd), &cmd);
- mutex_unlock(&mvm->mutex);
-
- return err ?: count;
-}
-#endif
-
#define MVM_DEBUGFS_WRITE_FILE_OPS(name, bufsz) \
_MVM_DEBUGFS_WRITE_FILE_OPS(name, bufsz, struct iwl_mvm)
#define MVM_DEBUGFS_READ_WRITE_FILE_OPS(name, bufsz) \
@@ -1881,11 +1698,6 @@ MVM_DEBUGFS_WRITE_FILE_OPS(inject_beacon_ie_restore, 512);

MVM_DEBUGFS_READ_FILE_OPS(uapsd_noagg_bssids);

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
-MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters, 256);
-MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters_macs, 256);
-#endif
-
#ifdef CONFIG_ACPI
MVM_DEBUGFS_READ_FILE_OPS(sar_geo_profile);
#endif
@@ -2097,21 +1909,6 @@ void iwl_mvm_dbgfs_register(struct iwl_mvm *mvm)

MVM_DEBUGFS_ADD_FILE(uapsd_noagg_bssids, mvm->debugfs_dir, S_IRUSR);

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
- if (mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_BCAST_FILTERING) {
- bcast_dir = debugfs_create_dir("bcast_filtering",
- mvm->debugfs_dir);
-
- debugfs_create_bool("override", 0600, bcast_dir,
- &mvm->dbgfs_bcast_filtering.override);
-
- MVM_DEBUGFS_ADD_FILE_ALIAS("filters", bcast_filters,
- bcast_dir, 0600);
- MVM_DEBUGFS_ADD_FILE_ALIAS("macs", bcast_filters_macs,
- bcast_dir, 0600);
- }
-#endif
-
#ifdef CONFIG_PM_SLEEP
MVM_DEBUGFS_ADD_FILE(d3_test, mvm->debugfs_dir, 0400);
debugfs_create_bool("d3_wake_sysassert", 0600, mvm->debugfs_dir,
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 65f4fe3ef504..4ac599f6ad22 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -55,79 +55,6 @@ static const struct ieee80211_iface_combination iwl_mvm_iface_combinations[] = {
},
};

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
-/*
- * Use the reserved field to indicate magic values.
- * these values will only be used internally by the driver,
- * and won't make it to the fw (reserved will be 0).
- * BC_FILTER_MAGIC_IP - configure the val of this attribute to
- * be the vif's ip address. in case there is not a single
- * ip address (0, or more than 1), this attribute will
- * be skipped.
- * BC_FILTER_MAGIC_MAC - set the val of this attribute to
- * the LSB bytes of the vif's mac address
- */
-enum {
- BC_FILTER_MAGIC_NONE = 0,
- BC_FILTER_MAGIC_IP,
- BC_FILTER_MAGIC_MAC,
-};
-
-static const struct iwl_fw_bcast_filter iwl_mvm_default_bcast_filters[] = {
- {
- /* arp */
- .discard = 0,
- .frame_type = BCAST_FILTER_FRAME_TYPE_ALL,
- .attrs = {
- {
- /* frame type - arp, hw type - ethernet */
- .offset_type =
- BCAST_FILTER_OFFSET_PAYLOAD_START,
- .offset = sizeof(rfc1042_header),
- .val = cpu_to_be32(0x08060001),
- .mask = cpu_to_be32(0xffffffff),
- },
- {
- /* arp dest ip */
- .offset_type =
- BCAST_FILTER_OFFSET_PAYLOAD_START,
- .offset = sizeof(rfc1042_header) + 2 +
- sizeof(struct arphdr) +
- ETH_ALEN + sizeof(__be32) +
- ETH_ALEN,
- .mask = cpu_to_be32(0xffffffff),
- /* mark it as special field */
- .reserved1 = cpu_to_le16(BC_FILTER_MAGIC_IP),
- },
- },
- },
- {
- /* dhcp offer bcast */
- .discard = 0,
- .frame_type = BCAST_FILTER_FRAME_TYPE_IPV4,
- .attrs = {
- {
- /* udp dest port - 68 (bootp client)*/
- .offset_type = BCAST_FILTER_OFFSET_IP_END,
- .offset = offsetof(struct udphdr, dest),
- .val = cpu_to_be32(0x00440000),
- .mask = cpu_to_be32(0xffff0000),
- },
- {
- /* dhcp - lsb bytes of client hw address */
- .offset_type = BCAST_FILTER_OFFSET_IP_END,
- .offset = 38,
- .mask = cpu_to_be32(0xffffffff),
- /* mark it as special field */
- .reserved1 = cpu_to_le16(BC_FILTER_MAGIC_MAC),
- },
- },
- },
- /* last filter must be empty */
- {},
-};
-#endif
-
static const struct cfg80211_pmsr_capabilities iwl_mvm_pmsr_capa = {
.max_peers = IWL_MVM_TOF_MAX_APS,
.report_ap_tsf = 1,
@@ -693,11 +620,6 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
}
#endif

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
- /* assign default bcast filtering configuration */
- mvm->bcast_filters = iwl_mvm_default_bcast_filters;
-#endif
-
ret = iwl_mvm_leds_init(mvm);
if (ret)
return ret;
@@ -1853,162 +1775,6 @@ static void iwl_mvm_config_iface_filter(struct ieee80211_hw *hw,
mutex_unlock(&mvm->mutex);
}

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
-struct iwl_bcast_iter_data {
- struct iwl_mvm *mvm;
- struct iwl_bcast_filter_cmd *cmd;
- u8 current_filter;
-};
-
-static void
-iwl_mvm_set_bcast_filter(struct ieee80211_vif *vif,
- const struct iwl_fw_bcast_filter *in_filter,
- struct iwl_fw_bcast_filter *out_filter)
-{
- struct iwl_fw_bcast_filter_attr *attr;
- int i;
-
- memcpy(out_filter, in_filter, sizeof(*out_filter));
-
- for (i = 0; i < ARRAY_SIZE(out_filter->attrs); i++) {
- attr = &out_filter->attrs[i];
-
- if (!attr->mask)
- break;
-
- switch (attr->reserved1) {
- case cpu_to_le16(BC_FILTER_MAGIC_IP):
- if (vif->bss_conf.arp_addr_cnt != 1) {
- attr->mask = 0;
- continue;
- }
-
- attr->val = vif->bss_conf.arp_addr_list[0];
- break;
- case cpu_to_le16(BC_FILTER_MAGIC_MAC):
- attr->val = *(__be32 *)&vif->addr[2];
- break;
- default:
- break;
- }
- attr->reserved1 = 0;
- out_filter->num_attrs++;
- }
-}
-
-static void iwl_mvm_bcast_filter_iterator(void *_data, u8 *mac,
- struct ieee80211_vif *vif)
-{
- struct iwl_bcast_iter_data *data = _data;
- struct iwl_mvm *mvm = data->mvm;
- struct iwl_bcast_filter_cmd *cmd = data->cmd;
- struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
- struct iwl_fw_bcast_mac *bcast_mac;
- int i;
-
- if (WARN_ON(mvmvif->id >= ARRAY_SIZE(cmd->macs)))
- return;
-
- bcast_mac = &cmd->macs[mvmvif->id];
-
- /*
- * enable filtering only for associated stations, but not for P2P
- * Clients
- */
- if (vif->type != NL80211_IFTYPE_STATION || vif->p2p ||
- !vif->bss_conf.assoc)
- return;
-
- bcast_mac->default_discard = 1;
-
- /* copy all configured filters */
- for (i = 0; mvm->bcast_filters[i].attrs[0].mask; i++) {
- /*
- * Make sure we don't exceed our filters limit.
- * if there is still a valid filter to be configured,
- * be on the safe side and just allow bcast for this mac.
- */
- if (WARN_ON_ONCE(data->current_filter >=
- ARRAY_SIZE(cmd->filters))) {
- bcast_mac->default_discard = 0;
- bcast_mac->attached_filters = 0;
- break;
- }
-
- iwl_mvm_set_bcast_filter(vif,
- &mvm->bcast_filters[i],
- &cmd->filters[data->current_filter]);
-
- /* skip current filter if it contains no attributes */
- if (!cmd->filters[data->current_filter].num_attrs)
- continue;
-
- /* attach the filter to current mac */
- bcast_mac->attached_filters |=
- cpu_to_le16(BIT(data->current_filter));
-
- data->current_filter++;
- }
-}
-
-bool iwl_mvm_bcast_filter_build_cmd(struct iwl_mvm *mvm,
- struct iwl_bcast_filter_cmd *cmd)
-{
- struct iwl_bcast_iter_data iter_data = {
- .mvm = mvm,
- .cmd = cmd,
- };
-
- if (IWL_MVM_FW_BCAST_FILTER_PASS_ALL)
- return false;
-
- memset(cmd, 0, sizeof(*cmd));
- cmd->max_bcast_filters = ARRAY_SIZE(cmd->filters);
- cmd->max_macs = ARRAY_SIZE(cmd->macs);
-
-#ifdef CONFIG_IWLWIFI_DEBUGFS
- /* use debugfs filters/macs if override is configured */
- if (mvm->dbgfs_bcast_filtering.override) {
- memcpy(cmd->filters, &mvm->dbgfs_bcast_filtering.cmd.filters,
- sizeof(cmd->filters));
- memcpy(cmd->macs, &mvm->dbgfs_bcast_filtering.cmd.macs,
- sizeof(cmd->macs));
- return true;
- }
-#endif
-
- /* if no filters are configured, do nothing */
- if (!mvm->bcast_filters)
- return false;
-
- /* configure and attach these filters for each associated sta vif */
- ieee80211_iterate_active_interfaces(
- mvm->hw, IEEE80211_IFACE_ITER_NORMAL,
- iwl_mvm_bcast_filter_iterator, &iter_data);
-
- return true;
-}
-
-static int iwl_mvm_configure_bcast_filter(struct iwl_mvm *mvm)
-{
- struct iwl_bcast_filter_cmd cmd;
-
- if (!(mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_BCAST_FILTERING))
- return 0;
-
- if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
- return 0;
-
- return iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
- sizeof(cmd), &cmd);
-}
-#else
-static inline int iwl_mvm_configure_bcast_filter(struct iwl_mvm *mvm)
-{
- return 0;
-}
-#endif
-
static int iwl_mvm_update_mu_groups(struct iwl_mvm *mvm,
struct ieee80211_vif *vif)
{
@@ -2520,7 +2286,6 @@ static void iwl_mvm_bss_info_changed_station(struct iwl_mvm *mvm,
}

iwl_mvm_recalc_multicast(mvm);
- iwl_mvm_configure_bcast_filter(mvm);

/* reset rssi values */
mvmvif->bf_data.ave_beacon_signal = 0;
@@ -2570,11 +2335,6 @@ static void iwl_mvm_bss_info_changed_station(struct iwl_mvm *mvm,
}
}

- if (changes & BSS_CHANGED_ARP_FILTER) {
- IWL_DEBUG_MAC80211(mvm, "arp filter changed\n");
- iwl_mvm_configure_bcast_filter(mvm);
- }
-
if (changes & BSS_CHANGED_BANDWIDTH)
iwl_mvm_apply_fw_smps_request(vif);
}
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index b1fe8434ab0d..d78f40730594 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -884,17 +884,6 @@ struct iwl_mvm {
/* rx chain antennas set through debugfs for the scan command */
u8 scan_rx_ant;

-#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
- /* broadcast filters to configure for each associated station */
- const struct iwl_fw_bcast_filter *bcast_filters;
-#ifdef CONFIG_IWLWIFI_DEBUGFS
- struct {
- bool override;
- struct iwl_bcast_filter_cmd cmd;
- } dbgfs_bcast_filtering;
-#endif
-#endif
-
/* Internal station */
struct iwl_mvm_int_sta aux_sta;
struct iwl_mvm_int_sta snif_sta;
@@ -1593,8 +1582,6 @@ int iwl_mvm_up(struct iwl_mvm *mvm);
int iwl_mvm_load_d3_fw(struct iwl_mvm *mvm);

int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm);
-bool iwl_mvm_bcast_filter_build_cmd(struct iwl_mvm *mvm,
- struct iwl_bcast_filter_cmd *cmd);

/*
* FW notifications / CMD responses handlers
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index 87630d38dc52..1f8b97995b94 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -469,7 +469,6 @@ static const struct iwl_hcmd_names iwl_mvm_legacy_names[] = {
HCMD_NAME(MCC_CHUB_UPDATE_CMD),
HCMD_NAME(MARKER_CMD),
HCMD_NAME(BT_PROFILE_NOTIFICATION),
- HCMD_NAME(BCAST_FILTER_CMD),
HCMD_NAME(MCAST_FILTER_CMD),
HCMD_NAME(REPLY_SF_CFG_CMD),
HCMD_NAME(REPLY_BEACON_FILTERING_CMD),
--
2.34.1

2022-02-01 15:42:17

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

Hi, this is your Linux kernel regression tracker speaking.

On 28.01.22 13:48, Luca Coelho wrote:
> From: Luca Coelho <[email protected]>
>
> This feature has been deprecated and should not be used anymore. With
> newer firmwares, namely *-67.ucode and above, trying to use it causes an
> assertion failure in the FW, similar to this:
>
> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>
> In order to prevent this feature from being used, remove it entirely
> and get rid of the Kconfig option that
> enables it (IWLWIFI_BCAST_FILTERING).
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488

FWIW there was another report about it afaics:

Link: https://bugzilla.kernel.org/show_bug.cgi?id=215550

That makes me hope that this is reviewed and merged to mainline relative
quickly, otherwise more users will be bothered by this.

> Signed-off-by: Luca Coelho <[email protected]>
> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
> Signed-off-by: Luca Coelho <[email protected]>

Shouldn't this also have:

Cc: [email protected] # 5.16.x

Ciao, Thorsten

#regzbot ^backmonitor:
https://lore.kernel.org/regressions/[email protected]/

> ---
> drivers/net/wireless/intel/iwlwifi/Kconfig | 13 -
> .../wireless/intel/iwlwifi/fw/api/commands.h | 5 -
> .../wireless/intel/iwlwifi/fw/api/filter.h | 88 -------
> drivers/net/wireless/intel/iwlwifi/fw/file.h | 2 -
> .../net/wireless/intel/iwlwifi/mvm/debugfs.c | 203 ---------------
> .../net/wireless/intel/iwlwifi/mvm/mac80211.c | 240 ------------------
> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 13 -
> drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 1 -
> 8 files changed, 565 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig b/drivers/net/wireless/intel/iwlwifi/Kconfig
> index c21c0c68849a..85e704283755 100644
> --- a/drivers/net/wireless/intel/iwlwifi/Kconfig
> +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
> @@ -80,19 +80,6 @@ config IWLWIFI_OPMODE_MODULAR
> comment "WARNING: iwlwifi is useless without IWLDVM or IWLMVM"
> depends on IWLDVM=n && IWLMVM=n
>
> -config IWLWIFI_BCAST_FILTERING
> - bool "Enable broadcast filtering"
> - depends on IWLMVM
> - help
> - Say Y here to enable default bcast filtering configuration.
> -
> - Enabling broadcast filtering will drop any incoming wireless
> - broadcast frames, except some very specific predefined
> - patterns (e.g. incoming arp requests).
> -
> - If unsure, don't enable this option, as some programs might
> - expect incoming broadcasts for their normal operations.
> -
> menu "Debugging Options"
>
> config IWLWIFI_DEBUG
> diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h b/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
> index 0703e41403a6..35b8856e511f 100644
> --- a/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
> +++ b/drivers/net/wireless/intel/iwlwifi/fw/api/commands.h
> @@ -501,11 +501,6 @@ enum iwl_legacy_cmds {
> */
> DEBUG_LOG_MSG = 0xf7,
>
> - /**
> - * @BCAST_FILTER_CMD: &struct iwl_bcast_filter_cmd
> - */
> - BCAST_FILTER_CMD = 0xcf,
> -
> /**
> * @MCAST_FILTER_CMD: &struct iwl_mcast_filter_cmd
> */
> diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h b/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
> index dd62a63956b3..e44c70b7c790 100644
> --- a/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
> +++ b/drivers/net/wireless/intel/iwlwifi/fw/api/filter.h
> @@ -36,92 +36,4 @@ struct iwl_mcast_filter_cmd {
> u8 addr_list[0];
> } __packed; /* MCAST_FILTERING_CMD_API_S_VER_1 */
>
> -#define MAX_BCAST_FILTERS 8
> -#define MAX_BCAST_FILTER_ATTRS 2
> -
> -/**
> - * enum iwl_mvm_bcast_filter_attr_offset - written by fw for each Rx packet
> - * @BCAST_FILTER_OFFSET_PAYLOAD_START: offset is from payload start.
> - * @BCAST_FILTER_OFFSET_IP_END: offset is from ip header end (i.e.
> - * start of ip payload).
> - */
> -enum iwl_mvm_bcast_filter_attr_offset {
> - BCAST_FILTER_OFFSET_PAYLOAD_START = 0,
> - BCAST_FILTER_OFFSET_IP_END = 1,
> -};
> -
> -/**
> - * struct iwl_fw_bcast_filter_attr - broadcast filter attribute
> - * @offset_type: &enum iwl_mvm_bcast_filter_attr_offset.
> - * @offset: starting offset of this pattern.
> - * @reserved1: reserved
> - * @val: value to match - big endian (MSB is the first
> - * byte to match from offset pos).
> - * @mask: mask to match (big endian).
> - */
> -struct iwl_fw_bcast_filter_attr {
> - u8 offset_type;
> - u8 offset;
> - __le16 reserved1;
> - __be32 val;
> - __be32 mask;
> -} __packed; /* BCAST_FILTER_ATT_S_VER_1 */
> -
> -/**
> - * enum iwl_mvm_bcast_filter_frame_type - filter frame type
> - * @BCAST_FILTER_FRAME_TYPE_ALL: consider all frames.
> - * @BCAST_FILTER_FRAME_TYPE_IPV4: consider only ipv4 frames
> - */
> -enum iwl_mvm_bcast_filter_frame_type {
> - BCAST_FILTER_FRAME_TYPE_ALL = 0,
> - BCAST_FILTER_FRAME_TYPE_IPV4 = 1,
> -};
> -
> -/**
> - * struct iwl_fw_bcast_filter - broadcast filter
> - * @discard: discard frame (1) or let it pass (0).
> - * @frame_type: &enum iwl_mvm_bcast_filter_frame_type.
> - * @reserved1: reserved
> - * @num_attrs: number of valid attributes in this filter.
> - * @attrs: attributes of this filter. a filter is considered matched
> - * only when all its attributes are matched (i.e. AND relationship)
> - */
> -struct iwl_fw_bcast_filter {
> - u8 discard;
> - u8 frame_type;
> - u8 num_attrs;
> - u8 reserved1;
> - struct iwl_fw_bcast_filter_attr attrs[MAX_BCAST_FILTER_ATTRS];
> -} __packed; /* BCAST_FILTER_S_VER_1 */
> -
> -/**
> - * struct iwl_fw_bcast_mac - per-mac broadcast filtering configuration.
> - * @default_discard: default action for this mac (discard (1) / pass (0)).
> - * @reserved1: reserved
> - * @attached_filters: bitmap of relevant filters for this mac.
> - */
> -struct iwl_fw_bcast_mac {
> - u8 default_discard;
> - u8 reserved1;
> - __le16 attached_filters;
> -} __packed; /* BCAST_MAC_CONTEXT_S_VER_1 */
> -
> -/**
> - * struct iwl_bcast_filter_cmd - broadcast filtering configuration
> - * @disable: enable (0) / disable (1)
> - * @max_bcast_filters: max number of filters (MAX_BCAST_FILTERS)
> - * @max_macs: max number of macs (NUM_MAC_INDEX_DRIVER)
> - * @reserved1: reserved
> - * @filters: broadcast filters
> - * @macs: broadcast filtering configuration per-mac
> - */
> -struct iwl_bcast_filter_cmd {
> - u8 disable;
> - u8 max_bcast_filters;
> - u8 max_macs;
> - u8 reserved1;
> - struct iwl_fw_bcast_filter filters[MAX_BCAST_FILTERS];
> - struct iwl_fw_bcast_mac macs[NUM_MAC_INDEX_DRIVER];
> -} __packed; /* BCAST_FILTERING_HCMD_API_S_VER_1 */
> -
> #endif /* __iwl_fw_api_filter_h__ */
> diff --git a/drivers/net/wireless/intel/iwlwifi/fw/file.h b/drivers/net/wireless/intel/iwlwifi/fw/file.h
> index e4ebda64cd52..efc6540d31af 100644
> --- a/drivers/net/wireless/intel/iwlwifi/fw/file.h
> +++ b/drivers/net/wireless/intel/iwlwifi/fw/file.h
> @@ -181,7 +181,6 @@ struct iwl_ucode_capa {
> * @IWL_UCODE_TLV_FLAGS_NEW_NSOFFL_LARGE: new NS offload (large version)
> * @IWL_UCODE_TLV_FLAGS_UAPSD_SUPPORT: General support for uAPSD
> * @IWL_UCODE_TLV_FLAGS_P2P_PS_UAPSD: P2P client supports uAPSD power save
> - * @IWL_UCODE_TLV_FLAGS_BCAST_FILTERING: uCode supports broadcast filtering.
> * @IWL_UCODE_TLV_FLAGS_EBS_SUPPORT: this uCode image supports EBS.
> */
> enum iwl_ucode_tlv_flag {
> @@ -196,7 +195,6 @@ enum iwl_ucode_tlv_flag {
> IWL_UCODE_TLV_FLAGS_UAPSD_SUPPORT = BIT(24),
> IWL_UCODE_TLV_FLAGS_EBS_SUPPORT = BIT(25),
> IWL_UCODE_TLV_FLAGS_P2P_PS_UAPSD = BIT(26),
> - IWL_UCODE_TLV_FLAGS_BCAST_FILTERING = BIT(29),
> };
>
> typedef unsigned int __bitwise iwl_ucode_tlv_api_t;
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index fb4920b01dbb..63432c24eb59 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -1369,189 +1369,6 @@ static ssize_t iwl_dbgfs_dbg_time_point_write(struct iwl_mvm *mvm,
> return count;
> }
>
> -#define ADD_TEXT(...) pos += scnprintf(buf + pos, bufsz - pos, __VA_ARGS__)
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> -static ssize_t iwl_dbgfs_bcast_filters_read(struct file *file,
> - char __user *user_buf,
> - size_t count, loff_t *ppos)
> -{
> - struct iwl_mvm *mvm = file->private_data;
> - struct iwl_bcast_filter_cmd cmd;
> - const struct iwl_fw_bcast_filter *filter;
> - char *buf;
> - int bufsz = 1024;
> - int i, j, pos = 0;
> - ssize_t ret;
> -
> - buf = kzalloc(bufsz, GFP_KERNEL);
> - if (!buf)
> - return -ENOMEM;
> -
> - mutex_lock(&mvm->mutex);
> - if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd)) {
> - ADD_TEXT("None\n");
> - mutex_unlock(&mvm->mutex);
> - goto out;
> - }
> - mutex_unlock(&mvm->mutex);
> -
> - for (i = 0; cmd.filters[i].attrs[0].mask; i++) {
> - filter = &cmd.filters[i];
> -
> - ADD_TEXT("Filter [%d]:\n", i);
> - ADD_TEXT("\tDiscard=%d\n", filter->discard);
> - ADD_TEXT("\tFrame Type: %s\n",
> - filter->frame_type ? "IPv4" : "Generic");
> -
> - for (j = 0; j < ARRAY_SIZE(filter->attrs); j++) {
> - const struct iwl_fw_bcast_filter_attr *attr;
> -
> - attr = &filter->attrs[j];
> - if (!attr->mask)
> - break;
> -
> - ADD_TEXT("\tAttr [%d]: offset=%d (from %s), mask=0x%x, value=0x%x reserved=0x%x\n",
> - j, attr->offset,
> - attr->offset_type ? "IP End" :
> - "Payload Start",
> - be32_to_cpu(attr->mask),
> - be32_to_cpu(attr->val),
> - le16_to_cpu(attr->reserved1));
> - }
> - }
> -out:
> - ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos);
> - kfree(buf);
> - return ret;
> -}
> -
> -static ssize_t iwl_dbgfs_bcast_filters_write(struct iwl_mvm *mvm, char *buf,
> - size_t count, loff_t *ppos)
> -{
> - int pos, next_pos;
> - struct iwl_fw_bcast_filter filter = {};
> - struct iwl_bcast_filter_cmd cmd;
> - u32 filter_id, attr_id, mask, value;
> - int err = 0;
> -
> - if (sscanf(buf, "%d %hhi %hhi %n", &filter_id, &filter.discard,
> - &filter.frame_type, &pos) != 3)
> - return -EINVAL;
> -
> - if (filter_id >= ARRAY_SIZE(mvm->dbgfs_bcast_filtering.cmd.filters) ||
> - filter.frame_type > BCAST_FILTER_FRAME_TYPE_IPV4)
> - return -EINVAL;
> -
> - for (attr_id = 0; attr_id < ARRAY_SIZE(filter.attrs);
> - attr_id++) {
> - struct iwl_fw_bcast_filter_attr *attr =
> - &filter.attrs[attr_id];
> -
> - if (pos >= count)
> - break;
> -
> - if (sscanf(&buf[pos], "%hhi %hhi %i %i %n",
> - &attr->offset, &attr->offset_type,
> - &mask, &value, &next_pos) != 4)
> - return -EINVAL;
> -
> - attr->mask = cpu_to_be32(mask);
> - attr->val = cpu_to_be32(value);
> - if (mask)
> - filter.num_attrs++;
> -
> - pos += next_pos;
> - }
> -
> - mutex_lock(&mvm->mutex);
> - memcpy(&mvm->dbgfs_bcast_filtering.cmd.filters[filter_id],
> - &filter, sizeof(filter));
> -
> - /* send updated bcast filtering configuration */
> - if (iwl_mvm_firmware_running(mvm) &&
> - mvm->dbgfs_bcast_filtering.override &&
> - iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
> - err = iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
> - sizeof(cmd), &cmd);
> - mutex_unlock(&mvm->mutex);
> -
> - return err ?: count;
> -}
> -
> -static ssize_t iwl_dbgfs_bcast_filters_macs_read(struct file *file,
> - char __user *user_buf,
> - size_t count, loff_t *ppos)
> -{
> - struct iwl_mvm *mvm = file->private_data;
> - struct iwl_bcast_filter_cmd cmd;
> - char *buf;
> - int bufsz = 1024;
> - int i, pos = 0;
> - ssize_t ret;
> -
> - buf = kzalloc(bufsz, GFP_KERNEL);
> - if (!buf)
> - return -ENOMEM;
> -
> - mutex_lock(&mvm->mutex);
> - if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd)) {
> - ADD_TEXT("None\n");
> - mutex_unlock(&mvm->mutex);
> - goto out;
> - }
> - mutex_unlock(&mvm->mutex);
> -
> - for (i = 0; i < ARRAY_SIZE(cmd.macs); i++) {
> - const struct iwl_fw_bcast_mac *mac = &cmd.macs[i];
> -
> - ADD_TEXT("Mac [%d]: discard=%d attached_filters=0x%x\n",
> - i, mac->default_discard, mac->attached_filters);
> - }
> -out:
> - ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos);
> - kfree(buf);
> - return ret;
> -}
> -
> -static ssize_t iwl_dbgfs_bcast_filters_macs_write(struct iwl_mvm *mvm,
> - char *buf, size_t count,
> - loff_t *ppos)
> -{
> - struct iwl_bcast_filter_cmd cmd;
> - struct iwl_fw_bcast_mac mac = {};
> - u32 mac_id, attached_filters;
> - int err = 0;
> -
> - if (!mvm->bcast_filters)
> - return -ENOENT;
> -
> - if (sscanf(buf, "%d %hhi %i", &mac_id, &mac.default_discard,
> - &attached_filters) != 3)
> - return -EINVAL;
> -
> - if (mac_id >= ARRAY_SIZE(cmd.macs) ||
> - mac.default_discard > 1 ||
> - attached_filters >= BIT(ARRAY_SIZE(cmd.filters)))
> - return -EINVAL;
> -
> - mac.attached_filters = cpu_to_le16(attached_filters);
> -
> - mutex_lock(&mvm->mutex);
> - memcpy(&mvm->dbgfs_bcast_filtering.cmd.macs[mac_id],
> - &mac, sizeof(mac));
> -
> - /* send updated bcast filtering configuration */
> - if (iwl_mvm_firmware_running(mvm) &&
> - mvm->dbgfs_bcast_filtering.override &&
> - iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
> - err = iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
> - sizeof(cmd), &cmd);
> - mutex_unlock(&mvm->mutex);
> -
> - return err ?: count;
> -}
> -#endif
> -
> #define MVM_DEBUGFS_WRITE_FILE_OPS(name, bufsz) \
> _MVM_DEBUGFS_WRITE_FILE_OPS(name, bufsz, struct iwl_mvm)
> #define MVM_DEBUGFS_READ_WRITE_FILE_OPS(name, bufsz) \
> @@ -1881,11 +1698,6 @@ MVM_DEBUGFS_WRITE_FILE_OPS(inject_beacon_ie_restore, 512);
>
> MVM_DEBUGFS_READ_FILE_OPS(uapsd_noagg_bssids);
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> -MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters, 256);
> -MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters_macs, 256);
> -#endif
> -
> #ifdef CONFIG_ACPI
> MVM_DEBUGFS_READ_FILE_OPS(sar_geo_profile);
> #endif
> @@ -2097,21 +1909,6 @@ void iwl_mvm_dbgfs_register(struct iwl_mvm *mvm)
>
> MVM_DEBUGFS_ADD_FILE(uapsd_noagg_bssids, mvm->debugfs_dir, S_IRUSR);
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> - if (mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_BCAST_FILTERING) {
> - bcast_dir = debugfs_create_dir("bcast_filtering",
> - mvm->debugfs_dir);
> -
> - debugfs_create_bool("override", 0600, bcast_dir,
> - &mvm->dbgfs_bcast_filtering.override);
> -
> - MVM_DEBUGFS_ADD_FILE_ALIAS("filters", bcast_filters,
> - bcast_dir, 0600);
> - MVM_DEBUGFS_ADD_FILE_ALIAS("macs", bcast_filters_macs,
> - bcast_dir, 0600);
> - }
> -#endif
> -
> #ifdef CONFIG_PM_SLEEP
> MVM_DEBUGFS_ADD_FILE(d3_test, mvm->debugfs_dir, 0400);
> debugfs_create_bool("d3_wake_sysassert", 0600, mvm->debugfs_dir,
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> index 65f4fe3ef504..4ac599f6ad22 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
> @@ -55,79 +55,6 @@ static const struct ieee80211_iface_combination iwl_mvm_iface_combinations[] = {
> },
> };
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> -/*
> - * Use the reserved field to indicate magic values.
> - * these values will only be used internally by the driver,
> - * and won't make it to the fw (reserved will be 0).
> - * BC_FILTER_MAGIC_IP - configure the val of this attribute to
> - * be the vif's ip address. in case there is not a single
> - * ip address (0, or more than 1), this attribute will
> - * be skipped.
> - * BC_FILTER_MAGIC_MAC - set the val of this attribute to
> - * the LSB bytes of the vif's mac address
> - */
> -enum {
> - BC_FILTER_MAGIC_NONE = 0,
> - BC_FILTER_MAGIC_IP,
> - BC_FILTER_MAGIC_MAC,
> -};
> -
> -static const struct iwl_fw_bcast_filter iwl_mvm_default_bcast_filters[] = {
> - {
> - /* arp */
> - .discard = 0,
> - .frame_type = BCAST_FILTER_FRAME_TYPE_ALL,
> - .attrs = {
> - {
> - /* frame type - arp, hw type - ethernet */
> - .offset_type =
> - BCAST_FILTER_OFFSET_PAYLOAD_START,
> - .offset = sizeof(rfc1042_header),
> - .val = cpu_to_be32(0x08060001),
> - .mask = cpu_to_be32(0xffffffff),
> - },
> - {
> - /* arp dest ip */
> - .offset_type =
> - BCAST_FILTER_OFFSET_PAYLOAD_START,
> - .offset = sizeof(rfc1042_header) + 2 +
> - sizeof(struct arphdr) +
> - ETH_ALEN + sizeof(__be32) +
> - ETH_ALEN,
> - .mask = cpu_to_be32(0xffffffff),
> - /* mark it as special field */
> - .reserved1 = cpu_to_le16(BC_FILTER_MAGIC_IP),
> - },
> - },
> - },
> - {
> - /* dhcp offer bcast */
> - .discard = 0,
> - .frame_type = BCAST_FILTER_FRAME_TYPE_IPV4,
> - .attrs = {
> - {
> - /* udp dest port - 68 (bootp client)*/
> - .offset_type = BCAST_FILTER_OFFSET_IP_END,
> - .offset = offsetof(struct udphdr, dest),
> - .val = cpu_to_be32(0x00440000),
> - .mask = cpu_to_be32(0xffff0000),
> - },
> - {
> - /* dhcp - lsb bytes of client hw address */
> - .offset_type = BCAST_FILTER_OFFSET_IP_END,
> - .offset = 38,
> - .mask = cpu_to_be32(0xffffffff),
> - /* mark it as special field */
> - .reserved1 = cpu_to_le16(BC_FILTER_MAGIC_MAC),
> - },
> - },
> - },
> - /* last filter must be empty */
> - {},
> -};
> -#endif
> -
> static const struct cfg80211_pmsr_capabilities iwl_mvm_pmsr_capa = {
> .max_peers = IWL_MVM_TOF_MAX_APS,
> .report_ap_tsf = 1,
> @@ -693,11 +620,6 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
> }
> #endif
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> - /* assign default bcast filtering configuration */
> - mvm->bcast_filters = iwl_mvm_default_bcast_filters;
> -#endif
> -
> ret = iwl_mvm_leds_init(mvm);
> if (ret)
> return ret;
> @@ -1853,162 +1775,6 @@ static void iwl_mvm_config_iface_filter(struct ieee80211_hw *hw,
> mutex_unlock(&mvm->mutex);
> }
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> -struct iwl_bcast_iter_data {
> - struct iwl_mvm *mvm;
> - struct iwl_bcast_filter_cmd *cmd;
> - u8 current_filter;
> -};
> -
> -static void
> -iwl_mvm_set_bcast_filter(struct ieee80211_vif *vif,
> - const struct iwl_fw_bcast_filter *in_filter,
> - struct iwl_fw_bcast_filter *out_filter)
> -{
> - struct iwl_fw_bcast_filter_attr *attr;
> - int i;
> -
> - memcpy(out_filter, in_filter, sizeof(*out_filter));
> -
> - for (i = 0; i < ARRAY_SIZE(out_filter->attrs); i++) {
> - attr = &out_filter->attrs[i];
> -
> - if (!attr->mask)
> - break;
> -
> - switch (attr->reserved1) {
> - case cpu_to_le16(BC_FILTER_MAGIC_IP):
> - if (vif->bss_conf.arp_addr_cnt != 1) {
> - attr->mask = 0;
> - continue;
> - }
> -
> - attr->val = vif->bss_conf.arp_addr_list[0];
> - break;
> - case cpu_to_le16(BC_FILTER_MAGIC_MAC):
> - attr->val = *(__be32 *)&vif->addr[2];
> - break;
> - default:
> - break;
> - }
> - attr->reserved1 = 0;
> - out_filter->num_attrs++;
> - }
> -}
> -
> -static void iwl_mvm_bcast_filter_iterator(void *_data, u8 *mac,
> - struct ieee80211_vif *vif)
> -{
> - struct iwl_bcast_iter_data *data = _data;
> - struct iwl_mvm *mvm = data->mvm;
> - struct iwl_bcast_filter_cmd *cmd = data->cmd;
> - struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
> - struct iwl_fw_bcast_mac *bcast_mac;
> - int i;
> -
> - if (WARN_ON(mvmvif->id >= ARRAY_SIZE(cmd->macs)))
> - return;
> -
> - bcast_mac = &cmd->macs[mvmvif->id];
> -
> - /*
> - * enable filtering only for associated stations, but not for P2P
> - * Clients
> - */
> - if (vif->type != NL80211_IFTYPE_STATION || vif->p2p ||
> - !vif->bss_conf.assoc)
> - return;
> -
> - bcast_mac->default_discard = 1;
> -
> - /* copy all configured filters */
> - for (i = 0; mvm->bcast_filters[i].attrs[0].mask; i++) {
> - /*
> - * Make sure we don't exceed our filters limit.
> - * if there is still a valid filter to be configured,
> - * be on the safe side and just allow bcast for this mac.
> - */
> - if (WARN_ON_ONCE(data->current_filter >=
> - ARRAY_SIZE(cmd->filters))) {
> - bcast_mac->default_discard = 0;
> - bcast_mac->attached_filters = 0;
> - break;
> - }
> -
> - iwl_mvm_set_bcast_filter(vif,
> - &mvm->bcast_filters[i],
> - &cmd->filters[data->current_filter]);
> -
> - /* skip current filter if it contains no attributes */
> - if (!cmd->filters[data->current_filter].num_attrs)
> - continue;
> -
> - /* attach the filter to current mac */
> - bcast_mac->attached_filters |=
> - cpu_to_le16(BIT(data->current_filter));
> -
> - data->current_filter++;
> - }
> -}
> -
> -bool iwl_mvm_bcast_filter_build_cmd(struct iwl_mvm *mvm,
> - struct iwl_bcast_filter_cmd *cmd)
> -{
> - struct iwl_bcast_iter_data iter_data = {
> - .mvm = mvm,
> - .cmd = cmd,
> - };
> -
> - if (IWL_MVM_FW_BCAST_FILTER_PASS_ALL)
> - return false;
> -
> - memset(cmd, 0, sizeof(*cmd));
> - cmd->max_bcast_filters = ARRAY_SIZE(cmd->filters);
> - cmd->max_macs = ARRAY_SIZE(cmd->macs);
> -
> -#ifdef CONFIG_IWLWIFI_DEBUGFS
> - /* use debugfs filters/macs if override is configured */
> - if (mvm->dbgfs_bcast_filtering.override) {
> - memcpy(cmd->filters, &mvm->dbgfs_bcast_filtering.cmd.filters,
> - sizeof(cmd->filters));
> - memcpy(cmd->macs, &mvm->dbgfs_bcast_filtering.cmd.macs,
> - sizeof(cmd->macs));
> - return true;
> - }
> -#endif
> -
> - /* if no filters are configured, do nothing */
> - if (!mvm->bcast_filters)
> - return false;
> -
> - /* configure and attach these filters for each associated sta vif */
> - ieee80211_iterate_active_interfaces(
> - mvm->hw, IEEE80211_IFACE_ITER_NORMAL,
> - iwl_mvm_bcast_filter_iterator, &iter_data);
> -
> - return true;
> -}
> -
> -static int iwl_mvm_configure_bcast_filter(struct iwl_mvm *mvm)
> -{
> - struct iwl_bcast_filter_cmd cmd;
> -
> - if (!(mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_BCAST_FILTERING))
> - return 0;
> -
> - if (!iwl_mvm_bcast_filter_build_cmd(mvm, &cmd))
> - return 0;
> -
> - return iwl_mvm_send_cmd_pdu(mvm, BCAST_FILTER_CMD, 0,
> - sizeof(cmd), &cmd);
> -}
> -#else
> -static inline int iwl_mvm_configure_bcast_filter(struct iwl_mvm *mvm)
> -{
> - return 0;
> -}
> -#endif
> -
> static int iwl_mvm_update_mu_groups(struct iwl_mvm *mvm,
> struct ieee80211_vif *vif)
> {
> @@ -2520,7 +2286,6 @@ static void iwl_mvm_bss_info_changed_station(struct iwl_mvm *mvm,
> }
>
> iwl_mvm_recalc_multicast(mvm);
> - iwl_mvm_configure_bcast_filter(mvm);
>
> /* reset rssi values */
> mvmvif->bf_data.ave_beacon_signal = 0;
> @@ -2570,11 +2335,6 @@ static void iwl_mvm_bss_info_changed_station(struct iwl_mvm *mvm,
> }
> }
>
> - if (changes & BSS_CHANGED_ARP_FILTER) {
> - IWL_DEBUG_MAC80211(mvm, "arp filter changed\n");
> - iwl_mvm_configure_bcast_filter(mvm);
> - }
> -
> if (changes & BSS_CHANGED_BANDWIDTH)
> iwl_mvm_apply_fw_smps_request(vif);
> }
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> index b1fe8434ab0d..d78f40730594 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> @@ -884,17 +884,6 @@ struct iwl_mvm {
> /* rx chain antennas set through debugfs for the scan command */
> u8 scan_rx_ant;
>
> -#ifdef CONFIG_IWLWIFI_BCAST_FILTERING
> - /* broadcast filters to configure for each associated station */
> - const struct iwl_fw_bcast_filter *bcast_filters;
> -#ifdef CONFIG_IWLWIFI_DEBUGFS
> - struct {
> - bool override;
> - struct iwl_bcast_filter_cmd cmd;
> - } dbgfs_bcast_filtering;
> -#endif
> -#endif
> -
> /* Internal station */
> struct iwl_mvm_int_sta aux_sta;
> struct iwl_mvm_int_sta snif_sta;
> @@ -1593,8 +1582,6 @@ int iwl_mvm_up(struct iwl_mvm *mvm);
> int iwl_mvm_load_d3_fw(struct iwl_mvm *mvm);
>
> int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm);
> -bool iwl_mvm_bcast_filter_build_cmd(struct iwl_mvm *mvm,
> - struct iwl_bcast_filter_cmd *cmd);
>
> /*
> * FW notifications / CMD responses handlers
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> index 87630d38dc52..1f8b97995b94 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
> @@ -469,7 +469,6 @@ static const struct iwl_hcmd_names iwl_mvm_legacy_names[] = {
> HCMD_NAME(MCC_CHUB_UPDATE_CMD),
> HCMD_NAME(MARKER_CMD),
> HCMD_NAME(BT_PROFILE_NOTIFICATION),
> - HCMD_NAME(BCAST_FILTER_CMD),
> HCMD_NAME(MCAST_FILTER_CMD),
> HCMD_NAME(REPLY_SF_CFG_CMD),
> HCMD_NAME(REPLY_BEACON_FILTERING_CMD),

2022-02-01 20:05:26

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

Thorsten Leemhuis <[email protected]> writes:

> Hi, this is your Linux kernel regression tracker speaking.
>
> On 28.01.22 13:48, Luca Coelho wrote:
>> From: Luca Coelho <[email protected]>
>>
>> This feature has been deprecated and should not be used anymore. With
>> newer firmwares, namely *-67.ucode and above, trying to use it causes an
>> assertion failure in the FW, similar to this:
>>
>> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>>
>> In order to prevent this feature from being used, remove it entirely
>> and get rid of the Kconfig option that
>> enables it (IWLWIFI_BCAST_FILTERING).
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
>
> FWIW there was another report about it afaics:
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215550

If it's the same issue, it should be marked as a duplicate.

> That makes me hope that this is reviewed and merged to mainline relative
> quickly, otherwise more users will be bothered by this.
>
>> Signed-off-by: Luca Coelho <[email protected]>
>> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
>> Signed-off-by: Luca Coelho <[email protected]>
>
> Shouldn't this also have:
>
> Cc: [email protected] # 5.16.x

I can add that.

BTW, please trim your quotes. You left a really long (and unnecessary)
quote, which makes use of patchwork much harder for us maintainers.
Unfortunately patchwork is not able to trim them automatically:

https://patchwork.kernel.org/project/linux-wireless/patch/iwlwifi.20220128144623.9241e049f13e.Ia4f282813ca2ddd24c13427823519113f2bbebf2@changeid/

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-02-01 20:15:29

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

On 31.01.22 12:12, Kalle Valo wrote:
> Thorsten Leemhuis <[email protected]> writes:
>> On 28.01.22 13:48, Luca Coelho wrote:
>>> From: Luca Coelho <[email protected]>
>>>
>>> This feature has been deprecated and should not be used anymore. With
>>> newer firmwares, namely *-67.ucode and above, trying to use it causes an
>>> assertion failure in the FW, similar to this:
>>>
>>> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>>>
>>> In order to prevent this feature from being used, remove it entirely
>>> and get rid of the Kconfig option that
>>> enables it (IWLWIFI_BCAST_FILTERING).
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
>>
>> FWIW there was another report about it afaics:
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215550
>
> If it's the same issue, it should be marked as a duplicate.

Understandable request, but sorry, for now I decided to not do that in
such situations for two reasons:

* it can easily go wrong, as I encounter all sorts of kernel bugs and
thus often lack detailed knowledge about the areas the bug is about

* I'm doing regression tacking in my spare time, which is hard enough
already; taking care of bugs would make it a lot harder -- especially as
some maintainers/subsystems seem to (mostly) ignore bugzilla, so I would
be starting to do their job. Hence I only look at bugzilla to make sure
no regressions reported there falls through the cracks. See also:
https://lore.kernel.org/regressions/[email protected]/


>> That makes me hope that this is reviewed and merged to mainline relative
>> quickly, otherwise more users will be bothered by this.
>>
>>> Signed-off-by: Luca Coelho <[email protected]>
>>> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
>>> Signed-off-by: Luca Coelho <[email protected]>
>>
>> Shouldn't this also have:
>>
>> Cc: [email protected] # 5.16.x
>
> I can add that.

thx.

> BTW, please trim your quotes. You left a really long (and unnecessary)
> quote, which makes use of patchwork much harder for us maintainers.
> Unfortunately patchwork is not able to trim them automatically:

Argh, sorry, of course, will do that. I normally trim a lot more, but
when I'm in "regression tracker mode" I work differently and quote more,
as I often have to poke discussions that got stalled -- and there the
context might be helpful. But you are obviously right, for patches and
quite a few other situations that's obviously unneeded and harmful.

Ciao, Thorsten

2022-02-03 21:23:51

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

Thorsten Leemhuis <[email protected]> writes:

> On 31.01.22 12:12, Kalle Valo wrote:
>> Thorsten Leemhuis <[email protected]> writes:
>>> On 28.01.22 13:48, Luca Coelho wrote:
>>>> From: Luca Coelho <[email protected]>
>>>>
>>>> This feature has been deprecated and should not be used anymore. With
>>>> newer firmwares, namely *-67.ucode and above, trying to use it causes an
>>>> assertion failure in the FW, similar to this:
>>>>
>>>> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>>>>
>>>> In order to prevent this feature from being used, remove it entirely
>>>> and get rid of the Kconfig option that
>>>> enables it (IWLWIFI_BCAST_FILTERING).
>>>>
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
>>>
>>> FWIW there was another report about it afaics:
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215550
>>
>> If it's the same issue, it should be marked as a duplicate.
>
> Understandable request, but sorry, for now I decided to not do that in
> such situations for two reasons:
>
> * it can easily go wrong, as I encounter all sorts of kernel bugs and
> thus often lack detailed knowledge about the areas the bug is about
>
> * I'm doing regression tacking in my spare time, which is hard enough
> already; taking care of bugs would make it a lot harder -- especially as
> some maintainers/subsystems seem to (mostly) ignore bugzilla, so I would
> be starting to do their job. Hence I only look at bugzilla to make sure
> no regressions reported there falls through the cracks. See also:
> https://lore.kernel.org/regressions/[email protected]/

I share your pain, I'm also overwhelmed by mail and patches :) To be
exact, I didn't mean that you Thorsten should mark the bug duplicate and
I was hoping that someone from Intel would do it. And Golan thankfully
did that:

https://bugzilla.kernel.org/show_bug.cgi?id=215550#c1

And I also agree about challenges with Bugzilla. I personally have time
to only follow ath11k bugzilla reports, everything else I'm forced to
ignore. This is a big disconnect between users and developers which
worries me as well.

>>> That makes me hope that this is reviewed and merged to mainline relative
>>> quickly, otherwise more users will be bothered by this.
>>>
>>>> Signed-off-by: Luca Coelho <[email protected]>
>>>> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
>>>> Signed-off-by: Luca Coelho <[email protected]>
>>>
>>> Shouldn't this also have:
>>>
>>> Cc: [email protected] # 5.16.x
>>
>> I can add that.
>
> thx.
>
>> BTW, please trim your quotes. You left a really long (and unnecessary)
>> quote, which makes use of patchwork much harder for us maintainers.
>> Unfortunately patchwork is not able to trim them automatically:
>
> Argh, sorry, of course, will do that. I normally trim a lot more, but
> when I'm in "regression tracker mode" I work differently and quote more,
> as I often have to poke discussions that got stalled -- and there the
> context might be helpful. But you are obviously right, for patches and
> quite a few other situations that's obviously unneeded and harmful.

Yeah, understandable that quoting more is helpful for regression
reports. I wish patchwork would have a feature to automatically trim
unnecessary quotes, that would be really helpful for us maintainers.

BTW, I also have a feature request for regzbot :) I try to follow
regression emails very carefully, but I find the emails quite hard to
read as it's not easy to see what part of text is written by a bot and
which is from a human. I get so much email that I try to skim the emails
quickly and ignore all the boilerplate text, but with regzbot emails
that's difficult. One way to solve this might be to add all boilerplate
code between "-----[cut]------" lines or something like that.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-02-04 13:54:52

by Kalle Valo

[permalink] [raw]
Subject: Re: [for,v5.17,1/2] iwlwifi: remove deprecated broadcast filtering feature

Luca Coelho <[email protected]> wrote:

> From: Luca Coelho <[email protected]>
>
> This feature has been deprecated and should not be used anymore. With
> newer firmwares, namely *-67.ucode and above, trying to use it causes an
> assertion failure in the FW, similar to this:
>
> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>
> In order to prevent this feature from being used, remove it entirely
> and get rid of the Kconfig option that
> enables it (IWLWIFI_BCAST_FILTERING).
>
> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
> Cc: [email protected] # 5.16.x
> Signed-off-by: Luca Coelho <[email protected]>

2 patches applied to wireless.git, thanks.

92883a524ae9 iwlwifi: remove deprecated broadcast filtering feature
5f06f6bf8d81 iwlwifi: mvm: don't send SAR GEO command for 3160 devices

--
https://patchwork.kernel.org/project/linux-wireless/patch/iwlwifi.20220128144623.9241e049f13e.Ia4f282813ca2ddd24c13427823519113f2bbebf2@changeid/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-02-04 21:44:01

by Thorsten Leemhuis

[permalink] [raw]
Subject: Re: [PATCH for v5.17 1/2] iwlwifi: remove deprecated broadcast filtering feature

On 03.02.22 10:02, Kalle Valo wrote:
> Thorsten Leemhuis <[email protected]> writes:
>> On 31.01.22 12:12, Kalle Valo wrote:
>>> Thorsten Leemhuis <[email protected]> writes:
>>>> On 28.01.22 13:48, Luca Coelho wrote:
>>>>> From: Luca Coelho <[email protected]>
>>>>>
>>>>> This feature has been deprecated and should not be used anymore. With
>>>>> newer firmwares, namely *-67.ucode and above, trying to use it causes an
>>>>> assertion failure in the FW, similar to this:
>>>>>
>>>>> [Tue Jan 11 20:05:24 2022] iwlwifi 0000:04:00.0: 0x00001062 | ADVANCED_SYSASSERT
>>>>>
>>>>> In order to prevent this feature from being used, remove it entirely
>>>>> and get rid of the Kconfig option that
>>>>> enables it (IWLWIFI_BCAST_FILTERING).
>>>>>
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215488
>>>>
>>>> FWIW there was another report about it afaics:
>>>>
>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215550
>>>
>>> If it's the same issue, it should be marked as a duplicate.
>>
>> Understandable request, but sorry, for now I decided to not do that in
>> such situations for two reasons:
>>
>> * it can easily go wrong, as I encounter all sorts of kernel bugs and
>> thus often lack detailed knowledge about the areas the bug is about
>>
>> * I'm doing regression tacking in my spare time, which is hard enough
>> already; taking care of bugs would make it a lot harder -- especially as
>> some maintainers/subsystems seem to (mostly) ignore bugzilla, so I would
>> be starting to do their job. Hence I only look at bugzilla to make sure
>> no regressions reported there falls through the cracks. See also:
>> https://lore.kernel.org/regressions/[email protected]/
>
> I share your pain, I'm also overwhelmed by mail and patches :) To be
> exact, I didn't mean that you Thorsten should mark the bug duplicate and
> I was hoping that someone from Intel would do it. And Golan thankfully
> did that:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215550#c1

Ahh, good.

> And I also agree about challenges with Bugzilla. I personally have time
> to only follow ath11k bugzilla reports, everything else I'm forced to
> ignore. This is a big disconnect between users and developers which
> worries me as well.

Maybe I can come up with some numbers over the course of the next few
weeks to improve the situation. Or find somebody that would hire me to
work on improving things in that area...

>>>> That makes me hope that this is reviewed and merged to mainline relative
>>>> quickly, otherwise more users will be bothered by this.
>>>>
>>>>> Signed-off-by: Luca Coelho <[email protected]>
>>>>> Fixes: cbaa6aeedee5 ("iwlwifi: bump FW API to 67 for AX devices")
>>>>> Signed-off-by: Luca Coelho <[email protected]>
>>>>
>>>> Shouldn't this also have:
>>>>
>>>> Cc: [email protected] # 5.16.x
>>>
>>> I can add that.
>>
>> thx.
>>
>>> BTW, please trim your quotes. You left a really long (and unnecessary)
>>> quote, which makes use of patchwork much harder for us maintainers.
>>> Unfortunately patchwork is not able to trim them automatically:
>>
>> Argh, sorry, of course, will do that. I normally trim a lot more, but
>> when I'm in "regression tracker mode" I work differently and quote more,
>> as I often have to poke discussions that got stalled -- and there the
>> context might be helpful. But you are obviously right, for patches and
>> quite a few other situations that's obviously unneeded and harmful.
>
> Yeah, understandable that quoting more is helpful for regression
> reports. I wish patchwork would have a feature to automatically trim
> unnecessary quotes, that would be really helpful for us maintainers.
>
> BTW, I also have a feature request for regzbot :) I try to follow
> regression emails very carefully, but I find the emails quite hard to
> read as it's not easy to see what part of text is written by a bot and
> which is from a human.

Right now the bot doesn't write any emails except the weekly reports.
The text is thus coming from me, but I obviously have some templates.

> I get so much email that I try to skim the emails
> quickly and ignore all the boilerplate text, but with regzbot emails
> that's difficult. One way to solve this might be to add all boilerplate
> code between "-----[cut]------" lines or something like that.

Hmmm. I was trying to make this easy already. When I'm adding a report
to the tracking I for example these days put this at the top of my mail:

```
[TLDR: I'm adding this regression to regzbot, the Linux kernel
regression tracking bot; most text you find below is compiled from a few
templates paragraphs some of you might have seen already.]
```

And I sometimes even use top-posting.

Anyway, thx for brining this up, I guess there are still some areas
where I can improve things (which likely means to use top-posting even
more often, which kinda feels wrong, but for some of my mails might be
the better approach).

Ciao, Thorsten