On Wed, Aug 14, 2013 at 10:56 AM, Jesse Gross <je...@nicira.com> wrote:
> On Mon, Jul 29, 2013 at 3:48 PM, Pravin B Shelar <pshe...@nicira.com>
> 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.
>
> right, I will write udp fixup function.
> > +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.
>
> ok.
> > 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.
>
> ok.
> > @@ -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().
>
> I will send separate patch to 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()?
>
right, I will move it up.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
