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

Reply via email to