linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/3] net: usb: ax88179_178a: Enable AX_RX_CTL_IPE only when NET_IP_ALIGN != 0
       [not found] <20221007142038.2814378-1-nothingstopsme@hotmail.com>
@ 2022-10-07 14:20 ` Chun-Chao Yen
  2023-02-28  9:44   ` [PATCH 2/3 RFC] " Yen ChunChao
  2022-10-07 14:20 ` [PATCH 3/3] net: usb: ax88179_178a: Allow live update of devices' mac address Chun-Chao Yen
  1 sibling, 1 reply; 4+ messages in thread
From: Chun-Chao Yen @ 2022-10-07 14:20 UTC (permalink / raw)
  To: davem; +Cc: linux-usb

Problem Description:
According to the comments in the hardware vendor's outdated source code
[1], AX_RX_CTL_IPE signals the hardware to do 32-bit(4-byte) IP header
alignment; such alignment is equivalent to the concept of NET_IP_ALIGN = 2
(an extra 2-byte "pesudo header" at the beginning of each etherner frame),
as described in skbuff.h.

In the current implementation, however, AX_RX_CTL_IPE is always enabled
regardless of the value of NET_IP_ALIGN; this can introduce waste in many
aspects, such as
1. hardware internal resource
2. USB bandwidth
3. host memory
4. cpu cycles (for updating frame start pointers and frame size variables)
when alignment is completely unnecessary, i.e. NET_IP_ALIGN = 0.

Solution:
Enable AX_RX_CTL_IPE and process pesudo headers only when NET_IP_ALIGN !=
0.

Verification:
Only tested on a platform where NET_IP_ALIGN = 0, with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet

References:
[1] https://www.asix.com.tw/en/support/download/file/120 (AX88179 USB3.0 to
10/100/1000M Gigabit Ethernet Controller, version 1.20.0)

Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
 drivers/net/usb/ax88179_178a.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index b50748b3776c..96ede3a131d4 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -858,7 +858,10 @@ static void ax88179_set_multicast(struct net_device *net)
 	struct ax88179_data *data = dev->driver_priv;
 	u8 *m_filter = ((u8 *)dev->data);
 
-	data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB | AX_RX_CTL_IPE);
+	data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB);
+
+	if (NET_IP_ALIGN)
+		data->rxctl |= AX_RX_CTL_IPE;
 
 	if (net->flags & IFF_PROMISC) {
 		data->rxctl |= AX_RX_CTL_PRO;
@@ -1424,7 +1427,7 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 		/* Check CRC or runt packet */
 		if ((*pkt_hdr & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) ||
-		    pkt_len < 2 + ETH_HLEN) {
+		    pkt_len < (NET_IP_ALIGN ? 2 : 0) + ETH_HLEN) {
 			dev->net->stats.rx_errors++;
 			goto advance_data_ptr;
 		}
@@ -1438,8 +1441,13 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		}
 		skb_trim(ax_skb, pkt_len);
 
-		/* Skip IP alignment pseudo header */
-		skb_pull(ax_skb, 2);
+		if (NET_IP_ALIGN) {
+			/* Skip the pseudo header, 2 bytes at the start of each
+			 * ethernet frame, resulting from hardware 4-byte
+			 * IP header alignment (triggered by AX_RX_CTL_IPE)
+			 */
+			skb_pull(ax_skb, 2);
+		}
 
 		ax_skb->truesize = SKB_TRUESIZE(pkt_len);
 		ax88179_rx_checksum(ax_skb, pkt_hdr);
@@ -1609,8 +1617,10 @@ static int ax88179_reset(struct usbnet *dev)
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, tmp);
 
 	/* Configure RX control register => start operation */
-	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
+	*tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_START |
 		 AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+	if (NET_IP_ALIGN)
+		*tmp16 |= AX_RX_CTL_IPE;
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16);
 
 	*tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL |
-- 
2.32.0


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

* [PATCH 3/3] net: usb: ax88179_178a: Allow live update of devices' mac address
       [not found] <20221007142038.2814378-1-nothingstopsme@hotmail.com>
  2022-10-07 14:20 ` [PATCH 2/3] net: usb: ax88179_178a: Enable AX_RX_CTL_IPE only when NET_IP_ALIGN != 0 Chun-Chao Yen
@ 2022-10-07 14:20 ` Chun-Chao Yen
  2023-02-28  9:44   ` [PATCH 3/3 RFC] " Yen ChunChao
  1 sibling, 1 reply; 4+ messages in thread
From: Chun-Chao Yen @ 2022-10-07 14:20 UTC (permalink / raw)
  To: davem; +Cc: linux-usb

Problem Description:
Live update of devices' mac address is currently blocked by this driver, as
it requires the evaluation of netif_running() given the corresponding
device being false. While appearing a harmless check, it can be disruptive
in some networking configurations, such as "Link Aggregation" operated in
active-backup mode with fail_over_mac=follow, where the mac address of a
device will be updated dynamically even when it is already up and running.

Solution:
Remove the check of netif_running() in ax88179_set_mac_addr(), so that the
update procedure can proceed irrespective of the boolean status returned by
netif_running().

Verification:
Only tested with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet

Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
 drivers/net/usb/ax88179_178a.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 96ede3a131d4..84016e0567d4 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -958,8 +958,6 @@ static int ax88179_set_mac_addr(struct net_device *net, void *p)
 	struct sockaddr *addr = p;
 	int ret;
 
-	if (netif_running(net))
-		return -EBUSY;
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
-- 
2.32.0


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

* [PATCH 2/3 RFC] net: usb: ax88179_178a: Enable AX_RX_CTL_IPE only when NET_IP_ALIGN != 0
  2022-10-07 14:20 ` [PATCH 2/3] net: usb: ax88179_178a: Enable AX_RX_CTL_IPE only when NET_IP_ALIGN != 0 Chun-Chao Yen
@ 2023-02-28  9:44   ` Yen ChunChao
  0 siblings, 0 replies; 4+ messages in thread
From: Yen ChunChao @ 2023-02-28  9:44 UTC (permalink / raw)
  To: davem; +Cc: pabeni, linux-usb

Problem Description:
According to the comments in the hardware vendor's outdated source code
[1], AX_RX_CTL_IPE signals the hardware to do 32-bit(4-byte) IP header
alignment; such alignment is equivalent to the concept of NET_IP_ALIGN = 2
(an extra 2-byte "pesudo header" at the beginning of each etherner frame),
as described in skbuff.h.

In the current implementation, however, AX_RX_CTL_IPE is always enabled
regardless of the value of NET_IP_ALIGN; this can introduce waste in many
aspects, such as
1. hardware internal resource
2. USB bandwidth
3. host memory
4. cpu cycles (for updating frame start pointers and frame size variables)
when alignment is completely unnecessary, i.e. NET_IP_ALIGN = 0.

Solution:
Enable AX_RX_CTL_IPE and process pesudo headers only when NET_IP_ALIGN !=
0.

Verification:
Only tested on a platform where NET_IP_ALIGN = 0, with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet

References:
[1] https://www.asix.com.tw/en/support/download/file/120 (AX88179 USB3.0 to
10/100/1000M Gigabit Ethernet Controller, version 1.20.0)

Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
This is the same patch as https://rb.gy/ieil2d sent in Oct. 2022.
I just would like to know the current state of this patch.
Has it been rejected or still under review?

Thanks

 drivers/net/usb/ax88179_178a.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index b50748b3776c..96ede3a131d4 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -858,7 +858,10 @@ static void ax88179_set_multicast(struct net_device *net)
         struct ax88179_data *data = dev->driver_priv;
         u8 *m_filter = ((u8 *)dev->data);
 
-       data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB | AX_RX_CTL_IPE);
+       data->rxctl = (AX_RX_CTL_START | AX_RX_CTL_AB);
+
+       if (NET_IP_ALIGN)
+               data->rxctl |= AX_RX_CTL_IPE;
 
         if (net->flags & IFF_PROMISC) {
                 data->rxctl |= AX_RX_CTL_PRO;
@@ -1424,7 +1427,7 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
                 /* Check CRC or runt packet */
                 if ((*pkt_hdr & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) ||
-                   pkt_len < 2 + ETH_HLEN) {
+                   pkt_len < (NET_IP_ALIGN ? 2 : 0) + ETH_HLEN) {
                         dev->net->stats.rx_errors++;
                         goto advance_data_ptr;
                 }
@@ -1438,8 +1441,13 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
                 }
                 skb_trim(ax_skb, pkt_len);
 
-               /* Skip IP alignment pseudo header */
-               skb_pull(ax_skb, 2);
+               if (NET_IP_ALIGN) {
+                       /* Skip the pseudo header, 2 bytes at the start of each
+                        * ethernet frame, resulting from hardware 4-byte
+                        * IP header alignment (triggered by AX_RX_CTL_IPE)
+                        */
+                       skb_pull(ax_skb, 2);
+               }
 
                 ax_skb->truesize = SKB_TRUESIZE(pkt_len);
                 ax88179_rx_checksum(ax_skb, pkt_hdr);
@@ -1609,8 +1617,10 @@ static int ax88179_reset(struct usbnet *dev)
         ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, tmp);
 
         /* Configure RX control register => start operation */
-       *tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_IPE | AX_RX_CTL_START |
+       *tmp16 = AX_RX_CTL_DROPCRCERR | AX_RX_CTL_START |
                  AX_RX_CTL_AP | AX_RX_CTL_AMALL | AX_RX_CTL_AB;
+       if (NET_IP_ALIGN)
+               *tmp16 |= AX_RX_CTL_IPE;
         ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_RX_CTL, 2, 2, tmp16);
 
         *tmp = AX_MONITOR_MODE_PMETYPE | AX_MONITOR_MODE_PMEPOL |
-- 
2.32.0

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

* [PATCH 3/3 RFC] net: usb: ax88179_178a: Allow live update of devices' mac address
  2022-10-07 14:20 ` [PATCH 3/3] net: usb: ax88179_178a: Allow live update of devices' mac address Chun-Chao Yen
@ 2023-02-28  9:44   ` Yen ChunChao
  0 siblings, 0 replies; 4+ messages in thread
From: Yen ChunChao @ 2023-02-28  9:44 UTC (permalink / raw)
  To: davem; +Cc: pabeni, linux-usb

Problem Description:
Live update of devices' mac address is currently blocked by this driver, as
it requires the evaluation of netif_running() given the corresponding
device being false. While appearing a harmless check, it can be disruptive
in some networking configurations, such as "Link Aggregation" operated in
active-backup mode with fail_over_mac=follow, where the mac address of a
device will be updated dynamically even when it is already up and running.

Solution:
Remove the check of netif_running() in ax88179_set_mac_addr(), so that the
update procedure can proceed irrespective of the boolean status returned by
netif_running().

Verification:
Only tested with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet

Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
This is the same patch as https://rb.gy/nxendp sent in Oct. 2022.
I just would like to know the current state of this patch.
Has it been rejected or still under review?

Thanks

 drivers/net/usb/ax88179_178a.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 96ede3a131d4..84016e0567d4 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -958,8 +958,6 @@ static int ax88179_set_mac_addr(struct net_device *net, void *p)
         struct sockaddr *addr = p;
         int ret;
 
-       if (netif_running(net))
-               return -EBUSY;
         if (!is_valid_ether_addr(addr->sa_data))
                 return -EADDRNOTAVAIL;
 
-- 
2.32.0

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

end of thread, other threads:[~2023-02-28  9:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20221007142038.2814378-1-nothingstopsme@hotmail.com>
2022-10-07 14:20 ` [PATCH 2/3] net: usb: ax88179_178a: Enable AX_RX_CTL_IPE only when NET_IP_ALIGN != 0 Chun-Chao Yen
2023-02-28  9:44   ` [PATCH 2/3 RFC] " Yen ChunChao
2022-10-07 14:20 ` [PATCH 3/3] net: usb: ax88179_178a: Allow live update of devices' mac address Chun-Chao Yen
2023-02-28  9:44   ` [PATCH 3/3 RFC] " Yen ChunChao

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).