Thanks, I see the re-sent version.
On Thu, Apr 14, 2016 at 09:55:50AM +0200, Miguel Angel Ajo Pelayo wrote: > Ouch, I wonder what happened. I will retry, sorry about it. > > On Thu, Apr 14, 2016 at 1:08 AM, Ben Pfaff <b...@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 <majop...@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 <majop...@redhat.com> > >> > Tested-by: Miguel Angel Ajo <majop...@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@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
