mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/5]
@ 2015-07-24  4:38 Joshua Clayton
  2015-07-24  4:38 ` [PATCH 1/5] staging: rtl8712: fix buggy size calculation Joshua Clayton
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24  4:38 UTC (permalink / raw)
  To: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman
  Cc: Nitin Kuppelur, Sudip Mukherjee, Joshua Clayton,
	Tapasweni Pathak, Vaishali Thakkar, devel, linux-kernel

The main goal of this series is to get rid of a needless and ugly typedef
in the rtl8712 wlan driver.

In the course of fixing that, I found a bug thati will can might (at least in theory)
lead to a overrun during a memcpy, as well as a duplicate struct.
Finally after cleaning up the typedef, I could not bring myself to leave
a variable called SupportedRates in the kernel with my name on it.

I have tested this on amd64. cwthe module loads and doesn't explode



Joshua Clayton (5):
  staging: rtl8712: fix buggy size calculation
  staging: rtl8712: simplify size calculation
  staging: rtl8712: remove duplicate struct
  staging: rtl8712: remove typedefs
  staging: rtl8712: style fix:

 drivers/staging/rtl8712/ieee80211.c           | 22 ++++++-------
 drivers/staging/rtl8712/rtl871x_cmd.c         | 28 +++++-----------
 drivers/staging/rtl8712/rtl871x_cmd.h         |  4 +--
 drivers/staging/rtl8712/rtl871x_event.h       |  2 +-
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 33 +++++++++----------
 drivers/staging/rtl8712/rtl871x_mlme.c        | 47 ++++++++++-----------------
 drivers/staging/rtl8712/rtl871x_mlme.h        |  2 +-
 drivers/staging/rtl8712/rtl871x_mp_ioctl.c    |  6 ++--
 drivers/staging/rtl8712/wlan_bssdef.h         | 32 +++---------------
 9 files changed, 63 insertions(+), 113 deletions(-)

-- 
2.4.6


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] staging: rtl8712: fix buggy size calculation
  2015-07-24  4:38 [PATCH 0/5] Joshua Clayton
@ 2015-07-24  4:38 ` Joshua Clayton
  2015-07-24 10:52   ` Dan Carpenter
  2015-07-24  4:38 ` [PATCH 2/5] staging: rtl8712: simplify " Joshua Clayton
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24  4:38 UTC (permalink / raw)
  To: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman
  Cc: Nitin Kuppelur, Sudip Mukherjee, Joshua Clayton,
	Tapasweni Pathak, Vaishali Thakkar, devel, linux-kernel

r8712_get_ndis_wlan_bssid_ex_sz has a "6 * sizeof(unsigned long)"
where the underlying struct has a 6 * unsigned char.
Simplify the calculation by just subtracting the variable part from
the size of the struct.

This also gets rid of a use of typedef NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index c044b0e..6b3451f 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -210,17 +210,7 @@ void r8712_generate_random_ibss(u8 *pibss)
 
 uint r8712_get_ndis_wlan_bssid_ex_sz(struct ndis_wlan_bssid_ex *bss)
 {
-	uint t_len;
-
-	t_len = sizeof(u32) + 6 * sizeof(unsigned long) + 2 +
-			sizeof(struct ndis_802_11_ssid) + sizeof(u32) +
-			sizeof(s32) +
-			sizeof(enum NDIS_802_11_NETWORK_TYPE) +
-			sizeof(struct NDIS_802_11_CONFIGURATION) +
-			sizeof(enum NDIS_802_11_NETWORK_INFRASTRUCTURE) +
-			sizeof(NDIS_802_11_RATES_EX) +
-			sizeof(u32) + bss->IELength;
-	return t_len;
+	return sizeof(*bss) + bss->IELength - MAX_IE_SZ;
 }
 
 u8 *r8712_get_capability_from_ie(u8 *ie)
-- 
2.4.6


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] staging: rtl8712: simplify size calculation
  2015-07-24  4:38 [PATCH 0/5] Joshua Clayton
  2015-07-24  4:38 ` [PATCH 1/5] staging: rtl8712: fix buggy size calculation Joshua Clayton
@ 2015-07-24  4:38 ` Joshua Clayton
  2015-07-24  4:52 ` [PATCH 4/5] staging: rtl8712: remove typedefs Joshua Clayton
  2015-07-24  4:53 ` [PATCH 5/5] staging: rtl8712: style fix: Joshua Clayton
  3 siblings, 0 replies; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24  4:38 UTC (permalink / raw)
  To: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman
  Cc: Nitin Kuppelur, Sudip Mukherjee, Joshua Clayton,
	Tapasweni Pathak, Vaishali Thakkar, devel, linux-kernel

replace item-by-item  size calculation of a struct
with the size of the struct.

This gets rid of a use of typedef NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_cmd.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index e35854d..f07050d 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -471,7 +471,6 @@ u8 r8712_createbss_cmd(struct _adapter *padapter)
 
 u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 {
-	uint t_len = 0;
 	struct ndis_wlan_bssid_ex *psecnetwork;
 	struct cmd_obj		*pcmd;
 	struct cmd_priv		*pcmdpriv = &padapter->cmdpriv;
@@ -486,14 +485,6 @@ u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 	pcmd = kmalloc(sizeof(*pcmd), GFP_ATOMIC);
 	if (pcmd == NULL)
 		return _FAIL;
-	t_len = sizeof(u32) + 6 * sizeof(unsigned char) + 2 +
-			sizeof(struct ndis_802_11_ssid) + sizeof(u32) +
-			sizeof(s32) +
-			sizeof(enum NDIS_802_11_NETWORK_TYPE) +
-			sizeof(struct NDIS_802_11_CONFIGURATION) +
-			sizeof(enum NDIS_802_11_NETWORK_INFRASTRUCTURE) +
-			sizeof(NDIS_802_11_RATES_EX) +
-			sizeof(u32) + MAX_IE_SZ;
 
 	/* for hidden ap to set fw_state here */
 	if (check_fwstate(pmlmepriv, WIFI_STATION_STATE|WIFI_ADHOC_STATE) !=
@@ -516,7 +507,7 @@ u8 r8712_joinbss_cmd(struct _adapter  *padapter, struct wlan_network *pnetwork)
 		kfree(pcmd);
 		return _FAIL;
 	}
-	memcpy(psecnetwork, &pnetwork->network, t_len);
+	memcpy(psecnetwork, &pnetwork->network, sizeof(*psecnetwork));
 	psecuritypriv->authenticator_ie[0] = (unsigned char)
 					     psecnetwork->IELength;
 	if ((psecnetwork->IELength-12) < (256 - 1))
-- 
2.4.6


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] staging: rtl8712: remove typedefs
  2015-07-24  4:38 [PATCH 0/5] Joshua Clayton
  2015-07-24  4:38 ` [PATCH 1/5] staging: rtl8712: fix buggy size calculation Joshua Clayton
  2015-07-24  4:38 ` [PATCH 2/5] staging: rtl8712: simplify " Joshua Clayton
@ 2015-07-24  4:52 ` Joshua Clayton
  2015-07-24  4:53 ` [PATCH 5/5] staging: rtl8712: style fix: Joshua Clayton
  3 siblings, 0 replies; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24  4:52 UTC (permalink / raw)
  To: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman
  Cc: Nitin Kuppelur, Sudip Mukherjee, Joshua Clayton,
	Tapasweni Pathak, Vaishali Thakkar, devel, linux-kernel

Coding style fix.
Get rid of typedefs NDIS_802_11_RATES and NDIS_802_11_RATES_EX

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 4 ++--
 drivers/staging/rtl8712/wlan_bssdef.h         | 7 +------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 4f5f69c..087fbf1 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -635,7 +635,7 @@ static int r8711_wx_get_name(struct net_device *dev,
 	u8 ht_cap = false;
 	struct	mlme_priv	*pmlmepriv = &(padapter->mlmepriv);
 	struct wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
-	NDIS_802_11_RATES_EX *prates = NULL;
+	u8 *prates;
 
 	if (check_fwstate(pmlmepriv, _FW_LINKED|WIFI_ADHOC_MASTER_STATE) ==
 	    true) {
@@ -644,7 +644,7 @@ static int r8711_wx_get_name(struct net_device *dev,
 				 &ht_ielen, pcur_bss->IELength - 12);
 		if (p && ht_ielen > 0)
 			ht_cap = true;
-		prates = &pcur_bss->SupportedRates;
+		prates = pcur_bss->SupportedRates;
 		if (r8712_is_cckratesonly_included((u8 *)prates) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
diff --git a/drivers/staging/rtl8712/wlan_bssdef.h b/drivers/staging/rtl8712/wlan_bssdef.h
index 7d769e8..7a410b5 100644
--- a/drivers/staging/rtl8712/wlan_bssdef.h
+++ b/drivers/staging/rtl8712/wlan_bssdef.h
@@ -32,11 +32,6 @@
 #define NDIS_802_11_LENGTH_RATES        8
 #define NDIS_802_11_LENGTH_RATES_EX     16
 
-/* Set of 8 data rates*/
-typedef unsigned char   NDIS_802_11_RATES[NDIS_802_11_LENGTH_RATES];
-/* Set of 16 data rates */
-typedef unsigned char   NDIS_802_11_RATES_EX[NDIS_802_11_LENGTH_RATES_EX];
-
 struct ndis_802_11_ssid {
 	u32 SsidLength;
 	u8  Ssid[32];
@@ -104,7 +99,7 @@ struct wlan_bssid_ex {
 	enum NDIS_802_11_NETWORK_TYPE  NetworkTypeInUse;
 	struct NDIS_802_11_CONFIGURATION  Configuration;
 	enum NDIS_802_11_NETWORK_INFRASTRUCTURE  InfrastructureMode;
-	NDIS_802_11_RATES_EX  SupportedRates;
+	u8 SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
 	u32 IELength;
 	/*(timestamp, beacon interval, and capability information) */
 	u8 IEs[MAX_IE_SZ];
-- 
2.4.6


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] staging: rtl8712: style fix:
  2015-07-24  4:38 [PATCH 0/5] Joshua Clayton
                   ` (2 preceding siblings ...)
  2015-07-24  4:52 ` [PATCH 4/5] staging: rtl8712: remove typedefs Joshua Clayton
@ 2015-07-24  4:53 ` Joshua Clayton
  2015-07-24 10:52   ` Dan Carpenter
  3 siblings, 1 reply; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24  4:53 UTC (permalink / raw)
  To: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman
  Cc: Nitin Kuppelur, Sudip Mukherjee, Joshua Clayton,
	Tapasweni Pathak, Vaishali Thakkar, devel, linux-kernel

change instances SupportedRates to compliant and sane "rates"
This change in no way harms readability, and brings several lines
under the 80 character limit.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/staging/rtl8712/ieee80211.c           | 22 +++++++++++-----------
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 21 +++++++++------------
 drivers/staging/rtl8712/wlan_bssdef.h         |  4 ++--
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/rtl8712/ieee80211.c b/drivers/staging/rtl8712/ieee80211.c
index 5786808..ca9dcaf 100644
--- a/drivers/staging/rtl8712/ieee80211.c
+++ b/drivers/staging/rtl8712/ieee80211.c
@@ -134,22 +134,22 @@ u8 *r8712_get_ie(u8 *pbuf, sint index, sint *len, sint limit)
 	return NULL;
 }
 
-static void set_supported_rate(u8 *SupportedRates, uint mode)
+static void set_supported_rate(u8 *rates, uint mode)
 {
-	memset(SupportedRates, 0, NDIS_802_11_LENGTH_RATES_EX);
+	memset(rates, 0, NDIS_802_11_LENGTH_RATES_EX);
 	switch (mode) {
 	case WIRELESS_11B:
-		memcpy(SupportedRates, WIFI_CCKRATES,
+		memcpy(rates, WIFI_CCKRATES,
 			IEEE80211_CCK_RATE_LEN);
 		break;
 	case WIRELESS_11G:
 	case WIRELESS_11A:
-		memcpy(SupportedRates, WIFI_OFDMRATES,
+		memcpy(rates, WIFI_OFDMRATES,
 			IEEE80211_NUM_OFDM_RATESLEN);
 		break;
 	case WIRELESS_11BG:
-		memcpy(SupportedRates, WIFI_CCKRATES, IEEE80211_CCK_RATE_LEN);
-		memcpy(SupportedRates + IEEE80211_CCK_RATE_LEN, WIFI_OFDMRATES,
+		memcpy(rates, WIFI_CCKRATES, IEEE80211_CCK_RATE_LEN);
+		memcpy(rates + IEEE80211_CCK_RATE_LEN, WIFI_OFDMRATES,
 			IEEE80211_NUM_OFDM_RATESLEN);
 		break;
 	}
@@ -195,17 +195,17 @@ int r8712_generate_ie(struct registry_priv *pregistrypriv)
 	ie = r8712_set_ie(ie, _SSID_IE_, pdev_network->Ssid.SsidLength,
 		    pdev_network->Ssid.Ssid, &sz);
 	/*supported rates*/
-	set_supported_rate(pdev_network->SupportedRates,
+	set_supported_rate(pdev_network->rates,
 			   pregistrypriv->wireless_mode);
-	rateLen = r8712_get_rateset_len(pdev_network->SupportedRates);
+	rateLen = r8712_get_rateset_len(pdev_network->rates);
 	if (rateLen > 8) {
 		ie = r8712_set_ie(ie, _SUPPORTEDRATES_IE_, 8,
-			    pdev_network->SupportedRates, &sz);
+			    pdev_network->rates, &sz);
 		ie = r8712_set_ie(ie, _EXT_SUPPORTEDRATES_IE_, (rateLen - 8),
-			    (pdev_network->SupportedRates + 8), &sz);
+			    (pdev_network->rates + 8), &sz);
 	} else
 		ie = r8712_set_ie(ie, _SUPPORTEDRATES_IE_,
-			    rateLen, pdev_network->SupportedRates, &sz);
+			    rateLen, pdev_network->rates, &sz);
 	/*DS parameter set*/
 	ie = r8712_set_ie(ie, _DSSET_IE_, 1,
 		    (u8 *)&(pdev_network->Configuration.DSConfig), &sz);
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 087fbf1..219239c 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -203,14 +203,12 @@ static inline char *translate_scan(struct _adapter *padapter,
 	}
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;
-	if ((r8712_is_cckratesonly_included((u8 *)&pnetwork->network.
-	     SupportedRates)) == true) {
+	if (r8712_is_cckratesonly_included(pnetwork->network.rates)) {
 		if (ht_cap == true)
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bn");
 		else
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11b");
-	} else if ((r8712_is_cckrates_included((u8 *)&pnetwork->network.
-		    SupportedRates)) == true) {
+	} else if (r8712_is_cckrates_included(pnetwork->network.rates)) {
 		if (ht_cap == true)
 			snprintf(iwe.u.name, IFNAMSIZ, "IEEE 802.11bgn");
 		else
@@ -270,9 +268,9 @@ static inline char *translate_scan(struct _adapter *padapter,
 	iwe.u.bitrate.disabled = 0;
 	iwe.u.bitrate.value = 0;
 	i = 0;
-	while (pnetwork->network.SupportedRates[i] != 0) {
+	while (pnetwork->network.rates[i] != 0) {
 		/* Bit rate given in 500 kb/s units */
-		iwe.u.bitrate.value = (pnetwork->network.SupportedRates[i++] &
+		iwe.u.bitrate.value = (pnetwork->network.rates[i++] &
 				      0x7F) * 500000;
 		current_val = iwe_stream_add_value(info, start, current_val,
 			      stop, &iwe, IW_EV_PARAM_LEN);
@@ -644,15 +642,15 @@ static int r8711_wx_get_name(struct net_device *dev,
 				 &ht_ielen, pcur_bss->IELength - 12);
 		if (p && ht_ielen > 0)
 			ht_cap = true;
-		prates = pcur_bss->SupportedRates;
-		if (r8712_is_cckratesonly_included((u8 *)prates) == true) {
+		prates = pcur_bss->rates;
+		if (r8712_is_cckratesonly_included(prates) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11bn");
 			else
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11b");
-		} else if ((r8712_is_cckrates_included((u8 *)prates)) == true) {
+		} else if ((r8712_is_cckrates_included(prates)) == true) {
 			if (ht_cap == true)
 				snprintf(wrqu->name, IFNAMSIZ,
 					 "IEEE 802.11bgn");
@@ -1444,9 +1442,8 @@ static int r8711_wx_get_rate(struct net_device *dev,
 				    (IEEE80211_HT_CAP_SGI_20 |
 				    IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
 		}
-		while ((pcur_bss->SupportedRates[i] != 0) &&
-			(pcur_bss->SupportedRates[i] != 0xFF)) {
-			rate = pcur_bss->SupportedRates[i] & 0x7F;
+		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
+			rate = pcur_bss->rates[i] & 0x7F;
 			if (rate > max_rate)
 				max_rate = rate;
 			wrqu->bitrate.fixed = 0;	/* no auto select */
diff --git a/drivers/staging/rtl8712/wlan_bssdef.h b/drivers/staging/rtl8712/wlan_bssdef.h
index 7a410b5..8e2d798 100644
--- a/drivers/staging/rtl8712/wlan_bssdef.h
+++ b/drivers/staging/rtl8712/wlan_bssdef.h
@@ -83,7 +83,7 @@ struct NDIS_802_11_FIXED_IEs {
  * 6 * sizeof (unsigned char) + 2 + sizeof (ndis_802_11_ssid) + sizeof (u32)
  * + sizeof (s32) + sizeof (NDIS_802_11_NETWORK_TYPE)
  * + sizeof (struct NDIS_802_11_CONFIGURATION)
- * + sizeof (NDIS_802_11_RATES_EX) + IELength
+ * + sizeof (rates) + IELength
 
  * Except the IELength, all other fields are fixed length. Therefore, we can
  * define a macro to present the partial sum.
@@ -99,7 +99,7 @@ struct wlan_bssid_ex {
 	enum NDIS_802_11_NETWORK_TYPE  NetworkTypeInUse;
 	struct NDIS_802_11_CONFIGURATION  Configuration;
 	enum NDIS_802_11_NETWORK_INFRASTRUCTURE  InfrastructureMode;
-	u8 SupportedRates[NDIS_802_11_LENGTH_RATES_EX];
+	u8 rates[NDIS_802_11_LENGTH_RATES_EX];
 	u32 IELength;
 	/*(timestamp, beacon interval, and capability information) */
 	u8 IEs[MAX_IE_SZ];
-- 
2.4.6


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] staging: rtl8712: style fix:
  2015-07-24  4:53 ` [PATCH 5/5] staging: rtl8712: style fix: Joshua Clayton
@ 2015-07-24 10:52   ` Dan Carpenter
  2015-07-24 13:06     ` Joshua Clayton
  0 siblings, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2015-07-24 10:52 UTC (permalink / raw)
  To: Joshua Clayton
  Cc: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman, devel,
	Tapasweni Pathak, Vaishali Thakkar, Nitin Kuppelur, linux-kernel,
	Sudip Mukherjee

Write a better subject line.

On Thu, Jul 23, 2015 at 09:53:18PM -0700, Joshua Clayton wrote:
> change instances SupportedRates to compliant and sane "rates"
> This change in no way harms readability, and brings several lines
> under the 80 character limit.
> 

Yeah, but it does a some other stuff as well like removing casts.

> -		while ((pcur_bss->SupportedRates[i] != 0) &&
> -			(pcur_bss->SupportedRates[i] != 0xFF)) {
> -			rate = pcur_bss->SupportedRates[i] & 0x7F;
> +		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
> +			rate = pcur_bss->rates[i] & 0x7F;
>  			if (rate > max_rate)
>  				max_rate = rate;
>  			wrqu->bitrate.fixed = 0;	/* no auto select */

I actually like the != 0 here because we're talking about the number
zero.  It should look like this:

		while (pcur_bss->rates[i] != 0 &&
		       pcur_bss->rates[i] != 0xFF) {

But removing the parens is something for a different patch.  I use a
script to help review these so when you mix different changes together
it means there is more manual review work for me.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/5] staging: rtl8712: fix buggy size calculation
  2015-07-24  4:38 ` [PATCH 1/5] staging: rtl8712: fix buggy size calculation Joshua Clayton
@ 2015-07-24 10:52   ` Dan Carpenter
  0 siblings, 0 replies; 9+ messages in thread
From: Dan Carpenter @ 2015-07-24 10:52 UTC (permalink / raw)
  To: Joshua Clayton
  Cc: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman, devel,
	Tapasweni Pathak, Vaishali Thakkar, Nitin Kuppelur, linux-kernel,
	Sudip Mukherjee

On Thu, Jul 23, 2015 at 09:38:33PM -0700, Joshua Clayton wrote:
> r8712_get_ndis_wlan_bssid_ex_sz has a "6 * sizeof(unsigned long)"
> where the underlying struct has a 6 * unsigned char.
> Simplify the calculation by just subtracting the variable part from
> the size of the struct.
> 
> This also gets rid of a use of typedef NDIS_802_11_RATES_EX
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>

Looks like you are right.

Reviewed-by: Dan Carpenter

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] staging: rtl8712: style fix:
  2015-07-24 10:52   ` Dan Carpenter
@ 2015-07-24 13:06     ` Joshua Clayton
  0 siblings, 0 replies; 9+ messages in thread
From: Joshua Clayton @ 2015-07-24 13:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Florian Schilhabel, Greg Kroah-Hartman, devel,
	Tapasweni Pathak, Vaishali Thakkar, Nitin Kuppelur, linux-kernel,
	Sudip Mukherjee

Dan,
On Friday, July 24, 2015 01:52:27 PM Dan Carpenter wrote:
> Write a better subject line.
> 
> On Thu, Jul 23, 2015 at 09:53:18PM -0700, Joshua Clayton wrote:
> > change instances SupportedRates to compliant and sane "rates"
> > This change in no way harms readability, and brings several lines
> > under the 80 character limit.
> 
> Yeah, but it does a some other stuff as well like removing casts.
I apologize. I thought it would make sense to double up (while I'm in there 
I'll just...).

> 
> > -		while ((pcur_bss->SupportedRates[i] != 0) &&
> > -			(pcur_bss->SupportedRates[i] != 0xFF)) {
> > -			rate = pcur_bss->SupportedRates[i] & 0x7F;
> > +		while (pcur_bss->rates[i] && (pcur_bss->rates[i] != 0xFF)) {
> > +			rate = pcur_bss->rates[i] & 0x7F;
> > 
> >  			if (rate > max_rate)
> >  			
> >  				max_rate = rate;
> >  			
> >  			wrqu->bitrate.fixed = 0;	/* no auto select */
> 
> I actually like the != 0 here because we're talking about the number
> zero.  It should look like this:
> 
> 		while (pcur_bss->rates[i] != 0 &&
> 		       pcur_bss->rates[i] != 0xFF) {
OK.

> 
> But removing the parens is something for a different patch.  I use a
> script to help review these so when you mix different changes together
> it means there is more manual review work for me.
Sorry about that.
I'll split or drop it in the next version
> 
> regards,
> dan carpenter

Joshua


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/5]
@ 2020-07-01  9:08 Jason Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Wang @ 2020-07-01  9:08 UTC (permalink / raw)
  To: mst, virtualization, linux-kernel
  Cc: rob.miller, lingshan.zhu, eperezma, lulu, shahafs, hanand,
	mhabets, gdawar, saugatm, vmireyno, zhangweining, eli,
	Jason Wang

Hi all:

This series tries to support batched IOTLB updating vhost-vdpa.

Currently vhost-vdpa accepts userspace mapping via IOTLB API, and it
can only forward one mapping to IOMMU or device through IOMMU API or
dma_map(). Though set_map() is designed to have the capability to pass
an rbtree based mapping to vDPA device, it's still be called at least
once for each VHOST_IOTLB_UPDATE or VHOST_IOTLB_INVALIDATE. This is
because vhost-vdpa doesn't know the userspace start or stop then
updating.

So this patch introduces two flags as hints for vhost-vdpa to call
set_map() only when userspace finish a batch of IOTLB updating.

So instead of:

1) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)
2) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)
...
n) VHOST_IOTLB_UPDATE/VHOST_IOTLB_INVALIDATE -> set_map() (s)

With the help of hints, we do:

0) VHOST_IOTLB_BATCH_START

1) VHOST_IOTLB_UPDATE/INVALIDATE
...
n) VHOST_IOTLB_UPDATE/INVALIDATE

n+1) VHOST_IOTLB_BATCH_END -> set_map()

One one call of set_map() to vDPA device for a batch of IOTLB
mappings. So for the device that has its own DMA translation logic, it
can efficiently structure the memory mapping to get best performance.

Note, this only impacts the devices that want its own DMA
translation (less times of set_map() call). For other type of devices,
there's no changes in the behaviour.

Changes from RFCV1:

- tweak the comments per Michael's request

Jason Wang (5):
  vhost-vdpa: refine ioctl pre-processing
  vhost: generialize backend features setting/getting
  vhost-vdpa: support get/set backend features
  vhost-vdpa: support IOTLB batching hints
  vdpasim: support batch updating

 drivers/vdpa/vdpa_sim/vdpa_sim.c | 40 +++++++++++++++++++++--
 drivers/vhost/net.c              | 18 ++---------
 drivers/vhost/vdpa.c             | 55 ++++++++++++++++++++++++++------
 drivers/vhost/vhost.c            | 15 +++++++++
 drivers/vhost/vhost.h            |  2 ++
 include/uapi/linux/vhost.h       |  2 ++
 include/uapi/linux/vhost_types.h | 11 +++++++
 7 files changed, 114 insertions(+), 29 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-07-01  9:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24  4:38 [PATCH 0/5] Joshua Clayton
2015-07-24  4:38 ` [PATCH 1/5] staging: rtl8712: fix buggy size calculation Joshua Clayton
2015-07-24 10:52   ` Dan Carpenter
2015-07-24  4:38 ` [PATCH 2/5] staging: rtl8712: simplify " Joshua Clayton
2015-07-24  4:52 ` [PATCH 4/5] staging: rtl8712: remove typedefs Joshua Clayton
2015-07-24  4:53 ` [PATCH 5/5] staging: rtl8712: style fix: Joshua Clayton
2015-07-24 10:52   ` Dan Carpenter
2015-07-24 13:06     ` Joshua Clayton
2020-07-01  9:08 [PATCH 0/5] Jason Wang

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

all inboxes | Powered by JetHome®