All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies
@ 2018-09-26 21:48 Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct Joe Hershberger
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

The issue [1] was reported by (Peter) Tran Tien Dat. Unfortunately his
fix for the issue broke notmal operation and I don't feel is a good way
to address the issue. Also, the situation was not covered in the unit
tests, so we'll add them now.

First we refactor the unit test capability of the sandbox ethernet fake
driver so that we can exercise that part of the network stack, then
add the tests where we prove that the async replies work, but that in
the process, the action expected by the user (ping in this case) is
broken.

Lastly, we correct the problem and change the unit tests to also expect
success of the user's operation.

[1] https://patchwork.ozlabs.org/patch/939617/

Changes in v2:
- Added parameter comments
- Changed return value to use typical error approach
- In test, stop calling reply functions when one matches
- add comments as to the use of the uts variable
- add missing commit message
- check the return of the injection handler and pass on overflow error
- rename local "uc_priv" variable to "dev_priv" to not be misleading.
- return an error instead of 0 / 1
- return bool instead of int

Joe Hershberger (10):
  net: sandbox: Move disabled flag into priv struct
  net: sandbox: Refactor sandbox send function
  net: sandbox: Make the fake eth driver response configurable
  net: sandbox: Share the priv structure with tests
  net: sandbox: Allow fake eth to handle more than 1 packet response
  net: Add an accessor to know if waiting for ARP
  net: sandbox: Add a priv ptr for tests to use
  test: eth: Add a test for ARP requests
  test: eth: Add a test for the target being pinged
  net: Don't overwrite waiting packets with asynchronous replies

 arch/sandbox/include/asm/eth.h |  93 +++++++++
 drivers/net/sandbox.c          | 417 +++++++++++++++++++++++++++++++----------
 include/net.h                  |   9 +
 net/arp.c                      |  20 +-
 net/arp.h                      |   1 +
 net/net.c                      |   8 +
 net/ping.c                     |   7 +-
 test/dm/eth.c                  | 170 +++++++++++++++++
 8 files changed, 616 insertions(+), 109 deletions(-)

-- 
2.11.0

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

* [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function Joe Hershberger
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

Store the per-device data with the device.

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

 drivers/net/sandbox.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
index b71c8f88d9..60fe065ee5 100644
--- a/drivers/net/sandbox.c
+++ b/drivers/net/sandbox.c
@@ -19,17 +19,18 @@ DECLARE_GLOBAL_DATA_PTR;
  *
  * 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
  */
 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;
 };
 
-static bool disabled[8] = {false};
 static bool skip_timeout;
 
 /*
@@ -40,7 +41,16 @@ static bool skip_timeout;
  */
 void sandbox_eth_disable_response(int index, bool disable)
 {
-	disabled[index] = disable;
+	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);
+	priv->disabled = disable;
 }
 
 /*
@@ -71,8 +81,7 @@ static int sb_eth_send(struct udevice *dev, void *packet, int length)
 
 	debug("eth_sandbox: Send packet %d\n", length);
 
-	if (dev->seq >= 0 && dev->seq < ARRAY_SIZE(disabled) &&
-	    disabled[dev->seq])
+	if (priv->disabled)
 		return 0;
 
 	if (ntohs(eth->et_protlen) == PROT_ARP) {
@@ -212,6 +221,7 @@ static int sb_eth_ofdata_to_platdata(struct udevice *dev)
 		return -EINVAL;
 	}
 	memcpy(priv->fake_host_hwaddr, mac, ARP_HLEN);
+	priv->disabled = false;
 
 	return 0;
 }
-- 
2.11.0

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

* [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-09-27  6:38   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 03/10] net: sandbox: Make the fake eth driver response configurable Joe Hershberger
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

Make the behavior of the send function reusable.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

---

Changes in v2:
- Added parameter comments
- Changed return value to use typical error approach
- In test, stop calling reply functions when one matches

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

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h
index bfcd11b593..3dc84f0e4b 100644
--- a/arch/sandbox/include/asm/eth.h
+++ b/arch/sandbox/include/asm/eth.h
@@ -13,4 +13,30 @@ void sandbox_eth_disable_response(int index, bool disable);
 
 void sandbox_eth_skip_timeout(void);
 
+/*
+ * sandbox_eth_arp_req_to_reply()
+ *
+ * Check for an arp request to be sent. If so, inject a reply
+ *
+ * @dev: device that received the packet
+ * @packet: pointer to the received pacaket buffer
+ * @len: length of received packet
+ * @return 0 if injected, -EAGAIN if not
+ */
+int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet,
+				 unsigned int len);
+
+/*
+ * sandbox_eth_ping_req_to_reply()
+ *
+ * Check for a ping request to be sent. If so, inject a reply
+ *
+ * @dev: device that received the packet
+ * @packet: pointer to the received pacaket buffer
+ * @len: length of received packet
+ * @return 0 if injected, -EAGAIN if not
+ */
+int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet,
+				  unsigned int len);
+
 #endif /* __ETH_H */
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
index 60fe065ee5..c472261568 100644
--- a/drivers/net/sandbox.c
+++ b/drivers/net/sandbox.c
@@ -63,6 +63,108 @@ void sandbox_eth_skip_timeout(void)
 	skip_timeout = true;
 }
 
+/*
+ * sandbox_eth_arp_req_to_reply()
+ *
+ * Check for an arp request to be sent. If so, inject a reply
+ *
+ * returns 0 if injected, -EAGAIN if not
+ */
+int sandbox_eth_arp_req_to_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;
+	struct ethernet_hdr *eth_recv;
+	struct arp_hdr *arp_recv;
+
+	if (ntohs(eth->et_protlen) != PROT_ARP)
+		return -EAGAIN;
+
+	arp = packet + ETHER_HDR_SIZE;
+
+	if (ntohs(arp->ar_op) != ARPOP_REQUEST)
+		return -EAGAIN;
+
+	/* 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;
+	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);
+
+	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_REPLY);
+	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, &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;
+
+	return 0;
+}
+
+/*
+ * sandbox_eth_ping_req_to_reply()
+ *
+ * Check for a ping request to be sent. If so, inject a reply
+ *
+ * returns 0 if injected, -EAGAIN if not
+ */
+int sandbox_eth_ping_req_to_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;
+	struct ethernet_hdr *eth_recv;
+	struct ip_udp_hdr *ipr;
+	struct icmp_hdr *icmpr;
+
+	if (ntohs(eth->et_protlen) != PROT_IP)
+		return -EAGAIN;
+
+	ip = packet + ETHER_HDR_SIZE;
+
+	if (ip->ip_p != IPPROTO_ICMP)
+		return -EAGAIN;
+
+	icmp = (struct icmp_hdr *)&ip->udp_src;
+
+	if (icmp->type != ICMP_ECHO_REQUEST)
+		return -EAGAIN;
+
+	/* reply to the ping */
+	eth_recv = (void *)priv->recv_packet_buffer;
+	memcpy(eth_recv, packet, len);
+	ipr = (void *)eth_recv + ETHER_HDR_SIZE;
+	icmpr = (struct icmp_hdr *)&ipr->udp_src;
+	memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN);
+	memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN);
+	ipr->ip_sum = 0;
+	ipr->ip_off = 0;
+	net_copy_ip((void *)&ipr->ip_dst, &ip->ip_src);
+	net_write_ip((void *)&ipr->ip_src, priv->fake_host_ipaddr);
+	ipr->ip_sum = compute_ip_checksum(ipr, IP_HDR_SIZE);
+
+	icmpr->type = ICMP_ECHO_REPLY;
+	icmpr->checksum = 0;
+	icmpr->checksum = compute_ip_checksum(icmpr, ICMP_HDR_SIZE);
+
+	priv->recv_packet_length = len;
+
+	return 0;
+}
+
 static int sb_eth_start(struct udevice *dev)
 {
 	struct eth_sandbox_priv *priv = dev_get_priv(dev);
@@ -77,86 +179,16 @@ static int sb_eth_start(struct udevice *dev)
 static int sb_eth_send(struct udevice *dev, void *packet, int length)
 {
 	struct eth_sandbox_priv *priv = dev_get_priv(dev);
-	struct ethernet_hdr *eth = packet;
 
 	debug("eth_sandbox: Send packet %d\n", length);
 
 	if (priv->disabled)
 		return 0;
 
-	if (ntohs(eth->et_protlen) == PROT_ARP) {
-		struct arp_hdr *arp = packet + ETHER_HDR_SIZE;
-
-		if (ntohs(arp->ar_op) == ARPOP_REQUEST) {
-			struct ethernet_hdr *eth_recv;
-			struct arp_hdr *arp_recv;
-
-			/* 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;
-			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);
-
-			arp_recv = (void *)priv->recv_packet_buffer +
-				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_REPLY);
-			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, &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;
-		}
-	} else if (ntohs(eth->et_protlen) == PROT_IP) {
-		struct ip_udp_hdr *ip = packet + ETHER_HDR_SIZE;
-
-		if (ip->ip_p == IPPROTO_ICMP) {
-			struct icmp_hdr *icmp = (struct icmp_hdr *)&ip->udp_src;
-
-			if (icmp->type == ICMP_ECHO_REQUEST) {
-				struct ethernet_hdr *eth_recv;
-				struct ip_udp_hdr *ipr;
-				struct icmp_hdr *icmpr;
-
-				/* reply to the ping */
-				memcpy(priv->recv_packet_buffer, packet,
-				       length);
-				eth_recv = (void *)priv->recv_packet_buffer;
-				ipr = (void *)priv->recv_packet_buffer +
-					ETHER_HDR_SIZE;
-				icmpr = (struct icmp_hdr *)&ipr->udp_src;
-				memcpy(eth_recv->et_dest, eth->et_src,
-				       ARP_HLEN);
-				memcpy(eth_recv->et_src, priv->fake_host_hwaddr,
-				       ARP_HLEN);
-				ipr->ip_sum = 0;
-				ipr->ip_off = 0;
-				net_copy_ip((void *)&ipr->ip_dst, &ip->ip_src);
-				net_write_ip((void *)&ipr->ip_src,
-					     priv->fake_host_ipaddr);
-				ipr->ip_sum = compute_ip_checksum(ipr,
-					IP_HDR_SIZE);
-
-				icmpr->type = ICMP_ECHO_REPLY;
-				icmpr->checksum = 0;
-				icmpr->checksum = compute_ip_checksum(icmpr,
-					ICMP_HDR_SIZE);
-
-				priv->recv_packet_length = length;
-			}
-		}
-	}
-
-	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;
 }
 
 static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
-- 
2.11.0

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

* [U-Boot] [PATCH v2 03/10] net: sandbox: Make the fake eth driver response configurable
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 04/10] net: sandbox: Share the priv structure with tests Joe Hershberger
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

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;
 }
-- 
2.11.0

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

* [U-Boot] [PATCH v2 04/10] net: sandbox: Share the priv structure with tests
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (2 preceding siblings ...)
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 03/10] net: sandbox: Make the fake eth driver response configurable Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 05/10] net: sandbox: Allow fake eth to handle more than 1 packet response Joe Hershberger
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

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;
 
 /*
-- 
2.11.0

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

* [U-Boot] [PATCH v2 05/10] net: sandbox: Allow fake eth to handle more than 1 packet response
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (3 preceding siblings ...)
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 04/10] net: sandbox: Share the priv structure with tests Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP Joe Hershberger
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

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,
 };
-- 
2.11.0

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

* [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (4 preceding siblings ...)
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 05/10] net: sandbox: Allow fake eth to handle more than 1 packet response Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-09-27  6:38   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use Joe Hershberger
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

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;
+}
-- 
2.11.0

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

* [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (5 preceding siblings ...)
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP Joe Hershberger
@ 2018-09-26 21:48 ` Joe Hershberger
  2018-09-27  6:52   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 08/10] test: eth: Add a test for ARP requests Joe Hershberger
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:48 UTC (permalink / raw)
  To: u-boot

Tests need to be able to pass their "unit test state" to the handlers
where asserts are evaluated. Add a function that allows the tests to set
this private data on the sandbox eth device.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

---

Changes in v2:
- add missing commit message
- rename local "uc_priv" variable to "dev_priv" to not be misleading.

 arch/sandbox/include/asm/eth.h |  9 +++++++++
 drivers/net/sandbox.c          | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h
index 7a49e98fa8..a6adec4e83 100644
--- a/arch/sandbox/include/asm/eth.h
+++ b/arch/sandbox/include/asm/eth.h
@@ -59,6 +59,7 @@ typedef int sandbox_eth_tx_hand_f(struct udevice *dev, void *pkt,
  * 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
+ * priv - a pointer to some structure a test may want to keep track of
  */
 struct eth_sandbox_priv {
 	uchar fake_host_hwaddr[ARP_HLEN];
@@ -68,6 +69,7 @@ struct eth_sandbox_priv {
 	int recv_packet_length[PKTBUFSRX];
 	int recv_packets;
 	sandbox_eth_tx_hand_f *tx_handler;
+	void *priv;
 };
 
 /*
@@ -77,4 +79,11 @@ struct eth_sandbox_priv {
  */
 void sandbox_eth_set_tx_handler(int index, sandbox_eth_tx_hand_f *handler);
 
+/*
+ * Set priv ptr
+ *
+ * priv - priv void ptr to store in the device
+ */
+void sandbox_eth_set_priv(int index, void *priv);
+
 #endif /* __ETH_H */
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
index 9c0e0d009e..e26e72ecc1 100644
--- a/drivers/net/sandbox.c
+++ b/drivers/net/sandbox.c
@@ -206,6 +206,26 @@ void sandbox_eth_set_tx_handler(int index, sandbox_eth_tx_hand_f *handler)
 		priv->tx_handler = sb_default_handler;
 }
 
+/*
+ * Set priv ptr
+ *
+ * priv - priv void ptr to store in the device
+ */
+void sandbox_eth_set_priv(int index, void *priv)
+{
+	struct udevice *dev;
+	struct eth_sandbox_priv *dev_priv;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_ETH, index, &dev);
+	if (ret)
+		return;
+
+	dev_priv = dev_get_priv(dev);
+
+	dev_priv->priv = priv;
+}
+
 static int sb_eth_start(struct udevice *dev)
 {
 	struct eth_sandbox_priv *priv = dev_get_priv(dev);
-- 
2.11.0

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

* [U-Boot] [PATCH v2 08/10] test: eth: Add a test for ARP requests
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (6 preceding siblings ...)
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use Joe Hershberger
@ 2018-09-26 21:49 ` Joe Hershberger
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 09/10] test: eth: Add a test for the target being pinged Joe Hershberger
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 10/10] net: Don't overwrite waiting packets with asynchronous replies Joe Hershberger
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:49 UTC (permalink / raw)
  To: u-boot

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 at 10002000");
+	ut_assert(net_loop(PING) == -ETIMEDOUT);
+	ut_asserteq_str("eth at 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);
-- 
2.11.0

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

* [U-Boot] [PATCH v2 09/10] test: eth: Add a test for the target being pinged
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (7 preceding siblings ...)
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 08/10] test: eth: Add a test for ARP requests Joe Hershberger
@ 2018-09-26 21:49 ` Joe Hershberger
  2018-10-11 19:26   ` [U-Boot] " Joe Hershberger
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 10/10] net: Don't overwrite waiting packets with asynchronous replies Joe Hershberger
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:49 UTC (permalink / raw)
  To: u-boot

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 at 10002000");
+	ut_assert(net_loop(PING) == -ETIMEDOUT);
+	ut_asserteq_str("eth at 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);
-- 
2.11.0

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

* [U-Boot] [PATCH v2 10/10] net: Don't overwrite waiting packets with asynchronous replies
  2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
                   ` (8 preceding siblings ...)
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 09/10] test: eth: Add a test for the target being pinged Joe Hershberger
@ 2018-09-26 21:49 ` Joe Hershberger
  2018-10-11 19:26   ` [U-Boot] " Joe Hershberger
  9 siblings, 1 reply; 24+ messages in thread
From: Joe Hershberger @ 2018-09-26 21:49 UTC (permalink / raw)
  To: u-boot

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 at 10002000");
-	ut_assert(net_loop(PING) == -ETIMEDOUT);
+	ut_assertok(net_loop(PING));
 	ut_asserteq_str("eth at 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 at 10002000");
-	ut_assert(net_loop(PING) == -ETIMEDOUT);
+	ut_assertok(net_loop(PING));
 	ut_asserteq_str("eth at 10002000", env_get("ethact"));
 
 	sandbox_eth_set_tx_handler(0, NULL);
-- 
2.11.0

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

* [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function Joe Hershberger
@ 2018-09-27  6:38   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2018-09-27  6:38 UTC (permalink / raw)
  To: u-boot

Hi Joe,

On Thu, Sep 27, 2018 at 5:49 AM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> Make the behavior of the send function reusable.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> ---
>
> Changes in v2:
> - Added parameter comments
> - Changed return value to use typical error approach
> - In test, stop calling reply functions when one matches
>
>  arch/sandbox/include/asm/eth.h |  26 ++++++
>  drivers/net/sandbox.c          | 180 ++++++++++++++++++++++++-----------------
>  2 files changed, 132 insertions(+), 74 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

But please see comments below.

> diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h
> index bfcd11b593..3dc84f0e4b 100644
> --- a/arch/sandbox/include/asm/eth.h
> +++ b/arch/sandbox/include/asm/eth.h
> @@ -13,4 +13,30 @@ void sandbox_eth_disable_response(int index, bool disable);
>
>  void sandbox_eth_skip_timeout(void);
>
> +/*
> + * sandbox_eth_arp_req_to_reply()
> + *
> + * Check for an arp request to be sent. If so, inject a reply
> + *
> + * @dev: device that received the packet
> + * @packet: pointer to the received pacaket buffer
> + * @len: length of received packet
> + * @return 0 if injected, -EAGAIN if not
> + */
> +int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet,
> +                                unsigned int len);
> +
> +/*
> + * sandbox_eth_ping_req_to_reply()
> + *
> + * Check for a ping request to be sent. If so, inject a reply
> + *
> + * @dev: device that received the packet
> + * @packet: pointer to the received pacaket buffer
> + * @len: length of received packet
> + * @return 0 if injected, -EAGAIN if not
> + */
> +int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet,
> +                                 unsigned int len);
> +
>  #endif /* __ETH_H */
> diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
> index 60fe065ee5..c472261568 100644
> --- a/drivers/net/sandbox.c
> +++ b/drivers/net/sandbox.c
> @@ -63,6 +63,108 @@ void sandbox_eth_skip_timeout(void)
>         skip_timeout = true;
>  }
>
> +/*
> + * sandbox_eth_arp_req_to_reply()
> + *
> + * Check for an arp request to be sent. If so, inject a reply
> + *
> + * returns 0 if injected, -EAGAIN if not
> + */
> +int sandbox_eth_arp_req_to_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;
> +       struct ethernet_hdr *eth_recv;
> +       struct arp_hdr *arp_recv;
> +
> +       if (ntohs(eth->et_protlen) != PROT_ARP)
> +               return -EAGAIN;
> +
> +       arp = packet + ETHER_HDR_SIZE;
> +
> +       if (ntohs(arp->ar_op) != ARPOP_REQUEST)
> +               return -EAGAIN;
> +
> +       /* 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;
> +       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);
> +
> +       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_REPLY);
> +       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, &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;
> +
> +       return 0;
> +}
> +
> +/*
> + * sandbox_eth_ping_req_to_reply()
> + *
> + * Check for a ping request to be sent. If so, inject a reply
> + *
> + * returns 0 if injected, -EAGAIN if not
> + */
> +int sandbox_eth_ping_req_to_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;
> +       struct ethernet_hdr *eth_recv;
> +       struct ip_udp_hdr *ipr;
> +       struct icmp_hdr *icmpr;
> +
> +       if (ntohs(eth->et_protlen) != PROT_IP)
> +               return -EAGAIN;
> +
> +       ip = packet + ETHER_HDR_SIZE;
> +
> +       if (ip->ip_p != IPPROTO_ICMP)
> +               return -EAGAIN;
> +
> +       icmp = (struct icmp_hdr *)&ip->udp_src;
> +
> +       if (icmp->type != ICMP_ECHO_REQUEST)
> +               return -EAGAIN;
> +
> +       /* reply to the ping */
> +       eth_recv = (void *)priv->recv_packet_buffer;
> +       memcpy(eth_recv, packet, len);
> +       ipr = (void *)eth_recv + ETHER_HDR_SIZE;
> +       icmpr = (struct icmp_hdr *)&ipr->udp_src;
> +       memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN);
> +       memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN);
> +       ipr->ip_sum = 0;
> +       ipr->ip_off = 0;
> +       net_copy_ip((void *)&ipr->ip_dst, &ip->ip_src);
> +       net_write_ip((void *)&ipr->ip_src, priv->fake_host_ipaddr);
> +       ipr->ip_sum = compute_ip_checksum(ipr, IP_HDR_SIZE);
> +
> +       icmpr->type = ICMP_ECHO_REPLY;
> +       icmpr->checksum = 0;
> +       icmpr->checksum = compute_ip_checksum(icmpr, ICMP_HDR_SIZE);
> +
> +       priv->recv_packet_length = len;
> +
> +       return 0;
> +}
> +
>  static int sb_eth_start(struct udevice *dev)
>  {
>         struct eth_sandbox_priv *priv = dev_get_priv(dev);
> @@ -77,86 +179,16 @@ static int sb_eth_start(struct udevice *dev)
>  static int sb_eth_send(struct udevice *dev, void *packet, int length)
>  {
>         struct eth_sandbox_priv *priv = dev_get_priv(dev);
> -       struct ethernet_hdr *eth = packet;
>
>         debug("eth_sandbox: Send packet %d\n", length);
>
>         if (priv->disabled)
>                 return 0;
>
> -       if (ntohs(eth->et_protlen) == PROT_ARP) {
> -               struct arp_hdr *arp = packet + ETHER_HDR_SIZE;
> -
> -               if (ntohs(arp->ar_op) == ARPOP_REQUEST) {
> -                       struct ethernet_hdr *eth_recv;
> -                       struct arp_hdr *arp_recv;
> -
> -                       /* 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;
> -                       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);
> -
> -                       arp_recv = (void *)priv->recv_packet_buffer +
> -                               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_REPLY);
> -                       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, &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;
> -               }
> -       } else if (ntohs(eth->et_protlen) == PROT_IP) {
> -               struct ip_udp_hdr *ip = packet + ETHER_HDR_SIZE;
> -
> -               if (ip->ip_p == IPPROTO_ICMP) {
> -                       struct icmp_hdr *icmp = (struct icmp_hdr *)&ip->udp_src;
> -
> -                       if (icmp->type == ICMP_ECHO_REQUEST) {
> -                               struct ethernet_hdr *eth_recv;
> -                               struct ip_udp_hdr *ipr;
> -                               struct icmp_hdr *icmpr;
> -
> -                               /* reply to the ping */
> -                               memcpy(priv->recv_packet_buffer, packet,
> -                                      length);
> -                               eth_recv = (void *)priv->recv_packet_buffer;
> -                               ipr = (void *)priv->recv_packet_buffer +
> -                                       ETHER_HDR_SIZE;
> -                               icmpr = (struct icmp_hdr *)&ipr->udp_src;
> -                               memcpy(eth_recv->et_dest, eth->et_src,
> -                                      ARP_HLEN);
> -                               memcpy(eth_recv->et_src, priv->fake_host_hwaddr,
> -                                      ARP_HLEN);
> -                               ipr->ip_sum = 0;
> -                               ipr->ip_off = 0;
> -                               net_copy_ip((void *)&ipr->ip_dst, &ip->ip_src);
> -                               net_write_ip((void *)&ipr->ip_src,
> -                                            priv->fake_host_ipaddr);
> -                               ipr->ip_sum = compute_ip_checksum(ipr,
> -                                       IP_HDR_SIZE);
> -
> -                               icmpr->type = ICMP_ECHO_REPLY;
> -                               icmpr->checksum = 0;
> -                               icmpr->checksum = compute_ip_checksum(icmpr,
> -                                       ICMP_HDR_SIZE);
> -
> -                               priv->recv_packet_length = length;
> -                       }
> -               }
> -       }
> -
> -       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;

I think you will get a warning here: no return value specified ..

>  }
>
>  static int sb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
> --

Regards,
Bin

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

* [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP Joe Hershberger
@ 2018-09-27  6:38   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2018-09-27  6:38 UTC (permalink / raw)
  To: u-boot

On Thu, Sep 27, 2018 at 5:49 AM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> 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(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use Joe Hershberger
@ 2018-09-27  6:52   ` Bin Meng
  2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2018-09-27  6:52 UTC (permalink / raw)
  To: u-boot

On Thu, Sep 27, 2018 at 5:49 AM Joe Hershberger <joe.hershberger@ni.com> wrote:
>
> Tests need to be able to pass their "unit test state" to the handlers
> where asserts are evaluated. Add a function that allows the tests to set
> this private data on the sandbox eth device.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> ---
>
> Changes in v2:
> - add missing commit message
> - rename local "uc_priv" variable to "dev_priv" to not be misleading.
>
>  arch/sandbox/include/asm/eth.h |  9 +++++++++
>  drivers/net/sandbox.c          | 20 ++++++++++++++++++++
>  2 files changed, 29 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

* [U-Boot] net: sandbox: Move disabled flag into priv struct
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct Joe Hershberger
@ 2018-10-11 19:25   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975411/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: sandbox: Refactor sandbox send function
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function Joe Hershberger
  2018-09-27  6:38   ` Bin Meng
@ 2018-10-11 19:25   ` Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975414/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: sandbox: Make the fake eth driver response configurable
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 03/10] net: sandbox: Make the fake eth driver response configurable Joe Hershberger
@ 2018-10-11 19:25   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975408/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: sandbox: Share the priv structure with tests
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 04/10] net: sandbox: Share the priv structure with tests Joe Hershberger
@ 2018-10-11 19:25   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975409/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: sandbox: Allow fake eth to handle more than 1 packet response
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 05/10] net: sandbox: Allow fake eth to handle more than 1 packet response Joe Hershberger
@ 2018-10-11 19:25   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975413/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: Add an accessor to know if waiting for ARP
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP Joe Hershberger
  2018-09-27  6:38   ` Bin Meng
@ 2018-10-11 19:25   ` Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975417/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: sandbox: Add a priv ptr for tests to use
  2018-09-26 21:48 ` [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use Joe Hershberger
  2018-09-27  6:52   ` Bin Meng
@ 2018-10-11 19:25   ` Joe Hershberger
  1 sibling, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975419/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] test: eth: Add a test for ARP requests
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 08/10] test: eth: Add a test for ARP requests Joe Hershberger
@ 2018-10-11 19:25   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:25 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975412/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] test: eth: Add a test for the target being pinged
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 09/10] test: eth: Add a test for the target being pinged Joe Hershberger
@ 2018-10-11 19:26   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:26 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975410/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

* [U-Boot] net: Don't overwrite waiting packets with asynchronous replies
  2018-09-26 21:49 ` [U-Boot] [PATCH v2 10/10] net: Don't overwrite waiting packets with asynchronous replies Joe Hershberger
@ 2018-10-11 19:26   ` Joe Hershberger
  0 siblings, 0 replies; 24+ messages in thread
From: Joe Hershberger @ 2018-10-11 19:26 UTC (permalink / raw)
  To: u-boot

Hi Joe,

https://patchwork.ozlabs.org/patch/975416/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe

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

end of thread, other threads:[~2018-10-11 19:26 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 21:48 [U-Boot] [PATCH v2 00/10] net: Fix packet corruption issue when handling asynch replies Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 01/10] net: sandbox: Move disabled flag into priv struct Joe Hershberger
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 02/10] net: sandbox: Refactor sandbox send function Joe Hershberger
2018-09-27  6:38   ` Bin Meng
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 03/10] net: sandbox: Make the fake eth driver response configurable Joe Hershberger
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 04/10] net: sandbox: Share the priv structure with tests Joe Hershberger
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 05/10] net: sandbox: Allow fake eth to handle more than 1 packet response Joe Hershberger
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 06/10] net: Add an accessor to know if waiting for ARP Joe Hershberger
2018-09-27  6:38   ` Bin Meng
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:48 ` [U-Boot] [PATCH v2 07/10] net: sandbox: Add a priv ptr for tests to use Joe Hershberger
2018-09-27  6:52   ` Bin Meng
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:49 ` [U-Boot] [PATCH v2 08/10] test: eth: Add a test for ARP requests Joe Hershberger
2018-10-11 19:25   ` [U-Boot] " Joe Hershberger
2018-09-26 21:49 ` [U-Boot] [PATCH v2 09/10] test: eth: Add a test for the target being pinged Joe Hershberger
2018-10-11 19:26   ` [U-Boot] " Joe Hershberger
2018-09-26 21:49 ` [U-Boot] [PATCH v2 10/10] net: Don't overwrite waiting packets with asynchronous replies Joe Hershberger
2018-10-11 19:26   ` [U-Boot] " Joe Hershberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.