diff mbox series

wifi: nl80211: remove set_fils_aad support

Message ID 20230926160950.d698c25528e3.If118a835a25c59de20e1728ab71949fdb4172fb2@changeid
State New
Headers show
Series wifi: nl80211: remove set_fils_aad support | expand

Commit Message

Johannes Berg Sept. 26, 2023, 2:09 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

There's no user for this, so remove the support.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h  |  6 ------
 net/wireless/nl80211.c  | 30 ------------------------------
 net/wireless/rdev-ops.h | 14 --------------
 net/wireless/trace.h    | 18 ------------------
 4 files changed, 68 deletions(-)

Comments

Jeff Johnson Sept. 27, 2023, 6:03 p.m. UTC | #1 
On 9/27/2023 12:00 AM, Johannes Berg wrote:
> On Tue, 2023-09-26 at 13:35 -0700, Jeff Johnson wrote:
>> On 9/26/2023 7:09 AM, Johannes Berg wrote:
>>> From: Johannes Berg <johannes.berg@intel.com>
>>>
>>> There's no user for this, so remove the support.
>>>
>>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>>
>> if you are going to remove it, why not just revert e306784a8de0
>> ("cfg80211: AP mode driver offload for FILS association crypto") to make
>> sure you get all of the artifacts? for example, I believe your patch
>> will leave behind an unused struct cfg80211_fils_aad and unused
>> FILS_AAD_ASSIGN() trace helper macro.
> 
> Hah, good point, I didn't do this well.
> 
>> the caveat to reverting is that it should only be a partial revert; the
>> UAPI definitions would need to be retained (and should be documented as
>> obsolete).
> 
> Yeah, that's why I didn't do it as a revert.
> 
>> however, let me check to make sure there is no plan to actually utilize
>> this interface upstream. as i've indicated earlier, we are in the
>> process of trying to transition to an "upstream first" mentality, but
>> this is not going to happen overnight, but instead will take years. that
>> said, i'd hate to rip out an interface now just to need to add it back
>> in the future.
> 
> Sure. I don't mind keeping something around that really _has_ a future,
> but it's been two years and nobody showed up ... but yeah, I also think
> that "has a future" means upstream.
> 
> And clearly the old "other people can use it" argument doesn't work any
> more either, the only other vendors who are doing something in AP mode
> are Mediatek and maybe to some extent Realtek, and they all work on top
> of mac80211 with thinner firmware. Broadcom has disappeared as far as I
> can tell, with the occasional patch like recently that I'm also
> suspecting serves pure out-of-tree driver purposes...

Based on internal discussion I don't see a path forward to using this 
interface upstream, so please continue with the extraction, 
incorporating my prior comments.

Thanks!
/jeff
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 3a4b684f89bf..9f930750db2e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4384,10 +4384,6 @@  struct mgmt_frame_regs {
  *
  * @color_change: Initiate a color change.
  *
- * @set_fils_aad: Set FILS AAD data to the AP driver so that the driver can use
- *	those to decrypt (Re)Association Request and encrypt (Re)Association
- *	Response frame.
- *
  * @set_radar_background: Configure dedicated offchannel chain available for
  *	radar/CAC detection on some hw. This chain can't be used to transmit
  *	or receive frames and it is bounded to a running wdev.
@@ -4748,8 +4744,6 @@  struct cfg80211_ops {
 	int	(*color_change)(struct wiphy *wiphy,
 				struct net_device *dev,
 				struct cfg80211_color_change_settings *params);
-	int     (*set_fils_aad)(struct wiphy *wiphy, struct net_device *dev,
-				struct cfg80211_fils_aad *fils_aad);
 	int	(*set_radar_background)(struct wiphy *wiphy,
 					struct cfg80211_chan_def *chandef);
 	int	(*add_link_station)(struct wiphy *wiphy, struct net_device *dev,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index de47838aca4f..45efc79bfa3c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -16079,29 +16079,6 @@  static int nl80211_color_change(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
-static int nl80211_set_fils_aad(struct sk_buff *skb,
-				struct genl_info *info)
-{
-	struct cfg80211_registered_device *rdev = info->user_ptr[0];
-	struct net_device *dev = info->user_ptr[1];
-	struct cfg80211_fils_aad fils_aad = {};
-	u8 *nonces;
-
-	if (!info->attrs[NL80211_ATTR_MAC] ||
-	    !info->attrs[NL80211_ATTR_FILS_KEK] ||
-	    !info->attrs[NL80211_ATTR_FILS_NONCES])
-		return -EINVAL;
-
-	fils_aad.macaddr = nla_data(info->attrs[NL80211_ATTR_MAC]);
-	fils_aad.kek_len = nla_len(info->attrs[NL80211_ATTR_FILS_KEK]);
-	fils_aad.kek = nla_data(info->attrs[NL80211_ATTR_FILS_KEK]);
-	nonces = nla_data(info->attrs[NL80211_ATTR_FILS_NONCES]);
-	fils_aad.snonce = nonces;
-	fils_aad.anonce = nonces + FILS_NONCE_LEN;
-
-	return rdev_set_fils_aad(rdev, dev, &fils_aad);
-}
-
 static int nl80211_add_link(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -17452,13 +17429,6 @@  static const struct genl_small_ops nl80211_small_ops[] = {
 		.flags = GENL_UNS_ADMIN_PERM,
 		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
 	},
-	{
-		.cmd = NL80211_CMD_SET_FILS_AAD,
-		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
-		.doit = nl80211_set_fils_aad,
-		.flags = GENL_UNS_ADMIN_PERM,
-		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
-	},
 	{
 		.cmd = NL80211_CMD_ADD_LINK,
 		.doit = nl80211_add_link,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 90bb7ac4b930..9dbad6ecbc6d 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1402,20 +1402,6 @@  static inline int rdev_color_change(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
-static inline int
-rdev_set_fils_aad(struct cfg80211_registered_device *rdev,
-		  struct net_device *dev, struct cfg80211_fils_aad *fils_aad)
-{
-	int ret = -EOPNOTSUPP;
-
-	trace_rdev_set_fils_aad(&rdev->wiphy, dev, fils_aad);
-	if (rdev->ops->set_fils_aad)
-		ret = rdev->ops->set_fils_aad(&rdev->wiphy, dev, fils_aad);
-	trace_rdev_return_int(&rdev->wiphy, ret);
-
-	return ret;
-}
-
 static inline int
 rdev_set_radar_background(struct cfg80211_registered_device *rdev,
 			  struct cfg80211_chan_def *chandef)
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 617c0d0dfa96..c6870c311cdf 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2706,24 +2706,6 @@  DEFINE_EVENT(wiphy_wdev_cookie_evt, rdev_abort_pmsr,
 	TP_ARGS(wiphy, wdev, cookie)
 );
 
-TRACE_EVENT(rdev_set_fils_aad,
-	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
-		 struct cfg80211_fils_aad *fils_aad),
-	TP_ARGS(wiphy, netdev, fils_aad),
-	TP_STRUCT__entry(WIPHY_ENTRY
-		NETDEV_ENTRY
-		__array(u8, macaddr, ETH_ALEN)
-		__field(u8, kek_len)
-	),
-	TP_fast_assign(WIPHY_ASSIGN;
-		NETDEV_ASSIGN;
-		FILS_AAD_ASSIGN(fils_aad);
-	),
-	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " FILS_AAD_PR_FMT,
-		  WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->macaddr,
-		  __entry->kek_len)
-);
-
 TRACE_EVENT(rdev_update_owe_info,
 	    TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		     struct cfg80211_update_owe_info *owe_info),