On Tue, Mar 31, 2020 at 08:15:25AM -0700, Usman S. Ansari wrote:
> On Tue, Mar 31, 2020 at 7:00 AM Simon Horman <[email protected]>
> wrote:
> 
> > On Mon, Mar 30, 2020 at 12:36:51PM -0700, Ben Pfaff wrote:
> > > On Mon, Mar 30, 2020 at 12:15:46PM -0700, [email protected] wrote:
> > > > From: Usman Ansari <[email protected]>
> > > >
> > > > Coverity reports unreachable code in "?" statement
> > > > Fixed by removing code segment
> > > >
> > > > Signed-off-by: Usman Ansari <[email protected]>
> > >
> > > This code looks pretty confused.  I don't think we should just change it
> > > without understanding it.
> > >
> > > Pieter and Simon, you were both involved in the following code in
> > > tc_add_matchall_policer().  It sets block_id to a constant zero, never
> > > changes it, and then tests its value.  Surely that was not the
> > > intention.  What's the real goal here?
> >
> > Hi Ben, Hi Usman,
> >
> > I think that this code was copied from elsewhere and
> > morphed into its current state during development.
> > I apologise for not catching the dead code during review.
> >
> > I think that the proposed change is safe. The function in question
> > relates to offload of ingress policing, and it appears to me that has never
> > been exercised in conjunction with indirect blocks, which the dead code
> > relates to.
> >
> > I would suggest that the cleanup patch could go a few steps further
> > by removing the local block_id and index variables entirely.
> >
> 
> 
> Will do, thanks.

Thanks. I think you could also look into removing the
definition of TCM_IFINDEX_MAGIC_BLOCK from this file as
it probably becomes unused with these changes.

...
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to