mirror of
https://github.com/openvswitch/ovs
synced 2025-08-31 14:25:26 +00:00
netdev-linux: Fix ingress policing burst rate configuration via tc
The tc_police structure was filled with a value calculated in bits instead of bytes while bytes were expected. This led the setting of an x8 higher burst value. Documentation and defaults have been corrected accordingly to minimize nuisances on users sticking to the defaults. The suggested burst value is now 80% of policing rate to make sure TCP works correctly. Signed-off-by: Miguel Angel Ajo <majopela@redhat.com> Tested-by: Miguel Angel Ajo <majopela@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This commit is contained in:
committed by
Ben Pfaff
parent
f0ae4e6359
commit
79abacc880
@@ -2045,7 +2045,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
|
||||
int error;
|
||||
|
||||
kbits_burst = (!kbits_rate ? 0 /* Force to 0 if no rate specified. */
|
||||
: !kbits_burst ? 1000 /* Default to 1000 kbits if 0. */
|
||||
: !kbits_burst ? 8000 /* Default to 8000 kbits if 0. */
|
||||
: kbits_burst); /* Stick with user-specified value. */
|
||||
|
||||
ovs_mutex_lock(&netdev->mutex);
|
||||
@@ -4720,21 +4720,15 @@ tc_add_policer(struct netdev *netdev,
|
||||
tc_police.mtu = mtu;
|
||||
tc_fill_rate(&tc_police.rate, ((uint64_t) kbits_rate * 1000)/8, mtu);
|
||||
|
||||
/* The following appears wrong in two ways:
|
||||
*
|
||||
* - tc_bytes_to_ticks() should take "bytes" as quantity for both of its
|
||||
* arguments (or at least consistently "bytes" as both or "bits" as
|
||||
* both), but this supplies bytes for the first argument and bits for the
|
||||
* second.
|
||||
*
|
||||
* - In networking a kilobit is usually 1000 bits but this uses 1024 bits.
|
||||
/* The following appears wrong in one way: In networking a kilobit is
|
||||
* usually 1000 bits but this uses 1024 bits.
|
||||
*
|
||||
* However if you "fix" those problems then "tc filter show ..." shows
|
||||
* "125000b", meaning 125,000 bits, when OVS configures it for 1000 kbit ==
|
||||
* 1,000,000 bits, whereas this actually ends up doing the right thing from
|
||||
* tc's point of view. Whatever. */
|
||||
tc_police.burst = tc_bytes_to_ticks(
|
||||
tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024);
|
||||
tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
|
||||
|
||||
tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
|
||||
NLM_F_EXCL | NLM_F_CREATE, &request);
|
||||
|
Reference in New Issue
Block a user