[U-Boot] [PATCH 0/1] Store waiting packet in a different buffer when making ARP requests

[U-Boot] [PATCH 0/1] Store waiting packet in a different buffer when making ARP requests

[Tran Tien Dat]

Hi,

This is my first email to U-Boot so please let me know if I am not doing it
correctly.

U-Boot has 1 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 to find out the MAC address of the IP address, and continue
sending the frame in the net_tx_packet after receiving a matching ARP
reply. However, in the mean time, if U-Boot needs to send out any network
packets (e.g. replying ping packets, etc.), the content of net_tx_packet
will be used to prepare the new packet. Thus it is no longer the packet
that is meant to be transmitted after the ARP reply.

U-Boot has another buffer, pointed to by arp_tx_packet which is used to
prepare ARP requests. ARP requests use this buffer instead of the normal
net_tx_packet in order to avoid modifying the waiting packet to be sent.
However, this approach does not prevent other part of the codes from
modifying the waiting packet to be sent, because net_tx_packet is a common
buffer used by all network protocol. This patch repurposes the
arp_tx_packet buffer to be used to store the waiting packet to be sent, and
use the normal net_tx_packet buffer to send ARP request instead.

Best Regards,
Dat



Tran Tien Dat (1):
  net: Store waiting packet in a different buffer when making ARP
    requests

 net/arp.c | 18 ++++++++++--------
 net/arp.h |  1 +
 net/net.c |  3 +++
 3 files changed, 14 insertions(+), 8 deletions(-)

-- 
2.18.0

[U-Boot] [PATCH 1/1] net: Store waiting packet in a different buffer when making ARP requests

[Tran Tien Dat]

Currently, upon receiving an appropriate ARP reply, the packet in
net_tx_packet is sent. However, this is a common buffer used by other
protocol as well, so it may not be the original packet waiting to be sent
after ARP.

This patch repurposes another buffer, arp_tx_packet to store the waiting
packet and use the net_tx_packet to prepare ARP request.

Signed-off-by: Tran Tien Dat <peter.trantiendat@gmail.com>
---

 net/arp.c | 18 ++++++++++--------
 net/arp.h |  1 +
 net/net.c |  3 +++
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/net/arp.c b/net/arp.c
index b8a71684cd..f5e2c0b0cf 100644
--- a/net/arp.c
+++ b/net/arp.c
@@ -35,8 +35,8 @@ int		arp_wait_tx_packet_size;
 ulong		arp_wait_timer_start;
 int		arp_wait_try;
 
-static uchar   *arp_tx_packet;	/* THE ARP transmit packet */
-static uchar	arp_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN];
+uchar   *arp_wait_tx_packet;	/* THE waiting transmit packet after ARP */
+static uchar	arp_wait_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN];
 
 void arp_init(void)
 {
@@ -45,8 +45,8 @@ void arp_init(void)
 	net_arp_wait_packet_ip.s_addr = 0;
 	net_arp_wait_reply_ip.s_addr = 0;
 	arp_wait_tx_packet_size = 0;
-	arp_tx_packet = &arp_tx_packet_buf[0] + (PKTALIGN - 1);
-	arp_tx_packet -= (ulong)arp_tx_packet % PKTALIGN;
+	arp_wait_tx_packet = &arp_wait_tx_packet_buf[0] + (PKTALIGN - 1);
+	arp_wait_tx_packet -= (ulong)arp_wait_tx_packet % PKTALIGN;
 }
 
 void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
@@ -58,7 +58,7 @@ void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
 
 	debug_cond(DEBUG_DEV_PKT, "ARP broadcast %d\n", arp_wait_try);
 
-	pkt = arp_tx_packet;
+	pkt = net_tx_packet;
 
 	eth_hdr_size = net_set_ether(pkt, net_bcast_ethaddr, PROT_ARP);
 	pkt += eth_hdr_size;
@@ -76,7 +76,7 @@ void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
 	memcpy(&arp->ar_tha, target_ethaddr, ARP_HLEN);	/* target ET addr */
 	net_write_ip(&arp->ar_tpa, target_ip);		/* target IP addr */
 
-	net_send_packet(arp_tx_packet, eth_hdr_size + ARP_HDR_SIZE);
+	net_send_packet(net_tx_packet, eth_hdr_size + ARP_HDR_SIZE);
 }
 
 void arp_request(void)
@@ -217,9 +217,11 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
 
 			/* set the mac address in the waiting packet's header
 			   and transmit it */
-			memcpy(((struct ethernet_hdr *)net_tx_packet)->et_dest,
+			memcpy(((struct ethernet_hdr *)arp_wait_tx_packet)
+					->et_dest,
 			       &arp->ar_sha, ARP_HLEN);
-			net_send_packet(net_tx_packet, arp_wait_tx_packet_size);
+			net_send_packet(arp_wait_tx_packet,
+					arp_wait_tx_packet_size);
 
 			/* no arp request pending now */
 			net_arp_wait_packet_ip.s_addr = 0;
diff --git a/net/arp.h b/net/arp.h
index afb86958f3..65d73927a7 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_wait_tx_packet;
 
 void arp_init(void);
 void arp_request(void);
diff --git a/net/net.c b/net/net.c
index f35695b4fc..6325ad3e1a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -836,6 +836,9 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
 
 		/* size of the waiting packet */
 		arp_wait_tx_packet_size = pkt_hdr_size + payload_len;
+		/* copy current packet to ARP waiting packet buffer */
+		memcpy(arp_wait_tx_packet, net_tx_packet,
+		       arp_wait_tx_packet_size);
 
 		/* and do the ARP request */
 		arp_wait_try = 1;
-- 
2.18.0

[U-Boot] [PATCH 1/1] net: Store waiting packet in a different buffer when making ARP requests

[Joe Hershberger]

On Wed, Jul 4, 2018 at 12:15 AM, Tran Tien Dat
<peter.trantiendat@gmail.com> wrote:
> Currently, upon receiving an appropriate ARP reply, the packet in
> net_tx_packet is sent. However, this is a common buffer used by other
> protocol as well, so it may not be the original packet waiting to be sent
> after ARP.

You'll need to be more detailed here. "used by other protocol as well"
doesn't say which one or what situation caused a problem. Please
include a clear description of what this is fixing.

>
> This patch repurposes another buffer, arp_tx_packet to store the waiting
> packet and use the net_tx_packet to prepare ARP request.
>
> Signed-off-by: Tran Tien Dat <peter.trantiendat@gmail.com>
> ---
>
>  net/arp.c | 18 ++++++++++--------
>  net/arp.h |  1 +
>  net/net.c |  3 +++
>  3 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/net/arp.c b/net/arp.c
> index b8a71684cd..f5e2c0b0cf 100644
> --- a/net/arp.c
> +++ b/net/arp.c
> @@ -35,8 +35,8 @@ int           arp_wait_tx_packet_size;
>  ulong          arp_wait_timer_start;
>  int            arp_wait_try;
>
> -static uchar   *arp_tx_packet; /* THE ARP transmit packet */
> -static uchar   arp_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN];
> +uchar   *arp_wait_tx_packet;   /* THE waiting transmit packet after ARP */
> +static uchar   arp_wait_tx_packet_buf[PKTSIZE_ALIGN + PKTALIGN];
>
>  void arp_init(void)
>  {
> @@ -45,8 +45,8 @@ void arp_init(void)
>         net_arp_wait_packet_ip.s_addr = 0;
>         net_arp_wait_reply_ip.s_addr = 0;
>         arp_wait_tx_packet_size = 0;
> -       arp_tx_packet = &arp_tx_packet_buf[0] + (PKTALIGN - 1);
> -       arp_tx_packet -= (ulong)arp_tx_packet % PKTALIGN;
> +       arp_wait_tx_packet = &arp_wait_tx_packet_buf[0] + (PKTALIGN - 1);
> +       arp_wait_tx_packet -= (ulong)arp_wait_tx_packet % PKTALIGN;
>  }
>
>  void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
> @@ -58,7 +58,7 @@ void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
>
>         debug_cond(DEBUG_DEV_PKT, "ARP broadcast %d\n", arp_wait_try);
>
> -       pkt = arp_tx_packet;
> +       pkt = net_tx_packet;
>
>         eth_hdr_size = net_set_ether(pkt, net_bcast_ethaddr, PROT_ARP);
>         pkt += eth_hdr_size;
> @@ -76,7 +76,7 @@ void arp_raw_request(struct in_addr source_ip, const uchar *target_ethaddr,
>         memcpy(&arp->ar_tha, target_ethaddr, ARP_HLEN); /* target ET addr */
>         net_write_ip(&arp->ar_tpa, target_ip);          /* target IP addr */
>
> -       net_send_packet(arp_tx_packet, eth_hdr_size + ARP_HDR_SIZE);
> +       net_send_packet(net_tx_packet, eth_hdr_size + ARP_HDR_SIZE);
>  }
>
>  void arp_request(void)
> @@ -217,9 +217,11 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
>
>                         /* set the mac address in the waiting packet's header
>                            and transmit it */
> -                       memcpy(((struct ethernet_hdr *)net_tx_packet)->et_dest,
> +                       memcpy(((struct ethernet_hdr *)arp_wait_tx_packet)
> +                                       ->et_dest,
>                                &arp->ar_sha, ARP_HLEN);
> -                       net_send_packet(net_tx_packet, arp_wait_tx_packet_size);
> +                       net_send_packet(arp_wait_tx_packet,
> +                                       arp_wait_tx_packet_size);
>
>                         /* no arp request pending now */
>                         net_arp_wait_packet_ip.s_addr = 0;
> diff --git a/net/arp.h b/net/arp.h
> index afb86958f3..65d73927a7 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_wait_tx_packet;
>
>  void arp_init(void);
>  void arp_request(void);
> diff --git a/net/net.c b/net/net.c
> index f35695b4fc..6325ad3e1a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -836,6 +836,9 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
>
>                 /* size of the waiting packet */
>                 arp_wait_tx_packet_size = pkt_hdr_size + payload_len;
> +               /* copy current packet to ARP waiting packet buffer */
> +               memcpy(arp_wait_tx_packet, net_tx_packet,
> +                      arp_wait_tx_packet_size);
>
>                 /* and do the ARP request */
>                 arp_wait_try = 1;
> --
> 2.18.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot