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 <[email protected]> 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 <[email protected]>
> Tested-by: Miguel Angel Ajo <[email protected]>
> ---
>  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
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to