Hi Joe,
On Tue, Mar 27, 2012 at 4:42 PM, Joe Hershberger <joe.hershber...@ni.com> wrote:
> Call a built-in dummy if none is registered... don't require
> protocols to register a handler (eliminating dummies)
> NetConsole now uses the ARP handler when waiting on arp
> (instead of needing a #define hack in arp.c)
> Clear handlers at the end of net loop
>
> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com>
> Cc: Joe Hershberger <joe.hershber...@gmail.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Mike Frysinger <vap...@gentoo.org>
Couple of questions, but not important.
Acked-by:: Simon Glass <s...@chromium.org>
> ---
> Changes for v2:
> - Change NULL checks into dummy functions
> - Must now call net_set_*_handler(NULL); in init to set dummies
> - Eliminate CamelCase for new functions
>
> drivers/net/netconsole.c | 4 +-
> include/net.h | 9 +++-
> net/arp.c | 6 +-
> net/bootp.c | 4 +-
> net/cdp.c | 8 ----
> net/dns.c | 2 +-
> net/net.c | 95
> ++++++++++++++++++++++++++++++----------------
> net/nfs.c | 2 +-
> net/sntp.c | 2 +-
> net/tftp.c | 4 +-
> 10 files changed, 80 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 28bb955..744f4d1 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -63,12 +63,12 @@ void NcStart(void)
> {
> if (!output_packet_len || memcmp(nc_ether, NetEtherNullAddr, 6)) {
> /* going to check for input packet */
> - NetSetHandler(nc_handler);
> + net_set_udp_handler(nc_handler);
> NetSetTimeout(net_timeout, nc_timeout);
> } else {
> /* send arp request */
> uchar *pkt;
> - NetSetHandler(nc_wait_arp_handler);
> + net_set_arp_handler(nc_wait_arp_handler);
> pkt = (uchar *)NetTxPacket + NetEthHdrSize() + IP_UDP_HDR_SIZE;
> memcpy(pkt, output_packet, output_packet_len);
> NetSendUDPPacket(nc_ether, nc_ip, nc_port, nc_port,
> diff --git a/include/net.h b/include/net.h
> index c33cd28..796d59f 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -454,8 +454,10 @@ extern int NetCksumOk(uchar *, int); /* Return
> true if cksum OK */
> extern uint NetCksum(uchar *, int); /* Calculate the checksum */
>
> /* Callbacks */
> -extern rxhand_f *NetGetHandler(void); /* Get RX packet handler */
> -extern void NetSetHandler(rxhand_f *); /* Set RX packet handler */
> +extern rxhand_f *net_get_udp_handler(void); /* Get UDP RX packet handler
> */
> +extern void net_set_udp_handler(rxhand_f *); /* Set UDP RX packet handler
> */
> +extern rxhand_f *net_get_arp_handler(void); /* Get ARP RX packet handler
> */
> +extern void net_set_arp_handler(rxhand_f *); /* Set ARP RX packet handler
> */
> extern void net_set_icmp_handler(rxhand_icmp_f *f); /* Set ICMP RX handler */
> extern void NetSetTimeout(ulong, thand_f *);/* Set timeout handler */
>
> @@ -475,7 +477,8 @@ static inline void NetSendPacket(uchar *pkt, int len)
> (void) eth_send(pkt, len);
> }
>
> -/* Transmit UDP packet, performing ARP request if needed */
> +/* Transmit UDP packet, performing ARP request if needed
> + (ether will be populated) */
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 90a6b01..90bcc09 100644
> --- a/net/arp.c
> +++ b/net/arp.c
> @@ -192,9 +192,9 @@ void ArpReceive(struct Ethernet_hdr *et, struct
> IP_UDP_hdr *ip, int len)
> memcpy(NetArpWaitPacketMAC,
> &arp->ar_sha, ARP_HLEN);
>
> -#ifdef CONFIG_NETCONSOLE
> - NetGetHandler()(0, 0, 0, 0, 0);
> -#endif
> + net_get_arp_handler()((uchar *)arp, 0,
> + reply_ip_addr, 0, len);
I presume this is too long for one line?
> +
> /* modify header, and transmit it */
> memcpy(((struct Ethernet_hdr *)NetArpWaitTxPacket)->
> et_dest, NetArpWaitPacketMAC, ARP_HLEN);
> diff --git a/net/bootp.c b/net/bootp.c
> index 2eeb14e..0cf8beb 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -671,9 +671,9 @@ BootpRequest(void)
>
> #if defined(CONFIG_CMD_DHCP)
> dhcp_state = SELECTING;
> - NetSetHandler(DhcpHandler);
> + net_set_udp_handler(DhcpHandler);
> #else
> - NetSetHandler(BootpHandler);
> + net_set_udp_handler(BootpHandler);
> #endif
> NetSendPacket(NetTxPacket, pktlen);
> }
> diff --git a/net/cdp.c b/net/cdp.c
> index f1c1c14..3b8a9a1 100644
> --- a/net/cdp.c
> +++ b/net/cdp.c
> @@ -237,13 +237,6 @@ CDPTimeout(void)
> net_set_state(NETLOOP_SUCCESS);
> }
>
> -static void
> -CDPDummyHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
> - unsigned len)
> -{
> - /* nothing */
> -}
> -
> void
> CDPReceive(const uchar *pkt, unsigned len)
> {
> @@ -368,7 +361,6 @@ CDPStart(void)
> CDPApplianceVLAN = htons(-1);
>
> NetSetTimeout(CDP_TIMEOUT, CDPTimeout);
> - NetSetHandler(CDPDummyHandler);
>
> CDPSendTrigger();
> }
> diff --git a/net/dns.c b/net/dns.c
> index cc0aa0a..ff9ddff 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -200,7 +200,7 @@ DnsStart(void)
> debug("%s\n", __func__);
>
> NetSetTimeout(DNS_TIMEOUT, DnsTimeout);
> - NetSetHandler(DnsHandler);
> + net_set_udp_handler(DnsHandler);
>
> DnsSend();
> }
> diff --git a/net/net.c b/net/net.c
> index 8076e5f..cac9406 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -178,10 +178,13 @@ uchar PktBuf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
> /* Receive packet */
> uchar *NetRxPackets[PKTBUFSRX];
>
> -/* Current RX packet handler */
> -static rxhand_f *packetHandler;
> +/* Current UDP RX packet handler */
> +static rxhand_f *udp_packet_handler;
> +/* Current ARP RX packet handler */
> +static rxhand_f *arp_packet_handler;
> #ifdef CONFIG_CMD_TFTPPUT
> -static rxhand_icmp_f *packet_icmp_handler; /* Current ICMP rx handler */
> +/* Current ICMP rx handler */
> +static rxhand_icmp_f *packet_icmp_handler;
> #endif
> /* Current timeout handler */
> static thand_f *timeHandler;
> @@ -252,6 +255,18 @@ static void NetInitLoop(enum proto_t protocol)
> return;
> }
>
> +static void net_clear_handlers(void)
> +{
> + net_set_udp_handler(NULL);
> + net_set_arp_handler(NULL);
> + NetSetTimeout(0, NULL);
> +}
> +
> +static void net_cleanup_loop(void)
> +{
> + net_clear_handlers();
If this is static anyway, and net_clear_handlers() also clear the
timeout, why not just put the three lines of code in here instead of a
separate function?
> +}
> +
> /**********************************************************************/
> /*
> * Main network processing loop.
> @@ -259,6 +274,7 @@ static void NetInitLoop(enum proto_t protocol)
>
> int NetLoop(enum proto_t protocol)
> {
> + int i;
> bd_t *bd = gd->bd;
> int ret = -1;
>
> @@ -269,17 +285,15 @@ int NetLoop(enum proto_t protocol)
> NetTryCount = 1;
>
> ArpInit();
> + net_clear_handlers();
>
> - if (!NetTxPacket) {
> - int i;
> - /*
> - * Setup packet buffers, aligned correctly.
> - */
> - NetTxPacket = &PktBuf[0] + (PKTALIGN - 1);
> - NetTxPacket -= (ulong)NetTxPacket % PKTALIGN;
> - for (i = 0; i < PKTBUFSRX; i++)
> - NetRxPackets[i] = NetTxPacket + (i+1)*PKTSIZE_ALIGN;
> - }
> + /*
> + * Setup packet buffers, aligned correctly.
> + */
> + NetTxPacket = &PktBuf[0] + (PKTALIGN - 1);
> + NetTxPacket -= (ulong)NetTxPacket % PKTALIGN;
> + for (i = 0; i < PKTBUFSRX; i++)
> + NetRxPackets[i] = NetTxPacket + (i+1)*PKTSIZE_ALIGN;
>
> bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> eth_halt();
> @@ -418,6 +432,7 @@ restart:
> * Abort if ctrl-c was pressed.
> */
> if (ctrlc()) {
> + net_cleanup_loop();
> eth_halt();
> puts("\nAbort\n");
> goto done;
> @@ -460,6 +475,7 @@ restart:
> goto restart;
>
> case NETLOOP_SUCCESS:
> + net_cleanup_loop();
> if (NetBootFileXferSize > 0) {
> char buf[20];
> printf("Bytes transferred = %ld (%lx hex)\n",
> @@ -476,6 +492,7 @@ restart:
> goto done;
>
> case NETLOOP_FAIL:
> + net_cleanup_loop();
> goto done;
>
> case NETLOOP_CONTINUE:
> @@ -486,7 +503,7 @@ restart:
> done:
> #ifdef CONFIG_CMD_TFTPPUT
> /* Clear out the handlers */
> - NetSetHandler(NULL);
> + net_set_upd_handler(NULL);
> net_set_icmp_handler(NULL);
> #endif
> return ret;
> @@ -500,13 +517,6 @@ startAgainTimeout(void)
> net_set_state(NETLOOP_RESTART);
> }
>
> -static void
> -startAgainHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
> - unsigned src, unsigned len)
> -{
> - /* Totally ignore the packet */
> -}
> -
> void NetStartAgain(void)
> {
> char *nretry;
> @@ -543,7 +553,7 @@ void NetStartAgain(void)
> NetRestartWrap = 0;
> if (NetDevExists) {
> NetSetTimeout(10000UL, startAgainTimeout);
> - NetSetHandler(startAgainHandler);
> + net_set_udp_handler(NULL);
> } else {
> net_set_state(NETLOOP_FAIL);
> }
> @@ -557,17 +567,36 @@ void NetStartAgain(void)
> * Miscelaneous bits.
> */
>
> -rxhand_f *
> -NetGetHandler(void)
> +static void dummy_handler(uchar *pkt, unsigned dport,
> + IPaddr_t sip, unsigned sport,
> + unsigned len)
> {
> - return packetHandler;
> }
>
> +rxhand_f *net_get_udp_handler(void)
> +{
> + return udp_packet_handler;
> +}
>
> -void
> -NetSetHandler(rxhand_f *f)
> +void net_set_udp_handler(rxhand_f *f)
> +{
> + if (f == NULL)
> + udp_packet_handler = dummy_handler;
> + else
> + udp_packet_handler = f;
> +}
> +
> +rxhand_f *net_get_arp_handler(void)
> {
> - packetHandler = f;
> + return arp_packet_handler;
> +}
> +
> +void net_set_arp_handler(rxhand_f *f)
> +{
> + if (f == NULL)
> + arp_packet_handler = dummy_handler;
> + else
> + arp_packet_handler = f;
> }
>
> #ifdef CONFIG_CMD_TFTPPUT
> @@ -1094,11 +1123,11 @@ NetReceive(uchar *inpkt, int len)
> /*
> * IP header OK. Pass the packet to the current handler.
> */
> - (*packetHandler)((uchar *)ip + IP_UDP_HDR_SIZE,
> - ntohs(ip->udp_dst),
> - src_ip,
> - ntohs(ip->udp_src),
> - ntohs(ip->udp_len) - UDP_HDR_SIZE);
> + (*udp_packet_handler)((uchar *)ip + IP_UDP_HDR_SIZE,
> + ntohs(ip->udp_dst),
> + src_ip,
> + ntohs(ip->udp_src),
> + ntohs(ip->udp_len) - UDP_HDR_SIZE);
> break;
> }
> }
> diff --git a/net/nfs.c b/net/nfs.c
> index 4f2041c..acae4de 100644
> --- a/net/nfs.c
> +++ b/net/nfs.c
> @@ -735,7 +735,7 @@ NfsStart(void)
> "Loading: *\b", load_addr);
>
> NetSetTimeout(NFS_TIMEOUT, NfsTimeout);
> - NetSetHandler(NfsHandler);
> + net_set_udp_handler(NfsHandler);
>
> NfsTimeoutCount = 0;
> NfsState = STATE_PRCLOOKUP_PROG_MOUNT_REQ;
> diff --git a/net/sntp.c b/net/sntp.c
> index 2adcc6f..73ade8b 100644
> --- a/net/sntp.c
> +++ b/net/sntp.c
> @@ -85,7 +85,7 @@ SntpStart(void)
> debug("%s\n", __func__);
>
> NetSetTimeout(SNTP_TIMEOUT, SntpTimeout);
> - NetSetHandler(SntpHandler);
> + net_set_udp_handler(SntpHandler);
> memset(NetServerEther, 0, 6);
>
> SntpSend();
> diff --git a/net/tftp.c b/net/tftp.c
> index bf32eab..b2e08b4 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -778,7 +778,7 @@ void TftpStart(enum proto_t protocol)
> TftpTimeoutCountMax = TftpRRQTimeoutCountMax;
>
> NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
> - NetSetHandler(TftpHandler);
> + net_set_udp_handler(TftpHandler);
> #ifdef CONFIG_CMD_TFTPPUT
> net_set_icmp_handler(icmp_handler);
> #endif
> @@ -840,7 +840,7 @@ TftpStartServer(void)
> #endif
>
> TftpState = STATE_RECV_WRQ;
> - NetSetHandler(TftpHandler);
> + net_set_udp_handler(TftpHandler);
> }
> #endif /* CONFIG_CMD_TFTPSRV */
>
> --
> 1.6.0.2
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
