Project

General

Profile

Actions

Bug #13157

open

maxbw can kill link with hires tick

Added by Ryan Zezeski almost 2 years ago. Updated almost 2 years ago.

Status:
New
Priority:
Normal
Assignee:
Category:
networking
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

The new default hires tick (#11499) can cause certain maxbw values to freeze a link's Rx SRS.

The maxbw setting is presented to the user as a per-second enforcement, e.g. 5M means 5Mbps. However, the actual enforcement in the kernel is done in terms of bytes-per-tick. This means a conversion must happen from Mbps to Bpt. Set maxbw low enough and your hz rate could bring the kernel's bw limit below the MTU of the link, leading to a frozen link. Because of this there is a check to make sure the user sets maxbw high enough to allow one MTU frame per tick. But this check is hard-coded for both 100hz and 1500 MTU. The simple solution is to fix the check to use the actual hz value and actual MTU value. However, because of this discrepancy between user-maxbw and kernel-maxbw, the higher the hz and MTU, the larger the minimum maxbw must be. I have a patch to modify the kernel enforcement to actually enforce the limit the way the user expects, but its not ready yet and we need a fix to keep people from freezing their links in the interim. The way I see it we have two choices.

1. The technically correct but potentially breaking to users way: fix the user-land check and don't allow any maxbw that would lead to a per-tick enforcement of less than MTU. Given there are potentially users with persistent link props of maxbw below 12M (higher for larger MTUs), then this will break them.

2. Non-breaking way that potentially allows more traffic than maxbw prescribes: implicitly raise kernel-side limit to MTU-frame-per-tick. User's persistent props don't break and we fix the freeze.

I lean towards number 2 because I have a series of changes that fix maxbw as a whole and would allow users to set arbitrarily low limits (like frame-per-second). Otherwise, we have to potentially break current users only to once again allow those lower limits in the future. Since this only affects users of "small" maxbw (anything below hz * MTU * 8), I think this is an acceptable trade off for now.

Actions

Also available in: Atom PDF