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

feat: add command to enable mptcp #4014

Merged
merged 2 commits into from
Nov 1, 2024
Merged
Changes from all commits
Commits
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
28 changes: 28 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,36 @@ enable-bbr:
fi
.PHONY: enable-bbr

## enable-mptcp: Enable mptcp over multiple ports (not interfaces). Only works on Linux Kernel 5.6 and above.
enable-mptcp:
@echo "Configuring system to use mptcp..."
@sudo sysctl -w net.mptcp.enabled=1
@sudo sysctl -w net.mptcp.mptcp_path_manager=ndiffports
@sudo sysctl -w net.mptcp.mptcp_ndiffports=16
@echo "Making MPTCP settings persistent across reboots..."
@echo "net.mptcp.enabled=1" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_path_manager=ndiffports" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_ndiffports=16" | sudo tee -a /etc/sysctl.conf
@echo "MPTCP configuration complete and persistent!"

.PHONY: enable-mptcp
Comment on lines +276 to +288
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add safety checks and improve error handling for enable-mptcp target

The current implementation has several potential issues:

  1. Missing kernel version check despite requiring Linux 5.6+
  2. No error handling for sudo operations
  3. No validation that MPTCP is supported
  4. Direct modification of system files without backup

Consider implementing these safety improvements:

 ## enable-mptcp: Enable mptcp over multiple ports (not interfaces). Only works on Linux Kernel 5.6 and above.
 enable-mptcp:
+	@if [ "$$(uname)" != "Linux" ]; then \
+		echo "Error: MPTCP configuration is only supported on Linux systems"; \
+		exit 1; \
+	fi
+	@if [ "$$(uname -r | cut -d. -f1,2)" \< "5.6" ]; then \
+		echo "Error: MPTCP requires Linux kernel 5.6 or higher"; \
+		exit 1; \
+	fi
+	@if ! command -v sudo >/dev/null 2>&1; then \
+		echo "Error: sudo is required to modify system settings"; \
+		exit 1; \
+	fi
 	@echo "Configuring system to use mptcp..."
+	@sudo cp /etc/sysctl.conf /etc/sysctl.conf.backup
 	@sudo sysctl -w net.mptcp.enabled=1
 	@sudo sysctl -w net.mptcp.mptcp_path_manager=ndiffports
 	@sudo sysctl -w net.mptcp.mptcp_ndiffports=16
 	@echo "Making MPTCP settings persistent across reboots..."
 	@echo "net.mptcp.enabled=1" | sudo tee -a /etc/sysctl.conf
 	@echo "net.mptcp.mptcp_path_manager=ndiffports" | sudo tee -a /etc/sysctl.conf
 	@echo "net.mptcp.mptcp_ndiffports=16" | sudo tee -a /etc/sysctl.conf
 	@echo "MPTCP configuration complete and persistent!"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## enable-mptcp: Enable mptcp over multiple ports (not interfaces). Only works on Linux Kernel 5.6 and above.
enable-mptcp:
@echo "Configuring system to use mptcp..."
@sudo sysctl -w net.mptcp.enabled=1
@sudo sysctl -w net.mptcp.mptcp_path_manager=ndiffports
@sudo sysctl -w net.mptcp.mptcp_ndiffports=16
@echo "Making MPTCP settings persistent across reboots..."
@echo "net.mptcp.enabled=1" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_path_manager=ndiffports" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_ndiffports=16" | sudo tee -a /etc/sysctl.conf
@echo "MPTCP configuration complete and persistent!"
.PHONY: enable-mptcp
## enable-mptcp: Enable mptcp over multiple ports (not interfaces). Only works on Linux Kernel 5.6 and above.
enable-mptcp:
@if [ "$$(uname)" != "Linux" ]; then \
echo "Error: MPTCP configuration is only supported on Linux systems"; \
exit 1; \
fi
@if [ "$$(uname -r | cut -d. -f1,2)" \< "5.6" ]; then \
echo "Error: MPTCP requires Linux kernel 5.6 or higher"; \
exit 1; \
fi
@if ! command -v sudo >/dev/null 2>&1; then \
echo "Error: sudo is required to modify system settings"; \
exit 1; \
fi
@echo "Configuring system to use mptcp..."
@sudo cp /etc/sysctl.conf /etc/sysctl.conf.backup
@sudo sysctl -w net.mptcp.enabled=1
@sudo sysctl -w net.mptcp.mptcp_path_manager=ndiffports
@sudo sysctl -w net.mptcp.mptcp_ndiffports=16
@echo "Making MPTCP settings persistent across reboots..."
@echo "net.mptcp.enabled=1" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_path_manager=ndiffports" | sudo tee -a /etc/sysctl.conf
@echo "net.mptcp.mptcp_ndiffports=16" | sudo tee -a /etc/sysctl.conf
@echo "MPTCP configuration complete and persistent!"
.PHONY: enable-mptcp

Copy link
Member

Choose a reason for hiding this comment

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

these seem to be interesting suggestions, but optional


## disable-mptcp: Disables mptcp over multiple ports. Only works on Linux Kernel 5.6 and above.
disable-mptcp:
Copy link
Member

Choose a reason for hiding this comment

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

would be nice to have a disable-bbr too :D

@echo "Disabling MPTCP..."
@sudo sysctl -w net.mptcp.enabled=0
@sudo sysctl -w net.mptcp.mptcp_path_manager=default
@echo "Removing MPTCP settings from /etc/sysctl.conf..."
@sudo sed -i '/net.mptcp.enabled=1/d' /etc/sysctl.conf
@sudo sed -i '/net.mptcp.mptcp_path_manager=ndiffports/d' /etc/sysctl.conf
@sudo sed -i '/net.mptcp.mptcp_ndiffports=16/d' /etc/sysctl.conf
@echo "MPTCP configuration reverted!"

.PHONY: disable-mptcp

## debug-version: Print the git tag and version.
debug-version:
@echo "GIT_TAG: $(GIT_TAG)"
@echo "VERSION: $(VERSION)"
.PHONY: debug-version

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Loading