| Message ID | 20230209162925.463101-1-ktraynor@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] netdev-dpdk: Check rx/tx descriptor sizes for device. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Thu, Feb 9, 2023 at 5:29 PM Kevin Traynor <ktraynor@redhat.com> wrote: > > By default OVS configures 2048 descriptors for tx and rx queues > on DPDK devices. It also allows the user to configure those values. > > If the values used are not acceptable to the device than queue setup > will fail. > > The device exposes it's max/min/alignment requirements, so use those > to ensure that an acceptable value is used during queue setup. > > If the default or user value is not acceptable, adjust to a suitable > value. > > Reported-at: https://bugzilla.redhat.com/2119876 > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > --- > lib/netdev-dpdk.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index fb0dd43f7..e901f857e 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1008,4 +1008,13 @@ dpdk_watchdog(void *dummy OVS_UNUSED) > } > > +static int > +dpdk_limit_desc_size(int desc_size, struct rte_eth_desc_lim *lim) > +{ > + desc_size = ROUND_UP(desc_size, lim->nb_align); Based on a quick grep, it seems possible a driver exposes nb_align == 0. Like for example: https://git.dpdk.org/dpdk/tree/drivers/net/bnxt/bnxt_ethdev.c#n1020 > + desc_size = MIN(desc_size, lim->nb_max); > + desc_size = MAX(desc_size, lim->nb_min); > + return desc_size; Automatically adjusting a default value seems normal to me. But in the case of user input, I think it would help if there was some log indicating that OVS adjusted the config. > +} > + > static int > dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) > @@ -1056,4 +1065,7 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) > } > > + dev->rxq_size = dpdk_limit_desc_size(dev->rxq_size, &info.rx_desc_lim); > + dev->txq_size = dpdk_limit_desc_size(dev->txq_size, &info.tx_desc_lim); > + > /* A device may report more queues than it makes available (this has > * been observed for Intel xl710, which reserves some of them for > -- > 2.39.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Mar 6, 2023 at 5:05 PM David Marchand <david.marchand@redhat.com> wrote: > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index fb0dd43f7..e901f857e 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -1008,4 +1008,13 @@ dpdk_watchdog(void *dummy OVS_UNUSED) > > } > > > > +static int > > +dpdk_limit_desc_size(int desc_size, struct rte_eth_desc_lim *lim) > > +{ > > + desc_size = ROUND_UP(desc_size, lim->nb_align); > > Based on a quick grep, it seems possible a driver exposes nb_align == 0. > Like for example: > https://git.dpdk.org/dpdk/tree/drivers/net/bnxt/bnxt_ethdev.c#n1020 Ok... you can scratch that, ethdev provides a 1 default value. https://git.dpdk.org/dpdk/tree/lib/ethdev/rte_ethdev.h#n3275 https://git.dpdk.org/dpdk/tree/lib/ethdev/rte_ethdev.c#n3577
On 06/03/2023 16:05, David Marchand wrote: > On Thu, Feb 9, 2023 at 5:29 PM Kevin Traynor <ktraynor@redhat.com> > wrote: >> >> By default OVS configures 2048 descriptors for tx and rx queues on >> DPDK devices. It also allows the user to configure those values. >> >> If the values used are not acceptable to the device than queue >> setup will fail. >> >> The device exposes it's max/min/alignment requirements, so use >> those to ensure that an acceptable value is used during queue >> setup. >> >> If the default or user value is not acceptable, adjust to a >> suitable value. >> >> Reported-at: https://bugzilla.redhat.com/2119876 Signed-off-by: >> Kevin Traynor <ktraynor@redhat.com> --- lib/netdev-dpdk.c | 12 >> ++++++++++++ 1 file changed, 12 insertions(+) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >> fb0dd43f7..e901f857e 100644 --- a/lib/netdev-dpdk.c +++ >> b/lib/netdev-dpdk.c @@ -1008,4 +1008,13 @@ dpdk_watchdog(void >> *dummy OVS_UNUSED) } >> >> +static int +dpdk_limit_desc_size(int desc_size, struct >> rte_eth_desc_lim *lim) +{ + desc_size = ROUND_UP(desc_size, >> lim->nb_align); > > Based on a quick grep, it seems possible a driver exposes nb_align == > 0. Like for example: > https://git.dpdk.org/dpdk/tree/drivers/net/bnxt/bnxt_ethdev.c#n1020 > >> + desc_size = MIN(desc_size, lim->nb_max); + desc_size = >> MAX(desc_size, lim->nb_min); + return desc_size; > > Automatically adjusting a default value seems normal to me. But in > the case of user input, I think it would help if there was some log > indicating that OVS adjusted the config. > The requested and actual are reported in the device status as they can already be adjusted elsewhere, but I take your point that having it at info level in the log could be easier to spot. I have code to add the log but I need to check it's not too noisy at info level and check about consistency with other requested config like mtu. Thanks. > >> +} + static int dpdk_eth_dev_port_config(struct netdev_dpdk *dev, >> int n_rxq, int n_txq) @@ -1056,4 +1065,7 @@ >> dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int >> n_txq) } >> >> + dev->rxq_size = dpdk_limit_desc_size(dev->rxq_size, >> &info.rx_desc_lim); + dev->txq_size = >> dpdk_limit_desc_size(dev->txq_size, &info.tx_desc_lim); + /* A >> device may report more queues than it makes available (this has * >> been observed for Intel xl710, which reserves some of them for -- >> 2.39.1 >> >> _______________________________________________ dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >
On 08/03/2023 17:54, Kevin Traynor wrote: > On 06/03/2023 16:05, David Marchand wrote: >> On Thu, Feb 9, 2023 at 5:29 PM Kevin Traynor <ktraynor@redhat.com> >> wrote: >>> >>> By default OVS configures 2048 descriptors for tx and rx queues on >>> DPDK devices. It also allows the user to configure those values. >>> >>> If the values used are not acceptable to the device than queue >>> setup will fail. >>> >>> The device exposes it's max/min/alignment requirements, so use >>> those to ensure that an acceptable value is used during queue >>> setup. >>> >>> If the default or user value is not acceptable, adjust to a >>> suitable value. >>> >>> Reported-at: https://bugzilla.redhat.com/2119876 Signed-off-by: >>> Kevin Traynor <ktraynor@redhat.com> --- lib/netdev-dpdk.c | 12 >>> ++++++++++++ 1 file changed, 12 insertions(+) >>> >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>> fb0dd43f7..e901f857e 100644 --- a/lib/netdev-dpdk.c +++ >>> b/lib/netdev-dpdk.c @@ -1008,4 +1008,13 @@ dpdk_watchdog(void >>> *dummy OVS_UNUSED) } >>> >>> +static int +dpdk_limit_desc_size(int desc_size, struct >>> rte_eth_desc_lim *lim) +{ + desc_size = ROUND_UP(desc_size, >>> lim->nb_align); >> >> Based on a quick grep, it seems possible a driver exposes nb_align == >> 0. Like for example: >> https://git.dpdk.org/dpdk/tree/drivers/net/bnxt/bnxt_ethdev.c#n1020 >> >>> + desc_size = MIN(desc_size, lim->nb_max); + desc_size = >>> MAX(desc_size, lim->nb_min); + return desc_size; >> >> Automatically adjusting a default value seems normal to me. But in >> the case of user input, I think it would help if there was some log >> indicating that OVS adjusted the config. >> > > The requested and actual are reported in the device status as they can > already be adjusted elsewhere, but I take your point that having it at > info level in the log could be easier to spot. > > I have code to add the log but I need to check it's not too noisy at > info level and check about consistency with other requested config like > mtu. Thanks. > In v2, I combined the new and existing sections that can adjust the desc sizes into one place, called before device reconfig. We need a bit more tracking, to prevent unnecessary reconfigs. But we would need that either way, as we don't want 'actual != requested' to think a reconfigure is needed, only to find out later the requested is not possible, and have the same thing keep occurring. It looks like previously, it could silently adjust the requested size and it was only being reported in dpctl/show (as the requested size!). Now we'll have a log for any adjustment and dpctl/show will show the real requested value. >> >>> +} + static int dpdk_eth_dev_port_config(struct netdev_dpdk *dev, >>> int n_rxq, int n_txq) @@ -1056,4 +1065,7 @@ >>> dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int >>> n_txq) } >>> >>> + dev->rxq_size = dpdk_limit_desc_size(dev->rxq_size, >>> &info.rx_desc_lim); + dev->txq_size = >>> dpdk_limit_desc_size(dev->txq_size, &info.tx_desc_lim); + /* A >>> device may report more queues than it makes available (this has * >>> been observed for Intel xl710, which reserves some of them for -- >>> 2.39.1 >>> >>> _______________________________________________ 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 fb0dd43f7..e901f857e 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1008,4 +1008,13 @@ dpdk_watchdog(void *dummy OVS_UNUSED) } +static int +dpdk_limit_desc_size(int desc_size, struct rte_eth_desc_lim *lim) +{ + desc_size = ROUND_UP(desc_size, lim->nb_align); + desc_size = MIN(desc_size, lim->nb_max); + desc_size = MAX(desc_size, lim->nb_min); + return desc_size; +} + static int dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) @@ -1056,4 +1065,7 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) } + dev->rxq_size = dpdk_limit_desc_size(dev->rxq_size, &info.rx_desc_lim); + dev->txq_size = dpdk_limit_desc_size(dev->txq_size, &info.tx_desc_lim); + /* A device may report more queues than it makes available (this has * been observed for Intel xl710, which reserves some of them for
By default OVS configures 2048 descriptors for tx and rx queues on DPDK devices. It also allows the user to configure those values. If the values used are not acceptable to the device than queue setup will fail. The device exposes it's max/min/alignment requirements, so use those to ensure that an acceptable value is used during queue setup. If the default or user value is not acceptable, adjust to a suitable value. Reported-at: https://bugzilla.redhat.com/2119876 Signed-off-by: Kevin Traynor <ktraynor@redhat.com> --- lib/netdev-dpdk.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)