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...

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

Reply via email to