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

Usman S. Ansari ua1422 at gmail.com
Tue Mar 31 15:15:25 UTC 2020


On Tue, Mar 31, 2020 at 7:00 AM Simon Horman <simon.horman at netronome.com>
wrote:

> 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.
>


Will do, thanks.

>
> >
> > >     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
> >
>


More information about the dev mailing list