On Mon, Jul 29, 2013 at 3:48 PM, Pravin B Shelar <[email protected]> wrote:
> diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
> index 847cb39..9ffa74f 100644
> --- a/datapath/vport-lisp.c
> +++ b/datapath/vport-lisp.c
> +static void handle_offloads(struct sk_buff *skb)
> {
> - int err;
> + if (skb_is_gso(skb))
> + OVS_GSO_CB(skb)->fix_segment = lisp_build_header;
Is it right to use lisp_build_header() here? It seems like this stuff
should be constant across segments and will just get regenerated but
the UDP length won't be updated.
> +static int tnl_send(struct vport *vport, struct sk_buff *skb)
> {
Is it possible to reorganize this function a little bit to more
closely mirror the GRE/VXLAN code paths? It seems like some of the
things like the split between lisp_tnl_send()/tnl_send() and the code
order are a little bit historical. The more the superficial
differences that we can remove the easier I think it will be to
maintain - particularly in areas like the checksum compatibility code.
> min_headroom = LL_RESERVED_SPACE(rt_dst(rt).dev) +
> rt_dst(rt).header_len
> + tunnel_hlen
I think we probably can remove the VLAN portion of the headroom
calculation given that we drop all the L2 information.
> @@ -480,77 +422,27 @@ static int ovs_tnl_send(struct vport *vport, struct
> sk_buff *skb,
> goto err_free_rt;
> }
>
> + __skb_push(skb, LISP_HLEN);
> + skb_reset_transport_header(skb);
> +
> + lisp_build_header(skb);
> + udph = udp_hdr(skb);
> + udph->dest = lisp_port->dst_port;
> + udph->source = htons(ovs_tnl_get_src_port(skb));
Should rename ovs_tnl_get_src_port() now that it is LISP-specific? I
think all of the other former tunnel functions have now been renamed
or deleted. On the other hand, maybe we should just use
vxlan_src_port().
> - /* Push IP header. */
> - iph = ip_hdr(skb);
> - iph->version = 4;
> - iph->ihl = sizeof(struct iphdr) >> 2;
> - iph->protocol = ipproto;
> - iph->daddr = OVS_CB(skb)->tun_key->ipv4_dst;
> - iph->saddr = saddr;
> - iph->tos = OVS_CB(skb)->tun_key->ipv4_tos;
> - iph->ttl = OVS_CB(skb)->tun_key->ipv4_ttl;
> - iph->frag_off = OVS_CB(skb)->tun_key->tun_flags &
> + handle_offloads(skb);
Do we want to do this before setting the transport header since we are
calling skb_reset_inner_headers()?
X-CudaMail-Whitelist-To: [email protected]
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
