First of all, great to see this work. This is awesome! I read through
the code a first time. Planning to do more reviewing but looks very
sane already.
On 10/16/14 at 11:38am, Pravin B Shelar wrote:
> + 192.168.1.1/24
> + +--------------+
> + | int-br | 192.168.1.2/24
> + +--------------+ +--------------+
> + | vxlan0 | | vxlan0 |
> + +--------------+ +--------------+
> + | |
> + | |
> + | |
> + 172.168.1.1/24 |
> + +--------------+ |
> + | br-eth1 | 172.168.1.2/24
> + +--------------+ +---------------+
> + | eth1 |----------------------------------| eth1 |
> + +--------------+ +----------------
Might be worth finding other names than int-br and br-eth1 due to
Neutron's usage of br-int and br-ethX to avoid confusion. I realize
that everybody is free to name their bridges but examples tend to get
used 1:1 ;-)
> +b...@openvswitch.org
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> b/datapath/linux/compat/include/linux/openvswitch.h
> index b2257e6..ff7c9d9 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -580,6 +580,15 @@ struct ovs_action_hash {
> uint32_t hash_basis;
> };
>
> +#define TNL_PUSH_HEADER_SIZE 128
> +struct ovs_action_push_tnl {
> + uint32_t tnl_port;
> + uint32_t out_port;
> + uint32_t header_len;
> + uint32_t tnl_type; /* For logging. */
> + uint8_t header[TNL_PUSH_HEADER_SIZE];
> +};
Any reason for this to live in the kernel header?
> /**
> * enum ovs_action_attr - Action types.
> *
> @@ -633,6 +642,10 @@ enum ovs_action_attr {
> * data immediately followed by a mask.
> * The data must be zero for the unmasked
> * bits. */
> +#ifndef __KERNEL__
> + OVS_ACTION_ATTR_TUNNEL_PUSH,
> + OVS_ACTION_ATTR_TUNNEL_POP,
> +#endif
> __OVS_ACTION_ATTR_MAX
> };
Should we remove the #ifndef so the action values are reserved in the
kernel datapath as well? We don't want conflicting actions types later on.
> +void
> +tnl_port_map_insert(odp_port_t port, ovs_be32 ip_dst, ovs_be16 udp_port,
> + const char dev_name[])
> +{
> + const struct cls_rule *cr;
> + struct tunnel_port *p;
> + struct match match;
> +
> + memset(&match, 0, sizeof match);
> +
> + match.flow.dl_type = htons(ETH_TYPE_IP);
> + if (udp_port) {
> + match.flow.nw_proto = IPPROTO_UDP;
> + } else {
> + match.flow.nw_proto = IPPROTO_GRE;
> + }
> +
> + match.flow.tp_dst = udp_port;
Move this flow init code into a function for use by tnl_port_map_delete()?
> +
> + /* When matching on incoming flow from remove tnl end point,
^^^^
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
