net: Add onlink and forwarding check to IPv6-prepare #78375
Conversation
zephyrbot
requested review from
jukkar,
pdgendt,
rlubos,
ssharks and
tbursztyka
subsys/net/ip/net_if.c
Outdated
There was a problem hiding this comment.
The net_ipv6_prepare_for_send() is also responsible for fragmentation, I'm not entirely sure if it's ok to skip it.
But anyway, inside net_ipv6_prepare_for_send() we already have a lengthy if condition, which verifies if the L2 MAC address is present, and decides whether to quit early:
zephyr/subsys/net/ip/ipv6_nbr.c
Line 853 in 4c6b1e5
I think it'd make more sense to fix that condition if it's wrong for VLAN case (maybe it should quit early for virtual interfaces?) instead of introducing similar check elsewhere.
There was a problem hiding this comment.
You are right, @rlubos. I forgot the fragmentation case.
I will investigate where to add the right checks in the place you mentioned.
shrek-wang
force-pushed
the
fix_net_ipv6_deadloop
branch
from
bc09594 to
b10f995
Compare
shrek-wang
changed the title
jukkar
left a comment
jukkar
left a comment
There was a problem hiding this comment.
I would like to also see in the commit message mentioned why this change is needed.
Deadloop happens when CONFIG_NET_ROUTING and VLAN are enabled. In function net_ipv6_prepare_for_send(), pkt->iface will be updated with net_pkt_set_iface(pkt, iface) in 2 scenarios: 1. ip_hdr->dst is onlink 2. check_route or nbr_lookup VLAN is virtual-iface which attaches to a physical-iface. Each time a packet being sent to a VLAN port will invoke twice of the net_send_data(). The 1st time, pkt->iface is set to virtual iface and the 2nd time to physical iface. However in above 2 scenarios, at the 2nd time of calling the net_send_data(), the pkt-iface will be changed back to virtual iface. The system runs into a deadloop. This can be proved by enabling CONFIG_NET_ROUTING with the VLAN sample. The main purpose for net_ipv6_prepare_for_send() is to set the right ll_dst address. If the ll_dst address is already set, then no need to go through it again. If the packet has done with the forwarding and set the ll_dst, then no need to check_route again. And, the pkt->iface will not be changed back to virtual iface. Fixes: zephyrproject-rtos#77402 Signed-off-by: Shrek Wang <[email protected]>
shrek-wang
force-pushed
the
fix_net_ipv6_deadloop
branch
from
b10f995 to
794121f
Compare
|
Thanks, @jukkar. |
jukkar
left a comment
jukkar
left a comment
There was a problem hiding this comment.
Thanks, the commit message looks very good and thorough.
5 participants
Deadloop happens when CONFIG_NET_ROUTING and VLAN are enabled. In function net_ipv6_prepare_for_send(), pkt->iface will be updated with net_pkt_set_iface(pkt, iface) in 2 scenarios:
VLAN is virtual-iface which attaches to a physical-iface. Each time a packet being sent to a VLAN port will invoke twice of the net_send_data(). The 1st time, pkt->iface is set to virtual iface and the 2nd time to physical iface. However in above 2 scenarios, at the 2nd time of calling the net_send_data(), the pkt-iface will be changed back to virtual iface. The system runs into a deadloop. This can be proved by enabling CONFIG_NET_ROUTING with the VLAN sample.
The main purpose for net_ipv6_prepare_for_send() is to set the right ll_dst address. If the ll_dst address is already set, then no need to go through it again. If the packet has done with the forwarding and set the ll_dst, then no need to check_route again. And, the pkt->iface will not be changed back to virtual iface.
Fixes: #77402