Skip to content

Commit

Permalink
mac80211: use the new drop reasons infrastructure
Browse files Browse the repository at this point in the history
It can be really hard to analyse or debug why packets are
going missing in mac80211, so add the needed infrastructure
to use use the new per-subsystem drop reasons.

We actually use two drop reason subsystems here because of
the different handling of frames that are dropped but still
go to monitor for old versions of hostapd, and those that
are just completely unusable (e.g. crypto failed.)

Annotate a few reasons here just to illustrate this, we'll
need to go through and annotate more of them later.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Johannes Berg authored and Jakub Kicinski committed Apr 21, 2023
1 parent 071c0fc commit baa951a
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 48 deletions.
12 changes: 12 additions & 0 deletions include/net/dropreason.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ enum skb_drop_reason_subsys {
/** @SKB_DROP_REASON_SUBSYS_CORE: core drop reasons defined above */
SKB_DROP_REASON_SUBSYS_CORE,

/**
* @SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE: mac80211 drop reasons
* for unusable frames, see net/mac80211/drop.h
*/
SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE,

/**
* @SKB_DROP_REASON_SUBSYS_MAC80211_MONITOR: mac80211 drop reasons
* for frames still going to monitor, see net/mac80211/drop.h
*/
SKB_DROP_REASON_SUBSYS_MAC80211_MONITOR,

/** @SKB_DROP_REASON_SUBSYS_NUM: number of subsystems defined */
SKB_DROP_REASON_SUBSYS_NUM
};
Expand Down
56 changes: 56 additions & 0 deletions net/mac80211/drop.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
* mac80211 drop reason list
*
* Copyright (C) 2023 Intel Corporation
*/

#ifndef MAC80211_DROP_H
#define MAC80211_DROP_H
#include <net/dropreason.h>

typedef unsigned int __bitwise ieee80211_rx_result;

#define MAC80211_DROP_REASONS_MONITOR(R) \
R(RX_DROP_M_UNEXPECTED_4ADDR_FRAME) \
R(RX_DROP_M_BAD_BCN_KEYIDX) \
R(RX_DROP_M_BAD_MGMT_KEYIDX) \
/* this line for the trailing \ - add before this */

#define MAC80211_DROP_REASONS_UNUSABLE(R) \
R(RX_DROP_U_MIC_FAIL) \
R(RX_DROP_U_REPLAY) \
R(RX_DROP_U_BAD_MMIE) \
/* this line for the trailing \ - add before this */

/* having two enums allows for checking ieee80211_rx_result use with sparse */
enum ___mac80211_drop_reason {
/* if we get to the end of handlers with RX_CONTINUE this will be the reason */
___RX_CONTINUE = SKB_CONSUMED,

/* this never gets used as an argument to kfree_skb_reason() */
___RX_QUEUED = SKB_NOT_DROPPED_YET,

#define ENUM(x) ___ ## x,
___RX_DROP_MONITOR = SKB_DROP_REASON_SUBSYS_MAC80211_MONITOR <<
SKB_DROP_REASON_SUBSYS_SHIFT,
MAC80211_DROP_REASONS_MONITOR(ENUM)

___RX_DROP_UNUSABLE = SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE <<
SKB_DROP_REASON_SUBSYS_SHIFT,
MAC80211_DROP_REASONS_UNUSABLE(ENUM)
#undef ENUM
};

enum mac80211_drop_reason {
RX_CONTINUE = (__force ieee80211_rx_result)___RX_CONTINUE,
RX_QUEUED = (__force ieee80211_rx_result)___RX_QUEUED,
RX_DROP_MONITOR = (__force ieee80211_rx_result)___RX_DROP_MONITOR,
RX_DROP_UNUSABLE = (__force ieee80211_rx_result)___RX_DROP_UNUSABLE,
#define DEF(x) x = (__force ieee80211_rx_result)___ ## x,
MAC80211_DROP_REASONS_MONITOR(DEF)
MAC80211_DROP_REASONS_UNUSABLE(DEF)
#undef DEF
};

#endif /* MAC80211_DROP_H */
8 changes: 1 addition & 7 deletions net/mac80211/ieee80211_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "key.h"
#include "sta_info.h"
#include "debug.h"
#include "drop.h"

extern const struct cfg80211_ops mac80211_config_ops;

Expand Down Expand Up @@ -170,13 +171,6 @@ struct ieee80211_tx_data {
unsigned int flags;
};


typedef unsigned __bitwise ieee80211_rx_result;
#define RX_CONTINUE ((__force ieee80211_rx_result) 0u)
#define RX_DROP_UNUSABLE ((__force ieee80211_rx_result) 1u)
#define RX_DROP_MONITOR ((__force ieee80211_rx_result) 2u)
#define RX_QUEUED ((__force ieee80211_rx_result) 3u)

/**
* enum ieee80211_packet_rx_flags - packet RX flags
* @IEEE80211_RX_AMSDU: a-MSDU packet
Expand Down
31 changes: 31 additions & 0 deletions net/mac80211/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <linux/bitmap.h>
#include <linux/inetdevice.h>
#include <net/net_namespace.h>
#include <net/dropreason.h>
#include <net/cfg80211.h>
#include <net/addrconf.h>

Expand Down Expand Up @@ -1542,6 +1543,28 @@ void ieee80211_free_hw(struct ieee80211_hw *hw)
}
EXPORT_SYMBOL(ieee80211_free_hw);

static const char * const drop_reasons_monitor[] = {
#define V(x) #x,
[0] = "RX_DROP_MONITOR",
MAC80211_DROP_REASONS_MONITOR(V)
};

static struct drop_reason_list drop_reason_list_monitor = {
.reasons = drop_reasons_monitor,
.n_reasons = ARRAY_SIZE(drop_reasons_monitor),
};

static const char * const drop_reasons_unusable[] = {
[0] = "RX_DROP_UNUSABLE",
MAC80211_DROP_REASONS_UNUSABLE(V)
#undef V
};

static struct drop_reason_list drop_reason_list_unusable = {
.reasons = drop_reasons_unusable,
.n_reasons = ARRAY_SIZE(drop_reasons_unusable),
};

static int __init ieee80211_init(void)
{
struct sk_buff *skb;
Expand All @@ -1559,6 +1582,11 @@ static int __init ieee80211_init(void)
if (ret)
goto err_netdev;

drop_reasons_register_subsys(SKB_DROP_REASON_SUBSYS_MAC80211_MONITOR,
&drop_reason_list_monitor);
drop_reasons_register_subsys(SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE,
&drop_reason_list_unusable);

return 0;
err_netdev:
rc80211_minstrel_exit();
Expand All @@ -1574,6 +1602,9 @@ static void __exit ieee80211_exit(void)

ieee80211_iface_exit();

drop_reasons_unregister_subsys(SKB_DROP_REASON_SUBSYS_MAC80211_MONITOR);
drop_reasons_unregister_subsys(SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE);

rcu_barrier();
}

Expand Down
55 changes: 26 additions & 29 deletions net/mac80211/rx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
cfg80211_rx_unexpected_4addr_frame(
rx->sdata->dev, sta->sta.addr,
GFP_ATOMIC);
return RX_DROP_MONITOR;
return RX_DROP_M_UNEXPECTED_4ADDR_FRAME;
}
/*
* Update counter and free packet here to avoid
Expand Down Expand Up @@ -1961,7 +1961,7 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
cfg80211_rx_unprot_mlme_mgmt(rx->sdata->dev,
skb->data,
skb->len);
return RX_DROP_MONITOR; /* unexpected BIP keyidx */
return RX_DROP_M_BAD_BCN_KEYIDX;
}

rx->key = ieee80211_rx_get_bigtk(rx, mmie_keyidx);
Expand All @@ -1975,7 +1975,7 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)

if (mmie_keyidx < NUM_DEFAULT_KEYS ||
mmie_keyidx >= NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS)
return RX_DROP_MONITOR; /* unexpected BIP keyidx */
return RX_DROP_M_BAD_MGMT_KEYIDX; /* unexpected BIP keyidx */
if (rx->link_sta) {
if (ieee80211_is_group_privacy_action(skb) &&
test_sta_flag(rx->sta, WLAN_STA_MFP))
Expand Down Expand Up @@ -3960,7 +3960,8 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_data *rx)
}

static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
struct ieee80211_rate *rate)
struct ieee80211_rate *rate,
ieee80211_rx_result reason)
{
struct ieee80211_sub_if_data *sdata;
struct ieee80211_local *local = rx->local;
Expand Down Expand Up @@ -4024,42 +4025,38 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
}

out_free_skb:
dev_kfree_skb(skb);
kfree_skb_reason(skb, (__force u32)reason);
}

static void ieee80211_rx_handlers_result(struct ieee80211_rx_data *rx,
ieee80211_rx_result res)
{
switch (res) {
case RX_DROP_MONITOR:
I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
if (rx->sta)
rx->link_sta->rx_stats.dropped++;
fallthrough;
case RX_CONTINUE: {
struct ieee80211_rate *rate = NULL;
struct ieee80211_supported_band *sband;
struct ieee80211_rx_status *status;

status = IEEE80211_SKB_RXCB((rx->skb));
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
struct ieee80211_supported_band *sband;
struct ieee80211_rate *rate = NULL;

sband = rx->local->hw.wiphy->bands[status->band];
if (status->encoding == RX_ENC_LEGACY)
rate = &sband->bitrates[status->rate_idx];
if (res == RX_QUEUED) {
I802_DEBUG_INC(rx->sdata->local->rx_handlers_queued);
return;
}

ieee80211_rx_cooked_monitor(rx, rate);
break;
}
case RX_DROP_UNUSABLE:
if (res != RX_CONTINUE) {
I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
if (rx->sta)
rx->link_sta->rx_stats.dropped++;
dev_kfree_skb(rx->skb);
break;
case RX_QUEUED:
I802_DEBUG_INC(rx->sdata->local->rx_handlers_queued);
break;
}

if (u32_get_bits((__force u32)res, SKB_DROP_REASON_SUBSYS_MASK) ==
SKB_DROP_REASON_SUBSYS_MAC80211_UNUSABLE) {
kfree_skb_reason(rx->skb, (__force u32)res);
return;
}

sband = rx->local->hw.wiphy->bands[status->band];
if (status->encoding == RX_ENC_LEGACY)
rate = &sband->bitrates[status->rate_idx];

ieee80211_rx_cooked_monitor(rx, rate, res);
}

static void ieee80211_rx_handlers(struct ieee80211_rx_data *rx,
Expand Down
24 changes: 12 additions & 12 deletions net/mac80211/wpa.c
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
if (res < 0 ||
(!res && !(status->flag & RX_FLAG_ALLOW_SAME_PN))) {
key->u.ccmp.replays++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_REPLAY;
}

if (!(status->flag & RX_FLAG_DECRYPTED)) {
Expand All @@ -564,7 +564,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
data_len,
skb->data + skb->len - mic_len))
return RX_DROP_UNUSABLE;
return RX_DROP_U_MIC_FAIL;
}

memcpy(key->u.ccmp.rx_pn[queue], pn, IEEE80211_CCMP_PN_LEN);
Expand Down Expand Up @@ -746,7 +746,7 @@ ieee80211_crypto_gcmp_decrypt(struct ieee80211_rx_data *rx)
if (res < 0 ||
(!res && !(status->flag & RX_FLAG_ALLOW_SAME_PN))) {
key->u.gcmp.replays++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_REPLAY;
}

if (!(status->flag & RX_FLAG_DECRYPTED)) {
Expand All @@ -761,7 +761,7 @@ ieee80211_crypto_gcmp_decrypt(struct ieee80211_rx_data *rx)
data_len,
skb->data + skb->len -
IEEE80211_GCMP_MIC_LEN))
return RX_DROP_UNUSABLE;
return RX_DROP_U_MIC_FAIL;
}

memcpy(key->u.gcmp.rx_pn[queue], pn, IEEE80211_GCMP_PN_LEN);
Expand Down Expand Up @@ -930,13 +930,13 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
(skb->data + skb->len - sizeof(*mmie));
if (mmie->element_id != WLAN_EID_MMIE ||
mmie->length != sizeof(*mmie) - 2)
return RX_DROP_UNUSABLE; /* Invalid MMIE */
return RX_DROP_U_BAD_MMIE; /* Invalid MMIE */

bip_ipn_swap(ipn, mmie->sequence_number);

if (memcmp(ipn, key->u.aes_cmac.rx_pn, 6) <= 0) {
key->u.aes_cmac.replays++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_REPLAY;
}

if (!(status->flag & RX_FLAG_DECRYPTED)) {
Expand All @@ -946,7 +946,7 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
skb->data + 24, skb->len - 24, mic);
if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_MIC_FAIL;
}
}

Expand Down Expand Up @@ -986,7 +986,7 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)

if (memcmp(ipn, key->u.aes_cmac.rx_pn, 6) <= 0) {
key->u.aes_cmac.replays++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_REPLAY;
}

if (!(status->flag & RX_FLAG_DECRYPTED)) {
Expand All @@ -996,7 +996,7 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
skb->data + 24, skb->len - 24, mic);
if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_MIC_FAIL;
}
}

Expand Down Expand Up @@ -1079,13 +1079,13 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
(skb->data + skb->len - sizeof(*mmie));
if (mmie->element_id != WLAN_EID_MMIE ||
mmie->length != sizeof(*mmie) - 2)
return RX_DROP_UNUSABLE; /* Invalid MMIE */
return RX_DROP_U_BAD_MMIE; /* Invalid MMIE */

bip_ipn_swap(ipn, mmie->sequence_number);

if (memcmp(ipn, key->u.aes_gmac.rx_pn, 6) <= 0) {
key->u.aes_gmac.replays++;
return RX_DROP_UNUSABLE;
return RX_DROP_U_REPLAY;
}

if (!(status->flag & RX_FLAG_DECRYPTED)) {
Expand All @@ -1104,7 +1104,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_gmac.icverrors++;
kfree(mic);
return RX_DROP_UNUSABLE;
return RX_DROP_U_MIC_FAIL;
}
kfree(mic);
}
Expand Down

0 comments on commit baa951a

Please sign in to comment.