* [PATCH 1/3] net: usb: ax88179_178a: Enable FLAG_MULTI_PACKET to improve tx stability
@ 2022-10-07 14:20 Chun-Chao Yen
[not found] ` <ME3P282MB2827E9AD698A7F925EB1E9BAD1AC9@ME3P282MB2827.AUSP282.PROD.OUTLOOK.COM>
0 siblings, 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:
The current way of handling the boundary case in tx, i.e. adding one-byte
padding when the size of an outbound packet is a multiple of the maximum
frame size used for USB bulk transfer, does not work with the hardware.
This can be shown by running a loading test via iperf with this hardware as
the sender; one can then tune the iperf parameters on the sender side (e.g.
"-M 510" in my testing environment) so that sent packets frequently hit the
boundary condition, and observe a significant drop in the transmission
rate. In the worst case (often after a long run), the hardware can stop
functioning (can not send or receive packets anymore, albeit the
corresponding network interface is still reported present by ifconfig).
It is also believed that this problem is highly relevant to this bug [1].
Solution:
Enable FLAG_MULTI_PACKET and modify both ax88179_rx_fixup() and
ax88179_tx_fixup() accordingly.
Rationale:
When FLAG_MULTI_PACKET is enabled (and FLAG_SEND_ZLP is off, which is the
case for this driver), usbnet will skip padding, and trust that each
sk_buff returned from the mini-driver's tx_fixup function has been taken
care of to cater for the requirement of its target hardware. That
mechanism allows this mini-driver to send, even when the boundary condition
is detected, "untampered" packets (no padding, no extra flags, as if in the
normal case) that the hardware accepts, and therefore resolves this
problem.
Note that rather than being viewed as a workaround, enabling
FLAG_MULTI_PACKET is intended to better match the overall behaviour of the
hardware, as it also echos well the rx procedure conducting multiple-packet
extraction from a single sk_buff in ax88179_rx_fixup().
Verification:
Only tested with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet
References:
[1] https://bugzilla.kernel.org/show_bug.cgi?id=212731
Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
drivers/net/usb/ax88179_178a.c | 62 ++++++++++++++--------------------
1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index aff39bf3161d..b50748b3776c 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1426,58 +1426,48 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
if ((*pkt_hdr & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) ||
pkt_len < 2 + ETH_HLEN) {
dev->net->stats.rx_errors++;
- skb_pull(skb, pkt_len_plus_padd);
- continue;
- }
-
- /* last packet */
- if (pkt_len_plus_padd == skb->len) {
- skb_trim(skb, pkt_len);
-
- /* Skip IP alignment pseudo header */
- skb_pull(skb, 2);
-
- skb->truesize = SKB_TRUESIZE(pkt_len_plus_padd);
- ax88179_rx_checksum(skb, pkt_hdr);
- return 1;
+ goto advance_data_ptr;
}
ax_skb = skb_clone(skb, GFP_ATOMIC);
- if (!ax_skb)
- return 0;
+ if (!ax_skb) {
+ /* Report a packet droped, and continue parsing the rest
+ */
+ dev->net->stats.rx_dropped++;
+ goto advance_data_ptr;
+ }
skb_trim(ax_skb, pkt_len);
/* Skip IP alignment pseudo header */
skb_pull(ax_skb, 2);
- skb->truesize = pkt_len_plus_padd +
- SKB_DATA_ALIGN(sizeof(struct sk_buff));
+ ax_skb->truesize = SKB_TRUESIZE(pkt_len);
ax88179_rx_checksum(ax_skb, pkt_hdr);
usbnet_skb_return(dev, ax_skb);
+advance_data_ptr:
skb_pull(skb, pkt_len_plus_padd);
}
- return 0;
+ return 1;
}
static struct sk_buff *
ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
u32 tx_hdr1, tx_hdr2;
- int frame_size = dev->maxpacket;
int headroom;
void *ptr;
tx_hdr1 = skb->len;
tx_hdr2 = skb_shinfo(skb)->gso_size; /* Set TSO mss */
- if (((skb->len + 8) % frame_size) == 0)
- tx_hdr2 |= 0x80008000; /* Enable padding */
headroom = skb_headroom(skb) - 8;
- if ((dev->net->features & NETIF_F_SG) && skb_linearize(skb))
+ if ((dev->net->features & NETIF_F_SG) && skb_linearize(skb)) {
+ dev_kfree_skb_any(skb);
return NULL;
+ }
if ((skb_header_cloned(skb) || headroom < 0) &&
pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
@@ -1680,7 +1670,7 @@ static const struct driver_info ax88179_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1693,7 +1683,7 @@ static const struct driver_info ax88178a_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1706,7 +1696,7 @@ static const struct driver_info cypress_GX3_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1719,7 +1709,7 @@ static const struct driver_info dlink_dub1312_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1732,7 +1722,7 @@ static const struct driver_info sitecom_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1745,7 +1735,7 @@ static const struct driver_info samsung_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1758,7 +1748,7 @@ static const struct driver_info lenovo_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1771,7 +1761,7 @@ static const struct driver_info belkin_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1784,7 +1774,7 @@ static const struct driver_info toshiba_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1797,7 +1787,7 @@ static const struct driver_info mct_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1810,7 +1800,7 @@ static const struct driver_info at_umc2000_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1823,7 +1813,7 @@ static const struct driver_info at_umc200_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1836,7 +1826,7 @@ static const struct driver_info at_umc2000sp_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 1/3 RFC] net: usb: ax88179_178a: Enable FLAG_MULTI_PACKET to improve tx stability
[not found] ` <ME3P282MB2827E9AD698A7F925EB1E9BAD1AC9@ME3P282MB2827.AUSP282.PROD.OUTLOOK.COM>
@ 2023-02-28 9:44 ` Yen ChunChao
2023-02-28 11:25 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Yen ChunChao @ 2023-02-28 9:44 UTC (permalink / raw)
To: davem; +Cc: pabeni, linux-usb
Problem Description:
The current way of handling the boundary case in tx, i.e. adding one-byte
padding when the size of an outbound packet is a multiple of the maximum
frame size used for USB bulk transfer, does not work with the hardware.
This can be shown by running a loading test via iperf with this hardware as
the sender; one can then tune the iperf parameters on the sender side (e.g.
"-M 510" in my testing environment) so that sent packets frequently hit the
boundary condition, and observe a significant drop in the transmission
rate. In the worst case (often after a long run), the hardware can stop
functioning (can not send or receive packets anymore, albeit the
corresponding network interface is still reported present by ifconfig).
It is also believed that this problem is highly relevant to this bug [1].
Solution:
Enable FLAG_MULTI_PACKET and modify both ax88179_rx_fixup() and
ax88179_tx_fixup() accordingly.
Rationale:
When FLAG_MULTI_PACKET is enabled (and FLAG_SEND_ZLP is off, which is the
case for this driver), usbnet will skip padding, and trust that each
sk_buff returned from the mini-driver's tx_fixup function has been taken
care of to cater for the requirement of its target hardware. That
mechanism allows this mini-driver to send, even when the boundary condition
is detected, "untampered" packets (no padding, no extra flags, as if in the
normal case) that the hardware accepts, and therefore resolves this
problem.
Note that rather than being viewed as a workaround, enabling
FLAG_MULTI_PACKET is intended to better match the overall behaviour of the
hardware, as it also echos well the rx procedure conducting multiple-packet
extraction from a single sk_buff in ax88179_rx_fixup().
Verification:
Only tested with this device:
0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet
References:
[1] https://bugzilla.kernel.org/show_bug.cgi?id=212731
Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
---
This is the same patch as https://rb.gy/199s5m 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 | 62 ++++++++++++++--------------------
1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index aff39bf3161d..b50748b3776c 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1426,58 +1426,48 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
if ((*pkt_hdr & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) ||
pkt_len < 2 + ETH_HLEN) {
dev->net->stats.rx_errors++;
- skb_pull(skb, pkt_len_plus_padd);
- continue;
- }
-
- /* last packet */
- if (pkt_len_plus_padd == skb->len) {
- skb_trim(skb, pkt_len);
-
- /* Skip IP alignment pseudo header */
- skb_pull(skb, 2);
-
- skb->truesize = SKB_TRUESIZE(pkt_len_plus_padd);
- ax88179_rx_checksum(skb, pkt_hdr);
- return 1;
+ goto advance_data_ptr;
}
ax_skb = skb_clone(skb, GFP_ATOMIC);
- if (!ax_skb)
- return 0;
+ if (!ax_skb) {
+ /* Report a packet droped, and continue parsing the rest
+ */
+ dev->net->stats.rx_dropped++;
+ goto advance_data_ptr;
+ }
skb_trim(ax_skb, pkt_len);
/* Skip IP alignment pseudo header */
skb_pull(ax_skb, 2);
- skb->truesize = pkt_len_plus_padd +
- SKB_DATA_ALIGN(sizeof(struct sk_buff));
+ ax_skb->truesize = SKB_TRUESIZE(pkt_len);
ax88179_rx_checksum(ax_skb, pkt_hdr);
usbnet_skb_return(dev, ax_skb);
+advance_data_ptr:
skb_pull(skb, pkt_len_plus_padd);
}
- return 0;
+ return 1;
}
static struct sk_buff *
ax88179_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
u32 tx_hdr1, tx_hdr2;
- int frame_size = dev->maxpacket;
int headroom;
void *ptr;
tx_hdr1 = skb->len;
tx_hdr2 = skb_shinfo(skb)->gso_size; /* Set TSO mss */
- if (((skb->len + 8) % frame_size) == 0)
- tx_hdr2 |= 0x80008000; /* Enable padding */
headroom = skb_headroom(skb) - 8;
- if ((dev->net->features & NETIF_F_SG) && skb_linearize(skb))
+ if ((dev->net->features & NETIF_F_SG) && skb_linearize(skb)) {
+ dev_kfree_skb_any(skb);
return NULL;
+ }
if ((skb_header_cloned(skb) || headroom < 0) &&
pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
@@ -1680,7 +1670,7 @@ static const struct driver_info ax88179_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1693,7 +1683,7 @@ static const struct driver_info ax88178a_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1706,7 +1696,7 @@ static const struct driver_info cypress_GX3_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1719,7 +1709,7 @@ static const struct driver_info dlink_dub1312_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1732,7 +1722,7 @@ static const struct driver_info sitecom_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1745,7 +1735,7 @@ static const struct driver_info samsung_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1758,7 +1748,7 @@ static const struct driver_info lenovo_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1771,7 +1761,7 @@ static const struct driver_info belkin_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1784,7 +1774,7 @@ static const struct driver_info toshiba_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1797,7 +1787,7 @@ static const struct driver_info mct_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1810,7 +1800,7 @@ static const struct driver_info at_umc2000_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1823,7 +1813,7 @@ static const struct driver_info at_umc200_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
@@ -1836,7 +1826,7 @@ static const struct driver_info at_umc2000sp_info = {
.link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.stop = ax88179_stop,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_MULTI_PACKET,
.rx_fixup = ax88179_rx_fixup,
.tx_fixup = ax88179_tx_fixup,
};
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3 RFC] net: usb: ax88179_178a: Enable FLAG_MULTI_PACKET to improve tx stability
2023-02-28 9:44 ` [PATCH 1/3 RFC] " Yen ChunChao
@ 2023-02-28 11:25 ` Greg KH
2023-02-28 12:36 ` Yen Chun-Chao
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2023-02-28 11:25 UTC (permalink / raw)
To: Yen ChunChao; +Cc: davem, pabeni, linux-usb
On Tue, Feb 28, 2023 at 09:44:43AM +0000, Yen ChunChao wrote:
> Problem Description:
> The current way of handling the boundary case in tx, i.e. adding one-byte
> padding when the size of an outbound packet is a multiple of the maximum
> frame size used for USB bulk transfer, does not work with the hardware.
> This can be shown by running a loading test via iperf with this hardware as
> the sender; one can then tune the iperf parameters on the sender side (e.g.
> "-M 510" in my testing environment) so that sent packets frequently hit the
> boundary condition, and observe a significant drop in the transmission
> rate. In the worst case (often after a long run), the hardware can stop
> functioning (can not send or receive packets anymore, albeit the
> corresponding network interface is still reported present by ifconfig).
>
> It is also believed that this problem is highly relevant to this bug [1].
>
> Solution:
> Enable FLAG_MULTI_PACKET and modify both ax88179_rx_fixup() and
> ax88179_tx_fixup() accordingly.
>
> Rationale:
> When FLAG_MULTI_PACKET is enabled (and FLAG_SEND_ZLP is off, which is the
> case for this driver), usbnet will skip padding, and trust that each
> sk_buff returned from the mini-driver's tx_fixup function has been taken
> care of to cater for the requirement of its target hardware. That
> mechanism allows this mini-driver to send, even when the boundary condition
> is detected, "untampered" packets (no padding, no extra flags, as if in the
> normal case) that the hardware accepts, and therefore resolves this
> problem.
>
> Note that rather than being viewed as a workaround, enabling
> FLAG_MULTI_PACKET is intended to better match the overall behaviour of the
> hardware, as it also echos well the rx procedure conducting multiple-packet
> extraction from a single sk_buff in ax88179_rx_fixup().
>
> Verification:
> Only tested with this device:
> 0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet
>
> References:
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=212731
>
> Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
> ---
> This is the same patch as https://rb.gy/199s5m sent in Oct. 2022.
> I just would like to know the current state of this patch.
> Has it been rejected or still under review?
Patches marked with "RFC" are never to be applied, what needs to be done
on this in order for you to feel comfortable enough to submit it for
actualy inclusion?
Please do that and then send it out properly.
Also, I think vger still bans hotmail.com emails, sorry, please use a
better email provider if you wish to contribute to the kernel.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH 1/3 RFC] net: usb: ax88179_178a: Enable FLAG_MULTI_PACKET to improve tx stability
2023-02-28 11:25 ` Greg KH
@ 2023-02-28 12:36 ` Yen Chun-Chao
0 siblings, 0 replies; 4+ messages in thread
From: Yen Chun-Chao @ 2023-02-28 12:36 UTC (permalink / raw)
To: Greg KH; +Cc: davem, pabeni, linux-usb
>On Tue, Feb 28, 2023 at 09:44:43AM +0000, Yen ChunChao wrote:
>> Problem Description:
>> The current way of handling the boundary case in tx, i.e. adding one-byte
>> padding when the size of an outbound packet is a multiple of the maximum
>> frame size used for USB bulk transfer, does not work with the hardware.
>> This can be shown by running a loading test via iperf with this hardware as
>> the sender; one can then tune the iperf parameters on the sender side (e.g.
>> "-M 510" in my testing environment) so that sent packets frequently hit the
>> boundary condition, and observe a significant drop in the transmission
>> rate. In the worst case (often after a long run), the hardware can stop
>> functioning (can not send or receive packets anymore, albeit the
>> corresponding network interface is still reported present by ifconfig).
>>
>> It is also believed that this problem is highly relevant to this bug [1].
>>
>> Solution:
>> Enable FLAG_MULTI_PACKET and modify both ax88179_rx_fixup() and
>> ax88179_tx_fixup() accordingly.
>>
>> Rationale:
>> When FLAG_MULTI_PACKET is enabled (and FLAG_SEND_ZLP is off, which is the
>> case for this driver), usbnet will skip padding, and trust that each
>> sk_buff returned from the mini-driver's tx_fixup function has been taken
>> care of to cater for the requirement of its target hardware. That
>> mechanism allows this mini-driver to send, even when the boundary condition
>> is detected, "untampered" packets (no padding, no extra flags, as if in the
>> normal case) that the hardware accepts, and therefore resolves this
>> problem.
>>
>> Note that rather than being viewed as a workaround, enabling
>> FLAG_MULTI_PACKET is intended to better match the overall behaviour of the
>> hardware, as it also echos well the rx procedure conducting multiple-packet
>> extraction from a single sk_buff in ax88179_rx_fixup().
>>
>> Verification:
>> Only tested with this device:
>> 0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet
>>
>> References:
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=212731
>>
>> Signed-off-by: Chun-Chao Yen <nothingstopsme@hotmail.com>
>> ---
>> This is the same patch as https://rb.gy/199s5m sent in Oct. 2022.
>> I just would like to know the current state of this patch.
>> Has it been rejected or still under review?
>
>Patches marked with "RFC" are never to be applied, what needs to be done
>on this in order for you to feel comfortable enough to submit it for
>actualy inclusion?
Sorry for the misuse of "RFC". I thought that tag could be used to ask
questions about the patches sent previously when no change is added.
As you might have noticed in the message after "---", my intention is to
check whether my patches are still under review or not, because I have
not received any feedback/comments since they were sent out in Oct. 2022.
>
>Please do that and then send it out properly.
>
>Also, I think vger still bans hotmail.com emails, sorry, please use a
>better email provider if you wish to contribute to the kernel.
Thank you very much for this information.
Since my patch mails have appeared at https://lore.kernel.org/linux-usb,
should I treat all of them (sent from hotmail.com) as invalid, and
send the same patch series again using another email provider,
in the way like I am sending them for the first time?
>
>thanks,
>
>greg k-h
Best,
Chao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-28 12:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-07 14:20 [PATCH 1/3] net: usb: ax88179_178a: Enable FLAG_MULTI_PACKET to improve tx stability Chun-Chao Yen
[not found] ` <ME3P282MB2827E9AD698A7F925EB1E9BAD1AC9@ME3P282MB2827.AUSP282.PROD.OUTLOOK.COM>
2023-02-28 9:44 ` [PATCH 1/3 RFC] " Yen ChunChao
2023-02-28 11:25 ` Greg KH
2023-02-28 12:36 ` Yen Chun-Chao
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).