[PATCH 0/9] ath10k: htt rx fixes and clean ups

[PATCH 0/9] ath10k: htt rx fixes and clean ups

[Michal Kazior]

Hi,

I've found a couple of issues while reworking rx
path.


Michal Kazior (9):
  ath10k: don't drop control and null func Rx
  ath10k: remove unused variable
  ath10k: use ieee80211 defines for crypto param lengths
  ath10k: fix rx buffer tracing
  ath10k: deduplicate htt rx dma unmapping
  ath10k: don't drop frames aggressively
  ath10k: add extra sanity check when popping amsdu
  ath10k: don't forget to replenish after fragmented Rx
  ath10k: clear htt->rx_confused on load

 drivers/net/wireless/ath/ath10k/htt_rx.c | 91 ++++++++++++++------------------
 1 file changed, 39 insertions(+), 52 deletions(-)

-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 0/9] ath10k: htt rx fixes and clean ups

[Michal Kazior]

Hi,

I've found a couple of issues while reworking rx
path.


Michal Kazior (9):
  ath10k: don't drop control and null func Rx
  ath10k: remove unused variable
  ath10k: use ieee80211 defines for crypto param lengths
  ath10k: fix rx buffer tracing
  ath10k: deduplicate htt rx dma unmapping
  ath10k: don't drop frames aggressively
  ath10k: add extra sanity check when popping amsdu
  ath10k: don't forget to replenish after fragmented Rx
  ath10k: clear htt->rx_confused on load

 drivers/net/wireless/ath/ath10k/htt_rx.c | 91 ++++++++++++++------------------
 1 file changed, 39 insertions(+), 52 deletions(-)

-- 
1.8.5.3

[PATCH 1/9] ath10k: don't drop control and null func Rx

[Michal Kazior]

HTT_RX_IND_MPDU_STATUS_MGMT_CTRL was pretty greedy
and because of that ath10k ended up dropping
Control Frames as well as Null Func frames.

Reported-by: Okhwan Lee <ohlee@mwnl.snu.ac.kr>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index fbb3175..63c69d8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1200,8 +1200,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 	}
 
 	/* Skip mgmt frames while we handle this in WMI */
-	if (status == HTT_RX_IND_MPDU_STATUS_MGMT_CTRL ||
-	    attention & RX_ATTENTION_FLAGS_MGMT_TYPE) {
+	if (attention & RX_ATTENTION_FLAGS_MGMT_TYPE) {
 		ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
 		return false;
 	}
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 1/9] ath10k: don't drop control and null func Rx

[Michal Kazior]

HTT_RX_IND_MPDU_STATUS_MGMT_CTRL was pretty greedy
and because of that ath10k ended up dropping
Control Frames as well as Null Func frames.

Reported-by: Okhwan Lee <ohlee@mwnl.snu.ac.kr>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index fbb3175..63c69d8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1200,8 +1200,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 	}
 
 	/* Skip mgmt frames while we handle this in WMI */
-	if (status == HTT_RX_IND_MPDU_STATUS_MGMT_CTRL ||
-	    attention & RX_ATTENTION_FLAGS_MGMT_TYPE) {
+	if (attention & RX_ATTENTION_FLAGS_MGMT_TYPE) {
 		ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
 		return false;
 	}
-- 
1.8.5.3

[PATCH 2/9] ath10k: remove unused variable

[Michal Kazior]

The rx descriptor variable was no longer used in
the rx handler.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 63c69d8..61ff2ac 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1230,7 +1230,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	struct ath10k *ar = htt->ar;
 	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
-	struct htt_rx_desc *rxd;
 	enum htt_rx_mpdu_status status;
 	struct ieee80211_hdr *hdr;
 	int num_mpdu_ranges;
@@ -1301,10 +1300,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				continue;
 			}
 
-			rxd = container_of((void *)msdu_head->data,
-					   struct htt_rx_desc,
-					   msdu_payload);
-
 			if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
 							 status,
 							 channel_set,
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 2/9] ath10k: remove unused variable

[Michal Kazior]

The rx descriptor variable was no longer used in
the rx handler.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 63c69d8..61ff2ac 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1230,7 +1230,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	struct ath10k *ar = htt->ar;
 	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
-	struct htt_rx_desc *rxd;
 	enum htt_rx_mpdu_status status;
 	struct ieee80211_hdr *hdr;
 	int num_mpdu_ranges;
@@ -1301,10 +1300,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				continue;
 			}
 
-			rxd = container_of((void *)msdu_head->data,
-					   struct htt_rx_desc,
-					   msdu_payload);
-
 			if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
 							 status,
 							 channel_set,
-- 
1.8.5.3

[PATCH 3/9] ath10k: use ieee80211 defines for crypto param lengths

[Michal Kazior]

Use the globally defined ieee80211 values instead
of re-defining them in the driver again.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 34 +++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 61ff2ac..ca466ce 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -588,41 +588,47 @@ static int ath10k_htt_rx_crypto_param_len(struct ath10k *ar,
 					  enum htt_rx_mpdu_encrypt_type type)
 {
 	switch (type) {
+	case HTT_RX_MPDU_ENCRYPT_NONE:
+		return 0;
 	case HTT_RX_MPDU_ENCRYPT_WEP40:
 	case HTT_RX_MPDU_ENCRYPT_WEP104:
-		return 4;
+		return IEEE80211_WEP_IV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC:
-	case HTT_RX_MPDU_ENCRYPT_WEP128: /* not tested */
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WPA:
-	case HTT_RX_MPDU_ENCRYPT_WAPI: /* not tested */
+		return IEEE80211_TKIP_IV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2:
-		return 8;
-	case HTT_RX_MPDU_ENCRYPT_NONE:
-		return 0;
+		return IEEE80211_CCMP_HDR_LEN;
+	case HTT_RX_MPDU_ENCRYPT_WEP128:
+	case HTT_RX_MPDU_ENCRYPT_WAPI:
+		break;
 	}
 
-	ath10k_warn(ar, "unknown encryption type %d\n", type);
+	ath10k_warn(ar, "unsupported encryption type %d\n", type);
 	return 0;
 }
 
+#define MICHAEL_MIC_LEN 8
+
 static int ath10k_htt_rx_crypto_tail_len(struct ath10k *ar,
 					 enum htt_rx_mpdu_encrypt_type type)
 {
 	switch (type) {
 	case HTT_RX_MPDU_ENCRYPT_NONE:
+		return 0;
 	case HTT_RX_MPDU_ENCRYPT_WEP40:
 	case HTT_RX_MPDU_ENCRYPT_WEP104:
-	case HTT_RX_MPDU_ENCRYPT_WEP128:
-	case HTT_RX_MPDU_ENCRYPT_WAPI:
-		return 0;
+		return IEEE80211_WEP_ICV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC:
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WPA:
-		return 4;
+		return IEEE80211_TKIP_ICV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2:
-		return 8;
+		return IEEE80211_CCMP_MIC_LEN;
+	case HTT_RX_MPDU_ENCRYPT_WEP128:
+	case HTT_RX_MPDU_ENCRYPT_WAPI:
+		break;
 	}
 
-	ath10k_warn(ar, "unknown encryption type %d\n", type);
+	ath10k_warn(ar, "unsupported encryption type %d\n", type);
 	return 0;
 }
 
@@ -1427,7 +1433,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 	/* last fragment of TKIP frags has MIC */
 	if (!ieee80211_has_morefrags(hdr->frame_control) &&
 	    enctype == HTT_RX_MPDU_ENCRYPT_TKIP_WPA)
-		trim += 8;
+		trim += MICHAEL_MIC_LEN;
 
 	if (trim > msdu_head->len) {
 		ath10k_warn(ar, "htt rx fragment: trailer longer than the frame itself? drop\n");
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 3/9] ath10k: use ieee80211 defines for crypto param lengths

[Michal Kazior]

Use the globally defined ieee80211 values instead
of re-defining them in the driver again.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 34 +++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 61ff2ac..ca466ce 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -588,41 +588,47 @@ static int ath10k_htt_rx_crypto_param_len(struct ath10k *ar,
 					  enum htt_rx_mpdu_encrypt_type type)
 {
 	switch (type) {
+	case HTT_RX_MPDU_ENCRYPT_NONE:
+		return 0;
 	case HTT_RX_MPDU_ENCRYPT_WEP40:
 	case HTT_RX_MPDU_ENCRYPT_WEP104:
-		return 4;
+		return IEEE80211_WEP_IV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC:
-	case HTT_RX_MPDU_ENCRYPT_WEP128: /* not tested */
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WPA:
-	case HTT_RX_MPDU_ENCRYPT_WAPI: /* not tested */
+		return IEEE80211_TKIP_IV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2:
-		return 8;
-	case HTT_RX_MPDU_ENCRYPT_NONE:
-		return 0;
+		return IEEE80211_CCMP_HDR_LEN;
+	case HTT_RX_MPDU_ENCRYPT_WEP128:
+	case HTT_RX_MPDU_ENCRYPT_WAPI:
+		break;
 	}
 
-	ath10k_warn(ar, "unknown encryption type %d\n", type);
+	ath10k_warn(ar, "unsupported encryption type %d\n", type);
 	return 0;
 }
 
+#define MICHAEL_MIC_LEN 8
+
 static int ath10k_htt_rx_crypto_tail_len(struct ath10k *ar,
 					 enum htt_rx_mpdu_encrypt_type type)
 {
 	switch (type) {
 	case HTT_RX_MPDU_ENCRYPT_NONE:
+		return 0;
 	case HTT_RX_MPDU_ENCRYPT_WEP40:
 	case HTT_RX_MPDU_ENCRYPT_WEP104:
-	case HTT_RX_MPDU_ENCRYPT_WEP128:
-	case HTT_RX_MPDU_ENCRYPT_WAPI:
-		return 0;
+		return IEEE80211_WEP_ICV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WITHOUT_MIC:
 	case HTT_RX_MPDU_ENCRYPT_TKIP_WPA:
-		return 4;
+		return IEEE80211_TKIP_ICV_LEN;
 	case HTT_RX_MPDU_ENCRYPT_AES_CCM_WPA2:
-		return 8;
+		return IEEE80211_CCMP_MIC_LEN;
+	case HTT_RX_MPDU_ENCRYPT_WEP128:
+	case HTT_RX_MPDU_ENCRYPT_WAPI:
+		break;
 	}
 
-	ath10k_warn(ar, "unknown encryption type %d\n", type);
+	ath10k_warn(ar, "unsupported encryption type %d\n", type);
 	return 0;
 }
 
@@ -1427,7 +1433,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 	/* last fragment of TKIP frags has MIC */
 	if (!ieee80211_has_morefrags(hdr->frame_control) &&
 	    enctype == HTT_RX_MPDU_ENCRYPT_TKIP_WPA)
-		trim += 8;
+		trim += MICHAEL_MIC_LEN;
 
 	if (trim > msdu_head->len) {
 		ath10k_warn(ar, "htt rx fragment: trailer longer than the frame itself? drop\n");
-- 
1.8.5.3

[PATCH 4/9] ath10k: fix rx buffer tracing

[Michal Kazior]

Tracing function was called before buffers were
unmapped from DMA.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ca466ce..a7d29c8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -291,9 +291,6 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
 	htt->rx_ring.sw_rd_idx.msdu_payld = idx;
 	htt->rx_ring.fill_cnt--;
 
-	trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-				     skb_tailroom(msdu));
-
 	return msdu;
 }
 
@@ -339,6 +336,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 
 		ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx pop: ",
 				msdu->data, msdu->len + skb_tailroom(msdu));
+		trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+					     skb_tailroom(msdu));
 
 		rx_desc = (struct htt_rx_desc *)msdu->data;
 
@@ -438,6 +437,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 			ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL,
 					"htt rx chained: ", next->data,
 					next->len + skb_tailroom(next));
+			trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+						     skb_tailroom(msdu));
 
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 4/9] ath10k: fix rx buffer tracing

[Michal Kazior]

Tracing function was called before buffers were
unmapped from DMA.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ca466ce..a7d29c8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -291,9 +291,6 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
 	htt->rx_ring.sw_rd_idx.msdu_payld = idx;
 	htt->rx_ring.fill_cnt--;
 
-	trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-				     skb_tailroom(msdu));
-
 	return msdu;
 }
 
@@ -339,6 +336,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 
 		ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx pop: ",
 				msdu->data, msdu->len + skb_tailroom(msdu));
+		trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+					     skb_tailroom(msdu));
 
 		rx_desc = (struct htt_rx_desc *)msdu->data;
 
@@ -438,6 +437,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 			ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL,
 					"htt rx chained: ", next->data,
 					next->len + skb_tailroom(next));
+			trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+						     skb_tailroom(msdu));
 
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
-- 
1.8.5.3

[PATCH 5/9] ath10k: deduplicate htt rx dma unmapping

[Michal Kazior]

Treat non-chained and chained popping the same
way. Also this makes netbuf pop fully symmetrical
to (re)filling.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a7d29c8..41a2803 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -291,6 +291,15 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
 	htt->rx_ring.sw_rd_idx.msdu_payld = idx;
 	htt->rx_ring.fill_cnt--;
 
+	dma_unmap_single(htt->ar->dev,
+			 ATH10K_SKB_CB(msdu)->paddr,
+			 msdu->len + skb_tailroom(msdu),
+			 DMA_FROM_DEVICE);
+	ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx netbuf pop: ",
+			msdu->data, msdu->len + skb_tailroom(msdu));
+	trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+				     skb_tailroom(msdu));
+
 	return msdu;
 }
 
@@ -329,16 +338,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 	while (msdu) {
 		int last_msdu, msdu_len_invalid, msdu_chained;
 
-		dma_unmap_single(htt->ar->dev,
-				 ATH10K_SKB_CB(msdu)->paddr,
-				 msdu->len + skb_tailroom(msdu),
-				 DMA_FROM_DEVICE);
-
-		ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx pop: ",
-				msdu->data, msdu->len + skb_tailroom(msdu));
-		trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-					     skb_tailroom(msdu));
-
 		rx_desc = (struct htt_rx_desc *)msdu->data;
 
 		/* FIXME: we must report msdu payload since this is what caller
@@ -429,17 +428,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		while (msdu_chained--) {
 			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
-			dma_unmap_single(htt->ar->dev,
-					 ATH10K_SKB_CB(next)->paddr,
-					 next->len + skb_tailroom(next),
-					 DMA_FROM_DEVICE);
-
-			ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL,
-					"htt rx chained: ", next->data,
-					next->len + skb_tailroom(next));
-			trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-						     skb_tailroom(msdu));
-
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
 			msdu_len -= next->len;
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 5/9] ath10k: deduplicate htt rx dma unmapping

[Michal Kazior]

Treat non-chained and chained popping the same
way. Also this makes netbuf pop fully symmetrical
to (re)filling.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a7d29c8..41a2803 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -291,6 +291,15 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
 	htt->rx_ring.sw_rd_idx.msdu_payld = idx;
 	htt->rx_ring.fill_cnt--;
 
+	dma_unmap_single(htt->ar->dev,
+			 ATH10K_SKB_CB(msdu)->paddr,
+			 msdu->len + skb_tailroom(msdu),
+			 DMA_FROM_DEVICE);
+	ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx netbuf pop: ",
+			msdu->data, msdu->len + skb_tailroom(msdu));
+	trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
+				     skb_tailroom(msdu));
+
 	return msdu;
 }
 
@@ -329,16 +338,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 	while (msdu) {
 		int last_msdu, msdu_len_invalid, msdu_chained;
 
-		dma_unmap_single(htt->ar->dev,
-				 ATH10K_SKB_CB(msdu)->paddr,
-				 msdu->len + skb_tailroom(msdu),
-				 DMA_FROM_DEVICE);
-
-		ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx pop: ",
-				msdu->data, msdu->len + skb_tailroom(msdu));
-		trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-					     skb_tailroom(msdu));
-
 		rx_desc = (struct htt_rx_desc *)msdu->data;
 
 		/* FIXME: we must report msdu payload since this is what caller
@@ -429,17 +428,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		while (msdu_chained--) {
 			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
-			dma_unmap_single(htt->ar->dev,
-					 ATH10K_SKB_CB(next)->paddr,
-					 next->len + skb_tailroom(next),
-					 DMA_FROM_DEVICE);
-
-			ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL,
-					"htt rx chained: ", next->data,
-					next->len + skb_tailroom(next));
-			trace_ath10k_htt_rx_pop_msdu(ar, msdu->data, msdu->len +
-						     skb_tailroom(msdu));
-
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
 			msdu_len -= next->len;
-- 
1.8.5.3

[PATCH 6/9] ath10k: don't drop frames aggressively

[Michal Kazior]

There's little point in dropping, e.g. frames with
FCS error early in ath10k.

This simplifies amsdu_allowed() and gets rid of
htt_rx_mpdu_status usage finally.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 41a2803..7fa4d87 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1171,7 +1171,6 @@ static int ath10k_unchain_msdu(struct sk_buff *msdu_head)
 
 static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 					struct sk_buff *head,
-					enum htt_rx_mpdu_status status,
 					bool channel_set,
 					u32 attention)
 {
@@ -1200,16 +1199,6 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 		return false;
 	}
 
-	if (status != HTT_RX_IND_MPDU_STATUS_OK &&
-	    status != HTT_RX_IND_MPDU_STATUS_TKIP_MIC_ERR &&
-	    status != HTT_RX_IND_MPDU_STATUS_ERR_INV_PEER &&
-	    !htt->ar->monitor_started) {
-		ath10k_dbg(ar, ATH10K_DBG_HTT,
-			   "htt rx ignoring frame w/ status %d\n",
-			   status);
-		return false;
-	}
-
 	if (test_bit(ATH10K_CAC_RUNNING, &htt->ar->dev_flags)) {
 		ath10k_dbg(ar, ATH10K_DBG_HTT,
 			   "htt rx CAC running\n");
@@ -1225,7 +1214,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	struct ath10k *ar = htt->ar;
 	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
-	enum htt_rx_mpdu_status status;
 	struct ieee80211_hdr *hdr;
 	int num_mpdu_ranges;
 	u32 attention;
@@ -1273,8 +1261,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				num_mpdu_ranges));
 
 	for (i = 0; i < num_mpdu_ranges; i++) {
-		status = mpdu_ranges[i].mpdu_range_status;
-
 		for (j = 0; j < mpdu_ranges[i].mpdu_count; j++) {
 			struct sk_buff *msdu_head, *msdu_tail;
 
@@ -1296,7 +1282,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			}
 
 			if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
-							 status,
 							 channel_set,
 							 attention)) {
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 6/9] ath10k: don't drop frames aggressively

[Michal Kazior]

There's little point in dropping, e.g. frames with
FCS error early in ath10k.

This simplifies amsdu_allowed() and gets rid of
htt_rx_mpdu_status usage finally.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 41a2803..7fa4d87 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1171,7 +1171,6 @@ static int ath10k_unchain_msdu(struct sk_buff *msdu_head)
 
 static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 					struct sk_buff *head,
-					enum htt_rx_mpdu_status status,
 					bool channel_set,
 					u32 attention)
 {
@@ -1200,16 +1199,6 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
 		return false;
 	}
 
-	if (status != HTT_RX_IND_MPDU_STATUS_OK &&
-	    status != HTT_RX_IND_MPDU_STATUS_TKIP_MIC_ERR &&
-	    status != HTT_RX_IND_MPDU_STATUS_ERR_INV_PEER &&
-	    !htt->ar->monitor_started) {
-		ath10k_dbg(ar, ATH10K_DBG_HTT,
-			   "htt rx ignoring frame w/ status %d\n",
-			   status);
-		return false;
-	}
-
 	if (test_bit(ATH10K_CAC_RUNNING, &htt->ar->dev_flags)) {
 		ath10k_dbg(ar, ATH10K_DBG_HTT,
 			   "htt rx CAC running\n");
@@ -1225,7 +1214,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	struct ath10k *ar = htt->ar;
 	struct ieee80211_rx_status *rx_status = &htt->rx_status;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
-	enum htt_rx_mpdu_status status;
 	struct ieee80211_hdr *hdr;
 	int num_mpdu_ranges;
 	u32 attention;
@@ -1273,8 +1261,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				num_mpdu_ranges));
 
 	for (i = 0; i < num_mpdu_ranges; i++) {
-		status = mpdu_ranges[i].mpdu_range_status;
-
 		for (j = 0; j < mpdu_ranges[i].mpdu_count; j++) {
 			struct sk_buff *msdu_head, *msdu_tail;
 
@@ -1296,7 +1282,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			}
 
 			if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
-							 status,
 							 channel_set,
 							 attention)) {
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
-- 
1.8.5.3

[PATCH 7/9] ath10k: add extra sanity check when popping amsdu

[Michal Kazior]

The netbuf pop can return NULL. Make sure to check
for that. It shouldn't happen but better safe than
sorry.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7fa4d87..ddb9fe9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -428,6 +428,15 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		while (msdu_chained--) {
 			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
+			if (!next) {
+				ath10k_err(ar, "failed to pop chained msdu\n");
+				ath10k_htt_rx_free_msdu_chain(*head_msdu);
+				*head_msdu = NULL;
+				msdu = NULL;
+				htt->rx_confused = true;
+				break;
+			}
+
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
 			msdu_len -= next->len;
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 7/9] ath10k: add extra sanity check when popping amsdu

[Michal Kazior]

The netbuf pop can return NULL. Make sure to check
for that. It shouldn't happen but better safe than
sorry.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7fa4d87..ddb9fe9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -428,6 +428,15 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		while (msdu_chained--) {
 			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
+			if (!next) {
+				ath10k_err(ar, "failed to pop chained msdu\n");
+				ath10k_htt_rx_free_msdu_chain(*head_msdu);
+				*head_msdu = NULL;
+				msdu = NULL;
+				htt->rx_confused = true;
+				break;
+			}
+
 			skb_trim(next, 0);
 			skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
 			msdu_len -= next->len;
-- 
1.8.5.3

Re: [PATCH 7/9] ath10k: add extra sanity check when popping amsdu

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> The netbuf pop can return NULL. Make sure to check
> for that. It shouldn't happen but better safe than
> sorry.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 7fa4d87..ddb9fe9 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -428,6 +428,15 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>  		while (msdu_chained--) {
>  			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
>  
> +			if (!next) {
> +				ath10k_err(ar, "failed to pop chained msdu\n");

ath10k_warn() is better here, right? I have folded this diff to the patch:

--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -429,7 +429,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
                        struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
                        if (!next) {
-                               ath10k_err(ar, "failed to pop chained msdu\n");
+                               ath10k_warn(ar, "failed to pop chained msdu\n");
                                ath10k_htt_rx_free_msdu_chain(*head_msdu);
                                *head_msdu = NULL;
                                msdu = NULL;

Full commit:

https://github.com/kvalo/ath/commit/03f316720d664a79e874c13df1e86a64c5f2bf5f

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 7/9] ath10k: add extra sanity check when popping amsdu

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> The netbuf pop can return NULL. Make sure to check
> for that. It shouldn't happen but better safe than
> sorry.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 7fa4d87..ddb9fe9 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -428,6 +428,15 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>  		while (msdu_chained--) {
>  			struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
>  
> +			if (!next) {
> +				ath10k_err(ar, "failed to pop chained msdu\n");

ath10k_warn() is better here, right? I have folded this diff to the patch:

--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -429,7 +429,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
                        struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
 
                        if (!next) {
-                               ath10k_err(ar, "failed to pop chained msdu\n");
+                               ath10k_warn(ar, "failed to pop chained msdu\n");
                                ath10k_htt_rx_free_msdu_chain(*head_msdu);
                                *head_msdu = NULL;
                                msdu = NULL;

Full commit:

https://github.com/kvalo/ath/commit/03f316720d664a79e874c13df1e86a64c5f2bf5f

-- 
Kalle Valo

[PATCH 8/9] ath10k: don't forget to replenish after fragmented Rx

[Michal Kazior]

In theory it was possible to drain entire HTT Rx
ring via fragmented Rx leading to Rx lockup.

In practice non-data traffic would always trigger
replenishment via the regular Rx handler.

For correctness sake make sure to replenish the
ring on fragmented Rx.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ddb9fe9..39e1969 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1355,6 +1355,8 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 				      &attention);
 	spin_unlock_bh(&htt->rx_ring.lock);
 
+	tasklet_schedule(&htt->rx_replenish_task);
+
 	ath10k_dbg(ar, ATH10K_DBG_HTT_DUMP, "htt rx frag ahead\n");
 
 	if (ret) {
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 8/9] ath10k: don't forget to replenish after fragmented Rx

[Michal Kazior]

In theory it was possible to drain entire HTT Rx
ring via fragmented Rx leading to Rx lockup.

In practice non-data traffic would always trigger
replenishment via the regular Rx handler.

For correctness sake make sure to replenish the
ring on fragmented Rx.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ddb9fe9..39e1969 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1355,6 +1355,8 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
 				      &attention);
 	spin_unlock_bh(&htt->rx_ring.lock);
 
+	tasklet_schedule(&htt->rx_replenish_task);
+
 	ath10k_dbg(ar, ATH10K_DBG_HTT_DUMP, "htt rx frag ahead\n");
 
 	if (ret) {
-- 
1.8.5.3

[PATCH 9/9] ath10k: clear htt->rx_confused on load

[Michal Kazior]

Once driver entered the rx_confused state it would
refuse to rx even after firmware is restarted.
Make sure to clear it so that rx works after, e.g.
hw restart or after all interfaces are stopped.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 39e1969..3e52a33 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -497,6 +497,8 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
 	size_t size;
 	struct timer_list *timer = &htt->rx_ring.refill_retry_timer;
 
+	htt->rx_confused = false;
+
 	htt->rx_ring.size = ath10k_htt_rx_ring_size(htt);
 	if (!is_power_of_2(htt->rx_ring.size)) {
 		ath10k_warn(ar, "htt rx ring size is not power of 2\n");
-- 
1.8.5.3


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH 9/9] ath10k: clear htt->rx_confused on load

[Michal Kazior]

Once driver entered the rx_confused state it would
refuse to rx even after firmware is restarted.
Make sure to clear it so that rx works after, e.g.
hw restart or after all interfaces are stopped.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 39e1969..3e52a33 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -497,6 +497,8 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
 	size_t size;
 	struct timer_list *timer = &htt->rx_ring.refill_retry_timer;
 
+	htt->rx_confused = false;
+
 	htt->rx_ring.size = ath10k_htt_rx_ring_size(htt);
 	if (!is_power_of_2(htt->rx_ring.size)) {
 		ath10k_warn(ar, "htt rx ring size is not power of 2\n");
-- 
1.8.5.3

Re: [PATCH 0/9] ath10k: htt rx fixes and clean ups

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> I've found a couple of issues while reworking rx
> path.
>
>
> Michal Kazior (9):
>   ath10k: don't drop control and null func Rx
>   ath10k: remove unused variable
>   ath10k: use ieee80211 defines for crypto param lengths
>   ath10k: fix rx buffer tracing
>   ath10k: deduplicate htt rx dma unmapping
>   ath10k: don't drop frames aggressively
>   ath10k: add extra sanity check when popping amsdu
>   ath10k: don't forget to replenish after fragmented Rx
>   ath10k: clear htt->rx_confused on load

Thanks, all nine applied.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH 0/9] ath10k: htt rx fixes and clean ups

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> I've found a couple of issues while reworking rx
> path.
>
>
> Michal Kazior (9):
>   ath10k: don't drop control and null func Rx
>   ath10k: remove unused variable
>   ath10k: use ieee80211 defines for crypto param lengths
>   ath10k: fix rx buffer tracing
>   ath10k: deduplicate htt rx dma unmapping
>   ath10k: don't drop frames aggressively
>   ath10k: add extra sanity check when popping amsdu
>   ath10k: don't forget to replenish after fragmented Rx
>   ath10k: clear htt->rx_confused on load

Thanks, all nine applied.

-- 
Kalle Valo