[ovs-dev] [PATCH] ofproto-dpif: Init ukey->dump_seq to zero
blp at ovn.org
Wed Apr 4 20:28:10 UTC 2018
Oh, that's weird. It's as if I didn't read the patch. Maybe I just
read some preliminary version in another thread.
Anyway, you're totally right. I applied this to master. If you're
seeing problems in another branch, let me know and I will backport.
On Wed, Apr 04, 2018 at 07:03:43PM +0000, Jan Scheurich wrote:
> Thanks Ben,
> I hope what my patch does is precisely what you suggest. I had the same thoughts when I had a closer look at the code.
> Regards, Jan
> > -----Original Message-----
> > From: Ben Pfaff [mailto:blp at ovn.org]
> > Sent: Wednesday, 04 April, 2018 19:20
> > To: Jan Scheurich <jan.scheurich at ericsson.com>
> > Cc: dev at openvswitch.org; Zoltán Balogh <zoltan.balogh at ericsson.com>; jpettit at ovn.org
> > Subject: Re: [PATCH] ofproto-dpif: Init ukey->dump_seq to zero
> > On Wed, Apr 04, 2018 at 01:26:02PM +0200, Jan Scheurich wrote:
> > > In the current implementation the dump_seq of a new datapath flow ukey
> > > is set to seq_read(udpif->dump_seq). This implies that any revalidation
> > > during the current dump_seq period (up to 500 ms) is skipped.
> > >
> > > This can trigger incorrect behavior, for example when the the creation of
> > > datapath flow triggers a PACKET_IN to the controller, which which course
> > > the controller installs a new flow entry that should invalidate the
> > > original datapath flow.
> > >
> > > Initializing ukey->dump_seq to zero implies that the first dump of the
> > > flow, be it for revalidation or dumping statistics, will always be
> > > executed as zero is not a valid value of the ovs_seq.
> > >
> > > Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
> > If we are going to do this, then we should delete the 'dump_seq' member
> > of struct upcall, because it will always be zero. It is also worth
> > considering whether the other caller of ukey_create__() should pass 0,
> > and if so then we can delete the 'dump_seq' parameter of
> > ukey_create__().
> > Thanks,
> > Ben.
More information about the dev