The current odp_pktio_send() uses odp_packet_l2_ptr() which requires that
the packet be parsed to have that set. There are a couple of ways around
that. I can submit a patch to make that call work whether or not
_odp_packet_parse() has been called.
On Wed, Jan 7, 2015 at 12:01 PM, Zoltan Kiss <zoltan.k...@linaro.org> wrote:
>
>
> On 07/01/15 14:02, Ciprian Barbu wrote:
>
>> @@ -334,19 +331,21 @@ clone_pkts(struct netdev_odp *dev, struct
>>>>> dpif_packet **pkts,
>>>>> dropped++;
>>>>> continue;
>>>>> }
>>>>> - pkt = odph_packet_alloc(dev->pkt_pool);
>>>>> + pkt = odp_packet_alloc(dev->pkt_pool, size);
>>>>>
>>>>> - if (OVS_UNLIKELY(!odph_packet_is_valid(pkt))) {
>>>>> + if (OVS_UNLIKELY(pkt == ODP_PACKET_INVALID)) {
>>>>> VLOG_WARN_RL(&rl, "Could not allocate packet");
>>>>> dropped += cnt -i;
>>>>> break;
>>>>> }
>>>>>
>>>>> - odp_packet_init(pkt);
>>>>> - odp_packet_set_l2_offset(pkt, 0);
>>>>> + odp_packet_reset(pkt, 0);
>>>>> + odp_packet_l2_offset_set(pkt, 0);
>>>>>
>>>>> - memcpy(odp_packet_l2(pkt), ofpbuf_data(&pkts[i]->ofpbuf),
>>>>> size);
>>>>> - odp_packet_parse(pkt, size, 0);
>>>>> + memcpy(odp_packet_l2_ptr(pkt, NULL),
>>>>> ofpbuf_data(&pkts[i]->ofpbuf),
>>>>> + size);
>>>>> + /* TODO: is this OK? */
>>>>>
>>>>>
>>>>> You shouldn't use memcpy on packets. Instead, to set the length and
>>>>> copy the packet this should be:
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> odp_packet_push_tail(pkt, size);
>>>>> odp_packet_copydata_in(pkt, 0, size,
>>>>> ofpbuf_data(&pkts[i]->ofpbuf));
>>>>>
>>>>> + _odp_packet_parse(pkt);
>>>>>
>>>>>
>>>>> This is an internal API and should not be called by an ODP app. It
>>>>> should be an ODP API but Petri hasn't approved it as such, hence it
>>>>> can't be used.
>>>>>
>>>>
>>>> If this came in on an ODP interface, then we don't need to parse it as
>>>> Ciprian wrote in an another mail, because it was parsed during
>>>> reception.
>>>> But if it came from another kind of port (I'm not sure that's possible
>>>> at
>>>> the moment, but I think we will need that), we need to parse here.
>>>>
>>>
>> The line in question is inside clone_pkts, which is on the TX path,
>> it's called when it's needed to transmit a packet that is not an ODP
>> buffer (from a tap port for example). I don't remember exactly why I
>> was parsing the packet at that point, but it doesn't seem to make
>> sense to keep it now. The odp-generator for example doesn't parse
>> anything on the TX side.
>>
>> All other things needed seem to have been covered by Bill's comments,
>> can you send version 2 now? Also can you did the deb packages build
>> for, you, Mike told me he had some problems with that. Regular build
>> worked though.
>>
>
>
> If odp_pktio_send doesn't need those values set by _odp_packet_parse, then
> it's not necessary indeed. As far as I can see it doesn't use it, Bill, can
> you confirm it?
>
> Zoli
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
