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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from Nov 12, 2021
Merged

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

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

@rlubos
net: arp: Fix ARP retransmission source address selection
5e4c310 
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 November 3, 2021 12:34
@rlubos
Copy link
Contributor Author

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
Copy link
Member

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
@ycsin
Copy link
Collaborator

ycsin commented Feb 10, 2022

@rlubos should this be backported?

@rlubos rlubos added the backport v2.7-branch Request backport to the v2.7-branch label Feb 10, 2022
@zephyrbot zephyrbot mentioned this pull request Feb 10, 2022
Merged
@ycsin ycsin mentioned this pull request Feb 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jukkar jukkar jukkar approved these changes

@mniestroj mniestroj mniestroj approved these changes

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@mengxianglinx mengxianglinx Awaiting requested review from mengxianglinx

@pfalcon pfalcon Awaiting requested review from pfalcon

Assignees

@rlubos rlubos

Labels
area: Networking backport v2.7-branch Request backport to the v2.7-branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rlubos @jukkar @ycsin @mniestroj @cfriedt