Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: arp: Fix ARP retransmission source address selection #40038

Merged
merged 1 commit into from Nov 12, 2021

Conversation

rlubos
Copy link
Contributor

@rlubos rlubos commented Nov 3, 2021

There was a problem with source address selection for ARP
retransmissions, when an ARP entry was already pending. In such case,
the entry value passed to arp_prepare() is NULL, which in result
caused the current_ip variable being used as the source value. The
problem with this approach is, that the current_ip is only set in
IPv4 autoconf, the Ethernet L2 does not set this variable. In result,
every retransmission of an ARP packet was sent with unspecified source
address, preventing the response from being handled.

Fix this by partially restoring the behaviour of the ARP source address
assignment from before IPv4 autoconf was introduced. If the ARP is sent
by the IPv4 autoconf, use the current_ip value provided. If entry is
not set, use the source IPv4 address set in the actual data packet.
Otherwise, search for a source address on the interface corresponding to
the entry.

Signed-off-by: Robert Lubos robert.lubos@nordicsemi.no

There was a problem with source address selection for ARP
retransmissions, when an ARP entry was already pending. In such case,
the `entry` value passed to `arp_prepare()` is NULL, which in result
caused the `current_ip` variable being used as the source value. The
problem with this approach is, that the `current_ip` is only set in
IPv4 autoconf, the Ethernet L2 does not set this variable. In result,
every retransmission of an ARP packet was sent with unspecified source
address, preventing the response from being handled.

Fix this by partially restoring the behaviour of the ARP source address
assignment from before IPv4 autoconf was introduced. If the ARP is sent
by the IPv4 autoconf, use the `current_ip` value provided. If entry is
not set, use the source IPv4 address set in the actual data packet.
Otherwise, search for a source address on the interface corresponding to
the `entry`.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
@rlubos rlubos requested a review from jukkar Nov 3, 2021
@rlubos rlubos requested a review from tbursztyka as a code owner Nov 3, 2021
@rlubos
Copy link
Contributor Author

@rlubos rlubos commented Nov 3, 2021

@jukkar I've noticed this wrong behaviour when testing big_http_download sample with overlay-e1000.conf, the initial TCP SYN retransmissions caused also ARP packet retransmission.

What bugs me however is the reason of the retransmissions at the TCP level. I've noticed, that e1000 ethernet driver will not process any incoming packets for about 1 second after it's initialized (no e1000_isr calls for initial ARP resposnes). I see in the wireshark that ARP responses are on the wire, but the initial ones are not processed by the driver. Is this a known issue? Any idea what could be causing it? I failed to figure this out on my own, I have little knowledge of this device.

jukkar
jukkar approved these changes Nov 3, 2021
@jukkar
Copy link
Member

@jukkar jukkar commented Nov 3, 2021

I see in the wireshark that ARP responses are on the wire, but the initial ones are not processed by the driver. Is this a known issue?

I do not recall such a problem, this could be related to some qemu issue eating first packets instead of delivering those to the driver.

@cfriedt cfriedt merged commit c479458 into zephyrproject-rtos:main Nov 12, 2021
16 checks passed
@ycsin
Copy link
Collaborator

@ycsin ycsin commented Feb 10, 2022

@rlubos should this be backported?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking backport v2.7-branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants