Thanks. After staring at it for a while, I think I'm comfortable with removing it.
Vishal, will you send a patch to do that? On Mon, Jul 09, 2018 at 02:43:24AM +0000, Ethan J. Jackson wrote: > Gosh, this is some old code. On my read, the `n_flows > 2000` is redundant. > My best guess is the line below where it sets a floor on the flow_limit of > 1000 was supposed to catch it. Really that floor should either be 2000, or > the check in the if statement should be `n_flows > 1000`. > > All of that said, I really don't see the point of the `n_flows > 2000` check > at all. I'm a bit worried there was some reason for it that I can't quite > remember, but if I was fixing this myself, I would just remove it., > > Ethan J. Jackson > ejj.sh ( http://ejj.sh ) > > On Fri, Jul 06, 2018 at 10:52 AM, Ben Pfaff < [email protected] > wrote: > > > > > > > > > On Fri, Jul 06, 2018 at 05:22:55PM +0000, Vishal Deep Ajmera wrote: > > > > > >> > >>> > >>> > >>> At first glance it looks like you're correct, but this code was added in > >>> 2013 with the following commit and hasn't changed since, so I wonder > >>> whether we're missing something important. Ethan, you wrote this code, do > >>> you have any thoughts? > >>> > >>> > >>> > >>> commit e79a6c833e0d72370951d6f8841098103cbb0b2d > >>> Author: Ethan J. Jackson < ejj@ eecs. berkeley. edu ( > >>> [email protected] > >>> ) > Tue Sep 24 13:39:56 2013 > >>> Committer: Ethan J. Jackson < ejj@ eecs. berkeley. edu ( > >>> [email protected] ) > Thu Dec 19 13:27:23 2013 > >>> > >>> > >>> > >>> ofproto: Handle flow installation and eviction in upcall. > >>> > >>> > >>> > >>> This patch moves flow installation and eviction from ofproto-dpif and the > >>> main thread, into ofproto-dpif-upcall. This performs significantly better > >>> (approximately 2x TCP_CRR improvement), and allows ovs-vswitchd to > >>> maintain significantly larger datapath flow tables. On top of that, it > >>> significantly simplifies the code, retiring "struct facet" and friends. > >>> > >>> > >>> > >>> Signed-off-by: Ethan Jackson < ethan@ nicira. com ( [email protected] ) > > >>> Acked-by: Ben Pfaff < blp@ nicira. com ( [email protected] ) > > >>> > >>> > >> > >> > >> > >> Thanks Ben for looking into this issue. Should I send a formal patch for > >> review by removing this check (n_flows > 2000) ? > >> > >> > > > > > > > > Let's see what Ethan has to say first. > > > > > > _______________________________________________ discuss mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
