Act as a TFTP server
Subsystems (3)
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
Dear Luca,
Quoted 4 lines · In message <[email protected]> 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.
Quoted 8 lines
From the user point of view, I would implement a new command, activated only
whenCONFIG_CMD_TFTPSRVis 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.
Quoted 1 line
Do you think my approach is correct?
Looks ok to me.
Quoted 1 line
Would this feature be considered for mainline inclusion?
Yes, of course.
Best regards,
Wolfgang Denk
Signature · 5 lines
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.
Wolfgang,
thanks for the feedback.
Quoted 6 lines · Wolfgang Denk
...
Quoted 5 lines
From the user point of view, I would implement a new command, activated only
whenCONFIG_CMD_TFTPSRVis defined:Usage: tftpsrv [<loadaddr>]
Quoted 7 lines
Quoted 3 lines
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.
Quoted 1 line
- 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
Dear Luca Ceresoli,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 6 lines
Quoted 6 lines
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.
Quoted 6 lines
Quoted 4 lines
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_COUNTtimes (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
Signature · 4 lines
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.
Il 30/04/2011 21:53, Wolfgang Denk ha scritto:
Quoted 12 lines
Dear Luca Ceresoli,
Quoted 9 lines · In message<[email protected]> you wrote:
Quoted 9 lines
Quoted 5 lines
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?
Quoted 8 lines
Quoted 2 lines
The TFTP client tries sending a RRQ packet
TIMEOUT_COUNTtimes (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
Dear Luca Ceresoli,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 2 lines
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
Signature · 5 lines
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
Quoted 9 lines · Wolfgang Denk
Dear Luca Ceresoli,
Quoted 5 lines · In message<[email protected]> you wrote:
Quoted 1 line
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
Quoted 6 lines · Luca Ceresoli
-
net/tftp.c: extend the TFTP state machine (TftpHandler()andTftpStart()) a newSTATE_WAITINGstate (it should be enough);
- the state machine would be different in the session setup: inSTATE_WAITINGhandle WRQ packets and select a new UDP port;
- after the setup phase, the state machine would converge on the currently-implementedSTATE_DATAfor 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
cover TFTP server
In reply to Luca CeresoliThis 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(-)
cover TFTP server
Hi Luca,
Le 14/04/2011 17:52, Luca Ceresoli a ?crit :
Quoted 1 line
A note about checkpatch.pl.
:)
Quoted 10 lines
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
(quoted diff) +2
-1-PingHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) +PingHandler (uchar * pkt, unsigned dest, IPaddr_t sip, unsigned src, +unsigned len) {
Quoted 7 lines
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,
Signature · 1 line
Albert.
v2 cover TFTP server
In reply to Luca CeresoliHi,
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:
Review
Quoted 3 lines
WARNING: consider using kstrto* in preference to
simple_strtol#125: FILE:net/tftp.c:619:(quoted diff) +1
-0+ TftpRemotePort = simple_strtol(ep, NULL, 10);
Which is Linux-only
Quoted 3 lines
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(-)
v2 cover TFTP server
Hi Luca,
Quoted 15 lines
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:
Review
Quoted 3 lines
WARNING: consider using kstrto* in preference to
simple_strtol#125: FILE:net/tftp.c:619:(quoted diff) +1
-0+ TftpRemotePort = simple_strtol(ep, NULL, 10);Which is Linux-only
Quoted 4 lines
Quoted 3 lines
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
Signature · 9 lines
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
v3 cover TFTP server
In reply to Luca CeresoliThis 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(-)
PATCH v3 1/5 TFTP: replace "server" with "remote" in local variable names
In reply to Luca CeresoliWith the upcoming TFTP server implementation, the remote node can be either a client or a server, so avoid ambiguities.
---
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(-)
net/tftp.c +14 -14
@@ -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 );
Signature · 1 line
1.7.1
PATCH v3 1/5 TFTP: replace "server" with "remote" in local variable names
Hi Luca,
Quoted 5 lines
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 <[email protected]>Cc Wolfgang Denk <[email protected]>
Cheers
Detlev
Signature · 5 lines
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
Wolfgang Denk
AppliedPATCH v3 1/5 TFTP: replace "server" with "remote" in local variable names
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 10 lines · In message <[email protected]> you…
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 <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2:
- fixed checkpatch issues.
Quoted 7 lines
Changes in v3:
- rebased on top of thenet/tftp.ccleanup;
- renamed also local variable ServerNet to RemoteNet inTftpStart();
- clarified commit message.net/tftp.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 7 lines
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 AndroidPATCH v3 2/5 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
In reply to Luca CeresoliWith the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
---
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(-)
net/tftp.c +6 -6
@@ -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 );
Signature · 1 line
1.7.1
PATCH v3 2/5 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
Quoted 5 lines · Luca Ceresoli
With the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Cheers
Detlev
Signature · 8 lines
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.deWolfgang Denk
AppliedPATCH v3 2/5 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 6 lines · In message <[email protected]> you…
With the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Quoted 8 lines
---
Changes in v2: none.Changes in v3:
- rebased on top of thenet/tftp.ccleanup.net/tftp.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
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
PATCH v3 3/5 TFTP: net/tftp.c: add server mode receive
In reply to Luca Ceresoli---
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(-)
net/tftp.c +58 -4
@@ -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 <[email protected]> */ #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. */
net/tftp.h +6 -0
@@ -2,6 +2,8 @@* LiMon - BOOTP/TFTP. * * Copyright 1994, 1995, 2000 Neil Russell. +* Copyright 2011 Comelit Group SpA +* Luca Ceresoli <[email protected]> * (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__ */
Signature · 1 line
1.7.1
PATCH v3 3/5 TFTP: net/tftp.c: add server mode receive
Hi Luca,
Quoted 2 lines
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Just for the future:
[...]
Quoted 14 lines
net/tftp.h +2
-0@@ -2,6 +2,8 @@* LiMon - BOOTP/TFTP. * * Copyright 1994, 1995, 2000 Neil Russell. +* Copyright 2011 Comelit Group SpA +* Luca Ceresoli <[email protected]> * (See License) */
net/tftp.h line 27
Quoted 10 lines
net/tftp.h +4
-0@@ -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
Signature · 8 lines
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 <[email protected]> -- 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
Wolfgang Denk
AppliedPATCH v3 3/5 TFTP: net/tftp.c: add server mode receive
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 12 lines · In message <[email protected]> you…
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.Changes in v3:
- rebased on top of thenet/tftp.ccleanup;
- 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.
Quoted 3 lines
net/tftp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- net/tftp.h | 6 +++++ 2 files changed, 64 insertions(+), 4 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 4 lines
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
PATCH v3 4/5 TFTP: add tftpsrv command
In reply to Luca Ceresoli---
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(-)
README +1 -0
@@ -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
common/cmd_net.c +17 -0
@@ -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[]) {
include/net.h +2 -1
@@ -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 */
net/net.c +6 -1
@@ -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 ;
Signature · 1 line
1.7.1
PATCH v3 4/5 TFTP: add tftpsrv command
Quoted 2 lines · Luca Ceresoli
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Cheers
Detlev
Signature · 6 lines
-- 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
Wolfgang Denk
AppliedPATCH v3 4/5 TFTP: add tftpsrv command
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 14 lines · In message <[email protected]> you…
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.Changes in v3:
- rebased on top of thenet/tftp.ccleanup;
- madedo_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.
Quoted 5 lines
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
Signature · 4 lines
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
PATCH v3 5/5 net/tftp.c: fix typo
In reply to Luca Ceresoli---
Changes in v3: this patch is new in v3.
net/tftp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
net/tftp.c +1 -1
@@ -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
Signature · 1 line
1.7.1
PATCH v3 5/5 net/tftp.c: fix typo
Quoted 2 lines · Luca Ceresoli
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Cheers
Detlev
Signature · 8 lines
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
Wolfgang Denk
AppliedPATCH v3 5/5 net/tftp.c: fix typo
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 8 lines · In message <[email protected]> you…
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
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
Signature · 5 lines
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.
PATCH v2 1/6 README: remove spurious line
In reply to Luca Ceresoli---
Changes in v2: none.
README | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
README +0 -1
@@ -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
Signature · 1 line
1.7.1
PATCH v2 1/6 README: remove spurious line
Hi Luca,
Quoted 13 lines
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.README | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)README +0
-0
README The following options need to be configured: · line 1303
Quoted 6 lines
README +0
-1@@ -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
Signature · 6 lines
debian is a prototype for a future version of emacs.
-- Thien-Thi Nguyen in <[email protected]>
--
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.dePATCH v2 2/6 NET: pass source IP address to packet handlers
In reply to Luca CeresoliThis is needed for the upcoming TFTP server implementation.
This also simplifies PingHandler() and fixes rxhand_f documentation.
---
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(-)
drivers/net/netconsole.c +3 -2
@@ -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)
include/net.h +10 -5
@@ -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.
net/bootp.c +6 -3
@@ -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;
net/dns.c +1 -1
@@ -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;
net/net.c +17 -13
@@ -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 ;
net/nfs.c +1 -1
@@ -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;
net/rarp.c +2 -1
@@ -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" );
net/sntp.c +2 -1
@@ -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;
net/tftp.c +2 -1
@@ -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;
Signature · 1 line
1.7.1
PATCH v2 2/6 NET: pass source IP address to packet handlers
Hi Luca,
Quoted 21 lines
This is needed for the upcoming TFTP server implementation.
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
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(-)
[...]
Quoted 4 lines
net/net.c +0
-0
[...]
Quoted 6 lines
net/net.c +1
-1@@ -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 );
net/net.c NetReceive(volatile uchar * inpkt, int len) · line 1609
Quoted 10 lines
net/net.c +3
-2return ;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
Signature · 8 lines
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
PATCH v2 2/6 NET: pass source IP address to packet handlers
Il 19/04/2011 16:15, Detlev Zundel ha scritto:
Quoted 45 lines
Hi Luca,
Quoted 21 lines
This is needed for the upcoming TFTP server implementation.
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli<[email protected]>Cc Wolfgang Denk<[email protected]>---
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(-)[...]
Quoted 4 lines
net/net.c +0
-0[...]
Quoted 14 lines
net/net.c +4
-3@@ -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);
Quoted 5 lines
net/net.c NetReceive(volatile uchar * inpkt, int len) · line 1609
Quoted 2 lines
net/net.c +0
-0memcpy (&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
Wolfgang Denk
AppliedPATCH v2 2/6 NET: pass source IP address to packet handlers
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 10 lines · In message <[email protected]> you…
This is needed for the upcoming TFTP server implementation.
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2:
- fixed checkpatch issues.
Quoted 10 lines
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
Signature · 6 lines
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.
PATCH v2 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliWith the upcoming TFTP server implementation, the remote node can be either a client or a server, so avoid ambiguities.
---
Changes in v2:
- fixed checkpatch issues.
net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)
net/tftp.c +21 -21
@@ -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 );
Signature · 1 line
1.7.1
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Luca,
Quoted 25 lines
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 <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2:
- fixed checkpatch issues.net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)net/tftp.c +2
-3@@ -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 */
net/tftp.c enum { · line 65
Quoted 10 lines
net/tftp.c +5
-4+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.
net/tftp.c enum { · line 69
Quoted 6 lines
net/tftp.c +2
-2static 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.
[...]
net/tftp.c TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, · line 424
Quoted 6 lines
net/tftp.c +1
-1@@ -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 ;)
[...]
net/tftp.c TftpStart (void) · line 571
Quoted 6 lines
net/tftp.c +1
-1@@ -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
Signature · 6 lines
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Il 19/04/2011 16:18, Detlev Zundel ha scritto:
Quoted 31 lines
Hi Luca,
Quoted 29 lines
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
Changes in v2:
- fixed checkpatch issues.net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)net/tftp.c +3
-5@@ -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 */
Quoted 8 lines
net/tftp.c enum { · line 65
Quoted 6 lines
net/tftp.c +4
-2-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.
Quoted 10 lines
net/tftp.c enum { · line 69
Quoted 6 lines
net/tftp.c +2
-2static 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.
[...]
Quoted 7 lines
net/tftp.c TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, · line 424
Quoted 6 lines
net/tftp.c +1
-1@@ -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.
Quoted 9 lines
[...]
net/tftp.c TftpStart (void) · line 571
Quoted 6 lines
net/tftp.c +1
-1@@ -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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Luca,
[...]
Quoted 3 lines
Quoted 1 line
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
Signature · 5 lines
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi,
Quoted 4 lines · just a few e-mails ago along this thread Albert Aribaud
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.
Quoted 5 lines · Now Detlev Zundel
...
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Luca,
Quoted 19 lines
Hi,
Quoted 4 lines · just a few e-mails ago along this thread Albert Aribaud
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.
Quoted 5 lines · Now Detlev Zundel
...
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.
Quoted 3 lines
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.
Quoted 1 line
There should be one precise policy, and that should be clearly documented.
I fully agree.
Quoted 2 lines
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
Signature · 6 lines
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.dePATCH v2 3/6 TFTP: rename "server" to "remote"
Quoted 4 lines · Detlev Zundel
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Luca,
Quoted 8 lines
Quoted 4 lines · Detlev Zundel
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.
Quoted 5 lines
net/net.calone 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.
Quoted 4 lines
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
Signature · 5 lines
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliHi Luca,
Quoted 41 lines
Il 19/04/2011 16:18, Detlev Zundel ha scritto:
Quoted 31 lines
Hi Luca,
Quoted 29 lines
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
Changes in v2:
- fixed checkpatch issues.net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)net/tftp.c +3
-5@@ -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 */ Quoted 8 lines
net/tftp.c enum { · line 65
Quoted 6 lines
net/tftp.c +4
-2-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.
Quoted 1 line
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
Signature · 7 lines
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.dePATCH v2 3/6 TFTP: rename "server" to "remote"
Quoted 56 lines · Detlev Zundel
Hi Luca,
Quoted 40 lines
Il 19/04/2011 16:18, Detlev Zundel ha scritto:
Quoted 31 lines
Hi Luca,
Quoted 29 lines
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
Changes in v2:- fixed checkpatch issues. net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)net/tftp.c +3
-5@@ -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 */ Quoted 8 lines
net/tftp.c enum { · line 65
Quoted 6 lines
net/tftp.c +4
-2-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.
Quoted 1 line
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);
Quoted 8 lines
total: 0 errors, 2 warnings, 99 lines checked
/home/dzu/transfer/p2has style problems, please review. If any of these errors are false positives report them to the maintainer, see
CHECKPATCH inMAINTAINERS.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"]
Hi Luca,
Quoted 20 lines
Quoted 20 lines
Quoted 1 line
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/p2has style problems, please review. If any of these errors are false positives report them to the maintainer, see
CHECKPATCH inMAINTAINERS.
Quoted 8 lines
Quoted 2 lines
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
Signature · 6 lines
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.dePATCH v2 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliQuoted 68 lines · Luca Ceresoli
Il 19/04/2011 16:18, Detlev Zundel ha scritto:
Quoted 39 lines
Hi Luca,
Quoted 37 lines
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
Changes in v2:
- fixed checkpatch issues.net/tftp.c | 42 +++++++++++++++++++++--------------------- 1 files changed, 21 insertions(+), 21 deletions(-)net/tftp.c +0
-2@@ -55,18 +55,18 @@ enum { TFTP_ERR_FILE_ALREADY_EXISTS = 6, }; -static IPaddr_t TftpServerIP; -static int TftpServerPort;/* The UDP port at their end */
net/tftp.c +2
-1-static int TftpOurPort;/* The UDP port at our end */ +static IPaddr_t TftpRemoteIP; +static int TftpRemotePort;/* The UDP port at their end */
net/tftp.c +1
-2+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 */Quoted 8 lines
Quoted 6 lines
+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.
Quoted 16 lines
Quoted 9 lines
static int TftpState;
#ifdefCONFIG_TFTP_TSIZE
-static int TftpTsize; /* The file size reported by the
server */
-static short TftpNumchars; /* The number of hashes we
printed */net/tftp.c +2
-0+static int TftpTsize;/* The file size reported by the server */ +static short TftpNumchars;/* The number of hashes we printed */
dito.
[...]
Quoted 3 lines
net/tftp.c +0
-0@@ -421,7 +421,7 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
Quoted 7 lines
Quoted 5 lines
net/tftp.c TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, · line 424
Quoted 4 lines
net/tftp.c +1
-1/* * 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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Luca,
[...]
Quoted 3 lines
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
Signature · 8 lines
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.dePATCH v2 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliQuoted 2 lines · 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"
net/tftp.c TftpStart (void) · line 586
Quoted 2 lines
net/tftp.c +1
-1-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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Quoted 7 lines · Mike Frysinger
Quoted 2 lines · 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.
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +1
-1- IPaddr_t ServerNet = TftpServerIP& NetOurSubnetMask; + IPaddr_t ServerNet = TftpRemoteIP& NetOurSubnetMask;shouldnt that now be RemoteNet ?
Done for v3.
Luca
PATCH v2 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 11 lines · In message <[email protected]> you…
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 <[email protected]>Cc Wolfgang Denk <[email protected]>---
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
Signature · 4 lines
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.
PATCH v2 3/6 TFTP: rename "server" to "remote"
Hi Wolfgang,
Quoted 18 lines · Wolfgang Denk
Dear Luca Ceresoli,
Quoted 11 lines · In message<[email protected]> you…
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
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.
Quoted 6 lines
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
PATCH v2 3/6 TFTP: rename "server" to "remote"
Quoted 27 lines · Luca Ceresoli
Hi Wolfgang,
Quoted 24 lines · Wolfgang Denk
Dear Luca Ceresoli,
In message<[email protected]>
Quoted 11 lines · you
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<[email protected]>Cc Wolfgang Denk<[email protected]>---
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...
Quoted 7 lines
Quoted 2 lines
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.ccleanup 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
PATCH v2 4/6 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
In reply to Luca CeresoliWith the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
---
Changes in v2: none.
net/tftp.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
net/tftp.c +6 -6
@@ -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 );
Signature · 1 line
1.7.1
PATCH v2 4/6 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
Hi Luca,
Quoted 7 lines
With the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.
Signature · 8 lines
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.dePATCH v2 5/6 TFTP: net/tftp.c: add server mode receive
In reply to Luca Ceresoli---
Changes in v2: none.
net/tftp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- net/tftp.h | 6 +++++ 2 files changed, 74 insertions(+), 4 deletions(-)
net/tftp.c +68 -4
@@ -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 <[email protected]> */ #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. */
net/tftp.h +6 -0
@@ -2,6 +2,8 @@* LiMon - BOOTP/TFTP. * * Copyright 1994, 1995, 2000 Neil Russell. +* Copyright 2011 Comelit Group SpA +* Luca Ceresoli <[email protected]> * (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__ */
Signature · 1 line
1.7.1
PATCH v2 5/6 TFTP: net/tftp.c: add server mode receive
Hi Luca,
Quoted 4 lines
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.
Only one comment below
Quoted 38 lines
net/tftp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- net/tftp.h | 6 +++++ 2 files changed, 74 insertions(+), 4 deletions(-)net/tftp.c +7
-0@@ -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 <[email protected]> */ #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;
net/tftp.c TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, · line 308
Quoted 10 lines
net/tftp.c +5
-1@@ -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
Signature · 6 lines
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.dePATCH v2 5/6 TFTP: net/tftp.c: add server mode receive
Quoted 55 lines · Detlev Zundel
Hi Luca,
Quoted 4 lines
Signed-off-by Luca Ceresoli<[email protected]>Cc Wolfgang Denk<[email protected]>---
Changes in v2: none.Only one comment below
Quoted 37 lines
net/tftp.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- net/tftp.h | 6 +++++ 2 files changed, 74 insertions(+), 4 deletions(-)net/tftp.c +7
-0@@ -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<[email protected]> */ #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;net/tftp.c TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, · line 308
Quoted 10 lines
net/tftp.c +5
-1@@ -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;
Quoted 8 lines
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_WRQwill 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
PATCH v2 5/6 TFTP: net/tftp.c: add server mode receive
Hi Luca,
[...]
Quoted 10 lines
Quoted 8 lines
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_WRQwill simply never be entered, correct?Here are my measurements for the dig297 board (ARMV7 OMAP3):
Quoted 7 lines
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.
Quoted 6 lines
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
Signature · 8 lines
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
PATCH v2 6/6 TFTP: add tftpsrv command
In reply to Luca Ceresoli---
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(-)
README +1 -0
@@ -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
common/cmd_net.c +14 -0
@@ -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[]) {
include/net.h +2 -1
@@ -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 */
net/net.c +8 -2
@@ -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 );
Signature · 1 line
1.7.1
PATCH v2 6/6 TFTP: add tftpsrv command
Hi Luca,
Quoted 4 lines
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>---
Changes in v2: none.
Signature · 7 lines
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
PATCH v2 6/6 TFTP: add tftpsrv command
In reply to Luca Ceresolicommon/cmd_net.c U_BOOT_CMD( · line 56
Quoted 2 lines · On Monday, April 18, 2011 12:19:54 Luca Ceresoli wrote:
common/cmd_net.c +2
-0+#ifdef CONFIG_CMD_TFTPSRV +int do_tftpsrv (cmd_tbl_t *cmdtp,int flag,int argc,char *const argv[])
should be static
common/cmd_net.c U_BOOT_CMD( · line 65
Quoted 5 lines
common/cmd_net.c +5
-0+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
PATCH v2 6/6 TFTP: add tftpsrv command
In reply to Luca CeresoliQuoted 1 line · Luca Ceresoli
+int
do_tftpsrv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
static
-mike
PATCH 1/6 README: remove spurious line
In reply to Luca Ceresoli--- README | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
README +0 -1
@@ -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
Signature · 1 line
1.7.1
Wolfgang Denk
AppliedPATCH 1/6 README: remove spurious line
Dear Luca Ceresoli,
Quoted 5 lines · In message <[email protected]> you…
Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>--- README | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
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.
PATCH 2/6 NET: pass source IP address to packet handlers
In reply to Luca CeresoliThis is needed for the upcoming TFTP server implementation.
This also simplifies PingHandler() and fixes rxhand_f documentation.
--- 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(-)
drivers/net/netconsole.c +3 -2
@@ -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)
include/net.h +10 -5
@@ -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.
net/bootp.c +6 -3
@@ -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;
net/dns.c +1 -1
@@ -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;
net/net.c +14 -11
@@ -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 ;
net/nfs.c +1 -1
@@ -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;
net/rarp.c +2 -1
@@ -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" );
net/sntp.c +2 -1
@@ -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;
net/tftp.c +2 -1
@@ -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;
Signature · 1 line
1.7.1
Wolfgang Denk
AppliedPATCH 2/6 NET: pass source IP address to packet handlers
Dear Luca Ceresoli,
Quoted 6 lines · In message <[email protected]> you…
This is needed for the upcoming TFTP server implementation.
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
Quoted 11 lines
--- 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
Signature · 5 lines
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
PATCH 2/6 NET: pass source IP address to packet handlers
In reply to Luca CeresoliDear Luca Ceresoli,
Quoted 6 lines · In message <[email protected]> you…
This is needed for the upcoming TFTP server implementation.
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli <[email protected]>Cc Wolfgang Denk <[email protected]>
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
Signature · 5 lines
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_
PATCH v2 2/6 NET: pass source IP address to packet handlers
Hi Wolfgang,
Quoted 5 lines · Wolfgang Denk
Dear Luca Ceresoli,
Quoted 2 lines · In message<[email protected]> you…
This is needed for the upcoming TFTP server implementation.
Quoted 7 lines
Quoted 4 lines
This also simplifies
PingHandler()and fixesrxhand_fdocumentation.Signed-off-by Luca Ceresoli<[email protected]>Cc Wolfgang Denk<[email protected]>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:
Quoted 9 lines
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
PATCH 3/6 TFTP: rename "server" to "remote"
In reply to Luca CeresoliWith the upcoming TFTP server implementation, the remote node can be either a client or a server, so avoid ambiguities.
--- net/tftp.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
net/tftp.c +14 -14
@@ -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 );
Signature · 1 line
1.7.1
PATCH 4/6 TFTP: rename STATE_RRQ to STATE_SEND_RRQ
In reply to Luca CeresoliWith the upcoming TFTP server implementation, requests can be either outgoing or incoming, so avoid ambiguities.
--- net/tftp.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
net/tftp.c +6 -6
@@ -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 );
Signature · 1 line
1.7.1
PATCH 5/6 TFTP: net/tftp.c: add server mode receive
In reply to Luca Ceresoli---
A note on the style of this patch.
There are many hunks like this:
Review
Quoted 6 lines
(quoted diff) +5
-1- 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(-)
net/tftp.c +68 -4
@@ -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 <[email protected]> */ #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. */
net/tftp.h +6 -0
@@ -2,6 +2,8 @@* LiMon - BOOTP/TFTP. * * Copyright 1994, 1995, 2000 Neil Russell. +* Copyright 2011 Comelit Group SpA +* Luca Ceresoli <[email protected]> * (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__ */
Signature · 1 line
1.7.1
PATCH 6/6 TFTP: add tftpsrv command
In reply to Luca Ceresoli--- README | 1 + common/cmd_net.c | 14 ++++++++++++++ include/net.h | 3 ++- net/net.c | 10 ++++++++-- 4 files changed, 25 insertions(+), 3 deletions(-)
README +1 -0
@@ -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
common/cmd_net.c +14 -0
@@ -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[]) {
include/net.h +2 -1
@@ -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 */
net/net.c +8 -2
@@ -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 );
Signature · 1 line
1.7.1