Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/9] cfg80211: add add_nan_func / rm_nan_func
From: Arend Van Spriel @ 2016-09-18 19:54 UTC (permalink / raw)
  To: Luca Coelho, johannes
  Cc: linux-wireless, Ayala Beker, Andrei Otcheretianski,
	Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20160916083321.5840-4-luca@coelho.fi>

On 16-9-2016 10:33, Luca Coelho wrote:
> From: Ayala Beker <ayala.beker@intel.com>
> 
> A NAN function can be either publish, subscribe or follow
> up. Make all the necessary verifications and just pass the
> request to the driver.
> Allow the user space application that starts NAN to
> forbid any other socket to add or remove functions.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Signed-off-by: Ayala Beker <ayala.beker@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---

[...]

> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 7ab18c8..ab16c8e 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -847,6 +847,24 @@
>   *	After this command NAN functions can be added.
>   * @NL80211_CMD_STOP_NAN: Stop the NAN operation, identified by
>   *	its %NL80211_ATTR_WDEV interface.
> + * @NL80211_CMD_ADD_NAN_FUNCTION: Add a NAN function. The function is defined
> + *	with %NL80211_ATTR_NAN_FUNC nested attribute. When called, this
> + *	operation returns the strictly positive and unique instance id
> + *	(%NL80211_ATTR_NAN_FUNC_INST_ID) and a cookie (%NL80211_ATTR_COOKIE)
> + *	of the function upon success.
> + *	Since instance ID's can be re-used, this cookie is the right
> + *	way to identify the function. This will avoid races when a termination
> + *	event is handled by the user space after it has already added a new
> + *	function that got the same instance id from the kernel as the one
> + *	which just terminated.
> + *	This cookie may be used in NAN events even before the command
> + *	returns, so userspace shouldn't process NAN events until it processes
> + *	the response to this command.
> + *	Look at %NL80211_ATTR_SOCKET_OWNER as well.
> + * @NL80211_CMD_RM_NAN_FUNCTION: Remove a NAN function by cookie.
> + *	This command is also used as a notification sent when a NAN function is
> + *	terminated. This will contain a %NL80211_ATTR_NAN_FUNC_INST_ID
> + *	and %NL80211_ATTR_COOKIE attributes.

This patch does not show the notification scenario as it is added in
patch 6. So those three lines could be added by that patch instead to
keep things logically together.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v2 4/9] cfg80211: allow the user space to change current NAN configuration
From: Arend Van Spriel @ 2016-09-18 19:43 UTC (permalink / raw)
  To: Luca Coelho, johannes
  Cc: linux-wireless, Ayala Beker, Andrei Otcheretianski,
	Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20160916083321.5840-5-luca@coelho.fi>

On 16-9-2016 10:33, Luca Coelho wrote:
> From: Ayala Beker <ayala.beker@intel.com>
> 
> Some NAN configuration paramaters may change during the operation of
> the NaN device. For example, a user may want to update master preference

typo: NAN iso NaN.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v2 3/9] cfg80211: add add_nan_func / rm_nan_func
From: Arend Van Spriel @ 2016-09-18 19:28 UTC (permalink / raw)
  To: Luca Coelho, johannes
  Cc: linux-wireless, Ayala Beker, Andrei Otcheretianski,
	Emmanuel Grumbach, Luca Coelho
In-Reply-To: <20160916083321.5840-4-luca@coelho.fi>

On 16-9-2016 10:33, Luca Coelho wrote:
> From: Ayala Beker <ayala.beker@intel.com>
> 
> A NAN function can be either publish, subscribe or follow
> up. Make all the necessary verifications and just pass the
> request to the driver.
> Allow the user space application that starts NAN to
> forbid any other socket to add or remove functions.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Signed-off-by: Ayala Beker <ayala.beker@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>  include/net/cfg80211.h       |  91 +++++++++++
>  include/uapi/linux/nl80211.h | 153 ++++++++++++++++++
>  net/wireless/core.c          |   3 +-
>  net/wireless/nl80211.c       | 368 +++++++++++++++++++++++++++++++++++++++++++
>  net/wireless/rdev-ops.h      |  21 +++
>  net/wireless/trace.h         |  39 +++++
>  net/wireless/util.c          |  22 +++
>  7 files changed, 696 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ca64d69..ced5b8a 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2306,6 +2306,73 @@ struct cfg80211_nan_conf {
>  };
>  
>  /**
> + * struct cfg80211_nan_func_filter - a NAN function Rx / Tx filter
> + *
> + * @filter: the content of the filter
> + * @len: the length of the filter
> + */
> +struct cfg80211_nan_func_filter {
> +	const u8 *filter;
> +	u8 len;
> +};
> +
> +/**
> + * struct cfg80211_nan_func - a NAN function
> + *
> + * @type: &enum nl80211_nan_function_type
> + * @service_id: the service ID of the function
> + * @publish_type: &nl80211_nan_publish_type
> + * @close_range: if true, the range should be limited. Threshold is
> + *	implementation specific.
> + * @publish_bcast: if true, the solicited publish should be broadcasted
> + * @subscribe_active: if true, the subscribe is active
> + * @followup_id: the instance ID for follow up
> + * @followup_reqid: the requestor instance ID for follow up
> + * @followup_dest: MAC address of the recipient of the follow up
> + * @ttl: time to live counter in DW.
> + * @serv_spec_info: Service Specific Info
> + * @serv_spec_info_len: Service Specific Info length
> + * @srf_include: if true, SRF is inclusive
> + * @srf_bf: Bloom Filter
> + * @srf_bf_len: Bloom Filter length
> + * @srf_bf_idx: Bloom Filter index
> + * @srf_macs: SRF MAC addresses
> + * @srf_num_macs: number of MAC addresses in SRF
> + * @rx_filters: rx filters that are matched with corresponding peer's tx_filter
> + * @tx_filters: filters that should be transmitted in the SDF.
> + * @num_rx_filters: length of &rx_filters.
> + * @num_tx_filters: length of &tx_filters.
> + * @instance_id: driver allocated id of the function.
> + * @cookie: unique NAN function identifier.

Might be wrong but it seems a subset of the fields is used depending on
the type of NAN function. Maybe better to separate the function type
specific field in a sub structure defintion.

> + */
> +struct cfg80211_nan_func {
> +	enum nl80211_nan_function_type type;
> +	u8 service_id[NL80211_NAN_FUNC_SERVICE_ID_LEN];
> +	u8 publish_type;
> +	bool close_range;
> +	bool publish_bcast;
> +	bool subscribe_active;
> +	u8 followup_id;
> +	u8 followup_reqid;
> +	struct mac_address followup_dest;
> +	u32 ttl;
> +	const u8 *serv_spec_info;
> +	u8 serv_spec_info_len;
> +	bool srf_include;
> +	const u8 *srf_bf;
> +	u8 srf_bf_len;
> +	u8 srf_bf_idx;
> +	struct mac_address *srf_macs;
> +	int srf_num_macs;
> +	struct cfg80211_nan_func_filter *rx_filters;
> +	struct cfg80211_nan_func_filter *tx_filters;
> +	u8 num_tx_filters;
> +	u8 num_rx_filters;
> +	u8 instance_id;
> +	u64 cookie;
> +};
> +
> +/**
>   * struct cfg80211_ops - backend description for wireless configuration
>   *
>   * This struct is registered by fullmac card drivers and/or wireless stacks
> @@ -2595,6 +2662,14 @@ struct cfg80211_nan_conf {
>   *	peers must be on the base channel when the call completes.
>   * @start_nan: Start the NAN interface.
>   * @stop_nan: Stop the NAN interface.
> + * @add_nan_func: Add a NAN function. Returns negative value on failure.
> + *	On success @nan_func ownership is transferred to the driver and
> + *	it may access it outside of the scope of this function. The driver
> + *	should free the @nan_func when no longer needed by calling
> + *	cfg80211_free_nan_func().
> + *	On success the driver should assign an instance_id in the
> + *	provided @nan_func.
> + * @rm_nan_func: Remove a NAN function.

Would prefer del_nan_func here. At least all other add_* callbacks in
this structure have a del_* counter part.

>   */
>  struct cfg80211_ops {
>  	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
> @@ -2863,6 +2938,10 @@ struct cfg80211_ops {
>  	int	(*start_nan)(struct wiphy *wiphy, struct wireless_dev *wdev,
>  			     struct cfg80211_nan_conf *conf);
>  	void	(*stop_nan)(struct wiphy *wiphy, struct wireless_dev *wdev);
> +	int	(*add_nan_func)(struct wiphy *wiphy, struct wireless_dev *wdev,
> +				struct cfg80211_nan_func *nan_func);
> +	void	(*rm_nan_func)(struct wiphy *wiphy, struct wireless_dev *wdev,
> +			       u64 cookie);
>  };
>  
>  /*
> @@ -3311,6 +3390,8 @@ struct wiphy_iftype_ext_capab {
>   * @bss_select_support: bitmask indicating the BSS selection criteria supported
>   *	by the driver in the .connect() callback. The bit position maps to the
>   *	attribute indices defined in &enum nl80211_bss_select_attr.
> + *
> + * @cookie_counter: unique generic cookie counter, used to identify objects.
>   */
>  struct wiphy {
>  	/* assign these fields before you register the wiphy */
> @@ -3440,6 +3521,8 @@ struct wiphy {
>  
>  	u32 bss_select_support;
>  
> +	u64 cookie_counter;
> +
>  	char priv[0] __aligned(NETDEV_ALIGN);
>  };
>  
> @@ -5529,6 +5612,14 @@ wiphy_ext_feature_isset(struct wiphy *wiphy,
>  	return (ft_byte & BIT(ftidx % 8)) != 0;
>  }
>  
> +/**
> + * cfg80211_free_nan_func - free NAN function
> + * @f: NAN function that should be freed
> + *
> + * Frees all the NAN function and all it's allocated members.
> + */
> +void cfg80211_free_nan_func(struct cfg80211_nan_func *f);
> +
>  /* ethtool helper */
>  void cfg80211_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info);
>  
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 7ab18c8..ab16c8e 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -847,6 +847,24 @@
>   *	After this command NAN functions can be added.
>   * @NL80211_CMD_STOP_NAN: Stop the NAN operation, identified by
>   *	its %NL80211_ATTR_WDEV interface.
> + * @NL80211_CMD_ADD_NAN_FUNCTION: Add a NAN function. The function is defined
> + *	with %NL80211_ATTR_NAN_FUNC nested attribute. When called, this
> + *	operation returns the strictly positive and unique instance id
> + *	(%NL80211_ATTR_NAN_FUNC_INST_ID) and a cookie (%NL80211_ATTR_COOKIE)
> + *	of the function upon success.
> + *	Since instance ID's can be re-used, this cookie is the right
> + *	way to identify the function. This will avoid races when a termination
> + *	event is handled by the user space after it has already added a new
> + *	function that got the same instance id from the kernel as the one
> + *	which just terminated.
> + *	This cookie may be used in NAN events even before the command
> + *	returns, so userspace shouldn't process NAN events until it processes
> + *	the response to this command.
> + *	Look at %NL80211_ATTR_SOCKET_OWNER as well.
> + * @NL80211_CMD_RM_NAN_FUNCTION: Remove a NAN function by cookie.
> + *	This command is also used as a notification sent when a NAN function is
> + *	terminated. This will contain a %NL80211_ATTR_NAN_FUNC_INST_ID
> + *	and %NL80211_ATTR_COOKIE attributes.
>   *
>   * @NL80211_CMD_MAX: highest used command number
>   * @__NL80211_CMD_AFTER_LAST: internal use
> @@ -1038,6 +1056,8 @@ enum nl80211_commands {
>  
>  	NL80211_CMD_START_NAN,
>  	NL80211_CMD_STOP_NAN,
> +	NL80211_CMD_ADD_NAN_FUNCTION,
> +	NL80211_CMD_RM_NAN_FUNCTION,

NL80211_CMD_DEL_NAN_FUNCTION?

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop NAN
From: Arend Van Spriel @ 2016-09-18 19:01 UTC (permalink / raw)
  To: Otcheretianski, Andrei, Luca Coelho, johannes@sipsolutions.net
  Cc: linux-wireless@vger.kernel.org, Beker, Ayala, Grumbach, Emmanuel,
	Coelho, Luciano
In-Reply-To: <B2CD83211F5FDD478D7EFFF030D107ED57286E64@hasmsx108.ger.corp.intel.com>

On 18-9-2016 9:59, Otcheretianski, Andrei wrote:
>> -----Original Message-----
>> From: Arend Van Spriel [mailto:arend.vanspriel@broadcom.com]
>> Sent: Friday, September 16, 2016 14:09
>> To: Luca Coelho <luca@coelho.fi>; johannes@sipsolutions.net
>> Cc: linux-wireless@vger.kernel.org; Beker, Ayala <ayala.beker@intel.com>;
>> Otcheretianski, Andrei <andrei.otcheretianski@intel.com>; Grumbach,
>> Emmanuel <emmanuel.grumbach@intel.com>; Coelho, Luciano
>> <luciano.coelho@intel.com>
>> Subject: Re: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop
>> NAN
>>
>> On 16-9-2016 10:33, Luca Coelho wrote:
>>> From: Ayala Beker <ayala.beker@intel.com>
>>>
>>> This code doesn't do much besides allowing to start and stop the vif.
>>>
>>> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
>>> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
>>> Signed-off-by: Ayala Beker <ayala.beker@intel.com>
>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>>> ---
>>>  include/net/mac80211.h    |  9 +++++++++
>>>  net/mac80211/cfg.c        | 36 ++++++++++++++++++++++++++++++++++
>>>  net/mac80211/chan.c       |  3 +++
>>>  net/mac80211/driver-ops.h | 29 ++++++++++++++++++++++++++-
>>>  net/mac80211/iface.c      |  8 ++++++--
>>>  net/mac80211/main.c       |  5 +++++
>>>  net/mac80211/offchannel.c |  3 ++-
>>>  net/mac80211/trace.h      | 50
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  net/mac80211/util.c       |  3 ++-
>>>  9 files changed, 141 insertions(+), 5 deletions(-)
>>
>> [...]
>>
>>> diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
>>> index fe35a1c..67b42c8 100644
>>> --- a/net/mac80211/driver-ops.h
>>> +++ b/net/mac80211/driver-ops.h
>>> @@ -163,7 +163,8 @@ static inline void drv_bss_info_changed(struct
>>> ieee80211_local *local,
>>>
>>>  	if (WARN_ON_ONCE(sdata->vif.type ==
>> NL80211_IFTYPE_P2P_DEVICE ||
>>>  			 (sdata->vif.type == NL80211_IFTYPE_MONITOR &&
>>> -			  !sdata->vif.mu_mimo_owner)))
>>> +			  !sdata->vif.mu_mimo_owner) ||
>>> +			 sdata->vif.type == NL80211_IFTYPE_NAN))
>>
>> Might be more clear to move this up right after P2P_DEVICE check.
> 
> Why? It's a completely separate new condition - so it goes to the end.

I would say readability. Both P2P_DEVICE and NAN checks are single
comparisons as opposed to the MONITOR check.

>>
>>>  		return;
>>>
>>>  	if (!check_sdata_in_driver(sdata))
>>> @@ -1165,4 +1166,30 @@ static inline void drv_wake_tx_queue(struct
>> ieee80211_local *local,
>>>  	local->ops->wake_tx_queue(&local->hw, &txq->txq);  }
>>>
>>> +static inline int drv_start_nan(struct ieee80211_local *local,
>>> +				struct ieee80211_sub_if_data *sdata,
>>> +				struct cfg80211_nan_conf *conf)
>>> +{
>>> +	int ret;
>>> +
>>> +	might_sleep();
>>> +	check_sdata_in_driver(sdata);
>>> +
>>> +	trace_drv_start_nan(local, sdata, conf);
>>> +	ret = local->ops->start_nan(&local->hw, &sdata->vif, conf);
>>> +	trace_drv_return_int(local, ret);
>>> +	return ret;
>>> +}
>>> +
>>> +static inline void drv_stop_nan(struct ieee80211_local *local,
>>> +				struct ieee80211_sub_if_data *sdata) {
>>> +	might_sleep();
>>> +	check_sdata_in_driver(sdata);
>>> +
>>> +	trace_drv_stop_nan(local, sdata);
>>> +	local->ops->stop_nan(&local->hw, &sdata->vif);
>>> +	trace_drv_return_void(local);
>>> +}
>>> +
>>>  #endif /* __MAC80211_DRIVER_OPS */
>>> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index
>>> e694ca2..507f46a 100644
>>> --- a/net/mac80211/iface.c
>>> +++ b/net/mac80211/iface.c
>>> @@ -327,6 +327,9 @@ static int ieee80211_check_queues(struct
>> ieee80211_sub_if_data *sdata,
>>>  	int n_queues = sdata->local->hw.queues;
>>>  	int i;
>>>
>>> +	if (iftype == NL80211_IFTYPE_NAN)
>>> +		return 0;
>>> +
>>>  	if (iftype != NL80211_IFTYPE_P2P_DEVICE) {
>>>  		for (i = 0; i < IEEE80211_NUM_ACS; i++) {
>>>  			if (WARN_ON_ONCE(sdata->vif.hw_queue[i] == @@
>> -647,7 +650,8 @@ int
>>> ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
>>>  			local->fif_probe_req++;
>>>  		}
>>>
>>> -		if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE)
>>> +		if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE &&
>>> +		    sdata->vif.type != NL80211_IFTYPE_NAN)
>>
>> similar check keeps reoccuring in various places so maybe we can create a
>> helper function for it.
> 
> Right, but not sure that it deserves a function.

If similar new iftypes are anticipated it would make sense as it would
mean adding it in one place.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH v2 1/9] cfg80211: add start / stop NAN commands
From: Arend Van Spriel @ 2016-09-18 18:54 UTC (permalink / raw)
  To: Otcheretianski, Andrei, Luca Coelho, johannes@sipsolutions.net
  Cc: linux-wireless@vger.kernel.org, Beker, Ayala, Grumbach, Emmanuel,
	Coelho, Luciano
In-Reply-To: <B2CD83211F5FDD478D7EFFF030D107ED57286E44@hasmsx108.ger.corp.intel.com>

On 18-9-2016 9:44, Otcheretianski, Andrei wrote:
>> -----Original Message-----
>> From: Arend Van Spriel [mailto:arend.vanspriel@broadcom.com]
>> Sent: Friday, September 16, 2016 13:59
>> To: Luca Coelho <luca@coelho.fi>; johannes@sipsolutions.net
>> Cc: linux-wireless@vger.kernel.org; Beker, Ayala <ayala.beker@intel.com>;
>> Otcheretianski, Andrei <andrei.otcheretianski@intel.com>; Grumbach,
>> Emmanuel <emmanuel.grumbach@intel.com>; Coelho, Luciano
>> <luciano.coelho@intel.com>
>> Subject: Re: [PATCH v2 1/9] cfg80211: add start / stop NAN commands
>>
>> On 16-9-2016 10:33, Luca Coelho wrote:
>>> From: Ayala Beker <ayala.beker@intel.com>
>>>
>>> This allows user space to start/stop NAN interface.
>>> A NAN interface is like P2P device in a few aspects: it doesn't have a
>>> netdev associated to it.
>>> Add the new interface type and prevent operations that can't be
>>> executed on NAN interface like scan.
>>>
>>> Define several attributes that may be configured by user space when
>>> starting NAN functionality (master preference and dual band operation)
>>>
>>> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
>>> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>>> ---
>>>  include/net/cfg80211.h       | 21 +++++++++-
>>>  include/uapi/linux/nl80211.h | 52 +++++++++++++++++++++++++
>>>  net/mac80211/cfg.c           |  2 +
>>>  net/mac80211/chan.c          |  3 ++
>>>  net/mac80211/iface.c         |  4 ++
>>>  net/mac80211/offchannel.c    |  1 +
>>>  net/mac80211/rx.c            |  3 ++
>>>  net/mac80211/util.c          |  1 +
>>>  net/wireless/chan.c          |  2 +
>>>  net/wireless/core.c          | 34 ++++++++++++++++
>>>  net/wireless/core.h          |  3 ++
>>>  net/wireless/mlme.c          |  1 +
>>>  net/wireless/nl80211.c       | 93
>> ++++++++++++++++++++++++++++++++++++++++++--
>>>  net/wireless/rdev-ops.h      | 20 ++++++++++
>>>  net/wireless/trace.h         | 27 +++++++++++++
>>>  net/wireless/util.c          |  6 ++-
>>>  16 files changed, 267 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index
>>> d5e7f69..ca64d69 100644
>>> --- a/include/net/cfg80211.h
>>> +++ b/include/net/cfg80211.h
>>> @@ -2293,6 +2293,19 @@ struct cfg80211_qos_map {  };
>>
>> [...]
>>
>>> +/**
>>> + * enum nl80211_nan_dual_band_conf - NAN dual band configuration
>>> + *
>>> + * Defines the NAN dual band mode of operation
>>
>> Does it make sense to have such a notion of bands in use. And what does
>> 5.2GHz mean. Is this a subband within 5G channels? Probably I should read
>> the NAN spec to understand what is meant here. I would consider 5.2G as
>> lower 5G, ie. discovery on channel 44 but not sure if that is meant here.
>>
> 
> The NAN spec defines single and dual band modes of operation. The channels are fixed for each band.
> On 2.4Ghz is channel 6 and 5GHz is either 44 or 149. Regarding 5.2 - it's just a typo. It should be 5G - no deeper meaning.

The thing is that it seems likely other bands will be added so that
would kinda obsolete this whole enum. So I would propose to have another
way to configure the bands to use. This enum is not really extensible
though it may reflect the current state of the spec, which is still in
draft if I am not mistaken.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Kalle Valo @ 2016-09-18 16:57 UTC (permalink / raw)
  To: Greg KH
  Cc: Christian Lamparter, Christian Lamparter, linux-kernel,
	linux-wireless, b43-dev, Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <20160918101455.GA8410@kroah.com>

Greg KH <gregkh@linuxfoundation.org> writes:

> On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
>> Greg KH <gregkh@linuxfoundation.org> writes:
>> 
>> > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
>> >> Ben Greear reported:
>> >> > I see lots of instability as soon as I load up the carl9710 NIC.
>> >> > My application is going to be poking at it's debugfs files...
>> >> >
>> >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
>> >> > [carl9170] at addr ffff8801bc1208b0
>> >> > Read of size 8 by task btserver/5888
>> >> > =======================================================================
>> >> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
>> >> > -----------------------------------------------------------------------
>> >> >
>> >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
>> >> >...
>> >> 
>> >> This breakage was caused by the introduction of intermediate
>> >> fops in debugfs by commit 9fd4dcece43a
>> >> ("debugfs: prevent access to possibly dead file_operations at file open")
>> >
>> > Because of this, these should all be backported to 4.7-stable, and
>> > 4.8-stable, right?
>> 
>> Via which tree should these go, Greg's or mine?
>
> I'll take it if you ack it, as it's a debugfs issue.

Good, thanks. The wireless patches look good to me so:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Greg KH @ 2016-09-18 16:44 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Kalle Valo, linux-kernel, linux-wireless, b43-dev, Nicolai Stange,
	Ben Greear, Larry Finger
In-Reply-To: <2731122.i7AxIUFRZe@debian64>

On Sun, Sep 18, 2016 at 02:49:33PM +0200, Christian Lamparter wrote:
> On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote:
> > On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
> > > Greg KH <gregkh@linuxfoundation.org> writes:
> > > 
> > > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> > > >> Ben Greear reported:
> > > >> > I see lots of instability as soon as I load up the carl9710 NIC.
> > > >> > My application is going to be poking at it's debugfs files...
> > > >> >
> > > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > > >> > [carl9170] at addr ffff8801bc1208b0
> > > >> > Read of size 8 by task btserver/5888
> > > >> > =======================================================================
> > > >> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
> > > >> > -----------------------------------------------------------------------
> > > >> >
> > > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> > > >> >...
> > > >> 
> > > >> This breakage was caused by the introduction of intermediate
> > > >> fops in debugfs by commit 9fd4dcece43a
> > > >> ("debugfs: prevent access to possibly dead file_operations at file open")
> > > >
> > > > Because of this, these should all be backported to 4.7-stable, and
> > > > 4.8-stable, right?
> Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 
> debugfs support is usually disabled.
> 
> Greg, would you take these four patches "as is" for -stable
> or do you want a "minimal version" which just replaces the
> 
> dfops = container_of(file->f_op, ...
> 
> with
> 
> dfops = container_of(file->f_path.dentry->d_fsdata, ...
> 
> in the three drivers for -stable?

No, I'll take this as is, we want things to remain as close as possible
to Linus's tree.  When we are not, is when things break.

> > > Via which tree should these go, Greg's or mine?
> > 
> > I'll take it if you ack it, as it's a debugfs issue.
> For carl9170: Ben Greear has reported:
> "I have verified this fixes my problem in the 4.7 kernel."
> 
> But this was with a preliminary/minimal version so I didn't
> add the tested-by tag.
> 
> As for b43, I'll see if I have a working b43 in my collection
> somewhere to confirm the issue and the fix. Question is, do
> you want to wait or not?

I'll queue these up this week, no rush.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Jes Sorensen @ 2016-09-18 16:26 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Joe Perches, Larry Finger, devel, linux-wireless
In-Reply-To: <8760pt7hzv.fsf@kamboji.qca.qualcomm.com>

Kalle Valo <kvalo@codeaurora.org> writes:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>
>> Joe Perches <joe@perches.com> writes:
>>> I think it'd be nicer to use dev_dbg for all these cases
>>> and as well use some new macro that includes the test
>>>
>>> Something like:
>>>
>>> #define rtl8xxxu_dbg(type, fmt, ...)			\
>>> do {							\
>>> 	if (rtl8xxxu_debug & (type))			\
>>> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
>>> } while (0)
>>
>> Yuck yuck yuck, no thanks!
>>
>> Any attempt of adding that kinda grossness to the driver will get a
>> NACK.
>
> Huh, how is that ugly? To me it's the opposite, original code is ugly
> and Joes' proposal makes sense. Lots of wireless drivers have something
> similar.

Sorry it's a classic case of obfuscating the code for zero gain. If
someone else likes this kinda wrapper in their code, by all means go
ahead. In my book it's just bad coding taste.

Jes

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Jes Sorensen @ 2016-09-18 15:00 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, devel, linux-wireless
In-Reply-To: <a9bcf666-719e-bda8-17bb-212e269ca497@lwfinger.net>

Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 09/17/2016 03:59 PM, Jes Sorensen wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>> As soon as debugging is turned on, the logs are filled with messages
>>> reporting the interrupt status. As this quantity is usually zero, this
>>> output is not needed. In fact, there will be a report if the status is
>>> not zero, thus the debug line in question could probably be deleted.
>>> Rather than taking that action, I have changed it to only be printed
>>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>>
>> Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
>> that.
>>
>> Which device do you see this with?
>
> OK. I will change the flag.
>
> I found this with a TP-Link TL-MN8200ND, which has some variant of the
> RTL8188CU chip. It transmits, but I see no evidence that the receiver
> is functioning at all. The same is true for driver rtl8192cu. Only the
> driver from Realtek's web site actually works.

I assume you mean TL-WN8200ND? That device is 'interesting' in the least
positive sense of the word. It seems abandoned by the manufacturer
too. I have one of them but never managed to get it working, not with
any driver under Linux nor Windows.

TP-Link shipped a driver disc with it, but you cannot install that in
any recent version of Windows because the OS ships with it's own driver
for the 8192cu/8188cu series and the device uses the common USB ID. I
have been meaning to see if I could find a box with Vista on it to
install their driver and run a USB trace on it.

> One other problem that I have found is that the debug option on module
> load seems to be ignored. So far, I've had to hard wire the
> flags. Once I find the reason, I'll send a patch for that as well.

That is odd - I use it regularly and haven't had problems with it.

Cheers,
Jes

^ permalink raw reply

* [PATCH 2/2] mac80211: Use rhltable instead of rhashtable
From: Herbert Xu @ 2016-09-18 13:54 UTC (permalink / raw)
  To: Johannes Berg, David S. Miller, netdev, linux-wireless,
	Thomas Graf, tom, Ben Greear
In-Reply-To: <20160918135030.GA7062@gondor.apana.org.au>

mac80211 currently uses rhashtable with insecure_elasticity set
to true.  The latter is because of duplicate objects.  What's
more, mac80211 walks the rhashtable chains by hand which is broken
as rhashtable may contain multiple tables due to resizing or
rehashing.

This patch fixes it by converting it to the newly added rhltable
interface which is designed for use with duplicate objects.

With rhltable a lookup returns a list of objects instead of a
single one.  This is then fed into the existing for_each_sta_info
macro.

This patch also deletes the sta_addr_hash function since rhashtable
defaults to jhash.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 net/mac80211/ieee80211_i.h |    2 -
 net/mac80211/rx.c          |    7 +-----
 net/mac80211/sta_info.c    |   52 ++++++++++++++++++---------------------------
 net/mac80211/sta_info.h    |   19 ++++++----------
 net/mac80211/status.c      |    7 +-----
 5 files changed, 33 insertions(+), 54 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f56d342..1a52cd4 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1208,7 +1208,7 @@ struct ieee80211_local {
 	spinlock_t tim_lock;
 	unsigned long num_sta;
 	struct list_head sta_list;
-	struct rhashtable sta_hash;
+	struct rhltable sta_hash;
 	struct timer_list sta_cleanup;
 	int sta_generation;
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 9dce3b1..5e26dc6 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3940,7 +3940,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
 	__le16 fc;
 	struct ieee80211_rx_data rx;
 	struct ieee80211_sub_if_data *prev;
-	struct rhash_head *tmp;
+	struct rhlist_head *tmp;
 	int err = 0;
 
 	fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
@@ -3983,13 +3983,10 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
 		goto out;
 	} else if (ieee80211_is_data(fc)) {
 		struct sta_info *sta, *prev_sta;
-		const struct bucket_table *tbl;
 
 		prev_sta = NULL;
 
-		tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
-
-		for_each_sta_info(local, tbl, hdr->addr2, sta, tmp) {
+		for_each_sta_info(local, hdr->addr2, sta, tmp) {
 			if (!prev_sta) {
 				prev_sta = sta;
 				continue;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 19f14c9..198d0bd 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -67,12 +67,10 @@
 
 static const struct rhashtable_params sta_rht_params = {
 	.nelem_hint = 3, /* start small */
-	.insecure_elasticity = true, /* Disable chain-length checks. */
 	.automatic_shrinking = true,
 	.head_offset = offsetof(struct sta_info, hash_node),
 	.key_offset = offsetof(struct sta_info, addr),
 	.key_len = ETH_ALEN,
-	.hashfn = sta_addr_hash,
 	.max_size = CONFIG_MAC80211_STA_HASH_MAX_SIZE,
 };
 
@@ -80,8 +78,8 @@ static const struct rhashtable_params sta_rht_params = {
 static int sta_info_hash_del(struct ieee80211_local *local,
 			     struct sta_info *sta)
 {
-	return rhashtable_remove_fast(&local->sta_hash, &sta->hash_node,
-				      sta_rht_params);
+	return rhltable_remove(&local->sta_hash, &sta->hash_node,
+			       sta_rht_params);
 }
 
 static void __cleanup_single_sta(struct sta_info *sta)
@@ -157,19 +155,22 @@ static void cleanup_single_sta(struct sta_info *sta)
 	sta_info_free(local, sta);
 }
 
+struct rhlist_head *sta_info_hash_lookup(struct ieee80211_local *local,
+					 const u8 *addr)
+{
+	return rhltable_lookup(&local->sta_hash, addr, sta_rht_params);
+}
+
 /* protected by RCU */
 struct sta_info *sta_info_get(struct ieee80211_sub_if_data *sdata,
 			      const u8 *addr)
 {
 	struct ieee80211_local *local = sdata->local;
+	struct rhlist_head *tmp;
 	struct sta_info *sta;
-	struct rhash_head *tmp;
-	const struct bucket_table *tbl;
 
 	rcu_read_lock();
-	tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
-
-	for_each_sta_info(local, tbl, addr, sta, tmp) {
+	for_each_sta_info(local, addr, sta, tmp) {
 		if (sta->sdata == sdata) {
 			rcu_read_unlock();
 			/* this is safe as the caller must already hold
@@ -190,14 +191,11 @@ struct sta_info *sta_info_get_bss(struct ieee80211_sub_if_data *sdata,
 				  const u8 *addr)
 {
 	struct ieee80211_local *local = sdata->local;
+	struct rhlist_head *tmp;
 	struct sta_info *sta;
-	struct rhash_head *tmp;
-	const struct bucket_table *tbl;
 
 	rcu_read_lock();
-	tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
-
-	for_each_sta_info(local, tbl, addr, sta, tmp) {
+	for_each_sta_info(local, addr, sta, tmp) {
 		if (sta->sdata == sdata ||
 		    (sta->sdata->bss && sta->sdata->bss == sdata->bss)) {
 			rcu_read_unlock();
@@ -263,8 +261,8 @@ void sta_info_free(struct ieee80211_local *local, struct sta_info *sta)
 static int sta_info_hash_add(struct ieee80211_local *local,
 			     struct sta_info *sta)
 {
-	return rhashtable_insert_fast(&local->sta_hash, &sta->hash_node,
-				      sta_rht_params);
+	return rhltable_insert(&local->sta_hash, &sta->hash_node,
+			       sta_rht_params);
 }
 
 static void sta_deliver_ps_frames(struct work_struct *wk)
@@ -450,9 +448,9 @@ static int sta_info_insert_check(struct sta_info *sta)
 		    is_multicast_ether_addr(sta->sta.addr)))
 		return -EINVAL;
 
-	/* Strictly speaking this isn't necessary as we hold the mutex, but
-	 * the rhashtable code can't really deal with that distinction. We
-	 * do require the mutex for correctness though.
+	/* The RCU read lock is required by rhashtable due to
+	 * asynchronous resize/rehash.  We also require the mutex
+	 * for correctness.
 	 */
 	rcu_read_lock();
 	lockdep_assert_held(&sdata->local->sta_mtx);
@@ -1040,16 +1038,11 @@ static void sta_info_cleanup(unsigned long data)
 		  round_jiffies(jiffies + STA_INFO_CLEANUP_INTERVAL));
 }
 
-u32 sta_addr_hash(const void *key, u32 length, u32 seed)
-{
-	return jhash(key, ETH_ALEN, seed);
-}
-
 int sta_info_init(struct ieee80211_local *local)
 {
 	int err;
 
-	err = rhashtable_init(&local->sta_hash, &sta_rht_params);
+	err = rhltable_init(&local->sta_hash, &sta_rht_params);
 	if (err)
 		return err;
 
@@ -1065,7 +1058,7 @@ int sta_info_init(struct ieee80211_local *local)
 void sta_info_stop(struct ieee80211_local *local)
 {
 	del_timer_sync(&local->sta_cleanup);
-	rhashtable_destroy(&local->sta_hash);
+	rhltable_destroy(&local->sta_hash);
 }
 
 
@@ -1135,17 +1128,14 @@ struct ieee80211_sta *ieee80211_find_sta_by_ifaddr(struct ieee80211_hw *hw,
 						   const u8 *localaddr)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
+	struct rhlist_head *tmp;
 	struct sta_info *sta;
-	struct rhash_head *tmp;
-	const struct bucket_table *tbl;
-
-	tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
 
 	/*
 	 * Just return a random station if localaddr is NULL
 	 * ... first in list.
 	 */
-	for_each_sta_info(local, tbl, addr, sta, tmp) {
+	for_each_sta_info(local, addr, sta, tmp) {
 		if (localaddr &&
 		    !ether_addr_equal(sta->sdata->vif.addr, localaddr))
 			continue;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 0556be3..d347ab5 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -452,7 +452,7 @@ struct sta_info {
 	/* General information, mostly static */
 	struct list_head list, free_list;
 	struct rcu_head rcu_head;
-	struct rhash_head hash_node;
+	struct rhlist_head hash_node;
 	u8 addr[ETH_ALEN];
 	struct ieee80211_local *local;
 	struct ieee80211_sub_if_data *sdata;
@@ -635,6 +635,9 @@ rcu_dereference_protected_tid_tx(struct sta_info *sta, int tid)
  */
 #define STA_INFO_CLEANUP_INTERVAL (10 * HZ)
 
+struct rhlist_head *sta_info_hash_lookup(struct ieee80211_local *local,
+					 const u8 *addr);
+
 /*
  * Get a STA info, must be under RCU read lock.
  */
@@ -644,17 +647,9 @@ struct sta_info *sta_info_get(struct ieee80211_sub_if_data *sdata,
 struct sta_info *sta_info_get_bss(struct ieee80211_sub_if_data *sdata,
 				  const u8 *addr);
 
-u32 sta_addr_hash(const void *key, u32 length, u32 seed);
-
-#define _sta_bucket_idx(_tbl, _a)					\
-	rht_bucket_index(_tbl, sta_addr_hash(_a, ETH_ALEN, (_tbl)->hash_rnd))
-
-#define for_each_sta_info(local, tbl, _addr, _sta, _tmp)		\
-	rht_for_each_entry_rcu(_sta, _tmp, tbl, 			\
-			       _sta_bucket_idx(tbl, _addr),		\
-			       hash_node)				\
-	/* compare address and run code only if it matches */		\
-	if (ether_addr_equal(_sta->addr, (_addr)))
+#define for_each_sta_info(local, _addr, _sta, _tmp)			\
+	rhl_for_each_entry_rcu(_sta, _tmp,				\
+			       sta_info_hash_lookup(local, _addr), hash_node)
 
 /*
  * Get STA info by index, BROKEN!
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index a2a6826..6361709 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -740,8 +740,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	__le16 fc;
 	struct ieee80211_supported_band *sband;
+	struct rhlist_head *tmp;
 	struct sta_info *sta;
-	struct rhash_head *tmp;
 	int retry_count;
 	int rates_idx;
 	bool send_to_cooked;
@@ -749,7 +749,6 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	struct ieee80211_bar *bar;
 	int shift = 0;
 	int tid = IEEE80211_NUM_TIDS;
-	const struct bucket_table *tbl;
 
 	rates_idx = ieee80211_tx_get_rates(hw, info, &retry_count);
 
@@ -758,9 +757,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 	sband = local->hw.wiphy->bands[info->band];
 	fc = hdr->frame_control;
 
-	tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
-
-	for_each_sta_info(local, tbl, hdr->addr1, sta, tmp) {
+	for_each_sta_info(local, hdr->addr1, sta, tmp) {
 		/* skip wrong virtual interface */
 		if (!ether_addr_equal(hdr->addr2, sta->sdata->vif.addr))
 			continue;

^ permalink raw reply related

* [PATCH 1/2] rhashtable: Add rhlist interface
From: Herbert Xu @ 2016-09-18 13:53 UTC (permalink / raw)
  To: Johannes Berg, David S. Miller, netdev, linux-wireless,
	Thomas Graf, tom, Ben Greear
In-Reply-To: <20160918135030.GA7062@gondor.apana.org.au>

The insecure_elasticity setting is an ugly wart brought out by
users who need to insert duplicate objects (that is, distinct
objects with identical keys) into the same table.

In fact, those users have a much bigger problem.  Once those
duplicate objects are inserted, they don't have an interface to
find them (unless you count the walker interface which walks
over the entire table).

Some users have resorted to doing a manual walk over the hash
table which is of course broken because they don't handle the
potential existence of multiple hash tables.  The result is that
they will break sporadically when they encounter a hash table
resize/rehash.

This patch provides a way out for those users, at the expense
of an extra pointer per object.  Essentially each object is now
a list of objects carrying the same key.  The hash table will
only see the lists so nothing changes as far as rhashtable is
concerned.

To use this new interface, you need to insert a struct rhlist_head
into your objects instead of struct rhash_head.  While the hash
table is unchanged, for type-safety you'll need to use struct
rhltable instead of struct rhashtable.  All the existing interfaces
have been duplicated for rhlist, including the hash table walker.

One missing feature is nulls marking because AFAIK the only potential
user of it does not need duplicate objects.  Should anyone need
this it shouldn't be too hard to add.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/linux/rhashtable.h |  490 ++++++++++++++++++++++++++++++++++-----------
 lib/rhashtable.c           |  231 ++++++++++++++++-----
 2 files changed, 560 insertions(+), 161 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index fd82584..dc7bea6 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -1,7 +1,7 @@
 /*
  * Resizable, Scalable, Concurrent Hash Table
  *
- * Copyright (c) 2015 Herbert Xu <herbert@gondor.apana.org.au>
+ * Copyright (c) 2015-2016 Herbert Xu <herbert@gondor.apana.org.au>
  * Copyright (c) 2014-2015 Thomas Graf <tgraf@suug.ch>
  * Copyright (c) 2008-2014 Patrick McHardy <kaber@trash.net>
  *
@@ -53,6 +53,11 @@ struct rhash_head {
 	struct rhash_head __rcu		*next;
 };
 
+struct rhlist_head {
+	struct rhash_head		rhead;
+	struct rhlist_head __rcu	*next;
+};
+
 /**
  * struct bucket_table - Table of hash buckets
  * @size: Number of hash buckets
@@ -137,6 +142,7 @@ struct rhashtable_params {
  * @key_len: Key length for hashfn
  * @elasticity: Maximum chain length before rehash
  * @p: Configuration parameters
+ * @rhlist: True if this is an rhltable
  * @run_work: Deferred worker to expand/shrink asynchronously
  * @mutex: Mutex to protect current/future table swapping
  * @lock: Spin lock to protect walker list
@@ -147,12 +153,21 @@ struct rhashtable {
 	unsigned int			key_len;
 	unsigned int			elasticity;
 	struct rhashtable_params	p;
+	bool				rhlist;
 	struct work_struct		run_work;
 	struct mutex                    mutex;
 	spinlock_t			lock;
 };
 
 /**
+ * struct rhltable - Hash table with duplicate objects in a list
+ * @ht: Underlying rhtable
+ */
+struct rhltable {
+	struct rhashtable ht;
+};
+
+/**
  * struct rhashtable_walker - Hash table walker
  * @list: List entry on list of walkers
  * @tbl: The table that we were walking over
@@ -163,9 +178,10 @@ struct rhashtable_walker {
 };
 
 /**
- * struct rhashtable_iter - Hash table iterator, fits into netlink cb
+ * struct rhashtable_iter - Hash table iterator
  * @ht: Table to iterate through
  * @p: Current pointer
+ * @list: Current hash list pointer
  * @walker: Associated rhashtable walker
  * @slot: Current slot
  * @skip: Number of entries to skip in slot
@@ -173,6 +189,7 @@ struct rhashtable_walker {
 struct rhashtable_iter {
 	struct rhashtable *ht;
 	struct rhash_head *p;
+	struct rhlist_head *list;
 	struct rhashtable_walker walker;
 	unsigned int slot;
 	unsigned int skip;
@@ -339,13 +356,11 @@ static inline int lockdep_rht_bucket_is_held(const struct bucket_table *tbl,
 
 int rhashtable_init(struct rhashtable *ht,
 		    const struct rhashtable_params *params);
+int rhltable_init(struct rhltable *hlt,
+		  const struct rhashtable_params *params);
 
-struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
-					    const void *key,
-					    struct rhash_head *obj,
-					    struct bucket_table *old_tbl,
-					    void **data);
-int rhashtable_insert_rehash(struct rhashtable *ht, struct bucket_table *tbl);
+void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
+			     struct rhash_head *obj);
 
 void rhashtable_walk_enter(struct rhashtable *ht,
 			   struct rhashtable_iter *iter);
@@ -507,6 +522,31 @@ void rhashtable_destroy(struct rhashtable *ht);
 	rht_for_each_entry_rcu_continue(tpos, pos, (tbl)->buckets[hash],\
 					tbl, hash, member)
 
+/**
+ * rhl_for_each_rcu - iterate over rcu hash table list
+ * @pos:	the &struct rlist_head to use as a loop cursor.
+ * @list:	the head of the list
+ *
+ * This hash chain list-traversal primitive should be used on the
+ * list returned by rhltable_lookup.
+ */
+#define rhl_for_each_rcu(pos, list)					\
+	for (pos = list; pos; pos = rcu_dereference_raw(pos->next))
+
+/**
+ * rhl_for_each_entry_rcu - iterate over rcu hash table list of given type
+ * @tpos:	the type * to use as a loop cursor.
+ * @pos:	the &struct rlist_head to use as a loop cursor.
+ * @list:	the head of the list
+ * @member:	name of the &struct rlist_head within the hashable struct.
+ *
+ * This hash chain list-traversal primitive should be used on the
+ * list returned by rhltable_lookup.
+ */
+#define rhl_for_each_entry_rcu(tpos, pos, list, member)			\
+	for (pos = list; pos && rht_entry(tpos, pos, member);		\
+	     pos = rcu_dereference_raw(pos->next))
+
 static inline int rhashtable_compare(struct rhashtable_compare_arg *arg,
 				     const void *obj)
 {
@@ -516,18 +556,8 @@ static inline int rhashtable_compare(struct rhashtable_compare_arg *arg,
 	return memcmp(ptr + ht->p.key_offset, arg->key, ht->p.key_len);
 }
 
-/**
- * rhashtable_lookup_fast - search hash table, inlined version
- * @ht:		hash table
- * @key:	the pointer to the key
- * @params:	hash table parameters
- *
- * Computes the hash value for the key and traverses the bucket chain looking
- * for a entry with an identical key. The first matching entry is returned.
- *
- * Returns the first entry on which the compare function returned true.
- */
-static inline void *rhashtable_lookup_fast(
+/* Internal function, do not use. */
+static inline struct rhash_head *__rhashtable_lookup(
 	struct rhashtable *ht, const void *key,
 	const struct rhashtable_params params)
 {
@@ -539,8 +569,6 @@ static inline void *rhashtable_lookup_fast(
 	struct rhash_head *he;
 	unsigned int hash;
 
-	rcu_read_lock();
-
 	tbl = rht_dereference_rcu(ht->tbl, ht);
 restart:
 	hash = rht_key_hashfn(ht, tbl, key, params);
@@ -549,8 +577,7 @@ restart:
 		    params.obj_cmpfn(&arg, rht_obj(ht, he)) :
 		    rhashtable_compare(&arg, rht_obj(ht, he)))
 			continue;
-		rcu_read_unlock();
-		return rht_obj(ht, he);
+		return he;
 	}
 
 	/* Ensure we see any new tables. */
@@ -559,96 +586,165 @@ restart:
 	tbl = rht_dereference_rcu(tbl->future_tbl, ht);
 	if (unlikely(tbl))
 		goto restart;
-	rcu_read_unlock();
 
 	return NULL;
 }
 
+/**
+ * rhashtable_lookup - search hash table
+ * @ht:		hash table
+ * @key:	the pointer to the key
+ * @params:	hash table parameters
+ *
+ * Computes the hash value for the key and traverses the bucket chain looking
+ * for a entry with an identical key. The first matching entry is returned.
+ *
+ * This must only be called under the RCU read lock.
+ *
+ * Returns the first entry on which the compare function returned true.
+ */
+static inline void *rhashtable_lookup(
+	struct rhashtable *ht, const void *key,
+	const struct rhashtable_params params)
+{
+	struct rhash_head *he = __rhashtable_lookup(ht, key, params);
+
+	return he ? rht_obj(ht, he) : NULL;
+}
+
+/**
+ * rhashtable_lookup_fast - search hash table, without RCU read lock
+ * @ht:		hash table
+ * @key:	the pointer to the key
+ * @params:	hash table parameters
+ *
+ * Computes the hash value for the key and traverses the bucket chain looking
+ * for a entry with an identical key. The first matching entry is returned.
+ *
+ * Only use this function when you have other mechanisms guaranteeing
+ * that the object won't go away after the RCU read lock is released.
+ *
+ * Returns the first entry on which the compare function returned true.
+ */
+static inline void *rhashtable_lookup_fast(
+	struct rhashtable *ht, const void *key,
+	const struct rhashtable_params params)
+{
+	void *obj;
+
+	rcu_read_lock();
+	obj = rhashtable_lookup(ht, key, params);
+	rcu_read_unlock();
+
+	return obj;
+}
+
+/**
+ * rhltable_lookup - search hash list table
+ * @hlt:	hash table
+ * @key:	the pointer to the key
+ * @params:	hash table parameters
+ *
+ * Computes the hash value for the key and traverses the bucket chain looking
+ * for a entry with an identical key.  All matching entries are returned
+ * in a list.
+ *
+ * This must only be called under the RCU read lock.
+ *
+ * Returns the list of entries that match the given key.
+ */
+static inline struct rhlist_head *rhltable_lookup(
+	struct rhltable *hlt, const void *key,
+	const struct rhashtable_params params)
+{
+	struct rhash_head *he = __rhashtable_lookup(&hlt->ht, key, params);
+
+	return he ? container_of(he, struct rhlist_head, rhead) : NULL;
+}
+
 /* Internal function, please use rhashtable_insert_fast() instead. This
  * function returns the existing element already in hashes in there is a clash,
  * otherwise it returns an error via ERR_PTR().
  */
 static inline void *__rhashtable_insert_fast(
 	struct rhashtable *ht, const void *key, struct rhash_head *obj,
-	const struct rhashtable_params params)
+	const struct rhashtable_params params, bool rhlist)
 {
 	struct rhashtable_compare_arg arg = {
 		.ht = ht,
 		.key = key,
 	};
-	struct bucket_table *tbl, *new_tbl;
+	struct rhash_head __rcu **pprev;
+	struct bucket_table *tbl;
 	struct rhash_head *head;
 	spinlock_t *lock;
-	unsigned int elasticity;
 	unsigned int hash;
-	void *data = NULL;
-	int err;
+	int elasticity;
+	void *data;
 
-restart:
 	rcu_read_lock();
 
 	tbl = rht_dereference_rcu(ht->tbl, ht);
+	hash = rht_head_hashfn(ht, tbl, obj, params);
+	lock = rht_bucket_lock(tbl, hash);
+	spin_lock_bh(lock);
 
-	/* All insertions must grab the oldest table containing
-	 * the hashed bucket that is yet to be rehashed.
-	 */
-	for (;;) {
-		hash = rht_head_hashfn(ht, tbl, obj, params);
-		lock = rht_bucket_lock(tbl, hash);
-		spin_lock_bh(lock);
-
-		if (tbl->rehash <= hash)
-			break;
-
+	if (unlikely(rht_dereference_bucket(tbl->future_tbl, tbl, hash))) {
+slow_path:
 		spin_unlock_bh(lock);
-		tbl = rht_dereference_rcu(tbl->future_tbl, ht);
+		rcu_read_unlock();
+		return rhashtable_insert_slow(ht, key, obj);
 	}
 
-	new_tbl = rht_dereference_rcu(tbl->future_tbl, ht);
-	if (unlikely(new_tbl)) {
-		tbl = rhashtable_insert_slow(ht, key, obj, new_tbl, &data);
-		if (!IS_ERR_OR_NULL(tbl))
-			goto slow_path;
+	elasticity = ht->elasticity;
+	pprev = &tbl->buckets[hash];
+	rht_for_each(head, tbl, hash) {
+		struct rhlist_head *plist;
+		struct rhlist_head *list;
+
+		elasticity--;
+		if (!key ||
+		    (params.obj_cmpfn ?
+		     params.obj_cmpfn(&arg, rht_obj(ht, head)) :
+		     rhashtable_compare(&arg, rht_obj(ht, head))))
+			continue;
+
+		data = rht_obj(ht, head);
 
-		err = PTR_ERR(tbl);
-		if (err == -EEXIST)
-			err = 0;
+		if (!rhlist)
+			goto out;
 
-		goto out;
-	}
 
-	err = -E2BIG;
-	if (unlikely(rht_grow_above_max(ht, tbl)))
-		goto out;
+		list = container_of(obj, struct rhlist_head, rhead);
+		plist = container_of(head, struct rhlist_head, rhead);
 
-	if (unlikely(rht_grow_above_100(ht, tbl))) {
-slow_path:
-		spin_unlock_bh(lock);
-		err = rhashtable_insert_rehash(ht, tbl);
-		rcu_read_unlock();
-		if (err)
-			return ERR_PTR(err);
+		RCU_INIT_POINTER(list->next, plist);
+		head = rht_dereference_bucket(head->next, tbl, hash);
+		RCU_INIT_POINTER(list->rhead.next, head);
+		rcu_assign_pointer(*pprev, obj);
 
-		goto restart;
+		goto good;
 	}
 
-	err = 0;
-	elasticity = ht->elasticity;
-	rht_for_each(head, tbl, hash) {
-		if (key &&
-		    unlikely(!(params.obj_cmpfn ?
-			       params.obj_cmpfn(&arg, rht_obj(ht, head)) :
-			       rhashtable_compare(&arg, rht_obj(ht, head))))) {
-			data = rht_obj(ht, head);
-			goto out;
-		}
-		if (!--elasticity)
-			goto slow_path;
-	}
+	if (elasticity <= 0)
+		goto slow_path;
+
+	data = ERR_PTR(-E2BIG);
+	if (unlikely(rht_grow_above_max(ht, tbl)))
+		goto out;
+
+	if (unlikely(rht_grow_above_100(ht, tbl)))
+		goto slow_path;
 
 	head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
 
 	RCU_INIT_POINTER(obj->next, head);
+	if (rhlist) {
+		struct rhlist_head *list;
+
+		list = container_of(obj, struct rhlist_head, rhead);
+		RCU_INIT_POINTER(list->next, NULL);
+	}
 
 	rcu_assign_pointer(tbl->buckets[hash], obj);
 
@@ -656,11 +752,14 @@ slow_path:
 	if (rht_grow_above_75(ht, tbl))
 		schedule_work(&ht->run_work);
 
+good:
+	data = NULL;
+
 out:
 	spin_unlock_bh(lock);
 	rcu_read_unlock();
 
-	return err ? ERR_PTR(err) : data;
+	return data;
 }
 
 /**
@@ -685,7 +784,7 @@ static inline int rhashtable_insert_fast(
 {
 	void *ret;
 
-	ret = __rhashtable_insert_fast(ht, NULL, obj, params);
+	ret = __rhashtable_insert_fast(ht, NULL, obj, params, false);
 	if (IS_ERR(ret))
 		return PTR_ERR(ret);
 
@@ -693,6 +792,58 @@ static inline int rhashtable_insert_fast(
 }
 
 /**
+ * rhltable_insert_key - insert object into hash list table
+ * @hlt:	hash list table
+ * @key:	the pointer to the key
+ * @list:	pointer to hash list head inside object
+ * @params:	hash table parameters
+ *
+ * Will take a per bucket spinlock to protect against mutual mutations
+ * on the same bucket. Multiple insertions may occur in parallel unless
+ * they map to the same bucket lock.
+ *
+ * It is safe to call this function from atomic context.
+ *
+ * Will trigger an automatic deferred table resizing if the size grows
+ * beyond the watermark indicated by grow_decision() which can be passed
+ * to rhashtable_init().
+ */
+static inline int rhltable_insert_key(
+	struct rhltable *hlt, const void *key, struct rhlist_head *list,
+	const struct rhashtable_params params)
+{
+	return PTR_ERR(__rhashtable_insert_fast(&hlt->ht, key, &list->rhead,
+						params, true));
+}
+
+/**
+ * rhltable_insert - insert object into hash list table
+ * @hlt:	hash list table
+ * @list:	pointer to hash list head inside object
+ * @params:	hash table parameters
+ *
+ * Will take a per bucket spinlock to protect against mutual mutations
+ * on the same bucket. Multiple insertions may occur in parallel unless
+ * they map to the same bucket lock.
+ *
+ * It is safe to call this function from atomic context.
+ *
+ * Will trigger an automatic deferred table resizing if the size grows
+ * beyond the watermark indicated by grow_decision() which can be passed
+ * to rhashtable_init().
+ */
+static inline int rhltable_insert(
+	struct rhltable *hlt, struct rhlist_head *list,
+	const struct rhashtable_params params)
+{
+	const char *key = rht_obj(&hlt->ht, &list->rhead);
+
+	key += params.key_offset;
+
+	return rhltable_insert_key(hlt, key, list, params);
+}
+
+/**
  * rhashtable_lookup_insert_fast - lookup and insert object into hash table
  * @ht:		hash table
  * @obj:	pointer to hash head inside object
@@ -722,7 +873,8 @@ static inline int rhashtable_lookup_insert_fast(
 
 	BUG_ON(ht->p.obj_hashfn);
 
-	ret = __rhashtable_insert_fast(ht, key + ht->p.key_offset, obj, params);
+	ret = __rhashtable_insert_fast(ht, key + ht->p.key_offset, obj, params,
+				       false);
 	if (IS_ERR(ret))
 		return PTR_ERR(ret);
 
@@ -759,7 +911,7 @@ static inline int rhashtable_lookup_insert_key(
 
 	BUG_ON(!ht->p.obj_hashfn || !key);
 
-	ret = __rhashtable_insert_fast(ht, key, obj, params);
+	ret = __rhashtable_insert_fast(ht, key, obj, params, false);
 	if (IS_ERR(ret))
 		return PTR_ERR(ret);
 
@@ -783,13 +935,14 @@ static inline void *rhashtable_lookup_get_insert_key(
 {
 	BUG_ON(!ht->p.obj_hashfn || !key);
 
-	return __rhashtable_insert_fast(ht, key, obj, params);
+	return __rhashtable_insert_fast(ht, key, obj, params, false);
 }
 
 /* Internal function, please use rhashtable_remove_fast() instead */
-static inline int __rhashtable_remove_fast(
+static inline int __rhashtable_remove_fast_one(
 	struct rhashtable *ht, struct bucket_table *tbl,
-	struct rhash_head *obj, const struct rhashtable_params params)
+	struct rhash_head *obj, const struct rhashtable_params params,
+	bool rhlist)
 {
 	struct rhash_head __rcu **pprev;
 	struct rhash_head *he;
@@ -804,39 +957,65 @@ static inline int __rhashtable_remove_fast(
 
 	pprev = &tbl->buckets[hash];
 	rht_for_each(he, tbl, hash) {
+		struct rhlist_head *list;
+
+		list = container_of(he, struct rhlist_head, rhead);
+
 		if (he != obj) {
+			struct rhlist_head __rcu **lpprev;
+
 			pprev = &he->next;
-			continue;
+
+			if (!rhlist)
+				continue;
+
+			do {
+				lpprev = &list->next;
+				list = rht_dereference_bucket(list->next,
+							      tbl, hash);
+			} while (list && obj != &list->rhead);
+
+			if (!list)
+				continue;
+
+			list = rht_dereference_bucket(list->next, tbl, hash);
+			RCU_INIT_POINTER(*lpprev, list);
+			err = 0;
+			break;
 		}
 
-		rcu_assign_pointer(*pprev, obj->next);
-		err = 0;
+		obj = rht_dereference_bucket(obj->next, tbl, hash);
+
+		if (rhlist) {
+			list = rht_dereference_bucket(list->next, tbl, hash);
+			if (list) {
+				RCU_INIT_POINTER(list->rhead.next, obj);
+				obj = &list->rhead;
+			}
+		}
+
+		rcu_assign_pointer(*pprev, obj);
+		err = 1;
 		break;
 	}
 
 	spin_unlock_bh(lock);
 
+	if (err > 0) {
+		atomic_dec(&ht->nelems);
+		if (unlikely(ht->p.automatic_shrinking &&
+			     rht_shrink_below_30(ht, tbl)))
+			schedule_work(&ht->run_work);
+		err = 0;
+	}
+
 	return err;
 }
 
-/**
- * rhashtable_remove_fast - remove object from hash table
- * @ht:		hash table
- * @obj:	pointer to hash head inside object
- * @params:	hash table parameters
- *
- * Since the hash chain is single linked, the removal operation needs to
- * walk the bucket chain upon removal. The removal operation is thus
- * considerable slow if the hash table is not correctly sized.
- *
- * Will automatically shrink the table via rhashtable_expand() if the
- * shrink_decision function specified at rhashtable_init() returns true.
- *
- * Returns zero on success, -ENOENT if the entry could not be found.
- */
-static inline int rhashtable_remove_fast(
+/* Internal function, please use rhashtable_remove_fast() instead */
+static inline int __rhashtable_remove_fast(
 	struct rhashtable *ht, struct rhash_head *obj,
-	const struct rhashtable_params params)
+	const struct rhashtable_params params, bool rhlist)
 {
 	struct bucket_table *tbl;
 	int err;
@@ -850,24 +1029,60 @@ static inline int rhashtable_remove_fast(
 	 * visible then that guarantees the entry to still be in
 	 * the old tbl if it exists.
 	 */
-	while ((err = __rhashtable_remove_fast(ht, tbl, obj, params)) &&
+	while ((err = __rhashtable_remove_fast_one(ht, tbl, obj, params,
+						   rhlist)) &&
 	       (tbl = rht_dereference_rcu(tbl->future_tbl, ht)))
 		;
 
-	if (err)
-		goto out;
-
-	atomic_dec(&ht->nelems);
-	if (unlikely(ht->p.automatic_shrinking &&
-		     rht_shrink_below_30(ht, tbl)))
-		schedule_work(&ht->run_work);
-
-out:
 	rcu_read_unlock();
 
 	return err;
 }
 
+/**
+ * rhashtable_remove_fast - remove object from hash table
+ * @ht:		hash table
+ * @obj:	pointer to hash head inside object
+ * @params:	hash table parameters
+ *
+ * Since the hash chain is single linked, the removal operation needs to
+ * walk the bucket chain upon removal. The removal operation is thus
+ * considerable slow if the hash table is not correctly sized.
+ *
+ * Will automatically shrink the table via rhashtable_expand() if the
+ * shrink_decision function specified at rhashtable_init() returns true.
+ *
+ * Returns zero on success, -ENOENT if the entry could not be found.
+ */
+static inline int rhashtable_remove_fast(
+	struct rhashtable *ht, struct rhash_head *obj,
+	const struct rhashtable_params params)
+{
+	return __rhashtable_remove_fast(ht, obj, params, false);
+}
+
+/**
+ * rhltable_remove - remove object from hash list table
+ * @hlt:	hash list table
+ * @list:	pointer to hash list head inside object
+ * @params:	hash table parameters
+ *
+ * Since the hash chain is single linked, the removal operation needs to
+ * walk the bucket chain upon removal. The removal operation is thus
+ * considerable slow if the hash table is not correctly sized.
+ *
+ * Will automatically shrink the table via rhashtable_expand() if the
+ * shrink_decision function specified at rhashtable_init() returns true.
+ *
+ * Returns zero on success, -ENOENT if the entry could not be found.
+ */
+static inline int rhltable_remove(
+	struct rhltable *hlt, struct rhlist_head *list,
+	const struct rhashtable_params params)
+{
+	return __rhashtable_remove_fast(&hlt->ht, &list->rhead, params, true);
+}
+
 /* Internal function, please use rhashtable_replace_fast() instead */
 static inline int __rhashtable_replace_fast(
 	struct rhashtable *ht, struct bucket_table *tbl,
@@ -958,4 +1173,51 @@ static inline int rhashtable_walk_init(struct rhashtable *ht,
 	return 0;
 }
 
+/**
+ * rhltable_walk_enter - Initialise an iterator
+ * @hlt:	Table to walk over
+ * @iter:	Hash table Iterator
+ *
+ * This function prepares a hash table walk.
+ *
+ * Note that if you restart a walk after rhashtable_walk_stop you
+ * may see the same object twice.  Also, you may miss objects if
+ * there are removals in between rhashtable_walk_stop and the next
+ * call to rhashtable_walk_start.
+ *
+ * For a completely stable walk you should construct your own data
+ * structure outside the hash table.
+ *
+ * This function may sleep so you must not call it from interrupt
+ * context or with spin locks held.
+ *
+ * You must call rhashtable_walk_exit after this function returns.
+ */
+static inline void rhltable_walk_enter(struct rhltable *hlt,
+				       struct rhashtable_iter *iter)
+{
+	return rhashtable_walk_enter(&hlt->ht, iter);
+}
+
+/**
+ * rhltable_free_and_destroy - free elements and destroy hash list table
+ * @hlt:	the hash list table to destroy
+ * @free_fn:	callback to release resources of element
+ * @arg:	pointer passed to free_fn
+ *
+ * See documentation for rhashtable_free_and_destroy.
+ */
+static inline void rhltable_free_and_destroy(struct rhltable *hlt,
+					     void (*free_fn)(void *ptr,
+							     void *arg),
+					     void *arg)
+{
+	return rhashtable_free_and_destroy(&hlt->ht, free_fn, arg);
+}
+
+static inline void rhltable_destroy(struct rhltable *hlt)
+{
+	return rhltable_free_and_destroy(hlt, NULL, NULL);
+}
+
 #endif /* _LINUX_RHASHTABLE_H */
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 06c2872..185fc14 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -378,22 +378,8 @@ static void rht_deferred_worker(struct work_struct *work)
 		schedule_work(&ht->run_work);
 }
 
-static bool rhashtable_check_elasticity(struct rhashtable *ht,
-					struct bucket_table *tbl,
-					unsigned int hash)
-{
-	unsigned int elasticity = ht->elasticity;
-	struct rhash_head *head;
-
-	rht_for_each(head, tbl, hash)
-		if (!--elasticity)
-			return true;
-
-	return false;
-}
-
-int rhashtable_insert_rehash(struct rhashtable *ht,
-			     struct bucket_table *tbl)
+static int rhashtable_insert_rehash(struct rhashtable *ht,
+				    struct bucket_table *tbl)
 {
 	struct bucket_table *old_tbl;
 	struct bucket_table *new_tbl;
@@ -439,57 +425,148 @@ fail:
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(rhashtable_insert_rehash);
 
-struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
-					    const void *key,
-					    struct rhash_head *obj,
-					    struct bucket_table *tbl,
-					    void **data)
+static void *rhashtable_insert_one(struct rhashtable *ht,
+				   struct bucket_table *tbl, unsigned int hash,
+				   const void *key, struct rhash_head *obj)
 {
+	struct rhashtable_compare_arg arg = {
+		.ht = ht,
+		.key = key,
+	};
+	struct rhash_head __rcu **pprev;
+	struct rhash_head *head;
+	int elasticity;
+
+	elasticity = ht->elasticity;
+	pprev = &tbl->buckets[hash];
+	rht_for_each(head, tbl, hash) {
+		struct rhlist_head *list;
+		struct rhlist_head *plist;
+
+		elasticity--;
+		if (!key ||
+		    (ht->p.obj_cmpfn ?
+		     ht->p.obj_cmpfn(&arg, rht_obj(ht, head)) :
+		     rhashtable_compare(&arg, rht_obj(ht, head))))
+			continue;
+
+		if (!ht->rhlist)
+			return rht_obj(ht, head);
+
+		list = container_of(obj, struct rhlist_head, rhead);
+		plist = container_of(head, struct rhlist_head, rhead);
+
+		RCU_INIT_POINTER(list->next, plist);
+		head = rht_dereference_bucket(head->next, tbl, hash);
+		RCU_INIT_POINTER(list->rhead.next, head);
+		rcu_assign_pointer(*pprev, obj);
+
+		return NULL;
+	}
+
+	if (elasticity <= 0)
+		return ERR_PTR(-EAGAIN);
+
+	return ERR_PTR(-ENOENT);
+}
+
+static void *rhashtable_try_insert(struct rhashtable *ht, const void *key,
+				   struct rhash_head *obj)
+{
+	struct bucket_table *tbl;
 	struct rhash_head *head;
 	unsigned int hash;
-	int err;
+	spinlock_t *lock;
+	void *data;
 
-	tbl = rhashtable_last_table(ht, tbl);
-	hash = head_hashfn(ht, tbl, obj);
-	spin_lock_nested(rht_bucket_lock(tbl, hash), SINGLE_DEPTH_NESTING);
-
-	err = -EEXIST;
-	if (key) {
-		*data = rhashtable_lookup_fast(ht, key, ht->p);
-		if (*data)
-			goto exit;
+	tbl = rht_dereference_rcu(ht->tbl, ht);
+
+	/* All insertions must grab the oldest table containing
+	 * the hashed bucket that is yet to be rehashed.
+	 */
+	for (;;) {
+		hash = rht_head_hashfn(ht, tbl, obj, ht->p);
+		lock = rht_bucket_lock(tbl, hash);
+		spin_lock_bh(lock);
+
+		if (tbl->rehash <= hash)
+			break;
+
+		spin_unlock_bh(lock);
+		tbl = rht_dereference_rcu(tbl->future_tbl, ht);
+	}
+
+	data = rhashtable_insert_one(ht, tbl, hash, key, obj);
+
+	while (PTR_ERR(data) == -ENOENT || PTR_ERR(data) == -EAGAIN) {
+		struct bucket_table *new_tbl;
+
+		new_tbl = rht_dereference_rcu(tbl->future_tbl, ht);
+		if (!new_tbl)
+			break;
+
+		tbl = new_tbl;
+		hash = rht_head_hashfn(ht, tbl, obj, ht->p);
+		spin_lock_nested(rht_bucket_lock(tbl, hash),
+				 SINGLE_DEPTH_NESTING);
+
+		data = rhashtable_insert_one(ht, tbl, hash, key, obj);
+
+		spin_unlock(rht_bucket_lock(tbl, hash));
 	}
 
-	err = -E2BIG;
+	if (PTR_ERR(data) != -ENOENT)
+		goto exit;
+
+	data = ERR_PTR(-E2BIG);
 	if (unlikely(rht_grow_above_max(ht, tbl)))
 		goto exit;
 
-	err = -EAGAIN;
-	if (rhashtable_check_elasticity(ht, tbl, hash) ||
-	    rht_grow_above_100(ht, tbl))
+	data = ERR_PTR(-EAGAIN);
+	if (unlikely(rht_grow_above_100(ht, tbl)))
 		goto exit;
 
-	err = 0;
+	data = NULL;
 
 	head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
 
 	RCU_INIT_POINTER(obj->next, head);
+	if (ht->rhlist) {
+		struct rhlist_head *list;
+
+		list = container_of(obj, struct rhlist_head, rhead);
+		RCU_INIT_POINTER(list->next, NULL);
+	}
 
 	rcu_assign_pointer(tbl->buckets[hash], obj);
 
 	atomic_inc(&ht->nelems);
+	if (rht_grow_above_75(ht, tbl))
+		schedule_work(&ht->run_work);
 
 exit:
-	spin_unlock(rht_bucket_lock(tbl, hash));
+	spin_unlock_bh(lock);
 
-	if (err == 0)
-		return NULL;
-	else if (err == -EAGAIN)
-		return tbl;
-	else
-		return ERR_PTR(err);
+	if (PTR_ERR(data) == -EAGAIN)
+		data = ERR_PTR(rhashtable_insert_rehash(ht, tbl) ?:
+			       -EAGAIN);
+
+	return data;
+}
+
+void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
+			     struct rhash_head *obj)
+{
+	void *data;
+
+	do {
+		rcu_read_lock();
+		data = rhashtable_try_insert(ht, key, obj);
+		rcu_read_unlock();
+	} while (PTR_ERR(data) == -EAGAIN);
+
+	return data;
 }
 EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
 
@@ -593,11 +670,16 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_start);
 void *rhashtable_walk_next(struct rhashtable_iter *iter)
 {
 	struct bucket_table *tbl = iter->walker.tbl;
+	struct rhlist_head *list = iter->list;
 	struct rhashtable *ht = iter->ht;
 	struct rhash_head *p = iter->p;
+	bool rhlist = ht->rhlist;
 
 	if (p) {
-		p = rht_dereference_bucket_rcu(p->next, tbl, iter->slot);
+		if (!rhlist || !(list = rcu_dereference(list->next))) {
+			p = rcu_dereference(p->next);
+			list = container_of(p, struct rhlist_head, rhead);
+		}
 		goto next;
 	}
 
@@ -605,6 +687,18 @@ void *rhashtable_walk_next(struct rhashtable_iter *iter)
 		int skip = iter->skip;
 
 		rht_for_each_rcu(p, tbl, iter->slot) {
+			if (rhlist) {
+				list = container_of(p, struct rhlist_head,
+						    rhead);
+				do {
+					if (!skip)
+						goto next;
+					skip--;
+					list = rcu_dereference(list->next);
+				} while (list);
+
+				continue;
+			}
 			if (!skip)
 				break;
 			skip--;
@@ -614,7 +708,8 @@ next:
 		if (!rht_is_a_nulls(p)) {
 			iter->skip++;
 			iter->p = p;
-			return rht_obj(ht, p);
+			iter->list = list;
+			return rht_obj(ht, rhlist ? &list->rhead : p);
 		}
 
 		iter->skip = 0;
@@ -803,6 +898,48 @@ int rhashtable_init(struct rhashtable *ht,
 EXPORT_SYMBOL_GPL(rhashtable_init);
 
 /**
+ * rhltable_init - initialize a new hash list table
+ * @hlt:	hash list table to be initialized
+ * @params:	configuration parameters
+ *
+ * Initializes a new hash list table.
+ *
+ * See documentation for rhashtable_init.
+ */
+int rhltable_init(struct rhltable *hlt, const struct rhashtable_params *params)
+{
+	int err;
+
+	/* No rhlist NULLs marking for now. */
+	if (params->nulls_base)
+		return -EINVAL;
+
+	err = rhashtable_init(&hlt->ht, params);
+	hlt->ht.rhlist = true;
+	return err;
+}
+EXPORT_SYMBOL_GPL(rhltable_init);
+
+static void rhashtable_free_one(struct rhashtable *ht, struct rhash_head *obj,
+				void (*free_fn)(void *ptr, void *arg),
+				void *arg)
+{
+	struct rhlist_head *list;
+
+	if (!ht->rhlist) {
+		free_fn(rht_obj(ht, obj), arg);
+		return;
+	}
+
+	list = container_of(obj, struct rhlist_head, rhead);
+	do {
+		obj = &list->rhead;
+		list = rht_dereference(list->next, ht);
+		free_fn(rht_obj(ht, obj), arg);
+	} while (list);
+}
+
+/**
  * rhashtable_free_and_destroy - free elements and destroy hash table
  * @ht:		the hash table to destroy
  * @free_fn:	callback to release resources of element
@@ -839,7 +976,7 @@ void rhashtable_free_and_destroy(struct rhashtable *ht,
 			     pos = next,
 			     next = !rht_is_a_nulls(pos) ?
 					rht_dereference(pos->next, ht) : NULL)
-				free_fn(rht_obj(ht, pos), arg);
+				rhashtable_free_one(ht, pos, free_fn, arg);
 		}
 	}
 

^ permalink raw reply related

* [PATCH 0/2] rhashtable: rhashtable with duplicate objects
From: Herbert Xu @ 2016-09-18 13:50 UTC (permalink / raw)
  To: Johannes Berg; +Cc: David S. Miller, netdev, linux-wireless, Thomas Graf, tom
In-Reply-To: <1470394233.2977.37.camel@sipsolutions.net>

On Fri, Aug 05, 2016 at 12:50:33PM +0200, Johannes Berg wrote:
> > My plan is to build support for this directly into rhashtable.
> > So I'm adding a struct rhlist_head that would be used in place
> > of rhash_head for these cases and it'll carry an extra pointer
> > for the list of identical entries.
> > 
> > I will then add an additional layer of insert/lookup interfaces
> > for rhlist_head.
> 
> Oh, ok.

OK, it's finally ready now.

This series contains one two patches.  The first adds the rhlist
interface and the second converts mac80211 to use it.  If this works
out I'll then proceed to convert the other insecure_elasticity
users over to this.

I've tested the rhlist code with test_rhashtable but I haven't
tested the mac80211 conversion.  So please give it a go and see
if it still works.

Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Christian Lamparter @ 2016-09-18 12:49 UTC (permalink / raw)
  To: Greg KH
  Cc: Kalle Valo, Christian Lamparter, linux-kernel, linux-wireless,
	b43-dev, Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <20160918101455.GA8410@kroah.com>

On Sunday, September 18, 2016 12:14:55 PM CEST Greg KH wrote:
> On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
> > Greg KH <gregkh@linuxfoundation.org> writes:
> > 
> > > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> > >> Ben Greear reported:
> > >> > I see lots of instability as soon as I load up the carl9710 NIC.
> > >> > My application is going to be poking at it's debugfs files...
> > >> >
> > >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > >> > [carl9170] at addr ffff8801bc1208b0
> > >> > Read of size 8 by task btserver/5888
> > >> > =======================================================================
> > >> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
> > >> > -----------------------------------------------------------------------
> > >> >
> > >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> > >> >...
> > >> 
> > >> This breakage was caused by the introduction of intermediate
> > >> fops in debugfs by commit 9fd4dcece43a
> > >> ("debugfs: prevent access to possibly dead file_operations at file open")
> > >
> > > Because of this, these should all be backported to 4.7-stable, and
> > > 4.8-stable, right?
Ok, only b43legacy has debugfs enabled by default. For b43 and carl9170 
debugfs support is usually disabled.

Greg, would you take these four patches "as is" for -stable
or do you want a "minimal version" which just replaces the

dfops = container_of(file->f_op, ...

with

dfops = container_of(file->f_path.dentry->d_fsdata, ...

in the three drivers for -stable?

> > Via which tree should these go, Greg's or mine?
> 
> I'll take it if you ack it, as it's a debugfs issue.
For carl9170: Ben Greear has reported:
"I have verified this fixes my problem in the 4.7 kernel."

But this was with a preliminary/minimal version so I didn't
add the tested-by tag.

As for b43, I'll see if I have a working b43 in my collection
somewhere to confirm the issue and the fix. Question is, do
you want to wait or not?

Regards,
Christian

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Greg KH @ 2016-09-18 10:14 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Christian Lamparter, Christian Lamparter, linux-kernel,
	linux-wireless, b43-dev, Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <87d1k17idx.fsf@kamboji.qca.qualcomm.com>

On Sun, Sep 18, 2016 at 10:54:18AM +0300, Kalle Valo wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
> 
> > On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> >> Ben Greear reported:
> >> > I see lots of instability as soon as I load up the carl9710 NIC.
> >> > My application is going to be poking at it's debugfs files...
> >> >
> >> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> >> > [carl9170] at addr ffff8801bc1208b0
> >> > Read of size 8 by task btserver/5888
> >> > =======================================================================
> >> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
> >> > -----------------------------------------------------------------------
> >> >
> >> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> >> >...
> >> 
> >> This breakage was caused by the introduction of intermediate
> >> fops in debugfs by commit 9fd4dcece43a
> >> ("debugfs: prevent access to possibly dead file_operations at file open")
> >
> > Because of this, these should all be backported to 4.7-stable, and
> > 4.8-stable, right?
> 
> Via which tree should these go, Greg's or mine?

I'll take it if you ack it, as it's a debugfs issue.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Kalle Valo @ 2016-09-18  8:02 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Joe Perches, Larry Finger, devel, linux-wireless
In-Reply-To: <wrfjfuoymeez.fsf@redhat.com>

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> Joe Perches <joe@perches.com> writes:
>> On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
>>> As soon as debugging is turned on, the logs are filled with messages
>>> reporting the interrupt status. As this quantity is usually zero, this
>>> output is not needed. In fact, there will be a report if the status is
>>> not zero, thus the debug line in question could probably be deleted.
>>> Rather than taking that action, I have changed it to only be printed
>>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>>
>> There are many uses of
>> 	if (rtl8xxxu_debug & <DEFINE>) {
>> 		dev_info(dev, ...)
>>
>> Emitting debugging information at KERN_INFO is odd.
>
> Not at all, it's a pain to enable it in debug fs post loading the
> driver, especially if you need the output immediately during driver
> init. That is why the flags are there.
>
>> I think it'd be nicer to use dev_dbg for all these cases
>> and as well use some new macro that includes the test
>>
>> Something like:
>>
>> #define rtl8xxxu_dbg(type, fmt, ...)			\
>> do {							\
>> 	if (rtl8xxxu_debug & (type))			\
>> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
>> } while (0)
>
> Yuck yuck yuck, no thanks!
>
> Any attempt of adding that kinda grossness to the driver will get a
> NACK.

Huh, how is that ugly? To me it's the opposite, original code is ugly
and Joes' proposal makes sense. Lots of wireless drivers have something
similar.

-- 
Kalle Valo

^ permalink raw reply

* RE: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop NAN
From: Otcheretianski, Andrei @ 2016-09-18  7:59 UTC (permalink / raw)
  To: Arend Van Spriel, Luca Coelho, johannes@sipsolutions.net
  Cc: linux-wireless@vger.kernel.org, Beker, Ayala, Grumbach, Emmanuel,
	Coelho, Luciano
In-Reply-To: <fae482d0-4bcd-6b06-9846-298a3218ab26@broadcom.com>

> -----Original Message-----
> From: Arend Van Spriel [mailto:arend.vanspriel@broadcom.com]
> Sent: Friday, September 16, 2016 14:09
> To: Luca Coelho <luca@coelho.fi>; johannes@sipsolutions.net
> Cc: linux-wireless@vger.kernel.org; Beker, Ayala <ayala.beker@intel.com>;
> Otcheretianski, Andrei <andrei.otcheretianski@intel.com>; Grumbach,
> Emmanuel <emmanuel.grumbach@intel.com>; Coelho, Luciano
> <luciano.coelho@intel.com>
> Subject: Re: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop
> NAN
> 
> On 16-9-2016 10:33, Luca Coelho wrote:
> > From: Ayala Beker <ayala.beker@intel.com>
> >
> > This code doesn't do much besides allowing to start and stop the vif.
> >
> > Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > Signed-off-by: Ayala Beker <ayala.beker@intel.com>
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > ---
> >  include/net/mac80211.h    |  9 +++++++++
> >  net/mac80211/cfg.c        | 36 ++++++++++++++++++++++++++++++++++
> >  net/mac80211/chan.c       |  3 +++
> >  net/mac80211/driver-ops.h | 29 ++++++++++++++++++++++++++-
> >  net/mac80211/iface.c      |  8 ++++++--
> >  net/mac80211/main.c       |  5 +++++
> >  net/mac80211/offchannel.c |  3 ++-
> >  net/mac80211/trace.h      | 50
> +++++++++++++++++++++++++++++++++++++++++++++++
> >  net/mac80211/util.c       |  3 ++-
> >  9 files changed, 141 insertions(+), 5 deletions(-)
> 
> [...]
> 
> > diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
> > index fe35a1c..67b42c8 100644
> > --- a/net/mac80211/driver-ops.h
> > +++ b/net/mac80211/driver-ops.h
> > @@ -163,7 +163,8 @@ static inline void drv_bss_info_changed(struct
> > ieee80211_local *local,
> >
> >  	if (WARN_ON_ONCE(sdata->vif.type ==
> NL80211_IFTYPE_P2P_DEVICE ||
> >  			 (sdata->vif.type == NL80211_IFTYPE_MONITOR &&
> > -			  !sdata->vif.mu_mimo_owner)))
> > +			  !sdata->vif.mu_mimo_owner) ||
> > +			 sdata->vif.type == NL80211_IFTYPE_NAN))
> 
> Might be more clear to move this up right after P2P_DEVICE check.

Why? It's a completely separate new condition - so it goes to the end.

> 
> >  		return;
> >
> >  	if (!check_sdata_in_driver(sdata))
> > @@ -1165,4 +1166,30 @@ static inline void drv_wake_tx_queue(struct
> ieee80211_local *local,
> >  	local->ops->wake_tx_queue(&local->hw, &txq->txq);  }
> >
> > +static inline int drv_start_nan(struct ieee80211_local *local,
> > +				struct ieee80211_sub_if_data *sdata,
> > +				struct cfg80211_nan_conf *conf)
> > +{
> > +	int ret;
> > +
> > +	might_sleep();
> > +	check_sdata_in_driver(sdata);
> > +
> > +	trace_drv_start_nan(local, sdata, conf);
> > +	ret = local->ops->start_nan(&local->hw, &sdata->vif, conf);
> > +	trace_drv_return_int(local, ret);
> > +	return ret;
> > +}
> > +
> > +static inline void drv_stop_nan(struct ieee80211_local *local,
> > +				struct ieee80211_sub_if_data *sdata) {
> > +	might_sleep();
> > +	check_sdata_in_driver(sdata);
> > +
> > +	trace_drv_stop_nan(local, sdata);
> > +	local->ops->stop_nan(&local->hw, &sdata->vif);
> > +	trace_drv_return_void(local);
> > +}
> > +
> >  #endif /* __MAC80211_DRIVER_OPS */
> > diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index
> > e694ca2..507f46a 100644
> > --- a/net/mac80211/iface.c
> > +++ b/net/mac80211/iface.c
> > @@ -327,6 +327,9 @@ static int ieee80211_check_queues(struct
> ieee80211_sub_if_data *sdata,
> >  	int n_queues = sdata->local->hw.queues;
> >  	int i;
> >
> > +	if (iftype == NL80211_IFTYPE_NAN)
> > +		return 0;
> > +
> >  	if (iftype != NL80211_IFTYPE_P2P_DEVICE) {
> >  		for (i = 0; i < IEEE80211_NUM_ACS; i++) {
> >  			if (WARN_ON_ONCE(sdata->vif.hw_queue[i] == @@
> -647,7 +650,8 @@ int
> > ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
> >  			local->fif_probe_req++;
> >  		}
> >
> > -		if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE)
> > +		if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE &&
> > +		    sdata->vif.type != NL80211_IFTYPE_NAN)
> 
> similar check keeps reoccuring in various places so maybe we can create a
> helper function for it.

Right, but not sure that it deserves a function.

> 
> >  			changed |= ieee80211_reset_erp_info(sdata);
> >  		ieee80211_bss_info_change_notify(sdata, changed);
> >
> 
> Regards,
> Arend

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Kalle Valo @ 2016-09-18  7:54 UTC (permalink / raw)
  To: Greg KH
  Cc: Christian Lamparter, Christian Lamparter, linux-kernel,
	linux-wireless, b43-dev, Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <20160917214539.GA28379@kroah.com>

Greg KH <gregkh@linuxfoundation.org> writes:

> On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
>> Ben Greear reported:
>> > I see lots of instability as soon as I load up the carl9710 NIC.
>> > My application is going to be poking at it's debugfs files...
>> >
>> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
>> > [carl9170] at addr ffff8801bc1208b0
>> > Read of size 8 by task btserver/5888
>> > =======================================================================
>> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
>> > -----------------------------------------------------------------------
>> >
>> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
>> >...
>> 
>> This breakage was caused by the introduction of intermediate
>> fops in debugfs by commit 9fd4dcece43a
>> ("debugfs: prevent access to possibly dead file_operations at file open")
>
> Because of this, these should all be backported to 4.7-stable, and
> 4.8-stable, right?

Via which tree should these go, Greg's or mine?

-- 
Kalle Valo

^ permalink raw reply

* RE: [PATCH v2 1/9] cfg80211: add start / stop NAN commands
From: Otcheretianski, Andrei @ 2016-09-18  7:44 UTC (permalink / raw)
  To: Arend Van Spriel, Luca Coelho, johannes@sipsolutions.net
  Cc: linux-wireless@vger.kernel.org, Beker, Ayala, Grumbach, Emmanuel,
	Coelho, Luciano
In-Reply-To: <6bfd6007-8650-3c75-2e90-c2c94202e2b9@broadcom.com>

> -----Original Message-----
> From: Arend Van Spriel [mailto:arend.vanspriel@broadcom.com]
> Sent: Friday, September 16, 2016 13:59
> To: Luca Coelho <luca@coelho.fi>; johannes@sipsolutions.net
> Cc: linux-wireless@vger.kernel.org; Beker, Ayala <ayala.beker@intel.com>;
> Otcheretianski, Andrei <andrei.otcheretianski@intel.com>; Grumbach,
> Emmanuel <emmanuel.grumbach@intel.com>; Coelho, Luciano
> <luciano.coelho@intel.com>
> Subject: Re: [PATCH v2 1/9] cfg80211: add start / stop NAN commands
> 
> On 16-9-2016 10:33, Luca Coelho wrote:
> > From: Ayala Beker <ayala.beker@intel.com>
> >
> > This allows user space to start/stop NAN interface.
> > A NAN interface is like P2P device in a few aspects: it doesn't have a
> > netdev associated to it.
> > Add the new interface type and prevent operations that can't be
> > executed on NAN interface like scan.
> >
> > Define several attributes that may be configured by user space when
> > starting NAN functionality (master preference and dual band operation)
> >
> > Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > ---
> >  include/net/cfg80211.h       | 21 +++++++++-
> >  include/uapi/linux/nl80211.h | 52 +++++++++++++++++++++++++
> >  net/mac80211/cfg.c           |  2 +
> >  net/mac80211/chan.c          |  3 ++
> >  net/mac80211/iface.c         |  4 ++
> >  net/mac80211/offchannel.c    |  1 +
> >  net/mac80211/rx.c            |  3 ++
> >  net/mac80211/util.c          |  1 +
> >  net/wireless/chan.c          |  2 +
> >  net/wireless/core.c          | 34 ++++++++++++++++
> >  net/wireless/core.h          |  3 ++
> >  net/wireless/mlme.c          |  1 +
> >  net/wireless/nl80211.c       | 93
> ++++++++++++++++++++++++++++++++++++++++++--
> >  net/wireless/rdev-ops.h      | 20 ++++++++++
> >  net/wireless/trace.h         | 27 +++++++++++++
> >  net/wireless/util.c          |  6 ++-
> >  16 files changed, 267 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index
> > d5e7f69..ca64d69 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -2293,6 +2293,19 @@ struct cfg80211_qos_map {  };
> 
> [...]
> 
> > +/**
> > + * enum nl80211_nan_dual_band_conf - NAN dual band configuration
> > + *
> > + * Defines the NAN dual band mode of operation
> 
> Does it make sense to have such a notion of bands in use. And what does
> 5.2GHz mean. Is this a subband within 5G channels? Probably I should read
> the NAN spec to understand what is meant here. I would consider 5.2G as
> lower 5G, ie. discovery on channel 44 but not sure if that is meant here.
>

The NAN spec defines single and dual band modes of operation. The channels are fixed for each band.
On 2.4Ghz is channel 6 and 5GHz is either 44 or 149. Regarding 5.2 - it's just a typo. It should be 5G - no deeper meaning.
 
> > + * @NL80211_NAN_BAND_DEFAULT: device default mode
> > + * @NL80211_NAN_BAND_SINGLE: 2.4GHz only mode
> > + * @NL80211_NAN_BAND_DUAL: 2.4GHz and 5.2GHz mode
> > +  */
> > +enum nl80211_nan_dual_band_conf {
> > +	NL80211_NAN_BAND_DEFAULT,
> > +	NL80211_NAN_BAND_SINGLE,
> > +	NL80211_NAN_BAND_DUAL,
> > +
> > +	/* keep last */
> > +	__NL80211_NAN_BAND_AFTER_LAST,
> > +	NL80211_NAN_BAND_MAX =
> > +	__NL80211_NAN_BAND_AFTER_LAST - 1,
> > +};
> > +
> >  #endif /* __LINUX_NL80211_H */
> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index
> > e29ff57..a74027f 100644
> > --- a/net/mac80211/cfg.c
> > +++ b/net/mac80211/cfg.c
> > @@ -257,6 +257,7 @@ static int ieee80211_add_key(struct wiphy *wiphy,
> struct net_device *dev,
> >  	case NL80211_IFTYPE_WDS:
> >  	case NL80211_IFTYPE_MONITOR:
> >  	case NL80211_IFTYPE_P2P_DEVICE:
> > +	case NL80211_IFTYPE_NAN:
> >  	case NL80211_IFTYPE_UNSPECIFIED:
> >  	case NUM_NL80211_IFTYPES:
> 
> Huh? What is this doing here? Not yours but weird still.
> 
> >  	case NL80211_IFTYPE_P2P_CLIENT:
> > @@ -2036,6 +2037,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
> >  		     !(req->flags & NL80211_SCAN_FLAG_AP)))
> >  			return -EOPNOTSUPP;
> >  		break;
> > +	case NL80211_IFTYPE_NAN:
> >  	default:
> >  		return -EOPNOTSUPP;
> >  	}
> > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index
> > 74142d0..acb50f8 100644
> > --- a/net/mac80211/chan.c
> > +++ b/net/mac80211/chan.c
> > @@ -274,6 +274,7 @@ ieee80211_get_chanctx_max_required_bw(struct
> ieee80211_local *local,
> >  				    ieee80211_get_max_required_bw(sdata));
> >  			break;
> >  		case NL80211_IFTYPE_P2P_DEVICE:
> > +		case NL80211_IFTYPE_NAN:
> >  			continue;
> >  		case NL80211_IFTYPE_ADHOC:
> >  		case NL80211_IFTYPE_WDS:
> > @@ -718,6 +719,7 @@ void ieee80211_recalc_smps_chanctx(struct
> > ieee80211_local *local,
> >
> >  		switch (sdata->vif.type) {
> >  		case NL80211_IFTYPE_P2P_DEVICE:
> > +		case NL80211_IFTYPE_NAN:
> >  			continue;
> >  		case NL80211_IFTYPE_STATION:
> >  			if (!sdata->u.mgd.associated)
> > @@ -981,6 +983,7 @@
> ieee80211_vif_chanctx_reservation_complete(struct ieee80211_sub_if_data
> *sdata)
> >  	case NL80211_IFTYPE_P2P_GO:
> >  	case NL80211_IFTYPE_P2P_DEVICE:
> >  	case NUM_NL80211_IFTYPES:
> 
> I think NUM_NL80211_IFTYPES should not be in the switch. If it must I would
> leave it as last one here.

Agree that it should go last

> 
> > +	case NL80211_IFTYPE_NAN:
> >  		WARN_ON(1);
> >  		break;
> >  	}
> 
> Regards,
> Arend

^ permalink raw reply

* [PATCH] ath6kl: fix return value in ath6kl_wmi_set_pvb_cmd
From: Chaehyun Lim @ 2016-09-18  6:30 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, netdev, Chaehyun Lim

When building with W=1, we got one warning as belows:
drivers/net/wireless/ath/ath6kl/wmi.c:3509:6: warning: variable ‘ret’
set but not used [-Wunused-but-set-variable]

At the end of ath6kl_wmi_set_pvb_cmd, it is returned by 0 regardless of
return value of ath6kl_wmi_cmd_send.
This patch fixes return value from 0 to ret that has result of
ath6kl_wmi_cmd_send execution.

Signed-off-by: Chaehyun Lim <chaehyun.lim@gmail.com>
---
 drivers/net/wireless/ath/ath6kl/wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index b8cf04d..3fd1cc9 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -3520,7 +3520,7 @@ int ath6kl_wmi_set_pvb_cmd(struct wmi *wmi, u8 if_idx, u16 aid,
 	ret = ath6kl_wmi_cmd_send(wmi, if_idx, skb, WMI_AP_SET_PVB_CMDID,
 				  NO_SYNC_WMIFLAG);
 
-	return 0;
+	return ret;
 }
 
 int ath6kl_wmi_set_rx_frame_format_cmd(struct wmi *wmi, u8 if_idx,
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Larry Finger @ 2016-09-17 23:06 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: kvalo, devel, linux-wireless
In-Reply-To: <wrfjbmzmmed0.fsf@redhat.com>

On 09/17/2016 03:59 PM, Jes Sorensen wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>> As soon as debugging is turned on, the logs are filled with messages
>> reporting the interrupt status. As this quantity is usually zero, this
>> output is not needed. In fact, there will be a report if the status is
>> not zero, thus the debug line in question could probably be deleted.
>> Rather than taking that action, I have changed it to only be printed
>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>
> Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
> that.
>
> Which device do you see this with?

OK. I will change the flag.

I found this with a TP-Link TL-MN8200ND, which has some variant of the RTL8188CU 
chip. It transmits, but I see no evidence that the receiver is functioning at 
all. The same is true for driver rtl8192cu. Only the driver from Realtek's web 
site actually works.

One other problem that I have found is that the debug option on module load 
seems to be ignored. So far, I've had to hard wire the flags. Once I find the 
reason, I'll send a patch for that as well.

Larry

^ permalink raw reply

* Re: [PATCH 2/4] carl9170: fix debugfs crashes
From: Greg KH @ 2016-09-17 21:45 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Kalle Valo, Christian Lamparter, linux-kernel, linux-wireless,
	b43-dev, Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <ccf228acb7af8ee3bbd82f72f28ebc068f37cb8e.1474140477.git.chunkeey@gmail.com>

On Sat, Sep 17, 2016 at 09:43:02PM +0200, Christian Lamparter wrote:
> Ben Greear reported:
> > I see lots of instability as soon as I load up the carl9710 NIC.
> > My application is going to be poking at it's debugfs files...
> >
> > BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> > [carl9170] at addr ffff8801bc1208b0
> > Read of size 8 by task btserver/5888
> > =======================================================================
> > BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
> > -----------------------------------------------------------------------
> >
> > INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
> >...
> 
> This breakage was caused by the introduction of intermediate
> fops in debugfs by commit 9fd4dcece43a
> ("debugfs: prevent access to possibly dead file_operations at file open")

Because of this, these should all be backported to 4.7-stable, and
4.8-stable, right?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Jes Sorensen @ 2016-09-17 20:59 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, devel, linux-wireless
In-Reply-To: <1474132155-9330-1-git-send-email-Larry.Finger@lwfinger.net>

Larry Finger <Larry.Finger@lwfinger.net> writes:
> As soon as debugging is turned on, the logs are filled with messages
> reporting the interrupt status. As this quantity is usually zero, this
> output is not needed. In fact, there will be a report if the status is
> not zero, thus the debug line in question could probably be deleted.
> Rather than taking that action, I have changed it to only be printed
> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.

Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
that.

Which device do you see this with?

Thanks,
Jes

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt
From: Jes Sorensen @ 2016-09-17 20:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: Larry Finger, kvalo, devel, linux-wireless
In-Reply-To: <1474133525.32273.97.camel@perches.com>

Joe Perches <joe@perches.com> writes:
> On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
>> As soon as debugging is turned on, the logs are filled with messages
>> reporting the interrupt status. As this quantity is usually zero, this
>> output is not needed. In fact, there will be a report if the status is
>> not zero, thus the debug line in question could probably be deleted.
>> Rather than taking that action, I have changed it to only be printed
>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>
> There are many uses of
> 	if (rtl8xxxu_debug & <DEFINE>) {
> 		dev_info(dev, ...)
>
> Emitting debugging information at KERN_INFO is odd.

Not at all, it's a pain to enable it in debug fs post loading the
driver, especially if you need the output immediately during driver
init. That is why the flags are there.

> I think it'd be nicer to use dev_dbg for all these cases
> and as well use some new macro that includes the test
>
> Something like:
>
> #define rtl8xxxu_dbg(type, fmt, ...)			\
> do {							\
> 	if (rtl8xxxu_debug & (type))			\
> 		dev_dbg(dev, fmt, ##__VA_ARGS__);	\
> } while (0)

Yuck yuck yuck, no thanks!

Any attempt of adding that kinda grossness to the driver will get a
NACK.

There is a reason the debug flag is there - it allows you to enable
specific items on driver load. If you enable that flag, expect to see
stuff in your log.

Jes

^ permalink raw reply

* [PATCH 3/4] b43: fix debugfs crash
From: Christian Lamparter @ 2016-09-17 19:43 UTC (permalink / raw)
  To: gregkh, Kalle Valo
  Cc: Christian Lamparter, linux-kernel, linux-wireless, b43-dev,
	Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <ccf228acb7af8ee3bbd82f72f28ebc068f37cb8e.1474140477.git.chunkeey@gmail.com>

This patch fixes a crash that happens because b43's
debugfs code expects file->f_op to be a pointer to
its own b43_debugfs_fops struct. This is no longer
the case since commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Reviewed-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 drivers/net/wireless/broadcom/b43/debugfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/debugfs.c b/drivers/net/wireless/broadcom/b43/debugfs.c
index b4bcd94..7704638 100644
--- a/drivers/net/wireless/broadcom/b43/debugfs.c
+++ b/drivers/net/wireless/broadcom/b43/debugfs.c
@@ -524,7 +524,8 @@ static ssize_t b43_debugfs_read(struct file *file, char __user *userbuf,
 		goto out_unlock;
 	}
 
-	dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+	dfops = container_of(debugfs_real_fops(file),
+			     struct b43_debugfs_fops, fops);
 	if (!dfops->read) {
 		err = -ENOSYS;
 		goto out_unlock;
@@ -585,7 +586,8 @@ static ssize_t b43_debugfs_write(struct file *file,
 		goto out_unlock;
 	}
 
-	dfops = container_of(file->f_op, struct b43_debugfs_fops, fops);
+	dfops = container_of(debugfs_real_fops(file),
+			     struct b43_debugfs_fops, fops);
 	if (!dfops->write) {
 		err = -ENOSYS;
 		goto out_unlock;
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/4] carl9170: fix debugfs crashes
From: Christian Lamparter @ 2016-09-17 19:43 UTC (permalink / raw)
  To: gregkh, Kalle Valo
  Cc: Christian Lamparter, linux-kernel, linux-wireless, b43-dev,
	Nicolai Stange, Ben Greear, Larry Finger
In-Reply-To: <48411543620969bebb37a1a9ea7b8f451cdfdd31.1474140477.git.chunkeey@gmail.com>

Ben Greear reported:
> I see lots of instability as soon as I load up the carl9710 NIC.
> My application is going to be poking at it's debugfs files...
>
> BUG: KASAN: slab-out-of-bounds in carl9170_debugfs_read+0xd5/0x2a0
> [carl9170] at addr ffff8801bc1208b0
> Read of size 8 by task btserver/5888
> =======================================================================
> BUG kmalloc-256 (Tainted: G        W      ): kasan: bad access detected
> -----------------------------------------------------------------------
>
> INFO: Allocated in seq_open+0x50/0x100 age=2690 cpu=2 pid=772
>...

This breakage was caused by the introduction of intermediate
fops in debugfs by commit 9fd4dcece43a
("debugfs: prevent access to possibly dead file_operations at file open")

Thankfully, the original/real fops are still available in d_fsdata.

Reported-by: Ben Greear <greearb@candelatech.com>
Reviewed-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 drivers/net/wireless/ath/carl9170/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c
index 01a0919..ad7ffd5 100644
--- a/drivers/net/wireless/ath/carl9170/debug.c
+++ b/drivers/net/wireless/ath/carl9170/debug.c
@@ -75,7 +75,7 @@ static ssize_t carl9170_debugfs_read(struct file *file, char __user *userbuf,
 
 	if (!ar)
 		return -ENODEV;
-	dfops = container_of(file->f_path.dentry->d_fsdata,
+	dfops = container_of(debugfs_real_fops(file),
 			     struct carl9170_debugfs_fops, fops);
 
 	if (!dfops->read)
@@ -128,7 +128,7 @@ static ssize_t carl9170_debugfs_write(struct file *file,
 
 	if (!ar)
 		return -ENODEV;
-	dfops = container_of(file->f_path.dentry->d_fsdata,
+	dfops = container_of(debugfs_real_fops(file),
 			     struct carl9170_debugfs_fops, fops);
 	if (!dfops->write)
 		return -ENOSYS;
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox