[ovs-dev] [PATCH v3] Extend OVS IPFIX exporter to export tunnel headers
Wenyu Zhang
wenyuz at vmware.com
Thu Jul 17 10:05:18 UTC 2014
Thanks for the review.
I have removed the incorrect whitespaces.
About XXX_get_out_tun_info(), I keep the caluclation of saddr, including route look up process, tp_src and tp_dst in ops for each vport, due to that the process are a little different for each vport. And I think that the potential changes for vport ops in future may affect the process too, it's better to keep this process in vport ops.
And the operation to fill the collected info into out_tun_info data is exactly same, I abstract a function ovs_flow_get_out_tun_info to do it.
I have sent the freshed patch just a moment ago. May you help?
Bests,
Wenyu
-----Original Message-----
From: Pravin Shelar [mailto:pshelar at nicira.com]
Sent: Thursday, July 17, 2014 1:03 AM
To: Wenyu Zhang
Cc: Romain Lenglet; Ben Pfaff; Jesse Gross; dev at openvswitch.org
Subject: Re: [PATCH v3] Extend OVS IPFIX exporter to export tunnel headers
git am warning:
Applying: Extend OVS IPFIX exporter to export tunnel headers
/home/pravin/ovs/w8/.git/rebase-apply/patch:143: trailing whitespace.
if (out_tun_key)
/home/pravin/ovs/w8/.git/rebase-apply/patch:236: space before tab in indent.
const struct udphdr * udp_hdr = tph;
/home/pravin/ovs/w8/.git/rebase-apply/patch:433: trailing whitespace.
/home/pravin/ovs/w8/.git/rebase-apply/patch:594: trailing whitespace.
/home/pravin/ovs/w8/.git/rebase-apply/patch:695: trailing whitespace.
warning: squelched 21 whitespace errors
warning: 26 lines add whitespace errors.
On Wed, Jul 16, 2014 at 12:55 AM, Wenyu Zhang <wenyuz at vmware.com> wrote:
> Extend IPFIX exporter to export tunnel headers when both input and
> output of the port.
> Add three other_config options in IPFIX table: enable-input-sampling,
> enable-output-sampling and enable-tunnel-sampling, to control whether
> sampling tunnel info, on which direction (input or output).
> Insert sampling action before output action and the output tunnel port
> is sent to datapath in the sampling action.
> Make datapath collect output tunnel info and send it back to userpace
> in upcall message with a new additional optional attribute.
> Add a tunnel ports map to make the tunnel port lookup faster in
> sampling upcalls in IPFIX exporter. Make the IPFIX exporter generate
> IPFIX template sets with enterprise elements for the tunnel info, save
> the tunnel info in IPFIX cache entries, and send IPFIX DATA with tunnel info.
> Add flowDirection element in IPFIX templates.
>
> Signed-off-by: Wenyu Zhang <wenyuz at vmware.com>
> Acked-by: Romain Lenglet <rlenglet at vmware.com>
> ---
> datapath/actions.c | 16 +-
> datapath/datapath.c | 42 +++-
> datapath/datapath.h | 2 +
> datapath/flow.h | 24 +-
> datapath/flow_netlink.c | 58 ++++-
> datapath/flow_netlink.h | 2 +
> datapath/vport-geneve.c | 63 ++++-
> datapath/vport-gre.c | 67 ++++-
> datapath/vport-lisp.c | 67 ++++-
> datapath/vport-vxlan.c | 71 +++++-
> datapath/vport.h | 4 +
> include/linux/openvswitch.h | 21 +-
> lib/dpif-linux.c | 2 +
> lib/dpif.h | 1 +
> lib/odp-util.c | 183 +++++++++-----
> lib/odp-util.h | 2 +
> lib/packets.h | 9 +-
> ofproto/automake.mk | 4 +
> ofproto/ipfix-enterprise-entities.def | 19 ++
> ofproto/ofproto-dpif-ipfix.c | 444 ++++++++++++++++++++++++++++++---
> ofproto/ofproto-dpif-ipfix.h | 14 +-
> ofproto/ofproto-dpif-upcall.c | 20 +-
> ofproto/ofproto-dpif-xlate.c | 51 +++-
> ofproto/ofproto-dpif.c | 20 ++
> ofproto/ofproto.h | 3 +
> ofproto/tunnel.c | 4 +
> tests/odp.at | 9 +-
> utilities/ovs-vsctl.8.in | 8 +-
> vswitchd/bridge.c | 9 +
> vswitchd/vswitch.ovsschema | 5 +-
> vswitchd/vswitch.xml | 27 ++
> 31 files changed, 1097 insertions(+), 174 deletions(-) create mode
> 100644 ofproto/ipfix-enterprise-entities.def
>
> diff --git a/datapath/actions.c b/datapath/actions.c index
> 39a21f4..226b849 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
...
> static inline void ovs_flow_tun_info_init(struct ovs_tunnel_info *tun_info,
> - const struct iphdr *iph, __be64 tun_id,
> + const struct iphdr *iph, const void *tph,
> + __be64 tun_id,
> __be16 tun_flags,
> struct geneve_opt *opts,
> u8 opts_len) @@ -79,6 +82,21
> @@ static inline void ovs_flow_tun_info_init(struct ovs_tunnel_info *tun_info,
> tun_info->tunnel.ipv4_ttl = iph->ttl;
> tun_info->tunnel.tun_flags = tun_flags;
>
> + /* For the tunnel types on the top of IPsec,the tp_src and tp_dst of
> + * the upper tunnel are used.
> + * E.g: GRE over IPSEC, the tp_src and tp_port are zero.
> + */
> + if (tph &&
> + (iph->protocol == IPPROTO_UDP ||
> + iph->protocol == IPPROTO_TCP)) {
> + const struct udphdr * udp_hdr = tph;
> + tun_info->tunnel.tp_src = udp_hdr->source;
> + tun_info->tunnel.tp_dst = udp_hdr->dest;
> + } else {
> + tun_info->tunnel.tp_src = 0;
> + tun_info->tunnel.tp_dst = 0;
> + }
> +
Can you pass source port and dst post to to this function, so that we can avoid these checks this function.
> /* clear struct padding. */
> memset((unsigned char *) &tun_info->tunnel + OVS_TUNNEL_KEY_SIZE, 0,
> sizeof(tun_info->tunnel) - OVS_TUNNEL_KEY_SIZE); diff
> --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c index
> 5f975a1..1550daa 100644
> --- a/datapath/flow_netlink.c
....
>
> +static int geneve_get_out_tun_info(struct vport *vport, struct sk_buff *skb,
> + struct ovs_tunnel_info
> +*out_tun_info) {
> + struct geneve_port *geneve_port = geneve_vport(vport);
> + struct ovs_key_ipv4_tunnel *tun_key;
> + struct ovs_key_ipv4_tunnel *out_tun_key = &out_tun_info->tunnel;
> + struct rtable *rt;
> + __be32 saddr;
> + int err = 0;
> +
> +
> + if (unlikely(!OVS_CB(skb)->tun_info))
> + return -EINVAL;
> +
> + tun_key = &OVS_CB(skb)->tun_info->tunnel;
> +
> + /* Route lookup */
> + saddr = tun_key->ipv4_src;
> + rt = find_route(ovs_dp_get_net(vport->dp),
> + &saddr, tun_key->ipv4_dst,
> + IPPROTO_UDP, tun_key->ipv4_tos,
> + skb->mark);
> + if (IS_ERR(rt)) {
> + err = PTR_ERR(rt);
> + goto error;
> + }
> + ip_rt_put(rt);
> +
> + memset(out_tun_info, 0, sizeof(*out_tun_key));
> + /* Now the tun_key should contain the output tun key*/
> + out_tun_key->tun_flags |= TUNNEL_KEY;
> + out_tun_key->tun_id = tun_key->tun_id;
> + out_tun_key->ipv4_src = saddr;
> + out_tun_key->ipv4_dst = tun_key->ipv4_dst;
> + out_tun_key->ipv4_tos = tun_key->ipv4_tos;
> + out_tun_key->ipv4_ttl = tun_key->ipv4_ttl;
> + /* Get tp_src and tp_dst, refert to geneve_build_header() */
> + out_tun_key->tp_src = vxlan_src_port(1, USHRT_MAX, skb);
> + out_tun_key->tp_dst = inet_sport(geneve_port->sock->sk);
> + /* Set GENEVE's option */
> + out_tun_info->options = OVS_CB(skb)->tun_info->options;
> + out_tun_info->options_len =
> + OVS_CB(skb)->tun_info->options_len;
> +
> +error:
> + return err;
> +}
> +
> const struct vport_ops ovs_geneve_vport_ops = {
> - .type = OVS_VPORT_TYPE_GENEVE,
> - .create = geneve_tnl_create,
> - .destroy = geneve_tnl_destroy,
> - .get_name = geneve_get_name,
> - .get_options = geneve_get_options,
> - .send = geneve_send,
> + .type = OVS_VPORT_TYPE_GENEVE,
> + .create = geneve_tnl_create,
> + .destroy = geneve_tnl_destroy,
> + .get_name = geneve_get_name,
> + .get_options = geneve_get_options,
> + .send = geneve_send,
> + .get_out_tun_info = geneve_get_out_tun_info,
> };
geneve_get_out_tun_info(), gre_get_out_tun_info(), vxlan_get_out_tun_info(), lisp_get_out_tun_info() has basically same code. Can you write one function then call that from all of these functions?
More information about the dev
mailing list