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-boun...@openvswitch.org on behalf of Darrell Ball" 
<ovs-dev-boun...@openvswitch.org on behalf of db...@vmware.com> wrote:

    LGTM
    
    Acked by: Darrell Ball <dlu...@gmail.com>
    
    On 6/8/17, 10:12 AM, "ovs-dev-boun...@openvswitch.org on behalf of Kevin 
Traynor" <ovs-dev-boun...@openvswitch.org on behalf of ktray...@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 <ktray...@redhat.com>
        Suggested-by: Sugesh Chandran <sugesh.chand...@intel.com>
        Signed-off-by: Kevin Traynor <ktray...@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
        d...@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
    d...@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=
 
    

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to