* [PATCH] staging: rtl8192u: Merge almost duplicate code
@ 2020-05-17 16:58 Pascal Terjan
2020-05-17 20:22 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Pascal Terjan @ 2020-05-17 16:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, devel, linux-kernel; +Cc: Pascal Terjan
This causes a change in behaviour:
- stats also get updated when reordering, this seems like it should be
the case but those lines were commented out.
- sub_skb NULL check now happens early in both cases, previously it
happened only after dereferencing it 12 times, so it may not actually
be needed.
Signed-off-by: Pascal Terjan <pterjan@google.com>
---
.../staging/rtl8192u/ieee80211/ieee80211_rx.c | 126 +++++++-----------
1 file changed, 49 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index e101f7b13c7e..3309f64be4c9 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -520,55 +520,67 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, struct rx_reorder_entry *p
return true;
}
-void ieee80211_indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb **prxbIndicateArray, u8 index)
+void indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb *rxb)
{
- u8 i = 0, j = 0;
+ struct net_device_stats *stats = &ieee->stats;
+ struct net_device *dev = ieee->dev;
u16 ethertype;
-// if(index > 1)
-// IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): hahahahhhh, We indicate packet from reorder list, index is %u\n",__func__,index);
- for (j = 0; j < index; j++) {
-//added by amy for reorder
- struct ieee80211_rxb *prxb = prxbIndicateArray[j];
- for (i = 0; i < prxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = prxb->subframes[i];
+ u8 i;
+
+ for (i = 0; i < rxb->nr_subframes; i++) {
+ struct sk_buff *sub_skb = rxb->subframes[i];
+
+ if (!sub_skb)
+ continue;
/* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
+ ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
+ if (sub_skb->len >= 8 &&
+ ((!memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) &&
+ ethertype != ETH_P_AARP &&
+ ethertype != ETH_P_IPX) ||
+ !memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE))) {
/* remove RFC1042 or Bridge-Tunnel encapsulation and
* replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- } else {
+ skb_pull(sub_skb, SNAP_SIZE);
+ } else {
/* Leave Ethernet header part of hdr and full payload */
- put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- }
- //stats->rx_packets++;
- //stats->rx_bytes += sub_skb->len;
+ put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
+ }
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->src, ETH_ALEN);
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->dst, ETH_ALEN);
+
+ stats->rx_packets++;
+ stats->rx_bytes += sub_skb->len;
+ if (is_multicast_ether_addr(rxb->dst))
+ stats->multicast++;
/* Indicate the packets to upper layer */
- if (sub_skb) {
- sub_skb->protocol = eth_type_trans(sub_skb, ieee->dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = ieee->dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ sub_skb->protocol = eth_type_trans(sub_skb, dev);
+ memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
+ sub_skb->dev = dev;
+ /* 802.11 crc not sufficient */
+ sub_skb->ip_summed = CHECKSUM_NONE;
+ ieee->last_rx_ps_time = jiffies;
+ netif_rx(sub_skb);
+ }
+}
+
+void ieee80211_indicate_packets(struct ieee80211_device *ieee,
+ struct ieee80211_rxb **prxbIndicateArray,
+ u8 index)
+{
+ u8 i;
+
+ for (i = 0; i < index; i++) {
+ struct ieee80211_rxb *prxb = prxbIndicateArray[i];
+
+ indicate_packets(ieee, prxb);
kfree(prxb);
prxb = NULL;
}
}
-
static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
struct ieee80211_rxb *prxb,
struct rx_ts_record *pTS, u16 SeqNum)
@@ -721,6 +733,7 @@ static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
/* Handling pending timer. Set this timer to prevent from long time Rx buffering.*/
if (index > 0) {
+ u8 i;
// Cancel previous pending timer.
// del_timer_sync(&pTS->rx_pkt_pending_timer);
pTS->rx_timeout_indicate_seq = 0xffff;
@@ -877,7 +890,6 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
u16 fc, type, stype, sc;
struct net_device_stats *stats;
unsigned int frag;
- u16 ethertype;
//added by amy for reorder
u8 TID = 0;
u16 SeqNum = 0;
@@ -1260,47 +1272,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
//added by amy for reorder
if (!ieee->pHTInfo->bCurRxReorderEnable || !pTS) {
-//added by amy for reorder
- for (i = 0; i < rxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = rxb->subframes[i];
-
- if (sub_skb) {
- /* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
- /* remove RFC1042 or Bridge-Tunnel encapsulation and
- * replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- } else {
- u16 len;
- /* Leave Ethernet header part of hdr and full payload */
- len = be16_to_cpu(htons(sub_skb->len));
- memcpy(skb_push(sub_skb, 2), &len, 2);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- }
-
- stats->rx_packets++;
- stats->rx_bytes += sub_skb->len;
- if (is_multicast_ether_addr(dst)) {
- stats->multicast++;
- }
-
- /* Indicate the packets to upper layer */
- sub_skb->protocol = eth_type_trans(sub_skb, dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ indicate_packets(ieee, rxb);
kfree(rxb);
rxb = NULL;
--
2.26.2.761.g0e0b3e54be-goog
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
2020-05-17 16:58 [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
@ 2020-05-17 20:22 ` kbuild test robot
2020-05-17 20:40 ` [PATCH v2] " Pascal Terjan
2020-05-17 20:22 ` [RFC PATCH] staging: rtl8192u: indicate_packets() can be static kbuild test robot
2020-05-17 20:25 ` [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2020-05-17 20:22 UTC (permalink / raw)
To: Pascal Terjan, Greg Kroah-Hartman, devel, linux-kernel
Cc: Pascal Terjan, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]
Hi Pascal,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v5.7-rc5 next-20200515]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Pascal-Terjan/staging-rtl8192u-Merge-almost-duplicate-code/20200518-005912
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git cef077e6aa4c7dbe2f23e1201cf705f9540ec467
config: i386-allyesconfig (attached as .config)
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-193-gb8fad4bc-dirty
# save the attached .config to linux build tree
make C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
/usr/lib/gcc/x86_64-linux-gnu/7/include/stddef.h:417:9: sparse: sparse: preprocessor token offsetof redefined
include/linux/stddef.h:17:9: sparse: this was the original definition
>> drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c:523:6: sparse: sparse: symbol 'indicate_packets' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72436 bytes --]
[-- Attachment #3: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2] staging: rtl8192u: Merge almost duplicate code
2020-05-17 20:22 ` kbuild test robot
@ 2020-05-17 20:40 ` Pascal Terjan
2020-05-19 14:16 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Pascal Terjan @ 2020-05-17 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, devel, linux-kernel; +Cc: Pascal Terjan
This causes a change in behaviour:
- stats also get updated when reordering, this seems like it should be
the case but those lines were commented out.
- sub_skb NULL check now happens early in both cases, previously it
happened only after dereferencing it 12 times, so it may not actually
be needed.
Signed-off-by: Pascal Terjan <pterjan@google.com>
---
v2: Made the new function static
.../staging/rtl8192u/ieee80211/ieee80211_rx.c | 127 +++++++-----------
1 file changed, 50 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index e101f7b13c7e..60dbb584b22c 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -520,55 +520,68 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, struct rx_reorder_entry *p
return true;
}
-void ieee80211_indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb **prxbIndicateArray, u8 index)
+static void indicate_packets(struct ieee80211_device *ieee,
+ struct ieee80211_rxb *rxb)
{
- u8 i = 0, j = 0;
+ struct net_device_stats *stats = &ieee->stats;
+ struct net_device *dev = ieee->dev;
u16 ethertype;
-// if(index > 1)
-// IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): hahahahhhh, We indicate packet from reorder list, index is %u\n",__func__,index);
- for (j = 0; j < index; j++) {
-//added by amy for reorder
- struct ieee80211_rxb *prxb = prxbIndicateArray[j];
- for (i = 0; i < prxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = prxb->subframes[i];
+ u8 i;
+
+ for (i = 0; i < rxb->nr_subframes; i++) {
+ struct sk_buff *sub_skb = rxb->subframes[i];
+
+ if (!sub_skb)
+ continue;
/* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
+ ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
+ if (sub_skb->len >= 8 &&
+ ((!memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) &&
+ ethertype != ETH_P_AARP &&
+ ethertype != ETH_P_IPX) ||
+ !memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE))) {
/* remove RFC1042 or Bridge-Tunnel encapsulation and
* replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- } else {
+ skb_pull(sub_skb, SNAP_SIZE);
+ } else {
/* Leave Ethernet header part of hdr and full payload */
- put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- }
- //stats->rx_packets++;
- //stats->rx_bytes += sub_skb->len;
+ put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
+ }
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->src, ETH_ALEN);
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->dst, ETH_ALEN);
+
+ stats->rx_packets++;
+ stats->rx_bytes += sub_skb->len;
+ if (is_multicast_ether_addr(rxb->dst))
+ stats->multicast++;
/* Indicate the packets to upper layer */
- if (sub_skb) {
- sub_skb->protocol = eth_type_trans(sub_skb, ieee->dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = ieee->dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ sub_skb->protocol = eth_type_trans(sub_skb, dev);
+ memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
+ sub_skb->dev = dev;
+ /* 802.11 crc not sufficient */
+ sub_skb->ip_summed = CHECKSUM_NONE;
+ ieee->last_rx_ps_time = jiffies;
+ netif_rx(sub_skb);
+ }
+}
+
+void ieee80211_indicate_packets(struct ieee80211_device *ieee,
+ struct ieee80211_rxb **prxbIndicateArray,
+ u8 index)
+{
+ u8 i;
+
+ for (i = 0; i < index; i++) {
+ struct ieee80211_rxb *prxb = prxbIndicateArray[i];
+
+ indicate_packets(ieee, prxb);
kfree(prxb);
prxb = NULL;
}
}
-
static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
struct ieee80211_rxb *prxb,
struct rx_ts_record *pTS, u16 SeqNum)
@@ -721,6 +734,7 @@ static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
/* Handling pending timer. Set this timer to prevent from long time Rx buffering.*/
if (index > 0) {
+ u8 i;
// Cancel previous pending timer.
// del_timer_sync(&pTS->rx_pkt_pending_timer);
pTS->rx_timeout_indicate_seq = 0xffff;
@@ -877,7 +891,6 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
u16 fc, type, stype, sc;
struct net_device_stats *stats;
unsigned int frag;
- u16 ethertype;
//added by amy for reorder
u8 TID = 0;
u16 SeqNum = 0;
@@ -1260,47 +1273,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
//added by amy for reorder
if (!ieee->pHTInfo->bCurRxReorderEnable || !pTS) {
-//added by amy for reorder
- for (i = 0; i < rxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = rxb->subframes[i];
-
- if (sub_skb) {
- /* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
- /* remove RFC1042 or Bridge-Tunnel encapsulation and
- * replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- } else {
- u16 len;
- /* Leave Ethernet header part of hdr and full payload */
- len = be16_to_cpu(htons(sub_skb->len));
- memcpy(skb_push(sub_skb, 2), &len, 2);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- }
-
- stats->rx_packets++;
- stats->rx_bytes += sub_skb->len;
- if (is_multicast_ether_addr(dst)) {
- stats->multicast++;
- }
-
- /* Indicate the packets to upper layer */
- sub_skb->protocol = eth_type_trans(sub_skb, dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ indicate_packets(ieee, rxb);
kfree(rxb);
rxb = NULL;
--
2.26.2.761.g0e0b3e54be-goog
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2] staging: rtl8192u: Merge almost duplicate code
2020-05-17 20:40 ` [PATCH v2] " Pascal Terjan
@ 2020-05-19 14:16 ` Greg Kroah-Hartman
2020-05-19 15:00 ` [PATCH v3] " Pascal Terjan
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-19 14:16 UTC (permalink / raw)
To: Pascal Terjan; +Cc: devel, linux-kernel
On Sun, May 17, 2020 at 09:40:45PM +0100, Pascal Terjan wrote:
> This causes a change in behaviour:
> - stats also get updated when reordering, this seems like it should be
> the case but those lines were commented out.
> - sub_skb NULL check now happens early in both cases, previously it
> happened only after dereferencing it 12 times, so it may not actually
> be needed.
>
> Signed-off-by: Pascal Terjan <pterjan@google.com>
This patch adds a new compiler warning, which no patch should do.
Please fix that up and resend.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] staging: rtl8192u: Merge almost duplicate code
2020-05-19 14:16 ` Greg Kroah-Hartman
@ 2020-05-19 15:00 ` Pascal Terjan
0 siblings, 0 replies; 10+ messages in thread
From: Pascal Terjan @ 2020-05-19 15:00 UTC (permalink / raw)
To: Greg Kroah-Hartman, devel, linux-kernel; +Cc: Pascal Terjan
This causes a change in behaviour:
- stats also get updated when reordering, this seems like it should be
the case but those lines were commented out.
- sub_skb NULL check now happens early in both cases, previously it
happened only after dereferencing it 12 times, so it may not actually
be needed.
Signed-off-by: Pascal Terjan <pterjan@google.com>
---
v2: Made the new function static
v3: Fixed an unused variable
.../staging/rtl8192u/ieee80211/ieee80211_rx.c | 126 +++++++-----------
1 file changed, 49 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index e101f7b13c7e..195d963c4fbb 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -520,55 +520,68 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, struct rx_reorder_entry *p
return true;
}
-void ieee80211_indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb **prxbIndicateArray, u8 index)
+static void indicate_packets(struct ieee80211_device *ieee,
+ struct ieee80211_rxb *rxb)
{
- u8 i = 0, j = 0;
+ struct net_device_stats *stats = &ieee->stats;
+ struct net_device *dev = ieee->dev;
u16 ethertype;
-// if(index > 1)
-// IEEE80211_DEBUG(IEEE80211_DL_REORDER,"%s(): hahahahhhh, We indicate packet from reorder list, index is %u\n",__func__,index);
- for (j = 0; j < index; j++) {
-//added by amy for reorder
- struct ieee80211_rxb *prxb = prxbIndicateArray[j];
- for (i = 0; i < prxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = prxb->subframes[i];
+ u8 i;
+
+ for (i = 0; i < rxb->nr_subframes; i++) {
+ struct sk_buff *sub_skb = rxb->subframes[i];
+
+ if (!sub_skb)
+ continue;
/* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
+ ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
+ if (sub_skb->len >= 8 &&
+ ((!memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) &&
+ ethertype != ETH_P_AARP &&
+ ethertype != ETH_P_IPX) ||
+ !memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE))) {
/* remove RFC1042 or Bridge-Tunnel encapsulation and
* replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- } else {
+ skb_pull(sub_skb, SNAP_SIZE);
+ } else {
/* Leave Ethernet header part of hdr and full payload */
- put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), prxb->dst, ETH_ALEN);
- }
- //stats->rx_packets++;
- //stats->rx_bytes += sub_skb->len;
+ put_unaligned_be16(sub_skb->len, skb_push(sub_skb, 2));
+ }
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->src, ETH_ALEN);
+ memcpy(skb_push(sub_skb, ETH_ALEN), rxb->dst, ETH_ALEN);
+
+ stats->rx_packets++;
+ stats->rx_bytes += sub_skb->len;
+ if (is_multicast_ether_addr(rxb->dst))
+ stats->multicast++;
/* Indicate the packets to upper layer */
- if (sub_skb) {
- sub_skb->protocol = eth_type_trans(sub_skb, ieee->dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = ieee->dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ sub_skb->protocol = eth_type_trans(sub_skb, dev);
+ memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
+ sub_skb->dev = dev;
+ /* 802.11 crc not sufficient */
+ sub_skb->ip_summed = CHECKSUM_NONE;
+ ieee->last_rx_ps_time = jiffies;
+ netif_rx(sub_skb);
+ }
+}
+
+void ieee80211_indicate_packets(struct ieee80211_device *ieee,
+ struct ieee80211_rxb **prxbIndicateArray,
+ u8 index)
+{
+ u8 i;
+
+ for (i = 0; i < index; i++) {
+ struct ieee80211_rxb *prxb = prxbIndicateArray[i];
+
+ indicate_packets(ieee, prxb);
kfree(prxb);
prxb = NULL;
}
}
-
static void RxReorderIndicatePacket(struct ieee80211_device *ieee,
struct ieee80211_rxb *prxb,
struct rx_ts_record *pTS, u16 SeqNum)
@@ -877,7 +890,6 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
u16 fc, type, stype, sc;
struct net_device_stats *stats;
unsigned int frag;
- u16 ethertype;
//added by amy for reorder
u8 TID = 0;
u16 SeqNum = 0;
@@ -1260,47 +1272,7 @@ int ieee80211_rx(struct ieee80211_device *ieee, struct sk_buff *skb,
//added by amy for reorder
if (!ieee->pHTInfo->bCurRxReorderEnable || !pTS) {
-//added by amy for reorder
- for (i = 0; i < rxb->nr_subframes; i++) {
- struct sk_buff *sub_skb = rxb->subframes[i];
-
- if (sub_skb) {
- /* convert hdr + possible LLC headers into Ethernet header */
- ethertype = (sub_skb->data[6] << 8) | sub_skb->data[7];
- if (sub_skb->len >= 8 &&
- ((memcmp(sub_skb->data, rfc1042_header, SNAP_SIZE) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- memcmp(sub_skb->data, bridge_tunnel_header, SNAP_SIZE) == 0)) {
- /* remove RFC1042 or Bridge-Tunnel encapsulation and
- * replace EtherType */
- skb_pull(sub_skb, SNAP_SIZE);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- } else {
- u16 len;
- /* Leave Ethernet header part of hdr and full payload */
- len = be16_to_cpu(htons(sub_skb->len));
- memcpy(skb_push(sub_skb, 2), &len, 2);
- memcpy(skb_push(sub_skb, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(sub_skb, ETH_ALEN), dst, ETH_ALEN);
- }
-
- stats->rx_packets++;
- stats->rx_bytes += sub_skb->len;
- if (is_multicast_ether_addr(dst)) {
- stats->multicast++;
- }
-
- /* Indicate the packets to upper layer */
- sub_skb->protocol = eth_type_trans(sub_skb, dev);
- memset(sub_skb->cb, 0, sizeof(sub_skb->cb));
- sub_skb->dev = dev;
- sub_skb->ip_summed = CHECKSUM_NONE; /* 802.11 crc not sufficient */
- //skb->ip_summed = CHECKSUM_UNNECESSARY; /* 802.11 crc not sufficient */
- ieee->last_rx_ps_time = jiffies;
- netif_rx(sub_skb);
- }
- }
+ indicate_packets(ieee, rxb);
kfree(rxb);
rxb = NULL;
--
2.26.2.761.g0e0b3e54be-goog
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH] staging: rtl8192u: indicate_packets() can be static
2020-05-17 16:58 [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2020-05-17 20:22 ` kbuild test robot
@ 2020-05-17 20:22 ` kbuild test robot
2020-05-17 20:49 ` Joe Perches
2020-05-17 20:25 ` [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2 siblings, 1 reply; 10+ messages in thread
From: kbuild test robot @ 2020-05-17 20:22 UTC (permalink / raw)
To: Pascal Terjan, Greg Kroah-Hartman, devel, linux-kernel
Cc: Pascal Terjan, kbuild-all
Signed-off-by: kbuild test robot <lkp@intel.com>
---
ieee80211_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
index 3309f64be4c94..bceff1ba3d7d4 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
@@ -520,7 +520,7 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, struct rx_reorder_entry *p
return true;
}
-void indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb *rxb)
+static void indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb *rxb)
{
struct net_device_stats *stats = &ieee->stats;
struct net_device *dev = ieee->dev;
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH] staging: rtl8192u: indicate_packets() can be static
2020-05-17 20:22 ` [RFC PATCH] staging: rtl8192u: indicate_packets() can be static kbuild test robot
@ 2020-05-17 20:49 ` Joe Perches
2020-05-18 12:16 ` Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2020-05-17 20:49 UTC (permalink / raw)
To: kbuild test robot, Pascal Terjan, Greg Kroah-Hartman, devel,
linux-kernel
Cc: kbuild-all
On Mon, 2020-05-18 at 04:22 +0800, kbuild test robot wrote:
> Signed-off-by: kbuild test robot <lkp@intel.com>
> ---
This doesn't apply on Linus' tree or -next so perhaps the
robot should put what tree and branch patches like these
are meant to be applied on after the --- line
> ieee80211_rx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> index 3309f64be4c94..bceff1ba3d7d4 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c
> @@ -520,7 +520,7 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, struct rx_reorder_entry *p
> return true;
> }
>
> -void indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb *rxb)
> +static void indicate_packets(struct ieee80211_device *ieee, struct ieee80211_rxb *rxb)
> {
> struct net_device_stats *stats = &ieee->stats;
> struct net_device *dev = ieee->dev;
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] staging: rtl8192u: indicate_packets() can be static
2020-05-17 20:49 ` Joe Perches
@ 2020-05-18 12:16 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-05-18 12:16 UTC (permalink / raw)
To: Joe Perches
Cc: devel, kbuild-all, Greg Kroah-Hartman, linux-kernel, Pascal Terjan
On Sun, May 17, 2020 at 01:49:50PM -0700, Joe Perches wrote:
> On Mon, 2020-05-18 at 04:22 +0800, kbuild test robot wrote:
> > Signed-off-by: kbuild test robot <lkp@intel.com>
> > ---
>
> This doesn't apply on Linus' tree or -next so perhaps the
> robot should put what tree and branch patches like these
> are meant to be applied on after the --- line
>
It's supposed to apply on top of the email which it is replying to.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
2020-05-17 16:58 [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2020-05-17 20:22 ` kbuild test robot
2020-05-17 20:22 ` [RFC PATCH] staging: rtl8192u: indicate_packets() can be static kbuild test robot
@ 2020-05-17 20:25 ` Pascal Terjan
2020-05-18 13:25 ` Dan Carpenter
2 siblings, 1 reply; 10+ messages in thread
From: Pascal Terjan @ 2020-05-17 20:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, devel, linux-kernel
On Sun, 17 May 2020 at 17:58, Pascal Terjan <pterjan@google.com> wrote:
>
> This causes a change in behaviour:
> - stats also get updated when reordering, this seems like it should be
> the case but those lines were commented out.
> - sub_skb NULL check now happens early in both cases, previously it
> happened only after dereferencing it 12 times, so it may not actually
> be needed.
>
Hi,
I actually noticed the same duplicated code (and same late NULL check)
in drivers/staging/rtl8192e/rtllib_rx.c
drivers/staging/rtl8712/rtl8712_recv.c has only one copy of the code
but with the late NULL check
drivers/staging/rtl8188eu/core/rtw_recv.c has only one copy of the
code and doesn't do any NULL check
Now I wonder how to proceed. The code is not great so it would not
feel right to make it reusable.
Should I continue improving it on this driver only first (maybe trying
to reuse ieee80211_data_to_8023_exthdr from net/wireless/util.c for
example)?
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: rtl8192u: Merge almost duplicate code
2020-05-17 20:25 ` [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
@ 2020-05-18 13:25 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-05-18 13:25 UTC (permalink / raw)
To: Pascal Terjan; +Cc: devel, Greg Kroah-Hartman, linux-kernel
On Sun, May 17, 2020 at 09:25:05PM +0100, Pascal Terjan wrote:
> On Sun, 17 May 2020 at 17:58, Pascal Terjan <pterjan@google.com> wrote:
> >
> > This causes a change in behaviour:
> > - stats also get updated when reordering, this seems like it should be
> > the case but those lines were commented out.
> > - sub_skb NULL check now happens early in both cases, previously it
> > happened only after dereferencing it 12 times, so it may not actually
> > be needed.
> >
>
> Hi,
> I actually noticed the same duplicated code (and same late NULL check)
> in drivers/staging/rtl8192e/rtllib_rx.c
> drivers/staging/rtl8712/rtl8712_recv.c has only one copy of the code
> but with the late NULL check
> drivers/staging/rtl8188eu/core/rtw_recv.c has only one copy of the
> code and doesn't do any NULL check
>
> Now I wonder how to proceed. The code is not great so it would not
> feel right to make it reusable.
> Should I continue improving it on this driver only first (maybe trying
> to reuse ieee80211_data_to_8023_exthdr from net/wireless/util.c for
> example)?
It looks like the NULL check could be removed, but it's also fine to
keep it so long as it's not after a NULL dereference.
Do whatever you have the energy to do... It would be nice if people who
fix bugs in these Realtek drivers would check the other drivers as well
but for cleanups basically everyone just works on one driver only.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-19 15:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-17 16:58 [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2020-05-17 20:22 ` kbuild test robot
2020-05-17 20:40 ` [PATCH v2] " Pascal Terjan
2020-05-19 14:16 ` Greg Kroah-Hartman
2020-05-19 15:00 ` [PATCH v3] " Pascal Terjan
2020-05-17 20:22 ` [RFC PATCH] staging: rtl8192u: indicate_packets() can be static kbuild test robot
2020-05-17 20:49 ` Joe Perches
2020-05-18 12:16 ` Dan Carpenter
2020-05-17 20:25 ` [PATCH] staging: rtl8192u: Merge almost duplicate code Pascal Terjan
2020-05-18 13:25 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).