[ovs-dev] [PATCH v1 1/2] netdev-dpdk: Fix netdev_dpdk_get_features().
Flavio Leitner
fbl at sysclose.org
Tue Oct 30 16:37:13 UTC 2018
On Thu, Oct 25, 2018 at 03:17:40PM +0100, Ian Stokes wrote:
> This commit fixes netdev_dpdk_get_features() by initializing a bitmap
> that represents current features to zero and accounting for non defined
> link speed values in the OpenFlow spec.
>
> The current approach for retrieving netdev dpdk features uses a
> pointer allocated in the stack without being initialized. As such there
> is no guarantee that the bitmap will be accurate. Fix this by declaring
> and initializing local variable 'feature' to be used when building the
> bitmap, with its value then assigned to the pointer. Also account for
> link speeds not defined in the OpenFlow spec by defaulting to
> NETDEV_F_OTHER for undefined link speeds.
>
> Fixes: 8a9562d21a40 ("dpif-netdev: Add DPDK netdev.")
> Signed-off-by: Ian Stokes <ian.stokes at intel.com>
> ---
> Flavio, this patch is based on suggestions from you
>
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-September/351809.html
>
> I'd like to add
>
> Co-authored-by: Flavio Leitner <fbl at sysclose.org>
> Signed-off-by: Flavio Leitner <fbl at sysclose.org>
>
> if you agree to sign off on this.
Yup, sounds good to me.
Thanks Ian!
fbl
> ---
> lib/netdev-dpdk.c | 65 ++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 40 insertions(+), 25 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index f91aa27cd..35eb30f8d 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2707,43 +2707,58 @@ netdev_dpdk_get_features(const struct netdev *netdev,
> {
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> struct rte_eth_link link;
> + uint32_t feature = 0;
>
> ovs_mutex_lock(&dev->mutex);
> link = dev->link;
> ovs_mutex_unlock(&dev->mutex);
>
> - if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
> - if (link.link_speed == ETH_SPEED_NUM_10M) {
> - *current = NETDEV_F_10MB_HD;
> - }
> - if (link.link_speed == ETH_SPEED_NUM_100M) {
> - *current = NETDEV_F_100MB_HD;
> - }
> - if (link.link_speed == ETH_SPEED_NUM_1G) {
> - *current = NETDEV_F_1GB_HD;
> - }
> - } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
> - if (link.link_speed == ETH_SPEED_NUM_10M) {
> - *current = NETDEV_F_10MB_FD;
> - }
> - if (link.link_speed == ETH_SPEED_NUM_100M) {
> - *current = NETDEV_F_100MB_FD;
> - }
> - if (link.link_speed == ETH_SPEED_NUM_1G) {
> - *current = NETDEV_F_1GB_FD;
> - }
> - if (link.link_speed == ETH_SPEED_NUM_10G) {
> - *current = NETDEV_F_10GB_FD;
> + if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
> + switch (link.link_speed) {
> + /* OpenFlow defined values: see enum ofp_port_features */
> + case ETH_SPEED_NUM_10M:
> + feature |= NETDEV_F_10MB_FD;
> + break;
> + case ETH_SPEED_NUM_100M:
> + feature |= NETDEV_F_100MB_FD;
> + break;
> + case ETH_SPEED_NUM_1G:
> + feature |= NETDEV_F_1GB_FD;
> + break;
> + case ETH_SPEED_NUM_10G:
> + feature |= NETDEV_F_10GB_FD;
> + break;
> + case ETH_SPEED_NUM_40G:
> + feature |= NETDEV_F_40GB_FD;
> + break;
> + case ETH_SPEED_NUM_100G:
> + feature |= NETDEV_F_100GB_FD;
> + break;
> + default:
> + feature |= NETDEV_F_OTHER;
> }
> - if (link.link_speed == ETH_SPEED_NUM_40G) {
> - *current = NETDEV_F_40GB_FD;
> + }
> + else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
> + switch (link.link_speed) {
> + case ETH_SPEED_NUM_10M:
> + feature |= NETDEV_F_10MB_HD;
> + break;
> + case ETH_SPEED_NUM_100M:
> + feature |= NETDEV_F_100MB_HD;
> + break;
> + case ETH_SPEED_NUM_1G:
> + feature |= NETDEV_F_1GB_HD;
> + break;
> + default:
> + feature |= NETDEV_F_OTHER;
> }
> }
>
> if (link.link_autoneg) {
> - *current |= NETDEV_F_AUTONEG;
> + feature |= NETDEV_F_AUTONEG;
> }
>
> + *current = feature;
> *advertised = *supported = *peer = 0;
>
> return 0;
> --
> 2.13.6
>
--
Flavio
More information about the dev
mailing list