On Mon, Jul 1, 2013 at 7:29 PM, Andy Zhou <az...@nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 3680391..0ac46d0 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -1353,12 +1353,14 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff
> *skb, struct genl_info *info)
> */
> error = -EEXIST;
> if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
> - info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
> + info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL)) {
> goto err_unlock_ovs;
> + }
I don't think we need this set of parentheses anymore.
> /* The unmasked key has to be the same for flow updates. */
> error = -EINVAL;
> if (!ovs_flow_cmp_unmasked_key(flow, &key, match.range.end))
> + OVS_NLERR("Flow modification message rejected,
> umasked key does not match.\n");
> goto err_unlock_ovs;
However, it looks like do need one here.
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index 559df69..17b1db9 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> @@ -204,4 +204,8 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *,
> u32 portid, u32 seq,
>
> int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb);
> void ovs_dp_notify_wq(struct work_struct *work);
> +
> +#define OVS_NLERR(fmt, ...) \
> + pr_info_once(fmt " netlink: ", ##__VA_ARGS__)
I don't think that the space is necessary before " netlink" since fmt
already has one.
I believe that we need backports for pr_info_once() on older kernels
(and I would just backport all the variants of that for different log
levels).
> diff --git a/datapath/flow.c b/datapath/flow.c
> index 2ac36b6..b1e0360 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
The log messages are much better now but can we be sure to include the
invalid values that we received? Also, can you make the formats all
the same and check for capitalization and spelling?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
