On Mon, Apr 20, 2015 at 5:56 PM, Jesse Gross <je...@nicira.com> wrote:
> On Mon, Apr 20, 2015 at 3:54 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>> On Mon, Apr 20, 2015 at 3:36 PM, Jesse Gross <je...@nicira.com> wrote:
>>> On Mon, Apr 20, 2015 at 1:29 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>>>> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
>>>> new file mode 100644
>>>> index 0000000..209bf1a
>>>> --- /dev/null
>>>> +++ b/datapath/linux/compat/stt.c
>>>> +static void update_headers(struct sk_buff *skb, bool head,
>>>> + unsigned int l4_offset, unsigned int
>>>> hdr_len,
>>>> + bool ipv4, u32 tcp_seq)
>>> [...]
>>>> + skb->truesize = SKB_TRUESIZE(skb_end_offset(skb)) + skb->data_len;
>>>> +}
>>>
>>> I wonder if there are any possible edge cases with resetting truesize
>>> where the packet is still in someone's transmit queue (such as if we
>>> are looping back packet). Do we need to orphan it first?
>>>
>> ok, I will orphan it in update_headers.
>
> Just to clarify - I was mostly just thinking aloud on orphaning it.
> I'm not totally sure if that is the right thing to do or if this is
> the right place to do it. I'm not sure what the conceptual
> justification would be for it and it could potentially result in the
> sender's buffers not being properly limited. Perhaps not resetting the
> truesize is the right thing too...
>
I have seen warning msg if we do no keep truesize update along with
changes to skb.
>>>> +static void stt_rcv(struct stt_sock *stt_sock, struct sk_buff *skb)
>>>> +{
>>> [...]
>>>> + if (skb->next) {
>>>> + err = coalesce_skb(&skb);
>>>> + if (err)
>>>> + goto drop;
>>>> + }
>>>> +
>>>> + err = iptunnel_pull_header(skb,
>>>> + sizeof(struct stthdr) + STT_ETH_PAD,
>>>> + htons(ETH_P_TEB));
>>>
>>> This version of coalesce_skb() doesn't create fully formed skbs (any
>>> remaining pieces are in skb->next instead of a frag_list), right? That
>>> means that iptunnel_pull_header() potentially won't be able to access
>>> the data.
>>
>> right. I want to avoid setting and resetting skb->next and fraglist
>> pointer. So I will only do it if iptunnel_pull_header fails to do it
>> in first attempt. I think it should be pretty rare to get skb without
>> the stt header in first skb.
>
> Hmm, we have several places where we do a pskb_may_pull(). Potentially
> each one of those call sites could be affected by this.
ok. I will use frag_list to keep the skb list.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
