Hi Darrell,

Thank you for the clarification! I removed the line below from the source. I 
sent v5 to the dev list:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-September/338567.html
https://patchwork.ozlabs.org/patch/812245/

Best regards,
Zoltan


> -----Original Message-----
> From: Darrell Ball [mailto:db...@vmware.com]
> Sent: Friday, September 08, 2017 6:24 PM
> To: Zoltán Balogh <zoltan.bal...@ericsson.com>; 'd...@openvswitch.org' 
> <d...@openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH v2] netdev-dpdk: reset packet_type for reused 
> dp_packets
> 
> 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" <zoltan.bal...@ericsson.com> 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=7M6jjf8mR02x3pm7OqQKPVuq0RoVq7odJtobax0
> JYJw&e=
> 
>     Best regards,
>     Zoltan
> 
> 
>     > -----Original Message-----
>     > From: ovs-dev-boun...@openvswitch.org 
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Zoltán Balogh
>     > Sent: Friday, September 08, 2017 10:43 AM
>     > To: 'd...@openvswitch.org' <d...@openvswitch.org>
>     > 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 <zoltan.bal...@ericsson.com>
>     > Signed-off-by: Laszlo Suru <laszlo.s...@ericsson.com>
>     > Co-authored-by: Laszlo Suru <laszlo.s...@ericsson.com>
>     > CC: Jan Scheurich <jan.scheur...@ericsson.com>
>     > CC: Sugesh Chandran <sugesh.chand...@intel.com>
>     > CC: Darrell Ball <dlu...@gmail.com>
>     > ---
>     >  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
>     > d...@openvswitch.org
>     > 
> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to