2019-01-28 12:21:54

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 0/7] mt76x02: Beacon support for USB

We can configure beaconing, but without TBTT interrupt we
can not support PS buffering. This can be added later using
kernel hrtimer, if we can keep it in sycn with device timer.

I tested AP and IBSS modes.

Stanislaw Gruszka (7):
mt76x02: use mask for vifs
mt76x02: use commmon add interface for mt76x2u
mt76x02: initialize mutli bss mode when set up address
mt76x02: minor beaconing init changes
mt76x02: init beacon config for mt76x2u
mt76: beaconing fixes for USB
mt76x02: enable support for IBSS, AP and MESH

drivers/net/wireless/mediatek/mt76/mac80211.c | 3 +-
.../net/wireless/mediatek/mt76/mt76x0/pci.c | 7 ++
drivers/net/wireless/mediatek/mt76/mt76x02.h | 5 +-
.../net/wireless/mediatek/mt76/mt76x02_mac.c | 44 ++++++++++-
.../net/wireless/mediatek/mt76/mt76x02_mac.h | 6 +-
.../net/wireless/mediatek/mt76/mt76x02_util.c | 73 +++++++++----------
.../wireless/mediatek/mt76/mt76x2/pci_init.c | 12 ++-
.../wireless/mediatek/mt76/mt76x2/usb_init.c | 6 +-
.../wireless/mediatek/mt76/mt76x2/usb_main.c | 15 +---
9 files changed, 100 insertions(+), 71 deletions(-)

--
2.19.2



2019-01-28 12:21:59

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 1/7] mt76x02: use mask for vifs

Use vif_mask to count interfaces to allow to set mac address in HW
if there is only one interface and report error if we create
interface with wrong BSSID resulting in already used index.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
v2:
- do not change vif index calculation
- return error for already used index

drivers/net/wireless/mediatek/mt76/mt76x02.h | 2 ++
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 +++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02.h b/drivers/net/wireless/mediatek/mt76/mt76x02.h
index 6d96766a6ed3..be077443bdb0 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02.h
@@ -73,6 +73,8 @@ struct mt76x02_dev {

struct mutex phy_mutex;

+ u16 vif_mask;
+
u8 txdone_seq;
DECLARE_KFIFO_PTR(txstatus_fifo, struct mt76x02_tx_status);

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 062614ad0d51..2e805d5493de 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -292,7 +292,6 @@ mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)

if (vif->addr[0] & BIT(1))
idx = 1 + (((dev->mt76.macaddr[0] ^ vif->addr[0]) >> 2) & 7);
-
/*
* Client mode typically only has one configurable BSSID register,
* which is used for bssidx=0. This is linked to the MAC address.
@@ -309,6 +308,15 @@ mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
if (vif->type == NL80211_IFTYPE_STATION)
idx += 8;

+ if (dev->vif_mask & BIT(idx))
+ return -EBUSY;
+
+ /* Allow to change address in HW if we create first interface. */
+ if (!dev->vif_mask && !ether_addr_equal(dev->mt76.macaddr, vif->addr))
+ mt76x02_mac_setaddr(dev, vif->addr);
+
+ dev->vif_mask |= BIT(idx);
+
mt76x02_vif_init(dev, vif, idx);
return 0;
}
@@ -318,8 +326,10 @@ void mt76x02_remove_interface(struct ieee80211_hw *hw,
struct ieee80211_vif *vif)
{
struct mt76x02_dev *dev = hw->priv;
+ struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;

mt76_txq_remove(&dev->mt76, vif->txq);
+ dev->vif_mask &= ~BIT(mvif->idx);
}
EXPORT_SYMBOL_GPL(mt76x02_remove_interface);

--
2.19.2


2019-01-28 12:22:02

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 2/7] mt76x02: use commmon add interface for mt76x2u

Since we now support mt76x2u feature to allow set mac address
when creating interface in common code we can use it for
mt76x2u.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x02.h | 3 +--
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 6 +++---
.../net/wireless/mediatek/mt76/mt76x2/usb_main.c | 15 +--------------
3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02.h b/drivers/net/wireless/mediatek/mt76/mt76x02.h
index be077443bdb0..6d9d9ddacc32 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02.h
@@ -130,8 +130,7 @@ void mt76x02_sta_remove(struct mt76_dev *mdev, struct ieee80211_vif *vif,
struct ieee80211_sta *sta);

void mt76x02_config_mac_addr_list(struct mt76x02_dev *dev);
-void mt76x02_vif_init(struct mt76x02_dev *dev, struct ieee80211_vif *vif,
- unsigned int idx);
+
int mt76x02_add_interface(struct ieee80211_hw *hw,
struct ieee80211_vif *vif);
void mt76x02_remove_interface(struct ieee80211_hw *hw,
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 2e805d5493de..187e600cf22e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -268,8 +268,9 @@ void mt76x02_sta_remove(struct mt76_dev *mdev, struct ieee80211_vif *vif,
}
EXPORT_SYMBOL_GPL(mt76x02_sta_remove);

-void mt76x02_vif_init(struct mt76x02_dev *dev, struct ieee80211_vif *vif,
- unsigned int idx)
+static void
+mt76x02_vif_init(struct mt76x02_dev *dev, struct ieee80211_vif *vif,
+ unsigned int idx)
{
struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;
struct mt76_txq *mtxq;
@@ -282,7 +283,6 @@ void mt76x02_vif_init(struct mt76x02_dev *dev, struct ieee80211_vif *vif,

mt76_txq_init(&dev->mt76, vif->txq);
}
-EXPORT_SYMBOL_GPL(mt76x02_vif_init);

int
mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
index 286c7f451090..5256c6f879a7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c
@@ -46,19 +46,6 @@ static void mt76x2u_stop(struct ieee80211_hw *hw)
mutex_unlock(&dev->mt76.mutex);
}

-static int mt76x2u_add_interface(struct ieee80211_hw *hw,
- struct ieee80211_vif *vif)
-{
- struct mt76x02_dev *dev = hw->priv;
- unsigned int idx = 8;
-
- if (!ether_addr_equal(dev->mt76.macaddr, vif->addr))
- mt76x02_mac_setaddr(dev, vif->addr);
-
- mt76x02_vif_init(dev, vif, idx);
- return 0;
-}
-
static int
mt76x2u_set_channel(struct mt76x02_dev *dev,
struct cfg80211_chan_def *chandef)
@@ -125,7 +112,7 @@ const struct ieee80211_ops mt76x2u_ops = {
.tx = mt76x02_tx,
.start = mt76x2u_start,
.stop = mt76x2u_stop,
- .add_interface = mt76x2u_add_interface,
+ .add_interface = mt76x02_add_interface,
.remove_interface = mt76x02_remove_interface,
.sta_state = mt76_sta_state,
.set_key = mt76x02_set_key,
--
2.19.2


2019-01-28 12:22:07

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 3/7] mt76x02: initialize mutli bss mode when set up address

BSSID is not strtirct related with beaconing (for example we can have
2 STA vifs) and more related with MAC address, so initaize BSSID when
setting MAC address.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x02_mac.c | 15 ++++++++++++++-
drivers/net/wireless/mediatek/mt76/mt76x02_mac.h | 2 +-
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 13 ++-----------
.../net/wireless/mediatek/mt76/mt76x2/pci_init.c | 4 +---
4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index 63fa27d2c404..ae1727e433f3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -548,8 +548,11 @@ mt76x02_mac_process_rate(struct mt76x02_dev *dev,
return 0;
}

-void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr)
+void mt76x02_mac_setaddr(struct mt76x02_dev *dev, const u8 *addr)
{
+ static const u8 null_addr[ETH_ALEN] = {};
+ int i;
+
ether_addr_copy(dev->mt76.macaddr, addr);

if (!is_valid_ether_addr(dev->mt76.macaddr)) {
@@ -563,6 +566,16 @@ void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr)
mt76_wr(dev, MT_MAC_ADDR_DW1,
get_unaligned_le16(dev->mt76.macaddr + 4) |
FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff));
+
+ mt76_wr(dev, MT_MAC_BSSID_DW0,
+ get_unaligned_le32(dev->mt76.macaddr));
+ mt76_wr(dev, MT_MAC_BSSID_DW1,
+ get_unaligned_le16(dev->mt76.macaddr + 4) |
+ FIELD_PREP(MT_MAC_BSSID_DW1_MBSS_MODE, 3) | /* 8 APs + 8 STAs */
+ MT_MAC_BSSID_DW1_MBSS_LOCAL_BIT);
+
+ for (i = 0; i < 16; i++)
+ mt76x02_mac_set_bssid(dev, i, null_addr);
}
EXPORT_SYMBOL_GPL(mt76x02_mac_setaddr);

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index 940c07f665cd..76e564b8f5da 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -191,7 +191,7 @@ int mt76x02_mac_process_rx(struct mt76x02_dev *dev, struct sk_buff *skb,
void mt76x02_mac_set_tx_protection(struct mt76x02_dev *dev, bool legacy_prot,
int ht_mode);
void mt76x02_mac_set_rts_thresh(struct mt76x02_dev *dev, u32 val);
-void mt76x02_mac_setaddr(struct mt76x02_dev *dev, u8 *addr);
+void mt76x02_mac_setaddr(struct mt76x02_dev *dev, const u8 *addr);
void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
struct sk_buff *skb, struct mt76_wcid *wcid,
struct ieee80211_sta *sta, int len);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 187e600cf22e..361811d6510c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -667,16 +667,8 @@ static void mt76x02_set_beacon_offsets(struct mt76x02_dev *dev)

void mt76x02_init_beacon_config(struct mt76x02_dev *dev)
{
- static const u8 null_addr[ETH_ALEN] = {};
int i;

- mt76_wr(dev, MT_MAC_BSSID_DW0,
- get_unaligned_le32(dev->mt76.macaddr));
- mt76_wr(dev, MT_MAC_BSSID_DW1,
- get_unaligned_le16(dev->mt76.macaddr + 4) |
- FIELD_PREP(MT_MAC_BSSID_DW1_MBSS_MODE, 3) | /* 8 beacons */
- MT_MAC_BSSID_DW1_MBSS_LOCAL_BIT);
-
/* Fire a pre-TBTT interrupt 8 ms before TBTT */
mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_PRE_TBTT,
8 << 4);
@@ -686,10 +678,9 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev)

mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff);

- for (i = 0; i < 8; i++) {
- mt76x02_mac_set_bssid(dev, i, null_addr);
+ for (i = 0; i < 8; i++)
mt76x02_mac_set_beacon(dev, i, NULL);
- }
+
mt76x02_set_beacon_offsets(dev);
}
EXPORT_SYMBOL_GPL(mt76x02_init_beacon_config);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
index 4347d5e7a915..0ccaa64d97ec 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
@@ -119,9 +119,7 @@ static int mt76x2_mac_reset(struct mt76x02_dev *dev, bool hard)
mt76_wr(dev, MT_MCU_CLOCK_CTL, 0x1401);
mt76_clear(dev, MT_FCE_L2_STUFF, MT_FCE_L2_STUFF_WR_MPDU_LEN_EN);

- mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(macaddr));
- mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(macaddr + 4));
-
+ mt76x02_mac_setaddr(dev, macaddr);
mt76x02_init_beacon_config(dev);
if (!hard)
return 0;
--
2.19.2


2019-01-28 12:22:08

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 4/7] mt76x02: minor beaconing init changes

Disable BEACON timer during init and remove interrupt registers
initialization form generic code since they are PCIe specific.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x0/pci.c | 7 +++++++
drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 11 ++++-------
drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c | 8 ++++++++
3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
index 1472c8699b29..cb68af5b7478 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/pci.c
@@ -128,6 +128,13 @@ static int mt76x0e_register_device(struct mt76x02_dev *dev)
if (err < 0)
return err;

+ /* Beaconing: fire a pre-TBTT interrupt 8 ms before TBTT */
+ mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_PRE_TBTT,
+ 8 << 4);
+ mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_GP_TIMER,
+ MT_DFS_GP_INTERVAL);
+ mt76_wr(dev, MT_INT_TIMER_EN, 0);
+
if (mt76_chip(&dev->mt76) == 0x7610) {
u16 val;

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 361811d6510c..793258616831 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -669,13 +669,10 @@ void mt76x02_init_beacon_config(struct mt76x02_dev *dev)
{
int i;

- /* Fire a pre-TBTT interrupt 8 ms before TBTT */
- mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_PRE_TBTT,
- 8 << 4);
- mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_GP_TIMER,
- MT_DFS_GP_INTERVAL);
- mt76_wr(dev, MT_INT_TIMER_EN, 0);
-
+ mt76_clear(dev, MT_BEACON_TIME_CFG, (MT_BEACON_TIME_CFG_TIMER_EN |
+ MT_BEACON_TIME_CFG_SYNC_MODE |
+ MT_BEACON_TIME_CFG_TBTT_EN |
+ MT_BEACON_TIME_CFG_BEACON_TX));
mt76_wr(dev, MT_BCN_BYPASS_MASK, 0xffff);

for (i = 0; i < 8; i++)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
index 0ccaa64d97ec..f86ff035fa7f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci_init.c
@@ -121,6 +121,14 @@ static int mt76x2_mac_reset(struct mt76x02_dev *dev, bool hard)

mt76x02_mac_setaddr(dev, macaddr);
mt76x02_init_beacon_config(dev);
+
+ /* Fire a pre-TBTT interrupt 8 ms before TBTT */
+ mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_PRE_TBTT,
+ 8 << 4);
+ mt76_rmw_field(dev, MT_INT_TIMER_CFG, MT_INT_TIMER_CFG_GP_TIMER,
+ MT_DFS_GP_INTERVAL);
+ mt76_wr(dev, MT_INT_TIMER_EN, 0);
+
if (!hard)
return 0;

--
2.19.2


2019-01-28 12:22:09

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 5/7] mt76x02: init beacon config for mt76x2u

Initialize beaconing also on mt76x2u.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
index 0be3784f44fb..3737e9fa8295 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c
@@ -207,11 +207,7 @@ int mt76x2u_init_hardware(struct mt76x02_dev *dev)
mt76x02_mac_shared_key_setup(dev, i, k, NULL);
}

- mt76_clear(dev, MT_BEACON_TIME_CFG,
- MT_BEACON_TIME_CFG_TIMER_EN |
- MT_BEACON_TIME_CFG_SYNC_MODE |
- MT_BEACON_TIME_CFG_TBTT_EN |
- MT_BEACON_TIME_CFG_BEACON_TX);
+ mt76x02_init_beacon_config(dev);

mt76_rmw(dev, MT_US_CYC_CFG, MT_US_CYC_CNT, 0x1e);
mt76_wr(dev, MT_TXOP_CTRL_CFG, 0x583f);
--
2.19.2


2019-01-28 12:22:13

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 6/7] mt76: beaconing fixes for USB

Configure beaconing on USB devices without PS buffering support.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
v2: move some code
drivers/net/wireless/mediatek/mt76/mac80211.c | 3 +-
.../net/wireless/mediatek/mt76/mt76x02_mac.c | 29 +++++++++++++++++--
.../net/wireless/mediatek/mt76/mt76x02_mac.h | 4 +--
.../net/wireless/mediatek/mt76/mt76x02_util.c | 8 ++---
4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index ee3b65a14870..65433d9aca83 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -317,7 +317,6 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,

ieee80211_hw_set(hw, SIGNAL_DBM);
ieee80211_hw_set(hw, PS_NULLFUNC_STACK);
- ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
ieee80211_hw_set(hw, AMPDU_AGGREGATION);
ieee80211_hw_set(hw, SUPPORTS_RC_TABLE);
ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
@@ -329,6 +328,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
ieee80211_hw_set(hw, AP_LINK_PS);
ieee80211_hw_set(hw, REPORTS_TX_ACK_STATUS);
ieee80211_hw_set(hw, NEEDS_UNIQUE_STA_ADDR);
+ if (dev->bus->type == MT76_BUS_MMIO)
+ ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);

wiphy->flags |= WIPHY_FLAG_IBSS_RSN;

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index ae1727e433f3..3b8dc6757df1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -1060,8 +1060,9 @@ int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
return 0;
}

-void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
- u8 vif_idx, bool val)
+static void
+__mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev, u8 vif_idx,
+ bool val, struct sk_buff *skb)
{
u8 old_mask = dev->beacon_mask;
bool en;
@@ -1069,6 +1070,8 @@ void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,

if (val) {
dev->beacon_mask |= BIT(vif_idx);
+ if (skb)
+ mt76x02_mac_set_beacon(dev, vif_idx, skb);
} else {
dev->beacon_mask &= ~BIT(vif_idx);
mt76x02_mac_set_beacon(dev, vif_idx, NULL);
@@ -1079,14 +1082,34 @@ void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,

en = dev->beacon_mask;

- mt76_rmw_field(dev, MT_INT_TIMER_EN, MT_INT_TIMER_EN_PRE_TBTT_EN, en);
reg = MT_BEACON_TIME_CFG_BEACON_TX |
MT_BEACON_TIME_CFG_TBTT_EN |
MT_BEACON_TIME_CFG_TIMER_EN;
mt76_rmw(dev, MT_BEACON_TIME_CFG, reg, reg * en);

+ if (mt76_is_usb(dev))
+ return;
+
+ mt76_rmw_field(dev, MT_INT_TIMER_EN, MT_INT_TIMER_EN_PRE_TBTT_EN, en);
if (en)
mt76x02_irq_enable(dev, MT_INT_PRE_TBTT | MT_INT_TBTT);
else
mt76x02_irq_disable(dev, MT_INT_PRE_TBTT | MT_INT_TBTT);
}
+
+void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
+ struct ieee80211_vif *vif, bool val)
+{
+ u8 vif_idx = ((struct mt76x02_vif *)vif->drv_priv)->idx;
+ struct sk_buff *skb = NULL;
+
+ if (mt76_is_mmio(dev))
+ tasklet_disable(&dev->pre_tbtt_tasklet);
+ else
+ skb = ieee80211_beacon_get(mt76_hw(dev), vif);
+
+ __mt76x02_mac_set_beacon_enable(dev, vif_idx, val, skb);
+
+ if (mt76_is_mmio(dev))
+ tasklet_enable(&dev->pre_tbtt_tasklet);
+}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
index 76e564b8f5da..3b04b1bd0abd 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.h
@@ -204,8 +204,8 @@ void mt76x02_mac_work(struct work_struct *work);
void mt76x02_mac_set_bssid(struct mt76x02_dev *dev, u8 idx, const u8 *addr);
int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
struct sk_buff *skb);
-void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev, u8 vif_idx,
- bool val);
+void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
+ struct ieee80211_vif *vif, bool val);

void mt76x02_edcca_init(struct mt76x02_dev *dev);
#endif
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index 793258616831..edc84699d621 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -695,12 +695,8 @@ void mt76x02_bss_info_changed(struct ieee80211_hw *hw,
if (changed & BSS_CHANGED_BSSID)
mt76x02_mac_set_bssid(dev, mvif->idx, info->bssid);

- if (changed & BSS_CHANGED_BEACON_ENABLED) {
- tasklet_disable(&dev->pre_tbtt_tasklet);
- mt76x02_mac_set_beacon_enable(dev, mvif->idx,
- info->enable_beacon);
- tasklet_enable(&dev->pre_tbtt_tasklet);
- }
+ if (changed & BSS_CHANGED_BEACON_ENABLED)
+ mt76x02_mac_set_beacon_enable(dev, vif, info->enable_beacon);

if (changed & BSS_CHANGED_HT || changed & BSS_CHANGED_ERP_CTS_PROT)
mt76x02_mac_set_tx_protection(dev, info->use_cts_prot,
--
2.19.2


2019-01-28 12:22:15

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH v2 7/7] mt76x02: enable support for IBSS, AP and MESH

Since we implement beconing on USB now, similar interfaces should be
supported for USB as are for MMIO. Tested only for AP and IBSS modes.

Signed-off-by: Stanislaw Gruszka <[email protected]>
---
v2: set VHT_IBSS, vht reates are supported in ibss.

.../net/wireless/mediatek/mt76/mt76x02_util.c | 23 ++++++++++---------
1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
index edc84699d621..7f3747d0791f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
@@ -150,19 +150,8 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
mt76x02_dfs_init_detector(dev);

wiphy->reg_notifier = mt76x02_regd_notifier;
- wiphy->iface_combinations = mt76x02_if_comb;
- wiphy->n_iface_combinations = ARRAY_SIZE(mt76x02_if_comb);
- wiphy->interface_modes =
- BIT(NL80211_IFTYPE_STATION) |
- BIT(NL80211_IFTYPE_AP) |
-#ifdef CONFIG_MAC80211_MESH
- BIT(NL80211_IFTYPE_MESH_POINT) |
-#endif
- BIT(NL80211_IFTYPE_ADHOC);
-
wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;

- wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_VHT_IBSS);

/* init led callbacks */
if (IS_ENABLED(CONFIG_MT76_LEDS)) {
@@ -172,6 +161,18 @@ void mt76x02_init_device(struct mt76x02_dev *dev)
}
}

+ wiphy->iface_combinations = mt76x02_if_comb;
+ wiphy->n_iface_combinations = ARRAY_SIZE(mt76x02_if_comb);
+ wiphy->interface_modes =
+ BIT(NL80211_IFTYPE_STATION) |
+ BIT(NL80211_IFTYPE_AP) |
+#ifdef CONFIG_MAC80211_MESH
+ BIT(NL80211_IFTYPE_MESH_POINT) |
+#endif
+ BIT(NL80211_IFTYPE_ADHOC);
+
+ wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_VHT_IBSS);
+
hw->sta_data_size = sizeof(struct mt76x02_sta);
hw->vif_data_size = sizeof(struct mt76x02_vif);

--
2.19.2


2019-01-28 13:35:50

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] mt76x02: use mask for vifs

> Use vif_mask to count interfaces to allow to set mac address in HW
> if there is only one interface and report error if we create
> interface with wrong BSSID resulting in already used index.
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---
> v2:
> - do not change vif index calculation
> - return error for already used index
>
> drivers/net/wireless/mediatek/mt76/mt76x02.h | 2 ++
> drivers/net/wireless/mediatek/mt76/mt76x02_util.c | 12 +++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02.h b/drivers/net/wireless/mediatek/mt76/mt76x02.h
> index 6d96766a6ed3..be077443bdb0 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02.h
> @@ -73,6 +73,8 @@ struct mt76x02_dev {
>
> struct mutex phy_mutex;
>
> + u16 vif_mask;
> +
> u8 txdone_seq;
> DECLARE_KFIFO_PTR(txstatus_fifo, struct mt76x02_tx_status);
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> index 062614ad0d51..2e805d5493de 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_util.c
> @@ -292,7 +292,6 @@ mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
>
> if (vif->addr[0] & BIT(1))
> idx = 1 + (((dev->mt76.macaddr[0] ^ vif->addr[0]) >> 2) & 7);
> -
> /*
> * Client mode typically only has one configurable BSSID register,
> * which is used for bssidx=0. This is linked to the MAC address.
> @@ -309,6 +308,15 @@ mt76x02_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
> if (vif->type == NL80211_IFTYPE_STATION)
> idx += 8;
>

Maybe we shoud grub dev->mt76.mutex here, since we can have multiple processes trying
to add an interface

> + if (dev->vif_mask & BIT(idx))
> + return -EBUSY;
> +
> + /* Allow to change address in HW if we create first interface. */
> + if (!dev->vif_mask && !ether_addr_equal(dev->mt76.macaddr, vif->addr))
> + mt76x02_mac_setaddr(dev, vif->addr);
> +
> + dev->vif_mask |= BIT(idx);
> +
> mt76x02_vif_init(dev, vif, idx);
> return 0;
> }
> @@ -318,8 +326,10 @@ void mt76x02_remove_interface(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif)
> {
> struct mt76x02_dev *dev = hw->priv;
> + struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;
>
> mt76_txq_remove(&dev->mt76, vif->txq);
> + dev->vif_mask &= ~BIT(mvif->idx);

If we are removing the first configured interface, should we have to update
MT_MAC_ADDR_DW{0,1} regs?

Regards,
Lorenzo

> }
> EXPORT_SYMBOL_GPL(mt76x02_remove_interface);
>
> --
> 2.19.2
>

2019-01-28 13:44:22

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] mt76: beaconing fixes for USB

[...]

> +
> +void mt76x02_mac_set_beacon_enable(struct mt76x02_dev *dev,
> + struct ieee80211_vif *vif, bool val)
> +{
> + u8 vif_idx = ((struct mt76x02_vif *)vif->drv_priv)->idx;
> + struct sk_buff *skb = NULL;
> +
> + if (mt76_is_mmio(dev))
> + tasklet_disable(&dev->pre_tbtt_tasklet);
> + else
> + skb = ieee80211_beacon_get(mt76_hw(dev), vif);

I guess we can avoid to allocate the skb here if we are disabling 'beaconing'

> +
> + __mt76x02_mac_set_beacon_enable(dev, vif_idx, val, skb);
> +
> + if (mt76_is_mmio(dev))
> + tasklet_enable(&dev->pre_tbtt_tasklet);
> +}

2019-01-28 14:24:08

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] mt76x02: use mask for vifs

On Mon, Jan 28, 2019 at 02:35:39PM +0100, Lorenzo Bianconi wrote:
> >
>
> Maybe we shoud grub dev->mt76.mutex here, since we can have multiple processes trying
> to add an interface

Interface creation and deletion are protected by rtnl_lock(), can not
be performed concurrently.

> > @@ -318,8 +326,10 @@ void mt76x02_remove_interface(struct ieee80211_hw *hw,
> > struct ieee80211_vif *vif)
> > {
> > struct mt76x02_dev *dev = hw->priv;
> > + struct mt76x02_vif *mvif = (struct mt76x02_vif *)vif->drv_priv;
> >
> > mt76_txq_remove(&dev->mt76, vif->txq);
> > + dev->vif_mask &= ~BIT(mvif->idx);
>
> If we are removing the first configured interface, should we have to update
> MT_MAC_ADDR_DW{0,1} regs?

What for ?

Thanks
Stanislaw

2019-01-28 14:31:42

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] mt76x02: use mask for vifs

> On Mon, Jan 28, 2019 at 02:35:39PM +0100, Lorenzo Bianconi wrote:
> > >
> >
> > Maybe we shoud grub dev->mt76.mutex here, since we can have multiple processes trying
> > to add an interface
>
> Interface creation and deletion are protected by rtnl_lock(), can not
> be performed concurrently.

Yes, right :)

2019-01-29 11:49:47

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On 2019-01-29 12:47, Kalle Valo wrote:
> Stanislaw Gruszka <[email protected]> writes:
>
>> We can configure beaconing, but without TBTT interrupt we
>> can not support PS buffering. This can be added later using
>> kernel hrtimer, if we can keep it in sycn with device timer.
>>
>> I tested AP and IBSS modes.
>
> So how does this work reliably so that there's no packet loss with
> clients using power save?
There will be multicast packet loss for clients using power save.

- Felix

2019-01-29 11:59:25

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

Stanislaw Gruszka <[email protected]> writes:

> We can configure beaconing, but without TBTT interrupt we
> can not support PS buffering. This can be added later using
> kernel hrtimer, if we can keep it in sycn with device timer.
>
> I tested AP and IBSS modes.

So how does this work reliably so that there's no packet loss with
clients using power save?

--
Kalle Valo

2019-01-29 12:07:22

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

Felix Fietkau <[email protected]> writes:

> On 2019-01-29 12:47, Kalle Valo wrote:
>> Stanislaw Gruszka <[email protected]> writes:
>>
>>> We can configure beaconing, but without TBTT interrupt we
>>> can not support PS buffering. This can be added later using
>>> kernel hrtimer, if we can keep it in sycn with device timer.
>>>
>>> I tested AP and IBSS modes.
>>
>> So how does this work reliably so that there's no packet loss with
>> clients using power save?
>
> There will be multicast packet loss for clients using power save.

Isn't that a problem? At least as a normal user I would very frustrated
if sometimes my connection work and sometimes not, for example if I'm
trying discover devices from my network. Hopefully nobody won't use USB
devices for any real AP stuff, but still enabling something which we
know doesn't work realiably is concerning.

--
Kalle Valo

2019-01-29 12:10:11

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On 2019-01-29 13:07, Kalle Valo wrote:
> Felix Fietkau <[email protected]> writes:
>
>> On 2019-01-29 12:47, Kalle Valo wrote:
>>> Stanislaw Gruszka <[email protected]> writes:
>>>
>>>> We can configure beaconing, but without TBTT interrupt we
>>>> can not support PS buffering. This can be added later using
>>>> kernel hrtimer, if we can keep it in sycn with device timer.
>>>>
>>>> I tested AP and IBSS modes.
>>>
>>> So how does this work reliably so that there's no packet loss with
>>> clients using power save?
>>
>> There will be multicast packet loss for clients using power save.
>
> Isn't that a problem? At least as a normal user I would very frustrated
> if sometimes my connection work and sometimes not, for example if I'm
> trying discover devices from my network. Hopefully nobody won't use USB
> devices for any real AP stuff, but still enabling something which we
> know doesn't work realiably is concerning.
I agree. Maybe we should leave out the flag for AP mode in this patch
until we have PS buffering and leave the rest of the code intact.

- Felix

2019-01-29 12:18:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

Felix Fietkau <[email protected]> writes:

> On 2019-01-29 13:07, Kalle Valo wrote:
>> Felix Fietkau <[email protected]> writes:
>>
>>> On 2019-01-29 12:47, Kalle Valo wrote:
>>>> Stanislaw Gruszka <[email protected]> writes:
>>>>
>>>>> We can configure beaconing, but without TBTT interrupt we
>>>>> can not support PS buffering. This can be added later using
>>>>> kernel hrtimer, if we can keep it in sycn with device timer.
>>>>>
>>>>> I tested AP and IBSS modes.
>>>>
>>>> So how does this work reliably so that there's no packet loss with
>>>> clients using power save?
>>>
>>> There will be multicast packet loss for clients using power save.
>>
>> Isn't that a problem? At least as a normal user I would very frustrated
>> if sometimes my connection work and sometimes not, for example if I'm
>> trying discover devices from my network. Hopefully nobody won't use USB
>> devices for any real AP stuff, but still enabling something which we
>> know doesn't work realiably is concerning.
>
> I agree. Maybe we should leave out the flag for AP mode in this patch
> until we have PS buffering and leave the rest of the code intact.

At least for me that sounds good.

--
Kalle Valo

2019-01-29 12:41:03

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

> Felix Fietkau <[email protected]> writes:
>
> > On 2019-01-29 13:07, Kalle Valo wrote:
> >> Felix Fietkau <[email protected]> writes:
> >>
> >>> On 2019-01-29 12:47, Kalle Valo wrote:
> >>>> Stanislaw Gruszka <[email protected]> writes:
> >>>>
> >>>>> We can configure beaconing, but without TBTT interrupt we
> >>>>> can not support PS buffering. This can be added later using
> >>>>> kernel hrtimer, if we can keep it in sycn with device timer.
> >>>>>
> >>>>> I tested AP and IBSS modes.
> >>>>
> >>>> So how does this work reliably so that there's no packet loss with
> >>>> clients using power save?
> >>>
> >>> There will be multicast packet loss for clients using power save.
> >>
> >> Isn't that a problem? At least as a normal user I would very frustrated
> >> if sometimes my connection work and sometimes not, for example if I'm
> >> trying discover devices from my network. Hopefully nobody won't use USB
> >> devices for any real AP stuff, but still enabling something which we
> >> know doesn't work realiably is concerning.
> >
> > I agree. Maybe we should leave out the flag for AP mode in this patch
> > until we have PS buffering and leave the rest of the code intact.
>
> At least for me that sounds good.

We can support ps buffering in AP as well using a hrtimer. In this way we
can reuse most of the existing code

Regards,
Lorenzo

>
> --
> Kalle Valo

2019-01-30 08:29:50

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote:
> On 2019-01-29 13:07, Kalle Valo wrote:
> > Felix Fietkau <[email protected]> writes:
> >
> >> On 2019-01-29 12:47, Kalle Valo wrote:
> >>> Stanislaw Gruszka <[email protected]> writes:
> >>>
> >>>> We can configure beaconing, but without TBTT interrupt we
> >>>> can not support PS buffering. This can be added later using
> >>>> kernel hrtimer, if we can keep it in sycn with device timer.
> >>>>
> >>>> I tested AP and IBSS modes.
> >>>
> >>> So how does this work reliably so that there's no packet loss with
> >>> clients using power save?
> >>
> >> There will be multicast packet loss for clients using power save.
> >
> > Isn't that a problem? At least as a normal user I would very frustrated
> > if sometimes my connection work and sometimes not, for example if I'm
> > trying discover devices from my network. Hopefully nobody won't use USB
> > devices for any real AP stuff, but still enabling something which we
> > know doesn't work realiably is concerning.
> I agree. Maybe we should leave out the flag for AP mode in this patch
> until we have PS buffering and leave the rest of the code intact.

But how serious problem of dropping multicast frames for PS stations is?
I don't think from user perspective this is "sometimes my connection
work and sometimes not", but something much less annoying.

Another thing is that this (D)TIM PS is not reliable by design,
that why UAPSD was introduced.

Moreover in the tree we have already bunch of drivers that do
advertise AP mode support without HOST_BROADCAST_PS_BUFFERING
like iwlwifi or brcm80211 .

So I don't think we should drop AP flag, AP mode works with
this patch set quite well.

Thanks
Stanislaw


2019-01-30 08:37:54

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote:
> > Felix Fietkau <[email protected]> writes:
> >
> > > On 2019-01-29 13:07, Kalle Valo wrote:
> > >> Felix Fietkau <[email protected]> writes:
> > >>
> > >>> On 2019-01-29 12:47, Kalle Valo wrote:
> > >>>> Stanislaw Gruszka <[email protected]> writes:
> > >>>>
> > >>>>> We can configure beaconing, but without TBTT interrupt we
> > >>>>> can not support PS buffering. This can be added later using
> > >>>>> kernel hrtimer, if we can keep it in sycn with device timer.
> > >>>>>
> > >>>>> I tested AP and IBSS modes.
> > >>>>
> > >>>> So how does this work reliably so that there's no packet loss with
> > >>>> clients using power save?
> > >>>
> > >>> There will be multicast packet loss for clients using power save.
> > >>
> > >> Isn't that a problem? At least as a normal user I would very frustrated
> > >> if sometimes my connection work and sometimes not, for example if I'm
> > >> trying discover devices from my network. Hopefully nobody won't use USB
> > >> devices for any real AP stuff, but still enabling something which we
> > >> know doesn't work realiably is concerning.
> > >
> > > I agree. Maybe we should leave out the flag for AP mode in this patch
> > > until we have PS buffering and leave the rest of the code intact.
> >
> > At least for me that sounds good.
>
> We can support ps buffering in AP as well using a hrtimer. In this way we
> can reuse most of the existing code

Yes, but there is issue to address, since kernel timer and device TBT
timer are independed, they possibly can get out of sync after some time,
for example few hours or days. So there is need to prevent/fix this
somehow.

Thanks
Stanislaw

2019-01-30 09:16:37

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On 2019-01-30 09:37, Stanislaw Gruszka wrote:
> On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote:
>> > Felix Fietkau <[email protected]> writes:
>> >
>> > > On 2019-01-29 13:07, Kalle Valo wrote:
>> > >> Felix Fietkau <[email protected]> writes:
>> > >>
>> > >>> On 2019-01-29 12:47, Kalle Valo wrote:
>> > >>>> Stanislaw Gruszka <[email protected]> writes:
>> > >>>>
>> > >>>>> We can configure beaconing, but without TBTT interrupt we
>> > >>>>> can not support PS buffering. This can be added later using
>> > >>>>> kernel hrtimer, if we can keep it in sycn with device timer.
>> > >>>>>
>> > >>>>> I tested AP and IBSS modes.
>> > >>>>
>> > >>>> So how does this work reliably so that there's no packet loss with
>> > >>>> clients using power save?
>> > >>>
>> > >>> There will be multicast packet loss for clients using power save.
>> > >>
>> > >> Isn't that a problem? At least as a normal user I would very frustrated
>> > >> if sometimes my connection work and sometimes not, for example if I'm
>> > >> trying discover devices from my network. Hopefully nobody won't use USB
>> > >> devices for any real AP stuff, but still enabling something which we
>> > >> know doesn't work realiably is concerning.
>> > >
>> > > I agree. Maybe we should leave out the flag for AP mode in this patch
>> > > until we have PS buffering and leave the rest of the code intact.
>> >
>> > At least for me that sounds good.
>>
>> We can support ps buffering in AP as well using a hrtimer. In this way we
>> can reuse most of the existing code
>
> Yes, but there is issue to address, since kernel timer and device TBT
> timer are independed, they possibly can get out of sync after some time,
> for example few hours or days. So there is need to prevent/fix this
> somehow.
We could read the TSF timer value from the hardware and sync the hrtimer
against that.

- Felix

2019-01-30 09:26:01

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On 2019-01-30 09:29, Stanislaw Gruszka wrote:
> On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote:
>> On 2019-01-29 13:07, Kalle Valo wrote:
>> > Felix Fietkau <[email protected]> writes:
>> >
>> >> On 2019-01-29 12:47, Kalle Valo wrote:
>> >>> Stanislaw Gruszka <[email protected]> writes:
>> >>>
>> >>>> We can configure beaconing, but without TBTT interrupt we
>> >>>> can not support PS buffering. This can be added later using
>> >>>> kernel hrtimer, if we can keep it in sycn with device timer.
>> >>>>
>> >>>> I tested AP and IBSS modes.
>> >>>
>> >>> So how does this work reliably so that there's no packet loss with
>> >>> clients using power save?
>> >>
>> >> There will be multicast packet loss for clients using power save.
>> >
>> > Isn't that a problem? At least as a normal user I would very frustrated
>> > if sometimes my connection work and sometimes not, for example if I'm
>> > trying discover devices from my network. Hopefully nobody won't use USB
>> > devices for any real AP stuff, but still enabling something which we
>> > know doesn't work realiably is concerning.
>> I agree. Maybe we should leave out the flag for AP mode in this patch
>> until we have PS buffering and leave the rest of the code intact.
>
> But how serious problem of dropping multicast frames for PS stations is?
> I don't think from user perspective this is "sometimes my connection
> work and sometimes not", but something much less annoying.
Actually, if you're considering two stations on an AP trying to connect
to each other, it is exactly "sometimes my connection work and sometimes
not", because of ARP. Clients won't see the ARP requests sent to each
other, if the client being asked is in powersave mode.

> Another thing is that this (D)TIM PS is not reliable by design,
> that why UAPSD was introduced.
UAPSD doesn't replace TIM based powersave indication, and I'm pretty
sure it doesn't handle multicast either.

> Moreover in the tree we have already bunch of drivers that do
> advertise AP mode support without HOST_BROADCAST_PS_BUFFERING
> like iwlwifi or brcm80211 .
I haven't looked at the code, but they may be handling buffered
multicast in a different way, possibly with firmware involvement.

- Felix

2019-01-30 10:12:09

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On Wed, Jan 30, 2019 at 10:16:34AM +0100, Felix Fietkau wrote:
> On 2019-01-30 09:37, Stanislaw Gruszka wrote:
> > On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote:
> >> > Felix Fietkau <[email protected]> writes:
> >> >
> >> > > On 2019-01-29 13:07, Kalle Valo wrote:
> >> > >> Felix Fietkau <[email protected]> writes:
> >> > >>
> >> > >>> On 2019-01-29 12:47, Kalle Valo wrote:
> >> > >>>> Stanislaw Gruszka <[email protected]> writes:
> >> > >>>>
> >> > >>>>> We can configure beaconing, but without TBTT interrupt we
> >> > >>>>> can not support PS buffering. This can be added later using
> >> > >>>>> kernel hrtimer, if we can keep it in sycn with device timer.
> >> > >>>>>
> >> > >>>>> I tested AP and IBSS modes.
> >> > >>>>
> >> > >>>> So how does this work reliably so that there's no packet loss with
> >> > >>>> clients using power save?
> >> > >>>
> >> > >>> There will be multicast packet loss for clients using power save.
> >> > >>
> >> > >> Isn't that a problem? At least as a normal user I would very frustrated
> >> > >> if sometimes my connection work and sometimes not, for example if I'm
> >> > >> trying discover devices from my network. Hopefully nobody won't use USB
> >> > >> devices for any real AP stuff, but still enabling something which we
> >> > >> know doesn't work realiably is concerning.
> >> > >
> >> > > I agree. Maybe we should leave out the flag for AP mode in this patch
> >> > > until we have PS buffering and leave the rest of the code intact.
> >> >
> >> > At least for me that sounds good.
> >>
> >> We can support ps buffering in AP as well using a hrtimer. In this way we
> >> can reuse most of the existing code
> >
> > Yes, but there is issue to address, since kernel timer and device TBT
> > timer are independed, they possibly can get out of sync after some time,
> > for example few hours or days. So there is need to prevent/fix this
> > somehow.
> We could read the TSF timer value from the hardware and sync the hrtimer
> against that.

Ok then. I'll implement hrtimer solution and repost this set. Patch
6 will no longer be necessery.

Regards
Stanislaw

2019-01-30 10:27:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

Stanislaw Gruszka <[email protected]> writes:

> On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote:
>> On 2019-01-29 13:07, Kalle Valo wrote:
>> > Felix Fietkau <[email protected]> writes:
>> >
>> >> On 2019-01-29 12:47, Kalle Valo wrote:
>> >>> Stanislaw Gruszka <[email protected]> writes:
>> >>>
>> >>>> We can configure beaconing, but without TBTT interrupt we
>> >>>> can not support PS buffering. This can be added later using
>> >>>> kernel hrtimer, if we can keep it in sycn with device timer.
>> >>>>
>> >>>> I tested AP and IBSS modes.
>> >>>
>> >>> So how does this work reliably so that there's no packet loss with
>> >>> clients using power save?
>> >>
>> >> There will be multicast packet loss for clients using power save.
>> >
>> > Isn't that a problem? At least as a normal user I would very frustrated
>> > if sometimes my connection work and sometimes not, for example if I'm
>> > trying discover devices from my network. Hopefully nobody won't use USB
>> > devices for any real AP stuff, but still enabling something which we
>> > know doesn't work realiably is concerning.
>> I agree. Maybe we should leave out the flag for AP mode in this patch
>> until we have PS buffering and leave the rest of the code intact.
>
> But how serious problem of dropping multicast frames for PS stations
> is?

It's _both_ broadcast and multicast frames. Felix already mentioned ARP
but there are also other protocols which rely on broadcast/multicast
frames.

> I don't think from user perspective this is "sometimes my connection
> work and sometimes not", but something much less annoying.

So basically you are saying that we should just depend on lock and hope
that users don't notice the packet loss. I don't think that's really
good design philosophy.

> Moreover in the tree we have already bunch of drivers that do
> advertise AP mode support without HOST_BROADCAST_PS_BUFFERING
> like iwlwifi or brcm80211 .

After seeing how much those drivers are tested I would be very surprised
to see that their broadcast and multicast packet handling is broken.

> So I don't think we should drop AP flag, AP mode works with
> this patch set quite well.

"Works" is a very broad statement and depends on what you have tested.
Sure, if you tested tested with ping and iperf from a client to AP
without using power save mode that will work. But if you try to connect
from another device on the network to another client using aggressive
power saving it's a totally different situation.

Back in the Nokia N800 days I wasted a lot of time debugging all sort of
buggy APs which didn't work correctly with power save, it was really
annoying and very frustrating for the users. Let's not do the same
mistakes, we are better than that.

--
Kalle Valo

2019-01-30 15:23:03

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On Wed, Jan 30, 2019 at 11:07:48AM +0100, Stanislaw Gruszka wrote:
> > >> We can support ps buffering in AP as well using a hrtimer. In this way we
> > >> can reuse most of the existing code
> > >
> > > Yes, but there is issue to address, since kernel timer and device TBT
> > > timer are independed, they possibly can get out of sync after some time,
> > > for example few hours or days. So there is need to prevent/fix this
> > > somehow.
> > We could read the TSF timer value from the hardware and sync the hrtimer
> > against that.
>
> Ok then. I'll implement hrtimer solution and repost this set. Patch
> 6 will no longer be necessery.

This require more changes than I expected to allow programming registers
from atomic context for mt76-usb. So I'll repost without AP support for
now.

Stanislaw

2019-02-05 15:02:44

by Stanislaw Gruszka

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] mt76x02: Beacon support for USB

On Wed, Jan 30, 2019 at 04:22:55PM +0100, Stanislaw Gruszka wrote:
> On Wed, Jan 30, 2019 at 11:07:48AM +0100, Stanislaw Gruszka wrote:
> > > >> We can support ps buffering in AP as well using a hrtimer. In this way we
> > > >> can reuse most of the existing code
> > > >
> > > > Yes, but there is issue to address, since kernel timer and device TBT
> > > > timer are independed, they possibly can get out of sync after some time,
> > > > for example few hours or days. So there is need to prevent/fix this
> > > > somehow.
> > > We could read the TSF timer value from the hardware and sync the hrtimer
> > > against that.
> >
> > Ok then. I'll implement hrtimer solution and repost this set. Patch
> > 6 will no longer be necessery.
>
> This require more changes than I expected to allow programming registers
> from atomic context for mt76-usb. So I'll repost without AP support for
> now.

There are more obstacles here. On USB we do not have PSD queue available
via endpoinds. I have some solutions to this problem, but non of them work
satisfactory so far. Still experimenting ...

Stanislaw