[ovs-dev] [PATCH] netdev-linux: Fix ingress policing burst rate configuration via tc
Miguel Angel Ajo Pelayo
majopela at redhat.com
Thu Apr 14 07:55:50 UTC 2016
Ouch, I wonder what happened. I will retry, sorry about it.
On Thu, Apr 14, 2016 at 1:08 AM, Ben Pfaff <blp at ovn.org> wrote:
> Thanks for working on this. The original patch, as opposed to the
> followup quoted below) doesn't seem to have made it to the mailing list
> archive or to patchwork. Can you resend it?
>
> On Wed, Apr 13, 2016 at 12:49:28PM +0200, Miguel Angel Ajo Pelayo wrote:
>> I verified the 10% / 80% changes with netperf, it's unable to sustain 10Mbps
>> without at least a 8Mbit burst. I've seen recommendations from cisco to use
>> policing bursts of around 150 to 200%, but I guess that depends on the path
>> latency or even the implementation.
>>
>> My netperf tests were in-host.
>>
>>
>>
>> On Wed, Apr 13, 2016 at 12:44 PM, Miguel Angel Ajo <majopela at redhat.com> wrote:
>> > 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 acordingly 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 at redhat.com>
>> > Tested-by: Miguel Angel Ajo <majopela at redhat.com>
>> > ---
>> > FAQ.md | 2 +-
>> > lib/netdev-linux.c | 14 ++++----------
>> > vswitchd/vswitch.xml | 4 ++--
>> > 3 files changed, 7 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/FAQ.md b/FAQ.md
>> > index 04ffc84..7dcdb4c 100644
>> > --- a/FAQ.md
>> > +++ b/FAQ.md
>> > @@ -1124,7 +1124,7 @@ A: A policing policy can be configured on an interface to drop packets
>> > generate to 10Mbps:
>> >
>> > ovs-vsctl set interface vif1.0 ingress_policing_rate=10000
>> > - ovs-vsctl set interface vif1.0 ingress_policing_burst=1000
>> > + ovs-vsctl set interface vif1.0 ingress_policing_burst=8000
>> >
>> > Traffic policing can interact poorly with some network protocols and
>> > can have surprising results. The "Ingress Policing" section of
>> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> > index a7d7ac7..a2b6b8a 100644
>> > --- a/lib/netdev-linux.c
>> > +++ b/lib/netdev-linux.c
>> > @@ -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);
>> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> > index 7d6976f..fca238d 100644
>> > --- a/vswitchd/vswitch.xml
>> > +++ b/vswitchd/vswitch.xml
>> > @@ -2434,7 +2434,7 @@
>> >
>> > <column name="ingress_policing_burst">
>> > <p>Maximum burst size for data received on this interface, in kb. The
>> > - default burst size if set to <code>0</code> is 1000 kb. This value
>> > + default burst size if set to <code>0</code> is 8000 kbit. This value
>> > has no effect if <ref column="ingress_policing_rate"/>
>> > is <code>0</code>.</p>
>> > <p>
>> > @@ -2442,7 +2442,7 @@
>> > which is important for protocols like TCP that react severely to
>> > dropped packets. The burst size should be at least the size of the
>> > interface's MTU. Specifying a value that is numerically at least as
>> > - large as 10% of <ref column="ingress_policing_rate"/> helps TCP come
>> > + large as 80% of <ref column="ingress_policing_rate"/> helps TCP come
>> > closer to achieving the full rate.
>> > </p>
>> > </column>
>> > --
>> > 1.8.3.1
>> >
>> > This patch fixes the burst setting of the ingress policing on netdev-linux, and
>> > corrects the documentation recommendations accordingly.
>> >
>> > Before the patch we had:
>> >
>> >
>> > # ovs-vsctl -- --may-exist add-port br-int test-port -- set Interface test-port type=internal
>> > # ovs-vsctl set interface test-port ingress_policing_rate=10000
>> > # ovs-vsctl set interface test-port ingress_policing_burst=1000
>> > # tc filter show dev test-port parent ffff:
>> > filter protocol all pref 49 basic
>> > filter protocol all pref 49 basic handle 0x1
>> > police 0x1 rate 10000Kbit burst 1000Kb mtu 64Kb action drop overhead 0b
>> > ref 1 bind 1
>> >
>> > After the patch we have:
>> >
>> > # ovs-vsctl set interface test-port ingress_policing_rate=10000
>> > # ovs-vsctl set interface test-port ingress_policing_burst=1000
>> > # tc filter show dev test-port parent ffff:
>> > filter protocol all pref 49 basic
>> > filter protocol all pref 49 basic handle 0x1
>> > police 0x2 rate 10000Kbit burst 125Kb mtu 64Kb action drop overhead 0b
>> > ref 1 bind 1
>> >
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list