Hi Andrei,
Add new interface type VIRTUAL_AMP80211 which emulates Bluetooth AMP
Controller. AMP is Alternate MAC/PHYs Controller for Bluetooth sybsystem.
When an AMP is common between the two devices, the Bluetooth system
provides mechanisms for moving data traffic from BR/EDR Controller to
an AMP Controller.
Interesting work. I would have expected to see more work in cfg80211
though, this is a bit unexpected. How is it controlled at all?
create mode 100644 net/mac80211/virtual_amp.c
Also, why is it "virtual"? I would rather you name the file btamp.c or
such -- to you, AMP may mean something, but to us wifi people it really
doesn't mean much. I suspect most wouldn't even know where to look for a
definition of it.
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1783,7 +1783,8 @@ static int __init init_mac80211_hwsim(void)
BIT(NL80211_IFTYPE_P2P_CLIENT) |
BIT(NL80211_IFTYPE_P2P_GO) |
BIT(NL80211_IFTYPE_ADHOC) |
- BIT(NL80211_IFTYPE_MESH_POINT);
+ BIT(NL80211_IFTYPE_MESH_POINT) |
+ BIT(NL80211_IFTYPE_VIRTUAL_AMP80211);
This is insufficient, it should also beacon.
+++ b/include/linux/nl80211.h
@@ -1546,6 +1546,7 @@ enum nl80211_iftype {
NL80211_IFTYPE_MESH_POINT,
NL80211_IFTYPE_P2P_CLIENT,
NL80211_IFTYPE_P2P_GO,
+ NL80211_IFTYPE_VIRTUAL_AMP80211,
I believe that this may need additional checks in cfg80211 so that you
can't simply add this interface type.
+config MAC80211_VIRTUAL_AMP
+ bool "Virtual AMP80211 device"
+ ---help---
+ Enable Bluetooth Virtual / Software AMP 80211 controller.
+ When AMP is common between two devices data may be routed
+ through fast 80211 connection from standard Bluetooth BR/EDR
802.11, and the whole virtual vs. Bluetooth thing again.
Also, it seems this really needs a dependency on BT.
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index d9798a3..0e39ed7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -709,6 +709,10 @@ struct ieee80211_sub_if_data {
u32 rc_rateidx_mask[IEEE80211_NUM_BANDS];
u8 rc_rateidx_mcs_mask[IEEE80211_NUM_BANDS][IEEE80211_HT_MCS_MASK_LEN];
+#ifdef CONFIG_MAC80211_VIRTUAL_AMP
+ struct hci_dev *hdev;
+#endif
That should be inside the union:
union {
struct ieee80211_if_ap ap;
struct ieee80211_if_wds wds;
@@ -898,6 +900,11 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
case NL80211_IFTYPE_WDS:
case NL80211_IFTYPE_AP_VLAN:
break;
+ case NL80211_IFTYPE_VIRTUAL_AMP80211:
+#ifdef CONFIG_MAC80211_VIRTUAL_AMP
+ ieee80211_vamp_setup_sdata(sdata);
I also really don't like the abbrevation "vamp". It means even less than
"virtual AMP", particularly for people not familiar with BT terminology.
Please also use btamp or so.
+static void ieee80211_clean_sdata(struct ieee80211_sub_if_data *sdata)
+{
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_VIRTUAL_AMP80211:
+#ifdef CONFIG_MAC80211_VIRTUAL_AMP
+ ieee80211_vamp_clean_sdata(sdata);
+#endif
That's really bad form. Please hide the ifdefs better, or use an if
construct with something like vif_is_mesh() below that will not need ifdefs:
@@ -1230,6 +1250,9 @@ void ieee80211_if_remove(struct ieee80211_sub_if_data *sdata)
if (ieee80211_vif_is_mesh(&sdata->vif))
mesh_path_flush_by_iface(sdata);
+ /* clean up type-dependent data */
+ ieee80211_clean_sdata(sdata);
Seems you should move the mesh pieces into the new function ... Possibly
as a previous refactor patch.
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1299,6 +1299,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
case NUM_NL80211_IFTYPES:
case NL80211_IFTYPE_P2P_CLIENT:
case NL80211_IFTYPE_P2P_GO:
+ case NL80211_IFTYPE_VIRTUAL_AMP80211:
WARN_ON(1);
Umm. This can happen, I'd say.
+#include<net/bluetooth/bluetooth.h>
I'm not sure I fully trust thunderbird, but there seem to be missing spaces.
+static int virtual_amp_init(struct ieee80211_sub_if_data *sdata)
+{
+ struct hci_dev *hdev;
+ struct vamp_data *data;
+
+ data = kzalloc(sizeof(struct vamp_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
Why can't you embed the data in the union inside the sdata? it's
probably smaller than managed anyway.
+ hdev->bus = HCI_VIRTUAL;
Should that really be that way?
+ hci_set_drvdata(hdev, data);
+
+ hdev->dev_type = HCI_AMP;
+
+ hdev->open = vamp_open_dev;
+ hdev->close = vamp_close_dev;
+ hdev->flush = vamp_flush;
+ hdev->send = vamp_send_frame;
+
+ if (hci_register_dev(hdev)< 0) {
+ BT_ERR("Can't register HCI device");
+ kfree(data);
+ hci_free_dev(hdev);
+ return -EBUSY;
+ }
Why go have return values when you don't use them:
+void ieee80211_vamp_setup_sdata(struct ieee80211_sub_if_data *sdata)
+{
+ pr_info("Create virtual AMP device");
+
+ virtual_amp_init(sdata);
+
+}
Anyway.
I don't get this patch at all. Why am I reviewing some very very basic
skeleton code when we should be discussing userspace APIs (we have
already discussed them with a few people years ago), how the AMP is
going to be managed, how the security handshake is going to work, etc.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
