Regards _Sugesh
> -----Original Message----- > From: Kevin Traynor [mailto:ktray...@redhat.com] > Sent: Wednesday, May 17, 2017 10:10 AM > To: Chandran, Sugesh <sugesh.chand...@intel.com> > Cc: d...@openvswitch.org > Subject: Re: [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure. > > 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. > Yes, I tried to configure initially as well as run time. [Sugesh] Also, At the time of Rx checksum offload implementation, one of the comments suggests not to use any configuration option at all. Keep it ON for all the physical ports when it is supported. The reason being the configuration option is added is due to the vectorization. In the earlier DPDK releases the vectorization will turned off when checksum offload is enabled. I feel that is not the case for the latest DPDK releases (Vectorization is ON with checksum offload). If there is no impact of vectorization, then there is no usecase for having this configuration option at all. This way there is one less thing for the user to worry about. What do you think? Do you think it makes any backward compatibility issues?? > 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
