On Mon, Jan 21, 2019 at 05:32:37PM +0200, Adi Nissim wrote:
> Currently the TC tunnel_key action is always
> initialized with the given tunnel id value. However,
> some tunneling protocols define the tunnel id as an optional field.
>
> This patch initializes the id field of tunnel_key:set and tunnel_key:unset
> only if a value is provided.
>
> In the case that a tunnel key value is not provided by the user
> the key flag will not be set.
>
> Signed-off-by: Adi Nissim <[email protected]>
> Acked-by: Paul Blakey <[email protected]>
> ---
> v1->v2: check if mask.tunnel.id == OVS_BE64_MAX
> so we won't do match in the case of a partial mask.
I am still a bit concerned about the partial mask case.
It looks like the code will now silently not offload such matches.
I think that a partial mask should either be offloaded or
offload of the entire flow should be rejected.
>
> lib/netdev-tc-offloads.c | 13 +++++++++++--
> lib/tc.c | 21 +++++++++++++++------
> lib/tc.h | 1 +
> 3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 73ce7b9..abfbaeb 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -574,7 +574,9 @@ parse_tc_flower_to_match(struct tc_flower *flower,
> }
>
> if (flower->tunnel) {
> - match_set_tun_id(match, flower->key.tunnel.id);
> + if (flower->mask.tunnel.id == OVS_BE64_MAX) {
> + match_set_tun_id(match, flower->key.tunnel.id);
> + }
> if (flower->key.tunnel.ipv4.ipv4_dst) {
> match_set_tun_src(match, flower->key.tunnel.ipv4.ipv4_src);
> match_set_tun_dst(match, flower->key.tunnel.ipv4.ipv4_dst);
> @@ -628,7 +630,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
> size_t tunnel_offset =
> nl_msg_start_nested(buf, OVS_KEY_ATTR_TUNNEL);
>
> - nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID,
> action->encap.id);
> + if (action->encap.id_present) {
> + nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID,
> + action->encap.id);
> + }
> if (action->encap.ipv4.ipv4_src) {
> nl_msg_put_be32(buf, OVS_TUNNEL_KEY_ATTR_IPV4_SRC,
> action->encap.ipv4.ipv4_src);
> @@ -830,11 +835,13 @@ parse_put_flow_set_action(struct tc_flower *flower,
> struct tc_action *action,
> tunnel_len = nl_attr_get_size(set);
>
> action->type = TC_ACT_ENCAP;
> + action->encap.id_present = false;
> flower->action_count++;
> NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
> switch (nl_attr_type(tun_attr)) {
> case OVS_TUNNEL_KEY_ATTR_ID: {
> action->encap.id = nl_attr_get_be64(tun_attr);
> + action->encap.id_present = true;
> }
> break;
> case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: {
> @@ -1099,6 +1106,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
> *match,
> flower.key.tunnel.tp_dst = tnl->tp_dst;
> flower.mask.tunnel.tos = tnl_mask->ip_tos;
> flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
> + flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
> + tnl_mask->tun_id : 0;
> flower_match_to_tun_opt(&flower, tnl, tnl_mask);
> flower.tunnel = true;
> }
> diff --git a/lib/tc.c b/lib/tc.c
> index b19f075..e435663 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -571,6 +571,7 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct
> tc_flower *flower)
> ovs_be32 id = nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_KEY_ID]);
>
> flower->key.tunnel.id = be32_to_be64(id);
> + flower->mask.tunnel.id = OVS_BE64_MAX;
> }
> if (attrs[TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK]) {
> flower->key.tunnel.ipv4.ipv4_src =
> @@ -1014,6 +1015,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct
> tc_flower *flower)
> action->encap.ipv6.ipv6_dst = nl_attr_get_in6_addr(ipv6_dst);
> }
> action->encap.id = id ? be32_to_be64(nl_attr_get_be32(id)) : 0;
> + action->encap.id_present = id ? true : false;
> action->encap.tp_dst = dst_port ? nl_attr_get_be16(dst_port) : 0;
> action->encap.tos = tos ? nl_attr_get_u8(tos) : 0;
> action->encap.ttl = ttl ? nl_attr_get_u8(ttl) : 0;
> @@ -1631,9 +1633,9 @@ nl_msg_put_act_tunnel_geneve_option(struct ofpbuf
> *request,
> }
>
> static void
> -nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, ovs_be64 id,
> - ovs_be32 ipv4_src, ovs_be32 ipv4_dst,
> - struct in6_addr *ipv6_src,
> +nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
> + ovs_be64 id, ovs_be32 ipv4_src,
> + ovs_be32 ipv4_dst, struct in6_addr *ipv6_src,
> struct in6_addr *ipv6_dst,
> ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
> struct tun_metadata tun_metadata,
> @@ -1650,7 +1652,9 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
> ovs_be64 id,
> nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, &tun, sizeof tun);
>
> ovs_be32 id32 = be64_to_be32(id);
> - nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
> + if (id_present) {
> + nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
> + }
> if (ipv4_dst) {
> nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC, ipv4_src);
> nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST, ipv4_dst);
> @@ -1906,7 +1910,9 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> break;
> case TC_ACT_ENCAP: {
> act_offset = nl_msg_start_nested(request, act_index++);
> - nl_msg_put_act_tunnel_key_set(request, action->encap.id,
> + nl_msg_put_act_tunnel_key_set(request,
> + action->encap.id_present,
> + action->encap.id,
> action->encap.ipv4.ipv4_src,
> action->encap.ipv4.ipv4_dst,
> &action->encap.ipv6.ipv6_src,
> @@ -2026,6 +2032,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct
> tc_flower *flower)
> uint8_t ttl = flower->key.tunnel.ttl;
> uint8_t tos_mask = flower->mask.tunnel.tos;
> uint8_t ttl_mask = flower->mask.tunnel.ttl;
> + ovs_be64 id_mask = flower->mask.tunnel.id;
>
> if (ipv4_dst) {
> nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC, ipv4_src);
> @@ -2045,7 +2052,9 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct
> tc_flower *flower)
> if (tp_dst) {
> nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
> }
> - nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> + if (id_mask == OVS_BE64_MAX) {
> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
> + }
> nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS,
> flower->key.tunnel.metadata);
> nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS_MASK,
> diff --git a/lib/tc.h b/lib/tc.h
> index 7196a32..6643f24 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -147,6 +147,7 @@ struct tc_action {
> } vlan;
>
> struct {
> + bool id_present;
> ovs_be64 id;
> ovs_be16 tp_src;
> ovs_be16 tp_dst;
> --
> 1.8.3.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
