diff mbox

[ovs-dev,v3,1/2] netdev-dpdk: Remove Rx checksum reconfigure.

Message ID 1496941940-10298-2-git-send-email-ktraynor@redhat.com
State Superseded
Delegated to: Darrell Ball
Headers show

Commit Message

Kevin Traynor June 8, 2017, 5:12 p.m. UTC 
Rx checksum offload is enabled by default on DPDK physical NICs
where available, with reconfiguration through
options:rx-checksum-offload. However, changing rx-checksum-offload
did not result in a reconfiguration of the NIC and wrong status is
reported for it.

As there seems to be diminishing reasons why a user would want
to disable Rx checksum offload, just remove the broken reconfiguration
option.

Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.")
Reported-by: Kevin Traynor <ktraynor@redhat.com>
Suggested-by: Sugesh Chandran <sugesh.chandran@intel.com>
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 Documentation/howto/dpdk.rst | 17 +---------------
 lib/netdev-dpdk.c            | 47 +++++++++++---------------------------------
 vswitchd/vswitch.xml         | 14 -------------
 3 files changed, 13 insertions(+), 65 deletions(-)

Comments

Darrell Ball June 8, 2017, 9:40 p.m. UTC | #1 
LGTM

Acked by: Darrell Ball <dlu998@gmail.com>

On 6/8/17, 10:12 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kevin Traynor" <ovs-dev-bounces@openvswitch.org on behalf of ktraynor@redhat.com> wrote:

    Rx checksum offload is enabled by default on DPDK physical NICs
    where available, with reconfiguration through
    options:rx-checksum-offload. However, changing rx-checksum-offload
    did not result in a reconfiguration of the NIC and wrong status is
    reported for it.
    
    As there seems to be diminishing reasons why a user would want
    to disable Rx checksum offload, just remove the broken reconfiguration
    option.
    
    Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.")
    Reported-by: Kevin Traynor <ktraynor@redhat.com>
    Suggested-by: Sugesh Chandran <sugesh.chandran@intel.com>
    Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
    ---
     Documentation/howto/dpdk.rst | 17 +---------------
     lib/netdev-dpdk.c            | 47 +++++++++++---------------------------------
     vswitchd/vswitch.xml         | 14 -------------
     3 files changed, 13 insertions(+), 65 deletions(-)
    
    diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
    index 93248b4..af01d3e 100644
    --- a/Documentation/howto/dpdk.rst
    +++ b/Documentation/howto/dpdk.rst
    @@ -277,16 +277,5 @@ Rx Checksum Offload
     -------------------
     
    -By default, DPDK physical ports are enabled with Rx checksum offload. Rx
    -checksum offload can be configured on a DPDK physical port either when adding
    -or at run time.
    -
    -To disable Rx checksum offload when adding a DPDK port dpdk-p0::
    -
    -    $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
    -      options:dpdk-devargs=0000:01:00.0 options:rx-checksum-offload=false
    -
    -Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk-p0::
    -
    -    $ ovs-vsctl set Interface dpdk-p0 options:rx-checksum-offload=false
    +By default, DPDK physical ports are enabled with Rx checksum offload.
     
     Rx checksum offload can offer performance improvement only for tunneling
    @@ -294,8 +283,4 @@ traffic in OVS-DPDK because the checksum validation of tunnel packets is
     offloaded to the NIC. Also enabling Rx checksum may slightly reduce the
     performance of non-tunnel traffic, specifically for smaller size packet.
    -DPDK vectorization is disabled when checksum offloading is configured on DPDK
    -physical ports which in turn effects the non-tunnel traffic performance.
    -So it is advised to turn off the Rx checksum offload for non-tunnel traffic use
    -cases to achieve the best performance.
     
     .. _extended-statistics:
    diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    index b770b70..79afda5 100644
    --- a/lib/netdev-dpdk.c
    +++ b/lib/netdev-dpdk.c
    @@ -719,27 +719,4 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
     
     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("Rx checksum offload is not supported on device %"PRIu8,
    -                       dev->port_id);
    -        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
    -        return;
    -    }
    -    netdev_request_reconfigure(&dev->up);
    -}
    -
    -static void
     dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
     {
    @@ -759,7 +736,19 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
         int diag;
         int n_rxq, n_txq;
    +    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);
     
    +    if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
    +            rx_chksm_offload_capa) {
    +        VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
    +                        dev->port_id);
    +        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
    +    } else {
    +        dev->hw_ol_features |= NETDEV_RX_CHECKSUM_OFFLOAD;
    +    }
    +
         n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
         n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
    @@ -1205,6 +1194,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
             {RTE_FC_RX_PAUSE, RTE_FC_FULL    }
         };
    -    bool rx_chksm_ofld;
    -    bool temp_flag;
         const char *new_devargs;
         int err = 0;
    @@ -1288,14 +1275,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
         }
     
    -    /* 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);
    -    }
    -
     out:
         ovs_mutex_unlock(&dev->mutex);
    diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
    index d219bfd..672772a 100644
    --- a/vswitchd/vswitch.xml
    +++ b/vswitchd/vswitch.xml
    @@ -3384,18 +3384,4 @@
         </group>
     
    -    <group title="Rx Checksum Offload Configuration">
    -      <p>
    -        The checksum validation on the incoming packets are performed on NIC
    -        using Rx checksum offload feature. Implemented only for <code>dpdk
    -        </code>physical interfaces.
    -      </p>
    -
    -      <column name="options" key="rx-checksum-offload"
    -              type='{"type": "boolean"}'>
    -        Set to <code>false</code> to disble Rx checksum offloading on <code>
    -        dpdk</code>physical ports. By default, Rx checksum offload is enabled.
    -      </column>
    -    </group>
    -
         <group title="Common Columns">
           The overall purpose of these columns is described under <code>Common
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=NSXw7qOD2HZzLP_rXKlsKwt8hRkPrRkHTyIlFQiOxsc&s=BWwu_9StV8xigBdp8zUXImjpD7vQd8MRNy5Prp-D35Y&e=
Darrell Ball June 9, 2017, 1:16 a.m. UTC | #2 
oops, I did notice a minor issue with the original code moved to the _init function
in this patch; in theory, it should be a separate patch.


On 6/8/17, 2:40 PM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf of dball@vmware.com> wrote:

    LGTM
    
    Acked by: Darrell Ball <dlu998@gmail.com>
    
    On 6/8/17, 10:12 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kevin Traynor" <ovs-dev-bounces@openvswitch.org on behalf of ktraynor@redhat.com> wrote:
    
        Rx checksum offload is enabled by default on DPDK physical NICs
        where available, with reconfiguration through
        options:rx-checksum-offload. However, changing rx-checksum-offload
        did not result in a reconfiguration of the NIC and wrong status is
        reported for it.
        
        As there seems to be diminishing reasons why a user would want
        to disable Rx checksum offload, just remove the broken reconfiguration
        option.
        
        Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.")
        Reported-by: Kevin Traynor <ktraynor@redhat.com>
        Suggested-by: Sugesh Chandran <sugesh.chandran@intel.com>
        Signed-off-by: Kevin Traynor <ktraynor@redhat.com>

        ---
         Documentation/howto/dpdk.rst | 17 +---------------
         lib/netdev-dpdk.c            | 47 +++++++++++---------------------------------
         vswitchd/vswitch.xml         | 14 -------------
         3 files changed, 13 insertions(+), 65 deletions(-)
        
        diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
        index 93248b4..af01d3e 100644
        --- a/Documentation/howto/dpdk.rst
        +++ b/Documentation/howto/dpdk.rst
        @@ -277,16 +277,5 @@ Rx Checksum Offload
         -------------------
         
        -By default, DPDK physical ports are enabled with Rx checksum offload. Rx
        -checksum offload can be configured on a DPDK physical port either when adding
        -or at run time.
        -
        -To disable Rx checksum offload when adding a DPDK port dpdk-p0::
        -
        -    $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
        -      options:dpdk-devargs=0000:01:00.0 options:rx-checksum-offload=false
        -
        -Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk-p0::
        -
        -    $ ovs-vsctl set Interface dpdk-p0 options:rx-checksum-offload=false
        +By default, DPDK physical ports are enabled with Rx checksum offload.
         
         Rx checksum offload can offer performance improvement only for tunneling
        @@ -294,8 +283,4 @@ traffic in OVS-DPDK because the checksum validation of tunnel packets is
         offloaded to the NIC. Also enabling Rx checksum may slightly reduce the
         performance of non-tunnel traffic, specifically for smaller size packet.
        -DPDK vectorization is disabled when checksum offloading is configured on DPDK
        -physical ports which in turn effects the non-tunnel traffic performance.
        -So it is advised to turn off the Rx checksum offload for non-tunnel traffic use
        -cases to achieve the best performance.
         
         .. _extended-statistics:
        diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
        index b770b70..79afda5 100644
        --- a/lib/netdev-dpdk.c
        +++ b/lib/netdev-dpdk.c
        @@ -719,27 +719,4 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
         
         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("Rx checksum offload is not supported on device %"PRIu8,
        -                       dev->port_id);
        -        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
        -        return;
        -    }
        -    netdev_request_reconfigure(&dev->up);
        -}
        -
        -static void
         dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
         {
        @@ -759,7 +736,19 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
             int diag;
             int n_rxq, n_txq;
        +    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);
         
        +    if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
        +            rx_chksm_offload_capa) {
        +        VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
        +                        dev->port_id);

I don’t think we want _ONCE for this.


        +        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
        +    } else {
        +        dev->hw_ol_features |= NETDEV_RX_CHECKSUM_OFFLOAD;
        +    }
        +
             n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
             n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
        @@ -1205,6 +1194,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
                 {RTE_FC_RX_PAUSE, RTE_FC_FULL    }
             };
        -    bool rx_chksm_ofld;
        -    bool temp_flag;
             const char *new_devargs;
             int err = 0;
        @@ -1288,14 +1275,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
             }
         
        -    /* 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);
        -    }
        -
         out:
             ovs_mutex_unlock(&dev->mutex);
        diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
        index d219bfd..672772a 100644
        --- a/vswitchd/vswitch.xml
        +++ b/vswitchd/vswitch.xml
        @@ -3384,18 +3384,4 @@
             </group>
         
        -    <group title="Rx Checksum Offload Configuration">
        -      <p>
        -        The checksum validation on the incoming packets are performed on NIC
        -        using Rx checksum offload feature. Implemented only for <code>dpdk
        -        </code>physical interfaces.
        -      </p>
        -
        -      <column name="options" key="rx-checksum-offload"
        -              type='{"type": "boolean"}'>
        -        Set to <code>false</code> to disble Rx checksum offloading on <code>
        -        dpdk</code>physical ports. By default, Rx checksum offload is enabled.
        -      </column>
        -    </group>
        -
             <group title="Common Columns">
               The overall purpose of these columns is described under <code>Common
        -- 
        1.8.3.1
        
        _______________________________________________
        dev mailing list
        dev@openvswitch.org
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=NSXw7qOD2HZzLP_rXKlsKwt8hRkPrRkHTyIlFQiOxsc&s=BWwu_9StV8xigBdp8zUXImjpD7vQd8MRNy5Prp-D35Y&e= 
        
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=qjE-blVbdo4YrmEEO3WtAc0Ov07SHlqxTGMC5dBFL_g&s=x2gW--rPdIVP2W0dsxrHq2DNyRRZmTBsVSqWagQNbvw&e=
Kevin Traynor June 9, 2017, 10:39 a.m. UTC | #3 
On 06/09/2017 02:16 AM, Darrell Ball wrote:
> oops, I did notice a minor issue with the original code moved to the _init function
> in this patch; in theory, it should be a separate patch.
> 

Thanks for reviewing. yeah, I agree any changes to the how it's logged
should be a separate patch.

> 
> On 6/8/17, 2:40 PM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf of dball@vmware.com> wrote:
> 
>     LGTM
>     
>     Acked by: Darrell Ball <dlu998@gmail.com>
>     
>     On 6/8/17, 10:12 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kevin Traynor" <ovs-dev-bounces@openvswitch.org on behalf of ktraynor@redhat.com> wrote:
>     
>         Rx checksum offload is enabled by default on DPDK physical NICs
>         where available, with reconfiguration through
>         options:rx-checksum-offload. However, changing rx-checksum-offload
>         did not result in a reconfiguration of the NIC and wrong status is
>         reported for it.
>         
>         As there seems to be diminishing reasons why a user would want
>         to disable Rx checksum offload, just remove the broken reconfiguration
>         option.
>         
>         Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.")
>         Reported-by: Kevin Traynor <ktraynor@redhat.com>
>         Suggested-by: Sugesh Chandran <sugesh.chandran@intel.com>
>         Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>         ---
>          Documentation/howto/dpdk.rst | 17 +---------------
>          lib/netdev-dpdk.c            | 47 +++++++++++---------------------------------
>          vswitchd/vswitch.xml         | 14 -------------
>          3 files changed, 13 insertions(+), 65 deletions(-)
>         
>         diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
>         index 93248b4..af01d3e 100644
>         --- a/Documentation/howto/dpdk.rst
>         +++ b/Documentation/howto/dpdk.rst
>         @@ -277,16 +277,5 @@ Rx Checksum Offload
>          -------------------
>          
>         -By default, DPDK physical ports are enabled with Rx checksum offload. Rx
>         -checksum offload can be configured on a DPDK physical port either when adding
>         -or at run time.
>         -
>         -To disable Rx checksum offload when adding a DPDK port dpdk-p0::
>         -
>         -    $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
>         -      options:dpdk-devargs=0000:01:00.0 options:rx-checksum-offload=false
>         -
>         -Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk-p0::
>         -
>         -    $ ovs-vsctl set Interface dpdk-p0 options:rx-checksum-offload=false
>         +By default, DPDK physical ports are enabled with Rx checksum offload.
>          
>          Rx checksum offload can offer performance improvement only for tunneling
>         @@ -294,8 +283,4 @@ traffic in OVS-DPDK because the checksum validation of tunnel packets is
>          offloaded to the NIC. Also enabling Rx checksum may slightly reduce the
>          performance of non-tunnel traffic, specifically for smaller size packet.
>         -DPDK vectorization is disabled when checksum offloading is configured on DPDK
>         -physical ports which in turn effects the non-tunnel traffic performance.
>         -So it is advised to turn off the Rx checksum offload for non-tunnel traffic use
>         -cases to achieve the best performance.
>          
>          .. _extended-statistics:
>         diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>         index b770b70..79afda5 100644
>         --- a/lib/netdev-dpdk.c
>         +++ b/lib/netdev-dpdk.c
>         @@ -719,27 +719,4 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
>          
>          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("Rx checksum offload is not supported on device %"PRIu8,
>         -                       dev->port_id);
>         -        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>         -        return;
>         -    }
>         -    netdev_request_reconfigure(&dev->up);
>         -}
>         -
>         -static void
>          dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
>          {
>         @@ -759,7 +736,19 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>              int diag;
>              int n_rxq, n_txq;
>         +    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);
>          
>         +    if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
>         +            rx_chksm_offload_capa) {
>         +        VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
>         +                        dev->port_id);
> 
> I don’t think we want _ONCE for this.
> 

I will change this and also use "port" instead of "device" as otherwise
we get this

2017-06-09T09:36:28Z|00088|netdev_dpdk|WARN|Rx checksum offload is not
supported on device 0
2017-06-09T09:36:28Z|00089|netdev_dpdk|INFO|Port 0: ec:f4:bb:db:fc:38


> 
>         +        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>         +    } else {
>         +        dev->hw_ol_features |= NETDEV_RX_CHECKSUM_OFFLOAD;
>         +    }
>         +
>              n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
>              n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
>         @@ -1205,6 +1194,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>                  {RTE_FC_RX_PAUSE, RTE_FC_FULL    }
>              };
>         -    bool rx_chksm_ofld;
>         -    bool temp_flag;
>              const char *new_devargs;
>              int err = 0;
>         @@ -1288,14 +1275,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>              }
>          
>         -    /* 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);
>         -    }
>         -
>          out:
>              ovs_mutex_unlock(&dev->mutex);
>         diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>         index d219bfd..672772a 100644
>         --- a/vswitchd/vswitch.xml
>         +++ b/vswitchd/vswitch.xml
>         @@ -3384,18 +3384,4 @@
>              </group>
>          
>         -    <group title="Rx Checksum Offload Configuration">
>         -      <p>
>         -        The checksum validation on the incoming packets are performed on NIC
>         -        using Rx checksum offload feature. Implemented only for <code>dpdk
>         -        </code>physical interfaces.
>         -      </p>
>         -
>         -      <column name="options" key="rx-checksum-offload"
>         -              type='{"type": "boolean"}'>
>         -        Set to <code>false</code> to disble Rx checksum offloading on <code>
>         -        dpdk</code>physical ports. By default, Rx checksum offload is enabled.
>         -      </column>
>         -    </group>
>         -
>              <group title="Common Columns">
>                The overall purpose of these columns is described under <code>Common
>         -- 
>         1.8.3.1
>         
>         _______________________________________________
>         dev mailing list
>         dev@openvswitch.org
>         https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=NSXw7qOD2HZzLP_rXKlsKwt8hRkPrRkHTyIlFQiOxsc&s=BWwu_9StV8xigBdp8zUXImjpD7vQd8MRNy5Prp-D35Y&e= 
>         
>     
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org
>     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=qjE-blVbdo4YrmEEO3WtAc0Ov07SHlqxTGMC5dBFL_g&s=x2gW--rPdIVP2W0dsxrHq2DNyRRZmTBsVSqWagQNbvw&e= 
>     
>
diff mbox

Patch

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 93248b4..af01d3e 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -277,16 +277,5 @@  Rx Checksum Offload
 -------------------
 
-By default, DPDK physical ports are enabled with Rx checksum offload. Rx
-checksum offload can be configured on a DPDK physical port either when adding
-or at run time.
-
-To disable Rx checksum offload when adding a DPDK port dpdk-p0::
-
-    $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
-      options:dpdk-devargs=0000:01:00.0 options:rx-checksum-offload=false
-
-Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk-p0::
-
-    $ ovs-vsctl set Interface dpdk-p0 options:rx-checksum-offload=false
+By default, DPDK physical ports are enabled with Rx checksum offload.
 
 Rx checksum offload can offer performance improvement only for tunneling
@@ -294,8 +283,4 @@  traffic in OVS-DPDK because the checksum validation of tunnel packets is
 offloaded to the NIC. Also enabling Rx checksum may slightly reduce the
 performance of non-tunnel traffic, specifically for smaller size packet.
-DPDK vectorization is disabled when checksum offloading is configured on DPDK
-physical ports which in turn effects the non-tunnel traffic performance.
-So it is advised to turn off the Rx checksum offload for non-tunnel traffic use
-cases to achieve the best performance.
 
 .. _extended-statistics:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index b770b70..79afda5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -719,27 +719,4 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
 
 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("Rx checksum offload is not supported on device %"PRIu8,
-                       dev->port_id);
-        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
-        return;
-    }
-    netdev_request_reconfigure(&dev->up);
-}
-
-static void
 dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
 {
@@ -759,7 +736,19 @@  dpdk_eth_dev_init(struct netdev_dpdk *dev)
     int diag;
     int n_rxq, n_txq;
+    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);
 
+    if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
+            rx_chksm_offload_capa) {
+        VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
+                        dev->port_id);
+        dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
+    } else {
+        dev->hw_ol_features |= NETDEV_RX_CHECKSUM_OFFLOAD;
+    }
+
     n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
     n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
@@ -1205,6 +1194,4 @@  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
         {RTE_FC_RX_PAUSE, RTE_FC_FULL    }
     };
-    bool rx_chksm_ofld;
-    bool temp_flag;
     const char *new_devargs;
     int err = 0;
@@ -1288,14 +1275,4 @@  netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
     }
 
-    /* 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);
-    }
-
 out:
     ovs_mutex_unlock(&dev->mutex);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index d219bfd..672772a 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3384,18 +3384,4 @@ 
     </group>
 
-    <group title="Rx Checksum Offload Configuration">
-      <p>
-        The checksum validation on the incoming packets are performed on NIC
-        using Rx checksum offload feature. Implemented only for <code>dpdk
-        </code>physical interfaces.
-      </p>
-
-      <column name="options" key="rx-checksum-offload"
-              type='{"type": "boolean"}'>
-        Set to <code>false</code> to disble Rx checksum offloading on <code>
-        dpdk</code>physical ports. By default, Rx checksum offload is enabled.
-      </column>
-    </group>
-
     <group title="Common Columns">
       The overall purpose of these columns is described under <code>Common