On Thu, Mar 25, 2021 at 03:51:11PM -0300, Marcelo Ricardo Leitner wrote: > On Fri, Mar 12, 2021 at 12:59:16PM +0100, Simon Horman wrote: > > From: "Yong.Xu" <yong...@corigine.com> > > > > Correct calculation of burst parameter used when configuring TC policer > > action for ingress port-based policing in the case where TC offload is in > > use. This now matches the value calculated for the case where TC offload is > > not in use. > > > > The division by 8 is to convert from bits to bytes. > > Its unclear why 64 was previously used. > > Yeah.. I have the feeling that it might be related to kernel's: > /* Avoid doing 64 bit divide */ > #define PSCHED_SHIFT 6 > #define PSCHED_TICKS2NS(x) ((s64)(x) << PSCHED_SHIFT) > #define PSCHED_NS2TICKS(x) ((x) >> PSCHED_SHIFT) > but I can't confirm it. > > > > > Fixes: e7f6ba220 ("lib/tc: add ingress ratelimiting support for tc-offload") > > Signed-off-by: Yong.Xu <yong...@corigine.com> > > [simon: reworked changelog] > > Signed-off-by: Simon Horman <simon.hor...@netronome.com> > > Signed-off-by: Louis Peens <louis.pe...@netronome.com> > > --- > > lib/netdev-linux.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > > index 15b25084b..f87a20075 100644 > > --- a/lib/netdev-linux.c > > +++ b/lib/netdev-linux.c > > @@ -2572,7 +2572,7 @@ exit: > > static struct tc_police > > tc_matchall_fill_police(uint32_t kbits_rate, uint32_t kbits_burst) > > { > > - unsigned int bsize = MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 64; > > + unsigned int bsize = MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8; > > unsigned int bps = ((uint64_t) kbits_rate * 1000) / 8; > > I know that the patch is not changing this but, while at this, why for > bsize the 'k' is 1024, while for bps it's 1000? > > AFAICT it backtracks to > netdev_set_policing(iface->netdev, > MIN(UINT32_MAX, iface->cfg->ingress_policing_rate), > MIN(UINT32_MAX, > iface->cfg->ingress_policing_burst)); > in iface_configure_qos() and I don't see a reason for them being > different. > > qos.rst states: > ``ingress_policing_rate`` > the maximum rate (in Kbps) that this VM should be allowed to send > > ``ingress_policing_burst`` > a parameter to the policing algorithm to indicate the maximum amount of data > (in Kb) that this interface can send beyond the policing rate. > > Both with capital K. So if the 64 was bothering, the 1024 is likely > doing so as well.
While vswitchd/vswitch.xml says: <column name="ingress_policing_rate"> <p> Maximum rate for data received on this interface, in kbps. Data <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 8000 kbit. This value I'm not sure which one has preference here. > > Nevertheless, as the patch is fixing the /64, patch LGTM. > > > struct tc_police police; > > struct tc_ratespec rate; > > -- > > 2.20.1 > > > > _______________________________________________ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev