[U-Boot] [RFC] Act as a TFTP server

[U-Boot] [RFC] Act as a TFTP server

[Luca Ceresoli]

Hi,

I am going to implement in U-Boot the ability to receive a file via TFTP acting
as a server, not a client. I've sketched up a way to implement it, and I would
appreciate any comments about it.

This is useful to solve the firewall issues that a Management Station can face
when sending files to a U-Boot device. These issues are only partially solved
using CONFIG_TFTP_PORT for '"punching through" the (Windows XP) firewall' (see
the README).

My implementation would be very basic (keep it simple):
- receive only (accept WRQ from remote client, not RRQ);
- the Filename in the WRQ is ignored: the destination is always a user-provided
   memory location;
- binary transfers only: the Mode in the WRQ is ignored; this is allowed by
   RFC1350 (section 5);
- no TFTP Option Extensions (RFC2347);
- no TFTP multicast.

After a preliminary analysis, I think the current TFTP clien implementation can
be mostly reused without code duplication, hence the amount of new code would
be pretty limited.

Even better, it would not increase binary size when the feature is disabled,
as I don't see any need to extend the existing code in a way that cannot be
easily put under appropriate #ifdefs.

 From the user point of view, I would implement a new command, activated only
when CONFIG_CMD_TFTPSRV is defined:

   Usage:    tftpsrv [<loadaddr>]

This would be used almost like tftpboot, except no file name is specified on
the commandline.
<loadaddr>  would default to the environment variable.

Here's a tentative implementation roadmap. It is presented in a roughly
top-down order, from user interface down to low level.
Many steps, except for those in net/tftp.c, are pretty trivial.

- add CONFIG_CMD_TFTPSRV config option;
- include/net.h: add TFTPSRV to proto_t, *as if it were a different protocol*;
   this may look dirty but I believe it makes implementation cleaner;
   - alternative: reuse TFTP protocol, but save in some flag (where?) whether
     the user wants a client or server transfer;
- common/cmd_net.c: add a tftpsrv command and do_tftpsrv() function;
   just like do_tftpb() simply calls netboot_common(TFTP, ...),
   do_tftpsrv() would simply call netboot_common(TFTPSRV, ...);
- common/cmd_net.c: netboot_common() should not need change, except maybe
   for the argv parsing: tftpsrv does not take a "hostaddr:filename" argument;
   to make it simpler, it could accept (but ignore) it if passed;
- net/net.c: change net_check_prereq() as needed: it should not check for
   NetServerIP when protocol==TFTPSRV;
- net/net.c: extend NetLoop() to handle the new protocol: call
   TftpStartServer() (not TftpStart()) when protocol==TFTPSRV;
- net/tftp.c: add TftpStartServer() with a role similar to TftpStart();
   - alternative: extend TftpStart() to handle both client and server mode;
     probably not worth doing as the actions to do are mostly different;
- net/tftp.c: extend the TFTP state machine (TftpHandler() and TftpStart())
   a new STATE_WAITING state (it should be enough);
   - the state machine would be different in the session setup: in
     STATE_WAITING handle WRQ packets and select a new UDP port;
   - after the setup phase, the state machine would converge on the
     currently-implemented STATE_DATA for normal DATA/ACK management.

Do you think my approach is correct?
Would this feature be considered for mainline inclusion?

Thanks in advance,
Luca

[U-Boot] [RFC] Act as a TFTP server

[Wolfgang Denk]

Dear Luca,

In message <4DA5682A.8040203@comelit.it> you wrote:
> 
> I am going to implement in U-Boot the ability to receive a file via TFTP acting
> as a server, not a client. I've sketched up a way to implement it, and I would
> appreciate any comments about it.

Thanks for the efforts, and especially for discussing the design
early.  Highly appreciated.

>  From the user point of view, I would implement a new command, activated only
> when CONFIG_CMD_TFTPSRV is defined:
> 
>    Usage:    tftpsrv [<loadaddr>]
> 
> This would be used almost like tftpboot, except no file name is specified on
> the commandline.
> <loadaddr>  would default to the environment variable.

I see some user interface issues here:

- When and how do you terminate this command?  After a successful
  download, ok.  But how long do we wait for that?  Forever? Or do we
  time out?

- It would be nice if the user could terminate the command using ^C.

> Do you think my approach is correct?

Looks ok to me.

> Would this feature be considered for mainline inclusion?

Yes, of course.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Always try to do things in chronological order; it's  less  confusing
that way.

[U-Boot] [RFC] Act as a TFTP server

[Luca Ceresoli]

Wolfgang,
thanks for the feedback.

Wolfgang Denk wrote:

> ...
>>    From the user point of view, I would implement a new command, activated only
>> when CONFIG_CMD_TFTPSRV is defined:
>>
>>     Usage:    tftpsrv [<loadaddr>]
>>
>> This would be used almost like tftpboot, except no file name is specified on
>> the commandline.
>> <loadaddr>   would default to the environment variable.
> I see some user interface issues here:
> - When and how do you terminate this command?  After a successful
>    download, ok.  But how long do we wait for that?  Forever? Or do we
>    time out?
The TFTP client tries sending a RRQ packet TIMEOUT_COUNT times (default 10),
spaced TIMEOUT msecs (default 5000) from each other. 50 seconds total by default.

I would keep the same scheme for the server, except it would obviously not send
anything after each timeout. This would allow to print something every 5 seconds,
for user feedback.

> - It would be nice if the user could terminate the command using ^C.
Sure. The main loop in NetLoop() would be unchanged, so I expect ctrlc() to work
for free in the same way it does for the TFTP client.

Luca

[U-Boot] [RFC] Act as a TFTP server

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <4DA59B5E.5000105@comelit.it> you wrote:
>
> >>     Usage:    tftpsrv [<loadaddr>]
> >>
> >> This would be used almost like tftpboot, except no file name is specified on
> >> the commandline.
> >> <loadaddr>   would default to the environment variable.
> > I see some user interface issues here:
> > - When and how do you terminate this command?  After a successful
> >    download, ok.  But how long do we wait for that?  Forever? Or do we
> >    time out?
> The TFTP client tries sending a RRQ packet TIMEOUT_COUNT times (default 10),
> spaced TIMEOUT msecs (default 5000) from each other. 50 seconds total by default.

Well - this requires that there actually _is_ a TFTP client which
attempts to connect.  But there is no guarantee that any such client
is running at all, or that it can connect through firewalls, routers,
etc.

So assuming we never see any incoming packets - how long will we wait?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If all you have is a hammer, everything looks like a nail.

[U-Boot] [RFC] Act as a TFTP server

[Luca Ceresoli]

Il 30/04/2011 21:53, Wolfgang Denk ha scritto:
> Dear Luca Ceresoli,
>
> In message<4DA59B5E.5000105@comelit.it>  you wrote:
>>>>      Usage:    tftpsrv [<loadaddr>]
>>>>
>>>> This would be used almost like tftpboot, except no file name is specified on
>>>> the commandline.
>>>> <loadaddr>    would default to the environment variable.
>>> I see some user interface issues here:
>>> - When and how do you terminate this command?  After a successful
>>>     download, ok.  But how long do we wait for that?  Forever? Or do we
>>>     time out?
>> The TFTP client tries sending a RRQ packet TIMEOUT_COUNT times (default 10),
>> spaced TIMEOUT msecs (default 5000) from each other. 50 seconds total by default.
> Well - this requires that there actually _is_ a TFTP client which
> attempts to connect.  But there is no guarantee that any such client
> is running at all, or that it can connect through firewalls, routers,
> etc.
>
> So assuming we never see any incoming packets - how long will we wait?

Once it has started, the server is stopped like the client is:
- on a complete file reception
- pressing Ctrl-C
- after waiting 5 seconds for 10 times.

Luca

[U-Boot] [RFC] Act as a TFTP server

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <4DBFF3AF.4040505@comelit.it> you wrote:
>
> > So assuming we never see any incoming packets - how long will we wait?
> 
> Once it has started, the server is stopped like the client is:
> - on a complete file reception
> - pressing Ctrl-C
> - after waiting 5 seconds for 10 times.

OK, fine.  Can you please document thisn, then.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Confound these ancestors.... They've stolen our best ideas!"
- Ben Jonson

[U-Boot] [RFC] Act as a TFTP server

[Luca Ceresoli]

Wolfgang Denk wrote:

> Dear Luca Ceresoli,
>
> In message<4DBFF3AF.4040505@comelit.it>  you wrote:
>>> So assuming we never see any incoming packets - how long will we wait?
>> Once it has started, the server is stopped like the client is:
>> - on a complete file reception
>> - pressing Ctrl-C
>> - after waiting 5 seconds for 10 times.
> OK, fine.  Can you please document thisn, then.  Thanks.
Incorporating also Mike's suggestion for the one-line help, I propose this:

U_BOOT_CMD(
	tftpsrv,        2,      1,      do_tftpsrv,
         "act as a TFTP server and boot the first received file",
         "[loadAddress]\n"
         "Listen for an incoming TFTP transfer, receive a file and boot it.\n"
         "The transfer is aborted if a transfer has not been started after\n"
         "about 50 seconds or if Ctrl-C is pressed."
);

Comments?

Luca

[U-Boot] [RFC] Act as a TFTP server

[Luca Ceresoli]

Luca Ceresoli wrote:

> - net/tftp.c: extend the TFTP state machine (TftpHandler() and TftpStart())
>     a new STATE_WAITING state (it should be enough);
>     - the state machine would be different in the session setup: in
>       STATE_WAITING handle WRQ packets and select a new UDP port;
>     - after the setup phase, the state machine would converge on the
>       currently-implemented STATE_DATA for normal DATA/ACK management.
Right now the TFTP server is working here, but before submitting patches I
have a question about the TftpHandler() callback function.

TftpHandler() is of type:

include/net.h:80: typedef void rxhand_f(uchar *, unsigned, unsigned, unsigned);

The 4 parameters are: packet pointer, UDP source port, UDP destination
port and packet length.

Upon reception of a Write Request packet, the TFTP server needs to know the
remote (client) IP address, or it won't be able to send any packet back.

This is different from currently implemented network file transfer modes,
which are always initiated by U-Boot in client mode, and thus the remote
(server) IP address is known a priori.

I plan to add a new parameter to the rxhand_t, and change accordingly all
handlers to have the new (unused) parameter:

- typedef void rxhand_f(uchar *, unsigned, unsigned, unsigned);
+ typedef void rxhand_f(uchar *pkt, unsigned dest_port,
                         IPaddr_t src_ip, unsigned src_port,
                         unsigned len);

Is this ok, or do you foresee any issues with the function footprint or
anything else?

Thanks,
Luca

[U-Boot] [PATCH 0/6] TFTP server

[Luca Ceresoli]

This patch series adds to U-Boot the ability to receive a file via TFTP acting
as a server, not a client.

The implementation is kept simple:
- receive only (accept WRQ from remote client, not RRQ);
- the Filename in the WRQ is ignored: the destination is always a user-provided
  memory location;
- binary transfers only: the Mode in the WRQ is ignored; this is allowed by
  RFC1350 (section 5);
- no TFTP Option Extensions (RFC2347);
- no TFTP multicast.

The implementation is discussed here:
http://lists.denx.de/pipermail/u-boot/2011-April/090405.html

Once it has started, the server is stopped like the client is: on a complete
file reception, Ctrl-C and after waiting 5 seconds for 10 times.

The first four patches are preliminary cleanups and extensions to the current
code. Most important, the second patch adds to incoming packet handlers an
argument containing the source IP address, and has impact in many places in
the networking code.

The fifth patch implements the core TFTP server.

The last patch adds a user command to launch the server.

A note about checkpatch.pl.
Some of these patches do have checkpatch errors and/or warnings. These are all
issues that were already present in the pre-existing code.
An example from patch 2:

 static void
-PingHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+PingHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+unsigned len)
 {

Raises:
* ERROR: "foo * bar" should be "foo *bar"
* WARNING: space prohibited between function name and open parenthesis '('

As I didn't touch the pointer parameters nor the stuff before the '(' (but I
changed the line somewhere else), should I also fix the checkpatch issues?
And in case, should it be a separate patch to make it cleaner?

Also please take a look at the note in patch number 5.

Luca

Luca Ceresoli (6):
  README: remove spurious line
  NET: pass source IP address to packet handlers
  TFTP: rename "server" to "remote"
  TFTP: rename STATE_RRQ to STATE_SEND_RRQ
  TFTP: net/tftp.c: add server mode receive
  TFTP: add tftpsrv command

 README                   |    2 +-
 common/cmd_net.c         |   14 ++++++
 drivers/net/netconsole.c |    5 +-
 include/net.h            |   18 +++++---
 net/bootp.c              |    9 +++-
 net/dns.c                |    2 +-
 net/net.c                |   35 +++++++++------
 net/nfs.c                |    2 +-
 net/rarp.c               |    3 +-
 net/sntp.c               |    3 +-
 net/tftp.c               |  109 ++++++++++++++++++++++++++++++++++++---------
 net/tftp.h               |    6 +++
 12 files changed, 157 insertions(+), 51 deletions(-)

[U-Boot] [PATCH 0/6] TFTP server

[Albert ARIBAUD]

Hi Luca,

Le 14/04/2011 17:52, Luca Ceresoli a ?crit :

> A note about checkpatch.pl.

:)

> Some of these patches do have checkpatch errors and/or warnings. These are all
> issues that were already present in the pre-existing code.
> An example from patch 2:
>
>   static void
> -PingHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
> +PingHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
> +unsigned len)
>   {
>
> Raises:
> * ERROR: "foo * bar" should be "foo *bar"
> * WARNING: space prohibited between function name and open parenthesis '('
>
> As I didn't touch the pointer parameters nor the stuff before the '(' (but I
> changed the line somewhere else), should I also fix the checkpatch issues?
> And in case, should it be a separate patch to make it cleaner?

My opinion is that you should make sure that at least the code you touch 
is checkpatch-clean, so yes, you should fix that; but there is no need 
to submit 'checkpatch-compliance' patches. Just fix the line here so 
that checkpatch does not complain.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v2 0/6] TFTP server

[Luca Ceresoli]

Hi,
here's the tftpsrv patch series, with checkpatch warnings fixed.
Only patches 2 and 3 are actually changed. Patch 1 is unchanged and patches 4-6
have been simply rebased.

There are still a few checkpatch issues (!), though, that I think can be ignored
and I'm not going to fix unless there's a specific request.

Here they are:

> WARNING: consider using kstrto* in preference to simple_strtol
> #125: FILE: net/tftp.c:619:
> +		TftpRemotePort = simple_strtol(ep, NULL, 10);
Which is Linux-only

> WARNING: do not add new typedefs
> #61: FILE: include/net.h:367:
> +typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
Is a typedef *enum* considered bad in U-Boot?

Luca

Luca Ceresoli (6):
  README: remove spurious line
  NET: pass source IP address to packet handlers
  TFTP: rename "server" to "remote"
  TFTP: rename STATE_RRQ to STATE_SEND_RRQ
  TFTP: net/tftp.c: add server mode receive
  TFTP: add tftpsrv command

 README                   |    2 +-
 common/cmd_net.c         |   14 +++++
 drivers/net/netconsole.c |    5 +-
 include/net.h            |   18 +++++--
 net/bootp.c              |    9 ++-
 net/dns.c                |    2 +-
 net/net.c                |   40 +++++++++------
 net/nfs.c                |    2 +-
 net/rarp.c               |    3 +-
 net/sntp.c               |    3 +-
 net/tftp.c               |  123 +++++++++++++++++++++++++++++++++++-----------
 net/tftp.h               |    6 ++
 12 files changed, 167 insertions(+), 60 deletions(-)

[U-Boot] [PATCH v2 0/6] TFTP server

[Detlev Zundel]

Hi Luca,

> Hi,
> here's the tftpsrv patch series, with checkpatch warnings fixed.
> Only patches 2 and 3 are actually changed. Patch 1 is unchanged and patches 4-6
> have been simply rebased.
>
> There are still a few checkpatch issues (!), though, that I think can be ignored
> and I'm not going to fix unless there's a specific request.
>
> Here they are:
>
>> WARNING: consider using kstrto* in preference to simple_strtol
>> #125: FILE: net/tftp.c:619:
>> +		TftpRemotePort = simple_strtol(ep, NULL, 10);
> Which is Linux-only
>
>> WARNING: do not add new typedefs
>> #61: FILE: include/net.h:367:
>> +typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> Is a typedef *enum* considered bad in U-Boot?

For new code yes.  We (well at least I) much rather prefer to see what
type a variable is without looking up the typedef for it.

Cheers
  Detlev

-- 
F?r jemanden, der in eine Religion geboren wurde, in der das Ringen um eine
einzige Seele ein Stafettenlauf ?ber viele Jahrhunderte sein kann [..], hat
das Tempo des Christentums etwas Schwindelerregendes.   Wenn der Hinduismus
friedlich dahinflie?t wie der Ganges,  dann ist das  Christentum Toronto in
der Rushhour.                        -- Yann Martel
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 0/5] TFTP server

[Luca Ceresoli]

This patch series adds to U-Boot the ability to receive a file via TFTP acting
as a server, instead of a client.

I rebased v3 on top of the net/tftp.c cleanup work that I submitted recently
(http://lists.denx.de/pipermail/u-boot/2011-May/092599.html) and removed all
the cleanups from this patchset.

Patches 1 and 2 of v2 have already been committed to master, so the remaining
patches shifted back by two.

Finally, I incorporated all changes requested on the ml.

Description:

The implementation is kept simple:
- receive only (accept WRQ from remote client, not RRQ);
- the Filename in the WRQ is ignored: the destination is always a user-provided
  memory location;
- binary transfers only: the Mode in the WRQ is ignored; this is allowed by
  RFC1350 (section 5);
- no TFTP Option Extensions (RFC2347);
- no TFTP multicast.

The implementation is discussed here:
http://lists.denx.de/pipermail/u-boot/2011-April/090405.html

Once it has started, the server is stopped like the client is: on a complete
file reception, Ctrl-C and after waiting 5 seconds for 10 times.

The first two patches are preliminary cleanups and extensions to the current
code.

The third patch implements the core TFTP server.

The fourth patch adds a user command to launch the server.

I also added a trailing patch (#5 in this series), a trivial typo fix
requested by Detlev.

Luca

Luca Ceresoli (5):
  TFTP: replace "server" with "remote" in local variable names
  TFTP: rename STATE_RRQ to STATE_SEND_RRQ
  TFTP: net/tftp.c: add server mode receive
  TFTP: add tftpsrv command
  net/tftp.c: fix typo

 README           |    1 +
 common/cmd_net.c |   17 +++++++++
 include/net.h    |    3 +-
 net/net.c        |    7 +++-
 net/tftp.c       |   98 +++++++++++++++++++++++++++++++++++++++++------------
 net/tftp.h       |    6 +++
 6 files changed, 108 insertions(+), 24 deletions(-)

[U-Boot] [PATCH v3 1/5] TFTP: replace "server" with "remote" in local variable names

[Luca Ceresoli]

With the upcoming TFTP server implementation, the remote node can be
either a client or a server, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>

---
Changes in v2:
 - fixed checkpatch issues.

Changes in v3:
 - rebased on top of the net/tftp.c cleanup;
 - renamed also local variable ServerNet to RemoteNet in TftpStart();
 - clarified commit message.

 net/tftp.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 0f74e6b..b9d0f3b 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -58,9 +58,9 @@ enum {
 	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
 };
 
-static IPaddr_t TftpServerIP;
+static IPaddr_t TftpRemoteIP;
 /* The UDP port at their end */
-static int	TftpServerPort;
+static int	TftpRemotePort;
 /* The UDP port at our end */
 static int	TftpOurPort;
 static int	TftpTimeoutCount;
@@ -289,7 +289,7 @@ TftpSend(void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort,
+	NetSendUDPPacket(NetServerEther, TftpRemoteIP, TftpRemotePort,
 			 TftpOurPort, len);
 }
 
@@ -309,7 +309,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 			return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpServerPort)
+	if (TftpState != STATE_RRQ && src != TftpRemotePort)
 		return;
 
 	if (len < 2)
@@ -333,7 +333,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			pkt,
 			pkt + strlen((char *)pkt) + 1);
 		TftpState = STATE_OACK;
-		TftpServerPort = src;
+		TftpRemotePort = src;
 		/*
 		 * Check for 'blksize' option.
 		 * Careful: "i" is signed, "len" is unsigned, thus
@@ -405,7 +405,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
-			TftpServerPort = src;
+			TftpRemotePort = src;
 			TftpLastBlock = 0;
 			TftpBlockWrap = 0;
 			TftpBlockWrapOffset = 0;
@@ -440,7 +440,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 
 		/*
 		 *	Acknoledge the block just received, which will prompt
-		 *	the server for the next one.
+		 *	the remote for the next one.
 		 */
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
@@ -569,7 +569,7 @@ TftpStart(void)
 	debug("TFTP blocksize = %i, timeout = %ld ms\n",
 		TftpBlkSizeOption, TftpTimeoutMSecs);
 
-	TftpServerIP = NetServerIP;
+	TftpRemoteIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
@@ -589,7 +589,7 @@ TftpStart(void)
 			strncpy(tftp_filename, BootFile, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		} else {
-			TftpServerIP = string_to_ip(BootFile);
+			TftpRemoteIP = string_to_ip(BootFile);
 			strncpy(tftp_filename, p + 1, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		}
@@ -599,14 +599,14 @@ TftpStart(void)
 	printf("Using %s device\n", eth_get_name());
 #endif
 	printf("TFTP from server %pI4"
-		"; our IP address is %pI4", &TftpServerIP, &NetOurIP);
+		"; our IP address is %pI4", &TftpRemoteIP, &NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 		IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
-		IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
+		IPaddr_t RemoteNet	= TftpRemoteIP & NetOurSubnetMask;
 
-		if (OurNet != ServerNet)
+		if (OurNet != RemoteNet)
 			printf("; sending through gateway %pI4",
 			       &NetOurGatewayIP);
 	}
@@ -630,7 +630,7 @@ TftpStart(void)
 	NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
 	NetSetHandler(TftpHandler);
 
-	TftpServerPort = WELL_KNOWN_PORT;
+	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
 	TftpState = STATE_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
@@ -639,7 +639,7 @@ TftpStart(void)
 #ifdef CONFIG_TFTP_PORT
 	ep = getenv("tftpdstp");
 	if (ep != NULL)
-		TftpServerPort = simple_strtol(ep, NULL, 10);
+		TftpRemotePort = simple_strtol(ep, NULL, 10);
 	ep = getenv("tftpsrcp");
 	if (ep != NULL)
 		TftpOurPort = simple_strtol(ep, NULL, 10);
-- 
1.7.1

[U-Boot] [PATCH v3 1/5] TFTP: replace "server" with "remote" in local variable names

[Detlev Zundel]

Hi Luca,

> With the upcoming TFTP server implementation, the remote node can be
> either a client or a server, so avoid ambiguities.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

Cheers
  Detlev

-- 
Algebraists do it in a ring, in fields, in groups.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 1/5] TFTP: replace "server" with "remote" in local variable names

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1305626621-15008-2-git-send-email-luca.ceresoli@comelit.it> you wrote:
> With the upcoming TFTP server implementation, the remote node can be
> either a client or a server, so avoid ambiguities.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> 
> ---
> Changes in v2:
>  - fixed checkpatch issues.
> 
> Changes in v3:
>  - rebased on top of the net/tftp.c cleanup;
>  - renamed also local variable ServerNet to RemoteNet in TftpStart();
>  - clarified commit message.
> 
>  net/tftp.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Pardon me for breathing, which I never do anyway so I don't know why
I bother to say it, oh God, I'm so depressed. Here's another of those
self-satisfied doors. Life! Don't talk to me about life."
                                        - Marvin the Paranoid Android

[U-Boot] [PATCH v3 2/5] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Luca Ceresoli]

With the upcoming TFTP server implementation, requests can be either
outgoing or incoming, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>

---
Changes in v2: none.

Changes in v3:
 - rebased on top of the net/tftp.c cleanup.

 net/tftp.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index b9d0f3b..6386740 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -80,7 +80,7 @@ static int	TftpTsize;
 static short	TftpNumchars;
 #endif
 
-#define STATE_RRQ	1
+#define STATE_SEND_RRQ	1
 #define STATE_DATA	2
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
@@ -215,7 +215,7 @@ TftpSend(void)
 
 	switch (TftpState) {
 
-	case STATE_RRQ:
+	case STATE_SEND_RRQ:
 		xp = pkt;
 		s = (ushort *)pkt;
 		*s++ = htons(TFTP_RRQ);
@@ -309,7 +309,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 			return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
 		return;
 
 	if (len < 2)
@@ -399,10 +399,10 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 				puts("\n\t ");
 		}
 
-		if (TftpState == STATE_RRQ)
+		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -632,7 +632,7 @@ TftpStart(void)
 
 	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
-	TftpState = STATE_RRQ;
+	TftpState = STATE_SEND_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
 	TftpOurPort = 1024 + (get_timer(0) % 3072);
 
-- 
1.7.1

[U-Boot] [PATCH v3 2/5] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Detlev Zundel]

Luca Ceresoli <luca.ceresoli@comelit.it> writes:

> With the upcoming TFTP server implementation, requests can be either
> outgoing or incoming, so avoid ambiguities.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

Cheers
  Detlev

-- 
More than any other time in history, mankind faces a crossroads.  One
path leads  to despair  and utter  hopelessness.   The other to total
extinction.  Let us pray, we have the wisdom to choose correctly.
                                        -- Woody Allen
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 2/5] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1305626621-15008-3-git-send-email-luca.ceresoli@comelit.it> you wrote:
> With the upcoming TFTP server implementation, requests can be either
> outgoing or incoming, so avoid ambiguities.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> 
> ---
> Changes in v2: none.
> 
> Changes in v3:
>  - rebased on top of the net/tftp.c cleanup.
> 
>  net/tftp.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Some programming languages manage to  absorb  change,  but  withstand
progress.          -- Epigrams in Programming, ACM SIGPLAN Sept. 1982

[U-Boot] [PATCH v3 3/5] TFTP: net/tftp.c: add server mode receive

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>

---
Changes in v2: none.

Changes in v3:
 - rebased on top of the net/tftp.c cleanup;
 - removed all #ifdefs that used to remove negligible amounts of compiled code,
   at the cost of a much less readable source file; after measurements, it
   turned out this change does not increase code size.

 net/tftp.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 net/tftp.h |    6 +++++
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 6386740..6d44298 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -2,6 +2,8 @@
  * Copyright 1994, 1995, 2000 Neil Russell.
  * (See License)
  * Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
+ * Copyright 2011 Comelit Group SpA,
+ *                Luca Ceresoli <luca.ceresoli@comelit.it>
  */
 
 #include <common.h>
@@ -85,6 +87,7 @@ static short	TftpNumchars;
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
 #define STATE_OACK	5
+#define STATE_RECV_WRQ	6
 
 /* default TFTP block size */
 #define TFTP_BLOCK_SIZE		512
@@ -257,6 +260,8 @@ TftpSend(void)
 							    (Mapsize*8), 0);
 		/*..falling..*/
 #endif
+
+	case STATE_RECV_WRQ:
 	case STATE_DATA:
 		xp = pkt;
 		s = (ushort *)pkt;
@@ -309,7 +314,8 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 			return;
 	}
-	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort &&
+	    TftpState != STATE_RECV_WRQ)
 		return;
 
 	if (len < 2)
@@ -322,12 +328,24 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 	switch (ntohs(proto)) {
 
 	case TFTP_RRQ:
-	case TFTP_WRQ:
 	case TFTP_ACK:
 		break;
 	default:
 		break;
 
+#ifdef CONFIG_CMD_TFTPSRV
+	case TFTP_WRQ:
+		debug("Got WRQ\n");
+		TftpRemoteIP = sip;
+		TftpRemotePort = src;
+		TftpOurPort = 1024 + (get_timer(0) % 3072);
+		TftpLastBlock = 0;
+		TftpBlockWrap = 0;
+		TftpBlockWrapOffset = 0;
+		TftpSend(); /* Send ACK(0) */
+		break;
+#endif
+
 	case TFTP_OACK:
 		debug("Got OACK: %s %s\n",
 			pkt,
@@ -402,7 +420,8 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK ||
+		    TftpState == STATE_RECV_WRQ) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -537,7 +556,8 @@ TftpTimeout(void)
 	} else {
 		puts("T ");
 		NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
-		TftpSend();
+		if (TftpState != STATE_RECV_WRQ)
+			TftpSend();
 	}
 }
 
@@ -661,6 +681,40 @@ TftpStart(void)
 	TftpSend();
 }
 
+#ifdef CONFIG_CMD_TFTPSRV
+void
+TftpStartServer(void)
+{
+	tftp_filename[0] = 0;
+
+#if defined(CONFIG_NET_MULTI)
+	printf("Using %s device\n", eth_get_name());
+#endif
+	printf("Listening for TFTP transfer on %pI4\n", &NetOurIP);
+	printf("Load address: 0x%lx\n", load_addr);
+
+	puts("Loading: *\b");
+
+	TftpTimeoutCountMax = TIMEOUT_COUNT;
+	TftpTimeoutCount = 0;
+	TftpTimeoutMSecs = TIMEOUT;
+	NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
+
+	/* Revert TftpBlkSize to dflt */
+	TftpBlkSize = TFTP_BLOCK_SIZE;
+	TftpBlock = 0;
+	TftpOurPort = WELL_KNOWN_PORT;
+
+#ifdef CONFIG_TFTP_TSIZE
+	TftpTsize = 0;
+	TftpNumchars = 0;
+#endif
+
+	TftpState = STATE_RECV_WRQ;
+	NetSetHandler(TftpHandler);
+}
+#endif /* CONFIG_CMD_TFTPSRV */
+
 #ifdef CONFIG_MCAST_TFTP
 /* Credits: atftp project.
  */
diff --git a/net/tftp.h b/net/tftp.h
index e3dfb26..3abdf7b 100644
--- a/net/tftp.h
+++ b/net/tftp.h
@@ -2,6 +2,8 @@
  *	LiMon - BOOTP/TFTP.
  *
  *	Copyright 1994, 1995, 2000 Neil Russell.
+ *	Copyright 2011 Comelit Group SpA
+ *	               Luca Ceresoli <luca.ceresoli@comelit.it>
  *	(See License)
  */
 
@@ -16,6 +18,10 @@
 /* tftp.c */
 extern void	TftpStart (void);	/* Begin TFTP get */
 
+#ifdef CONFIG_CMD_TFTPSRV
+extern void	TftpStartServer(void);	/* Wait for incoming TFTP put */
+#endif
+
 /**********************************************************************/
 
 #endif /* __TFTP_H__ */
-- 
1.7.1

[U-Boot] [PATCH v3 3/5] TFTP: net/tftp.c: add server mode receive

[Detlev Zundel]

Hi Luca,

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

Just for the future:

[...]

> diff --git a/net/tftp.h b/net/tftp.h
> index e3dfb26..3abdf7b 100644
> --- a/net/tftp.h
> +++ b/net/tftp.h
> @@ -2,6 +2,8 @@
>   *	LiMon - BOOTP/TFTP.
>   *
>   *	Copyright 1994, 1995, 2000 Neil Russell.
> + *	Copyright 2011 Comelit Group SpA
> + *	               Luca Ceresoli <luca.ceresoli@comelit.it>
>   *	(See License)
>   */
>  
> @@ -16,6 +18,10 @@
>  /* tftp.c */
>  extern void	TftpStart (void);	/* Begin TFTP get */
>  
> +#ifdef CONFIG_CMD_TFTPSRV
> +extern void	TftpStartServer(void);	/* Wait for incoming TFTP put */
> +#endif
> +
>  /**********************************************************************/
>  
>  #endif /* __TFTP_H__ */

I think we don't need the ifdefs in header files, or am I missing
something?  I _really_ like to avoid ifdefs wherever we can ;)

Cheers
  Detlev

-- 
I have a computer on which I can install any code I choose.  I don't
think that is a security flaw.  On the contrary, if only one company
can install a new version, that is a grave security flaw for me as a
user.      -- Richard Stallman <E1MJdJD-00010L-Gg@fencepost.gnu.org>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 3/5] TFTP: net/tftp.c: add server mode receive

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1305626621-15008-4-git-send-email-luca.ceresoli@comelit.it> you wrote:
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> 
> ---
> Changes in v2: none.
> 
> Changes in v3:
>  - rebased on top of the net/tftp.c cleanup;
>  - removed all #ifdefs that used to remove negligible amounts of compiled code,
>    at the cost of a much less readable source file; after measurements, it
>    turned out this change does not increase code size.
> 
>  net/tftp.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  net/tftp.h |    6 +++++
>  2 files changed, 64 insertions(+), 4 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Lots of folks confuse bad management with destiny.   -- Frank Hubbard

[U-Boot] [PATCH v3 4/5] TFTP: add tftpsrv command

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>

---
Changes in v2: none.

Changes in v3:
 - rebased on top of the net/tftp.c cleanup;
 - made do_tftpsrv() static;
 - improved tftpsrv command help;
 - removed all #ifdefs that used to remove negligible amounts of compiled code,
   at the cost of a much less readable source file; after measurements, it
   turned out this change does not increase code size.

 README           |    1 +
 common/cmd_net.c |   17 +++++++++++++++++
 include/net.h    |    3 ++-
 net/net.c        |    7 ++++++-
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 6f3748d..ed73981 100644
--- a/README
+++ b/README
@@ -709,6 +709,7 @@ The following options need to be configured:
 					  (requires CONFIG_CMD_MEMORY)
 		CONFIG_CMD_SOURCE	  "source" command Support
 		CONFIG_CMD_SPI		* SPI serial bus support
+		CONFIG_CMD_TFTPSRV	* TFTP transfer in server mode
 		CONFIG_CMD_USB		* USB support
 		CONFIG_CMD_VFD		* VFD support (TRAB)
 		CONFIG_CMD_CDP		* Cisco Discover Protocol support
diff --git a/common/cmd_net.c b/common/cmd_net.c
index 8c6f5c8..b2c9355 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -52,6 +52,23 @@ U_BOOT_CMD(
 	"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 
+#ifdef CONFIG_CMD_TFTPSRV
+static int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	return netboot_common(TFTPSRV, cmdtp, argc, argv);
+}
+
+U_BOOT_CMD(
+	tftpsrv,	2,	1,	do_tftpsrv,
+	"act as a TFTP server and boot the first received file",
+	"[loadAddress]\n"
+	"Listen for an incoming TFTP transfer, receive a file and boot it.\n"
+	"The transfer is aborted if a transfer has not been started after\n"
+	"about 50 seconds or if Ctrl-C is pressed."
+);
+#endif
+
+
 #ifdef CONFIG_CMD_RARP
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
diff --git a/include/net.h b/include/net.h
index 01f7159..018a744 100644
--- a/include/net.h
+++ b/include/net.h
@@ -364,7 +364,8 @@ extern int		NetState;		/* Network loop state		*/
 extern int		NetRestartWrap;		/* Tried all network devices	*/
 #endif
 
-typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP } proto_t;
+typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
+	       TFTPSRV } proto_t;
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
diff --git a/net/net.c b/net/net.c
index 2abf879..7a93542 100644
--- a/net/net.c
+++ b/net/net.c
@@ -423,7 +423,11 @@ restart:
 			/* always use ARP to get server ethernet address */
 			TftpStart();
 			break;
-
+#ifdef CONFIG_CMD_TFTPSRV
+		case TFTPSRV:
+			TftpStartServer();
+			break;
+#endif
 #if defined(CONFIG_CMD_DHCP)
 		case DHCP:
 			BootpTry = 0;
@@ -1791,6 +1795,7 @@ common:
 		/* Fall through */
 
 	case NETCONS:
+	case TFTPSRV:
 		if (NetOurIP == 0) {
 			puts("*** ERROR: `ipaddr' not set\n");
 			return 1;
-- 
1.7.1

[U-Boot] [PATCH v3 4/5] TFTP: add tftpsrv command

[Detlev Zundel]

Luca Ceresoli <luca.ceresoli@comelit.it> writes:

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

Cheers
  Detlev

-- 
-- Question authority!
-- Yeah, says who?
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 4/5] TFTP: add tftpsrv command

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1305626621-15008-5-git-send-email-luca.ceresoli@comelit.it> you wrote:
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> 
> ---
> Changes in v2: none.
> 
> Changes in v3:
>  - rebased on top of the net/tftp.c cleanup;
>  - made do_tftpsrv() static;
>  - improved tftpsrv command help;
>  - removed all #ifdefs that used to remove negligible amounts of compiled code,
>    at the cost of a much less readable source file; after measurements, it
>    turned out this change does not increase code size.
> 
>  README           |    1 +
>  common/cmd_net.c |   17 +++++++++++++++++
>  include/net.h    |    3 ++-
>  net/net.c        |    7 ++++++-
>  4 files changed, 26 insertions(+), 2 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Free markets select for winning solutions."        - Eric S. Raymond

[U-Boot] [PATCH v3 5/5] net/tftp.c: fix typo

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>

---
Changes in v3: this patch is new in v3.

 net/tftp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 6d44298..a893e02 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -458,7 +458,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		store_block(TftpBlock - 1, pkt + 2, len);
 
 		/*
-		 *	Acknoledge the block just received, which will prompt
+		 *	Acknowledge the block just received, which will prompt
 		 *	the remote for the next one.
 		 */
 #ifdef CONFIG_MCAST_TFTP
-- 
1.7.1

[U-Boot] [PATCH v3 5/5] net/tftp.c: fix typo

[Detlev Zundel]

Luca Ceresoli <luca.ceresoli@comelit.it> writes:

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Acked-by: Detlev Zundel <dzu@denx.de>

Cheers
  Detlev

-- 
The  mathematician's patterns,  like the  painter's or  the poet's,  must be
beautiful;  the ideas, like the colours or the words, must fit together in a
harmonious way. Beauty is the first test: there is no permanent place in the
world for ugly mathematics.                       -- G. H. Hardy
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v3 5/5] net/tftp.c: fix typo

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1305626621-15008-6-git-send-email-luca.ceresoli@comelit.it> you wrote:
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> 
> ---
> Changes in v3: this patch is new in v3.
> 
>  net/tftp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
[Braddock:] Mr. Churchill, you are drunk.
[Churchill:] And you madam, are ugly.  But I shall be sober tomorrow.

[U-Boot] [PATCH v2 1/6] README: remove spurious line

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2: none.

 README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/README b/README
index 4917e26..b9b0fcb 100644
--- a/README
+++ b/README
@@ -1299,7 +1299,6 @@ The following options need to be configured:
 		driver in use must provide a function: mcast() to join/leave a
 		multicast group.
 
-		CONFIG_BOOTP_RANDOM_DELAY
 - BOOTP Recovery Mode:
 		CONFIG_BOOTP_RANDOM_DELAY
 
-- 
1.7.1

[U-Boot] [PATCH v2 1/6] README: remove spurious line

[Detlev Zundel]

Hi Luca,

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2: none.
>
>  README |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/README b/README
> index 4917e26..b9b0fcb 100644
> --- a/README
> +++ b/README
> @@ -1299,7 +1299,6 @@ The following options need to be configured:
>  		driver in use must provide a function: mcast() to join/leave a
>  		multicast group.
>  
> -		CONFIG_BOOTP_RANDOM_DELAY
>  - BOOTP Recovery Mode:
>  		CONFIG_BOOTP_RANDOM_DELAY

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
debian is a prototype for a future version of emacs.
                         -- Thien-Thi Nguyen in <7eekubiffq.fsf@ada2.unipv.it>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers

[Luca Ceresoli]

This is needed for the upcoming TFTP server implementation.

This also simplifies PingHandler() and fixes rxhand_f documentation.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2:
 - fixed checkpatch issues.

 drivers/net/netconsole.c |    5 +++--
 include/net.h            |   15 ++++++++++-----
 net/bootp.c              |    9 ++++++---
 net/dns.c                |    2 +-
 net/net.c                |   30 +++++++++++++++++-------------
 net/nfs.c                |    2 +-
 net/rarp.c               |    3 ++-
 net/sntp.c               |    3 ++-
 net/tftp.c               |    3 ++-
 9 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index e27bb3e..e40efb8 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -40,13 +40,14 @@ static short nc_port;			/* source/target port */
 static const char *output_packet;	/* used by first send udp */
 static int output_packet_len = 0;
 
-static void nc_wait_arp_handler (uchar * pkt, unsigned dest, unsigned src,
+static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
+				 IPaddr_t sip, unsigned src,
 				 unsigned len)
 {
 	NetState = NETLOOP_SUCCESS;	/* got arp reply - quit net loop */
 }
 
-static void nc_handler (uchar * pkt, unsigned dest, unsigned src,
+static void nc_handler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			unsigned len)
 {
 	if (input_size)
diff --git a/include/net.h b/include/net.h
index 95ef8ab..01f7159 100644
--- a/include/net.h
+++ b/include/net.h
@@ -72,12 +72,17 @@
 typedef ulong		IPaddr_t;
 
 
-/*
- * The current receive packet handler.  Called with a pointer to the
- * application packet, and a protocol type (PORT_BOOTPC or PORT_TFTP).
- * All other packets are dealt with without calling the handler.
+/**
+ * An incoming packet handler.
+ * @param pkt    pointer to the application packet
+ * @param dport  destination UDP port
+ * @param sip    source IP address
+ * @param sport  source UDP port
+ * @param len    packet length
  */
-typedef void	rxhand_f(uchar *, unsigned, unsigned, unsigned);
+typedef void rxhand_f(uchar *pkt, unsigned dport,
+		      IPaddr_t sip, unsigned sport,
+		      unsigned len);
 
 /*
  *	A timeout handler.  Called after time interval has expired.
diff --git a/net/bootp.c b/net/bootp.c
index 87b027e..4db63cb 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -44,7 +44,8 @@ ulong		seed1, seed2;
 dhcp_state_t dhcp_state = INIT;
 unsigned long dhcp_leasetime = 0;
 IPaddr_t NetDHCPServerIP = 0;
-static void DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len);
+static void DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+			unsigned len);
 
 /* For Debug */
 #if 0
@@ -282,7 +283,8 @@ static void BootpVendorProcess (u8 * ext, int size)
  *	Handle a BOOTP received packet.
  */
 static void
-BootpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+BootpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	     unsigned len)
 {
 	Bootp_t *bp;
 	char	*s;
@@ -858,7 +860,8 @@ static void DhcpSendRequestPkt(Bootp_t *bp_offer)
  *	Handle DHCP received packets.
  */
 static void
-DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	    unsigned len)
 {
 	Bootp_t *bp = (Bootp_t *)pkt;
 
diff --git a/net/dns.c b/net/dns.c
index bb3e3f5..b51d1bd 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -101,7 +101,7 @@ DnsTimeout(void)
 }
 
 static void
-DnsHandler(uchar *pkt, unsigned dest, unsigned src, unsigned len)
+DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 {
 	struct header *header;
 	const unsigned char *p, *e, *s;
diff --git a/net/net.c b/net/net.c
index a609632..132f99b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -555,7 +555,8 @@ startAgainTimeout(void)
 }
 
 static void
-startAgainHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+startAgainHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
+		  unsigned src, unsigned len)
 {
 	/* Totally ignore the packet */
 }
@@ -752,13 +753,10 @@ PingTimeout (void)
 }
 
 static void
-PingHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+PingHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	    unsigned len)
 {
-	IPaddr_t tmp;
-	volatile IP_t *ip = (volatile IP_t *)pkt;
-
-	tmp = NetReadIP((void *)&ip->ip_src);
-	if (tmp != NetPingIP)
+	if (sip != NetPingIP)
 		return;
 
 	NetState = NETLOOP_SUCCESS;
@@ -990,7 +988,8 @@ CDPTimeout (void)
 }
 
 static void
-CDPDummyHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+CDPDummyHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+		unsigned len)
 {
 	/* nothing */
 }
@@ -1304,6 +1303,7 @@ NetReceive(volatile uchar * inpkt, int len)
 	IP_t	*ip;
 	ARP_t	*arp;
 	IPaddr_t tmp;
+	IPaddr_t src_ip;
 	int	x;
 	uchar *pkt;
 #if defined(CONFIG_CMD_CDP)
@@ -1477,7 +1477,7 @@ NetReceive(volatile uchar * inpkt, int len)
 				memcpy(NetArpWaitPacketMAC, &arp->ar_data[0], 6);
 
 #ifdef CONFIG_NETCONSOLE
-				(*packetHandler)(0,0,0,0);
+				(*packetHandler)(0, 0, 0, 0, 0);
 #endif
 				/* modify header, and transmit it */
 				memcpy(((Ethernet_t *)NetArpWaitTxPacket)->et_dest, NetArpWaitPacketMAC, 6);
@@ -1517,7 +1517,7 @@ NetReceive(volatile uchar * inpkt, int len)
 				NetCopyIP(&NetServerIP, &arp->ar_data[ 6]);
 			memcpy (NetServerEther, &arp->ar_data[ 0], 6);
 
-			(*packetHandler)(0,0,0,0);
+			(*packetHandler)(0, 0, 0, 0, 0);
 		}
 		break;
 #endif
@@ -1557,6 +1557,8 @@ NetReceive(volatile uchar * inpkt, int len)
 #endif
 			return;
 		}
+		/* Read source IP address for later use */
+		src_ip = NetReadIP(&ip->ip_src);
 		/*
 		 * The function returns the unchanged packet if it's not
 		 * a fragment, and either the complete packet or NULL if
@@ -1596,11 +1598,12 @@ NetReceive(volatile uchar * inpkt, int len)
 				 *	IP header OK.  Pass the packet to the current handler.
 				 */
 				/* XXX point to ip packet */
-				(*packetHandler)((uchar *)ip, 0, 0, 0);
+				(*packetHandler)((uchar *)ip, 0, src_ip, 0, 0);
 				return;
 			case ICMP_ECHO_REQUEST:
-				debug("Got ICMP ECHO REQUEST, return %d bytes \n",
-					ETHER_HDR_SIZE + len);
+				debug("Got ICMP ECHO REQUEST, "
+				      "return %d bytes\n",
+				      ETHER_HDR_SIZE + len);
 
 				memcpy (&et->et_dest[0], &et->et_src[0], 6);
 				memcpy (&et->et_src[ 0], NetOurEther, 6);
@@ -1678,6 +1681,7 @@ NetReceive(volatile uchar * inpkt, int len)
 		 */
 		(*packetHandler)((uchar *)ip +IP_HDR_SIZE,
 						ntohs(ip->udp_dst),
+						src_ip,
 						ntohs(ip->udp_src),
 						ntohs(ip->udp_len) - 8);
 		break;
diff --git a/net/nfs.c b/net/nfs.c
index d11bb4c..f76f60d 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -580,7 +580,7 @@ NfsTimeout (void)
 }
 
 static void
-NfsHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
+NfsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 {
 	int rlen;
 
diff --git a/net/rarp.c b/net/rarp.c
index 9444c03..94c86d3 100644
--- a/net/rarp.c
+++ b/net/rarp.c
@@ -43,7 +43,8 @@ int		RarpTry;
  *	Handle a RARP received packet.
  */
 static void
-RarpHandler(uchar * dummi0, unsigned dummi1, unsigned dummi2, unsigned dummi3)
+RarpHandler(uchar *dummi0, unsigned dummi1, IPaddr_t sip, unsigned dummi2,
+	    unsigned dummi3)
 {
 	char *s;
 	debug("Got good RARP\n");
diff --git a/net/sntp.c b/net/sntp.c
index 76c10ec..82f2fe6 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -48,7 +48,8 @@ SntpTimeout (void)
 }
 
 static void
-SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
+SntpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	    unsigned len)
 {
 	struct sntp_pkt_t *rpktp = (struct sntp_pkt_t *)pkt;
 	struct rtc_time tm;
diff --git a/net/tftp.c b/net/tftp.c
index ed559b7..00abec3 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -278,7 +278,8 @@ TftpSend (void)
 
 
 static void
-TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	    unsigned len)
 {
 	ushort proto;
 	ushort *s;
-- 
1.7.1

[U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers

[Detlev Zundel]

Hi Luca,

> This is needed for the upcoming TFTP server implementation.
>
> This also simplifies PingHandler() and fixes rxhand_f documentation.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2:
>  - fixed checkpatch issues.
>
>  drivers/net/netconsole.c |    5 +++--
>  include/net.h            |   15 ++++++++++-----
>  net/bootp.c              |    9 ++++++---
>  net/dns.c                |    2 +-
>  net/net.c                |   30 +++++++++++++++++-------------
>  net/nfs.c                |    2 +-
>  net/rarp.c               |    3 ++-
>  net/sntp.c               |    3 ++-
>  net/tftp.c               |    3 ++-
>  9 files changed, 44 insertions(+), 28 deletions(-)
>

[...]

> diff --git a/net/net.c b/net/net.c
> index a609632..132f99b 100644
> --- a/net/net.c
> +++ b/net/net.c

[...]

> @@ -1596,11 +1598,12 @@ NetReceive(volatile uchar * inpkt, int len)
>  				 *	IP header OK.  Pass the packet to the current handler.
>  				 */
>  				/* XXX point to ip packet */
> -				(*packetHandler)((uchar *)ip, 0, 0, 0);
> +				(*packetHandler)((uchar *)ip, 0, src_ip, 0, 0);
>  				return;
>  			case ICMP_ECHO_REQUEST:
> -				debug("Got ICMP ECHO REQUEST, return %d bytes \n",
> -					ETHER_HDR_SIZE + len);
> +				debug("Got ICMP ECHO REQUEST, "
> +				      "return %d bytes\n",
> +				      ETHER_HDR_SIZE + len);
>  
>  				memcpy (&et->et_dest[0], &et->et_src[0], 6);
>  				memcpy (&et->et_src[ 0], NetOurEther, 6);

This second hunk is not related to the patch at hand, so strictly
speaking it should not be in here.  It's not enough to NAK the patch
however, just something to look out for in the future.

Apart from that the changes look good, so

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
LISP has  jokingly been  described as  "the most  intelligent way to  misuse a
computer".  I think that  description a great  compliment because it transmits
the full  flavour of  liberation:  it has assisted a number of our most gifted
fellow humans in thinking previously impossible thoughts. - Edsger W. Dijkstra
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers

[Luca Ceresoli]

Il 19/04/2011 16:15, Detlev Zundel ha scritto:
> Hi Luca,
>
>> This is needed for the upcoming TFTP server implementation.
>>
>> This also simplifies PingHandler() and fixes rxhand_f documentation.
>>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>> Cc: Wolfgang Denk<wd@denx.de>
>> ---
>> Changes in v2:
>>   - fixed checkpatch issues.
>>
>>   drivers/net/netconsole.c |    5 +++--
>>   include/net.h            |   15 ++++++++++-----
>>   net/bootp.c              |    9 ++++++---
>>   net/dns.c                |    2 +-
>>   net/net.c                |   30 +++++++++++++++++-------------
>>   net/nfs.c                |    2 +-
>>   net/rarp.c               |    3 ++-
>>   net/sntp.c               |    3 ++-
>>   net/tftp.c               |    3 ++-
>>   9 files changed, 44 insertions(+), 28 deletions(-)
>>
> [...]
>
>> diff --git a/net/net.c b/net/net.c
>> index a609632..132f99b 100644
>> --- a/net/net.c
>> +++ b/net/net.c
> [...]
>
>> @@ -1596,11 +1598,12 @@ NetReceive(volatile uchar * inpkt, int len)
>>   				 *	IP header OK.  Pass the packet to the current handler.
>>   				 */
>>   				/* XXX point to ip packet */
>> -				(*packetHandler)((uchar *)ip, 0, 0, 0);
>> +				(*packetHandler)((uchar *)ip, 0, src_ip, 0, 0);
>>   				return;
>>   			case ICMP_ECHO_REQUEST:
>> -				debug("Got ICMP ECHO REQUEST, return %d bytes \n",
>> -					ETHER_HDR_SIZE + len);
>> +				debug("Got ICMP ECHO REQUEST, "
>> +				      "return %d bytes\n",
>> +				      ETHER_HDR_SIZE + len);
>>
>>   				memcpy (&et->et_dest[0],&et->et_src[0], 6);
>>   				memcpy (&et->et_src[ 0], NetOurEther, 6);
> This second hunk is not related to the patch at hand, so strictly
> speaking it should not be in here.  It's not enough to NAK the patch
> however, just something to look out for in the future.

Yep, but it's needed for checkpatch compliance.

Luca

[U-Boot] [PATCH v2 2/6] NET: pass source IP address to packet handlers

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1303143594-5345-3-git-send-email-luca.ceresoli@comelit.it> you wrote:
> This is needed for the upcoming TFTP server implementation.
> 
> This also simplifies PingHandler() and fixes rxhand_f documentation.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2:
>  - fixed checkpatch issues.
> 
>  drivers/net/netconsole.c |    5 +++--
>  include/net.h            |   15 ++++++++++-----
>  net/bootp.c              |    9 ++++++---
>  net/dns.c                |    2 +-
>  net/net.c                |   30 +++++++++++++++++-------------
>  net/nfs.c                |    2 +-
>  net/rarp.c               |    3 ++-
>  net/sntp.c               |    3 ++-
>  net/tftp.c               |    3 ++-
>  9 files changed, 44 insertions(+), 28 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
HEALTH WARNING: Care Should Be Taken When Lifting This Product, Since
Its Mass, and Thus Its Weight, Is Dependent on Its Velocity  Relative
to the User.

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

With the upcoming TFTP server implementation, the remote node can be
either a client or a server, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2:
 - fixed checkpatch issues.

 net/tftp.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 00abec3..da545c6 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -55,18 +55,18 @@ enum {
 	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
 };
 
-static IPaddr_t TftpServerIP;
-static int	TftpServerPort;		/* The UDP port at their end		*/
-static int	TftpOurPort;		/* The UDP port at our end		*/
+static IPaddr_t TftpRemoteIP;
+static int	TftpRemotePort;	/* The UDP port at their end		*/
+static int	TftpOurPort;	/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
-static ulong	TftpBlock;		/* packet sequence number		*/
-static ulong	TftpLastBlock;		/* last packet sequence number received */
-static ulong	TftpBlockWrap;		/* count of sequence number wraparounds */
-static ulong	TftpBlockWrapOffset;	/* memory offset due to wrapping	*/
+static ulong	TftpBlock;	/* packet sequence number		*/
+static ulong	TftpLastBlock;	/* last packet sequence number received */
+static ulong	TftpBlockWrap;	/* count of sequence number wraparounds */
+static ulong	TftpBlockWrapOffset; /* memory offset due to wrapping	*/
 static int	TftpState;
 #ifdef CONFIG_TFTP_TSIZE
-static int	TftpTsize;		/* The file size reported by the server */
-static short	TftpNumchars;		/* The number of hashes we printed      */
+static int	TftpTsize;	/* The file size reported by the server */
+static short	TftpNumchars;	/* The number of hashes we printed      */
 #endif
 
 #define STATE_RRQ	1
@@ -273,7 +273,8 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpRemoteIP, TftpRemotePort,
+			 TftpOurPort, len);
 }
 
 
@@ -292,9 +293,8 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpServerPort) {
+	if (TftpState != STATE_RRQ && src != TftpRemotePort)
 		return;
-	}
 
 	if (len < 2) {
 		return;
@@ -318,7 +318,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			pkt,
 			pkt + strlen((char *)pkt) + 1);
 		TftpState = STATE_OACK;
-		TftpServerPort = src;
+		TftpRemotePort = src;
 		/*
 		 * Check for 'blksize' option.
 		 * Careful: "i" is signed, "len" is unsigned, thus
@@ -386,7 +386,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
-			TftpServerPort = src;
+			TftpRemotePort = src;
 			TftpLastBlock = 0;
 			TftpBlockWrap = 0;
 			TftpBlockWrapOffset = 0;
@@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 
 		/*
 		 *	Acknoledge the block just received, which will prompt
-		 *	the server for the next one.
+		 *	the remote for the next one.
 		 */
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
@@ -548,7 +548,7 @@ TftpStart (void)
 	debug("TFTP blocksize = %i, timeout = %ld ms\n",
 		TftpBlkSizeOption, TftpTimeoutMSecs);
 
-	TftpServerIP = NetServerIP;
+	TftpRemoteIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
@@ -568,7 +568,7 @@ TftpStart (void)
 			strncpy(tftp_filename, BootFile, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		} else {
-			TftpServerIP = string_to_ip (BootFile);
+			TftpRemoteIP = string_to_ip(BootFile);
 			strncpy(tftp_filename, p + 1, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		}
@@ -578,12 +578,12 @@ TftpStart (void)
 	printf ("Using %s device\n", eth_get_name());
 #endif
 	printf("TFTP from server %pI4"
-		"; our IP address is %pI4", &TftpServerIP, &NetOurIP);
+		"; our IP address is %pI4", &TftpRemoteIP, &NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet	= TftpRemoteIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet)
 		printf("; sending through gateway %pI4", &NetOurGatewayIP);
@@ -608,7 +608,7 @@ TftpStart (void)
 	NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
 	NetSetHandler (TftpHandler);
 
-	TftpServerPort = WELL_KNOWN_PORT;
+	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
 	TftpState = STATE_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
@@ -616,7 +616,7 @@ TftpStart (void)
 
 #ifdef CONFIG_TFTP_PORT
 	if ((ep = getenv("tftpdstp")) != NULL) {
-		TftpServerPort = simple_strtol(ep, NULL, 10);
+		TftpRemotePort = simple_strtol(ep, NULL, 10);
 	}
 	if ((ep = getenv("tftpsrcp")) != NULL) {
 		TftpOurPort= simple_strtol(ep, NULL, 10);
-- 
1.7.1

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

> With the upcoming TFTP server implementation, the remote node can be
> either a client or a server, so avoid ambiguities.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2:
>  - fixed checkpatch issues.
>
>  net/tftp.c |   42 +++++++++++++++++++++---------------------
>  1 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/net/tftp.c b/net/tftp.c
> index 00abec3..da545c6 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -55,18 +55,18 @@ enum {
>  	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
>  };
>  
> -static IPaddr_t TftpServerIP;
> -static int	TftpServerPort;		/* The UDP port at their end		*/
> -static int	TftpOurPort;		/* The UDP port at our end		*/
> +static IPaddr_t TftpRemoteIP;
> +static int	TftpRemotePort;	/* The UDP port at their end		*/
> +static int	TftpOurPort;	/* The UDP port at our end		*/
>  static int	TftpTimeoutCount;
> -static ulong	TftpBlock;		/* packet sequence number		*/
> -static ulong	TftpLastBlock;		/* last packet sequence number received */
> -static ulong	TftpBlockWrap;		/* count of sequence number wraparounds */
> -static ulong	TftpBlockWrapOffset;	/* memory offset due to wrapping	*/
> +static ulong	TftpBlock;	/* packet sequence number		*/
> +static ulong	TftpLastBlock;	/* last packet sequence number received */
> +static ulong	TftpBlockWrap;	/* count of sequence number wraparounds */
> +static ulong	TftpBlockWrapOffset; /* memory offset due to wrapping	*/

These changes are indentation only changes, so they should be in a
separate patch.

>  static int	TftpState;
>  #ifdef CONFIG_TFTP_TSIZE
> -static int	TftpTsize;		/* The file size reported by the server */
> -static short	TftpNumchars;		/* The number of hashes we printed      */
> +static int	TftpTsize;	/* The file size reported by the server */
> +static short	TftpNumchars;	/* The number of hashes we printed      */

dito.

[...]

> @@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>  
>  		/*
>  		 *	Acknoledge the block just received, which will prompt
> -		 *	the server for the next one.
> +		 *	the remote for the next one.

Hey, while you're at it, please fix the "Acknoledge" typo ;)

[...]

> @@ -568,7 +568,7 @@ TftpStart (void)
>  			strncpy(tftp_filename, BootFile, MAX_LEN);
>  			tftp_filename[MAX_LEN-1] = 0;
>  		} else {
> -			TftpServerIP = string_to_ip (BootFile);
> +			TftpRemoteIP = string_to_ip(BootFile);

Whitespace fix.

Apart from that, patch looks simple enough, so

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
14474011154664524427946373126085988481573677491474835889066354349131199152128
If you know why this number is perfect - you're probably a mathematician...
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Il 19/04/2011 16:18, Detlev Zundel ha scritto:
> Hi Luca,
>
>> With the upcoming TFTP server implementation, the remote node can be
>> either a client or a server, so avoid ambiguities.
>>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>> Cc: Wolfgang Denk<wd@denx.de>
>> ---
>> Changes in v2:
>>   - fixed checkpatch issues.
>>
>>   net/tftp.c |   42 +++++++++++++++++++++---------------------
>>   1 files changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/net/tftp.c b/net/tftp.c
>> index 00abec3..da545c6 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -55,18 +55,18 @@ enum {
>>   	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
>>   };
>>
>> -static IPaddr_t TftpServerIP;
>> -static int	TftpServerPort;		/* The UDP port at their end		*/
>> -static int	TftpOurPort;		/* The UDP port at our end		*/
>> +static IPaddr_t TftpRemoteIP;
>> +static int	TftpRemotePort;	/* The UDP port at their end		*/
>> +static int	TftpOurPort;	/* The UDP port at our end		*/
>>   static int	TftpTimeoutCount;
>> -static ulong	TftpBlock;		/* packet sequence number		*/
>> -static ulong	TftpLastBlock;		/* last packet sequence number received */
>> -static ulong	TftpBlockWrap;		/* count of sequence number wraparounds */
>> -static ulong	TftpBlockWrapOffset;	/* memory offset due to wrapping	*/
>> +static ulong	TftpBlock;	/* packet sequence number		*/
>> +static ulong	TftpLastBlock;	/* last packet sequence number received */
>> +static ulong	TftpBlockWrap;	/* count of sequence number wraparounds */
>> +static ulong	TftpBlockWrapOffset; /* memory offset due to wrapping	*/
> These changes are indentation only changes, so they should be in a
> separate patch.

It's needed for checkpatch compliance.


>>   static int	TftpState;
>>   #ifdef CONFIG_TFTP_TSIZE
>> -static int	TftpTsize;		/* The file size reported by the server */
>> -static short	TftpNumchars;		/* The number of hashes we printed      */
>> +static int	TftpTsize;	/* The file size reported by the server */
>> +static short	TftpNumchars;	/* The number of hashes we printed      */
> dito.
>
> [...]
>
>> @@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>>
>>   		/*
>>   		 *	Acknoledge the block just received, which will prompt
>> -		 *	the server for the next one.
>> +		 *	the remote for the next one.
> Hey, while you're at it, please fix the "Acknoledge" typo ;)

Will do.

> [...]
>
>> @@ -568,7 +568,7 @@ TftpStart (void)
>>   			strncpy(tftp_filename, BootFile, MAX_LEN);
>>   			tftp_filename[MAX_LEN-1] = 0;
>>   		} else {
>> -			TftpServerIP = string_to_ip (BootFile);
>> +			TftpRemoteIP = string_to_ip(BootFile);
> Whitespace fix.

checkpatch again.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

[...]

>> Whitespace fix.
>
> checkpatch again.

Hm, I see.  Still, can we have one commit (with "cosmetic" in the
changelog) that silences checkpatch but does not have any functional
changes?  We really try hard to separate cosmetic from functional
changes.  This makes reviewing (and debugging) so much easier.

This is the second time that I personally encounter that problem and I
still don't see an automated way to take care of this.  This simply is
the result that checkpatch will flag only new errors when there are no
_old_ ones.  The reality of course looks different:

  [dzu at pollux u-boot-testing (master)]$ ../linux/scripts/checkpatch.pl -f net/net.c
  
  [...]
  
  total: 76 errors, 193 warnings, 1934 lines checked
  
  net/net.c has style problems, please review.  If any of these errors
  are false positives report them to the maintainer, see
  CHECKPATCH in MAINTAINERS.

If we strictly do require checkpatch cleanliness, maybe we should start
cleaning up the code base as is first with only cosmetic changes.  Do I
see volunteers? ;)

Cheers
  Detlev

-- 
Life is complex. It has real and imaginary components.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Hi,

just a few e-mails ago along this thread Albert Aribaud wrote:

>  My opinion is that you should make sure that at least the code you touch
>  is checkpatch-clean, so yes, you should fix that; but there is no need
>  to submit 'checkpatch-compliance' patches. Just fix the line here so
>  that checkpatch does not complain.


So I proceeded along that way.

Now Detlev Zundel wrote:
> ...
> Hm, I see.  Still, can we have one commit (with "cosmetic" in the
> changelog) that silences checkpatch but does not have any functional
> changes?  We really try hard to separate cosmetic from functional
> changes.  This makes reviewing (and debugging) so much easier.

While I appreciate the careful review of my patches, I cannot hide
that it is discouraging for new contributors to be requested for
contradictory modifications.

There should be one precise policy, and that should be clearly documented.

http://www.denx.de/wiki/U-Boot/CodingStyle is the place where I would
expect to find it.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

> Hi,
>
> just a few e-mails ago along this thread Albert Aribaud wrote:
>
>>  My opinion is that you should make sure that at least the code you touch
>>  is checkpatch-clean, so yes, you should fix that; but there is no need
>>  to submit 'checkpatch-compliance' patches. Just fix the line here so
>>  that checkpatch does not complain.
>
>
> So I proceeded along that way.
>
> Now Detlev Zundel wrote:
>> ...
>> Hm, I see.  Still, can we have one commit (with "cosmetic" in the
>> changelog) that silences checkpatch but does not have any functional
>> changes?  We really try hard to separate cosmetic from functional
>> changes.  This makes reviewing (and debugging) so much easier.
>
> While I appreciate the careful review of my patches, I cannot hide
> that it is discouraging for new contributors to be requested for
> contradictory modifications.

Sorry for that, but it is only that we start using checkpatch more
aggressively, that such problems turn up which we did not yet agree on
how to solve.

> There should be one precise policy, and that should be clearly documented.

I fully agree.

> http://www.denx.de/wiki/U-Boot/CodingStyle is the place where I would
> expect to find it.

I'll start a new thread to discuss this.  Hopefully we then come up with
a policy to stick into that wiki page.

Thanks for bearing with me
  Detlev

-- 
In God we trust.  All others we monitor
                       -- NSA motto
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Detlev Zundel wrote:
>
> I'll start a new thread to discuss this.  Hopefully we then come up with
> a policy to stick into that wiki page.
>

Now that the checkpatch policy is much more clear, I started to clean up
the networking stuff, on top of which the TFTP server sits.

net/net.c alone counts 76 errors and 197 warnings. In terms of modified
lines, it's going to be a big job, so I am doing it in separate patches, one
per each category of errors/warnings (whitespace issues, lines >80 chars,
parentheses issues etc).
Is this choice ok?

Also, it's going to be a bigger job than the TFTP server itself, so I'll 
send a
separate patchset for the cleanup work, and hold the TFTP server until the
cleanup is worked out.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

> Detlev Zundel wrote:
>>
>> I'll start a new thread to discuss this.  Hopefully we then come up with
>> a policy to stick into that wiki page.
>>
>
> Now that the checkpatch policy is much more clear, I started to clean up
> the networking stuff, on top of which the TFTP server sits.

Thanks a lot for starting this cleanup.  Your work is very much
appreciated.

> net/net.c alone counts 76 errors and 197 warnings. In terms of modified
> lines, it's going to be a big job, so I am doing it in separate patches, one
> per each category of errors/warnings (whitespace issues, lines >80 chars,
> parentheses issues etc).
> Is this choice ok?

This makes sense to me yes.

> Also, it's going to be a bigger job than the TFTP server itself, so
> I'll send a
> separate patchset for the cleanup work, and hold the TFTP server until the
> cleanup is worked out.

If you really start the clenaup, then this order makes absolute sense.

Thanks in advance!
  Detlev

-- 
Every time history repeats itself the price goes up.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

> Il 19/04/2011 16:18, Detlev Zundel ha scritto:
>> Hi Luca,
>>
>>> With the upcoming TFTP server implementation, the remote node can be
>>> either a client or a server, so avoid ambiguities.
>>>
>>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> ---
>>> Changes in v2:
>>>   - fixed checkpatch issues.
>>>
>>>   net/tftp.c |   42 +++++++++++++++++++++---------------------
>>>   1 files changed, 21 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/net/tftp.c b/net/tftp.c
>>> index 00abec3..da545c6 100644
>>> --- a/net/tftp.c
>>> +++ b/net/tftp.c
>>> @@ -55,18 +55,18 @@ enum {
>>>   	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
>>>   };
>>>
>>> -static IPaddr_t TftpServerIP;
>>> -static int	TftpServerPort;		/* The UDP port at their end		*/
>>> -static int	TftpOurPort;		/* The UDP port at our end		*/
>>> +static IPaddr_t TftpRemoteIP;
>>> +static int	TftpRemotePort;	/* The UDP port at their end		*/
>>> +static int	TftpOurPort;	/* The UDP port at our end		*/
>>>   static int	TftpTimeoutCount;
>>> -static ulong	TftpBlock;		/* packet sequence number		*/
>>> -static ulong	TftpLastBlock;		/* last packet sequence number received */
>>> -static ulong	TftpBlockWrap;		/* count of sequence number wraparounds */
>>> -static ulong	TftpBlockWrapOffset;	/* memory offset due to wrapping	*/
>>> +static ulong	TftpBlock;	/* packet sequence number		*/
>>> +static ulong	TftpLastBlock;	/* last packet sequence number received */
>>> +static ulong	TftpBlockWrap;	/* count of sequence number wraparounds */
>>> +static ulong	TftpBlockWrapOffset; /* memory offset due to wrapping	*/
>> These changes are indentation only changes, so they should be in a
>> separate patch.
>
> It's needed for checkpatch compliance.

I'm trying to understand the problems involved, but looking at this
again, it is not clear to me what you say here.  When I run your version
1 of the patches (where you only do the rename) through checkpatch, I
get:

  WARNING: line over 80 characters
  #116: FILE: net/tftp.c:59:
  +static int	TftpRemotePort;		/* The UDP port at their end		*/
  
  WARNING: consider using kstrto* in preference to simple_strtol
  #215: FILE: net/tftp.c:619:
  +		TftpRemotePort = simple_strtol(ep, NULL, 10);
  
  total: 0 errors, 2 warnings, 99 lines checked
  
  /home/dzu/transfer/p2 has style problems, please review.  If any of these errors
  are false positives report them to the maintainer, see
  CHECKPATCH in MAINTAINERS.

So I'm not sure why you say that the other changes are needed for
checkpatch.  What exactly do you mean by this?

Thanks
  Detlev

-- 
C hasn't changed much since the 1970s. And let's face it it's ugly.
Can't we do better? C++? (Sorry, never mind.)
                                    -- Rob Pike
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Detlev Zundel wrote:
> Hi Luca,
>
>> Il 19/04/2011 16:18, Detlev Zundel ha scritto:
>>> Hi Luca,
>>>
>>>> With the upcoming TFTP server implementation, the remote node can be
>>>> either a client or a server, so avoid ambiguities.
>>>>
>>>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>>>> Cc: Wolfgang Denk<wd@denx.de>
>>>> ---
>>>> Changes in v2:
>>>>    - fixed checkpatch issues.
>>>>
>>>>    net/tftp.c |   42 +++++++++++++++++++++---------------------
>>>>    1 files changed, 21 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/net/tftp.c b/net/tftp.c
>>>> index 00abec3..da545c6 100644
>>>> --- a/net/tftp.c
>>>> +++ b/net/tftp.c
>>>> @@ -55,18 +55,18 @@ enum {
>>>>    	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
>>>>    };
>>>>
>>>> -static IPaddr_t TftpServerIP;
>>>> -static int	TftpServerPort;		/* The UDP port at their end		*/
>>>> -static int	TftpOurPort;		/* The UDP port at our end		*/
>>>> +static IPaddr_t TftpRemoteIP;
>>>> +static int	TftpRemotePort;	/* The UDP port at their end		*/
>>>> +static int	TftpOurPort;	/* The UDP port at our end		*/
>>>>    static int	TftpTimeoutCount;
>>>> -static ulong	TftpBlock;		/* packet sequence number		*/
>>>> -static ulong	TftpLastBlock;		/* last packet sequence number received */
>>>> -static ulong	TftpBlockWrap;		/* count of sequence number wraparounds */
>>>> -static ulong	TftpBlockWrapOffset;	/* memory offset due to wrapping	*/
>>>> +static ulong	TftpBlock;	/* packet sequence number		*/
>>>> +static ulong	TftpLastBlock;	/* last packet sequence number received */
>>>> +static ulong	TftpBlockWrap;	/* count of sequence number wraparounds */
>>>> +static ulong	TftpBlockWrapOffset; /* memory offset due to wrapping	*/
>>> These changes are indentation only changes, so they should be in a
>>> separate patch.
>> It's needed for checkpatch compliance.
> I'm trying to understand the problems involved, but looking at this
> again, it is not clear to me what you say here.  When I run your version
> 1 of the patches (where you only do the rename) through checkpatch, I
> get:
>
>    WARNING: line over 80 characters
>    #116: FILE: net/tftp.c:59:
>    +static int	TftpRemotePort;		/* The UDP port at their end		*/
>
>    WARNING: consider using kstrto* in preference to simple_strtol
>    #215: FILE: net/tftp.c:619:
>    +		TftpRemotePort = simple_strtol(ep, NULL, 10);
>
>    total: 0 errors, 2 warnings, 99 lines checked
>
>    /home/dzu/transfer/p2 has style problems, please review.  If any of these errors
>    are false positives report them to the maintainer, see
>    CHECKPATCH in MAINTAINERS.
>
> So I'm not sure why you say that the other changes are needed for
> checkpatch.  What exactly do you mean by this?

All the comments were nicely columned before my patchset. Reducing the
length of a line would have broken this.

I chose to change all of them in order to preserve the pre-existing 
coding style.

Luca

[U-Boot] checkpatch - theory and practice [was: [PATCH v2 3/6] TFTP: rename "server" to "remote"]

[Detlev Zundel]

Hi Luca,

>>> It's needed for checkpatch compliance.
>> I'm trying to understand the problems involved, but looking at this
>> again, it is not clear to me what you say here.  When I run your version
>> 1 of the patches (where you only do the rename) through checkpatch, I
>> get:
>>
>>    WARNING: line over 80 characters
>>    #116: FILE: net/tftp.c:59:
>>    +static int	TftpRemotePort;		/* The UDP port at their end		*/
>>
>>    WARNING: consider using kstrto* in preference to simple_strtol
>>    #215: FILE: net/tftp.c:619:
>>    +		TftpRemotePort = simple_strtol(ep, NULL, 10);
>>
>>    total: 0 errors, 2 warnings, 99 lines checked
>>
>>    /home/dzu/transfer/p2 has style problems, please review.  If any of these errors
>>    are false positives report them to the maintainer, see
>>    CHECKPATCH in MAINTAINERS.
>>
>> So I'm not sure why you say that the other changes are needed for
>> checkpatch.  What exactly do you mean by this?
>
> All the comments were nicely columned before my patchset. Reducing the
> length of a line would have broken this.
>
> I chose to change all of them in order to preserve the pre-existing
> coding style.

Ok, this makes sense, alas the wording "it's needed for checkpatch
compliance" was somewhat misleading.

Ideally only the relevant changes should be in one commit and
re-indentation to align everything again should be in a separate commit.
As we saw that checkpatch also looks at context lines, this commit
usually needs to be logically _before_ your own changes.  Probably the
easiest way to achieve this is to commit the changes separately and
reorder them with git rebase -i.

I amended the wiki page[1] in the hope of getting more light into these
things.

Cheers
  Detlev

[1] http://www.denx.de/wiki/U-Boot/Patches

-- 
Irrationality is the square root of all evil.
                                     -- Douglas Hofstadter
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Luca Ceresoli wrote:
> Il 19/04/2011 16:18, Detlev Zundel ha scritto:
>> Hi Luca,
>>
>>> With the upcoming TFTP server implementation, the remote node can be
>>> either a client or a server, so avoid ambiguities.
>>>
>>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> ---
>>> Changes in v2:
>>>   - fixed checkpatch issues.
>>>
>>>   net/tftp.c |   42 +++++++++++++++++++++---------------------
>>>   1 files changed, 21 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/net/tftp.c b/net/tftp.c
>>> index 00abec3..da545c6 100644
>>> --- a/net/tftp.c
>>> +++ b/net/tftp.c
>>> @@ -55,18 +55,18 @@ enum {
>>>       TFTP_ERR_FILE_ALREADY_EXISTS = 6,
>>>   };
>>>
>>> -static IPaddr_t TftpServerIP;
>>> -static int    TftpServerPort;        /* The UDP port at their 
>>> end        */
>>> -static int    TftpOurPort;        /* The UDP port at our end        */
>>> +static IPaddr_t TftpRemoteIP;
>>> +static int    TftpRemotePort;    /* The UDP port at their 
>>> end        */
>>> +static int    TftpOurPort;    /* The UDP port at our end        */
>>>   static int    TftpTimeoutCount;
>>> -static ulong    TftpBlock;        /* packet sequence number        */
>>> -static ulong    TftpLastBlock;        /* last packet sequence 
>>> number received */
>>> -static ulong    TftpBlockWrap;        /* count of sequence number 
>>> wraparounds */
>>> -static ulong    TftpBlockWrapOffset;    /* memory offset due to 
>>> wrapping    */
>>> +static ulong    TftpBlock;    /* packet sequence number        */
>>> +static ulong    TftpLastBlock;    /* last packet sequence number 
>>> received */
>>> +static ulong    TftpBlockWrap;    /* count of sequence number 
>>> wraparounds */
>>> +static ulong    TftpBlockWrapOffset; /* memory offset due to 
>>> wrapping    */
>> These changes are indentation only changes, so they should be in a
>> separate patch.
>
> It's needed for checkpatch compliance.
>
>
>>>   static int    TftpState;
>>>   #ifdef CONFIG_TFTP_TSIZE
>>> -static int    TftpTsize;        /* The file size reported by the 
>>> server */
>>> -static short    TftpNumchars;        /* The number of hashes we 
>>> printed      */
>>> +static int    TftpTsize;    /* The file size reported by the server */
>>> +static short    TftpNumchars;    /* The number of hashes we 
>>> printed      */
>> dito.
>>
>> [...]
>>
>>> @@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t 
>>> sip, unsigned src,
>>>
>>>           /*
>>>            *    Acknoledge the block just received, which will prompt
>>> -         *    the server for the next one.
>>> +         *    the remote for the next one.
>> Hey, while you're at it, please fix the "Acknoledge" typo ;)
>
> Will do.

Done for v3.

I removed the checkpatch-related changes: they are now on the tftp 
cleanup patch series that I submitted on saturday, and on top of which 
v3 of the TFTP server will be based.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Detlev Zundel]

Hi Luca,

[...]

> I removed the checkpatch-related changes: they are now on the tftp
> cleanup patch series that I submitted on saturday, and on top of which
> v3 of the TFTP server will be based.

Thanks for your persistence - it is very much appreciated!
  Detlev

-- 
The proprietary-Unix players proved so ponderous, so blind, and so inept at
marketing that Microsoft was able to grab away a large part of their market
with the shockingly inferior technology of its Windows operating system.
                   -- "A Brief History of Hackerdom" by Eric Steven Raymond
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Mike Frysinger]

On Monday, April 18, 2011 12:19:51 Luca Ceresoli wrote:
> With the upcoming TFTP server implementation, the remote node can be
> either a client or a server, so avoid ambiguities.

the summary made me worried because i thought you were going to rename the env 
var "server" to "remote".  could you tweak the summary to avoid this ambiguity 
in what you're actually doing ?  how about:
	TFTP: use "remote" in local variable names instead of "server"

> -	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
> +	    IPaddr_t ServerNet	= TftpRemoteIP & NetOurSubnetMask;

shouldnt that now be RemoteNet ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110430/eeda7465/attachment.pgp 

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Mike Frysinger wrote:
> On Monday, April 18, 2011 12:19:51 Luca Ceresoli wrote:
>> With the upcoming TFTP server implementation, the remote node can be
>> either a client or a server, so avoid ambiguities.
> the summary made me worried because i thought you were going to rename the env
> var "server" to "remote".  could you tweak the summary to avoid this ambiguity
> in what you're actually doing ?  how about:
> 	TFTP: use "remote" in local variable names instead of "server"

Improved commit message in v3.

>> -	    IPaddr_t ServerNet	= TftpServerIP&  NetOurSubnetMask;
>> +	    IPaddr_t ServerNet	= TftpRemoteIP&  NetOurSubnetMask;
> shouldnt that now be RemoteNet ?

Done for v3.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1303143594-5345-4-git-send-email-luca.ceresoli@comelit.it> you wrote:
> With the upcoming TFTP server implementation, the remote node can be
> either a client or a server, so avoid ambiguities.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2:
>  - fixed checkpatch issues.
> 
>  net/tftp.c |   42 +++++++++++++++++++++---------------------
>  1 files changed, 21 insertions(+), 21 deletions(-)

How do we proceed now?  I've applied paches 1 + 2 of this series,
but for patch 3 we had changes requested, and the following patche
sdepend on it.

I understand you are now waiting for the "net/net.c: cosmetic:"
patches to go in?  Normally these would be stuff for the next
branch...

I'd even be willing to pull these in now, if you then would re-post
the remaining patches of the tftpserver code soon?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Diplomacy is the art of saying "nice doggy" until you can find a rock.

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Hi Wolfgang,

Wolfgang Denk wrote:
> Dear Luca Ceresoli,
>
> In message<1303143594-5345-4-git-send-email-luca.ceresoli@comelit.it>  you wrote:
>> With the upcoming TFTP server implementation, the remote node can be
>> either a client or a server, so avoid ambiguities.
>>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>> Cc: Wolfgang Denk<wd@denx.de>
>> ---
>> Changes in v2:
>>   - fixed checkpatch issues.
>>
>>   net/tftp.c |   42 +++++++++++++++++++++---------------------
>>   1 files changed, 21 insertions(+), 21 deletions(-)
> How do we proceed now?  I've applied paches 1 + 2 of this series,
> but for patch 3 we had changes requested, and the following patche
> sdepend on it.
>
> I understand you are now waiting for the "net/net.c: cosmetic:"
> patches to go in?  Normally these would be stuff for the next
> branch...
>
> I'd even be willing to pull these in now, if you then would re-post
> the remaining patches of the tftpserver code soon?

I see you've committed the net/net.c cleanup into master.
I'll rebase the TFTP server work (only patches 3 and 4) and resend them
as soon as possible for inclusion in master, perhaps today or tomorrow.

Luca

[U-Boot] [PATCH v2 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

Luca Ceresoli wrote:
> Hi Wolfgang,
>
> Wolfgang Denk wrote:
>> Dear Luca Ceresoli,
>>
>> In 
>> message<1303143594-5345-4-git-send-email-luca.ceresoli@comelit.it>  
>> you wrote:
>>> With the upcoming TFTP server implementation, the remote node can be
>>> either a client or a server, so avoid ambiguities.
>>>
>>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>>> Cc: Wolfgang Denk<wd@denx.de>
>>> ---
>>> Changes in v2:
>>>   - fixed checkpatch issues.
>>>
>>>   net/tftp.c |   42 +++++++++++++++++++++---------------------
>>>   1 files changed, 21 insertions(+), 21 deletions(-)
>> How do we proceed now?  I've applied paches 1 + 2 of this series,
>> but for patch 3 we had changes requested, and the following patche
>> sdepend on it.
>>
>> I understand you are now waiting for the "net/net.c: cosmetic:"
>> patches to go in?  Normally these would be stuff for the next
>> branch...
>>
>> I'd even be willing to pull these in now, if you then would re-post
>> the remaining patches of the tftpserver code soon?
>
> I see you've committed the net/net.c cleanup into master.
> I'll rebase the TFTP server work (only patches 3 and 4) and resend them
> as soon as possible for inclusion in master, perhaps today or tomorrow.
>

Damn. While rebasing, I hit many checkpatch issues. The cleanup soon became
larger that the TFTP server work, so I took a breath, and started a new 
branch
out of master to fully cleanup net/tftp.c.

I submitted right now a lengthy patch series just for that cleanup.
Since it's very similar to my net/net.c cleanup that has just been 
merged, I think
this will be merged also without a great need of discussion.

I'll take some time ASAP to rebase the TFTP server on top of this cleanup,
but alas not today nor tomorrow (I spent all the time with checkpatch...).

Luca

[U-Boot] [PATCH v2 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Luca Ceresoli]

With the upcoming TFTP server implementation, requests can be either
outgoing or incoming, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2: none.

 net/tftp.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index da545c6..e166a63 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -69,7 +69,7 @@ static int	TftpTsize;	/* The file size reported by the server */
 static short	TftpNumchars;	/* The number of hashes we printed      */
 #endif
 
-#define STATE_RRQ	1
+#define STATE_SEND_RRQ	1
 #define STATE_DATA	2
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
@@ -200,7 +200,7 @@ TftpSend (void)
 
 	switch (TftpState) {
 
-	case STATE_RRQ:
+	case STATE_SEND_RRQ:
 		xp = pkt;
 		s = (ushort *)pkt;
 		*s++ = htons(TFTP_RRQ);
@@ -293,7 +293,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
 		return;
 
 	if (len < 2) {
@@ -380,10 +380,10 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			}
 		}
 
-		if (TftpState == STATE_RRQ)
+		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -610,7 +610,7 @@ TftpStart (void)
 
 	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
-	TftpState = STATE_RRQ;
+	TftpState = STATE_SEND_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
 	TftpOurPort = 1024 + (get_timer(0) % 3072);
 
-- 
1.7.1

[U-Boot] [PATCH v2 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Detlev Zundel]

Hi Luca,

> With the upcoming TFTP server implementation, requests can be either
> outgoing or incoming, so avoid ambiguities.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2: none.

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
We can forgive a man for making a useful thing as long as he does not
admire it.  The only excuse  for making  a useless  thing is that one
admires it intensely.
                                    --- Oscar Wilde
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2: none.

 net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 net/tftp.h |    6 +++++
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index e166a63..87eb0b8 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -2,6 +2,8 @@
  *	Copyright 1994, 1995, 2000 Neil Russell.
  *	(See License)
  *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
+ *	Copyright 2011 Comelit Group SpA,
+ *	               Luca Ceresoli <luca.ceresoli@comelit.it>
  */
 
 #include <common.h>
@@ -74,6 +76,7 @@ static short	TftpNumchars;	/* The number of hashes we printed      */
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
 #define STATE_OACK	5
+#define STATE_RECV_WRQ	6
 
 #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
 #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
@@ -241,6 +244,10 @@ TftpSend (void)
 			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
 		/*..falling..*/
 #endif
+
+#ifdef CONFIG_CMD_TFTPSRV
+	case STATE_RECV_WRQ:
+#endif
 	case STATE_DATA:
 		xp = pkt;
 		s = (ushort *)pkt;
@@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ &&
+#ifdef CONFIG_CMD_TFTPSRV
+	    TftpState != STATE_RECV_WRQ &&
+#endif
+	    src != TftpRemotePort)
 		return;
 
 	if (len < 2) {
@@ -307,12 +318,24 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 	switch (ntohs(proto)) {
 
 	case TFTP_RRQ:
-	case TFTP_WRQ:
 	case TFTP_ACK:
 		break;
 	default:
 		break;
 
+#ifdef CONFIG_CMD_TFTPSRV
+	case TFTP_WRQ:
+		debug("Got WRQ\n");
+		TftpRemoteIP = sip;
+		TftpRemotePort = src;
+		TftpOurPort = 1024 + (get_timer(0) % 3072);
+		TftpLastBlock = 0;
+		TftpBlockWrap = 0;
+		TftpBlockWrapOffset = 0;
+		TftpSend(); /* Send ACK(0) */
+		break;
+#endif
+
 	case TFTP_OACK:
 		debug("Got OACK: %s %s\n",
 			pkt,
@@ -383,7 +406,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ ||
+#ifdef CONFIG_CMD_TFTPSRV
+		    TftpState == STATE_RECV_WRQ ||
+#endif
+		    TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -518,7 +545,10 @@ TftpTimeout (void)
 	} else {
 		puts ("T ");
 		NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
-		TftpSend ();
+#ifdef CONFIG_CMD_TFTPSRV
+		if (TftpState != STATE_RECV_WRQ)
+#endif
+			TftpSend();
 	}
 }
 
@@ -639,6 +669,40 @@ TftpStart (void)
 	TftpSend ();
 }
 
+#ifdef CONFIG_CMD_TFTPSRV
+void
+TftpStartServer(void)
+{
+	tftp_filename[0] = 0;
+
+#if defined(CONFIG_NET_MULTI)
+	printf("Using %s device\n", eth_get_name());
+#endif
+	printf("Listening for TFTP transfer on %pI4\n", &NetOurIP);
+	printf("Load address: 0x%lx\n", load_addr);
+
+	puts("Loading: *\b");
+
+	TftpTimeoutCountMax = TIMEOUT_COUNT;
+	TftpTimeoutCount = 0;
+	TftpTimeoutMSecs = TIMEOUT;
+	NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
+
+	/* Revert TftpBlkSize to dflt */
+	TftpBlkSize = TFTP_BLOCK_SIZE;
+	TftpBlock = 0;
+	TftpOurPort = WELL_KNOWN_PORT;
+
+#ifdef CONFIG_TFTP_TSIZE
+	TftpTsize = 0;
+	TftpNumchars = 0;
+#endif
+
+	TftpState = STATE_RECV_WRQ;
+	NetSetHandler(TftpHandler);
+}
+#endif /* CONFIG_CMD_TFTPSRV */
+
 #ifdef CONFIG_MCAST_TFTP
 /* Credits: atftp project.
  */
diff --git a/net/tftp.h b/net/tftp.h
index e3dfb26..3abdf7b 100644
--- a/net/tftp.h
+++ b/net/tftp.h
@@ -2,6 +2,8 @@
  *	LiMon - BOOTP/TFTP.
  *
  *	Copyright 1994, 1995, 2000 Neil Russell.
+ *	Copyright 2011 Comelit Group SpA
+ *	               Luca Ceresoli <luca.ceresoli@comelit.it>
  *	(See License)
  */
 
@@ -16,6 +18,10 @@
 /* tftp.c */
 extern void	TftpStart (void);	/* Begin TFTP get */
 
+#ifdef CONFIG_CMD_TFTPSRV
+extern void	TftpStartServer(void);	/* Wait for incoming TFTP put */
+#endif
+
 /**********************************************************************/
 
 #endif /* __TFTP_H__ */
-- 
1.7.1

[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Detlev Zundel]

Hi Luca,

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2: none.

Only one comment below

>
>  net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  net/tftp.h |    6 +++++
>  2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/net/tftp.c b/net/tftp.c
> index e166a63..87eb0b8 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -2,6 +2,8 @@
>   *	Copyright 1994, 1995, 2000 Neil Russell.
>   *	(See License)
>   *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
> + *	Copyright 2011 Comelit Group SpA,
> + *	               Luca Ceresoli <luca.ceresoli@comelit.it>
>   */
>  
>  #include <common.h>
> @@ -74,6 +76,7 @@ static short	TftpNumchars;	/* The number of hashes we printed      */
>  #define STATE_TOO_LARGE	3
>  #define STATE_BAD_MAGIC	4
>  #define STATE_OACK	5
> +#define STATE_RECV_WRQ	6
>  
>  #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
>  #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
> @@ -241,6 +244,10 @@ TftpSend (void)
>  			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
>  		/*..falling..*/
>  #endif
> +
> +#ifdef CONFIG_CMD_TFTPSRV
> +	case STATE_RECV_WRQ:
> +#endif
>  	case STATE_DATA:
>  		xp = pkt;
>  		s = (ushort *)pkt;
> @@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>  #endif
>  		return;
>  	}
> -	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
> +	if (TftpState != STATE_SEND_RRQ &&
> +#ifdef CONFIG_CMD_TFTPSRV
> +	    TftpState != STATE_RECV_WRQ &&
> +#endif
> +	    src != TftpRemotePort)
>  		return;

Hm, I have to admit that I do not really like adding so many ifdefs into
the tftp code and even into the state machine code.  Can you give me a
number on how much larger u-boot gets on your platform with and without
the server support?  If this is not too much, maybe we should include
support always.  If it is too much, maybe we can at least keep the state
machine without ifdefs - if I see it correctly, this will only add at
maximum a few bytes and STATE_RECW_WRQ will simply never be entered,
correct?

Cheers
  Detlev

-- 
Zivilisation ist der Zaubertrick, der uns unsere wahre Natur verbirgt.
                                            -- Salman Rushdie
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Luca Ceresoli]

Detlev Zundel wrote:

> Hi Luca,
>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>> Cc: Wolfgang Denk<wd@denx.de>
>> ---
>> Changes in v2: none.
> Only one comment below
>
>>   net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   net/tftp.h |    6 +++++
>>   2 files changed, 74 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/tftp.c b/net/tftp.c
>> index e166a63..87eb0b8 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -2,6 +2,8 @@
>>    *	Copyright 1994, 1995, 2000 Neil Russell.
>>    *	(See License)
>>    *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
>> + *	Copyright 2011 Comelit Group SpA,
>> + *	               Luca Ceresoli<luca.ceresoli@comelit.it>
>>    */
>>
>>   #include<common.h>
>> @@ -74,6 +76,7 @@ static short	TftpNumchars;	/* The number of hashes we printed      */
>>   #define STATE_TOO_LARGE	3
>>   #define STATE_BAD_MAGIC	4
>>   #define STATE_OACK	5
>> +#define STATE_RECV_WRQ	6
>>
>>   #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
>>   #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
>> @@ -241,6 +244,10 @@ TftpSend (void)
>>   			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
>>   		/*..falling..*/
>>   #endif
>> +
>> +#ifdef CONFIG_CMD_TFTPSRV
>> +	case STATE_RECV_WRQ:
>> +#endif
>>   	case STATE_DATA:
>>   		xp = pkt;
>>   		s = (ushort *)pkt;
>> @@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>>   #endif
>>   		return;
>>   	}
>> -	if (TftpState != STATE_SEND_RRQ&&  src != TftpRemotePort)
>> +	if (TftpState != STATE_SEND_RRQ&&
>> +#ifdef CONFIG_CMD_TFTPSRV
>> +	    TftpState != STATE_RECV_WRQ&&
>> +#endif
>> +	    src != TftpRemotePort)
>>   		return;
> Hm, I have to admit that I do not really like adding so many ifdefs into
> the tftp code and even into the state machine code.  Can you give me a
> number on how much larger u-boot gets on your platform with and without
> the server support?  If this is not too much, maybe we should include
> support always.  If it is too much, maybe we can at least keep the state
> machine without ifdefs - if I see it correctly, this will only add at
> maximum a few bytes and STATE_RECW_WRQ will simply never be entered,
> correct?
Here are my measurements for the dig297 board (ARMV7 OMAP3):

    text    data     bss     dec     hex
  357085    8684  214264  580033   8d9c1 TFTPSRV on
  356327    8660  214264  579251   8d6b3 TFTPSRV off, without #ifs
  356355    8660  214268  579283   8d6d3 TFTPSRV off, with #ifs (as in PATCH v2)

The TFTP server adds 730 bytes to the text, so I guess it's worth to keep it
optional.

To my big surprise, removing most bad-looking #ifs (all of those that save just
one line) I got a *smaller* U-Boot! I repeated the test three times to be extra
sure, the figures are correct: without the #ifs we save a few bytes.

Obvious enough, I'm going to remove the #ifs.

Luca

[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Detlev Zundel]

Hi Luca,

[...]

>> Hm, I have to admit that I do not really like adding so many ifdefs into
>> the tftp code and even into the state machine code.  Can you give me a
>> number on how much larger u-boot gets on your platform with and without
>> the server support?  If this is not too much, maybe we should include
>> support always.  If it is too much, maybe we can at least keep the state
>> machine without ifdefs - if I see it correctly, this will only add at
>> maximum a few bytes and STATE_RECW_WRQ will simply never be entered,
>> correct?
> Here are my measurements for the dig297 board (ARMV7 OMAP3):
>
>    text    data     bss     dec     hex
>  357085    8684  214264  580033   8d9c1 TFTPSRV on
>  356327    8660  214264  579251   8d6b3 TFTPSRV off, without #ifs
>  356355    8660  214268  579283   8d6d3 TFTPSRV off, with #ifs (as in PATCH v2)
>
> The TFTP server adds 730 bytes to the text, so I guess it's worth to
> keep it optional.

Ok, thanks for the number.

> To my big surprise, removing most bad-looking #ifs (all of those that
> save just one line) I got a *smaller* U-Boot! I repeated the test
> three times to be extra sure, the figures are correct: without the
> #ifs we save a few bytes.
>
> Obvious enough, I'm going to remove the #ifs.

:) Another instance of the difficulty to predict modern compilers.

Cheers
  Detlev

-- 
But in terms of creative  information, information that people can use
or enjoy, and that will be  used and enjoyed more  the more people who
have it, always we should encourage the copying.
	                                    -- Richard M. Stallman
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes in v2: none.

 README           |    1 +
 common/cmd_net.c |   14 ++++++++++++++
 include/net.h    |    3 ++-
 net/net.c        |   10 ++++++++--
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/README b/README
index b9b0fcb..83dcb2a 100644
--- a/README
+++ b/README
@@ -685,6 +685,7 @@ The following options need to be configured:
 					  (requires CONFIG_CMD_MEMORY)
 		CONFIG_CMD_SOURCE	  "source" command Support
 		CONFIG_CMD_SPI		* SPI serial bus support
+		CONFIG_CMD_TFTPSRV	* TFTP transfer in server mode
 		CONFIG_CMD_USB		* USB support
 		CONFIG_CMD_VFD		* VFD support (TRAB)
 		CONFIG_CMD_CDP		* Cisco Discover Protocol support
diff --git a/common/cmd_net.c b/common/cmd_net.c
index 8c6f5c8..053162a 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -52,6 +52,20 @@ U_BOOT_CMD(
 	"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 
+#ifdef CONFIG_CMD_TFTPSRV
+int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return netboot_common(TFTPSRV, cmdtp, argc, argv);
+}
+
+U_BOOT_CMD(
+	tftpsrv,	2,	1,	do_tftpsrv,
+	"boot image via network acting as a TFTP server",
+	"[loadAddress]"
+);
+#endif
+
+
 #ifdef CONFIG_CMD_RARP
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
diff --git a/include/net.h b/include/net.h
index 01f7159..018a744 100644
--- a/include/net.h
+++ b/include/net.h
@@ -364,7 +364,8 @@ extern int		NetState;		/* Network loop state		*/
 extern int		NetRestartWrap;		/* Tried all network devices	*/
 #endif
 
-typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP } proto_t;
+typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
+	       TFTPSRV } proto_t;
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
diff --git a/net/net.c b/net/net.c
index 132f99b..17eb06f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -388,7 +388,11 @@ restart:
 			/* always use ARP to get server ethernet address */
 			TftpStart();
 			break;
-
+#ifdef CONFIG_CMD_TFTPSRV
+		case TFTPSRV:
+			TftpStartServer();
+			break;
+#endif
 #if defined(CONFIG_CMD_DHCP)
 		case DHCP:
 			BootpTry = 0;
@@ -1731,7 +1735,9 @@ static int net_check_prereq (proto_t protocol)
 #if defined(CONFIG_CMD_PING) || defined(CONFIG_CMD_SNTP)
     common:
 #endif
-
+#ifdef CONFIG_CMD_TFTPSRV
+	case TFTPSRV:
+#endif
 		if (NetOurIP == 0) {
 			puts ("*** ERROR: `ipaddr' not set\n");
 			return (1);
-- 
1.7.1

[U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command

[Detlev Zundel]

Hi Luca,

> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
> Changes in v2: none.

Acked-by: Detlev Zundel <dzu@denx.de>

-- 
I have always observed that the pretensions of all people are in
exact inverse ratio to their merits; this is one of the axioms of
morals.                            -- Joseph Lagrange
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command

[Mike Frysinger]

On Monday, April 18, 2011 12:19:54 Luca Ceresoli wrote:
> +#ifdef CONFIG_CMD_TFTPSRV
> +int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

should be static

> +U_BOOT_CMD(
> +	tftpsrv,	2,	1,	do_tftpsrv,
> +	"boot image via network acting as a TFTP server",
> +	"[loadAddress]"
> +);

the desc is a bit confusing.  how about:
	act as a TFTP server and boot the first received file
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110430/74a5d8b0/attachment.pgp 

[U-Boot] [PATCH v2 6/6] TFTP: add tftpsrv command

[Mike Frysinger]

On Mon, Apr 18, 2011 at 12:19, Luca Ceresoli wrote:
> +int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

static
-mike

[U-Boot] [PATCH 1/6] README: remove spurious line

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
 README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/README b/README
index 4917e26..b9b0fcb 100644
--- a/README
+++ b/README
@@ -1299,7 +1299,6 @@ The following options need to be configured:
 		driver in use must provide a function: mcast() to join/leave a
 		multicast group.
 
-		CONFIG_BOOTP_RANDOM_DELAY
 - BOOTP Recovery Mode:
 		CONFIG_BOOTP_RANDOM_DELAY
 
-- 
1.7.1

[U-Boot] [PATCH 1/6] README: remove spurious line

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1302796377-3321-2-git-send-email-luca.ceresoli@comelit.it> you wrote:
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  README |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To be sure of hitting the target, shoot first and, whatever you  hit,
call it the target.

[U-Boot] [PATCH 2/6] NET: pass source IP address to packet handlers

[Luca Ceresoli]

This is needed for the upcoming TFTP server implementation.

This also simplifies PingHandler() and fixes rxhand_f documentation.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
 drivers/net/netconsole.c |    5 +++--
 include/net.h            |   15 ++++++++++-----
 net/bootp.c              |    9 ++++++---
 net/dns.c                |    2 +-
 net/net.c                |   25 ++++++++++++++-----------
 net/nfs.c                |    2 +-
 net/rarp.c               |    3 ++-
 net/sntp.c               |    3 ++-
 net/tftp.c               |    3 ++-
 9 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index e27bb3e..ed753d1 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -40,13 +40,14 @@ static short nc_port;			/* source/target port */
 static const char *output_packet;	/* used by first send udp */
 static int output_packet_len = 0;
 
-static void nc_wait_arp_handler (uchar * pkt, unsigned dest, unsigned src,
+static void nc_wait_arp_handler (uchar * pkt, unsigned dest,
+				 IPaddr_t sip, unsigned src,
 				 unsigned len)
 {
 	NetState = NETLOOP_SUCCESS;	/* got arp reply - quit net loop */
 }
 
-static void nc_handler (uchar * pkt, unsigned dest, unsigned src,
+static void nc_handler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			unsigned len)
 {
 	if (input_size)
diff --git a/include/net.h b/include/net.h
index 95ef8ab..01f7159 100644
--- a/include/net.h
+++ b/include/net.h
@@ -72,12 +72,17 @@
 typedef ulong		IPaddr_t;
 
 
-/*
- * The current receive packet handler.  Called with a pointer to the
- * application packet, and a protocol type (PORT_BOOTPC or PORT_TFTP).
- * All other packets are dealt with without calling the handler.
+/**
+ * An incoming packet handler.
+ * @param pkt    pointer to the application packet
+ * @param dport  destination UDP port
+ * @param sip    source IP address
+ * @param sport  source UDP port
+ * @param len    packet length
  */
-typedef void	rxhand_f(uchar *, unsigned, unsigned, unsigned);
+typedef void rxhand_f(uchar *pkt, unsigned dport,
+		      IPaddr_t sip, unsigned sport,
+		      unsigned len);
 
 /*
  *	A timeout handler.  Called after time interval has expired.
diff --git a/net/bootp.c b/net/bootp.c
index 87b027e..eafaae2 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -44,7 +44,8 @@ ulong		seed1, seed2;
 dhcp_state_t dhcp_state = INIT;
 unsigned long dhcp_leasetime = 0;
 IPaddr_t NetDHCPServerIP = 0;
-static void DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len);
+static void DhcpHandler(uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+			unsigned len);
 
 /* For Debug */
 #if 0
@@ -282,7 +283,8 @@ static void BootpVendorProcess (u8 * ext, int size)
  *	Handle a BOOTP received packet.
  */
 static void
-BootpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+BootpHandler(uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	     unsigned len)
 {
 	Bootp_t *bp;
 	char	*s;
@@ -858,7 +860,8 @@ static void DhcpSendRequestPkt(Bootp_t *bp_offer)
  *	Handle DHCP received packets.
  */
 static void
-DhcpHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+DhcpHandler(uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	    unsigned len)
 {
 	Bootp_t *bp = (Bootp_t *)pkt;
 
diff --git a/net/dns.c b/net/dns.c
index bb3e3f5..b51d1bd 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -101,7 +101,7 @@ DnsTimeout(void)
 }
 
 static void
-DnsHandler(uchar *pkt, unsigned dest, unsigned src, unsigned len)
+DnsHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 {
 	struct header *header;
 	const unsigned char *p, *e, *s;
diff --git a/net/net.c b/net/net.c
index a609632..79afd8b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -555,7 +555,8 @@ startAgainTimeout(void)
 }
 
 static void
-startAgainHandler(uchar * pkt, unsigned dest, unsigned src, unsigned len)
+startAgainHandler(uchar * pkt, unsigned dest, IPaddr_t sip,
+		  unsigned src, unsigned len)
 {
 	/* Totally ignore the packet */
 }
@@ -752,13 +753,10 @@ PingTimeout (void)
 }
 
 static void
-PingHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+PingHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	     unsigned len)
 {
-	IPaddr_t tmp;
-	volatile IP_t *ip = (volatile IP_t *)pkt;
-
-	tmp = NetReadIP((void *)&ip->ip_src);
-	if (tmp != NetPingIP)
+	if (sip != NetPingIP)
 		return;
 
 	NetState = NETLOOP_SUCCESS;
@@ -990,7 +988,8 @@ CDPTimeout (void)
 }
 
 static void
-CDPDummyHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+CDPDummyHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+		 unsigned len)
 {
 	/* nothing */
 }
@@ -1304,6 +1303,7 @@ NetReceive(volatile uchar * inpkt, int len)
 	IP_t	*ip;
 	ARP_t	*arp;
 	IPaddr_t tmp;
+	IPaddr_t src_ip;
 	int	x;
 	uchar *pkt;
 #if defined(CONFIG_CMD_CDP)
@@ -1477,7 +1477,7 @@ NetReceive(volatile uchar * inpkt, int len)
 				memcpy(NetArpWaitPacketMAC, &arp->ar_data[0], 6);
 
 #ifdef CONFIG_NETCONSOLE
-				(*packetHandler)(0,0,0,0);
+				(*packetHandler)(0,0,0,0,0);
 #endif
 				/* modify header, and transmit it */
 				memcpy(((Ethernet_t *)NetArpWaitTxPacket)->et_dest, NetArpWaitPacketMAC, 6);
@@ -1517,7 +1517,7 @@ NetReceive(volatile uchar * inpkt, int len)
 				NetCopyIP(&NetServerIP, &arp->ar_data[ 6]);
 			memcpy (NetServerEther, &arp->ar_data[ 0], 6);
 
-			(*packetHandler)(0,0,0,0);
+			(*packetHandler)(0,0,0,0,0);
 		}
 		break;
 #endif
@@ -1557,6 +1557,8 @@ NetReceive(volatile uchar * inpkt, int len)
 #endif
 			return;
 		}
+		/* Read source IP address for later use */
+		src_ip = NetReadIP(&ip->ip_src);
 		/*
 		 * The function returns the unchanged packet if it's not
 		 * a fragment, and either the complete packet or NULL if
@@ -1596,7 +1598,7 @@ NetReceive(volatile uchar * inpkt, int len)
 				 *	IP header OK.  Pass the packet to the current handler.
 				 */
 				/* XXX point to ip packet */
-				(*packetHandler)((uchar *)ip, 0, 0, 0);
+				(*packetHandler)((uchar *)ip, 0, src_ip, 0, 0);
 				return;
 			case ICMP_ECHO_REQUEST:
 				debug("Got ICMP ECHO REQUEST, return %d bytes \n",
@@ -1678,6 +1680,7 @@ NetReceive(volatile uchar * inpkt, int len)
 		 */
 		(*packetHandler)((uchar *)ip +IP_HDR_SIZE,
 						ntohs(ip->udp_dst),
+						src_ip,
 						ntohs(ip->udp_src),
 						ntohs(ip->udp_len) - 8);
 		break;
diff --git a/net/nfs.c b/net/nfs.c
index d11bb4c..23e43e1 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -580,7 +580,7 @@ NfsTimeout (void)
 }
 
 static void
-NfsHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
+NfsHandler (uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, unsigned len)
 {
 	int rlen;
 
diff --git a/net/rarp.c b/net/rarp.c
index 9444c03..d1c83e0 100644
--- a/net/rarp.c
+++ b/net/rarp.c
@@ -43,7 +43,8 @@ int		RarpTry;
  *	Handle a RARP received packet.
  */
 static void
-RarpHandler(uchar * dummi0, unsigned dummi1, unsigned dummi2, unsigned dummi3)
+RarpHandler(uchar * dummi0, unsigned dummi1, IPaddr_t sip, unsigned dummi2,
+	    unsigned dummi3)
 {
 	char *s;
 	debug("Got good RARP\n");
diff --git a/net/sntp.c b/net/sntp.c
index 76c10ec..7342cfd 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -48,7 +48,8 @@ SntpTimeout (void)
 }
 
 static void
-SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
+SntpHandler (uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	     unsigned len)
 {
 	struct sntp_pkt_t *rpktp = (struct sntp_pkt_t *)pkt;
 	struct rtc_time tm;
diff --git a/net/tftp.c b/net/tftp.c
index ed559b7..8e6df0a 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -278,7 +278,8 @@ TftpSend (void)
 
 
 static void
-TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
+	     unsigned len)
 {
 	ushort proto;
 	ushort *s;
-- 
1.7.1

[U-Boot] [PATCH 2/6] NET: pass source IP address to packet handlers

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1302796377-3321-3-git-send-email-luca.ceresoli@comelit.it> you wrote:
> This is needed for the upcoming TFTP server implementation.
> 
> This also simplifies PingHandler() and fixes rxhand_f documentation.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  drivers/net/netconsole.c |    5 +++--
>  include/net.h            |   15 ++++++++++-----
>  net/bootp.c              |    9 ++++++---
>  net/dns.c                |    2 +-
>  net/net.c                |   25 ++++++++++++++-----------
>  net/nfs.c                |    2 +-
>  net/rarp.c               |    3 ++-
>  net/sntp.c               |    3 ++-
>  net/tftp.c               |    3 ++-
>  9 files changed, 41 insertions(+), 26 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Power is danger.
	-- The Centurion, "Balance of Terror", stardate 1709.2

[U-Boot] [PATCH 2/6] NET: pass source IP address to packet handlers

[Wolfgang Denk]

Dear Luca Ceresoli,

In message <1302796377-3321-3-git-send-email-luca.ceresoli@comelit.it> you wrote:
> This is needed for the upcoming TFTP server implementation.
> 
> This also simplifies PingHandler() and fixes rxhand_f documentation.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
> Cc: Wolfgang Denk <wd@denx.de>

Undone this and previous one as there is a V2 version of this patch.

I wish you would thread yout postings!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You got to learn three things. What's  real,  what's  not  real,  and
what's the difference."           - Terry Pratchett, _Witches Abroad_

[U-Boot] [PATCH 2/6] NET: pass source IP address to packet handlers

[Luca Ceresoli]

Hi Wolfgang,

Wolfgang Denk wrote:

> Dear Luca Ceresoli,
>
> In message<1302796377-3321-3-git-send-email-luca.ceresoli@comelit.it>  you wrote:
>> This is needed for the upcoming TFTP server implementation.
>>
>> This also simplifies PingHandler() and fixes rxhand_f documentation.
>>
>> Signed-off-by: Luca Ceresoli<luca.ceresoli@comelit.it>
>> Cc: Wolfgang Denk<wd@denx.de>
> Undone this and previous one as there is a V2 version of this patch.
>
> I wish you would thread yout postings!
I'm sorry for your wasted time, but it's not clear (to me at least...) how
postings should be threaded in the U-Boot ml.

The Wiki says:

>  Make sure that your mailer adds or keeps correct
>  |"In-reply-to:"|  and|"References:"|  headers, so threading of
>  messages is working and everybody can see that the new message
>  refers to some older posting of the same topic.
>
>  Uncommented and un-threaded repostings are extremely annoying and
>  time-consuming, as we have to try to remember if anything similar has
>  been posted before, look up the old threads, and then manually
>  compare if anything has been changed, or what.

'correct|"In-reply-to:"|  and|"References:"|' can have many interpretations.

On the ML I see varying ones, and I even see submissions with NO THREADING
AT ALL: the first submit is in a thread, v2 starts a new thread, v3 yet
another one etc.

MY humble interpretation of the Wiki wording is this:

- [PATCH 0/2]
   |
   +->  [PATCH 1/2]
   +->  [PATCH 2/2]
   |   +->  Re: [PATCH 2/2] reviewers comments to patch 2/2
   |   +->  Re: [PATCH 2/2] more reviewers comments to patch 2/2
   |
   +->  Re: [PATCH 0/2] reviewers comments to the patchset (0/2)
   |   +->  Re: [PATCH 0/2] discussion...
   |
   +->  [PATCH v2 0/2] resubmit (In-Reply-To: the first submit)
       |
       +->  [PATCH v2 1/2]
       +->  [PATCH v2 2/2]
       |   ... more discussion - not shown...
       |
       +->  [PATCH v3 0/2] resubmit (In-Reply-To: the first submit)
           |
           +->  [PATCH v3 1/2]
           +->  [PATCH v3 2/2]

What's wrong with this?
Please state in absolutely unambiguous detail what the correct threading is,
and please docuemnt it in the wiki for everybody.

My postings look correctly threaded according to the above interpretation,
as you can see for example on the Gmane threaded interface.

Luca

[U-Boot] [PATCH 3/6] TFTP: rename "server" to "remote"

[Luca Ceresoli]

With the upcoming TFTP server implementation, the remote node can be
either a client or a server, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
 net/tftp.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 8e6df0a..42469e7 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -55,8 +55,8 @@ enum {
 	TFTP_ERR_FILE_ALREADY_EXISTS = 6,
 };
 
-static IPaddr_t TftpServerIP;
-static int	TftpServerPort;		/* The UDP port at their end		*/
+static IPaddr_t TftpRemoteIP;
+static int	TftpRemotePort;		/* The UDP port at their end		*/
 static int	TftpOurPort;		/* The UDP port at our end		*/
 static int	TftpTimeoutCount;
 static ulong	TftpBlock;		/* packet sequence number		*/
@@ -273,7 +273,8 @@ TftpSend (void)
 		break;
 	}
 
-	NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len);
+	NetSendUDPPacket(NetServerEther, TftpRemoteIP, TftpRemotePort,
+			 TftpOurPort, len);
 }
 
 
@@ -292,9 +293,8 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpServerPort) {
+	if (TftpState != STATE_RRQ && src != TftpRemotePort)
 		return;
-	}
 
 	if (len < 2) {
 		return;
@@ -318,7 +318,7 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			pkt,
 			pkt + strlen((char *)pkt) + 1);
 		TftpState = STATE_OACK;
-		TftpServerPort = src;
+		TftpRemotePort = src;
 		/*
 		 * Check for 'blksize' option.
 		 * Careful: "i" is signed, "len" is unsigned, thus
@@ -386,7 +386,7 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
-			TftpServerPort = src;
+			TftpRemotePort = src;
 			TftpLastBlock = 0;
 			TftpBlockWrap = 0;
 			TftpBlockWrapOffset = 0;
@@ -421,7 +421,7 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 
 		/*
 		 *	Acknoledge the block just received, which will prompt
-		 *	the server for the next one.
+		 *	the remote for the next one.
 		 */
 #ifdef CONFIG_MCAST_TFTP
 		/* if I am the MasterClient, actively calculate what my next
@@ -548,7 +548,7 @@ TftpStart (void)
 	debug("TFTP blocksize = %i, timeout = %ld ms\n",
 		TftpBlkSizeOption, TftpTimeoutMSecs);
 
-	TftpServerIP = NetServerIP;
+	TftpRemoteIP = NetServerIP;
 	if (BootFile[0] == '\0') {
 		sprintf(default_filename, "%02lX%02lX%02lX%02lX.img",
 			NetOurIP & 0xFF,
@@ -568,7 +568,7 @@ TftpStart (void)
 			strncpy(tftp_filename, BootFile, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		} else {
-			TftpServerIP = string_to_ip (BootFile);
+			TftpRemoteIP = string_to_ip(BootFile);
 			strncpy(tftp_filename, p + 1, MAX_LEN);
 			tftp_filename[MAX_LEN-1] = 0;
 		}
@@ -578,12 +578,12 @@ TftpStart (void)
 	printf ("Using %s device\n", eth_get_name());
 #endif
 	printf("TFTP from server %pI4"
-		"; our IP address is %pI4", &TftpServerIP, &NetOurIP);
+		"; our IP address is %pI4", &TftpRemoteIP, &NetOurIP);
 
 	/* Check if we need to send across this subnet */
 	if (NetOurGatewayIP && NetOurSubnetMask) {
 	    IPaddr_t OurNet	= NetOurIP    & NetOurSubnetMask;
-	    IPaddr_t ServerNet	= TftpServerIP & NetOurSubnetMask;
+	    IPaddr_t ServerNet	= TftpRemoteIP & NetOurSubnetMask;
 
 	    if (OurNet != ServerNet)
 		printf("; sending through gateway %pI4", &NetOurGatewayIP);
@@ -608,7 +608,7 @@ TftpStart (void)
 	NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
 	NetSetHandler (TftpHandler);
 
-	TftpServerPort = WELL_KNOWN_PORT;
+	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
 	TftpState = STATE_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
@@ -616,7 +616,7 @@ TftpStart (void)
 
 #ifdef CONFIG_TFTP_PORT
 	if ((ep = getenv("tftpdstp")) != NULL) {
-		TftpServerPort = simple_strtol(ep, NULL, 10);
+		TftpRemotePort = simple_strtol(ep, NULL, 10);
 	}
 	if ((ep = getenv("tftpsrcp")) != NULL) {
 		TftpOurPort= simple_strtol(ep, NULL, 10);
-- 
1.7.1

[U-Boot] [PATCH 4/6] TFTP: rename STATE_RRQ to STATE_SEND_RRQ

[Luca Ceresoli]

With the upcoming TFTP server implementation, requests can be either
outgoing or incoming, so avoid ambiguities.

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
 net/tftp.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 42469e7..2c96358 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -69,7 +69,7 @@ static int	TftpTsize;		/* The file size reported by the server */
 static short	TftpNumchars;		/* The number of hashes we printed      */
 #endif
 
-#define STATE_RRQ	1
+#define STATE_SEND_RRQ	1
 #define STATE_DATA	2
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
@@ -200,7 +200,7 @@ TftpSend (void)
 
 	switch (TftpState) {
 
-	case STATE_RRQ:
+	case STATE_SEND_RRQ:
 		xp = pkt;
 		s = (ushort *)pkt;
 		*s++ = htons(TFTP_RRQ);
@@ -293,7 +293,7 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
 		return;
 
 	if (len < 2) {
@@ -380,10 +380,10 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			}
 		}
 
-		if (TftpState == STATE_RRQ)
+		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -610,7 +610,7 @@ TftpStart (void)
 
 	TftpRemotePort = WELL_KNOWN_PORT;
 	TftpTimeoutCount = 0;
-	TftpState = STATE_RRQ;
+	TftpState = STATE_SEND_RRQ;
 	/* Use a pseudo-random port unless a specific port is set */
 	TftpOurPort = 1024 + (get_timer(0) % 3072);
 
-- 
1.7.1

[U-Boot] [PATCH 5/6] TFTP: net/tftp.c: add server mode receive

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---

A note on the style of this patch.
There are many hunks like this:

> -	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
> +	if (TftpState != STATE_SEND_RRQ &&
> +#ifdef CONFIG_CMD_TFTPSRV
> +	    TftpState != STATE_RECV_WRQ &&
> +#endif
> +	    src != TftpRemotePort)

where I put a comparison between TftpState and STATE_RECV_WRQ in #ifdefs.

The #ifdef is not necessary, as both TftpState and STATE_RECV_WRQ are defined
even when CONFIG_CMD_TFTPSRV is off. Simply, TftpState can never be equal to
STATE_RECV_WRQ.

I've put the #ifdefs with the intention of optimizing the code as much as
possible, avoiding a comparison that would be always true (or always false).
I understand that this makes the code a lot more difficult to read.

Should I instead remote the #ifdefs and make the code more readable (but less
optimized)?

Thanks,
Luca


 net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 net/tftp.h |    6 +++++
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index 2c96358..c586a9f 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -2,6 +2,8 @@
  *	Copyright 1994, 1995, 2000 Neil Russell.
  *	(See License)
  *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
+ *	Copyright 2011 Comelit Group SpA,
+ *	               Luca Ceresoli <luca.ceresoli@comelit.it>
  */
 
 #include <common.h>
@@ -74,6 +76,7 @@ static short	TftpNumchars;		/* The number of hashes we printed      */
 #define STATE_TOO_LARGE	3
 #define STATE_BAD_MAGIC	4
 #define STATE_OACK	5
+#define STATE_RECV_WRQ	6
 
 #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
 #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
@@ -241,6 +244,10 @@ TftpSend (void)
 			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
 		/*..falling..*/
 #endif
+
+#ifdef CONFIG_CMD_TFTPSRV
+	case STATE_RECV_WRQ:
+#endif
 	case STATE_DATA:
 		xp = pkt;
 		s = (ushort *)pkt;
@@ -293,7 +300,11 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 #endif
 		return;
 	}
-	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
+	if (TftpState != STATE_SEND_RRQ &&
+#ifdef CONFIG_CMD_TFTPSRV
+	    TftpState != STATE_RECV_WRQ &&
+#endif
+	    src != TftpRemotePort)
 		return;
 
 	if (len < 2) {
@@ -307,12 +318,24 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 	switch (ntohs(proto)) {
 
 	case TFTP_RRQ:
-	case TFTP_WRQ:
 	case TFTP_ACK:
 		break;
 	default:
 		break;
 
+#ifdef CONFIG_CMD_TFTPSRV
+	case TFTP_WRQ:
+		debug("Got WRQ\n");
+		TftpRemoteIP = sip;
+		TftpRemotePort = src;
+		TftpOurPort = 1024 + (get_timer(0) % 3072);
+		TftpLastBlock = 0;
+		TftpBlockWrap = 0;
+		TftpBlockWrapOffset = 0;
+		TftpSend(); /* Send ACK(0) */
+		break;
+#endif
+
 	case TFTP_OACK:
 		debug("Got OACK: %s %s\n",
 			pkt,
@@ -383,7 +406,11 @@ TftpHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src,
 		if (TftpState == STATE_SEND_RRQ)
 			debug("Server did not acknowledge timeout option!\n");
 
-		if (TftpState == STATE_SEND_RRQ || TftpState == STATE_OACK) {
+		if (TftpState == STATE_SEND_RRQ ||
+#ifdef CONFIG_CMD_TFTPSRV
+		    TftpState == STATE_RECV_WRQ ||
+#endif
+		    TftpState == STATE_OACK) {
 			/* first block received */
 			TftpState = STATE_DATA;
 			TftpRemotePort = src;
@@ -518,7 +545,10 @@ TftpTimeout (void)
 	} else {
 		puts ("T ");
 		NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
-		TftpSend ();
+#ifdef CONFIG_CMD_TFTPSRV
+		if (TftpState != STATE_RECV_WRQ)
+#endif
+			TftpSend();
 	}
 }
 
@@ -639,6 +669,40 @@ TftpStart (void)
 	TftpSend ();
 }
 
+#ifdef CONFIG_CMD_TFTPSRV
+void
+TftpStartServer(void)
+{
+	tftp_filename[0] = 0;
+
+#if defined(CONFIG_NET_MULTI)
+	printf("Using %s device\n", eth_get_name());
+#endif
+	printf("Listening for TFTP transfer on %pI4\n", &NetOurIP);
+	printf("Load address: 0x%lx\n", load_addr);
+
+	puts("Loading: *\b");
+
+	TftpTimeoutCountMax = TIMEOUT_COUNT;
+	TftpTimeoutCount = 0;
+	TftpTimeoutMSecs = TIMEOUT;
+	NetSetTimeout(TftpTimeoutMSecs, TftpTimeout);
+
+	/* Revert TftpBlkSize to dflt */
+	TftpBlkSize = TFTP_BLOCK_SIZE;
+	TftpBlock = 0;
+	TftpOurPort = WELL_KNOWN_PORT;
+
+#ifdef CONFIG_TFTP_TSIZE
+	TftpTsize = 0;
+	TftpNumchars = 0;
+#endif
+
+	TftpState = STATE_RECV_WRQ;
+	NetSetHandler(TftpHandler);
+}
+#endif /* CONFIG_CMD_TFTPSRV */
+
 #ifdef CONFIG_MCAST_TFTP
 /* Credits: atftp project.
  */
diff --git a/net/tftp.h b/net/tftp.h
index e3dfb26..3abdf7b 100644
--- a/net/tftp.h
+++ b/net/tftp.h
@@ -2,6 +2,8 @@
  *	LiMon - BOOTP/TFTP.
  *
  *	Copyright 1994, 1995, 2000 Neil Russell.
+ *	Copyright 2011 Comelit Group SpA
+ *	               Luca Ceresoli <luca.ceresoli@comelit.it>
  *	(See License)
  */
 
@@ -16,6 +18,10 @@
 /* tftp.c */
 extern void	TftpStart (void);	/* Begin TFTP get */
 
+#ifdef CONFIG_CMD_TFTPSRV
+extern void	TftpStartServer(void);	/* Wait for incoming TFTP put */
+#endif
+
 /**********************************************************************/
 
 #endif /* __TFTP_H__ */
-- 
1.7.1

[U-Boot] [PATCH 6/6] TFTP: add tftpsrv command

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca.ceresoli@comelit.it>
Cc: Wolfgang Denk <wd@denx.de>
---
 README           |    1 +
 common/cmd_net.c |   14 ++++++++++++++
 include/net.h    |    3 ++-
 net/net.c        |   10 ++++++++--
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/README b/README
index b9b0fcb..83dcb2a 100644
--- a/README
+++ b/README
@@ -685,6 +685,7 @@ The following options need to be configured:
 					  (requires CONFIG_CMD_MEMORY)
 		CONFIG_CMD_SOURCE	  "source" command Support
 		CONFIG_CMD_SPI		* SPI serial bus support
+		CONFIG_CMD_TFTPSRV	* TFTP transfer in server mode
 		CONFIG_CMD_USB		* USB support
 		CONFIG_CMD_VFD		* VFD support (TRAB)
 		CONFIG_CMD_CDP		* Cisco Discover Protocol support
diff --git a/common/cmd_net.c b/common/cmd_net.c
index 8c6f5c8..053162a 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -52,6 +52,20 @@ U_BOOT_CMD(
 	"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 
+#ifdef CONFIG_CMD_TFTPSRV
+int do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return netboot_common(TFTPSRV, cmdtp, argc, argv);
+}
+
+U_BOOT_CMD(
+	tftpsrv,	2,	1,	do_tftpsrv,
+	"boot image via network acting as a TFTP server",
+	"[loadAddress]"
+);
+#endif
+
+
 #ifdef CONFIG_CMD_RARP
 int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
diff --git a/include/net.h b/include/net.h
index 01f7159..018a744 100644
--- a/include/net.h
+++ b/include/net.h
@@ -364,7 +364,8 @@ extern int		NetState;		/* Network loop state		*/
 extern int		NetRestartWrap;		/* Tried all network devices	*/
 #endif
 
-typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP } proto_t;
+typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
+	       TFTPSRV } proto_t;
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
diff --git a/net/net.c b/net/net.c
index 79afd8b..a9a2f8b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -388,7 +388,11 @@ restart:
 			/* always use ARP to get server ethernet address */
 			TftpStart();
 			break;
-
+#ifdef CONFIG_CMD_TFTPSRV
+		case TFTPSRV:
+			TftpStartServer();
+			break;
+#endif
 #if defined(CONFIG_CMD_DHCP)
 		case DHCP:
 			BootpTry = 0;
@@ -1730,7 +1734,9 @@ static int net_check_prereq (proto_t protocol)
 #if defined(CONFIG_CMD_PING) || defined(CONFIG_CMD_SNTP)
     common:
 #endif
-
+#ifdef CONFIG_CMD_TFTPSRV
+	case TFTPSRV:
+#endif
 		if (NetOurIP == 0) {
 			puts ("*** ERROR: `ipaddr' not set\n");
 			return (1);
-- 
1.7.1