* [RFC 0/2] netconsole and networking co-existence
@ 2021-05-03 11:55 Ian Ray
2021-05-03 11:55 ` [RFC 1/2] net: net_up, net_down Ian Ray
2021-05-03 11:55 ` [RFC 2/2] netconsole and networking co-existence Ian Ray
0 siblings, 2 replies; 5+ messages in thread
From: Ian Ray @ 2021-05-03 11:55 UTC (permalink / raw)
To: u-boot
Commit d9506cd41ce98e10a29c25c4766878367103bb2d attempts to fix ping in
netconsole [1] but IIUC the fix is incomplete (at least in our config).
[1] https://patchwork.ozlabs.org/project/uboot/patch/20201221034439.2747170-1-yliu at cybertec.com.au/
Two issues were identified:
Firstly that the logic of eth_halt() in the various network operations
would break an already running netconsole.
Secondly, netconsole uses the net output buffer which means that any
nested network operation (such as arp or ping) will cause an overwrite
of the netconsole data.
Marked as RFC since not all network protocols have been tested, and not
all nc configurations have been tested (e.g. broadcast address).
Tested against ge_bx50v3_defconfig (i.MX6) using the fec.
Tested against 2020.10 050acee119b3757fee3bd128f55d720fdd9bb890, using a
configuration that includes CONFIG_NETCONSOLE, CONFIG_CMD_NFS, and
CONFIG_CMD_PING.
A sample session is as follows:
Configure (via serial console)
=> env set ethaddr ca:fe:de:ca:f0:11 ; env set ipaddr 192.168.120.49 ; setenv stdout serial,nc; setenv stdin serial,nc
=> env set ncip 192.168.120.1
Invoke net console
./netconsole 192.168.120.49
Invoke ping (via serial console)
=> ping 192.168.120.1
Using ethernet at 2188000 device
host 192.168.120.1 is alive
=> _
Output on net console
Board out port: 6666
Board in port: 6666
NOTE: the interrupt signal (normally ^C) has been remapped to ^T
ncb: not found
=> ping 192.168.120.1
Using ethernet at 2188000 device
host 192.168.120.1 is alive
=>
Invoke ping (via netconsole) for an IP address that is not reachable.
Type ^C to abort.
The ^C is not echoed.
=> ping 192.168.120.2
Using ethernet at 2188000 device
Abort
ping failed; host 192.168.120.2 is not alive
=> _
Output on serial console
=> ping 192.168.120.2
Using ethernet at 2188000 device
Abort
ping failed; host 192.168.120.2 is not alive
=>
The following tcpdump fragment captures the typing of the letters "ok"
in the net console. There was approximately 3 seconds between each
letter.
> sudo tcpdump -i enx001a9f0c4f04 -n host 192.168.120.49
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enx001a9f0c4f04, link-type EN10MB (Ethernet), capture size 262144 bytes
14:20:05.856372 IP 192.168.120.1.53891 > 192.168.120.49.6666: UDP, length 1
'o' from host (nc stdin) to target
14:20:05.856684 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:20:05.856709 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:20:05.856981 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 1
'o' from target (stdout) to host
14:20:06.626637 ARP, Request who-has 192.168.120.49 tell 192.168.120.1, length 28
14:20:06.626915 ARP, Reply 192.168.120.49 is-at ca:fe:de:ca:f0:11, length 46
14:20:09.364465 IP 192.168.120.1.53891 > 192.168.120.49.6666: UDP, length 1
'k' from host to target
14:20:09.364776 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:20:09.364794 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:20:09.365041 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 1
'k' from target to host
The following tcpdump fragment captures the typing of the letters "ok"
in the serial console. There was approximately 3 seconds between each
letter.
14:22:48.225004 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:22:48.225032 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:22:48.225291 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 1
'o' from target (stdout) to host
14:22:51.777002 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:22:51.777031 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:22:51.777251 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 1
'k' from target to host
The following tcpdump fragment captures ping to a known host.
(Not shown: the command "ping 192.168.120.1")
14:23:56.873103 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:23:56.873131 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:23:56.873426 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 1
'\n'
14:23:56.876020 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 30
"Using ethernet at 2188000 device"
14:23:56.876021 ARP, Request who-has 192.168.120.1 tell 192.168.120.49, length 46
14:23:56.876060 ARP, Reply 192.168.120.1 is-at 00:1a:9f:0c:4f:04, length 28
14:23:56.876354 IP 192.168.120.49 > 192.168.120.1: ICMP echo request, id 0, seq 5, length 8
14:23:56.876385 IP 192.168.120.1 > 192.168.120.49: ICMP echo reply, id 0, seq 5, length 8
14:23:56.879037 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 28
"host 192.168.120.1 is alive"
14:23:56.879272 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 3
"=> "
14:24:01.890653 ARP, Request who-has 192.168.120.49 tell 192.168.120.1, length 28
14:24:01.890933 ARP, Reply 192.168.120.49 is-at ca:fe:de:ca:f0:11, length 46
And, finally, the following tcpdump fragment captures ping to an unknown host.
(Not shown: the command "ping 192.168.120.2")
14:35:57.680889 ARP, Request who-has 192.168.120.2 tell 192.168.120.49, length 46
14:36:02.681255 ARP, Request who-has 192.168.120.2 tell 192.168.120.49, length 46
14:36:02.786656 ARP, Request who-has 192.168.120.49 tell 192.168.120.1, length 28
14:36:02.786953 ARP, Reply 192.168.120.49 is-at ca:fe:de:ca:f0:11, length 46
14:36:04.154698 IP 192.168.120.1.53891 > 192.168.120.49.6666: UDP, length 1
^C
14:36:04.155671 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 7
"Abort"
14:36:04.159672 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 45
"ping failed; host 192.168.120.2 is not alive"
14:36:04.159878 IP 192.168.120.49.6666 > 192.168.120.1.6666: UDP, length 3
"=> "
Ian Ray (2):
net: net_up, net_down
netconsole and networking co-existence
drivers/net/netconsole.c | 45 +++++++------------
include/net.h | 28 ++++--------
net/arp.c | 7 ++-
net/arp.h | 1 +
net/net.c | 111 ++++++++++++++++++++++++++++++++++-------------
net/ping.c | 4 +-
net/tftp.c | 4 --
net/wol.c | 1 -
8 files changed, 111 insertions(+), 90 deletions(-)
--
2.10.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC 1/2] net: net_up, net_down
2021-05-03 11:55 [RFC 0/2] netconsole and networking co-existence Ian Ray
@ 2021-05-03 11:55 ` Ian Ray
2021-05-05 2:02 ` Ramon Fried
2021-05-03 11:55 ` [RFC 2/2] netconsole and networking co-existence Ian Ray
1 sibling, 1 reply; 5+ messages in thread
From: Ian Ray @ 2021-05-03 11:55 UTC (permalink / raw)
To: u-boot
Calls made to eth_halt() by network operations (ping, nfs, tftp, etc.)
break netconsole if it is already running.
* Maintain the network up / down state based on a reference count,
using new APIs net_up() and net_down().
Note that when network is brought up by netconsole client, then network
will remain up until reboot (because there is no way to stop netconsole
itself).
* Remove net_loop_last_protocol, eth_is_on_demand_init(), and
eth_set_last_protocol(). This functionality is replaced by
net_up() / net_down().
* Replace usage of eth_init(), eth_halt() with net_up() and
net_down() in net.c, ping.c, tftp.c, and netconsole.c.
Signed-off-by: Ian Ray <ian.ray@ge.com>
---
drivers/net/netconsole.c | 26 +++--------------
include/net.h | 23 ++-------------
net/net.c | 75 +++++++++++++++++++++++++++++++++---------------
net/ping.c | 1 -
net/tftp.c | 4 ---
net/wol.c | 1 -
6 files changed, 59 insertions(+), 71 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index f1d0630..b6d2e22 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -27,11 +27,6 @@ static short nc_out_port; /* target output port */
static short nc_in_port; /* source input port */
static const char *output_packet; /* used by first send udp */
static int output_packet_len;
-/*
- * Start with a default last protocol.
- * We are only interested in NETCONS or not.
- */
-enum proto_t net_loop_last_protocol = BOOTP;
static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
struct in_addr sip, unsigned src,
@@ -177,7 +172,6 @@ static void nc_send_packet(const char *buf, int len)
#else
struct eth_device *eth;
#endif
- int inited = 0;
uchar *pkt;
uchar *ether;
struct in_addr ip;
@@ -200,29 +194,17 @@ static void nc_send_packet(const char *buf, int len)
return;
}
- if (!eth_is_active(eth)) {
- if (eth_is_on_demand_init()) {
- if (eth_init() < 0)
- return;
- eth_set_last_protocol(NETCONS);
- } else {
- eth_init_state_only();
- }
-
- inited = 1;
+ if (net_up(NETCONS) < 0) {
+ return;
}
+
pkt = (uchar *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
memcpy(pkt, buf, len);
ether = nc_ether;
ip = nc_ip;
net_send_udp_packet(ether, ip, nc_out_port, nc_in_port, len);
- if (inited) {
- if (eth_is_on_demand_init())
- eth_halt();
- else
- eth_halt_state_only();
- }
+ net_down();
}
static int nc_stdio_start(struct stdio_dev *dev)
diff --git a/include/net.h b/include/net.h
index 1bf9867..cc6be60 100644
--- a/include/net.h
+++ b/include/net.h
@@ -599,6 +599,9 @@ int net_loop(enum proto_t);
/* Load failed. Start again. */
int net_start_again(void);
+int net_up(enum proto_t protocol);
+int net_down(void);
+
/* Get size of the ethernet header when we send */
int net_eth_hdr_size(void);
@@ -706,26 +709,6 @@ int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
unsigned src_port, unsigned len);
#endif
-static __always_inline int eth_is_on_demand_init(void)
-{
-#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
- extern enum proto_t net_loop_last_protocol;
-
- return net_loop_last_protocol != NETCONS;
-#else
- return 1;
-#endif
-}
-
-static inline void eth_set_last_protocol(int protocol)
-{
-#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
- extern enum proto_t net_loop_last_protocol;
-
- net_loop_last_protocol = protocol;
-#endif
-}
-
/*
* Check if autoload is enabled. If so, use either NFS or TFTP to download
* the boot file.
diff --git a/net/net.c b/net/net.c
index 28d9eeb..45d4f19 100644
--- a/net/net.c
+++ b/net/net.c
@@ -404,6 +404,51 @@ void net_init(void)
* Main network processing loop.
*/
+static int up_ref;
+static bool up;
+static bool keep_up;
+
+/// Bring net up/active if needed.
+int net_up(enum proto_t protocol)
+{
+ int ret;
+ if (!up) {
+ eth_halt();
+ eth_set_current();
+ ret = eth_init();
+ if (ret < 0) {
+ debug_cond(DEBUG_INT_STATE, "net_up failed\n");
+ eth_halt();
+ return ret;
+ }
+ up = true;
+ } else if (up_ref == 0) {
+ eth_init_state_only();
+ }
+ up_ref++;
+ if (protocol == NETCONS) {
+ keep_up = true;
+ }
+ return 0;
+}
+
+/// Set net inactive if no clients remain.
+int net_down(void)
+{
+ if (up_ref > 0) {
+ up_ref--;
+ if (up_ref == 0) {
+ if (keep_up) {
+ eth_halt_state_only();
+ } else {
+ up = false;
+ eth_halt();
+ }
+ }
+ }
+ return 0;
+}
+
int net_loop(enum proto_t protocol)
{
int ret = -EINVAL;
@@ -420,16 +465,9 @@ int net_loop(enum proto_t protocol)
bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
net_init();
- if (eth_is_on_demand_init() || protocol != NETCONS) {
- eth_halt();
- eth_set_current();
- ret = eth_init();
- if (ret < 0) {
- eth_halt();
- return ret;
- }
- } else {
- eth_init_state_only();
+ ret = net_up(protocol);
+ if (ret < 0) {
+ return ret;
}
restart:
#ifdef CONFIG_USB_KEYBOARD
@@ -448,7 +486,7 @@ restart:
switch (net_check_prereq(protocol)) {
case 1:
/* network not configured */
- eth_halt();
+ net_down();
net_set_state(prev_net_state);
return -ENODEV;
@@ -589,9 +627,7 @@ restart:
net_arp_wait_packet_ip.s_addr = 0;
net_cleanup_loop();
- eth_halt();
- /* Invalidate the last protocol */
- eth_set_last_protocol(BOOTP);
+ net_down();
puts("\nAbort\n");
/* include a debug print as well incase the debug
@@ -647,12 +683,7 @@ restart:
env_set_hex("filesize", net_boot_file_size);
env_set_hex("fileaddr", image_load_addr);
}
- if (protocol != NETCONS)
- eth_halt();
- else
- eth_halt_state_only();
-
- eth_set_last_protocol(protocol);
+ net_down();
ret = net_boot_file_size;
debug_cond(DEBUG_INT_STATE, "--- net_loop Success!\n");
@@ -660,8 +691,7 @@ restart:
case NETLOOP_FAIL:
net_cleanup_loop();
- /* Invalidate the last protocol */
- eth_set_last_protocol(BOOTP);
+ net_down();
debug_cond(DEBUG_INT_STATE, "--- net_loop Fail!\n");
ret = -ENONET;
goto done;
@@ -719,7 +749,6 @@ int net_start_again(void)
}
if ((!retry_forever) && (net_try_count > retrycnt)) {
- eth_halt();
net_set_state(NETLOOP_FAIL);
/*
* We don't provide a way for the protocol to return an error,
diff --git a/net/ping.c b/net/ping.c
index 0e33660..2c92806 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -64,7 +64,6 @@ static int ping_send(void)
static void ping_timeout_handler(void)
{
- eth_halt();
net_set_state(NETLOOP_FAIL); /* we did not get the reply */
}
diff --git a/net/tftp.c b/net/tftp.c
index 84e970b..dcc387b 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -652,7 +652,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
if (store_block(tftp_cur_block - 1, pkt + 2, len)) {
- eth_halt();
net_set_state(NETLOOP_FAIL);
break;
}
@@ -680,7 +679,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
case TFTP_ERR_FILE_NOT_FOUND:
case TFTP_ERR_ACCESS_DENIED:
puts("Not retrying...\n");
- eth_halt();
net_set_state(NETLOOP_FAIL);
break;
case TFTP_ERR_UNDEFINED:
@@ -829,7 +827,6 @@ void tftp_start(enum proto_t protocol)
#endif
{
if (tftp_init_load_addr()) {
- eth_halt();
net_set_state(NETLOOP_FAIL);
puts("\nTFTP error: ");
puts("trying to overwrite reserved memory...\n");
@@ -885,7 +882,6 @@ void tftp_start_server(void)
tftp_filename[0] = 0;
if (tftp_init_load_addr()) {
- eth_halt();
net_set_state(NETLOOP_FAIL);
puts("\nTFTP error: trying to overwrite reserved memory...\n");
return;
diff --git a/net/wol.c b/net/wol.c
index 0a62566..cd4e67e 100644
--- a/net/wol.c
+++ b/net/wol.c
@@ -85,7 +85,6 @@ void wol_set_timeout(ulong timeout)
static void wol_timeout_handler(void)
{
- eth_halt();
net_set_state(NETLOOP_FAIL);
}
--
2.10.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC 2/2] netconsole and networking co-existence
2021-05-03 11:55 [RFC 0/2] netconsole and networking co-existence Ian Ray
2021-05-03 11:55 ` [RFC 1/2] net: net_up, net_down Ian Ray
@ 2021-05-03 11:55 ` Ian Ray
1 sibling, 0 replies; 5+ messages in thread
From: Ian Ray @ 2021-05-03 11:55 UTC (permalink / raw)
To: u-boot
The network stack does not appear to be designed to handle simultaneous
netconsole and network operations (such as ping, nfs, and tftp) because
of re-use of the netconsole output buffer.
* Add new net_send_udp_packet2, net_send_ip_packet2 APIs.
* Teach netconsole to use a private TX buffer. This is needed in order
to avoid overwriting the ICMP ECHO REQUEST if a ping is started while
netconsole is active.
Signed-off-by: Ian Ray <ian.ray@ge.com>
---
drivers/net/netconsole.c | 19 +++++++++++--------
include/net.h | 5 +++++
net/arp.c | 7 +++++--
net/arp.h | 1 +
net/net.c | 36 ++++++++++++++++++++++++++++--------
net/ping.c | 3 ++-
6 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index b6d2e22..ef65d75 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -27,6 +27,8 @@ static short nc_out_port; /* target output port */
static short nc_in_port; /* source input port */
static const char *output_packet; /* used by first send udp */
static int output_packet_len;
+static uchar nc_pkt_buf[(PKTBUFSRX+1) * PKTSIZE_ALIGN + PKTALIGN];
+static uchar *nc_tx_packet;
static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
struct in_addr sip, unsigned src,
@@ -120,10 +122,10 @@ void nc_start(void)
/* send arp request */
uchar *pkt;
net_set_arp_handler(nc_wait_arp_handler);
- pkt = (uchar *)net_tx_packet + net_eth_hdr_size() +
+ pkt = (uchar *)nc_tx_packet + net_eth_hdr_size() +
IP_UDP_HDR_SIZE;
memcpy(pkt, output_packet, output_packet_len);
- net_send_udp_packet(nc_ether, nc_ip, nc_out_port, nc_in_port,
+ net_send_udp_packet2(nc_tx_packet, nc_ether, nc_ip, nc_out_port, nc_in_port,
output_packet_len);
}
}
@@ -173,8 +175,6 @@ static void nc_send_packet(const char *buf, int len)
struct eth_device *eth;
#endif
uchar *pkt;
- uchar *ether;
- struct in_addr ip;
debug_cond(DEBUG_DEV_PKT, "output: \"%*.*s\"\n", len, len, buf);
@@ -198,11 +198,9 @@ static void nc_send_packet(const char *buf, int len)
return;
}
- pkt = (uchar *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
+ pkt = (uchar *)nc_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
memcpy(pkt, buf, len);
- ether = nc_ether;
- ip = nc_ip;
- net_send_udp_packet(ether, ip, nc_out_port, nc_in_port, len);
+ net_send_udp_packet2(nc_tx_packet, nc_ether, nc_ip, nc_out_port, nc_in_port, len);
net_down();
}
@@ -218,6 +216,11 @@ static int nc_stdio_start(struct stdio_dev *dev)
if (retval != 0)
return retval;
+ if (nc_tx_packet == NULL) {
+ nc_tx_packet = &nc_pkt_buf[0] + (PKTALIGN - 1);
+ nc_tx_packet -= (ulong)nc_tx_packet % PKTALIGN;
+ }
+
/*
* Initialize the static IP settings and buffer pointers
* incase we call net_send_udp_packet before net_loop
diff --git a/include/net.h b/include/net.h
index cc6be60..74f7ce1 100644
--- a/include/net.h
+++ b/include/net.h
@@ -694,9 +694,14 @@ static inline void net_send_packet(uchar *pkt, int len)
* @param sport Source UDP port
* @param payload_len Length of data after the UDP header
*/
+int net_send_ip_packet2(uchar *pkt, uchar *ether, struct in_addr dest, int dport, int sport,
+ int payload_len, int proto, u8 action, u32 tcp_seq_num,
+ u32 tcp_ack_num);
int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
int payload_len, int proto, u8 action, u32 tcp_seq_num,
u32 tcp_ack_num);
+int net_send_udp_packet2(uchar *pkt, uchar *ether, struct in_addr dest, int dport,
+ int sport, int payload_len);
int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport,
int sport, int payload_len);
diff --git a/net/arp.c b/net/arp.c
index 1d06ed2..f19958f 100644
--- a/net/arp.c
+++ b/net/arp.c
@@ -35,6 +35,7 @@ struct in_addr net_arp_wait_packet_ip;
static struct in_addr net_arp_wait_reply_ip;
/* MAC address of waiting packet's destination */
uchar *arp_wait_packet_ethaddr;
+uchar *arp_wait_tx_packet;
int arp_wait_tx_packet_size;
ulong arp_wait_timer_start;
int arp_wait_try;
@@ -47,6 +48,7 @@ void arp_init(void)
arp_wait_packet_ethaddr = NULL;
net_arp_wait_packet_ip.s_addr = 0;
net_arp_wait_reply_ip.s_addr = 0;
+ arp_wait_tx_packet = NULL;
arp_wait_tx_packet_size = 0;
arp_tx_packet = &arp_tx_packet_buf[0] + (PKTALIGN - 1);
arp_tx_packet -= (ulong)arp_tx_packet % PKTALIGN;
@@ -222,12 +224,13 @@ void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
/* set the mac address in the waiting packet's header
and transmit it */
- memcpy(((struct ethernet_hdr *)net_tx_packet)->et_dest,
+ memcpy(((struct ethernet_hdr *)arp_wait_tx_packet)->et_dest,
&arp->ar_sha, ARP_HLEN);
- net_send_packet(net_tx_packet, arp_wait_tx_packet_size);
+ net_send_packet(arp_wait_tx_packet, arp_wait_tx_packet_size);
/* no arp request pending now */
net_arp_wait_packet_ip.s_addr = 0;
+ arp_wait_tx_packet = NULL;
arp_wait_tx_packet_size = 0;
arp_wait_packet_ethaddr = NULL;
}
diff --git a/net/arp.h b/net/arp.h
index 25b3c00..8154ebc 100644
--- a/net/arp.h
+++ b/net/arp.h
@@ -17,6 +17,7 @@
extern struct in_addr net_arp_wait_packet_ip;
/* MAC address of waiting packet's destination */
extern uchar *arp_wait_packet_ethaddr;
+extern uchar *arp_wait_tx_packet;
extern int arp_wait_tx_packet_size;
extern ulong arp_wait_timer_start;
extern int arp_wait_try;
diff --git a/net/net.c b/net/net.c
index 45d4f19..b1dba80 100644
--- a/net/net.c
+++ b/net/net.c
@@ -855,17 +855,38 @@ int net_send_udp_packet(uchar *ether, struct in_addr dest, int dport, int sport,
IPPROTO_UDP, 0, 0, 0);
}
+int net_send_udp_packet2(uchar *pkt, uchar *ether, struct in_addr dest, int dport, int sport,
+ int payload_len)
+{
+ return net_send_ip_packet2(pkt, ether, dest, dport, sport, payload_len,
+ IPPROTO_UDP, 0, 0, 0);
+}
+
+/// Sends net_tx_packet
+
int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
int payload_len, int proto, u8 action, u32 tcp_seq_num,
u32 tcp_ack_num)
{
- uchar *pkt;
+ return net_send_ip_packet2(net_tx_packet, ether, dest, dport, sport,
+ payload_len,
+ proto, action, tcp_seq_num, tcp_ack_num);
+}
+
+/// Global variables / state.
+/// - Uses const net_bcast_ethaddr if destination is broadcast address
+/// - net_set_ether() checks net_our_vlan
+/// - If ether is null, fills net_arp_wait_packet_ip and does ARP
+
+int net_send_ip_packet2(uchar *pkt, uchar *ether, struct in_addr dest, int dport, int sport,
+ int payload_len, int proto, u8 action, u32 tcp_seq_num,
+ u32 tcp_ack_num)
+{
int eth_hdr_size;
int pkt_hdr_size;
- /* make sure the net_tx_packet is initialized (net_init() was called) */
- assert(net_tx_packet != NULL);
- if (net_tx_packet == NULL)
+ assert(pkt != NULL);
+ if (pkt == NULL)
return -1;
/* convert to new style broadcast */
@@ -876,8 +897,6 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
if (dest.s_addr == 0xFFFFFFFF)
ether = (uchar *)net_bcast_ethaddr;
- pkt = (uchar *)net_tx_packet;
-
eth_hdr_size = net_set_ether(pkt, ether, PROT_IP);
switch (proto) {
@@ -898,7 +917,8 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
net_arp_wait_packet_ip = dest;
arp_wait_packet_ethaddr = ether;
- /* size of the waiting packet */
+ /* waiting packet */
+ arp_wait_tx_packet = pkt;
arp_wait_tx_packet_size = pkt_hdr_size + payload_len;
/* and do the ARP request */
@@ -909,7 +929,7 @@ int net_send_ip_packet(uchar *ether, struct in_addr dest, int dport, int sport,
} else {
debug_cond(DEBUG_DEV_PKT, "sending UDP to %pI4/%pM\n",
&dest, ether);
- net_send_packet(net_tx_packet, pkt_hdr_size + payload_len);
+ net_send_packet(pkt, pkt_hdr_size + payload_len);
return 0; /* transmitted */
}
}
diff --git a/net/ping.c b/net/ping.c
index 2c92806..74c4726 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -52,7 +52,8 @@ static int ping_send(void)
set_icmp_header(pkt, net_ping_ip);
- /* size of the waiting packet */
+ /* waiting packet */
+ arp_wait_tx_packet = net_tx_packet;
arp_wait_tx_packet_size = eth_hdr_size + IP_ICMP_HDR_SIZE;
/* and do the ARP request */
--
2.10.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC 1/2] net: net_up, net_down
2021-05-03 11:55 ` [RFC 1/2] net: net_up, net_down Ian Ray
@ 2021-05-05 2:02 ` Ramon Fried
2021-05-05 7:04 ` EXT: " Ian Ray
0 siblings, 1 reply; 5+ messages in thread
From: Ramon Fried @ 2021-05-05 2:02 UTC (permalink / raw)
To: u-boot
On Mon, May 3, 2021 at 2:55 PM Ian Ray <ian.ray@ge.com> wrote:
>
> Calls made to eth_halt() by network operations (ping, nfs, tftp, etc.)
> break netconsole if it is already running.
>
> * Maintain the network up / down state based on a reference count,
> using new APIs net_up() and net_down().
>
> Note that when network is brought up by netconsole client, then network
> will remain up until reboot (because there is no way to stop netconsole
> itself).
>
> * Remove net_loop_last_protocol, eth_is_on_demand_init(), and
> eth_set_last_protocol(). This functionality is replaced by
> net_up() / net_down().
>
> * Replace usage of eth_init(), eth_halt() with net_up() and
> net_down() in net.c, ping.c, tftp.c, and netconsole.c.
>
> Signed-off-by: Ian Ray <ian.ray@ge.com>
> ---
> drivers/net/netconsole.c | 26 +++--------------
> include/net.h | 23 ++-------------
> net/net.c | 75 +++++++++++++++++++++++++++++++++---------------
> net/ping.c | 1 -
> net/tftp.c | 4 ---
> net/wol.c | 1 -
> 6 files changed, 59 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index f1d0630..b6d2e22 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -27,11 +27,6 @@ static short nc_out_port; /* target output port */
> static short nc_in_port; /* source input port */
> static const char *output_packet; /* used by first send udp */
> static int output_packet_len;
> -/*
> - * Start with a default last protocol.
> - * We are only interested in NETCONS or not.
> - */
> -enum proto_t net_loop_last_protocol = BOOTP;
>
> static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
> struct in_addr sip, unsigned src,
> @@ -177,7 +172,6 @@ static void nc_send_packet(const char *buf, int len)
> #else
> struct eth_device *eth;
> #endif
> - int inited = 0;
> uchar *pkt;
> uchar *ether;
> struct in_addr ip;
> @@ -200,29 +194,17 @@ static void nc_send_packet(const char *buf, int len)
> return;
> }
>
> - if (!eth_is_active(eth)) {
> - if (eth_is_on_demand_init()) {
> - if (eth_init() < 0)
> - return;
> - eth_set_last_protocol(NETCONS);
> - } else {
> - eth_init_state_only();
> - }
> -
> - inited = 1;
> + if (net_up(NETCONS) < 0) {
> + return;
> }
> +
> pkt = (uchar *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> memcpy(pkt, buf, len);
> ether = nc_ether;
> ip = nc_ip;
> net_send_udp_packet(ether, ip, nc_out_port, nc_in_port, len);
>
> - if (inited) {
> - if (eth_is_on_demand_init())
> - eth_halt();
> - else
> - eth_halt_state_only();
> - }
> + net_down();
> }
>
> static int nc_stdio_start(struct stdio_dev *dev)
> diff --git a/include/net.h b/include/net.h
> index 1bf9867..cc6be60 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -599,6 +599,9 @@ int net_loop(enum proto_t);
> /* Load failed. Start again. */
> int net_start_again(void);
>
> +int net_up(enum proto_t protocol);
> +int net_down(void);
> +
> /* Get size of the ethernet header when we send */
> int net_eth_hdr_size(void);
>
> @@ -706,26 +709,6 @@ int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
> unsigned src_port, unsigned len);
> #endif
>
> -static __always_inline int eth_is_on_demand_init(void)
> -{
> -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> - extern enum proto_t net_loop_last_protocol;
> -
> - return net_loop_last_protocol != NETCONS;
> -#else
> - return 1;
> -#endif
> -}
> -
> -static inline void eth_set_last_protocol(int protocol)
> -{
> -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> - extern enum proto_t net_loop_last_protocol;
> -
> - net_loop_last_protocol = protocol;
> -#endif
> -}
> -
> /*
> * Check if autoload is enabled. If so, use either NFS or TFTP to download
> * the boot file.
> diff --git a/net/net.c b/net/net.c
> index 28d9eeb..45d4f19 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -404,6 +404,51 @@ void net_init(void)
> * Main network processing loop.
> */
>
> +static int up_ref;
> +static bool up;
> +static bool keep_up;
> +
> +/// Bring net up/active if needed.
> +int net_up(enum proto_t protocol)
> +{
> + int ret;
> + if (!up) {
> + eth_halt();
> + eth_set_current();
> + ret = eth_init();
> + if (ret < 0) {
> + debug_cond(DEBUG_INT_STATE, "net_up failed\n");
> + eth_halt();
> + return ret;
> + }
> + up = true;
> + } else if (up_ref == 0) {
> + eth_init_state_only();
> + }
> + up_ref++;
> + if (protocol == NETCONS) {
> + keep_up = true;
> + }
> + return 0;
> +}
> +
> +/// Set net inactive if no clients remain.
> +int net_down(void)
> +{
> + if (up_ref > 0) {
> + up_ref--;
> + if (up_ref == 0) {
> + if (keep_up) {
> + eth_halt_state_only();
> + } else {
> + up = false;
> + eth_halt();
> + }
> + }
> + }
> + return 0;
> +}
> +
> int net_loop(enum proto_t protocol)
> {
> int ret = -EINVAL;
> @@ -420,16 +465,9 @@ int net_loop(enum proto_t protocol)
>
> bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> net_init();
> - if (eth_is_on_demand_init() || protocol != NETCONS) {
> - eth_halt();
> - eth_set_current();
> - ret = eth_init();
> - if (ret < 0) {
> - eth_halt();
> - return ret;
> - }
> - } else {
> - eth_init_state_only();
> + ret = net_up(protocol);
> + if (ret < 0) {
> + return ret;
> }
> restart:
> #ifdef CONFIG_USB_KEYBOARD
> @@ -448,7 +486,7 @@ restart:
> switch (net_check_prereq(protocol)) {
> case 1:
> /* network not configured */
> - eth_halt();
> + net_down();
> net_set_state(prev_net_state);
> return -ENODEV;
>
> @@ -589,9 +627,7 @@ restart:
> net_arp_wait_packet_ip.s_addr = 0;
>
> net_cleanup_loop();
> - eth_halt();
> - /* Invalidate the last protocol */
> - eth_set_last_protocol(BOOTP);
> + net_down();
>
> puts("\nAbort\n");
> /* include a debug print as well incase the debug
> @@ -647,12 +683,7 @@ restart:
> env_set_hex("filesize", net_boot_file_size);
> env_set_hex("fileaddr", image_load_addr);
> }
> - if (protocol != NETCONS)
> - eth_halt();
> - else
> - eth_halt_state_only();
> -
> - eth_set_last_protocol(protocol);
> + net_down();
>
> ret = net_boot_file_size;
> debug_cond(DEBUG_INT_STATE, "--- net_loop Success!\n");
> @@ -660,8 +691,7 @@ restart:
>
> case NETLOOP_FAIL:
> net_cleanup_loop();
> - /* Invalidate the last protocol */
> - eth_set_last_protocol(BOOTP);
> + net_down();
> debug_cond(DEBUG_INT_STATE, "--- net_loop Fail!\n");
> ret = -ENONET;
> goto done;
> @@ -719,7 +749,6 @@ int net_start_again(void)
> }
>
> if ((!retry_forever) && (net_try_count > retrycnt)) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> /*
> * We don't provide a way for the protocol to return an error,
> diff --git a/net/ping.c b/net/ping.c
> index 0e33660..2c92806 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -64,7 +64,6 @@ static int ping_send(void)
>
> static void ping_timeout_handler(void)
> {
> - eth_halt();
> net_set_state(NETLOOP_FAIL); /* we did not get the reply */
> }
>
> diff --git a/net/tftp.c b/net/tftp.c
> index 84e970b..dcc387b 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -652,7 +652,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
>
> if (store_block(tftp_cur_block - 1, pkt + 2, len)) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> break;
> }
> @@ -680,7 +679,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> case TFTP_ERR_FILE_NOT_FOUND:
> case TFTP_ERR_ACCESS_DENIED:
> puts("Not retrying...\n");
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> break;
> case TFTP_ERR_UNDEFINED:
> @@ -829,7 +827,6 @@ void tftp_start(enum proto_t protocol)
> #endif
> {
> if (tftp_init_load_addr()) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> puts("\nTFTP error: ");
> puts("trying to overwrite reserved memory...\n");
> @@ -885,7 +882,6 @@ void tftp_start_server(void)
> tftp_filename[0] = 0;
>
> if (tftp_init_load_addr()) {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> puts("\nTFTP error: trying to overwrite reserved memory...\n");
> return;
> diff --git a/net/wol.c b/net/wol.c
> index 0a62566..cd4e67e 100644
> --- a/net/wol.c
> +++ b/net/wol.c
> @@ -85,7 +85,6 @@ void wol_set_timeout(ulong timeout)
>
> static void wol_timeout_handler(void)
> {
> - eth_halt();
> net_set_state(NETLOOP_FAIL);
> }
>
> --
> 2.10.1
>
I like the idea, I think that network protocols should call net_down()
when they're done, however I'm not sure we need reference count.
net_down() can be a wrapper around eth_halt(), We can just check if
netconsole is running and call it eth_halt() only if its' not.
^ permalink raw reply [flat|nested] 5+ messages in thread
* EXT: Re: [RFC 1/2] net: net_up, net_down
2021-05-05 2:02 ` Ramon Fried
@ 2021-05-05 7:04 ` Ian Ray
0 siblings, 0 replies; 5+ messages in thread
From: Ian Ray @ 2021-05-05 7:04 UTC (permalink / raw)
To: u-boot
On Wed, May 05, 2021 at 05:02:21AM +0300, Ramon Fried wrote:
>
> On Mon, May 3, 2021 at 2:55 PM Ian Ray <ian.ray@ge.com> wrote:
> >
> > Calls made to eth_halt() by network operations (ping, nfs, tftp, etc.)
> > break netconsole if it is already running.
> >
> > * Maintain the network up / down state based on a reference count,
> > using new APIs net_up() and net_down().
> >
> > Note that when network is brought up by netconsole client, then network
> > will remain up until reboot (because there is no way to stop netconsole
> > itself).
> >
> > * Remove net_loop_last_protocol, eth_is_on_demand_init(), and
> > eth_set_last_protocol(). This functionality is replaced by
> > net_up() / net_down().
> >
> > * Replace usage of eth_init(), eth_halt() with net_up() and
> > net_down() in net.c, ping.c, tftp.c, and netconsole.c.
> >
> > Signed-off-by: Ian Ray <ian.ray@ge.com>
> > ---
> > drivers/net/netconsole.c | 26 +++--------------
> > include/net.h | 23 ++-------------
> > net/net.c | 75 +++++++++++++++++++++++++++++++++---------------
> > net/ping.c | 1 -
> > net/tftp.c | 4 ---
> > net/wol.c | 1 -
> > 6 files changed, 59 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index f1d0630..b6d2e22 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -27,11 +27,6 @@ static short nc_out_port; /* target output port */
> > static short nc_in_port; /* source input port */
> > static const char *output_packet; /* used by first send udp */
> > static int output_packet_len;
> > -/*
> > - * Start with a default last protocol.
> > - * We are only interested in NETCONS or not.
> > - */
> > -enum proto_t net_loop_last_protocol = BOOTP;
> >
> > static void nc_wait_arp_handler(uchar *pkt, unsigned dest,
> > struct in_addr sip, unsigned src,
> > @@ -177,7 +172,6 @@ static void nc_send_packet(const char *buf, int len)
> > #else
> > struct eth_device *eth;
> > #endif
> > - int inited = 0;
> > uchar *pkt;
> > uchar *ether;
> > struct in_addr ip;
> > @@ -200,29 +194,17 @@ static void nc_send_packet(const char *buf, int len)
> > return;
> > }
> >
> > - if (!eth_is_active(eth)) {
> > - if (eth_is_on_demand_init()) {
> > - if (eth_init() < 0)
> > - return;
> > - eth_set_last_protocol(NETCONS);
> > - } else {
> > - eth_init_state_only();
> > - }
> > -
> > - inited = 1;
> > + if (net_up(NETCONS) < 0) {
> > + return;
> > }
> > +
> > pkt = (uchar *)net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> > memcpy(pkt, buf, len);
> > ether = nc_ether;
> > ip = nc_ip;
> > net_send_udp_packet(ether, ip, nc_out_port, nc_in_port, len);
> >
> > - if (inited) {
> > - if (eth_is_on_demand_init())
> > - eth_halt();
> > - else
> > - eth_halt_state_only();
> > - }
> > + net_down();
> > }
> >
> > static int nc_stdio_start(struct stdio_dev *dev)
> > diff --git a/include/net.h b/include/net.h
> > index 1bf9867..cc6be60 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -599,6 +599,9 @@ int net_loop(enum proto_t);
> > /* Load failed. Start again. */
> > int net_start_again(void);
> >
> > +int net_up(enum proto_t protocol);
> > +int net_down(void);
> > +
> > /* Get size of the ethernet header when we send */
> > int net_eth_hdr_size(void);
> >
> > @@ -706,26 +709,6 @@ int nc_input_packet(uchar *pkt, struct in_addr src_ip, unsigned dest_port,
> > unsigned src_port, unsigned len);
> > #endif
> >
> > -static __always_inline int eth_is_on_demand_init(void)
> > -{
> > -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> > - extern enum proto_t net_loop_last_protocol;
> > -
> > - return net_loop_last_protocol != NETCONS;
> > -#else
> > - return 1;
> > -#endif
> > -}
> > -
> > -static inline void eth_set_last_protocol(int protocol)
> > -{
> > -#if defined(CONFIG_NETCONSOLE) && !defined(CONFIG_SPL_BUILD)
> > - extern enum proto_t net_loop_last_protocol;
> > -
> > - net_loop_last_protocol = protocol;
> > -#endif
> > -}
> > -
> > /*
> > * Check if autoload is enabled. If so, use either NFS or TFTP to download
> > * the boot file.
> > diff --git a/net/net.c b/net/net.c
> > index 28d9eeb..45d4f19 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -404,6 +404,51 @@ void net_init(void)
> > * Main network processing loop.
> > */
> >
> > +static int up_ref;
> > +static bool up;
> > +static bool keep_up;
> > +
> > +/// Bring net up/active if needed.
> > +int net_up(enum proto_t protocol)
> > +{
> > + int ret;
> > + if (!up) {
> > + eth_halt();
> > + eth_set_current();
> > + ret = eth_init();
> > + if (ret < 0) {
> > + debug_cond(DEBUG_INT_STATE, "net_up failed\n");
> > + eth_halt();
> > + return ret;
> > + }
> > + up = true;
> > + } else if (up_ref == 0) {
> > + eth_init_state_only();
> > + }
> > + up_ref++;
> > + if (protocol == NETCONS) {
> > + keep_up = true;
> > + }
> > + return 0;
> > +}
> > +
> > +/// Set net inactive if no clients remain.
> > +int net_down(void)
> > +{
> > + if (up_ref > 0) {
> > + up_ref--;
> > + if (up_ref == 0) {
> > + if (keep_up) {
> > + eth_halt_state_only();
> > + } else {
> > + up = false;
> > + eth_halt();
> > + }
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > int net_loop(enum proto_t protocol)
> > {
> > int ret = -EINVAL;
> > @@ -420,16 +465,9 @@ int net_loop(enum proto_t protocol)
> >
> > bootstage_mark_name(BOOTSTAGE_ID_ETH_START, "eth_start");
> > net_init();
> > - if (eth_is_on_demand_init() || protocol != NETCONS) {
> > - eth_halt();
> > - eth_set_current();
> > - ret = eth_init();
> > - if (ret < 0) {
> > - eth_halt();
> > - return ret;
> > - }
> > - } else {
> > - eth_init_state_only();
> > + ret = net_up(protocol);
> > + if (ret < 0) {
> > + return ret;
> > }
> > restart:
> > #ifdef CONFIG_USB_KEYBOARD
> > @@ -448,7 +486,7 @@ restart:
> > switch (net_check_prereq(protocol)) {
> > case 1:
> > /* network not configured */
> > - eth_halt();
> > + net_down();
> > net_set_state(prev_net_state);
> > return -ENODEV;
> >
> > @@ -589,9 +627,7 @@ restart:
> > net_arp_wait_packet_ip.s_addr = 0;
> >
> > net_cleanup_loop();
> > - eth_halt();
> > - /* Invalidate the last protocol */
> > - eth_set_last_protocol(BOOTP);
> > + net_down();
> >
> > puts("\nAbort\n");
> > /* include a debug print as well incase the debug
> > @@ -647,12 +683,7 @@ restart:
> > env_set_hex("filesize", net_boot_file_size);
> > env_set_hex("fileaddr", image_load_addr);
> > }
> > - if (protocol != NETCONS)
> > - eth_halt();
> > - else
> > - eth_halt_state_only();
> > -
> > - eth_set_last_protocol(protocol);
> > + net_down();
> >
> > ret = net_boot_file_size;
> > debug_cond(DEBUG_INT_STATE, "--- net_loop Success!\n");
> > @@ -660,8 +691,7 @@ restart:
> >
> > case NETLOOP_FAIL:
> > net_cleanup_loop();
> > - /* Invalidate the last protocol */
> > - eth_set_last_protocol(BOOTP);
> > + net_down();
> > debug_cond(DEBUG_INT_STATE, "--- net_loop Fail!\n");
> > ret = -ENONET;
> > goto done;
> > @@ -719,7 +749,6 @@ int net_start_again(void)
> > }
> >
> > if ((!retry_forever) && (net_try_count > retrycnt)) {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > /*
> > * We don't provide a way for the protocol to return an error,
> > diff --git a/net/ping.c b/net/ping.c
> > index 0e33660..2c92806 100644
> > --- a/net/ping.c
> > +++ b/net/ping.c
> > @@ -64,7 +64,6 @@ static int ping_send(void)
> >
> > static void ping_timeout_handler(void)
> > {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL); /* we did not get the reply */
> > }
> >
> > diff --git a/net/tftp.c b/net/tftp.c
> > index 84e970b..dcc387b 100644
> > --- a/net/tftp.c
> > +++ b/net/tftp.c
> > @@ -652,7 +652,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> > net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
> >
> > if (store_block(tftp_cur_block - 1, pkt + 2, len)) {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > break;
> > }
> > @@ -680,7 +679,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
> > case TFTP_ERR_FILE_NOT_FOUND:
> > case TFTP_ERR_ACCESS_DENIED:
> > puts("Not retrying...\n");
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > break;
> > case TFTP_ERR_UNDEFINED:
> > @@ -829,7 +827,6 @@ void tftp_start(enum proto_t protocol)
> > #endif
> > {
> > if (tftp_init_load_addr()) {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > puts("\nTFTP error: ");
> > puts("trying to overwrite reserved memory...\n");
> > @@ -885,7 +882,6 @@ void tftp_start_server(void)
> > tftp_filename[0] = 0;
> >
> > if (tftp_init_load_addr()) {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > puts("\nTFTP error: trying to overwrite reserved memory...\n");
> > return;
> > diff --git a/net/wol.c b/net/wol.c
> > index 0a62566..cd4e67e 100644
> > --- a/net/wol.c
> > +++ b/net/wol.c
> > @@ -85,7 +85,6 @@ void wol_set_timeout(ulong timeout)
> >
> > static void wol_timeout_handler(void)
> > {
> > - eth_halt();
> > net_set_state(NETLOOP_FAIL);
> > }
> >
> > --
> > 2.10.1
> >
> I like the idea, I think that network protocols should call net_down()
I'm not sure I understand this comment.
My (possibly incomplete) understanding of the design is as follows:
1. A command causes the net_loop() to be entered, and returns when the
command has completed (successfully or otherwise).
2. net/net.c calls protocol-specific entry points, which in turn assign
handler functions and send a UDP packet.
3. Handler functions are called on receive or timeout, and the protocol
implementation is responsible for creating new packets or
terminating the net_loop() by calling net_set_state().
Currently the net is brought _down_ by individual protocols (e.g.
net/ping.c, net/tftp.c) on failure (shortly before *or* after calling
net_set_state()) and of course by net/net.c on success.
In the RFC I tried to simplify this such that: net is brought down in
one place net/net.c (essentially on exit from net_loop) -- but sadly
drivers/net/netconsole.c needs to do this too. Maybe that could be
improved.
Why do you think that network protocols should call net_down() ?
> when they're done, however I'm not sure we need reference count.
The reference count does perhaps feel over-engineered. We would need to
be *sure* that there are no overlapping scenarios.
> net_down() can be a wrapper around eth_halt(), We can just check if
> netconsole is running and call it eth_halt() only if its' not.
>
Thanks for the feedback!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-05 7:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 11:55 [RFC 0/2] netconsole and networking co-existence Ian Ray
2021-05-03 11:55 ` [RFC 1/2] net: net_up, net_down Ian Ray
2021-05-05 2:02 ` Ramon Fried
2021-05-05 7:04 ` EXT: " Ian Ray
2021-05-03 11:55 ` [RFC 2/2] netconsole and networking co-existence Ian Ray
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.