[CFT]: ipfw named tables / different tabletypes

[Sato Kentney]

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 at 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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>