On Fri, Apr 17, 2015 at 12:34 PM, Jesse Gross <[email protected]> wrote:
> On Thu, Apr 16, 2015 at 7:17 PM, Pravin B Shelar <[email protected]> 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
