[U-Boot] [PATCH v2 18/18] net: Don't copy every packet that waits for an ARP

Simon Glass sjg at chromium.org
Sat Apr 21 04:41:50 CEST 2012


Hi Joe,

On Tue, Mar 27, 2012 at 4:43 PM, Joe Hershberger <joe.hershberger at ni.com> wrote:
> Use the NetArpTxPacket for the ARP packet, not to hold what used to
> be in NetTxPacket.
> This saves a copy and makes the code easier to understand.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Joe Hershberger <joe.hershberger at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Mike Frysinger <vapier at gentoo.org>

Acked-by: Simon Glass <sjg at chromium.org>

> ---
>  include/net.h |    4 ++--
>  net/arp.c     |   24 ++++++++++++------------
>  net/arp.h     |    2 --
>  net/net.c     |   24 ++++++------------------
>  net/ping.c    |    5 ++---
>  5 files changed, 22 insertions(+), 37 deletions(-)
>
> diff --git a/include/net.h b/include/net.h
> index 2c47604..b116d36 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -473,13 +473,13 @@ static inline void net_set_state(enum net_loop_state state)
>        net_state = state;
>  }
>
> -/* Transmit "NetTxPacket" */
> +/* Transmit a packet */
>  static inline void NetSendPacket(uchar *pkt, int len)
>  {
>        (void) eth_send(pkt, len);
>  }
>
> -/* Transmit UDP packet, performing ARP request if needed
> +/* Transmit "NetTxPacket" as UDP packet, performing ARP request if needed
>    (ether will be populated) */

Should fix up comment style.

>  extern int     NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport,
>                        int sport, int payload_len);
> diff --git a/net/arp.c b/net/arp.c
> index 7d0297d..e6c929b 100644
> --- a/net/arp.c
> +++ b/net/arp.c
> @@ -30,22 +30,22 @@ IPaddr_t    NetArpWaitPacketIP;
>  IPaddr_t       NetArpWaitReplyIP;
>  /* MAC address of waiting packet's destination */
>  uchar         *NetArpWaitPacketMAC;
> -/* THE transmit packet */
> -uchar         *NetArpWaitTxPacket;
>  int            NetArpWaitTxPacketSize;
> -uchar          NetArpWaitPacketBuf[PKTSIZE_ALIGN + PKTALIGN];
>  ulong          NetArpWaitTimerStart;
>  int            NetArpWaitTry;
>
> +uchar         *NetArpTxPacket; /* THE ARP transmit packet */
> +uchar          NetArpPacketBuf[PKTSIZE_ALIGN + PKTALIGN];
> +
>  void ArpInit(void)
>  {
>        /* XXX problem with bss workaround */
>        NetArpWaitPacketMAC = NULL;
>        NetArpWaitPacketIP = 0;
>        NetArpWaitReplyIP = 0;
> -       NetArpWaitTxPacket = &NetArpWaitPacketBuf[0] + (PKTALIGN - 1);
> -       NetArpWaitTxPacket -= (ulong)NetArpWaitTxPacket % PKTALIGN;
>        NetArpWaitTxPacketSize = 0;
> +       NetArpTxPacket = &NetArpPacketBuf[0] + (PKTALIGN - 1);
> +       NetArpTxPacket -= (ulong)NetArpTxPacket % PKTALIGN;

I don't understand what this is doing. Perhaps that is just as well.

>  }
>
>  void ArpRequest(void)
> @@ -56,7 +56,7 @@ void ArpRequest(void)
>
>        debug("ARP broadcast %d\n", NetArpWaitTry);
>
> -       pkt = NetTxPacket;
> +       pkt = NetArpTxPacket;
>
>        eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_ARP);
>        pkt += eth_hdr_size;
> @@ -88,7 +88,7 @@ void ArpRequest(void)
>        }
>
>        NetWriteIP(&arp->ar_tpa, NetArpWaitReplyIP);
> -       NetSendPacket(NetTxPacket, eth_hdr_size + ARP_HDR_SIZE);
> +       NetSendPacket(NetArpTxPacket, eth_hdr_size + ARP_HDR_SIZE);
>  }
>
>  void ArpTimeoutCheck(void)
> @@ -196,11 +196,11 @@ void ArpReceive(struct Ethernet_hdr *et, struct IP_UDP_hdr *ip, int len)
>                        net_get_arp_handler()((uchar *)arp, 0,
>                                reply_ip_addr, 0, len);
>
> -                       /* modify header, and transmit it */
> -                       memcpy(((struct Ethernet_hdr *)NetArpWaitTxPacket)->
> -                               et_dest, &arp->ar_sha, ARP_HLEN);
> -                       NetSendPacket(NetArpWaitTxPacket,
> -                                       NetArpWaitTxPacketSize);
> +                       /* set the mac address in the waiting packet's header
> +                          and transmit it */
> +                       memcpy(((struct Ethernet_hdr *)NetTxPacket)->et_dest,
> +                               &arp->ar_sha, ARP_HLEN);
> +                       NetSendPacket(NetTxPacket, NetArpWaitTxPacketSize);
>
>                        /* no arp request pending now */
>                        NetArpWaitPacketIP = 0;
> diff --git a/net/arp.h b/net/arp.h
> index 61a7a21..aba0c0b 100644
> --- a/net/arp.h
> +++ b/net/arp.h
> @@ -16,8 +16,6 @@
>  extern IPaddr_t        NetArpWaitPacketIP;
>  /* MAC address of waiting packet's destination */
>  extern uchar *NetArpWaitPacketMAC;
> -/* THE transmit packet */
> -extern uchar *NetArpWaitTxPacket;
>  extern int NetArpWaitTxPacketSize;
>  extern ulong NetArpWaitTimerStart;
>  extern int NetArpWaitTry;
> diff --git a/net/net.c b/net/net.c
> index 01e7b29..c41cfb2 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -443,6 +443,9 @@ restart:
>                 *      Abort if ctrl-c was pressed.
>                 */
>                if (ctrlc()) {
> +                       /* cancel any ARP that may not have completed */
> +                       NetArpWaitPacketIP = 0;
> +
>                        net_cleanup_loop();
>                        eth_halt();
>                        puts("\nAbort\n");
> @@ -634,7 +637,6 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport,
>                int payload_len)
>  {
>        uchar *pkt;
> -       int need_arp = 0;
>        int eth_hdr_size;
>        int pkt_hdr_size;
>
> @@ -651,35 +653,21 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport,
>        if (dest == 0xFFFFFFFF)
>                ether = NetBcastAddr;
>
> -       /*
> -        * if MAC address was not discovered yet, save the packet and do
> -        * an ARP request
> -        */
> -       if (memcmp(ether, NetEtherNullAddr, 6) == 0) {
> -               need_arp = 1;
> -               pkt = NetArpWaitTxPacket;
> -       } else
> -               pkt = (uchar *)NetTxPacket;
> +       pkt = (uchar *)NetTxPacket;
>
>        eth_hdr_size = NetSetEther(pkt, ether, PROT_IP);
>        pkt += eth_hdr_size;
>        net_set_udp_header(pkt, dest, dport, sport, payload_len);
>        pkt_hdr_size = eth_hdr_size + IP_UDP_HDR_SIZE;
>
> -       if (need_arp) {
> +       /* if MAC address was not discovered yet, do an ARP request */
> +       if (memcmp(ether, NetEtherNullAddr, 6) == 0) {
>                debug("sending ARP for %pI4\n", &dest);
>
>                /* save the ip and eth addr for the packet to send after arp */
>                NetArpWaitPacketIP = dest;
>                NetArpWaitPacketMAC = ether;
>
> -               /*
> -                * Copy the packet data from the NetTxPacket into the
> -                *   NetArpWaitTxPacket to send after arp
> -                */
> -               memcpy(pkt + IP_UDP_HDR_SIZE, (uchar *)NetTxPacket +
> -                       pkt_hdr_size, payload_len);
> -
>                /* size of the waiting packet */
>                NetArpWaitTxPacketSize = pkt_hdr_size + payload_len;
>
> diff --git a/net/ping.c b/net/ping.c
> index 50a1047..e72f7a6 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -49,9 +49,8 @@ static int ping_send(void)
>
>        NetArpWaitPacketIP = NetPingIP;
>
> -       eth_hdr_size = NetSetEther(NetArpWaitTxPacket, NetEtherNullAddr,
> -               PROT_IP);
> -       pkt = NetArpWaitTxPacket + eth_hdr_size;
> +       eth_hdr_size = NetSetEther(NetTxPacket, NetEtherNullAddr, PROT_IP);
> +       pkt = (uchar *)NetTxPacket + eth_hdr_size;
>
>        set_icmp_header(pkt, NetPingIP);
>
> --
> 1.6.0.2
>

Regards,
Simon


More information about the U-Boot mailing list