On Fri, Jul 14, 2017 at 10:11:04AM +0000, Fischetti, Antonio wrote:
> Thanks Ben for your feedback, my replies inline.
> 
> > -----Original Message-----
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Tuesday, July 11, 2017 8:54 PM
> > To: Fischetti, Antonio <antonio.fische...@intel.com>
> > Cc: d...@openvswitch.org
> > Subject: Re: [ovs-dev] [PATCH 3/3] dpctl: Add new 'ct-bkts' command.
> > 
> > On Fri, Jun 23, 2017 at 01:28:22PM +0100, antonio.fische...@intel.com
> > wrote:
> > > From: Antonio Fischetti <antonio.fische...@intel.com>
> > >
> > > With the command:
> > >  ovs-appctl dpctl/ct-bkts
> > > shows the number of connections per bucket.
> > >
> > > By using a threshold:
> > >  ovs-appctl dpctl/ct-bkts gt=N
> > > for each bucket shows the number of connections when they
> > > are greater than N.
> > >
> > > Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
> > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodire...@intel.com>
> > > Co-authored-by: Bhanuprakash Bodireddy
> > <bhanuprakash.bodire...@intel.com>
> > 
> > Is this concept of buckets one that the kernel conntracker has too?  If
> > so, then I'm concerned about the definition of conn_per_buckets[] in
> > dpctl_ct_bkts(), since it has a hardcoded CONNTRACK_BUCKETS size and
> > nothing checks whether cte.bkt is in the right range.  If not, then
> > should this command be one that is limited to the userspace conntracker?
> 
> [AF] You're right, I didn't realize the concept of buckets 
> is in the kernel CT too, I did this implementation with the 
> userspace ConnTracker in mind.
> 
> Would it be ok to have a first implementation limited to the userspace CT 
> like:
> 
> dpctl_ct_bkts(...)
> { 
>     if (!dpctl_p->is_appctl) { /* Not called by ovs-appctl command => exit. */
>         dpctl_print(dpctl_p, "Command is available for UserSpace ConnTracker 
> only.\n");
>         return 0;
>     }
> 
>     < implementation for Userspace CT here >
>     ...
> 
> }
> 
> so that later on it could be extended to cover also the kernel CT case?

Is it much work to extend it to cover the kernel?  If not, it would be
better to cover both.  Otherwise, sure, this approach is an OK way to
start.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to