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

Add support for IPv6 #93

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Add support for IPv6 #93

merged 4 commits into from
Sep 25, 2024

Conversation

mateuszkobak
Copy link
Contributor

@mateuszkobak mateuszkobak commented Apr 15, 2024 •
edited
Loading

Description

Provide IPv6 support. This includes:

  • Some issues in lib-lwip (see PR that this one depends on)
  • Fixing a bug in IPv6 ioctls
  • Changing multicast filter flags in RTL8139 driver
  • Provide IPv6 autonfiguration

For now I was working only on ia32-generic-qemu.

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: ia32 (list targets here).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

github-actions[bot]
github-actions bot reviewed Apr 15, 2024
View reviewed changes
include/default-opts/lwipopts.h Outdated Show resolved Hide resolved
port/devs.c Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 15, 2024 •
edited
Loading

Unit Test Results

7 700 tests  +240   6 985 ✅ +240   37m 26s ⏱️ -16s
  436 suites + 16     715 💤 ±  0 
    1 files   ±  0       0 ❌ ±  0 

Results for commit e7eadf0. ± Comparison against base commit b1ab9f6.

♻️ This comment has been updated with latest results.

@mateuszkobak mateuszkobak force-pushed the mateuszkobak/ipv6 branch 2 times, most recently from 18173ae to 87e19e1 Compare April 16, 2024 08:58
@mateuszkobak mateuszkobak marked this pull request as ready for review April 16, 2024 11:05
@mateuszkobak mateuszkobak marked this pull request as draft April 16, 2024 11:06
This was referenced Apr 17, 2024
Merged
Merged
@mateuszkobak mateuszkobak force-pushed the mateuszkobak/ipv6 branch 2 times, most recently from 63cf42b to d5840e9 Compare April 18, 2024 13:00
@mateuszkobak mateuszkobak marked this pull request as ready for review April 18, 2024 13:09
nalajcie
nalajcie reviewed Apr 18, 2024
View reviewed changes
Copy link
Member

@nalajcie nalajcie left a comment

Choose a reason for hiding this comment

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

small nitpicks, otherwise LGTM

port/devs.c Outdated Show resolved Hide resolved
include/default-opts/lwipopts.h Outdated Show resolved Hide resolved
drivers/netif-driver.c Outdated Show resolved Hide resolved
drivers/netif-driver.c Show resolved Hide resolved
@mateuszkobak mateuszkobak force-pushed the mateuszkobak/ipv6 branch 2 times, most recently from 818766a to 565b7ba Compare April 18, 2024 15:05
@agkaminski agkaminski requested a review from nalajcie May 27, 2024 10:18
@anglov
Copy link
Member

anglov commented Sep 25, 2024

Please update submodule update

@mateuszkobak @anglov
port/sockets.c: Fix IPv6 ioctls
b2fe81d 
Ioctls should not change in_data, so IPv6 addresses should be copied before conversion.

JIRA: COG-42
@mateuszkobak @anglov
drivers/rtl8139cp.c: Change multicast filters
5dd5df4 
Changed filters from declining all to accepting all

JIRA: COG-42
@mateuszkobak @anglov
Add IPv6 address autoconfiguration
2b510da 
JIRA: COG-42
@mateuszkobak @anglov
Update submodule
e7eadf0 
 * lib-lwip e05ec1e1...ce151524 (2):
   > raw_input: remove IPv6 header when using raw socket
   > lwip_sendto: add condition for allocating new buffer

JIRA: COG-42
@anglov anglov merged commit 4896655 into master Sep 25, 2024
30 checks passed
@anglov anglov deleted the mateuszkobak/ipv6 branch September 25, 2024 15:45
@badochov
Copy link
Contributor

badochov commented Oct 4, 2024 •
edited
Loading

This introduced bible length of warnings, due to ipv6 code from lwip breaking -Wchar-subscripts(enabled by -Wall). I dunno why the warning are not present in this build but appears on project master

https://github.com/phoenix-rtos/phoenix-rtos-project/actions/runs/11162531975/job/31027392614

Imho we should turn off this warn in lwip

@anglov
Copy link
Member

anglov commented Oct 4, 2024

It should be revised

@mateuszkobak will take a look at it. Some warnings are caused by invalid port (our) code. Some maybe not.

Please do not disable warnings without a caution.

@mateuszkobak
Copy link
Contributor Author

@badochov @anglov, concerning these -Wchar-subscripts warnings, I have chacked that in lwip-2.2.0 they still use int8 (they alias it as s8_t) for indexing arrays. I have investigated some of these situations and I have seen that they make sure that indexing variable is >=0 before indexing the array. So I understand that lwip authors know what they are doing here and I support ignoring those warings in lwip.

Regarding other two (if I am not mistaken) warnings I will take care of them. One is in lib-lwip and one in our port code.

@mateuszkobak mateuszkobak mentioned this pull request Oct 7, 2024
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@nalajcie nalajcie nalajcie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mateuszkobak @anglov @badochov @nalajcie