[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150721175311.GE23672@pox.localdomain>
Date: Tue, 21 Jul 2015 19:53:11 +0200
From: Thomas Graf <tgraf@...g.ch>
To: Alexei Starovoitov <ast@...mgrid.com>
Cc: roopa@...ulusnetworks.com, rshearma@...cade.com,
ebiederm@...ssion.com, hannes@...essinduktion.org,
pshelar@...ira.com, jesse@...ira.com, davem@...emloft.net,
daniel@...earbox.net, tom@...bertland.com, edumazet@...gle.com,
jiri@...nulli.us, marcelo.leitner@...il.com,
stephen@...workplumber.org, jpettit@...ira.com, kaber@...sh.net,
simon.horman@...ronome.com, joestringer@...ira.com, ja@....bg,
weichunc@...mgrid.com, netdev@...r.kernel.org, dev@...nvswitch.org
Subject: Re: [PATCH net-next 14/22] vxlan: Flow based tunneling
On 07/21/15 at 10:30am, Alexei Starovoitov wrote:
> RX:
> >+ info->mode = IP_TUNNEL_INFO_RX;
> >+ info->key.tun_flags = TUNNEL_KEY;
> >+ info->key.tun_id = cpu_to_be64(vni >> 8);
> ...
> TX:
> >+ dst_port = info->key.tp_dst ? : vxlan->dst_port;
> >+ vni = be64_to_cpu(info->key.tun_id);
>
> I think the copy paste of ovs_tunnel_info into ip_tunnel_info
> can be improved. In particular instead of '__be64 tun_id'
> we can use '__u64 tun_id' which will avoid extra byteswaps for rx/tx
> paths.
>
> netlink for this part also seems inconsistent.
> In the patch 16:
> +static const struct nla_policy ip_tun_policy[IP_TUN_MAX + 1] = {
> + [IP_TUN_ID] = { .type = NLA_U64 },
> ...
> + if (tb[IP_TUN_ID])
> + tun_info->key.tun_id = nla_get_u64(tb[IP_TUN_ID]);
>
> I think nla_get_be64 should be there?
> and with my suggestion we can add be64_to_cpu() here instead
> of doing it per packet.
> Thoughts?
I like this. The be64 originates from how OVS stores the tun_id in the
flow key. I agree that it makes sense to limit and delay the byteswaps
to when OVS inherits the flow key from the ip_tunnel_info. I will send
a follow-up.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists