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 <[email protected]> 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 [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
