This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: RedBoot Network support fix
At Mon, 07 Feb 2005 05:46:17 -0700,
Gary Thomas wrote:
>
> On Mon, 2005-02-07 at 15:48 +0900, Yoshinori Sato wrote:
> > At Thu, 03 Feb 2005 03:49:27 -0700,
> > Gary Thomas wrote:
> > >
> > > On Thu, 2005-02-03 at 13:59 +0900, Yoshinori Sato wrote:
> > > > At Tue, 01 Feb 2005 10:43:35 -0700,
> > > > Gary Thomas wrote:
> > > > >
> > > > > On Thu, 2005-01-27 at 13:41 +0900, Yoshinori Sato wrote:
> > > > > > I fix the handling of IP address
> > > > >
> > > > > I have some questions about these changes.
> > > > >
> > > > > >
> > > > > > * src/main.c (cyg_start): Fix compiler warning with
> > > > > > Index: packages/redboot/current/src/net/bootp.c
> > > > > > ===================================================================
> > > > > > RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/bootp.c,v
> > > > > > retrieving revision 1.1.1.6
> > > > > > retrieving revision 1.2
> > > > > > diff -u -r1.1.1.6 -r1.2
> > > > > > --- packages/redboot/current/src/net/bootp.c 4 Mar 2004 05:23:06 -0000 1.1.1.6
> > > > > > +++ packages/redboot/current/src/net/bootp.c 27 Jan 2005 04:23:46 -0000 1.2
> > > > > > @@ -135,6 +135,7 @@
> > > > > > // The request message has been sent, only accept an ack reply
> > > > > > if (*p == DHCP_MESS_TYPE_ACK) {
> > > > > > dhcpState = DHCP_ACK;
> > > > > > + bp_info->bp_siaddr = b->bp_siaddr;
> > > > > > return;
> > > > > > } else {
> > > > > > expected = DHCP_MESS_TYPE_ACK;
> > > > >
> > > > > What is this supposed to do? The default server is already being
> > > > > obtained from the DHCP response (at least it works properly on all
> > > > > the targets I've tested).
> > > >
> > > > What I put by testing stayed.
> > > > This is useless.
> > > >
> > > > > > Index: packages/redboot/current/src/net/http_client.c
> > > > > > ===================================================================
> > > > > > RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/http_client.c,v
> > > > > > retrieving revision 1.1.1.7
> > > > > > retrieving revision 1.7
> > > > > > diff -u -r1.1.1.7 -r1.7
> > > > > > --- packages/redboot/current/src/net/http_client.c 12 Jan 2005 02:42:44 -0000 1.1.1.7
> > > > > > +++ packages/redboot/current/src/net/http_client.c 27 Jan 2005 04:20:51 -0000 1.7
> > > > > > @@ -86,7 +86,12 @@
> > > > > > struct _stream *s = &http_stream;
> > > > > >
> > > > > > if (!info->server->sin_port)
> > > > > > - info->server->sin_port = 80; // HTTP port
> > > > > > + info->server->sin_port = htons(80); // HTTP port
> > > > > > + if (info->server->sin_addr.s_addr == 0) {
> > > > > > + // server address 0.0.0.0
> > > > > > + *err = HTTP_OPEN;
> > > > > > + return -1;
> > > > > > + }
> > > > > > if ((res = __tcp_open(&s->sock, info->server, get_port++, 5000, err)) < 0) {
> > > > > > *err = HTTP_OPEN;
> > > > > > return -1;
> > > > >
> > > > > This one seems OK.
> > > > >
> > > > > > Index: packages/redboot/current/src/net/net_io.c
> > > > > > ===================================================================
> > > > > > RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/net_io.c,v
> > > > > > retrieving revision 1.1.1.8
> > > > > > retrieving revision 1.9
> > > > > > diff -u -r1.1.1.8 -r1.9
> > > > > > --- packages/redboot/current/src/net/net_io.c 23 Nov 2004 09:00:42 -0000 1.1.1.8
> > > > > > +++ packages/redboot/current/src/net/net_io.c 27 Jan 2005 04:22:29 -0000 1.9
> > > > > > @@ -678,6 +678,7 @@
> > > > > > #endif
> > > > > > #ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> > > > > > char ip_addr[16];
> > > > > > + struct in_addr default_addr;
> > > > > > #endif
> > > > > >
> > > > > > // Set defaults as appropriate
> > > > > > @@ -782,7 +783,9 @@
> > > > > > #ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> > > > > > diag_sprintf(ip_addr, "%d.%d.%d.%d",
> > > > > > CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
> > > > > > - inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
> > > > > > + inet_aton(ip_addr, &default_addr);
> > > > > > + if (default_addr.s_addr)
> > > > > > + my_bootp_info.bp_siaddr = default_addr;
> > > > > > #endif
> > > > > > #ifdef CYGSEM_REDBOOT_FLASH_CONFIG
> > > > > > flash_get_IP("bootp_server_ip", (ip_addr_t *)&my_bootp_info.bp_siaddr);
> > > > >
> > > > > This seems pointless. If the server IP address has been configured in,
> > > > > it should be set to a legal, non-zero, value. All you've done is
> > > > > duplicate the existing behaviour that if the default server is not set
> > > > > (i.e. is 0.0.0.0) by fconfig, then get it from DHCP.
> > > > >
> > > > > Please explain.
> > > >
> > > > Because CYGDAT_REDBOOT_BOOTP_SERVER_IP_ADDR is defined by "0,0,0,0" when do not use fconfig,
> > > > default server which got with dhcp is destroyed.
> > >
> > > My point is that CYGDAT_REDBOOT_BOOTP_SERVER_IP_ADDR should only be
> > > used to define a valid server IP address.
> >
> > I ignored it in case of "0, 0, 0, 0".
> >
> > Index: packages/redboot/current/src/net/net_io.c
> > ===================================================================
> > RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/net_io.c,v
> > retrieving revision 1.1.1.8
> > diff -u -r1.1.1.8 net_io.c
> > --- packages/redboot/current/src/net/net_io.c 23 Nov 2004 09:00:42 -0000 1.1.1.8
> > +++ packages/redboot/current/src/net/net_io.c 4 Feb 2005 15:31:00 -0000
> > @@ -174,6 +174,9 @@
> > #define TELNET_DO 0xFD // Will you XXX
> > #define TELNET_TM 0x06 // Time marker (special DO/WONT after IP)
> >
> > +#define IP_ADDR(ADDR) _IP_ADDR(ADDR)
> > +#define _IP_ADDR(a, b, c, d) ((a << 24) | (b << 16) | (c << 8) | d)
> > +
> > static cyg_bool
> > _net_io_getc_nonblock(void* __ch_data, cyg_uint8* ch)
> > {
> > @@ -676,7 +679,8 @@
> > char *default_devname;
> > int default_index;
> > #endif
> > -#ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> > +#if defined(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) && \
> > + IP_ADDR(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) != 0
> > char ip_addr[16];
> > #endif
> >
> > @@ -779,7 +783,8 @@
> > }
> > if (have_net) {
> > show_eth_info();
> > -#ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> > +#if defined(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) && \
> > + IP_ADDR(CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR) != 0
> > diag_sprintf(ip_addr, "%d.%d.%d.%d",
> > CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
> > inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
> >
>
> This still seems the wrong way to me. Either the configuration
> defines a valid server IP via CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
> or that should be undefined and let the DHCP/BOOTP or FCONFIG define it.
>
I tried to set for action of description mentioned in redboot.cdl.
Index: packages/redboot/current/src/net/net_io.c
===================================================================
RCS file: /cvsroot/ecos-h8/ecos/packages/redboot/current/src/net/net_io.c,v
retrieving revision 1.1.1.8
diff -u -r1.1.1.8 net_io.c
--- packages/redboot/current/src/net/net_io.c 23 Nov 2004 09:00:42 -0000 1.1.1.8
+++ packages/redboot/current/src/net/net_io.c 10 Feb 2005 14:20:39 -0000
@@ -672,6 +672,7 @@
cyg_netdevtab_entry_t *t;
unsigned index;
struct eth_drv_sc *primary_net = (struct eth_drv_sc *)0;
+ bool bootp_success = false;
#if defined(CYGHWR_NET_DRIVERS) && (CYGHWR_NET_DRIVERS > 1)
char *default_devname;
int default_index;
@@ -758,6 +759,7 @@
if (use_bootp) {
if (__bootp_find_local_ip(&my_bootp_info) == 0) {
have_net = true;
+ bootp_success = true;
} else {
// Is it an unset address, or has it been set to a static addr
if (__local_ip_addr[0] == 0 && __local_ip_addr[1] == 0 &&
@@ -780,9 +782,12 @@
if (have_net) {
show_eth_info();
#ifdef CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR
- diag_sprintf(ip_addr, "%d.%d.%d.%d",
- CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
- inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
+ if (!bootp_success) {
+ // Because failed in BOOTP, use a specification address.
+ diag_sprintf(ip_addr, "%d.%d.%d.%d",
+ CYGDAT_REDBOOT_DEFAULT_BOOTP_SERVER_IP_ADDR);
+ inet_aton(ip_addr, &my_bootp_info.bp_siaddr);
+ }
#endif
#ifdef CYGSEM_REDBOOT_FLASH_CONFIG
flash_get_IP("bootp_server_ip", (ip_addr_t *)&my_bootp_info.bp_siaddr);
--
Yoshinori Sato
<ysato@users.sourceforge.jp>