On Wed, Jul 30, 2014 at 12:25 PM, Andy Zhou <az...@nicira.com> wrote:
> LGTM. A minor comment inline. It not essential, so please feel free to
> ignore.
>
> Acked-by: Andy Zhou <az...@nicira.com>
>
>
>
>> @@ -353,25 +353,25 @@ static int queue_gso_packets(struct datapath *dp,
>> struct sk_buff *skb,
>> if (IS_ERR(segs))
>> return PTR_ERR(segs);
>>
>> + if (gso_type & SKB_GSO_UDP) {
>> + /* The initial flow key extracted by ovs_flow_extract()
>> + * in this case is for a first fragment, so we need to
>> + * properly mark later fragments.
>> + */
>> + later_key = *OVS_CB(skb)->pkt_key;
>> + later_key.ip.frag = OVS_FRAG_TYPE_LATER;
>> + }
>> +
>> /* Queue all of the segments. */
>> skb = segs;
>> do {
>> + if (gso_type & SKB_GSO_UDP && skb != segs)
>> + OVS_CB(skb)->pkt_key = &later_key;
>> +
>> err = queue_userspace_packet(dp, skb, upcall_info);
>> if (err)
>> break;
>>
>> - if (skb == segs && gso_type & SKB_GSO_UDP) {
>> - /* The initial flow key extracted by
>> ovs_flow_extract()
>> - * in this case is for a first fragment, so we need
>> to
>> - * properly mark later fragments.
>> - */
>> - later_key = *upcall_info->key;
>> - later_key.ip.frag = OVS_FRAG_TYPE_LATER;
>> -
>> - later_info = *upcall_info;
>> - later_info.key = &later_key;
>> - upcall_info = &later_info;
>> - }
>> } while ((skb = skb->next));
>
> I think this function may be even easier to read if we switch from a
> "do while" loop to a "for" loop.
>>
I scanned gso segment iteration done in kernel, do {} while is general
practice for iterating segs. So I kept it as it is and pushed patch to
master.
Thanks.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
