Cast uint8_t variable to uint32_t explicitly to avoid implicit cast to
int, and thus potentially undefined behavior, reported by UBSAN:
net_pkt.c:1946:17: runtime error: left shift of 239 by 24 places
cannot be represented in type 'int'
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
In order to avoid alignment issues when casting void pointers to
in(6)_addr structures, create a properly aligned copy of the ip(v6)
address on stack.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Rework the IPv4-related code to avoid casting. Use raw variants of
IPv4-related functions whenever possible (especially on the critical
data path).
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
To address the misaligned access issues reported by UBSAN, introduce raw
variant of certain IPv4 functions used in the critical data path of the
network stack, operating on plain uint8_t buffers in stead of IPv4
address struct.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Rework the rest of the IPv6-related code to avoid casting. Use raw
variants of IPv6-related functions whenever possible (especially on the
critical data path). For the routing case, use a copy of the address to
avoid massive rework of the routing module.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
IPv6 Neighbor Discovery interfaces modules like neighbor or routing
tables - converting them to raw variants seems futile. Therefore, for
IPv6 ND case, copy the raw IP address from the packet into the in6_addr
structure, and then pass it to respective functions. Performance
overhead should not be a big problem in such case as those actions are
only performed if a respective ND packet is received.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Refactor local functions to work with byte buffers instead of struct
in6_addr and use switch to use raw variants of functions operating on
IPv6 addresses.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
To address the misaligned access issues reported by UBSAN, introduce raw
variant of certain IPv6 functions used in the critical data path of the
network stack, operating on plain uint8_t buffers in stead of IPv6
address struct.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
According to RFC 793, ch 3.9 Event Processing,
after the connection is sync-ed with seqnum of both sides then,
1. drop any received segment if the ACK bit is off.
2. validate the acknum like this:
SND.UNA =< SEG.ACK =< SND.NXT
The ACK validation is done before entering the state-machine, so
remove the flags <ACK> check in the state-machine processing.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
Use UNALIGNED_MEMBER_ADDR when getting the address of possibly
unaligned structures members instead of attempting to directly
get the address as an offset.
Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
Use UNALIGNED_MEMBER_ADDR when getting the address of possibly
unaligned structures members instead of attempting to directly
get the address as an offset.
Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
Remember which DNS server was added by a source like DHCPv4 or v6
message. This will allow system to remove DNS servers that were added by
that source. Then when stopping for example DHCP, we can remove those
specific DNS servers and not leaving DNS servers hanging in the system.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Instead of using 32 bit enum values for event numbers, convert
the code to use 64 bit long bit fields. This means that the
user API is changed to use 64 bit event values instead of 32
bit event values.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Make sure we cleanup only those DNS servers that belong to
certain network interface when the interface goes down.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
SUBALIGN forces alignment to the specified value, even if the object
requires stricter alignment. This causes mis-aligned access when accessing
the resulting value.
Signed-off-by: Keith Packard <keithp@keithp.com>
According to RFC 793, the seqnum test includes 4 cases when STATE >
TCP_SYN_SENT:
Seg-len Recv-win Test
------- -------- ---------------------------------------
0 0 SEG.SEQ = RCV.NXT
0 >0 RCV.NXT =< SEG.SEQ < RCV.NXT+RCV.WND
>0 0 not acceptable
>0 >0 RCV.NXT =< SEG.SEQ < RCV.NXT+RCV.WND
or RCV.NXT =< SEG.SEQ+SEG.LEN-1 <RCV.NXT+RCV.WND
After the seq validation, the 'send duplicated ACK' code in FIN_WAIT1/
2/CLOSING/TIMEWAIT state processing is duplicated, so remove them.
Added TEST_CLIENT_SEQ_VALIDATION ztest case in tests/net/tcp.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
In high throughput tests it's fairly easy to overflow the current 32-bit
byte counters in net statistics (it's just over 4 GB of data).
Therefore, make the byte counters 64-bit to prevent overflows.
Rearrange some fields to avoid padding.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
net_stats_t type is an unsigned type, therefore %u should be used
instead of %d when printing values of this type, otherwise negative
values will be printer if INT32_MAX is exceeded.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
When the network interface goes down, we call
net_ipv4_autoconf_reset() which removes the autoaddress
from the network interface.
The net_ipv4_autoconf_reset() is also called when ACD is started
in which case we could see this error message
<dbg> net_if_start_acd: Starting ACD for iface 2
<err> net_if: iface 2 addr 169.254.174.230 (net_if_ipv4_addr_rm():4625)
<dbg> net_if_ipv4_addr_rm: Address 169.254.174.230 not found (-22)
This error is superfluous and not needed. So before trying to
remove the address, check if the interface already has it set and
only then remove it.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
When adding IPv4 address to the network interface, there is no
need to start ACD procedure for localhost or point-to-point links.
The ACD start function would mark the IP address like 127.0.0.1 as
tentative and never make it preferred which would then cause issues
when selecting the network address for sending.
As the ACD start is also called when the network interface comes up,
add the localhost and point-to-point link check to ACD start function
so that we will avoid ACD checks in this case.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
htonl() and htons() take uint32_t/uint16_t as argument. Add the 'u' suffix
to constants to ensure the correct unsigned type is used and to avoid
undefined behavior if these functions are implemented as macros using
bit shifts.
Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
The send_queue was used as SYN/FIN packet retransmission. Before
the SYN/FIN being ACKed and dequeue-ed, the following packets in
the send_queue cannot be sent out. That's why Zephyr had to send
a FIN+ACK instead of a duplicated ACK-only in FINWAIT1, CLOSING.
In fact, we can take SYN/FIN as kind of data and use the same
send_data_timer for retransmission, like other OSes do. This way,
the send_queue is simply used for local traffics.
Benefits (in theory):
1. The code is easier,
2. TxPkt performance is better after skipping enq/deq send_queue,
3. The struct tcp{} node is a few bytes smaller, saving memory.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
TCP implementation provided the parent net_context pointer to the
accept callback instead of the user_data pointer registered with
net_tcp_accept(). This worked fine with the socket integration, as
sockets explicitly registered parent context as user_data, however it
shouldn't be hardcoded like this at the TCP level.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Based on TCP Spec., the outgoing TCP packets shoud use SND.NXT as
the seqnum. In Zephyr, the conn->seq works as the SND.UNA and the
conn->seq + conn->unacked_len works as the SND.NXT. Currently, it
uses SND.UNA in tcp_out() as the seqnum, which might get dropped
as old packets and could not deliver the message to the peer.
A few exceptions use SND.NXT - 1 as the seqnum are: keepalive,
zero-window-probe, FIN/SYN retransmissions. And, for closing a
connection, Zephyr won't send out FIN until all the data has been
ACKed, so the conn->unacked_len is 0 and it is ok to use conn->seq
as the SND.NXT.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
Verify if CONFIG_NET_SOCKETS_PACKET_DGRAM is enabled when creating a
datagram packet socket. Otherwise, it's possible to create
non-functional AF_PACKET/SOCK_DGRAM socket w/o an error.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
It is possible to manually set link address length past 6 at runtime
and trying to generate IPv6 IID address that way. This should fail
as we could read two bytes past the address buffer. There is no issues
in the copying as the target buffer has plenty of space.
Coverity-CID: 516232
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
In enum tcp_state {}, the CLOSED state was put at the last one.
When we do Sequence & Ack validation, we will need to skip the
CLOSED, LISTEN, SYNSENT states. It is easier for coding if we
put the CLOSED to the front, e.g. if state > SYNSENT. And, in
other OSes, the state sequence is normally defined like this.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
Don't reference `net_if_get_by_iface` if `CONFIG_NET_RAW_MODE=y`, since
`net_if.c` is not compiled in that case, leading to linker errors.
Signed-off-by: Jordan Yates <jordan@embeint.com>
Useful to see what prefix is being selected for a given
IPv6 address if debugging is enabled.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Always prefer preferred IPv6 address over deprecated one
regardless of prefix length. This works now same way as in
Linux.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
According to RFC793 chapter3.5 with the 'Reset Processing' part,
"In the SYN-SENT state (a RST received in response to an initial
SYN), the RST is acceptable if the ACK field acknowledges the SYN."
So, in the net_tcp_reply_rst() we should use 'ack++' if no ACK
flag but have SYN flag.
And, all the RST packet should use net_tcp_reply_rst() instead of
tcp_out().
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
Add missing curly braces in if/while/for statements.
This is a style guideline we have that was not enforced in CI. All
issues fixed here were detected by sonarqube SCA.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
The NULL-pkt parameter for tcp_in() was designed for generating
a SYN packet to start the 1st TCP handshake. It is only used
in net_tcp_connect() and tp_input().
To simplify the tcp_in() code logic and make it better under-
standable, a tcp_start_handshake() is added for net_tcp_connect()
and tp_input() to use. Thus, the tcp_in() only handles the in-
coming TCP packets.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
MLD APIs are commonly used across the codebase to configure IPv6
multicast addresses on network interfaces. Sending MLD reports however
works only for native interfaces as it uses low-level APIs. Therefore,
in order to make the APIs at least semi-functional for offloaded
interfaces as well (i.e. allow to configure multicast address on
the interface), return early in case interface is offloaded.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
IGMP APIs are commonly used across the codebase to configure IPv4
multicast addresses on network interfaces. Sending IGMP reports however
works only for native interfaces as it uses low-level APIs. Therefore,
in order to make the APIs at least semi-functional for offloaded
interfaces as well (i.e. allow to configure multicast address on
the interface), return early in case interface is offloaded.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Each incoming TCP packet has been completely handled in current
state. No need to do further process by 'goto next_state'.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
According to TCP Spec. RFC793, ACK flag should be always set
after sequences of both sides are sync-ed except for RST seg-
ment. It is not necessary to send FIN only packet in the
test case, using FIN | ACK instead.
Similarly, change the tcp_out(conn, FIN | ACK) in CLOSE_WAIT.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
warning: format '%d' expects argument of type 'int', but argument 3 has
type 'size_t' {aka 'long unsigned int'} [-Wformat=]
Change the printf modifier to %zd for size_t variable will resolve the
warning.
Signed-off-by: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com>
Default binding should take place before we actually make use of the
local address when registering packet socket "connection".
Also, instead of hard coding the protocol for default binding to
ETH_P_ALL, use the protocol that the socket was created with.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
In order to be able to receive packets on unbound packet sockets (which
should collect packet from all interfaces in such case), it's needed to
register receive callback at the socket layer as soon as the socket is
created.
In additional to that, the default binding for packet sockets need to be
revisited. Packet socket should not be bound to the default interface,
as this way the socket would only be receiving packets from that
particular interface. Instead, leave the interface unspecified in such
case.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Allow to update the local address on a registered connection when
rebinding.
This is needed for packet sockets, as by default packet socket
will be bound to "any" interface (interface index 0), and interface
index is part of the local address registered for packet socket.
In order to be able to explicitly bind to a specific interface later, it
needs to be possible to update the local address registered for the
connection, as we need to update the interface index, which is used
by net_conn_packet_input() for packet filtering.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
keep_alive_timer_restart() only works in ESTABLISHED state. In
tcp_in() SYN_SENT and SYN_RECEIVED state, it won't work by calling
this function. So remove the call in that 2 states while adding it
in the bottom after changing the conn->state to ESTABLISHED.
Signed-off-by: Shrek Wang <inet_eman@outlook.com>
This adjust the IPv6 source address selection so that it is possible
to select deprecated IPv6 address if no better preferred address is found.
From RFC 6724 chapter 5:
Rule 3: Avoid deprecated addresses.
If one of the two source addresses is "preferred" and one of them is
"deprecated" (in the RFC 4862 sense), then prefer the one that is
"preferred".
Rule 8: Use longest matching prefix.
If CommonPrefixLen(SA, D) > CommonPrefixLen(SB, D), then prefer SA.
Similarly, if CommonPrefixLen(SB, D) > CommonPrefixLen(SA, D), then
prefer SB.
So the fix allows deprecated address to be selected if it is a better
match than the preferred one. The reasoning here is that an address with
a longer matching prefix is generally considered topologically closer to
the destination. Using such a source address can lead to more efficient
routing, as it's more likely that the source and destination are within
the same network segment or a closely related one.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Make sure RS process is stopped if network interface goes down
as there is no point doing it any more.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
It might happen that if the network interface is going up/down
fast enough, the RS timer could get inserted to the active RS
timer list twice. This would then cause a forever loop in
rs_timeout() when traversing the active list.
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Update `net_arp_prepare` to return a return code instead of a pointer,
so that the various results of the function can be differentiated.
Signed-off-by: Jordan Yates <jordan@embeint.com>
Print a warning if sending a packet on the L2 interface fails. Currently
this is completely silent unless `NET_DBG` is enabled and the `context`
parameter is provided.
Signed-off-by: Jordan Yates <jordan@embeint.com>
The new DSA framework purpose and changes are as below.
- Aligned to Linux DSA framework which has been already mature framework
for many years, For now in zephyr, the DSA components were splited
as: switch, port, master(not need driver file for now), slave, and tag.
Seperated drivers were used for maintaining and developing new
features.
- The unified dts bindings (aligned to linux) were supported. The port
driver would parse DTS to decide the port type (master port, slave
port, or cpu port) to set up the switch. All the ports registered as
standard ethernet devices. (dsa port and dsa switch tree was not
supported.)
- How to add DSA device driver based on the framework? All the device
driver needing to do is providing dsa_spi implementation and private
data, and calling below initialization.
DSA_INIT_INSTANCE(n, _dapi, data)
- For switch tag case, recv/xmit helpers in dsa_api could be used for
taging/untagging. No modified ethernet drivers.
For no-tag type case, ethernet driver of master port should support
packet injection/extraction for slave ports leaving NULL recv/xmit.
The dsa_nxp_imx_netc.c driver will be the first example of the new DSA
framework.
The future work for DSA will be supporting bridge for ports. We may align
Linux to give users two options to use DSA device:
- Standalone mode: the switch ports work as regular ethernet ports.
- Bridge mode: switch mode with virtual bridge device which could be
assigned IP address.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
