On Mon, Mar 16, 2015 at 10:40:26AM -0700, Ansis Atteka wrote: > On Fri, Mar 13, 2015 at 11:31 AM, Ben Pfaff <[email protected]> wrote: > > On Tue, Mar 10, 2015 at 09:28:22PM -0700, Ansis Atteka wrote: > >> On Tue, Mar 10, 2015 at 1:37 PM, Ben Pfaff <[email protected]> wrote: > >> > Otherwise the policing limits could make no sense if large rates were > >> > specified. > >> > > >> > Reported-by: Zhangguanghui <[email protected]> > >> > Signed-off-by: Ben Pfaff <[email protected]> > >> > --- > >> > AUTHORS | 1 + > >> > lib/netdev-linux.c | 13 +++++++------ > >> > vswitchd/bridge.c | 4 ++-- > >> > 3 files changed, 10 insertions(+), 8 deletions(-) > >> > > >> > diff --git a/AUTHORS b/AUTHORS > >> > index 6e8b4da..00627ef 100644 > >> > --- a/AUTHORS > >> > +++ b/AUTHORS > >> > @@ -344,6 +344,7 @@ Voravit T. [email protected] > >> > Yeming Zhao [email protected] > >> > Ying Chen [email protected] > >> > Yongqiang Liu [email protected] > >> > +Zhangguanghui [email protected] > >> > Ziyou Wang [email protected] > >> > Zolt??n Balogh [email protected] > >> > ankur dwivedi [email protected] > >> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > >> > index 920c884..199e6ac 100644 > >> > --- a/lib/netdev-linux.c > >> > +++ b/lib/netdev-linux.c > >> > @@ -1,5 +1,5 @@ > >> > /* > >> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > >> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. > >> > * > >> > * Licensed under the Apache License, Version 2.0 (the "License"); > >> > * you may not use this file except in compliance with the License. > >> > @@ -405,8 +405,8 @@ static struct tcmsg *tc_make_request(const struct > >> > netdev *, int type, > >> > unsigned int flags, struct ofpbuf > >> > *); > >> > static int tc_transact(struct ofpbuf *request, struct ofpbuf **replyp); > >> > static int tc_add_del_ingress_qdisc(struct netdev *netdev, bool add); > >> > -static int tc_add_policer(struct netdev *netdev, int kbits_rate, > >> > - int kbits_burst); > >> > +static int tc_add_policer(struct netdev *, > >> > + uint32_t kbits_rate, uint32_t kbits_burst); > >> > > >> > static int tc_parse_qdisc(const struct ofpbuf *, const char **kind, > >> > struct nlattr **options); > >> > @@ -4525,7 +4525,8 @@ tc_add_del_ingress_qdisc(struct netdev *netdev, > >> > bool add) > >> > * Returns 0 if successful, otherwise a positive errno value. > >> > */ > >> > static int > >> > -tc_add_policer(struct netdev *netdev, int kbits_rate, int kbits_burst) > >> > +tc_add_policer(struct netdev *netdev, > >> > + uint32_t kbits_rate, uint32_t kbits_burst) > >> > { > >> > struct tc_police tc_police; > >> > struct ofpbuf request; > >> > @@ -4539,8 +4540,8 @@ tc_add_policer(struct netdev *netdev, int > >> > kbits_rate, int kbits_burst) > >> > tc_police.action = TC_POLICE_SHOT; > >> > tc_police.mtu = mtu; > >> > tc_fill_rate(&tc_police.rate, ((uint64_t) kbits_rate * 1000)/8, > >> > mtu); > >> > - tc_police.burst = tc_bytes_to_ticks(tc_police.rate.rate, > >> > - kbits_burst * 1024); > >> > + tc_police.burst = tc_bytes_to_ticks( > >> > + tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * > >> > 1024); > >> > >> Maybe silly questions, since I actually did not try out the code, but > >> 1. is it correct that above kbits_burst is multiplied with 1024, but > >> in tc_fill_rate() kbits_rate is multiplied with 1000? > >> 2. also, tc_fill_rate() above seems to init tc_police.rate in bytes > >> not bits, but tc_bytes_to_ticks() expects both arguments to be in same > >> units to cancel out? > > > > It's definitely weird, but testing shows that it does the right thing > > at least from "tc filter show"'s point of view. How about if I add > > this comment? > > > > /* 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. > > * > > * 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, so while this actually ends up doing the right thing > > * from tc's point of view. Whatever. */ > > > Acked-by: Ansis Atteka <[email protected]>
Thanks, applied with that additional comment. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
