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.
> +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?
> + /* 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?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
