[RFC 05/14] ath10k: introduce ieee80211_rx_status to htt_rx_info

Kalle Valo kvalo at qca.qualcomm.com
Tue Mar 11 05:59:55 EDT 2014


Janusz Dziedzic <janusz.dziedzic at tieto.com> writes:

> Will be used as a template, and final storage for
> rx_status.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic at tieto.com>

I think this patch is too small. It's good to have small atomic commits
but I think goes overboard.

> @@ -1174,6 +1175,7 @@ struct htt_peer_unmap_event {
>  
>  struct htt_rx_info {
>  	struct sk_buff *skb;
> +	struct ieee80211_rx_status rx_status;
>  	enum htt_rx_mpdu_status status;
>  	enum htt_rx_mpdu_encrypt_type encrypt_type;
>  	s8 signal;
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 3008a1d..2fca1fa 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -756,7 +756,7 @@ static void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
>  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)info->skb->data;
>  
>  	status = IEEE80211_SKB_RXCB(info->skb);
> -	memset(status, 0, sizeof(*status));
> +	memcpy(status, &info->rx_status, sizeof(*status));

So you are idea with the status template is that instead clearing
status, you copy it from the previous frame and reuse it? Why do you
want to do it? Performance reasons?

I didn't fully understand your idea here, but I assume patch 9 is
related as I see that you change the CRC flag handling from this:

-	if (info->fcs_err)
-		status->flag |= RX_FLAG_FAILED_FCS_CRC;

to this:

+			fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
+			if (fcs_err)
+				info.rx_status.flag |= RX_FLAG_FAILED_FCS_CRC;
+			else
+				info.rx_status.flag &= ~RX_FLAG_FAILED_FCS_CRC;

I don't really like that clearing the flag part. Can you describe what
is your plan behind this template?

-- 
Kalle Valo



More information about the ath10k mailing list