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

[Panu Matilainen]

On 04/13/2016 11:33 AM, Panu Matilainen wrote:
> On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:
>> -----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.
>
> Yup, that's what I meant. However turns out that isn't right either.
>
> Looking at lib/netdev-linux.c, *current is a bitfield with current speed
> set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I
> think this code was never quite correct to begin with.

To clarify, I think the DPDK code for this was never quite correct to 
begin with.

	- Panu -