Hi Zoltan

The packet_type initialization that I was referring to as becoming redundant 
now was the one moved from to  

void
dp_packet_init_dpdk(struct dp_packet *b, size_t allocated)
{
    dp_packet_set_allocated(b, allocated);
    b->source = DPBUF_DPDK;
    b->packet_type = htonl(PT_ETH);   <<<<<<<<<<<<<<<<<<<<<<<<
}

Essentially , it was ‘split out’ from dp_packet_init__ for dpdk.

Since the new packet_type was not being reset as would be needed if the buffer 
pool only init is done,
we are now initing all packets for packet_type with this patch, hence the above 
line for dpdk buffer
pool init becomes redundant.

Thanks Darrell


On 9/8/17, 3:01 AM, "Zoltán Balogh" <[email protected]> wrote:

    Hi,
    
    Please ignore this patch. It's faulty. We cannot remove initialization
    of packet_type from dp_packet_init__(). Beacause of non-dpdk netdevs 
    still need it. Here is a link to the corrected patch on patchwork:
    
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_811445_&d=DwIFAw&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=dGZmbKhBG9tJHY4odedsGA&m=8TOx50_GxR8BEr45vTRIK07hVU3zdUM74V8mfYZ71Pc&s=7M6jjf8mR02x3pm7OqQKPVuq0RoVq7odJtobax0JYJw&e=
 
    
    Best regards,
    Zoltan
    
    
    > -----Original Message-----
    > From: [email protected] 
[mailto:[email protected]] On Behalf Of Zoltán Balogh
    > Sent: Friday, September 08, 2017 10:43 AM
    > To: '[email protected]' <[email protected]>
    > Subject: [ovs-dev] [PATCH v2] netdev-dpdk: reset packet_type for reused 
dp_packets
    > 
    > DPDK uses dp-packet pool for storing received packets. The pool is
    > reused by rxq_recv funcions of the DPDK netdevs. The datapath is
    > capable to modify the packet_type property of packets. For instance
    > when encapsulated L3 packets are received on a ptap gre port.
    > In this case the packet_type property of struct dp_packet can be
    > modified and later the same dp_packet with the modified packet_type
    > can be reused in the rxq_rec function, so it can contain corrupted
    > data.
    > 
    > The dp_packet_batch_init_cutlen() in the rxq_recv functions iterates
    > over dp_packets and sets their cutlen. So I modified this function
    > to set packet_type to Ethernet for the dp_packets as well. I also
    > renamed this function because of the added functionality.
    > 
    > The dp_packet_batch_init_cutlen() iterates over batch->count dp_packet.
    > Therefore setting of batch->count = nb_rx needs to be done before the
    > former function is invoked. This is an additional fix.
    > 
    > This commit also removes initialization of packet_type from
    > dp_packet_init__(), since that was moved to dp_packet_init_dpdk() before.
    > 
    > Signed-off-by: Zoltan Balogh <[email protected]>
    > Signed-off-by: Laszlo Suru <[email protected]>
    > Co-authored-by: Laszlo Suru <[email protected]>
    > CC: Jan Scheurich <[email protected]>
    > CC: Sugesh Chandran <[email protected]>
    > CC: Darrell Ball <[email protected]>
    > ---
    >  lib/dp-packet.c   | 2 --
    >  lib/dp-packet.h   | 3 ++-
    >  lib/netdev-dpdk.c | 7 ++++---
    >  3 files changed, 6 insertions(+), 6 deletions(-)
    > 
    > diff --git a/lib/dp-packet.c b/lib/dp-packet.c
    > index e5d16a6..21dfe39 100644
    > --- a/lib/dp-packet.c
    > +++ b/lib/dp-packet.c
    > @@ -33,8 +33,6 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, 
enum dp_packet_source so
    >      dp_packet_rss_invalidate(b);
    >      dp_packet_mbuf_init(b);
    >      dp_packet_reset_cutlen(b);
    > -    /* By default assume the packet type to be Ethernet. */
    > -    b->packet_type = htonl(PT_ETH);
    >  }
    > 
    >  static void
    > diff --git a/lib/dp-packet.h b/lib/dp-packet.h
    > index 046f3ab..b4b721c 100644
    > --- a/lib/dp-packet.h
    > +++ b/lib/dp-packet.h
    > @@ -805,12 +805,13 @@ dp_packet_delete_batch(struct dp_packet_batch 
*batch, bool may_steal)
    >  }
    > 
    >  static inline void
    > -dp_packet_batch_init_cutlen(struct dp_packet_batch *batch)
    > +dp_packet_batch_init_packet_fields(struct dp_packet_batch *batch)
    >  {
    >      struct dp_packet *packet;
    > 
    >      DP_PACKET_BATCH_FOR_EACH (packet, batch) {
    >          dp_packet_reset_cutlen(packet);
    > +        packet->packet_type = htonl(PT_ETH);
    >      }
    >  }
    > 
    > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    > index f58e9be..ccccb9a 100644
    > --- a/lib/netdev-dpdk.c
    > +++ b/lib/netdev-dpdk.c
    > @@ -1644,8 +1644,9 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
    >                                           nb_rx, dropped);
    >      rte_spinlock_unlock(&dev->stats_lock);
    > 
    > -    dp_packet_batch_init_cutlen(batch);
    > -    batch->count = (int) nb_rx;
    > +    batch->count = nb_rx;
    > +    dp_packet_batch_init_packet_fields(batch);
    > +
    >      return 0;
    >  }
    > 
    > @@ -1684,8 +1685,8 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct 
dp_packet_batch *batch)
    >          rte_spinlock_unlock(&dev->stats_lock);
    >      }
    > 
    > -    dp_packet_batch_init_cutlen(batch);
    >      batch->count = nb_rx;
    > +    dp_packet_batch_init_packet_fields(batch);
    > 
    >      return 0;
    >  }
    > --
    > 1.9.1
    > 
    > _______________________________________________
    > dev mailing list
    > [email protected]
    > 
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFAw&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=dGZmbKhBG9tJHY4odedsGA&m=8TOx50_GxR8BEr45vTRIK07hVU3zdUM74V8mfYZ71Pc&s=nveC-LSAOFNOvZySoVIi2h6eVTnUWhZTPNBo_kYiTNM&e=
 
    

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to