[ovs-dev] [RFC 1/2] datapath: Refactor ovs_flow_cmd_fill_info().

[Joe Stringer]

On 14 July 2014 13:05, Pravin Shelar <pshelar at nicira.com> wrote:

> > @@ -734,9 +722,25 @@ static int ovs_flow_cmd_fill_info(struct datapath
> *dp,
> >         err = ovs_nla_put_flow(dp, &flow->key, &flow->mask->key, skb);
> >         if (err)
> >                 goto error;
> > -
> >         nla_nest_end(skb, nla);
> >
> > +       return 0;
> > +
> > +nla_put_failure:
> > +       err = -EMSGSIZE;
> > +error:
> > +       return err;
> > +}
> > +
>
> The error path are just returning error, so I think we can just return
> from error case rather than jumping over goto statement. Similarly we
> can avoid goto in newly added following functions.
>

I'll fix this up.



>  > @@ -751,6 +755,20 @@ static int ovs_flow_cmd_fill_info(struct datapath
> *dp,
> >              nla_put_u8(skb, OVS_FLOW_ATTR_TCP_FLAGS,
> (u8)ntohs(tcp_flags)))
> >                 goto nla_put_failure;
> >
> > +       return 0;
> > +
> > +nla_put_failure:
> > +       err = -EMSGSIZE;
> > +       return err;
> > +}
> > +
> > +/* Called with ovs_mutex or RCU read lock. */
> > +static int ovs_flow_cmd_fill_actions(const struct sw_flow *flow,
> > +                                    struct sk_buff *skb, int
> skb_orig_len)
> > +{
> > +       struct nlattr *start;
> > +       int err;
> > +
> >         /* If OVS_FLOW_ATTR_ACTIONS doesn't fit, skip dumping the
> actions if
> >          * this is the first flow to be dumped into 'skb'.  This is
> unusual for
> >          * Netlink but individual action lists can be longer than
> > @@ -780,11 +798,44 @@ static int ovs_flow_cmd_fill_info(struct datapath
> *dp,
> >         } else if (skb_orig_len)
> >                 goto nla_put_failure;
> >
> I know you are not changing this code, but can you add {} to "else if
> block" to fix kernel coding standard error.
>

OK.


Acked-by: Pravin B Shelar <pshelar at nicira.com>
>

Thanks for the review, I'll push this soon.