Commit ac3f26cc15ad7e3e9efc2b0b0e18c6e84d93af77
1 parent
72ff004258
Exists in
smarc_8mq_lf_v2020.04
and in
11 other branches
net: Don't overwrite waiting packets with asynchronous replies
Peter originally sent a fix, but it breaks a number of other things. This addresses the original reported issue in a different way. That report was: > U-Boot has 1 common buffer to send Ethernet frames, pointed to by > net_tx_packet. When sending to an IP address without knowing the MAC > address, U-Boot makes an ARP request (using the arp_tx_packet buffer) > to find out the MAC address of the IP addressr. When a matching ARP > reply is received, U-Boot continues sending the frame stored in the > net_tx_packet buffer. > > However, in the mean time, if U-Boot needs to send out any network > packets (e.g. replying ping packets or ARP requests for its own IP > address etc.), it will use the net_tx_packet buffer to prepare the > new packet. Thus this buffer is no longer the original packet meant > to be transmitted after the ARP reply. The original packet will be > lost. This instead uses the ARP tx buffer to send async replies in the case where we are actively waiting for an ARP reply. Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> Reported-by: Tran Tien Dat <peter.trantiendat@gmail.com> Reviewed-by: Simon Glass <sjg@chromium.org> Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com>
Showing 6 changed files with 29 additions and 10 deletions Side-by-side Diff
include/net.h
... | ... | @@ -655,6 +655,14 @@ |
655 | 655 | net_state = state; |
656 | 656 | } |
657 | 657 | |
658 | +/* | |
659 | + * net_get_async_tx_pkt_buf - Get a packet buffer that is not in use for | |
660 | + * sending an asynchronous reply | |
661 | + * | |
662 | + * returns - ptr to packet buffer | |
663 | + */ | |
664 | +uchar * net_get_async_tx_pkt_buf(void); | |
665 | + | |
658 | 666 | /* Transmit a packet */ |
659 | 667 | static inline void net_send_packet(uchar *pkt, int len) |
660 | 668 | { |
net/arp.c
... | ... | @@ -34,8 +34,7 @@ |
34 | 34 | int arp_wait_tx_packet_size; |
35 | 35 | ulong arp_wait_timer_start; |
36 | 36 | int arp_wait_try; |
37 | - | |
38 | -static uchar *arp_tx_packet; /* THE ARP transmit packet */ | |
37 | +uchar *arp_tx_packet; /* THE ARP transmit packet */ | |
39 | 38 | static uchar arp_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN]; |
40 | 39 | |
41 | 40 | void arp_init(void) |
... | ... | @@ -126,6 +125,7 @@ |
126 | 125 | struct arp_hdr *arp; |
127 | 126 | struct in_addr reply_ip_addr; |
128 | 127 | int eth_hdr_size; |
128 | + uchar *tx_packet; | |
129 | 129 | |
130 | 130 | /* |
131 | 131 | * We have to deal with two types of ARP packets: |
... | ... | @@ -182,8 +182,9 @@ |
182 | 182 | (net_read_ip(&arp->ar_spa).s_addr & net_netmask.s_addr)) |
183 | 183 | udelay(5000); |
184 | 184 | #endif |
185 | - memcpy(net_tx_packet, et, eth_hdr_size + ARP_HDR_SIZE); | |
186 | - net_send_packet(net_tx_packet, eth_hdr_size + ARP_HDR_SIZE); | |
185 | + tx_packet = net_get_async_tx_pkt_buf(); | |
186 | + memcpy(tx_packet, et, eth_hdr_size + ARP_HDR_SIZE); | |
187 | + net_send_packet(tx_packet, eth_hdr_size + ARP_HDR_SIZE); | |
187 | 188 | return; |
188 | 189 | |
189 | 190 | case ARPOP_REPLY: /* arp reply */ |
net/arp.h
net/net.c
... | ... | @@ -799,6 +799,14 @@ |
799 | 799 | } |
800 | 800 | } |
801 | 801 | |
802 | +uchar *net_get_async_tx_pkt_buf(void) | |
803 | +{ | |
804 | + if (arp_is_waiting()) | |
805 | + return arp_tx_packet; /* If we are waiting, we already sent */ | |
806 | + else | |
807 | + return net_tx_packet; | |
808 | +} | |
809 | + | |
802 | 810 | int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport, |
803 | 811 | int payload_len) |
804 | 812 | { |
net/ping.c
... | ... | @@ -84,6 +84,7 @@ |
84 | 84 | struct icmp_hdr *icmph = (struct icmp_hdr *)&ip->udp_src; |
85 | 85 | struct in_addr src_ip; |
86 | 86 | int eth_hdr_size; |
87 | + uchar *tx_packet; | |
87 | 88 | |
88 | 89 | switch (icmph->type) { |
89 | 90 | case ICMP_ECHO_REPLY: |
... | ... | @@ -107,8 +108,10 @@ |
107 | 108 | icmph->type = ICMP_ECHO_REPLY; |
108 | 109 | icmph->checksum = 0; |
109 | 110 | icmph->checksum = compute_ip_checksum(icmph, len - IP_HDR_SIZE); |
110 | - memcpy(net_tx_packet, et, eth_hdr_size + len); | |
111 | - net_send_packet(net_tx_packet, eth_hdr_size + len); | |
111 | + | |
112 | + tx_packet = net_get_async_tx_pkt_buf(); | |
113 | + memcpy(tx_packet, et, eth_hdr_size + len); | |
114 | + net_send_packet(tx_packet, eth_hdr_size + len); | |
112 | 115 | return; |
113 | 116 | /* default: |
114 | 117 | return;*/ |
test/dm/eth.c
... | ... | @@ -332,10 +332,9 @@ |
332 | 332 | sandbox_eth_set_tx_handler(0, sb_with_async_arp_handler); |
333 | 333 | /* Used by all of the ut_assert macros in the tx_handler */ |
334 | 334 | sandbox_eth_set_priv(0, uts); |
335 | - sandbox_eth_skip_timeout(); | |
336 | 335 | |
337 | 336 | env_set("ethact", "eth@10002000"); |
338 | - ut_assert(net_loop(PING) == -ETIMEDOUT); | |
337 | + ut_assertok(net_loop(PING)); | |
339 | 338 | ut_asserteq_str("eth@10002000", env_get("ethact")); |
340 | 339 | |
341 | 340 | sandbox_eth_set_tx_handler(0, NULL); |
342 | 341 | |
... | ... | @@ -418,10 +417,9 @@ |
418 | 417 | sandbox_eth_set_tx_handler(0, sb_with_async_ping_handler); |
419 | 418 | /* Used by all of the ut_assert macros in the tx_handler */ |
420 | 419 | sandbox_eth_set_priv(0, uts); |
421 | - sandbox_eth_skip_timeout(); | |
422 | 420 | |
423 | 421 | env_set("ethact", "eth@10002000"); |
424 | - ut_assert(net_loop(PING) == -ETIMEDOUT); | |
422 | + ut_assertok(net_loop(PING)); | |
425 | 423 | ut_asserteq_str("eth@10002000", env_get("ethact")); |
426 | 424 | |
427 | 425 | sandbox_eth_set_tx_handler(0, NULL); |