[ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

Weglicki, MichalX michalx.weglicki at intel.com
Wed Apr 13 08:16:56 UTC 2016


-----Original Message-----
From: Panu Matilainen [mailto:pmatilai at redhat.com] 
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX <michalx.weglicki at intel.com>; dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add support for DPDK 16.04.

On 04/12/2016 05:05 PM, mweglicx wrote:
[...]
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index e09b471..2295e53 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev *netdev_,
>       link = dev->link;
>       ovs_mutex_unlock(&dev->mutex);
>
> -    if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
> +    if (link.link_duplex == ETH_LINK_AUTONEG) {

This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or 
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both 
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having 
autonegotiation end up with half-duplex these days isn't that likely.

[MW] Thank you, your're right, just to be sure, you mean that I should check autoneg through 
link_autoneg flag, like this: 
    if (link.link_autoneg) {
        *current = NETDEV_F_AUTONEG;
    } else 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;
        }
    }

Based on the comments in header files this should work. Thanks in advance. 

>           if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
>               *current = NETDEV_F_AUTONEG;
>           }

Additionally link_speed in DPDK 16.04 reflects the actual negotiated 
speed and is never ETH_LINK_SPEED_AUTONEG, which is a bitmap flag 
relevant to link_speeds field.

The autoneg case should be something like this:

@@ -1573,31 +1573,29 @@ netdev_dpdk_get_features(const struct netdev 
*netdev_,
      link = dev->link;
      ovs_mutex_unlock(&dev->mutex);

-    if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
-        if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
-            *current = NETDEV_F_AUTONEG;
-        }
+    if (link.link_autoneg) {
+       *current = NETDEV_F_AUTONEG;
      } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {


>       } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
> -        if (link.link_speed == ETH_LINK_SPEED_10) {
> +        if (link.link_speed == ETH_SPEED_NUM_10M) {
>               *current = NETDEV_F_10MB_HD;
>           }
> -        if (link.link_speed == ETH_LINK_SPEED_100) {
> +        if (link.link_speed == ETH_SPEED_NUM_100M) {
>               *current = NETDEV_F_100MB_HD;
>           }
> -        if (link.link_speed == ETH_LINK_SPEED_1000) {
> +        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_LINK_SPEED_10) {
> +        if (link.link_speed == ETH_SPEED_NUM_10M) {
>               *current = NETDEV_F_10MB_FD;
>           }
> -        if (link.link_speed == ETH_LINK_SPEED_100) {
> +        if (link.link_speed == ETH_SPEED_NUM_100M) {
>               *current = NETDEV_F_100MB_FD;
>           }
> -        if (link.link_speed == ETH_LINK_SPEED_1000) {
> +        if (link.link_speed == ETH_SPEED_NUM_1G) {
>               *current = NETDEV_F_1GB_FD;
>           }
> -        if (link.link_speed == ETH_LINK_SPEED_10000) {
> +        if (link.link_speed == ETH_SPEED_NUM_10G) {
>               *current = NETDEV_F_10GB_FD;
>           }
>       }

The rest looks ok.

As an aside, I've been thinking maybe this is a case where OVS could 
support both DPDK 2.2 and 16.04. I know its unprecedented but maybe that 
could change, restricting OVS to just one DPDK version seems 
unnecessarily strict when talking about differences this trivial.

	- Panu -
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.


More information about the dev mailing list