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: ipv4: Fix ARP probe check in address conflict detection #80433

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

anhuba
Copy link
Contributor

@anhuba anhuba commented Oct 25, 2024

The second condition needs to check ARP probes only

Fixes #80429

Copy link

Hello @anhuba, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

subsys/net/ip/ipv4_acd.c Outdated Show resolved Hide resolved
@jukkar
Copy link
Member

jukkar commented Oct 25, 2024

Could you also add text from your issue to the commit message, at least this could be put there

The ACD is not properly implemented as described in RFC5227 ch. 2.1.1
The implementation incorrectly detects an IP conflict, if an ARP request is received for the target IP.
The reason is that the current implementation checks for ARP requests instead of ARP probes.

Copy link
Contributor

@rlubos rlubos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix itself looks good, but the branch needs some cleanup:

  • Please use rebase instead of merge, no merge commits allowed
  • The commit message in the fix commit is missing signoff entry.

@rlubos
Copy link
Contributor

rlubos commented Oct 28, 2024

Please squash the commits (and add signoff entry).

@anhuba
Copy link
Contributor Author

anhuba commented Oct 28, 2024

changed commit message and elminated local variable

@rlubos
Copy link
Contributor

rlubos commented Oct 28, 2024

Some more issues with the commit message:

6: UC4 Commit message body line exceeds max length (103>75): "The implementation incorrectly detects an IP conflict, if an ARP request is received for the target IP."
7: UC4 Commit message body line exceeds max length (92>75): "The reason is that the current implementation checks for ARP requests instead of ARP probes."

f453f3b45490a2d7a5d45d727b4609dd97cac815: author email (anhuba <113986680+anhuba@users.noreply.github.com>) needs to match one of the signed-off-by entries.
f453f3b45490a2d7a5d45d727b4609dd97cac815: author email (anhuba <113986680+anhuba@users.noreply.github.com>) does not follow the syntax: First Last .

The second condition needs to check ARP probes only

The ACD is not properly implemented as described in RFC5227 ch. 2.1.1
The implementation incorrectly detects an IP conflict, if an ARP request
is received for the target IP.
The reason is that the current implementation checks for ARP requests
instead of ARP probes.

Signed-off-by: Andreas Huber <andreas.huber@ch.sauter-bc.com>
@jukkar jukkar added backport v3.7-branch Request backport to the v3.7-branch and removed platform: STM32 ST Micro STM32 labels Oct 28, 2024
@jukkar jukkar added this to the v4.0.0 milestone Oct 29, 2024
@mmahadevan108 mmahadevan108 merged commit 27d93f8 into zephyrproject-rtos:main Oct 29, 2024
30 checks passed
Copy link

Hi @anhuba!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

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

Successfully merging this pull request may close these issues.

Bug in ACD implementation RFC5227
5 participants