On Fri, Apr 17, 2015 at 12:34 PM, Jesse Gross <je...@nicira.com> wrote: > On Thu, Apr 16, 2015 at 7:17 PM, Pravin B Shelar <pshe...@nicira.com> wrote: >> +static struct sk_buff *push_stt_header(struct sk_buff *head, __be64 tun_id, >> + __be16 s_port, __be16 d_port, >> + __be32 saddr, __be32 dst, >> + __be16 l3_proto, u8 l4_proto, >> + int dst_mtu) >> +{ >> + struct sk_buff *skb; >> + >> + if (skb_shinfo(head)->frag_list) { >> + bool ipv4 = (l3_proto == htons(ETH_P_IP)); >> + bool tcp = (l4_proto == IPPROTO_TCP); >> + int l4_offset = skb_transport_offset(head); >> + >> + if (unlikely(segment_skb(&head, ipv4, tcp, l4_offset))) >> + goto error; >> + } > > I don't know that it's necessarily guaranteed that we have all of this > information in the transmit path, especially given that we pass in > checksum partial as true later on. In the event that we have just a > bare Ethernet packet, we might not know the transport offset or that > the TCP header is fully formed. > Bare Ethernet packet will get caught in can_segment() and will be linearized. I am setting checksum partial true since stt push header will set it later on anyways.
>> +int stt_xmit_skb(struct sk_buff *skb, struct rtable *rt, >> + __be32 src, __be32 dst, __u8 tos, >> + __u8 ttl, __be16 df, __be16 src_port, __be16 dst_port, >> + __be64 tun_id) > [...] >> + while (skb) { >> + struct sk_buff *next_skb = skb->next; >> + >> + skb->next = NULL; >> + >> + if (next_skb) >> + dst_clone(&rt->dst); >> + >> + /* Push STT and TCP header. */ >> + skb = push_stt_header(skb, tun_id, src_port, dst_port, src, >> + dst, inner_l3_proto, inner_l4_proto, >> + dst_mtu(&rt->dst)); >> + if (unlikely(!skb)) >> + goto next; > > Does this leak the dst reference on error? > ok. >> + /* Push IP header. */ >> + ret += skb_list_xmit(rt, skb, src, dst, tos, ttl, df); > > I think that 'ret' is overloaded here as it contains both the return > code from stt_can_offload() and a size from skb_list_xmit() without > being cleared in between. Maybe use separate variables? ok. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev