Mark Lord [mailto:mlord@pobox.com]
Sent: Friday, November 18, 2016 8:03 PM
[..]
How does the RTL8152 know that the limit is 16KB,
rather than some other number? Is this a hardwired number
in the hardware, or is it a parameter that the software
sends to the chip during initialization?
It is the limitation of the hardware.
I have a USB analyzer, but it is difficult to figure out how
to program an appropriate trigger point for the capture,
since the problem (with 16KB URBs) takes minutes to hours
or even days to trigger.
It is good. Our hw engineers real want it. Maybe you could send
a specific packet, and trigger it. You could allocate a skb and
fill the data which you prefer, and call
skb_queue_tail(&tp->tx_queue, skb);
[...]
The first issue is that a packet sometimes begins in one URB,
and completes in the next URB, without an rx_desc at the start
of the second URB. This I have already reported earlier.
However, our hw engineer says it wouldn't happen. Our hw always
sends rx_desc + packet + padding. The hw wouldn't split it to
two or more transmission. That is why I wonder who does it.
But the driver, as written, sometimes accesses bytes outside
of the 16KB URB buffer, because it trusts the non-existent
rx_desc in these cases, and also because it accesses bytes
from the rx_desc without first checking whether there is
sufficient remaining space in the URB to hold an rx_desc.
I think I check them. According to the followning code,
list_for_each_safe(cursor, next, &rx_queue) {
struct rx_desc *rx_desc;
struct rx_agg *agg;
int len_used = 0;
struct urb *urb;
u8 *rx_data;
...
rx_desc = agg->head;
rx_data = agg->head;
len_used += sizeof(struct rx_desc); //<-- add the size of next rx_desc
while (urb->actual_length > len_used) {
struct net_device *netdev = tp->netdev;
struct net_device_stats *stats = &netdev->stats;
unsigned int pkt_len;
struct sk_buff *skb;
pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
if (pkt_len < ETH_ZLEN)
break;
len_used += pkt_len;
if (urb->actual_length < len_used)
break;
pkt_len -= CRC_SIZE;
rx_data += sizeof(struct rx_desc);
...
find_next_rx:
rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
rx_desc = (struct rx_desc *)rx_data;
len_used = (int)(rx_data - (u8 *)agg->head);
len_used += sizeof(struct rx_desc); //<-- add the size of next rx_desc
}
submit:
...
}
The while loop would check if the next rx_desc is inside the urb
buffer, because the len_used includes the size of the next rx_desc.
Then, in the while loop, the len_used adds the packet size and check
with urb->actual_length again. These make sure the rx_desc and the
packet are inside the urb buffer. Except the urb->actual_length
is more than agg_buf_sz. However, I don't think it would happen.
Best Regards,
Hayes