diff mbox

[ovs-dev,v3] netdev-dpdk: use rte_eth_dev_set_mtu

Message ID 1498661499-222614-1-git-send-email-mark.b.kavanagh@intel.com
State Superseded
Headers show

Commit Message

Mark Kavanagh June 28, 2017, 2:51 p.m. UTC
DPDK provides an API to set the MTU of compatible physical devices -
rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
implemented in some DPDK PMDs (i40e, specifically). To allow the use
of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
achieved by setting the jumbo frame flag, and corresponding maximum
permitted Rx frame size, in an rte_eth_conf structure for the NIC
port, and subsequently invoking rte_eth_dev_configure() with that
configuration.

However, that method does not set the MTU field of the underlying DPDK
structure (rte_eth_dev) for the corresponding physical device;
consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
OvS-DPDK phy device with non-standard MTU.

Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
or modifying the MTU of a DPDK phy port.

Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
Reported-by: Aaron Conole <aconole@redhat.com>
Reported-by: Vipin Varghese <vipin.varghese@intel.com>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
---

v3->v2:
    - enable scatter_rx explicitly for jumbo MTU

v2->v1:
    - add 'reported-by' tag for Aaron Conole
        - change VLOG_INFO to VLOG_ERR

 lib/netdev-dpdk.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Chandran, Sugesh July 3, 2017, 3:36 p.m. UTC | #1
Hi Mark, 

Regards
_Sugesh


> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
> Sent: Wednesday, June 28, 2017 3:52 PM
> To: ovs-dev@openvswitch.org; Varghese, Vipin
> <vipin.varghese@intel.com>; aconole@redhat.com
> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
> 
> DPDK provides an API to set the MTU of compatible physical devices -
> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
> implemented in some DPDK PMDs (i40e, specifically). To allow the use of
> jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
> achieved by setting the jumbo frame flag, and corresponding maximum
> permitted Rx frame size, in an rte_eth_conf structure for the NIC port, and
> subsequently invoking rte_eth_dev_configure() with that configuration.
> 
> However, that method does not set the MTU field of the underlying DPDK
> structure (rte_eth_dev) for the corresponding physical device;
> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
> OvS-DPDK phy device with non-standard MTU.
> 
> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up or
> modifying the MTU of a DPDK phy port.
> 
> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
> Reported-by: Aaron Conole <aconole@redhat.com>
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Reviewed-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> ---
> 
> v3->v2:
>     - enable scatter_rx explicitly for jumbo MTU
> 
> v2->v1:
>     - add 'reported-by' tag for Aaron Conole
>         - change VLOG_INFO to VLOG_ERR
> 
>  lib/netdev-dpdk.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bba4de3..671d585
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev, int n_rxq, int n_txq)
>      int i;
>      struct rte_eth_conf conf = port_conf;
> 
> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
> +     * enabled. */
>      if (dev->mtu > ETHER_MTU) {
> -        conf.rxmode.jumbo_frame = 1;
> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
> -    } else {
> -        conf.rxmode.jumbo_frame = 0;
> -        conf.rxmode.max_rx_pkt_len = 0;
> +        conf.rxmode.enable_scatter = 1;
>      }
> +
>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>      /* A device may report more queues than it makes available (this has @@
> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
> int n_rxq, int n_txq)
>              break;
>          }
> 
> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
> +        if (diag) {
> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
[Sugesh] I am still getting this error when I try to test this feature on a Niantic cards.

$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <-- This looks ok
$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=5000
2017-07-03T15:26:28Z|00073|netdev_dpdk|ERR|Interface dpdk0 MTU (5000) setup error: Invalid argument
2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2) configure error: Invalid argument
2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration
2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on nonexistent port 1
2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status on nonexistent port 1
sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$ 2017-07-03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on nonexistent port 1
2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on nonexistent port 1
2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on nonexistent port 1

I assume just the scatter_flag may not sufficient to enable jumbo_frames?? Because in my testing I noticed the behavior is same that reported by Ian in the v2 patch. No traffic is getting forwarded through the ports. 


> +            break;
> +        }
> +
>          for (i = 0; i < n_txq; i++) {
>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
>                                            dev->socket_id, NULL);
> --
> 1.9.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Mark Kavanagh July 5, 2017, 9:19 a.m. UTC | #2
>From: Chandran, Sugesh
>Sent: Monday, July 3, 2017 4:36 PM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-dev@openvswitch.org;
>Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com
>Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>
>Hi Mark,
>
>Regards
>_Sugesh
>
>
>> -----Original Message-----
>> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
>> Sent: Wednesday, June 28, 2017 3:52 PM
>> To: ovs-dev@openvswitch.org; Varghese, Vipin
>> <vipin.varghese@intel.com>; aconole@redhat.com
>> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>>
>> DPDK provides an API to set the MTU of compatible physical devices -
>> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
>> implemented in some DPDK PMDs (i40e, specifically). To allow the use of
>> jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
>> achieved by setting the jumbo frame flag, and corresponding maximum
>> permitted Rx frame size, in an rte_eth_conf structure for the NIC port, and
>> subsequently invoking rte_eth_dev_configure() with that configuration.
>>
>> However, that method does not set the MTU field of the underlying DPDK
>> structure (rte_eth_dev) for the corresponding physical device;
>> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
>> OvS-DPDK phy device with non-standard MTU.
>>
>> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up or
>> modifying the MTU of a DPDK phy port.
>>
>> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
>> Reported-by: Aaron Conole <aconole@redhat.com>
>> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>> Reviewed-by: Aaron Conole <aconole@redhat.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> ---
>>
>> v3->v2:
>>     - enable scatter_rx explicitly for jumbo MTU
>>
>> v2->v1:
>>     - add 'reported-by' tag for Aaron Conole
>>         - change VLOG_INFO to VLOG_ERR
>>
>>  lib/netdev-dpdk.c | 16 +++++++++++-----
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bba4de3..671d585
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
>> *dev, int n_rxq, int n_txq)
>>      int i;
>>      struct rte_eth_conf conf = port_conf;
>>
>> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
>> +     * enabled. */
>>      if (dev->mtu > ETHER_MTU) {
>> -        conf.rxmode.jumbo_frame = 1;
>> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
>> -    } else {
>> -        conf.rxmode.jumbo_frame = 0;
>> -        conf.rxmode.max_rx_pkt_len = 0;
>> +        conf.rxmode.enable_scatter = 1;
>>      }
>> +
>>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>      /* A device may report more queues than it makes available (this has @@
>> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
>> int n_rxq, int n_txq)
>>              break;
>>          }
>>
>> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
>> +        if (diag) {
>> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
>> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
>[Sugesh] I am still getting this error when I try to test this feature on a
>Niantic cards.

Hi Sugesh,

What version of DPDK are you using?

I'm guessing that it's 16.11.0 - please try testing with v16.11.2, since this contains a bugfix for the ixgbe set_mtu function that is required for this patch.

In any event, I've been in contact with DPDK colleagues wrt the inconsistent behavior between ixgbe and i40e drivers that warrant this change; they've pushed a patch to address same: http://www.dpdk.org/dev/patchwork/patch/26329/.
I haven't had a chance to test this patch yet, but if it works, it may not be necessary to set scatter_rx mode in OvS. 

Thanks,
Mark


>
>$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <-- This
>looks ok
>$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=5000
>2017-07-03T15:26:28Z|00073|netdev_dpdk|ERR|Interface dpdk0 MTU (5000) setup
>error: Invalid argument
>2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1 txq:2)
>configure error: Invalid argument
>2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface dpdk0 new
>configuration
>2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
>nonexistent port 1
>2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status on
>nonexistent port 1
>sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$ 2017-07-
>03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on nonexistent port
>1
>2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
>nonexistent port 1
>2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
>nonexistent port 1
>
>I assume just the scatter_flag may not sufficient to enable jumbo_frames??
>Because in my testing I noticed the behavior is same that reported by Ian in
>the v2 patch. No traffic is getting forwarded through the ports.
>
>
>> +            break;
>> +        }
>> +
>>          for (i = 0; i < n_txq; i++) {
>>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
>>                                            dev->socket_id, NULL);
>> --
>> 1.9.3
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Chandran, Sugesh July 7, 2017, 9:43 a.m. UTC | #3
Hi Mark, 
Thank you for the information.
Please find my comments below.

Regards
_Sugesh


> -----Original Message-----
> From: Kavanagh, Mark B
> Sent: Wednesday, July 5, 2017 10:20 AM
> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-
> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
> aconole@redhat.com
> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
> 
> 
> 
> >From: Chandran, Sugesh
> >Sent: Monday, July 3, 2017 4:36 PM
> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;
> >ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
> >aconole@redhat.com
> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
> >
> >Hi Mark,
> >
> >Regards
> >_Sugesh
> >
> >
> >> -----Original Message-----
> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
> >> Sent: Wednesday, June 28, 2017 3:52 PM
> >> To: ovs-dev@openvswitch.org; Varghese, Vipin
> >> <vipin.varghese@intel.com>; aconole@redhat.com
> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
> >>
> >> DPDK provides an API to set the MTU of compatible physical devices -
> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
> >> implemented in some DPDK PMDs (i40e, specifically). To allow the use
> >> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
> >> achieved by setting the jumbo frame flag, and corresponding maximum
> >> permitted Rx frame size, in an rte_eth_conf structure for the NIC
> >> port, and subsequently invoking rte_eth_dev_configure() with that
> configuration.
> >>
> >> However, that method does not set the MTU field of the underlying
> >> DPDK structure (rte_eth_dev) for the corresponding physical device;
> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
> >> OvS-DPDK phy device with non-standard MTU.
> >>
> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
> >> or modifying the MTU of a DPDK phy port.
> >>
> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
> >> Reported-by: Aaron Conole <aconole@redhat.com>
> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> >> Reviewed-by: Aaron Conole <aconole@redhat.com>
> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> >> ---
> >>
> >> v3->v2:
> >>     - enable scatter_rx explicitly for jumbo MTU
> >>
> >> v2->v1:
> >>     - add 'reported-by' tag for Aaron Conole
> >>         - change VLOG_INFO to VLOG_ERR
> >>
> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
> >>  1 file changed, 11 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> bba4de3..671d585
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
> netdev_dpdk
> >> *dev, int n_rxq, int n_txq)
> >>      int i;
> >>      struct rte_eth_conf conf = port_conf;
> >>
> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
> >> +     * enabled. */
> >>      if (dev->mtu > ETHER_MTU) {
> >> -        conf.rxmode.jumbo_frame = 1;
> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
> >> -    } else {
> >> -        conf.rxmode.jumbo_frame = 0;
> >> -        conf.rxmode.max_rx_pkt_len = 0;
> >> +        conf.rxmode.enable_scatter = 1;
> >>      }
> >> +
> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >>      /* A device may report more queues than it makes available (this
> >> has @@
> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev,
> >> int n_rxq, int n_txq)
> >>              break;
> >>          }
> >>
> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
> >> +        if (diag) {
> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
> >[Sugesh] I am still getting this error when I try to test this feature
> >on a Niantic cards.
> 
> Hi Sugesh,
> 
> What version of DPDK are you using?
> 
> I'm guessing that it's 16.11.0 - please try testing with v16.11.2, since this
> contains a bugfix for the ixgbe set_mtu function that is required for this
> patch.
> 
[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you can add
'Tested by'
And the changes are looks ok for me too.

> In any event, I've been in contact with DPDK colleagues wrt the inconsistent
> behavior between ixgbe and i40e drivers that warrant this change; they've
> pushed a patch to address same:
> http://www.dpdk.org/dev/patchwork/patch/26329/.
> I haven't had a chance to test this patch yet, but if it works, it may not be
> necessary to set scatter_rx mode in OvS.
> 
> Thanks,
> Mark
> 
> 
> >
> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
> >mtu_request=5000 2017-07-
> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
> >dpdk0 MTU (5000) setup
> >error: Invalid argument
> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
> txq:2)
> >configure error: Invalid argument
> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
> >dpdk0 new configuration
> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
> >nonexistent port 1
> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
> on
> >nonexistent port 1
> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
> >2017-07-
> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
> >nonexistent port
> >1
> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
> >nonexistent port 1
> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
> >nonexistent port 1
> >
> >I assume just the scatter_flag may not sufficient to enable jumbo_frames??
> >Because in my testing I noticed the behavior is same that reported by
> >Ian in the v2 patch. No traffic is getting forwarded through the ports.
> >
> >
> >> +            break;
> >> +        }
> >> +
> >>          for (i = 0; i < n_txq; i++) {
> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
> >>                                            dev->socket_id, NULL);
> >> --
> >> 1.9.3
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Mark Kavanagh July 7, 2017, 10:18 a.m. UTC | #4
>From: Chandran, Sugesh
>Sent: Friday, July 7, 2017 10:44 AM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-dev@openvswitch.org;
>Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com
>Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>
>Hi Mark,
>Thank you for the information.
>Please find my comments below.
>
>Regards
>_Sugesh
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Wednesday, July 5, 2017 10:20 AM
>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-
>> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
>> aconole@redhat.com
>> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>>
>>
>>
>> >From: Chandran, Sugesh
>> >Sent: Monday, July 3, 2017 4:36 PM
>> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;
>> >ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
>> >aconole@redhat.com
>> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>> >
>> >Hi Mark,
>> >
>> >Regards
>> >_Sugesh
>> >
>> >
>> >> -----Original Message-----
>> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
>> >> Sent: Wednesday, June 28, 2017 3:52 PM
>> >> To: ovs-dev@openvswitch.org; Varghese, Vipin
>> >> <vipin.varghese@intel.com>; aconole@redhat.com
>> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>> >>
>> >> DPDK provides an API to set the MTU of compatible physical devices -
>> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
>> >> implemented in some DPDK PMDs (i40e, specifically). To allow the use
>> >> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
>> >> achieved by setting the jumbo frame flag, and corresponding maximum
>> >> permitted Rx frame size, in an rte_eth_conf structure for the NIC
>> >> port, and subsequently invoking rte_eth_dev_configure() with that
>> configuration.
>> >>
>> >> However, that method does not set the MTU field of the underlying
>> >> DPDK structure (rte_eth_dev) for the corresponding physical device;
>> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
>> >> OvS-DPDK phy device with non-standard MTU.
>> >>
>> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
>> >> or modifying the MTU of a DPDK phy port.
>> >>
>> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
>> >> Reported-by: Aaron Conole <aconole@redhat.com>
>> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>> >> Reviewed-by: Aaron Conole <aconole@redhat.com>
>> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> >> ---
>> >>
>> >> v3->v2:
>> >>     - enable scatter_rx explicitly for jumbo MTU
>> >>
>> >> v2->v1:
>> >>     - add 'reported-by' tag for Aaron Conole
>> >>         - change VLOG_INFO to VLOG_ERR
>> >>
>> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
>> >>  1 file changed, 11 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>> >> bba4de3..671d585
>> >> 100644
>> >> --- a/lib/netdev-dpdk.c
>> >> +++ b/lib/netdev-dpdk.c
>> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
>> netdev_dpdk
>> >> *dev, int n_rxq, int n_txq)
>> >>      int i;
>> >>      struct rte_eth_conf conf = port_conf;
>> >>
>> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
>explicitly
>> >> +     * enabled. */
>> >>      if (dev->mtu > ETHER_MTU) {
>> >> -        conf.rxmode.jumbo_frame = 1;
>> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
>> >> -    } else {
>> >> -        conf.rxmode.jumbo_frame = 0;
>> >> -        conf.rxmode.max_rx_pkt_len = 0;
>> >> +        conf.rxmode.enable_scatter = 1;
>> >>      }
>> >> +
>> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>> >>      /* A device may report more queues than it makes available (this
>> >> has @@
>> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
>> *dev,
>> >> int n_rxq, int n_txq)
>> >>              break;
>> >>          }
>> >>
>> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
>> >> +        if (diag) {
>> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
>> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
>> >[Sugesh] I am still getting this error when I try to test this feature
>> >on a Niantic cards.
>>
>> Hi Sugesh,
>>
>> What version of DPDK are you using?
>>
>> I'm guessing that it's 16.11.0 - please try testing with v16.11.2, since
>this
>> contains a bugfix for the ixgbe set_mtu function that is required for this
>> patch.
>>
>[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you can
>add
>'Tested by'
>And the changes are looks ok for me too.

Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in that case?

Cheers,
Mark

>
>> In any event, I've been in contact with DPDK colleagues wrt the inconsistent
>> behavior between ixgbe and i40e drivers that warrant this change; they've
>> pushed a patch to address same:
>> http://www.dpdk.org/dev/patchwork/patch/26329/.
>> I haven't had a chance to test this patch yet, but if it works, it may not
>be
>> necessary to set scatter_rx mode in OvS.
>>
>> Thanks,
>> Mark
>>
>>
>> >
>> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
>> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
>> >mtu_request=5000 2017-07-
>> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
>> >dpdk0 MTU (5000) setup
>> >error: Invalid argument
>> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
>> txq:2)
>> >configure error: Invalid argument
>> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
>> >dpdk0 new configuration
>> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
>> >nonexistent port 1
>> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
>> on
>> >nonexistent port 1
>> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
>> >2017-07-
>> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
>> >nonexistent port
>> >1
>> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
>> >nonexistent port 1
>> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
>> >nonexistent port 1
>> >
>> >I assume just the scatter_flag may not sufficient to enable jumbo_frames??
>> >Because in my testing I noticed the behavior is same that reported by
>> >Ian in the v2 patch. No traffic is getting forwarded through the ports.
>> >
>> >
>> >> +            break;
>> >> +        }
>> >> +
>> >>          for (i = 0; i < n_txq; i++) {
>> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev-
>>txq_size,
>> >>                                            dev->socket_id, NULL);
>> >> --
>> >> 1.9.3
>> >>
>> >> _______________________________________________
>> >> dev mailing list
>> >> dev@openvswitch.org
>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Darrell Ball July 8, 2017, 9:33 p.m. UTC | #5
So, this required a 16.11.2 bug fix.

Are we sure this will work for all/most NICs/drivers ?
In previous alluding to error cases, did calling rte_eth_dev_set_mtu() block jumbo packets
that would otherwise be allowed ?  



On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces@openvswitch.org on behalf of mark.b.kavanagh@intel.com> wrote:

    >From: Chandran, Sugesh
    >Sent: Friday, July 7, 2017 10:44 AM
    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-dev@openvswitch.org;
    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com
    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >
    >Hi Mark,
    >Thank you for the information.
    >Please find my comments below.
    >
    >Regards
    >_Sugesh
    >
    >
    >> -----Original Message-----
    >> From: Kavanagh, Mark B
    >> Sent: Wednesday, July 5, 2017 10:20 AM
    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-
    >> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
    >> aconole@redhat.com
    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >>
    >>
    >>
    >> >From: Chandran, Sugesh
    >> >Sent: Monday, July 3, 2017 4:36 PM
    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;
    >> >ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
    >> >aconole@redhat.com
    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >> >
    >> >Hi Mark,
    >> >
    >> >Regards
    >> >_Sugesh
    >> >
    >> >
    >> >> -----Original Message-----
    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
    >> >> Sent: Wednesday, June 28, 2017 3:52 PM
    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin
    >> >> <vipin.varghese@intel.com>; aconole@redhat.com
    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >> >>
    >> >> DPDK provides an API to set the MTU of compatible physical devices -
    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
    >> >> implemented in some DPDK PMDs (i40e, specifically). To allow the use
    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
    >> >> achieved by setting the jumbo frame flag, and corresponding maximum
    >> >> permitted Rx frame size, in an rte_eth_conf structure for the NIC
    >> >> port, and subsequently invoking rte_eth_dev_configure() with that
    >> configuration.
    >> >>
    >> >> However, that method does not set the MTU field of the underlying
    >> >> DPDK structure (rte_eth_dev) for the corresponding physical device;
    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
    >> >> OvS-DPDK phy device with non-standard MTU.
    >> >>
    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
    >> >> or modifying the MTU of a DPDK phy port.
    >> >>
    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
    >> >> Reported-by: Aaron Conole <aconole@redhat.com>
    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>
    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
    >> >> ---
    >> >>
    >> >> v3->v2:
    >> >>     - enable scatter_rx explicitly for jumbo MTU
    >> >>
    >> >> v2->v1:
    >> >>     - add 'reported-by' tag for Aaron Conole
    >> >>         - change VLOG_INFO to VLOG_ERR
    >> >>
    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)
    >> >>
    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
    >> >> bba4de3..671d585
    >> >> 100644
    >> >> --- a/lib/netdev-dpdk.c
    >> >> +++ b/lib/netdev-dpdk.c
    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
    >> netdev_dpdk
    >> >> *dev, int n_rxq, int n_txq)
    >> >>      int i;
    >> >>      struct rte_eth_conf conf = port_conf;
    >> >>
    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
    >explicitly
    >> >> +     * enabled. */
    >> >>      if (dev->mtu > ETHER_MTU) {
    >> >> -        conf.rxmode.jumbo_frame = 1;
    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
    >> >> -    } else {
    >> >> -        conf.rxmode.jumbo_frame = 0;
    >> >> -        conf.rxmode.max_rx_pkt_len = 0;
    >> >> +        conf.rxmode.enable_scatter = 1;
    >> >>      }
    >> >> +
    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
    >> >>      /* A device may report more queues than it makes available (this
    >> >> has @@
    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
    >> *dev,
    >> >> int n_rxq, int n_txq)
    >> >>              break;
    >> >>          }
    >> >>
    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
    >> >> +        if (diag) {
    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
    >> >[Sugesh] I am still getting this error when I try to test this feature
    >> >on a Niantic cards.
    >>
    >> Hi Sugesh,
    >>
    >> What version of DPDK are you using?
    >>
    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2, since
    >this
    >> contains a bugfix for the ixgbe set_mtu function that is required for this
    >> patch.
    >>
    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you can
    >add
    >'Tested by'
    >And the changes are looks ok for me too.
    
    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in that case?
    
    Cheers,
    Mark
    
    >
    >> In any event, I've been in contact with DPDK colleagues wrt the inconsistent
    >> behavior between ixgbe and i40e drivers that warrant this change; they've
    >> pushed a patch to address same:
    >> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .
    >> I haven't had a chance to test this patch yet, but if it works, it may not
    >be
    >> necessary to set scatter_rx mode in OvS.
    >>
    >> Thanks,
    >> Mark
    >>
    >>
    >> >
    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
    >> >mtu_request=5000 2017-07-
    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
    >> >dpdk0 MTU (5000) setup
    >> >error: Invalid argument
    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
    >> txq:2)
    >> >configure error: Invalid argument
    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
    >> >dpdk0 new configuration
    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
    >> >nonexistent port 1
    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
    >> on
    >> >nonexistent port 1
    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
    >> >2017-07-
    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port
    >> >1
    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port 1
    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
    >> >nonexistent port 1
    >> >
    >> >I assume just the scatter_flag may not sufficient to enable jumbo_frames??
    >> >Because in my testing I noticed the behavior is same that reported by
    >> >Ian in the v2 patch. No traffic is getting forwarded through the ports.
    >> >
    >> >
    >> >> +            break;
    >> >> +        }
    >> >> +
    >> >>          for (i = 0; i < n_txq; i++) {
    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev-
    >>txq_size,
    >> >>                                            dev->socket_id, NULL);
    >> >> --
    >> >> 1.9.3
    >> >>
    >> >> _______________________________________________
    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-8bybqH21vcc86SOLmLk&e=
Mark Kavanagh July 10, 2017, 9:44 a.m. UTC | #6
>From: Darrell Ball [mailto:dball@vmware.com]
>Sent: Saturday, July 8, 2017 10:34 PM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh
><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin
><vipin.varghese@intel.com>; aconole@redhat.com
>Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>
>So, this required a 16.11.2 bug fix.
>
>Are we sure this will work for all/most NICs/drivers ?

I can't say tbh Darrell, as I can only test with the NICs that are available to me (namely, Niantic and Fortville).

However, AFAIA, this issue is unique to Niantic, since that H/W permits dynamic modification of MTU without stopping its PMD; for most NICs (including Fortville), a port's PMD must be stopped before modifying its MTU, and restarted thereafter.

>In previous alluding to error cases, did calling rte_eth_dev_set_mtu() block
>jumbo packets
>that would otherwise be allowed ?

For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would return EINVAL, which means that the MTU for the NIC's ports could never be changed beyond the standard MTU.

With this patch, that behavior is avoided, and the MTU can be set appropriately.


>
>
>
>On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh,
>Mark B" <ovs-dev-bounces@openvswitch.org on behalf of
>mark.b.kavanagh@intel.com> wrote:
>
>    >From: Chandran, Sugesh
>    >Sent: Friday, July 7, 2017 10:44 AM
>    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-
>dev@openvswitch.org;
>    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com
>    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>    >
>    >Hi Mark,
>    >Thank you for the information.
>    >Please find my comments below.
>    >
>    >Regards
>    >_Sugesh
>    >
>    >
>    >> -----Original Message-----
>    >> From: Kavanagh, Mark B
>    >> Sent: Wednesday, July 5, 2017 10:20 AM
>    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-
>    >> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
>    >> aconole@redhat.com
>    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>    >>
>    >>
>    >>
>    >> >From: Chandran, Sugesh
>    >> >Sent: Monday, July 3, 2017 4:36 PM
>    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;
>    >> >ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;
>    >> >aconole@redhat.com
>    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>    >> >
>    >> >Hi Mark,
>    >> >
>    >> >Regards
>    >> >_Sugesh
>    >> >
>    >> >
>    >> >> -----Original Message-----
>    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh
>    >> >> Sent: Wednesday, June 28, 2017 3:52 PM
>    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin
>    >> >> <vipin.varghese@intel.com>; aconole@redhat.com
>    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
>    >> >>
>    >> >> DPDK provides an API to set the MTU of compatible physical devices -
>    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was
>not
>    >> >> implemented in some DPDK PMDs (i40e, specifically). To allow the use
>    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration
>was
>    >> >> achieved by setting the jumbo frame flag, and corresponding maximum
>    >> >> permitted Rx frame size, in an rte_eth_conf structure for the NIC
>    >> >> port, and subsequently invoking rte_eth_dev_configure() with that
>    >> configuration.
>    >> >>
>    >> >> However, that method does not set the MTU field of the underlying
>    >> >> DPDK structure (rte_eth_dev) for the corresponding physical device;
>    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
>    >> >> OvS-DPDK phy device with non-standard MTU.
>    >> >>
>    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
>    >> >> or modifying the MTU of a DPDK phy port.
>    >> >>
>    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
>    >> >> Reported-by: Aaron Conole <aconole@redhat.com>
>    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>
>    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>    >> >> ---
>    >> >>
>    >> >> v3->v2:
>    >> >>     - enable scatter_rx explicitly for jumbo MTU
>    >> >>
>    >> >> v2->v1:
>    >> >>     - add 'reported-by' tag for Aaron Conole
>    >> >>         - change VLOG_INFO to VLOG_ERR
>    >> >>
>    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
>    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)
>    >> >>
>    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>    >> >> bba4de3..671d585
>    >> >> 100644
>    >> >> --- a/lib/netdev-dpdk.c
>    >> >> +++ b/lib/netdev-dpdk.c
>    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
>    >> netdev_dpdk
>    >> >> *dev, int n_rxq, int n_txq)
>    >> >>      int i;
>    >> >>      struct rte_eth_conf conf = port_conf;
>    >> >>
>    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
>    >explicitly
>    >> >> +     * enabled. */
>    >> >>      if (dev->mtu > ETHER_MTU) {
>    >> >> -        conf.rxmode.jumbo_frame = 1;
>    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
>    >> >> -    } else {
>    >> >> -        conf.rxmode.jumbo_frame = 0;
>    >> >> -        conf.rxmode.max_rx_pkt_len = 0;
>    >> >> +        conf.rxmode.enable_scatter = 1;
>    >> >>      }
>    >> >> +
>    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>    >> >>      /* A device may report more queues than it makes available
>(this
>    >> >> has @@
>    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
>    >> *dev,
>    >> >> int n_rxq, int n_txq)
>    >> >>              break;
>    >> >>          }
>    >> >>
>    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
>    >> >> +        if (diag) {
>    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
>    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
>    >> >[Sugesh] I am still getting this error when I try to test this feature
>    >> >on a Niantic cards.
>    >>
>    >> Hi Sugesh,
>    >>
>    >> What version of DPDK are you using?
>    >>
>    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2,
>since
>    >this
>    >> contains a bugfix for the ixgbe set_mtu function that is required for
>this
>    >> patch.
>    >>
>    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you
>can
>    >add
>    >'Tested by'
>    >And the changes are looks ok for me too.
>
>    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in
>that case?
>
>    Cheers,
>    Mark
>
>    >
>    >> In any event, I've been in contact with DPDK colleagues wrt the
>inconsistent
>    >> behavior between ixgbe and i40e drivers that warrant this change;
>they've
>    >> pushed a patch to address same:
>    >> https://urldefense.proofpoint.com/v2/url?u=http-
>3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&
>r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-
>540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .
>    >> I haven't had a chance to test this patch yet, but if it works, it may
>not
>    >be
>    >> necessary to set scatter_rx mode in OvS.
>    >>
>    >> Thanks,
>    >> Mark
>    >>
>    >>
>    >> >
>    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
>    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
>    >> >mtu_request=5000 2017-07-
>    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
>    >> >dpdk0 MTU (5000) setup
>    >> >error: Invalid argument
>    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
>    >> txq:2)
>    >> >configure error: Invalid argument
>    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
>    >> >dpdk0 new configuration
>    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
>    >> >nonexistent port 1
>    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
>    >> on
>    >> >nonexistent port 1
>    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
>    >> >2017-07-
>    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
>    >> >nonexistent port
>    >> >1
>    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
>    >> >nonexistent port 1
>    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
>    >> >nonexistent port 1
>    >> >
>    >> >I assume just the scatter_flag may not sufficient to enable
>jumbo_frames??
>    >> >Because in my testing I noticed the behavior is same that reported by
>    >> >Ian in the v2 patch. No traffic is getting forwarded through the
>ports.
>    >> >
>    >> >
>    >> >> +            break;
>    >> >> +        }
>    >> >> +
>    >> >>          for (i = 0; i < n_txq; i++) {
>    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev-
>    >>txq_size,
>    >> >>                                            dev->socket_id, NULL);
>    >> >> --
>    >> >> 1.9.3
>    >> >>
>    >> >> _______________________________________________
>    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-
>8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-
>8bybqH21vcc86SOLmLk&e=
>
Darrell Ball July 11, 2017, 1:03 a.m. UTC | #7
On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

    >From: Darrell Ball [mailto:dball@vmware.com]

    >Sent: Saturday, July 8, 2017 10:34 PM

    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

    ><vipin.varghese@intel.com>; aconole@redhat.com

    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >

    >So, this required a 16.11.2 bug fix.

    >

    >Are we sure this will work for all/most NICs/drivers ?

    
    I can't say tbh Darrell, as I can only test with the NICs that are available to me (namely, Niantic and Fortville).
    
    However, AFAIA, this issue is unique to Niantic, since that H/W permits dynamic modification of MTU without stopping its PMD; for most NICs (including Fortville), a port's PMD must be stopped before modifying its MTU, and restarted thereafter.

I understand

    
    >In previous alluding to error cases, did calling rte_eth_dev_set_mtu() block

    >jumbo packets

    >that would otherwise be allowed ?

    
    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would return EINVAL, which means that the MTU for the NIC's ports could never be changed beyond the standard MTU.
    
    With this patch, that behavior is avoided, and the MTU can be set appropriately.


The question that I was getting at is since we are now removing the 
jumbo_frame flag workaround with this patch, can there be a regression where we cannot 
forward jumbo frames because rte_eth_dev_set_mtu() fails due to still another bug in some
cases ?
If the answer is yes, does it make sense to keep the jumbo_frame flag workaround as well,
‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle unknown corner cases ?

    
    >

    >

    >

    >On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh,

    >Mark B" <ovs-dev-bounces@openvswitch.org on behalf of

    >mark.b.kavanagh@intel.com> wrote:

    >

    >    >From: Chandran, Sugesh

    >    >Sent: Friday, July 7, 2017 10:44 AM

    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-

    >dev@openvswitch.org;

    >    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com

    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >

    >    >Hi Mark,

    >    >Thank you for the information.

    >    >Please find my comments below.

    >    >

    >    >Regards

    >    >_Sugesh

    >    >

    >    >

    >    >> -----Original Message-----

    >    >> From: Kavanagh, Mark B

    >    >> Sent: Wednesday, July 5, 2017 10:20 AM

    >    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-

    >    >> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;

    >    >> aconole@redhat.com

    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >>

    >    >>

    >    >>

    >    >> >From: Chandran, Sugesh

    >    >> >Sent: Monday, July 3, 2017 4:36 PM

    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;

    >    >> >ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;

    >    >> >aconole@redhat.com

    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >> >

    >    >> >Hi Mark,

    >    >> >

    >    >> >Regards

    >    >> >_Sugesh

    >    >> >

    >    >> >

    >    >> >> -----Original Message-----

    >    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

    >    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh

    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM

    >    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin

    >    >> >> <vipin.varghese@intel.com>; aconole@redhat.com

    >    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >> >>

    >    >> >> DPDK provides an API to set the MTU of compatible physical devices -

    >    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was

    >not

    >    >> >> implemented in some DPDK PMDs (i40e, specifically). To allow the use

    >    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration

    >was

    >    >> >> achieved by setting the jumbo frame flag, and corresponding maximum

    >    >> >> permitted Rx frame size, in an rte_eth_conf structure for the NIC

    >    >> >> port, and subsequently invoking rte_eth_dev_configure() with that

    >    >> configuration.

    >    >> >>

    >    >> >> However, that method does not set the MTU field of the underlying

    >    >> >> DPDK structure (rte_eth_dev) for the corresponding physical device;

    >    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an

    >    >> >> OvS-DPDK phy device with non-standard MTU.

    >    >> >>

    >    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up

    >    >> >> or modifying the MTU of a DPDK phy port.

    >    >> >>

    >    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")

    >    >> >> Reported-by: Aaron Conole <aconole@redhat.com>

    >    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>

    >    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>

    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>

    >    >> >> ---

    >    >> >>

    >    >> >> v3->v2:

    >    >> >>     - enable scatter_rx explicitly for jumbo MTU

    >    >> >>

    >    >> >> v2->v1:

    >    >> >>     - add 'reported-by' tag for Aaron Conole

    >    >> >>         - change VLOG_INFO to VLOG_ERR

    >    >> >>

    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----

    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)

    >    >> >>

    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index

    >    >> >> bba4de3..671d585

    >    >> >> 100644

    >    >> >> --- a/lib/netdev-dpdk.c

    >    >> >> +++ b/lib/netdev-dpdk.c

    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct

    >    >> netdev_dpdk

    >    >> >> *dev, int n_rxq, int n_txq)

    >    >> >>      int i;

    >    >> >>      struct rte_eth_conf conf = port_conf;

    >    >> >>

    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be

    >    >explicitly

    >    >> >> +     * enabled. */

    >    >> >>      if (dev->mtu > ETHER_MTU) {

    >    >> >> -        conf.rxmode.jumbo_frame = 1;

    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;

    >    >> >> -    } else {

    >    >> >> -        conf.rxmode.jumbo_frame = 0;

    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;

    >    >> >> +        conf.rxmode.enable_scatter = 1;

    >    >> >>      }

    >    >> >> +

    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &

    >    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;

    >    >> >>      /* A device may report more queues than it makes available

    >(this

    >    >> >> has @@

    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk

    >    >> *dev,

    >    >> >> int n_rxq, int n_txq)

    >    >> >>              break;

    >    >> >>          }

    >    >> >>

    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);

    >    >> >> +        if (diag) {

    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",

    >    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));

    >    >> >[Sugesh] I am still getting this error when I try to test this feature

    >    >> >on a Niantic cards.

    >    >>

    >    >> Hi Sugesh,

    >    >>

    >    >> What version of DPDK are you using?

    >    >>

    >    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2,

    >since

    >    >this

    >    >> contains a bugfix for the ixgbe set_mtu function that is required for

    >this

    >    >> patch.

    >    >>

    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you

    >can

    >    >add

    >    >'Tested by'

    >    >And the changes are looks ok for me too.

    >

    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in

    >that case?

    >

    >    Cheers,

    >    Mark

    >

    >    >

    >    >> In any event, I've been in contact with DPDK colleagues wrt the

    >inconsistent

    >    >> behavior between ixgbe and i40e drivers that warrant this change;

    >they've

    >    >> pushed a patch to address same:

    >    >> https://urldefense.proofpoint.com/v2/url?u=http-

    >3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&

    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .

    >    >> I haven't had a chance to test this patch yet, but if it works, it may

    >not

    >    >be

    >    >> necessary to set scatter_rx mode in OvS.

    >    >>

    >    >> Thanks,

    >    >> Mark

    >    >>

    >    >>

    >    >> >

    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--

    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0

    >    >> >mtu_request=5000 2017-07-

    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface

    >    >> >dpdk0 MTU (5000) setup

    >    >> >error: Invalid argument

    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1

    >    >> txq:2)

    >    >> >configure error: Invalid argument

    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface

    >    >> >dpdk0 new configuration

    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on

    >    >> >nonexistent port 1

    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status

    >    >> on

    >    >> >nonexistent port 1

    >    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$

    >    >> >2017-07-

    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on

    >    >> >nonexistent port

    >    >> >1

    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on

    >    >> >nonexistent port 1

    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on

    >    >> >nonexistent port 1

    >    >> >

    >    >> >I assume just the scatter_flag may not sufficient to enable

    >jumbo_frames??

    >    >> >Because in my testing I noticed the behavior is same that reported by

    >    >> >Ian in the v2 patch. No traffic is getting forwarded through the

    >ports.

    >    >> >

    >    >> >

    >    >> >> +            break;

    >    >> >> +        }

    >    >> >> +

    >    >> >>          for (i = 0; i < n_txq; i++) {

    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev-

    >    >>txq_size,

    >    >> >>                                            dev->socket_id, NULL);

    >    >> >> --

    >    >> >> 1.9.3

    >    >> >>

    >    >> >> _______________________________________________

    >    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >8bybqH21vcc86SOLmLk&e=

    >
Mark Kavanagh July 11, 2017, 3:44 p.m. UTC | #8
>From: Darrell Ball [mailto:dball@vmware.com]

>Sent: Tuesday, July 11, 2017 2:03 AM

>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

><vipin.varghese@intel.com>; aconole@redhat.com

>Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

>

>

>

>On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

>

>    >From: Darrell Ball [mailto:dball@vmware.com]

>    >Sent: Saturday, July 8, 2017 10:34 PM

>    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

>    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

>    ><vipin.varghese@intel.com>; aconole@redhat.com

>    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

>    >

>    >So, this required a 16.11.2 bug fix.

>    >

>    >Are we sure this will work for all/most NICs/drivers ?

>

>    I can't say tbh Darrell, as I can only test with the NICs that are

>available to me (namely, Niantic and Fortville).

>

>    However, AFAIA, this issue is unique to Niantic, since that H/W permits

>dynamic modification of MTU without stopping its PMD; for most NICs (including

>Fortville), a port's PMD must be stopped before modifying its MTU, and

>restarted thereafter.

>

>I understand

>

>

>    >In previous alluding to error cases, did calling rte_eth_dev_set_mtu()

>block

>    >jumbo packets

>    >that would otherwise be allowed ?

>

>    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would

>return EINVAL, which means that the MTU for the NIC's ports could never be

>changed beyond the standard MTU.

>

>    With this patch, that behavior is avoided, and the MTU can be set

>appropriately.

>

>

>The question that I was getting at is since we are now removing the

>jumbo_frame flag workaround with this patch, can there be a regression where

>we cannot

>forward jumbo frames because rte_eth_dev_set_mtu() fails due to still another

>bug in some

>cases ?


Sure, I definitely understand your concerns Darrel - unfortunately, I can't definitively answer yes. However, since OvS now supports DPDK v16.11.2, I would hope that the stability of the latter would provide some level of assurance.

>If the answer is yes, does it make sense to keep the jumbo_frame flag

>workaround as well,

>‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle unknown

>corner cases ?


If we keep the workaround, it essentially negates the need for the netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx packet length are set by rte_eth_dev_configure. The downside of this approach is that it doesn't update the MTU for the Ethernet device itself; as a result, any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the MTU that was set using the existing method.

Thanks again,
Mark

>

>

>    >

>    >

>    >

>    >On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of

>Kavanagh,

>    >Mark B" <ovs-dev-bounces@openvswitch.org on behalf of

>    >mark.b.kavanagh@intel.com> wrote:

>    >

>    >    >From: Chandran, Sugesh

>    >    >Sent: Friday, July 7, 2017 10:44 AM

>    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-

>    >dev@openvswitch.org;

>    >    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com

>    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>rte_eth_dev_set_mtu

>    >    >

>    >    >Hi Mark,

>    >    >Thank you for the information.

>    >    >Please find my comments below.

>    >    >

>    >    >Regards

>    >    >_Sugesh

>    >    >

>    >    >

>    >    >> -----Original Message-----

>    >    >> From: Kavanagh, Mark B

>    >    >> Sent: Wednesday, July 5, 2017 10:20 AM

>    >    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-

>    >    >> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;

>    >    >> aconole@redhat.com

>    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>rte_eth_dev_set_mtu

>    >    >>

>    >    >>

>    >    >>

>    >    >> >From: Chandran, Sugesh

>    >    >> >Sent: Monday, July 3, 2017 4:36 PM

>    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;

>    >    >> >ovs-dev@openvswitch.org; Varghese, Vipin

><vipin.varghese@intel.com>;

>    >    >> >aconole@redhat.com

>    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>rte_eth_dev_set_mtu

>    >    >> >

>    >    >> >Hi Mark,

>    >    >> >

>    >    >> >Regards

>    >    >> >_Sugesh

>    >    >> >

>    >    >> >

>    >    >> >> -----Original Message-----

>    >    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

>    >    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh

>    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM

>    >    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin

>    >    >> >> <vipin.varghese@intel.com>; aconole@redhat.com

>    >    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use

>rte_eth_dev_set_mtu

>    >    >> >>

>    >    >> >> DPDK provides an API to set the MTU of compatible physical

>devices -

>    >    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API

>was

>    >not

>    >    >> >> implemented in some DPDK PMDs (i40e, specifically). To allow

>the use

>    >    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU

>configuration

>    >was

>    >    >> >> achieved by setting the jumbo frame flag, and corresponding

>maximum

>    >    >> >> permitted Rx frame size, in an rte_eth_conf structure for the

>NIC

>    >    >> >> port, and subsequently invoking rte_eth_dev_configure() with

>that

>    >    >> configuration.

>    >    >> >>

>    >    >> >> However, that method does not set the MTU field of the

>underlying

>    >    >> >> DPDK structure (rte_eth_dev) for the corresponding physical

>device;

>    >    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU

>for an

>    >    >> >> OvS-DPDK phy device with non-standard MTU.

>    >    >> >>

>    >    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when

>setting up

>    >    >> >> or modifying the MTU of a DPDK phy port.

>    >    >> >>

>    >    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")

>    >    >> >> Reported-by: Aaron Conole <aconole@redhat.com>

>    >    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>

>    >    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>

>    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>

>    >    >> >> ---

>    >    >> >>

>    >    >> >> v3->v2:

>    >    >> >>     - enable scatter_rx explicitly for jumbo MTU

>    >    >> >>

>    >    >> >> v2->v1:

>    >    >> >>     - add 'reported-by' tag for Aaron Conole

>    >    >> >>         - change VLOG_INFO to VLOG_ERR

>    >    >> >>

>    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----

>    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)

>    >    >> >>

>    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index

>    >    >> >> bba4de3..671d585

>    >    >> >> 100644

>    >    >> >> --- a/lib/netdev-dpdk.c

>    >    >> >> +++ b/lib/netdev-dpdk.c

>    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct

>    >    >> netdev_dpdk

>    >    >> >> *dev, int n_rxq, int n_txq)

>    >    >> >>      int i;

>    >    >> >>      struct rte_eth_conf conf = port_conf;

>    >    >> >>

>    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to

>be

>    >    >explicitly

>    >    >> >> +     * enabled. */

>    >    >> >>      if (dev->mtu > ETHER_MTU) {

>    >    >> >> -        conf.rxmode.jumbo_frame = 1;

>    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;

>    >    >> >> -    } else {

>    >    >> >> -        conf.rxmode.jumbo_frame = 0;

>    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;

>    >    >> >> +        conf.rxmode.enable_scatter = 1;

>    >    >> >>      }

>    >    >> >> +

>    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &

>    >    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD)

>!= 0;

>    >    >> >>      /* A device may report more queues than it makes available

>    >(this

>    >    >> >> has @@

>    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk

>    >    >> *dev,

>    >    >> >> int n_rxq, int n_txq)

>    >    >> >>              break;

>    >    >> >>          }

>    >    >> >>

>    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);

>    >    >> >> +        if (diag) {

>    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",

>    >    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-

>diag));

>    >    >> >[Sugesh] I am still getting this error when I try to test this

>feature

>    >    >> >on a Niantic cards.

>    >    >>

>    >    >> Hi Sugesh,

>    >    >>

>    >    >> What version of DPDK are you using?

>    >    >>

>    >    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2,

>    >since

>    >    >this

>    >    >> contains a bugfix for the ixgbe set_mtu function that is required

>for

>    >this

>    >    >> patch.

>    >    >>

>    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and

>you

>    >can

>    >    >add

>    >    >'Tested by'

>    >    >And the changes are looks ok for me too.

>    >

>    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you

>in

>    >that case?

>    >

>    >    Cheers,

>    >    Mark

>    >

>    >    >

>    >    >> In any event, I've been in contact with DPDK colleagues wrt the

>    >inconsistent

>    >    >> behavior between ixgbe and i40e drivers that warrant this change;

>    >they've

>    >    >> pushed a patch to address same:

>    >    >> https://urldefense.proofpoint.com/v2/url?u=http-

>

>>3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ

>&

>    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .

>    >    >> I haven't had a chance to test this patch yet, but if it works, it

>may

>    >not

>    >    >be

>    >    >> necessary to set scatter_rx mode in OvS.

>    >    >>

>    >    >> Thanks,

>    >    >> Mark

>    >    >>

>    >    >>

>    >    >> >

>    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500

><--

>    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0

>    >    >> >mtu_request=5000 2017-07-

>    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface

>    >    >> >dpdk0 MTU (5000) setup

>    >    >> >error: Invalid argument

>    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1

>    >    >> txq:2)

>    >    >> >configure error: Invalid argument

>    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set

>interface

>    >    >> >dpdk0 new configuration

>    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP

>status on

>    >    >> >nonexistent port 1

>    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP

>status

>    >    >> on

>    >    >> >nonexistent port 1

>    >    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$

>    >    >> >2017-07-

>    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on

>    >    >> >nonexistent port

>    >    >> >1

>    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats

>on

>    >    >> >nonexistent port 1

>    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats

>on

>    >    >> >nonexistent port 1

>    >    >> >

>    >    >> >I assume just the scatter_flag may not sufficient to enable

>    >jumbo_frames??

>    >    >> >Because in my testing I noticed the behavior is same that

>reported by

>    >    >> >Ian in the v2 patch. No traffic is getting forwarded through the

>    >ports.

>    >    >> >

>    >    >> >

>    >    >> >> +            break;

>    >    >> >> +        }

>    >    >> >> +

>    >    >> >>          for (i = 0; i < n_txq; i++) {

>    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i,

>dev-

>    >    >>txq_size,

>    >    >> >>                                            dev->socket_id,

>NULL);

>    >    >> >> --

>    >    >> >> 1.9.3

>    >    >> >>

>    >    >> >> _______________________________________________

>    >    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

>    >8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

>    >8bybqH21vcc86SOLmLk&e=

>    >

>

>
Darrell Ball July 11, 2017, 4:03 p.m. UTC | #9
On 7/11/17, 8:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

    >From: Darrell Ball [mailto:dball@vmware.com]

    >Sent: Tuesday, July 11, 2017 2:03 AM

    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

    ><vipin.varghese@intel.com>; aconole@redhat.com

    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >

    >

    >

    >On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

    >

    >    >From: Darrell Ball [mailto:dball@vmware.com]

    >    >Sent: Saturday, July 8, 2017 10:34 PM

    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    >    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

    >    ><vipin.varghese@intel.com>; aconole@redhat.com

    >    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >

    >    >So, this required a 16.11.2 bug fix.

    >    >

    >    >Are we sure this will work for all/most NICs/drivers ?

    >

    >    I can't say tbh Darrell, as I can only test with the NICs that are

    >available to me (namely, Niantic and Fortville).

    >

    >    However, AFAIA, this issue is unique to Niantic, since that H/W permits

    >dynamic modification of MTU without stopping its PMD; for most NICs (including

    >Fortville), a port's PMD must be stopped before modifying its MTU, and

    >restarted thereafter.

    >

    >I understand

    >

    >

    >    >In previous alluding to error cases, did calling rte_eth_dev_set_mtu()

    >block

    >    >jumbo packets

    >    >that would otherwise be allowed ?

    >

    >    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would

    >return EINVAL, which means that the MTU for the NIC's ports could never be

    >changed beyond the standard MTU.

    >

    >    With this patch, that behavior is avoided, and the MTU can be set

    >appropriately.

    >

    >

    >The question that I was getting at is since we are now removing the

    >jumbo_frame flag workaround with this patch, can there be a regression where

    >we cannot

    >forward jumbo frames because rte_eth_dev_set_mtu() fails due to still another

    >bug in some

    >cases ?

    
    Sure, I definitely understand your concerns Darrel - unfortunately, I can't definitively answer yes. However, since OvS now supports DPDK v16.11.2, I would hope that the stability of the latter would provide some level of assurance.
    
    >If the answer is yes, does it make sense to keep the jumbo_frame flag

    >workaround as well,

    >‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle unknown

    >corner cases ?

    
    If we keep the workaround, it essentially negates the need for the netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx packet length are set by rte_eth_dev_configure. The downside of this approach is that it doesn't update the MTU for the Ethernet device itself; as a result, any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the MTU that was set using the existing method.

Let us just go ahead with the patch and fix with any potential fallout/bugs, as it is the right approach
anyways.
    
    Thanks again,
    Mark
    
    >

    >

    >    >

    >    >

    >    >

    >    >On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of

    >Kavanagh,

    >    >Mark B" <ovs-dev-bounces@openvswitch.org on behalf of

    >    >mark.b.kavanagh@intel.com> wrote:

    >    >

    >    >    >From: Chandran, Sugesh

    >    >    >Sent: Friday, July 7, 2017 10:44 AM

    >    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-

    >    >dev@openvswitch.org;

    >    >    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com

    >    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >rte_eth_dev_set_mtu

    >    >    >

    >    >    >Hi Mark,

    >    >    >Thank you for the information.

    >    >    >Please find my comments below.

    >    >    >

    >    >    >Regards

    >    >    >_Sugesh

    >    >    >

    >    >    >

    >    >    >> -----Original Message-----

    >    >    >> From: Kavanagh, Mark B

    >    >    >> Sent: Wednesday, July 5, 2017 10:20 AM

    >    >    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-

    >    >    >> dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>;

    >    >    >> aconole@redhat.com

    >    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >rte_eth_dev_set_mtu

    >    >    >>

    >    >    >>

    >    >    >>

    >    >    >> >From: Chandran, Sugesh

    >    >    >> >Sent: Monday, July 3, 2017 4:36 PM

    >    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;

    >    >    >> >ovs-dev@openvswitch.org; Varghese, Vipin

    ><vipin.varghese@intel.com>;

    >    >    >> >aconole@redhat.com

    >    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >rte_eth_dev_set_mtu

    >    >    >> >

    >    >    >> >Hi Mark,

    >    >    >> >

    >    >    >> >Regards

    >    >    >> >_Sugesh

    >    >    >> >

    >    >    >> >

    >    >    >> >> -----Original Message-----

    >    >    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

    >    >    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh

    >    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM

    >    >    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin

    >    >    >> >> <vipin.varghese@intel.com>; aconole@redhat.com

    >    >    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >rte_eth_dev_set_mtu

    >    >    >> >>

    >    >    >> >> DPDK provides an API to set the MTU of compatible physical

    >devices -

    >    >    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API

    >was

    >    >not

    >    >    >> >> implemented in some DPDK PMDs (i40e, specifically). To allow

    >the use

    >    >    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU

    >configuration

    >    >was

    >    >    >> >> achieved by setting the jumbo frame flag, and corresponding

    >maximum

    >    >    >> >> permitted Rx frame size, in an rte_eth_conf structure for the

    >NIC

    >    >    >> >> port, and subsequently invoking rte_eth_dev_configure() with

    >that

    >    >    >> configuration.

    >    >    >> >>

    >    >    >> >> However, that method does not set the MTU field of the

    >underlying

    >    >    >> >> DPDK structure (rte_eth_dev) for the corresponding physical

    >device;

    >    >    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU

    >for an

    >    >    >> >> OvS-DPDK phy device with non-standard MTU.

    >    >    >> >>

    >    >    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when

    >setting up

    >    >    >> >> or modifying the MTU of a DPDK phy port.

    >    >    >> >>

    >    >    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")

    >    >    >> >> Reported-by: Aaron Conole <aconole@redhat.com>

    >    >    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>

    >    >    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>

    >    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>

    >    >    >> >> ---

    >    >    >> >>

    >    >    >> >> v3->v2:

    >    >    >> >>     - enable scatter_rx explicitly for jumbo MTU

    >    >    >> >>

    >    >    >> >> v2->v1:

    >    >    >> >>     - add 'reported-by' tag for Aaron Conole

    >    >    >> >>         - change VLOG_INFO to VLOG_ERR

    >    >    >> >>

    >    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----

    >    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)

    >    >    >> >>

    >    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index

    >    >    >> >> bba4de3..671d585

    >    >    >> >> 100644

    >    >    >> >> --- a/lib/netdev-dpdk.c

    >    >    >> >> +++ b/lib/netdev-dpdk.c

    >    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct

    >    >    >> netdev_dpdk

    >    >    >> >> *dev, int n_rxq, int n_txq)

    >    >    >> >>      int i;

    >    >    >> >>      struct rte_eth_conf conf = port_conf;

    >    >    >> >>

    >    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to

    >be

    >    >    >explicitly

    >    >    >> >> +     * enabled. */

    >    >    >> >>      if (dev->mtu > ETHER_MTU) {

    >    >    >> >> -        conf.rxmode.jumbo_frame = 1;

    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;

    >    >    >> >> -    } else {

    >    >    >> >> -        conf.rxmode.jumbo_frame = 0;

    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;

    >    >    >> >> +        conf.rxmode.enable_scatter = 1;

    >    >    >> >>      }

    >    >    >> >> +

    >    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &

    >    >    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD)

    >!= 0;

    >    >    >> >>      /* A device may report more queues than it makes available

    >    >(this

    >    >    >> >> has @@

    >    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk

    >    >    >> *dev,

    >    >    >> >> int n_rxq, int n_txq)

    >    >    >> >>              break;

    >    >    >> >>          }

    >    >    >> >>

    >    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);

    >    >    >> >> +        if (diag) {

    >    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",

    >    >    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-

    >diag));

    >    >    >> >[Sugesh] I am still getting this error when I try to test this

    >feature

    >    >    >> >on a Niantic cards.

    >    >    >>

    >    >    >> Hi Sugesh,

    >    >    >>

    >    >    >> What version of DPDK are you using?

    >    >    >>

    >    >    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2,

    >    >since

    >    >    >this

    >    >    >> contains a bugfix for the ixgbe set_mtu function that is required

    >for

    >    >this

    >    >    >> patch.

    >    >    >>

    >    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and

    >you

    >    >can

    >    >    >add

    >    >    >'Tested by'

    >    >    >And the changes are looks ok for me too.

    >    >

    >    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you

    >in

    >    >that case?

    >    >

    >    >    Cheers,

    >    >    Mark

    >    >

    >    >    >

    >    >    >> In any event, I've been in contact with DPDK colleagues wrt the

    >    >inconsistent

    >    >    >> behavior between ixgbe and i40e drivers that warrant this change;

    >    >they've

    >    >    >> pushed a patch to address same:

    >    >    >> https://urldefense.proofpoint.com/v2/url?u=http-

    >

    >>3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ

    >&

    >    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .

    >    >    >> I haven't had a chance to test this patch yet, but if it works, it

    >may

    >    >not

    >    >    >be

    >    >    >> necessary to set scatter_rx mode in OvS.

    >    >    >>

    >    >    >> Thanks,

    >    >    >> Mark

    >    >    >>

    >    >    >>

    >    >    >> >

    >    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500

    ><--

    >    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0

    >    >    >> >mtu_request=5000 2017-07-

    >    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface

    >    >    >> >dpdk0 MTU (5000) setup

    >    >    >> >error: Invalid argument

    >    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1

    >    >    >> txq:2)

    >    >    >> >configure error: Invalid argument

    >    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set

    >interface

    >    >    >> >dpdk0 new configuration

    >    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP

    >status on

    >    >    >> >nonexistent port 1

    >    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP

    >status

    >    >    >> on

    >    >    >> >nonexistent port 1

    >    >    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$

    >    >    >> >2017-07-

    >    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on

    >    >    >> >nonexistent port

    >    >    >> >1

    >    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats

    >on

    >    >    >> >nonexistent port 1

    >    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats

    >on

    >    >    >> >nonexistent port 1

    >    >    >> >

    >    >    >> >I assume just the scatter_flag may not sufficient to enable

    >    >jumbo_frames??

    >    >    >> >Because in my testing I noticed the behavior is same that

    >reported by

    >    >    >> >Ian in the v2 patch. No traffic is getting forwarded through the

    >    >ports.

    >    >    >> >

    >    >    >> >

    >    >    >> >> +            break;

    >    >    >> >> +        }

    >    >    >> >> +

    >    >    >> >>          for (i = 0; i < n_txq; i++) {

    >    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i,

    >dev-

    >    >    >>txq_size,

    >    >    >> >>                                            dev->socket_id,

    >NULL);

    >    >    >> >> --

    >    >    >> >> 1.9.3

    >    >    >> >>

    >    >    >> >> _______________________________________________

    >    >    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >    >8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >    >8bybqH21vcc86SOLmLk&e=

    >    >

    >

    >
Mark Kavanagh July 12, 2017, 8:04 a.m. UTC | #10
>From: Darrell Ball [mailto:dball@vmware.com]

>Sent: Tuesday, July 11, 2017 5:03 PM

>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

><vipin.varghese@intel.com>; aconole@redhat.com

>Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

>

>

>

>On 7/11/17, 8:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

>

>    >From: Darrell Ball [mailto:dball@vmware.com]

>    >Sent: Tuesday, July 11, 2017 2:03 AM

>    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

>    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

>    ><vipin.varghese@intel.com>; aconole@redhat.com

>    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

>    >

>    >

>    >

>    >On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com>

>wrote:

>    >

>    >    >From: Darrell Ball [mailto:dball@vmware.com]

>    >    >Sent: Saturday, July 8, 2017 10:34 PM

>    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

>    >    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese,

>Vipin

>    >    ><vipin.varghese@intel.com>; aconole@redhat.com

>    >    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use

>rte_eth_dev_set_mtu

>    >    >

>    >    >So, this required a 16.11.2 bug fix.

>    >    >

>    >    >Are we sure this will work for all/most NICs/drivers ?

>    >

>    >    I can't say tbh Darrell, as I can only test with the NICs that are

>    >available to me (namely, Niantic and Fortville).

>    >

>    >    However, AFAIA, this issue is unique to Niantic, since that H/W

>permits

>    >dynamic modification of MTU without stopping its PMD; for most NICs

>(including

>    >Fortville), a port's PMD must be stopped before modifying its MTU, and

>    >restarted thereafter.

>    >

>    >I understand

>    >

>    >

>    >    >In previous alluding to error cases, did calling

>rte_eth_dev_set_mtu()

>    >block

>    >    >jumbo packets

>    >    >that would otherwise be allowed ?

>    >

>    >    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would

>    >return EINVAL, which means that the MTU for the NIC's ports could never

>be

>    >changed beyond the standard MTU.

>    >

>    >    With this patch, that behavior is avoided, and the MTU can be set

>    >appropriately.

>    >

>    >

>    >The question that I was getting at is since we are now removing the

>    >jumbo_frame flag workaround with this patch, can there be a regression

>where

>    >we cannot

>    >forward jumbo frames because rte_eth_dev_set_mtu() fails due to still

>another

>    >bug in some

>    >cases ?

>

>    Sure, I definitely understand your concerns Darrel - unfortunately, I

>can't definitively answer yes. However, since OvS now supports DPDK v16.11.2,

>I would hope that the stability of the latter would provide some level of

>assurance.

>

>    >If the answer is yes, does it make sense to keep the jumbo_frame flag

>    >workaround as well,

>    >‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle

>unknown

>    >corner cases ?

>

>    If we keep the workaround, it essentially negates the need for the

>netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx

>packet length are set by rte_eth_dev_configure. The downside of this approach

>is that it doesn't update the MTU for the Ethernet device itself; as a result,

>any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the

>MTU that was set using the existing method.

>

>Let us just go ahead with the patch and fix with any potential fallout/bugs,

>as it is the right approach

>anyways.


Thanks Darrell - much appreciated.

I'm, spinning a v4 to add 'acked-by' and 'tested-by' tags for Sugesh - shall I add an 'acked-by' for you also?

Thanks again,
Mark



>

>    Thanks again,

>    Mark

>

>    >

>    >

>    >    >

>    >    >

>    >    >

>    >    >On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of

>    >Kavanagh,

>    >    >Mark B" <ovs-dev-bounces@openvswitch.org on behalf of

>    >    >mark.b.kavanagh@intel.com> wrote:

>    >    >

>    >    >    >From: Chandran, Sugesh

>    >    >    >Sent: Friday, July 7, 2017 10:44 AM

>    >    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-

>    >    >dev@openvswitch.org;

>    >    >    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com

>    >    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>    >rte_eth_dev_set_mtu

>    >    >    >

>    >    >    >Hi Mark,

>    >    >    >Thank you for the information.

>    >    >    >Please find my comments below.

>    >    >    >

>    >    >    >Regards

>    >    >    >_Sugesh

>    >    >    >

>    >    >    >

>    >    >    >> -----Original Message-----

>    >    >    >> From: Kavanagh, Mark B

>    >    >    >> Sent: Wednesday, July 5, 2017 10:20 AM

>    >    >    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-

>    >    >    >> dev@openvswitch.org; Varghese, Vipin

><vipin.varghese@intel.com>;

>    >    >    >> aconole@redhat.com

>    >    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>    >rte_eth_dev_set_mtu

>    >    >    >>

>    >    >    >>

>    >    >    >>

>    >    >    >> >From: Chandran, Sugesh

>    >    >    >> >Sent: Monday, July 3, 2017 4:36 PM

>    >    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;

>    >    >    >> >ovs-dev@openvswitch.org; Varghese, Vipin

>    ><vipin.varghese@intel.com>;

>    >    >    >> >aconole@redhat.com

>    >    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

>    >rte_eth_dev_set_mtu

>    >    >    >> >

>    >    >    >> >Hi Mark,

>    >    >    >> >

>    >    >    >> >Regards

>    >    >    >> >_Sugesh

>    >    >    >> >

>    >    >    >> >

>    >    >    >> >> -----Original Message-----

>    >    >    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

>    >    >    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh

>    >    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM

>    >    >    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin

>    >    >    >> >> <vipin.varghese@intel.com>; aconole@redhat.com

>    >    >    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use

>    >rte_eth_dev_set_mtu

>    >    >    >> >>

>    >    >    >> >> DPDK provides an API to set the MTU of compatible physical

>    >devices -

>    >    >    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this

>API

>    >was

>    >    >not

>    >    >    >> >> implemented in some DPDK PMDs (i40e, specifically). To

>allow

>    >the use

>    >    >    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU

>    >configuration

>    >    >was

>    >    >    >> >> achieved by setting the jumbo frame flag, and

>corresponding

>    >maximum

>    >    >    >> >> permitted Rx frame size, in an rte_eth_conf structure for

>the

>    >NIC

>    >    >    >> >> port, and subsequently invoking rte_eth_dev_configure()

>with

>    >that

>    >    >    >> configuration.

>    >    >    >> >>

>    >    >    >> >> However, that method does not set the MTU field of the

>    >underlying

>    >    >    >> >> DPDK structure (rte_eth_dev) for the corresponding

>physical

>    >device;

>    >    >    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect

>MTU

>    >for an

>    >    >    >> >> OvS-DPDK phy device with non-standard MTU.

>    >    >    >> >>

>    >    >    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when

>    >setting up

>    >    >    >> >> or modifying the MTU of a DPDK phy port.

>    >    >    >> >>

>    >    >    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo

>frames")

>    >    >    >> >> Reported-by: Aaron Conole <aconole@redhat.com>

>    >    >    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>

>    >    >    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>

>    >    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>

>    >    >    >> >> ---

>    >    >    >> >>

>    >    >    >> >> v3->v2:

>    >    >    >> >>     - enable scatter_rx explicitly for jumbo MTU

>    >    >    >> >>

>    >    >    >> >> v2->v1:

>    >    >    >> >>     - add 'reported-by' tag for Aaron Conole

>    >    >    >> >>         - change VLOG_INFO to VLOG_ERR

>    >    >    >> >>

>    >    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----

>    >    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)

>    >    >    >> >>

>    >    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index

>    >    >    >> >> bba4de3..671d585

>    >    >    >> >> 100644

>    >    >    >> >> --- a/lib/netdev-dpdk.c

>    >    >    >> >> +++ b/lib/netdev-dpdk.c

>    >    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct

>    >    >    >> netdev_dpdk

>    >    >    >> >> *dev, int n_rxq, int n_txq)

>    >    >    >> >>      int i;

>    >    >    >> >>      struct rte_eth_conf conf = port_conf;

>    >    >    >> >>

>    >    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode

>needs to

>    >be

>    >    >    >explicitly

>    >    >    >> >> +     * enabled. */

>    >    >    >> >>      if (dev->mtu > ETHER_MTU) {

>    >    >    >> >> -        conf.rxmode.jumbo_frame = 1;

>    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;

>    >    >    >> >> -    } else {

>    >    >    >> >> -        conf.rxmode.jumbo_frame = 0;

>    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;

>    >    >    >> >> +        conf.rxmode.enable_scatter = 1;

>    >    >    >> >>      }

>    >    >    >> >> +

>    >    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &

>    >    >    >> >>

>NETDEV_RX_CHECKSUM_OFFLOAD)

>    >!= 0;

>    >    >    >> >>      /* A device may report more queues than it makes

>available

>    >    >(this

>    >    >    >> >> has @@

>    >    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct

>netdev_dpdk

>    >    >    >> *dev,

>    >    >    >> >> int n_rxq, int n_txq)

>    >    >    >> >>              break;

>    >    >    >> >>          }

>    >    >    >> >>

>    >    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev-

>>mtu);

>    >    >    >> >> +        if (diag) {

>    >    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error:

>%s",

>    >    >    >> >> +                    dev->up.name, dev->mtu,

>rte_strerror(-

>    >diag));

>    >    >    >> >[Sugesh] I am still getting this error when I try to test

>this

>    >feature

>    >    >    >> >on a Niantic cards.

>    >    >    >>

>    >    >    >> Hi Sugesh,

>    >    >    >>

>    >    >    >> What version of DPDK are you using?

>    >    >    >>

>    >    >    >> I'm guessing that it's 16.11.0 - please try testing with

>v16.11.2,

>    >    >since

>    >    >    >this

>    >    >    >> contains a bugfix for the ixgbe set_mtu function that is

>required

>    >for

>    >    >this

>    >    >    >> patch.

>    >    >    >>

>    >    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup

>and

>    >you

>    >    >can

>    >    >    >add

>    >    >    >'Tested by'

>    >    >    >And the changes are looks ok for me too.

>    >    >

>    >    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for

>you

>    >in

>    >    >that case?

>    >    >

>    >    >    Cheers,

>    >    >    Mark

>    >    >

>    >    >    >

>    >    >    >> In any event, I've been in contact with DPDK colleagues wrt

>the

>    >    >inconsistent

>    >    >    >> behavior between ixgbe and i40e drivers that warrant this

>change;

>    >    >they've

>    >    >    >> pushed a patch to address same:

>    >    >    >> https://urldefense.proofpoint.com/v2/url?u=http-

>    >

>

>>>3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRg

>Q

>    >&

>    >    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>    >    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .

>    >    >    >> I haven't had a chance to test this patch yet, but if it

>works, it

>    >may

>    >    >not

>    >    >    >be

>    >    >    >> necessary to set scatter_rx mode in OvS.

>    >    >    >>

>    >    >    >> Thanks,

>    >    >    >> Mark

>    >    >    >>

>    >    >    >>

>    >    >    >> >

>    >    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0

>mtu_request=1500

>    ><--

>    >    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface

>dpdk0

>    >    >    >> >mtu_request=5000 2017-07-

>    >    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface

>    >    >    >> >dpdk0 MTU (5000) setup

>    >    >    >> >error: Invalid argument

>    >    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface

>dpdk0(rxq:1

>    >    >    >> txq:2)

>    >    >    >> >configure error: Invalid argument

>    >    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set

>    >interface

>    >    >    >> >dpdk0 new configuration

>    >    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP

>    >status on

>    >    >    >> >nonexistent port 1

>    >    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP

>    >status

>    >    >    >> on

>    >    >    >> >nonexistent port 1

>    >    >    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$

>    >    >    >> >2017-07-

>    >    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on

>    >    >    >> >nonexistent port

>    >    >    >> >1

>    >    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP

>stats

>    >on

>    >    >    >> >nonexistent port 1

>    >    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP

>stats

>    >on

>    >    >    >> >nonexistent port 1

>    >    >    >> >

>    >    >    >> >I assume just the scatter_flag may not sufficient to enable

>    >    >jumbo_frames??

>    >    >    >> >Because in my testing I noticed the behavior is same that

>    >reported by

>    >    >    >> >Ian in the v2 patch. No traffic is getting forwarded through

>the

>    >    >ports.

>    >    >    >> >

>    >    >    >> >

>    >    >    >> >> +            break;

>    >    >    >> >> +        }

>    >    >    >> >> +

>    >    >    >> >>          for (i = 0; i < n_txq; i++) {

>    >    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id,

>i,

>    >dev-

>    >    >    >>txq_size,

>    >    >    >> >>                                            dev->socket_id,

>    >NULL);

>    >    >    >> >> --

>    >    >    >> >> 1.9.3

>    >    >    >> >>

>    >    >    >> >> _______________________________________________

>    >    >    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

>    >    >8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

>    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

>    >    >8bybqH21vcc86SOLmLk&e=

>    >    >

>    >

>    >

>

>
Darrell Ball July 12, 2017, 4:47 p.m. UTC | #11
On 7/12/17, 1:04 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

    >From: Darrell Ball [mailto:dball@vmware.com]

    >Sent: Tuesday, July 11, 2017 5:03 PM

    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

    ><vipin.varghese@intel.com>; aconole@redhat.com

    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >

    >

    >

    >On 7/11/17, 8:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com> wrote:

    >

    >    >From: Darrell Ball [mailto:dball@vmware.com]

    >    >Sent: Tuesday, July 11, 2017 2:03 AM

    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    >    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese, Vipin

    >    ><vipin.varghese@intel.com>; aconole@redhat.com

    >    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

    >    >

    >    >

    >    >

    >    >On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh@intel.com>

    >wrote:

    >    >

    >    >    >From: Darrell Ball [mailto:dball@vmware.com]

    >    >    >Sent: Saturday, July 8, 2017 10:34 PM

    >    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Chandran, Sugesh

    >    >    ><sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Varghese,

    >Vipin

    >    >    ><vipin.varghese@intel.com>; aconole@redhat.com

    >    >    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >rte_eth_dev_set_mtu

    >    >    >

    >    >    >So, this required a 16.11.2 bug fix.

    >    >    >

    >    >    >Are we sure this will work for all/most NICs/drivers ?

    >    >

    >    >    I can't say tbh Darrell, as I can only test with the NICs that are

    >    >available to me (namely, Niantic and Fortville).

    >    >

    >    >    However, AFAIA, this issue is unique to Niantic, since that H/W

    >permits

    >    >dynamic modification of MTU without stopping its PMD; for most NICs

    >(including

    >    >Fortville), a port's PMD must be stopped before modifying its MTU, and

    >    >restarted thereafter.

    >    >

    >    >I understand

    >    >

    >    >

    >    >    >In previous alluding to error cases, did calling

    >rte_eth_dev_set_mtu()

    >    >block

    >    >    >jumbo packets

    >    >    >that would otherwise be allowed ?

    >    >

    >    >    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would

    >    >return EINVAL, which means that the MTU for the NIC's ports could never

    >be

    >    >changed beyond the standard MTU.

    >    >

    >    >    With this patch, that behavior is avoided, and the MTU can be set

    >    >appropriately.

    >    >

    >    >

    >    >The question that I was getting at is since we are now removing the

    >    >jumbo_frame flag workaround with this patch, can there be a regression

    >where

    >    >we cannot

    >    >forward jumbo frames because rte_eth_dev_set_mtu() fails due to still

    >another

    >    >bug in some

    >    >cases ?

    >

    >    Sure, I definitely understand your concerns Darrel - unfortunately, I

    >can't definitively answer yes. However, since OvS now supports DPDK v16.11.2,

    >I would hope that the stability of the latter would provide some level of

    >assurance.

    >

    >    >If the answer is yes, does it make sense to keep the jumbo_frame flag

    >    >workaround as well,

    >    >‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle

    >unknown

    >    >corner cases ?

    >

    >    If we keep the workaround, it essentially negates the need for the

    >netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx

    >packet length are set by rte_eth_dev_configure. The downside of this approach

    >is that it doesn't update the MTU for the Ethernet device itself; as a result,

    >any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the

    >MTU that was set using the existing method.

    >

    >Let us just go ahead with the patch and fix with any potential fallout/bugs,

    >as it is the right approach

    >anyways.

    
    Thanks Darrell - much appreciated.
    
    I'm, spinning a v4 to add 'acked-by' and 'tested-by' tags for Sugesh - shall I add an 'acked-by' for you also?
    
    Thanks again,
    Mark
    

Pls add my Acked-by

Thanks Darrell


    
    
    >

    >    Thanks again,

    >    Mark

    >

    >    >

    >    >

    >    >    >

    >    >    >

    >    >    >

    >    >    >On 7/7/17, 3:18 AM, "ovs-dev-bounces@openvswitch.org on behalf of

    >    >Kavanagh,

    >    >    >Mark B" <ovs-dev-bounces@openvswitch.org on behalf of

    >    >    >mark.b.kavanagh@intel.com> wrote:

    >    >    >

    >    >    >    >From: Chandran, Sugesh

    >    >    >    >Sent: Friday, July 7, 2017 10:44 AM

    >    >    >    >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; ovs-

    >    >    >dev@openvswitch.org;

    >    >    >    >Varghese, Vipin <vipin.varghese@intel.com>; aconole@redhat.com

    >    >    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >    >rte_eth_dev_set_mtu

    >    >    >    >

    >    >    >    >Hi Mark,

    >    >    >    >Thank you for the information.

    >    >    >    >Please find my comments below.

    >    >    >    >

    >    >    >    >Regards

    >    >    >    >_Sugesh

    >    >    >    >

    >    >    >    >

    >    >    >    >> -----Original Message-----

    >    >    >    >> From: Kavanagh, Mark B

    >    >    >    >> Sent: Wednesday, July 5, 2017 10:20 AM

    >    >    >    >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; ovs-

    >    >    >    >> dev@openvswitch.org; Varghese, Vipin

    ><vipin.varghese@intel.com>;

    >    >    >    >> aconole@redhat.com

    >    >    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >    >rte_eth_dev_set_mtu

    >    >    >    >>

    >    >    >    >>

    >    >    >    >>

    >    >    >    >> >From: Chandran, Sugesh

    >    >    >    >> >Sent: Monday, July 3, 2017 4:36 PM

    >    >    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>;

    >    >    >    >> >ovs-dev@openvswitch.org; Varghese, Vipin

    >    ><vipin.varghese@intel.com>;

    >    >    >    >> >aconole@redhat.com

    >    >    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >    >rte_eth_dev_set_mtu

    >    >    >    >> >

    >    >    >    >> >Hi Mark,

    >    >    >    >> >

    >    >    >    >> >Regards

    >    >    >    >> >_Sugesh

    >    >    >    >> >

    >    >    >    >> >

    >    >    >    >> >> -----Original Message-----

    >    >    >    >> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-

    >    >    >    >> >> bounces@openvswitch.org] On Behalf Of Mark Kavanagh

    >    >    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM

    >    >    >    >> >> To: ovs-dev@openvswitch.org; Varghese, Vipin

    >    >    >    >> >> <vipin.varghese@intel.com>; aconole@redhat.com

    >    >    >    >> >> Subject: [ovs-dev] [PATCH v3] netdev-dpdk: use

    >    >rte_eth_dev_set_mtu

    >    >    >    >> >>

    >    >    >    >> >> DPDK provides an API to set the MTU of compatible physical

    >    >devices -

    >    >    >    >> >> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this

    >API

    >    >was

    >    >    >not

    >    >    >    >> >> implemented in some DPDK PMDs (i40e, specifically). To

    >allow

    >    >the use

    >    >    >    >> >> of jumbo frames with affected NICs in OvS-DPDK, MTU

    >    >configuration

    >    >    >was

    >    >    >    >> >> achieved by setting the jumbo frame flag, and

    >corresponding

    >    >maximum

    >    >    >    >> >> permitted Rx frame size, in an rte_eth_conf structure for

    >the

    >    >NIC

    >    >    >    >> >> port, and subsequently invoking rte_eth_dev_configure()

    >with

    >    >that

    >    >    >    >> configuration.

    >    >    >    >> >>

    >    >    >    >> >> However, that method does not set the MTU field of the

    >    >underlying

    >    >    >    >> >> DPDK structure (rte_eth_dev) for the corresponding

    >physical

    >    >device;

    >    >    >    >> >> consequently, rte_eth_dev_get_mtu() reports the incorrect

    >MTU

    >    >for an

    >    >    >    >> >> OvS-DPDK phy device with non-standard MTU.

    >    >    >    >> >>

    >    >    >    >> >> Resolve this issue by invoking rte_eth_dev_set_mtu() when

    >    >setting up

    >    >    >    >> >> or modifying the MTU of a DPDK phy port.

    >    >    >    >> >>

    >    >    >    >> >> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo

    >frames")

    >    >    >    >> >> Reported-by: Aaron Conole <aconole@redhat.com>

    >    >    >    >> >> Reported-by: Vipin Varghese <vipin.varghese@intel.com>

    >    >    >    >> >> Reviewed-by: Aaron Conole <aconole@redhat.com>

    >    >    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>

    >    >    >    >> >> ---

    >    >    >    >> >>

    >    >    >    >> >> v3->v2:

    >    >    >    >> >>     - enable scatter_rx explicitly for jumbo MTU

    >    >    >    >> >>

    >    >    >    >> >> v2->v1:

    >    >    >    >> >>     - add 'reported-by' tag for Aaron Conole

    >    >    >    >> >>         - change VLOG_INFO to VLOG_ERR

    >    >    >    >> >>

    >    >    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----

    >    >    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)

    >    >    >    >> >>

    >    >    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index

    >    >    >    >> >> bba4de3..671d585

    >    >    >    >> >> 100644

    >    >    >    >> >> --- a/lib/netdev-dpdk.c

    >    >    >    >> >> +++ b/lib/netdev-dpdk.c

    >    >    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct

    >    >    >    >> netdev_dpdk

    >    >    >    >> >> *dev, int n_rxq, int n_txq)

    >    >    >    >> >>      int i;

    >    >    >    >> >>      struct rte_eth_conf conf = port_conf;

    >    >    >    >> >>

    >    >    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode

    >needs to

    >    >be

    >    >    >    >explicitly

    >    >    >    >> >> +     * enabled. */

    >    >    >    >> >>      if (dev->mtu > ETHER_MTU) {

    >    >    >    >> >> -        conf.rxmode.jumbo_frame = 1;

    >    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;

    >    >    >    >> >> -    } else {

    >    >    >    >> >> -        conf.rxmode.jumbo_frame = 0;

    >    >    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;

    >    >    >    >> >> +        conf.rxmode.enable_scatter = 1;

    >    >    >    >> >>      }

    >    >    >    >> >> +

    >    >    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &

    >    >    >    >> >>

    >NETDEV_RX_CHECKSUM_OFFLOAD)

    >    >!= 0;

    >    >    >    >> >>      /* A device may report more queues than it makes

    >available

    >    >    >(this

    >    >    >    >> >> has @@

    >    >    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct

    >netdev_dpdk

    >    >    >    >> *dev,

    >    >    >    >> >> int n_rxq, int n_txq)

    >    >    >    >> >>              break;

    >    >    >    >> >>          }

    >    >    >    >> >>

    >    >    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev-

    >>mtu);

    >    >    >    >> >> +        if (diag) {

    >    >    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error:

    >%s",

    >    >    >    >> >> +                    dev->up.name, dev->mtu,

    >rte_strerror(-

    >    >diag));

    >    >    >    >> >[Sugesh] I am still getting this error when I try to test

    >this

    >    >feature

    >    >    >    >> >on a Niantic cards.

    >    >    >    >>

    >    >    >    >> Hi Sugesh,

    >    >    >    >>

    >    >    >    >> What version of DPDK are you using?

    >    >    >    >>

    >    >    >    >> I'm guessing that it's 16.11.0 - please try testing with

    >v16.11.2,

    >    >    >since

    >    >    >    >this

    >    >    >    >> contains a bugfix for the ixgbe set_mtu function that is

    >required

    >    >for

    >    >    >this

    >    >    >    >> patch.

    >    >    >    >>

    >    >    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup

    >and

    >    >you

    >    >    >can

    >    >    >    >add

    >    >    >    >'Tested by'

    >    >    >    >And the changes are looks ok for me too.

    >    >    >

    >    >    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for

    >you

    >    >in

    >    >    >that case?

    >    >    >

    >    >    >    Cheers,

    >    >    >    Mark

    >    >    >

    >    >    >    >

    >    >    >    >> In any event, I've been in contact with DPDK colleagues wrt

    >the

    >    >    >inconsistent

    >    >    >    >> behavior between ixgbe and i40e drivers that warrant this

    >change;

    >    >    >they've

    >    >    >    >> pushed a patch to address same:

    >    >    >    >> https://urldefense.proofpoint.com/v2/url?u=http-

    >    >

    >

    >>>3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRg

    >Q

    >    >&

    >    >    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >    >    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .

    >    >    >    >> I haven't had a chance to test this patch yet, but if it

    >works, it

    >    >may

    >    >    >not

    >    >    >    >be

    >    >    >    >> necessary to set scatter_rx mode in OvS.

    >    >    >    >>

    >    >    >    >> Thanks,

    >    >    >    >> Mark

    >    >    >    >>

    >    >    >    >>

    >    >    >    >> >

    >    >    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0

    >mtu_request=1500

    >    ><--

    >    >    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface

    >dpdk0

    >    >    >    >> >mtu_request=5000 2017-07-

    >    >    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface

    >    >    >    >> >dpdk0 MTU (5000) setup

    >    >    >    >> >error: Invalid argument

    >    >    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface

    >dpdk0(rxq:1

    >    >    >    >> txq:2)

    >    >    >    >> >configure error: Invalid argument

    >    >    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set

    >    >interface

    >    >    >    >> >dpdk0 new configuration

    >    >    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP

    >    >status on

    >    >    >    >> >nonexistent port 1

    >    >    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP

    >    >status

    >    >    >    >> on

    >    >    >    >> >nonexistent port 1

    >    >    >    >> sugeshch@silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$

    >    >    >    >> >2017-07-

    >    >    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on

    >    >    >    >> >nonexistent port

    >    >    >    >> >1

    >    >    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP

    >stats

    >    >on

    >    >    >    >> >nonexistent port 1

    >    >    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP

    >stats

    >    >on

    >    >    >    >> >nonexistent port 1

    >    >    >    >> >

    >    >    >    >> >I assume just the scatter_flag may not sufficient to enable

    >    >    >jumbo_frames??

    >    >    >    >> >Because in my testing I noticed the behavior is same that

    >    >reported by

    >    >    >    >> >Ian in the v2 patch. No traffic is getting forwarded through

    >the

    >    >    >ports.

    >    >    >    >> >

    >    >    >    >> >

    >    >    >    >> >> +            break;

    >    >    >    >> >> +        }

    >    >    >    >> >> +

    >    >    >    >> >>          for (i = 0; i < n_txq; i++) {

    >    >    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id,

    >i,

    >    >dev-

    >    >    >    >>txq_size,

    >    >    >    >> >>                                            dev->socket_id,

    >    >NULL);

    >    >    >    >> >> --

    >    >    >    >> >> 1.9.3

    >    >    >    >> >>

    >    >    >    >> >> _______________________________________________

    >    >    >    >> >> 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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >    >    >8bybqH21vcc86SOLmLk&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=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-

    >    >540&s=hrqvPrPvbz7fNIQ2uSHjDH0-

    >    >    >8bybqH21vcc86SOLmLk&e=

    >    >    >

    >    >

    >    >

    >

    >
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bba4de3..671d585 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -650,13 +650,12 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
     int i;
     struct rte_eth_conf conf = port_conf;
 
+    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly
+     * enabled. */
     if (dev->mtu > ETHER_MTU) {
-        conf.rxmode.jumbo_frame = 1;
-        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
-    } else {
-        conf.rxmode.jumbo_frame = 0;
-        conf.rxmode.max_rx_pkt_len = 0;
+        conf.rxmode.enable_scatter = 1;
     }
+
     conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
                                   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
     /* A device may report more queues than it makes available (this has
@@ -676,6 +675,13 @@  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq)
             break;
         }
 
+        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
+        if (diag) {
+            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
+                    dev->up.name, dev->mtu, rte_strerror(-diag));
+            break;
+        }
+
         for (i = 0; i < n_txq; i++) {
             diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
                                           dev->socket_id, NULL);