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

new config for ipv6 related changes #624

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

shotasilagadzetaal
Copy link
Collaborator

@shotasilagadzetaal shotasilagadzetaal commented Oct 28, 2024

Description of Changes

New config parameters for ipv6 related changes

Linked Issues / Tickets

Closes #ARCO-229

Testing Procedure

I have added test case to check if all the values are correctly read from config

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have updated CHANGELOG.md with my changes

@shotasilagadzetaal shotasilagadzetaal force-pushed the new-ipv6-config-changes branch 2 times, most recently from 8968c28 to ac1ea33 Compare October 29, 2024 07:47
config/defaults.go Outdated Show resolved Hide resolved
test/config/config.yaml Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
@kuba-4chain
Copy link
Collaborator

Nice work. I just have a question, because maybe I'm missing something, this config is a preparation for further changes to implement the IPv6 communications, but for now it's not yet being used anywhere, right?

@arkadiuszos4chain
Copy link
Collaborator

arkadiuszos4chain commented Oct 30, 2024

I see two potential approaches:

1. Use P2P for Multicasting

Not my preferred option, but workable.

  • Update the PeerConfig structure as follows:
    Peer:
      mode: unicast | multicast
      address | host: url
      port: number
    
  • Explanation:
    • if mode is unicast, the peer operates as it currently does, with no additional changes
    • if mode is multicast, the url must be an IPv6 multicast group address

2. Implement a Multicast Handler (that works alongside P2P)

  • This handler would have similar logic to the peer but be simpler (no reconnections, no ping-pong messages, no handshakes).

  • Add a new configuration key:

    broadcast | network-communication:
      Peers: []           # (optional) same as current setup
      Multicast:
        group: url
        port: number      # (optional)
        
  • Currently, it is unclear how groups would function in the network. Possible options include:

    • using a single multicast group with different ports to handle distinct topics
    • using a separate multicast group per topic
      therefore, the final schema of Multicast could change.

I believe that go-p2p is agnostic regarding IP version, so there is no need to introduce the IPv6enabled flag.

test/config/config.yaml Outdated Show resolved Hide resolved
@shotasilagadzetaal
Copy link
Collaborator Author

Nice work. I just have a question, because maybe I'm missing something, this config is a preparation for further changes to implement the IPv6 communications, but for now it's not yet being used anywhere, right?

yeap, exactly

Comment on lines 22 to 28
multicast:
ipv6Enabled: true # indicates whether ipv6 is enabled for multicasting
multicastGroups: # must be specified if mode = multicast
- "172.28.56.77" # address of multicast group, needs to be ipv6 address if ipv6 is enabled
interfaces:
- "eth0"
- "eth1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: This can be removed I think

Copy link

sonarcloud bot commented Nov 4, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants