Make the send handler registerable so tests can check for different things.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---

Changes in v2: None

arch/sandbox/include/asm/eth.h | 17 +++++++++++++ drivers/net/sandbox.c | 55 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h index 3dc84f0e4b..20175862f1 100644 --- a/arch/sandbox/include/asm/eth.h +++ b/arch/sandbox/include/asm/eth.h @@ -39,4 +39,21 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, unsigned int len);

+/** + * A packet handler + * + * dev - device pointer + * pkt - pointer to the "sent" packet + * len - packet length + */ +typedef int sandbox_eth_tx_hand_f(struct udevice *dev, void *pkt, + unsigned int len); + +/* + * Set packet handler + * + * handler - The func ptr to call on send. If NULL, set to default handler + */ +void sandbox_eth_set_tx_handler(int index, sandbox_eth_tx_hand_f *handler); + #endif /* __ETH_H */ diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index c472261568..db461b892e 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -10,6 +10,7 @@ #include <dm.h> #include <malloc.h> #include <net.h> +#include <asm/eth.h> #include <asm/test.h>

DECLARE_GLOBAL_DATA_PTR; @@ -22,6 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; * disabled: Will not respond * recv_packet_buffer: buffer of the packet returned as received * recv_packet_length: length of the packet returned as received + * tx_handler - function to generate responses to sent packets */ struct eth_sandbox_priv { uchar fake_host_hwaddr[ARP_HLEN]; @@ -29,6 +31,7 @@ struct eth_sandbox_priv { bool disabled; uchar *recv_packet_buffer; int recv_packet_length; + sandbox_eth_tx_hand_f *tx_handler; };

static bool skip_timeout; @@ -165,6 +168,52 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, return 0; }

+/* + * sb_default_handler() + * + * perform typical responses to simple ping + * + * dev - device pointer + * pkt - "sent" packet buffer + * len - length of packet + */ +static int sb_default_handler(struct udevice *dev, void *packet, + unsigned int len) +{ + if (!sandbox_eth_arp_req_to_reply(dev, packet, len)) + return 0; + if (!sandbox_eth_ping_req_to_reply(dev, packet, len)) + return 0; + + return 0; +} + +/* + * sandbox_eth_set_tx_handler() + * + * Set a custom response to a packet being sent through the sandbox eth test + * driver + * + * index - interface to set the handler for + * handler - The func ptr to call on send. If NULL, set to default handler + */ +void sandbox_eth_set_tx_handler(int index, sandbox_eth_tx_hand_f *handler) +{ + struct udevice *dev; + struct eth_sandbox_priv *priv; + int ret; + + ret = uclass_get_device(UCLASS_ETH, index, &dev); + if (ret) + return; + + priv = dev_get_priv(dev); + if (handler) + priv->tx_handler = handler; + else + priv->tx_handler = sb_default_handler; +} + static int sb_eth_start(struct udevice *dev) { struct eth_sandbox_priv *priv = dev_get_priv(dev); @@ -185,10 +234,7 @@ static int sb_eth_send(struct udevice *dev, void *packet, int length) if (priv->disabled) return 0;

- if (!sandbox_eth_arp_req_to_reply(dev, packet, length)) - return 0; - if (!sandbox_eth_ping_req_to_reply(dev, packet, length)) - return 0; + return priv->tx_handler(dev, packet, length); }

static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp) @@ -254,6 +300,7 @@ static int sb_eth_ofdata_to_platdata(struct udevice *dev) } memcpy(priv->fake_host_hwaddr, mac, ARP_HLEN); priv->disabled = false; + priv->tx_handler = sb_default_handler;

return 0; }

If tests want to implement tx handlers, they will likely need access to the details in the priv structure.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---

Changes in v2: None

arch/sandbox/include/asm/eth.h | 19 +++++++++++++++++++ drivers/net/sandbox.c | 19 ------------------- 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h index 20175862f1..ced6d51999 100644 --- a/arch/sandbox/include/asm/eth.h +++ b/arch/sandbox/include/asm/eth.h @@ -49,6 +49,25 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, typedef int sandbox_eth_tx_hand_f(struct udevice *dev, void *pkt, unsigned int len);

+/** + * struct eth_sandbox_priv - memory for sandbox mock driver + * + * fake_host_hwaddr - MAC address of mocked machine + * fake_host_ipaddr - IP address of mocked machine + * disabled - Will not respond + * recv_packet_buffer - buffer of the packet returned as received + * recv_packet_length - length of the packet returned as received + * tx_handler - function to generate responses to sent packets + */ +struct eth_sandbox_priv { + uchar fake_host_hwaddr[ARP_HLEN]; + struct in_addr fake_host_ipaddr; + bool disabled; + uchar *recv_packet_buffer; + int recv_packet_length; + sandbox_eth_tx_hand_f *tx_handler; +}; + /* * Set packet handler * diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index db461b892e..6f0fe0ced5 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -15,25 +15,6 @@

DECLARE_GLOBAL_DATA_PTR;

-/** - * struct eth_sandbox_priv - memory for sandbox mock driver - * - * fake_host_hwaddr: MAC address of mocked machine - * fake_host_ipaddr: IP address of mocked machine - * disabled: Will not respond - * recv_packet_buffer: buffer of the packet returned as received - * recv_packet_length: length of the packet returned as received - * tx_handler - function to generate responses to sent packets - */ -struct eth_sandbox_priv { - uchar fake_host_hwaddr[ARP_HLEN]; - struct in_addr fake_host_ipaddr; - bool disabled; - uchar *recv_packet_buffer; - int recv_packet_length; - sandbox_eth_tx_hand_f *tx_handler; -}; - static bool skip_timeout;

/*

Use up to the max allocated receive buffers so as to be able to test more complex situations.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---

Changes in v2: None

arch/sandbox/include/asm/eth.h | 10 +++++--- drivers/net/sandbox.c | 57 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h index ced6d51999..7a49e98fa8 100644 --- a/arch/sandbox/include/asm/eth.h +++ b/arch/sandbox/include/asm/eth.h @@ -55,16 +55,18 @@ typedef int sandbox_eth_tx_hand_f(struct udevice *dev, void *pkt, * fake_host_hwaddr - MAC address of mocked machine * fake_host_ipaddr - IP address of mocked machine * disabled - Will not respond - * recv_packet_buffer - buffer of the packet returned as received - * recv_packet_length - length of the packet returned as received + * recv_packet_buffer - buffers of the packet returned as received + * recv_packet_length - lengths of the packet returned as received + * recv_packets - number of packets returned * tx_handler - function to generate responses to sent packets */ struct eth_sandbox_priv { uchar fake_host_hwaddr[ARP_HLEN]; struct in_addr fake_host_ipaddr; bool disabled; - uchar *recv_packet_buffer; - int recv_packet_length; + uchar * recv_packet_buffer[PKTBUFSRX]; + int recv_packet_length[PKTBUFSRX]; + int recv_packets; sandbox_eth_tx_hand_f *tx_handler; };

diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index 6f0fe0ced5..9c0e0d009e 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -71,11 +71,15 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, if (ntohs(arp->ar_op) != ARPOP_REQUEST) return -EAGAIN;

+ /* Don't allow the buffer to overrun */ + if (priv->recv_packets >= PKTBUFSRX) + return 0; + /* store this as the assumed IP of the fake host */ priv->fake_host_ipaddr = net_read_ip(&arp->ar_tpa);

/* Formulate a fake response */ - eth_recv = (void *)priv->recv_packet_buffer; + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); eth_recv->et_protlen = htons(PROT_ARP); @@ -91,7 +95,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, memcpy(&arp_recv->ar_tha, &arp->ar_sha, ARP_HLEN); net_copy_ip(&arp_recv->ar_tpa, &arp->ar_spa);

- priv->recv_packet_length = ETHER_HDR_SIZE + ARP_HDR_SIZE; + priv->recv_packet_length[priv->recv_packets] = + ETHER_HDR_SIZE + ARP_HDR_SIZE; + ++priv->recv_packets;

return 0; } @@ -127,8 +133,12 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, if (icmp->type != ICMP_ECHO_REQUEST) return -EAGAIN;

+ /* Don't allow the buffer to overrun */ + if (priv->recv_packets >= PKTBUFSRX) + return 0; + /* reply to the ping */ - eth_recv = (void *)priv->recv_packet_buffer; + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; memcpy(eth_recv, packet, len); ipr = (void *)eth_recv + ETHER_HDR_SIZE; icmpr = (struct icmp_hdr *)&ipr->udp_src; @@ -144,7 +154,8 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, icmpr->checksum = 0; icmpr->checksum = compute_ip_checksum(icmpr, ICMP_HDR_SIZE);

- priv->recv_packet_length = len; + priv->recv_packet_length[priv->recv_packets] = len; + ++priv->recv_packets;

return 0; } @@ -201,7 +212,11 @@ static int sb_eth_start(struct udevice *dev)

debug("eth_sandbox: Start\n");

- priv->recv_packet_buffer = net_rx_packets[0]; + priv->recv_packets = 0; + for (int i = 0; i < PKTBUFSRX; i++) { + priv->recv_packet_buffer[i] = net_rx_packets[i]; + priv->recv_packet_length[i] = 0; + }

return 0; } @@ -227,18 +242,37 @@ static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp) skip_timeout = false; }

- if (priv->recv_packet_length) { - int lcl_recv_packet_length = priv->recv_packet_length; + if (priv->recv_packets) { + int lcl_recv_packet_length = priv->recv_packet_length[0];

- debug("eth_sandbox: received packet %d\n", - priv->recv_packet_length); - priv->recv_packet_length = 0; - *packetp = priv->recv_packet_buffer; + debug("eth_sandbox: received packet[%d], %d waiting\n", + lcl_recv_packet_length, priv->recv_packets - 1); + *packetp = priv->recv_packet_buffer[0]; return lcl_recv_packet_length; } return 0; }

+static int sb_eth_free_pkt(struct udevice *dev, uchar *packet, int length) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + int i; + + if (!priv->recv_packets) + return 0; + + --priv->recv_packets; + for (i = 0; i < priv->recv_packets; i++) { + priv->recv_packet_length[i] = priv->recv_packet_length[i + 1]; + memcpy(priv->recv_packet_buffer[i], + priv->recv_packet_buffer[i + 1], + priv->recv_packet_length[i + 1]); + } + priv->recv_packet_length[priv->recv_packets] = 0; + + return 0; +} + static void sb_eth_stop(struct udevice *dev) { debug("eth_sandbox: Stop\n"); @@ -257,6 +291,7 @@ static const struct eth_ops sb_eth_ops = { .start = sb_eth_start, .send = sb_eth_send, .recv = sb_eth_recv, + .free_pkt = sb_eth_free_pkt, .stop = sb_eth_stop, .write_hwaddr = sb_eth_write_hwaddr, };

This single-sources the state of the ARP.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org

---

Changes in v2: - return bool instead of int

include/net.h | 1 + net/arp.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h index 2b2deb5aae..259c4a7ed4 100644 --- a/include/net.h +++ b/include/net.h @@ -635,6 +635,7 @@ rxhand_f *net_get_udp_handler(void); /* Get UDP RX packet handler */ void net_set_udp_handler(rxhand_f *); /* Set UDP RX packet handler */ rxhand_f *net_get_arp_handler(void); /* Get ARP RX packet handler */ void net_set_arp_handler(rxhand_f *); /* Set ARP RX packet handler */ +bool arp_is_waiting(void); /* Waiting for ARP reply? */ void net_set_icmp_handler(rxhand_icmp_f *f); /* Set ICMP RX handler */ void net_set_timeout_handler(ulong, thand_f *);/* Set timeout handler */

diff --git a/net/arp.c b/net/arp.c index b8a71684cd..ea685d9ac6 100644 --- a/net/arp.c +++ b/net/arp.c @@ -100,7 +100,7 @@ int arp_timeout_check(void) { ulong t;

- if (!net_arp_wait_packet_ip.s_addr) + if (!arp_is_waiting()) return 0;

t = get_timer(0); @@ -187,8 +187,8 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) return;

case ARPOP_REPLY: /* arp reply */ - /* are we waiting for a reply */ - if (!net_arp_wait_packet_ip.s_addr) + /* are we waiting for a reply? */ + if (!arp_is_waiting()) break;

#ifdef CONFIG_KEEP_SERVERADDR @@ -233,3 +233,8 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) return; } } + +bool arp_is_waiting(void) +{ + return !!net_arp_wait_packet_ip.s_addr; +}

This tests that ARP requests made to this target's IP address are responded-to by the target when it is doing other networking operations.

This currently corrupts the ongoing operation of the device if it happens to be awaiting an ARP reply of its own to whatever serverip it is attempting to communicate with. In the test, add an expectation that the user operation (ping, in this case) will fail. A later patch will address this problem.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com

---

Changes in v2: - add comments as to the use of the uts variable - check the return of the injection handler and pass on overflow error - return an error instead of 0 / 1

arch/sandbox/include/asm/eth.h | 10 +++++ drivers/net/sandbox.c | 41 ++++++++++++++++++++ test/dm/eth.c | 86 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h index a6adec4e83..d068486f0b 100644 --- a/arch/sandbox/include/asm/eth.h +++ b/arch/sandbox/include/asm/eth.h @@ -39,6 +39,16 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, unsigned int len);

+/* + * sandbox_eth_recv_arp_req() + * + * Inject an ARP request for this target + * + * @dev: device that received the packet + * @return 0 if injected, -EOVERFLOW if not + */ +int sandbox_eth_recv_arp_req(struct udevice *dev); + /** * A packet handler * diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index e26e72ecc1..81f0764fa6 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -161,6 +161,47 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, }

/* + * sandbox_eth_recv_arp_req() + * + * Inject an ARP request for this target + * + * returns 0 if injected, -EOVERFLOW if not + */ +int sandbox_eth_recv_arp_req(struct udevice *dev) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth_recv; + struct arp_hdr *arp_recv; + + /* Don't allow the buffer to overrun */ + if (priv->recv_packets >= PKTBUFSRX) + return -EOVERFLOW; + + /* Formulate a fake request */ + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; + memcpy(eth_recv->et_dest, net_bcast_ethaddr, ARP_HLEN); + memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); + eth_recv->et_protlen = htons(PROT_ARP); + + arp_recv = (void *)eth_recv + ETHER_HDR_SIZE; + arp_recv->ar_hrd = htons(ARP_ETHER); + arp_recv->ar_pro = htons(PROT_IP); + arp_recv->ar_hln = ARP_HLEN; + arp_recv->ar_pln = ARP_PLEN; + arp_recv->ar_op = htons(ARPOP_REQUEST); + memcpy(&arp_recv->ar_sha, priv->fake_host_hwaddr, ARP_HLEN); + net_write_ip(&arp_recv->ar_spa, priv->fake_host_ipaddr); + memcpy(&arp_recv->ar_tha, net_null_ethaddr, ARP_HLEN); + net_write_ip(&arp_recv->ar_tpa, net_ip); + + priv->recv_packet_length[priv->recv_packets] = + ETHER_HDR_SIZE + ARP_HDR_SIZE; + ++priv->recv_packets; + + return 0; +} + +/* * sb_default_handler() * * perform typical responses to simple ping diff --git a/test/dm/eth.c b/test/dm/eth.c index 1a7684a887..9e52d5cdfe 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -258,3 +258,89 @@ static int dm_test_net_retry(struct unit_test_state *uts) return retval; } DM_TEST(dm_test_net_retry, DM_TESTF_SCAN_FDT); + +static int sb_check_arp_reply(struct udevice *dev, void *packet, + unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth = packet; + struct arp_hdr *arp; + /* Used by all of the ut_assert macros */ + struct unit_test_state *uts = priv->priv; + + if (ntohs(eth->et_protlen) != PROT_ARP) + return 0; + + arp = packet + ETHER_HDR_SIZE; + + if (ntohs(arp->ar_op) != ARPOP_REPLY) + return 0; + + /* This test would be worthless if we are not waiting */ + ut_assert(arp_is_waiting()); + + /* Validate response */ + ut_assert(memcmp(eth->et_src, net_ethaddr, ARP_HLEN) == 0); + ut_assert(memcmp(eth->et_dest, priv->fake_host_hwaddr, ARP_HLEN) == 0); + ut_assert(eth->et_protlen == htons(PROT_ARP)); + + ut_assert(arp->ar_hrd == htons(ARP_ETHER)); + ut_assert(arp->ar_pro == htons(PROT_IP)); + ut_assert(arp->ar_hln == ARP_HLEN); + ut_assert(arp->ar_pln == ARP_PLEN); + ut_assert(memcmp(&arp->ar_sha, net_ethaddr, ARP_HLEN) == 0); + ut_assert(net_read_ip(&arp->ar_spa).s_addr == net_ip.s_addr); + ut_assert(memcmp(&arp->ar_tha, priv->fake_host_hwaddr, ARP_HLEN) == 0); + ut_assert(net_read_ip(&arp->ar_tpa).s_addr == + string_to_ip("1.1.2.4").s_addr); + + return 0; +} + +static int sb_with_async_arp_handler(struct udevice *dev, void *packet, + unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth = packet; + struct arp_hdr *arp = packet + ETHER_HDR_SIZE; + int ret; + + /* + * If we are about to generate a reply to ARP, first inject a request + * from another host + */ + if (ntohs(eth->et_protlen) == PROT_ARP && + ntohs(arp->ar_op) == ARPOP_REQUEST) { + /* Make sure sandbox_eth_recv_arp_req() knows who is asking */ + priv->fake_host_ipaddr = string_to_ip("1.1.2.4"); + + ret = sandbox_eth_recv_arp_req(dev); + if (ret) + return ret; + } + + sandbox_eth_arp_req_to_reply(dev, packet, len); + sandbox_eth_ping_req_to_reply(dev, packet, len); + + return sb_check_arp_reply(dev, packet, len); +} + +static int dm_test_eth_async_arp_reply(struct unit_test_state *uts) +{ + net_ping_ip = string_to_ip("1.1.2.2"); + + sandbox_eth_set_tx_handler(0, sb_with_async_arp_handler); + /* Used by all of the ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, uts); + sandbox_eth_skip_timeout(); + + env_set("ethact", "eth@10002000"); + ut_assert(net_loop(PING) == -ETIMEDOUT); + ut_asserteq_str("eth@10002000", env_get("ethact")); + + sandbox_eth_set_tx_handler(0, NULL); + + return 0; +} + +DM_TEST(dm_test_eth_async_arp_reply, DM_TESTF_SCAN_FDT);

The target will respond to pings while doing other network handling. Make sure that the response happens and is correct.

This currently corrupts the ongoing operation of the device if it happens to be awaiting an ARP reply of its own to whatever serverip it is attempting to communicate with. In the test, add an expectation that the user operation (ping, in this case) will fail. A later patch will address this problem.

Signed-off-by: Joe Hershberger joe.hershberger@ni.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com

---

Changes in v2: - add comments as to the use of the uts variable - check the return of the injection handler and pass on overflow error - return an error instead of 0 / 1

arch/sandbox/include/asm/eth.h | 10 +++++ drivers/net/sandbox.c | 51 +++++++++++++++++++++++++ test/dm/eth.c | 86 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h index d068486f0b..477fa00fad 100644 --- a/arch/sandbox/include/asm/eth.h +++ b/arch/sandbox/include/asm/eth.h @@ -49,6 +49,16 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet, */ int sandbox_eth_recv_arp_req(struct udevice *dev);

+/* + * sandbox_eth_recv_ping_req() + * + * Inject a ping request for this target + * + * @dev: device that received the packet + * @return 0 if injected, -EOVERFLOW if not + */ +int sandbox_eth_recv_ping_req(struct udevice *dev); + /** * A packet handler * diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index 81f0764fa6..decce2fa59 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -202,6 +202,57 @@ int sandbox_eth_recv_arp_req(struct udevice *dev) }

/* + * sandbox_eth_recv_ping_req() + * + * Inject a ping request for this target + * + * returns 0 if injected, -EOVERFLOW if not + */ +int sandbox_eth_recv_ping_req(struct udevice *dev) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth_recv; + struct ip_udp_hdr *ipr; + struct icmp_hdr *icmpr; + + /* Don't allow the buffer to overrun */ + if (priv->recv_packets >= PKTBUFSRX) + return -EOVERFLOW; + + /* Formulate a fake ping */ + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; + + memcpy(eth_recv->et_dest, net_ethaddr, ARP_HLEN); + memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); + eth_recv->et_protlen = htons(PROT_IP); + + ipr = (void *)eth_recv + ETHER_HDR_SIZE; + ipr->ip_hl_v = 0x45; + ipr->ip_len = htons(IP_ICMP_HDR_SIZE); + ipr->ip_off = htons(IP_FLAGS_DFRAG); + ipr->ip_p = IPPROTO_ICMP; + ipr->ip_sum = 0; + net_write_ip(&ipr->ip_src, priv->fake_host_ipaddr); + net_write_ip(&ipr->ip_dst, net_ip); + ipr->ip_sum = compute_ip_checksum(ipr, IP_HDR_SIZE); + + icmpr = (struct icmp_hdr *)&ipr->udp_src; + + icmpr->type = ICMP_ECHO_REQUEST; + icmpr->code = 0; + icmpr->checksum = 0; + icmpr->un.echo.id = 0; + icmpr->un.echo.sequence = htons(1); + icmpr->checksum = compute_ip_checksum(icmpr, ICMP_HDR_SIZE); + + priv->recv_packet_length[priv->recv_packets] = + ETHER_HDR_SIZE + IP_ICMP_HDR_SIZE; + ++priv->recv_packets; + + return 0; +} + +/* * sb_default_handler() * * perform typical responses to simple ping diff --git a/test/dm/eth.c b/test/dm/eth.c index 9e52d5cdfe..86482e9755 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -344,3 +344,89 @@ static int dm_test_eth_async_arp_reply(struct unit_test_state *uts) }

DM_TEST(dm_test_eth_async_arp_reply, DM_TESTF_SCAN_FDT); + +static int sb_check_ping_reply(struct udevice *dev, void *packet, + unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth = packet; + struct ip_udp_hdr *ip; + struct icmp_hdr *icmp; + /* Used by all of the ut_assert macros */ + struct unit_test_state *uts = priv->priv; + + if (ntohs(eth->et_protlen) != PROT_IP) + return 0; + + ip = packet + ETHER_HDR_SIZE; + + if (ip->ip_p != IPPROTO_ICMP) + return 0; + + icmp = (struct icmp_hdr *)&ip->udp_src; + + if (icmp->type != ICMP_ECHO_REPLY) + return 0; + + /* This test would be worthless if we are not waiting */ + ut_assert(arp_is_waiting()); + + /* Validate response */ + ut_assert(memcmp(eth->et_src, net_ethaddr, ARP_HLEN) == 0); + ut_assert(memcmp(eth->et_dest, priv->fake_host_hwaddr, ARP_HLEN) == 0); + ut_assert(eth->et_protlen == htons(PROT_IP)); + + ut_assert(net_read_ip(&ip->ip_src).s_addr == net_ip.s_addr); + ut_assert(net_read_ip(&ip->ip_dst).s_addr == + string_to_ip("1.1.2.4").s_addr); + + return 0; +} + +static int sb_with_async_ping_handler(struct udevice *dev, void *packet, + unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + struct ethernet_hdr *eth = packet; + struct arp_hdr *arp = packet + ETHER_HDR_SIZE; + int ret; + + /* + * If we are about to generate a reply to ARP, first inject a request + * from another host + */ + if (ntohs(eth->et_protlen) == PROT_ARP && + ntohs(arp->ar_op) == ARPOP_REQUEST) { + /* Make sure sandbox_eth_recv_arp_req() knows who is asking */ + priv->fake_host_ipaddr = string_to_ip("1.1.2.4"); + + ret = sandbox_eth_recv_ping_req(dev); + if (ret) + return ret; + } + + sandbox_eth_arp_req_to_reply(dev, packet, len); + sandbox_eth_ping_req_to_reply(dev, packet, len); + + return sb_check_ping_reply(dev, packet, len); +} + +static int dm_test_eth_async_ping_reply(struct unit_test_state *uts) +{ + net_ping_ip = string_to_ip("1.1.2.2"); + + sandbox_eth_set_tx_handler(0, sb_with_async_ping_handler); + /* Used by all of the ut_assert macros in the tx_handler */ + sandbox_eth_set_priv(0, uts); + sandbox_eth_skip_timeout(); + + env_set("ethact", "eth@10002000"); + ut_assert(net_loop(PING) == -ETIMEDOUT); + ut_asserteq_str("eth@10002000", env_get("ethact")); + + sandbox_eth_set_tx_handler(0, NULL); + + return 0; +} + +DM_TEST(dm_test_eth_async_ping_reply, DM_TESTF_SCAN_FDT);

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

Changes in v2: None

include/net.h | 8 ++++++++ net/arp.c | 9 +++++---- net/arp.h | 1 + net/net.c | 8 ++++++++ net/ping.c | 7 +++++-- test/dm/eth.c | 6 ++---- 6 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/include/net.h b/include/net.h index 259c4a7ed4..a789a381ab 100644 --- a/include/net.h +++ b/include/net.h @@ -654,6 +654,14 @@ static inline void net_set_state(enum net_loop_state state) net_state = state; }

+/* + * net_get_async_tx_pkt_buf - Get a packet buffer that is not in use for + * sending an asynchronous reply + * + * returns - ptr to packet buffer + */ +uchar * net_get_async_tx_pkt_buf(void); + /* Transmit a packet */ static inline void net_send_packet(uchar *pkt, int len) { diff --git a/net/arp.c b/net/arp.c index ea685d9ac6..b49c3d3ced 100644 --- a/net/arp.c +++ b/net/arp.c @@ -34,8 +34,7 @@ uchar *arp_wait_packet_ethaddr; int arp_wait_tx_packet_size; ulong arp_wait_timer_start; int arp_wait_try; - -static uchar *arp_tx_packet; /* THE ARP transmit packet */ +uchar *arp_tx_packet; /* THE ARP transmit packet */ static uchar arp_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN];

void arp_init(void) @@ -126,6 +125,7 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) struct arp_hdr *arp; struct in_addr reply_ip_addr; int eth_hdr_size; + uchar *tx_packet;

/* * We have to deal with two types of ARP packets: @@ -182,8 +182,9 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) (net_read_ip(&arp->ar_spa).s_addr & net_netmask.s_addr)) udelay(5000); #endif - memcpy(net_tx_packet, et, eth_hdr_size + ARP_HDR_SIZE); - net_send_packet(net_tx_packet, eth_hdr_size + ARP_HDR_SIZE); + tx_packet = net_get_async_tx_pkt_buf(); + memcpy(tx_packet, et, eth_hdr_size + ARP_HDR_SIZE); + net_send_packet(tx_packet, eth_hdr_size + ARP_HDR_SIZE); return;

case ARPOP_REPLY: /* arp reply */ diff --git a/net/arp.h b/net/arp.h index afb86958f3..25b3c00d5c 100644 --- a/net/arp.h +++ b/net/arp.h @@ -20,6 +20,7 @@ extern uchar *arp_wait_packet_ethaddr; extern int arp_wait_tx_packet_size; extern ulong arp_wait_timer_start; extern int arp_wait_try; +extern uchar *arp_tx_packet;

void arp_init(void); void arp_request(void); diff --git a/net/net.c b/net/net.c index 31cf306ae7..77a71415f0 100644 --- a/net/net.c +++ b/net/net.c @@ -799,6 +799,14 @@ void net_set_timeout_handler(ulong iv, thand_f *f) } }

+uchar *net_get_async_tx_pkt_buf(void) +{ + if (arp_is_waiting()) + return arp_tx_packet; /* If we are waiting, we already sent */ + else + return net_tx_packet; +} + int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport, int payload_len) { diff --git a/net/ping.c b/net/ping.c index 3e5461a36a..821d35d01d 100644 --- a/net/ping.c +++ b/net/ping.c @@ -84,6 +84,7 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) struct icmp_hdr *icmph = (struct icmp_hdr *)&ip->udp_src; struct in_addr src_ip; int eth_hdr_size; + uchar *tx_packet;

switch (icmph->type) { case ICMP_ECHO_REPLY: @@ -107,8 +108,10 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) icmph->type = ICMP_ECHO_REPLY; icmph->checksum = 0; icmph->checksum = compute_ip_checksum(icmph, len - IP_HDR_SIZE); - memcpy(net_tx_packet, et, eth_hdr_size + len); - net_send_packet(net_tx_packet, eth_hdr_size + len); + + tx_packet = net_get_async_tx_pkt_buf(); + memcpy(tx_packet, et, eth_hdr_size + len); + net_send_packet(tx_packet, eth_hdr_size + len); return; /* default: return;*/ diff --git a/test/dm/eth.c b/test/dm/eth.c index 86482e9755..850eabb9dc 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -332,10 +332,9 @@ static int dm_test_eth_async_arp_reply(struct unit_test_state *uts) sandbox_eth_set_tx_handler(0, sb_with_async_arp_handler); /* Used by all of the ut_assert macros in the tx_handler */ sandbox_eth_set_priv(0, uts); - sandbox_eth_skip_timeout();

env_set("ethact", "eth@10002000"); - ut_assert(net_loop(PING) == -ETIMEDOUT); + ut_assertok(net_loop(PING)); ut_asserteq_str("eth@10002000", env_get("ethact"));

sandbox_eth_set_tx_handler(0, NULL); @@ -418,10 +417,9 @@ static int dm_test_eth_async_ping_reply(struct unit_test_state *uts) sandbox_eth_set_tx_handler(0, sb_with_async_ping_handler); /* Used by all of the ut_assert macros in the tx_handler */ sandbox_eth_set_priv(0, uts); - sandbox_eth_skip_timeout();

env_set("ethact", "eth@10002000"); - ut_assert(net_loop(PING) == -ETIMEDOUT); + ut_assertok(net_loop(PING)); ut_asserteq_str("eth@10002000", env_get("ethact"));

sandbox_eth_set_tx_handler(0, NULL);