Date: Fri, 6 Jun 2014 11:23:29 +0800 From: Sato Kentney <satokentney@gmail.com> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: bycn82 <bycn82@gmail.com>, "Alexander V. Chernikov" <melifaro@ipfw.ru>, FreeBSD Net <net@freebsd.org> Subject: Re: [CFT]: ipfw named tables / different tabletypes Message-ID: <CAD3kpuWcuZ%2B93c4fRGKoXxg%2ByFxkiDOdhFpG6A-Jv-HtBcJ0CQ@mail.gmail.com> In-Reply-To: <20140605165806.GC81905@onelab2.iet.unipi.it> References: <20140522154740.GA76448@onelab2.iet.unipi.it> <537E2153.1040005@FreeBSD.org> <20140522163812.GA77634@onelab2.iet.unipi.it> <538B2FE5.6070407@FreeBSD.org> <539044E4.1020904@ipfw.ru> <000c01cf80be$41194370$c34bca50$@gmail.com> <20140605134256.GA81234@onelab2.iet.unipi.it> <000001cf80cd$5dc1d9b0$19458d10$@gmail.com> <20140605155402.GA81905@onelab2.iet.unipi.it> <000401cf80d8$ad1bb840$075328c0$@gmail.com> <20140605165806.GC81905@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
Your original porpose of pps is good enough for me, it controls packets
number for each second. it is simple and make sence, As a network
administrator, I think if you provide "packets per second" or "packets per
minute" ,that is good enough for 99% percent of users.
Thanks,
Sato K(佐藤柯德)
On Fri, Jun 6, 2014 at 12:58 AM, Luigi Rizzo <rizzo@iet.unipi.it> wrote:
> On Fri, Jun 06, 2014 at 12:10:26AM +0800, bycn82 wrote:
> > Sorry for waste you time to explain it again, I will read the code first.
> >
> > And the latest patch of `PPS` should be OK, I checked the logic
> carefully this time. I sent it out last weekend.
> >
> > logic as below, PPS actually will be fulfilled using `PPT`,(N packets
> per M ticks).
>
> Bill,
> sorry but even this version has multiple issues -- performance,
> correctness, style.
>
> performance:
> duration_in_ticks and limit do not need to be recomputed
> on every packet, just once when you install the rule.
>
> correctness
> limit should be not lower than the user-specified value,
> so if the equation has the form \lceil{a}{b} \rceil,
> the correct computation using integer arithmetic is
> (a + b - 1)/b and not (a + b/2) / b
>
> You should also check for corner cases, overflows etc.
> I haven't actually checked that the equation you use is
> correct, so a comment explaining why it is would be appropriate.
>
> BTW you have no checks on the input arguments, so you can
> have duration =0 and then you have a division by 0 in the kernel.
>
> style
> boring as it might be, we use spaces around keywords and { }
>
> I am also unhappy on the ppt option, because the duration of a tick
> can change across reboots, so this option would give variable
> behaviour. With all sources of uncertainty on packet arrivals
> and when the timer fires to advance 'ticks', there is really
> no point in having PPT to avoid the rounding error with some
> strange values of HZ.
>
>
> Now before you think i am too picky:
> some of the above (style) are trivial issues, but they require
> manual editing of the patch before applying and this is the reason
> many project and people usually bounce patches just because of style
> (the process does not scale otherwise).
> If it were just for that, i would not mind.
>
> But many of the other problems are more serious, and the fact they
> keep coming out after so many iterations suggest me that _you_
> should spend more time reviewing your code before submitting it
> and asking others to find and fix problems.
>
> cheers
> luigi
>
> >
> > + case O_PPT:
> > + case O_PPS:{
> > + ipfw_insn_pps *pps = (ipfw_insn_pps *)cmd;
> > + if(pps->limit==0){
> > + int limit,duration_in_ticks;
> > + if(1000/hz > pps->duration){
> > + duration_in_ticks=1;
> > + }else{
> > +
> duration_in_ticks=(pps->duration * hz +500)/1000;
> > + }
> > + limit=(cmd->arg1 *1000 *
> duration_in_ticks + hz * pps->duration/2)/(hz * pps->duration);
> > + pps->limit=limit;
> > +
> pps->duration_in_ticks=duration_in_ticks;
> > + }
> > +
> if(pps->start_time+pps->duration_in_ticks>= ticks){
> > + if(pps->count < pps->limit){
> > + retval = IP_FW_PASS;
> > + }else{
> > + retval = IP_FW_DENY;
> > + }
> > + pps->count++;
> > + }else{
> > + pps->start_time=ticks;
> > + pps->count=1;
> > + retval = IP_FW_PASS;
> > + }
> > + l = 0;
> > + done = 1;
> > + break;
> > + }
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAD3kpuWcuZ%2B93c4fRGKoXxg%2ByFxkiDOdhFpG6A-Jv-HtBcJ0CQ>