Skip to content

Hybla congestion control #1853

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Hybla congestion control #1853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ set(PICOQUIC_LIBRARY_FILES
picoquic/cubic.c
picoquic/fastcc.c
picoquic/frames.c
picoquic/hybla.c
picoquic/intformat.c
picoquic/logger.c
picoquic/logwriter.c
Expand Down Expand Up @@ -337,6 +338,7 @@ target_link_libraries(picoquic-core
PRIVATE
${OPENSSL_LIBRARIES}
${MBEDTLS_LIBRARIES}
m
PUBLIC
${PTLS_LIBRARIES}
Threads::Threads)
Expand Down
13 changes: 12 additions & 1 deletion picoquic/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ static option_table_line_t option_table[] = {
{ picoquic_option_DisablePortBlocking, 'X', "disable_block", 0, "", "Disable the check for blocked ports"},
{ picoquic_option_SOLUTION_DIR, 'S', "solution_dir", 1, "folder", "Set the path to the source files to find the default files" },
{ picoquic_option_CC_ALGO, 'G', "cc_algo", 1, "cc_algorithm",
"Use the specified congestion control algorithm: reno, cubic, bbr or fast. Defaults to bbr." },
"Use the specified congestion control algorithm: reno, cubic, bbr, fast, or hybla. Defaults to bbr." },
{ picoquic_option_HYBLA_RTT0, 'H', "rtt0", 1, "number", "Set RTT0 parameter for Hybla congestion control."},
{ picoquic_option_SPINBIT, 'P', "spinbit", 1, "number", "Set the default spinbit policy" },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please take a look at PR #1856 which removes the need to manually update that message.

{ picoquic_option_LOSSBIT, 'O', "lossbit", 1, "number", "Set the default lossbit policy" },
{ picoquic_option_MULTIPATH, 'M', "multipath", 0, "", "Enable QUIC multipath extension" },
Expand Down Expand Up @@ -291,6 +292,16 @@ static int config_set_option(option_table_line_t* option_desc, option_param_t* p
case picoquic_option_CC_ALGO:
ret = config_set_string_param(&config->cc_algo_id, params, nb_params, 0);
break;
case picoquic_option_HYBLA_RTT0: {
int v = config_atoi(params, nb_params, 0, &ret);
if (ret != 0) {
fprintf(stderr, "Invalid rtt0 value: %d\n", v);
}
else {
picoquic_hybla_set_rtt0_param(v);
}
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that you need a parameter, but I would rather try to find a generic way to pass parameters to CC algorithms. Maybe something like "-g hybla:360".

I would also need some clarity on whether this parameter is required at both clients and servers, or just on one side of the connection. Is it possible to set a default value, and see it use by all connections served by a given server?

Copy link
Author

Choose a reason for hiding this comment

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

I can see how providing a Hybla-specific parameter as a Picoquic option can be a bit intrusive. As for your question, RTT0 is meant to be a one-sided parameter that influences how the local node calculates the congestion window.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My plan is to add a "string" (text encoded parameter) after the algorithm ID, using maybe a colon as a separator. Then each algorithm can encode whatever they want in that string, or nothing. The parameter would be passed in the "init" call when a new congestion control context starts.

I now people already want something like that for experimental changes to BBR.

case picoquic_option_SPINBIT: {
int v = config_atoi(params, nb_params, 0, &ret);
if (ret != 0 || v < 0 || v > (int)picoquic_spinbit_on) {
Expand Down
Loading
Loading