This file has been a mess regarding the order of the functions.
By moving them around, they can be grouped into functions taking
part in normal data transmission (via unsent/unacked) and control
segments (which are allocated, directly sent and freed).
Signed-off-by: goldsimon <goldsimon@gmx.de>
This cleans up the code: sys_timeouts_mbox_fetch() was only used from
tcpip.c anyway, so let's move it there.
Signed-off-by: goldsimon <goldsimon@gmx.de>
fuzz test revealed that an ip header with options might land in ip4_frag() via ICMP. In this case, we can't use LWIP_ERROR() to check for not having ip options as that might be defined to assert
The callers already ensure the ipaddr/netmask/gw won't be NULL, so remove
the duplicated NULL checking in these static functions.
While at it, also move the code saving old_address for netmask/gw as
it's only used when address is actually being changed.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: Dirk Ziegelmeier <dirk@ziegelmeier.net>
Replace NULL pointers by IP4_ADDR_ANY4 - at sometime in the future, we make the NULL pointer handling obsolete and we can remove all the NULL pointer checks in the code
- Fix sys_untimeout implementation should not modify timer values since we are now using absolute timeouts.c
- Cleanup and simplify sys_check_timeouts() implementation
- Implement sys_restart_timeouts to rebase all timeouts based on next timer to expire
Changes by me:
- Rename TIME_LESS_THAN to TIME_LESS_OR_EQUAL_THAN
The netif_do_set_{ipaddr|netmask|gw} are static functions what won't be called
directly, thus move LWIP_ASSERT_CORE_LOCKED to netif_set_{ipaddr|netmask|gw}.
This avoid duplicated LWIP_ASSERT_CORE_LOCKED checking by netif_set_addr().
Signed-off-by: Axel Lin <axel.lin@ingics.com>
This fixes a bug in tcp_split_unsent_seg() where a chained pbuf was
not correctly updating pcb->snd_queuelen during trimming and snd_queuelen
would desynchronize if pbuf_realloc() freed some of the chain
Also, use pbuf_clen() for adding the new remaining segment rather than ++.
The new remaining segment should always be one pbuf due to the semantics
of PBUF_RAM, but this follows the best practice of using pbuf_clen()
This fixes a bug in tcp_split_unsent_seg where oversized segments were not
handled during the split, leading to pcb->unsent_oversized and
useg->oversize_left getting out of sync with the split segment
This would result in over-writing the pbuf if another call to tcp_write()
happened after the split, but before the remainder of the split was sent in
tcp_output
Now pcb->unsent_oversized is explicitly cleared (because the remainder at
the tail is never oversized) and useg->oversized_left is cleared after it is
trimmed
This also updates the test_tcp_persist_split unit test to explicitly check for
this case
The goto freepbuf code path is also used when IP_REASS_CHECK_OVERLAP=0.
Thus remove #if IP_REASS_CHECK_OVERLAP around the freepbuf label to fix
below build error:
cc -g -Wall -DLWIP_DEBUG -pedantic -Werror -Wparentheses -Wsequence-point -Wswitch-default -Wextra -Wundef -Wshadow -Wpointer-arith -Wcast-qual -Wc++-compat -Wwrite-strings -Wold-style-definition -Wcast-align -Wmissing-prototypes -Wredundant-decls -Wnested-externs -Wunreachable-code -Wuninitialized -Wlogical-op -I. -I../../.. -I../../../../lwip/src/include -I../../../ports/unix/port/include -I../../../../mbedtls/include -Wno-redundant-decls -DLWIP_HAVE_MBEDTLS=1 -c ../../../../lwip/src/core/ipv4/ip4_frag.c
../../../../lwip/src/core/ipv4/ip4_frag.c: In function
‘ip_reass_chain_frag_into_datagram_and_validate’: ../../../../lwip/src/core/ipv4/ip4_frag.c:412:7: error: label ‘freepbuf’ used but not defined
goto freepbuf;
^~~~
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: goldsimon <goldsimon@gmx.de>
Use NETIF_FOREACH macro to get some optimizations for LWIP_SINGLE_NETIF case.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: goldsimon <goldsimon@gmx.de>
We preserved the TIME_WAIT handling before, but it seems this is not correct: we want to issue
a RST later again if someone wants to talk to this port. With TIME_WAIT, this might not always
the case.
Slightly better readability by calling dns_backupserver_available()
instead of open-coded. Also move dns_backupserver_available() function
up to avoid forward declaration.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: goldsimon <goldsimon@gmx.de>
This fixes a bug where some callers of netif_issue_reports were not
checking that both link and admin states were up, leading to extraneous
reports when calling one of the following
1) netif_set_ipaddr
2) netif_ip6_addr_set_parts
3) netif_ip6_addr_set_state
The bug has been fixed by placing link and admin state checks in
netif_issue_reports and not requiring the callers to perform this
checking
It does not make sense to return success in p == NULL or
invalid header_size_increment/header_size_decrement cases. Fix it.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
The macros are functions from ctype.h, but ctype.h declares them as functions, not as #defines
It makes no sense to abstract them in lwIPs portability layer, the functions are of low complexity and they are only used in this file.
ARMCC when using __packed structures will not implicitly convert a
pointer to a member of a packed structure to something which does not
have __packed. This results in a compiler error and was found with calls
to icmp6_param_problem
While there is a #pragma pack mode in ARMCC that disables this error, it
does require existing ports to switch over their packing mode and
perform integration
This avoid having a second description for the same stuff that is "bit-rotting" because noone remembers to update this file.
Also remove outdated and misleading zero-copy TX information.
This fixes the following warnings:
test_tcp.c:266:5: error: code will never be executed [-Werror,-Wunreachable-code]
pbuf_free(p);
^~~~~~~~~
- The check API 'fail' aborts the test, thus pbuf_free(p) will never be executed
pbuf.c:783:111: error: format specifies type 'unsigned short' but the argument has type 'u8_t' (aka 'unsigned char') [-Werror,-Wformat]
LWIP_DEBUGF( PBUF_DEBUG | LWIP_DBG_TRACE, ("pbuf_free: %p has ref %"U16_F", ending here.\n", (void *)p, ref));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
- LWIP_PBUF_REF_T is u8_t by default and doesn't match U16_F, so cast to u16_t. The cast and formatter will need to be changed
if ref is larger than 16 bits
ethernet.c:105:16: error: format specifies type 'unsigned char' but the argument has type 'unsigned int' [-Werror,-Wformat]
(unsigned)ethhdr->dest.addr[0], (unsigned)ethhdr->dest.addr[1], (unsigned)ethhdr->dest.addr[2],
~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- addr[] is type u8_t, formatter is X8_F which should be 8 bits. 'unsigned' is an int, so cast to unsighed char instead
Some systems need to take into account an RX buffer pool size when
advising an appropriate number of RX pbufs to queue on the ooseq
list. For some systems there is a practical hard limit beyond which
the rx pool becomes exhausted blocking reception of further buffers
until some are freed.
It also helps to be able to consider the available dynamic memory when
advising an appropriate maximum number of bytes to buffer on the ooseq
list.
These decisions can also benefit from knowing the number already
allocated on a particular pcb, so the ooseq tcp segement is passed to
these functions. For example, if the system only wants to allow the
total number of rx pbufs queued on all the ooseq lists to grow by one
and a pcb already has two then it can return three for this call, but
might return one for another call - supporting a greedy allocation
strategy.
Signed-off-by: goldsimon <goldsimon@gmx.de>
Hop-by-Hop, Destination option header structures consist of 2 unsigned char; next option type and header length field.
And TLV(Type-Length-Value) option headers come by the number in header length field.
If the option type in TLV option header is not recognized and 2 MSB is not 0, it is handled as an exception.
Signed-off-by: goldsimon <goldsimon@gmx.de>
Hop-by-Hop, Destination option header structures consist of 2 unsigned char; next option type and header length field.
And TLV(Type-Length-Value) option headers come by the number in header length field.
If the option type in TLV option header is not recognized and 2 MSB is not 0, it is handled as an exception.
Signed-off-by: goldsimon <goldsimon@gmx.de>
For this, convert 'u8_t nexth' to a pointer and change 'icmp6_param_problem()' to take a pointer, not an offset number
Signed-off-by: goldsimon <goldsimon@gmx.de>
TCP SACKs were removed after some changes in the ooseq queue,
but before all unneeded packets were removed from it.
Because of that, we would sometimes include SACKs
for data already delivered in-order.
Signed-off-by: goldsimon <goldsimon@gmx.de>
This problem would appear to have only affected systems with multiple
interfaces. It was noted causing tcp resets when the pcb was lost, and there
might have been other associated problems.
Signed-off-by: Dirk Ziegelmeier <dirk@ziegelmeier.net>
There were a couple cases in-between that could cause an exit from
tcp_output which don't use useg. With large send buffers, pcb->unacked
may be large and calculating useg is wasted in these exit cases
Some compilers may be re-ordering this already, but it doesn't hurt to
correctly arrange the code
This re-works the persist timer to have the following behavior:
1) Only start persist timer when a buffered segment doesn't fit within
the current window and there is no in-fligh data. Previously, the
persist timer was always started when the window went to zero even
if there was no buffered data (since timer was managed in receive
pathway rather than transmit pathway)
2) Upon first fire of persist timer, fill the remaining window if
non-zero by splitting the unsent segment. If split segment is sent,
persist timer is stopped, RTO timer is now ensuring reliable window
updates
3) If window is already zero when persist timer fires, send 1 byte probe
4) Persist timer and zero window probe should only be active when the
following are true:
* no in-flight data (pcb->unacked == NULL)
* when there is buffered data (pcb->unsent != NULL)
* when pcb->unsent->len > pcb->snd_wnd
lwip_itoa would output the number 0 as \0. This fixes the issue by
adding a special check before the normal conversion loop
This was found via shell cmd idxtoname and win32 port. "lo0" should
be returned for index 1
- add a better-documented static function tcp_output_segment_busy
- try to reduce the number of checks
- tcp_rexmit_rto: iterate pcb->unacked only once
- no need to check for ref==1 in tcp_rexmit_fast when tcp_rexmit does
- call tcp_rexmit_fast if dupacks >= 3 (not == 3) and use TF_INFR flag to guard the fast-rexmit case (that way, it's triggered again on the next dupack)
There is already a guard in tcp_output_segment() for a pbuf still being
referenced by the netif driver due to deferred transmission, however the callers
are modifying state even when this gives up.
It seems cleaner to have the callers guard this case and avoid modifying their
state.
tcp_rexmit_rto() might better avoid re-transmission of any segments if any of
the unacked segments are deferred, to avoid loading the link further if it is
struggling to flush its buffered writes. Link level queues can be limited on
some devices and need spares for link management.
- expose `altcp_tcp_setup()` so we can wrap altcp over existing tcp pcb.
- avoid calling tcp_close() with NULL pcb.
- free altcp_pcb struct when error callback called.
According to `mqtt_tcp_err_cb()` in src/apps/mqtt/mqtt.c, altcp socket should
work the same way than raw tcp socket. So freeing altcp_pcb ensure this.
The new functions both take size_t as increment/decrement argument instead of s16_t (which needed to be range-checked before conversion everywhere) - in most places, the direction (increment or decrement) is known anyway, so no need to encode it in a sign bit
In tcp_output() there were a number of blocks of code performing
duplicate checks of 'if (seg == NULL)'. This combines them together
to reduce duplicate checks
TCP_OUTPUT_DEBUG and TCP_CWND_DEBUG also don't need to be guarded
by #if/#endif since the LWIP_DEBUGF infrastructure already compiles them
out when LWIP_DEBUG is not defined
The ip6_frag.drop counter is updated before all the code paths calling
goto nullreturn, so let's move updating ip6_frag.drop stats to nullreturn.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
This fixes a couple of occurrences where the src and dst parameters to
ip4_route_src() were swapped. This was most likely due to confusion between
ip_route(src, dst) and ip4_route_src(dst, src)
This was found in a system where LWIP_IPV4_SRC_ROUTING is 0
The UDP case was an application socket bound to INADDR_ANY with
IP_MULTICAST_IF set. Transmits would result in calling ip4_route(dst) where
dst was pcb->local_addr (which was INADDR_ANY) instead of pcb->mcast_ip4.
This resulted in a routing failure
The ICMP issue was found through code analysis only
There were uses of dhcp_release() followed immediately by dhcp_discover() but
dhcp_release() now stops dhcp so discovery would fail, so call dhcp_start()
after release which restarts discovery.
Signed-off-by: Dirk Ziegelmeier <dirk@ziegelmeier.net>
According to commit 1f780e86d5 ("PPP timeouts required depend on the number of allowed PPP sessions"):
Per PPP needs 6 timeouts (AUTH + PAP|CHAP|EAP + LCP + IPCP + IP6CP + PPPoE).
So update the minimal MEMP_NUM_SYS_TIMEOUT setting check accordingly.
Since we have LWIP_NUM_SYS_TIMEOUT_INTERNAL so just switch to use it.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: goldsimon <goldsimon@gmx.de>
