On Sat, Dec 15, 2012 at 2:14 AM, Jarno Rajahalme
<jarno.rajaha...@nsn.com> wrote:
> Make OVS_TUNNEL_ATTR_DST_IPV4 and OVS_TUNNEL_ATTR_FLAGS optional to allow
> configuration of null_ports.
>
> Existing code in userspace still always sets the flags, as they are needed
> internally.
>
> Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com>
I wasn't able to apply this patch because it conflicts with current
master. Can you provide a rebased version?
> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> index 1db60d2..66ed9e5 100644
> --- a/datapath/tunnel.c
> +++ b/datapath/tunnel.c
> @@ -1066,11 +1066,15 @@ static int tnl_set_config(struct net *net, struct
> nlattr *options,
> if (err)
> return err;
>
> - if (!a[OVS_TUNNEL_ATTR_FLAGS] || !a[OVS_TUNNEL_ATTR_DST_IPV4])
> - return -EINVAL;
> + if (a[OVS_TUNNEL_ATTR_FLAGS])
> + mutable->flags = nla_get_u32(a[OVS_TUNNEL_ATTR_FLAGS])
> + & TNL_F_PUBLIC;
> + else
> + mutable->flags = 0;
It shouldn't be necessary to set it to zero in the else case since
'mutable' should be cleared already. However, I think we should
either make flags optional everywhere (i.e. not return the attribute
on get and enable userspace to handle this) or still enforce its
existence.
> - mutable->flags = nla_get_u32(a[OVS_TUNNEL_ATTR_FLAGS]) & TNL_F_PUBLIC;
> - mutable->key.daddr = nla_get_be32(a[OVS_TUNNEL_ATTR_DST_IPV4]);
> + if (a[OVS_TUNNEL_ATTR_DST_IPV4]) {
> + mutable->key.daddr =
> nla_get_be32(a[OVS_TUNNEL_ATTR_DST_IPV4]);
> + }
It was actually already supposed to be possible to create null ports
already by not passing any options. This doesn't fully work any more
because the VXLAN destination port is an option that makes sense even
in the case of flow based tunneling so it makes sense to allow a set
of options without a destination port. However, there are some flags
that get set in this case that we don't want, primarily tunnel_type.
Therefore, we probably want to check for the VXLAN port, then for
destination IP address and if the latter isn't present then jump to
out.
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 032cd3d..bb7346f 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -752,12 +753,9 @@ parse_tunnel_config(const char *name, const char *type,
> set_key(args, "in_key", OVS_TUNNEL_ATTR_IN_KEY, options);
> set_key(args, "out_key", OVS_TUNNEL_ATTR_OUT_KEY, options);
>
> - if (!daddr) {
> - VLOG_ERR("%s: %s type requires valid 'remote_ip' argument",
> - name, type);
> - return EINVAL;
> + if (daddr) {
> + nl_msg_put_be32(options, OVS_TUNNEL_ATTR_DST_IPV4, daddr);
> }
I don't think we can make this optional to an external user yet since
none of the infrastructure to make use of it is there yet.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
