On 05/16/2017 05:48 PM, Chandran, Sugesh wrote: > Hi Kevin, > Thank you for sending out this patch series. > Have you tested the tunneling decap usecase with checksum offload? I am > seeing weird behavior when I testing the tunneling with Rx checksum offload > ON and OFF.(Seeing the same behavior on master as well) > > Here is the summary of issue with the steps, > > 1) Send tunnel traffic to OVS to do the decap. > 2) Set & unset the checksum offload. > 3) I don't see any performance difference in both case. > > Now I went ahead and put some debug message to see what is happening > > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c > index 2798324..49ca847 100644 > --- a/lib/netdev-native-tnl.c > +++ b/lib/netdev-native-tnl.c > @@ -86,6 +86,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, > struct flow_tnl *tnl, > ovs_be32 ip_src, ip_dst; > > if (OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) { > + VLOG_INFO("Checksum is not validated..."); > if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) { > VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum"); > return NULL; > @@ -182,6 +183,7 @@ udp_extract_tnl_md(struct dp_packet *packet, struct > flow_tnl *tnl, > > if (udp->udp_csum) { > if (OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) { > + VLOG_INFO("Checksum is not validated..."); > uint32_t csum; > if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) { > csum = packet_csum_pseudoheader6(dp_packet_l3(packet)); > sugeshch@silpixa00389816:~/repo/ovs_master$ > > These debug messages are not showing at all when I am sending the traffic. (I > tried it with rx checksum ON and OFF) > > Looks like ol_flags are always reporting checksum is good. > > I am using DPDK 17.02 for the testing. > If I remember correctly it was reporting properly at the time of rx checksum > offload. > Looks like DPDK is reporting checksum valid in all the cases even it is > disabled. > > Any inputs on this? >
Hi Sugesh, I was trying to fix the reconfiguration code not applying the OVSDB value so that's all I tested. I guess it's best to roll back to your original test and take it from there? You probably tested with DPDK 16.11.0 and I see some changes since then (e.g. below). Also, maybe you were testing enabled/disabled on first configure? It's the same configure code, but perhaps there is some different state in DPDK when the port is configured initially. commit f3a85f4ce04d9fb55ebdb392563f7c1711f3d943 Author: Qi Zhang <qi.z.zh...@intel.com> Date: Tue Jan 24 14:06:59 2017 -0500 net/i40e: fix checksum flag in x86 vector Rx When no error reported in Rx descriptor, we should set CKSUM_GOOD flag before return. Fixes: 9966a00a0688 ("net/i40e: enable bad checksum flags in vector Rx") Cc: sta...@dpdk.org Signed-off-by: Qi Zhang <qi.z.zh...@intel.com> HTH, Kevin. > > > Regards > _Sugesh > > >> -----Original Message----- >> From: Kevin Traynor [mailto:ktray...@redhat.com] >> Sent: Friday, May 12, 2017 6:22 PM >> To: d...@openvswitch.org >> Cc: Chandran, Sugesh <sugesh.chand...@intel.com>; Kevin Traynor >> <ktray...@redhat.com> >> Subject: [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure. >> >> Rx checksum offload is enabled by default where available, with >> reconfiguration through OVSDB options:rx-checksum-offload. >> However, setting rx-checksum-offload does not result in a reconfiguration of >> the NIC. >> >> Fix that by checking if the requested port config features (e.g. rx checksum >> offload) are currently applied on the NIC and if not, perform a >> reconfiguration. >> >> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature >> on DPDK physical ports.") >> Cc: Sugesh Chandran <sugesh.chand...@intel.com> >> Signed-off-by: Kevin Traynor <ktray...@redhat.com> >> --- >> lib/netdev-dpdk.c | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 609b8da..d1688ce >> 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -374,4 +374,5 @@ struct netdev_dpdk { >> int requested_rxq_size; >> int requested_txq_size; >> + uint32_t requested_hwol; >> >> /* Number of rx/tx descriptors for physical devices */ @@ -648,5 +649,5 >> @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int >> n_txq) >> conf.rxmode.max_rx_pkt_len = 0; >> } >> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >> + conf.rxmode.hw_ip_checksum = (dev->requested_hwol & >> NETDEV_RX_CHECKSUM_OFFLOAD) != 0; >> /* A device may report more queues than it makes available (this has @@ >> -702,4 +703,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, >> int n_rxq, int n_txq) >> dev->up.n_rxq = n_rxq; >> dev->up.n_txq = n_txq; >> + dev->hw_ol_features = dev->requested_hwol; >> >> return 0; >> @@ -719,5 +721,5 @@ dpdk_eth_checksum_offload_configure(struct >> netdev_dpdk *dev) >> 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; >> + rx_csum_ol_flag = (dev->requested_hwol & >> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0; >> >> if (rx_csum_ol_flag && >> @@ -726,5 +728,5 @@ dpdk_eth_checksum_offload_configure(struct >> netdev_dpdk *dev) >> VLOG_WARN_ONCE("Rx checksum offload is not supported on device >> %d", >> dev->port_id); >> - dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD; >> + dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD; >> return; >> } >> @@ -872,4 +874,5 @@ common_construct(struct netdev *netdev, unsigned >> int port_no, >> /* Initilize the hardware offload flags to 0 */ >> dev->hw_ol_features = 0; >> + dev->requested_hwol = 0; >> >> dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1260,5 +1263,5 @@ >> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, >> != 0; >> if (temp_flag != rx_chksm_ofld) { >> - dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD; >> + dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD; >> dpdk_eth_checksum_offload_configure(dev); >> } >> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev) >> && dev->rxq_size == dev->requested_rxq_size >> && dev->txq_size == dev->requested_txq_size >> - && dev->socket_id == dev->requested_socket_id) { >> + && dev->socket_id == dev->requested_socket_id >> + && dev->hw_ol_features == dev->requested_hwol) { >> /* Reconfiguration is unnecessary */ >> >> -- >> 1.8.3.1 > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev