On Mon, Apr 17, 2017 at 12:58:02PM -0700, Joe Stringer wrote:
> On 14 April 2017 at 20:48, Ben Pfaff <[email protected]> wrote:
> > On Wed, Mar 15, 2017 at 04:01:37PM -0700, Joe Stringer wrote:
> >> Commit 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs."), 
> >> on
> >> branch-2.7 as 9554b03d6ab7, attempted to address incorrect encode and 
> >> decode of
> >> variable length metaflow fields where the OXM/NXM encoding of the variable
> >> length fields would incorrectly serialize the length. The patch addresses 
> >> this
> >> by introducing a new per-bridge structure that adds additional metaflow 
> >> fields
> >> for the variable-length fields on demand when the TLVs are configured by a
> >> controller.
> >>
> >> Unfortunately, in the original patch there was nothing ensuring that flows
> >> referring to variable length fields would retain valid field references 
> >> when
> >> controllers reconfigure the TLVs. In practice, this could lead to a crash 
> >> of
> >> ovs-vswitchd by configuring a TLV field, adding a flow which refers to it,
> >> removing the TLV field, then running some traffic that hit the configured 
> >> flow.
> >>
> >> This series looks to remedy the situation by reference counting the 
> >> variable
> >> length fields and preventing a controller from reconfiguring TLV fields 
> >> when
> >> there are active flows whose match or actions refer to the field.
> >>
> >> This series was applied to master, but given the size of the change and the
> >> minor changes necessary to apply to branch-2.7, I would feel more 
> >> confident in
> >> backporting it if there was an extra round of review to ensure that 
> >> nothing was
> >> missed when this series was first applied to master.
> >
> > Thanks a lot for backporting this.  Backporting is sometimes difficult
> > work and rarely rewarding, so I really appreciate seeing it done.
> >
> > Who should review this?  Jarno, I see you made a comment; do you plan to
> > review it?
> 
> Hi Ben,
> 
> I think for the library ABI side changes, I would appreciate feedback
> from you on how in particular we expect this to be handled (as per my
> update to the thread below).
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329954.html
> 
> I spoke to Jarno offline and he mentioned he would be OK reviewing the
> code itself. The previous piece of feedback about including another
> patch will be addressed by another series which I intend to repost
> shortly.

OK.  I sent some thoughts in the direction of your longer more detailed
message on the subject.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to