> There is a field, no_80211_encap, added to ieee80211_tx_info:control
> to mark if the 802.11 encapsulation is offloaded to driver.
> There is also a new callback for tx completion status indication
> to handle data frames using 802.11 encap offload.
I'm not sure I see the need for this? Maybe I'll find out in this patch
:)
> + /* XXX: This frame is not encaptulated with
> 802.11
> + * header. Should this be added to
> %IEEE80211_TX_CTRL_*
> + * flags?.
> + */
> + bool no_80211_encap;
> + /* 3 bytes free */
> } control;
probably - just to preserve more space.
> + * @IEEE80211_CONF_CHANGE_80211_HDR_OFFL: Offload configuration
> + * implementing 802.11 encap/decap for data frames changed.
> */
> enum ieee80211_conf_changed {
> IEEE80211_CONF_CHANGE_SMPS = BIT(1),
> @@ -1279,6 +1286,7 @@ enum ieee80211_conf_changed {
> IEEE80211_CONF_CHANGE_CHANNEL = BIT(6),
> IEEE80211_CONF_CHANGE_RETRY_LIMITS = BIT(7),
> IEEE80211_CONF_CHANGE_IDLE = BIT(8),
> + IEEE80211_CONF_CHANGE_80211_HDR_OFFL = BIT(9),
> };
Given the requirements (PN check, etc.) I'm not sure how this can
change dynamically?
> + * @encap_decap_80211_offloaded: Whether 802.11 header encap/decap
> offload
> + * is enabled
> */
> struct ieee80211_conf {
> u32 flags;
> @@ -1346,6 +1357,7 @@ struct ieee80211_conf {
> struct cfg80211_chan_def chandef;
> bool radar_enabled;
> enum ieee80211_smps_mode smps_mode;
> + bool encap_decap_80211_offloaded;
Please don't add anything here that's interface specific.
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -107,6 +107,10 @@ static int ieee80211_change_iface(struct wiphy
> *wiphy,
> }
> }
>
> + ieee80211_if_check_80211_hdr_offl(sdata,
> + params ? params->use_4addr
> : false,
> + true);
> +
> return 0;
> }
Wouldn't it be better to simply prohibit changing this while the
interface is up, and re-init it later when it goes up?
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1373,6 +1373,8 @@ struct ieee80211_local {
> /* TDLS channel switch */
> struct work_struct tdls_chsw_work;
> struct sk_buff_head skb_queue_tdls_chsw;
> +
> + bool data_80211_hdr_offloaded;
Again, don't put interface specific things into device structures.
> +int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
> + struct sk_buff *skb,
> + struct sta_info **sta_out);
Return the sta_info pointer, and ERR_PTR() if needed.
> +++ b/net/mac80211/iface.c
> @@ -698,6 +698,11 @@ int ieee80211_do_open(struct wireless_dev *wdev,
> bool coming_up)
> rcu_assign_pointer(local->p2p_sdata, sdata);
> }
>
> + if (local->open_count == 0 && local-
> >data_80211_hdr_offloaded) {
> + local->hw.conf.encap_decap_80211_offloaded = true;
> + hw_reconf_flags |=
> IEEE80211_CONF_CHANGE_80211_HDR_OFFL;
> + }
I don't see how this helps anything, I think you should remove it.
> +void ieee80211_if_config_80211_hdr_offl(struct ieee80211_local
> *local,
> + bool enable_80211_hdr_offl)
> +{
> + struct ieee80211_sub_if_data *sdata;
> + unsigned long flags;
> + int n_acs = IEEE80211_NUM_ACS;
> + int ac;
> +
> + ASSERT_RTNL();
> +
> + if (!ieee80211_hw_check(&local->hw,
> SUPPORTS_80211_ENCAP_DECAP) ||
> + !(ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)))
> + return;
> +
> + if (local->hw.wiphy->frag_threshold != (u32)-1 &&
> + !local->ops->set_frag_threshold)
> + return;
> +
> + mutex_lock(&local->iflist_mtx);
> +
> + list_for_each_entry(sdata, &local->interfaces, list) {
> + if (!sdata->dev)
> + continue;
> +
> + netif_tx_stop_all_queues(sdata->dev);
> +
> + if (enable_80211_hdr_offl)
> + sdata->dev->netdev_ops =
> &ieee80211_dataif_8023_ops;
> + else
> + sdata->dev->netdev_ops =
> &ieee80211_dataif_ops;
> + }
> +
> + mutex_unlock(&local->iflist_mtx);
> +
> + local->data_80211_hdr_offloaded = enable_80211_hdr_offl;
> +
> + if (local->started) {
> + if (enable_80211_hdr_offl)
> + local->hw.conf.encap_decap_80211_offloaded =
> true;
> + else
> + local->hw.conf.encap_decap_80211_offloaded =
> false;
> + ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_80211_HDR_
> OFFL);
> + }
> +
> + mutex_lock(&local->iflist_mtx);
> +
> + list_for_each_entry(sdata, &local->interfaces, list) {
> + if (!sdata->dev)
> + continue;
> +
> + if (local->hw.queues < IEEE80211_NUM_ACS)
> + n_acs = 1;
> +
> + spin_lock_irqsave(&local->queue_stop_reason_lock,
> flags);
> + if (sdata->vif.cab_queue == IEEE80211_INVAL_HW_QUEUE
> ||
> + (local->queue_stop_reasons[sdata->vif.cab_queue]
> == 0 &&
> + skb_queue_empty(&local->pending[sdata-
> >vif.cab_queue]))) {
> + for (ac = 0; ac < n_acs; ac++) {
> + int ac_queue = sdata-
> >vif.hw_queue[ac];
> +
> + if (local-
> >queue_stop_reasons[ac_queue] == 0 &&
> + skb_queue_empty(&local-
> >pending[ac_queue]))
> + netif_start_subqueue(sdata-
> >dev, ac);
> + }
> + }
> + spin_unlock_irqrestore(&local-
> >queue_stop_reason_lock, flags);
> + }
> +
> + mutex_unlock(&local->iflist_mtx);
> +}
I really would prefer we could simply avoid doing these manipulations
while the interface is UP and can have data queued.
> +++ b/net/mac80211/key.c
> @@ -208,13 +208,25 @@ static int ieee80211_key_enable_hw_accel(struct
> ieee80211_key *key)
> case WLAN_CIPHER_SUITE_GCMP_256:
> /* all of these we can do in software - if driver
> can */
> if (ret == 1)
> - return 0;
> + goto check_8023_txrx;
> if (ieee80211_hw_check(&key->local->hw,
> SW_CRYPTO_CONTROL))
> return -EINVAL;
> - return 0;
> + goto check_8023_txrx;
> default:
> return -EINVAL;
> }
> +
> +check_8023_txrx:
> + /* When sw crypto is enabled make sure data tx/rx happens
> + * in 802.11 format.
> + */
> + if (key->local->data_80211_hdr_offloaded) {
> + rtnl_lock();
> + ieee80211_if_config_80211_hdr_offl(key->local,
> false);
> + rtnl_unlock();
> + }
> +
> + return 0;
> }
Why not just refuse the key instead? It also seems wrong to do anything
with local-> here, it should be per interface.
> +++ b/net/mac80211/status.c
> @@ -506,12 +506,14 @@ static void ieee80211_report_used_skb(struct
> ieee80211_local *local,
> struct sk_buff *skb, bool
> dropped)
> {
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> - struct ieee80211_hdr *hdr = (void *)skb->data;
> + struct ieee80211_hdr *hdr;
> bool acked = info->flags & IEEE80211_TX_STAT_ACK;
>
> if (dropped)
> acked = false;
>
> + hdr = (void *)skb->data;
That change make no sense.
> if (info->flags & IEEE80211_TX_INTFL_MLME_CONN_TX) {
> struct ieee80211_sub_if_data *sdata;
>
> @@ -945,6 +947,85 @@ void ieee80211_tx_status(struct ieee80211_hw
> *hw, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(ieee80211_tx_status);
>
> +void ieee80211_tx_status_8023(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct sk_buff *skb)
I think this could share some code with the 802.11 version?
> + /* XXX: Add a generic helper for this */
> + if (sdata->vif.type == NL80211_IFTYPE_AP ||
> + sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
> + sdata->vif.type == NL80211_IFTYPE_ADHOC)
> + ether_addr_copy(ra_addr, ehdr->h_dest);
nit, but the "A" in "RA" already means address ... :)
You also don't need to copy it - just keeping a pointer should be fine?
> + /* TODO: Handle frames requiring wifi tx status to be
> notified */
> + if (skb->sk && skb_shinfo(skb)->tx_flags &
> SKBTX_WIFI_STATUS)
> + goto out_free;
Surely you shouldn't free them, even if you don't handle the status?!
johannes
