lkml.org 
[lkml]   [2014]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC 2/2] mac80211: Add new interface type for OCB mode
From
Date
On Mon, 2014-05-19 at 16:49 +0200, Rostislav Lisovy wrote:

> include/uapi/linux/nl80211.h | 3 ++
> net/mac80211/Makefile | 3 +-
> net/mac80211/cfg.c | 1 +
> net/mac80211/chan.c | 2 +
> net/mac80211/driver-ops.h | 3 +-
> net/mac80211/ieee80211_i.h | 38 ++++++++++++++
> net/mac80211/iface.c | 28 +++++++++++
> net/mac80211/ocb.c | 115 +++++++++++++++++++++++++++++++++++++++++++
> net/mac80211/util.c | 5 ++
> net/wireless/util.c | 1 +

please avoid patches that modify both cfg80211 and mac80211 unless
they're really small.

> +/**
> + * struct ieee80211_if_ocb - the specific struct for ocb-mode
> + *
> + * In this struct all ocb-specific information of an interface is stored.
> + *
> + * @timer: the timer used for all tasks in the ocb-code

Why do you need your own timer?

> + * @work: holds the workqueue

and work struct (not a workqueue - that's something entirely different)?
we have a general interface work struct.

> + * @skb_queue: holds all queued skb to be processed

Why do you need to queue SKBs?

> + * @wrkq_flags: bitmask telling what work is pending
> + * @timer_running: tells if the timer is running
> + * @bssid: holds the BSSID (IEEE802.11p defines this to be
> + * ff:ff:ff:ff:ff:ff but this is more flexible)

Why would you ever need this flexibility? It seems pointless to prepare
for it - I'm guessing you're never going to implement it?

> @@ -622,6 +632,17 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
>
> if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE)
> changed |= ieee80211_reset_erp_info(sdata);
> +
> + if (sdata->vif.type == NL80211_IFTYPE_OCB) {
> + /* Disable beacons */
> + sdata->vif.bss_conf.enable_beacon = false;
> + changed |= BSS_CHANGED_BEACON;
> +
> + /* Receive all data frames */
> + local->fif_other_bss++;
> + ieee80211_configure_filter(local);
> + }

NACK. Please make a proper "join OCB" operation etc. In the wireless
stack, "ifup" shouldn't be used to start real operation.

> +/**
> + * ieee80211_ocb_housekeeping - Housekeeping function (expires stations)
> + *
> + * @sdata:
> + * @ifocb:

yes?

> + * This function is used for all periodical clean up work.
> + * It expires all stations that have not shown up for a given period of time.
> + * After all cleanups have been done it schedules the next run.
> + */
> +static void ieee80211_ocb_housekeeping(struct ieee80211_sub_if_data *sdata,
> + struct ieee80211_if_ocb *ifocb)
> +{
> + ieee80211_sta_expire(sdata, IEEE80211_OCB_PEER_INACTIVITY_LIMIT);

> + mod_timer(&ifocb->timer,
> + round_jiffies(jiffies + IEEE80211_OCB_HOUSEKEEPING_INTERVAL));
> +}

use a delayed work struct?

I don't even see a call to turn on the carrier?

Anyway I think you should have some OCB join method in the cfg80211 API
even if there's nothing really to do... at minimum, you need to specify
a channel somehow!

johannes



\
 
 \ /
  Last update: 2014-05-19 23:41    [W:0.636 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site