| Message ID | 1525353976-28278-1-git-send-email-ian.stokes@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev,v1] netdev-dpdk: Handle ENOTSUP for rte_eth_dev_set_mtu. | expand |
> > The function rte_eth_dev_set_mtu is not supported for all DPDK drivers. > Currently if it is not supported we return an error in > dpdk_eth_dev_queue_setup. There are two issues with this. > > (i) A device can still function even if rte_eth_dev_set_mtu is not > supported albeit with the default max rx packet length. > > (ii) When the ENOTSUP is returned it will not be caught in > port_reconfigure() at the dpif-netdev layer. Port_reconfigure() checks > if a netdev_reconfigure() function is supported for a given netdev and > ignores EOPNOTSUPP errors as it assumes errors of this value mean there > is no reconfiguration function. In this case the reconfiguration function > is supported for netdev dpdk but a function called as part of the > reconfigure (rte_eth_dev_set_mtu) may not be supported. > > As this is a corner case, this commit warns a user when > rte_eth_dev_set_mtu is not supported and informs them of the default > max rx packet length that will be used instead. > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > Co-author: Michal Weglicki <michalx.weglicki@intel.com> Tested-by: Ciara Loftus <ciara.loftus@intel.com> Hi Ian, Thanks for the patch. It needs a rebase. PCAP vdevs fall under the (i) category. Before this patch I always observed a SEGV on pcap rx. Queues were not setup for pcap devices due to the failed mtu_set function, and later polling on those non-existent queues caused the SEGV. This patch removes this case and reports the error as described instead. Thanks, Ciara > --- > lib/netdev-dpdk.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 50c7619..e6c483d 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -771,6 +771,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk > *dev, int n_rxq, int n_txq) > int diag = 0; > int i; > struct rte_eth_conf conf = port_conf; > + uint16_t conf_mtu; > > /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly > * enabled. */ > @@ -804,9 +805,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk > *dev, int n_rxq, int n_txq) > > 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; > + /* A device may not support rte_eth_dev_set_mtu, in this case > + * check flag a warning to the user and include the devices > + * configured MTU value that will be used instead. */ > + if (-ENOTSUP == diag) { > + rte_eth_dev_get_mtu(dev->port_id, &conf_mtu); > + VLOG_WARN("Interface %s does not support MTU configuration, " > + "max packet size supported is %d.", > + dev->up.name, conf_mtu); > + } else { > + 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++) { > -- > 2.7.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > > The function rte_eth_dev_set_mtu is not supported for all DPDK drivers. > > Currently if it is not supported we return an error in > > dpdk_eth_dev_queue_setup. There are two issues with this. > > > > (i) A device can still function even if rte_eth_dev_set_mtu is not > > supported albeit with the default max rx packet length. > > > > (ii) When the ENOTSUP is returned it will not be caught in > > port_reconfigure() at the dpif-netdev layer. Port_reconfigure() checks > > if a netdev_reconfigure() function is supported for a given netdev and > > ignores EOPNOTSUPP errors as it assumes errors of this value mean > > there is no reconfiguration function. In this case the reconfiguration > > function is supported for netdev dpdk but a function called as part of > > the reconfigure (rte_eth_dev_set_mtu) may not be supported. > > > > As this is a corner case, this commit warns a user when > > rte_eth_dev_set_mtu is not supported and informs them of the default > > max rx packet length that will be used instead. > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > > Co-author: Michal Weglicki <michalx.weglicki@intel.com> > > Tested-by: Ciara Loftus <ciara.loftus@intel.com> > > Hi Ian, > > Thanks for the patch. It needs a rebase. > > PCAP vdevs fall under the (i) category. Before this patch I always > observed a SEGV on pcap rx. Queues were not setup for pcap devices due to > the failed mtu_set function, and later polling on those non-existent > queues caused the SEGV. This patch removes this case and reports the error > as described instead. > > Thanks, > Ciara Thanks for testing Ciara, I've posted a v2 of the patch for the rebase and some minor changes to comments. https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/347399.html Thanks Ian > > > --- > > lib/netdev-dpdk.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > 50c7619..e6c483d 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -771,6 +771,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, > > int n_rxq, int n_txq) > > int diag = 0; > > int i; > > struct rte_eth_conf conf = port_conf; > > + uint16_t conf_mtu; > > > > /* For some NICs (e.g. Niantic), scatter_rx mode needs to be > explicitly > > * enabled. */ > > @@ -804,9 +805,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, > > int n_rxq, int n_txq) > > > > 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; > > + /* A device may not support rte_eth_dev_set_mtu, in this > case > > + * check flag a warning to the user and include the devices > > + * configured MTU value that will be used instead. */ > > + if (-ENOTSUP == diag) { > > + rte_eth_dev_get_mtu(dev->port_id, &conf_mtu); > > + VLOG_WARN("Interface %s does not support MTU > configuration, " > > + "max packet size supported is %d.", > > + dev->up.name, conf_mtu); > > + } else { > > + 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++) { > > -- > > 2.7.5 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 50c7619..e6c483d 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -771,6 +771,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq) int diag = 0; int i; struct rte_eth_conf conf = port_conf; + uint16_t conf_mtu; /* For some NICs (e.g. Niantic), scatter_rx mode needs to be explicitly * enabled. */ @@ -804,9 +805,19 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int n_txq) 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; + /* A device may not support rte_eth_dev_set_mtu, in this case + * check flag a warning to the user and include the devices + * configured MTU value that will be used instead. */ + if (-ENOTSUP == diag) { + rte_eth_dev_get_mtu(dev->port_id, &conf_mtu); + VLOG_WARN("Interface %s does not support MTU configuration, " + "max packet size supported is %d.", + dev->up.name, conf_mtu); + } else { + 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++) {
The function rte_eth_dev_set_mtu is not supported for all DPDK drivers. Currently if it is not supported we return an error in dpdk_eth_dev_queue_setup. There are two issues with this. (i) A device can still function even if rte_eth_dev_set_mtu is not supported albeit with the default max rx packet length. (ii) When the ENOTSUP is returned it will not be caught in port_reconfigure() at the dpif-netdev layer. Port_reconfigure() checks if a netdev_reconfigure() function is supported for a given netdev and ignores EOPNOTSUPP errors as it assumes errors of this value mean there is no reconfiguration function. In this case the reconfiguration function is supported for netdev dpdk but a function called as part of the reconfigure (rte_eth_dev_set_mtu) may not be supported. As this is a corner case, this commit warns a user when rte_eth_dev_set_mtu is not supported and informs them of the default max rx packet length that will be used instead. Signed-off-by: Ian Stokes <ian.stokes@intel.com> Co-author: Michal Weglicki <michalx.weglicki@intel.com> --- lib/netdev-dpdk.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)