[PATCH v1] realtek/rtlwifi: ensure unicast is reinitialized to false in each iteration

[PATCH v1] realtek/rtlwifi: ensure unicast is reinitialized to false in each iteration

[Les Boys]

Previously, the unicast variable was only initialized once during the function execution. Therefore, if unicast was set to true in the (n-1)th iteration, it would affect the nth iteration. This patch resolves the issue by reinitializing unicast to false at the start of each iteration.

Signed-off-by: LBLaiSiNanHai <lesboyspp43@outlook.com>
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index d08046926..fe7140328 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -752,6 +752,8 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
                        memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
                               sizeof(rx_status));

+      unicast = false;
+
                        if (is_broadcast_ether_addr(hdr->addr1)) {
                                ;/*TODO*/
                        } else if (is_multicast_ether_addr(hdr->addr1)) {
--
2.44.0

RE: [PATCH v1] realtek/rtlwifi: ensure unicast is reinitialized to false in each iteration

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:

Subject should be "[PATCH rtw-next] wifi: rtlwifi: ..."

> 
> Previously, the unicast variable was only initialized once during the function execution. Therefore, if
> unicast was set to true in the (n-1)th iteration, it would affect the nth iteration. This patch resolves
> the issue by reinitializing unicast to false at the start of each iteration.

75 characters per lines.

Does this affect your use actually?

> 
> Signed-off-by: LBLaiSiNanHai <lesboyspp43@outlook.com>

Not sure if this is your real name? And, it should be the same as From field.

> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index d08046926..fe7140328 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -752,6 +752,8 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>                         memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
>                                sizeof(rx_status));
> 
> +      unicast = false;
> +

Indentation is weird.

More, initializer of `unicast` can be removed, and here can be

      if (is_unicast_ether_addr(hdr->addr1)) {
               unicast = true;
               rtlpriv->stats.rxbytesunicast += skb->len;
      } else {
               unicast = false;
      }

(I don't think TODO is still useful for now)

>                         if (is_broadcast_ether_addr(hdr->addr1)) {
>                                 ;/*TODO*/
>                         } else if (is_multicast_ether_addr(hdr->addr1)) {
> --
> 2.44.0

[PATCH rtw-next v2] realtek/rtlwifi: ensure unicast is reinitialized to false in each iteration

[Les Boys]

Previously, the unicast variable was only initialized once during the
function execution. Therefore, if unicast was set to true in the (n-1)th
iteration, it would affect the nth iteration. This patch resolves the issue
by reinitializing unicast to false at the start of each iteration.

CHANGE:
1. Fix indention
2. Use a better way recommanded by maintainer to solve original problem
3. Fix subject
4. Remove useless if and TODO comment

(added CC this time)

Signed-off-by: LBLaiSiNanHai <lesboyspp43@outlook.com>
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index fe7140328..a07de6b57 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -752,16 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
                        memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
                               sizeof(rx_status));

-      unicast = false;
-
-                       if (is_broadcast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else if (is_multicast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else {
+                       if (is_unicast_ether_addr(hdr->addr1)) {
                                unicast = true;
                                rtlpriv->stats.rxbytesunicast += skb->len;
+                       } else {
+                               unicast = false;
                        }
+
                        rtl_is_special_data(hw, skb, false, true);

                        if (ieee80211_is_data(fc)) {
--
2.44.0

RE: [PATCH rtw-next v2] realtek/rtlwifi: ensure unicast is reinitialized to false in each iteration

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:
> Previously, the unicast variable was only initialized once during the
> function execution. Therefore, if unicast was set to true in the (n-1)th
> iteration, it would affect the nth iteration. This patch resolves the issue
> by reinitializing unicast to false at the start of each iteration.
> 
> CHANGE:
> 1. Fix indention
> 2. Use a better way recommanded by maintainer to solve original problem
> 3. Fix subject
> 4. Remove useless if and TODO comment
> 
> (added CC this time)
> 
> Signed-off-by: LBLaiSiNanHai <lesboyspp43@outlook.com>

This should be your real name which is identical to From field 
(I have mentioned this in v1).


---

Move your CHANGE here with three '-' as delimiter 

> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index fe7140328..a07de6b57 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -752,16 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>                         memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
>                                sizeof(rx_status));
> 
> -      unicast = false;

This is your change in v1, please use upstream as base. 

Also, you missed to remove initializer of `unicast` in original code. 

More, check status of your patch in patchwork [1].

[1] https://patchwork.kernel.org/project/linux-wireless/patch/SA2PR10MB4460861A8224AA99EE5ACF6BA6D1A@SA2PR10MB4460.namprd10.prod.outlook.com/

> -
> -                       if (is_broadcast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else if (is_multicast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else {
> +                       if (is_unicast_ether_addr(hdr->addr1)) {
>                                 unicast = true;
>                                 rtlpriv->stats.rxbytesunicast += skb->len;
> +                       } else {
> +                               unicast = false;
>                         }
> +
>                         rtl_is_special_data(hw, skb, false, true);
> 
>                         if (ieee80211_is_data(fc)) {
> --
> 2.44.0

[PATCH rtw-next v3] realtek/rtlwifi: remove unused two if blocks and put unicast set inner if and else block

[Les Boys]

---
CHANGE:
1. Ensure used the origin as base
2. Format fixings

Signed-off-by: Les Boys <lesboyspp43@outlook.com>
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index d08046926..a07de6b57 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
                        memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
                               sizeof(rx_status));

-                       if (is_broadcast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else if (is_multicast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else {
+                       if (is_unicast_ether_addr(hdr->addr1)) {
                                unicast = true;
                                rtlpriv->stats.rxbytesunicast += skb->len;
+                       } else {
+                               unicast = false;
                        }
+
                        rtl_is_special_data(hw, skb, false, true);

                        if (ieee80211_is_data(fc)) {
--
2.44.0

RE: [PATCH rtw-next v3] realtek/rtlwifi: remove unused two if blocks and put unicast set inner if and else block

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:


Not sure why you remove entire commit message?

> 
> ---
> CHANGE:
> 1. Ensure used the origin as base
> 2. Format fixings

Still wrong place. 

There are many examples in public. One of them is [1] I sent before.

[1] https://lore.kernel.org/linux-wireless/910a0ce4bf9c47d89f0da8399ef3927e@realtek.com/T/#md11d111fbdaaa8b37787c7f2197d41b500456f38

> 
> Signed-off-by: Les Boys <lesboyspp43@outlook.com>
> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index d08046926..a07de6b57 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)

I meant you need changes like this.

- bool unicast = false;
+ bool unicast;

>                         memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
>                                sizeof(rx_status));
> 
> -                       if (is_broadcast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else if (is_multicast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else {
> +                       if (is_unicast_ether_addr(hdr->addr1)) {
>                                 unicast = true;
>                                 rtlpriv->stats.rxbytesunicast += skb->len;
> +                       } else {
> +                               unicast = false;
>                         }
> +
>                         rtl_is_special_data(hw, skb, false, true);
> 
>                         if (ieee80211_is_data(fc)) {
> --
> 2.44.0

[PATCH rtw-next v4] realtek/rtlwifi: remove unused two if blocks and put unicast set inner if and else block

[Les Boys]

Removed unused if blocks and put the unicast set sentence into the
if/else block.

Signed-off-by: Les Boys <lesboyspp43@outlook.com>
---
CHANGE:
1. Move change block to correct location;
2. Removed initalization of unicast on the begin of function.
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index d08046926..eda3b80df 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -652,7 +652,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
        unsigned int count = rtlpci->rxringcount;
        u8 own;
        u8 tmp_one;
-       bool unicast = false;
+       bool unicast;
        u8 hw_queue = 0;
        unsigned int rx_remained_cnt = 0;
        struct rtl_stats stats = {
@@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
                        memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
                               sizeof(rx_status));

-                       if (is_broadcast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else if (is_multicast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else {
+                       if (is_unicast_ether_addr(hdr->addr1)) {
                                unicast = true;
                                rtlpriv->stats.rxbytesunicast += skb->len;
+                       } else {
+                               unicast = false;
                        }
+
                        rtl_is_special_data(hw, skb, false, true);

                        if (ieee80211_is_data(fc)) {
--
2.44.0

RE: [PATCH rtw-next v4] realtek/rtlwifi: remove unused two if blocks and put unicast set inner if and else block

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:
> Removed unused if blocks and put the unicast set sentence into the
> if/else block.

Commit message is always to explain why we need this patch, but this 
describes what this patch changes.

I think subject and commit message of v1 is more suitable than v4.

More, v5 subject should be "[PATCH rtw-next v5] wifi: rtlwifi: ..."
(please just copy & paste)

Otherwise, patch content is fine to me.

> 
> Signed-off-by: Les Boys <lesboyspp43@outlook.com>
> ---
> CHANGE:
> 1. Move change block to correct location;
> 2. Removed initalization of unicast on the begin of function.
> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index d08046926..eda3b80df 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -652,7 +652,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>         unsigned int count = rtlpci->rxringcount;
>         u8 own;
>         u8 tmp_one;
> -       bool unicast = false;
> +       bool unicast;
>         u8 hw_queue = 0;
>         unsigned int rx_remained_cnt = 0;
>         struct rtl_stats stats = {
> @@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>                         memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
>                                sizeof(rx_status));
> 
> -                       if (is_broadcast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else if (is_multicast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else {
> +                       if (is_unicast_ether_addr(hdr->addr1)) {
>                                 unicast = true;
>                                 rtlpriv->stats.rxbytesunicast += skb->len;
> +                       } else {
> +                               unicast = false;
>                         }
> +
>                         rtl_is_special_data(hw, skb, false, true);
> 
>                         if (ieee80211_is_data(fc)) {
> --
> 2.44.0

回复: [PATCH rtw-next v5] realtek: rtlwifi: remove dead code and ensure unicast is always set explicitly in every iteration

[Les Boys]

Removed dead code and ensured unicast is always set explicitly in every
iteration to ensure the unicast of (n-1)th iteration will not apply to nth
iteration, the previous code checked multicast and broadcast but no any
logic if the state is multicast or broadcast, this patch removed them and
check it is unicast or not only, and removed the initalizer of unicast
because all possible path will set unicast.

Signed-off-by: Les Boys <lesboyspp43@outlook.com>
---
CHANGE:
1. Optimize commit message
2. Change subject
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index d08046926..eda3b80df 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -652,7 +652,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
        unsigned int count = rtlpci->rxringcount;
        u8 own;
        u8 tmp_one;
-       bool unicast = false;
+       bool unicast;
        u8 hw_queue = 0;
        unsigned int rx_remained_cnt = 0;
        struct rtl_stats stats = {
@@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
                        memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
                               sizeof(rx_status));

-                       if (is_broadcast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else if (is_multicast_ether_addr(hdr->addr1)) {
-                               ;/*TODO*/
-                       } else {
+                       if (is_unicast_ether_addr(hdr->addr1)) {
                                unicast = true;
                                rtlpriv->stats.rxbytesunicast += skb->len;
+                       } else {
+                               unicast = false;
                        }
+
                        rtl_is_special_data(hw, skb, false, true);

                        if (ieee80211_is_data(fc)) {
--
2.44.0

RE: [PATCH rtw-next v5] realtek: rtlwifi: remove dead code and ensure unicast is always set explicitly in every iteration

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:

Subject should be "wifi: rtlwifi: ...". I can change it during getting merged.
No need to resend because of this.

> 
> Removed dead code and ensured unicast is always set explicitly in every
> iteration to ensure the unicast of (n-1)th iteration will not apply to nth
> iteration, the previous code checked multicast and broadcast but no any
> logic if the state is multicast or broadcast, this patch removed them and
> check it is unicast or not only, and removed the initalizer of unicast
> because all possible path will set unicast.
> 
> Signed-off-by: Les Boys <lesboyspp43@outlook.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
> CHANGE:
> 1. Optimize commit message
> 2. Change subject

Change log should point out changes of v2, v3, v4, ... 

No need to resend because of this neither.

> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index d08046926..eda3b80df 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -652,7 +652,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>         unsigned int count = rtlpci->rxringcount;
>         u8 own;
>         u8 tmp_one;
> -       bool unicast = false;
> +       bool unicast;
>         u8 hw_queue = 0;
>         unsigned int rx_remained_cnt = 0;
>         struct rtl_stats stats = {
> @@ -752,14 +752,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
>                         memcpy(IEEE80211_SKB_RXCB(skb), &rx_status,
>                                sizeof(rx_status));
> 
> -                       if (is_broadcast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else if (is_multicast_ether_addr(hdr->addr1)) {
> -                               ;/*TODO*/
> -                       } else {
> +                       if (is_unicast_ether_addr(hdr->addr1)) {
>                                 unicast = true;
>                                 rtlpriv->stats.rxbytesunicast += skb->len;
> +                       } else {
> +                               unicast = false;
>                         }
> +
>                         rtl_is_special_data(hw, skb, false, true);
> 
>                         if (ieee80211_is_data(fc)) {
> --
> 2.44.0

Re: [PATCH rtw-next v5] realtek: rtlwifi: remove dead code and ensure unicast is always set explicitly in every iteration

[Ping-Ke Shih]

Les Boys <lesboyspp43@outlook.com> wrote:

> Removed dead code and ensured unicast is always set explicitly in every
> iteration to ensure the unicast of (n-1)th iteration will not apply to nth
> iteration, the previous code checked multicast and broadcast but no any
> logic if the state is multicast or broadcast, this patch removed them and
> check it is unicast or not only, and removed the initalizer of unicast
> because all possible path will set unicast.
> 
> Signed-off-by: Les Boys <lesboyspp43@outlook.com>
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>

I can't apply this patch because of subject encoding and patch context
(might converted from tab to spaces). Please resend this patch by
'git send-email' to avoid format problem.

Set patchset state to Changes Requested

[PATCH rtw-next v5] realtek: rtlwifi: remove dead code and ensure unicast is always set explicitly in every iteration

---
https://github.com/pkshih/rtw.git