[ovs-dev] [recirc datapath V3 RFC 2/2] datapath: add recirc action

[Jesse Gross]

On Wed, Apr 16, 2014 at 11:49 PM, Andy Zhou <azhou at nicira.com> wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 820075f..9307ee1 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
> +                                const struct nlattr *a)
> +{
> +       struct sw_flow_key recirc_key;
> +       const struct vport *p = OVS_CB(skb)->input_vport;
> +       uint32_t hash = OVS_CB(skb)->pkt_key->ovs_flow_hash;
> +       int err;
> +
> +       err = ovs_flow_extract(skb, p->port_no, &recirc_key);

I think there's a possibility of OVS_CB(skb)->input_vport being NULL
if the packet comes from userspace through ovs_packet_cmd_execute().

> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index f464a5b..6edf54b 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -902,6 +911,10 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
>                                      output->ovs_flow_hash)))
>                 goto nla_put_failure;
>
> +       if ((swkey->recirc_id) &&
> +               (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id)))
> +                       goto nla_put_failure;

I think the extra sets of parentheses around each clause is
unnecessary - and actually I see this is in the hash patch as well.