Regards _Sugesh
> -----Original Message----- > From: Pravin Shelar [mailto:[email protected]] > Sent: Wednesday, December 14, 2016 9:42 PM > To: Chandran, Sugesh <[email protected]> > Cc: ovs dev <[email protected]>; Jesse Gross <[email protected]>; Ben > Pfaff <[email protected]> > Subject: Re: [ovs-dev] [PATCH v7] netdev-dpdk: Enable Rx checksum > offloading feature on DPDK physical ports. > > Thanks for the patch. I have couple of questions. > > On Wed, Dec 14, 2016 at 7:30 AM, Sugesh Chandran > <[email protected]> wrote: > > Add Rx checksum offloading feature support on DPDK physical ports. By > > default, the Rx checksum offloading is enabled if NIC supports. > > However, the checksum offloading can be turned OFF either while adding > > a new DPDK physical port to OVS or at runtime. > > > > The rx checksum offloading can be turned off by setting the parameter > > to 'false'. For eg: To disable the rx checksum offloading when adding > > a port, > > > > 'ovs-vsctl add-port br0 dpdk0 -- \ > > set Interface dpdk0 type=dpdk options:rx-checksum-offload=false' > > > > OR (to disable at run time after port is being added to OVS) > > > > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false' > > > > Similarly to turn ON rx checksum offloading at run time, > > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true' > > > > The Tx checksum offloading support is not implemented due to the > > following reasons. > > > > 1) Checksum offloading and vectorization are mutually exclusive in > > DPDK poll mode driver. Vector packet processing is turned OFF when > > checksum offloading is enabled which causes significant performance drop > at Tx side. > > > > 2) Normally, OVS generates checksum for tunnel packets in software at > > the 'tunnel push' operation, where the tunnel headers are created. > > However enabling Tx checksum offloading involves, > > > > *) Mark every packets for tx checksum offloading at 'tunnel_push' and > > recirculate. > > *) At the time of xmit, validate the same flag and instruct the NIC to > > do the checksum calculation. In case NIC doesnt support Tx checksum > > offloading, the checksum calculation has to be done in software before > > sending out the packets. > > > > No significant performance improvement noticed with Tx checksum > > offloading due to the e overhead of additional validations + non vector > packet processing. > > In some test scenarios, it introduces performance drop too. > > > > Rx checksum offloading still offers 8-9% of improvement on VxLAN > > tunneling decapsulation even though the SSE vector Rx function is > > disabled in DPDK poll mode driver. > > > > Signed-off-by: Sugesh Chandran <[email protected]> > > Acked-by: Jesse Gross <[email protected]> > > > ... > ... > > static void > > +dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev) > > + OVS_REQUIRES(dev->mutex) > > +{ > > + struct rte_eth_dev_info info; > > + bool rx_csum_ol_flag = false; > > + uint32_t rx_chksm_offload_capa = DEV_RX_OFFLOAD_UDP_CKSUM | > > + DEV_RX_OFFLOAD_TCP_CKSUM | > > + DEV_RX_OFFLOAD_IPV4_CKSUM; > > + rte_eth_dev_info_get(dev->port_id, &info); > > + rx_csum_ol_flag = (dev->hw_ol_features & > > +NETDEV_RX_CHECKSUM_OFFLOAD) != 0; > > + > > + if (rx_csum_ol_flag && > > + (info.rx_offload_capa & rx_chksm_offload_capa) != > > + rx_chksm_offload_capa) { > > + VLOG_WARN_ONCE("Failed to enable Rx checksum offload on device > %d", > > + dev->port_id); > > + dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD; > > + return; > > + } > > + netdev_request_reconfigure(&dev->up); > > +} > > + > I am not sure about need for netdev reconfigure here. [Sugesh] Reconfigure is called here to enable/disable the checksum at run time. Do you think the checksum will be updated on a user request without having this call? > > > +static void > > dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) > > OVS_REQUIRES(dev->mutex) { > > if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) { @@ > > -851,6 +884,9 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int > > port_no, > > > > /* Initialize the flow control to NULL */ > > memset(&dev->fc_conf, 0, sizeof dev->fc_conf); > > + > > + /* Initilize the hardware offload flags to 0 */ > > + dev->hw_ol_features = 0; > > if (type == DPDK_DEV_ETH) { > > err = dpdk_eth_dev_init(dev); > > if (err) { > > @@ -1118,6 +1154,8 @@ netdev_dpdk_set_config(struct netdev *netdev, > const struct smap *args) > > {RTE_FC_NONE, RTE_FC_TX_PAUSE}, > > {RTE_FC_RX_PAUSE, RTE_FC_FULL } > > }; > > + bool rx_chksm_ofld; > > + bool temp_flag; > > > > ovs_mutex_lock(&dev->mutex); > > > > @@ -1141,6 +1179,15 @@ netdev_dpdk_set_config(struct netdev > *netdev, const struct smap *args) > > dpdk_eth_flow_ctrl_setup(dev); > > } > > > > + /* Rx checksum offload configuration */ > > + /* By default the Rx checksum offload is ON */ > > + rx_chksm_ofld = smap_get_bool(args, "rx-checksum-offload", true); > > + temp_flag = (dev->hw_ol_features & > NETDEV_RX_CHECKSUM_OFFLOAD) > > + != 0; > > + if (temp_flag != rx_chksm_ofld) { > > + dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD; > > + dpdk_eth_checksum_offload_configure(dev); > > + } > > ovs_mutex_unlock(&dev->mutex); > > > Can you also reflect this feature back to user via get-status. > > > return 0; > > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index > > ce2582f..c730e72 100644 > > --- a/lib/netdev-native-tnl.c > > +++ b/lib/netdev-native-tnl.c > > @@ -85,9 +85,11 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet > > *packet, struct flow_tnl *tnl, > > > > ovs_be32 ip_src, ip_dst; > > > > - if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) { > > - VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum"); > > - return NULL; > > + if (OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) { > > + if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) { > > + VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum"); > > + return NULL; > > + } > > } > > > > if (ntohs(ip->ip_tot_len) > l3_size) { @@ -179,18 +181,21 @@ > > udp_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl, > > } > > > > if (udp->udp_csum) { > > - uint32_t csum; > > - if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) { > > - csum = packet_csum_pseudoheader6(dp_packet_l3(packet)); > > - } else { > > - csum = packet_csum_pseudoheader(dp_packet_l3(packet)); > > - } > > - > > - csum = csum_continue(csum, udp, dp_packet_size(packet) - > > - ((const unsigned char *)udp - > > - (const unsigned char > > *)dp_packet_l2(packet))); > > - if (csum_finish(csum)) { > > - return NULL; > > + if (OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) { > > + uint32_t csum; > > + if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) { > > + csum = packet_csum_pseudoheader6(dp_packet_l3(packet)); > > + } else { > > + csum = packet_csum_pseudoheader(dp_packet_l3(packet)); > > + } > > + > > + csum = csum_continue(csum, udp, dp_packet_size(packet) - > > + ((const unsigned char *)udp - > > + (const unsigned char > > *)dp_packet_l2(packet) > > + )); > > + if (csum_finish(csum)) { > > + return NULL; > > + } > > Can we make use of RX checksum offload for userspace conntrack lib? I think > there is already code duplication in checksum verification, we could refactor > code around checksum API with hardware offload to avoid it and likely > improve performance. [Sugesh] This is also a good place where we can think of using the Rx checksum offload. As I mentioned in the commit message, the checksum offload disables vectorization which impacts the performance. We must need to test the checksum offload in different conn-track use cases to make sure the checksum offload really offers the performance improvement before refactoring the code. I just had a glance on conntrack lib, it seems only L4 checksum is validated when compared to the tunneling case where both IP and L4 checksum validated for every packets. I can modify the conntrack lib and test to see if any performance improvement can be achieved with Rx checksum offload. If it offers acceptable performance boost, I will send out another patch to enable Rx checksum offload in conntrack module + refactoring the checksum offload around checksum API. What do you think? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
