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.
> +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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
