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 <[email protected]>
---
.../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
Signed-off-by: kbuild test robot <[email protected]>
---
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;
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 <[email protected]>
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/[email protected]
On Sun, 17 May 2020 at 17:58, Pascal Terjan <[email protected]> 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)?
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 <[email protected]>
---
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
On Mon, 2020-05-18 at 04:22 +0800, kbuild test robot wrote:
> Signed-off-by: kbuild test robot <[email protected]>
> ---
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;
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 <[email protected]>
> > ---
>
> 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
On Sun, May 17, 2020 at 09:25:05PM +0100, Pascal Terjan wrote:
> On Sun, 17 May 2020 at 17:58, Pascal Terjan <[email protected]> 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
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 <[email protected]>
This patch adds a new compiler warning, which no patch should do.
Please fix that up and resend.
thanks,
greg k-h
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 <[email protected]>
---
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