Re: svn commit: r338253 - head/sbin/pfctl

2018-08-23 Thread Patrick Kelsey
Fixed in r338260.

On Thu, Aug 23, 2018 at 1:30 PM Patrick Kelsey  wrote:

>
>
> On Thu, Aug 23, 2018 at 1:05 PM Stefan Esser  wrote:
>
>> Am 23.08.18 um 18:10 schrieb Patrick Kelsey:
>> > Author: pkelsey
>> > Date: Thu Aug 23 16:10:28 2018
>> > New Revision: 338253
>> > URL: https://svnweb.freebsd.org/changeset/base/338253
>> >
>> > Log:
>> >   Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
>> >   higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
>> >   which is the highest standard rate that could be used prior to
>> >   r338209, so the default behavior for all existing systems should
>> >   remain the same.
>> >
>> >   The value of 128 chosen is a balance between being big enough to
>> >   reduce potential precision/quantization effects stemming from frequent
>> >   bucket refills over small time intervals and being small enough to
>> >   prevent a greedy driver from burst dequeuing more packets than it has
>> >   available hardware ring slots for whenever altq transitions from idle
>> >   to backlogged.
>> >
>> >   Reviewed by:jmallett, kp
>> >   MFC after:  2 weeks
>> >   Sponsored by:   RG Nets
>> >   Differential Revision: https://reviews.freebsd.org/D16852
>> >
>> > Modified:
>> >   head/sbin/pfctl/pfctl_altq.c
>> >
>> > Modified: head/sbin/pfctl/pfctl_altq.c
>> >
>> ==
>> > --- head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 15:01:27 2018
>> (r338252)
>> > +++ head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 16:10:28 2018
>> (r338253)
>> > @@ -299,8 +299,10 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa,
>> stru
>> >   size = 4;
>> >   else if (rate <= 200 * 1000 * 1000)
>> >   size = 8;
>> > - else
>> > + else if (rate <= 2500 * 1000 * 1000)
>> >   size = 24;
>> > + else
>> > + size = 128;
>> >   size = size * getifmtu(pa->ifname);
>> >   pa->tbrsize = size;
>> >   }
>> >
>>
>> This breaks the build on my amd64 box:
>>
>> /usr/svn/base/head/sbin/pfctl/pfctl_altq.c:302:32: error: overflow in
>> expression; result is -1794967296 with type 'int'
>> [-Werror,-Winteger-overflow]
>> else if (rate <= 2500 * 1000 * 1000)
>>  ^
>>
>> While "rate" is unsigned long, the expression being calculated is not ...
>>
>> Regards, STefan
>>
>
> Will fix shortly...not sure why I did not encounter this error in my setup.
>
> -Patrick
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r338253 - head/sbin/pfctl

2018-08-23 Thread Kristof Provost
This seems to have broken a couple of platforms (aarch64, riscv64, 
sparc64, powerpc at least):


	/usr/src/sbin/pfctl/pfctl_altq.c:302:32: error: overflow in expression; 
result is -1794967296 with type 'int' [-Werror,-Winteger-overflow]

--- all_subdir_lib ---
--- puts.po ---
--- all_subdir_sbin ---
else if (rate <= 2500 * 1000 * 1000)
 ^

Regards,
Kristof

On 23 Aug 2018, at 18:10, Patrick Kelsey wrote:


Author: pkelsey
Date: Thu Aug 23 16:10:28 2018
New Revision: 338253
URL: https://svnweb.freebsd.org/changeset/base/338253

Log:
  Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
  higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
  which is the highest standard rate that could be used prior to
  r338209, so the default behavior for all existing systems should
  remain the same.

  The value of 128 chosen is a balance between being big enough to
  reduce potential precision/quantization effects stemming from 
frequent

  bucket refills over small time intervals and being small enough to
  prevent a greedy driver from burst dequeuing more packets than it 
has
  available hardware ring slots for whenever altq transitions from 
idle

  to backlogged.

  Reviewed by:  jmallett, kp
  MFC after:2 weeks
  Sponsored by: RG Nets
  Differential Revision: https://reviews.freebsd.org/D16852

Modified:
  head/sbin/pfctl/pfctl_altq.c

Modified: head/sbin/pfctl/pfctl_altq.c
==
--- head/sbin/pfctl/pfctl_altq.cThu Aug 23 15:01:27 2018
(r338252)
+++ head/sbin/pfctl/pfctl_altq.cThu Aug 23 16:10:28 2018
(r338253)
@@ -299,8 +299,10 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa, 
stru

size = 4;
else if (rate <= 200 * 1000 * 1000)
size = 8;
-   else
+   else if (rate <= 2500 * 1000 * 1000)
size = 24;
+   else
+   size = 128;
size = size * getifmtu(pa->ifname);
pa->tbrsize = size;
}

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r338253 - head/sbin/pfctl

2018-08-23 Thread Stefan Esser
Am 23.08.18 um 18:10 schrieb Patrick Kelsey:
> Author: pkelsey
> Date: Thu Aug 23 16:10:28 2018
> New Revision: 338253
> URL: https://svnweb.freebsd.org/changeset/base/338253
> 
> Log:
>   Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
>   higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
>   which is the highest standard rate that could be used prior to
>   r338209, so the default behavior for all existing systems should
>   remain the same.
>   
>   The value of 128 chosen is a balance between being big enough to
>   reduce potential precision/quantization effects stemming from frequent
>   bucket refills over small time intervals and being small enough to
>   prevent a greedy driver from burst dequeuing more packets than it has
>   available hardware ring slots for whenever altq transitions from idle
>   to backlogged.
>   
>   Reviewed by:jmallett, kp
>   MFC after:  2 weeks
>   Sponsored by:   RG Nets
>   Differential Revision: https://reviews.freebsd.org/D16852
> 
> Modified:
>   head/sbin/pfctl/pfctl_altq.c
> 
> Modified: head/sbin/pfctl/pfctl_altq.c
> ==
> --- head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 15:01:27 2018
> (r338252)
> +++ head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 16:10:28 2018
> (r338253)
> @@ -299,8 +299,10 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa, stru
>   size = 4;
>   else if (rate <= 200 * 1000 * 1000)
>   size = 8;
> - else
> + else if (rate <= 2500 * 1000 * 1000)
>   size = 24;
> + else
> + size = 128;
>   size = size * getifmtu(pa->ifname);
>   pa->tbrsize = size;
>   }
> 

This breaks the build on my amd64 box:

/usr/svn/base/head/sbin/pfctl/pfctl_altq.c:302:32: error: overflow in
expression; result is -1794967296 with type 'int' [-Werror,-Winteger-overflow]
else if (rate <= 2500 * 1000 * 1000)
 ^

While "rate" is unsigned long, the expression being calculated is not ...

Regards, STefan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r338253 - head/sbin/pfctl

2018-08-23 Thread Patrick Kelsey
On Thu, Aug 23, 2018 at 1:05 PM Stefan Esser  wrote:

> Am 23.08.18 um 18:10 schrieb Patrick Kelsey:
> > Author: pkelsey
> > Date: Thu Aug 23 16:10:28 2018
> > New Revision: 338253
> > URL: https://svnweb.freebsd.org/changeset/base/338253
> >
> > Log:
> >   Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
> >   higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
> >   which is the highest standard rate that could be used prior to
> >   r338209, so the default behavior for all existing systems should
> >   remain the same.
> >
> >   The value of 128 chosen is a balance between being big enough to
> >   reduce potential precision/quantization effects stemming from frequent
> >   bucket refills over small time intervals and being small enough to
> >   prevent a greedy driver from burst dequeuing more packets than it has
> >   available hardware ring slots for whenever altq transitions from idle
> >   to backlogged.
> >
> >   Reviewed by:jmallett, kp
> >   MFC after:  2 weeks
> >   Sponsored by:   RG Nets
> >   Differential Revision: https://reviews.freebsd.org/D16852
> >
> > Modified:
> >   head/sbin/pfctl/pfctl_altq.c
> >
> > Modified: head/sbin/pfctl/pfctl_altq.c
> >
> ==
> > --- head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 15:01:27 2018
> (r338252)
> > +++ head/sbin/pfctl/pfctl_altq.c  Thu Aug 23 16:10:28 2018
> (r338253)
> > @@ -299,8 +299,10 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa,
> stru
> >   size = 4;
> >   else if (rate <= 200 * 1000 * 1000)
> >   size = 8;
> > - else
> > + else if (rate <= 2500 * 1000 * 1000)
> >   size = 24;
> > + else
> > + size = 128;
> >   size = size * getifmtu(pa->ifname);
> >   pa->tbrsize = size;
> >   }
> >
>
> This breaks the build on my amd64 box:
>
> /usr/svn/base/head/sbin/pfctl/pfctl_altq.c:302:32: error: overflow in
> expression; result is -1794967296 with type 'int'
> [-Werror,-Winteger-overflow]
> else if (rate <= 2500 * 1000 * 1000)
>  ^
>
> While "rate" is unsigned long, the expression being calculated is not ...
>
> Regards, STefan
>

Will fix shortly...not sure why I did not encounter this error in my setup.

-Patrick
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r338253 - head/sbin/pfctl

2018-08-23 Thread Patrick Kelsey
Author: pkelsey
Date: Thu Aug 23 16:10:28 2018
New Revision: 338253
URL: https://svnweb.freebsd.org/changeset/base/338253

Log:
  Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
  higher bandwidth interfaces.  The new value is used above 2.5 Gbps,
  which is the highest standard rate that could be used prior to
  r338209, so the default behavior for all existing systems should
  remain the same.
  
  The value of 128 chosen is a balance between being big enough to
  reduce potential precision/quantization effects stemming from frequent
  bucket refills over small time intervals and being small enough to
  prevent a greedy driver from burst dequeuing more packets than it has
  available hardware ring slots for whenever altq transitions from idle
  to backlogged.
  
  Reviewed by:  jmallett, kp
  MFC after:2 weeks
  Sponsored by: RG Nets
  Differential Revision: https://reviews.freebsd.org/D16852

Modified:
  head/sbin/pfctl/pfctl_altq.c

Modified: head/sbin/pfctl/pfctl_altq.c
==
--- head/sbin/pfctl/pfctl_altq.cThu Aug 23 15:01:27 2018
(r338252)
+++ head/sbin/pfctl/pfctl_altq.cThu Aug 23 16:10:28 2018
(r338253)
@@ -299,8 +299,10 @@ eval_pfaltq(struct pfctl *pf, struct pf_altq *pa, stru
size = 4;
else if (rate <= 200 * 1000 * 1000)
size = 8;
-   else
+   else if (rate <= 2500 * 1000 * 1000)
size = 24;
+   else
+   size = 128;
size = size * getifmtu(pa->ifname);
pa->tbrsize = size;
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"