[ovs-dev] [PATCH 2/2] Subject: [PATCH] netdev-linux.c: unreachable code

[Simon Horman]

On Mon, Mar 30, 2020 at 12:36:51PM -0700, Ben Pfaff wrote:
> On Mon, Mar 30, 2020 at 12:15:46PM -0700, ua1422 at gmail.com wrote:
> > From: Usman Ansari <ua1422 at gmail.com>
> > 
> > Coverity reports unreachable code in "?" statement
> > Fixed by removing code segment
> > 
> > Signed-off-by: Usman Ansari <ua1422 at gmail.com>
> 
> 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.

> 
> >     int ifindex, index, err = 0;
> >     struct tc_police pol_act;
> >     uint32_t block_id = 0;
> >     struct ofpbuf request;
> >     struct ofpbuf *reply;
> >     struct tcmsg *tcmsg;
> >     uint32_t handle = 1;
> > 
> >     err = get_ifindex(netdev, &ifindex);
> >     if (err) {
> >         return err;
> >     }
> > 
> >     index = block_id ? TCM_IFINDEX_MAGIC_BLOCK : ifindex;
> >     tcmsg = tc_make_request(index, RTM_NEWTFILTER, NLM_F_CREATE | NLM_F_ECHO,
> >                             &request);
> >     tcmsg->tcm_parent = block_id ? : TC_INGRESS_PARENT;
> >     tcmsg->tcm_info = tc_make_handle(prio, eth_type);
> >     tcmsg->tcm_handle = handle;
> 
> Thanks,
> 
> Ben.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>