Regards _Sugesh
> -----Original Message----- > From: pravin shelar [mailto:pshe...@ovn.org] > Sent: Thursday, April 14, 2016 5:59 PM > To: Chandran, Sugesh <sugesh.chand...@intel.com> > Cc: ovs dev <dev@openvswitch.org> > Subject: Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling > performance using DPDK Rx checksum offloading feature. > > On Wed, Apr 13, 2016 at 7:42 AM, Sugesh Chandran > <sugesh.chand...@intel.com> wrote: > > Optimizing tunneling performance in userspace datapath by offloading > > the rx checksum validation on tunnel packets to the NIC when it is > supported. > > > > This patch improves the bidirectional VxLAN tunneling performance by > > 8% and decapsulation performance by 24%. However it introduces 1% > > performance drop in PHY-PHY case due to the overhead of validating > > invalid checksum flag reported by NIC. > > > > Signed-off-by: Sugesh Chandran <sugesh.chand...@intel.com> > > --- > > lib/dpif-netdev.c | 40 ++++++++++++++++++++++++++++++++-------- > > lib/netdev-dpdk.c | 39 +++++++++++++++++++++++++-------------- > > lib/netdev-provider.h | 3 +++ > > lib/netdev-vport.c | 25 +++++++++++++++---------- > > lib/netdev.c | 1 + > > lib/netdev.h | 5 +++++ > > lib/packets.h | 1 + > > 7 files changed, 82 insertions(+), 32 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index > > 2870951..5de5c6a 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -70,6 +70,7 @@ > > #include "util.h" > > > > #include "openvswitch/vlog.h" > > +#include "netdev-provider.h" > > > > VLOG_DEFINE_THIS_MODULE(dpif_netdev); > > > > @@ -479,7 +480,8 @@ static void dp_netdev_execute_actions(struct > dp_netdev_pmd_thread *pmd, > > const struct nlattr *actions, > > size_t actions_len); static > > void dp_netdev_input(struct dp_netdev_pmd_thread *, > > - struct dp_packet **, int cnt, odp_port_t > > port_no); > > + struct dp_packet **, int cnt, > > + struct dp_netdev_port *port); > > static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *, > > struct dp_packet **, int cnt); > > > > @@ -2572,7 +2574,7 @@ dp_netdev_process_rxq_port(struct > dp_netdev_pmd_thread *pmd, > > *recirc_depth_get() = 0; > > > > cycles_count_start(pmd); > > - dp_netdev_input(pmd, packets, cnt, port->port_no); > > + dp_netdev_input(pmd, packets, cnt, port); > > cycles_count_end(pmd, PMD_CYCLES_PROCESSING); > > } else if (error != EAGAIN && error != EOPNOTSUPP) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, > > 5); @@ -3394,6 +3396,20 @@ dp_netdev_queue_batches(struct > dp_packet *pkt, > > packet_batch_update(batch, pkt, mf); } > > > > +static inline bool > > +is_checksum_valid(struct dp_packet *packet) { #ifdef DPDK_NETDEV > > + if (packet->mbuf.ol_flags & (PKT_RX_IP_CKSUM_BAD | > > + PKT_RX_L4_CKSUM_BAD)) { > > + return 0; > > + } > > + packet->md.ol_flags = NETDEV_RX_CHECKSUM_OFFLOAD; > There is no need to define redundant flags for same information in > dp_packet. We can just access packet->mbuf members to check the > checksum flag. [Sugesh] mbuf doesn’t have a flag for checksum. However the checksum Invalid flags in mbuf get set when a packet received with invalid checksum on a checksum offloaded port. So a packet with a valid checksum cannot say if the checksum is already validated in the NIC/not. We need this information in the packet to bypass checksum validation in tunneling code. > > > + return 1; > > +#else > > + return 0; > > +#endif > > +} > > + > > /* Try to process all ('cnt') the 'packets' using only the exact match > > cache > > * 'pmd->flow_cache'. If a flow is not found for a packet 'packets[i]', the > > * miniflow is copied into 'keys' and the packet pointer is moved at > > the @@ -3409,11 +3425,13 @@ static inline size_t > > emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet > **packets, > > size_t cnt, struct netdev_flow_key *keys, > > struct packet_batch batches[], size_t *n_batches, > > - bool md_is_valid, odp_port_t port_no) > > + bool md_is_valid, struct dp_netdev_port *port) > > { > > struct emc_cache *flow_cache = &pmd->flow_cache; > > struct netdev_flow_key *key = &keys[0]; > > size_t i, n_missed = 0, n_dropped = 0; > > + bool rx_cksm_ol_enable = port && (port->netdev->ol_flags & > > + NETDEV_RX_CHECKSUM_OFFLOAD); > > > > for (i = 0; i < cnt; i++) { > > struct dp_netdev_flow *flow; > > @@ -3425,6 +3443,12 @@ emc_processing(struct dp_netdev_pmd_thread > *pmd, struct dp_packet **packets, > > continue; > > } > > > > + if (OVS_UNLIKELY(rx_cksm_ol_enable && > > + !is_checksum_valid(packet))) { > Is checking for rx_cksum_ol_enable really required? No other netdev > implementation would initialize the packet ckecksum flags. So only DPDK > packets should return a invalid checksum here. [Sugesh] Yes, But all the DPDK ports may not able to do the Rx checksum offloading. So its necessary to check if the port have rx checksum offloading enabled or not. > > > + dp_packet_delete(packet); > > + n_dropped++; > > + continue; > > + } > > + > Calling checksum validation from datapath for all packets is odd. I am not > sure > about the reasoning. I think we can validate the packet checksum flags in > tunnel code. [Sugesh] Agreed, Will move the code to the tunneling code. The idea was to discard packets as early as possible before spending more CPU cycles on the invalid packets. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
