diff mbox series

[ovs-dev,v1] netdev-dpdk: Enable HW_CRC_STRIP for virtual functions.

Message ID 1525353924-27686-1-git-send-email-ian.stokes@intel.com
State Accepted
Delegated to: Ian Stokes
Headers show
Series [ovs-dev,v1] netdev-dpdk: Enable HW_CRC_STRIP for virtual functions. | expand

Commit Message

Stokes, Ian May 3, 2018, 1:25 p.m. UTC 
From: Michal Weglicki <michalx.weglicki@intel.com>

Virtual functions such as igb_vf and i40e_vf require HW_CRC_STRIP to be
explicitly enabled before configuration, otherwise device configuration
will fail.

This commit achieves this by adding NETDEV_RX_HW_CRC_STRIP to
dpdk_hw_ol_features. When a dpdk device is added, the driver for the
device is examined, if the device is a virtual function enable
HW_CRC_STRIP.

Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
---
 lib/netdev-dpdk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Ferriter, Cian May 25, 2018, 12:30 p.m. UTC | #1 
Hi,

I did all my testing with DPDK 17.11.2 and the following OVS commit (patch doesn't apply cleanly to master):
cferrite@silpixa00393943:~/ovs$ git show --summary
commit 6b71df2fc06921758798e41833d67d9daf647d19
Author: Kevin Traynor <ktraynor@redhat.com>
Date:   Wed Apr 25 12:20:53 2018 +0100

    faq: Document DPDK version maintenance.

Before the patch was applied, the following was seen when a VF port is added:
cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0
ovs-vsctl: Error detected while setting up 'dpdk0': could not add network device dpdk0 to ofproto (No such device).  See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/usr/local/var/log/openvswitch".

The vswitchd log after the VF port is added:
2018-05-25T09:31:16Z|00080|dpif_netdev|INFO|PMD thread on numa_id: 0, core id:  3 created.
2018-05-25T09:31:16Z|00081|dpif_netdev|INFO|There are 1 pmd threads on numa node 0
2018-05-25T09:31:16Z|00082|dpdk|ERR|i40evf_dev_configure(): VF can't disable HW CRC Strip
2018-05-25T09:31:16Z|00083|netdev_dpdk|WARN|Interface dpdk0 eth_dev setup error Invalid argument
2018-05-25T09:31:16Z|00084|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2 lsc interrupt mode:false) configure error: Invalid argument
2018-05-25T09:31:16Z|00085|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration
2018-05-25T09:31:16Z|00086|bridge|WARN|could not add network device dpdk0 to ofproto (No such device)

After the patch is applied there is no error message when the VF port is added:
cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0

The vswitchd log after the VF port is added:
2018-05-25T10:00:31Z|00081|netdev_dpdk|INFO|Virtual function detected, HW_CRC_STRIP will be enabled
2018-05-25T10:00:33Z|00082|dpdk|WARN|i40evf_execute_vf_cmd(): No response for 28
2018-05-25T10:00:33Z|00083|dpdk|ERR|i40evf_disable_vlan_strip(): Failed to execute command of VIRTCHNL_OP_DISABLE_VLAN_STRIPPING
2018-05-25T10:00:33Z|00084|dpdk|ERR|i40evf_config_promisc(): fail to execute command CONFIG_PROMISCUOUS_MODE
2018-05-25T10:00:33Z|00085|dpdk|ERR|i40evf_config_promisc(): fail to execute command CONFIG_PROMISCUOUS_MODE

There are errors related to promiscuous mode, but there is a message stating that CRC Stripping will be enabled so the patch is working for me.

Appctl query on the port:
cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-appctl dpif/show
netdev@ovs-netdev: hit:0 missed:0
        br0:
                br0 65534/1: (tap)
                dpdk0 1/2: (dpdk: configured_rx_queues=1, configured_rxq_descriptors=2048, configured_tx_queues=2, configured_txq_descriptors=2048, mtu=1500, requested_rx_queues=1, requested_rxq_descriptors=2048, requested_tx_queues=2, requested_txq_descriptors=2048, rx_csum_offload=true)


I have also successfully passed traffic into this VF port and out a regular PF port as summarised by the below CMDs:
sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0
sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk1 -- set Interface dpdk1 type=dpdk options:dpdk-devargs=0000:07:00.1
sudo $OVS_DIR/utilities/ovs-ofctl del-flows br0
sudo $OVS_DIR/utilities/ovs-ofctl add-flow br0 idle_timeout=0,in_port=1,action=output:2
sudo $OVS_DIR/utilities/ovs-ofctl add-flow br0 idle_timeout=0,in_port=2,action=output:1

The two findings from my testing:
* There are error messages coming from DPDK about promiscuous mode on the port. Is this something that should be disabled or otherwise considered in the next revision of the patch?
* The patch doesn't apply cleanly to master. It looks like this is caused by the following patch:
cferrite@silpixa00393943:~/ovs$ git show 65a87968f4
commit 65a87968f4cfd9cf7a433a3156d98118078f9e4e
Author: Pablo Cascón <pablo.cascon@netronome.com>
Date:   Fri Apr 27 17:40:49 2018 +0100

    netdev-dpdk: don't enable scatter for jumbo RX support for nfp

I have one comment about the code below, if this is addressed/resolved I'm happy to give:

Acked-by: Cian Ferriter <cian.ferriter@intel.com>
Tested-by: Cian Ferriter <cian.ferriter@intel.com>

Thanks,
Cian

> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Ian Stokes
> Sent: 03 May 2018 14:25
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH v1] netdev-dpdk: Enable HW_CRC_STRIP for
> virtual functions.
> 
> From: Michal Weglicki <michalx.weglicki@intel.com>
> 
> Virtual functions such as igb_vf and i40e_vf require HW_CRC_STRIP to be
> explicitly enabled before configuration, otherwise device configuration will
> fail.
> 
> This commit achieves this by adding NETDEV_RX_HW_CRC_STRIP to
> dpdk_hw_ol_features. When a dpdk device is added, the driver for the
> device is examined, if the device is a virtual function enable HW_CRC_STRIP.
> 
> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> ---
>  lib/netdev-dpdk.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 3306b19..50c7619
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -341,6 +341,7 @@ struct ingress_policer {
> 
>  enum dpdk_hw_ol_features {
>      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
> +    NETDEV_RX_HW_CRC_STRIP = 1 << 1,
>  };
> 
>  /*
> @@ -779,6 +780,11 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev, int n_rxq, int n_txq)
> 
>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> +
> +    if ((dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) {
> +        conf.rxmode.hw_strip_crc = 1;
> +    }
> +

It seems that there's an extra set of parenthesis around the above if statement. Is this intentional?

>      /* A device may report more queues than it makes available (this has
>       * been observed for Intel xl710, which reserves some of them for
>       * SRIOV):  rte_eth_*_queue_setup will fail if a queue is not @@ -868,6
> +874,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> 
>      rte_eth_dev_info_get(dev->port_id, &info);
> 
> +    if (strstr(info.driver_name, "vf") != NULL) {
> +        VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be
> enabled");
> +        dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
> +    } else {
> +        dev->hw_ol_features &= ~NETDEV_RX_HW_CRC_STRIP;
> +    }
> +
>      if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
>              rx_chksm_offload_capa) {
>          VLOG_WARN("Rx checksum offload is not supported on port "
> --
> 2.7.5
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian May 30, 2018, 1:25 p.m. UTC | #2 
> Hi,
> 
> I did all my testing with DPDK 17.11.2 and the following OVS commit (patch
> doesn't apply cleanly to master):
> cferrite@silpixa00393943:~/ovs$ git show --summary commit
> 6b71df2fc06921758798e41833d67d9daf647d19
> Author: Kevin Traynor <ktraynor@redhat.com>
> Date:   Wed Apr 25 12:20:53 2018 +0100
> 
>     faq: Document DPDK version maintenance.
> 
> Before the patch was applied, the following was seen when a VF port is
> added:
> cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10
> add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-
> devargs=0000:07:02.0
> ovs-vsctl: Error detected while setting up 'dpdk0': could not add network
> device dpdk0 to ofproto (No such device).  See ovs-vswitchd log for
> details.
> ovs-vsctl: The default log directory is "/usr/local/var/log/openvswitch".
> 
> The vswitchd log after the VF port is added:
> 2018-05-25T09:31:16Z|00080|dpif_netdev|INFO|PMD thread on numa_id: 0, core
> id:  3 created.
> 2018-05-25T09:31:16Z|00081|dpif_netdev|INFO|There are 1 pmd threads on
> numa node 0
> 2018-05-25T09:31:16Z|00082|dpdk|ERR|i40evf_dev_configure(): VF can't
> disable HW CRC Strip 2018-05-25T09:31:16Z|00083|netdev_dpdk|WARN|Interface
> dpdk0 eth_dev setup error Invalid argument 2018-05-
> 25T09:31:16Z|00084|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2 lsc
> interrupt mode:false) configure error: Invalid argument 2018-05-
> 25T09:31:16Z|00085|dpif_netdev|ERR|Failed to set interface dpdk0 new
> configuration 2018-05-25T09:31:16Z|00086|bridge|WARN|could not add network
> device dpdk0 to ofproto (No such device)
> 
> After the patch is applied there is no error message when the VF port is
> added:
> cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10
> add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-
> devargs=0000:07:02.0
> 
> The vswitchd log after the VF port is added:
> 2018-05-25T10:00:31Z|00081|netdev_dpdk|INFO|Virtual function detected,
> HW_CRC_STRIP will be enabled
> 2018-05-25T10:00:33Z|00082|dpdk|WARN|i40evf_execute_vf_cmd(): No response
> for 28
> 2018-05-25T10:00:33Z|00083|dpdk|ERR|i40evf_disable_vlan_strip(): Failed to
> execute command of VIRTCHNL_OP_DISABLE_VLAN_STRIPPING
> 2018-05-25T10:00:33Z|00084|dpdk|ERR|i40evf_config_promisc(): fail to
> execute command CONFIG_PROMISCUOUS_MODE
> 2018-05-25T10:00:33Z|00085|dpdk|ERR|i40evf_config_promisc(): fail to
> execute command CONFIG_PROMISCUOUS_MODE

Hmmm, strange, I've tested a VF for xl710 (i40e), i350 (IGB) and an 82599ES (ixgbe) devices but did not see the i40evf_config_promisc() error above in the logs.

We could look at disabling the promiscuous flag in the case for SRIOV. Typically SRIOV don't allow promiscuous mode as a security step, in the case where the device is in pass through mode to a VM it can be preferred to stop devices receiving traffic that is destined to another VF interface. 

I'll have to look at this a little bit further. I will also fix the comment you flagged below.

Thanks
Ian

> 
> There are errors related to promiscuous mode, but there is a message
> stating that CRC Stripping will be enabled so the patch is working for me.
> 
> Appctl query on the port:
> cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-appctl dpif/show
> netdev@ovs-netdev: hit:0 missed:0
>         br0:
>                 br0 65534/1: (tap)
>                 dpdk0 1/2: (dpdk: configured_rx_queues=1,
> configured_rxq_descriptors=2048, configured_tx_queues=2,
> configured_txq_descriptors=2048, mtu=1500, requested_rx_queues=1,
> requested_rxq_descriptors=2048, requested_tx_queues=2,
> requested_txq_descriptors=2048, rx_csum_offload=true)
> 
> 
> I have also successfully passed traffic into this VF port and out a
> regular PF port as summarised by the below CMDs:
> sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 -- set
> Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0 sudo
> $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk1 -- set
> Interface dpdk1 type=dpdk options:dpdk-devargs=0000:07:00.1 sudo
> $OVS_DIR/utilities/ovs-ofctl del-flows br0 sudo $OVS_DIR/utilities/ovs-
> ofctl add-flow br0 idle_timeout=0,in_port=1,action=output:2
> sudo $OVS_DIR/utilities/ovs-ofctl add-flow br0
> idle_timeout=0,in_port=2,action=output:1
> 
> The two findings from my testing:
> * There are error messages coming from DPDK about promiscuous mode on the
> port. Is this something that should be disabled or otherwise considered in
> the next revision of the patch?
> * The patch doesn't apply cleanly to master. It looks like this is caused
> by the following patch:
> cferrite@silpixa00393943:~/ovs$ git show 65a87968f4 commit
> 65a87968f4cfd9cf7a433a3156d98118078f9e4e
> Author: Pablo Cascón <pablo.cascon@netronome.com>
> Date:   Fri Apr 27 17:40:49 2018 +0100
> 
>     netdev-dpdk: don't enable scatter for jumbo RX support for nfp
> 
> I have one comment about the code below, if this is addressed/resolved I'm
> happy to give:
> 
> Acked-by: Cian Ferriter <cian.ferriter@intel.com>
> Tested-by: Cian Ferriter <cian.ferriter@intel.com>
> 
> Thanks,
> Cian
> 
> > -----Original Message-----
> > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > bounces@openvswitch.org] On Behalf Of Ian Stokes
> > Sent: 03 May 2018 14:25
> > To: dev@openvswitch.org
> > Subject: [ovs-dev] [PATCH v1] netdev-dpdk: Enable HW_CRC_STRIP for
> > virtual functions.
> >
> > From: Michal Weglicki <michalx.weglicki@intel.com>
> >
> > Virtual functions such as igb_vf and i40e_vf require HW_CRC_STRIP to
> > be explicitly enabled before configuration, otherwise device
> > configuration will fail.
> >
> > This commit achieves this by adding NETDEV_RX_HW_CRC_STRIP to
> > dpdk_hw_ol_features. When a dpdk device is added, the driver for the
> > device is examined, if the device is a virtual function enable
> HW_CRC_STRIP.
> >
> > Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> > ---
> >  lib/netdev-dpdk.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > 3306b19..50c7619
> > 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -341,6 +341,7 @@ struct ingress_policer {
> >
> >  enum dpdk_hw_ol_features {
> >      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
> > +    NETDEV_RX_HW_CRC_STRIP = 1 << 1,
> >  };
> >
> >  /*
> > @@ -779,6 +780,11 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
> > int n_rxq, int n_txq)
> >
> >      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> >                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> > +
> > +    if ((dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) {
> > +        conf.rxmode.hw_strip_crc = 1;
> > +    }
> > +
> 
> It seems that there's an extra set of parenthesis around the above if
> statement. Is this intentional?

Sure will fix this for the v2.

Ian
> 
> >      /* A device may report more queues than it makes available (this
> has
> >       * been observed for Intel xl710, which reserves some of them for
> >       * SRIOV):  rte_eth_*_queue_setup will fail if a queue is not @@
> > -868,6
> > +874,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> >
> >      rte_eth_dev_info_get(dev->port_id, &info);
> >
> > +    if (strstr(info.driver_name, "vf") != NULL) {
> > +        VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be
> > enabled");
> > +        dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
> > +    } else {
> > +        dev->hw_ol_features &= ~NETDEV_RX_HW_CRC_STRIP;
> > +    }
> > +
> >      if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
> >              rx_chksm_offload_capa) {
> >          VLOG_WARN("Rx checksum offload is not supported on port "
> > --
> > 2.7.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian May 30, 2018, 2:43 p.m. UTC | #3 
> > Hi,
> >
> > I did all my testing with DPDK 17.11.2 and the following OVS commit
> > (patch doesn't apply cleanly to master):
> > cferrite@silpixa00393943:~/ovs$ git show --summary commit
> > 6b71df2fc06921758798e41833d67d9daf647d19
> > Author: Kevin Traynor <ktraynor@redhat.com>
> > Date:   Wed Apr 25 12:20:53 2018 +0100
> >
> >     faq: Document DPDK version maintenance.
> >
> > Before the patch was applied, the following was seen when a VF port is
> > added:
> > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl
> > --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> > options:dpdk-
> > devargs=0000:07:02.0
> > ovs-vsctl: Error detected while setting up 'dpdk0': could not add
> > network device dpdk0 to ofproto (No such device).  See ovs-vswitchd
> > log for details.
> > ovs-vsctl: The default log directory is
> "/usr/local/var/log/openvswitch".
> >
> > The vswitchd log after the VF port is added:
> > 2018-05-25T09:31:16Z|00080|dpif_netdev|INFO|PMD thread on numa_id: 0,
> > core
> > id:  3 created.
> > 2018-05-25T09:31:16Z|00081|dpif_netdev|INFO|There are 1 pmd threads on
> > numa node 0
> > 2018-05-25T09:31:16Z|00082|dpdk|ERR|i40evf_dev_configure(): VF can't
> > disable HW CRC Strip
> > 2018-05-25T09:31:16Z|00083|netdev_dpdk|WARN|Interface
> > dpdk0 eth_dev setup error Invalid argument 2018-05-
> > 25T09:31:16Z|00084|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2 lsc
> > interrupt mode:false) configure error: Invalid argument 2018-05-
> > 25T09:31:16Z|00085|dpif_netdev|ERR|Failed to set interface dpdk0 new
> > configuration 2018-05-25T09:31:16Z|00086|bridge|WARN|could not add
> > network device dpdk0 to ofproto (No such device)
> >
> > After the patch is applied there is no error message when the VF port
> > is
> > added:
> > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl
> > --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> > options:dpdk-
> > devargs=0000:07:02.0
> >
> > The vswitchd log after the VF port is added:
> > 2018-05-25T10:00:31Z|00081|netdev_dpdk|INFO|Virtual function detected,
> > HW_CRC_STRIP will be enabled
> > 2018-05-25T10:00:33Z|00082|dpdk|WARN|i40evf_execute_vf_cmd(): No
> > response for 28
> > 2018-05-25T10:00:33Z|00083|dpdk|ERR|i40evf_disable_vlan_strip():
> > Failed to execute command of VIRTCHNL_OP_DISABLE_VLAN_STRIPPING
> > 2018-05-25T10:00:33Z|00084|dpdk|ERR|i40evf_config_promisc(): fail to
> > execute command CONFIG_PROMISCUOUS_MODE
> > 2018-05-25T10:00:33Z|00085|dpdk|ERR|i40evf_config_promisc(): fail to
> > execute command CONFIG_PROMISCUOUS_MODE
> 
> Hmmm, strange, I've tested a VF for xl710 (i40e), i350 (IGB) and an
> 82599ES (ixgbe) devices but did not see the i40evf_config_promisc() error
> above in the logs.
> 
> We could look at disabling the promiscuous flag in the case for SRIOV.
> Typically SRIOV don't allow promiscuous mode as a security step, in the
> case where the device is in pass through mode to a VM it can be preferred
> to stop devices receiving traffic that is destined to another VF
> interface.

Hi Cian, just a follow up, what version of the i40e kernel driver were you using in your testing?

I was able to resolve the dpdk|ERR|i40evf_disable_vlan_strip() by upgrading to the latest kernel drive (2.4.6 atm) and I don't see any other DPDK errors.

I'm wondering if you have the same issue here?

Thanks
Ian
> 
> I'll have to look at this a little bit further. I will also fix the
> comment you flagged below.
> 
> Thanks
> Ian
> 
> >
> > There are errors related to promiscuous mode, but there is a message
> > stating that CRC Stripping will be enabled so the patch is working for
> me.
> >
> > Appctl query on the port:
> > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-appctl
> > dpif/show
> > netdev@ovs-netdev: hit:0 missed:0
> >         br0:
> >                 br0 65534/1: (tap)
> >                 dpdk0 1/2: (dpdk: configured_rx_queues=1,
> > configured_rxq_descriptors=2048, configured_tx_queues=2,
> > configured_txq_descriptors=2048, mtu=1500, requested_rx_queues=1,
> > requested_rxq_descriptors=2048, requested_tx_queues=2,
> > requested_txq_descriptors=2048, rx_csum_offload=true)
> >
> >
> > I have also successfully passed traffic into this VF port and out a
> > regular PF port as summarised by the below CMDs:
> > sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 --
> > set Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0 sudo
> > $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk1 -- set
> > Interface dpdk1 type=dpdk options:dpdk-devargs=0000:07:00.1 sudo
> > $OVS_DIR/utilities/ovs-ofctl del-flows br0 sudo
> > $OVS_DIR/utilities/ovs- ofctl add-flow br0
> > idle_timeout=0,in_port=1,action=output:2
> > sudo $OVS_DIR/utilities/ovs-ofctl add-flow br0
> > idle_timeout=0,in_port=2,action=output:1
> >
> > The two findings from my testing:
> > * There are error messages coming from DPDK about promiscuous mode on
> > the port. Is this something that should be disabled or otherwise
> > considered in the next revision of the patch?
> > * The patch doesn't apply cleanly to master. It looks like this is
> > caused by the following patch:
> > cferrite@silpixa00393943:~/ovs$ git show 65a87968f4 commit
> > 65a87968f4cfd9cf7a433a3156d98118078f9e4e
> > Author: Pablo Cascón <pablo.cascon@netronome.com>
> > Date:   Fri Apr 27 17:40:49 2018 +0100
> >
> >     netdev-dpdk: don't enable scatter for jumbo RX support for nfp
> >
> > I have one comment about the code below, if this is addressed/resolved
> > I'm happy to give:
> >
> > Acked-by: Cian Ferriter <cian.ferriter@intel.com>
> > Tested-by: Cian Ferriter <cian.ferriter@intel.com>
> >
> > Thanks,
> > Cian
> >
> > > -----Original Message-----
> > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > > bounces@openvswitch.org] On Behalf Of Ian Stokes
> > > Sent: 03 May 2018 14:25
> > > To: dev@openvswitch.org
> > > Subject: [ovs-dev] [PATCH v1] netdev-dpdk: Enable HW_CRC_STRIP for
> > > virtual functions.
> > >
> > > From: Michal Weglicki <michalx.weglicki@intel.com>
> > >
> > > Virtual functions such as igb_vf and i40e_vf require HW_CRC_STRIP to
> > > be explicitly enabled before configuration, otherwise device
> > > configuration will fail.
> > >
> > > This commit achieves this by adding NETDEV_RX_HW_CRC_STRIP to
> > > dpdk_hw_ol_features. When a dpdk device is added, the driver for the
> > > device is examined, if the device is a virtual function enable
> > HW_CRC_STRIP.
> > >
> > > Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> > > ---
> > >  lib/netdev-dpdk.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > > 3306b19..50c7619
> > > 100644
> > > --- a/lib/netdev-dpdk.c
> > > +++ b/lib/netdev-dpdk.c
> > > @@ -341,6 +341,7 @@ struct ingress_policer {
> > >
> > >  enum dpdk_hw_ol_features {
> > >      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
> > > +    NETDEV_RX_HW_CRC_STRIP = 1 << 1,
> > >  };
> > >
> > >  /*
> > > @@ -779,6 +780,11 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> > > *dev, int n_rxq, int n_txq)
> > >
> > >      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> > >                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> > > +
> > > +    if ((dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) {
> > > +        conf.rxmode.hw_strip_crc = 1;
> > > +    }
> > > +
> >
> > It seems that there's an extra set of parenthesis around the above if
> > statement. Is this intentional?
> 
> Sure will fix this for the v2.
> 
> Ian
> >
> > >      /* A device may report more queues than it makes available
> > > (this
> > has
> > >       * been observed for Intel xl710, which reserves some of them for
> > >       * SRIOV):  rte_eth_*_queue_setup will fail if a queue is not
> > > @@
> > > -868,6
> > > +874,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> > >
> > >      rte_eth_dev_info_get(dev->port_id, &info);
> > >
> > > +    if (strstr(info.driver_name, "vf") != NULL) {
> > > +        VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be
> > > enabled");
> > > +        dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
> > > +    } else {
> > > +        dev->hw_ol_features &= ~NETDEV_RX_HW_CRC_STRIP;
> > > +    }
> > > +
> > >      if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
> > >              rx_chksm_offload_capa) {
> > >          VLOG_WARN("Rx checksum offload is not supported on port "
> > > --
> > > 2.7.5
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ferriter, Cian May 31, 2018, 8:51 a.m. UTC | #4 
> 
> > > Hi,
> > >
> > > I did all my testing with DPDK 17.11.2 and the following OVS commit
> > > (patch doesn't apply cleanly to master):
> > > cferrite@silpixa00393943:~/ovs$ git show --summary commit
> > > 6b71df2fc06921758798e41833d67d9daf647d19
> > > Author: Kevin Traynor <ktraynor@redhat.com>
> > > Date:   Wed Apr 25 12:20:53 2018 +0100
> > >
> > >     faq: Document DPDK version maintenance.
> > >
> > > Before the patch was applied, the following was seen when a VF port
> > > is
> > > added:
> > > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl
> > > --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> > > options:dpdk-
> > > devargs=0000:07:02.0
> > > ovs-vsctl: Error detected while setting up 'dpdk0': could not add
> > > network device dpdk0 to ofproto (No such device).  See ovs-vswitchd
> > > log for details.
> > > ovs-vsctl: The default log directory is
> > "/usr/local/var/log/openvswitch".
> > >
> > > The vswitchd log after the VF port is added:
> > > 2018-05-25T09:31:16Z|00080|dpif_netdev|INFO|PMD thread on
> numa_id:
> > > 0, core
> > > id:  3 created.
> > > 2018-05-25T09:31:16Z|00081|dpif_netdev|INFO|There are 1 pmd
> threads
> > > on numa node 0
> > > 2018-05-25T09:31:16Z|00082|dpdk|ERR|i40evf_dev_configure(): VF can't
> > > disable HW CRC Strip
> > > 2018-05-25T09:31:16Z|00083|netdev_dpdk|WARN|Interface
> > > dpdk0 eth_dev setup error Invalid argument 2018-05-
> > > 25T09:31:16Z|00084|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2 lsc
> > > interrupt mode:false) configure error: Invalid argument 2018-05-
> > > 25T09:31:16Z|00085|dpif_netdev|ERR|Failed to set interface dpdk0 new
> > > configuration 2018-05-25T09:31:16Z|00086|bridge|WARN|could not add
> > > network device dpdk0 to ofproto (No such device)
> > >
> > > After the patch is applied there is no error message when the VF
> > > port is
> > > added:
> > > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-vsctl
> > > --timeout 10 add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> > > options:dpdk-
> > > devargs=0000:07:02.0
> > >
> > > The vswitchd log after the VF port is added:
> > > 2018-05-25T10:00:31Z|00081|netdev_dpdk|INFO|Virtual function
> > > detected, HW_CRC_STRIP will be enabled
> > > 2018-05-25T10:00:33Z|00082|dpdk|WARN|i40evf_execute_vf_cmd(): No
> > > response for 28
> > > 2018-05-25T10:00:33Z|00083|dpdk|ERR|i40evf_disable_vlan_strip():
> > > Failed to execute command of
> VIRTCHNL_OP_DISABLE_VLAN_STRIPPING
> > > 2018-05-25T10:00:33Z|00084|dpdk|ERR|i40evf_config_promisc(): fail to
> > > execute command CONFIG_PROMISCUOUS_MODE
> > > 2018-05-25T10:00:33Z|00085|dpdk|ERR|i40evf_config_promisc(): fail to
> > > execute command CONFIG_PROMISCUOUS_MODE
> >
> > Hmmm, strange, I've tested a VF for xl710 (i40e), i350 (IGB) and an
> > 82599ES (ixgbe) devices but did not see the i40evf_config_promisc()
> > error above in the logs.
> >
> > We could look at disabling the promiscuous flag in the case for SRIOV.
> > Typically SRIOV don't allow promiscuous mode as a security step, in
> > the case where the device is in pass through mode to a VM it can be
> > preferred to stop devices receiving traffic that is destined to
> > another VF interface.
> 
> Hi Cian, just a follow up, what version of the i40e kernel driver were you
> using in your testing?
> 
> I was able to resolve the dpdk|ERR|i40evf_disable_vlan_strip() by upgrading
> to the latest kernel drive (2.4.6 atm) and I don't see any other DPDK errors.
> 
> I'm wondering if you have the same issue here?
> 
> Thanks
> Ian

Hi Ian,

It looks like you are right here. I'm using a much older version of the i40e kernel driver:
cferrite@silpixa00393943:~$ modinfo i40e | grep version
version:        1.4.25-k
srcversion:     C3AC861FE4B55875D6B7C35
vermagic:       4.4.0-112-generic SMP mod_unload modversions

Thanks,
Cian

> >
> > I'll have to look at this a little bit further. I will also fix the
> > comment you flagged below.
> >
> > Thanks
> > Ian
> >
> > >
> > > There are errors related to promiscuous mode, but there is a message
> > > stating that CRC Stripping will be enabled so the patch is working
> > > for
> > me.
> > >
> > > Appctl query on the port:
> > > cferrite@silpixa00393943:~$ sudo $OVS_DIR/utilities/ovs-appctl
> > > dpif/show
> > > netdev@ovs-netdev: hit:0 missed:0
> > >         br0:
> > >                 br0 65534/1: (tap)
> > >                 dpdk0 1/2: (dpdk: configured_rx_queues=1,
> > > configured_rxq_descriptors=2048, configured_tx_queues=2,
> > > configured_txq_descriptors=2048, mtu=1500, requested_rx_queues=1,
> > > requested_rxq_descriptors=2048, requested_tx_queues=2,
> > > requested_txq_descriptors=2048, rx_csum_offload=true)
> > >
> > >
> > > I have also successfully passed traffic into this VF port and out a
> > > regular PF port as summarised by the below CMDs:
> > > sudo $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk0 --
> > > set Interface dpdk0 type=dpdk options:dpdk-devargs=0000:07:02.0 sudo
> > > $OVS_DIR/utilities/ovs-vsctl --timeout 10 add-port br0 dpdk1 -- set
> > > Interface dpdk1 type=dpdk options:dpdk-devargs=0000:07:00.1 sudo
> > > $OVS_DIR/utilities/ovs-ofctl del-flows br0 sudo
> > > $OVS_DIR/utilities/ovs- ofctl add-flow br0
> > > idle_timeout=0,in_port=1,action=output:2
> > > sudo $OVS_DIR/utilities/ovs-ofctl add-flow br0
> > > idle_timeout=0,in_port=2,action=output:1
> > >
> > > The two findings from my testing:
> > > * There are error messages coming from DPDK about promiscuous mode
> > > on the port. Is this something that should be disabled or otherwise
> > > considered in the next revision of the patch?
> > > * The patch doesn't apply cleanly to master. It looks like this is
> > > caused by the following patch:
> > > cferrite@silpixa00393943:~/ovs$ git show 65a87968f4 commit
> > > 65a87968f4cfd9cf7a433a3156d98118078f9e4e
> > > Author: Pablo Cascón <pablo.cascon@netronome.com>
> > > Date:   Fri Apr 27 17:40:49 2018 +0100
> > >
> > >     netdev-dpdk: don't enable scatter for jumbo RX support for nfp
> > >
> > > I have one comment about the code below, if this is
> > > addressed/resolved I'm happy to give:
> > >
> > > Acked-by: Cian Ferriter <cian.ferriter@intel.com>
> > > Tested-by: Cian Ferriter <cian.ferriter@intel.com>
> > >
> > > Thanks,
> > > Cian
> > >
> > > > -----Original Message-----
> > > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > > > bounces@openvswitch.org] On Behalf Of Ian Stokes
> > > > Sent: 03 May 2018 14:25
> > > > To: dev@openvswitch.org
> > > > Subject: [ovs-dev] [PATCH v1] netdev-dpdk: Enable HW_CRC_STRIP for
> > > > virtual functions.
> > > >
> > > > From: Michal Weglicki <michalx.weglicki@intel.com>
> > > >
> > > > Virtual functions such as igb_vf and i40e_vf require HW_CRC_STRIP
> > > > to be explicitly enabled before configuration, otherwise device
> > > > configuration will fail.
> > > >
> > > > This commit achieves this by adding NETDEV_RX_HW_CRC_STRIP to
> > > > dpdk_hw_ol_features. When a dpdk device is added, the driver for
> > > > the device is examined, if the device is a virtual function enable
> > > HW_CRC_STRIP.
> > > >
> > > > Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> > > > ---
> > > >  lib/netdev-dpdk.c | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > > > 3306b19..50c7619
> > > > 100644
> > > > --- a/lib/netdev-dpdk.c
> > > > +++ b/lib/netdev-dpdk.c
> > > > @@ -341,6 +341,7 @@ struct ingress_policer {
> > > >
> > > >  enum dpdk_hw_ol_features {
> > > >      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
> > > > +    NETDEV_RX_HW_CRC_STRIP = 1 << 1,
> > > >  };
> > > >
> > > >  /*
> > > > @@ -779,6 +780,11 @@ dpdk_eth_dev_queue_setup(struct
> netdev_dpdk
> > > > *dev, int n_rxq, int n_txq)
> > > >
> > > >      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> > > >                                    NETDEV_RX_CHECKSUM_OFFLOAD) !=
> > > > 0;
> > > > +
> > > > +    if ((dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) {
> > > > +        conf.rxmode.hw_strip_crc = 1;
> > > > +    }
> > > > +
> > >
> > > It seems that there's an extra set of parenthesis around the above
> > > if statement. Is this intentional?
> >
> > Sure will fix this for the v2.
> >
> > Ian
> > >
> > > >      /* A device may report more queues than it makes available
> > > > (this
> > > has
> > > >       * been observed for Intel xl710, which reserves some of them for
> > > >       * SRIOV):  rte_eth_*_queue_setup will fail if a queue is not
> > > > @@
> > > > -868,6
> > > > +874,13 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> > > >
> > > >      rte_eth_dev_info_get(dev->port_id, &info);
> > > >
> > > > +    if (strstr(info.driver_name, "vf") != NULL) {
> > > > +        VLOG_INFO("Virtual function detected, HW_CRC_STRIP will
> > > > + be
> > > > enabled");
> > > > +        dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
> > > > +    } else {
> > > > +        dev->hw_ol_features &= ~NETDEV_RX_HW_CRC_STRIP;
> > > > +    }
> > > > +
> > > >      if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
> > > >              rx_chksm_offload_capa) {
> > > >          VLOG_WARN("Rx checksum offload is not supported on port "
> > > > --
> > > > 2.7.5
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > dev@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 3306b19..50c7619 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -341,6 +341,7 @@  struct ingress_policer {
 
 enum dpdk_hw_ol_features {
     NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
+    NETDEV_RX_HW_CRC_STRIP = 1 << 1,
 };
 
 /*
@@ -779,6 +780,11 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
 
     conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
                                   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
+
+    if ((dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) {
+        conf.rxmode.hw_strip_crc = 1;
+    }
+
     /* A device may report more queues than it makes available (this has
      * been observed for Intel xl710, which reserves some of them for
      * SRIOV):  rte_eth_*_queue_setup will fail if a queue is not
@@ -868,6 +874,13 @@  dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
     rte_eth_dev_info_get(dev->port_id, &info);
 
+    if (strstr(info.driver_name, "vf") != NULL) {
+        VLOG_INFO("Virtual function detected, HW_CRC_STRIP will be enabled");
+        dev->hw_ol_features |= NETDEV_RX_HW_CRC_STRIP;
+    } else {
+        dev->hw_ol_features &= ~NETDEV_RX_HW_CRC_STRIP;
+    }
+
     if ((info.rx_offload_capa & rx_chksm_offload_capa) !=
             rx_chksm_offload_capa) {
         VLOG_WARN("Rx checksum offload is not supported on port "