Skip to content

YAML topology support#7

Open
Hraf-Jr wants to merge 10 commits intoqdeconinck:minitopo2from
Hraf-Jr:minitopo2
Open

YAML topology support#7
Hraf-Jr wants to merge 10 commits intoqdeconinck:minitopo2from
Hraf-Jr:minitopo2

Conversation

@Hraf-Jr
Copy link

@Hraf-Jr Hraf-Jr commented Nov 12, 2025

Summary

This Pull Request introduces YAML support for topology configuration files.
The runner automatically detects the input format and loads the appropriate parser.

Changes

  • Added YAML parsing support in runner.py
  • Added YAML versions of the existing topologies (topo_1.yaml to topo_5.yaml)
  • Updated README.rst with a short YAML example and usage instructions
  • Preserved full backward compatibility with legacy
  • Minor cleanup and refactoring inside the runner to support both formats

Rationale

YAML files provide a clearer structure, improved readability, and make it easier
to extend topology descriptions in future contributions.
This PR introduces YAML support without modifying the default behavior for
existing users.

Testing

Tested using:

sudo python3 runner.py -t config/topo/topo_2.yaml

Copy link
Owner

@qdeconinck qdeconinck left a comment

Choose a reason for hiding this comment

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

This PR is not ready for merging. There are too many unrelated changes and additions compared to the claimed contribution of this PR. You should rework your commits to have a cleaner history, possibly squashing them.

Also, please keep overall communications in English, including the PR messages.

README.rst Outdated

Usage
=====
Each experiment is defined by a *topology file* (``.para``) and an *experiment file* (``.xp``)
Copy link
Owner

Choose a reason for hiding this comment

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

There is no .para nor .xp file extensions here.

README.rst Outdated
Comment on lines 45 to 50
cargo build --release --bin http3-server --bin http3-client

After compilation, you can use the following binaries in your experiment files:

- ``target/release/quiche-server``
- ``target/release/quiche-client``
Copy link
Owner

Choose a reason for hiding this comment

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

This command does not generate the quiche-client and quiche-server files. It should instead build quiche-client and quiche-server (not http3-client and http3-server).

README.rst Outdated
basic Example
=============
This section describes a complete example showing how to establish
a **SP QUIC** connection using this framework.
Copy link
Owner

Choose a reason for hiding this comment

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

SP = ?

README.rst Outdated
1. Launch the topology
----------------------

Start a multi-interface topology (in our case ``topo_2``) **without any experiment**
Copy link
Owner

Choose a reason for hiding this comment

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

It would make more sense to describe a bit the network topology that this file is creating.

README.rst Outdated
This opens the interactive Mininet CLI with the nodes already connected
(client, router, server).

---
Copy link
Owner

Choose a reason for hiding this comment

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

The remaining sections sounds overly complicated in a README. Why not having all these interactions in the experience file directly? Maybe you could provide a simpler case than running a QUIC experiment if you want to show some CLI interactions.

Comment on lines -54 to +135
"""
Matches the name of the topo and find the corresponding Topo class.
"""
"""Matches the name of the topo and find the corresponding Topo class."""
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove such similar changes that are unrelated to the YAML support.

Copy link
Owner

Choose a reason for hiding this comment

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

Please remove that file.

Copy link
Owner

Choose a reason for hiding this comment

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

Please remove that file.

Copy link
Owner

Choose a reason for hiding this comment

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

Please discard all changes to that file.

Copy link
Owner

Choose a reason for hiding this comment

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

I did not reviewed that file, though it appear to have several flaws already raised in my previous comments (unneeded changes, inclusion of non-English comments,...).

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.

2 participants