Let's use a better name for the new files, say odp-execute.[ch], and
use an appropriately prefixed name for the function,
e.g. odp_actions_execute(). It would also be acceptable to break
action-related code out of odp-util.[ch] and put all of it in a new
odp-actions.[ch].
Some more comments:
On Wed, May 22, 2013 at 04:08:07PM +0900, Simon Horman wrote:
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a4ac23f..1717602 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -37,6 +37,7 @@
> #include "dummy.h"
> #include "dynamic-string.h"
> #include "flow.h"
> +#include "execute-actions.h"
> #include "hmap.h"
Please keep the #includes in alphabetical order.
> const struct dpif_class dpif_netdev_class = {
> diff --git a/lib/execute-actions.c b/lib/execute-actions.c
> new file mode 100644
> index 0000000..d1bbeee
> --- /dev/null
> +++ b/lib/execute-actions.c
> @@ -0,0 +1,200 @@
> +/*
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2013 Simon Horman
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include <linux/openvswitch.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "execute-actions.h"
#include "execute-actions.h" should go right after #include
<config.h>, please see CodingStyle.
> + case OVS_KEY_ATTR_ETHERNET:
> + eth_set_src_and_dst(packet,
> + nl_attr_get_unspec(a, sizeof(struct ovs_key_ethernet)));
Please fix indentation here.
> +#ifndef EXECUTE_ACTIONS_H
> +#define EXECUTE_ACTIONS_H 1
> +
> +#include "flow.h"
> +#include "netlink-protocol.h"
> +#include "ofpbuf.h"
It would be better to minimize the header dependencies, more like
this:
#include <stddef.h>
#include <stdint.h>
struct flow;
struct nlattr;
struct ofpbuf;
> +void
> +execute_actions(void *dp, struct ofpbuf *packet, struct flow *key,
> + const struct nlattr *actions, size_t actions_len,
> + void (*output)(void *dp, struct ofpbuf *packet,
> + uint32_t out_port),
> + void (*userspace)(void *dp, struct ofpbuf *packet,
> + const struct flow *key,
> + const struct nlattr *a));
> +#endif
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
